Repository navigation
fix(ci): cancel merge queue runs for destroyed merge groups (#38143) - #38144
Conversation
GitHub deletes a merge group's branch when the queue re-forms it but leaves the run going, so orphaned runs hold runners ahead of the live groups. Add a workflow-level concurrency group keyed on the squash message, which is unique per PR.
|
Claude finished @wezell's task in 55s —— View job Code ReviewI reviewed the diff against New Issues
Assessment of the mechanism (correct)The core logic is sound and matches the pattern already in
The one standing dependency (merge settings, not the message) is well documented in the PR body and the first-day watch items are the right things to confirm. No bugs, security, or convention issues in the diff. I could not run
|
|
Taken, and the watch item in the description now covers length as well as newlines. You are right that The failure analysis holds up and is worth stating plainly, because it is what makes this non-blocking. Identical truncation is harmless, since the key only has to be byte-identical per PR, not meaningful. Non-deterministic truncation would stop the old run being cancelled, which is exactly today's behaviour, so the change cannot be worse than not making it. Nothing here can cancel a live group, which is the one outcome that would actually hurt. I have added to the watch item that the first few entries should include a PR with a long multi-line body, and that its group name should be checked for stability across a re-formation. That is the case that would surface it. Thanks also for the speculative-batching check. Parallel speculative groups having distinct tip commits, so their keys never collide, is the part I had reasoned about but not verified, and it is the thing that would have made a per-PR key dangerous if it were not true. One correction to the premise this PR was opened on, since it affects how much to expect from it. I originally thought orphaned runs were starving the runner pool. They were not. Checking the job timings afterwards, the live merge-group runs had started within a minute of being created, long before the orphans were cancelled; |
|
Following up on the group-name question: I checked the repository settings rather than leaving it as an assumption, and it closes the multi-line concern.
The dependency is therefore the merge settings rather than the message, and the PR body now says so. If squash merge were enabled with For completeness on why this field and not another: |
wezell
left a comment
There was a problem hiding this comment.
Hope this is the right... lock key.
dotCMS-Machine-User
left a comment
There was a problem hiding this comment.
✅ dotbot review: all reviewer models (meta/muse-spark-1.3, ~z-ai/glm-latest) agree — patch is correct.
approved automatically by dotbot
|
dotbot code review:
Workflow-level concurrency keyed on the per-PR merge-group head_commit.message cancels only destroyed-group runs; per-PR uniqueness via embedded (#N) prevents live-group cancellation and failure modes fail safe to current behavior. Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads. reviewed by dotbot · meta/muse-spark-1.3 · medium |
|
dotbot code review:
The change adds a workflow-level concurrency group keyed on the merge-group head commit message, which this workflow already uses in run-name, so the expression resolves. Under this repository's merge settings (PR title with appended (#N)), the key is stable per PR across group re-formations and distinct between PRs, so cancel-in-progress can only cancel runs for destroyed groups; known failure modes fail safe to current behavior. No evidence-backed defects introduced by this patch were found. Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads. reviewed by dotbot · ~z-ai/glm-latest · medium |
Summary
Adds a workflow-level
concurrencygroup to.github/workflows/cicd_2-merge-queue.ymlso that a merge group's run is cancelled when the queue re-forms that PR's group.Closes #38143
Mechanism
When the queue re-forms a group (an entry ahead is ejected or added), GitHub creates a new group with a new head sha and deletes the old
gh-readonly-queue/main/pr-<N>-<sha>branch, but it does not cancel the old group's run. That run keeps its whole ~20-job matrix going and holds runners, so the runs for live groups sit inqueuedbehind it.On 2026-10-21 at 16:13Z, three live groups had three
queuedruns (39627643636, 39627642388, 39627639204) while three runs for deleted branches (39620380370, 39621293621, 39621297795) werein_progress, the oldest since 14:50. Merges to main that day were 2 against 26 the day before. One ejection (run 39621288435) was amirror.gcr.io"toomanyrequests: Quota exceeded" image-pull failure, which the extra concurrency plausibly causes.Workflow-level concurrency evaluates when a run is created, so the older run is cancelled as soon as the newer run for the same key exists, whole run including nested jobs, without waiting for a free runner.
Why the key is the squash message, and why it must be per PR
GitHub expressions have no regex or split, so the PR number cannot be parsed out of
head_ref, andhead_refis unique per group so it cannot be the key itself.head_commit.messageis the squash message (for examplefix(folders): require EDIT on the source folder when moving it (#38113)). Across six runs covering #38113 and #38114 it was byte-identical across re-formations of the same PR and different between PRs, and every message observed was single-line. The embedded(#N)means two PRs cannot collide. The workflow already uses this field inrun-name, so the expression resolves.The key must not be coarser than one PR. The queue deliberately builds several entries in parallel, and a key such as the workflow name or the base branch would cancel live groups and fail the queue (the RWX merge-queue docs warn about exactly this). A per-PR key is safe because a PR has one live group at a time, so a newer run for the same PR proves the older group was destroyed.
cicd_1-pr.ymlalready does the same thing keyed on the PR number.Why the message is a safe key, and what the key actually depends on
The merge settings on this repository are what make the group name single-line and per-PR unique,
so they are worth stating rather than assuming. From the repository API today:
allow_squash_merge: falseallow_merge_commit: true,merge_commit_title: PR_TITLE,merge_commit_message: BLANKSo the generated message is the PR title plus
(#N), and the body is never folded in. Every one ofthe eight real merge-group runs I sampled across four PRs was a single line of that shape, byte
identical across re-formations of the same PR and different between PRs. The appended
(#N)is whatmakes a collision between two PRs impossible, which matters because a collision is the one failure
mode that would cancel a live group rather than failing safe.
toJSON(...)is kept as belt and braces. It renders the message as one escaped line, so a multi-linemessage could not produce a multi-line group name even if the settings changed. It does not bound the
length, and I could find nothing documented about a limit on a concurrency group name; under current
settings the length is bounded by the PR title, so that question does not arise in practice.
The dependency to watch is therefore the merge settings, not the message. If someone enables
squash merge with
squash_merge_commit_message: PR_BODY, group names become long and multi-line withno code change and no review of this workflow. If a setting ever stopped the number being appended,
two PRs sharing a title would collide. Anyone changing the merge settings on this repository should
look at this block.
No other field in the
merge_grouppayload works as a key.head_refandhead_shachange on everyre-formation, so they would never match and never cancel anything;
base_refisrefs/heads/main,which is coarse enough to cancel live groups; the commit author would cancel across one person's
concurrent PRs. The message is the only field that is stable per PR and distinct between PRs.
First-day watch item: on the first few queue entries after merge, confirm the group name resolves,
that a re-formed group's old run shows as cancelled, and that no live group is cancelled.
What is and is not validated
Not validated by a run. A
merge_groupworkflow can only be exercised through the real queue, so this change is first proven by this PR's own merge queue entry, as #38125 was. I also did not test the concurrency semantics in isolation: aworkflow_dispatchworkflow is only dispatchable once it exists on the default branch, so a scratch workflow on this branch cannot be run, and I did not push a scratch push-triggered workflow to stand in for it because it would not exercisemerge_groupanyway.What was checked: the file parses as YAML and the
concurrencymapping is read as intended (RubyYAML.load_file).actionlintis not installed locally and I did not install it. The evidence for key stability comes fromgh api repos/dotCMS/core/actions/runs/<id>payloads, and the concurrency semantics come from GitHub's documentation.Known limits:
gh-readonly-queue/*branch no longer exists is the complementary fix for that case, deliberately left to a separate issue.This PR fixes: #38143