fix(update): preserve marketplace uninstall aliases

[Marco Frömbgen (mfroembgen)]

Description

Preserve plugin@marketplace uninstall aliases when apm update advances an installed dependency. The regression reproduces on 0.30.0 and current main (e38261c5) in project and global Codex scope.

After installing an alias, apm.yml contains canonical Git coordinates. Update builds a new lock entry without rediscovering the marketplace, dropping discovered_via and marketplace_plugin_name. Alias uninstall then loses its offline lookup and can safely abort at the existing registry/lock ownership guard.

LockfileBuilder now carries the four discovery fields from the previous entry for the same host-qualified dependency key and port. Fresh marketplace discovery replaces the entire tuple, including clearing obsolete catalog URL/digest fields. New commits and content hashes still come from the update.

This restores the existing alias-removal contract from #1323. The manifest-persistence design in #2903 and MCP-only uninstall in #2946 are separate work; this PR changes neither manifest grammar nor uninstall validation.

Fixes #2997.

Regression coverage

The generic lifecycle fixture registers a GitLab-style catalog backed by a local Git remote, installs a subpath via plugin@marketplace, commits and pushes a producer branch advance, and runs apm update --yes. It never edits consumer manifests or lockfiles after installation.

Both project and global cases verify the new commit and skill bytes, retained host/subpath identity and alias provenance, then unregister the catalog and uninstall by alias using the lockfile. An unrelated user-owned skill survives. Both cases fail on upstream main at the missing provenance assertion and pass with this fix.

Unit tests cover retention across commit changes, fresh discovery precedence, different hosts/ports/repos/subpaths, and removed dependencies. Catalog URL/digest remain historical discovery metadata, not assertions about updated package bytes. Already-lost provenance is not reconstructed.

Type of change

• Bug fix

Testing

• Full unit suite: 22,306 passed, 42 skipped, 21 xfailed.
• New lifecycle regression plus existing marketplace uninstall integration tests: 5 passed.
• Focused provenance and uninstall safety unit tests: 107 passed.
• Full Ruff lint and format checks, architecture boundaries, auth-signal lint, pylint duplication check, and git diff --check passed.

Validation used Python 3.12.13 on macOS and the source-installed CLI. Packaged binaries and other operating systems were not run locally.

How to test

uv sync --extra dev --frozen
uv run pytest tests/unit/marketplace/test_lockfile_provenance.py -n0 --no-cov
APM_E2E_TESTS=1 APM_BINARY_PATH="$PWD/.venv/bin/apm" \
  uv run pytest tests/integration/test_marketplace_update_provenance_lifecycle.py \
  tests/integration/test_uninstall_marketplace.py -n0 --no-cov

Spec conformance

apm-spec-waiver: Restore existing marketplace discovery provenance during lockfile rebuilds; no new manifest grammar, trust policy, or normative surface.

[Copilot]

🟢 Approval recommended

No unresolved blocking issues were identified.

Pull request overview

Preserves marketplace uninstall aliases across dependency updates by retaining discovery provenance in rebuilt lockfile entries.

Changes:

• Preserves matching marketplace provenance while refreshing package revisions.
• Adds unit and project/global lifecycle regression coverage.
• Documents the fix in the changelog.

File summaries

File	Summary
tests/unit/marketplace/test_lockfile_provenance.py	Tests provenance retention and replacement behavior.
tests/integration/test_marketplace_update_provenance_lifecycle.py	Verifies update and offline alias uninstall flows.
src/apm_cli/install/phases/lockfile.py	Preserves marketplace provenance during lockfile rebuilds.
CHANGELOG.md	Documents the marketplace alias fix.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Daniel Meppiel (danielmeppiel)]

Thanks Marco Frömbgen (@mfroembgen) for the focused repair and reproduction evidence.

Please open a short bug issue for marketplace uninstall aliases being lost after apm update, and link it here. You can reuse this PR’s problem statement and link to the existing reproduction evidence. Keep it separate from the broader marketplace design in #2903.

Our contribution process requires a canonical issue and explicit maintainer scope approval for code changes. The issue is where we agree on the bounded change, acceptance criteria and review contact before detailed implementation review.

Without a linked tracking issue, we’ll need to close this PR. Opening the issue enables that scope and capacity decision; it does not automatically approve the implementation or commit a review date.

[Sergio Sisternes (sergio-sisternes-epam)]

APM Review Panel: ship_now

Community fix restores plugin@marketplace uninstall after update by carrying marketplace provenance through LockfileBuilder; tests passed, CODEOWNERS tracking bug is issue #2997.

cc Marco Frömbgen (@mfroembgen) Daniel Meppiel (@danielmeppiel) -- a fresh advisory pass is ready for your review.

Specialists agree this is a bounded lockfile attach-path fix, not a new authority split. Provenance stay inside LockfileBuilder; unique-key plus port matching keeps catalog URL and digest historical while commits and hashes still move on the update path. DevX, supply chain, logging, docs, auth (inactive), and performance report no material concern. The CHANGELOG Unreleased Fixed line already states the user promise: update keeps discovery fields so name@marketplace uninstall still resolves in project and global scope.

Test-coverage evidence is load-bearing: e2e lifecycle (project and global) plus 15 unit provenance tests ran and passed, trapping the alias-after-update contract. That outweighs the single python-architect nit (keep the four-field copy inline rather than inventing a pattern). CODEOWNERS last comment asked to open or link a tracking bug; that is met via #2997 (status/accepted). Honor that request; do not drop or replace it.

Aligned with: Portable by manifest; OSS community driven; Pragmatic as npm.

Growth signal. Marketplace alias round-trip is a retention-critical npm-like promise; update dropping discovered_via silently broke second-use uninstall. Keep that story in release notes and credit the community author on the same line (PR #2949, #2997).

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	0	1	Provenance carry-forward stays inside LockfileBuilder; no split authority. Routine attach-path fix; ship.
CLI Logging Expert	0	0	0	No CLI logging surface. The PR only copies marketplace provenance fields in LockfileBuilder._attach_marketplace_provenance and adds lockfile tests; it does not add or change CommandLogger, DiagnosticCollector, rich* helpers, STATUS_SYMBOLS, warnings, errors, or progress output.
DevX UX Expert	0	0	0	Restores plugin@marketplace uninstall after update with no CLI surface, help, or error-wording change. Alias contract now matches install/update/uninstall.
Supply Chain Security Expert	0	0	0	Provenance copy is keyed by host-qualified unique key plus port; catalog URL/digest stay historical; commits and hashes stay on the update path.
OSS Growth Hacker	0	0	0	Community fix restores plugin@marketplace uninstall after update; CHANGELOG already user-shaped. No conversion-surface blocker. Ship.
Doc Writer	0	0	0	CHANGELOG Unreleased Fixed is accurate and user-facing. No docs/src drift; README correctly stays untouched. Ship from the docs lens.
Test Coverage Expert	0	0	0	Marketplace alias-after-update is trapped at e2e lifecycle floor (project+global) plus 15 unit provenance tests; S7 ran both, all passed.
Performance Expert	0	0	0	Dominant phase is not this path. One extra O(n) walk of lockfile.dependencies with O(1) lookup. No material concern.

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

Architecture

classDiagram
    direction LR
    class LockfileBuilder {
      <<Assembler>>
      +build_and_save() None
      -_attach_marketplace_provenance(lockfile) None
      -_merge_existing(lockfile) None
    }
    class InstallContext {
      +existing_lockfile LockFile
      +marketplace_provenance dict
      +installed_packages list
    }
    class LockFile {
      +dependencies dict
      +from_installed_packages()* LockFile
    }
    class LockedDependency {
      <<ValueObject>>
      +discovered_via str
      +marketplace_plugin_name str
      +source_url str
      +source_digest str
      +port int
      +get_unique_key() str
    }
    class UninstallEngine {
      +_resolve_marketplace_packages() dict
    }
    LockfileBuilder *-- InstallContext : reads
    LockfileBuilder ..> LockFile : assembles
    LockFile o-- LockedDependency : dependencies
    UninstallEngine ..> LockedDependency : reads alias fields
    note for LockfileBuilder "Single owner: copy prior discovery snapshot then overlay fresh ctx.marketplace_provenance"
    class LockfileBuilder:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    upd["apm update --yes\ncommands/update.py update_refs=True"] --> build["LockfileBuilder.build_and_save\ninstall/phases/lockfile.py"]
    build --> fromPkgs["LockFile.from_installed_packages"]
    fromPkgs --> attach["LockfileBuilder._attach_marketplace_provenance"]
    attach --> existing{"ctx.existing_lockfile?"}
    existing -->|no| fresh{"ctx.marketplace_provenance?"}
    existing -->|yes| lookup["existing.dependencies.get(dep_key)"]
    lookup --> portMatch{"previous is not None\nand previous.port == dep.port?"}
    portMatch -->|yes| copy["copy discovered_via marketplace_plugin_name source_url source_digest"]
    portMatch -->|no| fresh
    copy --> fresh
    fresh -->|yes| overlay["prov.get overlay including absent fields as None"]
    fresh -->|no| merge["LockfileBuilder._merge_existing"]
    overlay --> merge
    merge --> write["[FS] _write_if_changed apm.lock.yaml"]
    write --> uninstall["apm uninstall plugin@marketplace"]
    uninstall --> resolve["[I/O] _resolve_marketplace_packages\nuninstall/engine.py"]
    resolve --> exact{"exact_index.get via plugin name?"}
    exact -->|hit| ok["offline alias removal"]
    exact -->|miss| stage2["[NET] resolve_marketplace_plugin registry fallback"]

Loading

sequenceDiagram
    actor User
    participant CLI as apm CLI
    participant Builder as LockfileBuilder._attach_marketplace_provenance
    participant Lock as apm.lock.yaml
    participant Uninst as _resolve_marketplace_packages
    User->>CLI: apm install plugin@marketplace
    CLI->>Builder: attach from ctx.marketplace_provenance
    Builder->>Lock: write discovered_via snapshot
    User->>CLI: apm update --yes
    CLI->>Builder: copy prior snapshot then overlay fresh
    Builder->>Lock: same alias fields new resolved_commit
    User->>CLI: apm uninstall plugin@marketplace
    CLI->>Uninst: lockfile-first alias lookup
    Uninst->>Lock: discovered_via + marketplace_plugin_name

Loading

Recommendation

Advisory recommendation: land as-is. The only architect note is style (inline four-field copy). Tracking for the original bug is already #2997. Next human action is CODEOWNERS review by danielmeppiel and sergio-sisternes-epam, then cut the user-shaped CHANGELOG line into the next release notes with author credit.

---

Full per-persona findings

Python Architect

• [nit] Design patterns: none applied; keep the four-field copy inline. at src/apm_cli/install/phases/lockfile.py:383
  Design patterns: none -- straight-line procedural code, appropriate for the scope. LockfileBuilder._attach_marketplace_provenance remains the sole writer of discovered_via, marketplace_plugin_name, source_url, and source_digest. Two call sites sharing four assignments is below the extract-at-3 threshold.
  Suggested: keep the four-field copy inline.

CLI Logging Expert

No findings.

DevX UX Expert

No findings.

Supply Chain Security Expert

No findings.

OSS Growth Hacker

No findings.

Auth Expert -- inactive

Touched files are CHANGELOG.md, lockfile.py, and marketplace lockfile/provenance tests; no auth.py, token_manager, or credential-resolution paths.

Doc Writer

No findings.

Test Coverage Expert

No findings.

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.}