From 00117206d9d953f015b1a3dba21e9d10abece268 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Padez?= <andre.padez@gmail.com>
Date: Sat, 1 Aug 2026 23:39:52 +0000
Subject: [PATCH] vnc: reclaim port 5900 before mirroring, and do not trust a
 foreign listener
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Re-applies to the mirror what f22c667 did for Xvnc, which I dropped by restoring vnc-manager from an
older commit. The bug is in the lifecycle, not the server, so it came straight back: the running mirror
lives in module state, a sidecar restart forgets it while the process keeps running, and waitForPort
accepted ANY listener on 5900 as proof of a healthy start.

It bit immediately. An Xvnc left over from the virtual-desktop experiment kept hold of 5900, so the
restarted sidecar reported a healthy mirror while the browser was being served the stale XFCE session
underneath it — which read as "the revert did not work".

reclaimPort frees the port before spawning (TERM, then KILL after two seconds) and waitForPort now also
fails when the process we spawned has exited.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 src/servers/sidecar/vnc/vnc-manager.ts | 32 +++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/src/servers/sidecar/vnc/vnc-manager.ts b/src/servers/sidecar/vnc/vnc-manager.ts
index 372d93a3..c23af54b 100644
--- a/src/servers/sidecar/vnc/vnc-manager.ts
+++ b/src/servers/sidecar/vnc/vnc-manager.ts
@@ -169,10 +169,33 @@ async function isPortOpen(port: number): Promise<boolean> {
   }
 }
 
-// x11vnc stays in the foreground, so readiness is the listening port rather than exit code.
-async function waitForPort(port: number): Promise<boolean> {
+// A listener on the port is NOT proof our server started. The running mirror lives in module state, so
+// a sidecar restart forgets it while the process keeps running orphaned — and then this check sees the
+// ORPHAN listening and reports success, leaving the platform convinced it started something the browser
+// is not looking at. That is not hypothetical: on 2026-08-01 an Xvnc left over from the virtual-desktop
+// experiment held 5900, and the mirror reported healthy while the browser was served a stale XFCE
+// session. So reclaim the port first, and treat our own process dying as failure.
+async function reclaimPort(port: number): Promise<void> {
+  if (!(await isPortOpen(port))) return;
+
+  console.log(`[vnc] port ${port} already held — killing the orphan before starting`);
+  Bun.spawnSync({ cmd: ['fuser', '-k', '-TERM', `${port}/tcp`], stdout: 'ignore', stderr: 'ignore' });
+
+  for (let i = 0; i < 20; i++) {
+    await Bun.sleep(100);
+    if (!(await isPortOpen(port))) return;
+  }
+
+  Bun.spawnSync({ cmd: ['fuser', '-k', '-KILL', `${port}/tcp`], stdout: 'ignore', stderr: 'ignore' });
+  await Bun.sleep(300);
+}
+
+// x11vnc stays in the foreground, so readiness is the listening port rather than exit code — but only
+// once we know the process we spawned is the one alive to own it.
+async function waitForPort(port: number, proc: { exitCode: number | null }): Promise<boolean> {
   const deadline = Date.now() + READY_TIMEOUT_MS;
   while (Date.now() < deadline) {
+    if (proc.exitCode !== null) return false;
     if (await isPortOpen(port)) return true;
     await Bun.sleep(100);
   }
@@ -188,6 +211,9 @@ export async function startSession(params: VncStartParams): Promise<{ port: numb
   const homeDir = getOwnerHomeDir(params.email);
   const { passwdFile } = await ensureVncPassword(homeDir);
 
+  // An orphan from a previous sidecar life would still hold the port and be mistaken for a healthy start.
+  await reclaimPort(MIRROR_PORT);
+
   const displayNum = resolveDisplayNum();
   const display = `:${displayNum}`;
 
@@ -265,7 +291,7 @@ export async function startSession(params: VncStartParams): Promise<{ port: numb
     }
   })();
 
-  if (!(await waitForPort(MIRROR_PORT))) {
+  if (!(await waitForPort(MIRROR_PORT, proc))) {
     try {
       proc.kill();
     } catch {