From b9f84fa815a2696a85ed3f430d52b6ccaf36ac82 Mon Sep 17 00:00:00 2001 From: chopratejas Date: Tue, 5 May 2026 13:03:07 -0700 Subject: [PATCH] =?UTF-8?q?feat(ci):=20X2=20=E2=80=94=20PR-time=20release?= =?UTF-8?q?=20dry-run=20via=20path-filtered=20pull=5Frequest=20trigger?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The X1 smoke-import gate (PR #387) catches runtime symbol mismatches on the wheel before publish, but only at release time. Recent break patterns were upstream of that: - #379 (docker bake `name=` regression in PR #376) - #382 (sdist `os: ubuntu-latest` → `ubuntu-24.04` rename) - #384 / #385 / #386 (glibc shim alias / link-order iterations) - #387's own heredoc-indent regression that broke main on the FIRST release run after merge — the heredoc was inside `bash -ec '...'`, no PR-time check exercised it X2 adds a `pull_request:` trigger to release.yml with a NARROW path filter so the dry-run runs at PR time for changes that affect wheel layout / release pipeline, but skips for source-only PRs to `crates/headroom-core` / `crates/headroom-proxy`. Path filter: release.yml, docker.yml, crates/headroom-py/**, pyproject.toml, root Cargo.toml, Cargo.lock. publish-pypi / publish-npm / publish-github-packages / publish-docker / create-release all gate on `github.event_name != 'pull_request'`, so a PR run never publishes — the dry-run is build + collect-dist + smoke-import only. concurrency rules: - PR runs: namespaced by PR number (`pr-N`), cancel-in-progress=true. - main runs: namespaced by ref_name, cancel-in-progress=false (a tag-push release that's mid-flight must not be cancelled). Test pin covers all four invariants (trigger, path filter, publish gates, concurrency split). 21 tests in test_release_workflows.py, all green locally. --- .github/workflows/release.yml | 37 +++++++++-- tests/test_release_workflows.py | 109 ++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 6 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ae7c56bcc..807412c33 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -37,6 +37,22 @@ on: - ".actrc.local.example" - ".env.act.example" - ".github/act/**" + # X2: PR-time release dry-run. Trigger on PRs that touch release- + # critical paths so the wheel matrix + smoke-import gate run BEFORE + # merge — not after the tag is pushed and main is broken. Path + # filter is deliberately narrow: source-only PRs to headroom-core + # / headroom-proxy don't change wheel layout, so they don't need + # the dry-run. Issues this would have caught: #379 (docker bake + # name=), #382 (sdist os-mismatch), #384/#385/#386 (glibc shim + # iterations), #387's own shellcheck regression. + pull_request: + paths: + - ".github/workflows/release.yml" + - ".github/workflows/docker.yml" + - "crates/headroom-py/**" + - "pyproject.toml" + - "Cargo.toml" + - "Cargo.lock" workflow_dispatch: inputs: version: @@ -48,8 +64,13 @@ on: default: false concurrency: - group: release-${{ github.ref_name }} - cancel-in-progress: false + # Main release runs are namespaced by ref_name and never cancel + # (cancel-in-progress: false) — losing a tag-push release mid-flight + # would mean PyPI/Docker get partial state. PR dry-runs are + # namespaced by PR number and DO cancel: rapid PR pushes shouldn't + # spawn N parallel wheel builds, only the latest matters. + group: release-${{ github.event_name == 'pull_request' && format('pr-{0}', github.event.pull_request.number) || github.ref_name }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: detect-version: @@ -530,7 +551,10 @@ jobs: publish-pypi: needs: [collect-dist, smoke-import-wheels] - if: github.event.inputs.dry_run != 'true' && vars.PYPI_SKIP != 'true' + # X2: skip on pull_request — dry-run only verifies build+smoke, + # never publishes. The other two predicates remain (workflow_dispatch + # dry_run, vars-level skip). + if: github.event_name != 'pull_request' && github.event.inputs.dry_run != 'true' && vars.PYPI_SKIP != 'true' environment: pypi # NOTE: environment name must be a literal; update here if the GitHub environment name changes runs-on: ubuntu-latest permissions: @@ -554,7 +578,7 @@ jobs: publish-npm: needs: [detect-version, build] - if: github.event.inputs.dry_run != 'true' && vars.NPM_SKIP != 'true' + if: github.event_name != 'pull_request' && github.event.inputs.dry_run != 'true' && vars.NPM_SKIP != 'true' runs-on: ubuntu-latest # Publishes to npmjs.org via NPM_TOKEN secret; no GITHUB_TOKEN # write permissions needed. @@ -611,7 +635,7 @@ jobs: publish-github-packages: needs: [detect-version, build] - if: github.event.inputs.dry_run != 'true' && vars.GH_PACKAGES_SKIP != 'true' + if: github.event_name != 'pull_request' && github.event.inputs.dry_run != 'true' && vars.GH_PACKAGES_SKIP != 'true' runs-on: ubuntu-latest permissions: contents: read @@ -721,7 +745,7 @@ jobs: # image's `pip install` step. Failing here ~3 minutes earlier # than docker-build saves the matrix's wall-clock budget. needs: [detect-version, smoke-import-wheels] - if: github.event.inputs.dry_run != 'true' + if: github.event_name != 'pull_request' && github.event.inputs.dry_run != 'true' permissions: contents: read packages: write @@ -736,6 +760,7 @@ jobs: if: >- ${{ always() && + github.event_name != 'pull_request' && github.event.inputs.dry_run != 'true' && needs.detect-version.result == 'success' && needs.build.result == 'success' && diff --git a/tests/test_release_workflows.py b/tests/test_release_workflows.py index 44523f51e..c3ec3c792 100644 --- a/tests/test_release_workflows.py +++ b/tests/test_release_workflows.py @@ -710,3 +710,112 @@ def test_npm_publish_jobs_do_not_download_dist_artifact() -> None: f"its own tarball and the speculative download fails when " f"collect-dist hasn't run." ) + + +def test_release_workflow_runs_dry_run_on_pull_request() -> None: + """X2: the release workflow MUST trigger on `pull_request` for paths + that change wheel-layout / release pipeline so the wheel matrix + + smoke-import gate run BEFORE merge. + + Issues this gate would have caught at PR time instead of after-merge: + - #379 (docker bake `name=` regression in PR #376) + - #382 (sdist os-mismatch — `ubuntu-latest` → `ubuntu-24.04` rename) + - #384 / #385 / #386 (glibc shim iterations — alias, link-order) + - #387's heredoc-indent regression that broke main on first release + run after merge + + Required: + 1. `pull_request:` trigger present. + 2. Path filter is narrow enough to skip source-only PRs to + `crates/headroom-core` / `crates/headroom-proxy` (where wheel + layout doesn't change), but wide enough to cover release.yml, + docker.yml, headroom-py crate, pyproject.toml, root Cargo. + 3. publish-pypi / publish-npm / publish-github-packages / + publish-docker / create-release ALL gate on + `github.event_name != 'pull_request'` so a PR run never + publishes anything — the dry-run is build+smoke only. + 4. concurrency.group is namespaced by PR number for PR runs and by + ref_name for main runs, AND cancel-in-progress is true for PR + runs (rapid PR pushes cancel stale dry-runs) and false for main + runs (a tag-push release should never be cancelled mid-flight). + + A future "lighten CI by dropping the dry-run" refactor — exactly + the impulse that gave us PR #382 and PR #387 — fails this test + at PR time. + """ + content = (ROOT / ".github" / "workflows" / "release.yml").read_text(encoding="utf-8") + + # 1. pull_request trigger present. + on_block_end = content.index("\nconcurrency:") + on_block = content[:on_block_end] + assert "\n pull_request:" in on_block, ( + "release.yml must trigger on `pull_request` so the wheel matrix + " + "smoke-import gate run BEFORE merge. Without this, wheel-layout / " + "release-pipeline regressions are only caught after the tag is " + "pushed and main is broken (see #382, #387 for canonical examples)." + ) + + # 2. Path filter covers wheel-layout-affecting paths. + pr_idx = content.index("\n pull_request:") + pr_end = content.index("\n workflow_dispatch:", pr_idx) + pr_block = content[pr_idx:pr_end] + required_paths = [ + ".github/workflows/release.yml", + ".github/workflows/docker.yml", + "crates/headroom-py/**", + "pyproject.toml", + "Cargo.toml", + "Cargo.lock", + ] + for path in required_paths: + assert f'"{path}"' in pr_block, ( + f"pull_request path filter missing {path!r}. The dry-run must " + f"trigger when this path changes — otherwise a regression in " + f"that file lands on main without exercising the wheel matrix." + ) + + # 3. Each publish job + create-release gates on event_name != pull_request. + publish_jobs = [ + ("publish-pypi", "\n publish-npm:"), + ("publish-npm", "\n publish-github-packages:"), + ("publish-github-packages", "\n publish-docker:"), + ("publish-docker", "\n create-release:"), + ] + for job_name, next_marker in publish_jobs: + start = content.index(f"\n {job_name}:") + end = content.index(next_marker, start) + body = content[start:end] + assert "github.event_name != 'pull_request'" in body, ( + f"{job_name} must gate on `github.event_name != 'pull_request'`. " + f"Without this gate, a PR dry-run would attempt to publish — " + f"in the best case the publish credentials are missing and the " + f"job fails noisily; in the worst case it succeeds and a " + f"non-merged PR ships to PyPI / npm / GHCR." + ) + + create_release_idx = content.index("\n create-release:") + create_release_block = content[create_release_idx:] + assert "github.event_name != 'pull_request'" in create_release_block, ( + "create-release must gate on `github.event_name != 'pull_request'`. " + "Without it, a PR dry-run would cut a GitHub Release for an unmerged " + "branch." + ) + + # 4. Concurrency: PR runs use a per-PR group and DO cancel-in-progress; + # main runs use ref_name and DO NOT cancel. + concurrency_idx = content.index("\nconcurrency:") + jobs_idx = content.index("\njobs:", concurrency_idx) + concurrency_block = content[concurrency_idx:jobs_idx] + + assert "github.event.pull_request.number" in concurrency_block, ( + "concurrency.group must include the PR number for pull_request runs " + "(via `format('pr-{0}', github.event.pull_request.number)`) — " + "otherwise PR runs collide with each other or with main." + ) + assert "cancel-in-progress: ${{ github.event_name == 'pull_request' }}" in concurrency_block, ( + "concurrency.cancel-in-progress must be conditional: TRUE for " + "pull_request (rapid PR pushes shouldn't queue N parallel wheel " + "builds) and FALSE for main (a tag-push release that's mid-flight " + "must not be cancelled — partial PyPI/Docker state is worse than " + "a slow CI queue)." + )