[starnetdev0]

ELI5

Running Orca's tests from inside a Claude Code session can log you out of Claude Code. The session tells its child processes where your Claude account lives through CLAUDE_CONFIG_DIR; the test runner inherits it, Orca's code honours it, and the claude-accounts tests — which believe they are working in a temporary folder — delete and rewrite your real credentials. This PR removes that variable from the test environment.

What Changed

• config/scripts/vitest-claude-env-isolation.ts (new, first in setupFiles): deletes CLAUDE_CONFIG_DIR before any test module loads. Tests that need the variable already set it themselves.
• tests/e2e/helpers/electron-home-isolation.ts: adds CLAUDE_CONFIG_DIR to the restricted keys, next to CODEX_HOME, so the e2e Electron app — and the agents it spawns — never inherit the host account either.
• Tests: a guard that fails if any test can see the variable, and the e2e isolation test now covers CLAUDE_CONFIG_DIR both inherited and passed through launchEnv.

Why

Claude Code sessions (and multi-account launchers) export CLAUDE_CONFIG_DIR=<account dir>. vitest inherits the parent environment, and src/main/claude-accounts/runtime-auth-service-*.test.ts resolve the Claude config dir from it. Run that way, the tests:

• delete .credentials.json in the real account directory,
• rewrite it with test tokens,
• remove oauthAccount from .claude.json,

after which every Claude Code session on that account fails its next request with Not logged in · Please run /login. The only trace in the test output is 76 failures reading Unexpected Claude config dir: <real path>, which looks like an environment quirk rather than data loss — which is how it went unnoticed while it logged a developer out about twenty times over two days.

A setup file is the smallest change that protects every test at once, including ones added later; fixing individual tests would leave the next claude-accounts test exposed. The e2e launcher already strips CODEX_HOME for the same reason.

Linked Issue

No separate issue — the problem, reproduction and evidence are in this PR.

Visual Proof

N/A — test-environment change only; no UI or runtime behaviour changes.

Testing

Reproduction with a decoy config dir, so no real credentials are at risk:

DECOY=$(mktemp -d)
printf '{"claudeAiOauth":{"accessToken":"decoy","refreshToken":"decoy","expiresAt":4102444800000}}\n' > "$DECOY/.credentials.json"
printf '{"oauthAccount":{"emailAddress":"decoy@example.invalid"}}\n' > "$DECOY/.claude.json"
CLAUDE_CONFIG_DIR="$DECOY" pnpm exec vitest run --config config/vitest.config.ts src/main/claude-accounts
ls -la "$DECOY"

• On current main (f2d5711), before this change: the decoy's .credentials.json is deleted, .claude.json is rewritten without oauthAccount, and 76 tests fail in 7 files with Unexpected Claude config dir.
• With this change: the decoy is untouched (same hashes and mtimes), and all 212 tests in src/main/claude-accounts, the new guard and the e2e isolation test pass.

Platforms: tested on Linux. The change is platform-neutral — it deletes an environment variable, and Node's process.env is case-insensitive on Windows, so it holds there too.

• I manually tested these changes locally
• Automated tests added/updated, or explained why not below

AI Disclosure

Written with Claude Code (Claude Opus 5). The failure was found while investigating repeated Claude Code logouts on a developer's machine; the fix and the decoy verification above were run locally.

Review

Agent skill upstream boundary

• Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Security: stops the test suite from destroying a developer's real credentials. Cross-platform: platform-neutral. SSH/remote, mobile, backwards compatibility and performance: unaffected — test environment only.

Checklist

• This PR is small and focused
• I explained what changed and why (including ELI5)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred) — ran the affected tests and oxlint on the changed files; CI will cover the rest.

🤖 Generated with Claude Code

https://claude.ai/code/session_01L4p7iWdvZPkMh7TsuVkEi6

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: c85b155c-a8c8-4a6f-a93f-2fe710bef750

📥 Commits

Reviewing files that changed from the base of the PR and between f2d5711 and 3164a72.

📒 Files selected for processing (5)

• config/scripts/vitest-claude-env-isolation.test.ts
• config/scripts/vitest-claude-env-isolation.ts
• config/vitest.config.ts
• tests/e2e/helpers/electron-home-isolation.ts
• tests/e2e/helpers/electron-home-isolation.unit.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The Vitest setup removes CLAUDE_CONFIG_DIR before tests run and verifies its absence. Electron home isolation now strips the variable from inherited environments and rejects attempts to restore it through environment overlays. Unit tests cover both behaviors.

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 3164a

The change prevents tests from reaching a developer’s real Claude credentials while preserving explicit test isolation paths. The covered Vitest and Electron behaviors show no remaining merge-blocking risk.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely identifies the primary change: preventing tests from deleting developer Claude credentials through CLAUDE_CONFIG_DIR.
Description check	✅ Passed	The description is detailed and covers the change, reason, testing, platform impact, AI disclosure, and checklist. The Linked Issue section states that no separate issue exists, although the template …
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 5…
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.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[pullfrog]

✅ No new issues found.

Reviewed changes

This PR stops the test suite from writing through a host Claude Code session's CLAUDE_CONFIG_DIR by deleting the variable before any test module loads, and closes the same hole in the e2e Electron launcher. The fix is minimal, correctly scoped, and well justified.

• Vitest setup isolation — new config/scripts/vitest-claude-env-isolation.ts deletes process.env.CLAUDE_CONFIG_DIR, registered first in setupFiles so it runs before any test module resolves it.
• Regression guard — config/scripts/vitest-claude-env-isolation.test.ts fails if any test can observe the host variable.
• E2E isolation — CLAUDE_CONFIG_DIR joins RESTRICTED_ENV_KEYS, so the launcher strips it from inheritedEnv and rejects it in launchEnv/extraEnv, with unit coverage for both the inherited and passed-through cases.

I verified the mechanism end to end: ClaudeRuntimePathResolver.getRuntimePaths() reads process.env.CLAUDE_CONFIG_DIR instead of the mocked homedir(), so the runtime-auth-service-*.test.ts files (which mock homedir() to a temp dir) fall back to the temp fakeHomeDir/.claude once the variable is gone. Tests that genuinely need the variable set it themselves via explicit env fixtures, and the only config that runs these tests is the fixed one (the performance config inherits its setupFiles and scopes include elsewhere). No production code is touched.

  ^{｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}

[nwparker]

Thank you for the clear write-up of how CLAUDE_CONFIG_DIR could cause tests to wipe real Claude credentials. This is now covered by the write guard in #23979 (shipped in v1.4.219), which handles CLAUDE_CONFIG_DIR and fails any unit test that writes under a real agent home. Closing as superseded, and we appreciate the report.