Skip to content

Enforce a pre-spawn model budget for planning invocations (#163) - #196

Merged
mchwang merged 19 commits into
mainfrom
claude/stoic-torvalds-d69f6d
Oct 21, 2026
Merged

mchwang merged 19 commits into
mainfrom
claude/stoic-torvalds-d69f6d

Conversation

@mchwang

@mchwang mchwang commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Closes #163.

What changes

  • createClaudeCommand (agents/policy.ts) now checks a planning model budget when it builds the command. startClaudeInvocation builds the command before it allocates a container or network, so this is before the agent process starts.
    • Input: the prompt and schema together must be at most 64 KiB of UTF-8 (MAX_PLANNING_INPUT_BYTES). A larger request is refused. The error gives the size, split into prompt and schema, and says what to narrow. Bytes stand in for tokens because Claude's tokenizer is not available offline (labelled as assumed). This equals the 32 KiB + 32 KiB limits that prepareDraft/prepareSuggestions already apply, so it is a backstop for callers that skip them.
    • Run: the planning command passes --max-turns 50 (PLANNING_MAX_TURNS). Other phases do not.
  • parseClaudeOutput (agents/adapters/claude.ts) uses the envelope's errors strings as the text when a failed result's result is missing or empty. This applies in every Claude phase.
  • planningOutput (runner/planning-provider.ts) names the turn limit when that text includes "Reached maximum number of turns". Every other failure keeps the sign-in advice.
  • Docs: a "Planning model budget" section and a gate-table row in docs/implementation/agent-isolation.md. The section records why turns and bytes were chosen, and not tokens or dollars. Also updated: the alignment row in plan-library-alignment.md and the builder note in prompts/plan-author.md, which is stripped before sending.

Evidence for the CLI behaviour

  • Claude Code 2.1.281 (the image's pinned version) accepts --max-turns in --print mode even though its help hides it. Checked in the Linux arm64 binary's bundled source.
  • A real run of the 2.1.281 macOS arm64 build ran outside the container with --print --output-format json --json-schema … --max-turns 1. It exited 1 with subtype: "error_max_turns", is_error: true, no result key, errors: ["Reached maximum number of turns (1)"] and num_turns: 2. parseClaudeOutput on that file returned { text: 'Reached maximum number of turns (1)', providerFailed: true }. The test fixtures use that envelope verbatim, minus members the parser does not read.
  • Not shown: the Linux CLI's stop behaviour inside the container. Startup probing is Probe the planning profile at runner startup #164.

Re-recorded planning fixtures

test/planning-recorded.test.ts hashes the exact planning argv, so adding --max-turns 50 made the old recordings stale. The maintainer re-recorded test/fixtures/planning/recorded/claude-{draft,suggest}.json with scripts/record-planning.ts, running Claude Code 2.1.281 in the planning container with the new argv. The result is commit 69eb218. Only commandSha256, requestId, recordedAt and output changed. The prompt, schema and files hashes are unchanged, and neither file contains any credential text. The hashes were not edited by hand.

Validation (head 69eb218)

Gate Result
Focused tests: planning-recorded, agent-policy, agent-adapter, planning-author, planning-provider 117 passed
Type check clean
Full non-Docker suite (local, at intermediate checkpoint e58d019, not on head 58df0ce) 76 failed / 2383 passed. 67 are 20 s timeouts from machine load (other sessions' Docker). 2 are the expected planning-recorded failures. The rest are timing failures in pre-merge/runner-publishing, which this change does not touch; on an isolated rerun a different pair failed. Not a clean gate: CI is authoritative.
CI (test, real-docker) at 58df0ce both failed only in planning-recorded (83/84 and 90/91 files passed); at 69eb218 both passed (test, real-docker)
Real-Docker locally not run (CI owns it)

Review record

Base aced300 (origin/main) for every round. Each independent round was a fresh general-purpose subagent, reviewing the full diff.

Round Type / reviewer Head Findings Changed
0 Self-review 645fa16 Planning run that hits the turn limit gave an empty, misleading error Added errors fallback (e2a11c6)
1 Independent agent e2a11c6 M: input check is only a backstop, and overhead is uncounted. L: tuning advice backwards. L: turns→tokens claim unlabelled. L: misleading sign-in advice on a turn stop. L: result:"" hid errors All fixed in e58d019
2 Independent agent e58d019 M: turn-limit fake not checked against the real tool. L: unlabelled claim. Nit: wrap Ran the real 2.1.281 CLI with --max-turns 1; docs and comments cite it (f3998cb..98a6d1f)
3 Independent agent 98a6d1f L: fixture values edited by hand (50 vs observed 1). L: "(50)" stated as fact. L: limit-plus-one call count. Nit: wraps Fixture made verbatim; docs reworded (91a942f)
4 Independent agent 91a942f L: gate row overclaimed. L: "before anything is allocated" only true in the adapter. L: plan-author note implied a probe Reworded (77c276c)
5 Independent agent 77c276c L: advice keyed on text (documented). L: 64 KiB not tied to MAX_PROMPT_BYTES. Nit: wrap Test ties the constants (b01806f, 03d9da0)
6 Independent agent 03d9da0 L: schema bound untested. Nit: wrap. Optional: check earlier in the runner Draft-at-limit test added. Earlier check declined: the docs state where the check runs, and it meets "before the agent process starts" (4cde897)
7 Independent agent 4cde897 L: "50-turn" advice vs quoted "(1)". L: error did not say which part was too large Size split into prompt and schema (e661388). Advice declined: production passes 50, and the fixture is verbatim
8 Independent agent e661388 L: advice did not name feedback or previous plan Fixed (2498340)
9 Independent agent 2498340 L: docs said the schema bound was tested Docs say what is and is not tested (e966141, 4bb2fc1)
10 Independent agent 4bb2fc1 L: errors text reaches every phase (undocumented). L: startsWith fragile. Nit: wrap Documented; switched to includes (0f16acc)
11 Independent agent 0f16acc Nit: Markdown list continuation Fixed (cd5a8fb)
12 Independent agent cd5a8fb L: other output containing the text gets turn-limit advice Documented in code and docs (58df0ce)
13 Independent agent 58df0ce No findings —
Copilot copilot-pull-request-reviewer 58df0ce High: planning recordings must be re-recorded, not hash-edited Re-recorded by the maintainer (69eb218)
14 Independent agent 69eb218 No findings (checked hashes, provenance, no credentials, replay 7/7) —
Copilot copilot-pull-request-reviewer 69eb218 No open issues (recording finding resolved) —

Round 13 (aced300...58df0ce) authorized the first Copilot request. Round 14 (aced300...69eb218) authorizes the re-request.

Review-lesson audit

  • Unverified CLI behaviour and fakes not checked against the real tool (rounds 2, 3): covered by the existing CLAUDE.md rule "A fake of an external tool … Check those answers against the real tool and name the version checked."
  • Claims stated wider than their evidence (rounds 1, 3, 4, 9, 12): covered by "Treat every behavioural claim … as something to verify … and give it a test that fails if the claim is false."
  • Wording, wrap and advice-text findings (rounds 5–11): one-off. They are specific to this change's messages and docs and have no general rule to extract.
  • Copilot's stale-recording finding is covered by docs/implementation/planning-acceptance.md: a changed Claude argv makes a recording stale, so it must be recorded again, not have its hashes edited.
  • No AGENTS.md/CLAUDE.md change needed.

Follow-ups

🤖 Generated with Claude Code

mchwang and others added 18 commits October 20, 2026 17:54
createClaudeCommand now refuses a planning request whose prompt and schema
together exceed 64 KiB of UTF-8 (a byte count standing in for tokens, as
Claude's tokenizer is not available offline), and passes --max-turns 50 so
the run itself is bounded by turns, not only by time and output bytes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…163)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e count (#163)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… turn-limit detection (#163)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…163)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…y phase (#163)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…163)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Both planning recordings require authenticated re-recording because the new command invalidates their fingerprints and breaks replay checks.

1 open issue
What changed in this PR

Adds pre-spawn planning budgets for #163 within the Claude invocation pipeline.

Changes:

  • Caps combined prompt/schema input at 64 KiB and planning runs at 50 turns.
  • Preserves provider error details and adds turn-limit guidance.
  • Documents budget assumptions and adds boundary tests.
File Description
test/​planning-provider.test.ts Tests turn-limit error guidance.
test/​planning-author.test.ts Checks prepared-input budget compatibility.
test/​agent-policy.test.ts Tests exact byte boundaries and planning argv.
test/​agent-adapter.test.ts Tests early refusal and error decoding.
runner/​planning-provider.ts Adds turn-limit retry guidance.
prompts/​plan-author.md Updates builder-only budget instructions.
docs/​implementation/​plan-library-alignment.md Records model-budget implementation.
docs/​implementation/​agent-isolation.md Documents limits, assumptions, and evidence.
agents/​policy.ts Enforces input and turn budgets.
agents/​adapters/​claude.ts Preserves error text when results are empty.

Review effort: Balanced

Comment thread agents/policy.ts
Recorded by the maintainer with scripts/record-planning.ts through the
planning container (Claude Code 2.1.281). Both answers validate and
replay through E2/E3.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Required exact-head full-suite and real-Docker CI results remain unconfirmed in the supplied evidence.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@mchwang
mchwang marked this pull request as ready for review October 21, 2026 05:25
@mchwang
mchwang merged commit e46cb68 into main Oct 21, 2026
3 checks passed
mchwang added a commit that referenced this pull request Oct 21, 2026
…review round 13)

Merges main (#195, #196, #197) and updates the status lines this PR
rewrites; tests that a held cancel refusal also survives Refresh.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enforce a pre-spawn model budget for planning invocations

2 participants