Skip to content

fix(agents): unify claude-cli output classification across live and one-shot paths#98942

Merged
obviyus merged 2 commits into
mainfrom
codex/fix-cli-output-classification
Jul 2, 2026
Merged

fix(agents): unify claude-cli output classification across live and one-shot paths#98942
obviyus merged 2 commits into
mainfrom
codex/fix-cli-output-classification

Conversation

@obviyus

@obviyus obviyus commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

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:

  1. is_error results 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 checked is_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.
  2. Raw NDJSON transcripts could be delivered as the reply. When no result envelope was recognized, parseCliOutput fell 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.
  3. The one-shot streaming parser buffered lines without bound. The live path capped pending lines at 8 MB and aborted; the one-shot parser grew its line buffer indefinitely and execute.ts kept pushing chunks even after the parse cap tripped — a babbling child could allocate GBs before the overall timeout.
  4. Multibyte UTF-8 output was corrupted at chunk boundaries on macOS/Linux. Each stdout chunk was decoded independently (the incremental decoder was Windows-only), so a CJK/emoji character split across a 64 KB pipe read became U+FFFD on both halves — inside valid JSON, flowing silently into delivered replies.

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 into errorText (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.outputLimits already existed) and now bounds the one-shot parser identically; and stdout decoding uses a streaming TextDecoder on all platforms while preserving the Windows legacy-codepage behavior exactly. The live session's duplicate guards were deleted (−65 lines there). Errors classifying to context_overflow carry the cli_context_overflow retry code from all construction sites, extending #98934's fresh-session reseed recovery to one-shot is_error results.

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

  • New tests: 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.
  • Extraction proof: git diff origin/main -- src/agents/cli-runner.reliability.test.ts is 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.
  • Validation on the rebased branch: node scripts/run-vitest.mjs across cli-output, cli-runner spawn/reliability, execute supervisor-capture, child adapter, and windows-encoding tests (238 tests pass); full remote pnpm check:changed green on Blacksmith Testbox tbx_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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling maintainer Maintainer-authored PR size: L

Projects

None yet

1 participant