test: pin the suite interpreter on PATH for parity script subprocesses by mehdikarimi55 · Pull Request #5361 · github/spec-kit

mehdikarimi55 · 2026-11-10T10:45:20Z

Problem

Under the invocation AGENTS.md prescribes — uv sync --extra test then .venv/bin/python -m pytest — 11 parity tests fail on a fresh macOS checkout whose ambient python3 lacks PyYAML:

Error: PyYAML is required to resolve preset template composition

Affected: 7× tests/test_resolve_template_python_parity.py, 2× test_check_prerequisites_python_parity.py, 1× test_create_new_feature_python_parity.py, 1× test_setup_plan_python_parity.py, 1× test_setup_tasks_python_parity.py — all composed-template cases.

Root cause

run() in tests/parity_helpers.py defaults to clean_env() (a copy of os.environ), so the bash/PowerShell script variants resolve python3 -> python -> py -3 through the ambient PATH:

In CI, uv run pytest prepends the venv's bin/ to PATH → scripts see the venv interpreter (with PyYAML) → pass.
Direct .venv/bin/python -m pytest leaves PATH untouched → scripts find an ambient python3 without PyYAML → fail.

make_python3_path_shim()'s docstring in the same file already warns about exactly this hazard ("silently regains/loses that venv's site-packages (e.g. PyYAML)").

Fix

parity_helpers.clean_env() prepends the running interpreter's own bin directory to PATH — the same thing uv run does — so both invocations give script subprocesses the same Python the suite runs on. Tests that install their own shim (e.g. the no_yaml_python3_shim negative cases) prepend afterwards and keep precedence, so the negative tests are unaffected. test_check_prerequisites_python_parity.py had a file-local _clean_env() duplicate; it now delegates to the shared helper.

Test-only change; no product code touched.

Evidence

Regression evidence is the existing suite itself, run in the affected environment (macOS 15 arm64, Python 3.12.16, checkout path with a space, ambient python3 without PyYAML):

.venv/bin/python -m pytest tests/test_resolve_template_python_parity.py -q
# before: 7 failed   after: 11 passed
.venv/bin/python -m pytest tests/test_check_prerequisites_python_parity.py tests/test_create_new_feature_python_parity.py tests/test_setup_plan_python_parity.py tests/test_setup_tasks_python_parity.py -q
# before: 4 failed   after: all pass

Full local suite: 14,597 passed / 15 failed before → targeted runs green after (full-suite rerun attached below in CI).

AI disclosure

Agent-authored commit and PR — OpenCode running GLM 5.3 Flash (Z.ai), agent mode under human supervision by @mehdikarimi55. The agent performed environment reproduction, root-cause analysis, the code change, and test execution; changes were reviewed by the account owner before submission.

The parity tests' script variants resolve python3 -> python -> py -3 through the ambient PATH. Under `uv run pytest` the venv's bin directory is first on PATH, so the scripts see the suite interpreter (with PyYAML) and CI passes. Under the direct `.venv/bin/python -m pytest` invocation that AGENTS.md prescribes, an ambient python3 without PyYAML fails every composed-template test with "PyYAML is required" (11 failures on macOS). Prepend the running interpreter's own bin directory in parity_helpers.clean_env() so both invocations behave the same; tests that install their own shim prepend afterwards and keep precedence. Delegate test_check_prerequisites_python_parity's duplicate _clean_env() to the shared helper so the fix applies there too. Fixes github#5359 AI disclosure: agent-authored commit — OpenCode running GLM 5.3 Flash (Z.ai), agent mode under human supervision by mehdikarimi55; root-cause analysis, code change, and test execution performed by the agent.

Copilot

🟡 Changes recommended

An active moderate finding remains in tests/parity_helpers.py because Windows resolver logic can select an ambient python3 instead of the suite’s virtual-environment interpreter, leaving parity subprocesses unpinned.

1 open issue

Windows resolver can select ambient python3 instead of the venv · New

What changed in this PR

This test-only PR aligns parity-script subprocesses with the interpreter running the test suite, preventing ambient Python installations from causing composed-template tests to fail due to missing PyYAML.

Changes:

Prepends the suite interpreter’s directory to PATH in the shared parity environment helper.
Reuses the shared helper for check-prerequisites parity tests instead of maintaining a duplicate implementation.
Preserves existing interpreter-shim behavior for negative PyYAML cases.

File	Description
tests/test_check_prerequisites_python_parity.py	Delegates file-local parity environment setup to the shared environment helper.
tests/parity_helpers.py	Pins parity subprocess interpreter lookup to the environment containing the running test-suite interpreter.

_{Review effort: Lite (auto)}

Copilot review follow-up (github#5361): a Windows venv ships python.exe but no python3.exe, so putting the venv's bin directory first on PATH cannot shadow an ambient python3 — the first name the script resolvers probe (command -v python3 succeeds and wins before the python fallback is tried). clean_env() now also prepends a cached python3 shim built with the existing make_python3_path_shim() helper (shell shim plus a .cmd for PowerShell), so parity subprocesses resolve the suite interpreter on every platform. AI disclosure: agent-authored commit — OpenCode running GLM 5.3 Flash (Z.ai), agent mode under human supervision by mehdikarimi55; fix drafted in response to the Copilot review finding, verified by rerunning the affected parity suites.

Copilot

🟡 Changes recommended

A critical helper regression unconditionally starts Bash during environment setup, causing non-Bash Windows parity tests to fail before subprocess execution when Git Bash is unavailable.

1 open issue

Avoid requiring Bash to create the Windows Python shim · New

1 resolved since last review

Windows resolver can select ambient python3 instead of the venv

_{Review effort: Lite (auto)}

Copilot review follow-up (github#5361): _bash_posix_path() shells out to bash on Windows to convert the interpreter path, and a Windows machine with Git Bash not on PATH raised FileNotFoundError from clean_env() itself, failing every parity test during environment setup — including the python and PowerShell variants that never invoke bash. Catch the OSError and fall through to the static /mnt/ form, which the existing returncode fallback already accepts; bash-invoked tests skip when bash is absent, so the converted form is only needed where it can be exec'd. AI disclosure: agent-authored commit — OpenCode running GLM 5.3 Flash (Z.ai), agent mode under human supervision by mehdikarimi55; fix for the review finding, verified locally with the parity suites and a simulated no-bash subprocess failure.

Copilot

🔵 Needs a closer look

The changes add Windows-specific command-resolution and shim behavior, but the available macOS-only evidence cannot validate PowerShell, CreateProcess, or PATHEXT behavior, so a Windows parity run is still needed.

No open issues 🎉

1 resolved since last review

Avoid requiring Bash to create the Windows Python shim

_{Review effort: Lite (auto)}

mehdikarimi55 · 2026-11-10T12:29:25Z

On the Windows parity note: this environment is macOS only, so we cannot run Windows PowerShell 5.1 or exercise CreateProcess/PATHEXT directly. What the branch provides for Windows is by construction: make_python3_path_shim writes a python3.cmd shim only on os.name == "nt" (a .cmd extension is in the default PATHEXT set), and _bash_posix_path falls back to a static POSIX path when bash is missing, so bash-invoked tests skip instead of failing environment setup. A maintainer Windows run would be the meaningful validation and is very welcome.

Disclosure: AI-assisted comment (model: GLM), reviewed and posted by mehdikarimi55.

Copilot

🔵 Needs a closer look

The Windows-specific python3.cmd/PATH and no-Git-Bash fallback behavior could not be independently exercised in the available Linux/macOS environments, so Windows parity coverage is needed before approval.

No open issues 🎉

_{Review effort: Lite (auto)}

mnriem · 2026-11-11T14:05:46Z

Thanks for tracking down the failure under the documented test invocation.
Keeping this fix in the test helpers makes sense, but could you revise the
Windows approach before we merge? The python3.cmd wrapper forwards %*,
which does not preserve the multiline Python source passed through -c
during template composition. The green Windows jobs do not establish that
this works: the relevant composition and fallback tests are skipped behind
requires_bash, and the helpers prefer pwsh rather than explicitly testing
Windows PowerShell 5.1. Please use interpreter pinning that preserves those
arguments, avoid requiring Bash just to construct a Python/PowerShell test
environment, and add positive and negative Windows PowerShell 5.1 coverage
that runs independently of Bash availability. Please also check the tests
that prepend only a YAML-less venv's Scripts directory: the injected
python3 shim can still win over its python.exe, allowing an override test
to pass without exercising the intended condition. The missing-Bash handling
also needs to account for a stalled launcher, not just OSError. This should
remain a focused test-environment fix; no product resolver changes are needed.

AI disclosure: Drafted for @mnriem by GitHub Copilot using GPT-6.1 Sol in
user-directed agent mode; AI performed the assessment and drafted this note.

mehdikarimi55 requested a review from mnriem as a code owner November 10, 2026 10:45

Copilot AI auto review requested due to automatic review settings November 10, 2026 10:45

Copilot started reviewing on behalf of mehdikarimi55 November 10, 2026 10:45

View session

Copilot AI reviewed Nov 10, 2026

View reviewed changes

Comment thread tests/parity_helpers.py Resolved

Copilot AI auto review requested due to automatic review settings November 10, 2026 10:54

Copilot started reviewing on behalf of mehdikarimi55 November 10, 2026 10:54

View session

Copilot AI reviewed Nov 10, 2026

View reviewed changes

Comment thread tests/parity_helpers.py Resolved

Copilot AI auto review requested due to automatic review settings November 10, 2026 11:02

Copilot started reviewing on behalf of mehdikarimi55 November 10, 2026 11:03

View session

Copilot AI reviewed Nov 10, 2026

View reviewed changes

mnriem added the triage-must-have Verdict: high-value, important work for Spec Kit — do first label Nov 10, 2026

mnriem requested an auto review from Copilot November 11, 2026 12:46

Copilot started reviewing on behalf of mnriem November 11, 2026 12:46

View session

Copilot AI reviewed Nov 11, 2026

View reviewed changes

Navigation Menu

Repository navigation

test: pin the suite interpreter on PATH for parity script subprocesses - #5361

test: pin the suite interpreter on PATH for parity script subprocesses#5361
mehdikarimi55 wants to merge 3 commits into
github:mainfrom
mehdikarimi55:fix/5359-parity-tests-ambient-python3-path

mehdikarimi55 commented Nov 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

mehdikarimi55 commented Nov 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

mnriem commented Nov 11, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

mehdikarimi55 commented Nov 10, 2026

Problem

Root cause

Fix

Evidence

AI disclosure

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

mehdikarimi55 commented Nov 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

mnriem commented Nov 11, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mehdikarimi55 commented Nov 10, 2026 •

edited

Loading