Skip to content

fix(scripts): honor SPECKIT_PYTHON override for preset manifest parsing - #4445

Open
chelsealong wants to merge 2 commits into
github:mainfrom
chelsealong:fix/4443-speckit-python-override
Open

fix(scripts): honor SPECKIT_PYTHON override for preset manifest parsing#4445
chelsealong wants to merge 2 commits into
github:mainfrom
chelsealong:fix/4443-speckit-python-override

Conversation

@chelsealong

Copy link
Copy Markdown
Contributor

Summary

Fixes #4443.

Preset composition raises PyYAML is required to resolve preset template composition whenever the resolve/setup scripts are invoked under a bare
python3 that lacks PyYAML, while a uv tool install / pipx-isolated
CLI venv (which does have PyYAML, per pyproject.toml) is invisible to
that bare interpreter. This happens both when the bash/PowerShell twins
shell out to Python to parse a preset.yml manifest, and when the Python
twin (resolve_template.py / setup_plan.py / etc., via common.py)
tries to import yaml directly in its own process.

SPECKIT_PYTHON is already the established override for exactly this class
of problem elsewhere in the codebase (see
extensions/agent-context/scripts/bash/update-agent-context.sh and its
tests). This PR extends that same override to the three
template-resolution twins:

  • scripts/bash/common.sh: _python3_command() now prefers SPECKIT_PYTHON
    (verified to be a working Python 3 interpreter that also has PyYAML)
    before falling back to python3 / python / py -3 on PATH.
  • scripts/powershell/common.ps1: Get-Python3Command does the same.
  • scripts/python/common.py: since this twin runs in-process rather than
    shelling out, _preset_template_layer now tries import yaml first and,
    if that fails, delegates manifest parsing to SPECKIT_PYTHON via a small
    subprocess-based safe_load/YAMLError shim (_import_yaml /
    _DelegatedYAML) so the rest of the function's logic is unchanged.

No dependencies, lockfiles, or CI config were touched — pyyaml was already
a declared dependency; this only fixes which interpreter picks it up.

Update: an earlier revision of this PR let the bash/PowerShell twins
adopt SPECKIT_PYTHON as 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_PYTHON set to a valid
but PyYAML-less interpreter (e.g. pinned for an unrelated reason) would
shadow a working python3 on PATH and turn previously-succeeding preset
composition into a PyYAML is required failure. Both _python3_command()
and Get-Python3Command now also probe import yaml before accepting
SPECKIT_PYTHON, matching update-agent-context.sh's candidate-validation
loop; if the probe fails they fall through to the existing python3/
python/py -3 chain exactly as if SPECKIT_PYTHON were unset.

Test plan

Added test_all_variants_honor_speckit_python_override_when_yaml_missing to
tests/test_resolve_template_python_parity.py. It creates a real
PyYAML-less interpreter (python -m venv --without-pip) and:

  1. runs the bash and Python twins with that interpreter as the default
    python3 and no SPECKIT_PYTHON set — 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
  2. sets SPECKIT_PYTHON to the interpreter that has PyYAML — asserts both
    succeed and produce the correct composed template content.
    (PowerShell is included in the same assertions when pwsh is available.)

Also added test_all_variants_fall_back_when_speckit_python_lacks_pyyaml,
covering the regression an earlier revision introduced: it sets
SPECKIT_PYTHON to a real Python-3 interpreter without PyYAML while
leaving PATH's own python3 (which has PyYAML) untouched, and asserts
composition 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: with
    git checkout HEAD~2 -- scripts/bash/common.sh scripts/powershell/common.ps1 scripts/python/common.py, fails with
    assert False on the override-succeeds assertion.
  • test_all_variants_fall_back_when_speckit_python_lacks_pyyaml: with only
    the latest commit's two files reverted (git checkout HEAD~1 -- scripts/bash/common.sh scripts/powershell/common.ps1, i.e. the state the
    independent reviewer blocked), fails the same way — reproducing the
    reviewer's exact regression scenario.
    Restored the fix in both cases and reran; both pass.
$ .venv/bin/python -m pytest tests/test_resolve_template_python_parity.py -q
============================= 42 passed in 39.14s ==============================

$ .venv/bin/python -m pytest tests/extensions/test_extension_agent_context.py -q
======================== 39 passed, 1 skipped in 10.62s ========================

$ .venv/bin/python -m pytest tests/ -q
========== 7706 passed, 12 skipped, 48 warnings in 416.00s (0:06:56) ===========

$ shellcheck --severity=error scripts/bash/common.sh
(no output, exit 0)

$ uvx ruff@0.15.0 check tests/test_resolve_template_python_parity.py src
All checks passed!

AI disclosure

This PR was written by an autonomous coding agent (Claude/Anthropic) based on
the issue's description and existing SPECKIT_PYTHON precedent in the
codebase. An independent reviewer agent blocked the first revision for
letting the bash/PowerShell twins honor SPECKIT_PYTHON without checking
it 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.

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.
@chelsealong
chelsealong requested a review from mnriem as a code owner September 4, 2026 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Preset composition fails under isolated installs: scripts run on system python3 but PyYAML lives in the CLI venv

1 participant