Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Project-scope hook configs (`<repo>/.claude/settings.json`, `<repo>/.codex/hooks.json`, sidecar `<repo>/.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
Expand Down
14 changes: 13 additions & 1 deletion src/apm_cli/integration/hook_integrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,18 @@ 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
# ``<repo>/.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).
_is_user_scope = project_root.resolve() == Path.home().resolve()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in b576258 -- replaced the project_root == Path.home() heuristic with an explicit user_scope: bool kwarg threaded from services.integrate_package_primitives (where the InstallScope is already in scope).

Concrete answers to the three concerns:

  • $HOME-as-project misclassification: gone. User-scope is now driven by scope is InstallScope.USER, not deploy-root shape.
  • Implicit coupling to get_deploy_root: gone. If user-scope deploys later move to ~/.config/..., the gate still holds because it doesn't inspect project_root at all.
  • Inference vs intent: now explicit at the call site. Sibling integrators (prompt/agent/command/instruction) keep their existing signatures -- the user_scope kwarg is added only to integrate_hooks_for_target, the three deprecated per-target wrappers, and _integrate_merged_hooks, and the dispatch in services.py includes it only when _prim_name == "hooks".

The user-scope regression test (test_user_scope_still_writes_absolute_hook_paths) now sets user_scope=True directly instead of monkey-patching Path.home, so it exercises the same explicit signal production code uses.

_deploy_root_for_rewrite = project_root if _is_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:
Expand Down Expand Up @@ -799,7 +811,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)
Expand Down
137 changes: 131 additions & 6 deletions tests/unit/integration/test_hook_integrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import shutil
import tempfile
from pathlib import Path
from unittest.mock import MagicMock
from unittest.mock import MagicMock, patch

import pytest

Expand Down Expand Up @@ -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
Expand All @@ -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 (project_root == Path.home()) must still absolutize hook
commands -- ``~/.claude/settings.json`` runs without a fixed cwd, so
relative paths cannot resolve (#1310 / #1354).

This test patches Path.home() to equal the temp project root so the
scope check inside _integrate_merged_hooks treats this deploy as user
scope without touching the real $HOME.
"""
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()
with patch("pathlib.Path.home", return_value=temp_project):
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 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"
Expand Down Expand Up @@ -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 -----------------------------------------------

Expand Down
Loading