mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
refactor(browser): scope CDP sessions and harden stale target recovery
This commit is contained in:
@@ -74,6 +74,17 @@ function stripTargetIdFromActRequest(
|
||||
return retryRequest as Parameters<typeof browserAct>[1];
|
||||
}
|
||||
|
||||
function canRetryChromeActWithoutTargetId(request: Parameters<typeof browserAct>[1]): boolean {
|
||||
const typedRequest = request as Partial<Record<"kind" | "action", unknown>>;
|
||||
const kind =
|
||||
typeof typedRequest.kind === "string"
|
||||
? typedRequest.kind
|
||||
: typeof typedRequest.action === "string"
|
||||
? typedRequest.action
|
||||
: "";
|
||||
return kind === "hover" || kind === "scrollIntoView" || kind === "wait";
|
||||
}
|
||||
|
||||
export async function executeTabsAction(params: {
|
||||
baseUrl?: string;
|
||||
profile?: string;
|
||||
@@ -304,9 +315,18 @@ export async function executeActAction(params: {
|
||||
} catch (err) {
|
||||
if (isChromeStaleTargetError(profile, err)) {
|
||||
const retryRequest = stripTargetIdFromActRequest(request);
|
||||
const tabs = proxyRequest
|
||||
? ((
|
||||
(await proxyRequest({
|
||||
method: "GET",
|
||||
path: "/tabs",
|
||||
profile,
|
||||
})) as { tabs?: unknown[] }
|
||||
).tabs ?? [])
|
||||
: await browserTabs(baseUrl, { profile }).catch(() => []);
|
||||
// Some Chrome relay targetIds can go stale between snapshots and actions.
|
||||
// Retry once without targetId to let relay use the currently attached tab.
|
||||
if (retryRequest) {
|
||||
// Only retry safe read-only actions, and only when exactly one tab remains attached.
|
||||
if (retryRequest && canRetryChromeActWithoutTargetId(request) && tabs.length === 1) {
|
||||
try {
|
||||
const retryResult = proxyRequest
|
||||
? await proxyRequest({
|
||||
@@ -323,15 +343,6 @@ export async function executeActAction(params: {
|
||||
// Fall through to explicit stale-target guidance.
|
||||
}
|
||||
}
|
||||
const tabs = proxyRequest
|
||||
? ((
|
||||
(await proxyRequest({
|
||||
method: "GET",
|
||||
path: "/tabs",
|
||||
profile,
|
||||
})) as { tabs?: unknown[] }
|
||||
).tabs ?? [])
|
||||
: await browserTabs(baseUrl, { profile }).catch(() => []);
|
||||
if (!tabs.length) {
|
||||
throw new Error(
|
||||
"No Chrome tabs are attached via the OpenClaw Browser Relay extension. Click the toolbar icon on the tab you want to control (badge ON), then retry.",
|
||||
|
||||
@@ -571,17 +571,18 @@ describe("browser tool external content wrapping", () => {
|
||||
describe("browser tool act stale target recovery", () => {
|
||||
registerBrowserToolAfterEachReset();
|
||||
|
||||
it("retries chrome act once without targetId when tab id is stale", async () => {
|
||||
it("retries safe chrome act once without targetId when exactly one tab remains", async () => {
|
||||
browserActionsMocks.browserAct
|
||||
.mockRejectedValueOnce(new Error("404: tab not found"))
|
||||
.mockResolvedValueOnce({ ok: true });
|
||||
browserClientMocks.browserTabs.mockResolvedValueOnce([{ targetId: "only-tab" }]);
|
||||
|
||||
const tool = createBrowserTool();
|
||||
const result = await tool.execute?.("call-1", {
|
||||
action: "act",
|
||||
profile: "chrome",
|
||||
request: {
|
||||
action: "click",
|
||||
kind: "hover",
|
||||
targetId: "stale-tab",
|
||||
ref: "btn-1",
|
||||
},
|
||||
@@ -591,7 +592,7 @@ describe("browser tool act stale target recovery", () => {
|
||||
expect(browserActionsMocks.browserAct).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
undefined,
|
||||
expect.objectContaining({ targetId: "stale-tab", action: "click", ref: "btn-1" }),
|
||||
expect.objectContaining({ targetId: "stale-tab", kind: "hover", ref: "btn-1" }),
|
||||
expect.objectContaining({ profile: "chrome" }),
|
||||
);
|
||||
expect(browserActionsMocks.browserAct).toHaveBeenNthCalledWith(
|
||||
@@ -602,4 +603,24 @@ describe("browser tool act stale target recovery", () => {
|
||||
);
|
||||
expect(result?.details).toMatchObject({ ok: true });
|
||||
});
|
||||
|
||||
it("does not retry mutating chrome act requests without targetId", async () => {
|
||||
browserActionsMocks.browserAct.mockRejectedValueOnce(new Error("404: tab not found"));
|
||||
browserClientMocks.browserTabs.mockResolvedValueOnce([{ targetId: "only-tab" }]);
|
||||
|
||||
const tool = createBrowserTool();
|
||||
await expect(
|
||||
tool.execute?.("call-1", {
|
||||
action: "act",
|
||||
profile: "chrome",
|
||||
request: {
|
||||
kind: "click",
|
||||
targetId: "stale-tab",
|
||||
ref: "btn-1",
|
||||
},
|
||||
}),
|
||||
).rejects.toThrow(/Run action=tabs profile="chrome"/i);
|
||||
|
||||
expect(browserActionsMocks.browserAct).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -166,11 +166,23 @@ describe("cdp.helpers", () => {
|
||||
expect(url).toBe("https://connect.example.com/?token=abc");
|
||||
});
|
||||
|
||||
it("preserves auth and query params when normalizing secure loopback WebSocket CDP URLs", () => {
|
||||
const url = normalizeCdpHttpBaseForJsonEndpoints(
|
||||
"wss://user:pass@127.0.0.1:9222/devtools/browser/ABC?token=abc",
|
||||
);
|
||||
expect(url).toBe("https://user:pass@127.0.0.1:9222/?token=abc");
|
||||
});
|
||||
|
||||
it("strips a trailing /cdp suffix when normalizing HTTP bases", () => {
|
||||
const url = normalizeCdpHttpBaseForJsonEndpoints("ws://127.0.0.1:9222/cdp?token=abc");
|
||||
expect(url).toBe("http://127.0.0.1:9222/?token=abc");
|
||||
});
|
||||
|
||||
it("preserves base prefixes when stripping a trailing /cdp suffix", () => {
|
||||
const url = normalizeCdpHttpBaseForJsonEndpoints("ws://127.0.0.1:9222/browser/cdp?token=abc");
|
||||
expect(url).toBe("http://127.0.0.1:9222/browser?token=abc");
|
||||
});
|
||||
|
||||
it("adds basic auth headers when credentials are present", () => {
|
||||
const headers = getHeadersWithAuth("https://user:pass@example.com");
|
||||
expect(headers.Authorization).toBe(`Basic ${Buffer.from("user:pass").toString("base64")}`);
|
||||
|
||||
@@ -336,6 +336,26 @@ describe("cdp", () => {
|
||||
expect(normalized).toBe("ws://192.168.1.202:18850/devtools/browser/ABC");
|
||||
});
|
||||
|
||||
it("keeps existing websocket query params when appending remote CDP query params", () => {
|
||||
const normalized = normalizeCdpWsUrl(
|
||||
"ws://127.0.0.1:9222/devtools/browser/ABC?session=1&token=ws-token",
|
||||
"http://127.0.0.1:9222?token=cdp-token&apiKey=abc",
|
||||
);
|
||||
expect(normalized).toBe(
|
||||
"ws://127.0.0.1:9222/devtools/browser/ABC?session=1&token=ws-token&apiKey=abc",
|
||||
);
|
||||
});
|
||||
|
||||
it("rewrites wildcard bind addresses to secure remote CDP hosts without clobbering websocket params", () => {
|
||||
const normalized = normalizeCdpWsUrl(
|
||||
"ws://0.0.0.0:3000/devtools/browser/ABC?session=1&token=ws-token",
|
||||
"https://user:pass@example.com:9443?token=cdp-token&apiKey=abc",
|
||||
);
|
||||
expect(normalized).toBe(
|
||||
"wss://user:pass@example.com:9443/devtools/browser/ABC?session=1&token=ws-token&apiKey=abc",
|
||||
);
|
||||
});
|
||||
|
||||
it("upgrades ws to wss when CDP uses https", () => {
|
||||
const normalized = normalizeCdpWsUrl(
|
||||
"ws://production-sfo.browserless.io",
|
||||
|
||||
@@ -176,6 +176,28 @@ describe("browser config", () => {
|
||||
expect(profile?.cdpIsLoopback).toBe(false);
|
||||
});
|
||||
|
||||
it("preserves loopback direct WebSocket cdpUrl for explicit profiles", () => {
|
||||
const resolved = resolveBrowserConfig({
|
||||
profiles: {
|
||||
localws: {
|
||||
cdpUrl: "ws://127.0.0.1:9222/devtools/browser/ABC?token=test-key",
|
||||
color: "#0066CC",
|
||||
},
|
||||
},
|
||||
});
|
||||
const profile = resolveProfile(resolved, "localws");
|
||||
expect(profile?.cdpUrl).toBe("ws://127.0.0.1:9222/devtools/browser/ABC?token=test-key");
|
||||
expect(profile?.cdpPort).toBe(9222);
|
||||
expect(profile?.cdpIsLoopback).toBe(true);
|
||||
});
|
||||
|
||||
it("trims relayBindHost when configured", () => {
|
||||
const resolved = resolveBrowserConfig({
|
||||
relayBindHost: " 0.0.0.0 ",
|
||||
});
|
||||
expect(resolved.relayBindHost).toBe("0.0.0.0");
|
||||
});
|
||||
|
||||
it("rejects unsupported protocols", () => {
|
||||
expect(() => resolveBrowserConfig({ cdpUrl: "ftp://127.0.0.1:18791" })).toThrow(
|
||||
"must be http(s) or ws(s)",
|
||||
|
||||
49
src/browser/extension-relay.bind-host.test.ts
Normal file
49
src/browser/extension-relay.bind-host.test.ts
Normal file
@@ -0,0 +1,49 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { captureEnv } from "../test-utils/env.js";
|
||||
import {
|
||||
ensureChromeExtensionRelayServer,
|
||||
stopChromeExtensionRelayServer,
|
||||
} from "./extension-relay.js";
|
||||
import { getFreePort } from "./test-port.js";
|
||||
|
||||
describe("chrome extension relay bindHost coordination", () => {
|
||||
let cdpUrl = "";
|
||||
let envSnapshot: ReturnType<typeof captureEnv>;
|
||||
|
||||
beforeEach(() => {
|
||||
envSnapshot = captureEnv(["OPENCLAW_GATEWAY_TOKEN"]);
|
||||
process.env.OPENCLAW_GATEWAY_TOKEN = "test-gateway-token";
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
if (cdpUrl) {
|
||||
await stopChromeExtensionRelayServer({ cdpUrl }).catch(() => {});
|
||||
cdpUrl = "";
|
||||
}
|
||||
envSnapshot.restore();
|
||||
});
|
||||
|
||||
it("rebinds the relay when concurrent callers request different bind hosts", async () => {
|
||||
const port = await getFreePort();
|
||||
cdpUrl = `http://127.0.0.1:${port}`;
|
||||
|
||||
const [first, second] = await Promise.all([
|
||||
ensureChromeExtensionRelayServer({ cdpUrl }),
|
||||
ensureChromeExtensionRelayServer({ cdpUrl, bindHost: "0.0.0.0" }),
|
||||
]);
|
||||
|
||||
const settled = await ensureChromeExtensionRelayServer({
|
||||
cdpUrl,
|
||||
bindHost: "0.0.0.0",
|
||||
});
|
||||
|
||||
expect(first.port).toBe(port);
|
||||
expect(second.port).toBe(port);
|
||||
expect(second).not.toBe(first);
|
||||
expect(second.bindHost).toBe("0.0.0.0");
|
||||
expect(settled).toBe(second);
|
||||
|
||||
const res = await fetch(`http://127.0.0.1:${port}/`);
|
||||
expect(res.status).toBe(200);
|
||||
});
|
||||
});
|
||||
119
src/browser/pw-session.connections.test.ts
Normal file
119
src/browser/pw-session.connections.test.ts
Normal file
@@ -0,0 +1,119 @@
|
||||
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";
|
||||
|
||||
const connectOverCdpSpy = vi.spyOn(chromium, "connectOverCDP");
|
||||
const getChromeWebSocketUrlSpy = vi.spyOn(chromeModule, "getChromeWebSocketUrl");
|
||||
|
||||
type BrowserMockBundle = {
|
||||
browser: import("playwright-core").Browser;
|
||||
browserClose: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
|
||||
function makeBrowser(targetId: string, url: string): BrowserMockBundle {
|
||||
let context: import("playwright-core").BrowserContext;
|
||||
const browserClose = vi.fn(async () => {});
|
||||
const page = {
|
||||
on: vi.fn(),
|
||||
context: () => context,
|
||||
title: vi.fn(async () => `title:${targetId}`),
|
||||
url: vi.fn(() => url),
|
||||
} as unknown as import("playwright-core").Page;
|
||||
|
||||
context = {
|
||||
pages: () => [page],
|
||||
on: vi.fn(),
|
||||
newCDPSession: vi.fn(async () => ({
|
||||
send: vi.fn(async (method: string) =>
|
||||
method === "Target.getTargetInfo" ? { targetInfo: { targetId } } : {},
|
||||
),
|
||||
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 };
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
connectOverCdpSpy.mockReset();
|
||||
getChromeWebSocketUrlSpy.mockReset();
|
||||
await closePlaywrightBrowserConnection().catch(() => {});
|
||||
});
|
||||
|
||||
describe("pw-session connection scoping", () => {
|
||||
it("does not share in-flight connectOverCDP promises across different cdpUrls", async () => {
|
||||
const browserA = makeBrowser("A", "https://a.example");
|
||||
const browserB = makeBrowser("B", "https://b.example");
|
||||
let resolveA: ((value: import("playwright-core").Browser) => void) | undefined;
|
||||
|
||||
connectOverCdpSpy.mockImplementation((async (...args: unknown[]) => {
|
||||
const endpointText = String(args[0]);
|
||||
if (endpointText === "http://127.0.0.1:9222") {
|
||||
return await new Promise<import("playwright-core").Browser>((resolve) => {
|
||||
resolveA = resolve;
|
||||
});
|
||||
}
|
||||
if (endpointText === "http://127.0.0.1:9333") {
|
||||
return browserB.browser;
|
||||
}
|
||||
throw new Error(`unexpected endpoint: ${endpointText}`);
|
||||
}) as never);
|
||||
getChromeWebSocketUrlSpy.mockResolvedValue(null);
|
||||
|
||||
const pendingA = listPagesViaPlaywright({ cdpUrl: "http://127.0.0.1:9222" });
|
||||
await Promise.resolve();
|
||||
const pendingB = listPagesViaPlaywright({ cdpUrl: "http://127.0.0.1:9333" });
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(connectOverCdpSpy).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
expect(connectOverCdpSpy).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
"http://127.0.0.1:9222",
|
||||
expect.any(Object),
|
||||
);
|
||||
expect(connectOverCdpSpy).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
"http://127.0.0.1:9333",
|
||||
expect.any(Object),
|
||||
);
|
||||
|
||||
resolveA?.(browserA.browser);
|
||||
const [pagesA, pagesB] = await Promise.all([pendingA, pendingB]);
|
||||
expect(pagesA.map((page) => page.targetId)).toEqual(["A"]);
|
||||
expect(pagesB.map((page) => page.targetId)).toEqual(["B"]);
|
||||
});
|
||||
|
||||
it("closes only the requested scoped connection", async () => {
|
||||
const browserA = makeBrowser("A", "https://a.example");
|
||||
const browserB = makeBrowser("B", "https://b.example");
|
||||
|
||||
connectOverCdpSpy.mockImplementation((async (...args: unknown[]) => {
|
||||
const endpointText = String(args[0]);
|
||||
if (endpointText === "http://127.0.0.1:9222") {
|
||||
return browserA.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" });
|
||||
|
||||
await closePlaywrightBrowserConnection({ cdpUrl: "http://127.0.0.1:9222" });
|
||||
|
||||
expect(browserA.browserClose).toHaveBeenCalledTimes(1);
|
||||
expect(browserB.browserClose).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -1,11 +1,17 @@
|
||||
import { chromium } from "playwright-core";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import * as chromeModule from "./chrome.js";
|
||||
import { closePlaywrightBrowserConnection, getPageForTargetId } from "./pw-session.js";
|
||||
|
||||
const connectOverCdpSpy = vi.spyOn(chromium, "connectOverCDP");
|
||||
const getChromeWebSocketUrlSpy = vi.spyOn(chromeModule, "getChromeWebSocketUrl");
|
||||
|
||||
afterEach(async () => {
|
||||
connectOverCdpSpy.mockClear();
|
||||
getChromeWebSocketUrlSpy.mockClear();
|
||||
await closePlaywrightBrowserConnection().catch(() => {});
|
||||
});
|
||||
|
||||
describe("pw-session getPageForTargetId", () => {
|
||||
it("falls back to the only page when CDP session attachment is blocked (extension relays)", async () => {
|
||||
connectOverCdpSpy.mockClear();
|
||||
@@ -50,4 +56,63 @@ describe("pw-session getPageForTargetId", () => {
|
||||
await closePlaywrightBrowserConnection();
|
||||
expect(browserClose).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("uses the shared HTTP-base normalization when falling back to /json/list for direct WebSocket CDP URLs", async () => {
|
||||
const pageOn = vi.fn();
|
||||
const contextOn = vi.fn();
|
||||
const browserOn = vi.fn();
|
||||
const browserClose = vi.fn(async () => {});
|
||||
|
||||
const context = {
|
||||
pages: () => [],
|
||||
on: contextOn,
|
||||
newCDPSession: vi.fn(async () => {
|
||||
throw new Error("Not allowed");
|
||||
}),
|
||||
} as unknown as import("playwright-core").BrowserContext;
|
||||
|
||||
const pageA = {
|
||||
on: pageOn,
|
||||
context: () => context,
|
||||
url: () => "https://alpha.example",
|
||||
} as unknown as import("playwright-core").Page;
|
||||
const pageB = {
|
||||
on: pageOn,
|
||||
context: () => context,
|
||||
url: () => "https://beta.example",
|
||||
} as unknown as import("playwright-core").Page;
|
||||
|
||||
(context as unknown as { pages: () => unknown[] }).pages = () => [pageA, pageB];
|
||||
|
||||
const browser = {
|
||||
contexts: () => [context],
|
||||
on: browserOn,
|
||||
close: browserClose,
|
||||
} as unknown as import("playwright-core").Browser;
|
||||
|
||||
connectOverCdpSpy.mockResolvedValue(browser);
|
||||
getChromeWebSocketUrlSpy.mockResolvedValue(null);
|
||||
|
||||
const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => [
|
||||
{ id: "TARGET_A", url: "https://alpha.example" },
|
||||
{ id: "TARGET_B", url: "https://beta.example" },
|
||||
],
|
||||
} as Response);
|
||||
|
||||
try {
|
||||
const resolved = await getPageForTargetId({
|
||||
cdpUrl: "ws://127.0.0.1:18792/devtools/browser/SESSION?token=abc",
|
||||
targetId: "TARGET_B",
|
||||
});
|
||||
expect(resolved).toBe(pageB);
|
||||
expect(fetchSpy).toHaveBeenCalledWith(
|
||||
"http://127.0.0.1:18792/json/list?token=abc",
|
||||
expect.any(Object),
|
||||
);
|
||||
} finally {
|
||||
fetchSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -113,8 +113,8 @@ const MAX_CONSOLE_MESSAGES = 500;
|
||||
const MAX_PAGE_ERRORS = 200;
|
||||
const MAX_NETWORK_REQUESTS = 500;
|
||||
|
||||
let cached: ConnectedBrowser | null = null;
|
||||
let connecting: Promise<ConnectedBrowser> | null = null;
|
||||
const cachedByCdpUrl = new Map<string, ConnectedBrowser>();
|
||||
const connectingByCdpUrl = new Map<string, Promise<ConnectedBrowser>>();
|
||||
|
||||
function normalizeCdpUrl(raw: string) {
|
||||
return raw.replace(/\/$/, "");
|
||||
@@ -328,9 +328,11 @@ function observeBrowser(browser: Browser) {
|
||||
|
||||
async function connectBrowser(cdpUrl: string): Promise<ConnectedBrowser> {
|
||||
const normalized = normalizeCdpUrl(cdpUrl);
|
||||
if (cached?.cdpUrl === normalized) {
|
||||
const cached = cachedByCdpUrl.get(normalized);
|
||||
if (cached) {
|
||||
return cached;
|
||||
}
|
||||
const connecting = connectingByCdpUrl.get(normalized);
|
||||
if (connecting) {
|
||||
return await connecting;
|
||||
}
|
||||
@@ -348,12 +350,13 @@ async function connectBrowser(cdpUrl: string): Promise<ConnectedBrowser> {
|
||||
chromium.connectOverCDP(endpoint, { timeout, headers }),
|
||||
);
|
||||
const onDisconnected = () => {
|
||||
if (cached?.browser === browser) {
|
||||
cached = null;
|
||||
const current = cachedByCdpUrl.get(normalized);
|
||||
if (current?.browser === browser) {
|
||||
cachedByCdpUrl.delete(normalized);
|
||||
}
|
||||
};
|
||||
const connected: ConnectedBrowser = { browser, cdpUrl: normalized, onDisconnected };
|
||||
cached = connected;
|
||||
cachedByCdpUrl.set(normalized, connected);
|
||||
browser.on("disconnected", onDisconnected);
|
||||
observeBrowser(browser);
|
||||
return connected;
|
||||
@@ -370,11 +373,12 @@ async function connectBrowser(cdpUrl: string): Promise<ConnectedBrowser> {
|
||||
throw new Error(message);
|
||||
};
|
||||
|
||||
connecting = connectWithRetry().finally(() => {
|
||||
connecting = null;
|
||||
const pending = connectWithRetry().finally(() => {
|
||||
connectingByCdpUrl.delete(normalized);
|
||||
});
|
||||
connectingByCdpUrl.set(normalized, pending);
|
||||
|
||||
return await connecting;
|
||||
return await pending;
|
||||
}
|
||||
|
||||
async function getAllPages(browser: Browser): Promise<Page[]> {
|
||||
@@ -423,34 +427,29 @@ async function findPageByTargetId(
|
||||
// fall back to URL-based matching using the /json/list endpoint
|
||||
if (cdpUrl) {
|
||||
try {
|
||||
const baseUrl = cdpUrl
|
||||
.replace(/\/+$/, "")
|
||||
.replace(/^ws:/, "http:")
|
||||
.replace(/\/cdp$/, "");
|
||||
const listUrl = `${baseUrl}/json/list`;
|
||||
const response = await fetch(listUrl, { headers: getHeadersWithAuth(listUrl) });
|
||||
if (response.ok) {
|
||||
const targets = (await response.json()) as Array<{
|
||||
const cdpHttpBase = normalizeCdpHttpBaseForJsonEndpoints(cdpUrl);
|
||||
const targets = await fetchJson<
|
||||
Array<{
|
||||
id: string;
|
||||
url: string;
|
||||
title?: string;
|
||||
}>;
|
||||
const target = targets.find((t) => t.id === targetId);
|
||||
if (target) {
|
||||
// Try to find a page with matching URL
|
||||
const urlMatch = pages.filter((p) => p.url() === target.url);
|
||||
if (urlMatch.length === 1) {
|
||||
return urlMatch[0];
|
||||
}
|
||||
// If multiple URL matches, use index-based matching as fallback
|
||||
// This works when Playwright and the relay enumerate tabs in the same order
|
||||
if (urlMatch.length > 1) {
|
||||
const sameUrlTargets = targets.filter((t) => t.url === target.url);
|
||||
if (sameUrlTargets.length === urlMatch.length) {
|
||||
const idx = sameUrlTargets.findIndex((t) => t.id === targetId);
|
||||
if (idx >= 0 && idx < urlMatch.length) {
|
||||
return urlMatch[idx];
|
||||
}
|
||||
}>
|
||||
>(appendCdpPath(cdpHttpBase, "/json/list"), 2000);
|
||||
const target = targets.find((t) => t.id === targetId);
|
||||
if (target) {
|
||||
// Try to find a page with matching URL
|
||||
const urlMatch = pages.filter((p) => p.url() === target.url);
|
||||
if (urlMatch.length === 1) {
|
||||
return urlMatch[0];
|
||||
}
|
||||
// If multiple URL matches, use index-based matching as fallback
|
||||
// This works when Playwright and the relay enumerate tabs in the same order
|
||||
if (urlMatch.length > 1) {
|
||||
const sameUrlTargets = targets.filter((t) => t.url === target.url);
|
||||
if (sameUrlTargets.length === urlMatch.length) {
|
||||
const idx = sameUrlTargets.findIndex((t) => t.id === targetId);
|
||||
if (idx >= 0 && idx < urlMatch.length) {
|
||||
return urlMatch[idx];
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -539,17 +538,32 @@ export function refLocator(page: Page, ref: string) {
|
||||
return page.locator(`aria-ref=${normalized}`);
|
||||
}
|
||||
|
||||
export async function closePlaywrightBrowserConnection(): Promise<void> {
|
||||
const cur = cached;
|
||||
cached = null;
|
||||
connecting = null;
|
||||
if (!cur) {
|
||||
export async function closePlaywrightBrowserConnection(opts?: { cdpUrl?: string }): Promise<void> {
|
||||
const normalized = opts?.cdpUrl ? normalizeCdpUrl(opts.cdpUrl) : null;
|
||||
|
||||
if (normalized) {
|
||||
const cur = cachedByCdpUrl.get(normalized);
|
||||
cachedByCdpUrl.delete(normalized);
|
||||
connectingByCdpUrl.delete(normalized);
|
||||
if (!cur) {
|
||||
return;
|
||||
}
|
||||
if (cur.onDisconnected && typeof cur.browser.off === "function") {
|
||||
cur.browser.off("disconnected", cur.onDisconnected);
|
||||
}
|
||||
await cur.browser.close().catch(() => {});
|
||||
return;
|
||||
}
|
||||
if (cur.onDisconnected && typeof cur.browser.off === "function") {
|
||||
cur.browser.off("disconnected", cur.onDisconnected);
|
||||
|
||||
const connections = Array.from(cachedByCdpUrl.values());
|
||||
cachedByCdpUrl.clear();
|
||||
connectingByCdpUrl.clear();
|
||||
for (const cur of connections) {
|
||||
if (cur.onDisconnected && typeof cur.browser.off === "function") {
|
||||
cur.browser.off("disconnected", cur.onDisconnected);
|
||||
}
|
||||
await cur.browser.close().catch(() => {});
|
||||
}
|
||||
await cur.browser.close().catch(() => {});
|
||||
}
|
||||
|
||||
function cdpSocketNeedsAttach(wsUrl: string): boolean {
|
||||
@@ -655,31 +669,29 @@ export async function forceDisconnectPlaywrightForTarget(opts: {
|
||||
reason?: string;
|
||||
}): Promise<void> {
|
||||
const normalized = normalizeCdpUrl(opts.cdpUrl);
|
||||
if (cached?.cdpUrl !== normalized) {
|
||||
const cur = cachedByCdpUrl.get(normalized);
|
||||
if (!cur) {
|
||||
return;
|
||||
}
|
||||
const cur = cached;
|
||||
cached = null;
|
||||
// Also clear `connecting` so the next call does a fresh connectOverCDP
|
||||
cachedByCdpUrl.delete(normalized);
|
||||
// Also clear the per-url in-flight connect so the next call does a fresh connectOverCDP
|
||||
// rather than awaiting a stale promise.
|
||||
connecting = null;
|
||||
if (cur) {
|
||||
// Remove the "disconnected" listener to prevent the old browser's teardown
|
||||
// from racing with a fresh connection and nulling the new `cached`.
|
||||
if (cur.onDisconnected && typeof cur.browser.off === "function") {
|
||||
cur.browser.off("disconnected", cur.onDisconnected);
|
||||
}
|
||||
|
||||
// Best-effort: kill any stuck JS to unblock the target's execution context before we
|
||||
// disconnect Playwright's CDP connection.
|
||||
const targetId = opts.targetId?.trim() || "";
|
||||
if (targetId) {
|
||||
await tryTerminateExecutionViaCdp({ cdpUrl: normalized, targetId }).catch(() => {});
|
||||
}
|
||||
|
||||
// Fire-and-forget: don't await because browser.close() may hang on the stuck CDP pipe.
|
||||
cur.browser.close().catch(() => {});
|
||||
connectingByCdpUrl.delete(normalized);
|
||||
// Remove the "disconnected" listener to prevent the old browser's teardown
|
||||
// from racing with a fresh connection and nulling the new cached entry.
|
||||
if (cur.onDisconnected && typeof cur.browser.off === "function") {
|
||||
cur.browser.off("disconnected", cur.onDisconnected);
|
||||
}
|
||||
|
||||
// Best-effort: kill any stuck JS to unblock the target's execution context before we
|
||||
// disconnect Playwright's CDP connection.
|
||||
const targetId = opts.targetId?.trim() || "";
|
||||
if (targetId) {
|
||||
await tryTerminateExecutionViaCdp({ cdpUrl: normalized, targetId }).catch(() => {});
|
||||
}
|
||||
|
||||
// Fire-and-forget: don't await because browser.close() may hang on the stuck CDP pipe.
|
||||
cur.browser.close().catch(() => {});
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -99,7 +99,7 @@ describe("browser server-context ensureTabAvailable", () => {
|
||||
expect(second.targetId).toBe("A");
|
||||
});
|
||||
|
||||
it("falls back to the only attached tab when an invalid targetId is provided (extension)", async () => {
|
||||
it("rejects invalid targetId even when only one extension tab remains", async () => {
|
||||
const responses = [
|
||||
[{ id: "A", type: "page", url: "https://a.example", webSocketDebuggerUrl: "ws://x/a" }],
|
||||
[{ id: "A", type: "page", url: "https://a.example", webSocketDebuggerUrl: "ws://x/a" }],
|
||||
@@ -109,8 +109,7 @@ describe("browser server-context ensureTabAvailable", () => {
|
||||
|
||||
const ctx = createBrowserRouteContext({ getState: () => state });
|
||||
const chrome = ctx.forProfile("chrome");
|
||||
const chosen = await chrome.ensureTabAvailable("NOT_A_TAB");
|
||||
expect(chosen.targetId).toBe("A");
|
||||
await expect(chrome.ensureTabAvailable("NOT_A_TAB")).rejects.toThrow(/tab not found/i);
|
||||
});
|
||||
|
||||
it("returns a descriptive message when no extension tabs are attached", async () => {
|
||||
|
||||
@@ -97,4 +97,46 @@ describe("browser server-context loopback direct WebSocket profiles", () => {
|
||||
expect.any(Object),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses an HTTPS /json base for secure direct WebSocket profiles with a /cdp suffix", async () => {
|
||||
const fetchMock = vi.fn(async (url: unknown) => {
|
||||
const u = String(url);
|
||||
if (u === "https://127.0.0.1:18800/json/list?token=abc") {
|
||||
return {
|
||||
ok: true,
|
||||
json: async () => [
|
||||
{
|
||||
id: "T2",
|
||||
title: "Secure Tab",
|
||||
url: "https://example.com",
|
||||
webSocketDebuggerUrl: "wss://127.0.0.1/devtools/page/T2",
|
||||
type: "page",
|
||||
},
|
||||
],
|
||||
} as unknown as Response;
|
||||
}
|
||||
if (u === "https://127.0.0.1:18800/json/activate/T2?token=abc") {
|
||||
return { ok: true, json: async () => ({}) } as unknown as Response;
|
||||
}
|
||||
if (u === "https://127.0.0.1:18800/json/close/T2?token=abc") {
|
||||
return { ok: true, json: async () => ({}) } as unknown as Response;
|
||||
}
|
||||
throw new Error(`unexpected fetch: ${u}`);
|
||||
});
|
||||
|
||||
global.fetch = withFetchPreconnect(fetchMock);
|
||||
const state = makeState("openclaw");
|
||||
state.resolved.profiles.openclaw = {
|
||||
cdpUrl: "wss://127.0.0.1:18800/cdp?token=abc",
|
||||
color: "#FF4500",
|
||||
};
|
||||
const ctx = createBrowserRouteContext({ getState: () => state });
|
||||
const openclaw = ctx.forProfile("openclaw");
|
||||
|
||||
const tabs = await openclaw.listTabs();
|
||||
expect(tabs.map((tab) => tab.targetId)).toEqual(["T2"]);
|
||||
|
||||
await openclaw.focusTab("T2");
|
||||
await openclaw.closeTab("T2");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -139,7 +139,7 @@ describe("browser server-context remote profile tab operations", () => {
|
||||
expect(second.targetId).toBe("A");
|
||||
});
|
||||
|
||||
it("falls back to the only tab for remote profiles when targetId is stale", async () => {
|
||||
it("rejects stale targetId for remote profiles even when only one tab remains", async () => {
|
||||
const responses = [
|
||||
[{ targetId: "T1", title: "Tab 1", url: "https://example.com", type: "page" }],
|
||||
[{ targetId: "T1", title: "Tab 1", url: "https://example.com", type: "page" }],
|
||||
@@ -151,8 +151,7 @@ describe("browser server-context remote profile tab operations", () => {
|
||||
} as unknown as Awaited<ReturnType<typeof pwAiModule.getPwAiModule>>);
|
||||
|
||||
const { remote } = createRemoteRouteHarness();
|
||||
const chosen = await remote.ensureTabAvailable("STALE_TARGET");
|
||||
expect(chosen.targetId).toBe("T1");
|
||||
await expect(remote.ensureTabAvailable("STALE_TARGET")).rejects.toThrow(/tab not found/i);
|
||||
});
|
||||
|
||||
it("keeps rejecting stale targetId for remote profiles when multiple tabs exist", async () => {
|
||||
|
||||
@@ -112,7 +112,9 @@ describe("createProfileResetOps", () => {
|
||||
});
|
||||
expect(isHttpReachable).toHaveBeenCalledWith(300);
|
||||
expect(stopRunningBrowser).toHaveBeenCalledTimes(1);
|
||||
expect(pwAiMocks.closePlaywrightBrowserConnection).toHaveBeenCalledTimes(1);
|
||||
expect(pwAiMocks.closePlaywrightBrowserConnection).toHaveBeenCalledWith({
|
||||
cdpUrl: "http://127.0.0.1:18800",
|
||||
});
|
||||
expect(trashMocks.movePathToTrash).toHaveBeenCalledWith(profileDir);
|
||||
});
|
||||
|
||||
@@ -132,5 +134,11 @@ describe("createProfileResetOps", () => {
|
||||
await ops.resetProfile();
|
||||
expect(stopRunningBrowser).not.toHaveBeenCalled();
|
||||
expect(pwAiMocks.closePlaywrightBrowserConnection).toHaveBeenCalledTimes(2);
|
||||
expect(pwAiMocks.closePlaywrightBrowserConnection).toHaveBeenNthCalledWith(1, {
|
||||
cdpUrl: "http://127.0.0.1:18800",
|
||||
});
|
||||
expect(pwAiMocks.closePlaywrightBrowserConnection).toHaveBeenNthCalledWith(2, {
|
||||
cdpUrl: "http://127.0.0.1:18800",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -16,10 +16,10 @@ type ResetOps = {
|
||||
resetProfile: () => Promise<{ moved: boolean; from: string; to?: string }>;
|
||||
};
|
||||
|
||||
async function closePlaywrightBrowserConnection(): Promise<void> {
|
||||
async function closePlaywrightBrowserConnectionForProfile(cdpUrl?: string): Promise<void> {
|
||||
try {
|
||||
const mod = await import("./pw-ai.js");
|
||||
await mod.closePlaywrightBrowserConnection();
|
||||
await mod.closePlaywrightBrowserConnection(cdpUrl ? { cdpUrl } : undefined);
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
@@ -48,14 +48,14 @@ export function createProfileResetOps({
|
||||
const httpReachable = await isHttpReachable(300);
|
||||
if (httpReachable && !profileState.running) {
|
||||
// Port in use but not by us - kill it.
|
||||
await closePlaywrightBrowserConnection();
|
||||
await closePlaywrightBrowserConnectionForProfile(profile.cdpUrl);
|
||||
}
|
||||
|
||||
if (profileState.running) {
|
||||
await stopRunningBrowser();
|
||||
}
|
||||
|
||||
await closePlaywrightBrowserConnection();
|
||||
await closePlaywrightBrowserConnectionForProfile(profile.cdpUrl);
|
||||
|
||||
if (!fs.existsSync(userDataDir)) {
|
||||
return { moved: false, from: userDataDir };
|
||||
|
||||
@@ -86,16 +86,7 @@ export function createProfileSelectionOps({
|
||||
return page ?? candidates.at(0) ?? null;
|
||||
};
|
||||
|
||||
let chosen = targetId ? resolveById(targetId) : pickDefault();
|
||||
if (
|
||||
!chosen &&
|
||||
(profile.driver === "extension" || !profile.cdpIsLoopback) &&
|
||||
candidates.length === 1
|
||||
) {
|
||||
// If an agent passes a stale/foreign targetId but only one candidate remains,
|
||||
// recover by using that tab instead of failing hard.
|
||||
chosen = candidates[0] ?? null;
|
||||
}
|
||||
const chosen = targetId ? resolveById(targetId) : pickDefault();
|
||||
|
||||
if (chosen === "AMBIGUOUS") {
|
||||
throw new Error("ambiguous target id prefix");
|
||||
|
||||
Reference in New Issue
Block a user