Merge 00303ebf86 into a3259400a3

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,24 @@ 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.
+ *
+ * Uses signal-0 on every platform. Node/Bun implement `process.kill(pid, 0)`
+ * on Windows via `OpenProcess` — a pure existence check that spawns no child
+ * process and opens no console window. The old Windows branch shelled out to
+ * `tasklist`, which Windows gives its own console window; on the terminal-agent
+ * watchdog's per-tick existence check that flashed a `conhost.exe` window every
+ * 60s for the whole session (#1952). The parent watchdog already uses signal-0
+ * directly; this unifies on it.
+ */
 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) {
+    // ESRCH → gone (false); EPERM → exists but not signallable → still alive.
+    // The old Unix branch collapsed every error to false, missing the EPERM edge.
+    return err?.code === 'EPERM';
   }
 }

browse/test/error-handling.test.ts

@@ -2,6 +2,7 @@ import { describe, test, expect } from 'bun:test';
 import * as fs from 'fs';
 import * as os from 'os';
 import * as path from 'path';
+import { fileURLToPath } from 'url';
 import { safeUnlink, safeKill, isProcessAlive } from '../src/error-handling';
 
 describe('safeUnlink', () => {
@@ -44,4 +45,36 @@ describe('isProcessAlive', () => {
   test('returns false for non-existent process', () => {
     expect(isProcessAlive(99999999)).toBe(false);
   });
+
+  test('treats a live-but-EPERM process as alive (#1952)', () => {
+    // PID 1 (init/launchd) exists but a non-root user can't signal it →
+    // process.kill(1, 0) throws EPERM, which means alive, not dead.
+    // (Skip when running as root, where signalling PID 1 is permitted.)
+    if (typeof process.getuid === 'function' && process.getuid() === 0) return;
+    let threwEPERM = false;
+    try {
+      process.kill(1, 0);
+    } catch (err: any) {
+      threwEPERM = err?.code === 'EPERM';
+    }
+    if (threwEPERM) expect(isProcessAlive(1)).toBe(true);
+  });
+
+  // Regression tripwire for #1952: isProcessAlive must never spawn a child
+  // process to probe a PID. The Windows branch used to shell out to `tasklist`,
+  // which Windows gives its own console window — flashing a conhost.exe window
+  // every watchdog tick (default 60s) for the whole session. signal-0 is a pure
+  // syscall on all platforms (OpenProcess on Windows), so the source must not
+  // reintroduce tasklist or a spawn in this probe.
+  test('isProcessAlive source spawns nothing (no Windows console flash, #1952)', () => {
+    const src = fs.readFileSync(
+      path.join(path.dirname(fileURLToPath(import.meta.url)), '..', 'src', 'error-handling.ts'),
+      'utf-8'
+    );
+    const start = src.indexOf('export function isProcessAlive');
+    expect(start).toBeGreaterThan(-1);
+    const body = src.slice(start, src.indexOf('\n}', start));
+    expect(body).not.toMatch(/tasklist/i);
+    expect(body).not.toMatch(/spawnSync|spawn\(/);
+  });
 });