[kyle-sexton]

Closes #10835

Hold paths. This changes .github/workflows/pr-require-checks.yml, which is on the hold-path floor. No hold is loosened: every state that failed ci-status before still fails it, and a base module that cannot judge fails closed.

Summary

• ci-status's Fail a queued pull request carrying a hold label step now judges do-not-merge, the human-gated label and pr-verify-failed with the base checkout's merge-holds.mjs labelHolds, and names the human-gated label with its humanGatedLabel. Before this, the step had its own bash copy of those rules.
• The kill switch in Fail a lanes App pull request its body or a member holds now runs the base module's switchHold.
• Fail a pull request carrying the human-gated label (pull_request) resolves its label with humanGatedLabel too. That is the third copy of the label-name rule, and with it switched the label and kill-switch rules are stated once, in merge-holds.mjs. select-tests' queueHolds already uses them.

Fix

• Module source. Every step loads the module from the base_config sparse checkout of the base commit (.github/scripts/pr-merge/merge-holds.mjs was already in that checkout). It is never loaded from pull request code.
• Fail closed. Each of these fails the step with an ::error::, before any further read:
  • the module is missing or empty;
  • it does not export the function;
  • the function throws;
  • the module exits without printing;
  • it answers the wrong type: a label that is not a non-empty string, or a reason list that is not an array of non-empty strings.
• Main guard. The module path is never argv[1] of the inline node -e. merge-holds.mjs runs its main when argv[1] is its own URL, and its usage error would then set exit code 2.
• Error texts. They are the functions' texts, which are byte-identical to the old bash ones. reconcile.test.mjs keys on the kill-switch text, so its pin now checks that the step prints switchHold's reason and that switchHold's reason still carries the KILL_SWITCH_NOTE the reconciler matches.
• Order. The kill switch is still judged right after the author check: before the into-main exit, and before the contents-API reads of the module and config. A lanes App pull request into another branch still fails while the switch is on, and a new case pins that.
• Reads. Unchanged. The labels are still read live with gh_read (issues/N/labels), and the pull is still read through pr_read. The pr_read scope set in perf(ci): fail held merge-queue pull requests before the lanes run #10838 (pull, labels, events) is untouched, and comments, reviews and runs are still read live.

Verification

• API calls per ci-status run: unchanged. The diff adds and removes no gh_read, pr_read, read3, read_api or gh api call site (git diff -U0 | grep -cE 'gh_read|pr_read|read3|read_api|gh api' on the changed lines prints 0). The stubbed step bodies log the same calls as before:
  • merge_group label step: 1 labels read, then the trust hold's 1 GraphQL read for a person's pull request;
  • pull_request human-gated step: 1 labels read;
  • lanes App step: unchanged.
  • The step-level counts asserted in the tests are unchanged.
• Every old case still passes, unchanged. Suites: ci-hold-merge-group 76/0, ci-hold-human-gated 55/0, ci-lanes-pr-hold 240/0, ci-hold-lifted-by, ci-hold-pr-unblock-status, ci-early-queue-holds, ci-hold-cost-decision, ci-gh-read, ci-review-lanes, workflow-self-paths, check-lane-coverage.test.sh, check-lane-coverage.sh --check, merge-holds.test.mjs and reconcile.test.mjs all pass. actionlint passes on the workflow, with shellcheck on PATH, and shellcheck passes on the three test scripts.
• New cases:
  • an empty tracker config, every hold label named together, a trimmed label and a blank label;
  • fail-closed cases for a missing module, a module without the function, the wrong answer type, an empty reason, a silent exit and a throw, each failing before any further read;
  • the kill switch on a lanes App pull request into another branch, and in the merge queue.
• Mutation check: 6 of 6 mutants of merge-holds.mjs were killed:
  • each of the three labels dropped from labelHolds;
  • humanGatedLabel ignoring the tracker;
  • switchHold never holding;
  • switchHold ignoring CLAUDE_LANES_REPO_DISABLED.
• Left out: none of the CI-only suites (ci-review-plugin-source.test.sh, resolve-config/callers.test.mjs) touch these steps. CI runs the full selected corpus.

Related

• Refs: ci: fail held merge-queue pull requests before the lane legs run #10747
• perf(ci): fail held merge-queue pull requests before the lanes run #10838 added labelHolds, switchHold and queueHolds. This switches ci-status to them, now that the base carries them.

🤖 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-29T06:03:25.321729Z	50c9013	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 3aeee7c636e32e4270c6fa75a9eafbd5b0ae1521: 0 finding(s), 0 as inline comments.

I read the whole diff and checked it against merge-holds.mjs and the surrounding ci-status steps. I found no security issues, so there are no inline comments and no hold. I ran no tests, because this lane has no shell.

critical-findings: 0

Trust controls read. The change touches the ci-status hold steps in .github/workflows/pr-require-checks.yml: the human-gated label, the do-not-merge and pr-verify-failed queue holds, and the lanes kill switch. It replaces their bash copies with the base commit's merge-holds.mjs functions humanGatedLabel, labelHolds and switchHold. Nothing is widened.

• Module source. Every step loads the module from $BASE_CONFIG, the base_config sparse checkout of the base commit. That path is empty unless steps.base_config.outcome == 'success' (workflow lines 4526, 4978 and 5397). Pull request code never reaches node. The pull_request event steps use the same base-commit checkout, so a pull request cannot swap in a weaker rule.
• Equivalence of the rules. I compared labelHolds (merge-holds.mjs:1411-1423) with the removed bash.
  • The three labels are held and the error texts match.
  • Label matching is exact, as grep -qxF was.
  • The tracker label is trimmed and falls back to needs-human.
  • switchHold (merge-holds.mjs:1429-1438) holds unless both variables are exactly false, which is what the old != false test did.
  • For a whitespace-only tracker, JSON.parse throws and the step fails closed. The old jq also errored there.
• Fail closed. Every failure path ends in exit 1 with an ::error:::
  • a missing or empty module;
  • a missing export;
  • a throw;
  • a silent exit, which leaves reasons or switched empty and trips the -z checks;
  • a wrong answer type, caught by the typeof and Array.isArray checks inside the node -e script.
• No new dependency in the kill switch. The step already needs $BASE_CONFIG for the identity check (workflow line 5409) before it reaches the kill switch. The kill switch still runs right after the author check and before the into-main exit.
• Main guard. argv[1] is label, holds or the pull JSON path, never the module, so the module's main cannot run. pathToFileURL(process.argv[2]) uses the right -e argument offsets in all three scripts.
• Command-workflow injection. ${reason//%/%25} escapes %. The reasons contain only the pull request number and the label name, which comes from the base tracker. A pull request cannot control either. The old bash also echoed the label unescaped.
• Reads. I found no added or removed API call sites, which matches the description.

Spend. The change adds no paid-plan, billing or lane-run spend.

Manifests. The diff touches no plugin manifest.

[github-actions]

claude-review reviewed 3aeee7c636e32e4270c6fa75a9eafbd5b0ae1521: 0 finding(s), 0 as inline comments.

No findings. I read the full diff and checked it against merge-holds.mjs (humanGatedLabel, labelHolds, switchHold) and the lanes step in pr-require-checks.yml. I did not run any tests (no shell in this lane); the pass counts in the PR description are author-claimed and unverified.

• Module source and main guard: all three steps load merge-holds.mjs from the base_config checkout. The module is never argv[1] of the inline node -e, so its main guard can't run. The argv indexes match each call.
• Fail-closed paths: a missing module, missing export, throw, silent exit, or wrong answer type stops the step with an ::error:: before any further read. A silent exit in the lanes step leaves $switched empty, which the [ -z "$switched" ] check catches. In the merge-queue step, a [] answer is non-empty and so reaches jq and passes as "no holds".
• Equivalence with the old bash:
  • labelHolds matches label names exactly, as grep -qxF did, with the same three error texts in the same order.
  • humanGatedLabel trims and falls back to needs-human as the old jq did. A whitespace-only tracker config still fails closed because JSON.parse throws, and the new tests pin that.
  • switchHold returns the same kill-switch text as before.
• Kill-switch ordering: it still runs right after the author check, before the into-main exit. pull is the raw pulls-API object, so user.id, user.type and number are present. The new test for a lanes App pull request into another branch pins the ordering.
• Closing issue ci: switch ci-status's hold-label and kill-switch checks to merge-holds.mjs labelHolds and switchHold #10835: both "Done when" criteria are met. The label and kill-switch rules are now stated once, in merge-holds.mjs, and every state that failed before still fails. The new fail-closed, label and kill-switch cases cover them, and reconcile.test.mjs still pins the KILL_SWITCH_NOTE text.
• Cost: no billing or paid-resource change.

[melodic-automation-lanes]

The pr-unblock lane stopped resolve-conflicts on this pull request: the merge stages the pull request's own .github/workflows/ changes together with the base's, which the lane token cannot push, so a session merges main by hand. The lane sets no label: the run ends in a failing pr-unblock/conflict status on the head the lane gated.

• Reason: workflow-staged
• Head: 52b367b337e5b7c485ea9e7c76b1e423c5f16866
• Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/40827166525

When a retry on this head can change the outcome, the lane runs one on its own, up to its per-head cap, and edits this comment if it stops again; a run that does not stop clears the status. When no retry can (a review thread the lane does not take or settle, or a failing check after the retries are spent), a person resolves the threads or gets the checks green, then dispatches the workflow below: that run finds the cause gone and clears the status. On a pull request the lanes App opened, the dispatchers start that run once per stop; on any other, a person does. A new commit to the branch starts clean.

To run it again on this head now, dispatch the pr-unblock workflow:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=10876 -f activity=resolve-conflicts