Repository navigation
fix(codex): treat a bare WHAM 401 as transient while the main token is live - #1932
harryzhou2000 wants to merge 1 commit into
Conversation
|
⏳ DRAFT
What to do
Review readiness checklist
0/4 boxes ticked. This pull request was already a draft. Its draft status will be preserved after every issue above is resolved. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
bb47f79 to
c77aed5
Compare
|
@Wibias — when you get a chance, could you share your opinion on this one? It touches the auth surface (src/codex/auth-api.ts), so per MAINTAINERS.md it needs a maintainer to apply maintainer-sponsored after review. Happy to adjust the approach if you prefer something different. |
c77aed5 to
fa77e5c
Compare
|
@lidge-jun — could you share your opinion on this one when you have a moment? It changes the auth surface (src/codex/auth-api.ts): a bare WHAM 401 with a still-live access token is treated as transient instead of flipping the main account to needs-reauth. It is rebased onto the latest dev (fa77e5c) and tests are green locally; per MAINTAINERS.md it needs maintainer-sponsored after review. Happy to adjust if you prefer a different approach. |
…ly live main token Redesign of #1932. The idea — a bare 401 from WHAM should not flip the main account to needsReauth while the local access token is still valid — is correct, but its liveness gate (isMainAccountTokenLive) treats a JWT whose exp cannot be decoded as live, so a genuinely dead credential would keep every bare 401 'transient' and needsReauth could never flip. This lands the transient-401 gate on a strict liveness check: isMainAccountTokenVerifiablyLive() returns true only for a decodable, still-future exp. An undecodable exp fails toward the terminal/reauth path. Terminal body codes (invalid_refresh_token, invalid_workspace_selected) remain terminal even when the token is live. Credit: #1932 (transient bare-401 concept and test scaffolding). Co-authored-by: PR #1932
fix(codex): treat a bare WHAM 401 as transient only for a verifiably live token (lands #1932)
|
Landed via #2021 — your transient-401 gate with the reviewer-required tightening: liveness now requires a decodable future exp, so a dead credential cannot loop as transient. 186/0 + tsc. Thanks! |
…ly live main token Redesign of lidge-jun#1932. The idea — a bare 401 from WHAM should not flip the main account to needsReauth while the local access token is still valid — is correct, but its liveness gate (isMainAccountTokenLive) treats a JWT whose exp cannot be decoded as live, so a genuinely dead credential would keep every bare 401 'transient' and needsReauth could never flip. This lands the transient-401 gate on a strict liveness check: isMainAccountTokenVerifiablyLive() returns true only for a decodable, still-future exp. An undecodable exp fails toward the terminal/reauth path. Terminal body codes (invalid_refresh_token, invalid_workspace_selected) remain terminal even when the token is live. Credit: lidge-jun#1932 (transient bare-401 concept and test scaffolding). Co-authored-by: PR lidge-jun#1932
fix(codex): treat a bare WHAM 401 as transient only for a verifiably live token (lands lidge-jun#1932)
WHAM is the ChatGPT usage/quota endpoint (GET https://chatgpt.com/backend-api/wham/usage) that ocx polls for plan, rate-limit windows, and reset credits. A bare 401 from WHAM does not prove the local credential died: upstream edges can transiently reject a still-valid access token. ocx treated every 401 as terminal, so the GUI's periodic refresh flipped a healthy main account to needs-reauth within a minute.
This PR only marks the main account needsReauth when the response carries a terminal code (invalid_workspace_selected / invalid_refresh_token) or the local access-token JWT has actually expired. Bare 401s with a live token are transient and leave the account usable.
Tests: updated and added auth-api tests for live-token 401 (transient), expired-token 401 (terminal), and the BUG-R327 flow.
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.