diff --git a/products/desktop/packages/agent/package.json b/products/desktop/packages/agent/package.json index e159b71d0e2d..012826a2b553 100644 --- a/products/desktop/packages/agent/package.json +++ b/products/desktop/packages/agent/package.json @@ -147,7 +147,7 @@ "clean": "node ../../scripts/rimraf.mjs dist .turbo" }, "engines": { - "node": ">=20.0.0" + "node": ">=22.19.0" }, "devDependencies": { "@posthog/shared": "workspace:*", diff --git a/products/desktop/packages/agent/src/adapters/base-acp-agent.ts b/products/desktop/packages/agent/src/adapters/base-acp-agent.ts index eddfcf2db710..9b0bf22b71ea 100644 --- a/products/desktop/packages/agent/src/adapters/base-acp-agent.ts +++ b/products/desktop/packages/agent/src/adapters/base-acp-agent.ts @@ -47,6 +47,13 @@ export interface BaseSettingsManager { export interface BaseSession { notificationHistory: SessionNotification[]; cancelled: boolean; + /** + * Bumped on every cancel. A `prompt()` that awaits before registering its turn + * snapshots this on entry and re-checks it before handing the prompt to the + * backend: `cancelled` alone cannot tell a cancel landing in that window from a + * stale flag left by an earlier cancel. + */ + cancelSeq: number; interruptReason?: string; abortController: AbortController; settingsManager: BaseSettingsManager; @@ -78,10 +85,11 @@ export abstract class BaseAcpAgent implements Agent { throw new Error("Session ID mismatch"); } this.session.cancelled = true; + this.session.cancelSeq += 1; const meta = params._meta as { interruptReason?: string } | undefined; - if (meta?.interruptReason) { - this.session.interruptReason = meta.interruptReason; - } + // Assign even when absent, so a cancel that supplies no reason does not + // report a leftover reason from an earlier cancel. + this.session.interruptReason = meta?.interruptReason; await this.interrupt(); } diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.cancel-during-setup.test.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.cancel-during-setup.test.ts new file mode 100644 index 000000000000..bcd9245e99fd --- /dev/null +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.cancel-during-setup.test.ts @@ -0,0 +1,261 @@ +import type { AgentSideConnection } from "@agentclientprotocol/sdk"; +import type { + SDKMessage, + SDKUserMessage, +} from "@anthropic-ai/claude-agent-sdk"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + createMockQuery, + createSuccessResult, + type MockQuery, +} from "../../test/mocks/claude-sdk"; +import { Pushable } from "../../utils/streams"; + +vi.mock("@anthropic-ai/claude-agent-sdk", () => ({ + query: vi.fn(), +})); + +vi.mock("./mcp/tool-metadata", () => ({ + fetchMcpToolMetadata: vi.fn().mockResolvedValue(undefined), + getConnectedMcpServerNames: vi.fn().mockReturnValue([]), + getCachedMcpTools: vi.fn().mockReturnValue([]), + clearMcpToolMetadataCache: vi.fn(), + setMcpToolApprovalStates: vi.fn(), + isMcpToolReadOnly: vi.fn().mockReturnValue(false), + getMcpToolMetadata: vi.fn().mockReturnValue(undefined), + getMcpToolApprovalState: vi.fn().mockReturnValue(undefined), +})); + +const { ClaudeAcpAgent } = await import("./claude-agent"); +type Agent = InstanceType; + +const SESSION_ID = "s-cancel"; + +function tick(): Promise { + return new Promise((resolve) => setImmediate(resolve)); +} + +interface Harness { + agent: Agent; + session: { cancelled: boolean; cancelSeq: number }; + inSetup: Promise; + finishSetup: () => void; + completeTurn: () => Promise; + activateQueuedTurn: () => Promise; + finishTurn: () => void; + sdkReceivedMessage: () => boolean; + sdkPromptTexts: () => string[]; + prompt: (text?: string) => Promise<{ stopReason: string; _meta?: unknown }>; +} + +/** + * A session whose pre-prompt `ensureLocalToolsConnected` can be held open, the + * window a cancel lands in before the prompt reaches the SDK. `query.interrupt` + * is a deferred no-op on purpose: the default mock ends the stream synchronously, + * which rejects the still-queued turn as session-ended before `activateTurn`. + */ +function makeHarness(): Harness { + const client = { + sessionUpdate: vi.fn().mockResolvedValue(undefined), + extNotification: vi.fn().mockResolvedValue(undefined), + }; + const agent = new ClaudeAcpAgent(client as unknown as AgentSideConnection); + + const query = createMockQuery(); + query.interrupt = vi.fn(async () => {}); + const input = new Pushable(); + const pushToSdk = vi.spyOn(input, "push"); + const abortController = new AbortController(); + + const session = { + query, + queryOptions: { sessionId: SESSION_ID, cwd: "/tmp/repo", abortController }, + // Non-empty, so ensureLocalToolsConnected does not short-circuit. + localToolsServerNames: ["posthog-code-tools"], + buildInProcessMcpServers: () => ({ + "posthog-code-tools": { + type: "sdk", + name: "posthog-code-tools", + instance: {}, + }, + }), + input, + cancelled: false, + cancelSeq: 0, + interruptReason: undefined as string | undefined, + settingsManager: { dispose: vi.fn(), getRepoRoot: () => "/tmp/repo" }, + permissionMode: "auto" as const, + abortController, + accumulatedUsage: { + inputTokens: 0, + outputTokens: 0, + cachedReadTokens: 0, + cachedWriteTokens: 0, + }, + sessionResources: new Set(), + configOptions: [], + turnQueue: [], + activeTurn: null, + pendingOrphanResults: 0, + queryGeneration: 0, + cwd: "/tmp/repo", + notificationHistory: [] as unknown[], + taskRunId: "run-1", + lastContextWindowSize: 200_000, + modelId: "claude-sonnet-4-6", + taskState: new Map(), + }; + (agent as unknown as { session: typeof session }).session = session; + (agent as unknown as { sessionId: string }).sessionId = SESSION_ID; + + const { promise: inSetup, resolve: signalInSetup } = + Promise.withResolvers(); + const { promise: held, resolve: releaseSetup } = Promise.withResolvers<[]>(); + query.mcpServerStatus = vi.fn(() => { + signalInSetup(); + return held; + }) as unknown as MockQuery["mcpServerStatus"]; + + return { + agent, + session, + inSetup, + finishSetup: () => releaseSetup([]), + prompt: (text = "do the thing") => + agent.prompt({ + sessionId: SESSION_ID, + prompt: [{ type: "text", text }], + }) as Promise<{ stopReason: string; _meta?: unknown }>, + activateQueuedTurn: async () => { + query._mockHelpers.sendMessage({ + type: "system", + subtype: "local_command_output", + content: "context report", + uuid: crypto.randomUUID(), + session_id: SESSION_ID, + } as SDKMessage); + await tick(); + }, + finishTurn: () => query._mockHelpers.complete(createSuccessResult()), + // Echo the pushed user message back then complete, as the real SDK would. + completeTurn: async () => { + const { value: pushed } = await input[Symbol.asyncIterator]().next(); + query._mockHelpers.sendMessage(pushed as never); + await tick(); + query._mockHelpers.complete(createSuccessResult()); + }, + sdkReceivedMessage: () => pushToSdk.mock.calls.length > 0, + sdkPromptTexts: () => + pushToSdk.mock.calls.map(([message]) => { + const content = message.message.content; + if (typeof content === "string") { + return content; + } + return content + .map((block) => (block.type === "text" ? block.text : "")) + .join(""); + }), + }; +} + +describe("cancel arriving while a prompt is being set up", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("drops the prompt instead of handing it to the SDK", async () => { + const h = makeHarness(); + + const pending = h.prompt(); + await h.inSetup; + await h.agent.cancel({ sessionId: SESSION_ID }); + h.finishSetup(); + + await expect(pending).resolves.toMatchObject({ stopReason: "cancelled" }); + expect(h.sdkReceivedMessage()).toBe(false); + expect(h.session.cancelled).toBe(true); + }); + + it("carries the interrupt reason the cancel supplied", async () => { + const h = makeHarness(); + + const pending = h.prompt(); + await h.inSetup; + await h.agent.cancel({ + sessionId: SESSION_ID, + _meta: { interruptReason: "user_stopped" }, + }); + h.finishSetup(); + + await expect(pending).resolves.toMatchObject({ + stopReason: "cancelled", + _meta: { interruptReason: "user_stopped" }, + }); + }); + + it("runs the next prompt on the same session normally", async () => { + const h = makeHarness(); + + const cancelled = h.prompt(); + await h.inSetup; + await h.agent.cancel({ sessionId: SESSION_ID }); + h.finishSetup(); + await expect(cancelled).resolves.toMatchObject({ stopReason: "cancelled" }); + // The flag is left standing so a still-settling earlier turn can read it. + expect(h.session.cancelled).toBe(true); + + const pending = h.prompt(); + await h.completeTurn(); + + await expect(pending).resolves.toMatchObject({ stopReason: "end_turn" }); + expect(h.session.cancelled).toBe(false); + }); + + it("runs a prompt normally when the cancel predates it", async () => { + const h = makeHarness(); + + await h.agent.cancel({ sessionId: SESSION_ID }); + expect(h.session.cancelled).toBe(true); + + const pending = h.prompt(); + await h.inSetup; + h.finishSetup(); + await h.completeTurn(); + + await expect(pending).resolves.toMatchObject({ stopReason: "end_turn" }); + expect(h.session.cancelled).toBe(false); + }); + + it("drops the prompt even when a later turn activates during setup", async () => { + const h = makeHarness(); + + const cancelled = h.prompt(); + await h.inSetup; + await h.agent.cancel({ sessionId: SESSION_ID }); + + // A local-only command skips the status check the first prompt is parked in, + // so it activates during the stall and clears `session.cancelled`, leaving + // the count as the only record of the cancel. + const local = h.prompt("/context"); + await h.activateQueuedTurn(); + expect(h.session.cancelled).toBe(false); + + h.finishSetup(); + + await expect(cancelled).resolves.toMatchObject({ stopReason: "cancelled" }); + expect(h.sdkPromptTexts()).toEqual(["/context"]); + + h.finishTurn(); + await expect(local).resolves.toMatchObject({ stopReason: "end_turn" }); + }); + + it("leaves a mismatched cancel uncounted", async () => { + const h = makeHarness(); + + await expect(h.agent.cancel({ sessionId: "other" })).rejects.toThrow( + /Session ID mismatch/, + ); + expect(h.session.cancelSeq).toBe(0); + expect(h.session.cancelled).toBe(false); + }); +}); diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.permission-mode.test.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.permission-mode.test.ts index adfc6415f962..81be4e8d8fa0 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.permission-mode.test.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.permission-mode.test.ts @@ -54,6 +54,7 @@ function installFakeSession( localToolsServerNames: [] as string[], input, cancelled: false, + cancelSeq: 0, interruptReason: undefined, settingsManager: { dispose: vi.fn(), getRepoRoot: () => "/tmp/repo" }, permissionMode, diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.refresh.test.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.refresh.test.ts index 4222b68dd5b4..5524493dcf18 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.refresh.test.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.refresh.test.ts @@ -117,6 +117,7 @@ function installFakeSession( localToolsServerNames: ["posthog-code-tools"], input, cancelled: false, + cancelSeq: 0, settingsManager: { dispose: vi.fn() }, permissionMode: "default", abortController, diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.slash-command.test.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.slash-command.test.ts index 0e9f422a089f..2239cb9f455a 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.slash-command.test.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.slash-command.test.ts @@ -50,6 +50,7 @@ function installFakeSession( localToolsServerNames: [] as string[], input, cancelled: false, + cancelSeq: 0, interruptReason: undefined, settingsManager: { dispose: vi.fn(), getRepoRoot: () => "/tmp/repo" }, permissionMode: "default" as const, diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.streamed-text.test.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.streamed-text.test.ts index de9903954225..c650ae9ed90f 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.streamed-text.test.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.streamed-text.test.ts @@ -54,6 +54,7 @@ function installFakeSession( localToolsServerNames: [] as string[], input, cancelled: false, + cancelSeq: 0, interruptReason: undefined, settingsManager: { dispose: vi.fn(), getRepoRoot: () => "/tmp/repo" }, permissionMode: "default" as const, diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.task-notification.test.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.task-notification.test.ts index 5151e8dae111..35f8fdd85db9 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.task-notification.test.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.task-notification.test.ts @@ -54,6 +54,7 @@ function installFakeSession( localToolsServerNames: [] as string[], input, cancelled: false, + cancelSeq: 0, interruptReason: undefined, settingsManager: { dispose: vi.fn(), getRepoRoot: () => "/tmp/repo" }, permissionMode: "default" as const, diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts index e07a2c404fe4..6bb8b4e6eb9f 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts @@ -455,6 +455,9 @@ export class ClaudeAcpAgent extends BaseAcpAgent { } async prompt(params: PromptRequest): Promise { + // Read before any await: setup below can take seconds (the pre-prompt MCP + // reconnect), and a cancel landing in that window belongs to this prompt. + const cancelSeqAtEntry = this.session.cancelSeq; const userMessage = promptToClaude(params); const promptUuid = randomUUID(); userMessage.uuid = promptUuid; @@ -520,6 +523,14 @@ export class ClaudeAcpAgent extends BaseAcpAgent { }); } + // A cancel counted during setup targets this prompt: nothing was queued, so + // `interrupt()` had nothing to stop, and returning before the push is what + // cancels it. `session.cancelled` is left standing because an earlier turn + // may still be settling against it. + if (this.session.cancelSeq > cancelSeqAtEntry) { + return this.cancelledResponse(); + } + const turn: Turn = { promptUuid, pendingSteerUuids: new Set(), @@ -696,6 +707,8 @@ export class ClaudeAcpAgent extends BaseAcpAgent { const activateTurn = async (turn: Turn) => { session.activeTurn = turn; + // A cancel aimed at this turn already settled it (early return during + // setup, `interrupt()` sweep once queued), so a set flag here is stale. session.cancelled = false; session.interruptReason = undefined; session.pendingOrphanResults = 0; @@ -2111,6 +2124,7 @@ export class ClaudeAcpAgent extends BaseAcpAgent { localToolsServerNames, input, cancelled: false, + cancelSeq: 0, settingsManager, permissionMode, cloudMode: cloudRun, diff --git a/products/desktop/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts b/products/desktop/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts index 25e54abe5b46..ed3a01603416 100644 --- a/products/desktop/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts +++ b/products/desktop/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts @@ -342,6 +342,7 @@ export class CodexAppServerAgent extends BaseAcpAgent { ), notificationHistory: [], cancelled: false, + cancelSeq: 0, }; }