fix(browser): don't reuse a dead page/context lease in browser run (#324)

Thank you @Kaushik2003.

Fixes #314. Dead CDP connections are detected on lease reuse and after a successful run whose snapshot sees a closed context.

Also thank you @rohan911438 for #323 — same diagnosis; we landed this variant because it recovers at getPage time as well.

Author: Kaushik2003

src/browser/run/runner.test.ts

@@ -236,6 +236,40 @@ afterAll(async () => {
     }));
   });
 
+  it('signals a stale context when the post-snapshot fails with a closed-context error (webcmd#314)', async () => {
+    const newCDPSession = context.newCDPSession.bind(context);
+    let calls = 0;
+    context.newCDPSession = ((...args: Parameters<BrowserContext['newCDPSession']>) => {
+      calls += 1;
+      if (calls === 2) return Promise.reject(new Error('Target page, context or browser has been closed'));
+      return newCDPSession(...args);
+    }) as BrowserContext['newCDPSession'];
+    const onStaleContext = vi.fn();
+
+    const output = await run('return 7;', { snapshotDiff: true, onStaleContext });
+
+    expect(output.result).toBe(7);
+    expect(output.warnings).toContainEqual(expect.objectContaining({
+      code: 'BROWSER_RUN_SNAPSHOT_FAILED',
+    }));
+    expect(onStaleContext).toHaveBeenCalledTimes(1);
+  });
+
+  it('does not signal a stale context for an unrelated post-snapshot failure', async () => {
+    const newCDPSession = context.newCDPSession.bind(context);
+    let calls = 0;
+    context.newCDPSession = ((...args: Parameters<BrowserContext['newCDPSession']>) => {
+      calls += 1;
+      if (calls === 2) return Promise.reject(new Error('post snapshot failed'));
+      return newCDPSession(...args);
+    }) as BrowserContext['newCDPSession'];
+    const onStaleContext = vi.fn();
+
+    await run('return 7;', { snapshotDiff: true, onStaleContext });
+
+    expect(onStaleContext).not.toHaveBeenCalled();
+  });
+
   it('does not expose page.snapshotForAI inside browser-run code', async () => {
     const output = await run('return typeof page.snapshotForAI;');
 

src/browser/run/runner.ts

@@ -33,6 +33,7 @@ import {
   type BrowserRunResult,
   type BrowserRunTimings,
   type BrowserRunWarning,
+  isClosedContextError,
 } from './types.js';
 
 export interface BrowserRunSessionScope {
@@ -650,6 +651,10 @@ export async function runBrowserProgram(
           code: 'BROWSER_RUN_SNAPSHOT_FAILED',
           message: normalizeExecutionError(snapshotError).message,
         });
+        // The run itself already succeeded, so keep ok: true — but a closed-context
+        // signature here means the connection is dying underneath it. Signal the
+        // caller out-of-band so the dead Profile runtime doesn't get reused (#314).
+        if (isClosedContextError(snapshotError)) options.onStaleContext?.();
       } finally {
         timings.snapshot_ms = (timings.snapshot_ms ?? 0) + Math.max(0, Date.now() - snapshotStartedAt);
       }

src/browser/run/types.ts

@@ -88,6 +88,21 @@ export interface BrowserRunOptions {
   snapshotMode?: SnapshotTreeMode;
   snapshotBaselineStore?: SnapshotBaselineStore;
   signal?: AbortSignal;
+  /**
+   * Called when a closed-context signature ("Target page, context or browser
+   * has been closed") surfaces somewhere that doesn't itself fail the run —
+   * currently, the post-run snapshot capture (webcmd#314). The run still
+   * completes with `ok: true` (the program itself may have genuinely
+   * succeeded), but the caller should treat the underlying Profile runtime as
+   * dead so the next command on this Session isn't handed the same lease.
+   */
+  onStaleContext?: () => void;
+}
+
+/** Matches Playwright's "Target page, context or browser has been closed" error family. */
+export function isClosedContextError(error: unknown): boolean {
+  const message = error instanceof Error ? error.message : String(error);
+  return /Target page, context or browser has been closed/i.test(message);
 }
 
 export interface BrowserRunLogEntry {

src/browser/runtime/local-cloak/actions.ts

@@ -279,6 +279,7 @@ export async function dispatchCloakAction(manager: CloakSessionManager, command:
           snapshotDiff: command.noSnapshotDiff ? false : command.snapshotDiff,
           snapshotMode: command.snapshotMode === 'tree' ? 'tree' : 'act',
           snapshotBaselineStore: snapshotBaselineStore(manager),
+          onStaleContext: () => manager.invalidateIfClosedContext(lease.profileId, scope.context),
           ...(signal ? { signal } : {}),
         });
         return {

src/browser/runtime/local-cloak/session-manager.test.ts

@@ -850,6 +850,39 @@ describe('CloakSessionManager', () => {
     expect(launchPersistentContext).toHaveBeenCalledTimes(2);
   });
 
+  it('invalidates a reused getPage() lease when the CDP liveness probe finds a dead context (webcmd#314)', async () => {
+    const first = fakeContext();
+    const replacement = fakeContext();
+    replacement.context.pages.mockReturnValue([]);
+    const launchPersistentContext = vi.fn()
+      .mockResolvedValueOnce(first.context)
+      .mockResolvedValueOnce(replacement.context);
+    const manager = new CloakSessionManager({ baseDir: '/tmp/webcmd-test', launchPersistentContext });
+
+    const firstLease = await manager.getPage({ profileId: 'default', session: 'work', surface: 'browser' });
+    expect(firstLease.context).toBe(first.context);
+
+    // Simulate the issue's repro: isClosed() still reports false, but the
+    // underlying CDP connection is dead, so the liveness probe used on reuse
+    // (Browser.getWindowForTarget, via assertOwnedWindow) fails.
+    first.cdp.send.mockImplementation(async (command: string) => {
+      if (command === 'Browser.getWindowForTarget') {
+        throw new Error('Target page, context or browser has been closed');
+      }
+      return {};
+    });
+
+    // isClosed() still reports false right up to the reuse attempt — the fast
+    // path's precondition holds; only the liveness probe catches the dead lease.
+    expect(firstLease.page.isClosed()).toBe(false);
+    const secondLease = await manager.getPage({ profileId: 'default', session: 'work', surface: 'browser' });
+
+    expect(secondLease.context).toBe(replacement.context);
+    expect(secondLease.page).not.toBe(firstLease.page);
+    expect((firstLease.page as unknown as { close: ReturnType<typeof vi.fn> }).close).toHaveBeenCalled();
+    expect(launchPersistentContext).toHaveBeenCalledTimes(2);
+  });
+
   it('retries explicit newPage page creation once after a closed-context failure', async () => {
     const closed = new Error('browserContext.newPage: Target page, context or browser has been closed');
     const first = fakeContext();

src/browser/runtime/local-cloak/session-manager.ts

@@ -11,6 +11,7 @@ import { findPackageRoot } from '../../../package-paths.js';
 import { findExactCloakProfileProcesses } from './process-matcher.js';
 import { log } from '../../../logger.js';
 import { CliError, EXIT_CODES } from '../../../errors.js';
+import { isClosedContextError } from '../../run/types.js';
 
 const UNRESOLVED = Symbol('unresolved');
 const TARGET_PAGE_MATCH_TIMEOUT_MS = 1_000;
@@ -180,11 +181,6 @@ function pageIsClosed(page: PlaywrightPage): boolean {
   return page.isClosed?.() === true;
 }
 
-function isClosedContextError(error: unknown): boolean {
-  const message = error instanceof Error ? error.message : String(error);
-  return /Target page, context or browser has been closed/i.test(message);
-}
-
 function errorMessage(error: unknown): string {
   return error instanceof Error ? error.message : String(error);
 }
@@ -282,11 +278,21 @@ export class CloakSessionManager {
       const sessionRuntime = this.getSessionRuntime(runtime, sessionId);
       const existing = sessionRuntime.pages.get(leaseKey);
       if (existing && !pageIsClosed(existing.page) && !freshPage) {
-        await this.assertOwnedWindow(runtime, sessionId, existing);
-        runtime.lastSeenAt = Date.now();
-        existing.idleTimeout = input.idleTimeout;
-        this.refreshIdleTimer(runtime, sessionRuntime, leaseKey, existing);
-        return { profileId, leaseKey, context: runtime.context, page: existing.page, pageId: existing.pageId };
+        try {
+          await this.assertOwnedWindow(runtime, sessionId, existing);
+          runtime.lastSeenAt = Date.now();
+          existing.idleTimeout = input.idleTimeout;
+          this.refreshIdleTimer(runtime, sessionRuntime, leaseKey, existing);
+          return { profileId, leaseKey, context: runtime.context, page: existing.page, pageId: existing.pageId };
+        } catch (error) {
+          if (!isClosedContextError(error)) throw error;
+          // isClosed() reported false, but the liveness probe above shows the
+          // underlying CDP connection is actually dead. Invalidate the Profile
+          // runtime and fall through to acquire a fresh page instead of handing
+          // the same broken lease back out (webcmd#314).
+          this.invalidateProfileRuntime(profileId, runtime);
+          if (!pageIsClosed(existing.page)) await existing.page.close().catch(() => {});
+        }
       }
       const acquired = await this.acquireSessionPage(profileId, sessionId, input.windowMode);
       const entry = await this.registerOwnedPage(acquired.runtime, acquired.session, acquired.page, {
@@ -667,6 +673,19 @@ export class CloakSessionManager {
     return entries.length;
   }
 
+  /**
+   * Invalidates the Profile runtime backing `context`, if it's still the active
+   * one, without evicting or retrying the command that observed it. Used by the
+   * `run` action (webcmd#314) when a post-run snapshot capture surfaces a
+   * closed-context signature: the run itself may have genuinely succeeded, but
+   * the connection is dying, so the next command on this Session shouldn't be
+   * handed the same lease.
+   */
+  invalidateIfClosedContext(profileId: string, context: BrowserContext): void {
+    const runtime = this.profiles.get(profileId);
+    if (runtime?.context === context) this.invalidateProfileRuntime(profileId, runtime);
+  }
+
   async shutdown(): Promise<void> {
     this.shuttingDown = true;
     while (this.profileLaunches.size > 0) {