From e13031a951d7b548a1e3ceada0f36e2191ef873b Mon Sep 17 00:00:00 2001 From: hongming Date: Thu, 4 Jun 2026 06:00:18 +0000 Subject: [PATCH 1/5] =?UTF-8?q?fix(canvas):=20keep=20take-control=20alive?= =?UTF-8?q?=20=E2=80=94=20auto-reconnect=20+=20renew=20display-control=20l?= =?UTF-8?q?ease?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- canvas/src/components/tabs/DisplayTab.tsx | 75 +++++++++++++++++++++-- 1 file changed, 71 insertions(+), 4 deletions(-) diff --git a/canvas/src/components/tabs/DisplayTab.tsx b/canvas/src/components/tabs/DisplayTab.tsx index 94f50730c..9c9af9863 100644 --- a/canvas/src/components/tabs/DisplayTab.tsx +++ b/canvas/src/components/tabs/DisplayTab.tsx @@ -33,6 +33,11 @@ export function DisplayTab({ workspaceId }: Props) { const [controlBusy, setControlBusy] = useState(false); const [sessionUrl, setSessionUrl] = useState(null); const requestGeneration = useRef(0); + // Freshest signed session URL (token bound to the lease's expires_at). The + // renewal timer keeps this current WITHOUT swapping the live stream's + // sessionUrl (which would needlessly reconnect the desktop); the stream uses + // it only when it has to reconnect after an unclean drop. + const latestSessionUrlRef = useRef(null); useEffect(() => { const generation = requestGeneration.current + 1; @@ -41,6 +46,7 @@ export function DisplayTab({ workspaceId }: Props) { setStatus(null); setControl(null); setSessionUrl(null); + latestSessionUrlRef.current = null; setError(null); setControlError(null); setControlBusy(false); @@ -69,6 +75,35 @@ export function DisplayTab({ workspaceId }: Props) { }; }, [workspaceId]); + // Renew the display-control lease while we hold it. The lock is a 300s lease + // with no server-side auto-renewal, so without this the control (and the + // session token) silently expire mid-session — the user appears "kicked" + // every ~5 minutes. Re-acquiring as the same holder extends the lease + // server-side; we refresh the reconnect token in latestSessionUrlRef but do + // NOT swap the live stream's sessionUrl (that would reconnect the desktop). + useEffect(() => { + if (!sessionUrl) return; + const generation = requestGeneration.current; + const controlPath = `/workspaces/${workspaceId}/display/control`; + const renewMs = 120_000; // well under the 300s TTL + const timer = setInterval(async () => { + try { + const next = await api.post(`${controlPath}/acquire`, { + controller: "user", + ttl_seconds: 300, + }); + if (requestGeneration.current !== generation) return; + setControl(next); + if (next.session_url) latestSessionUrlRef.current = next.session_url; + } catch { + // Transient renewal failure (or another holder took over): the live + // stream keeps running on its existing connection; surfaced control + // state stays as-is until the next tick or an explicit re-acquire. + } + }, renewMs); + return () => clearInterval(timer); + }, [sessionUrl, workspaceId]); + const acquireControl = async () => { const generation = requestGeneration.current; const controlPath = `/workspaces/${workspaceId}/display/control`; @@ -82,6 +117,7 @@ export function DisplayTab({ workspaceId }: Props) { if (requestGeneration.current !== generation) return; setControl(next); setSessionUrl(next.session_url || null); + latestSessionUrlRef.current = next.session_url || null; } catch (err) { if (requestGeneration.current !== generation) return; setControlError("Failed to take control"); @@ -108,6 +144,7 @@ export function DisplayTab({ workspaceId }: Props) { if (requestGeneration.current !== generation) return; setControl(next); setSessionUrl(null); + latestSessionUrlRef.current = null; } catch (err) { if (requestGeneration.current !== generation) return; setControlError("Failed to release control"); @@ -235,7 +272,7 @@ export function DisplayTab({ workspaceId }: Props) { /> {sessionUrl ? ( - + ) : (
@@ -311,7 +348,13 @@ function DisplayControlBar({ ); } -function DesktopStream({ sessionUrl }: { sessionUrl: string }) { +function DesktopStream({ + sessionUrl, + latestSessionUrlRef, +}: { + sessionUrl: string; + latestSessionUrlRef: { current: string | null }; +}) { const containerRef = useRef(null); const rfbRef = useRef(null); const [streamError, setStreamError] = useState(null); @@ -329,12 +372,18 @@ function DesktopStream({ sessionUrl }: { sessionUrl: string }) { clipboardTimer = setTimeout(() => setClipboardStatus(null), 2500); }; + let attempts = 0; + let retryTimer: ReturnType | null = null; + const maxAttempts = 10; + async function connect() { setStreamError(null); try { const mod = await import("@novnc/novnc"); if (cancelled || !containerRef.current) return; - const stream = displayWebSocketConnection(sessionUrl); + // Use the freshest signed URL (kept current by the lease-renewal timer) + // so a reconnect after the original token's window still authenticates. + const stream = displayWebSocketConnection(latestSessionUrlRef.current || sessionUrl); rfb = new mod.default(containerRef.current, stream.url, { wsProtocols: ["binary", `molecule-display-token.${stream.token}`], }); @@ -343,6 +392,10 @@ function DesktopStream({ sessionUrl }: { sessionUrl: string }) { rfb.resizeSession = true; rfb.focusOnClick = true; rfb.focus({ preventScroll: true }); + rfb.addEventListener("connect", () => { + attempts = 0; + if (!cancelled) setStreamError(null); + }); rfb.addEventListener("clipboard", (event: Event) => { const text = (event as CustomEvent<{ text?: string }>).detail?.text ?? ""; if (!text) return; @@ -353,7 +406,20 @@ function DesktopStream({ sessionUrl }: { sessionUrl: string }) { }); rfb.addEventListener("disconnect", (event: Event) => { const detail = (event as CustomEvent<{ clean?: boolean }>).detail; - if (!cancelled && !detail?.clean) setStreamError("Desktop stream disconnected."); + rfbRef.current = null; + if (cancelled || detail?.clean) return; + // Auto-reconnect after an unclean drop (idle/network blip, brief + // agent hiccup); bounded backoff so a genuinely-dead session still + // surfaces an error instead of looping forever. + if (attempts < maxAttempts) { + attempts += 1; + setStreamError(`Reconnecting to desktop… (attempt ${attempts})`); + retryTimer = setTimeout(() => { + if (!cancelled) void connect(); + }, Math.min(1000 * attempts, 5000)); + } else { + setStreamError("Desktop stream disconnected."); + } }); } catch { if (!cancelled) setStreamError("Desktop stream could not be opened."); @@ -363,6 +429,7 @@ function DesktopStream({ sessionUrl }: { sessionUrl: string }) { connect(); return () => { cancelled = true; + if (retryTimer) clearTimeout(retryTimer); if (clipboardTimer) clearTimeout(clipboardTimer); rfbRef.current = null; rfb?.disconnect(); -- 2.52.0 From 5a41dc330c50444996af709e7a7120881082be04 Mon Sep 17 00:00:00 2001 From: hongming Date: Thu, 4 Jun 2026 06:00:19 +0000 Subject: [PATCH 2/5] test(canvas): cover take-control auto-reconnect on unclean disconnect --- .../tabs/__tests__/DisplayTab.test.tsx | 48 ++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/canvas/src/components/tabs/__tests__/DisplayTab.test.tsx b/canvas/src/components/tabs/__tests__/DisplayTab.test.tsx index afd84361c..188053514 100644 --- a/canvas/src/components/tabs/__tests__/DisplayTab.test.tsx +++ b/canvas/src/components/tabs/__tests__/DisplayTab.test.tsx @@ -2,12 +2,13 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; -const { mockGet, mockPost, mockRFBConstructor, mockRFBClipboardPasteFrom, mockRFBFocus } = vi.hoisted(() => ({ +const { mockGet, mockPost, mockRFBConstructor, mockRFBClipboardPasteFrom, mockRFBFocus, rfbInstances } = vi.hoisted(() => ({ mockGet: vi.fn(), mockPost: vi.fn(), mockRFBConstructor: vi.fn(), mockRFBClipboardPasteFrom: vi.fn(), mockRFBFocus: vi.fn(), + rfbInstances: [] as EventTarget[], })); vi.mock("@/lib/api", () => ({ @@ -31,6 +32,7 @@ vi.mock("@novnc/novnc", () => ({ this.url = url; this.options = options; mockRFBConstructor(target, url, options); + rfbInstances.push(this); } clipboardPasteFrom(text: string) { mockRFBClipboardPasteFrom(text); @@ -52,6 +54,7 @@ describe("DisplayTab", () => { mockRFBConstructor.mockReset(); mockRFBClipboardPasteFrom.mockReset(); mockRFBFocus.mockReset(); + rfbInstances.length = 0; }); it("renders unavailable state for non-display workspaces", async () => { @@ -400,6 +403,49 @@ describe("DisplayTab", () => { }); expect(screen.getByRole("button", { name: "Take control" })).toBeTruthy(); }); + + it("auto-reconnects the desktop stream after an unclean disconnect but not a clean one", async () => { + mockGet + .mockResolvedValueOnce({ + available: true, + mode: "desktop-control", + protocol: "novnc", + width: 1920, + height: 1080, + }) + .mockResolvedValueOnce({ controller: "none" }); + mockPost.mockResolvedValue({ + controller: "user", + controlled_by: "admin-token", + expires_at: "2026-05-23T08:48:27Z", + session_url: "/workspaces/ws-display/display/session/websockify#token=signed", + }); + + render(); + await waitFor(() => { + expect(screen.getByRole("button", { name: "Take control" })).toBeTruthy(); + }); + fireEvent.click(screen.getByRole("button", { name: "Take control" })); + await waitFor(() => { + expect(rfbInstances.length).toBe(1); + }); + + // An idle/network drop closes the websocket uncleanly. The client must + // reconnect (using the freshest signed URL) instead of giving up — this is + // the "disconnects every ~5 min and stays dead" report. + rfbInstances[0].dispatchEvent(new CustomEvent("disconnect", { detail: { clean: false } })); + await waitFor( + () => { + expect(rfbInstances.length).toBe(2); + }, + { timeout: 3000 }, + ); + + // A clean disconnect (the user released control) must NOT reconnect. + rfbInstances[1].dispatchEvent(new CustomEvent("disconnect", { detail: { clean: true } })); + await new Promise((resolve) => setTimeout(resolve, 1100)); + expect(rfbInstances.length).toBe(2); + }); }); function deferred() { -- 2.52.0 From 57fe54ccc316d0ee3387b36b2139d1e439313c87 Mon Sep 17 00:00:00 2001 From: hongming Date: Thu, 4 Jun 2026 06:14:06 +0000 Subject: [PATCH 3/5] =?UTF-8?q?fix(canvas):=20re-acquire=20a=20fresh=20tok?= =?UTF-8?q?en=20on=20reconnect=20(avoid=20stale-token=20401)=20=E2=80=94?= =?UTF-8?q?=20review=20fix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- canvas/src/components/tabs/DisplayTab.tsx | 78 +++++++++++++---------- 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/canvas/src/components/tabs/DisplayTab.tsx b/canvas/src/components/tabs/DisplayTab.tsx index 9c9af9863..11b200c6b 100644 --- a/canvas/src/components/tabs/DisplayTab.tsx +++ b/canvas/src/components/tabs/DisplayTab.tsx @@ -1,6 +1,6 @@ "use client"; -import { useEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { api } from "@/lib/api"; import type RFB from "@novnc/novnc"; @@ -75,34 +75,40 @@ export function DisplayTab({ workspaceId }: Props) { }; }, [workspaceId]); - // Renew the display-control lease while we hold it. The lock is a 300s lease - // with no server-side auto-renewal, so without this the control (and the - // session token) silently expire mid-session — the user appears "kicked" - // every ~5 minutes. Re-acquiring as the same holder extends the lease - // server-side; we refresh the reconnect token in latestSessionUrlRef but do - // NOT swap the live stream's sessionUrl (that would reconnect the desktop). + // Acquire (or re-acquire) the display-control lease as the current holder. + // Re-acquiring extends the 300s server-side lock AND returns a freshly-signed + // session URL (token bound to the new expires_at). Used both to renew the + // lease on a timer and to mint a non-stale token for each reconnect — a + // cached URL can be past its ~300s expiry, which would make a reconnect 401. + const reacquireSession = useCallback(async (): Promise => { + const generation = requestGeneration.current; + try { + const next = await api.post( + `/workspaces/${workspaceId}/display/control/acquire`, + { controller: "user", ttl_seconds: 300 }, + ); + if (requestGeneration.current !== generation) return null; + setControl(next); + if (next.session_url) latestSessionUrlRef.current = next.session_url; + return next.session_url ?? null; + } catch { + // Transient failure, or another holder took over: the live stream keeps + // running on its existing connection; a reconnect re-evaluates control. + return null; + } + }, [workspaceId]); + + // Renew the lease while we hold it. The lock is a 300s lease with no + // server-side auto-renewal, so without this the control (and the session + // token) silently expire mid-session — the user appears "kicked" every ~5 + // minutes. We renew well inside the TTL and do not touch the live stream. useEffect(() => { if (!sessionUrl) return; - const generation = requestGeneration.current; - const controlPath = `/workspaces/${workspaceId}/display/control`; - const renewMs = 120_000; // well under the 300s TTL - const timer = setInterval(async () => { - try { - const next = await api.post(`${controlPath}/acquire`, { - controller: "user", - ttl_seconds: 300, - }); - if (requestGeneration.current !== generation) return; - setControl(next); - if (next.session_url) latestSessionUrlRef.current = next.session_url; - } catch { - // Transient renewal failure (or another holder took over): the live - // stream keeps running on its existing connection; surfaced control - // state stays as-is until the next tick or an explicit re-acquire. - } - }, renewMs); + const timer = setInterval(() => { + void reacquireSession(); + }, 120_000); return () => clearInterval(timer); - }, [sessionUrl, workspaceId]); + }, [sessionUrl, reacquireSession]); const acquireControl = async () => { const generation = requestGeneration.current; @@ -272,7 +278,11 @@ export function DisplayTab({ workspaceId }: Props) { />
{sessionUrl ? ( - + ) : (
@@ -351,9 +361,11 @@ function DisplayControlBar({ function DesktopStream({ sessionUrl, latestSessionUrlRef, + reacquireSession, }: { sessionUrl: string; latestSessionUrlRef: { current: string | null }; + reacquireSession: () => Promise; }) { const containerRef = useRef(null); const rfbRef = useRef(null); @@ -376,13 +388,15 @@ function DesktopStream({ let retryTimer: ReturnType | null = null; const maxAttempts = 10; - async function connect() { + async function connect(reacquire = false) { setStreamError(null); try { + // On a reconnect, mint a fresh lease + token first — the original token + // is only ~300s, so a cached URL can be expired and would 401. The + // initial connect already holds a fresh token from acquireControl. + if (reacquire) await reacquireSession(); const mod = await import("@novnc/novnc"); if (cancelled || !containerRef.current) return; - // Use the freshest signed URL (kept current by the lease-renewal timer) - // so a reconnect after the original token's window still authenticates. const stream = displayWebSocketConnection(latestSessionUrlRef.current || sessionUrl); rfb = new mod.default(containerRef.current, stream.url, { wsProtocols: ["binary", `molecule-display-token.${stream.token}`], @@ -415,7 +429,7 @@ function DesktopStream({ attempts += 1; setStreamError(`Reconnecting to desktop… (attempt ${attempts})`); retryTimer = setTimeout(() => { - if (!cancelled) void connect(); + if (!cancelled) void connect(true); }, Math.min(1000 * attempts, 5000)); } else { setStreamError("Desktop stream disconnected."); @@ -434,7 +448,7 @@ function DesktopStream({ rfbRef.current = null; rfb?.disconnect(); }; - }, [sessionUrl]); + }, [sessionUrl, reacquireSession, latestSessionUrlRef]); useEffect(() => { const onPaste = (event: ClipboardEvent) => { -- 2.52.0 From 293bfb6abed0f2a1dc0523ff6440dbf526aefb76 Mon Sep 17 00:00:00 2001 From: hongming Date: Thu, 4 Jun 2026 06:14:07 +0000 Subject: [PATCH 4/5] test(canvas): assert reconnect dials with a freshly-minted token --- .../tabs/__tests__/DisplayTab.test.tsx | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/canvas/src/components/tabs/__tests__/DisplayTab.test.tsx b/canvas/src/components/tabs/__tests__/DisplayTab.test.tsx index 188053514..30b83ad99 100644 --- a/canvas/src/components/tabs/__tests__/DisplayTab.test.tsx +++ b/canvas/src/components/tabs/__tests__/DisplayTab.test.tsx @@ -414,12 +414,22 @@ describe("DisplayTab", () => { height: 1080, }) .mockResolvedValueOnce({ controller: "none" }); - mockPost.mockResolvedValue({ - controller: "user", - controlled_by: "admin-token", - expires_at: "2026-05-23T08:48:27Z", - session_url: "/workspaces/ws-display/display/session/websockify#token=signed", - }); + // Initial acquire returns token "signed"; the reconnect re-acquire mints a + // FRESH token "signed2" (the lock/token is only ~300s — reconnecting with a + // cached, possibly-expired token would 401 and never recover). + mockPost + .mockResolvedValueOnce({ + controller: "user", + controlled_by: "admin-token", + expires_at: "2026-05-23T08:48:27Z", + session_url: "/workspaces/ws-display/display/session/websockify#token=signed", + }) + .mockResolvedValue({ + controller: "user", + controlled_by: "admin-token", + expires_at: "2026-05-23T08:53:27Z", + session_url: "/workspaces/ws-display/display/session/websockify#token=signed2", + }); render(); await waitFor(() => { @@ -429,10 +439,11 @@ describe("DisplayTab", () => { await waitFor(() => { expect(rfbInstances.length).toBe(1); }); + expect(mockRFBConstructor.mock.calls[0][2].wsProtocols).toContain("molecule-display-token.signed"); // An idle/network drop closes the websocket uncleanly. The client must - // reconnect (using the freshest signed URL) instead of giving up — this is - // the "disconnects every ~5 min and stays dead" report. + // re-acquire a fresh token and reconnect instead of giving up — this is the + // "disconnects every ~5 min and stays dead" report. rfbInstances[0].dispatchEvent(new CustomEvent("disconnect", { detail: { clean: false } })); await waitFor( () => { @@ -440,6 +451,8 @@ describe("DisplayTab", () => { }, { timeout: 3000 }, ); + // Reconnect dialed with the FRESH token, not the stale original. + expect(mockRFBConstructor.mock.calls[1][2].wsProtocols).toContain("molecule-display-token.signed2"); // A clean disconnect (the user released control) must NOT reconnect. rfbInstances[1].dispatchEvent(new CustomEvent("disconnect", { detail: { clean: true } })); -- 2.52.0 From 4f26ed69c3cb44e19d288f705f1fd25727803d5d Mon Sep 17 00:00:00 2001 From: hongming Date: Thu, 4 Jun 2026 06:52:54 +0000 Subject: [PATCH 5/5] fix(canvas): stop requesting server-side resize (x11vnc rejects it; console spam) --- canvas/src/components/tabs/DisplayTab.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/canvas/src/components/tabs/DisplayTab.tsx b/canvas/src/components/tabs/DisplayTab.tsx index 11b200c6b..38b9a7c0d 100644 --- a/canvas/src/components/tabs/DisplayTab.tsx +++ b/canvas/src/components/tabs/DisplayTab.tsx @@ -403,7 +403,12 @@ function DesktopStream({ }); rfbRef.current = rfb; rfb.scaleViewport = true; - rfb.resizeSession = true; + // Do NOT request a server-side resize: the workspace display runs a + // fixed Xorg modeline and x11vnc rejects SetDesktopSize ("Resize is + // administratively prohibited"), which spams the console on every + // (re)connect. scaleViewport already fits the fixed framebuffer to the + // container client-side, so we don't need the server to resize. + rfb.resizeSession = false; rfb.focusOnClick = true; rfb.focus({ preventScroll: true }); rfb.addEventListener("connect", () => { -- 2.52.0