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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **BREAKING:** after consumers re-vendor the shared gh-aw `apm.md`, its import requires an explicit `target` instead of deprecated `all`; `apm-action` otherwise writes `all` into the isolated `apm.yml`, where it degrades to auto-detection without harness markers. Set the workflow engine's target and recompile; see the [gh-aw migration recipe](https://microsoft.github.io/apm/integrations/gh-aw/#shared-apmmd-import-recommended). (#2706)
- Re-vendored shared gh-aw workflows now default to APM 0.28.0 for both pack and restore, the version used for the recorded `microsoft/apm-action@v1.10.0` compatibility proof, not the latest CLI release; an explicit `apm-version` still overrides it. (#2706)

### Fixed

- User-scope hook commands now anchor to `$HOME` on POSIX hosts instead of the installing host's home prefix, so a `~/.claude/settings.json` kept in a dotfiles repo stops churning between machines; Windows targets and dynamic config roots outside the home directory keep absolute paths. (closes #2821)
Comment thread
Copilot marked this conversation as resolved.
Outdated

### Security

- The shared gh-aw APM pack job now declares `contents: read` (previously `permissions: {}`), the minimum the explicit built-in-token path needs. No write scope is added, and the token is not forwarded to restore or agent jobs. (#2706)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,18 @@ agent a procedure" fits a skill -- and reaches every harness.
alias) for scripts that ship inside the package, using the quoting forms
described above. Plain absolute paths break on consumers' machines.
- **Hook script path resolution.** `apm install -g` (user-scope)
rewrites `${PLUGIN_ROOT}` and relative `./` references to absolute
paths so Claude Code and Copilot CLI can execute scripts regardless
of the working directory. Project-scope `apm install` (no `-g`) keeps
rewrites `${PLUGIN_ROOT}` and relative `./` references so Claude Code
and Copilot CLI can execute scripts regardless of the working
directory. On POSIX hosts the rewritten path is anchored to `$HOME`
(for example `$HOME/.claude/hooks/<pkg>/run.sh`), which the shell
expands at invocation time, so a user-scope config kept in a dotfiles
Comment on lines +288 to +292
repo stays valid on a host with a different home directory. Because the
anchor resolves late, the hook runs the script under whatever `HOME` the
launching shell provides -- keep `HOME` pointing at the installing user's
home when a wrapper script, service, or CI job invokes the harness.
Windows keeps the absolute form, and a dynamic target root outside the
home directory (for example `CLAUDE_CONFIG_DIR`) stays absolute too.
Project-scope `apm install` (no `-g`) keeps
non-Claude command paths repo-relative. Claude project hooks use
`CLAUDE_PROJECT_DIR` (or `$env:CLAUDE_PROJECT_DIR` for PowerShell) so
checked-in settings remain portable while hooks can run from outside the
Expand Down
5 changes: 3 additions & 2 deletions docs/src/content/docs/reference/targets-matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,9 @@ GitHub Copilot (CLI and IDE).
`instructions` from all packages are concatenated into
`~/.copilot/copilot-instructions.md` (Copilot CLI reads only that single file
at user scope). User-scope deploys land under `~/.copilot/`, not
`~/.github/`; hook script commands are written as absolute paths so Copilot
CLI can invoke them from any working directory.
`~/.github/`; hook script commands are anchored so Copilot CLI can invoke
them from any working directory -- `$HOME`-relative on POSIX hosts,
absolute on Windows.
- **Global compile.** `apm compile -g` can also render global instructions to
`~/.copilot/AGENTS.md` for root-context readers that honor `AGENTS.md`.

Expand Down
5 changes: 3 additions & 2 deletions src/apm_cli/install/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,9 @@ def _log_integration(msg):
}
# 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.
# deploys make them cwd-independent (#1310 / #1354), anchored to
# ``$HOME`` on POSIX hosts. 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
_call_kwargs["dep_targets_active"] = dep_targets_active
Expand Down
29 changes: 24 additions & 5 deletions src/apm_cli/integration/hook_integrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@

import json
import logging
import os
import re
from collections.abc import Iterable
from dataclasses import dataclass, field
Expand Down Expand Up @@ -112,6 +113,8 @@

_log = logging.getLogger(__name__)

_POSIX_USER_HOOK_PATHS = os.name != "nt"

# Testability seam: tests can patch deprecated filename routing without
# replacing the imported helper for every call site.
_filter_hook_files_for_target = filter_hook_files_for_target
Expand Down Expand Up @@ -592,7 +595,23 @@ def _project_scoped_command_path(
) -> str:
"""Return a target-native script reference without sacrificing portability."""
if deploy_root is not None:
return str((deploy_root / target_rel).resolve())
target_path = (deploy_root / target_rel).resolve()
if _POSIX_USER_HOOK_PATHS:
try:
relative_path = target_path.relative_to(deploy_root.resolve())
except ValueError:
# A dynamic target root such as CLAUDE_CONFIG_DIR may live
# outside the deploy root. Keep that explicit absolute path
# rather than emitting a misleading $HOME-relative command.
pass
else:
# User-scope configs live in files people track in dotfiles
# repos, so anchor the path to $HOME instead of embedding
# the installing host's home prefix. Hooks run through a
# shell, which expands $HOME at invocation time and keeps
# the #1310 / #1354 cwd-independence intact.
return f"$HOME/{relative_path.as_posix()}"
Comment thread
Copilot marked this conversation as resolved.
return str(target_path)
if target != "claude":
return target_rel

Expand Down Expand Up @@ -757,7 +776,7 @@ def _rewrite_hooks_data(
hook_file_dir: Directory containing the hook JSON file (for ./path resolution)
root_dir: Override root directory (e.g. ".copilot" for user scope)
deploy_root: Absolute root of the deployment directory. When provided,
all rewritten script paths are resolved to absolute paths so the
all rewritten script paths are pinned to the deploy root so the
target can locate scripts regardless of the working directory.
When *None*, paths remain relative (backward-compatible behaviour).

Expand Down Expand Up @@ -1026,8 +1045,8 @@ def integrate_package_hooks(
force: If True, overwrite user-authored files on collision
managed_files: Set of relative paths known to be APM-managed
target: Optional TargetProfile for scope-resolved root_dir
user_scope: If True, rewrite hook script commands to absolute paths
so global hooks resolve from any working directory
user_scope: If True, rewrite hook script commands so global hooks
resolve from any working directory

Returns:
HookIntegrationResult: Results of the integration operation
Expand Down Expand Up @@ -1675,7 +1694,7 @@ def integrate_hooks_for_target(
``_MERGE_HOOK_TARGETS`` registry.

``user_scope`` controls whether merged-hook ``command`` paths are
rewritten to absolute paths (required when deploying to
rewritten so they resolve from any cwd (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).
Expand Down
18 changes: 14 additions & 4 deletions tests/integration/test_hook_integrator_copilot_casing_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from __future__ import annotations

import json
import os
from pathlib import Path
from typing import Any

Expand Down Expand Up @@ -192,8 +193,13 @@ def test_copilot_install_scope_controls_script_paths(
).resolve()
monkeypatch.chdir(package_info.install_path)
if user_scope:
assert Path(command).is_absolute()
assert Path(command).resolve() == installed_script
# User-scope commands must resolve from any cwd. On POSIX they anchor
# to $HOME so a settings file kept in a dotfiles repo stays valid on a
# host with a different home directory; Windows keeps absolute paths.
relative = f"{target.root_dir}/hooks/scripts/{package_info.package.name}/run.sh"
assert command == (f"$HOME/{relative}" if os.name != "nt" else str(installed_script))
expanded = command.replace("$HOME", str(Path.home()))
assert Path(expanded).resolve() == installed_script
else:
assert not Path(command).is_absolute()
assert command == f"{target.root_dir}/hooks/scripts/{package_info.package.name}/run.sh"
Expand Down Expand Up @@ -236,8 +242,12 @@ def test_kiro_install_scope_controls_script_paths(
).resolve()
monkeypatch.chdir(package_info.install_path)
if user_scope:
assert Path(command).is_absolute()
assert Path(command).resolve() == installed_script
# Same POSIX $HOME anchor as Copilot: Kiro consumes the shared hook
# scope rewrite decision.
relative = f"{target.root_dir}/hooks/{package_info.package.name}/run.sh"
assert command == (f"$HOME/{relative}" if os.name != "nt" else str(installed_script))
expanded = command.replace("$HOME", str(Path.home()))
assert Path(expanded).resolve() == installed_script
else:
assert not Path(command).is_absolute()
assert command == f"{target.root_dir}/hooks/{package_info.package.name}/run.sh"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -725,13 +725,17 @@ def test_required_global_copilot_sidecar_lifecycle(
descriptor = copilot_root.joinpath(*_GLOBAL_DESCRIPTOR.parts)
payload = json.loads(descriptor.read_text(encoding="utf-8"))
command = tuple(shlex.split(payload["hooks"]["preToolUse"][0]["bash"]))
assert Path(command[1]).is_absolute()
# POSIX user-scope commands anchor to $HOME so `~/.copilot` config stays
# portable across hosts. The real harness runs hooks through a shell, which
# expands it; mirror that here because the runner executes directly.
hook_argv = [arg.replace("$HOME", str(scenario.isolated.home)) for arg in command[1:]]
assert Path(hook_argv[0]).is_absolute()
hook_result = ApmLifecycleRunner(
(command[0],),
timeout_seconds=30,
scenario_timeout_seconds=30,
).run(
command[1:],
hook_argv,
scenario_id="global-copilot-execute",
cwd=cwd,
env=published.environment,
Expand Down
102 changes: 76 additions & 26 deletions tests/unit/integration/test_hook_integrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -850,16 +850,17 @@ def test_project_scope_writes_portable_hook_paths(self, temp_project):
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).
def test_user_scope_writes_portable_home_hook_paths(self, temp_project, monkeypatch):
"""POSIX user-scope commands anchor to HOME instead of the installer home.

``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``.
"""
from apm_cli.integration import hook_integrator as hi_mod

monkeypatch.setattr(hi_mod, "_POSIX_USER_HOOK_PATHS", True)
pkg_dir = temp_project / "scope-pkg"
hooks_dir = pkg_dir / "hooks"
hooks_dir.mkdir(parents=True, exist_ok=True)
Expand Down Expand Up @@ -889,10 +890,7 @@ def test_user_scope_still_writes_absolute_hook_paths(self, 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()
)
assert cmd == "$HOME/.claude/hooks/scope-pkg/hooks/stop.sh"

def test_no_hooks_returns_empty_result(self, temp_project):
"""Test Claude integration with no hook files returns empty result."""
Expand Down Expand Up @@ -3338,8 +3336,11 @@ def test_script_paths_rewritten_with_scope_root(self, monkeypatch):
monkeypatch.chdir(self.root)
assert Path(cmd).resolve() == (scripts_dir / "run.sh").resolve()

def test_copilot_user_scope_writes_absolute_hook_paths(self, monkeypatch):
"""Copilot user-scope hook commands must resolve from any cwd."""
def test_copilot_user_scope_writes_portable_home_hook_paths(self, monkeypatch):
"""POSIX Copilot user hooks anchor to HOME instead of the installer home."""
from apm_cli.integration import hook_integrator as hi_mod

monkeypatch.setattr(hi_mod, "_POSIX_USER_HOOK_PATHS", True)
hooks_dir = self.pkg_dir / ".apm" / "hooks"
script = hooks_dir / "run.sh"
script.write_text("#!/bin/bash\necho test", encoding="utf-8")
Expand All @@ -3363,11 +3364,7 @@ def test_copilot_user_scope_writes_absolute_hook_paths(self, monkeypatch):
(self.root / ".copilot" / "hooks" / "scope-pkg-hooks.json").read_text(encoding="utf-8")
)
cmd = hooks_config["hooks"]["sessionStart"][0]["bash"]
assert Path(cmd).is_absolute(), f"User-scope Copilot command must be absolute; got {cmd!r}"
expected = (self.root / ".copilot" / "hooks" / "scripts" / "scope-pkg" / "run.sh").resolve()
assert cmd == str(expected)
monkeypatch.chdir(self.pkg_dir)
assert Path(cmd).resolve() == expected
assert cmd == "$HOME/.copilot/hooks/scripts/scope-pkg/run.sh"

def test_sync_with_copilot_scope_prefix(self):
"""sync_integration removes .copilot/hooks/ files when target is present."""
Expand Down Expand Up @@ -3886,8 +3883,13 @@ def test_rewrite_partial_variable_no_match(self, tmp_path: Path) -> None:
assert cmd == original, "Unknown variable must not be modified"
assert scripts == [], "No scripts should be scheduled for copy"

def test_rewrite_command_deploy_root_produces_absolute_path(self, tmp_path: Path) -> None:
"""deploy_root parameter makes _rewrite_command_for_target produce absolute paths."""
def test_rewrite_command_deploy_root_uses_portable_home_path(
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
"""POSIX user hooks keep paths portable across home directories."""
from apm_cli.integration import hook_integrator as hi_mod

monkeypatch.setattr(hi_mod, "_POSIX_USER_HOOK_PATHS", True)
pkg_dir = tmp_path / "pkg"
script = pkg_dir / "hooks" / "run.sh"
script.parent.mkdir(parents=True, exist_ok=True)
Expand All @@ -3905,13 +3907,58 @@ def test_rewrite_command_deploy_root_produces_absolute_path(self, tmp_path: Path

assert "${CLAUDE_PLUGIN_ROOT}" not in cmd, "Variable must be replaced"
assert len(scripts) == 1, "Script copy entry must be produced"
assert cmd.startswith(str(deploy_root.resolve())), (
f"Command must be absolute path under deploy_root; got {cmd}"
assert cmd == "$HOME/.claude/hooks/my-pkg/hooks/run.sh"

def test_rewrite_command_deploy_root_keeps_windows_absolute_path(
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Windows user hooks keep their existing absolute path representation."""
from apm_cli.integration import hook_integrator as hi_mod

monkeypatch.setattr(hi_mod, "_POSIX_USER_HOOK_PATHS", False)
pkg_dir = tmp_path / "pkg"
script = pkg_dir / "hooks" / "run.sh"
script.parent.mkdir(parents=True, exist_ok=True)
script.write_text("#!/bin/bash\necho run", encoding="utf-8")
deploy_root = tmp_path / "home"

cmd, scripts = HookIntegrator()._rewrite_command_for_target(
"${CLAUDE_PLUGIN_ROOT}/hooks/run.sh",
pkg_dir,
"my-pkg",
"claude",
deploy_root=deploy_root,
)
assert cmd.replace("\\", "/").endswith(".claude/hooks/my-pkg/hooks/run.sh"), (
f"Command must end with .claude/hooks/my-pkg/hooks/run.sh; got {cmd}"

assert cmd == str((deploy_root / ".claude/hooks/my-pkg/hooks/run.sh").resolve())
assert len(scripts) == 1

def test_rewrite_command_dynamic_root_outside_home_stays_absolute(
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
"""An absolute target root outside HOME is never mislabeled as portable."""
from apm_cli.integration import hook_integrator as hi_mod

monkeypatch.setattr(hi_mod, "_POSIX_USER_HOOK_PATHS", True)
pkg_dir = tmp_path / "pkg"
script = pkg_dir / "hooks" / "run.sh"
script.parent.mkdir(parents=True, exist_ok=True)
script.write_text("#!/bin/bash\necho run", encoding="utf-8")
deploy_root = tmp_path / "home"
dynamic_root = tmp_path / "claude-config"

cmd, scripts = HookIntegrator()._rewrite_command_for_target(
"${CLAUDE_PLUGIN_ROOT}/hooks/run.sh",
pkg_dir,
"my-pkg",
"claude",
root_dir=str(dynamic_root),
deploy_root=deploy_root,
)

assert cmd == str(dynamic_root / "hooks" / "my-pkg" / "hooks" / "run.sh")
assert len(scripts) == 1

def test_rewrite_command_deploy_root_absent_script_resolves_to_source(
self, tmp_path: Path
) -> None:
Expand Down Expand Up @@ -3957,8 +4004,13 @@ def test_rewrite_claude_command_no_deploy_root_uses_project_root(self, tmp_path:
assert cmd == '"${CLAUDE_PROJECT_DIR}/.claude/hooks/my-pkg/hooks/run.sh"'
assert not cmd.startswith("/"), "Command must not be absolute without deploy_root"

def test_rewrite_command_deploy_root_relative_path_handler(self, tmp_path: Path) -> None:
"""deploy_root makes ./path references produce absolute paths too."""
def test_rewrite_command_deploy_root_relative_path_handler(
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
"""User-scope ./path references use the portable home prefix on POSIX."""
from apm_cli.integration import hook_integrator as hi_mod

monkeypatch.setattr(hi_mod, "_POSIX_USER_HOOK_PATHS", True)
pkg_dir = tmp_path / "pkg"
script = pkg_dir / "hooks" / "run.sh"
script.parent.mkdir(parents=True, exist_ok=True)
Expand All @@ -3977,9 +4029,7 @@ def test_rewrite_command_deploy_root_relative_path_handler(self, tmp_path: Path)

assert "./" not in cmd, "Relative ./ reference must be replaced"
assert len(scripts) == 1, "Script copy entry must be produced"
assert cmd.startswith(str(deploy_root.resolve())), (
f"Command must be absolute path under deploy_root; got {cmd}"
)
assert cmd == "$HOME/.claude/hooks/my-pkg/hooks/run.sh"
assert not cmd.startswith("./"), "Command must not be relative"

def test_rewrite_command_nonexistent_script_with_deploy_root(self, tmp_path: Path) -> None:
Expand Down