fix(install): prune lockfile when all dependencies are removed from manifest

[nadav-y]

Summary

Removing every apm dependency from apm.yml (leaving apm: []) cleans the deployed files but leaves the lockfile listing the dropped dependencies. The committed apm.lock.yaml then claims deps the manifest no longer declares.

Reproduced on a clean project:

$ apm install        # apm.yml declares one dep (-> 3 with transitives)
$ grep -c repo_url apm.lock.yaml
3
# edit apm.yml: dependencies.apm: []
$ apm install
[i] Cleaned 4 files from packages no longer in apm.yml
$ grep -c repo_url apm.lock.yaml
3      # <-- stale; should be 0

Root cause

LockfileBuilder.build_and_save early-returns when installed_packages is empty:

if not self.ctx.installed_packages and not self.ctx.lockfile_only:
    self._sync_cache_pin_markers_from_disk()
    return

Emptying the manifest leaves installed_packages empty, so this returns before the orphan-prune path (_merge_existing, which already drops entries not in intended_dep_keys) can run. Partial removal (drop one of several deps) was unaffected, because installed_packages was non-empty and the full path ran.

Fix

Guard the early-return with _has_orphan_lockfile_entries(): fall through to the rebuild/prune path when the existing lockfile records apm deps the manifest no longer declares. The fall-through reuses the existing prune + MCP/local-state-preservation logic unchanged.

• Suppressed for only_packages installs (partial install intentionally preserves unlisted entries).
• Ignores the . self-entry (local deployed files), which is not a manifest dep.
• No-op installs with deps still present do not churn the lockfile (verified: generated_at unchanged across a cached re-install).

Behavior

Scenario	Before	After
Remove all deps (apm: []), reinstall	lockfile keeps stale entries	lockfile pruned to dependencies: []
Remove one of several deps	pruned correctly	pruned correctly (unchanged)
Cached no-op install	no churn	no churn (unchanged)

Tests

tests/unit/install/phases/test_lockfile_orphan_prune.py -- 6 cases on _has_orphan_lockfile_entries: orphan detected on emptied manifest, partial orphan, no orphan when all intended, only_packages suppression, no existing lockfile, and . self-entry exclusion.

Verified live against a JFrog registry (registry deps) and microsoft/apm-sample-package (git deps) -- the desync and fix reproduce identically for both sources, confirming this is a general install-path fix, not source-specific.

apm-spec-waiver: lockfile-correctness fix -- prunes orphaned entries on a depless manifest so the lockfile matches apm.yml; reuses the existing prune path, no new normative behavior

[Copilot]

Pull request overview

This PR fixes a lockfile correctness gap in the install pipeline: when a project removes all APM dependencies from apm.yml, apm install now prunes orphaned entries from apm.lock.yaml instead of leaving stale locked dependencies behind.

Changes:

• Adjusts LockfileBuilder.build_and_save() to skip the “nothing installed” early-return when the existing lockfile contains dependency entries that are no longer intended by the manifest.
• Introduces _has_orphan_lockfile_entries() to detect orphaned lockfile deps (while suppressing the check for only_packages partial installs and ignoring the "." self-entry).
• Adds unit tests covering orphan detection edge cases.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/apm_cli/install/phases/lockfile.py	Avoids early-return when orphans exist; adds _has_orphan_lockfile_entries() guard logic.
tests/unit/install/phases/test_lockfile_orphan_prune.py	Adds regression tests for orphan detection, including only_packages and "." self-entry handling.

[danielmeppiel]

APM Review Panel: ship_now

Community fix restores lockfile-manifest consistency when all dependencies are removed, closing a reliability gap aligned with P6 Reliability over magic.

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

Unanimous panel with zero required findings. This is a focused community bug fix: a single guard predicate restores the package-manager invariant that the lockfile follows the manifest. The shepherd folded the two actionable follow-ups: a hermetic build_and_save regression test with mutation-break evidence, and a user-facing CHANGELOG entry crediting the contributor. CI is green, test evidence is concrete, and no specialist raised an architectural, security, output, docs, or performance concern above nit level.

Aligned with: portable_by_manifest: lockfile now faithfully reflects manifest state even when the dependency list is empty; oss_community_driven: external contributor fix is credited in the changelog; pragmatic_as_npm: removing all deps and reinstalling leaves no phantom lockfile entries.

Growth signal. The changelog entry now turns this community fix into a visible lockfile-integrity proof point for evaluators.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	0	1	Clean, minimal guard in LockfileBuilder; no architecture concern.
CLI Logging Expert	0	0	0	No CLI output or logging paths changed.
DevX UX Expert	0	0	0	Restores the expected package-manager mental model.
Supply Chain Security Expert	0	0	1	Security-positive lockfile-manifest consistency; no new risk surface.
OSS Growth Hacker	0	0	0	Community contribution is now credited in CHANGELOG.
Doc Writer	0	0	0	CHANGELOG entry is concise and format-correct.
Test Coverage Expert	0	0	0	Seven passing regression tests cover helper and write-path behavior.
Performance Expert	0	0	0	No perf regression; O(N) scan only on the empty-install edge path.

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

Architecture

classDiagram
    direction LR
    class LockfileBuilder {
        <<Facade>>
        +build_and_save() None
        -_has_orphan_lockfile_entries() bool
        -_merge_existing(lockfile) None
    }
    class InstallContext {
        <<ValueObject>>
        +installed_packages list
        +existing_lockfile LockFile
        +intended_dep_keys set
        +only_packages list
        +lockfile_only bool
    }
    class LockFile {
        +dependencies dict
        +from_installed_packages() LockFile
        +save(path) None
    }
    LockfileBuilder *-- InstallContext : reads
    LockfileBuilder ..> LockFile : builds
    class LockfileBuilder:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    A["build_and_save()"] --> B{"installed_packages empty and not lockfile_only?"}
    B -->|no| C["build lockfile normally"]
    B -->|yes| D{"_has_orphan_lockfile_entries()?"}
    D -->|no| E["sync cache pin markers and return"]
    D -->|yes| C
    C --> F["_merge_existing() drops orphans"]
    F --> G["_write_if_changed()"]

Loading

Recommendation

Ship now. The in-scope follow-ups were folded into this PR, CI is green, and remaining nits are either optional refactor ideas or defensive polish outside this bug-fix scope.

Folded in this run

• (panel) Add a hermetic build_and_save regression test for empty-manifest lockfile pruning -- resolved in 25a4f2e.
• (panel) Add a user-facing CHANGELOG entry crediting @nadav-y -- resolved in 25a4f2e.

Deferred (out-of-scope follow-ups)

These items were surfaced by the panel or Copilot but cross the stated scope of this PR. Each one suggests a separate follow-up.

• (panel) Extract shared orphan-detection helper if another caller appears -- scope boundary: PR scope is one lockfile prune bug; extracting a cross-command helper would refactor adjacent install surfaces without current need.

Regression-trap evidence (mutation-break gate)

• tests/unit/install/phases/test_lockfile_orphan_prune.py::TestHasOrphanLockfileEntries::test_build_and_save_prunes_orphans_when_manifest_is_empty -- deleted and not self._has_orphan_lockfile_entries(); 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, and bash scripts/lint-auth-signals.sh passed before push.

CI

All PR checks passed on head 25a4f2e8db24a349d90803cabc0e31d6f0d72d16: Lint, Build & Test Shard 1/2, Coverage Combine, PR Binary Smoke, APM Self-Check, Analyze, CodeQL, Spec conformance gate, NOTICE Drift Check, gate, and license/cla (after 0 CI fix iteration(s)).

Mergeability status

Captured from gh pr view 1926 --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
#1926	25a4f2e	ship_now	1	2	1	2	green	MERGEABLE	BLOCKED	pending required review

Convergence

1 outer iteration(s); 2 Copilot round(s). Final panel verdict: ship_now.

Ready for maintainer review.

---

Full per-persona findings

Python Architect

• [nit] _SELF_KEY import could be hoisted for readability. The runtime impact is negligible and the current lazy import is acceptable in this scope.

CLI Logging Expert

No findings.

DevX UX Expert

No findings.

Supply Chain Security Expert

• [nit] intended_dep_keys or set() is fail-closed if a future nonstandard context passes None; no action required for this dataclass-backed path.

OSS Growth Hacker

No findings after the CHANGELOG fold.

Auth Expert -- inactive

PR touches lockfile pruning code, tests, and changelog only; no auth surface.

Doc Writer

No findings.

Test Coverage Expert

No findings after the regression-test fold. Proof: uv run --extra dev pytest tests/unit/install/phases/test_lockfile_orphan_prune.py -q -> 7 passed.

Performance Expert

No findings.

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