Merge b4c22b04dc into a3259400a3

browse/src/bun-polyfill.cjs

@@ -75,6 +75,9 @@ globalThis.Bun = {
       timeout: options.timeout,
       env: options.env,
       cwd: options.cwd,
+      // The browse server runs console-less on Windows; without this every
+      // subprocess (tasklist, git, icacls) pops a visible console window.
+      windowsHide: true,
     });
 
     return {
@@ -91,6 +94,10 @@ globalThis.Bun = {
       stdio,
       env: options.env,
       cwd: options.cwd,
+      detached: options.detached,
+      // Same as spawnSync: detached daemons (terminal-agent respawns) must
+      // not flash a console window on Windows.
+      windowsHide: true,
     });
 
     return {

browse/src/error-handling.ts

@@ -7,8 +7,6 @@
 
 import * as fs from 'fs';
 
-const IS_WINDOWS = process.platform === 'win32';
-
 // ─── Filesystem ────────────────────────────────────────────────
 
 /** Remove a file, ignoring ENOENT (already gone). Rethrows other errors. */
@@ -36,23 +34,26 @@ export function safeKill(pid: number, signal: NodeJS.Signals | number): void {
   }
 }
 
-/** Check if a PID is alive. Pure boolean probe — returns false for ALL errors. */
+/**
+ * Check if a PID is alive. Pure boolean probe — never throws.
+ *
+ * signal-0 on every platform: Node and Bun both implement
+ * `process.kill(pid, 0)` on Windows via an OpenProcess existence check —
+ * no child process, no console window. The previous Windows branch spawned
+ * `tasklist`, which (a) flashed a visible console window from console-less
+ * daemons on every call (#1952), and (b) silently returned false for LIVE
+ * processes when tasklist exceeded its 3s timeout — node's spawnSync
+ * reports timeout via `result.error` without throwing, so the empty-stdout
+ * path read as "dead". A false "dead" makes the terminal-agent watchdog
+ * respawn around a live agent and orphan it (split-brain, #2151).
+ *
+ * EPERM ⇒ alive: the process exists but can't be signalled by this user.
+ */
 export function isProcessAlive(pid: number): boolean {
-  if (IS_WINDOWS) {
-    try {
-      const result = Bun.spawnSync(
-        ['tasklist', '/FI', `PID eq ${pid}`, '/NH', '/FO', 'CSV'],
-        { stdout: 'pipe', stderr: 'pipe', timeout: 3000 }
-      );
-      return result.stdout.toString().includes(`"${pid}"`);
-    } catch {
-      return false;
-    }
-  }
   try {
     process.kill(pid, 0);
     return true;
-  } catch {
-    return false;
+  } catch (err: any) {
+    return err?.code === 'EPERM';
   }
 }

browse/src/server.ts

@@ -1504,9 +1504,9 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
   // (which would create split-brain — two agents writing the port file,
   // tokens diverging between them, mystery PTY upgrade failures).
   //
-  // Crash-loop guard: 3 respawn attempts inside 60s → stop trying and emit
-  // a one-line error. Manual `forceRestart` from the sidebar clears the
-  // history (the user is the explicit signal to retry).
+  // Crash-loop guard: 3 respawn attempts inside 3 ticks → stop trying and
+  // emit a one-line error. Manual `forceRestart` from the sidebar clears
+  // the history (the user is the explicit signal to retry).
   //
   // Only active when ownsTerminalAgent === true. Embedders that pre-launch
   // their own PTY server (gbrowser phoenix overlay) must not be auto-respawned
@@ -1517,8 +1517,11 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
     process.env.GSTACK_AGENT_WATCHDOG_TICK_MS || '60000',
     10,
   );
-  const RESPAWN_GUARD_WINDOW_MS = 60_000;
   const RESPAWN_GUARD_MAX = 3;
+  // The guard must span at least RESPAWN_GUARD_MAX ticks or it can never
+  // trip: respawn attempts land at most once per tick, so a 60s window
+  // over a 60s tick sees a single attempt forever (see #2151).
+  const RESPAWN_GUARD_WINDOW_MS = Math.max(60_000, AGENT_WATCHDOG_TICK_MS * RESPAWN_GUARD_MAX);
   let agentRespawnGuardTripped = false;
 
   if (ownsTerminalAgent) {

browse/test/bun-polyfill.test.ts

@@ -1,4 +1,5 @@
 import { describe, test, expect, afterAll } from 'bun:test';
+import * as fs from 'fs';
 import * as path from 'path';
 
 // Load the polyfill into a fresh object (don't clobber globalThis.Bun)
@@ -48,6 +49,23 @@ describe('bun-polyfill', () => {
     expect(lines[2]).toBe('HAS_UNREF');
   });
 
+  test('spawn and spawnSync pass windowsHide (no console flash from console-less daemons)', () => {
+    // Static tripwire (#2151): on Windows the browse server always runs
+    // under Node via this polyfill, and node's child_process defaults to
+    // windowsHide: false — so every subprocess the console-less daemon
+    // spawns (terminal-agent respawns, git, icacls) pops a visible console
+    // window. Both spawn paths must pass windowsHide: true, and spawn must
+    // forward `detached` for daemon children. Behavioral assertion isn't
+    // possible cross-platform (window creation is invisible to the child),
+    // hence source-level pinning.
+    const src = fs.readFileSync(polyfillPath, 'utf-8');
+    const spawnSyncBlock = src.slice(src.indexOf('spawnSync(cmd'), src.indexOf('spawn(cmd'));
+    const spawnBlock = src.slice(src.indexOf('spawn(cmd'));
+    expect(spawnSyncBlock).toContain('windowsHide: true');
+    expect(spawnBlock).toContain('windowsHide: true');
+    expect(spawnBlock).toContain('detached: options.detached');
+  });
+
   test('Bun.serve creates an HTTP server that responds', async () => {
     const result = Bun.spawnSync(['node', '-e', `
       require('${polyfillPath}');

browse/test/error-handling.test.ts

@@ -44,4 +44,28 @@ describe('isProcessAlive', () => {
   test('returns false for non-existent process', () => {
     expect(isProcessAlive(99999999)).toBe(false);
   });
+
+  test('returns true for a live but unsignallable process (EPERM ⇒ alive)', () => {
+    // PID 4 = System on Windows, PID 1 = init/launchd on POSIX. Both are
+    // always alive; signalling them as an unprivileged user throws EPERM,
+    // which must read as "alive" (as root/admin the probe just succeeds).
+    const protectedPid = process.platform === 'win32' ? 4 : 1;
+    expect(isProcessAlive(protectedPid)).toBe(true);
+  });
+
+  test('never spawns a subprocess (no tasklist on Windows)', () => {
+    // Static tripwire: the pre-#2151 Windows branch shelled out to
+    // `tasklist`, which flashed a console window from console-less daemons
+    // on every watchdog tick (#1952) and silently reported live agents as
+    // dead when tasklist exceeded its timeout — triggering split-brain
+    // respawns (#2151). signal-0 is a pure syscall on every platform.
+    const src = fs.readFileSync(
+      path.resolve(import.meta.dir, '..', 'src', 'error-handling.ts'),
+      'utf-8',
+    );
+    // Reject only actual invocations — the doc comment on isProcessAlive
+    // may mention tasklist when explaining what the signal-0 probe replaced.
+    expect(src).not.toMatch(/spawnSync\s*\(\s*\[?\s*['"]tasklist/);
+    expect(src).not.toMatch(/spawn\s*\(\s*\[?\s*['"]tasklist/);
+  });
 });

browse/test/terminal-agent-watchdog.test.ts

@@ -50,7 +50,12 @@ describe('terminal-agent watchdog (v1.44+)', () => {
   test('4. crash-loop guard with rolling window', () => {
     const src = fs.readFileSync(SERVER_TS, 'utf-8');
     const block = sliceBetween(src, '─── Terminal-Agent Watchdog', 'Factory-scoped validateAuth');
-    expect(block).toContain('RESPAWN_GUARD_WINDOW_MS = 60_000');
+    // The window must span at least RESPAWN_GUARD_MAX ticks. Respawn
+    // attempts land at most once per tick, so a fixed 60s window over a
+    // 60s tick would see a single attempt forever and never trip (#2151).
+    expect(block).toContain(
+      'RESPAWN_GUARD_WINDOW_MS = Math.max(60_000, AGENT_WATCHDOG_TICK_MS * RESPAWN_GUARD_MAX)',
+    );
     expect(block).toContain('RESPAWN_GUARD_MAX = 3');
     expect(block).toContain('respawnHistory');
     expect(block).toContain('agentRespawnGuardTripped');