From e8d2de993136147c3e94a08adc880b3be0bb9072 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Mon, 4 May 2026 00:06:05 +0200 Subject: [PATCH] fix: Handle result origins in ACP agent Add origin-aware raw SDK message filtering and forward result origin metadata in usage updates. Ignore task-notification followup results when computing the user-turn stop reason or forwarding local slash-command output. --- src/acp-agent.ts | 42 +++++-- src/tests/acp-agent.test.ts | 214 +++++++++++++++++++++++++++++++++++- 2 files changed, 247 insertions(+), 9 deletions(-) diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 7774d737..93ba6ef1 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -56,6 +56,7 @@ import { query, Settings, SDKAssistantMessageError, + SDKMessageOrigin, SDKPartialAssistantMessage, SDKUserMessage, SlashCommand, @@ -179,6 +180,7 @@ type BackgroundTerminal = export type SDKMessageFilter = { type: string; subtype?: string; + origin?: SDKMessageOrigin["kind"]; }; /** @@ -873,6 +875,12 @@ export class ClaudeAcpAgent implements Agent { session.contextWindowSize = matchingModelUsage.contextWindow; } + // Task-notification followups are autonomous work triggered by a + // task-notification system message, not by the user's prompt. + // They should not influence the user-turn lifecycle (stop reason, + // slash-command output forwarding) but their cost is real. + const isTaskNotification = message.origin?.kind === "task-notification"; + // Send usage_update notification if (lastAssistantTotalUsage !== null) { await this.client.sessionUpdate({ @@ -885,12 +893,17 @@ export class ClaudeAcpAgent implements Agent { amount: message.total_cost_usd, currency: "USD", }, + ...(message.origin && { + _meta: { "_claude/origin": message.origin }, + }), }, }); } if (session.cancelled) { - stopReason = "cancelled"; + if (!isTaskNotification) { + stopReason = "cancelled"; + } break; } @@ -900,7 +913,9 @@ export class ClaudeAcpAgent implements Agent { throw RequestError.authRequired(); } if (message.stop_reason === "max_tokens") { - stopReason = "max_tokens"; + if (!isTaskNotification) { + stopReason = "max_tokens"; + } break; } if (message.is_error) { @@ -911,7 +926,9 @@ export class ClaudeAcpAgent implements Agent { } // For local-only commands (no model invocation), the result // text is the command output — forward it to the client. - if (isLocalOnlyCommand) { + // Task-notification followups never originate from a user + // slash command, so skip the forwarding for them. + if (isLocalOnlyCommand && !isTaskNotification) { for (const notification of toAcpNotifications( message.result, "assistant", @@ -927,7 +944,9 @@ export class ClaudeAcpAgent implements Agent { } case "error_during_execution": { if (message.stop_reason === "max_tokens") { - stopReason = "max_tokens"; + if (!isTaskNotification) { + stopReason = "max_tokens"; + } break; } if (message.is_error) { @@ -936,7 +955,9 @@ export class ClaudeAcpAgent implements Agent { message.errors.join(", ") || message.subtype, ); } - stopReason = "end_turn"; + if (!isTaskNotification) { + stopReason = "end_turn"; + } break; } case "error_max_budget_usd": @@ -948,7 +969,9 @@ export class ClaudeAcpAgent implements Agent { message.errors.join(", ") || message.subtype, ); } - stopReason = "max_turn_requests"; + if (!isTaskNotification) { + stopReason = "max_turn_requests"; + } break; default: unreachable(message, this.logger); @@ -2034,12 +2057,15 @@ export class ClaudeAcpAgent implements Agent { function shouldEmitRawMessage( config: boolean | SDKMessageFilter[], - message: { type: string; subtype?: string }, + message: { type: string; subtype?: string; origin?: SDKMessageOrigin }, ): boolean { if (config === true) return true; if (config === false) return false; return config.some( - (f) => f.type === message.type && (f.subtype === undefined || f.subtype === message.subtype), + (f) => + f.type === message.type && + (f.subtype === undefined || f.subtype === message.subtype) && + (f.origin === undefined || f.origin === message.origin?.kind), ); } diff --git a/src/tests/acp-agent.test.ts b/src/tests/acp-agent.test.ts index 064b082a..2afe3317 100644 --- a/src/tests/acp-agent.test.ts +++ b/src/tests/acp-agent.test.ts @@ -32,6 +32,7 @@ import { ClaudeAcpAgent, claudeCliPath, describeAlwaysAllow, + type SDKMessageFilter, } from "../acp-agent.js"; import { Pushable } from "../utils.js"; import { query, SDKAssistantMessage } from "@anthropic-ai/claude-agent-sdk"; @@ -3021,7 +3022,7 @@ describe("emitRawSDKMessages", () => { function injectSession( agent: ClaudeAcpAgent, messages: any[], - emitRawSDKMessages: boolean | { type: string; subtype?: string }[], + emitRawSDKMessages: boolean | SDKMessageFilter[], ) { const input = new Pushable(); async function* messageGenerator() { @@ -3198,4 +3199,215 @@ describe("emitRawSDKMessages", () => { expect(sdkMessages[0].params.message.subtype).toBe("compact_boundary"); expect(sdkMessages[1].params.message.type).toBe("result"); }); + + it("filter by origin kind only emits matching results", async () => { + const { agent, extNotifications } = createMockAgentWithExtNotification(); + injectSession( + agent, + [ + { ...createResultMessage(), origin: { kind: "channel", server: "acp" } }, + { ...createResultMessage(), origin: { kind: "task-notification" } }, + { type: "system", subtype: "session_state_changed", state: "idle" }, + ], + [{ type: "result", origin: "task-notification" }], + ); + + await agent.prompt({ sessionId: "test-session", prompt: [{ type: "text", text: "test" }] }); + + const sdkMessages = extNotifications.filter((n) => n.method === "_claude/sdkMessage"); + expect(sdkMessages).toHaveLength(1); + expect(sdkMessages[0].params.message.origin.kind).toBe("task-notification"); + }); + + it("filter without origin matches results regardless of origin", async () => { + const { agent, extNotifications } = createMockAgentWithExtNotification(); + injectSession( + agent, + [ + { ...createResultMessage(), origin: { kind: "channel", server: "acp" } }, + { ...createResultMessage(), origin: { kind: "task-notification" } }, + { type: "system", subtype: "session_state_changed", state: "idle" }, + ], + [{ type: "result" }], + ); + + await agent.prompt({ sessionId: "test-session", prompt: [{ type: "text", text: "test" }] }); + + const sdkMessages = extNotifications.filter((n) => n.method === "_claude/sdkMessage"); + expect(sdkMessages).toHaveLength(2); + }); +}); + +describe("result origin handling", () => { + function createMockAgentWithCapture() { + const updates: any[] = []; + const mockClient = { + sessionUpdate: async (notification: any) => { + updates.push(notification); + }, + } as unknown as AgentSideConnection; + const agent = new ClaudeAcpAgent(mockClient, { log: () => {}, error: () => {} }); + return { agent, updates }; + } + + function injectSession(agent: ClaudeAcpAgent, messages: any[]) { + const input = new Pushable(); + async function* messageGenerator() { + const iter = input[Symbol.asyncIterator](); + const { value: userMessage, done } = await iter.next(); + if (!done && userMessage) { + yield { + type: "user", + message: userMessage.message, + parent_tool_use_id: null, + uuid: userMessage.uuid, + session_id: "test-session", + isReplay: true, + }; + } + yield* messages; + } + agent.sessions["test-session"] = { + query: messageGenerator() as any, + input, + cancelled: false, + cwd: "/test", + sessionFingerprint: JSON.stringify({ cwd: "/test", mcpServers: [] }), + modes: { currentModeId: "default", availableModes: [] }, + models: { currentModelId: "default", availableModels: [] }, + modelInfos: [], + settingsManager: { dispose: vi.fn() } as any, + accumulatedUsage: { + inputTokens: 0, + outputTokens: 0, + cachedReadTokens: 0, + cachedWriteTokens: 0, + }, + configOptions: [], + promptRunning: false, + pendingMessages: new Map(), + nextPendingOrder: 0, + abortController: new AbortController(), + emitRawSDKMessages: false, + contextWindowSize: 200000, + }; + } + + function createAssistantMessage() { + return { + type: "assistant" as const, + parent_tool_use_id: null, + uuid: randomUUID(), + session_id: "test-session", + message: { + model: "claude-sonnet-4-6", + content: [{ type: "text", text: "hello" }], + usage: { + input_tokens: 10, + output_tokens: 5, + cache_read_input_tokens: 0, + cache_creation_input_tokens: 0, + }, + }, + }; + } + + function createResult(overrides: Record = {}) { + return { + type: "result" as const, + subtype: "success" as const, + stop_reason: "end_turn", + is_error: false, + result: "", + errors: [], + duration_ms: 0, + duration_api_ms: 0, + num_turns: 1, + total_cost_usd: 0.01, + usage: { + input_tokens: 10, + output_tokens: 5, + cache_read_input_tokens: 0, + cache_creation_input_tokens: 0, + }, + modelUsage: {}, + permission_denials: [], + uuid: randomUUID(), + session_id: "test-session", + ...overrides, + }; + } + + it("forwards origin in usage_update _meta", async () => { + const { agent, updates } = createMockAgentWithCapture(); + injectSession(agent, [ + createAssistantMessage(), + createResult({ origin: { kind: "channel", server: "acp" } }), + { type: "system", subtype: "session_state_changed", state: "idle" }, + ]); + + await agent.prompt({ sessionId: "test-session", prompt: [{ type: "text", text: "test" }] }); + + const usageUpdate = updates.find((u: any) => u.update?.sessionUpdate === "usage_update"); + expect(usageUpdate).toBeDefined(); + expect(usageUpdate.update._meta).toEqual({ + "_claude/origin": { kind: "channel", server: "acp" }, + }); + }); + + it("omits _meta when origin is absent", async () => { + const { agent, updates } = createMockAgentWithCapture(); + injectSession(agent, [ + createAssistantMessage(), + createResult(), + { type: "system", subtype: "session_state_changed", state: "idle" }, + ]); + + await agent.prompt({ sessionId: "test-session", prompt: [{ type: "text", text: "test" }] }); + + const usageUpdate = updates.find((u: any) => u.update?.sessionUpdate === "usage_update"); + expect(usageUpdate).toBeDefined(); + expect(usageUpdate.update._meta).toBeUndefined(); + }); + + it("task-notification result with max_tokens does not override the user-turn stopReason", async () => { + const { agent } = createMockAgentWithCapture(); + injectSession(agent, [ + createAssistantMessage(), + // User-turn result completes normally + createResult({ origin: { kind: "channel", server: "acp" } }), + // Task-notification followup hits max_tokens — must not bleed into the user's stopReason + createResult({ + stop_reason: "max_tokens", + origin: { kind: "task-notification" }, + }), + { type: "system", subtype: "session_state_changed", state: "idle" }, + ]); + + const response = await agent.prompt({ + sessionId: "test-session", + prompt: [{ type: "text", text: "test" }], + }); + + expect(response.stopReason).toBe("end_turn"); + }); + + it("user-prompted result with max_tokens still sets stopReason", async () => { + const { agent } = createMockAgentWithCapture(); + injectSession(agent, [ + createAssistantMessage(), + createResult({ + stop_reason: "max_tokens", + origin: { kind: "channel", server: "acp" }, + }), + { type: "system", subtype: "session_state_changed", state: "idle" }, + ]); + + const response = await agent.prompt({ + sessionId: "test-session", + prompt: [{ type: "text", text: "test" }], + }); + + expect(response.stopReason).toBe("max_tokens"); + }); });