Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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, single-quoted references, and dynamic config roots outside the home directory keep absolute paths. (closes #2821) (#2944)

### 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,19 @@ 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 so do single-quoted references (a
shell does not expand `$HOME` inside single quotes) and a dynamic target
root outside the home directory (for example `CLAUDE_CONFIG_DIR`).
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 and for single-quoted references.
- **Global compile.** `apm compile -g` can also render global instructions to
`~/.copilot/AGENTS.md` for root-context readers that honor `AGENTS.md`.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,13 @@ merged target keeps APM reconciliation ownership in a sibling `apm-hooks.json`
sidecar, so clones, contributors, and CI runners do not see the installer's
machine-local absolute prefix. `apm install -g` (user-scope, e.g.
`~/.claude/settings.json`) rewrites `${PLUGIN_ROOT}` and relative `./`
references to absolute paths because the user-scope config is read
without a fixed cwd. If a manifest in `hooks/` or `.apm/hooks/` uses
references so the user-scope config resolves without a fixed cwd: on POSIX
hosts the rewritten path is anchored to `$HOME` (for example
`$HOME/.claude/hooks/<pkg>/run.sh`), which the invoking shell expands, so the
merged file stays identical across machines. Windows keeps the absolute form,
and so do single-quoted references (a shell does not expand `$HOME` inside
single quotes) and dynamic target roots outside the home directory such as
`CLAUDE_CONFIG_DIR`. If a manifest in `hooks/` or `.apm/hooks/` uses
`./hooks/<script>`, APM first resolves it from the hook file directory,
then falls back to the package root to avoid deploying a doubled
`hooks/hooks/` path. If a referenced hook script is missing at install
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
55 changes: 40 additions & 15 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,11 +113,22 @@

_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


def _wrapping_quote(command: str, match: re.Match[str]) -> str:
"""Return the quote character wrapping a reference, or "" when it has none."""
if match.start() > 0 and match.end() < len(command):
quote = command[match.start() - 1]
if quote in "\"'" and command[match.end()] == quote:
return quote
return ""


# DEPRECATED -- use IntegrationResult directly for new code.
# Backward-compatible shim: accepts hooks_integrated= kwarg and
# exposes a hooks_integrated property for consumers of the old API.
Expand Down Expand Up @@ -588,11 +600,30 @@ def _project_scoped_command_path(
target_rel: str,
deploy_root: Path | None,
source_key: str | None = None,
path_is_quoted: bool = False,
path_quote: str = "",
) -> 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()
# A shell expands $HOME outside single quotes only, so a
# single-quoted reference keeps the previous absolute form instead
# of becoming a literal path the target would never expand.
if _POSIX_USER_HOOK_PATHS and path_quote != "'":
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 All @@ -605,7 +636,7 @@ def _project_scoped_command_path(
):
return f"$env:{project_dir}/{target_rel}"
path = f"${{{project_dir}}}/{target_rel}"
return path if path_is_quoted else f'"{path}"'
return path if path_quote else f'"{path}"'

def _rewrite_command_for_target(
self,
Expand Down Expand Up @@ -663,10 +694,7 @@ def _rewrite_command_for_target(
target_rel,
deploy_root,
source_key,
match.start() > 0
and match.end() < len(command)
and command[match.start() - 1] in "\"'"
and command[match.end()] == command[match.start() - 1],
_wrapping_quote(command, match),
)
new_command = new_command.replace(full_var, resolved_cmd)
else:
Expand Down Expand Up @@ -716,10 +744,7 @@ def _rewrite_command_for_target(
target_rel,
deploy_root,
source_key,
match.start() > 0
and match.end() < len(command)
and command[match.start() - 1] in "\"'"
and command[match.end()] == command[match.start() - 1],
_wrapping_quote(command, match),
)
new_command = new_command.replace(rel_ref, resolved_cmd)
else:
Expand Down Expand Up @@ -757,7 +782,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 +1051,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 +1700,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
Loading