feat: respect XDG_DATA_HOME and CLAUDE_CONFIG_DIR for path resolution#217
feat: respect XDG_DATA_HOME and CLAUDE_CONFIG_DIR for path resolution#217troian wants to merge 1 commit into
Conversation
|
hey @griffinmartin any chance you can take a look |
|
@troian Sorry I've been pretty busy so haven't been testing all the open PRs as much as I have in the past. I'll get to it this week. |
|
I have a similar issue. Running three Claude Team accounts across different project dirs via CLAUDE_CONFIG_DIR + direnv, and the account selection bleeds across all of them because claude-account-source.txt is always written to the hardcoded path. Ended up pre-writing it from a shell wrapper before each opencode launch as a workaround, but that breaks every update. |
|
@markthepixel do you mind trying this PR? |
|
I made a chance to keep the XDG_DATA_HOME and CLAUDE_CONFIG_DIR behavior as-is, but adds OPENCODE_ACCOUNT_SOURCE_FILE as an explicit override for just the persisted account selection file. That covers the case where someone wants one shared OpenCode db and session history, but still wants Claude account selection pinned per workspace, which is my issue |
5c134cf to
bed47d0
Compare
|
@griffinmartin bump for review |
bed47d0 to
a96ddad
Compare
The plugin hardcoded ~/.local/share/opencode/ and ~/.claude/ for all file paths, ignoring XDG_DATA_HOME and CLAUDE_CONFIG_DIR. This prevented running multiple OpenCode instances in parallel with different Claude accounts, since all instances shared the same claude-account-source.txt. Changes: - credentials.ts: getAccountStateFile() and getAuthJsonPaths() use $XDG_DATA_HOME with fallback to ~/.local/share - keychain.ts: readCredentialsFile() and writeBackCredentials() use $CLAUDE_CONFIG_DIR with fallback to ~/.claude - logger.ts: getDefaultLogPath() uses $XDG_DATA_HOME with fallback to ~/.local/share - Tests added for all new paths; existing tests updated to isolate env vars that now affect path resolution - README updated with env var docs and parallel instance usage When unset, behavior is unchanged (same defaults as before). Signed-off-by: Artur Troian <troian@users.noreply.github.com>
a96ddad to
5e10fea
Compare
Greptile SummaryThis PR adds
Confidence Score: 4/5Safe to merge for Linux and macOS users; the Windows parallel-instance path has a pre-existing dual-write behaviour that persists after this change. The core env-var logic is a minimal, correct three-line change repeated consistently across credentials.ts, keychain.ts, and logger.ts, with solid test coverage for the happy paths. The Windows dual-write in getAuthJsonPaths() means setting XDG_DATA_HOME for instance isolation still results in a shared LOCALAPPDATA write, so the documented parallel-instance workflow doesn't fully deliver on Windows. Files Needing Attention: src/credentials.ts — the getAuthJsonPaths() Windows branch; README.md — the parallel-instance example assumes full isolation which doesn't hold on Windows.
|
| Filename | Overview |
|---|---|
| src/credentials.ts | Adds XDG_DATA_HOME support to getAccountStateFile() and getAuthJsonPaths(); on Windows getAuthJsonPaths() still returns both the XDG path and the LOCALAPPDATA path, so auth.json is still written to the shared AppData location even when XDG_DATA_HOME is set for isolation. |
| src/keychain.ts | Adds CLAUDE_CONFIG_DIR support to readCredentialsFile() and writeBackCredentials(), both using the same env-var-or-default pattern correctly. |
| src/logger.ts | getDefaultLogPath() now respects XDG_DATA_HOME with the same fallback pattern; straightforward and correct. |
| src/credentials.test.ts | Adds XDG_DATA_HOME tests for saveAccountSource, loadPersistedAccountSource, and syncAuthJson; existing permission tests updated to isolate XDG_DATA_HOME. Missing a direct test for readCredentialsFile() with CLAUDE_CONFIG_DIR. |
| src/keychain.test.ts | Adds CLAUDE_CONFIG_DIR tests for writeBackCredentials; existing writeBackCredentials tests updated to explicitly unset CLAUDE_CONFIG_DIR for isolation. Well-structured and thorough. |
| src/logger.test.ts | Adds XDG_DATA_HOME test for default log path; existing tests correctly save/restore the env var. No issues. |
| src/index.test.ts | Updated several tests to isolate XDG_DATA_HOME (save/restore pattern); no new logic introduced, existing tests unaffected. |
| README.md | Documents the new XDG_DATA_HOME and CLAUDE_CONFIG_DIR env vars with a parallel instance example; the claim that auth.json goes to 'its own data directory' does not hold on Windows where the LOCALAPPDATA path is also written unconditionally. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Plugin startup] --> B{XDG_DATA_HOME set?}
B -- Yes --> C[dataHome = XDG_DATA_HOME]
B -- No --> D[dataHome = ~/.local/share]
C --> E[account-source.txt\ndataHome/opencode/claude-account-source.txt]
D --> E
C --> F[auth.json\ndataHome/opencode/auth.json]
D --> F
F --> G{Windows?}
G -- Yes --> H[Also write to LOCALAPPDATA/opencode/auth.json\n shared across instances]
G -- No --> I[Single XDG path only]
J[Credential read/write] --> K{CLAUDE_CONFIG_DIR set?}
K -- Yes --> L[claudeDir = CLAUDE_CONFIG_DIR]
K -- No --> M[claudeDir = ~/.claude]
L --> N[.credentials.json]
M --> N
O[Debug log] --> P{XDG_DATA_HOME set?}
P -- Yes --> Q[XDG_DATA_HOME/opencode/claude-auth-debug.log]
P -- No --> R[~/.local/share/opencode/claude-auth-debug.log]
Comments Outside Diff (1)
-
src/credentials.ts, line 97-108 (link)Windows isolation gap when
XDG_DATA_HOMEis setgetAuthJsonPaths()always returns two paths on Windows — the XDG path and theLOCALAPPDATApath. When a user setsXDG_DATA_HOMEspecifically to isolate parallel instances,auth.jsonis still written to the sharedLOCALAPPDATA/opencode/auth.jsonas a second write target. Both instances therefore clobber the sameLOCALAPPDATAcopy on every credential refresh, so whichever instance ran last "wins" there. If OpenCode ever falls back to theLOCALAPPDATApath, the wrong instance's credentials would be surfaced.The README's parallel-instance documentation ("each instance writes its
auth.jsonto its own data directory, avoiding conflicts") does not hold on Windows. Consider guarding the second write withif (!process.env.XDG_DATA_HOME), or at least adding a Windows caveat to the docs.Prompt To Fix With AI
This is a comment left during a code review. Path: src/credentials.ts Line: 97-108 Comment: **Windows isolation gap when `XDG_DATA_HOME` is set** `getAuthJsonPaths()` always returns *two* paths on Windows — the XDG path and the `LOCALAPPDATA` path. When a user sets `XDG_DATA_HOME` specifically to isolate parallel instances, `auth.json` is still written to the shared `LOCALAPPDATA/opencode/auth.json` as a second write target. Both instances therefore clobber the same `LOCALAPPDATA` copy on every credential refresh, so whichever instance ran last "wins" there. If OpenCode ever falls back to the `LOCALAPPDATA` path, the wrong instance's credentials would be surfaced. The README's parallel-instance documentation ("each instance writes its `auth.json` to its own data directory, avoiding conflicts") does not hold on Windows. Consider guarding the second write with `if (!process.env.XDG_DATA_HOME)`, or at least adding a Windows caveat to the docs. How can I resolve this? If you propose a fix, please make it concise.
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:97-108
**Windows isolation gap when `XDG_DATA_HOME` is set**
`getAuthJsonPaths()` always returns *two* paths on Windows — the XDG path and the `LOCALAPPDATA` path. When a user sets `XDG_DATA_HOME` specifically to isolate parallel instances, `auth.json` is still written to the shared `LOCALAPPDATA/opencode/auth.json` as a second write target. Both instances therefore clobber the same `LOCALAPPDATA` copy on every credential refresh, so whichever instance ran last "wins" there. If OpenCode ever falls back to the `LOCALAPPDATA` path, the wrong instance's credentials would be surfaced.
The README's parallel-instance documentation ("each instance writes its `auth.json` to its own data directory, avoiding conflicts") does not hold on Windows. Consider guarding the second write with `if (!process.env.XDG_DATA_HOME)`, or at least adding a Windows caveat to the docs.
### Issue 2 of 2
src/credentials.test.ts:547-599
**`readCredentialsFile()` with `CLAUDE_CONFIG_DIR` is untested**
The new `CLAUDE_CONFIG_DIR` behaviour in `keychain.ts` is exercised by tests for `writeBackCredentials`, but not for `readCredentialsFile()`. That function is the first place credentials are loaded (called from `readAllClaudeAccounts()` and `refreshAccount("file")`), so a misconfiguration there would silently serve stale credentials. A parallel-instance scenario where `CLAUDE_CONFIG_DIR` points to a per-instance directory would rely on `readCredentialsFile()` picking it up correctly, yet this is the path with no coverage. A test that sets `CLAUDE_CONFIG_DIR` and then calls `readAllClaudeAccounts()` on a non-macOS platform (or `refreshAccount("file")`) would close the gap.
Reviews (1): Last reviewed commit: "feat: respect XDG_DATA_HOME and CLAUDE_C..." | Re-trigger Greptile
| describe("XDG_DATA_HOME support", () => { | ||
| it("saveAccountSource writes to $XDG_DATA_HOME/opencode/ when set", async () => { | ||
| const originalXdg = process.env.XDG_DATA_HOME | ||
| const tempDir = await mkdtemp(join(tmpdir(), "opencode-claude-auth-xdg-")) | ||
| process.env.XDG_DATA_HOME = tempDir | ||
|
|
||
| try { | ||
| const mod = await import( | ||
| pathToFileURL(new URL("./credentials.ts", import.meta.url).pathname) | ||
| .href + `?xdg-save-${Date.now()}` | ||
| ) | ||
| mod.saveAccountSource("Claude Code-credentials-abc123") | ||
|
|
||
| const expected = join(tempDir, "opencode", "claude-account-source.txt") | ||
| const content = readFileSync(expected, "utf-8").trim() | ||
| assert.equal(content, "Claude Code-credentials-abc123") | ||
| } finally { | ||
| if (typeof originalXdg === "string") { | ||
| process.env.XDG_DATA_HOME = originalXdg | ||
| } else { | ||
| delete process.env.XDG_DATA_HOME | ||
| } | ||
| } | ||
| }) | ||
|
|
||
| it("loadPersistedAccountSource reads from $XDG_DATA_HOME/opencode/ when set", async () => { | ||
| const originalXdg = process.env.XDG_DATA_HOME | ||
| const tempDir = await mkdtemp(join(tmpdir(), "opencode-claude-auth-xdg-")) | ||
| process.env.XDG_DATA_HOME = tempDir | ||
|
|
||
| try { | ||
| const stateDir = join(tempDir, "opencode") | ||
| mkdirSync(stateDir, { recursive: true }) | ||
| writeFileSync( | ||
| join(stateDir, "claude-account-source.txt"), | ||
| "Claude Code-credentials-def456", | ||
| "utf-8", | ||
| ) | ||
|
|
||
| const mod = await import( | ||
| pathToFileURL(new URL("./credentials.ts", import.meta.url).pathname) | ||
| .href + `?xdg-load-${Date.now()}` | ||
| ) | ||
| const result = mod.loadPersistedAccountSource() | ||
| assert.equal(result, "Claude Code-credentials-def456") | ||
| } finally { | ||
| if (typeof originalXdg === "string") { | ||
| process.env.XDG_DATA_HOME = originalXdg | ||
| } else { | ||
| delete process.env.XDG_DATA_HOME | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
readCredentialsFile() with CLAUDE_CONFIG_DIR is untested
The new CLAUDE_CONFIG_DIR behaviour in keychain.ts is exercised by tests for writeBackCredentials, but not for readCredentialsFile(). That function is the first place credentials are loaded (called from readAllClaudeAccounts() and refreshAccount("file")), so a misconfiguration there would silently serve stale credentials. A parallel-instance scenario where CLAUDE_CONFIG_DIR points to a per-instance directory would rely on readCredentialsFile() picking it up correctly, yet this is the path with no coverage. A test that sets CLAUDE_CONFIG_DIR and then calls readAllClaudeAccounts() on a non-macOS platform (or refreshAccount("file")) would close the gap.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/credentials.test.ts
Line: 547-599
Comment:
**`readCredentialsFile()` with `CLAUDE_CONFIG_DIR` is untested**
The new `CLAUDE_CONFIG_DIR` behaviour in `keychain.ts` is exercised by tests for `writeBackCredentials`, but not for `readCredentialsFile()`. That function is the first place credentials are loaded (called from `readAllClaudeAccounts()` and `refreshAccount("file")`), so a misconfiguration there would silently serve stale credentials. A parallel-instance scenario where `CLAUDE_CONFIG_DIR` points to a per-instance directory would rely on `readCredentialsFile()` picking it up correctly, yet this is the path with no coverage. A test that sets `CLAUDE_CONFIG_DIR` and then calls `readAllClaudeAccounts()` on a non-macOS platform (or `refreshAccount("file")`) would close the gap.
How can I resolve this? If you propose a fix, please make it concise.
The plugin hardcoded ~/.local/share/opencode/ and ~/.claude/ for all file paths, ignoring XDG_DATA_HOME and CLAUDE_CONFIG_DIR. This prevented running multiple OpenCode instances in parallel with different Claude accounts, since all instances shared the same claude-account-source.txt. Changes:
When unset, behavior is unchanged (same defaults as before).
Summary
Related issue
Testing
Checklist
feat:,fix:,docs:,chore:, etc.)make allpasses locally (runs lint, build, and test)