From f48623ecac40bb5403930d249e65152474f1d72c Mon Sep 17 00:00:00 2001
From: Oliver Southgate <osouthgate@gmail.com>
Date: Thu, 2 Jul 2026 21:23:39 +0100
Subject: [PATCH] fix: isProcessAlive uses signal-0 on all platforms, not
 tasklist
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The Windows branch spawned tasklist per probe. Two failure modes:
(1) every call flashed a console window from console-less daemons —
the terminal-agent watchdog probes once per 60s tick for the whole
session (#1952); (2) node spawnSync reports a timeout via result.error
without throwing, so a slow tasklist (>3s under load) read as empty
stdout -> "dead" for a LIVE agent. A false "dead" makes the watchdog
clear the live agent record and respawn a duplicate around it, while
killAgentByRecord consults the same false probe and skips the kill —
the split-brain observed in #2151.

process.kill(pid, 0) is a pure OpenProcess existence check on Windows
under both Node and Bun (verified on Windows 11, node 22 / bun 1.3.14:
alive -> true, dead -> false, PID 4 System -> EPERM -> true). Unifying
on signal-0 also fixes the old Unix branch collapsing EPERM
(exists-but-unsignallable) to "dead", as proposed in #1952.

Tests: EPERM-implies-alive case, plus a static tripwire rejecting any
tasklist invocation in error-handling.ts.

Fixes #1952. Fixes the split-brain trigger in #2151.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 browse/src/error-handling.ts       | 33 +++++++++++++++---------------
 browse/test/error-handling.test.ts | 24 ++++++++++++++++++++++
 2 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/browse/src/error-handling.ts b/browse/src/error-handling.ts
index 2c4e271e8..c824ee094 100644
--- a/browse/src/error-handling.ts
+++ b/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';
   }
 }
diff --git a/browse/test/error-handling.test.ts b/browse/test/error-handling.test.ts
index b25d98805..b1c31bc26 100644
--- a/browse/test/error-handling.test.ts
+++ b/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/);
+  });
 });