fix(preview): relaunch the shared browser after it disconnects (#146)

src/ui/__tests__/preview-correctness.test.ts

@@ -22,12 +22,15 @@ import { getSessionCount, revokeAllSessions } from "../session.ts";
 
 type Cookie = { name: string; value: string; domain: string; path: string };
 type FakeContext = BrowserContext & { __cookies: Cookie[]; __closed: boolean };
-type FakeBrowser = Browser & { __contexts: FakeContext[]; __closed: boolean };
+type FakeBrowser = Browser & { __contexts: FakeContext[]; __closed: boolean; __disconnect: () => void };
 
-function makeFakeContext(): FakeContext {
+function makeFakeContext(parent: FakeBrowser): FakeContext {
 	const state = { __cookies: [] as Cookie[], __closed: false };
 	const ctx = {
 		...state,
+		// preview.ts reads context.browser() to test liveness before reusing a
+		// cached context, so the fake must report its parent (#146).
+		browser: () => parent,
 		addCookies: async (cookies: Cookie[]) => {
 			// Match Playwright semantics: replace any cookie with the same
 			// name+domain+path in place, otherwise append.
@@ -50,19 +53,30 @@ function makeFakeContext(): FakeContext {
 
 function makeFakeBrowser(): FakeBrowser {
 	const contexts: FakeContext[] = [];
-	const state = { __closed: false };
+	const state = { __closed: false, __connected: true };
 	const b = {
+		// preview.ts gates cache reuse on isConnected(); a crashed/killed
+		// browser process reports false and must trigger a relaunch (#146).
+		isConnected: () => state.__connected,
 		newContext: async () => {
-			const ctx = makeFakeContext();
+			const ctx = makeFakeContext(b);
 			contexts.push(ctx);
 			return ctx;
 		},
 		close: async () => {
 			state.__closed = true;
+			state.__connected = false;
 		},
 	} as unknown as FakeBrowser;
 	Object.defineProperty(b, "__contexts", { get: () => contexts });
 	Object.defineProperty(b, "__closed", { get: () => state.__closed });
+	// Simulate an out-of-band process death (renderer crash, OOM, kill) that
+	// disconnects the Browser without going through close().
+	Object.defineProperty(b, "__disconnect", {
+		value: () => {
+			state.__connected = false;
+		},
+	});
 	return b;
 }
 
@@ -115,6 +129,55 @@ describe("getOrCreatePreviewContext launch-failure recovery", () => {
 	});
 });
 
+describe("crashed-browser recovery (#146)", () => {
+	test("getOrCreateBrowser relaunches after the cached browser disconnects", async () => {
+		const spy = spyOn(chromium, "launch");
+		try {
+			const browsers: FakeBrowser[] = [];
+			spy.mockImplementation(async () => {
+				const b = makeFakeBrowser();
+				browsers.push(b);
+				return b;
+			});
+
+			const first = (await getOrCreateBrowser()) as FakeBrowser;
+			// A warm cache hit while still connected must not relaunch.
+			expect(await getOrCreateBrowser()).toBe(first);
+			expect(browsers.length).toBe(1);
+
+			// Simulate the process dying out from under us (the #146 symptom).
+			first.__disconnect();
+
+			const second = (await getOrCreateBrowser()) as FakeBrowser;
+			expect(second).not.toBe(first);
+			expect(second.isConnected()).toBe(true);
+			expect(browsers.length).toBe(2);
+		} finally {
+			spy.mockRestore();
+		}
+	});
+
+	test("getOrCreatePreviewContext rebuilds on a relaunched browser after a crash", async () => {
+		const spy = spyOn(chromium, "launch");
+		try {
+			spy.mockImplementation(async () => makeFakeBrowser());
+
+			const firstCtx = (await getOrCreatePreviewContext()) as FakeContext;
+			// Warm hit while the browser is alive returns the same context.
+			expect(await getOrCreatePreviewContext()).toBe(firstCtx);
+
+			// The browser behind the cached context dies.
+			(firstCtx.browser() as FakeBrowser).__disconnect();
+
+			const secondCtx = (await getOrCreatePreviewContext()) as FakeContext;
+			expect(secondCtx).not.toBe(firstCtx);
+			expect((secondCtx.browser() as FakeBrowser).isConnected()).toBe(true);
+		} finally {
+			spy.mockRestore();
+		}
+	});
+});
+
 describe("shuttingDown flag (review F7)", () => {
 	test("getOrCreateBrowser throws after closePreviewResources", async () => {
 		await closePreviewResources();

src/ui/preview.ts

@@ -70,7 +70,16 @@ async function injectFreshPreviewCookie(ctx: BrowserContext): Promise<void> {
 
 export async function getOrCreateBrowser(): Promise<Browser> {
 	if (shuttingDown) throw new Error("preview subsystem is shutting down");
-	if (browser) return browser;
+	if (browser) {
+		// A cached Browser is only reusable while its process is still alive.
+		// A renderer/browser crash, an OOM kill, or an external process sweep
+		// disconnects the Browser without clearing this reference, and every
+		// later call would otherwise hand back a dead handle that fails for the
+		// rest of the process lifetime (#146). Drop the stale reference on
+		// disconnect so the launch path below relaunches a fresh process.
+		if (browser.isConnected()) return browser;
+		browser = null;
+	}
 	if (browserPromise) return browserPromise;
 	// try/finally pattern: on either success OR failure we clear the cached
 	// promise. If we did not, a transient chromium.launch() throw would leave
@@ -94,14 +103,22 @@ export async function getOrCreateBrowser(): Promise<Browser> {
 export async function getOrCreatePreviewContext(): Promise<BrowserContext> {
 	if (shuttingDown) throw new Error("preview subsystem is shutting down");
 	if (currentContext) {
-		// Warm cache path: rotate the preview cookie if we are inside the 2
-		// minute safety margin before the 10 minute TTL expires. Playwright's
-		// addCookies replaces cookies with the same name+domain+path in place,
-		// so this is an O(1) refresh of the cached context (review F6, Codex P1).
-		if (Date.now() - lastCookieMintAt >= COOKIE_ROTATE_AFTER_MS) {
-			await injectFreshPreviewCookie(currentContext);
+		// A cached context dies with its browser: once the underlying process
+		// disconnects (crash, OOM, kill) the context handle is permanently
+		// dead, so drop it and fall through to rebuild a fresh context on a
+		// relaunched browser rather than returning a corpse (#146).
+		const ctxBrowser = currentContext.browser();
+		if (ctxBrowser?.isConnected()) {
+			// Warm cache path: rotate the preview cookie if we are inside the 2
+			// minute safety margin before the 10 minute TTL expires. Playwright's
+			// addCookies replaces cookies with the same name+domain+path in place,
+			// so this is an O(1) refresh of the cached context (review F6, Codex P1).
+			if (Date.now() - lastCookieMintAt >= COOKIE_ROTATE_AFTER_MS) {
+				await injectFreshPreviewCookie(currentContext);
+			}
+			return currentContext;
 		}
-		return currentContext;
+		currentContext = null;
 	}
 	if (currentContextPromise) return currentContextPromise;
 	currentContextPromise = (async () => {