From cebc074479e4ca15480dce866bbf73876d9b5428 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Wed, 2 Sep 2026 13:47:36 +0530 Subject: [PATCH 1/3] ci(release): stop requesting a retired runner, and run the build step in bash No ebuild release has ever published. All six tags -- v1.2.0 through v1.6.0 -- ran for exactly one day and failed, and Publish to PyPI was skipped every time because it needs a job that never finished. Two causes, both already fixed in ci.yml and never carried across. macos-13 is retired. A job requesting it is never assigned a runner and sits queued until GitHub kills it at 24h: cibuildwheel (macos-13) cancelled runner NEVER ASSIGNED Publish to PyPI (OIDC) skipped Create Release skipped Dropped rather than swapped for another image: macos-14 already succeeds, and CIBW_ARCHS_MACOS="x86_64 arm64" makes it produce the Intel wheels macos-13 was there for. The Build wheels step is bash -- a `||` fallback with trailing-backslash continuations -- and the matrix includes windows-latest, where the default shell is PowerShell. That job failed in every run for that reason alone: cibuildwheel (windows-latest) failure step "Build wheels" ci.yml carries the same `shell: bash` fix for the same matrix, with the same comment; release.yml never got it. Verified: no macos-13 remains in any workflow, and release.yml parses. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/release.yml | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 7121a089..0842c5dc 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -35,7 +35,16 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, ubuntu-22.04-arm, windows-latest, macos-13, macos-14] + # macos-13 is retired: jobs requesting it are never assigned a runner + # and sit queued until GitHub kills them at 24h. Every release run this + # repository has ever had ended that way -- six tags, six 1d failures, + # with Publish to PyPI skipped each time because it needs this job. + # ci.yml moved off it already; this workflow did not. + # + # Dropped rather than swapped: macos-14 builds both architectures via + # CIBW_ARCHS_MACOS below, so the x86_64 wheels macos-13 existed for are + # still produced. + os: [ubuntu-latest, ubuntu-22.04-arm, windows-latest, macos-14] steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 @@ -43,10 +52,16 @@ jobs: python-version: "3.12" - name: Install cibuildwheel run: pip install cibuildwheel==2.21.3 + # shell: bash -- the matrix includes windows-latest, where the default + # shell is PowerShell. The `||` fallback and the trailing-backslash + # continuations below are bash syntax; under PowerShell the step fails + # outright, which is the other half of why no release has published. - name: Build wheels + shell: bash env: CIBW_BUILD: "cp310-* cp311-* cp312-*" CIBW_SKIP: "*-musllinux_*" + CIBW_ARCHS_MACOS: "x86_64 arm64" run: | python -m cibuildwheel --output-dir wheelhouse || \ (echo "::warning::cibuildwheel failed (likely no C-extension); building pure-Python wheel" && \ From 51fe3a9f992c73fb82c2f23641533387c7c0126e Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Thu, 3 Sep 2026 16:00:03 +0530 Subject: [PATCH 2/3] ci(release): choose the wheel builder instead of catching a failure, and let the path be exercised Answers the review on #106. Finding 1 (Medium) -- `CIBW_ARCHS_MACOS: "x86_64 arm64"` was inert and the reason given for dropping macos-13 rested on wheels that have never existed. ebuild is pure Python: pyproject.toml declares setuptools.build_meta with no ext-modules, no cmdclass and no setup.py, so cibuildwheel cannot complete on any leg. All four fell through to `python -m build --wheel` and produced the same ebuild-X.Y.Z-py3-none-any.whl, which download-artifact --merge-multiple then collapsed back into one. macos-13's wheels were that same universal file. Dropped the env line and corrected the comment. Finding 2 (Medium) -- the `||` fallback was a fail-open build step, and this PR's `shell: bash` is what makes it reachable on Windows. It does not merely swallow a failure; it substitutes a different artefact and reports success, and `skip-existing: true` on the publish step means the substitution would not even collide with anything. Replaced with an explicit choice: detect whether the project builds a C extension, then run cibuildwheel or `python -m build` accordingly, with `set -euo pipefail` so a real failure fails. The step's name now means what it says. Verified both branches of that detection: against this repo it reports has_ext = False and takes the `python -m build` path; with a setup.py present it reports True. So the day a C extension lands the step switches by itself, and a genuine compile failure is a failure. Finding 4 (Low) -- added `workflow_dispatch`, with `pypi` and `release` gated on `github.event_name == 'push'`. The workflow's only trigger was a version tag, so the first execution of any change to it was a real release -- for a path whose last 15 runs all failed. A maintainer can now run validate + cibuildwheel + cross-compile on demand and watch the wheels land as artifacts without spending a tag, and a manual run cannot publish. Finding 3 (Low) is a PR-body correction and is handled there: the run history is worse than the body said -- the last 15 runs are failure or cancelled, back to v1.0.2 on 2026-04-28, and v1.0.2 failed in five minutes, which is neither the macOS queue timeout nor the shell syntax error. The two causes fixed here are the current ones, not the whole history, and the next tag is a test of these two fixes rather than proof the release path is healthy. Verified: yaml.safe_load of the result parses; jobs are ['validate', 'cibuildwheel', 'cross-compile', 'pypi', 'release'] triggers are ['push', 'workflow_dispatch'] pypi.if and release.if are both github.event_name == 'push' no `||` remains in the Build wheels step; `set -euo pipefail` is present pytest tests/ 1 failed, 557 passed, 3 skipped The one failure is PRE-EXISTING on origin/master and unrelated: the `python -m ninja` invocation from #66's open follow-up. test_ci_gate.py does not exist on this branch (it lives in #103), so it was NOT RUN here. NOT RUN: the workflow itself. It is tag- and dispatch-only, so this PR's CI cannot execute it -- which is exactly what finding 4 is about. The workflow_dispatch trigger only becomes usable once this is on master. Refs #106 --- .github/workflows/release.yml | 64 +++++++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 10 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0842c5dc..31fd45e7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -4,6 +4,13 @@ on: push: tags: - "v*.*.*" + # This workflow's only trigger was a version tag, so the first execution of + # any change to it was a real release -- an uncomfortable place to be for a + # path whose last 15 runs all failed. workflow_dispatch lets a maintainer + # run validate + cibuildwheel + cross-compile on demand and watch the wheels + # land as artifacts, without spending a tag. The publishing jobs stay gated + # on a tag push (see their `if:`), so a manual run can never publish. + workflow_dispatch: - "v*.*.*-*" permissions: @@ -42,8 +49,13 @@ jobs: # ci.yml moved off it already; this workflow did not. # # Dropped rather than swapped: macos-14 builds both architectures via - # CIBW_ARCHS_MACOS below, so the x86_64 wheels macos-13 existed for are - # still produced. + # macos-14. No per-architecture wheels are lost by that: ebuild is + # pure Python -- pyproject.toml declares setuptools.build_meta with no + # ext-modules, no cmdclass and no setup.py -- so cibuildwheel cannot + # complete here at all, every leg falls through to the `|| python -m + # build --wheel` below, and all four produce the same + # ebuild-X.Y.Z-py3-none-any.whl that download-artifact then merges + # back into one. macos-13's wheels were that same universal file. os: [ubuntu-latest, ubuntu-22.04-arm, windows-latest, macos-14] steps: - uses: actions/checkout@v4 @@ -53,20 +65,47 @@ jobs: - name: Install cibuildwheel run: pip install cibuildwheel==2.21.3 # shell: bash -- the matrix includes windows-latest, where the default - # shell is PowerShell. The `||` fallback and the trailing-backslash - # continuations below are bash syntax; under PowerShell the step fails - # outright, which is the other half of why no release has published. + # shell is PowerShell and the trailing-backslash continuations below are + # a syntax error, so the step failed outright there. That is the other + # half of why no release has ever published. + # + # The builder is chosen, not attempted-and-caught. This step used to be + # python -m cibuildwheel ... || (warn && python -m build --wheel) + # which is a fail-open build: it does not merely swallow the failure, it + # substitutes a *different artefact* and reports success. Today that + # fires on every leg by design, because ebuild is pure Python -- so the + # step's name has never meant what it says. The day a C extension lands, + # a genuine platform-specific compile failure on, say, Windows would + # have quietly published a pure-Python wheel in its place, and + # `skip-existing: true` on the publish step means it would not even + # collide with anything. - name: Build wheels shell: bash env: CIBW_BUILD: "cp310-* cp311-* cp312-*" CIBW_SKIP: "*-musllinux_*" - CIBW_ARCHS_MACOS: "x86_64 arm64" run: | - python -m cibuildwheel --output-dir wheelhouse || \ - (echo "::warning::cibuildwheel failed (likely no C-extension); building pure-Python wheel" && \ - pip install build && \ - python -m build --wheel --outdir wheelhouse) + set -euo pipefail + # Does this project actually build a C extension? cibuildwheel is + # the right tool only if it does. + if python - <<'EOF' + import sys, pathlib, tomllib + data = tomllib.loads(pathlib.Path("pyproject.toml").read_text()) + has_ext = bool( + data.get("tool", {}).get("setuptools", {}).get("ext-modules") + or data.get("tool", {}).get("setuptools", {}).get("cmdclass") + or pathlib.Path("setup.py").exists() + ) + sys.exit(0 if has_ext else 1) + EOF + then + echo "C extension detected: building platform wheels with cibuildwheel" + python -m cibuildwheel --output-dir wheelhouse + else + echo "Pure Python: building one universal wheel with python -m build" + pip install build + python -m build --wheel --outdir wheelhouse + fi - uses: actions/upload-artifact@v4 with: name: wheels-${{ matrix.os }} @@ -131,6 +170,10 @@ jobs: name: Publish to PyPI (OIDC) runs-on: ubuntu-latest needs: [validate, cibuildwheel] + # Publishing happens only for a real tag push. A workflow_dispatch run + # builds and uploads artifacts so the path can be exercised, and stops + # short of releasing anything. + if: github.event_name == 'push' permissions: id-token: write contents: read @@ -164,6 +207,7 @@ jobs: name: Create Release runs-on: ubuntu-latest needs: [validate, cibuildwheel, pypi] + if: github.event_name == 'push' steps: - uses: actions/checkout@v4 with: From 16137922a2d70e099644ccfeb6b59631e6e653a1 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Thu, 3 Sep 2026 17:41:25 +0530 Subject: [PATCH 3/3] ci(release): repair the trigger block I broke, and assert the shape rather than the parse Answers the second review on #106. Finding 1 is a defect I introduced in the previous commit. Finding 1 (High) -- `workflow_dispatch:` was inserted between the two tag patterns, so `- "v*.*.*-*"` became its value instead of a second entry under `push.tags`: {"push": {"tags": ["v*.*.*"]}, "workflow_dispatch": ["v*.*.*-*"]} Two consequences. Pre-release tags stopped triggering the workflow at all -- .github/STANDARDS.md names -rc.N as the org's recognised pre-release suffix and the release job handles it explicitly with `--prerelease`, so that code became unreachable. And `workflow_dispatch` takes null or a mapping; a sequence is not valid in GitHub's schema, so the workflow may not load at all -- the exact inverse of this PR's purpose. The verification in my last commit was `yaml.safe_load(...)` plus a print of the job list. That parses this file perfectly well. **Loading is not verifying**: the shape was wrong, not the syntax, and nothing I ran looked at the shape. So tests/unit/test_release_workflow.py asserts the structure: push.tags is exactly the two patterns; workflow_dispatch is null or a mapping; both publishing jobs are gated on `github.event_name == 'push'` so a manual run cannot release. Confirmed it catches what shipped. Reintroducing the exact breakage: safe_load parses fine test_push_tags_are_exactly_the_patterns_we_mean FAILS test_workflow_dispatch_is_a_null_key_not_a_value FAILS Finding 2 (Medium) -- `mv wheelhouse/*.whl dist/ 2>/dev/null || true` in the publish job. If no wheel had been produced -- every cibuildwheel leg red, the artifact upload skipped, the glob matching nothing -- dist/ held the sdist alone and PyPI was published with no wheels, silently, with `skip-existing: true` meaning it would not even collide. Now `set -euo pipefail`, a nullglob array, and an explicit failure when the array is empty. Two further tests pin that: staging cannot swallow its own failure, and no build or publish step in the workflow ends a line with `|| true`. Those tests read shell with comments stripped, because a comment explaining why a `|| true` was removed must not itself read as one. That is not hypothetical -- it failed on my own comment first. Finding 3 (Low) -- the macos-13 comment still described the `|| python -m build` fallback that the previous commit replaced with an explicit builder choice. Corrected. Finding 4 (Low) is CI timing, not a code change: four checks were pending when the review bundle was cut. Verified: pytest tests/unit/test_release_workflow.py 5 passed pytest tests/ 1 failed, 562 passed, 3 skipped The one failure is PRE-EXISTING on origin/master and unrelated: the `python -m ninja` invocation from #66's open follow-up. parsed triggers are now {"push": {"tags": ["v*.*.*", "v*.*.*-*"]}, "workflow_dispatch": None} NOT RUN: the workflow itself, still. It is tag- and dispatch-only. Refs #106 --- .github/workflows/release.yml | 30 ++++-- tests/unit/test_release_workflow.py | 149 ++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 7 deletions(-) create mode 100644 tests/unit/test_release_workflow.py diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 31fd45e7..4d758c4f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -4,6 +4,11 @@ on: push: tags: - "v*.*.*" + # -rc.N and friends. .github/STANDARDS.md names -rc.N as the org's only + # recognised pre-release suffix, and the release job below handles them + # explicitly (--prerelease). Without this pattern no pre-release tag + # starts the workflow at all. + - "v*.*.*-*" # This workflow's only trigger was a version tag, so the first execution of # any change to it was a real release -- an uncomfortable place to be for a # path whose last 15 runs all failed. workflow_dispatch lets a maintainer @@ -11,7 +16,6 @@ on: # land as artifacts, without spending a tag. The publishing jobs stay gated # on a tag push (see their `if:`), so a manual run can never publish. workflow_dispatch: - - "v*.*.*-*" permissions: contents: write @@ -51,11 +55,11 @@ jobs: # Dropped rather than swapped: macos-14 builds both architectures via # macos-14. No per-architecture wheels are lost by that: ebuild is # pure Python -- pyproject.toml declares setuptools.build_meta with no - # ext-modules, no cmdclass and no setup.py -- so cibuildwheel cannot - # complete here at all, every leg falls through to the `|| python -m - # build --wheel` below, and all four produce the same - # ebuild-X.Y.Z-py3-none-any.whl that download-artifact then merges - # back into one. macos-13's wheels were that same universal file. + # ext-modules, no cmdclass and no setup.py -- so the Build wheels step + # below selects `python -m build` on every leg, and all four produce + # the same ebuild-X.Y.Z-py3-none-any.whl that download-artifact then + # merges back into one. macos-13's wheels were that same universal + # file. os: [ubuntu-latest, ubuntu-22.04-arm, windows-latest, macos-14] steps: - uses: actions/checkout@v4 @@ -194,9 +198,21 @@ jobs: merge-multiple: true - name: Stage dist run: | + set -euo pipefail mkdir -p dist find dist -mindepth 1 -delete - mv wheelhouse/*.whl dist/ 2>/dev/null || true + # `2>/dev/null || true` here meant that if no wheel was downloaded -- + # every cibuildwheel leg red, the artifact upload skipped, the glob + # matching nothing -- dist/ ended up holding the sdist alone and + # PyPI was published with no wheels at all, silently. A release that + # ships less than it was asked to must fail, not publish. + shopt -s nullglob + wheels=(wheelhouse/*.whl) + if [ ${#wheels[@]} -eq 0 ]; then + echo "::error::no wheels were staged; refusing to publish a partial release" + exit 1 + fi + mv "${wheels[@]}" dist/ ls -lh dist/ - name: Publish to PyPI (trusted publishing) uses: pypa/gh-action-pypi-publish@release/v1 diff --git a/tests/unit/test_release_workflow.py b/tests/unit/test_release_workflow.py new file mode 100644 index 00000000..fbcacf8e --- /dev/null +++ b/tests/unit/test_release_workflow.py @@ -0,0 +1,149 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project +"""The release workflow's triggers and staging, asserted structurally. + +This file exists because `yaml.safe_load(...)` succeeding was mistaken for +verification. A `workflow_dispatch:` key inserted between two tag patterns +parses perfectly well -- and turns the second pattern into its value: + + on: + push: + tags: + - "v*.*.*" + workflow_dispatch: + - "v*.*.*-*" # now workflow_dispatch's value, not a tag + + {"push": {"tags": ["v*.*.*"]}, "workflow_dispatch": ["v*.*.*-*"]} + +Pre-release tags stop triggering the workflow, and `workflow_dispatch` as a +sequence is not valid in GitHub's schema at all. Neither shows up as a parse +error. The lesson generalises: assert the shape you meant, not that the file +is loadable. +""" + +import re +import shlex +from pathlib import Path + +import pytest +import yaml + +WORKFLOW = Path(__file__).resolve().parents[2] / ".github" / "workflows" / "release.yml" + +#: Tag patterns release.yml must fire on. -rc.N is the org's recognised +#: pre-release suffix (.github/STANDARDS.md) and the release job handles it +#: explicitly with --prerelease, so a tag pattern that does not match it makes +#: that code unreachable. +EXPECTED_TAGS = ["v*.*.*", "v*.*.*-*"] + +#: Jobs that publish. A workflow_dispatch run must never reach these. +PUBLISHING_JOBS = ("pypi", "release") + + +def _executable_lines(script): + """Shell lines with comments stripped. + + A comment explaining why a `|| true` was removed must not read as a + `|| true`. Splitting on an unquoted `#` is enough here and does not + mistake a `#` inside a string for the start of a comment. + """ + out = [] + for raw in str(script).splitlines(): + line = raw.strip() + if not line or line.startswith("#"): + continue + try: + lexer = shlex.shlex(raw, posix=True, punctuation_chars=True) + lexer.whitespace_split = True + list(lexer) # raises on an unbalanced quote + except ValueError: + pass + code = re.split(r'(?/dev/null || true` swallowed the case where no + wheel had been produced at all, and the publish step ran anyway. + """ + steps = workflow["jobs"]["pypi"]["steps"] + stage = [s for s in steps if s.get("name") == "Stage dist"] + assert stage, "the pypi job has no 'Stage dist' step" + code = "\n".join(_executable_lines(stage[0]["run"])) + + assert "|| true" not in code, ( + "staging must not swallow its own failure: publishing a release with " + "no wheels is worse than failing the job" + ) + assert "exit 1" in code, ( + "staging must fail explicitly when no wheel was produced" + ) + + +def test_no_build_or_publish_step_swallows_its_exit_status(workflow): + """`|| true` on a step that produces a release artefact. + + .ai/reviewer.md names this shape directly. Reporting steps may still use + it -- they claim no verdict -- so this checks the jobs that build or ship. + """ + offenders = [] + for job_id in ("validate", "cibuildwheel", "cross-compile", "pypi"): + for step in workflow["jobs"][job_id].get("steps", []): + for line in _executable_lines(step.get("run", "")): + if re.search(r"\|\|\s*true\s*$", line): + offenders.append( + f"{job_id}/{step.get('name', '?')}: {line.strip()}" + ) + assert not offenders, ( + "these steps discard their exit status:\n " + "\n ".join(offenders) + )