From 8238d8904dce29c26ba8a99e13d879ae1fad8c9e Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 31 Oct 2026 08:11:23 -0700 Subject: [PATCH 1/4] chore(quest): claim quest/m0/host-land/fixer-survives.md Quest-Claim: fa8a3d3599a442d4 From 78db49538758a337f58e9b3354d845a7328e05a7 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 31 Oct 2026 08:30:50 -0700 Subject: [PATCH 2/4] fix(land): keep the fixer alive through output bursts and say why it stopped moq's fixer died on every ejection on 2026-10-31. The host's per-job logs show why: - Exit 101 (14 jobs): Codex panicked with "failed printing to stderr: Resource temporarily unavailable". `quest land fix` let Codex inherit its stdout/stderr, and once Bun writes to a pipe it marks it non-blocking, so a command's output burst past the launcher FIFO's 64 KiB failed with EAGAIN. The agent now writes into pipes of its own, which Bun relays. - "fixer repository exceeds 100 MiB": the import bound counted the full history fetched into the fixer's repository, so every fixed result on moq was refused. Packs fetched at launch are now skipped; the bound covers what the fixer added. - Exit 143: a deploy's restart SIGTERMed the fixer with the daemon's unit. The handback now names the signal and the restart. Keeping fixers alive is the host's KillMode, recorded in the gate host quest. - "disappeared": the host briefly had ProcSubset=pid, so the daemon could not identify a live fixer. A job without an identity now waits for its exit status or the timeout. - A GitHub 502 while handing back a crash no longer replaces the crash. Tests cover a burst through a slow launcher pipe (fails before this fix), a collateral requeue beside a handback, SIGTERM during a restart, an unidentifiable fixer, and the import bound. Co-Authored-By: Claude Opus 5.5 --- README.md | 11 +++- quest/m0/host-land/README.md | 1 - quest/m0/host-land/fixer-survives.md | 37 ----------- quest/m0/host-land/host.md | 12 ++++ quest/m0/host-land/triage.md | 4 -- src/cli/land-fix.ts | 7 +- src/cli/land-fixer-repository.ts | 16 ++++- src/cli/land-fixer.ts | 29 ++++++-- test/land.test.ts | 99 ++++++++++++++++++++++++++-- 9 files changed, 160 insertions(+), 56 deletions(-) delete mode 100644 quest/m0/host-land/fixer-survives.md diff --git a/README.md b/README.md index 1afbb0eb..19b051e3 100644 --- a/README.md +++ b/README.md @@ -523,18 +523,23 @@ The payload adds `agent` and `model` metadata to each result (`--model ` selects the default Codex model; unknown models fall back to `Codex`). Reasons and diagnoses are single lines. Fixed heads must be committed on `pr-`. -Jobs and attempts persist across daemon restarts. After the wrapper exits, the +Jobs and attempts persist across daemon restarts. A running fixer outlives a +restart only where the host leaves its process group alive (under systemd, +`KillMode=process` on the daemon's unit); otherwise its handback names the signal +and the restart. After the wrapper exits, the daemon validates every result, ancestry, and protected path before lease-pushing as the bot. The protected set as a whole must match the pinned head or its merge with the current base, including content and mode. It covers `flake.nix`, `flake.lock`, `shell.nix`, `default.nix`, `nix/`, `.envrc`, `.github/`, `.gitmodules`, and gitlinks -anywhere. Collection copies at most 100 MiB of Git metadata into a clean object +anywhere. Collection copies at most 100 MiB of Git metadata the fixer added (packs +fetched at launch are already in the daemon's repository, so it skips them) into a clean object repository, ignores fixer configuration, and never follows Git alternates. Pack indexes are rebuilt with `git index-pack` rather than trusted; the fetch into the daemon's repository checks every object and link, since the import enables `fetch.fsckObjects=true`. It records `mechanical fix by : ` and the new head in `## Review`, enables head-pinned auto-merge, and adds `quest:merge`. Healthy PRs requeue unchanged. Forks refusing maintainer edits, refused pushes, invalid results, -crashes, or the 60-minute fixer deadline hand back with a marked comment. A flaky +crashes, or the 60-minute fixer deadline hand back with a marked comment naming the exit +status or signal, with the tail of the job's retained log. A flaky requeue suggests a follow-up. Each maintainer pick gets one attempt; the daemon records the fixer's new auto-merge `enabledAt` so its second ejection hands back. A new maintainer pick resets the attempt. Host provisioning and isolation diff --git a/quest/m0/host-land/README.md b/quest/m0/host-land/README.md index f2bc84cf..a89f20d3 100644 --- a/quest/m0/host-land/README.md +++ b/quest/m0/host-land/README.md @@ -95,5 +95,4 @@ can hand off instead. - [Gate host](/quest/m0/host-land/host.md) - kixelated/nixos runs the daemon as its own user, checks in nspawn containers, and retires the Actions runner - [Fixer proxy access](/quest/m0/host-land/fixer.md) - connect the nixos fixer container to CLIProxyAPI with the gate client key and maintainer login -- [Fixer survives ejection](/quest/m0/host-land/fixer-survives.md) - the fixer finishes after every ejection and requeues collateral PRs itself - [Batch triage](/quest/m0/host-land/triage.md) - the fixer blames the failing PR before ejection, and a red base halts the queue diff --git a/quest/m0/host-land/fixer-survives.md b/quest/m0/host-land/fixer-survives.md deleted file mode 100644 index 96174772..00000000 --- a/quest/m0/host-land/fixer-survives.md +++ /dev/null @@ -1,37 +0,0 @@ -# [S] The fixer survives an ejection and recognises collateral PRs - -## Goal - -After every batch ejection on moq-dev/moq, the fixer runs to completion and -records a result for each PR. A PR that was ejected only because it shared a -batch with the failing one is recognised as collateral and requeued, so no -maintainer has to hand it back. When the fixer does fail, its handback says -why, using its exit status and the end of its log. - -## Plan - -Decided 2026-10-31 (user, /quest-plan of the follow-ups from a moq-dev/moq -`/quest-complete` run). On 2026-10-31 the fixer crashed on every ejection, so -the maintainer handed collateral PRs back by hand -([moq-dev/moq#7416](https://github.com/moq-dev/moq/pull/7416), -[moq-dev/moq#7413](https://github.com/moq-dev/moq/pull/7413)). The failures -seen: - -- Exit 101, after a flood of `cargo_cyclonedx` "Hash not found in - Cargo.lock" errors. The fixer ran a check the container cannot complete, - or one that does not apply to the failure. -- Exit 143, which means it was killed by SIGTERM: the timeout in - `collectJob` (`src/cli/land-fixer.ts`), or the host stopping the unit. -- "fixer disappeared before recording an exit status; run /quest-iterate" - (`collectJob`). That text hides which of the two happened. Record the - exit status, or that the daemon restarted, and the tail of - `fixerLog`. - -Find each crash's cause in the host's per-job logs before changing anything. -Fix what is quest's here, and what is the host's wrapper or container in -kixelated/nixos as part of the [gate host](/quest/m0/host-land/host.md). -Cover a crashing fixer and a collateral requeue in the fixer tests. - -## Related - -- [Fixer proxy access](/quest/m0/host-land/fixer.md) - the fixer container's model access diff --git a/quest/m0/host-land/host.md b/quest/m0/host-land/host.md index 2f4cba51..ae8c6ccb 100644 --- a/quest/m0/host-land/host.md +++ b/quest/m0/host-land/host.md @@ -92,3 +92,15 @@ What remains: already rejects. - moq-bot's `organization_self_hosted_runners` write permission served only the retired runner; the maintainer may drop it. +- Keep fixers running across a deploy: set `KillMode=process` on each + `quest-land-.service` in `modules/quest-land.nix`. The default + `control-group` sends SIGTERM to the daemon's detached fixer supervisor + too, so a fixer running during a deploy ends with exit 143. On 2026-10-31 + that happened to moq's 23:22 batch + ([moq-dev/moq#7426](https://github.com/moq-dev/moq/pull/7426), + [moq-dev/moq#7084](https://github.com/moq-dev/moq/pull/7084)). The + daemon's in-flight check clients still exit on their next write to the + dead daemon's pipe. Check it by restarting the unit while a fixer runs: + the job should collect a result, not a handback naming SIGTERM. Keep + `ProcSubset=pid` off the unit: generation 45 had it, which hid + `/proc/sys/kernel/random/boot_id` from the daemon. diff --git a/quest/m0/host-land/triage.md b/quest/m0/host-land/triage.md index b8f9a2f3..af469dcf 100644 --- a/quest/m0/host-land/triage.md +++ b/quest/m0/host-land/triage.md @@ -49,10 +49,6 @@ wasting CI on bisection?" - Keep the fixer's untrusted-input rules: a PR's content cannot make the fixer blame another PR or skip the base check. -## Required - -- [Fixer survives ejection](/quest/m0/host-land/fixer-survives.md) - the fixer has to finish before its triage can be trusted - ## Related - [Fixer proxy access](/quest/m0/host-land/fixer.md) - how the fixer reaches its model diff --git a/src/cli/land-fix.ts b/src/cli/land-fix.ts index 48cf30af..eeee06e2 100644 --- a/src/cli/land-fix.ts +++ b/src/cli/land-fix.ts @@ -127,7 +127,12 @@ export async function fix( "-", ]; await new Promise((done, reject) => { - const child = spawn(agent, argv, { cwd: root, env, stdio: ["pipe", "inherit", "inherit"] }); + // Relay the agent's output through pipes of its own rather than letting it inherit ours. Once Bun writes + // to a pipe it marks that pipe non-blocking, and an inheriting child shares the flag: Codex then panicked + // (exit 101) with EAGAIN as soon as one command's output outran the host's reader. + const child = spawn(agent, argv, { cwd: root, env, stdio: ["pipe", "pipe", "pipe"] }); + child.stdout.pipe(process.stdout, { end: false }); + child.stderr.pipe(process.stderr, { end: false }); child.once("error", reject); child.stdin.on("error", () => {}); child.stdin.end(`${prompt}\n\nFailure log: ${JSON.stringify(log)}\nPRs: ${numbers.join(", ")}\n`); diff --git a/src/cli/land-fixer-repository.ts b/src/cli/land-fixer-repository.ts index 24057bc4..204e36c9 100644 --- a/src/cli/land-fixer-repository.ts +++ b/src/cli/land-fixer-repository.ts @@ -19,7 +19,20 @@ import { pathToFileURL } from "node:url"; import { git } from "./land"; export const MAX_FIXER_BYTES = 100 * 1024 * 1024; -export function importFixer(root: string, state: string, checkout: string, number: number, head: string): void { +/** + * Import pr-N's objects. `known` names the packs fetched from `root` at launch: their objects are already in `root`, + * so they are neither copied nor counted, and the size bound applies to what the fixer added rather than to the + * repository's history (moq's alone exceeds it). A fixer that rewrote one only leaves its objects out of the import. + */ +export function importFixer( + root: string, + state: string, + checkout: string, + number: number, + head: string, + known: string[] = [], +): void { + const launched = new Set(known.map((name) => name.replace(/\.pack$/, ""))); const source = join(checkout, ".git"); if (!lstatSync(checkout).isDirectory() || !lstatSync(source).isDirectory() || lstatSync(source).isSymbolicLink()) throw new Error("fixer .git must be a real directory"); @@ -35,6 +48,7 @@ export function importFixer(root: string, state: string, checkout: string, numbe if (stat.isSymbolicLink() || (!stat.isDirectory() && !stat.isFile())) throw new Error("fixer git metadata must contain only real files and directories"); if (stat.isDirectory()) walk(child, rel); + else if (relative === "objects/pack" && launched.has(name.replace(/\.[a-z]+$/, ""))) continue; else { bytes += stat.size; if (bytes > MAX_FIXER_BYTES) throw new Error("fixer repository exceeds 100 MiB"); diff --git a/src/cli/land-fixer.ts b/src/cli/land-fixer.ts index bfd55711..8c9cb8bd 100644 --- a/src/cli/land-fixer.ts +++ b/src/cli/land-fixer.ts @@ -13,6 +13,7 @@ import { rmSync, writeFileSync, } from "node:fs"; +import { constants as osConstants } from "node:os"; import { join } from "node:path"; import { MERGE_LABEL } from "../core"; import { GitHub, type GitHubDependencies, object, string } from "./github"; @@ -58,6 +59,10 @@ interface Job { terminating?: number; pid?: number; identity?: string; + /** The launching daemon's process identity, to tell a restart from a crash. */ + daemon?: string; + /** Packs fetched from the daemon's repository at launch; their objects are already there. */ + packs?: string[]; results?: FixResult[]; error?: string; handbacks?: Record; @@ -194,6 +199,8 @@ export async function startFixer( plan: failure.plan, numbers: entries.map((entry) => entry.number), started: deps.now(), + daemon: identity(process.pid), + packs: readdirSync(join(checkout, ".git", "objects", "pack")).filter((name) => name.endsWith(".pack")), published: [], }; save(path, job); @@ -327,7 +334,8 @@ export async function collectFixers( try { output.push(...(await collectJob(root, state, path, job, deps, token, timeout, read))); } catch (error) { - job.error = `fixer collection failed: ${String(error)}; run /quest-iterate`; + // Keep the first reason: a GitHub error while handing back a crash must not replace the crash. + job.error ??= `fixer collection failed: ${String(error)}; run /quest-iterate`; try { save(path, job); output.push(...(await collectJob(root, state, path, job, deps, token, timeout, read))); @@ -339,6 +347,12 @@ export async function collectFixers( return output; } +/** An exit status as a cause: a signal by name (143 is SIGTERM), or a crash with its code (101 is a Rust panic). */ +function exited(code: string): string { + const signal = Object.entries(osConstants.signals).find(([, number]) => 128 + number === Number(code))?.[0]; + return signal ? `stopped by ${signal} (exit ${code})` : `crashed (exit ${code})`; +} + async function collectJob( root: string, state: string, @@ -352,10 +366,13 @@ async function collectJob( const github = new GitHub(deps); const output: string[] = []; if (!job.results && !job.error) { + // A restart stops the daemon's whole unit, fixers included, unless the host keeps them running. + const daemon = identity(process.pid); + const restarted = job.daemon && daemon && job.daemon !== daemon ? "; the daemon restarted while it ran" : ""; if (existsSync(`${path}.exit`)) { const code = readFileSync(`${path}.exit`, "utf8"); if (job.terminating !== undefined) job.error = "fixer timed out; run /quest-iterate"; - else if (code !== "0") job.error = `fixer crashed (exit ${code}); run /quest-iterate`; + else if (code !== "0") job.error = `fixer ${exited(code)}${restarted}; run /quest-iterate`; else try { job.results = parseFixResult(readFixResult(job.checkout), job.numbers); @@ -380,8 +397,10 @@ async function collectJob( } catch {} } job.error = "fixer timed out; run /quest-iterate to diagnose and retry"; - } else if (job.pid && job.identity && identity(job.pid) === job.identity) return output; - else job.error = "fixer disappeared before recording an exit status; run /quest-iterate"; + } + // Without a launch identity (an unreadable /proc), only its exit status or the timeout can end the job. + else if (!job.identity || (job.pid && identity(job.pid) === job.identity)) return output; + else job.error = `fixer stopped without recording an exit status${restarted}; run /quest-iterate`; save(path, job); } while (job.numbers.length) { @@ -420,7 +439,7 @@ async function collectJob( if (entry.repository !== job.plan.repository && !entry.maintainerCanModify) throw new FixerRefusal("fork does not allow maintainer edits"); try { - importFixer(root, state, job.checkout, number, head); + importFixer(root, state, job.checkout, number, head, job.packs); } catch (error) { throw new FixerRefusal(String(error)); } diff --git a/test/land.test.ts b/test/land.test.ts index 7f491057..d0b69276 100644 --- a/test/land.test.ts +++ b/test/land.test.ts @@ -24,6 +24,7 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { ACTIONS_APP, type Pull, type World, fakeGitHub } from "../cloudflare/dev/github"; import { authenticatedRead, gitAuthentication } from "../src/cli/land-git"; import { collectFixers, validateFix, fixerText, handbackRepeated } from "../src/cli/land-fixer"; +import { importFixer } from "../src/cli/land-fixer-repository"; import fixerPrompt from "../assets/land-fix.md"; import { fix, parseFixResult } from "../src/cli/land-fix"; import { resolvePreparedQueue, prepareHead, finishFallback } from "../src/cli/land-head"; @@ -1599,7 +1600,16 @@ mv '${result}.new' '${result}' /** A fake agent reached through the real container payload, with no model or GitHub access. */ async function fixerFixture( script: string, - options: { fork?: boolean; conflict?: boolean; blocked?: boolean; raw?: boolean } = {}, + options: { + fork?: boolean; + conflict?: boolean; + blocked?: boolean; + raw?: boolean; + // The host's launcher: its output is a pipe that a slower reader drains. + pipe?: boolean; + // A second, healthy PR shares the failing batch. + collateral?: boolean; + } = {}, ) { const forge = new Forge(); const pull = forge.pull( @@ -1607,6 +1617,7 @@ async function fixerFixture( { "code.txt": "one\nII\nthree\n" }, options.fork ? { contributor: "outside", edits: false } : {}, ); + const collateral = options.collateral ? forge.pull("Bystander", { "other.txt": "fine\n" }) : undefined; if (options.conflict) forge.advance({ "code.txt": "one\n2\nthree\n" }); const deps = await forge.deps(); const plan = await resolveQueue(REPOSITORY, undefined, deps); @@ -1634,13 +1645,13 @@ ${script} wrapper, `#!/bin/sh set -eu -[ "$#" = 2 ] +[ "$#" = ${options.collateral ? 3 : 2} ] [ -z "\${HOME-}" ] [ -z "\${GH_TOKEN-}" ] [ -z "\${GITHUB_TOKEN-}" ] [ -z "\${QUEST_LAND_KEY-}" ] [ -z "\${CREDENTIALS_DIRECTORY-}" ] -${options.raw ? `exec '${agent}' "$@"` : `exec quest land fix --agent '${agent}' --agent-arg=- "$@"`} +${options.raw ? `exec '${agent}' "$@"` : options.pipe ? `quest land fix --agent '${agent}' --agent-arg=- "$@" 2>&1 | { sleep 1; cat; }` : `exec quest land fix --agent '${agent}' --agent-arg=- "$@"`} `, { mode: 0o755 }, ); @@ -1653,7 +1664,7 @@ ${options.raw ? `exec '${agent}' "$@"` : `exec quest land fix --agent '${agent}' const jobs = () => readdirSync(state.state).filter((name) => /^fix-job-.*\.json$/.test(name)); const collect = (dependencies = deps, timeout?: number) => collectFixers(root, state.state, dependencies, async () => deps.token, timeout); - return { forge, pull, deps, plan, state, root, finished, ready, fifo, jobs, collect, wrapper }; + return { forge, pull, collateral, deps, plan, state, root, finished, ready, fifo, jobs, collect, wrapper }; } function eventFile(dir: string, matches: (name: string) => boolean): Promise & { cancel: () => void } { let watcher: ReturnType; @@ -1784,6 +1795,86 @@ describe.skipIf(process.platform === "win32")("asynchronous land fixer", () => { expect(fixture.forge.comment(1)).toContain("fixer timed out"); fixture.finished.cancel(); }); + // moq's fixer panicked (exit 101) on every ejection of 2026-10-31: Codex's stderr was the launcher's pipe, which + // Bun had made non-blocking, so one command's output burst failed with EAGAIN. + test("survives an agent whose output outruns the launcher's reader", async () => { + const fixture = await fixerFixture(`head -c 1048576 /dev/zero | tr '\\0' x >&2\n${requeueScript}`, { + pipe: true, + }); + await fixture.finished; + expect(await fixture.collect()).toEqual(["requeued 1"]); + const log = readdirSync(fixture.state.state).find((name) => name.endsWith(".json.log"))!; + expect(readFileSync(join(fixture.state.state, log), "utf8")).toContain("x".repeat(1048576)); + }); + test("requeues a PR that only shared the batch, and hands back the one that broke it", async () => { + const fixture = await fixerFixture( + `printf '[{"number":1,"action":"handback","diagnosis":"breaks the check","recommendation":"run quest-iterate"},{"number":2,"action":"requeue","reason":"collateral: shared the batch with 1"}]' > quest-land-result.json`, + { collateral: true }, + ); + await fixture.finished; + expect(await fixture.collect()).toEqual(["handback 1", "requeued 2"]); + expect(fixture.pull.landing).toBe(false); + expect(fixture.forge.comment(1)).toContain("breaks the check"); + const collateral = fixture.collateral!; + expect(collateral.labels).toContain("quest:merge"); + expect(collateral.autoMerge?.commitBody).toBe(pin(collateral.head!)); + expect(fixture.forge.comment(2)).toContain("Requeued unchanged: collateral"); + }); + const editJob = (state: string, change: (job: Record) => void) => { + const path = join( + state, + readdirSync(state).find((name) => /^fix-job-.*\.json$/.test(name))!, + ); + const job = JSON.parse(readFileSync(path, "utf8")); + change(job); + writeFileSync(path, JSON.stringify(job)); + return job; + }; + test("names the signal that stopped a fixer and the daemon restart behind it", async () => { + const fixture = await fixerFixture(requeueScript, { blocked: true }); + await fixture.ready; + const job = editJob(fixture.state.state, (job) => (job.daemon = "another-boot:1")); + // A restart stops the unit's process group; the supervisor records the wrapper's status. + process.kill(-job.pid, "SIGTERM"); + await fixture.finished; + expect(await fixture.collect()).toEqual(["handback 1"]); + expect(fixture.forge.comment(1)).toContain( + "fixer stopped by SIGTERM (exit 143); the daemon restarted while it ran", + ); + }); + test("waits for a fixer whose process it could not identify, and says when one stops unrecorded", async () => { + const fixture = await fixerFixture(requeueScript, { blocked: true }); + await fixture.ready; + // A daemon that cannot read /proc/sys (systemd's ProcSubset=pid) records no identity. + editJob(fixture.state.state, (job) => delete job.identity); + expect(await fixture.collect()).toEqual([]); + editJob(fixture.state.state, (job) => { + job.identity = "gone:1"; + job.daemon = "another-boot:1"; + }); + expect(await fixture.collect()).toEqual(["handback 1"]); + expect(fixture.forge.comment(1)).toContain( + "fixer stopped without recording an exit status; the daemon restarted while it ran", + ); + writeFileSync(fixture.fifo, "continue\n"); + await fixture.finished; + }); + test("bounds a fixer's import by what it added, not the history fetched at launch", () => { + const root = tempDir(); + git(root, "init", "-q"); + git(root, "-c", "user.name=t", "-c", "user.email=t@localhost", "commit", "--allow-empty", "-qm", "base"); + const head = git(root, "rev-parse", "HEAD"); + const checkout = tempDir(); + git(checkout, "init", "-q"); + git(checkout, "fetch", "-q", root, `${head}:refs/heads/pr-1`); + // moq's history alone is larger than the bound; a sparse file stands in for it. + const pack = `pack-${"a".repeat(40)}`; + writeFileSync(join(checkout, ".git", "objects", "pack", `${pack}.pack`), ""); + truncateSync(join(checkout, ".git", "objects", "pack", `${pack}.pack`), 101 * 1024 * 1024); + writeFileSync(join(checkout, ".git", "objects", "pack", `${pack}.idx`), ""); + expect(() => importFixer(root, tempDir(), checkout, 1, head)).toThrow("exceeds 100 MiB"); + importFixer(root, tempDir(), checkout, 1, head, [`${pack}.pack`]); + }); test("passes no host credentials to wrapper or agent, including inherited Git configuration", async () => { for (const key of ["GH_TOKEN", "GITHUB_TOKEN", "QUEST_LAND_KEY", "CREDENTIALS_DIRECTORY", "SSH_AUTH_SOCK"]) vi.stubEnv(key, "must-not-reach-fixer"); From a4bfede38bd3f142dcc7a6ea1eeb4e5f62c27ab2 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 31 Oct 2026 08:36:06 -0700 Subject: [PATCH 3/4] fix(land): let the clean import borrow the launch history it skips Review: skipping the launch packs left the clean repository without a fixed commit's parents and unchanged objects, so its fetch failed. It now borrows them from the daemon's own object store; the fixer's alternates stay deleted. The test now packs real history and imports a descendant commit. Co-Authored-By: Claude Opus 5.5 --- README.md | 7 ++++--- src/cli/land-fixer-repository.ts | 7 +++++++ test/land.test.ts | 35 +++++++++++++++++++++++--------- 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 19b051e3..14e52204 100644 --- a/README.md +++ b/README.md @@ -531,9 +531,10 @@ daemon validates every result, ancestry, and protected path before lease-pushing as the bot. The protected set as a whole must match the pinned head or its merge with the current base, including content and mode. It covers `flake.nix`, `flake.lock`, `shell.nix`, `default.nix`, `nix/`, `.envrc`, `.github/`, `.gitmodules`, and gitlinks -anywhere. Collection copies at most 100 MiB of Git metadata the fixer added (packs -fetched at launch are already in the daemon's repository, so it skips them) into a clean object -repository, ignores fixer configuration, and never follows Git alternates. Pack indexes are rebuilt with +anywhere. Collection copies at most 100 MiB of Git metadata the fixer added into a +clean object repository. It skips the packs fetched at launch, whose objects the clean +repository borrows from the daemon's own. It ignores fixer configuration and never +follows the fixer's Git alternates. Pack indexes are rebuilt with `git index-pack` rather than trusted; the fetch into the daemon's repository checks every object and link, since the import enables `fetch.fsckObjects=true`. It records `mechanical fix by : ` and the new head in `## Review`, enables head-pinned auto-merge, and adds `quest:merge`. Healthy PRs diff --git a/src/cli/land-fixer-repository.ts b/src/cli/land-fixer-repository.ts index 204e36c9..3dc0266d 100644 --- a/src/cli/land-fixer-repository.ts +++ b/src/cli/land-fixer-repository.ts @@ -85,6 +85,13 @@ export function importFixer( const clean = mkdtempSync(join(state, "fix-import-")); try { git(clean, ["init", "--bare", "--quiet"]); + // The skipped launch packs' objects are the daemon's own: borrow them from its repository, never through + // any alternates the fixer wrote (deleted above), so the fixer's new commits connect to their history. + if (launched.size) + writeFileSync( + join(clean, "objects", "info", "alternates"), + `${join(git(root, ["rev-parse", "--absolute-git-dir"]), "objects")}\n`, + ); let copied = 0; for (const path of objects) { const data = read(path); diff --git a/test/land.test.ts b/test/land.test.ts index d0b69276..2923a554 100644 --- a/test/land.test.ts +++ b/test/land.test.ts @@ -1861,19 +1861,34 @@ describe.skipIf(process.platform === "win32")("asynchronous land fixer", () => { }); test("bounds a fixer's import by what it added, not the history fetched at launch", () => { const root = tempDir(); + const commit = (cwd: string, message: string) => + git(cwd, "-c", "user.name=t", "-c", "user.email=t@localhost", "commit", "-qam", message); git(root, "init", "-q"); - git(root, "-c", "user.name=t", "-c", "user.email=t@localhost", "commit", "--allow-empty", "-qm", "base"); - const head = git(root, "rev-parse", "HEAD"); + writeFileSync(join(root, "kept.txt"), "unchanged\n"); + writeFileSync(join(root, "code.txt"), "before\n"); + git(root, "add", "."); + commit(root, "base"); + const pinned = git(root, "rev-parse", "HEAD"); + // The launch fetch packs the history, as startFixer's does for a real repository. const checkout = tempDir(); git(checkout, "init", "-q"); - git(checkout, "fetch", "-q", root, `${head}:refs/heads/pr-1`); - // moq's history alone is larger than the bound; a sparse file stands in for it. - const pack = `pack-${"a".repeat(40)}`; - writeFileSync(join(checkout, ".git", "objects", "pack", `${pack}.pack`), ""); - truncateSync(join(checkout, ".git", "objects", "pack", `${pack}.pack`), 101 * 1024 * 1024); - writeFileSync(join(checkout, ".git", "objects", "pack", `${pack}.idx`), ""); - expect(() => importFixer(root, tempDir(), checkout, 1, head)).toThrow("exceeds 100 MiB"); - importFixer(root, tempDir(), checkout, 1, head, [`${pack}.pack`]); + git(checkout, "-c", "fetch.unpackLimit=1", "fetch", "-q", root, `${pinned}:refs/heads/pr-1`); + const packs = readdirSync(join(checkout, ".git", "objects", "pack")).filter((name) => name.endsWith(".pack")); + expect(packs).toHaveLength(1); + // The fix descends from that history, which it shares rather than carries. + git(checkout, "checkout", "-q", "pr-1"); + writeFileSync(join(checkout, "code.txt"), "after\n"); + commit(checkout, "mechanical fix"); + const fixed = git(checkout, "rev-parse", "HEAD"); + // moq's history alone is larger than the bound; a sparse launch pack stands in for it. + const large = `pack-${"a".repeat(40)}`; + writeFileSync(join(checkout, ".git", "objects", "pack", `${large}.pack`), ""); + truncateSync(join(checkout, ".git", "objects", "pack", `${large}.pack`), 101 * 1024 * 1024); + writeFileSync(join(checkout, ".git", "objects", "pack", `${large}.idx`), ""); + expect(() => importFixer(root, tempDir(), checkout, 1, fixed, packs)).toThrow("exceeds 100 MiB"); + importFixer(root, tempDir(), checkout, 1, fixed, [...packs, `${large}.pack`]); + expect(git(root, "show", `${fixed}:kept.txt`)).toBe("unchanged"); + expect(git(root, "rev-parse", `${fixed}^`)).toBe(pinned); }); test("passes no host credentials to wrapper or agent, including inherited Git configuration", async () => { for (const key of ["GH_TOKEN", "GITHUB_TOKEN", "QUEST_LAND_KEY", "CREDENTIALS_DIRECTORY", "SSH_AUTH_SOCK"]) From 6a1e3d7e3b0bfd365bf657fc3bd05b389c421acc Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 31 Oct 2026 11:29:10 -0700 Subject: [PATCH 4/4] test(land): count the burst instead of requiring it unbroken The fixer relays stdout and stderr into one log, so output from the other stream may land inside the 1 MiB burst. The gate's container did that and failed the contiguous match. Co-Authored-By: Claude Opus 5.5 --- test/land.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/land.test.ts b/test/land.test.ts index 2923a554..77b69e9b 100644 --- a/test/land.test.ts +++ b/test/land.test.ts @@ -1804,7 +1804,9 @@ describe.skipIf(process.platform === "win32")("asynchronous land fixer", () => { await fixture.finished; expect(await fixture.collect()).toEqual(["requeued 1"]); const log = readdirSync(fixture.state.state).find((name) => name.endsWith(".json.log"))!; - expect(readFileSync(join(fixture.state.state, log), "utf8")).toContain("x".repeat(1048576)); + // Both streams relay into one log, so other output may interleave with the burst. + const kept = readFileSync(join(fixture.state.state, log), "utf8").match(/x/g)?.length ?? 0; + expect(kept).toBeGreaterThanOrEqual(1048576); }); test("requeues a PR that only shared the batch, and hands back the one that broke it", async () => { const fixture = await fixerFixture(