Merge branch 'sidecars-vnc' into sidecars

src/servers/api/desktop/rest.ts

@@ -1,19 +1,19 @@
 import { createRouter } from '../../create-router';
-import { getVncPassword } from './vnc-config';
 import * as sidecar from '@@/sidecar-registry';
 
 export const desktopRouter = createRouter();
 
 // The desktop UI asks for the password before it can open the WebSocket — and that WebSocket is what
 // starts the VNC session. So this cannot wait for a session to exist: on a fresh install nothing has
-// ever written the password, and answering "not configured" deadlocked the page permanently. Ask the
+// ever written the password, and answering "not configured" deadlocked the page permanently.
-// sidecar to provision it instead; it owns the .vnc directory and the call is idempotent.
+//
+// Officer does not read the password file itself. It lives in the owner's ~/.vnc, next to the rfbauth
+// file x11vnc authenticates against, and the sidecar is the process that writes both — so it is the
+// process that answers for them too. `vnc:ensure-password` is idempotent: it returns the existing
+// pair when both files are already there, and provisions them when they are not.
 desktopRouter.get('/vnc-password', async (ctx) => {
   const user = ctx.get('user');
 
-  const existing = await getVncPassword(user.email);
-  if (existing) return ctx.json({ password: existing });
-
   if (!sidecar.isVncConnected()) {
     return ctx.json({ error: 'VNC sidecar is not connected' }, 503);
   }

src/servers/api/desktop/vnc-config.ts

@@ -1,10 +0,0 @@
-import { join } from 'node:path';
-import { getOwnerHomeDir } from '@@/data-path';
-
-const getVncDir = (email: string): string => join(getOwnerHomeDir(email), '.vnc');
-
-export async function getVncPassword(email: string): Promise<string | null> {
-  const file = Bun.file(join(getVncDir(email), 'password'));
-  if (!(await file.exists())) return null;
-  return (await file.text()).trim();
-}

src/workspaces/officerdev/src/apps/Desktop/DesktopView.tsx

@@ -41,7 +41,9 @@ export const DesktopView = ({ className, style }: DesktopViewProps) => {
   const rfbRef = useRef<RFBInstance | null>(null);
   const isMounted = useMounted();
   const client = useClient();
-  const [status, setStatus] = useState<'connecting' | 'connected' | 'disconnected' | 'error'>('connecting');
+  const [status, setStatus] = useState<'connecting' | 'connected' | 'reconnecting' | 'disconnected' | 'error'>(
+    'connecting',
+  );
   const [errorMsg, setErrorMsg] = useState('');
 
   useEffect(() => {
@@ -50,20 +52,64 @@ export const DesktopView = ({ className, style }: DesktopViewProps) => {
     if (!container) return;
 
     let disposed = false;
+    let attempts = 0;
+    let retryTimer: ReturnType<typeof setTimeout> | null = null;
+    // Tearing an RFB down makes it fire its own `disconnect`, and a bad password fires `securityfailure`
+    // and then `disconnect` too. Both would otherwise be read as "officer went away, reattach".
+    let generation = 0;
+    let fatal = false;
+    const MAX_ATTEMPTS = 5;
+    const RETRY_DELAYS = [1000, 2000, 3000, 5000, 5000];
+
+    const detach = () => {
+      const rfb = rfbRef.current;
+      rfbRef.current = null;
+      generation++;
+      if (!rfb) return;
+      try {
+        rfb.disconnect();
+      } catch {
+        /* ignore */
+      }
+    };
+
+    // x11vnc mirrors :0 with `-forever`, so the desktop itself outlives this socket — losing it means
+    // officer restarted under us, not that the session ended. Reattach instead of parking on
+    // "Disconnected" until someone reopens the panel.
+    const retry = (reason: string) => {
+      if (disposed || fatal || retryTimer) return;
+      detach();
+      if (attempts >= MAX_ATTEMPTS) {
+        setStatus('disconnected');
+        setErrorMsg(reason);
+        return;
+      }
+      const delay = RETRY_DELAYS[attempts] ?? 5000;
+      attempts++;
+      setStatus('reconnecting');
+      setErrorMsg(`${reason} — reconnecting (${attempts}/${MAX_ATTEMPTS})...`);
+      retryTimer = setTimeout(() => {
+        retryTimer = null;
+        void connect();
+      }, delay);
+    };
 
     const connect = async () => {
+      const gen = ++generation;
+      const isCurrent = () => !disposed && gen === generation;
       let password = '';
       try {
         const res = await client.get<{ password: string }>('/desktop/vnc-password');
         password = res.password;
       } catch {
-        if (disposed) return;
+        if (!isCurrent()) return;
-        setStatus('error');
+        // Officer being down is the common case here, and it comes back — so this is a retry, not a
-        setErrorMsg('Failed to fetch VNC password');
+        // dead end. A sidecar that is genuinely missing still ends up at "Disconnected" after five.
+        retry('Failed to fetch VNC password');
         return;
       }
 
-      if (disposed) return;
+      if (!isCurrent()) return;
 
       let RFB: Awaited<ReturnType<typeof loadRFB>>['default'];
       try {
@@ -71,13 +117,14 @@ export const DesktopView = ({ className, style }: DesktopViewProps) => {
         RFB = mod.default;
       } catch (err) {
         console.error('[desktop] Failed to load noVNC:', err);
-        if (disposed) return;
+        if (!isCurrent()) return;
+        fatal = true;
         setStatus('error');
         setErrorMsg('Failed to load noVNC library');
         return;
       }
 
-      if (disposed) return;
+      if (!isCurrent()) return;
 
       const wsUrl = buildWsUrl();
       const rfb = new RFB(container, wsUrl, {
@@ -90,15 +137,18 @@ export const DesktopView = ({ className, style }: DesktopViewProps) => {
       rfbRef.current = rfb;
 
       rfb.addEventListener('connect', () => {
-        if (!disposed) setStatus('connected');
+        if (!isCurrent()) return;
+        attempts = 0;
+        setErrorMsg('');
+        setStatus('connected');
       });
 
-      rfb.addEventListener('disconnect', (ev: CustomEvent) => {
+      // Every disconnect we did not ask for is worth retrying, clean or not: officer closing its side
-        if (disposed) return;
+      // tidily during a restart still reports `clean`, and the desktop behind it is still there.
-        setStatus('disconnected');
+      rfb.addEventListener('disconnect', () => {
-        if (!ev.detail.clean) {
+        if (!isCurrent()) return;
-          setErrorMsg('Connection lost');
+        rfbRef.current = null;
-        }
+        retry('Connection lost');
       });
 
       rfb.addEventListener('credentialsrequired', () => {
@@ -106,25 +156,30 @@ export const DesktopView = ({ className, style }: DesktopViewProps) => {
       });
 
       rfb.addEventListener('securityfailure', (ev: CustomEvent) => {
-        if (!disposed) {
+        if (!isCurrent()) return;
-          setStatus('error');
+        fatal = true;
-          setErrorMsg(ev.detail.reason || 'Security failure');
+        setStatus('error');
-        }
+        setErrorMsg(ev.detail.reason || 'Security failure');
       });
     };
 
+    // Coming back to the tab after the retries ran out should try once more rather than stay dead.
+    const handleVisibility = () => {
+      if (disposed || fatal || document.visibilityState !== 'visible') return;
+      if (rfbRef.current || retryTimer) return;
+      attempts = 0;
+      setStatus('connecting');
+      void connect();
+    };
+    document.addEventListener('visibilitychange', handleVisibility);
+
     void connect();
 
     return () => {
       disposed = true;
-      if (rfbRef.current) {
+      document.removeEventListener('visibilitychange', handleVisibility);
-        try {
+      if (retryTimer) clearTimeout(retryTimer);
-          rfbRef.current.disconnect();
+      detach();
-        } catch {
-          /* ignore */
-        }
-        rfbRef.current = null;
-      }
     };
   }, [isMounted, client]);
 
@@ -138,7 +193,7 @@ export const DesktopView = ({ className, style }: DesktopViewProps) => {
           Connecting to desktop...
         </div>
       )}
-      {(status === 'disconnected' || status === 'error') && (
+      {(status === 'reconnecting' || status === 'disconnected' || status === 'error') && (
         <div className="absolute inset-0 flex items-center justify-center text-sm text-muted-foreground">
           {errorMsg || 'Disconnected from desktop'}
         </div>