diff --git a/plugins/provider-acp/src/delta-translation.test.ts b/plugins/provider-acp/src/delta-translation.test.ts index 6e4cb91995..2e5abd36a0 100644 --- a/plugins/provider-acp/src/delta-translation.test.ts +++ b/plugins/provider-acp/src/delta-translation.test.ts @@ -1185,6 +1185,95 @@ describe("acp delta translation (native kinds → core kinds)", () => { }); }); + // The live data-loss case: one unseen `kind` used to fail the wire parse, + // so the tool_call never opened and its `completed` update closed a bare + // untitled generic row. The call now opens under the agent's own kind word + // and closes on the same item. + it("keeps a call whose kind the schema does not know", () => { + const harness = createHarness(); + harness.translate(turnStartedEvent()); + const opened = harness.translate( + updateEvent({ + sessionUpdate: "tool_call", + toolCallId: "call-deploy", + title: "Deploy preview", + kind: "deploy", + status: "in_progress", + }), + ); + expect(opened).toHaveLength(1); + expect(opened[0]).toMatchObject({ + type: "item/started", + item: { + type: "toolCall", + tool: "deploy", + status: "pending", + presentation: { + label: { pending: "Running tool", completed: "Ran tool" }, + icon: { glyph: "Toolbox" }, + title: "Deploy preview", + }, + }, + }); + const openedId = + opened[0]?.type === "item/started" ? opened[0].item.id : ""; + + const closed = harness.translate( + updateEvent({ + sessionUpdate: "tool_call_update", + toolCallId: "call-deploy", + status: "completed", + rawOutput: { url: "https://preview.example" }, + }), + ); + expect(closed).toHaveLength(1); + expect(closed[0]).toMatchObject({ + type: "item/completed", + item: { + type: "toolCall", + id: openedId, + tool: "deploy", + status: "completed", + presentation: { title: "Deploy preview" }, + }, + }); + }); + + it("settles a cancelled call as interrupted and a switch_mode call as its own kind", () => { + const harness = createHarness(); + harness.translate(turnStartedEvent()); + harness.translate( + updateEvent({ + sessionUpdate: "tool_call", + toolCallId: "call-mode", + title: "Switch to plan mode", + kind: "switch_mode", + status: "pending", + }), + ); + const closed = harness.translate( + updateEvent({ + sessionUpdate: "tool_call_update", + toolCallId: "call-mode", + status: "cancelled", + }), + ); + expect(closed).toHaveLength(1); + expect(closed[0]).toMatchObject({ + type: "item/completed", + item: { + type: "toolCall", + tool: "switch_mode", + status: "interrupted", + presentation: { + label: { pending: "Switching mode", completed: "Switched mode" }, + icon: { glyph: "SlidersHorizontal" }, + title: "Switch to plan mode", + }, + }, + }); + }); + it("names a generic call by its kind and keeps the title as the headline", () => { expect( openItem({ toolCallId: "other-1", title: "MCP: tool", kind: "other" }), diff --git a/plugins/provider-acp/src/delta-translation.ts b/plugins/provider-acp/src/delta-translation.ts index 050d7f0b06..7ca8eb0f7e 100644 --- a/plugins/provider-acp/src/delta-translation.ts +++ b/plugins/provider-acp/src/delta-translation.ts @@ -96,7 +96,9 @@ const PLAN_STEPS_CHANNEL = "planSteps"; function isTerminalAcpStatus( status: AcpToolCallUpdateEvent["status"], ): boolean { - return status === "completed" || status === "failed"; + return ( + status === "completed" || status === "failed" || status === "cancelled" + ); } function mapAcpToolCallStatus( @@ -107,6 +109,8 @@ function mapAcpToolCallStatus( return "completed"; case "failed": return "failed"; + case "cancelled": + return "interrupted"; default: return "pending"; } @@ -124,10 +128,23 @@ function mergeAcpToolCallEvents( if (!started) { return update; } + // A kind on the update replaces the started kind together with its raw + // form: a known kind clears a stale `rawKind`, an unknown one carries its + // own. + const { rawKind: startedRawKind, ...startedRest } = started; + const kindFields = + update.kind !== undefined + ? { + kind: update.kind, + ...(update.rawKind !== undefined ? { rawKind: update.rawKind } : {}), + } + : startedRawKind !== undefined + ? { rawKind: startedRawKind } + : {}; return { - ...started, + ...startedRest, + ...kindFields, ...(update.title !== undefined ? { title: update.title } : {}), - ...(update.kind !== undefined ? { kind: update.kind } : {}), ...(update.status !== undefined ? { status: update.status } : {}), ...(update.content !== undefined ? { content: update.content } : {}), ...(update.locations !== undefined ? { locations: update.locations } : {}), diff --git a/plugins/provider-acp/src/presentation.ts b/plugins/provider-acp/src/presentation.ts index 7d36e69756..a0d5a6d2c9 100644 --- a/plugins/provider-acp/src/presentation.ts +++ b/plugins/provider-acp/src/presentation.ts @@ -228,6 +228,10 @@ const KIND_PRESENTATIONS: Readonly> = label: { pending: "Fetching", completed: "Fetched" }, glyph: "Globe", }, + switch_mode: { + label: { pending: "Switching mode", completed: "Switched mode" }, + glyph: "SlidersHorizontal", + }, other: { label: { pending: "Running tool", completed: "Ran tool" }, glyph: "Toolbox", diff --git a/plugins/provider-acp/src/tool-classification.ts b/plugins/provider-acp/src/tool-classification.ts index 2d3eff25fd..c6d42f49ef 100644 --- a/plugins/provider-acp/src/tool-classification.ts +++ b/plugins/provider-acp/src/tool-classification.ts @@ -47,7 +47,6 @@ import { import { extractAcpContentText, type AcpToolCallUpdateEvent, - type AcpToolKind, } from "./wire.js"; /** A tool call's item shape plus the presentation that rides its lifecycle. */ @@ -315,13 +314,18 @@ function reasoningItem(event: AcpToolCallUpdateEvent): AcpClassifiedToolCall { }; } +/** + * A generic tool names itself by its kind; a kind the wire schema did not + * know keeps the agent's own word (`rawKind`) in the tool slot and presents + * as `other`. + */ function genericToolItem( - kind: AcpToolKind | undefined, + event: Pick, title: string | undefined, ): AcpClassifiedToolCall { return { - item: { type: "tool", tool: kind ?? "tool" }, - presentation: toolKindPresentation({ kind, title }), + item: { type: "tool", tool: event.rawKind ?? event.kind ?? "tool" }, + presentation: toolKindPresentation({ kind: event.kind, title }), }; } @@ -368,19 +372,20 @@ export function classifyAcpToolCall( const title = toOptionalString(event.title); switch (event.kind) { case "read": - return fileReadItem(event, title) ?? genericToolItem(event.kind, title); + return fileReadItem(event, title) ?? genericToolItem(event, title); case "search": - return searchItem(event) ?? genericToolItem(event.kind, title); + return searchItem(event) ?? genericToolItem(event, title); case "fetch": - return webFetchItem(event, title) ?? genericToolItem(event.kind, title); + return webFetchItem(event, title) ?? genericToolItem(event, title); case "think": return reasoningItem(event); case "execute": case "edit": case "delete": case "move": + case "switch_mode": case "other": case undefined: - return genericToolItem(event.kind, title); + return genericToolItem(event, title); } } diff --git a/plugins/provider-acp/src/wire.test.ts b/plugins/provider-acp/src/wire.test.ts index beff3c8cf7..47122d132b 100644 --- a/plugins/provider-acp/src/wire.test.ts +++ b/plugins/provider-acp/src/wire.test.ts @@ -1,10 +1,92 @@ import { describe, expect, it } from "vitest"; import { acpInitializeResultSchema, + acpRequestPermissionParamsSchema, acpSessionForkResultSchema, acpSessionNewResultSchema, + acpToolCallUpdateEventSchema, } from "./wire.js"; +describe("acpToolCallUpdateEventSchema", () => { + // ACP's ToolKind is an open enum upstream (`#[serde(other)]`). A closed zod + // enum rejected the whole tool_call for one unseen value, so the call never + // opened and its later `completed` update merged into nothing. + it("parses an unknown kind as `other` and keeps the agent's word on rawKind", () => { + const parsed = acpToolCallUpdateEventSchema.parse({ + sessionUpdate: "tool_call", + toolCallId: "call-1", + title: "Deploy preview", + kind: "deploy", + status: "in_progress", + }); + + expect(parsed.kind).toBe("other"); + expect(parsed.rawKind).toBe("deploy"); + expect(parsed.status).toBe("in_progress"); + }); + + it("accepts switch_mode and the v2 cancelled status", () => { + const parsed = acpToolCallUpdateEventSchema.parse({ + sessionUpdate: "tool_call_update", + toolCallId: "call-1", + kind: "switch_mode", + status: "cancelled", + }); + + expect(parsed.kind).toBe("switch_mode"); + expect(parsed.rawKind).toBeUndefined(); + expect(parsed.status).toBe("cancelled"); + }); + + it("parses an unknown status as pending and a null kind or status as absent", () => { + const unknownStatus = acpToolCallUpdateEventSchema.parse({ + sessionUpdate: "tool_call_update", + toolCallId: "call-1", + status: "queued", + }); + expect(unknownStatus.status).toBe("pending"); + + const nulls = acpToolCallUpdateEventSchema.parse({ + sessionUpdate: "tool_call", + toolCallId: "call-2", + kind: null, + status: null, + }); + expect(nulls.kind).toBeUndefined(); + expect(nulls.status).toBeUndefined(); + }); + + it("skips a content entry of an unknown type instead of dropping the call", () => { + const parsed = acpToolCallUpdateEventSchema.parse({ + sessionUpdate: "tool_call_update", + toolCallId: "call-1", + status: "completed", + content: [ + { type: "hologram", frames: 3 }, + { type: "content", content: { type: "text", text: "done" } }, + ], + }); + + expect(parsed.content).toEqual([ + { type: "content", content: { type: "text", text: "done" } }, + ]); + }); + + it("opens the enums on a permission request's tool call too", () => { + const parsed = acpRequestPermissionParamsSchema.parse({ + sessionId: "s", + toolCall: { toolCallId: "call-1", kind: "deploy", status: "queued" }, + options: [{ optionId: "y", name: "Allow", kind: "allow_once" }], + }); + + expect(parsed.toolCall).toMatchObject({ + kind: "other", + rawKind: "deploy", + status: "pending", + }); + }); +}); + describe("acpInitializeResultSchema", () => { it("exposes the unstable session fork capability", () => { const parsed = acpInitializeResultSchema.parse({ diff --git a/plugins/provider-acp/src/wire.ts b/plugins/provider-acp/src/wire.ts index 97234a07f8..5e62f5f0e2 100644 --- a/plugins/provider-acp/src/wire.ts +++ b/plugins/provider-acp/src/wire.ts @@ -44,8 +44,11 @@ export function extractAcpContentText( // Tool calls // --------------------------------------------------------------------------- -/** The ACP tool-call kind vocabulary; an absent kind reads as `other`. */ -export const acpToolKindSchema = z.enum([ +/** + * The ACP tool-call kind vocabulary (protocol v1 `ToolKind`); an absent kind + * reads as `other`. + */ +export const ACP_TOOL_KINDS = [ "read", "edit", "delete", @@ -54,16 +57,29 @@ export const acpToolKindSchema = z.enum([ "execute", "think", "fetch", + "switch_mode", "other", -]); +] as const; +export const acpToolKindSchema = z.enum(ACP_TOOL_KINDS); export type AcpToolKind = z.infer; +const ACP_TOOL_KIND_SET: ReadonlySet = new Set(ACP_TOOL_KINDS); -const acpToolCallStatusSchema = z.enum([ +/** + * The tool-call status vocabulary: the four v1 statuses plus the v2 draft's + * `cancelled`, which settles the call as interrupted. + */ +export const ACP_TOOL_CALL_STATUSES = [ "pending", "in_progress", "completed", "failed", -]); + "cancelled", +] as const; +const acpToolCallStatusSchema = z.enum(ACP_TOOL_CALL_STATUSES); +export type AcpToolCallStatus = z.infer; +const ACP_TOOL_CALL_STATUS_SET: ReadonlySet = new Set( + ACP_TOOL_CALL_STATUSES, +); const acpToolCallContentSchema = z.union([ z @@ -89,6 +105,21 @@ const acpToolCallContentSchema = z.union([ ]); export type AcpToolCallContent = z.infer; +/** + * A tool call's content list, one entry at a time: an entry of a type this + * schema does not know (the v2 draft adds an open `Other` content variant) + * is skipped, and the call keeps the entries it does know. A closed list + * here would drop the whole call for one foreign entry. + */ +const acpToolCallContentListSchema = z + .array(z.unknown()) + .transform((entries) => + entries.flatMap((entry) => { + const parsed = acpToolCallContentSchema.safeParse(entry); + return parsed.success ? [parsed.data] : []; + }), + ); + const acpToolCallLocationSchema = z .object({ path: z.string(), @@ -96,17 +127,60 @@ const acpToolCallLocationSchema = z }) .passthrough(); +/** + * The parsed tool-call fields. `kind` and `status` are the normalized + * vocabularies above; `rawKind` is the agent's own kind when it was not one + * of them (see `openAcpToolCallEnums`). + */ const acpToolCallFieldsSchema = z.object({ toolCallId: z.string(), title: z.string().optional(), kind: acpToolKindSchema.optional(), + rawKind: z.string().optional(), status: acpToolCallStatusSchema.optional(), - content: z.array(acpToolCallContentSchema).optional(), + content: acpToolCallContentListSchema.optional(), locations: z.array(acpToolCallLocationSchema).optional(), rawInput: z.unknown().optional(), rawOutput: z.unknown().optional(), }); +/** + * Open the tool-call enums at the wire boundary. ACP's `ToolKind` is an open + * enum upstream (`#[serde(other)]`; the v2 draft adds `Unknown(String)` and + * an open status), so an agent may send a `kind` or `status` this schema has + * never seen, and some agents serialize an absent optional as `null`. A + * closed enum here rejected the whole `tool_call`, so the call never opened + * and its later `completed` update merged into nothing — the row was lost. + * Now an unknown kind parses as `other` with the raw value kept on + * `rawKind` (it names the generic tool slot), an unknown status parses as + * `pending`, and a `null` reads as absent. + */ +function openAcpToolCallEnums(value: unknown): unknown { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return value; + } + // Freeform agent traffic: narrowed field by field below. + const fields = value as Record; + const { kind, status, ...rest } = fields; + const next: Record = rest; + if (typeof kind === "string") { + if (ACP_TOOL_KIND_SET.has(kind)) { + next["kind"] = kind; + } else { + next["kind"] = "other"; + next["rawKind"] = kind; + } + } else if (kind !== undefined && kind !== null) { + next["kind"] = kind; + } + if (typeof status === "string") { + next["status"] = ACP_TOOL_CALL_STATUS_SET.has(status) ? status : "pending"; + } else if (status !== undefined && status !== null) { + next["status"] = status; + } + return next; +} + // --------------------------------------------------------------------------- // Session updates (`session/update` notification payloads) // --------------------------------------------------------------------------- @@ -125,11 +199,14 @@ export const acpAgentThoughtChunkUpdateSchema = z }) .passthrough(); -export const acpToolCallUpdateEventSchema = acpToolCallFieldsSchema - .extend({ - sessionUpdate: z.enum(["tool_call", "tool_call_update"]), - }) - .passthrough(); +export const acpToolCallUpdateEventSchema = z.preprocess( + openAcpToolCallEnums, + acpToolCallFieldsSchema + .extend({ + sessionUpdate: z.enum(["tool_call", "tool_call_update"]), + }) + .passthrough(), +); export type AcpToolCallUpdateEvent = z.infer< typeof acpToolCallUpdateEventSchema >; @@ -399,7 +476,12 @@ export type AcpPermissionOption = z.infer; export const acpRequestPermissionParamsSchema = z .object({ sessionId: z.string(), - toolCall: acpToolCallFieldsSchema.partial().passthrough().optional(), + toolCall: z + .preprocess( + openAcpToolCallEnums, + acpToolCallFieldsSchema.partial().passthrough(), + ) + .optional(), options: z.array(acpPermissionOptionSchema).min(1), }) .passthrough();