Skip to content

Commit bfabf4c

Browse files
jawwad-aliclaude
andauthored
fix(bundler): read the authoritative default_integration field, not only its legacy aliases (#3880)
* fix(bundler): read the authoritative default_integration field `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) <noreply@anthropic.com> * docs(bundler): correct the justification for reading default_integration 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent b66044a commit bfabf4c

2 files changed

Lines changed: 59 additions & 1 deletion

File tree

src/specify_cli/bundler/lib/project.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,21 @@ def active_integration(project_root: Path) -> str | None:
8282
except BundlerError:
8383
return None
8484
if isinstance(data, dict):
85-
value = data.get("integration") or data.get("id") or data.get("active")
85+
# ``default_integration`` first, matching the canonical reader in
86+
# ``integration_state`` (line 199):
87+
# ``state.get("default_integration") or state.get("integration")``.
88+
# ``write_integration_json`` writes both keys, so a marker produced by
89+
# the current CLI already resolved through the ``integration`` alias --
90+
# this is about which field is authoritative when they disagree, and
91+
# about resolving a marker that carries only ``default_integration``
92+
# (hand-edited, or written by anything that follows the canonical
93+
# reader's shape). ``integration``/``id``/``active`` stay as fallbacks.
94+
value = (
95+
data.get("default_integration")
96+
or data.get("integration")
97+
or data.get("id")
98+
or data.get("active")
99+
)
86100
if isinstance(value, str) and value:
87101
return value
88102
return None

tests/integration/test_bundler_security_paths.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,50 @@ def test_active_integration_refuses_symlinked_specify_escape(tmp_path: Path):
126126
assert active_integration(project) is None
127127

128128

129+
def _write_marker(tmp_path: Path, payload: str) -> Path:
130+
project = tmp_path / "proj"
131+
(project / ".specify").mkdir(parents=True)
132+
(project / ".specify" / "integration.json").write_text(
133+
payload, encoding="utf-8"
134+
)
135+
return project
136+
137+
138+
def test_active_integration_reads_default_integration(tmp_path: Path):
139+
"""A marker carrying only ``default_integration`` must resolve.
140+
141+
``write_integration_json`` writes both ``integration`` and
142+
``default_integration``, so a marker produced by the current CLI already
143+
resolved through the alias. This covers the authoritative field on its own —
144+
hand-edited, or written by anything that follows the shape of the canonical
145+
reader (``integration_state`` line 199:
146+
``state.get("default_integration") or state.get("integration")``).
147+
"""
148+
from specify_cli.bundler.lib.project import active_integration
149+
150+
project = _write_marker(tmp_path, '{"default_integration": "copilot"}')
151+
assert active_integration(project) == "copilot"
152+
153+
154+
def test_active_integration_prefers_default_over_legacy_alias(tmp_path: Path):
155+
"""When both are present the authoritative field wins, matching
156+
``integration_state``'s own ordering."""
157+
from specify_cli.bundler.lib.project import active_integration
158+
159+
project = _write_marker(
160+
tmp_path, '{"integration": "stale", "default_integration": "copilot"}'
161+
)
162+
assert active_integration(project) == "copilot"
163+
164+
165+
def test_active_integration_still_reads_legacy_alias(tmp_path: Path):
166+
"""Projects initialised by older versions carry only ``integration``."""
167+
from specify_cli.bundler.lib.project import active_integration
168+
169+
project = _write_marker(tmp_path, '{"integration": "copilot"}')
170+
assert active_integration(project) == "copilot"
171+
172+
129173
def test_read_catalog_config_refuses_symlinked_specify_escape(tmp_path: Path):
130174
from specify_cli.bundler.commands_impl import catalog_config as cc
131175

0 commit comments

Comments
 (0)