From cecf16bec77fd4b49e3786210ec3b1e3d7409b9f Mon Sep 17 00:00:00 2001 From: parth Date: Fri, 2 Oct 2026 22:42:20 +0530 Subject: [PATCH 1/2] fix(hooks): type untyped flat command handlers for Claude Flat hook command entries that omit `type` (valid in Cursor, where it defaults to `command`) were wrapped into Claude's matcher groups without the handler `type` field that Claude's hook schema requires. The Claude renderer now supplies `"type": "command"` for flat entries that the neutral hook grammar reads as command handlers. Explicit handler types, untyped handlers already inside nested Claude groups, non-command entries, and all other targets are unchanged. Reinstall cleanup also matches the pre-fix untyped render, so stale root-package entries keep healing instead of duplicating (the #1329 / #1392 contract), mirroring the existing Codex legacy-content-key path. Refs #3130 apm-spec-waiver: Claude-native handler type default for #3130; OpenAPM v0.1 defines no target-native hook handler fields, the portable hook subset is deferred to #2111 (out of approved scope), and req-lk-021 ownership reconciliation is preserved. --- CHANGELOG.md | 4 + .../author-primitives/hooks-and-commands.md | 7 +- .../skills/apm-usage/package-authoring.md | 4 + src/apm_cli/integration/hook_integrator.py | 6 + .../integration/hook_native_formats.py | 25 +- .../test_hook_integrator_issue3130.py | 261 ++++++++++++++++++ 6 files changed, 303 insertions(+), 4 deletions(-) create mode 100644 tests/unit/integration/test_hook_integrator_issue3130.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c1a995f25..d954b38b10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- `apm install` now writes flat hook command entries that omit `type` to `.claude/settings.json` as schema-valid `"type": "command"` handlers, including when merged next to a Claude-shaped hook file. Explicit handler types and other targets are unchanged; reinstall to refresh existing entries. (by @Parth-Vasave, #3130) + ## [0.33.0] - 2026-10-02 ### Added 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 7265b8167d..691a986923 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 @@ -196,8 +196,11 @@ Supported targets and where the integrator writes: APM parses the source into vendor-neutral hook intent, then each target integrator renders its native schema. Flat command entries become Claude's required `{ "matcher": "*", "hooks": [...] }` entries in -`.claude/settings.json`. For Codex, flat command entries become intermediate -hook groups containing a nested `hooks` array in `.codex/hooks.json`. Kiro +`.claude/settings.json`; a flat command entry that omits `type` receives +Claude's required `"type": "command"`, while explicit handler types are kept +and other targets receive no default. For Codex, flat command entries become +intermediate hook groups containing a nested `hooks` array in +`.codex/hooks.json`. Kiro receives its current v1 standalone schema: `{ "version": "v1", "hooks": [{ "name", "trigger", "matcher", "action" }] }`. Kiro trigger names are PascalCase and command timeouts remain in seconds. 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 ba6026039f..af9a68cd23 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md +++ b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md @@ -181,6 +181,10 @@ becomes `PostToolUse` in Claude) and rewrites path variables the correct target-specific form. Kiro materializes one JSON document per hook action under `.kiro/hooks/`. +For Claude, APM wraps flat command entries in `{ "matcher", "hooks": [...] }` +groups in `.claude/settings.json` and adds the required `"type": "command"` +when a flat command entry omits `type`. Explicit handler types are kept. + For Codex, APM wraps flat command entries in hook groups containing a nested `hooks` array in `.codex/hooks.json`. diff --git a/src/apm_cli/integration/hook_integrator.py b/src/apm_cli/integration/hook_integrator.py index c1606db175..e0b0c63a53 100644 --- a/src/apm_cli/integration/hook_integrator.py +++ b/src/apm_cli/integration/hook_integrator.py @@ -1353,6 +1353,12 @@ def _integrate_merged_hooks( json_config[container][event_name] = [] legacy_content_keys: set[str] = set() if config.target_key == "claude": + # Match owned untyped groups from installs before handler typing. + legacy_content_keys = { + self._hook_entry_content_key(entry) + for entry in _to_claude_hook_entries(entries, default_handler_type=False) + if isinstance(entry, dict) + } entries = _to_claude_hook_entries(entries) elif config.target_key == "codex": # Match owned flat entries from installs before Codex nesting. diff --git a/src/apm_cli/integration/hook_native_formats.py b/src/apm_cli/integration/hook_native_formats.py index 994c12fa7c..84b4f0d3e7 100644 --- a/src/apm_cli/integration/hook_native_formats.py +++ b/src/apm_cli/integration/hook_native_formats.py @@ -221,8 +221,29 @@ def _to_gemini_hook_entries(entries: list) -> list: ) -def _to_claude_hook_entries(entries: list) -> list: - """Render portable bindings in Claude's nested matcher schema.""" +def _with_claude_default_handler_type(entry: object) -> object: + """Supply Claude's required ``type`` for an untyped flat command entry. + + Only flat entries that the neutral grammar reads as a command handler are + defaulted; explicit types, nested groups, and non-command entries pass + through unchanged. + """ + if not isinstance(entry, dict) or "type" in entry or isinstance(entry.get("hooks"), list): + return entry + if not isinstance(_handler_to_ir(entry, None).command, str): + return entry + return {"type": "command", **entry} + + +def _to_claude_hook_entries(entries: list, *, default_handler_type: bool = True) -> list: + """Render portable bindings in Claude's nested matcher schema. + + ``default_handler_type=False`` reproduces the output of installs before + untyped flat command entries were typed, so owned entries they wrote can + still be matched on reinstall. + """ + if default_handler_type: + entries = [_with_claude_default_handler_type(entry) for entry in entries] return _render_nested_document( _entries_to_ir(entries), timeout_milliseconds=False, diff --git a/tests/unit/integration/test_hook_integrator_issue3130.py b/tests/unit/integration/test_hook_integrator_issue3130.py new file mode 100644 index 0000000000..5795d85964 --- /dev/null +++ b/tests/unit/integration/test_hook_integrator_issue3130.py @@ -0,0 +1,261 @@ +"""Regression tests for #3130 -- untyped flat hook handlers in Claude settings. + +A flat hook entry may omit ``type`` (valid in Cursor, where it defaults to +``command``). The Claude render wrapped such entries into Claude's +``{"matcher", "hooks": [...]}`` groups but left the handler without the +``type`` field that Claude's hook schema requires. + +These tests assert the post-fix contract: +- Untyped flat command entries render as ``"type": "command"`` handlers in + ``.claude/settings.json`` and its ownership sidecar, both standalone and + when merged next to a Claude-shaped hook file from the same package. +- Explicit handler types and untyped entries that are not command handlers + are preserved unchanged. +- Untyped handlers already inside a nested Claude group are left unchanged. +- Other targets do not receive the Claude-only default. +- Reinstalling over settings written before the fix leaves one typed group, + including content-matched healing of stale root-package entries. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from apm_cli.integration.hook_integrator import HookIntegrator +from apm_cli.integration.targets import KNOWN_TARGETS +from apm_cli.models.apm_package import APMPackage, PackageInfo + +pytestmark = pytest.mark.component + +_PACKAGE_NAME = "demo-pkg" + +# Exact flat hook file from the #3130 report: no ``type`` on the handler. +_FLAT_UNTYPED_HOOK = { + "version": 1, + "hooks": { + "preToolUse": [ + { + "command": 'node "${PLUGIN_ROOT}/.apm/hooks/scripts/gate.mjs"', + "matcher": "Write", + "timeout": 10, + } + ] + }, +} + +_CLAUDE_SHAPED_HOOK = { + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [{"type": "command", "command": "echo native", "timeout": 5}], + } + ] + } +} + +_EXPECTED_FLAT_GROUP = { + "matcher": "Write", + "hooks": [ + { + "type": "command", + "command": ( + 'node "${CLAUDE_PROJECT_DIR}/.claude/hooks/demo-pkg/.apm/hooks/scripts/gate.mjs"' + ), + "timeout": 10, + } + ], +} + + +def _write_package(project_root: Path, hook_files: dict[str, dict]) -> PackageInfo: + """Materialise the #3130 package layout with the given hook files.""" + package_root = project_root / "apm_modules" / "owner" / _PACKAGE_NAME + hooks_dir = package_root / ".apm" / "hooks" + (hooks_dir / "scripts").mkdir(parents=True) + (hooks_dir / "scripts" / "gate.mjs").write_text("process.exit(0);\n", encoding="utf-8") + for name, document in hook_files.items(): + (hooks_dir / name).write_text(json.dumps(document), encoding="utf-8") + return PackageInfo( + package=APMPackage(name=_PACKAGE_NAME, version="0.0.1"), + install_path=package_root, + ) + + +def _integrate(project_root: Path, package_info: PackageInfo, target: str) -> None: + HookIntegrator().integrate_hooks_for_target( + KNOWN_TARGETS[target], + package_info, + project_root, + ) + + +def _read_json(path: Path) -> dict: + return json.loads(path.read_text(encoding="utf-8")) + + +def _without_ownership(entries: list) -> list: + """Drop APM ownership markers so sidecar entries compare to settings.""" + return [{k: v for k, v in entry.items() if k != "_apm_source"} for entry in entries] + + +def test_standalone_flat_untyped_entry_gets_command_type(tmp_path: Path) -> None: + (tmp_path / ".claude").mkdir() + package_info = _write_package(tmp_path, {"flat.json": _FLAT_UNTYPED_HOOK}) + + _integrate(tmp_path, package_info, "claude") + + settings = _read_json(tmp_path / ".claude" / "settings.json") + assert settings["hooks"] == {"PreToolUse": [_EXPECTED_FLAT_GROUP]} + sidecar = _read_json(tmp_path / ".claude" / "apm-hooks.json") + assert _without_ownership(sidecar["PreToolUse"]) == [_EXPECTED_FLAT_GROUP] + + +def test_flat_untyped_entry_merged_next_to_claude_shaped_file(tmp_path: Path) -> None: + (tmp_path / ".claude").mkdir() + package_info = _write_package( + tmp_path, + {"claude.json": _CLAUDE_SHAPED_HOOK, "flat.json": _FLAT_UNTYPED_HOOK}, + ) + + _integrate(tmp_path, package_info, "claude") + + settings = _read_json(tmp_path / ".claude" / "settings.json") + expected = [_CLAUDE_SHAPED_HOOK["hooks"]["PreToolUse"][0], _EXPECTED_FLAT_GROUP] + assert settings["hooks"] == {"PreToolUse": expected} + sidecar = _read_json(tmp_path / ".claude" / "apm-hooks.json") + assert _without_ownership(sidecar["PreToolUse"]) == expected + + +@pytest.mark.parametrize( + ("source_entry", "expected_handler"), + [ + pytest.param( + {"bash": "echo posix"}, + {"type": "command", "command": "echo posix"}, + id="untyped-bash-command", + ), + pytest.param( + {"type": "command", "command": "echo typed"}, + {"type": "command", "command": "echo typed"}, + id="explicit-command", + ), + pytest.param( + {"type": "prompt", "prompt": "Review the change"}, + {"type": "prompt", "prompt": "Review the change"}, + id="explicit-prompt", + ), + pytest.param( + {"foo": 1}, + {"foo": 1}, + id="untyped-without-command", + ), + ], +) +def test_flat_handler_types_are_defaulted_only_for_commands( + tmp_path: Path, source_entry: dict, expected_handler: dict +) -> None: + (tmp_path / ".claude").mkdir() + package_info = _write_package( + tmp_path, {"edge.json": {"hooks": {"PreToolUse": [source_entry]}}} + ) + + _integrate(tmp_path, package_info, "claude") + + settings = _read_json(tmp_path / ".claude" / "settings.json") + assert settings["hooks"]["PreToolUse"] == [{"matcher": "*", "hooks": [expected_handler]}] + + +def test_untyped_handler_inside_nested_claude_group_is_unchanged(tmp_path: Path) -> None: + (tmp_path / ".claude").mkdir() + nested = {"matcher": "Edit", "hooks": [{"command": "echo nested"}]} + package_info = _write_package(tmp_path, {"nested.json": {"hooks": {"PostToolUse": [nested]}}}) + + _integrate(tmp_path, package_info, "claude") + + settings = _read_json(tmp_path / ".claude" / "settings.json") + assert settings["hooks"] == {"PostToolUse": [nested]} + + +@pytest.mark.parametrize( + ("target", "config_dir", "config_file"), + [ + pytest.param("codex", ".codex", "hooks.json", id="codex"), + pytest.param("cursor", ".cursor", "hooks.json", id="cursor"), + ], +) +def test_claude_handler_default_does_not_reach_other_targets( + tmp_path: Path, target: str, config_dir: str, config_file: str +) -> None: + (tmp_path / config_dir).mkdir() + package_info = _write_package(tmp_path, {"flat.json": _FLAT_UNTYPED_HOOK}) + + _integrate(tmp_path, package_info, target) + + config = _read_json(tmp_path / config_dir / config_file) + rendered = json.dumps(config["hooks"]) + assert '"type"' not in rendered + assert "gate.mjs" in rendered + + +def test_reinstall_replaces_untyped_group_written_before_fix(tmp_path: Path) -> None: + claude_dir = tmp_path / ".claude" + claude_dir.mkdir() + package_info = _write_package(tmp_path, {"flat.json": _FLAT_UNTYPED_HOOK}) + legacy_group = { + "matcher": "Write", + "hooks": [ + { + "command": _EXPECTED_FLAT_GROUP["hooks"][0]["command"], + "timeout": 10, + } + ], + } + (claude_dir / "settings.json").write_text( + json.dumps({"hooks": {"PreToolUse": [legacy_group]}}), encoding="utf-8" + ) + (claude_dir / "apm-hooks.json").write_text( + json.dumps({"PreToolUse": [{**legacy_group, "_apm_source": _PACKAGE_NAME}]}), + encoding="utf-8", + ) + + _integrate(tmp_path, package_info, "claude") + _integrate(tmp_path, package_info, "claude") + + settings = _read_json(claude_dir / "settings.json") + assert settings["hooks"] == {"PreToolUse": [_EXPECTED_FLAT_GROUP]} + + +def test_stale_root_source_untyped_group_is_still_healed(tmp_path: Path) -> None: + """Content-matched healing must still recognise groups written before the fix.""" + (tmp_path / "apm.yml").write_text("name: consumer\nversion: 0.0.1\n", encoding="utf-8") + hooks_dir = tmp_path / ".apm" / "hooks" + hooks_dir.mkdir(parents=True) + (hooks_dir / "flat.json").write_text( + json.dumps({"hooks": {"PreToolUse": [{"command": "echo root", "matcher": "Write"}]}}), + encoding="utf-8", + ) + claude_dir = tmp_path / ".claude" + claude_dir.mkdir() + legacy_group = {"matcher": "Write", "hooks": [{"command": "echo root"}]} + (claude_dir / "settings.json").write_text( + json.dumps({"hooks": {"PreToolUse": [legacy_group]}}), encoding="utf-8" + ) + (claude_dir / "apm-hooks.json").write_text( + json.dumps({"PreToolUse": [{**legacy_group, "_apm_source": "old-root-name"}]}), + encoding="utf-8", + ) + root_package = PackageInfo( + package=APMPackage(name="consumer", version="0.0.1"), + install_path=tmp_path, + ) + + _integrate(tmp_path, root_package, "claude") + + settings = _read_json(claude_dir / "settings.json") + assert settings["hooks"] == { + "PreToolUse": [{"matcher": "Write", "hooks": [{"type": "command", "command": "echo root"}]}] + } From 3be1ab0586f98fe3e51e95f7648588b05444ff40 Mon Sep 17 00:00:00 2001 From: parth Date: Fri, 2 Oct 2026 22:51:29 +0530 Subject: [PATCH 2/2] docs(changelog): cite #3153 in the #3130 hook entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d954b38b10..bd935830b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- `apm install` now writes flat hook command entries that omit `type` to `.claude/settings.json` as schema-valid `"type": "command"` handlers, including when merged next to a Claude-shaped hook file. Explicit handler types and other targets are unchanged; reinstall to refresh existing entries. (by @Parth-Vasave, #3130) +- `apm install` now writes flat hook command entries that omit `type` to `.claude/settings.json` as schema-valid `"type": "command"` handlers, including when merged next to a Claude-shaped hook file. Explicit handler types and other targets are unchanged; reinstall to refresh existing entries. (by @Parth-Vasave, closes #3130, #3153) ## [0.33.0] - 2026-10-02