Skip to content

fix(scripts): run pytest suites under the setup-env venv, building it once when python3 lacks pytest - #9876

Merged
kyle-sexton merged 4 commits into
mainfrom
fix/test-runner-venv-self-heal
Oct 25, 2026
Merged

kyle-sexton merged 4 commits into
mainfrom
fix/test-runner-venv-self-heal

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: a local-host gap found when /source-control:pull-request ready refused three PRs; no issue was filed for it.

Summary

scripts/run-test-file.sh and scripts/selection-audit.sh ran Python suites as python3 -m pytest. On a host whose python3 has no pytest and is PEP 668 externally managed (pip install --user refuses), every Python suite run through them fails to start.

There are two observations, and they do not fully agree:

  • Reading the code on this base, scripts/check-affected.sh --tests does not run .py suites itself. affected-tests.sh --run runs only *.test.sh and lists the other ecosystems as not run (exit 3), which check-affected.sh counts as a pass. The *.test.sh wrappers that call pytest SKIP when it is missing.
  • On the refused PRs' heads, which lacked pytest, the checklist worker saw check-affected count the 14 selected .py suites as FAIL, and the ready gate recorded fail. Agents in the §1.5 verify step also run Python suites through run-test-file.sh or python3 -m pytest, and those runs fail the same way.

Either way the cause is the same: no interpreter with pytest. This change gives every local Python suite run one.

Metric: ready flips refused for missing pytest. Baseline: 3 today (#9836, #9798, #9835, all passing under unittest). Expected after this change: 0.

Fix

  • New sourced helper scripts/lib/pytest-python.sh, the one place that chooses the pytest interpreter. First match wins:
    1. the python of a scripts/setup-env.sh venv that imports pytest: SETUP_ENV_VENV, then this checkout's .work/venv, then the main checkout's .work/venv;
    2. python3, when it imports pytest;
    3. off CI, when the checkout has setup-env's inputs: one setup-env.sh run that builds the venv at SETUP_ENV_VENV or the main checkout's .work/venv, under flock so parallel callers build it once. setup-env has no Python-only step, so this run also checks ShellCheck and runs npm ci where the lockfile hash changed; its output goes to stderr;
    4. python3, so pytest's own error names what is missing.
  • run-test-file.sh and selection-audit.sh trace both use it. selection-audit.sh resolves the interpreter once, before any trace, and only when a .py suite is in the corpus.
  • The venv defaults to the main checkout's .work/venv (found through git rev-parse --git-common-dir), so every worktree of a clone shares one venv instead of building one each. It is about 114 MB on this host.
  • CI is unchanged. Step 3 never runs when CI or GITHUB_ACTIONS is set, and the workflows already install pytest (setup-env.sh puts the venv on GITHUB_PATH; the lane's verify step pip-installs .github/requirements-ci.txt), so step 2 resolves to python3 there as before.

Verification

  • New scripts/lib/pytest-python.test.sh: 9 cases through run-test-file.sh in a throwaway checkout plus a linked worktree, with stubbed python3, venv python and setup-env.sh. It covers the checkout's venv, the main checkout's venv used from a worktree, SETUP_ENV_VENV first, python3 with pytest, setup-env run exactly once from a worktree (building the main checkout's venv) with no second run, no build under CI or GITHUB_ACTIONS, and a failed setup-env falling back to python3. Against the old run-test-file.sh, 6 of the 9 fail. The 3 that pass check behavior this change leaves alone.
  • scripts/selection-audit.test.sh: one new trace case, where a Python suite runs under the checkout's venv python. 20 of 20 pass; the new case fails against the old script.
  • collect-base-activity.test.mjs: 127 of 127 pass, now with the helper staged beside checkout-path.sh (the real lane copies the whole base scripts/).
  • scripts/setup-env.test.mjs: 17 of 17 pass. live-config.test.mjs: 40 of 40 pass.
  • On the host (python3 3.14.8, no pytest, no venv): the first scripts/run-test-file.sh scripts/test_vendor_gitleaks_rules.py from a worktree built the main checkout's venv in about 11 s, then 14 passed. A second run took 1.3 s and ran no setup-env. A sweep of every tracked test_*.py outside /evals/fixtures/ through run-test-file.sh: 194 of 195 suites passed in 1164 s. In the other, plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py, 2 of 719 tests failed with vcs-state-unverified. That file then passed 717 of 717 (2 skipped) on two reruns through the same runner, and the two tests pass in isolation, so this looks like a flake in that suite's VCS probe and not something this change causes (cause UNVERIFIED).
  • scripts/check-affected.sh --tests: 10 passed, 4 skipped. shellcheck --rcfile .shellcheckrc is clean on every touched script, and shfmt -d is clean on the new files.

Related

  • scripts/setup-env.sh, which is unchanged; the helper reuses its venv and input-hash skip.

🤖 Generated with Claude Code

kyle-sexton and others added 2 commits October 24, 2026 17:16
… once when python3 lacks pytest

run-test-file.sh and selection-audit.sh ran Python suites as
python3 -m pytest, which fails on a host whose python3 has no pytest and
refuses pip install --user (PEP 668). Both now take the interpreter from
scripts/lib/pytest-python.sh: a setup-env venv that imports pytest
(SETUP_ENV_VENV, this checkout's .work/venv, then the main checkout's,
shared by every worktree), else python3 when it imports pytest, else,
off CI, the main checkout's venv after one scripts/setup-env.sh run
under a lock. Under CI or GITHUB_ACTIONS nothing is built, so CI keeps
running python3 as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 25, 2026 01:22
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-25T01:24:18.558429Z 9f13fa7 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed e55545395f8ba1bde408788ab11c6085d5b2e3a5: 0 finding(s), 0 as inline comments.

Security review: PR #9876 (head e555453)

No security findings.

critical-findings: 0

Scope read. I read the whole diff (6 files) and the PR context. I also read scripts/run-test-file.sh, the lane wrapper staging in .github/actions/collect-base-activity/action.yml (lines 190-300), pr-run-activity.md (lines 420-460) and the pytest setup step in pr-run-activity-write.yml (lines 999-1010).

Trusted paths. No changed file matches .github/standards/trusted-actors/**, .github/actions/check-trusted-trigger/**, **/CODEOWNERS or **/.gitmodules.

Manifests and instruction surfaces. No plugin.json or marketplace.json changed, so there are no manifest <field> lines. No instruction file, rules file or hook allowlist was deleted or softened, so I did not check the instruction exception register.

Spend. No billed resources or paid plans are touched. I found no extra review or lane runs that would use prepaid credit.

What I checked and why it did not become a finding

  • Token handling. run-test-file.sh still runs unset GH_TOKEN GITHUB_TOKEN (line 31) before pytest_python::resolve is called on line 33. Neither the new interpreter probe nor the setup-env build sees the token. run-test-file.sh also still validates the path before resolving, using checkout_path::require, the [A-Za-z0-9._/-] character check and -f. The interpreter is called with -m pytest ... -- "$1", so the argument cannot be read as an option.
  • Head-checkout venv ($top/.work/venv/bin/python).
    • In the lane, run-test-file runs in the PR head checkout, and the helper tries that checkout's .work/venv before python3. A head could force-add an executable there.
    • That gives no new capability. The same lane already runs the head's test_*.py under pytest with the network open, and pr-run-activity.md records this ("Its tests are checkout code with the network open").
    • The token exposure it already documents is unchanged.
    • As optional hardening, step 1 could skip the checkout-local and main-checkout venvs when CI or GITHUB_ACTIONS is set, so CI always uses the hash-pinned python3 the workflow installs. I'm not flagging it, because there is no privilege difference to exploit.
  • Setup-env build (step 3). It runs only when CI and GITHUB_ACTIONS are both unset and python3 has no pytest. In the lane, GITHUB_ACTIONS is not cleared by the wrapper. Also, Install the pinned pytest for the test-file runner puts pytest on python3 before the head checkout, so step 2 returns first. Off CI, it runs the checkout's own scripts/setup-env.sh with the checkout's inputs. That is the same trust level as running the checkout's test file.
  • Shell injection. $venv, $top and $main are always quoted. SETUP_ENV_VENV is operator-set and is passed as an argument or environment variable, never through eval. The eval in pytest-python.test.sh only evaluates literals written in that test file.
  • selection-audit.sh. AUDIT_PYTHON is resolved once and exported to the trace_one subshells. It is used as "$AUDIT_PYTHON" inside an array, so it is not word-split. The scheduled maintenance workflow runs it on trusted base code.
  • Lock file. The flock lock lives at $venv.lock, inside the gitignored .work/. It creates no symlink or traversal surface.

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed e55545395f8ba1bde408788ab11c6085d5b2e3a5: 0 finding(s), 0 as inline comments.

Code review: no findings

I read the full diff (6 files) and the PR context. I did not run any tests, because this lane has no shell. Everything the PR says about passing runs is author-claimed and unverified by me.

What I checked

  • scripts/lib/pytest-python.sh resolution order: the venv candidates (SETUP_ENV_VENV, the checkout's .work/venv, the main checkout's .work/venv), then python3 with pytest, then a one-time setup-env.sh build off CI, then plain python3. I found no logic error.
  • Concurrency and retry: flock serializes parallel callers. setup-env.sh writes its .setup-env stamp only after the pip install succeeds (scripts/setup-env.sh:101). A half-built venv is therefore rebuilt on the next run, not skipped.
  • CI behavior: the CI and GITHUB_ACTIONS guard keeps the python3 path unchanged, and the tests cover both variables.
  • scripts/selection-audit.sh: AUDIT_PYTHON is set and exported in cmd_trace before export -f trace_one and the xargs fan-out. trace_one is called only from there, so set -u cannot hit an unbound variable. The venv lives outside the tracked files, so it does not change the strace read-set filter.
  • scripts/run-test-file.sh: the exec "$(pytest_python::resolve)" call is safe. resolve always prints an interpreter, and its build output goes to stderr, so it cannot corrupt the command line.
  • The collect-base-activity.test.mjs change stages the new helper beside checkout-path.sh. That is needed because run-test-file.sh now sources it.
  • Tests: the new pytest-python.test.sh cases cover each branch of the resolution order, the worktree case and the failed-setup fallback. selection-audit.test.sh gains a case for the trace path.

Other checks

  • Closing issues: none. The PR says "No related issue", so there are no acceptance criteria to check.
  • Manifests: no plugin.json or marketplace.json changes, so no manifest lines apply.
  • Trusted paths: none of the changed files match the listed globs.
  • Paved paths and path-scoped rules: I did not find or read a standards-index row or a .claude/rules file for this change, so I report nothing under them.
  • Cost: none. The change adds no billed resource and no paid-plan setting.

Observation, not a finding

  • When a parallel caller waits on the flock, it runs setup-env.sh again after the first build finishes. The hash stamp makes that run a cheap skip, so the PR's "builds once" claim holds in effect.

kyle-sexton and others added 2 commits October 25, 2026 02:49
The pipefail-grep-q hygiene check rejects printf | grep -q; feed grep
from a process substitution instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 25, 2026
Merged via the queue into main with commit 3335800 Oct 25, 2026
36 checks passed
@kyle-sexton
kyle-sexton deleted the fix/test-runner-venv-self-heal branch October 25, 2026 07:39
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.

1 participant