Repository navigation
fix(codex): same-request multi-account failover on quota 429 (#584) #585
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -228,6 +228,140 @@ async function shouldRetryCodexPoolAccountModel400( | |
| } | ||
| } | ||
|
|
||
| /** Pre-stream quota/billing rejections that warrant one alternate-account attempt (#584). */ | ||
| function shouldRetryCodexPoolAccountQuota(response: Response): boolean { | ||
| return response.status === 429 || response.status === 402; | ||
| } | ||
|
|
||
| interface CodexPoolAccountRetryArgs { | ||
| req: Request; | ||
| config: OcxConfig; | ||
| route: { providerName: string; modelId: string; provider: OcxProviderConfig }; | ||
| parsed: OcxParsedRequest; | ||
| logCtx: RequestLogContext; | ||
| options: { | ||
| abortSignal?: AbortSignal; | ||
| onCodexAuthContextResolved?: (ctx: CodexAuthContext) => void; | ||
| }; | ||
| firstAuthCtx: Extract<CodexAuthContext, { kind: "pool" | "main-pool" }>; | ||
| firstResponse: Response; | ||
| outcomeStatus: number; | ||
| upstream: AbortController; | ||
| connectMs: number; | ||
| passthroughEstimate?: number; | ||
| stream: boolean; | ||
| } | ||
|
|
||
| type CodexPoolAccountRetryResult = | ||
| | { | ||
| kind: "retried"; | ||
| authCtx: Extract<CodexAuthContext, { kind: "pool" | "main-pool" }>; | ||
| request: Awaited<ReturnType<ReturnType<typeof resolveAdapter>["buildRequest"]>>; | ||
| upstreamResponse: Response; | ||
| selectedForwardHeaders: Headers; | ||
| } | ||
| | { kind: "no-alternate" } | ||
| | { | ||
| kind: "transport"; | ||
| error: unknown; | ||
| authCtx: Extract<CodexAuthContext, { kind: "pool" | "main-pool" }>; | ||
| }; | ||
|
|
||
| /** | ||
| * One bounded alternate-account retry for Codex pool auth. Used for allow-listed | ||
| * model-400 and for pre-stream 429/402 quota failures (#584). | ||
| */ | ||
| async function retryCodexPoolOnAlternateAccount( | ||
| args: CodexPoolAccountRetryArgs, | ||
| ): Promise<CodexPoolAccountRetryResult> { | ||
| const { | ||
| req, config, route, parsed, logCtx, options, firstAuthCtx, firstResponse, | ||
| outcomeStatus, upstream, connectMs, passthroughEstimate, stream, | ||
| } = args; | ||
| let retryAuthCtx: CodexAuthContext | undefined; | ||
| try { | ||
| retryAuthCtx = await resolveCodexAuthContext( | ||
| req.headers, | ||
| config, | ||
| "pool", | ||
| { excludeAccountId: firstAuthCtx.accountId }, | ||
| ); | ||
| } catch (error) { | ||
| if ( | ||
| !(error instanceof CodexPoolAuthenticationError) | ||
| && !(error instanceof CodexAuthContextError) | ||
| && !(error instanceof CodexAccountCooldownError) | ||
| ) throw error; | ||
| } | ||
| if (retryAuthCtx?.kind !== "pool" && retryAuthCtx?.kind !== "main-pool") { | ||
| return { kind: "no-alternate" }; | ||
| } | ||
|
|
||
| const retryAfterRaw = firstResponse.headers.get("retry-after"); | ||
| if (outcomeStatus === 429 || outcomeStatus === 402) { | ||
| const { applyAccountQuotaFromUpstreamHeaders } = await import("../../codex/auth-api"); | ||
| applyAccountQuotaFromUpstreamHeaders(firstAuthCtx.accountId, firstResponse.headers); | ||
| } | ||
| recordCodexUpstreamOutcome(config, firstAuthCtx.accountId, outcomeStatus, { | ||
| retryAfter: retryAfterRaw, | ||
| resetAt: [ | ||
| firstResponse.headers.get("x-codex-primary-reset-at"), | ||
| firstResponse.headers.get("x-codex-secondary-reset-at"), | ||
| firstResponse.headers.get("x-codex-tertiary-reset-at"), | ||
| ].filter(Boolean), | ||
| threadId: req.headers.get("x-codex-parent-thread-id"), | ||
| probeLeaseId: codexProbeLeaseId(firstAuthCtx), | ||
| }); | ||
|
|
||
| const retryHeaders = headersForCodexAuthContext(req.headers, retryAuthCtx); | ||
| const retryProvider = applyCodexAuthContextToProvider( | ||
| stripCodexRuntimeProviderFields(route.provider), | ||
| retryAuthCtx, | ||
| "pool", | ||
| ); | ||
| const retryAdapter = resolveAdapter( | ||
| resolveWireProtocolOverride(route.providerName, route.modelId, retryProvider), | ||
| config.cacheRetention, | ||
| ); | ||
| const request = await retryAdapter.buildRequest(parsed, { headers: retryHeaders }); | ||
| recordAdapterReasoning(logCtx, request); | ||
|
|
||
| await firstResponse.body?.cancel().catch(() => undefined); | ||
| options.onCodexAuthContextResolved?.(retryAuthCtx); | ||
| route.provider = retryProvider; | ||
| logCtx.provider = formatCodexProviderForLog( | ||
| route.providerName, | ||
| retryAuthCtx.accountId, | ||
| config, | ||
| ); | ||
|
|
||
| noteAttemptSend(logCtx.activeAttempt, passthroughEstimate); | ||
| try { | ||
| const upstreamResponse = await fetchWithHeaderTimeout( | ||
| request.url, | ||
| { | ||
| method: request.method, | ||
| headers: request.headers, | ||
| body: request.body, | ||
| }, | ||
| upstream.signal, | ||
| connectMs, | ||
| stream, | ||
| providerFetch(route.provider), | ||
| ); | ||
| return { | ||
| kind: "retried", | ||
| authCtx: retryAuthCtx, | ||
| request, | ||
| upstreamResponse, | ||
| selectedForwardHeaders: retryHeaders, | ||
| }; | ||
| } catch (error) { | ||
| // Attribute the transport failure to the alternate account (already selected). | ||
| return { kind: "transport", error, authCtx: retryAuthCtx }; | ||
| } | ||
| } | ||
|
|
||
|
|
||
|
|
||
| export function codexForwardTerminalOutcomeRecorder( | ||
|
|
@@ -1273,77 +1407,46 @@ export async function handleResponses( | |
| return transportFailureResponse(err); | ||
| } | ||
|
|
||
| if ( | ||
| usesCodexForwardPoolAuth(authCtx, route.provider) | ||
| && await shouldRetryCodexPoolAccountModel400( | ||
| if (usesCodexForwardPoolAuth(authCtx, route.provider)) { | ||
| let poolRetryOutcome: number | undefined; | ||
| if (await shouldRetryCodexPoolAccountModel400( | ||
| upstreamResponse, | ||
| route.modelId, | ||
| options.abortSignal, | ||
| ) | ||
| ) { | ||
| const firstAuthCtx = authCtx; | ||
| let retryAuthCtx: CodexAuthContext | undefined; | ||
| try { | ||
| retryAuthCtx = await resolveCodexAuthContext( | ||
| req.headers, | ||
| config, | ||
| "pool", | ||
| { excludeAccountId: firstAuthCtx.accountId }, | ||
| ); | ||
| } catch (error) { | ||
| if ( | ||
| !(error instanceof CodexPoolAuthenticationError) | ||
| && !(error instanceof CodexAuthContextError) | ||
| && !(error instanceof CodexAccountCooldownError) | ||
| ) throw error; | ||
| )) { | ||
| poolRetryOutcome = 400; | ||
| } else if (shouldRetryCodexPoolAccountQuota(upstreamResponse)) { | ||
| // Pre-stream only: once SSE has begun, mid-stream quota stays terminal. | ||
| poolRetryOutcome = upstreamResponse.status; | ||
| } | ||
|
|
||
| if (retryAuthCtx?.kind === "pool" || retryAuthCtx?.kind === "main-pool") { | ||
| recordCodexUpstreamOutcome(config, firstAuthCtx.accountId, 400, { | ||
| threadId: req.headers.get("x-codex-parent-thread-id"), | ||
| probeLeaseId: codexProbeLeaseId(firstAuthCtx), | ||
| }); | ||
|
|
||
| const retryHeaders = headersForCodexAuthContext(req.headers, retryAuthCtx); | ||
| const retryProvider = applyCodexAuthContextToProvider( | ||
| stripCodexRuntimeProviderFields(route.provider), | ||
| retryAuthCtx, | ||
| "pool", | ||
| ); | ||
| const retryAdapter = resolveAdapter( | ||
| resolveWireProtocolOverride(route.providerName, route.modelId, retryProvider), | ||
| config.cacheRetention, | ||
| ); | ||
| request = await retryAdapter.buildRequest(parsed, { headers: retryHeaders }); | ||
| recordAdapterReasoning(logCtx, request); | ||
|
|
||
| await upstreamResponse.body?.cancel().catch(() => undefined); | ||
| authCtx = retryAuthCtx; | ||
| options.onCodexAuthContextResolved?.(retryAuthCtx); | ||
| selectedForwardHeaders = retryHeaders; | ||
| route.provider = retryProvider; | ||
| logCtx.provider = formatCodexProviderForLog( | ||
| route.providerName, | ||
| retryAuthCtx.accountId, | ||
| if (poolRetryOutcome !== undefined) { | ||
| const retry = await retryCodexPoolOnAlternateAccount({ | ||
| req, | ||
| config, | ||
| ); | ||
|
|
||
| noteAttemptSend(logCtx.activeAttempt, passthroughEstimate); | ||
| try { | ||
| upstreamResponse = await fetchWithHeaderTimeout( | ||
| request.url, | ||
| { | ||
| method: request.method, | ||
| headers: request.headers, | ||
| body: request.body, | ||
| }, | ||
| upstream.signal, | ||
| connectMs, | ||
| parsed.stream, | ||
| providerFetch(route.provider), | ||
| ); | ||
| } catch (err) { | ||
| return transportFailureResponse(err); | ||
| route, | ||
| parsed, | ||
| logCtx, | ||
| options, | ||
| firstAuthCtx: authCtx, | ||
| firstResponse: upstreamResponse, | ||
| outcomeStatus: poolRetryOutcome, | ||
| upstream, | ||
| connectMs, | ||
| passthroughEstimate, | ||
| stream: parsed.stream, | ||
| }); | ||
| if (retry.kind === "transport") { | ||
| authCtx = retry.authCtx; | ||
| return transportFailureResponse(retry.error); | ||
| } | ||
| if (retry.kind === "retried") { | ||
| authCtx = retry.authCtx; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a AGENTS.md reference: AGENTS.md:L93-L95 Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in e589087: subagentFallbackAccountId updates to the retried account on successful failover. |
||
| request = retry.request; | ||
| upstreamResponse = retry.upstreamResponse; | ||
| selectedForwardHeaders = retry.selectedForwardHeaders; | ||
| // Keep subagent quota-failure health keyed to the account that actually served. | ||
| subagentFallbackAccountId = retry.authCtx.accountId; | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an account returns 402 without quota-utilization headers, the retry succeeds on the alternate but
recordCodexUpstreamOutcomeclassifies 402 as a caller error becauseclassifyCodexUpstreamOutcomeonly recognizes 429 as quota. Consequently the depleted account receives no cooldown, its thread affinities remain intact, and it remains active, so every subsequent request is sent to it and rejected before retrying the alternate. Include 402 in the quota classification or translate it before recording, with a focused 402 regression test.AGENTS.md reference: AGENTS.md:L93-L95
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 2556576: classifyCodexUpstreamOutcome treats 402 as quota (with classifier test).