Skip to content

Commit 2090f3d

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 keeps the server fully Bun-native on POSIX by using a detached Bun spawn instead of adding a new Node launch dependency, keeps headless daemons alive on SIGHUP, and replaces the weak helper-only regression test with a real multi-invocation lifecycle test against a local HTTP server. Constraint: The fix must work with Bun-built CLI binaries on macOS without adding dependencies Rejected: Keep using Bun.spawn(...).unref() without detached sessioning | daemon and Chromium still died after the launching shell exited Rejected: POSIX Node launcher | fixed the symptom locally but added an unnecessary Node runtime dependency 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 2090f3d

3 files changed

Lines changed: 131 additions & 22 deletions

File tree

browse/src/cli.ts

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,13 @@ 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+
97104
// ─── State File ────────────────────────────────────────────────
98105
function readState(): ServerState | null {
99106
try {
@@ -231,9 +238,11 @@ async function startServer(extraEnv?: Record<string, string>): Promise<ServerSta
231238
`${extraEnvStr})}).unref()`;
232239
Bun.spawnSync(['node', '-e', launcherCode], { stdio: ['ignore', 'ignore', 'ignore'] });
233240
} else {
234-
// macOS/Linux: Bun.spawn + unref works correctly
241+
// POSIX: spawn the Bun server in its own detached session/process group so
242+
// the daemon and Chromium survive the short-lived CLI shell.
235243
proc = Bun.spawn(['bun', 'run', SERVER_SCRIPT], {
236-
stdio: ['ignore', 'pipe', 'pipe'],
244+
detached: true,
245+
stdio: detachedServerSpawnStdio(),
237246
env: { ...process.env, BROWSE_STATE_FILE: config.stateFile, BROWSE_PARENT_PID: parentPid, ...extraEnv },
238247
});
239248
proc.unref();
@@ -251,27 +260,16 @@ async function startServer(extraEnv?: Record<string, string>): Promise<ServerSta
251260
await Bun.sleep(100);
252261
}
253262

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;
263+
// Server didn't start in time — read the startup error log if the server
264+
// wrote one. This works for detached launches on every platform.
265+
const errorLogPath = path.join(config.stateDir, 'browse-startup-error.log');
266+
try {
267+
const errorLog = fs.readFileSync(errorLogPath, 'utf-8').trim();
268+
if (errorLog) {
269+
throw new Error(`Server failed to start:\n${errorLog}`);
274270
}
271+
} catch (e: any) {
272+
if (e.code !== 'ENOENT') throw e;
275273
}
276274
throw new Error(`Server failed to start within ${MAX_START_WAIT / 1000}s`);
277275
}

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: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { describe, expect, test } from 'bun:test';
2+
import { spawnSync } from 'child_process';
3+
import * as path from 'path';
4+
5+
const ROOT = path.resolve(import.meta.dir, '..');
6+
const BROWSE_BIN = path.join(ROOT, 'browse', 'dist', 'browse');
7+
8+
describe('browse CLI daemon lifecycle', () => {
9+
test('reuses the same daemon across separate invocations', () => {
10+
const script = `
11+
set -euo pipefail
12+
TMP=$(mktemp -d)
13+
STATE="$TMP/browse.json"
14+
PORTFILE="$TMP/port"
15+
cleanup() {
16+
if [ -f "$STATE" ]; then
17+
PID=$(python3 - <<'PY' "$STATE"
18+
import json, sys
19+
print(json.load(open(sys.argv[1]))["pid"])
20+
PY
21+
)
22+
kill "$PID" 2>/dev/null || true
23+
fi
24+
if [ -f "$TMP/http.pid" ]; then
25+
kill "$(cat "$TMP/http.pid")" 2>/dev/null || true
26+
fi
27+
rm -rf "$TMP"
28+
}
29+
trap cleanup EXIT
30+
31+
python3 - <<'PY' "$PORTFILE" >/tmp/gstack-http-test.log 2>&1 &
32+
import http.server, socketserver, sys
33+
class H(http.server.BaseHTTPRequestHandler):
34+
def do_GET(self):
35+
self.send_response(200)
36+
self.send_header('Content-Type', 'text/html; charset=utf-8')
37+
self.end_headers()
38+
self.wfile.write(b'<!doctype html><title>daemon test</title><h1>daemon test</h1>')
39+
def log_message(self, *args):
40+
pass
41+
with socketserver.TCPServer(('127.0.0.1', 0), H) as s:
42+
open(sys.argv[1], 'w').write(str(s.server_address[1]))
43+
s.serve_forever()
44+
PY
45+
echo $! > "$TMP/http.pid"
46+
47+
while [ ! -f "$PORTFILE" ]; do sleep 0.1; done
48+
PORT=$(cat "$PORTFILE")
49+
50+
cd "$TMP"
51+
CI=1 BROWSE_STATE_FILE="$STATE" BROWSE_PORT=0 "${BROWSE_BIN}" goto "http://127.0.0.1:$PORT" >"$TMP/out1" 2>"$TMP/err1"
52+
PID1=$(python3 - <<'PY' "$STATE"
53+
import json, sys
54+
print(json.load(open(sys.argv[1]))["pid"])
55+
PY
56+
)
57+
CI=1 BROWSE_STATE_FILE="$STATE" BROWSE_PORT=0 "${BROWSE_BIN}" url >"$TMP/out2" 2>"$TMP/err2"
58+
PID2=$(python3 - <<'PY' "$STATE"
59+
import json, sys
60+
print(json.load(open(sys.argv[1]))["pid"])
61+
PY
62+
)
63+
64+
printf 'PID1=%s\\n' "$PID1"
65+
printf 'PID2=%s\\n' "$PID2"
66+
printf 'ERR1=%s\\n' "$(cat "$TMP/err1")"
67+
printf 'ERR2=%s\\n' "$(cat "$TMP/err2")"
68+
printf 'OUT2=%s\\n' "$(cat "$TMP/out2")"
69+
`;
70+
71+
const result = spawnSync('bash', ['-lc', script], {
72+
cwd: ROOT,
73+
encoding: 'utf-8',
74+
stdio: ['ignore', 'pipe', 'pipe'],
75+
timeout: 45000,
76+
});
77+
78+
if (result.error) throw result.error;
79+
expect(result.status).toBe(0);
80+
81+
const stdout = result.stdout;
82+
expect(stdout).toContain('ERR1=[browse] Starting server...');
83+
expect(stdout).toContain('ERR2=');
84+
expect(stdout).not.toContain('ERR2=[browse] Starting server...');
85+
expect(stdout).toMatch(/PID1=(\d+)/);
86+
expect(stdout).toMatch(/PID2=(\d+)/);
87+
expect(stdout).toContain('OUT2=http://127.0.0.1:');
88+
89+
const pid1 = stdout.match(/PID1=(\d+)/)?.[1];
90+
const pid2 = stdout.match(/PID2=(\d+)/)?.[1];
91+
expect(pid1).toBe(pid2);
92+
}, 45_000);
93+
});

0 commit comments

Comments
 (0)