diff --git a/browse/src/server.ts b/browse/src/server.ts index 47c2e2e4d..a31d3c83a 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -812,8 +812,17 @@ function parentWatchdogTick(parentPid: number = BROWSE_PARENT_PID): void { } } } +// Poll cadence. Env-overridable as a test seam: watchdog.test.ts shrinks it +// (250ms) so a free-tier test can observe a real tick deciding on a dead +// parent instead of sleeping through the 15s production cadence. Production +// launchers never set this; unparsable or non-positive values fall back to 15s. +const rawWatchdogIntervalMs = parseInt(process.env.BROWSE_PARENT_WATCHDOG_INTERVAL_MS || '', 10); +const PARENT_WATCHDOG_INTERVAL_MS = + Number.isFinite(rawWatchdogIntervalMs) && rawWatchdogIntervalMs > 0 + ? rawWatchdogIntervalMs + : 15_000; if (BROWSE_PARENT_PID > 0 && !IS_HEADED_WATCHDOG) { - setInterval(parentWatchdogTick, 15_000); + setInterval(parentWatchdogTick, PARENT_WATCHDOG_INTERVAL_MS); } else if (IS_HEADED_WATCHDOG) { console.log('[browse] Parent-process watchdog disabled (headed mode)'); } else if (BROWSE_PARENT_PID === 0) { diff --git a/browse/test/browser-skill-commands.test.ts b/browse/test/browser-skill-commands.test.ts index c93a908e2..9d200e10d 100644 --- a/browse/test/browser-skill-commands.test.ts +++ b/browse/test/browser-skill-commands.test.ts @@ -342,8 +342,15 @@ describe.skipIf(SKIP_SPAWN)('spawnSkill: lifecycle', () => { it('timeout fires, exit code 124, token revoked', async () => { const dir = makeSkillDir(tiers.bundled, 'sleeper', 'name: sleeper\nhost: x.com\ntrusted: true', - // Sleep longer than the test timeout; the spawn should kill us. - `await new Promise(r => setTimeout(r, 30000)); console.log("done");`, + // The child's self-lifetime is a bound, not a wait — the test blocks + // only for the 1s spawn timeout that kills it. 8s is sized to be far + // above that 1s (the kill always lands first) but below this test's + // 10s ceiling: if the timeout-kill ever regresses, the child completes, + // prints "done", and the assertions below fail cleanly in-budget + // instead of the test opaquely timing out while the child lingers. + // (runToFiles gives skill children no stdin pipe, so a parent-death + // EOF lifetime isn't available here — self-timing is required.) + `await new Promise(r => setTimeout(r, 8000)); console.log("done");`, ); const skill = readBrowserSkill('sleeper', tiers)!; const result = await spawnSkill({ @@ -351,6 +358,9 @@ describe.skipIf(SKIP_SPAWN)('spawnSkill: lifecycle', () => { }); expect(result.timedOut).toBe(true); expect(result.exitCode).toBe(124); + // The kill must land before the script completes — "done" ever appearing + // means the child outlived its timeout. + expect(result.stdout).not.toContain('done'); expect(listTokens().filter(t => t.clientId.startsWith('skill:sleeper:'))).toEqual([]); }, 10_000); diff --git a/browse/test/stop-dead-daemon.test.ts b/browse/test/stop-dead-daemon.test.ts index b92a669d3..e8ededa43 100644 --- a/browse/test/stop-dead-daemon.test.ts +++ b/browse/test/stop-dead-daemon.test.ts @@ -109,7 +109,16 @@ describe('stop --force-restart on a LIVE daemon', () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-stop-force-')); const stateFile = path.join(tmpDir, 'browse.json'); // Portable long-lived child standing in for the wedged daemon process. - const wedged = spawn('bun', ['-e', 'await Bun.sleep(300000)'], { stdio: 'ignore' }); + // Its lifetime is tied to this test process instead of a fixed sleep: it + // blocks until its stdin (a pipe we hold open) hits EOF. That means it + // can never self-exit mid-test — which would let the "pid is dead" + // assertion below pass without the CLI having killed anything — and it + // reaps itself the moment the test process dies, even on a hard kill + // where the finally block never runs. + const wedged = spawn('bun', ['-e', + "process.stdin.resume(); const bye = () => process.exit(0); " + + "process.stdin.on('end', bye); process.stdin.on('error', bye); process.stdin.on('close', bye);", + ], { stdio: ['pipe', 'ignore', 'ignore'] }); try { const port = await closedPort(); fs.writeFileSync(stateFile, JSON.stringify({ diff --git a/browse/test/terminal-agent-owner-watchdog.test.ts b/browse/test/terminal-agent-owner-watchdog.test.ts index e28502964..386718d2c 100644 --- a/browse/test/terminal-agent-owner-watchdog.test.ts +++ b/browse/test/terminal-agent-owner-watchdog.test.ts @@ -44,9 +44,16 @@ describe('terminal-agent owner lifecycle', () => { // process.execPath (the running bun) instead of `sleep`: coreutils are // not guaranteed on a bare windows-latest runner, and this test is on the // Windows CI curated list — the owner-orphan leak it pins is a Windows bug. + // The owner's lifetime is tied to this test process instead of a fixed + // 30s sleep: it blocks until its stdin (a pipe we hold open) hits EOF, so + // it is guaranteed alive until the SIGTERM below no matter how slow the + // runner is, and it reaps itself if the test process dies without running + // afterEach. Node-compatible stdin APIs, not Bun.stdin — Windows-portable. const owner = Bun.spawn( - [process.execPath, '-e', 'await Bun.sleep(30000)'], - { stdio: ['ignore', 'ignore', 'ignore'] }, + [process.execPath, '-e', + "process.stdin.resume(); const bye = () => process.exit(0); " + + "process.stdin.on('end', bye); process.stdin.on('error', bye); process.stdin.on('close', bye);"], + { stdio: ['pipe', 'ignore', 'ignore'] }, ); spawned.push(owner); const agent = Bun.spawn(['bun', 'run', AGENT_SCRIPT], { diff --git a/browse/test/watchdog.test.ts b/browse/test/watchdog.test.ts index 56201779e..ff4cfb84f 100644 --- a/browse/test/watchdog.test.ts +++ b/browse/test/watchdog.test.ts @@ -27,8 +27,10 @@ import { resolveConfig } from '../src/config'; // seam as idleCheckTick) and tunnelActive is simulated via setTunnelActive. // // Each test spawns the real server.ts. Tests 1 and 2 verify behavior via -// stdout log line (fast). Test 3 waits for the watchdog poll cycle to confirm -// the server REMAINS alive after parent death (slow — ~20s observation window). +// stdout log line (fast). Test 3 shrinks the poll cadence via +// BROWSE_PARENT_WATCHDOG_INTERVAL_MS (test seam in server.ts), waits for the +// tick's one-time "parent exited (server stays alive" log to prove a tick +// observed the death, then confirms the server REMAINS alive. const ROOT = path.resolve(import.meta.dir, '..'); const SERVER_SCRIPT = path.join(ROOT, 'src', 'server.ts'); @@ -139,21 +141,37 @@ describe('parent-process watchdog (v0.18.1.0)', () => { const parentPid = parentProc.pid!; // Default headless: no BROWSE_HEADED, real parent PID — watchdog active. - serverProc = spawnServer({ BROWSE_PARENT_PID: String(parentPid) }, 34903); + // The poll cadence is shrunk via the server's env seam (250ms instead of + // the production 15s) so observing a real tick doesn't cost a 20s sleep. + serverProc = spawnServer({ + BROWSE_PARENT_PID: String(parentPid), + BROWSE_PARENT_WATCHDOG_INTERVAL_MS: '250', + }, 34903); const serverPid = serverProc.pid!; - // Give the server a moment to start and register the watchdog interval. - await Bun.sleep(2000); + // Startup barrier: poll stdout for the listen line instead of a fixed 2s + // sleep. The watchdog interval is registered at module load, before this + // line prints, so once we see it the ticks are running. + const bootOut = await readStdoutUntil(serverProc, 'Server running on', 15_000); + expect(bootOut).toContain('Server running on'); expect(isProcessAlive(serverPid)).toBe(true); - // Kill the parent. The watchdog polls every 15s, so first tick after - // parent death lands within ~15s. Pre-#994 the server would shutdown - // here. Post-#994 the server logs the parent exit and stays alive. + // Kill the parent, then wait for a tick to OBSERVE the death: the + // stay-alive branch logs a one-time latched line. Seeing it proves a tick + // ran after parent death and chose NOT to shut down — pre-#994 the same + // tick called shutdown instead. (Await exited first so the PID is reaped + // and the tick's kill(pid, 0) probe sees ESRCH, not a zombie.) parentProc.kill('SIGKILL'); + await parentProc.exited; + const marker = `Parent process ${parentPid} exited (server stays alive`; + const out = await readStdoutUntil(serverProc, marker, 15_000); + expect(out).toContain(marker); + expect(out).not.toContain('shutting down'); - // Wait long enough for at least one watchdog tick (15s) plus margin. - // Server should still be alive — that's the whole point of #994. - await Bun.sleep(20_000); + // Let several more ticks land (4+ at 250ms — the old fixed 20s sleep + // covered ~1 production tick) and confirm the server is still alive — + // that's the whole point of #994. + await Bun.sleep(1_000); expect(isProcessAlive(serverPid)).toBe(true); }, 45_000); });