fix(prune): detect orphan skills and preserve retained bundles - #3057
fangkang (fangkangmi) wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Orphan detection still risks masking real SKILL.md-only install roots due to standalone classification not considering SKILL.md, and the new lifecycle integration module is missing the established e2e marker.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
This PR fixes apm prune orphan detection for manifestless skill installs by treating SKILL.md as an install marker, while ensuring skills nested under declared/retained roots (e.g., manifestless bundles) are preserved. It also centralizes orphan selection logic so both prune and advisory warnings route through the same declaration-aware selector, with architecture-owner + guard coverage.
Changes:
- Extend installed-package scanning to recognize
SKILL.mdand treat nested skills as owned by their parent package boundary. - Share orphan selection via
_find_orphaned_packages()and enforce it via a new architecture linter rule/owner record. - Add unit + integration lifecycle tests for orphan skill cleanup and retained bundle/sibling preservation; update prune docs and changelog.
| File | Description |
|---|---|
src/apm_cli/commands/deps/_utils.py |
Teach _scan_installed_packages() / nesting detection to treat SKILL.md as a package marker. |
src/apm_cli/commands/_helpers.py |
Centralize orphan selection in _find_orphaned_packages() and reuse it from _check_orphaned_packages(). |
src/apm_cli/commands/prune.py |
Use _find_orphaned_packages() to match advisory/orphan warning selection semantics. |
tests/unit/test_deps_utils.py |
Add unit coverage for skill-only packages and embedded-skill ownership under a parent marker. |
tests/unit/test_prune_command.py |
Add unit coverage for pruning skill-only installs, preserving siblings, embedded skills, and manifestless bundles. |
tests/integration/test_prune_skill_lifecycle.py |
Add real CLI lifecycle integration coverage for install/remove/prune and declared bundle preservation. |
scripts/architecture_linter/checks/install_uninstall_and_resolution.py |
Add install-deployment-orphan-selection guard enforcing routing through _find_orphaned_packages(). |
scripts/architecture_linter/checks/install_deployment_analyzers.py |
Register the new orphan-selection architecture rule. |
tests/unit/scripts/test_architecture_runner.py |
Include the new orphan-selection guard in the architecture runner expectations. |
tests/integration/test_architecture_owner_rule_mutations.py |
Add mutation case ensuring prune can’t bypass the shared orphan selector. |
.apm/architecture/owners/install-deployment.json |
Register the orphan selection decision owner and guard. |
docs/src/content/docs/reference/cli/prune.md |
Document manifestless SKILL.md orphan detection + retained bundle preservation behavior. |
CHANGELOG.md |
Add a Fixed entry for the prune behavior change (#3015). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| orphaned_packages = _find_orphaned_packages( | ||
| installed_packages, expected_installed, standalone_installed | ||
| ) |
There was a problem hiding this comment.
Fixed in 00016f6. Standalone detection now recognizes SKILL.md; added regression coverage for prune and dry-run.
|
Thank you for this pull request. It is linked to #3015, which a maintainer has accepted for implementation. This is advisory triage only, not merge or scope approval. CODEOWNERS review remains with Daniel Meppiel (@danielmeppiel) and Sergio Sisternes (@sergio-sisternes-epam) (already requested). A maintainer still has to review the diff. Suggested next step: wait for CODEOWNERS review. Copilot left inline notes; the author replied that SKILL.md standalone detection and the e2e marker were addressed in 00016f6. A reviewer should confirm those threads. Generated by autopilot-pr-triage-worker. This comment is AI-generated and may contain errors. |
Sergio Sisternes (sergio-sisternes-epam)
added
triage/recommended
Summary
Fixes #3015:
apm prunenow detects orphanedSKILL.mdpackages after their lockfile entries are removed, while preserving skills inside declared or transitive bundles. Prune and advisory warnings share the same selection logic, including standalone skill-root detection.Maintainer approval.
Validation
e2emarker. Windows execution remains unverified.Remaining limits
Ownership remains unresolved: manually copied skills inside
apm_modules/can be deleted without proof that APM installed them. Standalone-root pruning also follows existing whole-directory semantics, including children sharing that root.