From f617d2fdc1871e2e525522bd8243617fc0b64838 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Fri, 31 Jul 2026 11:41:23 +0500 Subject: [PATCH] fix(bundler): treat a blank active integration as indeterminate in FR-019 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resolve_install_plan`'s two FR-019 guards are a truthiness test and an `is None` test: if active_integration and required != active_integration: # clash if active_integration is None and not integration_explicit: # indeterminate An empty string satisfies neither, so it falls through to `effective_integration = required` and the bundle's pinned integration is silently adopted — the exact outcome the docstring says the guard prevents ("resolution fails instead of silently adopting the bundle's required integration"). active=None -> BundlerError: ... could not be determined active='' (blank) -> effective_integration='copilot' <-- silent adopt active='claude' -> BundlerError: ... targets integration 'copilot' Normalise a blank value to None before the guards, and strip first to match the writer (`integration_state.clean_integration_key`, which returns `None` for empty/whitespace and strips otherwise) so a padded value is not reported as clashing with its own unpadded form. Co-Authored-By: Claude Opus 5 (1M context) --- src/specify_cli/bundler/services/resolver.py | 10 ++++++++ tests/unit/test_bundler_resolver.py | 26 ++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/specify_cli/bundler/services/resolver.py b/src/specify_cli/bundler/services/resolver.py index 127fa683fd..9d9c61e79f 100644 --- a/src/specify_cli/bundler/services/resolver.py +++ b/src/specify_cli/bundler/services/resolver.py @@ -77,6 +77,16 @@ def resolve_install_plan( # FR-019: integration-compatibility — a bundle that pins a different # integration than the project's active one halts (no silent change). + # + # A blank integration arrives as ``""``, not ``None`` — which is not a usable + # integration id but satisfied NEITHER guard below (the first is a truthiness + # test, the second an ``is None`` test), so a pinned bundle was silently + # adopted: precisely the outcome this guard exists to prevent. Treat blank as + # indeterminate, and strip first like the writer + # (``integration_state.clean_integration_key``) so a padded value is not + # reported as clashing with itself. + if active_integration is not None: + active_integration = active_integration.strip() or None effective_integration = active_integration if manifest.integration is not None: required = manifest.integration.id diff --git a/tests/unit/test_bundler_resolver.py b/tests/unit/test_bundler_resolver.py index 7068a4813e..4045cc07a3 100644 --- a/tests/unit/test_bundler_resolver.py +++ b/tests/unit/test_bundler_resolver.py @@ -62,6 +62,32 @@ def test_pinned_integration_with_indeterminate_active_fails(): ) +@pytest.mark.parametrize("blank", ["", " ", "\t"]) +def test_pinned_integration_with_blank_active_fails(blank): + """A blank active integration is indeterminate, not a match. + + The clash guard is a truthiness test and the indeterminate guard is an + `is None` test, so `""` satisfied neither and fell through to + `effective_integration = required` — silently adopting the bundle's pinned + integration, the exact outcome the docstring says the guard prevents. + """ + manifest = _manifest(integration={"id": "claude"}) + with pytest.raises(BundlerError, match="could not be determined"): + resolve_install_plan( + manifest, speckit_version="0.11.2", active_integration=blank + ) + + +def test_padded_active_integration_is_not_a_clash_with_itself(): + """A padded value must strip, like the writer's clean_integration_key, + rather than be reported as clashing with its own unpadded form.""" + manifest = _manifest(integration={"id": "claude"}) + plan = resolve_install_plan( + manifest, speckit_version="0.11.2", active_integration=" claude " + ) + assert plan.effective_integration == "claude" + + def test_pinned_integration_with_indeterminate_active_allows_explicit_override(): manifest = _manifest(integration={"id": "claude"}) plan = resolve_install_plan(