fix: respect CLAUDE_CONFIG_DIR for claude target user-scope deploy by shuntaka9576 · Pull Request #1055 · microsoft/apm

shuntaka9576 · 2026-04-29T21:58:19Z

Description

Claude Code reads its config from $CLAUDE_CONFIG_DIR (default ~/.claude), but apm install -g --target claude always wrote to ~/.claude/. Users who set CLAUDE_CONFIG_DIR=~/.config/claude saw APM's deploys land in a tree Claude Code never reads.

TargetProfile.for_scope(user_scope=True) now resolves $CLAUDE_CONFIG_DIR into a home-relative posix string for claude's root_dir, preserving the project_root-relative prefix invariant used by partition_managed_files / validate_deploy_path. Deploy and cleanup both honor the override.

Type of change

Bug fix

Out of scope

CLAUDE_CONFIG_DIR values outside $HOME are intentionally not supported. At user scope project_root = Path.home(), so out-of-$HOME paths can't be expressed relative to project_root and the lockfile step fails for them. Claude Code's documented configurations (~/.claude, ~/.config/claude) all live under $HOME; covering arbitrary paths would need a claude:// URI scheme analogous to cowork://, which is much larger than this fix.

The fall-through (absolute string in root_dir) is pinned by test_user_scope_outside_home_keeps_absolute so it can be replaced cleanly when the URI scheme arrives.

Testing

uv run pytest tests/unit — passed (6706 tests).

End-to-end deploy + uninstall under $CLAUDE_CONFIG_DIR:

$ export CLAUDE_CONFIG_DIR="$HOME/.config/test-claude-pr1055"
$ uv run --python 3.13 apm install -g --target claude vercel-labs/agent-skills
...
  |-- 7 skill(s) integrated -> .config/skills/

$ ls -1 "$CLAUDE_CONFIG_DIR/skills/"
composition-patterns
deploy-to-vercel
react-best-practices
react-native-skills
react-view-transitions
vercel-cli-with-tokens
web-design-guidelines

$ ls -1 "$CLAUDE_CONFIG_DIR/skills/" > /tmp/deployed
$ ls "$HOME/.claude/skills/" 2>/dev/null | grep -F -x -f /tmp/deployed || echo "OK: ~/.claude/skills/ has none of the deployed skills"
OK: ~/.claude/skills/ has none of the deployed skills

$ uv run --python 3.13 apm uninstall -g vercel-labs/agent-skills
...
[+] Cleaned up 7 integrated skills
$ ls "$CLAUDE_CONFIG_DIR/skills/"
(empty)

All seven skills land under $CLAUDE_CONFIG_DIR, ~/.claude/ is untouched, and uninstall removes them via prefix matching.

Test additions:

test_scope_integration.py::TestClaudeScopeResolution — env-var override cases (~ expansion, blank fallback, out-of-$HOME fall-through, project-scope ignore).
test_scope_install_uninstall.py::test_user_scope_with_claude_config_dir — install→sync_for_target cycle under CLAUDE_CONFIG_DIR, locking the prefix-matching contract for cleanup.
Existing claude scope tests in test_data_driven_dispatch.py, test_scope_install_uninstall.py, and test_scope_integration.py are pinned with monkeypatch.delenv("CLAUDE_CONFIG_DIR", raising=False) so the default behaviour is exercised regardless of the developer's shell.

Copilot

Pull request overview

Fixes Claude user-scope integration so global installs can deploy into the directory Claude Code actually reads when CLAUDE_CONFIG_DIR is set, rather than always writing to ~/.claude.

Changes:

Add CLAUDE_CONFIG_DIR handling for the claude target during TargetProfile.for_scope(user_scope=True).
Add/adjust unit tests to cover env-var override semantics and to pin CLAUDE_CONFIG_DIR unset in other scope-resolution tests for determinism.
Update inline target registry comments for Claude to reflect the new behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`src/apm_cli/integration/targets.py`	Adds user-scope root override for the `claude` target based on `CLAUDE_CONFIG_DIR`; updates Claude target comments.
`tests/unit/integration/test_targets.py`	Adds new test class covering `CLAUDE_CONFIG_DIR` behavior for Claude user scope.
`tests/unit/integration/test_scope_integration.py`	Pins `CLAUDE_CONFIG_DIR` unset to keep existing scope resolution assertions stable.
`tests/unit/integration/test_scope_install_uninstall.py`	Pins `CLAUDE_CONFIG_DIR` unset for existing Claude user-scope install/uninstall test.
`tests/unit/integration/test_data_driven_dispatch.py`	Pins `CLAUDE_CONFIG_DIR` unset for the “user_root_dir is None” behavior test (Claude).

shuntaka9576 · 2026-04-29T23:29:54Z

@microsoft-github-policy-service agree

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Copilot · 2026-04-30T00:46:57Z

+        monkeypatch.setenv("HOME", str(self.project_root))
+        custom = self.project_root / ".config" / "test-claude"
+        monkeypatch.setenv("CLAUDE_CONFIG_DIR", str(custom))
+        custom.mkdir(parents=True)


This test patches only HOME to make Path.home() point at self.project_root, but on Windows Path.home() typically prefers USERPROFILE over HOME. In that case for_scope(user_scope=True) will treat CLAUDE_CONFIG_DIR as outside home and return an absolute root_dir, making this assertion fail on the Windows unit-test job. Consider also patching USERPROFILE (and/or HOMEDRIVE/HOMEPATH) when sys.platform == "win32", consistent with the repo's other cross-platform home-override helpers.

Copilot · 2026-04-30T00:46:58Z

+    # Claude Code -- the user-level config directory is whatever
+    # ``CLAUDE_CONFIG_DIR`` points to (default ``~/.claude``).  The env
+    # var override is honored by ``for_scope(user_scope=True)``.
    # All primitives are supported at user scope.
    # Ref: https://docs.anthropic.com/en/docs/claude-code/settings


User-scope Claude deploy root is no longer always ~/.claude/ (it can be overridden by CLAUDE_CONFIG_DIR). Several Starlight docs under docs/src/content/docs/ still hardcode ~/.claude/ for global installs (e.g. guides/dependencies.md and integrations/ide-tool-integration.md). Please update the relevant docs to mention the env-var override (keeping it concise) so guidance matches the new behavior.

…deploy Resolve $CLAUDE_CONFIG_DIR into a home-relative root_dir at user scope so claude deploys land where Claude reads and cleanup's prefix matching keeps holding. resolve() collapses `..` before relative_to to prevent traversal escapes. Out-of-$HOME values fall through unchanged.

…ool-integration Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

danielmeppiel · 2026-04-30T09:36:57Z

Maintainer rebase + panel verdict (per #1064 FIX-NOW orchestration)

Rebased on origin/main (clean). Ran apm-review-panel critique locally and applied doc-writer's HIGH machine-actionable findings in-situ on top of your fix.

Verdict: READY

Panel findings (7 personas)

python-architect: inline imports consistent with existing pattern in same file (lines 519, 582). OK.
cli-logging-expert: no CLI surface touched. N/A.
devx-ux-expert: behaviour mirrors what Claude Code itself reads -- correct user expectation. PR body documents the out-of-$HOME limitation. OK.
supply-chain-security-expert: resolve(strict=False) collapses .. BEFORE relative_to(home), traversal segments cannot leak. Pinned by test_user_scope_collapses_dotdot_segments. OK.
oss-growth-hacker: genuine bug fix for advanced Claude Code users. OK.
doc-writer: HIGH machine-actionable -- ~/.claude/ referenced in user-facing docs without mention of the override. Applied (commit c95e9e85):
- docs/src/content/docs/guides/dependencies.md: note $CLAUDE_CONFIG_DIR in user-level dir column + paragraph
- docs/src/content/docs/integrations/ide-tool-integration.md: callout under "Claude Integration" auto-detect note
apm-ceo: APPROVE. No product-direction conflict, no new public CLI surface, no security weakening.

Test evidence

uv run --extra dev pytest tests/unit -x --tb=short -q
=> 6775 passed, 1 warning, 27 subtests passed in 55.79s

uv run --extra dev pytest tests/integration/test_global_install_e2e.py -x --tb=short
=> 3 SKIPPED (network-gated, expected without opt-in env)

PR's own test_user_scope_with_claude_config_dir exercises the full install -> sync_for_target cycle under custom HOME + CLAUDE_CONFIG_DIR via monkeypatch (real AgentIntegrator, real package data, prefix-matching cleanup contract) -- matches fidelity matrix "real Claude config dir via env-var override".

Merging via squash next.

danielmeppiel

Approving — all required checks SUCCESS on c95e9e8 with the panel-doc commit. Doc-writer additions cover Claude config dir support in user-facing docs.

…#1065) * fix: address Copilot review findings on #1055 and #991 (consolidated) Folds the still-actionable findings from the post-merge Copilot reviews of PRs #1055 and #991 into a single PR (no follow-up issue / PR sprawl). #1055 (CLAUDE_CONFIG_DIR support): - Document why the absolute-path fallback in TargetProfile.for_scope() is safe for downstream consumers (pathlib's 'rhs absolute wins' rule on '/' joins). Comment-only; no behavior change. - Findings already addressed by the original PR (verified): * resolve(strict=False) collapses '..' before relative_to(home) * docs/guides/dependencies.md + integrations/ide-tool-integration.md already cover CLAUDE_CONFIG_DIR in the merged version * test_scope_install_uninstall.py:382 + test_scope_integration.py already exercise install/uninstall + outside-home + traversal cases #991 (link_resolver guards): - Correct the embedded-NUL-byte test docstring: Path.exists() raises ValueError on NUL on most platforms (it does not silently return False). Cosmetic; test behavior unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address Copilot review on #1065 Two valid findings from copilot-pull-request-reviewer: 1. link_resolver._resolve_path: NUL byte in input previously returned a Path that crashed downstream .exists() / .read_text() with ValueError, aborting markdown link resolution. Reject NUL at the resolver boundary so callers (resolve_markdown_links, validate_link_targets) get None as documented. Tightened test to assert None instead of 'either is fine'. 2. integration/targets.for_scope: prior comment claimed absolute root_dir was 'safe' for all downstream consumers. install/services. _deployed_path_entry would actually raise RuntimeError for an out-of-tree absolute path. Today this is unreachable because user-scope CLAUDE installs do not flow through that translator, but the comment now records the constraint so a future refactor that lockfiles user-scope deploys treats it as a dynamic-root case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* test(windows): patch USERPROFILE so Path.home() honors fake HOME The CLAUDE_CONFIG_DIR scope tests added in #1055 patched only HOME, but Path.home() on Windows uses USERPROFILE (with HOMEDRIVE+HOMEPATH as fallback) and ignores HOME. As a result, Path.home() returned the real runner profile, relative_to(home) succeeded against the AppData/Local tmp_path, and the assertions on root_dir failed on windows-latest. Add a small _set_home() helper in both scope test modules that also sets USERPROFILE / HOMEDRIVE / HOMEPATH on Windows, and use it in the three failing tests: - test_user_scope_with_claude_config_dir - test_user_scope_outside_home_keeps_absolute - test_user_scope_collapses_dotdot_segments plus test_user_scope_expands_tilde for consistency. Also resolve() the expected outside path in test_user_scope_outside_home_keeps_absolute to match the source's abs_path.expanduser().resolve() output (e.g. /var -> /private/var on macOS, 8.3 short-name expansion on Windows). Fixes failures from CI run 25190857376. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: address review nits on outside-home assertion - Update comment to reflect that paths outside $HOME are resolved (normalized), not preserved verbatim. - Make the .resolve() call explicit with strict=False to mirror the implementation in for_scope() and document the expected behavior for non-existent paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <copilot-rework@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

shuntaka9576 requested a review from danielmeppiel as a code owner April 29, 2026 21:58

Copilot AI review requested due to automatic review settings April 29, 2026 21:58

shuntaka9576 requested a review from sergio-sisternes-epam as a code owner April 29, 2026 21:58

Copilot started reviewing on behalf of shuntaka9576 April 29, 2026 21:59 View session

shuntaka9576 marked this pull request as draft April 29, 2026 22:00

Copilot AI reviewed Apr 29, 2026

View reviewed changes

shuntaka9576 force-pushed the fix/claude-config-dir-support branch 2 times, most recently from 814e680 to 8063b5f Compare April 30, 2026 00:08

shuntaka9576 marked this pull request as ready for review April 30, 2026 00:11

shuntaka9576 requested a review from Copilot April 30, 2026 00:11

Copilot AI reviewed Apr 30, 2026

View reviewed changes

Comment thread src/apm_cli/integration/targets.py

Comment thread src/apm_cli/integration/targets.py Outdated

Comment thread tests/unit/integration/test_scope_integration.py Outdated

Copilot started reviewing on behalf of shuntaka9576 April 30, 2026 00:21 View session

shuntaka9576 marked this pull request as draft April 30, 2026 00:31

shuntaka9576 force-pushed the fix/claude-config-dir-support branch from 8063b5f to e4f4e54 Compare April 30, 2026 00:38

shuntaka9576 requested a review from Copilot April 30, 2026 00:40

Copilot started reviewing on behalf of shuntaka9576 April 30, 2026 00:40 View session

shuntaka9576 marked this pull request as ready for review April 30, 2026 00:41

Copilot AI reviewed Apr 30, 2026

View reviewed changes

danielmeppiel mentioned this pull request Apr 30, 2026

[Summary] Next issues and PRs to solve, prioritized #1064

Closed

shuntaka9576 and others added 2 commits April 30, 2026 11:31

panel: doc CLAUDE_CONFIG_DIR override in dependencies guide and ide-t…

c95e9e8

…ool-integration Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

danielmeppiel force-pushed the fix/claude-config-dir-support branch from e4f4e54 to c95e9e8 Compare April 30, 2026 09:36

danielmeppiel approved these changes Apr 30, 2026

View reviewed changes

danielmeppiel merged commit 833f734 into microsoft:main Apr 30, 2026
11 checks passed

danielmeppiel mentioned this pull request Apr 30, 2026

fix: address Copilot review findings on #1055 and #991 (consolidated) #1065

Merged

danielmeppiel mentioned this pull request Apr 30, 2026

test(windows): fix CLAUDE_CONFIG_DIR scope tests on Windows #1086

Merged

danielmeppiel mentioned this pull request May 1, 2026

[docs] Update documentation for features from 2026-05-01 #1095

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: respect CLAUDE_CONFIG_DIR for claude target user-scope deploy#1055

fix: respect CLAUDE_CONFIG_DIR for claude target user-scope deploy#1055
danielmeppiel merged 2 commits intomicrosoft:mainfrom
shuntaka9576:fix/claude-config-dir-support

shuntaka9576 commented Apr 29, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shuntaka9576 commented Apr 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

danielmeppiel commented Apr 30, 2026

Uh oh!

danielmeppiel left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

shuntaka9576 commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Out of scope

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shuntaka9576 commented Apr 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

danielmeppiel commented Apr 30, 2026

Maintainer rebase + panel verdict (per #1064 FIX-NOW orchestration)

Panel findings (7 personas)

Test evidence

Uh oh!

danielmeppiel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shuntaka9576 commented Apr 29, 2026 •

edited

Loading