fix: update credential handling to prioritize primary service for token refresh#99
Conversation
|
@griffinmartin I see a lot of great changes made and I believe we can use some of the features we have here. Do you want me to update this? |
|
Hey @errhythm, yeah I'd love for you to update this! There's some really useful stuff here, especially the Bug 1 fix for suffixed keychain accounts and the email display in the account selector. A few things to keep in mind for the rebase:
The Bug 1 fix, configDir passthrough, keychainSuffixForDir, and the regex tightening are all good to go as-is. Looking forward to the update! |
446b7e3 to
a72a12a
Compare
|
@griffinmartin Please review if it is correct. |
…ting Update all config examples in README.md and installation.md to use `opencode-claude-auth@latest` so OpenCode always fetches the newest version on startup. Add a troubleshooting entry and "Updating the plugin" section for the "You're out of extra usage" / "Third-party apps" 400 error that affected users on stale cached versions. The fix: ensure @latest in config, clear ~/.cache/opencode/packages/opencode-claude-auth*, restart OpenCode, and re-authenticate if needed. Closes griffinmartin#145, closes griffinmartin#99 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Hi @griffinmartin, can you please review this if possible? The user account details feature at the time of switching account would be really helpful. If you can check this out it would be great. Let me know if any of the feature we do not need anymore, I can remove. |
|
@errhythm Then.. I re-checked out main, built, and it worked fine. |
8e15fc0 to
916f3cd
Compare
|
Thanks for checking. I have rebased it. So it should work. @griffinmartin |
|
Improved multi-account OAuth token refresh reliability for suffixed keychain accounts (Claude Code-credentials-). The core fix is in buildSuffixToDirCache: instead of scanning every ~/.* directory with a .claude.json and caching all of them, the scan now only maps directories whose SHA-256 hash matches a known keychain suffix — unrelated config dirs (API-key accounts, old sessions, etc.) are skipped entirely, and the scan exits early once all needed suffixes are resolved. I also tightened tryFallbackAccount to skip accounts whose in-memory expiresAt is already stale before attempting a live keychain read, avoiding unnecessary keychain hits when all fallback accounts are expired anyway. Requesting a review. 😅 |
|
@griffinmartin Can you test again after build please? Thanks. |
|
@errhythm I can look into it this weekend. Sorry for the delay! |
667eae2 to
8f1c204
Compare
|
Hi @griffinmartin, Sorry if I am bugging too much, did you get time to check? N.B.: It is basically hard for me to switch accounts 😭 |
Greptile SummaryThis PR fixes two independent credential-refresh bugs affecting macOS multi-account setups and all platforms respectively, and enriches the account-selection prompt with per-account email addresses.
Confidence Score: 5/5Safe to merge; both bug fixes are logically correct and covered by unit tests and a new headless E2E harness. The two core fixes are tightly scoped, all credential update paths have tests, and the headless harness accurately replicates the real CLI write-back behaviour. The only gap is a handful of removed parseCredentials negative-case tests that no longer have direct coverage. Files Needing Attention: src/keychain.test.ts — removed parseCredentials negative-case tests leave the field-validation branch without regression coverage.
|
| Filename | Overview |
|---|---|
| src/credentials.ts | Bug 1 fix (primary-entry fallback after CLI refresh) and Bug 2 fix (in-place credential update on ClaudeAccount) are correctly implemented. tryFallbackAccount properly updates borrowed account credentials in-place, and isSuffixedAccount gating prevents unnecessary CLI skips. |
| src/keychain.ts | Adds configDir propagation, keychainSuffixForDir hash, suffix-to-dir cache with hasAllNeeded incremental re-scan, email label enrichment, and readCredentialsFile export. |
| src/credentials.test.ts | Adds two focused tests for the tryFallbackAccount paths (externally-refreshed keychain and valid in-memory fallback), correctly mocking out OAuth and child-process to isolate the tested paths. |
| src/keychain.test.ts | Adds comprehensive integration tests for readAllClaudeAccounts with mocked security/child-process, keychainSuffixForDir, CLAUDE_CONFIG_DIR support, and label enrichment. Several negative-case parseCredentials tests were removed without replacement. |
| scripts/test-headless.ts | Well-structured headless E2E test covering happy-path, expired-primary-CLI-refresh, and Bug 1 suffixed-fallback scenarios. Safely backs up and restores account source state and guards against leaving junk keychain entries. |
| src/index.ts | Account selection prompt now shows 'active' hint instead of '(active)' suffix, and sourceDescription includes configDir and keychain service name for better context. |
| src/index.test.ts | Tests updated to match the new 'active' hint format. PRIMARY_SERVICE export is correctly stubbed in the fake keychain module. |
| package.json | Adds test:headless script for running the new headless end-to-end test. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[getCachedCredentials] --> B{Cache hit?}
B -- Yes --> Z[Return cached creds]
B -- No --> C[refreshIfNeeded]
C --> D{creds.expiresAt fresh?}
D -- Yes --> Z
D -- No --> E[refreshViaOAuth]
E --> F{OAuth succeeded?}
F -- Yes --> G[target.credentials = oauthCreds, writeBack, Return]
F -- No --> H{isSuffixedAccount?}
H -- Yes --> I[refreshViaCli requireConfigDir=true]
H -- No --> J[refreshViaCli requireConfigDir=false]
I --> K{configDir known?}
K -- No --> L[CLI skipped, cliSucceeded=false]
K -- Yes --> M[Run claude CLI with CLAUDE_CONFIG_DIR]
J --> M
M --> N{CLI succeeded?}
N -- No --> L
L --> O[tryFallbackAccount]
O --> P{Other account fresh?}
P -- Yes --> Q[target.credentials = fallback, Return]
P -- No --> R[refresh_exhausted, null]
N -- Yes --> S[refreshAccount target.source]
S --> T{Suffixed entry still stale?}
T -- Yes+isSuffixed --> U[refreshAccount PRIMARY_SERVICE - Bug1 fix]
U --> V{Primary fresh?}
V -- Yes --> W[refreshed = primaryCreds]
V -- No --> X[refresh_exhausted, null]
T -- No --> Y[refreshed = fresh]
W --> Y
Y --> AA[target.credentials = refreshed, Return - Bug2 fix]
Reviews (7): Last reviewed commit: "perf: borrow valid in-memory fallback cr..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…en refresh
This commit introduces a bug fix for the credential refresh logic, ensuring that if the active account is a suffixed entry and its credentials are stale, the system will fall back to the primary service ("Claude Code-credentials") to retrieve updated tokens. Additionally, the PRIMARY_SERVICE constant is now consistently exported across relevant files to maintain clarity and prevent duplication.
This commit updates the `refreshViaCli` function to accept an optional `configDir` parameter, allowing the CLI to read from and write back to the correct account directory. It also improves logging for credential refresh attempts and modifies the `buildAccountLabels` function to append email addresses when available. Additionally, new tests are added to ensure proper handling of the `CLAUDE_CONFIG_DIR` environment variable across different platforms.
… improvements This commit introduces a mock for the child process in the credential tests, allowing for better isolation during testing. Additionally, it improves the formatting of temporary directory creation and JSON file writing in the tests for better readability and consistency. The changes ensure that the tests remain robust while simulating the necessary environment for credential handling.
This commit modifies the keychain test file to replace instances of `process.platform` with a hardcoded value of `"darwin"`. This change ensures that the tests can run consistently in a controlled environment, improving test reliability and isolation.
This commit updates the `refreshViaCli` function to include a `requireConfigDir` parameter, improving error handling for suffixed accounts. It also introduces a new `tryFallbackAccount` function to attempt refreshing credentials from alternative accounts when the primary refresh fails. Additionally, the `discoverConfigDirsForKeychain` function is refactored to utilize a caching mechanism for improved performance.
…abel suffix, restore removed regression tests - buildSuffixToDirCache now extends the cache instead of freezing it after the first call, so accounts added while the process is running get their configDir resolved on the next readAllClaudeAccounts() call. - Non-macOS/no-keychain-account label building no longer passes the literal string "file" as a label suffix, avoiding labels like "Claude Pro: file" when no email is available. - Restored updateCredentialBlob root-level-format and mcpOAuth-preservation tests, and writeBackCredentials missing-file/invalid-JSON tests that were dropped during the credentials refactor.
8f1c204 to
2b6d503
Compare
…2 loop tryFallbackAccount's return value was never written back to target.credentials, so target kept its original stale expiresAt. After the 30s cache TTL, refreshIfNeeded would re-enter the full refresh sequence (including a fresh OAuth attempt) instead of reusing the still-valid borrowed token — the same infinite-refresh loop Bug 2 fixed on the OAuth/CLI success paths, just on the fallback branch.
|
@griffinmartin Done |
…eadless) Runs real opencode run invocations against the built plugin with a sandboxed fake keychain (PATH shims for security and claude) and asserts the refresh behaviour from an append-only shim log: - happy path: fresh credentials, no refresh - expired primary: OAuth fail -> CLI refresh -> re-read -> 200 - Bug 1: stale suffixed entry -> CLI writes to primary -> primary fallback -> 200 Assertions use the shim log rather than the plugin debug log because the plugin initialises more than once per opencode run and each init truncates the debug log, racily discarding early refresh events. macOS-only manual gate alongside validate:oauth and test:models; never runs in CI. The real keychain is read-only and the real refresh token is never transmitted.
- tryFallbackAccount now live-reads accounts whose in-memory expiry is stale instead of skipping them: another process (e.g. the Claude CLI) may have refreshed their keychain entry since startup. In-memory-fresh accounts are still tried first, keychain reads on one account no longer abort the remaining candidates, and the donor account's in-memory credentials are updated after a successful borrow. - Keychain service discovery matches any-length hex suffixes again so legacy entries stay visible; the suffix-to-config-dir mapping still only applies to the 8-char hashes the Claude CLI generates.
…read tryFallbackAccount now returns an account's in-memory credentials directly when they are still valid, instead of re-reading its keychain entry first. Live keychain reads only happen as a last resort when every candidate's in-memory expiry is stale. A 401 on a borrowed token is already handled by the reload-and-retry fetch path.
🤖 I have created a release *beep* *boop* --- ## [2.1.2](v2.1.1...v2.1.2) (2026-07-25) ### Bug Fixes * update credential handling to prioritize primary service for token refresh ([#99](#99)) ([9dd33d6](9dd33d6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2 participants
Two bugs were found by analyzing debug logs from users hitting the "Claude Code credentials are unavailable or expired" error.
Bug 1 — macOS / keychain (hard failure)
Users with multiple Claude Code accounts have a suffixed active account (e.g. Claude Code-credentials-b28bbb7c). When the token expires, the Claude CLI is invoked to refresh it and succeeds, but the CLI writes the new token back to the primary Keychain entry (Claude Code-credentials), not the suffixed one. refreshAccount(target.source) re-reads the suffixed entry, which still holds the old expiresAt, the post-refresh validity check fails, and refresh_exhausted → credentials_unavailable is emitted. The user sees:
Fix: if the suffixed account re-read is still stale after a CLI refresh, fall back to reading the primary entry. The fallback is gated on target.source.startsWith(PRIMARY_SERVICE + "-") so it only fires for suffixed accounts.
Bug 2 — all platforms (infinite refresh loop) (Potentially fixes #89)
After refreshIfNeeded returns fresh credentials, getCachedCredentials stores them in accountCacheMap with a 30-second TTL. When the cache expires, getActiveAccount() returns the same ClaudeAccount object from allAccounts — whose credentials.expiresAt was never updated. So refreshIfNeeded sees the original stale expiry, runs another full CLI refresh, and the cycle repeats every 30 seconds indefinitely. Visible in logs as refresh_needed firing with the same expiresAt value on every cache miss, hours after the token was successfully refreshed.
Fix: after a successful refresh, target.credentials is updated in-place on the ClaudeAccount object so subsequent cache misses see the correct expiresAt.
Also now
opencode auth loginwill show the user account details instead of just Claude Pro/Team/Max◆ Select which Claude Code account to use:
│ ● Claude Pro: john.doe@gmail.com (Claude Code-credentials)
│ ○ Claude Team: john@acme.com