From d0642e2b875d8727f42a2e9ad02fc6366411951d Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 15 Sep 2026 11:17:47 -0400 Subject: [PATCH 1/2] ci: stop dropping a commit's validation when the next one lands Merging two PRs a minute apart left the first commit with no unit tests, no compatibility run and no CodeQL analysis. Not a flake, and not the triggers -- `on: push: branches: [main]` is correct and the full twelve-job matrix does run on a quiet push. The loss came from the concurrency groups added in #392. Every workflow keyed its group on `github.ref`, so all of main's pushes shared one group. GitHub cancels a *pending* run whenever a newer one joins the group: `cancel-in-progress: false` protects a run that has already started, never one still queued. Measured on run 34970682137 for e0f0160a3 (the #393 merge), which finished `cancelled` with **zero jobs**, one second after the next push's run was created. That commit is on main having been validated by nothing at all, along with its CodeQL, Conda and Vendor runs. Non-pull-request events now key on `github.run_id`, giving each run its own group, so nothing coalesces and nothing is dropped. Pull requests still key on the ref and still cancel in progress, which is where the saving belongs and is untouched -- this does not undo #392's fan-out reduction, which came from removing the duplicate callers rather than from cancellation. Per workflow, and they differ on purpose: * unit-tests, python-compatibility, codeql -- per-run for pushes, so every commit on main is validated. unit-tests keeps its literal `unit-tests-` prefix because `github.run_id` inside a called workflow is the *caller's* run id, so without the prefix a callee would sit pending behind its own caller and deadlock. * create-release, cron-conda, cron-vendor -- per-run, never cancelled. These tag, upload and publish, so a dropped pending run is a release or a registry update that never happened. The crons additionally derive their value from *when* they ran rather than from which commit, so two scheduled runs at one SHA are both meaningful. * deploy-pages -- deliberately left keyed on the ref. It publishes a latest state rather than a verdict on a commit, so coalescing is correct, and publishing the older commit's docs after the newer ones would be wrong. Pages also allows only one live deployment. Validated: all seven files parse, zero duplicate keys at any nesting level, and the expression `${{ github.event_name == 'pull_request' && github.ref || github.run_id }}` resolves to the ref on a pull request and to the run id on every other event, per GitHub's value-returning `&&`/`||`. --- .github/workflows/codeql-analysis.yml | 5 ++++- .github/workflows/create-release.yml | 6 +++++- .github/workflows/cron-conda.yml | 5 ++++- .github/workflows/cron-vendor.yml | 6 +++++- .github/workflows/deploy-pages.yml | 6 ++++++ .github/workflows/python-compatibility.yml | 9 ++++++++- .github/workflows/unit-tests.yml | 22 +++++++++++++++------- 7 files changed, 47 insertions(+), 12 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 2c173a337c..92e9708c81 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -9,8 +9,11 @@ on: schedule: - cron: '0 2 * * 6' +# Only pull-request runs may be superseded -- see the note in unit-tests.yml. A +# ref-keyed group drops a *pending* run when a newer one joins it, which for a +# security scan means a commit reaching main with no analysis on record. concurrency: - group: codeql-${{ github.ref }} + group: codeql-${{ github.event_name == 'pull_request' && github.ref || github.run_id }} cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: diff --git a/.github/workflows/create-release.yml b/.github/workflows/create-release.yml index 7ade36e146..e6a6019263 100644 --- a/.github/workflows/create-release.yml +++ b/.github/workflows/create-release.yml @@ -20,8 +20,12 @@ name: Create Release # before either had tagged, which is a double-publish race. Serialise instead: # never cancel a release run, just make the second wait and find the tag # already there. +# One group per run, never cancelled. This path tags and uploads, so a run must +# neither be interrupted nor silently dropped -- and a ref-keyed group does drop +# a run that is still *pending* when a newer one joins it, which for a release +# means a version that never shipped. concurrency: - group: create-release-${{ github.ref }} + group: create-release-${{ github.run_id }} cancel-in-progress: false jobs: diff --git a/.github/workflows/cron-conda.yml b/.github/workflows/cron-conda.yml index b7baf93ec2..5ca9e8f722 100644 --- a/.github/workflows/cron-conda.yml +++ b/.github/workflows/cron-conda.yml @@ -11,8 +11,11 @@ on: # This workflow commits and pushes, so a cancelled run can leave the bump # half-applied -- queue superseded runs rather than killing them. +# One group per run, never cancelled. This publishes to Anaconda, so like the +# release path a run must neither be interrupted nor dropped while pending; and as +# a scheduled job its value is tied to when it ran, not to the commit it ran on. concurrency: - group: conda-update-${{ github.ref }} + group: conda-update-${{ github.run_id }} cancel-in-progress: false jobs: diff --git a/.github/workflows/cron-vendor.yml b/.github/workflows/cron-vendor.yml index 8cf79e015e..cc80c64207 100644 --- a/.github/workflows/cron-vendor.yml +++ b/.github/workflows/cron-vendor.yml @@ -8,8 +8,12 @@ on: # This workflow commits and pushes, so a cancelled run can leave the bump # half-applied -- queue superseded runs rather than killing them. +# One group per run, never cancelled. This crawls the IANA registries and commits +# what changed, so a run's value comes from *when* it ran rather than from which +# commit it ran on: two scheduled runs at the same SHA are both meaningful, and a +# ref-keyed group would drop the pending one and skip that cycle's update. concurrency: - group: vendor-update-${{ github.ref }} + group: vendor-update-${{ github.run_id }} cancel-in-progress: false jobs: diff --git a/.github/workflows/deploy-pages.yml b/.github/workflows/deploy-pages.yml index 0ed33d625b..5e4c601aa3 100644 --- a/.github/workflows/deploy-pages.yml +++ b/.github/workflows/deploy-pages.yml @@ -14,6 +14,12 @@ on: permissions: contents: write +# Deliberately still keyed on the ref, unlike the validation workflows. This one +# publishes a *latest state* rather than a verdict on a commit, so coalescing is +# correct: when two pushes land close together the pending build is dropped and +# the newer one publishes, and deploying the older commit's docs after the newer +# ones would actively be wrong. GitHub Pages also permits only one live +# deployment, so serialising here is a requirement rather than a saving. concurrency: group: pages-${{ github.ref }} cancel-in-progress: ${{ github.event_name == 'pull_request' }} diff --git a/.github/workflows/python-compatibility.yml b/.github/workflows/python-compatibility.yml index 893f9f0b6c..d1ee883a1c 100644 --- a/.github/workflows/python-compatibility.yml +++ b/.github/workflows/python-compatibility.yml @@ -11,8 +11,15 @@ on: permissions: contents: read +# Only pull-request runs may be superseded. Keying every event on the ref (as +# this did) coalesces all of main's pushes into one group, and GitHub cancels a +# *pending* run whenever a newer one joins the group -- `cancel-in-progress: +# false` protects a run that has already started, never one still queued. So a +# commit merged while its predecessor was still running lost its validation +# entirely. Keying non-PR events on the run id gives each one its own group, +# which is what "validate every commit" actually requires. concurrency: - group: python-compatibility-${{ github.ref }} + group: python-compatibility-${{ github.event_name == 'pull_request' && github.ref || github.run_id }} cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 8d3335f539..124278bd05 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -21,14 +21,22 @@ on: permissions: contents: read -# `github.workflow` is the *caller's* workflow name inside a called workflow, -# so the `unit-tests-` prefix is what stops a called run from ever sharing a -# group with the caller that invoked it -- if they shared one, the callee -# would cancel its own caller. Only pull-request runs are cancellable: on -# main, on a tag, or on the release path a cancelled run is worse than a slow -# one. +# Only pull-request runs may be superseded. Keying every event on the ref (as +# this did) coalesces all of main's pushes into one group, and GitHub cancels a +# *pending* run whenever a newer one joins the group -- `cancel-in-progress: +# false` protects a run that has already started, never one still queued. Merging +# two PRs a minute apart therefore left the first commit with no unit tests at +# all: measured on run 34970682137 for e0f0160a3, which finished `cancelled` with +# zero jobs one second after the next push's run was created. Keying non-PR +# events on the run id gives each commit its own group. +# +# `github.workflow` is the *caller's* workflow name inside a called workflow, and +# `github.run_id` is the caller's run id too, so the literal `unit-tests-` prefix +# is what stops a called run from sharing a group with the caller that invoked it +# -- if they shared one, the callee would sit pending behind its own caller and +# deadlock. concurrency: - group: unit-tests-${{ github.workflow }}-${{ github.ref }} + group: unit-tests-${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.run_id }} cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: From b6c584522e0be3dca2c1b02b0d298822e3cbe47f Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 15 Sep 2026 12:04:12 -0400 Subject: [PATCH 2/2] ci: keep the publishing paths serialised, per Copilot's review Copilot was right, and about create-release it was right in the way that matters. The first pass moved all six non-Pages workflows to per-run groups, which fixed dropped validation runs but silently removed serialisation from the paths that publish. Those are not the same trade: * a dropped *validation* run means a commit nobody ever checked, and there is no second chance at it; * a dropped *publishing* run means a publish that did not happen twice. So per-run is now only on unit-tests, python-compatibility and codeql, and create-release, cron-conda and cron-vendor go back to ref-keyed. create-release is the sharp case: unlike the two crons it has **no** job-level concurrency anywhere, so that group was the only thing stopping two runs tagging and uploading to PyPI and Anaconda at once. Per-run would have allowed exactly the race the file's own comment warned about. Also corrected the cron comments, which claimed the workflow-level group queued superseded runs. It does not, and it is not what protects the repository writes either -- the `vendor-update` and `conda-update` jobs each carry their own `repository-maintenance` group, shared between the two workflows, and that is what actually serialises the commits. The comments now say so, so the next reader does not credit the wrong mechanism. Validated: all seven parse, zero duplicate keys at any nesting level. --- .github/workflows/create-release.yml | 13 ++++++++----- .github/workflows/cron-conda.yml | 13 ++++++++----- .github/workflows/cron-vendor.yml | 15 +++++++++------ 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/.github/workflows/create-release.yml b/.github/workflows/create-release.yml index e6a6019263..a92cfe5b72 100644 --- a/.github/workflows/create-release.yml +++ b/.github/workflows/create-release.yml @@ -20,12 +20,15 @@ name: Create Release # before either had tagged, which is a double-publish race. Serialise instead: # never cancel a release run, just make the second wait and find the tag # already there. -# One group per run, never cancelled. This path tags and uploads, so a run must -# neither be interrupted nor silently dropped -- and a ref-keyed group does drop -# a run that is still *pending* when a newer one joins it, which for a release -# means a version that never shipped. +# Deliberately still keyed on the ref, unlike the validation workflows, and the +# distinction is the point: for them a dropped run means a commit nobody checked, +# which is unrecoverable, whereas here a dropped run means a publish that did not +# happen twice. This workflow tags, uploads to PyPI and uploads to Anaconda, and +# unlike cron-conda and cron-vendor it has *no* job-level concurrency to fall back +# on, so this group is the only thing serialising it. Keying it per run would let +# two runs tag and publish concurrently, which is the worse failure. concurrency: - group: create-release-${{ github.run_id }} + group: create-release-${{ github.ref }} cancel-in-progress: false jobs: diff --git a/.github/workflows/cron-conda.yml b/.github/workflows/cron-conda.yml index 5ca9e8f722..8a9a5dc7dc 100644 --- a/.github/workflows/cron-conda.yml +++ b/.github/workflows/cron-conda.yml @@ -10,12 +10,15 @@ on: branches: [main] # This workflow commits and pushes, so a cancelled run can leave the bump -# half-applied -- queue superseded runs rather than killing them. -# One group per run, never cancelled. This publishes to Anaconda, so like the -# release path a run must neither be interrupted nor dropped while pending; and as -# a scheduled job its value is tied to when it ran, not to the commit it ran on. +# half-applied. Kept keyed on the ref, unlike the validation workflows: this +# publishes to Anaconda, and letting two runs upload the same version +# concurrently is worse than dropping a superseded duplicate. +# +# Note this group is not what serialises the repository writes -- the +# `conda-update` job below carries its own `repository-maintenance` group, shared +# with cron-vendor, and that is what actually stops two jobs committing at once. concurrency: - group: conda-update-${{ github.run_id }} + group: conda-update-${{ github.ref }} cancel-in-progress: false jobs: diff --git a/.github/workflows/cron-vendor.yml b/.github/workflows/cron-vendor.yml index cc80c64207..509c3fb013 100644 --- a/.github/workflows/cron-vendor.yml +++ b/.github/workflows/cron-vendor.yml @@ -7,13 +7,16 @@ on: branches: [main] # This workflow commits and pushes, so a cancelled run can leave the bump -# half-applied -- queue superseded runs rather than killing them. -# One group per run, never cancelled. This crawls the IANA registries and commits -# what changed, so a run's value comes from *when* it ran rather than from which -# commit it ran on: two scheduled runs at the same SHA are both meaningful, and a -# ref-keyed group would drop the pending one and skip that cycle's update. +# half-applied. Kept keyed on the ref, unlike the validation workflows: a dropped +# run here just means a crawl that did not happen twice, and the next schedule +# picks the registries up again, whereas a dropped validation run means a commit +# nobody ever checked. +# +# Note this group is not what serialises the repository writes -- the +# `vendor-update` job below carries its own `repository-maintenance` group, shared +# with cron-conda, and that is what actually stops two jobs committing at once. concurrency: - group: vendor-update-${{ github.run_id }} + group: vendor-update-${{ github.ref }} cancel-in-progress: false jobs: