diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 4b378cc4f..65597ce45 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -195,6 +195,15 @@ export class BrowserManager { // ─── Headed State ──────────────────────────────────────── private connectionMode: 'launched' | 'headed' = 'launched'; + + /** + * Fired when a RUNNING daemon is promoted to headed mode (see handoff()), + * as opposed to starting headed. The server uses it to cancel the + * parent-process watchdog, which was registered on the assumption that mode + * is fixed at boot and would otherwise kill the freshly handed-off browser + * the next time the spawning shell exits. + */ + onHeadedPromotion?: () => void; private intentionalDisconnect = false; // ─── Tab Count Guardrail (D5 + Codex single-tab flag) ─────── @@ -1603,6 +1612,14 @@ export class BrowserManager { this.tabSessions.clear(); this.connectionMode = 'headed'; + // Promotion, not a headed boot. The server registered a parent-process + // watchdog because this daemon started headless, and that watchdog kills + // headed daemons when their parent exits — which for a CLI-spawned daemon + // is immediately. Without this the handed-off browser dies ~15s later, + // taking whatever the user was mid-way through (a login, an MFA prompt) + // with it. + this.onHeadedPromotion?.(); + // Same Layer C stealth as launch()/launchHeaded(). Must run BEFORE // restoreState() navigates so the init scripts apply to the restored // pages — without this the handed-off browser had cmdline args but no diff --git a/browse/src/server.ts b/browse/src/server.ts index 7bf5f439c..42130df84 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -696,9 +696,14 @@ const BROWSE_PARENT_PID = parseInt(process.env.BROWSE_PARENT_PID || '0', 10); // the closure every 15s. The CLI's connect path sets BROWSE_HEADED=1 + PID=0, // so this branch is the normal path for /open-gstack-browser. const IS_HEADED_WATCHDOG = process.env.BROWSE_HEADED === '1'; +// Kept so a runtime promotion to headed can cancel it. The env guards above only +// cover daemons that were headed at BOOT; `handoff` promotes a running headless +// daemon in place, and the watchdog registered here would then kill it on the +// next parent death. See clearParentWatchdog() below. +let parentWatchdogTimer: ReturnType | null = null; if (BROWSE_PARENT_PID > 0 && !IS_HEADED_WATCHDOG) { let parentGone = false; - setInterval(() => { + parentWatchdogTimer = setInterval(() => { try { process.kill(BROWSE_PARENT_PID, 0); // signal 0 = existence check only, no signal sent } catch { @@ -730,6 +735,28 @@ if (BROWSE_PARENT_PID > 0 && !IS_HEADED_WATCHDOG) { console.log('[browse] Parent-process watchdog disabled (BROWSE_PARENT_PID=0)'); } +/** + * Cancel the parent-process watchdog after a runtime promotion to headed mode. + * + * The watchdog's contract is "headless daemons outlive their parent, headed ones + * do not" — reasonable at boot, when mode is fixed by env. `handoff` breaks that + * assumption: it swaps in a headed context on a RUNNING daemon + * (browser-manager.ts, connectionMode = 'headed') without a restart, so a daemon + * that legitimately registered a watchdog is suddenly on the fatal side of the + * branch. The parent is typically a short-lived shell — Claude Code's Bash tool + * kills one after every invocation — so the next 15s poll shuts the daemon down, + * discarding whatever the user was handed off to do, such as a login. + * + * Once promoted, the user owns the window lifecycle exactly as if the daemon had + * been started headed, which is the case the env guards already exempt. + */ +function clearParentWatchdog(): void { + if (!parentWatchdogTimer) return; + clearInterval(parentWatchdogTimer); + parentWatchdogTimer = null; + console.log('[browse] Parent-process watchdog cleared (promoted to headed at runtime)'); +} + // ─── Command Sets (from commands.ts — single source of truth) ─── import { READ_COMMANDS, WRITE_COMMANDS, META_COMMANDS } from './commands'; export { READ_COMMANDS, WRITE_COMMANDS, META_COMMANDS }; @@ -772,6 +799,11 @@ function emitInspectorEvent(event: any): void { // ─── Server ──────────────────────────────────────────────────── const browserManager = new BrowserManager(); +// Declared here rather than beside clearParentWatchdog: that function sits with +// the watchdog it cancels, which is above this line, and binding it up there +// would touch `browserManager` in its temporal dead zone — aborting module +// evaluation and leaving every later const uninitialized. +browserManager.onHeadedPromotion = clearParentWatchdog; // Indirection for embedders. Module-level handlers (idleCheckTick, parent // watchdog, SIGTERM) read activeBrowserManager so that buildFetchHandler can // retarget them at a caller-supplied BrowserManager. Symmetric with the @@ -1651,6 +1683,11 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle { // after 30 min of HTTP idle because the dead module-level instance still // reports connectionMode === 'launched'. activeBrowserManager = cfgBrowserManager; + // Same reason as above: the watchdog reads activeBrowserManager, so the + // instance that can promote itself to headed must be the one that can cancel + // it. An embedder-supplied manager otherwise promotes silently and the + // watchdog keeps running against a mode it can no longer see. + cfgBrowserManager.onHeadedPromotion = clearParentWatchdog; // Wire the cfg-instance's onDisconnect to run shutdown when the user // closes the headed browser window. CHAIN any caller-provided handler diff --git a/browse/test/watchdog.test.ts b/browse/test/watchdog.test.ts index 42faa262a..15ce8e096 100644 --- a/browse/test/watchdog.test.ts +++ b/browse/test/watchdog.test.ts @@ -155,3 +155,43 @@ describe('parent-process watchdog (v0.18.1.0)', () => { expect(isProcessAlive(serverPid)).toBe(true); }, 45_000); }); + +// The three tests above all fix the mode via env at SPAWN time, so none of them +// reaches the headed branch of the watchdog. That branch is only reachable by a +// RUNTIME promotion, which `handoff` performs: it swaps in a headed context on a +// running daemon without a restart, moving a daemon that legitimately registered +// a watchdog onto the fatal side of the check. The parent is usually a +// short-lived shell (Claude Code's Bash tool kills one after every invocation), +// so the next poll shut the daemon down and discarded whatever the user had been +// handed off to do — observed as repeated session loss mid-login. +// +// Driving a real `handoff` needs a headed Chromium, which does not belong in the +// free tier, so this pins the WIRING instead — the same static-tripwire approach +// used by cdp-session-cleanup.test.ts and server-auth.test.ts. If either half of +// the contract is dropped, the crash returns silently and these fail. +describe('watchdog is cancelled on runtime promotion to headed', () => { + const read = (rel: string) => fs.readFileSync(path.join(ROOT, rel), 'utf-8'); + + test('handoff() notifies the server that it promoted the daemon', () => { + const src = read('src/browser-manager.ts'); + const promote = src.indexOf("this.connectionMode = 'headed';", src.indexOf('async handoff(')); + expect(promote).toBeGreaterThan(-1); + // The notification must follow the promotion closely; a call left far away + // (or removed) is the regression this guards. + expect(src.slice(promote, promote + 800)).toContain('this.onHeadedPromotion?.()'); + }); + + test('the server binds that callback to the watchdog canceller', () => { + const src = read('src/server.ts'); + // The timer must be reachable — `setInterval(` with its return value dropped + // cannot be cleared, which was the original defect. + expect(src).toContain('parentWatchdogTimer = setInterval('); + expect(src).toContain('function clearParentWatchdog()'); + expect(src).toContain('clearInterval(parentWatchdogTimer)'); + // Bound on BOTH the module-level manager and any embedder-supplied one; the + // watchdog reads activeBrowserManager, so binding only the default instance + // leaves embedders (e.g. gbrowser) promoting silently. + expect(src).toContain('browserManager.onHeadedPromotion = clearParentWatchdog'); + expect(src).toContain('cfgBrowserManager.onHeadedPromotion = clearParentWatchdog'); + }); +});