From 9f877534d32d1b7fd32b1c14bbcc6a5aaecaaa93 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 10:55:44 -0800 Subject: [PATCH 01/12] chore(quest): claim quest/m0/queue-watcher/own-queue.md Quest-Claim: 53fc21a70e48944d From 92fe30123ee5064fc83610b8b981a7620460f8a9 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 11:11:10 -0800 Subject: [PATCH 02/12] wip: own serial queue in quest land watch --- .github/workflows/queue.yml | 14 + src/cli/land-head.ts | 91 +++++ src/cli/land-queue.ts | 700 ++++++++++++++++++++++++++++++++++++ src/cli/land-serve.ts | 110 ++---- src/cli/land-watch-fix.ts | 61 ++-- src/cli/land-watch.ts | 158 +++++--- src/cli/land.ts | 31 +- src/cli/main.ts | 15 +- 8 files changed, 1019 insertions(+), 161 deletions(-) create mode 100644 src/cli/land-queue.ts diff --git a/.github/workflows/queue.yml b/.github/workflows/queue.yml index 1cc67106..b7209aac 100644 --- a/.github/workflows/queue.yml +++ b/.github/workflows/queue.yml @@ -10,6 +10,20 @@ name: Queue # also runs, as skipped, on main pull requests; a skipped job satisfies the # requirement. Only the merge group build runs code, and only a maintainer's # pick enters the queue. +# +# A repository without GitHub's merge queue, such as a private one on GitHub +# Team, lands through the watcher's own queue instead. Its base keeps requiring +# the unreported `quest land` context, and its queue workflow, under any name, +# also builds the watcher's queue branches: +# +# push: +# branches: ["quest-queue/**"] +# +# The watcher pushes `quest-queue//`, the base merged into PR +# 's picked head: a merge commit whose second parent (`HEAD^2`) is the +# base it tested. The build runs as for a merge group, and the base's required +# checks besides `quest land`, reported on that commit, decide whether the PR +# lands, so the required job reports the build's result there. on: pull_request: branches: [main] diff --git a/src/cli/land-head.ts b/src/cli/land-head.ts index 5163fafd..51862ca4 100644 --- a/src/cli/land-head.ts +++ b/src/cli/land-head.ts @@ -5,6 +5,7 @@ import { gitAuthentication } from "./land-git"; import { GitHub, type GitHubDependencies, type Repository, object, string } from "./github"; import { baseRemote, + type LandMethod, contains, eject, ejection, @@ -309,3 +310,93 @@ export async function landFallback( save(join(state, "fallback.json"), { plan, entry, commit: landed, parent }); return finishFallback(root, state, deps, token, read); } + +/** + * The merge needs workflow write when the commit landing it changes a workflow, or when any commit the PR + * brings changes one: GitHub refuses an App's merge of a PR whose commits touch workflows, such as a + * merge of a base that changed one, whether the gate or the author made it. + */ +export function touchesWorkflows(root: string, parent: string, commit: string, base: string, head: string): boolean { + if (git(root, ["diff", "--name-only", parent, commit, "--", ".github/workflows"]) !== "") return true; + // `-m` also lists a merge commit's changes against each parent, so a merged-in workflow change counts. + return git(root, ["log", "-m", "--format=", "--name-only", `${base}..${head}`, "--", ".github/workflows"]) !== ""; +} + +export type MergeResult = + { sha: string } | { reason: "head_moved" | "merge_conflict" | "merge_refused"; detail: string } | null; + +/** + * Retry only GitHub's pending mergeability or base refresh. Any other refusal, such as `method` being + * disabled or a rule the bot cannot bypass, would refuse every round after a full check, so it ejects. + * `previous` is the head before the gate pushed its own base merge as `head`, if it did. + */ +export async function mergePinned( + github: GitHub, + path: string, + method: LandMethod, + head: string, + previous = head, +): Promise { + for (let attempt = 0; ; attempt++) { + const response = await github.request(`${path}/merge`, { + method: "PUT", + body: { merge_method: method, sha: head }, + allow: [403, 405, 409], + }); + // The client waits out a rate limit; any other 403 is GitHub refusing this PR, which stops this PR + // alone rather than failing the whole batch on every retry. + const result = object(await response.json()); + if (response.ok && result.merged === true) return { sha: string(result.sha) }; + const pull = object(await (await github.request(path)).json()); + // A lost response or a manual merge is reconciled by rebuilding the remaining queue. + if (pull.merged === true) return null; + const detail = `GitHub refused ${path}: ${String(result.message)}`; + if (pull.state !== "open") return { reason: "merge_refused", detail }; + // 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 >= SETTLE_RETRIES) throw new Error(`${detail}; prepared head has not propagated`); + await github.pause(500 * 2 ** attempt); + continue; + } + if (current !== head) return { reason: "head_moved", detail }; + const conflict = pull.mergeable === false || pull.mergeable_state === "dirty"; + // Once GitHub shows the gate's own merge, it can still call it conflicting from the pin's stale + // mergeability, in its refusal or in the PR (moq-dev/quest#342). That merge already holds the + // base it was checked on, so it cannot conflict with it: wait. Only a base that moved since can + // conflict, and then the retries run out and the next round rebuilds on that base. + const stale = previous !== head && (conflict || /merge conflict/i.test(String(result.message))); + if (conflict && !stale) return { reason: "merge_conflict", detail }; + if (stale || pull.mergeable === null || /base branch was modified/i.test(String(result.message))) { + if (attempt >= SETTLE_RETRIES) throw new Error(`${detail}; mergeability did not settle`); + await github.pause(500 * 2 ** attempt); + continue; + } + return { reason: "merge_refused", detail }; + } +} + +/** + * What GitHub's commit `sha`, landing a PR with `method`, put on the base: its first parent, and how it + * differs from the tested `tree` and, for a merge commit, from `head` as its second parent. + */ +export async function landedCommit( + github: GitHub, + repository: string, + method: LandMethod, + sha: string, + tree: string, + head: string | null, +): Promise<{ parent: string | null; faults: string[] }> { + const commit = object(await (await github.request(`/repos/${repository}/git/commits/${sha}`)).json()); + const parents = Array.isArray(commit.parents) ? commit.parents.map((parent) => string(object(parent).sha)) : []; + const landed = string(object(commit.tree).sha); + const faults = [ + landed !== tree && `has tree ${landed}, but the tested one is ${tree}`, + method === "merge" && + (parents.length !== 2 || parents[1] !== head) && + `has parents ${parents.join(" ")}, but its second parent must be the landed head ${head}`, + ].filter((fault) => fault !== false); + return { parent: parents[0] ?? null, faults }; +} diff --git a/src/cli/land-queue.ts b/src/cli/land-queue.ts new file mode 100644 index 00000000..0a284235 --- /dev/null +++ b/src/cli/land-queue.ts @@ -0,0 +1,700 @@ +// The watcher's own serial queue, for a base without GitHub's merge queue, such as a private repository on +// GitHub Team: test one pick at a time on top of the base in the repository's queue workflow, then land +// exactly the tested tree with the App. +import { existsSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { + GitHub, + type GitHubDependencies, + object, + type RequiredCheck, + type Repository, + requirements, + string, +} from "./github"; +import { + baseRemote, + eject, + ejection, + type Entry, + git, + LAND_CHECK, + type LandMethod, + mergeBase, + type Plan, + type RemoteRead, + requiredChecks, + tryGit, +} from "./land"; +import { fixerText, post } from "./land-fixer"; +import { gitAuthentication } from "./land-git"; +import { + finishFallback, + forgetHeads, + landedCommit, + landFallback, + mergePinned, + prepareHead, + push, + remoteHead, + resolvePreparedQueue, + touchesWorkflows, +} from "./land-head"; +import { + account, + AFTER, + because, + type FailedJob, + failingJobs, + jobsSection, + listed, + login, + type QueuePick, + type Removal, +} from "./land-watch"; +import { EXCERPT, excerpt, save } from "./land-serve"; +import { advanceFixes, type FixContext, isHeld, readPull, readRemoval, requeuedHeads } from "./land-watch-fix"; + +/** The check run each pick in the own queue carries, showing its place; informational, never a required context. */ +export const QUEUE_CHECK = "quest queue"; + +/** The branch each build is pushed to, outside the reserved `quest/` namespace, which the queue workflow builds. */ +export const queueBranch = (base: string, number: number) => `quest-queue/${base}/${number}`; + +/** What the own queue needs beyond the API: the daemon's private repository, its pushes, and its reads. */ +export interface OwnQueue { + root: string; + state: string; + method: LandMethod; + /** Minutes a build may run before it is dropped as timed out. */ + timeout: number; + token: (workflows: boolean) => Promise; + read: RemoteRead; + /** Whether this process has pruned the queue branches an earlier one left behind. */ + pruned?: boolean; +} + +type DropReason = "failed_checks" | "checks_timed_out" | "merge_conflict"; + +/** The pick under test, in `queue.json`, from its build's start until it lands, is dropped, or leaves the queue. */ +interface Build { + /** The pick as the queue resolved it when the build started; `approved` is the head under test. */ + entry: Entry; + /** The pick it belongs to, which a drop hands the fixer. */ + pick: QueuePick; + /** The base it is built on. */ + base: string; + branch: string; + /** The base merged into the pick, as pushed to `branch`; absent for a conflict. */ + test?: { commit: string; tree: string }; + started: string; + /** A drop being recorded, so a restart finishes it: the fixer's removal, and the PR's comment and its jobs. */ + drop?: { removal: Removal; comment: string; blocks: string[] }; +} + +/** What a PR's queue check shows. */ +type View = { + status: "queued" | "in_progress" | "completed"; + conclusion?: "success" | "failure" | "neutral"; + title: string; + summary: string; + url?: string; +}; + +/** The queue check each PR last got, in `queue-checks.json`, so a pass writes only what changed. */ +interface Shown { + head: string; + id: number; + status: View["status"]; + title: string; +} + +const buildPath = (state: string) => join(state, "queue.json"); +const shownPath = (state: string) => join(state, "queue-checks.json"); + +function readBuild(state: string): Build | undefined { + const path = buildPath(state); + return existsSync(path) ? (JSON.parse(readFileSync(path, "utf8")) as Build) : undefined; +} + +/** One pass of the own queue, and the PRs it changed. `again` asks for the next pass at once, as after a landing. */ +interface Pass { + lines: string[]; + again: boolean; +} + +/** The pass's GitHub, its repository's name, the App's login, and the queue's own settings. */ +class Queue { + readonly github: GitHub; + readonly name: string; + readonly remote: string; + readonly lines: string[] = []; + private shown: Record; + + constructor( + readonly repository: Repository, + readonly base: string, + readonly queue: OwnQueue, + readonly deps: GitHubDependencies, + readonly app: string, + readonly fixing: boolean, + ) { + this.github = new GitHub(deps); + this.name = `${repository.owner}/${repository.repo}`; + this.remote = baseRemote({ repository: this.name }); + const path = shownPath(queue.state); + this.shown = existsSync(path) ? (JSON.parse(readFileSync(path, "utf8")) as Record) : {}; + } + + /** Show `view` on PR `number`'s head `head`, writing only a change, and a new check run once one completed. */ + async show(number: number, head: string, view: View): Promise { + const shown = this.shown[number]; + if (shown?.head === head && shown.status === view.status && shown.title === view.title) return; + // A head the PR left, as for the queue's own base merge before it lands, keeps no unfinished check. + if (shown && shown.head !== head && shown.status !== "completed") + await this.github.request(`/repos/${this.name}/check-runs/${shown.id}`, { + method: "PATCH", + body: { + status: "completed", + conclusion: "neutral", + output: { title: "Moved", summary: `This pull request's head moved to ${head}.` }, + }, + }); + const body = { + status: view.status, + ...(view.conclusion ? { conclusion: view.conclusion } : {}), + ...(view.url ? { details_url: view.url } : {}), + output: { title: view.title, summary: view.summary }, + }; + let id: number; + if (shown?.head === head && shown.status !== "completed") { + await this.github.request(`/repos/${this.name}/check-runs/${shown.id}`, { method: "PATCH", body }); + id = shown.id; + } else { + const response = await this.github.request(`/repos/${this.name}/check-runs`, { + body: { name: QUEUE_CHECK, head_sha: head, ...body }, + }); + id = Number(object(await response.json()).id); + } + this.shown[number] = { head, id, status: view.status, title: view.title }; + save(shownPath(this.queue.state), this.shown); + } + + /** Complete the queue check of each PR that left the queue without an outcome, and forget completed ones. */ + async leave(open: Set, why: string): Promise { + for (const [key, shown] of Object.entries(this.shown)) { + const number = Number(key); + if (open.has(number)) continue; + if (shown.status !== "completed") + await this.show(number, shown.head, { + status: "completed", + conclusion: "neutral", + title: "Left the queue", + summary: why, + }); + delete this.shown[number]; + save(shownPath(this.queue.state), this.shown); + } + } + + /** Cancel every unfinished run on a build's branch, and delete the branch. */ + async clear(branch: string): Promise { + const runs = await listed( + this.github, + `/repos/${this.name}/actions/runs?branch=${encodeURIComponent(branch)}&per_page=100`, + "workflow_runs", + ); + for (const run of runs.filter((item) => item.status !== "completed")) + await this.github.request(`/repos/${this.name}/actions/runs/${Number(run.id)}/cancel`, { + method: "POST", + allow: [409], + }); + await this.delete(`refs/heads/${branch}`); + } + + /** Delete `ref` from the repository, leased to the commit it holds now, if it exists. */ + async delete(ref: string): Promise { + const sha = await remoteHead(this.queue.root, this.remote, ref, this.queue.read); + if (sha === null) return; + const deleted = tryGit( + this.queue.root, + ["push", "--quiet", this.remote, `:${ref}`, `--force-with-lease=${ref}:${sha}`], + { + env: gitAuthentication(await this.queue.token(false)), + }, + ); + if (deleted.status !== 0) throw new Error(`deleting ${ref} failed: ${deleted.stderr.trim()}`); + } + + /** End `build`: its runs and branch go, its PR shows `view` on `head` if given, and the queue forgets it. */ + async end(build: Build, view?: View, head = build.entry.head): Promise { + await this.clear(build.branch); + if (view) await this.show(build.entry.number, head, view); + rmSync(buildPath(this.queue.state), { force: true }); + } + + /** Delete the queue branches of this base that no build holds, as a crashed daemon leaves them. */ + async prune(keep?: string): Promise { + const prefix = `refs/heads/${queueBranch(this.base, 0).replace(/0$/, "")}`; + const listing = await this.queue.read(this.queue.root, ["ls-remote", this.remote, `${prefix}*`]); + for (const line of listing.split("\n").filter(Boolean)) { + const ref = line.split("\t")[1]; + if (ref.startsWith(prefix) && ref !== `refs/heads/${keep}`) await this.delete(ref); + } + } + + /** + * The pick an armed PR belongs to: its arm, unless the App armed a head the watcher put back in the + * queue for a pick still going, which that arm continues. + */ + async pickOf(entry: Entry): Promise { + const data = await this.github.graphql( + "query($id: ID!) { node(id: $id) { ... on PullRequest { autoMergeRequest { enabledAt enabledBy { login } } } } }", + { id: entry.id }, + ); + const request = object(data.node).autoMergeRequest; + const by = request == null ? "" : login(object(request).enabledBy); + const own = requeuedHeads(this.queue.state, entry.number); + if (own && account(by) === this.app && own.heads.includes(entry.approved) && entry.enabledAt > own.pick) + return { by, at: own.pick }; + return { by, at: entry.enabledAt }; + } + + /** + * Start testing `entry` on `plan`'s base: merge the base into the pick with the quest merge driver, as + * the land fixer merges a conflict, push it to the pick's queue branch for the queue workflow, and + * record it. A conflict the driver cannot merge drops the pick. + */ + async start(plan: Plan, entry: Entry): Promise { + const { root, read, token } = this.queue; + const build: Build = { + entry, + pick: await this.pickOf(entry), + base: plan.base.sha, + branch: queueBranch(this.base, entry.number), + started: new Date(this.deps.now()).toISOString(), + }; + await read(root, ["fetch", "--quiet", "--no-tags", this.remote, plan.base.sha, entry.approved]); + const merged = mergeBase(root, entry.approved, plan.base.sha); + if ("reason" in merged) { + await this.drop(build, "merge_conflict", [], merged.detail); + return undefined; + } + const ref = `refs/heads/${build.branch}`; + // A branch an earlier build of this PR left behind, as before a crash, is replaced. + const lease = (await remoteHead(root, this.remote, ref, read)) ?? ""; + const workflows = + git(root, ["diff", "--name-only", plan.base.sha, merged.head, "--", ".github/workflows"]) !== ""; + const pushed = await push( + root, + this.remote, + entry.approved, + merged.head, + ref, + (more) => token(more || workflows), + lease, + ); + if (pushed.status !== 0) throw new Error(`pushing ${build.branch} failed: ${pushed.stderr.trim()}`); + build.test = { commit: merged.head, tree: merged.tree }; + save(buildPath(this.queue.state), build); + this.lines.push(`building ${entry.number} ${merged.head}`); + return build; + } + + /** + * Drop `build`'s pick, as GitHub's queue removes a PR: record it for the land fixer, disarm its + * auto-merge, and say why on the PR. The drop is recorded first, so a restart finishes it. + */ + async drop(build: Build, reason: DropReason, jobs: (FailedJob & { log: string })[], detail = ""): Promise { + const at = new Date(this.deps.now()).toISOString(); + const { entry } = build; + const number = entry.number; + const head = entry.head; + const log = join(this.queue.state, `removal-${number}-${at.replace(/[^0-9TZ]/g, "")}.log`); + writeFileSync( + log, + [ + `The queue removed ${entry.url} at ${head} for ${reason}, on ${this.base} at ${build.base}.`, + ...(detail ? [`\n${detail}`] : []), + ...jobs.map( + (job) => + `\n== ${job.workflow} / ${job.name}: ${job.conclusion}${job.step ? ` at step ${job.step}` : ""} (${job.url}) ==\n${job.log}`, + ), + "", + ].join("\n"), + { mode: 0o600 }, + ); + const fixed: Entry = { ...entry, enabledAt: build.pick.at, approved: head }; + const removal: Removal = { + event: `queue ${number} ${at}`, + at, + number, + url: entry.url, + reason, + actor: this.app, + head, + current: head, + pick: build.pick, + verdict: "fix", + jobs: jobs.map(({ log: _, ...job }) => job), + log, + failure: { + plan: { repository: this.name, base: { ref: this.base, sha: build.base }, queue: [fixed], eject: [] }, + ejections: [], + numbers: [number], + log, + tip: head, + reason: reason === "merge_conflict" ? "merge_conflict" : "failed_checks", + blame: [number], + }, + }; + const share = Math.floor(EXCERPT / Math.max(1, jobs.length)); + const blocks = jobs.map( + (job) => + `\n\n${fixerText(`${job.workflow} / ${job.name}`)} ([job](${job.url})) ${job.conclusion}${job.step ? ` at step ${fixerText(job.step)}` : ""}${job.log ? `:\n\n
${excerpt(job.log.trimEnd(), share)}
` : "."}`, + ); + const missing = + jobs.length === 0 && reason !== "merge_conflict" + ? "\n\nThe watcher found no failing job in the queue build's runs." + : ""; + const comment = `quest land watch: the queue removed this pull request at ${head} because ${because(reason, this.app, this.base, "queue")} (\`${reason}\`).${AFTER.fix(head, this.fixing)}${missing}`; + build.drop = { removal, comment, blocks }; + save(buildPath(this.queue.state), build); + await this.finishDrop(build); + } + + /** Finish recording `build`'s drop; each step is safe to repeat after a restart. */ + async finishDrop(build: Build): Promise { + const { removal, comment, blocks } = build.drop!; + const number = removal.number; + // A newer removal's record, as one the fixer took since, stays. + if ((readRemoval(this.queue.state, number)?.at ?? "") <= removal.at) + save(join(this.queue.state, `removal-${number}.json`), removal); + const pull = await readPull(this.github, build.entry.id); + const request = pull.autoMergeRequest == null ? null : object(pull.autoMergeRequest); + if (pull.state === "OPEN" && request !== null && string(request.enabledAt) === build.entry.enabledAt) + await this.github.graphql( + "mutation($id: ID!) { disablePullRequestAutoMerge(input: { pullRequestId: $id }) { clientMutationId } }", + { id: build.entry.id }, + ); + await post( + this.github, + { repository: this.name }, + { number, enabledAt: removal.pick.at }, + comment, + `removed ${removal.event}`, + "quest land watch", + undefined, + (room) => jobsSection(blocks, room), + ); + await this.end(build, { + status: "completed", + conclusion: "failure", + title: "Removed from the queue", + summary: `The queue removed this pull request for \`${removal.reason}\`; its comment says what happens next.`, + }); + this.lines.push(`removed ${number} ${removal.reason}`); + } + + /** + * Land `build`'s passing pick through `quest land serve`'s landing path: lease-push the base merge to + * the PR, merge it with GitHub's merge API pinned to that head, and verify the landed tree is the tested + * one, or fast-forward the base to the tested commit when the PR's branch refuses the push. + */ + async land(build: Build, plan: Plan, entry: Entry): Promise { + const { root, state, method, token, read } = this.queue; + const number = entry.number; + const test = build.test!; + const prepared = await prepareHead(root, state, plan, entry, build.base, test.tree, token, read); + if ("fallback" in prepared) { + // The tested tree, landed onto the tested base as GitHub's merge API would. + const parents = method === "merge" ? [build.base, entry.approved] : [build.base]; + const commit = git(root, ["commit-tree", test.tree, ...parents.flatMap((parent) => ["-p", parent])], { + input: `${entry.title} (#${number})`, + }); + const done = await landFallback(root, state, plan, entry, commit, build.base, this.deps, token, read); + return this.fallen(build, done); + } + if ("reason" in prepared) return this.refused(build, plan, entry, prepared.reason, prepared.detail); + const merge = new GitHub({ + ...this.deps, + token: await token(touchesWorkflows(root, build.base, test.commit, build.base, prepared.head)), + }); + const result = await mergePinned( + merge, + `/repos/${this.name}/pulls/${number}`, + method, + prepared.head, + entry.head, + ); + if (result === null) { + // GitHub shows it merged, as after a lost response: nothing is left to land. + forgetHeads(state, number); + this.lines.push(`merged ${number}`); + return this.end(build, { + status: "completed", + conclusion: "success", + title: "Merged", + summary: "GitHub shows this pull request merged.", + }); + } + if ("reason" in result) return this.refused(build, plan, entry, result.reason, result.detail); + forgetHeads(state, number); + const landed = await landedCommit(merge, this.name, method, result.sha, test.tree, prepared.head); + // The ruleset leaves the watcher the base's only writer, so a moved parent is an admin's bypass. + if (landed.parent !== build.base) + this.deps.warn( + `quest land watch: the base moved before ${method} #${number}: expected parent ${build.base}, got ${landed.parent}`, + ); + if (landed.faults.length > 0) + this.deps.warn( + `CRITICAL: GitHub ${method} #${number} at ${result.sha} ${landed.faults.join(", and ")}. It stays merged`, + ); + this.lines.push(`landed ${number} ${result.sha}`); + await this.end(build, landedView(result.sha), prepared.head); + } + + /** Settle a fallback landing, finished or not. */ + async fallen(build: Build, done: string | null): Promise { + if (done) this.lines.push(done); + if (done?.startsWith("landed ")) return this.end(build, landedView(done.split(" ")[2])); + // The base moved or the pick left the queue: the next pass rebuilds or forgets it. A refused push + // ejected the PR, with its comment. + return this.end( + build, + done?.startsWith("ejected ") + ? { + status: "completed", + conclusion: "failure", + title: "Removed from the queue", + summary: "GitHub refused the push of the tested commit to the base.", + } + : undefined, + ); + } + + /** Settle a landing GitHub refused, or one that found the pick changed. */ + async refused(build: Build, plan: Plan, entry: Entry, reason: string, detail: string): Promise { + this.lines.push(`refused ${entry.number} ${reason}`); + // A conflict with a head that holds the tested base comes from a base that moved since: rebuild. + if (reason === "merge_conflict") return this.end(build); + // A writer's push ends the pick; the next pass ejects it for the moved head. + if (reason === "head_moved") + return this.end(build, { + status: "completed", + conclusion: "neutral", + title: "Left the queue", + summary: detail, + }); + if (reason === "tree_mismatch") this.deps.warn(`CRITICAL: ${detail}`); + await eject( + this.github, + plan, + ejection(entry, reason === "tree_mismatch" ? "tree_mismatch" : "merge_refused", excerpt(detail)), + ); + forgetHeads(this.queue.state, entry.number); + await this.end(build, { + status: "completed", + conclusion: "failure", + title: "Removed from the queue", + summary: excerpt(detail), + }); + } + + /** Where `build`'s required checks stand on its test commit, and the runs on its branch. */ + async status(build: Build, required: RequiredCheck[]) { + const commit = build.test!.commit; + const checks = await requiredChecks(this.github, this.repository, commit, required); + const runs = ( + await listed( + this.github, + `/repos/${this.name}/actions/runs?head_sha=${commit}&per_page=100`, + "workflow_runs", + ) + ).filter((run) => run.head_branch === build.branch); + return { ...checks, runs }; + } +} + +const landedView = (sha: string): View => ({ + status: "completed", + conclusion: "success", + title: "Landed", + summary: `The queue landed this pull request as ${sha}.`, +}); + +const ordinal = (index: number, count: number) => `Position ${index + 1} of ${count}`; + +/** + * One pass of the own queue over `base`: finish a drop or a fallback landing a restart interrupted, + * eject picks GitHub's own records rule out, advance the build in progress (land it once its required + * checks pass on the test commit, drop it for the land fixer once one fails or it runs past its timeout, + * rebuild it on a moved base), start the next pick when none builds, and show each pick its place. + * With a fixer, it then advances the fixer's rounds, which put fixed picks back by arming auto-merge. + */ +export async function ownPass( + repository: Repository, + base: string, + queue: OwnQueue, + deps: GitHubDependencies, + fix?: FixContext, +): Promise { + const github = new GitHub(deps); + const app = account(string(object((await github.graphql("query { viewer { login } }", {})).viewer).login)); + const own = new Queue(repository, base, queue, deps, app, fix !== undefined); + const { state } = queue; + let build = readBuild(state); + let again = false; + if (build?.drop) { + await own.finishDrop(build); + build = undefined; + again = true; + } + if (!queue.pruned) { + await own.prune(build?.branch); + queue.pruned = true; + } + const fallen = await finishFallback(queue.root, state, deps, queue.token, queue.read); + if (fallen && build && fallen.split(" ")[1] === String(build.entry.number)) { + await own.fallen(build, fallen); + build = undefined; + again = true; + } else if (fallen) own.lines.push(fallen); + + const required = (await requirements(github, repository, base)).checks.filter((check) => check.name !== LAND_CHECK); + if (required.length === 0) + throw new Error( + `${own.name}'s ${base} requires no check besides ${LAND_CHECK}, so the queue has no build result to wait for`, + ); + const plan = await resolvePreparedQueue(queue.root, state, repository, { base, method: queue.method }, deps); + for (const item of plan.eject) { + if (await eject(github, plan, item)) own.lines.push(`ejected ${item.number} ${item.reason}`); + forgetHeads(state, item.number); + } + const held = new Set(); + for (const entry of plan.queue) if (isHeld(await readPull(github, entry.id))) held.add(entry.number); + + if (build) { + const tested = build; + const entry = plan.queue.find( + (item) => + item.number === tested.entry.number && + item.enabledAt === tested.entry.enabledAt && + item.approved === tested.entry.approved, + ); + if (!entry || held.has(entry.number)) { + // The pick ended: a person disarmed it, a writer pushed, it closed, or another session holds it, + // which leaves it armed for when the hold clears. + await own.end(build, { + status: "completed", + conclusion: "neutral", + title: "Left the queue", + summary: entry ? "Another session holds this pull request." : "The pick ended while it was building.", + }); + own.lines.push(`left ${build.entry.number}`); + build = undefined; + again = true; + } else if (plan.base.sha !== build.base) { + // The base moved: build the pick again on the new base, as no attempt of its own. + await own.clear(build.branch); + own.lines.push(`rebuilding ${entry.number} on ${plan.base.sha}`); + build = await own.start(plan, entry); + again ||= build === undefined; + } else { + const status = await own.status(build, required); + const url = status.runs.map((run) => string(run.html_url)).at(0); + if (status.failing.length > 0) { + await own.drop(build, "failed_checks", await failingJobs(github, own.name, status.runs, false)); + build = undefined; + again = true; + } else if (status.pending.length > 0) { + if (deps.now() - Date.parse(build.started) >= queue.timeout * 60_000) { + await own.drop(build, "checks_timed_out", await failingJobs(github, own.name, status.runs, true)); + build = undefined; + again = true; + } else + await own.show(entry.number, entry.head, { + status: "in_progress", + title: `Building on ${base}`, + summary: `The queue is building this pull request merged with ${base} at ${build.base}, waiting on ${status.pending.map((name) => `\`${name}\``).join(", ")}.`, + ...(url ? { url } : {}), + }); + } else { + await own.land(build, plan, entry); + build = undefined; + again = true; + } + } + } + + // The next pick waits for the next pass once one landed, since the base it builds on moved. + if (!build && !own.lines.some((line) => line.startsWith("landed "))) { + for (const entry of plan.queue) { + if (held.has(entry.number)) continue; + build = await own.start(plan, entry); + if (build) break; + again = true; + } + } + + const open = new Set(); + const picks = plan.queue.filter((entry) => !held.has(entry.number)); + for (const entry of plan.queue) { + open.add(entry.number); + if (entry.number === build?.entry.number) { + if (own.lines.some((line) => line.startsWith(`building ${entry.number} `))) + await own.show(entry.number, entry.head, { + status: "in_progress", + title: `Building on ${base}`, + summary: `The queue is building this pull request merged with ${base} at ${build.base}.`, + }); + continue; + } + await own.show( + entry.number, + entry.head, + held.has(entry.number) + ? { + status: "queued", + title: "Held", + summary: "Another session holds this pull request; it keeps its place until the hold clears.", + } + : { + status: "queued", + title: ordinal(picks.indexOf(entry), picks.length), + summary: `The queue tests one pull request at a time on top of ${base}, in the order they were picked; ${picks.indexOf(entry)} ahead of this one.`, + }, + ); + } + // A pick waiting on its own required checks keeps the check it last showed. + for (const waiting of plan.waiting ?? []) open.add(waiting.number); + await own.leave(open, "The pick ended: its auto-merge was disabled, or it closed."); + if (fix) + own.lines.push( + ...(await advanceFixes(own.name, plan.base, state, fix, deps, { kind: "own", method: queue.method })), + ); + return { lines: own.lines, again }; +} + +/** + * Leave the own queue once the base has GitHub's merge queue: finish a drop a restart interrupted, end + * the build in progress, and complete every queue check. A pass that finds no own queue state does nothing. + */ +export async function leaveOwnQueue( + repository: Repository, + base: string, + queue: OwnQueue, + deps: GitHubDependencies, + fixing: boolean, +): Promise { + const build = readBuild(queue.state); + if (!build && !existsSync(shownPath(queue.state))) return []; + const github = new GitHub(deps); + const app = account(string(object((await github.graphql("query { viewer { login } }", {})).viewer).login)); + const own = new Queue(repository, base, queue, deps, app, fixing); + if (build?.drop) await own.finishDrop(build); + else if (build) await own.end(build); + await own.leave(new Set(), `${base} lands through GitHub's merge queue now.`); + rmSync(shownPath(queue.state), { force: true }); + return [...own.lines, "left the own queue"]; +} diff --git a/src/cli/land-serve.ts b/src/cli/land-serve.ts index b3a37030..b008b3a3 100644 --- a/src/cli/land-serve.ts +++ b/src/cli/land-serve.ts @@ -21,7 +21,16 @@ import { homedir } from "node:os"; import { join, resolve } from "node:path"; import { setTimeout as sleep } from "node:timers/promises"; import { GitHub, type GitHubDependencies, type Repository, object, string } from "./github"; -import { forgetHeads, prepareHead, resolvePreparedQueue, landFallback, finishFallback } from "./land-head"; +import { + finishFallback, + forgetHeads, + landedCommit, + landFallback, + mergePinned, + prepareHead, + resolvePreparedQueue, + touchesWorkflows, +} from "./land-head"; import { type BatchPolicy, baseRemote, @@ -31,7 +40,6 @@ import { ejection, git, type LandMethod, - SETTLE_RETRIES, readRemote, type Overlap, type RemoteRead, @@ -87,30 +95,12 @@ export async function installationToken( ); } -type Chain = Awaited>; /** Replace `path` atomically, readable only by the daemon's user. */ export function save(path: string, value: unknown): void { writeFileSync(`${path}.new`, JSON.stringify(value), { mode: 0o600 }); renameSync(`${path}.new`, path); } -/** - * The merge needs workflow write when this PR's landed commit changes a workflow, or when any commit the PR - * brings changes one: GitHub refuses an App's merge of a PR whose commits touch workflows, such as a - * merge of a base that changed one, whether the gate or the author made it. - */ -export function touchesWorkflows( - root: string, - parent: string, - link: Chain["links"][number], - base: string, - head: string, -): boolean { - if (git(root, ["diff", "--name-only", parent, link.commit, "--", ".github/workflows"]) !== "") return true; - // `-m` also lists a merge commit's changes against each parent, so a merged-in workflow change counts. - return git(root, ["log", "-m", "--format=", "--name-only", `${base}..${head}`, "--", ".github/workflows"]) !== ""; -} - /** A fresh bare repository with no config, objects, or alternates inherited from the user's checkout. */ export function privateRepository(state: string): string { const path = join(state, "repository.git"); @@ -343,61 +333,6 @@ async function reportStalled(github: GitHub, plan: Plan, state: string, now: num return output; } -type MergeResult = - { sha: string } | { reason: "head_moved" | "merge_conflict" | "merge_refused"; detail: string } | null; - -/** - * Retry only GitHub's pending mergeability or base refresh. Any other refusal, such as `method` being - * disabled or a rule the bot cannot bypass, would refuse every round after a full check, so it ejects. - * `previous` is the head before the gate pushed its own base merge as `head`, if it did. - */ -async function mergePinned( - github: GitHub, - path: string, - method: LandMethod, - head: string, - previous = head, -): Promise { - for (let attempt = 0; ; attempt++) { - const response = await github.request(`${path}/merge`, { - method: "PUT", - body: { merge_method: method, sha: head }, - allow: [403, 405, 409], - }); - // The client waits out a rate limit; any other 403 is GitHub refusing this PR, which stops this PR - // alone rather than failing the whole batch on every retry. - const result = object(await response.json()); - if (response.ok && result.merged === true) return { sha: string(result.sha) }; - const pull = object(await (await github.request(path)).json()); - // A lost response or a manual merge is reconciled by rebuilding the remaining queue. - if (pull.merged === true) return null; - const detail = `GitHub refused ${path}: ${String(result.message)}`; - if (pull.state !== "open") return { reason: "merge_refused", detail }; - // 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 >= SETTLE_RETRIES) throw new Error(`${detail}; prepared head has not propagated`); - await github.pause(500 * 2 ** attempt); - continue; - } - if (current !== head) return { reason: "head_moved", detail }; - const conflict = pull.mergeable === false || pull.mergeable_state === "dirty"; - // Once GitHub shows the gate's own merge, it can still call it conflicting from the pin's stale - // mergeability, in its refusal or in the PR (moq-dev/quest#342). That merge already holds the - // base it was checked on, so it cannot conflict with it: wait. Only a base that moved since can - // conflict, and then the retries run out and the next round rebuilds on that base. - const stale = previous !== head && (conflict || /merge conflict/i.test(String(result.message))); - if (conflict && !stale) return { reason: "merge_conflict", detail }; - if (stale || pull.mergeable === null || /base branch was modified/i.test(String(result.message))) { - if (attempt >= SETTLE_RETRIES) throw new Error(`${detail}; mergeability did not settle`); - await github.pause(500 * 2 ** attempt); - continue; - } - return { reason: "merge_refused", detail }; - } -} - /** One batch: check once, then land through GitHub in order, verifying each resulting commit. */ const POLICY = "batch-policy.json"; @@ -675,7 +610,7 @@ export async function serveBatch( ? new GitHub(deps) : new GitHub({ ...deps, - token: await token(touchesWorkflows(root, parent, link, plan.base.sha, prepared.head)), + token: await token(touchesWorkflows(root, parent, link.commit, plan.base.sha, prepared.head)), }); const result = "reason" in prepared @@ -714,9 +649,16 @@ export async function serveBatch( const sha = result.sha; forgetHeads(options.state, link.entry.number); output.push(`landed ${link.entry.number} ${sha}`); - const commit = object(await (await merge.request(`/repos/${plan.repository}/git/commits/${sha}`)).json()); - const parents = Array.isArray(commit.parents) ? commit.parents.map((parent) => string(object(parent).sha)) : []; - const actualParent = parents[0] ?? null; + // GitHub merged the head the gate pinned, so its merge commit's second parent is that head: the + // approved pin, or the gate's own base merge on it. + const { parent: actualParent, faults } = await landedCommit( + merge, + plan.repository, + plan.method, + sha, + link.tree, + "head" in prepared ? prepared.head : null, + ); if (actualParent !== expectedParent) { const message = `quest land: base moved before ${plan.method} #${link.entry.number}: expected parent ${expectedParent}, got ${actualParent}; rebuilding the remaining queue`; appendFileSync(log, `${message}\n`); @@ -724,16 +666,6 @@ export async function serveBatch( return output; } expectedParent = sha; - const tree = string(object(commit.tree).sha); - // GitHub merged the head the gate pinned, so its merge commit's second parent is that head: the - // approved pin, or the gate's own base merge on it. - const landed = "head" in prepared ? prepared.head : null; - const faults = [ - tree !== link.tree && `has tree ${tree}, but the tested chain has ${link.tree}`, - plan.method === "merge" && - (parents.length !== 2 || parents[1] !== landed) && - `has parents ${parents.join(" ")}, but its second parent must be the landed head ${landed}`, - ].filter((fault) => fault !== false); if (faults.length === 0) continue; // The PR 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. diff --git a/src/cli/land-watch-fix.ts b/src/cli/land-watch-fix.ts index 744ef2cb..8869a29f 100644 --- a/src/cli/land-watch-fix.ts +++ b/src/cli/land-watch-fix.ts @@ -6,7 +6,7 @@ import { isHoldLabel } from "../core"; import { array, GitHub, type GitHubDependencies, GitHubGraphQLError, object, type ObjectValue, string } from "./github"; import { type Queued, queueHead } from "../github/merge"; import { UNREFUSED } from "./hold"; -import { baseRemote, type Entry, mergeBase, type RemoteRead } from "./land"; +import { baseRemote, enqueue, type Entry, type LandMethod, mergeBase, type RemoteRead } from "./land"; import { resolutionSection, type Resolution } from "./land-apply"; import { checksSection } from "./land-checks"; import { type Cause, EFFORT_VARIABLE, type FixResult, parseFixResult, readFixResult } from "./land-fix"; @@ -101,6 +101,12 @@ interface WatchJob extends Process { failures?: number; } +/** + * The queue a pass puts PRs back in: GitHub's merge queue, or the watcher's own, whose picks are + * auto-merge requests pinned to their heads, as `quest merge --land` arms them, with `method`. + */ +export type QueueMode = { kind: "github" } | { kind: "own"; method: LandMethod }; + const JOB = /^watch-job-.*\.json$/; const REMOVAL = /^removal-\d+\.json$/; /** The merge driver's own result, not an agent's: it is recorded in the PR's Review under this name. */ @@ -218,10 +224,11 @@ export async function advanceFixes( state: string, context: FixContext, deps: GitHubDependencies, + queue: QueueMode, ): Promise { const github = new GitHub(deps); // Collect first, so a finished job's slot is free, and a clean merge once it is made. - const output = await collectAll(github, state, context, deps); + const output = await collectAll(github, state, context, deps, queue); const waiting = readdirSync(state) .filter((name) => REMOVAL.test(name)) .map((name) => JSON.parse(readFileSync(join(state, name), "utf8")) as Removal) @@ -233,8 +240,8 @@ export async function advanceFixes( if (line?.startsWith("fixing ")) running++; if (line) output.push(line); } - output.push(...(await collectAll(github, state, context, deps, true))); - output.push(...(await advanceDisables(github, repository, state, context, deps))); + output.push(...(await collectAll(github, state, context, deps, queue, true))); + output.push(...(await advanceDisables(github, repository, state, context, deps, queue))); return output; } @@ -247,6 +254,7 @@ async function collectAll( state: string, context: FixContext, deps: GitHubDependencies, + queue: QueueMode, merges = false, ): Promise { const output: string[] = []; @@ -254,7 +262,7 @@ async function collectAll( if (merges && !job.merged) continue; const path = join(state, name); try { - output.push(...(await collect(github, state, path, job, context, deps))); + output.push(...(await collect(github, state, path, job, context, deps, queue))); } catch (error) { // A round that keeps failing, as when GitHub refuses its handback, is set aside, so it frees its // fixer slot for the other dropped PRs. Only the pass's full collection counts, once. @@ -278,7 +286,7 @@ async function collectAll( job.handback ??= `fixer collection failed: ${String(error)}`; try { save(path, job); - output.push(...(await collect(github, state, path, job, context, deps))); + output.push(...(await collect(github, state, path, job, context, deps, queue))); } catch (failure) { deps.warn(`quest land watch: handback pending for ${name}: ${String(failure)}`); } @@ -463,6 +471,7 @@ async function collect( job: WatchJob, context: FixContext, deps: GitHubDependencies, + queue: QueueMode, ): Promise { const { entry } = job; const number = entry.number; @@ -642,7 +651,7 @@ async function collect( job.requeuing = head; save(path, job); } - job.requeued = await requeue(github, entry.id, head, job.base.ref, job.dropped); + job.requeued = await requeue(github, entry.id, head, job.base.ref, job.dropped, queue); save(path, job); } catch (failure) { if (failure instanceof Held) return [...said, ...(await leave())]; @@ -686,12 +695,19 @@ async function collect( } /** - * Put `head` back in `base`'s merge queue, at the back. A PR queued or armed at it since the queue - * dropped it, at `dropped`, needs nothing more: a restart finds its own request, and a workflow may pick - * the fixer's push first. A refusal, or a PR retargeted from `base`, hands the PR back; a hold leaves - * it to the session that holds it. + * Put `head` back in `base`'s queue, at the back: GitHub's merge queue, or the watcher's own, by arming + * auto-merge pinned to it. A PR queued or armed at it since the queue dropped it, at `dropped`, needs + * nothing more: a restart finds its own request, and a workflow may pick the fixer's push first. A + * refusal, or a PR retargeted from `base`, hands the PR back; a hold leaves it to the session that holds it. */ -async function requeue(github: GitHub, id: string, head: string, base: string, dropped: string): Promise { +async function requeue( + github: GitHub, + id: string, + head: string, + base: string, + dropped: string, + queue: QueueMode, +): Promise { const how = (pull: ObjectValue): Queued => (pull.mergeQueueEntry != null ? "enqueued" : "armed"); let pull = await readPull(github, id); if (string(pull.baseRefName) !== base) throw new FixerRefusal(`retargeted from ${base} before it was re-enqueued`); @@ -699,13 +715,15 @@ async function requeue(github: GitHub, id: string, head: string, base: string, d if (!queued(pull, dropped)) { if (isHeld(pull)) throw new Held(); try { - await queueHead( - (query, variables) => github.graphql(query, variables), - id, - head, - string(pull.mergeStateStatus), - "merge", - ); + if (queue.kind === "own") await enqueue(github, id, head, queue.method); + else + await queueHead( + (query, variables) => github.graphql(query, variables), + id, + head, + string(pull.mergeStateStatus), + "merge", + ); } catch (error) { if (!(error instanceof GitHubGraphQLError) || error.types.some((type) => UNREFUSED.includes(type ?? ""))) throw error; @@ -730,6 +748,7 @@ async function advanceDisables( state: string, context: FixContext, deps: GitHubDependencies, + queue: QueueMode, ): Promise { const output: string[] = []; const tests = readFlaky(state); @@ -745,6 +764,7 @@ async function advanceDisables( test.disable, () => saveFlaky(state, tests), deps.warn, + queue, ), ); } catch (error) { @@ -774,6 +794,7 @@ async function openDisable( disable: Disable, record: () => void, warn: (text: string) => void, + queue: QueueMode, ): Promise { const name = questName(disable.quest); if (disable.id === undefined) { @@ -813,7 +834,7 @@ async function openDisable( } const entry = { number: disable.number!, enabledAt: disable.head }; try { - disable.done = await requeue(github, disable.id, disable.head, disable.base.ref, ""); + disable.done = await requeue(github, disable.id, disable.head, disable.base.ref, "", queue); } catch (error) { if (error instanceof Held) { // Left for a maintainer, as a refused one is, once recorded. diff --git a/src/cli/land-watch.ts b/src/cli/land-watch.ts index 7f8cf28a..305a7d0b 100644 --- a/src/cli/land-watch.ts +++ b/src/cli/land-watch.ts @@ -1,4 +1,5 @@ -// The merge queue watcher: notice each PR GitHub's merge queue drops unmerged, record why, and say so on it. +// The merge queue watcher: notice each PR GitHub's merge queue drops unmerged, record why, and say so on it, +// or, on a base without GitHub's merge queue, run the watcher's own serial queue (`land-queue.ts`). import { accessSync, constants, existsSync, readFileSync, writeFileSync } from "node:fs"; import { join, resolve } from "node:path"; import { setTimeout as sleep } from "node:timers/promises"; @@ -13,7 +14,7 @@ import { type Repository, string, } from "./github"; -import type { Entry, Plan } from "./land"; +import type { Entry, LandMethod, Plan } from "./land"; import { authenticatedRead } from "./land-git"; import { jobLog } from "./land-checks"; import { fixerText, post } from "./land-fixer"; @@ -38,12 +39,17 @@ import { readRemoval, requeuedHeads, } from "./land-watch-fix"; +import { leaveOwnQueue, type OwnQueue, ownPass } from "./land-queue"; import { enabledHead, headBefore } from "./status"; export { readRemoval } from "./land-watch-fix"; export interface WatchOptions { base?: string; + /** How the own queue lands each PR; GitHub's merge queue applies its own method. */ + method: LandMethod; + /** Minutes an own-queue build may run before it is dropped as timed out. */ + timeout: number; /** Empty for the default, `defaultState`, which `quest land serve` shares. */ state: string; appId: string; @@ -173,8 +179,8 @@ const ENQUEUES = `query($id: ID!) { }`; /** GraphQL names a bot `name` as an actor and `name[bot]` as a user, as REST does. */ -const account = (login: string) => login.replace(/\[bot\]$/, ""); -const login = (value: unknown) => (value == null ? "" : string(object(value).login)); +export const account = (login: string) => login.replace(/\[bot\]$/, ""); +export const login = (value: unknown) => (value == null ? "" : string(object(value).login)); /** Every removal on `pull` since `since`, oldest first: its first page, then the rest by node. */ async function removalsOf(github: GitHub, pull: ObjectValue, since: string): Promise { @@ -299,7 +305,7 @@ async function enqueueArmed( } /** Every page of an Actions listing, whose rows are the response's `field`. */ -async function listed(github: GitHub, path: string, field: string): Promise { +export async function listed(github: GitHub, path: string, field: string): Promise { const rows: ObjectValue[] = []; for (let next: string | undefined = path; next;) { const response = await github.request(next); @@ -336,8 +342,24 @@ async function failedBuild( const last = runs.at(-1); if (!last) return { jobs: [] }; const group = string(last.head_branch); + const jobs = await failingJobs( + github, + repository, + runs.filter((item) => item.head_sha === last.head_sha), + timedOut, + ); + return { jobs, base: group.slice(prefix.length) }; +} + +/** The failing jobs of `runs`, and with `timedOut` those still running, each with its log. */ +export async function failingJobs( + github: GitHub, + repository: string, + runs: ObjectValue[], + timedOut: boolean, +): Promise<(FailedJob & { log: string })[]> { const jobs: (FailedJob & { log: string })[] = []; - for (const run of runs.filter((item) => item.head_sha === last.head_sha)) { + for (const run of runs) { const path = `/repos/${repository}/actions/runs/${Number(run.id)}/jobs?filter=latest&per_page=100`; for (const job of await listed(github, path, "jobs")) { const conclusion = typeof job.conclusion === "string" ? job.conclusion : null; @@ -357,16 +379,16 @@ async function failedBuild( }); } } - return { jobs, base: group.slice(prefix.length) }; + return jobs; } -/** Why the queue dropped the PR, as the comment's first sentence ends. */ -function because(reason: string, actor: string, base: string): string { +/** Why the queue dropped the PR, as the comment's first sentence ends; `build` names the queue's build. */ +export function because(reason: string, actor: string, base: string, build = "`merge_group`"): string { switch (reason) { case "failed_checks": - return "its `merge_group` build failed"; + return `its ${build} build failed`; case "checks_timed_out": - return "its `merge_group` checks timed out"; + return `its ${build} checks timed out`; case "merge_conflict": return `it conflicts with ${fixerText(base)}`; case "manual": @@ -393,7 +415,7 @@ export function jobsSection(blocks: string[], room: number): string { } /** What the watcher does about it, if anything it needs to say; `fixing` when a land fixer is configured. */ -const AFTER: Record string> = { +export const AFTER: Record string> = { fix: (_, fixing) => fixing ? "\n\nThe land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs." @@ -587,20 +609,7 @@ export async function watchPass( ? (JSON.parse(readFileSync(path, "utf8")) as Progress) : { since: started, handled: {} }; const since = new Date(Date.parse(progress.since) - SETTLE).toISOString(); - const ref = - options.base ?? - string( - object( - object( - ( - await github.graphql( - "query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { defaultBranchRef { name } } }", - { owner: repository.owner, name: repository.repo }, - ) - ).repository, - ).defaultBranchRef, - ).name, - ); + const ref = options.base ?? (await defaultBranch(github, repository)); const app = account(string(object((await github.graphql("query { viewer { login } }", {})).viewer).login)); const pulls: ObjectValue[] = []; @@ -664,28 +673,67 @@ export async function watchPass( } progress.since = started; save(path, progress); - if (fix) output.push(...(await advanceFixes(name, { ref, sha: sha! }, options.state, fix, deps))); + if (fix) + output.push(...(await advanceFixes(name, { ref, sha: sha! }, options.state, fix, deps, { kind: "github" }))); return output; } +/** The repository's default branch. */ +async function defaultBranch(github: GitHub, repository: Repository): Promise { + const data = await github.graphql( + "query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { defaultBranchRef { name } } }", + { owner: repository.owner, name: repository.repo }, + ); + return string(object(object(data.repository).defaultBranchRef).name); +} + +/** + * Whether `base` lands through the watcher's own queue, since its rules have no `merge_queue` rule. + * Rules GitHub does not answer with fail the pass, which then does nothing. + */ +export async function ownQueue(github: GitHub, repository: Repository, base: string): Promise { + const path = `/repos/${repository.owner}/${repository.repo}/rules/branches/${encodeURIComponent(base)}?per_page=100`; + return !(await github.all(path)).some((rule) => rule.type === "merge_queue"); +} + type Permissions = Record; /** - * The installation token permissions the watcher asks for. `api` reads the queue's events, comments, - * and the `merge_group` logs (`actions: read`), and enqueues the armed picks GitHub left out of the - * queue, which `enqueuePullRequest` needs `merge_queues: write` for; with a fixer it also re-enqueues, - * which needs `contents: write` for arming auto-merge, edits the PR body's Review, and reads the PR's - * own checks for the fixer (`checks: read` and `statuses: read`). `push` lease-pushes a fix or merge, - * and `pushWorkflows` one that carries a workflow change, as `quest land serve` does; `read` fetches. + * The installation token permissions the watcher asks for. On GitHub's merge queue, `api` reads the + * queue's events, comments, and the `merge_group` logs (`actions: read`), and enqueues the armed picks + * GitHub left out of the queue, which `enqueuePullRequest` needs `merge_queues: write` for; with a fixer + * it also re-enqueues, which needs `contents: write` for arming auto-merge, edits the PR body's Review, + * and reads the PR's own checks for the fixer (`checks: read` and `statuses: read`). The `own` queue + * also disarms and arms auto-merge, cancels a build's runs (`actions: write`), posts each pick's queue + * check (`checks: write`), and reads the base's classic protection (`administration: read`), as + * `quest land serve` does. `push` lease-pushes a fix, a merge, a queue branch, or a landing, and + * `pushWorkflows` one that carries a workflow change; `read` fetches. */ -export function watchPermissions(fixing: boolean): { +export function watchPermissions( + fixing: boolean, + own = false, +): { api: Permissions; push?: Permissions; pushWorkflows?: Permissions; read?: Permissions; } { - if (!fixing) return { api: { contents: "read", pull_requests: "write", actions: "read", merge_queues: "write" } }; const push: Permissions = { contents: "write", pull_requests: "write" }; + if (own) + return { + api: { + contents: "write", + pull_requests: "write", + actions: "write", + checks: "write", + statuses: "read", + administration: "read", + }, + push, + pushWorkflows: { ...push, workflows: "write" }, + read: { contents: "read" }, + }; + if (!fixing) return { api: { contents: "read", pull_requests: "write", actions: "read", merge_queues: "write" } }; return { api: { contents: "write", @@ -702,8 +750,9 @@ export function watchPermissions(fixing: boolean): { } /** - * Watch the merge queue of `repository`'s base, one pass a minute, until the process stops. With a - * fixer, the PRs it drops are fixed from the daemon's private repository in the state directory. + * Watch the queue of `repository`'s base, one pass a minute, until the process stops: GitHub's merge + * queue where the base's rules have one, otherwise the watcher's own, each pass deciding from the rules. + * With a fixer, the PRs a queue drops are fixed from the daemon's private repository in the state directory. */ export async function watch( root: string, @@ -719,22 +768,45 @@ export async function watch( const unlock = await startDaemon(root, repository, options); try { const now = { fetch, now: Date.now }; - const permissions = watchPermissions(options.fixer !== undefined); + const fixing = options.fixer !== undefined; + const github = watchPermissions(fixing); + const own = watchPermissions(fixing, true); const mint = (wanted: Permissions) => installationToken(repository, options, wanted, now); - const deps = apiDependencies(() => mint(permissions.api), { ...now, sleep, warn }); + // Each mode's token is minted only once a pass needs it. + const deps = apiDependencies(() => mint(github.api), { ...now, sleep, warn }); + const ownDeps = apiDependencies(() => mint(own.api), { ...now, sleep, warn }); + const token = (workflows: boolean) => mint(workflows ? own.pushWorkflows! : own.push!); + const read = authenticatedRead(() => mint(own.read!)); + const trusted = privateRepository(options.state); const fix: FixContext | undefined = options.fixer ? { - root: privateRepository(options.state), + root: trusted, fixer: options.fixer, jobs: options.fixerJobs, effort: options.fixerEffort, timeout: 60, - token: (workflows) => mint(workflows ? permissions.pushWorkflows! : permissions.push!), - read: authenticatedRead(() => mint(permissions.read!)), + token, + read, } : undefined; + const queue: OwnQueue = { + root: trusted, + state: options.state, + method: options.method, + timeout: options.timeout, + token, + read, + }; await daemonLoop(async () => { - for (const line of await watchPass(repository, options, deps, fix)) warn(line); + const api = new GitHub(deps); + const base = options.base ?? (await defaultBranch(api, repository)); + if (await ownQueue(api, repository, base)) { + const pass = await ownPass(repository, base, queue, ownDeps, fix); + for (const line of pass.lines) warn(line); + return pass.again; + } + for (const line of await leaveOwnQueue(repository, base, queue, ownDeps, fixing)) warn(line); + for (const line of await watchPass(repository, { ...options, base }, deps, fix)) warn(line); return false; }, deps); } finally { diff --git a/src/cli/land.ts b/src/cli/land.ts index 68218082..49dcc857 100644 --- a/src/cli/land.ts +++ b/src/cli/land.ts @@ -423,7 +423,10 @@ export async function resolveQueue( } else if (resolution && (await unresolved(github, entry.id)) > 0) { eject("unresolved_conversations", "it has unresolved review threads; resolve them and land it again"); } else { - const unmet = checks(await contexts(github, repository, { ...node, headRefOid: approved }), required); + // The queue's first page holds the head's checks, unless the head moved from the pin since. + const commit = object(object(array(object(node.commits).nodes)[0]).commit); + const rollup = commit.oid === approved ? commit.statusCheckRollup : undefined; + const unmet = checks(await contexts(github, repository, approved, rollup), required); if (unmet.failing.length > 0) { eject("failed_checks", `required checks failed on its head: ${unmet.failing.join(", ")}`); } else if (unmet.pending.length > 0) { @@ -443,14 +446,16 @@ export async function resolveQueue( } /** - * Every check run and commit status on a PR node's head: the queue's first page, then the rest by - * the head's SHA, which also reads them all when the head moved between the queue's fields. + * Every check run and commit status on `head`: from `rollup`, the first page already read with it, + * when there is one, then the rest by the SHA. */ -async function contexts(github: GitHub, repository: Repository, node: ObjectValue): Promise { - const head = string(node.headRefOid); - const commit = object(object(array(object(node.commits).nodes)[0]).commit); +async function contexts( + github: GitHub, + repository: Repository, + head: string, + rollup?: unknown, +): Promise { const found: ObjectValue[] = []; - let rollup = commit.oid === head ? commit.statusCheckRollup : undefined; let after: string | null = null; for (;;) { if (rollup === undefined) { @@ -461,7 +466,7 @@ async function contexts(github: GitHub, repository: Repository, node: ObjectValu after, }); const target = object(data.repository).object; - if (target === null) throw new Error(`${string(node.url)}'s head ${head} is missing`); + if (target === null) throw new Error(`commit ${head} is missing`); rollup = object(target).statusCheckRollup; } if (rollup === null) return found; @@ -474,6 +479,16 @@ async function contexts(github: GitHub, repository: Repository, node: ObjectValu } } +/** The checks of `required` that commit `sha` failed, and those still pending or not yet reported. */ +export async function requiredChecks( + github: GitHub, + repository: Repository, + sha: string, + required: RequiredCheck[], +): Promise<{ failing: string[]; pending: string[] }> { + return checks(await contexts(github, repository, sha), required); +} + /** * The required checks a head failed, and those pending or not yet reported, each from its app. Only * each check's latest run counts, as for GitHub's mergeability, so a rerun supersedes a cancelled or diff --git a/src/cli/main.ts b/src/cli/main.ts index 23282c8e..d66931c2 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -698,9 +698,22 @@ export function main(argv: string[], out: Output): number | Promise { landCommand .command("watch") .description( - "Watch the base's GitHub merge queue as a daemon, one per repository: once a minute, read the base's open PRs and the merge queue's removals since the last pass, and comment once on each PR the queue dropped unmerged, naming GitHub's reason and, for a failed `merge_group` build, the failing jobs and their logs' tails. A failed build or a conflict is recorded for the land fixer; a push since the removal, a person's removal, or an unknown reason ends the pick. A PR whose auto-merge was armed, by a writer or by the watcher, that is ready (its required checks passed and it does not conflict) but that GitHub never added to the queue is enqueued at the head auto-merge was armed on, never at a head pushed since. A pick is the latest enqueue by anyone, the App included (as for Dependabot's PRs), except the App's enqueues of heads the watcher itself put back in the queue for the pick. With --fixer, a conflict is first merged with the base by the quest merge driver; otherwise the fixer runs in the background on the failure, at most --fixer-jobs at once, oldest removal first. A fixed or merged head is lease-pushed and recorded in Review, and each fixed, merged, or flaky head is re-enqueued at the back of the queue (or armed with auto-merge while its checks run). The pick keeps going while each round makes progress: a round with no new commit for a failure that isn't flaky, the failure a fix addressed recurring right after it, or three flaky rounds in a row hand it back, as does the fixer, with a comment. State keeps each removal handled, so a restart neither misses nor repeats one, each removal's full log, the fixer's jobs, and each pick's rounds. It shares `quest land serve`'s App key, state directory, private repository, and locks, so the two never run beside each other. The App needs `actions: read` for the logs, `merge_queues: write` to enqueue, and with a fixer `contents: write` to arm auto-merge and push, and `workflows: write` to push fixes that touch workflows.", + "Watch the base's queue as a daemon, one per repository: GitHub's merge queue where the base's rules have a merge_queue rule, otherwise the watcher's own serial queue, decided again each pass from GET /rules/branches/, so enabling GitHub's queue switches the watcher with no host change; a pass that cannot read the rules does nothing. On GitHub's queue, once a minute, read the base's open PRs and the merge queue's removals since the last pass, and comment once on each PR the queue dropped unmerged, naming GitHub's reason and, for a failed `merge_group` build, the failing jobs and their logs' tails. A failed build or a conflict is recorded for the land fixer; a push since the removal, a person's removal, or an unknown reason ends the pick. A PR whose auto-merge was armed, by a writer or by the watcher, that is ready (its required checks passed and it does not conflict) but that GitHub never added to the queue is enqueued at the head auto-merge was armed on, never at a head pushed since. A pick is the latest enqueue by anyone, the App included (as for Dependabot's PRs), except the App's enqueues of heads the watcher itself put back in the queue for the pick. " + + `The own queue, for a base that requires the unreported \`${LAND_CHECK}\` check so GitHub never merges a pick itself, reads its picks as quest land serve does: PRs whose auto-merge \`quest merge --land\` armed pinned to their head, in arming order, with the same ejections. It tests one at a time: it merges the base into the pick with the quest merge driver, pushes that to quest-queue//, which the repository's queue workflow builds on push, and waits for the base's required checks other than \`${LAND_CHECK}\` on it, at most --timeout minutes. A pass lands a passing pick through quest land serve's landing path with --method, only while the base is still the one it tested; a moved base rebuilds it. A failed or timed-out build, or a conflict, disarms the PR's auto-merge and comments with the failing jobs, as GitHub's queue removes a PR, and is recorded for the land fixer; a re-enqueue arms auto-merge again, at the back. A writer's push, a person disarming auto-merge, or a hold ends the build. Each pick carries an informational \`quest queue\` check run with its position, the build in progress and its run, and the outcome. Queue branches go once their pick lands or leaves, and those an earlier process left are pruned on its first pass. ` + + "With --fixer, a conflict is first merged with the base by the quest merge driver; otherwise the fixer runs in the background on the failure, at most --fixer-jobs at once, oldest removal first. A fixed or merged head is lease-pushed and recorded in Review, and each fixed, merged, or flaky head is re-enqueued at the back of the queue (or armed with auto-merge while its checks run). The pick keeps going while each round makes progress: a round with no new commit for a failure that isn't flaky, the failure a fix addressed recurring right after it, or three flaky rounds in a row hand it back, as does the fixer, with a comment. State keeps each removal handled, so a restart neither misses nor repeats one, each removal's full log, the fixer's jobs, each pick's rounds, and the own queue's build. It shares `quest land serve`'s App key, state directory, private repository, and locks, so the two never run beside each other. On GitHub's queue the App needs `actions: read` for the logs, `merge_queues: write` to enqueue, and with a fixer `contents: write` to arm auto-merge and push, and `workflows: write` to push fixes that touch workflows. The own queue's App needs `contents: write`, `pull_requests: write`, `actions: write` to read logs and cancel builds, `checks: write`, `administration: read`, `workflows: write` for heads that touch workflows, and a bypass of the base's ruleset.", ) .option("--base ", "Base branch (default: repository default)") + .addOption( + new Option("--method ", "On the own queue, land each PR as a squash commit or a merge commit") + .choices(["squash", "merge"]) + .default("squash"), + ) + .option( + "--timeout ", + "On the own queue, drop a build still running after this long", + positiveInteger("timeout"), + 60, + ) .option("--state ", "Private daemon state directory (default: $STATE_DIRECTORY or the XDG state home)") .requiredOption("--app-id ", "GitHub App id") .option( From 01a2b7fb34ef9151a602ffd7414907b94d6adc76 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 11:20:00 -0800 Subject: [PATCH 03/12] wip: own queue tests, docs, quest removal --- README.md | 69 +++++- cloudflare/dev/github.ts | 105 ++++++++- quest/m0/queue-watcher/README.md | 7 +- quest/m0/queue-watcher/own-queue.md | 94 -------- quest/m0/queue-watcher/pro-cutover.md | 21 +- quest/m0/queue-watcher/queue-page.md | 8 +- quest/m0/queue-watcher/retire.md | 7 +- src/cli/land-queue.ts | 76 ++++--- src/cli/land-watch.ts | 55 +++-- test/land.test.ts | 302 +++++++++++++++++++++++++- 10 files changed, 569 insertions(+), 175 deletions(-) delete mode 100644 quest/m0/queue-watcher/own-queue.md diff --git a/README.md b/README.md index 8d92e632..a681a09f 100644 --- a/README.md +++ b/README.md @@ -818,10 +818,16 @@ backoff. Grant the App the base ruleset's bypass and the read permissions needed to inspect branch protection. The bypass actor's API merge of a pinned PR with the unreported required `quest land` check must be verified on the live host. -## Watch GitHub's merge queue with quest land watch - -On a base that lands through GitHub's merge queue, `quest land watch` runs on a -maintainer's host, one process per repository, and tells each PR the queue +## Watch the merge queue with quest land watch + +`quest land watch` runs on a maintainer's host, one process per repository, +and lands its base through a serial queue: GitHub's merge queue where the +base's rules have a `merge_queue` rule, and otherwise +[its own](#the-watchers-own-queue), as on a private repository on GitHub Team, +which offers no merge queue. Each pass reads the base's rules +(`GET /rules/branches/`) and picks the mode from them, so enabling +GitHub's queue on a repository switches its watcher with no host change; a +pass that cannot read them does nothing. On GitHub's queue it tells each PR the queue drops unmerged why. With `--fixer`, it also runs the land fixer on each PR the queue dropped for a failure or a conflict, and puts it back at the back of the queue. It polls; the host needs no public endpoint. @@ -950,6 +956,61 @@ for arming auto-merge and pushing fixes, and `checks: read` and `statuses: read` for the PR's own checks the fixer reads; a push that carries a workflow change asks for `workflows: write`. +### The watcher's own queue + +On a base without GitHub's merge queue, the watcher runs a serial queue of its +own, one build at a time, so a failure is the one PR's with no triage. The +base keeps requiring the `quest land` context nobody reports, with the App as +the ruleset's bypass actor, so GitHub never merges a pick itself and the +watcher is the base's only writer outside an admin's emergency bypass. + +Its picks are those `quest land serve` reads: open PRs whose auto-merge +`quest merge --land` armed, pinned to their head, in the order they were armed, +with the same ejections for a moved head, a missing pin, a blocking review, or +a failing required check. A pick still waiting on its required checks keeps its +place, and one another session holds waits, armed, until its hold clears. + +For the oldest pick it merges the base into the picked head with the quest +merge driver, in the daemon's private repository, and pushes that merge to +`quest-queue//`. The repository's queue workflow builds that +branch on push, as it builds a merge group; the header of this repository's +`.github/workflows/queue.yml` shows the trigger. The base's required checks +other than `quest land`, reported on the merge, decide the build: once they +pass, the watcher lands the pick through `quest land serve`'s landing path. It +lease-pushes the merge to the PR's branch, merges it with GitHub's merge API +pinned to that head, as a squash or, with `--method merge`, a merge commit, and +checks that the landed tree is the tested one, logging `CRITICAL` when it is +not; a fork that refuses the push lands by a leased fast-forward of the base. +It lands only on the base it tested: a base that moved during the build +rebuilds the pick there, with no attempt counted. + +A failed build, a build still running after `--timeout` minutes (60), or a +conflict the quest merge driver cannot merge drops the pick as GitHub's queue +removes a PR: the watcher records it for the land fixer in `removal-.json` +with the failing jobs' logs, disarms the PR's auto-merge, and comments with +the failing jobs' tails, and the fixer's rounds, attempts, flaky handling, and +handbacks follow as on GitHub's queue. A re-enqueue arms auto-merge again, +pinned to the head, which puts the PR at the back. A writer's push, a person +disarming auto-merge, or a hold ends a build in progress. The drop is recorded +before any of its steps, so a restart finishes it. + +Each pick carries an informational `quest queue` check run, never a required +context: its position, the build in progress with a link to its run, and the +outcome. A queue branch goes, with its unfinished runs cancelled, once its pick +lands or leaves the queue, and the first pass deletes the queue branches an +earlier process left behind. Once the base has GitHub's merge queue, the next +pass ends any build and completes the checks. The ruleset change that adds the +`merge_queue` rule also replaces the required `quest land` context with the +queue workflow's required check, since GitHub's queue never reports +`quest land` and would stall. + +The own queue's token asks for `contents: write` and `pull_requests: write`, +to arm and disarm auto-merge and comment, `actions: write` to read logs and +cancel runs, `checks: write` for its check runs, `statuses: read`, and +`administration: read` for the base's classic protection, as `quest land serve` +does. Its pushes ask for `contents: write`, and `workflows: write` for one that +carries a workflow change. + ## Gate expensive commands `quest gate [args...]` holds an expensive command until the host has diff --git a/cloudflare/dev/github.ts b/cloudflare/dev/github.ts index 2fa8f408..22a03804 100644 --- a/cloudflare/dev/github.ts +++ b/cloudflare/dev/github.ts @@ -124,8 +124,37 @@ export type Repo = { rules?: Record[]; /** The default branch's classic protection, as `GET /branches//protection` returns it; none when absent. */ protection?: Record; - /** Check runs the App wrote through `POST /check-runs`, oldest first. */ - checkRuns?: { name: string; head_sha: string; conclusion: string; app: number; output: Record }[]; + /** Check runs the App wrote through `POST /check-runs`, oldest first; `status` is set while one is unfinished. */ + checkRuns?: { + name: string; + head_sha: string; + conclusion?: string; + status?: string; + details_url?: string; + app: number; + output: Record; + }[]; + /** The check runs reported on commits that are no pull request's head, such as a queue build's. */ + commitRuns?: Record>; + /** GitHub Actions workflow runs, as `GET /actions/runs` lists them, with their jobs and each job's log. */ + actionRuns?: { + id: number; + name: string; + head_branch: string; + head_sha: string; + status: string; + conclusion: string | null; + html_url: string; + jobs: { + id: number; + name: string; + status: string; + conclusion: string | null; + html_url: string; + steps?: { name: string; conclusion: string | null }[]; + log?: string; + }[]; + }[]; /** Rulesets by ID, as `GET /rulesets/` shows them to the App. */ rulesets?: Record; }; @@ -411,7 +440,7 @@ const UNAPPROVED = new Set(["BUSL-1.1", "SSPL-1.0", "Elastic-2.0"]); export const ACTIONS_APP = 15368; /** One page of a head's check runs and commit statuses, as `statusCheckRollup.contexts` pages them. */ -function contextPage(pull: Pull, after: unknown) { +function contextPage(pull: Pick, after: unknown) { const start = typeof after === "string" ? Number(after) : 0; const all = (pull.runs ?? []).map((run, index) => run.context @@ -510,6 +539,13 @@ function graphql(repo: Repo, query: string, variables: Record) return { data: { repository: { pullRequest: pull ? holdsTarget(repo, pull) : null } } }; } if (query.includes("object(oid: $oid)")) { + const runs = repo.commitRuns?.[variables.oid as string]; + if (runs) + return { + data: { + repository: { object: { statusCheckRollup: { contexts: contextPage({ runs }, variables.after) } } }, + }, + }; const pull = repo.pulls.find( (p) => headOf(repo, p) === variables.oid || p.autoMerge?.commitBody === `Quest-Land-Head: ${variables.oid}`, ); @@ -873,7 +909,11 @@ export function fakeGitHub(world: World) { data: { node: { state: pull.merged ? "MERGED" : pull.closed ? "CLOSED" : "OPEN", + baseRefName: pull.base ?? defaultOf(repo), headRefOid: headOf(repo, pull), + mergeStateStatus: mergeState(pull), + mergeQueueEntry: pull.queued ? { enqueuedAt: updatedAt } : null, + labels: { nodes: (pull.labels ?? []).map((name) => ({ name })) }, autoMergeRequest: pull.landing ? { enabledAt: pull.autoMerge?.enabledAt ?? updatedAt } : null, @@ -1010,19 +1050,74 @@ export function fakeGitHub(world: World) { const found = repo.rulesets?.[Number(ruleset[1])]; return found ? json({ id: Number(ruleset[1]), ...found }) : notFound(); } + const statuses = ["queued", "in_progress", "completed"]; if (tail === "/check-runs" && method === "POST") { if (!writes("checks")) return forbidden(); - if (body.status !== "completed" || typeof body.head_sha !== "string" || !repo.commits[body.head_sha]) + const known = + typeof body.head_sha === "string" && + (repo.commits[body.head_sha] || repo.pulls.some((pull) => headOf(repo, pull) === body.head_sha)); + if (!statuses.includes(body.status) || (body.status === "completed") !== !!body.conclusion || !known) return json({ message: "Validation Failed" }, 422); (repo.checkRuns ??= []).push({ name: body.name, head_sha: body.head_sha, conclusion: body.conclusion, + status: body.status === "completed" ? undefined : body.status, + details_url: body.details_url, app: world.app.id, output: body.output, }); return json({ id: repo.checkRuns.length, name: body.name, head_sha: body.head_sha }, 201); } + const checkRun = tail.match(/^\/check-runs\/(\d+)$/); + if (checkRun && method === "PATCH") { + if (!writes("checks")) return forbidden(); + const run = repo.checkRuns?.[Number(checkRun[1]) - 1]; + if (!run) return notFound(); + if (!statuses.includes(body.status) || (body.status === "completed") !== !!body.conclusion) + return json({ message: "Validation Failed" }, 422); + Object.assign(run, { + conclusion: body.conclusion, + status: body.status === "completed" ? undefined : body.status, + details_url: body.details_url ?? run.details_url, + output: body.output, + }); + return json({ id: Number(checkRun[1]), name: run.name, head_sha: run.head_sha }); + } + if (tail === "/actions/runs" && method === "GET") { + if (!scope.permissions.actions) return forbidden(); + const sha = url.searchParams.get("head_sha"); + const branch = url.searchParams.get("branch"); + const workflow_runs = (repo.actionRuns ?? []) + .filter((run) => (!sha || run.head_sha === sha) && (!branch || run.head_branch === branch)) + .map(({ jobs: _, ...run }) => run); + return json({ total_count: workflow_runs.length, workflow_runs }); + } + const actionRun = tail.match(/^\/actions\/runs\/(\d+)\/(jobs|cancel)$/); + if (actionRun) { + const run = repo.actionRuns?.find((item) => item.id === Number(actionRun[1])); + if (!run) return notFound(); + if (actionRun[2] === "jobs" && method === "GET") + return scope.permissions.actions + ? json({ total_count: run.jobs.length, jobs: run.jobs.map(({ log: _, ...job }) => job) }) + : forbidden(); + if (actionRun[2] === "cancel" && method === "POST") { + if (!writes("actions")) return forbidden(); + if (run.status === "completed") + return json({ message: "Cannot cancel a workflow run that is completed." }, 409); + run.status = "completed"; + run.conclusion = "cancelled"; + return json({}, 202); + } + } + const jobLog = tail.match(/^\/actions\/jobs\/(\d+)\/logs$/); + if (jobLog && method === "GET") { + if (!scope.permissions.actions) return forbidden(); + const job = (repo.actionRuns ?? []) + .flatMap((run) => run.jobs) + .find((item) => item.id === Number(jobLog[1])); + return job?.log === undefined ? notFound() : new Response(job.log); + } const runs = tail.match(/^\/commits\/([0-9a-f]{40})\/check-runs$/); if (runs && method === "GET") { if (!scope.permissions.checks) return forbidden(); @@ -1044,7 +1139,7 @@ export function fakeGitHub(world: World) { .filter((run) => run.head_sha === runs[1]) .reverse() .filter((run, i, all) => all.findIndex((other) => other.name === run.name) === i) - .map((run) => ({ ...run, status: "completed", app: { id: run.app } })); + .map((run) => ({ ...run, status: run.status ?? "completed", app: { id: run.app } })); const check_runs = [...written, ...found].filter( (run) => (!name || run.name === name) && (!app || String(run.app.id) === app), ); diff --git a/quest/m0/queue-watcher/README.md b/quest/m0/queue-watcher/README.md index 4e2a0d8a..a7f321a5 100644 --- a/quest/m0/queue-watcher/README.md +++ b/quest/m0/queue-watcher/README.md @@ -119,9 +119,9 @@ queues on private repositories only under Enterprise Cloud and moq-dev is on GitHub Team: moq.pro leaves the host gate for an own serial queue in `quest land watch`, rather than a plan upgrade, making moq.pro public, or a third-party queue. The maintainer expects the own queue to become every -repository's, for visibility. The decisions are recorded in -[the own queue](/quest/m0/queue-watcher/own-queue.md), -[moq.pro's cutover](/quest/m0/queue-watcher/pro-cutover.md) and +repository's, for visibility. The own queue landed in +[moq-dev/quest#540](https://github.com/moq-dev/quest/pull/540); the rest is +recorded in [moq.pro's cutover](/quest/m0/queue-watcher/pro-cutover.md) and [the queue page](/quest/m0/queue-watcher/queue-page.md). Shared shape, for the children to follow: @@ -153,7 +153,6 @@ Shared shape, for the children to follow: - [Tests fit their timeout on the queue runner](/quest/m0/queue-watcher/queue-test-timeouts.md) - the root and Worker suites stop timing out or stalling in merge-group runs, so the queue stops ejecting PRs for them - [The watcher writes flaky quests](/quest/m0/queue-watcher/flaky-quest.md) - a disable's follow-up quest comes from a template in `quest land watch`, so the headless land fixer writes no quest - [Daemons prune their logs](/quest/m0/queue-watcher/log-retention.md) - the watcher and gate delete their finished logs after a window, so state stays bounded (4.1 GiB on nixos on 2026-11-04) -- [The watcher runs its own serial queue](/quest/m0/queue-watcher/own-queue.md) - `quest land watch` lands a base without GitHub's merge queue, testing one pick at a time in the repository's queue workflow - [moq.pro lands main through the own queue](/quest/m0/queue-watcher/pro-cutover.md) - moq.pro's `main` leaves the batching gate for the watcher's own queue - [Retire the batching gate](/quest/m0/queue-watcher/retire.md) - once no repository lands through `quest land serve`, its batching code and host units go - [kix.quest shows each repository's landing queue](/quest/m0/queue-watcher/queue-page.md) - one page per repository with its queue order, current build, and recent drops, in both modes diff --git a/quest/m0/queue-watcher/own-queue.md b/quest/m0/queue-watcher/own-queue.md deleted file mode 100644 index 398a19e8..00000000 --- a/quest/m0/queue-watcher/own-queue.md +++ /dev/null @@ -1,94 +0,0 @@ -# [L] The watcher runs its own serial queue where GitHub has none - -## Goal - -`quest land watch` lands a base whose rules have no `merge_queue` rule, such -as private moq-dev/moq.pro on GitHub Team, through its own serial queue, and -keeps watching GitHub's queue where the rule exists. Each pass picks the mode -from the base's rules, so enabling GitHub's queue on a repository switches its -watcher with no host change. - -The own queue tests one pick at a time on top of the base, in the -repository's own queue workflow, and lands exactly the tested tree with -moq-bot. A failed build or a conflict goes through the same fixer, flaky, -attempt, and re-enqueue-at-the-back handling as a PR GitHub's queue drops. -Each PR in the own queue carries a check run showing its position, the build -in progress with a link to its run, and the outcome. `--method squash|merge` -lands squash or merge commits, as `quest land serve` does, so a `release` -base can use merge commits. - -Non-goals: deleting `quest land serve` (the -[retire quest](/quest/m0/queue-watcher/retire.md)), moq.pro's own switch -([its cutover](/quest/m0/queue-watcher/pro-cutover.md)), and the kix.quest -page ([queue page](/quest/m0/queue-watcher/queue-page.md)). - -## Plan - -Decided 2026-11-06 (maintainer, /quest-plan). Decision prompts, selected -options marked ✅: - -- Goal: - - ✅ Use GitHub's merge queue where the repository has one, otherwise quest's - own serial queue in `quest land watch`; the batching `quest land serve` - is then deleted. The maintainer expects the own queue to become the queue - everywhere, for visibility. - - Move every repository onto the own queue now - - A separate subcommand, such as a slimmed `quest land serve` -- Build: - - ✅ The repository's queue workflow: the daemon pushes the test merge to a - queue branch and runs the same `queue.yml` build GitHub's `merge_group` - runs, on the repository's self-hosted runner. One check definition per - repository in both modes, logs in Actions, and the watcher's failed-job - log reading (`failedBuild` in `src/cli/land-watch.ts`) is reused. - - A local sandbox, as `quest land serve`'s `--sandbox` -- Mode: - - ✅ Detected each pass from the base's rules - (`GET /repos/{owner}/{repo}/rules/branches/{base}`): a `merge_queue` rule - means GitHub's queue, none means the own queue. A pass that cannot read - the rules does nothing. Switching a base to GitHub's queue also replaces - its required unreported context (`quest land`) with the queue workflow's - required check, since GitHub's queue never reports `quest land` and - would stall; the ruleset change makes both edits at once. - - An explicit `--queue github|own` flag -- Concurrency stays 1, as the [epic](/quest/m0/queue-watcher/README.md) - decided: each build holds one PR on the base, so a failure blames that PR - with no triage. - -Implementation guidance: - -- Picks are PRs whose auto-merge a writer armed, in `enabledAt` order, as - `quest land watch` and `quest land serve` read them today. The base keeps - requiring a context no one reports (moq.pro's `quest land`), so GitHub never - merges a pick itself. Re-enqueueing at the back re-arms auto-merge, giving a - new `enabledAt`. A push by anyone but the fixer, or a person disarming - auto-merge, ends the pick, as a removal from GitHub's queue does. -- Build the test merge as `quest land serve` does today: merge the base into - the pick with `quest resolve` and the quest merge driver, and treat what - still conflicts as a GitHub `MERGE_CONFLICT` removal. Push it with moq-bot to - a queue branch outside the reserved `quest/` namespace, such as - `quest-queue//`, and start the queue workflow on it. The - workflow's build job runs as it does for `merge_group`, and its required - job reports the result. Document the trigger in the repository's - `queue.yml` header, since every own-queue repository adds it. -- Land through `quest land serve`'s landing path (`src/cli/land-head.ts`): - lease-push the tested head, merge it with GitHub's merge API pinned to that - head, verify the landed tree against the tested one, and fall back to a - leased fast-forward of the base when the branch push is refused. Land only - while the base is still the tested base: if it moved while building or - before the merge, rebuild on the new base without counting an attempt. - GitHub's merge API takes no expected base, so the ruleset closes the gap - between that check and the merge: its unreported required context, with - moq-bot as bypass actor, leaves the watcher the base's only writer outside - an admin's emergency bypass, and the landed-tree check catches that case. Move these helpers out of serve's files so the retire quest can - delete serve without touching them. -- The check run is informational and never the required context. moq-bot's - App posts it with `checks: write`, which `watchPermissions` adds only in - own-queue mode. -- Delete the queue branch once its pick lands or leaves the queue, and on - start prune stale ones the daemon left behind. -- Update `quest land watch`'s description in `src/cli/main.ts`, the README's - landing section, and `CONTRIBUTING.md` in the same change. - -## Related - -- [Merge through a host gate](/quest/m0/host-land/README.md) - the gate whose landing path this reuses diff --git a/quest/m0/queue-watcher/pro-cutover.md b/quest/m0/queue-watcher/pro-cutover.md index f626f822..6f5e27f3 100644 --- a/quest/m0/queue-watcher/pro-cutover.md +++ b/quest/m0/queue-watcher/pro-cutover.md @@ -2,9 +2,8 @@ ## Goal -moq-dev/moq.pro's `main` lands through -[the watcher's own queue](/quest/m0/queue-watcher/own-queue.md) instead of -the batching `quest land serve` gate: +moq-dev/moq.pro's `main` lands through the watcher's own queue (the README's +"The watcher's own queue") instead of the batching `quest land serve` gate: - moq.pro's queue workflow builds the watcher's queue branches on its private `moq-pro-gate` runner; @@ -38,7 +37,17 @@ queue, so it is the repository the own queue exists for. - moq.pro's `queue.yml` is still its label queue's control workflow, which `release` uses until moq.pro's Land release quest deletes it, so the own queue's build workflow takes another name, or moq.pro renames the label - queue's first. The own queue's trigger must not depend on that name. + queue's first. The own queue's trigger must not depend on that name: it + builds `quest-queue/main/` on push, as moq-dev/quest's `queue.yml` + header shows. +- The watcher decides each build from `main`'s required checks other than + `quest land`, reported on the queue branch's commit, and refuses to run + while there are none. So `main`'s ruleset also requires the queue workflow's + result job, which reports skipped on pull requests, as moq-dev/quest's + `Queue` does. +- The own queue's token asks moq-bot's App for `actions: write` (to cancel a + stale build) and `checks: write` (each pick's `quest queue` check run), + beyond what the gate uses; grant them on moq.pro's installation first. - Drain or carry over the gate's queue before switching: picks already armed stay armed, so the watcher reads them in the same order. Switch only once the gate has no fixer job running or uncollected: the gate records each in @@ -50,7 +59,3 @@ queue, so it is the repository the own queue exists for. queue workflow, as [Harden the queue runners](/quest/m0/queue-watcher/runners.md) does for the public queues. - -## Required - -- [The watcher runs its own serial queue](/quest/m0/queue-watcher/own-queue.md) - `quest land watch` lands a base without GitHub's merge queue diff --git a/quest/m0/queue-watcher/queue-page.md b/quest/m0/queue-watcher/queue-page.md index 1c8ee56c..eb5e4bb0 100644 --- a/quest/m0/queue-watcher/queue-page.md +++ b/quest/m0/queue-watcher/queue-page.md @@ -3,8 +3,8 @@ ## Goal quest.moq.dev shows each enrolled repository's landing queue, whether GitHub's -merge queue or the watcher's [own queue](/quest/m0/queue-watcher/own-queue.md) -runs it: +merge queue or the watcher's own queue (the README's "The watcher's own +queue") runs it: - the queued PRs in landing order; - the build in progress, with a link to its run; @@ -39,10 +39,6 @@ Sources, per mode: - Drops and fixer results: `RemovedFromMergeQueueEvent` and the watcher's comments, which name the reason and the fixer's outcome. -## Required - -- [The watcher runs its own serial queue](/quest/m0/queue-watcher/own-queue.md) - the own queue's per-PR check run the page reads - ## Related - [Private repositories](/quest/m0/kix/private.md) - how kix.quest gates a private repository's pages to its readers diff --git a/quest/m0/queue-watcher/retire.md b/quest/m0/queue-watcher/retire.md index 7d622c33..d48694d4 100644 --- a/quest/m0/queue-watcher/retire.md +++ b/quest/m0/queue-watcher/retire.md @@ -26,9 +26,10 @@ shared state directory still expire. moq, moq-dev/quest, rind and rust2 land through the merge queue (2026-11-03). moq.pro is the last: it is private and moq-dev is on GitHub Team, where GitHub offers merge queues on private repositories only under Enterprise Cloud. On -2026-11-06 the maintainer chose to move it to -[the watcher's own queue](/quest/m0/queue-watcher/own-queue.md), whose -landing helpers move out of serve's files there, so this deletion leaves them. +2026-11-06 the maintainer chose to move it to the watcher's own queue, whose +landing helpers ([moq-dev/quest#540](https://github.com/moq-dev/quest/pull/540)) +moved out of serve's files into `src/cli/land-head.ts`, so this deletion +leaves them. Its `main` ruleset keeps requiring the unreported `quest land` check, which the own queue relies on so GitHub never merges a pick itself. diff --git a/src/cli/land-queue.ts b/src/cli/land-queue.ts index 0a284235..73db25a0 100644 --- a/src/cli/land-queue.ts +++ b/src/cli/land-queue.ts @@ -1,7 +1,7 @@ // The watcher's own serial queue, for a base without GitHub's merge queue, such as a private repository on // GitHub Team: test one pick at a time on top of the base in the repository's queue workflow, then land // exactly the tested tree with the App. -import { existsSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, readFileSync, rmSync } from "node:fs"; import { join } from "node:path"; import { GitHub, @@ -26,7 +26,7 @@ import { requiredChecks, tryGit, } from "./land"; -import { fixerText, post } from "./land-fixer"; +import { post } from "./land-fixer"; import { gitAuthentication } from "./land-git"; import { finishFallback, @@ -46,20 +46,25 @@ import { because, type FailedJob, failingJobs, + jobBlocks, jobsSection, listed, login, type QueuePick, type Removal, + removalLog, } from "./land-watch"; -import { EXCERPT, excerpt, save } from "./land-serve"; +import { excerpt, save } from "./land-serve"; import { advanceFixes, type FixContext, isHeld, readPull, readRemoval, requeuedHeads } from "./land-watch-fix"; /** The check run each pick in the own queue carries, showing its place; informational, never a required context. */ export const QUEUE_CHECK = "quest queue"; -/** The branch each build is pushed to, outside the reserved `quest/` namespace, which the queue workflow builds. */ -export const queueBranch = (base: string, number: number) => `quest-queue/${base}/${number}`; +/** Where a base's builds are pushed, outside the reserved `quest/` namespace, for the queue workflow to build. */ +const queuePrefix = (base: string) => `quest-queue/${base}/`; + +/** The branch PR `number`'s build on `base` is pushed to. */ +export const queueBranch = (base: string, number: number) => `${queuePrefix(base)}${number}`; /** What the own queue needs beyond the API: the daemon's private repository, its pushes, and its reads. */ export interface OwnQueue { @@ -107,6 +112,7 @@ interface Shown { id: number; status: View["status"]; title: string; + url?: string; } const buildPath = (state: string) => join(state, "queue.json"); @@ -129,6 +135,10 @@ class Queue { readonly name: string; readonly remote: string; readonly lines: string[] = []; + /** The PRs this pass dropped or tried to land, which its plan still lists as picks. */ + readonly ended = new Set(); + /** Whether this pass tried to land a pick, which moves the base the next build needs. */ + landing = false; private shown: Record; constructor( @@ -149,7 +159,13 @@ class Queue { /** Show `view` on PR `number`'s head `head`, writing only a change, and a new check run once one completed. */ async show(number: number, head: string, view: View): Promise { const shown = this.shown[number]; - if (shown?.head === head && shown.status === view.status && shown.title === view.title) return; + if ( + shown?.head === head && + shown.status === view.status && + shown.title === view.title && + shown.url === view.url + ) + return; // A head the PR left, as for the queue's own base merge before it lands, keeps no unfinished check. if (shown && shown.head !== head && shown.status !== "completed") await this.github.request(`/repos/${this.name}/check-runs/${shown.id}`, { @@ -176,7 +192,13 @@ class Queue { }); id = Number(object(await response.json()).id); } - this.shown[number] = { head, id, status: view.status, title: view.title }; + this.shown[number] = { + head, + id, + status: view.status, + title: view.title, + ...(view.url ? { url: view.url } : {}), + }; save(shownPath(this.queue.state), this.shown); } @@ -235,7 +257,7 @@ class Queue { /** Delete the queue branches of this base that no build holds, as a crashed daemon leaves them. */ async prune(keep?: string): Promise { - const prefix = `refs/heads/${queueBranch(this.base, 0).replace(/0$/, "")}`; + const prefix = `refs/heads/${queuePrefix(this.base)}`; const listing = await this.queue.read(this.queue.root, ["ls-remote", this.remote, `${prefix}*`]); for (const line of listing.split("\n").filter(Boolean)) { const ref = line.split("\t")[1]; @@ -310,19 +332,12 @@ class Queue { const { entry } = build; const number = entry.number; const head = entry.head; - const log = join(this.queue.state, `removal-${number}-${at.replace(/[^0-9TZ]/g, "")}.log`); - writeFileSync( - log, - [ - `The queue removed ${entry.url} at ${head} for ${reason}, on ${this.base} at ${build.base}.`, - ...(detail ? [`\n${detail}`] : []), - ...jobs.map( - (job) => - `\n== ${job.workflow} / ${job.name}: ${job.conclusion}${job.step ? ` at step ${job.step}` : ""} (${job.url}) ==\n${job.log}`, - ), - "", - ].join("\n"), - { mode: 0o600 }, + const log = removalLog( + this.queue.state, + number, + at, + `The queue removed ${entry.url} at ${head} for ${reason}, on ${this.base} at ${build.base}.${detail ? `\n\n${detail}` : ""}`, + jobs, ); const fixed: Entry = { ...entry, enabledAt: build.pick.at, approved: head }; const removal: Removal = { @@ -348,17 +363,14 @@ class Queue { blame: [number], }, }; - const share = Math.floor(EXCERPT / Math.max(1, jobs.length)); - const blocks = jobs.map( - (job) => - `\n\n${fixerText(`${job.workflow} / ${job.name}`)} ([job](${job.url})) ${job.conclusion}${job.step ? ` at step ${fixerText(job.step)}` : ""}${job.log ? `:\n\n
${excerpt(job.log.trimEnd(), share)}
` : "."}`, - ); + const blocks = jobBlocks(jobs); const missing = jobs.length === 0 && reason !== "merge_conflict" ? "\n\nThe watcher found no failing job in the queue build's runs." : ""; const comment = `quest land watch: the queue removed this pull request at ${head} because ${because(reason, this.app, this.base, "queue")} (\`${reason}\`).${AFTER.fix(head, this.fixing)}${missing}`; build.drop = { removal, comment, blocks }; + this.ended.add(number); save(buildPath(this.queue.state), build); await this.finishDrop(build); } @@ -404,6 +416,8 @@ class Queue { async land(build: Build, plan: Plan, entry: Entry): Promise { const { root, state, method, token, read } = this.queue; const number = entry.number; + this.ended.add(number); + this.landing = true; const test = build.test!; const prepared = await prepareHead(root, state, plan, entry, build.base, test.tree, token, read); if ("fallback" in prepared) { @@ -627,10 +641,11 @@ export async function ownPass( } } - // The next pick waits for the next pass once one landed, since the base it builds on moved. - if (!build && !own.lines.some((line) => line.startsWith("landed "))) { + // After a landing, the next pick waits for the next pass, which reads the base it moved to, and + // rebuilds first a pick whose landing found the base moved. + if (!build && !own.landing) { for (const entry of plan.queue) { - if (held.has(entry.number)) continue; + if (held.has(entry.number) || own.ended.has(entry.number)) continue; build = await own.start(plan, entry); if (build) break; again = true; @@ -638,8 +653,9 @@ export async function ownPass( } const open = new Set(); - const picks = plan.queue.filter((entry) => !held.has(entry.number)); + const picks = plan.queue.filter((entry) => !held.has(entry.number) && !own.ended.has(entry.number)); for (const entry of plan.queue) { + if (own.ended.has(entry.number)) continue; open.add(entry.number); if (entry.number === build?.entry.number) { if (own.lines.some((line) => line.startsWith(`building ${entry.number} `))) diff --git a/src/cli/land-watch.ts b/src/cli/land-watch.ts index 305a7d0b..1a134c78 100644 --- a/src/cli/land-watch.ts +++ b/src/cli/land-watch.ts @@ -398,6 +398,35 @@ export function because(reason: string, actor: string, base: string, build = "`m } } +/** + * Write a removal's full log, `removal--.log` in `state`: `summary`, then each failing job's + * log, which the land fixer reads. Returns its path. + */ +export function removalLog( + state: string, + number: number, + at: string, + summary: string, + jobs: (FailedJob & { log: string })[], +): string { + const path = join(state, `removal-${number}-${at.replace(/[^0-9TZ]/g, "")}.log`); + const sections = jobs.map( + (job) => + `\n== ${job.workflow} / ${job.name}: ${job.conclusion}${job.step ? ` at step ${job.step}` : ""} (${job.url}) ==\n${job.log}`, + ); + writeFileSync(path, [summary, ...sections, ""].join("\n"), { mode: 0o600 }); + return path; +} + +/** Each failing job's block for a removal's comment, its log's tail sharing the excerpt's room with the others. */ +export function jobBlocks(jobs: (FailedJob & { log: string })[]): string[] { + const share = Math.floor(EXCERPT / Math.max(1, jobs.length)); + return jobs.map( + (job) => + `\n\n${fixerText(`${job.workflow} / ${job.name}`)} ([job](${job.url})) ${job.conclusion}${job.step ? ` at step ${fixerText(job.step)}` : ""}${job.log ? `:\n\n
${excerpt(job.log.trimEnd(), share)}
` : "."}`, + ); +} + /** * The failing jobs' blocks that fit in `room`, in order, so a PR whose workflow names many jobs, or * long ones, cannot make the comment too long to post and stall the watcher on it. @@ -500,26 +529,14 @@ async function handle( : { jobs: [] }; const built = /^[0-9a-f]{40}$/.test(build.base ?? "") ? build.base! : base.sha; - const log = join(state, `removal-${number}-${at.replace(/[^0-9TZ]/g, "")}.log`); - const summary = `The merge queue removed ${string(pull.url)} at ${head} for ${reason} (${actor || "unknown"}), on ${base.ref} at ${built}.`; - writeFileSync( - log, - [ - summary, - ...build.jobs.map( - (job) => - `\n== ${job.workflow} / ${job.name}: ${job.conclusion}${job.step ? ` at step ${job.step}` : ""} (${job.url}) ==\n${job.log}`, - ), - "", - ].join("\n"), - { mode: 0o600 }, - ); - - const share = Math.floor(EXCERPT / Math.max(1, build.jobs.length)); - const blocks = build.jobs.map( - (job) => - `\n\n${fixerText(`${job.workflow} / ${job.name}`)} ([job](${job.url})) ${job.conclusion}${job.step ? ` at step ${fixerText(job.step)}` : ""}${job.log ? `:\n\n
${excerpt(job.log.trimEnd(), share)}
` : "."}`, + const log = removalLog( + state, + number, + at, + `The merge queue removed ${string(pull.url)} at ${head} for ${reason} (${actor || "unknown"}), on ${base.ref} at ${built}.`, + build.jobs, ); + const blocks = jobBlocks(build.jobs); const missing = build.jobs.length === 0 && reason !== "merge_conflict" && FIXABLE.includes(reason) ? `\n\nThe watcher found no failing \`merge_group\` job for it before ${at}.` diff --git a/test/land.test.ts b/test/land.test.ts index 74e9b096..68cf0ba3 100644 --- a/test/land.test.ts +++ b/test/land.test.ts @@ -43,7 +43,7 @@ import { apply, parseResolution, pushCommand, resolutionSection } from "../src/c import fixerPrompt from "../assets/land-fix.md"; import { COMMENT_LIMIT, FLAKY_INPUT, fix, parseBaseFix, parseFixResult } from "../src/cli/land-fix"; import { resolvePreparedQueue, prepareHead, finishFallback } from "../src/cli/land-head"; -import type { GitHubDependencies } from "../src/cli/github"; +import { GitHub, type GitHubDependencies } from "../src/cli/github"; import { type Plan, type Target, @@ -54,8 +54,12 @@ import { build, mergeBase, pin, + readRemote, resolveQueue, } from "../src/cli/land"; +import { type OwnQueue, ownPass, QUEUE_CHECK, queueBranch } from "../src/cli/land-queue"; +import { ownQueue, readRemoval, watchPermissions } from "../src/cli/land-watch"; +import type { FixContext } from "../src/cli/land-watch-fix"; import { batchable, parked } from "../src/cli/land-park"; import { apiDependencies, @@ -2041,7 +2045,7 @@ mv '${result}.new' '${result}' expect(forge.merges.map(({ number }) => number)).toEqual([1, 2]); expect(warnings).toHaveLength(1); expect(warnings[0]).toContain("CRITICAL"); - expect(warnings[0]).toContain("tested chain"); + expect(warnings[0]).toContain("but the tested one is"); expect(forge.repo.pulls.slice(0, 2).every((pull) => pull.merged)).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); @@ -4263,3 +4267,297 @@ describe("land fixer stop rules", () => { expect(stopReason("requeued", round("flaky"))).toContain("last judged flaky"); }); }); + +/** The test commit the own queue pushed for PR `number`, if its branch exists. */ +function queued(forge: Forge, number: number): string | null { + const found = spawnSync("git", ["-C", forge.origin, "rev-parse", `refs/heads/${queueBranch("main", number)}`], { + encoding: "utf8", + }); + return found.status === 0 ? found.stdout.trim() : null; +} + +/** The queue workflow's run on PR `number`'s queue branch, reporting the required `test` check there. */ +function report(forge: Forge, number: number, conclusion: "SUCCESS" | "FAILURE" | null, log = "") { + const sha = queued(forge, number)!; + if (conclusion) + forge.repo.commitRuns = { + ...forge.repo.commitRuns, + [sha]: [{ name: "test", conclusion, url: "https://ci.test/queue" }], + }; + const id = (forge.repo.actionRuns ?? []).length + 1; + (forge.repo.actionRuns ??= []).push({ + id, + name: "Queue", + head_branch: queueBranch("main", number), + head_sha: sha, + status: conclusion ? "completed" : "in_progress", + conclusion: conclusion?.toLowerCase() ?? null, + html_url: `https://github.com/upstream/project/actions/runs/${id}`, + jobs: [ + { + id: id * 10, + name: "Build", + status: conclusion ? "completed" : "in_progress", + conclusion: conclusion?.toLowerCase() ?? null, + html_url: `https://github.com/upstream/project/actions/runs/${id}/job/${id * 10}`, + steps: [{ name: "Test", conclusion: conclusion?.toLowerCase() ?? null }], + log, + }, + ], + }); +} + +/** The latest queue check each PR's current head shows, as `status title`. */ +function shown(forge: Forge): Record { + const found: Record = {}; + for (const pull of forge.repo.pulls) { + const run = (forge.repo.checkRuns ?? []) + .filter((item) => item.name === QUEUE_CHECK && item.head_sha === pull.head) + .at(-1); + if (run) found[pull.number] = `${run.status ?? run.conclusion} ${String(run.output.title)}`; + } + return found; +} + +describe.skipIf(process.platform === "win32")("the watcher's own queue", () => { + function setup(options: Partial = {}) { + const forge = new Forge(); + const queue: OwnQueue = { + root: forge.clone(), + state: tempDir(), + method: "squash", + timeout: 60, + token: async () => "", + read: readRemote, + ...options, + }; + const now = Date.parse("2026-11-07T12:00:00Z"); + /** Requests GitHub refuses, each once. */ + const refuse: ((url: string, init?: RequestInit) => boolean)[] = []; + /** One pass at `minutes` after the first, as the daemon runs it, and GitHub's view after it. */ + const pass = async (minutes = 0, fix?: FixContext) => { + const forged = await forge.deps({ checks: "write", actions: "write" }); + const deps: GitHubDependencies = { + ...forged, + now: () => now + minutes * 60_000, + fetch: (async (input, init) => { + const index = refuse.findIndex((matches) => matches(String(input), init)); + if (index < 0) return forged.fetch(input, init); + refuse.splice(index, 1); + return Response.json({ message: "Validation Failed" }, { status: 422 }); + }) as typeof fetch, + }; + queue.token = async () => deps.token; + if (fix) fix.token = queue.token; + const result = await ownPass(REPOSITORY, "main", queue, deps, fix); + forge.settle(); + // GitHub knows each pushed queue commit, with no checks until the workflow reports them. + for (const pull of forge.repo.pulls) { + const sha = queued(forge, pull.number); + if (sha) (forge.repo.commitRuns ??= {})[sha] ??= []; + } + return result.lines.map((line) => line.split(" ").slice(0, 2).join(" ")); + }; + return { forge, queue, pass, refuse }; + } + + test("tests one pick at a time on the base, shows each its place, and lands exactly the tested tree", async () => { + const { forge, pass } = setup(); + const one = forge.pull("One", { "one.txt": "1\n" }); + const two = forge.pull("Two", { "two.txt": "2\n" }); + const base = forge.main(); + expect(await pass()).toEqual(["building 1"]); + const tested = queued(forge, 1)!; + // The base merged into the pick, with the base as its second parent, which the workflow builds on. + expect(git(forge.origin, "rev-parse", `${tested}^1`, `${tested}^2`).split("\n")).toEqual([one.head, base]); + expect(shown(forge)).toEqual({ 1: "in_progress Building on main", 2: "queued Position 2 of 2" }); + // Pending checks wait, writing nothing new. + const writes = forge.repo.checkRuns!.length; + expect(await pass()).toEqual([]); + expect(forge.repo.checkRuns!.length).toBe(writes); + report(forge, 1, null); + await pass(); + expect(forge.repo.checkRuns!.filter((run) => run.head_sha === one.head).at(-1)!.details_url).toBe( + "https://github.com/upstream/project/actions/runs/1", + ); + + report(forge, 1, "SUCCESS"); + expect(await pass()).toEqual(["landed 1"]); + expect(one.merged).toBe(true); + expect(git(forge.origin, "rev-parse", "main^{tree}")).toBe(git(forge.origin, "rev-parse", `${tested}^{tree}`)); + expect(queued(forge, 1)).toBeNull(); + expect(shown(forge)[1]).toBe("success Landed"); + // The next pick builds on the base the landing moved. + expect(await pass()).toEqual(["building 2"]); + expect(git(forge.origin, "rev-parse", `${queued(forge, 2)!}^2`)).toBe(forge.main()); + report(forge, 2, "SUCCESS"); + expect(await pass()).toEqual(["landed 2"]); + expect(two.merged).toBe(true); + expect(git(forge.origin, "show", "main:one.txt", "main:two.txt")).toBe("1\n2"); + }); + + test("lands merge commits with --method merge", async () => { + const { forge, pass } = setup({ method: "merge" }); + const one = forge.pull("One", { "one.txt": "1\n" }); + const base = forge.main(); + await pass(); + report(forge, 1, "SUCCESS"); + expect(await pass()).toEqual(["landed 1"]); + expect(git(forge.origin, "rev-parse", "main^1")).toBe(base); + expect(git(forge.origin, "merge-base", "--is-ancestor", one.head!, "main")).toBe(""); + }); + + test("a fork that refuses maintainer pushes lands by fast-forwarding the base to the tested tree", async () => { + const { forge, pass } = setup(); + const fork = forge.pull("Fork", { "fork.txt": "f\n" }, { contributor: "outside", edits: false }); + forge.advance({ "other.txt": "moved\n" }); + const base = forge.main(); + await pass(); + const tested = queued(forge, 1)!; + report(forge, 1, "SUCCESS"); + expect(await pass()).toEqual(["landed 1"]); + expect(git(forge.origin, "rev-parse", "main^1")).toBe(base); + expect(git(forge.origin, "rev-parse", "main^{tree}")).toBe(git(forge.origin, "rev-parse", `${tested}^{tree}`)); + expect(fork.closed).toBe(true); + expect(forge.comment(1)).toContain("by quest land"); + expect(shown(forge)[1]).toBe("success Landed"); + }); + + test("a failed build is dropped as GitHub's queue removes a PR, and recorded for the land fixer", async () => { + const { forge, queue, pass } = setup(); + const one = forge.pull("One", { "one.txt": "1\n" }); + forge.pull("Two", { "two.txt": "2\n" }); + const base = forge.main(); + await pass(); + report(forge, 1, "FAILURE", "error: one.test failed\n"); + expect(await pass()).toEqual(["removed 1", "building 2"]); + expect(one.landing).toBe(false); + expect(forge.comment(1)).toContain("the queue removed this pull request at"); + expect(forge.comment(1)).toContain("because its queue build failed (`failed_checks`)"); + expect(forge.comment(1)).toContain("error: one.test failed"); + const removal = readRemoval(queue.state, 1)!; + expect(removal.verdict).toBe("fix"); + expect(removal.jobs.map((job) => [job.name, job.step])).toEqual([["Build", "Test"]]); + expect(removal.failure!.plan.base).toEqual({ ref: "main", sha: base }); + expect(removal.failure!.plan.queue[0].head).toBe(one.head); + expect(shown(forge)).toEqual({ 1: "failure Removed from the queue", 2: "in_progress Building on main" }); + expect(queued(forge, 1)).toBeNull(); + }); + + test("a build past its timeout is dropped, and its runs cancelled", async () => { + const { forge, queue, pass } = setup({ timeout: 30 }); + forge.pull("One", { "one.txt": "1\n" }); + await pass(); + report(forge, 1, null); + expect(await pass(29)).toEqual([]); + expect(await pass(30)).toEqual(["removed 1"]); + expect(readRemoval(queue.state, 1)!.reason).toBe("checks_timed_out"); + expect(forge.repo.actionRuns![0].conclusion).toBe("cancelled"); + }); + + test("a moved base rebuilds the pick with no drop, and a pick that ends stops its build", async () => { + const { forge, queue, pass } = setup(); + const one = forge.pull("One", { "one.txt": "1\n" }); + forge.pull("Two", { "two.txt": "2\n" }); + await pass(); + const first = queued(forge, 1); + report(forge, 1, null); + forge.advance({ "other.txt": "admin bypass\n" }); + expect(await pass()).toEqual(["rebuilding 1", "building 1"]); + expect(queued(forge, 1)).not.toBe(first); + expect(git(forge.origin, "rev-parse", `${queued(forge, 1)}^2`)).toBe(forge.main()); + expect(forge.repo.actionRuns![0].conclusion).toBe("cancelled"); + expect(readRemoval(queue.state, 1)).toBeUndefined(); + // A person disarms it: the build stops, and the next pick starts. + one.landing = false; + delete one.autoMerge; + expect(await pass()).toEqual(["left 1", "building 2"]); + expect(queued(forge, 1)).toBeNull(); + expect(shown(forge)[1]).toBe("neutral Left the queue"); + }); + + test("a conflict the quest merge driver cannot merge is dropped before any build", async () => { + const { forge, queue, pass } = setup(); + forge.pull("Clash", { "code.txt": "one\nII\nthree\n" }); + forge.advance({ "code.txt": "one\n2\nthree\n" }); + expect(await pass()).toEqual(["removed 1"]); + expect(readRemoval(queue.state, 1)!.reason).toBe("merge_conflict"); + expect(forge.comment(1)).toContain(`because it conflicts with`); + expect(queued(forge, 1)).toBeNull(); + }); + + test("a held pick keeps its place unbuilt, and stale queue branches are pruned on the first pass", async () => { + const { forge, pass } = setup(); + const one = forge.pull("One", { "one.txt": "1\n" }); + forge.pull("Two", { "two.txt": "2\n" }); + one.labels = ["quest:active"]; + git(forge.origin, "update-ref", `refs/heads/${queueBranch("main", 9)}`, forge.main()); + expect(await pass()).toEqual(["building 2"]); + expect( + spawnSync("git", ["-C", forge.origin, "rev-parse", `refs/heads/${queueBranch("main", 9)}`]).status, + ).not.toBe(0); + expect(shown(forge)).toEqual({ 1: "queued Held", 2: "in_progress Building on main" }); + }); + + test("a drop interrupted before its comment is finished on the next pass, once", async () => { + const { forge, pass, refuse } = setup(); + const one = forge.pull("One", { "one.txt": "1\n" }); + await pass(); + report(forge, 1, "FAILURE", "boom\n"); + // GitHub refuses the comment once, after the drop disarmed the PR. + refuse.push( + (url, init) => url.endsWith("/issues/1/comments") && init?.method !== "GET" && init?.body !== undefined, + ); + await expect(pass()).rejects.toThrow(); + expect(one.landing).toBe(false); + expect(await pass()).toEqual(["removed 1"]); + expect(await pass()).toEqual([]); + expect(one.comments!.filter(({ body }) => body.includes("the queue removed this pull request"))).toHaveLength( + 1, + ); + }); + + test("the land fixer's requeue arms auto-merge pinned to the head, at the back of the queue", async () => { + const { forge, queue, pass } = setup(); + const one = forge.pull("One", { "one.txt": "1\n" }); + const two = forge.pull("Two", { "two.txt": "2\n" }); + const tools = tempDir(); + const wrapper = join(tools, "wrapper"); + writeFileSync( + wrapper, + `#!/bin/sh\nset -eu\nprintf '[{"number":%s,"action":"requeue","reason":"healthy","failure":"just test: render","cause":"flaky"}]' "$2" > quest-land-result.json\n`, + { mode: 0o755 }, + ); + const fix: FixContext = { + root: queue.root, + fixer: wrapper, + jobs: 1, + effort: "medium", + timeout: 60, + token: queue.token, + read: readRemote, + }; + await pass(0, fix); + report(forge, 1, "FAILURE", "flaked\n"); + const finished = eventFile(queue.state, (name) => name.endsWith(".exit")); + expect(await pass(0, fix)).toEqual(["removed 1", "building 2", "fixing 1"]); + await finished; + expect(await pass(0, fix)).toEqual(["requeued 1"]); + expect(one.landing).toBe(true); + expect(one.autoMerge!.commitBody).toBe(pin(one.head!)); + expect(one.autoMerge!.enabledAt > two.autoMerge!.enabledAt).toBe(true); + report(forge, 2, "SUCCESS"); + expect(await pass(0, fix)).toEqual(["landed 2"]); + expect(await pass(0, fix)).toEqual(["building 1"]); + }); +}); + +test("the watcher picks its queue from the base's rules each pass", async () => { + const forge = new Forge(); + const deps = await forge.deps(); + expect(await ownQueue(new GitHub(deps), REPOSITORY, "main")).toBe(true); + forge.repo.rules = [{ type: "merge_queue", parameters: {} }]; + expect(await ownQueue(new GitHub(deps), REPOSITORY, "main")).toBe(false); + expect(watchPermissions(false, true).api).toMatchObject({ checks: "write", actions: "write" }); + expect(watchPermissions(true).api).not.toHaveProperty("checks", "write"); +}); From 5a8fe79f84d81282a2347bbfea180349faa2e67d Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 11:27:39 -0800 Subject: [PATCH 04/12] fix(land): read the queue mode with a read-only token, and cancel orphaned builds when pruning Co-Authored-By: Claude Opus 5.5 --- README.md | 3 ++- src/cli/land-queue.ts | 8 ++++++-- src/cli/land-watch.ts | 6 ++++-- test/land.test.ts | 14 +++++++++++++- 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index a681a09f..989726a2 100644 --- a/README.md +++ b/README.md @@ -827,7 +827,8 @@ base's rules have a `merge_queue` rule, and otherwise which offers no merge queue. Each pass reads the base's rules (`GET /rules/branches/`) and picks the mode from them, so enabling GitHub's queue on a repository switches its watcher with no host change; a -pass that cannot read them does nothing. On GitHub's queue it tells each PR the queue +pass that cannot read them does nothing. It reads them with a `contents: read` +token, so an installation granted only one mode's permissions runs it. On GitHub's queue it tells each PR the queue drops unmerged why. With `--fixer`, it also runs the land fixer on each PR the queue dropped for a failure or a conflict, and puts it back at the back of the queue. It polls; the host needs no public endpoint. diff --git a/src/cli/land-queue.ts b/src/cli/land-queue.ts index 73db25a0..d0b0d1d9 100644 --- a/src/cli/land-queue.ts +++ b/src/cli/land-queue.ts @@ -255,13 +255,17 @@ class Queue { rmSync(buildPath(this.queue.state), { force: true }); } - /** Delete the queue branches of this base that no build holds, as a crashed daemon leaves them. */ + /** + * Clear the queue branches of this base that no build holds, as a daemon that crashed between a push + * and its record leaves them, cancelling their runs so none holds the queue runner. + */ async prune(keep?: string): Promise { const prefix = `refs/heads/${queuePrefix(this.base)}`; const listing = await this.queue.read(this.queue.root, ["ls-remote", this.remote, `${prefix}*`]); for (const line of listing.split("\n").filter(Boolean)) { const ref = line.split("\t")[1]; - if (ref.startsWith(prefix) && ref !== `refs/heads/${keep}`) await this.delete(ref); + if (ref.startsWith(prefix) && ref !== `refs/heads/${keep}`) + await this.clear(ref.slice("refs/heads/".length)); } } diff --git a/src/cli/land-watch.ts b/src/cli/land-watch.ts index 1a134c78..e8ad112b 100644 --- a/src/cli/land-watch.ts +++ b/src/cli/land-watch.ts @@ -789,7 +789,9 @@ export async function watch( const github = watchPermissions(fixing); const own = watchPermissions(fixing, true); const mint = (wanted: Permissions) => installationToken(repository, options, wanted, now); - // Each mode's token is minted only once a pass needs it. + // Each mode's token is minted only once a pass needs it, and the mode is read with neither, so an + // installation granted only one mode's permissions still runs it. + const probe = apiDependencies(() => mint({ contents: "read" }), { ...now, sleep, warn }); const deps = apiDependencies(() => mint(github.api), { ...now, sleep, warn }); const ownDeps = apiDependencies(() => mint(own.api), { ...now, sleep, warn }); const token = (workflows: boolean) => mint(workflows ? own.pushWorkflows! : own.push!); @@ -815,7 +817,7 @@ export async function watch( read, }; await daemonLoop(async () => { - const api = new GitHub(deps); + const api = new GitHub(probe); const base = options.base ?? (await defaultBranch(api, repository)); if (await ownQueue(api, repository, base)) { const pass = await ownPass(repository, base, queue, ownDeps, fix); diff --git a/test/land.test.ts b/test/land.test.ts index 68cf0ba3..f7f2b914 100644 --- a/test/land.test.ts +++ b/test/land.test.ts @@ -4486,13 +4486,25 @@ describe.skipIf(process.platform === "win32")("the watcher's own queue", () => { expect(queued(forge, 1)).toBeNull(); }); - test("a held pick keeps its place unbuilt, and stale queue branches are pruned on the first pass", async () => { + test("a held pick keeps its place unbuilt, and stale queue branches and their runs are cleared on the first pass", async () => { const { forge, pass } = setup(); const one = forge.pull("One", { "one.txt": "1\n" }); forge.pull("Two", { "two.txt": "2\n" }); one.labels = ["quest:active"]; + // A build a crashed daemon pushed but never recorded, still running. git(forge.origin, "update-ref", `refs/heads/${queueBranch("main", 9)}`, forge.main()); + (forge.repo.actionRuns ??= []).push({ + id: 99, + name: "Queue", + head_branch: queueBranch("main", 9), + head_sha: forge.main(), + status: "in_progress", + conclusion: null, + html_url: "https://github.com/upstream/project/actions/runs/99", + jobs: [], + }); expect(await pass()).toEqual(["building 2"]); + expect(forge.repo.actionRuns![0].conclusion).toBe("cancelled"); expect( spawnSync("git", ["-C", forge.origin, "rev-parse", `refs/heads/${queueBranch("main", 9)}`]).status, ).not.toBe(0); From 39f8659f21c73fff0084fe40b60ebe0c0ba2e298 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 11:32:04 -0800 Subject: [PATCH 05/12] chore(quest): claim quest/m0/pin-sync.md Quest-Claim: 7268442731a54c2f From 1b87b940b9b6735b19c05840d56e0b92d4d2f924 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 11:53:10 -0800 Subject: [PATCH 06/12] feat(land): the quest watcher bumps quest's pin in other repositories quest land watch --bump keeps each target's flake input on the watched repository at the watched base: it opens or updates one quest-bump PR per target as the App, with the input unpinned from flake.nix, locked by Nix, and quest init and quest check --fix run with the new quest, then lands it through quest merge's path. requirements() reads no rules from a private repository on GitHub Free instead of failing on its 403. Co-Authored-By: Claude Opus 5.5 --- README.md | 43 +++++ quest/m0/README.md | 2 +- quest/m0/pin-sync.md | 85 ++++---- src/cli/github.ts | 19 +- src/cli/land-bump.ts | 428 +++++++++++++++++++++++++++++++++++++++++ src/cli/land-watch.ts | 51 ++++- src/cli/land.ts | 2 +- src/cli/main.ts | 20 +- test/land-bump.test.ts | 393 +++++++++++++++++++++++++++++++++++++ 9 files changed, 986 insertions(+), 57 deletions(-) create mode 100644 src/cli/land-bump.ts create mode 100644 test/land-bump.test.ts diff --git a/README.md b/README.md index 989726a2..68fd1beb 100644 --- a/README.md +++ b/README.md @@ -1012,6 +1012,49 @@ cancel runs, `checks: write` for its check runs, `statuses: read`, and does. Its pushes ask for `contents: write`, and `workflows: write` for one that carries a workflow change. +### Pin bumps + +With `--bump `, repeatable, the watcher keeps each named +repository's flake input on the watched repository at the watched base, so +every push to the base reaches those repositories with no one touching them. +It needs `nix` on `PATH`. Once a pass sees the base at a commit a target does +not pin yet, it opens or updates that target's one `quest-bump` PR as the App: + +1. It reads the target's `flake.lock` on its default branch for the one root + input that locks the watched repository on GitHub; a target already at the + commit gets nothing, and an open bump PR there is closed. +2. In a fresh shallow checkout it drops a revision from that input's URL in + `flake.nix`, so `flake.lock` alone pins it from then on, and locks the input + at the commit with `nix flake lock --override-input`. +3. It builds the watched repository's `quest` at that commit with `nix build`, + and runs its `quest init` and `quest check --fix` in the checkout. +4. It commits the result as moq-bot and lease-pushes it to `quest-bump`. An open + bump PR's head becomes the new commit's first parent, beside the target's + base, so the branch never rewinds; any other `quest-bump` branch, left by a + merged or closed PR, is replaced. Auto-merge on the open PR is disarmed + before the push. The PR body links the comparison and records the review as + skipped, since a pin bump changes no repository code and the target's CI is + the gate. +5. Findings `quest check --fix` cannot resolve leave the PR a draft with a + `## Waiting` item quoting them. Otherwise it lands like any PR there, through + `quest merge`'s path: into GitHub's merge queue, the target's own queue, or + auto-merge. A base that requires no check would merge at once, so there the + bump first waits for every check on its head to pass, then merges directly + when the repository has no auto-merge, as on a private repository on GitHub + Free, whose rules GitHub does not serve. A failed check, or a landing + `quest merge` refuses, is commented on and left for a maintainer. + +From there the target's own gate and fixer take the PR, as any other; a newer +commit on the base replaces it with a fresh bump. `bump.json` in the state +directory records each target's last bump, so a restart repeats nothing and a +closed bump is not reopened for the same commit. A target's failure is logged +and retried on the next pass without holding up the others. Each target gets +its own tokens: `contents: write`, `pull_requests: write`, `merge_queues: +write`, and reads of `administration`, `checks`, and `statuses` for its API +calls, and `workflows: write` for a push whose diff from the branch's old head +carries a workflow change. Nix reads GitHub with the watched repository's +`contents: read` token. + ## Gate expensive commands `quest gate [args...]` holds an expensive command until the host has diff --git a/quest/m0/README.md b/quest/m0/README.md index fa0cc7c9..cde8b609 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -98,7 +98,7 @@ ownership-manifest design: - [Review sessions keyed by upstream branch](/quest/m0/review-session-key.md) - a round from a local branch tracking the PR branch continues that branch's review session - [Kix decision UI](/quest/m0/kix/README.md) - planning and implementation through prompts and summaries, run on the maintainer's own runners - [Route quests to models by size](/quest/m0/model-routing/README.md) - `/quest-spawn` starts each quest on the first model of its size's ranked list that can run, reviews rank the same way, and PRs record size, implementer, and reviewer -- [Repositories and hosts follow quest's main](/quest/m0/pin-sync.md) - every push to main reaches each enrolled repository and host through a bot-queued pin bump +- [Hosts run quest's pin bumps](/quest/m0/pin-sync.md) - the host runs the watcher's pin bumps for every repository pinning quest, and each host upgrades from kixelated/nixos's main - [Cut agent token waste](/quest/m0/token-waste/README.md) - sessions stop paying for loading rules, a second review pass, polling, oversized coordinators, and merge churn - [quest check rejects a misleveled Waiting heading](/quest/m0/waiting-heading-level.md) - a `### Waiting` (or any non-`##` level) fails `quest check` with its file and line, so no wait drops out of `quest waiting` - [Model check](/quest/m0/model-check.md) - recurring: the first audit of the routed PRs retunes the built-in size lists diff --git a/quest/m0/pin-sync.md b/quest/m0/pin-sync.md index 4a6666e1..f9a21781 100644 --- a/quest/m0/pin-sync.md +++ b/quest/m0/pin-sync.md @@ -1,58 +1,45 @@ -# [L] Repositories and hosts follow quest's main +# [S] Hosts run quest's pin bumps and upgrade on their own ## Goal -Every push to moq-dev/quest's `main` reaches each enrolled repository and -each host without a person or an agent touching it. The maintainer's host -opens or updates one pin-bump PR per repository, as moq-bot, and queues it. -kixelated/nixos is a target too, and its hosts apply its `main` on their -own, so the queue watcher and gate run the same quest within the hour. +Every push to moq-dev/quest's `main` reaches each repository that pins quest, +and each host, without a person or an agent touching it: the quest watcher on +the maintainer's host runs `quest land watch --bump` for every such +repository, kixelated/nixos included, and each host applies kixelated/nixos's +`main` through `system.autoUpgrade`, so the queue watchers run the quest +`main` has within the hour. ## Plan -Decided 2026-11-05 (user, /quest-plan, after the -[token waste](/quest/m0/token-waste/README.md) plan merged): "it's kind of -annoying to need to update every PR and repo." - -Measured that day: moq-dev/moq, studio, and rind pinned quest at -`4be3508`, rust2 at `dc8008b`, kixel.ai at `5790eea`, and kixelated/nixos at -`91714ab`; moq.pro and quest itself track `main` through their lockfiles. Each -dev shell puts its pinned `quest` first on `PATH`, so a skill or rule change -reaches a repository's agents only after a bump PR there. The NixOS hosts -change only on a manual `nixos-rebuild switch`. Machines' dotfiles now pull -themselves hourly ([kixelated/dotfiles#42](https://github.com/kixelated/dotfiles/pull/42)), -so this quest covers only the repositories and hosts. - -Decision prompts, chosen options marked ✅: - -- How updates reach repositories: ✅ the host bot bumps (rejected: - Dependabot with auto-merge, which lags a day and needs a write token in - each repository's workflows; unpinning agent content to a user-level - `quest`, which lets two versions disagree on the format). The nixos host - already holds moq-bot's App credentials and the enrolled repositories. -- Review of a bump PR: ✅ the bot records a `## Review` skip, since a pin - bump changes no repository code and each repository's CI, running - `quest check` with the new quest, is the gate (rejected: a Codex round per - bump). -- A format change: ✅ the bot runs `quest check --fix` with the new quest and - commits the result in the bump PR; a finding `--fix` cannot resolve leaves - the PR open with a `## Waiting` item (rejected: hand the failing PR to an - agent). -- Cadence: ✅ every push to `main`, updating the one open bump PR per - repository so several merges coalesce (rejected: tagged releases only). -- Hosts: ✅ kixelated/nixos is a bump target, and each host enables - `system.autoUpgrade` from its `main` (rejected: bump with manual deploys; - leave the hosts). - -Recommendations for the implementer: - -- Pin every repository the same way, a branch URL with the revision only in - `flake.lock`, so a bump is one `nix flake update` of the quest input. -- Reuse the queue watcher's App client, enrolled-repository list, and - enqueue path rather than a second daemon's. -- The kixelated/nixos half (the bump target and `system.autoUpgrade`) is a - PR there; check that an upgrade does not restart the queue watcher in the - middle of a fixer job. +The bump pass itself is done (README, "Pin bumps"). What remains is the host +configuration in kixelated/nixos, drafted on its `quest-pin-sync` branch: + +- `modules/quest-land.nix` passes `--bump` to the quest watcher for every other + land repository plus moq-dev/studio, moq-dev/kixel.ai, and kixelated/nixos, + and puts Nix on its `PATH`. +- `modules/base.nix` enables `system.autoUpgrade` hourly on both hosts from + `git+ssh://git@github.com/kixelated/nixos?ref=main#`, fetched with + each host's SSH host key as a read-only deploy key. +- Check and fixer containers already survive a switch + (`restartIfChanged = false`), and a watcher's detached fixer supervisor + outlives its daemon's restart. The batching gate stops its check with it, so + it is not restarted by a switch until moq.pro moves onto the watcher + ([moq.pro cutover](/quest/m0/queue-watcher/pro-cutover.md)). + +Once the bump pass is on quest's `main`, bump that branch's quest input to it +(the watcher rejects `--bump` before then), land it, and switch each host by +hand once; from then on the hosts upgrade themselves. + +## Waiting + +- Maintainer: add each host's `/etc/ssh/ssh_host_ed25519_key.pub` (nixos and + nixos2) to kixelated/nixos as a read-only deploy key, since only the + repository's owner can grant access to it; check that + `sudo systemctl start nixos-upgrade` succeeds on each host. +- Maintainer: install moq-bot on the kixelated account for kixelated/nixos, + with the permissions it has on moq-dev, since an App installation needs the + account owner's consent; check that the quest watcher's journal shows + `bump kixelated/nixos` rather than a failed token request. ## Related diff --git a/src/cli/github.ts b/src/cli/github.ts index d2889432..32e39a36 100644 --- a/src/cli/github.ts +++ b/src/cli/github.ts @@ -495,6 +495,23 @@ export interface Requirements { classic: boolean; } +/** + * Every rule a branch's active rulesets apply. GitHub Free offers no rulesets on a private repository and + * answers 403 with an upgrade notice, so none apply there; any other refusal fails. + */ +async function branchRules(github: GitHub, path: string): Promise { + const first = await github.request(path, { allow: [403] }); + if (first.status !== 403) { + const next = link(first, "next"); + return [...array(await first.json()), ...(next ? await github.all(next) : [])]; + } + const reason = explanation(await first.text()); + if (reason?.startsWith("Upgrade to GitHub Pro")) return []; + throw new Error( + `GitHub HTTP 403 for ${new URL(path, API).pathname}; ${reason ?? "check repository access and token permissions"}`, + ); +} + /** What `branch` of `repository` requires, read with read access from branch protection and rulesets. */ export async function requirements(github: GitHub, repository: Repository, branch: string): Promise { const base = `/repos/${encodeURIComponent(repository.owner)}/${encodeURIComponent(repository.repo)}`; @@ -512,7 +529,7 @@ export async function requirements(github: GitHub, repository: Repository, branc app: null, })) : array(legacy.checks).map((row) => ({ name: string(row.context), app: source(row.app_id) })); - const rules = await github.all(`${base}/rules/branches/${ref}?per_page=100`); + const rules = await branchRules(github, `${base}/rules/branches/${ref}?per_page=100`); const checks = [ ...classic, ...rules diff --git a/src/cli/land-bump.ts b/src/cli/land-bump.ts new file mode 100644 index 00000000..7b162b45 --- /dev/null +++ b/src/cli/land-bump.ts @@ -0,0 +1,428 @@ +// Pin bumps: the source repository's watcher keeps each target repository's flake input on the source at the +// source's base, through one bot PR per target that lands like any other PR there. +import { spawnSync } from "node:child_process"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { checksState } from "../github/checks"; +import { + GitHub, + type GitHubDependencies, + object, + type ObjectValue, + type Repository, + requirements, + string, +} from "./github"; +import { land } from "./hold"; +import { commandEnvironment, contexts, git, type RemoteRead } from "./land"; +import { push, remoteHead } from "./land-head"; +import { Refusal } from "./start"; +import { excerpt, save } from "./land-serve"; + +/** The one branch each target's bump PR uses. */ +export const BUMP_BRANCH = "quest-bump"; + +/** A target's bump of the source commit `sha`, in `bump.json`, so a pass repeats nothing already done. */ +interface Bump { + sha: string; + pr?: number; + /** The head the bump pushed. */ + head?: string; + /** + * `done`: the target pins `sha`, or its PR is in the target's queue and the target's own gate and fixer + * take it from here. `waiting`: findings `quest check --fix` cannot resolve, left for a maintainer. + * `checking`: no check is required on the target's base, so the bump waits for every check on its + * head to pass before landing. `failed`: such a check failed, left for a maintainer. + */ + status: "done" | "waiting" | "checking" | "failed"; +} + +/** The commands a bump runs outside this process. */ +export interface BumpTools { + /** Lock `input` in the flake at `checkout` to `url`, with `token` reading GitHub. */ + lock(checkout: string, input: string, url: string, token: string): void; + /** The `quest` executable of the source at `sha`. */ + quest(sha: string, token: string): string; + /** Run `quest` with `args` in `checkout`. */ + run(quest: string, checkout: string, args: string[]): { status: number | null; output: string }; +} + +/** One target's GitHub access: API calls, pushes, and authenticated fetches, each with a token for that target alone. */ +export interface BumpConnection { + /** The target's Git URL. */ + remote: string; + deps: GitHubDependencies; + token: (workflows: boolean) => Promise; + read: RemoteRead; +} + +export interface BumpContext { + source: Repository; + targets: Repository[]; + state: string; + connect: (target: Repository) => BumpConnection; + /** A read-only token for the source, which the tools fetch from GitHub. */ + reader: () => Promise; + tools: BumpTools; +} + +/** The installation permissions a target's bump API calls need: `land` reads the base's protection and the head's checks. */ +export const BUMP_PERMISSIONS = { + contents: "write", + pull_requests: "write", + merge_queues: "write", + administration: "read", + checks: "read", + statuses: "read", +} as const; + +const name = (repository: Repository) => `${repository.owner}/${repository.repo}`; + +/** The root input of `lock` that locks `source` on GitHub, and its locked and URL-pinned revisions. */ +export function questInput(lock: ObjectValue, source: Repository): { input: string; locked: string; pinned?: string } { + const nodes = object(lock.nodes); + const inputs = object(object(nodes[string(lock.root)]).inputs ?? {}); + const matches = Object.entries(inputs).flatMap(([input, node]) => { + // A follows path names another node's input, not a lock of its own. + if (typeof node !== "string") return []; + const entry = object(nodes[node]); + const locked = object(entry.locked ?? {}); + const original = object(entry.original ?? {}); + const same = (value: unknown, expected: string) => + typeof value === "string" && value.toLowerCase() === expected.toLowerCase(); + if (locked.type !== "github" || !same(locked.owner, source.owner) || !same(locked.repo, source.repo)) return []; + return [ + { + input, + locked: string(locked.rev), + ...(typeof original.rev === "string" ? { pinned: original.rev } : {}), + }, + ]; + }); + if (matches.length !== 1) + throw new Error( + `flake.lock has ${matches.length} root inputs locking github:${name(source)}, where a bump needs exactly one`, + ); + return matches[0]; +} + +/** + * `flake` with the revision dropped from the source's URL, so flake.lock alone pins the input and each + * bump is a lock-only change. The URL must appear exactly once. + */ +export function unpin(flake: string, source: Repository, rev: string): string { + const url = `github:${name(source)}/${rev}`; + const found = flake.split(`"${url}"`).length - 1; + if (found !== 1) throw new Error(`flake.nix names "${url}" ${found} times, where a bump can unpin exactly one`); + return flake.replace(`"${url}"`, `"github:${name(source)}"`); +} + +/** The bump PR's body, with a `## Waiting` item for findings, and the recorded review skip. */ +export function bumpBody(options: { + source: Repository; + input: string; + from: string; + to: string; + unpinned: boolean; + changed: string[]; + findings?: string; +}): string { + const { source, input, from, to } = options; + const lines = [ + `Bumps the \`${input}\` input, github:${name(source)}, from \`${from.slice(0, 7)}\` to \`${to.slice(0, 7)}\`: https://github.com/${name(source)}/compare/${from}...${to}`, + ]; + if (options.unpinned) + lines.push("", "It drops the revision from the input's URL in flake.nix, so flake.lock alone pins it."); + const extra = options.changed.filter((path) => path !== "flake.lock" && path !== "flake.nix"); + lines.push( + "", + extra.length === 0 + ? "`quest init` and `quest check --fix` with the new quest changed nothing else." + : `\`quest init\` and \`quest check --fix\` with the new quest changed ${extra.map((path) => `\`${path}\``).join(", ")}.`, + ); + if (options.findings !== undefined) + lines.push( + "", + "## Waiting", + "", + "- Maintainer: `quest check` with the new quest reports findings `--fix` cannot resolve. Fix them on this branch and land it; the next bump replaces this branch's tree.", + "", + `
${excerpt(options.findings.trimEnd())}
`, + ); + lines.push( + "", + "## Review", + "", + "Skipped: pin bump, no repository code changed; CI's quest check is the gate.", + "", + "Opened by `quest land watch`, which replaces this branch with each newer bump.", + ); + return `${lines.join("\n")}\n`; +} + +function readBumps(state: string): Record { + const path = join(state, "bump.json"); + return existsSync(path) ? (JSON.parse(readFileSync(path, "utf8")) as Record) : {}; +} + +/** + * Bump every target to `sha`, the source's base, and land each bump. A target's failure does not stop + * the others; the pass throws once all have run, so the daemon reports it and retries. + */ +export async function bumpPass(context: BumpContext, sha: string): Promise { + const bumps = readBumps(context.state); + const output: string[] = []; + const failures: string[] = []; + for (const target of context.targets) { + const key = name(target); + if (bumps[key]?.sha === sha && bumps[key].status !== "checking") continue; + try { + const connection = context.connect(target); + // Each step is recorded before the next, so a failed landing never pushes the bump again. + if (bumps[key]?.sha !== sha) { + bumps[key] = await bump(context, target, sha, connection, output); + save(join(context.state, "bump.json"), bumps); + } + if (bumps[key].status === "checking") { + bumps[key] = await settle(target, bumps[key], connection, output); + save(join(context.state, "bump.json"), bumps); + } + } catch (error) { + failures.push(`${key}: ${error instanceof Error ? error.message : String(error)}`); + } + } + if (failures.length > 0) throw new Error(`bump failed for ${failures.join("; ")}`); + return output; +} + +/** Bump `target` to `sha`: push the bump to its branch and open or update its PR, unqueued. */ +async function bump( + context: BumpContext, + target: Repository, + sha: string, + connection: BumpConnection, + output: string[], +): Promise { + const github = new GitHub(connection.deps); + const path = `/repos/${name(target)}`; + const repository = object(await (await github.request(path)).json()); + const base = string(repository.default_branch); + const baseSha = string( + object(object(await (await github.request(`${path}/git/ref/heads/${encodeURIComponent(base)}`)).json()).object) + .sha, + ); + const [open] = await github.all( + `${path}/pulls?state=open&head=${encodeURIComponent(`${target.owner}:${BUMP_BRANCH}`)}&per_page=100`, + ); + const lock = object(await (await github.request(`${path}/contents/flake.lock?ref=${baseSha}`)).json()); + const found = questInput( + object(JSON.parse(Buffer.from(string(lock.content), "base64").toString("utf8"))), + context.source, + ); + if (found.locked === sha) { + if (open) { + await github.request(`${path}/issues/${Number(open.number)}/comments`, { + body: { + body: `quest land watch: ${base} already pins ${name(context.source)} at ${sha}, so this bump is no longer needed.`, + }, + }); + await github.request(`${path}/pulls/${Number(open.number)}`, { + method: "PATCH", + body: { state: "closed" }, + }); + output.push(`bump closed ${name(target)} ${Number(open.number)}`); + } + return { sha, status: "done" }; + } + + const remote = connection.remote; + const ref = `refs/heads/${BUMP_BRANCH}`; + mkdirSync(join(context.state, "bump"), { recursive: true, mode: 0o700 }); + const checkout = mkdtempSync(join(context.state, "bump", "checkout-")); + try { + git(checkout, ["init", "--quiet"]); + await connection.read(checkout, ["fetch", "--quiet", "--no-tags", "--depth=1", remote, baseSha]); + git(checkout, ["checkout", "--quiet", "--detach", baseSha]); + // The open PR's head stays in the branch's history; a branch left by a merged or closed PR is replaced. + const previous = open ? string(object(open.head).sha) : ""; + const lease = previous || ((await remoteHead(checkout, remote, ref, connection.read)) ?? ""); + if (lease) await connection.read(checkout, ["fetch", "--quiet", "--no-tags", "--depth=1", remote, lease]); + + const reader = await context.reader(); + if (found.pinned !== undefined) { + const flake = join(checkout, "flake.nix"); + writeFileSync(flake, unpin(readFileSync(flake, "utf8"), context.source, found.pinned)); + } + context.tools.lock(checkout, found.input, `github:${name(context.source)}/${sha}`, reader); + const locked = questInput( + object(JSON.parse(readFileSync(join(checkout, "flake.lock"), "utf8"))), + context.source, + ); + if (locked.locked !== sha || locked.pinned !== undefined) + throw new Error( + `locking ${found.input} left it at ${locked.locked}${locked.pinned ? ", pinned in flake.nix" : ""}`, + ); + const quest = context.tools.quest(sha, reader); + const init = context.tools.run(quest, checkout, ["init"]); + if (init.status !== 0) throw new Error(`quest init failed: ${init.output.trim()}`); + const check = context.tools.run(quest, checkout, ["check", "--fix"]); + const findings = check.status === 0 ? undefined : check.output; + + git(checkout, ["add", "--all"]); + const tree = git(checkout, ["write-tree"]); + const changed = git(checkout, ["diff", "--cached", "--name-only", baseSha]).split("\n").filter(Boolean); + const title = `chore: bump quest to ${sha.slice(0, 7)}`; + // A newer bump merges the open PR's head with the base, with the fresh bump as its tree, so the + // branch only moves forward and the squash lands just the bump. + const parents = previous ? ["-p", previous, "-p", baseSha] : ["-p", baseSha]; + const commit = git(checkout, ["commit-tree", tree, ...parents, "-m", title], { env: IDENTITY }); + + const id = open ? string(open.node_id) : undefined; + // GitHub keeps auto-merge armed across a push, which would land the new head unchecked by this pass. + if (id && open!.auto_merge != null) + await github.graphql( + "mutation($id: ID!) { disablePullRequestAutoMerge(input: { pullRequestId: $id }) { clientMutationId } }", + { id }, + ); + const pushed = await push(checkout, remote, baseSha, commit, ref, connection.token, lease); + if (pushed.status !== 0) throw new Error(`push failed: ${pushed.stderr.trim()}`); + + const body = bumpBody({ + source: context.source, + input: found.input, + from: found.locked, + to: sha, + unpinned: found.pinned !== undefined, + changed, + findings, + }); + const draft = findings !== undefined; + let number: number; + if (open) { + number = Number(open.number); + await github.request(`${path}/pulls/${number}`, { method: "PATCH", body: { title, body } }); + if (open.draft !== draft) + await github.graphql( + `mutation($id: ID!) { ${draft ? "convertPullRequestToDraft" : "markPullRequestReadyForReview"}(input: { pullRequestId: $id }) { clientMutationId } }`, + { id: id! }, + ); + } else { + const pull = await github.request(`${path}/pulls`, { + body: { title, head: BUMP_BRANCH, base, body, draft }, + }); + number = Number(object(await pull.json()).number); + } + output.push(`bump ${name(target)} ${number} ${sha}${draft ? " waiting" : ""}`); + return { sha, pr: number, head: commit, status: draft ? "waiting" : "checking" }; + } finally { + rmSync(checkout, { recursive: true, force: true }); + } +} + +const IDENTITY = { + GIT_AUTHOR_NAME: "moq-bot", + GIT_AUTHOR_EMAIL: "moq-bot@users.noreply.github.com", + GIT_COMMITTER_NAME: "moq-bot", + GIT_COMMITTER_EMAIL: "moq-bot@users.noreply.github.com", +}; + +/** + * Land a bump whose head is `record.head`. A base that requires checks, or has a merge queue, gates the + * PR itself, so it is queued at once. Nothing gates any other base, where landing would merge at once, + * so the bump first waits for every check on its head to pass, and then merges directly when the + * repository has no auto-merge, as on a private repository on GitHub Free. + */ +async function settle(target: Repository, record: Bump, connection: BumpConnection, output: string[]): Promise { + const github = new GitHub(connection.deps); + const path = `/repos/${name(target)}`; + const pr = record.pr!; + const head = record.head!; + const pull = object(await (await github.request(`${path}/pulls/${pr}`)).json()); + if (pull.state !== "open") return { ...record, status: "done" }; + const shown = string(object(pull.head).sha); + if (shown !== head) { + // GitHub can show the old head briefly after the push; any other head is someone else's, such as the target's fixer. + const comparison = object(await (await github.request(`${path}/compare/${shown}...${head}`)).json()); + return comparison.status === "ahead" ? record : { ...record, status: "done" }; + } + const needs = await requirements(github, target, string(object(pull.base).ref)); + const gated = needs.checks.length > 0 || needs.rules.some((rule) => rule.type === "merge_queue"); + let direct = false; + const fail = async (why: string) => { + await github.request(`${path}/issues/${pr}/comments`, { + body: { body: `quest land watch: ${why}, so this bump waits for a maintainer to fix and land it.` }, + }); + output.push(`bump failed ${name(target)} ${pr}`); + return { ...record, status: "failed" as const }; + }; + if (!gated) { + const found = await contexts(github, target, head); + const state = found.length === 0 ? "PENDING" : checksState(found); + if (state === "PENDING") return record; + if (state === "FAILURE") return fail(`a check failed on its head ${head}`); + direct = object(await (await github.request(path)).json()).allow_auto_merge !== true; + } + try { + await land({ ...target, number: pr }, head, undefined, connection.deps, direct); + } catch (error) { + if (!(error instanceof Refusal)) throw error; + return fail(`landing it was refused: ${error.message}`); + } + output.push(`bump landing ${name(target)} ${pr}`); + return { ...record, status: "done" }; +} + +/** The tools on the host: Nix locks the input and builds the source's quest; its commands get no credentials. */ +export function hostTools(source: Repository, state: string): BumpTools { + const environment = (token: string) => ({ + ...commandEnvironment(), + HOME: join(state, "bump"), + XDG_CACHE_HOME: join(state, "bump", "cache"), + NIX_CONFIG: `access-tokens = github.com=${token}`, + }); + const nix = (cwd: string, args: string[], token: string) => { + const result = spawnSync("nix", ["--extra-experimental-features", "nix-command flakes", ...args], { + cwd, + encoding: "utf8", + env: environment(token), + maxBuffer: 1 << 30, + }); + if (result.error) throw result.error; + if (result.status !== 0) throw new Error(`nix ${args.join(" ")} failed: ${result.stderr.trim()}`); + return result.stdout.trim(); + }; + const built = new Map(); + return { + lock: (checkout, input, url, token) => { + nix(checkout, ["flake", "lock", "--override-input", input, url], token); + }, + quest: (sha, token) => { + let path = built.get(sha); + if (path === undefined) { + path = join( + nix(state, ["build", "--no-link", "--print-out-paths", `github:${name(source)}/${sha}`], token), + "bin", + "quest", + ); + built.set(sha, path); + } + return path; + }, + run: (quest, checkout, args) => { + const result = spawnSync(quest, args, { + cwd: checkout, + encoding: "utf8", + env: { ...commandEnvironment(), HOME: join(state, "bump"), NO_COLOR: "1" }, + maxBuffer: 1 << 30, + }); + if (result.error) throw result.error; + return { status: result.status, output: `${result.stdout}${result.stderr}` }; + }, + }; +} + +/** Fail at startup, not on the first bump, when the host cannot run one. */ +export function checkTools(): void { + const nix = spawnSync("nix", ["--version"], { encoding: "utf8" }); + if (nix.error || nix.status !== 0) throw new Error("quest land watch --bump requires nix on PATH"); +} diff --git a/src/cli/land-watch.ts b/src/cli/land-watch.ts index e8ad112b..d5ccd3c7 100644 --- a/src/cli/land-watch.ts +++ b/src/cli/land-watch.ts @@ -40,6 +40,7 @@ import { requeuedHeads, } from "./land-watch-fix"; import { leaveOwnQueue, type OwnQueue, ownPass } from "./land-queue"; +import { BUMP_PERMISSIONS, type BumpConnection, type BumpContext, bumpPass, checkTools, hostTools } from "./land-bump"; import { enabledHead, headBefore } from "./status"; export { readRemoval } from "./land-watch-fix"; @@ -59,6 +60,8 @@ export interface WatchOptions { fixerJobs: number; /** The fixer's `model_reasoning_effort`. */ fixerEffort: string; + /** Repositories whose flake input on this repository each pass bumps to this repository's base. */ + bump: Repository[]; } /** @@ -715,6 +718,9 @@ export async function ownQueue(github: GitHub, repository: Repository, base: str type Permissions = Record; +const same = (a: Repository, b: Repository) => + a.owner.toLowerCase() === b.owner.toLowerCase() && a.repo.toLowerCase() === b.repo.toLowerCase(); + /** * The installation token permissions the watcher asks for. On GitHub's merge queue, `api` reads the * queue's events, comments, and the `merge_group` logs (`actions: read`), and enqueues the armed picks @@ -782,6 +788,9 @@ export async function watch( options.fixer = resolve(options.fixer); accessSync(options.fixer, constants.X_OK); } + if (options.bump.length > 0) checkTools(); + if (options.bump.some((target) => same(target, repository))) + throw new Error("--bump names the watched repository, which cannot bump its own pin"); const unlock = await startDaemon(root, repository, options); try { const now = { fetch, now: Date.now }; @@ -816,17 +825,51 @@ export async function watch( token, read, }; + const connections = new Map(); + const bump: BumpContext | undefined = + options.bump.length === 0 + ? undefined + : { + source: repository, + targets: options.bump, + state: options.state, + reader: () => mint({ contents: "read" }), + tools: hostTools(repository, options.state), + connect: (target) => { + const key = `${target.owner}/${target.repo}`; + let connection = connections.get(key); + if (connection === undefined) { + const scoped = (wanted: Permissions) => installationToken(target, options, wanted, now); + const push: Permissions = { contents: "write", pull_requests: "write" }; + connection = { + remote: `https://github.com/${key}.git`, + deps: apiDependencies(() => scoped(BUMP_PERMISSIONS), { ...now, sleep, warn }), + token: (workflows) => scoped(workflows ? { ...push, workflows: "write" } : push), + read: authenticatedRead(() => scoped({ contents: "read" })), + }; + connections.set(key, connection); + } + return connection; + }, + }; await daemonLoop(async () => { const api = new GitHub(probe); const base = options.base ?? (await defaultBranch(api, repository)); + let again = false; if (await ownQueue(api, repository, base)) { const pass = await ownPass(repository, base, queue, ownDeps, fix); for (const line of pass.lines) warn(line); - return pass.again; + again = pass.again; + } else { + for (const line of await leaveOwnQueue(repository, base, queue, ownDeps, fixing)) warn(line); + for (const line of await watchPass(repository, { ...options, base }, deps, fix)) warn(line); + } + if (bump) { + const ref = `/repos/${repository.owner}/${repository.repo}/git/ref/heads/${encodeURIComponent(base)}`; + const sha = string(object(object(await (await api.request(ref)).json()).object).sha); + for (const line of await bumpPass(bump, sha)) warn(line); } - for (const line of await leaveOwnQueue(repository, base, queue, ownDeps, fixing)) warn(line); - for (const line of await watchPass(repository, { ...options, base }, deps, fix)) warn(line); - return false; + return again; }, deps); } finally { unlock(); diff --git a/src/cli/land.ts b/src/cli/land.ts index 49dcc857..3442fcb4 100644 --- a/src/cli/land.ts +++ b/src/cli/land.ts @@ -449,7 +449,7 @@ export async function resolveQueue( * Every check run and commit status on `head`: from `rollup`, the first page already read with it, * when there is one, then the rest by the SHA. */ -async function contexts( +export async function contexts( github: GitHub, repository: Repository, head: string, diff --git a/src/cli/main.ts b/src/cli/main.ts index d66931c2..6b57d269 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -44,7 +44,15 @@ import { import type { AdapterName } from "../runner/adapter"; import { held, openDrafts, reach, target, upstream, upstreamRefs } from "./claims"; import { gate, QUICK_MS } from "./gate"; -import { gitHubDependencies, GitHubInputError, mergedHead, pullRequest, readRemote, remoteRepository } from "./github"; +import { + gitHubDependencies, + GitHubInputError, + mergedHead, + pullRequest, + readRemote, + remoteRepository, + type Repository, +} from "./github"; import { type Hold, finish, land, release, take } from "./hold"; import { LAND_CHECK } from "./land"; import { apply, pushCommand } from "./land-apply"; @@ -729,6 +737,16 @@ export function main(argv: string[], out: Output): number | Promise { .choices(EFFORTS) .default("medium"), ) + .option( + "--bump ", + "Keep this repository's flake input in another repository at this repository's base (repeatable): each pass opens or updates that repository's quest-bump PR as the App and lands it", + (value: string, previous: Repository[]) => { + const match = /^([\w.-]+)\/([\w.-]+)$/.exec(value); + if (!match) throw new InvalidArgumentError("expected owner/repo"); + return [...previous, { owner: match[1], repo: match[2] }]; + }, + [] as Repository[], + ) .action((options: WatchOptions) => { const origin = readRemote(root(), "origin"); const repository = origin && remoteRepository(origin).repository; diff --git a/test/land-bump.test.ts b/test/land-bump.test.ts new file mode 100644 index 00000000..ba68592f --- /dev/null +++ b/test/land-bump.test.ts @@ -0,0 +1,393 @@ +// The pin bump pass against a fake GitHub and a local target repository, with the host's Nix and quest faked. + +import { execFileSync } from "node:child_process"; +import { readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; + +import { beforeEach, describe, expect, test, vi } from "vitest"; + +import { type GitHubDependencies, requirements } from "../src/cli/github"; +import { land } from "../src/cli/hold"; +import { readRemote } from "../src/cli/land"; +import { + BUMP_BRANCH, + type BumpContext, + type BumpTools, + bumpBody, + bumpPass, + questInput, + unpin, +} from "../src/cli/land-bump"; +import { configure, GIT_SETTINGS, tempDir } from "./fixture"; + +vi.mock("../src/cli/hold", () => ({ land: vi.fn(async () => {}) })); +const landed = vi.mocked(land); + +const SOURCE = { owner: "moq-dev", repo: "quest" }; +const OLD = "1".repeat(40); +const NEW = "2".repeat(40); + +/** A flake.lock whose root input `quest` locks the source at `rev`, pinned in its URL when `pinned`. */ +function lockFile(rev: string, pinned: boolean): string { + const original = { owner: "moq-dev", repo: "quest", type: "github", ...(pinned ? { rev } : {}) }; + return `${JSON.stringify( + { + nodes: { + nixpkgs: { locked: { type: "github", owner: "NixOS", repo: "nixpkgs", rev: "9".repeat(40) } }, + quest_2: { locked: { type: "github", owner: "moq-dev", repo: "quest", rev }, original }, + root: { inputs: { nixpkgs: "nixpkgs", quest: "quest_2", follows: ["nixpkgs"] } }, + }, + root: "root", + version: 7, + }, + null, + 2, + )}\n`; +} + +const flakeNix = (rev?: string) => + `{\n inputs.quest.url = "github:moq-dev/quest${rev ? `/${rev}` : ""}";\n outputs = { ... }: { };\n}\n`; + +function git(cwd: string, ...args: string[]): string { + return execFileSync("git", args, { cwd, encoding: "utf8" }).trim(); +} + +/** The target's repository, its `main` pinning the source at `OLD` in flake.nix's URL. */ +function target(): { remote: string; base: string } { + const work = tempDir(); + git(work, "init", "--quiet", "--initial-branch=main"); + configure(work, GIT_SETTINGS); + writeFileSync(join(work, "flake.nix"), flakeNix(OLD)); + writeFileSync(join(work, "flake.lock"), lockFile(OLD, true)); + git(work, "add", "."); + git(work, "commit", "--quiet", "-m", "init"); + const remote = tempDir(); + git(remote, "clone", "--quiet", "--bare", work, "."); + configure(remote, { "uploadpack.allowAnySHA1InWant": "true" }); + return { remote: `file://${remote}`, base: git(work, "rev-parse", "HEAD") }; +} + +interface World { + target: { owner: string; repo: string }; + remote: string; + base: string; + pulls: { number: number; head: string; state: string; draft: boolean; auto_merge: unknown; body?: string }[]; + rules: { type: string }[] | "free"; + allowAutoMerge: boolean; + /** Each check on a head, as `statusCheckRollup` contexts. */ + checks: Record[]>; + requests: string[]; + /** The flake.lock GitHub serves on `main`, when it differs from the remote's. */ + lock?: string; +} + +function deps(world: World): GitHubDependencies { + const path = `/repos/${world.target.owner}/${world.target.repo}`; + const json = (value: unknown, status = 200) => Response.json(value, { status }); + return { + token: "test", + now: () => 0, + sleep: async () => {}, + warn: () => {}, + fetch: (async (input: URL, init?: RequestInit) => { + const url = new URL(input); + const method = init?.method ?? "GET"; + const body = init?.body ? JSON.parse(String(init.body)) : undefined; + world.requests.push(`${method} ${url.pathname}${url.search}`); + if (url.pathname === "/graphql") { + const query = String(body.query); + if (query.includes("statusCheckRollup")) { + const nodes = world.checks[body.variables.oid] ?? []; + return json({ + data: { + repository: { + object: { + statusCheckRollup: { contexts: { nodes, pageInfo: { hasNextPage: false } } }, + }, + }, + }, + }); + } + const mutation = /mutation[^{]*\{\s*(\w+)/.exec(query)?.[1]; + const pull = world.pulls.find((candidate) => `PR_${candidate.number}` === body.variables.id)!; + if (mutation === "disablePullRequestAutoMerge") pull.auto_merge = null; + if (mutation === "convertPullRequestToDraft") pull.draft = true; + if (mutation === "markPullRequestReadyForReview") pull.draft = false; + return json({ data: {} }); + } + if (url.pathname === path) return json({ default_branch: "main", allow_auto_merge: world.allowAutoMerge }); + if (url.pathname === `${path}/git/ref/heads/main`) return json({ object: { sha: world.base } }); + if (url.pathname === `${path}/contents/flake.lock`) { + const text = + world.lock ?? + git(world.remote.slice("file://".length), "show", `${url.searchParams.get("ref")}:flake.lock`); + return json({ content: Buffer.from(text).toString("base64") }); + } + if (url.pathname === `${path}/branches/main`) return json({ protection: { enabled: false } }); + if (url.pathname === `${path}/rules/branches/main`) { + if (world.rules === "free") + return json( + { message: "Upgrade to GitHub Pro or make this repository public to enable this feature." }, + 403, + ); + return json(world.rules); + } + const view = (pull: World["pulls"][number]) => ({ + number: pull.number, + node_id: `PR_${pull.number}`, + state: pull.state, + draft: pull.draft, + auto_merge: pull.auto_merge, + head: { sha: pull.head }, + base: { ref: "main" }, + }); + if (url.pathname === `${path}/pulls` && method === "GET") { + expect(url.searchParams.get("head")).toBe(`${world.target.owner}:${BUMP_BRANCH}`); + return json(world.pulls.filter((pull) => pull.state === "open").map(view)); + } + if (url.pathname === `${path}/pulls` && method === "POST") { + const head = git(world.remote.slice("file://".length), "rev-parse", body.head); + const pull = { number: 7, head, state: "open", draft: body.draft, auto_merge: null, body: body.body }; + world.pulls.push(pull); + return json(view(pull), 201); + } + const number = /\/(?:pulls|issues)\/(\d+)/.exec(url.pathname)?.[1]; + const pull = world.pulls.find((candidate) => candidate.number === Number(number)); + if (pull && url.pathname === `${path}/pulls/${number}`) { + if (method === "PATCH") { + if (body.state) pull.state = body.state; + if (body.body) pull.body = body.body; + if (body.body) pull.head = git(world.remote.slice("file://".length), "rev-parse", BUMP_BRANCH); + } + return json(view(pull)); + } + if (pull && url.pathname === `${path}/issues/${number}/comments`) + return method === "GET" ? json([]) : json({ id: 1 }, 201); + throw new Error(`unexpected request ${method} ${url.pathname}`); + }) as typeof fetch, + }; +} + +/** Tools that lock as Nix would and a quest whose `init` writes a stub and whose `check --fix` passes unless told. */ +function tools(findings?: string): BumpTools & { locks: string[] } { + const locks: string[] = []; + return { + locks, + lock: (checkout, input, url) => { + locks.push(`${input} ${url}`); + const rev = url.split("/").at(-1)!; + writeFileSync(join(checkout, "flake.lock"), lockFile(rev, false)); + }, + quest: (sha) => `quest-${sha}`, + run: (quest, checkout, args) => { + if (args[0] === "init") writeFileSync(join(checkout, "stub.md"), `${quest}\n`); + return findings !== undefined && args[0] === "check" + ? { status: 1, output: findings } + : { status: 0, output: "" }; + }, + }; +} + +function context(world: World, faked = tools()): BumpContext { + return { + source: SOURCE, + targets: [world.target], + state: tempDir(), + reader: async () => "reader", + tools: faked, + connect: () => ({ remote: world.remote, deps: deps(world), token: async () => "push", read: readRemote }), + }; +} + +function world(overrides: Partial = {}): World { + const { remote, base } = target(); + return { + target: { owner: "moq-dev", repo: "widget" }, + remote, + base, + pulls: [], + rules: [{ type: "merge_queue" }], + allowAutoMerge: true, + checks: {}, + requests: [], + ...overrides, + }; +} + +const bare = (world: World) => world.remote.slice("file://".length); + +beforeEach(() => landed.mockClear()); + +describe("questInput", () => { + test("finds the one root input locking the source, skipping follows and other inputs", () => { + expect(questInput(JSON.parse(lockFile(OLD, true)), SOURCE)).toEqual({ + input: "quest", + locked: OLD, + pinned: OLD, + }); + expect(questInput(JSON.parse(lockFile(OLD, false)), { owner: "Moq-Dev", repo: "Quest" })).toEqual({ + input: "quest", + locked: OLD, + }); + }); + + test("fails on a lock with no such input, or more than one", () => { + expect(() => questInput(JSON.parse(lockFile(OLD, false)), { owner: "moq-dev", repo: "other" })).toThrow( + "0 root inputs", + ); + const lock = JSON.parse(lockFile(OLD, false)); + lock.nodes.root.inputs.again = "quest_2"; + expect(() => questInput(lock, SOURCE)).toThrow("2 root inputs"); + }); +}); + +describe("unpin", () => { + test("drops the revision from the source's URL", () => { + expect(unpin(flakeNix(OLD), SOURCE, OLD)).toBe(flakeNix()); + }); + + test("fails unless the pinned URL appears exactly once", () => { + expect(() => unpin(flakeNix(), SOURCE, OLD)).toThrow("0 times"); + expect(() => unpin(flakeNix(OLD) + flakeNix(OLD), SOURCE, OLD)).toThrow("2 times"); + }); +}); + +test("bumpBody waits on a maintainer only for findings", () => { + const options = { source: SOURCE, input: "quest", from: OLD, to: NEW, unpinned: false, changed: ["flake.lock"] }; + expect(bumpBody(options)).not.toContain("## Waiting"); + expect(bumpBody(options)).toContain("## Review\n\nSkipped: pin bump"); + expect(bumpBody({ ...options, findings: "quest/a.md: " })).toContain( + "## Waiting\n\n- Maintainer: `quest check`", + ); + expect(bumpBody({ ...options, findings: "quest/a.md: " })).toContain("<bad>"); +}); + +describe("requirements", () => { + test("reads no rules from a private repository on GitHub Free", async () => { + const free = world({ rules: "free" }); + expect( + (await requirements(new (await import("../src/cli/github")).GitHub(deps(free)), free.target, "main")).rules, + ).toEqual([]); + }); +}); + +describe("bumpPass", () => { + test("opens a bump PR on a gated base, unpinning the URL, and queues it once", async () => { + const target = world(); + const ctx = context(target); + expect(await bumpPass(ctx, NEW)).toEqual([`bump moq-dev/widget 7 ${NEW}`, "bump landing moq-dev/widget 7"]); + + const head = git(bare(target), "rev-parse", BUMP_BRANCH); + expect(git(bare(target), "show", "-s", "--format=%P %an", head)).toBe(`${target.base} moq-bot`); + expect(git(bare(target), "show", `${head}:flake.nix`)).toBe(flakeNix().trim()); + expect(JSON.parse(git(bare(target), "show", `${head}:flake.lock`)).nodes.quest_2.locked.rev).toBe(NEW); + expect(git(bare(target), "show", `${head}:stub.md`)).toBe(`quest-${NEW}`); + expect(target.pulls[0]).toMatchObject({ draft: false }); + expect(target.pulls[0].body).toContain("drops the revision"); + expect(target.pulls[0].body).toContain("changed `stub.md`"); + expect(landed).toHaveBeenCalledExactlyOnceWith( + { ...target.target, number: 7 }, + head, + undefined, + expect.anything(), + false, + ); + + // The next pass at the same commit asks GitHub nothing. + target.requests.length = 0; + expect(await bumpPass(ctx, NEW)).toEqual([]); + expect(target.requests).toEqual([]); + }); + + test("moves an open bump PR forward with a merge, disarming its auto-merge first", async () => { + const target = world(); + const ctx = context(target); + await bumpPass(ctx, OLD.replace(/1/g, "3")); + const first = git(bare(target), "rev-parse", BUMP_BRANCH); + target.pulls[0].auto_merge = { enabled_at: "now" }; + landed.mockClear(); + + expect(await bumpPass(ctx, NEW)).toEqual([`bump moq-dev/widget 7 ${NEW}`, "bump landing moq-dev/widget 7"]); + const head = git(bare(target), "rev-parse", BUMP_BRANCH); + expect(git(bare(target), "show", "-s", "--format=%P", head)).toBe(`${first} ${target.base}`); + expect(JSON.parse(git(bare(target), "show", `${head}:flake.lock`)).nodes.quest_2.locked.rev).toBe(NEW); + expect(target.pulls[0].auto_merge).toBeNull(); + expect(target.pulls).toHaveLength(1); + expect(landed).toHaveBeenCalledOnce(); + }); + + test("leaves findings --fix cannot resolve as a draft for a maintainer", async () => { + const target = world(); + expect(await bumpPass(context(target, tools("quest/a.md: broken")), NEW)).toEqual([ + `bump moq-dev/widget 7 ${NEW} waiting`, + ]); + expect(target.pulls[0]).toMatchObject({ draft: true }); + expect(target.pulls[0].body).toContain("## Waiting"); + expect(target.pulls[0].body).toContain("quest/a.md: broken"); + expect(landed).not.toHaveBeenCalled(); + }); + + test("closes an open bump PR once the base already pins the commit", async () => { + const target = world({ lock: lockFile(NEW, false) }); + target.pulls.push({ number: 3, head: target.base, state: "open", draft: false, auto_merge: null }); + expect(await bumpPass(context(target), NEW)).toEqual(["bump closed moq-dev/widget 3"]); + expect(target.pulls[0].state).toBe("closed"); + }); + + test("on an ungated base, waits for every check to pass, then merges directly without auto-merge", async () => { + const target = world({ rules: "free", allowAutoMerge: false }); + const ctx = context(target); + expect(await bumpPass(ctx, NEW)).toEqual([`bump moq-dev/widget 7 ${NEW}`]); + const head = git(bare(target), "rev-parse", BUMP_BRANCH); + target.checks[head] = [{ __typename: "CheckRun", databaseId: 1, name: "check", status: "IN_PROGRESS" }]; + expect(await bumpPass(ctx, NEW)).toEqual([]); + expect(landed).not.toHaveBeenCalled(); + + target.checks[head] = [ + { __typename: "CheckRun", databaseId: 1, name: "check", status: "COMPLETED", conclusion: "SUCCESS" }, + ]; + expect(await bumpPass(ctx, NEW)).toEqual(["bump landing moq-dev/widget 7"]); + expect(landed).toHaveBeenCalledExactlyOnceWith( + { ...target.target, number: 7 }, + head, + undefined, + expect.anything(), + true, + ); + }); + + test("a failed check on an ungated base is left for a maintainer", async () => { + const target = world({ rules: [] }); + const ctx = context(target); + await bumpPass(ctx, NEW); + const head = git(bare(target), "rev-parse", BUMP_BRANCH); + target.checks[head] = [ + { __typename: "CheckRun", databaseId: 1, name: "check", status: "COMPLETED", conclusion: "FAILURE" }, + ]; + expect(await bumpPass(ctx, NEW)).toEqual(["bump failed moq-dev/widget 7"]); + expect(target.requests).toContain("POST /repos/moq-dev/widget/issues/7/comments"); + target.requests.length = 0; + expect(await bumpPass(ctx, NEW)).toEqual([]); + expect(target.requests).toEqual([]); + expect(landed).not.toHaveBeenCalled(); + }); + + test("a target that fails does not hold up the others, and the pass reports it", async () => { + const broken = world({ + target: { owner: "moq-dev", repo: "broken" }, + lock: lockFile(OLD, false).replace(/moq-dev/g, "other"), + }); + const good = world(); + const ctx = context(good); + ctx.targets = [broken.target, good.target]; + ctx.connect = (target) => + target.repo === "broken" + ? { remote: broken.remote, deps: deps(broken), token: async () => "push", read: readRemote } + : { remote: good.remote, deps: deps(good), token: async () => "push", read: readRemote }; + await expect(bumpPass(ctx, NEW)).rejects.toThrow("moq-dev/broken: flake.lock has 0 root inputs"); + expect(good.pulls).toHaveLength(1); + expect(JSON.parse(readFileSync(join(ctx.state, "bump.json"), "utf8"))).toEqual({ + "moq-dev/widget": { sha: NEW, pr: 7, head: git(bare(good), "rev-parse", BUMP_BRANCH), status: "done" }, + }); + }); +}); From 22f1264f01701670d846bbcba7c12d4b67b98671 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 11:53:43 -0800 Subject: [PATCH 07/12] quest: drop per-base's links to the own-queue quest the merged branch finishes Co-Authored-By: Claude Opus 5.5 --- quest/m0/queue-watcher/per-base.md | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/quest/m0/queue-watcher/per-base.md b/quest/m0/queue-watcher/per-base.md index 8856aa19..f429a029 100644 --- a/quest/m0/queue-watcher/per-base.md +++ b/quest/m0/queue-watcher/per-base.md @@ -28,7 +28,7 @@ moq.pro's release landing. Decision prompts, selected options marked ✅: - Goal: - ✅ Plan it, with the goal as stated - - Fold it into [the own queue](/quest/m0/queue-watcher/own-queue.md) + - Fold it into the own queue - Drop it - Milestone: - ✅ m0, inside this epic @@ -74,7 +74,7 @@ What per-base separation needs: [the retire quest](/quest/m0/queue-watcher/retire.md)'s log pruning rely on. - Each instance keeps its own private repository, so a second base costs a second clone's disk rather than a shared object store with its own locking. -- `--method` comes from [the own queue](/quest/m0/queue-watcher/own-queue.md) +- `--method` comes from the own queue and is already per process; check that nothing the method affects, such as the landed-tree verification or a fixer's base merge, reads state outside the instance's directory. @@ -97,7 +97,3 @@ and moq.pro adds an entry such as `moq-pro-release` with `repository = "moq-dev/ takes a unique `id`. Both instances share the host's three fixer slots, so each keeps the default `--fixer-jobs`; `release` lands rarely. That host PR belongs to moq.pro's Land release quest, which consumes this one. - -## Required - -- [The watcher runs its own serial queue](/quest/m0/queue-watcher/own-queue.md) - adds `--method`, and the queue branches and pruning this keeps per base From 94e0e714fff25e0101383db4d54fc3e1f4e5b109 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 12:14:22 -0800 Subject: [PATCH 08/12] fix(land): contain a bump's tools, and wait for whole check suites on ungated bases Run the bump's quest from a directory of its own with --root, refuse a checkout with a symlink that leaves it or dangles, require every check suite that ran a check to complete before merging an ungated bump, and unpin a URL in any case. Co-Authored-By: Claude Opus 5.5 --- README.md | 11 ++++++--- src/cli/land-bump.ts | 54 +++++++++++++++++++++++++++++++++++------- test/land-bump.test.ts | 40 ++++++++++++++++++++++++++++++- 3 files changed, 93 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 68fd1beb..c0b8c5d1 100644 --- a/README.md +++ b/README.md @@ -1023,11 +1023,15 @@ not pin yet, it opens or updates that target's one `quest-bump` PR as the App: 1. It reads the target's `flake.lock` on its default branch for the one root input that locks the watched repository on GitHub; a target already at the commit gets nothing, and an open bump PR there is closed. -2. In a fresh shallow checkout it drops a revision from that input's URL in +2. In a fresh shallow checkout, which it refuses when a tracked symlink leaves + the checkout or dangles, since the tools write through symlinks, it drops a + revision from that input's URL in `flake.nix`, so `flake.lock` alone pins it from then on, and locks the input at the commit with `nix flake lock --override-input`. 3. It builds the watched repository's `quest` at that commit with `nix build`, - and runs its `quest init` and `quest check --fix` in the checkout. + and runs its `quest init` and `quest check --fix` on the checkout with + `--root`, from a directory of its own, so the target's `bunfig.toml` never + loads. 4. It commits the result as moq-bot and lease-pushes it to `quest-bump`. An open bump PR's head becomes the new commit's first parent, beside the target's base, so the branch never rewinds; any other `quest-bump` branch, left by a @@ -1039,7 +1043,8 @@ not pin yet, it opens or updates that target's one `quest-bump` PR as the App: `## Waiting` item quoting them. Otherwise it lands like any PR there, through `quest merge`'s path: into GitHub's merge queue, the target's own queue, or auto-merge. A base that requires no check would merge at once, so there the - bump first waits for every check on its head to pass, then merges directly + bump first waits for every check on its head to pass and every check suite + that ran one to complete, then merges directly when the repository has no auto-merge, as on a private repository on GitHub Free, whose rules GitHub does not serve. A failed check, or a landing `quest merge` refuses, is commented on and left for a maintainer. diff --git a/src/cli/land-bump.ts b/src/cli/land-bump.ts index 7b162b45..b371232e 100644 --- a/src/cli/land-bump.ts +++ b/src/cli/land-bump.ts @@ -1,10 +1,11 @@ // Pin bumps: the source repository's watcher keeps each target repository's flake input on the source at the // source's base, through one bot PR per target that lands like any other PR there. import { spawnSync } from "node:child_process"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; -import { join } from "node:path"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs"; +import { isAbsolute, join, relative, sep } from "node:path"; import { checksState } from "../github/checks"; import { + array, GitHub, type GitHubDependencies, object, @@ -43,7 +44,7 @@ export interface BumpTools { lock(checkout: string, input: string, url: string, token: string): void; /** The `quest` executable of the source at `sha`. */ quest(sha: string, token: string): string; - /** Run `quest` with `args` in `checkout`. */ + /** Run `quest` with `args` on the repository at `checkout`. */ run(quest: string, checkout: string, args: string[]): { status: number | null; output: string }; } @@ -112,9 +113,33 @@ export function questInput(lock: ObjectValue, source: Repository): { input: stri */ export function unpin(flake: string, source: Repository, rev: string): string { const url = `github:${name(source)}/${rev}`; - const found = flake.split(`"${url}"`).length - 1; + // GitHub names are case-insensitive, as `questInput` matches them. + const pattern = new RegExp(`"${url.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}"`, "gi"); + const found = flake.match(pattern)?.length ?? 0; if (found !== 1) throw new Error(`flake.nix names "${url}" ${found} times, where a bump can unpin exactly one`); - return flake.replace(`"${url}"`, `"github:${name(source)}"`); + return flake.replace(pattern, `"github:${name(source)}"`); +} + +/** + * Refuse a checkout with a tracked symlink that leaves it or dangles: the tools write through symlinks, + * which would let the target's tree overwrite the daemon's own files. + */ +export function contained(checkout: string): void { + const root = realpathSync(checkout); + const entries = git(checkout, ["ls-files", "--stage", "-z"]).split("\0").filter(Boolean); + for (const entry of entries) { + const [meta, file] = entry.split("\t"); + if (!meta.startsWith("120000 ")) continue; + let target: string; + try { + target = realpathSync(join(checkout, file)); + } catch { + throw new Error(`${file} is a symlink to nothing, which a bump refuses to write through`); + } + const outside = relative(root, target); + if (outside === ".." || outside.startsWith(`..${sep}`) || isAbsolute(outside)) + throw new Error(`${file} is a symlink out of the repository, which a bump refuses to write through`); + } } /** The bump PR's body, with a `## Waiting` item for findings, and the recorded review skip. */ @@ -243,6 +268,7 @@ async function bump( git(checkout, ["init", "--quiet"]); await connection.read(checkout, ["fetch", "--quiet", "--no-tags", "--depth=1", remote, baseSha]); git(checkout, ["checkout", "--quiet", "--detach", baseSha]); + contained(checkout); // The open PR's head stays in the branch's history; a branch left by a merged or closed PR is replaced. const previous = open ? string(object(open.head).sha) : ""; const lease = previous || ((await remoteHead(checkout, remote, ref, connection.read)) ?? ""); @@ -358,8 +384,17 @@ async function settle(target: Repository, record: Bump, connection: BumpConnecti if (!gated) { const found = await contexts(github, target, head); const state = found.length === 0 ? "PENDING" : checksState(found); - if (state === "PENDING") return record; if (state === "FAILURE") return fail(`a check failed on its head ${head}`); + // A workflow's later jobs report only once they start; its suite stays incomplete until they finish. + // Suites of apps that never ran a check stay queued forever, and count for nothing. + const suites = object(await (await github.request(`${path}/commits/${head}/check-suites?per_page=100`)).json()); + const listed = array(suites.check_suites); + if (Number(suites.total_count) > listed.length) + throw new Error(`${head} has more than ${listed.length} check suites, more than a bump reads`); + const running = listed.some( + (suite) => Number(suite.latest_check_runs_count) > 0 && suite.status !== "completed", + ); + if (state === "PENDING" || running) return record; direct = object(await (await github.request(path)).json()).allow_auto_merge !== true; } try { @@ -409,8 +444,11 @@ export function hostTools(source: Repository, state: string): BumpTools { return path; }, run: (quest, checkout, args) => { - const result = spawnSync(quest, args, { - cwd: checkout, + // Bun reads bunfig.toml, whose preload runs code, from its working directory: never the target's. + const trusted = join(state, "bump", "run"); + mkdirSync(trusted, { recursive: true, mode: 0o700 }); + const result = spawnSync(quest, ["--root", checkout, ...args], { + cwd: trusted, encoding: "utf8", env: { ...commandEnvironment(), HOME: join(state, "bump"), NO_COLOR: "1" }, maxBuffer: 1 << 30, diff --git a/test/land-bump.test.ts b/test/land-bump.test.ts index ba68592f..099dd863 100644 --- a/test/land-bump.test.ts +++ b/test/land-bump.test.ts @@ -1,7 +1,7 @@ // The pin bump pass against a fake GitHub and a local target repository, with the host's Nix and quest faked. import { execFileSync } from "node:child_process"; -import { readFileSync, writeFileSync } from "node:fs"; +import { readFileSync, symlinkSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { beforeEach, describe, expect, test, vi } from "vitest"; @@ -76,6 +76,8 @@ interface World { allowAutoMerge: boolean; /** Each check on a head, as `statusCheckRollup` contexts. */ checks: Record[]>; + /** Each check suite on a head. */ + suites: Record; requests: string[]; /** The flake.lock GitHub serves on `main`, when it differs from the remote's. */ lock?: string; @@ -123,6 +125,11 @@ function deps(world: World): GitHubDependencies { git(world.remote.slice("file://".length), "show", `${url.searchParams.get("ref")}:flake.lock`); return json({ content: Buffer.from(text).toString("base64") }); } + const suites = new RegExp(`^${path}/commits/(\\w+)/check-suites$`).exec(url.pathname); + if (suites) { + const listed = world.suites[suites[1]] ?? []; + return json({ total_count: listed.length, check_suites: listed }); + } if (url.pathname === `${path}/branches/main`) return json({ protection: { enabled: false } }); if (url.pathname === `${path}/rules/branches/main`) { if (world.rules === "free") @@ -209,6 +216,7 @@ function world(overrides: Partial = {}): World { rules: [{ type: "merge_queue" }], allowAutoMerge: true, checks: {}, + suites: {}, requests: [], ...overrides, }; @@ -246,6 +254,10 @@ describe("unpin", () => { expect(unpin(flakeNix(OLD), SOURCE, OLD)).toBe(flakeNix()); }); + test("matches the source's name in any case", () => { + expect(unpin(flakeNix(OLD).replace("moq-dev/quest", "Moq-Dev/Quest"), SOURCE, OLD)).toBe(flakeNix()); + }); + test("fails unless the pinned URL appears exactly once", () => { expect(() => unpin(flakeNix(), SOURCE, OLD)).toThrow("0 times"); expect(() => unpin(flakeNix(OLD) + flakeNix(OLD), SOURCE, OLD)).toThrow("2 times"); @@ -343,9 +355,18 @@ describe("bumpPass", () => { expect(await bumpPass(ctx, NEW)).toEqual([]); expect(landed).not.toHaveBeenCalled(); + // A workflow whose later job has not started yet shows only its passing first job. target.checks[head] = [ { __typename: "CheckRun", databaseId: 1, name: "check", status: "COMPLETED", conclusion: "SUCCESS" }, ]; + target.suites[head] = [ + { status: "in_progress", latest_check_runs_count: 1 }, + { status: "queued", latest_check_runs_count: 0 }, + ]; + expect(await bumpPass(ctx, NEW)).toEqual([]); + expect(landed).not.toHaveBeenCalled(); + + target.suites[head][0].status = "completed"; expect(await bumpPass(ctx, NEW)).toEqual(["bump landing moq-dev/widget 7"]); expect(landed).toHaveBeenCalledExactlyOnceWith( { ...target.target, number: 7 }, @@ -372,6 +393,23 @@ describe("bumpPass", () => { expect(landed).not.toHaveBeenCalled(); }); + test("refuses a target whose tree has a symlink out of the checkout", async () => { + const target = world(); + const work = tempDir(); + git(work, "clone", "--quiet", bare(target), "."); + configure(work, GIT_SETTINGS); + // The checkout's parent, the daemon's own directory, exists; the dangling link would be created on write. + symlinkSync("..", join(work, "AGENTS.md")); + symlinkSync("../../nothing", join(work, "CLAUDE.md")); + symlinkSync("flake.nix", join(work, "inside")); + git(work, "add", "."); + git(work, "commit", "--quiet", "-m", "link"); + git(work, "push", "--quiet", "origin", "HEAD:main"); + target.base = git(work, "rev-parse", "HEAD"); + await expect(bumpPass(context(target), NEW)).rejects.toThrow("AGENTS.md is a symlink out of the repository"); + expect(target.pulls).toEqual([]); + }); + test("a target that fails does not hold up the others, and the pass reports it", async () => { const broken = world({ target: { owner: "moq-dev", repo: "broken" }, From b7fd5007f4312fa5488d5a117dc019ce3e1fcbca Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 12:21:29 -0800 Subject: [PATCH 09/12] fix(land): an ungated bump waits for the head's workflow runs Co-Authored-By: Claude Opus 5.5 --- README.md | 7 ++++--- src/cli/land-bump.ts | 30 ++++++++++++++++++++++-------- test/land-bump.test.ts | 20 ++++++++++++++++---- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index c0b8c5d1..a1a34110 100644 --- a/README.md +++ b/README.md @@ -1043,8 +1043,9 @@ not pin yet, it opens or updates that target's one `quest-bump` PR as the App: `## Waiting` item quoting them. Otherwise it lands like any PR there, through `quest merge`'s path: into GitHub's merge queue, the target's own queue, or auto-merge. A base that requires no check would merge at once, so there the - bump first waits for every check on its head to pass and every check suite - that ran one to complete, then merges directly + bump first waits for every check and workflow run on its head to pass, and + for every other app's check suite that ran a check to complete, then merges + directly when the repository has no auto-merge, as on a private repository on GitHub Free, whose rules GitHub does not serve. A failed check, or a landing `quest merge` refuses, is commented on and left for a maintainer. @@ -1055,7 +1056,7 @@ directory records each target's last bump, so a restart repeats nothing and a closed bump is not reopened for the same commit. A target's failure is logged and retried on the next pass without holding up the others. Each target gets its own tokens: `contents: write`, `pull_requests: write`, `merge_queues: -write`, and reads of `administration`, `checks`, and `statuses` for its API +write`, and reads of `actions`, `administration`, `checks`, and `statuses` for its API calls, and `workflows: write` for a push whose diff from the branch's old head carries a workflow change. Nix reads GitHub with the watched repository's `contents: read` token. diff --git a/src/cli/land-bump.ts b/src/cli/land-bump.ts index b371232e..e3aecca5 100644 --- a/src/cli/land-bump.ts +++ b/src/cli/land-bump.ts @@ -67,16 +67,20 @@ export interface BumpContext { tools: BumpTools; } -/** The installation permissions a target's bump API calls need: `land` reads the base's protection and the head's checks. */ +/** The installation permissions a target's bump API calls need: `land` reads the base's protection, and an ungated bump the head's checks and workflow runs. */ export const BUMP_PERMISSIONS = { contents: "write", pull_requests: "write", merge_queues: "write", + actions: "read", administration: "read", checks: "read", statuses: "read", } as const; +/** Workflow run conclusions that do not fail. */ +const PASSING = ["success", "neutral", "skipped"]; + const name = (repository: Repository) => `${repository.owner}/${repository.repo}`; /** The root input of `lock` that locks `source` on GitHub, and its locked and URL-pinned revisions. */ @@ -385,15 +389,25 @@ async function settle(target: Repository, record: Bump, connection: BumpConnecti const found = await contexts(github, target, head); const state = found.length === 0 ? "PENDING" : checksState(found); if (state === "FAILURE") return fail(`a check failed on its head ${head}`); - // A workflow's later jobs report only once they start; its suite stays incomplete until they finish. - // Suites of apps that never ran a check stay queued forever, and count for nothing. + // A workflow's later jobs report only once they start, and a queued workflow may have no check run yet, + // so Actions is judged by its workflow runs. Another app's suite that never ran a check stays queued + // forever, and counts for nothing. + const runs = object(await (await github.request(`${path}/actions/runs?head_sha=${head}&per_page=100`)).json()); + const workflows = array(runs.workflow_runs); const suites = object(await (await github.request(`${path}/commits/${head}/check-suites?per_page=100`)).json()); const listed = array(suites.check_suites); - if (Number(suites.total_count) > listed.length) - throw new Error(`${head} has more than ${listed.length} check suites, more than a bump reads`); - const running = listed.some( - (suite) => Number(suite.latest_check_runs_count) > 0 && suite.status !== "completed", - ); + if (Number(runs.total_count) > workflows.length || Number(suites.total_count) > listed.length) + throw new Error(`${head} has more workflow runs or check suites than one page, more than a bump reads`); + if (workflows.some((run) => run.status === "completed" && !PASSING.includes(String(run.conclusion)))) + return fail(`a workflow failed on its head ${head}`); + const running = + workflows.some((run) => run.status !== "completed") || + listed.some( + (suite) => + object(suite.app ?? {}).slug !== "github-actions" && + Number(suite.latest_check_runs_count) > 0 && + suite.status !== "completed", + ); if (state === "PENDING" || running) return record; direct = object(await (await github.request(path)).json()).allow_auto_merge !== true; } diff --git a/test/land-bump.test.ts b/test/land-bump.test.ts index 099dd863..6cc287d9 100644 --- a/test/land-bump.test.ts +++ b/test/land-bump.test.ts @@ -77,7 +77,9 @@ interface World { /** Each check on a head, as `statusCheckRollup` contexts. */ checks: Record[]>; /** Each check suite on a head. */ - suites: Record; + suites: Record; + /** Each workflow run on a head. */ + runs: Record; requests: string[]; /** The flake.lock GitHub serves on `main`, when it differs from the remote's. */ lock?: string; @@ -125,6 +127,10 @@ function deps(world: World): GitHubDependencies { git(world.remote.slice("file://".length), "show", `${url.searchParams.get("ref")}:flake.lock`); return json({ content: Buffer.from(text).toString("base64") }); } + if (url.pathname === `${path}/actions/runs`) { + const listed = world.runs[url.searchParams.get("head_sha")!] ?? []; + return json({ total_count: listed.length, workflow_runs: listed }); + } const suites = new RegExp(`^${path}/commits/(\\w+)/check-suites$`).exec(url.pathname); if (suites) { const listed = world.suites[suites[1]] ?? []; @@ -217,6 +223,7 @@ function world(overrides: Partial = {}): World { allowAutoMerge: true, checks: {}, suites: {}, + runs: {}, requests: [], ...overrides, }; @@ -360,13 +367,18 @@ describe("bumpPass", () => { { __typename: "CheckRun", databaseId: 1, name: "check", status: "COMPLETED", conclusion: "SUCCESS" }, ]; target.suites[head] = [ - { status: "in_progress", latest_check_runs_count: 1 }, - { status: "queued", latest_check_runs_count: 0 }, + { status: "in_progress", latest_check_runs_count: 1, app: { slug: "ci-app" } }, + { status: "queued", latest_check_runs_count: 0, app: { slug: "never-reports" } }, ]; expect(await bumpPass(ctx, NEW)).toEqual([]); + target.suites[head][0].status = "completed"; + // A queued workflow with no check run yet still holds it. + target.suites[head].push({ status: "queued", latest_check_runs_count: 0, app: { slug: "github-actions" } }); + target.runs[head] = [{ status: "queued", conclusion: null }]; + expect(await bumpPass(ctx, NEW)).toEqual([]); expect(landed).not.toHaveBeenCalled(); - target.suites[head][0].status = "completed"; + target.runs[head] = [{ status: "completed", conclusion: "success" }]; expect(await bumpPass(ctx, NEW)).toEqual(["bump landing moq-dev/widget 7"]); expect(landed).toHaveBeenCalledExactlyOnceWith( { ...target.target, number: 7 }, From 9dd09c26e3674dccbadb76a24051e5b0bc7d4e84 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 21:13:36 -0800 Subject: [PATCH 10/12] fix(land): a bump checks symlinks by full name, keeps history, and follows its PR - contained() splits each ls-files entry at its first tab, so a symlink whose name holds a tab is checked under its own path. - A bare cache per target holds the base's whole history, which quest check --fix reads for deleted quests, and makes later fetches incremental. - The open bump PR is re-read before the push, and a PR that closed during the Nix work fails the pass, which starts over next time. - An open bump PR is retargeted when the target's default branch moved. Co-Authored-By: Claude Opus 5.5 --- src/cli/land-bump.ts | 32 +++++++++++++++--- test/land-bump.test.ts | 74 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 99 insertions(+), 7 deletions(-) diff --git a/src/cli/land-bump.ts b/src/cli/land-bump.ts index 07c151a0..32a0020f 100644 --- a/src/cli/land-bump.ts +++ b/src/cli/land-bump.ts @@ -132,7 +132,10 @@ export function contained(checkout: string): void { const root = realpathSync(checkout); const entries = git(checkout, ["ls-files", "--stage", "-z"]).split("\0").filter(Boolean); for (const entry of entries) { - const [meta, file] = entry.split("\t"); + // The path is everything after the first tab, which a file name may itself contain. + const tab = entry.indexOf("\t"); + const meta = entry.slice(0, tab); + const file = entry.slice(tab + 1); if (!meta.startsWith("120000 ")) continue; let target: string; try { @@ -267,16 +270,27 @@ async function bump( const remote = connection.remote; const ref = `refs/heads/${BUMP_BRANCH}`; mkdirSync(join(context.state, "bump"), { recursive: true, mode: 0o700 }); + // A bare cache per target keeps the base's whole history, which `quest check --fix` reads for deleted + // quests, and makes each later bump's fetch incremental. Its refs keep the fetched commits. + const cache = join(context.state, "bump", "repos", `${target.owner}-${target.repo}.git`); + if (!existsSync(join(cache, "HEAD"))) { + mkdirSync(cache, { recursive: true, mode: 0o700 }); + git(cache, ["init", "--quiet", "--bare"]); + } const checkout = mkdtempSync(join(context.state, "bump", "checkout-")); try { + await connection.read(cache, ["fetch", "--quiet", "--no-tags", remote, `+${baseSha}:refs/bump/base`]); git(checkout, ["init", "--quiet"]); - await connection.read(checkout, ["fetch", "--quiet", "--no-tags", "--depth=1", remote, baseSha]); + writeFileSync( + join(checkout, ".git", "objects", "info", "alternates"), + `${join(realpathSync(cache), "objects")}\n`, + ); git(checkout, ["checkout", "--quiet", "--detach", baseSha]); contained(checkout); // The open PR's head stays in the branch's history; a branch left by a merged or closed PR is replaced. const previous = open ? string(object(open.head).sha) : ""; const lease = previous || ((await remoteHead(checkout, remote, ref, connection.read)) ?? ""); - if (lease) await connection.read(checkout, ["fetch", "--quiet", "--no-tags", "--depth=1", remote, lease]); + if (lease) await connection.read(cache, ["fetch", "--quiet", "--no-tags", remote, `+${lease}:refs/bump/lease`]); const reader = await context.reader(); if (found.pinned !== undefined) { @@ -314,6 +328,14 @@ async function bump( "mutation($id: ID!) { disablePullRequestAutoMerge(input: { pullRequestId: $id }) { clientMutationId } }", { id }, ); + if (open) { + // The PR may have merged or closed during the Nix work, leaving its branch for the lease to match. + const now = object(await (await github.request(`${path}/pulls/${Number(open.number)}`)).json()); + if (now.state !== "open") + throw new Error( + `bump PR ${Number(open.number)} closed while the bump was prepared; the next pass starts over`, + ); + } const pushed = await push(checkout, remote, baseSha, commit, ref, connection.token, lease); if (pushed.status !== 0) throw new Error(`push failed: ${pushed.stderr.trim()}`); @@ -330,7 +352,9 @@ async function bump( let number: number; if (open) { number = Number(open.number); - await github.request(`${path}/pulls/${number}`, { method: "PATCH", body: { title, body } }); + // A target whose default branch moved gets its open bump retargeted, since the bump is built on the new one. + const retarget = string(object(open.base).ref) === base ? {} : { base }; + await github.request(`${path}/pulls/${number}`, { method: "PATCH", body: { title, body, ...retarget } }); if (open.draft !== draft) await github.graphql( `mutation($id: ID!) { ${draft ? "convertPullRequestToDraft" : "markPullRequestReadyForReview"}(input: { pullRequestId: $id }) { clientMutationId } }`, diff --git a/test/land-bump.test.ts b/test/land-bump.test.ts index 11a8268c..7fc51bd5 100644 --- a/test/land-bump.test.ts +++ b/test/land-bump.test.ts @@ -1,7 +1,7 @@ // The pin bump pass against a fake GitHub and a local target repository, with the host's Nix and quest faked. import { execFileSync } from "node:child_process"; -import { readFileSync, symlinkSync, writeFileSync } from "node:fs"; +import { mkdirSync, readFileSync, symlinkSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { beforeEach, describe, expect, test, vi } from "vitest"; @@ -72,7 +72,15 @@ interface World { target: { owner: string; repo: string }; remote: string; base: string; - pulls: { number: number; head: string; state: string; draft: boolean; auto_merge: unknown; body?: string }[]; + pulls: { + number: number; + head: string; + state: string; + draft: boolean; + auto_merge: unknown; + body?: string; + base?: string; + }[]; rules: { type: string }[] | "free"; allowAutoMerge: boolean; /** Each check on a head, as `statusCheckRollup` contexts. */ @@ -153,7 +161,7 @@ function deps(world: World): GitHubDependencies { draft: pull.draft, auto_merge: pull.auto_merge, head: { sha: pull.head }, - base: { ref: "main" }, + base: { ref: pull.base ?? "main" }, }); if (url.pathname === `${path}/pulls` && method === "GET") { expect(url.searchParams.get("head")).toBe(`${world.target.owner}:${BUMP_BRANCH}`); @@ -170,6 +178,7 @@ function deps(world: World): GitHubDependencies { if (pull && url.pathname === `${path}/pulls/${number}`) { if (method === "PATCH") { if (body.state) pull.state = body.state; + if (body.base) pull.base = body.base; if (body.body) pull.body = body.body; if (body.body) pull.head = git(world.remote.slice("file://".length), "rev-parse", BUMP_BRANCH); } @@ -476,6 +485,65 @@ describe("bumpPass", () => { expect(target.pulls).toEqual([]); }); + test("refuses an escaping symlink whose name holds a tab", async () => { + const target = world(); + const work = tempDir(); + git(work, "clone", "--quiet", bare(target), "."); + configure(work, GIT_SETTINGS); + mkdirSync(join(work, "quest")); + symlinkSync("../..", join(work, "quest", "\tescape.md")); + git(work, "add", "."); + git(work, "commit", "--quiet", "-m", "link"); + git(work, "push", "--quiet", "origin", "HEAD:main"); + target.base = git(work, "rev-parse", "HEAD"); + await expect(bumpPass(context(target), NEW)).rejects.toThrow("escape.md is a symlink out of the repository"); + }); + + test("runs quest check --fix on the base's whole history, which names deleted quests", async () => { + const target = world(); + const work = tempDir(); + git(work, "clone", "--quiet", bare(target), "."); + configure(work, GIT_SETTINGS); + writeFileSync(join(work, "second.md"), "second\n"); + git(work, "add", "."); + git(work, "commit", "--quiet", "-m", "second"); + git(work, "push", "--quiet", "origin", "HEAD:main"); + target.base = git(work, "rev-parse", "HEAD"); + const faked = tools(); + const counts: string[] = []; + const run = faked.run; + faked.run = (quest, checkout, args) => { + if (args[0] === "check") counts.push(git(checkout, "rev-list", "--count", "HEAD")); + return run(quest, checkout, args); + }; + const ctx = context(target, faked); + await bumpPass(ctx, NEW); + expect(counts).toEqual(["2"]); + }); + + test("starts over when the open bump PR closes while the bump is prepared", async () => { + const target = world(); + const ctx = context(target); + await bumpPass(ctx, OLD.replace(/1/g, "3")); + const first = git(bare(target), "rev-parse", BUMP_BRANCH); + const lock = ctx.tools.lock; + ctx.tools.lock = (...args) => { + target.pulls[0].state = "closed"; + lock(...args); + }; + await expect(bumpPass(ctx, NEW)).rejects.toThrow("bump PR 7 closed while the bump was prepared"); + expect(git(bare(target), "rev-parse", BUMP_BRANCH)).toBe(first); + }); + + test("retargets an open bump PR at the target's current default branch", async () => { + const target = world(); + const ctx = context(target); + await bumpPass(ctx, OLD.replace(/1/g, "3")); + target.pulls[0].base = "old"; + await bumpPass(ctx, NEW); + expect(target.pulls[0].base).toBe("main"); + }); + test("a target that fails does not hold up the others, and the pass reports it", async () => { const broken = world({ target: { owner: "moq-dev", repo: "broken" }, From 7ca104cbb3fc5934397c0eb27c6b5b97c83f0145 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 21:27:33 -0800 Subject: [PATCH 11/12] test(land): skip the tab-named symlink fixture on Windows Co-Authored-By: Claude Opus 5.5 --- test/land-bump.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/land-bump.test.ts b/test/land-bump.test.ts index 7fc51bd5..7bd08b12 100644 --- a/test/land-bump.test.ts +++ b/test/land-bump.test.ts @@ -485,7 +485,8 @@ describe("bumpPass", () => { expect(target.pulls).toEqual([]); }); - test("refuses an escaping symlink whose name holds a tab", async () => { + // Git for Windows refuses a path with a control character. + test.skipIf(process.platform === "win32")("refuses an escaping symlink whose name holds a tab", async () => { const target = world(); const work = tempDir(); git(work, "clone", "--quiet", bare(target), "."); From c73bca7cf566d42c2454a0d44a80019c3013475d Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 21:42:36 -0800 Subject: [PATCH 12/12] fix(land): rebuild a cached quest that Nix's garbage collection removed nix build --no-link roots nothing, so a retry for the same source commit could find the cached executable gone and fail until the watcher restarts. Co-Authored-By: Claude Opus 5.5 --- src/cli/land-bump.ts | 3 ++- test/land-bump.test.ts | 29 +++++++++++++++++++++++++++-- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/cli/land-bump.ts b/src/cli/land-bump.ts index 32a0020f..4c057de9 100644 --- a/src/cli/land-bump.ts +++ b/src/cli/land-bump.ts @@ -496,7 +496,8 @@ export function hostTools(source: Repository, state: string): BumpTools { }, quest: (sha, token) => { let path = built.get(sha); - if (path === undefined) { + // `--no-link` roots nothing, so Nix's garbage collection may remove a cached build before a retry. + if (path === undefined || !existsSync(path)) { path = join( nix(state, ["build", "--no-link", "--print-out-paths", `github:${name(source)}/${sha}`], token), "bin", diff --git a/test/land-bump.test.ts b/test/land-bump.test.ts index 7bd08b12..5c46f7e3 100644 --- a/test/land-bump.test.ts +++ b/test/land-bump.test.ts @@ -1,8 +1,8 @@ // The pin bump pass against a fake GitHub and a local target repository, with the host's Nix and quest faked. import { execFileSync } from "node:child_process"; -import { mkdirSync, readFileSync, symlinkSync, writeFileSync } from "node:fs"; -import { join } from "node:path"; +import { existsSync, mkdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { delimiter, join } from "node:path"; import { beforeEach, describe, expect, test, vi } from "vitest"; @@ -15,6 +15,7 @@ import { type BumpTools, bumpBody, bumpPass, + hostTools, latestRuns, questInput, unpin, @@ -545,6 +546,30 @@ describe("bumpPass", () => { expect(target.pulls[0].base).toBe("main"); }); + test.skipIf(process.platform === "win32")("rebuilds a cached quest that Nix's garbage collection removed", () => { + const dir = tempDir(); + const out = join(dir, "out"); + const calls = join(dir, "calls"); + writeFileSync( + join(dir, "nix"), + `#!/bin/sh\necho build >> "${calls}"\nmkdir -p "${out}/bin"\ntouch "${out}/bin/quest"\necho "${out}"\n`, + { mode: 0o755 }, + ); + const path = process.env.PATH; + process.env.PATH = `${dir}${delimiter}${path}`; + try { + const host = hostTools(SOURCE, dir); + const quest = host.quest(NEW, "token"); + expect(host.quest(NEW, "token")).toBe(quest); + rmSync(out, { recursive: true }); + expect(host.quest(NEW, "token")).toBe(quest); + expect(existsSync(quest)).toBe(true); + expect(readFileSync(calls, "utf8")).toBe("build\nbuild\n"); + } finally { + process.env.PATH = path; + } + }); + test("a target that fails does not hold up the others, and the pass reports it", async () => { const broken = world({ target: { owner: "moq-dev", repo: "broken" },