[parth (Parth-Vasave)]

fix(hooks): type untyped flat command handlers for Claude

Description

Claude-native rendering now supplies "type": "command" for flat hook command entries that omit type, so .claude/settings.json receives schema-valid handlers. Explicit handler types, nested Claude groups, non-command entries and every other target are unchanged.

TL;DR

Flat hook entries without type are valid in Cursor (where type defaults to command), but the Claude render wrapped them into { "matcher", "hooks": [...] } groups and left the handler without the type field Claude's hook schema requires. The Claude renderer now adds it for recognized command entries only. A shared legacy-native selector keeps reinstall cleanup recognizing entries written before this fix without changing ownership or consent policy.

Note

Flat input only. Nested Claude handlers remain as authored. Released main is integrated and its previously missing owner-mutation case now passes. The dated 0.34.0 changelog is unchanged; this undelivered fix remains under Unreleased. Technical readiness does not authorize draft publication. The contributor accepted the completed folds; no author rebase or additional push is requested. This PR remains draft.

Problem (WHY)

• The issue's exact repro (preToolUse with command, matcher, timeout, no type) rendered a Claude handler with no type. As the report puts it: "The nesting happens, but the handler is incomplete."
• The same happened when the flat file sat next to a Claude-shaped hook file in the same package: its group was merged into the same PreToolUse list, still untyped.
• [!] Typing the output changes the content key that reinstall cleanup matches on. Without legacy-form recognition, a project's own root .apm/hooks entries carrying a stale source marker would be left behind as an untyped duplicate instead of healed, regressing the #1329 / #1392 contract.

The approved Done-when requires schema-valid Claude command handlers from applicable untyped flat input while preserving ownership and other targets. This PR claims schema validity, not a demonstrated Claude Code runtime rejection. The regression and mutation evidence below makes that boundary checkable, following Agent Skills' validation pattern: "do the work, run a validator (a script, a reference checklist, or a self-check), fix any issues, and repeat until validation passes."

Approach (WHAT)

• In Claude rendering only, default type: "command" for a flat entry with no type that the neutral hook grammar reads as a command (command, or bash/powershell/windows normalized to command).
• Leave explicit types (including prompt with a command field), nested Claude groups, and untyped non-command entries as authored.
• Leave the shared hook IR (hook_contract.py, hook_ir.py) and other targets' rendering behavior unchanged; Codex's existing typing and event mapping are not overridden.
• Route legacy ownership-match shapes through _legacy_hook_entries(): pre-default Claude groups, existing flat Codex entries, and no legacy entries for other targets.
• Prove installation and upgrade through real CLI scenarios, with a static owner-boundary guard and mutation checks.

Implementation (HOW)

File	Change
src/apm_cli/integration/hook_native_formats.py	_with_claude_default_handler_type() reuses _handler_to_ir() for command recognition. _to_claude_hook_entries(..., default_handler_type=False) reproduces pre-fix output; _legacy_hook_entries() owns Claude/Codex legacy-shape selection.
src/apm_cli/integration/hook_integrator.py	Uses that selector for legacy_content_keys, replacing the inline Codex-only branch. Existing ownership removal policy is unchanged. The file is 2,099 of 2,100 lines at this head.
scripts/architecture_linter/checks/mutation_hook_contract.py	Registers _nhc_legacy_native_shapes under mutation_writes.neutral_hook_contract, requiring cleanup to call the native-shape owner.
tests/unit/integration/test_hook_integrator_issue3130.py	14 component cases cover flat/merged input, command aliases, explicit types, nested/non-command preservation, other-target isolation, and exact settings/sidecar/source-marker healing.
tests/integration/test_architecture_contract_guards.py	Covers typed/untyped Claude native edges and verifies the boundary linter detects bypassing the legacy selector.
tests/integration/test_required_lifecycle_state_machine.py	Two real CLI scenarios (standalone/merged) install, recreate legacy output and stale root ownership, then reinstall twice; assert durable-state convergence, user-setting preservation, an unchanged Codex sentinel, and no hook execution.
docs/src/content/docs/producer/author-primitives/hooks-and-commands.md	Documents Claude's flat-only input/output contract next to the passage the issue quotes.
packages/apm-guide/.apm/skills/apm-usage/package-authoring.md	Mirrors that contract in the packaged usage guide.
CHANGELOG.md	One Fixed entry under [Unreleased], crediting parth (@Parth-Vasave) and linking #3153 / #3130.

Diagrams

Legend: dashed nodes are new native-renderer helpers; legacy and current Claude forms meet at existing ownership reconciliation before settings and sidecar writes.

flowchart LR
    subgraph Source[Source]
        A["Flat Claude command entries"]
    end
    subgraph Render[Native shapes]
        L["_legacy_hook_entries: pre-fix form"]
        N["_with_claude_default_handler_type: recognized untyped flat entries"]
        C["_to_claude_hook_entries: nested groups"]
    end
    subgraph Reconcile[Ownership reconciliation]
        K["_hook_entry_content_key: compare old and current forms"]
        W["Write settings.json and apm-hooks.json"]
    end
    A --> L
    L --> K
    A --> N
    N --> C
    C --> K
    K --> W
    classDef new stroke-dasharray: 5 5;
    class L,N new;

Loading

Trade-offs

• Flat input only. Matches the approved Done-when; extending defaulting into nested Claude groups is not part of this fix.
• Default in the Claude renderer, not shared IR. Keeps target-specific requirements out of neutral grammar and avoids imposing this default on other targets.
• Keep legacy matching, centralize its shape selection. Dropping compatibility would reintroduce stale-root duplicates; the owner helper avoids duplicating that decision in the integrator and stays within its line limit.
• Spec waiver rather than spec edits. OpenAPM v0.1 has no normative target-native handler-type fields. The portable hook subset in #2111 remains separate. Nested-metadata rendering is now delivered on main and is retained here without claiming that issue's scope.
• Bounded validation. Component and real CLI tests verify materialized state; neither they nor the schema claim demonstrate Claude Code runtime rejection. No full local suite was rerun at this head.

Benefits

1. The issue's repro writes {"type": "command", "command": "...", "timeout": 10} instead of an untyped handler.
2. A flat file beside a Claude-shaped file yields the expected typed handlers in both settings.json and apm-hooks.json.
3. Explicit command / prompt types, nested handlers, and non-command entries retain their existing behavior.
4. Codex and Cursor tests prove the Claude-only default helper is not invoked for those targets.
5. Reinstalling over pre-fix settings converges to typed owned groups, including stale root-package entries, without claiming user hooks or executing commands.

Historical contributor observations (not current-head assertions)

The original write-up recorded these observations around cecf16be / 18c4c43c (v0.33.0). They were not revalidated as part of this body correction and are not evidence of failures at the reviewed head:

• Broad local tests wrote apm.lock.yaml, .github/mcp.json, .codex/config.toml, apm_modules/, .local/state/gh/ and build/lib/ into the checkout.
• An unqualified uv run pytest was reported to skip the suite through the performance opt-in collection hook; the v0.33.0 lockfile was reported to reference the Microsoft package feed.
• Some root-level tests had stale assertions, and the earlier local unit run reported 23,584 passed / 2 load-sensitive failures. An earlier lifecycle run reported 228 passed / 1 missing-pwsh failure.
• Earlier audit, NOTICE and docs checks were reported successful. Those results are historical, not a substitute for the exact-head evidence below.

Issue and approved scope

Issue: Fixes #3130

Human scope-approval comment: #3130 (comment)

This PR completes the bounded issue scope: Claude-native typing of untyped flat command handlers (standalone and merged alongside an existing Claude-shaped hook file), preserved explicit and non-command types, regression coverage for both cases, and related guidance. It does not change shared hook representation, other targets' behavior, Codex event-name mapping, ownership or consent behavior. Contributor coordination is resolved by acceptance of the folds; the existing advisory is ship_now for the reviewed head, not an instruction to publish or merge.

Type of change

• Bug fix
• New feature
• Documentation
• Maintenance / refactor

Testing

• Tested locally
• All existing tests pass
• Added tests for new functionality (if applicable)

Evidence below is bound to 832e19a9ff269abb9eddb12e319934923928a6c1, with tested base 897496177ba389c4708b110efa1ee5345ccefb79. "All existing tests pass" remains unchecked because no full local suite was run. This PR remains open and draft; no draft promotion, merge, assignment or reviewer request is authorized by technical success.

Validation evidence

Check	Recorded result
bash .worker-evidence/lint.sh	Exit 0: all seven canonical gates plus architecture; ranked-rolling-20261020/lint.log.
Exact functional/matrix command below	340 passed in 615.48s (0:10:15); includes the 14 issue-specific cases, standalone/merged real-CLI upgrades, sidecar drift, matrix completeness, affected guard mutations, delivered hook-path lifecycle and build provenance.
Matrix completeness and registry-delegation-build-id-verification mutation case	Passed within the same run, no skips. The previously missing case arrived unchanged from released main; no shared-policy repair was made here.
Exact-head owner v2 schema and semantic verifier	Passed, one touched canonical owner and all three mapped functional test IDs executed; ranked-rolling-20261020/owner-verification-technical.json and junit-mappings.json. The verifier took its evidence-required path, not the blocked-status exemption.
Retained genuine review and five mutation programs	Architect, coverage and CEO review at bbd89bf3e4ea5ff9ad59ea86f4e34a9203285b77 carried forward with byte/AST proof of PR-owned additions and faithful main integration; no new panel or original mutation-program execution is claimed.
Previous recovery's full owner matrix	719 passed in 2425.54s at 3241b81d54ce68184a99a2b8f6bb95a6558ec494, including the full owner mutation file. Preserved as prior-head evidence, not re-labelled as current-head execution.
Prior-head conformance / quality / docs-link evidence	319 passed + 2 skipped; 64 passed; 14 link-checker tests passed respectively at bbd89bf3e4ea5ff9ad59ea86f4e34a9203285b77. Not new local runs. Current remote conformance/docs succeeded.
Existing Mode B waiver	Unchanged 25 substantive critical-path added lines and existing contributor waiver; no new waiver or specification policy.

Standard technical workflows at this head: CodeQL, Spec conformance, NOTICE Drift Check, Deploy Docs, CI, Merge Gate, all success. This includes actual CI unit shards, lifecycle and Windows jobs, not just compliance checks. gh pr checks 3153 --repo microsoft/apm --watch --interval 30 exited 0. First-time-contributor execution permission covered only these existing reviewed-head workflows.

Exact current-head command and retained review provenance

Run from the isolated worktree. .worker-evidence holds retained worker artifacts, not committed project files.

TMPDIR="$PWD/.worker-evidence" UV_CACHE_DIR="$PWD/.worker-evidence/uv-cache" APM_E2E_TESTS=1 APM_BINARY_PATH="$PWD/.venv/bin/apm" uv run --frozen --extra dev pytest -q tests/unit/integration/test_hook_integrator*.py tests/unit/integration/test_hook_native_metadata.py tests/integration/test_hook_nested_metadata_install.py tests/integration/test_architecture_contract_guards.py tests/integration/test_required_lifecycle_state_machine.py::test_required_claude_untyped_hook_upgrade_preserves_ownership_and_converges tests/integration/test_architecture_owner_rule_mutations.py::test_matrix_covers_every_registry_guard_exactly_once 'tests/integration/test_architecture_owner_rule_mutations.py::test_owner_rule_catches_its_guard_mutation[hooks-integrations-neutral-hook-contract]' 'tests/integration/test_architecture_owner_rule_mutations.py::test_owner_rule_catches_its_guard_mutation[registry-delegation-build-id-verification]' tests/integration/test_hook_sidecar_drift_e2e.py tests/integration/test_hook_path_package_lifecycle.py tests/integration/test_architecture_hook_package_admission.py tests/integration/test_version_provenance.py --basetemp=.worker-evidence/ranked-rolling-20261020/pytest-functional --junitxml=.worker-evidence/ranked-rolling-20261020/functional.xml

340 passed in 615.48s (0:10:15)

bash .worker-evidence/lint.sh

All seven canonical lint gates plus architecture passed at 832e19a9ff269abb9eddb12e319934923928a6c1

The script runs CI's exact frozen ruff check/format and pylint R0801 commands over src/, tests/ and the required architecture/Windows entrypoints; macOS-compatible equivalents cover YAML I/O, portable relative paths and the 2,100-line limit, followed by auth and architecture boundaries.

The existing five mutation programs at bbd89bf3e4ea5ff9ad59ea86f4e34a9203285b77 produced 10, 3, 1, 1 and 4 expected test failures respectively, then restored the guards. They cover default typing, legacy cleanup, explicit type preservation, the original rewrite-scope check, and paired legacy-owner/static-guard removal. These are failed-test counts, not numbers of programs. PR-added helpers, regression traps and guards retain their AST. Main's delivered metadata renderers, guards and regression tests are retained unchanged alongside them; all current-main hook-path admission and build-provenance changes are also retained. Only the adjacent Unreleased entry overlap required resolution. The retained earlier proofs and ranked-rolling-20261020/provenance-final.json establish the chain. The complete PR and main patches are identical apart from diff metadata, and 73 main-only files are byte-identical. Original contributor commits remain ancestors.

Scenario Evidence

#	Scenario (user promise)	Principle(s)	Test(s) proving it	Type
1	A flat untyped hook becomes a typed Claude handler in settings and the sidecar.	Multi-harness support, Portability by manifest	tests/unit/integration/test_hook_integrator_issue3130.py::test_standalone_flat_untyped_entry_gets_command_type (regression-trap for #3130)	integration (component)
2	The same flat hook beside a Claude-shaped file retains both groups correctly.	Multi-harness support	tests/unit/integration/test_hook_integrator_issue3130.py::test_flat_untyped_entry_merged_next_to_claude_shaped_file	integration (component)
3	Command aliases are recognized; explicit types, non-command entries and nested handlers are not retyped.	Portability by manifest	tests/unit/integration/test_hook_integrator_issue3130.py::test_flat_handler_types_are_defaulted_only_for_commands (7 cases); tests/unit/integration/test_hook_integrator_issue3130.py::test_untyped_handler_inside_nested_claude_group_is_unchanged	integration (component)
4	Codex and Cursor installs do not invoke Claude-only defaulting.	Multi-harness support, Vendor-neutral	tests/unit/integration/test_hook_integrator_issue3130.py::test_claude_handler_default_does_not_reach_other_targets (2 cases; helper-isolation check)	integration (component)
5	Reinstall heals old untyped groups and stale root source markers without duplicate owned entries.	DevX (pragmatic as npm), Governed by policy	tests/unit/integration/test_hook_integrator_issue3130.py::test_reinstall_replaces_untyped_group_written_before_fix; tests/unit/integration/test_hook_integrator_issue3130.py::test_stale_root_source_untyped_group_is_still_healed	integration (component)
6	Real CLI install and two reinstalls preserve user settings, ownership, source files and unrelated target state without executing the installed hook.	DevX (pragmatic as npm), Governed by policy, Secure by default	tests/integration/test_required_lifecycle_state_machine.py::test_required_claude_untyped_hook_upgrade_preserves_ownership_and_converges[standalone] and [merged]	e2e

The registered owner guard is separately proven by tests/integration/test_architecture_contract_guards.py::test_legacy_hook_shapes_boundary_rejects_cleanup_bypass; it and both CLI mappings passed at 832e19a9ff269abb9eddb12e319934923928a6c1 within the 340-test run, with no skips.

How to test

• Run uv run --frozen --extra dev pytest -q tests/unit/integration/test_hook_integrator_issue3130.py; expect 14 passed.
• Run the focused command above with the project's .venv/bin/apm; expect both standalone/merged CLI upgrade scenarios and owner-boundary tests to pass.
• Install the issue's flat preToolUse hook without type using apm install --target claude; confirm PreToolUse in .claude/settings.json has "type": "command" and .claude/apm-hooks.json records the owned group.
• Add a Claude-shaped hook file beside it and reinstall twice; confirm expected groups, sidecar ownership and unrelated settings remain stable.
• Run uv run --frozen --extra dev pytest -q tests/unit/integration/test_hook_integrator_issue3130.py -k 'other_targets or nested or defaulted_only'; confirm other-target isolation and explicit/nested/non-command preservation.

Spec conformance (OpenAPM v0.1)

If this PR changes behaviour that an OpenAPM v0.1 req-XXX covers,
confirm the three-step ritual in the
development guide:

• Spec edit: docs/src/content/docs/specs/openapm-v0.1.md updated
  (new/changed <a id="req-XXX"></a> anchor + prose + Appendix C
  row).
• Manifest edit: docs/src/content/docs/specs/manifests/openapm-v0.1.requirements.yml
  updated.
• Test edit: a @pytest.mark.req("req-XXX") test under
  tests/spec_conformance/ added or extended.
• CONFORMANCE.{md,json} regenerated via
  uv run --extra dev python -m tests.spec_conformance.gen_statement
  and committed.
• N/A -- this PR does not change OpenAPM-observable behaviour.

src/apm_cli/integration/ is a Mode B critical path; the reviewed diff has 25 substantive added lines there. The unmodified Mode B script accepted the existing contributor waiver. No req-XXX covers target-native hook handler fields, and req-lk-021 ownership reconciliation is preserved, not changed.

apm-spec-waiver: Claude-native hook handler type default; OpenAPM v0.1 defines no target-native hook handler fields and req-lk-021 ownership reconciliation is preserved

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

[parth (Parth-Vasave)]

@microsoft-github-policy-service agree

[parth (Parth-Vasave)]

Hi Daniel Meppiel (@danielmeppiel), one scope question carried over from #3130:

This PR adds type: "command" only to flat untyped command entries, both standalone and merged into
an existing Claude-shaped group. Handlers that already sit inside a nested Claude group with a
command but no type still render without type today.

The approved scope says "recognized command handlers", which could cover these too. Done-when only
mentions flat input, though. Should I include the nested case in this PR? It's a one-condition
change plus a regression test. Or should it stay out of scope?

I'm also happy to mark this ready for review whenever it suits you, so CI can run.

[Mohammed Alkindi (MohammedAlkindi)]

The codex case in test_claude_handler_default_does_not_reach_other_targets asserts '"type"' not in rendered, but Codex needs that field too: MatcherGroup.hooks is Vec<HookHandlerConfig>, an enum with #[serde(tag = "type")] (codex-rs/config/src/hook_config.rs in openai/codex), so an untyped handler fails to deserialize. Leaving Codex alone fits the approved scope, but this assertion pins the invalid output. Limiting the parametrization to cursor keeps the scope check without blocking a Codex follow-up.

[Daniel Meppiel (danielmeppiel)]

APM review: current-main technical readiness

Published head 832e19a9ff269abb9eddb12e319934923928a6c1 integrates main 897496177ba389c4708b110efa1ee5345ccefb79 and passes the technical checks below. The PR remains open and draft for independent maintainer verification and publication.

The scope remains recognized untyped flat Claude command entries only. Explicit types, nested groups, noncommands, other targets, ownership and consent are unchanged relative to current main.

Faithful integration, not a fresh panel

Both complete patches - this PR's changes and main's changes - are content-identical after integration apart from diff metadata. All 73 main-only files match main exactly, including landed hook-path admission and build provenance. Only the adjacent Unreleased entry overlap needed resolution; all entries and released changelog bytes are preserved.

The genuine architect, coverage and CEO review at bbd89bf3e4ea5ff9ad59ea86f4e34a9203285b77 recommended ship_now. Exact-content proofs carry that review forward. No new panel or CEO synthesis is claimed. Original mutation programs and prior 719/291-test runs remain separately attributed historical evidence.

Current-head evidence

• 340 passed, no skips: focused hooks, standalone/merged installed-CLI upgrades, sidecar/metadata behavior, delivered hook-path lifecycle, build provenance, matrix completeness and affected guard mutations.
• All seven canonical lint gates plus architecture passed.
• Owner-v2 schema and semantic verification passed with all three functional mappings executed and terminal_evidence_required: true.
• All six workflows succeeded: CI, Merge Gate, CodeQL, docs, conformance, and NOTICE. Actual jobs include both Linux shards, lifecycle and Windows. No full local suite is claimed.

Thanks parth (@Parth-Vasave) for accepting the folds. Contributor history is preserved; no author rebase or extra push is requested. Original 2/0/0 counters and returns are unchanged; this separately recorded compatibility phase used 1 outer / 0 Copilot / 0 CI recovery. The worker did not promote the draft, merge, assign, request reviewers or alter issue state. Publication stays with the maintainer.

---

Generated by autopilot-pr-merge-worker. This comment is AI-generated and may contain errors.

[parth (Parth-Vasave)]

Thanks Daniel Meppiel (@danielmeppiel) for the panel pass, and Mohammed Alkindi (@MohammedAlkindi) for the Codex catch.

I'll take the follow-ups myself in this PR:

1. Sidecar assertions: extend test_reinstall_replaces_untyped_group_written_before_fix and test_stale_root_source_untyped_group_is_still_healed to also check .claude/apm-hooks.json, covering the typed content and the correct source marker.
2. CLI lifecycle coverage: add a real apm install / reinstall test for untyped flat Claude hooks that compares the durable settings.json and sidecar snapshots.
3. Codex assertion (Mohammed Alkindi (@MohammedAlkindi)): agreed, asserting no "type" in Codex output pins output Codex can't deserialize. I'll drop the codex case and keep the cursor one, so the Claude-only scope is still checked without blocking a Codex follow-up. Codex behaviour stays unchanged here, and I'm happy to open a separate issue for untyped Codex handlers if that's useful.

Per the panel, nested-group defaults stay out of scope, so the PR remains flat-input only.

I'll also rebase onto current main (only CHANGELOG.md conflicts) and push everything together. Daniel Meppiel (@danielmeppiel), could you hold the convergence pass on this branch so we don't overlap? Once it's pushed, could you approve the CI runs? They're all action_required at the moment.

[parth (Parth-Vasave)]

Thanks Daniel Meppiel (@danielmeppiel), your convergence pass covered all three follow-ups, including a cleaner fix for Mohammed Alkindi (@MohammedAlkindi)'s Codex point, so I've dropped my local version and won't push over it. Happy to help with anything else before this comes out of draft.

[Copilot]

🟢 Approval recommended

The bounded rendering change preserves compatibility and ownership safeguards, with focused regression coverage and no unresolved findings.

No open issues 🎉

What changed in this PR

Fixes #3130 by adding Claude's required "type": "command" to untyped flat command hooks while preserving explicit types, nested handlers, and other targets.

Changes:

• Adds Claude-only handler typing and legacy-format matching for reinstall cleanup.
• Adds rendering, ownership, upgrade, and architecture-guard regression coverage.
• Updates authoring guidance and the changelog.

File	Description
tests/​unit/​integration/​test_hook_integrator_issue3130.py	Covers typing, preservation, target isolation, and legacy cleanup.
tests/​integration/​test_required_lifecycle_state_machine.py	Tests real CLI upgrades and repeated-install convergence.
tests/​integration/​test_architecture_contract_guards.py	Tests default typing and legacy-selector enforcement.
src/​apm_cli/​integration/​hook_native_formats.py	Adds Claude typing and shared legacy-shape selection.
src/​apm_cli/​integration/​hook_integrator.py	Uses legacy shapes for ownership matching.
scripts/​architecture_linter/​checks/​mutation_hook_contract.py	Guards delegation to the legacy-shape selector.
packages/​apm-guide/​.apm/​skills/​apm-usage/​package-authoring.md	Documents the flat-only default in packaged guidance.
docs/​src/​content/​docs/​producer/​author-primitives/​hooks-and-commands.md	Clarifies Claude handler typing behavior.
CHANGELOG.md	Records the fix under Unreleased.

_{Review effort: Balanced}