Repository navigation
fix(cli): the schema travels in the prompt, not as a forced tool_choice - #45
Merged
Merged
Conversation
Closes both 400 families that took central review to ~50% completion on 2026-08-10. W16 counted them separately and they looked like two problems: 22x deserialize: messages[1] unknown variant 'thinking' 2x Thinking mode does not support this tool_choice They are one cause with two exits. --json-schema makes Claude Code FORCE a tool call to obtain structured output. Forcing is a request shape some upstreams reject outright, which is the 2x. And a model that answers with thinking + text does not satisfy the forced call, so the CLI builds a SECOND request - which carries the thinking block back in its body, and Console Go's deserialiser accepts only 'text' blocks. That is the 22x. Remove the forcing and there is no second request, so both exits close. Routing-dependent, deterministic, and invisible to every channel health check: the request never reaches a channel, so AxonHub records nothing and every channel measures healthy while reviews die. Nothing about the review needed the force. The schema is now stated in the prompt and the model returns the JSON as its result. Validation was never in the flag: validateStage already validates every stage and already feeds the exact validation error back through a schema-repair budget. The flag bought a guarantee the engine was enforcing anyway, and charged for it in the one currency that fails closed - request shape. DELIBERATELY MINIMAL. -p and the read-only tool whitelist are untouched, even though both are arguable; that argument is #40's and mixing it in would slow down a fix nineteen tasks are waiting on. #40 is CHANGES_REQUESTED on a credential-proxy design question and is not landing soon, so this is cut against main rather than stacked on it. MUTATION-VERIFIED, because a passing test proves nothing here. Restoring --json-schema turns three independent assertions red, including "a thinking block was echoed back in request 1" - against a real CLI, with a mock upstream that emits an actual thinking content block. A plain-text mock has nothing to echo and would have passed for free. The thinking test also asserts there is exactly ONE request, because "no thinking echoed" alone would hold if the CLI merely stripped it, and a strip we do not control is not a guarantee. Three mock terminations switched from a StructuredOutput tool_use to plain text: that tool only existed because --json-schema registered it, and naming a tool the CLI never advertised hangs the turn until timeout, which reads as a broken sandbox rather than a broken mock. Verified: 227 tests, 227 pass, 0 skipped, native suite included. Claude-Session: https://claude.ai/code/session_01YFwT3QhJukpnZ1AodcYbRn Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough本次变更将 JSON Schema 从 Claude CLI 参数迁移到 stdin prompt，支持 fenced JSON 解析，并更新 thinking、文本回复、工具白名单及 Schema 转换测试。 ChangesClaude Schema Flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant runFreshClaude
participant ClaudeCLI
participant stdin
participant parseJsonResult
runFreshClaude->>runFreshClaude: 构造带 Schema 的 promptText
runFreshClaude->>ClaudeCLI: 启动 CLI，不传递 -p 或 --json-schema
runFreshClaude->>stdin: 写入 promptText
stdin->>ClaudeCLI: 传递 prompt 和 Schema
ClaudeCLI-->>runFreshClaude: 返回 thinking 与文本 SSE
runFreshClaude->>parseJsonResult: 解析文本结果
parseJsonResult-->>runFreshClaude: 返回 JSON 结果
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The operator asked for this directly ("-p 就没必要,依旧老逻辑,把它当成工人,
不要去限制他") and I left it out of the first commit on the grounds that it
belonged to #40 along with the tool whitelist. That grouping was wrong. The
whitelist genuinely is #40's argument - it is the --dangerously-skip-permissions
change. -p has nothing to do with #40; it was in my own change set and I dropped
it to keep the diff small, then justified it with a category it does not belong
to.
-p was carried over from when this was a one-shot extractor. It is not the
headless entry point.
MEASURED IN THIS CONFIGURATION rather than carried over. The earlier measurement
was taken on the #40-based branch, which has --dangerously-skip-permissions;
this branch keeps --permission-mode dontAsk and the read-only whitelist, so the
context differs and the old result does not transfer - the same reasoning that
made me re-run the mutation test here. The native tests spawn a real claude in
bwrap without -p, with the whitelist in place: it runs non-interactively,
emits its result event, and exits.
227 tests, 227 pass, 0 skipped.
Claude-Session: https://claude.ai/code/session_01YFwT3QhJukpnZ1AodcYbRn
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Kizunad
pushed a commit
that referenced
this pull request
Aug 10, 2026
…n they are The classifier previously reported a classification and left the reading to the reader. A failure is only legible when the tool also says what the symptom historically meant and when that reading stops holding. Each entry encodes a symptom class from the corpus that pointed somewhere other than the fault: - 400 + api_error was the forced tool_choice class (thinking block echoed into a second request the upstream rejected); the #45 --json-schema removal retires it, so a 400 now is flagged as a NEW, uncharacterized cause - 503 + cpu overload is the host CPU gate, not an upstream outage - schema_error is the model misreading the contract, deterministic, not infra - 402 is the balance class; 524 is the relay origin timeout - the collapsed "after N attempts: claude exited 1" hides five causes Also tightens the shape-change canary: events with no recognized shapes now report that the artifact shape may have changed, so a future diagnostic format degrades loudly to unclassified rather than silently misreading. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: ENGINE-43-RECONCILE
Owner
Author
|
@coderabbitai review Merging ahead of review because nineteen tasks are parked behind a review pool completing about half its runs, and each cycle walks their attempt counts toward a terminal BLOCKED. If this review finds something real it will be handled as a follow-up PR — please still post it. |
|
|
Kizunad
pushed a commit
that referenced
this pull request
Aug 10, 2026
…45) Main moved after this branch forked: #45 replaced --json-schema (a forced tool_choice some upstreams hard-400) with promptWithSchema, keeping the tool whitelist. This merge keeps this PR's argument - the sandbox is the boundary, so the whitelist is replaced by --dangerously-skip-permissions - while adopting #45's schema-in-prompt contract: - buildClaudeArgs drops -p and --json-schema; the schema rides in the prompt (promptWithSchema) and is validated by validateStage as before. - No StructuredOutput tool is registered, so nothing forces a tool call and no second request is built that could echo a thinking block back. - The toolset native test now asserts Read+Bash present, Grep/Glob gone with the whitelist, no StructuredOutput, tool_choice null/auto, and the schema present in the first prompt. Main's whitelist-permission native tests are dropped with the permission fence they asserted (matching this PR's documented stance: granting the full toolset removes the Grep/Glob tools). - Main's thinking-echo native test is kept; the four writeToolUse helpers it no longer shares are removed. 224 pass / 0 fail / 4 skip (native bwrap tests skip without CLAUDE_EXECUTABLE/RIPGREP_EXECUTABLE). Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: engine-pr40-rework
Kizunad
added a commit
to Kizunad/Bong
that referenced
this pull request
Aug 10, 2026
Kizunad/review#45(07:22:52Z 合并)去掉 --json-schema 强制 tool_choice。此前 强制工具调用使 CLI 在模型以 thinking+text 作答时构造了第二个请求,该请求携带 Console Go 无法反序列化的 thinking content block——这是今晚把 review 池压到 ~50% 的 22x passthrough 400s 与 2x tool_choice 400s 的共同根因。变异验证: 恢复 flag 使三条断言转红;227/227 测试通过。 三处 pin 同步(site 数此前被误数过 1/2/3,本次经 check-engine-pins.sh 语义锚定 枚举,恰好三处):review-next.yml L35(reviews our PRs)、review-consumer-tests.yml L38(_central-contract checkout ref)与 L80(rev-parse 断言)。canary 族 (9dcee849,provider-canary.yml + consumer-tests 内 checkout/assert)独立 pin, 本次不动。consumer contract 测试断言 workflow 形态(inputs/secrets/permissions/ pinning),不触 CLI 内部,#45 不在其 blast radius;其自带的引擎 npm test(227) 会在 Bong CI 内执行并通过。 Claude-Session: w7-pin-bump-ae90091 Co-authored-by: server_kizuna <serverkizuna@server.server> Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes both HTTP 400 families that held central review at roughly 50% completion through the night of 2026-08-10.
One cause, two exits
W16 counted the two signatures separately and they looked like two problems:
They are one cause.
--json-schemamakes Claude Code force a tool call to obtain structured output.textblocks. That is the 22x.Remove the forcing and there is no second request, so both exits close.
The failure is routing-dependent, deterministic, and invisible to every channel health check: the request never reaches a channel, so AxonHub records nothing and every channel measures healthy while reviews die. That is why it survived a night of investigation, and why it was misattributed twice before W11 dissected the artifacts.
Validation is not lost with the flag
It was never in the flag.
validateStagealready validates every stage itself and already has a schema-repair budget that feeds the exact validation error back into the next attempt. The flag bought a guarantee the engine was enforcing anyway, and charged for it in the one currency that fails closed — request shape.Deliberately minimal
-pand the read-only tool whitelist are untouched, though both are arguable. That argument is #40's. #40 isCHANGES_REQUESTEDon a credential-proxy design question and is not landing soon, so this is cut againstmainrather than stacked on it — nineteen tasks are waiting on the fix and it should not inherit an unrelated review.Verification is a mutation, not a pass
A passing test proves nothing here, so the guard was checked by breaking it. Restoring
--json-schematurns three independent assertions red, including:That runs against a real CLI with a mock upstream emitting an actual thinking content block — a plain-text mock has nothing to echo and would pass for free. The test also asserts there is exactly one request, because "no thinking echoed" would equally hold if the CLI merely stripped it, and a strip we do not control is not a guarantee.
Three mock terminations moved from a
StructuredOutputtool_use to plain text: that tool existed only because--json-schemaregistered it, and naming a tool the CLI never advertised hangs the turn until timeout — which reads as a broken sandbox rather than a broken mock.Suite
227 tests, 227 pass, 0 skipped, native suite included (
CLAUDE_EXECUTABLE+RIPGREP_EXECUTABLEset, ripgrep 14.1.1 as pinned bysetup-claude).🤖 Generated with Claude Code
https://claude.ai/code/session_01YFwT3QhJukpnZ1AodcYbRn
Summary by CodeRabbit
新功能
改进