fix(browser): evict dead Profile runtime after a closed-target run failure

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

@@ -10,7 +10,7 @@ import {
 import { redactText, redactUrl } from '../../../observation/redaction.js';
 import { articleHtmlToMarkdown } from '../../../download/article-download.js';
 import { waitForDownload } from './downloads.js';
-import type { CloakSessionManager } from './session-manager.js';
+import { isClosedContextError, type CloakSessionManager } from './session-manager.js';
 import type { BrowserContext, Frame, Page as PlaywrightPage } from 'playwright-core';
 import { runBrowserProgram } from '../../run/runner.js';
 import { BROWSER_RUN_MAX_SOURCE_BYTES } from '../../run/types.js';
@@ -258,18 +258,29 @@ export async function dispatchCloakAction(manager: CloakSessionManager, command:
           idleTimeout: command.idleTimeout,
           windowMode: command.windowMode,
         }, lease.page);
-        const data = await runBrowserProgram({
-          ...scope,
-          pageId: lease.pageId,
-        }, command.source, {
-          timeoutMs: command.timeoutMs,
-          maxOutputChars: command.maxOutputChars,
-          memoryLimitBytes: command.memoryLimitBytes,
-          snapshotDiff: command.noSnapshotDiff ? false : command.snapshotDiff,
-          snapshotMode: command.snapshotMode === 'tree' ? 'tree' : 'act',
-          snapshotBaselineStore: snapshotBaselineStore(manager),
-          ...(signal ? { signal } : {}),
-        });
+        let data: Awaited<ReturnType<typeof runBrowserProgram>>;
+        try {
+          data = await runBrowserProgram({
+            ...scope,
+            pageId: lease.pageId,
+          }, command.source, {
+            timeoutMs: command.timeoutMs,
+            maxOutputChars: command.maxOutputChars,
+            memoryLimitBytes: command.memoryLimitBytes,
+            snapshotDiff: command.noSnapshotDiff ? false : command.snapshotDiff,
+            snapshotMode: command.snapshotMode === 'tree' ? 'tree' : 'act',
+            snapshotBaselineStore: snapshotBaselineStore(manager),
+            ...(signal ? { signal } : {}),
+          });
+        } catch (error) {
+          // A dead context can still report `page.isClosed() === false`, so a lease
+          // can look healthy right up until the program tries to use it. Evict the
+          // Profile runtime so the next command gets a fresh page instead of
+          // repeatedly leasing the same invalid one (#314). We don't retry here:
+          // the program may already have caused side effects, so replay isn't safe.
+          if (isClosedContextError(error)) manager.evictDeadRuntime(lease.profileId, lease.context);
+          throw error;
+        }
         return {
           id: command.id,
           ok: true,

src/browser/runtime/local-cloak/run-recovery.test.ts

@@ -0,0 +1,170 @@
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import { CloakSessionManager } from './session-manager.js';
+import { dispatchCloakAction } from './actions.js';
+
+const runBrowserProgram = vi.hoisted(() => vi.fn());
+
+vi.mock('../../run/runner.js', () => ({
+  runBrowserProgram,
+}));
+
+// Trimmed copy of the fakeContext helper in session-manager.test.ts — only the
+// surface `browser run` recovery touches (getPage/browserRunScope/CDP window
+// bookkeeping) needs to be here.
+function fakeContext() {
+  const listeners = new Map<string, Set<(...args: unknown[]) => void>>();
+  const targetIds = new WeakMap<object, string>();
+  const windowIds = new Map<string, number>();
+  let targetCounter = 0;
+  let windowCounter = 0;
+  const fakePage = () => {
+    let closed = false;
+    let currentUrl = 'https://example.com/';
+    const page: any = {
+      goto: vi.fn().mockImplementation(async (url: string) => { currentUrl = url; }),
+      evaluate: vi.fn().mockResolvedValue('ok'),
+      title: vi.fn().mockResolvedValue('Title'),
+      url: vi.fn(() => currentUrl),
+      isClosed: vi.fn(() => closed),
+      close: vi.fn(async () => { closed = true; }),
+      opener: vi.fn().mockResolvedValue(null),
+      on() {},
+      once() {},
+      off() {},
+    };
+    const targetId = `target-${++targetCounter}`;
+    targetIds.set(page, targetId);
+    windowIds.set(targetId, ++windowCounter);
+    return page;
+  };
+  const page = fakePage();
+  const allPages = [page];
+  const emit = (event: string, ...args: unknown[]) => {
+    for (const listener of listeners.get(event) ?? []) listener(...args);
+  };
+  const cdp = {
+    send: vi.fn(async (command: string, params?: { targetId?: string }) => {
+      if (command === 'Target.createTarget') {
+        const created = await context.newPage();
+        queueMicrotask(() => emit('page', created));
+        return { targetId: targetIds.get(created) };
+      }
+      if (command === 'Browser.getWindowForTarget') return { windowId: windowIds.get(params?.targetId ?? '') };
+      if (command === 'Target.closeTarget') return { success: true };
+      return {};
+    }),
+    on: vi.fn(),
+    detach: vi.fn().mockResolvedValue(undefined),
+  };
+  let context: any;
+  return {
+    context: context = {
+      on(event: string, listener: (...args: unknown[]) => void) {
+        const bucket = listeners.get(event) ?? new Set();
+        bucket.add(listener);
+        listeners.set(event, bucket);
+      },
+      off(event: string, listener: (...args: unknown[]) => void) {
+        listeners.get(event)?.delete(listener);
+      },
+      emit,
+      pages: vi.fn(() => allPages.filter((candidate) => !candidate.isClosed())),
+      newPage: vi.fn(async () => {
+        const created = fakePage();
+        allPages.push(created);
+        return created;
+      }),
+      newCDPSession: vi.fn(async (target: object) => ({
+        send: vi.fn(async (command: string, params?: { targetId?: string }) => {
+          if (command === 'Target.getTargetInfo') return { targetInfo: { targetId: targetIds.get(target) } };
+          if (command === 'Browser.getWindowForTarget') return { windowId: windowIds.get(params?.targetId ?? '') };
+          return {};
+        }),
+        detach: vi.fn().mockResolvedValue(undefined),
+      })),
+      browser: vi.fn().mockReturnValue({ newBrowserCDPSession: vi.fn().mockResolvedValue(cdp) }),
+      cookies: vi.fn().mockResolvedValue([]),
+      close: vi.fn().mockResolvedValue(undefined),
+    },
+    page,
+  };
+}
+
+describe('browser run recovery from a dead context (#314)', () => {
+  afterEach(() => {
+    runBrowserProgram.mockReset();
+  });
+
+  it('evicts the dead Profile runtime after a closed-target run failure so the next command gets a fresh page', async () => {
+    const dead = fakeContext();
+    const replacement = fakeContext();
+    const launchPersistentContext = vi.fn()
+      .mockResolvedValueOnce(dead.context)
+      .mockResolvedValueOnce(replacement.context);
+    const manager = new CloakSessionManager({ baseDir: '/tmp/webcmd-run-recovery-test', launchPersistentContext });
+    const command = (id: string, extra: Record<string, unknown> = {}) => ({
+      id,
+      action: 'run' as const,
+      profileId: 'default',
+      session: 'work',
+      surface: 'browser' as const,
+      source: "return 'ok';",
+      ...extra,
+    });
+
+    // Existing page still reports isClosed() === false, but the program that
+    // uses it hits the closed-target signature — the same shape the issue's
+    // diagnostic trace showed for a dead-but-not-yet-noticed runtime.
+    runBrowserProgram.mockRejectedValueOnce(new Error('Target page, context or browser has been closed'));
+    const failed = await dispatchCloakAction(manager, command('run-1'));
+
+    expect(failed.ok).toBe(false);
+    expect(dead.page.isClosed()).toBe(false);
+
+    runBrowserProgram.mockResolvedValueOnce({ ok: true, result: 'ok' });
+    const recovered = await dispatchCloakAction(manager, command('run-2'));
+
+    expect(recovered).toMatchObject({ ok: true, data: { ok: true, result: 'ok' } });
+    expect(launchPersistentContext).toHaveBeenCalledTimes(2);
+  });
+
+  it('does not evict the runtime for an unrelated failure', async () => {
+    const context = fakeContext();
+    const launchPersistentContext = vi.fn().mockResolvedValue(context.context);
+    const manager = new CloakSessionManager({ baseDir: '/tmp/webcmd-run-recovery-test-2', launchPersistentContext });
+    const command = (id: string) => ({
+      id,
+      action: 'run' as const,
+      profileId: 'default',
+      session: 'work',
+      surface: 'browser' as const,
+      source: "throw new Error('boom');",
+    });
+
+    runBrowserProgram.mockRejectedValueOnce(new Error('boom'));
+    const failed = await dispatchCloakAction(manager, command('run-1'));
+    expect(failed.ok).toBe(false);
+
+    runBrowserProgram.mockResolvedValueOnce({ ok: true, result: 'ok' });
+    await dispatchCloakAction(manager, command('run-2'));
+
+    expect(launchPersistentContext).toHaveBeenCalledTimes(1);
+  });
+
+  it('normalizes the profileId before evicting, so an un-normalized id still finds the runtime', async () => {
+    const dead = fakeContext();
+    const launchPersistentContext = vi.fn().mockResolvedValue(dead.context);
+    const manager = new CloakSessionManager({ baseDir: '/tmp/webcmd-run-recovery-test-3', launchPersistentContext });
+    const lease = await manager.getPage({ profileId: 'default', session: 'work', surface: 'browser' });
+
+    // The manager keys its runtime map by normalizeProfileId('default') === 'default';
+    // callers may reasonably pass an equivalent but un-normalized id.
+    manager.evictDeadRuntime('  default  ', lease.context);
+
+    const replacement = fakeContext();
+    launchPersistentContext.mockResolvedValueOnce(replacement.context);
+    await manager.getPage({ profileId: 'default', session: 'work', surface: 'browser' });
+
+    expect(launchPersistentContext).toHaveBeenCalledTimes(2);
+  });
+});

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

@@ -170,7 +170,7 @@ function pageIsClosed(page: PlaywrightPage): boolean {
   return page.isClosed?.() === true;
 }
 
-function isClosedContextError(error: unknown): boolean {
+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);
 }
@@ -422,6 +422,18 @@ export class CloakSessionManager {
     return this.navigatePageAttempt(input, url, waitUntil, 0);
   }
 
+  /**
+   * Drop a Profile runtime that a caller has independently proven dead (e.g. a
+   * `browser run` program observed the closed-target signature). No-ops if the
+   * Profile has already been replaced, so it never evicts a healthy runtime that
+   * raced ahead of the failed lease (see #314).
+   */
+  evictDeadRuntime(profileId: string, context: BrowserContext): void {
+    const normalized = normalizeProfileId(profileId);
+    const runtime = this.profiles.get(normalized);
+    if (runtime && runtime.context === context) this.invalidateProfileRuntime(normalized, runtime);
+  }
+
   private async newPageAttempt(input: SessionKeyInput & { url?: string }, attempt: number): Promise<CloakPageLease> {
     const profileId = normalizeProfileId(input.profileId);
     const session = requireSession(input.session);