Skip to content

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

Closed
qingminglong wants to merge 2 commits into
openclaw:mainfrom
qingminglong:codex/97849-tool-error-turn-boundary
Closed

fix(ui): keep failed autonomous tool turns visible#98888
qingminglong wants to merge 2 commits into
openclaw:mainfrom
qingminglong:codex/97849-tool-error-turn-boundary

Conversation

@qingminglong

@qingminglong qingminglong commented Jul 2, 2026 •
edited by steipete
Loading

Copy link
Copy Markdown
Contributor

Closes #97849

What Problem This Solves

When two autonomous agent turns are adjacent in Control UI history, a successful reply in the later turn could make a failed tool from the earlier turn render as Tool output instead of Tool error.

Why This Change Was Made

Gateway chat-history projection preserves sessions_send input provenance while displaying those messages as assistant-side forwarded groups. The Control UI now treats only groups with kind: inter_session and sourceTool: sessions_send as autonomous-turn boundaries before annotating tool outcomes.

The final implementation deliberately does not use senderLabel alone: ordinary relay-labeled assistant replies still count as replies to the current turn. It also preserves the existing behavior for non-text assistant content.

User Impact

Earlier autonomous tool failures remain visibly failed when a later autonomous turn recovers, while normal labeled assistant replies continue to mark their own tool turn as successful.

Evidence

Final head: 47a0e36593ddcf3532d88f158128e0f71c1af87d

  • Red proof on Blacksmith Testbox: reversing the production change made the Chromium regression fail with ["Tool output", "Tool output"] instead of ["Tool error", "Tool output"].
  • Green Blacksmith Testbox tbx_01kwt922gcs3c4p4pvfs42r0ts (coral-barnacle): 136 focused UI tests passed, plus the Chromium end-to-end regression.
  • Exact rebased head: focused test command and pnpm check:changed both passed on the same Testbox lease.
  • Live in-app browser: the text-bearing forwarded task and later recovery both rendered; outcome labels stayed ["Tool error", "Tool output"] before and after reload.
  • Source-blind behavior contract: all four clauses passed, including history reload reconstruction.
  • Final branch autoreview: clean, with no accepted/actionable findings.

Commands:

pnpm test ui/src/pages/chat/chat-thread.test.ts ui/src/pages/chat/components/chat-message.test.ts ui/src/e2e/chat-tool-turn-outcome.e2e.test.ts
pnpm check:changed

Known gaps: no remaining behavior or validation gap identified for the touched Control UI history surface.

AI-assisted: Yes (Codex)

@clawsweeper

clawsweeper Bot commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Close: this PR's useful Control UI fix has been superseded by a clean maintainer replacement at #100514, while this fork branch is now conflicting and carries broad unrelated drift.

Root-cause cluster
Relationship: superseded
Canonical: #100514
Summary: The same Control UI regression is now owned by a focused, proof-backed replacement PR; this conflicting fork PR is no longer the viable landing path.

Members:

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

Canonical path: Close this PR and review or land #100514 as the clean candidate for the linked Control UI regression.

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

Review details

Best possible solution:

Close this PR and review or land #100514 as the clean candidate for the linked Control UI regression.

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

Yes. Current source still carries the sawAssistantReply flag backward across non-user-separated autonomous turns, and the linked issue plus replacement PR show the expected Tool error/Tool output behavior.

Is this the best way to solve the issue?

Yes for the fix shape, but not for this branch. The clean replacement keeps the narrow sessions_send provenance boundary and focused tests without the conflicting fork drift.

Security review:

Security review cleared: No concrete security or supply-chain issue was found in the intended UI fix; the live branch's broad unrelated drift is a supersession and merge-readiness problem.

AGENTS.md: found and applied where relevant.

What I checked:

  • Live PR state: Live GitHub state shows this PR is open but conflicting (mergeStateStatus=DIRTY, mergeable=CONFLICTING) with 106 changed files at head 706fafc. (706fafc6a97c)
  • Superseding replacement: fix(ui): preserve autonomous tool failures #100514 is open, mergeable, focused to the same four files, closes the same issue, and its body explicitly says it supersedes this PR while preserving contributor credit. (1bff2b0e9ff2)
  • Current source behavior: Current main still has the backward sawAssistantReply annotation that only resets at user groups, so the linked regression remains a real source-level problem until a candidate fix lands. (ui/src/pages/chat/chat-thread.ts:385, 5a31666a06e5)
  • Gateway provenance contract: Current main preserves inter_session provenance from sessions_send on projected assistant messages, which is the specific boundary used by the replacement fix. (src/gateway/chat-display-projection.ts:1416, 5a31666a06e5)
  • Replacement patch shape: The replacement PR adds assistantGroupIsForwardedBoundary, focused unit coverage, and the Chromium E2E regression for Tool error followed by Tool output. (ui/src/pages/chat/chat-thread.ts:382, 1bff2b0e9ff2)
  • Feature history: The turnSucceeded behavior being refined came from the merged prior Control UI fix for non-terminal internal tool errors. (355c43fe0c9e)

Likely related people:

  • steipete: Authored the focused superseding PR, posted the land-ready verification on this PR, and recently created the current ui/src/pages/chat files that now host the affected code. (role: recent area contributor and replacement owner; confidence: high; commits: 8c19dbfb627c, 1bff2b0e9ff2; files: ui/src/pages/chat/chat-thread.ts, ui/src/pages/chat/components/chat-message.ts, ui/src/e2e/chat-tool-turn-outcome.e2e.test.ts)
  • harjothkhara: Authored the merged PR that introduced turnSucceeded annotation and error-label suppression for non-terminal tool failures. (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 PR metadata records this user as the merger of the prior PR whose behavior is being refined here. (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)

Codex review notes: model internal, reasoning high; reviewed against 5a31666a06e5.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jul 2, 2026
@qingminglong

qingminglong commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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 github-actions Bot mentioned this pull request Jul 2, 2026
Open
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 2, 2026
@SunnyShu0925 SunnyShu0925 mentioned this pull request Jul 2, 2026
Closed
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 2, 2026
@qingminglong

qingminglong commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated the PR body with stored-transcript proof for the current head. The new public artifact shows a real session JSONL read through the Gateway transcript reader/projection path, then grouped by Control UI buildChatItems, with the earlier failed autonomous tool remaining Tool error/turnSucceeded=false and the later recovered turn remaining Tool output/turnSucceeded=true.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 3, 2026
@steipete steipete self-assigned this Jul 5, 2026
@steipete
steipete force-pushed the codex/97849-tool-error-turn-boundary branch from c615771 to 47a0e36 Compare July 5, 2026 23:32
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Land-ready at 47a0e36593ddcf3532d88f158128e0f71c1af87d.

The patch was narrowed through review to the exact Gateway contract: only preserved sessions_send inter-session provenance starts a new autonomous turn. Ordinary labeled assistant replies remain successful replies, and non-text assistant content remains unchanged.

Verification:

  • Blacksmith Testbox tbx_01kwt922gcs3c4p4pvfs42r0ts (coral-barnacle): 136 focused UI tests and the Chromium E2E regression passed.
  • Exact rebased head: pnpm check:changed passed.
  • Red proof: reverting the production hunk made the E2E fail with both labels rendered as Tool output.
  • In-app browser: ["Tool error", "Tool output"] before and after reload with a visible text-bearing forwarded task.
  • Source-blind behavior contract: pass.
  • Final autoreview: clean; no accepted/actionable findings.

No proof gaps remain for the touched surface.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation channel: discord Channel integration: discord channel: slack Channel integration: slack channel: telegram Channel integration: telegram app: android App: android app: ios App: ios gateway Gateway runtime scripts Repository scripts agents Agent runtime and tooling extensions: diffs size: XL and removed size: S labels Jul 6, 2026
@steipete steipete mentioned this pull request Jul 6, 2026
Merged
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 6, 2026
@steipete

steipete commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Landed the reviewed fix as #100514 in commit 286c020748eadabebb35a35c470ead20c71dd773.

I rebuilt the patch on current main because the fork-branch update path could not preserve clean rebased ancestry and produced an inflated unrelated diff. The landed replacement preserves the intended behavior, adds focused history coverage, and carries the original contributor credit.

Thank you @qingminglong. For future contributions, keeping Allow edits by maintainers enabled gives us the best chance of applying fixups directly, though GitHub's fork update path can still occasionally prevent a clean ancestry-preserving update.

@steipete steipete closed this Jul 6, 2026
@steipete steipete mentioned this pull request Jul 6, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

agents Agent runtime and tooling app: android App: android app: ios App: ios app: web-ui App: web-ui channel: discord Channel integration: discord channel: slack Channel integration: slack channel: telegram Channel integration: telegram docs Improvements or additions to documentation extensions: diffs gateway Gateway runtime merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. scripts Repository scripts size: XL status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Failed cron/autonomous turn shown as succeeded: cross-turn tool-error banner suppression (regression of #90122)

2 participants

@qingminglong @steipete