Skip to content

fix(codex): treat a revoked main session as needing sign-in and keep its last plan - #6496

Closed
vadymhimself wants to merge 1 commit into
lidge-jun:devfrom
vadymhimself:fix/codex-token-invalidated-probe
Closed

vadymhimself wants to merge 1 commit into
lidge-jun:devfrom
vadymhimself:fix/codex-token-invalidated-probe

Conversation

@vadymhimself

@vadymhimself vadymhimself commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

When ChatGPT changes an account's plan (seen live on a Pro to Free downgrade), it revokes every session of that account. Usage reads then answer:

HTTP 401 {"error":{"message":"Your authentication token has been invalidated. Please try signing in again.","type":"invalid_request_error","code":"token_invalidated"}}

The access token's exp is still in the future. The main-account probe only treats a 401 from a verifiably live token as terminal when the body code is in MAIN_TERMINAL_AUTH_CODES, and token_invalidated was not in it. So the probe treated a dead session as a transient blip. The account list then showed needsReauth: false, plan: null and quota: null, and nothing told the operator to sign in again.

  • token_invalidated joins the terminal auth codes. The list moves to CODEX_TERMINAL_AUTH_CODES in src/codex/quota-refresh-outcome.ts, and MAIN_TERMINAL_AUTH_CODES is derived from it.
  • The quotaRefresh diagnostic gains an optional code, taken only from that fixed vocabulary, so a caller can say why. The CLI projection copies it the same way.
  • A main row that this call's 401 quarantined reports reauthReason: "unauthorized" instead of refresh_failed.
  • A failed usage read keeps the last-known plan instead of nulling it. After a downgrade, the plan the account had is what explains why it stopped working.

A bare 401 from a live token is still transient (#1932). Only the named code changes.

Verification

  • bun run typecheck: clean.
  • New tests/codex-integration/codex-main-token-invalidated.test.ts: 1/1 pass. It fails with src/ reverted to dev.
  • codex-auth-api 345/345, main-account-hard-lock-recovery 68/68, cli-account 153/153. Three exact-shape assertions in the first two files gain the new code field, which is the intended diagnostic change.
  • tests/codex, tests/cli and tests/codex-integration run as a batch against dev: no failure names added.
  • bun scripts/file-size-ratchet.ts, bun run structure:check, bun run privacy:scan and tests/test-layout.test.ts: all pass.
  • Running live on a fork build against the revoked account: the row now reads needsReauth: true, plan: "pro", quotaRefresh: {status: "http_error", httpStatus: 401, code: "token_invalidated"}.

Not run: the full bun run test.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed (structure/dashboard-and-usage.md, reference/configuration/server.md).
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults: code is a fixed enum, and no provider body text is forwarded.

🤖 Generated with Claude Code

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • New Features

    • Sign-in-related quota errors can now show recognized provider error codes, helping explain why reauthentication is needed.
    • When a main account’s session is revoked after a plan change, the account retains its last-known plan while indicating that sign-in is required.
  • Documentation

    • Clarified which error codes may appear in quota diagnostics and how failed quota reads affect the displayed plan.

…its last plan

ChatGPT revokes every session of an account whose plan changes (for example
Pro to Free) and answers usage reads with 401 `token_invalidated`, while the
access token's `exp` is still in the future. The main-account probe treated
that 401 as transient, so the account kept `needsReauth: false`, its plan and
quota went to null, and nothing told the operator to sign in again.

- `token_invalidated` joins the terminal auth codes (one list, now shared
  with the CLI projection).
- The quota-refresh diagnostic carries the terminal provider code
  (`quotaRefresh.code`), a fixed vocabulary, so a caller can say why.
- A main row quarantined by this call's 401 reports `reauthReason:
  "unauthorized"` instead of `refresh_failed`.
- A failed usage read keeps the last-known plan instead of nulling it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 78f025ae-e968-4b99-a7a6-60d81d5c963f
📥 Commits

Reviewing files that changed from the base of the PR and between 4b98328 and 8b64585.

📒 Files selected for processing (10)
  • docs-site/src/content/docs/reference/configuration/server.md
  • scripts/test-layout/layout.json
  • src/codex/auth-api/account-list.ts
  • src/codex/auth-api/main-account-probe.ts
  • src/codex/auth-api/pool-quota-probe.ts
  • src/codex/quota-refresh-outcome.ts
  • structure/dashboard-and-usage.md
  • tests/codex-integration/codex-auth-api.test.ts
  • tests/codex-integration/codex-main-token-invalidated.test.ts
  • tests/codex-integration/main-account-hard-lock-recovery.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The change adds allowlisted terminal-auth codes to quota-refresh errors. Main-account handling now preserves the last-known plan after a failed quota read and reports HTTP 401 reauthentication as unauthorized. Tests and documentation cover these outcomes.

Changes

Main account terminal-auth handling

Layer / File(s) Summary
Terminal-auth quota code contract
src/codex/quota-refresh-outcome.ts, src/codex/auth-api/pool-quota-probe.ts
HTTP-error outcomes can include a code from the shared terminal-auth allowlist. The pool probe uses that allowlist.
Main-account response handling
src/codex/auth-api/main-account-probe.ts, src/codex/auth-api/account-list.ts
The probe classifies terminal responses for HTTP 401 and 403, includes recognized codes in HTTP-error outcomes, and retains the cached or current plan. HTTP 401 quota errors now use the unauthorized reauthentication reason.
Integration tests and documentation
tests/codex-integration/codex-main-token-invalidated.test.ts, tests/codex-integration/codex-auth-api.test.ts, tests/codex-integration/main-account-hard-lock-recovery.test.ts, scripts/test-layout/layout.json, docs-site/src/content/docs/reference/configuration/server.md, structure/dashboard-and-usage.md
Integration tests cover token invalidation and terminal response codes. Documentation describes the recognized codes and retained plan behavior.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Suggested reviewers: lidge-jun

Merge Risk: ⚪ Minimal · up to 8b645

This change marks a revoked main session as needing sign-in and keeps the last-known plan after a failed usage read. No merge-blocking issues were found. The author did not run the full test suite, so run it in CI before merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 8b645

The change restricts reported authentication codes to a fixed vocabulary and preserves credential-identity checks before changing account state. No material security regression was established. Complete deployed access controls and behavior across process restarts were not verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected failure-state effects are bounded to the current main-account identity or the rejected pool credential generation. Recognizing the additional code can quarantine affected accounts earlier; it does not introduce a credential-issuance or privilege-granting path. Wider deployed route exposure was not established.

Trust Boundaries and Controls

  • observed — Provider response data crosses into diagnostics through bounded response parsing and fixed-code membership checks. The CLI treats the management response as untrusted and applies the same allowlist projection, preventing arbitrary provider messages or extra fields from being copied into this diagnostic contract.

Resilience and Maintainability Implications

  • observed — Recognized pool terminal codes short-circuit refresh recovery and mark the rejected generation. Bare 401s retain the existing refresh-and-replay recovery path. Main-account success clears quarantine only during an explicit refresh, not ordinary background polling.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 7 files. (3 skipped: 3… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: require sign-in after a revoked main session and retain the last-known plan.
Full details: Docstring Coverage

Explanation

Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 7 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 3, 2026
@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ✅ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request has been marked Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun @Ingwannu

Hygiene

✅ Deterministic PR hygiene checks passed.

@vadymhimself

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions
github-actions Bot marked this pull request as ready for review October 3, 2026 09:41
@lidge-jun

Copy link
Copy Markdown
Owner

Superseded by #6515, merged into dev as e601cefcebcc20b2e04a04821ab45df6538d98f9. The recognized revoked-session classification, retained last-known plan and allowlisted diagnostics were carried with source-author credit. The replacement also fixes stale-generation and pre-existing-reauth cause attribution, adds the missing test registrations, and preserves transient bare-401 behavior. No implementation part was dropped.

Final-head required scoped CI passed: https://github.com/lidge-jun/opencodex/actions/runs/37132811309. The intermediate failed diagnostic-count assertion and subsequent cause-attribution regression were corrected with their intended contracts documented. This is delivery to dev; live revoked-account and final integrated release evidence are not implied. Thank you for the original fix.

@lidge-jun lidge-jun closed this Oct 3, 2026
@lidge-jun lidge-jun mentioned this pull request Oct 4, 2026
3 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working review-ready superseded

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants