diff --git a/plugins/source-control/.claude-plugin/plugin.json b/plugins/source-control/.claude-plugin/plugin.json index 41c55c73ea..a2867e5976 100644 --- a/plugins/source-control/.claude-plugin/plugin.json +++ b/plugins/source-control/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "source-control", - "version": "0.79.6", + "version": "0.79.7", "description": "Git and GitHub delivery: /commit (convention-checked subject, Co-authored-by trailer, surgical staging), /pull-request (prep, create, CI monitoring, review triage, merge, CI logs), /babysit-prs (safe-by-default PR fleet loop, opt-in worker and autopilot tiers), /babysit-loop (merge lane; merge is human until the repo adopts it), /worktree, /resolve-conflicts (intent-first), /check, and /setup (layered source-control.md convention config; Conventional Commits by default).", "author": { "name": "Melodic Software", diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index 6a9c5a8c51..4f2d5a6ee1 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -3,6 +3,13 @@ All notable changes to the `source-control` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.79.7] - 2026-10-04 + +### Fixed + +- **The babysit merge gate reports a PR GitHub put in a merge queue as queued, and confirms it on later runs ([#5953](https://github.com/melodic-software/claude-code-plugins/issues/5953)).** + `gh pr merge` adds a PR to the queue on any base that has one, `--auto` or not, and exits 0, so a queue the branch-rules read missed was reported as `autoMergeEnabled` or `merged`. After every successful `gh pr merge` the gate now reads the PR's queue state back and reports `action: enqueue`, `enqueued: true`, and `mergeQueue` with the entry's state and position. With `--state-dir`, an enqueue from either the async API or `gh pr merge` is recorded, and every later run reports it still queued (merge pending, nothing sent), merged at the vetted head, or `dequeued` when it left the queue without merging. A queue base whose read-back shows the PR neither queued, armed, nor merged is re-read briefly, then reported `action: merge-pending` with `ready: false` and `mergeQueue.unconfirmed: true` and recorded, so later runs confirm it without sending another merge, and the second later run that still sees nothing reports it `dequeued`. A base without a merge queue reports as before. + ## [0.79.6] - 2026-10-04 ### Fixed diff --git a/plugins/source-control/skills/babysit-prs/SKILL.md b/plugins/source-control/skills/babysit-prs/SKILL.md index 76b2403178..3001b8e481 100644 --- a/plugins/source-control/skills/babysit-prs/SKILL.md +++ b/plugins/source-control/skills/babysit-prs/SKILL.md @@ -409,7 +409,7 @@ repo#number (@author) | checks | action | open items Material findings: fixes committed or pushed; new failing or pending required checks; new blocking bot feedback; new ordinary human comments (one notification per stable comment ID, -never an automatic reply); PRs merged, enqueued in a merge queue (`action: enqueue`, `enqueued: true`, not merged until a later cycle reads it merged), a merge still pending on GitHub (`action: merge-pending`, may still land), or armed for auto-merge (`action: auto-merge`, still open, stays queued); checks held for approval (escalate them); a PR the host runtime's permission layer left "ready, +never an automatic reply); PRs merged, enqueued in a merge queue (`action: enqueue`, `enqueued: true`, with its `mergeQueue.position`, not merged until a later cycle reads it merged), dequeued (`dequeued: true`, left the queue unmerged), a merge still pending on GitHub (`action: merge-pending`, may still land), or armed for auto-merge (`action: auto-merge`, still open, stays queued); checks held for approval (escalate them); a PR the host runtime's permission layer left "ready, awaiting human execution" with its exact pinned command ([reference/safety.md](reference/safety.md)); escalations that need a user decision; and suspicious state changes such as missing permissions, changed branch protection, merge diff --git a/plugins/source-control/skills/babysit-prs/reference/orchestration.md b/plugins/source-control/skills/babysit-prs/reference/orchestration.md index d0b2ba4769..7444ef805c 100644 --- a/plugins/source-control/skills/babysit-prs/reference/orchestration.md +++ b/plugins/source-control/skills/babysit-prs/reference/orchestration.md @@ -405,10 +405,13 @@ same-worktree protections. or no subagent tools to dispatch to: leave the thread unresolved, do not merge, and report the PR with the addressed-but-unresolvable thread named. Never resolve past a refusal, and never reach around the wrapper. -- A merge the gate reported `"action": "enqueue"` (`enqueued: true`) is queued, not merged. Keep - the PR and its worktree, and let a later cycle read its state: `MERGED` ends it like any merge; a - PR still open and out of the queue goes back through the gate. A merge reported - `status: pending`, or a later run reporting `"action": "merge-pending"`, is still live on GitHub +- A merge the gate reported `"action": "enqueue"` (`enqueued: true`) is queued, not merged, + whether the async API or `gh pr merge` put it there. Keep the PR and its worktree, and let a + later cycle's gate run read its queue entry: still queued (`mergeQueue.position`) is "queued, + may still land"; merged ends it like any merge; `dequeued: true` is reported as dequeued, and the + next cycle gates it again. A merge reported + `status: pending`, or any run reporting `"action": "merge-pending"` (`mergeQueue.unconfirmed: + true` included: a `gh pr merge` the queue has not shown yet), is still live on GitHub and may land whatever the gate now says: report it as "merge may still land", keep the PR, and let the next cycle read it again. `mergeUnconfirmed: true` or `stackVerification.verified` other than `true` goes to a human (`safety.md`, §Async Merge Path). diff --git a/plugins/source-control/skills/babysit-prs/reference/safety.md b/plugins/source-control/skills/babysit-prs/reference/safety.md index e63b6eeae8..d810c30318 100644 --- a/plugins/source-control/skills/babysit-prs/reference/safety.md +++ b/plugins/source-control/skills/babysit-prs/reference/safety.md @@ -706,10 +706,31 @@ it): a `PUT` to the PR's `merge-async` endpoint, then a `GET` on the request's U every merge form. - **Merge queue.** A default-branch base that requires a merge queue is no longer a blocker. Once every other condition holds, the merge enqueues (`action: enqueue`). `enqueued` is final for the - request and is not a merge: a later cycle reads the PR as merged, or finds it back out of the - queue and gates it again. Re-running the merge on a queued PR returns `enqueued` without a new - request. Enqueueing is a merge, so only a tier that may merge enqueues, and auto-merge is never - armed over a queue. A queue on any other base keeps the hold. + request and is not a merge. Enqueueing is a merge, so only a tier that may merge enqueues, and + auto-merge is never armed over a queue. A queue on any other base keeps the hold. +- **A `gh pr merge` the queue took.** `gh pr merge` adds a PR to the queue on any base that has + one, `--auto` or not, and exits `0`, so a queue the branch-rules read did not report would read + as a merge or an arm. After every successful `gh pr merge` the gate reads the PR's queue state + back over GraphQL. In the queue, it reports `action: enqueue`, `enqueued: true`, + `autoMergeEnabled: false`, `merged: false`, and `mergeQueue` with the entry's `state` and + `position`. Not yet in the queue but armed to enter it, it stays `action: auto-merge` with + `mergeQueue.entersWhenReady: true`. Neither queued, armed, nor merged, the read may simply + trail the merge, so the gate re-reads it a few times at the async poll interval, well inside + that path's 60-second bound. Still unseen, it reports `action: merge-pending`, `ready: false`, + `mergeQueue.unconfirmed: true`, and exit `10`, and records the success under `--state-dir` as + an unconfirmed queue entry. A base without a queue keeps the report it had; a failed read keeps + it too and names the failure in `merge.queueReadError`. +- **A queued PR is confirmed on later runs.** With `--state-dir`, an enqueue from either path is + recorded with the vetted head, and every later run reads the entry first. Still in the queue: the + run reports `action: merge-pending`, `enqueued: true`, and `mergeQueue`, with the queue hold first + in `blockers`, exit `10`, and sends nothing. Merged: the head is checked as for a pending + request, and a match reports `merged: true` from `pendingMergeRequest`, exit `0`. Out of the + queue unmerged: the run reports `dequeued: true` with that hold first in `blockers`, exit `10`, + clears the record, and the next run gates it again. Armed to enter the queue, it holds as merge + pending with `mergeQueue.entersWhenReady: true`. An unreadable queue keeps the record and holds + as merge pending. An unconfirmed entry that reads back queued, armed, or merged is confirmed and + reported that way; one still unseen holds as merge pending with `mergeQueue.unconfirmed: true`, + sending nothing, and the second later run that finds it unseen reports it `dequeued`. - **Stacks (`--stacked-prs`).** Only a native stack qualifies: the PR's REST `stack` object. A PR merely based on another PR's branch keeps the non-default-base hold. The layer is judged against the stack's trunk, every open layer below it runs the same gate pinned to the head the stack @@ -738,6 +759,17 @@ Recheck when a `gh` release adds an async-merge command, when either REST page c field, or when stacked pull requests leave public preview or GitHub announces merge-queue support for stacks. +**Claim, basis, as of, recheck:** that `gh pr merge` adds a PR to a required merge queue, or +enables auto-merge until its checks pass, and exits `0` for a PR already queued, +[merging a pull request with a merge queue](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/merging-a-pull-request-with-a-merge-queue), +the [`gh pr merge` manual](https://cli.github.com/manual/gh_pr_merge), and +[`pkg/cmd/pr/merge/merge.go`](https://github.com/cli/cli/blob/trunk/pkg/cmd/pr/merge/merge.go); +the queue fields read back (`isInMergeQueue`, `isMergeQueueEnabled`, `mergeQueueEntry`, +`autoMergeRequest`), the +[`PullRequest` GraphQL object](https://docs.github.com/en/graphql/reference/objects#pullrequest); +why a PR leaves the queue, the first page's removal section; 2026-10-04. Recheck when a `gh` +release changes how `gh pr merge` treats a queue base, or the GraphQL schema changes a queue field. + ### Lane-pinned merge authorization: report, don't re-pin A single-PR merge-capable invocation dispatched by `source-control:babysit-loop`'s rung partition, @@ -790,7 +822,9 @@ auto-merge enabled earlier could merge before AI review posts. A fully ready PR the same run, through §Async Merge Path. A base that requires a merge queue, and a stack layer, are never armed: they wait until fully ready and then enqueue or land. The gate's JSON reports `autoMerge.ready` and `autoMerge.blockers`; a successful arm exits `0` with `"action": "auto-merge"`, `autoMergeEnabled: true` and `merged: false`, so it -is reported as armed, not merged, and the PR stays in the queue with its worktree kept. Nothing +is reported as armed, not merged, and the PR stays in the queue with its worktree kept. An arm +GitHub put straight into a merge queue the rules read missed reports `"action": "enqueue"` +instead (§Async Merge Path, "A `gh pr merge` the queue took"). Nothing else enables auto-merge: not a Worker Contract subagent (`orchestration.md`), not a work-items worker lane, not a standalone invocation, not `/source-control:pull-request`. The `worker` tier name is unrelated: a lane-pinned invocation at that tier is the merge lane. diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py index 3ff76146ab..7e1f0fe582 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py @@ -21,10 +21,15 @@ without the endpoint (404) falls back to `gh pr merge` for a direct merge. Under `--stacked-prs` every stack member, the bottom layer included, merges through the async API too (GitHub's required API for a stacked PR) and never - falls back. Any other base, and every `--auto` arm, keeps `gh pr merge`. A - request still pending at the poll bound is recorded under `--state-dir` - (GitHub offers no cancel), and every later run reports it as merge pending - until it finishes. + falls back. Any other base, and every `--auto` arm, keeps `gh pr merge`, + which GitHub routes into a merge queue on any base that has one: its result + is read back and a PR found in the queue is reported enqueued, with its + position. A request still pending at the poll bound is recorded under + `--state-dir` (GitHub offers no cancel), and every later run reports it as + merge pending until it finishes. A PR put in the queue is recorded the same + way, and every later run reports it queued until it merges or leaves the + queue (`dequeued`). A queue base that shows no result yet is recorded + unconfirmed and held as merge pending until a later run reads one. - With `--stacked-prs`, a native stack layer is judged against the stack's trunk and every open layer below it runs the same gate, since the async merge lands them together. Without it a stack layer is held as before. @@ -191,6 +196,14 @@ def is_ai_review_check(check_name: str, names: tuple[str, ...]) -> bool: ASYNC_MERGE_POLL_INTERVAL_SECONDS = 3.0 ASYNC_TERMINAL_STATUSES = frozenset({"merged", "enqueued", "failed"}) ASYNC_UUID_RE = re.compile(r"[0-9A-Za-z-]{1,64}") +# A `gh pr merge` the queue base took can read back as neither queued, armed, +# nor merged for a moment. The run re-reads it at the poll interval, well inside +# the async poll bound the same run already accepts. A result still unseen is +# recorded unconfirmed. Later runs, at least one loop wake apart, hold it while +# it stays unseen, and the QUEUE_UNCONFIRMED_LATER_READS-th such run calls it +# dequeued. +QUEUE_READ_BACK_ATTEMPTS = 4 +QUEUE_UNCONFIRMED_LATER_READS = 2 def _poll_sleep(seconds: float) -> None: @@ -1686,6 +1699,136 @@ def pull_request_landed(repo: str, number: int) -> dict[str, Any]: return {"merged": merged, "head": head} +MERGE_QUEUE_QUERY = ( + "query($o:String!,$r:String!,$n:Int!){repository(owner:$o,name:$r){" + "pullRequest(number:$n){merged isInMergeQueue isMergeQueueEnabled " + "mergeQueueEntry{state position} autoMergeRequest{enabledAt}}}}" +) + + +def read_merge_queue(repo: str, number: int) -> dict[str, Any]: + """The PR's merge-queue standing as GitHub reports it now. + + GraphQL-only: REST exposes neither queue membership nor position. Any + failure, the session's GraphQL refusal included, sets `readError`, and the + caller reports the queue standing as unconfirmed rather than absent. + """ + owner, name = repo.split("/", 1) + try: + data = gh_json( + [ + "api", + "graphql", + "-f", + f"query={MERGE_QUEUE_QUERY}", + "-F", + f"o={owner}", + "-F", + f"r={name}", + "-F", + f"n={number}", + ] + ) + except (RuntimeError, json.JSONDecodeError) as exc: + return {"readError": f"could not read the merge queue: {exc}"} + pr = json_object( + json_object(json_object(json_object(data).get("data")).get("repository")).get( + "pullRequest" + ) + ) + flags = (pr.get("merged"), pr.get("isInMergeQueue"), pr.get("isMergeQueueEnabled")) + if not all(isinstance(flag, bool) for flag in flags): + return {"readError": "the merge queue read returned no pull request state"} + entry = pr.get("mergeQueueEntry") + entry = entry if is_json_object(entry) else None + return { + "readError": None, + "merged": pr["merged"], + "inQueue": pr["isInMergeQueue"] or entry is not None, + "queueRequired": pr["isMergeQueueEnabled"], + "autoMergeArmed": is_json_object(pr.get("autoMergeRequest")), + "state": entry.get("state") if entry else None, + "position": entry.get("position") if entry else None, + } + + +def queue_result_unseen(queue: dict[str, Any]) -> bool: + """A readable queue base on which the PR is neither queued, armed, nor merged.""" + return ( + not queue["readError"] + and queue["queueRequired"] + and not (queue["inQueue"] or queue["autoMergeArmed"] or queue["merged"]) + ) + + +def read_merge_queue_after_merge( + repo: str, + number: int, + *, + attempts: int = QUEUE_READ_BACK_ATTEMPTS, + interval_seconds: float = ASYNC_MERGE_POLL_INTERVAL_SECONDS, + sleep: Callable[[float], None] | None = None, +) -> dict[str, Any]: + """The queue standing after a successful `gh pr merge`, re-read while the + result is unseen. A failed re-read keeps the unseen read it followed, so a + transient error never turns an unseen result into a merge.""" + sleep = sleep or _poll_sleep + queue = read_merge_queue(repo, number) + for _ in range(attempts - 1): + if not queue_result_unseen(queue): + break + sleep(interval_seconds) + again = read_merge_queue(repo, number) + if not again["readError"]: + queue = again + return queue + + +def report_queue_routing(result: dict[str, Any], queue: dict[str, Any]) -> int: + """Correct a successful `gh pr merge` report for a base with a merge queue, + returning the exit code. + + `gh pr merge` routes every merge on a queue base into the queue, `--auto` + or not, and exits 0 either way; a queue the rules read did not report is + therefore seen only here. A base without a queue keeps the report as it was. + A queue base that shows no result yet reports `merge-pending` with + `mergeQueue.unconfirmed`, since the request may still be taking effect. + """ + if queue["readError"]: + result["merge"]["queueReadError"] = queue["readError"] + return 0 + if queue["inQueue"]: + result["action"] = "enqueue" + result["merged"] = result["autoMergeEnabled"] = False + result["enqueued"] = True + result["mergeQueue"] = {"state": queue["state"], "position": queue["position"]} + return 0 + if not queue["queueRequired"] or queue["merged"]: + return 0 + result["merged"] = False + result["autoMergeEnabled"] = queue["autoMergeArmed"] + result["mergeQueue"] = { + "state": None, + "position": None, + "entersWhenReady": queue["autoMergeArmed"], + } + if queue["autoMergeArmed"]: + result["action"] = "auto-merge" + return 0 + hold = ( + "gh pr merge succeeded on a merge-queue base, but the pull request does not " + "yet read back as queued, armed to enter the queue, or merged -- merge " + "pending, unconfirmed; it may still land; no new request is sent" + ) + result["action"] = "merge-pending" + result["ready"] = False + result["mergeQueue"]["unconfirmed"] = True + result.setdefault("blockers", []).insert(0, hold) + result["autoMerge"] = {"ready": False, "blockers": [hold]} + result["merge"]["message"] = hold + return 10 + + def async_merge( repo: str, number: int, @@ -1931,6 +2074,8 @@ def check_pending_request(repo: str, number: int, path: Path) -> dict[str, Any] entry = _load_pending(path).get(key) if not is_json_object(entry): return None + if entry.get("queued"): + return check_queued_entry(repo, number, path, entry) current = read_async_merge(repo, number, str(entry.get("uuid") or "")) report = { **entry, @@ -1950,6 +2095,78 @@ def check_pending_request(repo: str, number: int, path: Path) -> dict[str, Any] return report +def check_queued_entry( + repo: str, number: int, path: Path, entry: dict[str, Any] +) -> dict[str, Any]: + """A recorded merge-queue entry as GitHub reports it now. + + `queued` while the PR is still in the queue, `armed` while auto-merge will + put it there, `merged` once it landed (checked against the recorded head), + and `dequeued` when it left the queue without merging. An entry recorded + `unconfirmed` (the queue showed no result yet) that reads back queued or + armed is confirmed; one still showing nothing reads `unconfirmed` for + `QUEUE_UNCONFIRMED_LATER_READS` runs and then `dequeued`. An unreadable + queue keeps the record with status None; a merge whose head cannot be read + back keeps it with status `merged` and `verified` None, and + `queued_entry_hold` holds a merged entry on `verified`. + """ + key = f"{repo}#{number}" + queue = read_merge_queue(repo, number) + report: dict[str, Any] = {**entry, "status": None, "message": ""} + if queue["readError"]: + report["message"] = queue["readError"] + return report + if queue["inQueue"] or (queue["autoMergeArmed"] and not queue["merged"]): + if queue["inQueue"]: + report["status"] = "queued" + report["mergeQueue"] = { + "state": queue["state"], + "position": queue["position"], + } + else: + report["status"] = "armed" + report["mergeQueue"] = { + "state": None, + "position": None, + "entersWhenReady": True, + } + if entry.get("unconfirmed"): + confirmed = { + field: value + for field, value in entry.items() + if field not in ("unconfirmed", "unseenReads") + } + update_pending(path, key, confirmed) + return report + if queue["merged"]: + report["status"] = "merged" + report["verification"] = verify_request_landed(repo, number, entry) + if report["verification"]["verified"] is None: + return report + elif entry.get("unconfirmed"): + unseen = _unseen_reads(entry) + 1 + report["unseenReads"] = unseen + if unseen < QUEUE_UNCONFIRMED_LATER_READS: + report["status"] = "unconfirmed" + report["mergeQueue"] = { + "state": None, + "position": None, + "unconfirmed": True, + } + update_pending(path, key, {**entry, "unseenReads": unseen}) + return report + report["status"] = "dequeued" + else: + report["status"] = "dequeued" + update_pending(path, key, None) + return report + + +def _unseen_reads(entry: dict[str, Any]) -> int: + count = entry.get("unseenReads") + return count if isinstance(count, int) and not isinstance(count, bool) else 0 + + def _record_pending( path: Path, repo: str, @@ -1958,13 +2175,16 @@ def _record_pending( pin: str, result: dict[str, Any], ) -> None: - """Keep a request that is still live on GitHub; forget a finished one.""" + """Keep a request that is still live on GitHub, or a PR it put in the merge + queue; forget a finished one.""" key = f"{repo}#{number}" live = ( bool(record.get("uuid")) and record.get("status") not in ASYNC_TERMINAL_STATUSES ) entry: dict[str, Any] | None = None - if live: + if record.get("success") and record.get("status") == "enqueued": + entry = _queued_entry(pin) + elif live: entry = { "uuid": record["uuid"], "head": pin, @@ -1981,6 +2201,25 @@ def _record_pending( for layer, head in _evaluated_layers(result) ], } + _write_pending(path, key, entry, result) + + +def _queued_entry(pin: str, *, unconfirmed: bool = False) -> dict[str, Any]: + entry: dict[str, Any] = { + "queued": True, + "head": pin, + "mergeAction": "merge_queue", + "requestedAt": datetime.now(UTC).isoformat().replace("+00:00", "Z"), + } + if unconfirmed: + entry["unconfirmed"] = True + entry["unseenReads"] = 0 + return entry + + +def _write_pending( + path: Path, key: str, entry: dict[str, Any] | None, result: dict[str, Any] +) -> None: try: update_pending(path, key, entry) except (RuntimeError, OSError) as exc: @@ -1990,6 +2229,75 @@ def _record_pending( ) +def _queue_position(queue: dict[str, Any]) -> str: + position = queue.get("position") + where = "position unknown" if position is None else f"position {position}" + return f"{where}, state {queue.get('state') or 'unknown'}" + + +def queued_entry_hold( + prior: dict[str, Any], verified: Any +) -> tuple[str | None, str | None]: + """`(hold, reason)` for a recorded merge-queue entry. A hold with no reason + is a merge that may still land; `(None, None)` is a confirmed merge.""" + status = prior.get("status") + since = prior.get("requestedAt") + if status == "queued": + queue = json_object(prior.get("mergeQueue")) + return ( + f"in the merge queue ({_queue_position(queue)}) since {since} -- " + "queued, not merged; it may still land; no new request is sent", + None, + ) + if status == "armed": + return ( + f"armed since {since} to enter the merge queue when its requirements " + "pass -- not merged; it may still land; no new request is sent", + None, + ) + if status == "unconfirmed": + return ( + f"gh pr merge succeeded at {since} on a merge-queue base, and the pull " + f"request still reads back as neither queued, armed, nor merged (later " + f"read {prior.get('unseenReads')} of {QUEUE_UNCONFIRMED_LATER_READS}) -- " + "merge pending, unconfirmed; it may still land; no new request is sent", + None, + ) + if status == "dequeued" and prior.get("unconfirmed"): + unseen = ( + f"gh pr merge succeeded at {since} on a merge-queue base, but the pull " + f"request read back as neither queued, armed, nor merged on " + f"{prior.get('unseenReads')} later runs -- dequeued, not merged; the " + "record is cleared and the next run gates it again" + ) + return unseen, unseen + if status is None: + return ( + f"the merge-queue entry recorded at {since} could not be read " + f"({prior.get('message')}) -- merge pending; it may still land; no new " + "request is sent", + None, + ) + if status == "dequeued": + dequeued = ( + f"left the merge queue without merging (queued at {since}): removed by " + "hand or through the API, a failed or timed-out queue check, or a " + "requirement it no longer meets -- dequeued, not merged; the record is " + "cleared and the next run gates it again" + ) + return dequeued, dequeued + if verified is not True: + unconfirmed = "the merge-queue entry merged, but " + ( + "the head it landed could not be read back -- unconfirmed; the record " + "is kept and re-checked next run" + if verified is None + else "the head it landed is not the head the gate evaluated -- " + "escalate to a human" + ) + return unconfirmed, unconfirmed + return None, None + + def build_settle(logins: Iterable[str], minutes: str) -> ReviewSettleConfig | None: """The review-settle hold, or None when it would be inert. @@ -2394,7 +2702,14 @@ def _refuse(message: str, code: int, **envelope: object) -> int: result["pendingMergeRequest"] = prior verified = json_object(prior.get("verification")).get("verified", True) hold = reason = None - if prior.get("corrupt"): + if prior.get("queued"): + hold, reason = queued_entry_hold(prior, verified) + if prior.get("status") in ("queued", "armed", "unconfirmed"): + result["enqueued"] = prior.get("status") == "queued" + result["mergeQueue"] = prior.get("mergeQueue") + elif prior.get("status") == "dequeued": + result["dequeued"] = True + elif prior.get("corrupt"): hold = ( f"the recorded async merge request for this PR is corrupt " f"({prior.get('message')}) -- merge pending until a human " @@ -2577,8 +2892,25 @@ def _refuse(message: str, code: int, **envelope: object) -> int: } result["merged"] = proc.returncode == 0 and not arm_auto result["autoMergeEnabled"] = proc.returncode == 0 and arm_auto + exit_code = 0 if proc.returncode == 0 else 10 + if proc.returncode == 0: + queue = read_merge_queue_after_merge(repo, number) + exit_code = report_queue_routing(result, queue) + unconfirmed = bool(json_object(result.get("mergeQueue")).get("unconfirmed")) + if pending_path is not None and (result.get("enqueued") or unconfirmed): + _write_pending( + pending_path, + f"{repo}#{number}", + _queued_entry(pin, unconfirmed=unconfirmed), + result, + ) + elif unconfirmed: + result["merge"]["message"] += ( + "; without --state-dir nothing is recorded, so a later run cannot " + "confirm it" + ) print(json.dumps(result, indent=2)) - return 0 if proc.returncode == 0 else 10 + return exit_code if __name__ == "__main__": diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py index 2c62155184..cc29842828 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py @@ -39,6 +39,26 @@ LINKED_REF = f"owner/repo#{LINKED_ISSUE}" +def _queue_read( + *, + in_queue: bool = False, + required: bool = False, + armed: bool = False, + state: str | None = None, + position: int | None = None, +) -> dict[str, Any]: + """A `read_merge_queue` answer; the default is a base with no merge queue.""" + return { + "readError": None, + "merged": False, + "inQueue": in_queue, + "queueRequired": required, + "autoMergeArmed": armed, + "state": state, + "position": position, + } + + def _comment( login: str, body: str = "", @@ -1384,7 +1404,11 @@ def test_unresolved_thread_holds(self) -> None: self.assertFalse(result["autoMerge"]["ready"]) def _main( - self, auto_ready: bool, *extra: str, ready: bool = False + self, + auto_ready: bool, + *extra: str, + ready: bool = False, + queue: dict[str, Any] | None = None, ) -> tuple[int, list[list[str]]]: result = { "ready": ready, @@ -1415,12 +1439,93 @@ def capture(cmd: list[str]) -> Any: "pull_request_landed", return_value={"merged": True, "head": HEAD}, ), + mock.patch.object( + merge, "read_merge_queue", return_value=queue or _queue_read() + ), + mock.patch.object(merge, "_poll_sleep"), contextlib.redirect_stdout(io.StringIO()) as out, ): code = merge.main() self.output = json.loads(out.getvalue()) if out.getvalue() else {} return code, calls + def test_auto_on_a_queue_base_reports_queued_with_its_position(self) -> None: + # The rules read missed the queue, so the arm ran `gh pr merge --auto`, + # which GitHub answered by putting the PR in the queue with no auto-merge + # request. + code, _ = self._main( + True, + "--merge", + "--expected-head", + HEAD, + "--auto", + queue=_queue_read(in_queue=True, required=True, state="QUEUED", position=1), + ) + self.assertEqual(code, 0) + self.assertEqual(self.output["action"], "enqueue") + self.assertTrue(self.output["enqueued"]) + self.assertFalse(self.output["autoMergeEnabled"]) + self.assertFalse(self.output["merged"]) + self.assertEqual(self.output["mergeQueue"], {"state": "QUEUED", "position": 1}) + + def test_auto_armed_to_enter_a_queue_reports_the_queue(self) -> None: + code, _ = self._main( + True, + "--merge", + "--expected-head", + HEAD, + "--auto", + queue=_queue_read(required=True, armed=True), + ) + self.assertEqual((code, self.output["action"]), (0, "auto-merge")) + self.assertTrue(self.output["autoMergeEnabled"]) + self.assertNotIn("enqueued", self.output) + self.assertTrue(self.output["mergeQueue"]["entersWhenReady"]) + + def test_auto_on_a_plain_base_reports_the_arm_unchanged(self) -> None: + code, _ = self._main( + True, + "--merge", + "--expected-head", + HEAD, + "--auto", + queue=_queue_read(armed=True), + ) + self.assertEqual((code, self.output["action"]), (0, "auto-merge")) + self.assertTrue(self.output["autoMergeEnabled"]) + self.assertFalse(self.output["merged"]) + self.assertNotIn("enqueued", self.output) + self.assertNotIn("mergeQueue", self.output) + + def test_a_queue_base_showing_no_queue_entry_or_arm_is_not_reported_armed( + self, + ) -> None: + code, _ = self._main( + True, + "--merge", + "--expected-head", + HEAD, + "--auto", + queue=_queue_read(required=True), + ) + self.assertEqual((code, self.output["action"]), (10, "merge-pending")) + self.assertFalse(self.output["autoMergeEnabled"]) + self.assertFalse(self.output["ready"]) + self.assertTrue(self.output["mergeQueue"]["unconfirmed"]) + self.assertIn("merge pending, unconfirmed", self.output["blockers"][0]) + self.assertIn("without --state-dir", self.output["merge"]["message"]) + + def test_an_unreadable_queue_keeps_the_arm_report(self) -> None: + unreadable = {"readError": "could not read the merge queue: HTTP 403"} + code, _ = self._main( + True, "--merge", "--expected-head", HEAD, "--auto", queue=unreadable + ) + self.assertEqual(code, 0) + self.assertTrue(self.output["autoMergeEnabled"]) + self.assertEqual( + self.output["merge"]["queueReadError"], unreadable["readError"] + ) + def test_auto_arms_squash_pinned_to_head(self) -> None: code, calls = self._main(True, "--merge", "--expected-head", HEAD, "--auto") self.assertEqual(code, 0) @@ -1563,6 +1668,7 @@ def _run_stubbed_gate(self, *flags: str) -> tuple[int, dict[str, Any], mock.Mock # Not the default branch: the merge stays on `gh pr merge`, which is # the path these method-resolution tests read. mock.patch.object(merge, "repository_default_branch", return_value=None), + mock.patch.object(merge, "read_merge_queue", return_value=_queue_read()), contextlib.redirect_stdout(out), ): code = merge.main() diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py index 4c224dd876..1eefe2ac7f 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py @@ -49,6 +49,29 @@ def _async(status: str, message: str = "") -> dict[str, Any]: return {"status": status, "details": {"message": message, "uuid": UUID}} +def _queue( + *, + position: int | None = None, + state: str = "QUEUED", + required: bool = True, + merged: bool = False, + armed: bool = False, +) -> dict[str, Any]: + """A GraphQL `PullRequest` merge-queue read; a position puts the PR in the queue.""" + in_queue = position is not None + pull_request = { + "merged": merged, + "isInMergeQueue": in_queue, + "isMergeQueueEnabled": required, + "mergeQueueEntry": {"state": state, "position": position} if in_queue else None, + "autoMergeRequest": {"enabledAt": "2026-10-04T00:00:00Z"} if armed else None, + } + return {"data": {"repository": {"pullRequest": pull_request}}} + + +NO_QUEUE = _queue(required=False, merged=True) + + class AsyncMergeHarness(unittest.TestCase): """Runs `main --merge` over a stubbed ready verdict and records gh calls.""" @@ -78,6 +101,7 @@ def _run( head: str | None = HEAD, blockers: list[str] | None = None, stack_member: bool = False, + queue: list[dict[str, Any] | Exception] | None = None, ) -> int: if blockers is not None: ready = not blockers @@ -109,6 +133,7 @@ def _run( puts = list(put) if isinstance(put, list) else [put] poll_answers = list(polls or []) listings = list(stack_listings or []) + queue_answers = list(queue or [NO_QUEUE]) def capture(cmd: list[str]) -> Any: self.captures.append(cmd) @@ -129,9 +154,16 @@ def gh_json(args: list[str]) -> Any: if isinstance(listing, Exception): raise listing return {"number": 7, "pull_requests": listing} + if args[1] == "graphql": + self.queue_reads += 1 + answer = queue_answers.pop(0) + if isinstance(answer, Exception): + raise answer + return answer raise AssertionError(f"unexpected gh_json call: {args}") self.stack_reads = 0 + self.queue_reads = 0 ticks = iter(clock or [0.0] * 50) argv = [ "babysit_merge.py", @@ -307,6 +339,11 @@ def test_non_default_base_keeps_gh_pr_merge(self) -> None: self.assertFalse(any("merge-async" in " ".join(c) for c in self.captures)) [legacy] = self.captures self.assertEqual(legacy[:2], ["pr", "merge"]) + self.assertEqual( + (self.output["action"], self.output["merged"]), ("merge", True) + ) + self.assertNotIn("enqueued", self.output) + self.assertNotIn("mergeQueue", self.output) def test_unreadable_default_branch_keeps_gh_pr_merge(self) -> None: self._run(_proc(), default_branch=None) @@ -463,6 +500,322 @@ def test_queue_without_the_endpoint_is_held(self) -> None: self.assertFalse(any(c[:2] == ["pr", "merge"] for c in self.captures)) +class GhPrMergeOntoAQueueIsReportedQueued(AsyncMergeHarness): + """`gh pr merge` routes a merge on any base that has a merge queue into the + queue and exits 0, so a queue the rules read missed shows only in the PR's + own `isInMergeQueue` / `mergeQueueEntry` read back afterward.""" + + def test_a_merge_github_put_in_the_queue_is_queued_not_merged(self) -> None: + code = self._run(_proc(), base="release", queue=[_queue(position=2)]) + self.assertEqual(code, 0) + self.assertEqual(self.output["action"], "enqueue") + self.assertFalse(self.output["merged"]) + self.assertTrue(self.output["enqueued"]) + self.assertEqual(self.output["mergeQueue"], {"state": "QUEUED", "position": 2}) + + def test_an_unreadable_queue_keeps_the_merge_report_and_names_the_failure( + self, + ) -> None: + code = self._run( + _proc(), base="release", queue=[RuntimeError("gh: (HTTP 502)")] + ) + self.assertEqual((code, self.output["merged"]), (0, True)) + self.assertIn("merge queue", self.output["merge"]["queueReadError"]) + + def test_a_queued_merge_is_recorded_for_later_runs(self) -> None: + state = tempfile.TemporaryDirectory() + self.addCleanup(state.cleanup) + self._run( + _proc(), + base="release", + queue=[_queue(position=1)], + extra=("--state-dir", state.name), + ) + path = pathlib.Path(state.name) / merge.PENDING_MERGES_FILE + record = json.loads(path.read_text(encoding="utf-8"))["requests"][ + "owner/repo#1" + ] + self.assertEqual((record["queued"], record["head"]), (True, HEAD)) + + +class AnUnseenQueueResultIsHeldUntilConfirmed(AsyncMergeHarness): + """A `gh pr merge` the queue base took can read back, for a moment, as + neither queued, armed, nor merged. That success is re-read, then recorded + unconfirmed, and later runs confirm it without sending another merge.""" + + def setUp(self) -> None: + super().setUp() + self.state = tempfile.TemporaryDirectory() + self.addCleanup(self.state.cleanup) + + def _merge(self, answers: list[dict[str, Any] | Exception]) -> int: + return self._run( + _proc(), + base="release", + queue=answers, + extra=("--state-dir", self.state.name), + ) + + def _unseen_merge(self) -> None: + unseen = [_queue()] * merge.QUEUE_READ_BACK_ATTEMPTS + code = self._merge(unseen) + self.assertEqual((code, self.output["action"]), (10, "merge-pending")) + self.captures.clear() + self.sleeps.clear() + + def _later_run(self, answer: dict[str, Any] | Exception, **kwargs: Any) -> int: + return self._run( + _proc(), + base="release", + queue=[answer], + extra=("--state-dir", self.state.name), + **kwargs, + ) + + def _records(self) -> dict[str, Any]: + path = pathlib.Path(self.state.name) / merge.PENDING_MERGES_FILE + return json.loads(path.read_text(encoding="utf-8"))["requests"] + + def _merge_commands(self) -> list[list[str]]: + return [c for c in self.captures if c[:2] == ["pr", "merge"]] + + def test_an_unseen_result_is_re_read_then_held_and_recorded(self) -> None: + unseen = [_queue()] * merge.QUEUE_READ_BACK_ATTEMPTS + code = self._merge(unseen) + self.assertEqual((code, self.output["action"]), (10, "merge-pending")) + self.assertFalse(self.output["ready"]) + self.assertFalse(self.output["merged"]) + self.assertNotIn("enqueued", self.output) + self.assertTrue(self.output["mergeQueue"]["unconfirmed"]) + self.assertIn("merge pending, unconfirmed", self.output["blockers"][0]) + self.assertEqual(self.queue_reads, merge.QUEUE_READ_BACK_ATTEMPTS) + self.assertEqual( + self.sleeps, + [merge.ASYNC_MERGE_POLL_INTERVAL_SECONDS] + * (merge.QUEUE_READ_BACK_ATTEMPTS - 1), + ) + record = self._records()["owner/repo#1"] + self.assertEqual( + (record["queued"], record["unconfirmed"], record["head"]), + (True, True, HEAD), + ) + + def test_a_re_read_that_finds_the_entry_reports_it_queued(self) -> None: + code = self._merge([_queue(), _queue(position=1)]) + self.assertEqual((code, self.output["action"]), (0, "enqueue")) + self.assertTrue(self.output["enqueued"]) + self.assertEqual(self.sleeps, [merge.ASYNC_MERGE_POLL_INTERVAL_SECONDS]) + self.assertNotIn("unconfirmed", self._records()["owner/repo#1"]) + + def test_a_failed_re_read_never_turns_an_unseen_result_into_a_merge( + self, + ) -> None: + failures = [RuntimeError("gh: (HTTP 502)")] * ( + merge.QUEUE_READ_BACK_ATTEMPTS - 1 + ) + code = self._merge([_queue(), *failures]) + self.assertEqual((code, self.output["action"]), (10, "merge-pending")) + self.assertFalse(self.output["merged"]) + self.assertTrue(self._records()["owner/repo#1"]["unconfirmed"]) + + def test_the_next_run_holds_without_sending_another_merge(self) -> None: + self._unseen_merge() + code = self._later_run(_queue()) + self.assertEqual((code, self.output["action"]), (10, "merge-pending")) + self.assertFalse(self.output["ready"]) + self.assertTrue(self.output["mergeQueue"]["unconfirmed"]) + self.assertIn("merge pending, unconfirmed", self.output["blockers"][0]) + self.assertEqual(self._merge_commands(), []) + self.assertEqual(self._records()["owner/repo#1"]["unseenReads"], 1) + + def test_an_entry_that_appears_later_is_reported_queued_and_confirmed( + self, + ) -> None: + self._unseen_merge() + code = self._later_run(_queue(position=2, state="AWAITING_CHECKS")) + self.assertEqual((code, self.output["action"]), (10, "merge-pending")) + self.assertTrue(self.output["enqueued"]) + self.assertEqual( + self.output["mergeQueue"], {"state": "AWAITING_CHECKS", "position": 2} + ) + self.assertEqual(self._merge_commands(), []) + record = self._records()["owner/repo#1"] + self.assertNotIn("unconfirmed", record) + # Confirmed, it follows the queued-entry rule: leaving the queue unmerged + # is a dequeue on the very next read. + code = self._later_run(_queue()) + self.assertEqual(code, 10) + self.assertTrue(self.output["dequeued"]) + self.assertEqual(self._records(), {}) + + def test_an_arm_that_appears_later_holds_without_sending_another_merge( + self, + ) -> None: + self._unseen_merge() + code = self._later_run(_queue(armed=True)) + self.assertEqual((code, self.output["action"]), (10, "merge-pending")) + self.assertFalse(self.output["enqueued"]) + self.assertTrue(self.output["mergeQueue"]["entersWhenReady"]) + self.assertIn("armed", self.output["blockers"][0]) + self.assertEqual(self._merge_commands(), []) + self.assertNotIn("unconfirmed", self._records()["owner/repo#1"]) + + def test_a_merge_that_appears_later_reports_merged(self) -> None: + self._unseen_merge() + code = self._later_run( + _queue(merged=True), ready=False, blockers=["state=MERGED (not OPEN)"] + ) + self.assertEqual(code, 0) + self.assertTrue(self.output["merged"]) + self.assertEqual(self.output["pendingMergeRequest"]["status"], "merged") + self.assertEqual(self._merge_commands(), []) + self.assertEqual(self._records(), {}) + + def test_a_result_never_seen_is_dequeued_and_gated_again(self) -> None: + self._unseen_merge() + for _ in range(merge.QUEUE_UNCONFIRMED_LATER_READS - 1): + code = self._later_run(_queue()) + self.assertEqual((code, self.output["action"]), (10, "merge-pending")) + code = self._later_run(_queue()) + self.assertEqual(code, 10) + self.assertTrue(self.output["dequeued"]) + self.assertNotEqual(self.output["action"], "merge-pending") + self.assertIn("dequeued", self.output["blockers"][0]) + self.assertEqual(self._merge_commands(), []) + self.assertEqual(self._records(), {}) + self._later_run(_queue(position=1)) + self.assertEqual(len(self._merge_commands()), 1) + self.assertEqual(self.output["action"], "enqueue") + + +class QueuedPullRequestIsConfirmedLater(AsyncMergeHarness): + """An enqueue is not a merge: a later run reads the queue entry back and + reports it still queued, merged at the vetted head, or dequeued.""" + + def setUp(self) -> None: + super().setUp() + self.state = tempfile.TemporaryDirectory() + self.addCleanup(self.state.cleanup) + + def _enqueue(self) -> None: + code = self._run( + _proc(_async("enqueued")), + merge_action="merge_queue", + extra=("--state-dir", self.state.name), + ) + self.assertEqual((code, self.output["enqueued"]), (0, True)) + self.captures.clear() + + def _later_run(self, answer: dict[str, Any] | Exception, **kwargs: Any) -> int: + return self._run( + _proc(_async("enqueued")), + merge_action="merge_queue", + queue=[answer], + extra=("--state-dir", self.state.name), + **kwargs, + ) + + def _records(self) -> dict[str, Any]: + path = pathlib.Path(self.state.name) / merge.PENDING_MERGES_FILE + return json.loads(path.read_text(encoding="utf-8"))["requests"] + + def test_an_async_enqueue_is_recorded(self) -> None: + self._enqueue() + record = self._records()["owner/repo#1"] + self.assertEqual((record["queued"], record["head"]), (True, HEAD)) + + def test_a_pr_still_in_the_queue_reports_its_position_and_sends_nothing( + self, + ) -> None: + self._enqueue() + code = self._later_run(_queue(position=3, state="AWAITING_CHECKS")) + self.assertEqual((code, self.output["action"]), (10, "merge-pending")) + self.assertTrue(self.output["enqueued"]) + self.assertFalse(self.output["merged"]) + self.assertEqual( + self.output["mergeQueue"], {"state": "AWAITING_CHECKS", "position": 3} + ) + self.assertIn("position 3", self.output["blockers"][0]) + self.assertEqual(self.captures, []) + self.assertIn("owner/repo#1", self._records()) + + def test_a_pr_that_left_the_queue_merged_reports_the_merge(self) -> None: + self._enqueue() + code = self._later_run( + _queue(merged=True), ready=False, blockers=["state=MERGED (not OPEN)"] + ) + self.assertEqual(code, 0) + self.assertTrue(self.output["merged"]) + self.assertEqual(self.output["blockers"], []) + self.assertEqual(self.output["pendingMergeRequest"]["status"], "merged") + self.assertEqual(self._records(), {}) + + def test_a_queue_merge_at_another_head_escalates(self) -> None: + self._enqueue() + code = self._later_run(_queue(merged=True), landed_head="f" * 40) + self.assertEqual(code, 10) + self.assertIn("escalate", self.output["blockers"][0]) + self.assertFalse(self.output["merged"]) + + def test_a_pr_that_left_the_queue_unmerged_is_reported_dequeued(self) -> None: + self._enqueue() + code = self._later_run(_queue()) + self.assertEqual(code, 10) + self.assertTrue(self.output["dequeued"]) + self.assertFalse(self.output["merged"]) + self.assertNotEqual(self.output["action"], "merge-pending") + self.assertIn("dequeued", self.output["blockers"][0]) + self.assertEqual(self.captures, []) + self.assertEqual(self._records(), {}) + + def test_an_unreadable_queue_keeps_the_record_and_holds(self) -> None: + self._enqueue() + code = self._later_run(RuntimeError("gh: Server Error (HTTP 502)")) + self.assertEqual((code, self.output["action"]), (10, "merge-pending")) + self.assertIn("could not be read", self.output["blockers"][0]) + self.assertEqual(self.captures, []) + self.assertIn("owner/repo#1", self._records()) + + +class MergeQueueReadBack(unittest.TestCase): + def _read(self, payload: Any) -> dict[str, Any]: + with mock.patch.object(merge, "gh_json", return_value=payload) as gh_json: + result = merge.read_merge_queue("owner/repo", 5) + [args] = gh_json.call_args.args + self.assertEqual(args[:2], ["api", "graphql"]) + self.assertIn("n=5", args) + return result + + def test_a_queued_pull_request_reports_its_entry(self) -> None: + result = self._read(_queue(position=4, state="MERGEABLE")) + self.assertEqual( + ( + result["readError"], + result["inQueue"], + result["position"], + result["state"], + ), + (None, True, 4, "MERGEABLE"), + ) + + def test_a_pull_request_armed_to_enter_the_queue_is_not_in_it(self) -> None: + result = self._read(_queue(armed=True)) + self.assertEqual( + (result["inQueue"], result["queueRequired"], result["autoMergeArmed"]), + (False, True, True), + ) + + def test_a_response_without_the_pull_request_is_a_read_error(self) -> None: + self.assertIsNotNone(self._read({"data": {"repository": None}})["readError"]) + + def test_a_failed_read_is_a_read_error(self) -> None: + with mock.patch.object( + merge, "gh_json", side_effect=RuntimeError("not enabled for this session") + ): + result = merge.read_merge_queue("owner/repo", 5) + self.assertIn("not enabled for this session", result["readError"]) + + def _listed(number: int, sha: str, *, merged: bool = False) -> dict[str, Any]: return { "number": number,