Repository navigation
pr-pipeline: read merge-queue holds once and dequeue a PR when a hold label is added #12449
Description
Refs: #12284
Owner: poteto (pr-merge and merge-queue path).
Summary
The merge queue re-runs full CI for each group: 1,885 groups and 200 run-hours over 7 days, with 134 failures (7.1%). In a 2026-11-01 sample, holds caused 10 of the 12 failed groups: a label added after enqueue, a lanes App body hold, or a pr-unblock status. Code caused only 2. Each failure ejects the PR and rebuilds the group for its peers. Source: audit comment, gate #10. Example runs:
At cb8429f, .github/workflows/pr-require-checks.yml:958 (select-tests, "Fail a queued pull request a hold stops, before the lanes") already reads holds before the lanes run. ci-status repeats them at :5779, :6186 and :6247.
Proposal:
- Read the queue-time hold once, in select-tests.
- Dequeue the PR when a hold label is added, so the group does not run on it at all.
Done when
- The queue-time hold is read in one step, and the duplicate ci-status merge-group hold reads are removed. No hold is dropped.
- A hold label added to a queued PR removes it from the queue before its group runs.
- Failed merge groups per day caused by holds are measured before and after.
Activity
github-actions commentedon Nov 2, 2026
Proposed work class: work-class: scoped
Basis: automated intake triage; the lanes App applied this class.
Size: Medium
Complexity: Medium
Uncertainty: Knowable
Flags: none
Blocked by: none
I understood this as two changes to the merge-queue path in pr-require-checks.yml. First, read the queue-time holds once, in select-tests, and remove the repeated merge-group hold reads in ci-status, without dropping any hold. Second, dequeue a PR when a hold label is added, so its group doesn't run. The done-when also asks for before-and-after measurement of hold-caused group failures. That needs data collected over time, so it is probably a separate step from the workflow change. I found no clear duplicate among the open issues. #12284 is the audit this refs, and #12448's neighbour on do-not-merge holds looks related but different.
Automated triage, applied by the lanes App.
kyle-sexton commentedon Nov 2, 2026
BEFORE baseline (Done-when 3). Measured 2026-11-02T18:25Z. It covers every merge-group failure in the window, not a sample, and takes each cause from the failing step's annotation.
Corrections to the audit:
verify:unreadableandunexpected end of JSON input. Those belong to fix(source-control): retry a truncated artifact listing page in the verdict reader #12418 and the FAIL index, not to this issue.Still to check before building: whether the 16
pr-verify-failedejections had the label added after queue entry, or already had it at entry. If the label was already there, the cause is the stale label staying on until a pass verdict, and dequeue-on-label-add would fix none of them.github-actions commentedon Nov 2, 2026
kyle-sexton commentedon Nov 2, 2026
Status: parked for a builder (poteto session). Written by the issue-triage session.
The criteria change
.github/workflows/pr-require-checks.yml: the select-tests queue-time hold read and the duplicate ci-status merge-group hold reads. The issue lane cannot push workflow paths, so an issue-lane run would dead-end. The issue moves tostatus: confirmedwithoutadmission-deferred, and the poteto session builds it as part of its s5 work.github-actions commentedon Nov 2, 2026
melodic-automation-lanes commentedon Nov 2, 2026
The work-resolve-issue lane stopped on this issue (
patch-protected-path): the change needs a path the issue lane may not push, which a maintainer session builds by hand.It added
host-runand removedagent-ready: an agent run on a host builds it, and it waits on no decision from a person.In the run's words:
What the run says it did not do:
Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/41504475492
To hand it back to the lane, remove
host-runand addagent-readyagain.kyle-sexton commentedon Nov 2, 2026
Status: narrowed and ordered after #12446. Written by the issue-triage session.
#12446 removes the ci-status hold steps at
.github/workflows/pr-require-checks.yml:6186(the pr-unblock stop status) and:6247(the lanes App body or veto hold), which this issue's first criterion also names. To keep two builders out of the same steps, this issue is now blocked by #12446. After #12446 merges, criterion 1 covers only what is left: the queue-time hold read is one step (select-tests,:958), and the duplicate ci-status label read at:5779is removed. Criteria 2 and 3 are unchanged. The builder is still the poteto session (s5).kyle-sexton commentedon Nov 2, 2026
Design check: is a hold label already on at queue entry? (poteto)
The 22 hold-label merge-group failures in the 7-day baseline split in two:
pr-verify-failedand 1 wasdo-not-merge. Every one of these entries is dated 2026-10-29T00:30Z to 2026-10-31T23:49Z, and none is later.pr-verify-failedand 5 wereneeds-human.Both arm paths already refuse a PR that carries a hold label:
HOLD_LABELS(do-not-merge,pr-verify-failed, human-merge hold, outside-actor label) at.github/scripts/pr-merge/enable-auto-merge.mjs:253. Its comment says the human-gated label is checked too, at:11-12.do-not-merge,needs-humanandpr-verify-failed, and requires the head's exact-name pr-verify pass.So "refuse at arm" is already covered, and this issue narrows to dequeuing a PR when a hold label is added after it entered the queue. That covers the 9 above, about 1.3 a day. It adds a
labeledwake topr-merge.yml, so it is built after #12446 and alongside #12445, which edits the sameon:block. The AFTER measure counts only failures with the label added after entry.Criterion 1, removing the duplicate ci-status hold reads, is dropped. The ci-status merge-group reads are fail-closed, while select-tests is fail-open and runs pull-request code, so moving the holds there would weaken them, which the criterion itself rules out ("No hold is dropped"). The body-hold and pr-unblock steps belong to #12446.