mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 12:40:32 +00:00
fix: close the open tool calls of a held or settling turn that a cancel ends (#1221)
Refs #1061. > Reworked on top of #1327, which merged in the meantime and already closes a cancelled turn's open tool calls in the consumer. This PR now covers the one path #1327 does not reach. The original description is kept at the end. ## What it fixes When a cancel lands on a turn that already has its result, `cancelTurns` settles the turn itself instead of leaving it to the consumer: - a turn held open for its background subagents, while the session is idle; - a turn waiting on its checkpoint preview. That path skipped #1327's `closeCancelledToolCalls`, so a tool call still open there stayed `pending` after the cancelled response. `cancelTurns` now runs the same closing before it responds. ## Changes - **One rule for both paths.** The consumer exposes its closing as `Session.closeCancelledToolCalls`, the way it already exposes `nativeSubagentDeliver`. Every open call ends with `AcpToolCallRenderer.unfinished`, except a background subagent's calls, which the cancel did not stop. - **The closing runs once per turn** (`Turn.closingCancelledToolCalls`). Example: a first cancel lands during a held turn's follow-up cycle, so the consumer settles the turn at the idle and starts closing its calls. A second cancel then settles the turn inline. It waits for the consumer's closing instead of starting its own, so each call gets one report and the response comes after all of them. - **`cancelTurns`** marks the turn settled before it waits for the reports, so the consumer cannot settle it again. If a report fails, it logs the error and still responds and interrupts. - **Removed:** this PR's original consumer-side branch and its `failed` report, which #1327's closing replaces. ## Tests - **Kept from this PR:** inline settles of a deferred turn and of a checkpoint turn, overlapping and simultaneous cancels, a failed report, a checkpoint that finishes during the closing. - **Changed:** the deferred fixture's background shell had no tool result, though Claude Code answers a `run_in_background` call when it starts. It is now a background subagent whose own command stays open. - **Removed:** `terminates a tool whose start streams in after the cancel`, which #1327 covers. - **New:** a second cancel that settles a held turn while the consumer is closing its calls. - **Removal checks:** removing the inline closing fails 6 tests, the once-per-turn closing 1, settling before the wait 2, and the error handling 1. `npm run check`, build and the full suite (2909 tests) pass, with no golden changes. ## Not covered A tool start that streams in after the turn has settled (the incident in #1061) still opens a tool call that nothing closes. Neither this PR nor #1327 changes that. <details> <summary>Original description</summary> Refs #1061. This closes part of the issue, not all of it; the part it leaves open is described below. ## What it fixes `settleActive` already fails every unfinished foreground tool when a turn ends as `end_turn` ("Claude ended the turn without returning a result for this tool"). It did nothing when the turn settled as `cancelled`, so a tool start that never got a `tool_result` stayed in `emittedToolCalls` and the client was left with an open `tool_call` forever. How a start can end up with no terminal: the `stream_event` case has no `session.cancelled` gate, so `content_block_start` for a `tool_use` keeps becoming a `tool_call` after `cancel()`. The consolidated `user`/`assistant` path, which is where `tool_result`-derived terminals come from, drops everything at its `session.cancelled` guard. This adds a `cancelled` branch next to the `end_turn` one. It reuses the same selection (this turn's `foregroundToolCallIds` still in `emittedToolCalls`, minus confirmed background tasks) and sends one `tool_call_update` with `status: "failed"` per tool ("The turn was cancelled before this tool call finished."). ACP's `ToolCallStatus` has no cancelled value, so `failed` is the available terminal. Unlike `end_turn`, cancellation is expected, so there is no session failure and no early return. ## What it does not fix A tool start that arrives **after** the turn has already settled (the reporter's incident: starts streaming in minutes after the cancel). There is no unsettled turn to attach it to, and I could not find an exact gate: stream events carry no `origin` or turn attribution (the existing comment on `emittedAssistantText` says as much), so such a start cannot be told apart from an autonomous cycle or from the next turn's pre-echo events. Gating on `session.cancelled` has the inverse problem the reporter described. If maintainers see an attribution signal I missed, a lifecycle-scoped gate would be the complement to this change. ## Test changes - `preserves cancellation and does not attribute its unfinished tool to the next turn` asserted that the cancelled turn's tool gets **no** `failed` update. That encoded the old behaviour, so it now asserts exactly one cancellation terminal that the next turn neither repeats nor reports as `incomplete_tool_call`. The cancel-then-next-prompt assertions are unchanged. - New: `terminates a tool whose start streams in after the cancel` (start arrives between `cancel()` and the trailing idle). Both fail without the change and pass with it. Full suite (2127 tests), `npm run check` and `build` pass. One unrelated timeout in `acp-scenarios` (`schema check`) showed up under load in one full run and passes when run on its own. ## Live check Against the built adapter (real credentials), I asked for `sleep 45`, cancelled once the `tool_call` appeared, and listed every tool call and its statuses at the end. Before and after the change the result is the same: one tool call, `pending > ... > failed`, nothing open. So in the plain cancel-mid-tool case the SDK already delivers an interrupted result and there is no ghost to remove; I could not reproduce the reporter's leak live, only in the unit fixture above. What the live run does show is that the new branch does not send a duplicate terminal for a tool that was already terminated. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_019vH6BXrsYQN3MqQ93JgStf </details> --------- Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com> Co-authored-by: Ben Brandt <benjamin.j.brandt@gmail.com>
This commit is contained in:
Demetrios Chiuratto Agourakis
committed by
GitHub
GitHub
parent
c6510c27c1
commit
32c23db06d
@@ -683,6 +683,11 @@ type Turn = {
|
||||
* background handoff. Intersect with emittedToolCalls at settlement so
|
||||
* tool_result counts even when a presentation hook is still pending. */
|
||||
foregroundToolCallIds?: Set<string>;
|
||||
/** Closes the tool calls this turn left open when a cancel ended it. Both
|
||||
* the consumer and `cancelTurns` can settle a cancelled turn; they share
|
||||
* this one closing, so each call gets one report and both wait for all of
|
||||
* them before reporting the cancel. */
|
||||
closingCancelledToolCalls?: Promise<void>;
|
||||
/** Local-only slash commands (e.g. `/clear`) return a result without an echo,
|
||||
* so the consumer can't promote them via the replay; it falls back to
|
||||
* promoting the queue head when the result arrives. */
|
||||
@@ -1263,6 +1268,9 @@ export type Session = {
|
||||
nativeSubagentRuntime?: NativeSubagentRuntime;
|
||||
/** Child-aware delivery closure paired with {@link nativeSubagentRuntime}. */
|
||||
nativeSubagentDeliver?: (notification: AcpSessionNotification) => Promise<void>;
|
||||
/** The consumer's `closeCancelledToolCalls`, for `cancelTurns`, which settles
|
||||
* a held or settling turn itself. */
|
||||
closeCancelledToolCalls?: (turn: Turn) => Promise<void>;
|
||||
/** Session-owned async task controller. Prompt cancellation intentionally
|
||||
* does not finish it because background work may outlive a prompt. */
|
||||
asyncTaskRuntime?: AsyncTaskRuntime;
|
||||
@@ -4889,25 +4897,26 @@ export class ClaudeAcpAgent {
|
||||
* 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`).
|
||||
* Returns false when the turn ended meanwhile. */
|
||||
const closeCancelledToolCalls = async (turn: Turn): Promise<boolean> => {
|
||||
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);
|
||||
if (turn.settled || session.activeTurn !== turn) return false;
|
||||
}
|
||||
return true;
|
||||
};
|
||||
* Runs once per turn: a second call, from the other settling path, waits
|
||||
* for the first (see `Turn.closingCancelledToolCalls`). */
|
||||
const closeCancelledToolCalls = (turn: Turn): Promise<void> =>
|
||||
(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);
|
||||
}
|
||||
})());
|
||||
session.closeCancelledToolCalls = closeCancelledToolCalls;
|
||||
|
||||
/** 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
|
||||
@@ -5025,7 +5034,9 @@ export class ClaudeAcpAgent {
|
||||
// response on v1, `idle` on v2) only after every tool call that work
|
||||
// opened has ended.
|
||||
if (result.stopReason === "cancelled" && session.cancelled) {
|
||||
if (!(await closeCancelledToolCalls(turn))) return;
|
||||
await closeCancelledToolCalls(turn);
|
||||
// `cancelTurns` can settle the turn while the reports are in flight.
|
||||
if (turn.settled || session.activeTurn !== turn) return;
|
||||
}
|
||||
turn.settling = true;
|
||||
turn.settlingOutcome = result;
|
||||
@@ -8037,7 +8048,8 @@ export class ClaudeAcpAgent {
|
||||
// permission request) is left to that cycle's interrupted result, like any
|
||||
// live turn, or to the idle when no result comes. The captured outcome
|
||||
// preserves usage and metadata (issue #844), while reporter state makes a
|
||||
// late checkpoint response harmless.
|
||||
// late checkpoint response harmless. The turn's open tool calls close
|
||||
// before its response, as in the consumer's settle.
|
||||
// Since CLI 2.1.270, Claude Code reports `running` for as long as
|
||||
// background agents live, so the session state cannot tell a live
|
||||
// followup from a hold. With request markers, a followup is live when one
|
||||
@@ -8052,7 +8064,10 @@ export class ClaudeAcpAgent {
|
||||
(active.settling || (isHeldOpen(active) && !followupLive)) &&
|
||||
(active.deferredSettle ?? active.settlingOutcome)
|
||||
) {
|
||||
const closingToolCalls = session.closeCancelledToolCalls?.(active);
|
||||
session.fileChangeReporter?.finish(active.fileChangeReport, "cancelled");
|
||||
// Settled before the reports are awaited, so that the consumer, which
|
||||
// keeps running meanwhile, cannot settle the turn a second time.
|
||||
active.settled = true;
|
||||
// Mirror settleActive's invariants (it is consumer-scoped and
|
||||
// unreachable from here): disarm the backstop — none should be
|
||||
@@ -8084,7 +8099,16 @@ export class ClaudeAcpAgent {
|
||||
if (session.lastSessionState !== "idle") {
|
||||
session.owedTrailingIdles++;
|
||||
}
|
||||
active.resolve(cancelledOutcome(session, active));
|
||||
try {
|
||||
await closingToolCalls;
|
||||
} catch (error) {
|
||||
this.logger.error(
|
||||
`Session ${params.sessionId}: failed to publish cancelled tool state`,
|
||||
error,
|
||||
);
|
||||
} finally {
|
||||
active.resolve(cancelledOutcome(session, active));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -9,6 +9,18 @@ import {
|
||||
wrapQuery,
|
||||
} from "./session-doubles.js";
|
||||
|
||||
// The agent reads the CLI's auth status on each prompt. Answer it, so that the
|
||||
// tests checking `logError` don't see "claude auth status failed". The answer
|
||||
// makes the agent send an auth status `extNotification`; the client stubs it.
|
||||
vi.mock("node:child_process", async () => ({
|
||||
...(await vi.importActual<typeof import("node:child_process")>("node:child_process")),
|
||||
execFile: (...args: unknown[]) =>
|
||||
(args[args.length - 1] as (...values: unknown[]) => void)(null, {
|
||||
stdout: JSON.stringify({ loggedIn: false, authMethod: "none", apiProvider: "firstParty" }),
|
||||
stderr: "",
|
||||
}),
|
||||
}));
|
||||
|
||||
const sessionId = "test-session";
|
||||
const toolCallId = "unfinished-bash";
|
||||
|
||||
@@ -137,6 +149,340 @@ describe("incomplete foreground tools", () => {
|
||||
expect(failedTool()).toHaveLength(1);
|
||||
});
|
||||
|
||||
it.each(["deferred", "checkpoint"] as const)(
|
||||
"terminates unfinished tools when cancel settles a %s turn inline",
|
||||
async (settlement) => {
|
||||
let releaseStream!: () => void;
|
||||
const streamGate = new Promise<void>((resolve) => (releaseStream = resolve));
|
||||
const { agent, prompt, updates, logError } = createTestSession(async function* (input) {
|
||||
yield* echoNextPrompt(input);
|
||||
if (settlement === "deferred") {
|
||||
yield toolStart();
|
||||
yield toolUse();
|
||||
yield toolStart("second-tool");
|
||||
yield toolUse("second-tool");
|
||||
yield toolStart("completed-tool");
|
||||
yield toolResult("completed-tool");
|
||||
// A background subagent holds the turn open, and its command runs on.
|
||||
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");
|
||||
yield toolStart("bg-child", "bg-agent");
|
||||
yield toolUse("bg-child", "bg-agent");
|
||||
}
|
||||
yield successfulResultMessage();
|
||||
await streamGate;
|
||||
yield { type: "system", subtype: "session_state_changed", state: "idle" };
|
||||
yield* echoNextPrompt(input);
|
||||
yield successfulResultMessage();
|
||||
});
|
||||
if (settlement === "checkpoint") {
|
||||
agent.sessions[sessionId].fileChangeReporter = {
|
||||
request: vi.fn(),
|
||||
report: vi.fn(() => streamGate),
|
||||
finish: vi.fn(),
|
||||
} as any;
|
||||
}
|
||||
|
||||
try {
|
||||
const first = prompt();
|
||||
await vi.waitFor(() => {
|
||||
const active = agent.sessions[sessionId].activeTurn;
|
||||
expect(
|
||||
settlement === "deferred" ? active?.deferredSettle : active?.settlingOutcome,
|
||||
).toBeDefined();
|
||||
});
|
||||
if (settlement === "checkpoint") {
|
||||
for (const id of [toolCallId, "second-tool"]) {
|
||||
await agent.canUseTool(sessionId)("Bash", { command: "echo test" }, {
|
||||
toolUseID: id,
|
||||
signal: new AbortController().signal,
|
||||
suggestions: [],
|
||||
} as any);
|
||||
}
|
||||
}
|
||||
const active = agent.sessions[sessionId].activeTurn!;
|
||||
const pendingOutcome = active.deferredSettle ?? active.settlingOutcome!;
|
||||
await agent.cancel({ sessionId });
|
||||
await expect(first).resolves.toEqual({
|
||||
...pendingOutcome,
|
||||
stopReason: "cancelled",
|
||||
});
|
||||
for (const id of [toolCallId, "second-tool"]) {
|
||||
const terminals = updates.filter((u) => u.toolCallId === id && u.status === "failed");
|
||||
expect(terminals).toHaveLength(1);
|
||||
expect(terminals[0].content[0].content.text).toContain("cancelled");
|
||||
expect(agent.sessions[sessionId].emittedToolCalls.has(id)).toBe(false);
|
||||
expect(agent.sessions[sessionId].toolUseCache).not.toHaveProperty(id);
|
||||
expect(agent.sessions[sessionId].dispatchedToolCalls?.has(id)).not.toBe(true);
|
||||
expect(hasHookCallback(id)).toBe(false);
|
||||
}
|
||||
expect(
|
||||
updates.some((u) => u.toolCallId === "completed-tool" && u.status === "failed"),
|
||||
).toBe(false);
|
||||
if (settlement === "deferred") {
|
||||
// The cancel did not stop the background subagent, so its command stays open.
|
||||
expect(updates.some((u) => u.toolCallId === "bg-child" && u.status === "failed")).toBe(
|
||||
false,
|
||||
);
|
||||
expect(agent.sessions[sessionId].emittedToolCalls.has("bg-child")).toBe(true);
|
||||
}
|
||||
await agent.cancel({ sessionId });
|
||||
releaseStream();
|
||||
await expect(prompt()).resolves.toMatchObject({ stopReason: "end_turn" });
|
||||
expect(updates.filter((u) => u.status === "failed")).toHaveLength(2);
|
||||
expect(logError).not.toHaveBeenCalled();
|
||||
await agent.sessions[sessionId].consumer;
|
||||
} finally {
|
||||
releaseStream();
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.each(["overlapping", "simultaneous"] as const)(
|
||||
"does not duplicate tool terminals with %s cancellations",
|
||||
async (ordering) => {
|
||||
let releaseStream!: () => void;
|
||||
const streamGate = new Promise<void>((resolve) => (releaseStream = resolve));
|
||||
let releasePublication!: () => void;
|
||||
const publicationGate = new Promise<void>((resolve) => (releasePublication = resolve));
|
||||
let firstPublished!: () => void;
|
||||
const publicationStarted = new Promise<void>((resolve) => (firstPublished = resolve));
|
||||
const { agent, prompt, updates } = createTestSession(async function* (input) {
|
||||
yield* echoNextPrompt(input);
|
||||
yield toolStart();
|
||||
yield toolStart("second-tool");
|
||||
yield {
|
||||
type: "system",
|
||||
subtype: "task_started",
|
||||
session_id: sessionId,
|
||||
task_id: "child",
|
||||
tool_use_id: "parent-agent",
|
||||
subagent_type: "Explore",
|
||||
description: "investigate",
|
||||
};
|
||||
yield successfulResultMessage();
|
||||
await streamGate;
|
||||
});
|
||||
const publish = agent.client.sessionUpdate.bind(agent.client);
|
||||
agent.client.sessionUpdate = async (notification) => {
|
||||
await publish(notification);
|
||||
const update = notification.update;
|
||||
if (
|
||||
update.sessionUpdate === "tool_call_update" &&
|
||||
update.toolCallId === toolCallId &&
|
||||
update.status === "failed"
|
||||
) {
|
||||
firstPublished();
|
||||
await publicationGate;
|
||||
}
|
||||
};
|
||||
|
||||
try {
|
||||
const first = prompt();
|
||||
await vi.waitFor(() => {
|
||||
expect(agent.sessions[sessionId].activeTurn?.deferredSettle).toBeDefined();
|
||||
});
|
||||
if (ordering === "simultaneous") {
|
||||
const cancel = Promise.all([agent.cancel({ sessionId }), agent.cancel({ sessionId })]);
|
||||
await publicationStarted;
|
||||
releasePublication();
|
||||
await cancel;
|
||||
} else {
|
||||
const cancel = agent.cancel({ sessionId });
|
||||
await publicationStarted;
|
||||
await agent.cancel({ sessionId });
|
||||
releasePublication();
|
||||
await cancel;
|
||||
}
|
||||
await expect(first).resolves.toMatchObject({ stopReason: "cancelled" });
|
||||
for (const id of [toolCallId, "second-tool"]) {
|
||||
expect(updates.filter((u) => u.toolCallId === id && u.status === "failed")).toHaveLength(
|
||||
1,
|
||||
);
|
||||
}
|
||||
expect(agent.sessions[sessionId].emittedToolCalls.size).toBe(0);
|
||||
releaseStream();
|
||||
await agent.sessions[sessionId].consumer;
|
||||
} finally {
|
||||
releasePublication();
|
||||
releaseStream();
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("still interrupts the SDK when inline tool terminal publication fails", async () => {
|
||||
let releaseCheckpoint!: () => void;
|
||||
const checkpointGate = new Promise<void>((resolve) => (releaseCheckpoint = resolve));
|
||||
const { agent, prompt, logError } = createTestSession(async function* (input) {
|
||||
yield* echoNextPrompt(input);
|
||||
yield successfulResultMessage();
|
||||
});
|
||||
agent.sessions[sessionId].fileChangeReporter = {
|
||||
request: vi.fn(),
|
||||
report: vi.fn(() => checkpointGate),
|
||||
finish: vi.fn(),
|
||||
} as any;
|
||||
const publish = agent.client.sessionUpdate.bind(agent.client);
|
||||
const error = new Error("terminal publication failed");
|
||||
agent.client.sessionUpdate = async (notification) => {
|
||||
const update = notification.update;
|
||||
if (update.sessionUpdate === "tool_call_update" && update.status === "failed") {
|
||||
throw error;
|
||||
}
|
||||
await publish(notification);
|
||||
};
|
||||
try {
|
||||
const first = prompt();
|
||||
await vi.waitFor(() => {
|
||||
expect(agent.sessions[sessionId].activeTurn?.settlingOutcome).toBeDefined();
|
||||
});
|
||||
await agent.canUseTool(sessionId)("Bash", { command: "echo test" }, {
|
||||
toolUseID: toolCallId,
|
||||
signal: new AbortController().signal,
|
||||
suggestions: [],
|
||||
} as any);
|
||||
await expect(agent.cancel({ sessionId })).resolves.toBeUndefined();
|
||||
await expect(first).resolves.toMatchObject({ stopReason: "cancelled" });
|
||||
expect(agent.sessions[sessionId].query.interrupt).toHaveBeenCalledTimes(1);
|
||||
expect(logError).toHaveBeenCalledExactlyOnceWith(
|
||||
`Session ${sessionId}: failed to publish cancelled tool state`,
|
||||
error,
|
||||
);
|
||||
releaseCheckpoint();
|
||||
await agent.sessions[sessionId].consumer;
|
||||
} finally {
|
||||
releaseCheckpoint();
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps cancellation authoritative when a checkpoint finishes during tool cleanup", async () => {
|
||||
let releaseCheckpoint!: () => void;
|
||||
const checkpointGate = new Promise<void>((resolve) => (releaseCheckpoint = resolve));
|
||||
let checkpointDrained!: () => void;
|
||||
const consumerAdvanced = new Promise<void>((resolve) => (checkpointDrained = resolve));
|
||||
let releaseStream!: () => void;
|
||||
const streamGate = new Promise<void>((resolve) => (releaseStream = resolve));
|
||||
let releasePublication!: () => void;
|
||||
const publicationGate = new Promise<void>((resolve) => (releasePublication = resolve));
|
||||
let firstPublished!: () => void;
|
||||
const publicationStarted = new Promise<void>((resolve) => (firstPublished = resolve));
|
||||
const { agent, prompt } = createTestSession(async function* (input) {
|
||||
yield* echoNextPrompt(input);
|
||||
yield successfulResultMessage();
|
||||
checkpointDrained();
|
||||
await streamGate;
|
||||
});
|
||||
agent.sessions[sessionId].fileChangeReporter = {
|
||||
request: vi.fn(),
|
||||
report: vi.fn(() => checkpointGate),
|
||||
finish: vi.fn(),
|
||||
} as any;
|
||||
const publish = agent.client.sessionUpdate.bind(agent.client);
|
||||
agent.client.sessionUpdate = async (notification) => {
|
||||
await publish(notification);
|
||||
const update = notification.update;
|
||||
if (
|
||||
update.sessionUpdate === "tool_call_update" &&
|
||||
update.toolCallId === toolCallId &&
|
||||
update.status === "failed"
|
||||
) {
|
||||
firstPublished();
|
||||
await publicationGate;
|
||||
}
|
||||
};
|
||||
|
||||
try {
|
||||
let responded = false;
|
||||
const first = prompt().then((response) => {
|
||||
responded = true;
|
||||
return response;
|
||||
});
|
||||
await vi.waitFor(() => {
|
||||
expect(agent.sessions[sessionId].activeTurn?.settlingOutcome).toBeDefined();
|
||||
});
|
||||
await agent.canUseTool(sessionId)("Bash", { command: "echo test" }, {
|
||||
toolUseID: toolCallId,
|
||||
signal: new AbortController().signal,
|
||||
suggestions: [],
|
||||
} as any);
|
||||
const cancel = agent.cancel({ sessionId });
|
||||
await publicationStarted;
|
||||
releaseCheckpoint();
|
||||
await consumerAdvanced;
|
||||
expect(responded).toBe(false);
|
||||
releasePublication();
|
||||
await cancel;
|
||||
await expect(first).resolves.toMatchObject({ stopReason: "cancelled" });
|
||||
releaseStream();
|
||||
await agent.sessions[sessionId].consumer;
|
||||
} finally {
|
||||
releaseCheckpoint();
|
||||
releasePublication();
|
||||
releaseStream();
|
||||
}
|
||||
});
|
||||
|
||||
it("closes each tool call once, before the response, when a cancel settles a held turn that the consumer is closing", async () => {
|
||||
let releasePublication!: () => void;
|
||||
const publicationGate = new Promise<void>((resolve) => (releasePublication = resolve));
|
||||
let firstHeld!: () => void;
|
||||
const publicationHeld = new Promise<void>((resolve) => (firstHeld = resolve));
|
||||
const { agent, prompt, updates, logError } = createTestSession(async function* (input, agent) {
|
||||
yield* echoNextPrompt(input);
|
||||
// A background subagent holds the turn open after its result.
|
||||
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");
|
||||
yield successfulResultMessage();
|
||||
// The first cancel lands while a follow-up cycle runs two commands, so it
|
||||
// leaves the turn to the consumer, which settles it at the idle.
|
||||
yield { type: "system", subtype: "session_state_changed", state: "running" };
|
||||
yield toolStart();
|
||||
yield toolUse();
|
||||
yield toolStart("second-tool");
|
||||
yield toolUse("second-tool");
|
||||
await agent.cancel({ sessionId });
|
||||
yield { type: "system", subtype: "session_state_changed", state: "idle" };
|
||||
});
|
||||
const events: string[] = [];
|
||||
const publish = agent.client.sessionUpdate.bind(agent.client);
|
||||
agent.client.sessionUpdate = async (notification) => {
|
||||
const update = notification.update;
|
||||
if (update.sessionUpdate === "tool_call_update" && update.status === "failed") {
|
||||
if (update.toolCallId === toolCallId) {
|
||||
firstHeld();
|
||||
await publicationGate;
|
||||
}
|
||||
events.push(`end ${update.toolCallId}`);
|
||||
}
|
||||
await publish(notification);
|
||||
};
|
||||
|
||||
try {
|
||||
const response = prompt().then((outcome) => {
|
||||
events.push("response");
|
||||
return outcome;
|
||||
});
|
||||
await publicationHeld;
|
||||
// The session now reads idle, so the second cancel settles the held turn
|
||||
// itself, while the consumer's first report is in flight.
|
||||
const cancel = agent.cancel({ sessionId });
|
||||
await vi.waitFor(() => expect(agent.sessions[sessionId].activeTurn).toBeNull());
|
||||
releasePublication();
|
||||
await cancel;
|
||||
await expect(response).resolves.toMatchObject({ stopReason: "cancelled" });
|
||||
expect(events).toEqual([`end ${toolCallId}`, "end second-tool", "response"]);
|
||||
expect(logError).not.toHaveBeenCalled();
|
||||
await agent.sessions[sessionId].consumer;
|
||||
expect(updates.filter((u) => u.status === "failed")).toHaveLength(2);
|
||||
} finally {
|
||||
releasePublication();
|
||||
}
|
||||
});
|
||||
|
||||
it("closes a streamed tool_use that never reached a complete message without a failure", async () => {
|
||||
const { prompt, updates, logError, agent } = createTestSession(abandonedToolMessages);
|
||||
|
||||
@@ -409,6 +755,7 @@ function createTestSession(
|
||||
updates.push(notification.update);
|
||||
},
|
||||
requestPermission: async () => ({ outcome: { outcome: "selected", optionId: "allow-once" } }),
|
||||
extNotification: async () => {},
|
||||
} as unknown as AcpClient,
|
||||
{ log: () => {}, error: logError },
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user