Repository navigation
fix(source-control): confirm a merge-queue PR entered the queue after gh pr merge - #7075
Conversation
… gh pr merge `gh pr merge` on a merge-queue base can return with a CLEAN PR left out of the queue. The merge phase now reads isInMergeQueue after every merge on a queue base, enqueues a CLEAN PR left out once at the pinned head with enqueuePullRequest, and reports GitHub's refusal verbatim, so a run ends merged, queued, or held with a named blocker. Closes #6630 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L8KdG7fWg2YrErVYWtohmR
|
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 |
…on armed auto-merge 4.1 step 2 now captures HEAD_SHA from the PR's headRefOid, and the 4.2, async and 4.2.1 merge calls pin to it instead of re-reading the live head, so 4.2.2's head-moved check compares against the verified head. A 4.2.2 hold with auto-merge armed now says a later push could merge on ci-status alone and offers gh pr merge --disable-auto without running it. Refs: #6630 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L8KdG7fWg2YrErVYWtohmR
Resolves the pull-request SKILL.md merge row: keeps #7042's neutral wording (pinned to the reviewed head commit, never arm auto-merge) and adds the 4.2.2 queue-entry confirmation. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L8KdG7fWg2YrErVYWtohmR
… and prefix gaps From review of #7056: - A GraphQL merge, auto-merge or merge-queue mutation whose PR cannot be resolved (id from $ID, "$PR_ID", $(jq ...), an unreadable node, no gh) now fails closed, with the literal-id fix in the message. Tests cover the source-control 4.2.2 enqueue shape from #7075. - gh pr merge --help and -h are not merges; stay-quiet tests assert no gh call. - All reads share one 35 s budget, each call capped at what is left and at 15 s, enforced by timeout or, where it is missing, a watcher process, so the shared row stays under its 60 s timeout. - Leading reserved words (then, do, {, ...) and the command, time, exec and nohup prefixes are stepped over, and gh pr -R o/r merge is read. - The fail-open notice says the merge will run unchecked. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L8KdG7fWg2YrErVYWtohmR
Resolve merge.md against #7170, which moved the phase's gh commands into the GitHub provider file: the phase prose stays provider-neutral and the HEAD_SHA pin, --match-head-commit and the confirm-queue-entry script move to reference/providers/github/pull-request/merge.md. Also, per the owner's direction on #6953: poll mergeStateStatus while it reads UNKNOWN before any merge call in 4.2 and 4.2.1, merging or enqueueing only on CLEAN; and split the enqueue into a read that prints the PR node id and a separate enqueuePullRequest call with the node id and head written as literals, so the guardrails merge guard can resolve the PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WVW9hAwnSBrnFsR3kzXcB4
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WVW9hAwnSBrnFsR3kzXcB4
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @kyle-sexton's task in 4m 34s —— View job Code review complete
2 findings posted inline:
Everything else in the diff (the new §4.2.2 "Confirm queue entry" read/enqueue script, the claude-lane-summary: lane=claude-review sha=7de07c6fd97d01bc433d38fdc559b5ca069b554a run=38403543511 |
|
Claude finished @kyle-sexton's task in 7m 23s —— View job Security review complete
Scope: This PR is documentation-only — it changes skill-reference markdown that instructs an agent how to run
What I checked:
One candidate finding (the new "Confirm queue entry" No security findings to report. claude-lane-summary: lane=claude-security-review sha=7de07c6fd97d01bc433d38fdc559b5ca069b554a run=38403543473 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7de07c6fd9
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
claude-review has reviewed this pull request through 501539e; a later push is reviewed from there. |
|
claude-security-review has reviewed this pull request through 501539e; a later push is reviewed from there. |
…ueued moved head - Stale-base guard reads mergeStateStatus, baseRefName and headRefOid once and uses that head for both the compare and HEAD_SHA, so the pinned head is the one the compare checked. - The confirm read holds when the PR is queued at a head other than HEAD_SHA instead of reporting it queued. - Phase body keeps the hold policy and the provider pointer; the GitHub timing and enqueue behavior stays in the provider record. - readiness.md notes that a PR left UNSTABLE by a redundant stuck status goes to the user, since the merge phase merges only on CLEAN. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WVW9hAwnSBrnFsR3kzXcB4
|
Claude finished @kyle-sexton's task in 1m 54s —— View job Code review complete (incremental, since
|
|
Claude finished @kyle-sexton's task in 2m 45s —— View job Security review complete
Scope: incremental review of what changed since
What changed and what I checked:
Dispatched an independent adversarial verifier ( No security findings to report. claude-lane-summary: lane=claude-security-review sha=501539e4b9e369a1f6ce207032afac5addd131af run=38404845165 |
…sts or failing checks remain (#7056) <!-- claude-projects-attribution: {"github_login":"kyle-sexton"} --> _Requested by **Kyle** · [project thread](https://claude.ai/code/project/chan_01FAwvkDrP9dbkyZDmMQiMav?thread=cmsg_01FAwvkDrP9dbkyZDmMQiMav3VhqGrPAKf4CfZbLHxPHqU)_ 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 #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 #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 #7036 (blocked: failing `ci-status`, `ci-lanes`), #6910 (blocked: pending checks), #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 - #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.com/claude-code) https://claude.ai/code/session_01L8KdG7fWg2YrErVYWtohmR --- _Generated by [Claude Code](https://claude.ai/code/session_01L8KdG7fWg2YrErVYWtohmR)_ --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: melodic-automation-lanes[bot] <337595936+melodic-automation-lanes[bot]@users.noreply.github.com>
2 participants
Requested by Kyle · project thread
Closes #6630
Summary
Before: the merge phase (
plugins/source-control/skills/pull-request/reference/merge.md) saidgh pr mergeon a merge-queue base "enqueues and returns". When it returned with aCLEANPR left out of the queue (seen on #6481 and #6426), or ran while GitHub still readmergeStateStatus: UNKNOWNand armed auto-merge instead (#6953), the 4.2.1 poll loop readOPEN falseand reported "PR left the merge queue unmerged", and the 4.2 path printed "not merged yet" with nothing confirming the PR was queued at all.After: 4.2 and 4.2.1 poll the merge state while it reads
UNKNOWNand merge or enqueue only onCLEAN. A new §4.2.2 "Confirm queue entry" runs after every merge call on a merge-queue base. A run ends in one of three reported states: merged, queued, or held with the blocker GitHub returned.Fix
skills/pull-request/reference/merge.md) states conditions in provider-neutral prose, and theghcommands live inreference/providers/github/pull-request/merge.md.HEAD_SHAfrom the PR'sheadRefOid. Every merge call in 4.2 (regular checkout, linked worktree, async API) and 4.2.1 pins to it with--match-head-commit/sha, so a push after approval fails the merge instead of landing unreviewed.mergeStateStatusat the pinned head while it readsUNKNOWN(15 s, 20 tries) and goes on only onCLEAN; any other state or a moved head is a hold. 4.2 runs it before any merge call; the 4.2.1 script runs the same loop in place of its singleCLEANread. REST fallback:.mergeable_statewhileunknown, go on only onclean.UNKNOWNand printsmerged,queued, orenqueue <node id>, or exits 1 with the hold (head moved, notCLEAN, with auto-merge-armed advice). Onenqueue, theenqueuePullRequestcall runs as its own command with the node id and head written as literal values, not a variable or$(jq ...), so the fail-closed guardrails merge guard in feat(guardrails): block a PR merge while review threads, change requests or failing checks remain #7056 can resolve the PR from the literalPR_...id. Then the read is re-run:queuedis the result;enqueueagain is a hold.jump. An armed auto-merge is reported, and on a hold the user is offeredgh pr merge <n> --disable-auto; the phase does not run it. This changes what the merge path reports, not who may merge.mergerow mentions the §4.2.2 step. Changelog fragment for source-control (patch).Verification
shellcheck -s bash -e SC2016: clean.ghrun of the confirm read and the wait loop, 10 cases, all as written:UNKNOWNthenCLEANprintsenqueue PR_x; queued; merged;BLOCKEDholds; head moved holds; auto-merge armed hold prints the--disable-autoadvice; stuckUNKNOWNholds after the tries; wait loopUNKNOWNthenCLEANpasses,BLOCKEDholds, moved head holds.MergeStateStatusvalues checked by GraphQL introspection (DIRTY UNKNOWN BLOCKED BEHIND UNSTABLE HAS_HOOKS CLEAN); RESTmergeable_stateread live on this PR (unknownjust after a push).scripts/affected-tests.sh --base origin/main --run: 4 shell suites passed; the 2 Python suites (test_guards.py,test_check_contract_clause_coverage.py) withuvx pytest: 48 passed.markdownlint-cli2on the 4 changed files: 0 issues.scripts/check-changelog-fragments.sh --check-required origin/main: pass.scripts/check-changed-skills.sh origin/main: pull-request PASS (3 pre-existing warnings).Left out: no live run against a real merge queue, and #7056's hook was not run against the enqueue command; the literal-id shape follows its documented rule.
Related
UNKNOWNarms auto-merge) is covered by this PR and is a duplicate of fix(source-control): pull-request merge must confirm the PR entered the merge queue #6630.babysit-prsalready reads queue state back inbabysit_merge.py; this brings the single-PR merge path in line.🤖 Generated with Claude Code
https://claude.ai/code/session_01WVW9hAwnSBrnFsR3kzXcB4