fix(canvas): keep desktop take-control connected (auto-reconnect + lease renewal) #2216

Merged

hongming merged 5 commits from fix/desktop-takecontrol-reconnect-renewal into main 2026-06-04 07:00:02 +00:00

Conversation 6 Commits 5 Files Changed 2

+154 -9

hongming commented 2026-06-04 06:00:19 +00:00

Owner

Copy Link

Copy Source

Problem

Desktop "take control" (noVNC) disconnects every ~5 minutes and never recovers.

Root cause (client half — see molecule-controlplane#522 for the websockify keepalive)

DisplayTab had two gaps:

1. No reconnect. On an unclean websocket drop the noVNC client just showed "Desktop stream disconnected" and the effect only re-ran when sessionUrl changed — so a single idle/network blip left the user permanently disconnected.
2. No lease renewal. acquireControl posts ttl_seconds: 300 only on click; the display-control lock is a 300s lease with no client heartbeat, so the lock (and the expires_at-bound session token) silently expired mid-session.

Fix

• Auto-reconnect on unclean disconnect (bounded backoff, max 10) using the freshest signed URL; a clean disconnect (user released) does not reconnect.
• Renew the lease on a 120s timer while controlling — re-acquire as the same holder extends the lock server-side (the existing ON CONFLICT ... controlled_by = EXCLUDED path). The fresh session URL is kept in a ref for reconnects without swapping the live stream's sessionUrl (which would needlessly reconnect the desktop).

Tests

13 passed — added "auto-reconnects after an unclean disconnect but not a clean one"; existing acquire/release/clipboard/observe tests unchanged.

Companion: molecule-controlplane#522 adds websockify --heartbeat 30 (prevents the idle drop in the first place). Together: the stream stays alive through idle, and recovers from any drop.

## Problem Desktop "take control" (noVNC) disconnects every ~5 minutes and never recovers. ## Root cause (client half — see molecule-controlplane#522 for the websockify keepalive) `DisplayTab` had two gaps: 1. **No reconnect.** On an unclean websocket drop the noVNC client just showed "Desktop stream disconnected" and the effect only re-ran when `sessionUrl` changed — so a single idle/network blip left the user permanently disconnected. 2. **No lease renewal.** `acquireControl` posts `ttl_seconds: 300` only on click; the display-control lock is a 300s lease with no client heartbeat, so the lock (and the `expires_at`-bound session token) silently expired mid-session. ## Fix - **Auto-reconnect** on unclean disconnect (bounded backoff, max 10) using the freshest signed URL; a *clean* disconnect (user released) does not reconnect. - **Renew the lease** on a 120s timer while controlling — re-acquire as the same holder extends the lock server-side (the existing `ON CONFLICT ... controlled_by = EXCLUDED` path). The fresh session URL is kept in a ref for reconnects **without** swapping the live stream's `sessionUrl` (which would needlessly reconnect the desktop). ## Tests `13 passed` — added "auto-reconnects after an unclean disconnect but not a clean one"; existing acquire/release/clipboard/observe tests unchanged. Companion: **molecule-controlplane#522** adds `websockify --heartbeat 30` (prevents the idle drop in the first place). Together: the stream stays alive through idle, and recovers from any drop.

hongming added 2 commits 2026-06-04 06:00:20 +00:00

fix(canvas): keep take-control alive — auto-reconnect + renew display-control lease e13031a951

test(canvas): cover take-control auto-reconnect on unclean disconnect 5a41dc330c

hongming added 1 commit 2026-06-04 06:00:20 +00:00

test(canvas): cover take-control auto-reconnect on unclean disconnect 5a41dc330c

hongming added 1 commit 2026-06-04 06:14:08 +00:00

fix(canvas): re-acquire a fresh token on reconnect (avoid stale-token 401) — review fix 57fe54ccc3

hongming added 1 commit 2026-06-04 06:14:08 +00:00

test(canvas): assert reconnect dials with a freshly-minted token 293bfb6abe

hongming commented 2026-06-04 06:14:48 +00:00

Author

Owner

Copy Link

Copy Source

Independent adversarial review done; one BLOCKER found + fixed.

The reviewer flagged a real correctness bug in the first cut: the reconnect reused a cached signed session URL, whose token is only ~300s — so a reconnect after the token's window (especially after any swallowed renewal failure) would 401 and never recover, reproducing the exact symptom this PR fixes.

Fix (pushed): reconnect now re-acquires a fresh lease+token first (reacquireSession() — shared by the renewal timer and the reconnect path) rather than dialing with a possibly-stale ref. The renewal generation-guard is now consistent with acquireControl (rides requestGeneration). Added a test asserting the reconnect dials with a freshly-minted token (signed2), not the stale original (signed) — the previous test would have passed even with the bug.

13 passed, eslint clean (0 errors), tsc clean for DisplayTab. Verified the noVNC connect/disconnect event + clean flag assumptions against @novnc/novnc@1.7.0 source.

**Independent adversarial review done; one BLOCKER found + fixed.** The reviewer flagged a real correctness bug in the first cut: the reconnect reused a **cached** signed session URL, whose token is only ~300s — so a reconnect after the token's window (especially after any swallowed renewal failure) would 401 and never recover, reproducing the exact symptom this PR fixes. **Fix (pushed):** reconnect now **re-acquires a fresh lease+token first** (`reacquireSession()` — shared by the renewal timer and the reconnect path) rather than dialing with a possibly-stale ref. The renewal generation-guard is now consistent with `acquireControl` (rides `requestGeneration`). Added a test asserting the reconnect dials with a **freshly-minted** token (`signed2`), not the stale original (`signed`) — the previous test would have passed even with the bug. `13 passed`, eslint clean (0 errors), tsc clean for DisplayTab. Verified the noVNC `connect`/`disconnect` event + `clean` flag assumptions against `@novnc/novnc@1.7.0` source.

hongming added the tier:low label 2026-06-04 06:16:35 +00:00

core-security approved these changes 2026-06-04 06:18:07 +00:00

Dismissed

core-security left a comment

Member

Copy Link

Copy Source

Security approve. Client-only canvas change (display take-control reconnect + lease renewal). No new secrets, no auth-boundary change — reuses the existing signed display-session token + the existing acquire endpoint (same-holder lease extension). Independent adversarial review done; the stale-token reconnect BLOCKER it found is fixed (reconnect re-acquires a fresh token). 13 tests pass.

Security approve. Client-only canvas change (display take-control reconnect + lease renewal). No new secrets, no auth-boundary change — reuses the existing signed display-session token + the existing acquire endpoint (same-holder lease extension). Independent adversarial review done; the stale-token reconnect BLOCKER it found is fixed (reconnect re-acquires a fresh token). 13 tests pass.

core-qa approved these changes 2026-06-04 06:18:52 +00:00

Dismissed

core-qa left a comment

Member

Copy Link

Copy Source

QA approve. 13 canvas tests pass incl reconnect-uses-fresh-token + clean-disconnect-no-reconnect; lint clean; tsc clean for DisplayTab. noVNC event assumptions verified vs @novnc/novnc 1.7.0. Companion CP#522 keepalive already deployed to prod.

QA approve. 13 canvas tests pass incl reconnect-uses-fresh-token + clean-disconnect-no-reconnect; lint clean; tsc clean for DisplayTab. noVNC event assumptions verified vs @novnc/novnc 1.7.0. Companion CP#522 keepalive already deployed to prod.

hongming commented 2026-06-04 06:18:53 +00:00

Author

Owner

Copy Link

Copy Source

/qa-recheck

/qa-recheck

hongming commented 2026-06-04 06:18:54 +00:00

Author

Owner

Copy Link

Copy Source

/security-recheck

/security-recheck

hongming commented 2026-06-04 06:46:49 +00:00

Author

Owner

Copy Link

Copy Source

/qa-recheck

/qa-recheck

hongming added 1 commit 2026-06-04 06:52:55 +00:00

fix(canvas): stop requesting server-side resize (x11vnc rejects it; console spam) 4f26ed69c3

hongming dismissed core-security's review 2026-06-04 06:52:55 +00:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

hongming dismissed core-qa's review 2026-06-04 06:52:55 +00:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

core-security approved these changes 2026-06-04 06:53:21 +00:00

core-security left a comment

Member

Copy Link

Copy Source

Security re-approve on new head (added resizeSession=false to quiet the x11vnc resize-prohibited console spam; no security surface change).

Security re-approve on new head (added resizeSession=false to quiet the x11vnc resize-prohibited console spam; no security surface change).

core-qa approved these changes 2026-06-04 06:53:23 +00:00

core-qa left a comment

Member

Copy Link

Copy Source

QA re-approve on new head. 13 tests pass; resizeSession=false quiets resize spam.

QA re-approve on new head. 13 tests pass; resizeSession=false quiets resize spam.

hongming commented 2026-06-04 06:53:24 +00:00

Author

Owner

Copy Link

Copy Source

/qa-recheck

/qa-recheck

hongming commented 2026-06-04 06:53:25 +00:00

Author

Owner

Copy Link

Copy Source

/security-recheck

/security-recheck

hongming merged commit 8186baf902 into main 2026-06-04 07:00:02 +00:00

core-be referenced this pull request 2026-06-04 08:01:31 +00:00

[main-red] molecule-ai/molecule-core: 8186baf902 #2224

Sign in to join this conversation.

Reviewers

No Reviewers

core-security

core-qa

Labels

Clear labels

area/ci

CI/CD pipeline issues

do-not-auto-merge

Opt out of autonomous merge-queue merging

kind/infrastructure

Infrastructure-related issues

merge-queue

Ready for serialized Gitea merge queue

merge-queue-hold

Temporarily hold PR in merge queue

platform/go

Go platform test issues

release-blocker

Blocks the staging→main promotion / a release

release-test

security

test-label-sre

tier:high

High risk per dev-sop §SOP-6 — ceo only, 24h cooldown

tier:low

Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve

tier:medium

Medium risk per dev-sop §SOP-6 — managers/ceo can approve

triage-test

test

wip

Work in progress — do not auto-merge

No Label tier:low

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

agent-dev-a agent-dev-b agent-pm agent-researcher agent-reviewer agent-reviewer-1 agent-reviewer-cr2 app-fe (Molecule AI · app-fe) app-lead (Molecule AI · app-lead) app-qa (Molecule AI · app-qa) claude-ceo-assistant claude-ci-reader claude-status-reaper core-be (Molecule AI · core-be) core-devops (Molecule AI · core-devops) core-fe (Molecule AI · core-fe) core-lead (Molecule AI · core-lead) core-offsec (Molecule AI · core-offsec) core-qa (Molecule AI · core-qa) core-security (Molecule AI · core-security) core-uiux (Molecule AI · core-uiux) cp-be (Molecule AI · cp-be) cp-lead (Molecule AI · cp-lead) cp-qa (Molecule AI · cp-qa) cp-security (Molecule AI · cp-security) cui (Zhanlin Cui) dev-lead (Molecule AI · dev-lead) devops-engineer documentation-specialist (Molecule AI · documentation-specialist) fullstack-engineer (Molecule AI · fullstack-engineer) hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 infra-lead (Molecule AI · infra-lead) infra-runtime-be (Molecule AI · infra-runtime-be) infra-sre (Molecule AI · infra-sre) integration-tester (Molecule AI · integration-tester) mc-drift-bot molecule-code-reviewer plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot pypi-publisher (Molecule AI PyPI Publisher (RFC#596)) release-manager (Molecule AI · release-manager) sdk-dev (Molecule AI · sdk-dev) sdk-lead (Molecule AI · sdk-lead) sop-drift-bot sop-tier-bot (SOP Tier-Check Bot) technical-writer (Molecule AI · technical-writer) triage-operator (Molecule AI · triage-operator)

No Assignees

3 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#2216