fix(agents): unify claude-cli output classification across live and one-shot paths#98942
Merged
Conversation
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.
Fixes #98896
What Problem This Solves
The claude-cli backend's two output paths (live session vs one-shot) had drifted apart, leaving four reliability defects — three of them one-shot-only because guards existed solely on the live path:
is_errorresults were delivered as successful replies. Claude CLI emits{"type":"result","subtype":"success","is_error":true,"result":"API Error: 400 ..."}(real payload, pinned by the repo's own fixture). Only the live session checkedis_error; the one-shot JSONL and JSON parsers delivered the raw API error string to the user as the assistant's answer, persisted the session binding, and skipped failover. Error-subtype envelopes with no result string also collapsed to a generic "Claude CLI failed." with the subtype dropped.parseCliOutputfell back to the entire raw stream-json transcript (up to 1 MiB one-shot / 8 MB live) as the message text; the streamed-assistant-text fallback existed only for the Gemini dialect.execute.tskept pushing chunks even after the parse cap tripped — a babbling child could allocate GBs before the overall timeout.Why This Change Was Made
Instead of four local patches, result classification and output limits now live in one shared seam in
cli-output.ts, consumed by both paths — the same funnel-parity principle applied to the Telegram send paths.is_error,type: "error",error_*subtypes, error status, and explicit error payloads all classify intoerrorText(with the subtype named when no message exists); stream-json dialects never fall back to raw transcript text (streamed assistant text is preferred, then a loud missing-result error); the live path's output-limit resolver moved into the shared module (no new config surface —reliability.outputLimitsalready existed) and now bounds the one-shot parser identically; and stdout decoding uses a streamingTextDecoderon all platforms while preserving the Windows legacy-codepage behavior exactly. The live session's duplicate guards were deleted (−65 lines there). Errors classifying tocontext_overflowcarry thecli_context_overflowretry code from all construction sites, extending #98934's fresh-session reseed recovery to one-shotis_errorresults.User Impact
Users stop receiving raw API error strings or walls of NDJSON as "answers" — those turns now fail properly and take failover/recovery. Non-English text, emoji, and accented characters no longer get corrupted mid-reply. Runaway CLI output can no longer exhaust gateway memory from the one-shot path.
Evidence
is_error/error-subtype envelopes classify to errors on both parser paths (fixture-backed); stream-json parse without a result envelope yields streamed text or a loud error, never raw transcript; one-shot line/turn caps trip at the shared limits; a multibyte character split across two chunks reassembles cleanly on POSIX.git diff origin/main -- src/agents/cli-runner.reliability.test.tsis empty — the context-overflow recovery tests landed by fix(agents): recover claude-cli context-overflow sessions and keep retry artifacts alive #98934 pass unchanged through the unified seam.node scripts/run-vitest.mjsacross cli-output, cli-runner spawn/reliability, execute supervisor-capture, child adapter, and windows-encoding tests (238 tests pass); full remotepnpm check:changedgreen on Blacksmith Testboxtbx_01kwgnq5g3b36qs6zde8fk8jkm(https://github.com/openclaw/openclaw/actions/runs/28568311707, exit 0) pre-rebase with focused revalidation post-rebase (rebase had no textual conflicts); repo autoreview (branch mode vs origin/main) clean.