From 610824401a488515f6e451d7b926b928ee9d005a Mon Sep 17 00:00:00 2001 From: Vadym O Date: Sat, 3 Oct 2026 18:16:53 +0800 Subject: [PATCH 1/2] fix(codex): keep native main in Pool health on translated Claude turns A Claude Code turn routed through a combo whose first leg is the canonical ChatGPT forward provider sent a revoked credential upstream on every single request, indefinitely, and reported it as a provider error. The Claude Messages ingress attaches the stored ~/.codex credential to a translated turn so forward sidecars stay reachable, and `codexRouteCredentialDomainHeaders` puts it on the headers the Codex auth resolution reads. `hasForwardableCodexBearer` then sees a Codex JWT carrying an account id and cannot tell our credential from one the client supplied, so `requestScopedMainCredential` came back true and the native main slot resolved as a caller-owned `main` context instead of `main-pool`. Caller-owned credentials deliberately own no Pool state, so the upstream 401 recorded no outcome, attempted no refresh, retired nothing, and the next request sent the same dead token again. The ingress already states the rule it needs here -- it passes `nativeCallerAuth` and `callerDirectAuth` as null because its stored-main enrichment is not an original caller credential -- so ownership is now decided by provenance where both the resolution and the lineage preview read it, and they cannot disagree. A bearer the client really did supply sets no enrichment and stays caller-owned. With the slot back in Pool health, the rest of the path had to work: - A refresh refusal is classified from the token endpoint's structured `error` code alone, by one rule now shared with the stored-pool refresh, so one dead grant cannot be terminal for a Pool account and transient for main. The native-main classifier searched its own formatted message, which missed `refresh_token_reused` and read a 5xx's "session expired" prose as proof. - A code-confirmed refusal marks the account AND retires the grant by fingerprint, because a present refresh grant otherwise cancels the quarantine it just earned. The verdict survives a reauth clear -- the WHAM probe retracts quarantines on an explicit refresh, which an open dashboard triggers -- and is retracted only by a successful refresh or a replacement credential. - The refusal says what happened and what fixes it, on the request that discovers it and on every request after, instead of "needs reauthentication" once and "no usable account credential" forever. - Each refresh verdict logs once with the endpoint status and code and no credential material. This is what identified the live fault. - The combo failure warning is one line again: it preferred the whole error envelope, so a single 401 printed a multi-line JSON body across the log. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/codex/account-runtime-state.ts | 29 ++ src/codex/account-store.ts | 108 ++++--- src/codex/auth-context.ts | 18 +- src/codex/main-account.ts | 113 ++++++- src/server/responses/codex-auth-error.ts | 5 +- src/server/responses/core-auth.ts | 32 +- src/server/responses/core-combo-failure.ts | 1 + src/server/responses/core-combo.ts | 16 +- src/server/responses/core-options.ts | 6 + structure/providers/openai-accounts.md | 14 + .../codex-account-unusable-reason.test.ts | 6 +- .../responses-native-main-refresh.test.ts | 286 +++++++++++++++++- 12 files changed, 573 insertions(+), 61 deletions(-) diff --git a/src/codex/account-runtime-state.ts b/src/codex/account-runtime-state.ts index 35f3c9cca53..e79da5c3ea8 100644 --- a/src/codex/account-runtime-state.ts +++ b/src/codex/account-runtime-state.ts @@ -17,6 +17,28 @@ import { isCodexAccountGenerationLive } from "./account-store"; const reauthAccounts = new Map(); let lastReconciledGeneration = 0; let liveAccountIds = new Set(); +/** + * Fingerprint of the native-main refresh grant the token endpoint itself refused. + * + * A main quarantine is deliberately overridable by the presence of a refresh grant + * ({@link import("./main-account").hasMainAccountRefreshGrant}), because a bare WHAM 401 can + * quarantine a credential the next refresh would have fixed. A grant the token endpoint answered + * with `invalid_grant` can fix nothing, so it must stop vouching for the account — otherwise a + * revoked session is rediscovered by an upstream round trip on every single request. + * + * A fingerprint, never a token, and never the account id: a replacement credential written by + * login, reauth, or another process carries a different grant and makes this verdict inert. + */ +let deadMainRefreshGrant: string | undefined; + +/** `undefined` retracts the verdict: a refresh that succeeded proved the grant is alive. */ +export function setMainRefreshGrantDead(fingerprint: string | undefined): void { + deadMainRefreshGrant = fingerprint; +} + +export function isMainRefreshGrantDead(fingerprint: string): boolean { + return deadMainRefreshGrant === fingerprint; +} export function markAccountNeedsReauth( id: string, @@ -67,4 +89,11 @@ export function clearAccountNeedsReauth(id: string, credentialGeneration?: numbe && (reauthAccounts.get(id) !== credentialGeneration || !isCodexAccountGenerationLive(id, credentialGeneration))) return; reauthAccounts.delete(id); + // The dead-grant verdict deliberately SURVIVES this. It is a fact about one refresh token, not + // about the quarantine that accompanied it, and it already self-invalidates by fingerprint when + // the credential is replaced. Retracting it here let the WHAM probe's explicit-refresh clear + // (#327) -- which `ocx account list` and an open dashboard both trigger -- hand a grant upstream + // had already refused back to routing, and the probe's own re-mark could not stop it: with a + // grant apparently alive, `hasMainAccountRefreshGrant` cancels the quarantine. Every look at the + // dashboard bought another upstream 401. } diff --git a/src/codex/account-store.ts b/src/codex/account-store.ts index 8a019d77005..4e1e811bd56 100644 --- a/src/codex/account-store.ts +++ b/src/codex/account-store.ts @@ -524,6 +524,69 @@ export function tombstoneCodexAccount(id: string): number { const CHATGPT_TOKEN_URL = "https://auth.openai.com/oauth/token"; const CHATGPT_CLIENT_ID = "app_EMoamEEZ73f0CkXaXp7hrann"; +/** + * What a failed ChatGPT token-endpoint refresh says about the refresh grant itself. + * + * `invalid_grant` is the standard OAuth code for a refresh token that is no longer usable, and + * upstream sends it bare with no description. Without it here the dead grant is classified + * "unknown", which callers treat as transient — so the account is never retired and every + * request repeats the same doomed refresh (#2887). Production only ever confirmed that bare + * code, so the siblings upstream also emits for a revoked or downgraded session are listed + * beside it rather than discovered one incident at a time. + * + * Matched on the exact `error` CODE, not anywhere in the combined text: a transient + * `server_error` whose description happens to mention invalid_grant would otherwise retire a + * healthy account, which is the failure this whole rule exists to remove. + * + * That rule binds the DESCRIPTION words too. "invalidated", "revoked" and "expired" read as + * terminal prose, but upstream puts arbitrary text there: a `server_error` whose description + * says "token was revoked" or "session expired" is still a 5xx blip, and retiring the account on + * it is exactly the false quarantine #2887 exists to prevent. So a body that carries a + * structured code is classified by that code ALONE. The substring fallback survives only where + * there is no structured code to read at all -- a description-only body, or one this parser + * could not decode -- because there the prose is the only signal upstream gave us. + * + * Shared by the stored-pool refresh and the native-main refresh. Native main classified the same + * failures by searching its own formatted message, which both missed `refresh_token_reused` and + * read a 5xx's terminal-sounding prose as proof -- so one dead grant could be terminal for a + * pool account and transient for `__main__`, and a live one could be retired. + */ +export function classifyChatgptRefreshFailure( + status: number, + body: string, +): { reason: "expired" | "revoked" | "unknown"; description: string; code?: string } { + let errDesc: string; + let errCodeExact: string | undefined; + try { + const parsed = JSON.parse(body) as { + error?: string | { code?: string; message?: string }; + error_description?: string; + }; + if (typeof parsed.error === "string") { + errCodeExact = parsed.error.trim(); + errDesc = [parsed.error, parsed.error_description].filter(Boolean).join(": "); + } else if (parsed.error && typeof parsed.error === "object") { + errCodeExact = typeof parsed.error.code === "string" ? parsed.error.code.trim() : undefined; + errDesc = [parsed.error.code, parsed.error.message, parsed.error_description].filter(Boolean).join(": "); + } else { + errDesc = parsed.error_description || `HTTP ${status}`; + } + if (!errDesc) errDesc = `HTTP ${status}`; + } catch { errDesc = `HTTP ${status}`; } + const structuredCode = errCodeExact ? errCodeExact : undefined; + const proseIsOnlySignal = structuredCode === undefined; + const reason = structuredCode === "invalid_grant" + || structuredCode === "refresh_token_invalidated" + || structuredCode === "token_invalidated" + || structuredCode === "refresh_token_reused" + || (proseIsOnlySignal + && (errDesc.includes("invalidated") || errDesc.includes("revoked"))) ? "revoked" as const + : structuredCode === "refresh_token_expired" + || (proseIsOnlySignal && errDesc.includes("expired")) ? "expired" as const + : "unknown" as const; + return { reason, description: errDesc, ...(structuredCode ? { code: structuredCode } : {}) }; +} + export class TokenRefreshError extends Error { reason: "expired" | "revoked" | "unknown"; constructor(reason: "expired" | "revoked" | "unknown", message: string) { @@ -1323,50 +1386,7 @@ async function resolveCodexToken( }); if (!res.ok) { const errText = await res.text().catch(() => ""); - let errDesc: string; - let errCodeExact: string | undefined; - try { - const parsed = JSON.parse(errText) as { - error?: string | { code?: string; message?: string }; - error_description?: string; - }; - if (typeof parsed.error === "string") { - errCodeExact = parsed.error.trim(); - errDesc = [parsed.error, parsed.error_description].filter(Boolean).join(": "); - } else if (parsed.error && typeof parsed.error === "object") { - errCodeExact = typeof parsed.error.code === "string" ? parsed.error.code.trim() : undefined; - errDesc = [parsed.error.code, parsed.error.message, parsed.error_description].filter(Boolean).join(": "); - } else { - errDesc = parsed.error_description || `HTTP ${res.status}`; - } - if (!errDesc) errDesc = `HTTP ${res.status}`; - } catch { errDesc = `HTTP ${res.status}`; } - // `invalid_grant` is the standard OAuth code for a refresh token that is no longer - // usable, and upstream sends it bare with no description. Without it here the dead - // grant is classified "unknown", which callers treat as transient — so the account - // is never retired and every request repeats the same doomed refresh (#2887). - // - // Matched on the exact `error` CODE, not anywhere in the combined text: a transient - // `server_error` whose description happens to mention invalid_grant would otherwise - // retire a healthy account, which is the failure this whole change exists to remove. - // - // That rule binds the DESCRIPTION words too. "invalidated", "revoked" and "expired" read - // as terminal prose, but upstream puts arbitrary text there: a `server_error` whose - // description says "token was revoked" or "session expired" is still a 5xx blip, and - // retiring the account on it is exactly the false quarantine #2887 exists to prevent. - // So a body that carries a structured code is classified by that code ALONE. The - // substring fallback survives only where there is no structured code to read at all -- - // a description-only body, or one this parser could not decode -- because there the - // prose is the only signal upstream gave us. - const structuredCode = errCodeExact ? errCodeExact : undefined; - const proseIsOnlySignal = structuredCode === undefined; - const reason = structuredCode === "invalid_grant" - || structuredCode === "refresh_token_invalidated" - || (proseIsOnlySignal - && (errDesc.includes("invalidated") || errDesc.includes("revoked"))) ? "revoked" as const - : structuredCode === "refresh_token_expired" - || (proseIsOnlySignal && errDesc.includes("expired")) ? "expired" as const - : "unknown" as const; + const { reason } = classifyChatgptRefreshFailure(res.status, errText); throw new TokenRefreshError(reason, `Codex token refresh failed (${reason}); reauthenticate the account.`); } const data = (await res.json()) as { access_token: string; refresh_token?: string; expires_in: number }; diff --git a/src/codex/auth-context.ts b/src/codex/auth-context.ts index f2355961567..8588c1b6ddc 100644 --- a/src/codex/auth-context.ts +++ b/src/codex/auth-context.ts @@ -385,6 +385,18 @@ export class CodexPoolAuthenticationError extends Error { } } +/** + * The refusal a revoked or downgraded ChatGPT session earns, on the request that discovers it + * and on every request after. + * + * "No usable account credential" is true and useless: it reads as a proxy fault and sends the + * operator to the provider's status page. The two things they need are that the credential was + * rejected by OpenAI rather than by this proxy, and the command that fixes it. + */ +export const CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE = + "Codex account needs sign-in: OpenAI rejected its credential (token invalidated — revoked " + + "session or plan change). Run `codex login` to sign in again."; + export type CodexModelAvailabilityReason = "unsupported" | "temporarily_unavailable"; /** A model/account compatibility failure is not a credential failure. */ @@ -1341,7 +1353,11 @@ export async function resolveCodexAuthContext( : "Codex accounts that support this model are currently unavailable", ); } - throw new CodexPoolAuthenticationError(); + // The pool is empty because a credential was quarantined, not because none is configured. + // Say which, so the refusal names the thing the operator has to do. + throw new CodexPoolAuthenticationError( + isAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID) ? CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE : undefined, + ); } accountId = selected; if (accountId === MAIN_CODEX_ACCOUNT_ID) assertMainAccountPolicy(policy); diff --git a/src/codex/main-account.ts b/src/codex/main-account.ts index 09a11eaa86a..d2dd0b2815a 100644 --- a/src/codex/main-account.ts +++ b/src/codex/main-account.ts @@ -3,21 +3,29 @@ import { readFileSync, statSync } from "node:fs"; import { join } from "node:path"; import { readCodexTokens } from "./auth-collision"; import { + CHATGPT_CLIENT_ID, + CHATGPT_TOKEN_URL, + credsFromToken, decodeJwtPayload, extractAccountId, - refreshChatGPTToken, } from "../oauth/chatgpt"; import type { OAuthCredentials } from "../oauth/types"; import { extractChatgptPlanType } from "./plan"; import { MAIN_CODEX_ACCOUNT_ID } from "./account-id"; import { + classifyChatgptRefreshFailure, refreshGrantFingerprintForToken, withCodexRefreshFileLock, } from "./account-store"; import { atomicWriteFile, resolveWriteTarget } from "../config/atomic-write"; import { resolveCodexHomeDir } from "./home"; import { assertNotRealCodexHomeUnderTest } from "../lib/test-home-guard"; -import { clearAccountNeedsReauth } from "./account-runtime-state"; +import { + clearAccountNeedsReauth, + isMainRefreshGrantDead, + markAccountNeedsReauth, + setMainRefreshGrantDead, +} from "./account-runtime-state"; import { advanceCodexCredentialMutationEpoch } from "./credential-mutation-epoch"; import { withNativeMainExclusiveClaim } from "./native-main-claim"; import { resolveNativeProfileContext } from "./native-profile-store"; @@ -78,6 +86,61 @@ export class MainAccountTokenRefreshError extends Error { } } +/** + * The native-main refresh, posted here rather than through `refreshChatGPTToken` so the token + * endpoint's structured `error` code survives to {@link classifyChatgptRefreshFailure}. + * + * `refreshChatGPTToken` collapses the refusal into one display string, and classifying a grant + * from that string is what let `refresh_token_reused` pass as transient -- the grant never + * retired, every request re-earning its own 401 -- while a 5xx whose description merely read + * "session expired" passed as proof and would have retired a live grant. + * + * Same endpoint, client id and grant type as every other ChatGPT refresh, and the success path + * is `credsFromToken` itself, so only the failure branch differs: it classifies by code, exactly + * as the stored-pool refresh does. + */ +async function refreshNativeMainGrant( + refreshToken: string, + options: { signal: AbortSignal }, +): Promise { + const resp = await fetch(CHATGPT_TOKEN_URL, { + method: "POST", + headers: { "Content-Type": "application/x-www-form-urlencoded" }, + body: new URLSearchParams({ + grant_type: "refresh_token", + client_id: CHATGPT_CLIENT_ID, + refresh_token: refreshToken, + }).toString(), + signal: options.signal, + }); + if (!resp.ok) { + const body = await resp.text().catch(() => ""); + const { reason, description, code } = classifyChatgptRefreshFailure(resp.status, body); + const verdict = reason === "unknown" ? "transient" as const : "reauth" as const; + noteNativeMainRefresh(verdict, resp.status, code); + throw new MainAccountTokenRefreshError(verdict, { + cause: new Error(`ChatGPT refresh failed: ${resp.status} ${description}`), + }); + } + noteNativeMainRefresh("ok", resp.status); + return credsFromToken((await resp.json()) as Record); +} + +/** + * One line per native-main refresh verdict, so a dead grant is diagnosable from service.log. + * + * A revoked session is invisible from the outside: the grant is refused, nothing is written, and + * the request fails with a status that looks like every other 401. Carries the token endpoint's + * HTTP status and its structured `error` code and nothing else -- never a token, never a body. + */ +function noteNativeMainRefresh(outcome: string, status?: number, code?: string): void { + console.warn( + `[codex] native main refresh: ${outcome}` + + (status === undefined ? "" : ` status=${status}`) + + ` code=${code && /^[A-Za-z0-9._-]{1,64}$/.test(code) ? code : "none"}`, + ); +} + function nonEmptyString(value: unknown): string | undefined { return typeof value === "string" && value.trim() ? value : undefined; } @@ -153,7 +216,10 @@ export function isMainAccountCredentialUsable(now = Date.now()): boolean { } export function hasMainAccountRefreshGrant(): boolean { - return !!readMainAuthJsonCredential()?.refreshToken; + const refreshToken = readMainAuthJsonCredential()?.refreshToken; + // A grant the token endpoint has already refused cannot revive the account, so it must not + // override the quarantine that refusal produced. + return !!refreshToken && !isMainRefreshGrantDead(refreshGrantFingerprintForToken(refreshToken)); } function assertMainAuthJsonSnapshotUnchanged(expected: MainAuthJsonCredential): void { @@ -343,13 +409,23 @@ async function resolveMainAccountToken( rejectedAccessToken?: string, ): Promise<{ accessToken: string; chatgptAccountId: string } | null> { const initial = readMainAuthJsonCredential(); - if (!initial) return null; + // Every skip below ends a forced refresh WITHOUT asking the token endpoint anything, which from + // the outside is indistinguishable from a refusal: same failed request, no write, no verdict. + // A forced refresh is the only caller that passes `rejectedAccessToken`, so only it reports. + if (!initial) { + if (rejectedAccessToken !== undefined) noteNativeMainRefresh("skipped-no-credential"); + return null; + } const now = Date.now(); if (initial.accessToken !== rejectedAccessToken && mainAccessTokenFresh(initial.accessToken, now, MAIN_TOKEN_REFRESH_SKEW_MS)) { + // The stored credential is no longer the one upstream rejected: somebody else replaced it + // while this request was in flight, so there is nothing here to refresh or to retire. + if (rejectedAccessToken !== undefined) noteNativeMainRefresh("skipped-credential-replaced"); return { accessToken: initial.accessToken!, chatgptAccountId: initial.chatgptAccountId }; } if (!initial.refreshToken) { + if (rejectedAccessToken !== undefined) noteNativeMainRefresh("skipped-no-refresh-grant"); return initial.accessToken !== rejectedAccessToken && mainAccessTokenFresh(initial.accessToken, now, 0) ? { accessToken: initial.accessToken!, chatgptAccountId: initial.chatgptAccountId } @@ -392,16 +468,30 @@ async function resolveMainAccountToken( && mainAccessTokenFresh(locked.accessToken, Date.now(), MAIN_TOKEN_REFRESH_SKEW_MS)) { return { accessToken: locked.accessToken!, chatgptAccountId: locked.chatgptAccountId }; } - const refresh = dependencies.refreshToken - ?? ((refreshToken: string, options: { signal: AbortSignal }) => refreshChatGPTToken(refreshToken, options)); + const refresh = dependencies.refreshToken ?? refreshNativeMainGrant; let refreshed: OAuthCredentials; try { refreshed = await refresh(locked.refreshToken, { signal }); } catch (cause) { + // A refusal already classified at the endpoint keeps that verdict. An injected refresh + // threw an ordinary Error and has only its message, so prose still decides there -- + // and only there, where there is no structured code to read. const message = cause instanceof Error ? cause.message.toLowerCase() : ""; - const reason = /invalid_grant|invalidated|revoked|expired/.test(message) - ? "reauth" as const - : "transient" as const; + const reason = cause instanceof MainAccountTokenRefreshError + ? cause.reason + : /invalid_grant|invalidated|revoked|expired/.test(message) + ? "reauth" as const + : "transient" as const; + // The shared point every request path reaches a terminal native-main verdict through: + // the forced refresh after an upstream 401, the pre-send refresh of an expired bearer, + // and the compact twin all land here. Recording the quarantine at the callers instead + // left the native main slot alone unrecorded -- `refreshNativeMainForwardAuth` has no + // `quarantine` flag for its caller to act on the way the stored-pool twin does -- so a + // revoked session stayed `needsReauth: false` and every request re-earned its own 401. + if (reason === "reauth") { + setMainRefreshGrantDead(lockKey); + markAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); + } throw new MainAccountTokenRefreshError(reason, { cause }); } // The refresh may resolve after the caller went away (an implementation that does @@ -411,6 +501,11 @@ async function resolveMainAccountToken( // before its own commit above. if (dependencies.signal?.aborted) throw dependencies.signal.reason; const result = persistRefreshedMainAuthJson(locked, refreshed); + // Retracted even under `preserveReauth`: that option preserves a quarantine whose cause + // this refresh did not speak to, while a 200 from the token endpoint is direct proof + // about the grant itself. Unconditional because a rotation-less success keeps the same + // refresh token, so the fingerprint alone would not clear the verdict. + setMainRefreshGrantDead(undefined); if (dependencies.preserveReauth !== true) clearAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); return result; }), diff --git a/src/server/responses/codex-auth-error.ts b/src/server/responses/codex-auth-error.ts index 12ab9a40fa5..0c906446490 100644 --- a/src/server/responses/codex-auth-error.ts +++ b/src/server/responses/codex-auth-error.ts @@ -1,5 +1,6 @@ import { formatErrorResponse } from "../../bridge"; import { + CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE, CodexAccountCooldownError, codexMainProfileDrainingResponse, cooldownErrorResponse, @@ -32,7 +33,9 @@ export function codexModelAvailabilityErrorResponse(error: CodexModelAvailabilit export function nativeMainRefreshFailureResponse(error: unknown): Response { if (error instanceof MainAccountTokenRefreshError && error.reason === "reauth") { - return formatErrorResponse(401, "authentication_error", "Codex main account needs reauthentication"); + // Same sentence the quarantine produces on every later request, so the first refusal and the + // ones after it do not describe one dead credential two different ways. + return formatErrorResponse(401, "authentication_error", CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE); } if (error instanceof MainAccountTokenRefreshError || error instanceof MainAuthJsonChangedDuringRefreshError diff --git a/src/server/responses/core-auth.ts b/src/server/responses/core-auth.ts index 3eb902cb3de..55459f3fb37 100644 --- a/src/server/responses/core-auth.ts +++ b/src/server/responses/core-auth.ts @@ -67,15 +67,32 @@ export type ResponsesAuthResolution = * an HMAC of exactly this Authorization header. Two copies of this rule would put preview and * final auth in different scopes the first time one of them changed. */ +/** + * The stored native-main credential this process injected itself, when it is the bearer the + * resolution will see. + * + * `handleClaudeMessages` reads `~/.codex/auth.json` and attaches it so OpenAI-backed sidecars stay + * reachable on a translated turn. It is OUR credential, not the caller's -- the ingress says so + * where it sets `nativeCallerAuth` and `callerDirectAuth` to null for exactly this reason -- and + * every reader of the resolution's input headers has to agree about that, or one of them conclud + * the request owns a credential that in fact belongs to the Pool. + */ +export function injectedClaudeMainBearer( + route: RouteResult, + options: HandleResponsesOptions, +): { authorization: string; chatgptAccountId?: string } | undefined { + return options.stripClaudeMainAuthForNoncanonicalForward === true + && isCanonicalOpenAiForwardProvider(route.provider) + ? options.trustedClaudeMainAuth : undefined; +} + export function codexRouteCredentialDomainHeaders( req: Request, route: RouteResult, options: HandleResponsesOptions, credentialDomainWasRewritten: boolean, ): Headers { - const trustedClaudeMainForFinalRoute = options.stripClaudeMainAuthForNoncanonicalForward === true - && isCanonicalOpenAiForwardProvider(route.provider) - ? options.trustedClaudeMainAuth : undefined; + const trustedClaudeMainForFinalRoute = injectedClaudeMainBearer(route, options); if (trustedClaudeMainForFinalRoute) { const claudeMainHeaders = new Headers(req.headers); claudeMainHeaders.set("authorization", trustedClaudeMainForFinalRoute.authorization); @@ -124,8 +141,17 @@ export function codexRouteCredentialOwnership( && (route.codexAccountMode !== undefined || isCanonicalOpenAiForwardProvider(route.provider)); return { substituteMainCredential, + // A request-scoped main credential is one the CALLER owns: it exists for this request only, + // owns no Pool state, and must never be quarantined or refreshed on the Pool's behalf. The + // stored credential this process injected is the opposite of that in every respect, and + // `hasForwardableCodexBearer` cannot tell them apart -- it sees a Codex JWT with an account + // id either way. Counting ours as the caller's made the native main slot caller-owned on + // every translated Claude turn: `main` instead of `main-pool`, so an upstream 401 on a + // revoked session recorded no outcome, refreshed nothing, and retired nothing, and the next + // request sent the same dead token again. requestScopedMainCredential: route.codexAccountMode !== undefined && !substituteMainCredential + && injectedClaudeMainBearer(route, options) === undefined && hasForwardableCodexBearer(authInputHeaders, config), }; } diff --git a/src/server/responses/core-combo-failure.ts b/src/server/responses/core-combo-failure.ts index 18e9f6e5c6a..6b6216dfce3 100644 --- a/src/server/responses/core-combo-failure.ts +++ b/src/server/responses/core-combo-failure.ts @@ -117,6 +117,7 @@ export async function consumeComboFailure( ...(nonReplayable ? { nonReplayable: true } : {}), classificationText, ...(normalizedUpstreamCode !== undefined ? { upstreamCode: normalizedUpstreamCode } : {}), + ...(upstreamMessage !== undefined ? { upstreamMessage } : {}), ...(upstreamType !== undefined ? { upstreamType } : {}), ...(!cyberFailure && cooldownRetryAfter !== undefined ? { retryAfter: cooldownRetryAfter } : {}), // The EFFECTIVE classification decides, not the raw status. An upstream that wraps a quota diff --git a/src/server/responses/core-combo.ts b/src/server/responses/core-combo.ts index 0901faf30e7..64a1fa60b8f 100644 --- a/src/server/responses/core-combo.ts +++ b/src/server/responses/core-combo.ts @@ -40,6 +40,7 @@ import { } from "../../combos"; import { formatErrorResponse } from "../../bridge"; import { SEND_BUDGET_EXHAUSTED_CODE } from "../../lib/errors"; +import { redactSecretString } from "../../lib/redact"; import { expandPreviousResponseInput, previousResponseReplayFailure, @@ -1040,8 +1041,21 @@ export async function executeComboResponses( } return lastFailure; } + // The status alone is what made a revoked ChatGPT session read as a provider outage: 300 + // identical `failed with 401` lines and nothing saying the credential was refused. Both + // fields come from the bounded body this failure already consumed -- no second read -- and + // `normalizeUpstreamErrorText` has capped the text before it gets here. + // + // The parsed `error.message` first: `classificationText` is the whole envelope, so building + // the line from it printed a raw multi-line JSON body across the log. Whitespace is collapsed + // for the same reason -- one failure must be one line, or a reader cannot count them. + const failureReason = redactSecretString( + [failure.upstreamCode, failure.upstreamMessage ?? failure.classificationText] + .filter(Boolean).join(": ").replace(/\s+/g, " ").trim(), + ).slice(0, 200); console.warn( - `[combo] ${comboId}: ${targetKey(pick.target)} failed with ${failure.response.status} after ${Date.now() - started}ms`, + `[combo] ${comboId}: ${targetKey(pick.target)} failed with ${failure.response.status}` + + ` after ${Date.now() - started}ms${failureReason ? `: ${failureReason}` : ""}`, ); const failureNow = Date.now(); const attemptedTargets = pick.attempted; diff --git a/src/server/responses/core-options.ts b/src/server/responses/core-options.ts index 680d13840e7..3153942bbde 100644 --- a/src/server/responses/core-options.ts +++ b/src/server/responses/core-options.ts @@ -26,6 +26,12 @@ export interface ConsumedComboFailure { classificationText: string; /** Structured upstream `error.code` when present in the failure body. */ upstreamCode?: string; + /** + * Structured upstream `error.message`, already redacted and bounded. Preferred over + * {@link classificationText} for anything human-facing: that field is the whole envelope, so a + * log line built from it prints the raw JSON body instead of the sentence inside it. + */ + upstreamMessage?: string; /** Complete structured provider type, retained for conservative recovery classification. */ upstreamType?: string; /** Valid numeric/date value used only for cooldown calculation. */ diff --git a/structure/providers/openai-accounts.md b/structure/providers/openai-accounts.md index 537368a7945..0e415ec11a5 100644 --- a/structure/providers/openai-accounts.md +++ b/structure/providers/openai-accounts.md @@ -201,6 +201,12 @@ sidecar candidate and cannot hide a failed Codex credential with separately bill `src/server/audio-upstream.ts` uses the same selection for standalone transcription. Explicit native Direct auth remains caller-owned; proxy-key-only Direct claims stored main before materialization, replacing both bearer and account identity exclusively from that credential. +Credential OWNERSHIP is decided by provenance, not by the shape of the bearer. The Claude Messages +ingress attaches the stored native-main credential to a translated turn so forward sidecars stay +reachable, and that credential is indistinguishable from a caller's own by inspection. It is never +request-scoped: a translated Claude turn resolves through Pool selection like any other, so native +main keeps its health, quarantine and refresh-and-classify handling. Only a bearer the client +itself supplied is caller-owned and exempt from stored state. Both synchronous and asynchronous stored-main substitution in `src/codex/auth-context.ts` remove a caller account header before copying the stored identity; an absent stored account ID leaves no account header. Caller-owned native Direct authentication retains its existing passthrough behavior. `src/providers/openai-sidecar.ts` releases quota-probe ownership on every materialization or usability failure before transferring a resolved context to its caller. @@ -226,6 +232,14 @@ model settings, and noncanonical `openai` rows never receive that recovery path. terminal 403 codes as `needsReauth`; generic permission failures remain non-terminal, and a successful main usage refresh clears the runtime mark. +A refresh refusal is classified from the token endpoint's structured `error` code alone, by one +rule shared between the stored-pool and native-main refreshes, so one dead grant cannot be terminal +for a Pool account and transient for main. Description prose decides only when no code was sent. A +code-confirmed refusal retires the grant by fingerprint as well as marking the account, because a +present refresh grant otherwise overrides the quarantine; that verdict survives a reauth clear and +is retracted only by a successful refresh or a replacement credential. Each refresh verdict is +logged once with the endpoint status and code and no credential material. + Canonical forwarding alone can apply the optional client-output safety-buffering hint filter; API-key and custom forward destinations preserve their metadata. See [Responses transport](../transports/responses.md). diff --git a/tests/codex-integration/codex-account-unusable-reason.test.ts b/tests/codex-integration/codex-account-unusable-reason.test.ts index ca71c4adc9a..2552e12c54f 100644 --- a/tests/codex-integration/codex-account-unusable-reason.test.ts +++ b/tests/codex-integration/codex-account-unusable-reason.test.ts @@ -8,6 +8,7 @@ import { } from "../../src/codex/account-usability"; import { saveCodexAccountCredential } from "../../src/codex/account-store"; import { clearAccountNeedsReauth, markAccountNeedsReauth } from "../../src/codex/account-runtime-state"; +import { CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE } from "../../src/codex/auth-context"; import { MAIN_CODEX_ACCOUNT_ID, MainAccountTokenRefreshError } from "../../src/codex/main-account"; import { nativeMainRefreshFailureResponse } from "../../src/server/responses/codex-auth-error"; import type { OcxConfig } from "../../src/types"; @@ -155,6 +156,9 @@ describe("native main refresh refusal", () => { const response = nativeMainRefreshFailureResponse(new MainAccountTokenRefreshError("reauth")); expect(response.status).toBe(401); const message = ((await response.json()) as { error: { message: string } }).error.message; - expect(message).toBe("Codex main account needs reauthentication"); + // Names the provider as the refuser and the command that fixes it: a bare "needs + // reauthentication" reads as a proxy fault for the operator whose plan was downgraded. + expect(message).toBe(CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE); + expect(message).toContain("codex login"); }); }); diff --git a/tests/responses/responses-native-main-refresh.test.ts b/tests/responses/responses-native-main-refresh.test.ts index 3a318cc0830..1f829b4d323 100644 --- a/tests/responses/responses-native-main-refresh.test.ts +++ b/tests/responses/responses-native-main-refresh.test.ts @@ -3,8 +3,9 @@ import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { clearAccountNeedsReauth } from "../../src/codex/auth-api"; +import { CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE } from "../../src/codex/auth-context"; import { saveCodexAccountCredential } from "../../src/codex/account-store"; -import { isAccountNeedsReauth } from "../../src/codex/account-runtime-state"; +import { isAccountNeedsReauth, markAccountNeedsReauth } from "../../src/codex/account-runtime-state"; import { getValidMainAccountToken, MAIN_CODEX_ACCOUNT_ID } from "../../src/codex/main-account"; import { withNativeMainSharedClaim } from "../../src/codex/native-main-claim"; import type { NativeProfileContext } from "../../src/codex/native-profile-store"; @@ -12,11 +13,24 @@ import { clearCodexUpstreamHealth, clearThreadAccountMap } from "../../src/codex import { resolveResponsesApiAuth } from "../../src/server/auth-cors"; import { tryAdmitTurn } from "../../src/server/lifecycle"; import { handleResponses, handleResponsesCompact } from "../../src/server/responses"; +import { handleClaudeMessages } from "../../src/server/claude-messages"; +import { clearComboTargetCooldowns } from "../../src/combos/failover"; +import { clearComboSelectionState } from "../../src/combos/resolve"; import type { RequestLogContext } from "../../src/server/request-log"; import type { OcxConfig } from "../../src/types"; import { acquireOwnedSpendHome } from "../helpers/owned-spend-home"; import { removeTreeWithRetry } from "../helpers/remove-tree"; +/** Smallest complete Anthropic stream: the combo's second target has to actually answer. */ +const ANTHROPIC_SSE = [ + ["message_start", { type: "message_start", message: { id: "msg_fallback", type: "message", role: "assistant", model: "m2", content: [], stop_reason: null, usage: { input_tokens: 1, output_tokens: 0 } } }], + ["content_block_start", { type: "content_block_start", index: 0, content_block: { type: "text", text: "" } }], + ["content_block_delta", { type: "content_block_delta", index: 0, delta: { type: "text_delta", text: "served" } }], + ["content_block_stop", { type: "content_block_stop", index: 0 }], + ["message_delta", { type: "message_delta", delta: { stop_reason: "end_turn" }, usage: { output_tokens: 1 } }], + ["message_stop", { type: "message_stop" }], +].map(([event, data]) => `event: ${event}\ndata: ${JSON.stringify(data)}\n\n`).join(""); + const originalFetch = globalThis.fetch; let home = ""; let previousOcxHome: string | undefined; @@ -315,6 +329,121 @@ describe("native main 401 refresh and replay", () => { }); } + /** + * The revoked-session harness: the access-token JWT still looks live, every send earns + * `token_invalidated`, and only the forced refresh can prove the credential is dead. The + * token endpoint's answer to that refresh is the variable. + */ + function installRevokedSessionHarness(refusal: { status: number; body: unknown }): { + sends: string[]; + refreshes: string[]; + } { + const payload = Buffer.from(JSON.stringify({ exp: Math.floor(Date.now() / 1000) + 86_400 })).toString("base64url"); + writeFileSync(join(home, "auth.json"), JSON.stringify({ + tokens: { + access_token: `header.${payload}.signature`, + refresh_token: "refresh-grant", + account_id: "account-main", + }, + })); + const sends: string[] = []; + const refreshes: string[] = []; + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = new URL(input instanceof Request ? input.url : String(input)); + if (url.hostname === "auth.openai.com") { + refreshes.push(new URLSearchParams(String(init?.body)).get("refresh_token") ?? ""); + return Response.json(refusal.body, { status: refusal.status }); + } + if (!url.pathname.endsWith("/responses")) { + return Response.json({ rate_limit: { primary_window: { used_percent: 10 } } }); + } + sends.push(new Headers(init?.headers).get("authorization") ?? ""); + return Response.json({ + error: { + message: "Your authentication token has been invalidated. Please try signing in again.", + type: "invalid_request_error", + code: "token_invalidated", + }, + }, { status: 401 }); + }) as typeof fetch; + return { sends, refreshes }; + } + + // Production only ever confirmed bare 400 `invalid_grant`. These are the siblings upstream + // also emits for a revoked or downgraded session; each must retire the grant, not just the + // one code the incident happened to produce. + test.each([ + [400, "invalid_grant"], + [401, "refresh_token_invalidated"], + [401, "token_invalidated"], + [401, "refresh_token_expired"], + [401, "refresh_token_reused"], + ] as const)( + "retires native main on %i %s so the next request never sends", + async (status, code) => { + const { sends, refreshes } = installRevokedSessionHarness({ status, body: { error: code } }); + const refreshLines: string[] = []; + const warnSpy = spyOn(console, "warn").mockImplementation((...args: unknown[]) => { + refreshLines.push(args.map(String).join(" ")); + }); + + const first = await handleResponses( + request("/v1/responses"), + config(), + { model: "", provider: "" } as RequestLogContext, + ); + warnSpy.mockRestore(); + expect(first.status).toBe(401); + expect(JSON.parse(await first.text()).error.message).toBe(CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE); + // The verdict is on the record, with the endpoint's own status and code and nothing else. + // A revoked session is otherwise invisible: no write, and a status like any other 401. + expect(refreshLines).toContain(`[codex] native main refresh: reauth status=${status} code=${code}`); + expect(sends).toHaveLength(1); + expect(refreshes).toEqual(["refresh-grant"]); + expect(isAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID)).toBe(true); + + const second = await handleResponses( + request("/v1/responses"), + config(), + { model: "", provider: "" } as RequestLogContext, + ); + // The point of the quarantine: the second request is refused locally, so a combo's next + // target is reached without paying another upstream round trip on a dead credential -- and + // it says the same actionable thing the discovering request said, not "no usable credential". + expect(second.status).toBe(401); + expect(JSON.parse(await second.text()).error.message).toBe(CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE); + expect(sends).toHaveLength(1); + expect(refreshes).toEqual(["refresh-grant"]); + }, + ); + + test("keeps a 5xx whose description reads terminal transient and leaves the grant alive", async () => { + // The other half of the classification, and the reason it reads the structured code rather + // than the formatted message: upstream puts arbitrary prose in `error_description`, so a + // token-endpoint 5xx that happens to say "session expired" must stay a retryable refusal. + // Quarantining on it would retire a live grant and demand a sign-in nobody needs. + const { sends, refreshes } = installRevokedSessionHarness({ + status: 503, + body: { error: "server_error", error_description: "session expired or revoked; retry" }, + }); + + const transientLines: string[] = []; + const warnSpy = spyOn(console, "warn").mockImplementation((...args: unknown[]) => { + transientLines.push(args.map(String).join(" ")); + }); + const response = await handleResponses( + request("/v1/responses"), + config(), + { model: "", provider: "" } as RequestLogContext, + ); + warnSpy.mockRestore(); + expect(transientLines).toContain("[codex] native main refresh: transient status=503 code=server_error"); + expect(response.status).toBe(503); + expect(sends).toHaveLength(1); + expect(refreshes).toEqual(["refresh-grant"]); + expect(isAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID)).toBe(false); + }); + test.each(["/v1/responses", "/v1/responses/compact"] as const)( "%s keeps the WebSocket string-abort claim cancellation as 499 without quarantining main", async path => { @@ -369,4 +498,159 @@ describe("native main 401 refresh and replay", () => { } }, ); + + /** + * The shape that actually runs in production: Claude Code POSTs /v1/messages with + * `model: combo/codexfirst`, whose first leg is the openai pool-main provider. The inbound + * wire is claude-messages, so that leg is TRANSLATED rather than passed through -- it + * dispatches through `prepareAdapterExchange`, not the passthrough route, and that dispatch + * recorded no Codex account outcome and attempted no refresh on a pre-stream 401. The + * revoked session was therefore rediscovered by a fresh upstream 401 on every request. + */ + test("a /v1/messages combo leg retires the dead grant instead of re-earning its 401", async () => { + const payload = Buffer.from(JSON.stringify({ exp: Math.floor(Date.now() / 1000) + 86_400 })).toString("base64url"); + writeFileSync(join(home, "auth.json"), JSON.stringify({ + tokens: { + access_token: `header.${payload}.signature`, + refresh_token: "refresh-grant", + account_id: "account-main", + }, + })); + const codexSends: string[] = []; + const refreshes: string[] = []; + let fallbackSends = 0; + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = new URL(input instanceof Request ? input.url : String(input)); + if (url.hostname === "auth.openai.com") { + refreshes.push(new URLSearchParams(String(init?.body)).get("refresh_token") ?? ""); + return Response.json({ error: "token_invalidated" }, { status: 401 }); + } + if (url.hostname === "fallback.test") { + fallbackSends += 1; + return new Response(ANTHROPIC_SSE, { headers: { "content-type": "text/event-stream" } }); + } + if (url.hostname !== "chatgpt.com") { + return Response.json({ rate_limit: { primary_window: { used_percent: 10 } } }); + } + if (!url.pathname.endsWith("/responses")) { + return Response.json({ rate_limit: { primary_window: { used_percent: 10 } } }); + } + codexSends.push(new Headers(init?.headers).get("authorization") ?? ""); + return Response.json({ + error: { + message: "Your authentication token has been invalidated. Please try signing in again.", + type: "invalid_request_error", + code: "token_invalidated", + }, + }, { status: 401 }); + }) as typeof fetch; + + const comboConfig = { + ...config(), + providers: { + ...config().providers, + fallback: { + adapter: "anthropic", + baseUrl: "https://fallback.test", + apiKey: "k", + }, + }, + combos: { + codexfirst: { + strategy: "failover", + targets: [ + { provider: "openai", model: "gpt-5.5" }, + { provider: "fallback", model: "m2" }, + ], + }, + }, + } as unknown as OcxConfig; + const messagesRequest = (): Request => new Request("http://localhost/v1/messages", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + model: "combo/codexfirst", + max_tokens: 128, + messages: [{ role: "user", content: "hi" }], + }), + }); + + // The stored-main enrichment in handleClaudeMessages is gated on a claimed native-main + // profile for the turn, which needs a real admission lease. Without one the enrichment never + // fires and the leg resolves to `main-pool` -- which is exactly why every earlier attempt to + // reproduce the Mini from a leaseless harness came out already fixed. + const comboTurn = tryAdmitTurn(); + expect(comboTurn).not.toBeNull(); + const warnings: string[] = []; + const warnSpy = spyOn(console, "warn").mockImplementation((...args: unknown[]) => { + warnings.push(args.map(String).join(" ")); + }); + let first: Response; + try { + first = await handleClaudeMessages( + messagesRequest(), + comboConfig, + { model: "", provider: "" } as RequestLogContext, + { requestId: `combo-401-${crypto.randomUUID()}`, start: Date.now(), turnAdmissionLease: comboTurn! }, + ); + await first.text(); + } finally { + warnSpy.mockRestore(); + } + // One failure is one line, and it names the refusal rather than reprinting the envelope. + // The raw body is multi-line JSON; a log built from it spread a single 401 over the screen. + const comboLine = warnings.find(line => line.startsWith("[combo] codexfirst: openai/")); + expect(comboLine).toBeDefined(); + expect(comboLine).not.toContain("\n"); + expect(comboLine).not.toContain("{"); + expect(comboLine).toContain("codex login"); + // The combo still serves the turn from its second target; that was never the problem. + expect(first.status).toBe(200); + expect(fallbackSends).toBe(1); + // One send, one refresh attempt, and the refusal retired the grant for good. + expect(codexSends).toHaveLength(1); + expect(refreshes).toEqual(["refresh-grant"]); + expect(isAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID)).toBe(true); + + // Re-offer the openai leg. Without this the combo's own target cooldown skips it and the + // assertions below would pass whether or not the account was ever quarantined. + clearComboTargetCooldowns(); + clearComboSelectionState(); + const second = await handleClaudeMessages( + messagesRequest(), + comboConfig, + { model: "", provider: "" } as RequestLogContext, + { requestId: `combo-401-${crypto.randomUUID()}`, start: Date.now(), turnAdmissionLease: comboTurn! }, + ); + await second.text(); + expect(second.status).toBe(200); + expect(fallbackSends).toBe(2); + // THE POINT: the quarantined main account is not sent to again, and its dead grant is not + // retried either. Before the fix this was a second upstream 401, three hundred times over. + expect(codexSends).toHaveLength(1); + expect(refreshes).toEqual(["refresh-grant"]); + + // The production loop, and the reason the quarantine did not hold on the Mini: the WHAM + // probe retracts a reauth quarantine on an EXPLICIT refresh (#327), which `ocx account list` + // and an open dashboard both trigger, and then re-marks the account from its own terminal + // verdict. The dead-grant finding must survive that round trip -- it is a fact about a + // specific refresh token, not about the quarantine that accompanied it -- or main becomes + // selectable again and buys another upstream 401 every time the operator looks at it. + clearAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); + markAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); + clearComboTargetCooldowns(); + clearComboSelectionState(); + const third = await handleClaudeMessages( + messagesRequest(), + comboConfig, + { model: "", provider: "" } as RequestLogContext, + { requestId: `combo-401-${crypto.randomUUID()}`, start: Date.now(), turnAdmissionLease: comboTurn! }, + ); + await third.text(); + expect(third.status).toBe(200); + expect(fallbackSends).toBe(3); + expect(codexSends).toHaveLength(1); + expect(refreshes).toEqual(["refresh-grant"]); + comboTurn?.release(); + }); }); From 7ffd1a856957c320d139ff262b4adaf4dd3da07b Mon Sep 17 00:00:00 2001 From: Vadym O Date: Sat, 3 Oct 2026 19:05:13 +0800 Subject: [PATCH 2/2] fix(codex): address review on the native-main refresh change - Keep the sign-in refusal in the response layer, so the change no longer touches src/codex/auth-context.ts. - A non-string error_description falls back to the HTTP status instead of throwing a TypeError out of the refusal classifier. - The combo test takes a fresh turn lease per request; with dead-grant retirement disabled, it now fails. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/codex/account-store.ts | 3 ++- src/codex/auth-context.ts | 18 +------------- src/server/responses/codex-auth-error.ts | 24 +++++++++++++++++-- .../codex-account-unusable-reason.test.ts | 2 +- .../responses-native-main-refresh.test.ts | 22 +++++++++++------ 5 files changed, 41 insertions(+), 28 deletions(-) diff --git a/src/codex/account-store.ts b/src/codex/account-store.ts index 4e1e811bd56..a495c2aad14 100644 --- a/src/codex/account-store.ts +++ b/src/codex/account-store.ts @@ -569,7 +569,8 @@ export function classifyChatgptRefreshFailure( errCodeExact = typeof parsed.error.code === "string" ? parsed.error.code.trim() : undefined; errDesc = [parsed.error.code, parsed.error.message, parsed.error_description].filter(Boolean).join(": "); } else { - errDesc = parsed.error_description || `HTTP ${status}`; + errDesc = typeof parsed.error_description === "string" && parsed.error_description + ? parsed.error_description : `HTTP ${status}`; } if (!errDesc) errDesc = `HTTP ${status}`; } catch { errDesc = `HTTP ${status}`; } diff --git a/src/codex/auth-context.ts b/src/codex/auth-context.ts index 8588c1b6ddc..f2355961567 100644 --- a/src/codex/auth-context.ts +++ b/src/codex/auth-context.ts @@ -385,18 +385,6 @@ export class CodexPoolAuthenticationError extends Error { } } -/** - * The refusal a revoked or downgraded ChatGPT session earns, on the request that discovers it - * and on every request after. - * - * "No usable account credential" is true and useless: it reads as a proxy fault and sends the - * operator to the provider's status page. The two things they need are that the credential was - * rejected by OpenAI rather than by this proxy, and the command that fixes it. - */ -export const CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE = - "Codex account needs sign-in: OpenAI rejected its credential (token invalidated — revoked " - + "session or plan change). Run `codex login` to sign in again."; - export type CodexModelAvailabilityReason = "unsupported" | "temporarily_unavailable"; /** A model/account compatibility failure is not a credential failure. */ @@ -1353,11 +1341,7 @@ export async function resolveCodexAuthContext( : "Codex accounts that support this model are currently unavailable", ); } - // The pool is empty because a credential was quarantined, not because none is configured. - // Say which, so the refusal names the thing the operator has to do. - throw new CodexPoolAuthenticationError( - isAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID) ? CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE : undefined, - ); + throw new CodexPoolAuthenticationError(); } accountId = selected; if (accountId === MAIN_CODEX_ACCOUNT_ID) assertMainAccountPolicy(policy); diff --git a/src/server/responses/codex-auth-error.ts b/src/server/responses/codex-auth-error.ts index 0c906446490..c093f01b44c 100644 --- a/src/server/responses/codex-auth-error.ts +++ b/src/server/responses/codex-auth-error.ts @@ -1,6 +1,6 @@ import { formatErrorResponse } from "../../bridge"; +import { isAccountNeedsReauth } from "../../codex/account-runtime-state"; import { - CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE, CodexAccountCooldownError, codexMainProfileDrainingResponse, cooldownErrorResponse, @@ -31,6 +31,20 @@ export function codexModelAvailabilityErrorResponse(error: CodexModelAvailabilit return formatErrorResponse(400, "invalid_request_error", error.message); } +/** + * The refusal a revoked or downgraded ChatGPT session earns, on the request that discovers it + * and on every request after. + * + * "No usable account credential" is true and useless: it reads as a proxy fault and sends the + * operator to the provider's status page. The two things they need are that the credential was + * rejected by OpenAI rather than by this proxy, and the command that fixes it. + */ +export const CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE = + "Codex account needs sign-in: OpenAI rejected its credential (token invalidated — revoked " + + "session or plan change). Run `codex login` to sign in again."; + +const DEFAULT_POOL_AUTHENTICATION_MESSAGE = new CodexPoolAuthenticationError().message; + export function nativeMainRefreshFailureResponse(error: unknown): Response { if (error instanceof MainAccountTokenRefreshError && error.reason === "reauth") { // Same sentence the quarantine produces on every later request, so the first refusal and the @@ -101,7 +115,13 @@ export function mapCodexAuthContextErrorToResponse( return codexModelAvailabilityErrorResponse(error); } if (error instanceof CodexPoolAuthenticationError || error instanceof CodexDirectAuthenticationError) { - return formatErrorResponse(401, "authentication_error", error.message); + // An empty pool because native main was quarantined, not because none is configured: say which, + // so the refusal names the thing the operator has to do. + const quarantinedMain = error instanceof CodexPoolAuthenticationError + && error.message === DEFAULT_POOL_AUTHENTICATION_MESSAGE + && isAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); + return formatErrorResponse(401, "authentication_error", + quarantinedMain ? CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE : error.message); } if (error instanceof CodexMainSubstitutionUnavailableError) { return formatErrorResponse( diff --git a/tests/codex-integration/codex-account-unusable-reason.test.ts b/tests/codex-integration/codex-account-unusable-reason.test.ts index 2552e12c54f..c5c045bc2d0 100644 --- a/tests/codex-integration/codex-account-unusable-reason.test.ts +++ b/tests/codex-integration/codex-account-unusable-reason.test.ts @@ -8,7 +8,7 @@ import { } from "../../src/codex/account-usability"; import { saveCodexAccountCredential } from "../../src/codex/account-store"; import { clearAccountNeedsReauth, markAccountNeedsReauth } from "../../src/codex/account-runtime-state"; -import { CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE } from "../../src/codex/auth-context"; +import { CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE } from "../../src/server/responses/codex-auth-error"; import { MAIN_CODEX_ACCOUNT_ID, MainAccountTokenRefreshError } from "../../src/codex/main-account"; import { nativeMainRefreshFailureResponse } from "../../src/server/responses/codex-auth-error"; import type { OcxConfig } from "../../src/types"; diff --git a/tests/responses/responses-native-main-refresh.test.ts b/tests/responses/responses-native-main-refresh.test.ts index 1f829b4d323..3eeaf5741b2 100644 --- a/tests/responses/responses-native-main-refresh.test.ts +++ b/tests/responses/responses-native-main-refresh.test.ts @@ -3,7 +3,7 @@ import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { clearAccountNeedsReauth } from "../../src/codex/auth-api"; -import { CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE } from "../../src/codex/auth-context"; +import { CODEX_MAIN_SIGN_IN_REQUIRED_MESSAGE } from "../../src/server/responses/codex-auth-error"; import { saveCodexAccountCredential } from "../../src/codex/account-store"; import { isAccountNeedsReauth, markAccountNeedsReauth } from "../../src/codex/account-runtime-state"; import { getValidMainAccountToken, MAIN_CODEX_ACCOUNT_ID } from "../../src/codex/main-account"; @@ -579,8 +579,16 @@ describe("native main 401 refresh and replay", () => { // profile for the turn, which needs a real admission lease. Without one the enrichment never // fires and the leg resolves to `main-pool` -- which is exactly why every earlier attempt to // reproduce the Mini from a leaseless harness came out already fixed. - const comboTurn = tryAdmitTurn(); - expect(comboTurn).not.toBeNull(); + // A fresh turn lease per request: reading a response to completion releases its lease, and a + // reused inactive lease would refuse the main-profile claim before refresh or send, letting the + // send-count assertions below pass without exercising the quarantine at all. + const turns: NonNullable>[] = []; + const freshTurn = () => { + const turn = tryAdmitTurn(); + expect(turn).not.toBeNull(); + turns.push(turn!); + return turn!; + }; const warnings: string[] = []; const warnSpy = spyOn(console, "warn").mockImplementation((...args: unknown[]) => { warnings.push(args.map(String).join(" ")); @@ -591,7 +599,7 @@ describe("native main 401 refresh and replay", () => { messagesRequest(), comboConfig, { model: "", provider: "" } as RequestLogContext, - { requestId: `combo-401-${crypto.randomUUID()}`, start: Date.now(), turnAdmissionLease: comboTurn! }, + { requestId: `combo-401-${crypto.randomUUID()}`, start: Date.now(), turnAdmissionLease: freshTurn() }, ); await first.text(); } finally { @@ -620,7 +628,7 @@ describe("native main 401 refresh and replay", () => { messagesRequest(), comboConfig, { model: "", provider: "" } as RequestLogContext, - { requestId: `combo-401-${crypto.randomUUID()}`, start: Date.now(), turnAdmissionLease: comboTurn! }, + { requestId: `combo-401-${crypto.randomUUID()}`, start: Date.now(), turnAdmissionLease: freshTurn() }, ); await second.text(); expect(second.status).toBe(200); @@ -644,13 +652,13 @@ describe("native main 401 refresh and replay", () => { messagesRequest(), comboConfig, { model: "", provider: "" } as RequestLogContext, - { requestId: `combo-401-${crypto.randomUUID()}`, start: Date.now(), turnAdmissionLease: comboTurn! }, + { requestId: `combo-401-${crypto.randomUUID()}`, start: Date.now(), turnAdmissionLease: freshTurn() }, ); await third.text(); expect(third.status).toBe(200); expect(fallbackSends).toBe(3); expect(codexSends).toHaveLength(1); expect(refreshes).toEqual(["refresh-grant"]); - comboTurn?.release(); + for (const turn of turns) turn.release(); }); });