From fc40dbcd0147e1557edd6fb131a02f1cfe364308 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 4 Nov 2026 12:39:45 -0800 Subject: [PATCH 01/11] chore(quest): claim quest/m0/model-routing/implementer.md Quest-Claim: af9bdd5d836cc14a From 999128a34de2cb4ef2d854a52fd410bebf093f45 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 4 Nov 2026 13:14:09 -0800 Subject: [PATCH 02/11] feat: commits and PRs record their models Every agent commit names its harness, model, and effort in Agent-Harness, Agent-Model, and Agent-Effort trailers, and every PR body names its size and each implementing session in a ## Models section beside ## Review. quest context tells agents to add both and where Claude Code, Codex, and Grok expose their model and effort. The land fixer rebuilds the commits its agent made with the trailers, since its Codex never reads quest context, and the kix runner adds them to every message it publishes, from Claude Code's init event and a hook that sees CLAUDE_EFFORT, or from opencode's assistant messages. Agent-Harness: claude Agent-Model: claude-opus-5-5 Agent-Effort: high Co-Authored-By: Claude Opus 5.5 --- assets/CONTEXT.md | 6 ++ quest/m0/model-routing/README.md | 1 - quest/m0/model-routing/implementer.md | 39 ----------- quest/m0/model-routing/pins.md | 1 - quest/m0/model-routing/review-effort.md | 9 ++- src/cli/git.ts | 4 +- src/cli/land-fix.ts | 88 +++++++++++++++++++++++-- src/cli/trailers.ts | 48 ++++++++++++++ src/runner/adapter.ts | 2 + src/runner/claude.ts | 29 +++++++- src/runner/job.ts | 8 ++- src/runner/opencode.ts | 7 +- test/land.test.ts | 40 +++++++++++ test/runner.test.ts | 21 +++++- 14 files changed, 243 insertions(+), 60 deletions(-) delete mode 100644 quest/m0/model-routing/implementer.md create mode 100644 src/cli/trailers.ts diff --git a/assets/CONTEXT.md b/assets/CONTEXT.md index d0b99ced..ff4f73e0 100644 --- a/assets/CONTEXT.md +++ b/assets/CONTEXT.md @@ -108,6 +108,10 @@ A PR that another session pushed to in the last hour is theirs: report its event Before each push to a branch that already has a PR, confirm the PR is still open: a push after it merges brings the deleted branch back. Run the repository's checks locally while the PR is a draft; CI may run only on PRs ready for review. +End every commit message with `Agent-Harness`, `Agent-Model`, and `Agent-Effort` trailers beside any `Co-Authored-By`: your harness (`claude`, `codex`, `grok`, or `opencode`), its model id, and its effort. +Read them rather than guess: Claude Code knows its model and exports `CLAUDE_EFFORT`; Codex records `model` and `effort` in the last `turn_context` of `~/.codex/sessions/*/*/*/rollout-*-$CODEX_THREAD_ID.jsonl`; Grok records `model_id` and `reasoning_effort` in `~/.grok/sessions/*/$GROK_SESSION_ID/chat_history.jsonl`. +Write `unknown` for any you cannot find. + ## Reviews Before a PR is marked ready for review, and before each push to one that is, the other vendor's model reviews the committed branch locally against the PR's base branch with `quest review --base /`: Claude Code reviews Codex's work, and Codex reviews Claude Code's. @@ -127,6 +131,8 @@ Once the reset passes or the login is restored, the item has cleared: whoever fi The PR body's `## Review` section records the latest round: the reviewer harness, model, and effort, the reviewed head SHA, and each finding's disposition (fixed, or refuted with the reason), or the skip and its reason. Rewrite it after each round. A review counts for the head whose SHA it names. +Beside it, a `## Models` section names the PR's size, as `Size: M`, and each implementing session on its own line, as `Implementer: Codex, gpt-6.1-sol, high effort`, which the session adds when it first pushes. +A quest PR's size is its quest's; size any other PR, such as a skill's own, as you would a quest. Once "Ready to Review", CI and any online reviewers run. Address their findings already posted on the head when merging; never wait for or request one. diff --git a/quest/m0/model-routing/README.md b/quest/m0/model-routing/README.md index 132490e0..84b2b2e3 100644 --- a/quest/m0/model-routing/README.md +++ b/quest/m0/model-routing/README.md @@ -81,6 +81,5 @@ are mostly judgment. - [Review by size](/quest/m0/model-routing/review-effort.md) - Sol reviews at an effort set by size, and Opus reviews what Sol wrote - [Review score](/quest/m0/model-routing/review-score.md) - every `quest review` round scores the change out of 10 - [Sizes include judgment](/quest/m0/model-routing/size-judgment.md) - the guide and `/quest-plan` size a quest by the agent's effort, judgment included -- [Commits and PRs record their models](/quest/m0/model-routing/implementer.md) - commit trailers and PR records name each harness, model, and effort, and the PR's size - [Fixes link their culprits](/quest/m0/model-routing/culprits.md) - a fix lists the PR that introduced the problem under `## Caused by` - [moq-dev pins](/quest/m0/model-routing/pins.md) - moq-dev/moq and moq-dev/moq.pro bump their `quest` pin to pick up the routing diff --git a/quest/m0/model-routing/implementer.md b/quest/m0/model-routing/implementer.md deleted file mode 100644 index aba3df09..00000000 --- a/quest/m0/model-routing/implementer.md +++ /dev/null @@ -1,39 +0,0 @@ -# [M] Commits and PRs record their models - -## Goal - -Every commit an agent makes carries its harness, model, and effort in -trailers, and every PR's body names the PR's size and each implementing -session's harness, model, and effort beside `## Review`, which already -names the reviewer's. This covers quest PRs, skills' own PRs such as -`/quest-plan`'s, the land fixer's commits, and kix runner jobs, so the -[model check](/quest/m1/model-check.md) and -[`quest usage`](/quest/m1/usage.md) can audit how each ranking performs. - -## Plan - -Decided 2026-11-04 (user, /quest-plan follow-ups of -[moq-dev/quest#487](https://github.com/moq-dev/quest/pull/487)): record the -model and effort for every commit and PR, to audit the rankings. - -- Commit trailers (agent's choice of names): `Agent-Harness`, - `Agent-Model`, and `Agent-Effort`, one each, beside the - `Co-Authored-By` line a harness already adds. `quest context` tells - agents to add them; the land fixer (`src/cli/land-fix.ts`) and the kix - runner (`src/runner/`) add them in code, since no agent writes those - commit messages. -- The PR record: `quest context`'s Reviews section (`assets/CONTEXT.md`) - already requires the reviewer's harness, model, and effort in - `## Review`. Beside that rule, it says the PR body also names the PR's - size and one line per implementing session, added by each session when - it pushes, so a quest that `/quest-iterate` finishes on another model - lists both. A quest PR's size is its quest's; a skill's own PR is sized - by that skill, as [Review by size](/quest/m0/model-routing/review-effort.md) - uses. -- An agent finds its own settings rather than guessing: Claude Code - exports `CLAUDE_EFFORT` and knows its model. Find where Codex and Grok - expose their model and effort to the agent, and name them in the rule. - When an agent cannot find its effort, it writes `unknown` rather than - leaving the record out. -- Linking the PRs that caused later fixes is - [its own quest](/quest/m0/model-routing/culprits.md). diff --git a/quest/m0/model-routing/pins.md b/quest/m0/model-routing/pins.md index 55bab84b..813a6678 100644 --- a/quest/m0/model-routing/pins.md +++ b/quest/m0/model-routing/pins.md @@ -22,4 +22,3 @@ repository uses the same settings, so nothing in them changes but the pin. - [Route by size](/quest/m0/model-routing/models.md) - `/quest-spawn` reads the built-in size table - [Review by size](/quest/m0/model-routing/review-effort.md) - `quest review` reads the built-in review table - [Sizes include judgment](/quest/m0/model-routing/size-judgment.md) - the guide's size definition the tables rely on -- [PRs record their implementer](/quest/m0/model-routing/implementer.md) - the record that shows the routing working diff --git a/quest/m0/model-routing/review-effort.md b/quest/m0/model-routing/review-effort.md index a7b171ce..8cb475fd 100644 --- a/quest/m0/model-routing/review-effort.md +++ b/quest/m0/model-routing/review-effort.md @@ -41,11 +41,10 @@ earlier ranked lists of Grok, DeepSeek, Sol, and Opus reviewers: first creating it. This replaces the 2026-11-03 efforts (Codex xhigh, Claude high) recorded in the [model check](/quest/m1/model-check.md), which audits the list each milestone; the PR updates its Plan. -- The implementers' models come from the PR body's record - ([PRs record their models](/quest/m0/model-routing/implementer.md)), - passed by the reviewing session as `--exclude-model`; without a record, - the reviewing session's own model is excluded, as "the other vendor" is - today. +- The implementers' models come from the PR body's `## Models` record, + which `quest context`'s Reviews section defines, passed by the reviewing + session as `--exclude-model`; without a record, the reviewing session's + own model is excluded, as "the other vendor" is today. - Where the size comes from, read by the reviewing session, never from the head's files, so a change cannot lower its own review: - A quest branch: its quest's size, read from the base branch's tip, as diff --git a/src/cli/git.ts b/src/cli/git.ts index 1d9fe536..0c9f7217 100644 --- a/src/cli/git.ts +++ b/src/cli/git.ts @@ -33,10 +33,10 @@ export class GitError extends Error { * Git run in the repository at `root`, without prompting, returning its output without trailing newlines. * A `config` lookup that matches nothing is empty rather than an error. */ -export function git(root: string, args: string[], input?: string): string { +export function git(root: string, args: string[], input?: string, env?: NodeJS.ProcessEnv): string { const result = spawnSync("git", ["-C", root, ...args], { encoding: "utf8", - env: { ...discoveryEnv(), GIT_TERMINAL_PROMPT: "0" }, + env: { ...discoveryEnv(), GIT_TERMINAL_PROMPT: "0", ...env }, input, maxBuffer: 1 << 30, stdio: [input === undefined ? "ignore" : "pipe", "pipe", "pipe"], diff --git a/src/cli/land-fix.ts b/src/cli/land-fix.ts index f6a0543b..3a40928b 100644 --- a/src/cli/land-fix.ts +++ b/src/cli/land-fix.ts @@ -4,8 +4,9 @@ import { spawn, spawnSync } from "node:child_process"; import { constants, closeSync, existsSync, fstatSync, openSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { basename, join, resolve } from "node:path"; import prompt from "../../assets/land-fix.md"; -import { discoveryEnv } from "./git"; +import { discoveryEnv, git } from "./git"; import { programEnvironment, requireProgram } from "./program"; +import { type Implementer, withTrailers } from "./trailers"; export class FixerAgentError extends Error { constructor(readonly exitCode: number) { @@ -254,6 +255,9 @@ export async function fix( if (!/^[a-f0-9]{40,64}$/.test(head)) throw new Error(`fix needs a ${ref} branch`); return `- #${number}: branch ${ref} at ${head}`; }); + // Every commit reachable from none of these is the agent's. + const start = git(root, ["for-each-ref", "--format=%(objectname)", "refs/heads"]).split("\n").filter(Boolean); + if (!start.length) throw new Error("fix needs the checkout's branches"); // The agent runs in the PRs' checkout, so neither it nor its commands are found there: a name is // looked up in PATH's absolute directories, and a path is resolved here rather than in the checkout. const program = agent.includes("/") ? resolve(agent) : requireProgram(agent, process.env); @@ -295,6 +299,8 @@ export async function fix( ...(model ? ["--model", model] : []), "-", ]; + // Codex names its model in the header it prints first, whether or not the command line does. + let header = ""; await new Promise((done, reject) => { // Relay the agent's output through pipes of its own rather than letting it inherit ours. Once Bun writes // to a pipe it marks that pipe non-blocking, and an inheriting child shares the flag: Codex then panicked @@ -302,6 +308,9 @@ export async function fix( const child = spawn(program, argv, { cwd: root, env, stdio: ["pipe", "pipe", "pipe"] }); child.stdout.pipe(process.stdout, { end: false }); child.stderr.pipe(process.stderr, { end: false }); + child.stderr.on("data", (chunk: Buffer) => { + if (header.length < 4096) header += chunk.toString("utf8"); + }); child.once("error", reject); child.stdin.on("error", () => {}); child.stdin.end( @@ -310,13 +319,84 @@ export async function fix( child.once("close", (code) => (code === 0 ? done() : reject(new FixerAgentError(code ?? 1)))); }); const parse = (text: string) => (numbers.length ? parseFixResult(text, numbers) : parseBaseFix(text)); - const results = parse(readFixResult(root)); + const parsed = parse(readFixResult(root)); const index = argv.findIndex((arg) => arg === "-m" || arg === "--model"); - const ranModel = index >= 0 ? argv[index + 1] : argv.find((arg) => arg.startsWith("--model="))?.slice(8); + const ranModel = + (index >= 0 ? argv[index + 1] : argv.find((arg) => arg.startsWith("--model="))?.slice(8)) ?? + /^model: ([a-zA-Z0-9._/-]{1,100})$/m.exec(header)?.[1]; + const results = trailed(root, start, Array.isArray(parsed) ? parsed : [parsed], { + harness: basename(agent), + model: ranModel ?? "unknown", + // Replaced arguments may run the agent at any effort. + effort: args ? "unknown" : effort, + }); const identity = { agent: basename(agent), model: ranModel ?? "Codex" }; const result = JSON.stringify( - Array.isArray(results) ? results.map((item) => ({ ...item, ...identity })) : { ...results, ...identity }, + Array.isArray(parsed) ? results.map((item) => ({ ...item, ...identity })) : { ...results[0], ...identity }, ); parse(result); writeFileSync(join(root, FIX_RESULT), result); } + +/** + * `results` once every commit the agent made, reachable from a head it reports but from no branch in + * `start`, carries `by`'s trailers. Each is rebuilt with its tree, parents, authorship, and dates; the + * branches that held one move to its replacement, and the results name the replacements, also where their + * prose abbreviates an old commit. + */ +function trailed( + root: string, + start: string[], + results: (FixResult | BaseFix)[], + by: Implementer, +): (FixResult | BaseFix)[] { + const tips = results.flatMap((item) => [ + ...(item.action === "fixed" ? [item.head] : []), + ...("disable" in item && item.disable ? [item.disable] : []), + ]); + if (!tips.length) return results; + const replaced = new Map(); + const commits = git(root, ["rev-list", "--topo-order", "--reverse", ...tips, "--not", ...start]); + for (const commit of commits.split("\n").filter(Boolean)) { + const format = ["%T", "%P", "%an", "%ae", "%ad", "%cn", "%ce", "%cd"].join("%x00"); + const [tree, parents, ...people] = git(root, ["show", "-s", "--date=raw", `--format=${format}`, commit]).split( + "\0", + ); + const [authorName, authorEmail, authorDate, committerName, committerEmail, committerDate] = people; + const message = withTrailers(git(root, ["show", "-s", "--format=%B", commit]), by); + const args = [ + tree!, + ...parents!.split(" ").flatMap((parent) => (parent ? ["-p", replaced.get(parent) ?? parent] : [])), + ]; + replaced.set( + commit, + git(root, ["commit-tree", ...args], message, { + GIT_AUTHOR_NAME: authorName, + GIT_AUTHOR_EMAIL: authorEmail, + GIT_AUTHOR_DATE: `@${authorDate}`, + GIT_COMMITTER_NAME: committerName, + GIT_COMMITTER_EMAIL: committerEmail, + GIT_COMMITTER_DATE: `@${committerDate}`, + }), + ); + } + for (const line of git(root, ["for-each-ref", "--format=%(objectname) %(refname)", "refs/heads"]).split("\n")) { + const [sha, ref] = line.split(" "); + const next = sha && replaced.get(sha); + if (next) git(root, ["update-ref", ref!, next, sha]); + } + const commit = (sha: string) => replaced.get(sha) ?? sha; + const prose = (text: string) => + text.replace(/\b[0-9a-f]{7,40}\b/g, (short) => { + for (const [old, next] of replaced) if (old.startsWith(short)) return next.slice(0, short.length); + return short; + }); + return results.map((item) => { + const copy: Record = { ...item }; + for (const key of ["reason", "comment", "diagnosis", "recommendation"]) + if (typeof copy[key] === "string") copy[key] = prose(copy[key]); + if (item.action === "fixed") copy.head = commit(item.head); + if ("disable" in item && item.disable) copy.disable = commit(item.disable); + return copy as FixResult | BaseFix; + }); +} diff --git a/src/cli/trailers.ts b/src/cli/trailers.ts new file mode 100644 index 00000000..a99f3bf1 --- /dev/null +++ b/src/cli/trailers.ts @@ -0,0 +1,48 @@ +// The trailers that name the harness, model, and effort behind a commit, so audits can tell which of +// them made it. Agents add their own as `quest context` says; code that commits for an agent adds them here. +import { spawnSync } from "node:child_process"; + +import { discoveryEnv } from "./git"; + +/** The agent session that wrote a commit. */ +export interface Implementer { + harness: string; + model: string; + effort: string; +} + +/** `value` when it fits on one trailer line, otherwise `unknown`. */ +const field = (value: string) => (/^[\w.:/@+-]{1,100}$/.test(value) ? value : "unknown"); + +/** + * `message` ending with `by`'s trailers, replacing any it already gives, by Git's own trailer rules so + * they join an existing block such as `Co-Authored-By`. + */ +export function withTrailers(message: string, by: Implementer): string { + const result = spawnSync( + "git", + [ + "interpret-trailers", + // A `---` line in the message is prose, not the start of a patch. + "--no-divider", + "--if-exists", + "replace", + "--trailer", + `Agent-Harness: ${field(by.harness)}`, + "--trailer", + `Agent-Model: ${field(by.model)}`, + "--trailer", + `Agent-Effort: ${field(by.effort)}`, + ], + { + // Outside any repository, with no user or system config, so no `trailer.*` setting reshapes them. + cwd: "/", + encoding: "utf8", + env: { ...discoveryEnv(), GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_NOSYSTEM: "1" }, + input: message, + }, + ); + if (result.error) throw result.error; + if (result.status !== 0) throw new Error(`git interpret-trailers failed: ${result.stderr.trim()}`); + return result.stdout; +} diff --git a/src/runner/adapter.ts b/src/runner/adapter.ts index ceca968b..19eb2d73 100644 --- a/src/runner/adapter.ts +++ b/src/runner/adapter.ts @@ -69,6 +69,8 @@ export interface Running { done: Promise; /** Ask the CLI to stop, killing it after a grace period. */ stop(): void; + /** The model and effort the session last ran on, each `unknown` until the CLI shows it. */ + ran(): { model: string; effort: string }; } export interface SignedIn { diff --git a/src/runner/claude.ts b/src/runner/claude.ts index b04b933d..a12569c8 100644 --- a/src/runner/claude.ts +++ b/src/runner/claude.ts @@ -9,7 +9,7 @@ // runner's settings apply: the checkout's own settings could add hooks that // run outside the sandbox, so neither they nor its CLAUDE.md are loaded. import { execFile, spawn } from "node:child_process"; -import { rmSync, writeFileSync } from "node:fs"; +import { readFileSync, rmSync, writeFileSync } from "node:fs"; import { constants } from "node:os"; import { join } from "node:path"; import { promisify } from "node:util"; @@ -59,8 +59,15 @@ export function jobEnvironment(env: NodeJS.ProcessEnv, home: string, sandbox: Sa * only the checkout and read only the checkout (outside it Claude Code would * ask), and WebFetch, which would run outside the sandbox, is off. */ -export function claudeSettings(sandbox: Sandbox): Record { +export function claudeSettings(sandbox: Sandbox, effort: string): Record { + // Claude Code shows its effort only to hooks and commands, as CLAUDE_EFFORT, and only within a turn. + // Hooks run outside the sandbox, so `effort` is in the account's home, which commands cannot reach. + const record = { type: "command", command: `printf %s "$CLAUDE_EFFORT" > '${effort.replaceAll("'", "'\\''")}'` }; return { + hooks: { + PreToolUse: [{ matcher: "mcp__kix__.*", hooks: [record] }], + Stop: [{ hooks: [record] }], + }, sandbox: { enabled: true, failIfUnavailable: true, @@ -102,11 +109,16 @@ const LIMIT = /usage limit|limit reached|hit your (?:usage )?limit|out of extra export function streamReader() { let result: Record | undefined; let limited: { until: number | null } | null = null; + let model = "unknown"; return { + /** The model the session started on. */ + model: () => model, line(line: string) { const event = json(line); if (!event) return; if (event.type === "result") result = event; + if (event.type === "system" && event.subtype === "init" && typeof event.model === "string") + model = event.model; if (event.type === "rate_limit_event") { const info = event.rate_limit_info as Record | undefined; const resets = typeof info?.resetsAt === "number" ? info.resetsAt * 1000 : null; @@ -186,10 +198,20 @@ export class Claude implements Adapter { const config = join(launch.home, `kix-tools-${launch.session}.json`); writeFileSync(config, JSON.stringify(mcp), { mode: 0o600 }); const settings = join(launch.home, `kix-settings-${launch.session}.json`); - writeFileSync(settings, JSON.stringify(claudeSettings(launch.sandbox)), { mode: 0o600 }); + const effortFile = join(launch.home, `kix-effort-${launch.session}`); + writeFileSync(settings, JSON.stringify(claudeSettings(launch.sandbox, effortFile)), { mode: 0o600 }); + let effort = "unknown"; + const readEffort = () => { + try { + effort = readFileSync(effortFile, "utf8").trim() || effort; + } catch {} + return effort; + }; const cleanup = () => { + readEffort(); rmSync(config, { force: true }); rmSync(settings, { force: true }); + rmSync(effortFile, { force: true }); }; const argv = [ "-p", @@ -248,6 +270,7 @@ export class Claude implements Adapter { }); return { done, + ran: () => ({ model: reader.model(), effort: readEffort() }), stop() { signal("SIGTERM"); killer ??= setTimeout(() => signal("SIGKILL"), STOP_GRACE_MS); diff --git a/src/runner/job.ts b/src/runner/job.ts index 89ec8186..ad06ba69 100644 --- a/src/runner/job.ts +++ b/src/runner/job.ts @@ -6,6 +6,7 @@ import { randomUUID } from "node:crypto"; import { type Round, ResponsesError } from "../cli/review"; +import { withTrailers } from "../cli/trailers"; import type { Adapter, Exit, Running } from "./adapter"; import { Refused } from "./connection"; @@ -115,6 +116,8 @@ export class Attempt { /** Settles once no review runs, so the attempt never removes a checkout a review still uses. */ #reviewRun: Promise = Promise.resolve(); #agent: Running | undefined; + /** The CLI the attempt's account runs, which its commits name. */ + #harness = "unknown"; /** Aborts the devShell build, which runs before there is an agent to stop. */ #preparing = new AbortController(); #publishing: Promise = Promise.resolve(); @@ -209,6 +212,7 @@ export class Attempt { return await this.#finish("failed", `The account ${job.account} is no longer on this runner.`); // Before any checkout: an account added while the runner runs may lack its CLI or sandbox. const adapter = ctx.adapter(account); + this.#harness = adapter.name; this.#account = account; this.#workspace = await Workspace.prepare(ctx.root, job, ctx.remote(job)); if (this.#workspace.preserved) @@ -382,13 +386,15 @@ export class Attempt { const idle = () => !snapshot.changes.length && !snapshot.merge && !pull && !land; if (idle()) return { head: workspace.head }; + const ran = this.#agent?.ran() ?? { model: "unknown", effort: "unknown" }; + const commit = withTrailers(message, { harness: this.#harness, ...ran }); const send = () => this.ctx.publish({ job: this.job.id, attempt: this.job.attempt, head: workspace.head, session: this.#session, - ...((snapshot.changes.length || snapshot.merge) && { message }), + ...((snapshot.changes.length || snapshot.merge) && { message: commit }), changes: snapshot.changes, ...(snapshot.merge && { merge: snapshot.merge }), // kix verifies these pointers against the existing head before carrying them into the merge tree. diff --git a/src/runner/opencode.ts b/src/runner/opencode.ts index 4cf26203..47aee986 100644 --- a/src/runner/opencode.ts +++ b/src/runner/opencode.ts @@ -474,6 +474,7 @@ export class Opencode implements Adapter { void closed.then(() => listening(null)); let stopping = false; + let ran = { model: "unknown", effort: "unknown" }; let killer: ReturnType | undefined; let abort: () => Promise = async () => {}; let watched: Promise = Promise.resolve(); @@ -519,7 +520,10 @@ export class Opencode implements Adapter { // Token deltas repeat what the finished parts carry; other sessions' events are not this job's. if (properties.sessionID !== session || event.type.endsWith(".delta")) continue; launch.output(`${JSON.stringify(event)}\n`); - if (event.type === "session.status") { + if (event.type === "message.updated" && event.properties.info.role === "assistant") { + const { providerID, modelID, variant } = event.properties.info; + ran = { model: `${providerID}/${modelID}`, effort: variant ?? "unknown" }; + } else if (event.type === "session.status") { const hit = retryLimit(event.properties.status); if (hit && !limited) { limited = hit; @@ -561,6 +565,7 @@ export class Opencode implements Adapter { }); return { done, + ran: () => ran, stop() { stopping = true; // Abort the turn so opencode records it, then stop the server whether or not it answered. diff --git a/test/land.test.ts b/test/land.test.ts index 42a884ec..bfcfab4e 100644 --- a/test/land.test.ts +++ b/test/land.test.ts @@ -3687,6 +3687,46 @@ printf '[{"number":1,"action":"handback","diagnosis":"saw %s","recommendation":" expect(result.model).toBe("actual-model"); }); + test("gives the agent's new commits its trailers, moving their branches and the SHAs its result names", async () => { + const dir = fixRepository(); + const start = git(dir, "rev-parse", "pr-1"); + const log = join(dir, "failure"); + writeFileSync(log, "failure"); + const agent = join(dir, "codex"); + // Codex prints its model in the header on stderr; the agent's own trailers stay beside quest's. + writeFileSync( + agent, + `#!/bin/sh +set -eu +cat >/dev/null +printf 'model: header-model\\nreasoning effort: xhigh\\n' >&2 +git checkout -q pr-1 +printf 'fixed\\n' > code.txt +git add code.txt +git -c user.name=Agent -c user.email=a@a -c commit.gpgsign=false commit -qm 'fix: thing' -m 'Co-Authored-By: Someone ' +head=$(git rev-parse HEAD) +short=$(git rev-parse --short=10 HEAD) +printf '[{"number":1,"action":"fixed","head":"%s","reason":"committed %s"}]' "$head" "$short" > quest-land-result.json +`, + { mode: 0o755 }, + ); + await fix(dir, log, [1], agent, ["-"]); + const [result] = parseFixResult(readFileSync(join(dir, "quest-land-result.json"), "utf8"), [1]); + const head = git(dir, "rev-parse", "pr-1"); + expect(result).toMatchObject({ + action: "fixed", + head, + reason: `committed ${head.slice(0, 10)}`, + model: "header-model", + }); + expect(git(dir, "rev-parse", `${head}^`)).toBe(start); + expect(git(dir, "show", "-s", "--format=%an %B", head)).toBe( + "Agent fix: thing\n\nCo-Authored-By: Someone \nAgent-Harness: codex\nAgent-Model: header-model\nAgent-Effort: unknown", + ); + // The branch tips the agent started from keep their commits. + expect(git(dir, "show", "-s", "--format=%B", start)).toBe("head"); + }); + test("rejects mixing individually approved protected files", async () => { const forge = new Forge(); forge.advance({ "flake.nix": "old", "shell.nix": "old" }); diff --git a/test/runner.test.ts b/test/runner.test.ts index d0c24858..7bb94967 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -1334,7 +1334,14 @@ log({ timeout: kix.timeout, }); const out = (value) => process.stdout.write(JSON.stringify(value) + "\\n"); -out({ type: "system", subtype: "init", session_id: session }); +out({ type: "system", subtype: "init", session_id: session, model: "claude-test" }); +// Claude Code runs the settings' hooks for a tool, with the turn's effort, before calling it. +const hooks = (tool) => { + for (const entry of JSON.parse(fs.readFileSync(at("--settings"), "utf8")).hooks?.PreToolUse ?? []) + if (new RegExp("^(?:" + entry.matcher + ")$").test("mcp__kix__" + tool)) + for (const hook of entry.hooks) + require("node:child_process").execSync(hook.command, { env: { ...process.env, CLAUDE_EFFORT: "high" } }); +}; let id = 0; async function rpc(method, params) { const response = await fetch(kix.url, { @@ -1357,6 +1364,7 @@ async function rpc(method, params) { await rpc("tools/call", { name: "context", arguments: {} }); log({ backgroundWaitPid: process.pid }); } + if (step.tool) hooks(step.tool); if (step.tool) log({ tool: step.tool, reply: await rpc("tools/call", { name: step.tool, arguments: step.args }) }); if (step.wait) { process.on("SIGTERM", () => { log({ terminated: true }); process.exit(143); }); @@ -1419,6 +1427,7 @@ async function prompt(session, input) { if (step.shell) log({ shell: require("node:child_process").execFileSync(config.shell, ["-c", step.shell], { encoding: "utf8" }) }); if (step.emit) send({ ...step.emit, properties: { ...step.emit.properties, sessionID: session } }); + if (step.tool) hooks(step.tool); if (step.tool) log({ tool: step.tool, reply: await rpc("tools/call", { name: step.tool, arguments: step.args }) }); if (step.wait) { await new Promise((resolve) => (aborted = resolve)); @@ -1783,7 +1792,13 @@ describe("quest runner", () => { expect(r.hub.of("status").some((s) => s.attempts.length === 1 && s.slots === 1)).toBe(true); expect(r.hub.of("checkpoint")).toMatchObject([{ summary: "Wrote a.txt", session: expect.any(String) }]); const [checkpoint, delivery] = r.publishes; - expect(checkpoint).toMatchObject({ job: "job1", attempt: 1, head: null, message: "Wrote a.txt" }); + // The runner names the agent behind each commit from the CLI's own report of its model and effort. + expect(checkpoint).toMatchObject({ + job: "job1", + attempt: 1, + head: null, + message: "Wrote a.txt\n\nAgent-Harness: claude\nAgent-Model: claude-test\nAgent-Effort: high\n", + }); expect(checkpoint!.changes).toEqual([{ path: "a.txt", content: Buffer.from("first\n").toString("base64") }]); expect(delivery).toMatchObject({ head: "0".repeat(39) + "1", @@ -2213,7 +2228,7 @@ describe("quest runner", () => { posix("continues from the head kix reports after a publication's reply was lost", async () => { let landed = ""; const r = rig((body, source) => { - if (body.message === "First") { + if (body.message.startsWith("First\n")) { // kix commits the checkpoint, but its reply never arrives. for (const change of body.changes) writeFileSync(join(source.dir, change.path), Buffer.from(change.content, "base64")); From 4e2275ed9a2cbbeb12c93f5deea1fbf4bdd8c2c4 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 4 Nov 2026 13:34:44 -0800 Subject: [PATCH 03/11] fix: cover opencode's trailers and build on the ## Models record The fake opencode no longer runs the fake Claude's hooks, and its test checks the trailers from opencode's assistant message. quest context keeps the ## Models definition moq-dev/quest#493 adds and settles only its line format. Agent-Harness: claude Agent-Model: claude-opus-5-5 Agent-Effort: high Co-Authored-By: Claude Opus 5.5 --- assets/CONTEXT.md | 5 +++-- test/runner.test.ts | 7 ++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/assets/CONTEXT.md b/assets/CONTEXT.md index ff4f73e0..317c19a6 100644 --- a/assets/CONTEXT.md +++ b/assets/CONTEXT.md @@ -128,11 +128,12 @@ A reviewer that is out of quota or logged out is an outage, not a skip: keep the Give the reset time when the error names one; a logged-out reviewer waits on `Maintainer:`, with the login command `quest review` prints. Once the reset passes or the login is restored, the item has cleared: whoever finds it runs `/quest-review` on the PR. +The PR body's `## Models` section records the PR's size and each implementing session's harness, model, and effort. +A quest PR's size is its quest's; a skill that opens its own PR, such as `/quest-plan`, sizes it as a quest is sized. +List them as `Size: M` and one `Implementer: Codex, gpt-6.1-sol, high effort` line per session, which each session adds when it first pushes. The PR body's `## Review` section records the latest round: the reviewer harness, model, and effort, the reviewed head SHA, and each finding's disposition (fixed, or refuted with the reason), or the skip and its reason. Rewrite it after each round. A review counts for the head whose SHA it names. -Beside it, a `## Models` section names the PR's size, as `Size: M`, and each implementing session on its own line, as `Implementer: Codex, gpt-6.1-sol, high effort`, which the session adds when it first pushes. -A quest PR's size is its quest's; size any other PR, such as a skill's own, as you would a quest. Once "Ready to Review", CI and any online reviewers run. Address their findings already posted on the head when merging; never wait for or request one. diff --git a/test/runner.test.ts b/test/runner.test.ts index 7bb94967..a4af00f8 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -1427,7 +1427,6 @@ async function prompt(session, input) { if (step.shell) log({ shell: require("node:child_process").execFileSync(config.shell, ["-c", step.shell], { encoding: "utf8" }) }); if (step.emit) send({ ...step.emit, properties: { ...step.emit.properties, sessionID: session } }); - if (step.tool) hooks(step.tool); if (step.tool) log({ tool: step.tool, reply: await rpc("tools/call", { name: step.tool, arguments: step.args }) }); if (step.wait) { await new Promise((resolve) => (aborted = resolve)); @@ -1940,8 +1939,10 @@ describe("quest runner", () => { posix("drives opencode serve through a question and a resume in the same session", async () => { const r = rig(undefined, "opencode"); + const info = { role: "assistant", providerID: "openai", modelID: "gpt-test", variant: "high" }; r.steps({ start: [ + { emit: { type: "message.updated", properties: { info } } }, { write: "a.txt", content: "draft\n" }, { shell: 'printf "%s|%s|%s|%s|%s" "$IN_DEVSHELL" "$HOME" "${OPENCODE_SERVER_PASSWORD:-none}" "${QUEST_COMMAND:-none}" "$TMPDIR"', @@ -1958,6 +1959,10 @@ describe("quest runner", () => { const question = await r.hub.next((m) => m.type === "question"); await idle(r.runner); expect(r.hub.of("checkpoint")).toMatchObject([{ summary: "Drafted", session: question.session }]); + // The commit names the model and effort opencode's assistant message reported. + expect(r.publishes[0]!.message).toBe( + "Drafted\n\nAgent-Harness: opencode\nAgent-Model: openai/gpt-test\nAgent-Effort: high\n", + ); const [nix, first, srt, shell] = r.calls(); expect(nix.nix.slice(0, 3)).toEqual(["print-dev-env", "--accept-flake-config", "--no-write-lock-file"]); const home = r.home.accountHome("acct1"); From bc6764609c8d11e37908746b457251307824caea Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 4 Nov 2026 13:48:52 -0800 Subject: [PATCH 04/11] fix(trailers): replace every repeat of an agent trailer git interpret-trailers --if-exists replace replaces only the last of a key's repeats, so drop every Agent-* line from the closing paragraph before adding the authoritative values. Agent-Harness: claude Agent-Model: claude-opus-5-5 Agent-Effort: high Co-Authored-By: Claude Opus 5.5 --- src/cli/trailers.ts | 17 ++++++++++++----- test/trailers.test.ts | 22 ++++++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) create mode 100644 test/trailers.test.ts diff --git a/src/cli/trailers.ts b/src/cli/trailers.ts index a99f3bf1..f9747205 100644 --- a/src/cli/trailers.ts +++ b/src/cli/trailers.ts @@ -15,18 +15,25 @@ export interface Implementer { const field = (value: string) => (/^[\w.:/@+-]{1,100}$/.test(value) ? value : "unknown"); /** - * `message` ending with `by`'s trailers, replacing any it already gives, by Git's own trailer rules so - * they join an existing block such as `Co-Authored-By`. + * `message` ending with `by`'s trailers, which replace every one it already gives, by Git's own trailer + * rules so they join an existing block such as `Co-Authored-By`. */ export function withTrailers(message: string, by: Implementer): string { + // Git replaces only the last of a key's repeats, so drop them all from the closing paragraph first. + const paragraphs = message.trimEnd().split(/\n[ \t]*\n/); + if (paragraphs.length > 1) { + const kept = paragraphs + .pop()! + .split("\n") + .filter((line) => !/^Agent-(?:Harness|Model|Effort)\s*:/i.test(line)); + if (kept.length) paragraphs.push(kept.join("\n")); + } const result = spawnSync( "git", [ "interpret-trailers", // A `---` line in the message is prose, not the start of a patch. "--no-divider", - "--if-exists", - "replace", "--trailer", `Agent-Harness: ${field(by.harness)}`, "--trailer", @@ -39,7 +46,7 @@ export function withTrailers(message: string, by: Implementer): string { cwd: "/", encoding: "utf8", env: { ...discoveryEnv(), GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_NOSYSTEM: "1" }, - input: message, + input: `${paragraphs.join("\n\n")}\n`, }, ); if (result.error) throw result.error; diff --git a/test/trailers.test.ts b/test/trailers.test.ts new file mode 100644 index 00000000..a81584fb --- /dev/null +++ b/test/trailers.test.ts @@ -0,0 +1,22 @@ +import { expect, test } from "vitest"; + +import { withTrailers } from "../src/cli/trailers"; + +const by = { harness: "codex", model: "gpt-test", effort: "high" }; +const trailers = "Agent-Harness: codex\nAgent-Model: gpt-test\nAgent-Effort: high\n"; + +test("joins an existing trailer block and replaces every repeat of its own keys", () => { + expect( + withTrailers( + "fix: thing\n\nBody\n---\nmore\n\nagent-model: old\nCo-Authored-By: X \nAgent-Model: stale\nAgent-Effort: low\n", + by, + ), + ).toBe(`fix: thing\n\nBody\n---\nmore\n\nCo-Authored-By: X \n${trailers}`); +}); + +test("starts a trailer block under a lone subject, and writes unknown for a value that cannot be one", () => { + expect(withTrailers("Agent-Model: subject", by)).toBe(`Agent-Model: subject\n\n${trailers}`); + expect(withTrailers("subject\n\nAgent-Harness: codex\n", { ...by, model: "two words\nAgent-Model: x" })).toBe( + "subject\n\nAgent-Harness: codex\nAgent-Model: unknown\nAgent-Effort: high\n", + ); +}); From 1a8dcd4b43d1f99b728892d52b474d2f1d125ce0 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 4 Nov 2026 14:01:30 -0800 Subject: [PATCH 05/11] fix(trailers): strip old agent trailers only from Git's trailer block Normalize CRLF, skip trailing comment paragraphs, and strip Agent-* lines only when git interpret-trailers reads them as trailers, so lookalike lines in a body paragraph survive. Agent-Harness: claude Agent-Model: claude-opus-5-5 Agent-Effort: high Co-Authored-By: Claude Opus 5.5 --- src/cli/trailers.ts | 55 +++++++++++++++++++++++++------------------ test/trailers.test.ts | 8 +++++++ 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/src/cli/trailers.ts b/src/cli/trailers.ts index f9747205..7ae46da1 100644 --- a/src/cli/trailers.ts +++ b/src/cli/trailers.ts @@ -14,24 +14,42 @@ export interface Implementer { /** `value` when it fits on one trailer line, otherwise `unknown`. */ const field = (value: string) => (/^[\w.:/@+-]{1,100}$/.test(value) ? value : "unknown"); +/** `git interpret-trailers` on `input`, outside any repository and with no user or system config, so no `trailer.*` setting reshapes it. */ +function interpret(args: string[], input: string): string { + const result = spawnSync("git", ["interpret-trailers", ...args], { + cwd: "/", + encoding: "utf8", + env: { ...discoveryEnv(), GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_NOSYSTEM: "1" }, + input, + }); + if (result.error) throw result.error; + if (result.status !== 0) throw new Error(`git interpret-trailers failed: ${result.stderr.trim()}`); + return result.stdout; +} + +const OWN = /^Agent-(?:Harness|Model|Effort)\s*:/i; + /** - * `message` ending with `by`'s trailers, which replace every one it already gives, by Git's own trailer - * rules so they join an existing block such as `Co-Authored-By`. + * `message` ending with `by`'s trailers, which replace every one its trailer block already gives, by Git's + * own trailer rules so they join an existing block such as `Co-Authored-By`. */ export function withTrailers(message: string, by: Implementer): string { - // Git replaces only the last of a key's repeats, so drop them all from the closing paragraph first. - const paragraphs = message.trimEnd().split(/\n[ \t]*\n/); - if (paragraphs.length > 1) { - const kept = paragraphs - .pop()! - .split("\n") - .filter((line) => !/^Agent-(?:Harness|Model|Effort)\s*:/i.test(line)); - if (kept.length) paragraphs.push(kept.join("\n")); + const paragraphs = message + .replace(/\r\n/g, "\n") + .trimEnd() + .split(/\n[ \t]*\n/); + // Git replaces only the last of a key's repeats, so drop them all first, but only from the block Git + // reads as trailers: the last paragraph that is not all comments, when Git finds trailers there. + let block = paragraphs.length - 1; + while (block > 0 && paragraphs[block]!.split("\n").every((line) => line.startsWith("#"))) block--; + const found = interpret(["--only-trailers", "--only-input", "--no-divider"], `${paragraphs.join("\n\n")}\n`); + if (block > 0 && found.split("\n").some((line) => OWN.test(line))) { + const kept = paragraphs[block]!.split("\n").filter((line) => !OWN.test(line)); + if (kept.length) paragraphs[block] = kept.join("\n"); + else paragraphs.splice(block, 1); } - const result = spawnSync( - "git", + return interpret( [ - "interpret-trailers", // A `---` line in the message is prose, not the start of a patch. "--no-divider", "--trailer", @@ -41,15 +59,6 @@ export function withTrailers(message: string, by: Implementer): string { "--trailer", `Agent-Effort: ${field(by.effort)}`, ], - { - // Outside any repository, with no user or system config, so no `trailer.*` setting reshapes them. - cwd: "/", - encoding: "utf8", - env: { ...discoveryEnv(), GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_NOSYSTEM: "1" }, - input: `${paragraphs.join("\n\n")}\n`, - }, + `${paragraphs.join("\n\n")}\n`, ); - if (result.error) throw result.error; - if (result.status !== 0) throw new Error(`git interpret-trailers failed: ${result.stderr.trim()}`); - return result.stdout; } diff --git a/test/trailers.test.ts b/test/trailers.test.ts index a81584fb..6f90154e 100644 --- a/test/trailers.test.ts +++ b/test/trailers.test.ts @@ -20,3 +20,11 @@ test("starts a trailer block under a lone subject, and writes unknown for a valu "subject\n\nAgent-Harness: codex\nAgent-Model: unknown\nAgent-Effort: high\n", ); }); + +test("finds the trailer block as Git does, leaving lookalike lines in the body alone", () => { + expect(withTrailers("fix: x\r\n\r\nAgent-Model: old\r\n", by)).toBe(`fix: x\n\n${trailers}`); + expect(withTrailers("fix: x\n\nAgent-Model: old\n\n# comment\n", by)).toContain("\n\n# comment\n"); + expect(withTrailers("fix: x\n\nAgent-Model: old\n\n# comment\n", by)).not.toContain("old"); + const body = "fix: x\n\nThe example configuration is:\nAgent-Model: old\nThis value identifies the model."; + expect(withTrailers(body, by)).toBe(`${body}\n\n${trailers}`); +}); From 4c03c51ca171b86dbab055381c443c2d964fc12c Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 4 Nov 2026 14:07:05 -0800 Subject: [PATCH 06/11] fix(trailers): let Git mark the trailer block it reads Rather than guess Git's trailer block, append a unique marker trailer, which Git places at the end of the block it detects, then drop every Agent-* trailer and its folded lines from that block before adding the current values. Agent-Harness: claude Agent-Model: claude-opus-5-5 Agent-Effort: high Co-Authored-By: Claude Opus 5.5 --- src/cli/trailers.ts | 35 +++++++++++++++++++++-------------- test/trailers.test.ts | 23 +++++++++++++++++++++++ 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/src/cli/trailers.ts b/src/cli/trailers.ts index 7ae46da1..19a02bba 100644 --- a/src/cli/trailers.ts +++ b/src/cli/trailers.ts @@ -1,6 +1,7 @@ // The trailers that name the harness, model, and effort behind a commit, so audits can tell which of // them made it. Agents add their own as `quest context` says; code that commits for an agent adds them here. import { spawnSync } from "node:child_process"; +import { randomUUID } from "node:crypto"; import { discoveryEnv } from "./git"; @@ -34,19 +35,25 @@ const OWN = /^Agent-(?:Harness|Model|Effort)\s*:/i; * own trailer rules so they join an existing block such as `Co-Authored-By`. */ export function withTrailers(message: string, by: Implementer): string { - const paragraphs = message - .replace(/\r\n/g, "\n") - .trimEnd() - .split(/\n[ \t]*\n/); - // Git replaces only the last of a key's repeats, so drop them all first, but only from the block Git - // reads as trailers: the last paragraph that is not all comments, when Git finds trailers there. - let block = paragraphs.length - 1; - while (block > 0 && paragraphs[block]!.split("\n").every((line) => line.startsWith("#"))) block--; - const found = interpret(["--only-trailers", "--only-input", "--no-divider"], `${paragraphs.join("\n\n")}\n`); - if (block > 0 && found.split("\n").some((line) => OWN.test(line))) { - const kept = paragraphs[block]!.split("\n").filter((line) => !OWN.test(line)); - if (kept.length) paragraphs[block] = kept.join("\n"); - else paragraphs.splice(block, 1); + let text = `${message.replace(/\r\n/g, "\n").trimEnd()}\n`; + // Git replaces only one of a key's repeats, so drop them all first, folded lines included, from the + // block Git reads as trailers: a marker trailer Git appends to that block shows where it ends. + const found = interpret(["--only-trailers", "--only-input", "--no-divider"], text); + if (found.split("\n").some((line) => OWN.test(line))) { + const marker = `Quest-Block-${randomUUID()}: end`; + const lines = interpret(["--no-divider", "--trailer", marker], text).split("\n"); + const end = lines.indexOf(marker); + let begin = end; + while (begin > 0 && lines[begin - 1]!.trim()) begin--; + const block: string[] = []; + let dropping = false; + for (const line of lines.slice(begin, end)) { + dropping = OWN.test(line) || (dropping && /^[ \t]/.test(line)); + if (!dropping) block.push(line); + } + // An emptied block takes its separating blank line with it. + const before = block.length ? lines.slice(0, begin) : lines.slice(0, Math.max(begin - 1, 0)); + text = [...before, ...block, ...lines.slice(end + 1)].join("\n"); } return interpret( [ @@ -59,6 +66,6 @@ export function withTrailers(message: string, by: Implementer): string { "--trailer", `Agent-Effort: ${field(by.effort)}`, ], - `${paragraphs.join("\n\n")}\n`, + text, ); } diff --git a/test/trailers.test.ts b/test/trailers.test.ts index 6f90154e..8e5248d7 100644 --- a/test/trailers.test.ts +++ b/test/trailers.test.ts @@ -1,7 +1,19 @@ +import { execFileSync } from "node:child_process"; + import { expect, test } from "vitest"; import { withTrailers } from "../src/cli/trailers"; +/** The trailers Git reads in `message`, unfolded. */ +function parsed(message: string): string[] { + return execFileSync("git", ["interpret-trailers", "--only-trailers", "--only-input", "--unfold", "--no-divider"], { + input: message, + encoding: "utf8", + }) + .split("\n") + .filter(Boolean); +} + const by = { harness: "codex", model: "gpt-test", effort: "high" }; const trailers = "Agent-Harness: codex\nAgent-Model: gpt-test\nAgent-Effort: high\n"; @@ -28,3 +40,14 @@ test("finds the trailer block as Git does, leaving lookalike lines in the body a const body = "fix: x\n\nThe example configuration is:\nAgent-Model: old\nThis value identifies the model."; expect(withTrailers(body, by)).toBe(`${body}\n\n${trailers}`); }); + +test("drops old agent trailers wherever Git's block ends, with their folded lines", () => { + const own = ["Agent-Harness: codex", "Agent-Model: gpt-test", "Agent-Effort: high"]; + for (const tail of ["\n\n\n# comment\n", "\n\nConflicts:\n\tfile.txt\n"]) { + const out = withTrailers(`fix: x\n\nCo-Authored-By: X \nAgent-Model: old${tail}`, by); + expect(out).not.toContain("old"); + expect(parsed(out).filter((line) => line.startsWith("Agent-"))).toEqual(own); + } + const folded = withTrailers("fix: x\n\nCo-Authored-By: X \nAgent-Model: old\n continuation\n", by); + expect(parsed(folded)).toEqual(["Co-Authored-By: X ", ...own]); +}); From 24a1124997336b6ddccb39d4525a8f4d01370b20 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 4 Nov 2026 14:11:15 -0800 Subject: [PATCH 07/11] fix(trailers): write agent trailers where Git's block ends Replace the marker Git places at the end of the trailer block with the current trailers, in one Git call, so a paragraph the old block hid is never read as trailers. Agent-Harness: claude Agent-Model: claude-opus-5-5 Agent-Effort: high Co-Authored-By: Claude Opus 5.5 --- src/cli/trailers.ts | 56 +++++++++++++++++++------------------------ test/trailers.test.ts | 5 ++++ 2 files changed, 29 insertions(+), 32 deletions(-) diff --git a/src/cli/trailers.ts b/src/cli/trailers.ts index 19a02bba..6ecf8d77 100644 --- a/src/cli/trailers.ts +++ b/src/cli/trailers.ts @@ -35,37 +35,29 @@ const OWN = /^Agent-(?:Harness|Model|Effort)\s*:/i; * own trailer rules so they join an existing block such as `Co-Authored-By`. */ export function withTrailers(message: string, by: Implementer): string { - let text = `${message.replace(/\r\n/g, "\n").trimEnd()}\n`; - // Git replaces only one of a key's repeats, so drop them all first, folded lines included, from the - // block Git reads as trailers: a marker trailer Git appends to that block shows where it ends. - const found = interpret(["--only-trailers", "--only-input", "--no-divider"], text); - if (found.split("\n").some((line) => OWN.test(line))) { - const marker = `Quest-Block-${randomUUID()}: end`; - const lines = interpret(["--no-divider", "--trailer", marker], text).split("\n"); - const end = lines.indexOf(marker); - let begin = end; - while (begin > 0 && lines[begin - 1]!.trim()) begin--; - const block: string[] = []; - let dropping = false; - for (const line of lines.slice(begin, end)) { - dropping = OWN.test(line) || (dropping && /^[ \t]/.test(line)); - if (!dropping) block.push(line); - } - // An emptied block takes its separating blank line with it. - const before = block.length ? lines.slice(0, begin) : lines.slice(0, Math.max(begin - 1, 0)); - text = [...before, ...block, ...lines.slice(end + 1)].join("\n"); + // Git appends a marker trailer to the block it reads as trailers, or starts one. Old agent trailers + // leave that block, folded lines included, since Git would replace only one of a key's repeats, and the + // current ones take the marker's place, so no paragraph the block hid is ever read as trailers. + const marker = `Quest-Block-${randomUUID()}: end`; + const lines = interpret( + // A `---` line in the message is prose, not the start of a patch. + ["--no-divider", "--trailer", marker], + `${message.replace(/\r\n/g, "\n").trimEnd()}\n`, + ).split("\n"); + const end = lines.indexOf(marker); + if (end < 0) throw new Error("git interpret-trailers did not place the trailer block's marker"); + let begin = end; + while (begin > 0 && lines[begin - 1]!.trim()) begin--; + const block: string[] = []; + let dropping = false; + for (const line of lines.slice(begin, end)) { + dropping = OWN.test(line) || (dropping && /^[ \t]/.test(line)); + if (!dropping) block.push(line); } - return interpret( - [ - // A `---` line in the message is prose, not the start of a patch. - "--no-divider", - "--trailer", - `Agent-Harness: ${field(by.harness)}`, - "--trailer", - `Agent-Model: ${field(by.model)}`, - "--trailer", - `Agent-Effort: ${field(by.effort)}`, - ], - text, - ); + const own = [ + `Agent-Harness: ${field(by.harness)}`, + `Agent-Model: ${field(by.model)}`, + `Agent-Effort: ${field(by.effort)}`, + ]; + return [...lines.slice(0, begin), ...block, ...own, ...lines.slice(end + 1)].join("\n"); } diff --git a/test/trailers.test.ts b/test/trailers.test.ts index 8e5248d7..8002f7fc 100644 --- a/test/trailers.test.ts +++ b/test/trailers.test.ts @@ -51,3 +51,8 @@ test("drops old agent trailers wherever Git's block ends, with their folded line const folded = withTrailers("fix: x\n\nCo-Authored-By: X \nAgent-Model: old\n continuation\n", by); expect(parsed(folded)).toEqual(["Co-Authored-By: X ", ...own]); }); + +test("never reads a paragraph the old block hid as trailers", () => { + const out = withTrailers("fix: document models\n\nAgent-Model: documented-example\n\nAgent-Model: old\n", by); + expect(out).toBe(`fix: document models\n\nAgent-Model: documented-example\n\n${trailers}`); +}); From bd669a98f0e34804573e6d0b02c5877ee5277770 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 4 Nov 2026 15:12:20 -0800 Subject: [PATCH 08/11] Drop commit trailers; the PR record names each model The land fixer and kix runner report their harness, model, and effort where they write PR text instead of rewriting commits: the fixer's attribution names its effort, and the runner adds its session's Implementer line to the body's ## Models record. Co-Authored-By: Claude Opus 5.5 --- assets/CONTEXT.md | 6 +-- quest/m0/model-routing/README.md | 14 ++++-- quest/m1/model-check.md | 6 +-- quest/m1/usage.md | 4 +- src/cli/git.ts | 4 +- src/cli/land-fix.ts | 86 ++++---------------------------- src/cli/land-fixer.ts | 9 ++-- src/cli/trailers.ts | 63 ----------------------- src/runner/adapter.ts | 4 +- src/runner/claude.ts | 21 ++++---- src/runner/job.ts | 28 ++++++++--- src/runner/opencode.ts | 17 ++++--- src/runner/prompt.ts | 2 +- test/land.test.ts | 46 +++++------------ test/runner.test.ts | 41 +++++++++------ test/trailers.test.ts | 58 --------------------- 16 files changed, 115 insertions(+), 294 deletions(-) delete mode 100644 src/cli/trailers.ts delete mode 100644 test/trailers.test.ts diff --git a/assets/CONTEXT.md b/assets/CONTEXT.md index 107b3b34..8a4afdbf 100644 --- a/assets/CONTEXT.md +++ b/assets/CONTEXT.md @@ -113,10 +113,6 @@ Find it before opening the fix: `git blame` the lines the fix changes to the com When no single PR caused it, such as an upstream change or a bug as old as the code, say so in one line instead of leaving the section out. Review findings fixed in the same PR are not culprits; the record is for problems that reached the base branch. -End every commit message with `Agent-Harness`, `Agent-Model`, and `Agent-Effort` trailers beside any `Co-Authored-By`: your harness (`claude`, `codex`, `grok`, or `opencode`), its model id, and its effort. -Read them rather than guess: Claude Code knows its model and exports `CLAUDE_EFFORT`; Codex records `model` and `effort` in the last `turn_context` of `~/.codex/sessions/*/*/*/rollout-*-$CODEX_THREAD_ID.jsonl`; Grok records `model_id` and `reasoning_effort` in `~/.grok/sessions/*/$GROK_SESSION_ID/chat_history.jsonl`. -Write `unknown` for any you cannot find. - ## Reviews Before a PR is marked ready for review, and before each push to one that is, the other vendor's model reviews the committed branch locally against the PR's base branch with `quest review --base /`: Claude Code reviews Codex's work, and Codex reviews Claude Code's. @@ -136,6 +132,8 @@ Once the reset passes or the login is restored, the item has cleared: whoever fi The PR body's `## Models` section records the PR's size and each implementing session's harness, model, and effort. A quest PR's size is its quest's; a skill that opens its own PR, such as `/quest-plan`, sizes it as a quest is sized. List them as `Size: M` and one `Implementer: Codex, gpt-6.1-sol, high effort` line per session, which each session adds when it first pushes. +Read your model and effort rather than guess: Claude Code knows its model and exports `CLAUDE_EFFORT`; Codex records `model` and `effort` in the last `turn_context` of `~/.codex/sessions/*/*/*/rollout-*-$CODEX_THREAD_ID.jsonl`; Grok records `model_id` and `reasoning_effort` in `~/.grok/sessions/*/$GROK_SESSION_ID/chat_history.jsonl`. +Write `unknown` for any you cannot find. The PR body's `## Review` section records the latest round: the reviewer harness, model, and effort, the reviewed head SHA, and each finding's disposition (fixed, or refuted with the reason), or the skip and its reason. Rewrite it after each round. A review counts for the head whose SHA it names. diff --git a/quest/m0/model-routing/README.md b/quest/m0/model-routing/README.md index d7f9a7d3..b1172c7b 100644 --- a/quest/m0/model-routing/README.md +++ b/quest/m0/model-routing/README.md @@ -7,14 +7,14 @@ can run from a ranked list `quest` builds in for the quest's size, the same for every repository unless its `AGENTS.md` overrides it: Grok for XS, Opus from S up, with Sol as the fallback through M, and a usage limit falling through to the next entry. Sol reviews, at an effort set by size, and Opus -reviews whatever Sol wrote, so no model reviews its own code. Every commit -and PR records the models and efforts behind it, and fixes link the PR that +reviews whatever Sol wrote, so no model reviews its own code. Every PR +records the models and efforts behind it, and fixes link the PR that caused them, so the rankings can be audited and retuned. This epic's own work is one live spawn in moq-dev/quest from Claude Code that starts an XS quest on Grok and an S or larger on Opus, each recorded and reviewed by Sol. Non-goals: routing kix.quest's runner jobs, which pick their own accounts -and models (their commits still record their models); billing agents to API +and models (their PRs still record their models); billing agents to API keys, which this epic removes. ## Plan @@ -67,8 +67,12 @@ are mostly judgment. Sol xhigh. L and XL: Opus xhigh, no fallback. Reviews: Sol low, medium, high, xhigh, xhigh for XS to XL, never the model that wrote the code; Pullfrog and CodeRabbit add free reviews on top. -- Every commit and PR records its models and efforts, and every fix links - the PR that caused it, so the rankings can be audited (user). +- Every PR records its models and efforts, and every fix links the PR that + caused it, so the rankings can be audited (user). The PR record alone, + without commit trailers (user, 2026-11-04 design checkpoint of + [moq-dev/quest#494](https://github.com/moq-dev/quest/pull/494), revising + that day's /quest-plan pick of `Agent-*` trailers): squash merges drop + branch trailers from `main`, so audits read the PR record anyway. - Every `/quest-review` scores the change out of 10 (user, while planning [the council](/quest/m2/council.md), which joins m2). - Which milestone? ✅ m0 (rejected: m1). It ranks first among m0's work diff --git a/quest/m1/model-check.md b/quest/m1/model-check.md index 3157930f..85f809e3 100644 --- a/quest/m1/model-check.md +++ b/quest/m1/model-check.md @@ -25,9 +25,9 @@ the size tables of [model routing](/quest/m0/model-routing/README.md): ([m0's run](/quest/m0/model-check.md)), then each milestone (rejected: each milestone only) -Audit the routing from the PRs merged since the last run, with -[`quest usage`](/quest/m1/usage.md) once it lands, which record -their size, implementer, and reviewer beside `## Review`: +Audit the routing from the records of the PRs merged since the last run, +with [`quest usage`](/quest/m1/usage.md) once it lands: each body's +`## Models` size and implementers and its `## Review` reviewer: - Tally by size and by implementing harness, model, and effort: review scores, review rounds, findings fixed versus refuted, design checkpoints reached, PRs diff --git a/quest/m1/usage.md b/quest/m1/usage.md index b7d173ae..c64fe69c 100644 --- a/quest/m1/usage.md +++ b/quest/m1/usage.md @@ -39,8 +39,8 @@ Agent's choices, for the implementer to revisit: - A session belongs to a quest or PR by its working directory's checkout and branch: a quest branch names its quest, and a PR's head branch names the PR. Sessions on no quest or PR branch roll up as unattributed. -- Accuracy comes from the records: commit trailers and the PR body's - size, implementers, and each `## Review` round's reviewer, score +- Accuracy comes from the PR record: the body's `## Models` size and + implementers, and each `## Review` round's reviewer, score ([review score](/quest/m0/model-routing/review-score.md)), and findings with their dispositions; follow-up fixes are later PRs that list it under `## Caused by` (`quest context`'s Pull requests section). diff --git a/src/cli/git.ts b/src/cli/git.ts index 0c9f7217..1d9fe536 100644 --- a/src/cli/git.ts +++ b/src/cli/git.ts @@ -33,10 +33,10 @@ export class GitError extends Error { * Git run in the repository at `root`, without prompting, returning its output without trailing newlines. * A `config` lookup that matches nothing is empty rather than an error. */ -export function git(root: string, args: string[], input?: string, env?: NodeJS.ProcessEnv): string { +export function git(root: string, args: string[], input?: string): string { const result = spawnSync("git", ["-C", root, ...args], { encoding: "utf8", - env: { ...discoveryEnv(), GIT_TERMINAL_PROMPT: "0", ...env }, + env: { ...discoveryEnv(), GIT_TERMINAL_PROMPT: "0" }, input, maxBuffer: 1 << 30, stdio: [input === undefined ? "ignore" : "pipe", "pipe", "pipe"], diff --git a/src/cli/land-fix.ts b/src/cli/land-fix.ts index 3a40928b..4d17ae22 100644 --- a/src/cli/land-fix.ts +++ b/src/cli/land-fix.ts @@ -4,9 +4,8 @@ import { spawn, spawnSync } from "node:child_process"; import { constants, closeSync, existsSync, fstatSync, openSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { basename, join, resolve } from "node:path"; import prompt from "../../assets/land-fix.md"; -import { discoveryEnv, git } from "./git"; +import { discoveryEnv } from "./git"; import { programEnvironment, requireProgram } from "./program"; -import { type Implementer, withTrailers } from "./trailers"; export class FixerAgentError extends Error { constructor(readonly exitCode: number) { @@ -88,7 +87,7 @@ export const signature = (failure: string) => failure.replace(/[`'"]/g, "").repl * a commit on the base that skips the test and adds its follow-up quest, which the daemon opens as a PR * of its own. */ -export type FixResult = { agent?: string; model?: string; blame?: boolean; comment?: string; failure?: string } & ( +export type FixResult = { agent?: string; model?: string; effort?: string; blame?: boolean; comment?: string; failure?: string } & ( | { number: number; action: "requeue"; reason: string; cause?: Cause; disable?: string } | { number: number; action: "fixed"; head: string; reason: string } | { number: number; action: "handback"; diagnosis: string; recommendation: string; log?: [number, number] } @@ -98,7 +97,7 @@ export type FixResult = { agent?: string; model?: string; blame?: boolean; comme * The fixer's verdict on the base alone, when no PR is supplied: a mechanical fix committed on its * `base` branch, which the daemon opens as a PR, or a handback, which halts the queue with an issue. */ -export type BaseFix = { agent?: string; model?: string } & ( +export type BaseFix = { agent?: string; model?: string; effort?: string } & ( | { action: "fixed"; head: string; reason: string } | { action: "handback"; diagnosis: string; recommendation: string } ); @@ -123,7 +122,7 @@ function validate(item: any, pr: boolean): void { : item.action === "handback" ? [...named, "action", "diagnosis", "recommendation"] : []; - const metadata = ["agent", "model"].filter((key) => key in item); + const metadata = ["agent", "model", "effort"].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"); if (pr && "blame" in item) { @@ -255,9 +254,6 @@ export async function fix( if (!/^[a-f0-9]{40,64}$/.test(head)) throw new Error(`fix needs a ${ref} branch`); return `- #${number}: branch ${ref} at ${head}`; }); - // Every commit reachable from none of these is the agent's. - const start = git(root, ["for-each-ref", "--format=%(objectname)", "refs/heads"]).split("\n").filter(Boolean); - if (!start.length) throw new Error("fix needs the checkout's branches"); // The agent runs in the PRs' checkout, so neither it nor its commands are found there: a name is // looked up in PATH's absolute directories, and a path is resolved here rather than in the checkout. const program = agent.includes("/") ? resolve(agent) : requireProgram(agent, process.env); @@ -319,84 +315,20 @@ export async function fix( child.once("close", (code) => (code === 0 ? done() : reject(new FixerAgentError(code ?? 1)))); }); const parse = (text: string) => (numbers.length ? parseFixResult(text, numbers) : parseBaseFix(text)); - const parsed = parse(readFixResult(root)); + const results = parse(readFixResult(root)); const index = argv.findIndex((arg) => arg === "-m" || arg === "--model"); const ranModel = (index >= 0 ? argv[index + 1] : argv.find((arg) => arg.startsWith("--model="))?.slice(8)) ?? /^model: ([a-zA-Z0-9._/-]{1,100})$/m.exec(header)?.[1]; - const results = trailed(root, start, Array.isArray(parsed) ? parsed : [parsed], { - harness: basename(agent), + const identity = { + agent: basename(agent), model: ranModel ?? "unknown", // Replaced arguments may run the agent at any effort. effort: args ? "unknown" : effort, - }); - const identity = { agent: basename(agent), model: ranModel ?? "Codex" }; + }; const result = JSON.stringify( - Array.isArray(parsed) ? results.map((item) => ({ ...item, ...identity })) : { ...results[0], ...identity }, + Array.isArray(results) ? results.map((item) => ({ ...item, ...identity })) : { ...results, ...identity }, ); parse(result); writeFileSync(join(root, FIX_RESULT), result); } - -/** - * `results` once every commit the agent made, reachable from a head it reports but from no branch in - * `start`, carries `by`'s trailers. Each is rebuilt with its tree, parents, authorship, and dates; the - * branches that held one move to its replacement, and the results name the replacements, also where their - * prose abbreviates an old commit. - */ -function trailed( - root: string, - start: string[], - results: (FixResult | BaseFix)[], - by: Implementer, -): (FixResult | BaseFix)[] { - const tips = results.flatMap((item) => [ - ...(item.action === "fixed" ? [item.head] : []), - ...("disable" in item && item.disable ? [item.disable] : []), - ]); - if (!tips.length) return results; - const replaced = new Map(); - const commits = git(root, ["rev-list", "--topo-order", "--reverse", ...tips, "--not", ...start]); - for (const commit of commits.split("\n").filter(Boolean)) { - const format = ["%T", "%P", "%an", "%ae", "%ad", "%cn", "%ce", "%cd"].join("%x00"); - const [tree, parents, ...people] = git(root, ["show", "-s", "--date=raw", `--format=${format}`, commit]).split( - "\0", - ); - const [authorName, authorEmail, authorDate, committerName, committerEmail, committerDate] = people; - const message = withTrailers(git(root, ["show", "-s", "--format=%B", commit]), by); - const args = [ - tree!, - ...parents!.split(" ").flatMap((parent) => (parent ? ["-p", replaced.get(parent) ?? parent] : [])), - ]; - replaced.set( - commit, - git(root, ["commit-tree", ...args], message, { - GIT_AUTHOR_NAME: authorName, - GIT_AUTHOR_EMAIL: authorEmail, - GIT_AUTHOR_DATE: `@${authorDate}`, - GIT_COMMITTER_NAME: committerName, - GIT_COMMITTER_EMAIL: committerEmail, - GIT_COMMITTER_DATE: `@${committerDate}`, - }), - ); - } - for (const line of git(root, ["for-each-ref", "--format=%(objectname) %(refname)", "refs/heads"]).split("\n")) { - const [sha, ref] = line.split(" "); - const next = sha && replaced.get(sha); - if (next) git(root, ["update-ref", ref!, next, sha]); - } - const commit = (sha: string) => replaced.get(sha) ?? sha; - const prose = (text: string) => - text.replace(/\b[0-9a-f]{7,40}\b/g, (short) => { - for (const [old, next] of replaced) if (old.startsWith(short)) return next.slice(0, short.length); - return short; - }); - return results.map((item) => { - const copy: Record = { ...item }; - for (const key of ["reason", "comment", "diagnosis", "recommendation"]) - if (typeof copy[key] === "string") copy[key] = prose(copy[key]); - if (item.action === "fixed") copy.head = commit(item.head); - if ("disable" in item && item.disable) copy.disable = commit(item.disable); - return copy as FixResult | BaseFix; - }); -} diff --git a/src/cli/land-fixer.ts b/src/cli/land-fixer.ts index 3f90fd92..40131382 100644 --- a/src/cli/land-fixer.ts +++ b/src/cli/land-fixer.ts @@ -711,14 +711,15 @@ export function fixerText(text: string): string { .replace(/[\r\n\u0000-\u001f\u007f]/g, " "); return `\`${value}\``; } -export function author(result?: Pick): string { +/** The fixer's harness, model, and effort, as a PR's `## Models` record names a session. */ +export function author(result?: Pick): string { const agent = result?.agent ?? "Codex"; const model = result?.model; return ( - [agent, model && model !== agent ? model : undefined] + [agent, model && model !== agent ? model : undefined, result?.effort && `${result.effort} effort`] .filter(Boolean) - .join(" / ") - .replace(/[^a-zA-Z0-9 ._/-]/g, " ") + .join(", ") + .replace(/[^a-zA-Z0-9 ,._/-]/g, " ") .slice(0, 100) || "Codex" ); } diff --git a/src/cli/trailers.ts b/src/cli/trailers.ts deleted file mode 100644 index 6ecf8d77..00000000 --- a/src/cli/trailers.ts +++ /dev/null @@ -1,63 +0,0 @@ -// The trailers that name the harness, model, and effort behind a commit, so audits can tell which of -// them made it. Agents add their own as `quest context` says; code that commits for an agent adds them here. -import { spawnSync } from "node:child_process"; -import { randomUUID } from "node:crypto"; - -import { discoveryEnv } from "./git"; - -/** The agent session that wrote a commit. */ -export interface Implementer { - harness: string; - model: string; - effort: string; -} - -/** `value` when it fits on one trailer line, otherwise `unknown`. */ -const field = (value: string) => (/^[\w.:/@+-]{1,100}$/.test(value) ? value : "unknown"); - -/** `git interpret-trailers` on `input`, outside any repository and with no user or system config, so no `trailer.*` setting reshapes it. */ -function interpret(args: string[], input: string): string { - const result = spawnSync("git", ["interpret-trailers", ...args], { - cwd: "/", - encoding: "utf8", - env: { ...discoveryEnv(), GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_NOSYSTEM: "1" }, - input, - }); - if (result.error) throw result.error; - if (result.status !== 0) throw new Error(`git interpret-trailers failed: ${result.stderr.trim()}`); - return result.stdout; -} - -const OWN = /^Agent-(?:Harness|Model|Effort)\s*:/i; - -/** - * `message` ending with `by`'s trailers, which replace every one its trailer block already gives, by Git's - * own trailer rules so they join an existing block such as `Co-Authored-By`. - */ -export function withTrailers(message: string, by: Implementer): string { - // Git appends a marker trailer to the block it reads as trailers, or starts one. Old agent trailers - // leave that block, folded lines included, since Git would replace only one of a key's repeats, and the - // current ones take the marker's place, so no paragraph the block hid is ever read as trailers. - const marker = `Quest-Block-${randomUUID()}: end`; - const lines = interpret( - // A `---` line in the message is prose, not the start of a patch. - ["--no-divider", "--trailer", marker], - `${message.replace(/\r\n/g, "\n").trimEnd()}\n`, - ).split("\n"); - const end = lines.indexOf(marker); - if (end < 0) throw new Error("git interpret-trailers did not place the trailer block's marker"); - let begin = end; - while (begin > 0 && lines[begin - 1]!.trim()) begin--; - const block: string[] = []; - let dropping = false; - for (const line of lines.slice(begin, end)) { - dropping = OWN.test(line) || (dropping && /^[ \t]/.test(line)); - if (!dropping) block.push(line); - } - const own = [ - `Agent-Harness: ${field(by.harness)}`, - `Agent-Model: ${field(by.model)}`, - `Agent-Effort: ${field(by.effort)}`, - ]; - return [...lines.slice(0, begin), ...block, ...own, ...lines.slice(end + 1)].join("\n"); -} diff --git a/src/runner/adapter.ts b/src/runner/adapter.ts index 19eb2d73..bc75f609 100644 --- a/src/runner/adapter.ts +++ b/src/runner/adapter.ts @@ -69,8 +69,8 @@ export interface Running { done: Promise; /** Ask the CLI to stop, killing it after a grace period. */ stop(): void; - /** The model and effort the session last ran on, each `unknown` until the CLI shows it. */ - ran(): { model: string; effort: string }; + /** The model and effort the session runs on, each `unknown` where the CLI does not show it. */ + ran(): Promise<{ model: string; effort: string }>; } export interface SignedIn { diff --git a/src/runner/claude.ts b/src/runner/claude.ts index b43b33ec..799175dd 100644 --- a/src/runner/claude.ts +++ b/src/runner/claude.ts @@ -61,14 +61,12 @@ export function jobEnvironment(env: NodeJS.ProcessEnv, home: string, sandbox: Sa * ask), and WebFetch, which would run outside the sandbox, is off. */ export function claudeSettings(sandbox: Sandbox, effort: string): Record { - // Claude Code shows its effort only to hooks and commands, as CLAUDE_EFFORT, and only within a turn. - // Hooks run outside the sandbox, so `effort` is in the account's home, which commands cannot reach. + // Claude Code shows its effort only to hooks and commands, as CLAUDE_EFFORT, and only within a turn, so + // a hook on the kix tools records it before `deliver` reads it. Hooks run outside the sandbox, so + // `effort` is in the account's home, which commands cannot reach. const record = { type: "command", command: `printf %s "$CLAUDE_EFFORT" > '${effort.replaceAll("'", "'\\''")}'` }; return { - hooks: { - PreToolUse: [{ matcher: "mcp__kix__.*", hooks: [record] }], - Stop: [{ hooks: [record] }], - }, + hooks: { PreToolUse: [{ matcher: "mcp__kix__.*", hooks: [record] }] }, sandbox: { enabled: true, failIfUnavailable: true, @@ -198,15 +196,14 @@ export class Claude implements Adapter { const settings = join(launch.home, `kix-settings-${launch.session}.json`); const effortFile = join(launch.home, `kix-effort-${launch.session}`); writeFileSync(settings, JSON.stringify(claudeSettings(launch.sandbox, effortFile)), { mode: 0o600 }); - let effort = "unknown"; const readEffort = () => { try { - effort = readFileSync(effortFile, "utf8").trim() || effort; - } catch {} - return effort; + return readFileSync(effortFile, "utf8").trim() || "unknown"; + } catch { + return "unknown"; + } }; const cleanup = () => { - readEffort(); rmSync(config, { force: true }); rmSync(settings, { force: true }); rmSync(effortFile, { force: true }); @@ -268,7 +265,7 @@ export class Claude implements Adapter { }); return { done, - ran: () => ({ model: reader.model(), effort: readEffort() }), + ran: async () => ({ model: reader.model(), effort: readEffort() }), stop() { signal("SIGTERM"); killer ??= setTimeout(() => signal("SIGKILL"), STOP_GRACE_MS); diff --git a/src/runner/job.ts b/src/runner/job.ts index 13812d38..fea4e58a 100644 --- a/src/runner/job.ts +++ b/src/runner/job.ts @@ -6,7 +6,6 @@ import { randomUUID } from "node:crypto"; import { type Round, ResponsesError } from "../cli/review"; -import { withTrailers } from "../cli/trailers"; import type { Adapter, Exit, Running } from "./adapter"; import { Refused } from "./connection"; @@ -91,6 +90,22 @@ function subject(summary: string): string { return line.length <= 72 ? line : `${line.slice(0, 71)}…`; } +/** `value` when it fits in a `## Models` line, otherwise `unknown`. */ +const field = (value: string) => (/^[\w.:/@+-]{1,100}$/.test(value) ? value : "unknown"); + +/** `body` with `line` as an item of its `## Models` section, which it gains when it has none. */ +function withModels(body: string, line: string): string { + const item = `- ${line}`; + if (body.split(/\r?\n/).some((each) => each.trim() === item)) return body; + const section = /^## Models[ \t]*\r?$/m.exec(body); + if (!section) return `${body.trimEnd()}\n\n## Models\n\n${item}\n`; + const start = section.index + section[0].length; + const next = /^## /m.exec(body.slice(start)); + const end = next ? start + next.index : body.length; + const gap = body.slice(start, end).trim() ? "\n" : "\n\n"; + return `${body.slice(0, end).trimEnd()}${gap}${item}\n${next ? "\n" : ""}${body.slice(end)}`; +} + function string(value: unknown, name: string, max: number): string { if (typeof value !== "string" || !value.trim()) throw new Error(`${name} is required`); if (value.length > max) throw new Error(`${name} is longer than ${max} characters`); @@ -116,7 +131,7 @@ export class Attempt { /** Settles once no review runs, so the attempt never removes a checkout a review still uses. */ #reviewRun: Promise = Promise.resolve(); #agent: Running | undefined; - /** The CLI the attempt's account runs, which its commits name. */ + /** The CLI the attempt's account runs, which its pull request's `## Models` record names. */ #harness = "unknown"; /** Aborts the devShell build, which runs before there is an agent to stop. */ #preparing = new AbortController(); @@ -386,15 +401,13 @@ export class Attempt { const idle = () => !snapshot.changes.length && !snapshot.merge && !pull && !land; if (idle()) return { head: workspace.head }; - const ran = this.#agent?.ran() ?? { model: "unknown", effort: "unknown" }; - const commit = withTrailers(message, { harness: this.#harness, ...ran }); const send = () => this.ctx.publish({ job: this.job.id, attempt: this.job.attempt, head: workspace.head, session: this.#session, - ...((snapshot.changes.length || snapshot.merge) && { message: commit }), + ...((snapshot.changes.length || snapshot.merge) && { message }), changes: snapshot.changes, ...(snapshot.merge && { merge: snapshot.merge }), // kix verifies these pointers against the existing head before carrying them into the merge tree. @@ -744,11 +757,14 @@ export class Attempt { const closed = this.#closed(); if (closed) return closed; const text = string(args.summary, "summary", 4000); + // kix names this session in the body's `## Models` record, from what the CLI reports. + const ran = (await this.#agent?.ran()) ?? { model: "unknown", effort: "unknown" }; + const implementer = `Implementer: ${field(this.#harness)}, ${field(ran.model)}, ${field(ran.effort)} effort`; const pull = pulled ? undefined : { title: string(args.title, "title", 256), - body: string(args.body, "body", 60_000), + body: withModels(string(args.body, "body", 59_000), implementer), draft: args.draft !== false, }; const method = args.method as Land["method"]; diff --git a/src/runner/opencode.ts b/src/runner/opencode.ts index b684b58c..935bf1a2 100644 --- a/src/runner/opencode.ts +++ b/src/runner/opencode.ts @@ -471,7 +471,8 @@ export class Opencode implements Adapter { void closed.then(() => listening(null)); let stopping = false; - let ran = { model: "unknown", effort: "unknown" }; + const unknown = { model: "unknown", effort: "unknown" }; + let ran: () => Promise<{ model: string; effort: string }> = async () => unknown; let killer: ReturnType | undefined; let abort: () => Promise = async () => {}; let watched: Promise = Promise.resolve(); @@ -502,6 +503,13 @@ export class Opencode implements Adapter { writeFileSync(join(sessions, launch.session), session, { mode: 0o600 }); } abort = () => client.session.abort({ sessionID: session }); + // The assistant message that is calling a kix tool is the session's latest. + ran = async () => { + const { data } = await client.session.messages({ sessionID: session, limit: 1 }); + const info = data?.at(-1)?.info; + if (info?.role !== "assistant") return unknown; + return { model: `${info.providerID}/${info.modelID}`, effort: info.variant ?? "unknown" }; + }; if (stopping) return failure("stopped"); let limited: Exit["limited"] = null; @@ -517,10 +525,7 @@ export class Opencode implements Adapter { // Token deltas repeat what the finished parts carry; other sessions' events are not this job's. if (properties.sessionID !== session || event.type.endsWith(".delta")) continue; launch.output(`${JSON.stringify(event)}\n`); - if (event.type === "message.updated" && event.properties.info.role === "assistant") { - const { providerID, modelID, variant } = event.properties.info; - ran = { model: `${providerID}/${modelID}`, effort: variant ?? "unknown" }; - } else if (event.type === "session.status") { + if (event.type === "session.status") { const hit = retryLimit(event.properties.status); if (hit && !limited) { limited = hit; @@ -562,7 +567,7 @@ export class Opencode implements Adapter { }); return { done, - ran: () => ran, + ran: () => ran().catch(() => unknown), stop() { stopping = true; // Abort the turn so opencode records it, then stop the server whether or not it answered. diff --git a/src/runner/prompt.ts b/src/runner/prompt.ts index e84fc8f5..f131946f 100644 --- a/src/runner/prompt.ts +++ b/src/runner/prompt.ts @@ -22,7 +22,7 @@ export function systemPrompt(domains: string[]): string { Report only through the kix tools: - \`checkpoint\`: at each meaningful step, with a short summary the maintainer reads on their phone. kix publishes the working tree's changes to the job's branch, so another runner can continue if this one goes away. - \`ask\`: one round of independent questions for the maintainer, with options and your recommendation. kix pauses the job; end your turn right after asking. You are resumed in this session with the answers. -- \`deliver\`: when the work is done, with a summary and the pull request's title, body, and draft flag. List the follow-ups you suggest as \`followups\` too, which kix offers the maintainer to plan. kix publishes the working tree and opens or updates the pull request. Unless it stays a draft, the runner then reviews the head kix committed with the other vendor's model, as \`quest review\` does, and kix records the round as the pull request's \`quest review\` check. If the review found something, fix or refute each finding and deliver again with \`responses\`; otherwise end your turn after delivering. +- \`deliver\`: when the work is done, with a summary and the pull request's title, body, and draft flag. List the follow-ups you suggest as \`followups\` too, which kix offers the maintainer to plan. kix publishes the working tree and opens or updates the pull request, adding this session's \`Implementer:\` line to the body's \`## Models\` record, so write only its \`Size:\` there. Unless it stays a draft, the runner then reviews the head kix committed with the other vendor's model, as \`quest review\` does, and kix records the round as the pull request's \`quest review\` check. If the review found something, fix or refute each finding and deliver again with \`responses\`; otherwise end your turn after delivering. - \`context\`: on a job for an issue or pull request, read it as GitHub has it now: its text and comments, and a pull request's reviews, unresolved review threads, and checks. Use it wherever the workflow reads them with \`gh\` or \`quest status\`. On a new plan, it reads the maintainer's idea and any context pasted with it. Where the workflow says to claim with \`quest start\`, or to take or release a pull request with \`quest iterate\`, \`quest merge\`, or \`quest release\`, skip it; kix holds the job's pull request. Where it says to push, open or update a pull request, mark it ready, or comment on it, call \`deliver\` instead. Where it says to run \`quest review\` or \`/quest-review\`, deliver instead and let the runner review: its check is the head's review record, so write in the body's \`## Review\` section that the \`quest review\` check records it. Where it says to ask or prompt the user, call \`ask\`; you can always prompt, so ignore instructions for agents that cannot. Never run \`git push\`, \`gh\`, or anything else that writes to GitHub. Commit locally if you like; kix publishes your working tree as it is. diff --git a/test/land.test.ts b/test/land.test.ts index bfcfab4e..a3137662 100644 --- a/test/land.test.ts +++ b/test/land.test.ts @@ -25,6 +25,7 @@ import { ACTIONS_APP, type Pull, type World, fakeGitHub } from "../cloudflare/de import { authenticatedRead, gitAuthentication } from "../src/cli/land-git"; import { ADMITTED, + author, COLLECTION_ATTEMPTS, collectFixers, validateFix, @@ -2770,7 +2771,7 @@ describe.skipIf(process.platform === "win32")("asynchronous land fixer", () => { expect(fix.branch).toBe(branch); const head = forge.ref("upstream/project", `refs/heads/${branch}`); expect(git(forge.origin, "rev-parse", `${head}^`)).toBe(fixture.base.sha); - expect(fix.body).toContain(`mechanical fix by agent / Codex: \`restore the missing file\` (head ${head})`); + expect(fix.body).toContain(`mechanical fix by agent, unknown, unknown effort: \`restore the missing file\` (head ${head})`); expect(fix.body).toContain("Merge it as an administrator"); // Collected once, and the queue stays halted on this base. expect(await fixture.collect()).toEqual([]); @@ -2848,7 +2849,7 @@ printf '{"action":"fixed","head":"%s","reason":"edit the flake"}' "$(git rev-par expect(fixture.pull.head).not.toBe(fixture.plan.queue[0].head); expect(git(fixture.root, "show", "-s", "--format=%P", fixture.pull.head!)).toContain(fixture.plan.base.sha); expect(fixture.pull.body).toContain( - `mechanical fix by agent / Codex: \`preserve both conflict sides\` (head ${fixture.pull.head})`, + `mechanical fix by agent, unknown, unknown effort: \`preserve both conflict sides\` (head ${fixture.pull.head})`, ); expect(fixture.pull.autoMerge?.commitBody).toBe(pin(fixture.pull.head!)); }); @@ -3433,7 +3434,7 @@ ${requeueScript}`); expect(fixture.pull.autoMerge?.commitBody).toBe(pin(fixture.pull.head!)); // The fix is recorded in `## Review`; a PR that stays queued gets no outcome comment. expect(fixture.pull.body).toContain( - `mechanical fix by agent / Codex: \`preserve both conflict sides\` (head ${fixture.pull.head})`, + `mechanical fix by agent, unknown, unknown effort: \`preserve both conflict sides\` (head ${fixture.pull.head})`, ); expect(fixture.forge.comment(1)).toBeUndefined(); }); @@ -3687,44 +3688,22 @@ printf '[{"number":1,"action":"handback","diagnosis":"saw %s","recommendation":" expect(result.model).toBe("actual-model"); }); - test("gives the agent's new commits its trailers, moving their branches and the SHAs its result names", async () => { + test("records the model Codex's header names and the effort it ran at", async () => { const dir = fixRepository(); - const start = git(dir, "rev-parse", "pr-1"); const log = join(dir, "failure"); writeFileSync(log, "failure"); const agent = join(dir, "codex"); - // Codex prints its model in the header on stderr; the agent's own trailers stay beside quest's. writeFileSync( agent, - `#!/bin/sh -set -eu -cat >/dev/null -printf 'model: header-model\\nreasoning effort: xhigh\\n' >&2 -git checkout -q pr-1 -printf 'fixed\\n' > code.txt -git add code.txt -git -c user.name=Agent -c user.email=a@a -c commit.gpgsign=false commit -qm 'fix: thing' -m 'Co-Authored-By: Someone ' -head=$(git rev-parse HEAD) -short=$(git rev-parse --short=10 HEAD) -printf '[{"number":1,"action":"fixed","head":"%s","reason":"committed %s"}]' "$head" "$short" > quest-land-result.json -`, + `#!/bin/sh\ncat >/dev/null\nprintf 'model: header-model\\nreasoning effort: high\\n' >&2\n${requeueScript}\n`, { mode: 0o755 }, ); - await fix(dir, log, [1], agent, ["-"]); + vi.stubEnv("OPENAI_BASE_URL", "http://proxy.test/v1"); + vi.stubEnv("OPENAI_API_KEY", "proxy-client"); + await fix(dir, log, [1], agent, undefined, undefined, "high"); const [result] = parseFixResult(readFileSync(join(dir, "quest-land-result.json"), "utf8"), [1]); - const head = git(dir, "rev-parse", "pr-1"); - expect(result).toMatchObject({ - action: "fixed", - head, - reason: `committed ${head.slice(0, 10)}`, - model: "header-model", - }); - expect(git(dir, "rev-parse", `${head}^`)).toBe(start); - expect(git(dir, "show", "-s", "--format=%an %B", head)).toBe( - "Agent fix: thing\n\nCo-Authored-By: Someone \nAgent-Harness: codex\nAgent-Model: header-model\nAgent-Effort: unknown", - ); - // The branch tips the agent started from keep their commits. - expect(git(dir, "show", "-s", "--format=%B", start)).toBe("head"); + expect(result).toMatchObject({ agent: "codex", model: "header-model", effort: "high" }); + expect(author(result)).toBe("codex, header-model, high effort"); }); test("rejects mixing individually approved protected files", async () => { @@ -3785,11 +3764,12 @@ printf '[{"number":1,"action":"fixed","head":"%s","reason":"committed %s"}]' "$h const results = JSON.parse(readFileSync(resultPath, "utf8")); results[0].agent = "Claude"; results[0].model = "actual-model"; + results[0].effort = "xhigh"; writeFileSync(resultPath, JSON.stringify(results)); fixture.pull.body = "Author content\r\n\r\n## Review\r\n\r\nPrevious review\r\n\r\n(written by old-model)\n\n(written by older-model)"; expect(await fixture.collect()).toEqual(["requeued 1"]); - expect(fixture.pull.body).toContain("mechanical fix by Claude / actual-model:"); + expect(fixture.pull.body).toContain("mechanical fix by Claude, actual-model, xhigh effort:"); expect(fixture.pull.body).toContain("Author content"); expect(fixture.pull.body).toContain("Previous review"); // The author's attributions stay; the fix is credited only in its own Review line. diff --git a/test/runner.test.ts b/test/runner.test.ts index f637e403..39722542 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -1407,6 +1407,8 @@ const streams = []; const send = (event) => { for (const s of streams) s.write("data: " + JSON.stringify(event) + "\\n\\n"); }; let aborted = () => {}; let sessions = 0; +// The assistant message the turn is on, which the session's latest message reports. +let latest; let id = 0; async function rpc(method, params) { const response = await fetch(kix.url, { @@ -1438,6 +1440,7 @@ async function prompt(session, input) { if (step.shell) log({ shell: require("node:child_process").execFileSync(config.shell, ["-c", step.shell], { encoding: "utf8" }) }); if (step.emit) send({ ...step.emit, properties: { ...step.emit.properties, sessionID: session } }); + if (step.assistant) latest = { info: step.assistant, parts: [] }; if (step.tool) log({ tool: step.tool, reply: await rpc("tools/call", { name: step.tool, arguments: step.args }) }); if (step.wait) { await new Promise((resolve) => (aborted = resolve)); @@ -1457,6 +1460,9 @@ const server = http.createServer(async (request, response) => { } if (path === "/session") return json(response, { id: "ses_" + ++sessions + "_" + process.pid }); const message = /^\\/session\\/([^/]+)\\/message$/.exec(path); + if (message && request.method === "GET") { + return json(response, latest ? [latest] : []); + } if (message) return json(response, await prompt(message[1], await body(request))); if (/\\/abort$/.test(path)) { aborted(); @@ -1802,17 +1808,16 @@ describe("quest runner", () => { expect(r.hub.of("status").some((s) => s.attempts.length === 1 && s.slots === 1)).toBe(true); expect(r.hub.of("checkpoint")).toMatchObject([{ summary: "Wrote a.txt", session: expect.any(String) }]); const [checkpoint, delivery] = r.publishes; - // The runner names the agent behind each commit from the CLI's own report of its model and effort. - expect(checkpoint).toMatchObject({ - job: "job1", - attempt: 1, - head: null, - message: "Wrote a.txt\n\nAgent-Harness: claude\nAgent-Model: claude-test\nAgent-Effort: high\n", - }); + expect(checkpoint).toMatchObject({ job: "job1", attempt: 1, head: null, message: "Wrote a.txt" }); expect(checkpoint!.changes).toEqual([{ path: "a.txt", content: Buffer.from("first\n").toString("base64") }]); expect(delivery).toMatchObject({ head: "0".repeat(39) + "1", - pull: { title: "feat: thing", body: "Body", draft: false }, + // The runner names the session in the body's record, from the CLI's own report of its model and effort. + pull: { + title: "feat: thing", + body: "Body\n\n## Models\n\n- Implementer: claude, claude-test, high effort\n", + draft: false, + }, }); expect(delivery!.changes.map((c: Sent) => [c.path, c.content === null])).toEqual([ ["b.txt", false], @@ -1950,10 +1955,8 @@ describe("quest runner", () => { posix("drives opencode serve through a question and a resume in the same session", async () => { const r = rig(undefined, "opencode"); - const info = { role: "assistant", providerID: "openai", modelID: "gpt-test", variant: "high" }; r.steps({ start: [ - { emit: { type: "message.updated", properties: { info } } }, { write: "a.txt", content: "draft\n" }, { shell: 'printf "%s|%s|%s|%s|%s" "$IN_DEVSHELL" "$HOME" "${OPENCODE_SERVER_PASSWORD:-none}" "${QUEST_COMMAND:-none}" "$TMPDIR"', @@ -1964,16 +1967,18 @@ describe("quest runner", () => { args: { summary: "Need a choice", questions: [{ question: "Which?", options: [{ label: "A" }] }] }, }, ], - resume: [{ tool: "deliver", args: { summary: "Went with A", title: "feat: a", body: "Body" } }], + resume: [ + { assistant: { role: "assistant", providerID: "openai", modelID: "gpt-test", variant: "high" } }, + { + tool: "deliver", + args: { summary: "Went with A", title: "feat: a", body: "## Models\n\n- Size: S\n\n## Review\n\nNone" }, + }, + ], }); r.assign(job(r.upstream.commit), "Extra rule."); const question = await r.hub.next((m) => m.type === "question"); await idle(r.runner); expect(r.hub.of("checkpoint")).toMatchObject([{ summary: "Drafted", session: question.session }]); - // The commit names the model and effort opencode's assistant message reported. - expect(r.publishes[0]!.message).toBe( - "Drafted\n\nAgent-Harness: opencode\nAgent-Model: openai/gpt-test\nAgent-Effort: high\n", - ); const [nix, first, srt, shell] = r.calls(); expect(nix.nix.slice(0, 3)).toEqual(["print-dev-env", "--accept-flake-config", "--no-write-lock-file"]); const home = r.home.accountHome("acct1"); @@ -2064,6 +2069,10 @@ describe("quest runner", () => { const result = await r.result(); await idle(r.runner); expect(result).toMatchObject({ attempt: 2, outcome: "completed", summary: "Went with A" }); + // The resumed session names itself from its latest assistant message, in the agent's own record. + expect(r.publishes.at(-1)!.pull.body).toBe( + "## Models\n\n- Size: S\n- Implementer: opencode, openai/gpt-test, high effort\n\n## Review\n\nNone", + ); const resumed = r.calls().filter((c) => c.prompted); expect(resumed).toHaveLength(2); // The kix session maps to the opencode session that asked. @@ -2244,7 +2253,7 @@ describe("quest runner", () => { posix("continues from the head kix reports after a publication's reply was lost", async () => { let landed = ""; const r = rig((body, source) => { - if (body.message.startsWith("First\n")) { + if (body.message === "First") { // kix commits the checkpoint, but its reply never arrives. for (const change of body.changes) writeFileSync(join(source.dir, change.path), Buffer.from(change.content, "base64")); diff --git a/test/trailers.test.ts b/test/trailers.test.ts deleted file mode 100644 index 8002f7fc..00000000 --- a/test/trailers.test.ts +++ /dev/null @@ -1,58 +0,0 @@ -import { execFileSync } from "node:child_process"; - -import { expect, test } from "vitest"; - -import { withTrailers } from "../src/cli/trailers"; - -/** The trailers Git reads in `message`, unfolded. */ -function parsed(message: string): string[] { - return execFileSync("git", ["interpret-trailers", "--only-trailers", "--only-input", "--unfold", "--no-divider"], { - input: message, - encoding: "utf8", - }) - .split("\n") - .filter(Boolean); -} - -const by = { harness: "codex", model: "gpt-test", effort: "high" }; -const trailers = "Agent-Harness: codex\nAgent-Model: gpt-test\nAgent-Effort: high\n"; - -test("joins an existing trailer block and replaces every repeat of its own keys", () => { - expect( - withTrailers( - "fix: thing\n\nBody\n---\nmore\n\nagent-model: old\nCo-Authored-By: X \nAgent-Model: stale\nAgent-Effort: low\n", - by, - ), - ).toBe(`fix: thing\n\nBody\n---\nmore\n\nCo-Authored-By: X \n${trailers}`); -}); - -test("starts a trailer block under a lone subject, and writes unknown for a value that cannot be one", () => { - expect(withTrailers("Agent-Model: subject", by)).toBe(`Agent-Model: subject\n\n${trailers}`); - expect(withTrailers("subject\n\nAgent-Harness: codex\n", { ...by, model: "two words\nAgent-Model: x" })).toBe( - "subject\n\nAgent-Harness: codex\nAgent-Model: unknown\nAgent-Effort: high\n", - ); -}); - -test("finds the trailer block as Git does, leaving lookalike lines in the body alone", () => { - expect(withTrailers("fix: x\r\n\r\nAgent-Model: old\r\n", by)).toBe(`fix: x\n\n${trailers}`); - expect(withTrailers("fix: x\n\nAgent-Model: old\n\n# comment\n", by)).toContain("\n\n# comment\n"); - expect(withTrailers("fix: x\n\nAgent-Model: old\n\n# comment\n", by)).not.toContain("old"); - const body = "fix: x\n\nThe example configuration is:\nAgent-Model: old\nThis value identifies the model."; - expect(withTrailers(body, by)).toBe(`${body}\n\n${trailers}`); -}); - -test("drops old agent trailers wherever Git's block ends, with their folded lines", () => { - const own = ["Agent-Harness: codex", "Agent-Model: gpt-test", "Agent-Effort: high"]; - for (const tail of ["\n\n\n# comment\n", "\n\nConflicts:\n\tfile.txt\n"]) { - const out = withTrailers(`fix: x\n\nCo-Authored-By: X \nAgent-Model: old${tail}`, by); - expect(out).not.toContain("old"); - expect(parsed(out).filter((line) => line.startsWith("Agent-"))).toEqual(own); - } - const folded = withTrailers("fix: x\n\nCo-Authored-By: X \nAgent-Model: old\n continuation\n", by); - expect(parsed(folded)).toEqual(["Co-Authored-By: X ", ...own]); -}); - -test("never reads a paragraph the old block hid as trailers", () => { - const out = withTrailers("fix: document models\n\nAgent-Model: documented-example\n\nAgent-Model: old\n", by); - expect(out).toBe(`fix: document models\n\nAgent-Model: documented-example\n\n${trailers}`); -}); From 69b378e7d08955853a7a20b805ef8c97e2f02588 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 4 Nov 2026 15:14:58 -0800 Subject: [PATCH 09/11] Format Co-Authored-By: Claude Opus 5.5 --- src/cli/land-fix.ts | 9 ++++++++- test/land.test.ts | 4 +++- test/runner.test.ts | 6 +++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/cli/land-fix.ts b/src/cli/land-fix.ts index 4d17ae22..141bd7c9 100644 --- a/src/cli/land-fix.ts +++ b/src/cli/land-fix.ts @@ -87,7 +87,14 @@ export const signature = (failure: string) => failure.replace(/[`'"]/g, "").repl * a commit on the base that skips the test and adds its follow-up quest, which the daemon opens as a PR * of its own. */ -export type FixResult = { agent?: string; model?: string; effort?: string; blame?: boolean; comment?: string; failure?: string } & ( +export type FixResult = { + agent?: string; + model?: string; + effort?: string; + blame?: boolean; + comment?: string; + failure?: string; +} & ( | { number: number; action: "requeue"; reason: string; cause?: Cause; disable?: string } | { number: number; action: "fixed"; head: string; reason: string } | { number: number; action: "handback"; diagnosis: string; recommendation: string; log?: [number, number] } diff --git a/test/land.test.ts b/test/land.test.ts index a3137662..74e9b096 100644 --- a/test/land.test.ts +++ b/test/land.test.ts @@ -2771,7 +2771,9 @@ describe.skipIf(process.platform === "win32")("asynchronous land fixer", () => { expect(fix.branch).toBe(branch); const head = forge.ref("upstream/project", `refs/heads/${branch}`); expect(git(forge.origin, "rev-parse", `${head}^`)).toBe(fixture.base.sha); - expect(fix.body).toContain(`mechanical fix by agent, unknown, unknown effort: \`restore the missing file\` (head ${head})`); + expect(fix.body).toContain( + `mechanical fix by agent, unknown, unknown effort: \`restore the missing file\` (head ${head})`, + ); expect(fix.body).toContain("Merge it as an administrator"); // Collected once, and the queue stays halted on this base. expect(await fixture.collect()).toEqual([]); diff --git a/test/runner.test.ts b/test/runner.test.ts index 4f49988f..fc257192 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -1971,7 +1971,11 @@ describe("quest runner", () => { { assistant: { role: "assistant", providerID: "openai", modelID: "gpt-test", variant: "high" } }, { tool: "deliver", - args: { summary: "Went with A", title: "feat: a", body: "## Models\n\n- Size: S\n\n## Review\n\nNone" }, + args: { + summary: "Went with A", + title: "feat: a", + body: "## Models\n\n- Size: S\n\n## Review\n\nNone", + }, }, ], }); From 5c9e4cc851e450084434f27509eada88fd6c6f2e Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 4 Nov 2026 15:25:53 -0800 Subject: [PATCH 10/11] kix keeps earlier implementers when a later session delivers The Worker carries the Implementer lines of the job's existing pull request into the body a later session delivers, so a session that continues another's job adds to the ## Models record rather than replacing it. The record's helpers move to src/github/models.ts, shared by the runner and the Worker. Co-Authored-By: Claude Opus 5.5 --- cloudflare/src/github/service.ts | 9 +++++++-- cloudflare/test/seed.test.ts | 20 +++++++++++++++++++ src/github/models.ts | 33 ++++++++++++++++++++++++++++++++ src/runner/job.ts | 14 +------------- 4 files changed, 61 insertions(+), 15 deletions(-) create mode 100644 src/github/models.ts diff --git a/cloudflare/src/github/service.ts b/cloudflare/src/github/service.ts index d9d6c1a8..81af9a78 100644 --- a/cloudflare/src/github/service.ts +++ b/cloudflare/src/github/service.ts @@ -13,6 +13,8 @@ import { } from "quest/core"; import { checksState, latestContexts } from "../../../src/github/checks"; import { cleanAutoMergeRefusal, immediateMerge, queueHead } from "../../../src/github/merge"; +import { implementers, withModels } from "../../../src/github/models"; +import { BODY_LIMIT } from "../jobs/trail"; import { osiApproved } from "../licenses"; import { CI_PERMISSIONS, type CIPermission, ciProblems } from "./ci"; import { OBSERVED, type Observed, observedGap, REVIEW_CHECK, unreviewed } from "./review"; @@ -2204,10 +2206,13 @@ export class GitHubService { if (pull?.state !== "open") throw error; } } - if (pull.title !== input.title || pull.body !== input.body) + // A later session's body keeps the sessions that delivered before it in its `## Models` record. + const body = implementers(pull.body ?? "").reduce(withModels, input.body); + if (body.length > BODY_LIMIT) throw new GitHubError("The pull request body is too long"); + if (pull.title !== input.title || pull.body !== body) pull = await this.#api(token, `${pulls}/${number(pull.number)}`, "PATCH", { title: input.title, - body: input.body, + body, }); // Finished work leaves draft; a maintainer's own choice to mark it ready is never reverted. if (pull.draft && !input.draft) { diff --git a/cloudflare/test/seed.test.ts b/cloudflare/test/seed.test.ts index 84126828..004920cc 100644 --- a/cloudflare/test/seed.test.ts +++ b/cloudflare/test/seed.test.ts @@ -465,6 +465,26 @@ it("publishes jobs' work end to end, through the upstream branch to the pull req // Finished work marks it ready for review. await publish({ job: "seed-start-offline", head: first.head, pull: { ...pull, draft: false } }); expect(rocket.pulls.at(-1)!.draft).toBe(false); + // A later session's body keeps the sessions that delivered before it in its `## Models` record. + const models = (lines: string) => ({ + ...pull, + draft: false, + body: `Done.\n\n## Models\n\n${lines}\n\n## Review\n\nNone`, + }); + await publish({ + job: "seed-start-offline", + head: first.head, + pull: models("- Implementer: claude, first, high effort"), + }); + await publish({ + job: "seed-start-offline", + head: first.head, + pull: models("- Size: S\n- Implementer: opencode, second, high effort"), + }); + expect(rocket.pulls.at(-1)!.body).toBe( + models("- Size: S\n- Implementer: opencode, second, high effort\n- Implementer: claude, first, high effort") + .body, + ); const continued = await latest("quest/m0/import.md"); expect(continued).toMatchObject({ id: retry.id, state: "running", head: rocket.branches["quest/m0/import"] }); diff --git a/src/github/models.ts b/src/github/models.ts new file mode 100644 index 00000000..c23f9dc5 --- /dev/null +++ b/src/github/models.ts @@ -0,0 +1,33 @@ +// A pull request body's `## Models` record, which names the PR's size and each session that implemented it. + +const SECTION = /^## Models[ \t]*\r?$/m; + +/** Where `body`'s `## Models` section runs, after its heading, or null without one. */ +function section(body: string): { start: number; end: number } | null { + const heading = SECTION.exec(body); + if (!heading) return null; + const start = heading.index + heading[0].length; + const next = /^## /m.exec(body.slice(start)); + return { start, end: next ? start + next.index : body.length }; +} + +/** The `Implementer:` items of `body`'s `## Models` section, each as its line without the bullet. */ +export function implementers(body: string): string[] { + const found = section(body); + if (!found) return []; + return body + .slice(found.start, found.end) + .split(/\r?\n/) + .flatMap((line) => /^- (Implementer: .+?)\s*$/.exec(line)?.[1] ?? []); +} + +/** `body` with `line` as an item of its `## Models` section, which it gains when it has none. */ +export function withModels(body: string, line: string): string { + const item = `- ${line}`; + if (body.split(/\r?\n/).some((each) => each.trim() === item)) return body; + const found = section(body); + if (!found) return `${body.trimEnd()}\n\n## Models\n\n${item}\n`; + const { start, end } = found; + const gap = body.slice(start, end).trim() ? "\n" : "\n\n"; + return `${body.slice(0, end).trimEnd()}${gap}${item}\n${end < body.length ? "\n" : ""}${body.slice(end)}`; +} diff --git a/src/runner/job.ts b/src/runner/job.ts index 52eb367c..c77ad68d 100644 --- a/src/runner/job.ts +++ b/src/runner/job.ts @@ -6,6 +6,7 @@ import { randomUUID } from "node:crypto"; import { type Round, ResponsesError } from "../cli/review"; +import { withModels } from "../github/models"; import type { Adapter, Exit, Running } from "./adapter"; import { Refused } from "./connection"; @@ -93,19 +94,6 @@ function subject(summary: string): string { /** `value` when it fits in a `## Models` line, otherwise `unknown`. */ const field = (value: string) => (/^[\w.:/@+-]{1,100}$/.test(value) ? value : "unknown"); -/** `body` with `line` as an item of its `## Models` section, which it gains when it has none. */ -function withModels(body: string, line: string): string { - const item = `- ${line}`; - if (body.split(/\r?\n/).some((each) => each.trim() === item)) return body; - const section = /^## Models[ \t]*\r?$/m.exec(body); - if (!section) return `${body.trimEnd()}\n\n## Models\n\n${item}\n`; - const start = section.index + section[0].length; - const next = /^## /m.exec(body.slice(start)); - const end = next ? start + next.index : body.length; - const gap = body.slice(start, end).trim() ? "\n" : "\n\n"; - return `${body.slice(0, end).trimEnd()}${gap}${item}\n${next ? "\n" : ""}${body.slice(end)}`; -} - function string(value: unknown, name: string, max: number): string { if (typeof value !== "string" || !value.trim()) throw new Error(`${name} is required`); if (value.length > max) throw new Error(`${name} is longer than ${max} characters`); From dda75a3e5363597469fe9706d117283e99a2f057 Mon Sep 17 00:00:00 2001 From: quest fixer Date: Wed, 4 Nov 2026 16:51:00 -0800 Subject: [PATCH 11/11] Remove completed implementer quest dependency --- quest/m0/model-routing/kix-sessions.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/quest/m0/model-routing/kix-sessions.md b/quest/m0/model-routing/kix-sessions.md index 439481f1..a4c45f2d 100644 --- a/quest/m0/model-routing/kix-sessions.md +++ b/quest/m0/model-routing/kix-sessions.md @@ -48,7 +48,3 @@ Implementation guidance: comment too, and `quest context`'s `## Models` rule and its `--exclude-model` rule ("each implementing model the PR body records") to name it. - -## Required - -- [Commits and PRs record their models](/quest/m0/model-routing/implementer.md) - this builds on its `## Models` record and the runner's reporting