From 795008f56bd449112d881408be4879338a49ea30 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?=
 <zhangyanghaha0407@outlook.com>
Date: Sun, 19 Apr 2026 00:51:54 -0400
Subject: [PATCH] fix(runtime): enforce approval + wait-state gate in
 beforeToolCall
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Approval creation already flowed through the API server, but pi's
`bash` / `write` / `edit` tools never traversed that layer, so the
gate was purely advisory: the agent kept mutating the workspace while
the board tried to hold it. The fix: policy runs at the one place
every destructive tool invocation must pass, pi's `beforeToolCall`.

`checkApprovalBlock` fails closed on lookup errors — an unreadable
approvals table is a safety incident, not a licence to execute. It
re-reads `getBlockingApproval` on every call so an approval created
mid-run takes effect on the next tool without any cache.

`checkWaitStateBlock` rides the unified
`hasUnresolvedWaitStateForAgent` query so task, approval, and
freeform thread wait states all gate the same tools. Read-only tools
are untouched; only bash/write/edit are gated because gating
observation would slow investigation without adding safety.

The destructive-tool set is promoted to a named predicate so the
Task 5 artifact projector can share the same definition when it
extends to bash outputs.
---
 src/main/agent-runtime/hook-handlers.ts | 66 +++++++++++++++++++++----
 1 file changed, 56 insertions(+), 10 deletions(-)

diff --git a/src/main/agent-runtime/hook-handlers.ts b/src/main/agent-runtime/hook-handlers.ts
index 723be0ea..75269486 100644
--- a/src/main/agent-runtime/hook-handlers.ts
+++ b/src/main/agent-runtime/hook-handlers.ts
@@ -67,9 +67,13 @@ export function isDestructiveTool(toolName: string): boolean {
 }
 
 /**
- * Approval gate. A pending approval for this agent's current task
- * blocks every destructive tool — the agent must wait for the board
- * (or user) to resolve the approval before mutating anything.
+ * 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.
+ *
+ * Read-only tools (read / grep / glob / list) are always allowed —
+ * gating them would only slow observation while adding no safety.
  */
 export function handleBeforeToolCall(
   deps: HookHandlerDeps,
@@ -80,17 +84,59 @@ export function handleBeforeToolCall(
     return { block: false, reason: null };
   }
 
-  const blocking = deps.db.getBlockingApproval(ctx.companyId, ctx.agentId, ctx.taskId);
-  if (blocking) {
-    return {
-      block: true,
-      reason: `Approval ${blocking.id} (${blocking.state}) is outstanding: ${blocking.payloadSummary}. Wait for the board to decide before retrying ${payload.toolName}.`,
-    };
-  }
+  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 };
 }
 
+function checkApprovalBlock(
+  deps: HookHandlerDeps,
+  ctx: ActiveRunContext,
+  toolName: string,
+): BeforeToolCallRpcResult | null {
+  try {
+    const blocking = deps.db.getBlockingApproval(ctx.companyId, ctx.agentId, ctx.taskId);
+    if (!blocking) return null;
+    return {
+      block: true,
+      reason: `Approval ${blocking.id} (${blocking.type}, state=${blocking.state}) is outstanding: ${blocking.payloadSummary}. Wait for the decision before retrying ${toolName}.`,
+    };
+  } catch (error) {
+    // Fail closed: an unreadable approval table is a safety incident,
+    // not a reason to run destructive code.
+    deps.logger.error(
+      `[hook-handlers] approval lookup failed for agent ${ctx.agentId}: ${error instanceof Error ? error.message : String(error)}`,
+    );
+    return {
+      block: true,
+      reason: `Approval lookup failed — ${toolName} is held until the approval subsystem recovers.`,
+    };
+  }
+}
+
+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