fix: own demo browser lifetime through cross-platform IPC

electron/main.cjs

@@ -64,7 +64,7 @@ function serverEntry() {
 //   <this binary> --browser-service
 // and connects over CDP (system Chrome stays the fallback). Handshake:
 // the helper prints "BROWSER_SERVICE_CDP <port>" on stdout once the CDP
-// endpoint listens. Lifetime: stdin stays open; parent death = EOF = quit.
+// endpoint listens. Lifetime: parent IPC disconnect/death = quit.
 // MUST run before the single-instance lock — a helper is a second
 // instance of this very binary, and before the LH_DEV_URL redirect — the
 // helper owns no user data at all.
@@ -73,14 +73,15 @@ if (process.argv.includes("--browser-service")) {
 	// Own the helper's entire startup lifetime, including the readiness wait.
 	const lifetime = new AbortController();
 	const parentClosed = () => {
-		lifetime.abort(new Error("browser-service parent stdin closed"));
+		lifetime.abort(new Error("browser-service parent IPC disconnected"));
 		app.quit();
 	};
-	process.stdin.resume();
-	process.stdin.on("end", parentClosed);
-	process.stdin.on("close", parentClosed);
 	(async () => {
 		try {
+			// Full Windows Electron replaces stdin with synthetic EOF. Use the
+			// real Node IPC channel, and refuse to start an unowned helper.
+			if (!process.channel || !process.connected) throw new Error("browser-service requires connected parent IPC");
+			process.once("disconnect", parentClosed);
 			app.dock?.hide();
 			const port = await freePort();
 			// Must land before app-ready for Chromium to pick it up.
@@ -95,7 +96,7 @@ if (process.argv.includes("--browser-service")) {
 			await waitForBrowserService(port, { signal: lifetime.signal });
 			if (!lifetime.signal.aborted) console.log(`BROWSER_SERVICE_CDP ${port}`);
 		} catch (err) {
-			if (lifetime.signal.aborted) return; // EOF is a normal shutdown.
+			if (lifetime.signal.aborted) return; // Parent disconnect is normal.
 			console.error("browser-service failed:", err);
 			app.exit(1);
 		}

scripts/smoke-demo-runtime.mjs

@@ -98,6 +98,46 @@ async function importPayload(standalone) {
 	}
 }
 
+// Explicit IPC disconnect can omit ChildProcess 'close' even after Electron
+// exits. Wait for actual exit, boundedly; never infer shutdown intent from
+// Node's automatic stdin cleanup after an unexpected child exit.
+/** @returns {Promise<{code: number | null, signal: NodeJS.Signals | null, requested: boolean}>} */
+export async function stopBrowserService(helper) {
+	if (!helper.pid || helper.exitCode !== null || helper.signalCode !== null) {
+		return { code: helper.exitCode, signal: helper.signalCode, requested: false };
+	}
+	let onExit, onError;
+	const exited = new Promise((resolve, reject) => {
+		onExit = (code, signal) => resolve({ code, signal });
+		onError = reject;
+		helper.once("exit", onExit);
+		helper.once("error", onError);
+	});
+	const wait = async (ms) => {
+		let timer;
+		try {
+			return await Promise.race([exited, new Promise((resolve) => { timer = setTimeout(() => resolve(null), ms); })]);
+		} finally { clearTimeout(timer); }
+	};
+	try {
+		const requested = helper.connected;
+		if (requested) {
+			console.log("[smoke-demo-runtime] requesting parent IPC disconnect");
+			helper.disconnect();
+		}
+		const result = await wait(5000);
+		if (!result) {
+			helper.kill("SIGKILL");
+			if (!await wait(3000)) throw new Error("browser-service did not exit after forced cleanup");
+			throw new Error("browser-service did not exit after IPC disconnect; force-killed");
+		}
+		return { ...result, requested };
+	} finally {
+		helper.removeListener("exit", onExit);
+		helper.removeListener("error", onError);
+	}
+}
+
 async function browserProbe(puppeteer) {
 	assert.ok(process.versions.electron, "--browser requires a packaged Electron --runtime binary");
 	const home = process.env.HOME;
@@ -109,12 +149,12 @@ async function browserProbe(puppeteer) {
 		"--disable-background-networking", "--disable-component-update", "--no-proxy-server",
 		// Hosted Linux runners disallow Chromium's unprivileged sandbox.
 		...(process.platform === "linux" ? ["--no-sandbox"] : []),
-	], { cwd: home, env, stdio: ["pipe", "pipe", "pipe"] });
+	], { cwd: home, env, stdio: ["ignore", "pipe", "pipe", "ipc"] });
 	let output = "";
 	let errors = "";
 	helper.stderr.on("data", (chunk) => { errors = (errors + chunk).slice(-16_000); });
-	const closed = new Promise((resolve) => helper.once("close", (code, signal) => resolve({ code, signal })));
 	let browser;
+	let failed = false;
 	try {
 		const port = await new Promise((resolve, reject) => {
 			const timer = setTimeout(() => reject(new Error("browser-service handshake timed out")), 15_000);
@@ -152,23 +192,22 @@ async function browserProbe(puppeteer) {
 		assert.deepEqual(failures, [], "demo console/page/network errors");
 		console.log("[smoke-demo-runtime] packaged browser-service: JS button 0 → 1, clean console/page, page reset ok (no Chrome fallback)");
 	} catch (err) {
+		failed = true;
 		throw new Error(`browser-service probe failed at local CDP/browser interaction\n--- browser-service output ---\n${output}\n${errors}`, { cause: err });
 	} finally {
 		browser?.disconnect();
-		helper.stdin.end(); // The shipping lifetime contract: EOF must quit the helper.
-		let timer;
-		const result = await Promise.race([
-			closed,
-			new Promise((resolve) => { timer = setTimeout(() => resolve(null), 5000); }),
-		]);
-		clearTimeout(timer);
-		if (!result) {
-			helper.kill("SIGKILL");
-			await closed;
-			throw new Error("browser-service did not exit on stdin EOF; force-killed");
+		try {
+			const { code, signal, requested } = await stopBrowserService(helper);
+			if (requested) console.log(`[smoke-demo-runtime] observed helper exit after requested IPC disconnect: code=${code}, signal=${signal} (no resident helper)`);
+			else console.log(`[smoke-demo-runtime] helper already exited/disconnected before cleanup: code=${code}, signal=${signal}; no shutdown requested`);
+			if (!failed) {
+				assert.ok(requested, "browser-service exited/disconnected before requested cleanup");
+				assert.deepEqual({ code, signal }, { code: 0, signal: null }, "browser-service shutdown failed");
+			}
+		} catch (error) {
+			if (!failed) throw error;
+			console.error(`[smoke-demo-runtime] cleanup also failed: ${formatProbeError(error)}`);
 		}
-		assert.deepEqual(result, { code: 0, signal: null }, "browser-service shutdown failed");
-		console.log("[smoke-demo-runtime] browser-service exited cleanly on stdin EOF (no resident helper)");
 	}
 }
 

src/agent/subagents/headless.ts

@@ -116,8 +116,9 @@ const NO_CHROME_MESSAGE =
 // Demos execute JS and need a browser. PRIMARY: this very app's binary
 // spawned with --browser-service (main.cjs boots a hidden window with a CDP
 // port and prints "BROWSER_SERVICE_CDP <port>"). FALLBACK: system Chrome.
-// The helper's stdin is held open by this process; server death = EOF = the
-// helper quits — no orphans. PDFs and SVGs never come here (native now).
+// A Node IPC channel owns its lifetime: parent death/disconnect quits the
+// helper on every OS (Windows Electron stdin is synthetic EOF). No orphans.
+// PDFs and SVGs never come here (native now).
 
 /** Handshake line parser (pure; exported for tests). */
 export function parseBrowserServiceLine(line: string): number | null {
@@ -180,7 +181,7 @@ async function connectBrowserService(): Promise<import("puppeteer-core").Browser
 			// Cast: the sanitizer is pure over a loose record; NODE_ENV is
 			// always present in a real process.env.
 			env: browserServiceSpawnEnv(process.env) as NodeJS.ProcessEnv,
-			stdio: ["pipe", "pipe", "pipe"] as const,
+			stdio: ["ignore", "pipe", "pipe", "ipc"] as const,
 		});
 		browserServiceProc = proc;
 		port = await new Promise<number | null>((resolve) => {
@@ -197,7 +198,7 @@ async function connectBrowserService(): Promise<import("puppeteer-core").Browser
 					resolve(parseBrowserServiceLine(line));
 				}
 			};
-			proc.stdout.on("data", onData);
+			proc.stdout!.on("data", onData);
 			proc.once("exit", () => {
 				clearTimeout(timer);
 				resolve(null);
@@ -215,7 +216,7 @@ async function connectBrowserService(): Promise<import("puppeteer-core").Browser
 		const info = (await version.json()) as { webSocketDebuggerUrl?: string };
 		if (!info.webSocketDebuggerUrl) return null;
 		// connect ≠ launch: browser.close() below merely disconnects — the
-		// helper stays resident for the next job (stdin-held lifetime).
+		// helper stays resident for the next job (IPC-owned lifetime).
 		return await puppeteer.connect({
 			browserWSEndpoint: info.webSocketDebuggerUrl,
 			defaultViewport: null,

tests/browser-service-lifetime.test.ts

@@ -0,0 +1,149 @@
+/** Execute the real helper entry with fake Electron; no display or user profile. */
+import assert from "node:assert/strict";
+import { EventEmitter } from "node:events";
+import { readFileSync } from "node:fs";
+import { createRequire } from "node:module";
+import { dirname } from "node:path";
+import { Readable } from "node:stream";
+import { describe, it } from "node:test";
+import { setImmediate as turn } from "node:timers/promises";
+import { fileURLToPath } from "node:url";
+import { runInNewContext } from "node:vm";
+import { stopBrowserService } from "../scripts/smoke-demo-runtime.mjs";
+
+const mainFile = fileURLToPath(new URL("../electron/main.cjs", import.meta.url));
+const mainSource = readFileSync(mainFile, "utf8");
+const requireMain = createRequire(mainFile);
+
+function helperFixture(connected = true, channel = true) {
+	let allowAppReady!: () => void;
+	const appReady = new Promise<void>((resolve) => { allowAppReady = resolve; });
+	let allowReadiness!: () => void;
+	const readiness = new Promise<void>((resolve) => { allowReadiness = resolve; });
+	const state = { quits: 0, exits: [] as number[], windows: 0, readinessCalls: 0, stdinReads: 0, signal: undefined as AbortSignal | undefined };
+	const logs: string[] = [];
+	const errors: string[] = [];
+	// Match Electron 44's Windows override, not a native pipe.
+	const stdin = new Readable({ read() {} });
+	stdin.push(null);
+	const process = Object.assign(new EventEmitter(), {
+		platform: "win32", argv: ["electron", "--browser-service"], env: {},
+		connected, channel: channel ? {} : undefined,
+	});
+	Object.defineProperty(process, "stdin", { get() { state.stdinReads++; return stdin; } });
+	const app = {
+		setName() {}, commandLine: { appendSwitch() {} }, whenReady: () => appReady,
+		quit() { state.quits++; }, exit(code: number) { state.exits.push(code); },
+	};
+	class FakeWindow {
+		constructor() { state.windows++; }
+		async loadURL(url: string) { assert.equal(url, "about:blank"); }
+	}
+	const net = {
+		createServer: () => Object.assign(new EventEmitter(), {
+			listen(_port: number, _host: string, callback: () => void) { callback(); },
+			address: () => ({ port: 54321 }), close(callback: () => void) { callback(); },
+		}),
+	};
+	const waitForBrowserService = (_port: number, { signal }: { signal: AbortSignal }) => {
+		state.readinessCalls++;
+		state.signal = signal;
+		return new Promise<void>((resolve, reject) => {
+			if (signal.aborted) return reject(signal.reason);
+			signal.addEventListener("abort", () => reject(signal.reason), { once: true });
+			readiness.then(resolve);
+		});
+	};
+	runInNewContext(mainSource, {
+		require: (id: string) => id === "electron" ? { app, BrowserWindow: FakeWindow }
+			: id === "node:net" ? net
+			: id === "./browser-service-readiness.cjs" ? { waitForBrowserService }
+			: requireMain(id),
+		process, __dirname: dirname(mainFile), AbortController,
+		console: { log: (line: string) => logs.push(line), error: (...parts: unknown[]) => errors.push(parts.join(" ")) },
+	}, { filename: mainFile });
+	return {
+		state, logs, errors, stdin, process, allowAppReady, allowReadiness,
+		disconnect() { process.connected = false; process.emit("disconnect"); },
+	};
+}
+
+describe("browser-service IPC-owned lifetime", () => {
+	it("ignores synthetic Windows stdin EOF while IPC remains connected", async () => {
+		const helper = helperFixture();
+		helper.stdin.resume();
+		await turn();
+		assert.equal(helper.stdin.readableEnded, true);
+		assert.equal(helper.state.stdinReads, 0);
+		assert.equal(helper.state.quits, 0);
+		helper.allowAppReady();
+		await turn();
+		assert.equal(helper.state.readinessCalls, 1);
+		assert.equal(helper.state.signal?.aborted, false);
+		helper.allowReadiness();
+		await turn();
+		assert.deepEqual(helper.logs, ["BROWSER_SERVICE_CDP 54321"]);
+		assert.deepEqual(helper.state.exits, []);
+		helper.disconnect();
+		assert.equal(helper.state.quits, 1);
+	});
+
+	it("disconnect during readiness aborts it, quits once, and never announces", async () => {
+		const helper = helperFixture();
+		helper.allowAppReady();
+		await turn();
+		assert.equal(helper.state.readinessCalls, 1);
+		helper.disconnect();
+		helper.disconnect();
+		await turn();
+		assert.equal(helper.state.signal?.aborted, true);
+		assert.equal(helper.state.quits, 1);
+		assert.deepEqual(helper.logs, []);
+		assert.deepEqual(helper.errors, []);
+		assert.deepEqual(helper.state.exits, []);
+		assert.equal(helper.state.stdinReads, 0);
+	});
+
+	it("owns disconnect before app.whenReady resolves", async () => {
+		const helper = helperFixture();
+		helper.disconnect();
+		await turn();
+		assert.equal(helper.state.quits, 1);
+		assert.equal(helper.state.windows, 0);
+		assert.deepEqual(helper.logs, []);
+	});
+
+	it("fails closed without both a connected IPC channel and connection state", async () => {
+		for (const [connected, channel] of [[false, false], [true, false], [false, true]]) {
+			const helper = helperFixture(connected, channel);
+			await turn();
+			assert.deepEqual(helper.state.exits, [1]);
+			assert.match(helper.errors.join("\n"), /requires connected parent IPC/);
+			assert.equal(helper.state.windows, 0);
+			assert.equal(helper.state.readinessCalls, 0);
+			assert.equal(helper.state.stdinReads, 0);
+			assert.equal(helper.process.listenerCount("disconnect"), 0);
+		}
+	});
+});
+
+describe("browser-service probe cleanup", () => {
+	it("requests IPC disconnect and accepts actual exit without a close event", async () => {
+		const helper = Object.assign(new EventEmitter(), {
+			pid: 123, connected: true, exitCode: null as number | null, signalCode: null,
+			disconnect() {
+				helper.connected = false;
+				queueMicrotask(() => { helper.exitCode = 0; helper.emit("exit", 0, null); });
+			},
+		});
+		assert.deepEqual(await stopBrowserService(helper), { code: 0, signal: null, requested: true });
+		assert.equal(helper.listenerCount("exit"), 0);
+		assert.equal(helper.listenerCount("error"), 0);
+	});
+
+	it("does not label an already-exited helper as parent-requested shutdown", async () => {
+		const helper = { pid: 123, connected: false, exitCode: 0, signalCode: null,
+			disconnect() { assert.fail("must not request shutdown after early exit"); } };
+		assert.deepEqual(await stopBrowserService(helper), { code: 0, signal: null, requested: false });
+	});
+});