diff --git a/CHANGELOG.md b/CHANGELOG.md index 674411a76..5cbc889b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,9 +18,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `apm install --target intellij` now configures JetBrains Copilot MCP support while routing package file primitives through the Copilot profile. (by @sergio-sisternes-epam; closes #1957) (#2041) - -### Fixed - +- Flat hook manifests now work with Claude: `apm install --target claude` + generates the required `matcher` and `hooks` nesting while preserving other + settings, so one source manifest targets both Copilot and Claude. + (closes #2062) (#2097) +- `apm install host/org/repo/subpath#ref` on an unrecognised self-hosted FQDN + no longer fails with a misleading "not accessible or doesn't exist" error; + the failure reason now suggests setting `GITLAB_HOST` / `APM_GITLAB_HOSTS` + if the target is a self-hosted GitLab instance, or using an explicit + `git:` + `path:` entry in `apm.yml` otherwise. (by @rrazvd; closes #2066) (#2074) - Azure DevOps marketplace checks now preserve suffix-free `/_git/` URLs and pass Azure CLI bearer authentication through to `git ls-remote`. (closes #2119) diff --git a/docs/src/content/docs/producer/author-primitives/hooks-and-commands.md b/docs/src/content/docs/producer/author-primitives/hooks-and-commands.md index 26f76d86f..92f3a5d7d 100644 --- a/docs/src/content/docs/producer/author-primitives/hooks-and-commands.md +++ b/docs/src/content/docs/producer/author-primitives/hooks-and-commands.md @@ -40,13 +40,32 @@ your-package/ Each file is a JSON document keyed by lifecycle event. APM accepts the Claude (`PreToolUse`, `PostToolUse`) and Copilot (`preToolUse`, -`postToolUse`) shapes; events are renamed per target during merge. +`postToolUse`) shapes; events are renamed per target during merge. Flat +Copilot hook entries (entries without an outer `matcher` and `hooks` group) +are also portable: Copilot consumes the flat entry as-is, while for Claude +APM wraps each one in Claude's required group. The generated `matcher: "*"` +preserves the flat entry's global scope by matching all tools. + +**Portable source (Copilot and Claude):** + +```json +{ + "hooks": { + "PreToolUse": [ + {"type": "command", "command": "${PLUGIN_ROOT}/scripts/validate.sh", "timeout": 10} + ] + } +} +``` + +**Claude output (generated by APM):** ```json { "hooks": { "PreToolUse": [ { + "matcher": "*", "hooks": [ {"type": "command", "command": "${PLUGIN_ROOT}/scripts/validate.sh", "timeout": 10} ] @@ -73,7 +92,7 @@ straight from there works as a standalone APM hook file: } ``` -Both shapes are normalized internally before merge. A file whose `"hooks"` +Flat and nested shapes are normalized per target before merge. A file whose `"hooks"` key is present but not a JSON object fails closed with a warning; a file that parses cleanly but contributes zero entries also logs a warning so authors notice empty merges during development. diff --git a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md index 4272dc7b2..5c6cedd6f 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md +++ b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md @@ -131,6 +131,24 @@ becomes `PostToolUse` in Claude) and rewrites path variables the correct target-specific form. Kiro materializes one JSON document per hook action under `.kiro/hooks/`. +Authors may use flat Copilot hook entries for portable manifests: + +```json +{ + "hooks": { + "PreToolUse": [ + {"type": "command", "command": "${PLUGIN_ROOT}/hooks/check.sh", "timeout": 15} + ] + } +} +``` + +When deploying to Claude, APM wraps each flat entry in the required +`{"matcher": "*", "hooks": [...]}` group, where `"*"` matches all tools and +preserves the flat entry's global scope. Copilot keeps the flat entry, so one +source manifest is valid for both targets. Already nested Claude entries remain +nested. + When a hook command references a script inside `hooks/` or `.apm/hooks/`, APM deploys that hook source bundle so sibling helper files resolve at runtime. Claude-family merged targets (Claude, Cursor, Codex, Gemini, diff --git a/src/apm_cli/integration/hook_integrator.py b/src/apm_cli/integration/hook_integrator.py index 412a60f74..4328699d6 100644 --- a/src/apm_cli/integration/hook_integrator.py +++ b/src/apm_cli/integration/hook_integrator.py @@ -243,13 +243,14 @@ def _emit_hook_event_diagnostics( ) -def _to_nested_hook_entries(entries: list, key_fixer) -> list: +def _to_nested_hook_entries(entries: list, key_fixer, default_matcher: str | None = None) -> list: """Wrap flat Copilot hook entries in the ``{"hooks": [...]}`` nesting. - Shared by the Gemini and Antigravity transforms (both use the Claude + Shared by the Claude, Gemini, and Antigravity transforms (all use the nested matcher shape for tool events). *key_fixer* renames the inner - command/timeout keys in place for the specific target. Entries already - in nested form have only their inner keys fixed. + command/timeout keys in place for the specific target. *default_matcher* + is added to newly wrapped entries when the target requires one. Entries + already in nested form have only their inner keys fixed. """ result = [] for entry in entries: @@ -267,12 +268,23 @@ def _to_nested_hook_entries(entries: list, key_fixer) -> list: key_fixer(inner) apm_source = inner.pop("_apm_source", None) outer: dict = {"hooks": [inner]} + if default_matcher is not None: + outer["matcher"] = default_matcher if apm_source: outer["_apm_source"] = apm_source result.append(outer) return result +def _preserve_claude_hook_keys(_hook: dict) -> None: + """Keep Claude hook handler keys unchanged.""" + + +def _to_claude_hook_entries(entries: list) -> list: + """Wrap globally scoped flat entries in Claude matcher groups.""" + return _to_nested_hook_entries(entries, _preserve_claude_hook_keys, default_matcher="*") + + def _to_gemini_hook_entries(entries: list) -> list: """Transform hook entries into Gemini CLI format. @@ -1504,7 +1516,9 @@ def _integrate_merged_hooks( # Transform flat Copilot entries to the target's nested / # native hook shape. - if config.target_key == "gemini": + if config.target_key == "claude": + entries = _to_claude_hook_entries(entries) + elif config.target_key == "gemini": entries = _to_gemini_hook_entries(entries) elif config.target_key == "antigravity": entries = _to_antigravity_hook_entries(entries, event_name) diff --git a/tests/integration/test_claude_flat_hook_install_e2e.py b/tests/integration/test_claude_flat_hook_install_e2e.py new file mode 100644 index 000000000..f7232b45d --- /dev/null +++ b/tests/integration/test_claude_flat_hook_install_e2e.py @@ -0,0 +1,80 @@ +"""End-to-end coverage for Claude flat hook normalization (#2062).""" + +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +CLI = [sys.executable, "-m", "apm_cli.cli"] +TIMEOUT = 180 +pytestmark = pytest.mark.integration + + +def _run(cwd: Path, *args: str) -> subprocess.CompletedProcess[str]: + """Run the APM CLI in *cwd* and return the completed subprocess.""" + return subprocess.run( + CLI + list(args), + cwd=str(cwd), + capture_output=True, + text=True, + timeout=TIMEOUT, + check=False, + ) + + +def test_install_claude_nests_flat_hooks_and_preserves_settings(tmp_path: Path) -> None: + """The real install path writes valid Claude groups without replacing settings.""" + workspace = tmp_path / "workspace" + package = workspace / "flat-hooks-package" + consumer = workspace / "consumer" + hooks_dir = package / ".apm" / "hooks" + hooks_dir.mkdir(parents=True) + consumer.joinpath(".claude").mkdir(parents=True) + + package.joinpath("apm.yml").write_text( + "name: flat-hooks-package\nversion: 1.0.0\n", + encoding="utf-8", + ) + hooks_dir.joinpath("test.json").write_text( + json.dumps({"hooks": {"PreToolUse": [{"type": "command", "command": "echo installed"}]}}), + encoding="utf-8", + ) + consumer.joinpath("apm.yml").write_text( + """name: flat-hooks-consumer +version: 1.0.0 +dependencies: + apm: + - path: ../flat-hooks-package +""", + encoding="utf-8", + ) + existing_group = { + "matcher": "Bash", + "hooks": [{"type": "command", "command": "echo existing"}], + } + consumer.joinpath(".claude", "settings.json").write_text( + json.dumps( + { + "permissions": {"allow": ["Read"]}, + "hooks": {"PreToolUse": [existing_group]}, + } + ), + encoding="utf-8", + ) + + result = _run(consumer, "install", "--target", "claude") + + assert result.returncode == 0, f"stdout:\n{result.stdout}\nstderr:\n{result.stderr}" + settings = json.loads(consumer.joinpath(".claude", "settings.json").read_text(encoding="utf-8")) + sidecar = json.loads(consumer.joinpath(".claude", "apm-hooks.json").read_text(encoding="utf-8")) + installed_group = { + "matcher": "*", + "hooks": [{"type": "command", "command": "echo installed"}], + } + assert settings["permissions"] == {"allow": ["Read"]} + assert settings["hooks"]["PreToolUse"] == [existing_group, installed_group] + assert sidecar["PreToolUse"] == [{**installed_group, "_apm_source": "flat-hooks-package"}] diff --git a/tests/unit/integration/test_hook_integrator_issue2062.py b/tests/unit/integration/test_hook_integrator_issue2062.py new file mode 100644 index 000000000..6b47a76c2 --- /dev/null +++ b/tests/unit/integration/test_hook_integrator_issue2062.py @@ -0,0 +1,48 @@ +"""Regression coverage for Claude flat hook normalization (#2062).""" + +import json +from pathlib import Path + +from apm_cli.integration.hook_integrator import HookIntegrator, _to_claude_hook_entries +from apm_cli.models.apm_package import APMPackage, PackageInfo + + +def test_claude_wraps_flat_hook_entries_in_settings_and_sidecar(tmp_path: Path) -> None: + """Claude receives matcher groups while ownership tracks the same shape.""" + package_root = tmp_path / "package" + hooks_dir = package_root / ".apm" / "hooks" + hooks_dir.mkdir(parents=True) + (hooks_dir / "test.json").write_text( + json.dumps( + {"hooks": {"PreToolUse": [{"type": "command", "command": "echo hi", "timeout": 15}]}} + ), + encoding="utf-8", + ) + package = PackageInfo( + package=APMPackage(name="hook-repro-pkg", version="1.0.0"), + install_path=package_root, + ) + project_root = tmp_path / "consumer" + (project_root / ".claude").mkdir(parents=True) + + result = HookIntegrator().integrate_package_hooks_claude(package, project_root) + + expected_entry = { + "matcher": "*", + "hooks": [{"type": "command", "command": "echo hi", "timeout": 15}], + } + settings = json.loads((project_root / ".claude" / "settings.json").read_text(encoding="utf-8")) + sidecar = json.loads((project_root / ".claude" / "apm-hooks.json").read_text(encoding="utf-8")) + assert result.files_integrated == 1 + assert settings["hooks"]["PreToolUse"] == [expected_entry] + assert sidecar["PreToolUse"] == [{**expected_entry, "_apm_source": "package"}] + + +def test_claude_preserves_already_nested_hook_entries() -> None: + """Claude-native matcher groups retain their authored matcher.""" + nested = { + "matcher": "Bash", + "hooks": [{"type": "command", "command": "echo nested"}], + } + + assert _to_claude_hook_entries([nested]) == [nested]