From 764f339c3e83eac8ca34f4ddc8ed65f8199b651d Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Wed, 14 Oct 2026 11:44:15 +0200 Subject: [PATCH] fix: end the terminal of a command whose tool call is closed without a result (#1335) ## 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. --- docs/air-extensions.md | 1 + src/acp-agent.ts | 8 ++- src/tests/acp-v2.test.ts | 65 +++++++++++++++++ src/tests/incomplete-tools.test.ts | 108 +++++++++++++++++++++++++++++ src/tests/tools.test.ts | 29 ++++++++ src/tool-calls/renderer.ts | 34 +++++++-- 6 files changed, 239 insertions(+), 6 deletions(-) diff --git a/docs/air-extensions.md b/docs/air-extensions.md index 468b24e0..cc8a4045 100644 --- a/docs/air-extensions.md +++ b/docs/air-extensions.md @@ -229,6 +229,7 @@ An AIR client that declares `terminal_output_delta` gets appends instead of `ter A command tool call then has `content: [{type: "terminal", terminalId}]` and `_meta.terminal_info = {terminal_id}`. The output goes in `_meta.terminal_output = {terminal_id, data}`. The end sends `_meta.terminal_exit = {terminal_id, exit_code, signal: null}`. + A command that ends without a result (a cancel stopped its subagent, say) gets the exit on the report that closes its tool call, with no exit code (`null`; AIR gets `1`). - A client that declares neither `terminal_output` nor `terminal_output_delta` gets no terminal. The command output is a `console` code block in `content`. - A client that declares `clientCapabilities._meta["terminal-auth"]: true` gets `_meta["terminal-auth"] = {command, args, label}` on the terminal login methods. diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 6884f2fc..530b5d6d 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -4872,8 +4872,14 @@ export class ClaudeAcpAgent { /** Sends `report`, which ends the tool call `toolCallId` without its result * (see `AcpToolCallRenderer.unfinished`), and forgets the call. Forgetting * comes first, so that a late hook or a second close cannot overwrite the - * report. */ + * report. A command has a display terminal on a client that renders them, + * and no result will end it, so the report ends it too. */ const closeToolCall = async (toolCallId: string, report: ToolCallUpdate) => { + // Read before the cache entry goes: it names the tool. + const toolUse = session.toolUseCache[toolCallId]; + const terminalExit = + toolUse && new AcpToolCallRenderer(this.toolCallCapabilities).terminalExit(toolUse); + if (terminalExit) report._meta = { ...report._meta, ...terminalExit }; unregisterHookCallback(toolCallId); session.emittedToolCalls.delete(toolCallId); delete session.toolUseCache[toolCallId]; diff --git a/src/tests/acp-v2.test.ts b/src/tests/acp-v2.test.ts index 13ab8624..44203eaa 100644 --- a/src/tests/acp-v2.test.ts +++ b/src/tests/acp-v2.test.ts @@ -1640,6 +1640,71 @@ describe("ACP v2 prompts", () => { expect(childEnd).toBeLessThan(idle); }); + it("ends the terminal of a stopped subagent's command with the tool call", async () => { + const interrupted = Promise.withResolvers(); + sdk.interrupt = async () => interrupted.resolve(); + const subagentTask = ( + options: Options, + subtype: string, + fields: Record, + ) => ({ + type: "system", + subtype, + task_id: "agent-1", + tool_use_id: "toolu_agent", + uuid: randomUUID(), + session_id: options.sessionId, + ...fields, + }); + scriptTurns(async function* (options) { + yield toolUseMessage(options, "toolu_agent", "Agent", { + description: "Run a slow command", + subagent_type: "general-purpose", + prompt: "Run it", + }); + yield subagentTask(options, "task_started", { + description: "Run a slow command", + subagent_type: "general-purpose", + prompt: "Run it", + }); + yield toolUseMessage(options, "toolu_child", "Bash", { command: "make" }, "toolu_agent"); + await interrupted.promise; + // Claude Code sends no result for the subagent's command. + yield subagentTask(options, "task_notification", { + status: "stopped", + output_file: "", + summary: "stopped", + }); + yield toolResultMessage(options, "toolu_agent", { is_error: true }, "user-rejected"); + yield result(options, ABORTED_TOOLS); + yield idleState(options); + }); + + const { root, updates } = await cancelWhileRunning("toolu_child"); + + // The command's terminal shows in the child's session, and its exit comes + // with the tool call's end, before the root reports idle. The exit code is + // unknown, so the exit carries none. + const terminalAt = (predicate: (update: Seen["update"]) => boolean) => + updates.findIndex( + ({ sessionId, update }) => + sessionId === "agent-1" && + update.sessionUpdate === "terminal_update" && + predicate(update), + ); + const exit = terminalAt((update) => update.exitStatus !== undefined); + expect(terminalAt((update) => update.exitStatus === undefined)).toBeGreaterThanOrEqual(0); + expect(updates[exit]).toMatchObject({ + update: { terminalId: "toolu_child", exitStatus: {} }, + }); + expect(exit).toBeGreaterThan(terminalAt((update) => update.exitStatus === undefined)); + expect(exit).toBeLessThan(idleAt(root, updates)); + expect(updates[lastStatusAt(updates, "toolu_child")]).toMatchObject({ + sessionId: "agent-1", + update: { status: "cancelled" }, + }); + }); + it("ends a tool call that finished as the cancel landed as completed, not cancelled", async () => { const interrupted = Promise.withResolvers(); sdk.interrupt = async () => interrupted.resolve(); diff --git a/src/tests/incomplete-tools.test.ts b/src/tests/incomplete-tools.test.ts index 6c38b086..8ac60523 100644 --- a/src/tests/incomplete-tools.test.ts +++ b/src/tests/incomplete-tools.test.ts @@ -524,6 +524,95 @@ describe("incomplete foreground tools", () => { }); }); +describe("the display terminal of a command that ends without a result", () => { + const TERMINAL_CLIENT = { _meta: { terminal_output: true } }; + const AIR_TERMINAL_CLIENT = { + _meta: { terminal_output: true, jetbrains: { air: { version: 1, capabilities: [] } } }, + }; + /** The reports that end the tool call `id`. */ + const endsOf = (updates: any[], id = toolCallId) => + updates.filter( + (u) => u.toolCallId === id && ["completed", "failed", "cancelled"].includes(u.status), + ); + + /** Runs the cancelled turn of `interruptedSubagentMessages` for a client with `capabilities`. */ + async function cancelledSubagentCommand(capabilities: Record) { + const session = createTestSession(interruptedSubagentMessages); + await session.agent.initialize({ protocolVersion: 1, clientCapabilities: capabilities }); + await expect(session.prompt()).resolves.toMatchObject({ stopReason: "cancelled" }); + return endsOf(session.updates, "child-tool"); + } + + it("ends the terminal of a command that a cancel closes", async () => { + const [end, ...more] = await cancelledSubagentCommand(TERMINAL_CLIENT); + + expect(more).toEqual([]); + // The report that ends the call ends its terminal too, and still names the subagent. + expect(end).toMatchObject({ + status: "failed", + _meta: { + terminal_exit: { terminal_id: "child-tool", exit_code: null, signal: null }, + claudeCode: { parentToolUseId: "parent-agent" }, + }, + }); + }); + + it("gives an AIR client the number that it reads for a command that did not succeed", async () => { + const [end] = await cancelledSubagentCommand(AIR_TERMINAL_CLIENT); + + expect(end._meta.terminal_exit).toEqual({ + terminal_id: "child-tool", + exit_code: 1, + signal: null, + }); + }); + + it("sends no terminal exit to a client that renders no terminals", async () => { + const [end] = await cancelledSubagentCommand({}); + + expect(end._meta?.terminal_exit).toBeUndefined(); + }); + + it("sends no terminal exit for a tool that is not a command", async () => { + const session = createTestSession(interruptedSubagentReadMessages); + await session.agent.initialize({ protocolVersion: 1, clientCapabilities: TERMINAL_CLIENT }); + + await expect(session.prompt()).resolves.toMatchObject({ stopReason: "cancelled" }); + + const [end] = endsOf(session.updates, "child-tool"); + expect(end.status).toBe("failed"); + expect(end._meta.terminal_exit).toBeUndefined(); + }); + + it("ends the terminal of a command that ran, but got no result by the end of the turn", async () => { + const session = createTestSession((input) => unfinishedToolMessages(input, "result")); + await session.agent.initialize({ protocolVersion: 1, clientCapabilities: TERMINAL_CLIENT }); + + await expect(session.prompt()).rejects.toMatchObject({ + data: { errorKind: "incomplete_tool_call" }, + }); + + const [end] = endsOf(session.updates); + expect(end.content[0].content.text).toContain("without returning a result"); + expect(end._meta.terminal_exit).toEqual({ + terminal_id: toolCallId, + exit_code: null, + signal: null, + }); + }); + + it("ends the terminal of a command that Claude abandoned before it ran", async () => { + const session = createTestSession(abandonedToolMessages); + await session.agent.initialize({ protocolVersion: 1, clientCapabilities: TERMINAL_CLIENT }); + + await expect(session.prompt()).resolves.toMatchObject({ stopReason: "end_turn" }); + + const [end] = endsOf(session.updates); + expect(end.content[0].content.text).toBe("Claude stopped this tool call before it ran."); + expect(end._meta.terminal_exit).toMatchObject({ terminal_id: toolCallId }); + }); +}); + describe("tool calls of a cancelled turn", () => { /** The reports that end the tool call `id`. */ const ends = (updates: any[], id = toolCallId) => @@ -1026,6 +1115,25 @@ async function* interruptedSubagentMessages(input: Pushable, agent: ClaudeA yield abortedResult(); } +/** Like `interruptedSubagentMessages`, but the subagent reads a file, not runs a command. */ +async function* interruptedSubagentReadMessages(input: Pushable, agent: ClaudeAcpAgent) { + yield* echoNextPrompt(input); + yield toolStart("parent-agent", null, "Agent"); + yield toolUse("parent-agent", null, "Agent", { + description: "Read a file", + subagent_type: "general-purpose", + prompt: "Read it", + }); + yield taskStarted("agent-1", "parent-agent", "general-purpose"); + yield toolStart("child-tool", "parent-agent", "Read"); + yield toolUse("child-tool", "parent-agent", "Read", { file_path: "/test/file.txt" }); + await agent.cancel({ sessionId }); + yield taskStopped("agent-1", "parent-agent"); + yield interruptedResult("parent-agent"); + yield interruptNote; + yield abortedResult(); +} + /** A command that a permission request announced without its subagent, and the stream attributed to it. */ async function* attributedLaterMessages(input: Pushable, agent: ClaudeAcpAgent) { yield* echoNextPrompt(input); diff --git a/src/tests/tools.test.ts b/src/tests/tools.test.ts index 1b4a5b5c..bd083f9a 100644 --- a/src/tests/tools.test.ts +++ b/src/tests/tools.test.ts @@ -3877,6 +3877,35 @@ describe("tool_result_meta non-execution stamping", () => { }); }); + it("ends the terminal of a command that gets no result, for a client that renders terminals", () => { + const terminalExit = (name: string, capabilities?: ToolCallCapabilities) => + new AcpToolCallRenderer(capabilities).terminalExit({ + id: "toolu_bash", + name, + input: { command: "make" }, + }); + const zed = ToolCallCapabilities.from({ _meta: { terminal_output: true } }); + const airWithTerminal = ToolCallCapabilities.from({ + _meta: { ...AIR_CLIENT._meta, terminal_output: true }, + }); + const exit = (exitCode: number | null) => ({ + terminal_exit: { terminal_id: "toolu_bash", exit_code: exitCode, signal: null }, + }); + + // The command ended without a result, so no exit code is known. + expect(terminalExit("Bash", zed)).toEqual(exit(null)); + expect(terminalExit("PowerShell", zed)).toEqual(exit(null)); + // A v2 client renders terminals without declaring them. + expect(terminalExit("Bash", ToolCallCapabilities.from({}, { v2: true }))).toEqual(exit(null)); + // AIR always gets a number, and reads 1 for a command that did not succeed. + expect(terminalExit("Bash", airWithTerminal)).toEqual(exit(1)); + // No terminal was started for these tool calls. + expect(terminalExit("Bash")).toBeUndefined(); + expect(terminalExit("Bash", ToolCallCapabilities.from(AIR_CLIENT))).toBeUndefined(); + expect(terminalExit("Read", zed)).toBeUndefined(); + expect(terminalExit("mcp__server__tool", zed)).toBeUndefined(); + }); + it("reports an aborted run of a tool call that only a permission request surfaced as cancelled on v2", () => { // TodoWrite renders as a plan, so only the permission flow emits its tool call. const todoWrite = { type: "tool_use" as const, id: "toolu_todo", name: "TodoWrite", input: {} }; diff --git a/src/tool-calls/renderer.ts b/src/tool-calls/renderer.ts index 01e64aa5..a3b90630 100644 --- a/src/tool-calls/renderer.ts +++ b/src/tool-calls/renderer.ts @@ -162,6 +162,14 @@ export class AcpToolCallRenderer { return this.capabilities.air.client || this.capabilities.v2; } + /** + * Whether a tool call with these facts gets a display terminal: a command, for a client + * that renders terminals. + */ + private hasTerminal(facts: ToolUseFacts): boolean { + return facts.command === true && this.capabilities.terminalOutput; + } + /** * The status of a tool result. Claude Code reports a run that it aborted in * one of three ways: @@ -231,9 +239,7 @@ export class AcpToolCallRenderer { return { _meta: { ...this.toolUseMeta(toolUse, options.cwd), - ...(facts.command && this.capabilities.terminalOutput - ? { terminal_info: { terminal_id: toolUse.id } } - : {}), + ...(this.hasTerminal(facts) ? { terminal_info: { terminal_id: toolUse.id } } : {}), // Only AIR gets the MCP flag. The upstream adapter does not send it. ...(isMcp && this.capabilities.air.client ? { is_mcp_tool_call: true } : {}), } satisfies ToolUpdateMeta, @@ -641,7 +647,7 @@ export class AcpToolCallRenderer { * client `failed`, since v1 has no `cancelled`. If Claude Code ran it but * returned no result, it failed, unless a `session/cancel` ended the turn * (`cancelled`): the cancel stopped the run, so a v2 client gets `cancelled` - * for it too. + * for it too. The report does not end the call's terminal: see {@link terminalExit}. */ unfinished(call: { toolCallId: string; ran: boolean; cancelled?: boolean }): ToolCallUpdate { const stopped = !call.ran || call.cancelled === true; @@ -661,6 +667,24 @@ export class AcpToolCallRenderer { }; } + /** + * The `_meta` that ends the display terminal of a command whose tool call gets no result, to + * add to the report from {@link unfinished}. Undefined when the tool call has no terminal + * (see {@link toolCall}). The exit code is unknown, since the command ended without a result: + * null, or 1 for AIR, which always gets a number and reads 1 for a command that did not + * succeed. + */ + terminalExit(toolUse: ToolUse): Pick | undefined { + if (!this.hasTerminal(this.facts(toolUse))) return undefined; + return { + terminal_exit: { + terminal_id: toolUse.id, + exit_code: this.capabilities.air.client ? 1 : null, + signal: null, + }, + }; + } + /** * The report of a tool progress beat. The field tracker sends the * `in_progress` status once, so a later beat carries only the progress. @@ -720,7 +744,7 @@ export class AcpToolCallRenderer { * does not render `rawInput` itself, and never next to a terminal. */ toolUseContent(toolCallId: string, facts: ToolUseFacts): ToolCallContent[] { - const terminal = facts.command === true && this.capabilities.terminalOutput; + const terminal = this.hasTerminal(facts); return [ ...(terminal ? [{ type: "terminal" as const, terminalId: toolCallId }] : []), ...(facts.change ?? []),