fix(hooks): rebuild merged hooks from transitive lockfile survivors by Shaurya2k06 · Pull Request #2256 · microsoft/apm

Shaurya2k06 · 2026-07-16T14:26:23Z

fix(hooks): rebuild merged hooks from transitive lockfile survivors

TL;DR

apm prune and apm uninstall clear every _apm_source merged-hook entry, then rebuild from survivors. That rebuild walked only apm.yml directs, so a still-installed transitive package lost its hooks until the next apm install. Both paths now share surviving_dependency_refs_for_reintegration(), which prefers the post-removal lockfile (direct + transitive).

Note

Closes #2254. Follow-up from the #2252 review panel. Does not change #2250 target-scope wipe asymmetry.

apm-spec-waiver: hook integration has no existing OpenAPM v0.1 req coverage for prune/uninstall reintegration membership; this restores the clear+rebuild contract to include transitive survivors already present in the lockfile, not a net-new capability.

Problem (WHY)

HookIntegrator.reconcile_after_removal() rebuilt only get_all_apm_dependencies() (direct prod+dev). Its own docstring named the gap: "rebuilds only direct dependencies… a transitive dependency's merged hooks are wiped… but never re-integrated."
_sync_integrations_after_uninstall() Phase 2 walked only get_apm_dependencies() (prod directs). Hooks are re-integrated in that loop (hooks are not multi_target), so the same wipe left transitive hooks gone.
[!] Public docs (prune.md, uninstall.md) never mentioned the depth gap — it lived only in an internal docstring.

Why these matter: P6 — Reliability over magic requires predictable, auditable cleanup. Architecture discipline requires one owner for the survivor set (.github/instructions/architecture.instructions.md): "Every durable decision… has exactly ONE canonical owner." Two divergent direct-only loops were split authority for the same fact.

Approach (WHAT)

#	Fix
1	Add `surviving_dependency_refs_for_reintegration()` next to `build_installed_package_info`: lockfile package deps (excl. `.`), else manifest directs.
2	`reconcile_after_removal` uses the helper (loads on-disk lockfile — prune already wrote it).
3	Uninstall Phase 2 uses the helper with the in-memory post-removal lockfile (disk is still stale).
4	Docs + CHANGELOG describe the fixed behavior (not a "Known limitations" note).

Implementation (HOW)

src/apm_cli/models/apm_package.py — New helper; deferred LockFile import to avoid the existing circular import.
src/apm_cli/integration/hook_integrator.py — Rebuild membership via helper; docstring cites Transitive-dependency hook entries wiped but never rebuilt by uninstall/prune reconciliation #2254 fixed; keeps Hook wipe scans all merge targets but rebuild only re-populates canonical_targets (uninstall + prune) #2250 target-scope note.
src/apm_cli/commands/uninstall/engine.py + cli.py — lockfile= on _sync_integrations_after_uninstall; Phase 2 walks helper output.
tests/integration/test_prune_hook_reconciliation_e2e.py — Hermetic keeper → transitive-hooks + orphan prune; asserts transitive hooks survive.
tests/unit/test_surviving_deps_reintegration.py — Helper + reconcile + uninstall Phase 2 regressions.
docs/.../prune.md, uninstall.md, manage-dependencies.md, CHANGELOG.md — Document lockfile-wide rebuild.

Diagrams

Legend: clear+rebuild after removal — wipe is unchanged; rebuild membership now includes transitive lockfile survivors.

sequenceDiagram
    participant Cmd as prune_or_uninstall
    participant Lock as apm.lock.yaml
    participant HI as HookIntegrator_or_Phase2
    participant CFG as merge_target_JSON

    Cmd->>Lock: remove orphans / mutate survivors
    Cmd->>HI: wipe all _apm_source entries
    HI->>CFG: sync_integration managed_files empty set
    HI->>Lock: surviving_dependency_refs_for_reintegration
    Note over HI,Lock: direct plus transitive survivors
    loop each surviving dep_ref
        HI->>CFG: integrate_hooks_for_target
    end

Trade-offs

Lockfile over live resolver. Post-removal lockfile is already the committed survivor graph; re-running APMDependencyResolver would add I/O and risk diverging from what prune/uninstall just wrote.
Uninstall must pass in-memory lockfile. Reading disk during sync would still see removed packages; build_installed_package_info would skip missing dirs, but membership would be wrong for edge cases. Explicit lockfile= keeps authority with the uninstall pipeline.
Widening uninstall Phase 2 to all primitives. Same loop reintegrates prompts/agents/skills/hooks; fixing membership there restores transitive skills/prompts too — same root cause, not a separate feature.
Hook wipe scans all merge targets but rebuild only re-populates canonical_targets (uninstall + prune) #2250 left alone. Wipe-all-targets vs rebuild-canonical_targets remains tracked separately.

Benefits

Transitive packages that remain installed keep their merged hooks after prune/uninstall without a follow-up apm install.
One survivor-list owner shared by prune reconcile and uninstall Phase 2.
Hermetic e2e + unit coverage encode the Transitive-dependency hook entries wiped but never rebuilt by uninstall/prune reconciliation #2254 failure mode (RED without the helper walk).
Docs match runtime behavior instead of hiding the old gap in a source docstring.

Validation

Focused tests + lint

$ uv run pytest tests/unit/test_surviving_deps_reintegration.py \
    tests/integration/test_prune_hook_reconciliation_e2e.py -v
============================= test session starts ==============================
collected 13 items

tests/unit/test_surviving_deps_reintegration.py::test_surviving_refs_include_lockfile_transitive PASSED
tests/unit/test_surviving_deps_reintegration.py::test_surviving_refs_fallback_without_lockfile PASSED
tests/unit/test_surviving_deps_reintegration.py::test_surviving_refs_prefer_passed_in_memory_lockfile PASSED
tests/unit/test_surviving_deps_reintegration.py::test_reconcile_after_removal_rebuilds_transitive_hooks PASSED
tests/unit/test_surviving_deps_reintegration.py::test_uninstall_phase2_reintegrates_transitive_hooks PASSED
tests/integration/test_prune_hook_reconciliation_e2e.py::test_prune_removes_merged_hook_entries_and_sidecar[claude] PASSED
tests/integration/test_prune_hook_reconciliation_e2e.py::test_prune_removes_merged_hook_entries_and_sidecar[cursor] PASSED
tests/integration/test_prune_hook_reconciliation_e2e.py::test_prune_preserves_sibling_hooks_and_manual_entries PASSED
tests/integration/test_prune_hook_reconciliation_e2e.py::test_prune_preserves_transitive_dependency_hooks PASSED
tests/integration/test_prune_hook_reconciliation_e2e.py::test_prune_hook_reconciliation_is_idempotent PASSED
tests/integration/test_prune_hook_reconciliation_e2e.py::test_prune_dry_run_does_not_touch_hook_files PASSED
tests/integration/test_prune_hook_reconciliation_e2e.py::test_prune_hook_reconciliation_failure_does_not_abort_package_removal PASSED
tests/integration/test_prune_hook_reconciliation_e2e.py::test_prune_orchestration_call_is_load_bearing PASSED

============================== 13 passed in 2.55s ==============================

$ uv run ruff check <touched files>
All checks passed!

$ uv run ruff format --check <touched files>
6 files already formatted

$ uv run python -m pylint --disable=all --enable=R0801 \
    --min-similarity-lines=10 --fail-on=R0801 <touched src>
Your code has been rated at 10.00/10

Scenario Evidence

Scenario	Principle	Proof
Prune orphan while a hooked transitive dep remains via a keeper	P6	`test_prune_preserves_transitive_dependency_hooks`
`reconcile_after_removal` rebuilds depth>1 hooks from on-disk lockfile	P6	`test_reconcile_after_removal_rebuilds_transitive_hooks`
Uninstall Phase 2 rebuilds transitive hooks from in-memory lockfile	P6	`test_uninstall_phase2_reintegrates_transitive_hooks`
Helper prefers passed lockfile over stale disk	P6	`test_surviving_refs_prefer_passed_in_memory_lockfile`
Existing sibling / manual-entry prune behavior unchanged	P6	existing `test_prune_preserves_sibling_hooks_and_manual_entries`

How to test

Checkout fix/2254-transitive-hook-reintegration and run the pytest commands above.
Manually: install a project where direct A depends on hooked transitive B, plus sibling C; remove C from apm.yml, run apm prune; confirm B's hook entries remain in .claude/settings.json / sidecar.
Same graph: apm uninstall C (or another sibling); confirm B's hooks survive reintegration.
Confirm docs wording on prune/uninstall mentions direct and transitive lockfile survivors.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Shaurya2k06 · 2026-07-16T14:27:57Z

@microsoft-github-policy-service agree

danielmeppiel · 2026-07-17T00:34:11Z

APM Review Panel: `ship_now`

All in-scope panel findings were folded; transitive hook reintegration is now covered through prune and uninstall command paths.

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

The panel converged on the same read: this is a bounded reliability fix for the clear-then-rebuild hook owner. The shepherd fold added CLI-level uninstall coverage, threaded prune's already-mutated lockfile through reconcile_after_removal, removed a behavioral marker that tripped the ratchet suite, and re-ran CI to green. No panelist raised an in-scope remaining concern.

Aligned with: pragmatic_as_npm: uninstall/prune now preserve hooks from still-required transitive dependencies, matching package-manager expectations; oss_community_driven: community bug fix with reviewer feedback folded in the same PR.

Panel summary

Persona	N	Takeaway
Python Architect	0	Shared survivor helper remains the canonical rebuild-membership owner; pass-through symmetry folded.
CLI Logging Expert	0	No in-scope CLI output concerns.
DevX UX Expert	0	User-facing command behavior is predictable; docs are adequate.
Supply Chain Security Expert	0	Lockfile authority is preserved; no security concerns.
OSS Growth Hacker	1	Community reliability fix is good release-note material.
Doc Writer	0	Docs and changelog accurately describe transitive lockfile survivors.
Test Coverage Expert	0	CLI-level uninstall coverage was folded and mutation-checked.
Performance Expert	0	Current O(deps * targets) work is acceptable for this fix.

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

Recommendation

Ship now. The only remaining idea is future profiling/caching for very large monorepos, which crosses this PR's correctness-fix scope.

Folded in this run

(panel) Added CLI-level uninstall regression coverage for transitive hook rebuild -- resolved in 2e95edc3f0428bf98fdde6d396950339fe15d08f.
(panel) Threaded prune's post-removal lockfile through reconcile_after_removal -- resolved in 2e95edc3f0428bf98fdde6d396950339fe15d08f.
(CI recovery) Removed the module-level behavioral marker that failed the architecture ratchet suite -- resolved in 2e95edc3f0428bf98fdde6d396950339fe15d08f.

Copilot signals reviewed

Review 4714716652 -- NOT-LEGIT: quota notice, not a code-review signal.

Deferred (out-of-scope follow-ups)

(panel) Evaluate caching for hook-target membership in very large monorepos -- scope boundary: PR scope is a correctness fix for transitive hook reintegration; caching is a future performance optimization requiring profiling.

Regression-trap evidence (mutation-break gate)

tests/integration/test_prune_hook_reconciliation_e2e.py::test_uninstall_preserves_transitive_dependency_hooks -- replaced the lockfile package-dependency path in surviving_dependency_refs_for_reintegration with the manifest-direct fallback; test FAILED as expected; guard restored.

Lint contract

uv run --extra dev ruff check src/ tests/ and uv run --extra dev ruff format --check src/ tests/ both silent. Additional CI lint guards passed locally: pylint R0801, auth-signals, file length, YAML I/O, raw relative_to, and architecture boundaries.

CI

gh pr checks 2256 --repo microsoft/apm is green at head 2e95edc3f0428bf98fdde6d396950339fe15d08f: CI, CodeQL, Spec conformance, Merge Gate, Deploy Docs build, NOTICE Drift Check, and license/cla succeeded; deploy skipped as expected (after 1 CI fix iteration).

Mergeability status

Captured from gh pr view 2256 --json mergeable,mergeStateStatus,statusCheckRollup immediately after the last push of this run.

PR	head SHA	CEO stance	iters	folds	defers	Copilot rounds	CI	mergeable	mergeStateStatus	notes
#2256	`2e95edc`	ship_now	1	3	1	1	green	MERGEABLE	BLOCKED	awaiting maintainer review

Convergence

1 outer iteration; 1 Copilot round. Final panel recommendation: ship_now.

Ready for maintainer review.

Full per-persona findings

Python Architect: no remaining findings after lockfile pass-through fold.
CLI Logging Expert: unrelated diagnostics notes were outside this PR and not carried.
DevX UX Expert: docs wording nits were non-essential and covered by existing concise docs.
Supply Chain Security Expert: no remaining findings after lockfile pass-through fold.
OSS Growth Hacker: release-note/community shout-out idea only.
Auth Expert: inactive; no auth surface touched.
Doc Writer: no findings.
Test Coverage Expert: CLI-level uninstall coverage gap folded.
Performance Expert: future cache/profiling idea deferred.

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

Add exact type annotations, survivor-set debug breadcrumbs, clearer warning and docs wording, plus a small scaling guard for lockfile survivor mapping. Addresses shepherd panel follow-ups for microsoft#2256. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

danielmeppiel · 2026-07-17T08:24:56Z

APM Review Panel: `ship_now`

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

Closes the transitive-hooks gap: apm prune and apm uninstall now rebuild merged hooks from lockfile survivors, not just manifest-direct dependencies.

Nine panel lenses converged without dissent. The branch now centralizes post-removal survivor selection, keeps transitive hook owners intact, and documents the behavior. All in-scope follow-ups surfaced during shepherding were folded into the PR before this comment.

Aligned with: lockfile survivors drive hook reconstruction across prune/uninstall cycles; survivor-source debug logging improves auditability; package-manager behavior now matches the expectation that removing one package does not erase hooks from a still-needed transitive package.

Panel summary

Persona	R	N	Takeaway
Python Architect	0	1	Correct single-owner centralization; type-annotation nit folded.
CLI Logging Expert	1	1	Survivor-source debug logging and warning detail folded.
DevX UX Expert	0	2	No CLI surface concern; PR prose nits only.
Supply Chain Security Expert	0	1	No integrity regression; fallback auditability folded.
OSS Growth Hacker	0	2	Docs/CHANGELOG story is clean after merge with current main.
Doc Writer	0	0	Docs and CHANGELOG accurately describe the fixed behavior.
Test Coverage Expert	1	1	Coverage now includes unit, integration, CLI, mutation-break, and scaling guard evidence.
Performance Expert	1	2	Transitive expansion remains linear enough for this path; scaling guard folded.

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

Folded in this run

(panel) Added lockfile parameter type annotations on survivor reintegration paths -- resolved in b6a944985b77f77a39baf04dac4d334a35390b47.
(panel) Added survivor-set debug breadcrumbs and clearer best-effort warning text -- resolved in b6a944985b77f77a39baf04dac4d334a35390b47.
(panel) Clarified docs wording and added unit marker plus scaling guard -- resolved in b6a944985b77f77a39baf04dac4d334a35390b47.

Copilot signals reviewed

review 4714716652 -- NOT-LEGIT: quota-limit notice only; no code review signal to fold.

Regression-trap evidence (mutation-break gate)

tests/unit/test_surviving_deps_reintegration.py::test_reconcile_after_removal_rebuilds_transitive_hooks -- deleted lockfile survivor mapping in surviving_dependency_refs_for_reintegration; test FAILED as expected; guard restored.
tests/unit/test_surviving_deps_reintegration.py::test_uninstall_phase2_reintegrates_transitive_hooks -- deleted lockfile survivor mapping in surviving_dependency_refs_for_reintegration; test FAILED as expected; guard restored.
tests/integration/test_prune_hook_reconciliation_e2e.py::test_prune_preserves_transitive_dependency_hooks -- deleted lockfile survivor mapping in surviving_dependency_refs_for_reintegration; test FAILED as expected; guard restored.

Lint contract

uv run --extra dev ruff check src/ tests/, uv run --extra dev ruff format --check src/ tests/, pylint R0801, auth-signal lint, and architecture boundary lint all exit 0 on the pushed head.

CI

license/cla succeeded. Repository workflows for the pushed fork head are action_required awaiting maintainer approval, so CI-observed-green cannot be established by the shepherd in this run.

Mergeability status

Captured from gh pr view 2256 --repo microsoft/apm --json mergeable,mergeStateStatus,statusCheckRollup immediately after the last push of this run.

PR	head SHA	CEO stance	iters	folds	defers	Copilot rounds	CI	mergeable	mergeStateStatus	notes
#2256	`b6a9449`	ship_now	1	3	0	1	green-local/action-required-remote	MERGEABLE	BLOCKED	awaiting Actions approval

Convergence

1 outer iteration; 1 Copilot round. Final panel stance: ship_now after folded follow-ups. Terminal status is shepherd-blocked only because fork workflows require maintainer approval before CI can run.

Add exact type annotations, survivor-set debug breadcrumbs, clearer warning and docs wording, plus a small scaling guard for lockfile survivor mapping. Addresses shepherd panel follow-ups for microsoft#2256. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

danielmeppiel · 2026-07-17T08:46:27Z

CI drift resolution pushed for PR #2256.

Fixes applied:

Rebasing onto current main preserved the Transitive-dependency hook entries wiped but never rebuilt by uninstall/prune reconciliation #2254 transitive-hook reintegration behavior and the newer Hook wipe scans all merge targets but rebuild only re-populates canonical_targets (uninstall + prune) #2250 target-scoped hook wipe intent.
Trimmed one blank line from src/apm_cli/integration/hook_integrator.py; final count is 2100 lines, matching the tightened guardrail.
Registered tests/unit/test_surviving_deps_reintegration.py in tests/quality/critical_suite.toml as unit and updated the manifest count to 19, so its behavioral marker is manifest-owned.

Pushed head: 37a450c

Local validation passed:

uv run --extra dev ruff check src/ tests/
uv run --extra dev ruff format --check src/ tests/
YAML I/O, file-length, and raw relative_to guards (portable local equivalents; hook_integrator.py = 2100 lines)
uv run --extra dev python -m pylint --disable=all --enable=R0801 --min-similarity-lines=10 --fail-on=R0801 src/apm_cli/
bash scripts/lint-auth-signals.sh
uv run --extra dev pytest -q tests/quality/test_test_taxonomy.py::test_tm004_behavioral_markers_are_not_used_outside_manifest
uv run --extra dev pytest -q tests/unit/test_surviving_deps_reintegration.py

Re-probe after push: mergeStateStatus=BLOCKED, mergeable=MERGEABLE. GitHub currently shows only license/cla as passing in statusCheckRollup; CI jobs for the new fork head have not appeared yet.

Clear-then-rebuild after prune/uninstall wiped transitive packages' hooks because reintegration walked only apm.yml directs. Walk the post-removal lockfile (direct + transitive) via one shared helper so still-installed transitive hooks are restored. Closes microsoft#2254. Signed-off-by: shaurya2k06 <shaurya2k06@gmail.com>

Add CLI-level coverage for the uninstall clear-and-rebuild path so transitive hook survivors are proven through the user-facing command. Also threads prunes already-mutated lockfile into the shared reconcile owner, addressing panel follow-ups on end-to-end coverage and lockfile pass-through symmetry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add exact type annotations, survivor-set debug breadcrumbs, clearer warning and docs wording, plus a small scaling guard for lockfile survivor mapping. Addresses shepherd panel follow-ups for microsoft#2256. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Register the new reintegration tests in the critical suite manifest and trim hook_integrator.py to the tightened 2100-line limit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Main added a critical-suite module; rebasing this branch unions both additions to 20 entries. Update the TM002 count assertion to match. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

…traction (#2275) * fix(install): reconcile dropped-target merge-hook state on target contraction Narrowing a project's apm.yml targets: list (e.g. [claude, codex] -> [claude]) never cleaned up the dropped target's APM-owned merge-hook JSON config + apm-hooks.json ownership sidecar, even after apm install + apm prune. This left stale, APM-attributed hook state (.codex/hooks.json, .codex/apm-hooks.json, etc.) behind indefinitely. Root cause: HookIntegrator.sync_integration/reconcile_after_removal are intentionally scoped (#2250) to the SAME resolved targets the rebuild loop uses -- correct for that bug, but it permanently walls prune/ uninstall off from a target dropped from targets: entirely, since the rebuild loop never touches it again. Merge-hook config files are also deliberately excluded from deployed_files tracking, so the generic manifest_reconcile.py path-list reconciliation can never see this state either. Fix extends the two existing canonical owners rather than inventing a third authority: - install/manifest_reconcile.py gains reconcile_dropped_merge_hook_targets(), reusing the existing "allowed = active union declared" target-detection semantics (#2059 symmetry preserved for declared_targets=None). - integration/hook_integrator.py gains a new public method, HookIntegrator.reconcile_dropped_targets(), which fails closed on partial/malformed state (a sidecar-only orphan, or malformed native/ sidecar JSON, is left byte-identical with an actionable warning, never silently swallowed) -- stricter than the existing best-effort posture _clean_apm_entries_from_json keeps for its unchanged prune/ uninstall callers. Wired at the top of LockfileBuilder.build_and_save() (every non- lockfile_only apm install, independent of whether any package installed/orphaned this run) and at the end of manifest_reconcile.reconcile_deployed_state() (apm compile/update). Unreachable under apm install --dry-run and apm lock. HookIntegrator.reconcile_dropped_targets' implementation is split into a private sibling module (integration/_hook_dropped_targets.py) solely to respect this file's CI line-length guardrail (2100 lines) -- this is NOT a second target-cleanup authority: it is invoked exclusively from the one public HookIntegrator method, and all JSON mutation still runs through the existing, unchanged HookIntegrator._clean_apm_entries_from_json primitive. Adds a semantic AST-based lint guard (scripts/check_hook_config_write_owner.py, wired into lint-architecture-boundaries.sh as AC15) that flags any write-mutating call (open/.open/.write_text/.write_bytes/.unlink) on a merge-hook config/sidecar path composed outside hook_integrator.py, plus a lexical guard keeping prune/uninstall from calling the new target-contraction APIs directly (#2250 scope). Both guards have regression/mutation/positive-control tests in test_architecture_authorities.py. Full test matrix (unit + component + CLI-level integration-contract, CliRunner-driven with a monkeypatched downloader, no real network): main RED-then-GREEN scenario, retained-target/user-owned-entries/no-op negative twins, idempotence, final-uninstall composition, prune-alone negative control, dry-run non-reconciliation, compile/update parity, narrow-and-remove-last-dependency lifecycle-timing regression, the active/declared union-rule twins (transient --target override, declared_targets=None legacy no-op), native-only/sidecar-only-orphan/ malformed-JSON fail-closed coverage. Evidence: independently reproduced on current main via a real local-Git hook package fixture (Claude + Codex targets, narrowed to Claude); .codex/apm-hooks.json survived install + prune with its ownership marker intact, confirming the defect. This mirrors (but does not modify, and is not based on) PR #2266's covering-array fixture row fixture-claude-codex-hook-narrow, whose currently-committed assertion for that row is narrower than this evidence and is left untouched here. Mutation-break check (both call sites, performed manually before this commit, restored cleanly): stubbing either wiring call to a no-op reverts the main scenario test to RED with the exact expected assertion failure; restoring the call returns it to GREEN. Performance: O(k) per install/compile/update run, k = size of the fixed _MERGE_HOOK_TARGETS catalog (6), independent of dependency graph size -- at most one JSON read + one JSON write + one sidecar read/unlink per dropped target. Closes #2253. Out of scope: #2256 (unrelated transitive-hook-rebuild gap, #2254). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * spec: add req-lk-021 for dropped-target merge-hook reconciliation The Mode B silent-extension gate correctly fired on the prior commit: this fix adds genuinely new normative behavior under a critical path (install/integration) with no spec citation. req-lk-020 (inactive- target lockfile reconciliation) does not cover it -- merge-hook JSON config and its apm-hooks.json ownership sidecar are deliberately excluded from deployed_files/local_deployed_files tracking, so req-lk-020's literal text never reaches this state category. Adds req-lk-021 (Section 5.2, consumer MUST), extending req-lk-020's preserve/remove decision to merge-based hook configuration: remove only consumer-owned entries (and an ownership record emptied by that removal) attributable to a dropped target, preserve entries that don't carry consumer ownership regardless of target, and preserve state for targets still attributable per req-lk-020's own (a)-(c) test. Closes the #2253 Mode-B gate. - docs/src/content/docs/specs/openapm-v0.1.md: new req-lk-021 anchor + body text, Section 11.3.2 enumeration, Appendix C row, Section 1.3 + Appendix C statement-count bump (100 -> 101), Appendix D revision history entry (0.1.16). - docs/public/specs/manifests/openapm-v0.1.requirements.yml: matching manifest row. - tests/spec_conformance/test_lockfile_reqs.py: new test_dropped_target_merge_hook_state_reconciled_fail_safe, calling HookIntegrator.reconcile_dropped_targets directly (matching the existing req-lk-020 test's direct-call style) and asserting the full preserve/remove matrix: dropped-target owned entry removed, user-authored entry in the same file preserved, ownership sidecar removed once empty, retained target's state byte-identical. - CONFORMANCE.{json,md}: regenerated via `python -m tests.spec_conformance.gen_statement` (101 total requirements, 72 active consumer statements). Verified: orphan_check passes (101 requirements aligned across anchors/manifest/Appendix C/pytest markers); full tests/spec_conformance/ suite passes (141 passed, 2 skipped); mode_b_detector.sh passes locally against origin/main. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fold: address apm-review-panel recommended-severity findings on #2275 Folds the "recommended"-severity findings from the apm-review-panel advisory run into this branch (all panelists returned zero blocking findings; these are the highest-signal follow-ups the CEO synthesis curated). Spec text (req-lk-021, doc-writer): - The precondition clause did not textually cover the sidecar-only- orphan cleanup case (native merge-hook config already absent, only the apm-hooks.json ownership record remains) even though _reconcile_sidecar_only_orphan implements and tests exactly that case. Added a sentence binding the same preserve-or-remove decision to an orphaned ownership record. - The fail-closed-on-malformed-JSON guarantee -- called out in this PR's own Trade-offs section as a deliberate posture distinct from prune/uninstall's best-effort handling, and covered by two dedicated test scenarios -- was not codified in the requirement text, unlike sibling requirements that write such guarantees directly into their normative text (req-tg-006, req-lk-013). Added a matching sentence. - Extended test_dropped_target_merge_hook_state_reconciled_fail_safe's assert_spec_contains needles to bind both new sentences to the spec body (exact line-wrapped substrings, matching the existing pattern). - No anchor/manifest/Appendix-C change needed (same req-lk-021 id); orphan_check and gen_statement both confirm no drift. CHANGELOG (oss-growth-hacker, doc-writer): - Tightened the #2253 entry to lead with user impact, drop internal class::method names and the PR #2266 evidence-provenance clause, and keep a single closing issue reference, matching the voice of its sibling Fixed entries. CLI output UX (cli-logging-expert, devx-ux-expert): - Added a `_rich_warning` alongside each existing `_log.warning` in `_hook_dropped_targets.py` (malformed native JSON, malformed sidecar JSON) so the fail-closed diagnostic reaches the user's console during `apm install`/`compile`/`update`, matching the dual-channel pattern already used elsewhere in `hook_integrator.py` for forward-command warnings. Reworded both messages to name a concrete recovery action ("delete or fix the file manually, then re-run apm install") instead of the vaguer "manual review". Docs consolidation (doc-writer, devx-ux-expert): - install.md/update.md/compile.md repeated the same merge-hook filename parenthetical verbatim; trimmed update.md/compile.md to a short clause cross-linking to install.md's Notes section and to hooks-and-commands.md's canonical Hooks section instead of re-enumerating file names. - hooks-and-commands.md's "created and cleaned up automatically" sidecar claim is exactly what #2253 falsified for dropped targets; added a cross-reference to install.md's target-contraction note so a producer reading the canonical hooks page has a path to the exact contract. - prune.md's remedy sentence used internal "lifecycle owner" framing; reworded to name apm install/compile/update as the remedy directly. - install.md's "not apm lock" parenthetical was noise for most readers; moved to its own trailing sentence. Validation: - tests/unit/integration/test_hook_integrator_reconcile_dropped_targets.py, tests/unit/install/test_manifest_reconcile_dropped_hook_targets.py, tests/integration/test_hook_target_contraction_reconciliation.py, tests/integration/test_architecture_authorities.py, tests/spec_conformance/ -- 206 passed, 2 skipped. - orphan_check.py -- 101 requirements aligned (unchanged). - gen_statement -- CONFORMANCE.json/CONFORMANCE.md byte-identical (no regeneration diff; prose-only spec edit, no id/count change). - Full CI-mirror lint chain (ruff check, ruff format --check, pylint R0801 10.00/10, lint-auth-signals.sh, lint-architecture-boundaries.sh incl. AC15) -- all clean. Not folded (deferred as lower-priority follow-ups per the panel comment, since they are nit-severity or carry marginal regression-trap value): the AST checker's single-string-literal composition blind spot, os.remove/Path.rename coverage, an explicit OWNER_MODULES allowlist naming _hook_dropped_targets.py, the HookIntegrator() staticmethod simplification, the AST checker's pre-filter performance optimization, and an explicit apm-update parity test (the compile test already transitively proves the shared reconcile_deployed_state entry point). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * docs(spec): fold req-lk-021 guardian feedback Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 25bf0dc2-94a9-44e7-94da-405b27400fc5 * docs(spec): fold apm-spec-guardian F1 (Section 5.7 enumeration) Adds req-lk-019 and req-lk-021 to the Section 5.7 Consumer MUST enumeration in ascending sequence, per the spec-guardian advisory (issuecomment-5000654065, fold_and_ship, 7.75/10). F2-F4 from the same advisory were already folded by a prior commit on this branch (970e628). F5-F11 are explicitly deferred to v0.1.1 per the advisory. Verified after fold: - orphan_check: 101 requirements aligned (anchors/manifest/Appendix C/markers) - tests/spec_conformance/: 141 passed, 2 skipped - mode_b_detector.sh: spec-concurrent edit short-circuit, exit 0 - gen_statement regeneration: zero diff on CONFORMANCE.{json,md} (no hand-edit needed or performed) - No product/test code touched -- diff is 1 file, docs/spec only Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

…traction Narrowing a project's apm.yml targets: list (e.g. [claude, codex] -> [claude]) never cleaned up the dropped target's APM-owned merge-hook JSON config + apm-hooks.json ownership sidecar, even after apm install + apm prune. This left stale, APM-attributed hook state (.codex/hooks.json, .codex/apm-hooks.json, etc.) behind indefinitely. Root cause: HookIntegrator.sync_integration/reconcile_after_removal are intentionally scoped (#2250) to the SAME resolved targets the rebuild loop uses -- correct for that bug, but it permanently walls prune/ uninstall off from a target dropped from targets: entirely, since the rebuild loop never touches it again. Merge-hook config files are also deliberately excluded from deployed_files tracking, so the generic manifest_reconcile.py path-list reconciliation can never see this state either. Fix extends the two existing canonical owners rather than inventing a third authority: - install/manifest_reconcile.py gains reconcile_dropped_merge_hook_targets(), reusing the existing "allowed = active union declared" target-detection semantics (#2059 symmetry preserved for declared_targets=None). - integration/hook_integrator.py gains a new public method, HookIntegrator.reconcile_dropped_targets(), which fails closed on partial/malformed state (a sidecar-only orphan, or malformed native/ sidecar JSON, is left byte-identical with an actionable warning, never silently swallowed) -- stricter than the existing best-effort posture _clean_apm_entries_from_json keeps for its unchanged prune/ uninstall callers. Wired at the top of LockfileBuilder.build_and_save() (every non- lockfile_only apm install, independent of whether any package installed/orphaned this run) and at the end of manifest_reconcile.reconcile_deployed_state() (apm compile/update). Unreachable under apm install --dry-run and apm lock. HookIntegrator.reconcile_dropped_targets' implementation is split into a private sibling module (integration/_hook_dropped_targets.py) solely to respect this file's CI line-length guardrail (2100 lines) -- this is NOT a second target-cleanup authority: it is invoked exclusively from the one public HookIntegrator method, and all JSON mutation still runs through the existing, unchanged HookIntegrator._clean_apm_entries_from_json primitive. Adds a semantic AST-based lint guard (scripts/check_hook_config_write_owner.py, wired into lint-architecture-boundaries.sh as AC15) that flags any write-mutating call (open/.open/.write_text/.write_bytes/.unlink) on a merge-hook config/sidecar path composed outside hook_integrator.py, plus a lexical guard keeping prune/uninstall from calling the new target-contraction APIs directly (#2250 scope). Both guards have regression/mutation/positive-control tests in test_architecture_authorities.py. Full test matrix (unit + component + CLI-level integration-contract, CliRunner-driven with a monkeypatched downloader, no real network): main RED-then-GREEN scenario, retained-target/user-owned-entries/no-op negative twins, idempotence, final-uninstall composition, prune-alone negative control, dry-run non-reconciliation, compile/update parity, narrow-and-remove-last-dependency lifecycle-timing regression, the active/declared union-rule twins (transient --target override, declared_targets=None legacy no-op), native-only/sidecar-only-orphan/ malformed-JSON fail-closed coverage. Evidence: independently reproduced on current main via a real local-Git hook package fixture (Claude + Codex targets, narrowed to Claude); .codex/apm-hooks.json survived install + prune with its ownership marker intact, confirming the defect. This mirrors (but does not modify, and is not based on) PR #2266's covering-array fixture row fixture-claude-codex-hook-narrow, whose currently-committed assertion for that row is narrower than this evidence and is left untouched here. Mutation-break check (both call sites, performed manually before this commit, restored cleanly): stubbing either wiring call to a no-op reverts the main scenario test to RED with the exact expected assertion failure; restoring the call returns it to GREEN. Performance: O(k) per install/compile/update run, k = size of the fixed _MERGE_HOOK_TARGETS catalog (6), independent of dependency graph size -- at most one JSON read + one JSON write + one sidecar read/unlink per dropped target. Closes #2253. Out of scope: #2256 (unrelated transitive-hook-rebuild gap, #2254). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Preserve lockfile-wide survivor rebuilding alongside dropped-target reconciliation, user-scope hook rewriting, and unconditional per-file routing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 90103f4e-afe4-4b0e-898a-35d98f0fa6c9

danielmeppiel

Reviewed after additive resolution against merged #2259. The conflict preserves transitive survivor reintegration plus all current HookIntegrator owner contracts; focused regression, mutation-break, architecture, and full lint evidence are green.

* test(lifecycle): add shared state substrate Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 465e6b19-4416-416f-a0f3-f3b1cdb1181a * test(lifecycle): fold substrate review findings Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 465e6b19-4416-416f-a0f3-f3b1cdb1181a * test(lifecycle): reject unknown config kinds Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 465e6b19-4416-416f-a0f3-f3b1cdb1181a * test(git): add isolated URL rewrite fixture Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 465e6b19-4416-416f-a0f3-f3b1cdb1181a * test(lifecycle): complete shared foundation API Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 465e6b19-4416-416f-a0f3-f3b1cdb1181a * test(lifecycle): fold final panel followups Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 465e6b19-4416-416f-a0f3-f3b1cdb1181a * test: cover primitive target lifecycle matrix Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c032ae2-9e7a-4876-a7d8-77374ece5d64 * test: cover widened target lifecycle Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c032ae2-9e7a-4876-a7d8-77374ece5d64 * fix(install): reconcile dropped-target merge-hook state on target contraction Narrowing a project's apm.yml targets: list (e.g. [claude, codex] -> [claude]) never cleaned up the dropped target's APM-owned merge-hook JSON config + apm-hooks.json ownership sidecar, even after apm install + apm prune. This left stale, APM-attributed hook state (.codex/hooks.json, .codex/apm-hooks.json, etc.) behind indefinitely. Root cause: HookIntegrator.sync_integration/reconcile_after_removal are intentionally scoped (#2250) to the SAME resolved targets the rebuild loop uses -- correct for that bug, but it permanently walls prune/ uninstall off from a target dropped from targets: entirely, since the rebuild loop never touches it again. Merge-hook config files are also deliberately excluded from deployed_files tracking, so the generic manifest_reconcile.py path-list reconciliation can never see this state either. Fix extends the two existing canonical owners rather than inventing a third authority: - install/manifest_reconcile.py gains reconcile_dropped_merge_hook_targets(), reusing the existing "allowed = active union declared" target-detection semantics (#2059 symmetry preserved for declared_targets=None). - integration/hook_integrator.py gains a new public method, HookIntegrator.reconcile_dropped_targets(), which fails closed on partial/malformed state (a sidecar-only orphan, or malformed native/ sidecar JSON, is left byte-identical with an actionable warning, never silently swallowed) -- stricter than the existing best-effort posture _clean_apm_entries_from_json keeps for its unchanged prune/ uninstall callers. Wired at the top of LockfileBuilder.build_and_save() (every non- lockfile_only apm install, independent of whether any package installed/orphaned this run) and at the end of manifest_reconcile.reconcile_deployed_state() (apm compile/update). Unreachable under apm install --dry-run and apm lock. HookIntegrator.reconcile_dropped_targets' implementation is split into a private sibling module (integration/_hook_dropped_targets.py) solely to respect this file's CI line-length guardrail (2100 lines) -- this is NOT a second target-cleanup authority: it is invoked exclusively from the one public HookIntegrator method, and all JSON mutation still runs through the existing, unchanged HookIntegrator._clean_apm_entries_from_json primitive. Adds a semantic AST-based lint guard (scripts/check_hook_config_write_owner.py, wired into lint-architecture-boundaries.sh as AC15) that flags any write-mutating call (open/.open/.write_text/.write_bytes/.unlink) on a merge-hook config/sidecar path composed outside hook_integrator.py, plus a lexical guard keeping prune/uninstall from calling the new target-contraction APIs directly (#2250 scope). Both guards have regression/mutation/positive-control tests in test_architecture_authorities.py. Full test matrix (unit + component + CLI-level integration-contract, CliRunner-driven with a monkeypatched downloader, no real network): main RED-then-GREEN scenario, retained-target/user-owned-entries/no-op negative twins, idempotence, final-uninstall composition, prune-alone negative control, dry-run non-reconciliation, compile/update parity, narrow-and-remove-last-dependency lifecycle-timing regression, the active/declared union-rule twins (transient --target override, declared_targets=None legacy no-op), native-only/sidecar-only-orphan/ malformed-JSON fail-closed coverage. Evidence: independently reproduced on current main via a real local-Git hook package fixture (Claude + Codex targets, narrowed to Claude); .codex/apm-hooks.json survived install + prune with its ownership marker intact, confirming the defect. This mirrors (but does not modify, and is not based on) PR #2266's covering-array fixture row fixture-claude-codex-hook-narrow, whose currently-committed assertion for that row is narrower than this evidence and is left untouched here. Mutation-break check (both call sites, performed manually before this commit, restored cleanly): stubbing either wiring call to a no-op reverts the main scenario test to RED with the exact expected assertion failure; restoring the call returns it to GREEN. Performance: O(k) per install/compile/update run, k = size of the fixed _MERGE_HOOK_TARGETS catalog (6), independent of dependency graph size -- at most one JSON read + one JSON write + one sidecar read/unlink per dropped target. Closes #2253. Out of scope: #2256 (unrelated transitive-hook-rebuild gap, #2254). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * spec: add req-lk-021 for dropped-target merge-hook reconciliation The Mode B silent-extension gate correctly fired on the prior commit: this fix adds genuinely new normative behavior under a critical path (install/integration) with no spec citation. req-lk-020 (inactive- target lockfile reconciliation) does not cover it -- merge-hook JSON config and its apm-hooks.json ownership sidecar are deliberately excluded from deployed_files/local_deployed_files tracking, so req-lk-020's literal text never reaches this state category. Adds req-lk-021 (Section 5.2, consumer MUST), extending req-lk-020's preserve/remove decision to merge-based hook configuration: remove only consumer-owned entries (and an ownership record emptied by that removal) attributable to a dropped target, preserve entries that don't carry consumer ownership regardless of target, and preserve state for targets still attributable per req-lk-020's own (a)-(c) test. Closes the #2253 Mode-B gate. - docs/src/content/docs/specs/openapm-v0.1.md: new req-lk-021 anchor + body text, Section 11.3.2 enumeration, Appendix C row, Section 1.3 + Appendix C statement-count bump (100 -> 101), Appendix D revision history entry (0.1.16). - docs/public/specs/manifests/openapm-v0.1.requirements.yml: matching manifest row. - tests/spec_conformance/test_lockfile_reqs.py: new test_dropped_target_merge_hook_state_reconciled_fail_safe, calling HookIntegrator.reconcile_dropped_targets directly (matching the existing req-lk-020 test's direct-call style) and asserting the full preserve/remove matrix: dropped-target owned entry removed, user-authored entry in the same file preserved, ownership sidecar removed once empty, retained target's state byte-identical. - CONFORMANCE.{json,md}: regenerated via `python -m tests.spec_conformance.gen_statement` (101 total requirements, 72 active consumer statements). Verified: orphan_check passes (101 requirements aligned across anchors/manifest/Appendix C/pytest markers); full tests/spec_conformance/ suite passes (141 passed, 2 skipped); mode_b_detector.sh passes locally against origin/main. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fold: address apm-review-panel recommended-severity findings on #2275 Folds the "recommended"-severity findings from the apm-review-panel advisory run into this branch (all panelists returned zero blocking findings; these are the highest-signal follow-ups the CEO synthesis curated). Spec text (req-lk-021, doc-writer): - The precondition clause did not textually cover the sidecar-only- orphan cleanup case (native merge-hook config already absent, only the apm-hooks.json ownership record remains) even though _reconcile_sidecar_only_orphan implements and tests exactly that case. Added a sentence binding the same preserve-or-remove decision to an orphaned ownership record. - The fail-closed-on-malformed-JSON guarantee -- called out in this PR's own Trade-offs section as a deliberate posture distinct from prune/uninstall's best-effort handling, and covered by two dedicated test scenarios -- was not codified in the requirement text, unlike sibling requirements that write such guarantees directly into their normative text (req-tg-006, req-lk-013). Added a matching sentence. - Extended test_dropped_target_merge_hook_state_reconciled_fail_safe's assert_spec_contains needles to bind both new sentences to the spec body (exact line-wrapped substrings, matching the existing pattern). - No anchor/manifest/Appendix-C change needed (same req-lk-021 id); orphan_check and gen_statement both confirm no drift. CHANGELOG (oss-growth-hacker, doc-writer): - Tightened the #2253 entry to lead with user impact, drop internal class::method names and the PR #2266 evidence-provenance clause, and keep a single closing issue reference, matching the voice of its sibling Fixed entries. CLI output UX (cli-logging-expert, devx-ux-expert): - Added a `_rich_warning` alongside each existing `_log.warning` in `_hook_dropped_targets.py` (malformed native JSON, malformed sidecar JSON) so the fail-closed diagnostic reaches the user's console during `apm install`/`compile`/`update`, matching the dual-channel pattern already used elsewhere in `hook_integrator.py` for forward-command warnings. Reworded both messages to name a concrete recovery action ("delete or fix the file manually, then re-run apm install") instead of the vaguer "manual review". Docs consolidation (doc-writer, devx-ux-expert): - install.md/update.md/compile.md repeated the same merge-hook filename parenthetical verbatim; trimmed update.md/compile.md to a short clause cross-linking to install.md's Notes section and to hooks-and-commands.md's canonical Hooks section instead of re-enumerating file names. - hooks-and-commands.md's "created and cleaned up automatically" sidecar claim is exactly what #2253 falsified for dropped targets; added a cross-reference to install.md's target-contraction note so a producer reading the canonical hooks page has a path to the exact contract. - prune.md's remedy sentence used internal "lifecycle owner" framing; reworded to name apm install/compile/update as the remedy directly. - install.md's "not apm lock" parenthetical was noise for most readers; moved to its own trailing sentence. Validation: - tests/unit/integration/test_hook_integrator_reconcile_dropped_targets.py, tests/unit/install/test_manifest_reconcile_dropped_hook_targets.py, tests/integration/test_hook_target_contraction_reconciliation.py, tests/integration/test_architecture_authorities.py, tests/spec_conformance/ -- 206 passed, 2 skipped. - orphan_check.py -- 101 requirements aligned (unchanged). - gen_statement -- CONFORMANCE.json/CONFORMANCE.md byte-identical (no regeneration diff; prose-only spec edit, no id/count change). - Full CI-mirror lint chain (ruff check, ruff format --check, pylint R0801 10.00/10, lint-auth-signals.sh, lint-architecture-boundaries.sh incl. AC15) -- all clean. Not folded (deferred as lower-priority follow-ups per the panel comment, since they are nit-severity or carry marginal regression-trap value): the AST checker's single-string-literal composition blind spot, os.remove/Path.rename coverage, an explicit OWNER_MODULES allowlist naming _hook_dropped_targets.py, the HookIntegrator() staticmethod simplification, the AST checker's pre-filter performance optimization, and an explicit apm-update parity test (the compile test already transitively proves the shared reconcile_deployed_state entry point). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * docs(spec): fold req-lk-021 guardian feedback Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 25bf0dc2-94a9-44e7-94da-405b27400fc5 --------- Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Preserve the #2256 quality inventory alongside the live and packaged topology entries. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bfde2bb5-f5f1-425f-bd4b-1cda0c95819f

* fix(install): reconcile dropped-target merge-hook state on target contraction Narrowing a project's apm.yml targets: list (e.g. [claude, codex] -> [claude]) never cleaned up the dropped target's APM-owned merge-hook JSON config + apm-hooks.json ownership sidecar, even after apm install + apm prune. This left stale, APM-attributed hook state (.codex/hooks.json, .codex/apm-hooks.json, etc.) behind indefinitely. Root cause: HookIntegrator.sync_integration/reconcile_after_removal are intentionally scoped (#2250) to the SAME resolved targets the rebuild loop uses -- correct for that bug, but it permanently walls prune/ uninstall off from a target dropped from targets: entirely, since the rebuild loop never touches it again. Merge-hook config files are also deliberately excluded from deployed_files tracking, so the generic manifest_reconcile.py path-list reconciliation can never see this state either. Fix extends the two existing canonical owners rather than inventing a third authority: - install/manifest_reconcile.py gains reconcile_dropped_merge_hook_targets(), reusing the existing "allowed = active union declared" target-detection semantics (#2059 symmetry preserved for declared_targets=None). - integration/hook_integrator.py gains a new public method, HookIntegrator.reconcile_dropped_targets(), which fails closed on partial/malformed state (a sidecar-only orphan, or malformed native/ sidecar JSON, is left byte-identical with an actionable warning, never silently swallowed) -- stricter than the existing best-effort posture _clean_apm_entries_from_json keeps for its unchanged prune/ uninstall callers. Wired at the top of LockfileBuilder.build_and_save() (every non- lockfile_only apm install, independent of whether any package installed/orphaned this run) and at the end of manifest_reconcile.reconcile_deployed_state() (apm compile/update). Unreachable under apm install --dry-run and apm lock. HookIntegrator.reconcile_dropped_targets' implementation is split into a private sibling module (integration/_hook_dropped_targets.py) solely to respect this file's CI line-length guardrail (2100 lines) -- this is NOT a second target-cleanup authority: it is invoked exclusively from the one public HookIntegrator method, and all JSON mutation still runs through the existing, unchanged HookIntegrator._clean_apm_entries_from_json primitive. Adds a semantic AST-based lint guard (scripts/check_hook_config_write_owner.py, wired into lint-architecture-boundaries.sh as AC15) that flags any write-mutating call (open/.open/.write_text/.write_bytes/.unlink) on a merge-hook config/sidecar path composed outside hook_integrator.py, plus a lexical guard keeping prune/uninstall from calling the new target-contraction APIs directly (#2250 scope). Both guards have regression/mutation/positive-control tests in test_architecture_authorities.py. Full test matrix (unit + component + CLI-level integration-contract, CliRunner-driven with a monkeypatched downloader, no real network): main RED-then-GREEN scenario, retained-target/user-owned-entries/no-op negative twins, idempotence, final-uninstall composition, prune-alone negative control, dry-run non-reconciliation, compile/update parity, narrow-and-remove-last-dependency lifecycle-timing regression, the active/declared union-rule twins (transient --target override, declared_targets=None legacy no-op), native-only/sidecar-only-orphan/ malformed-JSON fail-closed coverage. Evidence: independently reproduced on current main via a real local-Git hook package fixture (Claude + Codex targets, narrowed to Claude); .codex/apm-hooks.json survived install + prune with its ownership marker intact, confirming the defect. This mirrors (but does not modify, and is not based on) PR #2266's covering-array fixture row fixture-claude-codex-hook-narrow, whose currently-committed assertion for that row is narrower than this evidence and is left untouched here. Mutation-break check (both call sites, performed manually before this commit, restored cleanly): stubbing either wiring call to a no-op reverts the main scenario test to RED with the exact expected assertion failure; restoring the call returns it to GREEN. Performance: O(k) per install/compile/update run, k = size of the fixed _MERGE_HOOK_TARGETS catalog (6), independent of dependency graph size -- at most one JSON read + one JSON write + one sidecar read/unlink per dropped target. Closes #2253. Out of scope: #2256 (unrelated transitive-hook-rebuild gap, #2254). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * spec: add req-lk-021 for dropped-target merge-hook reconciliation The Mode B silent-extension gate correctly fired on the prior commit: this fix adds genuinely new normative behavior under a critical path (install/integration) with no spec citation. req-lk-020 (inactive- target lockfile reconciliation) does not cover it -- merge-hook JSON config and its apm-hooks.json ownership sidecar are deliberately excluded from deployed_files/local_deployed_files tracking, so req-lk-020's literal text never reaches this state category. Adds req-lk-021 (Section 5.2, consumer MUST), extending req-lk-020's preserve/remove decision to merge-based hook configuration: remove only consumer-owned entries (and an ownership record emptied by that removal) attributable to a dropped target, preserve entries that don't carry consumer ownership regardless of target, and preserve state for targets still attributable per req-lk-020's own (a)-(c) test. Closes the #2253 Mode-B gate. - docs/src/content/docs/specs/openapm-v0.1.md: new req-lk-021 anchor + body text, Section 11.3.2 enumeration, Appendix C row, Section 1.3 + Appendix C statement-count bump (100 -> 101), Appendix D revision history entry (0.1.16). - docs/public/specs/manifests/openapm-v0.1.requirements.yml: matching manifest row. - tests/spec_conformance/test_lockfile_reqs.py: new test_dropped_target_merge_hook_state_reconciled_fail_safe, calling HookIntegrator.reconcile_dropped_targets directly (matching the existing req-lk-020 test's direct-call style) and asserting the full preserve/remove matrix: dropped-target owned entry removed, user-authored entry in the same file preserved, ownership sidecar removed once empty, retained target's state byte-identical. - CONFORMANCE.{json,md}: regenerated via `python -m tests.spec_conformance.gen_statement` (101 total requirements, 72 active consumer statements). Verified: orphan_check passes (101 requirements aligned across anchors/manifest/Appendix C/pytest markers); full tests/spec_conformance/ suite passes (141 passed, 2 skipped); mode_b_detector.sh passes locally against origin/main. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fold: address apm-review-panel recommended-severity findings on #2275 Folds the "recommended"-severity findings from the apm-review-panel advisory run into this branch (all panelists returned zero blocking findings; these are the highest-signal follow-ups the CEO synthesis curated). Spec text (req-lk-021, doc-writer): - The precondition clause did not textually cover the sidecar-only- orphan cleanup case (native merge-hook config already absent, only the apm-hooks.json ownership record remains) even though _reconcile_sidecar_only_orphan implements and tests exactly that case. Added a sentence binding the same preserve-or-remove decision to an orphaned ownership record. - The fail-closed-on-malformed-JSON guarantee -- called out in this PR's own Trade-offs section as a deliberate posture distinct from prune/uninstall's best-effort handling, and covered by two dedicated test scenarios -- was not codified in the requirement text, unlike sibling requirements that write such guarantees directly into their normative text (req-tg-006, req-lk-013). Added a matching sentence. - Extended test_dropped_target_merge_hook_state_reconciled_fail_safe's assert_spec_contains needles to bind both new sentences to the spec body (exact line-wrapped substrings, matching the existing pattern). - No anchor/manifest/Appendix-C change needed (same req-lk-021 id); orphan_check and gen_statement both confirm no drift. CHANGELOG (oss-growth-hacker, doc-writer): - Tightened the #2253 entry to lead with user impact, drop internal class::method names and the PR #2266 evidence-provenance clause, and keep a single closing issue reference, matching the voice of its sibling Fixed entries. CLI output UX (cli-logging-expert, devx-ux-expert): - Added a `_rich_warning` alongside each existing `_log.warning` in `_hook_dropped_targets.py` (malformed native JSON, malformed sidecar JSON) so the fail-closed diagnostic reaches the user's console during `apm install`/`compile`/`update`, matching the dual-channel pattern already used elsewhere in `hook_integrator.py` for forward-command warnings. Reworded both messages to name a concrete recovery action ("delete or fix the file manually, then re-run apm install") instead of the vaguer "manual review". Docs consolidation (doc-writer, devx-ux-expert): - install.md/update.md/compile.md repeated the same merge-hook filename parenthetical verbatim; trimmed update.md/compile.md to a short clause cross-linking to install.md's Notes section and to hooks-and-commands.md's canonical Hooks section instead of re-enumerating file names. - hooks-and-commands.md's "created and cleaned up automatically" sidecar claim is exactly what #2253 falsified for dropped targets; added a cross-reference to install.md's target-contraction note so a producer reading the canonical hooks page has a path to the exact contract. - prune.md's remedy sentence used internal "lifecycle owner" framing; reworded to name apm install/compile/update as the remedy directly. - install.md's "not apm lock" parenthetical was noise for most readers; moved to its own trailing sentence. Validation: - tests/unit/integration/test_hook_integrator_reconcile_dropped_targets.py, tests/unit/install/test_manifest_reconcile_dropped_hook_targets.py, tests/integration/test_hook_target_contraction_reconciliation.py, tests/integration/test_architecture_authorities.py, tests/spec_conformance/ -- 206 passed, 2 skipped. - orphan_check.py -- 101 requirements aligned (unchanged). - gen_statement -- CONFORMANCE.json/CONFORMANCE.md byte-identical (no regeneration diff; prose-only spec edit, no id/count change). - Full CI-mirror lint chain (ruff check, ruff format --check, pylint R0801 10.00/10, lint-auth-signals.sh, lint-architecture-boundaries.sh incl. AC15) -- all clean. Not folded (deferred as lower-priority follow-ups per the panel comment, since they are nit-severity or carry marginal regression-trap value): the AST checker's single-string-literal composition blind spot, os.remove/Path.rename coverage, an explicit OWNER_MODULES allowlist naming _hook_dropped_targets.py, the HookIntegrator() staticmethod simplification, the AST checker's pre-filter performance optimization, and an explicit apm-update parity test (the compile test already transitively proves the shared reconcile_deployed_state entry point). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * docs(spec): fold req-lk-021 guardian feedback Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 25bf0dc2-94a9-44e7-94da-405b27400fc5 * fix(install): contract generic shared target rows Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1cd014c7-c4ea-4fad-b943-b405a9de4e55 * docs(changelog): note shared target contraction fix Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1cd014c7-c4ea-4fad-b943-b405a9de4e55 * fix(lockfile): preserve reconciled target rows Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1cd014c7-c4ea-4fad-b943-b405a9de4e55 * docs(changelog): clarify shared skill recovery Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1cd014c7-c4ea-4fad-b943-b405a9de4e55 * fix(lockfile): retain canonical locator on refresh Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1cd014c7-c4ea-4fad-b943-b405a9de4e55 * fix(lockfile): route orphan retention through codec Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1cd014c7-c4ea-4fad-b943-b405a9de4e55 * chore(ci): record target reconciliation spec waiver apm-spec-waiver: Implements existing req-lk-020 reconciliation semantics without a new normative contract. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1cd014c7-c4ea-4fad-b943-b405a9de4e55 * fix(cleanup): initialize retained state for fixture contexts Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1cd014c7-c4ea-4fad-b943-b405a9de4e55 --------- Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Shaurya2k06 requested a review from danielmeppiel as a code owner July 16, 2026 14:26

Copilot AI review requested due to automatic review settings July 16, 2026 14:26

Shaurya2k06 requested a review from sergio-sisternes-epam as a code owner July 16, 2026 14:26

Copilot AI reviewed Jul 16, 2026

danielmeppiel force-pushed the fix/2254-transitive-hook-reintegration branch from f0590a0 to 2e95edc Compare July 17, 2026 00:29

danielmeppiel force-pushed the fix/2254-transitive-hook-reintegration branch from b6a9449 to 37a450c Compare July 17, 2026 08:44

Shaurya2k06 and others added 5 commits July 17, 2026 10:56

chore(ci): resolve drift gates for hook reintegration

39bb9ab

Register the new reintegration tests in the critical suite manifest and trim hook_integrator.py to the tightened 2100-line limit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

test(ci): sync TM002 manifest count after main rebase

a9c55ba

Main added a critical-suite module; rebasing this branch unions both additions to 20 entries. Update the TM002 count assertion to match. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

danielmeppiel force-pushed the fix/2254-transitive-hook-reintegration branch from 37a450c to a9c55ba Compare July 17, 2026 09:00

danielmeppiel approved these changes Jul 17, 2026

View reviewed changes

danielmeppiel merged commit 999133d into microsoft:main Jul 17, 2026
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(hooks): rebuild merged hooks from transitive lockfile survivors#2256

fix(hooks): rebuild merged hooks from transitive lockfile survivors#2256
danielmeppiel merged 6 commits into
microsoft:mainfrom
Shaurya2k06:fix/2254-transitive-hook-reintegration

Shaurya2k06 commented Jul 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Shaurya2k06 commented Jul 16, 2026

Uh oh!

danielmeppiel commented Jul 17, 2026

Uh oh!

danielmeppiel commented Jul 17, 2026

Uh oh!

danielmeppiel commented Jul 17, 2026

Uh oh!

danielmeppiel left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Shaurya2k06 commented Jul 16, 2026

fix(hooks): rebuild merged hooks from transitive lockfile survivors

TL;DR

Problem (WHY)

Approach (WHAT)

Implementation (HOW)

Diagrams

Trade-offs

Benefits

Validation

Scenario Evidence

How to test

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Shaurya2k06 commented Jul 16, 2026

Uh oh!

danielmeppiel commented Jul 17, 2026

APM Review Panel: ship_now

Panel summary

Recommendation

Folded in this run

Copilot signals reviewed

Deferred (out-of-scope follow-ups)

Regression-trap evidence (mutation-break gate)

Lint contract

CI

Mergeability status

Convergence

Uh oh!

danielmeppiel commented Jul 17, 2026

APM Review Panel: ship_now

Panel summary

Folded in this run

Copilot signals reviewed

Regression-trap evidence (mutation-break gate)

Lint contract

CI

Mergeability status

Convergence

Uh oh!

danielmeppiel commented Jul 17, 2026

Uh oh!

danielmeppiel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

APM Review Panel: `ship_now`

APM Review Panel: `ship_now`