Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Fixed Claude hook ownership metadata lost due to `additionalProperties` schema restriction: ownership is now stored in a `.claude/apm-hooks.json` sidecar file and re-injected on read, so APM can track which hooks it owns without violating the Claude settings schema. (#1279)
- `apm pack` no longer prints a misleading `No plugin.json found` warning for marketplace-publishing projects (`marketplace:` block in `apm.yml`, no `dependencies:`); the synthesis path is the APM-native source of truth and is reported as an `[i]` info line when authoring a bare plugin, suppressed otherwise. (#1348)
- `apm marketplace init` and `apm init --marketplace` now scaffold the snake-case `# tag_pattern: "{name}-v{version}"` per package instead of the schema-invalid camelCase `tagPattern` example. (#1348)
- MCP server installation now respects the `targets:` whitelist exactly like `apm install`: drop a non-listed runtime even when its `.cursor/`, `.codex/`, or other on-disk signal exists. Previously the MCP install path called `active_targets()` reading the singular `target:` key only, so projects whitelisting `targets: [copilot]` could still receive `~/.codex/config.toml` and `.cursor/mcp.json` writes from foreign signals. The fix audits both paths: (a) the call site at `local_bundle_handler.py` now forwards the canonical plural list; (b) the gate now delegates to the same `resolve_targets` resolver that backs `apm install` skills, so a malformed `targets:` field (conflicting `target:` + `targets:`, `targets: []`, or unknown target name) fails closed with the same `[x]` red error voice + remediation block. The same delegation closes a related asymmetry: a greenfield project (no `targets:`, no `--target` flag, no detected signals) used to silently fall back to `[copilot]` for MCP-only invocations, while `apm install` raised `NoHarnessError` on the same input -- both surfaces now error consistently. Drop lines now use the `[i] Skipped MCP config for X (active targets: Y)` format mirroring the canonical `Targets: X (source: Y)` provenance line. The `-g`/`--global` carve-out is unchanged: `apm install -g --mcp NAME` writes to user-scope (`~/.config/...`, `~/.codex/`, etc.) bypassing the project-scope gate by design. (#1335)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ Copilot hook files are namespaced with the source package name to avoid
collisions across installed deps; bundled scripts land alongside under
`.github/hooks/scripts/<pkg>/`.

Claude's `settings.json` uses `additionalProperties: false` in its JSON
schema, which rejects any unknown keys (including APM's internal
`_apm_source` ownership marker). APM therefore writes a companion sidecar
file `.claude/apm-hooks.json` that stores the ownership metadata separately.
This sidecar is created and cleaned up automatically alongside
`settings.json`; it is an APM implementation detail and should not be edited
by hand.

Verified against `src/apm_cli/integration/targets.py` and
`src/apm_cli/integration/hook_integrator.py`.

Expand Down
132 changes: 132 additions & 0 deletions src/apm_cli/integration/hook_integrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class _MergeHookConfig:
config_filename: str # e.g. "settings.json" or "hooks.json"
target_key: str # target name passed to _rewrite_hooks_data
require_dir: bool # True = skip if target dir doesn't exist
schema_strict: bool = False # True = strip _apm_source before writing to disk
Comment thread
sergio-sisternes-epam marked this conversation as resolved.


# Per-target hook event name mapping. Packages are authored with
Expand Down Expand Up @@ -156,6 +157,7 @@ def _copilot_keys_to_gemini(hook: dict) -> None:
config_filename="settings.json",
target_key="claude",
require_dir=False,
schema_strict=True,
),
"cursor": _MergeHookConfig(
config_filename="hooks.json",
Expand All @@ -179,6 +181,47 @@ def _copilot_keys_to_gemini(hook: dict) -> None:
),
}

_APM_HOOKS_SIDECAR = "apm-hooks.json"


def _reinject_apm_source_from_sidecar(hooks: dict, sidecar_data: dict) -> None:
"""Restore _apm_source markers from sidecar into in-memory hook entries.

Schema-strict targets (e.g. Claude) do not persist ``_apm_source`` in
their settings file. Instead, ownership metadata is stored in a
sidecar file. This helper re-injects those markers so the rest of
the integration logic can work with them as normal.

Each sidecar entry is consumed at most once to prevent falsely claiming
user-owned hooks that happen to have identical content to an APM hook.

Args:
hooks: The ``"hooks"`` dict loaded from the target config file
(mutated in-place).
sidecar_data: The dict loaded from the sidecar file.
"""
for event_name, sidecar_entries in sidecar_data.items():
if event_name not in hooks or not isinstance(sidecar_entries, list):
continue
# Build a consumable pool of (normalised-content, source) pairs.
# Each entry is popped on first match so identical content shared
# between APM and the user is only claimed once.
pool: list[tuple[dict, str]] = []
for sc_entry in sidecar_entries:
if isinstance(sc_entry, dict) and "_apm_source" in sc_entry:
cmp = {k: v for k, v in sorted(sc_entry.items()) if k != "_apm_source"}
pool.append((cmp, sc_entry["_apm_source"]))

for disk_entry in hooks[event_name]:
if not isinstance(disk_entry, dict) or "_apm_source" in disk_entry:
continue
disk_cmp = {k: v for k, v in sorted(disk_entry.items()) if k != "_apm_source"}
for i, (sc_cmp, source) in enumerate(pool):
if disk_cmp == sc_cmp:
disk_entry["_apm_source"] = source
pool.pop(i)
break


# Mapping from hook-file stem suffix to the set of target keys that
# should receive the file. Files whose stem does not match any
Expand Down Expand Up @@ -683,6 +726,29 @@ def _integrate_merged_hooks(
except (json.JSONDecodeError, OSError):
json_config = {}

# Load sidecar ownership metadata (schema-strict targets)
sidecar_path = target_dir / _APM_HOOKS_SIDECAR
sidecar_data: dict = {}
if config.schema_strict and sidecar_path.exists():
try:
with open(sidecar_path, encoding="utf-8") as f:
_raw = json.load(f)
if isinstance(_raw, dict):
sidecar_data = _raw
else:
_log.warning(
"Sidecar file %s contains non-dict JSON; treating as empty.",
sidecar_path,
)
sidecar_data = {}
except (json.JSONDecodeError, OSError) as exc:
_log.warning("Failed to read sidecar %s: %s; treating as empty.", sidecar_path, exc)
sidecar_data = {}

# Re-inject _apm_source from sidecar into matching in-memory entries
if sidecar_data and "hooks" in json_config:
_reinject_apm_source_from_sidecar(json_config["hooks"], sidecar_data)

if "hooks" not in json_config:
json_config["hooks"] = {}

Expand Down Expand Up @@ -808,6 +874,37 @@ def _integrate_merged_hooks(
# Don't track the config file in target_paths -- it's a shared
# file cleaned via _apm_source markers, not file-level deletion
json_path.parent.mkdir(parents=True, exist_ok=True)

if config.schema_strict:
# Build sidecar from entries that have _apm_source
sidecar_out: dict = {}
for event_name, entries_list in json_config.get("hooks", {}).items():
if not isinstance(entries_list, list):
continue
owned = [e for e in entries_list if isinstance(e, dict) and "_apm_source" in e]
if owned:
sidecar_out[event_name] = [dict(e) for e in owned]

# Strip _apm_source from entries before writing to disk
for entries_list in json_config.get("hooks", {}).values():
if isinstance(entries_list, list):
for entry in entries_list:
if isinstance(entry, dict):
entry.pop("_apm_source", None)

# Write sidecar
sidecar_path = target_dir / _APM_HOOKS_SIDECAR
if sidecar_out:
try:
with open(sidecar_path, "w", encoding="utf-8") as f:
json.dump(sidecar_out, f, indent=2)
f.write("\n")
except OSError as exc:
_log.warning("Failed to write sidecar %s: %s", sidecar_path, exc)
elif sidecar_path.exists():
sidecar_path.unlink()

# Write the (now schema-clean) config
with open(json_path, "w", encoding="utf-8") as f:
json.dump(json_config, f, indent=2)
f.write("\n")
Expand Down Expand Up @@ -1011,6 +1108,33 @@ def sync_integration(
with open(json_path, encoding="utf-8") as f:
settings = json.load(f)

# Load sidecar to restore _apm_source markers
sidecar_path = json_path.parent / _APM_HOOKS_SIDECAR
sidecar_data: dict = {}
if sidecar_path.exists():
try:
with open(sidecar_path, encoding="utf-8") as sf:
_raw = json.load(sf)
if isinstance(_raw, dict):
sidecar_data = _raw
else:
_log.warning(
"Sidecar file %s contains non-dict JSON; treating as empty.",
sidecar_path,
)
sidecar_data = {}
except (json.JSONDecodeError, OSError) as exc:
_log.warning(
"Failed to read sidecar %s: %s; treating as empty.",
sidecar_path,
exc,
)
sidecar_data = {}

# Re-inject _apm_source from sidecar
if sidecar_data and "hooks" in settings:
_reinject_apm_source_from_sidecar(settings["hooks"], sidecar_data)

if "hooks" in settings:
modified = False
for event_name in list(settings["hooks"].keys()):
Expand All @@ -1035,6 +1159,14 @@ def sync_integration(
json.dump(settings, f, indent=2)
f.write("\n")
stats["files_removed"] += 1

# Clean up sidecar
if sidecar_path.exists():
sidecar_path.unlink()

# Remove stale sidecar when no hooks section remains
if sidecar_path.exists() and "hooks" not in settings:
sidecar_path.unlink()
Comment thread
sergio-sisternes-epam marked this conversation as resolved.
except (json.JSONDecodeError, OSError):
stats["errors"] += 1
else:
Expand Down
Loading
Loading