[kyle-sexton]

Closes #12449

Summary

When the lanes App adds the human-gated label (needs-human) to a pull request that is in the merge queue, it now takes that PR out of the queue at the same call site. Before this, the PR's merge group still ran, and ci-status failed it on the label. verify-notes.mjs leaveQueue already did this for pr-verify-failed.

Why this design: of the 22 hold-label merge-group failures in the 7-day BEFORE baseline, 9 had the label added after the PR entered the queue (design check):

• 5 were needs-human, which the lanes App added.
• 4 were pr-verify-failed, whose writer already dequeues.

The earlier version of this PR woke pr-merge on a person's labeled event. That design catches none of the 5, because a bot-sent event is skipped and the token broker refuses a bot actor. So the dequeue now sits in the writers themselves. The merge-group ci-status already re-reads hold labels in the queue (.github/workflows/pr-require-checks.yml). That check stays the backstop, and the dequeue spares the failed group.

The label set follows the owner's decision on #12446 (Q3), quoted from the issue comment:

Question (verbatim): "Q3 (revised): which label goes on routine stuck-agent PRs in #12446?" with recommendation (a): "match each label to its condition. Vetoes keep do-not-merge. Lane stops and stuck review findings get needs-session, which already exists. needs-human stays only for intent, design, security and cost. human-merge-hold is removed."

Owner's answer (verbatim): "Go with recommendations."

So needs-human is the hold these writers add, resolved by humanGatedLabelFrom in .github/scripts/pr-unblock/address-feedback.mjs. needs-session is not a hold, and human-merge-hold is gone.

Fix

• address-feedback.mjs:
  • New leaveQueueOnHold: after a hold-label add, it makes one QUEUE_ENTRY_QUERY read. If the PR is queued, it sends one DEQUEUE_MUTATION.
  • It prints one line per add: ::notice title=hold-label-dequeue::pr=<n> label=<l> outcome=<dequeued|not-queued|refused:<why>>.
  • A refused read or dequeue warns and never fails the label add.
  • QUEUE_ENTRY_QUERY and DEQUEUE_MUTATION are exported, so there is one copy of each.
• Writers that add the human-gated label with the lanes App token, all wired:
  • report-escalation.sh, hold route, right after it adds the label. It loads both queries from address-feedback.mjs, which the base-script copy carries beside it, and sends them with gh api graphql.
  • enable-auto-merge.mjs, the refusal note: both where it adds the label and where it puts it back.
  • verify-notes.mjs already dequeues its own adds (leaveQueue), and is unchanged.
• Writers left out, because they use the job's GITHUB_TOKEN, not the lanes App token:
  • security-outside-actor/flag.mjs
  • the check-signed-commits action
• pr-merge.yml is unchanged from main: the labeled wake, its job condition and its concurrency group are gone.

Verification

• node --test on .github/scripts/pr-merge/enable-auto-merge.test.mjs, workflow.test.mjs and clear-escalations.test.mjs, and on .github/scripts/pr-unblock/address-feedback.test.mjs and verify-notes.test.mjs: 646 tests, 0 failed. .github/scripts/pr-unblock/report-escalation.test.sh: 1147 ok, exit 0. shellcheck is clean on both shell files.
• New cases, in both enable-auto-merge and report-escalation:
  • A label add on a queued PR does one read, then dequeues it.
  • A label add on a PR that is not queued does 1 read and no write.
  • A refused dequeue warns, prints refused:dequeue-failed, and the label add and the escalation comment still stand.
  • No label add (a lane refusal, a fail route, the label already on) does no queue read.
• I broke the dequeue in leaveQueueOnHold on purpose, and the dequeue tests failed (2 of 2).
• scripts/check-affected.sh --base origin/main: 11 passed, 4 skipped. No plugin changed, so no changelog fragment is required.
• Call delta: one extra GraphQL read per needs-human add by the lanes App, plus at most one write (the dequeue) when the PR is queued. Every other path makes no new call.
• AFTER measure: count hold-label-dequeue notices with outcome=dequeued, against hold-label merge-group failures where the label was added after queue entry.

Related

• pr-pipeline: converge own-actor merge holds on the do-not-merge label #12446, fixed by fix(pr-pipeline): converge our own actors' merge holds on do-not-merge #12522: the label set and the Q3 decision.
• verify-notes.mjs leaveQueue: the same step for pr-verify-failed.
• fix(source-control): retry a truncated artifact listing page in the verdict reader #12418: verdict-reader refusals, kept out of this issue's metric.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VcupomBPg7kuobwLoNpxEW

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-03T03:11:52.666469Z	3931880	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

claude-security-review reviewed e500cf25dd4fc4099a927b26a10fa3318f6439ba: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12608, incremental since 3931880

No security issues found.

critical-findings: 0

Scope: This was an incremental review. The only changed file is .github/scripts/pr-merge/enable-auto-merge.test.mjs, and I read its single hunk in full. It is a test file, and it does not match any listed trusted path; only its sibling enable-auto-merge.mjs does. Every other file in the PR is unchanged since my last review of 3931880, which found no issues.

What changed: The push adds one test, "a refused pull request whose label the lane puts back while queued is dequeued". It checks that when the refusal-note path puts needs-human back on a PR that is in the merge queue, two things happen:

• the label is added (POST /issues/1/labels);
• the PR is dequeued, and the run prints a dequeued hold notice.

Why it is not a security concern:

• The test runs against mocks, so no production code path, token request, workflow or trust control changed.
• It covers the hold label being put back, which is the path that keeps a hold in place, so nothing here weakens a merge hold.

Untrusted text: Nothing in the PR context tries to steer the review.

Owner threat model: This push weakens, narrows or removes no named friction hold.

[github-actions]

claude-review: 1 inline finding(s) on 3931880eb9d6440be1a8087db454fe06d444dcbb; the summary is in this lane's pull request comment.

claude-lane: claude-review

[github-actions]

claude-review reviewed e500cf25dd4fc4099a927b26a10fa3318f6439ba: read 1 of 1 changed file(s); 1 finding(s), 0 as inline comments.

Code review (incremental, 3931880 → e500cf2): dequeue on a lanes-App needs-human add (#12608)

Scope: I read the one changed file, .github/scripts/pr-merge/enable-auto-merge.test.mjs, which adds one test. I read its whole change.

Last round's inline finding is resolved

The new test, a refused pull request whose label the lane puts back while queued is dequeued (enable-auto-merge.test.mjs#L4081-L4089), uses the same setup as the existing relabel case at L2236-L2241: the lane's refusal comment and its hold note are present, no label is on, and no unlabel event exists. The only difference is queued: true.

• Its notes assertion allows only the label POST. That forces the writes.body === null branch at enable-auto-merge.mjs#L1912-L1922.
• Its writes and dequeued notice assertions then depend on the leaveQueue call at L1920.

So deleting that call now fails a test, which is what the finding asked for. I traced this by reading the code; I did not run the tests.

Findings

1. Advisory, restated from the last round (it has no line anchor, so it is repeated each round): closing issue pr-pipeline: read merge-queue holds once and dequeue a PR when a hold label is added #12449, criterion 2. The criterion reads: "A hold label added to a queued PR removes it from the queue before its group runs."
   • What the PR covers: needs-human added by the lanes App (report-escalation.sh#L368, enable-auto-merge.mjs#L1920, L1929), and pr-verify-failed (verify-notes, which already dequeued).
   • What it leaves out: a person adding do-not-merge or needs-human, security-outside-actor/flag.mjs, and the check-signed-commits action. None of these dequeue.
   • Why the gap matters: the issue's design-check comment narrowed the criterion only to labels added after queue entry. It did not narrow it to particular writers, and nothing in the PR context has changed that since the last round.
   • What to do: before merging, either have the owner narrow criterion 2 on pr-pipeline: read merge-queue holds once and dequeue a PR when a hold label is added #12449 or open a follow-up for the remaining writers. Otherwise closing the issue records the criterion as met when it isn't.

Other notes

• No settled thread was re-raised. The owner's reply on the L1920 thread says the fix is in this commit, and I checked that the fix holds, as described above.
• Round count: 1 earlier round with findings, cap 3; the cap did not apply.
• No plugin manifest changed. No embedded instructions were found in the PR context, settled threads or prior findings.