Skip to content

fix(ui): keep dashboard mounted during reconnect#94703

Closed
wuqxuan wants to merge 1 commit into
openclaw:mainfrom
wuqxuan:issue-72500-control-ui-reconnect
Closed

fix(ui): keep dashboard mounted during reconnect#94703
wuqxuan wants to merge 1 commit into
openclaw:mainfrom
wuqxuan:issue-72500-control-ui-reconnect

Conversation

@wuqxuan

@wuqxuan wuqxuan commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the Control UI reconnect gate so a transient Gateway disconnect after a successful connection keeps the dashboard/chat shell mounted instead of replacing it with the login gate.

The fix tracks whether the current Gateway identity has connected successfully, resets that state when the Gateway URL/token/password changes, and preserves the mounted dashboard during the bounded cached-device-token retry path for AUTH_TOKEN_MISMATCH.

Verification

  • node node_modules/vitest/vitest.mjs run --config test/vitest/vitest.ui.config.ts --reporter=verbose ui/src/ui/app-render.assistant-avatar.test.ts
  • node node_modules/vitest/vitest.mjs run --config test/vitest/vitest.ui.config.ts --reporter=dot ui/src/ui/app-gateway.node.test.ts
  • node node_modules/vitest/vitest.mjs run --config test/vitest/vitest.ui.config.ts --reporter=dot ui/src/ui/navigation.browser.test.ts
  • node node_modules/vitest/vitest.mjs run --config test/vitest/vitest.ui.config.ts --reporter=dot ui/src/ui/gateway.node.test.ts
  • node_modules/oxfmt/bin/oxfmt --check --threads=1 ui/src/ui/app-render.ts ui/src/ui/app-gateway.ts ui/src/ui/app.ts ui/src/ui/app-view-state.ts ui/src/ui/app-render.assistant-avatar.test.ts ui/src/ui/app-gateway.node.test.ts ui/src/ui/gateway.ts ui/src/ui/gateway.node.test.ts ui/src/ui/navigation.browser.test.ts ui/src/ui/views/login-gate.ts
  • git diff --check
  • node node_modules/@typescript/native-preview/bin/tsgo.js -p test/tsconfig/tsconfig.test.ui.json --pretty false --incremental --tsBuildInfoFile .artifacts/tsgo-cache/test-ui.local-fix.tsbuildinfo
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main

Real behavior proof

Behavior addressed: Control UI no longer replaces an already-mounted dashboard/chat view with the login gate during transient reconnects after a successful Gateway connection.

Real environment tested: Local Control UI E2E server with mocked Gateway WebSocket in Chrome on macOS, plus focused Vitest/jsdom coverage for reconnect, auth failure, device-token retry, and Gateway identity edit paths.

Exact steps or command run after this patch: node --import tsx .artifacts/control-ui-e2e/reconnect-gate/capture.mjs against the patched branch; focused Vitest, oxfmt, diff check, tsgo, and autoreview commands listed above.

Evidence after fix: Screenshot artifacts were generated locally under .artifacts/control-ui-e2e/reconnect-gate/ and intentionally not committed per repository policy. transient-reconnect-dashboard.png shows the dashboard/chat shell still mounted with a disconnect warning; auth-scope-login-gate.png shows explicit auth-scope failure still routes to the login gate. transient-reconnect-before-login-gate.png was captured from origin/main and shows the previous behavior returning to the login gate for the same transient disconnect.

Observed result after fix: A generic disconnected (1006): no reason after a prior successful connection keeps .shell visible and keeps .login-gate detached. Explicit auth/config failures still render the login gate, and cached device-token retry windows do not flash the credential screen.

What was not tested: Full packaged app flow, live remote Gateway, and cross-OS browser proof were not run; coverage is scoped to the Control UI reconnect behavior and mocked Gateway protocol paths.

AI-assisted: yes.

Agent Transcript

Redacted agent session transcript 
[user]
扫描有没有适合修复的issue

[assistant]
Scanned open issues and excluded duplicates or already-covered items. Selected #72500 because the current Control UI render path still returned to the login gate whenever connected became false, while Gateway reconnect paths set connected=false for transient disconnects.

[user]
修复

[assistant]
Implemented a Control UI reconnect gate: keep the shell mounted after a successful connection during generic reconnects, reset first-connect state when Gateway URL/token/password changes, keep explicit auth/config failures on the login gate, and avoid flashing the login gate during the cached device-token retry path.

[user]
可以提供截图验证吗

[assistant]
Generated local screenshot proof with a mocked Gateway WebSocket: before on origin/main, after on the patched branch, and an auth-scope regression guard. The screenshots were kept under .artifacts and not committed, per repo policy.

[assistant]
Ran focused verification: render tests, app gateway tests, navigation tests, Gateway client tests, oxfmt, git diff --check, tsgo, and repeated autoreview. Addressed review findings for Gateway identity changes, password edits, overview password edits, and cached device-token retry behavior.

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

clawsweeper Bot commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 18, 2026, 7:33 PM ET / 23:33 UTC.

Summary
The PR adds sticky Control UI Gateway connection state so transient disconnects after a successful connection keep the dashboard/chat shell mounted while explicit login-required states still route to the login gate.

PR surface: Source +89, Tests +147. Total +236 across 10 files.

Reproducibility: yes. Current main is source-reproducible for the linked bug, and the PR regression is source-reproducible by following a post-hello 4001 auth/device revocation close with no structured connect error into the new render gate.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #72500
Summary: This PR is a candidate fix for the open Control UI reconnect/login-gate flash issue, but it is not merge-ready as-is.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Handle Gateway 4001 auth/device revocation closes with regression coverage.
  • Attach redacted before/after Control UI proof showing transient reconnect preservation and auth/device revocation login-gate behavior.
  • Update the PR body after adding proof so a fresh ClawSweeper review can run automatically, or ask a maintainer for @clawsweeper re-review.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body says screenshots were generated under .artifacts and intentionally not committed, but no inspectable screenshot, recording, log, terminal output, or linked artifact is attached. 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 visible Control UI proof would materially help verify both transient reconnect preservation and auth-failure login-gate behavior after the code defect is fixed. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: verify the Control UI keeps the dashboard mounted during transient Gateway reconnect and shows the login gate after auth or device revocation.

Risk before merge

  • [P1] Merging as-is can keep cached Control UI dashboard/chat content visible after Gateway auth rotation or device removal closes the WebSocket with code 4001 and no structured connect error.
  • [P1] The PR body describes local screenshot artifacts, but no screenshot, recording, log, terminal output, or linked artifact is attached for reviewer inspection.

Maintainer options:

  1. Handle revocation closes before merge (recommended)
    Map known Gateway 4001 auth/device revocation close reasons to the login-gate reset path, add focused regression tests, and require inspectable browser proof before merge.
  2. Replace with a maintainer-owned fix
    If the contributor cannot update the branch or attach proof, keep the canonical issue open and replace this with a narrow fix that preserves the same transient reconnect behavior safely.

Next step before merge

  • [P1] The PR needs contributor/maintainer code changes plus inspectable real behavior proof; missing proof keeps this human-only rather than a safe repair lane.

Security
Needs attention: The sticky reconnect gate can preserve privileged cached Control UI state after current Gateway auth or device revocation closes.

Review findings

  • [P1] Reset the gate on auth-revocation closes — ui/src/ui/app-gateway.ts:914-919
Review details

Best possible solution:

Preserve the shell during transient reconnects, but reset the prior-success gate for structured auth failures and known 4001 auth/device revocation close reasons, with regression coverage and inspectable browser proof.

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

Yes. Current main is source-reproducible for the linked bug, and the PR regression is source-reproducible by following a post-hello 4001 auth/device revocation close with no structured connect error into the new render gate.

Is this the best way to solve the issue?

No, not as-is. The sticky prior-success approach is the right shape for transient reconnects, but it must also handle current Gateway auth-revocation close frames and provide inspectable UI proof before merge.

Full review comments:

  • [P1] Reset the gate on auth-revocation closes — ui/src/ui/app-gateway.ts:914-919
    After this change, a post-hello Gateway close such as 4001 / gateway auth changed or device removed carries no structured connect error, so lastErrorCode stays null and the new render gate keeps cached dashboard UI mounted. Please translate those known revocation closes into a prior-success reset/login-gate path and add coverage.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 55323103b912.

Label changes

Label justifications:

  • P2: This is a focused Control UI bug-fix PR with limited blast radius, but it has a merge-blocking auth-gating defect.
  • merge-risk: 🚨 auth-provider: The PR changes UI behavior around Gateway auth, token retry, scope failure, and revocation-state routing.
  • merge-risk: 🚨 security-boundary: The diff can preserve cached privileged UI after current Gateway auth/device revocation closes if merged as-is.
  • 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 says screenshots were generated under .artifacts and intentionally not committed, but no inspectable screenshot, recording, log, terminal output, or linked artifact is attached. 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 +89, Tests +147. Total +236 across 10 files.

View PR surface stats
Area Files Added Removed Net
Source 6 118 29 +89
Tests 4 147 0 +147
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 10 265 29 +236

Security concerns:

  • [medium] Sticky UI survives auth revocation close — ui/src/ui/app-gateway.ts:914
    Current Gateway code closes stale shared-auth and removed-device clients with code 4001 and reason text, but the PR only gates on structured connect-error details or selected strings later in rendering. That can leave cached chat/admin UI visible after credentials or device access are revoked.
    Confidence: 0.9

What I checked:

  • repository policy read: Root and UI-scoped AGENTS.md were read; the review applied the Control UI proof, security-boundary, and read-beyond-diff guidance. (AGENTS.md:1, 55323103b912)
  • live PR head: Live GitHub metadata shows the PR is draft, cross-repository, and still at head 09190e4 with no attached proof comments beyond the existing ClawSweeper review. (09190e44e378)
  • current main login-gate behavior: Current main still renders the login gate whenever state.connected is false, which matches the linked source-level bug report. (ui/src/ui/app-render.ts:1391, 55323103b912)
  • PR render gate: At PR head, shouldRenderLoginGate keeps the shell mounted after a prior successful connection unless a structured error code, recognized hint, insecure-context hint, or protocol-mismatch text is present. (ui/src/ui/app-render.ts:271, 09190e44e378)
  • PR close handler: The PR close handler records deviceTokenRetryPending but does not reset hasConnectedGateway or synthesize an auth detail for known revocation close reasons. (ui/src/ui/app-gateway.ts:914, 09190e44e378)
  • current auth-rotation close: Current main closes stale shared-auth clients with code 4001 and reason 'gateway auth changed', an unstructured close path that reaches the browser close handler without connect-error details. (src/gateway/server-shared-auth-generation.ts:32, 55323103b912)

Likely related people:

  • vincentkoc: Current blame points the affected Control UI render/close-gate lines and Gateway revocation close lines to recent work by Vincent Koc; nearby history also shows recent Control UI startup and reconnect work. (role: recent area contributor; confidence: high; commits: 2ae84f75efde, 227757079cc0, 8c8c8c8e32e2; files: ui/src/ui/app-render.ts, ui/src/ui/app-gateway.ts, src/gateway/server-shared-auth-generation.ts)
  • BunsDev: Recent commits by BunsDev define Control UI login failure guidance and Gateway auth retry-loop behavior that this PR must preserve. (role: recent adjacent contributor; confidence: high; commits: 79e31421225b, 348ffe60612c; files: ui/src/ui/views/login-gate.ts, ui/src/ui/gateway.ts, ui/src/ui/app-gateway.ts)
  • steipete: Recent Gateway client extraction and protocol-mismatch documentation touched the browser Gateway/auth contract used by this reconnect gate. (role: recent adjacent contributor; confidence: medium; commits: b1117d98622f, a535978352ca, 12a56d4d46dd; files: ui/src/ui/gateway.ts, packages/gateway-protocol/src/connect-error-details.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: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jun 18, 2026
@wuqxuan

wuqxuan commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Screenshot proof for the Control UI reconnect fix:

  1. Before: transient reconnect on origin/main returns to the login gate.
transient-reconnect-before-login-gate
  1. After: transient reconnect keeps the dashboard/chat shell mounted with the disconnect warning.transient-reconnect-dashboard

@wuqxuan wuqxuan closed this Jun 19, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 26, 2026
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Jul 6, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

app: web-ui App: web-ui merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. 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: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@wuqxuan