mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 12:40:32 +00:00
fix: end the tool calls of a cycle that Claude Code started and a cancel stopped, on v1 (#1342)
On v1, when a cancel stops a model cycle that Claude Code started on its own, the cycle's open tool calls now end. Before, they stayed `pending` for good. ## The bug Example, from a live run (Claude Code 2.1.296, a v1 client): 1. A prompt starts a background command, and its turn ends. 2. The command finishes, and Claude Code starts a cycle on its own: it launches a foreground subagent, which runs a command. 3. The client sends `session/cancel` while no prompt runs. Claude Code stops both and sends the Agent call's interrupted result (`user-rejected`). The client never got it, and the Agent call stayed `pending`. Before #1336, the subagent's command stayed `pending` too. **Why:** the window after a cancel drops output once the client has been told that the stopped work ended (#1313). On v1 that report is the prompt response, read as "no active turn". A cycle never has a turn, so from the cancel on, everything it sent counted as late output, including Claude Code's results for its own tool calls. v2 tracks a cycle as foreground work and closes its calls before `idle cancelled` (#1339). v1 tracked nothing. ## The fix - **The agent tracks foreground work on every connection.** Only v2 reports it, through the `foreground` option; v1 gets events that do nothing (`UNREPORTED_FOREGROUND`). The rules for when a cycle starts and ends are the ones v2 already uses. - **On v1, a cycle's end is its result or its idle** (`workEndReported`). Until then, Claude Code's interrupted results reach the client. At that end, `closeCancelledCycleToolCalls` closes the calls that got none, as on v2. After it, the window drops what comes, as before. A v1 turn's rule is unchanged: no active turn. - **A held turn's follow-up shares the held turn's closing.** v1 holds a turn for its background subagents, and the held turn's follow-up is a cycle too, now on v1 as well. When a second cancel settled the held turn in `cancelTurns` while the cycle's closing was in flight, the two ran separate passes. The response then came before the cycle's first report, and on v2 a call could be closed twice. `closeCancelledCycleToolCalls` now uses the held turn's closing (`Turn.closingCancelledToolCalls`), which every settle of the turn waits for. The existing test "closes each tool call once, before the response, when a cancel settles a held turn that the consumer is closing" caught this. ## Evidence **Live, Claude Code 2.1.296, the scenario above**, every update checked against the schema (no violations): | Client | Agent call | Subagent's command | |---|---|---| | v1, before | `pending` | `failed` (closed by #1336) | | v1, this PR | `failed`, with Claude Code's own `user-rejected` result | `failed`, closed at the cycle's end | | v1 RFD client, this PR | the same | the same, in the child's session | | v2, this PR | `cancelled`, before `idle cancelled` | `cancelled`, before `idle cancelled` (unchanged) | A v1 cancel with a queued prompt, followed by the next prompt, still left no tool call open, and the next prompt's Write asked for its permission and ran. **Tests** (`src/tests/incomplete-tools.test.ts`): - **#1339's test for v1 now asserts the fix:** a cycle's command gets Claude Code's interrupted result. - **New:** the live scenario, where the Agent call gets Claude Code's result and the subagent's command is closed; and a cycle that ends at an idle with no result, where its call is closed at the idle and Claude Code's later result is dropped. - **#1336's "no turn" fixture** launched the background subagents in a cycle it never ended before the cancel, so v1 now delivered Claude Code's real result there. The fixture now ends that cycle, which is the "cancel while nothing runs" case the tests mean. - **Mutation checks:** each of these fails at least one test: - the v1 rule without the cycle; - no cycles on v1; - no foreground tracking on v1; - a separate closing for a held turn's follow-up. `npm run check`, build and the full suite (2979 tests) pass, with no golden changes. ## Not covered - A cycle that ends normally with a `tool_use` that Claude abandoned keeps that call open. - A subagent that completes after a cancel keeps its calls open.
This commit is contained in:
@@ -1422,6 +1422,16 @@ const AUTONOMOUS_RESULT_ORIGINS: ReadonlySet<SDKMessageOrigin["kind"]> = new Set
|
||||
const AUTONOMOUS_CYCLE = "autonomous-cycle";
|
||||
type ForegroundOwner = Turn | typeof AUTONOMOUS_CYCLE;
|
||||
|
||||
/** The events of a connection that reports no foreground work: v1, whose
|
||||
* client has no session state. The agent still tracks the work, which
|
||||
* decides what the window after a cancel drops (see `workEndReported`). */
|
||||
const UNREPORTED_FOREGROUND: ForegroundEvents = {
|
||||
running: () => {},
|
||||
awaitingUser: () => {},
|
||||
ended: () => {},
|
||||
failed: () => {},
|
||||
};
|
||||
|
||||
/** Whether this turn's terminal result arrived but its settlement is being
|
||||
* held for background subagents it spawned (see Turn.deferredSettle). The
|
||||
* single spelling of the hold predicate, shared by the consumer's settle
|
||||
@@ -2608,7 +2618,8 @@ export class ClaudeAcpAgent {
|
||||
private readonly foregroundEvents?: (sessionId: string) => ForegroundEvents;
|
||||
/**
|
||||
* The foreground work of each session while it runs, by the session id that
|
||||
* the client knows. A clear-context restart replaces the Claude Code session
|
||||
* the client knows, on every connection: only the `foreground` option
|
||||
* reports it. A clear-context restart replaces the Claude Code session
|
||||
* behind that id and moves the running turn to it, so the work belongs to the
|
||||
* id, not to a {@link Session}.
|
||||
*/
|
||||
@@ -2632,8 +2643,9 @@ export class ClaudeAcpAgent {
|
||||
/**
|
||||
* Where the agent reports a session's foreground work (see
|
||||
* {@link ForegroundEvents}), called each time work starts in the session
|
||||
* `sessionId`. Without it, nothing reports the work: a v1 client has no
|
||||
* session state, and learns that a turn ended from its prompt response.
|
||||
* `sessionId`. Without it, nothing reports the work, which the agent
|
||||
* still tracks: a v1 client has no session state, and learns that a turn
|
||||
* ended from its prompt response.
|
||||
*/
|
||||
foreground?: (sessionId: string) => ForegroundEvents;
|
||||
} = {},
|
||||
@@ -5017,10 +5029,17 @@ export class ClaudeAcpAgent {
|
||||
* gives the tools of the main conversation results, which
|
||||
* `sendInterruptedToolResults` delivers while the cycle runs. What is still
|
||||
* open has none, such as the command of a foreground subagent. A turn that
|
||||
* took the cycle over closes its own calls, when it settles. */
|
||||
* took the cycle over closes its own calls, when it settles.
|
||||
* The follow-up of a turn that v1 holds for its background subagents is a
|
||||
* cycle too, and the cancel ends the held turn as well, possibly in
|
||||
* `cancelTurns` while these reports are in flight. So the cycle's calls
|
||||
* close with the held turn's closing, which each settle of the turn waits
|
||||
* for, and the response never comes before them. */
|
||||
const closeCancelledCycleToolCalls = async (): Promise<void> => {
|
||||
if (!session.cancelled || !this.runsAutonomousCycle(params.sessionId)) return;
|
||||
await closeOpenToolCalls(closedAtCancel);
|
||||
const held = session.activeTurn;
|
||||
if (held && !held.settled) await closeCancelledToolCalls(held);
|
||||
else await closeOpenToolCalls(closedAtCancel);
|
||||
};
|
||||
|
||||
/** At the actual turn boundary, preview its checkpoint, settle the active
|
||||
@@ -9097,13 +9116,12 @@ export class ClaudeAcpAgent {
|
||||
* reports nothing new: a turn takes over a cycle that Claude Code started on
|
||||
* its own, and only the owner's end ends the work. */
|
||||
private startForeground(sessionId: string, owner: ForegroundOwner): void {
|
||||
if (!this.foregroundEvents) return;
|
||||
const current = this.foregroundWork.get(sessionId);
|
||||
if (current) {
|
||||
current.owner = owner;
|
||||
return;
|
||||
}
|
||||
const events = this.foregroundEvents(sessionId);
|
||||
const events = this.foregroundEvents?.(sessionId) ?? UNREPORTED_FOREGROUND;
|
||||
this.foregroundWork.set(sessionId, { owner, events, awaitingUser: false });
|
||||
events.running();
|
||||
const session = this.sessions[sessionId];
|
||||
@@ -9131,7 +9149,7 @@ export class ClaudeAcpAgent {
|
||||
* whose work ended at its result, so its follow-up is a cycle. Output of
|
||||
* the work a cancel stopped starts nothing (see Session.cancelled). */
|
||||
private startAutonomousCycle(sessionId: string, session: Session, message: SDKMessage): void {
|
||||
if (!this.foregroundEvents || this.foregroundWork.has(sessionId) || session.cancelled) return;
|
||||
if (this.foregroundWork.has(sessionId) || session.cancelled) return;
|
||||
if (message.type !== "assistant" && message.type !== "stream_event") return;
|
||||
if (message.parent_tool_use_id !== null) return;
|
||||
// A turn that Claude Code started is the foreground work already. Without
|
||||
@@ -9148,9 +9166,13 @@ export class ClaudeAcpAgent {
|
||||
/** Whether the client has been told that the session's work ended: on v2 by
|
||||
* an idle state, so no foreground work is open, including a cycle that
|
||||
* Claude Code started on its own; on v1 by the response to the turn's
|
||||
* prompt, so no turn is active. */
|
||||
* prompt, so no turn is active. v1 reports nothing for a cycle, so its end
|
||||
* is its result or idle: until then, Claude Code's results for the cycle's
|
||||
* tool calls still reach the client after a cancel. */
|
||||
private workEndReported(sessionId: string, session: Session): boolean {
|
||||
return this.foregroundEvents ? !this.foregroundWork.has(sessionId) : !session.activeTurn;
|
||||
return this.foregroundEvents
|
||||
? !this.foregroundWork.has(sessionId)
|
||||
: !session.activeTurn && !this.runsAutonomousCycle(sessionId);
|
||||
}
|
||||
|
||||
/** Whether the consumer drops what it reaches now as output of the work a
|
||||
@@ -9225,8 +9247,7 @@ export class ClaudeAcpAgent {
|
||||
}
|
||||
|
||||
/** Whether a model cycle that Claude Code started on its own is the session's
|
||||
* foreground work. Only a client that gets the session's work reported (v2)
|
||||
* has any; a turn that took a cycle over owns the work from then on. */
|
||||
* foreground work. A turn that took a cycle over owns the work from then on. */
|
||||
private runsAutonomousCycle(sessionId: string): boolean {
|
||||
return this.foregroundWork.get(sessionId)?.owner === AUTONOMOUS_CYCLE;
|
||||
}
|
||||
|
||||
@@ -721,17 +721,62 @@ describe("tool calls of a cancelled turn", () => {
|
||||
expect(textOf(ends(updates)[0])).toBe("Claude stopped this tool call before it ran.");
|
||||
});
|
||||
|
||||
it("does not close the tool calls of a cycle that Claude Code started on its own, which v1 does not report", async () => {
|
||||
const { prompt, updates, agent } = createTestSession(cancelledCycleMessages);
|
||||
describe("of a cycle that Claude Code started on its own, which v1 does not report", () => {
|
||||
it("delivers Claude Code's interrupted result for the cycle's command", async () => {
|
||||
const { prompt, updates, agent, logError } = createTestSession(cancelledCycleMessages);
|
||||
|
||||
await expect(prompt()).resolves.toMatchObject({ stopReason: "end_turn" });
|
||||
await agent.sessions[sessionId].consumer;
|
||||
await expect(prompt()).resolves.toMatchObject({ stopReason: "end_turn" });
|
||||
await agent.sessions[sessionId].consumer;
|
||||
|
||||
// v1 sends no state for such a cycle, so no prompt response or idle follows
|
||||
// the cycle's end, and its output after the cancel is dropped. v2 closes the
|
||||
// tool calls that the cycle left open as it reports the cycle's end.
|
||||
expect(ends(updates, "cycle-tool")).toEqual([]);
|
||||
expect(agent.sessions[sessionId].emittedToolCalls.has("cycle-tool")).toBe(true);
|
||||
// v1 reports nothing for the cycle, so it ends at its result: until then,
|
||||
// Claude Code's result for its command still reaches the client.
|
||||
const [end, ...more] = ends(updates, "cycle-tool");
|
||||
expect(more).toEqual([]);
|
||||
expect(end).toMatchObject({
|
||||
status: "failed",
|
||||
_meta: { claudeCode: { nonExecutionKind: "user-rejected" } },
|
||||
});
|
||||
expect(textOf(end)).toContain("doesn't want to proceed with this tool use");
|
||||
expect(agent.sessions[sessionId].emittedToolCalls.size).toBe(0);
|
||||
expect(logError).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("delivers the result of its Agent call, and closes the command of the subagent, which Claude Code reports no result for", async () => {
|
||||
const { prompt, updates, agent, logError } = createTestSession(cancelledCycleAgentMessages);
|
||||
|
||||
await expect(prompt()).resolves.toMatchObject({ stopReason: "end_turn" });
|
||||
await agent.sessions[sessionId].consumer;
|
||||
|
||||
const [agentEnd, ...moreAgentEnds] = ends(updates, "cycle-agent");
|
||||
expect(moreAgentEnds).toEqual([]);
|
||||
expect(textOf(agentEnd)).toContain("doesn't want to proceed with this tool use");
|
||||
const [childEnd, ...moreChildEnds] = ends(updates, "cycle-child");
|
||||
expect(moreChildEnds).toEqual([]);
|
||||
expect(childEnd).toMatchObject({
|
||||
status: "failed",
|
||||
_meta: { claudeCode: { parentToolUseId: "cycle-agent" } },
|
||||
});
|
||||
expect(textOf(childEnd)).toBe(
|
||||
"The turn was cancelled before this tool call returned a result.",
|
||||
);
|
||||
expect(agent.sessions[sessionId].emittedToolCalls.size).toBe(0);
|
||||
expect(logError).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("closes a call that gets no result at the idle that ends the cycle, and drops what comes after", async () => {
|
||||
const { prompt, updates, agent, logError } = createTestSession(cycleEndedByIdleMessages);
|
||||
|
||||
await expect(prompt()).resolves.toMatchObject({ stopReason: "end_turn" });
|
||||
await agent.sessions[sessionId].consumer;
|
||||
|
||||
// Claude Code's result comes only after the idle that ended the cycle, so
|
||||
// the call was closed at the idle, and the late result is dropped.
|
||||
const [end, ...more] = ends(updates, "cycle-tool");
|
||||
expect(more).toEqual([]);
|
||||
expect(textOf(end)).toBe("The turn was cancelled before this tool call returned a result.");
|
||||
expect(agent.sessions[sessionId].emittedToolCalls.size).toBe(0);
|
||||
expect(logError).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it("drops Claude Code's result for a tool call once the cancelled turn has ended", async () => {
|
||||
@@ -1301,7 +1346,8 @@ type SubagentEndReports = "both" | "update only" | "notification only" | "comple
|
||||
/**
|
||||
* 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
|
||||
* Claude Code's own follow-up started them after the turn ended, and that
|
||||
* follow-up ended too. After `reported`, the SDK reports how the first
|
||||
* subagent ended, and Claude Code's result for its command.
|
||||
*/
|
||||
async function* backgroundSubagentsEndMessages(
|
||||
@@ -1311,7 +1357,10 @@ async function* backgroundSubagentsEndMessages(
|
||||
reports: SubagentEndReports,
|
||||
) {
|
||||
yield* echoNextPrompt(input);
|
||||
if (settlement === "no turn") yield successfulResultMessage();
|
||||
if (settlement === "no turn") {
|
||||
yield successfulResultMessage();
|
||||
yield { type: "system", subtype: "session_state_changed", state: "idle" };
|
||||
}
|
||||
for (const n of [1, 2]) {
|
||||
yield toolStart(`bg-agent-${n}`, null, "Agent");
|
||||
yield toolUse(`bg-agent-${n}`, null, "Agent", { description: "Work", prompt: "Work" });
|
||||
@@ -1321,6 +1370,10 @@ async function* backgroundSubagentsEndMessages(
|
||||
yield toolUse(`bg-child-${n}`, `bg-agent-${n}`);
|
||||
}
|
||||
if (settlement === "held turn") yield successfulResultMessage();
|
||||
else {
|
||||
yield successfulResultMessage({ origin: { kind: "task-notification" } });
|
||||
yield { type: "system", subtype: "session_state_changed", state: "idle" };
|
||||
}
|
||||
await reported;
|
||||
const done = { type: "system", session_id: sessionId, task_id: "agent-bg-1" };
|
||||
if (reports === "completed") {
|
||||
@@ -1391,6 +1444,41 @@ async function* cancelledCycleMessages(input: Pushable<any>, agent: ClaudeAcpAge
|
||||
yield { type: "system", subtype: "session_state_changed", state: "idle" };
|
||||
}
|
||||
|
||||
/**
|
||||
* After the turn ended, Claude Code's own follow-up runs a foreground subagent
|
||||
* that runs a command, and the cancel stops both. As seen live: the SDK reports
|
||||
* the subagent stopped, and Claude Code sends the Agent call's interrupted
|
||||
* result, but none for the subagent's command.
|
||||
*/
|
||||
async function* cancelledCycleAgentMessages(input: Pushable<any>, agent: ClaudeAcpAgent) {
|
||||
yield* echoNextPrompt(input);
|
||||
yield successfulResultMessage();
|
||||
yield { type: "system", subtype: "session_state_changed", state: "idle" };
|
||||
yield toolStart("cycle-agent", null, "Agent");
|
||||
yield toolUse("cycle-agent", null, "Agent", { description: "Work", prompt: "Work" });
|
||||
yield taskStarted("agent-fg", "cycle-agent", "general-purpose");
|
||||
yield toolStart("cycle-child", "cycle-agent");
|
||||
yield toolUse("cycle-child", "cycle-agent");
|
||||
await agent.cancel({ sessionId });
|
||||
yield taskStopped("agent-fg", "cycle-agent");
|
||||
yield interruptedResult("cycle-agent");
|
||||
yield interruptNote;
|
||||
yield { ...abortedResult(), origin: { kind: "task-notification" } };
|
||||
yield { type: "system", subtype: "session_state_changed", state: "idle" };
|
||||
}
|
||||
|
||||
/** A cancelled follow-up ends at an idle with no result, and Claude Code's result for its command comes after. */
|
||||
async function* cycleEndedByIdleMessages(input: Pushable<any>, agent: ClaudeAcpAgent) {
|
||||
yield* echoNextPrompt(input);
|
||||
yield successfulResultMessage();
|
||||
yield { type: "system", subtype: "session_state_changed", state: "idle" };
|
||||
yield toolStart("cycle-tool");
|
||||
yield toolUse("cycle-tool");
|
||||
await agent.cancel({ sessionId });
|
||||
yield { type: "system", subtype: "session_state_changed", state: "idle" };
|
||||
yield interruptedResult("cycle-tool");
|
||||
}
|
||||
|
||||
/** The result of the turn comes before Claude Code's result for the tool. */
|
||||
async function* lateInterruptedResultMessages(
|
||||
input: Pushable<any>,
|
||||
|
||||
Reference in New Issue
Block a user