diff --git a/CHANGELOG.md b/CHANGELOG.md index 7eba73a6d..e4e1f766c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Project-scope hook configs (`/.claude/settings.json`, `/.codex/hooks.json`, sidecar `/.claude/apm-hooks.json`, and equivalents for Cursor / Gemini / Windsurf) keep repo-relative `command` paths again, so checked-in configs stay portable across clones, contributors, and CI runners. User-scope deploys (`~/.claude/settings.json` via `apm install -g`) continue to write absolute paths -- the #1310 / #1354 fix is preserved by scoping its rewrite to the user-scope branch. (#1394) - `apm install` honors the SSH user portion of dependency URLs (`ssh://user@host/...` and scp shorthand `user@host:org/repo`) instead of hardcoding `git@`; unblocks EMU accounts and other non-`git` SSH identities. User values are validated against a strict allowlist before composing the clone URL. (#1385, closes #1383) ## [0.14.0] - 2026-05-18 diff --git a/src/apm_cli/install/services.py b/src/apm_cli/install/services.py index 9c36027f4..57d66e891 100644 --- a/src/apm_cli/install/services.py +++ b/src/apm_cli/install/services.py @@ -113,6 +113,8 @@ def integrate_package_primitives( """ from apm_cli.integration.dispatch import get_dispatch_table + from ..core.scope import InstallScope + _dispatch = get_dispatch_table() result = { "prompts": 0, @@ -243,13 +245,22 @@ def _format_target_collapse(paths: list[str], verbose: bool) -> tuple[str, list[ _mapping = _target.primitives.get(_prim_name) if _mapping is None: continue + _call_kwargs: dict[str, Any] = { + "force": force, + "managed_files": managed_files, + "diagnostics": diagnostics, + } + # Hook integrator alone needs the scope signal: project-scope + # deploys keep ``command`` paths repo-relative (#1394), user-scope + # deploys absolutize them (#1310 / #1354). Sibling integrators + # don't accept this kwarg, so include it only for hooks. + if _prim_name == "hooks": + _call_kwargs["user_scope"] = scope is InstallScope.USER _int_result = getattr(_integrator, _entry.integrate_method)( _target, package_info, project_root, - force=force, - managed_files=managed_files, - diagnostics=diagnostics, + **_call_kwargs, ) result["links_resolved"] += _int_result.links_resolved for tp in _int_result.target_paths: diff --git a/src/apm_cli/integration/hook_integrator.py b/src/apm_cli/integration/hook_integrator.py index 4c8272d88..3520e92f5 100644 --- a/src/apm_cli/integration/hook_integrator.py +++ b/src/apm_cli/integration/hook_integrator.py @@ -713,6 +713,7 @@ def _integrate_merged_hooks( managed_files: set = None, # noqa: RUF013 diagnostics=None, target=None, + user_scope: bool = False, ) -> HookIntegrationResult: """Integrate hooks by merging into a target-specific JSON config. @@ -734,6 +735,19 @@ def _integrate_merged_hooks( if config.require_dir and not target_dir.exists(): return _empty + # Absolutize hook commands only for user-scope deploys. Claude + # Code (and the Codex/Cursor/Gemini equivalents) reads + # ``~/.claude/settings.json`` without a fixed cwd and does not + # expand ``${CLAUDE_PLUGIN_ROOT}`` in that file (see #1310 / #1354), + # so user-scope deploys must write absolute paths. Project-scope + # ``/.claude/settings.json`` is typically checked in and runs + # with cwd at the repo root, where repo-relative paths resolve + # correctly -- baking absolute machine paths into checked-in config + # breaks portability across clones, contributors, and CI (#1394). + # ``user_scope`` is threaded from the caller's ``InstallScope`` so + # the gate is explicit rather than inferred from deploy-root shape. + _deploy_root_for_rewrite = project_root if user_scope else None + hook_files = self.find_hook_files(package_info.install_path) hook_files = _filter_hook_files_for_target(hook_files, config.target_key) if not hook_files: @@ -799,7 +813,7 @@ def _integrate_merged_hooks( config.target_key, hook_file_dir=hook_file.parent, root_dir=root_dir, - deploy_root=project_root, + deploy_root=_deploy_root_for_rewrite, ) # Merge hooks into config (additive) @@ -964,6 +978,8 @@ def integrate_package_hooks_claude( force: bool = False, managed_files: set = None, # noqa: RUF013 diagnostics=None, + *, + user_scope: bool = False, ) -> HookIntegrationResult: """Integrate hooks into .claude/settings.json. @@ -976,6 +992,7 @@ def integrate_package_hooks_claude( force=force, managed_files=managed_files, diagnostics=diagnostics, + user_scope=user_scope, ) def integrate_package_hooks_cursor( @@ -985,6 +1002,8 @@ def integrate_package_hooks_cursor( force: bool = False, managed_files: set = None, # noqa: RUF013 diagnostics=None, + *, + user_scope: bool = False, ) -> HookIntegrationResult: """Integrate hooks into .cursor/hooks.json. @@ -997,6 +1016,7 @@ def integrate_package_hooks_cursor( force=force, managed_files=managed_files, diagnostics=diagnostics, + user_scope=user_scope, ) def integrate_package_hooks_codex( @@ -1006,6 +1026,8 @@ def integrate_package_hooks_codex( force: bool = False, managed_files: set = None, # noqa: RUF013 diagnostics=None, + *, + user_scope: bool = False, ) -> HookIntegrationResult: """Integrate hooks into .codex/hooks.json. @@ -1018,6 +1040,7 @@ def integrate_package_hooks_codex( force=force, managed_files=managed_files, diagnostics=diagnostics, + user_scope=user_scope, ) # ------------------------------------------------------------------ @@ -1033,12 +1056,19 @@ def integrate_hooks_for_target( force: bool = False, managed_files: set = None, # noqa: RUF013 diagnostics=None, + user_scope: bool = False, ) -> "HookIntegrationResult": """Integrate hooks for a single *target*. Copilot uses individual JSON files (genuinely different pattern). All other merge-based targets are dispatched via the ``_MERGE_HOOK_TARGETS`` registry. + + ``user_scope`` controls whether merged-hook ``command`` paths are + rewritten to absolute paths (required when deploying to + ``~/.claude/settings.json`` -- see #1310 / #1354) or left + repo-relative so checked-in project-scope configs stay portable + across clones, contributors, and CI runners (#1394). """ if target.name == "copilot": return self.integrate_package_hooks( @@ -1060,6 +1090,7 @@ def integrate_hooks_for_target( managed_files=managed_files, diagnostics=diagnostics, target=target, + user_scope=user_scope, ) return HookIntegrationResult( diff --git a/tests/unit/integration/test_hook_integrator.py b/tests/unit/integration/test_hook_integrator.py index 9ee3e2d00..7fd7cad4a 100644 --- a/tests/unit/integration/test_hook_integrator.py +++ b/tests/unit/integration/test_hook_integrator.py @@ -763,12 +763,13 @@ def extract_commands(text: str) -> set: assert all("_apm_source" not in e for e in stop) return {h["command"] for entry in stop for h in entry["hooks"]} - # Commands are now absolute paths (deploy_root=project_root is threaded - # through _integrate_merged_hooks so Claude Code never sees an unexpanded - # ${CLAUDE_PLUGIN_ROOT} variable). Assert on the absolute form. + # Project-scope writes repo-relative paths so checked-in + # settings.json stays portable across clones / CI (#1394). + # User-scope deploys (project_root == ~) still absolutize via + # _rewrite_command_for_target's deploy_root branch -- see #1354. assert extract_commands(first) == { - str((temp_project / ".claude/hooks/multi-stop-pkg/hooks/stop-a.sh").resolve()), - str((temp_project / ".claude/hooks/multi-stop-pkg/hooks/stop-b.sh").resolve()), + ".claude/hooks/multi-stop-pkg/hooks/stop-a.sh", + ".claude/hooks/multi-stop-pkg/hooks/stop-b.sh", } # Verify sidecar has the ownership info @@ -782,6 +783,99 @@ def extract_commands(text: str) -> set: assert settings_path.read_text() == first + def test_project_scope_writes_relative_hook_paths(self, temp_project): + """Project-scope (.claude/settings.json checked into the repo) must keep + repo-relative hook commands so the generated config is portable across + clones / contributors / CI runners (#1394). + + Regression: #1354 unconditionally threaded deploy_root=project_root to + absolutize commands -- correct for user-scope (~/.claude/settings.json, + which #1310 was about) but wrong for project-scope, where it baked the + installer's machine-local absolute prefix into the committed JSON. + """ + pkg_dir = temp_project / "scope-pkg" + hooks_dir = pkg_dir / "hooks" + hooks_dir.mkdir(parents=True, exist_ok=True) + (hooks_dir / "hooks.json").write_text( + json.dumps( + { + "hooks": { + "Stop": [ + { + "hooks": [ + { + "type": "command", + "command": "${CLAUDE_PLUGIN_ROOT}/hooks/stop.sh", + } + ] + } + ] + } + } + ) + ) + (hooks_dir / "stop.sh").write_text("#!/bin/bash\nexit 0") + + pkg_info = _make_package_info(pkg_dir, "scope-pkg") + integrator = HookIntegrator() + integrator.integrate_package_hooks_claude(pkg_info, temp_project) + + settings = json.loads((temp_project / ".claude" / "settings.json").read_text()) + cmd = settings["hooks"]["Stop"][0]["hooks"][0]["command"] + assert cmd == ".claude/hooks/scope-pkg/hooks/stop.sh", ( + f"Project-scope command must be repo-relative; got {cmd!r}" + ) + assert not Path(cmd).is_absolute(), ( + f"Project-scope command must not be absolute; got {cmd!r}" + ) + assert str(temp_project) not in cmd, ( + f"Project-scope command must not embed the installer's absolute prefix; got {cmd!r}" + ) + + def test_user_scope_still_writes_absolute_hook_paths(self, temp_project): + """User-scope deploys must still absolutize hook commands -- + ``~/.claude/settings.json`` runs without a fixed cwd, so relative + paths cannot resolve (#1310 / #1354). + + ``user_scope=True`` is the explicit signal the production dispatch + (``services.integrate_package_primitives``) computes from the + ``InstallScope`` enum, kept independent of deploy-root layout in + ``core/scope.py``. + """ + pkg_dir = temp_project / "scope-pkg" + hooks_dir = pkg_dir / "hooks" + hooks_dir.mkdir(parents=True, exist_ok=True) + (hooks_dir / "hooks.json").write_text( + json.dumps( + { + "hooks": { + "Stop": [ + { + "hooks": [ + { + "type": "command", + "command": "${CLAUDE_PLUGIN_ROOT}/hooks/stop.sh", + } + ] + } + ] + } + } + ) + ) + (hooks_dir / "stop.sh").write_text("#!/bin/bash\nexit 0") + + pkg_info = _make_package_info(pkg_dir, "scope-pkg") + integrator = HookIntegrator() + integrator.integrate_package_hooks_claude(pkg_info, temp_project, user_scope=True) + + settings = json.loads((temp_project / ".claude" / "settings.json").read_text()) + cmd = settings["hooks"]["Stop"][0]["hooks"][0]["command"] + assert Path(cmd).is_absolute(), f"User-scope command must be absolute; got {cmd!r}" + assert cmd == str( + (temp_project / ".claude" / "hooks" / "scope-pkg" / "hooks" / "stop.sh").resolve() + ) + def test_no_hooks_returns_empty_result(self, temp_project): """Test Claude integration with no hook files returns empty result.""" pkg_dir = temp_project / "pkg" @@ -2450,6 +2544,37 @@ def test_codex_hooks_not_deployed_without_codex_dir(self): assert result.files_integrated == 0 + def test_codex_project_scope_keeps_relative_hook_paths(self): + """Project-scope .codex/hooks.json must stay portable (#1394). + + Mirrors the Claude regression test for the Codex target so the + shared _integrate_merged_hooks scope check is asserted from both + public entry points. + """ + hook_data = { + "hooks": { + "SessionStart": [ + {"type": "command", "command": "${CURSOR_PLUGIN_ROOT}/hooks/run.sh"} + ] + } + } + pi = self._make_package_info(hook_data=hook_data) + # ${CURSOR_PLUGIN_ROOT}/hooks/run.sh resolves under package root. + scripts_dir = pi.install_path / "hooks" + scripts_dir.mkdir(parents=True, exist_ok=True) + (scripts_dir / "run.sh").write_text("#!/bin/bash\nexit 0") + + integrator = HookIntegrator() + integrator.integrate_package_hooks_codex(pi, self.root) + + data = json.loads((self.root / ".codex" / "hooks.json").read_text()) + cmd = data["hooks"]["SessionStart"][0]["command"] + assert cmd == ".codex/hooks/test-pkg/hooks/run.sh", ( + f"Project-scope Codex command must be repo-relative; got {cmd!r}" + ) + assert not Path(cmd).is_absolute() + assert str(self.root) not in cmd + # --- Gemini hook integration tests -----------------------------------------------