From 6020e7de9d744c0a02faf297dcc8aee80857793a Mon Sep 17 00:00:00 2001 From: RKS Date: Tue, 8 Sep 2026 15:49:21 -0400 Subject: [PATCH 1/4] fix: reject bundle version changes during install --- src/specify_cli/bundler/services/installer.py | 16 +++++++++++++- .../integration/test_bundler_install_flow.py | 22 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/bundler/services/installer.py b/src/specify_cli/bundler/services/installer.py index 58e220638d..4c8c977d8a 100644 --- a/src/specify_cli/bundler/services/installer.py +++ b/src/specify_cli/bundler/services/installer.py @@ -80,7 +80,9 @@ def install_bundle( Version-pin enforcement is install-time only. The primitive ``is_installed`` checks are id-based (they do not compare versions), so when a component is already present and *refresh* is False it is skipped without verifying that - the on-disk version matches the manifest pin. Pins are therefore only + the on-disk version matches the manifest pin. A recorded bundle whose + resolved version changes is rejected unless *refresh* is True, preventing + the record from advancing past stale components. Pins are therefore only guaranteed to be applied when the bundler actually performs an install or a refresh; running ``specify bundle update`` re-applies every owned component at its pinned version. @@ -94,6 +96,18 @@ def install_bundle( result = InstallResult(bundle_id=plan.bundle_id) existing = find_record(records, plan.bundle_id) + if ( + existing is not None + and not refresh + and existing.version != plan.version + ): + raise BundlerError( + f"Bundle '{plan.bundle_id}' is already installed at version " + f"{existing.version}, but version {plan.version} was requested. " + "Use 'specify bundle update' to refresh its components before " + "advancing the installed record." + ) + prior_ours = { (c.kind, c.id) for c in existing.contributed_components } if existing is not None else set() diff --git a/tests/integration/test_bundler_install_flow.py b/tests/integration/test_bundler_install_flow.py index 0966008a74..6715eb51e4 100644 --- a/tests/integration/test_bundler_install_flow.py +++ b/tests/integration/test_bundler_install_flow.py @@ -51,6 +51,28 @@ def test_install_is_idempotent(tmp_path: Path): assert len(load_records(tmp_path)) == 1 +def test_install_rejects_version_change_without_refresh(tmp_path: Path): + """A normal install must not advance a record past stale components. + + ``bundle install`` is intentionally idempotent. When the same bundle ID + resolves to a different version, callers must use ``bundle update`` so the + owned primitives are refreshed before the record is changed. + """ + make_project(tmp_path) + installer = FakeInstaller() + + version_one = _bundle("demo", ["ext-a"], version="1.0.0") + install_bundle(tmp_path, _plan(version_one), installer, manifest=version_one) + + version_two = _bundle("demo", ["ext-a"], version="2.0.0") + with pytest.raises(BundlerError, match="bundle update"): + install_bundle(tmp_path, _plan(version_two), installer, manifest=version_two) + + record = load_records(tmp_path)[0] + assert record.version == "1.0.0" + assert len(installer.install_calls) == 1 + + def test_partial_failure_rolls_back_and_records_nothing(tmp_path: Path): make_project(tmp_path) manifest = BundleManifest.from_dict(valid_manifest_dict()) From 3b6d00c5c57aa506bd353f1d85cbb96d8a9bd6db Mon Sep 17 00:00:00 2001 From: RKS Date: Wed, 9 Sep 2026 11:00:24 -0400 Subject: [PATCH 2/4] fix: support explicit local bundle refresh Assisted-by: OpenAI Codex (autonomous) --- docs/reference/bundles.md | 13 +++- src/specify_cli/bundler/services/installer.py | 5 +- src/specify_cli/commands/bundle/__init__.py | 14 +++- .../integration/test_bundler_local_install.py | 75 ++++++++++++++++++- 4 files changed, 100 insertions(+), 7 deletions(-) diff --git a/docs/reference/bundles.md b/docs/reference/bundles.md index 2bd33c960b..eb6ced4019 100644 --- a/docs/reference/bundles.md +++ b/docs/reference/bundles.md @@ -40,10 +40,19 @@ specify bundle install | ---------------- | ------------------------------------------------------------------ | | `--integration` | Override the integration used when initializing/installing | | `--offline` | Do not access the network | +| `--refresh` | Refresh owned components from the supplied bundle source | Installs a bundle's full component set through each primitive's machinery. The argument may be a catalog bundle id, or a local path to a built `.zip` artifact, a bundle directory, or a `bundle.yml` file; local sources install directly without consulting the catalog stack. -If the current directory is not yet a Spec Kit project, `install` initializes one first so a fresh checkout reaches a working state in a single command. `--integration` selects the integration when initializing a new project, and confirms the target when a bundle pins a specific integration but the project's active integration can't be determined (missing or unreadable `.specify/integration.json`). It does **not** override an already-initialized project's active integration: if a bundle targets a different integration than the project's, install aborts with no changes. Integration-agnostic bundles inherit the project's active integration. Installation is idempotent — components already present are skipped. On failure, no provenance record is written (a failed install records nothing), and the components installed during that run are removed on a best-effort basis — removal errors are swallowed, so partial on-disk state may remain. +If the current directory is not yet a Spec Kit project, `install` initializes one first so a fresh checkout reaches a working state in a single command. `--integration` selects the integration when initializing a new project, and confirms the target when a bundle pins a specific integration but the project's active integration can't be determined (missing or unreadable `.specify/integration.json`). It does **not** override an already-initialized project's active integration: if a bundle targets a different integration than the project's, install aborts with no changes. Integration-agnostic bundles inherit the project's active integration. Without `--refresh`, installation is idempotent — components already present are skipped. On failure, no provenance record is written (a failed install records nothing), and the components installed during that run are removed on a best-effort basis — removal errors are swallowed, so partial on-disk state may remain. + +A normal install rejects a change to an already-recorded bundle's version. To upgrade a local bundle without adding it to a catalog, pass the newer source with `--refresh`: + +```bash +specify bundle install ./new-release/bundle.yml --refresh --offline +``` + +The source may also be a bundle directory or `.zip` artifact. Refresh uses the same primitive update path as `bundle update`, re-applies components owned by a bundle, and removes previously owned components omitted from the new manifest unless another bundle still needs them. Components installed independently remain untouched and are not adopted. The success summary includes refreshed and removed counts. The bundle record advances only after the operation succeeds; as with `bundle update`, already-installed components modified during a failed refresh are not rolled back. ## Update Bundles @@ -59,7 +68,7 @@ specify bundle update [] Re-resolves a bundle and **refreshes** its components through each primitive's update path, bringing already-installed components up to the bundle's newly pinned versions while preserving primitive-level overrides (such as preset priority). Provide a bundle id, or use `--all` to update everything installed. -> **Pin enforcement is install-time only.** Idempotency checks are id-based, not version-aware: a component that is already present is skipped during `install` without comparing its on-disk version to the manifest pin. Version pins are therefore guaranteed to be applied only when the bundler actually installs a component for the first time or refreshes it. Run `specify bundle update` to re-apply every owned component at its pinned version. +> **Pin enforcement is install-time only.** Idempotency checks are id-based, not version-aware: a component that is already present is skipped during `install` without comparing its on-disk version to the manifest pin. Version pins are therefore guaranteed to be applied only when the bundler actually installs a component for the first time or refreshes it. Run `specify bundle update ` for catalog bundles or `specify bundle install --refresh` for local sources to re-apply owned components at their pinned versions. ## Remove a Bundle diff --git a/src/specify_cli/bundler/services/installer.py b/src/specify_cli/bundler/services/installer.py index 4c8c977d8a..1e5072fd9e 100644 --- a/src/specify_cli/bundler/services/installer.py +++ b/src/specify_cli/bundler/services/installer.py @@ -104,8 +104,9 @@ def install_bundle( raise BundlerError( f"Bundle '{plan.bundle_id}' is already installed at version " f"{existing.version}, but version {plan.version} was requested. " - "Use 'specify bundle update' to refresh its components before " - "advancing the installed record." + "Use 'specify bundle update ' for a catalog bundle, or " + "'specify bundle install --refresh' for a local source, " + "to refresh owned components before advancing the installed record." ) prior_ours = { diff --git a/src/specify_cli/commands/bundle/__init__.py b/src/specify_cli/commands/bundle/__init__.py index 165f674a36..b809afba80 100644 --- a/src/specify_cli/commands/bundle/__init__.py +++ b/src/specify_cli/commands/bundle/__init__.py @@ -353,12 +353,16 @@ def bundle_install( ), integration: str = typer.Option(None, "--integration", help="Override integration"), offline: bool = typer.Option(False, "--offline", help="Do not access the network"), + refresh: bool = typer.Option( + False, "--refresh", help="Refresh owned components from this bundle source", + ), ) -> None: """Install a bundle's full component set through each primitive's machinery. ``bundle_id`` may be a catalog bundle id, or a local path to a built artifact (``.zip``), a bundle directory, or a ``bundle.yml`` file. Local - sources install directly without consulting the catalog stack. + sources install directly without consulting the catalog stack. Use + ``--refresh`` to update owned components from a newer local source. """ try: from ...bundler.lib.project import find_project_root @@ -428,14 +432,20 @@ def bundle_install( plan, DefaultPrimitiveInstaller(allow_network=not offline), manifest=manifest, + refresh=refresh, ) except BundlerError as exc: _fail(str(exc)) return + refresh_summary = ( + f", {len(result.refreshed)} refreshed, {len(result.uninstalled)} removed" + if refresh else "" + ) console.print( f"[green]✓[/green] Installed '{_escape_markup(str(result.bundle_id))}' " - f"({len(result.installed)} added, {len(result.skipped)} already present)." + f"({len(result.installed)} added, {len(result.skipped)} already present" + f"{refresh_summary})." ) diff --git a/tests/integration/test_bundler_local_install.py b/tests/integration/test_bundler_local_install.py index 630c981a73..bf173ac10c 100644 --- a/tests/integration/test_bundler_local_install.py +++ b/tests/integration/test_bundler_local_install.py @@ -18,7 +18,7 @@ from specify_cli import app from specify_cli.bundler import BundlerError from specify_cli.commands.bundle import _local_manifest_source -from tests.bundler_helpers import make_project, valid_manifest_dict, write_manifest +from tests.bundler_helpers import FakeInstaller, make_project, valid_manifest_dict, write_manifest def test_local_source_none_for_non_path(): @@ -309,3 +309,76 @@ def test_incompatible_local_manifest_is_rejected_before_project_init( assert result.exit_code == 1 assert "requires Spec Kit >=999.0.0" in result.output run_init.assert_not_called() + + +@pytest.mark.parametrize("source_kind", ["manifest", "directory", "zip"]) +def test_local_install_refresh_updates_owned_components( + tmp_path: Path, monkeypatch, source_kind: str, +): + """Local upgrades refresh owned pins before advancing the bundle record.""" + from specify_cli.bundler.models.records import load_records, records_path + + project = make_project(tmp_path / "proj") + monkeypatch.chdir(project) + versions = {} + + class VersionedInstaller(FakeInstaller): + def install(self, root, component): + super().install(root, component) + versions[(component.kind, component.id)] = component.version + + def refresh(self, root, component): + assert load_records(root)[0].version == "1.2.0" + super().refresh(root, component) + versions[(component.kind, component.id)] = component.version + + installer = VersionedInstaller() + monkeypatch.setattr( + "specify_cli.bundler.services.adapters.DefaultPrimitiveInstaller", + lambda **kwargs: installer, + ) + data = valid_manifest_dict() + manifest_path = write_manifest(tmp_path / "local bundle", data) + runner = CliRunner() + first = runner.invoke(app, ["bundle", "install", str(manifest_path), "--offline"]) + assert first.exit_code == 0, first.output + original_record = records_path(project).read_bytes() + original_versions = dict(versions) + + data["bundle"]["version"] = "2.0.0" + data["provides"]["extensions"][0]["version"] = "2.0.0" + data["provides"]["presets"][0]["version"] = "3.0.0" + data["provides"]["workflows"][0]["version"] = "0.4.0" + write_manifest(manifest_path.parent, data) + if source_kind == "manifest": + source = manifest_path + elif source_kind == "directory": + source = manifest_path.parent + else: + source = tmp_path / "local bundle.zip" + with zipfile.ZipFile(source, "w") as archive: + archive.write(manifest_path, "bundle.yml") + + rejected = runner.invoke(app, ["bundle", "install", str(source), "--offline"]) + assert rejected.exit_code == 1, rejected.output + assert records_path(project).read_bytes() == original_record + assert versions == original_versions + assert installer.refresh_calls == [] + + refreshed = runner.invoke( + app, ["bundle", "install", str(source), "--offline", "--refresh"], + ) + assert refreshed.exit_code == 0, refreshed.output + assert "--refresh" in rejected.output + assert "4 refreshed" in refreshed.output + expected = { + ("extensions", "ext-a"): "2.0.0", + ("presets", "preset-a"): "3.0.0", + ("steps", "step-a"): None, + ("workflows", "wf-a"): "0.4.0", + } + assert versions == expected + assert set(installer.refresh_calls) == set(expected) + record = load_records(project)[0] + assert record.version == "2.0.0" + assert {(c.kind, c.id): c.version for c in record.contributed_components} == expected From 2fd5c88cc69c539dc09442275f89d995b5639c62 Mon Sep 17 00:00:00 2001 From: RKS Date: Wed, 9 Sep 2026 15:13:18 -0400 Subject: [PATCH 3/4] fix: clarify catalog requirements for local bundle refresh Exercise local manifest, directory, and ZIP refresh through the real extension installer with deterministic catalog artifacts. Preserve state on offline failure and verify the online retry refreshes the owned version. Assisted-by: OpenAI Codex (model: GPT-6 Astra, autonomous) --- docs/reference/bundles.md | 4 +- .../bundler/services/primitives.py | 16 ++-- .../integration/test_bundler_local_install.py | 86 +++++++++++++++++++ tests/unit/test_bundler_primitives.py | 12 +++ 4 files changed, 109 insertions(+), 9 deletions(-) diff --git a/docs/reference/bundles.md b/docs/reference/bundles.md index eb6ced4019..7d3ba3f69f 100644 --- a/docs/reference/bundles.md +++ b/docs/reference/bundles.md @@ -49,11 +49,13 @@ If the current directory is not yet a Spec Kit project, `install` initializes on A normal install rejects a change to an already-recorded bundle's version. To upgrade a local bundle without adding it to a catalog, pass the newer source with `--refresh`: ```bash -specify bundle install ./new-release/bundle.yml --refresh --offline +specify bundle install ./new-release/bundle.yml --refresh ``` The source may also be a bundle directory or `.zip` artifact. Refresh uses the same primitive update path as `bundle update`, re-applies components owned by a bundle, and removes previously owned components omitted from the new manifest unless another bundle still needs them. Components installed independently remain untouched and are not adopted. The success summary includes refreshed and removed counts. The bundle record advances only after the operation succeeds; as with `bundle update`, already-installed components modified during a failed refresh are not rolled back. +A local bundle source supplies the manifest, not its component payloads. Components resolved through catalogs still require network access to refresh, even when already installed. Add `--offline` only when the components being installed or refreshed ship with Spec Kit; otherwise the command reports which component needs network access. Re-run without `--offline` to fetch that component through its catalog. + ## Update Bundles ```bash diff --git a/src/specify_cli/bundler/services/primitives.py b/src/specify_cli/bundler/services/primitives.py index 01fa14769e..3208b08645 100644 --- a/src/specify_cli/bundler/services/primitives.py +++ b/src/specify_cli/bundler/services/primitives.py @@ -188,8 +188,8 @@ def _do_install(self, component: ComponentRef, *, force: bool) -> None: if not self._allow_network: raise BundlerError( f"Preset '{component.id}' is not bundled and network access is " - f"disabled; re-run without --offline or install it first with " - f"'specify preset add {component.id}'." + "disabled. Installing or refreshing this component requires " + "network access; re-run without --offline." ) from ...presets import PresetCatalog @@ -272,8 +272,8 @@ def _do_install(self, component: ComponentRef, *, force: bool) -> None: if not self._allow_network: raise BundlerError( f"Extension '{component.id}' is not bundled and network access is " - f"disabled; re-run without --offline or install it first with " - f"'specify extension add {component.id}'." + "disabled. Installing or refreshing this component requires " + "network access; re-run without --offline." ) from ...extensions import ExtensionCatalog @@ -330,8 +330,8 @@ def install(self, component: ComponentRef) -> None: if not self._allow_network and not self._is_bundled(component.id): raise BundlerError( f"Workflow '{component.id}' installs from a catalog and network " - f"access is disabled; re-run without --offline or install it first " - f"with 'specify workflow add {component.id}'." + "access is disabled. Installing or refreshing this component " + "requires network access; re-run without --offline." ) self._assert_pinned_version(component) from ... import workflow_add @@ -396,8 +396,8 @@ def install(self, component: ComponentRef) -> None: if not self._allow_network: raise BundlerError( f"Step '{component.id}' installs from a catalog and network access " - f"is disabled; re-run without --offline or install it first with " - f"'specify workflow step add {component.id}'." + "is disabled. Installing or refreshing this component requires " + "network access; re-run without --offline." ) from ... import workflow_step_add diff --git a/tests/integration/test_bundler_local_install.py b/tests/integration/test_bundler_local_install.py index bf173ac10c..58217b5f6d 100644 --- a/tests/integration/test_bundler_local_install.py +++ b/tests/integration/test_bundler_local_install.py @@ -382,3 +382,89 @@ def refresh(self, root, component): record = load_records(project)[0] assert record.version == "2.0.0" assert {(c.kind, c.id): c.version for c in record.contributed_components} == expected + + +@pytest.mark.parametrize("source_kind", ["manifest", "directory", "zip"]) +def test_local_refresh_catalog_extension_requires_network( + tmp_path: Path, monkeypatch, source_kind: str, +): + """Use the real installer; replace only catalog I/O with local artifacts.""" + from specify_cli.bundler.models.records import load_records, records_path + from specify_cli.extensions import ExtensionCatalog + + project = make_project(tmp_path / "project") + monkeypatch.chdir(project) + monkeypatch.setattr("specify_cli.commands.bundle._bundle_overlaps", lambda *a, **kw: []) + monkeypatch.setattr("specify_cli._assets._locate_bundled_extension", lambda cid: None) + version = "1.0.0" + downloads = [] + + def download_extension(self, extension_id): + downloads.append((extension_id, version)) + artifact = tmp_path / "extension.zip" + extension = { + "schema_version": "1.0", + "extension": { + "id": extension_id, "name": "Catalog extension", + "version": version, "description": "Refresh regression", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": {"commands": [ + {"name": "speckit.catalog-ext.hello", "file": "commands/hello.md"}, + ]}, + } + with zipfile.ZipFile(artifact, "w") as archive: + archive.writestr("extension.yml", yaml.safe_dump(extension)) + archive.writestr("commands/hello.md", f"---\ndescription: Test\n---\n{version}\n") + return artifact + + monkeypatch.setattr( + ExtensionCatalog, "get_extension_info", + lambda self, cid: {"id": cid, "version": version, "_install_allowed": True}, + ) + monkeypatch.setattr(ExtensionCatalog, "download_extension", download_extension) + data = valid_manifest_dict(provides={"extensions": [{"id": "catalog-ext", "version": version}]}) + manifest_path = write_manifest(tmp_path / "local bundle", data) + runner = CliRunner() + first = runner.invoke(app, ["bundle", "install", str(manifest_path)]) + assert first.exit_code == 0, first.output + installed_dir = project / ".specify" / "extensions" / "catalog-ext" + payload = installed_dir / "commands" / "hello.md" + original_payload = payload.read_bytes() + original_manifest = (installed_dir / "extension.yml").read_bytes() + original_record = records_path(project).read_bytes() + + version = "2.0.0" + data["bundle"]["version"] = version + data["provides"]["extensions"][0]["version"] = version + write_manifest(manifest_path.parent, data) + if source_kind == "manifest": + source = manifest_path + elif source_kind == "directory": + source = manifest_path.parent + else: + source = tmp_path / "local bundle.zip" + with zipfile.ZipFile(source, "w") as archive: + archive.write(manifest_path, "bundle.yml") + + offline = runner.invoke(app, ["bundle", "install", str(source), "--refresh", "--offline"]) + assert offline.exit_code == 1, offline.output + output = " ".join(offline.output.split()) + assert "catalog-ext" in output + assert "refreshing this component requires network access" in output + assert "re-run without --offline" in output + assert "install it first" not in output + assert downloads == [("catalog-ext", "1.0.0")] + assert records_path(project).read_bytes() == original_record + assert payload.read_bytes() == original_payload + assert (installed_dir / "extension.yml").read_bytes() == original_manifest + + refreshed = runner.invoke(app, ["bundle", "install", str(source), "--refresh"]) + assert refreshed.exit_code == 0, refreshed.output + assert "1 refreshed" in refreshed.output + assert downloads == [("catalog-ext", "1.0.0"), ("catalog-ext", "2.0.0")] + assert payload.read_text(encoding="utf-8").endswith("2.0.0\n") + assert yaml.safe_load((installed_dir / "extension.yml").read_text(encoding="utf-8"))["extension"]["version"] == version + record = load_records(project)[0] + assert record.version == version + assert record.contributed_components[0].version == version diff --git a/tests/unit/test_bundler_primitives.py b/tests/unit/test_bundler_primitives.py index bbbac1133b..aa2874d4fe 100644 --- a/tests/unit/test_bundler_primitives.py +++ b/tests/unit/test_bundler_primitives.py @@ -70,6 +70,18 @@ def test_default_installer_threads_allow_network(tmp_path: Path): installer.install(tmp_path, _component("workflows")) +@pytest.mark.parametrize("kind", ["presets", "extensions", "workflows", "steps"]) +def test_offline_refresh_explains_component_needs_network(tmp_path: Path, kind: str): + installer = DefaultPrimitiveInstaller(allow_network=False) + with pytest.raises(BundlerError) as exc: + installer.refresh(tmp_path, _component(kind, "definitely-not-bundled")) + message = str(exc.value) + assert "definitely-not-bundled" in message + assert "refreshing this component requires network access" in message + assert "re-run without --offline" in message + assert "install it first" not in message + + def test_offline_workflow_allows_bundled(tmp_path: Path, monkeypatch): # A workflow that ships with Spec Kit must install even with --offline. import specify_cli From 1d1807436be7a4da4fa8c84c8c7850cc4a8b1027 Mon Sep 17 00:00:00 2001 From: RKS Date: Thu, 10 Sep 2026 09:43:36 -0400 Subject: [PATCH 4/4] fix: require refresh for owned bundle component changes Compare recorded component metadata with the requested plan before primitive operations. Reject changed pins, sources, preset options, and removals even when the bundle version is unchanged. Preserve idempotent installs, reordering, and additions; exercise refresh through lifecycle and real-installer CLI regressions. Assisted-by: OpenAI Codex (autonomous) --- docs/reference/bundles.md | 2 +- src/specify_cli/bundler/services/installer.py | 14 ++-- .../integration/test_bundler_install_flow.py | 78 +++++++++++++++++++ .../integration/test_bundler_local_install.py | 22 ++++-- 4 files changed, 104 insertions(+), 12 deletions(-) diff --git a/docs/reference/bundles.md b/docs/reference/bundles.md index 7d3ba3f69f..6035725c47 100644 --- a/docs/reference/bundles.md +++ b/docs/reference/bundles.md @@ -46,7 +46,7 @@ Installs a bundle's full component set through each primitive's machinery. The a If the current directory is not yet a Spec Kit project, `install` initializes one first so a fresh checkout reaches a working state in a single command. `--integration` selects the integration when initializing a new project, and confirms the target when a bundle pins a specific integration but the project's active integration can't be determined (missing or unreadable `.specify/integration.json`). It does **not** override an already-initialized project's active integration: if a bundle targets a different integration than the project's, install aborts with no changes. Integration-agnostic bundles inherit the project's active integration. Without `--refresh`, installation is idempotent — components already present are skipped. On failure, no provenance record is written (a failed install records nothing), and the components installed during that run are removed on a best-effort basis — removal errors are swallowed, so partial on-disk state may remain. -A normal install rejects a change to an already-recorded bundle's version. To upgrade a local bundle without adding it to a catalog, pass the newer source with `--refresh`: +A normal install rejects a change to an already-recorded bundle's version or owned component metadata (version, source, preset priority, or strategy), including removal of an owned component. This applies even if a local manifest keeps the same bundle version. Reordering unchanged components or adding new components does not require refresh. To apply changes to a local bundle without adding it to a catalog, pass the revised source with `--refresh`: ```bash specify bundle install ./new-release/bundle.yml --refresh diff --git a/src/specify_cli/bundler/services/installer.py b/src/specify_cli/bundler/services/installer.py index 1e5072fd9e..cd877864c5 100644 --- a/src/specify_cli/bundler/services/installer.py +++ b/src/specify_cli/bundler/services/installer.py @@ -80,9 +80,9 @@ def install_bundle( Version-pin enforcement is install-time only. The primitive ``is_installed`` checks are id-based (they do not compare versions), so when a component is already present and *refresh* is False it is skipped without verifying that - the on-disk version matches the manifest pin. A recorded bundle whose - resolved version changes is rejected unless *refresh* is True, preventing - the record from advancing past stale components. Pins are therefore only + the on-disk version matches the manifest pin. Changes to a recorded bundle's + version or owned component metadata, including removals, are rejected unless + *refresh* is True, preventing stale or orphaned components. Pins are only guaranteed to be applied when the bundler actually performs an install or a refresh; running ``specify bundle update`` re-applies every owned component at its pinned version. @@ -99,11 +99,15 @@ def install_bundle( if ( existing is not None and not refresh - and existing.version != plan.version + and ( + existing.version != plan.version + or not set(existing.contributed_components).issubset(plan.components) + ) ): raise BundlerError( f"Bundle '{plan.bundle_id}' is already installed at version " - f"{existing.version}, but version {plan.version} was requested. " + f"{existing.version}, but the requested manifest changes the bundle " + "version or changes/removes owned components. " "Use 'specify bundle update ' for a catalog bundle, or " "'specify bundle install --refresh' for a local source, " "to refresh owned components before advancing the installed record." diff --git a/tests/integration/test_bundler_install_flow.py b/tests/integration/test_bundler_install_flow.py index 6715eb51e4..8b149c9f49 100644 --- a/tests/integration/test_bundler_install_flow.py +++ b/tests/integration/test_bundler_install_flow.py @@ -73,6 +73,84 @@ def test_install_rejects_version_change_without_refresh(tmp_path: Path): assert len(installer.install_calls) == 1 +@pytest.mark.parametrize("kind,updates", [ + ("extensions", {"version": "2.0.0"}), + ("presets", {"version": "3.0.0"}), + ("steps", {"version": "1.0.0"}), + ("workflows", {"version": "0.4.0"}), + ("extensions", {"source": "https://example.com/catalog.json"}), + ("presets", {"priority": 20}), + ("presets", {"strategy": "prepend"}), + ("extensions", None), + ("presets", None), + ("steps", None), + ("workflows", None), +]) +def test_install_requires_refresh_for_owned_component_changes( + tmp_path: Path, kind: str, updates: dict | None, +): + make_project(tmp_path) + data = valid_manifest_dict() + original = BundleManifest.from_dict(data) + installer = FakeInstaller() + install_bundle(tmp_path, _plan(original), installer, manifest=original) + original_record = records_path(tmp_path).read_bytes() + original_installed = set(installer.installed) + installer.install_calls.clear() + + component_id = data["provides"][kind][0]["id"] + if updates is None: + data["provides"][kind] = [] + else: + data["provides"][kind][0].update(updates) + # Even a new component ordered before the changed one must not be installed. + data["provides"]["extensions"].insert(0, {"id": "ext-new", "version": "1.0.0"}) + changed = BundleManifest.from_dict(data) + plan = _plan(changed) + + with pytest.raises(BundlerError, match="--refresh"): + install_bundle(tmp_path, plan, installer, manifest=changed) + + assert records_path(tmp_path).read_bytes() == original_record + assert installer.installed == original_installed + assert installer.install_calls == [] + assert installer.refresh_calls == [] + assert installer.remove_calls == [] + + result = install_bundle(tmp_path, plan, installer, manifest=changed, refresh=True) + record = load_records(tmp_path)[0] + assert record.version == original.bundle.version + assert record.contributed_components == tuple(plan.components) + assert {(c.kind, c.id) for c in result.installed} == {("extensions", "ext-new")} + if updates is None: + assert installer.remove_calls == [(kind, component_id)] + assert (kind, component_id) not in installer.installed + else: + assert (kind, component_id) in installer.refresh_calls + assert installer.remove_calls == [] + + +def test_install_allows_reordered_components_and_additions(tmp_path: Path): + make_project(tmp_path) + data = valid_manifest_dict() + data["provides"]["extensions"].append({"id": "ext-b", "version": "1.0.0"}) + original = BundleManifest.from_dict(data) + installer = FakeInstaller() + install_bundle(tmp_path, _plan(original), installer, manifest=original) + + data["provides"]["extensions"].reverse() + # Identity includes kind: a step can have the same ID as an extension. + data["provides"]["steps"].append({"id": "ext-a"}) + changed = BundleManifest.from_dict(data) + plan = _plan(changed) + result = install_bundle(tmp_path, plan, installer, manifest=changed) + + assert {(c.kind, c.id) for c in result.installed} == {("steps", "ext-a")} + assert len(result.skipped) == 5 + assert installer.refresh_calls == [] + assert load_records(tmp_path)[0].contributed_components == tuple(plan.components) + + def test_partial_failure_rolls_back_and_records_nothing(tmp_path: Path): make_project(tmp_path) manifest = BundleManifest.from_dict(valid_manifest_dict()) diff --git a/tests/integration/test_bundler_local_install.py b/tests/integration/test_bundler_local_install.py index 58217b5f6d..7655543ebb 100644 --- a/tests/integration/test_bundler_local_install.py +++ b/tests/integration/test_bundler_local_install.py @@ -312,8 +312,9 @@ def test_incompatible_local_manifest_is_rejected_before_project_init( @pytest.mark.parametrize("source_kind", ["manifest", "directory", "zip"]) +@pytest.mark.parametrize("bundle_version", ["1.2.0", "2.0.0"]) def test_local_install_refresh_updates_owned_components( - tmp_path: Path, monkeypatch, source_kind: str, + tmp_path: Path, monkeypatch, source_kind: str, bundle_version: str, ): """Local upgrades refresh owned pins before advancing the bundle record.""" from specify_cli.bundler.models.records import load_records, records_path @@ -345,7 +346,7 @@ def refresh(self, root, component): original_record = records_path(project).read_bytes() original_versions = dict(versions) - data["bundle"]["version"] = "2.0.0" + data["bundle"]["version"] = bundle_version data["provides"]["extensions"][0]["version"] = "2.0.0" data["provides"]["presets"][0]["version"] = "3.0.0" data["provides"]["workflows"][0]["version"] = "0.4.0" @@ -380,13 +381,14 @@ def refresh(self, root, component): assert versions == expected assert set(installer.refresh_calls) == set(expected) record = load_records(project)[0] - assert record.version == "2.0.0" + assert record.version == bundle_version assert {(c.kind, c.id): c.version for c in record.contributed_components} == expected @pytest.mark.parametrize("source_kind", ["manifest", "directory", "zip"]) +@pytest.mark.parametrize("bundle_version", ["1.2.0", "2.0.0"]) def test_local_refresh_catalog_extension_requires_network( - tmp_path: Path, monkeypatch, source_kind: str, + tmp_path: Path, monkeypatch, source_kind: str, bundle_version: str, ): """Use the real installer; replace only catalog I/O with local artifacts.""" from specify_cli.bundler.models.records import load_records, records_path @@ -435,7 +437,7 @@ def download_extension(self, extension_id): original_record = records_path(project).read_bytes() version = "2.0.0" - data["bundle"]["version"] = version + data["bundle"]["version"] = bundle_version data["provides"]["extensions"][0]["version"] = version write_manifest(manifest_path.parent, data) if source_kind == "manifest": @@ -447,6 +449,14 @@ def download_extension(self, extension_id): with zipfile.ZipFile(source, "w") as archive: archive.write(manifest_path, "bundle.yml") + rejected = runner.invoke(app, ["bundle", "install", str(source), "--offline"]) + assert rejected.exit_code == 1, rejected.output + assert "--refresh" in rejected.output + assert downloads == [("catalog-ext", "1.0.0")] + assert records_path(project).read_bytes() == original_record + assert payload.read_bytes() == original_payload + assert (installed_dir / "extension.yml").read_bytes() == original_manifest + offline = runner.invoke(app, ["bundle", "install", str(source), "--refresh", "--offline"]) assert offline.exit_code == 1, offline.output output = " ".join(offline.output.split()) @@ -466,5 +476,5 @@ def download_extension(self, extension_id): assert payload.read_text(encoding="utf-8").endswith("2.0.0\n") assert yaml.safe_load((installed_dir / "extension.yml").read_text(encoding="utf-8"))["extension"]["version"] == version record = load_records(project)[0] - assert record.version == version + assert record.version == bundle_version assert record.contributed_components[0].version == version