quest land: a failed fixer's handback quotes the gate's failure

src/cli/land-fixer.ts

@@ -64,20 +64,26 @@ 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)
 		.join("\n")
 		.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]

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,
 	);

test/land-fixer-diagnostics.test.ts

@@ -19,7 +19,7 @@ import type { Plan } from "../src/cli/land";
 
 // Linux only, like the land daemon: the fixture uses mkfifo and POSIX shells.
 test.skipIf(process.platform === "win32")(
-	"retains agent output and actual exit code, with a bounded escaped handback tail",
+	"retains agent output and actual exit code, and hands back the gate's escaped failure",
 	async () => {
 		const dir = mkdtempSync(join(tmpdir(), "quest-fixer-diagnostics-"));
 		const root = join(dir, "repo"),
@@ -83,7 +83,7 @@ test.skipIf(process.platform === "win32")(
 		const ready = join(dir, "ready"),
 			release = join(dir, "release");
 		execFileSync("mkfifo", [release]);
-		writeFileSync(log, "fixture failure\n");
+		writeFileSync(log, "fixture failure: <panic>& in the gate\n");
 		writeFileSync(
 			agent,
 			`#!/bin/sh\ncat >/dev/null\ni=0; while [ $i -lt 150 ]; do printf 'prefix-%s\\n' $i; i=$((i+1)); done\nprintf '<panic>& diagnostic stdout\\n'\nprintf 'diagnostic stderr\\n' >&2\ntouch '${ready}'\nread -r release < '${release}'\nexit 101\n`,
@@ -122,8 +122,9 @@ test.skipIf(process.platform === "win32")(
 			expect(output).toContain("diagnostic stderr");
 			expect(await collectFixers(root, state, deps, async () => "fake")).toEqual(["handback 1"]);
 			expect(comment).toContain("fixer crashed (exit 101)");
-			expect(comment).toContain("&lt;panic>&amp; diagnostic stdout");
-			expect(comment).toContain("diagnostic stderr");
+			// The handback quotes the gate's failure, escaped; the fixer's own output stays in its job log.
+			expect(comment).toContain("fixture failure: &lt;panic>&amp; in the gate");
+			expect(comment).not.toContain("diagnostic");
 			expect(comment).not.toContain("prefix-0\n");
 			expect(comment.length).toBeLessThan(65000);
 			expect(existsSync(path)).toBe(false);

test/land-watch.test.ts

@@ -1688,7 +1688,11 @@ git commit -qm 'disable render_timeout'`;
 		expect(handback).toContain("<pre>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("running tests\nFAIL queue.test.ts");
+		expect(crash).not.toContain("agent crashed");
 		expect(history(state, 11)!.rounds).toMatchObject([
 			{ outcome: "handback", before: crashed, failure: "Check / test: just test" },
 		]);

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("<!-- quest land fixer:");
+		expect(fixture.forge.comment(1)).toContain("FAIL gate check");
+		expect(fixture.forge.comment(1)).not.toContain("agent output");
 	});
 	const minutes = (value: number) => value * 60_000;
 	/** Collect at each minute mark, expecting the fixer to stay running until the last two terminate it. */