Skip to content

Commit 7cd81a8

Browse files
mchwangclaude
andauthored
Pass Claude's structured answer to the plan parser unchanged (#166) (#169)
* Load plan schemas through the registry and close #6 Every version in schema/versions.json is now compiled from its retained snapshots, and plans and edit replies are checked by the version they declare. The loader refuses a registry that disagrees with the snapshots or validators, and treats a version with no validator as unsupported. Pin the released v1 files by SHA-256, keep them LF on checkout, and add the remaining #6 tests: path identity and Git mode mapping, hostile text in every authoring prompt, and output limits at the exact byte. Runtime gaps move to #162-#166. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass Claude's structured answer to the plan parser unchanged (#166) parseClaudeOutput re-encoded structured_output with JSON.parse and JSON.stringify, so a duplicate key or an inexact integer reached parseV1 already resolved. Cut the exact text of structured_output from the envelope with a length-bounded scanner instead, and refuse a planning envelope that repeats a top-level key before classifying it. Whether the Claude API or CLI resolves duplicates before writing the envelope is not verified; the docs say so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Name the current base in the plan library alignment note Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Compare large decoded answers without a diff so a broken decoder fails at once Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep the diff-free comparison for the megabyte answers only Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent dd35566 commit 7cd81a8

5 files changed

Lines changed: 132 additions & 23 deletions

File tree

‎agents/adapters/claude.ts‎

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,60 @@ import { setUpProfile } from './setup.ts';
77
import { assertAdapterRequest, capAdapterInvocationBudget, createAdapterInvocationBudget,
88
type AgentAdapterOptions, type AgentAdapterRequest } from './types.ts';
99

10+
/**
11+
* The exact source text of each member of a top-level JSON object, by key. `json` must already have passed
12+
* `JSON.parse` as an object, so only string and nesting boundaries are tracked here; every loop also stops at the end
13+
* of the text, so other input cannot hang it. Refuses a key that appears twice, where `JSON.parse` would silently keep
14+
* the last value.
15+
*/
16+
function topLevelMembers(json: string): Map<string, string> {
17+
const members = new Map<string, string>();
18+
const isSpace = (char: string | undefined) => char === ' ' || char === '\t' || char === '\n' || char === '\r';
19+
let at = 0;
20+
const skipSpace = () => { while (isSpace(json[at])) at++; };
21+
const skipString = () => { // `at` is on the opening quote; leaves `at` after the closing one.
22+
for (at++; at < json.length && json[at] !== '"'; at++) if (json[at] === '\\') at++;
23+
at++;
24+
};
25+
skipSpace(); at++; // '{'
26+
for (skipSpace(); at < json.length && json[at] !== '}'; skipSpace()) {
27+
const keyStart = at; skipString();
28+
const key = JSON.parse(json.slice(keyStart, at)) as string;
29+
skipSpace(); at++; skipSpace(); // ':'
30+
const valueStart = at;
31+
if (json[at] === '"') skipString();
32+
else if (json[at] === '{' || json[at] === '[') {
33+
let depth = 0;
34+
do {
35+
if (json[at] === '"') { skipString(); continue; }
36+
if (json[at] === '{' || json[at] === '[') depth++;
37+
else if (json[at] === '}' || json[at] === ']') depth--;
38+
at++;
39+
} while (depth > 0 && at < json.length);
40+
} else while (at < json.length && !isSpace(json[at]) && json[at] !== ',' && json[at] !== '}') at++;
41+
if (members.has(key)) throw new Error('Claude returned a malformed output envelope.');
42+
members.set(key, json.slice(valueStart, at));
43+
skipSpace(); if (json[at] === ',') at++;
44+
}
45+
return members;
46+
}
47+
1048
/**
1149
* Read Claude's `--output-format json` envelope. A result whose `subtype` is not `success`, or whose `is_error` is
1250
* true, is a provider failure; it can omit `result`. With `--json-schema` (planning), a successful answer needs
13-
* `subtype: "success"` and is the schema-validated `structured_output` object, returned as JSON text; `result` then
14-
* holds only Claude's prose. Plain-text envelopes without a `subtype` are still accepted.
51+
* `subtype: "success"` and is the schema-validated `structured_output` object. It is returned as the exact JSON text
52+
* of that member in the CLI's envelope, never re-encoded here, so the plan parser sees any duplicate key or inexact
53+
* integer the envelope holds (#166). Whether the API or CLI resolves those before writing the envelope is not verified.
54+
* With a schema, an envelope that repeats a top-level key is malformed, whatever it reports. `result` then holds only
55+
* Claude's prose. Plain-text envelopes without a `subtype` are still accepted.
1556
*/
1657
export function parseClaudeOutput(raw: Buffer, structured = false): { text: string; providerFailed: boolean } {
17-
const envelope = JSON.parse(new TextDecoder('utf-8', { fatal: true }).decode(raw)) as
58+
const source = new TextDecoder('utf-8', { fatal: true }).decode(raw);
59+
const envelope = JSON.parse(source) as
1860
{ subtype?: unknown; result?: unknown; is_error?: unknown; structured_output?: unknown };
61+
// Read every member before classifying: JSON.parse keeps only the last of a repeated key, such as `is_error`.
62+
const members = structured && envelope !== null && typeof envelope === 'object' && !Array.isArray(envelope)
63+
? topLevelMembers(source) : null;
1964
if (typeof envelope.is_error !== 'boolean' || (envelope.result !== undefined && typeof envelope.result !== 'string')
2065
|| (envelope.subtype !== undefined && typeof envelope.subtype !== 'string'))
2166
throw new Error('Claude returned a malformed output envelope.');
@@ -31,7 +76,7 @@ export function parseClaudeOutput(raw: Buffer, structured = false): { text: stri
3176
const answer = envelope.structured_output;
3277
if (answer === null || typeof answer !== 'object' || Array.isArray(answer))
3378
throw new Error('Claude returned no structured output for a schema-constrained answer.');
34-
return Object.freeze({ text: JSON.stringify(answer), providerFailed: false });
79+
return Object.freeze({ text: members!.get('structured_output')!, providerFailed: false });
3580
}
3681

3782
/**

‎docs/implementation/agent-isolation.md‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,15 @@ Use only these entry points to run an agent:
9292
- **Planning answers.** Use `startClaudeInvocation` for planning. It reads `schema.json` from the input directory
9393
and passes its exact text to Claude as `--json-schema`. The container profile refuses the command if that text
9494
differs from the file it mounts. The result's `stdout` is Claude's `structured_output` object as JSON text, with
95-
no prose or Markdown around it. A Claude run that fails its own schema check settles with a nonzero exit code.
96-
Questions answer in plain text and carry no schema flag. The schema file must be a regular file with one link,
97-
at most 64 KiB of UTF-8, and a JSON object with `"type": "object"`; otherwise the start call throws. Leave out a
98-
draft 2020-12 `$schema` line: Claude's `--json-schema` rejects it (see [plan format](../plan-format.md)).
95+
no prose or Markdown around it. It is the exact text of that member, cut from the CLI's envelope and never
96+
re-encoded here, so the plan parser sees any duplicate key or inexact integer the envelope holds and refuses it
97+
(#166). The Claude API and CLI may resolve those before writing the envelope; that is not verified, because the
98+
only real-CLI probes need credentials and do not run in CI. A planning envelope that repeats a top-level key is
99+
refused as malformed, whatever it reports. A Claude run that fails its own schema check settles with a nonzero
100+
exit code. Questions answer in plain text and carry no schema flag. The schema file must be a regular file with
101+
one link, at most 64 KiB of UTF-8, and a JSON object with `"type": "object"`; otherwise the start call throws.
102+
Leave out a draft 2020-12 `$schema` line: Claude's `--json-schema` rejects it (see [plan
103+
format](../plan-format.md)).
99104
- **Codex.** `startCodexInvocation` throws for every phase, and allocates nothing. Use Claude. See
100105
[Codex is refused in every phase](#codex-is-refused-in-every-phase).
101106
5. To keep a stopped writable attempt's partial output, call `exportTaskDiff(storage, { base, imageId })` before

‎docs/implementation/plan-library-alignment.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@ Issue #6 tracked the v1 plan library contract and the runner obligations that ca
2121
| Container mounts: Git metadata on its own filesystem, read-only, with no writable hard links or aliases into it | Done before this change | `agents/container/profile.ts`, probe `metadata-alias` in `agents/policy.ts`; `test/agent-container.test.ts` ("keeps Git metadata read-only, on another filesystem, and mounted against replacement", "keeps Git metadata unchanged under link, alias, truncation and replacement attempts during %s"), negative controls in `test/agent-gate.test.ts` (real Docker) |
2222
| Process termination: output overflow, deadline and cancellation stop the whole container, including a process that ignores SIGTERM | Done before this change | `agents/adapters/supervisor.ts`; `test/agent-supervisor.test.ts` ("terminates %s at bounded output limits", "enforces a finite wall deadline and force-settles the container", "preserves the first cancellation reason until an ignored SIGTERM fully settles") (real Docker) |
2323
| Output shown in the UI bounded: captured output comes only from the supervisor, which bounds stdout and stderr; when a planning run exits with an error, the provider's message quotes its output with whitespace collapsed, cut to at most 300 UTF-16 code units (`runner/planning-provider.ts`) | Done before this change; 300-unit quote tested in this change | `test/agent-supervisor.test.ts` output-limit cases (real Docker); `test/planning-provider.test.ts` ("quotes at most 300 code units of a failed run's output, with whitespace collapsed"). The Docker daemon's own log is #165 |
24-
| Output limits at the exact byte | **This change** (tests; the supervisor already enforced the limits) | Supervisor enforcement: `test/agent-supervisor.test.ts` (real Docker). Decoder behaviour only, not a limit: the Claude decoder handles an envelope of exactly the stdout ceiling, and the 1 MiB parser limit applies to the decoded answer at the exact byte: `test/agent-adapter.test.ts`. The decoder re-serializes the answer, so the limit applies to the decoded text, not to the provider's bytes; #166 may change that |
24+
| Output limits at the exact byte | **This change** (tests; the supervisor already enforced the limits) | Supervisor enforcement: `test/agent-supervisor.test.ts` (real Docker). Decoder behaviour only, not a limit: the Claude decoder handles an envelope of exactly the stdout ceiling, and the 1 MiB parser limit applies to the decoded answer at the exact byte: `test/agent-adapter.test.ts`. At #6 the decoder re-serialized the answer, so the limit applied to the re-encoded text. *Later:* #166 returns the exact text of `structured_output` in the CLI's envelope, so the limit applies to those bytes |
2525
| Prompt byte budget before spawn | Done before this change | `MAX_PROMPT_BYTES` in `core/planning-author.ts`; `test/planning-author.test.ts` ("accepts exactly the prompt byte limit and rejects the next byte without truncation") |
2626
| Model budget before spawn | Moved to #163 | — |
2727
| Planning profile probed at startup | Moved to #164 | — |
2828
| Docker daemon log limits | Moved to #165 | — |
29-
| Claude structured output under the v1 parsing contract | Moved to #166 (found during this change) | — |
29+
| Claude structured output under the v1 parsing contract | Moved to #166 (found during this change). *Later:* fixed in #166 | `agents/adapters/claude.ts`; `test/agent-adapter.test.ts` ("lets draft validation refuse %s in a structured answer") |
3030

3131
## Schema loading
3232

‎test/agent-adapter.test.ts‎

Lines changed: 69 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -137,29 +137,86 @@ describe('production agent adapters', () => {
137137
const answer = JSON.stringify({ summary: 'x'.repeat(4 * 1024 * 1024) });
138138
const raw = structuredEnvelope(answer, OUTPUT_LIMITS.stdoutBytes);
139139
expect(raw.length).toBe(OUTPUT_LIMITS.stdoutBytes);
140+
// Compared without a diff: on a mismatch, vitest would spend minutes diffing megabyte strings instead of failing.
141+
const decoded = parseClaudeOutput(raw, true);
142+
expect(decoded.providerFailed).toBe(false);
143+
expect(decoded.text.length).toBe(answer.length);
144+
expect(decoded.text === answer).toBe(true);
145+
});
146+
147+
it('returns a structured answer as the exact text of structured_output in the envelope, whatever surrounds it', () => {
148+
// Spacing, escapes, number spellings and brackets inside strings all survive, and so do members on either side.
149+
const answer = '{ "summary" : "caf\\u00e9 } ] \\" \\\\", "n": [1e20, 1.0, -0E+0],\n "nested": {"a": [{}, []]} }';
150+
const raw = Buffer.from('{"type":"result","subtype":"success","is_error":false,"result":"text with } and \\" in it",'
151+
+ `"structured_output":\t${answer}\n, "usage": {"input_tokens": 1}, "end": null}`);
140152
expect(parseClaudeOutput(raw, true)).toEqual({ text: answer, providerFailed: false });
153+
expect(parseClaudeOutput(Buffer.from(`{"subtype":"success","is_error":false,"structured_output":${answer}}`), true).text)
154+
.toBe(answer);
155+
});
156+
157+
// #166: re-encoding the answer once let a duplicate key or an inexact integer reach the plan parser already resolved.
158+
const validPlan = { schema_version: 1, issue: 1, revision: 1, summary: 'Plan', questions: [],
159+
items: [{ id: 'P1', title: 'Change', intent: 'Improve', files: [{ path: 'a', kind: 'edit', renamed_from: null, change: 'Change' }],
160+
acceptance: [{ type: 'check', text: 'Works' }], depends_on: [] }] };
161+
const draft = () => prepareDraft({ context: { identity: { repositoryId: 'repo', taskId: 'task', planId: 'plan' }, issue: 1,
162+
baseEntries: [{ path: 'a', kind: 'file' }], pathKey: path => path, allowedCommands: [] }, requestId: 'request', revision: 1,
163+
repo: { name: 'repo', baseRef: 'main', baseSha: 'a'.repeat(40), paths: ['a'] },
164+
issue: { number: 1, title: 'Fix', body: '', comments: [] }, approvedLessons: [], feedback: '' });
165+
it.each([
166+
['a duplicate key', JSON.stringify(validPlan).replace('"summary":"Plan"', '"summary":"Plan","summary":"Other"'), 'Duplicate key: summary'],
167+
['an inexact integer', JSON.stringify(validPlan).replace('"revision":1', '"revision":1.0000000000000001'), 'Integer is not exactly representable.'],
168+
])('lets draft validation refuse %s in a structured answer', (_case, answer, refusal) => {
169+
const { text } = parseClaudeOutput(structuredEnvelope(answer, answer.length + 256), true);
170+
// Compared without a diff, so a broken decoder fails at once rather than in a minutes-long diff of a large string.
171+
expect(text.length).toBe(answer.length);
172+
expect(text === answer).toBe(true);
173+
expect(() => draft().validate(text)).toThrow(refusal);
174+
// The same plan without the defect is accepted, so the refusal is the defect's.
175+
const valid = JSON.stringify(validPlan);
176+
expect(draft().validate(parseClaudeOutput(structuredEnvelope(valid, valid.length + 256), true).text).value).toEqual(validPlan);
177+
});
178+
179+
it('refuses a structured envelope that repeats a top-level key, before reading what it reports', () => {
180+
for (const envelope of ['{"subtype":"success","is_error":false,"structured_output":{"a":1},"structured_output":{"a":2}}',
181+
'{"subtype":"success","is_error":true,"is_error":false,"structured_output":{"a":1}}',
182+
// The last is_error would otherwise make this a provider failure rather than a refusal.
183+
'{"subtype":"success","is_error":false,"is_error":true,"structured_output":{"a":1}}',
184+
// Keys are compared decoded, so another spelling of the same key is a repeat.
185+
'{"subtype":"success","is_error":false,"structured_output":{"a":1},"structured\\u005foutput":{"a":2}}'])
186+
expect(() => parseClaudeOutput(Buffer.from(envelope), true)).toThrow('malformed');
187+
// Without a schema the envelope is read as before.
188+
expect(parseClaudeOutput(Buffer.from('{"is_error":false,"result":"a","result":"b"}'))).toEqual({ text: 'b', providerFailed: false });
189+
});
190+
191+
// The base refused these too. They guard the scanner's precondition: it is given only an object envelope.
192+
it.each(['[{"is_error":false}]', '[]', '"text"', 'true'])('refuses a structured envelope that is not an object: %s', envelope => {
193+
expect(() => parseClaudeOutput(Buffer.from(envelope), true)).toThrow('malformed');
194+
});
195+
196+
it('finds structured_output however the envelope is spaced or its key is spelled', () => {
197+
for (const envelope of [' \r\n{ "subtype" : "success" , "is_error" : false , "structured_output" : {"a":1} } \n',
198+
'{"subtype":"success","is_error":false,"structured\\u005foutput":{"a":1}}'])
199+
expect(parseClaudeOutput(Buffer.from(envelope), true).text).toBe('{"a":1}');
141200
});
142201

143-
it('re-serializes a structured answer, so decoded text can exceed its raw bytes (#166 may change this)', () => {
144-
// Describes today's decoder, which re-serializes the answer (#166 may change that): JSON.parse then JSON.stringify
145-
// writes 1e20 in full, so 4 raw bytes become 21. The supervisor bounds the decoded text itself; see the
146-
// decoded-output limit cases in agent-supervisor.test.ts.
147-
const raw = structuredEnvelope(JSON.stringify({ n: [] }).replace('[]', '[' + Array(1000).fill('1e20').join(',') + ']'), 8 * 1024);
148-
const { text } = parseClaudeOutput(raw, true);
149-
expect(Buffer.byteLength(text)).toBeGreaterThan(raw.length);
202+
it('applies the 1 MiB plan parser limit to the bytes of structured_output in the envelope, not to a re-encoding', () => {
203+
// Spacing inside the answer counts: re-encoded, this answer would be far below the limit.
204+
const answer = `{ "summary": "x" ${' '.repeat(MAX_SOURCE_BYTES)}}`;
205+
expect(Buffer.byteLength(JSON.stringify(JSON.parse(answer)))).toBeLessThan(MAX_SOURCE_BYTES);
206+
const { text } = parseClaudeOutput(structuredEnvelope(answer, answer.length + 256), true);
207+
// Compared without a diff, so a broken decoder fails at once rather than in a minutes-long diff of a large string.
208+
expect(text.length).toBe(answer.length);
209+
expect(text === answer).toBe(true);
210+
expect(() => draft().validate(text)).toThrow('Input size exceeds 1 MiB.');
150211
});
151212

152213
it('applies the 1 MiB plan parser limit to the decoded answer at the exact byte in draft validation', () => {
153-
const draft = prepareDraft({ context: { identity: { repositoryId: 'repo', taskId: 'task', planId: 'plan' }, issue: 1,
154-
baseEntries: [{ path: 'a', kind: 'file' }], pathKey: path => path, allowedCommands: [] }, requestId: 'request', revision: 1,
155-
repo: { name: 'repo', baseRef: 'main', baseSha: 'a'.repeat(40), paths: ['a'] },
156-
issue: { number: 1, title: 'Fix', body: '', comments: [] }, approvedLessons: [], feedback: '' });
157214
const answerOf = (bytes: number) => JSON.stringify({ summary: 'x'.repeat(bytes - '{"summary":""}'.length) });
158215
for (const [bytes, accepted] of [[MAX_SOURCE_BYTES, true], [MAX_SOURCE_BYTES + 1, false]] as const) {
159216
const { text } = parseClaudeOutput(structuredEnvelope(answerOf(bytes), bytes + 1024), true);
160217
expect(Buffer.byteLength(text)).toBe(bytes);
161218
// At the limit the answer parses and fails only later, on its missing revision; one byte more fails in the parser.
162-
expect(() => draft.validate(text)).toThrow(accepted ? 'Response revision mismatch.' : 'Input size exceeds 1 MiB.');
219+
expect(() => draft().validate(text)).toThrow(accepted ? 'Response revision mismatch.' : 'Input size exceeds 1 MiB.');
163220
}
164221
});
165222

‎test/planning-extraction.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ it.each([
6666
expect(extract(raw)).toMatchObject({ providerFailed: true });
6767
});
6868

69+
// `structured` writes its envelope with JSON.stringify, so the exact text of the answer is JSON.stringify(answer).
6970
it('returns structured_output as JSON text and ignores Claude\'s prose result', () => {
7071
const answer = reply();
7172
expect(extract(structured(answer))).toEqual({ text: JSON.stringify(answer), providerFailed: false });
@@ -74,7 +75,8 @@ it('returns structured_output as JSON text and ignores Claude\'s prose result',
7475
it('extracts 1 MiB of structured output, E2\'s size limit, byte for byte', () => {
7576
const answer = replyOfSize(MIB), text = extract(structured(answer)).text;
7677
expect(Buffer.byteLength(text)).toBe(MIB);
77-
expect(text).toBe(JSON.stringify(answer));
78+
// Compared without a diff, so a broken decoder fails at once rather than in a minutes-long diff of a 1 MiB string.
79+
expect(text === JSON.stringify(answer)).toBe(true);
7880
});
7981

8082
it('passes exactly 1 MiB of extracted text to schema validation, not the size limit', async () => {

0 commit comments

Comments
 (0)