fix(windows): support CLAUDE_CODE_OAUTH_TOKEN env var and write auth.json to Roaming AppData#200
Conversation
…json to Roaming AppData Two Windows-specific bugs fixed: 1. When Claude Code is launched from Claude Desktop, it inherits the CLAUDE_CODE_OAUTH_TOKEN env var. With that var set, `claude auth login` considers the session already authenticated and never writes credentials to disk (~/.claude/.credentials.json). The plugin fell back to an empty account list, showing "credentials unavailable". Fix: readAllClaudeAccounts() now falls back to reading the env var directly (source: "env") when the credentials file is absent. The JWT expiry claim is decoded to populate expiresAt. writeBackCredentials and refreshAccount handle the "env" source as read-only. refreshIfNeeded skips the 60-second CLI fallback for env tokens, returning null immediately on expiry instead of hanging. 2. OpenCode stores its data directory in %APPDATA% (Roaming), but getAuthJsonPaths() only wrote auth.json to %LOCALAPPDATA% and the XDG path. OpenCode never saw the credentials the plugin synced. Fix: getAuthJsonPaths() now includes %APPDATA%\opencode\auth.json on Windows in addition to the existing paths.
bvironn
left a comment
There was a problem hiding this comment.
Read the diff — base on main: 213/213 tests pass, but the PR adds 0 new tests despite introducing three non-trivial code paths:
readEnvToken()(JWT decode + 10h fallback) —src/keychain.ts:181-217- Roaming AppData path in
getAuthJsonPaths()—src/credentials.ts:98-115 - Env-source short-circuits in
refreshIfNeeded(),writeBackCredentials(),refreshAccount()
Suggestions for tests:
readEnvToken()with valid JWT, malformed JWT (covers 10h fallback), and missing env vargetAuthJsonPaths()withprocess.platform === 'win32'mocked, asserting both Local and Roaming paths appearrefreshIfNeeded()returnsnullimmediately forsource: "env"when token is expired (avoiding the 60s CLI hang you mention in the PR body)
Windows-specific behavior is hard to verify in CI without coverage — easy to regress silently. Code itself looks correct; just needs the safety net.
…t-circuit Address review feedback from @bvironn on PR griffinmartin#200. Adds 14 tests for the three Windows-specific code paths the PR introduced; macOS/Linux CI can now exercise this behaviour via platform mocking and stubbed fixtures. Test additions: - src/keychain.test.ts: readEnvToken (via refreshAccount('env')) - decodes JWT exp claim into expiresAt - returns env value as accessToken with empty refreshToken - falls back to ~10h default for malformed JWTs (3 variants: not-base64, single-segment, missing-exp claim) - returns null when CLAUDE_CODE_OAUTH_TOKEN is unset - src/keychain.test.ts: writeBackCredentials('env') returns false (env credentials are read-only) - src/credentials.test.ts: getAuthJsonPaths - non-Windows returns only the XDG path - Windows returns XDG + Local + Roaming when env vars are set - Windows includes both Local and Roaming (regression guard for bug 2 in this PR) - Windows falls back to homedir-derived paths when env vars unset - src/credentials.test.ts: refreshIfNeeded(env source short-circuit) - returns null immediately when env token is expired - returns refreshed creds when env var rotated to a fresh token - returns null when env var is unset All three assert refreshAccount() is called exactly once, proving the env block does not fall through to the CLI fallback (which would call refreshAccount a second time). Production-code changes (additive, supporting the tests): - src/credentials.ts: export getAuthJsonPaths so it is unit-testable. No behaviour change. - src/keychain.ts: log JWT decode failures inside readEnvToken's catch block instead of swallowing silently. Matches the diagnostic style used elsewhere in this file (readKeychainEntry) and follows through on the spirit of the PR ("never silent").
|
Pushed 7eaa628 to address @bvironn's review (no new tests). Added 14 tests covering all three areas you flagged. Tests now go from 213 → 227, all passing on a non-Windows runner via platform mocking.
Small production-code nit also included ( Verified: |
… sync Address README gap noted during PR review: PR griffinmartin#200 introduces a new consumed env var and changes the Windows auth.json sync target list, neither of which was reflected in README.md. - 'How it works': update Windows auth.json path list to include %APPDATA% (Roaming) alongside %LOCALAPPDATA%, and note that OpenCode itself reads from the Roaming location. - 'How it works': add a paragraph explaining the CLAUDE_CODE_OAUTH_TOKEN fallback on Windows + Claude Desktop. - 'Prerequisites': mention the env-var fallback path. - 'Environment variable overrides' table: add a row for CLAUDE_CODE_OAUTH_TOKEN, marked read-only and documented as set automatically by Claude Desktop (not intended for manual use). - 'How it works (technical)': replace the obsolete two-path Windows bullet with the new three-path version, and add a bullet describing the env-var fallback's read-only nature, JWT exp decoding, and the no-CLI-fallback short-circuit on expiry.
|
Pushed aefe0a8 to update the README. Three things were stale or missing:
|
Resolve conflicts and apply two follow-up fixes from the regression analysis on Windows code paths. Conflicts resolved: - README.md: env-var override table — both branches added a row. Kept the new OPENCODE_CLAUDE_AUTH_MAX_RETRY_MS row from main and re-added the CLAUDE_CODE_OAUTH_TOKEN row from this PR after it, rewidened to the wider column shape main introduced. - src/keychain.ts: auto-merged. PR griffinmartin#201 (in main) edited the catch block of listClaudeKeychainServices to capture the error; this PR added readEnvToken further down in the file. Both changes preserved cleanly. Follow-up fixes for issues found in the Windows regression analysis: 1. src/index.ts (authorize): the sourceDescription used a 2-arm conditional that bucketed every non-'file' source as 'macOS Keychain'. With this PR's new 'env' source, a Windows user authenticated via Claude Desktop's CLAUDE_CODE_OAUTH_TOKEN would see 'credentials loaded from macOS Keychain' which is wrong on every axis. Added an 'env' arm that says 'CLAUDE_CODE_OAUTH_TOKEN environment variable'. 2. src/credentials.ts (syncAuthJson): the sync loop re-threw on the first per-path failure, aborting subsequent paths and tearing down the unhandled callers (plugin init at index.ts, account switch authorize). With this PR adding a third Windows path, the failure surface is 50% larger; an antivirus-locked or network-share-borked Roaming auth.json could now break plugin startup. Each path is independent — failures are now logged per-path and the loop continues. 3. src/credentials.test.ts: regression test for fix griffinmartin#2. Pre-creates a directory at the XDG target path so writeFileSync fails with EISDIR, asserts syncAuthJson does not throw.
|
@jstruv-bot @bvironn — pushed 930cf37: merged latest Merge conflict resolution
Follow-up fixes from regression analysis
Verification: PR is back to non-conflicting state and ready for re-review. |
Greptile SummaryThis PR fixes two Windows-specific bugs that prevented the plugin from working when OpenCode is launched from Claude Desktop: credentials were never found because
Confidence Score: 4/5Safe to merge; the new env-token path is well-isolated and tested, and the two concrete Windows bugs are correctly addressed. The two main code changes (env fallback and Roaming AppData path) are correct and well-tested. The one behavioural change worth watching is in syncAuthJson: removing throw err was necessary for the Windows multi-path case but also makes macOS/Linux sync failures silent — a persistent filesystem error on those platforms will now only appear in debug logs. There is also a mildly misleading comment in refreshIfNeeded about the parent process rotating the env var, which could confuse future maintainers. Neither concern affects runtime correctness for the target scenario. Files Needing Attention: src/credentials.ts — the syncAuthJson error-swallowing change and the env-rotation comment both live here.
|
| Filename | Overview |
|---|---|
| src/keychain.ts | Adds readEnvToken() to decode CLAUDE_CODE_OAUTH_TOKEN as a fallback credential source; extends readAllClaudeAccounts(), writeBackCredentials(), and refreshAccount() to handle the new "env" source cleanly. |
| src/credentials.ts | Exports getAuthJsonPaths() and adds the Roaming %APPDATA% path for Windows; adds env source short-circuit in refreshIfNeeded(); removes the throw in syncAuthJson so a single bad path no longer aborts subsequent writes. |
| src/index.ts | Minor UI copy update: adds an "env" branch to the sourceDescription ternary so the auth confirmation message is accurate for the new env credential source. |
| src/keychain.test.ts | Adds comprehensive tests for readEnvToken (via refreshAccount("env")): JWT exp decoding, malformed/missing exp fallback, unset var, and writeBackCredentials no-op for "env" source. |
| src/credentials.test.ts | Adds tests for getAuthJsonPaths() (XDG, Local+Roaming Windows, fallback without env vars), syncAuthJson error-handling (EISDIR resilience), and the refreshIfNeeded env short-circuit with a counting stub keychain. |
| README.md | Documentation updated to cover the Roaming AppData path, the env var fallback behaviour, and a new CLAUDE_CODE_OAUTH_TOKEN row in the env-vars reference table. |
Sequence Diagram
sequenceDiagram
participant CD as Claude Desktop
participant P as Plugin
participant F as Credentials File
participant E as CLAUDE_CODE_OAUTH_TOKEN env var
participant R as refreshIfNeeded
CD->>CD: "Set CLAUDE_CODE_OAUTH_TOKEN in child env"
CD->>P: "Spawn OpenCode process"
P->>F: "readCredentialsFile()"
F-->>P: "null — CLI never wrote it"
P->>E: "readEnvToken()"
E-->>P: "{ accessToken, refreshToken:'', expiresAt }"
P-->>P: "return [{ source:'env', credentials }]"
Note over R: "On token near-expiry..."
R->>E: "refreshAccount('env') → readEnvToken()"
alt "env token still valid"
E-->>R: "fresh credentials"
else "env token expired"
E-->>R: "expiresAt in past → return null, skip CLI"
end
Note over P: "Windows auth.json sync"
P->>P: "syncToPath(XDG path)"
P->>P: "syncToPath(%LOCALAPPDATA%)"
P->>P: "syncToPath(%APPDATA% Roaming)"
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
src/credentials.ts:308-312
The comment overstates what re-reading the env var can accomplish. Standard OS process semantics don't allow a parent process to update the environment of an already-running child process, so "Claude Desktop refreshed and updated its child environment" won't happen through normal means. The re-read is still worth doing (it's cheap and handles any in-process mutation of `process.env`), but the stated rationale will mislead future maintainers.
```suggestion
// Env-var credentials have no refresh token and no CLI path.
// Re-read the env var in case process.env was mutated in-process since the
// last readAllClaudeAccounts() call. If the token has genuinely expired,
// return null immediately rather than blocking for up to 60 s on a CLI
// timeout that cannot help (the CLI would also see CLAUDE_CODE_OAUTH_TOKEN
// and skip writing credentials to disk).
```
### Issue 2 of 2
src/credentials.ts:149-166
**Silent failure now applies to all platforms**
Removing `throw err` was the right call for the Windows multi-path case (a failure on the first path would otherwise abort subsequent writes), but the change also affects macOS and Linux where there is only one target path (XDG). Previously, a persistent filesystem error on those platforms surfaced to the caller; now it is silently swallowed and only visible in the debug log. If the XDG path is unwritable on Linux (e.g. a permission regression from a botched install), the plugin will appear to work while auth.json never gets updated, which can be hard to diagnose without debug logging enabled. Consider emitting a warning-level log entry or a console message when `syncToPath` fails on non-Windows so the silent-failure behaviour remains intentional and observable.
Reviews (1): Last reviewed commit: "Merge origin/main into fix/windows-env-t..." | Re-trigger Greptile
| // Env-var credentials have no refresh token and no CLI path. | ||
| // Re-read the env var in case it was rotated by the parent process (e.g. | ||
| // Claude Desktop refreshed and updated its child environment). If it has | ||
| // genuinely expired, return null immediately rather than blocking for up to | ||
| // 60 s on a CLI timeout that cannot help. |
There was a problem hiding this comment.
The comment overstates what re-reading the env var can accomplish. Standard OS process semantics don't allow a parent process to update the environment of an already-running child process, so "Claude Desktop refreshed and updated its child environment" won't happen through normal means. The re-read is still worth doing (it's cheap and handles any in-process mutation of
process.env), but the stated rationale will mislead future maintainers.
| // Env-var credentials have no refresh token and no CLI path. | |
| // Re-read the env var in case it was rotated by the parent process (e.g. | |
| // Claude Desktop refreshed and updated its child environment). If it has | |
| // genuinely expired, return null immediately rather than blocking for up to | |
| // 60 s on a CLI timeout that cannot help. | |
| // Env-var credentials have no refresh token and no CLI path. | |
| // Re-read the env var in case process.env was mutated in-process since the | |
| // last readAllClaudeAccounts() call. If the token has genuinely expired, | |
| // return null immediately rather than blocking for up to 60 s on a CLI | |
| // timeout that cannot help (the CLI would also see CLAUDE_CODE_OAUTH_TOKEN | |
| // and skip writing credentials to disk). |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/credentials.ts
Line: 308-312
Comment:
The comment overstates what re-reading the env var can accomplish. Standard OS process semantics don't allow a parent process to update the environment of an already-running child process, so "Claude Desktop refreshed and updated its child environment" won't happen through normal means. The re-read is still worth doing (it's cheap and handles any in-process mutation of `process.env`), but the stated rationale will mislead future maintainers.
```suggestion
// Env-var credentials have no refresh token and no CLI path.
// Re-read the env var in case process.env was mutated in-process since the
// last readAllClaudeAccounts() call. If the token has genuinely expired,
// return null immediately rather than blocking for up to 60 s on a CLI
// timeout that cannot help (the CLI would also see CLAUDE_CODE_OAUTH_TOKEN
// and skip writing credentials to disk).
```
How can I resolve this? If you propose a fix, please make it concise.| @@ -148,7 +162,6 @@ export function syncAuthJson(creds: ClaudeCredentials): void { | |||
| success: false, | |||
| error: err instanceof Error ? err.message : String(err), | |||
| }) | |||
| throw err | |||
| } | |||
| } | |||
There was a problem hiding this comment.
Silent failure now applies to all platforms
Removing throw err was the right call for the Windows multi-path case (a failure on the first path would otherwise abort subsequent writes), but the change also affects macOS and Linux where there is only one target path (XDG). Previously, a persistent filesystem error on those platforms surfaced to the caller; now it is silently swallowed and only visible in the debug log. If the XDG path is unwritable on Linux (e.g. a permission regression from a botched install), the plugin will appear to work while auth.json never gets updated, which can be hard to diagnose without debug logging enabled. Consider emitting a warning-level log entry or a console message when syncToPath fails on non-Windows so the silent-failure behaviour remains intentional and observable.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/credentials.ts
Line: 149-166
Comment:
**Silent failure now applies to all platforms**
Removing `throw err` was the right call for the Windows multi-path case (a failure on the first path would otherwise abort subsequent writes), but the change also affects macOS and Linux where there is only one target path (XDG). Previously, a persistent filesystem error on those platforms surfaced to the caller; now it is silently swallowed and only visible in the debug log. If the XDG path is unwritable on Linux (e.g. a permission regression from a botched install), the plugin will appear to work while auth.json never gets updated, which can be hard to diagnose without debug logging enabled. Consider emitting a warning-level log entry or a console message when `syncToPath` fails on non-Windows so the silent-failure behaviour remains intentional and observable.
How can I resolve this? If you propose a fix, please make it concise.
Problem
Two Windows-specific bugs that prevent the plugin from working when Claude Code is launched from Claude Desktop:
Bug 1 — Credentials never found when
CLAUDE_CODE_OAUTH_TOKENis setClaude Desktop sets
CLAUDE_CODE_OAUTH_TOKENin the environment of every process it spawns (including terminal windows). When this env var is present,claude auth loginconsiders the session already authenticated and exits without writing~/.claude/.credentials.json. The plugin'sreadAllClaudeAccounts()finds no credentials file and returns an empty list, causing OpenCode to show:Bug 2 — Plugin syncs
auth.jsonto the wrong Windows pathgetAuthJsonPaths()wroteauth.jsonto%LOCALAPPDATA%\opencode\auth.jsonand the XDG path, but OpenCode stores its data directory in%APPDATA%(Roaming). OpenCode never saw the credentials the plugin synced.Fix
src/keychain.tsreadEnvToken(): readsCLAUDE_CODE_OAUTH_TOKEN, decodes the JWTexpclaim forexpiresAt(defaults to 10 h if decode fails), returns credentials with emptyrefreshTokenreadAllClaudeAccounts(): falls back toreadEnvToken()(source:"env") when the credentials file is absent on non-macOSwriteBackCredentials("env"): returnsfalseimmediately (read-only source)refreshAccount("env"): re-reads the env varsrc/credentials.tsgetAuthJsonPaths(): adds%APPDATA%\opencode\auth.json(Roaming) to the Windows sync targetsrefreshIfNeeded(): for"env"source, skips the 60-second CLI fallback and returnsnullimmediately if the token has expired, avoiding a long hang