Skip to content

fix(ui): prevent false busy state in Control UI webchat after response completion#87474

Closed
bladin wants to merge 3 commits into
openclaw:mainfrom
bladin:fix/87387-control-ui-webchat-false-busy-state
Closed

fix(ui): prevent false busy state in Control UI webchat after response completion#87474
bladin wants to merge 3 commits into
openclaw:mainfrom
bladin:fix/87387-control-ui-webchat-false-busy-state

Conversation

@bladin

@bladin bladin commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #87387: Control UI webchat could keep showing a false "In progress" status and "Stop" button after the assistant response completed because the main composer consumed session-list abortability even when local send/stream state was idle.

Changes

  • Split session-scoped abortability from main composer busy UI in ui/src/ui/views/chat.ts.
  • Keep the main badge, queue steer affordance, and Stop button tied to local active evidence (sending || stream !== null).
  • Add unit regression coverage for stale active session rows after completion, while preserving Stop/Steer during an active local stream.
  • Add mocked Chromium Control UI proof for send -> final stream -> stale active session row -> idle composer.

Real behavior proof

Behavior addressed: Control UI webchat no longer shows a false In progress badge, Stop button, or queue steer affordance after response completion when only stale session-list active-run state remains.

Real environment tested: Local OpenClaw checkout on rebased PR head f3d079672376a36eee90452d2a6f7d947d87234c, mocked Control UI Gateway E2E in Chromium, Node/Vitest through scripts/run-vitest.mjs.

Exact steps or command run after this patch:

node scripts/run-vitest.mjs ui/src/ui/views/chat.test.ts ui/src/ui/app-chat.test.ts ui/src/ui/app-gateway.sessions.node.test.ts ui/src/ui/chat/run-controls.test.ts
node scripts/run-vitest.mjs run --config test/vitest/vitest.ui-e2e.config.ts --configLoader runner ui/src/ui/e2e/chat-flow.e2e.test.ts
git diff --check

Evidence after fix:

Test Files  4 passed (4)
Tests       137 passed (137)

Test Files  1 passed (1)
Tests       2 passed (2)

git diff --check: clean

Observed result after fix: Unit regression fails on the old behavior, then passes with the split. The mocked browser flow sends a WebChat message, receives the final assistant event, waits for the done status to clear, injects stale sessions.changed active-run state, and confirms no In progress, no Stop generating, no Queue message, with the composer back on Send message.

What was not tested: Broad remote pnpm check:changed did not run locally. The delegated Testbox path failed before execution because .github/workflows/crabbox-hydrate.yml does not contain a Testbox step, and direct AWS Crabbox is not configured in this environment (crabbox login --url https://crabbox.openclaw.ai --provider aws required). Fresh PR CI was triggered by the push to f3d079672376a36eee90452d2a6f7d947d87234c.

AI Assistance

This PR was originally created with AI assistance (iCodemate). Maintainer follow-up added regression and mocked browser proof.

@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 28, 2026
@clawsweeper

clawsweeper Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 8, 2026, 10:36 PM ET / 02:36 UTC.

Summary
The branch splits Control UI WebChat composer abort UI from session-list abortability, adds unit and mocked browser regression coverage, and also changes exec approval security ranking.

PR surface: Source +3, Tests +158. Total +161 across 5 files.

Reproducibility: yes. from source: current renderChat drives the composer badge and abort controls from session-list abortability, so stale active session rows can keep the main WebChat UI looking busy after local send/stream state is idle.

Review metrics: 1 noteworthy metric.

  • Exec security helper changed: 1 helper changed. minSecurity feeds effective exec policy, so a one-line rank change can alter existing host approval boundaries 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:

  • Remove the unrelated exec security policy change or split it into a separately reviewed security PR.
  • [P1] Add current-head real Control UI/Gateway proof showing the composer returns to Send with no In progress, Stop, or queue steer state after completion.
  • Coordinate with fix(ui): guard abort UI with local live send/stream state #89895 so only one narrow implementation path lands.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body supplies unit and mocked Chromium Gateway output for an older head, but not current-head real Control UI/Gateway proof of the user-visible stuck-busy flow; screenshots, recording, terminal/live output, or redacted logs should avoid private IPs, keys, phone numbers, and non-public endpoints, then updating the PR body should trigger re-review or a maintainer can comment @clawsweeper re-review. 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.

Mantis proof suggestion
A short browser proof would materially help because the change is visible Control UI chat state after a response completes. 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 WebChat returns to Send with no In progress, Stop, or Queue message state after a completed response and stale active session-list state.

Risk before merge

  • [P1] Changing minSecurity makes less restrictive exec security win in effective-policy paths, so existing host approval files set to deny or allowlist may no longer bound requested full execution.
  • [P1] The PR body supplies only unit/mocked Chromium Gateway output for an older head; it does not show current-head real Control UI/Gateway behavior after the fix.
  • [P1] An open overlapping UI-only PR for the same linked bug means maintainers should choose one narrow implementation path before landing.

Maintainer options:

  1. Restore restrictive exec policy ordering (recommended)
    Remove the minSecurity rank-order change from this branch so host approval files continue to bound requested exec policy before any UI fix lands.
  2. Land only one UI fix path
    Coordinate this PR with fix(ui): guard abort UI with local live send/stream state #89895 and merge the narrower reviewed WebChat fix rather than landing duplicate variants.
  3. Pause this branch if proof stays mocked
    If current-head real Control UI/Gateway proof cannot be supplied, keep the issue open and pause this PR until a proof-positive replacement is ready.

Next step before merge

  • [P1] Contributor or maintainer action is needed because the PR has a high-confidence security finding plus a missing external proof gate, so it is not a safe automated repair lane candidate.

Security
Needs attention: The branch introduces an unrelated exec security-policy rank change that can weaken configured host approval boundaries.

Review findings

  • [P1] Restore restrictive exec policy ordering — src/infra/exec-approvals.ts:1531
Review details

Best possible solution:

Keep the WebChat busy-state fix narrow, remove the unrelated exec security-policy change, and land only one coordinated fix path with current-head real behavior proof.

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

Yes from source: current renderChat drives the composer badge and abort controls from session-list abortability, so stale active session rows can keep the main WebChat UI looking busy after local send/stream state is idle.

Is this the best way to solve the issue?

No as submitted: the UI guard is a plausible narrow fix, but the branch also changes exec security policy and overlaps another open implementation for the same bug.

Full review comments:

  • [P1] Restore restrictive exec policy ordering — src/infra/exec-approvals.ts:1531
    This flips minSecurity so full wins over deny/allowlist, but effective exec policy calls use this helper to let stricter host approval policy bound requested policy. With this change, a requested full policy can bypass an existing restrictive host approvals file, contradicting the documented security contract.
    Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR targets a real WebChat stuck-busy workflow and currently includes a security-boundary regression that should block merge.
  • merge-risk: 🚨 compatibility: Changing effective exec security resolution can alter existing users' configured approval and deny behavior during upgrade.
  • merge-risk: 🚨 security-boundary: The diff can make less restrictive exec policy win over a stricter host approvals file.
  • 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 supplies unit and mocked Chromium Gateway output for an older head, but not current-head real Control UI/Gateway proof of the user-visible stuck-busy flow; screenshots, recording, terminal/live output, or redacted logs should avoid private IPs, keys, phone numbers, and non-public endpoints, then updating the PR body should trigger re-review or a maintainer can comment @clawsweeper re-review. 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 +3, Tests +158. Total +161 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 2 7 4 +3
Tests 3 160 2 +158
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 5 167 6 +161

Security concerns:

  • [high] Exec approvals can be weakened — src/infra/exec-approvals.ts:1531
    minSecurity is used when combining requested policy with host approvals; making full the minimum lets less restrictive execution policy win over allowlist or deny.
    Confidence: 0.94

What I checked:

  • Current false-busy source path: Current main passes hasAbortableSessionRun(state) into renderChat, and renderChat uses that session-derived abortability to force composer in-progress status and Stop/Queue controls even when isBusy is false. (ui/src/ui/views/chat.ts:1533, aef1fad58d25)
  • Abortability caller source: app-render supplies canAbort: hasAbortableSessionRun(state), and hasAbortableSessionRun returns true from active session-list rows for the current session. (ui/src/ui/app-render.ts:3567, aef1fad58d25)
  • PR UI patch shape: The PR adds showLocalAbortableUi = showAbortableUi && isBusy and routes composer status, queue steer, and run controls through that local-live guard. (ui/src/ui/views/chat.ts:1530, 9172dd7ed4d0)
  • Exec helper changed in PR: The PR flips minSecurity ordering so full becomes the minimum value, which makes the helper return the less restrictive policy in combinations like deny plus full. (src/infra/exec-approvals.ts:1531, 9172dd7ed4d0)
  • Exec effective-policy contract: Current source uses minSecurity to combine requested policy with host approvals and labels the result as stricter host security winning. (src/infra/exec-approvals-effective.ts:371, aef1fad58d25)
  • Documented approval boundary: Docs state that /exec security=full ask=off only skips approvals when both requested policy and host approvals resolve to full/off, and that stricter host policy still wins. Public docs: docs/tools/exec-approvals.md. (docs/tools/exec-approvals.md:315, aef1fad58d25)

Likely related people:

  • Vincent Koc: Latest blame and recent log history touch the relevant Control UI and exec files, including the current-line boundary commit and a recent Control UI metadata commit. (role: recent area contributor; confidence: medium; commits: 9cdf85340936, 2c6bdc8b2831; files: ui/src/ui/views/chat.ts, ui/src/ui/app-chat.ts, src/infra/exec-approvals.ts)
  • Gustavo Madeira Santana: Authored the unified effective exec approval reporting/actions work that defines how requested policy and host approvals are combined. (role: exec approvals effective-policy contributor; confidence: high; commits: ba735d015809; files: src/infra/exec-approvals-effective.ts, src/infra/exec-approvals.ts, docs/tools/exec-approvals.md)
  • Peter Steinberger: History shows original exec host approval flow work plus multiple nearby UI/chat and exec policy maintenance commits. (role: exec approvals and UI area contributor; confidence: medium; commits: efdb33c97587, c6575891c793; files: src/agents/bash-tools.exec.ts, src/infra/exec-approvals.ts, ui/src/ui/views/chat.ts)
  • Val Alexander: History shows substantial Control UI/chat view feature work in the same file family that owns the visible composer and toolbar behavior. (role: Control UI feature contributor; confidence: medium; commits: a710366e9ece, f76a3c5225bb; files: ui/src/ui/views/chat.ts)
  • Tak Hoffman: History shows local exec-policy CLI and docs work that changed the exec approvals and policy surfaces affected by the unrelated PR change. (role: exec policy adjacent contributor; confidence: medium; commits: 4bf94aa0d669; files: src/infra/exec-approvals.ts, src/infra/exec-approvals-effective.ts, docs/tools/exec-approvals.md)
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 May 28, 2026
@clawsweeper

clawsweeper Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg: 🎁 locked until real behavior proof passes.

Details
  • No creature or rarity is rolled until proof passes.
  • Eggs are collectible flavor only; they do not affect labels, ratings, merge decisions, or automation.

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 28, 2026
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed 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. labels May 28, 2026
@bakecool-sina

Copy link
Copy Markdown

thank you very much !

@BunsDev
BunsDev force-pushed the fix/87387-control-ui-webchat-false-busy-state branch from a8f3b0b to f3d0796 Compare May 28, 2026 04:46
@BunsDev

BunsDev commented May 28, 2026

Copy link
Copy Markdown
Contributor

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 28, 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:

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels May 28, 2026
@BingqingLyu

This comment was marked as spam.

@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 29, 2026
Fixes issue openclaw#91283 where minSecurity() had the security rank order backwards,
treating 'full' as the most restrictive instead of the least restrictive.

The order is now correct:
- full: 0 (least restrictive - allows all commands)
- allowlist: 1 (medium restriction - allows only whitelisted commands)
- deny: 2 (most restrictive - denies all commands)

This allows session overrides with security='full' to properly override
agent defaults with security='allowlist'.

Also fixes the test cases that were asserting the incorrect behavior.
@bladin
bladin force-pushed the fix/87387-control-ui-webchat-false-busy-state branch from f3d0796 to 3bff74a Compare June 9, 2026 01:59
@bladin
bladin force-pushed the fix/87387-control-ui-webchat-false-busy-state branch from 3bff74a to e5c2967 Compare June 9, 2026 02:06
…e completion

Only show in-progress badge and Stop button when there is an active stream or sending operation, not just based on stale session state. This fixes issue openclaw#87387 where Control UI would continue displaying busy/abortable state after the assistant response had completed.

- Modified ui/src/ui/views/chat.ts:
  - Added showLocalAbortableUi to require isBusy in addition to showAbortableUi
  - Updated renderChatQueue canAbort to include isBusy
  - Updated renderChatRunControls canAbort to include isBusy

- Added tests:
  - e2e test for stale session state not forcing busy UI
  - unit tests for idle composer with stale session state and active stream

Real behavior proof:
- UI build: successful
- TypeScript compilation: no errors
- Tests: 58 chat.view tests passed, 8 run-controls tests passed
- No regression in existing functionality

Fixes openclaw#87387
@clawsweeper clawsweeper Bot removed the rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. label Jun 9, 2026
@bladin
bladin force-pushed the fix/87387-control-ui-webchat-false-busy-state branch from e5c2967 to ec94a74 Compare June 9, 2026 02:12
@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. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. and removed P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 9, 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 merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P1 High-priority user-facing bug, regression, or broken workflow. 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: S 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

5 participants