diff --git a/browse/src/cli.ts b/browse/src/cli.ts index ed6703901..75d63db1d 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -961,6 +961,14 @@ async function handlePairAgent(state: ServerState, args: string[]): Promise { headedParentShutdownSuppressed = false; parentGone = false; }, setTunnelActive: (v: boolean) => { tunnelActive = v; }, setLastActivity: (t: number) => { lastActivity = t; }, formatExplicitPortUnavailableError, @@ -700,39 +706,49 @@ 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; - parentWatchdogTimer = setInterval(() => { - try { - process.kill(BROWSE_PARENT_PID, 0); // signal 0 = existence check only, no signal sent - } catch { - // Parent exited. Resolution order: - // 1. Active cookie picker (one-time code or session live)? Stay alive - // regardless of mode — tearing down the server mid-import leaves the - // picker UI with a stale "Failed to fetch" error. - // 2. Headed / tunnel mode? Shutdown. The idle timeout doesn't apply in - // these modes (see idleCheckInterval above — both early-return), so - // ignoring parent death here would leak orphan daemons after - // /pair-agent or /open-gstack-browser sessions. - // 3. Normal (headless) mode? Stay alive. Claude Code's Bash tool kills - // the parent shell between invocations. The idle timeout (30 min) - // handles eventual cleanup. - if (hasActivePicker()) return; - const headed = activeBrowserManager.getConnectionMode() === 'headed'; - if (headed || tunnelActive) { - console.log(`[browse] Parent process ${BROWSE_PARENT_PID} exited in ${headed ? 'headed' : 'tunnel'} mode, shutting down`); - activeShutdown?.(); - } else if (!parentGone) { - parentGone = true; - console.log(`[browse] Parent process ${BROWSE_PARENT_PID} exited (server stays alive, idle timeout will clean up)`); - } +// Runtime promotion to headed (`handoff`) must NOT clear this interval — the +// same tick is the tunnel-orphan reaper, and idle timeout is disabled in +// tunnel mode, so parent death is the ONLY thing that reaps an +// internet-exposed daemon after handoff → resume → /pair-agent. Promotion +// sets this suppress flag instead; the tick re-reads it (and tunnelActive) +// every pass. See suppressHeadedParentShutdown() below. +let headedParentShutdownSuppressed = false; +// Latch for the one-time "parent exited, staying alive" log line. +let parentGone = false; +// Named + parameterized (default: the boot-time env PID) so watchdog.test.ts +// can drive the tick deterministically via __testInternals__, mirroring +// idleCheckTick above. setInterval invokes it with no args in production. +function parentWatchdogTick(parentPid: number = BROWSE_PARENT_PID): void { + try { + process.kill(parentPid, 0); // signal 0 = existence check only, no signal sent + } catch { + // Parent exited. Resolution order: + // 1. Active cookie picker (one-time code or session live)? Stay alive + // regardless of mode — tearing down the server mid-import leaves the + // picker UI with a stale "Failed to fetch" error. + // 2. Headed (unless suppressed by a runtime promotion) / tunnel mode? + // Shutdown. The idle timeout doesn't apply in these modes (see + // idleCheckInterval above — both early-return), so ignoring parent + // death here would leak orphan daemons after /pair-agent or + // /open-gstack-browser sessions. + // 3. Normal (headless) mode, or headed-by-promotion? Stay alive. Claude + // Code's Bash tool kills the parent shell between invocations, and a + // promoted daemon's user owns the window lifecycle. The idle timeout + // (30 min) handles eventual cleanup. + if (hasActivePicker()) return; + const headed = activeBrowserManager.getConnectionMode() === 'headed' + && !headedParentShutdownSuppressed; + if (headed || tunnelActive) { + console.log(`[browse] Parent process ${parentPid} exited in ${headed ? 'headed' : 'tunnel'} mode, shutting down`); + activeShutdown?.(); + } else if (!parentGone) { + parentGone = true; + console.log(`[browse] Parent process ${parentPid} exited (server stays alive, idle timeout will clean up)`); } - }, 15_000); + } +} +if (BROWSE_PARENT_PID > 0 && !IS_HEADED_WATCHDOG) { + setInterval(parentWatchdogTick, 15_000); } else if (IS_HEADED_WATCHDOG) { console.log('[browse] Parent-process watchdog disabled (headed mode)'); } else if (BROWSE_PARENT_PID === 0) { @@ -740,7 +756,7 @@ if (BROWSE_PARENT_PID > 0 && !IS_HEADED_WATCHDOG) { } /** - * Cancel the parent-process watchdog after a runtime promotion to headed mode. + * Suppress the headed-mode parent-death shutdown after a runtime promotion. * * 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 @@ -753,12 +769,18 @@ if (BROWSE_PARENT_PID > 0 && !IS_HEADED_WATCHDOG) { * * 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. + * + * A flag, NOT clearInterval: the tick doubles as the tunnel-orphan reaper + * (its `tunnelActive` branch), and idle timeout is disabled in tunnel mode — + * clearing the whole interval here left handoff → resume → /pair-agent with + * an internet-exposed daemon nothing could ever reap. After promotion, parent + * death no longer kills the daemon for BEING HEADED, but still kills it when + * a tunnel is active. */ -function clearParentWatchdog(): void { - if (!parentWatchdogTimer) return; - clearInterval(parentWatchdogTimer); - parentWatchdogTimer = null; - console.log('[browse] Parent-process watchdog cleared (promoted to headed at runtime)'); +function suppressHeadedParentShutdown(): void { + if (headedParentShutdownSuppressed) return; + headedParentShutdownSuppressed = true; + console.log('[browse] Parent-death headed shutdown suppressed (promoted to headed at runtime); watchdog stays armed as the tunnel-orphan reaper'); } // ─── Command Sets (from commands.ts — single source of truth) ─── @@ -803,11 +825,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; +// Declared here rather than beside suppressHeadedParentShutdown: that function +// sits with the watchdog it gates, 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 = suppressHeadedParentShutdown; // 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 @@ -826,6 +848,11 @@ let activeBrowserManager: BrowserManager = browserManager; // any buildFetchHandler call rebinds onDisconnect onto the cfg instance. browserManager.onDisconnect = (code) => activeShutdown?.(code ?? 2); let isShuttingDown = false; +// Session-persist ticker handle. Registered in start() (module scope so the +// factory's shutdown() can reach it), cleared by shutdown() BEFORE the final +// snapshot — a tick landing during browser teardown would otherwise overwrite +// the good final snapshot with a degraded one (zero tabs). +let sessionPersistInterval: ReturnType | null = null; type PortCheckResult = | { available: true } @@ -1656,16 +1683,31 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle { clearInterval(flushInterval); clearInterval(idleCheckInterval); if (agentWatchdogInterval) clearInterval(agentWatchdogInterval); + // Stop the session-persist ticker BEFORE the final snapshot below — + // paired with the isShuttingDown gate inside the tick, this guarantees + // no interval snapshot can race the final one during teardown. + if (sessionPersistInterval) { + clearInterval(sessionPersistInterval); + sessionPersistInterval = null; + } await flushBuffers(); // Final session snapshot before the browser goes away (#778). Best - // effort: shutdown must never hang on a wedged page.evaluate. + // effort with a hard 2s deadline: shutdown must never hang on a wedged + // page.evaluate — after the deadline we proceed to browser close and let + // the previous interval snapshot stand (atomic writes guarantee it's + // intact). The .catch is attached to the persist promise itself so a + // late rejection after losing the race can't become an unhandled + // rejection. if (isSessionPersistEnabled()) { - try { - await persistSessionState(cfgBrowserManager, path.join(config.stateDir, SESSION_STATE_FILE)); - } catch (err: any) { - console.warn(`[browse] SESSION_PERSIST_FAILED at shutdown: ${err?.message ?? err}`); - } + const finalSnapshot = persistSessionState(cfgBrowserManager, path.join(config.stateDir, SESSION_STATE_FILE)) + .catch((err: any) => { + console.warn(`[browse] SESSION_PERSIST_FAILED at shutdown: ${err?.message ?? err}`); + }); + await Promise.race([ + finalSnapshot, + new Promise((resolve) => setTimeout(resolve, 2_000)), + ]); } await cfgBrowserManager.close(); @@ -1698,10 +1740,11 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle { // 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; + // instance that can promote itself to headed must be the one that can + // suppress the headed parent-death branch. An embedder-supplied manager + // otherwise promotes silently and the watchdog keeps shutting down on a + // promotion it can no longer see. + cfgBrowserManager.onHeadedPromotion = suppressHeadedParentShutdown; // Wire the cfg-instance's onDisconnect to run shutdown when the user // closes the headed browser window. CHAIN any caller-provided handler @@ -3092,36 +3135,6 @@ export async function start() { } else { await browserManager.launch(); } - - // ─── Opt-in session persistence (#778 class) ───────────────── - // BROWSE_PERSIST_STATE=1: restore cookies/storage/tabs from the last - // snapshot, then keep snapshotting on an interval. Launched mode only — - // the headed persistent profile owns its own state. The final snapshot - // at clean shutdown lives in buildFetchHandler's shutdown(). - if (isSessionPersistEnabled() && browserManager.getConnectionMode() === 'launched') { - const sessionStatePath = path.join(config.stateDir, SESSION_STATE_FILE); - try { - if (await restoreSessionState(browserManager, sessionStatePath)) { - const st = await browserManager.saveState(); - console.log(`[browse] Session state restored: ${st.cookies.length} cookies / ${st.pages.length} tabs (BROWSE_PERSIST_STATE=1)`); - } else { - console.log('[browse] Session persistence on; no prior state — fresh session (BROWSE_PERSIST_STATE=1)'); - } - } catch (err: any) { - console.warn(`[browse] SESSION_RESTORE_FAILED: ${err?.message ?? err}`); - } - let persistWarned = false; - setInterval(() => { - persistSessionState(browserManager, sessionStatePath).catch((err: any) => { - // Warn once — a full disk must not spam the log every 30s, and a - // snapshot failure must never kill the daemon (R3). - if (!persistWarned) { - persistWarned = true; - console.warn(`[browse] SESSION_PERSIST_FAILED: ${err?.message ?? err} (further failures suppressed)`); - } - }); - }, sessionPersistIntervalMs()).unref(); - } } const startTime = Date.now(); @@ -3168,6 +3181,58 @@ export async function start() { browserManager.serverPort = port; + // ─── Opt-in session persistence (#778 class) ───────────────── + // BROWSE_PERSIST_STATE=1: restore cookies/storage/tabs from the last + // snapshot, then keep snapshotting on an interval. Launched mode only — + // the headed persistent profile owns its own state. The final snapshot at + // clean shutdown lives in buildFetchHandler's shutdown(). + // + // Runs AFTER Bun.serve() + the state-file write, in the BACKGROUND: + // restore re-creates tabs sequentially with up-to-15s goto timeouts while + // the CLI's readiness probe gives up at 8s — one slow/unreachable saved + // URL must never make every `$B` command report "Server failed to start". + // Fire-and-forget: a restore failure is logged and never affects the + // daemon. + if (!skipBrowser && isSessionPersistEnabled() && browserManager.getConnectionMode() === 'launched') { + const sessionStatePath = path.join(config.stateDir, SESSION_STATE_FILE); + restoreSessionState(browserManager, sessionStatePath) + .then((restored) => { + if (restored) { + // Counts come from the deserialized snapshot itself — no extra + // saveState() round-trip against pages that may still be loading. + console.log(`[browse] Session state restored: ${restored.cookies.length} cookies / ${restored.pages.length} tabs (BROWSE_PERSIST_STATE=1)`); + } else { + console.log('[browse] Session persistence on; no prior state — fresh session (BROWSE_PERSIST_STATE=1)'); + } + }) + .catch((err: any) => { + console.warn(`[browse] SESSION_RESTORE_FAILED: ${err?.message ?? err}`); + }); + let persistWarned = false; + // In-flight guard: never start a new snapshot while the previous one is + // still pending (a slow page.evaluate would otherwise pile up ticks). + let persistInFlight = false; + sessionPersistInterval = setInterval(() => { + // Shutdown gate (belt; shutdown()'s clearInterval is the suspenders): + // a tick that fires during browser teardown snapshots a degraded state + // (zero tabs) over the good final snapshot. + if (isShuttingDown) return; + if (persistInFlight) return; // skip the tick + persistInFlight = true; + persistSessionState(browserManager, sessionStatePath) + .catch((err: any) => { + // Warn once — a full disk must not spam the log every 30s, and a + // snapshot failure must never kill the daemon (R3). + if (!persistWarned) { + persistWarned = true; + console.warn(`[browse] SESSION_PERSIST_FAILED: ${err?.message ?? err} (further failures suppressed)`); + } + }) + .finally(() => { persistInFlight = false; }); + }, sessionPersistIntervalMs()); + (sessionPersistInterval as any)?.unref?.(); + } + // Navigate to welcome page if in headed mode and still on about:blank if (browserManager.getConnectionMode() === 'headed') { try { diff --git a/browse/test/pair-agent-optin-gate.test.ts b/browse/test/pair-agent-optin-gate.test.ts index 7220a16cb..cd795a989 100644 --- a/browse/test/pair-agent-optin-gate.test.ts +++ b/browse/test/pair-agent-optin-gate.test.ts @@ -95,6 +95,22 @@ describe('gate wiring — every tunnel activation point consults the guard', () expect(CLI_SRC).toContain('const ngrokAvailable = pairEnabled && isNgrokAvailable();'); }); + test('CLI consent-off branch names the real remedy, never ngrok reinstall', () => { + // When pair_agent is off but ngrok is installed+authed, telling the user + // to `ngrok config add-authtoken` can never fix it — the gate is consent, + // not tooling. The consent branch must carry the same remedy wording as + // the /tunnel/start 403 body, and must not mention ngrok setup. + const branchAt = CLI_SRC.indexOf('} else if (!pairEnabled) {'); + expect(branchAt).toBeGreaterThan(-1); + const branchEnd = CLI_SRC.indexOf('} else {', branchAt); + expect(branchEnd).toBeGreaterThan(branchAt); + const branch = CLI_SRC.slice(branchAt, branchEnd); + expect(branch).toContain('gstack-config set pair_agent on'); + expect(branch).toContain('/pair-agent'); + expect(branch).not.toContain('ngrok config add-authtoken'); + expect(branch).not.toContain('install ngrok'); + }); + test('/tunnel/start refuses with the enable hint when disabled', () => { const startIdx = SERVER_SRC.indexOf("url.pathname === '/tunnel/start'"); const block = SERVER_SRC.slice(startIdx, startIdx + 1200); diff --git a/browse/test/server-lock-errors.test.ts b/browse/test/server-lock-errors.test.ts index a60afebb3..e62b28b3c 100644 --- a/browse/test/server-lock-errors.test.ts +++ b/browse/test/server-lock-errors.test.ts @@ -15,6 +15,10 @@ import { describe, test, expect, afterAll } from 'bun:test'; import * as fs from 'fs'; +// Default (CJS) export — its properties are mutable in Bun, unlike the frozen +// `* as fs` namespace, and mutations propagate to cli.ts's own fs import. +// Used only for the depth-cap livelock simulations below (restored in finally). +import fsMutable from 'fs'; import * as os from 'os'; import * as path from 'path'; import { acquireServerLock, ServerLockError } from '../src/cli'; @@ -77,4 +81,43 @@ describe('acquireServerLock (#1084 error honesty)', () => { expect(acquireServerLock(lockPath)).toBeNull(); fs.unlinkSync(lockPath); }); + + test('EEXIST + garbage lockfile content: NaN pid is treated as stale, lock acquired', () => { + const lockPath = path.join(tmpRoot, 'garbage.lock'); + fs.writeFileSync(lockPath, 'not-a-pid\n'); // parseInt → NaN → falsy → stale path + const release = acquireServerLock(lockPath); + expect(release).not.toBeNull(); + // Our pid replaced the garbage — the stale lock was removed and re-acquired. + expect(fs.readFileSync(lockPath, 'utf8').trim()).toBe(String(process.pid)); + release!(); + expect(fs.existsSync(lockPath)).toBe(false); + }); + + // NOTE: the "stale lock that survives unlink" livelock variant is deliberately + // not simulated here — the source removes locks through safeUnlink's own fs + // binding, which a test-side fs monkey-patch cannot reliably intercept in Bun. + // The depth cap itself is exercised by the vanish-race test below. + + test('depth cap: holder that vanishes between open and read returns null after 5 retries', () => { + // The EEXIST → readFileSync ENOENT race: the lock exists at openSync but + // is gone by the read (holder released in between). Repeated forever + // (open/release storm), the same depth cap must bound the retry loop. + const lockPath = path.join(tmpRoot, 'vanish.lock'); + fs.writeFileSync(lockPath, `${process.pid}\n`); + const origRead = fsMutable.readFileSync; + try { + (fsMutable as any).readFileSync = (p: fs.PathLike | number, ...rest: unknown[]) => { + if (p === lockPath) { + const e: NodeJS.ErrnoException = new Error('mock: lock vanished before read'); + e.code = 'ENOENT'; + throw e; + } + return (origRead as any)(p, ...rest); + }; + expect(acquireServerLock(lockPath)).toBeNull(); + } finally { + (fsMutable as any).readFileSync = origRead; + fs.unlinkSync(lockPath); + } + }); }); diff --git a/browse/test/watchdog.test.ts b/browse/test/watchdog.test.ts index 15ce8e096..56201779e 100644 --- a/browse/test/watchdog.test.ts +++ b/browse/test/watchdog.test.ts @@ -1,8 +1,12 @@ -import { describe, test, expect, afterEach } from 'bun:test'; +import { describe, test, expect, afterEach, beforeEach, mock } from 'bun:test'; import { spawn, type Subprocess } from 'bun'; import * as path from 'path'; import * as fs from 'fs'; import * as os from 'os'; +import * as crypto from 'crypto'; +import { buildFetchHandler, __testInternals__, type ServerConfig } from '../src/server'; +import { __resetRegistry } from '../src/token-registry'; +import { resolveConfig } from '../src/config'; // End-to-end regression tests for the parent-process watchdog in server.ts. // The watchdog has layered behavior since v0.18.1.0 (#1025) and v0.18.2.0 @@ -18,11 +22,9 @@ import * as os from 'os'; // eventual cleanup. // // Tunnel mode coverage (parent dies → shutdown because idle timeout doesn't -// apply) is not covered by an automated test here — tunnelActive is a runtime -// variable set by /pair-agent's tunnel-create flow, not an env var, so faking -// it would require invasive test-only hooks. The mode check is documented -// inline at the watchdog and SIGTERM handlers, and would regress visibly for -// /pair-agent users (server lingers after disconnect). +// apply) is covered behaviorally in the in-process suite at the bottom of this +// file: the tick is exported via __testInternals__.parentWatchdogTick (same +// seam as idleCheckTick) and tunnelActive is simulated via setTunnelActive. // // Each test spawns the real server.ts. Tests 1 and 2 verify behavior via // stdout log line (fast). Test 3 waits for the watchdog poll cycle to confirm @@ -165,11 +167,18 @@ describe('parent-process watchdog (v0.18.1.0)', () => { // 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. // +// The fix must NOT clear the interval, though: the same tick is the +// tunnel-orphan reaper (idle timeout is disabled in tunnel mode, so parent +// death is the ONLY thing that reaps an internet-exposed daemon). Promotion +// sets a suppress flag the tick re-reads each pass — "being headed" no longer +// kills the daemon on parent death, but an active tunnel still does. +// // 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', () => { +// the contract is dropped, the crash returns silently and these fail. The +// behavioral halves (suppression + tunnel reaping) run in-process below. +describe('headed parent-death shutdown is suppressed on runtime promotion', () => { const read = (rel: string) => fs.readFileSync(path.join(ROOT, rel), 'utf-8'); test('handoff() notifies the server that it promoted the daemon', () => { @@ -181,17 +190,130 @@ describe('watchdog is cancelled on runtime promotion to headed', () => { expect(src.slice(promote, promote + 800)).toContain('this.onHeadedPromotion?.()'); }); - test('the server binds that callback to the watchdog canceller', () => { + test('the server binds that callback to the suppress-flag setter', () => { 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)'); + expect(src).toContain('function suppressHeadedParentShutdown()'); // 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'); + expect(src).toContain('browserManager.onHeadedPromotion = suppressHeadedParentShutdown'); + expect(src).toContain('cfgBrowserManager.onHeadedPromotion = suppressHeadedParentShutdown'); + }); + + test('promotion must NOT clear the interval — the tick doubles as the tunnel-orphan reaper', () => { + const src = read('src/server.ts'); + // The original #2565 absorption cleared the ENTIRE interval on promotion. + // Sequence handoff → resume → /pair-agent tunnel then left an + // internet-exposed daemon that nothing reaps. The tick must stay + // registered and re-check the suppress flag + tunnelActive every pass. + expect(src).not.toContain('clearInterval(parentWatchdogTimer)'); + expect(src).toContain('setInterval(parentWatchdogTick'); + const tickStart = src.indexOf('function parentWatchdogTick('); + expect(tickStart).toBeGreaterThan(-1); + const tick = src.slice(tickStart, src.indexOf('\n}', tickStart)); + expect(tick).toContain('headedParentShutdownSuppressed'); + expect(tick).toContain('tunnelActive'); + }); +}); + +// ─── Behavioral: suppressed watchdog still reaps tunnel orphans ──────────── +// +// In-process, via the same __testInternals__ seam server-factory.test.ts uses +// for idleCheckTick. parentWatchdogTick(deadPid) simulates the 15s poll +// discovering a dead parent; setTunnelActive simulates /pair-agent's +// tunnel-create flow; suppressHeadedParentShutdown is exactly what the +// handoff promotion callback invokes. +function makeMinimalConfig(mode: 'launched' | 'headed', tmpDir: string): ServerConfig { + const base = resolveConfig(); + return { + authToken: 'watchdog-test-' + crypto.randomBytes(16).toString('hex'), + browsePort: 34567, + idleTimeoutMs: 1_800_000, + // State paths pointed at a scratch dir so shutdown()'s cleanup can never + // touch a real daemon's files on the machine running the tests. + config: { ...base, stateFile: path.join(tmpDir, 'browse-state.json'), stateDir: tmpDir }, + browserManager: { + getConnectionMode: () => mode, + isWatching: () => false, + stopWatch: () => {}, + close: async () => {}, + onDisconnect: null, + } as any, + startTime: Date.now(), + // Skip terminal-agent teardown: identity files live under the REAL state + // dir conventions and this suite must stay hermetic. + ownsTerminalAgent: false, + }; +} + +describe('suppressed watchdog still reaps tunnel orphans (behavioral)', () => { + // A PID above darwin/linux default pid_max: process.kill(pid, 0) throws + // ESRCH, which the tick reads as "parent exited". + const DEAD_PID = 999_999; + let scratch: string; + const savedChromiumProfile = process.env.CHROMIUM_PROFILE; + + beforeEach(() => { + scratch = fs.mkdtempSync(path.join(os.tmpdir(), 'watchdog-tick-')); + // shutdown() runs cleanSingletonLocks(resolveChromiumProfile()); point it + // at scratch so the operator's real profile is never inspected. + process.env.CHROMIUM_PROFILE = path.join(scratch, 'chromium-profile'); + __resetRegistry(); + __testInternals__.setTunnelActive(false); + __testInternals__.setLastActivity(Date.now()); + __testInternals__.resetShutdownState(); + __testInternals__.resetParentWatchdogState(); + }); + + afterEach(() => { + if (savedChromiumProfile === undefined) delete process.env.CHROMIUM_PROFILE; + else process.env.CHROMIUM_PROFILE = savedChromiumProfile; + __testInternals__.setTunnelActive(false); + __testInternals__.resetShutdownState(); + __testInternals__.resetParentWatchdogState(); + try { fs.rmSync(scratch, { recursive: true, force: true }); } catch {} + }); + + // Drain the fire-and-forget shutdown promise chain (flushBuffers + close) + // the same way server-factory.test.ts does before asserting on exit. + async function drainShutdown(): Promise { + await Promise.resolve(); + await Promise.resolve(); + await new Promise((r) => setImmediate(r)); + await new Promise((r) => setImmediate(r)); + } + + test('after promotion suppression, parent death does NOT shut down a headed daemon (#2565)', async () => { + const exitMock = mock((_code?: number) => {}); + const originalExit = process.exit; + (process as any).exit = exitMock; + try { + buildFetchHandler(makeMinimalConfig('headed', scratch)); + __testInternals__.suppressHeadedParentShutdown(); // what handoff promotion triggers + __testInternals__.parentWatchdogTick(DEAD_PID); + await drainShutdown(); + expect(exitMock).not.toHaveBeenCalled(); + } finally { + (process as any).exit = originalExit; + } + }); + + test('CRITICAL: suppression active + tunnel live — parent death still shuts down', async () => { + const exitMock = mock((_code?: number) => {}); + const originalExit = process.exit; + (process as any).exit = exitMock; + try { + buildFetchHandler(makeMinimalConfig('headed', scratch)); + __testInternals__.suppressHeadedParentShutdown(); + __testInternals__.setTunnelActive(true); // handoff → resume → /pair-agent tunnel + __testInternals__.parentWatchdogTick(DEAD_PID); + await drainShutdown(); + // The tick is the ONLY reaper for tunnel orphans (idle timeout is + // disabled in tunnel mode). If this fails, an internet-exposed daemon + // outlives its parent forever. + expect(exitMock).toHaveBeenCalled(); + } finally { + (process as any).exit = originalExit; + } }); });