mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 12:40:32 +00:00
764f339c3e
## Problem A tool call that ends without a tool result is closed by a report that only sets its status (`AcpToolCallRenderer.unfinished`). That happens after `session/cancel`, for a subagent's command (Claude Code sends no result for it) or a call that Claude abandoned, and when a turn ends with a call that has no result (`closeUnfinished` in `settleActive`). A client that renders terminals (the `terminal_output` / `terminal_output_delta` extension, and the display terminals of ACP v2) holds a terminal for a Bash command, which its `tool_call` started with `_meta.terminal_info`. The closing report did not end it, so the call shows as ended next to a terminal that never exits. Example, live (Claude Code 2.1.296, Sonnet): a foreground Agent call whose subagent runs `python3 -c "import time; time.sleep(45)"`, and `session/cancel` while it runs. Claude Code answers the Agent call and sends nothing for the subagent's command, so the adapter closes that call itself: - v1 client with `_meta.terminal_output`: `tool_call_update` `failed`, with no `terminal_exit`. - v2 client: `terminal_update` with the command, then `tool_call_update` `cancelled`, and no `terminal_update` that carries an exit. ## Change - `closeToolCall` is the one function that every closing report goes through (the cancelled-turn closing and the end-of-turn closing today, and any closing added later). It now also ends the call's terminal, in the same update, as the result of a command does. It reads the tool from the tool use cache before it drops the entry. - The closings that #1336 and #1339 add go through `closeToolCall` too, so they end the terminal as well once merged. - `AcpToolCallRenderer.terminalExit` decides whether the call has a terminal. It uses the rule that `toolCall` uses to start one, a command on a client that renders terminals, which is now one private method (`hasTerminal`) that `toolCall`, `toolUseContent` and `terminalExit` share. - The exit code is unknown, since the command ended without a result: `exit_code: null`, as `endBackgroundTerminal` sends for a background command. AIR always gets a number and reads 1 for a command that did not succeed (#1252), so it gets 1. A v2 client gets `terminal_update` with an empty `exitStatus`. - `docs/air-extensions.md`: one line in the terminal conventions. ## Evidence **Tests** (8 new): - `incomplete-tools.test.ts`: the cancel's closing of a subagent's command ends its terminal, still names the subagent, and sends `exit_code: null`; AIR gets `exit_code: 1`; a client without terminals gets no exit; a subagent's `Read` gets none; a command that ran but got no result by the end of the turn gets the exit; so does one that Claude abandoned before it ran. - `tools.test.ts`: `terminalExit` for Bash, PowerShell, v2, AIR, a client without terminals, and tools that are not commands. - `acp-v2.test.ts`: the stopped subagent's terminal exits in the child's session, after the command's terminal started and before the root's `idle cancelled`, and the call ends `cancelled`. **Removal checks.** Each of these fails at least one test: no exit in `closeToolCall` (5), AIR gets null (2), an exit for a client without terminals (2), an exit for a tool that is not a command (3), another terminal id (5), the exit replacing the parent meta (1), the tool read after the cache entry is dropped (5). **Live**, each update validated against the SDK's JSON schema (v1 `SessionNotification`, v2 `session/update`), 0 violations. The Agent call and the subagent's command from the example, cancelled 4 s after the command's permission request was answered: | client | `main` | this PR | | --- | --- | --- | | v1, `_meta.terminal_output` | closing report `failed`, no `terminal_exit` | the same update carries `terminal_exit {exit_code: null, signal: null}` | | v1, AIR with `terminal_output` | not run on `main` (same code path) | `terminal_exit` with `exit_code: 1` | | v2 | `cancelled`, no terminal update | `terminal_update` with `exitStatus: {}`, then `cancelled`, before the root's `idle cancelled` | `npm run check`, `npm run build` and `npm run test:run` (2932 tests) pass. No golden file changes. With #1336 and #1339 merged into it, in any of the six orders, the branch merges without conflicts and all 2948 tests pass. ## Not covered - A call that only a permission request announced, whose tool use never streamed before it was closed, has no entry in the tool use cache. Its tool is unknown here, so its terminal stays open. This is the lookup that `AsyncTaskRuntime` uses too (`toolUseCache`, then `resolvedToolNames`). Covering it needs the tool name where the permission request emits the call. - The fallback for a tool result without a tool use (`toAcpNotifications`) still reports the status only, with no exit. - The end-of-turn closing is covered by a test only: Claude Code does not end a turn with an unanswered call on demand.