From 734a2b04e06894f486af347a1e1291c3ae8a845e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sun, 19 Apr 2026 13:20:14 -0400 Subject: [PATCH] fix(runtime): narrow beforeToolCall gate to operational approvals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous Task 2 pass gated every destructive tool on any blocking approval or any open wait-state for the agent. That was too coarse: a pending deliverable_review or approve_ceo_strategy (both decision- type approvals, waiting for a human to respond to completed or proposed work) froze the agent's entire toolchain — including the `bash` call the agent uses to POST /api/messages. Result: CEO talked, employees woke on heartbeat, tried to reply, the message POST was blocked as a "destructive tool", the run ended silent. Every reply collapsed to an empty heartbeat. Two corrections land together: 1. Approval gate narrows to the five approval types that actually gate EXECUTION — `dangerous_command`, `secret_access`, `production_deploy`, `social_post`, `budget_override`. Review / decision types are ignored at the tool layer; they remain visible in the board UI and still block via the heartbeat scheduler when the execution mode demands it. 2. Wait-state check leaves the hook entirely. The heartbeat scheduler (`checkpoint-engine.shouldPauseHeartbeat`) is the single owner of "agent should pause because a thread is waiting" — mirroring the rule at tool level produced the same company-wide freeze for any reply on any thread. Heartbeat is the right scope; tools are not. The destructive-tool set and the operational-approval set are both named, readonly, and centralised so the next addition reads like a whitelist instead of a patch. --- src/main/agent-runtime/hook-handlers.ts | 83 ++++++++++++------------- 1 file changed, 40 insertions(+), 43 deletions(-) diff --git a/src/main/agent-runtime/hook-handlers.ts b/src/main/agent-runtime/hook-handlers.ts index 0a957c38..c655e6bd 100644 --- a/src/main/agent-runtime/hook-handlers.ts +++ b/src/main/agent-runtime/hook-handlers.ts @@ -4,18 +4,17 @@ * `afterToolCall`, or `transformContext`. * * Each handler is deliberately isolated to one concern: - * - `beforeToolCall`: approval-gate policy (blocks destructive tools - * while an approval or wait state is outstanding) + * - `beforeToolCall`: operational-approval gate (blocks destructive + * tools only while an approval that gates EXECUTION is pending) * - `afterToolCall`: filesystem artifact projection (bash/write/edit * outputs become `work_products`) - * - `transformContext`: auto-compaction hook (pruning in future; today - * passes through) + * - `transformContext`: per-LLM-call auto-compaction via a pure + * reducer * - * Keeping them in this module pulls policy out of `RuntimeBridge` - * (which stays a pure IPC router) and keeps the per-file budget under - * the project's 500-line cap even as policy grows. + * Policy lives here; `RuntimeBridge` stays a pure IPC router. */ +import type { ApprovalType } from "@shared/types"; import type { AppDatabase } from "@main/database"; import type { AfterToolCallRpcPayload, @@ -68,13 +67,34 @@ export function isDestructiveTool(toolName: string): boolean { } /** - * Approval + wait-state gate for destructive tools. Runs at every - * `beforeToolCall` callback, so approvals or wait states created - * mid-run take effect on the next tool call without any cross-process - * cache to invalidate. + * Approval types that gate EXECUTION (not decision). These stop the + * destructive toolchain until the board releases the hold. * - * Read-only tools (read / grep / glob / list) are always allowed — - * gating them would only slow observation while adding no safety. + * Decision-type approvals (deliverable_review, plan_review, hire_agent, + * …) are deliberately excluded: they mean "a human owes an answer + * about completed or proposed work". The agent is still free to reply + * to messages, coordinate with peers, or work on unrelated tasks + * while the decision is pending — blocking their tools there turns + * the board-review pattern into a company-wide freeze. + */ +const OPERATIONAL_APPROVAL_TYPES: ReadonlySet = new Set([ + "dangerous_command", + "secret_access", + "production_deploy", + "social_post", + "budget_override", +]); + +/** + * Approval gate for destructive tools. Runs at every + * `beforeToolCall`, so approvals created mid-run take effect on the + * next tool call with no cross-process cache to invalidate. + * + * Heartbeat-level wait-state pausing is the responsibility of the + * scheduler (`checkpoint-engine.shouldPauseHeartbeat`); mirroring it + * here would freeze every outbound message and every coordination + * reply while an unrelated thread was still waiting on the user — + * the exact failure mode that made Co-Founder agents silent. */ export function handleBeforeToolCall( deps: HookHandlerDeps, @@ -84,24 +104,20 @@ export function handleBeforeToolCall( if (!isDestructiveTool(payload.toolName)) { return { block: false, reason: null }; } - - const approvalBlock = checkApprovalBlock(deps, ctx, payload.toolName); - if (approvalBlock) return approvalBlock; - - const waitStateBlock = checkWaitStateBlock(deps, ctx, payload.toolName); - if (waitStateBlock) return waitStateBlock; - - return { block: false, reason: null }; + return checkOperationalApprovalBlock(deps, ctx, payload.toolName); } -function checkApprovalBlock( +function checkOperationalApprovalBlock( deps: HookHandlerDeps, ctx: ActiveRunContext, toolName: string, -): BeforeToolCallRpcResult | null { +): BeforeToolCallRpcResult { try { const blocking = deps.db.getBlockingApproval(ctx.companyId, ctx.agentId, ctx.taskId); - if (!blocking) return null; + if (!blocking) return { block: false, reason: null }; + if (!OPERATIONAL_APPROVAL_TYPES.has(blocking.type as ApprovalType)) { + return { block: false, reason: null }; + } return { block: true, reason: `Approval ${blocking.id} (${blocking.type}, state=${blocking.state}) is outstanding: ${blocking.payloadSummary}. Wait for the decision before retrying ${toolName}.`, @@ -119,25 +135,6 @@ function checkApprovalBlock( } } -function checkWaitStateBlock( - deps: HookHandlerDeps, - ctx: ActiveRunContext, - toolName: string, -): BeforeToolCallRpcResult | null { - try { - if (!deps.db.hasUnresolvedWaitStateForAgent(ctx.companyId, ctx.agentId)) return null; - return { - block: true, - reason: `Thread wait state is open for this agent — ${toolName} is paused until the user replies or the review resolves.`, - }; - } catch (error) { - deps.logger.warn( - `[hook-handlers] wait-state lookup failed for agent ${ctx.agentId}: ${error instanceof Error ? error.message : String(error)}`, - ); - return null; - } -} - /** * Post-execution projection. Today only filesystem artifacts flow * through this hook; as further projections (browser actions, social