diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 942b56148..c5b875755 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -316,10 +316,14 @@ export async function reapRecordedChromium(state: { const cmd = readPidCmdline(pid).toLowerCase(); if (!/chrom|headless_shell/.test(cmd)) return; safeKill(pid, 'SIGTERM'); - await new Promise(r => setTimeout(r, 1000)); + // Poll instead of a fixed sleep: the common case (daemon's own close is + // finishing concurrently) exits in ~100-200ms instead of always paying 1s. + const deadline = Date.now() + 1000; + while (Date.now() < deadline && isProcessAlive(pid)) { + await new Promise(r => setTimeout(r, 100)); + } if (isProcessAlive(pid)) { safeKill(pid, 'SIGKILL'); - await new Promise(r => setTimeout(r, 500)); } } @@ -496,7 +500,14 @@ async function startServer(extraEnv?: Record): Promise { + const fs = require('node:fs') as typeof import('node:fs'); + const path = require('node:path') as typeof import('node:path'); + const SRC = path.resolve(import.meta.dir, '..', 'src'); + const read = (f: string) => fs.readFileSync(path.join(SRC, f), 'utf-8'); + + test('server.ts persists chromiumPid/chromiumStartTime from getChromiumProcInfo() into the state file', () => { + const src = read('server.ts'); + const stateStart = src.indexOf('const state: Record = {'); + expect(stateStart, 'state-object literal not found in server.ts').toBeGreaterThan(-1); + // The object literal ends where the daemon serializes it and renames the + // tmp file into place — identity must be INSIDE what gets persisted. + const stateEnd = src.indexOf('fs.renameSync(tmpFile, config.stateFile)', stateStart); + expect(stateEnd, 'state-file rename not found after the state object').toBeGreaterThan(stateStart); + const stateObj = src.slice(stateStart, stateEnd); + expect(stateObj).toContain('browserManager.getChromiumProcInfo()'); + expect(stateObj).toContain('chromiumPid: info.pid'); + expect(stateObj).toContain('chromiumStartTime: info.startTime'); + }); + + test('cli.ts reaps on every stop + stale-state path (>=5 call sites)', () => { + const src = read('cli.ts'); + // Every call site awaits; the only other occurrence is the definition. + expect(src).toContain('export async function reapRecordedChromium('); + const callSites = src.split('await reapRecordedChromium(').length - 1; + expect(callSites, 'a reap call site was removed — every stop/stale path must reap').toBeGreaterThanOrEqual(5); + + const between = (from: string, to: string) => { + const a = src.indexOf(from); + expect(a, `anchor not found in cli.ts: ${from}`).toBeGreaterThan(-1); + const b = src.indexOf(to, a); + expect(b, `anchor not found after "${from}": ${to}`).toBeGreaterThan(a); + return src.slice(a, b); + }; + + // 1. Dead-daemon stop branch: reap BEFORE destroying the state file — the + // state file is the only carrier of the child's identity. + const deadDaemon = between( + '!isProcessAlive(stopState.pid) && !(await isServerHealthy(stopState.port))', + 'No daemon running (cleaned stale state)', + ); + expect(deadDaemon).toMatch( + /await reapRecordedChromium\(stopState\);[\s\S]*safeUnlinkQuiet\(config\.stateFile\);/, + ); + + // 2. Force-stop on a live daemon (stop --force-restart short-circuit). + const forceStop = between( + 'isProcessAlive(stopState.pid) && globalFlags.forceRestart', + 'Daemon stopped (forced', + ); + expect(forceStop).toContain('await reapRecordedChromium(stopState);'); + + // 3. startServer stale-state cleanup, before safeUnlink(config.stateFile). + const startServer = between('async function startServer(', 'safeUnlink(config.stateFile);'); + expect(startServer).toMatch( + /const staleState = readState\(\);\s*\n\s*if \(staleState\) await reapRecordedChromium\(staleState\);/, + ); + + // 4. Headed-connect stale path: reap sits between killOrphanChromium() + // (which cannot see the lock-less headless child) and the unlink. + const connect = between("if (command === 'connect')", 'Launching headed Chromium'); + expect(connect).toMatch( + /await killOrphanChromium\(\);[\s\S]*if \(staleState\) await reapRecordedChromium\(staleState\);[\s\S]*safeUnlinkQuiet\(config\.stateFile\);/, + ); + + // 5. Post-graceful-stop: the daemon closed Chromium via Playwright, but a + // surviving GPU process must still be reaped after sendCommand('stop'). + const postStop = between('await sendCommand(state, command, commandArgs);', "if (command === 'focus')"); + expect(postStop).toMatch( + /if \(command === 'stop'\) \{\s*\n\s*await reapRecordedChromium\(state\);/, + ); + }); + + test('browser-manager.ts pushes headlessGpuArgs only under the headless launch guard', () => { + const src = read('browser-manager.ts'); + // Exactly one call site (the exported definition aside) — a second, + // unguarded push would strip the GPU from headed/GBrowser sessions. + const calls = src.match(/headlessGpuArgs\(process\.platform, process\.env\)/g) ?? []; + expect(calls.length).toBe(1); + // And that one call site is guarded by the headless flag: the extensions + // path above it forces useHeadless = false, so extension-loaded and headed + // launches never receive the GPU-disable flags. + expect(src).toMatch( + /if \(useHeadless\) \{\s*\n\s*launchArgs\.push\(\.\.\.headlessGpuArgs\(process\.platform, process\.env\)\);\s*\n\s*\}/, + ); + }); +});