From 03e5911dba13b9e57d35e1d3391829099a4a044d Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 29 Sep 2026 14:27:26 +0000 Subject: [PATCH] fix(browse): make connect --supervise actually respawn a crashed server The supervisor respawned with a block-scoped env that no longer existed, so every attempt threw and the loop gave up after five tries. The headed env is now one pure helper used by connect and respawn, the loop is an injectable runHeadedSupervisor with behavioral tests, failures name the daemon log and relaunch command, and connect's usage advertises --supervise. --- browse/sections/command-list.md | 2 +- browse/src/cli.ts | 192 ++++++++++++++++++----------- browse/src/commands.ts | 2 +- browse/test/cli-supervisor.test.ts | 119 +++++++++++++++++- gstack/llms.txt | 2 +- 5 files changed, 242 insertions(+), 75 deletions(-) diff --git a/browse/sections/command-list.md b/browse/sections/command-list.md index 24f7b93af..cd0ef01a6 100644 --- a/browse/sections/command-list.md +++ b/browse/sections/command-list.md @@ -163,7 +163,7 @@ Refs are invalidated on navigation — run `snapshot` again after `goto`. ### Server | Command | Description | |---------|-------------| -| `connect` | Launch headed Chromium with Chrome extension | +| `connect [--supervise]` | Launch headed Chromium with Chrome extension; --supervise keeps the CLI attached and respawns a crashed server | | `disconnect` | Disconnect headed browser, return to headless mode | | `focus [@ref]` | Bring headed browser window to foreground (macOS) | | `handoff [message]` | Open visible Chrome at current page for user takeover | diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 980a311dd..fe44feaf7 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -130,7 +130,7 @@ interface ServerState { configHash?: string; /** Xvfb child PID for cleanup on disconnect. */ xvfbPid?: number; - xvfbStartTime?: number; + xvfbStartTime?: string; xvfbDisplay?: string; /** Launched-Chromium identity for post-stop reaping (#2709). */ chromiumPid?: number; @@ -423,6 +423,102 @@ export function buildRestartEnv( return env; } +/** + * Build the env for the headed `$B connect` server. Used by the initial + * connect and by the opt-in supervisor's respawn, so a respawned server keeps + * the same port, watchdog setting, proxy and config hash. Pure + exported for tests. + */ +export function buildHeadedServerEnv( + globalFlags: Pick, +): Record { + return { + BROWSE_HEADED: '1', + // Use a well-known port so the Chrome extension auto-connects. + BROWSE_PORT: '34567', + // Disable parent-process watchdog: the user controls the headed browser + // window lifecycle. The CLI exits immediately after connect, so watching + // it would kill the server ~15s later. Cleanup happens via browser + // disconnect event or $B disconnect. + BROWSE_PARENT_PID: '0', + // Apply --proxy from this invocation if present. Without this, + // `browse --proxy connect` would launch headed Chromium + // bypassing the SOCKS bridge entirely. + ...(globalFlags.proxyUrl ? { BROWSE_PROXY_URL: globalFlags.proxyUrl } : {}), + ...(globalFlags.configHash ? { BROWSE_CONFIG_HASH: globalFlags.configHash } : {}), + }; +} + +export const SUPERVISOR_GUARD_WINDOW_MS = 5 * 60_000; +export const SUPERVISOR_GUARD_MAX = 5; + +export interface HeadedSupervisorDeps { + env: Record; + tickMs: number; + backoffMs: number[]; + daemonLog: string; + readState: () => { pid?: number } | null; + isProcessAlive: (pid: number) => boolean; + startServer: (env: Record) => Promise<{ pid: number; port: number }>; + spawnTerminalAgent: (server: { pid: number; port: number }) => void; + sleep: (ms: number) => Promise; + now: () => number; + isExiting: () => boolean; + log: (line: string) => void; + warn: (line: string) => void; + error: (line: string) => void; +} + +/** + * The opt-in `$B connect --supervise` loop: poll the server PID every tick and + * respawn it with the connect env when it dies. Five respawns inside the + * rolling five-minute window give up. Returns 'stopped' when a signal asked it + * to exit and 'gave_up' when the crash-loop guard tripped. + */ +export async function runHeadedSupervisor(deps: HeadedSupervisorDeps): Promise<'stopped' | 'gave_up'> { + const respawns: number[] = []; + while (!deps.isExiting()) { + await deps.sleep(deps.tickMs); + if (deps.isExiting()) break; + const state = deps.readState(); + if (state?.pid && deps.isProcessAlive(state.pid)) continue; + // Server died. Prune rolling window and check guard. + const now = deps.now(); + while (respawns.length && now - respawns[0] > SUPERVISOR_GUARD_WINDOW_MS) { + respawns.shift(); + } + if (respawns.length >= SUPERVISOR_GUARD_MAX) { + deps.error( + `[browse] Supervisor: ${SUPERVISOR_GUARD_MAX} server crashes in ${SUPERVISOR_GUARD_WINDOW_MS / 1000}s, giving up. ` + + `Crash reasons: ${deps.daemonLog}. Relaunch: $B connect --supervise`, + ); + return 'gave_up'; + } + const attempt = respawns.length; + respawns.push(now); + const backoff = deps.backoffMs[Math.min(attempt, deps.backoffMs.length - 1)] ?? 30_000; + deps.warn(`[browse] Supervisor: server PID gone — respawning in ${backoff}ms (attempt ${attempt + 1}/${SUPERVISOR_GUARD_MAX})...`); + await deps.sleep(backoff); + if (deps.isExiting()) break; + let respawned: { pid: number; port: number }; + try { + respawned = await deps.startServer(deps.env); + } catch (err: any) { + // Let the next tick try again — the crash-loop guard already + // bounded the retries via the rolling window. + deps.error(`[browse] Supervisor: server respawn failed: ${err?.message || err}. Daemon log: ${deps.daemonLog}`); + continue; + } + deps.log(`[browse] Supervisor: server respawned (PID ${respawned.pid}, port ${respawned.port}).`); + // Re-spawn the terminal-agent too; same env wiring as the initial connect. + try { + deps.spawnTerminalAgent(respawned); + } catch (err: any) { + deps.warn(`[browse] Supervisor: terminal-agent respawn failed: ${err?.message || err}`); + } + } + return 'stopped'; +} + /** macOS only: pull the headed Chromium window to the user's current Space. * "Google Chrome for Testing" frequently opens behind the active window or on * another Space — the first thing users read as "I can't see the browser" @@ -1640,22 +1736,7 @@ Refs: After 'snapshot', use @e1, @e2... as selectors: console.log('Launching headed Chromium with extension + terminal agent...'); try { // Start server in headed mode with extension auto-loaded - // Use a well-known port so the Chrome extension auto-connects - const serverEnv: Record = { - BROWSE_HEADED: '1', - BROWSE_PORT: '34567', - // Disable parent-process watchdog: the user controls the headed browser - // window lifecycle. The CLI exits immediately after connect, so watching - // it would kill the server ~15s later. Cleanup happens via browser - // disconnect event or $B disconnect. - BROWSE_PARENT_PID: '0', - // Apply --proxy from this invocation if present. Without this, - // `browse --proxy connect` would launch headed Chromium - // bypassing the SOCKS bridge entirely. - ...(globalFlags.proxyUrl ? { BROWSE_PROXY_URL: globalFlags.proxyUrl } : {}), - ...(globalFlags.configHash ? { BROWSE_CONFIG_HASH: globalFlags.configHash } : {}), - }; - const newState = await startServer(serverEnv); + const newState = await startServer(buildHeadedServerEnv(globalFlags)); // Print connected status const resp = await fetch(`http://127.0.0.1:${newState.port}/command`, { @@ -1737,58 +1818,31 @@ Refs: After 'snapshot', use @e1, @e2... as selectors: process.on('SIGINT', () => teardownAndExit('SIGINT')); process.on('SIGTERM', () => teardownAndExit('SIGTERM')); - const SUPERVISOR_TICK_MS = parseInt( - process.env.GSTACK_SUPERVISOR_TICK_MS || '30000', - 10, - ); - const SUPERVISOR_GUARD_WINDOW_MS = 5 * 60_000; - const SUPERVISOR_GUARD_MAX = 5; - const SUPERVISOR_BACKOFF_MS = (process.env.GSTACK_SUPERVISOR_BACKOFF || '1000,2000,4000,8000,30000') - .split(',').map(s => parseInt(s.trim(), 10)).filter(n => Number.isFinite(n)); - const respawns: number[] = []; - - while (!supervisorExiting) { - await new Promise(resolve => setTimeout(resolve, SUPERVISOR_TICK_MS)); - if (supervisorExiting) break; - const state = readState(); - if (state?.pid && isProcessAlive(state.pid)) continue; - // Server died. Prune rolling window and check guard. - const now = Date.now(); - while (respawns.length && now - respawns[0] > SUPERVISOR_GUARD_WINDOW_MS) { - respawns.shift(); - } - if (respawns.length >= SUPERVISOR_GUARD_MAX) { - console.error( - `[browse] Supervisor: ${SUPERVISOR_GUARD_MAX} crashes in ${SUPERVISOR_GUARD_WINDOW_MS / 1000}s — giving up.`, - ); - process.exit(1); - } - const attempt = respawns.length; - respawns.push(now); - const backoff = SUPERVISOR_BACKOFF_MS[Math.min(attempt, SUPERVISOR_BACKOFF_MS.length - 1)] ?? 30_000; - console.warn(`[browse] Supervisor: server PID gone — respawning in ${backoff}ms (attempt ${attempt + 1}/${SUPERVISOR_GUARD_MAX})...`); - await new Promise(resolve => setTimeout(resolve, backoff)); - if (supervisorExiting) break; - try { - const respawned = await startServer(serverEnv); - console.log(`[browse] Supervisor: server respawned (PID ${respawned.pid}, port ${respawned.port}).`); - // Re-spawn the terminal-agent too; same env wiring as the initial connect. - try { - spawnTerminalAgent({ - stateFile: config.stateFile, - serverPort: respawned.port, - ownerPid: respawned.pid, - cwd: config.projectDir, - }); - } catch (err: any) { - console.warn(`[browse] Supervisor: terminal-agent respawn failed: ${err?.message || err}`); - } - } catch (err: any) { - console.error(`[browse] Supervisor: server respawn failed: ${err?.message || err}`); - // Let the next tick try again — the crash-loop guard already - // bounded the retries via the rolling window. - } - } + const outcome = await runHeadedSupervisor({ + env: buildHeadedServerEnv(globalFlags), + tickMs: parseInt(process.env.GSTACK_SUPERVISOR_TICK_MS || '30000', 10), + backoffMs: (process.env.GSTACK_SUPERVISOR_BACKOFF || '1000,2000,4000,8000,30000') + .split(',').map(s => parseInt(s.trim(), 10)).filter(n => Number.isFinite(n)), + daemonLog: daemonLogPath(), + readState, + isProcessAlive, + startServer, + spawnTerminalAgent: (respawned) => { + spawnTerminalAgent({ + stateFile: config.stateFile, + serverPort: respawned.port, + ownerPid: respawned.pid, + cwd: config.projectDir, + }); + }, + sleep: (ms) => new Promise(resolve => setTimeout(resolve, ms)), + now: Date.now, + isExiting: () => supervisorExiting, + log: (line) => console.log(line), + warn: (line) => console.warn(line), + error: (line) => console.error(line), + }); + if (outcome === 'gave_up') process.exit(1); process.exit(0); } diff --git a/browse/src/commands.ts b/browse/src/commands.ts index 8b9a7ea5d..3be130715 100644 --- a/browse/src/commands.ts +++ b/browse/src/commands.ts @@ -161,7 +161,7 @@ export const COMMAND_DESCRIPTIONS: Record { }); }); +// A scripted world for runHeadedSupervisor: `alive` decides the PID probe per +// tick, sleep advances the injected clock, and every side effect is recorded. +function harness(opts: { + alive: (tick: number) => boolean; + startServer?: (call: number) => Promise<{ pid: number; port: number }>; + spawnTerminalAgent?: () => void; + tickMs?: number; + exitAfterSleeps?: number; +}) { + let clock = 1_000_000, sleeps = 0, tick = 0, exiting = false, starts = 0; + const calls = { startEnv: [] as Record[], agents: [] as number[], log: [] as string[], warn: [] as string[], error: [] as string[] }; + const deps: HeadedSupervisorDeps = { + env: buildHeadedServerEnv({ proxyUrl: 'socks5://127.0.0.1:9050', configHash: 'abc123' }), + tickMs: opts.tickMs ?? 30_000, + backoffMs: [1000, 2000, 4000, 8000, 30000], + daemonLog: '/state/browse-daemon.log', + readState: () => ({ pid: 4242 }), + isProcessAlive: () => opts.alive(tick++), + startServer: async (env) => { + calls.startEnv.push(env); + const call = starts++; + return opts.startServer ? opts.startServer(call) : { pid: 5000 + call, port: 34567 }; + }, + spawnTerminalAgent: (server) => { calls.agents.push(server.pid); opts.spawnTerminalAgent?.(); }, + sleep: async (ms) => { + clock += ms; sleeps++; + if (opts.exitAfterSleeps !== undefined && sleeps >= opts.exitAfterSleeps) exiting = true; + }, + now: () => clock, + isExiting: () => exiting, + log: (line) => calls.log.push(line), + warn: (line) => calls.warn.push(line), + error: (line) => calls.error.push(line), + }; + return { deps, calls, stop: () => { exiting = true; } }; +} + +describe('runHeadedSupervisor (behavior)', () => { + test('a dead server is respawned with exactly the initial connect env, and its terminal agent too', async () => { + const h = harness({ alive: (t) => t !== 0, exitAfterSleeps: 4 }); + expect(await runHeadedSupervisor(h.deps)).toBe('stopped'); + expect(h.calls.startEnv).toHaveLength(1); + expect(h.calls.startEnv[0]).toEqual({ + BROWSE_HEADED: '1', BROWSE_PORT: '34567', BROWSE_PARENT_PID: '0', + BROWSE_PROXY_URL: 'socks5://127.0.0.1:9050', BROWSE_CONFIG_HASH: 'abc123', + }); + expect(h.calls.startEnv[0]).toBe(h.deps.env); + expect(h.calls.agents).toEqual([5000]); + expect(h.calls.error).toEqual([]); + expect(h.calls.log.join('\n')).toContain('server respawned (PID 5000, port 34567)'); + }); + + test('a failed respawn is logged with the daemon log path and counted toward the guard', async () => { + const h = harness({ alive: () => false, startServer: async () => { throw new Error('port 34567 busy'); } }); + expect(await runHeadedSupervisor(h.deps)).toBe('gave_up'); + const failures = h.calls.error.filter(line => line.includes('server respawn failed')); + expect(failures).toHaveLength(5); + expect(failures[0]).toBe('[browse] Supervisor: server respawn failed: port 34567 busy. Daemon log: /state/browse-daemon.log'); + }); + + test('five crashes inside the window give up with the cause and the relaunch command', async () => { + const h = harness({ alive: () => false }); + expect(await runHeadedSupervisor(h.deps)).toBe('gave_up'); + expect(h.calls.startEnv).toHaveLength(5); + expect(h.calls.error.at(-1)).toBe( + '[browse] Supervisor: 5 server crashes in 300s, giving up. Crash reasons: /state/browse-daemon.log. Relaunch: $B connect --supervise', + ); + }); + + test('crashes spread wider than the rolling window never trip the guard', async () => { + // One crash per tick with a tick longer than the window: every earlier + // respawn is pruned before the guard is checked. + const h = harness({ alive: (t) => t >= 12, tickMs: SUPERVISOR_GUARD_WINDOW_MS + 1, exitAfterSleeps: 30 }); + expect(await runHeadedSupervisor(h.deps)).toBe('stopped'); + expect(h.calls.startEnv).toHaveLength(12); + expect(h.calls.error).toEqual([]); + }); + + test('a terminal-agent failure after a successful respawn warns and keeps supervising', async () => { + const h = harness({ alive: (t) => t !== 0, spawnTerminalAgent: () => { throw new Error('no pty'); }, exitAfterSleeps: 4 }); + expect(await runHeadedSupervisor(h.deps)).toBe('stopped'); + expect(h.calls.warn.some(line => line === '[browse] Supervisor: terminal-agent respawn failed: no pty')).toBe(true); + expect(h.calls.error).toEqual([]); + }); + + test('an exit requested during backoff stops without starting a server', async () => { + // Sleep 1 is the tick, sleep 2 the backoff; exiting flips during backoff. + const h = harness({ alive: () => false, exitAfterSleeps: 2 }); + expect(await runHeadedSupervisor(h.deps)).toBe('stopped'); + expect(h.calls.startEnv).toEqual([]); + }); + + test('a live server is left alone', async () => { + const h = harness({ alive: () => true, exitAfterSleeps: 5 }); + expect(await runHeadedSupervisor(h.deps)).toBe('stopped'); + expect(h.calls.startEnv).toEqual([]); + }); +}); + +describe('buildHeadedServerEnv', () => { + test('omits proxy and config hash when this invocation has none', () => { + expect(buildHeadedServerEnv({})).toEqual({ BROWSE_HEADED: '1', BROWSE_PORT: '34567', BROWSE_PARENT_PID: '0' }); + }); +}); + function sliceBetween(source: string, start: string, end: string): string { const i = source.indexOf(start); if (i === -1) throw new Error(`marker not found: ${start}`); diff --git a/gstack/llms.txt b/gstack/llms.txt index 5abf41b19..e7fbdb7cd 100644 --- a/gstack/llms.txt +++ b/gstack/llms.txt @@ -139,7 +139,7 @@ Run with `browse [args]`. Full reference: `browse/SKILL.md`. - `text [selector|@ref]`: Cleaned visible page text, or cleaned text for a CSS selector/@ref when one is provided ### Server -- `connect`: Launch headed Chromium with Chrome extension +- `connect [--supervise]`: Launch headed Chromium with Chrome extension; --supervise keeps the CLI attached and respawns a crashed server - `disconnect`: Disconnect headed browser, return to headless mode - `focus [@ref]`: Bring headed browser window to foreground (macOS) - `handoff [message]`: Open visible Chrome at current page for user takeover