Fan the Validator's Pytest Leg Out Over a python-versions Input by ptr727 · Pull Request #1484 · ptr727/ProjectTemplate

ptr727 · 2026-09-09T17:36:03Z

Addresses #1475.

validate-task.yml pinned the interpreter in both places that use one and exposed no input to change it, so a repository whose package claims more than one interpreter lost every leg but 3.13 on adoption. Nothing failed and nothing warned: the required check went green on the one version the task ran while the repository went on claiming the rest.

What changes

A python-versions input, a JSON array defaulting to ["3.13"], that the unit-test job reads through fromJSON as its matrix, one leg per entry, with fail-fast: false so a failing interpreter reports beside a passing one rather than cancelling it. Each leg's Codecov upload carries a python-<version> flag, which keeps the legs apart in the dashboard rather than merging them into one number.

A caller naming nothing runs the single leg this job ran before. Two things move on adoption, and the adopter docs say so rather than claiming nothing changes: the nested check reads Unit test job (Python 3.13) rather than Unit test job, which binds no ruleset since only the caller's own aggregator is bound, and a repo that already uploaded Python coverage here uploaded it unflagged, so that series stops and a python-3.13 one starts beside it.

Why the input is checked

An entry holding no non-whitespace character is the value that would otherwise carry a run green on an interpreter nobody chose. setup-uv trims what it reads and then reads an empty input as an absent one, so the leg expands, tests on whatever uv picks, and reports success under the flag python- and the name Unit test job (Python ). A caller reaches that by interpolating an unset value of its own, and a stray space beside it reaches the same place, which is why each entry is tested for content rather than for length.

An unquoted entry is the other half of the same slip and announces itself either way. fromJSON admits a JSON array of numbers, so '[3.13, 3.14]' expands into legs; a decimal form that survives the round trip runs the right interpreter, while one that loses a trailing zero, 3.10 becoming 3.1, fails in the Sync dependencies step rather than in setup-uv, which exports the version rather than resolving it. What the check buys there is a named cause rather than a silent pass prevented.

The check is the unit-test job's entry step, the shape GOVERNANCE.md "Workflow YAML Conventions" gives for an assertion whose guarded work is in the same job. It reads the raw input rather than matrix.python-version, which by then cannot say whether its 3.2 was written 3.2 or 3.20. An empty array, and anything that is not a JSON array at all, never reach it: the matrix is expanded before any step runs, so those fail there, loudly and with an unnamed cause. A job of its own would name them too, and costs a job allocation on the critical path of every pull request and publish run in the fleet to do it.

What is deliberately not in it

The lint job stays on the one interpreter the task pins, which costs a repository two things. Its type check is performed at that interpreter, mypy and pyright each defaulting their target to the one they run under where the repository's own section does not name a version, while ruff takes its target from requires-python where a [project] table declares one. That much a repository settles in its own configuration. What it cannot settle there is a lockfile: one carrying a uv.lock whose requires-python floor is above the pinned interpreter fails the lint job's sync step outright, and no value of python-versions moves that job. #1475 called the lint job a separate decision, and that half is filed rather than taken here.

A caller carrying a .NET test project as well repeats that half of the job once per leg, the matrix being on the job rather than on its Python steps. That spends runner minutes and re-uploads an identical report rather than measuring anything new. Splitting the two halves into jobs of their own is what that costs, and no repository in the fleet carries both a .NET test project and more than one interpreter today.

On the tests

test_validator_pytest_leg_fans_out_over_every_named_interpreter asserts the matrix expression, the uv setup reading the matrix rather than a literal, fail-fast: false, the per-leg flag, the job name's matrix interpolation, and the default's shape rather than its content, so neither bumping the fleet default nor adding a second interpreter to it is a regression there.

test_validator_refuses_a_python_versions_value_fromjson_would_admit lifts the entry step's whole run: script out of the workflow and executes it over ten values, and asserts the step is that job's first, matched on the step's dash rather than on a name: key so a step leading with uses: cannot slip in ahead unseen. Running the script rather than its filter alone is what makes the test cover the gate: inverting the condition and exiting zero each leave a filter-only assertion green while the step admits everything.

Eight regressions were run against these tests and each one fails them: dropping the per-entry content test, narrowing it back to a length test, inverting the condition, exit 0 instead of exit 1, weakening the filter to an array check, re-pinning the interpreter literal, removing the matrix interpolation from the job name, and inserting a bare uses: step ahead of the guard.

Verification

prose_lint.py, repo_gate.py across eol, eol-coverage and sha-pin, spec/validate.py, spec/audit.py --selftest, spec/workflow_reuse.py --selftest, build_dist.py --check, ruff check, ruff format --check, mypy, the full unittest suite at 1170 passed, and docker_lint.py over actionlint, markdownlint, editorconfig-checker and cspell. canonical_review.py check reports no carried canonical unit changed, and a local_review.py receipt covers this content.

Nine adversarial review rounds ran over this branch before it was pushed, raising 45 findings. Two shaped the result rather than polishing it. The guard was first placed in the lint job, where a matrix expanding per job meant it gated nothing; and its filter twice admitted a value that would run green on an unpinned interpreter, [""] and then [" "], the second because setup-uv trims before it decides an input is absent. One measured hole is left deliberately: [""] is admitted, since jq's \S treats a BOM as non-whitespace while JavaScript's String.trim() strips it. Reaching it means typing a BOM codepoint inside a JSON array literal in a caller's own workflow YAML, the fleet charset gate catches that upstream, and widening the regex to chase one codepoint is enumeration growth rather than a fix.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ADz7dCBe7e1uosGx779vTw

Addresses #1475. `validate-task.yml` pinned the interpreter in both places that use one and exposed no input to change it, so a repository whose package claims more than one interpreter lost every leg but 3.13 on adoption. Nothing failed and nothing warned: the required check went green on the one version the task ran while the repository went on claiming the rest. ## What changes A `python-versions` input, a JSON array defaulting to `["3.13"]`, that the `unit-test` job reads through `fromJSON` as its matrix, one leg per entry, with `fail-fast: false` so a failing interpreter reports beside a passing one rather than cancelling it. Each leg's Codecov upload carries a `python-<version>` flag, which keeps the legs apart in the dashboard rather than merging them into one number. A caller naming nothing runs the single leg this job ran before. Two things move on adoption. The nested check reads `Unit test job (Python 3.13)` rather than `Unit test job`, which binds no ruleset since only the caller's own aggregator is bound, and it reads that way whether or not a Python step ran, so a .NET-only caller reports a version it never used. And a repo that already uploaded Python coverage here uploaded it unflagged, so that series stops and a `python-3.13` one starts beside it. ## Why the input is checked An entry holding no non-whitespace character is the value that would otherwise carry a run green on an interpreter nobody chose. `setup-uv` trims what it reads and then reads an empty input as an absent one, so the leg expands, tests on whatever uv picks, and reports success under the flag `python-` and the name `Unit test job (Python )`. A caller reaches that by interpolating an unset value of its own, and a stray space beside it reaches the same place, which is why each entry is tested for content (`test("\\S")`) rather than for length. That is the case that justifies the step. An unquoted entry is the other half of the same slip and announces itself either way. `fromJSON` admits a JSON array of numbers, so `'[3.13, 3.14]'` expands into legs; a decimal form that survives the round trip runs the right interpreter, while one that loses a trailing zero, `3.10` becoming `3.1`, fails in the `Sync dependencies step` rather than in `setup-uv`, which exports the version rather than resolving it. What the check buys there is a named cause rather than a silent pass prevented, and an earlier draft of this change claimed otherwise. The check is the `unit-test` job's entry step, the shape `GOVERNANCE.md` "Workflow YAML Conventions" gives for an assertion whose guarded work is in the same job. It reads the raw input rather than `matrix.python-version`, which by then cannot say whether its `3.2` was written `3.2` or `3.20`. An empty array, and anything that is not a JSON array at all, never reach it: the matrix is expanded before any step runs, so those fail there, loudly and with an unnamed cause. A job of its own would name them too, and costs a job allocation on the critical path of every pull request and publish run in the fleet to do it. ## What is deliberately not in it The `lint` job stays on the one interpreter the task pins, which costs a repository two things. Its type check is performed at that interpreter, `mypy` and `pyright` each defaulting their target to the one they run under where the repository's own section does not name a version, while `ruff` takes its target from `requires-python` where a `[project]` table declares one. That much a repository settles in its own configuration. What it cannot settle there is a lockfile: one carrying a `uv.lock` whose `requires-python` floor is above the pinned interpreter fails the lint job's sync step outright, and no value of `python-versions` moves that job. The adopter docs say both, rather than resting on an unexamined "linting twice reports the same findings". A caller carrying a .NET test project as well repeats that half of the job once per leg, the matrix being on the job rather than on its Python steps. That spends runner minutes and re-uploads an identical report rather than measuring anything new. Splitting the two halves into jobs of their own is what that costs, and no repository in the fleet carries both a .NET test project and more than one interpreter today. ## On the tests `test_validator_pytest_leg_fans_out_over_every_named_interpreter` asserts the matrix expression, the uv setup reading the matrix rather than a literal, `fail-fast: false`, and the per-leg flag, plus the default's shape rather than its content, so neither bumping the fleet default nor adding a second interpreter to it is a regression there. `test_validator_refuses_a_python_versions_value_fromjson_would_admit` lifts the entry step's whole `run:` script out of the workflow and executes it over eight values, and asserts the step is that job's first, matched on the step's dash rather than on a `name:` key so a step leading with `uses:` cannot slip in ahead unseen. Running the script rather than its filter alone is what makes the test cover the gate: inverting the condition and exiting zero each leave a filter-only assertion green while the step admits everything. Eight regressions were run against these tests, those two plus dropping the per-entry content test, narrowing it back to a length test, weakening the filter to an array check, a re-pinned interpreter, a job name with the matrix interpolation removed, and a bare `uses:` step inserted ahead of the guard. Each one fails them. ## Verification `prose_lint.py`, `repo_gate.py --check eol-coverage`, `spec/validate.py`, `spec/audit.py --selftest`, `spec/workflow_reuse.py --selftest`, `build_dist.py --check`, `ruff check`, `ruff format --check`, `mypy`, the full `unittest` suite at 1170 passed, and `docker_lint.py` over actionlint, markdownlint, editorconfig-checker and cspell. `canonical_review.py check` reports no carried canonical unit changed. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-09-09T17:36:10Z

Warning

Review limit reached

Next included review available in 41 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: c8d69c33-f71c-42c4-8e5a-d5d39d71db3e

📥 Commits

Reviewing files that changed from the base of the PR and between c23a4b8 and 3f487a6.

📒 Files selected for processing (3)

.github/workflows/validate-task.yml
docs/reusable-workflows.md
scripts/tests/test_release_guards.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Copilot

🟡 Changes recommended

The new ::error:: annotation echoes unescaped input and should escape %/newlines to avoid GitHub Actions workflow-command injection.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR extends the hub validate-task.yml reusable workflow to support multi-interpreter pytest runs via a new python-versions input, updates documentation to describe the new interface and adoption impact, and adds regression tests to keep the matrix/guard behavior stable.

Changes:

Add a python-versions workflow input and use it to drive a unit-test job matrix with fail-fast: false, plus per-leg Codecov flags.
Add unittest coverage asserting the new matrix wiring and validating the input-guard script behavior.
Update reusable-workflow documentation to explain how callers should set python-versions and what changes on adoption.

File summaries

File	Description
`.github/workflows/validate-task.yml`	Adds `python-versions` input, fans out `unit-test` via a matrix, and flags Codecov uploads per interpreter.
`scripts/tests/test_release_guards.py`	Adds tests asserting the new matrix expression and the `python-versions` guard behavior.
`docs/reusable-workflows.md`	Documents the new `python-versions` input, behavior, and adoption implications.

Review details

Files reviewed: 3/3 changed files
Comments generated: 1
Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

The guard's ::error:: echoed the raw python-versions value, so a value holding a newline would end the annotation and let whatever followed it start a workflow command of its own. Raised by Copilot on #1484. Redirecting to stderr, the other remedy offered, closes nothing: the runner reads a workflow command off stderr as readily as off stdout. So the value is escaped per the workflow command encoding rules instead, and it stays in the annotation, which is the whole point of a named cause. The percent substitution runs first. Running it after the others would re-encode the percent each of them had just introduced, turning %0A into %250A. ## On the tests Three assertions cover it. A newline-carrying value must leave the step emitting one line and that line must hold %0A, which is what proves no second command was started. A bare percent must reach the annotation as %25. Three regressions were run against them and each fails: echoing the raw value rather than the escaped one, dropping the newline substitution, and moving the percent substitution last. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

The \r substitution was the one escape with no assertion behind it: deleting the line left the suite green while a CR-bearing value reached the annotation raw, which is the class of defect the escaping was added for. A case now asserts it renders %0D, and removing the line fails it. The percent case's comment also claimed to prove the substitutions run percent-first, which it cannot: its input carries no newline, so both orderings render %25. The %0A case is what covers the order, since substituting the percent last renders %250A there. The comment now says so. Both found by a local review pass over the previous commit. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Three surfaces claimed an unquoted entry "announces itself either way", so the check only ever named a cause there rather than preventing a silent pass. That is false for an integer. fromJSON parses [3] to the number 3, the leg expands, and uv resolves UV_PYTHON=3 to whatever interpreter of that major version it finds, exit 0 and no warning, which is green on an interpreter nobody chose. Measured on uv 0.12.4: UV_PYTHON=3 resolves, UV_PYTHON=3.1 errors. So the guard prevents two silent passes rather than one, the blank entry and the integer entry, and names a cause for the decimal that loses a trailing zero. The workflow comment, the adopter docs, and the case table each say that now, and a [3] case covers it. The claim mattered beyond accuracy: an editor reading the old wording would have had the check down as cosmetic on that half, which is the reasoning that deletes it. Found by a local review pass over the previous commit. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Two defects from the previous commit, both found by a local review pass over it. The test's own docstring still carried "an unquoted entry announces itself either way", the sentence that commit existed to remove. Three surfaces moved and the fourth did not, so the test covering the case taught the opposite of what it asserts. `grep` now finds the phrase nowhere. That commit also replaced the false claim with an over-precise one: an integer does not resolve to "whatever interpreter of that major version uv finds". Measured on uv 0.12.4, uv reads a dotless digit run as a full version rather than a major-version wildcard, so 313 and 314 select exactly 3.13 and 3.14, a bare 3 selects a 3.x, and -3, 0, 30 and 300 each fail. The conclusion holds, since some integers resolve and the leg then runs green on an interpreter nobody chose, so the wording now states that category and stops, rather than describing a rule it had wrong. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟢 Approval recommended

The workflow change is covered by targeted regression tests (including injection/escaping cases) and the documentation matches the implemented behavior.

Review details

Files reviewed: 3/3 changed files
Comments generated: 0 new
Review effort level: Lite

ptr727 · 2026-09-09T20:37:10Z

@coderabbitai review

coderabbitai · 2026-09-09T20:37:17Z

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…g, and the Release-Trigger Retirement (#1489) Promotes thirteen merged pull requests from `develop` to `main`. ## What this promotes - **#1486** State the `host-tools.json` carry rule and stop the empty-overlay warning. `spec/files.json` now requires the declaration unconditionally, and `host_gate.py` no longer warns on an empty one. - **#1484** Fan the validator's pytest leg out over a `python-versions` input, so a caller with more than one interpreter no longer silently loses its other legs to a pinned 3.13, with an entry guard that refuses a value `fromJson` would otherwise admit. - **#1476** Retire the publish-on-merge release trigger, which `spec/files.json` applied to an interface contract whose task has no publish-on-merge path. - **#1479** Ship the release caller snippet and retire the not-yet-released claim. - **#1474** Exclude `node_modules` from markdownlint and Docker lint discovery at any depth. - **#1468** Move the carried-surface cluster off decision, recording the measurements and what stays unmeasured on the issues that hold them. - **#1498** Correct the release-caller snippet's publish-on-merge claim, raised by CodeRabbit against this promotion and fixed on its own branch, since a promotion PR's head is `develop`. - **#1494** Settle the markdown lint exclusion seam as a nested config. - **#1492** Check declared deployment environments and fix the carried-payload rule. - **#1501** Move the session handoff to a chain of labeled issues. Two of its units are verbatim fidelity, `AGENTS.md` "Session Scope" and `GOVERNANCE.md` "Communicating with the User", so every fleet repo picks them up on its next resync. - **#1505** Refuse a bare handoff link rather than reading it as no successor, answering a finding raised against this promotion. - **#1504** Reject a deployment environment name or branch carrying surrounding whitespace, answering two more. - **#1506** Encode an environment name before it becomes a path segment, delete a branch that cannot fire, and dedupe a registry name by case, answering two more and declining one on a reproduction. ## Closing references A `develop`-targeted pull request registers no closing reference, so the issues those pull requests settled close with this promotion. #1470 is the one that needs a word. Two of its seven acceptance items are not met as literally written, and the maintainer's decision to close it here anyway, with the basis for both, is recorded on the issue itself. Closes #1483 Closes #1473 Closes #1487 Closes #1482 Closes #1475 Closes #1470 ## Left open deliberately - **#1499**, **#1500** follow-ups #1501 filed rather than fixed. - **#1503** the validator-looser-than-its-consumer class, filed from findings on this promotion and deliberately not closed by it. Five instances recorded, and the fix is a design decision rather than a check. - **#1497** the Docker delta list never says to remove the `push:` block. - **#1472** the fleet-level pin call, deliberately not pre-empted by #1479. - **#1321**, **#1464** carry #1468's decision record and remain in their own scope. ## Verification `origin/main..origin/develop` merges clean, no end-of-line conflict. Each promoted pull request ran the fleet gate set on its own branch before merging; this promotion adds no content of its own. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01817rckKywjfoaEV5WF9TKs

Copilot AI lite review requested due to automatic review settings September 9, 2026 17:36

Copilot started reviewing on behalf of ptr727 September 9, 2026 17:36

View session

ptr727 mentioned this pull request Sep 9, 2026

validate-task.yml's lint job pins Python 3.13, narrowing the type check and blocking a uv.lock repo above that floor #1485

Closed

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Comment thread .github/workflows/validate-task.yml Resolved

ptr727 and others added 4 commits September 9, 2026 12:59

Copilot AI review requested due to automatic review settings September 9, 2026 20:32

Copilot started reviewing on behalf of ptr727 September 9, 2026 20:33

View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

ptr727 merged commit 81f0047 into develop Sep 9, 2026
9 checks passed

ptr727 deleted the feature/validate-task-python-versions branch September 9, 2026 20:42

Navigation Menu

Repository navigation

Fan the Validator's Pytest Leg Out Over a python-versions Input - #1484

Fan the Validator's Pytest Leg Out Over a python-versions Input#1484
ptr727 merged 5 commits into
developfrom
feature/validate-task-python-versions

ptr727 commented Sep 9, 2026

Uh oh!

coderabbitai Bot commented Sep 9, 2026 •

edited

Loading

Review limit reached

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

ptr727 commented Sep 9, 2026

Uh oh!

coderabbitai Bot commented Sep 9, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ptr727 commented Sep 9, 2026

What changes

Why the input is checked

What is deliberately not in it

On the tests

Verification

Uh oh!

coderabbitai Bot commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

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

🟢 Approval recommended

Uh oh!

ptr727 commented Sep 9, 2026

Uh oh!

coderabbitai Bot commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Sep 9, 2026 •

edited

Loading

coderabbitai Bot commented Sep 9, 2026 •

edited

Loading