From d8a7d014ae022f159ddfe461865c291164fe2700 Mon Sep 17 00:00:00 2001 From: Sinabina Date: Fri, 10 Jul 2026 16:31:24 -0700 Subject: [PATCH] fix(browse): preserve live daemon during busy loads --- browse/src/cli.ts | 37 ++++++++++++++------ browse/test/live-daemon-preservation.test.ts | 18 ++++++++++ 2 files changed, 44 insertions(+), 11 deletions(-) create mode 100644 browse/test/live-daemon-preservation.test.ts diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 59327b7923..b1d41b13b2 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -89,7 +89,7 @@ if (IS_WINDOWS && !NODE_SERVER_SCRIPT) { ); } -interface ServerState { +export interface ServerState { pid: number; port: number; token: string; @@ -105,6 +105,16 @@ interface ServerState { xvfbDisplay?: string; } +/** A daemon whose process still exists may be busy even when HTTP health checks + * time out. Preserve it (and its browser session) instead of guessing that it + * is dead. The caller can retry once the current page load settles. */ +export function shouldPreserveUnresponsiveServer( + state: ServerState | null, + processAlive: (pid: number) => boolean = isProcessAlive, +): boolean { + return !!state?.pid && processAlive(state.pid); +} + // ─── State File ──────────────────────────────────────────────── function readState(): ServerState | null { try { @@ -451,12 +461,14 @@ async function ensureServer(flags?: GlobalFlags): Promise { process.exit(1); } - // Guard: never silently replace a headed server with a headless one. - // Headed mode means a user-visible Chrome window is (or was) controlled. - // Silently replacing it would be confusing — tell the user to reconnect. - if (state && state.mode === 'headed' && isProcessAlive(state.pid)) { - console.error(`[browse] Headed server running (PID ${state.pid}) but not responding.`); - console.error(`[browse] Run '/open-gstack-browser' to restart.`); + // A live daemon can stop answering HTTP while Chromium finishes a heavy + // navigation. Restarting here destroys tabs, cookies, and login state. Only + // auto-restart after the daemon process itself is demonstrably gone. + if (shouldPreserveUnresponsiveServer(state)) { + console.error(`[browse] Server running (PID ${state!.pid}) but not responding — busy, not restarting.`); + console.error(state!.mode === 'headed' + ? `[browse] Retry shortly, or run '/open-gstack-browser' to restart intentionally.` + : '[browse] Retry shortly, or run `browse disconnect` to restart intentionally.'); process.exit(1); } @@ -590,10 +602,13 @@ async function sendCommand(state: ServerState, command: string, args: string[], // can briefly stop answering HTTP while still alive. Before declaring a // crash, if the process is alive give /health a bounded chance to recover // and just retry the command — never kill+restart a live-but-busy server. - if (oldState?.pid && isProcessAlive(oldState.pid) && await probeHealthWithBackoff(oldState.port)) { - if (retries >= 1) throw new Error('[browse] Server unresponsive after retry — aborting'); - console.error('[browse] Server was briefly unresponsive (busy); retrying command...'); - return sendCommand(oldState, command, args, retries + 1); + if (shouldPreserveUnresponsiveServer(oldState)) { + if (await probeHealthWithBackoff(oldState!.port)) { + if (retries >= 1) throw new Error('[browse] Server unresponsive after retry — aborting'); + console.error('[browse] Server was briefly unresponsive (busy); retrying command...'); + return sendCommand(oldState!, command, args, retries + 1); + } + throw new Error('[browse] Server is still running but unresponsive — busy, not restarting. Retry shortly.'); } // Truly dead (or health never recovered) → restart. if (retries >= 1) throw new Error('[browse] Server crashed twice in a row — aborting'); diff --git a/browse/test/live-daemon-preservation.test.ts b/browse/test/live-daemon-preservation.test.ts new file mode 100644 index 0000000000..c74bfb7443 --- /dev/null +++ b/browse/test/live-daemon-preservation.test.ts @@ -0,0 +1,18 @@ +import { describe, expect, test } from 'bun:test'; +import { shouldPreserveUnresponsiveServer, type ServerState } from '../src/cli'; + +const state = { pid: 42 } as ServerState; + +describe('live daemon preservation (#2219)', () => { + test('preserves an unresponsive daemon while its process is alive', () => { + expect(shouldPreserveUnresponsiveServer(state, pid => pid === 42)).toBe(true); + }); + + test('allows restart after the daemon process is gone', () => { + expect(shouldPreserveUnresponsiveServer(state, () => false)).toBe(false); + }); + + test('allows startup when no prior daemon state exists', () => { + expect(shouldPreserveUnresponsiveServer(null, () => true)).toBe(false); + }); +});