feat(land): post the fixer's comments from its result

README.md

@@ -524,10 +524,18 @@ least 10 seconds for container cleanup, then sends SIGKILL on a later collection
 to `codex exec`; `--agent <executable>` and repeatable `--agent-arg <argument>`
 replace it for another CLI. It writes `<cwd>/quest-land-result.json`: an array of
 `{number, action:"requeue", reason}`, `{number, action:"fixed", head, reason}`, or
-`{number, action:"handback", diagnosis, recommendation}`, with no extra fields.
+`{number, action:"handback", diagnosis, recommendation}`, with no extra fields
+but an optional `comment`.
 The payload adds `agent` and `model` metadata to each result (`--model <id>`
 selects the default Codex model; unknown models fall back to `Codex`).
 Reasons and diagnoses are single lines. Fixed heads must be committed on `pr-<n>`.
+A `comment` is Markdown of at most 4000 characters that the fixer wants on its PR.
+The fixer holds no GitHub credential, since PR code it builds can read the
+container's environment, so the daemon posts the comment as the bot when it
+collects the job, before its own outcome comment. It quotes the text under the
+recorded agent, strips HTML comments so the text can never carry the daemon's
+hidden markers, defuses @-mentions, and redacts GitHub tokens, as it does for
+every fixer text and log it posts.
 
 Jobs and attempts persist across daemon restarts. A running fixer outlives a
 restart only where the host leaves its process group alive (under systemd,

assets/land-fix.md

@@ -51,6 +51,15 @@ one entry per supplied PR, no extra fields. Use one of:
 {"number":123,"action":"fixed","head":"full 40-character SHA","reason":"one-line mechanical reason"}
 {"number":123,"action":"handback","diagnosis":"one line","recommendation":"one line"}
 
+You have no GitHub credential and must not seek one. To tell a PR's maintainer
+something, such as what a fix changed and why, a flaky test, or the decision a
+handback needs, add an optional `"comment"` field to that PR's entry: Markdown of
+at most 4000 characters, one per PR. The daemon posts it on that PR, quoted under
+your name, after it collects the result. Write plain prose with no @-mentions,
+HTML comments, or secrets: the daemon strips mentions, HTML comments, and GitHub
+tokens. A comment decides nothing: the action sets the PR's outcome, and the
+daemon still posts its own outcome comment.
+
 A fixed head must be the tip of pr-<number>, descend from its pinned head, and
 contain only that PR's fix and base history. Leave all other PR branches intact.
 Keep the result file uncommitted.

quest/m0/land-fixer/README.md

@@ -83,7 +83,13 @@ Decision prompts, selected options marked ✅:
     disable the flaky test in the base PR then."
   - The flaky commit: ✅ in this batch (on its base PR, per the answer above)
     · in the next batch
-  - GitHub comments: the daemon posts them · ✅ a comment-only token · both
+  - GitHub comments: ✅ the daemon posts them from the fixer's result · a
+    comment-only token in the container · both. The user first picked the
+    token, then reversed it on 2026-11-01 after an independent audit: PR code
+    the fixer builds can read the container's environment, so a token there
+    would let PR code post as moq-bot[bot]. `quest land apply`, the daemon's
+    own comment deduplication, and `quest status`'s ejection reason all trust
+    moq-bot comments.
   - Workflow pushes: ✅ grant moq-bot `workflows` · the fixer avoids
     workflows
 - The user, starting the work: "The fixer is meant to handle conflicts and
@@ -100,7 +106,6 @@ resolves itself and what it hands back; that docs update finishes the epic.
 ## Required
 
 - [Workflow permission](/quest/m0/land-fixer/workflows-permission.md) - waits on the maintainer to grant moq-bot `workflows`, so pushes touching workflows are accepted
-- [Fixer comments](/quest/m0/land-fixer/comments.md) - a per-job, comment-only GitHub token lets the fixer explain its work on the PRs
 - [Attempt budget](/quest/m0/land-fixer/attempts.md) - only a fix that fails again spends a pick's single attempt
 - [Batch triage](/quest/m0/land-fixer/triage.md) - the fixer blames the failing PR before ejection, and a red base halts the queue
 - [CI failures park](/quest/m0/land-fixer/ci-park.md) - a PR whose own required CI fails is parked for the fixer with the failing log, not ejected

quest/m0/land-fixer/check-status.md

@@ -29,8 +29,8 @@ landing through its own push or the pinned squash merge, as now.
 - Needs the App's `checks: write`, an installation permission the maintainer
   accepts. Check it during this work; it may already be granted for the
   existing check.
-- [Fixer comments](/quest/m0/land-fixer/comments.md) explain decisions on
-  the PR; the check run shows state only.
+- The fixer's comments, which the daemon posts on the PR, explain
+  decisions; the check run shows state only.
 
 ## Related
 

src/cli/land-fix.ts

@@ -1,4 +1,5 @@
-// Untrusted agent side. The host wrapper supplies only a model endpoint and its client key.
+// Untrusted agent side. The host wrapper supplies only a model endpoint and its client key; the agent holds no
+// GitHub credential, and the daemon posts the comments it writes into its result.
 import { spawn, spawnSync } from "node:child_process";
 import { constants, closeSync, fstatSync, openSync, readFileSync, rmSync, writeFileSync } from "node:fs";
 import { basename, join } from "node:path";
@@ -21,13 +22,15 @@ export function readFixResult(root: string): string {
 		closeSync(fd);
 	}
 }
-export type FixResult = { agent?: string; model?: string } & (
+/** The longest Markdown comment a result may ask the daemon to post on its PR. */
+export const COMMENT_LIMIT = 4000;
+export type FixResult = { agent?: string; model?: string; comment?: string } & (
 	| { number: number; action: "requeue"; reason: string }
 	| { number: number; action: "fixed"; head: string; reason: string }
 	| { number: number; action: "handback"; diagnosis: string; recommendation: string }
 );
 
-/** Reject extra fields, duplicate/missing PRs, multiline reasons, and oversized output. */
+/** Reject extra fields, duplicate/missing PRs, multiline reasons, oversized comments, and oversized output. */
 export function parseFixResult(text: string, numbers: number[]): FixResult[] {
 	if (text.length > 100_000) throw new Error("fixer result exceeds 100 KB");
 	let value: unknown;
@@ -54,9 +57,19 @@ export function parseFixResult(text: string, numbers: number[]): FixResult[] {
 		const metadata = ["agent", "model"].filter((key) => key in item);
 		if (metadata.some((key) => typeof item[key] !== "string" || !/^[a-zA-Z0-9 ._/-]{1,100}$/.test(item[key])))
 			throw new Error("invalid fixer identity");
+		// One optional Markdown comment per PR: multiline, but no other control characters.
+		if (
+			"comment" in item &&
+			(typeof item.comment !== "string" ||
+				!item.comment.trim() ||
+				item.comment.length > COMMENT_LIMIT ||
+				/[\u0000-\u0008\u000b\u000c\u000e-\u001f\u007f]/.test(item.comment))
+		)
+			throw new Error("invalid fixer comment");
+		const optional = metadata.length + ("comment" in item ? 1 : 0);
 		if (
 			fields.length === 0 ||
-			Object.keys(item).length !== fields.length + metadata.length ||
+			Object.keys(item).length !== fields.length + optional ||
 			fields.some((key) => !(key in item))
 		)
 			throw new Error("invalid fixer action or fields");

src/cli/land-fixer.ts

@@ -95,14 +95,14 @@ function identity(pid: number): string | undefined {
 		return undefined;
 	}
 }
-async function comment(
+/** Post `content` once per pick and kind: a retry finds the bot's own marker and posts nothing. */
+async function post(
 	github: GitHub,
 	plan: Plan,
 	entry: Pick<Entry, "number" | "enabledAt">,
-	text: string,
+	content: string,
 	suffix: string,
-	attribution = "Codex",
-	log = "",
+	attribution: string,
 	section?: (room: number) => string,
 ): Promise<void> {
 	const path = `/repos/${plan.repository}/issues/${entry.number}/comments`;
@@ -114,10 +114,22 @@ async function comment(
 		)
 	)
 		return;
-	const lead = `${attribution} land fixer: ${fixerText(text)}${log ? `\n\n<pre>${excerpt(log)}</pre>` : ""}`;
 	const tail = `\n\n${marker}\n\n(written by ${attribution})`;
-	const extra = section?.(COMMENT_LIMIT - lead.length - tail.length) ?? "";
-	await github.request(path, { body: { body: `${lead}${extra}${tail}` } });
+	const extra = section?.(COMMENT_LIMIT - content.length - tail.length) ?? "";
+	await github.request(path, { body: { body: `${content}${extra}${tail}` } });
+}
+async function comment(
+	github: GitHub,
+	plan: Plan,
+	entry: Pick<Entry, "number" | "enabledAt">,
+	text: string,
+	suffix: string,
+	attribution = "Codex",
+	log = "",
+	section?: (room: number) => string,
+): Promise<void> {
+	const content = `${attribution} land fixer: ${fixerText(text)}${log ? `\n\n<pre>${excerpt(redactTokens(log))}</pre>` : ""}`;
+	await post(github, plan, entry, content, suffix, attribution, section);
 }
 
 /** Queue eligibility can eject a previously requeued PR before building another batch. */
@@ -286,9 +298,29 @@ export function validateFix(root: string, entry: Entry, head: string, base: stri
 		throw new Error("off-limits fixer edit: protected set must equal pinned head or base merge result");
 }
 
+/** Anything the fixer writes may echo a token it read; GitHub tokens never reach a post. */
+export function redactTokens(text: string): string {
+	return text.replace(/\b(?:gh[oprsu]_|github_pat_)[A-Za-z0-9_]{20,}/g, "[redacted token]");
+}
+
+/**
+ * The fixer's own Markdown comment, quoted under its name. It cannot carry a hidden marker, so it never
+ * passes for the daemon's handback, ejection, or apply comments, and it mentions no one.
+ */
+export function fixerMarkdown(text: string): string {
+	let value = redactTokens(text);
+	// Removing one opener can join the text around it into another.
+	while (value.includes("<!--")) value = value.replaceAll("<!--", "");
+	return value
+		.replaceAll("@", "[at]")
+		.split(/\r?\n|\r/)
+		.map((line) => `> ${line}`)
+		.join("\n");
+}
+
 /** Contributor prose is quoted data, never GitHub markup, mentions, or issue-closing instructions. */
 export function fixerText(text: string): string {
-	const value = text
+	const value = redactTokens(text)
 		.replaceAll("<!--", "")
 		.slice(0, 500)
 		.replaceAll("@", "[at]")
@@ -521,6 +553,16 @@ async function collectJob(
 				error = `${String(failure)}; run /quest-iterate before picking it again`;
 			}
 		}
+		// The fixer's own note goes first, so the daemon's outcome comment follows it.
+		if (result?.comment)
+			await post(
+				github,
+				job.plan,
+				entry,
+				`${author(result)} land fixer note:\n\n${fixerMarkdown(result.comment)}`,
+				"note",
+				author(result),
+			);
 		if (!error && result?.action !== "handback") {
 			await github.request(`/repos/${job.plan.repository}/issues/${number}/labels`, {
 				body: { labels: [MERGE_LABEL] },

src/cli/main.ts

@@ -507,7 +507,7 @@ export function main(argv: string[], out: Output): number | Promise<number> {
 	landCommand
 		.command("fix")
 		.description(
-			"Run a mechanical agent fix inside the credential-free fixer container; write quest-land-result.json. Model endpoint and client key: OPENAI_BASE_URL and OPENAI_API_KEY.",
+			"Run a mechanical agent fix inside the credential-free fixer container; write quest-land-result.json, whose optional comments the daemon posts. Model endpoint and client key: OPENAI_BASE_URL and OPENAI_API_KEY.",
 		)
 		.argument("<log>", "Full failure log exposed by the wrapper")
 		.argument("<prs...>", "PR numbers with pr-<n> branches")
@@ -552,7 +552,7 @@ export function main(argv: string[], out: Output): number | Promise<number> {
 	landCommand
 		.command("serve")
 		.description(
-			"Run one daemon per repository. As a batch is filled, eject alone each PR it reaches whose squash onto the base fails quest check with findings the base lacks, and comment once on each newly queued PR that conflicts with the four PRs landing soonest ahead of it. Check a base PR plus up to floor(log2 n) cleanly merging PRs once through the sandbox, alternating the oldest and the newest queued PR as the base. Failed batches are ejected together, never bisected, and halve the next batch's size down to one PR; each landed batch grows it back by one. Requires quest on PATH and flock. Sandbox and fixer wrappers MUST run code as a different user from the App key owner (for example in nspawn containers); environment filtering alone cannot isolate /proc or host files. The sandbox receives <checkout> <base-sha>, with the base also in QUEST_LAND_BASE and present in the standalone shallow clone; the optional tracked background fixer receives the log path followed by PR numbers. Neither receives credentials. State retains logs, fixer jobs, and one mechanical attempt per maintainer pick across restarts. Fixer results are validated, lease-pushed, recorded in Review, and pinned for requeue; invalid results, crashes, and timeouts are handed back. A validated fix whose push GitHub refuses is handed back with its resolution for `quest land apply`. After revalidating the queue, merge the base into each behind-base PR head with quest resolve, verify the tested tree, and lease-push that head before the pinned GitHub squash API merge. A permissions-refused branch push falls back to a leased fast-forward of the tested squash to the base, closing the PR with a linked-commit comment and removing quest:merge. A recorded daemon merge atop the original pin is recognized after restart; other moved heads require maintainer requeueing. Pending mergeability is retried briefly; confirmed conflicts or moved heads eject the remaining batch. A refusal that would repeat, such as squash being disabled, ejects as merge_refused without the fixer. An unexpected squash parent rebuilds the queue without ejecting; a tree mismatch logs CRITICAL and ejects the rest as tree_mismatch. Merged PRs drop out of the queue after restart.",
+			"Run one daemon per repository. As a batch is filled, eject alone each PR it reaches whose squash onto the base fails quest check with findings the base lacks, and comment once on each newly queued PR that conflicts with the four PRs landing soonest ahead of it. Check a base PR plus up to floor(log2 n) cleanly merging PRs once through the sandbox, alternating the oldest and the newest queued PR as the base. Failed batches are ejected together, never bisected, and halve the next batch's size down to one PR; each landed batch grows it back by one. Requires quest on PATH and flock. Sandbox and fixer wrappers MUST run code as a different user from the App key owner (for example in nspawn containers); environment filtering alone cannot isolate /proc or host files. The sandbox receives <checkout> <base-sha>, with the base also in QUEST_LAND_BASE and present in the standalone shallow clone; the optional tracked background fixer receives the log path followed by PR numbers. Neither receives credentials. State retains logs, fixer jobs, and one mechanical attempt per maintainer pick across restarts. Fixer results are validated, lease-pushed, recorded in Review, and pinned for requeue, and the comment a result carries for its PR is posted there; invalid results, crashes, and timeouts are handed back. A validated fix whose push GitHub refuses is handed back with its resolution for `quest land apply`. After revalidating the queue, merge the base into each behind-base PR head with quest resolve, verify the tested tree, and lease-push that head before the pinned GitHub squash API merge. A permissions-refused branch push falls back to a leased fast-forward of the tested squash to the base, closing the PR with a linked-commit comment and removing quest:merge. A recorded daemon merge atop the original pin is recognized after restart; other moved heads require maintainer requeueing. Pending mergeability is retried briefly; confirmed conflicts or moved heads eject the remaining batch. A refusal that would repeat, such as squash being disabled, ejects as merge_refused without the fixer. An unexpected squash parent rebuilds the queue without ejecting; a tree mismatch logs CRITICAL and ejects the rest as tree_mismatch. Merged PRs drop out of the queue after restart.",
 		)
 		.option("--base <branch>", "Base branch (default: repository default)")
 		.option("--state <path>", "Private daemon state directory (default: $STATE_DIRECTORY or the XDG state home)")