fix(tui): keep leftover same-name collect under the stall budget

docs/ARCHITECTURE.md

@@ -422,7 +422,7 @@ tool call
 
 **Approval log** (`src/permission/approval-log.ts`, CL-5666): every consequential decision the gate makes — auto-mode allow/deny or an interactive prompt's allow-once/allow-with-scope/deny/timeout/abort — is appended as one JSONL record to `approvals.jsonl` in the session dir, carrying the classifier/auto-shell rule name that fired (the existing `auto-shell-policy.ts`/`classify.ts` rule names, plus a small closed set of additional fixed literals the log itself defines for decisions those modules don't otherwise name — `auto-allowed-tool`, `non-interactive`, `mega-chain` — never model- or user-authored text), whether the decision was `auto` or `interactive`, a shell chain's segment count, and queued/displayed/settled timestamps. `displayedAt` is set by `PermissionRequest.markDisplayed`, called from `gate-wire.ts`'s `open()` the moment a request actually reaches the overlay host — distinct from when it was raised, so the gap it exposes is the CL-5664 signal (a queued gate arming its timeout before the operator could see it). No command text, file content, path, credential, or other free text is ever recorded — only tool name, rule, mode, segment count, and timing; a sub-agent's free-text dispatch label is deliberately left out, even though it would enable a per-agent breakdown, because nothing constrains what a model puts in it. A hard size cap on the serialized line is defense in depth against a future field reintroducing free text. Writes are fire-and-forget and swallow their own errors; the log defaults to a no-op so nothing depends on it being wired. `scripts/approval-forensics.ts` aggregates across local sessions the same way `intervention-forensics.ts` does for stop/nudge events: per-tool counts by outcome and mode, duration/display-delay percentiles, mega-chain counts, and a duplicate-rate proxy (sessions that hit the same rule more than once).
 
-**Tool wall-clock budget vs. permission prompts.** Each tool `run()` is wrapped by an outer execution watchdog (`src/tui/tool-execution-watchdog.ts`). The watchdog arms when Settings set `tools.timeoutMs` / `tools.maxTimeoutMs`, when foreground `run_shell` has an effective timeout (120s default or per-call, plus 1000ms slack so this layer cannot beat shell-guard), or for `mcp__*` calls. Fleet wait tools are exempt, regardless of Settings: the generic per-tool budget never aborts a sub-agent run while the parent waits for it. That exemption is unconditional, not because the worker is otherwise bounded — there is no turn budget; `deadlineMs` is opt-in, and there is no no-progress or thrash stop. A stuck worker that never trips stall or deadline still runs until the parent turn's stall budget fires, the operator cancels, or, in eval mode, `--agent-timeout-ms` bounds it. `spawn_agent`, `wait_agents`, `ask_director`, `shell_collect`, and `run_shell` with `background: true` stay exempt from the per-tool watchdog. On TUI, an in-flight `shell_collect` or `ask_director` is still bounded by the stall watchdog — including after a tool batch resolves and after compact continuation re-entry — so a typical poll does not pin the turn. Concurrent same-name `shell_collect` calls still collide in `callIdByName` (one slot per name); a sibling collect finishing can drop the mapping and the remaining poll can lose that stall bound. That residual lives in the name-keyed tracker (`src/tui/turn-state.ts`), not in the post-tool / post-compact abort itself. `wait_agents` is exec-primary only and is not mounted on TUI. By default (`tools.waitForApproval`, Settings → Tools, **On**), an armed budget freezes while the operator is deciding on a permission prompt, so a late approve still runs the tool and the agent waits for the decision instead of timing out under the modal. When **Off**, the budget keeps ticking during the prompt; if it expires first the tool is skipped and the permission modal is dismissed via the budget AbortSignal (auto-deny with a timeout message). The TUI permission queue (`src/tui/gate-wire.ts`, backed by `src/permission/queue.ts`) attaches that signal so ghost prompts cannot outlive an already-aborted tool.
+**Tool wall-clock budget vs. permission prompts.** Each tool `run()` is wrapped by an outer execution watchdog (`src/tui/tool-execution-watchdog.ts`). The watchdog arms when Settings set `tools.timeoutMs` / `tools.maxTimeoutMs`, when foreground `run_shell` has an effective timeout (120s default or per-call, plus 1000ms slack so this layer cannot beat shell-guard), or for `mcp__*` calls. Fleet wait tools are exempt, regardless of Settings: the generic per-tool budget never aborts a sub-agent run while the parent waits for it. That exemption is unconditional, not because the worker is otherwise bounded — there is no turn budget; `deadlineMs` is opt-in, and there is no no-progress or thrash stop. A stuck worker that never trips stall or deadline still runs until the parent turn's stall budget fires, the operator cancels, or, in eval mode, `--agent-timeout-ms` bounds it. `spawn_agent`, `wait_agents`, `ask_director`, `shell_collect`, and `run_shell` with `background: true` stay exempt from the per-tool watchdog. On TUI, an in-flight `shell_collect` or `ask_director` is still bounded by the stall watchdog — including after a tool batch resolves and after compact continuation re-entry — so a typical poll does not pin the turn. Concurrent same-name `shell_collect` calls still share one `callIdByName` slot (one id per name); the leftover stays named in the per-id `callNameById` map so the stall watchdog still bounds it after the mapping-owning sibling finishes. `wait_agents` is exec-primary only and is not mounted on TUI. By default (`tools.waitForApproval`, Settings → Tools, **On**), an armed budget freezes while the operator is deciding on a permission prompt, so a late approve still runs the tool and the agent waits for the decision instead of timing out under the modal. When **Off**, the budget keeps ticking during the prompt; if it expires first the tool is skipped and the permission modal is dismissed via the budget AbortSignal (auto-deny with a timeout message). The TUI permission queue (`src/tui/gate-wire.ts`, backed by `src/permission/queue.ts`) attaches that signal so ghost prompts cannot outlive an already-aborted tool.
 
 `mcp__*` tool calls are the exception to "arms only when Settings set it": they arm unconditionally with a 5-minute default (`DEFAULT_MCP_TOOL_TIMEOUT_MS`), overridable via `mcp.timeoutMs` and still capped by `tools.maxTimeoutMs` (CL-6895). Nothing else bounds an MCP call — the stall watchdog treats an in-flight tool as activity by design, so a wedged MCP server previously hung a tool call, and the turn, forever. On expiry the call returns a normal tool-error result ("MCP tool `<name>` timed out after `<n>`s — the server may be wedged; retry or continue without it"); the turn is never aborted. The MCP client itself (`src/mcp/client.ts`, wrapping `@modelcontextprotocol/sdk`) multiplexes concurrent requests over one connection by JSON-RPC message id with no serial queue or mutex in our code or in the vendored SDK's `Protocol.request()` — so concurrent calls to the same server are not expected to deadlock each other. Live forensics for CL-6895 showed multi-minute MCP calls that eventually completed successfully, consistent with a slow server response rather than a client-side deadlock.
 

docs/IMPLEMENTATION.md

@@ -288,7 +288,7 @@ Provider and model configuration lives in JSON settings files. The global file h
   }
   ```
 
-  - `timeoutMs` / `maxTimeoutMs` — outer execution watchdog around each tool `run()`. Unset leaves the generic watchdog unarmed; set these to arm it. `maxTimeoutMs` clamps non-shell tools when set and does not cap a longer requested `run_shell`. Foreground `run_shell` always arms at the effective shell timeout (120s default, `settings.shell.timeoutMs` override, or per-call) plus 1000ms slack. Fleet wait tools are exempt: a dispatched sub-agent is bounded by stall, opt-in `deadlineMs`, and operator cancel, not the generic per-tool budget. Background shell is exempt too: a `run_shell` with `background: true` arms nothing (only a per-call timeout bounds the process; there is no 120s default) and `shell_collect` never arms the per-tool watchdog (a bounded poll over a process that outlives the turn). An in-flight TUI `shell_collect` or `ask_director` is still bounded by the TUI stall watchdog, including after a tool batch resolves and after compact continuation re-entry, so a typical poll does not pin the turn. Concurrent same-name `shell_collect` calls still collide in `callIdByName` (one slot per name); a sibling collect finishing can drop the mapping and the remaining poll can lose that stall bound — a residual of the name-keyed tracker, not a hole in the post-tool / post-compact abort. `wait_agents` is exec-primary only and is not mounted on TUI.
+  - `timeoutMs` / `maxTimeoutMs` — outer execution watchdog around each tool `run()`. Unset leaves the generic watchdog unarmed; set these to arm it. `maxTimeoutMs` clamps non-shell tools when set and does not cap a longer requested `run_shell`. Foreground `run_shell` always arms at the effective shell timeout (120s default, `settings.shell.timeoutMs` override, or per-call) plus 1000ms slack. Fleet wait tools are exempt: a dispatched sub-agent is bounded by stall, opt-in `deadlineMs`, and operator cancel, not the generic per-tool budget. Background shell is exempt too: a `run_shell` with `background: true` arms nothing (only a per-call timeout bounds the process; there is no 120s default) and `shell_collect` never arms the per-tool watchdog (a bounded poll over a process that outlives the turn). An in-flight TUI `shell_collect` or `ask_director` is still bounded by the TUI stall watchdog, including after a tool batch resolves and after compact continuation re-entry, so a typical poll does not pin the turn. Concurrent same-name `shell_collect` calls still share one `callIdByName` slot (one id per name); the leftover stays named in the per-id `callNameById` map so the stall watchdog still bounds it after the mapping-owning sibling finishes. `wait_agents` is exec-primary only and is not mounted on TUI.
   - `waitForApproval` (default **true** when unset) — freeze that budget while a permission prompt is open so a late approve still runs the tool. **Settings → Tools** toggles this live for the next tool call and persists it here. When **false**, the budget keeps ticking during the prompt; on expiry the tool is skipped and the modal is auto-dismissed. The freeze is bounded: after **30 minutes** with the prompt still unanswered the budget resumes ticking on its own, so a prompt that never becomes visible (overlay open, UI gone) cannot hang a tool run indefinitely.
 
   Optional `mcp` block bounds MCP tool calls (`mcp__*` names) specifically — unlike `tools.*`, this arms **unconditionally** even with no settings at all, defaulting to **5 minutes**, since a wedged MCP server otherwise hangs a call forever with nothing to bound it (CL-6895):

docs/TUI.md

@@ -208,12 +208,11 @@ arrives (right after submit, after a tool batch resolves, or after compact
 continuation re-entry), and an in-flight poll the per-tool execution
 watchdog leaves unarmed (`shell_collect`, `ask_director`). TUI primary does
 not mount `wait_agents` — mailbox mail is the collect path.
-Concurrent same-name `shell_collect` polls still collide in `callIdByName`
-(one slot per name), so a sibling collect finishing can drop the mapping and
-the remaining poll can lose that stall bound. That residual is in the
-name-keyed tracker, not a hole in the post-tool / post-compact abort; the
-usual single-collect shape stays bounded, including after a sibling
-`tool.done` that clears `currentToolName`.
+Concurrent same-name `shell_collect` polls still share one `callIdByName`
+slot (one id per name); the leftover stays named in the per-id
+`callNameById` map, so a sibling collect finishing does not drop the stall
+bound. The usual single-collect shape stays bounded too, including after a
+sibling `tool.done` that clears `currentToolName`.
 Ordinary in-flight tools stay exempt here — they have their own execution
 budget — and an open operator gate still blocks the clock. The notice
 still arms first at `STALL_NOTICE_MS` and hands over to the abort at the

src/tui/runtime-bridge.ts

@@ -1551,6 +1551,7 @@ export function attachSessionBridge(
     currentToolName: bag.turn.currentToolName,
     activeToolCalls: bag.turn.activeToolCalls,
     callIdByName: bag.turn.callIdByName,
+    callNameById: bag.turn.callNameById,
     stallNoticeMs,
     repeating: bag.turn.repeating,
   });

src/tui/stall-watchdog.test.ts

@@ -11,6 +11,7 @@ import {
   STALL_RECOVERY_MESSAGE,
   STALL_TIMEOUT_MS,
 } from "./stall-watchdog.js";
+import { initialTurnState, turnStateFromEvent } from "./turn-state.js";
 
 describe("shouldAbortForStall", () => {
   // Mid-stream hang: tokens already flowed, then everything went silent.
@@ -25,6 +26,7 @@ describe("shouldAbortForStall", () => {
     currentToolName: null,
     activeToolCalls: [],
     callIdByName: {},
+    callNameById: {},
   };
 
   test("aborts a mid-stream hang past the timeout", () => {
@@ -98,6 +100,7 @@ describe("shouldAbortForStall — awaiting-response with a null stream eventuall
     currentToolName: null,
     activeToolCalls: [],
     callIdByName: {},
+    callNameById: {},
   };
 
   test("aborts a run awaiting a response once the stall budget elapses", () => {
@@ -161,6 +164,7 @@ describe("shouldAbortForStall — execution-watchdog-exempt tools do not pin for
     currentToolName: "shell_collect",
     activeToolCalls: ["collect-1"],
     callIdByName: { shell_collect: "collect-1" },
+    callNameById: { "collect-1": "shell_collect" },
   };
 
   test("in-flight collect auto-aborts after the stall budget", () => {
@@ -177,6 +181,7 @@ describe("shouldAbortForStall — execution-watchdog-exempt tools do not pin for
         currentToolName: "wait_agents",
         activeToolCalls: ["wait-1"],
         callIdByName: { wait_agents: "wait-1" },
+        callNameById: { "wait-1": "wait_agents" },
       }),
     ).toBe(true);
   });
@@ -192,6 +197,7 @@ describe("shouldAbortForStall — execution-watchdog-exempt tools do not pin for
       awaitingResponse: false,
       activeToolCalls: ["collect-1"],
       callIdByName: { shell_collect: "collect-1" },
+      callNameById: { "collect-1": "shell_collect" },
     };
     expect(
       shouldAbortForStall({ ...afterSiblingDone, nowMs: STALL_TIMEOUT_MS - 1 }),
@@ -208,6 +214,78 @@ describe("shouldAbortForStall — execution-watchdog-exempt tools do not pin for
         awaitingResponse: false,
         activeToolCalls: ["bash-1"],
         callIdByName: { bash: "bash-1" },
+        callNameById: { "bash-1": "bash" },
+      }),
+    ).toBe(false);
+  });
+
+  // Two concurrent collects share one callIdByName slot: the second
+  // registration overwrites the first, so when the mapping-owning sibling
+  // resolves first and clears the slot, the leftover earlier collect is
+  // invisible to the name-keyed check above. The per-id record keeps it
+  // bounded (CL-8059).
+  test("concurrent collects with the mapping owner done first still abort at the stall budget", () => {
+    const leftover = {
+      ...collect,
+      currentToolName: null,
+      streamingType: null,
+      awaitingResponse: false,
+      activeToolCalls: ["collect-1"],
+      callIdByName: {},
+      callNameById: { "collect-1": "shell_collect" },
+    };
+    expect(
+      shouldAbortForStall({ ...leftover, nowMs: STALL_TIMEOUT_MS - 1 }),
+    ).toBe(false);
+    expect(shouldAbortForStall(leftover)).toBe(true);
+  });
+
+  test("folded leftover collect after mapping owner done still aborts at the stall budget", () => {
+    const leftover = [
+      { type: "inference.start" },
+      {
+        type: "tool.start",
+        data: { call: { id: "collect-1", name: "shell_collect" } },
+      },
+      {
+        type: "tool.start",
+        data: { call: { id: "collect-2", name: "shell_collect" } },
+      },
+      {
+        type: "tool.done",
+        data: { result: { callId: "collect-2" } },
+      },
+    ].reduce(
+      (state, event, i) => turnStateFromEvent(state, event, i + 1),
+      initialTurnState(0),
+    );
+    expect(leftover.callIdByName).toEqual({});
+    expect(
+      shouldAbortForStall({
+        ...leftover,
+        nowMs: leftover.lastActivityAt + STALL_TIMEOUT_MS - 1,
+        stallTimeoutMs: STALL_TIMEOUT_MS,
+      }),
+    ).toBe(false);
+    expect(
+      shouldAbortForStall({
+        ...leftover,
+        nowMs: leftover.lastActivityAt + STALL_TIMEOUT_MS,
+        stallTimeoutMs: STALL_TIMEOUT_MS,
+      }),
+    ).toBe(true);
+  });
+
+  test("a leftover ordinary tool tracked per-id is not a stall", () => {
+    expect(
+      shouldAbortForStall({
+        ...collect,
+        currentToolName: null,
+        streamingType: null,
+        awaitingResponse: false,
+        activeToolCalls: ["bash-1"],
+        callIdByName: {},
+        callNameById: { "bash-1": "bash" },
       }),
     ).toBe(false);
   });
@@ -255,6 +333,7 @@ describe("shouldNoticeStall", () => {
     repeating: false,
     activeToolCalls: [],
     callIdByName: {},
+    callNameById: {},
   };
 
   test("a parallel fan-out with sibling tools still running does not notice", () => {
@@ -340,6 +419,7 @@ describe("the stall level the indicator reads", () => {
     currentToolName: null,
     activeToolCalls: [],
     callIdByName: {},
+    callNameById: {},
     repeating: false,
   };
 

src/tui/stall-watchdog.ts

@@ -25,6 +25,12 @@ export interface ShouldAbortForStallArgs {
   readonly currentToolName: string | null;
   readonly activeToolCalls: readonly string[];
   readonly callIdByName: Readonly<Record<string, string>>;
+  /**
+   * Tool name for each in-flight real call id. Concurrent same-name siblings
+   * share one `callIdByName` slot; this map keeps the leftover named after
+   * the mapping owner resolves. See `isStallBoundedInFlightTool`.
+   */
+  readonly callNameById: Readonly<Record<string, string>>;
 }
 
 /**
@@ -62,7 +68,10 @@ function silentPastThreshold(
  * bound they hold the turn with no other wall-clock limit. A sibling
  * tool.done clears `currentToolName` / `streamingType` while the poll is
  * still in `activeToolCalls`, so the bound keys any in-flight stall-bounded
- * name, not only the last announced tool.
+ * name, not only the last announced tool. The name-keyed `callIdByName`
+ * holds one id per tool, so concurrent same-name siblings overwrite each
+ * other there; the per-id `callNameById` covers the leftover when the
+ * mapping-owning sibling resolves first and clears the shared slot.
  */
 function isStallBoundedToolName(name: string | null | undefined): boolean {
   return (
@@ -79,6 +88,12 @@ function isStallBoundedInFlightTool(args: ShouldAbortForStallArgs): boolean {
       return true;
     }
   }
+  // Concurrent same-name siblings share one callIdByName slot, so the
+  // mapping-owning sibling clearing it on tool.done must not release the
+  // leftover earlier call: its own id still maps to the bounded name here.
+  for (const id of args.activeToolCalls) {
+    if (isStallBoundedToolName(args.callNameById[id])) return true;
+  }
   // Name-only announcements track the call under its name until a real id
   // arrives, so the placeholder itself is the stall-bounded name.
   return args.activeToolCalls.some(isStallBoundedToolName);