mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 04:30:59 +00:00
fix: report a command whose result says interrupted as cancelled on the experimental ACP v2 surface (#1301)
#1289 made a v2 client get `cancelled` for a run that Claude Code aborted, when Claude Code reports it as an error result with `non_execution_kind`. Claude Code has a second way to report an aborted command: a **successful** Bash result whose structured output says `interrupted: true`. A v2 client got `completed` for that. With this PR it gets `cancelled` too, so the rule is the same however Claude Code reports the abort. ## When Claude Code sends that result I read the CLI that SDK 0.3.293 bundles. I didn't reproduce these cases live. | What stops the command | What the adapter receives | v2 before | v2 after | | --- | --- | --- | --- | | The client cancels (`session/cancel`) | an error result, `non_execution_kind: "interrupted"` | `cancelled` (#1289) | same | | A timeout | an error, "Command timed out after …", **not** `interrupted` (or the command is backgrounded) | `failed` | same | | The output limit or a replaced output file | an error with `Exit code 137`, not `interrupted` | `failed` | same | | A `now`-priority message such as `_session/steering`, landing just before the command starts, or while the output limit kills it | a successful result, `interrupted: true` | `completed` | **`cancelled`** | A successful `interrupted` result needs the abort reason `interrupt`, which a `now`-priority message causes. That abort doesn't kill a running command, so this result only comes from the races in the last row. Clients will rarely see it. The point of the change is that v2's status follows what Claude Code reports, however it reports it. v1 and AIR clients are unchanged: `completed` with no exit code, and AIR still gets exit code 1. ## How - `AcpToolCallRenderer.resultStatus` takes the command's `interrupted` fact alongside `nonExecution`. - `result()` used to call `resultFields`, which drops the command facts. It now computes the facts once (`resultFacts`) and builds the fields from them (`fieldsOfResult`). That lets it pass `interrupted` without adding a field to `RenderedResult`, which the public `toolUpdateFromToolResult` returns. - The agent's checks for a finished tool call already count `cancelled` (`isFinalToolCallStatus`, #1289). ## Checks - `npm run check`, `npm run build` and all 2730 tests pass. No golden file changes: no recorded scenario has an interrupted command. - New tests: - **renderer:** an interrupted command on v2, v1 and AIR, and one that isn't interrupted on v2; - **v2 surface:** a v2 client receives `cancelled`, and the turn still ends `end_turn`. - I made four mutations and each failed a test: the rule ignores `interrupted`, every client gets `cancelled`, `result()` doesn't pass the fact, and `result()` inverts it.
This commit is contained in:
@@ -1279,6 +1279,69 @@ describe("ACP v2 prompts", () => {
|
||||
expect(statuses.at(-1)).toBe("cancelled");
|
||||
});
|
||||
|
||||
it("reports a command whose result says interrupted as cancelled", async () => {
|
||||
scriptTurns(async function* (options) {
|
||||
yield {
|
||||
type: "assistant",
|
||||
message: {
|
||||
id: "msg_bash",
|
||||
type: "message",
|
||||
role: "assistant",
|
||||
model: "claude-sonnet-4-6",
|
||||
content: [
|
||||
{ type: "tool_use", id: "toolu_bash", name: "Bash", input: { command: "make" } },
|
||||
],
|
||||
stop_reason: "tool_use",
|
||||
stop_sequence: null,
|
||||
usage: { input_tokens: 1, output_tokens: 1 },
|
||||
},
|
||||
parent_tool_use_id: null,
|
||||
uuid: randomUUID(),
|
||||
session_id: options.sessionId,
|
||||
};
|
||||
// Claude Code aborted the command for a steering message: a successful
|
||||
// result whose structured output says `interrupted`.
|
||||
yield {
|
||||
type: "user",
|
||||
message: {
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "tool_result",
|
||||
tool_use_id: "toolu_bash",
|
||||
content:
|
||||
"Command aborted before execution<error>Command was aborted before completion</error>",
|
||||
},
|
||||
],
|
||||
},
|
||||
tool_use_result: {
|
||||
stdout: "",
|
||||
stderr: "Command aborted before execution",
|
||||
interrupted: true,
|
||||
isImage: false,
|
||||
},
|
||||
parent_tool_use_id: null,
|
||||
uuid: randomUUID(),
|
||||
session_id: options.sessionId,
|
||||
};
|
||||
yield result(options);
|
||||
});
|
||||
const client = v2Client();
|
||||
await client.app.connectWith(connectRouter(), async (agent) => {
|
||||
await initializeV2(agent);
|
||||
const { sessionId } = await agent.request(v2.methods.agent.session.new, { cwd });
|
||||
await agent.request(v2.methods.agent.session.prompt, { sessionId, prompt: text("hi") });
|
||||
await vi.waitFor(() => expect(turnTrace(client.sessionUpdates)).toContain("idle end_turn"));
|
||||
await client.authUpdate(1);
|
||||
});
|
||||
|
||||
const statuses = client
|
||||
.updates("tool_call_update")
|
||||
.filter((update) => update.toolCallId === "toolu_bash" && update.status)
|
||||
.map((update) => update.status);
|
||||
expect(statuses.at(-1)).toBe("cancelled");
|
||||
});
|
||||
|
||||
it("reports a tool call that Claude dropped before it ran as cancelled", async () => {
|
||||
scriptTurns(async function* (options) {
|
||||
// Claude starts a tool_use, but no complete assistant message holds it,
|
||||
|
||||
@@ -3820,6 +3820,31 @@ describe("tool_result_meta non-execution stamping", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("reports an interrupted command as cancelled to a v2 client, and as completed to every other client", () => {
|
||||
const status = (interrupted: boolean, toolCallCapabilities?: ToolCallCapabilities) =>
|
||||
toAcpNotifications(
|
||||
[{ type: "tool_result", tool_use_id: "toolu_bash", content: "partial" }] as any,
|
||||
"user",
|
||||
"test-session",
|
||||
{ toolu_bash: bashToolUse },
|
||||
mockClient,
|
||||
mockLogger,
|
||||
{
|
||||
toolUseResult: { stdout: "partial", stderr: "", interrupted, isImage: false },
|
||||
...(toolCallCapabilities ? { toolCallCapabilities } : {}),
|
||||
},
|
||||
).at(-1)?.update;
|
||||
const v2 = ToolCallCapabilities.from({}, { v2: true });
|
||||
|
||||
expect(status(true, v2)).toMatchObject({ status: "cancelled" });
|
||||
expect(status(false, v2)).toMatchObject({ status: "completed" });
|
||||
// v1 has no cancelled status.
|
||||
expect(status(true)).toMatchObject({ status: "completed" });
|
||||
expect(status(true, ToolCallCapabilities.from(AIR_CLIENT))).toMatchObject({
|
||||
status: "completed",
|
||||
});
|
||||
});
|
||||
|
||||
it("closes a tool call that Claude dropped before it ran as cancelled on v2, and as failed elsewhere", () => {
|
||||
const unfinished = (ran: boolean, capabilities?: ToolCallCapabilities) =>
|
||||
new AcpToolCallRenderer(capabilities).unfinished({ toolCallId: "toolu_bash", ran });
|
||||
|
||||
@@ -151,14 +151,22 @@ export class AcpToolCallRenderer {
|
||||
}
|
||||
|
||||
/**
|
||||
* The status of a tool result. `nonExecution` is why Claude Code stamped an
|
||||
* error result instead of the tool's output. A run that was aborted is
|
||||
* `cancelled` for a v2 client, which has that status; every other client
|
||||
* gets `failed`, with the kind in `_meta.claudeCode.nonExecutionKind`.
|
||||
* The status of a tool result. Claude Code reports a run that it aborted in
|
||||
* one of two ways: an error result whose `nonExecution` kind says so (a
|
||||
* cancelled turn), or a command result that says `interrupted` (for example,
|
||||
* a steering message landed just before the command started). Such a run is
|
||||
* `cancelled` for a v2 client, which has that status. Every other client gets
|
||||
* `failed` for the error, with the kind in `_meta.claudeCode.nonExecutionKind`,
|
||||
* and `completed` for the command.
|
||||
*/
|
||||
resultStatus(isError: boolean, nonExecution: ToolResultMeta | undefined): ToolCallStatus {
|
||||
if (!isError) return "completed";
|
||||
return this.capabilities.v2 && isAbortedToolResult(nonExecution) ? V2_CANCELLED : "failed";
|
||||
resultStatus(
|
||||
isError: boolean,
|
||||
nonExecution: ToolResultMeta | undefined,
|
||||
interrupted = false,
|
||||
): ToolCallStatus {
|
||||
const aborted = interrupted || (isError && isAbortedToolResult(nonExecution));
|
||||
if (this.capabilities.v2 && aborted) return V2_CANCELLED;
|
||||
return isError ? "failed" : "completed";
|
||||
}
|
||||
|
||||
/** The facts of a tool use. */
|
||||
@@ -332,7 +340,14 @@ export class AcpToolCallRenderer {
|
||||
|
||||
/** The result fields of a tool result, before the status and the tool name. */
|
||||
resultFields(toolUse: ToolUse, result: ResultBlock, structured?: unknown): RenderedResult {
|
||||
const facts = this.resultFacts(toolUse, result, structured);
|
||||
return this.fieldsOfResult(toolUse, result, this.resultFacts(toolUse, result, structured));
|
||||
}
|
||||
|
||||
private fieldsOfResult(
|
||||
toolUse: ToolUse,
|
||||
result: ResultBlock,
|
||||
facts: ToolResultFacts,
|
||||
): RenderedResult {
|
||||
const { command, ...fields } = facts;
|
||||
if (!command) return fields;
|
||||
// The terminal was announced under the tool_use id (see `toolCall`), so the
|
||||
@@ -397,7 +412,8 @@ export class AcpToolCallRenderer {
|
||||
result: ResultBlock,
|
||||
options: { structured?: unknown; nonExecution?: ToolResultMeta } = {},
|
||||
): ToolCallUpdate[] {
|
||||
const { _meta: resultMeta, ...fields } = this.resultFields(toolUse, result, options.structured);
|
||||
const facts = this.resultFacts(toolUse, result, options.structured);
|
||||
const { _meta: resultMeta, ...fields } = this.fieldsOfResult(toolUse, result, facts);
|
||||
// AIR shows a read or a search that names a path as the list of viewed
|
||||
// files. That view does not show the text of the result, so it stays out.
|
||||
const viewedFiles =
|
||||
@@ -445,7 +461,11 @@ export class AcpToolCallRenderer {
|
||||
} satisfies ToolUpdateMeta,
|
||||
toolCallId: toolUse.id,
|
||||
sessionUpdate: "tool_call_update",
|
||||
status: this.resultStatus(result.is_error === true, options.nonExecution),
|
||||
status: this.resultStatus(
|
||||
result.is_error === true,
|
||||
options.nonExecution,
|
||||
facts.command?.interrupted === true,
|
||||
),
|
||||
...(rawOutput !== undefined ? { rawOutput } : {}),
|
||||
...(planFilePath ? { rawInput: planFileInput(toolUse.input, planFilePath) } : {}),
|
||||
...fields,
|
||||
|
||||
Reference in New Issue
Block a user