Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 9:15 PM ET / 01:15 UTC.

Summary
The branch preserves already-streamed Claude CLI assistant text when a successful terminal JSONL result is empty, reuses the incremental parser in reusable Claude sessions, and adds focused parser/live-session regressions.

PR surface: Source +14, Tests +175. Total +189 across 4 files.

Reproducibility: yes. Source inspection shows current main discards accumulated streamed Claude text on empty successful result events, and the PR discussion includes sanitized production JSONL plus source-blind CLI proof of the same path.

Review metrics: 1 noteworthy metric.

• Message-delivery parser paths: 2 parser result paths changed, 1 live-session handoff changed. Both buffered and incremental consumers must preserve streamed text or one runtime path can still drop the reply.

Stored data model
Persistent data-model change detected: serialized state: src/agents/cli-output.test.ts, serialized state: src/agents/cli-runner.spawn.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• [P2] Wait for exact-head required checks to finish.
• [P2] Have the maintainer explicitly accept the streamed-text-over-empty-result delivery behavior before merge.

Risk before merge

• [P1] The merge intentionally changes message delivery precedence so already-streamed Claude text wins over an empty successful terminal result; maintainers should accept that parser-owned behavior before landing.
• [P1] Exact-head status was still unstable at review time because QA Smoke CI was in progress, so the PR should not merge until required checks settle.

Maintainer options:

1. Accept streamed-text delivery precedence (recommended)
   Merge after exact-head checks pass, with maintainers explicitly accepting that already-streamed Claude text should not be discarded by an empty successful result.
2. Require one more exact-head proof artifact
   Wait for or request a current-head source-blind CLI artifact if the maintainer wants fresh proof after the final force-push.
3. Pause in favor of broader empty-result work
   Do not merge this PR if maintainers want one canonical fix for all synthetic, genuinely empty, and streamed-then-empty Claude result paths.

Next step before merge

• [P2] Needs maintainer acceptance of the message-delivery precedence and exact-head required checks before merge, not an automated repair.

Maintainer decision needed

• Question: Should OpenClaw treat already-streamed Claude CLI assistant text as authoritative when the final successful result event is empty?
• Rationale: This is a user-visible message-delivery precedence choice even though the patch preserves non-empty results, structured errors, and genuine-empty failover behavior.
• Likely owner: steipete — He is assigned to the PR and owns most recent history for the affected agent parser and live-session paths.
• Options:
  • Accept parser-owned preservation (recommended): Land this PR after required checks pass, treating streamed assistant text as delivered when the terminal Claude result is empty.
  • Request stricter rollout proof: Ask for additional exact-head live or source-blind proof before merging if maintainers want more confidence in rare Claude JSONL edge cases.
  • Keep fallback authoritative: Close or replace the PR if maintainers want empty terminal results to continue overriding streamed text and triggering fallback.

Security
Cleared: The diff changes agent parser/live-session handling and tests only; it does not alter dependencies, workflows, secrets, auth, package resolution, or code-execution surfaces.

Addressing the two P2 risk notes from the review, since they're the design trade-offs maintainers need to accept:

1. Malformed empty result with a tool_use block → silent turn instead of fallback

This is a real trade-off, but silence is the safer side of it. By the time the empty result arrives, the turn's tool calls have already executed (side effects included). Falling back re-answers the turn on a model that never saw those tool calls or their results, so the fallback reply is generated without the context of actions that already happened — arguably worse than a silent turn. Also note the window is narrow: the guard only suppresses the failover when the turn verifiably emitted tool_use blocks (structured detection, not heuristics) and both the final result text and every streamed text delta were empty.

2. Requested-provider/no-fallback contract for tool-only turns

2026.6.1 already introduced allowEmptyAssistantReplyAsSilent, so the codebase already treats "empty reply = intentional silence" as a legitimate, opt-in outcome. This PR applies the same semantics automatically to the one case where the parser has structural evidence the emptiness is intentional (tool-only work), instead of requiring the operator to opt in globally. Turns with no tool calls and no streamed text keep the exact previous behavior: FailoverError → fallback chain.

If maintainers prefer a more conservative rollout, gating the hadToolCalls bypass behind the existing allowEmptyAssistantReplyAsSilent flag (or a dedicated config) would be a small change — happy to adjust.

Separately: we left an instrumented hook in our production gateway that dumps the raw JSONL stream the next time Claude emits an empty final result after streaming deltas; we'll attach a sanitized capture here when it fires.

The malformed-import blocker from the last review was a casualty of syncing this branch with main earlier today — the conflict resolution dropped the opening brace on the normalizeLowercaseStringOrEmpty import. Fixed at head 97c3825d:

import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/string-coerce";

The prior verdict was computed against 90a79d74 (the broken intermediate), so the P1 "would break the parser module" no longer applies to the current head. CI on 97c3825d is green: 130 checks passing, 0 failing (the build/check/test/lint matrix, CodeQL, and Opengrep). The only non-green item is the bot's own auto-response, pending this re-review. The parser/runner logic is the originally-submitted, proven version — only the import line and a post-merge if-block closure were touched.

On the two P2 notes (tool-only → silent vs. fallback): covered in my earlier comment — silence is the safer side, since by the time the empty result arrives the tool calls have already executed, so a fallback re-answers without that context. Still happy to gate the hadToolCalls bypass behind the existing allowEmptyAssistantReplyAsSilent flag if maintainers prefer a more conservative default.

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/27181845535
• Updated: 2026-06-09T03:35:07.563Z

Pushed ac2f807c implementing Option 1 (gate tool-only silence behind the existing policy) from the last review.

Change (src/agents/cli-runner.ts): removed the unconditional output.hadToolCalls empty-response bypass. An empty CLI turn — tool-only or not — now fails over by default and stays silent only when the caller set allowEmptyAssistantReplyAsSilent. This restores the prior fallback contract: existing fallback chains are no longer silently swallowed on a tool-only empty turn. The parser-level streamed-text preservation is unchanged, so genuinely-streamed replies still never reach this branch empty.

Tests (src/agents/cli-runner.reliability.test.ts): added focused coverage for both policy states on a verifiably tool-only empty turn (tool_use block + empty result):

• allowEmptyAssistantReplyAsSilent unset → FailoverError (empty_response).
• allowEmptyAssistantReplyAsSilent: true → SILENT_REPLY_TOKEN, no fallback.

Local verification on ac2f807c: tsc --noEmit clean, oxlint clean, and the focused suites green — cli-output.test.ts (36) and cli-runner.reliability.test.ts (39, incl. the 2 new cases). git diff --check clean.

This directly addresses the P1 at src/agents/cli-runner.ts:548 and should clear the merge-risk: message-delivery / auth-provider / availability concerns, since the default fallback behavior is preserved.

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/27182587183
• Updated: 2026-06-09T03:57:14.729Z

Rebased onto current main — this resolves the earlier check-test-types failure (a TS2322 drift in packages/sdk, now green).

The 3 remaining red shards (checks-node-core-fast, checks-node-core-tooling, checks-node-agentic-agents-core-runner) are not related to this PR — they fail identically on every currently-open PR (e.g. #95596, #95598, #95599, #95600, #95602, #95603, #95605) due to environment issues in those suites, not application code:

• checks-node-core-fast → test/scripts/resolve-openclaw-ref.test.ts: git ls-remote returns empty in the runner (expected {} to deeply equal {...})
• checks-node-core-tooling → test/scripts/bench-gateway-restart.test.ts:676 (ENOENT /tmp/...gateway-restart-intent.json) + src/scripts/test-projects.test.ts:857
• checks-node-agentic-agents-core-runner → src/agents/cli-runner.spawn.test.ts "keeps captured live prepared backend cleanup": ENOENT /tmp/mcp-captured.json (fixed /tmp path, races under parallel test execution)

This PR only touches src/agents/cli-output.ts and two cli-* test files. A re-run won't help (the same tests fail deterministically across all PRs), so this likely needs a maintainer with repo admin to either re-run after the CI infra is fixed or merge past the known-unrelated reds.

The merge-risk: 🚨 auth-provider label looks misapplied to this PR and is likely the main thing holding it in the maintainer queue.

This PR is parser-only (src/agents/cli-output.ts + tests): zero changes to token handling, credential flow, provider authentication, or model routing. cli-runner.ts is byte-identical to main. The only behavior change is in the JSONL parsers: when Claude's final result event arrives with empty text after assistant text was already streamed, preserve the accumulated streamed text instead of discarding it. If nothing was streamed, the existing empty_response → failover behavior is unchanged.

Request: drop the auth-provider label (or narrow to message-delivery, which is accurate — the change does decide whether already-streamed text is delivered vs. lost).

Impact confirmed still live on 2026.6.11: the vulnerable parser path (result with empty text overwriting accumulated deltas) ships natively — there is no native fix in the current release. I verified this today by upgrading a production gateway to 2026.6.11; the empty-response patch had to be applied to the fresh bundle (it was not a no-op), confirming the upstream code is still vulnerable. The fix is carried as an out-of-tree dist-bundle patch that re-breaks on each upstream refactor of the FailoverError line (it broke on the 6.8 and 6.9 bundles and had to be re-pinned by hand each time). Merging upstream removes that whole class of breakage.

The 3 red CI shards (checks-node-core-fast, checks-node-core-tooling, checks-node-agentic-agents-core-runner) are environmental and reproduce identically on unrelated open PRs (empty git ls-remote in the runner, ENOENT /tmp/...), not from this diff — covered in the comment above.

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Sanitized raw JSONL capture (the promised follow-up).

The instrumented debug hook left in our production gateway has now captured this intermittent empty-final-result event 218 times. Here is a representative capture, fully sanitized: all assistant/user text redacted to [REDACTED N chars], and session_id / cwd / uuid stripped — only event structure and telemetry are retained.

The smoking gun: a turn that completed successfully (subtype: "success", is_error: false, stop_reason: "end_turn") generated 11,330 output tokens over 32 turns and streamed 847 chars of assistant text across 468 content_block_delta / text_delta events — yet the final result event carried result: "". The current parser trusts that empty terminal result, discards the 847 accumulated chars, and returns empty → empty_response → fallback. A fully-streamed, fully-paid reply is thrown away.

Reproduced identically by both parsers this PR fixes (createCliJsonlStreamingParser and parseCliJsonl — the hook fires at both sites):

assistantTextLen: 847              # chars streamed & accumulated before the empty result
raw stream: 995 events →
  stream_event/content_block_delta: 468    # the streamed assistant text
  stream_event/message_start: 32, content_block_start: 52, content_block_stop: 52
  assistant: 52, user: 32, system: 241, result: 1

sample text_delta (redacted):
  {"type":"stream_event","event":{"type":"content_block_delta","index":1,
   "delta":{"type":"text_delta","text":"[REDACTED 30 chars]"}}}

final result event (telemetry only, text redacted):
  {"type":"result","subtype":"success","is_error":false,"stop_reason":"end_turn",
   "num_turns":32,"result":"","duration_ms":305032,
   "usage":{"input_tokens":34,"output_tokens":11330}}

This is exactly the case the parser fallback recovers: result.text is empty but assistantText is non-empty, so the streamed text is preserved instead of dropped. Genuinely-empty turns keep empty accumulated text and fail over exactly as before — the empty_response contract is unchanged.

This should close the remaining "raw JSONL capture not attached" item from the review.

Branch updated to current main (merge c9adc839, no conflicts — the cli-output.ts change and tests are unchanged; cli-runner.ts stays byte-identical to main).

The 3 previously-red CI shards (checks-node-core-fast, checks-node-core-tooling, checks-node-agentic-agents-core-runner) were environmental (empty git ls-remote, ENOENT /tmp) and have since been fixed upstream — they were red only because this branch's head predated those fixes. After the update GitHub now reports mergeable_state: clean, 52 checks passing, 0 failing.

Contributor-side items are all resolved now:

• ✅ merge-risk: auth-provider label dropped (parser-only diff, no auth/credential/routing changes).
• ✅ Sanitized raw JSONL capture attached (the smoking-gun empty-result turn with 847 streamed chars).
• ✅ CI green / mergeable_state: clean.
• ✅ Bug still reproduces natively on 2026.6.11.

The only remaining item is the merge-risk: message-delivery acceptance, which is a maintainer judgment call (the change does decide whether already-streamed Claude text is delivered vs. lost to fallback — that's the whole point of the fix).

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Status recap for maintainers — this is now a single, low-risk decision.

Latest ClawSweeper re-review has this at platinum hermit / diamond lobster proof, "ready for maintainer review", 0 P1, Security Cleared (parser + tests only). Two residual signals are stale and I'd flag them as noise:

1. P1 label — the bot's own "Risk before merge" section now lists only a single P2; the P1 label just wasn't auto-removed after the last re-review.
2. "Persistent data-model change detected" — this points at src/agents/cli-output.test.ts / src/agents/cli-runner.reliability.test.ts. Those are test files, not a serialized data model — there is no migration or upgrade-compat surface here. (CODEOWNERS also confirms the diff touches zero secops-owned paths — no *auth*, *secret*, or sandbox* files.)

The one real decision is the P2 (message-delivery): after this fix, a Claude turn that already streamed its full reply but ends with an empty result keeps that streamed text instead of failing over to the next model. Safety is symmetric — if the model genuinely returned nothing, the accumulated text is empty too, so FailoverError fires exactly as today. cli-runner.ts is byte-identical to main; the change is confined to the two JSONL parsers.

@shakkernerd @vincentkoc — you've both been in this exact area very recently (empty-response diagnostics and CLI message-delivery evidence), so this may be a quick call for you: is preserving already-streamed Claude text over an empty terminal result the behavior you'd want? Happy to adjust the framing if you'd prefer empty results keep driving fallback even after deltas have streamed.

@clawsweeper re-review

Rebuilt onto current main — conflict resolved, back to mergeable

The branch had gone mergeable_state: dirty since the last review: the head was unchanged, but main advanced ~415 commits past the old merge-base and drifted into conflict. Re-synced onto current main (head now 8572299ec1), and the diff stays exactly the fix — 3 files, +329/−5, cli-runner.ts byte-identical to main.

What the merge touched:

• cli-output.test.ts — the only textual conflict, an add/add: this PR's empty-result tests and main's new no-result-envelope + output-limit tests were inserted at the same anchor. Resolved by keeping all of them.
• cli-output.ts — main refactored parseCliJsonl since this PR was last green: stream-json assistant deltas now accumulate into streamJsonText (not assistantText), and error results are classified via parseClaudeCliJsonlResult. I re-ported the fix onto that new shape (2-line adjustment):
  1. the empty-result fallback now also falls back to streamJsonText.trim() (so streamed text routed through the stream-json channel is preserved, not just the plain assistantText/texts channels);
  2. the fallback no longer fires when the Claude result carries an errorText — error results return as-is, so hadToolCalls is not spuriously attached to a classified error. This keeps main's new error-classification tests intact while preserving this PR's streamed-text/tool-only contract.

Local validation on 8572299ec1: cli-output.test.ts 61/61, cli-runner.reliability.test.ts 67/67, pnpm check:test-types (tsgo core + extensions) clean.

mergeable is now true again; the only remaining item is still the maintainer message-delivery call (P2) — preserving already-streamed Claude text over an empty terminal result.

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

I re-checked this PR because new issue #100162 is now pointing at the same empty-completion failure. That issue does not need a competing fix lane: its source-repro path maps back to the two parser branches this PR owns (createCliJsonlStreamingParser and parseCliJsonl), and the current issue note already names this PR as the focused repair.

Current head 8572299ec1ef still looks like the BEST-FIX lane to me: the change is parser-owned, cli-runner.ts empty-response failover policy stays unchanged, the PR carries checked-in parser + runner policy coverage, and current GitHub checks are green including Real behavior proof.

I also reran the focused proof locally with Node 24:

node scripts/run-vitest.mjs src/agents/cli-output.test.ts src/agents/cli-runner.reliability.test.ts

Result: passed 2 Vitest shards in 60.77s (cli-output.test.ts 61/61, cli-runner.reliability.test.ts 67/67).

So #100162 looks like additional maintainer-priority evidence for this PR rather than a new implementation target. Remaining gate is still the maintainer message-delivery call: when Claude streamed assistant text but the terminal result is empty, preserve the already-streamed Claude text instead of treating the empty terminal result as canonical.

Thanks for the re-check, @harjothkhara — agreed on the framing, and the local numbers match here (61/61 + 67/67 on 8572299ec1ef).

One scoping note so the maintainer message-delivery call has the full picture, since #100162 is now in the mix:

This PR fixes the streamed-then-empty subset — assistant text (or a tool_use turn) already arrived over the stream, but the final result event lands empty. There we preserve the accumulated text instead of treating the empty terminal event as canonical. If the model genuinely returned nothing, the accumulated text is also empty and the FailoverError fires exactly as on main — no behavior change, no silent drop.

Mapping to #100162: the "reply had already fully arrived but was dropped" path is what this PR closes. The "embedded_run completes genuinely empty, nothing to recover" path is complementary — this PR intentionally leaves that failing over. Options (a)/(b) in that issue (retry / delivery-failed signal) live in the runner/supervisor layer, and this PR keeps cli-runner.ts byte-identical to main on purpose.

Happy to open a follow-up for the genuinely-empty signal if the maintainer wants it, but that's a separate, larger contract change — kept this one parser-only and low-risk.

Land-ready proof for exact head 41df24a113d38edffb0a3bb09407f357a674db6f:

• Rebased onto current main and rewrote the original +83 production-line approach to +10 net production lines. Removed unused tool-call bookkeeping and runner-policy coverage; kept the fix at the JSONL parser boundary.
• Preserves streamed assistant text in both the incremental parser and the completed-transcript parser only when the terminal result is successful and empty. Structured errors and genuinely empty turns remain unchanged.
• Sanitized AWS Crabbox cbx_e7e30cd5d814, run run_adc17841a7a1: public network, no Tailscale, no hydration, exact-SHA bootstrap. pnpm test src/agents/cli-output.test.ts passed 68/68.
• Source-blind operator proof via openclaw agent --local --json: two different streamed answers followed by empty results each returned the exact current text; a no-text empty result still exited 1 with the existing empty-response error.
• Fresh exact-head autoreview: clean, no accepted/actionable findings, confidence 0.95.
• Formatting and git diff --check: clean.

Known gap: the empty terminal envelope is intermittent against live Anthropic credentials, so exact-head proof uses a deterministic local CLI backend. The PR's earlier production Ubuntu logs provide real-credential before/after corroboration.

Maintainer closeout at exact head 4b8cb8fce4b5e25adfa4f7aa2aee49a2d484c36b:

• Rewrote the fix around parser-owned accumulated text. A successful empty terminal envelope now preserves streamed assistant text; a non-empty result or structured error still wins.
• Reusable Claude sessions consume the same incremental parser result, preserving the commentary/final-answer boundary instead of reparsing raw JSONL.
• Sanitized AWS Crabbox cbx_3dfa68399e4a: public network, no instance role, no Tailscale, no hydration.
• Focused run run_8ffd5b59ad11: 135/135 parser and reusable-session tests passed.
• Source-blind CLI run run_af6ab48c56b7: two distinct streamed answers survived empty terminal results; a genuinely empty turn retained the existing failure path.
• Exact-head hosted CI run 28761397180: all required checks passed.
• Fresh exact-head autoreview: clean, no actionable findings, confidence 0.92.
• Targeted oxfmt and git diff --check: pass.
• Contributor-credit changelog entry is carried by final batch closeout PR fix(tlon): bound urbit scry JSON response reads #100376, sequenced after this merge.

Known scope: this fixes loss of text already present in the stream. The broader genuine-empty-completion investigation remains tracked by #100162.

Merged via squash.

• Prepared head SHA: 4b8cb8fce4b5e25adfa4f7aa2aee49a2d484c36b
• Landed commit: 3c2316c43cd29c4459ddec26b863bd97c0f7b76f

Thanks for landing this, @steipete 🙏

Quick one before I open anything: the merged parser-only version covers streamed-then-empty, but leaves out the tool-only empty turn (turn emitted tool_use blocks, then empty text + empty result → still fails over and re-answers without the tool context). I've run a narrow patch for exactly this in prod since 2026-05-31, zero false empty_response fallbacks across 6.5/6.6/6.11.

Want a small follow-up gated behind the existing allowEmptyAssistantReplyAsSilent (opt-in, no default change), or is tool-only-empty → failover your intended design? Either's fine — just checking before I reopen a call you already made.