Skip to content

Commit fe4155e

Browse files
mchwangclaude
andauthored
E4: planning acceptance fixtures and recorded Claude output (#45)
* docs: audit lane E planning contracts and remaining gaps * feat: prepare bounded read-only planning requests * docs: qualify v1 dispatch coverage and name Store correctly * docs: use exported Store name in planning handoff * feat: coordinate identity-bound planning suggestions * fix: narrow planning issue data and initial revision * test: verify shutdown refuses a distinct new plan * test: add planning import replay and hostile-input acceptance fixtures * fix: isolate stable provider identity fields * docs: clarify provider identity boundary wording * fix: classify suggestion publication races from durable state * fix: preserve external terminal state after provider rejection * test: assert durable generated draft after import and reopen * Refresh planning acceptance against merged lifecycle * E4: record and replay real Claude planning output through lane D Add the planning provider over lane D's container (read-only planning phase, vendor-only egress, Claude only), a credentialed recording script, and replay tests that drive real Claude output through E2 validation and E3/Store. Codex is refused in every phase (#75, #93), so two recordings are kept: claude-draft and claude-suggest. Each recording stores hashes of the repository files, prompt, schema and the exact Claude argv, so a lane D or prompt change makes it stale. Extraction tests follow the structured (--json-schema / structured_output) envelope. Planning keeps its own record of leftovers, so its refusals never turn Ask off. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * E4: address review round 1 on the planning provider and tests - Keep a failed request's own error when its cleanup also fails; leftovers are retained and the next request names them. - RetainedStorage takes a LeftoverPolicy (refusal wording and host-copy removal); planning uses PLANNING_LEFTOVERS instead of a wrapper that swallowed RetainedStorage's errors. - Assert exact reasons in the T18 broken-plan and malformed-response cases, test each planning refusal, and drop an assertion that could not fail. - Use fileURLToPath for fixture directories. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * E4: address review round 2 on planning leftovers - PlanningStorage types planning's leftovers apart from Ask's, so an Ask-policy RetainedStorage cannot be passed to the planning provider. - removePlanningRoot deletes only planning roots under the temporary directory, and makes the read-only input directory writable first, also on retries. - A failed request reports its own error followed by the cleanup failure. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent adde4fc commit fe4155e

14 files changed

Lines changed: 1176 additions & 7 deletions
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
# E4 planning acceptance and remaining gate
2+
3+
The dedicated E4 suite composes E2/E3 with the real SQLite Store. The checked-in
4+
plan, edit cards and hostile text under `test/fixtures/planning/` are **synthetic**.
5+
They are not captured Claude/Codex responses and do not establish live adapter,
6+
semantic prompt-injection resistance, Docker isolation or product performance.
7+
Only files under `test/fixtures/planning/recorded/` are vendor output.
8+
9+
Runnable checks:
10+
11+
```sh
12+
npm test
13+
npm run typecheck
14+
npm run test:browser
15+
```
16+
17+
The PR body records the final pushed head and observed counts. The browser suite
18+
is the existing integrated review baseline, not planning-screen acceptance (G).
19+
20+
| Acceptance | Evidence |
21+
| --- | --- |
22+
| JSON/YAML import, selected issue, eight broken plans, atomic failed import | `planning-acceptance.test.ts` against real Store |
23+
| 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 |
24+
| Malformed extracted output and invalid resulting cards never become ready | E4 asserts returned failure, durable failed reason, null reply and unchanged plan revision |
25+
| 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 |
26+
| Initial/revised draft identity and revision; input/UTF-8/prompt budgets | `planning-author.test.ts`, including exact 32 KiB boundary and post-escaping expansion |
27+
| Parser depth/size, duplicates, aliases, tags, safe numbers and registry copies | Existing frozen `plan-v1.test.ts`, `registry.test.ts`, `plan.test.ts` reused |
28+
| Late response, cancellation, timeout ownership, shutdown admission, input changes | `planning-suggestions.test.ts` controlled promises/timers with real Store |
29+
| Unapproved appended flags | E4 asserts `command-not-allowed` and exact-argv denial; no execution occurs in E |
30+
31+
## Recorded vendor output
32+
33+
D5 has merged. Codex is refused in every phase (#75, #93): it reads files only
34+
through its shell, and planning runs no process. So only Claude is recorded, and
35+
the recorded-output part of T9/T18 needs one manual run of
36+
`scripts/record-planning.ts` with a Claude token:
37+
38+
```sh
39+
CODEBOOST_RUN_AUTH_PROBES=1 CLAUDE_CODE_OAUTH_TOKEN=... node scripts/record-planning.ts
40+
```
41+
42+
The script sends one draft request and one suggestion request through
43+
`runner/planning-provider.ts`. That provider uses D's container, the read-only
44+
planning phase and vendor-only egress. D passes the request schema to Claude as
45+
`--json-schema` and returns Claude's `structured_output` as JSON text. The script
46+
saves that text exactly as D returned it to
47+
`test/fixtures/planning/recorded/claude-<mode>.json`, with the image tag, CLI
48+
version, and hashes of the repository files, prompt, schema and the exact argv D
49+
ran Claude with. It saves before it validates, and it refuses
50+
to write output that contains the token. It removes the token from its own
51+
environment before any Docker call, and replaces it in every error it prints. Read each file before you commit it.
52+
Never put credentials in a fixture, an issue or a PR.
53+
54+
The adapter passes no model flag, so each recording names the CLI's default model,
55+
not a pinned one.
56+
57+
| Acceptance | Evidence |
58+
| --- | --- |
59+
| Real output validates and persists | `planning-recorded.test.ts` replays the draft into the Store and the suggestion through E3, then reopens SQLite |
60+
| 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 |
61+
| Recordings exist | The same test fails until both files exist |
62+
| 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 |
63+
| 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 |
64+
65+
The edit schema caps every text field, so no valid reply is near 1 MiB. The size
66+
tests therefore show which check rejects the text: at exactly 1 MiB the schema,
67+
and at 1 MiB + 1 byte the size limit. If a real output fails validation, fix the
68+
prompt or schema and record again. Do not edit the recording.
69+
70+
Left for F's wiring: the provider's image build, clone and storage calls are
71+
synchronous, so a server must run it off its request thread, as Ask does. That
72+
worker should also set TMPDIR to a planning root, as Ask's does, so the input and
73+
auth staging that lane D leaves when it gives up on cleanup is covered. At that
74+
point the provider and `askInContainer` should share one container runner; they
75+
are separate copies today. E3's
76+
default 120-second suggestion timeout is shorter than the provider's ten-minute
77+
budget. Startup recovery does not yet remove planning's Docker resources after a
78+
crash, because they are not attempt allocations in the Store. The recording
79+
script prints its runner label if it leaves any.
80+
81+
F/G still own live API and UI integration. The Store now guarantees revision/snapshot
82+
binding and pending-only settlement across processes, and E3 requires the caller's
83+
snapshot identity before admission. G must not treat these fixture-only results as a
84+
completed production planning milestone. T18/T9 remain incomplete until all assigned
85+
lane slices pass their original acceptance.

‎runner/planning-provider.ts‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
import { randomUUID } from 'node:crypto';
2+
import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
3+
import { tmpdir } from 'node:os';
4+
import { basename, dirname, join } from 'node:path';
5+
import type { InvocationContext, InvocationInput, InvocationResult, StopReason } from '../agents/contract.ts';
6+
import type { TaskFilesystems, TaskStorageLimits } from '../agents/container/storage.ts';
7+
import type { AuthorProvider, AuthorRequest } from '../core/planning-author.ts';
8+
import { allocations, RetainedStorage, stopOf, type ContainerDependencies, type LeftoverPolicy, type Provider } from './question-container.ts';
9+
10+
/** Ask's lane D dependencies, less startup recovery, which the runner owns for planning. */
11+
export type PlanningDependencies = Omit<ContainerDependencies, 'recover'>;
12+
13+
/** What the runner knows about the checkout a plan is written against. */
14+
export interface PlanningProviderOptions {
15+
/** The vendor chosen in Settings. Only Claude can plan; Codex is refused before any work. */
16+
readonly vendor: Provider;
17+
/** Host checkout to clone. The agent sees a read-only copy of `head`, never this directory. */
18+
readonly repository: string;
19+
/** The committed head the plan's snapshot was taken from. */
20+
readonly head: string;
21+
readonly snapshotId: string;
22+
/** Written as `io.codeboost.runner` on every Docker object an invocation creates. */
23+
readonly runnerOwner: string;
24+
/** Whole-invocation budget, setup included. Lane D caps the invocation itself at ten minutes. */
25+
readonly timeoutMs?: number;
26+
readonly deps: PlanningDependencies;
27+
/** Shared across providers in one process, so the image is built once. */
28+
readonly image?: { id?: string };
29+
/** What earlier requests left behind. Invocations stay refused while any of it remains. */
30+
readonly retained?: PlanningStorage;
31+
}
32+
33+
/**
34+
* Planning's leftovers: lane D allocations, allocations lane D returned no handle for, and host copies of the planned
35+
* code. Use one instance for planning and never share it with Ask, so a planning failure never turns Ask off.
36+
*/
37+
export const PLANNING_LEFTOVERS: LeftoverPolicy = Object.freeze({
38+
removePath: removePlanningRoot,
39+
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.`,
40+
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.',
41+
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.`,
42+
});
43+
/** Planning's own leftovers. Typed apart from Ask's, so an instance with Ask's wording and removal cannot be passed in. */
44+
export class PlanningStorage extends RetainedStorage {
45+
readonly feature = 'planning';
46+
constructor() { super(PLANNING_LEFTOVERS); }
47+
}
48+
49+
const ROOT_PREFIX = 'codeboost-planning-';
50+
/**
51+
* Delete a planning root: a direct child of the temporary directory named with planning's prefix, and nothing else.
52+
* Its input directory is made writable first, since the request left it read-only.
53+
*/
54+
export function removePlanningRoot(root: string): void {
55+
if (dirname(root) !== tmpdir() || !basename(root).startsWith(ROOT_PREFIX))
56+
throw new Error(`Refusing to remove a path that is not a planning root (${root}).`);
57+
try { chmodSync(join(root, 'input'), 0o700); } catch { /* never created, or already removed */ }
58+
rmSync(root, { recursive: true, force: true, maxRetries: 3, retryDelay: 50 });
59+
}
60+
61+
// Planning reads the code; it needs no room to write. The same ceilings as Ask.
62+
export const PLANNING_STORAGE: TaskStorageLimits = Object.freeze({
63+
workBytes: 512 * 1024 * 1024, workInodes: 131_072, metadataBytes: 512 * 1024 * 1024, metadataInodes: 131_072,
64+
});
65+
const DEFAULT_TIMEOUT_MS = 10 * 60_000;
66+
67+
/** Codex reads files only through its shell, and planning runs no process, so Codex cannot plan (#75, #93). */
68+
export const CODEX_PLANNING_REFUSED = 'Codex cannot write plans yet: it can read the code only by running commands, '
69+
+ 'and planning runs none. Choose Claude Code in Settings, then retry.';
70+
71+
export function planningCredential(vendor: Provider, env: PlanningDependencies['env']): string {
72+
if (vendor === 'codex') throw new Error(CODEX_PLANNING_REFUSED);
73+
const token = env.CLAUDE_CODE_OAUTH_TOKEN;
74+
if (!token) throw new Error('Planning with Claude Code needs CLAUDE_CODE_OAUTH_TOKEN. Create one with `claude setup-token`.');
75+
return token;
76+
}
77+
78+
const stopMessages: Record<StopReason, string> = {
79+
cancelled: 'Planning agent cancelled.', timeout: 'Planning agent timed out.', shutdown: 'Server stopped during planning.',
80+
'output-limit': 'Planning agent output exceeded its limit.', 'capture-failure': 'Planning agent output could not be captured.',
81+
};
82+
const sameContext = (left: InvocationContext, right: InvocationContext) =>
83+
(Object.keys(right) as (keyof InvocationContext)[]).every(key => left[key] === right[key])
84+
&& Object.keys(left).length === Object.keys(right).length;
85+
/** Accept only this exact invocation's output, and only after a clean exit. The text is still unvalidated. */
86+
export function planningOutput(result: InvocationResult, invocation: InvocationInput): string {
87+
if (result.attemptId !== invocation.attemptId || !result.context || !sameContext(result.context, invocation.context))
88+
throw new Error('The planning agent returned a result for a different attempt.');
89+
if (result.stopReason) throw new Error(stopMessages[result.stopReason]);
90+
if (result.exitCode === null || result.signal !== null)
91+
throw new Error(`Claude stopped unexpectedly${result.signal ? ` (${result.signal})` : ''}.`);
92+
if (result.exitCode !== 0) {
93+
const detail = result.stdout.replace(/\s+/g, ' ').trim().slice(0, 300);
94+
throw new Error(`Claude could not write the plan. Check its sign-in and usage limits.${detail ? ` Claude said: ${detail}` : ''}`);
95+
}
96+
return result.stdout;
97+
}
98+
99+
/**
100+
* E2's provider over lane D: each request runs in a fresh container on a read-only copy of `head`, in the "planning"
101+
* phase (read, list and search only; no commands) with vendor-only egress. The request's schema is the only file in
102+
* the input mount; D passes it to Claude as `--json-schema` and returns Claude's `structured_output` as JSON text. That
103+
* text is still unvalidated; E2's `validate` decides whether it is a plan. Codex is refused before any work.
104+
*
105+
* Lane D's image build, clone and storage allocation are synchronous Docker and Git calls. A serving process must run
106+
* this provider off its request thread, as Ask does in its worker.
107+
*/
108+
export interface PlanningProvider extends AuthorProvider {
109+
invoke(request: AuthorRequest, signal: AbortSignal): Promise<string>;
110+
}
111+
export function createPlanningProvider(options: PlanningProviderOptions): PlanningProvider {
112+
const { vendor, deps } = options, image = options.image ?? {}, retained = options.retained ?? new PlanningStorage();
113+
const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS;
114+
if (!Number.isSafeInteger(timeoutMs) || timeoutMs < 1) throw new Error('Planning timeout must be a positive integer.');
115+
return { async invoke(request: AuthorRequest, signal: AbortSignal): Promise<string> {
116+
if (request.phase !== 'planning' || request.access !== 'read-only') throw new Error('Planning requests must be read-only.');
117+
const deadline = Date.now() + timeoutMs;
118+
const remaining = () => {
119+
signal.throwIfAborted();
120+
const value = deadline - Date.now();
121+
if (value < 1) throw new Error(stopMessages.timeout);
122+
return value;
123+
};
124+
signal.throwIfAborted();
125+
const credential = planningCredential(vendor, deps.env);
126+
retained.release(deps.removeFilesystems);
127+
image.id ??= deps.buildImage(remaining());
128+
const root = mkdtempSync(join(tmpdir(), ROOT_PREFIX));
129+
const staging = join(root, 'staging'), input = join(root, 'input');
130+
let filesystems: TaskFilesystems | undefined, failure: { error: unknown } | undefined;
131+
try {
132+
mkdirSync(staging); mkdirSync(input);
133+
writeFileSync(join(input, 'schema.json'), request.schemaText, { mode: 0o444 });
134+
chmodSync(input, 0o555);
135+
// The clone is a full host copy with no byte limit of its own, so the repository must fit before it is made.
136+
const size = deps.measureRepository(options.repository, options.head, Math.min(60_000, remaining()));
137+
if (size.checkoutBytes > PLANNING_STORAGE.workBytes || size.entries > PLANNING_STORAGE.workInodes
138+
|| size.objectBytes > PLANNING_STORAGE.metadataBytes)
139+
throw new Error('The repository is too large for planning.');
140+
const clone = deps.createClone({ source: options.repository, parent: staging, taskId: `planning-${request.requestId}`,
141+
head: options.head, timeoutMs: Math.min(120_000, remaining()) });
142+
const owner = { runnerOwner: options.runnerOwner, attemptId: request.requestId, allocationId: randomUUID() };
143+
try { filesystems = deps.prepareFilesystems(clone, PLANNING_STORAGE, image.id, owner, Math.min(60_000, remaining())); }
144+
catch (error) {
145+
// D throws an AggregateError only when a failed allocation's own cleanup did not settle; it returns no handle.
146+
if (error instanceof AggregateError) retained.markUntracked();
147+
throw error;
148+
}
149+
remaining();
150+
const invocation = deps.capture({ clone, phase: 'planning', vendor: 'claude', approvedArgv: [], deadline,
151+
attemptId: request.requestId, runnerOwner: options.runnerOwner,
152+
context: { snapshotId: options.snapshotId, planId: request.identity.planId, planRevision: request.revision,
153+
assignmentId: `${request.mode}-${request.issue}`, referencedCodeHash: options.head, stateVersion: 0 } });
154+
const adapterRequest = { invocation, filesystems, inputDirectory: input, imageId: image.id, prompt: request.prompt,
155+
networkAllocationId: randomUUID() };
156+
const handle = deps.startClaude(adapterRequest, credential);
157+
const cancel = () => handle.cancel(stopOf(signal.reason));
158+
if (signal.aborted) cancel(); else signal.addEventListener('abort', cancel, { once: true });
159+
let result: InvocationResult;
160+
try { result = await handle.settled; }
161+
finally { signal.removeEventListener('abort', cancel); }
162+
// D stopped before confirming cleanup; the resources are found again by their labels after a restart.
163+
if (result.unreleased !== undefined) retained.markUntracked();
164+
return planningOutput(result, invocation);
165+
} catch (error) {
166+
failure = { error };
167+
throw error;
168+
} finally {
169+
const failures: unknown[] = [];
170+
if (filesystems) try { deps.removeFilesystems(filesystems); } catch (error) { retained.retain(filesystems); failures.push(error); }
171+
try { removePlanningRoot(root); }
172+
catch (error) {
173+
retained.retainPath(root);
174+
failures.push(new Error(`A copy of the planned code could not be deleted (${root}).`, { cause: error }));
175+
}
176+
// What cleanup left is retained above, and the next request names it. A successful request is refused rather than
177+
// returned, as Ask does, so a plan is never kept from an unsettled run. A failed one keeps its own message first.
178+
if (failures.length) {
179+
const text = (error: unknown) => error instanceof Error ? error.message : String(error);
180+
throw failure
181+
? new AggregateError([failure.error, ...failures], `${text(failure.error)} Cleanup also did not settle: ${failures.map(text).join('; ')}`)
182+
: new AggregateError(failures, 'Planning container cleanup did not settle.');
183+
}
184+
}
185+
} };
186+
}

0 commit comments

Comments
 (0)