This commit is contained in:
Time Attakc 2026-07-14 19:02:46 -07:00 committed by GitHub
commit 3e00759cf3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 44 additions and 11 deletions

View File

@ -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<ServerState> {
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');

View File

@ -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);
});
});