diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 055def9a..044a2514 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -4912,6 +4912,18 @@ export class ClaudeAcpAgent { task.parentToolUseId === agentToolUseId, ); + /** Whether output of the conversation `parentToolUseId` (null for the main + * one) is a background subagent's that still runs. A cancel does not stop + * such a subagent's work (agentclientprotocol/agent-client-protocol#2308): + * the closing at the cancel leaves its tool calls open for their results + * (`closedAtCancel`), and the window after the cancel lets its output + * through (see the `session.cancelled` checks of the stream and the + * user/assistant cases). Claude Code currently stops background subagents + * at an interrupt, so this decides only the output that comes before the + * SDK reports a subagent's end. */ + const isRunningBackgroundSubagentOutput = (parentToolUseId: string | null): boolean => + parentToolUseId !== null && runsInBackground(parentToolUseId); + /** 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. */ @@ -7217,7 +7229,8 @@ export class ClaudeAcpAgent { // `cancelled` check in the user/assistant case). Otherwise the // answer to a prompt the cancel abandoned, which Claude Code had // already taken off its queue, would stream to the client after the - // cancel was reported. + // cancel was reported. A background subagent that still runs is not + // work the cancel stopped (see isRunningBackgroundSubagentOutput). // // Where the consolidated message delivers the text (the same // condition as in the user/assistant case), a stream that loses a @@ -7229,7 +7242,9 @@ export class ClaudeAcpAgent { // prefix of the text, so the text would be sent again (see // unstreamedRemainder). const streamKey = message.parent_tool_use_id ?? ""; - const stoppedWorkOutput = this.dropsStoppedWorkOutput(params.sessionId, session); + const stoppedWorkOutput = + this.dropsStoppedWorkOutput(params.sessionId, session) && + !isRunningBackgroundSubagentOutput(message.parent_tool_use_id); if ( message.event.type === "content_block_start" && isToolUseBlock(message.event.content_block) @@ -7319,7 +7334,9 @@ export class ClaudeAcpAgent { recordDispatchedToolUses(session, message.message.content); // A tool use that did not stream is reached here first. const content = message.message.content; - const stoppedWorkOutput = this.dropsStoppedWorkOutput(params.sessionId, session); + const stoppedWorkOutput = + this.dropsStoppedWorkOutput(params.sessionId, session) && + !isRunningBackgroundSubagentOutput(message.parent_tool_use_id); for (const block of Array.isArray(content) ? content : []) { if (isToolUseBlock(block)) { recordReachedToolUse(session, block.id, stoppedWorkOutput); @@ -7432,11 +7449,15 @@ export class ClaudeAcpAgent { } } - if (session.cancelled) { + if ( + session.cancelled && + !isRunningBackgroundSubagentOutput(message.parent_tool_use_id) + ) { // The work the cancel stopped ends its tools with error results. // Until the client is told that the work ended, they close the // tool calls it opened. Everything else that work sends is - // dropped, and so is everything after that report. + // dropped, and so is everything after that report. A background + // subagent that still runs is not that work: its messages go on. if (message.type === "user" && !this.workEndReported(params.sessionId, session)) { await sendInterruptedToolResults(message); } diff --git a/src/tests/incomplete-tools.test.ts b/src/tests/incomplete-tools.test.ts index 9a46aa60..effdb021 100644 --- a/src/tests/incomplete-tools.test.ts +++ b/src/tests/incomplete-tools.test.ts @@ -909,11 +909,93 @@ describe("tool calls of a background subagent that a cancel stops", () => { }, ); - it("leaves the command of a subagent that completed after the cancel as it is", async () => { - const { afterReports } = await endFirstSubagent("no turn", { reports: "completed" }); + it.each(["held turn", "no turn"] as const)( + "delivers the result of a command of a subagent that the cancel did not stop, which then completes (%s)", + async (settlement) => { + const { afterReports, agent, logError } = await endFirstSubagent(settlement, { + reports: "completed", + }); - // The subagent was not stopped, so nothing says that its command was cancelled. - expect(ends(afterReports, "bg-child-1")).toEqual([]); + // The cancel did not stop the subagent: its command's own result reaches + // the client, though the cancel was reported, and nothing is made up. + const [end, ...more] = ends(afterReports, "bg-child-1"); + expect(more).toEqual([]); + expect(end).toMatchObject({ + status: "completed", + _meta: { claudeCode: { parentToolUseId: "bg-agent-1" } }, + }); + expect(agent.sessions[sessionId].emittedToolCalls.has("bg-child-1")).toBe(false); + expect(logError).not.toHaveBeenCalled(); + }, + ); + + it("delivers what a subagent that the cancel did not stop does after the cancel was reported", async () => { + const { afterReports, agent } = await endFirstSubagent("held turn", { + reports: "new work", + }); + + // It starts and runs another command: the streamed start, the complete + // message, and the result all reach the client. + expect( + afterReports.filter((u) => u.toolCallId === "bg-child-3").map((u) => u.sessionUpdate), + ).toEqual(expect.arrayContaining(["tool_call", "tool_call_update"])); + expect(ends(afterReports, "bg-child-3").map((u) => u.status)).toEqual(["completed"]); + expect(agent.sessions[sessionId].emittedToolCalls.has("bg-child-3")).toBe(false); + }); + + it("lets a subagent that the cancel did not stop ask for permissions after the cancel was reported", async () => { + const decisions: unknown[] = []; + const reported = Promise.withResolvers(); + const { agent, prompt, updates } = createTestSession(async function* (input, agent) { + yield* echoNextPrompt(input); + yield toolStart("bg-agent", null, "Agent"); + yield toolUse("bg-agent", null, "Agent", { description: "Work", prompt: "Work" }); + yield taskStarted("agent-bg", "bg-agent", "general-purpose", true); + yield toolResult("bg-agent"); + // v1 holds the turn for the subagent. + yield successfulResultMessage(); + await reported.promise; + const ask = (id: string) => + agent.canUseTool(sessionId)("Bash", { command: "ls" }, { + toolUseID: id, + agentID: "agent-bg", + signal: new AbortController().signal, + suggestions: [], + } as any); + // A tool use that streams first, and one that only its complete message holds. + yield toolStart("bg-streamed", "bg-agent"); + decisions.push(await ask("bg-streamed")); + yield toolUse("bg-whole", "bg-agent"); + decisions.push(await ask("bg-whole")); + yield { type: "system", subtype: "session_state_changed", state: "idle" }; + }); + const first = prompt(); + await vi.waitFor(() => + expect(agent.sessions[sessionId].activeTurn?.deferredSettle).toBeDefined(), + ); + await agent.cancel({ sessionId }); + await expect(first).resolves.toMatchObject({ stopReason: "cancelled" }); + reported.resolve(); + await agent.sessions[sessionId].consumer; + + // #1334 denies only the tools of the work that the cancel stopped. + expect(decisions).toEqual([ + expect.objectContaining({ behavior: "allow" }), + expect.objectContaining({ behavior: "allow" }), + ]); + expect(updates.filter((u) => u.sessionUpdate === "tool_call").map((u) => u.toolCallId)).toEqual( + expect.arrayContaining(["bg-streamed", "bg-whole"]), + ); + }); + + it("still drops what a stopped subagent sends after its stop was reported", async () => { + const { afterReports } = await endFirstSubagent("held turn"); + + // The SDK reported the subagent killed first, so its command's late result + // is the stopped work's, and the made-up report closed the call instead. + expect(ends(afterReports, "bg-child-1").map(textOf)).toEqual([ + "The turn was cancelled before this tool call returned a result.", + ]); }); it("leaves a subagent that stopped without a cancel to Claude Code's result", async () => { @@ -1341,7 +1423,7 @@ async function* backgroundSubagentMessages(input: Pushable, agent: ClaudeAc } /** 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"; +type SubagentEndReports = "both" | "update only" | "notification only" | "completed" | "new work"; /** * Two background subagents each run a command. With a "held turn", the turn's @@ -1376,7 +1458,14 @@ async function* backgroundSubagentsEndMessages( } await reported; const done = { type: "system", session_id: sessionId, task_id: "agent-bg-1" }; - if (reports === "completed") { + if (reports === "new work") { + // The subagent runs on, and starts more work. + yield toolStart("bg-child-3", "bg-agent-1"); + yield toolUse("bg-child-3", "bg-agent-1"); + yield toolResult("bg-child-3", "bg-agent-1"); + } else if (reports === "completed") { + // The command ends with its own result, and then the subagent completes. + yield toolResult("bg-child-1", "bg-agent-1"); yield { ...done, subtype: "task_updated", patch: { status: "completed" } }; yield { ...done,