fix(ui): render tool progress text from stream:item events#92764
fix(ui): render tool progress text from stream:item events#92764kangjinghang wants to merge 2 commits into
Conversation
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]>
|
Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 8:22 PM ET / 00:22 UTC. Summary PR surface: Source +60, Tests +181. Total +241 across 4 files. Reproducibility: yes. at source level: current main emits public typed tool progress on Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Proof path suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Keep the item-stream state handling, render Do we have a high-confidence way to reproduce the issue? Yes, at source level: current main emits public typed tool progress on 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:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 5a73361ed2b3. Label changesLabel justifications:
Evidence reviewedPR surface: Source +60, Tests +181. Total +241 across 4 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
|
|
Friendly ping for review — the
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 Two ways forward:
|
- 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]>
|
This pull request has been automatically marked as stale due to inactivity. |
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
handleToolExecutionUpdate→emitTrackedItemEvent→ UI), the runtime is correctly forwarding typed tool progress on thestream:"item"channel. The runtime comment insrc/agents/embedded-agent-subscribe.handlers.tools.tsnotes this path is intentional ("Typed progress already has a sanitized item update path ... avoid duplicate preview lines").The break is UI-side:
handleAgentEventinui/src/ui/app-tool-stream.tsearly-returns onstream !== "tool", so everystream:"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 astream:"item"branch (before thestream !== "tool"guard) that readsdata.progressTextforkind:"tool" + phase:"update"events and updates the matchingToolStreamEntry.progressText.buildToolStreamMessagenow emits atoolprogresscontent item (mirroring the existingtoolcall/toolresultitems). When the finalphase:"result"arrives,progressTextis cleared so a stale progress line can't render alongside the result.tool-cards.ts:extractToolCardsrecognizes thetoolprogresscontent type and attaches it to the unmatchedToolCard.progressText(reusingfindFirstUnmatchedCard, symmetric with howtoolresultmatches).renderToolCardrenders a muted line under the tool name, only whileprogressTextis present and nooutputTextexists yet.progressText?: stringtoToolCard.Fully additive — tools that never emit progress behave identically (the
toolprogresscontent item and the render line are both gated on a presentprogressText).Test Plan
app-tool-stream.node.test.ts(stream:item toolprogress events): progressText updates on valid item events; ignored when no progressText; ignored for unknowntoolCallId; 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
progressTextsets on a validstream:"item"event; ignored whenprogressTextis absent (guard); ignored for unknowntoolCallId(guard); cleared when the final result arrives (lifecycle correctness).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).src/agents/embedded-agent-subscribe.handlers.tools.ts, read-only — not modified):readChannelToolProgress(:371-383) readsprogress.text, validatesvisibility === "channel"+privacy === "public".handleToolExecutionUpdate(:1101-1102) emitsprogressText: toolProgress.textin thestream:"item"event.data.progressText— exact 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