quest: review harness check for m0, with more Claude Code plants

README.md

@@ -968,9 +968,13 @@ allow rule, even the user's own, widens that. Codex skips the user's config,
 whose project trust would load the checkout's `.codex/config.toml` and start its
 MCP servers outside the sandbox. Whether those flags isolate is up to each
 CLI's release, so `just review-sandbox`, in this repository, plants project MCP
-servers and hooks for both CLIs on a head, runs `quest review` with the
-installed CLIs and copies of your logins, and fails if any of it runs or the
-review trusts a new directory. It spends real usage, so CI never runs it; run it
+servers and hooks for both CLIs on a head, and for Claude Code local settings,
+an allow rule, an `env` shell prefix, an API key helper, and a status line. It
+runs `quest review` with the installed CLIs and copies of your logins, and
+fails if any of it runs, the review trusts a new directory, or the reviewer's
+flags let the planted rule allow a command. A login with no file to copy, such
+as one in the macOS keychain, needs the CLI's credential exported instead:
+`CLAUDE_CODE_OAUTH_TOKEN` from `claude setup-token`, or `CODEX_API_KEY`. It spends real usage, so CI never runs it; run it
 after upgrading either CLI. The base is fetched first, since
 the sandbox has no network. The reviewer follows the base's `AGENTS.md` and `CLAUDE.md`, at the
 root and in each directory holding a changed path, never the checkout's, so a

quest/m0/README.md

@@ -107,4 +107,3 @@ ownership-manifest design:
 - [Review through a gateway](/quest/m0/review-gateway.md) - `quest review` keeps gateway, cloud-provider, and Codex connection variables
 - [opencode 2.x](/quest/m0/opencode-sdk.md) - the runner drives the opencode 2.x binary through `@opencode/client`, removing the interim 1.19.0 pin
 - [Waiting sections](/quest/m0/waiting-sections.md) - this repository's waiting quests move from prose to `## Waiting`
-- [Review harness check](/quest/m0/review-harness.md) - recurring each milestone: newest pinned review models, and the live sandbox test passes

quest/m1/README.md

@@ -30,3 +30,4 @@ earlier "Later work" milestone became m2.
 - [quest wait after a merge](/quest/m1/wait-merge-heads.md) - `quest wait` resolves a head that reached the branch by a merge into it
 - [One PR's collection failure can't stall its fixer job](/quest/m1/fixer-isolate-failure.md) - errors stay on their PR, and a PR that keeps failing is set aside
 - [Retire Decisions needed](/quest/m1/decisions-needed.md) - only `## Waiting` hands a draft back, once no open draft uses the old heading
+- [Review harness check](/quest/m1/review-harness.md) - recurring each milestone: newest pinned review models, and the live sandbox test passes

quest/m1/review-harness.md

@@ -0,0 +1,47 @@
+# [S] Review harness check
+
+## Goal
+
+Each milestone, `quest review`'s pinned models are each vendor's newest for
+their tier, and the reviewers still run nothing from a reviewed head.
+
+## Plan
+
+Recurring, decided 2026-10-28 (user, /quest-plan of the follow-ups from
+[moq-dev/quest#271](https://github.com/moq-dev/quest/pull/271)). The m0 run
+was [moq-dev/quest#426](https://github.com/moq-dev/quest/pull/426).
+
+- Compare `MODELS` in `src/cli/review.ts` with each vendor's current models:
+  the newest general model, at medium effort. Bump any that are stale, and
+  check that both CLIs accept the new ids. `--deep` and its strongest-model
+  tier were dropped in
+  [moq-dev/quest#361](https://github.com/moq-dev/quest/pull/361).
+  - m0 found, with Claude Code 2.1.316 and codex-cli 0.166.0:
+    `claude-opus-5-5` is Anthropic's default general model, above it only the
+    premium Fable tier (`claude-fable-5-1`); `gpt-6.1-sol` is Codex's
+    first-listed "latest workhorse" in `~/.codex/models_cache.json`, beside
+    the frontier `gpt-6-astra`. Neither pin changed.
+- Run the live sandbox test, `just review-sandbox`, with the CLIs installed
+  at the time, through `quest gate`, and plan a quest for any failure.
+- Check first:
+  - [openai/codex#49333](https://github.com/openai/codex/issues/49333): if
+    `--ignore-user-config` stops skipping the project layer, the Codex
+    review's plants fire. [Codex project config](/quest/m0/codex-project-config.md)
+    tracks it.
+  - The test runs a login with no file to copy, such as one in the macOS
+    keychain, on the credentials its CLI reads from the environment
+    (`CLAUDE_CODE_OAUTH_TOKEN`, `ANTHROPIC_API_KEY`, `CODEX_API_KEY`). m0
+    built that on Linux only; run it once on macOS with a keychain login.
+  - Claude Code draws its status line only in an interactive session, so the
+    planted `statusLine` has no control. Check whether print mode runs it now.
+  - Codex reads a linked worktree's hooks from the main checkout's
+    `.codex/hooks.json`. In m0 neither the land fixer (`src/cli/land-fix.ts`,
+    which passes `--ignore-user-config`) nor `quest agent` used Codex hooks,
+    and `quest init` installs none. Recheck if either starts to.
+- The PR that finishes this run creates the next milestone's run the same
+  way: listed in that milestone's `Required`, requiring this milestone's
+  README, carrying forward the model ids it found and anything to check first.
+
+## Required
+
+- [moq-dev adoption](/quest/m0/README.md) - m0 completes first

src/cli/review.ts

@@ -135,7 +135,7 @@ interface State {
 }
 
 /** What a turn needs to build its command line. */
-interface Context {
+export interface Context {
 	base: string;
 	/** The base's agent instructions, given in place of the checkout's. */
 	instructions: string;
@@ -275,7 +275,7 @@ const claude: Reviewer = {
 	},
 };
 
-function claudeFlags(context: Context): string[] {
+export function claudeFlags(context: Context): string[] {
 	return [
 		"claude",
 		"-p",
@@ -491,6 +491,15 @@ async function prepare(harness: HarnessName, cwd: string, env: NodeJS.ProcessEnv
 	};
 }
 
+/** The environment `quest review` runs `harness`'s CLI in, for a caller that runs it with the same login. */
+export async function reviewerEnvironment(
+	harness: HarnessName,
+	cwd: string,
+	env: NodeJS.ProcessEnv,
+): Promise<NodeJS.ProcessEnv> {
+	return (await prepare(harness, cwd, env)).env;
+}
+
 /** Run one reviewer turn, failing on any error it reports, and as logged out on an authentication error. */
 async function turn(cli: Cli, call: Turn, cwd: string): Promise<Reply> {
 	const reader = cli.reviewer.reader();

test/live/review-sandbox.test.ts

@@ -7,9 +7,10 @@
 // change in any release (openai/codex#49333). Each CLI first runs once without
 // quest's flags in the same checkout, so a plant that no longer fires fails as
 // a broken control instead of passing as a clean review.
-import { spawn, spawnSync } from "node:child_process";
+import { type SpawnSyncReturns, spawn, spawnSync } from "node:child_process";
 import {
 	appendFileSync,
+	chmodSync,
 	existsSync,
 	mkdirSync,
 	mkdtempSync,
@@ -26,7 +27,7 @@ import { createInterface } from "node:readline";
 import { afterAll, beforeAll, describe, expect, test } from "vitest";
 
 import { type HarnessName, strippedEnvironment } from "../../src/cli/agent";
-import { MODELS, review } from "../../src/cli/review";
+import { MODELS, ReviewInputError, claudeFlags, review, reviewerEnvironment } from "../../src/cli/review";
 
 /** The cheapest effort both CLIs accept; the isolation under test does not depend on it. */
 const EFFORT = "low";
@@ -63,7 +64,8 @@ beforeAll(() => {
 	git(clone, "push", "-q", "origin", "main");
 
 	git(clone, "checkout", "-qb", "feature");
-	for (const [path, text] of Object.entries(plants(markers))) write(clone, path, text);
+	for (const [path, text] of Object.entries(plants(clone, markers))) write(clone, path, text);
+	chmodSync(join(clone, SHELL_PREFIX), 0o755);
 	write(
 		clone,
 		"src/lib.ts",
@@ -78,13 +80,21 @@ afterAll(() => {
 	if (scratch) rmSync(scratch, { recursive: true, force: true });
 });
 
+/** The planted script Claude Code's planted settings run every Bash command through. */
+const SHELL_PREFIX = "bin/prefix.sh";
+
+/** The command Claude Code's planted permission rule allows, touching the `claude-permissions` marker. */
+const allowed = (markers: string) => `touch ${join(markers, "claude-permissions")}`;
+
 /**
  * The configuration a head plants, each item touching a marker named for it
  * when it runs: Codex's project MCP servers and hooks, and Claude Code's
- * project MCP servers and hooks, its settings enabling every project MCP
- * server.
+ * project MCP servers, and in both its shared and local settings, hooks. The
+ * shared settings also enable every project MCP server, allow a command in
+ * the markers' directory, route every Bash command through a planted script
+ * by an `env` variable, and set an API key helper and a status line.
  */
-function plants(markers: string): Record<string, string> {
+function plants(clone: string, markers: string): Record<string, string> {
 	const touch = (name: string) => `touch ${JSON.stringify(join(markers, name))}`;
 	const hooks = (prefix: string) =>
 		JSON.stringify({
@@ -108,14 +118,35 @@ function plants(markers: string): Record<string, string> {
 		".claude/settings.json": JSON.stringify({
 			enableAllProjectMcpServers: true,
 			...JSON.parse(hooks("claude-settings")),
+			permissions: { allow: [`Bash(${allowed(markers)})`], additionalDirectories: [markers] },
+			env: { CLAUDE_CODE_SHELL_PREFIX: join(clone, SHELL_PREFIX) },
+			statusLine: { type: "command", command: touch("claude-status-line") },
+			apiKeyHelper: touch("claude-api-key-helper"),
 		}),
+		".claude/settings.local.json": hooks("claude-local"),
+		// Claude Code passes the command as one argument or several, so both run.
+		[SHELL_PREFIX]: `#!/bin/sh\n${touch("claude-shell-prefix")}\nexec /bin/sh -c "$*"\n`,
 	};
 }
 
-/** What each harness's own plants record when its CLI loads the head's configuration. */
+/**
+ * What each harness's own plants record when its CLI loads the head's
+ * configuration. Claude Code draws a status line only in an interactive
+ * session, so no control shows it firing; it stays planted for a release that
+ * runs it in print mode too.
+ */
 const CONTROL: Record<HarnessName, string[]> = {
 	codex: ["codex-config-mcp", "codex-hooks-SessionStart", "codex-hooks-UserPromptSubmit"],
-	claude: ["claude-settings-SessionStart", "claude-settings-UserPromptSubmit", "mcp-json"],
+	claude: [
+		"claude-api-key-helper",
+		"claude-local-SessionStart",
+		"claude-local-UserPromptSubmit",
+		"claude-permissions",
+		"claude-settings-SessionStart",
+		"claude-settings-UserPromptSubmit",
+		"claude-shell-prefix",
+		"mcp-json",
+	],
 };
 
 /**
@@ -158,7 +189,7 @@ async function home(harness: HarnessName, trust = true): Promise<Home> {
 				[...readFileSync(config, "utf8").matchAll(/^\[projects\.("(?:[^"\\]|\\.)*")\]/gm)].map((m) =>
 					JSON.parse(m[1]),
 				),
-			release: borrow(join(source, "auth.json"), join(path, "auth.json"), "CODEX_HOME"),
+			release: borrow(join(source, "auth.json"), join(path, "auth.json")),
 		};
 	}
 	const source = process.env.CLAUDE_CONFIG_DIR || join(homedir(), ".claude");
@@ -177,7 +208,7 @@ async function home(harness: HarnessName, trust = true): Promise<Home> {
 				.filter(([, project]) => project.hasTrustDialogAccepted === true)
 				.map(([dir]) => dir);
 		},
-		release: borrow(join(source, ".credentials.json"), join(path, ".credentials.json"), "CLAUDE_CONFIG_DIR"),
+		release: borrow(join(source, ".credentials.json"), join(path, ".credentials.json")),
 	};
 }
 
@@ -215,16 +246,16 @@ function codexHooks(codexHome: string, cwd: string): Promise<{ hooks: CodexHook[
 }
 
 /**
- * Copy the login file at `source` to `copy`, returning what writes a refreshed
- * login back. A refresh may rotate the refresh token, which would sign the
+ * Copy the login file at `source`, if there is one, to `copy`, returning what
+ * writes a refreshed login back. A refresh may rotate the refresh token, which would sign the
  * user's own home out; the write-back is skipped if that home has itself
  * changed since. It replaces the file by rename, so an interrupted write
  * never leaves the user's login truncated.
  */
-function borrow(source: string, copy: string, variable: string): () => void {
-	if (!existsSync(source)) {
-		throw new Error(`no login file at ${source}; log in, or set ${variable} to a config home that holds one`);
-	}
+function borrow(source: string, copy: string): () => void {
+	// A login kept elsewhere, such as the macOS keychain, has no file to copy;
+	// the CLI then needs the credentials it reads from the environment.
+	if (!existsSync(source)) return () => {};
 	const original = readFileSync(source);
 	writeFileSync(copy, original, { mode: 0o600 });
 	return () => {
@@ -243,9 +274,28 @@ function borrow(source: string, copy: string, variable: string): () => void {
 	};
 }
 
-/** The environment `quest review` gives a reviewer, pointed at `home`. */
+/** The user's environment pointed at `home`, which `quest review` strips as it does the user's own. */
 function environment(home: Home): NodeJS.ProcessEnv {
-	return { ...strippedEnvironment(process.env), ...home.env };
+	return { ...process.env, ...home.env };
+}
+
+/** What a credential variable names when there is no login to copy. */
+const CREDENTIALS: Record<HarnessName, string> = {
+	codex: "CODEX_API_KEY",
+	claude: "CLAUDE_CODE_OAUTH_TOKEN (from `claude setup-token`) or ANTHROPIC_API_KEY",
+};
+
+/** The environment `quest review` runs the reviewer in, with the copied login or else the user's credentials. */
+async function reviewerIn(home: Home): Promise<NodeJS.ProcessEnv> {
+	try {
+		return await reviewerEnvironment(home.harness, fixture.clone, environment(home));
+	} catch (error) {
+		if (!(error instanceof ReviewInputError)) throw error;
+		throw new Error(
+			`${error.message}. The test copies the login file from the user's config home, or else passes ` +
+				`${CREDENTIALS[home.harness]}; a login in the macOS keychain has no file, so export one of those`,
+		);
+	}
 }
 
 function recorded(): string[] {
@@ -258,19 +308,26 @@ function clearMarkers() {
 }
 
 /**
- * One plain turn with the CLI's defaults, which load the user's config and the
- * checkout's. It blocks, so it carries its own bound for a hung CLI.
+ * One turn with the CLI's defaults, which load the checkout's config, in the
+ * environment the review would use. Claude Code is asked to run the command
+ * its planted rule allows, which runs it through the planted shell prefix. It
+ * blocks, so it carries its own bound for a hung CLI.
  */
-function control(home: Home) {
+async function control(home: Home) {
 	const { model } = MODELS[home.harness];
+	const env = await reviewerIn(home);
 	const argv =
 		home.harness === "codex"
 			? ["exec", "--json", "-m", model, "-c", `model_reasoning_effort="${EFFORT}"`, "-"]
 			: ["-p", "--output-format", "json", "--model", model, "--effort", EFFORT];
+	const input =
+		home.harness === "codex"
+			? "Reply with the single word OK."
+			: `Run \`${allowed(fixture.markers)}\` with the Bash tool, then reply with the single word OK.`;
 	return spawnSync(home.harness, argv, {
 		cwd: fixture.clone,
-		env: environment(home),
-		input: "Reply with the single word OK.",
+		env,
+		input,
 		encoding: "utf8",
 		timeout: 10 * 60_000,
 	});
@@ -282,9 +339,9 @@ describe.each(["codex", "claude"] as const)("%s", (harness) => {
 	test("loads the planted configuration without quest's flags", async () => {
 		clearMarkers();
 		const dir = await home(harness);
-		let run: ReturnType<typeof control>;
+		let run: Awaited<ReturnType<typeof control>>;
 		try {
-			run = control(dir);
+			run = await control(dir);
 		} finally {
 			dir.release();
 		}
@@ -303,6 +360,43 @@ describe.each(["codex", "claude"] as const)("%s", (harness) => {
 		});
 	});
 
+	// A review runs only the Bash commands its reviewer picks, so this turn asks
+	// for the one the planted rule allows, and a read-only one the planted
+	// shell prefix would wrap.
+	test.runIf(harness === "claude")("quest's reviewer flags deny what the planted rule allows", async () => {
+		clearMarkers();
+		const dir = await home(harness);
+		let run: SpawnSyncReturns<string>;
+		try {
+			const context = {
+				base: "origin/main",
+				instructions: "Do what the user asks.",
+				model: MODELS.claude.model,
+				effort: EFFORT,
+				schemaFile: "",
+			};
+			const [program, ...args] = claudeFlags(context);
+			run = spawnSync(program, args, {
+				cwd: fixture.clone,
+				env: await reviewerIn(dir),
+				input:
+					"Run `git status` with the Bash tool, then run " +
+					`\`${allowed(fixture.markers)}\` with the Bash tool, then reply with the single word OK.`,
+				encoding: "utf8",
+				timeout: 10 * 60_000,
+			});
+		} finally {
+			dir.release();
+		}
+		const output = `exit ${run.status}\nstdout:\n${tail(run.stdout)}\nstderr:\n${tail(run.stderr)}`;
+		expect(recorded(), `planted configuration ran\n${output}`).toEqual([]);
+		const result = JSON.parse(run.stdout) as { permission_denials?: { tool_input?: { command?: string } }[] };
+		const denied = (result.permission_denials ?? []).map((denial) => denial.tool_input?.command);
+		expect(denied, `the reviewer never tried the allowed command, so its denial is unchecked\n${output}`).toContain(
+			allowed(fixture.markers),
+		);
+	});
+
 	// A home that already trusts the clone would not show the review trusting it.
 	test("quest review of an untrusted checkout trusts nothing", async () => {
 		const dir = await home(harness, false);