fix(uninstall): clean removed-only target files

[Daniel Meppiel (danielmeppiel)]

fix(uninstall): clean removed-only target files

TL;DR

Global uninstall now cleans files owned only by removed packages before deleting the ownership state needed to find them. The canonical manifest reconciliation owner performs the cleanup without rewriting shared ownership before the existing survivor handoff. An edited file aborts with its path and preserves manifest, lockfile, and module state for retry. Closes #2656.

Important

Cleanup failure preserves package state and names every retained path so users can resolve it and rerun uninstall.

Problem (WHY)

• The post-removal target set omitted an OpenCode file owned only by the removed package, leaving an orphan. Issue #2656 records the reported outcome.
• Removing manifest, module, and lockfile state first would discard the ownership evidence required for safe cleanup and retry.
• A direct uninstall cleanup path would split target-scoped contraction away from its canonical owner.

Approach (WHAT)

• Reconcile selected removed owners before package-state mutation through reconcile_target_deployed_files().
• Preserve shared legacy ownership until the existing survivor handoff, while deleting only unique filesystem paths.
• Keep URI-backed runtime resources on their existing runtime-specific uninstall path.
• Batch compatibility-view updates for ordinary target contraction and reuse a per-value ledger index.
• Enforce the uninstall route with the static target-contraction ownership check.

Implementation (HOW)

• src/apm_cli/commands/uninstall/cli.py -- invokes selected-owner cleanup before state mutation, routes visibility through CommandLogger, and names retained paths before exiting for retry.
• src/apm_cli/install/manifest_reconcile.py -- adds selected-owner physical cleanup that preserves shared legacy ownership, skips URI resources, scopes existing ledger records by value, and batches normal compatibility-view updates.
• src/apm_cli/core/deployment_ledger.py and src/apm_cli/core/command_logger.py -- centralize batch legacy projection and the command-level cleanup messages.
• Tests and docs -- add active-target and edited-file lifecycle traps, update uninstall guidance, and record the fix in Unreleased.

Diagrams

Legend: selected-owner cleanup deletes unique filesystem paths before state mutation, while shared ownership and URI resources continue through their existing handoff paths.

flowchart LR
    U[uninstall] --> R[reconcile target deployed files]
    R --> F[delete unique filesystem paths]
    R --> H[preserve shared ownership and URI resources]
    F --> S[remove manifest lockfile module state]
    H --> S
    classDef new stroke-dasharray: 5 5;
    class R,F new;

Loading

Trade-offs

• Preserve shared legacy rows. Selected-owner cleanup does not rewrite shared ownership before the established survivor reintegration path runs.
• Leave URI resources to their runtime owner. Copilot App and similar URI-backed entries remain on their existing uninstall cleanup path.
• Fail before state mutation. Edited or undeletable unique files stop uninstall with an actionable retry message.

Benefits

1. A removed-only OpenCode agent is deleted before its ownership state disappears.
2. Same-target survivor files and URI-backed runtime resources retain their established ownership handoff.
3. Edited files preserve all package state and report the exact path to resolve.
4. Static routing and lifecycle tests trap future regressions.

Validation

PYTHONPATH=src uv run --no-sync pytest <focused ownership, uninstall, and architecture nodes> -q
17 passed

Current-source global lifecycle tests
2 passed

uv run --no-sync ruff check src/ tests/
All checks passed!

uv run --no-sync ruff format --check src/ tests/
1639 files already formatted

bash scripts/lint-auth-signals.sh
[+] auth-signal lint clean

GitHub CI on e29d7139d
Lint, both Linux shards, and architecture ratchets passed

Warning

Local pylint could not run because uv could not fetch setuptools/wheel: Socket is not connected (os error 57). GitHub CI ran pylint successfully in its passing Lint job.

Scenario Evidence

#	Scenario (user promise)	Principle(s)	Test(s) proving it	Type
1	Uninstalling a global package removes a file deployed only for its removed OpenCode target.	DevX (pragmatic as npm), Multi-harness support	tests/integration/test_global_scope_e2e.py::TestGlobalUninstallLifecycle::test_uninstall_global_cleans_removed_only_target_before_state_removal (regression-trap for #2656)	integration
2	Removing a package does not delete another package's same-target file.	DevX (pragmatic as npm), Governed by policy	tests/unit/test_uninstall_shared_ownership.py::test_uninstall_transfers_shared_deployed_path_ownership	unit
3	An edited removed-only file preserves package state and identifies the retry path.	DevX (pragmatic as npm), Governed by policy	tests/integration/test_global_scope_e2e.py::TestGlobalUninstallLifecycle::test_uninstall_global_preserves_state_for_user_edited_removed_target_file	integration
4	Future uninstall code keeps target cleanup at the canonical reconciliation owner.	Governed by policy	tests/integration/test_architecture_authorities.py::test_target_instruction_contraction_uses_manifest_reconciliation	integration

How to test

• Run the two global uninstall lifecycle tests and confirm successful cleanup plus edited-file state preservation.
• Run tests/unit/test_uninstall_shared_ownership.py and confirm a shared file is retained for the survivor.
• Run bash scripts/lint-architecture-boundaries.sh and confirm the canonical route is accepted.

apm-spec-waiver: Restores existing target ownership cleanup; no new OpenAPM contract.

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

[Copilot]

Pull request overview

This PR fixes a global uninstall edge case where tracked files deployed under a target used only by the removed package could be orphaned after uninstall. It does so by running target-scoped reconciliation before mutating/removing the manifest and lockfile state, and by scoping reconciliation to only the removed dependency keys.

Changes:

• Invoke target-scoped deployed-file reconciliation early in apm uninstall --global, before state removal, and abort uninstall (preserving state) if tracked files remain.
• Extend reconcile_target_deployed_files() to support optional dependency_keys scoping and apply user-scope target profiles when classifying legacy ledger entries.
• Add a static architecture check enforcing the uninstall -> manifest reconciliation route, plus unit/integration regression coverage for the orphaned-file scenario.

Show a summary per file

File	Description
src/apm_cli/commands/uninstall/cli.py	Runs target-file reconciliation before manifest/lockfile mutation; aborts uninstall if tracked files remain.
src/apm_cli/install/manifest_reconcile.py	Adds dependency-key scoping and user-scope target profile handling for reconciliation/ledger classification.
scripts/check_target_instruction_contraction_owner.py	Extends the contraction-owner static check to cover uninstall routing.
tests/unit/install/test_target_contraction_logging.py	Adds regression test ensuring selected removed ownership is cleaned without affecting same-target survivors.
tests/unit/scripts/test_check_target_instruction_contraction_owner.py	Adds mutation test proving the new uninstall routing rule is enforced.
tests/integration/test_global_scope_e2e.py	Adds end-to-end test that reproduces and prevents the global uninstall orphan-file case.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Lite

[Daniel Meppiel (danielmeppiel)]

APM Review Panel: needs_rework

PR #2658 improves uninstall cleanup, but CI failures show same-target ownership handling currently aborts valid declared-target uninstalls.

cc Daniel Meppiel (@danielmeppiel) -- a fresh advisory pass is ready for your review.

The highest-signal finding is the failing CI coverage in
tests/unit/test_uninstall_shared_ownership.py: normal declared-target
uninstall retained active target paths and then treated those retained paths as
failure. The repair must distinguish stale paths owned only by the removed
target from paths still owned by the active target, while preserving the
canonical owner and requested uninstall scope.

Security, architecture, logging, and performance signals converge rather than
conflict. Survivor-owned paths must remain protected in same-target removal,
cleanup diagnostics must be visible before mutation, and the implementation
must avoid repeated full legacy-projection rebuilds. The repair, lifecycle
coverage, documentation, and changelog follow-ups are folded in the current
recovery iteration.

Aligned with: secure-by-default: ownership-aware cleanup must never delete
paths still owned by a surviving package or target; portable-by-manifest:
uninstall follows canonical manifest ownership and declared target scope;
pragmatic-as-npm: valid uninstall completes predictably and explains retained
paths.

Growth signal. The retry-safe cleanup behavior now has a concise release
note and gives affected users a discoverable upgrade reason.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	1	0	0	Declared-target uninstall needed ownership-aware cleanup.
CLI Logging Expert	1	0	0	Pre-mutation cleanup needed command-visible diagnostics.
DevX UX Expert	0	1	0	Retained paths must be named before retry.
Supply Chain Security Expert	1	0	0	Same-target cleanup must preserve survivor-owned files.
OSS Growth Hacker	0	2	0	Recovery and release messaging should be actionable.
Doc Writer	0	1	1	Retry-safe behavior and ordering needed reference updates.
Test Coverage Expert	1	0	0	Edited-file state preservation needed lifecycle coverage.
Performance Expert	1	0	0	Selected cleanup needed to avoid repeated full projections.

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

Top 5 follow-ups

1. [Python Architect] (blocking-severity) Repair declared-target
   uninstall so active-target retention is not classified as failure.
2. [Supply Chain Security Expert] (blocking-severity) Preserve
   survivor-owned paths during same-target package removal.
3. [CLI Logging Expert] (blocking-severity) Surface cleanup through the
   command logger and cover emitted diagnostics.
4. [Performance Expert] (blocking-severity) Avoid repeated full legacy
   projection rebuilds for selected ownership cleanup.
5. [Test Coverage Expert] (blocking-severity) Add a lifecycle regression
   for edited files that preserves state for retry.

Recommendation

The indicated follow-ups have been folded into the recovery iteration. CI
evidence on the updated head will determine the next advisory pass.

---

Full per-persona findings

Python Architect

• [blocking] Declared-target uninstall classified expected active-target
  retention as failure. The current recovery keeps physical deletion in the
  canonical reconciler while preserving legacy ownership for the existing
  survivor handoff.

CLI Logging Expert

• [blocking] Pre-mutation cleanup needed default-visible output. The
  recovery routes cleanup through CommandLogger methods.

DevX UX Expert

• [recommended] Retained paths need an actionable retry message. The
  recovery lists every retained path and tells the user to resolve it.

Supply Chain Security Expert

• [blocking] Same-target survivor ownership must not be deleted. The
  recovery separates shared survivor paths from unique removed paths.

OSS Growth Hacker

• [recommended] The retry path and release narrative need to be
  user-actionable. The recovery adds a changelog entry.

Auth Expert -- inactive

The touched uninstall, manifest reconciliation, checker, and test files do not
modify token handling, credential resolution, remote operations, or
authentication diagnostics.

Doc Writer

• [recommended] The reference uninstall behavior needed the retry-safe
  failure mode and corrected ordering.

Test Coverage Expert

• [blocking] Edited-file retry behavior lacked an integration regression.
  The recovery adds a current-source global lifecycle test.

Performance Expert

• [blocking] Selected cleanup needed to avoid repeated full projection
  rebuilds. The recovery batches compatibility ownership updates.

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