diff --git a/CHANGELOG.md b/CHANGELOG.md index fa58f250b..1b21ff8cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ - Sessions: reset `compactionCount` on `/new` and `/reset`, and preserve `sessions.json` file mode (0600). - 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. - 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/auto-reply/reply/stage-sandbox-media.ts b/src/auto-reply/reply/stage-sandbox-media.ts index a2944a4c9..88a39b007 100644 --- a/src/auto-reply/reply/stage-sandbox-media.ts +++ b/src/auto-reply/reply/stage-sandbox-media.ts @@ -33,7 +33,9 @@ export async function stageSandboxMedia(params: { }); // For remote attachments without sandbox, use ~/.clawdbot/media (not agent workspace for privacy) - const remoteMediaCacheDir = ctx.MediaRemoteHost ? path.join(CONFIG_DIR, "media", "remote-cache", sessionKey) : null; + const remoteMediaCacheDir = ctx.MediaRemoteHost + ? path.join(CONFIG_DIR, "media", "remote-cache", sessionKey) + : null; const effectiveWorkspaceDir = sandbox?.workspaceDir ?? remoteMediaCacheDir; if (!effectiveWorkspaceDir) return; @@ -53,7 +55,9 @@ export async function stageSandboxMedia(params: { try { // For sandbox: /media/inbound, for remote cache: use dir directly - const destDir = sandbox ? path.join(effectiveWorkspaceDir, "media", "inbound") : effectiveWorkspaceDir; + const destDir = sandbox + ? path.join(effectiveWorkspaceDir, "media", "inbound") + : effectiveWorkspaceDir; await fs.mkdir(destDir, { recursive: true }); const usedNames = new Set(); diff --git a/src/browser/pw-session.ts b/src/browser/pw-session.ts index f99694dc8..fb15da350 100644 --- a/src/browser/pw-session.ts +++ b/src/browser/pw-session.ts @@ -394,9 +394,7 @@ export async function closePlaywrightBrowserConnection(): Promise { * List all pages/tabs from the persistent Playwright connection. * Used for remote profiles where HTTP-based /json/list is ephemeral. */ -export async function listPagesViaPlaywright(opts: { - cdpUrl: string; -}): Promise< +export async function listPagesViaPlaywright(opts: { cdpUrl: string }): Promise< Array<{ targetId: string; title: string; @@ -432,25 +430,15 @@ export async function listPagesViaPlaywright(opts: { * Used for remote profiles where HTTP-based /json/new is ephemeral. * Returns the new page's targetId and metadata. */ -export async function createPageViaPlaywright(opts: { - cdpUrl: string; - url: string; -}): Promise<{ +export async function createPageViaPlaywright(opts: { cdpUrl: string; url: string }): Promise<{ targetId: string; title: string; url: string; type: string; }> { const { browser } = await connectBrowser(opts.cdpUrl); - const contexts = browser.contexts(); - // Use the first context if available, otherwise this is a fresh connection - // and we need to use the default context that Browserless provides - let context = contexts[0]; - if (!context) { - // For Browserless over CDP, there should be at least one context - // If not, we can try accessing pages directly from contexts - throw new Error("No browser context available for creating a new page"); - } + const context = browser.contexts()[0] ?? (await browser.newContext()); + ensureContextState(context); const page = await context.newPage(); ensurePageState(page); diff --git a/src/browser/server-context.remote-tab-ops.test.ts b/src/browser/server-context.remote-tab-ops.test.ts new file mode 100644 index 000000000..ad678331f --- /dev/null +++ b/src/browser/server-context.remote-tab-ops.test.ts @@ -0,0 +1,197 @@ +import { describe, expect, it, vi } from "vitest"; + +import type { BrowserServerState } from "./server-context.js"; + +vi.mock("./chrome.js", () => ({ + isChromeCdpReady: vi.fn(async () => true), + isChromeReachable: vi.fn(async () => true), + launchClawdChrome: vi.fn(async () => { + throw new Error("unexpected launch"); + }), + resolveClawdUserDataDir: vi.fn(() => "/tmp/clawd"), + stopClawdChrome: vi.fn(async () => {}), +})); + +function makeState( + profile: "remote" | "clawd", +): BrowserServerState & { profiles: Map } { + return { + // biome-ignore lint/suspicious/noExplicitAny: test stub + server: null as any, + port: 0, + resolved: { + enabled: true, + controlUrl: "http://127.0.0.1:18791", + controlHost: "127.0.0.1", + controlPort: 18791, + cdpProtocol: profile === "remote" ? "https" : "http", + cdpHost: profile === "remote" ? "browserless.example" : "127.0.0.1", + cdpIsLoopback: profile !== "remote", + remoteCdpTimeoutMs: 1500, + remoteCdpHandshakeTimeoutMs: 3000, + color: "#FF4500", + headless: true, + noSandbox: false, + attachOnly: false, + defaultProfile: profile, + profiles: { + remote: { + cdpUrl: "https://browserless.example/chrome?token=abc", + cdpPort: 443, + color: "#00AA00", + }, + clawd: { cdpPort: 18800, color: "#FF4500" }, + }, + }, + profiles: new Map(), + }; +} + +describe("browser server-context remote profile tab operations", () => { + it("uses Playwright tab operations when available", async () => { + vi.resetModules(); + const listPagesViaPlaywright = vi.fn(async () => [ + { targetId: "T1", title: "Tab 1", url: "https://a.example", type: "page" }, + ]); + const createPageViaPlaywright = vi.fn(async () => ({ + targetId: "T2", + title: "Tab 2", + url: "https://b.example", + type: "page", + })); + const closePageByTargetIdViaPlaywright = vi.fn(async () => {}); + + vi.doMock("./pw-ai.js", () => ({ + listPagesViaPlaywright, + createPageViaPlaywright, + closePageByTargetIdViaPlaywright, + })); + + 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 tabs = await remote.listTabs(); + expect(tabs.map((t) => t.targetId)).toEqual(["T1"]); + + const opened = await remote.openTab("https://b.example"); + expect(opened.targetId).toBe("T2"); + expect(state.profiles.get("remote")?.lastTargetId).toBe("T2"); + + await remote.closeTab("T1"); + expect(closePageByTargetIdViaPlaywright).toHaveBeenCalledWith({ + cdpUrl: "https://browserless.example/chrome?token=abc", + targetId: "T1", + }); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("does not swallow Playwright runtime errors for remote profiles", async () => { + vi.resetModules(); + vi.doMock("./pw-ai.js", () => ({ + listPagesViaPlaywright: vi.fn(async () => { + throw new Error("boom"); + }), + })); + + 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 expect(remote.listTabs()).rejects.toThrow(/boom/); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("falls back to /json/list when Playwright is not available", async () => { + vi.resetModules(); + vi.doMock("./pw-ai.js", () => ({ + listPagesViaPlaywright: undefined, + createPageViaPlaywright: undefined, + closePageByTargetIdViaPlaywright: undefined, + })); + + const fetchMock = vi.fn(async (url: unknown) => { + const u = String(url); + if (!u.includes("/json/list")) throw new Error(`unexpected fetch: ${u}`); + return { + ok: true, + json: async () => [ + { + id: "T1", + title: "Tab 1", + url: "https://a.example", + webSocketDebuggerUrl: "wss://browserless.example/devtools/page/T1", + type: "page", + }, + ], + } as unknown as Response; + }); + // @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 tabs = await remote.listTabs(); + expect(tabs.map((t) => t.targetId)).toEqual(["T1"]); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); +}); + +describe("browser server-context tab selection state", () => { + it("updates lastTargetId when openTab is created via CDP", async () => { + vi.resetModules(); + vi.doUnmock("./pw-ai.js"); + vi.doMock("./cdp.js", async () => { + const actual = await vi.importActual("./cdp.js"); + return { + ...actual, + createTargetViaCdp: vi.fn(async () => ({ targetId: "CREATED" })), + }; + }); + + const fetchMock = vi.fn(async (url: unknown) => { + const u = String(url); + if (!u.includes("/json/list")) throw new Error(`unexpected fetch: ${u}`); + return { + ok: true, + json: async () => [ + { + id: "CREATED", + title: "New Tab", + url: "https://created.example", + webSocketDebuggerUrl: "ws://127.0.0.1/devtools/page/CREATED", + type: "page", + }, + ], + } as unknown as Response; + }); + // @ts-expect-error test override + global.fetch = fetchMock; + + const { createBrowserRouteContext } = await import("./server-context.js"); + const state = makeState("clawd"); + const ctx = createBrowserRouteContext({ getState: () => state }); + const clawd = ctx.forProfile("clawd"); + + const opened = await clawd.openTab("https://created.example"); + expect(opened.targetId).toBe("CREATED"); + expect(state.profiles.get("clawd")?.lastTargetId).toBe("CREATED"); + }); +}); diff --git a/src/browser/server-context.ts b/src/browser/server-context.ts index e18deeeb3..80d7de8a2 100644 --- a/src/browser/server-context.ts +++ b/src/browser/server-context.ts @@ -1,5 +1,7 @@ import fs from "node:fs"; +import { extractErrorCode, formatErrorMessage } from "../infra/errors.js"; + import { appendCdpPath, createTargetViaCdp, getHeadersWithAuth, normalizeCdpWsUrl } from "./cdp.js"; import { isChromeCdpReady, @@ -25,6 +27,36 @@ import { 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, @@ -102,20 +134,19 @@ function createProfileContext( const listTabs = async (): Promise => { // For remote profiles, use Playwright's persistent connection to avoid ephemeral sessions if (!profile.cdpIsLoopback) { - try { - const mod = await import("./pw-ai.js"); - const pages = await mod.listPagesViaPlaywright({ cdpUrl: profile.cdpUrl }); + const mod = await getPwAiModule(); + const listPagesViaPlaywright = (mod as Partial | null)?.listPagesViaPlaywright; + if (typeof listPagesViaPlaywright === "function") { + const pages = await listPagesViaPlaywright({ cdpUrl: profile.cdpUrl }); return pages.map((p) => ({ targetId: p.targetId, title: p.title, url: p.url, type: p.type, })); - } catch { - // Fall back to HTTP-based listing if Playwright is not available } } - + const raw = await fetchJson< Array<{ id?: string; @@ -140,9 +171,10 @@ 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) { - try { - const mod = await import("./pw-ai.js"); - const page = await mod.createPageViaPlaywright({ cdpUrl: profile.cdpUrl, url }); + const mod = await getPwAiModule(); + const createPageViaPlaywright = (mod as Partial | null)?.createPageViaPlaywright; + if (typeof createPageViaPlaywright === "function") { + const page = await createPageViaPlaywright({ cdpUrl: profile.cdpUrl, url }); const profileState = getProfileState(); profileState.lastTargetId = page.targetId; return { @@ -151,16 +183,9 @@ function createProfileContext( url: page.url, type: page.type, }; - } catch (err) { - // Fall back to HTTP-based tab creation if Playwright is not available - // (though it will likely be ephemeral for remote profiles) - if (String(err).includes("No browser context")) { - // This is a real error, not a missing Playwright issue - throw err; - } } } - + const createdViaCdp = await createTargetViaCdp({ cdpUrl: profile.cdpUrl, url, @@ -169,6 +194,8 @@ function createProfileContext( .catch(() => null); if (createdViaCdp) { + const profileState = getProfileState(); + profileState.lastTargetId = createdViaCdp; const deadline = Date.now() + 2000; while (Date.now() < deadline) { const tabs = await listTabs().catch(() => [] as BrowserTab[]); @@ -363,9 +390,10 @@ function createProfileContext( const tabs = await listTabs(); // For remote profiles using Playwright's persistent connection, we don't need wsUrl // because we access pages directly through Playwright, not via individual WebSocket URLs. - const candidates = profile.driver === "extension" || !profile.cdpIsLoopback - ? tabs - : tabs.filter((t) => Boolean(t.wsUrl)); + const candidates = + profile.driver === "extension" || !profile.cdpIsLoopback + ? tabs + : tabs.filter((t) => Boolean(t.wsUrl)); const resolveById = (raw: string) => { const resolved = resolveTargetIdFromTabs(raw, candidates); @@ -423,21 +451,21 @@ function createProfileContext( } throw new Error("tab not found"); } - + // For remote profiles, use Playwright's persistent connection to close tabs if (!profile.cdpIsLoopback) { - try { - const mod = await import("./pw-ai.js"); - await mod.closePageByTargetIdViaPlaywright({ + const mod = await getPwAiModule(); + const closePageByTargetIdViaPlaywright = (mod as Partial | null) + ?.closePageByTargetIdViaPlaywright; + if (typeof closePageByTargetIdViaPlaywright === "function") { + await closePageByTargetIdViaPlaywright({ cdpUrl: profile.cdpUrl, targetId: resolved.targetId, }); return; - } catch { - // Fall back to HTTP-based close if Playwright is not available } } - + await fetchOk(appendCdpPath(profile.cdpUrl, `/json/close/${resolved.targetId}`)); };