diff --git a/packages/ui/src/lib/opencode/client.ts b/packages/ui/src/lib/opencode/client.ts index 3e8ab753..7b618f4a 100644 --- a/packages/ui/src/lib/opencode/client.ts +++ b/packages/ui/src/lib/opencode/client.ts @@ -12,6 +12,7 @@ import type { TextPartInput, FilePartInput, } from "@opencode-ai/sdk/v2"; +import { isAmbiguousTransportFailure, markAmbiguousTransportFailure } from "@/lib/relay/transport-error"; import type { PermissionRequest } from "@/types/permission"; import type { QuestionRequest } from "@/types/question"; @@ -878,7 +879,13 @@ class OpencodeService { // failure) — there is no HTTP response to report. Never fabricate a // status: surface it as a transport error so callers treat it like // any other network failure instead of a server 500. - throw new Error(`Message send transport failure: ${formatSdkError(result.error)}`); + // Preserve the transport's "dispatched, outcome unknown" tag through + // the wrap: without it the caller cannot tell a lost response from a + // send that never reached the server, and re-sends a running prompt. + const transportError = new Error(`Message send transport failure: ${formatSdkError(result.error)}`); + throw isAmbiguousTransportFailure(result.error) + ? markAmbiguousTransportFailure(transportError) + : transportError; } response = new Response(JSON.stringify(result.error), { status }); } else { diff --git a/packages/ui/src/lib/relay/transport-error.ts b/packages/ui/src/lib/relay/transport-error.ts new file mode 100644 index 00000000..8d3c1159 --- /dev/null +++ b/packages/ui/src/lib/relay/transport-error.ts @@ -0,0 +1,42 @@ +/** + * Ambiguous transport failures. + * + * When a request dies after it was already handed to the transport, the client + * knows the response was lost — it does NOT know whether the server processed + * the request. Over the relay tunnel this is the common case: a reconnect, a + * host-side stream abort, or a dead channel all fail an in-flight POST that may + * already be running server-side. + * + * Callers must be able to tell that state apart from a definite failure, and + * string-matching the message text is not a contract — a renamed abort reason + * silently reclassifies a send. Transports therefore tag these errors, and + * callers read the tag (see `isAmbiguousTransportFailure`). + * + * `prompt_async` is the motivating case: treating an ambiguous failure as a + * definite one rolls back the user message and lets the queue re-send a prompt + * the engine is already answering, producing two independent AI responses. + */ + +const AMBIGUOUS_TRANSPORT_FLAG = '__openchamberAmbiguousTransport'; + +/** + * Mark an error as "dispatched, outcome unknown". Returns the same error so it + * can be thrown inline. + */ +export const markAmbiguousTransportFailure = (error: T): T => { + Object.defineProperty(error, AMBIGUOUS_TRANSPORT_FLAG, { + value: true, + enumerable: false, + configurable: true, + }); + return error; +}; + +/** + * True when a transport tagged this error as dispatched-but-unconfirmed. + * Deliberately tag-only: text heuristics belong to the caller that owns them. + */ +export const isAmbiguousTransportFailure = (error: unknown): boolean => { + if (!error || typeof error !== 'object') return false; + return (error as Record)[AMBIGUOUS_TRANSPORT_FLAG] === true; +}; diff --git a/packages/ui/src/lib/relay/tunnel-client.test.ts b/packages/ui/src/lib/relay/tunnel-client.test.ts index 0fd903b8..7972b342 100644 --- a/packages/ui/src/lib/relay/tunnel-client.test.ts +++ b/packages/ui/src/lib/relay/tunnel-client.test.ts @@ -11,6 +11,7 @@ import { } from './crypto'; import { createHostHandshake } from './handshake'; import { TunnelFrameType } from './protocol'; +import { isAmbiguousTransportFailure } from './transport-error'; import { createFragmentAssembler, decodeFrameBatch, @@ -339,6 +340,24 @@ describe('createRelayTunnelClient', () => { await expect(reader.read()).rejects.toThrow(); }); + // A POST that dies after dispatch may already have been processed by the + // server. Callers must be able to tell that apart from a definite failure — + // a prompt re-sent on this error produces a second AI response (#2425). + test('tags an in-flight request killed by reconnect as an ambiguous failure', async () => { + const { client, killWire } = await setupClient({ silent: true }); + track(client); + const pending = client.fetch('/api/session/s1/prompt_async', { method: 'POST', body: '{}' }); + let caught: unknown = null; + const settled = pending.catch((error: unknown) => { + caught = error; + }); + await wait(20); + killWire(); + await settled; + expect(caught).toBeInstanceOf(Error); + expect(isAmbiguousTransportFailure(caught)).toBe(true); + }); + test('opens, echoes, and closes a tunneled WebSocket', async () => { const { client } = await setupClient(); track(client); diff --git a/packages/ui/src/lib/relay/tunnel-client.ts b/packages/ui/src/lib/relay/tunnel-client.ts index ab24d78f..fd12b3cb 100644 --- a/packages/ui/src/lib/relay/tunnel-client.ts +++ b/packages/ui/src/lib/relay/tunnel-client.ts @@ -35,6 +35,7 @@ import { isWsClosePayload, normalizeTunnelRequest, } from './tunnel-payloads'; +import { markAmbiguousTransportFailure } from './transport-error'; const EMPTY_PAYLOAD = new Uint8Array(0); const textEncoder = new TextEncoder(); @@ -721,6 +722,13 @@ export const createRelayTunnelClient = (options: RelayTunnelClientOptions): Rela } }; + // The request head is written to the channel below before any of these + // failures can fire, so losing the stream never proves the server did + // not process the request — only that the response was lost. Callers + // that would otherwise retry (prompt sends) must see that distinction. + const dispatchedFailure = (message: string): Error => + markAmbiguousTransportFailure(new Error(message)); + onAbort = () => { sendAbort('aborted'); finishError(abortError()); @@ -735,7 +743,7 @@ export const createRelayTunnelClient = (options: RelayTunnelClientOptions): Rela head = decodeJsonPayload(payload, isHttpResponsePayload); } catch (error) { sendAbort('malformed response head'); - finishError(toError(error)); + finishError(dispatchedFailure(toError(error).message)); return; } const nullBody = head.status === 204 || head.status === 205 || head.status === 304; @@ -773,7 +781,7 @@ export const createRelayTunnelClient = (options: RelayTunnelClientOptions): Rela if (frameType === TunnelFrameType.StreamEnd) { if (finished) return; if (!responseDelivered) { - finishError(new Error('tunnel stream ended before response head')); + finishError(dispatchedFailure('tunnel stream ended before response head')); return; } finished = true; @@ -792,11 +800,15 @@ export const createRelayTunnelClient = (options: RelayTunnelClientOptions): Rela } catch { // Keep the generic reason. } - finishError(new Error(reason)); + finishError(dispatchedFailure(reason)); } }, fail(error) { - finishError(error); + // Channel death (reconnect, keepalive timeout) with this stream still + // open — same rule as above: dispatched, outcome unknown. A fresh + // error is tagged instead of the shared one so the tag cannot leak to + // waiters whose request never reached the wire. + finishError(dispatchedFailure(error.message)); }, }); @@ -824,7 +836,7 @@ export const createRelayTunnelClient = (options: RelayTunnelClientOptions): Rela } } catch (error) { sendAbort('request body failed'); - finishError(toError(error)); + finishError(dispatchedFailure(toError(error).message)); } })(); }); diff --git a/packages/ui/src/sync/DOCUMENTATION.md b/packages/ui/src/sync/DOCUMENTATION.md index 78152f32..96761339 100644 --- a/packages/ui/src/sync/DOCUMENTATION.md +++ b/packages/ui/src/sync/DOCUMENTATION.md @@ -232,6 +232,7 @@ Rules: 3. `session-ui-store.ts` should delegate to `session-actions.ts` for these mutations instead of duplicating SDK calls. 4. Sending after a revert commits the new branch optimistically: remove the reverted tail and marker before inserting the new message, and restore both if the send is rejected. 5. After session creation, the directory returned by the server is authoritative over the requested draft directory. The server may canonicalize a worktree path, and the first prompt must use the same directory identity as the created session. +6. A prompt send that fails **after** the request left the client is ambiguous, never a definite failure: the server may already be answering it. Transports tag those errors (`markAmbiguousTransportFailure` in `@/lib/relay/transport-error`; the relay tunnel tags every stream that dies with a request in flight), and `isAmbiguousSendFailure` reads the tag before falling back to status/text heuristics. An ambiguous failure waits for the connection to return, refetches recent messages, and confirms the optimistic message in place instead of rolling it back — rolling it back lets the message queue re-send a prompt the engine is already running, producing two independent AI responses for one user message. Examples of global-store updates performed in `session-actions.ts`: diff --git a/packages/ui/src/sync/session-actions.test.ts b/packages/ui/src/sync/session-actions.test.ts index c9c4325a..d0cd9f16 100644 --- a/packages/ui/src/sync/session-actions.test.ts +++ b/packages/ui/src/sync/session-actions.test.ts @@ -1014,6 +1014,53 @@ describe("optimisticSend target directory", () => { expect(targetStore.getState().part[sentMessageID]?.[0]?.id).toBe("server-part") }) + // Relay tunnel aborts carry no HTTP status and no wording the text-matching + // heuristic recognizes. Without the transport tag they were classified as + // definite failures, the accepted prompt was rolled back, and the queue + // re-sent a message the engine was already answering (#2425). + test("confirms a tunnel-tagged transport failure that no text heuristic matches", async () => { + const targetStore = createStore({}) + const childStores = createChildStores([["/target/project", targetStore]]) + let optimisticRemove: OptimisticRemoveCall | null = null + let optimisticConfirm: OptimisticRemoveCall | null = null + let sentMessageID = "" + + const { markAmbiguousTransportFailure } = await import("@/lib/relay/transport-error") + const { optimisticSend, setActionRefs, setOptimisticRefs } = await import("./session-actions") + setActionRefs(mockSdk as unknown as OpencodeClient, childStores, () => "/target/project") + setOptimisticRefs( + () => {}, + (input) => { + optimisticRemove = input + }, + (input) => { + optimisticConfirm = input + }, + ) + + await optimisticSend({ + sessionId: "session-tunnel", + directory: "/target/project", + content: "hello", + providerID: "provider", + modelID: "model", + send: async (messageID) => { + sentMessageID = messageID + sessionMessagesResult = { + data: [{ + info: { id: messageID, role: "user", sessionID: "session-tunnel", time: { created: 1 } } as Message, + parts: [{ id: "server-part", type: "text", text: "hello" } as Part], + }], + } + throw markAmbiguousTransportFailure(new Error("stream aborted by host")) + }, + }) + + expect(optimisticRemove).toBe(null) + expect((optimisticConfirm as OptimisticRemoveCall | null)?.messageID).toBe(sentMessageID) + expect(targetStore.getState().message["session-tunnel"]?.[0]?.id).toBe(sentMessageID) + }) + test("rolls back an ambiguous send failure when recent messages do not contain the sent ID", async () => { const targetStore = createStore({}) const childStores = createChildStores([["/target/project", targetStore]]) diff --git a/packages/ui/src/sync/session-actions.ts b/packages/ui/src/sync/session-actions.ts index 5a6eab26..7e01309b 100644 --- a/packages/ui/src/sync/session-actions.ts +++ b/packages/ui/src/sync/session-actions.ts @@ -29,11 +29,21 @@ import { withContextObligatoryMessage, type ContextObligatoryMessage } from "@/l import { getImperativeSessionMessageLoader } from "./session-message-loader" import { cleanupPersistedSessionState } from "./session-deletion-cleanup" import { getRuntimeKey } from "@/lib/runtime-switch" +import { isAmbiguousTransportFailure } from "@/lib/relay/transport-error" const MESSAGE_REFETCH_LIMIT = 100 const SEND_CONFIRMATION_REFETCH_LIMIT = 30 -const SEND_CONFIRMATION_REFETCH_ATTEMPTS = 2 -const SEND_CONFIRMATION_REFETCH_RETRY_MS = 150 +// A relay-tunnel send fails when the tunnel drops, and the confirming refetch +// then has to travel over that same tunnel to answer "did my message land?". +// Two attempts 150ms apart always answered "no" on a remote connection, so an +// accepted prompt looked like a failed one and got re-sent — two AI responses +// for one user message. Wait for the connection to actually come back (an +// authoritative signal, not a blind sleep), then retry with backoff. A healthy +// connection skips the wait and answers on the first attempt. +const SEND_CONFIRMATION_REFETCH_ATTEMPTS = 3 +const SEND_CONFIRMATION_REFETCH_BASE_RETRY_MS = 250 +const SEND_CONFIRMATION_RECONNECT_TIMEOUT_MS = 3000 +const SEND_CONFIRMATION_RECONNECT_POLL_MS = 100 const MESSAGE_REFETCH_SKIP_PARTS = new Set(["patch", "step-start", "step-finish"]) const UNREVERT_REFETCH_ATTEMPTS = 3 const UNREVERT_REFETCH_RETRY_MS = 150 @@ -360,6 +370,13 @@ function getErrorStatus(error: unknown): number | null { } function isAmbiguousSendFailure(error: unknown): boolean { + // Authoritative first: the transport that lost the request says whether it + // had already been dispatched. The text matching below only covers direct + // fetch/HTTP failures, whose wording we do not control either — relay tunnel + // aborts ("stream aborted by host", "relay keepalive timeout", …) match none + // of those patterns and used to be misread as definite failures. + if (isAmbiguousTransportFailure(error)) return true + const status = getErrorStatus(error) if (status === 503 || status === 504 || status === 408) return true if (error instanceof TypeError) return true @@ -1255,8 +1272,15 @@ async function fetchRecentSendConfirmationRecords( messageID: string, directory?: string | null, ): Promise | null> { + // Bounded: a connection that never returns must still let the send fail + // rather than hang the composer. + const reconnectDeadline = Date.now() + SEND_CONFIRMATION_RECONNECT_TIMEOUT_MS + while (!useConfigStore.getState().isConnected && Date.now() < reconnectDeadline) { + await wait(SEND_CONFIRMATION_RECONNECT_POLL_MS) + } + for (let attempt = 0; attempt < SEND_CONFIRMATION_REFETCH_ATTEMPTS; attempt += 1) { - if (attempt > 0) await wait(SEND_CONFIRMATION_REFETCH_RETRY_MS) + if (attempt > 0) await wait(SEND_CONFIRMATION_REFETCH_BASE_RETRY_MS * 2 ** (attempt - 1)) try { const result = await sdk().session.messages({ sessionID: sessionId,