fix(bundler): bundle update uninstalls components dropped by new version (#3353)

On refresh (bundle update), install_bundle iterated only the new plan's
components, so a component the previous version owned but the new one no
longer ships was left installed on disk while being dropped from the
rewritten record (contributed only holds plan.components). With no
record referencing it, remove_bundle could never clean it up —
permanently orphaned, violating the provenance invariant (FR-022). After
the component loop, when refresh and a prior record exists, uninstall
each previously-owned component absent from the new plan — unless another
bundle still needs it (components_still_needed refcount, mirroring
remove_bundle), in which case it stays installed and is simply
de-attributed. Runs inside the existing try so a failed removal takes the
same no-record-written rollback path.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Ali jawwad
2026-07-08 00:59:27 +05:00
committed by GitHub
parent 12faf7b5b5
commit d5ba062eab
2 changed files with 87 additions and 0 deletions

View File

@@ -130,6 +130,28 @@ def install_bundle(
done.append(component)
result.installed.append(component)
contributed.append(component)
# On update (refresh), uninstall components this bundle used to own
# that the new version no longer ships. Otherwise they are dropped
# from the record below (contributed only holds plan.components) yet
# left on disk — permanently orphaned, since no bundle record can
# ever remove them. A stale component still owned by another bundle
# is kept installed and simply de-attributed here (it stays in that
# bundle's record). Mirrors remove_bundle's refcount logic.
if refresh and existing is not None:
planned = {(c.kind, c.id) for c in plan.components}
still_needed = components_still_needed(
records, exclude_bundle_id=plan.bundle_id
)
for component in existing.contributed_components:
key = (component.kind, component.id)
if key in planned:
continue
if key in still_needed:
continue
if installer.is_installed(project_root, component):
installer.remove(project_root, component)
result.uninstalled.append(component)
except BundlerError:
_rollback(project_root, installer, done)
raise

View File

@@ -220,3 +220,68 @@ def test_pre_existing_component_is_not_attributed_or_removed(tmp_path: Path):
remove_bundle(tmp_path, "demo-bundle", installer)
assert ("extensions", "ext-a") in installer.installed
def _bundle(manifest_id, ext_ids, *, version="1.0.0"):
data = valid_manifest_dict()
data["bundle"]["id"] = manifest_id
data["bundle"]["version"] = version
data["provides"] = {
"extensions": [{"id": e, "version": version} for e in ext_ids]
}
return BundleManifest.from_dict(data)
def test_update_uninstalls_components_dropped_by_new_version(tmp_path: Path):
"""`bundle update` must uninstall components the new version no longer
ships, instead of orphaning them (installed on disk, tracked by nothing)."""
make_project(tmp_path)
installer = FakeInstaller()
man_v1 = _bundle("demo", ["ext-a", "ext-b"])
install_bundle(tmp_path, _plan(man_v1), installer, manifest=man_v1)
assert ("extensions", "ext-b") in installer.installed
man_v2 = _bundle("demo", ["ext-a"], version="2.0.0")
result = install_bundle(
tmp_path, _plan(man_v2), installer, manifest=man_v2, refresh=True
)
# ext-b was dropped by v2 -> uninstalled and reported.
assert ("extensions", "ext-b") in installer.remove_calls
assert ("extensions", "ext-b") in {(c.kind, c.id) for c in result.uninstalled}
assert ("extensions", "ext-b") not in installer.installed
assert ("extensions", "ext-a") in installer.installed
# The saved record lists only ext-a.
rec = next(r for r in load_records(tmp_path) if r.bundle_id == "demo")
keys = {(c.kind, c.id) for c in rec.contributed_components}
assert ("extensions", "ext-a") in keys
assert ("extensions", "ext-b") not in keys
def test_update_keeps_component_still_needed_by_sibling_bundle(tmp_path: Path):
"""A dropped component still owned by another bundle stays installed."""
make_project(tmp_path)
installer = FakeInstaller()
man_sib = _bundle("sibling", ["ext-b"])
install_bundle(tmp_path, _plan(man_sib), installer, manifest=man_sib)
man_v1 = _bundle("demo", ["ext-a", "ext-b"])
install_bundle(tmp_path, _plan(man_v1), installer, manifest=man_v1)
man_v2 = _bundle("demo", ["ext-a"], version="2.0.0")
install_bundle(
tmp_path, _plan(man_v2), installer, manifest=man_v2, refresh=True
)
# ext-b is still needed by 'sibling' -> not removed, stays installed.
assert ("extensions", "ext-b") not in installer.remove_calls
assert ("extensions", "ext-b") in installer.installed
# But demo's record no longer attributes it.
rec = next(r for r in load_records(tmp_path) if r.bundle_id == "demo")
assert ("extensions", "ext-b") not in {
(c.kind, c.id) for c in rec.contributed_components
}