mirror of
https://github.com/github/spec-kit.git
synced 2026-08-15 14:02:04 +02:00
fix(bundler): treat a blank active integration as indeterminate in FR-019 (#3886)
`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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
f2583e675c
commit
197dde6253
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user