Pass Claude's structured answer to the plan parser unchanged (#166)

agents/adapters/claude.ts

@@ -7,15 +7,60 @@ import { setUpProfile } from './setup.ts';
 import { assertAdapterRequest, capAdapterInvocationBudget, createAdapterInvocationBudget,
   type AgentAdapterOptions, type AgentAdapterRequest } from './types.ts';
 
+/**
+ * The exact source text of each member of a top-level JSON object, by key. `json` must already have passed
+ * `JSON.parse` as an object, so only string and nesting boundaries are tracked here; every loop also stops at the end
+ * of the text, so other input cannot hang it. Refuses a key that appears twice, where `JSON.parse` would silently keep
+ * the last value.
+ */
+function topLevelMembers(json: string): Map<string, string> {
+  const members = new Map<string, string>();
+  const isSpace = (char: string | undefined) => char === ' ' || char === '\t' || char === '\n' || char === '\r';
+  let at = 0;
+  const skipSpace = () => { while (isSpace(json[at])) at++; };
+  const skipString = () => { // `at` is on the opening quote; leaves `at` after the closing one.
+    for (at++; at < json.length && json[at] !== '"'; at++) if (json[at] === '\\') at++;
+    at++;
+  };
+  skipSpace(); at++; // '{'
+  for (skipSpace(); at < json.length && json[at] !== '}'; skipSpace()) {
+    const keyStart = at; skipString();
+    const key = JSON.parse(json.slice(keyStart, at)) as string;
+    skipSpace(); at++; skipSpace(); // ':'
+    const valueStart = at;
+    if (json[at] === '"') skipString();
+    else if (json[at] === '{' || json[at] === '[') {
+      let depth = 0;
+      do {
+        if (json[at] === '"') { skipString(); continue; }
+        if (json[at] === '{' || json[at] === '[') depth++;
+        else if (json[at] === '}' || json[at] === ']') depth--;
+        at++;
+      } while (depth > 0 && at < json.length);
+    } else while (at < json.length && !isSpace(json[at]) && json[at] !== ',' && json[at] !== '}') at++;
+    if (members.has(key)) throw new Error('Claude returned a malformed output envelope.');
+    members.set(key, json.slice(valueStart, at));
+    skipSpace(); if (json[at] === ',') at++;
+  }
+  return members;
+}
+
 /**
  * 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`. With `--json-schema` (planning), a successful answer needs
- * `subtype: "success"` and is the schema-validated `structured_output` object, returned as JSON text; `result` then
- * holds only Claude's prose. Plain-text envelopes without a `subtype` are still accepted.
+ * `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 envelope that repeats a top-level key is malformed, whatever it reports. `result` then holds only
+ * Claude's prose. Plain-text envelopes without a `subtype` are still accepted.
  */
 export function parseClaudeOutput(raw: Buffer, structured = false): { text: string; providerFailed: boolean } {
-  const envelope = JSON.parse(new TextDecoder('utf-8', { fatal: true }).decode(raw)) as
+  const source = new TextDecoder('utf-8', { fatal: true }).decode(raw);
+  const envelope = JSON.parse(source) as
     { subtype?: unknown; result?: unknown; is_error?: unknown; structured_output?: unknown };
+  // Read every member before classifying: JSON.parse keeps only the last of a repeated key, such as `is_error`.
+  const members = structured && envelope !== null && typeof envelope === 'object' && !Array.isArray(envelope)
+    ? topLevelMembers(source) : null;
   if (typeof envelope.is_error !== 'boolean' || (envelope.result !== undefined && typeof envelope.result !== 'string')
     || (envelope.subtype !== undefined && typeof envelope.subtype !== 'string'))
     throw new Error('Claude returned a malformed output envelope.');
@@ -31,7 +76,7 @@ export function parseClaudeOutput(raw: Buffer, structured = false): { text: stri
   const answer = envelope.structured_output;
   if (answer === null || typeof answer !== 'object' || Array.isArray(answer))
     throw new Error('Claude returned no structured output for a schema-constrained answer.');
-  return Object.freeze({ text: JSON.stringify(answer), providerFailed: false });
+  return Object.freeze({ text: members!.get('structured_output')!, providerFailed: false });
 }
 
 /**

docs/implementation/agent-isolation.md

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

docs/implementation/plan-library-alignment.md

@@ -21,12 +21,12 @@ Issue #6 tracked the v1 plan library contract and the runner obligations that ca
 | 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) |
 | 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) |
 | 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 |
-| 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 |
+| 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 |
 | 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") |
 | Model budget before spawn | Moved to #163 | — |
 | Planning profile probed at startup | Moved to #164 | — |
 | Docker daemon log limits | Moved to #165 | — |
-| Claude structured output under the v1 parsing contract | Moved to #166 (found during this change) | — |
+| 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") |
 
 ## Schema loading
 

test/agent-adapter.test.ts

@@ -140,26 +140,75 @@ describe('production agent adapters', () => {
     expect(parseClaudeOutput(raw, true)).toEqual({ text: answer, providerFailed: false });
   });
 
-  it('re-serializes a structured answer, so decoded text can exceed its raw bytes (#166 may change this)', () => {
-    // Describes today's decoder, which re-serializes the answer (#166 may change that): JSON.parse then JSON.stringify
-    // writes 1e20 in full, so 4 raw bytes become 21. The supervisor bounds the decoded text itself; see the
-    // decoded-output limit cases in agent-supervisor.test.ts.
-    const raw = structuredEnvelope(JSON.stringify({ n: [] }).replace('[]', '[' + Array(1000).fill('1e20').join(',') + ']'), 8 * 1024);
-    const { text } = parseClaudeOutput(raw, true);
-    expect(Buffer.byteLength(text)).toBeGreaterThan(raw.length);
+  it('returns a structured answer as the exact text of structured_output in the envelope, whatever surrounds it', () => {
+    // Spacing, escapes, number spellings and brackets inside strings all survive, and so do members on either side.
+    const answer = '{ "summary" : "caf\\u00e9 } ] \\" \\\\", "n": [1e20, 1.0, -0E+0],\n "nested": {"a": [{}, []]} }';
+    const raw = Buffer.from('{"type":"result","subtype":"success","is_error":false,"result":"text with } and \\" in it",'
+      + `"structured_output":\t${answer}\n, "usage": {"input_tokens": 1}, "end": null}`);
+    expect(parseClaudeOutput(raw, true)).toEqual({ text: answer, providerFailed: false });
+    expect(parseClaudeOutput(Buffer.from(`{"subtype":"success","is_error":false,"structured_output":${answer}}`), true).text)
+      .toBe(answer);
+  });
+
+  // #166: re-encoding the answer once let a duplicate key or an inexact integer reach the plan parser already resolved.
+  const validPlan = { schema_version: 1, issue: 1, revision: 1, summary: 'Plan', questions: [],
+    items: [{ id: 'P1', title: 'Change', intent: 'Improve', files: [{ path: 'a', kind: 'edit', renamed_from: null, change: 'Change' }],
+      acceptance: [{ type: 'check', text: 'Works' }], depends_on: [] }] };
+  const draft = () => prepareDraft({ context: { identity: { repositoryId: 'repo', taskId: 'task', planId: 'plan' }, issue: 1,
+    baseEntries: [{ path: 'a', kind: 'file' }], pathKey: path => path, allowedCommands: [] }, requestId: 'request', revision: 1,
+    repo: { name: 'repo', baseRef: 'main', baseSha: 'a'.repeat(40), paths: ['a'] },
+    issue: { number: 1, title: 'Fix', body: '', comments: [] }, approvedLessons: [], feedback: '' });
+  it.each([
+    ['a duplicate key', JSON.stringify(validPlan).replace('"summary":"Plan"', '"summary":"Plan","summary":"Other"'), 'Duplicate key: summary'],
+    ['an inexact integer', JSON.stringify(validPlan).replace('"revision":1', '"revision":1.0000000000000001'), 'Integer is not exactly representable.'],
+  ])('lets draft validation refuse %s in a structured answer', (_case, answer, refusal) => {
+    const { text } = parseClaudeOutput(structuredEnvelope(answer, answer.length + 256), true);
+    expect(text).toBe(answer);
+    expect(() => draft().validate(text)).toThrow(refusal);
+    // The same plan without the defect is accepted, so the refusal is the defect's.
+    const valid = JSON.stringify(validPlan);
+    expect(draft().validate(parseClaudeOutput(structuredEnvelope(valid, valid.length + 256), true).text).value).toEqual(validPlan);
+  });
+
+  it('refuses a structured envelope that repeats a top-level key, before reading what it reports', () => {
+    for (const envelope of ['{"subtype":"success","is_error":false,"structured_output":{"a":1},"structured_output":{"a":2}}',
+      '{"subtype":"success","is_error":true,"is_error":false,"structured_output":{"a":1}}',
+      // The last is_error would otherwise make this a provider failure rather than a refusal.
+      '{"subtype":"success","is_error":false,"is_error":true,"structured_output":{"a":1}}',
+      // Keys are compared decoded, so another spelling of the same key is a repeat.
+      '{"subtype":"success","is_error":false,"structured_output":{"a":1},"structured\\u005foutput":{"a":2}}'])
+      expect(() => parseClaudeOutput(Buffer.from(envelope), true)).toThrow('malformed');
+    // Without a schema the envelope is read as before.
+    expect(parseClaudeOutput(Buffer.from('{"is_error":false,"result":"a","result":"b"}'))).toEqual({ text: 'b', providerFailed: false });
+  });
+
+  // The base refused these too. They guard the scanner's precondition: it is given only an object envelope.
+  it.each(['[{"is_error":false}]', '[]', '"text"', 'true'])('refuses a structured envelope that is not an object: %s', envelope => {
+    expect(() => parseClaudeOutput(Buffer.from(envelope), true)).toThrow('malformed');
+  });
+
+  it('finds structured_output however the envelope is spaced or its key is spelled', () => {
+    for (const envelope of [' \r\n{ "subtype" : "success" , "is_error" : false , "structured_output" : {"a":1} } \n',
+      '{"subtype":"success","is_error":false,"structured\\u005foutput":{"a":1}}'])
+      expect(parseClaudeOutput(Buffer.from(envelope), true).text).toBe('{"a":1}');
+  });
+
+  it('applies the 1 MiB plan parser limit to the bytes of structured_output in the envelope, not to a re-encoding', () => {
+    // Spacing inside the answer counts: re-encoded, this answer would be far below the limit.
+    const answer = `{ "summary": "x" ${' '.repeat(MAX_SOURCE_BYTES)}}`;
+    expect(Buffer.byteLength(JSON.stringify(JSON.parse(answer)))).toBeLessThan(MAX_SOURCE_BYTES);
+    const { text } = parseClaudeOutput(structuredEnvelope(answer, answer.length + 256), true);
+    expect(text).toBe(answer);
+    expect(() => draft().validate(text)).toThrow('Input size exceeds 1 MiB.');
   });
 
   it('applies the 1 MiB plan parser limit to the decoded answer at the exact byte in draft validation', () => {
-    const draft = prepareDraft({ context: { identity: { repositoryId: 'repo', taskId: 'task', planId: 'plan' }, issue: 1,
-      baseEntries: [{ path: 'a', kind: 'file' }], pathKey: path => path, allowedCommands: [] }, requestId: 'request', revision: 1,
-      repo: { name: 'repo', baseRef: 'main', baseSha: 'a'.repeat(40), paths: ['a'] },
-      issue: { number: 1, title: 'Fix', body: '', comments: [] }, approvedLessons: [], feedback: '' });
     const answerOf = (bytes: number) => JSON.stringify({ summary: 'x'.repeat(bytes - '{"summary":""}'.length) });
     for (const [bytes, accepted] of [[MAX_SOURCE_BYTES, true], [MAX_SOURCE_BYTES + 1, false]] as const) {
       const { text } = parseClaudeOutput(structuredEnvelope(answerOf(bytes), bytes + 1024), true);
       expect(Buffer.byteLength(text)).toBe(bytes);
       // At the limit the answer parses and fails only later, on its missing revision; one byte more fails in the parser.
-      expect(() => draft.validate(text)).toThrow(accepted ? 'Response revision mismatch.' : 'Input size exceeds 1 MiB.');
+      expect(() => draft().validate(text)).toThrow(accepted ? 'Response revision mismatch.' : 'Input size exceeds 1 MiB.');
     }
   });
 

test/planning-extraction.test.ts

@@ -66,6 +66,7 @@ it.each([
   expect(extract(raw)).toMatchObject({ providerFailed: true });
 });
 
+// `structured` writes its envelope with JSON.stringify, so the exact text of the answer is JSON.stringify(answer).
 it('returns structured_output as JSON text and ignores Claude\'s prose result', () => {
   const answer = reply();
   expect(extract(structured(answer))).toEqual({ text: JSON.stringify(answer), providerFailed: false });