fix(browser): retry stale cached playwright attach once

(cherry picked from commit ec252ebd7b45d54e431e0d7599532e5a0c1a9b73)

# Conflicts:
#	extensions/browser/src/browser/pw-session.ts

extensions/browser/src/browser/pw-session.connections.test.ts

@@ -1,7 +1,11 @@
 import { chromium } from "playwright-core";
 import { afterEach, describe, expect, it, vi } from "vitest";
 import * as chromeModule from "./chrome.js";
-import { closePlaywrightBrowserConnection, listPagesViaPlaywright } from "./pw-session.js";
+import {
+  closePlaywrightBrowserConnection,
+  getPageForTargetId,
+  listPagesViaPlaywright,
+} from "./pw-session.js";
 
 const connectOverCdpSpy = vi.spyOn(chromium, "connectOverCDP");
 const getChromeWebSocketUrlSpy = vi.spyOn(chromeModule, "getChromeWebSocketUrl");
@@ -42,6 +46,24 @@ function makeBrowser(targetId: string, url: string): BrowserMockBundle {
   return { browser, browserClose };
 }
 
+function makeEmptyBrowser(): BrowserMockBundle {
+  const browserClose = vi.fn(async () => {});
+  const context = {
+    pages: () => [],
+    on: vi.fn(),
+    newCDPSession: vi.fn(),
+  } as unknown as import("playwright-core").BrowserContext;
+
+  const browser = {
+    contexts: () => [context],
+    on: vi.fn(),
+    off: vi.fn(),
+    close: browserClose,
+  } as unknown as import("playwright-core").Browser;
+
+  return { browser, browserClose };
+}
+
 afterEach(async () => {
   connectOverCdpSpy.mockReset();
   getChromeWebSocketUrlSpy.mockReset();
@@ -116,4 +138,37 @@ describe("pw-session connection scoping", () => {
     expect(browserA.browserClose).toHaveBeenCalledTimes(1);
     expect(browserB.browserClose).not.toHaveBeenCalled();
   });
+
+  it("evicts only the stale cdpUrl when getPageForTargetId retries a cached connection", async () => {
+    const staleA = makeEmptyBrowser();
+    const refreshedA = makeBrowser("A", "https://a.example/recovered");
+    const browserB = makeBrowser("B", "https://b.example");
+    let callsForA = 0;
+
+    connectOverCdpSpy.mockImplementation((async (...args: unknown[]) => {
+      const endpointText = String(args[0]);
+      if (endpointText === "http://127.0.0.1:9222") {
+        callsForA += 1;
+        return callsForA === 1 ? staleA.browser : refreshedA.browser;
+      }
+      if (endpointText === "http://127.0.0.1:9333") {
+        return browserB.browser;
+      }
+      throw new Error(`unexpected endpoint: ${endpointText}`);
+    }) as never);
+    getChromeWebSocketUrlSpy.mockResolvedValue(null);
+
+    await listPagesViaPlaywright({ cdpUrl: "http://127.0.0.1:9222" });
+    await listPagesViaPlaywright({ cdpUrl: "http://127.0.0.1:9333" });
+
+    const recoveredA = await getPageForTargetId({ cdpUrl: "http://127.0.0.1:9222" });
+    const stillCachedB = await getPageForTargetId({ cdpUrl: "http://127.0.0.1:9333" });
+
+    expect(recoveredA.url()).toBe("https://a.example/recovered");
+    expect(stillCachedB.url()).toBe("https://b.example");
+    expect(staleA.browserClose).toHaveBeenCalledTimes(1);
+    expect(refreshedA.browserClose).not.toHaveBeenCalled();
+    expect(browserB.browserClose).not.toHaveBeenCalled();
+    expect(connectOverCdpSpy).toHaveBeenCalledTimes(3);
+  });
 });

extensions/browser/src/browser/pw-session.get-page-for-targetid.extension-fallback.test.ts

@@ -1,14 +1,69 @@
 import { chromium } from "playwright-core";
 import { afterEach, describe, expect, it, vi } from "vitest";
 import * as chromeModule from "./chrome.js";
-import { closePlaywrightBrowserConnection, getPageForTargetId } from "./pw-session.js";
+import {
+  closePlaywrightBrowserConnection,
+  getPageForTargetId,
+  listPagesViaPlaywright,
+} from "./pw-session.js";
 
 const connectOverCdpSpy = vi.spyOn(chromium, "connectOverCDP");
 const getChromeWebSocketUrlSpy = vi.spyOn(chromeModule, "getChromeWebSocketUrl");
 
+type MockPageSpec = {
+  targetId?: string;
+  url?: string;
+  title?: string;
+};
+
+type BrowserMockBundle = {
+  browser: import("playwright-core").Browser;
+  browserClose: ReturnType<typeof vi.fn>;
+  pages: import("playwright-core").Page[];
+};
+
+function makeBrowser(pages: MockPageSpec[]): BrowserMockBundle {
+  let context: import("playwright-core").BrowserContext;
+  const browserClose = vi.fn(async () => {});
+  const targetIdByPage = new Map<import("playwright-core").Page, string | undefined>();
+
+  const pageObjects = pages.map((spec, index) => {
+    const page = {
+      on: vi.fn(),
+      context: () => context,
+      title: vi.fn(async () => spec.title ?? spec.targetId ?? `page-${index + 1}`),
+      url: vi.fn(() => spec.url ?? `https://page-${index + 1}.example`),
+    } as unknown as import("playwright-core").Page;
+    targetIdByPage.set(page, spec.targetId);
+    return page;
+  });
+
+  context = {
+    pages: () => pageObjects,
+    on: vi.fn(),
+    newCDPSession: vi.fn(async (page: import("playwright-core").Page) => ({
+      send: vi.fn(async (method: string) =>
+        method === "Target.getTargetInfo"
+          ? { targetInfo: { targetId: targetIdByPage.get(page) } }
+          : {},
+      ),
+      detach: vi.fn(async () => {}),
+    })),
+  } as unknown as import("playwright-core").BrowserContext;
+
+  const browser = {
+    contexts: () => [context],
+    on: vi.fn(),
+    off: vi.fn(),
+    close: browserClose,
+  } as unknown as import("playwright-core").Browser;
+
+  return { browser, browserClose, pages: pageObjects };
+}
+
 afterEach(async () => {
-  connectOverCdpSpy.mockClear();
-  getChromeWebSocketUrlSpy.mockClear();
+  connectOverCdpSpy.mockReset();
+  getChromeWebSocketUrlSpy.mockReset();
   await closePlaywrightBrowserConnection().catch(() => {});
 });
 
@@ -119,4 +174,73 @@ describe("pw-session getPageForTargetId", () => {
       fetchSpy.mockRestore();
     }
   });
+
+  it("evicts a stale cached page-less browser once and succeeds on a fresh reconnect", async () => {
+    const stale = makeBrowser([]);
+    const fresh = makeBrowser([{ targetId: "TARGET_OK", url: "https://fresh.example" }]);
+
+    connectOverCdpSpy.mockResolvedValueOnce(stale.browser).mockResolvedValueOnce(fresh.browser);
+    getChromeWebSocketUrlSpy.mockResolvedValue(null);
+
+    await listPagesViaPlaywright({ cdpUrl: "http://127.0.0.1:9222" });
+
+    const resolved = await getPageForTargetId({ cdpUrl: "http://127.0.0.1:9222" });
+
+    expect(resolved).toBe(fresh.pages[0]);
+    expect(connectOverCdpSpy).toHaveBeenCalledTimes(2);
+    expect(stale.browserClose).toHaveBeenCalledTimes(1);
+  });
+
+  it("evicts a stale cached tab-selection miss once and succeeds on a fresh reconnect", async () => {
+    const stale = makeBrowser([
+      { targetId: "TARGET_A", url: "https://alpha.example" },
+      { targetId: "TARGET_C", url: "https://charlie.example" },
+    ]);
+    const fresh = makeBrowser([
+      { targetId: "TARGET_A", url: "https://alpha.example" },
+      { targetId: "TARGET_B", url: "https://beta.example" },
+    ]);
+
+    connectOverCdpSpy.mockResolvedValueOnce(stale.browser).mockResolvedValueOnce(fresh.browser);
+    getChromeWebSocketUrlSpy.mockResolvedValue(null);
+
+    await getPageForTargetId({ cdpUrl: "http://127.0.0.1:9333" });
+
+    const resolved = await getPageForTargetId({
+      cdpUrl: "http://127.0.0.1:9333",
+      targetId: "TARGET_B",
+    });
+
+    expect(resolved).toBe(fresh.pages[1]);
+    expect(connectOverCdpSpy).toHaveBeenCalledTimes(2);
+    expect(stale.browserClose).toHaveBeenCalledTimes(1);
+  });
+
+  it("fails after a single reconnect when the refreshed browser is still page-less", async () => {
+    const stale = makeBrowser([]);
+    const stillBroken = makeBrowser([]);
+
+    connectOverCdpSpy
+      .mockResolvedValueOnce(stale.browser)
+      .mockResolvedValueOnce(stillBroken.browser);
+    getChromeWebSocketUrlSpy.mockResolvedValue(null);
+
+    await listPagesViaPlaywright({ cdpUrl: "http://127.0.0.1:9444" });
+
+    await expect(getPageForTargetId({ cdpUrl: "http://127.0.0.1:9444" })).rejects.toThrow(
+      "No pages available in the connected browser.",
+    );
+    expect(connectOverCdpSpy).toHaveBeenCalledTimes(2);
+    expect(stale.browserClose).toHaveBeenCalledTimes(1);
+  });
+
+  it("does not add an extra top-level retry for non-recoverable connect failures", async () => {
+    connectOverCdpSpy.mockRejectedValue(new Error("connectOverCDP exploded"));
+    getChromeWebSocketUrlSpy.mockResolvedValue(null);
+
+    await expect(getPageForTargetId({ cdpUrl: "http://127.0.0.1:9555" })).rejects.toThrow(
+      "connectOverCDP exploded",
+    );
+    expect(connectOverCdpSpy).toHaveBeenCalledTimes(3);
+  });
 });

extensions/browser/src/browser/pw-session.ts

@@ -123,6 +123,27 @@ function normalizeCdpUrl(raw: string) {
   return raw.replace(/\/$/, "");
 }
 
+function hasCachedPlaywrightBrowserConnection(cdpUrl: string): boolean {
+  return cachedByCdpUrl.has(normalizeCdpUrl(cdpUrl));
+}
+
+function isRecoverableStalePageSelectionError(err: unknown, reusedCachedBrowser: boolean): boolean {
+  if (!reusedCachedBrowser) {
+    return false;
+  }
+  if (
+    err instanceof Error &&
+    err.message.includes("No pages available in the connected browser.")
+  ) {
+    return true;
+  }
+  if (err instanceof BrowserTabNotFoundError) {
+    return true;
+  }
+  const message = err instanceof Error ? err.message : formatErrorMessage(err);
+  return message.toLowerCase().includes("tab not found");
+}
+
 function findNetworkRequestById(state: PageState, id: string): BrowserNetworkRequest | undefined {
   for (let i = state.requests.length - 1; i >= 0; i -= 1) {
     const candidate = state.requests[i];
@@ -625,7 +646,7 @@ async function resolvePageByTargetIdOrThrow(opts: {
   return page;
 }
 
-export async function getPageForTargetId(opts: {
+async function getPageForTargetIdOnce(opts: {
   cdpUrl: string;
   targetId?: string;
   ssrfPolicy?: SsrFPolicy;
@@ -671,6 +692,23 @@ export async function getPageForTargetId(opts: {
   throw new BrowserTabNotFoundError();
 }
 
+export async function getPageForTargetId(opts: {
+  cdpUrl: string;
+  targetId?: string;
+  ssrfPolicy?: SsrFPolicy;
+}): Promise<Page> {
+  const reusedCachedBrowser = hasCachedPlaywrightBrowserConnection(opts.cdpUrl);
+  try {
+    return await getPageForTargetIdOnce(opts);
+  } catch (err) {
+    if (!isRecoverableStalePageSelectionError(err, reusedCachedBrowser)) {
+      throw err;
+    }
+    await closePlaywrightBrowserConnection({ cdpUrl: opts.cdpUrl });
+    return await getPageForTargetIdOnce(opts);
+  }
+}
+
 function isTopLevelNavigationRequest(page: Page, request: Request): boolean {
   let sameMainFrame = false;
   try {
@@ -848,6 +886,22 @@ export async function gotoPageWithNavigationGuard(
   }
 }
 
+export async function getPageForTargetId(opts: {
+  cdpUrl: string;
+  targetId?: string;
+}): Promise<Page> {
+  const reusedCachedBrowser = hasCachedPlaywrightBrowserConnection(opts.cdpUrl);
+  try {
+    return await getPageForTargetIdOnce(opts);
+  } catch (err) {
+    if (!isRecoverableStalePageSelectionError(err, reusedCachedBrowser)) {
+      throw err;
+    }
+    await closePlaywrightBrowserConnection({ cdpUrl: opts.cdpUrl });
+    return await getPageForTargetIdOnce(opts);
+  }
+}
+
 export function refLocator(page: Page, ref: string) {
   const normalized = ref.startsWith("@")
     ? ref.slice(1)