From a063fd0007def4a795dc69310955cf8904f6b07c Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Sat, 19 Sep 2026 23:59:03 +0900 Subject: [PATCH 1/2] fix(codex): preserve pool reauthentication failure causes PoolQuotaResult now carries the failure source observed while obtaining WHAM/quota or refreshing tokens, so poolAccountDto reports the actual cause instead of inferring it from overlapping booleans. Terminal WHAM 401s surface as quota_unauthorized and terminal refresh failures as refresh_failed. --- src/codex/auth-api/account-list.ts | 24 ++++++++---- src/codex/auth-api/pool-quota-probe.ts | 12 +++--- src/oauth/health.ts | 3 +- .../codex-integration/codex-auth-api.test.ts | 38 +++++++++++++++++++ 4 files changed, 63 insertions(+), 14 deletions(-) diff --git a/src/codex/auth-api/account-list.ts b/src/codex/auth-api/account-list.ts index faa8eeefc08..23563a01a73 100644 --- a/src/codex/auth-api/account-list.ts +++ b/src/codex/auth-api/account-list.ts @@ -105,18 +105,26 @@ export function poolAccountDto( const plan = codexPlanValue(account.plan); const quota = quotaForPlan(quotaResult.quota, plan); const runtimeReauth = isAccountNeedsReauth(account.id); + const rawReauthReason: CodexAccountReauthReason | undefined = !hasCredential + ? "missing_credential" + : quotaResult.reauthReason + ? quotaResult.reauthReason + : runtimeReauth + ? "refresh_failed" + : undefined; const needsReauth = !hasCredential || quotaResult.needsReauth || runtimeReauth; - const health = projectCodexAccountHealth({ accountId: account.id, needsReauth }); + const healthReason = rawReauthReason === "quota_unauthorized" || rawReauthReason === "missing_credential" + ? "unauthorized" + : "refresh_failed"; + const health = projectCodexAccountHealth({ + accountId: account.id, + needsReauth, + reauthReason: needsReauth ? healthReason : undefined, + }); // `needsReauth` is an OR of three independent causes plus a persisted verdict resolved inside the // health projection. Emitting only the boolean is what left #4212's reporter guessing which // account took their model away and why, so name the cause they actually have to act on. - const reauthReason: CodexAccountReauthReason | undefined = !hasCredential - ? "missing_credential" - : runtimeReauth - ? "refresh_failed" - : quotaResult.needsReauth - ? "quota_unauthorized" - : health.status === "reauth_required" ? health.reason : undefined; + const reauthReason: CodexAccountReauthReason | undefined = rawReauthReason ?? (health.status === "reauth_required" ? health.reason : undefined); return { id: account.id, email: projectEmail(account.email, maskEmails) ?? account.email, diff --git a/src/codex/auth-api/pool-quota-probe.ts b/src/codex/auth-api/pool-quota-probe.ts index 84f8519af1e..4b27ad875a4 100644 --- a/src/codex/auth-api/pool-quota-probe.ts +++ b/src/codex/auth-api/pool-quota-probe.ts @@ -46,6 +46,8 @@ export interface PoolQuotaResult { resetRefreshLineage?: ManualResetRefreshLineage; quota: StoredAccountQuota | null; needsReauth: boolean; + /** Failure source observed while obtaining the quota, when reauthentication is required. */ + reauthReason?: "refresh_failed" | "quota_unauthorized"; /** Credential generation whose cache or network result this DTO state belongs to. */ credentialGeneration?: number; /** Present only when this call freshly parsed a WHAM usage response. */ @@ -178,7 +180,7 @@ export async function recoverPoolQuotaFrom401(ctx: { // the credential it condemned, so a late terminal response arriving after the operator // re-authenticated would quarantine the replacement. markAccountNeedsReauth(accountId, captureConfigGeneration(), rejectedGeneration); - return { quota: existing ?? null, needsReauth: true, credentialGeneration: rejectedGeneration }; + return { quota: existing ?? null, needsReauth: true, reauthReason: "quota_unauthorized", credentialGeneration: rejectedGeneration }; } const claim = claimQuotaRecovery(accountId, rejectedGeneration); @@ -186,7 +188,7 @@ export async function recoverPoolQuotaFrom401(ctx: { // A lineage fenced by a TERMINAL refresh failure stays terminal. Without this, the // budget being used would make the next bare 401 report a dead credential as healthy. if (quotaRecoveryTerminalFor(accountId, rejectedGeneration)) { - return { quota: existing ?? null, needsReauth: true, credentialGeneration: rejectedGeneration }; + return { quota: existing ?? null, needsReauth: true, reauthReason: "refresh_failed", credentialGeneration: rejectedGeneration }; } // Otherwise: this lineage spent its attempt, another caller is mid-refresh, or a // transient failure is backing off. Report transient and let the next poll try — @@ -226,7 +228,7 @@ export async function recoverPoolQuotaFrom401(ctx: { // Everything else is unknown, and unknown is not proof. if (e instanceof TokenRefreshError && isTerminalRefreshError(e)) { markAccountNeedsReauth(accountId, captureConfigGeneration(), rejectedGeneration); - return { quota: existing ?? null, needsReauth: true, credentialGeneration: rejectedGeneration }; + return { quota: existing ?? null, needsReauth: true, reauthReason: "refresh_failed", credentialGeneration: rejectedGeneration }; } return { quota: existing ?? null, needsReauth: false, credentialGeneration: rejectedGeneration }; } @@ -258,7 +260,7 @@ export async function recoverPoolQuotaFrom401(ctx: { // let the next poll call a dead credential healthy. The evidence is about the // REFRESHED credential, which is what the replay used. markAccountNeedsReauth(accountId, writerGeneration, refreshed.generation); - return { quota: existing ?? null, needsReauth: true, credentialGeneration: refreshed.generation }; + return { quota: existing ?? null, needsReauth: true, reauthReason: "quota_unauthorized", credentialGeneration: refreshed.generation }; } return { quota: existing ?? null, needsReauth: false, credentialGeneration: refreshed.generation }; } @@ -401,7 +403,7 @@ export async function fetchFreshPoolAccountQuota( } if (e instanceof TokenRefreshError) { return withQuotaProbeEvidence( - { quota: existing ?? null, needsReauth: true, credentialGeneration: requestCredentialGeneration }, + { quota: existing ?? null, needsReauth: true, reauthReason: "refresh_failed", credentialGeneration: requestCredentialGeneration }, quotaProbeEvidence, ); } diff --git a/src/oauth/health.ts b/src/oauth/health.ts index 8c4332af850..3379f700aeb 100644 --- a/src/oauth/health.ts +++ b/src/oauth/health.ts @@ -202,6 +202,7 @@ export function projectStoredOAuthAccountHealth( export function projectCodexAccountHealth(input: { accountId: string; needsReauth: boolean; + reauthReason?: "unauthorized" | "forbidden" | "refresh_failed"; now?: number; }): OAuthAccountHealth { // One read serves every verdict below. Each lookup re-reads and re-hardens the whole store @@ -239,7 +240,7 @@ export function projectCodexAccountHealth(input: { const snap = getCodexAccountHealthSnapshot(input.accountId, now); return projectOAuthAccountHealth({ needsReauth, - reauthReason: needsReauth ? "refresh_failed" : undefined, + reauthReason: needsReauth ? (input.reauthReason ?? "refresh_failed") : undefined, cooldownUntilMs: snap?.cooldownUntil, cooldownReason: cooldownReasonFromSource(snap?.cooldownSource), now, diff --git a/tests/codex-integration/codex-auth-api.test.ts b/tests/codex-integration/codex-auth-api.test.ts index d02474985e4..3c885491164 100644 --- a/tests/codex-integration/codex-auth-api.test.ts +++ b/tests/codex-integration/codex-auth-api.test.ts @@ -2090,6 +2090,44 @@ describe("codex-auth API", () => { expect(existsSync(join(TEST_DIR, "config.json"))).toBe(false); }); + test("pool quota rejection reports quota authorization as the reauthentication cause", async () => { + const config = makeConfig(); + seedPoolAccount(config, { id: "pool-quota-rejected", email: "pool-quota-rejected@example.com" }); + globalThis.fetch = (async () => Response.json( + { detail: { code: "invalid_refresh_token" } }, + { status: 401 }, + )) as typeof fetch; + + const req = new Request("http://localhost/api/codex-auth/accounts?refresh=1"); + const resp = await handleCodexAuthAPI(req, new URL(req.url), config); + const data = await resp!.json() as { accounts: Array<{ id: string; reauthReason?: string }> }; + + expect(data.accounts.find(account => account.id === "pool-quota-rejected")) + .toMatchObject({ reauthReason: "quota_unauthorized" }); + }); + + test("pool token refresh rejection reports refresh failure as the reauthentication cause", async () => { + const config = makeConfig(); + seedPoolAccount(config, { + id: "pool-refresh-rejected", + email: "pool-refresh-rejected@example.com", + expiresAt: Date.now() - 1, + }); + const urls: string[] = []; + globalThis.fetch = (async input => { + urls.push(String(input)); + return Response.json({ error: "invalid_grant" }, { status: 400 }); + }) as typeof fetch; + + const req = new Request("http://localhost/api/codex-auth/accounts/refresh", { method: "POST" }); + const resp = await handleCodexAuthAPI(req, new URL(req.url), config); + const data = await resp!.json() as { accounts: Array<{ id: string; reauthReason?: string }> }; + + expect(urls).toEqual(["https://auth.openai.com/oauth/token"]); + expect(data.accounts.find(account => account.id === "pool-refresh-rejected")) + .toMatchObject({ reauthReason: "refresh_failed" }); + }); + test("pool plan refresh batches multiple authoritative changes into one config save", async () => { const config = makeConfig(); seedPoolAccount(config, { id: "pool-plan-a", email: "pool-plan-a@example.com", plan: "plus" }); From 8ba026a591ca5de8ed6ddf02e92d51b8a276dcfe Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Sun, 20 Sep 2026 04:11:25 +0900 Subject: [PATCH 2/2] fix(codex): keep transient pool refresh failures transient and surface stored auth causes --- src/codex/auth-api/account-list.ts | 6 +- src/codex/auth-api/pool-quota-probe.ts | 10 +++- src/oauth/health.ts | 12 +++- .../codex-integration/codex-auth-api.test.ts | 60 +++++++++++++++++-- tests/oauth/oauth-health.test.ts | 19 ++++++ 5 files changed, 96 insertions(+), 11 deletions(-) diff --git a/src/codex/auth-api/account-list.ts b/src/codex/auth-api/account-list.ts index 23563a01a73..3ffd01dddda 100644 --- a/src/codex/auth-api/account-list.ts +++ b/src/codex/auth-api/account-list.ts @@ -82,9 +82,9 @@ export function mainQuotaWithCarriedResetCredits( /** * Why an account needs the operator. `missing_credential`, `refresh_failed`, and * `quota_unauthorized` are the three causes this surface tells apart on its own. `unauthorized` - * and `forbidden` exist because the shared health projection may return them; today - * `projectCodexAccountHealth` only ever produces `refresh_failed`, so accepting the full union - * keeps this field correct if that projection widens rather than silently dropping a reason. + * and `forbidden` come from the shared health projection: `projectCodexAccountHealth` maps a + * stored verification failure's `http_status:401`/`http_status:403` to them, so the union has + * to accept every reason the projection can emit rather than silently dropping one. */ export type CodexAccountReauthReason = | "missing_credential" diff --git a/src/codex/auth-api/pool-quota-probe.ts b/src/codex/auth-api/pool-quota-probe.ts index 4b27ad875a4..496142ecff7 100644 --- a/src/codex/auth-api/pool-quota-probe.ts +++ b/src/codex/auth-api/pool-quota-probe.ts @@ -1,4 +1,4 @@ -import { capturePoolQuotaWriter, getValidCodexToken, isCodexAccountGenerationLive, forceRefreshCodexPoolToken, markCodexAccountValidated, markCodexAccountValidationFailed, readCodexAccountRecord, CodexCredentialGenerationConflictError, CodexCredentialRefreshLockTimeoutError, CodexCredentialRefreshBusyError, CodexCredentialRefreshStaleError, TokenRefreshError } from "../account-store"; +import { capturePoolQuotaWriter, getValidCodexToken, isCodexAccountGenerationLive, forceRefreshCodexPoolToken, markCodexAccountValidated, markCodexAccountValidationFailed, readCodexAccountRecord, isTerminalCodexPoolRefreshFailure, CodexCredentialGenerationConflictError, CodexCredentialRefreshLockTimeoutError, CodexCredentialRefreshBusyError, CodexCredentialRefreshStaleError, TokenRefreshError } from "../account-store"; import type { PoolQuotaWriter } from "../quota-types"; import { isValidWhamHistoryObservation, getAccountQuota, isCompleteCodexQuotaRecoverySnapshot, parseUsageQuota, setAccountQuotaFromParsed } from "../quota"; import type { StoredAccountQuota, WhamUsageResponse } from "../quota"; @@ -401,7 +401,13 @@ export async function fetchFreshPoolAccountQuota( quotaProbeSkipped: true, }, quotaProbeEvidence); } - if (e instanceof TokenRefreshError) { + // Terminal means the grant itself is dead or missing; an `unknown` refresh failure (a + // token-endpoint 5xx, a transport blip) may clear, so it reports transient like the + // 401-recovery path instead of quarantining a healthy account (#2887). The dead grant is + // marked so the next listing still names the cause rather than flipping back to healthy + // once this response's quota snapshot is cached. + if (isTerminalCodexPoolRefreshFailure(e)) { + markAccountNeedsReauth(accountId, captureConfigGeneration(), requestCredentialGeneration); return withQuotaProbeEvidence( { quota: existing ?? null, needsReauth: true, reauthReason: "refresh_failed", credentialGeneration: requestCredentialGeneration }, quotaProbeEvidence, diff --git a/src/oauth/health.ts b/src/oauth/health.ts index 3379f700aeb..7f596f13464 100644 --- a/src/oauth/health.ts +++ b/src/oauth/health.ts @@ -231,6 +231,16 @@ export function projectCodexAccountHealth(input: { const needsReauth = input.needsReauth || validationAuthFailed || terminalGrantFailure; + // A stored verification failure carries its own cause. When the caller did not name a + // reason, surface the persisted one instead of flattening every verdict into + // `refresh_failed`. + const storedVerificationFailed = validationAuthFailed || terminalGrantFailure; + const storedFailureReason: "unauthorized" | "forbidden" | undefined = !storedVerificationFailed + ? undefined + : record?.lastCodexValidationError === "http_status:403" ? "forbidden" + : record?.lastCodexValidationError === "http_status:401" ? "unauthorized" + : undefined; + // Deferred validation is only worth reporting while the credential itself is still viable. A // revoked grant needs a re-login, not a "Refresh quotas" click, so reauth is resolved first. if (!needsReauth && record?.codexValidationPending) { @@ -240,7 +250,7 @@ export function projectCodexAccountHealth(input: { const snap = getCodexAccountHealthSnapshot(input.accountId, now); return projectOAuthAccountHealth({ needsReauth, - reauthReason: needsReauth ? (input.reauthReason ?? "refresh_failed") : undefined, + reauthReason: needsReauth ? (input.reauthReason ?? storedFailureReason ?? "refresh_failed") : undefined, cooldownUntilMs: snap?.cooldownUntil, cooldownReason: cooldownReasonFromSource(snap?.cooldownSource), now, diff --git a/tests/codex-integration/codex-auth-api.test.ts b/tests/codex-integration/codex-auth-api.test.ts index 3c885491164..23ea7c8b7f9 100644 --- a/tests/codex-integration/codex-auth-api.test.ts +++ b/tests/codex-integration/codex-auth-api.test.ts @@ -2100,10 +2100,15 @@ describe("codex-auth API", () => { const req = new Request("http://localhost/api/codex-auth/accounts?refresh=1"); const resp = await handleCodexAuthAPI(req, new URL(req.url), config); - const data = await resp!.json() as { accounts: Array<{ id: string; reauthReason?: string }> }; + const data = await resp!.json() as { + accounts: Array<{ id: string; reauthReason?: string; health?: { status: string; reason?: string } }>; + }; expect(data.accounts.find(account => account.id === "pool-quota-rejected")) - .toMatchObject({ reauthReason: "quota_unauthorized" }); + .toMatchObject({ + reauthReason: "quota_unauthorized", + health: { status: "reauth_required", reason: "unauthorized" }, + }); }); test("pool token refresh rejection reports refresh failure as the reauthentication cause", async () => { @@ -2113,6 +2118,7 @@ describe("codex-auth API", () => { email: "pool-refresh-rejected@example.com", expiresAt: Date.now() - 1, }); + setAccountQuotaFromParsed("pool-refresh-rejected", { weeklyPercent: 12 }, captureConfigGeneration()); const urls: string[] = []; globalThis.fetch = (async input => { urls.push(String(input)); @@ -2121,11 +2127,52 @@ describe("codex-auth API", () => { const req = new Request("http://localhost/api/codex-auth/accounts/refresh", { method: "POST" }); const resp = await handleCodexAuthAPI(req, new URL(req.url), config); - const data = await resp!.json() as { accounts: Array<{ id: string; reauthReason?: string }> }; + const data = await resp!.json() as { + accounts: Array<{ id: string; needsReauth?: boolean; reauthReason?: string }>; + }; expect(urls).toEqual(["https://auth.openai.com/oauth/token"]); expect(data.accounts.find(account => account.id === "pool-refresh-rejected")) .toMatchObject({ reauthReason: "refresh_failed" }); + + // A dead grant stays dead: the cached-quota listing performs no refresh at all, so only + // the persisted reauth mark keeps the cause visible instead of flipping back to healthy. + const cachedReq = new Request("http://localhost/api/codex-auth/accounts"); + const cachedResp = await handleCodexAuthAPI(cachedReq, new URL(cachedReq.url), config); + const cachedData = await cachedResp!.json() as { + accounts: Array<{ id: string; needsReauth?: boolean; reauthReason?: string }>; + }; + expect(urls).toEqual(["https://auth.openai.com/oauth/token"]); + expect(cachedData.accounts.find(account => account.id === "pool-refresh-rejected")) + .toMatchObject({ needsReauth: true, reauthReason: "refresh_failed" }); + }); + + test("a transient pool token refresh failure does not raise reauthentication", async () => { + const config = makeConfig(); + seedPoolAccount(config, { + id: "pool-refresh-transient", + email: "pool-refresh-transient@example.com", + expiresAt: Date.now() - 1, + }); + // A token-endpoint 5xx classifies as `unknown`, which the account store treats as + // transient: the credential may still be fine, so no reauth cause may surface (#2887). + globalThis.fetch = (async () => Response.json({ error: "server_error" }, { status: 500 })) as typeof fetch; + + const req = new Request("http://localhost/api/codex-auth/accounts/refresh", { method: "POST" }); + const resp = await handleCodexAuthAPI(req, new URL(req.url), config); + const data = await resp!.json() as { + accounts: Array<{ + id: string; + needsReauth?: boolean; + reauthReason?: string; + health?: { status: string; reason?: string }; + }>; + }; + + const account = data.accounts.find(row => row.id === "pool-refresh-transient"); + expect(account?.needsReauth).toBe(false); + expect(account?.reauthReason).toBeUndefined(); + expect(account?.health?.status).not.toBe("reauth_required"); }); test("pool plan refresh batches multiple authoritative changes into one config save", async () => { @@ -5375,14 +5422,17 @@ describe("codex-auth API", () => { if (restart) clearAccountNeedsReauth(accountId); const rows = await listCodexAuthAccounts(config, false); const authFailed = !replace && (status === 401 || status === 403); + // A bare runtime mark only knows the refresh failed; once volatile state is cleared the + // stored verdict's own http status names the cause instead. + const expectedReason = restart ? (status === 403 ? "forbidden" : "unauthorized") : "refresh_failed"; const row = rows.find(entry => entry.id === accountId); expect(row).toMatchObject({ needsReauth: authFailed, - health: { status: authFailed ? "reauth_required" : "warning", reason: authFailed ? "refresh_failed" : "validation_pending" }, + health: { status: authFailed ? "reauth_required" : "warning", reason: authFailed ? expectedReason : "validation_pending" }, }); // The reason travels with the state, so an operator reading the account surface can tell a // failed refresh from a pending validation without inferring it from `health` (#4212). - if (authFailed) expect(row).toMatchObject({ reauthReason: "refresh_failed" }); + if (authFailed) expect(row).toMatchObject({ reauthReason: expectedReason }); else expect(row).not.toHaveProperty("reauthReason"); fail = false; await refresh(); diff --git a/tests/oauth/oauth-health.test.ts b/tests/oauth/oauth-health.test.ts index 8b3aa068b6d..2f42bac60b8 100644 --- a/tests/oauth/oauth-health.test.ts +++ b/tests/oauth/oauth-health.test.ts @@ -166,6 +166,25 @@ describe("projectCodexAccountHealth", () => { .toEqual({ status: "healthy" }); }); + test.each([ + ["http_status:401", "unauthorized"], + ["http_status:403", "forbidden"], + ] as const)("a stored verification failure surfaces %s as %s", async (validationError, reason) => { + withPoolStoreDir(); + const { markCodexAccountValidationFailed, readCodexAccountRecord, saveCodexAccountCredential } = + await import("../../src/codex/account-store"); + const accountId = "pool-stored-" + reason; + saveCodexAccountCredential(accountId, { + accessToken: "a", refreshToken: "r", expiresAt: Date.now() + 3_600_000, chatgptAccountId: "cg", + }, { validationPending: true }); + markCodexAccountValidationFailed(accountId, validationError, { + expectedGeneration: readCodexAccountRecord(accountId)!.generation, + }); + + expect(projectCodexAccountHealth({ accountId, needsReauth: false })) + .toEqual({ status: "reauth_required", reason }); + }); + test("the CLI collector reports the terminal verdict too", async () => { withPoolStoreDir(); const { markCodexAccountValidationFailed, readCodexAccountRecord, saveCodexAccountCredential } =