fix(claude-cli): keep late diagnostics in the CLI exit buffer - #701
Open
annieyii wants to merge 1 commit into
Open
fix(claude-cli): keep late diagnostics in the CLI exit buffer#701annieyii wants to merge 1 commit into
annieyii wants to merge 1 commit into
Conversation
When the CLI exits non-zero with an empty stderr, the transport pastes its captured stdout into the error so the user has something to act on. That buffer was filled head-first and stopped accepting once it passed 4096 UTF-16 code units, so the lines that arrived first won it. With local hooks active the lines that arrive first are hook events, and a SessionStart hook that echoes a skill file runs to several KB. The budget was spent during startup, and whatever the CLI reported later -- including the line naming the failure -- was refused. nashsu#366 is a report of exactly that output. Three changes: - isDiagnosticLine() classifies the stdout frames the token parser did not consume. An explicit error signal (is_error, is_api_error_message, outcome "error", status "failed", a populated error or stderr field, a subtype naming itself) keeps a frame whatever its type. Failing that, assistant, user, non-error stream_event and successful result frames are dropped by type, as is an allowed rate_limit_event -- a top-level type whose status is nested under rate_limit_info, and whose overageStatus reads "rejected" on a perfectly healthy account. Under `system` only init, hook_started, hook_progress, thinking_tokens and a hook_response reporting outcome "success" count as routine; every other subtype is kept, including ones added upstream after this was written. That default matters for `system` specifically: status, informational, model_refusal_no_fallback and files_persisted can each carry failure information in fields this code has no reason to know about, so recognizing errors by enumeration would eat them. - The buffer evicts from the front instead of refusing to append, so a diagnostic that arrives late still fits. - A single line over budget is truncated to its head AND tail rather than its head alone. Hook JSON writes stdout first and its verdict (stderr, exit_code, outcome) last, so head-only truncation drops exactly the fields that say it failed. The marker counts against the budget, and cuts step off surrogate pairs rather than through them. nashsu#366 was fixed by running provider tests under an isolated config, which kept local hooks out of that one path. v0.6.11 removed that wrapper in 70e5e57, so provider tests now use the config as given and localCliIsolation still defaults to false. No path forces isolation today, which puts callers on that default back in range of the output nashsu#366 reported. This fixes the diagnostic itself, so it holds however that setting moves. Out of scope, both left as they are: an assistant frame that carries an API error as text is consumed by the token parser and never reaches this classifier, and nashsu#526's login failure is unrelated -- it is referenced only as evidence that a result frame can report is_error true while its subtype still reads "success". Co-Authored-By: Claude Opus 5 (1M context) <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 #700. Related: #366, #526.
captureUnparsedfilled its 4096 UTF-16 code unit buffer head-first, so withlocal hooks active the budget was spent during session startup and whatever
the CLI reported later — including the line naming the failure — was refused.
Three changes to
src/lib/claude-cli-transport.ts:isDiagnosticLine()classifies the stdout frames the token parser didnot consume. An explicit error signal (
is_error,is_api_error_message,outcome: "error",status: "failed", a populatederrororstderrfield, a subtype naming itself) keeps a frame whatever its type. Failing
that,
assistant,user, non-errorstream_eventand successfulresultframes are dropped by type, as is an allowed
rate_limit_event— atop-level type whose status is nested under
rate_limit_info, and whoseoverageStatusreads"rejected"on a perfectly healthy account. Undersystemonlyinit,hook_started,hook_progress,thinking_tokensand a
hook_responsereportingoutcome: "success"count as routine;every other subtype is kept, including ones added upstream after this was
written. That default matters for
systemspecifically:status,informational,model_refusal_no_fallbackandfiles_persistedcan eachcarry failure information in fields this code has no reason to know about,
so recognizing errors by enumeration would eat them.
The buffer evicts from the front instead of refusing to append, so a
diagnostic that arrives late still fits.
A single line over budget is truncated to its head AND tail rather than
its head alone. Hook JSON writes stdout first and its verdict (
stderr,exit_code,outcome) last, so head-only truncation drops exactly thefields that say it failed. The marker counts against the budget, and cuts
step off surrogate pairs rather than through them.
Not addressed here: why any given CLI invocation fails. This only makes the
existing diagnostic readable. An
assistantframe that carries an API erroras text is consumed by the token parser and never reaches this classifier,
which is a separate problem. #526's login failure is unchanged.
Tests
src/lib/__tests__/claude-cli-transport.test.ts, 40 passing. Each of thethree changes was mutation-tested: removing the eviction loop, the head+tail
truncation, or the classifier's default-keep behaviour each turns a dedicated
test red.
Event shapes used as fixtures were taken from live
claudeCLI 2.1.226output rather than invented, including the nested
rate_limit_info.statusand the
thinking_tokensestimated_tokensfields.Verification
npm run build,npm run test:mocks(132 files / 1893 tests),npm run mcp:test(22/22),cargo build,cargo fmt --check.