Cover duplicate parked calls and document the heuristic-only lookup

Author: TheGreatAxios

src/exec/runner.ts

@@ -976,6 +976,8 @@ export async function runExec(config: Config): Promise<ExecResult> {
       const sendResult = await activeAgent.send(operatorTaskMessage(task));
       // A suspension must not park silently in exec: the approval resume owns
       // the terminal prompt flow and delivers the decision to the reactor.
+      // No resolveParkedCallId: the vendored reactor exposes no
+      // correlationId-to-call lookup, so the history heuristic is the path.
       await createApprovalResume({
         getAgent: () => activeAgent,
         gate: permissionGate,

src/session/approval-resume.test.ts

@@ -165,4 +165,48 @@ describe("approval-resume parallel-parked approvals", () => {
     expect(deliveredCorrelationId(message)).toBe("corr-A");
     expect(decisionBody(message).outcome).toBe("approved");
   });
+
+  test("identical name+args twin: sibling timeout still delivers", async () => {
+    const { resume, delivered } = setup({
+      preTurns: [
+        assistantTurn([
+          { id: "call-A", name: "run_shell", command: "echo same" },
+          { id: "call-B", name: "run_shell", command: "echo same" },
+        ]),
+      ],
+      onGate: (turns) => {
+        turns.push(timeoutTurn("call-B"));
+      },
+    });
+
+    const handled = await resume.handle(suspension("corr-A", "echo same"));
+
+    expect(handled).toBe(true);
+    expect(delivered).toHaveLength(1);
+  });
+
+  // Known limitation of the exact-one history derivation: with identical
+  // name+args twins, the answered own call drops out of the candidates and
+  // the derivation resolves to the unanswered twin, so the settled check
+  // misses and the genuinely-late decision is delivered. Telling the twins
+  // apart needs the resolveParkedCallId lookup; this test locks the current
+  // shape so a future fix can flip it to a drop.
+  test("identical name+args twin: own timeout with unanswered twin delivers", async () => {
+    const { resume, delivered } = setup({
+      preTurns: [
+        assistantTurn([
+          { id: "call-A", name: "run_shell", command: "echo same" },
+          { id: "call-B", name: "run_shell", command: "echo same" },
+        ]),
+      ],
+      onGate: (turns) => {
+        turns.push(timeoutTurn("call-A"));
+      },
+    });
+
+    const handled = await resume.handle(suspension("corr-A", "echo same"));
+
+    expect(handled).toBe(true);
+    expect(delivered).toHaveLength(1);
+  });
 });

src/session/approval-resume.ts

@@ -78,23 +78,24 @@ export function requestFromApprovalSnapshot(
 // along and match block.callId; without it the text-only scan stays as the
 // fallback so a genuinely late decision is still dropped.
 
-function settledAfterSuspend(
+function timeoutResultAfterSuspend(
   turns: Awaited<ReturnType<Agent["history"]>>,
   fromIndex: number,
   parkedCallId: string | undefined,
-): boolean {
-  return turns
-    .slice(fromIndex)
-    .flatMap((turn) => turn.content)
-    .some(
-      (block) =>
-        block.type === "tool_result" &&
-        (parkedCallId === undefined || block.callId === parkedCallId) &&
-        block.content.some(
-          (part) =>
-            part.type === "text" && part.text === APPROVAL_TIMEOUT_RESULT_TEXT,
-        ),
-    );
+): string | undefined {
+  for (const block of turns.slice(fromIndex).flatMap((turn) => turn.content)) {
+    if (
+      block.type === "tool_result" &&
+      (parkedCallId === undefined || block.callId === parkedCallId) &&
+      block.content.some(
+        (part) =>
+          part.type === "text" && part.text === APPROVAL_TIMEOUT_RESULT_TEXT,
+      )
+    ) {
+      return block.callId;
+    }
+  }
+  return undefined;
 }
 
 function stableArgs(value: unknown): string {
@@ -111,10 +112,18 @@ function stableArgs(value: unknown): string {
 
 // Derive this suspension's parked call id from history: the pre-watermark
 // tool_call matching the snapshot's name and arguments that has no tool
-// result anywhere yet. A parked call is neither run nor answered, so the only
-// unanswered match is this suspension's own call; a timed-out sibling is
-// answered by its timeout result and drops out of the candidates. Returns
-// undefined unless exactly one candidate matches.
+// result anywhere yet. A parked call is neither run nor answered, so an
+// unanswered match is normally this suspension's own call; a timed-out
+// sibling is answered by its timeout result and drops out of the candidates.
+// Returns undefined unless exactly one candidate matches.
+//
+// Known limitation: identical name+args twins are ambiguous. When this
+// suspension's own timeout fires while an identical twin sits unanswered,
+// the twin is the exact-one survivor, so the settled check misses and the
+// late decision is delivered instead of dropped. The sibling-timeout mirror
+// (identical args, sibling answered) still resolves to the live call and
+// delivers. Telling identical twins apart needs the resolveParkedCallId
+// lookup below; the history heuristic cannot do it.
 function parkedCallIdFromHistory(
   turns: Awaited<ReturnType<Agent["history"]>>,
   fromIndex: number,
@@ -193,6 +202,14 @@ export function createApprovalResume(args: {
   // the minted correlationId, so this is what ties them together. Takes
   // precedence over the history derivation below; absent callers fall back
   // to it.
+  //
+  // Heuristic-only in production: neither the exec nor the TUI caller wires
+  // this, because the vendored reactor surface ({ start, deliver, abort })
+  // exposes no correlationId-to-call lookup (see
+  // permission/decline-markers.ts), and the suspension snapshot carries
+  // name+arguments without the call id. Wire this if upstream ever exports
+  // the pending-operation lookup; until then the history derivation is the
+  // live path.
   resolveParkedCallId?: (correlationId: string) => string | undefined;
   gate: PermissionGate;
 }): ApprovalResume {
@@ -290,11 +307,18 @@ export function createApprovalResume(args: {
         const parkedCallId =
           args.resolveParkedCallId?.(correlationId) ??
           parkedCallIdFromHistory(history, turnsAtSuspend, approvalSnapshot);
-        if (settledAfterSuspend(history, turnsAtSuspend, parkedCallId)) {
+        const matchedTimeoutCallId = timeoutResultAfterSuspend(
+          history,
+          turnsAtSuspend,
+          parkedCallId,
+        );
+        if (matchedTimeoutCallId !== undefined) {
           // The reactor already answered the parked call (its approval timeout
           // fired while the surface was still up). Delivering now would append
           // the raw decision JSON as an uncorrelated user turn — drop and log.
-          logger.warn`late approval decision dropped correlation=${correlationId} outcome=${outcome?.allow === true ? "approved" : "rejected"}`;
+          // The matched call id rides along so a fallback drop (parkedCallId
+          // undefined) can still be attributed to the timeout that caused it.
+          logger.warn`late approval decision dropped correlation=${correlationId} timeoutCall=${matchedTimeoutCallId} outcome=${outcome?.allow === true ? "approved" : "rejected"}`;
           return true;
         }
         if (outcome === undefined || !outcome.allow) {

src/tui/runner/session.ts

@@ -495,6 +495,8 @@ export async function assembleTUISession(
   // Reload, interrupt, compaction continuation, and proxy deliver share one queue
   // so a rebuild never races an in-flight deliver.
   const sessionOps = createSessionOperationQueue();
+  // No resolveParkedCallId: the vendored reactor exposes no
+  // correlationId-to-call lookup, so the history heuristic is the path.
   const approvalResume = createApprovalResume({
     getAgent: () => state.currentAgent,
     captureGeneration: deliveryGeneration.capture,