From b1cca7228f2f3f0822b19ef1419f4fd57d650b23 Mon Sep 17 00:00:00 2001 From: Muhammad Mirza <114640561+mirzasnowy@users.noreply.github.com> Date: Wed, 28 Oct 2026 22:24:36 +0700 Subject: [PATCH 1/2] fix(bundles): reject integration mappings without an id Fixes #5091. Assisted-by: Muse (model: Muse Spark, supervised) --- src/specify_cli/bundles/manifest.py | 13 ++++++++-- .../bundles/test_command_validate.py | 11 ++++++++ tests/specify_cli/bundles/test_manifest.py | 25 +++++++++++++++++++ 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/bundles/manifest.py b/src/specify_cli/bundles/manifest.py index 4476cb5856..c5dc8f7626 100644 --- a/src/specify_cli/bundles/manifest.py +++ b/src/specify_cli/bundles/manifest.py @@ -145,8 +145,17 @@ def from_dict(cls, data: Any) -> BundleManifest: # leaving the bundle wrongly integration-agnostic. Reject it instead. if integration_raw is not None and not isinstance(integration_raw, dict): raise BundlerError("'integration' must be a mapping when present.") - if isinstance(integration_raw, dict) and integration_raw.get("id"): - integration = IntegrationRef(id=str(integration_raw["id"]).strip()) + if isinstance(integration_raw, dict): + # A mapping without a usable id (missing, empty, or whitespace-only) + # fell through to integration=None just as silently as the bare + # string above: a declared integration was dropped and the bundle + # validated as integration-agnostic. Reject it instead. + integration_id = _text(integration_raw.get("id")) + if not integration_id: + raise BundlerError( + "'integration' mapping must include a non-empty 'id'." + ) + integration = IntegrationRef(id=integration_id) provides = data.get("provides") if provides is None: diff --git a/tests/specify_cli/bundles/test_command_validate.py b/tests/specify_cli/bundles/test_command_validate.py index 3674fb4388..1d39dfa2a2 100644 --- a/tests/specify_cli/bundles/test_command_validate.py +++ b/tests/specify_cli/bundles/test_command_validate.py @@ -125,6 +125,17 @@ def test_validate_reports_invalid_manifest(project: Path): assert "license" in result.output +@pytest.mark.parametrize("bad_integration", [{}, {"name": "claude"}]) +def test_validate_rejects_integration_mapping_without_id( + project: Path, bad_integration +): + data = valid_manifest_dict(integration=bad_integration) + (project / "bundle.yml").write_text(yaml.safe_dump(data), encoding="utf-8") + result = runner.invoke(app, ["bundle", "validate", "--offline"]) + assert result.exit_code == 1 + assert "integration" in result.output + + @pytest.mark.parametrize("version", ["1.2.0", "1.20.30-12alpha.1+build.01", "V10.20.30"]) def test_validate_accepts_valid_manifest(project: Path, version: str): data = valid_manifest_dict() diff --git a/tests/specify_cli/bundles/test_manifest.py b/tests/specify_cli/bundles/test_manifest.py index 6341cc92f4..8d60a6e1ed 100644 --- a/tests/specify_cli/bundles/test_manifest.py +++ b/tests/specify_cli/bundles/test_manifest.py @@ -230,3 +230,28 @@ def test_non_list_bundles_rejected(bundles): def test_non_mapping_bundle_entry_rejected(entry): with pytest.raises(BundlerError, match=r"provides\.bundles entry must be a mapping"): _manifest(reference_manifest_dict([entry])) + + +@pytest.mark.parametrize( + "bad_integration", + [{}, {"name": "claude"}, {"id": ""}, {"id": " "}, {"id": None}], +) +def test_integration_mapping_without_usable_id_rejected(bad_integration): + data = valid_manifest_dict(integration=bad_integration) + with pytest.raises( + BundlerError, match="'integration' mapping must include a non-empty 'id'" + ): + _manifest(data) + + +def test_integration_mapping_with_id_accepted_and_stripped(): + manifest = _manifest(valid_manifest_dict(integration={"id": " claude "})) + assert manifest.integration is not None + assert manifest.integration.id == "claude" + assert not manifest.is_agnostic() + + +def test_absent_integration_stays_agnostic(): + manifest = _manifest(valid_manifest_dict()) + assert manifest.integration is None + assert manifest.is_agnostic() From 66b00741a319fe4d9d6323f07aca1982e6386ee8 Mon Sep 17 00:00:00 2001 From: Muhammad Mirza <114640561+mirzasnowy@users.noreply.github.com> Date: Wed, 28 Oct 2026 23:44:25 +0700 Subject: [PATCH 2/2] docs(bundles): distinguish whitespace-only id in integration comment Addresses Copilot review feedback on #5161: a whitespace-only id did not fall through to integration=None; it passed the old truthiness check and became IntegrationRef(id=""). Assisted-by: Muse (model: Muse Spark, supervised) --- src/specify_cli/bundles/manifest.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/bundles/manifest.py b/src/specify_cli/bundles/manifest.py index c5dc8f7626..dec18f5377 100644 --- a/src/specify_cli/bundles/manifest.py +++ b/src/specify_cli/bundles/manifest.py @@ -146,10 +146,12 @@ def from_dict(cls, data: Any) -> BundleManifest: if integration_raw is not None and not isinstance(integration_raw, dict): raise BundlerError("'integration' must be a mapping when present.") if isinstance(integration_raw, dict): - # A mapping without a usable id (missing, empty, or whitespace-only) - # fell through to integration=None just as silently as the bare - # string above: a declared integration was dropped and the bundle - # validated as integration-agnostic. Reject it instead. + # A mapping without a usable id was silently mishandled: a + # missing or empty id fell through to integration=None (the + # declared integration was dropped and the bundle validated as + # integration-agnostic), while a whitespace-only id passed the + # old truthiness check and became IntegrationRef(id=""). + # Reject all three instead. integration_id = _text(integration_raw.get("id")) if not integration_id: raise BundlerError(