diff --git a/devlog/_plan/260911_l3_account_pool/020_wp2_4212_attribution.md b/devlog/_plan/260911_l3_account_pool/020_wp2_4212_attribution.md new file mode 100644 index 0000000000..63bf52d267 --- /dev/null +++ b/devlog/_plan/260911_l3_account_pool/020_wp2_4212_attribution.md @@ -0,0 +1,73 @@ +# WP2 — #4212 a pool account stuck on a failed credential refresh drops its models without naming itself + +## Scope, and why this is `Refs` rather than `Closes` + +The reporter saw two things: gated models vanished from the model list, and requests failed with a +generic 503. Neither publisher is L3's. + +- The 503 they quoted is inlined at `src/server/responses/core.ts:2336` and `compact.ts:383`. L1 owns + both. Recorded as a follow-up. +- The model-list drop is published from `src/codex/catalog/sync.ts:1777`. Out of scope. Recorded as a + follow-up. + +What L3 owns is the layer underneath: the decision that removes the account, and the surfaces that +describe it. Packet decision, followed as written. + +## The actual defect + +The issue names it precisely: `isAccountNeedsReauth(accountId)` makes the account unselectable +"with no reason carried to callers". `isCodexAccountUsable()` returned a bare boolean, so every +surface that wanted to explain a refusal had to re-derive the cause from a different source. That is +how a surface ends up reporting an account healthy while routing is dropping it. + +So the reason now comes from the same function as the decision. `codexAccountUnusableReason()` holds +every branch and returns the cause; `isCodexAccountUsable()` is its boolean projection rather than a +second copy. A reason cannot name a cause routing did not use, and routing cannot refuse an account +for a cause no surface can name. + +That refactor is the risky part of this change — `isCodexAccountUsable` is called from routing, +auth-context, sidecar auth, and subagent fallback — so it was audited for exact equivalence rather +than reviewed by eye. See below. + +## Attribution on the account surface + +`poolAccountDto` computed `needsReauth` as an OR of three independent causes plus a persisted +verdict resolved inside the health projection, and emitted only the boolean. It now also emits +`reauthReason`: `missing_credential` for a credential that was never stored, `refresh_failed` for a +refresh that keeps failing — the reporter's case — and `quota_unauthorized` when the usage lookup +itself was rejected. `/api/oauth/accounts` already carried that field name, so the Codex account +surface now matches its sibling. The main row carries it too, so the field's contract holds for +every row rather than only pool rows. + +## The refusal string + +`nativeMainRefreshFailureResponse` said "retry this request" and nothing else, which is how the +reporter concluded the proxy had broken. It stays a retryable 503 with `Retry-After`, because the +refresh genuinely may succeed, and now adds that a failure which persists means the main account +needs reauthentication. + +The pool-account 401 was deliberately left alone. `tests/server/server-search.test.ts:344` asserts +that message must not contain the account id, alias, or email — naming the account there is a +privacy decision this repository already made against, and it is not L3's to reverse. + +## Audit + +Three read-only `xai/grok-4.6` subagents, in parallel. + +- **Equivalence (pass).** No input changes the truth value, helper call count, call order, or throw + set. `readCodexAccountRecord` is still called exactly once and only after the existence and reauth + checks; the `isMainAccountTokenLive` seam still fires 0 or 1 times, not 2; the expanded pool tail + is truth-equivalent for a null record, a record without a credential, `deletedAt` set, and + `codexValidationPending`. +- **DTO and error layer (pass, 5 non-blocking findings).** Three were folded in: the main row now + carries `reauthReason`, the union comment no longer overclaims what the current health projection + can produce, and the DTO-layer assertion was added to the existing refresh test. Two were recorded + rather than fixed: the request-path 401 (privacy, above) and the GUI not yet reading the field. +- **Re-audit after fold-in (pass).** Confirmed the main-row `||` still short-circuits so the + stale-generation cleanup call count is unchanged, that the main row cannot emit a reason without + the boolean or the reverse, and that all eight locale inserts are localized and correctly placed. + +## Verification + +Local suite, typecheck, and build: NOT RUN by operator instruction. Hosted CI on the pushed head is +the evidence. diff --git a/docs-site/src/content/docs/fr/guides/codex-integration.md b/docs-site/src/content/docs/fr/guides/codex-integration.md index bd8157e747..1d9704e55f 100644 --- a/docs-site/src/content/docs/fr/guides/codex-integration.md +++ b/docs-site/src/content/docs/fr/guides/codex-integration.md @@ -383,6 +383,12 @@ Si la lecture authentifiée des quotas avec le nouveau jeton OAuth confirme un q La revalidation en arrière-plan est distincte et désactivée par défaut. Elle nécessite Token Guardian, la politique `proactive` du fournisseur `openai` et `tokenGuardian.codexWarmupEnabled`, et ignore les comptes dont la validation d’inscription est en attente. +### Pourquoi un compte a cessé de servir les requêtes + +Lorsqu'un compte quitte la sélection du pool, la raison accompagne la décision au lieu d'être recalculée pour l'affichage : une interface ne peut donc pas présenter un compte comme sain pendant que le routage l'écarte. `GET /api/codex-auth/accounts` expose `reauthReason` à côté de `needsReauth` pour chaque compte : `missing_credential` si aucun identifiant n'a été enregistré, `refresh_failed` si le renouvellement échoue de façon répétée, et `quota_unauthorized` si la lecture des quotas elle-même a été refusée. + +Un renouvellement du compte principal qui n'aboutit pas répond toujours `503` avec `Retry-After`, car une nouvelle tentative peut réussir. Le message précise désormais qu'un échec persistant signifie que le compte principal doit être réauthentifié, au lieu de demander seulement de réessayer. + ## Restauration de Codex natif `ocx stop` arrête le proxy et le service d'arrière-plan installé, puis tente de restaurer Codex natif. OpenCodex retire les éléments de routage dont il peut vérifier la propriété et signale une restauration incomplète si les fichiers de configuration ne peuvent pas être récupérés en toute sécurité. diff --git a/docs-site/src/content/docs/guides/codex-integration.md b/docs-site/src/content/docs/guides/codex-integration.md index 397ef277c6..c1e296eb49 100644 --- a/docs-site/src/content/docs/guides/codex-integration.md +++ b/docs-site/src/content/docs/guides/codex-integration.md @@ -714,6 +714,12 @@ If the new OAuth credential's authenticated usage lookup confirms an exhausted 5 Background revalidation is separate and off by default. It requires Token Guardian, the `openai` provider's `proactive` refresh policy, and `tokenGuardian.codexWarmupEnabled`. It skips accounts awaiting deferred registration validation. +### Why an account stopped serving requests + +When an account leaves pool selection, the reason travels with the decision instead of being recomputed for display, so a surface can never report an account healthy while routing is dropping it. `GET /api/codex-auth/accounts` carries `reauthReason` next to `needsReauth` on each account: `missing_credential` for a credential that was never stored, `refresh_failed` for a credential refresh that keeps failing, and `quota_unauthorized` when the usage lookup itself was rejected. + +A main-account refresh that does not complete still answers `503` with `Retry-After`, because a retry may still succeed. The message now adds that a failure which persists means the main account needs reauthentication, rather than only asking for another attempt. + ## Restoring native Codex `ocx stop` stops the proxy and any installed background service, then attempts to restore native Codex. OpenCodex removes verified routing artifacts and reports an incomplete restore when it cannot safely recover configuration files. diff --git a/docs-site/src/content/docs/ja/guides/codex-integration.md b/docs-site/src/content/docs/ja/guides/codex-integration.md index 58181c0aba..e64a87e77b 100644 --- a/docs-site/src/content/docs/ja/guides/codex-integration.md +++ b/docs-site/src/content/docs/ja/guides/codex-integration.md @@ -249,6 +249,12 @@ ocx service install # persistent: auto-starts on login and respawns on crash バックグラウンド再検証は別機能で既定では無効です。Token Guardian、`openai` の `proactive` 更新ポリシー、`tokenGuardian.codexWarmupEnabled` が必要で、登録検証待ちのアカウントは除外します。 +### アカウントがリクエストを処理しなくなった理由 + +アカウントがプール選択から外れるとき、その理由は表示用に再計算されるのではなく判断とともに伝わります。そのため、ルーティングが除外している最中に画面が正常と表示することはありません。`GET /api/codex-auth/accounts` は各アカウントの `needsReauth` と並べて `reauthReason` を返します。資格情報が保存されていない場合は `missing_credential`、更新が繰り返し失敗する場合は `refresh_failed`、使用量の取得自体が拒否された場合は `quota_unauthorized` です。 + +メインアカウントの更新が完了しない場合も、再試行で成功する可能性があるため `Retry-After` 付きの `503` を返します。ただしメッセージには、失敗が続くならメインアカウントの再認証が必要である旨を加えました。 + ## ネイティブ Codexの復元 `ocx stop` はプロキシとインストール済みのバックグラウンドサービスを停止し、ネイティブ Codex の復元を試みます。OpenCodex は所有を確認できるルーティング設定を削除し、設定ファイルを安全に復元できない場合は未完了として報告します。 diff --git a/docs-site/src/content/docs/ko/guides/codex-integration.md b/docs-site/src/content/docs/ko/guides/codex-integration.md index 391a3e63fc..12c26d382e 100644 --- a/docs-site/src/content/docs/ko/guides/codex-integration.md +++ b/docs-site/src/content/docs/ko/guides/codex-integration.md @@ -260,6 +260,12 @@ ChatGPT 계정을 추가하거나 재인증할 때 OpenCodex는 일반적으로 별도의 백그라운드 재검증은 기본적으로 꺼져 있습니다. Token Guardian, `openai`의 `proactive` 갱신 정책, `tokenGuardian.codexWarmupEnabled`가 필요하며 등록 검증 대기 계정은 제외합니다. +### 계정이 요청을 처리하지 못하게 된 이유 + +계정이 풀 선택에서 빠질 때 그 이유는 표시용으로 다시 계산되지 않고 판단과 함께 전달됩니다. 라우팅이 계정을 제외하는 동안 화면에서만 정상으로 보이는 일이 생기지 않습니다. `GET /api/codex-auth/accounts`는 계정마다 `needsReauth` 옆에 `reauthReason`을 함께 반환합니다. 자격 증명이 저장된 적 없으면 `missing_credential`, 갱신이 계속 실패하면 `refresh_failed`, 사용량 조회 자체가 거부되면 `quota_unauthorized`입니다. + +메인 계정 갱신이 끝나지 않은 경우에도 재시도로 성공할 수 있으므로 `Retry-After`와 함께 `503`을 반환합니다. 다만 실패가 계속되면 메인 계정을 다시 인증해야 한다는 내용을 메시지에 덧붙였습니다. + ## 네이티브 Codex 복원 `ocx stop`은 proxy와 설치된 background service를 중지한 뒤 네이티브 Codex 복원을 시도합니다. OpenCodex 소유로 확인된 라우팅 항목을 제거하며, 설정 파일을 안전하게 복구할 수 없으면 미완료로 보고합니다. diff --git a/docs-site/src/content/docs/ru/guides/codex-integration.md b/docs-site/src/content/docs/ru/guides/codex-integration.md index a22c347f2d..44dcc35bff 100644 --- a/docs-site/src/content/docs/ru/guides/codex-integration.md +++ b/docs-site/src/content/docs/ru/guides/codex-integration.md @@ -376,6 +376,12 @@ v1/base/v2 при делегировании и fallback — в Фоновая проверка — отдельная функция, выключенная по умолчанию. Она требует Token Guardian, политики `proactive` провайдера `openai` и `tokenGuardian.codexWarmupEnabled` и пропускает аккаунты, ожидающие проверки регистрации. +### Почему аккаунт перестал обслуживать запросы + +Когда аккаунт выпадает из выбора пула, причина передаётся вместе с решением, а не вычисляется заново для отображения, поэтому интерфейс не может показывать аккаунт исправным, пока маршрутизация его исключает. `GET /api/codex-auth/accounts` возвращает `reauthReason` рядом с `needsReauth` для каждого аккаунта: `missing_credential` — учётные данные не сохранялись, `refresh_failed` — обновление продолжает падать, `quota_unauthorized` — сам запрос квоты отклонён. + +Незавершённое обновление основного аккаунта по-прежнему отвечает `503` с `Retry-After`, потому что повтор может пройти. Теперь сообщение добавляет, что стойкий сбой означает необходимость повторной аутентификации основного аккаунта, а не только очередную попытку. + ## Восстановление нативного Codex `ocx stop` останавливает прокси и установленную фоновую службу, затем пытается восстановить нативный Codex. OpenCodex удаляет настройки маршрутизации, принадлежность которых может подтвердить, и сообщает о неполном восстановлении, если файлы конфигурации нельзя безопасно восстановить. diff --git a/docs-site/src/content/docs/tr/guides/codex-integration.md b/docs-site/src/content/docs/tr/guides/codex-integration.md index fcf4f8de66..fcfa9f299d 100644 --- a/docs-site/src/content/docs/tr/guides/codex-integration.md +++ b/docs-site/src/content/docs/tr/guides/codex-integration.md @@ -433,6 +433,12 @@ Yeni OAuth belirteciyle yapılan kota sorgusu 5 saatlik, haftalık veya aylık k Arka plan doğrulaması ayrı ve varsayılan olarak kapalıdır. Token Guardian, `openai` için `proactive` yenileme ilkesi ve `tokenGuardian.codexWarmupEnabled` gerektirir; kayıt doğrulaması bekleyen hesapları atlar. +### Bir hesabın istek karşılamayı bırakma nedeni + +Bir hesap havuz seçiminden çıktığında neden, görüntüleme için yeniden hesaplanmak yerine kararla birlikte taşınır; böylece yönlendirme hesabı dışarıda bırakırken hiçbir yüzey onu sağlıklı gösteremez. `GET /api/codex-auth/accounts` her hesapta `needsReauth` yanında `reauthReason` döndürür: kimlik bilgisi hiç kaydedilmediyse `missing_credential`, yenileme sürekli başarısızsa `refresh_failed`, kullanım sorgusunun kendisi reddedildiyse `quota_unauthorized`. + +Tamamlanmayan bir ana hesap yenilemesi, yeniden denemede başarılı olabileceği için hâlâ `Retry-After` ile `503` yanıtı verir. Mesaj artık kalıcı bir başarısızlığın ana hesabın yeniden kimlik doğrulaması gerektirdiğini de belirtiyor. + ## Yerel Codex'i geri yükleme `ocx stop`, proxy'yi ve kurulu arka plan servisini durdurur, ardından yerel Codex'i geri yüklemeyi dener. OpenCodex yalnızca sahipliğini doğrulayabildiği yönlendirme öğelerini kaldırır; yapılandırma dosyaları güvenle geri yüklenemiyorsa işlemin tamamlanmadığını bildirir. diff --git a/docs-site/src/content/docs/zh-cn/guides/codex-integration.md b/docs-site/src/content/docs/zh-cn/guides/codex-integration.md index bfe76b0c72..92a3e94368 100644 --- a/docs-site/src/content/docs/zh-cn/guides/codex-integration.md +++ b/docs-site/src/content/docs/zh-cn/guides/codex-integration.md @@ -320,6 +320,12 @@ fallback 行为，参见 [Sub-agent Surface](/guides/sub-agent-surface/)。 后台重新验证是独立功能，默认关闭。它要求 Token Guardian、`openai` 的 `proactive` 刷新策略及 `tokenGuardian.codexWarmupEnabled`，并跳过等待注册验证的账号。 +### 账号停止处理请求的原因 + +账号退出账号池选择时，原因随判定一起传递，而不是为显示重新计算，因此界面不会在路由已排除该账号时仍显示正常。`GET /api/codex-auth/accounts` 在每个账号的 `needsReauth` 旁返回 `reauthReason`：从未保存凭据为 `missing_credential`，刷新持续失败为 `refresh_failed`，用量查询本身被拒绝为 `quota_unauthorized`。 + +主账号刷新未完成时仍返回带 `Retry-After` 的 `503`，因为重试仍可能成功。消息中现在补充说明：若持续失败，则主账号需要重新认证，而不只是再试一次。 + ## 恢复原生 Codex `ocx stop` 会停止 proxy 和已安装的后台服务，然后尝试恢复原生 Codex。OpenCodex 只移除能够确认归属的路由配置；如果无法安全恢复配置文件，会报告恢复未完成。 diff --git a/docs-site/src/content/docs/zh-tw/guides/codex-integration.md b/docs-site/src/content/docs/zh-tw/guides/codex-integration.md index 708341e5bf..4da96fe7f1 100644 --- a/docs-site/src/content/docs/zh-tw/guides/codex-integration.md +++ b/docs-site/src/content/docs/zh-tw/guides/codex-integration.md @@ -327,6 +327,12 @@ ocx service install # 常駐：登入時自動啟動，崩潰後自動重新 背景重新驗證是獨立功能，預設關閉。它需要 Token Guardian、`openai` 的 `proactive` 更新政策及 `tokenGuardian.codexWarmupEnabled`，並略過等待註冊驗證的帳號。 +### 帳號停止處理請求的原因 + +帳號退出帳號池選擇時，原因會隨判定一起傳遞，而不是為了顯示重新計算，因此介面不會在路由已排除該帳號時仍顯示正常。`GET /api/codex-auth/accounts` 會在每個帳號的 `needsReauth` 旁回傳 `reauthReason`：從未儲存憑證為 `missing_credential`，更新持續失敗為 `refresh_failed`，用量查詢本身遭拒為 `quota_unauthorized`。 + +主帳號更新未完成時仍回傳帶 `Retry-After` 的 `503`，因為重試仍可能成功。訊息現在補充說明：若持續失敗，代表主帳號需要重新認證，而不只是再試一次。 + ## 恢復原生 Codex `ocx stop` 會停止 proxy 與已安裝的背景服務，然後嘗試恢復原生 Codex。OpenCodex 只移除能確認歸屬的路由設定；若無法安全恢復設定檔，會回報恢復未完成。 diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index b5ea45c4a3..2ab3271e2b 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -382,6 +382,7 @@ "codex-account-mode-state.test.ts": "gui", "codex-account-namespaces.test.ts": "codex-integration", "codex-account-store.test.ts": "codex-integration", + "codex-account-unusable-reason.test.ts": "codex-integration", "codex-admission-primitives.test.ts": "codex-integration", "codex-admission.test.ts": "codex-integration", "codex-affinity-debug.test.ts": "codex-integration", diff --git a/src/codex/account-usability.ts b/src/codex/account-usability.ts index 3001eb2b23..5427fed841 100644 --- a/src/codex/account-usability.ts +++ b/src/codex/account-usability.ts @@ -20,34 +20,70 @@ export interface CodexAccountUsabilityOptions { modelEligibleAccountIds?: ReadonlySet; } -export function isCodexAccountUsable( +/** + * Why an account was refused, in the order the checks run. This is the attribution half of + * selection: an operator whose model quietly disappeared needs to know that one account fell out + * and why, not merely that the pool got smaller (#4212). + */ +export type CodexAccountUnusableReason = + | "model_not_entitled" + | "main_hard_locked" + | "main_traffic_blocked" + | "legacy_pool_sentinel" + | "needs_reauth" + | "main_credential_unavailable" + | "not_in_pool" + | "missing_credential" + | "deleted" + | "validation_pending"; + +/** + * The single source of truth for both selection and its explanation. `isCodexAccountUsable()` is + * this function's boolean projection rather than a parallel copy of the same branches, so a reason + * can never claim an account is fine while routing drops it, or name a cause routing did not use. + */ +export function codexAccountUnusableReason( config: OcxConfig, accountId: string, options: CodexAccountUsabilityOptions = {}, -): boolean { - if (options.modelEligibleAccountIds && !options.modelEligibleAccountIds.has(accountId)) return false; +): CodexAccountUnusableReason | undefined { + if (options.modelEligibleAccountIds && !options.modelEligibleAccountIds.has(accountId)) { + return "model_not_entitled"; + } if (accountId === MAIN_CODEX_ACCOUNT_ID) { - if (isMainAccountHardLocked(config)) return false; + if (isMainAccountHardLocked(config)) return "main_hard_locked"; // Startup recovery owns the physical auth/vault boundary. Never parse or select // native __main__ while an encrypted switch journal is pending or inconclusive. - if (!options.nativeMainSelectionOnly && isNativeMainTrafficBlocked()) return false; + if (!options.nativeMainSelectionOnly && isNativeMainTrafficBlocked()) return "main_traffic_blocked"; // A legacy pool row with the sentinel makes an active `__main__` ambiguous. // Fail closed until the authenticated compatibility-delete path removes it. - if (hasLegacyMainCodexPoolAccount(config.codexAccounts)) return false; - if (isAccountNeedsReauth(accountId) && !hasMainAccountRefreshGrant()) return false; + if (hasLegacyMainCodexPoolAccount(config.codexAccounts)) return "legacy_pool_sentinel"; + if (isAccountNeedsReauth(accountId) && !hasMainAccountRefreshGrant()) return "needs_reauth"; // A selection-only caller owns the recovery/drain fence and will reject main // before reservation or token materialization. Treat cached main as a routing // candidate without touching the credential file so affinity is not rebound. - if (options.nativeMainSelectionOnly) return true; + if (options.nativeMainSelectionOnly) return undefined; // Main account: a refresh grant is enough to route; materialization refreshes before I/O. - return options.isMainAccountTokenLive + const mainLive = options.isMainAccountTokenLive ? options.isMainAccountTokenLive() : isMainAccountCredentialUsable(); + return mainLive ? undefined : "main_credential_unavailable"; } const exists = (config.codexAccounts ?? []) .some(account => isSelectableCodexPoolAccount(account) && account.id === accountId); - if (!exists) return false; - if (isAccountNeedsReauth(accountId)) return false; + if (!exists) return "not_in_pool"; + if (isAccountNeedsReauth(accountId)) return "needs_reauth"; const record = readCodexAccountRecord(accountId); - return !!record?.credential && record.deletedAt == null && !record.codexValidationPending; + if (!record?.credential) return "missing_credential"; + if (record.deletedAt != null) return "deleted"; + if (record.codexValidationPending) return "validation_pending"; + return undefined; +} + +export function isCodexAccountUsable( + config: OcxConfig, + accountId: string, + options: CodexAccountUsabilityOptions = {}, +): boolean { + return codexAccountUnusableReason(config, accountId, options) === undefined; } diff --git a/src/codex/auth-api.ts b/src/codex/auth-api.ts index 79fca9470f..09becf51ea 100644 --- a/src/codex/auth-api.ts +++ b/src/codex/auth-api.ts @@ -364,6 +364,20 @@ 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. + */ +export type CodexAccountReauthReason = + | "missing_credential" + | "refresh_failed" + | "quota_unauthorized" + | "unauthorized" + | "forbidden"; + function poolAccountDto( account: CodexAccount, quotaResult: PoolQuotaResult, @@ -374,8 +388,19 @@ function poolAccountDto( ): CodexAuthAccountDto { const plan = codexPlanValue(account.plan); const quota = quotaForPlan(quotaResult.quota, plan); - const needsReauth = !hasCredential || quotaResult.needsReauth || isAccountNeedsReauth(account.id); + const runtimeReauth = isAccountNeedsReauth(account.id); + const needsReauth = !hasCredential || quotaResult.needsReauth || runtimeReauth; const health = projectCodexAccountHealth({ accountId: account.id, needsReauth }); + // `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; return { id: account.id, email: projectEmail(account.email, maskEmails) ?? account.email, @@ -387,6 +412,7 @@ function poolAccountDto( priority, quota: quota ? { ...quota } : null, needsReauth: needsReauth || health.status === "reauth_required", + ...(reauthReason !== undefined ? { reauthReason } : {}), hasCredential, ...(quotaResult.quotaProbeSkipped ? { quotaProbeSkipped: true as const } : {}), ...oauthAccountHealthFields("codex", account.id, health), @@ -1161,6 +1187,11 @@ export interface CodexAuthAccountDto { priority: number; quota: (StoredAccountQuota | (Omit & { updatedAt: number })) | null; needsReauth?: boolean; + /** + * Which of the independent causes behind `needsReauth` fired. Present only when the account + * needs the operator; `/api/oauth/accounts` already carries the same field name. + */ + reauthReason?: CodexAccountReauthReason; hasCredential: boolean; health: OAuthAccountHealth; healthLabel: OAuthHealthLabel; @@ -2009,12 +2040,20 @@ export async function listCodexAuthAccountsSnapshot( const hasMainCredential = mainSnapshotLive && mainResult.credentialChecked ? mainResult.hasCredential : getMainAccountCredentialPresence() ?? false; - const mainNeedsReauth = (mainSnapshotLive && mainResult.credentialChecked && !hasMainCredential) - || isAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); + const mainMissingCredential = mainSnapshotLive && mainResult.credentialChecked && !hasMainCredential; + const mainNeedsReauth = mainMissingCredential || isAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); const mainHealth = projectCodexAccountHealth({ accountId: MAIN_CODEX_ACCOUNT_ID, needsReauth: mainNeedsReauth, }); + // The main row carries the same attribution as a pool row. Reaching this point without + // `mainMissingCredential` means the runtime reauth flag is what set `mainNeedsReauth`, so the + // cause is a refresh that did not complete. + const mainReauthReason: CodexAccountReauthReason | undefined = mainMissingCredential + ? "missing_credential" + : mainNeedsReauth + ? "refresh_failed" + : mainHealth.status === "reauth_required" ? mainHealth.reason : undefined; const main: CodexAuthAccountDto = { id: MAIN_CODEX_ACCOUNT_ID, email: projectEmail(mainInfo.email, maskEmails) ?? "Codex App login", @@ -2029,6 +2068,7 @@ export async function listCodexAuthAccountsSnapshot( priority: getCodexAccountPriority(runtimeConfig, MAIN_CODEX_ACCOUNT_ID), hasCredential: hasMainCredential, needsReauth: mainNeedsReauth, + ...(mainReauthReason !== undefined ? { reauthReason: mainReauthReason } : {}), quota: mainInfo.quota ? { ...quotaForPlan(mainQuotaWithCarriedResetCredits(mainInfo.quota), mainInfo.plan), } : null, diff --git a/src/server/responses/codex-auth-error.ts b/src/server/responses/codex-auth-error.ts index 8d84c54392..8ae85e837b 100644 --- a/src/server/responses/codex-auth-error.ts +++ b/src/server/responses/codex-auth-error.ts @@ -29,10 +29,15 @@ export function nativeMainRefreshFailureResponse(error: unknown): Response { if (error instanceof MainAccountTokenRefreshError || error instanceof MainAuthJsonChangedDuringRefreshError || (error instanceof NativeProfileError && error.retryable)) { + // A bare "retry this request" reads as a transient server fault, which is how #4212's reporter + // concluded the proxy had broken while one account was the thing that needed them. The refusal + // stays a retryable 503 because the refresh genuinely may succeed, but it now names what is + // failing and what to do when retrying stops helping. const response = formatErrorResponse( 503, "server_busy", - "Codex main credential refresh did not complete; retry this request", + "Codex main credential refresh did not complete; retry this request. " + + "If it keeps failing, the main Codex account needs reauthentication.", ); const headers = new Headers(response.headers); headers.set("Retry-After", "1"); diff --git a/tests/codex-integration/codex-account-unusable-reason.test.ts b/tests/codex-integration/codex-account-unusable-reason.test.ts new file mode 100644 index 0000000000..fbb4c452fe --- /dev/null +++ b/tests/codex-integration/codex-account-unusable-reason.test.ts @@ -0,0 +1,142 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { + codexAccountUnusableReason, + isCodexAccountUsable, + type CodexAccountUnusableReason, +} from "../../src/codex/account-usability"; +import { saveCodexAccountCredential } from "../../src/codex/account-store"; +import { clearAccountNeedsReauth, markAccountNeedsReauth } from "../../src/codex/account-runtime-state"; +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"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +const STORE_DIR = join(import.meta.dir, ".tmp-unusable-reason-store"); +const CODEX_DIR = join(import.meta.dir, ".tmp-unusable-reason-codex"); +let prevOpencodexHome: string | undefined; +let prevCodexHome: string | undefined; + +function writeMainAuth(): void { + mkdirSync(CODEX_DIR, { recursive: true }); + writeFileSync( + join(CODEX_DIR, "auth.json"), + JSON.stringify({ tokens: { access_token: "main_access", account_id: "main_acct" } }), + ); +} + +function saveCred(id: string): void { + saveCodexAccountCredential(id, { + accessToken: `access-${id}`, + refreshToken: `refresh-${id}`, + expiresAt: Date.now() + 5 * 60_000, + chatgptAccountId: `acct-${id}`, + }); +} + +function makeConfig(): OcxConfig { + return { + providers: {}, + codexAccounts: [ + { id: "paid", email: "paid@test", isMain: false }, + { id: "stuck", email: "stuck@test", isMain: false }, + { id: "uncredentialed", email: "none@test", isMain: false }, + ], + activeCodexAccountId: "paid", + } as OcxConfig; +} + +const ACCOUNT_IDS = ["paid", "stuck", "uncredentialed", MAIN_CODEX_ACCOUNT_ID]; + +describe("codex account unusable reason", () => { + beforeEach(() => { + prevOpencodexHome = process.env.OPENCODEX_HOME; + prevCodexHome = process.env.CODEX_HOME; + for (const dir of [STORE_DIR, CODEX_DIR]) if (existsSync(dir)) removeTreeWithRetry(dir); + mkdirSync(STORE_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = STORE_DIR; + process.env.CODEX_HOME = CODEX_DIR; + for (const id of ACCOUNT_IDS) clearAccountNeedsReauth(id); + saveCred("paid"); + saveCred("stuck"); + writeMainAuth(); + }); + + afterEach(() => { + for (const id of ACCOUNT_IDS) clearAccountNeedsReauth(id); + for (const dir of [STORE_DIR, CODEX_DIR]) if (existsSync(dir)) removeTreeWithRetry(dir); + if (prevOpencodexHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = prevOpencodexHome; + if (prevCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = prevCodexHome; + }); + + test("a healthy pool account reports no reason", () => { + expect(codexAccountUnusableReason(makeConfig(), "paid")).toBeUndefined(); + }); + + test("an account stuck on a failed credential refresh names itself", () => { + // The #4212 case: routing drops the account and, before this, said nothing about why. + markAccountNeedsReauth("stuck"); + expect(codexAccountUnusableReason(makeConfig(), "stuck")).toBe("needs_reauth"); + }); + + test("a pool row without a stored credential is distinguishable from a failed refresh", () => { + expect(codexAccountUnusableReason(makeConfig(), "uncredentialed")).toBe("missing_credential"); + }); + + test("an id that is not a pool row reports not_in_pool", () => { + expect(codexAccountUnusableReason(makeConfig(), "never-added")).toBe("not_in_pool"); + }); + + test("an account outside a gated model's entitled set reports model_not_entitled", () => { + const reason = codexAccountUnusableReason(makeConfig(), "paid", { + modelEligibleAccountIds: new Set(["stuck"]), + }); + expect(reason).toBe("model_not_entitled"); + }); + + test("the main account without a native credential reports main_credential_unavailable", () => { + rmSync(join(CODEX_DIR, "auth.json")); + expect(codexAccountUnusableReason(makeConfig(), MAIN_CODEX_ACCOUNT_ID)) + .toBe("main_credential_unavailable"); + }); + + test("the boolean projection never disagrees with the reason", () => { + // isCodexAccountUsable() is defined as this function's projection rather than a second copy of + // the same branches, so an account can never be refused for a cause no surface can name. + markAccountNeedsReauth("stuck"); + const config = makeConfig(); + const cases: { id: string; expected: CodexAccountUnusableReason | undefined }[] = [ + { id: "paid", expected: undefined }, + { id: "stuck", expected: "needs_reauth" }, + { id: "uncredentialed", expected: "missing_credential" }, + { id: "never-added", expected: "not_in_pool" }, + { id: MAIN_CODEX_ACCOUNT_ID, expected: undefined }, + ]; + for (const { id, expected } of cases) { + const reason = codexAccountUnusableReason(config, id); + expect(reason).toBe(expected as CodexAccountUnusableReason); + expect(isCodexAccountUsable(config, id)).toBe(reason === undefined); + } + }); +}); + +describe("native main refresh refusal", () => { + test("a retryable refresh failure stays a 503 but names the account and the action", async () => { + const response = nativeMainRefreshFailureResponse(new MainAccountTokenRefreshError("transient")); + expect(response.status).toBe(503); + expect(response.headers.get("Retry-After")).toBe("1"); + const message = ((await response.json()) as { error: { message: string } }).error.message; + expect(message).toContain("Codex main credential refresh did not complete"); + expect(message).toContain("reauthentication"); + }); + + test("a terminal reauth failure still refuses with 401 rather than a retry promise", async () => { + 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"); + }); +}); diff --git a/tests/codex-integration/codex-auth-api.test.ts b/tests/codex-integration/codex-auth-api.test.ts index ba80be3422..89813bb058 100644 --- a/tests/codex-integration/codex-auth-api.test.ts +++ b/tests/codex-integration/codex-auth-api.test.ts @@ -5281,10 +5281,15 @@ describe("codex-auth API", () => { if (restart) clearAccountNeedsReauth(accountId); const rows = await listCodexAuthAccounts(config, false); const authFailed = !replace && (status === 401 || status === 403); - expect(rows.find(row => row.id === accountId)).toMatchObject({ + 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" }, }); + // 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" }); + else expect(row).not.toHaveProperty("reauthReason"); fail = false; await refresh(); expect(readCodexAccountRecord(accountId)?.codexValidationPending).toBeUndefined(); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index d772205061..2c2c04c2e4 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -217,6 +217,7 @@ "codex-account-mode-state.test.ts": "gui", "codex-account-namespaces.test.ts": "codex-integration", "codex-account-store.test.ts": "codex-integration", + "codex-account-unusable-reason.test.ts": "codex-integration", "codex-admission-primitives.test.ts": "codex-integration", "codex-admission.test.ts": "codex-integration", "codex-affinity-debug.test.ts": "codex-integration",