fix(runtime): stop gating pi tool calls; Co-Founder approvals live at API layer
The Co-Founder beforeToolCall hook (introduced in 77f81a7) gated every
non-readonly pi tool call as a `dangerous_command` approval. Because
agents call `POST /api/companies/:id/messages`, `/agents` (hire), and
every other company mutation via bash→node/curl, the first bash call in
every Co-Founder run got parked on a 30-minute approval timeout. Users
observed it as "agents never talk to each other" and "every bash command
waits for approval"; both symptoms are the same bug.
The business-level approval flow (`hire_agent`, `document_review`,
`plan_review`, etc. in execution-policy.ts) already gates the events
users actually care about, at the HTTP route layer where the event
originates (routes-agents.ts:368 for hiring, document routes for
deliverables). Stacking a pi-level hook on top was duplicate gating with
the wrong taxonomy.
Removed:
- src/main/agent-runtime/approval-hook.ts (entire file, 137 LOC)
- `beforeToolCall` conditional in worker-runner.ts and the
corresponding RPC method in worker-types.ts and runtime-bridge.ts
- `approvalHook` field + `resolveApproval` callback + shutdown
`cancelAll` in runtime-bridge.ts / service-graph-pi-runtime.ts /
wire-api-server.ts
Unchanged:
- `dangerous_command` approval type itself (still used by automation
workflows and routes-misc.ts)
- CEO mode (already never attached the hook)
- All business-level approval types and their wiring
This commit is contained in:
1 parent
4cfb926971
commit
02ba887610
7 files changed
+8
-200
No files matched your search
@@ -1,136 +0,0 @@
|
||||
import type { AppDatabase } from "@main/database";
|
||||
import type { DesktopEvent } from "@shared/types";
|
||||
|
||||
interface BeforeToolCallPayload {
|
||||
agentId: string;
|
||||
companyId: string;
|
||||
taskId: string | null;
|
||||
toolCallId: string;
|
||||
toolName: string;
|
||||
args: unknown;
|
||||
}
|
||||
|
||||
interface PendingApproval {
|
||||
resolve: (verdict: { block?: boolean; reason?: string }) => void;
|
||||
}
|
||||
|
||||
export interface HumanApprovalHookDeps {
|
||||
db: AppDatabase;
|
||||
emitEvent: (event: DesktopEvent) => void;
|
||||
logger: {
|
||||
info: (...args: unknown[]) => void;
|
||||
warn: (...args: unknown[]) => void;
|
||||
error: (...args: unknown[]) => void;
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Tools whose side-effects are read-only (safe to auto-approve in Co-Founder
|
||||
* mode). Anything outside this set waits for a human decision.
|
||||
*
|
||||
* This is intentionally conservative: `bash` is gated even for obviously
|
||||
* safe commands like `ls`, because parsing shell syntax for safety is its
|
||||
* own class of bug. If users want per-command auto-approval they can
|
||||
* approve once and the run proceeds; the alternative is a false sense of
|
||||
* security.
|
||||
*/
|
||||
const AUTO_APPROVED_TOOLS = new Set<string>([
|
||||
"read",
|
||||
"ls",
|
||||
"glob",
|
||||
"grep",
|
||||
"find",
|
||||
"tree",
|
||||
]);
|
||||
|
||||
function summarizeArgs(args: unknown, maxLen = 240): string {
|
||||
if (args === undefined || args === null) return "";
|
||||
try {
|
||||
const json = typeof args === "string" ? args : JSON.stringify(args);
|
||||
return json.length <= maxLen ? json : `${json.slice(0, maxLen - 1)}…`;
|
||||
} catch {
|
||||
return "<unserialisable>";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Gates pi `beforeToolCall` invocations against the Human-in-the-Loop
|
||||
* approval system used by AI Co-Founder mode.
|
||||
*
|
||||
* Contract:
|
||||
* - Tools in AUTO_APPROVED_TOOLS resolve synchronously with `{}` (no-op).
|
||||
* - Everything else creates a `dangerous_command` approval row, emits an
|
||||
* `approval-updated` event so the renderer can surface it, and blocks
|
||||
* the worker RPC until the human resolves the approval.
|
||||
* - Resolutions flow in via `resolveApproval` from the approval-resolution
|
||||
* pipeline in `wire-api-server.ts` (shared with automation + thread).
|
||||
* - On shutdown, `cancelAll` releases every pending wait with a block
|
||||
* verdict so the worker can exit cleanly.
|
||||
*
|
||||
* In AI CEO mode the worker never attaches `beforeToolCall` to the pi
|
||||
* Agent, so this class is instantiated but sees zero traffic.
|
||||
*/
|
||||
export class HumanApprovalHook {
|
||||
private readonly pendingByApproval = new Map<string, PendingApproval>();
|
||||
|
||||
constructor(private readonly deps: HumanApprovalHookDeps) {}
|
||||
|
||||
async before(
|
||||
runId: string,
|
||||
payload: BeforeToolCallPayload,
|
||||
): Promise<{ block?: boolean; reason?: string }> {
|
||||
if (AUTO_APPROVED_TOOLS.has(payload.toolName)) return {};
|
||||
|
||||
const summary = `${payload.toolName}: ${summarizeArgs(payload.args, 160)}`;
|
||||
const approvalId = this.deps.db.requestApproval({
|
||||
companyId: payload.companyId,
|
||||
relatedTaskId: payload.taskId ?? null,
|
||||
requestedByAgentId: payload.agentId,
|
||||
type: "dangerous_command",
|
||||
payloadSummary: summary,
|
||||
impactSummary: "Agent requested a gated tool call; Co-Founder mode requires a human decision.",
|
||||
payloadJson: JSON.stringify({
|
||||
runId,
|
||||
toolCallId: payload.toolCallId,
|
||||
toolName: payload.toolName,
|
||||
args: payload.args,
|
||||
}),
|
||||
});
|
||||
|
||||
this.deps.emitEvent({ type: "approval-updated", approvalId, action: "created" });
|
||||
this.deps.logger.info(
|
||||
`[approval-hook] gated ${payload.toolName} for agent ${payload.agentId} (approval ${approvalId})`,
|
||||
);
|
||||
|
||||
return new Promise<{ block?: boolean; reason?: string }>((resolve) => {
|
||||
this.pendingByApproval.set(approvalId, { resolve });
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Called by the approval-resolution pipeline in `wire-api-server.ts`
|
||||
* for every approval decision (automation, UI, HTTP). No-ops when the
|
||||
* approvalId is not one we are waiting on.
|
||||
*/
|
||||
resolveApproval(approvalId: string, decision: string): void {
|
||||
const pending = this.pendingByApproval.get(approvalId);
|
||||
if (!pending) return;
|
||||
this.pendingByApproval.delete(approvalId);
|
||||
if (decision === "approved") {
|
||||
pending.resolve({});
|
||||
} else {
|
||||
pending.resolve({ block: true, reason: `tool call ${decision} by human reviewer` });
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Releases every pending wait with a block verdict. Called on worker
|
||||
* shutdown so in-flight RPC promises resolve and the worker can exit.
|
||||
*/
|
||||
cancelAll(reason: string): void {
|
||||
for (const [approvalId, pending] of this.pendingByApproval) {
|
||||
pending.resolve({ block: true, reason });
|
||||
this.pendingByApproval.delete(approvalId);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1,4 +1,3 @@
|
||||
export { SqliteSessionStore } from "./sqlite-session-store";
|
||||
export { CostWriter } from "./cost-writer";
|
||||
export { HumanApprovalHook } from "./approval-hook";
|
||||
export { RuntimeBridge } from "./runtime-bridge";
|
||||
@@ -4,7 +4,6 @@ import type { DesktopEvent } from "@shared/types";
|
||||
import { ensureTaskThread } from "@main/thread-integration/thread-helpers";
|
||||
import { SqliteSessionStore } from "./sqlite-session-store";
|
||||
import { CostWriter } from "./cost-writer";
|
||||
import { HumanApprovalHook } from "./approval-hook";
|
||||
import type { CredentialResolver } from "@main/credentials";
|
||||
import { getConnectorDefinition } from "@main/connectors";
|
||||
import { projectInboxMessageToPi } from "./inbox-projection";
|
||||
@@ -133,23 +132,19 @@ interface ActiveRunContext {
|
||||
* - Forwards `agent-event` from the worker to the renderer (push channel).
|
||||
* - Persists each `message_end` message via `SqliteSessionStore`.
|
||||
* - Calls `CostWriter` for assistant messages with usage.
|
||||
* - Hosts `HumanApprovalHook` for Co-Founder mode tool gating; the worker
|
||||
* only invokes the hook RPC when running in `ai_cofounder` mode, so
|
||||
* CEO-mode runs pay zero IPC overhead.
|
||||
*
|
||||
* Co-Founder mode's human-review gates live at the API endpoint layer
|
||||
* (hire_agent / document_review approvals created by `routes-*.ts`), not
|
||||
* at the pi tool-call layer; the worker constructs `Agent` without a
|
||||
* `beforeToolCall` hook in either mode.
|
||||
*/
|
||||
export class RuntimeBridge {
|
||||
readonly approvalHook: HumanApprovalHook;
|
||||
private readonly costWriter: CostWriter;
|
||||
private readonly activeRuns = new Map<string, ActiveRunContext>();
|
||||
private readonly sessionStores = new Map<string, SqliteSessionStore>();
|
||||
|
||||
constructor(private readonly deps: RuntimeBridgeDeps) {
|
||||
this.costWriter = new CostWriter({ db: deps.db });
|
||||
this.approvalHook = new HumanApprovalHook({
|
||||
db: deps.db,
|
||||
emitEvent: deps.emitEvent,
|
||||
logger: deps.logger,
|
||||
});
|
||||
}
|
||||
|
||||
private registerActiveRun(context: ActiveRunContext): void {
|
||||
@@ -299,7 +294,7 @@ export class RuntimeBridge {
|
||||
/** Handles RPC requests issued by the worker (`rpc-request` envelope). */
|
||||
async handleRpc(request: {
|
||||
rpcId: string;
|
||||
method: "persistMessage" | "prepareRun" | "beforeToolCall";
|
||||
method: "persistMessage" | "prepareRun";
|
||||
runId: string;
|
||||
payload: unknown;
|
||||
}): Promise<{ ok: true; result: unknown } | { ok: false; error: string }> {
|
||||
@@ -309,18 +304,6 @@ export class RuntimeBridge {
|
||||
this.getOrCreateStore(payload.taskSessionId).appendMessage(payload.message);
|
||||
return { ok: true, result: undefined };
|
||||
}
|
||||
if (request.method === "beforeToolCall") {
|
||||
const payload = request.payload as {
|
||||
agentId: string;
|
||||
companyId: string;
|
||||
taskId: string | null;
|
||||
toolCallId: string;
|
||||
toolName: string;
|
||||
args: unknown;
|
||||
};
|
||||
const verdict = await this.approvalHook.before(request.runId, payload);
|
||||
return { ok: true, result: verdict };
|
||||
}
|
||||
if (request.method === "prepareRun") {
|
||||
const payload = request.payload as {
|
||||
connectorId: string;
|
||||
|
||||
Reference in new issue
Block a user