Skip to content

fix(install): respect $CLAUDE_CONFIG_DIR in install/uninstall/update#321

Closed
halindrome wants to merge 1 commit into
DeusData:mainfrom
halindrome:fix/claude-config-dir-respect
Closed

fix(install): respect $CLAUDE_CONFIG_DIR in install/uninstall/update#321
halindrome wants to merge 1 commit into
DeusData:mainfrom
halindrome:fix/claude-config-dir-respect

Conversation

@halindrome

Copy link
Copy Markdown
Contributor

Summary

codebase-memory-mcp install (and therefore update's Step 6 refresh) hardcoded $HOME/.claude/... for skills, MCP entries, hook scripts, .claude.json, and settings.json. When users had CLAUDE_CONFIG_DIR set to a non-default location (e.g. ~/.config/claude-code, where Claude Code's own config lives), the installer silently created a separate ~/.claude/ tree that the agent never saw. Hooks installed at runtime already honored CLAUDE_CONFIG_DIR; only the install/uninstall paths were mismatched.

This PR fixes that. It also bundles a migration nudge so the same release that fixes the bug helps users with stale ~/.claude/ trees clean them up — explained under "Bundling rationale" below.

Closes #320.

What changes

Core helpers in src/cli/cli.c:

  • cbm_claude_config_dir(home, out, sz) — returns $CLAUDE_CONFIG_DIR or $home/.claude.
  • cbm_claude_user_root(home, out, sz) — returns $CLAUDE_CONFIG_DIR or $home (parent dir for .claude.json, which moves to $CLAUDE_CONFIG_DIR/.claude.json when the env var is set, matching Claude Code's own behavior).
  • cbm_resolve_hook_command(script_name, out, sz) — produces the hook command string written into settings.json. When CLAUDE_CONFIG_DIR is unset, it preserves the legacy ~/.claude/hooks/<script> tilde form so existing settings.json files stay portable across HOME values.

Call sites routed through the helpers:

  • install_claude_code_config — skills, .mcp.json, .claude.json, settings.json, hook script install
  • uninstall_claude_code — same set, mirrored
  • cbm_install_hook_gate_script, cbm_install_session_reminder_script — hook script destination
  • cbm_detect_agentsagents.claude_code now true when either ~/.claude or $CLAUDE_CONFIG_DIR exists

The CMM_HOOK_COMMAND / CMM_SESSION_COMMAND macros (which baked ~/.claude/hooks/... into settings.json) are replaced by CMM_HOOK_GATE_SCRIPT / CMM_SESSION_REMINDER_SCRIPT (basenames), with the full path resolved at install time via cbm_resolve_hook_command. Removal still works because is_cmm_hook_entry matches by matcher_str, not by command path.

Shell layer:

  • scripts/setup.shconfigure_claude now uses ${CLAUDE_CONFIG_DIR:-$HOME/.claude} for settings_file and the printed instructions.
  • scripts/security-install.shSKILLS_DIR likewise.

Bundling rationale

This PR rolls out the fix and the migration nudge together rather than splitting them across two releases:

  • If we shipped only the fix, users who upgrade with CLAUDE_CONFIG_DIR set would see their config silently migrate to the env-pointed dir. Their old ~/.claude/{skills,hooks,settings.json,.mcp.json} would still be on disk — orphaned, never written again, easy to forget.
  • The migration nudge is a single stderr line printed by install when (a) CLAUDE_CONFIG_DIR is set, and (b) a legacy ~/.claude directory still exists. It names both paths so users can choose to clean up.
  • Shipping them together means the user sees the corrective behavior and the cleanup hint in the same release. Splitting them would force a two-release rollout where the first release leaves stale state on disk with no in-product signal, and the second release shows up later when many users would have already noticed and worked around it.

The nudge is non-fatal, prints once per install invocation, and is silent in the legacy-default case (CLAUDE_CONFIG_DIR unset → no divergence to nudge about).

Tests

  • tests/test_cli.c: new cli_detect_agents_finds_claude_via_env asserts that cbm_detect_agents reports claude_code=true when CLAUDE_CONFIG_DIR points at an existing dir outside home_dir/.claude. cli_detect_agents_finds_claude and cli_detect_agents_none_found now unset/restore CLAUDE_CONFIG_DIR so the runner's real env does not leak in. All 2813 unit tests pass.
  • scripts/smoke-test.sh Phase 9c: end-to-end install/uninstall under CLAUDE_CONFIG_DIR=$FAKE_HOME/custom-claude with a pre-existing $FAKE_HOME/.claude. Asserts:
    • skills/, settings.json, .mcp.json, .claude.json, hooks/cbm-code-discovery-gate all land under $CLAUDE_CONFIG_DIR.
    • None of those land under $HOME/.claude (regression guard).
    • Hook command in settings.json is the absolute $CLAUDE_CONFIG_DIR/hooks/... path, not ~/.claude/hooks/....
    • Migration nudge fires when both dirs are present.
    • Uninstall removes from $CLAUDE_CONFIG_DIR and leaves legacy $HOME/.claude untouched.
    • With CLAUDE_CONFIG_DIR unset, settings.json keeps the ~/.claude/hooks/... tilde form (legacy default preserved).
  • Local manual verification: install + uninstall cycle with both env-set and env-unset configs, confirmed file placement and migration nudge surface match expectations.

Out of scope (intentionally separate)

  • Project-local install (--scope=project): a real feature gap, but a separate change. Not bundled.
  • The wider EXPECTED_PATTERNS allow-list in scripts/security-install.sh is unchanged; .config/... already matches typical CLAUDE_CONFIG_DIR setups, and broadening it without a concrete test is not worth the security audit risk.

Test plan

  • scripts/build.sh
  • scripts/test.sh (2813 passed)
  • Manual install/uninstall with CLAUDE_CONFIG_DIR set
  • Manual install/uninstall with CLAUDE_CONFIG_DIR unset (legacy default)
  • CI: _lint.yml (clang-format-20)
  • CI: _build.yml
  • CI: smoke-test (full Phase 1-15 including new Phase 9c)

🤖 Generated with Claude Code

Previously, `codebase-memory-mcp install` and `codebase-memory-mcp update`
unconditionally wrote skills, MCP entries, hook scripts, and settings.json
under $HOME/.claude/, even when CLAUDE_CONFIG_DIR pointed at a different
location. Users with non-default Claude Code config dirs ended up with a
new (silently created) ~/.claude/ tree while their real config dir stayed
untouched, so the agent could not see the artifacts the installer claimed
to write.

This commit:

- Adds cbm_claude_config_dir(), cbm_claude_user_root(), and
  cbm_resolve_hook_command() helpers in src/cli/cli.c that honor
  $CLAUDE_CONFIG_DIR with $HOME/.claude as the legacy fallback.
- Routes install_claude_code_config(), uninstall_claude_code(),
  cbm_install_hook_gate_script(), cbm_install_session_reminder_script(),
  and cbm_detect_agents()'s claude_code probe through those helpers.
- Replaces the CMM_HOOK_COMMAND / CMM_SESSION_COMMAND macro literals
  ("~/.claude/hooks/...") with runtime-resolved hook command strings
  written into settings.json. The legacy tilde form is preserved when
  CLAUDE_CONFIG_DIR is unset so existing settings.json files stay
  portable across HOME values.
- Mirrors the change in scripts/setup.sh (settings_file path) and
  scripts/security-install.sh (SKILLS_DIR path) so the shell-side
  helpers also honor CLAUDE_CONFIG_DIR.

Bundled migration safeguard:

- When install runs with CLAUDE_CONFIG_DIR set and a legacy ~/.claude
  directory still exists, install emits a one-line stderr nudge naming
  both paths so users can clean up stale skills/hooks/settings without
  needing a follow-up release.

Tests:

- Adds cli_detect_agents_finds_claude_via_env to verify CLAUDE_CONFIG_DIR
  detection. Updates cli_detect_agents_finds_claude and
  cli_detect_agents_none_found to unset/restore CLAUDE_CONFIG_DIR so
  the runner's real env does not leak in.
- Adds Phase 9c to scripts/smoke-test.sh covering: skills/.mcp.json/
  .claude.json/settings.json/hooks all landing under $CLAUDE_CONFIG_DIR
  (not $HOME/.claude), settings.json hook command path, migration
  nudge surface, uninstall removal, legacy ~/.claude untouched, and
  legacy default preserving the tilde form when CLAUDE_CONFIG_DIR is
  unset.

Closes DeusData#320

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@halindrome

Copy link
Copy Markdown
Contributor Author

QA Round 1 — PR #321 (fix/claude-config-dir-respect)

Reviewer: QA agent
Branch: halindrome:fix/claude-config-dir-respectDeusData:main
Diff scope: src/cli/cli.c, tests/test_cli.c, scripts/{setup,security-install,smoke-test}.sh (+311/-25)

Per-check verdict

# What Status Evidence / Notes
1 Helper trio correct (buffer size handling, NULL guards, env precedence, tilde preserved only when env unset) PASS All three helpers (cbm_claude_config_dir, cbm_claude_user_root, cbm_resolve_hook_command) early-return on out_sz==0, zero-init out[0]='\0', branch on cbm_safe_getenv (non-empty wins) then fall back to home_dir. cbm_resolve_hook_command emits ~/.claude/hooks/<name> only when env unset.
2 Every ~/.claude/... write site routed through helpers PASS Remaining literals in src/cli/cli.c: lines 973/984/988/1017 (helper bodies), 1692 (comment), 2715/2736/3048 (paths derived from home-based legacy_dir/mcp_path2/comment — all correct uses). cli.h lines 67/113/168 are doc comments. No missed callsite.
3 .claude.json placement: $CCD/.claude.json when env set, else $HOME/.claude.json PASS cbm_claude_user_root returns env-or-home; mcp_path2 is <user_root>/.claude.json in both install_claude_code_config (line 2715) and uninstall_claude_code (line 3048). Manual T1 confirms .claude.json written under $CCD, no stray $HOME/.claude.json.
4 Hook command in settings.json: tilde when env unset, absolute when env set PASS Manual T1: PreToolUse: $CCD/hooks/cbm-code-discovery-gate; SessionStart: $CCD/hooks/cbm-session-reminder. Manual T2 (env unset): PreToolUse: ~/.claude/hooks/cbm-code-discovery-gate; SessionStart: ~/.claude/hooks/cbm-session-reminder.
5 Removal symmetry — old ~/.claude/... entries replaced by env-set absolute path PASS is_cmm_hook_entry matches by matcher_str only (line 1525). Manual T5: installed with env unset (1 PreToolUse entry, tilde cmd) → re-installed with CCD set → settings.json still has 1 PreToolUse entry, command rewritten to absolute path. SessionStart: still exactly 4 entries, all rewritten.
6 Migration nudge condition (CCD set AND legacy exists AND differs); no false positives PASS T1 (CCD set, legacy present, differs): nudge fires. T2 (CCD unset): silent. T3 (CCD set, no legacy dir): silent. T4 (CCD == $HOME/.claude): silent (the strcmp(legacy_dir, config_dir) != 0 guard handles the identical-path case).
7 Test coverage matches PR claims PASS New cli_detect_agents_finds_claude_via_env exercises env-set path with no home/.claude. Existing cli_detect_agents_finds_claude and cli_detect_agents_none_found now save/restore CLAUDE_CONFIG_DIR so the runner's real env can't leak. Phase 9c covers 10 placement/nudge/uninstall/symmetry assertions.
8 Shell layer minimal & correct PASS setup.sh swaps the literal for ${CLAUDE_CONFIG_DIR:-$HOME/.claude} plus updates the printed instruction text. security-install.sh swaps SKILLS_DIR. Both use the standard :- fallback idiom. No accidental side effects.
9 Style — conventional commits, code style consistent, no debug prints, no leftover TODO PASS All commits follow fix({scope}) pattern. Helpers follow the file's static-helper / snprintf / CLI_BUF_1K conventions. No printf debug prints. No // TODO.
10 Backwards compatibility — pre-existing settings.json with ~/.claude/hooks/... upserts cleanly PASS T5 confirmed matcher-based dedup: only one PreToolUse entry survives the env-flip upgrade, with the command rewritten to the new absolute form.
11 Full test suite PASS scripts/test.sh: 2813 passed, 0 failed.

Findings

Must-fix: None.

Nice-to-have (advisory only):

  1. Doc comment drift. src/cli/cli.c:1692 and src/cli/cli.h:{67,113,168} still describe paths as ~/.claude/... even though install location is now CCD-aware. Comments are not load-bearing for behavior, but a one-line update to mention "or $CLAUDE_CONFIG_DIR" would prevent future grep confusion.
  2. Migration nudge wording. The current nudge prints the absolute legacy path (e.g. legacy /tmp/x/.claude still exists). If the user invoked install from a non-interactive context the absolute path is fine; for interactive use, ~/.claude reads more cleanly. Strictly cosmetic.
  3. config_dir[0]==0 guard divergence. cbm_install_hook_gate_script and cbm_install_session_reminder_script early-return when config_dir[0] is empty, but install_claude_code_config itself does not check config_dir[0] before deriving skills_dir/mcp_path/settings_path. In practice home==NULL is already filtered upstream, so this is unreachable, but adding a symmetric if (!config_dir[0]) return; would be defensive.
  4. cli.h cbm_detected_agents_t.claude_code comment still says /* ~/.claude/ exists */; could read /* ~/.claude or $CLAUDE_CONFIG_DIR exists */ to match the new detection semantics.

No security or correctness regression detected.

Final verdict

PASS-WITH-ADVISORY — the fix is correct, well-tested, and backwards-compatible. The four advisory items are documentation-comment polish, not behavior bugs. Recommend marking the PR ready for review.

@halindrome
halindrome marked this pull request as ready for review May 6, 2026 18:07
@DeusData DeusData added bug Something isn't working ux/behavior Display bugs, docs, adoption UX labels May 8, 2026
DeusData pushed a commit that referenced this pull request May 30, 2026
Route Claude Code config paths (skills, .mcp.json, .claude.json,
settings.json, hook scripts) and agent detection through
CLAUDE_CONFIG_DIR-aware helpers, falling back to ~/.claude. Hook command
strings written to settings.json keep the legacy tilde form when the env
var is unset, so existing configs stay portable across HOME values. Prints
a one-line migration nudge when CLAUDE_CONFIG_DIR is set and a legacy
~/.claude tree still exists.

Adds cli_detect_agents_finds_claude_via_env and isolates CLAUDE_CONFIG_DIR
in the existing detection tests so the runner env can't leak in.

Distilled from #321 onto current main (adapts to the v0.7.0 non-blocking
augmenter hook signature, which the original branch predated). Closes #320.
@DeusData

Copy link
Copy Markdown
Owner

Thank you, @halindrome — this is genuinely excellent work. The three-helper design (cbm_claude_config_dir / cbm_claude_user_root / cbm_resolve_hook_command), preserving the legacy ~/.claude/hooks/... tilde form when CLAUDE_CONFIG_DIR is unset so existing settings.json files stay portable, and the migration nudge bundled with the fix — all of it was carefully thought through. The smoke-test Phase 9c with its regression guards (asserting nothing leaks into $HOME/.claude) was especially appreciated.

Since this branch predated the v0.7.0 hook rework (#288/#362 — the PreToolUse gate became a non-blocking augmenter, which changed CMM_HOOK_MATCHER and the cbm_install_hook_gate_script signature), it no longer applied cleanly to main. Rather than ask you to rebase, I distilled your change onto current main, preserving your design and crediting you as the commit author:

Landed in dedd33d (fix(install): respect $CLAUDE_CONFIG_DIR in install/uninstall/update), which closes #320. All 3,616 tests pass, including your cli_detect_agents_finds_claude_via_env test.

Closing this PR as the work is now on main. Thank you again for the contribution and the thoroughness — it made the distill trivial. 🙏

@DeusData DeusData closed this May 30, 2026
zaruous pushed a commit to zaruous/codebase-memory-mcp that referenced this pull request May 31, 2026
Route Claude Code config paths (skills, .mcp.json, .claude.json,
settings.json, hook scripts) and agent detection through
CLAUDE_CONFIG_DIR-aware helpers, falling back to ~/.claude. Hook command
strings written to settings.json keep the legacy tilde form when the env
var is unset, so existing configs stay portable across HOME values. Prints
a one-line migration nudge when CLAUDE_CONFIG_DIR is set and a legacy
~/.claude tree still exists.

Adds cli_detect_agents_finds_claude_via_env and isolates CLAUDE_CONFIG_DIR
in the existing detection tests so the runner env can't leak in.

Distilled from DeusData#321 onto current main (adapts to the v0.7.0 non-blocking
augmenter hook signature, which the original branch predated). Closes DeusData#320.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ux/behavior Display bugs, docs, adoption UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

install/update ignore $CLAUDE_CONFIG_DIR; writes always go to ~/.claude

3 participants