mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 04:30:59 +00:00
fix: deliver the output of a background subagent that a cancel did not stop (#1345)
After a cancel, the output of a background subagent that the cancel did not stop now reaches the client: its tool results, its new tool calls, and its permission requests. ## The bug The adapter has one rule for work that a cancel does not stop, as agentclientprotocol/agent-client-protocol#2308 asks: a background subagent that still runs (`runsInBackground`). The closing at the cancel leaves that subagent's tool calls open for their results (`closedAtCancel`). But the window after the cancel dropped the same subagent's output, results included: - **A subagent that completes after the cancel** kept its calls open. Its commands' results came after the cancel was reported and were dropped, and since it was not stopped, nothing closed them. - **A subagent that runs on** after the cancel showed nothing more: new tool calls were dropped, and its permission requests were denied as the stopped work's (#1334). Claude Code currently stops background subagents at an interrupt, as live runs show, so today this applies only to the output that comes before the SDK reports a subagent's end. It decides the rest once Claude Code lets them run. ## The fix `isRunningBackgroundSubagentOutput` uses the closing's predicate for the three places the window applies: - the drop of streamed output; - the record that `canUseTool` and the v2 preview hook read (#1334); - the guard of the user/assistant messages. A subagent's output after the SDK reported its end is still dropped, and #1336 closes its calls. ## Evidence **Tests** (`src/tests/incomplete-tools.test.ts`): - **A command's own result,** for a subagent that completes after the cancel, reaches the client, for a held turn and with no turn. This replaces "leaves the command of a subagent that completed after the cancel as it is", whose fixture sent no result at all. - **New work after the cancel was reported:** a command's streamed start, complete message and result reach the client. - **Permission requests after the cancel:** they are allowed, for a tool use that streamed first and for one that only its complete message holds. - **Unchanged:** a stopped subagent's late result is still dropped, and its call closed. - **Mutation checks:** removing each of the three uses, or widening the predicate to every subagent, fails at least one test. **Live, Claude Code 2.1.296,** a v1 cancel of a turn held for a background subagent that runs a command, on a plain client and an RFD client: Claude Code stopped the subagent, and its command was closed after the cancelled response, as on `main`. No schema violations. A subagent that survives a cancel cannot be set up live today. `npm run check`, build and the full suite (2983 tests) pass, with no golden changes. ## Not covered - **A foreground subagent nested in a background one:** its own Agent call is not backgrounded, so the closing closes its calls at the cancel, and the window drops its output, both as before. The two stay consistent.
This commit is contained in:
Ben Brandt
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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<void>();
|
||||
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<any>, 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,
|
||||
|
||||
Reference in New Issue
Block a user