diff --git a/src/agentMode/backends/opencode/OpencodeBackend.ts b/src/agentMode/backends/opencode/OpencodeBackend.ts index 9abf15b6..d83d10a4 100644 --- a/src/agentMode/backends/opencode/OpencodeBackend.ts +++ b/src/agentMode/backends/opencode/OpencodeBackend.ts @@ -317,9 +317,9 @@ export async function buildOpencodeConfig( // native `build` agent this only adds the external_directory key — bash/edit // stay at opencode's permissive defaults, so it does not start asking. // - // NOTE (version-sensitive, pinned opencode 1.15.13): the `external_directory` + // NOTE (version-sensitive, pinned opencode 1.16.0): the `external_directory` // permission key and the `{ "": "allow" }` shape are confirmed against - // 1.15.13; re-verify when the pinned opencode version changes. + // 1.16.0; re-verify when the pinned opencode version changes. const externalDirectoryPermission = cacheRoot ? { external_directory: { [`${cacheRoot}/**`]: "allow" } } : undefined; diff --git a/src/agentMode/backends/opencode/OpencodeBinaryManager.test.ts b/src/agentMode/backends/opencode/OpencodeBinaryManager.test.ts index 7b8c33cc..8447255d 100644 --- a/src/agentMode/backends/opencode/OpencodeBinaryManager.test.ts +++ b/src/agentMode/backends/opencode/OpencodeBinaryManager.test.ts @@ -56,6 +56,7 @@ import * as os from "node:os"; import * as path from "node:path"; import { computeInstallState, + isOpencodeVersionOutdated, legacyVaultDataDir, opencodeManagedDataDir, OpencodeBinaryManager, @@ -65,6 +66,13 @@ import { verifyOpencodeBinary, } from "./OpencodeBinaryManager"; +describe("isOpencodeVersionOutdated", () => { + it("requires the ACP cancellation fix shipped in 1.16.0", () => { + expect(isOpencodeVersionOutdated("1.15.13")).toBe(true); + expect(isOpencodeVersionOutdated("1.16.0")).toBe(false); + }); +}); + // Pull the mock helpers off the mocked module without TS complaints. const settingsMock = jest.requireMock("@/settings/model"); diff --git a/src/agentMode/backends/opencode/OpencodeBinaryManager.ts b/src/agentMode/backends/opencode/OpencodeBinaryManager.ts index 582ab930..93d88967 100644 --- a/src/agentMode/backends/opencode/OpencodeBinaryManager.ts +++ b/src/agentMode/backends/opencode/OpencodeBinaryManager.ts @@ -128,10 +128,10 @@ export function readOpencodeSettings(): OpencodeBackendSettings { /** * Whether an installed opencode version predates the minimum the plugin - * supports ({@link OPENCODE_MIN_ACP_VERSION}). Older binaries advertise models - * through the now-removed ACP `models` state, so the picker can't surface them. - * An unknown version isn't flagged — we can't prove it's old, and a missing - * install is handled by the install prompt instead. + * supports ({@link OPENCODE_MIN_ACP_VERSION}). Older binaries either predate + * the current model catalog or leave the backing turn running after ACP + * cancellation. An unknown version isn't flagged — we can't prove it's old, + * and a missing install is handled by the install prompt instead. */ export function isOpencodeVersionOutdated(version: string | undefined): boolean { if (!version) return false; diff --git a/src/agentMode/session/AgentSession.test.ts b/src/agentMode/session/AgentSession.test.ts index 59d25476..f4f36205 100644 --- a/src/agentMode/session/AgentSession.test.ts +++ b/src/agentMode/session/AgentSession.test.ts @@ -1029,6 +1029,97 @@ describe("AgentSession.sendPrompt", () => { resolvePrompt!({ stopReason: "cancelled" }); expect(await turn).toBe("cancelled"); }); + + it("settles locally and drops retry output when backend cancellation fails", async () => { + const mock = makeMockBackend(); + let resolveBackingPrompt: ((value: { stopReason: "cancelled" }) => void) | null = null; + let resolveSecondPrompt: ((value: { stopReason: "end_turn" }) => void) | null = null; + mock.prompt + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveBackingPrompt = resolve; + }) + ) + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveSecondPrompt = resolve; + }) + ); + const session = new AgentSession({ + backend: mock.asBackend, + backendSessionId: "acp-1", + internalId: "internal-1", + backendId: "opencode", + }); + const emitChunk = ( + sessionUpdate: "agent_thought_chunk" | "agent_message_chunk", + text: string, + messageId = "msg-first" + ) => + mock.emit({ + sessionId: "acp-1", + update: { sessionUpdate, messageId, content: { type: "text", text } }, + }); + const { turn } = session.sendPrompt("first"); + + emitChunk("agent_thought_chunk", "initial thought"); + emitChunk("agent_message_chunk", "partial answer"); + mock.cancel.mockImplementation(() => { + emitChunk("agent_thought_chunk", " retry during cancel"); + mock.emit({ + sessionId: "acp-1", + update: { + sessionUpdate: "tool_call", + toolCallId: "tc-stale", + title: "Stale retry tool", + status: "pending", + }, + }); + return Promise.reject(new Error("cancel unsupported")); + }); + + await session.cancel(); + await expect(turn).resolves.toBe("cancelled"); + expect(session.getStatus()).toBe("idle"); + + emitChunk("agent_message_chunk", " retry after cancel"); + const firstAnswer = session.store + .getDisplayMessages() + .find((message) => message.sender === AI_SENDER); + expect(firstAnswer?.message).toBe("partial answer"); + expect(firstAnswer?.parts).toEqual( + expect.arrayContaining([ + expect.objectContaining({ kind: "thought", text: "initial thought" }), + ]) + ); + expect(firstAnswer?.parts?.some((part) => part.kind === "tool_call")).toBe(false); + expect(firstAnswer?.turnStopReason).toBe("cancelled"); + + const next = session.sendPrompt("second"); + expect(mock.prompt).toHaveBeenCalledTimes(1); + emitChunk("agent_message_chunk", " stale retry", "msg-retry"); + + jest.useFakeTimers(); + try { + resolveBackingPrompt!({ stopReason: "cancelled" }); + await jest.advanceTimersByTimeAsync(500); + emitChunk("agent_message_chunk", " delayed stale retry", "msg-retry"); + await jest.advanceTimersByTimeAsync(500); + expect(mock.prompt).toHaveBeenCalledTimes(1); + + await jest.advanceTimersByTimeAsync(1_000); + expect(mock.prompt).toHaveBeenCalledTimes(2); + emitChunk("agent_message_chunk", " stale trailing chunk", "msg-retry"); + emitChunk("agent_message_chunk", "recovered", "msg-second"); + resolveSecondPrompt!({ stopReason: "end_turn" }); + await expect(next.turn).resolves.toBe("end_turn"); + expect(session.store.getDisplayMessages().at(-1)?.message).toBe("recovered"); + } finally { + jest.useRealTimers(); + } + }); }); describe("withReadOnlyPreamble", () => { diff --git a/src/agentMode/session/AgentSession.ts b/src/agentMode/session/AgentSession.ts index ac6fcd52..eb10913d 100644 --- a/src/agentMode/session/AgentSession.ts +++ b/src/agentMode/session/AgentSession.ts @@ -25,6 +25,7 @@ import { PlanSummary, PromptContent, PromptInput, + PromptOutput, SessionEvent, SessionId, SessionUsage, @@ -84,6 +85,10 @@ export const DEFAULT_TITLE_PREFIX = "New session"; * agent's own context always receives the full output regardless. */ const MAX_TOOL_OUTPUT_TEXT_CHARS = 256_000; +// ACP prompt results can arrive before the last session/update frames. Require +// a quiet interval after a cancelled backing prompt settles before dispatching +// a follow-up on the same session. +const CANCELLED_TURN_QUIET_MS = 1_000; // Shared sentinel so `getPendingToolPermissions()` returns a stable reference // when nothing is pending — preserves React `useState` setter bail-out // behavior on idle subscription ticks. @@ -376,6 +381,12 @@ export class AgentSession { // a newer turn's placeholder. Replaced on the next completion, cleared on // cancel/dispose. private settledStream: { placeholderId: string; messageIds: Set } | null = null; + // A locally-cancelled prompt whose backend promise has not settled yet. A + // follow-up may be composed immediately, but its backend prompt waits on this + // barrier so output from the cancelled generation cannot target the new turn. + private cancelledPromptDrain: Promise | null = null; + private cancelledTurnActivity = 0; + private cancelledMessageIds = new Set(); private abortController: AbortController | null = null; private listeners = new Set(); private unregisterSessionHandler: (() => void) | null = null; @@ -1002,7 +1013,18 @@ export class AgentSession { ): Promise { const placeholderId = this.placeholderId; const sessionId = this.backendSessionId!; + const signal = this.abortController!.signal; try { + const priorPromptDrain = this.cancelledPromptDrain; + if (priorPromptDrain) { + await Promise.race([ + priorPromptDrain, + new Promise((resolve) => { + signal.addEventListener("abort", () => resolve(), { once: true }); + }), + ]); + } + // Extract live Web Viewer content (reader-mode markdown, YouTube // transcripts) just before the prompt is built so it reflects the page // at send/flush time, not at compose time. Only take the async hop when @@ -1083,7 +1105,39 @@ export class AgentSession { sessionId, prompt: promptBlocks, }; - const resp = await this.backend.prompt(req); + const promptStarted = !signal.aborted; + let resp: PromptOutput = { stopReason: "cancelled" }; + if (promptStarted) { + const backingPrompt = this.backend.prompt(req); + resp = await Promise.race([ + backingPrompt, + new Promise((resolve) => { + signal.addEventListener("abort", () => resolve({ stopReason: "cancelled" }), { + once: true, + }); + }), + ]); + if (signal.aborted) { + const settledPrompt = backingPrompt.then( + () => undefined, + () => undefined + ); + const drain = settledPrompt.then(async () => { + let activity: number; + do { + activity = this.cancelledTurnActivity; + await new Promise((resolve) => + window.setTimeout(resolve, CANCELLED_TURN_QUIET_MS) + ); + } while (activity !== this.cancelledTurnActivity); + }); + this.cancelledPromptDrain = drain; + void drain.then(() => { + if (this.cancelledPromptDrain === drain) this.cancelledPromptDrain = null; + }); + resp = { stopReason: "cancelled" }; + } + } // Flush the buffer only on a non-cancelled completion — only then has the // backend durably ingested the `` block. A cancelled prompt may // have stopped before ingesting it, so keep and re-inject (a duplicate is @@ -1095,13 +1149,13 @@ export class AgentSession { // the turn, so a hard `prompt()` failure (transport/auth) leaves the flag // unset and the user's retry re-delivers the context (a user cancel still // resolves here, and the prompt did reach the backend, so it counts as delivered). - if (isFirstTurn) this.firstPromptSent = true; + if (isFirstTurn && promptStarted) this.firstPromptSent = true; // Same delivery contract as `firstPromptSent`: the note reached the backend // in this accepted (possibly cancelled) prompt, so advance the session's // cursor. A hard `prompt()` failure throws before here, leaving the cursor // unmoved so the retry re-delivers the note. Fan-out returns earlier and // never reaches this ack. - if (projectContextUpdates) { + if (projectContextUpdates && promptStarted) { this.markProjectContextUpdatesDeliveredFn?.(projectContextUpdates.epoch); } if ( @@ -1266,9 +1320,9 @@ export class AgentSession { } /** - * Cancel any in-flight turn. The backend may still emit a few trailing - * session events before the prompt promise resolves with - * `stopReason: "cancelled"` — that's expected. + * Cancel any in-flight turn. Local cancellation settles the prompt even if + * the backend ignores ACP `session/cancel`; the backend notification still + * runs so a compliant agent can stop work and keep its session reusable. * * Flushes any pending tool-permission and AskUserQuestion resolvers (rejects * / empty answers respectively) so the inline cards disappear immediately @@ -1279,6 +1333,10 @@ export class AgentSession { const status = this.getStatus(); if (status !== "running" && status !== "awaiting_permission") return; if (!this.backendSessionId) return; + this.abortController?.abort(); + this.settledStream = null; + for (const messageId of this.currentMessageIds) this.cancelledMessageIds.add(messageId); + this.currentMessageIds = new Set(); if (this.pendingToolResolvers.size > 0) { this.flushResolvers(this.pendingToolResolvers); this.notifyMessages(); @@ -1292,7 +1350,6 @@ export class AgentSession { } catch (e) { logWarn(`[AgentMode] cancel notification failed`, e); } - this.abortController?.abort(); } /** Detach from the backend. Does not cancel — call `cancel()` first. */ @@ -1308,6 +1365,8 @@ export class AgentSession { this.currentTodoList = null; this.currentTodoListSignature = null; this.settledStream = null; + this.cancelledPromptDrain = null; + this.cancelledMessageIds.clear(); this.currentMessageIds = new Set(); // Fire the `"closed"` transition before clearing listeners so // subscribers still observe it. `disposed = true` above guarantees @@ -1618,6 +1677,27 @@ export class AgentSession { private handleSessionEvent(event: SessionEvent): void { const update = event.update; + if (this.cancelledPromptDrain || this.abortController?.signal.aborted) { + switch (update.sessionUpdate) { + case "agent_message_chunk": + case "agent_thought_chunk": + this.cancelledTurnActivity += 1; + if (update.messageId) this.cancelledMessageIds.add(update.messageId); + logWarn( + `[AgentMode] dropping ${update.sessionUpdate} from cancelled turn ${this.internalId}` + ); + return; + case "tool_call": + case "tool_call_update": + case "plan": + this.cancelledTurnActivity += 1; + logWarn( + `[AgentMode] dropping ${update.sessionUpdate} from cancelled turn ${this.internalId}` + ); + return; + } + } + // Refresh the live todo snapshot before any placeholder gating — the // snapshot is session-scoped state, not part of the message trail. if (update.sessionUpdate === "plan" && this.applyCurrentTodoList(update.entries)) { @@ -1742,6 +1822,13 @@ export class AgentSession { * when there's no message to append to (a genuinely stray update). */ private resolveContentTarget(messageId: string | undefined): string | null { + if ( + this.cancelledPromptDrain || + this.abortController?.signal.aborted || + (messageId !== undefined && this.cancelledMessageIds.has(messageId)) + ) { + return null; + } if (messageId && this.settledStream?.messageIds.has(messageId)) { return this.settledStream.placeholderId; } diff --git a/src/constants.ts b/src/constants.ts index 2e21703a..38762010 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -935,15 +935,14 @@ export const RESTRICTION_MESSAGES = { `${extension.toUpperCase()} files are not supported in the current mode.`, } as const; -export const OPENCODE_PINNED_VERSION = "1.15.13"; +export const OPENCODE_PINNED_VERSION = "1.16.0"; export const OPENCODE_RELEASE_TAG = `v${OPENCODE_PINNED_VERSION}`; /** - * Minimum opencode the plugin supports. v1.15.13 (PR sst/opencode#29929, - * "promote next ACP implementation") dropped the dedicated ACP `models` state - * and moved the model catalog to a `category:"model"` config option; older - * binaries report no models to our picker, so we force an upgrade. + * Minimum opencode the plugin supports. v1.15.13 introduced the config-option + * model catalog we consume, and v1.16.0 made ACP `session/cancel` abort the + * backing turn so a stopped session remains reusable. */ -export const OPENCODE_MIN_ACP_VERSION = "1.15.13"; +export const OPENCODE_MIN_ACP_VERSION = "1.16.0"; export const OPENCODE_RELEASE_URL_TEMPLATE = "https://github.com/sst/opencode/releases/download/v{version}/{asset}"; export const OPENCODE_RELEASE_API_URL_TEMPLATE =