Skip to content

fix(ui): guard abort UI with local live send/stream state#89895

Closed
dwc1997 wants to merge 2 commits into
openclaw:mainfrom
dwc1997:fix/87387-abort-ui-live-guard
Closed

fix(ui): guard abort UI with local live send/stream state#89895
dwc1997 wants to merge 2 commits into
openclaw:mainfrom
dwc1997:fix/87387-abort-ui-live-guard

Conversation

@dwc1997

@dwc1997 dwc1997 commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

fix(ui): guard abort UI with local live send/stream state

Summary

Problem: The "In progress" badge and Stop button were driven by showAbortableUi, which can stay true from stale session-list state (hasAbortableSessionRun) even after the response has completed. Users see a false busy/abortable state until a hard refresh.

Fix: Add isBusy guard (sending || stream !== null) so the abort UI only shows while a local send or stream is actively in progress. The new showActiveAbortUi variable gates the "In progress" badge and Stop button, while showAbortableUi continues to drive queue steer buttons and compact disable logic.

Out of scope: No changes to hasAbortableSessionRun, isSessionRunActive, or session-list state management.

Linked context

Closes #87387

Real behavior proof

  • Behavior or issue addressed: Control UI webchat shows false "In progress" / "Stop" state after response completes because canAbort stays true from stale session-list state
  • Real environment tested: Windows 10, Node.js v22.11.0, OpenClaw latest main (commit 70a989a, 2026-06-03)
  • Exact steps or command run after this patch: Added regression tests verifying Stop button visibility requires local active send/stream state
  • Evidence after fix: Terminal output showing the tests passing:
$ node scripts/run-vitest.mjs ui/src/ui/views/chat.test.ts -t "abort UI live guard"

 ✓  ui  ui/src/ui/views/chat.test.ts
   ✓ abort UI live guard (regression #87387)
     ✓ hides Stop button when canAbort is true but isBusy is false
     ✓ shows Stop button when canAbort is true and isBusy is true
     ✓ shows Stop button when sending is true

 Test Files  1 passed (1)
      Tests  3 passed (3)
  • Observed result after fix: Stop button is hidden when canAbort is true but there's no active send/stream (stale state). Stop button is shown when there's an active stream or sending is in progress.
  • What was not tested: Live Control UI browser screenshot (requires running gateway + browser)

Tests and validation

  • ui/src/ui/views/chat.test.ts: 3 new regression tests for abort UI live guard
  • No CHANGELOG.md edits

Risk checklist

  • userVisible: Yes (Stop button and badge no longer show after response completes)
  • configEnvMigration: No
  • securityAuthNetwork: No
  • Mitigation: Only adds isBusy guard; queue steer and compact logic unchanged

Current review state

  • Addressed ClawSweeper P1: Added focused regression tests for stale session-list-only abortability and active send/stream abortability
  • AI-assisted (built with Claude Code)

@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 3, 2026
@clawsweeper

clawsweeper Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 4, 2026, 11:16 AM ET / 15:16 UTC.

Summary
The PR gates the Control UI chat In progress badge and Stop button on sending || stream !== null and adds render tests for stale abort state.

PR surface: Source +4, Tests +40. Total +44 across 2 files.

Reproducibility: yes. from source inspection: current tool-stream code can leave chatRunId set while chatStream is null, and the PR guard treats that state as not active. I did not run a live browser/gateway reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

  • Visible abort gate: 1 changed. The PR changes the single gate that decides whether visible Stop/In progress controls render, which is the availability-sensitive surface under review.

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] Revise the guard so active local runs with chatRunId set and stream null still show Stop, and add regression coverage for that path.
  • [P1] Add redacted real Control UI browser/gateway proof after the code change; screenshots, recordings, terminal/live output, or logs are fine if they show the changed behavior.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body only provides Vitest output; for this external user-visible Control UI change, the contributor still needs redacted browser/gateway proof and should update the PR body to trigger re-review or ask a maintainer for @clawsweeper re-review.

Mantis proof suggestion
A short browser proof would materially help verify the visible Stop/In progress behavior across completed and active tool-run states. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify Control UI chat returns to Send/idle after a completed response and keeps Stop visible during an active streamless tool run.

Risk before merge

  • [P1] Merging the current guard can remove the Stop control while a local run is still active and abortable during tool-only/streamless output.
  • [P1] The PR has only unit-test terminal output, so the visible Control UI browser/gateway behavior after the patch is not proven in a real setup.

Maintainer options:

  1. Fix the local active-run guard before merge (recommended)
    Include the local active run signal, such as chatRunId or a dedicated prop derived from it, so Stop remains visible for active streamless tool runs while stale session-list-only rows stay idle.
  2. Require real browser proof after the code change
    Ask the contributor to update the PR body with redacted Control UI browser/gateway evidence showing the completed-response idle case and the active tool-run Stop case.
  3. Pause in favor of the canonical fix path
    If maintainers prefer the broader open fix at fix(ui): prevent false busy state in Control UI webchat after response completion #87474, pause this PR until that path is proven safe for the same streamless active-run invariant.

Next step before merge

  • [P1] Human review should hold the PR until the contributor revises the guard and supplies real Control UI proof; ClawSweeper should not queue an automated repair marker because the external proof gate still requires contributor action.

Security
Cleared: The diff only changes Control UI render logic and tests; it does not touch dependencies, workflows, secrets, auth, or package execution paths.

Review findings

  • [P1] Keep Stop visible for streamless active runs — ui/src/ui/views/chat.ts:1536-1537
Review details

Best possible solution:

Split stale session-list abortability from authoritative local active-run abortability with a signal that includes chatRunId, then verify both stale-completed and active streamless tool-run states in regression tests plus real Control UI proof.

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

Yes, from source inspection: current tool-stream code can leave chatRunId set while chatStream is null, and the PR guard treats that state as not active. I did not run a live browser/gateway reproduction in this read-only review.

Is this the best way to solve the issue?

No: sending || stream !== null is a plausible stale-completion heuristic, but it is not the best fix because local chatRunId is the existing authoritative abortability signal for streamless active runs.

Full review comments:

  • [P1] Keep Stop visible for streamless active runs — ui/src/ui/views/chat.ts:1536-1537
    The new showActiveAbortUi only trusts sending || stream !== null, but current tool-stream handling can set chatStream to null while chatRunId remains set for an active run. In that state hasAbortableSessionRun is still true and onAbort can stop the run, but this patch passes canAbort: false to the controls and removes the visible Stop affordance. Include the local active-run signal, or a dedicated prop derived from it, and cover the streamless tool-run path.
    Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 20c4e9475a7f.

Label changes

Label justifications:

  • P2: This is a normal-priority Control UI behavior fix with limited blast radius, though the current patch still needs revision.
  • merge-risk: 🚨 availability: The diff can hide the only visible Stop affordance while an active streamless tool run remains abortable.
  • 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 body only provides Vitest output; for this external user-visible Control UI change, the contributor still needs redacted browser/gateway proof and should update the PR body to trigger re-review or ask a maintainer for @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +4, Tests +40. Total +44 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 6 2 +4
Tests 1 40 0 +40
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 46 2 +44

What I checked:

  • PR diff changes the visible abort gate: The PR adds showActiveAbortUi = showAbortableUi && isBusy and passes that narrower value into renderChatRunControls, so stream === null and sending === false now hide Stop even when canAbort came from an active local run. (ui/src/ui/views/chat.ts:1536, 0f54c477605b)
  • Current render live-state source: Current main computes isBusy from props.sending || props.stream !== null, while showAbortableUi remains the broader abortability signal that includes local run/session state. (ui/src/ui/views/chat.ts:1529, 20c4e9475a7f)
  • Abortability includes local run id: hasAbortableSessionRun returns true immediately when host.chatRunId is set, before consulting possibly stale session-list rows. (ui/src/ui/app-chat.ts:212, 20c4e9475a7f)
  • Active tool runs can clear chatStream: Tool streaming moves visible text into stream segments and sets host.chatStream = null while host.chatRunId still matches the active run. (ui/src/ui/app-tool-stream.ts:789, 20c4e9475a7f)
  • Existing tests preserve streamless active-run state: An existing chat controller test constructs chatRunId: "run-1" with chatStream: null and verifies the run id remains while stream state is null after history/tool reconciliation. (ui/src/ui/controllers/chat.test.ts:2826, 20c4e9475a7f)
  • Prior history fixed the same invariant: Commit d5c42a0 added a fix and regression coverage for keeping the Stop button visible when aborting is available without an active stream, which this PR would undo for that state. (ui/src/ui/views/chat.ts, d5c42a07ef2e)

Likely related people:

  • chziyue: Authored commit d5c42a0, which specifically fixed and tested that Stop remains visible when aborting is available without an active stream. (role: introduced adjacent invariant; confidence: high; commits: d5c42a07ef2e; files: ui/src/ui/views/chat.ts, ui/src/ui/views/chat.test.ts)
  • Ayaan Zaidi: Current blame for the touched render, abortability, and tool-stream lines points to bd065c1 in the recent UI source refresh. (role: recent area contributor; confidence: medium; commits: bd065c1154ea; files: ui/src/ui/views/chat.ts, ui/src/ui/app-chat.ts, ui/src/ui/app-tool-stream.ts)
  • Peter Steinberger: Recent history shows Peter split the UI app modules and authored the live tool-stream work that makes streamless active run states relevant to this guard. (role: feature-history contributor; confidence: medium; commits: e3ff8c4d288b, de2ccffec166; files: ui/src/ui/app-chat.ts, ui/src/ui/app-tool-stream.ts, ui/src/ui/controllers/chat.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. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 3, 2026
@dwc1997

dwc1997 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 4, 2026

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.

Re-review progress:

@dwc1997
dwc1997 force-pushed the fix/87387-abort-ui-live-guard branch 3 times, most recently from 722dc8d to 00c8257 Compare June 4, 2026 14:50
dwc1997 and others added 2 commits June 4, 2026 23:07
The In progress badge and Stop button were driven by showAbortableUi,
which can stay true from stale session-list state even after the response
has completed. Add isBusy guard (sending || stream !== null) so the abort
UI only shows while a local send or stream is actively in progress.

Fixes openclaw#87387

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Add tests verifying Stop button visibility requires local active
send/stream state, not just stale session-list abortability.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@leorivastech

Copy link
Copy Markdown

Hi @dwc1997 — are you still working on this? It looks like the re-review didn't get it past ClawSweeper's real-behavior-proof gate and it's been quiet since June 4, so I wanted to ask before stepping on your work. I'd like to help #87387 land. If you're done with it, I'm glad to pick it up or open an alternative PR — just checking with you first. Thanks!

@dwc1997

dwc1997 commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

Closing: proof uses boolean simulation rather than real UI evidence. Competing PR #91606 addresses the same issue with a different approach.

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 merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Control UI webchat can leave false "In progress" / "Stop" state after completed response

2 participants