fix(ui): guard abort UI with local live send/stream state#89895
fix(ui): guard abort UI with local live send/stream state#89895dwc1997 wants to merge 2 commits into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 4, 2026, 11:16 AM ET / 15:16 UTC. Summary PR surface: Source +4, Tests +40. Total +44 across 2 files. Reproducibility: yes. from source inspection: current tool-stream code can leave Review metrics: 1 noteworthy metric.
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:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Split stale session-list abortability from authoritative local active-run abortability with a signal that includes Do we have a high-confidence way to reproduce the issue? Yes, from source inspection: current tool-stream code can leave Is this the best way to solve the issue? No: Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 20c4e9475a7f. Label changesLabel justifications:
Evidence reviewedPR surface: Source +4, Tests +40. Total +44 across 2 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
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
722dc8d to
00c8257
Compare
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]>
00c8257 to
0f54c47
Compare
|
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! |
|
Closing: proof uses boolean simulation rather than real UI evidence. Competing PR #91606 addresses the same issue with a different approach. |
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 staytruefrom stale session-list state (hasAbortableSessionRun) even after the response has completed. Users see a false busy/abortable state until a hard refresh.Fix: Add
isBusyguard (sending || stream !== null) so the abort UI only shows while a local send or stream is actively in progress. The newshowActiveAbortUivariable gates the "In progress" badge and Stop button, whileshowAbortableUicontinues 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
canAbortstays true from stale session-list statecanAbortis 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.Tests and validation
ui/src/ui/views/chat.test.ts: 3 new regression tests for abort UI live guardRisk checklist
Current review state