Skip to content

fix(hooks): normalize flat entries for Claude#2097

Open
danielmeppiel wants to merge 5 commits into
mainfrom
fix/claude-hook-nesting
Open

fix(hooks): normalize flat entries for Claude#2097
danielmeppiel wants to merge 5 commits into
mainfrom
fix/claude-hook-nesting

Conversation

@danielmeppiel

Copy link
Copy Markdown
Collaborator

fix(hooks): normalize flat entries for Claude

TL;DR

Claude integration now converts flat Copilot-style command entries into Claude's required matcher/hooks groups before merge. The same normalized shape is written to .claude/settings.json and the APM ownership sidecar, preventing Claude from rejecting the entire settings file.

Important

Closes #2062.

Problem (WHY)

  • Flat hook entries targeting Claude were written unchanged, so Claude reported hooks: Expected array, but received undefined and skipped the settings file.
  • The ownership sidecar also retained the invalid flat shape, making a settings-only repair unstable on later installs.
  • [!] Authors could not use one flat manifest for both Copilot and Claude without manually maintaining target-specific shapes.

The fix keeps the change deterministic and testable: "Grounding outputs in deterministic tool execution transforms probabilistic generation into verifiable action."

Approach (WHAT)

# Fix
1 Add a Claude sibling transform around the existing Gemini and Antigravity normalization path.
2 Wrap only flat entries with matcher: "*"; preserve already nested entries.
3 Transform before ownership marking so settings and sidecar stay structurally aligned.
4 Document the portable flat authoring contract in Starlight and the bundled APM guide.

Implementation (HOW)

File Change
src/apm_cli/integration/hook_integrator.py Extends the shared nested-entry helper with an optional default matcher and dispatches Claude entries through it before merge.
tests/unit/integration/test_hook_integrator_issue2062.py Reproduces the package/consumer flow and asserts the exact settings and sidecar structures.
docs/src/content/docs/producer/author-primitives/hooks-and-commands.md Shows flat source input and Claude's normalized output.
packages/apm-guide/.apm/skills/apm-usage/package-authoring.md Records that one flat manifest can target both Copilot and Claude.

Diagrams

Legend: the dashed stage is the new target-specific normalization added before the existing ownership and write path.

flowchart LR
    subgraph Parse[Parse]
        P1[Flat hook entry]
    end
    subgraph Normalize[Normalize for target]
        N1[Claude matcher group]
    end
    subgraph Persist[Persist]
        W1[settings.json]
        W2[apm-hooks.json]
    end
    P1 --> N1
    N1 --> W1
    N1 --> W2
    classDef new stroke-dasharray: 5 5;
    class N1 new;
Loading

Trade-offs

  • Reuse the shared transform instead of adding a separate merge path. This keeps target normalization next to the existing Gemini and Antigravity behavior and avoids duplicate wrapping logic.
  • Use matcher: "*" only for newly wrapped entries. Existing nested Claude entries retain their authored matcher rather than being rewritten.
  • Keep ownership metadata on the outer group. Moving it into command handlers would diverge from the sidecar's existing matching and cleanup contract.

Benefits

  1. A flat PreToolUse command becomes one schema-valid Claude matcher group.
  2. Settings and sidecar contain the same normalized hook content, with ownership metadata only in the sidecar.
  3. Existing nested manifests and non-Claude target transforms remain covered by the 169-test relevant suite.
  4. Package authors have one documented source shape for Copilot and Claude.

Validation

uv run --extra dev pytest tests/unit/integration/test_hook_integrator_issue2062.py tests/unit/integration/test_hook_integrator.py tests/unit/integration/test_hook_naked_format.py -q:

169 passed in 15.88s

uv run --extra dev pytest tests/unit tests/test_console.py -q:

exit 0
Lint mirror and mutation evidence

uv run --extra dev ruff check src/ tests/:

All checks passed!

uv run --extra dev ruff format --check src/ tests/:

1411 files already formatted

uv run --extra dev python -m pylint --disable=all --enable=R0801 --min-similarity-lines=10 --fail-on=R0801 src/apm_cli/:

Your code has been rated at 10.00/10

bash scripts/lint-auth-signals.sh:

[+] auth-signal lint clean

Disabling the Claude transform made test_hook_integrator_issue2062.py fail on the unchanged flat entry; restoring it returned the test to green.

Scenario Evidence

# Scenario (user promise) Principle(s) Test(s) proving it Type
1 Installing a package with flat hooks for Claude writes loadable matcher groups to settings and preserves ownership for later installs. Portability by manifest, Multi-harness support, DevX tests/unit/integration/test_hook_integrator_issue2062.py::test_claude_wraps_flat_hook_entries_in_settings_and_sidecar (regression-trap for #2062) integration

How to test

  • Run the issue regression test and observe 1 passed.
  • Install a package containing a flat PreToolUse command with target claude.
  • Inspect .claude/settings.json and confirm the entry has matcher: "*" plus a hooks array.
  • Inspect .claude/apm-hooks.json and confirm the same outer shape includes _apm_source.
  • Re-run the install and confirm the hook is not duplicated.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 9, 2026 22:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Claude hook integration by normalizing flat (Copilot-style) hook entries into Claude’s nested matcher/hooks grouping prior to merge, keeping both .claude/settings.json and the APM ownership sidecar structurally aligned (and preventing Claude from skipping the whole settings file).

Changes:

  • Extend the shared “wrap flat entries into nested groups” helper to optionally add a default matcher, and apply it for the claude target.
  • Add a regression test covering settings + sidecar output shape for Claude (#2062).
  • Document the “flat input -> nested Claude output” contract in Starlight docs and the bundled APM guide.
Show a summary per file
File Description
src/apm_cli/integration/hook_integrator.py Adds a Claude-specific normalization step and extends the shared nesting helper with default_matcher.
tests/unit/integration/test_hook_integrator_issue2062.py New regression test ensuring Claude output is nested (and sidecar preserves ownership).
docs/src/content/docs/producer/author-primitives/hooks-and-commands.md Documents portable flat hook entries and shows Claude’s normalized nested output.
packages/apm-guide/.apm/skills/apm-usage/package-authoring.md Updates the packaged guide to describe the same flat->nested portability contract.

Review details

  • Files reviewed: 4/4 changed files
  • Comments generated: 3
  • Review effort level: Low

Comment on lines +249 to +253
Shared by the Gemini and Antigravity transforms (both use the Claude
nested matcher shape for tool events). *key_fixer* renames the inner
command/timeout keys in place for the specific target. Entries already
in nested form have only their inner keys fixed.
command/timeout keys in place for the specific target. *default_matcher*
is added to newly wrapped entries when the target requires one. Entries
already in nested form have only their inner keys fixed.
Comment on lines +43 to +45
`postToolUse`) shapes; events are renamed per target during merge. Flat
Copilot command entries are also portable: when the target is Claude, APM
wraps each one in Claude's required `matcher` and `hooks` group.
Comment on lines +146 to +149
When deploying to Claude, APM wraps each flat entry in the required
`{"matcher": "*", "hooks": [...]}` group. Copilot keeps the flat entry, so
one source manifest is valid for both targets. Already nested Claude entries
remain nested.
@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_with_followups

Fixes Claude hook rejection by normalizing flat entries into matcher/hooks groups, keeping one portable manifest for both Copilot and Claude.

cc @danielmeppiel @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

All active panelists converged on a minimal, sound fix. The shared nested-entry helper is the right abstraction, no auth or supply-chain surface changes, and the runtime cost remains linear in the small hook-entry set. The remaining in-scope work is stronger CLI-level regression evidence and small documentation precision improvements.

Aligned with: portable by manifest; multi-harness support; pragmatic package authoring; OSS community feedback.

Panel summary

Persona B R N Takeaway
Python Architect 0 0 2 Clean shared-helper design; update one stale docstring.
CLI Logging Expert 0 0 2 No CLI output regression.
DevX UX Expert 0 0 2 Portable authoring contract is clear; explain the wildcard.
Supply Chain Security Expert 0 0 1 Global wildcard preserves flat-entry semantics.
OSS Growth Hacker 0 0 2 Lead release notes with write-once portability.
Doc Writer 0 1 1 Docs are accurate; keep mirrored guidance concise.
Test Coverage Expert 0 2 1 CLI test passes; assert sidecar and marker contract.
Performance Expert 0 0 2 O(entries) transform has negligible cost.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

  1. [Test Coverage Expert] Assert the sidecar structure and use module-level pytestmark in the CLI integration test.
  2. [Doc Writer] Keep the Starlight and bundled APM guide explanations concise and synchronized.
  3. [OSS Growth Hacker] Add a user-benefit CHANGELOG line and scannable source/output captions.
  4. [Supply Chain Security Expert] Explain that matcher: "*" preserves the global semantics of a flat entry.
  5. [Performance Expert] Replace the per-call no-op lambda with a named helper for clarity.

Recommendation

Fold the CLI-level sidecar proof, mutation-break evidence, and small code/doc polish, then run a final panel pass.


Full per-persona findings
  • Python Architect: update the shared-helper docstring to include Claude; commit the new CLI integration test.
  • CLI Logging Expert: make the Claude wrapper docstring precise; optional transform debug breadcrumb.
  • DevX UX Expert: explain matcher: "*" in both documentation surfaces.
  • Supply Chain Security Expert: state that the wildcard is semantically equivalent to flat global hooks.
  • OSS Growth Hacker: add benefit-led release notes and source/output captions.
  • Auth Expert: inactive; no auth surface changed.
  • Doc Writer: keep duplicate mirrored prose bounded while preserving required APM guide synchronization.
  • Test Coverage Expert: assert .claude/apm-hooks.json, use module-level marker placement, and preserve nested-input behavior.
  • Performance Expert: name the no-op key fixer; shallow copies are expected and safe.

This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.

danielmeppiel and others added 2 commits July 10, 2026 03:09
Add full CLI install coverage for settings and sidecar preservation, and fold the panel's documentation and regression-test follow-ups for #2062.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Clarify the flat-entry shape and limit the example claim to the Copilot and Claude targets it demonstrates.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_now

Claude flat hook manifests now install as valid matcher groups while preserving existing settings and APM ownership metadata.

cc @danielmeppiel @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

All active panelists converge on shipping. The implementation reuses the existing target-normalization strategy, preserves the flat entry's global semantics with matcher: "*" and adds no auth, supply-chain, logging, or meaningful performance surface. The CLI integration test exercises a real apm install --target claude, while unit coverage protects sidecar ownership and already-nested input.

The only remaining observation is a contradictory style preference about representing the no-op key transform. The named helper was requested and folded in the prior pass; it is explicit and consistent with the callable strategy contract, so no further action is warranted.

Aligned with: portable by manifest; secure by default; multi-harness support; pragmatic package authoring; OSS community feedback.

Panel summary

Persona B R N Takeaway
Python Architect 0 0 1 Clean reuse of the shared target-normalization strategy.
CLI Logging Expert 0 0 0 No CLI output or logging surface changed.
DevX UX Expert 0 0 0 The portable source and generated Claude shape are clear.
Supply Chain Security Expert 0 0 0 Provenance and global matcher semantics are preserved.
OSS Growth Hacker 0 0 0 Benefit-led release and docs framing is ready.
Doc Writer 0 0 0 Starlight and the bundled guide are accurate and synchronized.
Test Coverage Expert 0 0 0 CLI, unit, and mutation-break evidence protect the user promise.
Performance Expert 0 0 0 The O(entries) in-memory transform is negligible.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Folded in this run

  • (panel) Added an empirical CLI integration test for apm install --target claude -- resolved in c3650318c1118a401300e472f7afca84a5809a57.
  • (panel) Proved existing settings and valid nested hooks survive the install -- resolved in c3650318c1118a401300e472f7afca84a5809a57.
  • (panel) Asserted normalized sidecar ownership and module-level integration marker placement -- resolved in c3650318c1118a401300e472f7afca84a5809a57.
  • (panel) Clarified wildcard semantics, source/output captions, and the Copilot-and-Claude scope -- resolved in c4e290220d523ae54f055d6317cf3c9f6b97ce13.
  • (panel) Added a benefit-led changelog entry and precise named Claude key strategy -- resolved in c3650318c1118a401300e472f7afca84a5809a57.

Copilot signals reviewed

Copilot review round 1 produced a summary and zero inline findings. Round 2 after the first push also produced zero inline findings.

Regression-trap evidence (mutation-break gate)

  • tests/integration/test_claude_flat_hook_install_e2e.py::test_install_claude_nests_flat_hooks_and_preserves_settings -- deleted the Claude dispatch to _to_claude_hook_entries; the test failed on the unchanged flat entry; the guard was restored and the test passed.

Lint contract

The canonical ruff pair was silent at the final SHA. Pylint R0801 rated 10.00/10, the YAML/file-length/relative-path guards were clean, and scripts/lint-auth-signals.sh reported clean.

CI

All checks on exact head c4e290220d523ae54f055d6317cf3c9f6b97ce13 completed successfully or were intentionally skipped. CI run: https://github.com/microsoft/apm/actions/runs/29061945387

Mergeability status

PR head SHA CEO stance iters folds defers Copilot rounds CI mergeable mergeStateStatus notes
#2097 c4e2902 ship_now 2 5 0 2 green MERGEABLE BLOCKED awaiting required review

Recommendation

All in-scope findings are folded, exact-SHA CI is green, and the regression test is mutation-proven. Ready for maintainer review.


Full per-persona findings

Python Architect

  • [nit] An optional None key-fixer could replace the named no-op helper. The named helper was explicitly requested in the prior pass and keeps the strategy callable and intent grep-able, so no change is recommended.

CLI Logging Expert

No findings.

DevX UX Expert

No findings.

Supply Chain Security Expert

No findings.

OSS Growth Hacker

No findings.

Auth Expert -- inactive

PR touches hooks, tests, documentation, and changelog; no authentication surface changed.

Doc Writer

No findings.

Test Coverage Expert

No findings. Relevant tests passed, and the CLI regression test failed when the production guard was removed.

Performance Expert

No findings.

This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.

danielmeppiel and others added 2 commits July 11, 2026 19:03
Fold panel doc nits: call them hook entries (not command entries) to
disambiguate from slash-commands on the same page, and state explicitly
that Copilot consumes the flat entry as-is while only Claude requires
the nested matcher group.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e0c0aa7f-c584-4f8c-8931-960e4836d376
@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_now

Fix silent-failure cliff where Claude Code rejected flat hook entries, restoring portable manifest portability across Copilot, Claude, Gemini, and Antigravity targets.

cc @danielmeppiel @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

This PR closes a real adoption blocker: any package author shipping a flat hook manifest (the natural authoring format) found that apm install --target claude silently disabled ALL hooks in the target environment. Users received no error -- Claude Code simply refused the malformed contract. The fix reuses the existing shared _to_nested_hook_entries helper with a no-op Claude key fixer, applying the same proven wrap-if-flat logic already live for Gemini and Antigravity. Already-nested entries (including those with authored matcher or action-level if clauses) pass through unchanged; no lossy vendor IR is introduced.

The panel converges cleanly. Zero blocking findings across all seven active personas. Supply-chain-security confirms no new trust boundary (pure in-memory reshape of local data, value-safe, no exec/network surface). Test-coverage-expert confirms the E2E and unit tests are load-bearing per mutation-break gate -- removing the guard reproduces #2062; restoring it fixes it. Auth and performance lenses are correctly inactive (no auth surface touched, O(entries) off hot path). The two in-scope doc-clarity nits have been folded at the final head.

The single deferred item -- an optional _log.debug breadcrumb on flat-to-nested wrapping -- was correctly scoped out because it would modify the shared helper used by all three targets, whereas this PR is intentionally the minimal exact-Claude fix. Parity with the existing silent sibling transforms is the right default; the observability improvement belongs in a follow-up issue that adds the breadcrumb uniformly across all targets.

Aligned with: Portable by manifest -- one flat hook manifest now materializes correctly for both Copilot (flat passthrough) and Claude (nested wrap), eliminating an author-invisible portability gap; Secure by default -- pure in-memory value-safe reshape, no new exec/network/credential surface, idempotent sidecar upsert with provenance marker; Multi-harness/multi-host -- Claude reaches parity with Gemini and Antigravity, all three targets now handle flat-authored hooks without author intervention.

Growth signal. Removes an adoption cliff that silently broke portable hook manifests on Claude Code, the second-most-popular AI coding assistant; package authors can now ship one manifest and trust it works everywhere APM targets.

Panel summary

Persona B R N Takeaway
Python architect 0 0 1 Fix reuses the shared _to_nested_hook_entries helper with a no-op Claude key fixer; _apm_source provenance moves to the outer group for idempotent sidecar upsert. Sound and consistent with the gemini/antigravity siblings.
Cli logging expert 0 0 1 Value-safe: the transform adds no new user-facing output and leaks no command/env/secret payloads. Parity with the silent gemini/antigravity transforms is maintained.
Devx ux expert 0 1 2 Removes a silent-failure cliff (Claude previously rejected flat hooks and disabled them all); portable manifests now just work. Minor doc-wording polish suggested (now folded).
Supply chain security expert 0 0 0 No new trust boundary: a pure in-memory reshape of already-trusted local data, no network/exec surface, value-safe handling preserved. Sidecar upsert is idempotent. No findings.
Oss growth hacker 0 0 1 Fixes a real adoption cliff (portable hook manifests silently broke on Claude). CHANGELOG line is adequate; only optional wording polish.
Doc writer 0 0 2 Docs accurately describe the new flat->nested Claude behavior on both the Starlight page and the apm-guide usage resource. Starlight clarity clause now folded.
Test coverage expert 0 0 0 All critical hook/install promises defended by the e2e subprocess test and unit tests; mutation-break gate confirms the tests are load-bearing. Ship.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 1 follow-ups

  1. [Cli logging expert] Add a _log.debug breadcrumb when _to_nested_hook_entries wraps flat entries, uniformly across Claude/Gemini/Antigravity targets. -- Aids field diagnosis when hooks silently do nothing; scoped out of this PR to avoid cross-target helper churn on a minimal-fix branch.

Recommendation

Merge once required checks are green on 308e01b. The fix is minimal, load-bearing tests prove the user promise holds, all in-scope nits are folded, and the single deferred observability item has a clean scope-boundary note. Track the debug-breadcrumb follow-up as a small DevX issue for a future uniform pass across all hook transforms.


Full per-persona findings

Python architect

  • [nit] _preserve_claude_hook_keys is a no-op key fixer at src/apm_cli/integration/hook_integrator.py
    At a single call-site the named no-op is self-documenting; a shared sentinel would only pay off with a second Claude-family caller. No change needed now.

Cli logging expert

  • [nit] Optional debug breadcrumb when wrapping flat entries at src/apm_cli/integration/hook_integrator.py
    A single _log.debug counting wrapped entries would aid field diagnosis, but is not required; current silence matches sibling transforms.

Devx ux expert

  • [recommended] Consider a debug breadcrumb on flat->nested wrap at src/apm_cli/integration/hook_integrator.py
    When a hook silently does nothing, authors have little to go on; a debug line would shorten diagnosis. Non-gating; parity with existing transforms is acceptable.
  • [nit] Disambiguate 'command entries' from slash-commands at docs/src/content/docs/producer/author-primitives/hooks-and-commands.md
    On a page titled hooks-and-commands, 'Copilot command entries' can be misread as slash-commands; 'hook entries' is clearer. (Folded at 308e01b.)
  • [nit] State that Copilot keeps the flat entry at docs/src/content/docs/producer/author-primitives/hooks-and-commands.md
    The portability section shows Claude output but did not explicitly say Copilot consumes the flat entry unchanged. (Folded at 308e01b.)

Supply chain security expert

No findings.

Oss growth hacker

  • [nit] CHANGELOG could lead with user benefit over mechanism at CHANGELOG.md
    Optional stylistic preference; the precise term 'flat hook manifests' is accurate and matches the issue, so keeping it is defensible.

Auth expert -- inactive

PR #2097 touches only hook normalization (hook_integrator.py), tests, and docs. No token management, credential resolution, git auth, or remote-host surface is modified; the auth lens is not engaged.

Doc writer

  • [nit] Make the Copilot-keeps-flat behavior explicit on the Starlight page at docs/src/content/docs/producer/author-primitives/hooks-and-commands.md
    The page showed Claude output but should state Copilot consumes the flat entry unchanged. (Folded at 308e01b.)
  • [nit] CHANGELOG [Unreleased] header has trailing whitespace at CHANGELOG.md
    Cosmetic markdownlint MD009; pre-existing on main and outside this PR's diff, so not introduced here.

Performance expert -- inactive

The transform is O(entries) over a handful of hook entries at install time, off any hot path (no resolution/download/materialization loop). Negligible cost; performance lens not engaged.

Test coverage expert

No findings.

This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] apm install --target claude writes flat hook entries to .claude/settings.json (missing matcher/hooks nesting)

2 participants