From 283060fffe6b8f2c4c6606c64b3299b8a165ca4f Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Fri, 31 Jul 2026 10:59:24 +0500 Subject: [PATCH 1/2] fix(bundler): read the authoritative default_integration field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `active_integration()` resolves a project's integration with data.get("integration") or data.get("id") or data.get("active") and never looks at `default_integration` — which is the key the CLI actually writes. `integration_state.set_default_integration` persists `data["default_integration"] = integration_key`, and the canonical reader in that module orders it the other way round: key = state.get("default_integration") or state.get("integration") So a project initialised by any current version of the CLI looks to the bundler as though it has no active integration: {"default_integration": "copilot"} -> None (expected "copilot") {"integration": "copilot"} -> "copilot" (legacy alias) That silently changes bundler behaviour that keys off the active integration, including the FR-019 clash guard, which treats an undeterminable integration differently from a known one. Read `default_integration` first and keep the three legacy aliases as fallbacks for projects initialised by older versions. Co-Authored-By: Claude Opus 5 (1M context) --- src/specify_cli/bundler/lib/project.py | 14 +++++- .../test_bundler_security_paths.py | 43 +++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/bundler/lib/project.py b/src/specify_cli/bundler/lib/project.py index 6b9e9642f7..67617d6f61 100644 --- a/src/specify_cli/bundler/lib/project.py +++ b/src/specify_cli/bundler/lib/project.py @@ -82,7 +82,19 @@ def active_integration(project_root: Path) -> str | None: except BundlerError: return None if isinstance(data, dict): - value = data.get("integration") or data.get("id") or data.get("active") + # ``default_integration`` first: that is the key the CLI actually + # writes (``integration_state.set_default_integration``), and the + # canonical reader orders it the same way -- + # ``state.get("default_integration") or state.get("integration")``. + # ``integration``/``id``/``active`` are legacy aliases kept for + # projects initialised by older versions, so reading only those made + # every current project look like it had no active integration. + 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 return None diff --git a/tests/integration/test_bundler_security_paths.py b/tests/integration/test_bundler_security_paths.py index 0c01fe6406..51049f989b 100644 --- a/tests/integration/test_bundler_security_paths.py +++ b/tests/integration/test_bundler_security_paths.py @@ -126,6 +126,49 @@ def test_active_integration_refuses_symlinked_specify_escape(tmp_path: Path): assert active_integration(project) is None +def _write_marker(tmp_path: Path, payload: str) -> Path: + project = tmp_path / "proj" + (project / ".specify").mkdir(parents=True) + (project / ".specify" / "integration.json").write_text( + payload, encoding="utf-8" + ) + return project + + +def test_active_integration_reads_default_integration(tmp_path: Path): + """``default_integration`` is the key the CLI writes, so it must be read. + + ``integration_state.set_default_integration`` persists + ``data["default_integration"] = integration_key``, and the canonical + reader is ``state.get("default_integration") or state.get("integration")``. + Reading only the legacy aliases made every current project look as though + it had no active integration. + """ + from specify_cli.bundler.lib.project import active_integration + + project = _write_marker(tmp_path, '{"default_integration": "copilot"}') + assert active_integration(project) == "copilot" + + +def test_active_integration_prefers_default_over_legacy_alias(tmp_path: Path): + """When both are present the authoritative field wins, matching + ``integration_state``'s own ordering.""" + from specify_cli.bundler.lib.project import active_integration + + project = _write_marker( + tmp_path, '{"integration": "stale", "default_integration": "copilot"}' + ) + assert active_integration(project) == "copilot" + + +def test_active_integration_still_reads_legacy_alias(tmp_path: Path): + """Projects initialised by older versions carry only ``integration``.""" + from specify_cli.bundler.lib.project import active_integration + + project = _write_marker(tmp_path, '{"integration": "copilot"}') + assert active_integration(project) == "copilot" + + def test_read_catalog_config_refuses_symlinked_specify_escape(tmp_path: Path): from specify_cli.bundler.commands_impl import catalog_config as cc From d82680fe164a0236c2a154caa10b6288ac24e727 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Fri, 31 Jul 2026 22:52:32 +0500 Subject: [PATCH 2/2] docs(bundler): correct the justification for reading default_integration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review catch: the comment cited a nonexistent `integration_state.set_default_integration` and overstated the impact. The real writer is `write_integration_json`, which persists BOTH `integration` and `default_integration` (integration_state.py:248-250), so a marker produced by the current CLI already resolved through the `integration` alias. Measured: {"integration": "copilot", "default_integration": "copilot"} -> 'copilot' {"default_integration": "copilot"} -> 'copilot' (after fix) Reword both the source comment and the test docstring: this is about which field is authoritative when they disagree, plus resolving a marker that carries only `default_integration` — not about every current project being undetectable. The precedence itself still has its precedent, the canonical reader at integration_state.py:199. Behaviour unchanged; comments and docstrings only. Co-Authored-By: Claude Opus 5 (1M context) --- src/specify_cli/bundler/lib/project.py | 14 ++++++++------ tests/integration/test_bundler_security_paths.py | 15 ++++++++------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/specify_cli/bundler/lib/project.py b/src/specify_cli/bundler/lib/project.py index 67617d6f61..c895bf579d 100644 --- a/src/specify_cli/bundler/lib/project.py +++ b/src/specify_cli/bundler/lib/project.py @@ -82,13 +82,15 @@ def active_integration(project_root: Path) -> str | None: except BundlerError: return None if isinstance(data, dict): - # ``default_integration`` first: that is the key the CLI actually - # writes (``integration_state.set_default_integration``), and the - # canonical reader orders it the same way -- + # ``default_integration`` first, matching the canonical reader in + # ``integration_state`` (line 199): # ``state.get("default_integration") or state.get("integration")``. - # ``integration``/``id``/``active`` are legacy aliases kept for - # projects initialised by older versions, so reading only those made - # every current project look like it had no active integration. + # ``write_integration_json`` writes both keys, so a marker produced by + # the current CLI already resolved through the ``integration`` alias -- + # this is about which field is authoritative when they disagree, and + # 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") diff --git a/tests/integration/test_bundler_security_paths.py b/tests/integration/test_bundler_security_paths.py index 51049f989b..e575dccb88 100644 --- a/tests/integration/test_bundler_security_paths.py +++ b/tests/integration/test_bundler_security_paths.py @@ -136,13 +136,14 @@ def _write_marker(tmp_path: Path, payload: str) -> Path: def test_active_integration_reads_default_integration(tmp_path: Path): - """``default_integration`` is the key the CLI writes, so it must be read. - - ``integration_state.set_default_integration`` persists - ``data["default_integration"] = integration_key``, and the canonical - reader is ``state.get("default_integration") or state.get("integration")``. - Reading only the legacy aliases made every current project look as though - it had no active integration. + """A marker carrying only ``default_integration`` must resolve. + + ``write_integration_json`` writes both ``integration`` and + ``default_integration``, so a marker produced by the current CLI already + resolved through the alias. This covers the authoritative field on its own — + hand-edited, or written by anything that follows the shape of the canonical + reader (``integration_state`` line 199: + ``state.get("default_integration") or state.get("integration")``). """ from specify_cli.bundler.lib.project import active_integration