Fix CI double-build race by keying concurrency on commit SHA - #1265
Merged
Conversation
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 bf878a9, 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 bf878a9's bug, since a different commit on the same branch gets its own group. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Investigated why the existing push-vs-PR CI dedup mechanism
(
check-duplicate-push, PR #1246) works inconsistently — "most instancesproduce double builds, some skip the branch-push build."
Root cause:
check-duplicate-pushis a one-shot, point-in-time poll. Ona
pushevent it queriesgh pr list --head <branch> --state openonce,near the start of the run, and decides then and there whether to skip.
Push and PR-creation are independent, asynchronously-fired GitHub events
with no ordering guarantee between them. In the ordinary "push a branch,
then separately open a PR from it" flow — the dominant real-world case —
the poll almost always runs before the PR exists, finds nothing, and lets
the full run proceed to completion. The later
pull_request-triggered runthen duplicates it, with no way to detect or cancel the redundant push run
(the
concurrency:block was deliberately keyed byevent_namespecifically so push and PR runs for the same commit couldn't cancel each
other — itself a fix for an earlier bug, commit
bf878a97, wherebranch-only concurrency grouping let a new commit's push run wrongly cancel
an older commit's still-running PR check). Outright skips of the push
run only happen in the narrower case where the PR already existed before
the push (e.g. a follow-up commit to an already-open PR).
Fix
Key the concurrency group on the commit SHA instead of branch+event, and
remove the custom polling job entirely:
github.event.pull_request.head.shais the PR branch's real head commit ona
pull_requestevent;github.shais the pushed commit on apushevent. For the identical commit, both resolve to the same group key — so a
push run and a later
pull_requestrun for that commit now land in thesame concurrency group, and GitHub's own
cancel-in-progresscancelswhichever started first the instant the second one starts. This is
event-driven, not a fixed-point-in-time guess, so it correctly handles a PR
being opened while the push run's multi-job matrix (several minutes) is
still in flight — not just the first few seconds after push. Keying on the
commit rather than the branch avoids reintroducing
bf878a97's bug, sincea different commit on the same branch gets its own group.
Residual gap: if the push run fully completes before the PR is ever
opened, there's nothing left to cancel and it still runs twice for that
commit — a much narrower window than today's near-universal race.
Alternatives considered (not adopted)
doesn't eliminate the race, and adds latency to every push run.
push:trigger to protected branches only, relying onpull_requestalone for feature branches: would eliminate the problemfor the common case, but is a workflow philosophy change (no CI feedback
on a bare push to a new feature branch until a PR exists), not a plumbing
fix — kept as a separate, independent option if wanted later.
in that direction (a PR can only reference an already-pushed commit), and
would close the residual gap above, but adds custom-script complexity for
a narrow edge case — flagged for later if the residual gap proves to
matter in practice.
Verification
This PR is itself a live test of the fix (push already happened; opening
this PR now exercises the exact "push, then promptly open a PR" scenario
the old mechanism handled worst).
actionlintconfirms no new issuesintroduced. Full verification plan (see plan file) also covers a follow-up
commit to this open PR and a fresh-branch/immediate-PR test.
🤖 Generated with Claude Code