Fix Windows terminal-agent leak: one orphaned process + console window per watchdog tick

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,39 @@ 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 map `process.kill(pid, 0)` to
+ * an OpenProcess existence check on Windows, so the POSIX idiom is portable
+ * here — no shell-out needed.
+ *
+ * Windows used to shell out to `tasklist /FI "PID eq <pid>"` and string-match
+ * the CSV. That was wrong in two ways, both of which bit in production:
+ *
+ *   1. FALSE NEGATIVES UNDER LOAD. `tasklist` takes ~700-1700ms on an idle
+ *      Windows box and far longer under memory pressure. A Bun.spawnSync that
+ *      hits its `timeout` still RETURNS, carrying partial stdout — so the
+ *      `.includes()` match came back false and a LIVE process was reported
+ *      dead. Callers (killAgentByRecord, the terminal-agent watchdog) then
+ *      skipped the kill and respawned around the survivor, leaking one
+ *      terminal-agent per watchdog tick. The leak was self-reinforcing: every
+ *      orphan added memory pressure, which made the next tasklist slower,
+ *      which produced the next false negative.
+ *   2. A VISIBLE CONSOLE WINDOW per probe (no windowsHide), so a background
+ *      watchdog strobed a terminal into the foreground every 60 seconds.
+ *
+ * Signal 0 is ~74,000x faster (0.004ms vs 270ms, measured), spawns nothing,
+ * and cannot time out.
+ *
+ * EPERM means the process EXISTS but we lack rights to signal it. That is
+ * alive; returning false there would reintroduce failure mode 1.
+ */
 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

@@ -1517,8 +1517,18 @@ 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 window MUST span enough ticks for RESPAWN_GUARD_MAX respawns to
+  // land inside it. This was a fixed 60_000 against a 60_000 tick, so at most
+  // ONE respawn could ever be in the window and `respawnHistory.length >= 3`
+  // was unreachable — the guard could not fire at the default tick rate, and a
+  // steady one-per-tick leak ran unbounded instead of stopping after 3. Scale
+  // with the tick so the intent ("3 crashes in quick succession → stop") holds
+  // at any tick value: 3 respawns within 5 ticks trips it.
+  const RESPAWN_GUARD_WINDOW_MS = Math.max(
+    60_000,
+    AGENT_WATCHDOG_TICK_MS * (RESPAWN_GUARD_MAX + 2),
+  );
   let agentRespawnGuardTripped = false;
 
   if (ownsTerminalAgent) {

browse/src/terminal-agent-control.ts

@@ -77,6 +77,11 @@ export function spawnTerminalAgent(opts: {
       ...(opts.extraEnv || {}),
     },
     stdio: ['ignore', 'ignore', 'ignore'],
+    // Without this, Windows allocates a console for the child and pops it to
+    // the FOREGROUND, stealing focus from whatever the user is typing into.
+    // The agent is a background daemon with all three stdio streams already
+    // ignored, so it has nothing to show. Harmless no-op on macOS/Linux.
+    windowsHide: true,
   });
   proc.unref?.();
   return proc.pid ?? null;

browse/test/cdp-session-cleanup.test.ts

@@ -18,7 +18,7 @@ import { withCdpSession, getOrCreateCdpSession } from '../src/cdp-bridge';
 // browse/test/server-sanitize-surrogates.test.ts: read source files
 // directly, assert an invariant on their contents.
 
-const SRC_DIR = path.resolve(new URL(import.meta.url).pathname, '..', '..', 'src');
+const SRC_DIR = path.resolve(import.meta.path, '..', '..', 'src');
 
 function readAllSourceFiles(): Array<{ file: string; content: string }> {
   const out: Array<{ file: string; content: string }> = [];

browse/test/cli-supervisor.test.ts

@@ -15,7 +15,7 @@ import * as path from 'path';
 // 3-8s each). These tripwires defend the load-bearing invariants:
 // opt-in by default, signal handlers wired, crash-loop guard, env knobs.
 
-const CLI_TS = path.resolve(new URL(import.meta.url).pathname, '..', '..', 'src', 'cli.ts');
+const CLI_TS = path.resolve(import.meta.path, '..', '..', 'src', 'cli.ts');
 
 describe('CLI outer supervisor (v1.44+)', () => {
   test('1. supervisor is opt-in via --supervise flag or BROWSE_SUPERVISE env', () => {

browse/test/process-liveness-windows.test.ts

@@ -0,0 +1,134 @@
+import { describe, test, expect } from 'bun:test';
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+import { isProcessAlive } from '../src/error-handling';
+import { spawnTerminalAgent } from '../src/terminal-agent-control';
+
+// REGRESSION TEST for the Windows terminal-agent leak.
+//
+// Symptom (reported on Windows 11, 48GB box under a heavy parallel build):
+// a console window popped to the foreground every 60 seconds, and orphaned
+// `bun run terminal-agent.ts` processes accumulated at one per minute until
+// the machine ran out of committable memory.
+//
+// Root cause was a three-bug chain, each of which this file pins:
+//
+//   1. `isProcessAlive` shelled out to `tasklist` on Windows with a 3s
+//      timeout. A Bun.spawnSync that hits its timeout STILL RETURNS, carrying
+//      partial stdout — so the `.includes()` PID match came back false and a
+//      LIVE agent was reported dead. Measured tasklist latency was 700-1700ms
+//      idle, and far worse under memory pressure, so the timeout was reachable
+//      in ordinary use.
+//   2. That false negative made `killAgentByRecord` skip the kill (it
+//      validates liveness first) while the watchdog respawned anyway —
+//      leaking the survivor. Each orphan added memory pressure, slowing the
+//      next tasklist, producing the next false negative. Self-reinforcing.
+//   3. Neither the tasklist probe nor the agent spawn passed `windowsHide`,
+//      so every tick allocated a visible console and stole focus.
+//
+// The guard-window arithmetic bug that let this run unbounded instead of
+// tripping the crash-loop guard is pinned separately, in test 6.
+
+const SRC_DIR = path.resolve(import.meta.dir, '..', 'src');
+
+function readAllSourceFiles(): Array<{ file: string; content: string }> {
+  return fs
+    .readdirSync(SRC_DIR)
+    .filter((e) => e.endsWith('.ts'))
+    .map((e) => ({ file: e, content: fs.readFileSync(path.join(SRC_DIR, e), 'utf-8') }));
+}
+
+/** Strip line and block comments so static greps only see real code. */
+function stripComments(src: string): string {
+  return src.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^\s*\/\/.*$/gm, '');
+}
+
+describe('process liveness probe (Windows terminal-agent leak)', () => {
+  test('1. isProcessAlive reports the current process alive', () => {
+    expect(isProcessAlive(process.pid)).toBe(true);
+  });
+
+  test('2. isProcessAlive reports an unused PID dead', () => {
+    // Below Linux PID_MAX_LIMIT, far above any realistic Windows/macOS PID.
+    expect(isProcessAlive(2147483646)).toBe(false);
+  });
+
+  test('3. isProcessAlive spawns NO subprocess', () => {
+    // The heart of the bug: a liveness probe that forks is slow enough to
+    // time out, and a timed-out probe silently answers "dead". Signal 0
+    // cannot time out because it never leaves the process.
+    const origSpawn = (Bun as any).spawn;
+    const origSpawnSync = (Bun as any).spawnSync;
+    const spawns: string[] = [];
+    (Bun as any).spawn = (...args: any[]) => { spawns.push(`spawn:${JSON.stringify(args[0])}`); return origSpawn(...args); };
+    (Bun as any).spawnSync = (...args: any[]) => { spawns.push(`spawnSync:${JSON.stringify(args[0])}`); return origSpawnSync(...args); };
+    try {
+      isProcessAlive(process.pid);
+      isProcessAlive(2147483646);
+      expect(spawns).toEqual([]);
+    } finally {
+      (Bun as any).spawn = origSpawn;
+      (Bun as any).spawnSync = origSpawnSync;
+    }
+  });
+
+  test('4. no source file probes liveness via tasklist', () => {
+    // Static tripwire: re-introducing a tasklist-based existence check
+    // anywhere in src/ resurrects the false-negative class.
+    const offenders: string[] = [];
+    for (const { file, content } of readAllSourceFiles()) {
+      const code = stripComments(content);
+      // `PID eq` is the existence-probe form specifically. Other tasklist
+      // uses (e.g. IMAGENAME filters for browser detection) are unaffected.
+      if (/tasklist/.test(code) && /PID eq/.test(code)) offenders.push(file);
+    }
+    expect(offenders).toEqual([]);
+  });
+
+  test('5. spawnTerminalAgent passes windowsHide so no console is shown', () => {
+    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-hide-'));
+    const script = path.join(tmpDir, 'fake-agent.ts');
+    fs.writeFileSync(script, '// no-op\n');
+    const origSpawn = (Bun as any).spawn;
+    let captured: any = null;
+    (Bun as any).spawn = (_cmd: any, opts: any) => {
+      captured = opts;
+      return { pid: 4242, unref() {} };
+    };
+    try {
+      const pid = spawnTerminalAgent({
+        stateFile: path.join(tmpDir, 'state.json'),
+        serverPort: 12345,
+        cwd: tmpDir,
+        scriptPath: script,
+      });
+      expect(pid).toBe(4242);
+      expect(captured).not.toBeNull();
+      expect(captured.windowsHide).toBe(true);
+      // Detached background daemon — must not inherit a terminal either.
+      expect(captured.stdio).toEqual(['ignore', 'ignore', 'ignore']);
+    } finally {
+      (Bun as any).spawn = origSpawn;
+      fs.rmSync(tmpDir, { recursive: true, force: true });
+    }
+  });
+
+  test('6. respawn guard window spans enough ticks for the guard to fire', () => {
+    // The guard was `RESPAWN_GUARD_WINDOW_MS = 60_000` against a 60_000ms
+    // tick, allowing at most ONE respawn in the window — so the
+    // `>= RESPAWN_GUARD_MAX (3)` trip condition was unreachable and a steady
+    // one-per-tick leak never self-limited. Assert the window is derived from
+    // the tick rather than fixed.
+    const src = fs.readFileSync(path.join(SRC_DIR, 'server.ts'), 'utf-8');
+    const match = src.match(/const RESPAWN_GUARD_WINDOW_MS =([\s\S]{0,160}?);/);
+    expect(match).not.toBeNull();
+    expect(match![1]).toContain('AGENT_WATCHDOG_TICK_MS');
+
+    // Pin the arithmetic itself: at the default tick, three respawns must fit.
+    const tick = 60_000;
+    const guardMax = 3;
+    const windowMs = Math.max(60_000, tick * (guardMax + 2));
+    expect(windowMs).toBeGreaterThanOrEqual(tick * guardMax);
+  });
+});

browse/test/server-embedder-terminal-port.test.ts

@@ -217,7 +217,7 @@ describe('buildFetchHandler ownsTerminalAgent gate', () => {
     // Resolves browse/src/server.ts relative to this test file so the test
     // works regardless of cwd. import.meta.url is the test file's URL.
     const serverTsPath = path.resolve(
-      new URL(import.meta.url).pathname,
+      import.meta.path,
       '..',
       '..',
       'src',

browse/test/server-pty-lease-routes.test.ts

@@ -7,7 +7,7 @@ import * as path from 'path';
 // loopback to be live (e2e-tier); these static-grep tripwires pin the
 // load-bearing protocol invariants.
 
-const SERVER_TS = path.resolve(new URL(import.meta.url).pathname, '..', '..', 'src', 'server.ts');
+const SERVER_TS = path.resolve(import.meta.path, '..', '..', 'src', 'server.ts');
 
 describe('server: PTY lease routes (v1.44+ Commit 2)', () => {
   test('1. /pty-session returns the 4-tuple shape (sessionId, attachToken, leaseExpiresAt)', () => {

browse/test/sidepanel-patient-autoconnect.test.ts

@@ -12,7 +12,7 @@ import * as path from 'path';
 // explicit unrecoverable signals (401 auth invalid).
 
 const CLIENT_JS = path.resolve(
-  new URL(import.meta.url).pathname,
+  import.meta.path,
   '..',
   '..',
   '..',

browse/test/sidepanel-reattach.test.ts

@@ -13,7 +13,7 @@ import * as path from 'path';
 // in the e2e tier.
 
 const TERMINAL_JS = path.resolve(
-  new URL(import.meta.url).pathname, '..', '..', '..', 'extension', 'sidepanel-terminal.js',
+  import.meta.path, '..', '..', '..', 'extension', 'sidepanel-terminal.js',
 );
 
 describe('sidepanel re-attach loop (v1.44+ Commit 3)', () => {

browse/test/sidepanel-restart-dispose.test.ts

@@ -16,10 +16,10 @@ import * as path from 'path';
 // doesn't leak a 60s-zombie claude.
 
 const TERMINAL_JS = path.resolve(
-  new URL(import.meta.url).pathname, '..', '..', '..', 'extension', 'sidepanel-terminal.js',
+  import.meta.path, '..', '..', '..', 'extension', 'sidepanel-terminal.js',
 );
 const SIDEPANEL_JS = path.resolve(
-  new URL(import.meta.url).pathname, '..', '..', '..', 'extension', 'sidepanel.js',
+  import.meta.path, '..', '..', '..', 'extension', 'sidepanel.js',
 );
 
 describe('sidepanel-terminal: forceRestart via /pty-restart (v1.44+)', () => {

browse/test/terminal-agent-detach-reattach.test.ts

@@ -10,7 +10,7 @@ import * as path from 'path';
 // in the e2e tier; these static-grep tripwires defend the load-bearing
 // protocol + correctness properties.
 
-const AGENT_TS = path.resolve(new URL(import.meta.url).pathname, '..', '..', 'src', 'terminal-agent.ts');
+const AGENT_TS = path.resolve(import.meta.path, '..', '..', 'src', 'terminal-agent.ts');
 
 describe('terminal-agent detach + re-attach (v1.44+ Commit 3)', () => {
   test('1. PtySession carries ring buffer + alt-screen + detach state', () => {

browse/test/terminal-agent-internal-handler.test.ts

@@ -12,7 +12,7 @@ import * as path from 'path';
 // (token grant/revoke behavior) already live in
 // browse/test/terminal-agent-integration.test.ts.
 
-const AGENT_TS = path.resolve(new URL(import.meta.url).pathname, '..', '..', 'src', 'terminal-agent.ts');
+const AGENT_TS = path.resolve(import.meta.path, '..', '..', 'src', 'terminal-agent.ts');
 
 describe('terminal-agent internalHandler refactor (v1.44+)', () => {
   test('1. internalHandler<T> exists with the documented signature', () => {

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

@@ -11,8 +11,8 @@ import * as path from 'path';
 // regressed by a refactor. These tests fail CI if either side stops sending
 // or stops accepting the protocol frames.
 
-const AGENT_TS = path.resolve(new URL(import.meta.url).pathname, '..', '..', 'src', 'terminal-agent.ts');
-const CLIENT_JS = path.resolve(new URL(import.meta.url).pathname, '..', '..', '..', 'extension', 'sidepanel-terminal.js');
+const AGENT_TS = path.resolve(import.meta.path, '..', '..', 'src', 'terminal-agent.ts');
+const CLIENT_JS = path.resolve(import.meta.path, '..', '..', '..', 'extension', 'sidepanel-terminal.js');
 
 describe('terminal-agent WS keepalive (v1.44+)', () => {
   test('1. agent has a KEEPALIVE_INTERVAL_MS env knob, default 25000', () => {

browse/test/terminal-agent-pid-identity.test.ts

@@ -30,7 +30,7 @@ import {
 // and browse/test/server-sanitize-surrogates.test.ts: read source files
 // directly, assert an invariant on their contents.
 
-const SRC_DIR = path.resolve(new URL(import.meta.url).pathname, '..', '..', 'src');
+const SRC_DIR = path.resolve(import.meta.path, '..', '..', 'src');
 
 function readAllSourceFiles(): Array<{ file: string; content: string }> {
   const out: Array<{ file: string; content: string }> = [];

browse/test/terminal-agent-session-routing.test.ts

@@ -13,7 +13,7 @@ import * as path from 'path';
 //   - {type:"start"} triggers spawn for eager UX after forceRestart
 //   - maybeSpawnPty helper is the single entry point for both spawn paths
 
-const AGENT_TS = path.resolve(new URL(import.meta.url).pathname, '..', '..', 'src', 'terminal-agent.ts');
+const AGENT_TS = path.resolve(import.meta.path, '..', '..', 'src', 'terminal-agent.ts');
 
 describe('terminal-agent session routing (v1.44+ Commit 2)', () => {
   test('1. validTokens is a Map binding token → sessionId', () => {

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

@@ -10,8 +10,8 @@ import * as path from 'path';
 // load-bearing properties: identity-based liveness check (not name match),
 // crash-loop guard, gated on ownsTerminalAgent, and cleared on shutdown.
 
-const SERVER_TS = path.resolve(new URL(import.meta.url).pathname, '..', '..', 'src', 'server.ts');
-const CONTROL_TS = path.resolve(new URL(import.meta.url).pathname, '..', '..', 'src', 'terminal-agent-control.ts');
+const SERVER_TS = path.resolve(import.meta.path, '..', '..', 'src', 'server.ts');
+const CONTROL_TS = path.resolve(import.meta.path, '..', '..', 'src', 'terminal-agent-control.ts');
 
 describe('terminal-agent watchdog (v1.44+)', () => {
   test('1. spawnTerminalAgent helper exists with PID return type', () => {
@@ -50,7 +50,13 @@ 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 be derived from the tick, not a fixed 60_000. It was
+    // hardcoded to 60_000 against a 60_000ms tick, so at most ONE respawn
+    // could ever sit inside the window and the `>= RESPAWN_GUARD_MAX` trip
+    // was unreachable — a steady one-respawn-per-tick leak ran unbounded
+    // instead of self-limiting after 3. Pinning the literal is what let that
+    // ship, so pin the relationship instead.
+    expect(block).toMatch(/RESPAWN_GUARD_WINDOW_MS =[\s\S]{0,200}AGENT_WATCHDOG_TICK_MS/);
     expect(block).toContain('RESPAWN_GUARD_MAX = 3');
     expect(block).toContain('respawnHistory');
     expect(block).toContain('agentRespawnGuardTripped');
@@ -72,7 +78,7 @@ describe('terminal-agent watchdog (v1.44+)', () => {
 
   test('7. CLI cold-start path uses the same spawnTerminalAgent helper', () => {
     const cli = fs.readFileSync(
-      path.resolve(new URL(import.meta.url).pathname, '..', '..', 'src', 'cli.ts'),
+      path.resolve(import.meta.path, '..', '..', 'src', 'cli.ts'),
       'utf-8',
     );
     // Otherwise the CLI and watchdog could drift on spawn env/cwd, and

test/hermetic-wiring.test.ts

@@ -17,7 +17,7 @@ import { describe, test, expect } from 'bun:test';
 import * as fs from 'fs';
 import * as path from 'path';
 
-const ROOT = path.resolve(new URL(import.meta.url).pathname, '..', '..');
+const ROOT = path.resolve(import.meta.path, '..', '..');
 
 const RUNNERS = [
   'test/helpers/session-runner.ts',