Skip to content

Commit beaa11b

Browse files
committed
docs(tui): qualify stall abort cannot-pin-forever claim
Post-tool and post-compact silence still abort. Concurrent same-name shell_collect can still lose the stall bound via callIdByName collision, so docs must not promise an absolute cannot-pin-forever.
1 parent 8288e86 commit beaa11b

3 files changed

Lines changed: 8 additions & 2 deletions

File tree

‎docs/ARCHITECTURE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ tool call
419419

420420
**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).
421421

422-
**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, in-flight `shell_collect` and `ask_director` are still bounded by the stall watchdog so those polls cannot pin the turn forever. `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.
422+
**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.
423423

424424
`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.
425425

‎docs/IMPLEMENTATION.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ Provider and model configuration lives in JSON settings files. The global file h
286286
}
287287
```
288288

289-
- `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 so it cannot pin the turn forever. `wait_agents` is exec-primary only and is not mounted on TUI.
289+
- `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.
290290
- `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.
291291

292292
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‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,12 @@ arrives (right after submit, after a tool batch resolves, or after compact
208208
continuation re-entry), and an in-flight poll the per-tool execution
209209
watchdog leaves unarmed (`shell_collect`, `ask_director`). TUI primary does
210210
not mount `wait_agents` — mailbox mail is the collect path.
211+
Concurrent same-name `shell_collect` polls still collide in `callIdByName`
212+
(one slot per name), so a sibling collect finishing can drop the mapping and
213+
the remaining poll can lose that stall bound. That residual is in the
214+
name-keyed tracker, not a hole in the post-tool / post-compact abort; the
215+
usual single-collect shape stays bounded, including after a sibling
216+
`tool.done` that clears `currentToolName`.
211217
Ordinary in-flight tools stay exempt here — they have their own execution
212218
budget — and an open operator gate still blocks the clock. The notice
213219
still arms first at `STALL_NOTICE_MS` and hands over to the abort at the

0 commit comments

Comments
 (0)