From 163e315f8c8558f95fa87d0d90fda1e339d52696 Mon Sep 17 00:00:00 2001 From: yxshee <84263676+yxshee@users.noreply.github.com> Date: Sun, 18 Oct 2026 00:39:55 +0530 Subject: [PATCH] fix(bundles): reject non-positive preset and extension priorities in validation `specify bundle validate` reported a manifest whose preset (or extension) reference declared `priority: 0` or a negative priority as well-formed and valid, and `specify bundle build` packaged it, yet every install of that bundle failed: the preset and extension managers refuse a priority below 1, and the bundle primitives pass the manifest priority through unchanged. Because components install in manifest order, the error only surfaced after earlier components (such as extensions) had been installed and rolled back, which left files like .specify/extensions/.registry behind while the command reported "No changes were recorded." BundleManifest.structural_errors() only checked that a preset priority was present. Also reject a priority below 1 on preset and extension references, so validate, build and install refuse the manifest before any component is touched. Assisted-by: Claude Code (model: Claude Opus 5.5, autonomous) Co-Authored-By: Claude Opus 5.5 --- src/specify_cli/bundles/manifest.py | 11 +++++++ tests/contract/test_manifest_schema.py | 20 ++++++++++++ .../bundles/test_command_install.py | 31 +++++++++++++++++++ .../bundles/test_command_validate.py | 27 ++++++++++++++++ 4 files changed, 89 insertions(+) diff --git a/src/specify_cli/bundles/manifest.py b/src/specify_cli/bundles/manifest.py index 179abebd12..2811a31b96 100644 --- a/src/specify_cli/bundles/manifest.py +++ b/src/specify_cli/bundles/manifest.py @@ -211,6 +211,17 @@ def structural_errors(self) -> list[str]: errors.append( f"{ref.kind[:-1]} '{ref.id}' has invalid version '{ref.version}'." ) + # Preset and extension installs refuse a priority below 1, which + # would otherwise surface only mid-install, after earlier components. + if ( + ref.kind in ("extensions", "presets") + and ref.priority is not None + and ref.priority < 1 + ): + errors.append( + f"{ref.kind[:-1]} '{ref.id}' priority must be 1 or higher, " + f"got {ref.priority}." + ) for ref in self.presets: if ref.priority is None: diff --git a/tests/contract/test_manifest_schema.py b/tests/contract/test_manifest_schema.py index edad666ebb..b4ff75be4e 100644 --- a/tests/contract/test_manifest_schema.py +++ b/tests/contract/test_manifest_schema.py @@ -100,6 +100,26 @@ def test_non_integer_priority_raises_actionable_error(): BundleManifest.from_dict(data) +@pytest.mark.parametrize( + ("kind", "priority"), [("presets", 0), ("presets", -3), ("extensions", 0)] +) +def test_non_positive_priority_is_rejected(kind, priority): + """Preset and extension installs reject a priority below 1, so validation must too.""" + data = valid_manifest_dict() + data["provides"][kind][0]["priority"] = priority + errors = BundleManifest.from_dict(data).structural_errors() + assert any( + "priority must be 1 or higher" in e and f"got {priority}" in e for e in errors + ), errors + + +@pytest.mark.parametrize("kind", ["presets", "extensions"]) +def test_priority_of_one_is_accepted(kind): + data = valid_manifest_dict() + data["provides"][kind][0]["priority"] = 1 + assert BundleManifest.from_dict(data).structural_errors() == [] + + def test_non_step_components_must_be_pinned(): data = valid_manifest_dict() data["provides"]["extensions"] = [{"id": "ext-unpinned"}] diff --git a/tests/specify_cli/bundles/test_command_install.py b/tests/specify_cli/bundles/test_command_install.py index 2f54f051de..ad50c6f8ae 100644 --- a/tests/specify_cli/bundles/test_command_install.py +++ b/tests/specify_cli/bundles/test_command_install.py @@ -147,6 +147,37 @@ def test_local_bundle_rejects_mismatched_bundled_workflow_pin_offline(project: P assert not (project / ".specify" / "workflows" / "bugfix").exists() +def test_local_bundle_with_non_positive_priority_is_rejected_before_any_install( + project: Path, +): + """A priority the preset installer refuses must fail before earlier components install.""" + bundle_dir = project / "zero-priority" + bundle_dir.mkdir() + (bundle_dir / "bundle.yml").write_text( + yaml.safe_dump( + valid_manifest_dict( + provides={ + "extensions": [ + {"id": "bug", "version": bundled_extension_version("bug")} + ], + "presets": [ + {"id": "lean", "version": "1.0.0", "priority": 0, "strategy": "replace"} + ], + } + ) + ), + encoding="utf-8", + ) + + result = runner.invoke(app, ["bundle", "install", str(bundle_dir), "--offline"]) + + assert result.exit_code == 1 + assert "preset 'lean' priority must be 1 or higher, got 0" in " ".join( + result.output.split() + ) + assert not (project / ".specify" / "extensions").exists() + + def test_local_bundle_refuses_unbundled_workflow_offline(project: Path): bundle_dir = project / "unbundled-workflow" (bundle_dir / "bundle.yml").parent.mkdir() diff --git a/tests/specify_cli/bundles/test_command_validate.py b/tests/specify_cli/bundles/test_command_validate.py index d09f7a4d66..a2d57913e9 100644 --- a/tests/specify_cli/bundles/test_command_validate.py +++ b/tests/specify_cli/bundles/test_command_validate.py @@ -47,6 +47,33 @@ def test_validate_bundled_patch_compatibility( assert ("newer packaged patch" in " ".join(result.output.split())) is warning +def test_validate_rejects_non_positive_preset_priority(project: Path, monkeypatch): + """A manifest every install would reject must not be reported as valid.""" + import specify_cli._assets as assets + + payload = project / "packaged-preset" + payload.mkdir() + (payload / "preset.yml").write_text( + yaml.safe_dump({"preset": {"id": "p", "version": "1.0.0"}}), encoding="utf-8", + ) + monkeypatch.setattr(assets, "_locate_bundled_preset", lambda cid: payload) + data = valid_manifest_dict( + provides={ + "presets": [ + {"id": "p", "version": "1.0.0", "priority": 0, "strategy": "replace"} + ] + }, + ) + (project / "bundle.yml").write_text(yaml.safe_dump(data), encoding="utf-8") + + result = runner.invoke(app, ["bundle", "validate", "--offline"]) + + assert result.exit_code == 1, result.output + assert "preset 'p' priority must be 1 or higher, got 0" in " ".join( + result.output.split() + ) + + @pytest.mark.parametrize("offline", [False, True]) def test_validate_bundled_workflow_patch_warns_without_catalog_lookup( project: Path, monkeypatch, offline: bool,