Skip to content

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

Merged
moq-bot[bot] merged 8 commits into
mainfrom
quest/m0/land-fixer/comments
Nov 1, 2026
Merged

moq-bot[bot] merged 8 commits into
mainfrom
quest/m0/land-fixer/comments

Conversation

@kixelated

@kixelated kixelated commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

Implements and deletes quest/m0/land-fixer/comments.md: the land fixer can now comment on the batch's PRs. It writes each comment into its result, and the daemon posts it. No GitHub credential enters the fixer's container.

Summary

  • Result field (src/cli/land-fix.ts): each entry in quest-land-result.json may carry one optional comment. It is Markdown of at most 4000 characters (COMMENT_LIMIT), may span several lines, and may contain no control characters other than tab, CR, and LF. A malformed comment invalidates the result, as any other malformed field does. Its target is the entry's own PR, which parseFixResult already limits to the job's PRs, so there is at most one comment per PR and none outside the job.

  • Posting (src/cli/land-fixer.ts): when the daemon collects a job, it posts each entry's comment on that PR, before its own requeue or handback comment. The comment is headed <agent / model> land fixer note: and quoted line by line (fixerMarkdown):

    • every <!-- is removed, repeating until none is left, so the fixer's text can never carry a hidden marker: not the daemon's quest land fixer marker that its deduplication and moq-dev/quest#358's quest land apply trust, and not the quest land: ejection reason that quest status reads;
    • @ becomes [at], so no one gets mentioned;
    • GitHub tokens (gh[oprsu]_…, github_pat_…) are redacted;
    • the quote also closes any unclosed code fence before the daemon's marker and attribution.

    The daemon adds its own note marker, so a retried collection posts the comment once. comment() is split into a post() that does the deduplication and framing.

  • Redaction kept: token redaction still covers the handback and Review text (fixerText) and the log tail in handback comments. The fixer's text and log go out under the bot's name, so the daemon still redacts tokens they echo.

  • Prompt (assets/land-fix.md): one new paragraph after the result schema. It says the fixer has no GitHub credential and must not seek one. It describes the comment field, says what the daemon strips, and says a comment decides nothing. The existing "Never push or contact GitHub" sentence stays.

  • Docs: the README landing contract and the quest land serve/fix --help describe the field. The land fixer epic records the reversal in its GitHub-comments decision.

  • Tests (test/land.test.ts):

    • an end-to-end fixer job whose comment carries a forged ejection marker, a nested <!<!---- marker, a mention, a token, and an unclosed fence. The test checks the note is posted before the outcome comment, its only <!-- is the daemon's own marker, and the mention and token are defused;
    • redaction of a token the fixer echoes in its diagnosis and log;
    • fixerMarkdown edge cases;
    • parsing that accepts a valid comment and rejects a non-string, an empty or whitespace-only comment, an oversized one, control characters, and extra fields;
    • the prompt paragraph.

    Mutation-checked: a single-pass <!-- strip, dropping the log redaction, and skipping the post each fail them.

just check and just test pass locally on 4004ec3, with CLAUDE_CONFIG_DIR unset. That variable leaks into test/agent.test.ts inside a Claude Code session, which this PR doesn't touch.

Decisions

  • How the fixer gets to comment (maintainer, 2026-11-01, reversing the earlier pick after an independent audit): ✅ the daemon posts them from the result · a token in the container. PR code the fixer builds can read the container's environment, so a token there would let PR code post as moq-bot[bot]. Three things trust moq-bot comments: quest land apply, the daemon's skip when its own marker exists, and quest status's ejection reason. The per-job token minting, the GH_TOKEN/GH_REPO passing, and the prompt paragraph that let the fixer post directly are gone. The host wrapper needs no change, and quest/m0/host-land/fixer.md no longer mentions a comment token.

Choices made without asking

  • The reversal is recorded in the epic, not in comments.md. The brief asked to record it in that quest's Plan. This PR finishes the quest, though, and the PR that finishes a quest deletes the file. So the decision lives in the epic's "GitHub comments" line and here.
  • The comment is a field on each PR's entry, not a separate list. This gives one comment per PR, targets only PRs in the job, and adds no new schema.
  • The cap is 4000 characters, the same as every other result field. Defusing mentions and quoting can at most quadruple that, which stays far under GitHub's 65,536-character limit.
  • A malformed comment rejects the whole result, like any malformed field. The result stays all-or-nothing.
  • The comment is posted whatever the outcome, including when the daemon refuses the fix. The daemon's own handback comment follows it and says why.
  • Mentions become [at], as in fixerText, rather than an invisible zero-width break. Closing keywords and #123 references are left alone, since a comment cannot close anything.
  • The gate's proxy client key is not redacted, because the daemon does not know it. The prompt tells the fixer to include no secrets.
  • Merge of origin/main (moq-dev/quest#366, moq-dev/quest#368): the merge takes main's side of every file, setting aside the token design. The next commit implements the new one.

Follow-ups

  • None. The host wrapper no longer needs any change for comments.

Review

Codex (quest review --new), gpt-6.1-sol, medium effort, session 01a19870-2394-77b1-bfa5-cfed404dd2e8. It was a new session for the design change.

The earlier token design's two rounds (on a13af17 and e7aa2d9) reviewed code this PR no longer contains.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

kixelated and others added 2 commits November 1, 2026 10:00
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 <noreply@anthropic.com>
@kixelated kixelated changed the title The fixer comments on GitHub feat(land): give the fixer a per-job token to comment on the batch's PRs Nov 1, 2026
A failed mint left each PR's attempt recorded with no job, so the retry
handed the PRs back as a repeated pick. Mint first, and test the retry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review November 1, 2026 18:15
@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Important

  • Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 58bfe934-e6be-4207-b58e-7e028d0bce8d
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

The maintainer reversed the comment design on 2026-11-01: no GitHub token
enters the fixer container. This merge takes main's side of every file; the
next commit implements comments posted by the daemon from the fixer's result.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The fixer holds no GitHub credential: PR code it builds can read the
container's environment, so a token there would let PR code post as the bot,
whose comments `quest land apply`, the daemon's deduplication, and
`quest status` trust. Instead each result entry may carry one optional
Markdown `comment` of at most 4000 characters, and the daemon posts it on that
PR when it collects the job, before its own outcome comment. The text is
quoted under the recorded agent, stripped of every HTML comment so it cannot
carry a hidden marker, has its @-mentions defused, and has GitHub tokens
redacted, as do the handback text and log the daemon posts.

Finishes quest/m0/land-fixer/comments.md; the epic records the reversal.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title feat(land): give the fixer a per-job token to comment on the batch's PRs feat(land): post the fixer's comments from its result Nov 1, 2026
@kixelated

Copy link
Copy Markdown
Collaborator Author

Design change: the fixer no longer gets a GitHub token. The maintainer reversed the comment design 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]. Three things trust that identity's comments: quest land apply, the daemon's marker-based deduplication, and quest status's ejection reason.

What changed:

  • The per-job token minting, the GH_TOKEN/GH_REPO passing, and the prompt paragraph that let the fixer post are removed.
  • The fixer now writes at most one Markdown comment per PR into quest-land-result.json.
  • The daemon posts each comment on its PR when it collects the job. It quotes the text, strips HTML comments so no hidden marker can survive, defuses mentions, and redacts tokens.

The review restarted with quest review --new.

(Written by Claude Opus 5.5)

@kixelated
kixelated enabled auto-merge (squash) November 1, 2026 19:02
@moq-bot
moq-bot Bot disabled auto-merge November 1, 2026 19:07
@moq-bot moq-bot Bot removed the quest:merge label Nov 1, 2026
@moq-bot

moq-bot Bot commented Nov 1, 2026

Copy link
Copy Markdown

quest land removed this pull request from the queue: Conflicts with the base: src/cli/land-fixer.ts, src/cli/main.ts
.

kixelated and others added 2 commits November 1, 2026 11:08
# Conflicts:
#	src/cli/land-fixer.ts
#	src/cli/main.ts
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated enabled auto-merge (squash) November 1, 2026 19:21
@moq-bot
moq-bot Bot merged commit 8980aa8 into main Nov 1, 2026
@moq-bot
moq-bot Bot deleted the quest/m0/land-fixer/comments branch November 1, 2026 19:30
@moq-bot

moq-bot Bot commented Nov 1, 2026

Copy link
Copy Markdown

codex / Codex land fixer: PR was picked again while fixing; keeping the newer pick. Recommendation: run /quest-iterate before picking it again.

		"@types/node": "26.6.5",
		"prettier": "3.9.10",
		"typescript": "7.0.2",
		"vitest": "5.0.5"
	},
	"trustedDependencies": []
}
diff --git a/quest/m0/land-fixer/comments.md b/quest/m0/land-fixer/comments.md
deleted file mode 100644
index 23740fc..0000000
--- a/quest/m0/land-fixer/comments.md
+++ /dev/null
@@ -1,40 +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: [<repo>]`. 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.

codex
PR #367 adds instructions in `assets/land-fix.md` telling this fixer to add an optional `"comment"` field to its result. That directly targets the fixer’s result format, so the supplied rules require a handback. I’ll record the instruction and location in the diagnosis and leave the branch unchanged.

exec
/bin/bash -lc "nl -ba assets/land-fix.md | sed -n '50,67p'; git status --short" in /work
 succeeded in 0ms:
    50	{"number":123,"action":"requeue","reason":"healthy; flaky test"}
    51	{"number":123,"action":"fixed","head":"full 40-character SHA","reason":"one-line mechanical reason"}
    52	{"number":123,"action":"handback","diagnosis":"one line","recommendation":"one line"}
    53	
    54	You have no GitHub credential and must not seek one. To tell a PR's maintainer
    55	something, such as what a fix changed and why, a flaky test, or the decision a
    56	handback needs, add an optional `"comment"` field to that PR's entry: Markdown of
    57	at most 4000 characters, one per PR. The daemon posts it on that PR, quoted under
    58	your name, after it collects the result. Write plain prose with no @-mentions,
    59	HTML comments, or secrets: the daemon strips mentions, HTML comments, and GitHub
    60	tokens. A comment decides nothing: the action sets the PR's outcome, and the
    61	daemon still posts its own outcome comment.
    62	
    63	A fixed head must be the tip of pr-<number>, descend from its pinned head, and
    64	contain only that PR's fix and base history. Leave all other PR branches intact.
    65	Keep the result file uncommitted.

exec
/bin/bash -lc "python3 - <<'PY'
import json
result = [{\"number\":367,\"action\":\"handback\",\"diagnosis\":\"PR-added assets/land-fix.md:56-58 addresses this fixer and its result: 'add an optional \\\"comment\\\" field to that PR’s entry', conflicting with the authorized result schema; failure log also reports base conflicts in src/cli/land-fixer.ts and src/cli/main.ts.\",\"recommendation\":\"Have a maintainer review the fixer-prompt/result-schema change separately and resolve the base conflicts before requeueing.\"}]
with open('quest-land-result.json', 'w') as f:
    json.dump(result, f, indent=2)
    f.write('\\n')
PY
 git status --short; git rev-parse pr-367" in /work
 succeeded in 0ms:
?? quest-land-result.json
4004ec3a840069fcc26a02de474218644df20a23

codex
Wrote [quest-land-result.json](/work/quest-land-result.json) uncommitted with a handback for PR #367.

The PR adds instructions targeting this fixer’s result format in `assets/land-fix.md:56`, triggering the required handback. Branches remain unchanged; no build or tests were run.
tokens used
28,158
Wrote [quest-land-result.json](/work/quest-land-result.json) uncommitted with a handback for PR #367.

The PR adds instructions targeting this fixer’s result format in `assets/land-fix.md:56`, triggering the required handback. Branches remain unchanged; no build or tests were run.
fixer result written

(written by codex / Codex)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant