From b36ed0ff2a60ee1974172d4d40bf21760ada7f7a Mon Sep 17 00:00:00 2001 From: Mohammed Alkindi Date: Fri, 9 Oct 2026 10:45:44 -0700 Subject: [PATCH] fix(hooks): serialize nested hook metadata in native hook files The hook IR freezes dicts into MappingProxyType and lists into tuples. Claude, Codex, Gemini and Antigravity rendering copied handler and binding metadata with a shallow dict() and appended raw entries as-is, so a nested value such as an env object reached json.dumps as a mappingproxy and install/update failed. Render through a recursive _thaw() that rebuilds plain dicts and lists, so output is JSON-serializable and independent of the frozen IR. Fixes #3167 --- CHANGELOG.md | 1 + .../author-primitives/hooks-and-commands.md | 5 +- .../integration/hook_native_formats.py | 18 +- .../test_hook_nested_metadata_install_e2e.py | 108 ++++++++++++ .../integration/test_hook_native_formats.py | 159 ++++++++++++++++++ 5 files changed, 286 insertions(+), 5 deletions(-) create mode 100644 tests/integration/test_hook_nested_metadata_install_e2e.py create mode 100644 tests/unit/integration/test_hook_native_formats.py 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()