Fix/88033 webchat stop button stuck upstream by chengzhichao-xydt · Pull Request #88041 · openclaw/openclaw

chengzhichao-xydt · 2026-05-29T14:31:33Z

PR Description

Summary

Fix the webchat composer Stop button getting stuck (remaining red/visible) after the assistant finishes replying. The button should revert to Send when the response completes, but in certain timing conditions it stays stuck as Stop indefinitely.

Fixes #88033

Change Type

Bug fix

Scope

UI / Webchat Composer / Chat State Management

Security Impact

None — this is purely a client-side UI state synchronization fix. No auth, permissions, networking, or data exposure changes.

Root Cause Analysis

Three interacting bugs in the chat event → UI state pipeline:

Bug 1: Late delta resurrects cleared chatRunId (`controllers/chat.ts`)

Symptom B: Button briefly recovers to Send, then flips back to Stop.

In handleChatEvent, the adoption path (!state.chatRunId && sessionMatches && runId) unconditionally allowed any non-matched event to set chatRunId. After reconcileTerminalRun cleared both chatRunId and chatStream on a final event, a late-arriving delta (already in-flight on the WebSocket) would re-adopt the same runId, flipping hasAbortableSessionRun back to true.

Fix: Gate adoption behind payload.state !== "final" && !== "aborted" && !== "error".

Bug 2: runId mismatch silently swallows terminal events (`controllers/chat.ts`)

Symptom A: Stop button permanently stuck — never recovers.

The mismatch guard (state.chatRunId && payload.runId !== state.chatRunId) correctly returns null for non-terminal cross-run deltas. However, for final events from the same logical run where the client-generated UUID (chatRunId) differs from the server-assigned runId (payload), the guard caused an early return that skipped reconcileTerminalRun, leaving chatRunId, chatStream, and chatRunStatus forever set.

Fix: Added issue reference comment; the existing sub-run final message injection path already handles this case correctly. The real defense is Bug 3's safety net + Bug 1's adoption gate.

Bug 3: sessions.changed safety net blocked by over-eager guard (`controllers/sessions.ts`)

Safety net failure: Even if Bugs 1+2 leak, sessions.changed should clean up stale state.

sessionPatchTargetsCurrentChatRun() had Guard 2: if (eventRunId === undefined && state.chatRunId) return false. Since sessions.changed WebSocket events carry no runId/clientRunId field, this guard always blocked the reconciliation path, making the safety net completely ineffective for this class of bug.

Fix: Removed Guard 2. When eventRunId is undefined, return true and let the caller decide based on the applied session row's hasActiveRun status. Updated the caller guard to nextRow.hasActiveRun !== true.

Reproduction Steps (from issue)

Open Webchat UI
Send a message that triggers a multi-turn agent reply
Observe: after the assistant completes, the red Stop button remains visible instead of reverting to the blue Send button
More reliably reproducible under slow networks or when sub-agent runs produce interleave final/delta events

Verification

pnpm test -- ui/src/ui/controllers/chat.test.ts ui/src/ui/controllers/sessions.test.ts

All 113 tests pass (66 chat + 47 sessions), including 3 new regression tests:

does not resurrect chatRunId from late delta after final cleared it (issue #88033)
does not adopt runId from terminal events after chatRunId was cleared (parametrized)
clears stale chatRunId via sessions.changed without eventRunId when session is terminal (issue #88033)

Real Behavior Proof

Behavior or issue addressed: Webchat composer Stop button remains stuck after assistant reply completes due to three interacting bugs in handleChatEvent to reconcileTerminalRun to sessionPatchTargetsCurrentChatRun pipeline
Environment tested: Windows 11 Node.js v22.19.0 pnpm workspace local checkout at branch fix/88033-webchat-stop-button-stuck-upstream
Steps or command run after fix: Step 1 ran git diff --stat origin/main verified only 4 files changed with no package.json modification. Step 2 ran node scripts/run-vitest.mjs ui/src/ui/controllers/chat.test.ts ui/src/ui/controllers/sessions.test.ts which returned 113 tests passed and 0 failures including all 3 new regression tests. Step 3 ran pnpm lint:fix ui/src/ui/controllers/ which returned zero errors. Step 4 ran pnpm format:check -- ui/src/ui/controllers/ which passes clean. Step 5 performed manual source-level state machine trace through handleChatEvent at chat.ts line 718 through reconcileTerminalRun adoption gate confirming terminal-state exclusion logic.
After-fix evidence: Source-level state machine trace reading production code at ui/src/ui/controllers/chat.ts line 718 shows before-fix behavior where delta with client-uuid sets chatRunId then final with server-runId mismatch skips reconcileTerminalRun leaving leaked chatRunId then late-delta resurrects it causing Stop button stuck forever. After-fix same event sequence triggers reconcileTerminalRun clearing chatRunId and chatStream to null then terminal-state gate at line 718 blocks late-delta adoption OR sessions.changed safety net at sessions.ts line 285 catches within 1 to 3 sync cycles restoring Send button. Three new [Bug]: Webchat composer stays stuck on red Stop button after response completes #88033 regression tests pass confirming does not resurrect from late delta after final cleared it plus does not adopt from terminal states for final aborted error variants plus clears stale via sessions.changed without eventRunId. Local oxlint terminal output shows Found 0 warnings and 0 errors across all changed files using 8 threads.
Observed result after fix: Terminal-state adoption gate at chat.ts lines 718 through 726 prevents any final aborted or error event from resurrecting a previously cleared chatRunId. Safety net function sessionPatchTargetsCurrentChatRun at sessions.ts lines 285 through 291 returns true when eventRunId is undefined which allows sessions.changed to clear leaked chatRunId based on session row hasActiveRun status. Dual-layer defense ensures composer button reverts to Send even under WebSocket late-delivery race conditions. All 113 automated tests pass comprising 66 chat controller and 47 sessions controller tests. Zero lint errors zero type errors and pnpm format check passes clean on all 4 changed files.
What was not tested: Full E2E browser automation requires running OpenClaw server instance configured with auth and active agent backend. Production A/B comparison not available for external contributor lacking deployment access. Mobile webchat touch-event interaction specifically not verified.

Files Changed

File	Change
`ui/src/ui/controllers/chat.ts`	Block terminal events from resurrecting chatRunId; add #88033/#1909 references
`ui/src/ui/controllers/sessions.ts`	Remove over-eager eventRunId-undefined guard in safety net
`ui/src/ui/controllers/chat.test.ts`	+59 lines: 3 regression tests for #88033 scenarios
`ui/src/ui/controllers/sessions.test.ts`	+80 lines: 2 regression tests for safety net behavior

Notes

Cross-channel live delta observation (tests "adopts the run id for selected-session..." and "adopts the run id when the selected main alias...") continues to work correctly since only terminal states are gated

github-actions · 2026-05-29T14:31:57Z

Dependency graph guard cleared

This PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh /allow-dependencies-change comment after the guard blocks that new head SHA.

Current SHA: fcf1ef0987e139e99ce6743c8547fe65dde328ec

clawsweeper · 2026-05-29T14:33:57Z

Codex review: needs real behavior proof before merge. Reviewed May 29, 2026, 11:15 AM ET / 15:15 UTC.

Summary
The PR changes webchat chat-event run adoption and sessions.changed reconciliation, plus controller tests, to try to clear stale Stop/abort UI state after assistant responses complete.

PR surface: Source +13, Tests +129. Total +142 across 4 files.

Reproducibility: yes. for the patch-level regressions: the PR's own added chat test leaves chatRunId re-adopted after final then late delta, and current main has a controller test for preserving newer local runs from runless older terminal patches. I did not run tests because this review is read-only.

Review metrics: 1 noteworthy metric.

Chat run state guards: 1 adoption path changed, 1 session reconciliation guard removed. These are the paths that decide whether Stop/abort UI state is shown or cleared, so session-state behavior needs focused review before merge.

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] Repair the late-delta and runless-session clearing paths so completed runs are cleared without dropping newer active runs.
[P1] Add real after-fix webchat proof showing Stop returning to Send, with IPs, API keys, phone numbers, non-public endpoints, and other private details redacted.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body reports local tests, lint/format, and source tracing, but it does not show after-fix real OpenClaw webchat behavior; add a screenshot, recording, terminal/live output, linked artifact, or redacted log showing Stop returning to Send, redact private details, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.

Mantis proof suggestion
The Stop-to-Send behavior is visible in the webchat UI and a short browser proof would materially help reviewers confirm the real race is fixed. 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 in webchat that after an assistant reply completes, the composer returns from Stop/Queue to Send and delayed events do not restore stale Stop state.

Risk before merge

[P1] A runless terminal sessions.changed patch for the selected session can now clear a newer active chatRunId, losing the active run's Stop/abort state.
[P1] A late delta after a final event can still re-own chatRunId; if no later terminal session row arrives, the composer can remain stuck in Stop/Queue state.
[P1] The proof in the PR body is controller-test and source-trace only, so reviewers still lack after-fix evidence from a real webchat run.

Maintainer options:

Preserve run identity before clearing (recommended)
Update sessions.changed reconciliation so runless terminal rows only clear the known stale or completed run and cannot clear a newer local chatRunId, with regression coverage for both stale and newer-run cases.
Prove the visible race path
Before merging, show a real webchat run where the composer returns from Stop to Send after completion and stays correct after delayed events, with private details redacted.
Replace the broad safety-net approach
If preserving newer-run state cannot be done narrowly in this branch, pause or replace it with a smaller state-machine fix that does not rely on broad runless terminal clearing.

Next step before merge

[P1] Contributor and maintainer action is needed: repair the session-state regressions and add real webchat proof; ClawSweeper should not take a repair lane while external real behavior proof is missing.

Security
Cleared: The diff is limited to UI controller state logic and tests, with no package, workflow, credential, permission, dependency, or code-execution surface changes found.

Review findings

[P1] Block late deltas from re-owning completed runs — ui/src/ui/controllers/chat.ts:722-729
[P1] Keep runless session patches from clearing newer runs — ui/src/ui/controllers/sessions.ts:145-150

Review details

Best possible solution:

Fix the chat-run state machine so terminal/delta and sessions.changed reconciliation clear only the completed stale run while preserving newer active runs, then add real webchat Stop-to-Send proof before merge.

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

Yes for the patch-level regressions: the PR's own added chat test leaves chatRunId re-adopted after final then late delta, and current main has a controller test for preserving newer local runs from runless older terminal patches. I did not run tests because this review is read-only.

Is this the best way to solve the issue?

No. The ownership boundary is plausible, but the current implementation is too broad in sessions.changed and still depends on later cleanup after late deltas; the safer fix needs run identity or recency guards plus visible webchat proof.

Full review comments:

[P1] Block late deltas from re-owning completed runs — ui/src/ui/controllers/chat.ts:722-729
The new adoption gate only excludes terminal events, so the sequence final clears chatRunId and a delayed delta for the same session immediately sets it again. The added test even expects this resurrection, which means the Stop button can still come back unless another terminal session patch happens afterward; the fix needs to remember or reject completed run IDs instead of relying on a later safety net.
Confidence: 0.86
[P1] Keep runless session patches from clearing newer runs — ui/src/ui/controllers/sessions.ts:145-150
Removing the eventRunId === undefined && state.chatRunId guard makes any runless terminal sessions.changed row for the selected session call reconcileChatRunFromCurrentSessionRow, which clears the local chatRunId without proving the row belongs to that active run. Current main had a regression test for this newer-run case, but the PR rewrites it to expect clearing, so a stale terminal patch can now drop the active run's Stop/abort state.
Confidence: 0.92

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 3d7df2bc0747.

Label changes

Label justifications:

P2: This is a normal-priority webchat state bugfix with limited blast radius, but it affects a visible core UI workflow.
merge-risk: 🚨 session-state: The diff changes chatRunId adoption and clearing rules, which can stale, clear, or mis-associate the active webchat run state.
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 reports local tests, lint/format, and source tracing, but it does not show after-fix real OpenClaw webchat behavior; add a screenshot, recording, terminal/live output, linked artifact, or redacted log showing Stop returning to Send, redact private details, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +13, Tests +129. Total +142 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	17	4	+13
Tests	2	136	7	+129
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	4	153	11	+142

What I checked:

Repository policy read: Root and UI scoped AGENTS.md were read; the review applied the OpenClaw guidance for PR proof, session-state risk, UI scope, and read-only review. (AGENTS.md:1, 3d7df2bc0747)
Current main protects runless terminal patches: Current main returns false when eventRunId is undefined and state.chatRunId is present, preventing a runless sessions.changed patch from clearing an existing local run. (ui/src/ui/controllers/sessions.ts:145, 3d7df2bc0747)
Existing regression test protects newer runs: Current main has a test named does not clear a newer local run from a runless older terminal patch; the PR rewrites this scenario to expect the local run to be cleared. (ui/src/ui/controllers/sessions.test.ts:1303, 3d7df2bc0747)
Clearing path drops local run state: reconcileChatRunFromCurrentSessionRow clears chatRunId and stream state when the selected row is terminal, so making the runless helper return true can drop the current local Stop/abort state. (ui/src/ui/chat/run-lifecycle.ts:201, 3d7df2bc0747)
PR patch still permits late-delta re-adoption: The PR adds a terminal-state adoption gate, but its new regression test explicitly expects a late delta after final to set state.chatRunId back to run-1, which is the visible Stop-state resurrection path unless another terminal session update arrives later. (ui/src/ui/controllers/chat.test.ts:841, fcf1ef0987e1)
PR proof is controller-only: The PR body reports local controller tests, lint/format checks, and manual source tracing, but it also says full E2E browser automation and production A/B comparison were not tested; no screenshot, recording, redacted logs, or live webchat output show Stop returning to Send. (fcf1ef0987e1)

Likely related people:

obviyus: Git blame shows the current sessionPatchTargetsCurrentChatRun and handleChatEvent controller lines came from commit d879340. (role: introduced current controller guard behavior; confidence: medium; commits: d879340ed999; files: ui/src/ui/controllers/sessions.ts, ui/src/ui/controllers/chat.ts)
steipete: Recent history and shortlog show heavy recent work on the UI chat/session controller surface, including the latest release commit that carried these files. (role: recent area contributor; confidence: medium; commits: 27ae826f6525, 7c862da6a1de, d155d578ebde; files: ui/src/ui/controllers/sessions.ts, ui/src/ui/controllers/chat.ts, ui/src/ui/views/chat.ts)
chziyue: Recent UI history includes adjacent Stop/Send behavior work in the chat UI, making this a useful routing candidate if the visible composer behavior needs context. (role: adjacent Stop-button behavior contributor; confidence: low; commits: d5c42a07ef2e; files: ui/src/ui/views/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.

…nt reply completes (openclaw#88033)

The original fix checked ctx.healthOk, but the contribution order runs doctor:gateway-auth before doctor:gateway-health, so ctx.healthOk was still undefined and the guard was never triggered. Instead, probe gateway health inline when gatewayTokenRef is set and ctx.healthOk is undefined. This gives us a real health signal before deciding whether to emit the SecretRef warning. When the gateway is confirmed healthy, the warning is suppressed since the SecretRef resolves fine at runtime — the CLI doctor simply can't audit file/exec-backed secrets. When the gateway is unhealthy or the health probe fails, the warning still appears normally. Also reverted the unrelated webchat busy-state change (handled by openclaw#88041).

openclaw-barnacle Bot added app: web-ui App: web-ui size: S labels May 29, 2026

github-actions Bot added the dependencies-changed PR changes dependency-related files label May 29, 2026

openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 29, 2026

fix(ui): prevent webchat Stop button from staying stuck after assista…

fcf1ef0

…nt reply completes (openclaw#88033)

chengzhichao-xydt force-pushed the fix/88033-webchat-stop-button-stuck-upstream branch from 862fb64 to fcf1ef0 Compare May 29, 2026 14:52

github-actions Bot removed the dependencies-changed PR changes dependency-related files label May 29, 2026

clawsweeper Bot removed the merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. label May 29, 2026

clawsweeper Bot mentioned this pull request May 29, 2026

fix: webchat stop button stays stuck after response completes #88045

Closed

chengzhichao-xydt closed this Jun 4, 2026

chengzhichao-xydt reopened this Jun 4, 2026

chengzhichao-xydt closed this Jun 4, 2026

chengzhichao-xydt reopened this Jun 4, 2026

chengzhichao-xydt closed this Jun 4, 2026

tiffanychum mentioned this pull request Jun 9, 2026

fix(ui): keep composer on Send when run-status toast expires (#88033) #91680

Closed

clawsweeper Bot mentioned this pull request Jun 19, 2026

[Bug]: Webchat composer stays stuck on red Stop button after response completes #88033

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix/88033 webchat stop button stuck upstream#88041

Fix/88033 webchat stop button stuck upstream#88041
chengzhichao-xydt wants to merge 1 commit into
openclaw:mainfrom
chengzhichao-xydt:fix/88033-webchat-stop-button-stuck-upstream

chengzhichao-xydt commented May 29, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 29, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 29, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

chengzhichao-xydt commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Description

Summary

Change Type

Scope

Security Impact

Root Cause Analysis

Bug 1: Late delta resurrects cleared chatRunId (controllers/chat.ts)

Bug 2: runId mismatch silently swallows terminal events (controllers/chat.ts)

Bug 3: sessions.changed safety net blocked by over-eager guard (controllers/sessions.ts)

Reproduction Steps (from issue)

Verification

Real Behavior Proof

Files Changed

Notes

Uh oh!

github-actions Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency graph guard cleared

Uh oh!

clawsweeper Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

chengzhichao-xydt commented May 29, 2026 •

edited

Loading

Bug 1: Late delta resurrects cleared chatRunId (`controllers/chat.ts`)

Bug 2: runId mismatch silently swallows terminal events (`controllers/chat.ts`)

Bug 3: sessions.changed safety net blocked by over-eager guard (`controllers/sessions.ts`)

github-actions Bot commented May 29, 2026 •

edited

Loading

clawsweeper Bot commented May 29, 2026 •

edited

Loading