diff --git a/README.md b/README.md index b27e8d25..75f14ffe 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,9 @@ Task subagent as its own child session, following the [subagents RFD](https://gi withdrawn. The adapter ends no child at a cancel itself, so a child that keeps running keeps its open requests and its open tool calls. A tool call of a stopped child that Claude Code reports no result for, such as the command it was running, ends in the child's session before the cancelled - prompt resolves. A child that still runs when Claude Code exits is `idle` with no stop reason. + prompt resolves. When Claude Code stops a background child after that, or while no prompt runs, + the tool call ends right after the child reports `idle`. A child that still runs when Claude Code + exits is `idle` with no stop reason. - A child whose parent is not yet known when it ends is not exposed, since the RFD forbids guessing its parent. - A replay restores each child under its live session id, from its own transcript: its prompt and diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 530b5d6d..c37b4381 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -4900,6 +4900,35 @@ export class ClaudeAcpAgent { task.parentToolUseId === agentToolUseId, ); + /** Ends the open tool calls that `include` accepts, with the report of a call + * that a cancel stopped and sent no result for. `include` gets the Agent/Task + * call of the subagent that made the tool call, if a subagent did. */ + const closeOpenToolCalls = async ( + include: (parentToolUseId: string | undefined) => boolean, + ): Promise => { + const renderer = new AcpToolCallRenderer(this.toolCallCapabilities); + for (const toolCallId of [...session.emittedToolCalls]) { + const parentToolUseId = session.toolCallParents?.get(toolCallId); + if (!include(parentToolUseId)) continue; + const report = renderer.unfinished({ + toolCallId, + ran: session.dispatchedToolCalls?.has(toolCallId) === true, + cancelled: true, + }); + // Named as the call's other updates are: the subagent runtime routes the + // report with them, to the child session, or to none that has ended. + if (parentToolUseId !== undefined) stampParentToolUseId(report, parentToolUseId); + await closeToolCall(toolCallId, report); + } + }; + + /** Whether the closing at the end of the work a cancel stopped takes a tool + * call that the subagent started by the Agent call `parentToolUseId` made + * (undefined for a call of the main conversation): not one of a running + * background subagent, which the cancel may not have stopped. */ + const closedAtCancel = (parentToolUseId: string | undefined): boolean => + parentToolUseId === undefined || !runsInBackground(parentToolUseId); + /** Closes the tool calls that the work a cancel stopped left open, as that * work ends. Claude Code's interrupt ends each running tool of the main * conversation with an error result, which `sendInterruptedToolResults` @@ -4912,28 +4941,33 @@ export class ClaudeAcpAgent { * A foreground subagent ends with the work that waits on it. A background * subagent keeps its tool calls open until the SDK reports its end, which * Claude Code 2.1.293 does for the ones it stops, before the turn's result - * (see `NativeSubagentRuntime.parentCancelled`). + * (see `NativeSubagentRuntime.parentCancelled`). When the SDK reports it + * only after the work ended, `closeStoppedSubagentToolCalls` closes them. * Runs once per turn: a second call, from the other settling path, waits * for the first (see `Turn.closingCancelledToolCalls`). */ const closeCancelledToolCalls = (turn: Turn): Promise => - (turn.closingCancelledToolCalls ??= (async () => { - const renderer = new AcpToolCallRenderer(this.toolCallCapabilities); - for (const toolCallId of [...session.emittedToolCalls]) { - const parentToolUseId = session.toolCallParents?.get(toolCallId); - if (parentToolUseId !== undefined && runsInBackground(parentToolUseId)) continue; - const report = renderer.unfinished({ - toolCallId, - ran: session.dispatchedToolCalls?.has(toolCallId) === true, - cancelled: true, - }); - // Named as the call's other updates are: the subagent runtime routes the - // report with them, to the child session, or to none that has ended. - if (parentToolUseId !== undefined) stampParentToolUseId(report, parentToolUseId); - await closeToolCall(toolCallId, report); - } - })()); + (turn.closingCancelledToolCalls ??= closeOpenToolCalls(closedAtCancel)); session.closeCancelledToolCalls = closeCancelledToolCalls; + /** Closes the tool calls of the subagent that the Agent call `agentToolUseId` + * started, once the SDK reports the subagent stopped, if a cancel stopped it + * after the work that waited on it ended. Nothing else ends them then. + * Example: the cancel settles a turn that v1 holds for a background + * subagent, so `closeCancelledToolCalls` leaves the subagent's command + * open, since the subagent runs. Claude Code's interrupt then stops the + * subagent. Claude Code 2.1.296 sends the command's interrupted result + * after that, but the result is dropped like all output after the work's + * end (see the `session.cancelled` check of the user/assistant case). + * While the work still runs, the calls of a stopped subagent get Claude + * Code's result, or `closeCancelledToolCalls` at the work's end. */ + const closeStoppedSubagentToolCalls = async ( + agentToolUseId: string | null | undefined, + ): Promise => { + if (!agentToolUseId || !session.cancelled) return; + if (!this.workEndReported(params.sessionId, session)) return; + await closeOpenToolCalls((parentToolUseId) => parentToolUseId === agentToolUseId); + }; + /** Delivers the results that the work a cancel stopped sends for tool calls * that the client holds open. Claude Code's interrupt ends each running tool * of the main conversation with an error result, before the result that @@ -6068,6 +6102,9 @@ export class ClaudeAcpAgent { droppedStreams.delete(message.tool_use_id); } await endBackgroundTerminal(message.task_id); + if (message.status === "stopped") { + await closeStoppedSubagentToolCalls(message.tool_use_id); + } settleLiveTask(message.task_id); break; case "task_updated": @@ -6110,6 +6147,9 @@ export class ClaudeAcpAgent { droppedStreams.delete(parentToolUseId); } await endBackgroundTerminal(message.task_id); + if (message.patch.status === "killed") { + await closeStoppedSubagentToolCalls(parentToolUseId); + } settleLiveTask(message.task_id); } else if ( message.patch.status === "running" || diff --git a/src/tests/acp-agent.test.ts b/src/tests/acp-agent.test.ts index 69924076..43d88608 100644 --- a/src/tests/acp-agent.test.ts +++ b/src/tests/acp-agent.test.ts @@ -7384,6 +7384,124 @@ describe("subagent permission attribution (issue #851)", () => { } }); + it("closes a background child's tool call in its session when the SDK reports the stop after the cancelled response", async () => { + const log: string[] = []; + const updates: AcpSessionNotification[] = []; + const logError = vi.fn(); + const agent = new ClaudeAcpAgent( + { + sessionUpdate: async (update: AcpSessionNotification) => { + updates.push(update); + if (update.update.sessionUpdate === "subagent_update") { + log.push(`${update.update.sessionId} ${JSON.stringify(update.update.state)}`); + } + if (update.update.sessionUpdate === "tool_call_update" && update.update.status) { + log.push( + `tool @${update.sessionId} ${update.update.toolCallId} ${update.update.status}`, + ); + } + }, + } as unknown as AcpClient, + { log: () => {}, error: logError }, + ); + await agent.initialize({ + protocolVersion: 1, + clientCapabilities: { subagents: {} } as ClientCapabilities, + }); + const responded = Promise.withResolvers(); + injectGeneratorSession(agent, (input) => + (async function* () { + const { value } = await input[Symbol.asyncIterator]().next(); + yield userEcho(value); + yield rootToolUse("toolu_agent", "Agent", { + description: "Run a slow command", + subagent_type: "general-purpose", + prompt: "Run it", + run_in_background: true, + }); + yield { + ...taskStarted("agent-42", "toolu_agent"), + subagent_type: "general-purpose", + is_backgrounded: true, + }; + // Claude Code answers a background Agent call when the subagent launches. + yield { + type: "user", + parent_tool_use_id: null, + uuid: randomUUID(), + session_id: "test-session", + message: { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "toolu_agent", + content: "Async agent launched", + }, + ], + }, + }; + yield { + ...rootToolUse("toolu_child", "Bash", { command: "make" }), + parent_tool_use_id: "toolu_agent", + }; + // The turn's result comes while the subagent runs, so v1 holds the turn + // for it. The cancel settles the held turn, which leaves the subagent's + // command open, since the subagent runs. + yield successResult(); + await agent.cancel({ sessionId: "test-session" }); + await responded.promise; + // Claude Code's interrupt stops the subagent only now, and sends the + // command's interrupted result after that. + yield taskUpdated({ status: "killed" }); + yield taskNotification("toolu_agent", "stopped", { summary: "stopped" }); + yield { + type: "user", + parent_tool_use_id: "toolu_agent", + uuid: randomUUID(), + session_id: "test-session", + tool_result_meta: [{ id: "toolu_child", non_execution_kind: "user-rejected" }], + message: { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "toolu_child", + is_error: true, + content: "The user doesn't want to proceed with this tool use.", + }, + ], + }, + }; + })(), + ); + + await expect( + agent + .prompt({ sessionId: "test-session", prompt: [{ type: "text", text: "go" }] }) + .then((response) => { + log.push(`prompt ${response.stopReason}`); + responded.resolve(); + return response; + }), + ).resolves.toMatchObject({ stopReason: "cancelled" }); + await agent.sessions["test-session"]?.consumer; + + // The command ends in the child's session, after the child reported idle. + expect(log).toEqual([ + 'agent-42 {"state":"running"}', + "tool @test-session toolu_agent completed", + "prompt cancelled", + 'agent-42 {"state":"idle","stopReason":"cancelled"}', + "tool @agent-42 toolu_child failed", + ]); + expect(agent.sessions["test-session"]?.emittedToolCalls.size).toBe(0); + expect(logError).not.toHaveBeenCalled(); + for (const payload of updates) { + expect(validateRecorded({ kind: "sessionUpdate", payload }, new Set())).toEqual([]); + } + }); + it("adds the SDK's error to a failure that a notification reported first", async () => { const updates = await run([ { ...taskStarted("agent-42", "toolu_parent"), subagent_type: "Explore" }, @@ -7524,6 +7642,74 @@ describe("subagent permission attribution (issue #851)", () => { ).toBe(false); }); + it("releases a background child's tool call without a report when the SDK reports the stop after the cancel (AIR)", async () => { + const updates: AcpSessionNotification[] = []; + const logs: string[] = []; + const agent = new ClaudeAcpAgent( + { + sessionUpdate: async (update: AcpSessionNotification) => { + updates.push(update); + }, + } as unknown as AcpClient, + { log: (message: string) => logs.push(message), error: () => {} }, + ); + await agent.initialize({ protocolVersion: 1, clientCapabilities: AIR_NATIVE_SUBAGENTS }); + const responded = Promise.withResolvers(); + injectGeneratorSession(agent, (input) => + (async function* () { + const { value } = await input[Symbol.asyncIterator]().next(); + yield userEcho(value); + yield rootToolUse("toolu_agent", "Agent", { description: "Explore", prompt: "Look" }); + yield { + ...taskStarted("agent-42", "toolu_agent"), + subagent_type: "Explore", + is_backgrounded: true, + }; + yield { + ...rootToolUse("toolu_child", "Bash", { command: "make" }), + parent_tool_use_id: "toolu_agent", + }; + yield successResult(); + await agent.cancel({ sessionId: "test-session" }); + await responded.promise; + yield { + type: "system", + subtype: "task_notification", + task_id: "agent-42", + tool_use_id: "toolu_agent", + status: "stopped", + output_file: "", + summary: "stopped", + uuid: randomUUID(), + session_id: "test-session", + }; + })(), + ); + + await agent + .prompt({ sessionId: "test-session", prompt: [{ type: "text", text: "go" }] }) + .finally(() => responded.resolve()); + await agent.sessions["test-session"]?.consumer; + + // AIR's draft ended the child at the cancel, so the closing report for its + // command is dropped with every later update of the child. + const childEnded = updates.findIndex( + ({ update }) => + update.sessionUpdate === "subagent_state_update" && + update.subagentSessionId === "agent-42" && + update.state === "cancelled", + ); + expect(childEnded).toBeGreaterThanOrEqual(0); + expect( + updates.slice(childEnded + 1).filter(({ sessionId }) => sessionId === "agent-42"), + ).toEqual([]); + expect(logs).toContain( + "Session test-session: ignoring late update for terminal subagent agent-42", + ); + // The call is not kept open here, though. + expect(agent.sessions["test-session"]?.emittedToolCalls.size).toBe(0); + }); + it("buffers child output until spawn and drops duplicates and late updates", async () => { const updates: AcpSessionNotification[] = []; const agent = new ClaudeAcpAgent( diff --git a/src/tests/acp-v2.test.ts b/src/tests/acp-v2.test.ts index 44203eaa..133714e0 100644 --- a/src/tests/acp-v2.test.ts +++ b/src/tests/acp-v2.test.ts @@ -1722,6 +1722,107 @@ describe("ACP v2 prompts", () => { expect(updates[at]).toMatchObject({ update: { status: "completed" } }); expect(at).toBeLessThan(idleAt(root, updates)); }); + + it("ends the tool call of a background subagent as cancelled when the SDK reports it stopped while the root is idle", 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", + run_in_background: true, + }); + yield subagentTask(options, "task_started", { + description: "Run a slow command", + subagent_type: "general-purpose", + prompt: "Run it", + is_backgrounded: true, + }); + // Claude Code answers a background Agent call when the subagent launches. + yield toolResultMessage(options, "toolu_agent", {}); + yield toolUseMessage(options, "toolu_child", "Bash", { command: "make" }, "toolu_agent"); + yield result(options); + yield idleState(options); + await interrupted.promise; + // The interrupt stops the subagent while nothing else runs. Claude Code + // sends the interrupted result of its command after that. + yield subagentTask(options, "task_updated", { patch: { status: "killed" } }); + yield subagentTask(options, "task_notification", { + status: "stopped", + output_file: "", + summary: "stopped", + }); + yield { + ...toolResultMessage(options, "toolu_child", { is_error: true }, "user-rejected"), + parent_tool_use_id: "toolu_agent", + }; + yield idleState(options); + }); + + const client = v2Client(); + let root = ""; + await client.app.connectWith(connectRouter(), async (agent) => { + await initializeV2(agent); + ({ sessionId: root } = await agent.request(v2.methods.agent.session.new, { cwd })); + await agent.request(v2.methods.agent.session.prompt, { + sessionId: root, + prompt: text("go"), + }); + await vi.waitFor(() => expect(turnTrace(client.sessionUpdates)).toContain("idle end_turn")); + await agent.notify(v2.methods.agent.session.cancel, { sessionId: root }); + await vi.waitFor(() => + expect( + client + .updates("tool_call_update") + .filter((update) => update.toolCallId === "toolu_child") + .map((update) => update.status), + ).toContain("cancelled"), + ); + await client.authUpdate(1); + }); + + const updates = client.sessionUpdates as unknown as Seen[]; + // The child reported idle first, and then its command ended in its session. + const childIdle = updates.findIndex( + ({ sessionId, update }) => + sessionId === "agent-1" && + update.sessionUpdate === "state_update" && + update.state === "idle" && + update.stopReason === "cancelled", + ); + expect(childIdle).toBeGreaterThanOrEqual(0); + expect(updates[lastStatusAt(updates, "toolu_child")]).toMatchObject({ + sessionId: "agent-1", + update: { status: "cancelled" }, + }); + expect(lastStatusAt(updates, "toolu_child")).toBeGreaterThan(childIdle); + // Claude Code's result for the command was dropped, so only one report ended it. + expect( + updates.filter( + ({ update }) => update.toolCallId === "toolu_child" && update.status === "cancelled", + ), + ).toHaveLength(1); + // The cancel found no work to end: the root reported nothing after its idle. + expect(turnTrace(updates.filter(({ sessionId }) => sessionId === root) as never)).toEqual([ + "user_message", + "running", + "idle end_turn", + ]); + }); }); it("reports a tool call that Claude dropped before it ran as cancelled", async () => { diff --git a/src/tests/incomplete-tools.test.ts b/src/tests/incomplete-tools.test.ts index 8ac60523..e7dc5ddc 100644 --- a/src/tests/incomplete-tools.test.ts +++ b/src/tests/incomplete-tools.test.ts @@ -774,6 +774,114 @@ describe("tool calls of a cancelled turn", () => { }); }); +describe("tool calls of a background subagent that a cancel stops", () => { + /** The reports that end the tool call `id`. */ + const ends = (updates: any[], id: string) => + updates.filter( + (u) => u.toolCallId === id && ["completed", "failed", "cancelled"].includes(u.status), + ); + const textOf = (update: any): string => + (update.content ?? []).map((item: any) => item.content?.text).join(""); + + /** + * Runs the fixture until two background subagents each run a command, and + * the cancel (if any) came. `settlement` is what the cancel finds: a turn + * that v1 holds open for the subagents, which the cancel settles at once, or + * no turn, because it ended earlier. Then the SDK reports how the first + * subagent ended (`reports`). Returns the updates that its reports caused. + */ + async function endFirstSubagent( + settlement: "held turn" | "no turn", + options: { cancel?: boolean; reports?: SubagentEndReports } = {}, + ) { + const { cancel = true, reports = "both" } = options; + const reported = Promise.withResolvers(); + const session = createTestSession((input) => + backgroundSubagentsEndMessages(input, settlement, reported.promise, reports), + ); + const { agent, updates } = session; + const first = session.prompt(); + await vi.waitFor(() => { + const state = agent.sessions[sessionId]; + expect(state.emittedToolCalls.has("bg-child-2")).toBe(true); + if (settlement === "held turn") expect(state.activeTurn?.deferredSettle).toBeDefined(); + }); + if (cancel) await agent.cancel({ sessionId }); + if (settlement === "held turn" && cancel) { + await expect(first).resolves.toMatchObject({ stopReason: "cancelled" }); + } + const before = updates.length; + reported.resolve(); + await agent.sessions[sessionId].consumer; + return { ...session, before, afterReports: updates.slice(before) }; + } + + it.each([ + ["held turn", "both"], + ["no turn", "both"], + // Each report alone ends the subagent, and the SDK can send either. + ["no turn", "update only"], + ["no turn", "notification only"], + ] as const)( + "closes the command of a subagent that the SDK reports stopped after the cancel (%s, %s)", + async (settlement, reports) => { + const { updates, agent, logError, before, afterReports } = await endFirstSubagent( + settlement, + { reports }, + ); + + // The cancel left the command open, since its subagent ran. The SDK then + // stopped the subagent, and Claude Code's result for the command came after + // the work's end, which is dropped like all output after it. + expect(ends(updates.slice(0, before), "bg-child-1")).toEqual([]); + const [end, ...more] = ends(afterReports, "bg-child-1"); + expect(more).toEqual([]); + expect(end).toMatchObject({ + status: "failed", + _meta: { claudeCode: { parentToolUseId: "bg-agent-1" } }, + }); + expect(textOf(end)).toBe("The turn was cancelled before this tool call returned a result."); + expect(logError).not.toHaveBeenCalled(); + // The other subagent runs on, and so does its command. + expect(ends(updates, "bg-child-2")).toEqual([]); + expect([...agent.sessions[sessionId].emittedToolCalls]).toEqual(["bg-child-2"]); + expect([...(agent.sessions[sessionId].toolCallParents ?? [])]).toEqual([ + ["bg-child-2", "bg-agent-2"], + ]); + }, + ); + + it("leaves the command of a subagent that completed after the cancel as it is", async () => { + const { afterReports } = await endFirstSubagent("no turn", { reports: "completed" }); + + // The subagent was not stopped, so nothing says that its command was cancelled. + expect(ends(afterReports, "bg-child-1")).toEqual([]); + }); + + it("leaves a subagent that stopped without a cancel to Claude Code's result", async () => { + const { updates, logError } = await endFirstSubagent("no turn", { cancel: false }); + + // For example, the model stops the subagent. Its command gets Claude Code's + // own result, and no report is made up for it first. + const [end, ...more] = ends(updates, "bg-child-1"); + expect(more).toEqual([]); + expect(textOf(end)).toContain("doesn't want to proceed with this tool use"); + expect(logError).not.toHaveBeenCalled(); + }); + + it("leaves the command of a subagent that stopped before the work's end to Claude Code's result", async () => { + const { prompt, updates, logError } = createTestSession(stoppedMidTurnSubagentMessages); + + await expect(prompt()).resolves.toMatchObject({ stopReason: "cancelled" }); + + // The turn still runs, so the result of the command reaches the client. + const [end, ...more] = ends(updates, "bg-child-1"); + expect(more).toEqual([]); + expect(textOf(end)).toContain("doesn't want to proceed with this tool use"); + expect(logError).not.toHaveBeenCalled(); + }); +}); + function toolStart(id = toolCallId, parent: string | null = null, name = "Bash") { return { type: "stream_event", @@ -1174,6 +1282,71 @@ async function* backgroundSubagentMessages(input: Pushable, agent: ClaudeAc yield abortedResult(); } +/** The reports with which the SDK ends a subagent: a patch and a notification, or one of them. */ +type SubagentEndReports = "both" | "update only" | "notification only" | "completed"; + +/** + * Two background subagents each run a command. With a "held turn", the turn's + * result came while they run, so v1 holds the turn for them; with "no turn", + * it ended before they started. After `reported`, the SDK reports how the first + * subagent ended, and Claude Code's result for its command. + */ +async function* backgroundSubagentsEndMessages( + input: Pushable, + settlement: "held turn" | "no turn", + reported: Promise, + reports: SubagentEndReports, +) { + yield* echoNextPrompt(input); + if (settlement === "no turn") yield successfulResultMessage(); + for (const n of [1, 2]) { + yield toolStart(`bg-agent-${n}`, null, "Agent"); + yield toolUse(`bg-agent-${n}`, null, "Agent", { description: "Work", prompt: "Work" }); + yield taskStarted(`agent-bg-${n}`, `bg-agent-${n}`, "general-purpose", true); + yield toolResult(`bg-agent-${n}`); + yield toolStart(`bg-child-${n}`, `bg-agent-${n}`); + yield toolUse(`bg-child-${n}`, `bg-agent-${n}`); + } + if (settlement === "held turn") yield successfulResultMessage(); + await reported; + const done = { type: "system", session_id: sessionId, task_id: "agent-bg-1" }; + if (reports === "completed") { + yield { ...done, subtype: "task_updated", patch: { status: "completed" } }; + yield { + ...done, + subtype: "task_notification", + tool_use_id: "bg-agent-1", + status: "completed", + summary: "done", + output_file: "", + }; + } else { + if (reports !== "notification only") { + yield { ...done, subtype: "task_updated", patch: { status: "killed" } }; + } + if (reports !== "update only") yield taskStopped("agent-bg-1", "bg-agent-1"); + // Claude Code's result for the command comes after the stop. + yield interruptedResult("bg-child-1", "bg-agent-1"); + } + yield { type: "system", subtype: "session_state_changed", state: "idle" }; +} + +/** A background subagent stops while the cancelled turn still runs, and Claude Code sends the result of its command. */ +async function* stoppedMidTurnSubagentMessages(input: Pushable, agent: ClaudeAcpAgent) { + yield* echoNextPrompt(input); + yield toolStart("bg-agent-1", null, "Agent"); + yield toolUse("bg-agent-1", null, "Agent", { description: "Work", prompt: "Work" }); + yield taskStarted("agent-bg-1", "bg-agent-1", "general-purpose", true); + yield toolResult("bg-agent-1"); + yield toolStart("bg-child-1", "bg-agent-1"); + yield toolUse("bg-child-1", "bg-agent-1"); + await agent.cancel({ sessionId }); + yield taskStopped("agent-bg-1", "bg-agent-1"); + yield interruptedResult("bg-child-1", "bg-agent-1"); + yield interruptNote; + yield abortedResult(); +} + /** A turn that the SDK never answers after the cancel, with a tool that ran and one that did not. */ async function* wedgedToolMessages(input: Pushable) { yield* echoNextPrompt(input);