diff --git a/CHANGELOG.md b/CHANGELOG.md index beb4e53002..b3a1a6802f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `apm install` and `apm update` no longer fail with `Object of type mappingproxy is not JSON serializable` when a hook carries nested metadata such as an `env` object; Claude, Codex, Gemini and Antigravity hook files keep it as written. (#3167) - Codex agent conversion preserves native `model` and `model_reasoning_effort` settings and warns about dropped metadata instead of silently losing it. (#3150) - Cursor hooks use native v1 events and flat handlers; unsupported input and overlapping Claude imports now fail explicitly. Choose one hook route per dependency; see the [supported mappings](docs/src/content/docs/producer/author-primitives/hooks-and-commands.md#cursor-native-hooks-and-claude-import) and `openapm-v0.1.md` requirements `req-tg-016/017`. (#3149) 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 f36d3c4160..cd1af5262a 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 @@ -284,7 +284,10 @@ Copilot hook files are namespaced with the source package name to avoid collisions across installed deps; bundled scripts land alongside under `.github/hooks/scripts//`. -Merged hook files contain only each target's native upstream fields. APM writes +Merged hook files contain only each target's native upstream fields. Other +handler and matcher-group fields, including nested objects such as `env` and +arrays, are copied unchanged into Claude, Codex, Gemini and Antigravity files. +APM writes ownership metadata to a sibling `apm-hooks.json` sidecar for Claude, Cursor, Gemini, Codex, Windsurf, and Antigravity. The sidecar is created and cleaned up automatically alongside the native config; it is an APM implementation detail diff --git a/src/apm_cli/integration/hook_native_formats.py b/src/apm_cli/integration/hook_native_formats.py index 05e931628f..e85a82912b 100644 --- a/src/apm_cli/integration/hook_native_formats.py +++ b/src/apm_cli/integration/hook_native_formats.py @@ -3,6 +3,7 @@ from __future__ import annotations import math +from collections.abc import Mapping from dataclasses import dataclass from typing import Any @@ -348,9 +349,18 @@ def inspect_native_hooks( return tuple(result) +def _thaw(value: Any) -> Any: + """Copy frozen IR metadata into independent JSON-compatible containers.""" + if isinstance(value, Mapping): + return {key: _thaw(item) for key, item in value.items()} + if isinstance(value, (list, tuple)): + return [_thaw(item) for item in value] + return value + + def _handler_from_ir(handler: HookHandler, *, timeout_milliseconds: bool) -> dict[str, Any]: """Render a portable handler into one native command object.""" - result = dict(handler.metadata) + result = _thaw(handler.metadata) if handler.command is not None: result["command"] = handler.command if handler.timeout_seconds is not None: @@ -372,9 +382,9 @@ def _render_nested_document( result: list = [] for binding in document.bindings: if "raw_entry" in binding.metadata: - result.append(binding.metadata["raw_entry"]) + result.append(_thaw(binding.metadata["raw_entry"])) continue - outer = dict(binding.metadata) + outer = _thaw(binding.metadata) if binding.matcher is not None or default_matcher is not None: outer["matcher"] = binding.matcher or default_matcher outer["hooks"] = [ @@ -441,7 +451,7 @@ def _to_antigravity_hook_entries(entries: list, event_name: str) -> list: flat: list[dict[str, Any]] = [] for binding in document.bindings: if "raw_entry" in binding.metadata: - flat.append(binding.metadata["raw_entry"]) + flat.append(_thaw(binding.metadata["raw_entry"])) continue for handler in binding.handlers: rendered = _handler_from_ir(handler, timeout_milliseconds=False) diff --git a/tests/integration/test_hook_nested_metadata_install_e2e.py b/tests/integration/test_hook_nested_metadata_install_e2e.py new file mode 100644 index 0000000000..3475b070fb --- /dev/null +++ b/tests/integration/test_hook_nested_metadata_install_e2e.py @@ -0,0 +1,108 @@ +"""Hermetic ``apm install`` reproduction for #3167: nested hook metadata. + +A dependency whose hook handler carries a nested ``env`` object failed to +integrate with ``Object of type mappingproxy is not JSON serializable``. This +drives the real ``apm install`` CLI with only the download seam stubbed, so +no network access is needed and no hook is executed. +""" + +from __future__ import annotations + +import json +from datetime import datetime +from pathlib import Path +from unittest.mock import patch + +import pytest +import yaml +from click.testing import CliRunner + +from apm_cli.cli import cli +from apm_cli.deps.github_downloader import GitHubPackageDownloader +from apm_cli.models.apm_package import ( + APMPackage, + GitReferenceType, + PackageInfo, + ResolvedReference, + clear_apm_yml_cache, +) +from apm_cli.models.dependency.reference import DependencyReference + +pytestmark = [pytest.mark.integration] + +_ENV = {"APM_TEST": "value", "NESTED": {"LIST": ["a", {"b": 1}]}} +_HANDLER = {"type": "command", "command": "./scripts/check.sh", "env": _ENV} + + +def _download( + _self: GitHubPackageDownloader, + repo_ref: object, + install_path: Path, + *_args: object, + **_kwargs: object, +) -> PackageInfo: + dep_ref = ( + repo_ref + if isinstance(repo_ref, DependencyReference) + else DependencyReference.parse(str(repo_ref)) + ) + install_path = Path(install_path) + hooks_dir = install_path / ".apm" / "hooks" + hooks_dir.mkdir(parents=True, exist_ok=True) + (install_path / "apm.yml").write_text( + yaml.safe_dump({"name": "nested-env", "version": "1.0.0"}), encoding="utf-8" + ) + (hooks_dir / "pre.json").write_text( + json.dumps({"hooks": {"PreToolUse": [{"matcher": "Bash", "hooks": [_HANDLER]}]}}), + encoding="utf-8", + ) + return PackageInfo( + package=APMPackage.from_apm_yml(install_path / "apm.yml"), + install_path=install_path, + installed_at=datetime.now().isoformat(), + dependency_ref=dep_ref, + resolved_reference=ResolvedReference( + original_ref="main", + ref_type=GitReferenceType.BRANCH, + resolved_commit=None, + ref_name="main", + ), + ) + + +def test_install_writes_nested_hook_metadata( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + project = tmp_path / "project" + project.mkdir() + (project / "apm.yml").write_text( + yaml.safe_dump( + { + "name": "nested-env-consumer", + "version": "1.0.0", + "targets": ["claude", "codex"], + "dependencies": {"apm": ["acme/nested-env"]}, + }, + sort_keys=False, + ), + encoding="utf-8", + ) + clear_apm_yml_cache() + monkeypatch.chdir(project) + + with ( + patch("apm_cli.commands._helpers.check_for_updates", return_value=None), + patch.object( + GitHubPackageDownloader, "download_package", autospec=True, side_effect=_download + ), + ): + result = CliRunner().invoke(cli, ["install", "--no-policy"], catch_exceptions=False) + + assert result.exit_code == 0, result.output + assert "mappingproxy" not in result.output + for config in (".claude/settings.json", ".codex/hooks.json"): + document = json.loads((project / config).read_text(encoding="utf-8")) + [entry] = document["hooks"]["PreToolUse"] + assert entry["matcher"] == "Bash", config + [handler] = entry["hooks"] + assert handler["env"] == _ENV, config diff --git a/tests/unit/integration/test_hook_native_formats.py b/tests/unit/integration/test_hook_native_formats.py new file mode 100644 index 0000000000..c481cfb9a7 --- /dev/null +++ b/tests/unit/integration/test_hook_native_formats.py @@ -0,0 +1,159 @@ +"""Native hook rendering materializes frozen IR metadata (#3167).""" + +from __future__ import annotations + +import json +from typing import Any + +import pytest + +from apm_cli.hook_contract import _entries_to_ir +from apm_cli.integration.hook_native_formats import ( + _render_nested_document, + _to_antigravity_hook_entries, + _to_claude_hook_entries, + _to_codex_hook_entries, + _to_gemini_hook_entries, +) + +pytestmark = pytest.mark.unit + +_ENV = {"APM_TEST": "value", "NESTED": {"LEVEL": ["a", {"b": 1}]}} +_ARGS = ["--flag", {"key": [1, 2]}] + + +def _nested_entries() -> list[dict[str, Any]]: + return [ + { + "matcher": "Bash", + "_apm_source": "pkg", + "labels": {"team": ["core"]}, + "hooks": [ + { + "type": "command", + "command": "./scripts/check.sh", + "timeout": 5, + "env": dict(_ENV), + "args": list(_ARGS), + } + ], + } + ] + + +def _flat_entries() -> list[dict[str, Any]]: + return [{"bash": "echo test", "env": {"APM_TEST": "value"}, "args": list(_ARGS)}] + + +def _assert_plain_json(value: object) -> None: + """Every container is a builtin dict or list, so json.dumps needs no default.""" + if isinstance(value, dict): + for item in value.values(): + _assert_plain_json(item) + elif isinstance(value, list): + for item in value: + _assert_plain_json(item) + else: + assert value is None or isinstance(value, (str, int, float, bool)), type(value) + + +def test_issue_reproduction_serializes() -> None: + rendered = _to_claude_hook_entries([{"bash": "echo test", "env": {"APM_TEST": "value"}}]) + + assert json.loads(json.dumps(rendered)) == [ + {"matcher": "*", "hooks": [{"env": {"APM_TEST": "value"}, "command": "echo test"}]} + ] + _assert_plain_json(rendered) + + +@pytest.mark.parametrize( + ("render", "matcher", "timeout"), + [ + (_to_claude_hook_entries, "Bash", 5), + (_to_codex_hook_entries, "Bash", 5), + (_to_gemini_hook_entries, "Bash", 5000), + (lambda entries: _to_antigravity_hook_entries(entries, "PreToolUse"), "Bash", 5), + ], + ids=["claude", "codex", "gemini", "antigravity-nested"], +) +def test_nested_targets_preserve_metadata(render, matcher, timeout) -> None: + rendered = render(_nested_entries()) + + _assert_plain_json(rendered) + assert json.loads(json.dumps(rendered)) == [ + { + "labels": {"team": ["core"]}, + "matcher": matcher, + "hooks": [ + { + "type": "command", + "env": _ENV, + "args": _ARGS, + "command": "./scripts/check.sh", + "timeout": timeout, + } + ], + "_apm_source": "pkg", + } + ] + + +def test_antigravity_flat_preserves_metadata_and_provenance() -> None: + entries = [{"_apm_source": "pkg", **_flat_entries()[0], "timeout": 3}] + rendered = _to_antigravity_hook_entries(entries, "SessionStart") + + _assert_plain_json(rendered) + assert rendered == [ + { + "env": {"APM_TEST": "value"}, + "args": _ARGS, + "command": "echo test", + "timeout": 3, + "_apm_source": "pkg", + } + ] + + +@pytest.mark.parametrize( + "render", + [ + _to_claude_hook_entries, + _to_codex_hook_entries, + _to_gemini_hook_entries, + lambda entries: _to_antigravity_hook_entries(entries, "PreToolUse"), + lambda entries: _to_antigravity_hook_entries(entries, "SessionStart"), + ], + ids=["claude", "codex", "gemini", "antigravity-nested", "antigravity-flat"], +) +def test_raw_entries_are_materialized(render) -> None: + raw = [{"env": {"APM_TEST": "value"}}, ["x", {"y": [1]}]] + + rendered = render([raw]) + + assert rendered == [raw] + _assert_plain_json(rendered) + + +def test_rendered_output_is_independent_of_ir() -> None: + document = _entries_to_ir([*_nested_entries(), [{"env": {"A": "1"}}]]) + + first = _render_nested_document(document, timeout_milliseconds=False) + first[0]["labels"]["team"].append("mutated") + first[0]["hooks"][0]["env"]["NESTED"]["LEVEL"][1]["b"] = 2 + first[0]["hooks"][0]["args"].clear() + first[1][0]["env"]["A"] = "mutated" + + second = _render_nested_document(document, timeout_milliseconds=False) + assert second[0]["labels"] == {"team": ["core"]} + assert second[0]["hooks"][0]["env"] == _ENV + assert second[0]["hooks"][0]["args"] == _ARGS + assert second[1] == [{"env": {"A": "1"}}] + + +def test_rendering_does_not_mutate_source_entries() -> None: + entries = _nested_entries() + rendered = _to_claude_hook_entries(entries) + + rendered[0]["hooks"][0]["env"]["APM_TEST"] = "mutated" + + assert entries == _nested_entries()