From 173e92b5cdc423fa64ce71e637fa7a639de8a32e Mon Sep 17 00:00:00 2001 From: JUN Date: Tue, 15 Sep 2026 00:20:59 +0900 Subject: [PATCH 1/3] fix(codex): stop retrying a doomed pool credential refresh, and say it was local (#4546) Reproduced live: every request routed to one pool account returned 503 server_is_overloaded, five of five sequential probes, with a healthy stored record and not one line in the service log. The refusal was this proxy's own poolCredentialRefreshIncompleteResponse, and because only revoked/expired counted as terminal, a missing record or a token-endpoint 5xx became an endlessly retryable 503 on an account selection kept returning to. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. --- scripts/test-layout/layout.json | 3 +- src/codex/account-lifecycle.ts | 3 + src/codex/account-store.ts | 75 +++++++++-- src/codex/pool-refresh-backoff.ts | 100 +++++++++++++++ src/codex/routing.ts | 10 +- src/server/request-log.ts | 15 ++- src/server/responses/core.ts | 18 ++- .../codex-pool-refresh-backoff.test.ts | 116 ++++++++++++++++++ tests/fixtures/test-layout-expected.json | 3 +- 9 files changed, 328 insertions(+), 15 deletions(-) create mode 100644 src/codex/pool-refresh-backoff.ts create mode 100644 tests/codex-integration/codex-pool-refresh-backoff.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 83deb4ba9f..d69ce45ac9 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1450,7 +1450,8 @@ "adapter-input-media-guard.test.ts": "adapters", "chat-media-translation.test.ts": "responses", "execution-budget-permits.test.ts": "lib", - "spend-instrumentation-log.test.ts": "server" + "spend-instrumentation-log.test.ts": "server", + "codex-pool-refresh-backoff.test.ts": "codex-integration" }, "migrated": [ "adapters", diff --git a/src/codex/account-lifecycle.ts b/src/codex/account-lifecycle.ts index 88208bfb1b..c274f49085 100644 --- a/src/codex/account-lifecycle.ts +++ b/src/codex/account-lifecycle.ts @@ -11,6 +11,8 @@ import { getMainChatgptAccountId, readCodexTokensResult } from "./auth-collision import { MAIN_CODEX_ACCOUNT_ID, setMainAccountPlan } from "./main-account"; import { clearAccountQuota } from "./quota"; import { clearCodexUpstreamHealthForAccount, clearThreadAccountMapForAccount } from "./routing"; + +import { clearCodexPoolRefreshFailure } from "./pool-refresh-backoff"; import { invalidateCodexWebSocketsForAccount } from "./websocket-registry"; import { clearMainAccountCredentialPresence, clearMainAccountInfoCache, observeMainQuotaCredential, observeMainQuotaIdentity } from "./main-account-cache"; import { extractAccountIdClaims } from "../oauth/chatgpt"; @@ -41,6 +43,7 @@ export function purgeCodexAccountRuntimeState(accountId: string): void { clearAccountQuota(accountId); clearThreadAccountMapForAccount(accountId); clearCodexUpstreamHealthForAccount(accountId); + clearCodexPoolRefreshFailure(accountId); if (accountId === MAIN_CODEX_ACCOUNT_ID) { clearMainAccountInfoCache(); clearMainAccountCredentialPresence(); diff --git a/src/codex/account-store.ts b/src/codex/account-store.ts index 4b151707a1..b3c6313d83 100644 --- a/src/codex/account-store.ts +++ b/src/codex/account-store.ts @@ -17,6 +17,13 @@ import { isValidCodexAccountId } from "./account-id"; import type { PoolQuotaWriter } from "./quota-types"; import { CODEX_REFRESH_FLIGHT_CEILING_MS } from "./quota-recovery-timing"; +import { + CodexPoolRefreshCooldownError, + clearCodexPoolRefreshFailure, + isCodexPoolRefreshCooling, + noteCodexPoolRefreshFailure, +} from "./pool-refresh-backoff"; + type LegacyCodexAccountStore = Record; type CodexAccountStore = Record; type RawCodexAccountStore = Record; @@ -480,6 +487,17 @@ export class TokenRefreshError extends Error { } } +/** + * The stored record or its refresh-grant fingerprint is gone. Retrying cannot + * conjure a missing credential, so callers must treat this as terminal. + */ +export class CodexCredentialUnavailableError extends Error { + constructor(message = "Codex account credential is unavailable; reauthenticate the account.") { + super(message); + this.name = "CodexCredentialUnavailableError"; + } +} + export class CodexCredentialGenerationConflictError extends Error { constructor(message = "Codex account changed during refresh") { super(message); @@ -514,6 +532,30 @@ export class CodexCredentialRefreshStaleError extends Error { } } +/** + * Terminal means the grant itself is dead, or there is no grant to refresh. + * Token-endpoint 5xx (`unknown`) and a generation CAS loss stay transient + * because those genuinely may clear (#2887). + */ +export function isTerminalCodexPoolRefreshFailure(error: unknown): boolean { + return (error instanceof TokenRefreshError && (error.reason === "revoked" || error.reason === "expired")) + || error instanceof CodexCredentialUnavailableError; +} + +function isOperationalCodexPoolRefreshFailure(error: unknown): boolean { + if (error instanceof CodexPoolRefreshCooldownError) return true; + if (error instanceof CodexCredentialRefreshBusyError) return true; + if (error instanceof CodexCredentialRefreshStaleError) return true; + if (error instanceof CodexCredentialRefreshLockTimeoutError) return true; + return error instanceof Error && (error.name === "AbortError" || error.name === "TimeoutError"); +} + +function classifyCodexPoolRefreshFailureReason(error: unknown): string { + if (error instanceof TokenRefreshError) return error.reason; + if (error instanceof CodexCredentialGenerationConflictError) return "generation_conflict"; + return "network"; +} + /** Credential writers share the config mutation coordinator; contention is transient, not reauth. */ function withCredentialMutationLockSync(fn: () => T): T { try { @@ -797,6 +839,11 @@ export async function forceRefreshCodexPoolToken( settle({ kind: "failed", error: options.signal.reason }); throw options.signal.reason; } + if (isCodexPoolRefreshCooling(id)) { + const error = new CodexPoolRefreshCooldownError(); + settle({ kind: "failed", error }); + throw error; + } const completion = resolveCodexToken( id, { rejectedGeneration: options.rejectedGeneration, rejectedAccessToken: options.rejectedAccessToken }, @@ -805,13 +852,23 @@ export async function forceRefreshCodexPoolToken( undefined, ); completion.then( - resolved => settle({ - kind: "resolved", - provenance: classify(resolved), - generation: resolved.generation, - rotated: resolved.accessToken !== options.rejectedAccessToken, - }), - error => settle({ kind: "failed", error }), + resolved => { + clearCodexPoolRefreshFailure(id); + settle({ + kind: "resolved", + provenance: classify(resolved), + generation: resolved.generation, + rotated: resolved.accessToken !== options.rejectedAccessToken, + }); + }, + error => { + if (isTerminalCodexPoolRefreshFailure(error) || isOperationalCodexPoolRefreshFailure(error)) { + if (isTerminalCodexPoolRefreshFailure(error)) clearCodexPoolRefreshFailure(id); + } else { + noteCodexPoolRefreshFailure(id, classifyCodexPoolRefreshFailureReason(error)); + } + settle({ kind: "failed", error }); + }, ); const result = await awaitOwnCancellation(completion, options.signal); const provenance = classify(result); @@ -851,9 +908,9 @@ async function resolveCodexToken( if (callerSignal?.aborted) throw callerSignal.reason; const record = readCodexAccountRecord(id); const cred = record?.deletedAt == null ? record?.credential : undefined; - if (!record || !cred) throw new Error("Codex account credential is unavailable; reauthenticate the account."); + if (!record || !cred) throw new CodexCredentialUnavailableError(); const refreshGrantFingerprint = recordGrantFingerprint(record); - if (!refreshGrantFingerprint) throw new Error("Codex account credential is unavailable; reauthenticate the account."); + if (!refreshGrantFingerprint) throw new CodexCredentialUnavailableError(); // The freshness shortcut is exactly what makes a 401 on a time-valid token // unrecoverable, so a forced caller skips it — but only while the stored credential diff --git a/src/codex/pool-refresh-backoff.ts b/src/codex/pool-refresh-backoff.ts new file mode 100644 index 0000000000..342631d478 --- /dev/null +++ b/src/codex/pool-refresh-backoff.ts @@ -0,0 +1,100 @@ + +/** + * Per-account cooldown for a stored Codex pool credential whose forced refresh + * failed without proving the grant is dead. + * + * A token-endpoint 5xx, a generation CAS loss, or a network blip is transient + * (#2887): it must not quarantine the account or drop its binding. Retrying the + * same doomed refresh on every request, though, is how a single unhealthy + * account pinned the pool at 503 while healthy siblings sat idle. Consecutive + * non-terminal failures open a bounded growing cooldown; during that window no + * new forced refresh starts, and selection prefers a sibling. The first + * successful refresh clears it. + */ + +import { fallbackCodexAccountLogLabel } from "./account-label"; + +export const CODEX_POOL_REFRESH_INCOMPLETE_LOG_REASON = "codex_pool_refresh_incomplete"; + +/** Growing delays between forced-refresh attempts for one account. */ +export const CODEX_POOL_REFRESH_FAILURE_BACKOFF_MS = [2_000, 5_000, 15_000, 30_000, 60_000] as const; + +export class CodexPoolRefreshCooldownError extends Error { + readonly retryable = true; + readonly code = "CODEX_REFRESH_COOLING"; + + constructor(message = "Codex credential refresh is cooling down") { + super(message); + this.name = "CodexPoolRefreshCooldownError"; + } +} + +type RefreshFailureBackoff = { + consecutiveFailures: number; + cooldownUntil: number; + reason: string; +}; + +const backoffByAccount = new Map(); +let nowOverride: number | undefined; + +export function setCodexPoolRefreshFailureNowForTests(now?: number): void { + nowOverride = now; +} + +export function resetCodexPoolRefreshFailureBackoffForTests(): void { + backoffByAccount.clear(); + nowOverride = undefined; +} + +export function clearCodexPoolRefreshFailure(accountId: string): void { + backoffByAccount.delete(accountId); +} + +function currentNow(now?: number): number { + return now ?? nowOverride ?? Date.now(); +} + +function delayFor(consecutiveFailures: number): number { + const index = Math.min(Math.max(consecutiveFailures, 1), CODEX_POOL_REFRESH_FAILURE_BACKOFF_MS.length) - 1; + return CODEX_POOL_REFRESH_FAILURE_BACKOFF_MS[index]!; +} + +export function getCodexPoolRefreshCooldownUntil(accountId: string, now = currentNow()): number | null { + const entry = backoffByAccount.get(accountId); + if (!entry) return null; + return entry.cooldownUntil > now ? entry.cooldownUntil : null; +} + +export function isCodexPoolRefreshCooling(accountId: string, now = currentNow()): boolean { + return getCodexPoolRefreshCooldownUntil(accountId, now) !== null; +} + +/** + * Record a non-terminal forced-refresh failure. Already-cooling accounts do not + * grow the window: growth requires another real attempt after the previous one + * expired. Logs the classified reason once per account per window, with the + * durable hash label — never a token and never an email. + */ +export function noteCodexPoolRefreshFailure( + accountId: string, + reason: string, + now = currentNow(), +): { consecutiveFailures: number; cooldownUntil: number; openedWindow: boolean } { + const existing = backoffByAccount.get(accountId); + if (existing && existing.cooldownUntil > now) { + return { + consecutiveFailures: existing.consecutiveFailures, + cooldownUntil: existing.cooldownUntil, + openedWindow: false, + }; + } + const consecutiveFailures = (existing?.consecutiveFailures ?? 0) + 1; + const cooldownUntil = now + delayFor(consecutiveFailures); + backoffByAccount.set(accountId, { consecutiveFailures, cooldownUntil, reason }); + const label = fallbackCodexAccountLogLabel(accountId); + console.warn( + `[codex-auth] Codex pool account ${label} credential refresh failed (${reason})`, + ); + return { consecutiveFailures, cooldownUntil, openedWindow: true }; +} diff --git a/src/codex/routing.ts b/src/codex/routing.ts index 51779de4c1..7644ccb62b 100644 --- a/src/codex/routing.ts +++ b/src/codex/routing.ts @@ -38,6 +38,8 @@ import { isCanonicalOpenAiForwardProvider } from "../providers/openai-tiers"; import { retainedUtf8Bytes } from "../lib/admission"; import { recordUpstreamHostFailure } from "./upstream-host-health"; +import { isCodexPoolRefreshCooling } from "./pool-refresh-backoff"; + type ThreadAffinityEntry = { accountId: string; generation: number; @@ -1335,6 +1337,7 @@ function isCodexAccountSelectable( && getCodexQuotaHealthSnapshot(accountId, quotaScope, now) === null && !isCodexQuotaAvoided(accountId, quotaScope, now) && !isCodexAccountSoftAvoided(accountId, now) + && !isCodexPoolRefreshCooling(accountId, now) && isCodexAccountUsable(config, accountId, selectionOptions); } @@ -1360,6 +1363,7 @@ function codexAccountBlockReason( if (getCodexQuotaHealthSnapshot(accountId, quotaScope, now) !== null) return "cooldown"; if (isCodexQuotaAvoided(accountId, quotaScope, now)) return "quota_avoided"; if (isCodexAccountSoftAvoided(accountId, now)) return "transient"; + if (isCodexPoolRefreshCooling(accountId, now)) return "transient"; if (!isCodexAccountUsable(config, accountId, selectionOptions)) return "unusable"; return undefined; } @@ -1600,6 +1604,7 @@ function getEligiblePoolAccounts( .filter(account => getCodexQuotaHealthSnapshot(account.id, quotaScope, now) === null) .filter(account => !isCodexAccountSoftAvoided(account.id, now)) .filter(account => !isCodexQuotaAvoided(account.id, quotaScope, now)) + .filter(account => !isCodexPoolRefreshCooling(account.id, now)) .filter(account => isCodexAccountUsable(config, account.id, selectionOptions)) .map(account => account.id); // The main Codex account is not stored in config.codexAccounts; include it as a @@ -1616,6 +1621,7 @@ function getEligiblePoolAccounts( // earned it: the cooldown caps at fifteen minutes, the window runs up to six hours, and // in between the main account returns as a first-class candidate. && !isCodexQuotaAvoided(MAIN_CODEX_ACCOUNT_ID, quotaScope, now) + && !isCodexPoolRefreshCooling(MAIN_CODEX_ACCOUNT_ID, now) && (!skipFailoverReadyCandidates || !shouldFailover(config, MAIN_CODEX_ACCOUNT_ID, now)) && isCodexAccountUsable(config, MAIN_CODEX_ACCOUNT_ID, selectionOptions) ) { @@ -1725,7 +1731,9 @@ function isTransientOnlyAffinityBlock( if (!isCodexAccountUsable(config, entry.accountId, selectionOptions)) return false; if (getCodexQuotaHealthSnapshot(entry.accountId, quotaScope, now) !== null) return false; if (isCodexQuotaAvoided(entry.accountId, quotaScope, now)) return false; - return shouldFailover(config, entry.accountId, now) || isCodexAccountSoftAvoided(entry.accountId, now); + return shouldFailover(config, entry.accountId, now) + || isCodexAccountSoftAvoided(entry.accountId, now) + || isCodexPoolRefreshCooling(entry.accountId, now); } /** Has a held binding waited longer than a transient failure can reasonably explain? */ diff --git a/src/server/request-log.ts b/src/server/request-log.ts index 0449f81e5e..ebd0edb299 100644 --- a/src/server/request-log.ts +++ b/src/server/request-log.ts @@ -254,7 +254,11 @@ export interface RequestLogEntry { affinityReason?: CodexAffinityReason; /** Where the upstream terminal/failure was observed. */ transportPhase?: "pre_headers" | "mid_stream" | "terminal_sse"; - /** Whether the terminal came from a real upstream SSE event or a proxy synthetic tail. */ + /** + * Whether the HTTP status and message originated upstream or were synthesized by this + * proxy. Covers SSE tails and pre-stream JSON refusals. Management surfaces this so a + * local refusal cannot be presented as an upstream reason. + */ terminalSource?: "upstream" | "synthetic"; /** Bounded route-decision trace (RI-01); never contains secrets. */ routeDecision?: RouteDecisionTraceV1; @@ -815,6 +819,15 @@ export function usageFromResponsesPayload(usage: unknown): OcxUsage | undefined return undefined; } +/** + * Mark a refusal this proxy synthesized locally. Sets origin to `synthetic` and a + * distinct local reason so the request log cannot be read as an upstream overload. + */ +export function markLocalRequestLogRefusal(logCtx: RequestLogContext, reason: string): void { + logCtx.localTerminalReason = reason; + logCtx.terminalSource = "synthetic"; +} + export function inspectResponseLogJson(logCtx: RequestLogContext, text: string): void { try { applyResponseLogMetadata(logCtx, JSON.parse(text)); diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index abf699ef35..cab98df8db 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -1,4 +1,5 @@ import { capturePoolQuotaWriter } from "../../codex/account-store"; +import { CODEX_POOL_REFRESH_INCOMPLETE_LOG_REASON } from "../../codex/pool-refresh-backoff"; import type { Server } from "bun"; import { recordContextSessionOwner } from "../../codex/context-owner"; import { contextRelayActivated } from "../../codex/context-compat"; @@ -211,6 +212,7 @@ import { } from "../../codex/routing"; import { TokenRefreshError, + isTerminalCodexPoolRefreshFailure, forceRefreshCodexPoolToken, readCodexAccountRecord, } from "../../codex/account-store"; @@ -349,6 +351,7 @@ import { recordAttemptCredentialSource, usageFromResponsesPayload, type RequestLogContext, + markLocalRequestLogRefusal, } from "../request-log"; import { conversationIdFromResponsesRequest, @@ -2521,7 +2524,11 @@ async function resolveResponsesCodexAuth( * defect this path exists to fix (#2887). */ function isTerminalPoolRefreshFailure(error: unknown): boolean { - return error instanceof TokenRefreshError && (error.reason === "revoked" || error.reason === "expired"); + // Delegated so "terminal" has ONE definition. A missing record or a missing refresh-grant + // fingerprint is permanent -- retrying cannot conjure a credential -- and used to be a bare + // Error, which fell through to the retryable 503 and told the operator to keep retrying a + // request that could never succeed. + return isTerminalCodexPoolRefreshFailure(error); } /** @@ -2554,7 +2561,12 @@ export function poolCredentialRefreshIncompleteResponse(args: { authCtx: CodexAuthContext; config: Pick; accountSelector?: string; + logCtx?: RequestLogContext; }): Response { + // The wire contract below is unchanged on purpose, so the record has to carry the origin + // instead. Without it an operator reads this sentence under a field named "Upstream reason" + // and goes looking at the provider's status page for a refusal that never left this process. + if (args.logCtx) markLocalRequestLogRefusal(args.logCtx, CODEX_POOL_REFRESH_INCOMPLETE_LOG_REASON); const label = args.accountSelector ?? codexAuthContextLogLabel(args.authCtx, args.config); const account = label ? `Codex pool account ${label}` : "the selected Codex pool account"; const response = formatErrorResponse( @@ -2574,6 +2586,7 @@ export function poolCredentialRefreshIncompleteResponse(args: { * which must retire the account, from a transient failure, which must not. */ async function refreshPoolForwardAuth(args: { + logCtx?: RequestLogContext; req: Request; config: OcxConfig; route: RouteResult; @@ -2646,6 +2659,7 @@ async function refreshPoolForwardAuth(args: { authCtx, config, accountSelector: route.codexAccountNamespace, + logCtx: args.logCtx, }), }; } @@ -6002,7 +6016,7 @@ async function handleResponsesInner( try { void upstreamResponse.body?.cancel().catch(() => {}); } catch { /* already consumed */ } const poolAuthCtx = authCtx.kind === "pool" ? authCtx : undefined; const poolReplay = poolAuthCtx - ? await refreshPoolForwardAuth({ req, config, route, authCtx: poolAuthCtx, substituteMainCredential, options }) + ? await refreshPoolForwardAuth({ req, config, route, authCtx: poolAuthCtx, substituteMainCredential, options, logCtx }) : undefined; const replay = poolReplay ?? await refreshNativeMainForwardAuth({ req, config, route, authCtx, substituteMainCredential, options }); diff --git a/tests/codex-integration/codex-pool-refresh-backoff.test.ts b/tests/codex-integration/codex-pool-refresh-backoff.test.ts new file mode 100644 index 0000000000..1810fe2666 --- /dev/null +++ b/tests/codex-integration/codex-pool-refresh-backoff.test.ts @@ -0,0 +1,116 @@ +import { describe, expect, test, beforeEach } from "bun:test"; +import { + CODEX_POOL_REFRESH_FAILURE_BACKOFF_MS, + CodexPoolRefreshCooldownError, + clearCodexPoolRefreshFailure, + getCodexPoolRefreshCooldownUntil, + isCodexPoolRefreshCooling, + noteCodexPoolRefreshFailure, + resetCodexPoolRefreshFailureBackoffForTests, + setCodexPoolRefreshFailureNowForTests, +} from "../../src/codex/pool-refresh-backoff"; +import { + CodexCredentialGenerationConflictError, + CodexCredentialUnavailableError, + TokenRefreshError, + isTerminalCodexPoolRefreshFailure, +} from "../../src/codex/account-store"; + +/** + * #4546: a pool account whose forced refresh failed answered every subsequent request with a + * retryable 503 whose body asked the client to retry, so the loop sustained the very condition + * it was waiting out while six healthy siblings sat idle. Reproduced live: five sequential + * probes, five 503s, and not one line in the service log. + */ +describe("codex pool refresh failure backoff", () => { + beforeEach(() => { + resetCodexPoolRefreshFailureBackoffForTests(); + }); + + test("consecutive failures open a bounded, growing cooldown", () => { + const now = 1_000_000; + setCodexPoolRefreshFailureNowForTests(now); + + const first = noteCodexPoolRefreshFailure("acct-a", "unknown"); + expect(first.openedWindow).toBe(true); + expect(first.consecutiveFailures).toBe(1); + expect(first.cooldownUntil).toBe(now + CODEX_POOL_REFRESH_FAILURE_BACKOFF_MS[0]!); + expect(isCodexPoolRefreshCooling("acct-a")).toBe(true); + + // A second failure INSIDE the window does not grow it. Growth requires another real + // attempt, otherwise a burst of concurrent requests would race the account to the ceiling. + const during = noteCodexPoolRefreshFailure("acct-a", "unknown"); + expect(during.openedWindow).toBe(false); + expect(during.consecutiveFailures).toBe(1); + expect(during.cooldownUntil).toBe(first.cooldownUntil); + + setCodexPoolRefreshFailureNowForTests(first.cooldownUntil + 1); + expect(isCodexPoolRefreshCooling("acct-a")).toBe(false); + + const second = noteCodexPoolRefreshFailure("acct-a", "unknown"); + expect(second.consecutiveFailures).toBe(2); + expect(second.cooldownUntil - (first.cooldownUntil + 1)) + .toBe(CODEX_POOL_REFRESH_FAILURE_BACKOFF_MS[1]!); + }); + + test("the cooldown is bounded by the last configured step", () => { + let now = 0; + setCodexPoolRefreshFailureNowForTests(now); + const ceiling = CODEX_POOL_REFRESH_FAILURE_BACKOFF_MS[CODEX_POOL_REFRESH_FAILURE_BACKOFF_MS.length - 1]!; + for (let attempt = 0; attempt < CODEX_POOL_REFRESH_FAILURE_BACKOFF_MS.length + 3; attempt += 1) { + const opened = noteCodexPoolRefreshFailure("acct-ceiling", "unknown", now); + expect(opened.cooldownUntil - now).toBeLessThanOrEqual(ceiling); + now = opened.cooldownUntil + 1; + setCodexPoolRefreshFailureNowForTests(now); + } + }); + + test("a success clears the cooldown so recovery is automatic", () => { + const now = 5_000; + setCodexPoolRefreshFailureNowForTests(now); + noteCodexPoolRefreshFailure("acct-b", "generation_conflict"); + expect(isCodexPoolRefreshCooling("acct-b")).toBe(true); + + clearCodexPoolRefreshFailure("acct-b"); + expect(isCodexPoolRefreshCooling("acct-b")).toBe(false); + expect(getCodexPoolRefreshCooldownUntil("acct-b")).toBeNull(); + }); + + test("one account cooling never cools a sibling", () => { + setCodexPoolRefreshFailureNowForTests(10_000); + noteCodexPoolRefreshFailure("acct-broken", "unknown"); + expect(isCodexPoolRefreshCooling("acct-broken")).toBe(true); + // The whole point of the cooldown is that selection moves to a healthy sibling. + expect(isCodexPoolRefreshCooling("acct-healthy")).toBe(false); + }); + + test("the cooldown error is retryable and does not claim reauthentication", () => { + const error = new CodexPoolRefreshCooldownError(); + expect(error.retryable).toBe(true); + // A body carrying "reauthentication" is reclassified away from server_is_overloaded, which + // would disable the retry-after backoff this refusal exists to ask for. + expect(error.message.toLowerCase()).not.toContain("reauthentication"); + expect(isTerminalCodexPoolRefreshFailure(error)).toBe(false); + }); +}); + +describe("terminal has one definition", () => { + test("a missing credential or grant fingerprint is terminal, not retryable", () => { + // This is the case that made the live incident unrecoverable: it was thrown as a bare + // Error, classified transient, and answered with a 503 asking the client to keep retrying + // a request that could never succeed. + expect(isTerminalCodexPoolRefreshFailure(new CodexCredentialUnavailableError())).toBe(true); + }); + + test("a dead grant is terminal", () => { + expect(isTerminalCodexPoolRefreshFailure(new TokenRefreshError("revoked", "x"))).toBe(true); + expect(isTerminalCodexPoolRefreshFailure(new TokenRefreshError("expired", "x"))).toBe(true); + }); + + test("a token-endpoint 5xx and a CAS loss stay transient", () => { + // #2887: a token-endpoint failure must not retire a healthy account. + expect(isTerminalCodexPoolRefreshFailure(new TokenRefreshError("unknown", "x"))).toBe(false); + expect(isTerminalCodexPoolRefreshFailure(new CodexCredentialGenerationConflictError())).toBe(false); + }); +}); + diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 27c6af6716..cef91aa256 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1282,5 +1282,6 @@ "adapter-input-media-guard.test.ts": "adapters", "chat-media-translation.test.ts": "responses", "execution-budget-permits.test.ts": "lib", - "spend-instrumentation-log.test.ts": "server" + "spend-instrumentation-log.test.ts": "server", + "codex-pool-refresh-backoff.test.ts": "codex-integration" } From 4c0ac81efdb1944e93dd1297a75243ed77301ca0 Mon Sep 17 00:00:00 2001 From: JUN Date: Tue, 15 Sep 2026 01:30:12 +0900 Subject: [PATCH 2/3] fix(codex): withhold a pool refresh only after repeated failures (#4546) Hosted CI failed six rows of the pool 401 refresh suite with 503 where 401 or 200 were expected. Withholding on the FIRST non-terminal failure was wrong twice over: a single token-endpoint blip is the ordinary case the next attempt clears, and a withheld refresh never runs, so an account whose grant is actually revoked could no longer discover that - the terminal 401 it owes the operator became a retryable 503 that never resolves. The cooldown now withholds only after three consecutive failures, and the do-not-grow-inside-the-window rule applies only while it is actually withholding, so a client retrying once a second can still reach the threshold. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. --- src/codex/pool-refresh-backoff.ts | 20 ++++++++- .../codex-pool-refresh-backoff.test.ts | 44 ++++++++++++------- 2 files changed, 48 insertions(+), 16 deletions(-) diff --git a/src/codex/pool-refresh-backoff.ts b/src/codex/pool-refresh-backoff.ts index 342631d478..dd260ca2d5 100644 --- a/src/codex/pool-refresh-backoff.ts +++ b/src/codex/pool-refresh-backoff.ts @@ -60,9 +60,21 @@ function delayFor(consecutiveFailures: number): number { return CODEX_POOL_REFRESH_FAILURE_BACKOFF_MS[index]!; } +/** + * How many consecutive non-terminal failures must land before a refresh is WITHHELD. + * + * Withholding on the first failure was wrong twice over. A single token-endpoint blip is the + * ordinary case that the very next attempt clears, and -- worse -- a withheld refresh never runs, + * so an account whose grant is actually revoked can no longer discover that: the terminal 401 it + * owes the operator turns into a retryable 503 that never resolves. The cooldown exists for the + * account that keeps failing, not for the one that failed once. + */ +export const CODEX_POOL_REFRESH_COOLDOWN_AFTER_FAILURES = 3; + export function getCodexPoolRefreshCooldownUntil(accountId: string, now = currentNow()): number | null { const entry = backoffByAccount.get(accountId); if (!entry) return null; + if (entry.consecutiveFailures < CODEX_POOL_REFRESH_COOLDOWN_AFTER_FAILURES) return null; return entry.cooldownUntil > now ? entry.cooldownUntil : null; } @@ -82,7 +94,13 @@ export function noteCodexPoolRefreshFailure( now = currentNow(), ): { consecutiveFailures: number; cooldownUntil: number; openedWindow: boolean } { const existing = backoffByAccount.get(accountId); - if (existing && existing.cooldownUntil > now) { + // The "do not grow inside an open window" rule applies only once the window is actually + // WITHHOLDING. Below the threshold no refresh is being withheld, so every failure is a real + // attempt that really failed and must count -- otherwise a client retrying the 503 once a + // second can never reach the threshold the cooldown is meant to protect against. + const withholding = existing !== undefined + && existing.consecutiveFailures >= CODEX_POOL_REFRESH_COOLDOWN_AFTER_FAILURES; + if (existing && withholding && existing.cooldownUntil > now) { return { consecutiveFailures: existing.consecutiveFailures, cooldownUntil: existing.cooldownUntil, diff --git a/tests/codex-integration/codex-pool-refresh-backoff.test.ts b/tests/codex-integration/codex-pool-refresh-backoff.test.ts index 1810fe2666..eca22b97a8 100644 --- a/tests/codex-integration/codex-pool-refresh-backoff.test.ts +++ b/tests/codex-integration/codex-pool-refresh-backoff.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test, beforeEach } from "bun:test"; import { + CODEX_POOL_REFRESH_COOLDOWN_AFTER_FAILURES, CODEX_POOL_REFRESH_FAILURE_BACKOFF_MS, CodexPoolRefreshCooldownError, clearCodexPoolRefreshFailure, @@ -31,26 +32,34 @@ describe("codex pool refresh failure backoff", () => { const now = 1_000_000; setCodexPoolRefreshFailureNowForTests(now); - const first = noteCodexPoolRefreshFailure("acct-a", "unknown"); - expect(first.openedWindow).toBe(true); - expect(first.consecutiveFailures).toBe(1); - expect(first.cooldownUntil).toBe(now + CODEX_POOL_REFRESH_FAILURE_BACKOFF_MS[0]!); + // The first failures do NOT withhold anything. One token-endpoint blip is the ordinary case + // the next attempt clears, and a withheld refresh never runs -- so withholding early would + // stop a revoked grant from ever being discovered and turn its terminal 401 into a 503 that + // never resolves. + for (let attempt = 1; attempt < CODEX_POOL_REFRESH_COOLDOWN_AFTER_FAILURES; attempt += 1) { + const early = noteCodexPoolRefreshFailure("acct-a", "unknown"); + expect(early.consecutiveFailures).toBe(attempt); + expect(isCodexPoolRefreshCooling("acct-a")).toBe(false); + } + + const opened = noteCodexPoolRefreshFailure("acct-a", "unknown"); + expect(opened.consecutiveFailures).toBe(CODEX_POOL_REFRESH_COOLDOWN_AFTER_FAILURES); expect(isCodexPoolRefreshCooling("acct-a")).toBe(true); + expect(getCodexPoolRefreshCooldownUntil("acct-a")).toBe(opened.cooldownUntil); - // A second failure INSIDE the window does not grow it. Growth requires another real - // attempt, otherwise a burst of concurrent requests would race the account to the ceiling. + // Once it IS withholding, a further failure inside the window does not grow it: growth needs + // another real attempt, or a burst of concurrent requests would race it to the ceiling. const during = noteCodexPoolRefreshFailure("acct-a", "unknown"); expect(during.openedWindow).toBe(false); - expect(during.consecutiveFailures).toBe(1); - expect(during.cooldownUntil).toBe(first.cooldownUntil); + expect(during.consecutiveFailures).toBe(CODEX_POOL_REFRESH_COOLDOWN_AFTER_FAILURES); + expect(during.cooldownUntil).toBe(opened.cooldownUntil); - setCodexPoolRefreshFailureNowForTests(first.cooldownUntil + 1); + setCodexPoolRefreshFailureNowForTests(opened.cooldownUntil + 1); expect(isCodexPoolRefreshCooling("acct-a")).toBe(false); - const second = noteCodexPoolRefreshFailure("acct-a", "unknown"); - expect(second.consecutiveFailures).toBe(2); - expect(second.cooldownUntil - (first.cooldownUntil + 1)) - .toBe(CODEX_POOL_REFRESH_FAILURE_BACKOFF_MS[1]!); + const next = noteCodexPoolRefreshFailure("acct-a", "unknown"); + expect(next.consecutiveFailures).toBe(CODEX_POOL_REFRESH_COOLDOWN_AFTER_FAILURES + 1); + expect(isCodexPoolRefreshCooling("acct-a")).toBe(true); }); test("the cooldown is bounded by the last configured step", () => { @@ -61,6 +70,7 @@ describe("codex pool refresh failure backoff", () => { const opened = noteCodexPoolRefreshFailure("acct-ceiling", "unknown", now); expect(opened.cooldownUntil - now).toBeLessThanOrEqual(ceiling); now = opened.cooldownUntil + 1; + // Below the threshold nothing is withheld, so the window is advisory until it opens. setCodexPoolRefreshFailureNowForTests(now); } }); @@ -68,7 +78,9 @@ describe("codex pool refresh failure backoff", () => { test("a success clears the cooldown so recovery is automatic", () => { const now = 5_000; setCodexPoolRefreshFailureNowForTests(now); - noteCodexPoolRefreshFailure("acct-b", "generation_conflict"); + for (let i = 0; i < CODEX_POOL_REFRESH_COOLDOWN_AFTER_FAILURES; i += 1) { + noteCodexPoolRefreshFailure("acct-b", "generation_conflict", now + i); + } expect(isCodexPoolRefreshCooling("acct-b")).toBe(true); clearCodexPoolRefreshFailure("acct-b"); @@ -78,7 +90,9 @@ describe("codex pool refresh failure backoff", () => { test("one account cooling never cools a sibling", () => { setCodexPoolRefreshFailureNowForTests(10_000); - noteCodexPoolRefreshFailure("acct-broken", "unknown"); + for (let i = 0; i < CODEX_POOL_REFRESH_COOLDOWN_AFTER_FAILURES; i += 1) { + noteCodexPoolRefreshFailure("acct-broken", "unknown", 10_000 + i); + } expect(isCodexPoolRefreshCooling("acct-broken")).toBe(true); // The whole point of the cooldown is that selection moves to a healthy sibling. expect(isCodexPoolRefreshCooling("acct-healthy")).toBe(false); From 6966470e639bc63e88d96ed356c4fb5bc3106703 Mon Sep 17 00:00:00 2001 From: JUN Date: Tue, 15 Sep 2026 01:48:17 +0900 Subject: [PATCH 3/3] fix(codex): drop refresh cooldowns when the routing layer clears its account state (#4546) A cooldown is per-account runtime state learned alongside the thread bindings, but it outlived clearThreadAccountMap. An account that had failed a refresh therefore stayed out of selection after the roster it belonged to was gone - which is what kept a replayed account unselectable on the NEXT request in the pool 401 suite. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. --- src/codex/pool-refresh-backoff.ts | 9 +++++++++ src/codex/routing.ts | 5 ++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/codex/pool-refresh-backoff.ts b/src/codex/pool-refresh-backoff.ts index dd260ca2d5..410d11600a 100644 --- a/src/codex/pool-refresh-backoff.ts +++ b/src/codex/pool-refresh-backoff.ts @@ -51,6 +51,15 @@ export function clearCodexPoolRefreshFailure(accountId: string): void { backoffByAccount.delete(accountId); } +/** + * Drop every remembered failure. Called when the routing layer discards its per-account state, + * because a cooldown outliving the binding it was learned alongside would keep an account out of + * selection for a roster the operator has already replaced. + */ +export function clearAllCodexPoolRefreshFailures(): void { + backoffByAccount.clear(); +} + function currentNow(now?: number): number { return now ?? nowOverride ?? Date.now(); } diff --git a/src/codex/routing.ts b/src/codex/routing.ts index 7644ccb62b..8fd3582fbe 100644 --- a/src/codex/routing.ts +++ b/src/codex/routing.ts @@ -38,7 +38,7 @@ import { isCanonicalOpenAiForwardProvider } from "../providers/openai-tiers"; import { retainedUtf8Bytes } from "../lib/admission"; import { recordUpstreamHostFailure } from "./upstream-host-health"; -import { isCodexPoolRefreshCooling } from "./pool-refresh-backoff"; +import { clearAllCodexPoolRefreshFailures, isCodexPoolRefreshCooling } from "./pool-refresh-backoff"; type ThreadAffinityEntry = { accountId: string; @@ -430,6 +430,9 @@ export function listLiveCodexAccountIds(config: OcxConfig): ReadonlySet export function clearThreadAccountMap(): void { threadAccountMap.clear(); threadAffinityEntryTotal = 0; + // A refresh cooldown is per-account runtime state learned alongside these bindings. Leaving it + // behind here keeps an account out of selection after the roster it belonged to is gone. + clearAllCodexPoolRefreshFailures(); } export function clearThreadAccountMapForAccount(