From ee9522ef8d5cc50751130f70882faae262794579 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 11 May 2026 12:39:54 +0100 Subject: [PATCH] fix: keep browser status page probe within timeout --- CHANGELOG.md | 1 + .../browser/src/browser/chrome-mcp.test.ts | 28 ++++ extensions/browser/src/browser/chrome-mcp.ts | 131 +++++++++++++++--- .../routes/basic.existing-session.test.ts | 44 +++++- .../browser/src/browser/routes/basic.ts | 45 ++++-- .../browser/server-context.availability.ts | 15 +- .../src/browser/server-context.types.ts | 5 +- 7 files changed, 223 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1605bc31963..ac566c0fb76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Browser: report Chrome MCP existing-session page readiness in browser status without letting status probes exceed the client timeout. Fixes #80268. (#80280) Thanks @ai-hpc. - Memory: reject symlinked directory components in configured extra memory paths before reading Markdown files. (#80331) Thanks @samzong. - Sessions/transcripts: replace whole-file `readFile` scans with shared streaming helpers (`streamSessionTranscriptLines` and `streamSessionTranscriptLinesReverse`) for idempotency lookup, latest/tail assistant text reads, delivery-mirror dedupe, and compaction fork loading, so long-running sessions no longer materialize the full transcript in memory. Forward scans use `readline` over a bounded `createReadStream`; reverse scans read bounded chunks from the file end and decode complete JSONL lines newest-first without a fixed tail cap. Synthetic 200 MiB transcript: peak RSS delta drops from +252 MiB to +27 MiB while preserving malformed-line tolerance and idempotency-key return semantics. Fixes #54296. Thanks @jack-stormentswe. - WhatsApp: apply hot-reloaded `dmPolicy` and `allowFrom` settings to the active Web listener before processing new inbound DMs. Fixes #80538. Thanks @Ampaskopi129. diff --git a/extensions/browser/src/browser/chrome-mcp.test.ts b/extensions/browser/src/browser/chrome-mcp.test.ts index f400b847a8b..2601f5935a5 100644 --- a/extensions/browser/src/browser/chrome-mcp.test.ts +++ b/extensions/browser/src/browser/chrome-mcp.test.ts @@ -732,4 +732,32 @@ describe("chrome MCP page parsing", () => { await expectation; expect(closeMock).toHaveBeenCalledTimes(1); }); + + it("honors abort signals while waiting for ephemeral availability probes", async () => { + const closeMock = vi.fn().mockResolvedValue(undefined); + const factory: ChromeMcpSessionFactory = async () => + ({ + client: { + callTool: vi.fn(), + listTools: vi.fn(), + close: closeMock, + connect: vi.fn(), + }, + transport: { + pid: 123, + }, + ready: new Promise(() => {}), + }) as unknown as ChromeMcpSession; + setChromeMcpSessionFactoryForTest(factory); + + const ctrl = new AbortController(); + const promise = ensureChromeMcpAvailable("chrome-live", undefined, { + ephemeral: true, + signal: ctrl.signal, + }); + ctrl.abort(new Error("status budget exhausted")); + + await expect(promise).rejects.toThrow(/status budget exhausted/); + expect(closeMock).toHaveBeenCalledTimes(1); + }); }); diff --git a/extensions/browser/src/browser/chrome-mcp.ts b/extensions/browser/src/browser/chrome-mcp.ts index e0fda417c18..6e40a2d1f2a 100644 --- a/extensions/browser/src/browser/chrome-mcp.ts +++ b/extensions/browser/src/browser/chrome-mcp.ts @@ -460,30 +460,97 @@ async function waitForChromeMcpReady( session: ChromeMcpSession, profileName: string, timeoutMs?: number, + signal?: AbortSignal, ): Promise { - if (!timeoutMs || timeoutMs <= 0) { + if (signal?.aborted) { + throw signal.reason ?? new Error("aborted"); + } + if ((!timeoutMs || timeoutMs <= 0) && !signal) { await session.ready; return; } let timer: ReturnType | undefined; + let abortListener: (() => void) | undefined; try { - await Promise.race([ - session.ready, - new Promise((_, reject) => { - timer = setTimeout(() => { - reject( - new BrowserProfileUnavailableError( - `Chrome MCP existing-session attach for profile "${profileName}" timed out after ${timeoutMs}ms.`, - ), - ); - }, timeoutMs); - }), - ]); + const racers: Array | Promise> = [session.ready]; + if (timeoutMs && timeoutMs > 0) { + racers.push( + new Promise((_, reject) => { + timer = setTimeout(() => { + reject( + new BrowserProfileUnavailableError( + `Chrome MCP existing-session attach for profile "${profileName}" timed out after ${timeoutMs}ms.`, + ), + ); + }, timeoutMs); + }), + ); + } + if (signal) { + racers.push( + new Promise((_, reject) => { + abortListener = () => reject(signal.reason ?? new Error("aborted")); + signal.addEventListener("abort", abortListener, { once: true }); + }), + ); + } + await Promise.race(racers); } finally { if (timer) { clearTimeout(timer); } + if (signal && abortListener) { + signal.removeEventListener("abort", abortListener); + } + } +} + +async function waitForChromeMcpPendingSession( + pending: Promise, + signal?: AbortSignal, +): Promise { + if (signal?.aborted) { + throw signal.reason ?? new Error("aborted"); + } + if (!signal) { + return await pending; + } + + let abortListener: (() => void) | undefined; + try { + return await Promise.race([ + pending, + new Promise((_, reject) => { + abortListener = () => reject(signal.reason ?? new Error("aborted")); + signal.addEventListener("abort", abortListener, { once: true }); + }), + ]); + } finally { + if (abortListener) { + signal.removeEventListener("abort", abortListener); + } + } +} + +async function createChromeMcpSession( + profileName: string, + options: NormalizedChromeMcpProfileOptions, + signal?: AbortSignal, +): Promise { + const created = (sessionFactory ?? createRealSession)(profileName, options); + try { + const session = await waitForChromeMcpPendingSession(created, signal); + if (signal?.aborted) { + await session.client.close().catch(() => {}); + throw signal.reason ?? new Error("aborted"); + } + return session; + } catch (err) { + if (signal?.aborted) { + void created.then((session) => session.client.close()).catch(() => {}); + } + throw err; } } @@ -491,6 +558,7 @@ async function getSession( profileName: string, profileOptions?: ChromeMcpOptionsInput, timeoutMs?: number, + signal?: AbortSignal, ): Promise { const options = normalizeChromeMcpOptions(profileOptions); const cacheKey = buildChromeMcpSessionCacheKey(profileName, options); @@ -505,7 +573,7 @@ async function getSession( let pending = pendingSessions.get(cacheKey); if (!pending) { pending = (async () => { - const created = await (sessionFactory ?? createRealSession)(profileName, options); + const created = await createChromeMcpSession(profileName, options, signal); if (pendingSessions.get(cacheKey) === pending) { sessions.set(cacheKey, created); } else { @@ -524,7 +592,7 @@ async function getSession( } } try { - await waitForChromeMcpReady(session, profileName, timeoutMs); + await waitForChromeMcpReady(session, profileName, timeoutMs, signal); return session; } catch (err) { const current = sessions.get(cacheKey); @@ -539,6 +607,7 @@ async function getExistingSession( cacheKey: string, profileName: string, timeoutMs?: number, + signal?: AbortSignal, ): Promise { let session = sessions.get(cacheKey); if (session && session.transport.pid === null) { @@ -547,7 +616,7 @@ async function getExistingSession( } if (session) { try { - await waitForChromeMcpReady(session, profileName, timeoutMs); + await waitForChromeMcpReady(session, profileName, timeoutMs, signal); return session; } catch (err) { const current = sessions.get(cacheKey); @@ -563,9 +632,9 @@ async function getExistingSession( return null; } - session = await pending; + session = await waitForChromeMcpPendingSession(pending, signal); try { - await waitForChromeMcpReady(session, profileName, timeoutMs); + await waitForChromeMcpReady(session, profileName, timeoutMs, signal); return session; } catch (err) { const current = sessions.get(cacheKey); @@ -580,11 +649,12 @@ async function createEphemeralSession( profileName: string, profileOptions?: ChromeMcpOptionsInput, timeoutMs?: number, + signal?: AbortSignal, ): Promise { const options = normalizeChromeMcpOptions(profileOptions); - const session = await (sessionFactory ?? createRealSession)(profileName, options); + const session = await createChromeMcpSession(profileName, options, signal); try { - await waitForChromeMcpReady(session, profileName, timeoutMs); + await waitForChromeMcpReady(session, profileName, timeoutMs, signal); return session; } catch (err) { await session.client.close().catch(() => {}); @@ -601,7 +671,12 @@ async function leaseSession( const cacheKey = buildChromeMcpSessionCacheKey(profileName, normalizedProfileOptions); if (!options.ephemeral) { return { - session: await getSession(profileName, normalizedProfileOptions, options.timeoutMs), + session: await getSession( + profileName, + normalizedProfileOptions, + options.timeoutMs, + options.signal, + ), cacheKey, temporary: false, }; @@ -609,7 +684,12 @@ async function leaseSession( // 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); + const existingSession = await getExistingSession( + cacheKey, + profileName, + options.timeoutMs, + options.signal, + ); if (existingSession) { return { session: existingSession, @@ -619,7 +699,12 @@ async function leaseSession( } return { - session: await createEphemeralSession(profileName, normalizedProfileOptions, options.timeoutMs), + session: await createEphemeralSession( + profileName, + normalizedProfileOptions, + options.timeoutMs, + options.signal, + ), cacheKey, temporary: true, }; diff --git a/extensions/browser/src/browser/routes/basic.existing-session.test.ts b/extensions/browser/src/browser/routes/basic.existing-session.test.ts index ee51b6b3399..40b950f31af 100644 --- a/extensions/browser/src/browser/routes/basic.existing-session.test.ts +++ b/extensions/browser/src/browser/routes/basic.existing-session.test.ts @@ -11,7 +11,10 @@ const { registerBrowserBasicRoutes } = await import("./basic.js"); function createExistingSessionProfileState(params?: { isHttpReachable?: (timeoutMs?: number) => Promise; isTransportAvailable?: (timeoutMs?: number) => Promise; - isReachable?: (timeoutMs?: number, options?: { ephemeral?: boolean }) => Promise; + isReachable?: ( + timeoutMs?: number, + options?: { ephemeral?: boolean; signal?: AbortSignal }, + ) => Promise; }) { return { resolved: { @@ -326,7 +329,10 @@ describe("basic browser routes", () => { expect(response.statusCode).toBe(200); expect(isTransportAvailable).toHaveBeenCalledTimes(1); expect(isTransportAvailable).toHaveBeenCalledWith(5_000); - expect(isReachable).toHaveBeenCalledWith(5_000, { ephemeral: true }); + expect(isReachable).toHaveBeenCalledWith( + expect.any(Number), + expect.objectContaining({ ephemeral: true, signal: expect.any(AbortSignal) }), + ); expect(isHttpReachable).not.toHaveBeenCalled(); const body = responseBodyRecord(response); expect(body.cdpHttp).toBe(true); @@ -335,9 +341,37 @@ describe("basic browser routes", () => { expect(body.running).toBe(true); }); + it("keeps Chrome MCP page-readiness inside the status budget", async () => { + vi.useFakeTimers(); + vi.setSystemTime(1_000); + const isReachable = vi.fn(async () => true); + try { + const response = await callBasicRouteWithState({ + state: createExistingSessionProfileState({ + isTransportAvailable: async () => { + vi.setSystemTime(4_000); + return true; + }, + isReachable, + }), + }); + + expect(response.statusCode).toBe(200); + expect(isReachable).toHaveBeenCalledWith( + 4_000, + expect.objectContaining({ ephemeral: true, signal: expect.any(AbortSignal) }), + ); + } finally { + vi.useRealTimers(); + } + }); + it("page-readiness probe runs in ephemeral mode so status does not seed a cached session", async () => { const isReachable = vi.fn< - (timeoutMs?: number, options?: { ephemeral?: boolean }) => Promise + ( + timeoutMs?: number, + options?: { ephemeral?: boolean; signal?: AbortSignal }, + ) => Promise >(async () => true); await callBasicRouteWithState({ @@ -348,7 +382,9 @@ describe("basic browser routes", () => { }); expect(isReachable).toHaveBeenCalledTimes(1); - expect(isReachable.mock.calls[0]?.[1]).toEqual({ ephemeral: true }); + expect(isReachable.mock.calls[0]?.[1]).toEqual( + expect.objectContaining({ ephemeral: true, signal: expect.any(AbortSignal) }), + ); }); it("skips the page-reachability probe when transport is unavailable", async () => { diff --git a/extensions/browser/src/browser/routes/basic.ts b/extensions/browser/src/browser/routes/basic.ts index 727fb8a658f..72382b1a85d 100644 --- a/extensions/browser/src/browser/routes/basic.ts +++ b/extensions/browser/src/browser/routes/basic.ts @@ -19,8 +19,29 @@ import { const STATUS_CDP_HTTP_TIMEOUT_MS = 300; const STATUS_CDP_TRANSPORT_TIMEOUT_MS = 600; +const STATUS_CHROME_MCP_TOTAL_TIMEOUT_MS = 7_000; const STATUS_CHROME_MCP_TRANSPORT_TIMEOUT_MS = 5_000; -const STATUS_CHROME_MCP_PAGE_TIMEOUT_MS = 5_000; + +function remainingChromeMcpStatusTimeoutMs(startedAtMs: number): number { + return Math.max(1, STATUS_CHROME_MCP_TOTAL_TIMEOUT_MS - (Date.now() - startedAtMs)); +} + +async function probeChromeMcpPageReady(profileCtx: ProfileContext, timeoutMs: number) { + const abort = new AbortController(); + const timer = setTimeout(() => { + abort.abort(new Error(`Chrome MCP page-readiness probe timed out after ${timeoutMs}ms.`)); + }, timeoutMs); + try { + return await profileCtx.isReachable(timeoutMs, { + ephemeral: true, + signal: abort.signal, + }); + } catch { + return false; + } finally { + clearTimeout(timer); + } +} function handleBrowserRouteError(res: BrowserResponse, err: unknown) { const mapped = toBrowserErrorResponse(err); @@ -77,26 +98,20 @@ async function buildBrowserStatus(req: BrowserRequest, ctx: BrowserRouteContext) const capabilities = getBrowserProfileCapabilities(profileCtx.profile); const [cdpHttp, cdpReady, pageReady] = capabilities.usesChromeMcp ? await (async () => { + const statusStartedAtMs = Date.now(); const transportReady = await profileCtx.isTransportAvailable( STATUS_CHROME_MCP_TRANSPORT_TIMEOUT_MS, ); if (!transportReady) { return [false, false, false] as const; } - let pageReachable = false; - try { - // Status-safe page probe: ephemeral so a passive status call does not seed - // a persistent cached Chrome MCP session. Reuses an existing cached session - // if one is already live, otherwise opens a temporary session that is closed - // immediately after the round-trip. - pageReachable = await profileCtx.isReachable(STATUS_CHROME_MCP_PAGE_TIMEOUT_MS, { - ephemeral: true, - }); - } catch { - // Page-tool round-trip failed (timeout, MCP error, Chrome rejected - // the call). Transport stays green; pageReady reports the gap. - pageReachable = false; - } + // Status-safe page probe: ephemeral so a passive status call does not seed + // a persistent cached Chrome MCP session. Keep the whole status route inside + // the public client timeout; page probe failures degrade to pageReady=false. + const pageReachable = await probeChromeMcpPageReady( + profileCtx, + remainingChromeMcpStatusTimeoutMs(statusStartedAtMs), + ); return [transportReady, transportReady, pageReachable] as const; })() : await (async () => { diff --git a/extensions/browser/src/browser/server-context.availability.ts b/extensions/browser/src/browser/server-context.availability.ts index 816bb695a97..c7d13eb94b4 100644 --- a/extensions/browser/src/browser/server-context.availability.ts +++ b/extensions/browser/src/browser/server-context.availability.ts @@ -47,7 +47,10 @@ type AvailabilityDeps = { type AvailabilityOps = { isHttpReachable: (timeoutMs?: number) => Promise; isTransportAvailable: (timeoutMs?: number) => Promise; - isReachable: (timeoutMs?: number, options?: { ephemeral?: boolean }) => Promise; + isReachable: ( + timeoutMs?: number, + options?: { ephemeral?: boolean; signal?: AbortSignal }, + ) => Promise; ensureBrowserAvailable: (opts?: { headless?: boolean }) => Promise; stopRunningBrowser: () => Promise<{ stopped: boolean }>; }; @@ -150,19 +153,25 @@ export function createProfileAvailability({ const getCdpReachabilityPolicy = () => resolveCdpReachabilityPolicy(profile, state().resolved.ssrfPolicy); - const isReachable = async (timeoutMs?: number, options?: { ephemeral?: boolean }) => { + const isReachable = async ( + timeoutMs?: number, + options?: { ephemeral?: boolean; signal?: AbortSignal }, + ) => { if (capabilities.usesChromeMcp) { // listChromeMcpTabs creates the session if needed — no separate ensureChromeMcpAvailable call required. // Status probes opt into ephemeral so they reuse a cached attach session if one exists, // but do not seed a new persistent session as a side effect of read-only status calls. const { listChromeMcpTabs } = await getChromeMcpModule(); - const callOptions: { timeoutMs?: number; ephemeral?: boolean } = {}; + const callOptions: { timeoutMs?: number; ephemeral?: boolean; signal?: AbortSignal } = {}; if (timeoutMs != null) { callOptions.timeoutMs = timeoutMs; } if (options?.ephemeral) { callOptions.ephemeral = true; } + if (options?.signal) { + callOptions.signal = options.signal; + } await listChromeMcpTabs(profile.name, profile, callOptions); return true; } diff --git a/extensions/browser/src/browser/server-context.types.ts b/extensions/browser/src/browser/server-context.types.ts index dc07194a05c..799430e3be7 100644 --- a/extensions/browser/src/browser/server-context.types.ts +++ b/extensions/browser/src/browser/server-context.types.ts @@ -45,7 +45,10 @@ type BrowserProfileActions = { ensureTabAvailable: (targetId?: string) => Promise; isHttpReachable: (timeoutMs?: number) => Promise; isTransportAvailable: (timeoutMs?: number) => Promise; - isReachable: (timeoutMs?: number, options?: { ephemeral?: boolean }) => Promise; + isReachable: ( + timeoutMs?: number, + options?: { ephemeral?: boolean; signal?: AbortSignal }, + ) => Promise; listTabs: () => Promise; openTab: (url: string, opts?: { label?: string }) => Promise; labelTab: (targetId: string, label: string) => Promise;