From 3b9ebac0bbeb0e840c2bb420a0c71526c71235bd Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Wed, 9 Sep 2026 08:48:04 -0700 Subject: [PATCH 1/5] Fan the Validator's Pytest Leg Out Over a python-versions Input 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-` 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) --- .github/workflows/validate-task.yml | 39 +++++++++-- docs/reusable-workflows.md | 22 +++++- scripts/tests/test_release_guards.py | 101 +++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 5 deletions(-) diff --git a/.github/workflows/validate-task.yml b/.github/workflows/validate-task.yml index 4285923ed..725ecd567 100644 --- a/.github/workflows/validate-task.yml +++ b/.github/workflows/validate-task.yml @@ -1,9 +1,9 @@ name: Validate task # The fleet validation gate, hosted here once and reached by every repo's test-pull-request stub and its own publish-release stub. -# Three jobs: lint (the fleet doc-lint block plus language lint by tree detection, the prose gate, and the repo gate), unit-test (a generic dotnet test or pytest, skipped where the caller has no test project), and validate (the validate hook, a repo's own domain checks such as an ESPHome compile, a Hugo build, a KiCad ERC, a codegen-drift check, or PowerShell tests). +# Three jobs: lint (the fleet doc-lint block plus language lint by tree detection, the prose gate, and the repo gate), unit-test (a generic dotnet test or pytest, skipped where the caller has no test project, and run once per interpreter the python-versions input names), and validate (the validate hook, a repo's own domain checks such as an ESPHome compile, a Hugo build, a KiCad ERC, a codegen-drift check, or PowerShell tests). # No permissions beyond contents: read where a job needs one, since every job here only checks out and reads. -# No required inputs, markdown-exclude-globs and repo-gate-exclude-globs are the two optional inputs, and CODECOV_TOKEN is the one optional secret, since coverage upload is best-effort. +# No required inputs, markdown-exclude-globs, repo-gate-exclude-globs and python-versions are the three optional inputs, and CODECOV_TOKEN is the one optional secret, since coverage upload is best-effort. # Hub-owned gates and default hooks resolve through $/ at the reusable workflow's commit, so each implementation is reproducible against the caller's released pin without a second checkout. on: workflow_call: @@ -20,6 +20,12 @@ on: required: false type: string default: '' + # Read through fromJSON into the unit-test job's matrix, which is expanded before any step runs, so the check below reaches only the values that expand at all. + python-versions: + description: JSON array of the Python versions the pytest leg runs under, one leg per entry, for example ["3.13", "3.14"]. Quote each entry, since an unquoted one is a JSON number rather than a version string. + required: false + type: string + default: '["3.13"]' secrets: CODECOV_TOKEN: required: false @@ -260,14 +266,37 @@ jobs: # A caller with neither a *Tests*.csproj nor a tests/ directory beside a pyproject.toml and a dependency manifest runs every step's guard false, and the job reports success having done nothing, which is the clean skip this job promises. # That manifest is a committed uv.lock or a root requirements*.txt, the two dependency mechanisms spec/project-types.json names, so a pip-based Python repo with tests is served here rather than skipped, per D1.6. # What excludes the lint-only Python profile here is the root tests/ guard rather than the manifest one, that profile being tooling scripts embedded in a non-Python repo rather than a tested package at the root. + # The matrix dimension is the interpreter, and a caller naming none runs the single leg this job ran before the input existed. + # The leg names an interpreter whether or not one ran, so a .NET-only caller reports a Python version for a run whose Python steps were all guarded off. + # A caller naming several also repeats this job's .NET half once per leg, which spends runner minutes and re-uploads an identical report rather than measuring anything new, and splitting the two halves into jobs of their own is what that costs, owed by the first repo to carry both a .NET test project and more than one interpreter. unit-test: - name: Unit test job + name: Unit test job (Python ${{ matrix.python-version }}) runs-on: ubuntu-latest permissions: contents: read + strategy: + # A failing interpreter must not cancel its siblings, since which of them fail is the whole point of running more than one. + fail-fast: false + matrix: + python-version: ${{ fromJSON(inputs.python-versions) }} steps: + # An entry holding no non-whitespace character is the value that would otherwise run green on an interpreter nobody chose, since setup-uv trims what it reads and then reads an empty input as an absent one. + # A caller reaches it by interpolating an unset value of its own, and a stray space in that template reaches it just as well, which is why the entry test is for content rather than for length. + # An unquoted entry is the other half of the same slip and announces itself either way, running the right interpreter where its decimal form survives fromJSON and failing where a trailing zero does not. + # That failure lands in the dependency sync below rather than in setup-uv, which exports the version rather than resolving it, so what the check buys here is a named cause rather than a silent pass prevented. + # The check 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, and it reaches only a value that expanded into a leg at all. + - name: Validate python-versions input step + env: + PYTHON_VERSIONS: ${{ inputs.python-versions }} + run: | + set -Eeuo pipefail + if ! jq -e 'type == "array" and length > 0 and all(.[]; type == "string" and test("\\S"))' <<<"$PYTHON_VERSIONS" >/dev/null 2>&1; then + echo "::error::The python-versions input must be a non-empty JSON array of non-empty quoted version strings, for example '[\"3.13\", \"3.14\"]', got: $PYTHON_VERSIONS" + exit 1 + fi + - name: Checkout code step uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -305,7 +334,7 @@ jobs: && (hashFiles('uv.lock') != '' || hashFiles('requirements*.txt') != '') uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1 with: - python-version: "3.13" + python-version: ${{ matrix.python-version }} # One resolve over the collected requirements files, since the glob sorts the base file last and a per-file install would let its pins downgrade what the test file just resolved. # The editable install matches what uv sync gives the other branch, and its guard leaves a layout declaring no [project] alone. @@ -353,6 +382,7 @@ jobs: fi # Best-effort: continue-on-error plus fail_ci_if_error false, so a missing token never reds the gate. + # The flag names the leg, so a multi-interpreter repo reads its legs apart in Codecov rather than seeing one merged number for all of them. - name: Upload coverage to Codecov step (Python) if: >- hashFiles('pyproject.toml') != '' && hashFiles('tests/**') != '' @@ -362,6 +392,7 @@ jobs: with: token: ${{ secrets.CODECOV_TOKEN }} files: ./coverage.xml + flags: python-${{ matrix.python-version }} fail_ci_if_error: false validate: diff --git a/docs/reusable-workflows.md b/docs/reusable-workflows.md index be83e14fd..1791d1158 100644 --- a/docs/reusable-workflows.md +++ b/docs/reusable-workflows.md @@ -156,7 +156,7 @@ Hub: `validate-task.yml` hosts a `lint` job (the fleet doc-lint block, language - [x] Hook override path observed on a hub pull request run, [proof run][override-path-run] (runs `./.github/actions/validate`, no hub checkout). Default path observed on PhotoCleaner's adoption pull request, [pilot smoke run][pilot-smoke-run], where the hub's `validate-default` ran because that repo carries no `validate` hook. The follow-up self-reference pilot also runs the bundled prose and repository gates through `$/.github/actions/` without checking out the hub. - [x] PhotoCleaner (pilot, release trigger shape with smoke, the same repo that piloted stage 1): ptr727/PhotoCleaner#55 on `develop` (`c80cb29`), promoted in ptr727/PhotoCleaner#56 (`fa91db0`), both on 2026-08-16. `test-pull-request.yml` calls the hub validate task and no repo hook was needed. - [ ] HomeAutomation-Config (second pilot, operational trigger shape) -- [ ] The remaining repos, one checkbox each added when the pilots close, since the sweep list is every cataloged repo. A Python adopter owes one precondition before its bump: the `unit-test` job fails when the run wrote no root `coverage.xml`, and that job's Python leg runs where the repository root carries `pyproject.toml`, `tests/`, and a dependency manifest it installs from, a committed `uv.lock` or a root `requirements*.txt`, so an adopter of that shape puts `pytest-cov` among its test dependencies and a `--cov=` selector in its own `pyproject.toml` before it bumps, per D1.6, which binds that selector for every Python repo with tests, lint-only excepted. aiopurpleair and Financial-Modeling carry both. homeassistant-purpleair owes the same precondition, its `requirements*.txt` and `tests/` reaching the leg though it carries no `uv.lock`, and its adoption is a design question rather than a bump, its pytest run being a matrix over several Home Assistant versions on Python 3.14 where the hub leg pins 3.13 and expresses no matrix at all. PlexCleaner's Python is a stdlib-only tooling subtree with no tests, and ESPHome-Config's Python is lint-only. The hub cannot smoke-test this itself, having no `tests/` of its own. +- [ ] The remaining repos, one checkbox each added when the pilots close, since the sweep list is every cataloged repo. A Python adopter owes one precondition before its bump: the `unit-test` job fails when the run wrote no root `coverage.xml`, and that job's Python leg runs where the repository root carries `pyproject.toml`, `tests/`, and a dependency manifest it installs from, a committed `uv.lock` or a root `requirements*.txt`, so an adopter of that shape puts `pytest-cov` among its test dependencies and a `--cov=` selector in its own `pyproject.toml` before it bumps, per D1.6, which binds that selector for every Python repo with tests, lint-only excepted. aiopurpleair and Financial-Modeling carry both. homeassistant-purpleair owes the same precondition, its `requirements*.txt` and `tests/` reaching the leg though it carries no `uv.lock`, and its adoption is a design question rather than a bump, its pytest run being a matrix over several Home Assistant versions on Python 3.14. The `python-versions` input settles the interpreter half, a caller naming `["3.14"]` moving the leg there, and leaves the dependency-version axis open, which is a dimension the hub task does not express. PlexCleaner's Python is a stdlib-only tooling subtree with no tests, and ESPHome-Config's Python is lint-only. The hub cannot smoke-test this itself, having no `tests/` of its own. - [ ] `reports/workflow-reuse.md` regenerated with `validate-task.yml` at 0 copies (a hub-only file no repo carries) and `test-pull-request.yml` showing callers equal to copies. ### Stage 3: The Pure Functions @@ -396,6 +396,26 @@ The repo gate's `sha-pin` and `eol-coverage` checks read the same tracked-file l `validate-task.yml` passes each line straight through to the `repo-gate` composite action's own `exclude-globs` input. That input turns each line into a `--exclude` argument for `repo_gate.py`. Unlike `markdown-exclude-globs`, a line here is never negated: it is always a pathspec to drop from `git ls-files`. So `themes/PaperMod/**` excludes, rather than `!themes/PaperMod/**`. +A repo whose package claims more than one interpreter names them all in `python-versions`, a JSON array the `unit-test` job reads through `fromJSON` as its matrix, one leg per entry: + +```yaml + validate: + name: Validate sources job + uses: ptr727/ProjectTemplate/.github/workflows/validate-task.yml@ # + permissions: + contents: read + secrets: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + with: + python-versions: '["3.13", "3.14"]' +``` + +The default is `["3.13"]`, so a caller naming nothing runs the single leg this job has always run. Each leg uploads under a `python-` Codecov flag of its own, which keeps the legs apart in the dashboard rather than merging them into one number, and `fail-fast: false` reports a failing interpreter beside a passing one rather than cancelling the run at the first. Two things move on adoption. The nested check reads `Unit test job (Python 3.13)` rather than `Unit test job`, which binds no ruleset, only the caller's own aggregator being 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 its series stops there and a `python-3.13` one starts beside it. + +Quote each entry, and give each one a version. The `unit-test` job's entry step refuses an array holding an entry with no non-whitespace character, which is the value that would otherwise run green on an interpreter nobody chose: `setup-uv` trims what it reads and then reads an empty input as an absent one, and a caller reaches that by interpolating an unset value of its own, or by leaving a stray space beside it. It refuses an unquoted entry too, though that one announces itself either way, since `fromJSON` admits a JSON array of numbers and 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 instead. That failure lands 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. An empty array, and anything that is not a JSON array at all, never reach the step: they fail while the matrix is expanded, loudly and with an unnamed cause. + +Two things are deliberately outside the input. 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, so naming a second version there buys runner minutes rather than a second measurement. And the `lint` job stays on the one interpreter the task pins, which costs a repo two different things. Its type check is performed at that interpreter, `mypy` and `pyright` each defaulting their target to the one they run under where the repo's own `[tool.mypy]` or `[tool.pyright]` section does not name a version, while `ruff` takes its target from `requires-python` where the `pyproject.toml` declares a `[project]` table and from its own default where it does not. That much a repo settles in its own configuration. What it cannot settle there is a lockfile: a repo carrying a `uv.lock` whose `requires-python` floor is above the pinned interpreter fails the `Sync Python dependencies step` outright, and no value of `python-versions` moves that job. + ## Adopting the Pure Functions Neither `get-version-task.yml` nor `publish-plan-task.yml` has a caller-stub snippet of its own, since a caller reaching either one is a job inside a repo's own `publish-release.yml` or a future `build-release-task.yml`, not a standalone top-level workflow. A repo whose publisher reads NBGV's version outputs directly, without carrying the whole release orchestrator, reaches `get-version-task.yml` by pin in place of its own copy: diff --git a/scripts/tests/test_release_guards.py b/scripts/tests/test_release_guards.py index 3e43b46fe..f89178733 100755 --- a/scripts/tests/test_release_guards.py +++ b/scripts/tests/test_release_guards.py @@ -3,7 +3,10 @@ from __future__ import annotations +import json +import os import re +import shutil import unittest from pathlib import Path from subprocess import run @@ -262,6 +265,104 @@ def test_validator_python_leg_reaches_a_pip_dependency_repo(self) -> None: self.assertIn("uv pip install -e .", job) self.assertIn(r"grep -Eq '^[[:space:]]*\[project\]' pyproject.toml", job) + def test_validator_pytest_leg_fans_out_over_every_named_interpreter(self) -> None: + """A pinned interpreter drops an adopter's other legs with nothing failing or warning. + + The required check goes green on the one version the task ran while the repository goes on + claiming the rest, so the fan-out is asserted here rather than left to be read by eye. + """ + workflow = (REPO / ".github/workflows/validate-task.yml").read_text(encoding="utf-8") + job = workflow.split("\n unit-test:\n", 1)[1].split("\n validate:\n", 1)[0] + + # The default's shape is asserted and its content is not. + # Neither bumping the fleet default nor adding a second interpreter to it is a regression here. + declaration = workflow.split(" python-versions:\n", 1)[1].split("\n secrets:", 1)[0] + default = re.search(r"(?m)^ default: ['\"](.*)['\"]$", declaration) + self.assertIsNotNone(default) + assert default is not None + versions = json.loads(default.group(1)) + self.assertTrue(versions) + self.assertTrue(all(isinstance(version, str) for version in versions)) + + # An explicit name: is used verbatim rather than falling back to a matrix-suffixed default. + # Without the interpolation every leg renders one indistinguishable check name. + self.assertIn(" name: Unit test job (Python ${{ matrix.python-version }})\n", job) + + # The matrix reads the input and the uv setup reads the matrix, so no literal survives between them. + self.assertIn(" python-version: ${{ fromJSON(inputs.python-versions) }}\n", job) + self.assertIn(" python-version: ${{ matrix.python-version }}\n", job) + self.assertNotIn('python-version: "', job) + + # One interpreter failing must not cancel the others, which is what a second leg is run to learn. + self.assertIn(" fail-fast: false\n", job) + + # Without a flag naming its leg, each upload merges into one number that hides which leg it came from. + self.assertIn(" flags: python-${{ matrix.python-version }}\n", job) + + @unittest.skipUnless(shutil.which("jq"), "jq is what the step under test runs") + def test_validator_refuses_a_python_versions_value_fromjson_would_admit(self) -> None: + """fromJSON admits a JSON array of numbers, which is not a list of interpreter versions. + + An empty entry is the value that would otherwise carry a run green on an interpreter nobody + chose, since setup-uv reads an empty input as an absent one. An unquoted entry announces + itself either way. The step's whole script is run here, not just its filter, since + inverting the condition or exiting zero would leave a filter-only assertion green while + the step admitted everything. + """ + workflow = (REPO / ".github/workflows/validate-task.yml").read_text(encoding="utf-8") + job = workflow.split("\n unit-test:\n", 1)[1].split("\n validate:\n", 1)[0] + + # The guard has to precede the steps it guards, so its position is asserted, not just its presence. + # Matched on the dash rather than on a name: key, or a step leading with uses: would slip in ahead unseen. + first_step = re.search(r"(?m)^ - (.*)$", job) + self.assertIsNotNone(first_step) + assert first_step is not None + self.assertEqual("name: Validate python-versions input step", first_step.group(1)) + + marker = " - name: Validate python-versions input step\n" + self.assertIn(marker, job) + body = job.split(marker, 1)[1] + opener = re.search(r"(?m)^ run: \|-?\n", body) + self.assertIsNotNone(opener, "the guard's script must be a literal block scalar") + assert opener is not None + script = body[opener.end() :] + lines: list[str] = [] + for line in script.splitlines(): + if line and not line.startswith(" " * 10): + break + lines.append(line[10:]) + script = "\n".join(lines) + self.assertIn("jq -e", script) + + cases = { + # Reachable: a non-empty JSON array expands into legs whatever its entries hold. + '["3.13"]': 0, + '["3.13", "3.14"]': 0, + # Reached by interpolating an unset value, and green on an unpinned interpreter if admitted. + '[""]': 1, + # The same path, since setup-uv trims before it decides an input is absent. + '[" "]': 1, + # The plausible slip, the input being a quoted string already, and well-formed JSON. + "[3.13, 3.14]": 1, + # Unreachable today: each of these fails while the matrix is expanded, before the step runs. + # They pin the rest of the filter's contract, which moving the check into a job of its own would ask for. + "[]": 1, + '"3.13"': 1, + "{}": 1, + "3.13": 1, + "": 1, + } + for value, expected in cases.items(): + with self.subTest(value=value): + verdict = run( + ["bash", "-c", script], + env={**os.environ, "PYTHON_VERSIONS": value}, + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(expected, verdict.returncode) + def test_audit_bash_blocks_are_not_labeled_as_posix_shell(self) -> None: audit_lines = (REPO / "AUDIT.md").read_text(encoding="utf-8").splitlines() bash_only = ("<(", "<<<", "$'", "[[") From 21b4675ea7e4da1eaafff69b6589e45d99b1d9f6 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Wed, 9 Sep 2026 12:59:19 -0700 Subject: [PATCH 2/5] Escape the Rejected Input Before the Annotation Carries It 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) --- .github/workflows/validate-task.yml | 8 +++++++- scripts/tests/test_release_guards.py | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/.github/workflows/validate-task.yml b/.github/workflows/validate-task.yml index 725ecd567..bc9aaad07 100644 --- a/.github/workflows/validate-task.yml +++ b/.github/workflows/validate-task.yml @@ -293,7 +293,13 @@ jobs: run: | set -Eeuo pipefail if ! jq -e 'type == "array" and length > 0 and all(.[]; type == "string" and test("\\S"))' <<<"$PYTHON_VERSIONS" >/dev/null 2>&1; then - echo "::error::The python-versions input must be a non-empty JSON array of non-empty quoted version strings, for example '[\"3.13\", \"3.14\"]', got: $PYTHON_VERSIONS" + # The annotation carries the rejected value, so it is escaped per the workflow command encoding rules first. + # A raw newline would end the annotation and let whatever follows it start a command of its own, and the runner reads a command off stderr as readily as stdout, so redirecting rather than escaping would close nothing. + # The percent substitution runs first, since running it after the others would re-encode the percent each of them just introduced. + escaped=${PYTHON_VERSIONS//'%'/%25} + escaped=${escaped//$'\r'/%0D} + escaped=${escaped//$'\n'/%0A} + echo "::error::The python-versions input must be a non-empty JSON array of non-empty quoted version strings, for example '[\"3.13\", \"3.14\"]', got: $escaped" exit 1 fi diff --git a/scripts/tests/test_release_guards.py b/scripts/tests/test_release_guards.py index f89178733..b93741b62 100755 --- a/scripts/tests/test_release_guards.py +++ b/scripts/tests/test_release_guards.py @@ -363,6 +363,32 @@ def test_validator_refuses_a_python_versions_value_fromjson_would_admit(self) -> ) self.assertEqual(expected, verdict.returncode) + # The annotation carries the rejected value back. + # The runner reads a workflow command off any line starting with one, so an unescaped newline starts a second. + injected = '["3.13"]\n::error::injected' + verdict = run( + ["bash", "-c", script], + env={**os.environ, "PYTHON_VERSIONS": injected}, + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(1, verdict.returncode) + emitted = (verdict.stdout + verdict.stderr).splitlines() + self.assertEqual(1, len(emitted)) + self.assertIn("%0A", emitted[0]) + + # A percent is escaped too, and first, or escaping the newline would re-encode its own percent. + verdict = run( + ["bash", "-c", script], + env={**os.environ, "PYTHON_VERSIONS": "%"}, + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(1, verdict.returncode) + self.assertIn("got: %25", verdict.stdout + verdict.stderr) + def test_audit_bash_blocks_are_not_labeled_as_posix_shell(self) -> None: audit_lines = (REPO / "AUDIT.md").read_text(encoding="utf-8").splitlines() bash_only = ("<(", "<<<", "$'", "[[") From ac705b93f4eb53502a645a2de533deefcc3eaba9 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Wed, 9 Sep 2026 13:09:18 -0700 Subject: [PATCH 3/5] Assert the Carriage-Return Escape and Correct What Proves the Order 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) --- scripts/tests/test_release_guards.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/scripts/tests/test_release_guards.py b/scripts/tests/test_release_guards.py index b93741b62..6dfc44915 100755 --- a/scripts/tests/test_release_guards.py +++ b/scripts/tests/test_release_guards.py @@ -378,7 +378,20 @@ def test_validator_refuses_a_python_versions_value_fromjson_would_admit(self) -> self.assertEqual(1, len(emitted)) self.assertIn("%0A", emitted[0]) - # A percent is escaped too, and first, or escaping the newline would re-encode its own percent. + # A carriage return is the third escape, and without this nothing here would notice its removal. + verdict = run( + ["bash", "-c", script], + env={**os.environ, "PYTHON_VERSIONS": "x\ry"}, + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(1, verdict.returncode) + self.assertIn("got: x%0Dy", verdict.stdout + verdict.stderr) + + # A bare percent must reach the annotation encoded. + # Their order is what the %0A assertion above covers instead, not this one. + # Substituting the percent last renders a newline %250A, which fails there and passes here. verdict = run( ["bash", "-c", script], env={**os.environ, "PYTHON_VERSIONS": "%"}, From 6e9c7b057f26edeedd3d2f5f4ed04054f2fb3e1d Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Wed, 9 Sep 2026 13:17:59 -0700 Subject: [PATCH 4/5] Correct What an Unquoted Integer Entry Costs 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) --- .github/workflows/validate-task.yml | 5 +++-- docs/reusable-workflows.md | 2 +- scripts/tests/test_release_guards.py | 2 ++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/validate-task.yml b/.github/workflows/validate-task.yml index bc9aaad07..d48378e75 100644 --- a/.github/workflows/validate-task.yml +++ b/.github/workflows/validate-task.yml @@ -284,8 +284,9 @@ jobs: # An entry holding no non-whitespace character is the value that would otherwise run green on an interpreter nobody chose, since setup-uv trims what it reads and then reads an empty input as an absent one. # A caller reaches it by interpolating an unset value of its own, and a stray space in that template reaches it just as well, which is why the entry test is for content rather than for length. - # An unquoted entry is the other half of the same slip and announces itself either way, running the right interpreter where its decimal form survives fromJSON and failing where a trailing zero does not. - # That failure lands in the dependency sync below rather than in setup-uv, which exports the version rather than resolving it, so what the check buys here is a named cause rather than a silent pass prevented. + # An unquoted entry is the other half of the same slip, and what it costs depends on the number fromJSON parsed it into. + # An integer resolves to whatever interpreter of that major version uv happens to find, so that one is a second silent pass this check prevents rather than a cause it names. + # A decimal runs the right interpreter where its form survives the round trip and fails where a trailing zero does not, in the dependency sync below rather than in setup-uv, which exports the version rather than resolving it. # The check 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, and it reaches only a value that expanded into a leg at all. - name: Validate python-versions input step env: diff --git a/docs/reusable-workflows.md b/docs/reusable-workflows.md index 1791d1158..2cc811f5b 100644 --- a/docs/reusable-workflows.md +++ b/docs/reusable-workflows.md @@ -412,7 +412,7 @@ A repo whose package claims more than one interpreter names them all in `python- The default is `["3.13"]`, so a caller naming nothing runs the single leg this job has always run. Each leg uploads under a `python-` Codecov flag of its own, which keeps the legs apart in the dashboard rather than merging them into one number, and `fail-fast: false` reports a failing interpreter beside a passing one rather than cancelling the run at the first. Two things move on adoption. The nested check reads `Unit test job (Python 3.13)` rather than `Unit test job`, which binds no ruleset, only the caller's own aggregator being 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 its series stops there and a `python-3.13` one starts beside it. -Quote each entry, and give each one a version. The `unit-test` job's entry step refuses an array holding an entry with no non-whitespace character, which is the value that would otherwise run green on an interpreter nobody chose: `setup-uv` trims what it reads and then reads an empty input as an absent one, and a caller reaches that by interpolating an unset value of its own, or by leaving a stray space beside it. It refuses an unquoted entry too, though that one announces itself either way, since `fromJSON` admits a JSON array of numbers and 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 instead. That failure lands 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. An empty array, and anything that is not a JSON array at all, never reach the step: they fail while the matrix is expanded, loudly and with an unnamed cause. +Quote each entry, and give each one a version. The `unit-test` job's entry step refuses an array holding an entry with no non-whitespace character, which is the value that would otherwise run green on an interpreter nobody chose: `setup-uv` trims what it reads and then reads an empty input as an absent one, and a caller reaches that by interpolating an unset value of its own, or by leaving a stray space beside it. It refuses an unquoted entry too, since `fromJSON` admits a JSON array of numbers, and what that costs depends on the number. An integer resolves to whatever interpreter of that major version uv happens to find, so `'[3]'` is a second silent pass the check prevents. A decimal runs the right interpreter where its form survives the round trip, and where a trailing zero does not survive it, `3.10` becoming `3.1`, fails in the `Sync dependencies step` rather than in `setup-uv`, which exports the version rather than resolving it. There the check names a cause rather than preventing a silent pass. An empty array, and anything that is not a JSON array at all, never reach the step: they fail while the matrix is expanded, loudly and with an unnamed cause. Two things are deliberately outside the input. 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, so naming a second version there buys runner minutes rather than a second measurement. And the `lint` job stays on the one interpreter the task pins, which costs a repo two different things. Its type check is performed at that interpreter, `mypy` and `pyright` each defaulting their target to the one they run under where the repo's own `[tool.mypy]` or `[tool.pyright]` section does not name a version, while `ruff` takes its target from `requires-python` where the `pyproject.toml` declares a `[project]` table and from its own default where it does not. That much a repo settles in its own configuration. What it cannot settle there is a lockfile: a repo carrying a `uv.lock` whose `requires-python` floor is above the pinned interpreter fails the `Sync Python dependencies step` outright, and no value of `python-versions` moves that job. diff --git a/scripts/tests/test_release_guards.py b/scripts/tests/test_release_guards.py index 6dfc44915..0c4a117e0 100755 --- a/scripts/tests/test_release_guards.py +++ b/scripts/tests/test_release_guards.py @@ -344,6 +344,8 @@ def test_validator_refuses_a_python_versions_value_fromjson_would_admit(self) -> '[" "]': 1, # The plausible slip, the input being a quoted string already, and well-formed JSON. "[3.13, 3.14]": 1, + # The integer form of it, which uv resolves to whatever interpreter of that major version it finds. + "[3]": 1, # Unreachable today: each of these fails while the matrix is expanded, before the step runs. # They pin the rest of the filter's contract, which moving the check into a job of its own would ask for. "[]": 1, From 3f487a655411b7496a566763348445c51dfa98eb Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Wed, 9 Sep 2026 13:24:00 -0700 Subject: [PATCH 5/5] Sweep the Fourth Surface and Drop the Mechanism Claim 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) --- .github/workflows/validate-task.yml | 4 ++-- docs/reusable-workflows.md | 2 +- scripts/tests/test_release_guards.py | 7 ++++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.github/workflows/validate-task.yml b/.github/workflows/validate-task.yml index d48378e75..0a1491ebc 100644 --- a/.github/workflows/validate-task.yml +++ b/.github/workflows/validate-task.yml @@ -285,8 +285,8 @@ jobs: # An entry holding no non-whitespace character is the value that would otherwise run green on an interpreter nobody chose, since setup-uv trims what it reads and then reads an empty input as an absent one. # A caller reaches it by interpolating an unset value of its own, and a stray space in that template reaches it just as well, which is why the entry test is for content rather than for length. # An unquoted entry is the other half of the same slip, and what it costs depends on the number fromJSON parsed it into. - # An integer resolves to whatever interpreter of that major version uv happens to find, so that one is a second silent pass this check prevents rather than a cause it names. - # A decimal runs the right interpreter where its form survives the round trip and fails where a trailing zero does not, in the dependency sync below rather than in setup-uv, which exports the version rather than resolving it. + # Some of those numbers uv resolves to a real interpreter rather than refusing, so that shape is a second silent pass this check prevents rather than a cause it names. + # The rest fail in the dependency sync below rather than in setup-uv, which exports the version rather than resolving it, and there the check names a cause instead. # The check 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, and it reaches only a value that expanded into a leg at all. - name: Validate python-versions input step env: diff --git a/docs/reusable-workflows.md b/docs/reusable-workflows.md index 2cc811f5b..5841e3d17 100644 --- a/docs/reusable-workflows.md +++ b/docs/reusable-workflows.md @@ -412,7 +412,7 @@ A repo whose package claims more than one interpreter names them all in `python- The default is `["3.13"]`, so a caller naming nothing runs the single leg this job has always run. Each leg uploads under a `python-` Codecov flag of its own, which keeps the legs apart in the dashboard rather than merging them into one number, and `fail-fast: false` reports a failing interpreter beside a passing one rather than cancelling the run at the first. Two things move on adoption. The nested check reads `Unit test job (Python 3.13)` rather than `Unit test job`, which binds no ruleset, only the caller's own aggregator being 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 its series stops there and a `python-3.13` one starts beside it. -Quote each entry, and give each one a version. The `unit-test` job's entry step refuses an array holding an entry with no non-whitespace character, which is the value that would otherwise run green on an interpreter nobody chose: `setup-uv` trims what it reads and then reads an empty input as an absent one, and a caller reaches that by interpolating an unset value of its own, or by leaving a stray space beside it. It refuses an unquoted entry too, since `fromJSON` admits a JSON array of numbers, and what that costs depends on the number. An integer resolves to whatever interpreter of that major version uv happens to find, so `'[3]'` is a second silent pass the check prevents. A decimal runs the right interpreter where its form survives the round trip, and where a trailing zero does not survive it, `3.10` becoming `3.1`, fails in the `Sync dependencies step` rather than in `setup-uv`, which exports the version rather than resolving it. There the check names a cause rather than preventing a silent pass. An empty array, and anything that is not a JSON array at all, never reach the step: they fail while the matrix is expanded, loudly and with an unnamed cause. +Quote each entry, and give each one a version. The `unit-test` job's entry step refuses an array holding an entry with no non-whitespace character, which is the value that would otherwise run green on an interpreter nobody chose: `setup-uv` trims what it reads and then reads an empty input as an absent one, and a caller reaches that by interpolating an unset value of its own, or by leaving a stray space beside it. It refuses an unquoted entry too, since `fromJSON` admits a JSON array of numbers, and what that costs depends on the number. Some of them uv resolves to a real interpreter rather than refusing, `'[3]'` among them, so an unquoted entry is a second silent pass the check prevents. The rest fail in the `Sync dependencies step` rather than in `setup-uv`, which exports the version rather than resolving it, and there the check names a cause instead. An empty array, and anything that is not a JSON array at all, never reach the step: they fail while the matrix is expanded, loudly and with an unnamed cause. Two things are deliberately outside the input. 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, so naming a second version there buys runner minutes rather than a second measurement. And the `lint` job stays on the one interpreter the task pins, which costs a repo two different things. Its type check is performed at that interpreter, `mypy` and `pyright` each defaulting their target to the one they run under where the repo's own `[tool.mypy]` or `[tool.pyright]` section does not name a version, while `ruff` takes its target from `requires-python` where the `pyproject.toml` declares a `[project]` table and from its own default where it does not. That much a repo settles in its own configuration. What it cannot settle there is a lockfile: a repo carrying a `uv.lock` whose `requires-python` floor is above the pinned interpreter fails the `Sync Python dependencies step` outright, and no value of `python-versions` moves that job. diff --git a/scripts/tests/test_release_guards.py b/scripts/tests/test_release_guards.py index 0c4a117e0..7f1a697d1 100755 --- a/scripts/tests/test_release_guards.py +++ b/scripts/tests/test_release_guards.py @@ -304,8 +304,9 @@ def test_validator_refuses_a_python_versions_value_fromjson_would_admit(self) -> """fromJSON admits a JSON array of numbers, which is not a list of interpreter versions. An empty entry is the value that would otherwise carry a run green on an interpreter nobody - chose, since setup-uv reads an empty input as an absent one. An unquoted entry announces - itself either way. The step's whole script is run here, not just its filter, since + chose, since setup-uv reads an empty input as an absent one. An unquoted entry is admitted + by fromJSON as a number, and uv resolves some of those rather than refusing them, so that + shape carries a run green too. The step's whole script is run here, not just its filter, since inverting the condition or exiting zero would leave a filter-only assertion green while the step admitted everything. """ @@ -344,7 +345,7 @@ def test_validator_refuses_a_python_versions_value_fromjson_would_admit(self) -> '[" "]': 1, # The plausible slip, the input being a quoted string already, and well-formed JSON. "[3.13, 3.14]": 1, - # The integer form of it, which uv resolves to whatever interpreter of that major version it finds. + # The integer form of it, which uv resolves rather than refuses. "[3]": 1, # Unreachable today: each of these fails while the matrix is expanded, before the step runs. # They pin the rest of the filter's contract, which moving the check into a job of its own would ask for.