From 04060e919e0e77409407a713dccfc44a89b8f948 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Sat, 5 Sep 2026 09:49:17 -0400 Subject: [PATCH 1/2] Fix CI double-build race by keying concurrency on commit SHA check-duplicate-push (PR #1246) was a one-shot poll: on a push event it queried `gh pr list` once, near the start of the run, for an open PR on that branch. Push and PR-creation are independent, asynchronously-fired GitHub events with no ordering guarantee between them, so in the ordinary "push a branch, then separately open a PR from it" flow, the poll almost always ran before the PR existed, found nothing, and let the full run proceed -- only to have the later pull_request-triggered run duplicate it. The concurrency block couldn't help either: it was deliberately keyed by event_name specifically so a push run and a pull_request run for the same commit could never cancel each other (itself a fix for an earlier bug, commit bf878a97, where branch-only grouping let a new commit's push run wrongly cancel an older commit's still-running PR check). Keying the concurrency group on the commit SHA instead removes the race and the custom polling job entirely: a push run and a later pull_request run for the identical commit now land in the same group, so GitHub's own cancel-in-progress cancels whichever started first the moment the second one starts -- event-driven, not a fixed-point-in-time guess, and correct for the realistic case of a PR being opened while the push run's multi-job matrix is still in flight. Keying on the commit rather than the branch avoids reintroducing bf878a97's bug, since a different commit on the same branch gets its own group. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 82 +++++++++------------------------------- 1 file changed, 17 insertions(+), 65 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eb431b464..f0569dafd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,79 +48,39 @@ on: workflow_dispatch: -# Cancel any in-progress run for the same unit of work when a new event arrives. -# github.head_ref carries the bare source branch name for a pull_request event and -# is empty for a push, while github.ref_name carries that same bare branch name for -# a push (and an unrelated merge-ref name for a pull_request, which is why it's only -# ever reached as the fallback here) -- so `head_ref || ref_name` resolves to the -# identical branch name for both event types on the same branch. github.event_name -# is included specifically so push and pull_request runs for that same branch land -# in *separate* groups: a push to a branch that also has an open PR fires both -# triggers for the very same commit, and without the event name in the group key, -# whichever run happened to start second would cancel the other -- sometimes the -# pull_request-attached run, which is the one whose checks actually matter to a -# reviewer. Each event type still dedupes/cancels superseded runs of its own kind; -# see the check-duplicate-push job below for how the redundant push-triggered run -# is skipped instead of run to completion in parallel. +# Cancel any in-progress run for the same commit when a new event arrives. +# github.event.pull_request.head.sha is the PR branch's real head commit on a +# pull_request event (github.sha there is the synthetic PR-merge commit, not +# useful for this); github.sha is the pushed commit on a push event. For the +# identical commit, both resolve to the same group key -- so a push to a +# branch that also gets a PR opened from it (immediately or later, whenever +# a runner picks up the pull_request-triggered run) lands its push and +# pull_request runs in the *same* group, and GitHub's own cancel-in-progress +# cancels whichever started first the instant the second one starts. This is +# keyed on the commit rather than the branch specifically so a *different*, +# newer commit pushed to that same branch never collides with -- and so +# never wrongly cancels -- an older commit's still-running pull_request +# check: each commit gets its own group. concurrency: - group: ci-${{ github.event_name }}-${{ github.head_ref || github.ref_name }} + group: ci-${{ github.event.pull_request.head.sha || github.sha }} cancel-in-progress: true permissions: contents: read - pull-requests: read jobs: - # Job: Skip this run's real work if it's a redundant push-triggered duplicate - # A push to a branch with an open PR fires both a `push` event and a - # `pull_request: synchronize` event for the very same commit -- the separate - # concurrency groups above (keyed on event name) mean neither run can cancel - # the other, so without this check both would run the full matrix in - # parallel for every such commit. This job only performs the check for - # `push` events; for `pull_request`/`workflow_dispatch` events `skip` stays - # empty (falsy), so every downstream job's `if:` proceeds normally. - check-duplicate-push: - name: "Check for Duplicate PR-Triggered Run" - runs-on: ubuntu-latest - outputs: - skip: ${{ steps.check.outputs.skip }} - steps: - - name: Check whether this push's branch already has an open PR - id: check - if: github.event_name == 'push' - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - numbers=$(gh pr list --repo "${{ github.repository }}" --head "${{ github.ref_name }}" --state open --json number --jq '.[].number') - if [ -n "$numbers" ]; then - echo "Branch '${{ github.ref_name }}' has an open PR (#$(echo "$numbers" | paste -sd, -)) -- skipping this redundant push-triggered run." - echo "skip=true" >> "$GITHUB_OUTPUT" - else - echo "No open PR found for branch '${{ github.ref_name }}' -- proceeding normally." - echo "skip=false" >> "$GITHUB_OUTPUT" - fi - - # Job: Build MkDocs HTML documentation bundle for gem inclusion generate-docs: name: "Generate Local Docs Bundle for Gem Inclusion" - needs: check-duplicate-push - # A custom `if:` replaces GitHub's default (implicit) `success()` check on - # `needs:` entirely rather than being ANDed with it -- `success()` is - # included explicitly on every job below that gained this skip guard, to - # preserve each job's original "only run if its other needs succeeded" - # behavior alongside the new check. - if: success() && needs.check-duplicate-push.outputs.skip != 'true' uses: ./.github/workflows/_generate-docs.yml # Job: Linux test suite tests-linux: name: "Linux Test Suite" - needs: [check-duplicate-push, generate-docs] - if: success() && needs.check-duplicate-push.outputs.skip != 'true' + needs: generate-docs runs-on: ubuntu-latest strategy: fail-fast: false @@ -277,8 +237,7 @@ jobs: # Job: Windows test suite tests-windows: name: "Windows Test Suite" - needs: [check-duplicate-push, generate-docs] - if: success() && needs.check-duplicate-push.outputs.skip != 'true' + needs: generate-docs runs-on: windows-latest strategy: fail-fast: false @@ -361,8 +320,7 @@ jobs: # Note for the future: Ruby 3.1 will itself be dropped starting with macOS 26. tests-macos: name: "macOS Test Suite" - needs: [check-duplicate-push, generate-docs] - if: success() && needs.check-duplicate-push.outputs.skip != 'true' + needs: generate-docs runs-on: macos-latest strategy: fail-fast: false @@ -450,8 +408,6 @@ jobs: # a path-based Bundler deployment (deploy_gem) that does not require a gem build. tests-linux-locale: name: "Linux Test Suite (ja_JP.UTF-8 Locale)" - needs: check-duplicate-push - if: success() && needs.check-duplicate-push.outputs.skip != 'true' runs-on: ubuntu-latest steps: - uses: actions/cache@v6 @@ -523,8 +479,6 @@ jobs: # deployment (deploy_gem) that does not require a gem build. tests-linux-encoding-stress: name: "Linux Encoding Stress Test (C/POSIX)" - needs: check-duplicate-push - if: success() && needs.check-duplicate-push.outputs.skip != 'true' runs-on: ubuntu-latest steps: - uses: actions/cache@v6 @@ -594,13 +548,11 @@ jobs: build-gem: name: "Validate Ceedling Gem Build" needs: - - check-duplicate-push - tests-linux - tests-windows - tests-macos - tests-linux-locale - tests-linux-encoding-stress - if: success() && needs.check-duplicate-push.outputs.skip != 'true' runs-on: ubuntu-latest steps: From 546322ef874b5317b19d18bd7e9f96ee83597870 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Sun, 6 Sep 2026 16:14:48 -0400 Subject: [PATCH 2/2] Scope push CI to integration branches; skip the matrix for draft PRs A feature/topic branch's own push event used to also trigger this workflow, so a push immediately followed by opening a PR from it fired two runs for the identical commit. Push and pull_request are independent, asynchronously-fired events with no ordering guarantee between them, so no point-in-time check on either side -- including the SHA-scoped concurrency cancellation just added -- can prevent the redundant run from being created in the first place, only clean it up after the fact. push: is now scoped to master and next_version only. A feature branch gets CI exactly once, via the PR opened from it, removing the duplicate trigger at its source rather than deduping it afterward. The concurrency fix stays in place as a backstop for the narrower cases this doesn't cover (e.g. a PR opened from master/next_version itself). Also skips the full matrix for a still-draft PR until it's marked ready, via a guard on every downstream job -- pull_request fires for draft PRs the same as ready ones, and this team doesn't have a reason to spend the whole matrix on a PR its author hasn't asked for review on yet. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 41 +++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f0569dafd..370a2dd31 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,10 +14,12 @@ # and release.yml, which trigger only on intentional Git tags. # # Triggers: -# - Push to any branch +# - Push to a long-lived integration branch (master, next_version) only — +# a feature/topic branch gets checks exclusively through the pull_request +# event once a PR exists (see the push: trigger below for why) # - Pull request targeting any branch (a PR can be opened against any # integration branch, not just master, as the project matures multiple -# branches at once) +# branches at once); skipped for a still-draft PR until marked ready # - Manual dispatch (workflow_dispatch) # # Skip-CI: @@ -33,13 +35,22 @@ --- name: CI -# Triggers the workflow on push to any branch (tag pushes excluded — handled by -# prerelease.yml and release.yml), pull requests targeting any branch, or manual dispatch +# Triggers the workflow on push to a long-lived integration branch (tag pushes +# excluded — handled by prerelease.yml and release.yml), pull requests targeting +# any branch, or manual dispatch on: push: - branches: - - '**' # All branches; branches: ['**'] scopes push to refs/heads/ only, - # excluding refs/tags/ pushes — see Skip-CI above + # Deliberately narrow, not '**': a feature/topic branch's own push event used + # to also trigger this workflow, so a push immediately followed by opening a + # PR from it fired two runs for the identical commit -- push and pull_request + # are independent, asynchronously-fired events with no ordering guarantee + # between them, so no point-in-time check on either side can reliably catch + # this before the fact (see the concurrency: block below for the backstop + # that still exists for whatever this doesn't prevent). Scoping push: to only + # the branches that are never themselves the source of a PR removes the + # redundant trigger at its source instead: a feature branch now gets CI + # exactly once, via the PR opened from it. + branches: [master, next_version] pull_request: # No branch restriction: a PR merging into any branch gets checks. The one # exclusion is gh-pages, which only ever receives generated site output and @@ -74,6 +85,12 @@ jobs: # Job: Build MkDocs HTML documentation bundle for gem inclusion generate-docs: name: "Generate Local Docs Bundle for Gem Inclusion" + # pull_request fires for a draft PR the same as a ready one; github.event.pull_request + # is only ever populated on that event type, so this has no effect on push or + # workflow_dispatch runs. Holds off the full matrix's cost until a PR is actually + # ready for review -- a draft's own incremental commits get this workflow's checks + # for free the moment it's marked ready, no separate action needed. + if: github.event_name != 'pull_request' || github.event.pull_request.draft == false uses: ./.github/workflows/_generate-docs.yml @@ -81,6 +98,11 @@ jobs: tests-linux: name: "Linux Test Suite" needs: generate-docs + # success() is spelled out explicitly here (not left implicit) because a custom + # if: replaces GitHub's default success()-on-needs: check entirely rather than + # being ANDed with it -- every job below with a needs: dependency carries this + # same explicit success() alongside its own draft-PR guard for that reason. + if: success() && (github.event_name != 'pull_request' || github.event.pull_request.draft == false) runs-on: ubuntu-latest strategy: fail-fast: false @@ -238,6 +260,7 @@ jobs: tests-windows: name: "Windows Test Suite" needs: generate-docs + if: success() && (github.event_name != 'pull_request' || github.event.pull_request.draft == false) runs-on: windows-latest strategy: fail-fast: false @@ -321,6 +344,7 @@ jobs: tests-macos: name: "macOS Test Suite" needs: generate-docs + if: success() && (github.event_name != 'pull_request' || github.event.pull_request.draft == false) runs-on: macos-latest strategy: fail-fast: false @@ -408,6 +432,7 @@ jobs: # a path-based Bundler deployment (deploy_gem) that does not require a gem build. tests-linux-locale: name: "Linux Test Suite (ja_JP.UTF-8 Locale)" + if: github.event_name != 'pull_request' || github.event.pull_request.draft == false runs-on: ubuntu-latest steps: - uses: actions/cache@v6 @@ -479,6 +504,7 @@ jobs: # deployment (deploy_gem) that does not require a gem build. tests-linux-encoding-stress: name: "Linux Encoding Stress Test (C/POSIX)" + if: github.event_name != 'pull_request' || github.event.pull_request.draft == false runs-on: ubuntu-latest steps: - uses: actions/cache@v6 @@ -553,6 +579,7 @@ jobs: - tests-macos - tests-linux-locale - tests-linux-encoding-stress + if: success() && (github.event_name != 'pull_request' || github.event.pull_request.draft == false) runs-on: ubuntu-latest steps: