fix: treat lost relay sends as ambiguous instead of failed

A prompt whose response is lost after the request left the client may
already be running server-side. The relay tunnel reported those failures
as plain text errors ("stream aborted by host", "relay keepalive
timeout"), which matched none of the patterns in isAmbiguousSendFailure,
so an accepted prompt was rolled back and the message queue re-sent it —
two independent AI responses for one user message (#2425). Direct
connections never hit the path.

Transports now tag dispatched-but-unconfirmed failures and the classifier
reads the tag before falling back to status/text heuristics. Confirmation
waits for the connection to actually return (bounded) and retries with
backoff instead of two attempts 150ms apart over the just-broken tunnel.
This commit is contained in:
Bohdan Triapitsyn
2026-08-03 23:09:43 +03:00
parent e4fddabb19
commit fe38f7a56b
7 changed files with 161 additions and 9 deletions
+8 -1
View File
@@ -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 {
@@ -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 = <T extends Error>(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<string, unknown>)[AMBIGUOUS_TRANSPORT_FLAG] === true;
};
@@ -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);
+17 -5
View File
@@ -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));
}
})();
});