From a76cbc43bb01bf557490f762ab7a8fd524d0883e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 17 Jan 2026 01:28:22 +0000 Subject: [PATCH] fix(browser): remote profile tab ops follow-up (#1060) (thanks @mukhtharcm) Landed via follow-up to #1057. Gate: pnpm lint && pnpm build && pnpm test --- CHANGELOG.md | 1 + src/browser/pw-ai-module.ts | 41 +++++++++ src/browser/pw-ai.ts | 1 + .../pw-session.browserless.live.test.ts | 49 +++++++++++ src/browser/pw-session.ts | 28 ++++++ src/browser/routes/agent.shared.ts | 16 +--- .../server-context.remote-tab-ops.test.ts | 88 +++++++++++++++++++ src/browser/server-context.ts | 56 +++++------- 8 files changed, 232 insertions(+), 48 deletions(-) create mode 100644 src/browser/pw-ai-module.ts create mode 100644 src/browser/pw-session.browserless.live.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f38647659..ade17991f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ - Sessions: repair orphaned user turns before embedded prompts. - Channels: treat replies to the bot as implicit mentions across supported channels. - Browser: remote profile tab operations prefer persistent Playwright and avoid silent HTTP fallbacks. (#1057) — thanks @mukhtharcm. +- Browser: remote profile tab ops follow-up: shared Playwright loader, Playwright-based focus, and more coverage (incl. opt-in live Browserless test). (follow-up to #1057) — thanks @mukhtharcm. - WhatsApp: scope self-chat response prefix; inject pending-only group history and clear after any processed message. - Agents: drop unsigned Gemini tool calls and avoid JSON Schema `format` keyword collisions. - Auth: inherit/merge sub-agent auth profiles from the main agent. diff --git a/src/browser/pw-ai-module.ts b/src/browser/pw-ai-module.ts new file mode 100644 index 000000000..2fd88ad59 --- /dev/null +++ b/src/browser/pw-ai-module.ts @@ -0,0 +1,41 @@ +import { extractErrorCode, formatErrorMessage } from "../infra/errors.js"; + +export type PwAiModule = typeof import("./pw-ai.js"); + +type PwAiLoadMode = "soft" | "strict"; + +let pwAiModuleSoft: Promise | null = null; +let pwAiModuleStrict: Promise | null = null; + +function isModuleNotFoundError(err: unknown): boolean { + const code = extractErrorCode(err); + if (code === "ERR_MODULE_NOT_FOUND") return true; + const msg = formatErrorMessage(err); + return ( + msg.includes("Cannot find module") || + msg.includes("Cannot find package") || + msg.includes("Failed to resolve import") || + msg.includes("Failed to resolve entry for package") || + msg.includes("Failed to load url") + ); +} + +async function loadPwAiModule(mode: PwAiLoadMode): Promise { + try { + return await import("./pw-ai.js"); + } catch (err) { + if (mode === "soft") return null; + if (isModuleNotFoundError(err)) return null; + throw err; + } +} + +export async function getPwAiModule(opts?: { mode?: PwAiLoadMode }): Promise { + const mode: PwAiLoadMode = opts?.mode ?? "soft"; + if (mode === "soft") { + if (!pwAiModuleSoft) pwAiModuleSoft = loadPwAiModule("soft"); + return await pwAiModuleSoft; + } + if (!pwAiModuleStrict) pwAiModuleStrict = loadPwAiModule("strict"); + return await pwAiModuleStrict; +} diff --git a/src/browser/pw-ai.ts b/src/browser/pw-ai.ts index 3e8cf4a3a..446ecd0a4 100644 --- a/src/browser/pw-ai.ts +++ b/src/browser/pw-ai.ts @@ -4,6 +4,7 @@ export { closePlaywrightBrowserConnection, createPageViaPlaywright, ensurePageState, + focusPageByTargetIdViaPlaywright, getPageForTargetId, listPagesViaPlaywright, refLocator, diff --git a/src/browser/pw-session.browserless.live.test.ts b/src/browser/pw-session.browserless.live.test.ts new file mode 100644 index 000000000..51df758d3 --- /dev/null +++ b/src/browser/pw-session.browserless.live.test.ts @@ -0,0 +1,49 @@ +import { describe, it } from "vitest"; + +const LIVE = process.env.LIVE === "1" || process.env.CLAWDBOT_LIVE_TEST === "1"; +const CDP_URL = process.env.CLAWDBOT_LIVE_BROWSER_CDP_URL?.trim() || ""; +const describeLive = LIVE && CDP_URL ? describe : describe.skip; + +async function waitFor( + fn: () => Promise, + opts: { timeoutMs: number; intervalMs: number }, +): Promise { + const deadline = Date.now() + opts.timeoutMs; + while (Date.now() < deadline) { + if (await fn()) return; + await new Promise((r) => setTimeout(r, opts.intervalMs)); + } + throw new Error("timed out"); +} + +describeLive("browser (live): remote CDP tab persistence", () => { + it("creates, lists, focuses, and closes tabs via Playwright", { timeout: 60_000 }, async () => { + const pw = await import("./pw-ai.js"); + await pw.closePlaywrightBrowserConnection().catch(() => {}); + + const created = await pw.createPageViaPlaywright({ cdpUrl: CDP_URL, url: "about:blank" }); + try { + await waitFor( + async () => { + const pages = await pw.listPagesViaPlaywright({ cdpUrl: CDP_URL }); + return pages.some((p) => p.targetId === created.targetId); + }, + { timeoutMs: 10_000, intervalMs: 250 }, + ); + + await pw.focusPageByTargetIdViaPlaywright({ cdpUrl: CDP_URL, targetId: created.targetId }); + + await pw.closePageByTargetIdViaPlaywright({ cdpUrl: CDP_URL, targetId: created.targetId }); + + await waitFor( + async () => { + const pages = await pw.listPagesViaPlaywright({ cdpUrl: CDP_URL }); + return !pages.some((p) => p.targetId === created.targetId); + }, + { timeoutMs: 10_000, intervalMs: 250 }, + ); + } finally { + await pw.closePlaywrightBrowserConnection().catch(() => {}); + } + }); +}); diff --git a/src/browser/pw-session.ts b/src/browser/pw-session.ts index fb15da350..f15d3a918 100644 --- a/src/browser/pw-session.ts +++ b/src/browser/pw-session.ts @@ -480,3 +480,31 @@ export async function closePageByTargetIdViaPlaywright(opts: { } await page.close(); } + +/** + * Focus a page/tab by targetId using the persistent Playwright connection. + * Used for remote profiles where HTTP-based /json/activate can be ephemeral. + */ +export async function focusPageByTargetIdViaPlaywright(opts: { + cdpUrl: string; + targetId: string; +}): Promise { + const { browser } = await connectBrowser(opts.cdpUrl); + const page = await findPageByTargetId(browser, opts.targetId); + if (!page) { + throw new Error("tab not found"); + } + try { + await page.bringToFront(); + } catch (err) { + const session = await page.context().newCDPSession(page); + try { + await session.send("Page.bringToFront"); + return; + } catch { + throw err; + } finally { + await session.detach().catch(() => {}); + } + } +} diff --git a/src/browser/routes/agent.shared.ts b/src/browser/routes/agent.shared.ts index a09926d70..da8c608e3 100644 --- a/src/browser/routes/agent.shared.ts +++ b/src/browser/routes/agent.shared.ts @@ -1,6 +1,8 @@ import type express from "express"; import type { BrowserRouteContext, ProfileContext } from "../server-context.js"; +import type { PwAiModule } from "../pw-ai-module.js"; +import { getPwAiModule as getPwAiModuleBase } from "../pw-ai-module.js"; import { getProfileContext, jsonError } from "./utils.js"; export const SELECTOR_UNSUPPORTED_MESSAGE = [ @@ -38,20 +40,8 @@ export function resolveProfileContext( return profileCtx; } -export type PwAiModule = typeof import("../pw-ai.js"); - -let pwAiModule: Promise | null = null; - export async function getPwAiModule(): Promise { - if (pwAiModule) return pwAiModule; - pwAiModule = (async () => { - try { - return await import("../pw-ai.js"); - } catch { - return null; - } - })(); - return pwAiModule; + return await getPwAiModuleBase({ mode: "soft" }); } export async function requirePwAi( diff --git a/src/browser/server-context.remote-tab-ops.test.ts b/src/browser/server-context.remote-tab-ops.test.ts index ad678331f..379394497 100644 --- a/src/browser/server-context.remote-tab-ops.test.ts +++ b/src/browser/server-context.remote-tab-ops.test.ts @@ -93,6 +93,94 @@ describe("browser server-context remote profile tab operations", () => { expect(fetchMock).not.toHaveBeenCalled(); }); + it("prefers lastTargetId for remote profiles when targetId is omitted", async () => { + vi.resetModules(); + const responses = [ + // ensureTabAvailable() calls listTabs twice + [ + { targetId: "A", title: "A", url: "https://a.example", type: "page" }, + { targetId: "B", title: "B", url: "https://b.example", type: "page" }, + ], + [ + { targetId: "A", title: "A", url: "https://a.example", type: "page" }, + { targetId: "B", title: "B", url: "https://b.example", type: "page" }, + ], + // second ensureTabAvailable() calls listTabs twice, order flips + [ + { targetId: "B", title: "B", url: "https://b.example", type: "page" }, + { targetId: "A", title: "A", url: "https://a.example", type: "page" }, + ], + [ + { targetId: "B", title: "B", url: "https://b.example", type: "page" }, + { targetId: "A", title: "A", url: "https://a.example", type: "page" }, + ], + ]; + + const listPagesViaPlaywright = vi.fn(async () => { + const next = responses.shift(); + if (!next) throw new Error("no more responses"); + return next; + }); + + vi.doMock("./pw-ai.js", () => ({ + listPagesViaPlaywright, + createPageViaPlaywright: vi.fn(async () => { + throw new Error("unexpected create"); + }), + closePageByTargetIdViaPlaywright: vi.fn(async () => { + throw new Error("unexpected close"); + }), + })); + + const fetchMock = vi.fn(async () => { + throw new Error("unexpected fetch"); + }); + // @ts-expect-error test override + global.fetch = fetchMock; + + const { createBrowserRouteContext } = await import("./server-context.js"); + const state = makeState("remote"); + const ctx = createBrowserRouteContext({ getState: () => state }); + const remote = ctx.forProfile("remote"); + + const first = await remote.ensureTabAvailable(); + expect(first.targetId).toBe("A"); + const second = await remote.ensureTabAvailable(); + expect(second.targetId).toBe("A"); + }); + + it("uses Playwright focus for remote profiles when available", async () => { + vi.resetModules(); + const listPagesViaPlaywright = vi.fn(async () => [ + { targetId: "T1", title: "Tab 1", url: "https://a.example", type: "page" }, + ]); + const focusPageByTargetIdViaPlaywright = vi.fn(async () => {}); + + vi.doMock("./pw-ai.js", () => ({ + listPagesViaPlaywright, + focusPageByTargetIdViaPlaywright, + })); + + const fetchMock = vi.fn(async () => { + throw new Error("unexpected fetch"); + }); + // @ts-expect-error test override + global.fetch = fetchMock; + + const { createBrowserRouteContext } = await import("./server-context.js"); + const state = makeState("remote"); + const ctx = createBrowserRouteContext({ getState: () => state }); + const remote = ctx.forProfile("remote"); + + await remote.focusTab("T1"); + expect(focusPageByTargetIdViaPlaywright).toHaveBeenCalledWith({ + cdpUrl: "https://browserless.example/chrome?token=abc", + targetId: "T1", + }); + expect(fetchMock).not.toHaveBeenCalled(); + expect(state.profiles.get("remote")?.lastTargetId).toBe("T1"); + }); + it("does not swallow Playwright runtime errors for remote profiles", async () => { vi.resetModules(); vi.doMock("./pw-ai.js", () => ({ diff --git a/src/browser/server-context.ts b/src/browser/server-context.ts index 80d7de8a2..d39d8a7f3 100644 --- a/src/browser/server-context.ts +++ b/src/browser/server-context.ts @@ -1,7 +1,5 @@ import fs from "node:fs"; -import { extractErrorCode, formatErrorMessage } from "../infra/errors.js"; - import { appendCdpPath, createTargetViaCdp, getHeadersWithAuth, normalizeCdpWsUrl } from "./cdp.js"; import { isChromeCdpReady, @@ -24,39 +22,11 @@ import { ensureChromeExtensionRelayServer, stopChromeExtensionRelayServer, } from "./extension-relay.js"; +import type { PwAiModule } from "./pw-ai-module.js"; +import { getPwAiModule } from "./pw-ai-module.js"; import { resolveTargetIdFromTabs } from "./target-id.js"; import { movePathToTrash } from "./trash.js"; -type PwAiModule = typeof import("./pw-ai.js"); - -let pwAiModule: Promise | null = null; - -function isModuleNotFoundError(err: unknown): boolean { - const code = extractErrorCode(err); - if (code === "ERR_MODULE_NOT_FOUND") return true; - const msg = formatErrorMessage(err); - return ( - msg.includes("Cannot find module") || - msg.includes("Cannot find package") || - msg.includes("Failed to resolve import") || - msg.includes("Failed to resolve entry for package") || - msg.includes("Failed to load url") - ); -} - -async function getPwAiModule(): Promise { - if (pwAiModule) return pwAiModule; - pwAiModule = (async () => { - try { - return await import("./pw-ai.js"); - } catch (err) { - if (isModuleNotFoundError(err)) return null; - throw err; - } - })(); - return pwAiModule; -} - export type { BrowserRouteContext, BrowserServerState, @@ -134,7 +104,7 @@ function createProfileContext( const listTabs = async (): Promise => { // For remote profiles, use Playwright's persistent connection to avoid ephemeral sessions if (!profile.cdpIsLoopback) { - const mod = await getPwAiModule(); + const mod = await getPwAiModule({ mode: "strict" }); const listPagesViaPlaywright = (mod as Partial | null)?.listPagesViaPlaywright; if (typeof listPagesViaPlaywright === "function") { const pages = await listPagesViaPlaywright({ cdpUrl: profile.cdpUrl }); @@ -171,7 +141,7 @@ function createProfileContext( // For remote profiles, use Playwright's persistent connection to create tabs // This ensures the tab persists beyond a single request if (!profile.cdpIsLoopback) { - const mod = await getPwAiModule(); + const mod = await getPwAiModule({ mode: "strict" }); const createPageViaPlaywright = (mod as Partial | null)?.createPageViaPlaywright; if (typeof createPageViaPlaywright === "function") { const page = await createPageViaPlaywright({ cdpUrl: profile.cdpUrl, url }); @@ -437,6 +407,22 @@ function createProfileContext( } throw new Error("tab not found"); } + + if (!profile.cdpIsLoopback) { + const mod = await getPwAiModule({ mode: "strict" }); + const focusPageByTargetIdViaPlaywright = (mod as Partial | null) + ?.focusPageByTargetIdViaPlaywright; + if (typeof focusPageByTargetIdViaPlaywright === "function") { + await focusPageByTargetIdViaPlaywright({ + cdpUrl: profile.cdpUrl, + targetId: resolved.targetId, + }); + const profileState = getProfileState(); + profileState.lastTargetId = resolved.targetId; + return; + } + } + await fetchOk(appendCdpPath(profile.cdpUrl, `/json/activate/${resolved.targetId}`)); const profileState = getProfileState(); profileState.lastTargetId = resolved.targetId; @@ -454,7 +440,7 @@ function createProfileContext( // For remote profiles, use Playwright's persistent connection to close tabs if (!profile.cdpIsLoopback) { - const mod = await getPwAiModule(); + const mod = await getPwAiModule({ mode: "strict" }); const closePageByTargetIdViaPlaywright = (mod as Partial | null) ?.closePageByTargetIdViaPlaywright; if (typeof closePageByTargetIdViaPlaywright === "function") {