Skip to content

fix(ci): run the selection's Python suites when a file is unmapped - #6115

Merged
kyle-sexton merged 3 commits into
mainfrom
fix/ci-unmapped-python-suites
Oct 3, 2026
Merged

kyle-sexton merged 3 commits into
mainfrom
fix/ci-unmapped-python-suites

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

No related issue: follow-up to #6059 (wave 3 PR 2 of the CI performance program); a verifier found its selected Python suites unrun on some pull requests. Refs #3932.

Summary

When a pull request changes a file the test selector cannot map, test-bash falls back to the full shell corpus, and that branch runs no Python suite. Every test_*.py the selector picked in such a pull request went unrun, and the whole-tree runs (schedule, dispatch, a push with no diff base) never ran pytest at all. #6059 dropped the Python import rule, which makes more .py files unmapped, so its replay of 895 merged pull requests leaves 111 selected (PR, Python suite) pairs unrun over 19 PRs, against 85 over 14 with main's selector before it. This change makes both branches run them: 0 unrun.

Fix

.github/workflows/ci.yml, step "Run plugin contract tests" of test-bash:

  • Unmapped file (exit 1 with UNMAPPED:). Keeps the full shell corpus and the two outside-Node packages, then lists this leg's slice of scripts/affected-tests.sh --unmapped-corpus and runs every suite in it that is not a shell suite, the way the exit-3 branch does: test_*.py under pytest, the rest through scripts/run-outside-node-suites.sh --paths. The listing holds the selection's own Python and Node suites, plus every Python suite when the unmapped file is a .py and every Node suite when it is Node, so an unmapped .py now runs the Python corpus as perf(ci): select only the suites a change runs or reads #6059's body said the S9 fallback would. A listing that fails (any exit but 0 or 4) fails the step. A failing shell corpus no longer stops the step before the Python suites run; the step still fails.
  • Whole tree (no diff base). Also runs every Python suite, each leg every legs-th one.
  • One pytest process per Python suite, in all three places. Suites in different directories import same-named helpers (from conftest import ...), so one shared process fails collection once suites from two such directories run together (measured: running the corpus in one process per shard errors on cannot import name 'parsed_output' from 'conftest'). The exit-3 branch had the same latent fault.
  • Eval fixtures are not run. */evals/fixtures/* holds data the skill evals read; two of its test_*.py files error under pytest by design. The Node side already excludes that directory through scripts/outside-node-exclusions.txt.

The --unmapped-corpus wiring that replaces the whole-shell-corpus fallback with each language's own corpus stays with PR 3; this change keeps the shell corpus and adds what it misses.

  • test-bash timeout 11 -> 16 minutes. The first whole-tree run of this change (37141114979) passed on legs 0-2 in 7 to 7.5 minutes; leg 3, which also runs the outside-Node packages and the Node sub-projects, was cancelled at 11 minutes in its Node steps after its Python share had passed (plugins/planning/surface/test_server.py alone took 202 s). 16 is the ceiling for a job here.
  • xargs -t logs each pytest command, so a leg's log names every Python suite it ran.

Verification

Replay check: selected suites each CI branch leaves unrun

The verifier's replay of the 895 pull requests merged in the 7 days to 9671ece (main's selector at 9671ece, #6059's at 71ed6fc, and #6059's --unmapped-corpus listing on each PR with an unmapped file). For every PR, the test-bash branch its exit code takes, and each selected suite that branch runs nowhere. Node counts as run when it sits in a registered package, in a sub-project whose steps run, beside a wrapper that runs, or under the exclusion list.

selector, workflow Python suites unrun Node suites unrun
main's, ci.yml before 85 pairs over 14 PRs (unmapped branch) 65 pairs over 53 PRs (wrapper not selected)
#6059's, ci.yml before 111 pairs over 19 PRs (unmapped branch) 0
#6059's, this ci.yml 0 0

Of the 8 PRs where main's CI ran Python suites and #6059's ran none because a file was unmapped (#5907 dfd7938, #5925 cb30935, b3bb5f3, 0b48d8f, a501f14, 28ed10f, 030bb3d, c94a299), 7 run Python suites again; c94a299's one suite (test_save_point.py) was selected on main through a mention #6059's rules reject, so nothing selects it to run. The whole tree now runs all 139 Python suites, and every Node suite in the tree is owned (6 in packages, 98 in sub-projects, 16 with a wrapper, 6 excluded).

CI

  • Whole tree, dispatch run 37141960428 at c9fb3e2: every job green; test-bash legs 8:03, 6:03, 6:57, 9:23. The xargs -t lines show each of the 139 Python suites (every test_*.py outside */evals/fixtures/*) ran exactly once across the four legs.
  • Unmapped .py, probe test(ci): probe an unmapped Python file through the fallback #6116 (closed unmerged): one comment added to plugins/harness-ops/lib/plugin_cache_versions.py, which the selector reports UNMAPPED. Run 37141969828 at 0f244c3 (c9fb3e2 plus the probe commit): all four test-bash legs green (6:57 to 9:49), each warning the fallback, running its shell-corpus share and then its slice of the Python corpus; the 139 Python suites ran exactly once between them. check-plugins failed there as expected (an unbumped plugin edit to a shared copy).
  • This pull request: run 37143001647 green at c9fb3e2 (4 selected shell suites, exit 0); merge-group run 37143445446 (whole tree) green, test-bash legs 5:28 to 10:55, the longest 5 s under the former 11-minute limit.

Local (WSL Ubuntu 26.04, Python 3.14.7 with both CI requirement files)

  • The step's run: block extracted from this ci.yml and run in a scratch clone, with run-plugin-tests.sh and run-outside-node-suites.sh stubbed to record their arguments:
    • exit 3 (plugins/code-metrics/scripts/report.py changed): exit 0, the 7 selected Python suites each in its own pytest;
    • unmapped .markdownlint-cli2.jsonc: exit 0, shell corpus and the Node packages, no Python suite (none selected);
    • unmapped plugin_cache_versions.py: exit 0, 147 listed, 139 run, the 8 eval fixtures skipped;
    • whole tree, leg 3 of 4: exit 0, 34 of 36 run (2 eval fixtures skipped).
  • The 139 Python suites one pytest process each: all pass. The same suites one process per four-way shard: every shard fails collection on cannot import name 'parsed_output' from 'conftest'.
  • The 8 eval fixtures under pytest: 6 pass, plugins/testing/skills/audit/evals/fixtures/{negative,positive}/test_*.py error at collection.
  • actionlint, scripts/check-docs-only-gate.sh --check, scripts/check-lane-coverage.sh --check, zizmor --offline: clean. scripts/selection-audit.sh's fallback_corpus still finds run-plugin-tests.sh in the UNMAPPED block.

Related

🤖 Generated with Claude Code

An unmapped changed file sent test-bash to the full shell corpus and ran no
Python suite, so every test_*.py the selector picked in that pull request went
unrun, and the whole-tree runs (schedule, dispatch, a push with no base) never
ran pytest at all. #6059 widened the unmapped set by dropping the Python import
rule, so this now hits more pull requests than before.

- The UNMAPPED branch keeps the full shell corpus and also runs this leg's
  slice of the selector's --unmapped-corpus listing that is not a shell suite:
  the selection's Python and Node suites, plus every Python suite when the
  unmapped file is a .py and every Node suite when it is Node.
- The whole tree runs every Python suite, each leg every legs-th one.
- Python suites run one pytest process each: suites in different directories
  import same-named helpers (from conftest import ...), so one process broke
  collection once suites from two such directories ran together.
- Eval fixtures (*/evals/fixtures/*) are data the skill evals read and are not
  run; two of them error under pytest by design.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
kyle-sexton and others added 2 commits October 3, 2026 13:49
A whole-tree dispatch run (37141114979) ran the Python corpus on every leg
for the first time: legs 0-2 passed in 7 to 7.5 minutes, and leg 3, which
also runs the outside-Node packages and the Node sub-projects, was cancelled
at the 11-minute limit in its Node steps after its Python share (the
planning surface server suite alone took 202 s) had passed. 16 minutes is
the ceiling for a job here.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
xargs -t prints every pytest command before it runs, so a leg's log names
each Python suite it ran, not only pytest's count line.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 3, 2026 18:06
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 3, 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-03T18:09:52.030723Z c9fb3e2 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.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 5m 57s —— View job


Code review complete

  • Gather context (PR diff, body, CLAUDE.md/AGENTS.md)
  • Understand request (invoke /review:code-review for PR 6115)
  • Review .github/workflows/ci.yml diff against criteria
  • Post findings (inline + summary)

Summary

Reviewed the diff to .github/workflows/ci.yml (the test-bash job's "Run plugin contract tests" step and its timeout-minutes bump) against origin/main. Scope: correctness and maintainability only — no security lane runs on this repo's CI config per the available conventions, so I checked for security issues too and found none worth flagging.

I traced all three branches that now call the shared pytest_each/run_delegated helpers against the actual behavior of scripts/affected-tests.sh and scripts/run-outside-node-suites.sh (read, not just assumed):

  • Whole-tree branch ([ -z "$DIFF_BASE" ], L2295-2307): the git ls-files | awk ... n++ % legs == leg partition is deterministic across legs (git's index order is stable for a given commit), and the round-robin correctly includes every test_*.py basename once across the four legs.
  • Exit-3 branch (L2317-2322): relies on set -e (re-enabled at L2312) to propagate run_delegated's failure, rather than an explicit status=1; exit. This is a style asymmetry against the exit-1 branch's explicit tracking, but not a bug — there's nothing after run_delegated in that branch, so implicit errexit propagation and explicit tracking produce the same result.
  • Exit-1 (UNMAPPED) branch (L2323-2347): verified against scripts/affected-tests.sh's actual exit contract (read lines 1285-1444) — the second --unmapped-corpus call (no --run) only ever exits 0, 2, or 4, never 3 or 1 when --unmapped-corpus is passed, so the listed != 0 && listed != 4 guard is the correct and complete check. The grep -v '\.test\.sh$' filter correctly strips shell suites from the listing because the repo's shell-suite naming convention is uniformly *.test.sh (confirmed from scripts/affected-tests.sh's own header), not just a Windows-specific suffix.
  • A plausible latent concern I chased down and ruled out: whether a Pester (*.Tests.ps1) suite pulled into the --unmapped-corpus listing (e.g., from an unmapped .ps1/.psm1 file) would leak into outside-node-paths.txt and get mis-handled by scripts/run-outside-node-suites.sh --paths on this Linux job. It doesn't — that script already classifies *.Tests.ps1 paths as owned by the Windows Pester lane and skips them (confirmed by reading run-outside-node-suites.sh). No repo suites currently match that pattern anyway (only .fixture-suffixed and doc-referenced ones exist), so this isn't reachable today regardless.

No correctness bugs found in the diff; the control flow for all three branches is internally consistent and matches the PR description's claims about what each path now runs. No inline comments needed.

Note on verification claims: the PR body's replay-script numbers, the linked CI run outcomes, and the local WSL verification steps are author-claimed; I did not independently re-run them and am not asserting them as verified here.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 14s —— View job


Security review

  • Gather context (PR diff, changed files)
  • Apply skip gate
  • Review .github/workflows/ci.yml diff for security-relevant issues
  • Check for instruction-surface deletions
  • Post findings

Scope: only file changed is .github/workflows/ci.yml (test-selection logic inside the test-bash job's "Run plugin contract tests" step).

No security issues found.

Reasoning:

  • The diff only changes which already-tracked repository files (test_*.py suites, outside-Node suite paths) get executed by pytest/run-outside-node-suites.sh, and how the shell/Python corpora are sharded across legs. It does not introduce new handling of secrets, tokens, credentials, or external/untrusted network input.
  • All filename lists are passed through xargs -0 ... -n1 (via the new pytest_each/run_delegated helpers at ci.yml#L2277-L2295), so each path is passed as a single argument rather than re-interpreted by a shell — no new command-injection surface from filenames with spaces/metacharacters.
  • DIFF_BASE (used at ci.yml#L2335) is unchanged from the existing needs.scope.outputs.lane_base/steps.base.outputs.ref computed value, not raw attacker-controlled text, and its use here is unchanged from the existing exit-3 branch.
  • Executing test files that are already committed to the PR branch is the pre-existing trust model of this CI (any PR can add/modify test_*.py files it already causes to run); this change doesn't widen that — it only closes a gap where some selected Python suites previously went unrun, per the PR's own stated goal.
  • No pull_request_target/workflow_run triggers, no permissions: changes, no secrets exposed in run: blocks, and no unpinned/loosened action references — GitHub Actions hardening concerns of that kind are zizmor's lane per this skill's scope and are out of scope here regardless.
  • No standing instruction (CLAUDE.md/AGENTS.md/rules/skill or agent bodies/hook allow-deny lists) is touched by this diff, so the instruction-surface-deletion check does not apply.

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

claude-security-review has reviewed this pull request through c9fb3e2; a later push is reviewed from there.

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

claude-review has reviewed this pull request through c9fb3e2; a later push is reviewed from there.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 3, 2026
Merged via the queue into main with commit ceed24b Oct 3, 2026
54 of 56 checks passed
@kyle-sexton
kyle-sexton deleted the fix/ci-unmapped-python-suites branch October 3, 2026 18:26
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