diff --git a/README.md b/README.md index b37bdee6..e7f000c5 100644 --- a/README.md +++ b/README.md @@ -470,22 +470,28 @@ hold a PR like any `quest:*` label; passing their token to `quest iterate` swaps them for `quest:active`. Their `quest:merge` holds nothing and is ignored. -`quest merge` takes no label: the head-pinned auto-merge it enables is the -queue signal, so a queued PR shows no hold. It refuses while any session holds -the PR, so release your own hold first. `--land` needs the full 40-character +`quest merge` takes no label: the head-pinned auto-merge or merge-queue entry +it creates is the queue signal, so a queued PR shows no hold. It refuses while +any session holds the PR, so release your own hold first. `--land` needs the full 40-character head SHA and refuses once the head moves. -It enables auto-merge bound to that SHA, which enters GitHub's merge queue -when the base has one, or merges at once when nothing is left to wait for. -`--squash` or `--rebase` picks the method, and it refuses one the repository -disallows; otherwise it uses the only method the repository allows, or else a -merge commit, and refuses when merge commits are off and more than one method -remains. A merge queue applies its own. On a base that requires the `quest land` check, it +It enables auto-merge bound to that SHA, or merges at once when nothing is +left to wait for. `--squash` or `--rebase` picks the method, and it refuses one +the repository disallows; otherwise it uses the only method the repository +allows, or else a merge commit, and refuses when merge commits are off and more +than one method remains. On a base that requires the `quest land` check, it queues the PR for [`quest land`](#land-a-batch-with-quest-land) instead. -On an ungated base with auto-merge disabled, it refuses. Follow the repository’s own landing +On a base with GitHub's merge queue, which applies its own method, it enqueues +that head, or arms auto-merge bound to it while required checks may still be +pending. The queue builds each PR on the current base, so a head behind it is +enqueued as is. When GitHub refuses to enqueue the head, such as after the +base moved into a conflict, the command fails: fix the PR, for example with +`/quest-iterate`, and land it again. +On a base with neither the `quest land` check nor a merge queue, it refuses +while repository auto-merge is disabled. Follow the repository’s own landing instructions in CONTRIBUTING.md or AGENTS.md, such as its label queue; never retry with `--direct` unless the user asks. `--direct` permits the usual landing behavior despite that repository setting. -On an ungated base, it compares the live base against that head and refuses a +On such a base, it also compares the live base against that head and refuses a head that is behind or diverged: merge the base, rerun the repository's checks, and land the new head. This is a preflight check; GitHub can still move the base between that comparison and its head-pinned merge mutation. A PR that leaves diff --git a/assets/skills/merge.md b/assets/skills/merge.md index f3d934f1..71f9d5f4 100644 --- a/assets/skills/merge.md +++ b/assets/skills/merge.md @@ -29,8 +29,9 @@ A finding means the PR leaves its quest inaccurate: recommend `/quest-iterate`. Queue the head with `quest merge --land `. It uses the only merge method the repository allows, or else a merge commit; add `--squash` or `--rebase` when the repository's root instructions name that method for the PR's base. On a base that requires the `quest land` check, it queues the PR for `quest land`, whose chain merges and tests against the current base. -On an ungated base with auto-merge disabled, it refuses. Follow the repository’s own landing instructions (for example, its label queue); never retry with `--direct` unless the user explicitly asks. -On an ungated base, it refuses a head that does not contain the current base: recommend `/quest-iterate` to merge the base. +On a base with GitHub's merge queue, it enqueues the head; if GitHub refuses, such as after the base moved into a conflict, it fails: recommend `/quest-iterate`. +On an ungated base without a merge queue, it refuses while auto-merge is disabled. Follow the repository’s own landing instructions (for example, its label queue); never retry with `--direct` unless the user explicitly asks. +There, it also refuses a head that does not contain the current base: recommend `/quest-iterate` to merge the base. Report once it is queued, and do not wait for it to merge. Plan the PR's suggested follow-ups as soon as it is queued, as if it lands, rather than waiting for the merge, unless a coordinator such as `/quest-complete` called this skill and batches them. diff --git a/quest/m0/land-fixer/README.md b/quest/m0/land-fixer/README.md index 0b6481f0..5217f79e 100644 --- a/quest/m0/land-fixer/README.md +++ b/quest/m0/land-fixer/README.md @@ -135,8 +135,9 @@ Decision prompts, selected options marked ✅: this epic keeps only the fixer's own quality work. Moved to the watcher: the attempt budget (now the stop rules, [moq-dev/quest#384](https://github.com/moq-dev/quest/pull/384)), - local mergeability and CI-failure parking (now - [Bad picks](/quest/m0/queue-watcher/bad-picks.md)), and flaky tests (now + local mergeability and CI-failure parking (then + [Bad picks](/quest/m0/queue-watcher/bad-picks.md), dropped 2026-11-02: a + pick GitHub refuses fails `quest merge` instead), and flaky tests (now [Flaky failures](/quest/m0/queue-watcher/flaky.md), without the rerun). Deleted: the batch quest check, pre-checks, and throughput tuning, which only served batches (the fixer cap and reasoning effort moved to diff --git a/quest/m0/land-fixer/pr-signal.md b/quest/m0/land-fixer/pr-signal.md index 99cf442f..e411509c 100644 --- a/quest/m0/land-fixer/pr-signal.md +++ b/quest/m0/land-fixer/pr-signal.md @@ -31,8 +31,9 @@ non-required check failed on the head it is about to pick, without refusing. The fixer gets a summary of the PR's checks (check name, conclusion, run URL, and a trimmed log of each failure) in its job input, next to the queue's failure log. A failing non-required check never stops a PR on its own; a -failing required check keeps GitHub from enqueuing it, which -[Bad picks](/quest/m0/queue-watcher/bad-picks.md) handles. +failing required check keeps GitHub from enqueuing it: `/quest-merge` +refuses such a head, `quest merge` fails when GitHub refuses it, and either +way the PR needs `/quest-iterate`. Retargeted 2026-11-01 to the [queue watcher](/quest/m0/queue-watcher/README.md), which replaces the batched host gate. diff --git a/quest/m0/queue-watcher/README.md b/quest/m0/queue-watcher/README.md index 9f75c951..a13e262e 100644 --- a/quest/m0/queue-watcher/README.md +++ b/quest/m0/queue-watcher/README.md @@ -79,9 +79,30 @@ Decision prompts, selected options marked ✅: - ✅ The fixer re-enqueues them unchanged, and a test seen flaking twice is disabled in a separate PR with a follow-up quest; the rerun before blame (#375) is dropped. See [Flaky failures](/quest/m0/queue-watcher/flaky.md). -- Picks GitHub refuses to enqueue: - - ✅ The watcher fixes them, then enqueues; see - [Bad picks](/quest/m0/queue-watcher/bad-picks.md) +- Pick record (decided 2026-11-02, replacing "Picks GitHub refuses to + enqueue: ✅ The watcher fixes them"): + - ✅ GitHub's records only. Drop the comment and `pick.ts`. The watcher + reads queue events and auto-merge state. A pick GitHub refuses fails the + command, and /quest-merge recommends /quest-iterate as it does today. + `bad-picks.md` shrinks to enqueueing PRs with auto-merge armed that + GitHub never queued + ([community#209348](https://github.com/orgs/community/discussions/209348)). + - Comment only on refusal + - Keep the comment on every pick + + Reasons: for queued picks, GitHub already keeps a durable, + writer-authenticated record. `AddedToMergeQueueEvent` and + `RemovedFromMergeQueueEvent` give the enqueuer (who must have write + access), the head before removal, and the reason, and `autoMergeRequest` + gives `enabledBy`/`enabledAt` for picks waiting on checks. Auto-merge can't + reliably be armed on a conflicting PR, and on merge-queue bases enabling it + returns HTTP 422 unless every requirement is met + ([community#190610](https://github.com/orgs/community/discussions/190610), + [community#50327](https://github.com/orgs/community/discussions/50327)). + /quest-merge already refuses heads with failing checks or conflicts, so a + refusal only happens if the base moves between that check and the enqueue. + See [Queue removals](/quest/m0/queue-watcher/removals.md) and + [Bad picks](/quest/m0/queue-watcher/bad-picks.md). - Milestone: - ✅ m0, at the top @@ -109,10 +130,9 @@ Shared shape, for the children to follow: ## Required -- [Picks enter the merge queue](/quest/m0/queue-watcher/pick.md) - `quest merge --land` enqueues the pinned head into GitHub's merge queue and records the pick where the watcher reads it - [Queue removals](/quest/m0/queue-watcher/removals.md) - the watcher sees each PR the queue drops, why, and the failing build's log - [Fix and re-enqueue](/quest/m0/queue-watcher/fix.md) - the fixer runs on each dropped PR in the background, and the watcher re-enqueues it at the back -- [Bad picks](/quest/m0/queue-watcher/bad-picks.md) - a pick GitHub refuses to enqueue is merged locally with the quest driver or fixed, then pushed and enqueued +- [Bad picks](/quest/m0/queue-watcher/bad-picks.md) - a pick whose auto-merge is armed on a ready PR GitHub never queued is enqueued at the head it was armed on - [Flaky failures](/quest/m0/queue-watcher/flaky.md) - a flaky failure is re-enqueued unchanged, and a test seen flaking twice is disabled in a separate PR with a follow-up quest - [Watcher on the host](/quest/m0/queue-watcher/host.md) - kixelated/nixos runs the watcher for moq-dev/moq beside its fixer container - [The other repositories follow](/quest/m0/queue-watcher/follow.md) - condition: moq's queue is proven; then moq-dev/quest switches and the others plan theirs diff --git a/quest/m0/queue-watcher/bad-picks.md b/quest/m0/queue-watcher/bad-picks.md index e5f5f8c4..d44d6e34 100644 --- a/quest/m0/queue-watcher/bad-picks.md +++ b/quest/m0/queue-watcher/bad-picks.md @@ -1,49 +1,44 @@ -# [M] The watcher fixes a pick GitHub refuses to enqueue +# [S] The watcher enqueues an armed pick GitHub never queued ## Goal -A picked PR that GitHub refuses to enqueue, because it conflicts with the -base or otherwise can't merge, is fixed by the watcher instead of handed back. -The watcher merges the base into it locally with the quest merge driver; a -clean merge is pushed and enqueued. When the local merge still conflicts, or -the PR's own required checks fail, the fixer resolves it with the conflicting -paths or the failing checks' logs, and the watcher pushes and enqueues the -result. The PR reaches a person only on a handback. The watcher never trusts -GitHub's mergeability flag alone. +A PR on a merge-queue base whose auto-merge a writer armed, as +`quest merge --land` does while required checks are pending, and that is now +ready (its required checks passed and it doesn't conflict) but still not in +the queue, is enqueued by the watcher at the head auto-merge was armed on. +Since about 2026-09-10 GitHub often leaves such PRs out of the queue +([community#209348](https://github.com/orgs/community/discussions/209348)), +while `enqueuePullRequest` works. A head pushed since by anyone but the fixer +is not enqueued. ## Plan -Decided 2026-11-01 (maintainer, /quest-plan; see the -[epic](/quest/m0/queue-watcher/README.md)): - -- Picks GitHub refuses to enqueue: - - ✅ The watcher fixes them, including conflicts resolved with the quest - merge driver, then pushes and enqueues - - Hand them back - -This quest was the host gate's local mergeability quest, decided 2026-10-31 -(user, /quest-plan of the follow-ups from a moq-dev/moq `/quest-complete` -run): GitHub reported conflicts on -[moq-dev/moq#7407](https://github.com/moq-dev/moq/pull/7407), which merges -cleanly, and on [moq-dev/moq#7382](https://github.com/moq-dev/moq/pull/7382), -whose only conflict was in quest link lists, which the driver resolves. It -also absorbs the host gate's plan to park a PR whose own required CI fails for -the fixer with the failing check's log (decided 2026-11-01, settling -[moq-dev/quest#355](https://github.com/moq-dev/quest/pull/355)'s decision 8), -since on a merge-queue base such a PR is a pick GitHub won't enqueue. - -- The watcher learns of refused picks from the marker - [Picks enter the merge queue](/quest/m0/queue-watcher/pick.md) records, and - from picks whose auto-merge stays armed while the PR is conflicting or its - required checks failed. -- Merge locally with `mergeBase` (`src/cli/land.ts`), which already uses the - driver; the host gate's `prepareHead` (`src/cli/land-head.ts`) shows the - push. A clean driver merge needs no review: it is a conflict-free base merge. -- Fetch failing required checks' logs through the App's `checks: read` and - `actions: read`, trimmed like a queue failure's. -- The same stop rules apply (`stopRule` in `src/cli/land-fixer.ts`). +Decided 2026-11-02 (maintainer, /quest-plan; see the +[epic](/quest/m0/queue-watcher/README.md)), narrowing this quest from fixing +every pick GitHub refuses: GitHub's records are the only pick record, and a +pick GitHub refuses to enqueue fails `quest merge`, which says to fix the PR +with `/quest-iterate` and land it again. The plans this quest held for +refused picks are dropped with it: merging the base locally with the quest +driver (the host gate's local mergeability, from +[moq-dev/moq#7407](https://github.com/moq-dev/moq/pull/7407) and +[moq-dev/moq#7382](https://github.com/moq-dev/moq/pull/7382)), and parking a +PR whose own required checks fail for the fixer +([moq-dev/quest#355](https://github.com/moq-dev/quest/pull/355)'s +decision 8). + +- Find the candidates in the pass + [Queue removals](/quest/m0/queue-watcher/removals.md) runs: open PRs with an + `autoMergeRequest`, no `mergeQueueEntry`, and a ready `mergeStateStatus` + (`CLEAN`, `HAS_HOOKS`, or `UNSTABLE`). +- Enqueue with `enqueuePullRequest` and `expectedHeadOid` set to the head + `enabledHead` (`src/cli/status.ts`) says auto-merge was armed on, only when + that is the current head. GitHub keeps auto-merge across a writer's push, + but that push ends the pick; a head `enabledHead` can't tell yet waits for + the next pass. An "already queued" refusal means GitHub moved it first. +- An armed pick whose own required checks fail, or that turns conflicting, + is left alone: it needs `/quest-iterate` and a new pick, and + `quest status` shows its checks. ## Required -- [Fix and re-enqueue](/quest/m0/queue-watcher/fix.md) - the fixer path and re-enqueue this reuses -- [Picks enter the merge queue](/quest/m0/queue-watcher/pick.md) - the pick marker a refused pick leaves +- [Queue removals](/quest/m0/queue-watcher/removals.md) - the pass that reads each PR's auto-merge and queue state diff --git a/quest/m0/queue-watcher/pick.md b/quest/m0/queue-watcher/pick.md deleted file mode 100644 index b7866fff..00000000 --- a/quest/m0/queue-watcher/pick.md +++ /dev/null @@ -1,44 +0,0 @@ -# [S] quest merge --land enqueues into GitHub's merge queue - -## Goal - -On a base with a merge queue, `quest merge --land ` -enqueues the pinned head into GitHub's merge queue, or arms auto-merge bound -to it while its required checks are pending, and records the pick so the -watcher knows which head the maintainer picked. It never refuses a head for -being behind the base, since the queue builds each PR on the current base. A -pick GitHub refuses to enqueue (conflicts, failing required checks, or another -mergeability refusal) is still recorded, and the command says the watcher will -fix it rather than failing. - -## Plan - -Decided 2026-11-01 (maintainer, /quest-plan; see the -[epic](/quest/m0/queue-watcher/README.md)): picks enqueue into GitHub's merge -queue, and the watcher fixes a pick GitHub refuses. - -`land` in `src/cli/hold.ts` already enables auto-merge bound to the head on a -queue base (`snapshot.queue`), which enters the queue once checks pass. What -changes: - -- Skip the behind-or-diverged comparison on a queue base. It exists for - ungated bases, where nothing else tests the PR on the current base. -- Prefer `enqueuePullRequest` with `expectedHeadOid` when the PR is already - mergeable, so the head is pinned and enqueued at once. GitHub enqueues a PR - only once its required checks pass on the PR itself. -- Record the pick durably. On the host gate the pin lives in squash - auto-merge's commit body, which a conflicting PR cannot get, and GitHub drops - auto-merge when the queue removes a PR. Pick a marker the watcher can read - for a refused or removed PR, such as a comment naming the picked head, and - coordinate with [moq-dev/quest#326](https://github.com/moq-dev/quest/pull/326), - which drops the `quest:merge` label. The pick covers that head and the heads - the fixer pushes from it. -- `quest:merge` (or its successor) must not be released when the queue drops - the PR while the watcher still holds it (`dequeued` in `src/cli/hold.ts`); - only a handback ends the pick. -- Bases that require the `quest land` check keep queuing for the host gate - until their repository [follows](/quest/m0/queue-watcher/follow.md). - -## Related - -- [Bad picks](/quest/m0/queue-watcher/bad-picks.md) - what the watcher does with a refused pick diff --git a/quest/m0/queue-watcher/removals.md b/quest/m0/queue-watcher/removals.md index d8fb410e..4861289a 100644 --- a/quest/m0/queue-watcher/removals.md +++ b/quest/m0/queue-watcher/removals.md @@ -15,16 +15,28 @@ Decided 2026-11-01 (maintainer, /quest-plan; see the [epic](/quest/m0/queue-watcher/README.md)). - Poll, don't receive webhooks: the host has no public endpoint. Reconcile - from the picks, not from queue snapshots alone: a PR enqueued and dropped - between two polls, or while the watcher was down, never shows in a - snapshot. Each pass reads every open PR holding an active pick (the marker - [Picks enter the merge queue](/quest/m0/queue-watcher/pick.md) records) with - its `mergeQueueEntry` and its `RemovedFromMergeQueueEvent`s newer than the - last one handled, with `reason`, `actor`, and `enqueuer`. `quest status` - already reads that event (`src/cli/status.ts`). The base's - `mergeQueue { entries }` can still serve as a cheap change signal. The App's - installation token has its own rate limit, separate from the maintainers' - sessions. + from GitHub's records of each pick, not from queue snapshots alone: a PR + enqueued and dropped between two polls, or while the watcher was down, + never shows in a snapshot. Each pass reads the base's open PRs, many per + GraphQL query, with their `mergeQueueEntry`, their + `autoMergeRequest { enabledBy enabledAt }`, and their + `AddedToMergeQueueEvent`s and `RemovedFromMergeQueueEvent`s newer than the + last one handled (`timelineItems(since:)`): `enqueuer`, `actor`, `reason`, + and the removal's `beforeCommit`. `quest status` already reads the removal + event (`src/cli/status.ts`). The base's `mergeQueue { entries }` can still + serve as a cheap change signal. The App's installation token has its own + rate limit, separate from the maintainers' sessions. +- A pick is GitHub's own record, with no marker of quest's + (decided 2026-11-02; see the [epic](/quest/m0/queue-watcher/README.md)): + the PR was enqueued, or had auto-merge armed while its required checks were + pending, as `quest merge --land` does on a merge-queue base. GitHub lets + only a writer enqueue a PR or enable auto-merge, so the `enqueuer` or + `enabledBy` is the trust signal; nothing reads comments or checks a + commenter's permission. The removal's `beforeCommit` is the head the queue + dropped. A pick covers that head and the heads the fixer pushes from it, + which the watcher's App then enqueues itself, and stays active until a + handback, a push by anyone else, or a person's removal; a writer enqueuing + the PR again starts a new one. - For a failed build, find the PR's `merge_group` workflow runs (event `merge_group`, head branch `gh-readonly-queue//pr--`) and fetch the failing jobs' logs through the App's `actions: read`, trimmed the @@ -39,7 +51,3 @@ Decided 2026-11-01 (maintainer, /quest-plan; see the - Start it as `quest land watch` beside `quest land serve`, sharing its App credential options, state directory, and comment marker and deduplication. - -## Required - -- [Picks enter the merge queue](/quest/m0/queue-watcher/pick.md) - the durable pick marker the watcher reconciles from diff --git a/src/cli/github.ts b/src/cli/github.ts index 51d5b130..b76787b2 100644 --- a/src/cli/github.ts +++ b/src/cli/github.ts @@ -33,6 +33,16 @@ export class GitHubDeadline extends Error { } } +/** GitHub answered a GraphQL operation with errors; `types` are their types, `undefined` for an untyped one. */ +export class GitHubGraphQLError extends Error { + constructor( + readonly types: (string | undefined)[], + messages: string[], + ) { + super(`GitHub GraphQL error: ${messages.join("; ")}`); + } +} + export type ObjectValue = Record; /** Retries of a 5xx or a dropped connection, waiting 1, 2, 4, then 8 seconds unless GitHub says how long. */ @@ -424,7 +434,10 @@ export class GitHub { if (body.errors === undefined) return object(body.data); const errors = array(body.errors); if (!errors.some((error) => error.type === "RATE_LIMITED")) - throw new Error(`GitHub GraphQL error: ${errors.map((error) => String(error.message)).join("; ")}`); + throw new GitHubGraphQLError( + errors.map((error) => (typeof error.type === "string" ? error.type : undefined)), + errors.map((error) => String(error.message)), + ); // GitHub turned the operation away unprocessed, so any operation repeats. await this.waitOut(response.headers, limits, "GitHub GraphQL rate limit persisted"); } @@ -458,6 +471,10 @@ export class GitHub { */ const PULL_MUTATIONS: Record = { mergePullRequest: { fields: "merged", landed: (pull) => pull.merged === true }, + enqueuePullRequest: { + fields: "merged mergeQueueEntry { state }", + landed: (pull) => pull.merged === true || pull.mergeQueueEntry != null, + }, enablePullRequestAutoMerge: { fields: "merged autoMergeRequest { enabledAt } mergeQueueEntry { state }", landed: (pull) => pull.merged === true || pull.autoMergeRequest != null || pull.mergeQueueEntry != null, diff --git a/src/cli/hold.ts b/src/cli/hold.ts index 2f22d187..1da2915e 100644 --- a/src/cli/hold.ts +++ b/src/cli/hold.ts @@ -1,7 +1,7 @@ // `quest iterate`, `ready`, `merge`, and `release`: one session works on a PR at // a time. It holds the `quest:active` label, and the ID of the timeline event // that added the label is its hold token, since sessions often share an account. -// `quest merge` takes no label: the pinned auto-merge it enables is the queue signal. +// `quest merge` takes no label: the pinned auto-merge or merge-queue entry it creates is the queue signal. import { ACTIVE_LABEL, handedBack, isHoldLabel, latestHold, heldToken } from "../core"; import { cleanAutoMergeRefusal, immediateMerge } from "../github/merge"; import { repositoryPath } from "./claims"; @@ -9,6 +9,7 @@ import { array, GitHub, type GitHubDependencies, + GitHubGraphQLError, object, type ObjectValue, type PullRequest, @@ -24,7 +25,7 @@ export type Method = "merge" | "squash" | "rebase"; const READ = `query HoldOwnership($owner: String!, $repo: String!, $number: Int!) { repository(owner: $owner, name: $repo) { pullRequest(number: $number) { - id url state isDraft body updatedAt baseRefName headRefOid mergeStateStatus isMergeQueueEnabled + id url state isDraft body updatedAt baseRefName headRefOid mergeStateStatus isMergeQueueEnabled isInMergeQueue labels(first: 100) { nodes { name } } timelineItems(last: 100, itemTypes: [LABELED_EVENT]) { nodes { __typename ... on LabeledEvent { id createdAt label { name } } } @@ -51,6 +52,8 @@ interface Snapshot { head: string; mergeState: string; queue: boolean; + /** Whether the PR is in its base's merge queue. */ + queued: boolean; /** The hold labels on the PR. */ labels: string[]; /** Oldest first. */ @@ -187,8 +190,8 @@ function mergeMethod(repository: ObjectValue, named: Method | undefined, url: st /** * Land `sha`, which must still be the head of `pr`, while no session holds it: - * enable auto-merge bound to it, which enters the merge queue when the base has one, - * or merge at once when GitHub would refuse auto-merge because nothing is left to wait for. + * enable auto-merge bound to it, or merge at once when GitHub would refuse auto-merge because + * nothing is left to wait for. A base with a merge queue gets {@link pick} instead. * A base that requires the `quest land` check queues it for `quest land` instead: * squash auto-merge whose commit body pins `sha`. It takes no label. * `named` is the method the caller asked for; otherwise {@link mergeMethod} picks one. @@ -201,11 +204,7 @@ export async function land( direct = false, ): Promise { const labels = new Labels(pr, deps); - const current = (snapshot: Snapshot) => { - if (snapshot.head.toLowerCase() !== sha.toLowerCase()) { - throw new Refusal(`${snapshot.url} head moved to ${snapshot.head}; check the new head before landing it`); - } - }; + const current = (snapshot: Snapshot) => headAt(snapshot, sha); const snapshot = await labels.read(); open(snapshot); current(snapshot); @@ -231,23 +230,28 @@ export async function land( return; } // A merge queue is the repository's own gate: it applies its own method and needs no auto-merge setting. - let method: Method = named ?? "merge"; - if (!snapshot.queue) { - const repository = object(await (await labels.github.request(repositoryPath(pr))).json()); - method = mergeMethod(repository, named, snapshot.url); - if (!direct) { - // GitHub shows the merge settings only to writers; fail closed, with a next step. - if (typeof repository.allow_auto_merge !== "boolean") - throw new Refusal( - `${snapshot.url} repository's auto-merge setting is not visible to this token, so a direct merge cannot be ruled out; follow its landing instructions in CONTRIBUTING.md or AGENTS.md`, - ); - if (!repository.allow_auto_merge) - throw new Refusal( - `${snapshot.url} repository has auto-merge disabled; follow its landing instructions in CONTRIBUTING.md or AGENTS.md (for example, its label queue). Only use --direct if the user explicitly asks`, - ); - } + if (snapshot.queue) { + if (named !== undefined && named !== "merge") + deps.warn(`${snapshot.url} lands through a merge queue, which applies its own merge method`); + await unchanged(); + await pick(labels, snapshot, named ?? "merge"); + return; } - // The batch gate tests its own current-base chain. Ungated landings need that base in the PR head. + const repository = object(await (await labels.github.request(repositoryPath(pr))).json()); + const method = mergeMethod(repository, named, snapshot.url); + if (!direct) { + // Otherwise auto-merge must be on. + // GitHub shows the merge settings only to writers; fail closed, with a next step. + if (typeof repository.allow_auto_merge !== "boolean") + throw new Refusal( + `${snapshot.url} repository's auto-merge setting is not visible to this token, so a direct merge cannot be ruled out; follow its landing instructions in CONTRIBUTING.md or AGENTS.md`, + ); + if (!repository.allow_auto_merge) + throw new Refusal( + `${snapshot.url} repository has auto-merge disabled; follow its landing instructions in CONTRIBUTING.md or AGENTS.md (for example, its label queue). Only use --direct if the user explicitly asks`, + ); + } + // The batch gate and a merge queue test the current base themselves. Here nothing does, so the head must contain it. const comparison = object( await ( await labels.github.request( @@ -261,9 +265,6 @@ export async function land( `${snapshot.url} head ${snapshot.head} does not contain ${snapshot.base} at ${base}; merge the base and rerun the repository's checks before landing`, ); } - if (snapshot.queue && named !== undefined && named !== "merge") { - deps.warn(`${snapshot.url} lands through a merge queue, which applies its own merge method`); - } await unchanged(); const submit = (mutation: "mergePullRequest" | "enablePullRequestAutoMerge") => labels.github.graphql( @@ -279,7 +280,7 @@ export async function land( try { await submit("enablePullRequestAutoMerge"); } catch (error) { - if (snapshot.queue || !cleanAutoMergeRefusal(error instanceof Error ? error.message : null)) throw error; + if (!cleanAutoMergeRefusal(error instanceof Error ? error.message : null)) throw error; // GitHub may finish computing UNKNOWN while it handles auto-merge. Reread once, without polling. const fresh = await labels.read(); current(fresh); @@ -296,6 +297,60 @@ export async function land( } } +/** Merge states in which GitHub may still be running required checks, so only auto-merge can wait for them. */ +const PENDING = ["BLOCKED", "UNKNOWN"]; + +/** GraphQL error types that fault the token or the request, not the PR GitHub would enqueue. */ +const UNREFUSED = ["FORBIDDEN", "NOT_FOUND", "INSUFFICIENT_SCOPES", "INTERNAL"]; + +/** + * Pick `snapshot`'s head to land through the base's merge queue, as {@link land} checked it: + * enqueue that head, or arm auto-merge bound to it while its required checks may be pending. + * The queue builds each PR on the current base, so a head behind it is enqueued as is. + * GitHub's queue events and auto-merge request record the pick, naming who made it. + * A pick GitHub refuses, such as one that conflicts or fails its required checks, fails: + * the PR needs a fix before it can land. + */ +async function pick(labels: Labels, snapshot: Snapshot, method: Method): Promise { + const enqueuePull = () => + labels.github.graphql( + `mutation($id: ID!, $sha: GitObjectID!) { + enqueuePullRequest(input: { pullRequestId: $id, expectedHeadOid: $sha }) { clientMutationId } + }`, + { id: snapshot.id, sha: snapshot.head }, + ); + try { + if (!PENDING.includes(snapshot.mergeState)) { + await enqueuePull(); + return; + } + try { + await labels.github.graphql( + `mutation($id: ID!, $sha: GitObjectID!, $method: PullRequestMergeMethod!) { + enablePullRequestAutoMerge(input: { pullRequestId: $id, expectedHeadOid: $sha, mergeMethod: $method }) { clientMutationId } + }`, + { id: snapshot.id, sha: snapshot.head, method: method.toUpperCase() }, + ); + } catch (error) { + // An UNKNOWN state GitHub finished computing as ready: nothing is left to wait for. + if (!cleanAutoMergeRefusal(error instanceof Error ? error.message : null)) throw error; + await enqueuePull(); + } + } catch (error) { + if (!(error instanceof GitHubGraphQLError) || error.types.some((type) => UNREFUSED.includes(type ?? ""))) + throw error; + // A head that moved since is no longer the one being landed; say so rather than ask for a fix. + const fresh = await labels.read(); + open(fresh); + headAt(fresh, snapshot.head); + // Already queued, as when this repeats an earlier landing: GitHub refused nothing that matters. + if (fresh.queued) return; + throw new Refusal( + `${snapshot.url}: GitHub refused to enqueue ${snapshot.head} (${error.message}); fix the PR, for example with /quest-iterate, and land it again`, + ); + } +} + /** * Finish `work` under `hold`, which may be a label just swapped in for the caller's. * A failure names the hold, so the caller keeps the token it needs to release or retry. @@ -357,6 +412,13 @@ async function acquire(labels: Labels, token: string | undefined): Promise<{ hol return { hold: { label, token: ours.id }, snapshot: { ...after, labels: [label] } }; } +/** Refuse once the head is no longer `sha`, the head being landed. */ +function headAt(snapshot: Snapshot, sha: string): void { + if (snapshot.head.toLowerCase() !== sha.toLowerCase()) { + throw new Refusal(`${snapshot.url} head moved to ${snapshot.head}; check the new head before landing it`); + } +} + function open(snapshot: Snapshot): void { if (snapshot.state !== "OPEN") { throw new Refusal(`${snapshot.url} is ${snapshot.state.toLowerCase()}; a push now would bring its branch back`); @@ -455,6 +517,7 @@ class Labels { head: string(row.headRefOid), mergeState: string(row.mergeStateStatus), queue: row.isMergeQueueEnabled === true, + queued: row.isInMergeQueue === true, labels: array(object(row.labels).nodes) .map((label) => string(label.name)) .filter(isHoldLabel), diff --git a/src/cli/main.ts b/src/cli/main.ts index e639eafc..7a8cb0fe 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -480,12 +480,14 @@ export function main(argv: string[], out: Output): number | Promise { .command("merge") .summary("Land a PR's reviewed head") .description( - "Land a PR's reviewed head: enable auto-merge bound to the full head SHA --land names, which enters the " + - "base's merge queue when it has one, or merge at once when nothing is left to wait for. It takes no " + - "label: the auto-merge it enables is the queue signal. It refuses while a session holds the PR, so " + - "release your own hold first, and once the head has moved. It uses the method --squash or " + - "--rebase names, refusing one the repository disallows; otherwise the only method the repository " + - "allows, or else a merge commit. A merge queue applies its own. On a base " + + "Land a PR's reviewed head: enable auto-merge bound to the full head SHA --land names, or merge at " + + "once when nothing is left to wait for. It takes no label: the auto-merge or merge-queue entry it " + + "creates is the queue signal. It refuses while a session holds the PR, so release your own hold " + + "first, and once the head has moved. It uses the method --squash or --rebase names, refusing one " + + "the repository disallows; otherwise the only method the repository allows, or else a merge commit. " + + "On a base with GitHub's merge queue, which applies its own method, it enqueues that head, or arms " + + "auto-merge bound to it while required checks may be pending; a head behind the base is enqueued as " + + "is, and a head GitHub refuses to enqueue fails, to be fixed and landed again. On a base " + `that requires the \`${LAND_CHECK}\` check, it queues the PR for \`quest land\` instead: squash ` + "auto-merge whose commit body pins the head. On an ungated base with auto-merge disabled, it refuses " + "unless --direct is passed. Follow the repository landing instructions in CONTRIBUTING.md or " + diff --git a/test/github.test.ts b/test/github.test.ts index 0260baa8..41278a00 100644 --- a/test/github.test.ts +++ b/test/github.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "vitest"; -import { GitHub, GitHubDeadline, type GitHubDependencies } from "../src/cli/github"; +import { GitHub, GitHubDeadline, type GitHubDependencies, GitHubGraphQLError } from "../src/cli/github"; const repo = "/repos/example/project"; const HEAD = "a".repeat(40); @@ -324,6 +324,7 @@ describe("GraphQL", () => { test.each([ ["mergePullRequest", { merged: true }], ["enablePullRequestAutoMerge", { merged: false, autoMergeRequest: null, mergeQueueEntry: { state: "QUEUED" } }], + ["enqueuePullRequest", { merged: false, mergeQueueEntry: { state: "QUEUED" } }], ["markPullRequestReadyForReview", { isDraft: false }], ])("%s is confirmed from the pull request at its pinned head", async (mutation, node) => { const query = `mutation($id: ID!, $sha: GitObjectID!) { ${mutation}(input: { pullRequestId: $id, expectedHeadOid: $sha }) { clientMutationId } }`; @@ -377,10 +378,15 @@ describe("GraphQL", () => { }); test("other GraphQL errors fail without waiting", async () => { - const { github, sleeps } = fixture([json({ errors: [{ type: "NOT_FOUND", message: "Not Found" }] })]); - await expect(github.graphql("query { viewer { login } }", {})).rejects.toThrow( - "GitHub GraphQL error: Not Found", - ); + const { github, sleeps } = fixture([ + json({ errors: [{ type: "NOT_FOUND", message: "Not Found" }, { message: "Not mergeable" }] }), + ]); + const error = await github.graphql("query { viewer { login } }", {}).catch((caught: unknown) => caught); + expect(error).toBeInstanceOf(GitHubGraphQLError); + expect(error).toMatchObject({ + message: "GitHub GraphQL error: Not Found; Not mergeable", + types: ["NOT_FOUND", undefined], + }); expect(sleeps).toEqual([]); }); diff --git a/test/hold.test.ts b/test/hold.test.ts index edcf19f2..dd0eadc3 100644 --- a/test/hold.test.ts +++ b/test/hold.test.ts @@ -49,6 +49,8 @@ class FakePull { /** Runs after each label is added, as when another session adds one a moment later. */ afterAdd?: () => void; beforeAutoMerge?: () => string | undefined; + /** The error GitHub answers `enqueuePullRequest` with, if any. */ + beforeEnqueue?: () => { message: string; type?: string } | undefined; reads = 0; private clock = Date.parse("2026-10-21T12:00:00Z"); @@ -132,7 +134,7 @@ class FakePull { if (variables.id !== "PR_7") throw new Error("mutation on the wrong PR"); const method = variables.method ?? /mergeMethod: (\w+)/.exec(query)?.[1]; const body = variables.body ? ` ${String(variables.body)}` : ""; - this.writes.push(method ? `${name} ${variables.sha} ${method}${body}` : name); + this.writes.push([name, variables.sha, method].filter(Boolean).join(" ") + body); if (name === "markPullRequestReadyForReview") this.draft = false; if (name === "mergePullRequest") this.state = "MERGED"; if (name === "enablePullRequestAutoMerge") { @@ -143,6 +145,11 @@ class FakePull { this.event("AutoMergeEnabledEvent"); } if (name === "disablePullRequestAutoMerge") this.autoMerge = null; + if (name === "enqueuePullRequest") { + const error = this.beforeEnqueue?.(); + if (error) return { errors: [error] }; + this.queueEntry = { state: "QUEUED" }; + } return { data: { [name]: { clientMutationId: null } } }; } if (query.includes("node(id: $id)")) return { data: { node: { autoMergeRequest: this.autoMerge } } }; @@ -169,6 +176,7 @@ class FakePull { isMergeQueueEnabled: this.queue, autoMergeRequest: this.autoMerge, mergeQueueEntry: this.queueEntry, + isInMergeQueue: this.queueEntry !== null, labels: { nodes: this.labels.map((name) => ({ name })) }, timelineItems: { nodes: this.events.slice(-100) }, }, @@ -369,6 +377,84 @@ describe("merge --land", () => { expect(pull.writes.at(-1)).toBe(`enablePullRequestAutoMerge ${HEAD} MERGE`); }); + test.each(["CLEAN", "HAS_HOOKS", "UNSTABLE", "BEHIND", "DIRTY"])( + "enqueues a %s head on a queue base without comparing it to the base", + async (state) => { + const pull = new FakePull(); + pull.queue = true; + pull.mergeState = state; + pull.comparison = "diverged"; + const warnings: string[] = []; + await land(PR, HEAD, "merge", pull.deps(warnings)); + expect(pull.writes).toEqual([`enqueuePullRequest ${HEAD}`]); + expect(pull.queueEntry).not.toBeNull(); + expect(pull.comparisons).toEqual([]); + expect(warnings).toEqual([]); + }, + ); + + test("arms auto-merge bound to the head while a queue base's required checks are pending", async () => { + const pull = new FakePull(); + pull.queue = true; + pull.mergeState = "BLOCKED"; + await land(PR, HEAD, "merge", pull.deps()); + expect(pull.writes).toEqual([`enablePullRequestAutoMerge ${HEAD} MERGE`]); + }); + + test("enqueues an UNKNOWN head GitHub finds clean when arming auto-merge", async () => { + const pull = new FakePull(); + pull.queue = true; + pull.mergeState = "UNKNOWN"; + pull.beforeAutoMerge = () => "Pull request is in clean status"; + await land(PR, HEAD, "squash", pull.deps()); + expect(pull.writes).toEqual([`enablePullRequestAutoMerge ${HEAD} SQUASH`, `enqueuePullRequest ${HEAD}`]); + }); + + test("fails when GitHub refuses to enqueue the head, saying to fix the PR", async () => { + const pull = new FakePull(); + pull.queue = true; + pull.mergeState = "DIRTY"; + pull.beforeEnqueue = () => ({ message: "Pull request is not mergeable", type: "UNPROCESSABLE" }); + expect(await refused(land(PR, HEAD, "merge", pull.deps()))).toBe( + `${URL_}: GitHub refused to enqueue ${HEAD} (GitHub GraphQL error: Pull request is not mergeable); fix the PR, for example with /quest-iterate, and land it again`, + ); + expect(pull.queueEntry).toBeNull(); + }); + + test("says nothing when GitHub refuses a head already in the queue", async () => { + const pull = new FakePull(); + pull.queue = true; + pull.mergeState = "CLEAN"; + pull.beforeEnqueue = () => { + pull.queueEntry = { state: "QUEUED" }; + return { message: "Pull request is already queued" }; + }; + const warnings: string[] = []; + await land(PR, HEAD, "merge", pull.deps(warnings)); + expect(warnings).toEqual([]); + }); + + test("refuses when the head moved before GitHub refused to enqueue it", async () => { + const pull = new FakePull(); + pull.queue = true; + pull.mergeState = "CLEAN"; + pull.beforeEnqueue = () => { + pull.head = "c".repeat(40); + return { message: "Head ref has moved" }; + }; + expect(await refused(land(PR, HEAD, "merge", pull.deps()))).toContain(`head moved to ${"c".repeat(40)}`); + }); + + test("passes on an enqueue error that faults the token rather than the PR", async () => { + const pull = new FakePull(); + pull.queue = true; + pull.mergeState = "CLEAN"; + pull.beforeEnqueue = () => ({ message: "Resource not accessible by integration", type: "FORBIDDEN" }); + const error = await land(PR, HEAD, "merge", pull.deps()).catch((caught: unknown) => caught); + expect(error).not.toBeInstanceOf(Refusal); + expect(error).toMatchObject({ message: "GitHub GraphQL error: Resource not accessible by integration" }); + }); + test("--direct explicitly permits a clean merge with auto-merge off", async () => { const pull = new FakePull(); pull.allowAutoMerge = false; @@ -492,13 +578,13 @@ describe("merge --land", () => { }); }); - test("enters the merge queue through auto-merge even when clean", async () => { + test("warns that a merge queue applies its own method", async () => { const pull = new FakePull(); pull.mergeState = "CLEAN"; pull.queue = true; const warnings: string[] = []; await land(PR, HEAD, "squash", pull.deps(warnings)); - expect(pull.writes.at(-1)).toBe(`enablePullRequestAutoMerge ${HEAD} SQUASH`); + expect(pull.writes.at(-1)).toBe(`enqueuePullRequest ${HEAD}`); expect(warnings).toEqual([`${URL_} lands through a merge queue, which applies its own merge method`]); });