diff --git a/README.md b/README.md index fe3743b4..ed9557bd 100644 --- a/README.md +++ b/README.md @@ -561,15 +561,16 @@ PR in that batch, without bisection. Ejection disables auto-merge, removes `$STATE_DIRECTORY` or the XDG state home, never inside the checkout. A check that runs past `--timeout` minutes (60 by default) is killed and fails its batch. -Before batching, each queued PR is squashed alone onto the current base, and -`quest check` runs on the result. A PR with findings the base lacks, such as +As a batch is filled, each PR it reaches is squashed alone onto the current +base, and `quest check` runs on the result; PRs the batch does not reach wait +for a later round, so a long queue costs only each batch's candidates. A PR with findings the base lacks, such as a link to a quest the base has since finished, is ejected alone as `failed_checks` with those findings, so it never fails a batch; a configured fixer receives it as it would a failed batch, starting from that squash. This check runs in the daemon, never the sandbox: it parses quest files read from git objects and executes nothing from the tree. A base without a `quest/` directory skips -it. A newly queued PR is also squashed onto each PR ahead of it; if any -conflict, the daemon comments once with their numbers and files, so its author +it. A newly queued PR the batch reaches is also squashed onto the four PRs +landing soonest ahead of it; if any conflict, the daemon comments once with their numbers and files, so its author can resolve the overlap before its turn. It is screened again only once requeued. diff --git a/assets/skills/quest-close.md b/assets/skills/quest-close.md new file mode 100644 index 00000000..cda257d2 --- /dev/null +++ b/assets/skills/quest-close.md @@ -0,0 +1,9 @@ +--- +name: quest-close +description: Abandon a quest or PR. +--- + +Abandon the specified quest. + +If the quest has an open PR, repurpose it, otherwise make a new PR. +Delete the quest file and merge. diff --git a/assets/skills/quest-merge.md b/assets/skills/quest-merge.md new file mode 100644 index 00000000..d4d22ec7 --- /dev/null +++ b/assets/skills/quest-merge.md @@ -0,0 +1,19 @@ +--- +name: quest-merge +description: Merge a GitHub PR once reviews and CI pass. +--- + +Land a pull request. +If unsure about any course of action, interactively prompt the user for guidance. +Continue any non-blocking tasks asynchrnously. + +- Parse the arguments to determine the PR number, otherwise resolve it from the current context/branch. +- Fix any issues with the PR, such as merge conflicts and failing CI checks. +- If a draft, flip it to "Ready for Review" then wait for at least one automated review to complete. Never wait on CodeRabbit. +- Fix any review feedback you agree with, and leave a comment if you disagree. Don't wait for another review round after narrow fixes. + +If everything looks good, leave a summary of the changes made, then enable auto-merge with the full 40-character head SHA. +Enable it only once every open decision is resolved: prompt the user for each with your recommendation, or, as a background agent, list them in your report. +Record the outcome as a PR comment when it isn't already in the PR: each decision and its reason, and any follow-up the user declined. +When done, explain the result in a few lines and offer any follow-ups as a multi-select. +Never close a PR to unstick it, and never work around a refused merge; ask instead. diff --git a/src/cli/land.ts b/src/cli/land.ts index 3e74aec9..fc19f542 100644 --- a/src/cli/land.ts +++ b/src/cli/land.ts @@ -496,14 +496,12 @@ export interface BatchPolicy { } /** - * Fetch the base and every queued head, then screen each head and squash the - * rest in turn onto the chain in a scratch worktree inside the git directory, - * resolving quest files with `quest resolve`, in `policy`'s order. Stop at its batch - * limit, skipping conflicting additions; a conflicting base stops the batch. - * - * Screening squashes each head alone onto the base and rejects one that fails - * `quest check` there with findings the base lacks. Each head `fresh` accepts is - * also squashed onto each PR ahead of it, to report which ones it conflicts with. + * Fetch the base and every queued head, then fill a batch in `policy`'s order in a scratch + * worktree inside the git directory, resolving quest files with `quest resolve`. Each candidate + * is first squashed alone onto the base: one that fails `quest check` there with findings the + * base lacks is rejected, and a fresh one is also squashed onto the PRs landing soonest ahead of + * it, to report which ones it conflicts with. Stop at the policy's batch limit, skipping + * conflicting additions; a conflicting base stops the batch. */ export async function build( root: string, @@ -521,19 +519,28 @@ export async function build( ...plan.queue.map((entry) => entry.head), ]); return mergeWorktree(root, plan.base.sha, (dir, driver) => { - const chain: Chain = { links: [], ...screen(dir, driver, plan, fresh) }; - const queue = plan.queue.filter((entry) => !chain.rejected.some((rejection) => rejection.entry === entry)); - const ordered = policy.order === "newest" ? [...queue].reverse() : queue; - const limit = Math.max(1, Math.min(policy.size, 1 + Math.floor(Math.log2(queue.length)))); - git(dir, ["reset", "--quiet", "--hard", plan.base.sha]); + const chain: Chain = { links: [], rejected: [], overlaps: [] }; + const ordered = policy.order === "newest" ? [...plan.queue].reverse() : plan.queue; + const limit = Math.max(1, Math.min(policy.size, 1 + Math.floor(Math.log2(plan.queue.length)))); + const screening = screener(dir, driver, plan); + // Screen each head only as the batch reaches it, so a long queue costs this batch's + // candidates, not every queued PR on every round. for (const entry of ordered) { if (chain.links.length >= limit) break; - const files = squash(dir, driver, entry); - if (files !== null) { + const alone = screening.alone(entry); + if (alone === null) { if (chain.links.length > 0) continue; - chain.conflict = { entry, files }; + git(dir, ["reset", "--quiet", "--hard", plan.base.sha]); + chain.conflict = { entry, files: squash(dir, driver, entry)! }; break; } + if (alone.findings.length > 0) { + chain.rejected.push({ entry, commit: alone.commit, findings: alone.findings.map(formatFinding) }); + continue; + } + if (fresh(entry)) chain.overlaps.push({ entry, ahead: screening.overlaps(entry) }); + git(dir, ["reset", "--quiet", "--hard", chain.links.at(-1)?.commit ?? plan.base.sha]); + if (squash(dir, driver, entry) !== null) continue; chain.links.push({ entry, commit: git(dir, ["rev-parse", "HEAD"]), @@ -557,51 +564,52 @@ function squash(dir: string, driver: string, entry: Entry): string[] | null { return null; } +/** How many queued PRs ahead of a newly queued one its conflict report covers: those landing soonest. */ +const OVERLAP_AHEAD = 4; + /** - * Squash each queued head alone onto the base, reject those failing `quest check` there, and - * squash each fresh head onto each one ahead of it. A head conflicting with the base is left to - * the chain, which ejects it once it is the batch's base PR. + * Screening for one batch: each head squashed alone onto the base, with `quest check` findings + * the base lacks, and a fresh head's conflicts with the PRs landing soonest ahead of it. Results are + * memoized, since a head is squashed alone once per batch however often it is needed. */ -function screen( - dir: string, - driver: string, - plan: Plan, - fresh: (entry: Entry) => boolean, -): Pick { +function screener(dir: string, driver: string, plan: Plan) { // A base without quests is a repository that does not use them. const known = git(dir, ["ls-tree", "--name-only", plan.base.sha, "quest"]) === "" ? null : questCheck(dir, plan.base.sha); - const alone = new Map(); - const rejected: Rejection[] = []; - for (const entry of plan.queue) { + const cache = new Map(); + const alone = (entry: Entry) => { + if (cache.has(entry)) return cache.get(entry)!; git(dir, ["reset", "--quiet", "--hard", plan.base.sha]); - if (squash(dir, driver, entry) !== null) continue; - const commit = git(dir, ["rev-parse", "HEAD"]); - // A finding the base already has is not this PR's, even on a line the PR moved. - const findings = - known === null - ? [] - : questCheck(dir, commit).filter( - (finding) => !known.some((old) => old.path === finding.path && old.message === finding.message), - ); - if (findings.length > 0) rejected.push({ entry, commit, findings: findings.map(formatFinding) }); - else alone.set(entry, commit); - } - const overlaps: Overlap[] = []; - const ahead: Entry[] = []; - for (const entry of plan.queue) { - if (!alone.has(entry)) continue; - if (fresh(entry)) { - const conflicts = ahead.flatMap((earlier) => { - git(dir, ["reset", "--quiet", "--hard", alone.get(earlier)!]); + let result: { commit: string; findings: Finding[] } | null = null; + if (squash(dir, driver, entry) === null) { + const commit = git(dir, ["rev-parse", "HEAD"]); + // A finding the base already has is not this PR's, even on a line the PR moved. + const findings = + known === null + ? [] + : questCheck(dir, commit).filter( + (finding) => + !known.some((old) => old.path === finding.path && old.message === finding.message), + ); + result = { commit, findings }; + } + cache.set(entry, result); + return result; + }; + const overlaps = (entry: Entry) => { + const index = plan.queue.indexOf(entry); + return plan.queue + .slice(0, Math.max(0, index)) + .slice(0, OVERLAP_AHEAD) + .flatMap((earlier) => { + const base = alone(earlier); + if (base === null || base.findings.length > 0) return []; + git(dir, ["reset", "--quiet", "--hard", base.commit]); const files = squash(dir, driver, entry); return files === null ? [] : [{ entry: earlier, files }]; }); - overlaps.push({ entry, ahead: conflicts }); - } - ahead.push(entry); - } - return { rejected, overlaps }; + }; + return { alone, overlaps }; } /** diff --git a/src/cli/main.ts b/src/cli/main.ts index a96317c5..ee0a587e 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. Before batching, eject alone each PR whose squash onto the base fails quest check with findings the base lacks, and comment once on each newly queued PR that conflicts with PRs 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. Failed batches are ejected together, never bisected, and halve the next batch's size down to one PR; each landed batch grows it back by one. 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. As a batch is filled, eject alone 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. Failed batches are ejected together, never bisected, and halve the next batch's size down to one PR; each landed batch grows it back by one. 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.", ) .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 076fe6bf..0089b7c7 100644 --- a/test/land.test.ts +++ b/test/land.test.ts @@ -58,6 +58,10 @@ import { tempDir } from "./fixture"; const bin = fileURLToPath(new URL("../src/cli/bin.ts", import.meta.url)); const bunfig = fileURLToPath(new URL("../bunfig.toml", import.meta.url)); +// Each daemon test drives many git merges. On the land gate's host under load 28 (2026-10-31), +// tests that take 1.1-1.8 s unloaded took 5.2-6.8 s and failed vitest's 5 s default. +vi.setConfig({ testTimeout: 30_000 }); + const REPOSITORY = { owner: "upstream", repo: "project" }; const PATH = process.env.PATH; @@ -1072,6 +1076,19 @@ describe.skipIf(process.platform === "win32")("host daemon", () => { expect(linking.labels).toEqual([]); }); + test("screens only the PRs a batch reaches, leaving the rest of a long queue for later", async () => { + const forge = new Forge(); + forge.advance({ "quest/m0/README.md": EPIC(["a", "b"]), "quest/m0/b.md": QUEST("B") }); + for (let i = 0; i < 4; i++) forge.pull(`PR ${i}`, { [`${i}.txt`]: "ok" }); + forge.pull("Link b", { "quest/m0/a.md": `${QUEST("A")}\n## Related\n\n- [B](/quest/m0/b.md) - b\n` }); + // The base finishes b, so the last PR's link dangles, but an oldest-first batch of three never reaches it. + forge.advance({ "quest/m0/README.md": EPIC(["a"]), "quest/m0/b.md": null }); + const { checked, landing } = await forge.round(); + expect(checked).toEqual([[1, 2, 3]]); + expect(landing.map((line) => line.split(" ")[0])).toEqual(["landed", "landed", "landed"]); + expect(forge.repo.pulls[4].landing).toBe(true); + }); + test("blames no PR for a finding the base already has", async () => { const forge = new Forge(); forge.advance({ "quest/m0/a.md": `${QUEST("A")}\n## Related\n\n- [Gone](/quest/m0/gone.md) - gone\n` });