From 22ed02d1fc80599c57a4f62a59cd9ea4e781ac14 Mon Sep 17 00:00:00 2001 From: parth Date: Sun, 25 Oct 2026 21:54:24 +0530 Subject: [PATCH] fix(hooks): type untyped flat command handlers for Codex Codex's HookHandlerConfig is an internally tagged enum (#[serde(tag = "type")]) with no default, so the nested handler APM generated from a flat command entry without `type` matched no variant. Share the flat-command type default introduced for Claude in #3153 with the Codex renderer: untyped flat command entries now render as `"type": "command"` handlers in .codex/hooks.json, including when they join existing groups. Explicit types, non-command entries and handlers already nested in the source are unchanged, as are other targets. Ownership matching on reinstall and update now recognises both earlier Codex shapes APM wrote (the original flat entry and the nested untyped group), so proven APM-owned output converges to one typed entry. Manual and unowned hooks are not rewritten or removed. Fixes #3249 --- CHANGELOG.md | 4 + .../author-primitives/hooks-and-commands.md | 11 +- .../skills/apm-usage/package-authoring.md | 4 +- .../integration/hook_native_formats.py | 24 +- .../test_architecture_contract_guards.py | 16 + .../test_required_lifecycle_state_machine.py | 117 ++++++++ .../test_hook_integrator_issue3130.py | 15 +- .../test_hook_integrator_issue3249.py | 281 ++++++++++++++++++ 8 files changed, 452 insertions(+), 20 deletions(-) create mode 100644 tests/unit/integration/test_hook_integrator_issue3249.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 82960dc779..3e19bb3c14 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` and `apm update` now write untyped flat command hooks as Codex `"type": "command"` handlers in `.codex/hooks.json`, as Codex's hook schema requires, including alongside Codex-shaped hook files. Reinstalling replaces older APM-owned untyped entries; explicit types, already-nested source handlers, manual hooks and other targets are unchanged. -- by @Parth-Vasave (closes #3249) + ## [0.35.0] - 2026-10-22 ### 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 9fb6fd96d0..8aecce483e 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,12 +196,13 @@ 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`; a flat command entry that omits `type` receives -Claude's required `"type": "command"`, while explicit handler types are kept -and other targets are unchanged. This default applies only to flat command -entries, not handlers already inside nested Claude groups. For Codex, flat command entries become +`.claude/settings.json`. 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: +`.codex/hooks.json`. For both Claude and Codex, a flat command entry that +omits `type` receives the required `"type": "command"`, while explicit handler +types are kept and other targets are unchanged. This default applies only to +flat command entries, not handlers already inside nested source groups; +supply `type` yourself when authoring nested groups. 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. For Claude, Codex, Gemini, and Antigravity, other handler and hook-group 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 a364715a37..2dda6df3e3 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md +++ b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md @@ -233,7 +233,9 @@ when a flat command entry omits `type`. Explicit handler types are kept. Handlers already inside nested Claude groups and other targets are unchanged. For Codex, APM wraps flat command entries in hook groups containing a -nested `hooks` array in `.codex/hooks.json`. +nested `hooks` array in `.codex/hooks.json` and adds the required +`"type": "command"` when a flat command entry omits `type`. Explicit handler +types are kept. Handlers already inside nested Codex groups are unchanged. Claude, Codex, Gemini, and Antigravity preserve nested handler and hook-group metadata, including `env` objects and arrays, in native JSON. diff --git a/src/apm_cli/integration/hook_native_formats.py b/src/apm_cli/integration/hook_native_formats.py index 1083a44de5..96901639b0 100644 --- a/src/apm_cli/integration/hook_native_formats.py +++ b/src/apm_cli/integration/hook_native_formats.py @@ -416,12 +416,15 @@ def _legacy_hook_entries(entries: list, target: str) -> list: """Render old native shapes for ownership matching, never for deployment.""" if target == "claude": return _to_claude_hook_entries(entries, default_handler_type=False) - # Codex installs before native nesting wrote the original flat entries. - return entries if target == "codex" else [] + if target == "codex": + # Codex installs wrote the original flat entries before native nesting, + # then nested them without typing untyped flat command entries. + return [*entries, *_to_codex_hook_entries(entries, default_handler_type=False)] + return [] -def _with_claude_default_handler_type(entry: object) -> object: - """Supply Claude's required ``type`` for an untyped flat command entry. +def _with_default_command_type(entry: object) -> object: + """Supply the 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 @@ -442,7 +445,7 @@ def _to_claude_hook_entries(entries: list, *, default_handler_type: bool = True) still be matched on reinstall. """ if default_handler_type: - entries = [_with_claude_default_handler_type(entry) for entry in entries] + entries = [_with_default_command_type(entry) for entry in entries] return _render_nested_document( _entries_to_ir(entries), timeout_milliseconds=False, @@ -450,8 +453,15 @@ def _to_claude_hook_entries(entries: list, *, default_handler_type: bool = True) ) -def _to_codex_hook_entries(entries: list) -> list: - """Render portable bindings in Codex's nested hook schema.""" +def _to_codex_hook_entries(entries: list, *, default_handler_type: bool = True) -> list: + """Render portable bindings in Codex's nested hook 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_default_command_type(entry) for entry in entries] return _render_nested_document( _entries_to_ir(entries), timeout_milliseconds=False, diff --git a/tests/integration/test_architecture_contract_guards.py b/tests/integration/test_architecture_contract_guards.py index 8a60f0c39f..66e61aa4e7 100644 --- a/tests/integration/test_architecture_contract_guards.py +++ b/tests/integration/test_architecture_contract_guards.py @@ -173,6 +173,22 @@ def test_codex_native_edge_preserves_nested_entries() -> None: assert _to_codex_hook_entries(source) == source +def test_codex_native_edge_types_untyped_flat_commands() -> None: + """Codex handlers are type-tagged, so flat commands need ``type`` (#3249).""" + from apm_cli.integration.hook_native_formats import ( + _legacy_hook_entries, + _to_codex_hook_entries, + ) + + source = [{"command": "echo check", "matcher": "Bash"}] + untyped = [{"matcher": "Bash", "hooks": [{"command": "echo check"}]}] + + assert _to_codex_hook_entries(source) == [ + {"matcher": "Bash", "hooks": [{"type": "command", "command": "echo check"}]} + ] + assert _legacy_hook_entries(source, "codex") == [*source, *untyped] + + _NESTED_HOOK_METADATA = { "env": {"APM_TEST": "value", "PATHS": ["a", "b"]}, "x-extension": {"tags": [{"name": "t"}], "enabled": True, "weight": 1.5, "note": None}, diff --git a/tests/integration/test_required_lifecycle_state_machine.py b/tests/integration/test_required_lifecycle_state_machine.py index 17308eda5c..b829f9aeba 100644 --- a/tests/integration/test_required_lifecycle_state_machine.py +++ b/tests/integration/test_required_lifecycle_state_machine.py @@ -1938,6 +1938,123 @@ def test_required_claude_untyped_hook_upgrade_preserves_ownership_and_converges( baseline = snapshot +@pytest.mark.parametrize("legacy_shape", ["nested-untyped", "flat"]) +@pytest.mark.parametrize("with_native_group", [False, True], ids=["standalone", "merged"]) +def test_required_codex_untyped_hook_upgrade_preserves_ownership_and_converges( + tmp_path: Path, apm_binary_path: Path, with_native_group: bool, legacy_shape: str +) -> None: + """Real installs and updates heal pre-fix Codex handlers without claiming user hooks.""" + scenario = _new_scenario(tmp_path / "codex-untyped", apm_binary_path) + source = _publish(scenario, "flat-kit", hook_command="echo native") + hook_root = source.repository.worktree / ".apm" / "hooks" + if not with_native_group: + (hook_root / "pretool.json").unlink() + command = "echo must-not-run > hook-executed" + flat = {"command": command, "matcher": "Write", "timeout": 10} + (hook_root / "flat.json").write_text( + json.dumps({"hooks": {"PreToolUse": [flat]}}), encoding="ascii" + ) + scenario.repositories.commit(source.repository, message="publish untyped flat hook") + consumer = scenario.consumers.create( + "codex-consumer", + dependencies=({**source.dependency, "ref": "main"},), + targets=("codex",), + ) + scenario.consumers.add_hook( + consumer, "root", {"hooks": {"PreToolUse": [{"command": "echo root"}]}} + ) + hooks_path = consumer.root / ".codex/hooks.json" + sidecar_path = consumer.root / ".codex/apm-hooks.json" + hooks_path.parent.mkdir() + user_group = {"matcher": "Read", "hooks": [{"type": "prompt"}]} + manual_untyped = {"matcher": "Edit", "hooks": [{"command": "echo manual"}]} + hooks_path.write_text( + json.dumps({"hooks": {"PreToolUse": [user_group, manual_untyped]}}), encoding="ascii" + ) + sentinel = consumer.root / ".claude/settings.json" + sentinel.parent.mkdir() + sentinel.write_bytes(b'{"hooks": {}, "user": "unchanged"}\n') + authored = ArtifactSnapshot.capture(consumer.root / ".apm") + owned_commands = {command, "echo root"} + + _run_success( + scenario, + consumer, + (*_INSTALL_ARGS, "--target", "codex"), + environment=source.environment, + scenario_id="codex-untyped-install", + ) + expected_hooks = json.loads(hooks_path.read_text(encoding="utf-8")) + expected_sidecar = json.loads(sidecar_path.read_text(encoding="utf-8")) + groups = expected_hooks["hooks"]["PreToolUse"] + handlers = [handler for group in groups for handler in group["hooks"]] + assert {"type": "command", "command": command, "timeout": 10} in handlers + assert {"type": "command", "command": "echo root"} in handlers + assert groups[:2] == [user_group, manual_untyped] + assert len(groups) == 4 + int(with_native_group) + assert {entry["_apm_source"] for entry in expected_sidecar["PreToolUse"]} == { + f"{_OWNER}/flat-kit", + "_local/codex-consumer", + } + assert all( + entry["hooks"] not in (user_group["hooks"], manual_untyped["hooks"]) + for entry in expected_sidecar["PreToolUse"] + ) + + def write_legacy_output() -> None: + """Recreate the previous renderer's untyped output and a renamed root source.""" + legacy_hooks = json.loads(hooks_path.read_text(encoding="utf-8")) + legacy_sidecar = json.loads(sidecar_path.read_text(encoding="utf-8")) + for entries in (legacy_hooks["hooks"]["PreToolUse"], legacy_sidecar["PreToolUse"]): + for index, entry in enumerate(entries): + handler = entry["hooks"][0] + if handler.get("command") not in owned_commands: + continue + handler.pop("type") + if legacy_shape == "flat": + group = {key: value for key, value in entry.items() if key != "hooks"} + entries[index] = entry = {**handler, **group} + if entry.get("_apm_source") == "_local/codex-consumer": + entry["_apm_source"] = "old-checkout-name" + hooks_path.write_text(json.dumps(legacy_hooks), encoding="ascii") + sidecar_path.write_text(json.dumps(legacy_sidecar), encoding="ascii") + + def assert_converged() -> None: + assert json.loads(hooks_path.read_text(encoding="utf-8")) == expected_hooks + assert json.loads(sidecar_path.read_text(encoding="utf-8")) == expected_sidecar + assert sentinel.read_bytes() == b'{"hooks": {}, "user": "unchanged"}\n' + assert not (consumer.root / "hook-executed").exists() + assert_unchanged(authored, ArtifactSnapshot.capture(consumer.root / ".apm")) + + write_legacy_output() + baseline = None + for cycle in range(2): + _run_success( + scenario, + consumer, + (*_INSTALL_ARGS, "--target", "codex"), + environment=source.environment, + scenario_id=f"codex-untyped-reinstall-{cycle}", + ) + assert_converged() + snapshot = LifecycleStateSnapshot.capture(consumer.root, targets=("codex",)) + if baseline is not None: + _assert_same_state(baseline, snapshot) + baseline = snapshot + + commit_b = _publish_revision(scenario, source, "b") + write_legacy_output() + _run_success( + scenario, + consumer, + ("update", "--yes", "--parallel-downloads", "0"), + environment=source.environment, + scenario_id="codex-untyped-update", + ) + assert commit_b.sha in (consumer.root / "apm.lock.yaml").read_text(encoding="utf-8") + assert_converged() + + @pytest.mark.parametrize("alias", [".safe", "safe.", "foo..bar", "my-skill.v2"]) def test_required_reinstall_is_byte_idempotent_across_durable_state( tmp_path: Path, diff --git a/tests/unit/integration/test_hook_integrator_issue3130.py b/tests/unit/integration/test_hook_integrator_issue3130.py index cd01979f56..14a8505a5c 100644 --- a/tests/unit/integration/test_hook_integrator_issue3130.py +++ b/tests/unit/integration/test_hook_integrator_issue3130.py @@ -12,7 +12,8 @@ - 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. +- Targets without a required handler ``type`` do not receive the default + (Codex shares it since #3249). - Reinstalling over settings written before the fix leaves one typed group, including content-matched healing of stale root-package entries. """ @@ -199,22 +200,22 @@ def test_untyped_handler_inside_nested_claude_group_is_unchanged(tmp_path: Path) @pytest.mark.parametrize( ("target", "config_dir", "config_file"), [ - pytest.param("codex", ".codex", "hooks.json", id="codex"), pytest.param("cursor", ".cursor", "hooks.json", id="cursor"), + pytest.param("gemini", ".gemini", "settings.json", id="gemini"), ], ) -def test_claude_handler_default_does_not_reach_other_targets( +def test_handler_type_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}) with patch( - "apm_cli.integration.hook_native_formats._with_claude_default_handler_type", - side_effect=AssertionError("Claude handler typing reached another target"), - ) as claude_default: + "apm_cli.integration.hook_native_formats._with_default_command_type", + side_effect=AssertionError("handler typing reached another target"), + ) as default_type: _integrate(tmp_path, package_info, target) - claude_default.assert_not_called() + default_type.assert_not_called() config = _read_json(tmp_path / config_dir / config_file) rendered = json.dumps(config["hooks"]) diff --git a/tests/unit/integration/test_hook_integrator_issue3249.py b/tests/unit/integration/test_hook_integrator_issue3249.py new file mode 100644 index 0000000000..96f7579d5d --- /dev/null +++ b/tests/unit/integration/test_hook_integrator_issue3249.py @@ -0,0 +1,281 @@ +"""Regression tests for #3249 -- untyped flat hook handlers in Codex hooks. + +A flat hook entry may omit ``type`` (valid in Cursor, where it defaults to +``command``). The Codex render wrapped such entries into Codex's +``{"matcher", "hooks": [...]}`` groups but left the handler without the +``type`` discriminator that Codex's ``HookHandlerConfig`` requires. + +These tests assert the post-fix contract: +- Untyped flat command entries render as ``"type": "command"`` handlers in + ``.codex/hooks.json`` and its ownership sidecar, both standalone and when + merged next to a Codex-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 Codex source group are unchanged. +- Reinstalling over APM-owned output from older installs -- the original flat + shape and the nested-untyped shape -- leaves one typed group, including + content-matched healing of stale root-package entries. +- Manual, unowned hooks are neither rewritten nor removed. +""" + +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 #3249 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, + } + ] + }, +} + +_CODEX_SHAPED_HOOK = { + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [{"type": "command", "command": "echo native", "timeout": 5}], + } + ] + } +} + +_DEPLOYED_COMMAND = 'node ".codex/hooks/demo-pkg/.apm/hooks/scripts/gate.mjs"' + +_EXPECTED_FLAT_GROUP = { + "matcher": "Write", + "hooks": [{"type": "command", "command": _DEPLOYED_COMMAND, "timeout": 10}], +} + +# Nested output written by installs before #3249 typed flat command entries. +_LEGACY_NESTED_GROUP = { + "matcher": "Write", + "hooks": [{"command": _DEPLOYED_COMMAND, "timeout": 10}], +} + +# Flat output written by installs before Codex native nesting (#3060). +_LEGACY_FLAT_ENTRY = {"command": _DEPLOYED_COMMAND, "matcher": "Write", "timeout": 10} + + +def _write_package(project_root: Path, hook_files: dict[str, dict]) -> PackageInfo: + """Materialise the #3249 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) -> None: + HookIntegrator().integrate_hooks_for_target( + KNOWN_TARGETS["codex"], + 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 hooks.json.""" + return [{k: v for k, v in entry.items() if k != "_apm_source"} for entry in entries] + + +def _write_existing(codex_dir: Path, hooks: list, owned: list) -> None: + """Seed hooks.json and the ownership sidecar with prior install output.""" + (codex_dir / "hooks.json").write_text( + json.dumps({"hooks": {"PreToolUse": hooks}}), encoding="utf-8" + ) + (codex_dir / "apm-hooks.json").write_text(json.dumps({"PreToolUse": owned}), encoding="utf-8") + + +def test_standalone_flat_untyped_entry_gets_command_type(tmp_path: Path) -> None: + (tmp_path / ".codex").mkdir() + package_info = _write_package(tmp_path, {"flat.json": _FLAT_UNTYPED_HOOK}) + + _integrate(tmp_path, package_info) + + config = _read_json(tmp_path / ".codex" / "hooks.json") + assert config["hooks"] == {"PreToolUse": [_EXPECTED_FLAT_GROUP]} + sidecar = _read_json(tmp_path / ".codex" / "apm-hooks.json") + assert _without_ownership(sidecar["PreToolUse"]) == [_EXPECTED_FLAT_GROUP] + + +def test_flat_untyped_entry_merged_next_to_codex_shaped_file(tmp_path: Path) -> None: + (tmp_path / ".codex").mkdir() + package_info = _write_package( + tmp_path, + {"codex.json": _CODEX_SHAPED_HOOK, "flat.json": _FLAT_UNTYPED_HOOK}, + ) + + _integrate(tmp_path, package_info) + + config = _read_json(tmp_path / ".codex" / "hooks.json") + expected = [_CODEX_SHAPED_HOOK["hooks"]["PreToolUse"][0], _EXPECTED_FLAT_GROUP] + assert config["hooks"] == {"PreToolUse": expected} + sidecar = _read_json(tmp_path / ".codex" / "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( + {"powershell": "Write-Output windows"}, + {"type": "command", "command": "Write-Output windows"}, + id="untyped-powershell-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( + {"type": "mcp_tool", "server": "s", "tool": "t"}, + {"type": "mcp_tool", "server": "s", "tool": "t"}, + id="explicit-mcp-tool", + ), + 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 / ".codex").mkdir() + package_info = _write_package( + tmp_path, {"edge.json": {"hooks": {"PreToolUse": [source_entry]}}} + ) + + _integrate(tmp_path, package_info) + + config = _read_json(tmp_path / ".codex" / "hooks.json") + assert config["hooks"]["PreToolUse"] == [{"hooks": [expected_handler]}] + + +def test_untyped_handler_inside_nested_codex_group_is_unchanged(tmp_path: Path) -> None: + (tmp_path / ".codex").mkdir() + nested = {"matcher": "Edit", "hooks": [{"command": "echo nested"}]} + package_info = _write_package(tmp_path, {"nested.json": {"hooks": {"PostToolUse": [nested]}}}) + + _integrate(tmp_path, package_info) + + config = _read_json(tmp_path / ".codex" / "hooks.json") + assert config["hooks"] == {"PostToolUse": [nested]} + + +@pytest.mark.parametrize( + "legacy_entry", + [ + pytest.param(_LEGACY_NESTED_GROUP, id="nested-untyped"), + pytest.param(_LEGACY_FLAT_ENTRY, id="flat"), + ], +) +def test_reinstall_replaces_owned_output_written_before_fix( + tmp_path: Path, legacy_entry: dict +) -> None: + codex_dir = tmp_path / ".codex" + codex_dir.mkdir() + package_info = _write_package(tmp_path, {"flat.json": _FLAT_UNTYPED_HOOK}) + _write_existing(codex_dir, [legacy_entry], [{**legacy_entry, "_apm_source": _PACKAGE_NAME}]) + + _integrate(tmp_path, package_info) + _integrate(tmp_path, package_info) + + config = _read_json(codex_dir / "hooks.json") + assert config["hooks"] == {"PreToolUse": [_EXPECTED_FLAT_GROUP]} + sidecar = _read_json(codex_dir / "apm-hooks.json") + assert sidecar == {"PreToolUse": [{**_EXPECTED_FLAT_GROUP, "_apm_source": _PACKAGE_NAME}]} + + +def test_unowned_untyped_hook_is_not_rewritten_or_removed(tmp_path: Path) -> None: + """A manual hook identical to old APM output stays when no ownership proves it.""" + codex_dir = tmp_path / ".codex" + codex_dir.mkdir() + package_info = _write_package(tmp_path, {"flat.json": _FLAT_UNTYPED_HOOK}) + manual = {"matcher": "Read", "hooks": [{"command": "echo manual"}]} + _write_existing(codex_dir, [manual, _LEGACY_NESTED_GROUP], []) + + _integrate(tmp_path, package_info) + _integrate(tmp_path, package_info) + + config = _read_json(codex_dir / "hooks.json") + assert config["hooks"] == {"PreToolUse": [manual, _LEGACY_NESTED_GROUP, _EXPECTED_FLAT_GROUP]} + sidecar = _read_json(codex_dir / "apm-hooks.json") + assert sidecar == {"PreToolUse": [{**_EXPECTED_FLAT_GROUP, "_apm_source": _PACKAGE_NAME}]} + + +@pytest.mark.parametrize( + "legacy_entry", + [ + pytest.param( + {"matcher": "Write", "hooks": [{"command": "echo root"}]}, id="nested-untyped" + ), + pytest.param({"command": "echo root", "matcher": "Write"}, id="flat"), + ], +) +def test_stale_root_source_output_is_still_healed(tmp_path: Path, legacy_entry: dict) -> None: + """Content-matched healing must still recognise output 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", + ) + codex_dir = tmp_path / ".codex" + codex_dir.mkdir() + _write_existing(codex_dir, [legacy_entry], [{**legacy_entry, "_apm_source": "old-root-name"}]) + root_package = PackageInfo( + package=APMPackage(name="consumer", version="0.0.1"), + install_path=tmp_path, + ) + + _integrate(tmp_path, root_package) + + config = _read_json(codex_dir / "hooks.json") + assert config["hooks"] == { + "PreToolUse": [{"matcher": "Write", "hooks": [{"type": "command", "command": "echo root"}]}] + } + sidecar = _read_json(codex_dir / "apm-hooks.json") + assert sidecar == { + "PreToolUse": [{**config["hooks"]["PreToolUse"][0], "_apm_source": "_local/consumer"}] + }