fix(runtime): narrow beforeToolCall gate to operational approvals
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.
This commit is contained in:
ZacharyZhang-NY committed
1 parent
f976b59b8f
commit
734a2b04e0
1 file changed
+40
-43
@@ -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<ApprovalType> = new Set<ApprovalType>([
|
||||
"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
|
||||
|
||||
Reference in new issue
Block a user