Skip to content

feat(claude): support multiple config dirs#227

Open
ozymandiashh wants to merge 3 commits intogetagentseal:mainfrom
ozymandiashh:feat/multiple-claude-dirs
Open

feat(claude): support multiple config dirs#227
ozymandiashh wants to merge 3 commits intogetagentseal:mainfrom
ozymandiashh:feat/multiple-claude-dirs

Conversation

@ozymandiashh
Copy link
Copy Markdown
Contributor

Summary

  • add CLAUDE_CONFIG_DIRS support for tracking multiple Claude Code config directories
  • keep existing CLAUDE_CONFIG_DIR and default ~/.claude behavior as fallback
  • dedupe normalized config/project paths and keep Claude Desktop discovery intact
  • document the new env var and add an Unreleased changelog entry

Closes #208.

Validation

  • npx tsc --noEmit
  • npx vitest run tests/providers/claude.test.ts tests/security/prototype-pollution.test.ts tests/provider-registry.test.ts
  • npx vitest run
  • npm run build

@ozymandiashh ozymandiashh marked this pull request as ready for review May 5, 2026 10:17
@iamtoruk
Copy link
Copy Markdown
Member

iamtoruk commented May 6, 2026

Reviewed end-to-end: pulled the branch, type-check + full vitest suite (483 tests pass), then ran 8 adversarial probes against createClaudeProvider. Code is clean and the factory refactor is a nice testability win. Notes below.

Probe results

Case Behavior Verdict
Same dir listed twice Deduped via resolve()
Trailing/double delimiters (:work::personal:) Empty entries skipped
Whitespace-only env value Falls back to CLAUDE_CONFIG_DIR
Empty string env value Falls back to CLAUDE_CONFIG_DIR
Mix valid + nonexistent dirs Nonexistent silently skipped (readdir throws → caught)
Same project name across dirs Merged into one ProjectSummary by the parser ⚠ see #1
Relative paths (./work) Resolved against CWD
Tilde paths (~/.claude-foo) Not expanded — stays literal, silent failure ⚠ see #2

Issues to address before merge

1. Aggregation vs attribution mismatch with the linked issue

The issue asks for tracking multiple accounts. The PR delivers aggregation across multiple dirs. parser.ts:652-661 merges ProjectSummary entries by p.project name — so a project that exists in both ~/.claude-work and ~/.claude-personal collapses into one dashboard row with combined totals.

That may or may not be what @ccrvlh wants. I posted the question on #208 — pausing the merge until they clarify. If aggregation is fine, we ship as-is plus a note. If they need per-account splits, the data model needs a configDir / account label on SessionSource now rather than as a follow-up.

2. Tilde ~/ not expanded — silent failure

resolve() does not expand ~. CLAUDE_CONFIG_DIRS=~/.claude:~/.claude-work resolves to <cwd>/~/.claude (etc.), neither valid, falls through to default. No warning. Two options:

  • Expand: if (trimmed.startsWith('~/')) trimmed = join(homedir(), trimmed.slice(2)) in normalizeDirs.
  • Document: explicitly note in README that tilde isn't supported, require absolute or $HOME-substituted paths.

I'd vote for expansion. Five-line change, no failure mode.

3. Test gap on the merge behavior

The two new tests don't cover the same-project-name-across-dirs case (which is the most likely real-world scenario for a multi-account user). If a future refactor unintentionally splits them by path, no test fails. Worth adding before merge.

Smaller things (non-blocking)

  • Symlinks not resolved. resolve() not realpath() — two symlinks to the same real dir get scanned twice. Per-message dedup keeps correctness; just wasted work.
  • Sequential discovery. for (const claudeDir of …) instead of Promise.all(map(...)). Fine at typical 1–3 dirs.
  • Doc nit. README says "Overrides CLAUDE_CONFIG_DIR when set" — strictly, only when set and containing ≥1 valid entry after trim/dedupe.
  • No CLI flag. Only env var. Probably fine.

Security review

No new attack surface. Env var is user-controlled but its only effect is choosing dirs they read from. readdir/stat only — no execution, no traversal risk into anything sensitive.

Verdict

Hold for #208 reply, then either:

  • Merge as-is + tilde patch + merge-behavior test (if @ccrvlh confirms aggregation is fine).
  • Hold longer and rework the data model for per-account attribution (if they need splits).

I have a worktree set up locally and can apply the tilde fix + the missing test in <10 minutes once we know which path.

@ozymandiashh
Copy link
Copy Markdown
Contributor Author

ozymandiashh commented May 6, 2026

Addressed the hold items from the earlier review:

  • Same project across multiple Claude config dirs now stays split by account via account/accountPath attribution in discovery, parsing, dashboard identity, filters, JSON, and CSV export.
  • Tilde ~/ expansion is supported for CLAUDE_CONFIG_DIR and CLAUDE_CONFIG_DIRS.
  • Added regression coverage for same-project separate accounts, single-entry CLAUDE_CONFIG_DIRS labeling, tilde expansion, duplicate label suffixing, project filters, CSV Account column, and JSON output.
  • Merged latest origin/main and preserved the new canonical cwd grouping per account.

Local verification before push:

  • npm test: passed, 40 files / 543 tests
  • npm run build: passed
  • git diff --check: clean
  • Gemini 3.1 Pro Preview review: PASS
  • Claude Opus 4.7 effort max Argus-style review: PASS CU OBSERVATII, no blockers

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.

feat: allow multiple account tracking

2 participants