Skip to content

fix(ui): render tool progress text from stream:item events#92764

Open
kangjinghang wants to merge 2 commits into
openclaw:mainfrom
kangjinghang:fix/control-ui-tool-progress
Open

fix(ui): render tool progress text from stream:item events#92764
kangjinghang wants to merge 2 commits into
openclaw:mainfrom
kangjinghang:fix/control-ui-tool-progress

Conversation

@kangjinghang

@kangjinghang kangjinghang commented Jun 13, 2026

Copy link
Copy Markdown

Problem

During long-running tool executions (e.g. a 5-minute analysis tool), webchat shows no interim progress — only the final result. Users see the tool card sit idle until completion.

Tracing the full pipeline (SDK contract → runtime event wiring → runtime handleToolExecutionUpdateemitTrackedItemEvent → UI), the runtime is correctly forwarding typed tool progress on the stream:"item" channel. The runtime comment in src/agents/embedded-agent-subscribe.handlers.tools.ts notes this path is intentional ("Typed progress already has a sanitized item update path ... avoid duplicate preview lines").

The break is UI-side: handleAgentEvent in ui/src/ui/app-tool-stream.ts early-returns on stream !== "tool", so every stream:"item" event — including tool progress — was discarded before reaching the tool-stream state.

Fix

UI-only change (no runtime/SDK edits). Consumes the existing stream:"item" progress events:

  • app-tool-stream.ts: add a stream:"item" branch (before the stream !== "tool" guard) that reads data.progressText for kind:"tool" + phase:"update" events and updates the matching ToolStreamEntry.progressText. buildToolStreamMessage now emits a toolprogress content item (mirroring the existing toolcall / toolresult items). When the final phase:"result" arrives, progressText is cleared so a stale progress line can't render alongside the result.
  • tool-cards.ts: extractToolCards recognizes the toolprogress content type and attaches it to the unmatched ToolCard.progressText (reusing findFirstUnmatchedCard, symmetric with how toolresult matches). renderToolCard renders a muted line under the tool name, only while progressText is present and no outputText exists yet.
  • types/chat-types.ts: add progressText?: string to ToolCard.

Fully additive — tools that never emit progress behave identically (the toolprogress content item and the render line are both gated on a present progressText).

Test Plan

  • New unit tests in app-tool-stream.node.test.ts (stream:item toolprogress events): progressText updates on valid item events; ignored when no progressText; ignored for unknown toolCallId; cleared when the final result arrives.
  • pnpm --filter openclaw-control-ui exec vitest run src/ui/chat/tool-cards.test.ts src/ui/chat/tool-cards.node.test.ts src/ui/chat/grouped-render.test.ts src/ui/app-tool-stream.node.test.ts — 122 passed.
  • pnpm --filter openclaw-control-ui build — succeeds (tsc + Vite).

Real behavior proof

Requesting a maintainer proof: override. No live screenshot/recording, but the verification below is rigorous and the change is fully additive.

Verified

  1. Unit tests (27 in scope, 4 new) — all four branches of the new code path covered: progressText sets on a valid stream:"item" event; ignored when progressText is absent (guard); ignored for unknown toolCallId (guard); cleared when the final result arrives (lifecycle correctness).
  2. No regressions — 122 tests pass across tool-cards.test.ts, tool-cards.node.test.ts, grouped-render.test.ts, app-tool-stream.node.test.ts. Build green (tsc strict + Vite production bundle).
  3. End-to-end field mapping verified against runtime source (src/agents/embedded-agent-subscribe.handlers.tools.ts, read-only — not modified):
    • readChannelToolProgress (:371-383) reads progress.text, validates visibility === "channel" + privacy === "public".
    • handleToolExecutionUpdate (:1101-1102) emits progressText: toolProgress.text in the stream:"item" event.
    • This PR's UI handler reads data.progressTextexact field match. ✅

Why no screenshot

I consume OpenClaw via the published npm package in my daily workflow and don't have a source-tree dev environment set up to run the patched UI against a live tool execution. The change is purely additive (+176/-0 across 4 files), gated on a present progressText — tools that don't emit progress are byte-for-byte identical to current behavior. The runtime path feeding this UI handler is already shipped and battle-tested.

If a maintainer agrees the verification above is sufficient, please apply the proof: override so the "Real behavior proof" check passes. Happy to capture a live screenshot if someone can point me at the simplest dev-mode invocation.

🤖 Generated with Claude Code

The runtime forwards AgentToolProgress on the "item" stream, but the
control UI's handleAgentEvent early-returned on stream !== "tool", so
webchat never showed interim tool progress during long executions.

Add a stream:"item" branch that updates ToolStreamEntry.progressText
and emits a "toolprogress" content item (mirroring toolcall/toolresult).
tool-cards extracts it onto ToolCard.progressText and renders a muted
line under the tool name while the tool is still running. The progress
line clears once the final result arrives.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 13, 2026
@clawsweeper

clawsweeper Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 8:22 PM ET / 00:22 UTC.

Summary
The PR adds Control UI handling for tool progress carried on stream:"item" events, maps it into tool cards, renders progress while the tool is running, and adds focused node tests.

PR surface: Source +60, Tests +181. Total +241 across 4 files.

Reproducibility: yes. at source level: current main emits public typed tool progress on stream:"item", while the current WebChat handler returns before non-tool streams can update tool cards. I did not establish a live browser reproduction in this read-only review.

Review metrics: none identified.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Fix the check-test-types failure in the added test cleanup.
  • [P1] Render progressText in the shared standalone tool-shell path and add grouped-render coverage for toolprogress.
  • [P1] Add redacted real WebChat proof showing progress during a running tool and clearing after the final result.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR provides unit tests, build claims, and source mapping only; it still needs redacted after-fix WebChat proof showing progress during a running tool and clearing after the result. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Proof path suggestion
A short WebChat visual proof would directly show the user-visible progress line and final cleanup path that unit tests do not prove. Mantis is currently scoped to Telegram, Discord, and web UI chat proof, so it is not the right proof path for this surface.
Use maintainer screenshot/manual proof, browser or Playwright proof, Crabbox where appropriate, or normal local artifact proof instead.

Risk before merge

  • [P1] The visible progress line is not rendered in the shared standalone tool-shell path, so normal WebChat tool cards can still appear idle after merge.
  • [P1] The PR head has a concrete check-test-types failure in the added test cleanup.
  • [P1] The external contributor has not supplied after-fix WebChat proof; tests, build claims, and source mapping do not prove the real browser behavior.
  • [P1] fix(webchat): surface Codex commentary progress #92236 changes the same item-stream gate for sibling commentary progress, so whichever branch lands second needs to preserve both kind:"preamble" and kind:"tool" handling.

Maintainer options:

  1. Decide the mitigation before merge
    Keep the item-stream state handling, render progressText in the shared expanded tool-card/tool-shell content path, fix the test type-check failure, add grouped-render coverage, and require redacted real WebChat proof or an explicit maintainer proof override before merge.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Human review remains the right lane because the PR has a contributor proof gate, a failing type-check job, and conflict-sensitive overlap with a sibling item-stream WebChat PR.

Security
Cleared: No concrete security or supply-chain concern found; the diff is limited to Control UI TypeScript/tests and renders already-public progress text through Lit interpolation.

Review findings

  • [P1] Render progress in the tool-shell path — ui/src/ui/chat/tool-cards.ts:686-688
  • [P1] Make the window cleanup type-check — ui/src/ui/app-tool-stream.node.test.ts:853
Review details

Best possible solution:

Keep the item-stream state handling, render progressText in the shared expanded tool-card/tool-shell content path, fix the test type-check failure, add grouped-render coverage, and require redacted real WebChat proof or an explicit maintainer proof override before merge.

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

Yes, at source level: current main emits public typed tool progress on stream:"item", while the current WebChat handler returns before non-tool streams can update tool cards. I did not establish a live browser reproduction in this read-only review.

Is this the best way to solve the issue?

No. The event-state fix is in the right area, but the current patch is not the best complete fix until it covers the shared standalone tool-shell render path, fixes the type-check failure, and proves the real WebChat behavior.

Full review comments:

  • [P1] Render progress in the tool-shell path — ui/src/ui/chat/tool-cards.ts:686-688
    The progress line is added only in renderToolCard, but grouped rendering sends a single standalone tool message through renderExpandedToolCardContent. Normal live tool-shell cards can therefore still sit idle; render progressText in the shared expanded content path and cover it with a grouped-render test.
    Confidence: 0.94
  • [P1] Make the window cleanup type-check — ui/src/ui/app-tool-stream.node.test.ts:853
    check-test-types still fails on this added delete globalWithWindow.window line because the intersected typeof globalThis type keeps window non-optional for the delete operand. Restore the previous value or delete through an optional-only record type so the test typecheck lane can pass.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a normal-priority WebChat progress rendering bug with limited blast radius and no data-loss, security, or outage signal.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR provides unit tests, build claims, and source mapping only; it still needs redacted after-fix WebChat proof showing progress during a running tool and clearing after the result. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +60, Tests +181. Total +241 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 3 60 0 +60
Tests 1 182 1 +181
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 242 1 +241

What I checked:

  • Current main drops item-stream tool progress: On current main, handleAgentEvent returns for any stream other than tool after lifecycle/fallback handling, so stream:"item" tool progress cannot update tool-card state. (ui/src/ui/app-tool-stream.ts:761, 5a73361ed2b3)
  • Runtime emits typed tool progress as item events: The agent runtime accepts only explicit channel/public progress text and emits it on item updates as kind:"tool", phase:"update", and progressText. (src/agents/embedded-agent-subscribe.handlers.tools.ts:447, 5a73361ed2b3)
  • Typed progress is an existing agent-core contract: AgentToolProgress is documented as public channel-safe progress text, so the UI should consume an existing contract rather than inventing a new runtime path. (packages/agent-core/src/types.ts:428, 5a73361ed2b3)
  • PR head updates state from item-stream tool progress: The PR head adds an item-stream branch that reads kind:"tool", phase:"update", toolCallId, and progressText, then updates the matching tool stream entry. (ui/src/ui/app-tool-stream.ts:774, 60a5f1f1bc85)
  • PR head renders progress only in the manual card path: The new visible progress line is added to renderToolCard, while the shared expanded tool-card content helper still has no progress rendering. (ui/src/ui/chat/tool-cards.ts:686, 60a5f1f1bc85)
  • Standalone tool messages use the shared tool-shell path: Grouped rendering routes a single standalone tool message through renderExpandedToolCardContent, so progress rendered only in renderToolCard will miss the normal live tool-shell case. (ui/src/ui/chat/grouped-render.ts:1872, 5a73361ed2b3)

Likely related people:

  • steipete: Authored the typed tool-progress contract/emission commit and earlier Control UI live tool-stream work that this PR extends. (role: typed progress feature owner and earlier live tool-stream contributor; confidence: high; commits: aff6d079d35b, de2ccffec166, 12a56d4d46dd; files: src/agents/embedded-agent-subscribe.handlers.tools.ts, packages/agent-core/src/types.ts, ui/src/ui/app-tool-stream.ts)
  • vincentkoc: Recent commits touched grouped rendering, tool-card rendering, and Control UI chat performance around the affected render path. (role: recent Control UI render-path contributor; confidence: medium; commits: b9b057b6d58f, f49f5973b0c1, 402e2bb81a73; files: ui/src/ui/chat/grouped-render.ts, ui/src/ui/chat/tool-cards.ts)
  • osolmaz: Authored the recent stream/history reconciliation commit adjacent to this transient stream and tool-message behavior. (role: recent WebChat stream reconciliation contributor; confidence: medium; commits: 17a285f29825; files: ui/src/ui/app-tool-stream.ts, ui/src/ui/controllers/chat.ts, ui/src/ui/chat/stream-reconciliation.ts)
  • chinar-amrutkar: Authored the recent tool-result pairing change in tool-cards.ts, including matching behavior that this PR extends for progress items. (role: recent tool-card pairing contributor; confidence: medium; commits: c2c20a0b0d45; files: ui/src/ui/chat/tool-cards.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@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. labels Jun 13, 2026
@kangjinghang

Copy link
Copy Markdown
Author

Friendly ping for review — the ## Real behavior proof section at the bottom lays out the verification done:

  • 27 unit tests in scope (4 new) covering all four code branches
  • 122 tests pass, no regressions
  • tsc strict + Vite build green
  • End-to-end field mapping checked against src/agents/embedded-agent-subscribe.handlers.tools.ts (read-only)

Couldn't produce a live screenshot — I consume OpenClaw via the published npm package and don't have a source-tree dev environment set up. The change is purely additive (+176/-0 across 4 files), gated on a present progressText, so tools that don't emit progress are byte-for-byte identical to current behavior.

Two ways forward:

  1. If the verification above is sufficient, please apply the proof: override so the "Real behavior proof" check passes.
  2. Otherwise, point me at the simplest dev-mode invocation and I'll capture a screenshot.

- Add afterAll to clean up the window installed by beforeAll (only when
  we installed it, so we don't disturb sibling describe blocks that may
  rely on a pre-existing window).
- Add two guard tests for the stream:"item" branch in handleAgentEvent:
  events whose kind is not "tool" or whose phase is not "update" are
  ignored, leaving the entry's progressText untouched.

In-scope suite: 124 pass (was 122). No regressions.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@openclaw-barnacle

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added stale Marked as stale due to inactivity triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. and removed stale Marked as stale due to inactivity labels Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: web-ui App: web-ui P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant