fix(scripts): honor SPECKIT_PYTHON override for preset manifest parsing - #4445
Open
chelsealong wants to merge 2 commits into
Open
fix(scripts): honor SPECKIT_PYTHON override for preset manifest parsing#4445chelsealong wants to merge 2 commits into
chelsealong wants to merge 2 commits into
Conversation
Preset composition raises "PyYAML is required" whenever the resolve/setup scripts run under a bare `python3` that lacks PyYAML but a `uv tool install`/`pipx`-isolated CLI venv has it. SPECKIT_PYTHON is already the established override for this class of problem (see the agent-context extension's update-agent-context.sh); extend it to the bash, PowerShell, and Python template-resolution twins so a user (or the CLI, in future) can point scripts at an interpreter that actually has PyYAML. Fixes github#4443
…owerShell _python3_command() (bash) and Get-Python3Command (PowerShell) accepted SPECKIT_PYTHON as soon as it resolved to any Python 3 interpreter, unlike their cited precedent (update-agent-context.sh) which also verifies `import yaml` succeeds before trusting it. That let a SPECKIT_PYTHON without PyYAML shadow a PATH python3 that has it, turning previously working preset composition into a "PyYAML is required" failure. Gate the SPECKIT_PYTHON branch on a successful `import yaml` in both twins so it's only preferred when it can actually serve manifest parsing; otherwise the existing python3/python/py fallback chain runs unchanged, matching pre-fix behavior for that case.
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.
Summary
Fixes #4443.
Preset composition raises
PyYAML is required to resolve preset template compositionwhenever the resolve/setup scripts are invoked under a barepython3that lacks PyYAML, while auv tool install/pipx-isolatedCLI venv (which does have PyYAML, per
pyproject.toml) is invisible tothat bare interpreter. This happens both when the bash/PowerShell twins
shell out to Python to parse a
preset.ymlmanifest, and when the Pythontwin (
resolve_template.py/setup_plan.py/ etc., viacommon.py)tries to
import yamldirectly in its own process.SPECKIT_PYTHONis already the established override for exactly this classof problem elsewhere in the codebase (see
extensions/agent-context/scripts/bash/update-agent-context.shand itstests). This PR extends that same override to the three
template-resolution twins:
scripts/bash/common.sh:_python3_command()now prefersSPECKIT_PYTHON(verified to be a working Python 3 interpreter that also has PyYAML)
before falling back to
python3/python/py -3on PATH.scripts/powershell/common.ps1:Get-Python3Commanddoes the same.scripts/python/common.py: since this twin runs in-process rather thanshelling out,
_preset_template_layernow triesimport yamlfirst and,if that fails, delegates manifest parsing to
SPECKIT_PYTHONvia a smallsubprocess-based
safe_load/YAMLErrorshim (_import_yaml/_DelegatedYAML) so the rest of the function's logic is unchanged.No dependencies, lockfiles, or CI config were touched —
pyyamlwas alreadya declared dependency; this only fixes which interpreter picks it up.
Update: an earlier revision of this PR let the bash/PowerShell twins
adopt
SPECKIT_PYTHONas soon as it resolved to any Python 3 interpreter,without checking it actually had PyYAML — unlike the precedent this PR
cites, which validates both. That meant a
SPECKIT_PYTHONset to a validbut PyYAML-less interpreter (e.g. pinned for an unrelated reason) would
shadow a working
python3on PATH and turn previously-succeeding presetcomposition into a
PyYAML is requiredfailure. Both_python3_command()and
Get-Python3Commandnow also probeimport yamlbefore acceptingSPECKIT_PYTHON, matchingupdate-agent-context.sh's candidate-validationloop; if the probe fails they fall through to the existing
python3/python/py -3chain exactly as ifSPECKIT_PYTHONwere unset.Test plan
Added
test_all_variants_honor_speckit_python_override_when_yaml_missingtotests/test_resolve_template_python_parity.py. It creates a realPyYAML-less interpreter (
python -m venv --without-pip) and:python3and noSPECKIT_PYTHONset — asserts both fail (reproduces[Bug] Preset composition fails under isolated installs: scripts run on system python3 but PyYAML lives in the CLI venv #4443), then
SPECKIT_PYTHONto the interpreter that has PyYAML — asserts bothsucceed and produce the correct composed template content.
(PowerShell is included in the same assertions when
pwshis available.)Also added
test_all_variants_fall_back_when_speckit_python_lacks_pyyaml,covering the regression an earlier revision introduced: it sets
SPECKIT_PYTHONto a real Python-3 interpreter without PyYAML whileleaving PATH's own
python3(which has PyYAML) untouched, and assertscomposition still succeeds via that PATH fallback across the bash and
PowerShell twins.
Confirmed both tests fail without their respective fixes:
test_all_variants_honor_speckit_python_override_when_yaml_missing: withgit checkout HEAD~2 -- scripts/bash/common.sh scripts/powershell/common.ps1 scripts/python/common.py, fails withassert Falseon the override-succeeds assertion.test_all_variants_fall_back_when_speckit_python_lacks_pyyaml: with onlythe latest commit's two files reverted (
git checkout HEAD~1 -- scripts/bash/common.sh scripts/powershell/common.ps1, i.e. the state theindependent reviewer blocked), fails the same way — reproducing the
reviewer's exact regression scenario.
Restored the fix in both cases and reran; both pass.
AI disclosure
This PR was written by an autonomous coding agent (Claude/Anthropic) based on
the issue's description and existing
SPECKIT_PYTHONprecedent in thecodebase. An independent reviewer agent blocked the first revision for
letting the bash/PowerShell twins honor
SPECKIT_PYTHONwithout checkingit actually had PyYAML (a regression relative to the cited precedent); this
revision fixes that specific gap, adds a regression test reproducing it,
and reruns the full test suite, shellcheck, and ruff before pushing.