From b85a202dfb8c16d24cb7085ebebeeff10bb65e8f Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 1 Nov 2026 10:00:05 -0800 Subject: [PATCH 1/5] chore(quest): claim quest/m0/land-fixer/comments.md Quest-Claim: 97a040916277568a From a13af17a6ae52cc19e470f7732e58a72ed672747 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 1 Nov 2026 10:11:39 -0800 Subject: [PATCH 2/5] feat(land): give the fixer a per-job token to comment on the batch's PRs The daemon mints an installation token for the job's repository with only `issues: write` and passes it to the fixer wrapper as GH_TOKEN, with GH_REPO; `quest land fix` forwards both to the agent and nothing else. GitHub expires the token within the hour, matching the fixer's 60-minute deadline. The prompt lets the fixer post or edit its own comments on the supplied PRs, and the daemon redacts GitHub tokens from the fixer's text and log in its own posts. Co-Authored-By: Claude Opus 5.5 --- README.md | 21 ++++++---- assets/land-fix.md | 20 +++++++--- quest/m0/host-land/fixer.md | 7 +++- quest/m0/land-fixer/README.md | 1 - quest/m0/land-fixer/comments.md | 44 --------------------- quest/m0/land-fixer/launch.md | 4 -- src/cli/land-fix.ts | 5 ++- src/cli/land-fixer.ts | 20 ++++++++-- src/cli/land-serve.ts | 22 ++++++----- src/cli/main.ts | 6 +-- test/land.test.ts | 69 ++++++++++++++++++++++++++++----- 11 files changed, 131 insertions(+), 88 deletions(-) delete mode 100644 quest/m0/land-fixer/comments.md diff --git a/README.md b/README.md index e91eaf5b..40ad3516 100644 --- a/README.md +++ b/README.md @@ -504,18 +504,25 @@ The sandbox runs the repository's checks (`just check` and `just test` here). No credential or inherited home configuration is passed to it. The daemon refuses to start without `--sandbox`. The optional fixer runs as tracked background work, receiving the full log path followed by -PR numbers, with the same restricted environment (`PATH` and `LANG` only). +PR numbers, with the same restricted environment plus `GH_TOKEN` and `GH_REPO`. +`GH_TOKEN` is an installation token the daemon mints for each job with only +`issues: write` on this repository, so the fixer can comment on the batch's PRs. +It cannot push, merge, or reach another repository, though it can also edit or +close issues and change labels there, and GitHub expires it within the hour, +matching the fixer's 60-minute deadline. The daemon's own comments redact GitHub +tokens from the fixer's text and log. Its cwd is the standalone repository, with `batch`, `base`, and `pr-` branches. The wrapper starts an ephemeral container and runs `quest land fix ` there, exposing that repository read/write and the log read-only at the given paths. It supplies a fresh HOME, `OPENAI_BASE_URL` (the OpenAI-compatible Responses -endpoint, including `/v1`) and `OPENAI_API_KEY` (the gate's proxy client key). -The daemon never supplies these or any Git/GitHub credential, and the wrapper -must keep every other credential out of the container. Its network must reach the +endpoint, including `/v1`) and `OPENAI_API_KEY` (the gate's proxy client key), +and passes on `GH_TOKEN` and `GH_REPO`. The daemon never supplies the model +credentials or any other Git/GitHub credential, and the wrapper must keep every +other credential out of the container. Its network must reach the model proxy and, so the fixer can build and test its fix, the package registries and `github.com`/`codeload.github.com` that the repository's dependencies come from. -The prompt allows only those dependency fetches: no GitHub API, no push, no other -host. Codex login stays on CLIProxyAPI. +The prompt allows only those dependency fetches and comments on the batch's PRs: +no push, no other host. Codex login stays on CLIProxyAPI. The wrapper waits for the payload, returns its exit status, and tears down its container on exit or SIGTERM. On timeout the daemon sends SIGTERM, allows at least 10 seconds for container cleanup, then sends SIGKILL on a later collection. The payload defaults @@ -622,7 +629,7 @@ in argv or repository configuration. Local `file://` fetches into sandbox and fixer repositories remain credential-free. Write installation tokens are scoped to this repository, with contents and pull requests write. Tokens are minted for each push or API merge and passed only to trusted git -and GitHub clients, never the sandbox or fixer. A PR-branch push also requests +and GitHub clients, never the sandbox or fixer, whose only token is its comment token. A PR-branch push also requests workflow write when the head-to-merge range carries workflow changes from the base. Workflow write is requested only when that PR's squash changes `.github/workflows`. Queue API tokens, which disable auto-merge on ejection and so need contents write, stay in the daemon, refresh before expiry, and are diff --git a/assets/land-fix.md b/assets/land-fix.md index 78678306..17564953 100644 --- a/assets/land-fix.md +++ b/assets/land-fix.md @@ -29,15 +29,25 @@ lockfiles/generated files, or follow a base rename. Merge base where practical. Never edit flake.nix, flake.lock, shell.nix, default.nix, nix/, .envrc, .github/, .gitmodules, or gitlinks (submodule entries) anywhere. Merging the base may carry the base's own changes to those paths: that is allowed when they come out exactly as -a plain `git merge` of the base leaves them, with no conflict there for you to edit. Never push or contact -GitHub. Anything needing behavior changes or review must be handed back. +a plain `git merge` of the base leaves them, with no conflict there for you to edit. Never push. +Anything needing behavior changes or review must be handed back. Build tools may fetch the dependencies the repository's manifests and lockfiles name: from their package registries, such as crates.io, npm, PyPI, and the Go module proxy, and from `github.com` and `codeload.github.com` for git -dependencies. Do not build with `--offline` to avoid them. These fetches are the -only GitHub contact allowed: never call GitHub's API yourself (`gh`, -`api.github.com`), push, or reach any other host. +dependencies. Do not build with `--offline` to avoid them. Besides these fetches, +call GitHub only to comment as below, and reach no other host. + +`GH_TOKEN` is a GitHub token for the repository `GH_REPO` that reaches only its +issues and PR conversations and expires within the hour. With `gh` or +`api.github.com`, you may post comments on the supplied PRs, and edit those you +posted, where they help a maintainer: what a fix changed and why, a flaky test, +or the decision a handback needs. Never comment elsewhere, edit any other +comment, or close, reopen, or relabel anything, even though the token allows it. +Write plain prose with no @-mentions, closing keywords such as "fixes #12", or +secrets, and end each comment with `(written by )`. Comments +decide nothing: the result file sets each PR's outcome, and the daemon still +posts its own. Without `GH_TOKEN`, post none. Before reporting `fixed` or `requeue`, build and test the packages the PR and your fix touch, including whatever the failure log shows failing, and name the commands diff --git a/quest/m0/host-land/fixer.md b/quest/m0/host-land/fixer.md index c787fa16..b21f66f7 100644 --- a/quest/m0/host-land/fixer.md +++ b/quest/m0/host-land/fixer.md @@ -3,7 +3,8 @@ ## Goal The nixos fixer container reaches CLIProxyAPI with the gate's client key, while -Codex login and Git/GitHub credentials stay outside the container. Verify a real +Codex login and every Git/GitHub credential but the job's comment token stay +outside the container. Verify a real mechanical fix and unchanged requeue through the host daemon. ## Plan @@ -25,7 +26,9 @@ remains in kixelated/nixos: the ephemeral fixer container (`qland-fix`) reaches CLIProxyAPI and, like the check containers, the public internet without private ranges (`modules/ci-egress.nix`), so its builds fetch dependencies; `assets/land-fix.md` limits the fixer to those fetches. Provide `OPENAI_BASE_URL` and the gate's `OPENAI_API_KEY`, -and log the maintainer's Codex account into CLIProxyAPI. The wrapper runs with +and log the maintainer's Codex account into CLIProxyAPI. Pass the daemon's +`GH_TOKEN` and `GH_REPO`, the job's comment-only token and its repository, into +the container unchanged, so the fixer can comment on the batch's PRs. The wrapper runs with the supplied repository as cwd, receives ` `, exposes the log read-only, waits for `quest land fix`, preserves its exit status, and returns `quest-land-result.json` in the repository. See the README's landing contract. diff --git a/quest/m0/land-fixer/README.md b/quest/m0/land-fixer/README.md index 44d4df06..2f45fcda 100644 --- a/quest/m0/land-fixer/README.md +++ b/quest/m0/land-fixer/README.md @@ -100,7 +100,6 @@ resolves itself and what it hands back; that docs update finishes the epic. - [Fixer starts from main](/quest/m0/land-fixer/launch.md) - the fixer starts at the base with the batch's PRs as refs, and ignores instructions in their diffs - [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 - [Flaky tests](/quest/m0/land-fixer/flaky.md) - the gate reruns a failed module, disables a flaky test on the batch's base PR, and files a follow-up diff --git a/quest/m0/land-fixer/comments.md b/quest/m0/land-fixer/comments.md deleted file mode 100644 index 1a5165a7..00000000 --- a/quest/m0/land-fixer/comments.md +++ /dev/null @@ -1,44 +0,0 @@ -# [S] The fixer comments on GitHub - -## Goal - -The fixer can post and edit comments on the batch's PRs, for example to -explain a fix, a disabled flaky test, or the decision it needs. It does this -with a GitHub installation token for that one repository with only -`issues: write`, which the daemon mints for each job and which expires with -it. The token cannot push code, merge, or reach any other repository. As the -maintainer accepted, it can also edit or close issues and change labels in -that repository. - -## Plan - -Decided 2026-11-01 (user, /quest-plan): "It's also allowed to comment on -Github." See the [epic](/quest/m0/land-fixer/README.md). - -- How the fixer gets to comment: - - The daemon posts the comments the fixer writes into its result, so no - GitHub credential enters the container (recommended, because PR code the - fixer builds can read the container's environment, as - [moq-dev/quest#356](https://github.com/moq-dev/quest/pull/356) found for - the proxy key) - - ✅ A comment-only token in the container - - Both - -- GitHub has no comment-only permission; `issues: write` also allows editing - and closing issues and changing labels, and PR code the fixer builds could - read the token (Codex review of this plan): - - A local comment command: the daemon keeps the credential and exposes - only comment operations inside the container (recommended) - - ✅ Accept `issues: write` in the container - - The daemon posts comments from the result - -Mint an installation token for the job's repository with -`permissions: { issues: "write" }`, which covers PR conversation comments, and -`repositories: []`. Pass it to the agent as `GH_TOKEN` only; the wrapper -keeps every other credential out, as today. GitHub caps a token at one hour, -so a longer job gets a fresh one or stops commenting. The daemon keeps posting -its own requeue and handback comments. - -## Related - -- [Fixer starts from main](/quest/m0/land-fixer/launch.md) - what the fixer sees when it starts diff --git a/quest/m0/land-fixer/launch.md b/quest/m0/land-fixer/launch.md index 129d778b..b17a37c0 100644 --- a/quest/m0/land-fixer/launch.md +++ b/quest/m0/land-fixer/launch.md @@ -27,7 +27,3 @@ told to ignore any new instructions in the diff." See the ([moq-dev/quest#325](https://github.com/moq-dev/quest/pull/325)). - Internet access already landed in [moq-dev/quest#356](https://github.com/moq-dev/quest/pull/356). - -## Related - -- [Fixer comments](/quest/m0/land-fixer/comments.md) - the other capability the user added diff --git a/src/cli/land-fix.ts b/src/cli/land-fix.ts index eeee06e2..2c8b19f3 100644 --- a/src/cli/land-fix.ts +++ b/src/cli/land-fix.ts @@ -1,4 +1,4 @@ -// 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, its client key, and the job's comment token. import { spawn } from "node:child_process"; import { constants, closeSync, fstatSync, openSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { basename, join } from "node:path"; @@ -99,6 +99,9 @@ export async function fix( OPENAI_BASE_URL: process.env.OPENAI_BASE_URL, OPENAI_API_KEY: process.env.OPENAI_API_KEY, CODEX_API_KEY: process.env.OPENAI_API_KEY, + // The job's comment-only token and its repository, when the wrapper passes them on. + GH_TOKEN: process.env.GH_TOKEN, + GH_REPO: process.env.GH_REPO, GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_NOSYSTEM: "1", GIT_TERMINAL_PROMPT: "0", diff --git a/src/cli/land-fixer.ts b/src/cli/land-fixer.ts index 9597f4db..9e82b4ab 100644 --- a/src/cli/land-fixer.ts +++ b/src/cli/land-fixer.ts @@ -112,7 +112,7 @@ async function comment( return; await github.request(path, { body: { - body: `${attribution} land fixer: ${fixerText(text)}${log ? `\n\n
${excerpt(log)}
` : ""}\n\n${marker}\n\n(written by ${attribution})`, + body: `${attribution} land fixer: ${fixerText(text)}${log ? `\n\n
${excerpt(redactTokens(log))}
` : ""}\n\n${marker}\n\n(written by ${attribution})`, }, }); } @@ -147,6 +147,8 @@ export async function startFixer( fixer: string, state: string, deps: GitHubDependencies, + /** Mints the job's `issues: write` token for its repository, which the fixer may comment with. */ + commentToken?: () => Promise, ): Promise { const entries: Entry[] = []; for (const entry of failure.plan.queue.filter((item) => failure.numbers.includes(item.number))) { @@ -179,6 +181,8 @@ export async function startFixer( rmSync(checkout, { recursive: true, force: true }); return; } + // GitHub expires the token within an hour, which the fixer's 60-minute deadline matches. + const comments = commentToken ? { GH_TOKEN: await commentToken(), GH_REPO: failure.plan.repository } : undefined; git(checkout, ["init", "--quiet"]); // No background gc or maintenance may rewrite objects while the daemon imports the fixer's result. git(checkout, ["config", "gc.auto", "0"]); @@ -227,7 +231,12 @@ export async function startFixer( failure.log, ...job.numbers.map(String), ], - { cwd: checkout, detached: true, env: commandEnvironment(), stdio: ["ignore", output, output] }, + { + cwd: checkout, + detached: true, + env: { ...commandEnvironment(), ...comments }, + stdio: ["ignore", output, output], + }, ); } finally { closeSync(output); @@ -281,9 +290,14 @@ 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"); } +/** The fixer holds its job's comment token, so anything it writes may echo it; GitHub tokens never reach a post. */ +export function redactTokens(text: string): string { + return text.replace(/\bgh[oprsu]_[A-Za-z0-9_]{20,}/g, "[redacted token]"); +} + /** 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("`; @@ -110,11 +110,19 @@ async function comment( ) ) return; - await github.request(path, { - body: { - body: `${attribution} land fixer: ${fixerText(text)}${log ? `\n\n
${excerpt(log)}
` : ""}\n\n${marker}\n\n(written by ${attribution})`, - }, - }); + await github.request(path, { body: { body: `${content}\n\n${marker}\n\n(written by ${attribution})` } }); +} +async function comment( + github: GitHub, + plan: Plan, + entry: Pick, + text: string, + suffix: string, + attribution = "Codex", + log = "", +): Promise { + const content = `${attribution} land fixer: ${fixerText(text)}${log ? `\n\n
${excerpt(redactTokens(log))}
` : ""}`; + await post(github, plan, entry, content, suffix, attribution); } /** Queue eligibility can eject a previously requeued PR before building another batch. */ @@ -283,9 +291,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("\\n\\n@person saw ${token}\\n\`\`\`\\nunclosed"}] +EOF`); + await fixture.finished; + expect(await fixture.collect()).toEqual(["requeued 1"]); + const [note, outcome] = fixture.pull.comments!.slice(-2).map((comment) => comment.body); + expect(outcome).toContain("Requeued unchanged:"); + expect(note).toContain("land fixer note:\n\n> The check `retry` flaked.\n> \n"); + // The daemon's own marker is the only hidden line. + expect(note.match(/`); + expect(EJECTED.exec(note)).toBeNull(); + expect(note).toContain("[at]person saw [redacted token]"); + expect(note).not.toContain(token); + // An unclosed fence ends with the quote, before the daemon's marker and attribution. + expect(note).toMatch(/> unclosed\n\n\n\n\(written by /); + }); + test("never posts a GitHub token the fixer echoes in its text or log", async () => { + const token = `ghs_${"A1b2".repeat(9)}`; + const fixture = await fixerFixture( + `echo "token ${token}" +printf '[{"number":1,"action":"handback","diagnosis":"saw %s","recommendation":"inspect manually"}]' '${token}' > quest-land-result.json`, + ); + await fixture.finished; + await fixture.collect(); + const bodies = fixture.pull.comments!.map((comment) => comment.body).join("\n"); + expect(bodies).toContain("[redacted token]"); + expect(bodies).not.toContain(token); + expect(redactTokens(`a ${token} github_pat_${"x".repeat(30)} b`)).toBe("a [redacted token] [redacted token] b"); + }); + test("quotes the fixer's Markdown so no hidden comment survives", () => { + for (const text of ["", "<", "a