From cf770788b9036cdeb300d644ee55a31e9c25d8f8 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 31 Oct 2026 08:11:25 -0700 Subject: [PATCH 1/3] chore(quest): claim quest/m0/land-head-moved.md Quest-Claim: 59ca5b235503842f From 8be9669164a8f498cc70cc8e883c108c67e1bc12 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 31 Oct 2026 08:31:48 -0700 Subject: [PATCH 2/3] fix(land): report head_moved only when a writer moved the head Right after the gate pushes its base merge, GitHub can still report the pin as the PR's head and refuse the merge for it with a 405 "merge conflicts", since the pin conflicts with the base that push merged. mergePinned retried only a 409 there, so moq-dev/moq#7407 was ejected as head_moved after the gate's own push. Retry any refusal while GitHub still shows the pin. When one PR stops a batch, the later PRs in it are now ejected as batch_stopped, with a comment naming that PR, instead of inheriting its reason. Co-Authored-By: Claude Opus 5.5 --- README.md | 13 +++++++---- assets/skills/complete.md | 1 + quest/m0/README.md | 1 - quest/m0/land-head-moved.md | 29 ------------------------- src/cli/land-serve.ts | 24 +++++++++++++++------ src/cli/land.ts | 4 +++- src/cli/main.ts | 2 +- test/land.test.ts | 43 ++++++++++++++++++++++++++++++++++--- 8 files changed, 71 insertions(+), 46 deletions(-) delete mode 100644 quest/m0/land-head-moved.md diff --git a/README.md b/README.md index 1afbb0eb..ff173a3f 100644 --- a/README.md +++ b/README.md @@ -569,13 +569,18 @@ tested squash to the base as a fast-forward, leased on the expected parent. It closes the PR with a comment linking the landed commit and removes `quest:merge`. Such a fork PR is closed rather than shown as merged by GitHub. A PR branch lease failure ejects `head_moved`. A locally prepared or API squash tree mismatch logs -CRITICAL and ejects the remaining batch as `tree_mismatch`. +CRITICAL and stops the batch; a PR whose prepared tree differs ejects as +`tree_mismatch`, while an already-squashed one stays merged. After an API refusal, the daemon re-reads mergeability and briefly retries -pending mergeability or a base refresh. Confirmed conflicts and moved heads eject -the remainder; a persistent policy refusal ejects as `merge_refused` without the +pending mergeability or a base refresh. It also retries while GitHub still +reports the pin as the head of a PR it just prepared, since GitHub can refuse the +merge for that stale head; only another writer's head ejects `head_moved`. +Confirmed conflicts and moved heads eject the remainder; a persistent policy refusal ejects as `merge_refused` without the fixer. Already-merged PRs are skipped. An unexpected API squash parent means the -base moved: the remainder is rebuilt without ejection. +base moved: the remainder is rebuilt without ejection. Whenever a PR stops the +batch, it is ejected with its own reason, and every later PR in the batch as +`batch_stopped`, with a comment naming the PR that stopped it. Restart accepts a changed head only if the private state records that exact daemon-authored merge for the same original pin and queue time, with the pin as diff --git a/assets/skills/complete.md b/assets/skills/complete.md index c5d9801e..e3c3361b 100644 --- a/assets/skills/complete.md +++ b/assets/skills/complete.md @@ -37,6 +37,7 @@ Each sub-agent reports back only when done or blocked, and never waits on GitHub Do not watch the queue or wait for merges. An open PR that left the queue unmerged shows why under `landing-stopped` in `quest status `. After `merge_conflict` or `failed_checks`, its pick still holds, even from an earlier session: dispatch `/quest-iterate` without asking, then run `/quest-merge` when it reports. +After `batch_stopped`, another PR in its batch failed and this one never merged; its pick still holds: run `/quest-merge` again. After `checks_pending`, or a pick `/quest-merge` left for pending checks, run `/quest-merge` again on a later refresh once they pass. Any other reason, such as a person removing it, is the user's call, so ask about the PR again. diff --git a/quest/m0/README.md b/quest/m0/README.md index 9322d412..07757178 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -42,7 +42,6 @@ ownership-manifest design: - [Merge through a host gate](/quest/m0/host-land/README.md) - inline merge picks, small batches landed from the maintainer's host, and a Codex fixer instead of bisection - [Landing priority](/quest/m0/land-priority.md) - cheap checks wait a bounded time and the land daemon's check runs ahead of agent builds - [Local mergeability](/quest/m0/land-mergeability.md) - `quest land` merges with the quest driver instead of ejecting on GitHub's conflict flag -- [Accurate head_moved](/quest/m0/land-head-moved.md) - the gate's own base-merge push and batch-mate failures are not reported as a moved head - [Queue-time checks](/quest/m0/land-queue-checks.md) - queued PRs are checked against each other and `quest check` runs per PR before batching - [Drop emptied Required headings](/quest/m0/required-heading.md) - a clean driver merge that empties a `Required` list passes `quest check` - [Codex cloud off](/quest/m0/codex-cloud-off.md) - automatic Codex cloud reviews stop once no open PR relies on them diff --git a/quest/m0/land-head-moved.md b/quest/m0/land-head-moved.md deleted file mode 100644 index ef2e0e2f..00000000 --- a/quest/m0/land-head-moved.md +++ /dev/null @@ -1,29 +0,0 @@ -# [S] quest land reports head_moved only when a writer moved the head - -## Goal - -`quest land` ejects a PR as `head_moved` only when someone other than the -gate pushed to it after it was queued. The gate's own base-merge push is -never counted as a moved head. A PR stopped because a batch-mate conflicted -or moved is ejected with that reason, naming the batch-mate. - -## Plan - -Decided 2026-10-31 (user, /quest-plan of the follow-ups from a moq-dev/moq -`/quest-complete` run). PRs were wrongly ejected as `head_moved`: -[moq-dev/moq#7407](https://github.com/moq-dev/moq/pull/7407), -[moq-dev/moq#7416](https://github.com/moq-dev/moq/pull/7416), -[moq-dev/moq#7353](https://github.com/moq-dev/moq/pull/7353), -[moq-dev/moq#7413](https://github.com/moq-dev/moq/pull/7413). - -- Own push. `prepareHead` (`src/cli/land-head.ts`) pushes a base merge to a - PR's head, and `isPrepared` recognises that head from a private record. Land - still reported `head_moved` after that push, so one of the checks in - `resolveQueue` (`src/cli/land.ts`), `prepareHead`, or `mergePinned` - (`src/cli/land-serve.ts`) missed it. - Find the case the record misses, such as GitHub lagging, a restart, or - `forgetHeads` running too early, and cover it with a test. -- Batch-mates. When one link fails to merge, `serveBatch` - (`src/cli/land-serve.ts`) ejects every remaining link with that link's - reason and detail. Give the other PRs their own reason, one that names - the PR that stopped the batch. diff --git a/src/cli/land-serve.ts b/src/cli/land-serve.ts index 2987ca6a..2f395dcd 100644 --- a/src/cli/land-serve.ts +++ b/src/cli/land-serve.ts @@ -227,16 +227,15 @@ async function mergePinned(github: GitHub, path: string, head: string, previous if (pull.merged === true) return null; const detail = `GitHub refused ${path}: ${String(result.message)}`; if (pull.state !== "open") return { reason: "merge_refused", detail }; - if ( - previous !== head && - response.status === 409 && - (object(pull.head).sha === previous || object(pull.head).sha === head) - ) { + // Right after the gate's own push, GitHub can still report the pin as the head and refuse the merge + // for it: a 409, or a 405 when the pin conflicts with the base that push merged. Neither is a writer. + const current = object(pull.head).sha; + if (previous !== head && (current === previous || (response.status === 409 && current === head))) { if (attempt >= 3) throw new Error(`${detail}; prepared head has not propagated`); await github.pause(500 * 2 ** attempt); continue; } - if (object(pull.head).sha !== head) return { reason: "head_moved", detail }; + if (current !== head) return { reason: "head_moved", detail }; if (pull.mergeable === false || pull.mergeable_state === "dirty") return { reason: "merge_conflict", detail }; if (pull.mergeable === null || /base branch was modified/i.test(String(result.message))) { if (attempt >= 3) throw new Error(`${detail}; mergeability did not settle`); @@ -430,7 +429,18 @@ export async function serveBatch( log, // A refusal is the repository's settings or rules, which the fixer cannot change. numbers: reason === "merge_refused" ? [] : remaining.map(({ entry }) => entry.number), - ejections: remaining.map(({ entry }) => ejection(entry, reason, excerpt(detail))), + // Only the PR that stopped the batch carries its reason; the rest name it. + ejections: remaining.map(({ entry }) => + entry.number === link.entry.number + ? ejection(entry, reason, excerpt(detail)) + : ejection( + entry, + "batch_stopped", + excerpt( + `PR #${link.entry.number} stopped the batch as ${reason} before this PR merged: ${detail}`, + ), + ), + ), }; break; } diff --git a/src/cli/land.ts b/src/cli/land.ts index c55feec3..e793b707 100644 --- a/src/cli/land.ts +++ b/src/cli/land.ts @@ -73,7 +73,9 @@ export type Reason = | "head_moved" | "not_pinned" | "tree_mismatch" - | "merge_refused"; + | "merge_refused" + /** Another PR in its batch failed to merge, so this one never reached GitHub's merge. */ + | "batch_stopped"; /** The hidden line an ejection comment carries, which `quest status` reads. */ export const EJECTED = /^$/m; diff --git a/src/cli/main.ts b/src/cli/main.ts index 226224d4..9464bf47 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -534,7 +534,7 @@ export function main(argv: string[], out: Output): number | Promise { landCommand .command("serve") .description( - "Run one daemon per repository. Check a base PR plus up to floor(log2 n) cleanly merging PRs once through the sandbox. Failed batches are ejected together, never bisected. Requires quest on PATH and flock. Sandbox and fixer wrappers MUST run code as a different user from the App key owner (for example in nspawn containers); environment filtering alone cannot isolate /proc or host files. The sandbox receives , with the base also in QUEST_LAND_BASE and present in the standalone shallow clone; the optional tracked background fixer receives the log path followed by PR numbers. Neither receives credentials. State retains logs, fixer jobs, and one mechanical attempt per maintainer pick across restarts. Fixer results are validated, lease-pushed, recorded in Review, and pinned for requeue; invalid results, crashes, and timeouts are handed back. After revalidating the queue, merge the base into each behind-base PR head with quest resolve, verify the tested tree, and lease-push that head before the pinned GitHub squash API merge. A permissions-refused branch push falls back to a leased fast-forward of the tested squash to the base, closing the PR with a linked-commit comment and removing quest:merge. A recorded daemon merge atop the original pin is recognized after restart; other moved heads require maintainer requeueing. Pending mergeability is retried briefly; confirmed conflicts or moved heads eject the remaining batch. A refusal that would repeat, such as squash being disabled, ejects as merge_refused without the fixer. An unexpected squash parent rebuilds the queue without ejecting; a tree mismatch logs CRITICAL and ejects the rest as tree_mismatch. Merged PRs drop out of the queue after restart.", + "Run one daemon per repository. Check a base PR plus up to floor(log2 n) cleanly merging PRs once through the sandbox. Failed batches are ejected together, never bisected. Requires quest on PATH and flock. Sandbox and fixer wrappers MUST run code as a different user from the App key owner (for example in nspawn containers); environment filtering alone cannot isolate /proc or host files. The sandbox receives , with the base also in QUEST_LAND_BASE and present in the standalone shallow clone; the optional tracked background fixer receives the log path followed by PR numbers. Neither receives credentials. State retains logs, fixer jobs, and one mechanical attempt per maintainer pick across restarts. Fixer results are validated, lease-pushed, recorded in Review, and pinned for requeue; invalid results, crashes, and timeouts are handed back. After revalidating the queue, merge the base into each behind-base PR head with quest resolve, verify the tested tree, and lease-push that head before the pinned GitHub squash API merge. A permissions-refused branch push falls back to a leased fast-forward of the tested squash to the base, closing the PR with a linked-commit comment and removing quest:merge. A recorded daemon merge atop the original pin is recognized after restart; other moved heads require maintainer requeueing. Pending mergeability, and GitHub still showing the pin after the daemon's own push, are retried briefly; confirmed conflicts or another writer's moved head eject the remaining batch. A refusal that would repeat, such as squash being disabled, ejects as merge_refused without the fixer. An unexpected squash parent rebuilds the queue without ejecting; a tree mismatch logs CRITICAL and stops the batch. The PR that stops a batch is ejected with its reason and later PRs in it as batch_stopped, naming that PR. Merged PRs drop out of the queue after restart.", ) .option("--base ", "Base branch (default: repository default)") .option("--state ", "Private daemon state directory (default: $STATE_DIRECTORY or the XDG state home)") diff --git a/test/land.test.ts b/test/land.test.ts index 7f491057..ec881e67 100644 --- a/test/land.test.ts +++ b/test/land.test.ts @@ -693,6 +693,38 @@ describe.skipIf(process.platform === "win32")("quest land", () => { expect(forge.comment(1)).toBeUndefined(); }); + // moq-dev/moq#7407: GitHub still showed the pin after the gate's base-merge push and refused the + // merge as conflicting, since that pin conflicts with the base. It was ejected as head_moved. + test("a pushed head's 405 conflict on the stale pin is retried rather than ejected as head_moved", async () => { + const forge = new Forge(); + const pull = forge.pull("Old", { "old.txt": "yes" }); + const pin = pull.head; + forge.advance({ "new.txt": "yes" }); + const deps = await forge.deps(); + const plan = await resolveQueue(REPOSITORY, undefined, deps); + pull.mergeRefusals = [{ status: 405, message: "Pull Request has merge conflicts" }]; + const seen: (string | undefined)[] = []; + forge.beforeMerge = () => { + seen.push(pull.head); + }; + const pauses: number[] = []; + deps.sleep = async (delay) => { + pauses.push(delay); + }; + let pushed = false; + expect( + await serveBatch(forge.clone(), plan, sandbox(), deps, async () => { + // The first token is the base-merge push; GitHub misses it for the merge and the re-read. + if (!pushed) forge.lag = 2; + pushed = true; + return deps.token; + }), + ).toEqual([`landed 1 ${pull.squash}`]); + expect(seen[1]).toBe(pin); + expect(pauses).toEqual([500]); + expect(forge.comment(1)).toBeUndefined(); + }); + test("restart rejects a writer's commit after a recorded merge", async () => { const forge = new Forge(); forge.pull("Old", { "old.txt": "yes" }); @@ -1123,7 +1155,7 @@ mv '${result}.new' '${result}' }); test.each(["merge_conflict", "head_moved"])( - "a refused second merge ejects the remaining batch as %s and starts the fixer", + "a refused second merge ejects it as %s, the rest as batch_stopped, and starts the fixer", async (reason) => { const forge = new Forge(); for (let i = 1; i <= 4; i++) forge.pull(`PR ${i}`, { [`${i}.txt`]: "yes" }); @@ -1157,13 +1189,16 @@ mv '${result}.new' '${result}' expect(output).toEqual([ `landed 1 ${forge.repo.pulls[0].squash}`, `ejected 2 ${reason}`, - `ejected 3 ${reason}`, + "ejected 3 batch_stopped", ]); expect(forge.merges.map(({ number }) => number)).toEqual([1, 2]); expect(readFileSync(result, "utf8").trim().split("\n").slice(1)).toEqual(["2", "3"]); expect(forge.repo.pulls[0].merged).toBe(true); expect(forge.repo.pulls[3].landing).toBe(true); expect(forge.comment(2)).toContain(``); + // The batch-mate names the PR that stopped the batch instead of inheriting its reason. + expect(forge.comment(3)).toContain(`PR #2 stopped the batch as ${reason}`); + expect(forge.comment(3)).toContain(""); } finally { watcher!.close(); } @@ -1292,11 +1327,13 @@ mv '${result}.new' '${result}' expect(output).toEqual([ `landed 1 ${forge.repo.pulls[0].squash}`, `landed 2 ${forge.repo.pulls[1].squash}`, - "ejected 3 tree_mismatch", + "ejected 3 batch_stopped", ]); expect(forge.merges.map(({ number }) => number)).toEqual([1, 2]); expect(warnings[0]).toContain("CRITICAL"); + expect(forge.comment(3)).toContain("PR #2 stopped the batch as tree_mismatch"); expect(forge.comment(3)).toContain("tested chain"); + expect(forge.comment(3)).toContain(""); expect(forge.repo.pulls.slice(0, 2).every((pull) => pull.merged)).toBe(true); expect(forge.repo.pulls[3].landing).toBe(true); }); From 8632fe3fb38357affdf14bb2310dac8a292061a8 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 1 Nov 2026 13:58:10 -0800 Subject: [PATCH 3/3] fix(land): eject only the mismatched PR after a tree mismatch A maintainer decided (2026-11-01) that a tree mismatch, prepared or after GitHub's squash, ejects only the mismatched PR, as tree_mismatch. Its batch-mates stay queued and the next batch rebuilds and checks them on the current base, as after conflicts and moved heads. Drop the batch_stopped reason and its /quest-complete line. Co-Authored-By: Claude Opus 5.5 --- CONTRIBUTING.md | 5 ++- README.md | 6 +-- assets/skills/complete.md | 1 - src/cli/land-serve.ts | 42 ++++-------------- src/cli/land.ts | 4 +- src/cli/main.ts | 2 +- test/land.test.ts | 92 +++++++++++++++++++++++++++++++++++---- 7 files changed, 100 insertions(+), 52 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2a84acbc..6c3aee43 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -55,8 +55,9 @@ configured sandbox. Before each landing, the daemon merges the base into the PR head with the quest merge driver and verifies the tested tree, then pushes that head with a lease and squash-merges it through GitHub. If branch permissions refuse the push, it fast-forwards the base to the tested squash with a lease and -closes the PR with a linked-commit comment instead. Confirmed conflicts, moved heads, or a tree mismatch eject the -remaining batch. Transient refusals are retried; a racing base rebuilds the queue; already merged PRs stay merged. Failed batches are ejected together, with +closes the PR with a linked-commit comment instead. A confirmed conflict parks only +that PR for the fixer, and a moved head or a prepared tree mismatch ejects only that PR; the +rest of the batch stays queued. Transient refusals are retried; a racing base rebuilds the queue; already merged PRs stay merged. Failed batches are ejected together, with a log tail in each comment and the full log retained on the host. The optional credential-free `quest land fix` container makes one mechanical attempt per maintainer pick. The daemon validates and lease-pushes fixes, records the new diff --git a/README.md b/README.md index 8151a3e1..13575666 100644 --- a/README.md +++ b/README.md @@ -638,9 +638,9 @@ tested squash to the base as a fast-forward, leased on the expected parent. It closes the PR with a comment linking the landed commit and removes `quest:merge`. Such a fork PR is closed rather than shown as merged by GitHub. A PR branch lease failure ejects `head_moved`. A locally prepared or API squash tree mismatch logs -CRITICAL and stops the batch; a PR whose prepared tree differs ejects as -`tree_mismatch`, while an already-squashed one stays merged. Every later PR in -the batch ejects as `batch_stopped`, with a comment naming the PR that stopped it. +CRITICAL. A PR whose prepared tree differs ejects alone as `tree_mismatch`; an +already-squashed one stays merged. Either way, the rest of the batch stays queued +and is rebuilt and checked on the new base. After an API refusal, the daemon re-reads mergeability and briefly retries pending mergeability or a base refresh. It also retries while GitHub still diff --git a/assets/skills/complete.md b/assets/skills/complete.md index e3c3361b..c5d9801e 100644 --- a/assets/skills/complete.md +++ b/assets/skills/complete.md @@ -37,7 +37,6 @@ Each sub-agent reports back only when done or blocked, and never waits on GitHub Do not watch the queue or wait for merges. An open PR that left the queue unmerged shows why under `landing-stopped` in `quest status `. After `merge_conflict` or `failed_checks`, its pick still holds, even from an earlier session: dispatch `/quest-iterate` without asking, then run `/quest-merge` when it reports. -After `batch_stopped`, another PR in its batch failed and this one never merged; its pick still holds: run `/quest-merge` again. After `checks_pending`, or a pick `/quest-merge` left for pending checks, run `/quest-merge` again on a later refresh once they pass. Any other reason, such as a person removing it, is the user's call, so ask about the PR again. diff --git a/src/cli/land-serve.ts b/src/cli/land-serve.ts index cc6e730e..b7969684 100644 --- a/src/cli/land-serve.ts +++ b/src/cli/land-serve.ts @@ -29,8 +29,6 @@ import { commandEnvironment, eject, ejection, - type Ejection, - type Entry, git, readRemote, type Overlap, @@ -648,8 +646,8 @@ export async function serveBatch( } if ("reason" in result) { // Only this PR failed to land. The rest of the batch passed its check and stays queued, - // to be rebuilt on the new base, except after a tree mismatch, which stops the later ones as batch_stopped. - const detail = `${result.detail}; the rest of the batch was not merged`; + // to be rebuilt on the new base. + const detail = `${result.detail}; the rest of the batch stays queued`; appendFileSync(log, `${detail}\n`); if (result.reason === "merge_conflict") { output.push( @@ -665,15 +663,11 @@ export async function serveBatch( ); return output; } - const mismatch = result.reason === "tree_mismatch"; - const reported = mismatch ? `CRITICAL: ${detail}` : detail; - if (mismatch) deps.warn(reported); - const rest = mismatch ? chain.links.slice(index + 1) : []; + // A prepared tree that differs from the tested one is the gate's own fault, for a maintainer to judge. + const reported = result.reason === "tree_mismatch" ? `CRITICAL: ${detail}` : detail; + if (reported !== detail) deps.warn(reported); return output.concat( - await stop(root, options, deps, batch, [ - ejection(link.entry, result.reason, excerpt(reported)), - ...rest.map(({ entry }) => batchStopped(entry, link.entry.number, result.reason, reported)), - ]), + await stop(root, options, deps, batch, [ejection(link.entry, result.reason, excerpt(reported))]), ); } const sha = result.sha; @@ -691,20 +685,11 @@ export async function serveBatch( expectedParent = sha; const tree = string(object(commit.tree).sha); if (tree === link.tree) continue; - const detail = `CRITICAL: GitHub squash #${link.entry.number} at ${sha} has tree ${tree}, but the tested chain has ${link.tree}. Already merged PRs stay merged; the remaining batch was stopped`; + // The squash is already merged, so there is nothing to eject. The rest of the batch stays queued and + // is rebuilt and checked on the base as it now is, like after a moved base. + const detail = `CRITICAL: GitHub squash #${link.entry.number} at ${sha} has tree ${tree}, but the tested chain has ${link.tree}. It stays merged; the rest of the batch stays queued, to be rebuilt on the new base`; deps.warn(detail); appendFileSync(log, `${detail}\n`); - const remaining = chain.links.slice(index + 1); - if (remaining.length) - output.push( - ...(await stop( - root, - options, - deps, - batch, - remaining.map(({ entry }) => batchStopped(entry, link.entry.number, "tree_mismatch", detail)), - )), - ); return output; } return output; @@ -714,15 +699,6 @@ export async function serveBatch( * Eject PRs that cannot land as tested, such as a moved head, a merge GitHub refuses, or a tree * mismatch. The fixer cannot repair these: a writer moved the head, or the repository's settings refuse. */ -/** A batch-mate that never reached its merge names the PR that stopped the batch rather than taking its reason. */ -function batchStopped(entry: Entry, by: number, reason: Reason, detail: string): Ejection { - return ejection( - entry, - "batch_stopped", - excerpt(`PR #${by} stopped the batch as ${reason} before this PR merged: ${detail}`), - ); -} - async function stop( root: string, options: Pick, diff --git a/src/cli/land.ts b/src/cli/land.ts index 4719001e..125b347c 100644 --- a/src/cli/land.ts +++ b/src/cli/land.ts @@ -73,9 +73,7 @@ export type Reason = | "head_moved" | "not_pinned" | "tree_mismatch" - | "merge_refused" - /** Another PR in its batch failed to merge, so this one never reached GitHub's merge. */ - | "batch_stopped"; + | "merge_refused"; /** The hidden line an ejection comment carries, which `quest status` reads. */ export const EJECTED = /^$/m; diff --git a/src/cli/main.ts b/src/cli/main.ts index 7477d0cb..e8edbfae 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -552,7 +552,7 @@ export function main(argv: string[], out: Output): number | Promise { landCommand .command("serve") .description( - "Run one daemon per repository. As a batch is filled, park alone for the fixer (eject alone, without one) each PR it reaches whose squash onto the base fails quest check with findings the base lacks, and comment once on each newly queued PR that conflicts with the four PRs landing soonest ahead of it. Check a base PR plus up to floor(log2 n) cleanly merging PRs once through the sandbox, alternating the oldest and the newest queued PR as the base. A failed batch is never bisected: its PRs are parked for the fixer, which blames the ones at fault, or ejected together without a fixer. It halves the next batch's size down to one PR; each landed batch grows it back by one. When the fixer blames none and the base fails its check alone, the queue halts until a moved base passes, and the fixer proposes a fix of the base as a PR. Requires quest on PATH and flock. Sandbox and fixer wrappers MUST run code as a different user from the App key owner (for example in nspawn containers); environment filtering alone cannot isolate /proc or host files. The sandbox receives , with the base also in QUEST_LAND_BASE and present in the standalone shallow clone; the optional tracked background fixer receives the log path followed by PR numbers, or none for a base that fails alone. Neither receives credentials. State retains logs, fixer jobs, and one mechanical attempt per maintainer pick across restarts. Fixer results are validated, lease-pushed, recorded in Review, and pinned for requeue, and the comment a result carries for its PR is posted there; invalid results, crashes, and timeouts are handed back. A validated fix whose push GitHub refuses is handed back with its resolution for `quest land apply`. After revalidating the queue, merge the base into each behind-base PR head with quest resolve, verify the tested tree, and lease-push that head before the pinned GitHub squash API merge. A permissions-refused branch push falls back to a leased fast-forward of the tested squash to the base, closing the PR with a linked-commit comment and removing quest:merge. A recorded daemon merge atop the original pin is recognized after restart; other moved heads require maintainer requeueing. Pending mergeability, and GitHub still showing the pin after the daemon's own push, are retried briefly; a confirmed conflict parks only that PR for the fixer and a moved head ejects only that PR, and the rest of the batch stays queued. A refusal that would repeat, such as squash being disabled, ejects as merge_refused without the fixer. An unexpected squash parent rebuilds the queue without ejecting; a tree mismatch logs CRITICAL and stops the batch: a PR whose prepared tree differs is ejected as tree_mismatch, and later PRs in the batch as batch_stopped, naming it. Merged PRs drop out of the queue after restart.", + "Run one daemon per repository. As a batch is filled, park alone for the fixer (eject alone, without one) each PR it reaches whose squash onto the base fails quest check with findings the base lacks, and comment once on each newly queued PR that conflicts with the four PRs landing soonest ahead of it. Check a base PR plus up to floor(log2 n) cleanly merging PRs once through the sandbox, alternating the oldest and the newest queued PR as the base. A failed batch is never bisected: its PRs are parked for the fixer, which blames the ones at fault, or ejected together without a fixer. It halves the next batch's size down to one PR; each landed batch grows it back by one. When the fixer blames none and the base fails its check alone, the queue halts until a moved base passes, and the fixer proposes a fix of the base as a PR. Requires quest on PATH and flock. Sandbox and fixer wrappers MUST run code as a different user from the App key owner (for example in nspawn containers); environment filtering alone cannot isolate /proc or host files. The sandbox receives , with the base also in QUEST_LAND_BASE and present in the standalone shallow clone; the optional tracked background fixer receives the log path followed by PR numbers, or none for a base that fails alone. Neither receives credentials. State retains logs, fixer jobs, and one mechanical attempt per maintainer pick across restarts. Fixer results are validated, lease-pushed, recorded in Review, and pinned for requeue, and the comment a result carries for its PR is posted there; invalid results, crashes, and timeouts are handed back. A validated fix whose push GitHub refuses is handed back with its resolution for `quest land apply`. After revalidating the queue, merge the base into each behind-base PR head with quest resolve, verify the tested tree, and lease-push that head before the pinned GitHub squash API merge. A permissions-refused branch push falls back to a leased fast-forward of the tested squash to the base, closing the PR with a linked-commit comment and removing quest:merge. A recorded daemon merge atop the original pin is recognized after restart; other moved heads require maintainer requeueing. Pending mergeability, and GitHub still showing the pin after the daemon's own push, are retried briefly; a confirmed conflict parks only that PR for the fixer and a moved head ejects only that PR, and the rest of the batch stays queued. A refusal that would repeat, such as squash being disabled, ejects as merge_refused without the fixer. An unexpected squash parent rebuilds the queue without ejecting; a tree mismatch logs CRITICAL, ejects only a PR whose prepared tree differs as tree_mismatch (an already squashed PR stays merged), and leaves the rest of the batch queued to be rebuilt on the new base. Merged PRs drop out of the queue after restart.", ) .option("--base ", "Base branch (default: repository default)") .option("--state ", "Private daemon state directory (default: $STATE_DIRECTORY or the XDG state home)") diff --git a/test/land.test.ts b/test/land.test.ts index 5dcf7098..b30b973e 100644 --- a/test/land.test.ts +++ b/test/land.test.ts @@ -1686,7 +1686,7 @@ mv '${result}.new' '${result}' }, ); - test("a tree mismatch logs CRITICAL, keeps landed PRs, and ejects only the unmerged batch suffix", async () => { + test("a squash tree mismatch logs CRITICAL, stays merged, and the next round rebuilds the rest on it", async () => { const forge = new Forge(); for (let i = 1; i <= 4; i++) forge.pull(`PR ${i}`, { [`${i}.txt`]: "yes" }); forge.mismatch = 2; @@ -1696,20 +1696,94 @@ mv '${result}.new' '${result}' warnings.push(message); }; const plan = await resolveQueue(REPOSITORY, undefined, deps); - const options = sandbox(); - const output = await serveBatch(forge.clone(), plan, options, deps, async () => deps.token); - expect(output).toEqual([ + const bases = join(tempDir(), "bases"); + const options = sandbox(`echo "$2" >> '${bases}'`); + expect(await serveBatch(forge.clone(), plan, options, deps, async () => deps.token)).toEqual([ `landed 1 ${forge.repo.pulls[0].squash}`, `landed 2 ${forge.repo.pulls[1].squash}`, - "ejected 3 batch_stopped", ]); expect(forge.merges.map(({ number }) => number)).toEqual([1, 2]); + expect(warnings).toHaveLength(1); expect(warnings[0]).toContain("CRITICAL"); - expect(forge.comment(3)).toContain("PR #2 stopped the batch as tree_mismatch"); - expect(forge.comment(3)).toContain("tested chain"); - expect(forge.comment(3)).toContain(""); + expect(warnings[0]).toContain("tested chain"); expect(forge.repo.pulls.slice(0, 2).every((pull) => pull.merged)).toBe(true); - expect(forge.repo.pulls[3].landing).toBe(true); + // PR 3 passed the check in the batch, and PR 4 was not in it: both stay queued, without a comment. + expect(forge.repo.pulls.slice(2).every((pull) => pull.landing)).toBe(true); + expect(forge.repo.pulls.every((pull) => !pull.comments?.length)).toBe(true); + + // The next round, based on the newest PR, checks and lands the rest on the base as it is, the mismatched squash included. + forge.mismatch = undefined; + forge.settle(); + const base = forge.main(); + expect(base).toBe(forge.repo.pulls[1].squash); + const next = await resolveQueue(REPOSITORY, undefined, deps); + expect(next.base.sha).toBe(base); + expect(await serveBatch(forge.clone(), next, options, deps, async () => deps.token)).toEqual([ + `landed 4 ${forge.repo.pulls[3].squash}`, + `landed 3 ${forge.repo.pulls[2].squash}`, + ]); + expect(readFileSync(bases, "utf8").trim().split("\n")).toEqual([plan.base.sha, base]); + expect(git(forge.origin, "rev-parse", `${forge.repo.pulls[3].squash}^`)).toBe(base); + expect(warnings).toHaveLength(1); + }); + + test("a prepared tree mismatch ejects only that PR, and the next round rebuilds the rest on the new base", async () => { + const forge = new Forge(); + // Both PRs edit the epic, so preparing PR 2 on PR 1's squash runs the quest merge driver. + forge.pull("Add b", { "quest/m0/README.md": EPIC(["a", "b"]), "quest/m0/b.md": QUEST("B") }); + forge.pull("Add c", { "quest/m0/README.md": EPIC(["a", "c"]), "quest/m0/c.md": QUEST("C") }); + forge.pull("Three", { "3.txt": "yes" }); + forge.pull("Four", { "4.txt": "yes" }); + const deps = await forge.deps(); + const warnings: string[] = []; + deps.warn = (message) => { + warnings.push(message); + }; + const plan = await resolveQueue(REPOSITORY, undefined, deps); + const pin = forge.repo.pulls[1].head; + // After the batch's check, the driver resolves differently than it did for the check. + const quest = join(process.env.PATH!.split(":")[0], "quest"); + forge.beforeMerge = (number) => { + if (number !== 1) return; + renameSync(quest, `${quest}.real`); + writeFileSync( + quest, + `#!/bin/sh\n'${quest}.real' "$@" || exit\nif [ "$1" = resolve ]; then echo drift >> "$2"; fi\n`, + { mode: 0o755 }, + ); + }; + const bases = join(tempDir(), "bases"); + const options = sandbox(`echo "$2" >> '${bases}'`); + expect(await serveBatch(forge.clone(), plan, options, deps, async () => deps.token)).toEqual([ + `landed 1 ${forge.repo.pulls[0].squash}`, + "ejected 2 tree_mismatch", + ]); + expect(forge.merges.map(({ number }) => number)).toEqual([1]); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain("CRITICAL"); + expect(forge.comment(2)).toContain("differs from tested"); + expect(forge.comment(2)).toContain(""); + expect(forge.repo.pulls[1].head).toBe(pin); + expect(forge.repo.pulls[1].landing).toBeFalsy(); + // PR 3 passed the check in the batch, and PR 4 was not in it: both stay queued, without a comment. + expect(forge.repo.pulls.slice(2).every((pull) => pull.landing)).toBe(true); + expect(forge.comment(3)).toBeUndefined(); + expect(forge.comment(4)).toBeUndefined(); + + // The next round, based on the newest PR, leaves PR 2 out and checks and lands the rest on PR 1's squash. + renameSync(`${quest}.real`, quest); + forge.beforeMerge = undefined; + forge.settle(); + const base = forge.main(); + expect(base).toBe(forge.repo.pulls[0].squash); + const next = await resolveQueue(REPOSITORY, undefined, deps); + expect(next.queue.map(({ number }) => number)).toEqual([3, 4]); + expect(await serveBatch(forge.clone(), next, options, deps, async () => deps.token)).toEqual([ + `landed 4 ${forge.repo.pulls[3].squash}`, + `landed 3 ${forge.repo.pulls[2].squash}`, + ]); + expect(readFileSync(bases, "utf8").trim().split("\n")).toEqual([plan.base.sha, base]); + expect(git(forge.origin, "rev-parse", `${forge.repo.pulls[3].squash}^`)).toBe(base); }); test("scopes each merge token to that PR's squash, preserving it through the queue token cache", async () => {