diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index c7535203297..dda1c8fffe1 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -242,6 +242,11 @@ mode, or base URL during search or provider pacing ends the turn with a bridge e provider request is sent. Changing away and back also ends that continuation. Start a new turn to use the new selection. Selection changes before the first provider send retain normal reselection. +A bridged search result is shown to the provider again on the conversation's next turn only for +the same caller, conversation, provider, model and selected key. The caller is identified by the +opencodex API key it presents, so a client that sends no opencodex API key gets no such replay: +its earlier search cells reach the provider unchanged, as they do for a provider without the bridge. + Custom-model `reasoningEfforts` normally override discovered provider metadata. The bounded exception is an explicit custom row whose model id has pinned native Codex capabilities, including Astra or Daybreak on an arbitrary gateway: its advertised list is intersected with diff --git a/src/adapters/openai-responses/passthrough.ts b/src/adapters/openai-responses/passthrough.ts index cadcb2a38c2..0bef9c1cf06 100644 --- a/src/adapters/openai-responses/passthrough.ts +++ b/src/adapters/openai-responses/passthrough.ts @@ -329,11 +329,11 @@ export function createResponsesPassthroughAdapter(provider: OcxProviderConfig): outBody = stripUnsupportedReasoningSummaryDelivery(outBody, parsed.modelId); // #4587: on a bridged provider, hand the destination back the search call and result the // proxy executed on its behalf, in place of the hosted cell the caller replays. Scoped to - // this destination and recorded by the bridge itself, so a provider without the opt-in - // computes no identity and keeps the body reference it already had. This runs before the - // query backfill below because a restored cell is no longer a web_search_call to repair. + // its exact conversation and serving identity and recorded by the bridge itself, so a + // provider without the opt-in computes no identity and keeps the body reference it already + // had. This runs before query backfill because a restored cell is no longer one to repair. if (provider.webSearchBridge?.enabled === true) { - outBody = restoreBridgedWebSearchCalls(outBody, bridgeSearchReplayScope(provider.baseUrl)); + outBody = restoreBridgedWebSearchCalls(outBody, bridgeSearchReplayScope(parsed._reasoningReplayScope)); } // Repair stored history from before the bridge emitted both keys, in either // direction: a conversation that already recorded a web_search_call replays it diff --git a/src/adapters/openai-responses/tool-output-recovery.ts b/src/adapters/openai-responses/tool-output-recovery.ts index bd05234cbbd..531b0092c66 100644 --- a/src/adapters/openai-responses/tool-output-recovery.ts +++ b/src/adapters/openai-responses/tool-output-recovery.ts @@ -289,9 +289,11 @@ export function backfillWebSearchQueries(body: unknown): unknown { * - It never restores a call id the body already carries. If the history somehow holds that * `function_call` too, emitting a second one would be a duplicate the upstream must reject. * - * Entries are scoped to the upstream destination, so a history replayed against a different - * provider cannot resurrect a call that provider never made. Callers pass `undefined` for any - * provider without the bridge armed, and the common path then returns the original reference. + * Entries are scoped to the caller principal, conversation and exact serving identity, so a + * history replayed by another caller or against a different provider, model, destination or + * credential cannot resurrect a call that pairing never made. Callers pass `undefined` for any + * provider without the bridge armed and for a caller with no principal, and the common path then + * returns the original reference. */ export function restoreBridgedWebSearchCalls(body: unknown, destinationScope: string | undefined): unknown { if (destinationScope === undefined) return body; diff --git a/src/lib/errors.ts b/src/lib/errors.ts index 18b27d34827..58693c5978f 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -258,12 +258,14 @@ export function isClientClosedMessage(text: string): boolean { /** * Ambiguous-reset refusal wording owned by this proxy (src/lib/upstream-retry.ts): - * the upstream connection closed before any response arrived, so the request may - * already have been processed and automatic replay was stopped. Matched narrowly - * so a provider-sent message is never relabeled by it. + * the upstream exchange did not complete reliably, so the request may already have + * been processed and automatic replay was stopped. Matched narrowly so a + * provider-sent message is never relabeled by it. */ export function isUpstreamResetReplayRefusedMessage(text: string): boolean { - return text.toLowerCase().includes("connection closed before a response was received"); + return text.toLowerCase().includes( + "the upstream exchange did not complete reliably. the request may already have been processed", + ); } export function classifyError(status: number, type: string, message: string): OcxErrorPayload { diff --git a/src/lib/upstream-retry.ts b/src/lib/upstream-retry.ts index 23d1f0c4ffd..3419b8ffb8a 100644 --- a/src/lib/upstream-retry.ts +++ b/src/lib/upstream-retry.ts @@ -422,6 +422,24 @@ export function cancelResponseBodyBestEffort(res: Response): void { } } +/** + * Whether an answer to a spent operator replacement would invite yet another send. + * + * Once the one replacement a request may spend has gone out, the first send may already have run + * the turn, so nothing this exchange returns may cause a third send. Two parties would send again: + * the client, whose retry table covers 408, 409, 429 and every 5xx (the Codex client retries 5xx + * whatever the headers say; see {@link REPLAY_REFUSED_STATUS}), and this proxy, whose credential + * and quota recovery resends on 401 (token refresh, key and pool rotation) and on 402/429 + * (account rotation). A client that follows a 307 or 308 sends the same POST body again, and a 413 + * is answered as a context overflow the client compacts and resends, so those belong here too. + * {@link isTransientUpstreamStatus} is only the gateway subset of that set: 429 and 529 escaped + * it. These statuses settle as the refusal instead. + */ +function invitesResendAfterReplacement(status: number): boolean { + return status === 401 || status === 402 || status === 408 || status === 409 || status === 429 + || status === 307 || status === 308 || status === 413 || status >= 500; +} + export async function fetchWithAttemptDeadline( url: string, init: RequestInit, @@ -569,7 +587,7 @@ export function replayRefusalResponse(): Response { const response = new Response(JSON.stringify({ error: { 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.", + message: "The upstream exchange did not complete reliably. The request may already have been processed; automatic replay was stopped.", } }), { status: REPLAY_REFUSED_STATUS, headers: { "content-type": "application/json", ...REPLAY_REFUSAL_CLIENT_HEADERS }, @@ -594,9 +612,9 @@ export async function fetchWithResetRetry( if (attempts === 0) throw new SendBudgetExhaustedError(opts.label); let lastError: unknown; let sawReset = false; - // True once this leg has spent the request's operator allowance. From that point the leg can - // only settle as the refusal: a second send of a possibly-executed turn is already out, and - // handing the client anything it would retry compounds it. + // True once this leg has spent the request's operator allowance. From that point the leg + // settles as the refusal or an unambiguous answer: a second send of a possibly-executed turn + // is already out, and handing the client anything it would retry compounds it. let spentOperatorReplacement = false; for (let attempt = 0; attempt < attempts; attempt++) { if (opts.abortSignal?.aborted) throw abortError(opts.abortSignal); @@ -605,7 +623,19 @@ export async function fetchWithResetRetry( // rethrow, abort), so a per-send report is the only shape that is correct on all of them. opts.onSendsConsumed?.(1); try { - return await doFetch(attempt === 0 ? firstRecovery : "connection-reset"); + const response = await doFetch(attempt === 0 ? firstRecovery : "connection-reset"); + if (spentOperatorReplacement && !response.ok) { + if (invitesResendAfterReplacement(response.status)) { + cancelResponseBodyBestEffort(response); + return replayRefusalResponse(); + } + // Any other answer keeps its real status: no client retries it, and the caller needs the + // evidence (a 400 names the request defect). The marker still stops this process from + // using it as a recovery trigger, such as the opaque-blob rebuild of a 400 or a combo hop + // on a context overflow, because each of those checks it before sending again. + markResponseNonReplayable(response); + } + return response; } catch (err) { if (opts.abortSignal?.aborted) throw err; if (!isConnectionResetError(err)) { diff --git a/src/responses/bridge-search-replay-cache.ts b/src/responses/bridge-search-replay-cache.ts index 6de24cdce39..07213682c33 100644 --- a/src/responses/bridge-search-replay-cache.ts +++ b/src/responses/bridge-search-replay-cache.ts @@ -14,9 +14,9 @@ * what `appendBridgeSearchTurn` would have written onto a continuation leg, so a replayed turn * and a continued turn show the destination the same conversation. * - * Scope. Entries are keyed by the upstream destination in addition to the cell id. The cell id is - * a v4 UUID minted here, so it cannot collide across conversations, but an unscoped key would let - * a history replayed against a DIFFERENT provider resurrect a call that provider never made. + * Scope. Entries are keyed by the exact conversation and serving identity in addition to the cell + * id. The cell id is a v4 UUID minted here, but possession of a client-visible id is not authority + * to recover result text under another provider, model, destination, or credential. * * Bounds and privacy. Result text is web content the caller already received, but it is still * request-derived data: it lives in memory only, is never logged, serialized, or exported, and is @@ -26,7 +26,7 @@ * alone. Neither re-running the search nor inventing a result is an acceptable recovery. */ -import { reasoningReplayDestinationIdentity } from "./reasoning-replay-cache"; +import type { OcxReasoningReplayScopeRef } from "../types"; const MAX_ENTRIES = 64; const MAX_TOTAL_BYTES = 512 * 1024; @@ -58,14 +58,24 @@ let clockForTests: (() => number) | null = null; const now = (): number => clockForTests?.() ?? Date.now(); /** - * Identify the upstream destination a bridged search belongs to. + * Identify the exact conversation and upstream binding a bridged search belongs to. * - * Reuses the salted process-local destination digest the reasoning replay cache already defines, - * so both stores agree on what "the same upstream" means and neither invents a second notion of - * destination identity. + * The serving route binds this holder only after provider, model, and physical credential + * selection. A missing conversation or binding fails closed: a cell id is client-visible and is + * not itself authority to recover another request's retained result. */ -export function bridgeSearchReplayScope(baseUrl: string | undefined): string | undefined { - return reasoningReplayDestinationIdentity(baseUrl); +export function bridgeSearchReplayScope(scope: OcxReasoningReplayScopeRef | undefined): string | undefined { + const identity = scope?.current; + if (!scope?.clientPrincipalId || !scope.clientThreadId || !identity) return undefined; + return JSON.stringify([ + scope.clientPrincipalId, + scope.clientThreadId, + identity.providerName, + identity.providerDestinationIdentity, + identity.adapterName, + identity.modelId, + identity.credentialIdentity, + ]); } function keyFor(scope: string, cellItemId: string): string { diff --git a/src/server/responses/core-codex-account.ts b/src/server/responses/core-codex-account.ts index 99796846d8f..0645dda4cd3 100644 --- a/src/server/responses/core-codex-account.ts +++ b/src/server/responses/core-codex-account.ts @@ -212,6 +212,10 @@ export async function codexPoolAccountModel400Denial( wireModelId?: string, ): Promise { if (response.status !== 400) return undefined; + // A response that must not be sent again cannot open an alternate-account retry either. The + // reset helper marks the answer to a spent operator replacement this way, and that turn may + // already have run on the first send. Same rule as the quota and transient ladders below. + if (isNonReplayableResponse(response)) return undefined; try { const body = await readBoundedResponseBody(response.clone(), { signal }); if (!body.displaySafe || body.truncated) return undefined; diff --git a/src/server/responses/core-combo-failure.ts b/src/server/responses/core-combo-failure.ts index c35fc90a3df..c14c3bb680b 100644 --- a/src/server/responses/core-combo-failure.ts +++ b/src/server/responses/core-combo-failure.ts @@ -13,6 +13,7 @@ import { import { normalizeUpstreamErrorText } from "./core-errors"; import { resolveClientRetryAfter } from "../../lib/retry-after"; import { formatErrorResponse } from "../../bridge"; +import { isNonReplayableResponse, markResponseNonReplayable } from "../../lib/upstream-retry"; import { usageFromResponsesPayload } from "../request-log"; import type { ResponsesTerminalStatus } from "../../bridge"; @@ -30,6 +31,9 @@ export async function consumeComboFailure( signal?: AbortSignal, now = Date.now(), ): Promise { + // Read before the body: the marker lives on this Response object, and the failure below is + // rebuilt as a new one that would otherwise lose it. + const nonReplayable = isNonReplayableResponse(response); const fallback = `Provider error ${response.status}`; let classificationText = fallback; let usage: OcxUsage | undefined; @@ -97,16 +101,19 @@ export async function consumeComboFailure( now, includeDefault: false, }); + const failureResponse = formatErrorResponse( + response.status, + cyberFailure ? (upstreamType ?? CYBER_POLICY_ERROR_CODE) : "upstream_error", + message, + { + ...(normalizedUpstreamCode !== undefined ? { code: normalizedUpstreamCode } : {}), + ...(clientRetryAfter !== undefined ? { retryAfter: clientRetryAfter } : {}), + }, + ); + if (nonReplayable) markResponseNonReplayable(failureResponse); return { - response: formatErrorResponse( - response.status, - cyberFailure ? (upstreamType ?? CYBER_POLICY_ERROR_CODE) : "upstream_error", - message, - { - ...(normalizedUpstreamCode !== undefined ? { code: normalizedUpstreamCode } : {}), - ...(clientRetryAfter !== undefined ? { retryAfter: clientRetryAfter } : {}), - }, - ), + response: failureResponse, + ...(nonReplayable ? { nonReplayable: true } : {}), classificationText, ...(normalizedUpstreamCode !== undefined ? { upstreamCode: normalizedUpstreamCode } : {}), ...(!cyberFailure && cooldownRetryAfter !== undefined ? { retryAfter: cooldownRetryAfter } : {}), diff --git a/src/server/responses/core-combo.ts b/src/server/responses/core-combo.ts index 7b92529e710..7712904aaf4 100644 --- a/src/server/responses/core-combo.ts +++ b/src/server/responses/core-combo.ts @@ -678,9 +678,13 @@ export async function executeComboResponses( attemptRetained = true; lastFailure = failure.response; lastFailedChildLog = childLog; - const failureDecision = comboFailureDecision(failure.response.status, failure.classificationText, { - code: failure.upstreamCode, - }); + // A non-replayable failure (the answer to a spent ambiguous-reset replacement) may follow a + // send that already ran the turn, so no later target may receive it, whatever its status says. + const failureDecision = failure.nonReplayable + ? "stop" + : comboFailureDecision(failure.response.status, failure.classificationText, { + code: failure.upstreamCode, + }); const wantsStream = (rawBody as { stream?: unknown } | null)?.stream === true; // Local byte admission has its own diagnostic; do not relabel it as an upstream refusal. const classifyOverflow = failure.response.status === 413 diff --git a/src/server/responses/core-options.ts b/src/server/responses/core-options.ts index 5334cf4e02d..ec11e37045f 100644 --- a/src/server/responses/core-options.ts +++ b/src/server/responses/core-options.ts @@ -28,6 +28,12 @@ export interface ConsumedComboFailure { resetAt?: string[]; /** Reserved for 040 usage attribution without adding another body read. */ usage?: OcxUsage; + /** + * The failed attempt's response was marked non-replayable, such as the answer to a spent + * ambiguous-reset replacement. The re-wrapped {@link response} cannot carry that in-memory + * marker, so the combo loop reads it here and stops instead of sending the turn to a later target. + */ + nonReplayable?: boolean; } diff --git a/src/server/responses/passthrough-delivery.ts b/src/server/responses/passthrough-delivery.ts index 969cd84e589..b4941fbd6dd 100644 --- a/src/server/responses/passthrough-delivery.ts +++ b/src/server/responses/passthrough-delivery.ts @@ -417,10 +417,9 @@ export async function deliverPassthroughResponse( describeImages: requiresVisionPreprocessing(config, route.provider, route.modelId, route.providerName), sidecar: config.webSearchSidecar, }), - // Scope the executed-search memo to this exact upstream (#4587). The Responses adapter - // derives the same scope from the same base URL before the NEXT turn is dispatched, so - // a replayed hosted cell can be turned back into the destination's own call and result. - destinationScope: bridgeSearchReplayScope(route.provider.baseUrl), + // Snapshot the bound conversation, provider, model, destination, and credential. The + // next turn must match every dimension before its hosted cell can recover this result. + destinationScope: bridgeSearchReplayScope(parsed._reasoningReplayScope), // Appending a search result can push the continuation past the ceiling the first leg // was admitted under, so the same limit is re-applied before every later send. checkOutboundBody: (continuationBody: string) => { diff --git a/src/server/responses/request-prepare.ts b/src/server/responses/request-prepare.ts index 867f816414d..492c839b80c 100644 --- a/src/server/responses/request-prepare.ts +++ b/src/server/responses/request-prepare.ts @@ -20,6 +20,7 @@ import { sessionIdHeaderFromRequest, reasoningReplayConversationIdFromResponsesRequest, } from "../request-log-conversation"; +import { resolveContextPrincipal } from "../auth-cors"; import { isShadowSourceModel, shadowSourceModelPrefix, @@ -407,6 +408,17 @@ export async function prepareResponsesRequest( parsed._reasoningReplayScope = { clientThreadId: reasoningReplayConversationId }; } } + if (parsed._reasoningReplayScope) { + // Scope replay cells to the caller principal. On loopback, admission carries no identity, + // so resolve it from an opencodex API key the caller volunteered (same rule as context + // history ownership). A caller that presents none has no principal, and none is invented: + // every keyless local process would otherwise share one bucket, and a client-visible cell id + // would become enough to read another caller's retained search result. Without a principal + // bridgeSearchReplayScope yields no scope, so nothing is recorded or restored for it. The + // field is always rewritten so an absent principal also clears one a reused holder carried. + const clientPrincipalId = resolveContextPrincipal(req, config, options.admission); + parsed._reasoningReplayScope = { ...parsed._reasoningReplayScope, clientPrincipalId }; + } // Prefer a pre-populated id (routed Claude) over Responses headers that may be // absent or synthetically injected (session_id from prompt_cache_key). if (!logCtx.conversationId) { diff --git a/src/server/responses/request-transport.ts b/src/server/responses/request-transport.ts index e6834c7e941..2b302402f38 100644 --- a/src/server/responses/request-transport.ts +++ b/src/server/responses/request-transport.ts @@ -445,6 +445,11 @@ export async function prepareResponsesTransport( return response; } const nextAdapter = await refreshDispatchAdapter(requestParsed); + // Rebind before rebuilding: the rebuild's bridged-search restore and continuation + // restore key on the serving identity, which must be the refreshed route's, not the + // credential whose selection just lapsed. + bindRouteReasoningReplayScope({ parsed: requestParsed, providerName: route.providerName, provider: route.provider, + adapterName: nextAdapter.name, oauthCredentialSnapshot: replayOAuthCredentialSnapshot }); const rebuilt = await nextAdapter.buildRequest(requestParsed, { headers: requestState.selectedForwardHeaders, translatorBudget, ...(imageTierBias > 0 ? { imageTierBias } : {}), @@ -467,8 +472,6 @@ export async function prepareResponsesTransport( sameTargetToken = transportToken; destination = rebuilt.url; dispatchInit = { ...dispatchInit, method: rebuilt.method, headers, body: rebuilt.body }; - bindRouteReasoningReplayScope({ parsed: requestParsed, providerName: route.providerName, provider: route.provider, - adapterName: nextAdapter.name, oauthCredentialSnapshot: replayOAuthCredentialSnapshot }); // The next iteration validates synchronously and calls fetch in that same turn. } throw new Error("OAuth account selection changed repeatedly before dispatch"); diff --git a/src/types/request.ts b/src/types/request.ts index 7a73eaa5987..e1c87acdbfd 100644 --- a/src/types/request.ts +++ b/src/types/request.ts @@ -30,6 +30,11 @@ export interface OcxReasoningReplayIdentity { * the holder, so late tool-call cache writes see the active physical identity. */ export interface OcxReasoningReplayScopeRef { + /** + * Process-local caller principal from resolveContextPrincipal. Absent when the caller presented + * no identity (keyless loopback); replay state keyed by it then fails closed. + */ + readonly clientPrincipalId?: string; /** * Conversation namespace for replay state. Historically this was always the Codex parent-thread * id; headerless Responses callers use a raw sanitized thread/Cursor/session fallback, never the diff --git a/src/web-search/executor.ts b/src/web-search/executor.ts index 33b6a1eb6cc..e6abc5004ae 100644 --- a/src/web-search/executor.ts +++ b/src/web-search/executor.ts @@ -48,13 +48,17 @@ export type SidecarOutcome = WebSearchResult & { error?: string }; * * The forward backend throttles burst sidecar traffic, and without a replay the 429 becomes a * failed tool result that poisons the query for the whole turn (see failedQueries in loop.ts). - * 1 initial send + 2 replays; Retry-After is honored as a lower bound and capped by - * RETRY_AFTER_CEILING_MS (an instruction past the ceiling ends with the 429 instead of - * parking the search). Each wait releases the unread 429 body first so sockets do not - * accumulate under a rate-limit storm. Abort or timeout ends the wait through the existing - * catch, exactly like an abort during the SSE parse. + * 1 initial send + 2 replays, counted as physical sends: connection-reset recovery inside each + * send draws from the same SIDECAR_MAX_SENDS budget, so the two layers cannot multiply into nine + * paid requests during a degraded period. Retry-After is honored as a lower bound and capped by + * RETRY_AFTER_CEILING_MS and the remaining sidecar deadline (an instruction past either + * ends with the 429 instead of parking the search). Each wait releases the unread 429 body first so sockets do not + * accumulate under a rate-limit storm. The release itself may take up to a second, so a + * deadline landing during release or backoff ends with the 429 already in hand rather than + * a timeout; a caller abort still ends the wait through the shared catch, exactly like an + * abort during the SSE parse. An exhausted budget likewise ends with the 429 in hand. */ -const SIDECAR_429_MAX_ATTEMPTS = 3; +const SIDECAR_MAX_SENDS = 3; const SIDECAR_429_BASE_DELAY_MS = 1_000; const SIDECAR_429_MAX_DELAY_MS = 10_000; @@ -98,10 +102,14 @@ export async function runWebSearch( stream: true, }; const url = `${forwardProvider.baseUrl}/responses`; + // t0 precedes the deadline timer's start so the remaining-time check stays conservative. + const t0 = Date.now(); const linkedSignal = signalWithTimeout(settings.timeoutMs, abortSignal); const sidecarExit = sidecarEnter("web-search"); - const t0 = Date.now(); try { + // One physical-send budget for the whole search. Each helper call receives only what is left + // and reports every send it makes, reset retries included. + let sendsLeft = SIDECAR_MAX_SENDS; const sendOnce = () => fetchWithResetRetry( // Recovery nests INSIDE the version helper: applyUpstreamRecoveryInit then always receives a // defined init, and withUpstreamHttpVersion spreads the result, so `protocol` and the @@ -117,10 +125,18 @@ export async function runWebSearch( // `session_id`, and `x-codex-turn-metadata` to the redirect target. redirect: "manual", }, recovery), forwardProvider)), - { replaySafe: true, abortSignal: linkedSignal.signal, label: "web-search-sidecar" }, + { + replaySafe: true, + abortSignal: linkedSignal.signal, + label: "web-search-sidecar", + attempts: sendsLeft, + onSendsConsumed: sends => { sendsLeft -= sends; }, + }, ); let res = await sendOnce(); - for (let attempt = 0; res.status === 429 && attempt + 1 < SIDECAR_429_MAX_ATTEMPTS; attempt++) { + // Checked before the 429 body is released: a budget found spent after the release could only + // end in a send-budget error, recorded as a connection failure instead of the quota evidence. + for (let attempt = 0; res.status === 429 && sendsLeft > 0; attempt++) { const delay = retryBackoffDelayMs(attempt, { baseDelayMs: SIDECAR_429_BASE_DELAY_MS, maxDelayMs: SIDECAR_429_MAX_DELAY_MS, @@ -129,10 +145,19 @@ export async function runWebSearch( }); // A deadline, not a clamp: an instruction past the ceiling ends the search with the // 429 instead of parking it at a provider that already said it would refuse. - if (delay > RETRY_AFTER_CEILING_MS) break; - console.warn(`[web-search] sidecar HTTP 429 — retrying (${attempt + 2}/${SIDECAR_429_MAX_ATTEMPTS}) after ${delay}ms`); - await releaseResponseBodyBestEffort(res.body, linkedSignal.signal); - await sleepWithAbort(delay, linkedSignal.signal); + if (delay > RETRY_AFTER_CEILING_MS || delay >= settings.timeoutMs - (Date.now() - t0)) break; + console.warn(`[web-search] sidecar HTTP 429 — retrying (send ${SIDECAR_MAX_SENDS - sendsLeft + 1}/${SIDECAR_MAX_SENDS}) after ${delay}ms`); + try { + await releaseResponseBodyBestEffort(res.body, linkedSignal.signal); + await sleepWithAbort(delay, linkedSignal.signal); + } catch (e) { + // The release above may consume up to 1s, so the sidecar deadline can land during + // cleanup or mid-backoff — before the replay is dispatched. The observed 429 is + // already in hand: end with it rather than laundering it into a timeout. A caller + // abort (or a non-deadline throw) still propagates to the shared catch below. + if (!linkedSignal.signal.aborted || linkedSignal.signal.reason === abortSignal?.reason) throw e; + break; + } res = await sendOnce(); } // Attach the body guard before ANY branch reads it. The success path guarded itself below, diff --git a/structure/providers-and-adapters.md b/structure/providers-and-adapters.md index 60ebc96b2bf..bf4271d54fe 100644 --- a/structure/providers-and-adapters.md +++ b/structure/providers-and-adapters.md @@ -180,17 +180,26 @@ searches run, their hosted cells complete, the held client calls are released fo execute, and the leg's own terminal closes the turn with no continuation sent upstream. The destination therefore does not receive that search result during the turn. It gets it on the next one: every search the bridge executes is recorded in `src/responses/bridge-search-replay-cache.ts` -under the hosted cell's proxy-minted id, scoped to the upstream destination and bounded by entry -count, total bytes, and a one-hour TTL. When the caller replays that cell, +under the hosted cell's proxy-minted id, scoped to the admitted caller principal, client +conversation, and exact provider, adapter, model, destination, and physical credential binding, and bounded by entry count, total +bytes, and a one-hour TTL. An unavailable scope fails closed. The caller principal comes from +`resolveContextPrincipal`; a caller that presents no opencodex API key (a keyless loopback +client) has none and is never given a shared one, so nothing is recorded or restored for it and +its hosted cells reach the destination unchanged. When the caller replays that cell, `restoreBridgedWebSearchCalls` in `src/adapters/openai-responses/tool-output-recovery.ts` puts the destination's own `function_call` and the executed `function_call_output` back in the cell's position before the next turn's first leg is dispatched, recording exactly the text `appendBridgeSearchTurn` would have sent on a continuation leg so a replayed turn and a continued turn show the destination one consistent conversation. The rewrite runs only for a provider with -`webSearchBridge.enabled`, and a miss — unknown id, expired entry, a different destination, or a -`call_id` the body already carries — leaves the replayed item untouched. Re-running the search or -synthesizing result text is not a permitted recovery. The bridge finalizes request-scoped OpenAI sidecar authority on completion, failure, and client cancellation — cancellation releases immediately rather than waiting on an abandoned upstream read — so a recovery probe lease no search consumed is always returned. +`webSearchBridge.enabled`, and a miss — unknown id, expired entry, a different conversation or +serving binding, or a `call_id` the body already carries — leaves the replayed item untouched. +Re-running the search or synthesizing result text is not a permitted recovery. The bridge finalizes +request-scoped OpenAI sidecar authority on completion, failure, and client cancellation — +cancellation releases immediately rather than waiting on an abandoned upstream read — so a +recovery probe lease no search consumed is always returned. `tests/web-search/web-search-bridge-replay.test.ts` pins the restore and each of those refusals. +A forward OpenAI search sidecar retries a 429 only when the requested delay fits both its retry ceiling and the remaining overall sidecar deadline. A delay that cannot fit returns and records the original 429 so pool routing retains quota evidence. +One search makes at most three physical sends in total: connection-reset recovery and 429 replays draw from the same budget, and a budget spent with a 429 in hand ends with that 429 as the recorded outcome. A leg whose upstream terminal is `response.failed` or `response.incomplete` runs no search at all and closes any cell it opened rather than leaving it in progress. Assistant text is not treated as a search diff --git a/structure/runtime.md b/structure/runtime.md index 28a3bcc6808..1b2d8eaf338 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -395,6 +395,8 @@ Automatic Codex pool selection and account status share the [plan exclusion cont ### Empty forced search answers `src/web-search/loop.ts` makes at most one extra answer attempt after a clean forced-answer terminal with no visible output or tool call. The recovery has no tools and reuses gathered search results. Malformed calls fail before refusal/truncation passthrough, and well-formed recognized refusal/truncation terminals pass through unchanged, including empty or partial answers. The extra generation may incur provider usage. + +OpenAI sidecar 429 replays run only when their backoff fits the remaining sidecar deadline; otherwise the original 429 remains the routing-health outcome rather than becoming a timeout. Reset recovery and 429 replays share one three-send budget per search, so the two layers cannot multiply physical sends. ## Scoped provider quota for Combo selection `src/providers/quota/report-cache.ts` publishes routing evidence only when a producer explicitly supplies its diff --git a/structure/transports/responses.md b/structure/transports/responses.md index 1efd99ee638..023bddbe114 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -1358,6 +1358,27 @@ gone, or the leg has no send left, or a later attempt fails any other way, the l this same refusal. Nothing on that path hands the client a status that invites the whole turn to be sent again. See [ambiguous-resend gate](#ambiguous-resend-gate). +That includes what the replacement send itself answers. Once the grant is spent, the first send +may already have run the turn, so `fetchWithResetRetry` sorts the replacement's answer: + +| Replacement answer | Result | +| --- | --- | +| 2xx | Returned unchanged. | +| 307, 308, 401, 402, 408, 409, 413, 429, or any 5xx | Body released; settles as the refusal. | +| Any other status | Real status and body kept, marked non-replayable. | + +The refusal set is everything that would send again: the client retry table (408, 409, 429, +every 5xx, which the Codex client retries whatever the headers say), a client following a +307/308 with the same body, a 413 answered as a context overflow the client compacts and resends, +and this proxy's credential and quota recovery (401 refresh or rotation, 402/429 account +rotation). The gateway statuses in `isTransientUpstreamStatus` are only +a subset; 429 and 529 escaped them before. A kept status stays the upstream's evidence for the +caller, and the marker stops every recovery loop that checks it, such as the opaque-blob rebuild +of a 400 or the Codex pool's gated-model retry. A combo rebuilds a failed attempt as a new +response, so `consumeComboFailure` records `nonReplayable` and the combo stops rather than hopping +on, say, a context overflow. The cost is that a real 401, 402 or 429 on a replacement send is not +recorded against its credential on that request. + **An upstream reset observed mid-stream or after a terminal keeps its existing behaviour.** The passthrough read path still settles a genuine upstream reset as a synthetic 502, and the Codex WebSocket transport still settles `upstream_closed_before_response` (socket closed diff --git a/tests/codex-integration/codex-model-denial-evidence.test.ts b/tests/codex-integration/codex-model-denial-evidence.test.ts index 18e12f7d44b..1e5e2b4d62d 100644 --- a/tests/codex-integration/codex-model-denial-evidence.test.ts +++ b/tests/codex-integration/codex-model-denial-evidence.test.ts @@ -12,6 +12,7 @@ import { isAllowListedCodexAccountModel400, shouldRetryCodexPoolAccountModel400, } from "../../src/server/responses/core-codex-account"; +import { markResponseNonReplayable } from "../../src/lib/upstream-retry"; /** Credential generation these fixtures record under (#4952). */ const GEN = 1; @@ -182,6 +183,15 @@ describe("unsupported-model refusal detection", () => { SOL, )).toBe(false); }); + + test("a non-replayable refusal never opens an alternate-account retry", async () => { + // The answer to a spent ambiguous-reset replacement arrives marked: the turn may already + // have run, so even the exact unsupported-model refusal cannot send it from another account. + const marked = refusalResponse(SOL); + markResponseNonReplayable(marked); + expect(await shouldRetryCodexPoolAccountModel400(marked, SOL)).toBe(false); + expect(await shouldRetryCodexPoolAccountModel400(refusalResponse(SOL), SOL)).toBe(true); + }); }); // ─── Credential generation (#4952) ─────────────────────────────────────────── diff --git a/tests/lib/upstream-retry.test.ts b/tests/lib/upstream-retry.test.ts index f55d8ecf300..72cc343ec7b 100644 --- a/tests/lib/upstream-retry.test.ts +++ b/tests/lib/upstream-retry.test.ts @@ -5,6 +5,7 @@ import { fetchWithTransientRetry, isConnectionResetError, isNonReplayableResponse, + isReplayRefusalResponse, UPSTREAM_RESET_REPLAY_REFUSED_CODE, prepareSameTarget429Wait, releaseResponseBodyBestEffort, @@ -586,6 +587,86 @@ describe("operator-granted replacement of an ambiguous reset", () => { expect(mock.calls).toHaveLength(2); }); + test("a transient response after a replacement settles as the refusal", async () => { + silenceWarn(); + const mock = mockDoFetch([ + bunResetError(), new Response("busy", { status: 502 }), new Response("duplicate"), + ]); + const response = await fetchWithTransientRetry(mock.doFetch, { + attempts: 3, claimAmbiguousResend: () => true, + }); + expect(response.status).toBe(429); + expect(isNonReplayableResponse(response)).toBe(true); + expect((await response.json()).error.code).toBe(UPSTREAM_RESET_REPLAY_REFUSED_CODE); + expect(mock.calls).toHaveLength(2); + }); + + // 429 and 529 are the cases the gateway-only transient set let through: the client retry table + // and the proxy's own quota rotation both resend them. 401 and 402 are proxy recovery triggers. + test.each([307, 308, 401, 402, 408, 409, 413, 429, 500, 501, 503, 507, 529])( + "a %d answer to a spent replacement settles as the refusal and releases its body", + async (status) => { + silenceWarn(); + let cancelled = false; + const body = new ReadableStream({ cancel: () => { cancelled = true; } }); + const mock = mockDoFetch([ + bunResetError(), new Response(body, { status }), new Response("duplicate"), + ]); + const response = await fetchWithTransientRetry(mock.doFetch, { + attempts: 3, claimAmbiguousResend: () => true, + }); + expect(response.status).toBe(429); + expect(isNonReplayableResponse(response)).toBe(true); + expect(isReplayRefusalResponse(response)).toBe(true); + expect(response.headers.get("x-should-retry")).toBe("false"); + expect((await response.json()).error.code).toBe(UPSTREAM_RESET_REPLAY_REFUSED_CODE); + expect(cancelled).toBe(true); + expect(mock.calls).toHaveLength(2); + }, + ); + + test.each([400, 403, 404, 422])( + "a %d answer to a spent replacement keeps its status but can no longer trigger recovery", + async (status) => { + silenceWarn(); + const mock = mockDoFetch([ + bunResetError(), new Response("request defect", { status }), new Response("duplicate"), + ]); + const response = await fetchWithTransientRetry(mock.doFetch, { + attempts: 3, claimAmbiguousResend: () => true, + }); + expect(response.status).toBe(status); + expect(isNonReplayableResponse(response)).toBe(true); + // Still the upstream's own answer: quota and credential recorders must not treat it as a + // refusal this proxy synthesized. + expect(isReplayRefusalResponse(response)).toBe(false); + expect(await response.text()).toBe("request defect"); + expect(mock.calls).toHaveLength(2); + }, + ); + + test("a successful answer to a spent replacement is returned unchanged", async () => { + silenceWarn(); + const mock = mockDoFetch([bunResetError(), new Response("answer"), new Response("duplicate")]); + const response = await fetchWithTransientRetry(mock.doFetch, { + attempts: 3, claimAmbiguousResend: () => true, + }); + expect(response.status).toBe(200); + expect(isNonReplayableResponse(response)).toBe(false); + expect(await response.text()).toBe("answer"); + expect(mock.calls).toHaveLength(2); + }); + + test("an error answer with no replacement spent stays an ordinary recoverable response", async () => { + const mock = mockDoFetch([new Response("request defect", { status: 400 })]); + const response = await fetchWithResetRetry(mock.doFetch, { + attempts: 3, claimAmbiguousResend: () => true, + }); + expect(response.status).toBe(400); + expect(isNonReplayableResponse(response)).toBe(false); + expect(mock.calls).toHaveLength(1); + }); + test("the transient layer carries the grant into its inner reset layer", async () => { silenceWarn(); const reports: number[] = []; diff --git a/tests/server/replay-refusal-parity.test.ts b/tests/server/replay-refusal-parity.test.ts index 0c6975ccb13..d4b3fce38e5 100644 --- a/tests/server/replay-refusal-parity.test.ts +++ b/tests/server/replay-refusal-parity.test.ts @@ -196,3 +196,77 @@ test("the same client still resends an ordinary upstream rate limit", async () = await server.stop(true); } }); + +/** + * The same refusal has to hold inside a combo. The answer to a spent replacement can keep its real + * status (a 400 naming a context overflow) with only an in-memory marker, and the combo rebuilds a + * failed attempt as a new response. If that dropped the marker, the combo would read the overflow + * as target-local and send the same turn to its next target, although the first send may already + * have run it. + */ +const COMBO_FIRST_HOST = "replay-combo-first.example.test"; +const COMBO_SECOND_HOST = "replay-combo-second.example.test"; + +function comboReplayConfig(): OcxConfig { + const provider = (host: string, apiKey: string, extra: Record = {}) => ({ + adapter: "openai-responses", + baseUrl: `https://${host}/v1`, + authMode: "key", + apiKey, + models: ["model"], + ...extra, + }); + return { + port: 0, + defaultProvider: "first", + providers: { + // The first target opts in to one ambiguous-reset replacement; the second never should be sent. + first: provider(COMBO_FIRST_HOST, "sk-combo-first", { retryOnReset: {} }), + second: provider(COMBO_SECOND_HOST, "sk-combo-second"), + }, + combos: { pair: { strategy: "failover", targets: [ + { provider: "first", model: "model" }, + { provider: "second", model: "model" }, + ] } }, + } as unknown as OcxConfig; +} + +test.each([ + { name: "a context overflow", status: 400, expectedStatus: 400 }, + { name: "a 413", status: 413, expectedStatus: REPLAY_REFUSED_STATUS }, +])("a combo never sends a spent replacement's $name to its next target", async ({ status, expectedStatus }) => { + saveConfig(comboReplayConfig()); + let firstSends = 0; + let secondSends = 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(COMBO_FIRST_HOST)) { + firstSends += 1; + // The first send leaves and resets before any header; the granted replacement is answered. + if (firstSends === 1) preHeaderReset(); + return new Response(JSON.stringify({ error: { + message: "context_length_exceeded", type: "invalid_request_error", code: "context_length_exceeded", + } }), { status, headers: { "content-type": "application/json" } }); + } + if (url.includes(COMBO_SECOND_HOST)) { + secondSends += 1; + return Response.json({ + id: "resp_second", object: "response", status: "completed", model: "model", + output: [{ type: "message", id: "msg_second", role: "assistant", status: "completed", + content: [{ type: "output_text", text: "duplicate", annotations: [] }] }], + usage: { input_tokens: 1, output_tokens: 1, total_tokens: 2 }, + }); + } + return originalFetch(input as RequestInfo, init); + }) as typeof fetch; + const server = startServer(0); + try { + const { response, attempts } = await sendWithClientRetries(new URL("/v1/responses", server.url), { + model: "combo/pair", store: false, stream: false, ...RESPONSES_TURN, + }); + expect({ firstSends, secondSends, attempts }).toEqual({ firstSends: 2, secondSends: 0, attempts: 1 }); + expect(response.status).toBe(expectedStatus); + } finally { + await server.stop(true); + } +}); diff --git a/tests/server/server-key-failover-e2e.test.ts b/tests/server/server-key-failover-e2e.test.ts index a9753c6f27e..0a5e40a8a1e 100644 --- a/tests/server/server-key-failover-e2e.test.ts +++ b/tests/server/server-key-failover-e2e.test.ts @@ -7,13 +7,23 @@ import { readUsageEntries, resetUsageReadCacheForTests } from "../../src/usage/l import { loadConfig, saveConfig } from "../../src/config"; import { clearKeyCooldowns, getKeyCooldownUntil, rotateKeyOn429 } from "../../src/providers/key-failover"; import { deriveXaiConvId } from "../../src/providers/xai-transport"; -import { clearReasoningReplayCacheForTests } from "../../src/responses/reasoning-replay-cache"; +import { + clearReasoningReplayCacheForTests, + reasoningReplayDestinationIdentity, + reasoningReplayKeyCredentialIdentity, +} from "../../src/responses/reasoning-replay-cache"; +import { + bridgeSearchReplayScope, + clearBridgeSearchReplayCacheForTests, + rememberBridgeSearchReplay, +} from "../../src/responses/bridge-search-replay-cache"; import { startServer } from "../../src/server"; import { handleResponses } from "../../src/server/responses"; import type { OcxConfig } from "../../src/types"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "../helpers/isolated-codex-home"; import { removeTreeWithRetry } from "../helpers/remove-tree"; import { managementFetch } from "../helpers/management-auth"; +import { resolveContextPrincipal } from "../../src/server/auth-cors"; import { resetProviderRequestPacingForTest, setProviderRequestPacingRuntimeForTest, waitForProviderRequestSlot } from "../../src/providers/request-pacing"; import { providerApiKeySelectionIsCurrent, resolveCurrentProviderApiKeyTransport } from "../../src/providers/api-key-selection"; import { routedProviderConfig } from "../../src/router"; @@ -31,6 +41,7 @@ beforeEach(() => { process.env.OPENCODEX_HOME = testDir; clearKeyCooldowns(); clearReasoningReplayCacheForTests(); + clearBridgeSearchReplayCacheForTests(); }); afterEach(() => { @@ -43,6 +54,7 @@ afterEach(() => { if (testDir) removeTreeWithRetry(testDir); clearKeyCooldowns(); clearReasoningReplayCacheForTests(); + clearBridgeSearchReplayCacheForTests(); }); describe("server 429 key failover (end-to-end)", () => { @@ -1182,3 +1194,199 @@ test.each([false, true])("key refetch retains transient recovery metadata (strea usage: { inputTokens: 12, outputTokens: 2 } }); } finally { await server.stop(true); } }); + +const BRIDGE_CALLER_KEY = "synthetic-bridge-caller-key"; +const BRIDGE_THREAD = "thread-keyrace"; +const BRIDGE_CELL = "ws_keyrace"; + +function bridgedReplayConfig(baseUrl: string, pacing: boolean): OcxConfig { + return { + port: 0, hostname: "127.0.0.1", defaultProvider: "pooled", + apiKeys: [{ id: "bridge-caller", name: "bridge-caller", key: BRIDGE_CALLER_KEY, createdAt: "2026-01-01" }], + providers: { pooled: { + adapter: "openai-responses", baseUrl, allowPrivateNetwork: true, + authMode: "key", apiKey: "synthetic-first", + apiKeyPool: [{ id: "first", key: "synthetic-first" }, { id: "second", key: "synthetic-second" }], + webSearchBridge: { enabled: true, backend: "ollama" }, + ...(pacing ? { requestPacing: { enabled: true, minIntervalMs: 100 } } : {}), + } }, + } as OcxConfig; +} + +/** The principal the server derives for a loopback caller that presents the fixture key. */ +function bridgeCallerPrincipal(config: OcxConfig): string { + const principal = resolveContextPrincipal( + new Request("http://127.0.0.1/v1/responses", { headers: { "x-opencodex-api-key": BRIDGE_CALLER_KEY } }), + config, + { kind: "loopback", source: "loopback" }, + ); + if (!principal) throw new Error("the fixture caller key must resolve to a principal"); + return principal; +} + +/** Record one executed bridged search as the FIRST pool key would have served it. */ +function seedBridgedSearch(baseUrl: string, clientPrincipalId: string): void { + rememberBridgeSearchReplay( + bridgeSearchReplayScope({ + clientPrincipalId, + clientThreadId: BRIDGE_THREAD, + current: { + providerName: "pooled", + providerDestinationIdentity: reasoningReplayDestinationIdentity(baseUrl), + adapterName: "openai-responses", + modelId: "test", + credentialIdentity: reasoningReplayKeyCredentialIdentity({ apiKey: "synthetic-first" }), + }, + }), + BRIDGE_CELL, + { callId: "call_ws_1", name: "web_search", + argumentsText: "{\"query\":\"opencodex release\"}", output: "cached bridged result" }, + ); +} + +type SeenUpstreamTurn = { authorization: string | null; input: Record[] }; + +function serveBridgedUpstream(seen: SeenUpstreamTurn[]): string { + upstream = Bun.serve({ hostname: "127.0.0.1", port: 0, async fetch(req) { + const body = await req.json() as { input?: unknown }; + seen.push({ + authorization: req.headers.get("authorization"), + input: Array.isArray(body.input) ? body.input as Record[] : [], + }); + return Response.json({ + id: "resp_keyrace", object: "response", status: "completed", model: "test", + output: [{ type: "message", id: "msg_keyrace", role: "assistant", status: "completed", + content: [{ type: "output_text", text: "done", annotations: [] }] }], + usage: { input_tokens: 1, output_tokens: 1, total_tokens: 2 }, + }); + } }); + return `http://127.0.0.1:${upstream.port}/v1`; +} + +function bridgedReplayRequest(callerKey: string | undefined, signal?: AbortSignal): RequestInit { + return { + method: "POST", + headers: { + "content-type": "application/json", + "thread-id": BRIDGE_THREAD, + ...(callerKey ? { "x-opencodex-api-key": callerKey } : {}), + }, + ...(signal ? { signal } : {}), + body: JSON.stringify({ + model: "pooled/test", stream: false, + input: [ + { role: "user", content: [{ type: "input_text", text: "what is the latest release?" }] }, + { type: "web_search_call", id: BRIDGE_CELL, status: "completed", + action: { type: "search", query: "opencodex release", queries: ["opencodex release"] } }, + ], + }), + }; +} + +function hostedCellReachedUpstream(turn: SeenUpstreamTurn): boolean { + return turn.input.some(item => item.type === "web_search_call" && item.id === BRIDGE_CELL); +} + +function recordedResultRestored(turn: SeenUpstreamTurn): boolean { + return turn.input.some(item => item.call_id === "call_ws_1"); +} + +test("a keyed caller's bridged search is restored on its next turn", async () => { + // Positive control for the two refusals below: the same seed, principal, thread and + // credential does restore, so a miss there is the scope doing its job, not a broken fixture. + const seen: SeenUpstreamTurn[] = []; + const baseUrl = serveBridgedUpstream(seen); + const config = bridgedReplayConfig(baseUrl, false); + saveConfig(config); + seedBridgedSearch(baseUrl, bridgeCallerPrincipal(config)); + const server = startServer(0); + try { + const response = await fetch(new URL("/v1/responses", server.url), bridgedReplayRequest(BRIDGE_CALLER_KEY)); + expect(response.status).toBe(200); + await response.text(); + expect(seen).toHaveLength(1); + expect(recordedResultRestored(seen[0]!)).toBe(true); + expect(hostedCellReachedUpstream(seen[0]!)).toBe(false); + } finally { + await server.stop(true); + } +}); + +test("a keyless loopback caller neither restores nor shares a bridged search", async () => { + // A caller that presents no opencodex key has no principal. It must not fall into a shared + // bucket: seed both the keyed caller's cell and the literal bucket a fallback would have used. + const seen: SeenUpstreamTurn[] = []; + const baseUrl = serveBridgedUpstream(seen); + const config = bridgedReplayConfig(baseUrl, false); + saveConfig(config); + seedBridgedSearch(baseUrl, bridgeCallerPrincipal(config)); + seedBridgedSearch(baseUrl, "loopback"); + const server = startServer(0); + try { + const response = await fetch(new URL("/v1/responses", server.url), bridgedReplayRequest(undefined)); + expect(response.status).toBe(200); + await response.text(); + expect(seen).toHaveLength(1); + expect(recordedResultRestored(seen[0]!)).toBe(false); + expect(hostedCellReachedUpstream(seen[0]!)).toBe(true); + } finally { + await server.stop(true); + } +}); + +test("a dispatch-time key switch rebuilds the bridged-search restore under the new credential", async () => { + // Regression for the oauthDispatch rebuild order: the Responses adapter restores a replayed + // web_search_call from the memo keyed by _reasoningReplayScope, so the rebuild must rebind + // that scope to the refreshed credential BEFORE buildRequest runs. Restoring under the key + // whose selection just lapsed, then sending under the newly selected key, would hand the + // first credential's recorded result to the second credential's upstream. + let now = 0; + let resumePacing: (() => void) | undefined; + const queued = Promise.withResolvers(); + setProviderRequestPacingRuntimeForTest({ + now: () => now, + setTimer(callback, delayMs) { + resumePacing = () => { now += delayMs; callback(); }; + queued.resolve(); + return callback; + }, + clearTimer() {}, + enqueueMicrotask: queueMicrotask, + }); + const seen: SeenUpstreamTurn[] = []; + const baseUrl = serveBridgedUpstream(seen); + const config = bridgedReplayConfig(baseUrl, true); + saveConfig(config); + + // Seed the memo under the identity the FIRST key binds: the same caller principal and thread + // the request below carries, but the credential whose selection is about to lapse. + seedBridgedSearch(baseUrl, bridgeCallerPrincipal(config)); + + const server = startServer(0); + const abort = new AbortController(); + try { + await waitForProviderRequestSlot("pooled", config.providers.pooled); + const pending = fetch(new URL("/v1/responses", server.url), bridgedReplayRequest(BRIDGE_CALLER_KEY, abort.signal)); + await queued.promise; + const selected = await managementFetch(new URL("/api/providers/keys/active", server.url), { + method: "PUT", headers: { "content-type": "application/json" }, + body: JSON.stringify({ name: "pooled", id: "second" }), + }); + expect(selected.status).toBe(200); + await selected.text(); + resumePacing!(); + const response = await pending; + expect(response.status).toBe(200); + await response.text(); + expect(seen).toHaveLength(1); + expect(seen[0]!.authorization).toBe("Bearer synthetic-second"); + // Rebound before rebuild: the memo lookup misses under the new credential, so the hosted + // cell reaches the second key's upstream verbatim instead of the first key's result. + expect(hostedCellReachedUpstream(seen[0]!)).toBe(true); + expect(recordedResultRestored(seen[0]!)).toBe(false); + } finally { + abort.abort(); + await server.stop(true); + resetProviderRequestPacingForTest(); + } +}); diff --git a/tests/usage/request-log.test.ts b/tests/usage/request-log.test.ts index ae4c9b9493d..36c59c30b7c 100644 --- a/tests/usage/request-log.test.ts +++ b/tests/usage/request-log.test.ts @@ -832,7 +832,7 @@ describe("request log metadata", () => { expect(requestLogErrorCode(429)).toBe("rate_limit_exceeded"); expect(requestLogErrorCode( 429, - "The upstream connection closed before a response was received. The request may already have been processed; automatic replay was stopped.", + "The upstream exchange did not complete reliably. The request may already have been processed; automatic replay was stopped.", )).toBe("upstream_reset_replay_refused"); expect(requestLogErrorCode(499)).toBe("client_closed_request"); expect(requestLogErrorCode(502, "client closed request during web-search")).toBe("client_closed_request"); diff --git a/tests/web-search/web-search-bridge-replay.test.ts b/tests/web-search/web-search-bridge-replay.test.ts index 3d5a6aab28c..3a330ae9146 100644 --- a/tests/web-search/web-search-bridge-replay.test.ts +++ b/tests/web-search/web-search-bridge-replay.test.ts @@ -20,7 +20,7 @@ import { } from "../../src/responses/bridge-search-replay-cache"; import { createResponsesPassthroughAdapter as createResponsesPassthroughAdapterProduction } from "../../src/adapters/openai-responses"; import { withTestTranslatorBudget } from "../helpers/translator-budget"; -import type { OcxProviderConfig } from "../../src/types"; +import type { OcxProviderConfig, OcxReasoningReplayScopeRef } from "../../src/types"; const createResponsesPassthroughAdapter = (...args: Parameters) => withTestTranslatorBudget(createResponsesPassthroughAdapterProduction(...args)); @@ -28,6 +28,21 @@ const createResponsesPassthroughAdapter = (...args: Parameters = {}): OcxReasoningReplayScopeRef { + return { + clientPrincipalId: "principal-a", + clientThreadId: "thread-a", + current: { + providerName: "bridge-a", + providerDestinationIdentity: overrides.providerDestinationIdentity ?? GATEWAY_BASE_URL, + adapterName: "openai-responses", + modelId: "glm-4.7", + credentialIdentity: "key-a", + ...overrides, + }, + }; +} + function frame(type: string, payload: Record): string { return "event: " + type + "\ndata: " + JSON.stringify({ type, ...payload }); } @@ -115,7 +130,7 @@ async function runBridgedMixedLeg(baseUrl: string, result = "opencodex 2.50.0 sh throw new Error("a mixed leg must not send a continuation"); }, execute: async () => ({ text: result, sources: [{ url: "https://example.test/rel", title: "Releases" }] }), - destinationScope: bridgeSearchReplayScope(baseUrl), + destinationScope: bridgeSearchReplayScope(replayScope({ providerDestinationIdentity: baseUrl })), }); const body = await new Response(stream).text(); const added = clientEvents(body).find(event => @@ -154,7 +169,7 @@ describe("bridged web_search replay to the destination", () => { const restored = restoreBridgedWebSearchCalls( nextTurnBody(cellId), - bridgeSearchReplayScope(GATEWAY_BASE_URL), + bridgeSearchReplayScope(replayScope()), ) as { input: Record[] }; // The item type the destination never produced is gone, replaced in place by the exchange @@ -188,7 +203,7 @@ describe("bridged web_search replay to the destination", () => { throw new Error("a mixed leg must not send a continuation"); }, execute: async () => ({ text: "", sources: [], error: "backend refused" }), - destinationScope: bridgeSearchReplayScope(GATEWAY_BASE_URL), + destinationScope: bridgeSearchReplayScope(replayScope()), }); const body = await new Response(stream).text(); const added = clientEvents(body).find(event => @@ -198,7 +213,7 @@ describe("bridged web_search replay to the destination", () => { const restored = restoreBridgedWebSearchCalls( nextTurnBody(cellId), - bridgeSearchReplayScope(GATEWAY_BASE_URL), + bridgeSearchReplayScope(replayScope()), ) as { input: Record[] }; expect(restored.input[2]).toEqual({ type: "function_call_output", @@ -209,7 +224,7 @@ describe("bridged web_search replay to the destination", () => { test("a cell this proxy never executed is left exactly as the caller sent it", () => { const body = nextTurnBody("ws_never-recorded"); - const restored = restoreBridgedWebSearchCalls(body, bridgeSearchReplayScope(GATEWAY_BASE_URL)); + const restored = restoreBridgedWebSearchCalls(body, bridgeSearchReplayScope(replayScope())); // Same reference: a miss allocates nothing and invents nothing. expect(restored).toBe(body); }); @@ -217,7 +232,31 @@ describe("bridged web_search replay to the destination", () => { test("a search recorded for one destination is not replayed into another", async () => { const cellId = await runBridgedMixedLeg(GATEWAY_BASE_URL); const body = nextTurnBody(cellId); - expect(restoreBridgedWebSearchCalls(body, bridgeSearchReplayScope(OTHER_BASE_URL))).toBe(body); + expect(restoreBridgedWebSearchCalls(body, bridgeSearchReplayScope(replayScope({ providerDestinationIdentity: OTHER_BASE_URL })))).toBe(body); + }); + + test("a cell cannot cross any conversation or serving-identity boundary", async () => { + const cellId = await runBridgedMixedLeg(GATEWAY_BASE_URL); + const body = nextTurnBody(cellId); + const mismatchedScopes: OcxReasoningReplayScopeRef[] = [ + { ...replayScope(), clientPrincipalId: "principal-b" }, + { ...replayScope(), clientThreadId: "thread-b" }, + replayScope({ providerName: "bridge-b" }), + replayScope({ adapterName: "other-adapter" }), + replayScope({ modelId: "other-model" }), + replayScope({ credentialIdentity: "key-b" }), + ]; + for (const scope of mismatchedScopes) { + expect(restoreBridgedWebSearchCalls(body, bridgeSearchReplayScope(scope))).toBe(body); + } + expect(bridgeSearchReplayScope(undefined)).toBeUndefined(); + expect(bridgeSearchReplayScope({ clientThreadId: "thread-a" })).toBeUndefined(); + expect(bridgeSearchReplayScope({ clientPrincipalId: "principal-a", clientThreadId: "thread-a" })).toBeUndefined(); + // A bound serving identity is not enough on its own: without a caller principal there is no + // owner, so the recorded cell above must stay unreachable and no new cell can be recorded. + const unowned: OcxReasoningReplayScopeRef = { clientThreadId: "thread-a", current: replayScope().current }; + expect(bridgeSearchReplayScope(unowned)).toBeUndefined(); + expect(restoreBridgedWebSearchCalls(body, bridgeSearchReplayScope(unowned))).toBe(body); }); test("an expired entry behaves exactly like a miss", async () => { @@ -226,13 +265,13 @@ describe("bridged web_search replay to the destination", () => { const cellId = await runBridgedMixedLeg(GATEWAY_BASE_URL); const body = nextTurnBody(cellId); // Still inside the TTL. - expect(restoreBridgedWebSearchCalls(body, bridgeSearchReplayScope(GATEWAY_BASE_URL))).not.toBe(body); + expect(restoreBridgedWebSearchCalls(body, bridgeSearchReplayScope(replayScope()))).not.toBe(body); clockMs += 61 * 60 * 1000; - expect(restoreBridgedWebSearchCalls(body, bridgeSearchReplayScope(GATEWAY_BASE_URL))).toBe(body); + expect(restoreBridgedWebSearchCalls(body, bridgeSearchReplayScope(replayScope()))).toBe(body); }); test("a call id the body already carries is never duplicated", () => { - const scope = bridgeSearchReplayScope(GATEWAY_BASE_URL); + const scope = bridgeSearchReplayScope(replayScope()); rememberBridgeSearchReplay(scope, "ws_dup", { callId: "call_2", name: "web_search", @@ -244,7 +283,7 @@ describe("bridged web_search replay to the destination", () => { }); test("an unbridged provider is never given a scope to restore from", () => { - const scope = bridgeSearchReplayScope(GATEWAY_BASE_URL); + const scope = bridgeSearchReplayScope(replayScope()); rememberBridgeSearchReplay(scope, "ws_unbridged", { callId: "call_1", name: "web_search", @@ -274,6 +313,7 @@ describe("the Responses passthrough adapter", () => { stream: true, options: {}, _rawBody: nextTurnBody(cellId), + _reasoningReplayScope: replayScope(), }, { headers: new Headers() }); return (JSON.parse(request.body) as { input: Record[] }).input; } diff --git a/tests/web-search/web-search-sidecar-429.test.ts b/tests/web-search/web-search-sidecar-429.test.ts index 6956b855333..a8cd4167645 100644 --- a/tests/web-search/web-search-sidecar-429.test.ts +++ b/tests/web-search/web-search-sidecar-429.test.ts @@ -31,17 +31,31 @@ describe("web-search sidecar 429 replays", () => { return new Response("data: [DONE]\n\n", { headers: { "content-type": "text/event-stream" } }); } - function searchWith(fetchImpl: () => Promise) { + function searchWith( + fetchImpl: () => Promise, + timeoutMs = 30_000, + recordOutcome?: (outcome: number | "connect_error" | "connect_neutral" | "timeout") => void, + abortSignal?: AbortSignal, + ) { globalThis.fetch = fetchImpl as unknown as typeof fetch; return runOpenAiWebSearch( "current docs", { type: "web_search" }, sidecarProvider(), new Headers({ authorization: "Bearer selected-token" }), - { model: "gpt-5.6-luna", reasoning: "low", timeoutMs: 30_000 }, + { model: "gpt-5.6-luna", reasoning: "low", timeoutMs }, + abortSignal, + recordOutcome, ); } + function socketReset(): Error { + // Shape of Bun's fetch rejection on a stale pooled socket. + const err = new Error("The socket connection was closed unexpectedly"); + (err as Error & { code: string }).code = "ECONNRESET"; + return err; + } + test("a burst 429 is replayed and the recovered answer is returned", async () => { let calls = 0; const outcome = await searchWith(async () => { @@ -72,4 +86,87 @@ describe("web-search sidecar 429 replays", () => { expect(calls).toBe(1); expect(outcome.error).toContain("429"); }); + + test("a Retry-After that cannot fit the sidecar deadline preserves the 429", async () => { + let calls = 0; + const recorded: Array = []; + const outcome = await searchWith(async () => { + calls += 1; + return new Response("slow down", { status: 429, headers: { "retry-after": "0.1" } }); + }, 50, value => recorded.push(value)); + expect(calls).toBe(1); + expect(outcome.error).toContain("429"); + expect(recorded).toEqual([429]); + }); + + test("a deadline expiring during pre-retry body cleanup preserves the 429", async () => { + // The never-settling body is a worse leak than the other mocks leave behind: restore + // fetch so a later file's shared search loop does not inherit a 1s release per retry. + const originalFetch = globalThis.fetch; + try { + let calls = 0; + const recorded: Array = []; + const outcome = await searchWith(async () => { + calls += 1; + // A cancel() that never settles makes the bounded 1s release run to its cap; the + // remaining deadline then cannot fit the backoff, so the wait ends mid-sleep. The + // observed 429 must survive that expiry instead of being recorded as a timeout. + const body = new ReadableStream({ + start: controller => controller.enqueue(new TextEncoder().encode("rate limited")), + cancel: () => new Promise(() => {}), + }); + return new Response(body, { status: 429, headers: { "retry-after": "1" } }); + }, 1_500, value => recorded.push(value)); + expect(calls).toBe(1); + expect(outcome.error).toContain("429"); + expect(recorded).toEqual([429]); + } finally { + globalThis.fetch = originalFetch; + } + }); + + test("reset recovery and 429 replays share one three-send budget", async () => { + // Each quota leg used to open its own three-send reset allowance, so resets in front of every + // 429 could reach nine paid requests. The script repeats reset, reset, 429 indefinitely. + let calls = 0; + const recorded: Array = []; + const outcome = await searchWith(async () => { + calls += 1; + if (calls % 3 !== 0) throw socketReset(); + return new Response("rate limited", { status: 429 }); + }, 30_000, value => recorded.push(value)); + expect(calls).toBe(3); + // The budget ran out with the 429 in hand, so the quota evidence survives rather than being + // replaced by a send-budget error recorded as a connection failure. + expect(outcome.error).toContain("429"); + expect(recorded).toEqual([429]); + }); + + test("a reset in front of the first 429 leaves only one quota replay", async () => { + let calls = 0; + const recorded: Array = []; + const outcome = await searchWith(async () => { + calls += 1; + if (calls === 1) throw socketReset(); + return new Response("rate limited", { status: 429 }); + }, 30_000, value => recorded.push(value)); + expect(calls).toBe(3); + expect(outcome.error).toContain("429"); + expect(recorded).toEqual([429]); + }); + + test("a caller abort during 429 backoff ends the search as a cancellation", async () => { + let calls = 0; + const recorded: Array = []; + const caller = new AbortController(); + const outcome = await searchWith(async () => { + calls += 1; + setTimeout(() => caller.abort(new DOMException("caller left", "AbortError")), 20); + return new Response("rate limited", { status: 429, headers: { "retry-after": "1" } }); + }, 30_000, value => recorded.push(value), caller.signal); + expect(calls).toBe(1); + expect(outcome.error).toBeDefined(); + // A caller that left is neither a quota signal nor a connection failure. + expect(recorded).toEqual(["connect_neutral"]); + }); });