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..a495c2aad14 100644 --- a/src/codex/account-store.ts +++ b/src/codex/account-store.ts @@ -524,6 +524,70 @@ 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 = typeof parsed.error_description === "string" && parsed.error_description + ? 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 +1387,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/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..c093f01b44c 100644 --- a/src/server/responses/codex-auth-error.ts +++ b/src/server/responses/codex-auth-error.ts @@ -1,4 +1,5 @@ import { formatErrorResponse } from "../../bridge"; +import { isAccountNeedsReauth } from "../../codex/account-runtime-state"; import { CodexAccountCooldownError, codexMainProfileDrainingResponse, @@ -30,9 +31,25 @@ 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") { - 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 @@ -98,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/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..c5c045bc2d0 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/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"; @@ -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..3eeaf5741b2 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/server/responses/codex-auth-error"; 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,167 @@ 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. + // 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(" ")); + }); + let first: Response; + try { + first = await handleClaudeMessages( + messagesRequest(), + comboConfig, + { model: "", provider: "" } as RequestLogContext, + { requestId: `combo-401-${crypto.randomUUID()}`, start: Date.now(), turnAdmissionLease: freshTurn() }, + ); + 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: freshTurn() }, + ); + 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: freshTurn() }, + ); + await third.text(); + expect(third.status).toBe(200); + expect(fallbackSends).toBe(3); + expect(codexSends).toHaveLength(1); + expect(refreshes).toEqual(["refresh-grant"]); + for (const turn of turns) turn.release(); + }); });