feat(guardrails): block a PR merge while review threads, change requests or failing checks remain

[kyle-sexton]

Requested by Kyle · project thread

Closes #6463

Summary

Before: nothing checked a direct gh pr merge, REST or GraphQL merge, or MCP merge call. An agent could merge (or ask for --admin) without reading an unresolved review thread.

After: a new guardrails guard, block-unresolved-pr-merge, blocks a merge while the PR has an unresolved review thread, a reviewer whose latest decisive review requests changes, or a failed check run or commit status on its head. The block message lists each thread (path:line, first line of the comment, link), each changes-requested reviewer and each failing check, and names the fix. --admin and --auto are held to the same rule.

Fix

• plugins/guardrails/hooks/block-unresolved-pr-merge.sh, on the existing Bash|PowerShell run-guards row plus a new MCP row (^mcp__.+__(merge_pull_request|enable_pr_auto_merge)$). Matches the scope update's table: gh pr merge (all flags), PUT repos/{o}/{r}/pulls/{n}/merge via gh api or curl, the mergePullRequest / enablePullRequestAutoMerge / enqueuePullRequest mutations via gh api graphql or curl, and the MCP merge tools. Matching runs on parsed argv at a command position, so a quoted mention (commit message, echo, heredoc) stays quiet. Commands that name neither merge nor enqueue leave after one string test.
• GraphQL is refused in Claude Code cloud sessions (HTTP 403), so PR state is read over REST: pulls/{n}, pulls/{n}/reviews, commits/{sha}/check-runs, commits/{sha}/status. Thread resolution has no plain REST route: GraphQL reviewThreads where served, else the cloud route GET pulls/{n}/ccr/review_threads named in that 403 body (a CLAUDE_CODE_REMOTE session tries the cloud route first). The same 403 body names PUT pulls/{n}/ccr/auto_merge as the cloud auto-merge path, so that route is matched too.
• Fails open with a notice on both channels (systemMessage and additionalContext) saying the merge will run unchecked when gh is missing or unauthenticated, a read fails, the read budget runs out, or the PR cannot be identified.
• Fails closed for a GraphQL merge, auto-merge or merge-queue mutation whose PR cannot be resolved. That covers an id from $ID, "$PR_ID", $(jq ...) (the source-control merge reference 4.2.2 enqueue shape from fix(source-control): confirm a merge-queue PR entered the queue after gh pr merge #7075), a node that can't be read, or no gh. The block message says how to fix it: pass a literal -f id=PR_... or use gh pr merge <n>.
• One time budget: all reads share 35 s, and each call is capped at the time left and at 15 s, so the shared Bash row stays under its 60 s timeout. A row timeout would drop every guard's block (run-guards.sh, AGGREGATION). The cap is enforced by timeout where it exists, and by a watcher process where it doesn't (stock macOS). No environment variable changes the budget: settings.json env reaches a hook's environment, so a knob could make every merge run unchecked. The budget tests run a copy of the hook with its one budget line rewritten.
• Command position: the guard steps over leading assignments, reserved words the tokenizer leaves at a segment head (then, do, {, ...), and the command/time [-p]/exec/nohup prefixes. It reads gh pr -R o/r merge. gh pr merge --help/-h and --disable-auto are not merges. The guard header lists the remaining gaps: wrappers like sudo/env/xargs, child shells, $(...), and --input mutation files.
• Registered in guard-requires.sh (MCP inputs declared unprimed), hooks.json, plugin.json userConfig block_unresolved_pr_merge_enabled (default on), README guard/option tables, changelog fragment (minor).

Defaults I picked where the issue left a detail open (please confirm):

1. gh pr ready is not matched. The 2026-10-06 scope update dropped the advisory and its table lists merge paths only; the issue title still says "and gh pr ready". Adding it is one line if wanted.
2. Pending checks block an immediate merge but not an auto-merge or merge-queue request (--auto, enablePullRequestAutoMerge, enqueuePullRequest, enable_pr_auto_merge, ccr/auto_merge), since those wait for checks by design. Failed checks, threads and change requests block all paths.
3. Every unreadable state fails open (not only missing/unauthenticated gh), with the org ruleset as the server-side backstop, as the issue's residual note says. The one exception is the unresolvable GraphQL mutation above, which fails closed.
4. Outdated unresolved threads count, matching the babysit merge gate.

Open for Kyle (found by the overnight verifier, not changed here):
5. The fail-closed GraphQL rule also covers gh missing or unauthenticated. A mutation with a literal PR_... id whose node can't be read is blocked. That covers no gh, a 401, and the cloud GraphQL 403. The issue's acceptance says the guard fails open when gh is missing or unauthenticated. Option A: keep it, since the mutation can't be checked. Option B: fail closed only for a non-literal id and fail open, with the notice, for a literal one.
6. Interaction with #7075. Its source-control merge reference 4.2.2 passes the enqueue id through $(jq ...). With this guard on, that step is always blocked, even on a clean PR (verified against branch fix/6630-merge-confirm-queue-entry). One side must change before both merge: #7075 passes a literal id or uses gh pr merge, or this guard relaxes the rule.

Verification

• bash plugins/guardrails/hooks/block-unresolved-pr-merge.test.sh -> 162 passed, 0 failed (at 96f4c12). Since the first push, the suite also covers:

  • the fail-closed GraphQL cases (substitution id, $ID, "$PR_ID", --input, the full fix(source-control): confirm a merge-queue PR entered the queue after gh pr merge #7075 shape, an unreadable literal node)
  • --help/-h stay-quiet cases that assert no gh call
  • every command-position prefix, plus gh pr -R o/r merge
  • the budget with a hung fake gh, both with and without a timeout binary on PATH (returns in under 8 s), run on a copy with a 2 s budget
  • a case where an env var can't shorten the budget (GUARDRAILS_BUPM_BUDGET_S=1 with a 0.7 s gh still blocks). It failed before the knob was removed.

  A fake gh on PATH serves fixtures and logs calls. Covers every merge path, both shells, MCP, GraphQL-refused fallback to the cloud route, current-branch lookup, the fail-open cases (gh missing, 401, no thread source, no node id), and MUST-stay-quiet cases that also assert no gh call (commit message, echo, gh pr view, --disable-auto, GET of the merge route, a GraphQL query, grep, git merge, heredoc, PowerShell quoted mention, an unrelated MCP tool). Also runs through run-guards.sh.

• scripts/affected-tests.sh --base origin/main --run:

  • First push: 35 shell suites passed, including run-guards.test.sh's declaration checks for the new guard. The 2 selected .mjs suites, run separately with node --test, also passed.
  • Fix push: on a loaded host the run hit the 30-minute background limit. Every suite that finished passed: all 26 guardrails hook suites, plus apply-plan, splice-assets, both routing suites and the lib suites. The four script suites it had not reached all passed when run directly: check-killswitch-hoist 28/28, check-script-contract 51/51, node-notice-rows 6/6, check-guardrails-ps-differential 21/21.
  • Knob-removal push (96f4c12): the run was stopped early at wrap-up. The suites that finished passed, apart from block-hook-bypass, which fails identically on origin/main when the checkout sits under /tmp. A full affected run on this head is still to do. shellcheck -x, check-killswitch-hoist, check-changelog-fragments --check-required origin/main and check-changelog-parity --check pass.

• Live, read-only, in a Claude Code cloud session: the guard against open PRs fix(planning): word interview questions so yes accepts the recommendation #7036 (blocked: failing ci-status, ci-lanes), chore(release): release 27 plugins #6910 (blocked: pending checks), feat: adopt the 2,000-PRs talk decisions across the marketplace #7009 (allowed); 3.0 to 3.6 s per merge command (five sequential calls), recorded in the guard header and README.

• shellcheck -x (repo .shellcheckrc) clean; scripts/validate-plugins.sh, scripts/check-changelog-fragments.sh --check-required origin/main, scripts/check-changelog-parity.sh --check, scripts/check-killswitch-hoist.sh, markdownlint on the README and fragment: all pass.

This changes what agents may do unattended (it blocks merges), so it needs a human merge decision.

Related

• feat(guardrails): add an opt-in ask guard for high-stakes gh, az and git push #5947 (opt-in ask guard for high-stakes gh calls; independent)
• /source-control:babysit-prs merge gate counts threads the same way (outdated included)

🤖 Generated with Claude Code

https://claude.ai/code/session_01L8KdG7fWg2YrErVYWtohmR

---

Generated by Claude Code

---

Generated by Claude Code

[kyle-sexton]

ci-status is red because this PR is a draft, not because of the change. On a draft every lint and test lane skips (lint-repo, test-bash, test-node, test-python, check-skills, check-plugins), and the aggregate counts a skip as not passing. See .github/workflows/pr-require-checks.yml:507-513.

The lanes run once the PR is marked ready. That flip waits for Kyle, because it also starts the review lanes. Until then, the local runs listed under Verification in the PR body are the signal.

---

Generated by Claude Code