Skip to content

fix(claude-cli): keep late diagnostics in the CLI exit buffer - #701

Open
annieyii wants to merge 1 commit into
nashsu:mainfrom
annieyii:fix/cli-diagnostic-buffer
Open

fix(claude-cli): keep late diagnostics in the CLI exit buffer#701
annieyii wants to merge 1 commit into
nashsu:mainfrom
annieyii:fix/cli-diagnostic-buffer

Conversation

@annieyii

Copy link
Copy Markdown
Contributor

Closes #700. Related: #366, #526.

captureUnparsed filled its 4096 UTF-16 code unit buffer head-first, so with
local 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 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.

Not addressed here: why any given CLI invocation fails. This only makes the
existing diagnostic readable. An assistant frame that carries an API error
as 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 the
three 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 claude CLI 2.1.226
output rather than invented, including the nested rate_limit_info.status
and the thinking_tokens estimated_tokens fields.

Verification

npm run build, npm run test:mocks (132 files / 1893 tests),
npm run mcp:test (22/22), cargo build, cargo fmt --check.

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>
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.

CLI exit diagnostics are unreadable when local hooks are active

1 participant