[kyle-sexton]

Refs: #10747

This delivers part of #10747: the pr-unblock rule stated once (ci-status's step now runs merge-holds.mjs unblockStops) and the early merge-queue step in select-tests. The remaining part, switching ci-status's hold-label and kill-switch steps to labelHolds/switchHold, is tracked in #10835 and can land only once this is on main.

Owner merge approval required. This changes gate and trust hold paths (.github/workflows/pr-require-checks.yml, .github/scripts/pr-merge/merge-holds.mjs, both on the hold-path floor). No hold is loosened: every state that fails ci-status today still fails it.

Summary

• ci-status's Fail a head a pr-unblock lane stopped step now judges the head's statuses with the base commit's merge-holds.mjs unblockStops instead of its own jq/awk copy, so that rule is stated once.
• In merge_group runs, select-tests (which every lane leg needs) first judges the holds that cost no lane work: hold labels (do-not-merge, the tracker's human-gated label, pr-verify-failed), a pr-unblock/* stop on the queued head, and the lanes kill switch on a lanes App pull request. A held group fails select-tests in seconds and the legs skip. ci-status keeps all its own hold steps, so the aggregate still fails on the hold.
• Separate commit (coordinator scope): ci-status reads each pull request fact once per job through a new pr_read cache in the read helper.

Fix

Shared rules. merge-holds.mjs gains humanGatedLabel, labelHolds, switchHold and queueHolds (which composes them with the existing unblockStops). The reasons they return are ci-status's current error texts.

Early step. Two steps at the top of select-tests, merge_group only:

1. One shallow git fetch of the group's base commit, then git show of its merge-holds.mjs, api-budget.bash and .work-item-tracker.json. This step never fails the job; it sets ok=true only when the module and wrapper read. The rules always come from the base, never from the queued pull request's code. The runner's Node runs the module (queueHolds needs nothing newer than the image's).
2. Read the pull request (pulls/N, which carries its labels and author) and its head's statuses through the api-budget gh wrapper, then run the base's queueHolds. Only a positive hold fails the job. A failed read, an unparsable ref, or a base module without queueHolds logs a warning or notice and leaves the decision to ci-status, which retries reads and fails closed. That way a transient API error cannot become a new reason for the queue to eject a pull request. Nothing from this job feeds a ci-status hold decision, because this job runs pull request code.

select-tests gains statuses: read. "List commit statuses for a reference" needs the "Commit statuses" read permission (permissions required for GitHub Apps). Labels come from the pulls response, which pull-requests: read covers.

Skips cannot pass. When select-tests fails, each skipped leg stays skipped in Check each skipped lane against scope (that step passes a skip only when select-tests succeeded), and treat-skipped-as is fail in merge_group. The existing case "a skip behind a failed scope stays skipped" in scripts/ci-ordered-skips.test.sh covers this.

Sequencing (deviation from the issue's done-when). ci-status loads merge-holds.mjs from the merge group's base. A ci-status that called labelHolds or switchHold in this PR would fail closed in this PR's own merge group, and the PR could never merge. So ci-status keeps its bash label and kill-switch checks for now, and the follow-up issue linked below switches them, the same two-step path #10737 took for unblockStops. Until then the label and kill-switch rules exist in two places. The early step also does nothing in this PR's own merge group, because the base lacks queueHolds and the step logs that. It takes effect for groups whose base contains this commit.

Read cache (separate commits). The coordinator asked for it: ci-status was the top GITHUB_TOKEN spender, at 528 requests an hour measured 23:55 to 00:55Z (39 runs, median 17 calls each), with each run reading the same pull request facts again. The read helper writes pr_read, used for three reads only: the pull request, its labels and its issue events. The step answers from a read an earlier step of the same job made, else reads live and keeps the answer in $RUNNER_TEMP/pr-reads. Comments, reviews, runs, jobs and annotations are always read live, so a member's veto or a new review is never missed. A failed read keeps nothing, so the next step reads again and fails closed as before. The cost-hold step reads everything live, because it acts on its reads, and keeps only its pull and events answers. Every step that writes to the pull request (the cost-hold step's comment and label removal, the lanes App step's auto-merge disarm) drops every kept answer after the write. check-contract writes before anything is kept. Measured saving: in a dry run of the step bodies against a gh stub, a pull_request run sends 3 requests instead of 4 (the events read is shared) and a merge_group run sends 2 instead of 3 (the pull read is shared).

I did not replace the review-lane step's REST run and job reads with one GraphQL query. The step filters runs by head_repository.full_name == this repo, so a fork's run on the same SHA never counts as the review. GraphQL WorkflowRun has no head-repository field (introspection: checkSuite createdAt databaseId deploymentReviews displayTitle event file id pendingDeploymentRequests resourcePath runAttempt runNumber updatedAt url workflow), and CheckSuite.repository is the base repository. The swap would drop that filter.

Verification

Measurement: merge-group runs failing on a designed hold. I took 10-27 and 10-28 UTC (10-29 was partial and is excluded). There were 385 merge-group runs and 31 failures. Of the failures, 13 were designed holds (about 6.5 a day; the issue's 20 to 26 a day did not reproduce). 9 of those 13 are holds the early step catches (about 4.5 a day): 3 needs-human, 3 pr-unblock stops and 3 kill-switch stops on lanes App pull requests; the other 4 are pr-verify verdict holds, which still need ci-status's lanes-hold step. The 9 caught runs spent 283.6 job-minutes in lane legs (excluding ci-status, mark-lanes and select-tests), which works out to about 142 lane job-minutes a day saved. Each held group's wall time drops from 5 to 10 minutes to the select-tests runtime plus about 0.5 minutes of ci-status. Commands:

gh run list --repo melodic-software/claude-code-plugins --workflow pr-require-checks.yml --event merge_group --created '>=2026-10-27T00:00:00Z' --json databaseId,createdAt,conclusion,status -L 400
gh api repos/melodic-software/claude-code-plugins/actions/runs/<id>/jobs?per_page=100        # one per failed run: failed ci-status step, job start/end times
gh api repos/melodic-software/claude-code-plugins/check-runs/<ci-status job id>/annotations   # one per hold run: which hold

API calls.

• The early step adds 2 GITHUB_TOKEN requests per merge-group run (the pull and one statuses page per 100 statuses) and ends with its own api-calls notice. At 140 to 245 merge-group runs a day, that is 280 to 490 a day.
• ci-status must still read the statuses again, because select-tests runs pull request code and its reads cannot feed ci-status.
• The read cache saves requests on the same hold steps. A dry run of the step bodies against a gh stub sent 3 requests instead of 4 on a pull_request run (cost-hold, human-gated and hold-lift steps share the events read), and 2 instead of 3 on a merge_group run (pr-unblock and lanes App steps share the pull read). That is 1 request saved per ci-status run, about 39 an hour at the measured 39 runs an hour, against the early step's 12 to 20 an hour.
• More is saved when the review-lane step shares the pull read for exempt authors.

Tests.

• actionlint passes on the workflow.
• scripts/ci-early-queue-holds.test.sh is new. It runs the early step body for each hold (labels, the tracker's own label, a pr-unblock stop, the kill switch), for the pass-to-ci-status paths, and for the api-calls notice of 2.
• Mutation-checked, with all 6 mutants killed: the step never failing, the gh wrapper removed, queueHolds dropping the kill switch, unblockStops ignoring the status creator (killed after adding the "another account's failure does not hold" case), the human-gated label ignoring the tracker, and an unreadable pull failing closed.
• merge-holds.test.mjs gets queueHolds cases.
• ci-hold-pr-unblock-status.test.sh adds cases for a missing base config, a base module without unblockStops, and a module answering no list (each fails closed).
• ci-gh-read.test.sh adds pr_read cases.
• ci-hold-cost-decision.test.sh checks that the step reads live even with a kept answer, keeps its events read, and drops kept reads after writing.
• These pass locally: ci-gh-read, ci-hold-cost-decision, ci-hold-human-gated, ci-hold-lifted-by, ci-review-lanes, ci-lanes-pr-hold, ci-hold-merge-group, ci-hold-pr-unblock-status, ci-ordered-skips, check-docs-only-gate, workflow-self-paths, check-lane-coverage, lint-shell-filter, the lint-shell gates, log-api-budget.test.mjs, the pr-merge node tests, and reconcile.
• ci-review-plugin-source.test.sh and resolve-config/callers.test.mjs were not run locally because packages are missing here (yaml, ajv). CI runs the full selected corpus.

Related

• Refs: fix(pr-pipeline): end designed stops cleanly and keep held pull requests out of the merge queue #10737
• Follow-up: ci: switch ci-status's hold-label and kill-switch checks to merge-holds.mjs labelHolds and switchHold #10835 (switch ci-status's label and kill-switch checks to the shared functions)

🤖 Generated with Claude Code

[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-10-29T02:23:16.915202Z	—	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 6f466e936a475377c22f73c99b26999318a55f77: 0 finding(s), 0 as inline comments.

Incremental security review of .github/workflows/pr-require-checks.yml and scripts/ci-lanes-pr-hold.test.sh since b5669e3. I found no security issues.

critical-findings: 0

What changed, and the trust controls it touches

• The pr_read cache is now limited to three reads: the pull, its labels and its issue events. Comment reads in the lanes-hold, reason and held-at steps now go live through gh_read. read_api/read3 default to gh_read, and only the pull reads opt in with READER=pr_read.
• The cost-hold step's read_api now keeps an answer only when its path matches pulls/N or issues/N/events?per_page=100. The regex is anchored and matches those two shapes.
• After a successful auto-merge disarm, the lanes App step calls pr_read_forget, so later reads take the pull live.
• The test adds a check that no kept reads remain after the disarm.

What I checked

• These edits touch the ci-status hold steps, which are a merge-hold path. Each edit narrows what is cached, or drops the cache after a write, so the steps see fresher state than before. A member's veto in a comment, or a new review, can no longer be missed through a stale kept answer.
• Failure handling is unchanged. A failed read keeps nothing and the step fails closed after three attempts.
• READER is set only as a per-command prefix inside the workflow's own shell. A pull request author cannot set it, and it falls back to the live gh_read.
• The one remaining cache reuse is the pull read. The cost-hold step forgets it after its writes, and the push step forgets it after its disarm. The earlier push-time disarm path lacked this and now has it.

No manifest fields are in this scope. No billable resource changes, so there is no cost hold. The change can only reduce API request spend, which is not a hold.

[github-actions]

claude-review reviewed 6f466e936a475377c22f73c99b26999318a55f77: 0 finding(s), 0 as inline comments.

Incremental review of .github/workflows/pr-require-checks.yml and scripts/ci-lanes-pr-hold.test.sh since b5669e3. I read the files only; I ran nothing. No findings.

What I checked:

• Cost-hold read_api (L4244): the regex keeps only the pull (pulls/N) and events (issues/N/events?per_page=100) answers. The keys match the later pr_read calls, which pass the same args and --paginate. Labels are no longer kept from this step. That is fine, because its labels read uses --jq, so the args differ from the later pr_read labels call. Comments, runs, jobs and annotations are no longer cached, so a member's veto or a new review is read live.
• READER switch: read_api and read3 now default to gh_read and use pr_read only where a caller sets READER=pr_read, and only for the pull. The three comment readers (L4552, L4617, and the one in the cost-hold block) now use gh_read. No remaining pr_read call reads comments, reviews, runs, jobs or annotations.
• Disarm path (L5382): pr_read_forget runs after a successful disablePullRequestAutoMerge. The in-memory $pull still holds the pre-disarm value, but nothing after it re-reads auto_merge. Later reads of the pull go live.
• Test (L632): RUNNER_TEMP is $TMP_ROOT, and the step's earlier pr_read of the pull creates pr-reads. The cache is reset per run at L264, so the absence check after the armed push run is not vacuous.

No cost or critical-security hold. The change spends no billed resources.

[github-actions]

The pr-verify verifier failed head b5669e3a58fc8b14b9b7333a19cee8d809b8deae. What it wrote, as data:

Summary: The PR closes #10747 but does not deliver all of it. The pr-unblock rule now lives only in `merge-holds.mjs`, and the merge-queue step in `select-tests` is in place with read-only `statuses: read`. But the issue asks that select-tests and ci-status call one shared script so no rule is copied, and this PR does the opposite: it adds JS copies of the hold-label and kill-switch rules (`labelHolds`/`switchHold`) while ci-status keeps its bash versions. The PR body admits this and moves the work to follow-up #10835. A PR that leaves a required part for later must link the issue with `Refs:`, not `Closes`. It also adds a per-job read cache (`pr_read`) to ci-status that the issue did not ask for.
- (spec) Issue #10747 asks that select-tests and ci-status call 'one script or composite action' for the hold checks so no rule is copied. The refinement's criterion says the same: 'Both select-tests and ci-status call the same single script or composite action for those checks, so no rule is copied.' This PR leaves ci-status's bash hold-label and kill-switch steps as they are and adds a second copy of both rules in JS. Fix: change the PR body's `Closes #10747` to a `Refs: #10747` line, and state that the delivered part is the shared unblockStops rule plus the early merge-queue step. Name what remains: switching ci-status's label and kill-switch steps to labelHolds/switchHold, tracked in #10835. Otherwise, deliver that switch in this PR.
- .github/scripts/pr-merge/merge-holds.mjs:12: humanGatedLabel, labelHolds and switchHold restate rules that ci-status's hold-label and lanes App steps still apply in bash. Until #10835 lands, a change to one copy can drift from the other. The early step can then eject a group that ci-status would pass, or the reverse.
- .github/workflows/pr-require-checks.yml:5213: the issue says to switch this step to `holds_js unblockStops`. Instead it runs its own inline `node -e` import of the base module. The rule itself is still stated once, so this deviation is minor, but either use holds_js or say in the step comment why it can't be used.
- .github/workflows/pr-require-checks.yml:3923: the pr_read per-job read cache, and the move of ci-status readers from gh `--jq` to raw pages, is unasked work (the body calls it 'coordinator scope'). pr_read keeps any successful answer for any args. Confirm that no ci-status step reads through pr_read/read_api in a loop that waits for state to change, such as a run or job status at line 4729 or 5327. A kept answer would make that wait read stale data. Also confirm that every step that writes to the pull request calls pr_read_forget, not only the cost-hold step.
- .github/workflows/pr-require-checks.yml:293: select-tests gains `statuses: read`, with a comment explaining why, and no write scope. This matches the issue. The early step's GH_TOKEN can only read in a job that runs pull request code.

This pull request leaves the merge queue, and ci-status holds it out while it carries pr-verify-failed. A pr-verify pass at a new commit removes the label.