diff --git a/.changes/source-control/fix-6630-merge-confirm-queue-entry-4f16c791.md b/.changes/source-control/fix-6630-merge-confirm-queue-entry-4f16c791.md new file mode 100644 index 0000000000..6b9b8a7a9b --- /dev/null +++ b/.changes/source-control/fix-6630-merge-confirm-queue-entry-4f16c791.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- **`/source-control:pull-request merge` now confirms a merge-queue PR actually entered the queue.** Before any merge call it polls the PR's merge state while it reads `UNKNOWN` and merges or enqueues only on `CLEAN`, since a merge call made while the state is `UNKNOWN` arms auto-merge instead of enqueueing. After `gh pr merge` on a merge-queue base it reads `isInMergeQueue`; a `CLEAN` PR left out of the queue is enqueued once at the pinned head, through a separate call with the node id and head written as literals, and a refusal is reported with GitHub's error text verbatim, so a run ends merged, queued, or held with a named blocker. The head is now captured once in the pre-merge stale-base check and every merge call pins to it, and a hold with auto-merge armed warns that a later push could merge on `ci-status` alone and offers `gh pr merge --disable-auto`. diff --git a/plugins/source-control/reference/providers/github/pull-request/merge.md b/plugins/source-control/reference/providers/github/pull-request/merge.md index 39a872b6f8..4c96d7822e 100644 --- a/plugins/source-control/reference/providers/github/pull-request/merge.md +++ b/plugins/source-control/reference/providers/github/pull-request/merge.md @@ -18,17 +18,48 @@ gh api --paginate "repos/{owner}/{repo}/issues//comments?per_page=100 ## Stale-base guard +Read the head once and use that value for both the compare and `HEAD_SHA`, so the pinned head is +the one the compare checked: + ```bash -gh pr view --json mergeStateStatus,baseRefName,headRefOid -gh api repos/{owner}/{repo}/compare/... +read -r MSS BASE HEAD_SHA <<<"$(gh pr view --json mergeStateStatus,baseRefName,headRefOid \ + --jq '"\(.mergeStateStatus) \(.baseRefName) \(.headRefOid)"')" +echo "mergeStateStatus: $MSS" +gh api "repos/{owner}/{repo}/compare/$BASE...$HEAD_SHA" ``` +When GraphQL is refused, read `.base.ref` and `.head.sha` from one +`gh api "repos/{owner}/{repo}/pulls/"` call instead. + Update a behind branch with a merge-forward or `gh pr update-branch`. +## Wait for a known merge state + +GitHub reports `mergeStateStatus` as `UNKNOWN` while it recomputes it after a push or a ready flip, +and `gh pr merge` on a merge-queue base run then arms auto-merge instead of enqueueing. Poll at the +pinned head while it reads `UNKNOWN`; go on only on `CLEAN`: + +```bash +: "${HEAD_SHA:?set HEAD_SHA in 4.1 step 2}" +for _ in $(seq 20); do + S=$(gh pr view --json headRefOid,mergeStateStatus \ + --jq "if .headRefOid == \"$HEAD_SHA\" then .mergeStateStatus else \"HEAD_MOVED\" end") + [ "$S" = UNKNOWN ] || break + sleep 15 +done +[ "$S" = CLEAN ] || { echo "PR reads ${S:-nothing} at the pinned head, not CLEAN: hold" >&2; exit 1; } +``` + +When GraphQL is refused, poll `.mergeable_state` from `gh api "repos/{owner}/{repo}/pulls/"` +(after checking `.head.sha` equals `HEAD_SHA`) while it reads `unknown`, and go on only on `clean`. +`mergeStateStatus` values: +[MergeStateStatus enum](https://docs.github.com/graphql/reference/enums#mergestatestatus), as of +2026-10-13; recheck when that enum changes. + ## Squash merge in a regular checkout ```bash -gh pr merge --squash --delete-branch +gh pr merge --squash --delete-branch --match-head-commit "$HEAD_SHA" ``` ## Squash merge in a linked worktree @@ -37,7 +68,7 @@ Omit `--delete-branch`: on older gh it tries to check out the default branch loc while another worktree holds it, and exits 1 even though the merge succeeded. ```bash -gh pr merge --squash && { +gh pr merge --squash --match-head-commit "$HEAD_SHA" && { if [ "$(gh pr view --json state -q .state)" = MERGED ]; then REMOTE=$(bash "/scripts/resolve-remote.sh" --push '') && git push "$REMOTE" --delete --end-of-options '' else @@ -58,7 +89,7 @@ for it yet, so call it with `gh api`, pinned to the head verified before merge a `bypass_rules` true: ```bash -HEAD_SHA=$(gh api "repos/{owner}/{repo}/pulls/" --jq .head.sha) +: "${HEAD_SHA:?set HEAD_SHA in 4.1 step 2}" gh api -X PUT "repos/{owner}/{repo}/pulls//merge-async" \ -f sha="$HEAD_SHA" -f merge_action=direct_merge -f merge_method=squash -F bypass_rules=false ``` @@ -76,7 +107,7 @@ status. ```bash [ "$(gh repo view --json owner --jq .owner.login)" = melodic-software ] \ || { echo 'not a melodic-software repository: wait for the user' >&2; exit 1; } -HEAD_SHA=$(gh api "repos/{owner}/{repo}/pulls/" --jq .head.sha) +: "${HEAD_SHA:?set HEAD_SHA in 4.1 step 2}" RULES=$(gh api --paginate --slurp "repos/{owner}/{repo}/rules/branches/" | jq 'add') jq -e 'any(.[]; .type == "required_status_checks" and any(.parameters.required_status_checks[]; .context == "ci-status"))' <<<"$RULES" >/dev/null \ @@ -90,9 +121,13 @@ jq -e '[.[].check_runs[]] as $r and lane(["pr-review-security / security-review", "security-review / security-review", "claude-security-review-status"])' <<<"$RUNS" >/dev/null \ || { echo 'AI review not complete on the pinned head: hold' >&2; exit 1; } -[ "$(gh pr view --json headRefOid,mergeStateStatus \ - --jq "select(.headRefOid == \"$HEAD_SHA\") | .mergeStateStatus")" = CLEAN ] \ - || { echo 'PR not CLEAN at the pinned head: hold' >&2; exit 1; } +for _ in $(seq 20); do + S=$(gh pr view --json headRefOid,mergeStateStatus \ + --jq "if .headRefOid == \"$HEAD_SHA\" then .mergeStateStatus else \"HEAD_MOVED\" end") + [ "$S" = UNKNOWN ] || break + sleep 15 +done +[ "$S" = CLEAN ] || { echo "PR reads ${S:-nothing} at the pinned head, not CLEAN: hold" >&2; exit 1; } if jq -e 'any(.[]; .type == "merge_queue")' <<<"$RULES" >/dev/null; then gh pr merge --match-head-commit "$HEAD_SHA" else @@ -100,8 +135,9 @@ else fi ``` -Behind a merge queue `gh pr merge` enqueues and returns; had required checks not passed it would arm -auto-merge instead, which the `CLEAN` read rules out. +Behind a merge queue `gh pr merge` should enqueue and return; had required checks not passed it +would arm auto-merge instead. It has also been seen to return with a `CLEAN` PR left out of the +queue, so [confirm queue entry](#confirm-queue-entry) before the poll. ### Poll the merge queue @@ -126,6 +162,67 @@ done - **Recheck trigger**: a gh release that changes `gh pr merge` flags or merge-queue handling, or a rename of the AI review lanes' check names. +## Confirm queue entry + +Read the PR's queue state. The script prints `merged`, `queued` or `enqueue `, or exits 1 +with the hold: + +```bash +: "${HEAD_SHA:?set HEAD_SHA in 4.1 step 2}" +for _ in $(seq 20); do + PR=$(gh api graphql -F owner='{owner}' -F name='{repo}' -F number= -f query=' + query($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { pullRequest(number: $number) { + id state isInMergeQueue mergeStateStatus headRefOid autoMergeRequest { enabledAt } } } }' \ + --jq '.data.repository.pullRequest') || exit 1 + [ "$(jq -r .mergeStateStatus <<<"$PR")" = UNKNOWN ] || break + sleep 15 +done +AUTO=$(jq -r '.autoMergeRequest != null' <<<"$PR") +[ "$AUTO" = true ] && echo 'auto-merge is armed on this PR; tell the user (4.2.1 never arms it)' >&2 +hold() { + echo "$1" >&2 + [ "$AUTO" = true ] && echo 'auto-merge is still armed: a later push could merge on ci-status alone. +The user may disarm it with: gh pr merge --disable-auto' >&2 + exit 1 +} +case "$(jq -r '"\(.state) \(.isInMergeQueue) \(.mergeStateStatus)"' <<<"$PR")" in + MERGED*) echo 'merged' ;; + 'OPEN true '*) + [ "$(jq -r .headRefOid <<<"$PR")" = "$HEAD_SHA" ] \ + || hold 'queued at a head other than the one pinned in 4.1: hold and report it' + echo 'queued' ;; + 'OPEN false CLEAN') + [ "$(jq -r .headRefOid <<<"$PR")" = "$HEAD_SHA" ] \ + || hold 'head moved since 4.1: hold and re-run readiness' + echo "enqueue $(jq -r .id <<<"$PR")" ;; + *) hold "not queued and not CLEAN ($(jq -c '{state,isInMergeQueue,mergeStateStatus}' <<<"$PR")): report it" ;; +esac +``` + +On `enqueue `, run the mutation as its own command, with the printed node id and the +`HEAD_SHA` value written in as literals, never a variable or command substitution: the guardrails +merge guard reads the PR from a literal `PR_...` id and blocks a merge-queue mutation whose PR it +cannot resolve. Never pass `jump: true`. + +```bash +gh api graphql --silent -F id='' -F head='' -f query=' + mutation($id: ID!, $head: GitObjectID!) { + enqueuePullRequest(input: {pullRequestId: $id, expectedHeadOid: $head}) { + mergeQueueEntry { position } } }' +``` + +A non-zero exit is the refusal; its error text is the blocker. Then re-run the read: `queued` is +the result, and `enqueue` again means the mutation returned but the PR is not in the queue, a hold. + +- **Pointer**: a PR joins a queue through the merge button or auto-merge: + [managing a merge queue](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue); + the mutation and its `expectedHeadOid` input: + [enqueuePullRequest](https://docs.github.com/graphql/reference/mutations#enqueuepullrequest). +- **As of**: 2026-10-11 +- **Recheck trigger**: a gh release that changes how `gh pr merge` enqueues, or a change to + `enqueuePullRequest` or its input on that page. + ## Latest run on the default branch ```bash diff --git a/plugins/source-control/skills/pull-request/SKILL.md b/plugins/source-control/skills/pull-request/SKILL.md index 70b988de24..fb4f562a4f 100644 --- a/plugins/source-control/skills/pull-request/SKILL.md +++ b/plugins/source-control/skills/pull-request/SKILL.md @@ -69,7 +69,7 @@ For PR lifecycle runs spanning 3+ phases, copy `${CLAUDE_PLUGIN_ROOT}/skills/pul | `ready` | Phase 2.5 | Merge the base into the branch (never a rewrite of its history), run the code and security reviews over the PR diff in fresh-context subagents and the verify gate on the merged head, flip the draft to ready, then hand the PR to `/review:explain-change`, whose `digest_policy` decides whether a digest is offered or built (it gates nothing). Never arms auto-merge: ready starts the AI reviews. Refuses on a branch with no PR, and leaves a held or superseded draft in draft. See [reference/ready-for-review.md](reference/ready-for-review.md) | | `monitor` | Phase 3 | Watch CI, fix failures, evaluate comments. **Three-tier event delivery: (1) push channel** when your environment ships a forge-events channel (an MCP server delivering webhook events into the session), ~0 idle requests; **(2) Monitor tool** fallback (30s poll); **(3) plain polling** in cloud/headless sessions. Check the push channel FIRST per [monitor.md](reference/monitor.md) §3.0.05 before falling back | | `comments` | Phase 3.5 | Evaluate/respond to PR comments only | -| `merge` | Phase 4 | Squash merge, or merge or enqueue pinned to the reviewed head commit once the PR is `CLEAN` and the AI review checks have passed on that head, then wait for `MERGED` (no strategy flag behind a merge queue, never arm auto-merge; [reference/merge.md](reference/merge.md) §4.2.1) + worktree transition (reuse it, or release this session's lock) + verify. Never removes a worktree: that is `/source-control:worktree cleanup` | +| `merge` | Phase 4 | Squash merge, or merge or enqueue pinned to the reviewed head commit once the PR is `CLEAN` and the AI review checks have passed on that head, then confirm a merge-queue PR entered the queue (enqueue a `CLEAN` one left out, report the refusal text verbatim; §4.2.2) and wait for `MERGED` (no strategy flag behind a merge queue, never arm auto-merge; [reference/merge.md](reference/merge.md) §4.2.1) + worktree transition (reuse it, or release this session's lock) + verify. Never removes a worktree: that is `/source-control:worktree cleanup` | | `status` | Report only | Unified status across all phases | | `full` | Phase 1-4 | Run prep → create → monitor → merge end-to-end | | `fetch-logs [--raw\|--job ]` | CI log retrieval | Pull failed-CI evidence: default = `::error`/`::warning` annotations only (cheapest); `--raw` = full ZIP dump for archive review; `--job ` = per-job plain text | diff --git a/plugins/source-control/skills/pull-request/reference/merge.md b/plugins/source-control/skills/pull-request/reference/merge.md index 04c9f9ebe7..649c53e3a9 100644 --- a/plugins/source-control/skills/pull-request/reference/merge.md +++ b/plugins/source-control/skills/pull-request/reference/merge.md @@ -24,7 +24,9 @@ Re-check all check runs for any state changes, then re-check for new comments si 2. **Stale-base guard:** confirm the PR head is not behind its base on overlapping paths before squash-merging. Read the PR's `mergeStateStatus`, `baseRefName` and `headRefOid`, then compare `...` - ([GitHub](../../../reference/providers/github/pull-request/merge.md#stale-base-guard)). If + ([GitHub](../../../reference/providers/github/pull-request/merge.md#stale-base-guard)). Capture + that `headRefOid` as `HEAD_SHA` here: it is the head every later step in this phase pins to, so + a push after this point holds the merge instead of riding along. If `behind_by > 0`, update the branch (merge-forward, or the forge's update-branch) and re-run readiness; do **not** squash-merge a behind head. Under a non-strict ruleset, GitHub can still report `CLEAN` while the head is behind, and a stale-base squash can silently revert @@ -45,6 +47,12 @@ Re-check all check runs for any state changes, then re-check for new comments si Default merge mode is squash: one squashed commit per PR onto the default branch. Follow the consuming project's convention when it differs (merge commit / rebase-merge). +Every merge call in 4.2 pins to the `HEAD_SHA` captured in 4.1 step 2, so a push after approval +fails the merge instead of landing unreviewed. Before any merge call, poll the PR's merge state at +the pinned head while it reads `UNKNOWN`, and merge or enqueue only once it reads `CLEAN`; any +other state, or a moved head, is a hold to report +([GitHub](../../../reference/providers/github/pull-request/merge.md#wait-for-a-known-merge-state)). + In a regular checkout, squash-merge and delete the head branch in one call ([GitHub](../../../reference/providers/github/pull-request/merge.md#squash-merge-in-a-regular-checkout)). @@ -52,14 +60,18 @@ In a linked worktree (`git rev-parse --git-dir` differs from `git rev-parse --gi When the repo deletes head branches on merge, the push fails with "remote ref does not exist"; that is expected, and it is the only failure to ignore. 4.3 deletes the local branch. +On a merge-queue base, the CLI merge returning is not proof the PR is queued: confirm queue entry +per [4.2.2](#422-confirm-queue-entry), with the `HEAD_SHA` captured in 4.1 step 2, before +reporting the merge as done. + **Through the async merge API.** Use it instead of the CLI merge when the session refuses GraphQL (the CLI's merge and PR view run over GraphQL; this is REST) or when the PR is a stack layer -([stacks.md](stacks.md)). Pin it to the head you verified in 4.1, and never let it bypass rules -([GitHub](../../../reference/providers/github/pull-request/merge.md#async-merge-api)). For a merge -queue, request a queue action instead of a direct merge. The response carries a request id to poll -until it reads `merged`, `enqueued`, or `failed`. `enqueued` means queued, not merged: delete the -head branch only once the PR reads `MERGED`. GitHub checks only basic PR state when it accepts the -request and applies the base's rules when the merge runs, so 4.1 still comes first. +([stacks.md](stacks.md)). Pin it to the `HEAD_SHA` captured in 4.1 step 2, and never let it bypass +rules ([GitHub](../../../reference/providers/github/pull-request/merge.md#async-merge-api)). For a +merge queue, request a queue action instead of a direct merge. The response carries a request id to +poll until it reads `merged`, `enqueued`, or `failed`. `enqueued` means queued, not merged: delete +the head branch only once the PR reads `MERGED`. GitHub checks only basic PR state when it accepts +the request and applies the base's rules when the merge runs, so 4.1 still comes first. **Always use the explicit `` resolved at phase entry.** The PR title becomes the squash commit message. It is shaped to satisfy the resolved subject/title convention, per pull-request SKILL.md's "PR title format" ladder (Conventional Commits by default). @@ -88,18 +100,19 @@ after arming, a push by anyone with write access would merge the new head on `ci Run the merge script ([GitHub](../../../reference/providers/github/pull-request/merge.md#merge-after-ai-review)). It -stops with a hold message unless the repository owner is melodic-software, the base branch's live -rules require `ci-status`, every check run on the pinned head was read (the count matches the -reported total) and both AI review lanes, matched by the exact names above, completed with success, -and the PR reads `CLEAN` at the pinned head. Only then does it merge, pinned to that head with -`--match-head-commit`. +stops with a hold message unless `HEAD_SHA` was captured in 4.1 step 2, the repository owner is +melodic-software, the base branch's live rules require `ci-status`, every check run on the pinned +head was read (the count matches the reported total) and both AI review lanes, matched by the exact +names above, completed with success, and the PR reads `CLEAN` at the pinned head (it polls while +the merge state reads `UNKNOWN`, as in 4.2). Only then does it +merge, pinned to that head with `--match-head-commit`. Behind a merge queue (claude-code-plugins) the queue sets the strategy, so pass no strategy flag. -There the merge enqueues and returns; had required checks not passed it would arm auto-merge -instead, which the `CLEAN` read rules out. Never pass `--auto`, `--admin`, `--merge`/`-m` or -`--rebase`/`-r`. After enqueueing, poll the PR's state and merge-queue membership every 30 seconds -until it reads `MERGED` or leaves the queue unmerged (run the loop in the background or under a -monitor; [GitHub](../../../reference/providers/github/pull-request/merge.md#poll-the-merge-queue)). +Never pass `--auto`, +`--admin`, `--merge`/`-m` or `--rebase`/`-r`. Confirm queue entry per +[4.2.2](#422-confirm-queue-entry), then poll the PR's state and merge-queue membership every 30 +seconds until it reads `MERGED` or leaves the queue unmerged (run the loop in the background or under +a monitor; [GitHub](../../../reference/providers/github/pull-request/merge.md#poll-the-merge-queue)). A PR that leaves the queue unmerged is reported to the user, not re-enqueued, and 4.3 does not run. Delete the head branch per 4.2 only once the PR reads `MERGED`. @@ -108,6 +121,33 @@ The CLI's merge-queue behavior and its recheck trigger, which also fires on a re review lanes' check names, are recorded in the [GitHub provider reference](../../../reference/providers/github/pull-request/merge.md#poll-the-merge-queue). +### 4.2.2 Confirm queue entry + +Run this after every merge call on a merge-queue base, from 4.2 or 4.2.1. A merge run ends in one of +three states, and the report names which: merged, queued, or held with the blocker GitHub returned. +It changes what the run reports, not who may merge: it enqueues only a PR the user already approved +in 4.1 step 4 or that 4.2.1 cleared, and only at the head pinned there. + +Run the confirm read +([GitHub](../../../reference/providers/github/pull-request/merge.md#confirm-queue-entry)). It reads +the PR's state, queue membership, merge state, head and auto-merge request, polling while the merge +state reads `UNKNOWN`. Merged or queued is reported as such. Open, not queued and `CLEAN` at +`HEAD_SHA`: it prints the PR's node id to enqueue. Any other state, or a moved head, is a hold that +prints the state. An armed auto-merge is reported, since 4.2.1 never arms it. + +To enqueue, run the enqueue call once as its own command, with the printed node id and `HEAD_SHA` +written in as literal values rather than a variable or command substitution, pinned to that head, +then re-run the read. `queued` is the result; a second enqueue answer means the call returned but +the PR is not in the queue, which is a hold. + +A non-zero exit from the read or the enqueue call is a hold: report the printed state or GitHub's error text unedited (for example +"A conversation must be resolved" names the blocker), do not retry the enqueue, and 4.3 does not +run. One exception: when the error says the PR is already queued, the merge call won the race; +re-run the script once, and a `queued` read is the result. A hold with auto-merge armed also tells +the user that a later push could merge on `ci-status` alone (4.2.1's reason for never arming it) +and offers disarming auto-merge as their option; this phase does not disarm it. Never jump the +queue. Only `queued` moves on to the poll in 4.2.1; `merged` moves on to 4.3. + ## 4.3 Worktree transition and next-task setup This phase never removes a worktree; one left behind is a cleanup candidate for `/source-control:worktree cleanup`. diff --git a/plugins/source-control/skills/pull-request/reference/readiness.md b/plugins/source-control/skills/pull-request/reference/readiness.md index 2e4b3a0844..1e2d3e963a 100644 --- a/plugins/source-control/skills/pull-request/reference/readiness.md +++ b/plugins/source-control/skills/pull-request/reference/readiness.md @@ -74,7 +74,7 @@ List the PR's checks with name, state and bucket ([GitHub](../../../reference/pr **Gotcha: one name may show duplicate entries (`SUCCESS` check-run + stuck `PENDING` commit-status).** The checks list aggregates BOTH workflow check-runs AND external commit-statuses, so a workflow and an external app posting under the same name produce two rows: the workflow's check-run resolves cleanly, while the app's redundant commit status may never finalize and sits at `PENDING` indefinitely. When you see two rows for one name, one `pass|SUCCESS` with a `link` and one `pending|PENDING` with no link, treat the check-run as authoritative. Verify by listing every check run on the head with its name, status and conclusion, and finding the duplicated name's row in the output; the name never goes into the query ([GitHub](../../../reference/providers/github/pull-request/readiness.md#verify-a-duplicated-check-name)). -Match the name by reading the output, never by typing it into the command: a check name is chosen by whoever wrote the workflow (a fork PR names its own jobs), so inside a shell argument or a jq program it can run shell code or act as a regex. If its row reads `completed success`, the stuck commit-status is the redundant external bot. Classify it as non-blocking, document, and proceed. `mergeStateStatus=UNSTABLE` will reflect the stuck status but does NOT block merge when the repo's required checks are green. +Match the name by reading the output, never by typing it into the command: a check name is chosen by whoever wrote the workflow (a fork PR names its own jobs), so inside a shell argument or a jq program it can run shell code or act as a regex. If its row reads `completed success`, the stuck commit-status is the redundant external bot. Classify it as non-blocking, document, and proceed. `mergeStateStatus=UNSTABLE` will reflect the stuck status but does NOT block merge when the repo's required checks are green on GitHub's side. The merge phase still merges or enqueues only on `CLEAN` ([merge.md](merge.md) §4.2), so a PR held at `UNSTABLE` by such a status goes to the user. The pagination is not optional and the completeness assertion is not hygiene. See [Reading GitHub list APIs](#reading-github-list-apis).