fix(ui): reset sawAssistantReply for assistant groups without reply text to prevent cross-turn tool-error banner suppression by SunnyShu0925 · Pull Request #97873 · openclaw/openclaw

SunnyShu0925 · 2026-06-29T16:34:56Z

Related to #97849.

What Problem This Solves

Failed cron/scheduled/autonomous agent turns are silently rendered as successful in the Control UI and WebChat. When two agent-initiated turns run back-to-back with no user message between them, a later turn's clean assistant reply leaks backward through annotateToolTurnOutcome and stamps an earlier genuinely-failed turn as turnSucceeded=true, collapsing its Tool error banner.

Why This Change Was Made

annotateToolTurnOutcome resets its sawAssistantReply flag only at role === "user" groups, but agent-initiated turns have no user group separating them. The fix adds a second reset point: an assistant group without reply text marks a turn boundary where the agent tried and failed to produce a visible reply.

This approach is the smallest safe change:

An assistant group with reply text still sets sawAssistantReply = true → the original fix(ui): collapse non-terminal internal tool errors #90122 within-turn suppression is unchanged.
An assistant group without reply text now explicitly resets sawAssistantReply = false → a failed turn's tool errors are no longer stamped turnSucceeded by a later turn's reply.

Alternative approaches (parsing runId/turnId metadata) would require ~50+ line diffs across the grouping layer. The display-projection heuristic used here is already part of the existing annotation contract and is simpler to audit.

Root Cause

annotateToolTurnOutcome in ui/src/ui/chat/build-chat-items.ts (introduced by 355c43fe0c / PR #90122): the backward pass only resets sawAssistantReply at user boundaries, so a later agent-initiated turn's reply propagates backward across an unmarked turn boundary and stamps an earlier genuinely-failed turn as succeeded. Confidence: clear.

User Impact

Failed autonomous turns (cron jobs, scheduled tasks) now correctly show their Tool error banner instead of silently collapsing it. The prior behavior for a failed internal tool followed by a same-turn assistant reply is preserved (regression-tested).

Evidence

Behavior addressed: A genuinely-failed agent-initiated turn was marked turnSucceeded=true when a later adjacent agent turn produced an assistant reply with no user message separating the two turns.

Real environment tested: Local OpenClaw source checkout on current upstream/main at 6de357ad47.

Exact steps or command run after this patch:

$ npx tsx proof-97849.ts

=== #97849 Real Behavior Proof ===

  tool           sender=null         turnSucceeded=false   → VISIBLE ✅
  assistant      sender=cron-turn-1  turnSucceeded=undefined → undefined
  tool           sender=null         turnSucceeded=true    → HIDDEN
  assistant      sender=cron-turn-2  turnSucceeded=undefined → undefined

  turnSucceeded = [false, true]   expected=[false, true]   PASS ✅

The proof script drives the exported buildChatItems with realistic agent-initiated turn messages — a failed toolResult (exitCode=1) followed by an empty assistant group (turn 1, no reply), then another failed tool result followed by an assistant with reply text (turn 2, succeeded). The function correctly stamps turn 1 as turnSucceeded=false and turn 2 as turnSucceeded=true.

$ cd ui && pnpm exec vitest run --config vitest.config.ts src/ui/chat/build-chat-items.test.ts
 Test Files  1 passed (1)
      Tests  48 passed (48)

Observed result after fix: The earlier failed agent-initiated turn keeps turnSucceeded=false (red error banner stays visible ✅), while the later turn's tool group gets turnSucceeded=true (collapsed because the turn produced a clean reply ✅). All 4 existing #90122 regression tests pass unchanged.

What was not tested: Live scheduled cron job with browser screenshot. The proof script and unit tests cover the buildChatItems projection function that both Control UI and WebChat consume — this is the shared entry point and the exact function modified.

Regression Test Plan

$ cd ui && pnpm exec vitest run --config vitest.config.ts src/ui/chat/build-chat-items.test.ts
 Test Files  1 passed (1)
      Tests  48 passed (48)

Changes

ui/src/ui/chat/build-chat-items.ts: +6 lines — reset sawAssistantReply for assistant groups without reply text
ui/src/ui/chat/build-chat-items.test.ts: +15 lines — add cross-turn agent-initiated scenario

clawsweeper · 2026-06-29T16:36:50Z

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

This PR should close because the same Web UI chat-turn bug is now covered by a narrower, open, proof-positive replacement PR, while this branch has grown into a broad 21-file diff that reverts unrelated current-main UI fixes.

Root-cause cluster
Relationship: superseded
Canonical: #98888
Summary: This PR and the open replacement target the same canonical chat projection regression; the replacement is the viable landing path because it is narrow, clean, and proof-positive.

Members:

canonical: fix(ui): keep failed autonomous tool turns visible #98888 - Open, mergeable replacement PR that closes the same linked issue and changes only the focused chat projection files.
fixed_by_candidate: Failed cron/autonomous turn shown as succeeded: cross-turn tool-error banner suppression (regression of #90122) #97849 - The issue describes the same adjacent autonomous-turn tool-error suppression bug and is closed by the replacement PR.
superseded: fix(ui): keep failed agent turns visibly failed [AI-assisted] #97860 - Earlier closed attempt for the same linked issue, already superseded by later replacement work.
partial_overlap: fix(ui): collapse non-terminal internal tool errors #90122 - Merged predecessor introduced the within-turn tool-error annotation behavior that this follow-up refines.
partial_overlap: [Bug]: Control UI chat surfaces failed internal tool calls as visible red error banners #89683 - Closed predecessor issue covered the within-turn failed internal tool case rather than adjacent autonomous turns.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Close this branch and let #98888 carry the linked chat projection fix, while preserving current-main cron, pairing QR, and workspace rail behavior.

So I’m closing this here and keeping the remaining discussion on #98888.

Review details

Best possible solution:

Close this branch and let #98888 carry the linked chat projection fix, while preserving current-main cron, pairing QR, and workspace rail behavior.

Do we have a high-confidence way to reproduce the issue?

Yes. The linked issue and current source show the backward annotation pass resets only at user groups, so adjacent agent-initiated turns can inherit a later assistant reply's success flag.

Is this the best way to solve the issue?

No. The shared chat projection layer is the right place to fix the bug, but this branch is no longer the best fix because the narrow open replacement covers the same behavior without unrelated UI regressions.

Security review:

Security review cleared: No dependency, workflow, secret, package, or supply-chain surface was introduced; the blocking concerns are functional/UI regressions.

AGENTS.md: found and applied where relevant.

What I checked:

Viable replacement PR: Live GitHub data shows fix(ui): keep failed autonomous tool turns visible #98888 is open, mergeable, closes the same linked issue, changes only the shared chat projection files, and includes browser/DOM proof for the adjacent autonomous-turn behavior. (868e83a03f92)
Current PR is broad and conflicting: The live patch for this PR is 21 files with 777 additions and 516 deletions, including unrelated cron, pairing QR, workspace rail, stream metadata, and controller changes beyond the stated chat outcome fix. (b0156188cec4)
Current-main on-exit cron support: Current main still models on-exit schedules as a CronSchedule variant and the controller/presenter preserve and display that read-only schedule kind; this PR removes those paths. (ui/src/ui/types.ts:590, 76ccd17ac3c2)
Current-main expired QR behavior: Current main suppresses expired openclaw_pairing_qr images, renders an expired notice, and schedules a refresh; the PR deletes that merged behavior and its tests. (ui/src/ui/chat/grouped-render.ts:337, 76ccd17ac3c2)
Expired QR provenance: Merged PR fix: hide expired pairing QR cards in Control UI #98049 added the expired pairing QR UI behavior in commit b334fbf on 2026-07-01. (ui/src/ui/chat/grouped-render.ts:337, b334fbf1298e)
Current-main workspace rail behavior: Current main keeps workspace rail sections flex-shrinking and scrollable; this PR removes both CSS properties and the E2E regression test. (ui/src/styles/chat/sidebar.css:182, 76ccd17ac3c2)

Likely related people:

harjothkhara: Authored the merged UI fix that added turnSucceeded annotation for failed internal tool rendering. (role: introduced related behavior; confidence: high; commits: 355c43fe0c9e; files: ui/src/ui/chat/build-chat-items.ts, ui/src/ui/chat/grouped-render.ts, ui/src/ui/types/chat-types.ts)
altaywtf: GitHub metadata for the merged related UI fix records this user as the squash merger of the annotation commit. (role: merger of related behavior; confidence: medium; commits: 355c43fe0c9e; files: ui/src/ui/chat/build-chat-items.ts, ui/src/ui/chat/grouped-render.ts)
ooiuuii: Authored the merged expired pairing QR UI fix that this PR would revert. (role: adjacent feature contributor; confidence: high; commits: b334fbf1298e; files: ui/src/ui/chat/grouped-render.ts, ui/src/ui/chat/grouped-render.test.ts)
wuqxuan: Authored the merged workspace rail scroll fix and E2E coverage that this PR would revert. (role: adjacent feature contributor; confidence: high; commits: 006f4e3ee8f7; files: ui/src/styles/chat/sidebar.css, ui/src/ui/e2e/chat-flow.e2e.test.ts)
joshavant: Git history shows repeated recent work in the cron controller area that includes the on-exit preservation path affected by this PR. (role: recent cron UI contributor; confidence: medium; commits: 6c231a78a456, 57d40a415a33, 393c791466fd; files: ui/src/ui/controllers/cron.ts)

Codex review notes: model internal, reasoning high; reviewed against 76ccd17ac3c2.

SunnyShu0925 · 2026-06-29T17:27:06Z

@clawsweeper re-review

clawsweeper · 2026-06-29T17:27:10Z

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

github-actions · 2026-07-01T00:24:09Z

Dependency graph guard cleared

This PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh /allow-dependencies-change comment after the guard blocks that new head SHA.

Current SHA: ff7c8c1a64b1d8cd5bfd28760e4568c9df2cbd26

SunnyShu0925 · 2026-07-02T14:15:46Z

@steipete We spent some time tonight cleaning up this branch and also reviewed the competing PR. Here's a summary:

What we did on #97873 tonight:

Rebased from the stale base (6de357ad47) onto latest upstream/main
Only 1 commit to avoid the cherry-pick conflicts that the old base caused (too many unrelated files)
Preserved both @SunnyShu0925 and your credit via Co-authored-by
CI is now clean — 0 failures, the previous dependency-graph-guard / labeler / security-sensitive-guard-detect failures are all gone
21 UI-only files changed (no dependency, workflow, docs, or iOS noise)

However: we also reviewed #98888 (qingminglong, same issue #97849). It takes the senderLabel boundary approach instead of runId + stopReason classification. ClawSweeper gave it 🐚 platinum hermit with no findings, while ours is 🦐 gold shrimp with a [P2] about missing stop-reason aliases. It also changes only 2 files vs our 21.

Our take: #98888's approach is simpler and more precise. We lean toward closing #97873 in favor of #98888, but we wanted your call as the maintainer who added the supplementary refactoring here. The grouped-render and controller changes you attached could also be submitted as an independent follow-up PR.

What do you think?

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

SunnyShu0925 · 2026-07-02T15:01:23Z

Closing in favor of #98888 which uses a simpler senderLabel boundary approach (2 files, 🐚 platinum hermit from ClawSweeper). Our rebased branch is preserved at fix/tool-error-cross-turn-97849 for reference.

Co-authored-by: SunnyShu0925 [email protected]
Co-authored-by: Peter Steinberger [email protected]

openclaw-barnacle Bot added app: web-ui App: web-ui size: XS labels Jun 29, 2026

harjothkhara mentioned this pull request Jun 29, 2026

fix(ui): keep failed agent turns visibly failed [AI-assisted] #97860

Closed

steipete self-assigned this Jun 30, 2026

openclaw-barnacle Bot added size: M and removed size: XS labels Jul 1, 2026

steipete requested a review from a team as a code owner July 1, 2026 00:22

clawsweeper Bot mentioned this pull request Jul 1, 2026

fix(ui): prevent cross-turn tool-error banner suppression (#97849) #98431

Closed

BradGroux mentioned this pull request Jul 1, 2026

WORKING: All Microsoft Issues and PRs (refresh) #74163

Draft

qingminglong mentioned this pull request Jul 2, 2026

fix(ui): keep failed autonomous tool turns visible #98888

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(ui): reset sawAssistantReply for assistant groups without reply text to prevent cross-turn tool-error banner suppression#97873

fix(ui): reset sawAssistantReply for assistant groups without reply text to prevent cross-turn tool-error banner suppression#97873
SunnyShu0925 wants to merge 1 commit into
openclaw:mainfrom
SunnyShu0925:fix/tool-error-cross-turn-97849

SunnyShu0925 commented Jun 29, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

Uh oh!

SunnyShu0925 commented Jun 29, 2026

Uh oh!

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

SunnyShu0925 commented Jul 2, 2026

Uh oh!

SunnyShu0925 commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

SunnyShu0925 commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

Root Cause

User Impact

Evidence

Regression Test Plan

Changes

Uh oh!

clawsweeper Bot commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SunnyShu0925 commented Jun 29, 2026

Uh oh!

clawsweeper Bot commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency graph guard cleared

Uh oh!

SunnyShu0925 commented Jul 2, 2026

Uh oh!

SunnyShu0925 commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SunnyShu0925 commented Jun 29, 2026 •

edited

Loading

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

github-actions Bot commented Jul 1, 2026 •

edited

Loading