Skip to content

fix: treat a blank Claude credential as an expired login - #459

Merged
xiufengsun merged 2 commits into
xiufengsun:mainfrom
tcvieira:fix/claude-blank-credentials
Aug 13, 2026
Merged

fix: treat a blank Claude credential as an expired login#459
xiufengsun merged 2 commits into
xiufengsun:mainfrom
tcvieira:fix/claude-blank-credentials

Conversation

@tcvieira

@tcvieira tcvieira commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Why

When a Claude Code login expires, the credential entry is not removed — the secrets are blanked in place. The macOS Keychain item Claude Code-credentials keeps claudeAiOauth with accessToken: "", refreshToken: "", expiresAt: 0, and all of the metadata (scopes, subscriptionType, rateLimitTier) intact.

readClaudeCodeAccessToken() normalizes that empty string to null, so getUsageLimits() reports { configured: false } and the dashboard hides the Claude section completely — the same rendering as a machine that never installed Claude Code. Meanwhile the usage bars keep updating (parsed from local logs, no auth needed), so the failure reads as "TokenTracker doesn't support my plan" instead of "your CLI login expired". Nothing is logged, and claude-usage-limits-cache.json is never written.

#330 already solved the equivalent case on the wire: a 401 sets auth_action_required: "reauth", which the panel renders as a "Re-auth" badge with a run \claude`` tooltip. The blank-credential case never gets there, because there is no token to send.

Fixes #458

What

  • Tell "never signed in" from "signed in and expired" in the !claudeToken branch by reusing detectClaudeCodeCredentialsPresence(), which already exists and is existence-only (it reads no secret — on macOS it probes the Keychain without fetching the password).
  • Route the second case into the existing reauth path: configured: true + auth_action_required: "reauth" + the same message the 401 path uses, now shared as CLAUDE_AUTH_EXPIRED_MESSAGE.
  • A missing credential entry still reports configured: false, so machines without Claude Code are unaffected.

No new user-facing strings — limits.reauth.badge and limits.reauth.tooltip already cover the rendering, and REAUTH_CLI_COMMANDS already maps claude.

Tests

Two cases added to test/usage-limits.test.js:

  • a credential entry whose token fields are blank → configured: true, auth_action_required: "reauth", and the usage API is never called (the fake fetchImpl throws if it is);
  • no credential entry at all → configured: false, no reauth flag.

The first fails on main (configured: false, no flag) and passes with this change. Full suite: npm test.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Claude authentication status detection when credentials are present but incomplete or missing an access token.
    • Clearly indicates when reauthentication is required instead of incorrectly reporting Claude as unconfigured.
    • Prevents unnecessary usage API requests when credentials are invalid or incomplete.
    • Standardized the authentication-expired message shown for Claude authorization failures.
    • Improved handling of blank credential fields and missing credential entries for more accurate authentication status reporting.

Claude Code blanks accessToken/refreshToken in place when its login expires
instead of removing the credential entry, so a null token is ambiguous: never
signed in, or signed in and expired. Reporting configured:false for both hides
the Claude section entirely while usage bars keep updating, which reads as an
unsupported plan rather than an expired login.

Reuse detectClaudeCodeCredentialsPresence() (existence-only, reads no secret)
to tell the two apart and route the expired case into the reauth path added in
xiufengsun#330, sharing the 401 handler's message. No new user-facing strings.
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a144770e-3677-47c6-ac45-cc0459223844

📥 Commits

Reviewing files that changed from the base of the PR and between df4120b and 1439817.

📒 Files selected for processing (1)
  • test/usage-limits.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/usage-limits.test.js

📝 Walkthrough

Walkthrough

The change exposes Claude credential-presence detection and uses it to classify blank-token credentials as configured but requiring reauthentication. Missing credentials remain unconfigured. Tests cover both states.

Changes

Claude authentication state

Layer / File(s) Summary
Credential presence detection
src/lib/subscriptions.js, src/lib/usage-limits.js
The Claude credential-presence detector is exported and imported for usage-limit evaluation.
Expired credential result handling
src/lib/usage-limits.js, test/usage-limits.test.js
Claude uses a shared authentication-expiration message. Existing credentials without tokens return configured status with reauthentication required. Missing credentials remain unconfigured. Tests cover both cases.

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

Mergeability Score: ⚪ Minimal · up to 14398

The change is localized to expired Claude credential handling and its tests; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: xiufengsun

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary fix for blank Claude credentials.
Linked Issues check ✅ Passed The changes distinguish blank expired credentials from missing credentials and return the required reauthentication state without calling the usage API.
Out of Scope Changes check ✅ Passed All changes support issue #458 by implementing credential detection, reauthentication handling, and focused tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/usage-limits.test.js`:
- Around line 1715-1735: Update the test around getUsageLimits to track calls to
the Anthropic usage URL instead of relying solely on fetchImpl throwing, then
assert that the tracked call count is zero after the result assertions. Preserve
the existing rejection/error assertions while explicitly verifying the Claude
usage API was never invoked.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 79d275a5-5257-4ce4-8c7e-cda106e4cfb7

📥 Commits

Reviewing files that changed from the base of the PR and between e0f97c1 and df4120b.

📒 Files selected for processing (3)
  • src/lib/subscriptions.js
  • src/lib/usage-limits.js
  • test/usage-limits.test.js

Comment thread test/usage-limits.test.js
Throwing inside fetchImpl was not an assertion: provider failures are gathered
with allSettled and the blank-credential branch never reads claudeResult, so a
forbidden call would be swallowed and the test would still pass.

@xiufengsun xiufengsun left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MERGE — reviewed exact head 1439817. The blank-vs-missing Claude credential states are correctly distinguished, focused tests pass, the combined integration with PR #452 is clean, and all GitHub checks are green.

@xiufengsun
xiufengsun merged commit f32b4af into xiufengsun:main Aug 13, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Claude disappears from LIMITS when the CLI login expires — a blank accessToken is treated as "not configured"

2 participants