Skip to content

Commit b4689d1

Browse files
committed
Keep browse daemons alive across separate CLI invocations on macOS
The browse CLI was starting a server that looked healthy during the first command, but the daemon and its Chromium children were still tied to the launching shell session. A follow-up invocation would miss the prior server, start from about:blank, and lose all persisted browse state. This switches the POSIX launcher to a detached Node child that starts Bun in a separate session/process group, and adds explicit SIGHUP handling on the server side so headless daemons stay alive when short-lived shells exit. It also adds a small regression guard around the detached stdio contract. Constraint: The fix must work with Bun-built CLI binaries on macOS without adding dependencies Rejected: Keep using Bun.spawn(...).unref() | daemon and Chromium still died after the launching shell exited Rejected: nohup wrapper only | shell hangup still dropped the launched browser session in practice Confidence: medium Scope-risk: moderate Reversibility: clean Directive: Detached launch behavior is part of browse state persistence; do not revert to parent-attached spawn without re-testing separate invocations Tested: bun test test/browse-cli-daemon.test.ts; bun run build; manual browse goto https://example.com then separate browse url reuse check Not-tested: Full /qa and /canary flows on macOS after prolonged idle periods
1 parent 656df0e commit b4689d1

3 files changed

Lines changed: 58 additions & 26 deletions

File tree

browse/src/cli.ts

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,17 @@ interface ServerState {
9494
mode?: 'launched' | 'headed';
9595
}
9696

97+
export function detachedServerSpawnStdio(isWindows: boolean = IS_WINDOWS): ['ignore', 'ignore', 'ignore'] {
98+
// Keep detached server stdio disconnected from the parent shell on every
99+
// platform. On macOS/Bun, piped stdio can take the daemon down when the
100+
// short-lived CLI process exits between invocations.
101+
return ['ignore', 'ignore', 'ignore'];
102+
}
103+
104+
export function resolveBunExecutable(): string {
105+
return process.env.BUN_BIN || Bun.which('bun') || 'bun';
106+
}
107+
97108
// ─── State File ────────────────────────────────────────────────
98109
function readState(): ServerState | null {
99110
try {
@@ -231,12 +242,17 @@ async function startServer(extraEnv?: Record<string, string>): Promise<ServerSta
231242
`${extraEnvStr})}).unref()`;
232243
Bun.spawnSync(['node', '-e', launcherCode], { stdio: ['ignore', 'ignore', 'ignore'] });
233244
} else {
234-
// macOS/Linux: Bun.spawn + unref works correctly
235-
proc = Bun.spawn(['bun', 'run', SERVER_SCRIPT], {
236-
stdio: ['ignore', 'pipe', 'pipe'],
237-
env: { ...process.env, BROWSE_STATE_FILE: config.stateFile, BROWSE_PARENT_PID: parentPid, ...extraEnv },
238-
});
239-
proc.unref();
245+
// POSIX: run the Bun server inside a detached Node child process so the
246+
// daemon and its Chromium children live in a separate session/process
247+
// group. Bun.spawn(...).unref() alone is not enough on macOS here.
248+
const extraEnvStr = JSON.stringify({ BROWSE_STATE_FILE: config.stateFile, BROWSE_PARENT_PID: parentPid, ...(extraEnv || {}) });
249+
const bunBin = JSON.stringify(resolveBunExecutable());
250+
const launcherCode =
251+
`const{spawn}=require('child_process');` +
252+
`spawn(${bunBin},['run',${JSON.stringify(SERVER_SCRIPT)}],` +
253+
`{detached:true,stdio:['ignore','ignore','ignore'],env:Object.assign({},process.env,` +
254+
`${extraEnvStr})}).unref()`;
255+
Bun.spawnSync(['node', '-e', launcherCode], { stdio: ['ignore', 'ignore', 'ignore'] });
240256
}
241257

242258
// Wait for server to become healthy.
@@ -251,27 +267,16 @@ async function startServer(extraEnv?: Record<string, string>): Promise<ServerSta
251267
await Bun.sleep(100);
252268
}
253269

254-
// Server didn't start in time — try to get error details
255-
if (proc?.stderr) {
256-
// macOS/Linux: read stderr from the spawned process
257-
const reader = proc.stderr.getReader();
258-
const { value } = await reader.read();
259-
if (value) {
260-
const errText = new TextDecoder().decode(value);
261-
throw new Error(`Server failed to start:\n${errText}`);
262-
}
263-
} else {
264-
// Windows: check startup error log (server writes errors to disk since
265-
// stderr is unavailable due to stdio: 'ignore' for detachment)
266-
const errorLogPath = path.join(config.stateDir, 'browse-startup-error.log');
267-
try {
268-
const errorLog = fs.readFileSync(errorLogPath, 'utf-8').trim();
269-
if (errorLog) {
270-
throw new Error(`Server failed to start:\n${errorLog}`);
271-
}
272-
} catch (e: any) {
273-
if (e.code !== 'ENOENT') throw e;
270+
// Server didn't start in time — read the startup error log if the server
271+
// wrote one. This works for detached launches on every platform.
272+
const errorLogPath = path.join(config.stateDir, 'browse-startup-error.log');
273+
try {
274+
const errorLog = fs.readFileSync(errorLogPath, 'utf-8').trim();
275+
if (errorLog) {
276+
throw new Error(`Server failed to start:\n${errorLog}`);
274277
}
278+
} catch (e: any) {
279+
if (e.code !== 'ENOENT') throw e;
275280
}
276281
throw new Error(`Server failed to start within ${MAX_START_WAIT / 1000}s`);
277282
}

browse/src/server.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1489,6 +1489,24 @@ process.on('SIGTERM', () => {
14891489
console.log('[browse] Received SIGTERM (ignoring — use /stop or Ctrl+C for intentional shutdown)');
14901490
}
14911491
});
1492+
// Some shells send SIGHUP to background children when the launching shell exits.
1493+
// Treat it like SIGTERM: keep headless daemons alive across CLI invocations, but
1494+
// still allow headed/tunnel sessions to terminate with their controlling shell.
1495+
if (process.platform !== 'win32') {
1496+
process.on('SIGHUP', () => {
1497+
if (hasActivePicker()) {
1498+
console.log('[browse] Received SIGHUP but cookie picker is active, ignoring to avoid stranding the picker UI');
1499+
return;
1500+
}
1501+
const headed = browserManager.getConnectionMode() === 'headed';
1502+
if (headed || tunnelActive) {
1503+
console.log(`[browse] Received SIGHUP in ${headed ? 'headed' : 'tunnel'} mode, shutting down`);
1504+
shutdown();
1505+
} else {
1506+
console.log('[browse] Received SIGHUP (ignoring — headless daemon stays alive across CLI invocations)');
1507+
}
1508+
});
1509+
}
14921510
// Windows: taskkill /F bypasses SIGTERM, but 'exit' fires for some shutdown paths.
14931511
// Defense-in-depth — primary cleanup is the CLI's stale-state detection via health check.
14941512
if (process.platform === 'win32') {

test/browse-cli-daemon.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { describe, expect, test } from 'bun:test';
2+
import { detachedServerSpawnStdio } from '../browse/src/cli';
3+
4+
describe('browse CLI daemon lifecycle', () => {
5+
test('detached server stdio is disconnected from the parent shell', () => {
6+
expect(detachedServerSpawnStdio(false)).toEqual(['ignore', 'ignore', 'ignore']);
7+
expect(detachedServerSpawnStdio(true)).toEqual(['ignore', 'ignore', 'ignore']);
8+
});
9+
});

0 commit comments

Comments
 (0)