mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 07:40:44 +00:00
Fix sticky Chrome MCP status probes
# Conflicts: # extensions/browser/src/browser/chrome-mcp.ts # extensions/browser/src/browser/server-context.ts
This commit is contained in:
@@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
clickChromeMcpElement,
|
||||
buildChromeMcpArgs,
|
||||
ensureChromeMcpAvailable,
|
||||
evaluateChromeMcpScript,
|
||||
listChromeMcpTabs,
|
||||
navigateChromeMcpPage,
|
||||
@@ -186,6 +187,72 @@ describe("chrome MCP page parsing", () => {
|
||||
expect(result).toBe(123);
|
||||
});
|
||||
|
||||
it("does not cache an ephemeral availability probe before the next real attach", async () => {
|
||||
let factoryCalls = 0;
|
||||
const closeMocks: Array<ReturnType<typeof vi.fn>> = [];
|
||||
const factory: ChromeMcpSessionFactory = async () => {
|
||||
factoryCalls += 1;
|
||||
const session = createFakeSession();
|
||||
const closeMock = vi.fn().mockResolvedValue(undefined);
|
||||
session.client.close = closeMock as typeof session.client.close;
|
||||
closeMocks.push(closeMock);
|
||||
return session;
|
||||
};
|
||||
setChromeMcpSessionFactoryForTest(factory);
|
||||
|
||||
await ensureChromeMcpAvailable("chrome-live", undefined, { ephemeral: true });
|
||||
|
||||
expect(factoryCalls).toBe(1);
|
||||
expect(closeMocks[0]).toHaveBeenCalledTimes(1);
|
||||
|
||||
const tabs = await listChromeMcpTabs("chrome-live");
|
||||
|
||||
expect(factoryCalls).toBe(2);
|
||||
expect(closeMocks[1]).not.toHaveBeenCalled();
|
||||
expect(tabs).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("does not poison the next real attach after an ephemeral no-page probe", async () => {
|
||||
let factoryCalls = 0;
|
||||
const closeMocks: Array<ReturnType<typeof vi.fn>> = [];
|
||||
const factory: ChromeMcpSessionFactory = async () => {
|
||||
factoryCalls += 1;
|
||||
const session = createFakeSession();
|
||||
const closeMock = vi.fn().mockResolvedValue(undefined);
|
||||
session.client.close = closeMock as typeof session.client.close;
|
||||
closeMocks.push(closeMock);
|
||||
if (factoryCalls === 1) {
|
||||
const callTool = vi.fn(async ({ name }: ToolCall) => {
|
||||
if (name === "list_pages") {
|
||||
return {
|
||||
content: [{ type: "text", text: "No page selected" }],
|
||||
isError: true,
|
||||
};
|
||||
}
|
||||
throw new Error(`unexpected tool ${name}`);
|
||||
});
|
||||
session.client.callTool = callTool as typeof session.client.callTool;
|
||||
}
|
||||
return session;
|
||||
};
|
||||
setChromeMcpSessionFactoryForTest(factory);
|
||||
|
||||
await expect(
|
||||
listChromeMcpTabs("chrome-live", undefined, {
|
||||
ephemeral: true,
|
||||
}),
|
||||
).rejects.toThrow(/No page selected/);
|
||||
|
||||
expect(factoryCalls).toBe(1);
|
||||
expect(closeMocks[0]).toHaveBeenCalledTimes(1);
|
||||
|
||||
const tabs = await listChromeMcpTabs("chrome-live");
|
||||
|
||||
expect(factoryCalls).toBe(2);
|
||||
expect(closeMocks[1]).not.toHaveBeenCalled();
|
||||
expect(tabs).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("surfaces MCP tool errors instead of JSON parse noise", async () => {
|
||||
const factory: ChromeMcpSessionFactory = async () => {
|
||||
const session = createFakeSession();
|
||||
|
||||
@@ -31,6 +31,18 @@ type ChromeMcpSession = {
|
||||
ready: Promise<void>;
|
||||
};
|
||||
|
||||
type ChromeMcpCallOptions = {
|
||||
ephemeral?: boolean;
|
||||
timeoutMs?: number;
|
||||
signal?: AbortSignal;
|
||||
};
|
||||
|
||||
type ChromeMcpSessionLease = {
|
||||
session: ChromeMcpSession;
|
||||
cacheKey: string;
|
||||
temporary: boolean;
|
||||
};
|
||||
|
||||
type ChromeMcpSessionFactory = (
|
||||
profileName: string,
|
||||
userDataDir?: string,
|
||||
@@ -332,7 +344,42 @@ async function createRealSession(
|
||||
};
|
||||
}
|
||||
|
||||
async function getSession(profileName: string, userDataDir?: string): Promise<ChromeMcpSession> {
|
||||
async function waitForChromeMcpReady(
|
||||
session: ChromeMcpSession,
|
||||
profileName: string,
|
||||
timeoutMs?: number,
|
||||
): Promise<void> {
|
||||
if (!timeoutMs || timeoutMs <= 0) {
|
||||
await session.ready;
|
||||
return;
|
||||
}
|
||||
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
try {
|
||||
await Promise.race([
|
||||
session.ready,
|
||||
new Promise<never>((_, reject) => {
|
||||
timer = setTimeout(() => {
|
||||
reject(
|
||||
new BrowserProfileUnavailableError(
|
||||
`Chrome MCP existing-session attach for profile "${profileName}" timed out after ${timeoutMs}ms.`,
|
||||
),
|
||||
);
|
||||
}, timeoutMs);
|
||||
}),
|
||||
]);
|
||||
} finally {
|
||||
if (timer) {
|
||||
clearTimeout(timer);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async function getSession(
|
||||
profileName: string,
|
||||
userDataDir?: string,
|
||||
timeoutMs?: number,
|
||||
): Promise<ChromeMcpSession> {
|
||||
const cacheKey = buildChromeMcpSessionCacheKey(profileName, userDataDir);
|
||||
await closeChromeMcpSessionsForProfile(profileName, cacheKey);
|
||||
|
||||
@@ -364,7 +411,7 @@ async function getSession(profileName: string, userDataDir?: string): Promise<Ch
|
||||
}
|
||||
}
|
||||
try {
|
||||
await session.ready;
|
||||
await waitForChromeMcpReady(session, profileName, timeoutMs);
|
||||
return session;
|
||||
} catch (err) {
|
||||
const current = sessions.get(cacheKey);
|
||||
@@ -375,23 +422,110 @@ async function getSession(profileName: string, userDataDir?: string): Promise<Ch
|
||||
}
|
||||
}
|
||||
|
||||
async function getExistingSession(
|
||||
cacheKey: string,
|
||||
profileName: string,
|
||||
timeoutMs?: number,
|
||||
): Promise<ChromeMcpSession | null> {
|
||||
let session = sessions.get(cacheKey);
|
||||
if (session && session.transport.pid === null) {
|
||||
sessions.delete(cacheKey);
|
||||
session = undefined;
|
||||
}
|
||||
if (session) {
|
||||
try {
|
||||
await waitForChromeMcpReady(session, profileName, timeoutMs);
|
||||
return session;
|
||||
} catch (err) {
|
||||
const current = sessions.get(cacheKey);
|
||||
if (current?.transport === session.transport) {
|
||||
sessions.delete(cacheKey);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
const pending = pendingSessions.get(cacheKey);
|
||||
if (!pending) {
|
||||
return null;
|
||||
}
|
||||
|
||||
session = await pending;
|
||||
try {
|
||||
await waitForChromeMcpReady(session, profileName, timeoutMs);
|
||||
return session;
|
||||
} catch (err) {
|
||||
const current = sessions.get(cacheKey);
|
||||
if (current?.transport === session.transport) {
|
||||
sessions.delete(cacheKey);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
async function createEphemeralSession(
|
||||
profileName: string,
|
||||
userDataDir?: string,
|
||||
timeoutMs?: number,
|
||||
): Promise<ChromeMcpSession> {
|
||||
const session = await (sessionFactory ?? createRealSession)(profileName, userDataDir);
|
||||
try {
|
||||
await waitForChromeMcpReady(session, profileName, timeoutMs);
|
||||
return session;
|
||||
} catch (err) {
|
||||
await session.client.close().catch(() => {});
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
async function leaseSession(
|
||||
profileName: string,
|
||||
userDataDir?: string,
|
||||
options: ChromeMcpCallOptions = {},
|
||||
): Promise<ChromeMcpSessionLease> {
|
||||
const cacheKey = buildChromeMcpSessionCacheKey(profileName, userDataDir);
|
||||
if (!options.ephemeral) {
|
||||
return {
|
||||
session: await getSession(profileName, userDataDir, options.timeoutMs),
|
||||
cacheKey,
|
||||
temporary: false,
|
||||
};
|
||||
}
|
||||
|
||||
// Status probes should avoid seeding the shared attach session cache, but they can safely
|
||||
// reuse a real cached session if one already exists.
|
||||
const existingSession = await getExistingSession(cacheKey, profileName, options.timeoutMs);
|
||||
if (existingSession) {
|
||||
return {
|
||||
session: existingSession,
|
||||
cacheKey,
|
||||
temporary: false,
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
session: await createEphemeralSession(profileName, userDataDir, options.timeoutMs),
|
||||
cacheKey,
|
||||
temporary: true,
|
||||
};
|
||||
}
|
||||
|
||||
async function callTool(
|
||||
profileName: string,
|
||||
userDataDir: string | undefined,
|
||||
name: string,
|
||||
args: Record<string, unknown> = {},
|
||||
opts?: { timeoutMs?: number; signal?: AbortSignal },
|
||||
options: ChromeMcpCallOptions = {},
|
||||
): Promise<ChromeMcpToolResult> {
|
||||
const cacheKey = buildChromeMcpSessionCacheKey(profileName, userDataDir);
|
||||
const timeoutMs = opts?.timeoutMs;
|
||||
const signal = opts?.signal;
|
||||
const timeoutMs = options.timeoutMs;
|
||||
const signal = options.signal;
|
||||
if (signal?.aborted) {
|
||||
throw signal.reason ?? new Error("aborted");
|
||||
}
|
||||
|
||||
for (let attempt = 0; attempt < 2; attempt += 1) {
|
||||
const session = await getSession(profileName, userDataDir);
|
||||
const rawCall = session.client.callTool({
|
||||
const lease = await leaseSession(profileName, userDataDir, options);
|
||||
const rawCall = lease.session.client.callTool({
|
||||
name,
|
||||
arguments: args,
|
||||
}) as Promise<ChromeMcpToolResult>;
|
||||
@@ -430,10 +564,12 @@ async function callTool(
|
||||
void rawCall.catch(() => {});
|
||||
// Transport/connection error, timeout, or abort: tear down session so it reconnects.
|
||||
// Transport-identity check prevents clobbering a replacement session created concurrently.
|
||||
const cur = sessions.get(cacheKey);
|
||||
if (cur?.transport === session.transport) {
|
||||
sessions.delete(cacheKey);
|
||||
await session.client.close().catch(() => {});
|
||||
if (!lease.temporary) {
|
||||
const cur = sessions.get(lease.cacheKey);
|
||||
if (cur?.transport === lease.session.transport) {
|
||||
sessions.delete(lease.cacheKey);
|
||||
await lease.session.client.close().catch(() => {});
|
||||
}
|
||||
}
|
||||
throw err;
|
||||
} finally {
|
||||
@@ -443,6 +579,9 @@ async function callTool(
|
||||
if (signal && abortListener) {
|
||||
signal.removeEventListener("abort", abortListener);
|
||||
}
|
||||
if (lease.temporary) {
|
||||
await lease.session.client.close().catch(() => {});
|
||||
}
|
||||
}
|
||||
// Tool-level errors (element not found, script error, etc.) don't indicate a
|
||||
// broken connection. A stale selected-page error does poison the Chrome MCP
|
||||
@@ -450,10 +589,12 @@ async function callTool(
|
||||
if (result.isError) {
|
||||
const message = extractToolErrorMessage(result, name);
|
||||
if (shouldReconnectForToolError(name, message)) {
|
||||
const cur = sessions.get(cacheKey);
|
||||
if (cur?.transport === session.transport) {
|
||||
sessions.delete(cacheKey);
|
||||
await session.client.close().catch(() => {});
|
||||
if (!lease.temporary) {
|
||||
const cur = sessions.get(lease.cacheKey);
|
||||
if (cur?.transport === lease.session.transport) {
|
||||
sessions.delete(lease.cacheKey);
|
||||
await lease.session.client.close().catch(() => {});
|
||||
}
|
||||
}
|
||||
if (attempt === 0) {
|
||||
continue;
|
||||
@@ -492,8 +633,12 @@ async function findPageById(
|
||||
export async function ensureChromeMcpAvailable(
|
||||
profileName: string,
|
||||
userDataDir?: string,
|
||||
options: ChromeMcpCallOptions = {},
|
||||
): Promise<void> {
|
||||
await getSession(profileName, userDataDir);
|
||||
const lease = await leaseSession(profileName, userDataDir, options);
|
||||
if (lease.temporary) {
|
||||
await lease.session.client.close().catch(() => {});
|
||||
}
|
||||
}
|
||||
|
||||
export function getChromeMcpPid(profileName: string): number | null {
|
||||
@@ -519,16 +664,18 @@ export async function stopAllChromeMcpSessions(): Promise<void> {
|
||||
export async function listChromeMcpPages(
|
||||
profileName: string,
|
||||
userDataDir?: string,
|
||||
options: ChromeMcpCallOptions = {},
|
||||
): Promise<ChromeMcpStructuredPage[]> {
|
||||
const result = await callTool(profileName, userDataDir, "list_pages");
|
||||
const result = await callTool(profileName, userDataDir, "list_pages", {}, options);
|
||||
return extractStructuredPages(result);
|
||||
}
|
||||
|
||||
export async function listChromeMcpTabs(
|
||||
profileName: string,
|
||||
userDataDir?: string,
|
||||
options: ChromeMcpCallOptions = {},
|
||||
): Promise<BrowserTab[]> {
|
||||
return toBrowserTabs(await listChromeMcpPages(profileName, userDataDir));
|
||||
return toBrowserTabs(await listChromeMcpPages(profileName, userDataDir, options));
|
||||
}
|
||||
|
||||
export async function openChromeMcpTab(
|
||||
|
||||
@@ -90,7 +90,10 @@ export function createProfileAvailability({
|
||||
|
||||
const isTransportAvailable = async (timeoutMs?: number) => {
|
||||
if (capabilities.usesChromeMcp) {
|
||||
await ensureChromeMcpAvailable(profile.name, profile.userDataDir);
|
||||
await ensureChromeMcpAvailable(profile.name, profile.userDataDir, {
|
||||
ephemeral: true,
|
||||
timeoutMs,
|
||||
});
|
||||
return true;
|
||||
}
|
||||
return await isReachable(timeoutMs);
|
||||
|
||||
@@ -13,7 +13,7 @@ vi.mock("./chrome-mcp.js", () => ({
|
||||
openChromeMcpTab: vi.fn(async () => ({
|
||||
targetId: "8",
|
||||
title: "",
|
||||
url: "https://openclaw.ai",
|
||||
url: "about:blank",
|
||||
type: "page",
|
||||
})),
|
||||
closeChromeMcpTab: vi.fn(async () => {}),
|
||||
@@ -101,6 +101,53 @@ describe("browser server-context existing-session profile", () => {
|
||||
tabCount: 0,
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(chromeMcp.ensureChromeMcpAvailable).toHaveBeenCalledWith(
|
||||
"chrome-live",
|
||||
"/tmp/brave-profile",
|
||||
{ ephemeral: true },
|
||||
);
|
||||
expect(chromeMcp.listChromeMcpTabs).toHaveBeenCalledWith("chrome-live", "/tmp/brave-profile", {
|
||||
ephemeral: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps the next real attach on the normal sticky session path after an idle status probe", async () => {
|
||||
fs.mkdirSync("/tmp/brave-profile", { recursive: true });
|
||||
const state = makeState();
|
||||
const ctx = createBrowserRouteContext({ getState: () => state });
|
||||
const live = ctx.forProfile("chrome-live");
|
||||
|
||||
vi.mocked(chromeMcp.listChromeMcpTabs).mockRejectedValueOnce(new Error("No page selected"));
|
||||
|
||||
await expect(ctx.listProfiles()).resolves.toEqual([
|
||||
expect.objectContaining({
|
||||
name: "chrome-live",
|
||||
running: true,
|
||||
tabCount: 0,
|
||||
}),
|
||||
]);
|
||||
|
||||
vi.mocked(chromeMcp.listChromeMcpTabs).mockClear();
|
||||
|
||||
await live.ensureBrowserAvailable();
|
||||
const tabs = await live.listTabs();
|
||||
|
||||
expect(tabs.map((tab) => tab.targetId)).toEqual(["7"]);
|
||||
expect(chromeMcp.ensureChromeMcpAvailable).toHaveBeenLastCalledWith(
|
||||
"chrome-live",
|
||||
"/tmp/brave-profile",
|
||||
);
|
||||
expect(chromeMcp.listChromeMcpTabs).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
"chrome-live",
|
||||
"/tmp/brave-profile",
|
||||
);
|
||||
expect(chromeMcp.listChromeMcpTabs).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
"chrome-live",
|
||||
"/tmp/brave-profile",
|
||||
);
|
||||
});
|
||||
|
||||
it("routes tab operations through the Chrome MCP backend", async () => {
|
||||
@@ -118,22 +165,22 @@ describe("browser server-context existing-session profile", () => {
|
||||
])
|
||||
.mockResolvedValueOnce([
|
||||
{ targetId: "7", title: "", url: "https://example.com", type: "page" },
|
||||
{ targetId: "8", title: "", url: "https://openclaw.ai", type: "page" },
|
||||
{ targetId: "8", title: "", url: "about:blank", type: "page" },
|
||||
])
|
||||
.mockResolvedValueOnce([
|
||||
{ targetId: "7", title: "", url: "https://example.com", type: "page" },
|
||||
{ targetId: "8", title: "", url: "https://openclaw.ai", type: "page" },
|
||||
{ targetId: "8", title: "", url: "about:blank", type: "page" },
|
||||
])
|
||||
.mockResolvedValueOnce([
|
||||
{ targetId: "7", title: "", url: "https://example.com", type: "page" },
|
||||
{ targetId: "8", title: "", url: "https://openclaw.ai", type: "page" },
|
||||
{ targetId: "8", title: "", url: "about:blank", type: "page" },
|
||||
]);
|
||||
|
||||
await live.ensureBrowserAvailable();
|
||||
const tabs = await live.listTabs();
|
||||
expect(tabs.map((tab) => tab.targetId)).toEqual(["7"]);
|
||||
|
||||
const opened = await live.openTab("https://openclaw.ai");
|
||||
const opened = await live.openTab("about:blank");
|
||||
expect(opened.targetId).toBe("8");
|
||||
|
||||
const selected = await live.ensureTabAvailable();
|
||||
@@ -149,7 +196,7 @@ describe("browser server-context existing-session profile", () => {
|
||||
expect(chromeMcp.listChromeMcpTabs).toHaveBeenCalledWith("chrome-live", "/tmp/brave-profile");
|
||||
expect(chromeMcp.openChromeMcpTab).toHaveBeenCalledWith(
|
||||
"chrome-live",
|
||||
"https://openclaw.ai",
|
||||
"about:blank",
|
||||
"/tmp/brave-profile",
|
||||
);
|
||||
expect(chromeMcp.focusChromeMcpTab).toHaveBeenCalledWith(
|
||||
|
||||
@@ -3,6 +3,7 @@ import {
|
||||
resolveCdpReachabilityPolicy,
|
||||
} from "./cdp-reachability-policy.js";
|
||||
import { usesFastLoopbackCdpProbeClass } from "./cdp-timeouts.js";
|
||||
import { listChromeMcpTabs } from "./chrome-mcp.js";
|
||||
import { isChromeReachable, resolveOpenClawUserDataDir } from "./chrome.js";
|
||||
import type { ResolvedBrowserProfile } from "./config.js";
|
||||
import { resolveProfile } from "./config.js";
|
||||
@@ -181,7 +182,9 @@ export function createBrowserRouteContext(opts: ContextOptions): BrowserRouteCon
|
||||
try {
|
||||
running = await profileCtx.isTransportAvailable(300);
|
||||
if (running) {
|
||||
const tabs = await profileCtx.listTabs().catch(() => [] as BrowserTab[]);
|
||||
const tabs = await listChromeMcpTabs(profile.name, profile.userDataDir, {
|
||||
ephemeral: true,
|
||||
}).catch(() => [] as BrowserTab[]);
|
||||
tabCount = tabs.filter((t) => t.type === "page").length;
|
||||
}
|
||||
} catch {
|
||||
|
||||
Reference in New Issue
Block a user