diff --git a/src/specify_cli/bundler/lib/project.py b/src/specify_cli/bundler/lib/project.py index c895bf579d..9cf85b71bf 100644 --- a/src/specify_cli/bundler/lib/project.py +++ b/src/specify_cli/bundler/lib/project.py @@ -4,6 +4,7 @@ from pathlib import Path from ..._project import _resolve_init_dir_override +from ...integration_state import clean_integration_key from .. import BundlerError from .yamlio import ensure_within, load_json @@ -91,12 +92,23 @@ def active_integration(project_root: Path) -> str | None: # about resolving a marker that carries only ``default_integration`` # (hand-edited, or written by anything that follows the canonical # reader's shape). ``integration``/``id``/``active`` stay as fallbacks. - value = ( - data.get("default_integration") - or data.get("integration") - or data.get("id") - or data.get("active") - ) - if isinstance(value, str) and value: - return value + # Clean EACH candidate before selecting it, rather than picking the + # first truthy raw value and normalizing only that one. A raw ``or`` + # chain selects a whitespace-only ``default_integration`` (truthy) and + # then normalizes it to ``None``, losing the valid legacy key behind + # it -- whereas ``normalize_integration_state`` does + # ``clean_integration_key(data.get("default_integration")) or + # legacy_key`` and falls through: + # {"default_integration": " ", "integration": "copilot"} + # raw-then-clean -> None canonical -> 'copilot' + # + # Normalizing through the shared helper also fixes the original + # divergence: ``isinstance(value, str) and value`` accepted a + # whitespace-only key as real -- truthy, so it suppressed the "not + # determinable" fallback -- and returned a padded key verbatim, which + # matches no registered integration. + for field in ("default_integration", "integration", "id", "active"): + cleaned = clean_integration_key(data.get(field)) + if cleaned: + return cleaned return None diff --git a/tests/contract/test_bundle_cli.py b/tests/contract/test_bundle_cli.py index 6db4dab769..1e8c1c2243 100644 --- a/tests/contract/test_bundle_cli.py +++ b/tests/contract/test_bundle_cli.py @@ -1075,3 +1075,79 @@ def fake_open_url(url, timeout=None, extra_headers=None, redirect_validator=None # Rich may wrap the message across lines; normalise whitespace before checking. output_flat = " ".join(result.output.split()) assert "exceeds maximum size of 100 bytes" in output_flat + + +@pytest.mark.parametrize( + "recorded,expected", + [ + ("copilot", "copilot"), + (" copilot ", "copilot"), # padded: previously returned verbatim + (" ", None), # whitespace-only: previously truthy + ("\t\n", None), + ("", None), + (None, None), + (5, None), + ], + ids=["plain", "padded", "spaces", "tabs", "empty", "null", "non_string"], +) +def test_active_integration_matches_the_canonical_key_reader( + tmp_path: Path, recorded, expected +): + """`active_integration` must normalize the way the canonical reader does. + + Its own comment says it matches `integration_state`'s reader, but that + reader runs every value through `clean_integration_key`, while this one + only checked `isinstance(value, str) and value`. A whitespace-only key is + truthy, so it was returned as a real integration *and* suppressed the + "not determinable" fallback; a padded key was returned verbatim and + matches no registered integration. + """ + from specify_cli.bundler.lib.project import active_integration + + project = tmp_path / "proj" + (project / ".specify").mkdir(parents=True) + (project / ".specify" / "integration.json").write_text( + json.dumps({"default_integration": recorded}), encoding="utf-8" + ) + + assert active_integration(project) == expected + + +@pytest.mark.parametrize( + "recorded,expected", + [ + ({"default_integration": " ", "integration": "copilot"}, "copilot"), + ({"default_integration": "\t\n", "integration": " copilot "}, "copilot"), + ({"default_integration": 5, "integration": "copilot"}, "copilot"), + ({"integration": " ", "id": "claude"}, "claude"), + ({"default_integration": " ", "integration": " "}, None), + ({"default_integration": "cursor", "integration": "copilot"}, "cursor"), + ], + ids=[ + "blank_default", + "blank_default_padded_legacy", + "non_string_default", + "blank_legacy_falls_to_id", + "all_blank", + "precedence_kept", + ], +) +def test_active_integration_cleans_each_candidate_before_selecting( + tmp_path: Path, recorded, expected +): + """Each candidate must be cleaned before selection, not just the winner. + + A raw `or` chain selects a whitespace-only `default_integration` (truthy) + and then normalizes it to None, losing the valid legacy key behind it — + while `normalize_integration_state` does + `clean_integration_key(default) or legacy_key` and falls through. + """ + from specify_cli.bundler.lib.project import active_integration + + project = tmp_path / "proj" + (project / ".specify").mkdir(parents=True) + (project / ".specify" / "integration.json").write_text( + json.dumps(recorded), encoding="utf-8" + ) + + assert active_integration(project) == expected