From 3878e434c0e2348587e26e4fd8ec9e11a0000049 Mon Sep 17 00:00:00 2001 From: SYKhayyat Date: Fri, 31 Jul 2026 12:11:06 -0400 Subject: [PATCH 1/2] fix(test): use import.meta.path so static tripwires run on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `path.resolve(new URL(import.meta.url).pathname, ...)` yields a leading-slash drive path on Windows (`/C:/Users/...`), so path.resolve prepends the cwd's drive and produces `C:\C:\Users\...`. Every static-grep tripwire that locates itself this way threw ENOENT on readdir/readFile instead of asserting anything. Measured on Windows 11 across the 14 affected files: before: 12 pass / 80 fail after: 92 pass / 0 fail The inert tripwires included the pkill-teardown regression guard, CDP session cleanup, SSE subscriber wiring, and the PTY lease routes — i.e. exactly the invariants that are supposed to fail CI when someone regresses them. CI stayed green because windows-free-tests.yml runs a curated 13-file allowlist that excludes all of these; its own comments track full Windows parity as a follow-up. import.meta.path is Bun's absolute native path for the current file. Drop-in with identical arity, so every '..' segment count is unchanged. Co-Authored-By: Claude Opus 5 --- browse/test/cdp-session-cleanup.test.ts | 2 +- browse/test/cli-supervisor.test.ts | 2 +- browse/test/server-embedder-terminal-port.test.ts | 2 +- browse/test/server-pty-lease-routes.test.ts | 2 +- browse/test/sidepanel-patient-autoconnect.test.ts | 2 +- browse/test/sidepanel-reattach.test.ts | 2 +- browse/test/sidepanel-restart-dispose.test.ts | 4 ++-- browse/test/terminal-agent-detach-reattach.test.ts | 2 +- browse/test/terminal-agent-internal-handler.test.ts | 2 +- browse/test/terminal-agent-keepalive.test.ts | 4 ++-- browse/test/terminal-agent-pid-identity.test.ts | 2 +- browse/test/terminal-agent-session-routing.test.ts | 2 +- browse/test/terminal-agent-watchdog.test.ts | 6 +++--- test/hermetic-wiring.test.ts | 2 +- 14 files changed, 18 insertions(+), 18 deletions(-) diff --git a/browse/test/cdp-session-cleanup.test.ts b/browse/test/cdp-session-cleanup.test.ts index 25ca6760cb..f474df9f63 100644 --- a/browse/test/cdp-session-cleanup.test.ts +++ b/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 }> = []; diff --git a/browse/test/cli-supervisor.test.ts b/browse/test/cli-supervisor.test.ts index d9cec7b89d..22bdb57d9d 100644 --- a/browse/test/cli-supervisor.test.ts +++ b/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', () => { diff --git a/browse/test/server-embedder-terminal-port.test.ts b/browse/test/server-embedder-terminal-port.test.ts index f24ee35101..051d931954 100644 --- a/browse/test/server-embedder-terminal-port.test.ts +++ b/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', diff --git a/browse/test/server-pty-lease-routes.test.ts b/browse/test/server-pty-lease-routes.test.ts index 2c12618830..aad16ff909 100644 --- a/browse/test/server-pty-lease-routes.test.ts +++ b/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)', () => { diff --git a/browse/test/sidepanel-patient-autoconnect.test.ts b/browse/test/sidepanel-patient-autoconnect.test.ts index faf38499b4..5c8679f8e3 100644 --- a/browse/test/sidepanel-patient-autoconnect.test.ts +++ b/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, '..', '..', '..', diff --git a/browse/test/sidepanel-reattach.test.ts b/browse/test/sidepanel-reattach.test.ts index 9179e57c40..815b04cd91 100644 --- a/browse/test/sidepanel-reattach.test.ts +++ b/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)', () => { diff --git a/browse/test/sidepanel-restart-dispose.test.ts b/browse/test/sidepanel-restart-dispose.test.ts index 8ec44690b7..2883dcc1f1 100644 --- a/browse/test/sidepanel-restart-dispose.test.ts +++ b/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+)', () => { diff --git a/browse/test/terminal-agent-detach-reattach.test.ts b/browse/test/terminal-agent-detach-reattach.test.ts index 89fbe5a1ca..fcca6684da 100644 --- a/browse/test/terminal-agent-detach-reattach.test.ts +++ b/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', () => { diff --git a/browse/test/terminal-agent-internal-handler.test.ts b/browse/test/terminal-agent-internal-handler.test.ts index 04e7f35971..b3a7c1ee6e 100644 --- a/browse/test/terminal-agent-internal-handler.test.ts +++ b/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 exists with the documented signature', () => { diff --git a/browse/test/terminal-agent-keepalive.test.ts b/browse/test/terminal-agent-keepalive.test.ts index 812f70f818..2e94406813 100644 --- a/browse/test/terminal-agent-keepalive.test.ts +++ b/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', () => { diff --git a/browse/test/terminal-agent-pid-identity.test.ts b/browse/test/terminal-agent-pid-identity.test.ts index 52503fe2ea..f0250416a8 100644 --- a/browse/test/terminal-agent-pid-identity.test.ts +++ b/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 }> = []; diff --git a/browse/test/terminal-agent-session-routing.test.ts b/browse/test/terminal-agent-session-routing.test.ts index acc51b2af3..1a8184b624 100644 --- a/browse/test/terminal-agent-session-routing.test.ts +++ b/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', () => { diff --git a/browse/test/terminal-agent-watchdog.test.ts b/browse/test/terminal-agent-watchdog.test.ts index f012dc406e..a54bcf8fe4 100644 --- a/browse/test/terminal-agent-watchdog.test.ts +++ b/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', () => { @@ -72,7 +72,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 diff --git a/test/hermetic-wiring.test.ts b/test/hermetic-wiring.test.ts index 08528586d2..8276db0d77 100644 --- a/test/hermetic-wiring.test.ts +++ b/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', From 7d9e4ff7e11bddd163c5f7e5909ffe7ef0fe3b18 Mon Sep 17 00:00:00 2001 From: SYKhayyat Date: Fri, 31 Jul 2026 12:11:34 -0400 Subject: [PATCH 2/2] fix(windows): stop terminal-agent leaking a process + console window per tick MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows the browse daemon leaked one `bun run terminal-agent.ts` process and one visible console window every 60 seconds, without bound, until the host ran out of committable memory. Observed on a 48GB Windows 11 box: 7 orphaned agents and climbing, with focus stolen from the foreground app once a minute. Three bugs had to line up: 1. isProcessAlive() shelled out to `tasklist /FI "PID eq "` with a 3000ms timeout. A Bun.spawnSync that hits its timeout STILL RETURNS, carrying partial stdout — so the .includes() PID match came back false and a LIVE process was reported dead. Measured tasklist latency was 685-1665ms on an idle box and considerably worse under memory pressure, so the timeout was reachable in ordinary use, not just pathological cases. 2. That false negative made killAgentByRecord() skip the kill (it validates liveness before signalling) while the watchdog respawned regardless, orphaning the survivor. The failure was self-reinforcing: every orphan added memory pressure, which slowed the next tasklist, which produced the next false negative. 3. RESPAWN_GUARD_WINDOW_MS was a fixed 60_000 against a 60_000ms tick, so at most ONE respawn could ever sit inside the rolling window and the `respawnHistory.length >= RESPAWN_GUARD_MAX (3)` trip was unreachable. The crash-loop guard was structurally incapable of firing at the default tick, so nothing bounded the leak. Fixes: - isProcessAlive() uses 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. Measured 0.004ms vs 270ms (~74,000x), spawns nothing, and cannot time out. EPERM means the process exists but we lack rights to signal it, which is alive — returning false there would reintroduce the original failure. - spawnTerminalAgent() passes windowsHide: true. The agent is a background daemon with all three stdio streams already ignored, so it has nothing to display. No-op on macOS/Linux. - RESPAWN_GUARD_WINDOW_MS scales with the tick, so "3 respawns in quick succession stops the loop" holds at any tick value. browse/test/process-liveness-windows.test.ts pins all three, including an assertion that the liveness probe spawns no subprocess at all — the property that made the original implementation timeout-prone. The watchdog tripwire previously asserted the literal `RESPAWN_GUARD_WINDOW_MS = 60_000`, pinning the bug as an invariant; it now pins the window's relationship to the tick instead. Co-Authored-By: Claude Opus 5 --- browse/src/error-handling.ts | 46 ++++--- browse/src/server.ts | 12 +- browse/src/terminal-agent-control.ts | 5 + browse/test/process-liveness-windows.test.ts | 134 +++++++++++++++++++ browse/test/terminal-agent-watchdog.test.ts | 8 +- 5 files changed, 187 insertions(+), 18 deletions(-) create mode 100644 browse/test/process-liveness-windows.test.ts diff --git a/browse/src/error-handling.ts b/browse/src/error-handling.ts index 2c4e271e87..ba6f1b61cf 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,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 "` 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'; } } diff --git a/browse/src/server.ts b/browse/src/server.ts index 301781acce..7f47b1e957 100644 --- a/browse/src/server.ts +++ b/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) { diff --git a/browse/src/terminal-agent-control.ts b/browse/src/terminal-agent-control.ts index 094ba668fa..411e02a12e 100644 --- a/browse/src/terminal-agent-control.ts +++ b/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; diff --git a/browse/test/process-liveness-windows.test.ts b/browse/test/process-liveness-windows.test.ts new file mode 100644 index 0000000000..672e725417 --- /dev/null +++ b/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); + }); +}); diff --git a/browse/test/terminal-agent-watchdog.test.ts b/browse/test/terminal-agent-watchdog.test.ts index a54bcf8fe4..ff48b0bb3d 100644 --- a/browse/test/terminal-agent-watchdog.test.ts +++ b/browse/test/terminal-agent-watchdog.test.ts @@ -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');