diff --git a/agents/adapters/claude.ts b/agents/adapters/claude.ts index ef01af72..1bf788a3 100644 --- a/agents/adapters/claude.ts +++ b/agents/adapters/claude.ts @@ -47,17 +47,18 @@ function topLevelMembers(json: string): Map { /** * 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. 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. + * 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 + * 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 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 }; + { subtype?: unknown; result?: unknown; is_error?: unknown; structured_output?: unknown; errors?: 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; @@ -65,8 +66,11 @@ export function parseClaudeOutput(raw: Buffer, structured = false): { text: stri || (envelope.subtype !== undefined && typeof envelope.subtype !== 'string')) throw new Error('Claude returned a malformed output envelope.'); const prose = envelope.result ?? ''; - if (envelope.is_error || (envelope.subtype !== undefined && envelope.subtype !== 'success')) - return Object.freeze({ text: prose, providerFailed: true }); + if (envelope.is_error || (envelope.subtype !== undefined && envelope.subtype !== 'success')) { + // A run the CLI stops itself, such as at `--max-turns` (#163), gives no `result`, only its `errors` strings. + const errors = Array.isArray(envelope.errors) ? envelope.errors.filter(error => typeof error === 'string') : []; + return Object.freeze({ text: envelope.result || errors.join('; '), providerFailed: true }); + } if (!structured) { if (envelope.result === undefined) throw new Error('Claude returned a malformed output envelope.'); return Object.freeze({ text: prose, providerFailed: false }); diff --git a/agents/policy.ts b/agents/policy.ts index 9d6797e1..4d1701fe 100644 --- a/agents/policy.ts +++ b/agents/policy.ts @@ -76,6 +76,21 @@ export function dispatchApprovedCommand(policy: PhasePolicy, argv: readonly s return execute(Object.freeze([...argv])); } +/** + * 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 + * 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, + * outside the container), which the adapter reports as a provider failure. See docs/implementation/agent-isolation.md, + * "Planning model budget". + */ +export const MAX_PLANNING_INPUT_BYTES = 64 * 1024; +export const PLANNING_MAX_TURNS = 50; + /** * 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 @@ -86,11 +101,12 @@ export function createClaudeCommand(policy: PhasePolicy, prompt: string, schema? 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.'); - const schemaArguments = schema === undefined ? [] : ['--json-schema', assertPlanningSchema(schema)]; + const planningArguments = schema === undefined ? [] : ['--json-schema', + assertPlanningInput(prompt, assertPlanningSchema(schema)), '--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. - return command(policy, ['claude', '--print', '--output-format', 'json', ...schemaArguments, '--restricted', + return command(policy, ['claude', '--print', '--output-format', 'json', ...planningArguments, '--restricted', '--strict-mcp-config', '--mcp-config', '{"mcpServers":{}}', '--disable-slash-commands', '--no-chrome', '--permission-prompts', 'none', '--permission-mode', writable ? 'acceptEdits' : 'plan', '--tools', allowed, '--allowedTools', allowed, '--disallowedTools', 'Bash,WebFetch,WebSearch,NotebookEdit', '--add-dir', @@ -117,6 +133,16 @@ 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 { + 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.'); + 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. diff --git a/docs/implementation/agent-isolation.md b/docs/implementation/agent-isolation.md index f5a1ea7d..5aa70ba1 100644 --- a/docs/implementation/agent-isolation.md +++ b/docs/implementation/agent-isolation.md @@ -36,6 +36,7 @@ Each row is a T9 requirement for the Docker suite. The suite fails if any row fa | A read-only phase cannot write `/work` | `agent-container`: phase worktree for all five phases | | Planning and questions cannot run a process | `agent-policy`: tool sets exclude the command tool, and command dispatch refuses these phases | | Codex runs in no phase, because it cannot read code without its shell (see [Codex is refused in every phase](#codex-is-refused-in-every-phase)) | `agent-policy`: the Codex command refuses all five phases; `agent-adapter`: the Codex start call refuses them and allocates nothing; `runner-execution`: an execute task with Codex is refused before task storage exists | +| A planning request over its model budget is refused before the start, and the planning command passes `--max-turns 50` (see [planning model budget](#planning-model-budget) for what is and is not shown about the stop) | `agent-policy`: a prompt and schema of exactly 64 KiB build a command with `--max-turns 50`, and one byte more is refused; `agent-adapter`: the start call refuses an over-budget request and allocates nothing, and an `error_max_turns` envelope is a provider failure whose text is its `errors`; `planning-provider`: the planning error quotes the turn limit | | A planning answer is checked against the mounted schema | `agent-policy`: only the Claude planning command passes `--json-schema`; `agent-container`: the profile refuses a command whose schema differs from the mounted file; `agent-adapter`: the answer is read from `structured_output` | | Task and scratch byte and inode limits hold | `agent-container`: task capacity; scratch capacity for Codex and Claude (`/tmp`, `HOME`, `CODEX_HOME`, output directory) | | Hard links and alias writes from `.git/config` and objects fail, and metadata stays unchanged | `agent-container`: metadata alias probe in planning, review and execute, with a digest of `.git` before and after | @@ -101,6 +102,36 @@ Use only these entry points to run an agent: 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)). + - **Planning model budget (#163).** `createClaudeCommand` checks it when it + builds the planning command, so the adapter's start call throws before it creates a container or network. The + runner's host clone and task storage already exist by then, and the runner removes them. The budget has two parts: + - *Input, before the start.* The prompt and the schema together must be at most `MAX_PLANNING_INPUT_BYTES` (64 KiB) + of UTF-8: the 32 KiB prompt plus 32 KiB of schema text. A larger request is refused with an error that gives its + size, split into prompt and schema, and asks the person to narrow the request (the issue and comments, the paths, + the lessons, the feedback or the previous plan). Bytes stand in for tokens because Claude's tokenizer is not + available offline. A count of bytes is an upper bound on the tokens if every token covers at least one byte; this + is assumed, not verified against Claude's tokenizer. `prepareDraft` and `prepareSuggestions` already limit the + prompt and the schema text to 32 KiB each, so this check refuses nothing they produce. It is a backstop for a + caller that skips them. A test checks a draft at the prompt limit with today's schema; no test fails if the + 32 KiB schema bound in `core/planning-author.ts` is removed. It does not count Claude Code's own system prompt + and tool definitions, or the tokens of the answer. + - *The run.* The command passes `--max-turns 50` (`PLANNING_MAX_TURNS`). If each turn sends at most one context + window, a turn limit bounds the tokens a run can use with no tokenizer. That is assumed: calls the CLI makes for + itself, such as compaction, may not count as turns, and the bound is loose (about 50 context windows; the CLI + reported `num_turns: 2` when it stopped at a limit of 1, so it may make one call more than the limit). Claude + Code 2.1.281 accepts the flag in `--print` mode (its help hides it) and stops at the limit with an + `error_max_turns` result and exit status 1. This was checked with `--max-turns 1` in a run of the 2.1.281 macOS + arm64 build outside the container; the Linux build in the image was read from its bundled source only. The + adapter reports that result as a provider failure whose text is the result's `errors` strings ("Reached maximum + number of turns (1)" in that run). That holds in every Claude phase, so a failed question run quotes them too. + The planning error then names the 50-turn limit, asks the person to narrow the request, and quotes that text. The + planning error finds the stop by that text, so other output that contains it, such as Claude's own `result`, gets + the same advice; the output is quoted either way. A token or dollar limit was not chosen: `--max-budget-usd` + counts the dollar cost of API calls, which is not what an OAuth subscription token pays, and the CLI has no flag + that limits the tokens of a whole run. 50 turns is about 12 s for each turn in the ten-minute budget; a planning + run reads files and answers, so most turns are reads. Lower it if runs are seen to loop; raise it only with a + recorded run that needs more. + - *Other phases.* They do not pass `--max-turns` yet; their runs are bounded by time and output bytes only. - **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 diff --git a/docs/implementation/plan-library-alignment.md b/docs/implementation/plan-library-alignment.md index 7e76cac8..278ccb3d 100644 --- a/docs/implementation/plan-library-alignment.md +++ b/docs/implementation/plan-library-alignment.md @@ -23,7 +23,7 @@ Issue #6 tracked the v1 plan library contract and the runner obligations that ca | 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`. 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 | — | +| Model budget before spawn | Moved to #163. *Later:* done in #163 | `MAX_PLANNING_INPUT_BYTES` and `PLANNING_MAX_TURNS` in `agents/policy.ts`; `test/agent-policy.test.ts` ("accepts a planning request at exactly its model budget and refuses one byte more"); [agent isolation](agent-isolation.md#planning-model-budget) | | 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). *Later:* fixed in #166 | `agents/adapters/claude.ts`; `test/agent-adapter.test.ts` ("lets draft validation refuse %s in a structured answer") | diff --git a/prompts/plan-author.md b/prompts/plan-author.md index 2aa9f9b8..f1f635d8 100644 --- a/prompts/plan-author.md +++ b/prompts/plan-author.md @@ -40,12 +40,14 @@ Reject NUL input and any oversized request before spawning; do not silently truncate any source. Report which input exceeded the budget and ask the person to narrow the issue/comments, selected path set, or lesson set before retrying. - Also enforce the selected pinned model profile's token budget (including - schema/system overhead and reserved answer tokens) before launch; missing - budget/tokenizer support fails closed. The launch probe must test these caps - on the supported container OS/CLI; lower them if needed, never raise them - automatically. Tests include one huge field, many small fields exceeding the - aggregate, post-escaping expansion, and exact boundary accepted/rejected inputs. + Also enforce the model budget before launch. With no tokenizer available + offline, lane D counts the prompt and schema in UTF-8 bytes (64 KiB together) + and bounds the run with Claude's --max-turns (#163; see agent-isolation.md, + "Planning model budget"; no probe tests them yet, #164). The launch probe + must test these caps on the supported container OS/CLI; lower them if + needed, never raise them automatically. Tests include one huge field, many + small fields exceeding the aggregate, post-escaping expansion, and exact + boundary accepted/rejected inputs. In every serialized data string, escape <, >, and & as JSON Unicode escapes. Never insert raw source text or recursively render placeholders inside serialized values. These wrappers do not prevent semantic prompt injection: container permissions, diff --git a/runner/planning-provider.ts b/runner/planning-provider.ts index 8f9192bf..2e04ea18 100644 --- a/runner/planning-provider.ts +++ b/runner/planning-provider.ts @@ -3,6 +3,7 @@ import { tmpdir } from 'node:os'; import { basename, dirname, join } from 'node:path'; import type { InvocationInput, InvocationResult, StopReason } from '../agents/contract.ts'; import type { TaskStorageLimits } from '../agents/container/storage.ts'; +import { PLANNING_MAX_TURNS } from '../agents/policy.ts'; import type { AuthorProvider, AuthorRequest } from '../core/planning-author.ts'; import { allocations, exceedsStorage, RetainedStorage, runReadOnlyAgent, sameContext, type ContainerDependencies, type LeftoverPolicy, type Provider, type ReadOnlyFeature, type ReadOnlyRun, type RepositorySize } from './question-container.ts'; @@ -94,7 +95,13 @@ export function planningOutput(result: InvocationResult, invocation: InvocationI 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}` : ''}`); + // Claude Code's own text when `--max-turns` stops the run (#163). The decoder keeps no subtype, so this matches the + // text: a stop without it gets the general advice, and other output that contains it (Claude's own `result` text + // can) gets the turn-limit advice. Only the advice can be wrong; the text is still quoted. + const advice = detail.includes('Reached maximum number of turns') + ? `Planning stopped at its ${PLANNING_MAX_TURNS}-turn limit before Claude answered. Narrow the request, then retry.` + : 'Check its sign-in and usage limits.'; + throw new Error(`Claude could not write the plan. ${advice}${detail ? ` Claude said: ${detail}` : ''}`); } return result.stdout; } diff --git a/test/agent-adapter.test.ts b/test/agent-adapter.test.ts index 890ba583..a19a9188 100644 --- a/test/agent-adapter.test.ts +++ b/test/agent-adapter.test.ts @@ -9,9 +9,11 @@ import { startCodexInvocation } from '../agents/adapters/codex.ts'; import { isInvocationActive, OUTPUT_LIMITS, retainSetupCleanup } from '../agents/adapters/supervisor.ts'; import { capAdapterInvocationBudget, createAdapterInvocationBudget, createInvocationBudget } from '../agents/adapters/types.ts'; import { captureInvocation } from '../agents/contract.ts'; +import { MAX_PLANNING_INPUT_BYTES } from '../agents/policy.ts'; import { MAX_SOURCE_BYTES } from '../core/parse-v1.ts'; import { prepareDraft } from '../core/planning-author.ts'; const TEST_RUNNER_OWNER = '0123456789abcdef0123456789abcdef'; +const SCHEMA_TEXT = '{"type":"object"}'; describe('production agent adapters', () => { const capturedInvocation = (attemptId: string, deadline: number, vendor: 'codex' | 'claude' = 'codex') => @@ -39,10 +41,11 @@ describe('production agent adapters', () => { .toEqual({ text: '{"title":"Plan","items":[]}', providerFailed: false }); // Without a schema the prose result is still the answer. expect(parseClaudeOutput(Buffer.from(success))).toEqual({ text: 'Here is the plan.', providerFailed: false }); - // A failed structured run can omit result; it is a provider failure, not malformed output. + // A failed structured run can omit result; it is a provider failure, not malformed output, and its errors are + // the text. expect(parseClaudeOutput(Buffer.from( '{"type":"result","subtype":"error_max_structured_output_retries","is_error":true,"errors":["x"]}'), true)) - .toEqual({ text: '', providerFailed: true }); + .toEqual({ text: 'x', providerFailed: true }); for (const missing of ['{"subtype":"success","result":"{\\"title\\":\\"Plan\\"}","is_error":false}', '{"subtype":"success","result":"","is_error":false,"structured_output":null}', '{"subtype":"success","result":"","is_error":false,"structured_output":[1]}', @@ -57,8 +60,18 @@ describe('production agent adapters', () => { it.each([false, true])('treats any non-success subtype as a provider failure (structured: %s)', structured => { for (const envelope of ['{"subtype":"error_max_turns","is_error":false,"structured_output":{"a":1}}', '{"subtype":"error_during_execution","is_error":true}', - '{"subtype":"error_max_structured_output_retries","is_error":false,"result":"gave up"}']) + '{"subtype":"error_max_structured_output_retries","is_error":false,"result":"gave up"}', + // What Claude Code 2.1.281 (macOS arm64 build) wrote, with exit status 1, when `--max-turns 1` stopped a real + // run (#163): no result member, and an errors list. Members the parser does not read, such as usage, are left out. + '{"type":"result","subtype":"error_max_turns","is_error":true,"num_turns":2,"errors":["Reached maximum number of turns (1)"]}']) expect(parseClaudeOutput(Buffer.from(envelope), structured)).toMatchObject({ providerFailed: true }); + expect(parseClaudeOutput(Buffer.from('{"subtype":"error_max_turns","is_error":true,' + + '"errors":["Reached maximum number of turns (50)",3]}'), structured)) + .toEqual({ text: 'Reached maximum number of turns (50)', providerFailed: true }); + expect(parseClaudeOutput(Buffer.from('{"subtype":"error_during_execution","is_error":true,"result":"r","errors":["e"]}'), + structured)).toEqual({ text: 'r', providerFailed: true }); + expect(parseClaudeOutput(Buffer.from('{"subtype":"error_during_execution","is_error":true,"result":"","errors":["e"]}'), + structured)).toEqual({ text: 'e', providerFailed: true }); expect(() => parseClaudeOutput(Buffer.from('{"subtype":1,"is_error":false,"result":"x"}'), structured)) .toThrow('malformed'); expect(() => parseClaudeOutput(Buffer.from('{"subtype":"success","is_error":false,"result":3}'), structured)) @@ -104,6 +117,13 @@ describe('production agent adapters', () => { expect(() => startClaudeInvocation({ ...missing, inputDirectory: root, invocation: invalid }, 'token')) .toThrow('"type": "object"'); expect(isInvocationActive(invalid.attemptId)).toBe(false); + // A request over the planning model budget is refused there too (#163). + writeFileSync(join(root, 'schema.json'), SCHEMA_TEXT); + const overBudget = captureInvocation({ ...planning, attemptId: 'claude-over-budget' }); + const prompt = 'p'.repeat(MAX_PLANNING_INPUT_BYTES - Buffer.byteLength(SCHEMA_TEXT) + 1); + expect(() => startClaudeInvocation({ ...missing, prompt, inputDirectory: root, invocation: overBudget }, 'token')) + .toThrow('model budget'); + expect(isInvocationActive(overBudget.attemptId)).toBe(false); } finally { rmSync(root, { recursive: true, force: true }); } expect(isInvocationActive(planning.attemptId)).toBe(false); // Questions carry no schema, so the same request gets past the schema read to the image check. diff --git a/test/agent-policy.test.ts b/test/agent-policy.test.ts index 75f4a5c9..ca8df504 100644 --- a/test/agent-policy.test.ts +++ b/test/agent-policy.test.ts @@ -1,7 +1,8 @@ import { describe, expect, it, vi } from 'vitest'; import { captureInvocation, type InvocationInput, type Phase } from '../agents/contract.ts'; import { assertAgentCommand, assertAgentTool, assertCommandSchema, codexBaseArguments, createClaudeCommand, - createCodexCommand, createPhasePolicy, createRunnerCommand, dispatchApprovedCommand, MAX_COMMAND_SCHEMA_BYTES } from '../agents/policy.ts'; + createCodexCommand, createPhasePolicy, createRunnerCommand, dispatchApprovedCommand, MAX_COMMAND_SCHEMA_BYTES, + MAX_PLANNING_INPUT_BYTES, PLANNING_MAX_TURNS } from '../agents/policy.ts'; import planSchema from '../schema/versions/1/plan.schema.json' with { type: 'json' }; import editSchema from '../schema/versions/1/plan-edit.schema.json' with { type: 'json' }; const TEST_RUNNER_OWNER = '0123456789abcdef0123456789abcdef'; @@ -98,6 +99,26 @@ describe('agent phase policy', () => { } }); + it('accepts a planning request at exactly its model budget and refuses one byte more', () => { + const planning = () => createPhasePolicy(request('planning')); + // A multibyte character proves the budget counts UTF-8 bytes, not UTF-16 code units. + const fill = (bytes: number) => '\u00e9' + 'p'.repeat(bytes - Buffer.byteLength(SCHEMA) - 2); + const exact = fill(MAX_PLANNING_INPUT_BYTES); + expect(Buffer.byteLength(exact) + Buffer.byteLength(SCHEMA)).toBe(64 * 1024); + expect(createClaudeCommand(planning(), exact, SCHEMA).argv).toEqual(['claude', '--print', '--output-format', 'json', + '--json-schema', SCHEMA, '--max-turns', '50', '--restricted', '--strict-mcp-config', '--mcp-config', + '{"mcpServers":{}}', '--disable-slash-commands', '--no-chrome', '--permission-prompts', 'none', '--permission-mode', + 'plan', '--tools', 'Read,Glob,Grep', '--allowedTools', 'Read,Glob,Grep', '--disallowedTools', + 'Bash,WebFetch,WebSearch,NotebookEdit', '--add-dir', '/run/codeboost-input', '--', exact]); + expect(PLANNING_MAX_TURNS).toBe(50); + expect(() => createClaudeCommand(planning(), fill(MAX_PLANNING_INPUT_BYTES + 1), SCHEMA)) + .toThrow(`Planning request is ${64 * 1024 + 1} bytes (prompt ${64 * 1024 + 1 - Buffer.byteLength(SCHEMA)}, schema ` + + `${Buffer.byteLength(SCHEMA)}), over the 65536-byte model budget.`); + // Only planning has the budget: the other Claude phases carry no turn limit. + for (const phase of ['questions', 'review', 'execute', 'fix'] as const) + expect(createClaudeCommand(createPhasePolicy(request(phase)), 'Work.').argv).not.toContain('--max-turns'); + }); + it('refuses a planning schema that is not a bounded JSON object', () => { const planning = () => createPhasePolicy(request('planning')); for (const schema of ['', 'not json', '[]', 'null', '"string"', '{"type":"object"}\0', '{"type":"array"}', diff --git a/test/fixtures/planning/recorded/claude-draft.json b/test/fixtures/planning/recorded/claude-draft.json index 5ea49302..04972adc 100644 --- a/test/fixtures/planning/recorded/claude-draft.json +++ b/test/fixtures/planning/recorded/claude-draft.json @@ -3,7 +3,7 @@ "version": 1, "vendor": "claude", "mode": "draft", - "recordedAt": "2026-10-03T22:43:46.042Z", + "recordedAt": "2026-10-21T04:33:42.533Z", "provenance": { "image": "codeboost-agent:node26-codex0.153.4-claude2.1.281", "cliVersion": "2.1.281", @@ -11,14 +11,14 @@ "phase": "planning" }, "request": { - "requestId": "6185171e-49c5-48c3-8483-a56e4e758571", + "requestId": "35e4a401-57dc-40ee-a604-735b3c664d13", "revision": 1, "issue": 412, "baseSha": "b8e168bb67541c4ad21c00c8f6873f882d26521b", "promptSha256": "adec3564684205bb51346afaaf5d36abe5f29e26079a9c5002617ecc9e8719b4", "schemaSha256": "55b04a7df0f02165460c4ebc5cf40a00ca2a0ca8fa56a3bf8434aacfe8350179", "filesSha256": "f0ff86518eb6ac0bc07aa7167da6f6e0c72695ae75f87d6e1bf86e06cec7543e", - "commandSha256": "35cc440bb5ad4b80ad77bf80be3f279d5e9b6d8846cdd88fe52aa4e5c4b5ecd7" + "commandSha256": "49e1e3256e897992fb184377392f010a44e96a48b4cce265bf3a7a6def08aa2c" }, - "output": "{\"schema_version\":1,\"issue\":412,\"revision\":1,\"summary\":\"Remove the leftover call counter in label() so it returns deterministic text for the same input every time, and add a test that calls it twice.\",\"items\":[{\"id\":\"P1\",\"title\":\"Make label() deterministic and test repeated calls\",\"intent\":\"label() currently uses a module-level call counter left over from debugging, causing it to alternate output text ('value N' vs 'even call: N') across calls with the same input. Remove the counter so the function is pure and always returns the same text for the same input, and add a regression test that calls label() twice with the same argument.\",\"files\":[{\"path\":\"src/example.ts\",\"kind\":\"edit\",\"renamed_from\":null,\"change\":\"Remove the module-level `calls` variable and the increment logic inside label(). Change label(value) to always return `value ${value}` (the non-alternating branch), removing the parity check entirely.\"},{\"path\":\"test/example.test.ts\",\"kind\":\"edit\",\"renamed_from\":null,\"change\":\"Add a test case that calls label(3) twice and asserts both results are equal to each other (and match the expected `value 3` text), verifying the output no longer changes between calls.\"}],\"acceptance\":[{\"type\":\"cmd\",\"text\":\"npm test\"}],\"depends_on\":[]}],\"questions\":[]}" + "output": "{\"schema_version\":1,\"issue\":412,\"revision\":1,\"summary\":\"Remove the hidden call counter in label() so it returns deterministic output for the same input, and add a regression test that calls it twice.\",\"items\":[{\"id\":\"P1\",\"title\":\"Fix label() nondeterminism and add regression test\",\"intent\":\"label() currently tracks a module-level call counter and alternates its output format based on whether the call count is even or odd, so calling label(3) twice returns two different strings. The fix removes the counter so the function is pure and always returns the same text for the same input.\",\"files\":[{\"path\":\"src/example.ts\",\"kind\":\"edit\",\"renamed_from\":null,\"change\":\"Remove the module-level `calls` counter and the increment logic in label(). Change label(value) to always return `value ${value}` (drop the `even call: ${value}` branch), so the function is deterministic and side-effect free.\"},{\"path\":\"test/example.test.ts\",\"kind\":\"edit\",\"renamed_from\":null,\"change\":\"Add a test that calls label(3) twice and asserts both results are strictly equal to each other (and to the expected `value 3` text), covering the regression described in issue 412. Keep the existing 'labels a value' test.\"}],\"acceptance\":[{\"type\":\"cmd\",\"text\":\"npm test\"}],\"depends_on\":[]}],\"questions\":[]}" } diff --git a/test/fixtures/planning/recorded/claude-suggest.json b/test/fixtures/planning/recorded/claude-suggest.json index 90c66302..988a7b0d 100644 --- a/test/fixtures/planning/recorded/claude-suggest.json +++ b/test/fixtures/planning/recorded/claude-suggest.json @@ -3,7 +3,7 @@ "version": 1, "vendor": "claude", "mode": "suggest", - "recordedAt": "2026-10-03T22:44:48.704Z", + "recordedAt": "2026-10-21T04:34:17.068Z", "provenance": { "image": "codeboost-agent:node26-codex0.153.4-claude2.1.281", "cliVersion": "2.1.281", @@ -11,14 +11,14 @@ "phase": "planning" }, "request": { - "requestId": "9c972d36-a1f1-42ce-af32-b19b09fe4da5", + "requestId": "34189586-6238-4618-918c-7ab52e39b53b", "revision": 1, "issue": 412, "baseSha": "b8e168bb67541c4ad21c00c8f6873f882d26521b", "promptSha256": "9766b6b2f256691bf32c80799725b8880fd8772996d63030cf0ae677e1c942e5", "schemaSha256": "b3c46f36405c4c7c57c9d8d903b6759a5ba0cf9b17ce99e3bba670f6f2ac5473", "filesSha256": "f0ff86518eb6ac0bc07aa7167da6f6e0c72695ae75f87d6e1bf86e06cec7543e", - "commandSha256": "8bbe71dee7a100ed8d9a44797323031249a5777299055793ef4047c9124b854e" + "commandSha256": "39d91f75f3b4af45689582c696b6dc8e09a380991b6945a8a8e4ea28bcff6710" }, - "output": "{\"schema_version\":1,\"base_revision\":1,\"reply\":\"Split the fix and the test into two items: P1 removes the call counter so label() is pure, and P2 (depends on P1) adds a test to test/example.test.ts asserting assert.strictEqual(label(3), label(3)).\",\"edits\":[{\"op\":\"remove_file\",\"item\":\"P1\",\"summary\":\"Remove test/example.test.ts from P1's declared files\",\"reason\":\"The test is moving to its own item (P2) that depends on P1, per the one-concern-per-item rule.\",\"field\":null,\"value\":\"test/example.test.ts\",\"file\":null,\"check\":null,\"check_index\":null,\"depends_on\":null,\"new_item\":null},{\"op\":\"set_field\",\"item\":\"P1\",\"summary\":\"Update P1's intent to cover only the source fix\",\"reason\":\"P1 no longer includes the test, so its intent should describe only the code change.\",\"field\":\"intent\",\"value\":\"label(value) must depend only on value, not on how many times it has been called. Remove the hidden `calls` counter so the function is pure.\",\"file\":null,\"check\":null,\"check_index\":null,\"depends_on\":null,\"new_item\":null},{\"op\":\"add_item\",\"item\":\"P2\",\"summary\":\"Add item P2: test that label() is deterministic\",\"reason\":\"Per feedback, the test belongs in its own item that depends on the fix, with the exact assertion named.\",\"field\":null,\"value\":null,\"file\":null,\"check\":null,\"check_index\":null,\"depends_on\":null,\"new_item\":{\"id\":\"P2\",\"title\":\"Add test: label() returns the same text on repeated calls\",\"intent\":\"Cover the bug from issue 412 so a regression (like the old call counter) is caught: calling label with the same input twice must return identical text.\",\"files\":[{\"path\":\"test/example.test.ts\",\"kind\":\"edit\",\"renamed_from\":null,\"change\":\"Keep the existing 'labels a value' test. Add a new test, e.g. 'label returns the same text for repeated calls', that asserts assert.strictEqual(label(3), label(3)).\"}],\"acceptance\":[{\"type\":\"cmd\",\"text\":\"npm test\"},{\"type\":\"check\",\"text\":\"test/example.test.ts contains a test asserting assert.strictEqual(label(3), label(3))\"}],\"depends_on\":[\"P1\"]}}]}" + "output": "{\"schema_version\":1,\"base_revision\":1,\"reply\":\"Split the test out of P1 into a new P2 that depends on P1. P2 names the exact assertions it adds: label(3) called twice must return equal values, and that value must be the literal string \\\"value 3\\\".\",\"edits\":[{\"op\":\"remove_file\",\"item\":\"P1\",\"summary\":\"Remove test/example.test.ts from P1's declared files\",\"reason\":\"The test now belongs to its own item (P2), which depends on P1, so P1 should only touch the fix.\",\"field\":null,\"value\":\"test/example.test.ts\",\"file\":null,\"check\":null,\"check_index\":null,\"depends_on\":null,\"new_item\":null},{\"op\":\"add_item\",\"item\":\"P2\",\"summary\":\"Add P2: test that label(3) returns the same text on repeated calls\",\"reason\":\"The issue asks for a test calling label(3) twice; it belongs in its own item that depends on the P1 fix per the one-concern-per-item rule.\",\"field\":null,\"value\":null,\"file\":null,\"check\":null,\"check_index\":null,\"depends_on\":null,\"new_item\":{\"id\":\"P2\",\"title\":\"Add regression test for repeated label() calls\",\"intent\":\"Verify that label(value) is pure: calling it twice with the same argument returns the same string, closing out issue 412.\",\"files\":[{\"path\":\"test/example.test.ts\",\"kind\":\"edit\",\"renamed_from\":null,\"change\":\"Add a new test 'label returns the same text on repeated calls' that calls label(3) twice, storing both results, and asserts assert.equal(first, second) and assert.equal(first, 'value 3'). Keep the existing 'labels a value' test unchanged.\"}],\"acceptance\":[{\"type\":\"cmd\",\"text\":\"npm test\"},{\"type\":\"check\",\"text\":\"The new test calls label(3) exactly twice and asserts both calls return the identical string 'value 3', not just that both match /3/.\"}],\"depends_on\":[\"P1\"]}}]}" } diff --git a/test/planning-author.test.ts b/test/planning-author.test.ts index 18b65e3d..e3ef389f 100644 --- a/test/planning-author.test.ts +++ b/test/planning-author.test.ts @@ -1,5 +1,6 @@ import { readFileSync } from 'node:fs'; import { expect, it } from 'vitest'; +import { assertPlanningInput, MAX_PLANNING_INPUT_BYTES } from '../agents/policy.ts'; import { MAX_PROMPT_BYTES, prepareDraft, prepareSuggestions, type AuthorInput } from '../core/planning-author.ts'; import type { EditReply, Plan } from '../core/plan.ts'; @@ -74,6 +75,18 @@ it('rejects aggregate small fields and post-escaping expansion', () => { value.issue.comments = []; value.issue.body = '<'.repeat(6000); expect(() => prepareDraft(value)).toThrow(/KiB/); }); +it('fits the largest prompt and schema text it prepares into the planning model budget (#163)', () => { + // prepare() bounds the prompt and the schema text by MAX_PROMPT_BYTES each, so the backstop refuses none of them. + // This checks a draft at the prompt limit with today's schema file, not the schema bound itself. + expect(MAX_PLANNING_INPUT_BYTES).toBe(2 * MAX_PROMPT_BYTES); + const value = input(), first = prepareDraft(value).request; + value.issue.body = 'x'.repeat(MAX_PROMPT_BYTES - Buffer.byteLength(first.prompt)); + const { prompt, schemaText } = prepareDraft(value).request; + expect(Buffer.byteLength(prompt)).toBe(MAX_PROMPT_BYTES); + expect(Buffer.byteLength(schemaText)).toBeLessThanOrEqual(MAX_PROMPT_BYTES); + expect(() => assertPlanningInput(prompt, schemaText)).not.toThrow(); +}); + it('accepts exactly the prompt byte limit and rejects the next byte without truncation', () => { const value = input(); const overhead = Buffer.byteLength(prepareDraft(value).request.prompt); value.issue.body = 'x'.repeat(MAX_PROMPT_BYTES - overhead); diff --git a/test/planning-provider.test.ts b/test/planning-provider.test.ts index 7c848b2b..f83049d3 100644 --- a/test/planning-provider.test.ts +++ b/test/planning-provider.test.ts @@ -3,6 +3,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, expect, it } from 'vitest'; import type { InvocationHandle, InvocationInput, InvocationResult, StopReason } from '../agents/contract.ts'; +import { parseClaudeOutput } from '../agents/adapters/claude.ts'; import type { AgentAdapterRequest } from '../agents/adapters/types.ts'; import type { TaskFilesystems } from '../agents/container/storage.ts'; import { SuggestionCoordinator } from '../core/planning-suggestions.ts'; @@ -97,6 +98,19 @@ it('quotes at most 300 code units of a failed run\'s output, with whitespace col expect(await quote(' x \n\t y ')).toBe('x y'); }); +it('names the turn limit when Claude stops a planning run at --max-turns (#163)', async () => { + // The supervisor passes on the decoded text, and a nonzero exit, for a provider failure. The envelope is what Claude + // Code 2.1.281 (macOS arm64 build) wrote when `--max-turns 1` stopped a real run, less members nothing reads. + const { text } = parseClaudeOutput(Buffer.from('{"type":"result","subtype":"error_max_turns","is_error":true,' + + '"num_turns":2,"errors":["Reached maximum number of turns (1)"]}'), true); + await expect(provider(fakeDeps({ exitCode: 1, stdout: text }).deps).invoke(draftRequest(), new AbortController().signal)) + .rejects.toThrow('Claude could not write the plan. Planning stopped at its 50-turn limit before Claude answered. ' + + 'Narrow the request, then retry. Claude said: Reached maximum number of turns (1)'); + // Any other failure keeps the sign-in advice. + await expect(provider(fakeDeps({ exitCode: 1, stdout: 'login required' }).deps).invoke(draftRequest(), + new AbortController().signal)).rejects.toThrow('Check its sign-in and usage limits. Claude said: login required'); +}); + it.each([ ['another attempt', (input: InvocationInput) => ({ attemptId: 'someone-else', context: input.context })], ['another plan revision', (input: InvocationInput) => ({ context: { ...input.context, planRevision: 2 } })],