From c4bfa06b6dd6aa94702d56c51021654e527a062a Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 21 Sep 2026 07:25:57 +0900 Subject: [PATCH 1/3] fix(transports): answer an ambiguous resend the same way on every HTTP surface The pre-header replay refusal was correct where it was written and absent one wrapper away. The translated Chat surface preserved only the cyber-policy code and model_not_found, took the upstream code only when classifyError had produced none, and then attached the default Retry-After for a retryable 429, so a refusal to resend reached the client as an ordinary rate limit with an instruction to send the turn again. Carry the verdict as a property of the response every surface shares rather than something each one re-derives: retainReplayRefusal and carryReplayRefusal restate the in-process marker at each re-wrap, including the deferred-logging wrapper that silently dropped it. All four formatters now read the marker, or the code a body kept through an intermediate formatter, and never the status -- 429 is exactly what a refusal and a real rate limit have in common. Dropping Retry-After is necessary and not sufficient. The Stainless-generated clients (openai and anthropic, Python and Node) retry 408, 409, 429 and every 5xx from their own table and compute their own backoff when no wait is named, so the status stays 429 (Codex stops there, a 5xx invites four more sends) and every surface also emits x-should-retry: false, which those clients read first. --- src/bridge/errors.ts | 10 +++- src/lib/upstream-retry.ts | 58 +++++++++++++++++++++-- src/server/chat-completions.ts | 42 +++++++++++++++- src/server/chat-native.ts | 18 +++++-- src/server/relay.ts | 8 +++- src/server/responses/passthrough-error.ts | 35 ++++++++++---- 6 files changed, 149 insertions(+), 22 deletions(-) diff --git a/src/bridge/errors.ts b/src/bridge/errors.ts index aa270195792..e39aa1f4b45 100644 --- a/src/bridge/errors.ts +++ b/src/bridge/errors.ts @@ -1,9 +1,10 @@ import { + applyReplayRefusalClientHeaders, isNonReplayableUpstreamCode, isReplayRefusalCode, - markReplayRefusalResponse, markResponseNonReplayable, REPLAY_REFUSED_STATUS, + retainReplayRefusal, } from "../lib/upstream-retry"; import { adapterFailureFromMessage, @@ -45,6 +46,11 @@ export function formatErrorResponse( && retryAfter.length <= 128) { headers.set("Retry-After", retryAfter); } + // The refusal's client policy, restated here for the same reason its status is: this + // formatter is the last thing several adapter and combo paths touch before the client, + // and no wait of its own does not stop a client that retries every 429 by default. + const refusal = isReplayRefusalCode(error.code) && replayBlocked; + if (refusal) applyReplayRefusalClientHeaders(headers); const response = new Response(JSON.stringify({ error }), { status: finalStatus, headers, @@ -53,6 +59,6 @@ export function formatErrorResponse( // Re-wrapping is where the refusal loses its provenance: combo failure consumption parses // the JSON and builds a new Response, and the code alone does not tell a later quota // recorder that no upstream produced this status. Carry the narrower marker across too. - if (replayBlocked && isReplayRefusalCode(error.code)) markReplayRefusalResponse(response); + if (refusal) retainReplayRefusal(response); return response; } diff --git a/src/lib/upstream-retry.ts b/src/lib/upstream-retry.ts index 30f17a7b5d8..23d1f0c4ffd 100644 --- a/src/lib/upstream-retry.ts +++ b/src/lib/upstream-retry.ts @@ -98,6 +98,55 @@ export function isReplayRefusalCode(code: unknown): boolean { /** Client-facing status for {@link UPSTREAM_RESET_REPLAY_REFUSED_CODE}. */ export const REPLAY_REFUSED_STATUS = 429; +/** + * The header every surface attaches to a replay refusal, and its only accepted value. + * + * Dropping `Retry-After` is necessary and not sufficient. The Stainless-generated clients -- + * `openai` and `anthropic` for both Python and Node, which is what most callers of this proxy + * actually are -- decide from a status table (408, 409, 429 and every 5xx) and compute their own + * backoff when no wait is named, so a 429 with no header is still resent. `x-should-retry` is + * the one signal each of them reads BEFORE that table, and `"false"` is the exact string they + * compare against. + */ +export const REPLAY_REFUSAL_NO_RETRY_HEADER = "x-should-retry"; +export const REPLAY_REFUSAL_NO_RETRY_VALUE = "false"; + +/** Spreadable form for the surfaces that build their headers as an object literal. */ +export const REPLAY_REFUSAL_CLIENT_HEADERS: Readonly> = Object.freeze({ + [REPLAY_REFUSAL_NO_RETRY_HEADER]: REPLAY_REFUSAL_NO_RETRY_VALUE, +}); + +/** + * Apply the one client-facing retry policy a refusal carries: no wait, and no automatic resend. + * + * Kept as a single function rather than two rules each surface repeats, because the two halves + * are only correct together -- a surface that removed the wait but not the suppression still + * hands a retrying client a turn it may already have run. + */ +export function applyReplayRefusalClientHeaders(headers: Headers): void { + headers.delete("retry-after"); + headers.set(REPLAY_REFUSAL_NO_RETRY_HEADER, REPLAY_REFUSAL_NO_RETRY_VALUE); +} + +/** + * Mark a response that re-wraps a refusal as the same refusal. + * + * The verdict has to be a property of the result the surfaces pass around, because the thing it + * would otherwise be read from is the status, and 429 is exactly what a refusal and a real rate + * limit have in common. Every formatter between the helper that made the refusal and the client + * builds a new Response, so each of them restates the verdict rather than dropping it. + */ +export function retainReplayRefusal(response: T): T { + markResponseNonReplayable(response); + markReplayRefusalResponse(response); + return response; +} + +/** Carry the verdict from a response onto the one that replaces it. */ +export function carryReplayRefusal(source: Response, rewrapped: T): T { + return isReplayRefusalResponse(source) ? retainReplayRefusal(rewrapped) : rewrapped; +} + // 1 initial + 2 retries: the pool may hold more than one stale socket. const RESET_RETRY_MAX_ATTEMPTS = 3; const RESET_RETRY_BASE_DELAY_MS = 150; @@ -521,10 +570,11 @@ export function replayRefusalResponse(): Response { type: "upstream_error", code: UPSTREAM_RESET_REPLAY_REFUSED_CODE, message: "The upstream connection closed before a response was received. The request may already have been processed; automatic replay was stopped.", - } }), { status: REPLAY_REFUSED_STATUS, headers: { "content-type": "application/json" } }); - markResponseNonReplayable(response); - markReplayRefusalResponse(response); - return response; + } }), { + status: REPLAY_REFUSED_STATUS, + headers: { "content-type": "application/json", ...REPLAY_REFUSAL_CLIENT_HEADERS }, + }); + return retainReplayRefusal(response); } /** diff --git a/src/server/chat-completions.ts b/src/server/chat-completions.ts index 4f4209d11c0..3cb6c9a3798 100644 --- a/src/server/chat-completions.ts +++ b/src/server/chat-completions.ts @@ -22,6 +22,15 @@ import { import { classifyError, cyberPolicyErrorType, CYBER_POLICY_ERROR_CODE, isCyberPolicyCode } from "../lib/errors"; import { redactSecretString } from "../lib/redact"; import { resolveClientRetryAfter } from "../lib/retry-after"; +import { + applyReplayRefusalClientHeaders, + isReplayRefusalCode, + isReplayRefusalResponse, + REPLAY_REFUSAL_CLIENT_HEADERS, + REPLAY_REFUSED_STATUS, + retainReplayRefusal, + UPSTREAM_RESET_REPLAY_REFUSED_CODE, +} from "../lib/upstream-retry"; import { estimateTokens } from "../lib/token-estimate"; import { captureRouteStaticPolicy, NoEligiblePolicyCandidateError, UnknownRoutingPolicyError, routeModel } from "../router"; import { evidenceFromBody } from "../routing/request-evidence"; @@ -419,9 +428,19 @@ async function handleChatCompletionsWithBudget( : "invalid_request_error"), message, ); + // The same verdict the native Chat surface reads, from the same two places: the response + // this wrapper still holds, and the code a body kept through an intermediate formatter. + // Not the status -- a refusal and a real rate limit are both 429, which is the whole + // reason this surface used to report one as the other. + const replayRefusal = isReplayRefusalResponse(upstream) || isReplayRefusalCode(upstreamCode); if (isCyberPolicyCode(upstreamCode) || classified.code === CYBER_POLICY_ERROR_CODE) { classified.code = CYBER_POLICY_ERROR_CODE; classified.type = cyberPolicyErrorType(upstreamType); + } else if (replayRefusal) { + // 429 classifies as a rate limit, which already carries a code, so the empty-code branch + // below could never restore this one -- the translated client was told the provider + // throttled the turn, and handed a two-second wait to send it again. + classified.code = UPSTREAM_RESET_REPLAY_REFUSED_CODE; } else if (upstreamCode === "model_not_found") { // Structured model_not_found must win over classifyError's generic remaps. classified.code = "model_not_found"; @@ -429,8 +448,10 @@ async function handleChatCompletionsWithBudget( } else if (upstreamCode !== undefined && upstreamCode !== null && classified.code == null) { classified.code = upstreamCode; } - const status = isCyberPolicyCode(classified.code) ? 400 : upstream.status; - const retryAfter = isCyberPolicyCode(classified.code) + const status = isCyberPolicyCode(classified.code) ? 400 + : replayRefusal ? REPLAY_REFUSED_STATUS + : upstream.status; + const retryAfter = isCyberPolicyCode(classified.code) || replayRefusal ? undefined : resolveClientRetryAfter({ status: upstream.status, @@ -449,9 +470,12 @@ async function handleChatCompletionsWithBudget( headers: { "Content-Type": "application/json", ...(retryAfter ? { "Retry-After": retryAfter } : {}), + ...(replayRefusal ? REPLAY_REFUSAL_CLIENT_HEADERS : {}), }, }); + if (replayRefusal) retainReplayRefusal(rewritten); return logIds + // Deferred logging re-wraps this response and carries the verdict with it. ? responseWithDeferredRequestLog(rewritten, logIds.requestId, logIds.start, logCtx) : rewritten; } @@ -518,11 +542,25 @@ async function handleChatCompletionsWithBudget( } else if (isCyberPolicyCode(error?.code) || classified.code === CYBER_POLICY_ERROR_CODE) { classified.code = CYBER_POLICY_ERROR_CODE; classified.type = cyberPolicyErrorType(error?.type); + } else if (isReplayRefusalCode(error?.code)) { + // The refusal can also arrive as a failed Responses envelope rather than a non-2xx. + // Reporting that as the 502 below would invite the four resends the refusal prevents. + classified.code = UPSTREAM_RESET_REPLAY_REFUSED_CODE; } else if (error?.code === "model_not_found") { // Same deliberate preserve as the non-OK path: structured code beats generic classify. classified.code = "model_not_found"; classified.type = "invalid_request_error"; } + if (isReplayRefusalCode(classified.code)) { + const refusal = chatCompletionsErrorResponse( + REPLAY_REFUSED_STATUS, message, classified.type, classified.code, + ); + const headers = new Headers(refusal.headers); + applyReplayRefusalClientHeaders(headers); + return finishJson(retainReplayRefusal( + new Response(refusal.body, { status: refusal.status, headers }), + )); + } return finishJson(chatCompletionsErrorResponse( classified.code === "translation_buffer_limit" ? 502 diff --git a/src/server/chat-native.ts b/src/server/chat-native.ts index c77b6fda592..71ef323ebf5 100644 --- a/src/server/chat-native.ts +++ b/src/server/chat-native.ts @@ -29,6 +29,9 @@ import { isReplayRefusalCode, isReplayRefusalResponse, prepareSameTarget429Wait, + REPLAY_REFUSAL_CLIENT_HEADERS, + REPLAY_REFUSED_STATUS, + retainReplayRefusal, type UpstreamSendRecovery, UPSTREAM_RESET_REPLAY_REFUSED_CODE, } from "../lib/upstream-retry"; @@ -497,10 +500,13 @@ export async function handleNativeChatCompletions(options: HandleNativeChatOptio : response.status >= 500 ? "server_error" : "invalid_request_error"), clientMessage, ); + // The verdict is read once, from the response that carries it and from the code a + // re-wrapped body kept -- never from the status, which a real rate limit shares. + const replayRefusal = isReplayRefusalResponse(response) || isReplayRefusalCode(upstreamCode); if (isCyberPolicyCode(upstreamCode) || classified.code === CYBER_POLICY_ERROR_CODE) { classified.code = CYBER_POLICY_ERROR_CODE; classified.type = cyberPolicyErrorType(upstreamType); - } else if (isReplayRefusalResponse(response) || isReplayRefusalCode(upstreamCode)) { + } else if (replayRefusal) { // 429 classifies as a rate limit and a rate limit already carries a code, so the branch // below -- which only fills an EMPTY code -- could never restore this one. Without it the // client is told the provider throttled the turn, when what happened is that this proxy @@ -512,11 +518,13 @@ export async function handleNativeChatCompletions(options: HandleNativeChatOptio } else if (upstreamCode !== undefined && upstreamCode !== null && classified.code == null) { classified.code = upstreamCode; } - const status = isCyberPolicyCode(classified.code) ? 400 : response.status; + const status = isCyberPolicyCode(classified.code) ? 400 + : replayRefusal ? REPLAY_REFUSED_STATUS + : response.status; // A refusal this proxy made has no wait to report. Synthesizing one here would hand the // client the default two-second retry for a rate limit that never happened, which is the // duplicate send the refusal exists to prevent. - const retryAfter = isCyberPolicyCode(classified.code) || isReplayRefusalCode(classified.code) + const retryAfter = isCyberPolicyCode(classified.code) || replayRefusal ? undefined : resolveClientRetryAfter({ status: response.status, @@ -524,13 +532,15 @@ export async function handleNativeChatCompletions(options: HandleNativeChatOptio upstreamRetryAfter: response.headers.get("retry-after"), }); finishLog(status, classified.message); - return new Response(JSON.stringify(chatCompletionsErrorBody(status, classified.message, classified.type, classified.code)), { + const rewritten = new Response(JSON.stringify(chatCompletionsErrorBody(status, classified.message, classified.type, classified.code)), { status, headers: { "Content-Type": "application/json", ...(retryAfter ? { "Retry-After": retryAfter } : {}), + ...(replayRefusal ? REPLAY_REFUSAL_CLIENT_HEADERS : {}), }, }); + return replayRefusal ? retainReplayRefusal(rewritten) : rewritten; } const contentType = response.headers.get("content-type")?.toLowerCase() ?? ""; diff --git a/src/server/relay.ts b/src/server/relay.ts index 4e2f8bbd67a..1bbb9fd72c7 100644 --- a/src/server/relay.ts +++ b/src/server/relay.ts @@ -12,6 +12,7 @@ import { } from "../lib/errors"; import { redactSecretString } from "../lib/redact"; import { isTranslatorBudgetExceededError } from "../lib/translator-budget"; +import { carryReplayRefusal } from "../lib/upstream-retry"; import { isUsageDebugEnabled } from "../usage/debug"; import { addRequestLog, @@ -833,11 +834,14 @@ export function responseWithDeferredRequestLog( }, addLog); }, }); - return new Response(body, { + // Logging re-wraps the response, and an in-process verdict does not survive a re-wrap on + // its own. A replay refusal that lost it here would read to a later quota recorder or + // Retry-After synthesizer as a 429 some upstream produced. + return carryReplayRefusal(response, new Response(body, { status: response.status, statusText: response.statusText, headers: response.headers, - }); + })); } if (isUsageDebugEnabled() && logCtx.usageDebugBodyKind === undefined) { logCtx.usageDebugBodyKind = response.body ? "other" : "none"; diff --git a/src/server/responses/passthrough-error.ts b/src/server/responses/passthrough-error.ts index eee33c02a96..7a993255ee3 100644 --- a/src/server/responses/passthrough-error.ts +++ b/src/server/responses/passthrough-error.ts @@ -1,6 +1,11 @@ import { formatErrorResponse } from "../../bridge"; import { isCyberPolicyCode, isCyberPolicyMessage } from "../../lib/errors"; -import { isReplayRefusalCode, UPSTREAM_RESET_REPLAY_REFUSED_CODE } from "../../lib/upstream-retry"; +import { + applyReplayRefusalClientHeaders, + isReplayRefusalCode, + retainReplayRefusal, + UPSTREAM_RESET_REPLAY_REFUSED_CODE, +} from "../../lib/upstream-retry"; import { resolveClientRetryAfter, validateClientRetryAfterHeader, @@ -62,6 +67,10 @@ function isReplayRefusalBody(body: string): boolean { * - a replay refusal this proxy wrote gets none and keeps none: the whole point of the * refusal is that the turn may already be running, and the synthetic default for a * retryable 429 is a direct instruction to the client to send it a second time + * + * A refusal also leaves with the shared no-retry header and the in-process marker, so the + * verdict survives this re-wrap as a property of the response rather than as a status a later + * reader would have to guess from. */ export function formatPassthroughUpstreamError( status: number, @@ -84,11 +93,10 @@ export function formatPassthroughUpstreamError( const upstreamRetryAfter = options?.headers?.get("retry-after")?.trim() || undefined; const originalValid = validateClientRetryAfterHeader(upstreamRetryAfter, now); const cyberPolicyFailure = isCyberPolicyBody(trimmed); + const replayRefusal = options?.replayRefusal === true || isReplayRefusalBody(trimmed); // Two different reasons to answer with no wait at all, handled the same way: a hard policy // block will not become servable, and a refusal we made was never a rate limit. - const suppressRetryAfter = cyberPolicyFailure - || options?.replayRefusal === true - || isReplayRefusalBody(trimmed); + const suppressRetryAfter = cyberPolicyFailure || replayRefusal; const resolved = suppressRetryAfter ? undefined : resolveClientRetryAfter({ @@ -105,7 +113,9 @@ export function formatPassthroughUpstreamError( && upstreamRetryAfter !== undefined && originalValid === undefined); - if (!needsSet && !needsDelete) { + // A refusal always takes the rewriting path: it has a header to add even when the + // upstream named no wait for it to remove. + if (!needsSet && !needsDelete && !replayRefusal) { return new Response(bodyText, { status, ...(options?.statusText ? { statusText: options.statusText } : {}), @@ -118,21 +128,30 @@ export function formatPassthroughUpstreamError( : new Headers({ "Content-Type": "application/json" }); if (needsSet) headers.set("Retry-After", resolved!); else headers.delete("Retry-After"); - return new Response(bodyText, { + if (replayRefusal) applyReplayRefusalClientHeaders(headers); + const rewritten = new Response(bodyText, { status, ...(options?.statusText ? { statusText: options.statusText } : {}), headers, }); + return replayRefusal ? retainReplayRefusal(rewritten) : rewritten; } const response = formatErrorResponse( status, "upstream_error", `Provider error ${status}: (empty body)`, - resolved !== undefined ? { retryAfter: resolved } : undefined, + // Provenance is all that is left when the bounded read returned nothing display-safe, and + // it is enough: the formatter allowlists this code, restates the refusal status and marks + // the response, so an unreadable refusal reaches the client as the same refusal. + replayRefusal + ? { code: UPSTREAM_RESET_REPLAY_REFUSED_CODE } + : resolved !== undefined ? { retryAfter: resolved } : undefined, ); const headers = new Headers(response.headers); headers.set("Content-Type", "application/json"); if (resolved !== undefined) headers.set("Retry-After", resolved); - return new Response(response.body, { status: response.status, headers }); + if (replayRefusal) applyReplayRefusalClientHeaders(headers); + const wrapped = new Response(response.body, { status: response.status, headers }); + return replayRefusal ? retainReplayRefusal(wrapped) : wrapped; } From a2a8331259b532a72ac7613279011758bbee0594 Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 21 Sep 2026 07:26:06 +0900 Subject: [PATCH 2/3] test(server): count the upstream sends a retrying client causes The acceptance unit for this refusal is not the shape of one response: it is how many times the turn physically reaches upstream when a client with retries enabled is the one deciding. A single fetch cannot see that, because the proxy can answer correctly and the duplicate inference still happens. The case runs the proxy over a socket, counts sends at the upstream boundary and drives all three surfaces with a client that implements the published SDK rule (an explicit x-should-retry first, then 408/409/429/5xx). Its header literals are deliberate: the double stands in for the third party and has to keep believing what those clients believe. A rate-limit control shows the same client resending, so "one send" is a property of the answer rather than of the double. --- scripts/test-layout/layout.json | 1 + tests/fixtures/test-layout-expected.json | 1 + tests/server/replay-refusal-parity.test.ts | 194 +++++++++++++++++++++ 3 files changed, 196 insertions(+) create mode 100644 tests/server/replay-refusal-parity.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index b0a1b3b81ea..74ac8238aea 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1226,6 +1226,7 @@ "remote-workspace-protocol.test.ts": "clients", "remote-workspace-rpc-framing.test.ts": "clients", "remove-tree-helper.test.ts": "lib", + "replay-refusal-parity.test.ts": "server", "repo-hygiene.test.ts": "ci-workflows", "request-decompress.test.ts": "usage", "request-evidence.test.ts": "usage", diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index d5413609fb8..99ab9033496 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1052,6 +1052,7 @@ "remote-workspace-protocol.test.ts": "clients", "remote-workspace-rpc-framing.test.ts": "clients", "remove-tree-helper.test.ts": "lib", + "replay-refusal-parity.test.ts": "server", "repo-hygiene.test.ts": "ci-workflows", "request-decompress.test.ts": "usage", "request-evidence.test.ts": "usage", diff --git a/tests/server/replay-refusal-parity.test.ts b/tests/server/replay-refusal-parity.test.ts new file mode 100644 index 00000000000..9b8af2cf14c --- /dev/null +++ b/tests/server/replay-refusal-parity.test.ts @@ -0,0 +1,194 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { saveConfig } from "../../src/config"; +import { startServer } from "../../src/server"; +import { + REPLAY_REFUSAL_NO_RETRY_HEADER, + REPLAY_REFUSAL_NO_RETRY_VALUE, + REPLAY_REFUSED_STATUS, + UPSTREAM_RESET_REPLAY_REFUSED_CODE, +} from "../../src/lib/upstream-retry"; +import { installIsolatedCodexHome, type IsolatedCodexHome } from "../helpers/isolated-codex-home"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; +import type { OcxConfig } from "../../src/types"; + +/** + * The acceptance unit for the ambiguous-resend refusal is not the shape of one response: it is + * how many times the turn physically reaches upstream when a real client is allowed to retry. + * A single `fetch` cannot see that, because a client with retries enabled is the thing that + * resends -- the proxy answered correctly and the duplicate inference happened anyway. + * + * So these cases run the proxy over a real socket, count the sends at the upstream boundary, + * and drive it with a client that retries the way the published SDKs do. The three surfaces + * are asserted against one expectation because a client cannot tell them apart: it sent one + * turn and the turn may already have executed, whichever endpoint carried it. + */ +const UPSTREAM_HOST = "replay-refusal-parity.example.test"; +const originalFetch = globalThis.fetch; +let testDir = ""; +let previousHome: string | undefined; +let isolatedCodexHome: IsolatedCodexHome | null = null; + +beforeEach(() => { + previousHome = process.env.OPENCODEX_HOME; + isolatedCodexHome = installIsolatedCodexHome("ocx-replay-refusal-"); + testDir = mkdtempSync(join(tmpdir(), "ocx-replay-refusal-")); + process.env.OPENCODEX_HOME = testDir; + globalThis.fetch = originalFetch; +}); + +afterEach(() => { + globalThis.fetch = originalFetch; + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + isolatedCodexHome?.restore(); + isolatedCodexHome = null; + if (testDir) removeTreeWithRetry(testDir); +}); + +/** + * The retry rule of the official clients, written as they write it. + * + * The header name and its two accepted values are deliberately literals here rather than the + * constants this repository exports. This function stands in for the third party: it has to + * keep believing what `openai` and `anthropic` believe -- an explicit verdict first, then the + * 408/409/429/5xx table -- even if our own constant were changed to something no client reads. + */ +function sdkWouldRetry(response: Response): boolean { + const verdict = response.headers.get("x-should-retry"); + if (verdict === "true") return true; + if (verdict === "false") return false; + return response.status === 408 || response.status === 409 + || response.status === 429 || response.status >= 500; +} + +/** One logical request through a client whose retries are enabled. */ +async function sendWithClientRetries( + url: URL, + body: Record, + maxRetries = 2, +): Promise<{ response: Response; attempts: number; json: { error?: { code?: string } } }> { + let attempts = 0; + for (;;) { + attempts += 1; + const response = await originalFetch(url, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); + if (attempts > maxRetries || !sdkWouldRetry(response)) { + return { response, attempts, json: await response.json() as { error?: { code?: string } } }; + } + // Release the body before the next attempt, as the SDKs do. + await response.text(); + } +} + +/** + * Count physical upstream sends and answer each one as the fixture dictates. Everything not + * addressed to the fixture host -- the client's own calls included -- keeps the real fetch. + */ +function countingUpstream(answer: () => Response): () => number { + let sends = 0; + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input + : input instanceof URL ? input.href + : input.url; + if (!url.includes(UPSTREAM_HOST)) return originalFetch(input as RequestInfo, init); + sends += 1; + return answer(); + }) as typeof fetch; + return () => sends; +} + +/** A half-closed pooled socket: the request has left, and nothing comes back. */ +function preHeaderReset(): never { + throw Object.assign( + new Error("The socket connection was closed unexpectedly."), + { code: "ECONNRESET" }, + ); +} + +function parityConfig(): OcxConfig { + const provider = (apiKey: string, adapter: string) => ({ + adapter, + baseUrl: `https://${UPSTREAM_HOST}/v1`, + authMode: "key", + apiKey, + models: ["model"], + }); + return { + port: 0, + defaultProvider: "native", + providers: { + // Native Chat keeps the caller on the Chat wire; the bridged row translates through + // Responses and back, which is the surface that used to lose the refusal. + native: provider("sk-native", "openai-chat"), + bridged: provider("sk-bridged", "openai-responses"), + }, + } as unknown as OcxConfig; +} + +const CHAT_TURN = { messages: [{ role: "user", content: "ping" }] }; +const RESPONSES_TURN = { input: [{ role: "user", content: [{ type: "input_text", text: "ping" }] }] }; + +test("every HTTP surface answers an ambiguous reset with one send and no client resend", async () => { + saveConfig(parityConfig()); + const sends = countingUpstream(preHeaderReset); + const server = startServer(0); + const surfaces = [ + { name: "native Chat", path: "/v1/chat/completions", body: { model: "native/model", ...CHAT_TURN } }, + { name: "translated Chat", path: "/v1/chat/completions", body: { model: "bridged/model", ...CHAT_TURN } }, + { name: "Responses", path: "/v1/responses", body: { model: "bridged/model", ...RESPONSES_TURN } }, + ]; + try { + for (const surface of surfaces) { + const before = sends(); + const { response, attempts, json } = await sendWithClientRetries( + new URL(surface.path, server.url), + surface.body, + ); + // The number this refusal exists to hold at one, per logical request. + expect({ surface: surface.name, sends: sends() - before, attempts }) + .toEqual({ surface: surface.name, sends: 1, attempts: 1 }); + expect({ surface: surface.name, status: response.status, code: json.error?.code }).toEqual({ + surface: surface.name, + status: REPLAY_REFUSED_STATUS, + code: UPSTREAM_RESET_REPLAY_REFUSED_CODE, + }); + // No wait to honour, and no automatic resend of a turn that may already have run. + expect(response.headers.get("Retry-After")).toBeNull(); + expect(response.headers.get(REPLAY_REFUSAL_NO_RETRY_HEADER)).toBe(REPLAY_REFUSAL_NO_RETRY_VALUE); + } + } finally { + await server.stop(true); + } +}); + +/** + * The control that keeps the assertion above honest. A client double that never resends would + * pin "one send" for any answer at all, so the same client has to be shown resending a real + * rate limit -- the answer a refusal was indistinguishable from on the translated surface. + */ +test("the same client still resends an ordinary upstream rate limit", async () => { + saveConfig(parityConfig()); + const sends = countingUpstream(() => new Response( + JSON.stringify({ error: { message: "Too many requests", type: "rate_limit_error" } }), + { status: 429, headers: { "content-type": "application/json", "retry-after": "0" } }, + )); + const server = startServer(0); + try { + const { response, attempts } = await sendWithClientRetries( + new URL("/v1/chat/completions", server.url), + { model: "native/model", ...CHAT_TURN }, + ); + expect(response.status).toBe(429); + expect(response.headers.get(REPLAY_REFUSAL_NO_RETRY_HEADER)).toBeNull(); + expect(attempts).toBe(3); + expect(sends()).toBeGreaterThan(1); + } finally { + await server.stop(true); + } +}); From a235bcfbd175eb6dcf163a75295fb1d4e77d757b Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 21 Sep 2026 07:26:06 +0900 Subject: [PATCH 3/3] docs(transports): record the one replay-refusal client contract Both documents stated the refusal carries no Retry-After and stopped there, which is the half of the policy that does not survive contact with a retrying client. Name the fourth writer, the property that now travels with the response, and the suppression header, and point the invariant at the count that holds it. --- .../docs/reference/configuration/server.md | 12 ++++++--- structure/transports/responses.md | 27 +++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/docs-site/src/content/docs/reference/configuration/server.md b/docs-site/src/content/docs/reference/configuration/server.md index 78df10f7513..92aae00980f 100644 --- a/docs-site/src/content/docs/reference/configuration/server.md +++ b/docs-site/src/content/docs/reference/configuration/server.md @@ -64,10 +64,14 @@ arrives, the proxy cannot tell whether the model already processed the request, to send it again and answers HTTP 429 with `upstream_reset_replay_refused`. The status is deliberate: a 5xx here is an instruction to most clients, including Codex, to send the whole turn again, which is the duplicate the refusal exists to prevent. No `Retry-After` is -attached, and the proxy performs no key rotation, account failover or same-target replay on -it, nor does it record the refusal as rate-limit or quota evidence against the credential it -was holding. Tool-call side requests such as vision and web search are replayed normally, because -repeating them cannot duplicate a turn. +attached, and the response carries `x-should-retry: false`, which the official OpenAI and +Anthropic SDKs read before their own status rules — without it those clients retry a 429 on +their own schedule and resend the turn anyway. The answer is identical on `/v1/responses` and +on `/v1/chat/completions`, whether the request is forwarded natively or translated. The proxy +performs no key rotation, account failover or same-target replay on it, nor does it record the +refusal as rate-limit or quota evidence against the credential it was holding. Tool-call side +requests such as vision and web search are replayed normally, because repeating them cannot +duplicate a turn. A native Responses provider can opt into replacing that send with [`retryOnReset`](providers.md#provider-entries-ocxproviderconfig). The same grant covers the diff --git a/structure/transports/responses.md b/structure/transports/responses.md index 3b40dd43eda..6751211b966 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -1338,6 +1338,33 @@ the default fires on; and `src/server/chat-native.ts` restores the code its own 429 maps to `rate_limit_error`, which already carries a code, so the branch that copies an upstream code could never reach it — and suppresses the same synthetic wait. +**The verdict is a property of the response, and every surface states it the same way.** The +translated Chat wrapper in `src/server/chat-completions.ts` was the fourth writer and the one +that had none of this: it preserved the cyber-policy code and `model_not_found`, took the +upstream code only when `classifyError` had produced none, and then attached the retryable-429 +default. A refusal therefore left the Chat bridge as an ordinary rate limit carrying an +instruction to send the turn again. It now reads the same two things the native surface reads — +`isReplayRefusalResponse` on the response it still holds, and `isReplayRefusalCode` on a body +that came through an intermediate formatter — and never the status, which a refusal and a real +rate limit share. The failed-envelope path in the same function restates it too, so a refusal +arriving as `status: "failed"` is not reported as the 502 a Codex client retries four times. +Because a re-wrap is where the in-process marker is lost, `retainReplayRefusal` and +`carryReplayRefusal` in `src/lib/upstream-retry.ts` are what each formatter calls: +`src/bridge/errors.ts`, `src/server/responses/passthrough-error.ts`, both Chat wrappers, and the +deferred-logging re-wrap in `src/server/relay.ts`. + +**Dropping `Retry-After` is necessary and not sufficient.** The status stays 429 because Codex +stops there and a 5xx invites four more sends, but the Stainless-generated clients — `openai` +and `anthropic`, Python and Node — decide from a status table that includes 429 and compute +their own backoff when no wait is named, so a bare 429 is still resent by most callers of this +proxy. Every surface therefore also emits `x-should-retry: false`, the one signal those clients +read before that table. The refusal is the only code that gets it: the WebSocket post-send +verdicts are genuine upstream observations and keep their existing 502/504 contract. The +acceptance evidence is a count, not a shape — `tests/server/replay-refusal-parity.test.ts` runs +the proxy over a socket, drives all three surfaces with a client that implements the published +SDK rule, and asserts one physical upstream send per logical request, with a rate-limit control +that shows the same client resending. + The existing provider HTTP-status policy and the shared physical-send budget remain independent: zero refuses dispatch, invalid counts fail, and a stopped send is counted once. `src/bridge/errors.ts` retains only the allowlisted non-replayable transport codes,