fix(update): preserve marketplace uninstall aliases - #2949
Open
Marco Frömbgen (mfroembgen) wants to merge 2 commits into
Open
fix(update): preserve marketplace uninstall aliases#2949Marco Frömbgen (mfroembgen) wants to merge 2 commits into
Marco Frömbgen (mfroembgen) wants to merge 2 commits into
Conversation
Marco Frömbgen (mfroembgen)
requested review from
Daniel Meppiel (danielmeppiel) and
Sergio Sisternes (sergio-sisternes-epam)
as code owners
September 11, 2026 14:06
Copilot started reviewing on behalf of
Marco Frömbgen (mfroembgen)
September 11, 2026 14:07
View session
Contributor
There was a problem hiding this comment.
🟢 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Preserve
plugin@marketplaceuninstall aliases whenapm updateadvances an installed dependency. The regression reproduces on 0.30.0 and currentmain(e38261c5) in project and global Codex scope.After installing an alias,
apm.ymlcontains canonical Git coordinates. Update builds a new lock entry without rediscovering the marketplace, droppingdiscovered_viaandmarketplace_plugin_name. Alias uninstall then loses its offline lookup and can safely abort at the existing registry/lock ownership guard.LockfileBuildernow 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.
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 runsapm 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
Testing
git diff --checkpassed.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-covSpec conformance
apm-spec-waiver: Restore existing marketplace discovery provenance during lockfile rebuilds; no new manifest grammar, trust policy, or normative surface.