fix(browse): cancel the parent watchdog when handoff promotes a daemon to headed

The parent-process watchdog assumes connection mode is fixed at boot: headless
daemons outlive their parent, headed ones do not. The env guards
(BROWSE_PARENT_PID=0, BROWSE_HEADED=1) only cover daemons that were headed when
they started.

handoff breaks that assumption. It swaps in a headed context on a RUNNING daemon
and sets connectionMode = 'headed' without a restart, so a daemon that
legitimately registered a watchdog lands on the fatal side of the branch. The
parent is usually a short-lived shell, and Claude Code's Bash tool kills one after
every invocation, so the next 15s poll shuts the daemon down.

The user-visible effect is that handoff destroys the thing it just created. It
exists so a human can log in, solve a CAPTCHA, or clear an MFA prompt; the browser
disappears about fifteen seconds later and takes the session with it. Observed
while driving two registrar control panels: five daemon deaths and three logins,
each one discarding the authenticated session.

BrowserManager now exposes onHeadedPromotion, fired only on runtime promotion and
not on a headed boot, and the server binds it to a canceller for the interval it
already owned but previously discarded. Bound on both the module-level manager and
any embedder-supplied one, since the watchdog reads activeBrowserManager and
binding only the default would let embedders promote silently.

The binding sits next to the browserManager declaration rather than next to
clearParentWatchdog. Placing it with the function, which lives with the watchdog it
cancels, reads better but touches browserManager in its temporal dead zone, which
aborts module evaluation and leaves every later const uninitialized. findport
tests catch that immediately.

Tests: watchdog.test.ts already noted in its header that its three cases all fix
mode via env at spawn time, so none reaches the headed branch. Driving a real
handoff needs a headed Chromium, so the wiring is pinned with static tripwires
instead, matching cdp-session-cleanup.test.ts and server-auth.test.ts. Verified
they fail when the notification call is removed and pass when restored.

Full `bun test` shows the same 6 pre-existing failures on this branch and on main
(gstack-gbrain-detect, gstack-artifacts-init), which pass in isolation on both, so
they are test-order pollution rather than a regression here.
This commit is contained in:
Shawn Reddy
2026-08-14 12:44:16 -07:00
committed by Garry Tan
parent 39bc1c34f6
commit f3ea49ff6e
3 changed files with 95 additions and 1 deletions
+40
View File
@@ -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');
});
});