fix(ui): collapse non-terminal internal tool errors#90122
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 8:19 AM ET / 12:19 UTC. Summary PR surface: Source +48, Tests +149. Total +197 across 5 files. Reproducibility: yes. source-reproducible. Current main and v2026.6.10 still map failed tool cards directly to visible error UI without checking whether the same turn later produced a normal assistant reply. Review metrics: none identified. Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land this render-boundary fix, or an equivalent maintainer-approved UI projection change, while leaving broader successful tool-output visibility work to #89781. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible. Current main and v2026.6.10 still map failed tool cards directly to visible error UI without checking whether the same turn later produced a normal assistant reply. Is this the best way to solve the issue? Yes. The Control UI projection/render boundary has the needed turn-completion context and preserves terminal failures; changing Codex exec semantics or the direct-chat dispatch path would be broader or the wrong surface. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against db2488b6e3d6. Label changesLabel justifications:
Evidence reviewedPR surface: Source +48, Tests +149. Total +197 across 5 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
For maintainer context: #89975 ( |
|
Heads-up for reviewers: both red checks here appear to be pre-existing on
The diff also applies cleanly onto current |
|
@clawsweeper re-review please, with the context added since your last pass:
Could you re-evaluate the merge-readiness shape with that in mind? Happy to rebase once |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
5d3720d to
cde5df6
Compare
cde5df6 to
b92deba
Compare
|
Rebased onto current It's a contained Control-UI fix: collapse non-terminal internal tool errors so a transient internal failure during a turn that still replied doesn't surface to the user as a red error banner. Whenever a maintainer has a window I'd appreciate a look — happy to re-rebase or adjust scope, and "Allow edits by maintainers" is on. @vincentkoc |
…ied (openclaw#89683) A non-zero internal tool exit (Codex marks any non-zero exit as failed, e.g. a no-match shell search) was promoted to a primary red "Tool error" banner in Control UI even when the turn produced a clean assistant reply. Compute a per-tool-group turnSucceeded signal at the chat projection boundary and de-promote such non-terminal failures to a collapsed tool summary; detail stays available on expand. Genuinely terminal tool failures still surface. Co-Authored-By: Claude Opus 4.8 <[email protected]>
d600187 to
13f63a1
Compare
|
Merged via squash.
|
…ied (openclaw#89683) (openclaw#90122) A non-zero internal tool exit (Codex marks any non-zero exit as failed, e.g. a no-match shell search) was promoted to a primary red "Tool error" banner in Control UI even when the turn produced a clean assistant reply. Compute a per-tool-group turnSucceeded signal at the chat projection boundary and de-promote such non-terminal failures to a collapsed tool summary; detail stays available on expand. Genuinely terminal tool failures still surface. Co-authored-by: Claude Opus 4.8 <[email protected]>
…ied (openclaw#89683) (openclaw#90122) A non-zero internal tool exit (Codex marks any non-zero exit as failed, e.g. a no-match shell search) was promoted to a primary red "Tool error" banner in Control UI even when the turn produced a clean assistant reply. Compute a per-tool-group turnSucceeded signal at the chat projection boundary and de-promote such non-terminal failures to a collapsed tool summary; detail stays available on expand. Genuinely terminal tool failures still surface. Co-authored-by: Claude Opus 4.8 <[email protected]>
…ied (openclaw#89683) (openclaw#90122) A non-zero internal tool exit (Codex marks any non-zero exit as failed, e.g. a no-match shell search) was promoted to a primary red "Tool error" banner in Control UI even when the turn produced a clean assistant reply. Compute a per-tool-group turnSucceeded signal at the chat projection boundary and de-promote such non-terminal failures to a collapsed tool summary; detail stays available on expand. Genuinely terminal tool failures still surface. Co-authored-by: Claude Opus 4.8 <[email protected]>
In openclaw#90122, annotateToolTurnOutcome was added to collapse non-terminal failed tool errors when the turn produced a reply. The backward pass resets sawAssistantReply only at user groups, assuming every turn starts with a user message. Agent-initiated turns (cron/scheduled/autonomous) have no user group between them, so a later turn reply leaks backward and stamps an earlier genuinely-failed turn as turnSucceeded=true, hiding its error banner. Fix: add a terminal-turn-boundary reset for assistant groups without reply text, scope tool errors by terminal turn outcome, and attribute transcript messages to agent runs. Related to openclaw#97849 Co-authored-by: SunnyShu0925 <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>
In openclaw#90122, annotateToolTurnOutcome was added to collapse non-terminal failed tool errors when the turn produced a reply. The backward pass resets sawAssistantReply only at user groups, assuming every turn starts with a user message. Agent-initiated turns (cron/scheduled/autonomous) have no user group between them, so a later turn reply leaks backward and stamps an earlier genuinely-failed turn as turnSucceeded=true, hiding its error banner. Fix: add assistantMessageTurnSucceeded with stop-reason classification and runId-scoped outcome tracking for accurate cross-turn boundary detection. Related to openclaw#97849 Co-authored-by: SunnyShu0925 <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>
…ied (openclaw#89683) (openclaw#90122) A non-zero internal tool exit (Codex marks any non-zero exit as failed, e.g. a no-match shell search) was promoted to a primary red "Tool error" banner in Control UI even when the turn produced a clean assistant reply. Compute a per-tool-group turnSucceeded signal at the chat projection boundary and de-promote such non-terminal failures to a collapsed tool summary; detail stays available on expand. Genuinely terminal tool failures still surface. Co-authored-by: Claude Opus 4.8 <[email protected]>
Summary
Fixes #89683. In Control UI / WebChat, a failed internal tool call was promoted to a prominent red "Tool error" banner in the chat timeline even when the turn produced a clean final assistant reply, for example a hidden shell search that exits 1 for no matches (
toolResult { isError:true, status:"failed", exitCode:1 }). The banner competed with the real answer and leaked internal tool activity as a product failure.Codex marks any non-zero exec exit as failed and cannot distinguish a benign no-match from a fatal error (verified in the
@openai/codexruntime:codex-rs/core/src/tools/events.rs:341-345returnsErrforexit_code != 0, and:474-478setsExecCommandStatus::Failed). So the fix is OpenClaw/UI-side and runtime-agnostic: it normalizes the chat projection boundary so a failed internal tool that is not the turn's outcome is not surfaced as primary error content, while keeping full detail in the existing expand/activity view.What it does
buildChatItemsnow stamps each toolMessageGroupwith aturnSucceededsignal (a single backward pass: a failed tool group whose turn still produced an assistant reply with text, before the next user message, is non-terminal).grouped-rendergates the error promotion (hasError/toolMessageHasError, which drive the red--errorstyling, the "Tool error" label, the error badge, and auto-expand) onturnSucceeded !== true.showToolCalls: falsestill hides tool activity entirely.Collision check
Boundary choice
The Codex runtime correctly reports non-zero internal tool exits as failed. The UI renderer has the additional turn-completion context needed to decide whether that failed internal tool should be the primary visible outcome. This keeps terminal failures prominent while collapsing non-terminal internal failures after a clean assistant reply.
Verification
Local, Node 22, real
mainbase:ui/src/ui/chat/grouped-render.test.ts+build-chat-items.test.ts- 98 passed (9 new cases)tsgo(test/tsconfig/tsconfig.test.ui.json),oxfmt,oxlint,git diff --check- cleanReal behavior proof
renderMessageGroup) into a real DOM (jsdom) on Node v22.14.0 off the currentmain, the same lit render path the Control UI dashboard uses. The transcript mirrors the report: ashelltoolResultwith{ status:"failed", exitCode:1 }(isError:true) whose turn produced a reply, vs. the same failure with no reply.renderMessageGrouptwice, once asbuildChatItemsnow annotates it for a turn that replied (turnSucceeded:true) and once as the pre-fix projection emitted it (turnSucceededunset), then read the.chat-tool-msg-summarynode.{ "before_turnSucceeded_undefined": { "summaryClass": "chat-tool-msg-summary chat-tool-msg-summary--error", "label": "Tool error", "errorBadge": true }, "after_turnSucceeded_true": { "summaryClass": "chat-tool-msg-summary", "label": "Tool output", "errorBadge": false } }--errorclass, no error badge), so the assistant's answer stays the prominent content; the failed status/output is still shown on expand. When no reply follows, the red "Tool error" banner is unchanged.renderMessageGroupview in a real DOM instead.