E4: planning acceptance fixtures and recorded Claude output

docs/implementation/planning-acceptance.md

@@ -0,0 +1,85 @@
+# E4 planning acceptance and remaining gate
+
+The dedicated E4 suite composes E2/E3 with the real SQLite Store. The checked-in
+plan, edit cards and hostile text under `test/fixtures/planning/` are **synthetic**.
+They are not captured Claude/Codex responses and do not establish live adapter,
+semantic prompt-injection resistance, Docker isolation or product performance.
+Only files under `test/fixtures/planning/recorded/` are vendor output.
+
+Runnable checks:
+
+```sh
+npm test
+npm run typecheck
+npm run test:browser
+```
+
+The PR body records the final pushed head and observed counts. The browser suite
+is the existing integrated review baseline, not planning-screen acceptance (G).
+
+| Acceptance | Evidence |
+| --- | --- |
+| JSON/YAML import, selected issue, eight broken plans, atomic failed import | `planning-acceptance.test.ts` against real Store |
+| Immutable historical revisions, replay, sibling invalidation and stable identity | E4 reopens SQLite and tries repository/task/plan mismatches; `store.test.ts` additionally races Apply in independent processes |
+| Malformed extracted output and invalid resulting cards never become ready | E4 asserts returned failure, durable failed reason, null reply and unchanged plan revision |
+| Hostile filenames, branches, argv, issue/comments, lessons and feedback | E4 decodes each prompt block at the provider boundary and asserts exact source data plus escaped delimiters; no recursive rendering |
+| Initial/revised draft identity and revision; input/UTF-8/prompt budgets | `planning-author.test.ts`, including exact 32 KiB boundary and post-escaping expansion |
+| Parser depth/size, duplicates, aliases, tags, safe numbers and registry copies | Existing frozen `plan-v1.test.ts`, `registry.test.ts`, `plan.test.ts` reused |
+| Late response, cancellation, timeout ownership, shutdown admission, input changes | `planning-suggestions.test.ts` controlled promises/timers with real Store |
+| Unapproved appended flags | E4 asserts `command-not-allowed` and exact-argv denial; no execution occurs in E |
+
+## Recorded vendor output
+
+D5 has merged. Codex is refused in every phase (#75, #93): it reads files only
+through its shell, and planning runs no process. So only Claude is recorded, and
+the recorded-output part of T9/T18 needs one manual run of
+`scripts/record-planning.ts` with a Claude token:
+
+```sh
+CODEBOOST_RUN_AUTH_PROBES=1 CLAUDE_CODE_OAUTH_TOKEN=... node scripts/record-planning.ts
+```
+
+The script sends one draft request and one suggestion request through
+`runner/planning-provider.ts`. That provider uses D's container, the read-only
+planning phase and vendor-only egress. D passes the request schema to Claude as
+`--json-schema` and returns Claude's `structured_output` as JSON text. The script
+saves that text exactly as D returned it to
+`test/fixtures/planning/recorded/claude-<mode>.json`, with the image tag, CLI
+version, and hashes of the repository files, prompt, schema and the exact argv D
+ran Claude with. It saves before it validates, and it refuses
+to write output that contains the token. It removes the token from its own
+environment before any Docker call, and replaces it in every error it prints. Read each file before you commit it.
+Never put credentials in a fixture, an issue or a PR.
+
+The adapter passes no model flag, so each recording names the CLI's default model,
+not a pinned one.
+
+| Acceptance | Evidence |
+| --- | --- |
+| Real output validates and persists | `planning-recorded.test.ts` replays the draft into the Store and the suggestion through E3, then reopens SQLite |
+| A recording matches today's request | The same test fails when the repository files, prompt, schema, Claude argv (flags, tool sets, `--json-schema`), image tag or CLI version changes; record again. Other image changes, such as Dockerfile packages, are not detected |
+| Recordings exist | The same test fails until both files exist |
+| Provider runs only read-only Claude planning and settles before release | `planning-provider.test.ts` with injected lane D stand-ins: Codex is refused before any Docker or Git work; a result for another attempt or context is refused; storage Docker did not remove, or that D returned no handle for, keeps planning off, with planning's own refusals |
+| Missing, malformed, error and boundary-size envelopes | `planning-extraction.test.ts`: structured envelopes through `parseClaudeOutput`, then E3 and the Store at exactly 1 MiB and 1 MiB + 1 byte |
+
+The edit schema caps every text field, so no valid reply is near 1 MiB. The size
+tests therefore show which check rejects the text: at exactly 1 MiB the schema,
+and at 1 MiB + 1 byte the size limit. If a real output fails validation, fix the
+prompt or schema and record again. Do not edit the recording.
+
+Left for F's wiring: the provider's image build, clone and storage calls are
+synchronous, so a server must run it off its request thread, as Ask does. That
+worker should also set TMPDIR to a planning root, as Ask's does, so the input and
+auth staging that lane D leaves when it gives up on cleanup is covered. At that
+point the provider and `askInContainer` should share one container runner; they
+are separate copies today. E3's
+default 120-second suggestion timeout is shorter than the provider's ten-minute
+budget. Startup recovery does not yet remove planning's Docker resources after a
+crash, because they are not attempt allocations in the Store. The recording
+script prints its runner label if it leaves any.
+
+F/G still own live API and UI integration. The Store now guarantees revision/snapshot
+binding and pending-only settlement across processes, and E3 requires the caller's
+snapshot identity before admission. G must not treat these fixture-only results as a
+completed production planning milestone. T18/T9 remain incomplete until all assigned
+lane slices pass their original acceptance.

runner/planning-provider.ts

@@ -0,0 +1,186 @@
+import { randomUUID } from 'node:crypto';
+import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { basename, dirname, join } from 'node:path';
+import type { InvocationContext, InvocationInput, InvocationResult, StopReason } from '../agents/contract.ts';
+import type { TaskFilesystems, TaskStorageLimits } from '../agents/container/storage.ts';
+import type { AuthorProvider, AuthorRequest } from '../core/planning-author.ts';
+import { allocations, RetainedStorage, stopOf, type ContainerDependencies, type LeftoverPolicy, type Provider } from './question-container.ts';
+
+/** Ask's lane D dependencies, less startup recovery, which the runner owns for planning. */
+export type PlanningDependencies = Omit<ContainerDependencies, 'recover'>;
+
+/** What the runner knows about the checkout a plan is written against. */
+export interface PlanningProviderOptions {
+  /** The vendor chosen in Settings. Only Claude can plan; Codex is refused before any work. */
+  readonly vendor: Provider;
+  /** Host checkout to clone. The agent sees a read-only copy of `head`, never this directory. */
+  readonly repository: string;
+  /** The committed head the plan's snapshot was taken from. */
+  readonly head: string;
+  readonly snapshotId: string;
+  /** Written as `io.codeboost.runner` on every Docker object an invocation creates. */
+  readonly runnerOwner: string;
+  /** Whole-invocation budget, setup included. Lane D caps the invocation itself at ten minutes. */
+  readonly timeoutMs?: number;
+  readonly deps: PlanningDependencies;
+  /** Shared across providers in one process, so the image is built once. */
+  readonly image?: { id?: string };
+  /** What earlier requests left behind. Invocations stay refused while any of it remains. */
+  readonly retained?: PlanningStorage;
+}
+
+/**
+ * Planning's leftovers: lane D allocations, allocations lane D returned no handle for, and host copies of the planned
+ * code. Use one instance for planning and never share it with Ask, so a planning failure never turns Ask off.
+ */
+export const PLANNING_LEFTOVERS: LeftoverPolicy = Object.freeze({
+  removePath: removePlanningRoot,
+  paths: (paths: readonly string[]) => `A copy of the planned code from an earlier request could not be deleted (${paths.join(', ')}). Planning stays off until it is deleted.`,
+  untracked: () => 'A planning agent\'s setup or cleanup failed and was not confirmed, so codeboost cannot tell which Docker resources were left. Planning is off until codeboost restarts.',
+  retained: (count: number) => `Planning storage from an earlier request could not be removed (${allocations(count)}). Planning stays off until Docker removes it. Check that Docker is running, then retry.`,
+});
+/** Planning's own leftovers. Typed apart from Ask's, so an instance with Ask's wording and removal cannot be passed in. */
+export class PlanningStorage extends RetainedStorage {
+  readonly feature = 'planning';
+  constructor() { super(PLANNING_LEFTOVERS); }
+}
+
+const ROOT_PREFIX = 'codeboost-planning-';
+/**
+ * Delete a planning root: a direct child of the temporary directory named with planning's prefix, and nothing else.
+ * Its input directory is made writable first, since the request left it read-only.
+ */
+export function removePlanningRoot(root: string): void {
+  if (dirname(root) !== tmpdir() || !basename(root).startsWith(ROOT_PREFIX))
+    throw new Error(`Refusing to remove a path that is not a planning root (${root}).`);
+  try { chmodSync(join(root, 'input'), 0o700); } catch { /* never created, or already removed */ }
+  rmSync(root, { recursive: true, force: true, maxRetries: 3, retryDelay: 50 });
+}
+
+// Planning reads the code; it needs no room to write. The same ceilings as Ask.
+export const PLANNING_STORAGE: TaskStorageLimits = Object.freeze({
+  workBytes: 512 * 1024 * 1024, workInodes: 131_072, metadataBytes: 512 * 1024 * 1024, metadataInodes: 131_072,
+});
+const DEFAULT_TIMEOUT_MS = 10 * 60_000;
+
+/** Codex reads files only through its shell, and planning runs no process, so Codex cannot plan (#75, #93). */
+export const CODEX_PLANNING_REFUSED = 'Codex cannot write plans yet: it can read the code only by running commands, '
+  + 'and planning runs none. Choose Claude Code in Settings, then retry.';
+
+export function planningCredential(vendor: Provider, env: PlanningDependencies['env']): string {
+  if (vendor === 'codex') throw new Error(CODEX_PLANNING_REFUSED);
+  const token = env.CLAUDE_CODE_OAUTH_TOKEN;
+  if (!token) throw new Error('Planning with Claude Code needs CLAUDE_CODE_OAUTH_TOKEN. Create one with `claude setup-token`.');
+  return token;
+}
+
+const stopMessages: Record<StopReason, string> = {
+  cancelled: 'Planning agent cancelled.', timeout: 'Planning agent timed out.', shutdown: 'Server stopped during planning.',
+  'output-limit': 'Planning agent output exceeded its limit.', 'capture-failure': 'Planning agent output could not be captured.',
+};
+const sameContext = (left: InvocationContext, right: InvocationContext) =>
+  (Object.keys(right) as (keyof InvocationContext)[]).every(key => left[key] === right[key])
+  && Object.keys(left).length === Object.keys(right).length;
+/** Accept only this exact invocation's output, and only after a clean exit. The text is still unvalidated. */
+export function planningOutput(result: InvocationResult, invocation: InvocationInput): string {
+  if (result.attemptId !== invocation.attemptId || !result.context || !sameContext(result.context, invocation.context))
+    throw new Error('The planning agent returned a result for a different attempt.');
+  if (result.stopReason) throw new Error(stopMessages[result.stopReason]);
+  if (result.exitCode === null || result.signal !== null)
+    throw new Error(`Claude stopped unexpectedly${result.signal ? ` (${result.signal})` : ''}.`);
+  if (result.exitCode !== 0) {
+    const detail = result.stdout.replace(/\s+/g, ' ').trim().slice(0, 300);
+    throw new Error(`Claude could not write the plan. Check its sign-in and usage limits.${detail ? ` Claude said: ${detail}` : ''}`);
+  }
+  return result.stdout;
+}
+
+/**
+ * E2's provider over lane D: each request runs in a fresh container on a read-only copy of `head`, in the "planning"
+ * phase (read, list and search only; no commands) with vendor-only egress. The request's schema is the only file in
+ * the input mount; D passes it to Claude as `--json-schema` and returns Claude's `structured_output` as JSON text. That
+ * text is still unvalidated; E2's `validate` decides whether it is a plan. Codex is refused before any work.
+ *
+ * Lane D's image build, clone and storage allocation are synchronous Docker and Git calls. A serving process must run
+ * this provider off its request thread, as Ask does in its worker.
+ */
+export interface PlanningProvider extends AuthorProvider {
+  invoke(request: AuthorRequest, signal: AbortSignal): Promise<string>;
+}
+export function createPlanningProvider(options: PlanningProviderOptions): PlanningProvider {
+  const { vendor, deps } = options, image = options.image ?? {}, retained = options.retained ?? new PlanningStorage();
+  const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS;
+  if (!Number.isSafeInteger(timeoutMs) || timeoutMs < 1) throw new Error('Planning timeout must be a positive integer.');
+  return { async invoke(request: AuthorRequest, signal: AbortSignal): Promise<string> {
+    if (request.phase !== 'planning' || request.access !== 'read-only') throw new Error('Planning requests must be read-only.');
+    const deadline = Date.now() + timeoutMs;
+    const remaining = () => {
+      signal.throwIfAborted();
+      const value = deadline - Date.now();
+      if (value < 1) throw new Error(stopMessages.timeout);
+      return value;
+    };
+    signal.throwIfAborted();
+    const credential = planningCredential(vendor, deps.env);
+    retained.release(deps.removeFilesystems);
+    image.id ??= deps.buildImage(remaining());
+    const root = mkdtempSync(join(tmpdir(), ROOT_PREFIX));
+    const staging = join(root, 'staging'), input = join(root, 'input');
+    let filesystems: TaskFilesystems | undefined, failure: { error: unknown } | undefined;
+    try {
+      mkdirSync(staging); mkdirSync(input);
+      writeFileSync(join(input, 'schema.json'), request.schemaText, { mode: 0o444 });
+      chmodSync(input, 0o555);
+      // The clone is a full host copy with no byte limit of its own, so the repository must fit before it is made.
+      const size = deps.measureRepository(options.repository, options.head, Math.min(60_000, remaining()));
+      if (size.checkoutBytes > PLANNING_STORAGE.workBytes || size.entries > PLANNING_STORAGE.workInodes
+        || size.objectBytes > PLANNING_STORAGE.metadataBytes)
+        throw new Error('The repository is too large for planning.');
+      const clone = deps.createClone({ source: options.repository, parent: staging, taskId: `planning-${request.requestId}`,
+        head: options.head, timeoutMs: Math.min(120_000, remaining()) });
+      const owner = { runnerOwner: options.runnerOwner, attemptId: request.requestId, allocationId: randomUUID() };
+      try { filesystems = deps.prepareFilesystems(clone, PLANNING_STORAGE, image.id, owner, Math.min(60_000, remaining())); }
+      catch (error) {
+        // D throws an AggregateError only when a failed allocation's own cleanup did not settle; it returns no handle.
+        if (error instanceof AggregateError) retained.markUntracked();
+        throw error;
+      }
+      remaining();
+      const invocation = deps.capture({ clone, phase: 'planning', vendor: 'claude', approvedArgv: [], deadline,
+        attemptId: request.requestId, runnerOwner: options.runnerOwner,
+        context: { snapshotId: options.snapshotId, planId: request.identity.planId, planRevision: request.revision,
+          assignmentId: `${request.mode}-${request.issue}`, referencedCodeHash: options.head, stateVersion: 0 } });
+      const adapterRequest = { invocation, filesystems, inputDirectory: input, imageId: image.id, prompt: request.prompt,
+        networkAllocationId: randomUUID() };
+      const handle = deps.startClaude(adapterRequest, credential);
+      const cancel = () => handle.cancel(stopOf(signal.reason));
+      if (signal.aborted) cancel(); else signal.addEventListener('abort', cancel, { once: true });
+      let result: InvocationResult;
+      try { result = await handle.settled; }
+      finally { signal.removeEventListener('abort', cancel); }
+      // D stopped before confirming cleanup; the resources are found again by their labels after a restart.
+      if (result.unreleased !== undefined) retained.markUntracked();
+      return planningOutput(result, invocation);
+    } catch (error) {
+      failure = { error };
+      throw error;
+    } finally {
+      const failures: unknown[] = [];
+      if (filesystems) try { deps.removeFilesystems(filesystems); } catch (error) { retained.retain(filesystems); failures.push(error); }
+      try { removePlanningRoot(root); }
+      catch (error) {
+        retained.retainPath(root);
+        failures.push(new Error(`A copy of the planned code could not be deleted (${root}).`, { cause: error }));
+      }
+      // What cleanup left is retained above, and the next request names it. A successful request is refused rather than
+      // returned, as Ask does, so a plan is never kept from an unsettled run. A failed one keeps its own message first.
+      if (failures.length) {
+        const text = (error: unknown) => error instanceof Error ? error.message : String(error);
+        throw failure
+          ? new AggregateError([failure.error, ...failures], `${text(failure.error)} Cleanup also did not settle: ${failures.map(text).join('; ')}`)
+          : new AggregateError(failures, 'Planning container cleanup did not settle.');
+      }
+    }
+  } };
+}