From adc05f090adf25dbce4caab6376cdbe720e7f9c5 Mon Sep 17 00:00:00 2001 From: Dongseok Paeng Date: Thu, 9 Apr 2026 12:15:30 +0900 Subject: [PATCH] fix(browser): time out stuck chrome mcp clicks Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../browser/src/browser/chrome-mcp.test.ts | 36 ++++++++ extensions/browser/src/browser/chrome-mcp.ts | 85 ++++++++++++------- 2 files changed, 90 insertions(+), 31 deletions(-) diff --git a/extensions/browser/src/browser/chrome-mcp.test.ts b/extensions/browser/src/browser/chrome-mcp.test.ts index 8d1a9ea08aa..193a2084e87 100644 --- a/extensions/browser/src/browser/chrome-mcp.test.ts +++ b/extensions/browser/src/browser/chrome-mcp.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { + clickChromeMcpElement, buildChromeMcpArgs, evaluateChromeMcpScript, listChromeMcpTabs, @@ -305,6 +306,41 @@ describe("chrome MCP page parsing", () => { expect(tabs).toHaveLength(2); }); + it("times out a stuck click and recovers on the next call", async () => { + let factoryCalls = 0; + 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}`); + }); + session.client.callTool = callTool as typeof session.client.callTool; + return session; + }; + setChromeMcpSessionFactoryForTest(factory); + + await expect( + clickChromeMcpElement({ + profileName: "chrome-live", + targetId: "1", + uid: "btn-1", + timeoutMs: 25, + }), + ).rejects.toThrow(/timed out/i); + + const tabs = await listChromeMcpTabs("chrome-live"); + expect(factoryCalls).toBe(2); + expect(tabs).toHaveLength(1); + }); + it("creates a fresh session when userDataDir changes for the same profile", async () => { const createdSessions: ChromeMcpSession[] = []; const closeMocks: Array> = []; diff --git a/extensions/browser/src/browser/chrome-mcp.ts b/extensions/browser/src/browser/chrome-mcp.ts index 8429559fad9..edaef0fcd98 100644 --- a/extensions/browser/src/browser/chrome-mcp.ts +++ b/extensions/browser/src/browser/chrome-mcp.ts @@ -310,10 +310,15 @@ async function callTool( userDataDir: string | undefined, name: string, args: Record = {}, - timeoutMs?: number, + opts?: { timeoutMs?: number; signal?: AbortSignal }, ): Promise { const cacheKey = buildChromeMcpSessionCacheKey(profileName, userDataDir); const session = await getSession(profileName, userDataDir); + const timeoutMs = opts?.timeoutMs; + const signal = opts?.signal; + if (signal?.aborted) { + throw signal.reason ?? new Error("aborted"); + } const rawCall = session.client.callTool({ name, @@ -321,35 +326,39 @@ async function callTool( }) as Promise; let timeoutHandle: ReturnType | undefined; - const callPromise: Promise = - timeoutMs !== undefined && timeoutMs > 0 - ? Promise.race([ - rawCall, - new Promise((_, reject) => { - timeoutHandle = setTimeout(() => { - // Use transport-identity check so we never delete a freshly-created replacement session. - const cur = sessions.get(cacheKey); - if (cur?.transport === session.transport) { - sessions.delete(cacheKey); - } - void session.client.close().catch(() => {}); - reject( - new Error( - `Chrome MCP "${name}" timed out after ${timeoutMs}ms. Session reset for reconnect.`, - ), - ); - }, timeoutMs); - }), - ]) - : rawCall; + 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(signal.reason ?? new Error("aborted")); + signal.addEventListener("abort", abortListener, { once: true }); + }), + ); + } let result: ChromeMcpToolResult; try { - result = await callPromise; + result = racers.length === 1 ? await rawCall : await Promise.race(racers); } catch (err) { - // Transport/connection error or safety-net timeout — tear down session so it reconnects. + 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. - // Only close the client here if the timeout callback hasn't already done so. const cur = sessions.get(cacheKey); if (cur?.transport === session.transport) { sessions.delete(cacheKey); @@ -360,6 +369,9 @@ async function callTool( if (timeoutHandle !== undefined) { clearTimeout(timeoutHandle); } + if (signal && abortListener) { + signal.removeEventListener("abort", abortListener); + } } // Tool-level errors (element not found, script error, etc.) don't indicate a // broken connection — don't tear down the session for these. @@ -507,7 +519,7 @@ export async function navigateChromeMcpPage(params: { url: params.url, timeout: resolvedTimeoutMs, }, - resolvedTimeoutMs + 5_000, + { timeoutMs: resolvedTimeoutMs + 5_000 }, ); const page = await findPageById( params.profileName, @@ -554,12 +566,23 @@ export async function clickChromeMcpElement(params: { targetId: string; uid: string; doubleClick?: boolean; + timeoutMs?: number; + signal?: AbortSignal; }): Promise { - await callTool(params.profileName, params.userDataDir, "click", { - pageId: parsePageId(params.targetId), - uid: params.uid, - ...(params.doubleClick ? { dblClick: true } : {}), - }); + await callTool( + params.profileName, + params.userDataDir, + "click", + { + pageId: parsePageId(params.targetId), + uid: params.uid, + ...(params.doubleClick ? { dblClick: true } : {}), + }, + { + timeoutMs: params.timeoutMs, + signal: params.signal, + }, + ); } export async function fillChromeMcpElement(params: {