Skip to content

Commit 3fbb624

Browse files
fix(hooks): scope merged-hook wipe to resolved targets, not all KNOWN_TARGETS (#2252)
* fix(hooks): scope merged-hook wipe to resolved targets, not all KNOWN_TARGETS HookIntegrator.sync_integration() wiped package-owned merged-hook entries across every supported harness (.claude/settings.json, .cursor/hooks.json, .gemini/settings.json, .kiro/hooks/), while the uninstall/prune rebuild paths that follow it only repopulate the project's currently-declared targets: list. Narrowing targets: silently dropped a still-installed sibling package's hooks (and its apm-hooks.json ownership sidecar) in the now-undeclared target, with no rebuild to restore them. HookIntegrator remains the single canonical owner of this decision: - sync_integration() now decouples the file-deletion prefix guard (kept as the union of KNOWN_TARGETS + caller targets, never narrower than before -- a safety floor) from a new merge_source variable that scopes the merged-hook JSON cleanup loop to the caller-supplied targets, falling back to all KNOWN_TARGETS only when targets is None. - reconcile_after_removal() now passes targets=targets into its internal sync_integration() call (previously omitted). - uninstall/engine.py's _sync_integrations_after_uninstall() now passes targets=_resolved_targets (previously omitted). Both apm uninstall and apm prune delegate to the same HookIntegrator methods with the same resolved-target semantics the rebuild step uses -- no caller-specific cleanup logic. Adds tests/integration/test_hook_wipe_target_scope_e2e.py: 8 hermetic CLI-driven tests parametrized across both callers (uninstall, prune) covering the narrowing repro with sibling + manual-entry preservation, a negative twin (in-scope wipe still works), idempotency, zero-write abort before the destructive wipe on resolve failure, and a mutation- break proof that the scoped-targets behavior is load-bearing. Confirmed RED (2/8 fail, reproducing #2250 for both callers) with the fix reverted, GREEN (8/8) restored. closes #2250 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8f0f6bce-c30a-4694-934e-53ebec71d384 * chore: trigger spec-conformance re-run with waiver apm-spec-waiver: restores the reconcile-and-preserve hook contract already documented in prune.md/manage-dependencies.md; corrects an implementation bug against an existing promise, no new normative behavior added Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8f0f6bce-c30a-4694-934e-53ebec71d384 --------- Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 49bfdc6 commit 3fbb624

4 files changed

Lines changed: 527 additions & 30 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2020

2121
### Fixed
2222

23+
- `apm uninstall` and `apm prune` no longer wipe still-installed dependencies'
24+
merged hook entries out of a harness (e.g. `.cursor/hooks.json`) that was
25+
dropped from a project's `targets:` list -- the hook wipe is now scoped to
26+
the same resolved target set the rebuild step repopulates, so a narrowed
27+
`targets:` no longer silently deletes a sibling package's hooks (and its
28+
`apm-hooks.json` sidecar) in the now-undeclared harness. (closes #2250)
2329
- `apm prune` no longer leaves stale, executable hook entries behind for a
2430
removed package: it now reconciles merged hook ownership when it removes
2531
an orphaned package, clearing entries it contributed to

src/apm_cli/commands/uninstall/engine.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -635,11 +635,18 @@ def _sync_integrations_after_uninstall(
635635
)
636636
counts["prompts"] += result.get("files_removed", 0)
637637

638-
# Hooks (multi-target sync_integration handles all targets)
638+
# Hooks: managed-file removal always scans every KNOWN_TARGETS prefix
639+
# (safe -- it only ever deletes files already tracked as this
640+
# package's own deployed_files). The merged-hook JSON wipe, in
641+
# contrast, is scoped to `_resolved_targets` -- the SAME set the
642+
# Phase 2 rebuild loop below iterates -- so a harness dropped from
643+
# this project's `targets:` list is never wiped for packages that
644+
# remain declared and installed (#2250).
639645
result = _integrators["hooks"].sync_integration(
640646
apm_package,
641647
project_root,
642648
managed_files=_buckets["hooks"] if _buckets else None,
649+
targets=_resolved_targets,
643650
)
644651
counts["hooks"] = result.get("files_removed", 0)
645652

src/apm_cli/integration/hook_integrator.py

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1877,27 +1877,32 @@ def sync_integration(
18771877
"""Remove APM-managed hook files.
18781878
18791879
Uses *managed_files* (relative paths) to surgically remove only
1880-
APM-tracked files. Falls back to legacy ``*-apm.json`` glob when
1881-
*managed_files* is ``None``.
1882-
1883-
**Never** calls ``shutil.rmtree``.
1884-
1880+
APM-tracked files; falls back to legacy ``*-apm.json`` glob when
1881+
*managed_files* is ``None``. **Never** calls ``shutil.rmtree``.
18851882
Also cleans APM entries from merged-hook JSON files via the
18861883
``_apm_source`` marker.
1884+
1885+
``targets`` (#2250) scopes ONLY the merged-hook JSON cleanup below,
1886+
NOT the ``managed_files`` prefix guard (union of ``KNOWN_TARGETS``
1887+
+ ``targets``): that guard defends against deleting outside a
1888+
recognized ``hooks/`` dir, and narrowing it would strand real
1889+
files deployed under a since-dropped target.
18871890
"""
18881891
from .targets import KNOWN_TARGETS
18891892

18901893
stats: dict[str, int] = {"files_removed": 0, "errors": 0}
18911894

1892-
# Derive hook prefixes dynamically from targets
1893-
source = targets if targets is not None else list(KNOWN_TARGETS.values())
1894-
hook_prefixes = []
1895-
for t in source:
1896-
if t.supports("hooks"):
1897-
sm = t.primitives["hooks"]
1898-
effective_root = sm.deploy_root or t.root_dir
1899-
hook_prefixes.append(f"{effective_root}/hooks/")
1900-
hook_prefix_tuple = tuple(hook_prefixes)
1895+
# Prefix guard: union of KNOWN_TARGETS + caller `targets`, never
1896+
# narrower than the unscoped default -- see docstring above.
1897+
guard_targets = list(KNOWN_TARGETS.values())
1898+
if targets is not None:
1899+
guard_targets = guard_targets + list(targets)
1900+
hook_prefixes = [
1901+
f"{(t.primitives['hooks'].deploy_root or t.root_dir)}/hooks/"
1902+
for t in guard_targets
1903+
if t.supports("hooks")
1904+
]
1905+
hook_prefix_tuple = tuple(dict.fromkeys(hook_prefixes))
19011906

19021907
if managed_files is not None:
19031908
# Manifest-based removal -- only remove tracked files
@@ -1929,8 +1934,10 @@ def sync_integration(
19291934
except Exception:
19301935
stats["errors"] += 1
19311936

1932-
# Clean APM entries from merged-hook JSON configs using external ownership.
1933-
for t in source:
1937+
# Clean APM entries from merged-hook JSON configs, scoped to
1938+
# `targets` when supplied -- matches the rebuild phase (#2250).
1939+
merge_source = targets if targets is not None else list(KNOWN_TARGETS.values())
1940+
for t in merge_source:
19341941
config = _MERGE_HOOK_TARGETS.get(t.name)
19351942
if config is not None:
19361943
json_path = project_root / t.root_dir / config.config_filename
@@ -1977,10 +1984,16 @@ def reconcile_after_removal(
19771984
reconcile, consistent with prune's existing per-package
19781985
error-tolerant semantics.
19791986
1980-
Known boundary (shared with uninstall, tracked in #2250):
1981-
rebuilds only direct dependencies from ``get_all_apm_dependencies()``
1982-
-- a transitive dependency's merged hooks are wiped by the clear
1983-
phase above but never re-integrated here.
1987+
Target scope (#2250): the merged-hook JSON wipe below is scoped to
1988+
the SAME resolved ``targets`` the rebuild loop uses, so a harness
1989+
dropped from ``targets:`` (but still holding stale merged-hook
1990+
entries) is never wiped for still-declared, still-installed
1991+
packages while the rebuild silently skips repopulating it.
1992+
1993+
Known boundary (shared with uninstall, #2250): rebuilds only
1994+
direct dependencies from ``get_all_apm_dependencies()`` -- a
1995+
transitive dependency's merged hooks are wiped above but never
1996+
re-integrated here.
19841997
"""
19851998
from apm_cli.constants import APM_MODULES_DIR
19861999
from apm_cli.models.apm_package import build_installed_package_info
@@ -2000,15 +2013,11 @@ def reconcile_after_removal(
20002013
surviving_deps = list(apm_package.get_all_apm_dependencies())
20012014

20022015
# Empty managed_files (not None) skips file-level deletion while
2003-
# still triggering the unconditional wipe of every _apm_source
2004-
# entry (and its ownership sidecar) across all KNOWN_TARGETS --
2005-
# see _clean_apm_entries_from_json. The narrower `targets` list
2006-
# resolved above is deliberately NOT passed here: doing so would
2007-
# make prune's wipe scope diverge from uninstall's identical
2008-
# call (both currently wipe all KNOWN_TARGETS by omitting
2009-
# targets=); that divergence is tracked as a shared-primitive
2010-
# fix in #2250, evaluated against all three callers together.
2011-
stats = self.sync_integration(apm_package, project_root, managed_files=set())
2016+
# still triggering the merged-hook JSON wipe, scoped to the same
2017+
# resolved `targets` the rebuild loop below uses (#2250).
2018+
stats = self.sync_integration(
2019+
apm_package, project_root, managed_files=set(), targets=targets
2020+
)
20122021

20132022
for dep_ref in surviving_deps:
20142023
pkg_info = build_installed_package_info(dep_ref, project_root / APM_MODULES_DIR)

0 commit comments

Comments
 (0)