Skip to content

Fall back to legacy keychain when stale scoped Claude credentials 401#8767

Merged
brennanb2025 merged 2 commits into
mainfrom
brennanb2025/claude-usage-stale-scoped-keychain
Jul 14, 2026
Merged

Fall back to legacy keychain when stale scoped Claude credentials 401#8767
brennanb2025 merged 2 commits into
mainfrom
brennanb2025/claude-usage-stale-scoped-keychain

Conversation

@brennanb2025

Copy link
Copy Markdown
Contributor

Problem

Claude Code maintains the legacy Claude Code-credentials Keychain item for the default config dir, but a scoped item (Claude Code-credentials-<sha256(configDir)[:8]>) can be left behind by sessions that ran with CLAUDE_CONFIG_DIR set explicitly. Once that scoped item's access token expires, nothing ever refreshes it — and readFromKeychain returned the scoped token unconditionally (expiresAt is deliberately not authoritative), so:

  1. Every usage fetch sent the dead scoped token → 401 → stale-token.
  2. System-default auth has no CLI recovery lane (by design — background refresh must not launch auth flows).
  3. Status bar showed "Refreshing sign-in" forever while the legacy item held a perfectly valid token (verified live: scoped token → HTTP 401, legacy token → HTTP 200, wedged for hours).

Fix

  • readFromKeychain: an actual access token from the legacy item now beats scoped refresh-only credentials — Orca cannot refresh tokens itself, so refresh-only must not shadow a working token.
  • On a stale-token OAuth failure with scoped-keychain credentials, retry once with the legacy item's token before classifying the failure. Gated to host system-default auth only — managed/WSL credentials never fall back to the host user's legacy keychain item.
  • The legacy retry runs before CLI repair, so a readable working token is preferred over launching a hidden 25s claude PTY.

Testing

  • New: scoped-401 → legacy retry succeeds (no PTY); legacy token preferred over scoped refresh-only; managed accounts never legacy-retry.
  • Updated the CLI-repair test to pin the legacy item absent so it still exercises the repair path.
  • vitest run src/main/rate-limits src/main/claude-accounts — 448 tests pass; typecheck:tsc:node clean.

Claude Code maintains the legacy 'Claude Code-credentials' Keychain item
for the default config dir, but a scoped item (service suffixed with
sha256(configDir)) can be left behind by sessions that ran with
CLAUDE_CONFIG_DIR set. Once its access token expires, nothing refreshes
it: readFromKeychain returned the scoped token unconditionally, every
usage fetch 401'd as 'stale-token', and system-default auth has no CLI
recovery lane — so the status bar showed 'Refreshing sign-in' forever
while the legacy item held a perfectly valid token.

Two changes:
- readFromKeychain: an actual access token from the legacy item now
  beats scoped refresh-only credentials (Orca cannot refresh tokens
  itself, so refresh-only must not shadow a working token).
- On a stale-token OAuth failure with scoped-keychain credentials, retry
  once with the legacy item's token before classifying the failure.
  Host system-default auth only — managed/WSL credentials never fall
  back to the host user's legacy keychain item.

The legacy retry runs before CLI repair, so a readable working token is
preferred over launching a hidden 25s claude PTY.
The legacy-keychain retry must never answer a WSL target with the host
user's keychain account, and must not double the usage request when the
legacy item mirrors the failed scoped token. Add tests locking both.
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Updated Claude rate-limit credential selection to prefer legacy access tokens when scoped tokens are unavailable. Added centralized OAuth success completion and conditional retries using legacy Keychain credentials after stale scoped-token failures. Added tests for fallback conditions, credential provenance, WSL behavior, mirrored tokens, disabled PTY fallback, and CLI repair sequencing.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the core change: falling back to the legacy keychain for stale scoped Claude credentials.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/main/rate-limits/claude-fetcher.ts (1)

692-720: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Both the source and test files grow further under pre-existing max-lines suppressions. As per coding guidelines, **/*.{js,jsx,ts,tsx} files should "Never disable max-lines via ESLint or oxlint, including line-specific variants," but this PR adds substantial new code to two files that already carry such a disable comment.

  • src/main/rate-limits/claude-fetcher.ts#L692-L720: extract completeOAuthUsageSuccess, canRetryWithLegacyKeychainToken, and retryOAuthWithLegacyKeychainToken (and their related helpers) into a dedicated legacy-retry module rather than growing this file further.
  • src/main/rate-limits/claude-fetcher.test.ts#L181-L343: move the new legacy-fallback test cases into a dedicated spec file alongside the extracted module.

Source: Coding guidelines


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: c45fee64-d4ba-47f1-aefc-21de7217677e

📥 Commits

Reviewing files that changed from the base of the PR and between 3453179 and e19d3ed.

📒 Files selected for processing (2)
  • src/main/rate-limits/claude-fetcher.test.ts
  • src/main/rate-limits/claude-fetcher.ts

@brennanb2025
brennanb2025 merged commit 792e113 into main Jul 14, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant