mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-11 23:49:01 +02:00
fix(browse): daemon owns signal policy — handleSIG*:false at launch sites + SIGHUP shutdown (#2220)
Playwright's default handleSIGINT/handleSIGTERM/handleSIGHUP handlers close Chromium the moment the DAEMON process receives a signal — which fights the deliberate headless SIGTERM-ignore in server.ts (Claude Code's Bash sandbox fires SIGTERM when the parent shell exits between tool invocations; the daemon survives it by design, but Playwright's handler killed its browser out from under it). All three flags are now false at all three launch sites (headless launch, headed launchPersistentContext, handoff relaunch). ENG-OV4: the daemon had NO process-level SIGHUP handler (only SIGINT and the mode-aware SIGTERM handler), so flipping handleSIGHUP:false alone would remove the ONLY Chromium cleanup on hangup. server.ts now routes SIGHUP to activeShutdown — the same shutdown path SIGINT uses (closes Chromium, releases ports, removes the state file). Static tripwire (browse/test/launch-signal-flags.test.ts, house grep-style): every chromium.launch/launchPersistentContext site must carry the three flags (site count pinned at 3 so a NEW launch site trips it), server.ts must keep the SIGHUP→activeShutdown route, and the deliberate headless SIGTERM-ignore must still exist (the reason handleSIGTERM:false is safe — pinned in the test's header comment). Tests: launch-signal-flags 3 pass; browser-manager-unit 28 pass; bridge-chromium-e2e real-launch smoke 3 pass. Fixes #2220. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
822de7d0c3
commit
25ccda996d
@@ -466,6 +466,16 @@ export class BrowserManager {
|
||||
// Playwright cache (no executablePath), so the heal is never scoped out.
|
||||
this.browser = await launchWithXProtectHeal(() => chromium.launch({
|
||||
headless: useHeadless,
|
||||
// #2220: the daemon owns signal policy, not Playwright. Playwright's
|
||||
// default handlers close Chromium the moment THIS process receives
|
||||
// SIGINT/SIGTERM/SIGHUP — which fights the deliberate headless
|
||||
// SIGTERM-ignore in server.ts (the daemon survives the signal but
|
||||
// loses its browser out from under it). All three are false; server.ts
|
||||
// routes the signals it actually honors through activeShutdown, which
|
||||
// closes Chromium itself.
|
||||
handleSIGINT: false,
|
||||
handleSIGTERM: false,
|
||||
handleSIGHUP: false,
|
||||
// On Windows, Chromium's sandbox fails when the server is spawned through
|
||||
// the Bun→Node process chain (GitHub #276). Disable it — local daemon
|
||||
// browsing user-specified URLs has marginal sandbox benefit. Also disabled
|
||||
@@ -663,6 +673,10 @@ export class BrowserManager {
|
||||
// reinstalled over (probePoisonedChromiumBundle's scope contract).
|
||||
this.context = await launchWithXProtectHeal(() => chromium.launchPersistentContext(userDataDir, {
|
||||
headless: false,
|
||||
// #2220: daemon owns signal policy — see launch() for the rationale.
|
||||
handleSIGINT: false,
|
||||
handleSIGTERM: false,
|
||||
handleSIGHUP: false,
|
||||
// Match the sandbox policy used by launch() above. Without this,
|
||||
// Playwright auto-adds --no-sandbox on every headed launch and the user
|
||||
// sees Chromium's "unsupported command-line flag" yellow infobar.
|
||||
@@ -1717,6 +1731,10 @@ export class BrowserManager {
|
||||
// exactly as in launch()/launchHeaded().
|
||||
newContext = await launchWithXProtectHeal(() => chromium.launchPersistentContext(userDataDir, {
|
||||
headless: false,
|
||||
// #2220: daemon owns signal policy — see launch() for the rationale.
|
||||
handleSIGINT: false,
|
||||
handleSIGTERM: false,
|
||||
handleSIGHUP: false,
|
||||
// Match the sandbox policy used by launchHeaded() / launch(). The
|
||||
// handoff path is the headless→headed re-launch and shares the same
|
||||
// anti-detection posture, including no spurious --no-sandbox infobar.
|
||||
|
||||
@@ -1493,6 +1493,13 @@ async function handleCommand(body: any, tokenInfo?: TokenInfo | null): Promise<R
|
||||
if (import.meta.main) {
|
||||
// SIGINT (Ctrl+C): user intentionally stopping → shutdown.
|
||||
process.on('SIGINT', () => activeShutdown?.());
|
||||
// SIGHUP (terminal hangup): with handleSIGHUP:false at the three launch
|
||||
// sites (#2220), Playwright no longer closes Chromium when this process
|
||||
// gets hung up on — this handler is now the ONLY Chromium cleanup on
|
||||
// SIGHUP (ENG-OV4). Route to the same shutdown path as SIGINT:
|
||||
// activeShutdown closes the browser, releases ports, and removes the
|
||||
// state file. Without it, a hangup would leak a live Chromium.
|
||||
process.on('SIGHUP', () => activeShutdown?.());
|
||||
// SIGTERM behavior depends on mode:
|
||||
// - Normal (headless) mode: Claude Code's Bash sandbox fires SIGTERM when the
|
||||
// parent shell exits between tool invocations. Ignoring it keeps the server
|
||||
|
||||
Reference in New Issue
Block a user