diff --git a/extensions/browser/src/browser/chrome-mcp.test.ts b/extensions/browser/src/browser/chrome-mcp.test.ts index fb8cda72515e..e4aa67d9b3b8 100644 --- a/extensions/browser/src/browser/chrome-mcp.test.ts +++ b/extensions/browser/src/browser/chrome-mcp.test.ts @@ -2,6 +2,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { ErrorCode, McpError } from "@modelcontextprotocol/sdk/types.js"; import { MAX_TIMER_TIMEOUT_MS } from "openclaw/plugin-sdk/number-runtime"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { @@ -27,10 +28,22 @@ type ToolCall = { }; type ToolCallMock = { mock: { - calls: Array<[ToolCall]>; + calls: Array<[ToolCall, unknown?, { signal?: AbortSignal; timeout?: number }?]>; }; }; +function createSdkTimeoutCallTool() { + return vi.fn( + async (_call: ToolCall, _resultSchema?: unknown, options?: { timeout?: number }) => + await new Promise((_resolve, reject) => { + setTimeout( + () => reject(new McpError(ErrorCode.RequestTimeout, "Request timed out")), + options?.timeout, + ); + }), + ); +} + type ChromeMcpSessionFactory = Exclude< Parameters[0], null @@ -1195,20 +1208,29 @@ describe("chrome MCP page parsing", () => { it("times out a stuck click and recovers on the next call", async () => { let factoryCalls = 0; + let forwardedTimeout: number | undefined; const factory: ChromeMcpSessionFactory = async () => { factoryCalls += 1; const session = createFakeSession(); - const callTool = vi.fn(async ({ name }: ToolCall) => { - if (name === "click") { - return await new Promise(() => {}); - } - if (name === "list_pages") { - return { - content: [{ type: "text", text: "## Pages\n1: https://example.com [selected]" }], - }; - } - throw new Error(`unexpected tool ${name}`); - }); + const callTool = vi.fn( + async ({ name }: ToolCall, _resultSchema?: unknown, options?: { timeout?: number }) => { + if (name === "click") { + forwardedTimeout = options?.timeout; + return await new Promise((_, reject) => { + setTimeout( + () => reject(new McpError(ErrorCode.RequestTimeout, "Request timed out")), + options?.timeout, + ); + }); + } + if (name === "list_pages") { + return { + content: [{ type: "text", text: "## Pages\n1: https://example.com [selected]" }], + }; + } + throw new Error(`unexpected tool ${name}`); + }, + ); session.client.callTool = callTool as typeof session.client.callTool; return session; }; @@ -1223,11 +1245,61 @@ describe("chrome MCP page parsing", () => { }), ).rejects.toThrow(/timed out/i); + expect(forwardedTimeout).toBe(25); const tabs = await listChromeMcpTabs("chrome-live"); expect(factoryCalls).toBe(2); expect(tabs).toHaveLength(1); }); + it("cancels a stuck evaluate through the SDK signal and reconnects", async () => { + let factoryCalls = 0; + let forwardedSignal: AbortSignal | undefined; + let notifyToolStarted: (() => void) | undefined; + const toolStarted = new Promise((resolve) => { + notifyToolStarted = resolve; + }); + const factory: ChromeMcpSessionFactory = async () => { + factoryCalls += 1; + const session = createFakeSession(); + if (factoryCalls === 1) { + session.client.callTool = vi.fn( + async (_call: ToolCall, _resultSchema?: unknown, options?: { signal?: AbortSignal }) => + await new Promise((_resolve, reject) => { + const signal = options?.signal; + forwardedSignal = signal; + notifyToolStarted?.(); + signal?.addEventListener( + "abort", + () => { + reject(signal.reason instanceof Error ? signal.reason : new Error("aborted")); + }, + { + once: true, + }, + ); + }), + ) as typeof session.client.callTool; + } + return session; + }; + setChromeMcpSessionFactoryForTest(factory); + const ctrl = new AbortController(); + const evaluatePromise = evaluateChromeMcpScript({ + profileName: "chrome-live", + targetId: "1", + fn: "() => window.location.href", + signal: ctrl.signal, + }); + + await toolStarted; + expect(forwardedSignal).toBe(ctrl.signal); + ctrl.abort(new Error("target browser crashed")); + + await expect(evaluatePromise).rejects.toThrow(/target browser crashed/i); + await expect(listChromeMcpTabs("chrome-live")).resolves.toHaveLength(2); + expect(factoryCalls).toBe(2); + }); + it("does not dispatch a click when the signal is already aborted", async () => { const session = createFakeSession(); const callTool = vi.fn(async (_call: ToolCall) => { @@ -1403,11 +1475,7 @@ describe("chrome MCP page parsing", () => { factoryCalls += 1; const session = createFakeSession(); if (factoryCalls === 1) { - // First session: all tool calls hang — simulates a Chrome MCP subprocess that is - // completely blocked (e.g., stuck waiting for a slow navigation to complete). - session.client.callTool = vi.fn( - async () => new Promise(() => {}), - ) as typeof session.client.callTool; + session.client.callTool = createSdkTimeoutCallTool() as typeof session.client.callTool; } return session; }; @@ -1437,12 +1505,10 @@ describe("chrome MCP page parsing", () => { expect(tabs).toHaveLength(2); }); - it("forwards an explicit timeoutMs to take_snapshot via the callTool race", async () => { + it("forwards an explicit timeoutMs to take_snapshot through the SDK", async () => { vi.useFakeTimers(); const session = createFakeSession(); - session.client.callTool = vi.fn( - async () => new Promise(() => {}), - ) as typeof session.client.callTool; + session.client.callTool = createSdkTimeoutCallTool() as typeof session.client.callTool; setChromeMcpSessionFactoryForTest(async () => session); const snapshotPromise = takeChromeMcpSnapshot({ diff --git a/extensions/browser/src/browser/chrome-mcp.ts b/extensions/browser/src/browser/chrome-mcp.ts index 06332795887d..6333250894f6 100644 --- a/extensions/browser/src/browser/chrome-mcp.ts +++ b/extensions/browser/src/browser/chrome-mcp.ts @@ -13,6 +13,7 @@ import { setTimeout as sleepTimeout } from "node:timers/promises"; import { promisify } from "node:util"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import { ErrorCode, McpError } from "@modelcontextprotocol/sdk/types.js"; import { addTimerTimeoutGraceMs, parseStrictPositiveInteger, @@ -54,12 +55,24 @@ type ChromeMcpSession = { ownsProcessTree?: boolean; }; -type ChromeMcpCallOptions = { - ephemeral?: boolean; +export type ChromeMcpOperationOptions = { timeoutMs?: number; signal?: AbortSignal; }; +type ChromeMcpTargetOperation = ChromeMcpOperationOptions & { + profileName: string; + profile?: ChromeMcpProfileOptions; + userDataDir?: string; + targetId: string; +}; + +type ChromeMcpCallOptions = ChromeMcpOperationOptions & { + ephemeral?: boolean; +}; + +const MCP_REQUEST_TIMEOUT_CODE: number = ErrorCode.RequestTimeout; + /** Browser profile options used to connect or launch chrome-devtools-mcp. */ export type ChromeMcpProfileOptions = { userDataDir?: string; @@ -1231,44 +1244,22 @@ async function callTool( for (let attempt = 0; attempt < 2; attempt += 1) { const lease = await leaseSession(profileName, normalizedProfileOptions, options); - const rawCall = lease.session.client.callTool({ - name, - arguments: args, - }) as Promise; - - let timeoutHandle: ReturnType | undefined; - let abortListener: (() => void) | undefined; - const racers: Array | Promise> = [rawCall]; - - if (timeoutMs !== undefined && timeoutMs > 0) { - racers.push( - new Promise((_, reject) => { - timeoutHandle = setTimeout(() => { - reject( - new Error( - `Chrome MCP "${name}" timed out after ${timeoutMs}ms. Session reset for reconnect.`, - ), - ); - }, timeoutMs); - }), - ); - } - - if (signal) { - racers.push( - new Promise((_, reject) => { - abortListener = () => - reject(toLintErrorObject(signal.reason ?? new Error("aborted"), "Non-Error rejection")); - signal.addEventListener("abort", abortListener, { once: true }); - }), - ); - } + // SDK-owned cancellation removes its request correlation entry. An outer race would return + // early while leaving the underlying MCP request pending after a target-browser crash. + const request = { name, arguments: args }; + const rawCall = ( + (timeoutMs !== undefined && timeoutMs > 0) || signal + ? lease.session.client.callTool(request, undefined, { + ...(timeoutMs !== undefined && timeoutMs > 0 ? { timeout: timeoutMs } : {}), + ...(signal ? { signal } : {}), + }) + : lease.session.client.callTool(request) + ) as Promise; let result: ChromeMcpToolResult; try { - result = racers.length === 1 ? await rawCall : await Promise.race(racers); + result = await rawCall; } catch (err) { - 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. if (!lease.temporary) { @@ -1278,14 +1269,17 @@ async function callTool( await closeChromeMcpSessionHandle(lease.session); } } + if (signal?.aborted) { + throw toLintErrorObject(signal.reason ?? err, "Non-Error abort reason"); + } + if (timeoutMs && err instanceof McpError && err.code === MCP_REQUEST_TIMEOUT_CODE) { + throw new Error( + `Chrome MCP "${name}" timed out after ${timeoutMs}ms. Session reset for reconnect.`, + { cause: err }, + ); + } throw err; } finally { - if (timeoutHandle !== undefined) { - clearTimeout(timeoutHandle); - } - if (signal && abortListener) { - signal.removeEventListener("abort", abortListener); - } if (lease.temporary) { await closeChromeMcpSessionHandle(lease.session); } @@ -1321,6 +1315,20 @@ async function callTool( throw new Error(`Chrome MCP tool "${name}" failed after reconnect.`); } +async function callTargetTool( + params: ChromeMcpTargetOperation, + name: string, + args: Record, +): Promise { + return await callTool( + params.profileName, + chromeMcpProfileOptionsFromParams(params), + name, + args, + params, + ); +} + async function withTempFile(fn: (filePath: string) => Promise): Promise { const dir = await fs.mkdtemp(path.join(resolvePreferredOpenClawTmpDir(), "openclaw-chrome-mcp-")); const filePath = path.join(dir, randomUUID()); @@ -1335,8 +1343,9 @@ async function findPageById( profileName: string, pageId: number, profileOptions?: string | ChromeMcpProfileOptions, + options: ChromeMcpOperationOptions = {}, ): Promise { - const pages = await listChromeMcpPages(profileName, profileOptions); + const pages = await listChromeMcpPages(profileName, profileOptions, options); const page = pages.find((entry) => entry.id === pageId); if (!page) { throw new BrowserTabNotFoundError(); @@ -1441,11 +1450,18 @@ export async function focusChromeMcpTab( profileName: string, targetId: string, profileOptions?: string | ChromeMcpProfileOptions, + options: ChromeMcpOperationOptions = {}, ): Promise { - await callTool(profileName, profileOptions, "select_page", { - pageId: parsePageId(targetId), - bringToFront: true, - }); + await callTool( + profileName, + profileOptions, + "select_page", + { + pageId: parsePageId(targetId), + bringToFront: true, + }, + options, + ); } /** Close a Chrome MCP page by target id. */ @@ -1453,8 +1469,15 @@ export async function closeChromeMcpTab( profileName: string, targetId: string, profileOptions?: string | ChromeMcpProfileOptions, + options: ChromeMcpOperationOptions = {}, ): Promise { - await callTool(profileName, profileOptions, "close_page", { pageId: parsePageId(targetId) }); + await callTool( + profileName, + profileOptions, + "close_page", + { pageId: parsePageId(targetId) }, + options, + ); } /** Navigate a Chrome MCP page and return its resolved URL. */ @@ -1465,6 +1488,7 @@ export async function navigateChromeMcpPage(params: { targetId: string; url: string; timeoutMs?: number; + signal?: AbortSignal; }): Promise<{ url: string }> { const resolvedTimeoutMs = params.timeoutMs ?? CHROME_MCP_NAVIGATE_TIMEOUT_MS; const callTimeoutMs = resolveChromeMcpNavigateCallTimeoutMs(resolvedTimeoutMs); @@ -1478,12 +1502,13 @@ export async function navigateChromeMcpPage(params: { url: params.url, timeout: resolvedTimeoutMs, }, - { timeoutMs: callTimeoutMs }, + { timeoutMs: callTimeoutMs, signal: params.signal }, ); const page = await findPageById( params.profileName, parsePageId(params.targetId), chromeMcpProfileOptionsFromParams(params), + { timeoutMs: callTimeoutMs, signal: params.signal }, ); return { url: page.url ?? params.url }; } @@ -1494,94 +1519,60 @@ export function resolveChromeMcpNavigateCallTimeoutMs(timeoutMs: number): number } /** Take a structured Chrome MCP snapshot for one page. */ -export async function takeChromeMcpSnapshot(params: { - profileName: string; - profile?: ChromeMcpProfileOptions; - userDataDir?: string; - targetId: string; - timeoutMs?: number; -}): Promise { - const result = await callTool( - params.profileName, - chromeMcpProfileOptionsFromParams(params), - "take_snapshot", - { - pageId: parsePageId(params.targetId), - }, - { timeoutMs: params.timeoutMs }, - ); +export async function takeChromeMcpSnapshot( + params: ChromeMcpTargetOperation, +): Promise { + const result = await callTargetTool(params, "take_snapshot", { + pageId: parsePageId(params.targetId), + }); return extractSnapshot(result); } /** Take a screenshot via Chrome MCP and return the image bytes. */ -export async function takeChromeMcpScreenshot(params: { - profileName: string; - profile?: ChromeMcpProfileOptions; - userDataDir?: string; - targetId: string; - uid?: string; - fullPage?: boolean; - format?: "png" | "jpeg"; - timeoutMs?: number; -}): Promise { +export async function takeChromeMcpScreenshot( + params: ChromeMcpTargetOperation & { + uid?: string; + fullPage?: boolean; + format?: "png" | "jpeg"; + }, +): Promise { return await withTempFile(async (filePath) => { const format = params.format ?? "png"; - await callTool( - params.profileName, - chromeMcpProfileOptionsFromParams(params), - "take_screenshot", - { - pageId: parsePageId(params.targetId), - filePath, - format, - ...(params.uid ? { uid: params.uid } : {}), - ...(params.fullPage ? { fullPage: true } : {}), - }, - { timeoutMs: params.timeoutMs }, - ); + await callTargetTool(params, "take_screenshot", { + pageId: parsePageId(params.targetId), + filePath, + format, + ...(params.uid ? { uid: params.uid } : {}), + ...(params.fullPage ? { fullPage: true } : {}), + }); return await fs.readFile(`${filePath}.${format}`); }); } /** Click a Chrome MCP snapshot element by uid. */ -export async function clickChromeMcpElement(params: { - profileName: string; - profile?: ChromeMcpProfileOptions; - userDataDir?: string; - targetId: string; - uid: string; - doubleClick?: boolean; - timeoutMs?: number; - signal?: AbortSignal; -}): Promise { - await callTool( - params.profileName, - chromeMcpProfileOptionsFromParams(params), - "click", - { - pageId: parsePageId(params.targetId), - uid: params.uid, - ...(params.doubleClick ? { dblClick: true } : {}), - }, - { - timeoutMs: params.timeoutMs, - signal: params.signal, - }, - ); +export async function clickChromeMcpElement( + params: ChromeMcpTargetOperation & { + uid: string; + doubleClick?: boolean; + }, +): Promise { + await callTargetTool(params, "click", { + pageId: parsePageId(params.targetId), + uid: params.uid, + ...(params.doubleClick ? { dblClick: true } : {}), + }); } /** Dispatch mouse events at page coordinates through an in-page script. */ -export async function clickChromeMcpCoords(params: { - profileName: string; - profile?: ChromeMcpProfileOptions; - userDataDir?: string; - targetId: string; - x: number; - y: number; - doubleClick?: boolean; - button?: "left" | "right" | "middle"; - delayMs?: number; -}): Promise { +export async function clickChromeMcpCoords( + params: ChromeMcpTargetOperation & { + x: number; + y: number; + doubleClick?: boolean; + button?: "left" | "right" | "middle"; + delayMs?: number; + }, +): Promise { const button = params.button ?? "left"; const buttonCode = button === "middle" ? 1 : button === "right" ? 2 : 0; const pressedButtons = button === "middle" ? 4 : button === "right" ? 2 : 1; @@ -1590,10 +1581,7 @@ export async function clickChromeMcpCoords(params: { const delayMs = JSON.stringify(resolveNonNegativeIntegerOption(params.delayMs, 0)); const doubleClick = params.doubleClick ? "true" : "false"; await evaluateChromeMcpScript({ - profileName: params.profileName, - profile: params.profile, - userDataDir: params.userDataDir, - targetId: params.targetId, + ...params, fn: `async () => { const x = ${x}; const y = ${y}; @@ -1633,15 +1621,10 @@ export async function clickChromeMcpCoords(params: { } /** Fill one Chrome MCP element by uid. */ -export async function fillChromeMcpElement(params: { - profileName: string; - profile?: ChromeMcpProfileOptions; - userDataDir?: string; - targetId: string; - uid: string; - value: string; -}): Promise { - await callTool(params.profileName, chromeMcpProfileOptionsFromParams(params), "fill", { +export async function fillChromeMcpElement( + params: ChromeMcpTargetOperation & { uid: string; value: string }, +): Promise { + await callTargetTool(params, "fill", { pageId: parsePageId(params.targetId), uid: params.uid, value: params.value, @@ -1649,43 +1632,32 @@ export async function fillChromeMcpElement(params: { } /** Fill multiple Chrome MCP form elements in one tool call. */ -export async function fillChromeMcpForm(params: { - profileName: string; - profile?: ChromeMcpProfileOptions; - userDataDir?: string; - targetId: string; - elements: Array<{ uid: string; value: string }>; -}): Promise { - await callTool(params.profileName, chromeMcpProfileOptionsFromParams(params), "fill_form", { +export async function fillChromeMcpForm( + params: ChromeMcpTargetOperation & { + elements: Array<{ uid: string; value: string }>; + }, +): Promise { + await callTargetTool(params, "fill_form", { pageId: parsePageId(params.targetId), elements: params.elements, }); } /** Hover a Chrome MCP snapshot element by uid. */ -export async function hoverChromeMcpElement(params: { - profileName: string; - profile?: ChromeMcpProfileOptions; - userDataDir?: string; - targetId: string; - uid: string; -}): Promise { - await callTool(params.profileName, chromeMcpProfileOptionsFromParams(params), "hover", { +export async function hoverChromeMcpElement( + params: ChromeMcpTargetOperation & { uid: string }, +): Promise { + await callTargetTool(params, "hover", { pageId: parsePageId(params.targetId), uid: params.uid, }); } /** Drag between two Chrome MCP snapshot element uids. */ -export async function dragChromeMcpElement(params: { - profileName: string; - profile?: ChromeMcpProfileOptions; - userDataDir?: string; - targetId: string; - fromUid: string; - toUid: string; -}): Promise { - await callTool(params.profileName, chromeMcpProfileOptionsFromParams(params), "drag", { +export async function dragChromeMcpElement( + params: ChromeMcpTargetOperation & { fromUid: string; toUid: string }, +): Promise { + await callTargetTool(params, "drag", { pageId: parsePageId(params.targetId), from_uid: params.fromUid, to_uid: params.toUid, @@ -1693,15 +1665,10 @@ export async function dragChromeMcpElement(params: { } /** Upload a local file into a Chrome MCP file input by uid. */ -export async function uploadChromeMcpFile(params: { - profileName: string; - profile?: ChromeMcpProfileOptions; - userDataDir?: string; - targetId: string; - uid: string; - filePath: string; -}): Promise { - await callTool(params.profileName, chromeMcpProfileOptionsFromParams(params), "upload_file", { +export async function uploadChromeMcpFile( + params: ChromeMcpTargetOperation & { uid: string; filePath: string }, +): Promise { + await callTargetTool(params, "upload_file", { pageId: parsePageId(params.targetId), uid: params.uid, filePath: params.filePath, @@ -1709,29 +1676,20 @@ export async function uploadChromeMcpFile(params: { } /** Press a keyboard key in a Chrome MCP page. */ -export async function pressChromeMcpKey(params: { - profileName: string; - profile?: ChromeMcpProfileOptions; - userDataDir?: string; - targetId: string; - key: string; -}): Promise { - await callTool(params.profileName, chromeMcpProfileOptionsFromParams(params), "press_key", { +export async function pressChromeMcpKey( + params: ChromeMcpTargetOperation & { key: string }, +): Promise { + await callTargetTool(params, "press_key", { pageId: parsePageId(params.targetId), key: params.key, }); } /** Resize a Chrome MCP page viewport. */ -export async function resizeChromeMcpPage(params: { - profileName: string; - profile?: ChromeMcpProfileOptions; - userDataDir?: string; - targetId: string; - width: number; - height: number; -}): Promise { - await callTool(params.profileName, chromeMcpProfileOptionsFromParams(params), "resize_page", { +export async function resizeChromeMcpPage( + params: ChromeMcpTargetOperation & { width: number; height: number }, +): Promise { + await callTargetTool(params, "resize_page", { pageId: parsePageId(params.targetId), width: params.width, height: params.height, @@ -1739,24 +1697,14 @@ export async function resizeChromeMcpPage(params: { } /** Evaluate a JavaScript function in a Chrome MCP page. */ -export async function evaluateChromeMcpScript(params: { - profileName: string; - profile?: ChromeMcpProfileOptions; - userDataDir?: string; - targetId: string; - fn: string; - args?: string[]; -}): Promise { - const result = await callTool( - params.profileName, - chromeMcpProfileOptionsFromParams(params), - "evaluate_script", - { - pageId: parsePageId(params.targetId), - function: params.fn, - ...(params.args?.length ? { args: params.args } : {}), - }, - ); +export async function evaluateChromeMcpScript( + params: ChromeMcpTargetOperation & { fn: string; args?: string[] }, +): Promise { + const result = await callTargetTool(params, "evaluate_script", { + pageId: parsePageId(params.targetId), + function: params.fn, + ...(params.args?.length ? { args: params.args } : {}), + }); return extractJsonMessage(result); } diff --git a/extensions/browser/src/browser/routes/agent.act.existing-session-navigation-guard.test.ts b/extensions/browser/src/browser/routes/agent.act.existing-session-navigation-guard.test.ts index cb6593de3eaa..6d531d9d0742 100644 --- a/extensions/browser/src/browser/routes/agent.act.existing-session-navigation-guard.test.ts +++ b/extensions/browser/src/browser/routes/agent.act.existing-session-navigation-guard.test.ts @@ -7,9 +7,10 @@ import { import { createBrowserRouteApp, createBrowserRouteResponse } from "./test-helpers.js"; const chromeMcpMocks = vi.hoisted(() => ({ + clickChromeMcpCoords: vi.fn(async () => {}), clickChromeMcpElement: vi.fn(async () => {}), dragChromeMcpElement: vi.fn(async () => {}), - evaluateChromeMcpScript: vi.fn(async () => "https://example.com"), + evaluateChromeMcpScript: vi.fn(async (_params: unknown) => "https://example.com"), fillChromeMcpElement: vi.fn(async () => {}), fillChromeMcpForm: vi.fn(async () => {}), hoverChromeMcpElement: vi.fn(async () => {}), @@ -25,6 +26,7 @@ const navigationGuardMocks = vi.hoisted(() => ({ })); vi.mock("../chrome-mcp.js", () => ({ + clickChromeMcpCoords: chromeMcpMocks.clickChromeMcpCoords, clickChromeMcpElement: chromeMcpMocks.clickChromeMcpElement, closeChromeMcpTab: vi.fn(async () => {}), dragChromeMcpElement: chromeMcpMocks.dragChromeMcpElement, @@ -52,6 +54,7 @@ function getActPostHandler( registerBrowserAgentActRoutes(app, { state: () => ({ resolved: { + actionTimeoutMs: 60_000, evaluateEnabled: true, ssrfPolicy: ssrfPolicy ?? undefined, }, @@ -147,6 +150,54 @@ describe("existing-session interaction navigation guard", () => { expectNavigationProbeUrls(Array.from({ length: 8 }, () => "https://example.com")); }); + it("threads one request budget through coordinate actions and navigation probes", async () => { + const handler = getActPostHandler(); + const response = createBrowserRouteResponse(); + const ctrl = new AbortController(); + const pending = handler?.( + { + params: {}, + query: {}, + body: { kind: "clickCoords", x: 20, y: 30 }, + signal: ctrl.signal, + }, + response.res, + ); + + await vi.runAllTimersAsync(); + await pending; + + const expectedOptions = { signal: ctrl.signal, timeoutMs: 60_000 }; + expect(chromeMcpMocks.clickChromeMcpCoords).toHaveBeenCalledWith( + expect.objectContaining(expectedOptions), + ); + for (const [params] of chromeMcpMocks.evaluateChromeMcpScript.mock.calls) { + expect(params).toEqual(expect.objectContaining(expectedOptions)); + } + expect(routeState.profileCtx.listTabs).toHaveBeenCalledWith(expectedOptions); + }); + + it("cancels a pending existing-session wait when its request aborts", async () => { + const handler = getActPostHandler(null); + const response = createBrowserRouteResponse(); + const ctrl = new AbortController(); + const pending = handler?.( + { + params: {}, + query: {}, + body: { kind: "wait", timeMs: 30_000 }, + signal: ctrl.signal, + }, + response.res, + ); + void pending?.catch(() => {}); + + ctrl.abort(new Error("request cancelled after browser crash")); + + await expect(pending).rejects.toThrow(/aborted|cancelled/i); + expect(chromeMcpMocks.evaluateChromeMcpScript).not.toHaveBeenCalled(); + }); + it("rechecks the page url after delayed navigation-triggering interactions", async () => { chromeMcpMocks.evaluateChromeMcpScript .mockResolvedValueOnce(42 as never) diff --git a/extensions/browser/src/browser/routes/agent.act.hooks.ts b/extensions/browser/src/browser/routes/agent.act.hooks.ts index 376b6bde3fcf..a4ecf23cf90b 100644 --- a/extensions/browser/src/browser/routes/agent.act.hooks.ts +++ b/extensions/browser/src/browser/routes/agent.act.hooks.ts @@ -80,6 +80,8 @@ export function registerBrowserAgentActHookRoutes( targetId: tab.targetId, uid, filePath: resolvedPaths[0] ?? "", + timeoutMs: timeoutMs ?? ctx.state().resolved.actionTimeoutMs, + signal: req.signal, }); return res.json({ ok: true }); } @@ -157,6 +159,8 @@ export function registerBrowserAgentActHookRoutes( profileName: profileCtx.profile.name, profile: profileCtx.profile, targetId: tab.targetId, + timeoutMs: ctx.state().resolved.actionTimeoutMs, + signal: req.signal, // Existing-session Chrome MCP has no dialog hook primitive. Patch // one-shot window dialog functions in-page, then restore them. fn: `() => { diff --git a/extensions/browser/src/browser/routes/agent.act.ts b/extensions/browser/src/browser/routes/agent.act.ts index b07d48a0831b..09f22428f9f3 100644 --- a/extensions/browser/src/browser/routes/agent.act.ts +++ b/extensions/browser/src/browser/routes/agent.act.ts @@ -4,7 +4,7 @@ * Dispatches normalized actions to either Playwright-backed OpenClaw browser * control or Chrome MCP existing-session operations with navigation guards. */ -import { sleep } from "openclaw/plugin-sdk/runtime-env"; +import { setTimeout as sleep } from "node:timers/promises"; import { formatErrorMessage } from "../../infra/errors.js"; import { clickChromeMcpElement, @@ -17,6 +17,7 @@ import { hoverChromeMcpElement, pressChromeMcpKey, resizeChromeMcpPage, + type ChromeMcpOperationOptions, type ChromeMcpProfileOptions, } from "../chrome-mcp.js"; import type { BrowserActRequest } from "../client-actions.types.js"; @@ -54,17 +55,16 @@ import { asyncBrowserRoute, jsonError, toStringOrEmpty } from "./utils.js"; const EXISTING_SESSION_INTERACTION_NAVIGATION_RECHECK_DELAYS_MS = [0, 250, 500] as const; -async function readExistingSessionLocationHref(params: { +type ExistingSessionOperation = ChromeMcpOperationOptions & { profileName: string; profile?: ChromeMcpProfileOptions; userDataDir?: string; targetId: string; -}): Promise { +}; + +async function readExistingSessionLocationHref(params: ExistingSessionOperation): Promise { const currentUrl = await evaluateChromeMcpScript({ - profileName: params.profileName, - profile: params.profile, - userDataDir: params.userDataDir, - targetId: params.targetId, + ...params, fn: "() => window.location.href", }); if (typeof currentUrl !== "string") { @@ -77,15 +77,13 @@ async function readExistingSessionLocationHref(params: { return normalizedUrl; } -async function assertExistingSessionPostInteractionNavigationAllowed(params: { - profileName: string; - profile?: ChromeMcpProfileOptions; - userDataDir?: string; - targetId: string; - ssrfPolicy?: BrowserNavigationPolicyOptions["ssrfPolicy"]; - listTabs: () => Promise>; - initialTabTargetIds: ReadonlySet; -}): Promise { +async function assertExistingSessionPostInteractionNavigationAllowed( + params: ExistingSessionOperation & { + ssrfPolicy?: BrowserNavigationPolicyOptions["ssrfPolicy"]; + listTabs: () => Promise>; + initialTabTargetIds: ReadonlySet; + }, +): Promise { const ssrfPolicyOpts = withBrowserNavigationPolicy(params.ssrfPolicy); if (!ssrfPolicyOpts.ssrfPolicy) { return; @@ -110,12 +108,13 @@ async function assertExistingSessionPostInteractionNavigationAllowed(params: { let sawStableAllowedUrl = false; for (const delayMs of EXISTING_SESSION_INTERACTION_NAVIGATION_RECHECK_DELAYS_MS) { if (delayMs > 0) { - await sleep(delayMs); + await sleep(delayMs, undefined, { signal: params.signal }); } let currentUrl: string; try { currentUrl = await readExistingSessionLocationHref(params); } catch { + params.signal?.throwIfAborted(); sawStableAllowedUrl = false; continue; } @@ -144,7 +143,7 @@ async function assertExistingSessionPostInteractionNavigationAllowed(params: { EXISTING_SESSION_INTERACTION_NAVIGATION_RECHECK_DELAYS_MS[ EXISTING_SESSION_INTERACTION_NAVIGATION_RECHECK_DELAYS_MS.length - 1 ]; - await sleep(lastDelay); + await sleep(lastDelay, undefined, { signal: params.signal }); try { const followUpUrl = await readExistingSessionLocationHref(params); await assertBrowserNavigationResultAllowed({ @@ -156,6 +155,7 @@ async function assertExistingSessionPostInteractionNavigationAllowed(params: { return; } } catch { + params.signal?.throwIfAborted(); // Probe failed — fall through to throw } } @@ -217,22 +217,19 @@ function buildExistingSessionWaitPredicate(params: { return checks.length === 1 ? checks[0] : checks.map((check) => `(${check})`).join(" && "); } -async function waitForExistingSessionCondition(params: { - profileName: string; - profile?: ChromeMcpProfileOptions; - userDataDir?: string; - targetId: string; - timeMs?: number; - text?: string; - textGone?: string; - selector?: string; - url?: string; - loadState?: "load" | "domcontentloaded" | "networkidle"; - fn?: string; - timeoutMs?: number; -}): Promise { +async function waitForExistingSessionCondition( + params: ExistingSessionOperation & { + timeMs?: number; + text?: string; + textGone?: string; + selector?: string; + url?: string; + loadState?: "load" | "domcontentloaded" | "networkidle"; + fn?: string; + }, +): Promise { if (params.timeMs && params.timeMs > 0) { - await sleep(params.timeMs); + await sleep(params.timeMs, undefined, { signal: params.signal }); } const predicate = buildExistingSessionWaitPredicate(params); if (!predicate && !params.url) { @@ -245,20 +242,14 @@ async function waitForExistingSessionCondition(params: { if (predicate) { ready = Boolean( await evaluateChromeMcpScript({ - profileName: params.profileName, - profile: params.profile, - userDataDir: params.userDataDir, - targetId: params.targetId, + ...params, fn: `async () => ${predicate}`, }), ); } if (ready && params.url) { const currentUrl = await evaluateChromeMcpScript({ - profileName: params.profileName, - profile: params.profile, - userDataDir: params.userDataDir, - targetId: params.targetId, + ...params, fn: "() => window.location.href", }); ready = typeof currentUrl === "string" && matchBrowserUrlPattern(params.url, currentUrl); @@ -266,7 +257,7 @@ async function waitForExistingSessionCondition(params: { if (ready) { return; } - await sleep(250); + await sleep(250, undefined, { signal: params.signal }); } throw new Error("Timed out waiting for condition"); } @@ -404,6 +395,14 @@ export function registerBrowserAgentActRoutes( const evaluateEnabled = ctx.state().resolved.evaluateEnabled; const ssrfPolicy = ctx.state().resolved.ssrfPolicy; const isExistingSession = getBrowserProfileCapabilities(profileCtx.profile).usesChromeMcp; + const requestedTimeoutMs = + "timeoutMs" in action && typeof action.timeoutMs === "number" + ? action.timeoutMs + : undefined; + const existingSessionCallOptions: ChromeMcpOperationOptions = { + timeoutMs: requestedTimeoutMs ?? ctx.state().resolved.actionTimeoutMs, + signal: req.signal, + }; const hasNavigationResultPolicy = Boolean( withBrowserNavigationPolicy(ssrfPolicy).ssrfPolicy, ); @@ -417,7 +416,7 @@ export function registerBrowserAgentActRoutes( ? await resolveTargetIdAfterNavigate({ oldTargetId: tab.targetId, navigatedUrl: tab.url, - listTabs: () => profileCtx.listTabs(), + listTabs: () => profileCtx.listTabs(existingSessionCallOptions), }) : tab.targetId; const url = @@ -428,6 +427,7 @@ export function registerBrowserAgentActRoutes( profileCtx, targetId: responseTargetId, fallbackUrl: tab.url, + ...(isExistingSession ? existingSessionCallOptions : {}), }); return res.json({ ok: true, @@ -449,15 +449,23 @@ export function registerBrowserAgentActRoutes( } const profileName = profileCtx.profile.name; if (isExistingSession) { - const initialTabTargetIds = hasNavigationResultPolicy - ? new Set((await profileCtx.listTabs()).map((currentTab) => currentTab.targetId)) - : new Set(); - const existingSessionNavigationGuard = { + const existingSessionTarget: ExistingSessionOperation = { profileName, profile: profileCtx.profile, targetId: tab.targetId, + ...existingSessionCallOptions, + }; + const initialTabTargetIds = hasNavigationResultPolicy + ? new Set( + (await profileCtx.listTabs(existingSessionCallOptions)).map( + (currentTab) => currentTab.targetId, + ), + ) + : new Set(); + const existingSessionNavigationGuard = { + ...existingSessionTarget, ssrfPolicy, - listTabs: () => profileCtx.listTabs(), + listTabs: () => profileCtx.listTabs(existingSessionCallOptions), initialTabTargetIds, }; const unsupportedMessage = getExistingSessionUnsupportedMessage(action); @@ -474,13 +482,9 @@ export function registerBrowserAgentActRoutes( await runExistingSessionActionWithNavigationGuard({ execute: () => clickChromeMcpElement({ - profileName, - profile: profileCtx.profile, - targetId: tab.targetId, + ...existingSessionTarget, uid: action.ref!, doubleClick: action.doubleClick ?? false, - timeoutMs: action.timeoutMs, - signal: req.signal, }), guard: existingSessionNavigationGuard, }); @@ -489,9 +493,7 @@ export function registerBrowserAgentActRoutes( await runExistingSessionActionWithNavigationGuard({ execute: () => clickChromeMcpCoords({ - profileName, - profile: profileCtx.profile, - targetId: tab.targetId, + ...existingSessionTarget, x: action.x, y: action.y, doubleClick: action.doubleClick ?? false, @@ -505,17 +507,13 @@ export function registerBrowserAgentActRoutes( await runExistingSessionActionWithNavigationGuard({ execute: async () => { await fillChromeMcpElement({ - profileName, - profile: profileCtx.profile, - targetId: tab.targetId, + ...existingSessionTarget, uid: action.ref!, value: action.text, }); if (action.submit) { await pressChromeMcpKey({ - profileName, - profile: profileCtx.profile, - targetId: tab.targetId, + ...existingSessionTarget, key: "Enter", }); } @@ -527,9 +525,7 @@ export function registerBrowserAgentActRoutes( await runExistingSessionActionWithNavigationGuard({ execute: () => pressChromeMcpKey({ - profileName, - profile: profileCtx.profile, - targetId: tab.targetId, + ...existingSessionTarget, key: action.key, }), guard: existingSessionNavigationGuard, @@ -539,9 +535,7 @@ export function registerBrowserAgentActRoutes( await runExistingSessionActionWithNavigationGuard({ execute: () => hoverChromeMcpElement({ - profileName, - profile: profileCtx.profile, - targetId: tab.targetId, + ...existingSessionTarget, uid: action.ref!, }), guard: existingSessionNavigationGuard, @@ -551,9 +545,7 @@ export function registerBrowserAgentActRoutes( await runExistingSessionActionWithNavigationGuard({ execute: () => evaluateChromeMcpScript({ - profileName, - profile: profileCtx.profile, - targetId: tab.targetId, + ...existingSessionTarget, fn: `(el) => { el.scrollIntoView({ block: "center", inline: "center" }); return true; }`, args: [action.ref!], }), @@ -564,9 +556,7 @@ export function registerBrowserAgentActRoutes( await runExistingSessionActionWithNavigationGuard({ execute: () => dragChromeMcpElement({ - profileName, - profile: profileCtx.profile, - targetId: tab.targetId, + ...existingSessionTarget, fromUid: action.startRef!, toUid: action.endRef!, }), @@ -577,9 +567,7 @@ export function registerBrowserAgentActRoutes( await runExistingSessionActionWithNavigationGuard({ execute: () => fillChromeMcpElement({ - profileName, - profile: profileCtx.profile, - targetId: tab.targetId, + ...existingSessionTarget, uid: action.ref!, value: action.values[0] ?? "", }), @@ -590,9 +578,7 @@ export function registerBrowserAgentActRoutes( await runExistingSessionActionWithNavigationGuard({ execute: () => fillChromeMcpForm({ - profileName, - profile: profileCtx.profile, - targetId: tab.targetId, + ...existingSessionTarget, elements: action.fields.map((field) => ({ uid: field.ref, value: String(field.value ?? ""), @@ -603,18 +589,14 @@ export function registerBrowserAgentActRoutes( return await jsonOk(); case "resize": await resizeChromeMcpPage({ - profileName, - profile: profileCtx.profile, - targetId: tab.targetId, + ...existingSessionTarget, width: action.width, height: action.height, }); return await jsonOk(); case "wait": await waitForExistingSessionCondition({ - profileName, - profile: profileCtx.profile, - targetId: tab.targetId, + ...existingSessionTarget, timeMs: action.timeMs, text: action.text, textGone: action.textGone, @@ -622,16 +604,13 @@ export function registerBrowserAgentActRoutes( url: action.url, loadState: action.loadState, fn: action.fn, - timeoutMs: action.timeoutMs, }); return await jsonOk(); case "evaluate": { const result = await runExistingSessionActionWithNavigationGuard({ execute: () => evaluateChromeMcpScript({ - profileName, - profile: profileCtx.profile, - targetId: tab.targetId, + ...existingSessionTarget, fn: normalizeBrowserEvaluateFunctionSource( action.fn, action.ref ? { argumentName: "el" } : undefined, @@ -643,7 +622,12 @@ export function registerBrowserAgentActRoutes( return await jsonOk({ result }); } case "close": - await closeChromeMcpTab(profileName, tab.targetId, profileCtx.profile); + await closeChromeMcpTab( + profileName, + tab.targetId, + profileCtx.profile, + existingSessionCallOptions, + ); return await jsonOk(); case "batch": return jsonActError( @@ -787,6 +771,8 @@ export function registerBrowserAgentActRoutes( profile: profileCtx.profile, targetId: tab.targetId, args: [ref], + timeoutMs: ctx.state().resolved.actionTimeoutMs, + signal: req.signal, fn: `(el) => { if (!(el instanceof Element)) { return false; diff --git a/extensions/browser/src/browser/routes/agent.existing-session.test.ts b/extensions/browser/src/browser/routes/agent.existing-session.test.ts index bcfd4d1dfe13..1b87e95aae62 100644 --- a/extensions/browser/src/browser/routes/agent.existing-session.test.ts +++ b/extensions/browser/src/browser/routes/agent.existing-session.test.ts @@ -163,7 +163,11 @@ describe("existing-session browser routes", () => { it("allows labeled AI snapshots for existing-session profiles", async () => { const handler = getSnapshotGetHandler(); const response = createBrowserRouteResponse(); - await handler?.({ params: {}, query: { format: "ai", labels: "1" } }, response.res); + const ctrl = new AbortController(); + await handler?.( + { params: {}, query: { format: "ai", labels: "1" }, signal: ctrl.signal }, + response.res, + ); expect(response.statusCode).toBe(200); const body = requireRecord(response.body, "response body"); @@ -179,6 +183,16 @@ describe("existing-session browser routes", () => { expect(snapshotParams.profileName).toBe("chrome-live"); expectExistingSessionProfile(snapshotParams.profile); expect(snapshotParams.targetId).toBe("7"); + const renderParams = requireRecord( + callArg(chromeMcpMocks.evaluateChromeMcpScript, 0, 0, "label params"), + "label params", + ); + const cleanupParams = requireRecord( + callArg(chromeMcpMocks.evaluateChromeMcpScript, 1, 0, "label cleanup params"), + "label cleanup params", + ); + expect(renderParams.signal).toBe(ctrl.signal); + expect(cleanupParams.signal).toBeUndefined(); expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).not.toHaveBeenCalled(); expect(chromeMcpMocks.takeChromeMcpScreenshot).toHaveBeenCalled(); }); diff --git a/extensions/browser/src/browser/routes/agent.shared.test.ts b/extensions/browser/src/browser/routes/agent.shared.test.ts index cb7f98933418..cc1a20bdf155 100644 --- a/extensions/browser/src/browser/routes/agent.shared.test.ts +++ b/extensions/browser/src/browser/routes/agent.shared.test.ts @@ -62,6 +62,7 @@ function routeContextForTab( forProfile: () => profileCtx, state: () => ({ resolved: { + actionTimeoutMs: 60_000, ssrfPolicy: {}, }, }), @@ -153,6 +154,8 @@ describe("browser route shared helpers", () => { expect(ensureTabAvailable).toHaveBeenCalledWith(undefined, { allowPlaywrightFallback: true, + signal: undefined, + timeoutMs: 60_000, }); }); diff --git a/extensions/browser/src/browser/routes/agent.shared.ts b/extensions/browser/src/browser/routes/agent.shared.ts index af109a618e43..9c70f1999f10 100644 --- a/extensions/browser/src/browser/routes/agent.shared.ts +++ b/extensions/browser/src/browser/routes/agent.shared.ts @@ -150,6 +150,8 @@ export async function withRouteTabContext( // Agent routes can address local-managed tabs through Playwright when per-tab WS discovery lags. const tab = await profileCtx.ensureTabAvailable(params.targetId, { allowPlaywrightFallback: true, + signal: params.req.signal, + timeoutMs: params.ctx.state().resolved.actionTimeoutMs, }); if (params.enforceCurrentUrlAllowed) { await assertBrowserNavigationResultAllowed({ @@ -167,6 +169,8 @@ export async function withRouteTabContext( profileCtx, targetId: tab.targetId, fallbackUrl, + signal: params.req.signal, + timeoutMs: params.ctx.state().resolved.actionTimeoutMs, }), }); } catch (err) { @@ -184,8 +188,16 @@ export async function resolveSafeRouteTabUrl(params: { profileCtx: ProfileContext; targetId: string; fallbackUrl?: string; + signal?: AbortSignal; + timeoutMs?: number; }): Promise { - const tabs = await params.profileCtx.listTabs().catch(() => []); + let tabs: Array<{ targetId: string; url: string }>; + try { + tabs = await params.profileCtx.listTabs({ signal: params.signal, timeoutMs: params.timeoutMs }); + } catch { + params.signal?.throwIfAborted(); + tabs = []; + } const candidateUrl = tabs.find((tab) => tab.targetId === params.targetId)?.url ?? params.fallbackUrl; if (!candidateUrl) { diff --git a/extensions/browser/src/browser/routes/agent.snapshot.local-managed.test.ts b/extensions/browser/src/browser/routes/agent.snapshot.local-managed.test.ts index 9bd068972d03..3f7445bd965d 100644 --- a/extensions/browser/src/browser/routes/agent.snapshot.local-managed.test.ts +++ b/extensions/browser/src/browser/routes/agent.snapshot.local-managed.test.ts @@ -101,6 +101,7 @@ function getSnapshotGetHandler() { registerBrowserAgentSnapshotRoutes(app, { state: () => ({ resolved: { + actionTimeoutMs: 60_000, extraArgs: [], ssrfPolicy: { dangerouslyAllowPrivateNetwork: false }, }, @@ -130,6 +131,8 @@ describe("local-managed browser snapshot routes", () => { expect(response.body).toEqual({ error: "browser navigation blocked by policy" }); expect(routeState.profileCtx.ensureTabAvailable).toHaveBeenCalledWith(undefined, { allowPlaywrightFallback: false, + signal: undefined, + timeoutMs: undefined, }); expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).toHaveBeenCalledWith({ url: "http://127.0.0.1:8080/admin", diff --git a/extensions/browser/src/browser/routes/agent.snapshot.ts b/extensions/browser/src/browser/routes/agent.snapshot.ts index f25c2c0c4f9d..9cfd80f396e9 100644 --- a/extensions/browser/src/browser/routes/agent.snapshot.ts +++ b/extensions/browser/src/browser/routes/agent.snapshot.ts @@ -14,6 +14,7 @@ import { navigateChromeMcpPage, takeChromeMcpScreenshot, takeChromeMcpSnapshot, + type ChromeMcpOperationOptions, type ChromeMcpProfileOptions, } from "../chrome-mcp.js"; import { @@ -59,17 +60,18 @@ import { asyncBrowserRoute, jsonError, toBoolean, toStringOrEmpty } from "./util const CHROME_MCP_OVERLAY_ATTR = "data-openclaw-mcp-overlay"; -async function collectChromeMcpSnapshotUrls(params: { +type ChromeMcpSnapshotOperation = ChromeMcpOperationOptions & { profileName: string; profile?: ChromeMcpProfileOptions; userDataDir?: string; targetId: string; -}): Promise { +}; + +async function collectChromeMcpSnapshotUrls( + params: ChromeMcpSnapshotOperation, +): Promise { const result = await evaluateChromeMcpScript({ - profileName: params.profileName, - profile: params.profile, - userDataDir: params.userDataDir, - targetId: params.targetId, + ...params, fn: `() => { const seen = new Set(); const out = []; @@ -103,17 +105,11 @@ async function collectChromeMcpSnapshotUrls(params: { : []; } -async function clearChromeMcpOverlay(params: { - profileName: string; - profile?: ChromeMcpProfileOptions; - userDataDir?: string; - targetId: string; -}): Promise { +async function clearChromeMcpOverlay(params: ChromeMcpSnapshotOperation): Promise { await evaluateChromeMcpScript({ - profileName: params.profileName, - profile: params.profile, - userDataDir: params.userDataDir, - targetId: params.targetId, + ...params, + // Cleanup must outlive a route abort or injected labels remain in the user's tab. + signal: undefined, fn: `() => { document.querySelectorAll("[${CHROME_MCP_OVERLAY_ATTR}]").forEach((node) => node.remove()); return true; @@ -121,19 +117,14 @@ async function clearChromeMcpOverlay(params: { }).catch(() => {}); } -async function renderChromeMcpLabels(params: { - profileName: string; - profile?: ChromeMcpProfileOptions; - userDataDir?: string; - targetId: string; - refs: string[]; -}): Promise<{ labels: number; skipped: number }> { +async function renderChromeMcpLabels( + params: ChromeMcpSnapshotOperation & { + refs: string[]; + }, +): Promise<{ labels: number; skipped: number }> { const refList = JSON.stringify(params.refs); const result = await evaluateChromeMcpScript({ - profileName: params.profileName, - profile: params.profile, - userDataDir: params.userDataDir, - targetId: params.targetId, + ...params, args: params.refs, fn: `(...elements) => { const refs = ${refList}; @@ -343,6 +334,7 @@ export function registerBrowserAgentSnapshotRoutes( profile: profileCtx.profile, targetId: tab.targetId, url, + signal: req.signal, }); await assertBrowserNavigationResultAllowed({ url: result.url, ...ssrfPolicyOpts }); return res.json({ ok: true, targetId: tab.targetId, ...result }); @@ -438,6 +430,13 @@ export function registerBrowserAgentSnapshotRoutes( enforceCurrentUrlAllowed: true, run: async ({ profileCtx, tab, cdpUrl }) => { if (getBrowserProfileCapabilities(profileCtx.profile).usesChromeMcp) { + const operation: ChromeMcpSnapshotOperation = { + profileName: profileCtx.profile.name, + profile: profileCtx.profile, + targetId: tab.targetId, + timeoutMs, + signal: req.signal, + }; const ssrfPolicyOpts = browserNavigationPolicyForProfile(ctx, profileCtx); if (ssrfPolicyOpts.ssrfPolicy) { await assertBrowserNavigationResultAllowed({ @@ -449,26 +448,17 @@ export function registerBrowserAgentSnapshotRoutes( return jsonError(res, 400, EXISTING_SESSION_LIMITS.snapshot.screenshotElement); } if (labels) { - const snapshot = await takeChromeMcpSnapshot({ - profileName: profileCtx.profile.name, - profile: profileCtx.profile, - targetId: tab.targetId, - }); + const snapshot = await takeChromeMcpSnapshot(operation); const built = buildAiSnapshotFromChromeMcpSnapshot({ root: snapshot }); const labelResult = await renderChromeMcpLabels({ - profileName: profileCtx.profile.name, - profile: profileCtx.profile, - targetId: tab.targetId, + ...operation, refs: Object.keys(built.refs), }); try { const buffer = await takeChromeMcpScreenshot({ - profileName: profileCtx.profile.name, - profile: profileCtx.profile, - targetId: tab.targetId, + ...operation, fullPage, format: type, - timeoutMs, }); await saveNormalizedScreenshotResponse({ res, @@ -481,22 +471,15 @@ export function registerBrowserAgentSnapshotRoutes( labelsSkipped: labelResult.skipped, }); } finally { - await clearChromeMcpOverlay({ - profileName: profileCtx.profile.name, - profile: profileCtx.profile, - targetId: tab.targetId, - }); + await clearChromeMcpOverlay(operation); } return; } const buffer = await takeChromeMcpScreenshot({ - profileName: profileCtx.profile.name, - profile: profileCtx.profile, - targetId: tab.targetId, + ...operation, uid: ref, fullPage, format: type, - timeoutMs, }); await saveNormalizedScreenshotResponse({ res, @@ -602,6 +585,8 @@ export function registerBrowserAgentSnapshotRoutes( try { const tab = await profileCtx.ensureTabAvailable(targetId || undefined, { allowPlaywrightFallback: hasPlaywright, + signal: req.signal, + timeoutMs: plan.timeoutMs, }); const usesChromeMcp = getBrowserProfileCapabilities(profileCtx.profile).usesChromeMcp; const ssrfPolicyOpts = browserNavigationPolicyForProfile(ctx, profileCtx); @@ -628,12 +613,14 @@ export function registerBrowserAgentSnapshotRoutes( .catch(() => undefined); } if (usesChromeMcp) { - const snapshot = await takeChromeMcpSnapshot({ + const operation: ChromeMcpSnapshotOperation = { profileName: profileCtx.profile.name, profile: profileCtx.profile, targetId: tab.targetId, timeoutMs: plan.timeoutMs, - }); + signal: req.signal, + }; + const snapshot = await takeChromeMcpSnapshot(operation); if (plan.format === "aria") { return res.json({ ok: true, @@ -657,29 +644,20 @@ export function registerBrowserAgentSnapshotRoutes( ...built, snapshot: appendSnapshotUrls( built.snapshot, - await collectChromeMcpSnapshotUrls({ - profileName: profileCtx.profile.name, - profile: profileCtx.profile, - targetId: tab.targetId, - }), + await collectChromeMcpSnapshotUrls(operation), ), } : built; if (plan.labels) { const refs = Object.keys(builtWithUrls.refs); const labelResult = await renderChromeMcpLabels({ - profileName: profileCtx.profile.name, - profile: profileCtx.profile, - targetId: tab.targetId, + ...operation, refs, }); try { const labeled = await takeChromeMcpScreenshot({ - profileName: profileCtx.profile.name, - profile: profileCtx.profile, - targetId: tab.targetId, + ...operation, format: "png", - timeoutMs: plan.timeoutMs, }); const normalized = await normalizeBrowserScreenshot(labeled, { maxSide: DEFAULT_BROWSER_SCREENSHOT_MAX_SIDE, @@ -705,11 +683,7 @@ export function registerBrowserAgentSnapshotRoutes( ...builtWithUrls, }); } finally { - await clearChromeMcpOverlay({ - profileName: profileCtx.profile.name, - profile: profileCtx.profile, - targetId: tab.targetId, - }); + await clearChromeMcpOverlay(operation); } } return res.json({ diff --git a/extensions/browser/src/browser/server-context.selection.test.ts b/extensions/browser/src/browser/server-context.selection.test.ts index 4b7e144ab7f9..05b06f15c230 100644 --- a/extensions/browser/src/browser/server-context.selection.test.ts +++ b/extensions/browser/src/browser/server-context.selection.test.ts @@ -89,6 +89,30 @@ describe("browser profile tab selection", () => { expect(listTabs).toHaveBeenCalledTimes(2); }); + it("passes the request budget to tab discovery", async () => { + const ready = tab("READY", "ws://127.0.0.1/devtools/page/READY"); + const { selection, listTabs } = createSelectionHarness({ snapshots: [[ready]] }); + const ctrl = new AbortController(); + + await expect( + selection.ensureTabAvailable(undefined, { signal: ctrl.signal, timeoutMs: 1234 }), + ).resolves.toEqual(ready); + + expect(listTabs).toHaveBeenCalledWith({ signal: ctrl.signal, timeoutMs: 1234 }); + }); + + it("does not start tab discovery for an aborted request", async () => { + const { selection, listTabs } = createSelectionHarness({ snapshots: [[tab("READY")]] }); + const ctrl = new AbortController(); + ctrl.abort(new Error("request cancelled")); + + await expect(selection.ensureTabAvailable(undefined, { signal: ctrl.signal })).rejects.toThrow( + /request cancelled/i, + ); + + expect(listTabs).not.toHaveBeenCalled(); + }); + it("preserves a target-id-only opened tab for a Playwright-backed caller", async () => { vi.useFakeTimers(); const openedTab = tab("OPENED"); diff --git a/extensions/browser/src/browser/server-context.selection.ts b/extensions/browser/src/browser/server-context.selection.ts index 2d49e0f80cbe..d4d39a662da0 100644 --- a/extensions/browser/src/browser/server-context.selection.ts +++ b/extensions/browser/src/browser/server-context.selection.ts @@ -18,6 +18,7 @@ import { } from "./server-context.constants.js"; import type { BrowserTab, + BrowserOperationOptions, EnsureTabAvailableOptions, ProfileRuntimeState, } from "./server-context.types.js"; @@ -28,7 +29,7 @@ type SelectionDeps = { getProfileState: () => ProfileRuntimeState; getCdpControlPolicy: () => SsrFPolicy | undefined; ensureBrowserAvailable: (opts?: { headless?: boolean }) => Promise; - listTabs: () => Promise; + listTabs: (options?: BrowserOperationOptions) => Promise; openTab: (url: string) => Promise; }; @@ -83,7 +84,9 @@ export function createProfileSelectionOps({ targetId?: string, options?: EnsureTabAvailableOptions, ): Promise => { + options?.signal?.throwIfAborted(); await ensureBrowserAvailable(); + options?.signal?.throwIfAborted(); const profileState = getProfileState(); let lastNonEmptyTabs: BrowserTab[] = []; let lastListError: unknown; @@ -92,13 +95,14 @@ export function createProfileSelectionOps({ const readTabs = async (): Promise => { try { - const tabs = await listTabs(); + const tabs = await listTabs(options); sawSuccessfulList = true; if (tabs.length > 0) { lastNonEmptyTabs = tabs; } return tabs; } catch (err) { + options?.signal?.throwIfAborted(); lastListError = err; return []; } diff --git a/extensions/browser/src/browser/server-context.tab-ops.ts b/extensions/browser/src/browser/server-context.tab-ops.ts index 853a52137126..60d4c99cde61 100644 --- a/extensions/browser/src/browser/server-context.tab-ops.ts +++ b/extensions/browser/src/browser/server-context.tab-ops.ts @@ -32,6 +32,7 @@ import { OPEN_TAB_DISCOVERY_WINDOW_MS, } from "./server-context.constants.js"; import type { + BrowserOperationOptions, BrowserServerState, BrowserTab, ProfileRuntimeState, @@ -45,7 +46,7 @@ type TabOpsDeps = { }; type ProfileTabOps = { - listTabs: () => Promise; + listTabs: (options?: BrowserOperationOptions) => Promise; openTab: (url: string, opts?: { label?: string }) => Promise; labelTab: (targetId: string, label: string) => Promise; }; @@ -200,10 +201,10 @@ export function createProfileTabOps({ }; }; - const readTabs = async (): Promise => { + const readTabs = async (options?: BrowserOperationOptions): Promise => { if (capabilities.usesChromeMcp) { const { listChromeMcpTabs } = await getChromeMcpModule(); - return await listChromeMcpTabs(profile.name, profile); + return await listChromeMcpTabs(profile.name, profile, options); } if (capabilities.usesPersistentPlaywright) { @@ -261,8 +262,8 @@ export function createProfileTabOps({ return tabs; }; - const listTabs = async (): Promise => { - const tabs = await readTabs(); + const listTabs = async (options?: BrowserOperationOptions): Promise => { + const tabs = await readTabs(options); return assignTabAliases(getProfileState(), tabs); }; diff --git a/extensions/browser/src/browser/server-context.types.ts b/extensions/browser/src/browser/server-context.types.ts index 2c8b01858184..b901695f2695 100644 --- a/extensions/browser/src/browser/server-context.types.ts +++ b/extensions/browser/src/browser/server-context.types.ts @@ -46,7 +46,12 @@ export type BrowserServerState = { stopUnhandledRejectionHandler?: () => void; }; -export type EnsureTabAvailableOptions = { +export type BrowserOperationOptions = { + signal?: AbortSignal; + timeoutMs?: number; +}; + +export type EnsureTabAvailableOptions = BrowserOperationOptions & { /** Allow a target-id-only tab when the caller can continue through Playwright. */ allowPlaywrightFallback?: boolean; }; @@ -63,7 +68,7 @@ type BrowserProfileActions = { timeoutMs?: number, options?: { ephemeral?: boolean; signal?: AbortSignal }, ) => Promise; - listTabs: () => Promise; + listTabs: (options?: BrowserOperationOptions) => Promise; openTab: (url: string, opts?: { label?: string }) => Promise; labelTab: (targetId: string, label: string) => Promise; focusTab: (targetId: string) => Promise;