fix(bundler): read the authoritative default_integration field, not only its legacy aliases - #3880
Open
jawwad-ali wants to merge 1 commit into
Open
Conversation
`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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
active_integration()insrc/specify_cli/bundler/lib/project.pyresolves a project's active integration like this:It never reads
default_integration— the key the CLI actually writes.integration_state.set_default_integrationpersists it (integration_state.py:250):…and the canonical reader in the same module orders it the other way round (
integration_state.py:199):default_integrationis authoritative;integration/id/activeare legacy aliases. 57 references across 12 modules usedefault_integration. The bundler is the outlier.Reproduction on current
main(81bf741)So a project initialised by any current version of the CLI looks to the bundler as though it has no active integration.
Why it matters
Anything in the bundler that keys off the active integration silently takes the "indeterminable" path. That includes the FR-019 clash guard in
resolve_install_plan, which deliberately treats an unknown integration differently from a known one — so the guard's behaviour depends on whether the project happened to be written with the modern field or the legacy alias.Fix
Read the authoritative field first, keep all three legacy aliases as fallbacks:
Not a breaking change. Every payload that resolves today resolves to the same value: the aliases are still consulted, only after the field that should have been first. The only inputs whose behaviour changes are the ones that resolved to
Noneincorrectly. A payload carrying both now prefersdefault_integration, which is whatintegration_stateitself does.Verification
srcand pass with the fix.tests/integration/test_bundler_security_paths.py→ 16 passed, 3 failed; those same 3 fail on cleanmain(Windows symlink-privilege tests, part of a 61-failure baseline I captured on81bf741before starting). Scoped regression check overtests/integrationshows a failure set identical to that baseline.uvx ruff@0.15.0 check src tests→ cleanTests go beside the existing
test_active_integration_refuses_symlinked_specify_escape, the only current test of this function. Three cases: the authoritative field alone, both present (authoritative wins), and the legacy alias alone (still works).Written with assistance from Claude Code. Bug found, reproduced, and verified by me on current
main; every value above is from output I ran.