Add the review agent's prompt, answer schema and lane D command (#201 part 1)

.github/workflows/agent-isolation.yml

@@ -10,8 +10,8 @@ jobs:
   real-docker:
     runs-on: ubuntu-latest
     # Covers the typecheck, the ordinary Vitest project, the 10 min image build and ordinary run times of the
-    # Docker-backed files. It does not cover every test and hook at its own Vitest limit: the five afterAll hooks that
-    # call the shared sweep allow 46 min between them (#189), which only a stalled daemon reaches.
+    # Docker-backed files. It does not cover every test and hook at its own Vitest limit: the six afterAll hooks that
+    # call the shared sweep allow 52 min between them (#189), which only a stalled daemon reaches.
     timeout-minutes: 45
     steps:
       - uses: actions/checkout@v4

.github/workflows/ci.yml

@@ -22,6 +22,6 @@ jobs:
       # The Docker-backed files run one at a time in the Agent isolation workflow; running them here would put them in
       # parallel against the same image tag and daemon. Exclude exactly the dockerTests list in vitest.config.ts;
       # test/vitest-config.test.ts fails if the two differ.
-      - run: npm test -- --exclude test/agent-container.test.ts --exclude test/agent-gate.test.ts --exclude test/agent-network.test.ts --exclude test/agent-planning.test.ts --exclude test/agent-question.test.ts --exclude test/agent-supervisor.test.ts --exclude test/runner-workspace.test.ts
+      - run: npm test -- --exclude test/agent-container.test.ts --exclude test/agent-gate.test.ts --exclude test/agent-network.test.ts --exclude test/agent-planning.test.ts --exclude test/agent-question.test.ts --exclude test/agent-review.test.ts --exclude test/agent-supervisor.test.ts --exclude test/runner-workspace.test.ts
       - run: npx playwright install --with-deps chromium
       - run: npm run test:browser

AGENTS.md

@@ -164,6 +164,7 @@ A check that an event changed nothing (a late answer, a stale poll, a reload rea
 - Neutralise issue references (`#N`, `GH-N`, `owner/repo#N`, issue URLs) in any text codeboost writes that can become a commit message, such as a PR title or description, and neutralise @-mentions in any of that text that is not fenced (a title). Code fences do not protect commit messages, and GitHub closes issues from closing keywords in default-branch commits.
 - Never spread a collection whose size follows unbounded input into function arguments (`Math.max(...runs)`); engines limit the argument count, so use a loop.
 - A hardened Git invocation must also keep Git out of nested repositories and populated submodules, whose own config and hooks are the agent's: pass `--ignore-submodules` on the command line (the config default does not bind plumbing or override `.gitmodules`), and never run Git with a nested repository as its working directory.
+- A literal pathspec still matches every path under it. A Git call that must show one file passes `:(exclude,literal)<path>/` too; where another path the call needs (a rename's other side) is under it, it excludes each other changed path under it by name instead. Test a file replaced by a directory, a type change, and a rename into a directory of its own name next to another changed file there.
 - Decide a filesystem fact (whether a name exists, where a link resolves, what a path's entries are) on the file system that will serve it, not on a copy. A host that folds case or Unicode normalization, or decodes names as text, answers differently from the container's; check links and names inside the container, as its kernel resolves them, by raw bytes.
 - Never infer that agent-written content is unchanged from timestamps, sizes or an index's stat cache: not every write moves them (tmpfs, for one). Compare the content itself.
 - Hand agent-chosen names to tools as literal data: disable or sidestep pathspec magic (a leading `:`), quote them in the tool's own input format (a leading `"` in `--stdin-paths`), pass them as bytes, and keep empty fields when splitting NUL-separated output. Check a name's safety only where it is reported, never refusing unchanged input for a name the report never carries.

agents/adapters/claude.ts

@@ -1,7 +1,7 @@
 import { join } from 'node:path';
-import type { InvocationHandle } from '../contract.ts';
+import type { InvocationHandle, Phase } from '../contract.ts';
 import { readCapturedFile } from '../container/profile.ts';
-import { createClaudeCommand, createPhasePolicy, MAX_COMMAND_SCHEMA_BYTES } from '../policy.ts';
+import { createClaudeCommand, createPhasePolicy, MAX_COMMAND_SCHEMA_BYTES, STRUCTURED_PHASES } from '../policy.ts';
 import { launchInvocation } from './supervisor.ts';
 import { setUpProfile } from './setup.ts';
 import { assertAdapterRequest, capAdapterInvocationBudget, createAdapterInvocationBudget,
@@ -48,7 +48,7 @@ function topLevelMembers(json: string): Map<string, string> {
 /**
  * Read Claude's `--output-format json` envelope. A result whose `subtype` is not `success`, or whose `is_error` is
  * true, is a provider failure; it can omit `result`, and when `result` is missing or empty its `errors` strings are
- * the text. With `--json-schema` (planning), a successful answer needs `subtype: "success"` and is the
+ * the text. With `--json-schema` (planning and review), a successful answer needs `subtype: "success"` and is the
  * schema-validated `structured_output` object. It is returned as the exact JSON text of that member in the CLI's
  * envelope, never re-encoded here, so the plan parser sees any duplicate key or inexact integer the envelope holds
  * (#166). Whether the API or CLI resolves those before writing the envelope is not verified. With a schema, an
@@ -84,11 +84,11 @@ export function parseClaudeOutput(raw: Buffer, structured = false): { text: stri
 }
 
 /**
- * Read the planning schema for `--json-schema` with the profile's bounded no-follow reader. The profile captures the
- * mounted file again and refuses a command whose schema differs.
+ * Read the planning or review schema for `--json-schema` with the profile's bounded no-follow reader. The profile
+ * captures the mounted file again and refuses a command whose schema differs.
  */
-export function readPlanningSchema(inputDirectory: string): string {
-  const { content } = readCapturedFile(join(inputDirectory, 'schema.json'), 'Planning schema', MAX_COMMAND_SCHEMA_BYTES);
+export function readAnswerSchema(inputDirectory: string, label: string): string {
+  const { content } = readCapturedFile(join(inputDirectory, 'schema.json'), label, MAX_COMMAND_SCHEMA_BYTES);
   return new TextDecoder('utf-8', { fatal: true }).decode(content);
 }
 
@@ -99,14 +99,18 @@ export function startClaudeInvocation(request: AgentAdapterRequest,
   const remaining = options.invocationBudget
     ? capAdapterInvocationBudget(options.invocationBudget, options.timeoutMs)
     : createAdapterInvocationBudget(request.invocation, options.timeoutMs);
-  const structured = policy.phase === 'planning';
+  const structured = STRUCTURED_PHASES.has(policy.phase);
   // Invalid input throws here, before anything is allocated; only Docker setup runs inside the handle.
   const command = createClaudeCommand(policy, request.prompt,
-    structured ? readPlanningSchema(request.inputDirectory) : undefined);
+    structured ? readAnswerSchema(request.inputDirectory, policy.phase === 'review' ? 'Review schema' : 'Planning schema') : undefined);
   assertAdapterRequest(request);
   return launchInvocation(request.invocation, remaining, (signal, start) => setUpProfile(request, remaining, signal,
     network => ({ ...request, policy, network, command, claudeToken: oauthToken }),
     profile => start(profile, { ...options, secrets: { CLAUDE_CODE_OAUTH_TOKEN: oauthToken },
       processLifecycle: request.processLifecycle,
-      invocationBudget: remaining, decode: (_profile, raw) => parseClaudeOutput(raw, structured) })));
+      invocationBudget: remaining, decode: (_profile, raw) => decodeClaudeAnswer(policy.phase, raw) })));
+}
+/** How a Claude invocation's output is read in `phase`: a planning or review answer from `structured_output` (#201). */
+export function decodeClaudeAnswer(phase: Phase, raw: Buffer): { text: string; providerFailed: boolean } {
+  return parseClaudeOutput(raw, STRUCTURED_PHASES.has(phase));
 }

agents/policy.ts

@@ -80,8 +80,9 @@ export function dispatchApprovedCommand<T>(policy: PhasePolicy, argv: readonly s
  * Planning's model budget (#163), checked when the command is built, before the adapter creates a container. The input
  * is counted in UTF-8 bytes of the prompt and schema: Claude's tokenizer is not available offline, and a count of bytes
  * bounds the tokens if every token covers at least one byte (assumed, not verified against Claude's tokenizer). 64 KiB
- * is the 32 KiB prompt plus the 32 KiB of schema text that core/planning-author.ts allows, so this is a backstop for
- * callers that skip it; it does not count Claude Code's own system prompt and tools, or the answer. The run is bounded
+ * is the 32 KiB prompt plus the 32 KiB of schema text that core/planning-author.ts allows, so for planning this is a
+ * backstop for callers that skip it. A review's prompt may use all of it less its schema (`MAX_REVIEW_PROMPT_BYTES`,
+ * #201). It does not count Claude Code's own system prompt and tools, or the answer. The run is bounded
  * by turns: if each turn sends at most one context window (assumed; the CLI's own calls, such as compaction, are not
  * counted), `--max-turns` bounds the tokens a run can use (the CLI may make one call more than the limit). Claude stops
  * at the limit with an `error_max_turns` result and exit status 1 (seen in a run of Claude Code 2.1.281 on macOS,
@@ -91,18 +92,25 @@ export function dispatchApprovedCommand<T>(policy: PhasePolicy, argv: readonly s
 export const MAX_PLANNING_INPUT_BYTES = 64 * 1024;
 export const PLANNING_MAX_TURNS = 50;
 
+/** The phases whose Claude answer is a JSON object that must match the request's schema: plans, and review problems (#201). */
+export const STRUCTURED_PHASES: ReadonlySet<Phase> = new Set(['planning', 'review']);
 /**
- * Planning answers must match the request's schema, so the planning command carries it. Claude's `--json-schema` takes
- * the schema text, not a path; the profile checks that text against the mounted file. Questions answer in plain text
- * (their schema is `{"type":"string"}`), and the other phases return no JSON answer, so they carry no schema.
+ * Planning and review answers must match the request's schema, so their commands carry it. Claude's `--json-schema`
+ * takes the schema text, not a path; the profile checks that text against the mounted file. Both have the same model
+ * budget. Questions answer in plain text (their schema is `{"type":"string"}`), and execute and fix return no JSON
+ * answer, so they carry no schema.
  */
 export function createClaudeCommand(policy: PhasePolicy, prompt: string, schema?: string): AgentCommand {
   if (!prompt || prompt.includes('\0')) throw new Error('Claude prompt must be nonempty and contain no NUL.');
   if (assertPhasePolicy(policy).vendor !== 'claude') throw new Error('Claude command requires a Claude invocation policy.');
-  if ((policy.phase === 'planning') !== (schema !== undefined))
-    throw new Error('Only the Claude planning command carries an answer schema, and it must.');
+  if (STRUCTURED_PHASES.has(policy.phase) !== (schema !== undefined))
+    throw new Error('Only the Claude planning and review commands carry an answer schema, and they must.');
+  // Claude has no tool here that runs approved argv (Bash is disallowed), so a review captured with any is a caller's
+  // mistake, refused rather than left to the tool lists (#201).
+  if (policy.phase === 'review' && assertPhasePolicy(policy).approvedArgv.length)
+    throw new Error('A Claude review runs no commands: capture it with no approved argv.');
   const planningArguments = schema === undefined ? [] : ['--json-schema',
-    assertPlanningInput(prompt, assertPlanningSchema(schema)), '--max-turns', String(PLANNING_MAX_TURNS)];
+    assertPlanningInput(prompt, assertPlanningSchema(schema, policy.phase), policy.phase), '--max-turns', String(PLANNING_MAX_TURNS)];
   const writable = policy.worktree === 'read-write';
   const allowed = writable ? 'Read,Glob,Grep,Edit,Write' : 'Read,Glob,Grep';
   // `--` ends option parsing, so a prompt beginning with `-` stays prompt data.
@@ -133,28 +141,33 @@ export function createRunnerCommand(policy: PhasePolicy, commands: string): Agen
   return command(policy, ['node', '/usr/local/bin/codeboost-command-check', '/run/codeboost-input/schema.json'], commands);
 }
 
-/** Refuse a planning request whose prompt and schema together exceed the model budget. Returns the schema unchanged. */
-export function assertPlanningInput(prompt: string, schema: string): string {
+/**
+ * Refuse a planning or review request whose prompt and schema together exceed the model budget. Returns the schema
+ * unchanged. A review shares planning's budget (#201).
+ */
+export function assertPlanningInput(prompt: string, schema: string, phase: Phase = 'planning'): string {
   const promptBytes = Buffer.byteLength(prompt, 'utf8'), schemaBytes = Buffer.byteLength(schema, 'utf8');
   if (promptBytes + schemaBytes > MAX_PLANNING_INPUT_BYTES)
-    throw new Error(`Planning request is ${promptBytes + schemaBytes} bytes (prompt ${promptBytes}, schema ${schemaBytes}), `
-      + `over the ${MAX_PLANNING_INPUT_BYTES}-byte model budget. Narrow the request (the issue and comments, the selected `
-      + 'paths, the lessons, the feedback or the previous plan), then retry.');
+    throw new Error(`${phase === 'review' ? 'Review' : 'Planning'} request is ${promptBytes + schemaBytes} bytes (prompt ${promptBytes}, schema ${schemaBytes}), `
+      + `over the ${MAX_PLANNING_INPUT_BYTES}-byte model budget. ${phase === 'review'
+        ? 'Build review requests with prepareReview (core/review-prompt.ts), which keeps them within it.'
+        : 'Narrow the request (the issue and comments, the selected paths, the lessons, the feedback or the previous plan), then retry.'}`);
   return schema;
 }
 
 /**
- * A planning schema must describe a JSON object (Claude returns `structured_output` as an object) and fit one command
- * argument. Returns the schema unchanged.
+ * A planning or review schema must describe a JSON object (Claude returns `structured_output` as an object) and fit one
+ * command argument. Returns the schema unchanged.
  */
-export function assertPlanningSchema(schema: string): string {
+export function assertPlanningSchema(schema: string, phase: Phase = 'planning'): string {
+  const name = phase === 'review' ? 'Review schema' : 'Planning schema';
   if (schema.includes('\0') || Buffer.byteLength(schema, 'utf8') > MAX_COMMAND_SCHEMA_BYTES)
-    throw new Error('Planning schema must contain no NUL and fit the command schema limit.');
+    throw new Error(`${name} must contain no NUL and fit the command schema limit.`);
   let parsed: unknown;
-  try { parsed = JSON.parse(schema); } catch { throw new Error('Planning schema must be JSON.'); }
+  try { parsed = JSON.parse(schema); } catch { throw new Error(`${name} must be JSON.`); }
   if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)
     || (parsed as { type?: unknown }).type !== 'object')
-    throw new Error('Planning schema must be a JSON object schema with "type": "object".');
+    throw new Error(`${name} must be a JSON object schema with "type": "object".`);
   return schema;
 }
 

core/planning-author.ts

@@ -46,16 +46,20 @@ export interface PreparedAuthor<T> {
 function integer(value: number, label: string): void {
   if (!Number.isSafeInteger(value) || value < 1) throw new Error(`${label} must be a positive safe integer.`);
 }
-function boundedText(value: string, label: string): string {
+/** A data block or one of its strings is over its size limit; any other refusal is a plain Error. */
+export class DataTooLarge extends Error {}
+function boundedText(value: string, label: string, limit = MAX_PROMPT_BYTES): string {
   if (typeof value !== 'string' || value.includes('\0') || !value.isWellFormed())
     throw new Error(`${label} must be valid text without NUL.`);
-  if (Buffer.byteLength(value, 'utf8') > MAX_PROMPT_BYTES) throw new Error(`${label} exceeds 32 KiB.`);
+  if (Buffer.byteLength(value, 'utf8') > limit) throw new DataTooLarge(`${label} exceeds ${limit === MAX_PROMPT_BYTES ? '32 KiB' : `${limit} bytes`}.`);
   return value;
 }
 /** Bound each field and the aggregate before serialization; never truncate source data.
- * Shared with the execution prompts (core/execution-prompt.ts). */
-export function dataJSON(value: unknown, label: string): string {
+ * Shared with the execution and review prompts (core/execution-prompt.ts, core/review-prompt.ts). Each string is at most
+ * 32 KiB; the whole block at most `limit` bytes (32 KiB unless a prompt with a larger budget asks for more). */
+export function dataJSON(value: unknown, label: string, limit = MAX_PROMPT_BYTES): string {
   let bytes = 0;
+  const over = () => new DataTooLarge(`${label} exceeds ${limit === MAX_PROMPT_BYTES ? '32 KiB' : `${limit} bytes`}.`);
   function check(item: unknown, depth: number): void {
     if (depth > 50) throw new Error(`${label} is too deep.`);
     if (typeof item === 'string') bytes += Buffer.byteLength(boundedText(item, label));
@@ -65,16 +69,16 @@ export function dataJSON(value: unknown, label: string): string {
     } else if (item === null || typeof item === 'boolean') bytes += 5;
     else if (Array.isArray(item)) {
       bytes += item.length + 2;
-      if (bytes > MAX_PROMPT_BYTES) throw new Error(`${label} exceeds 32 KiB.`);
+      if (bytes > limit) throw over();
       for (const child of item) check(child, depth + 1);
     } else if (typeof item === 'object' && Object.getPrototypeOf(item) === Object.prototype) {
       for (const [key, child] of Object.entries(item)) { check(key, depth + 1); check(child, depth + 1); }
     } else throw new Error(`${label} is not JSON data.`);
-    if (bytes > MAX_PROMPT_BYTES) throw new Error(`${label} exceeds 32 KiB.`);
+    if (bytes > limit) throw over();
   }
   check(value, 0);
   const encoded = JSON.stringify(value).replace(/[<>&]/gu, char => ({ '<': '\\u003c', '>': '\\u003e', '&': '\\u0026' })[char]!);
-  return boundedText(encoded, label);
+  return boundedText(encoded, label, limit);
 }
 
 export function prepareDraft(input: AuthorInput): PreparedAuthor<Plan> {