Merge pull request #1265 from ThrowTheSwitch/ci-sha-scoped-concurrency

Fix CI double-build race by keying concurrency on commit SHA

Author: mkarlesky

.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
@@ -48,79 +59,50 @@ 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'
+    # 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
 
 
   # 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
+    # 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
@@ -277,8 +259,8 @@ 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
+    if: success() && (github.event_name != 'pull_request' || github.event.pull_request.draft == false)
     runs-on: windows-latest
     strategy:
       fail-fast: false
@@ -361,8 +343,8 @@ 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
+    if: success() && (github.event_name != 'pull_request' || github.event.pull_request.draft == false)
     runs-on: macos-latest
     strategy:
       fail-fast: false
@@ -450,8 +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)"
-    needs: check-duplicate-push
-    if: success() && needs.check-duplicate-push.outputs.skip != 'true'
+    if: github.event_name != 'pull_request' || github.event.pull_request.draft == false
     runs-on: ubuntu-latest
     steps:
       - uses: actions/cache@v6
@@ -523,8 +504,7 @@ 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'
+    if: github.event_name != 'pull_request' || github.event.pull_request.draft == false
     runs-on: ubuntu-latest
     steps:
       - uses: actions/cache@v6
@@ -594,13 +574,12 @@ 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'
+    if: success() && (github.event_name != 'pull_request' || github.event.pull_request.draft == false)
     runs-on: ubuntu-latest
 
     steps: