From 1ccc31908fd6d0305f6b3b05a131a98599d9077b Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 11 Nov 2026 00:53:52 -0800 Subject: [PATCH 1/3] chore(quest): claim quest/m0/queue-watcher/fixer-crash-quotes-gate.md Quest-Claim: 4f40924e39d55490 From 84ab023501b84cccdf443d252e69f36a96e9e879 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 11 Nov 2026 01:46:25 -0800 Subject: [PATCH 2/3] quest land: a failed fixer's handback quotes the gate's failure Co-Authored-By: Claude Opus 5.5 --- .../queue-watcher/fixer-crash-quotes-gate.md | 32 ------------------- src/cli/land-fixer.ts | 27 ++++++++-------- src/cli/land-watch-fix.ts | 5 ++- test/land-watch.test.ts | 6 +++- test/land.test.ts | 7 ++-- 5 files changed, 25 insertions(+), 52 deletions(-) delete mode 100644 quest/m0/queue-watcher/fixer-crash-quotes-gate.md diff --git a/quest/m0/queue-watcher/fixer-crash-quotes-gate.md b/quest/m0/queue-watcher/fixer-crash-quotes-gate.md deleted file mode 100644 index 91501f7f..00000000 --- a/quest/m0/queue-watcher/fixer-crash-quotes-gate.md +++ /dev/null @@ -1,32 +0,0 @@ -# [S] A failed fixer still hands back the gate's failure - -## Goal - -When the land fixer fails for its own reasons (a crash, a refused launch, a -timeout, an invalid result), its handback still quotes the gate's own failure: -the check output that dropped the PR. The PR's author sees the real cause -without reading the host's logs. A test fails a fixer run on purpose and finds -the gate's failing output in the handback's comment. - -## Plan - -Planned 2026-11-10 (maintainer, /quest-plan in moq-dev/moq.pro): ✅ m0, in this -epic (rejected: m1). Every hidden failure costs a person a round trip. - -Found on moq-dev/moq.pro on 2026-11-10. From 00:33Z to 22:43Z the host's fixer -launcher refused every run (kixelated/nixos, fixed by its #46), and each -handback quoted only `quest land check: [Errno 32] Broken pipe`. The gate's own -logs held the real causes, all fixable without a person: a type error from a -base merge, a migration check that needs a remote, and a batchmate's oversized -skill file. Requeueing by hand then dropped the same innocent PRs again. - -`land-fixer.ts` already keeps "the failure log the fixer read, which a -handback's comment quotes", but on these runs the quoted log was the wrapper's -41-byte error. Find where that log is chosen on each failure path in `settle`, -and quote the check's failing output there. Name the fixer's own error -separately, as the comment's reason. - -## Related - -- [Land through a serial merge queue with an async fixer](/quest/m0/queue-watcher/README.md) - - the fixer this hands back from diff --git a/src/cli/land-fixer.ts b/src/cli/land-fixer.ts index 7bd6df2b..0f1e02e4 100644 --- a/src/cli/land-fixer.ts +++ b/src/cli/land-fixer.ts @@ -64,9 +64,15 @@ class BaseCheckError extends Error {} /** A fix that edits protected paths beyond what the base merge carries: only a maintainer's push lands it. */ export class ProtectedEdit extends Error {} -/** The failure log's lines the fixer cited, first to last; the comment bounds them. */ -export function cited(log: string, [first, last]: [number, number]): string { +/** + * The failure log a handback quotes: the lines the fixer cited, else the tail of the gate's failing + * output, so a fixer that failed for its own reasons still shows why the gate dropped the PR. The + * fixer's own output stays in its job log on the host; the comment's reason names its error. + */ +export function handbackLog(log: string, lines?: [number, number]): string { + if (!lines) return readLog(log, "tail").trimEnd().split("\n").slice(-100).join("\n"); if (!existsSync(log)) return ""; + const [first, last] = lines; return readFileSync(log, "utf8") .split("\n") .slice(first - 1, last) @@ -74,10 +80,10 @@ export function cited(log: string, [first, last]: [number, number]): string { .trimEnd(); } -/** Read a bounded window of the job's log, or "" when it is missing or not a regular file. */ +/** Read a bounded window of a log, or "" when it is missing or not a regular file. */ function readLog(path: string, window: "head" | "tail"): string { - if (!existsSync(`${path}.log`)) return ""; - const fd = openSync(`${path}.log`, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK); + if (!path || !existsSync(path)) return ""; + const fd = openSync(path, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK); try { const stat = fstatSync(fd); if (!stat.isFile()) return ""; @@ -89,11 +95,6 @@ function readLog(path: string, window: "head" | "tail"): string { } } -/** Read a bounded tail, retaining the complete per-job log for host diagnosis. */ -export function fixerLog(path: string): string { - return readLog(path, "tail").trimEnd().split("\n").slice(-100).join("\n"); -} - /** * A wrapper that queues for a shared launcher prints this line, followed by the Unix time in * seconds, once the launcher admits the job. Only the first one counts: the agent's own output @@ -108,7 +109,7 @@ export const ADMITTED = "quest land fixer: admitted"; */ function deadline(path: string, job: Process, now: number, timeout: number): number { if (job.admitted === undefined) { - const match = new RegExp(`^${ADMITTED} (\\d{1,15})$`, "m").exec(readLog(path, "head")); + const match = new RegExp(`^${ADMITTED} (\\d{1,15})$`, "m").exec(readLog(`${path}.log`, "head")); if (match) { // Kept within [started, now], so a wrong clock or a forged line cannot move it out of the run. job.admitted = Math.min(Math.max(Number(match[1]) * 1000, job.started), now); @@ -1188,9 +1189,7 @@ async function collectJob( : "missing result; run /quest-iterate", "handback", author(result), - !error && result?.action === "handback" && result.log && job.log - ? cited(job.log, result.log) - : fixerLog(path), + handbackLog(job.log ?? "", !error && result?.action === "handback" ? result.log : undefined), // The marker `quest status` reads, now that no ejection comment precedes this one. job.parked && leaves ? (job.reason ?? "failed_checks") : undefined, error && job.resolutions?.[number] diff --git a/src/cli/land-watch-fix.ts b/src/cli/land-watch-fix.ts index 3bf7c6ff..42f85406 100644 --- a/src/cli/land-watch-fix.ts +++ b/src/cli/land-watch-fix.ts @@ -12,12 +12,11 @@ import { checksSection } from "./land-checks"; import { type Cause, EFFORT_VARIABLE, type FixResult, knownFlaky, parseFixResult, readFixResult } from "./land-fix"; import { author, - cited, COLLECTION_ATTEMPTS, comment, conflictFailure, FixerRefusal, - fixerLog, + handbackLog, fixerMarkdown, fixerText, history, @@ -688,7 +687,7 @@ async function collect( : "missing result; run /quest-iterate", `handback ${name}`, attribution, - !handback && result?.action === "handback" && result.log ? cited(job.log, result.log) : fixerLog(path), + handbackLog(job.log, !handback && result?.action === "handback" ? result.log : undefined), job.reason, job.resolution ? (room) => resolutionSection(number, job.resolution!, room) : undefined, ); diff --git a/test/land-watch.test.ts b/test/land-watch.test.ts index 4e7c28ac..f7f20e49 100644 --- a/test/land-watch.test.ts +++ b/test/land-watch.test.ts @@ -1688,7 +1688,11 @@ git commit -qm 'disable render_timeout'`; expect(handback).toContain("
The merge queue removed");
 		expect(history(state, 9)!.rounds).toMatchObject([{ outcome: "handback" }]);
 		// A crash ends the pick too, with a round of its own.
-		expect(github.comments[11].at(-1)!.body).toContain("fixer crashed (exit 3)");
+		// Its handback names the crash as the reason and quotes the gate's failure, not the fixer's output.
+		const crash = github.comments[11].at(-1)!.body;
+		expect(crash).toContain("fixer crashed (exit 3)");
+		expect(crash).toContain("FAIL queue.test.ts
"); + expect(crash).not.toContain("agent crashed"); expect(history(state, 11)!.rounds).toMatchObject([ { outcome: "handback", before: crashed, failure: "Check / test: just test" }, ]); diff --git a/test/land.test.ts b/test/land.test.ts index 81f622b5..1116ba14 100644 --- a/test/land.test.ts +++ b/test/land.test.ts @@ -2638,7 +2638,7 @@ ${options.raw ? `exec '${agent}' "$@"` : options.pipe ? `quest land fix --agent { mode: 0o755 }, ); // Fail a check so the ordinary ejection and launcher paths run. - writeFileSync(state.sandbox, "#!/bin/sh\nexit 1\n", { mode: 0o755 }); + writeFileSync(state.sandbox, "#!/bin/sh\necho 'FAIL gate check'\nexit 1\n", { mode: 0o755 }); const root = forge.clone(); const finished = eventFile(state.state, (name) => name.endsWith(".exit")); const ready = options.blocked ? eventFile(tools, (name) => name === "ready") : undefined; @@ -3106,7 +3106,8 @@ fi`); ["default.nix", fixedScript("default.nix"), {}, "off-limits fixer edit"], [".gitmodules", fixedScript(".gitmodules"), {}, "off-limits fixer edit"], ["fork", fixedScript(), { fork: true }, "fork does not allow maintainer edits"], - ["crash", "exit 17", {}, "fixer crashed"], + // The fixer's own failure is the reason; the quote is still the gate's failing check. + ["crash", "echo 'agent output' >&2; exit 17", {}, "fixer crashed"], ["invalid result", "printf '{}' > quest-land-result.json", { raw: true }, "invalid fixer result"], ["symlink result", 'ln -s "$1" quest-land-result.json', { raw: true }, "invalid fixer result"], ] as const)("hands back %s without requeueing", async (_name, script, options, message) => { @@ -3117,6 +3118,8 @@ fi`); expect(fixture.pull.head).toBe(fixture.plan.queue[0].head); expect(fixture.forge.comment(1)).toContain(message); expect(fixture.forge.comment(1)).toContain("