fix(ci): stop one main commit's CI from cancelling another's - #281
Merged
Conversation
The concurrency group was scoped to the ref, so every push and dispatch on main shared `CI-refs/heads/main` and each new one cancelled the last. On a pull request that is correct -- a new push makes the previous run worthless, and cancelling saves runner minutes. On main it is not. Every commit there is a permanent, releasable point that `self-gate` stamps and `dogfood` publishes from, and a commit whose run was cancelled is unverified forever because no later run covers it. The workflow comment reasoned the risk away on the grounds that "main commits rarely arrive that fast". They do, by construction: `merge-on-green` must re-dispatch main's CI after every auto-merge, because a GITHUB_TOKEN push does not fire `on: push` and the merge commit would otherwise land with no CI at all. Three merges landing together therefore had each dispatch kill the previous commit's still-running matrix, and two commits went unverified. Qualifying the group by `github.sha` on main gives each commit its own group, so nothing contends. `cancel-in-progress: false` would NOT have been enough: GitHub keeps at most one running and one *pending* run per group and cancels the older pending one, so the middle commit of three would still have lost its run. Distinct groups are the only thing that works, which is also why the guard checks the group rather than the cancel flag. Behaviour is unchanged everywhere else. Pull-request pushes still share a group and still supersede their predecessors; a deliberate re-dispatch of the same main SHA still replaces the stale run. Audited the other six workflows: container and release are tag-triggered so their groups are already unique per release, bench already sets `cancel-in-progress: false`, and the two reusable workflows declare no concurrency. The guard proves itself rather than only passing: it rejects the exact ref-only group that caused this, accepts the shipped one, and asserts its pinned copy still equals what the workflow ships so the two cannot drift apart.
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.
Root-cause fix for the cancelled CI runs observed on
6382f11and0eedd37tonight.What happened
Three PRs merged within ~23 minutes (#278, then #279 and #280 auto-merging). Each left the previous commit's CI cancelled, so two commits on
mainare permanently unverified.The cause is not the merges.
merge-on-greenmust re-dispatch main's CI after every auto-merge, because aGITHUB_TOKENpush does not fireon: push— without the dispatch an auto-merged commit gets no CI at all. That dispatch is correct. The defect is that it joined concurrency groupCI-refs/heads/main, wherecancel-in-progress: truemade it kill the previous commit's still-running matrix.The old comment reasoned the risk away:
Both halves are wrong. They arrive that fast by construction, and on a protected branch the older commit is not disposable —
self-gatestamps it anddogfoodpublishes from it.The fix, and why the obvious one is insufficient
The ecosystem-standard fix is
cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}. It would not have fixed this. GitHub keeps at most one running and one pending run per group and cancels the older pending one — so with three rapid merges the middle commit still loses its run. That is exactly tonight's case, so that fix would have masked the defect rather than removed it.Qualifying the group by
github.shaon main is the complete fix — each commit gets its own group and never contends:CI-refs/heads/main-<shaA>/-<shaB>CI-refs/pull/N/merge-(both)CI-refs/tags/vX-cancel-in-progressstaystruebecause it remains correct within a group: on a PR it supersedes the stale push, and on main it only ever sees a deliberate re-dispatch of the same SHA.Scope audit
Checked all seven workflows. Only
ci.ymlhad the defect —container.yml/release.ymlare tag-triggered so their groups are already unique per release,bench.ymlalready setscancel-in-progress: false, and the two reusable workflows declare no concurrency.Guard
workflow_concurrency_test.rspins the property and proves itself rather than only passing: it rejects the exact ref-only group that caused this, accepts the shipped one, and asserts its pinned copy still equals what the workflow ships so the two cannot drift apart.Verification
cargo fmt --all --check— cleanThe full local gate could not be re-run: the earlier
ENOSPCleft the shared cargo target directory with dangling fingerprints (extern location for gix_trace does not exist, thenregex-automata). Clearing it is a shared-cache decision I have not taken unilaterally, so CI carries that verification.