From a16722aa2d71d51d4adfb29f4be3c72bcc47fea6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 26 Aug 2026 02:53:44 -0700 Subject: [PATCH] perf(browser): reuse cold status process discovery (#128692) --- .../src/browser/chrome-mcp-contracts.ts | 6 + .../browser/src/browser/chrome-mcp-process.ts | 57 ++-- .../browser/src/browser/chrome-mcp-tabs.ts | 18 +- .../browser/src/browser/chrome-mcp.test.ts | 69 +++++ .../routes/basic.existing-session.test.ts | 25 +- .../browser/src/browser/routes/basic.ts | 38 +-- .../browser/server-context.availability.ts | 26 +- .../server-context.existing-session.test.ts | 35 ++- .../server-context.list-profiles.test.ts | 256 +++++++++++++++++- .../browser/src/browser/server-context.ts | 15 +- .../src/browser/server-context.types.ts | 7 +- 11 files changed, 464 insertions(+), 88 deletions(-) diff --git a/extensions/browser/src/browser/chrome-mcp-contracts.ts b/extensions/browser/src/browser/chrome-mcp-contracts.ts index b1c9ae519886..f83aa0543cd8 100644 --- a/extensions/browser/src/browser/chrome-mcp-contracts.ts +++ b/extensions/browser/src/browser/chrome-mcp-contracts.ts @@ -74,6 +74,12 @@ export function rethrowChromeMcpDocumentError(error: unknown): never { export type ChromeMcpCallOptions = ChromeMcpOperationOptions & { ephemeral?: boolean; + pageProbe?: ChromeMcpPageProbe; +}; + +export type ChromeMcpPageProbe = { + timeoutMs?: () => number; + onResult: (tabCount: number | null) => void; }; export const MCP_REQUEST_TIMEOUT_CODE: number = ErrorCode.RequestTimeout; diff --git a/extensions/browser/src/browser/chrome-mcp-process.ts b/extensions/browser/src/browser/chrome-mcp-process.ts index c40854defdc0..73682f048419 100644 --- a/extensions/browser/src/browser/chrome-mcp-process.ts +++ b/extensions/browser/src/browser/chrome-mcp-process.ts @@ -246,9 +246,16 @@ async function terminateChromeMcpProcessTree( } const deps = getChromeMcpProcessCleanupDeps(); + const targets = [...target.descendants.toReversed(), target.root]; + let surviving = await currentChromeMcpProcesses(targets, deps); + // A fresh absence proof ends cleanup; snapshots from before awaited shutdown + // must never authorize signals against a recycled PID. + if (surviving.length === 0) { + return; + } if ((deps?.platform ?? process.platform) === "win32") { let firstError: Error | undefined; - if ((await currentChromeMcpProcesses([target.root], deps)).length > 0) { + if (surviving.some(({ pid }) => pid === target.root.pid)) { try { await taskkillChromeMcpProcessTree(target.root.pid, deps); } catch (err) { @@ -256,7 +263,15 @@ async function terminateChromeMcpProcessTree( } } await (deps?.sleep ?? sleepTimeout)(CHROME_MCP_PROCESS_EXIT_GRACE_MS); - for (const descendant of await currentChromeMcpProcesses(target.descendants, deps)) { + surviving = await currentChromeMcpProcesses(targets, deps); + if (surviving.length === 0) { + return; + } + for (const descendant of surviving.filter(({ pid }) => pid !== target.root.pid)) { + // An earlier awaited taskkill can recycle the next descendant's PID. + if ((await currentChromeMcpProcesses([descendant], deps)).length === 0) { + continue; + } try { await taskkillChromeMcpProcessTree(descendant.pid, deps); } catch (err) { @@ -264,7 +279,7 @@ async function terminateChromeMcpProcessTree( } } await (deps?.sleep ?? sleepTimeout)(CHROME_MCP_PROCESS_EXIT_GRACE_MS); - const surviving = await currentChromeMcpProcesses([target.root, ...target.descendants], deps); + surviving = await currentChromeMcpProcesses(targets, deps); if (surviving.length > 0) { throw ( firstError ?? @@ -278,29 +293,23 @@ async function terminateChromeMcpProcessTree( const killProcess = deps?.killProcess ?? ((pid, signal) => process.kill(pid, signal)); const sleep = deps?.sleep ?? sleepTimeout; - const targets = [...target.descendants.toReversed(), target.root]; - for (const owned of await currentChromeMcpProcesses(targets, deps)) { - try { - killProcess(owned.pid, "SIGTERM"); - } catch { - // The process may already have exited as part of client.close(). + for (const signal of ["SIGTERM", "SIGKILL"] as const) { + for (const owned of surviving) { + try { + killProcess(owned.pid, signal); + } catch { + // An owned process can exit after its identity was revalidated. + } + } + await sleep(CHROME_MCP_PROCESS_EXIT_GRACE_MS); + surviving = await currentChromeMcpProcesses(targets, deps); + if (surviving.length === 0) { + return; } } - await sleep(CHROME_MCP_PROCESS_EXIT_GRACE_MS); - for (const owned of await currentChromeMcpProcesses(targets, deps)) { - try { - killProcess(owned.pid, "SIGKILL"); - } catch { - // Best-effort cleanup only. - } - } - await sleep(CHROME_MCP_PROCESS_EXIT_GRACE_MS); - const surviving = await currentChromeMcpProcesses(targets, deps); - if (surviving.length > 0) { - throw new Error( - `Chrome MCP process cleanup failed for pid ${surviving.map(({ pid }) => pid).join(", ")}.`, - ); - } + throw new Error( + `Chrome MCP process cleanup failed for pid ${surviving.map(({ pid }) => pid).join(", ")}.`, + ); } async function closeChromeMcpSessionHandle(session: ChromeMcpSession): Promise { diff --git a/extensions/browser/src/browser/chrome-mcp-tabs.ts b/extensions/browser/src/browser/chrome-mcp-tabs.ts index b06a0ff482f7..555c56d2b121 100644 --- a/extensions/browser/src/browser/chrome-mcp-tabs.ts +++ b/extensions/browser/src/browser/chrome-mcp-tabs.ts @@ -40,7 +40,23 @@ export async function ensureChromeMcpAvailable( profileOptions?: string | ChromeMcpProfileOptions, options: ChromeMcpCallOptions = {}, ): Promise { - await withChromeMcpLease(profileName, profileOptions, options, async () => {}); + await withChromeMcpLease(profileName, profileOptions, options, async (lease, normalized) => { + if (!options.pageProbe) { + return; + } + try { + const pages = await listChromeMcpTargetsWithLease({ + profileName, + profileOptions: normalized, + lease, + options: { ...options, timeoutMs: options.pageProbe.timeoutMs?.() ?? options.timeoutMs }, + }); + options.pageProbe.onResult(pages.length); + } catch { + options.signal?.throwIfAborted(); + options.pageProbe.onResult(null); + } + }); } /** Return the cached Chrome MCP process pid for a profile, when present. */ diff --git a/extensions/browser/src/browser/chrome-mcp.test.ts b/extensions/browser/src/browser/chrome-mcp.test.ts index 345bb5f5b09e..5f03f4c2c542 100644 --- a/extensions/browser/src/browser/chrome-mcp.test.ts +++ b/extensions/browser/src/browser/chrome-mcp.test.ts @@ -1317,6 +1317,34 @@ describe("chrome MCP page parsing", () => { ]); }); + it.each(["linux", "darwin", "win32"] as const)( + "stops %s cleanup immediately once every owned process is absent", + async (platform) => { + const session = createFakeSession(); + Object.assign(session, { processCleanup: { status: "open" } }); + let alive = true; + session.client.close = vi.fn(async () => { + alive = false; + }) as typeof session.client.close; + const listProcesses = vi.fn(async () => (alive ? [processSnapshot(123, 1)] : [])); + const sleep = vi.fn().mockResolvedValue(undefined); + setChromeMcpProcessCleanupDepsForTest({ + platform, + listProcesses, + sleep, + taskkillProcessTree: async () => { + alive = false; + }, + }); + setChromeMcpSessionFactoryForTest(async () => session); + + await ensureChromeMcpAvailable("chrome-live", undefined, { ephemeral: true }); + + expect(listProcesses).toHaveBeenCalledTimes(platform === "win32" ? 3 : 2); + expect(sleep).toHaveBeenCalledTimes(platform === "win32" ? 1 : 0); + }, + ); + it("retains the proven root while skipping exited and reparented descendants", async () => { const session = createFakeSession(); Object.assign(session, { processCleanup: { status: "open" } }); @@ -1507,6 +1535,47 @@ describe("chrome MCP page parsing", () => { expect(taskkillProcessTree).not.toHaveBeenCalledWith(123); }); + it("never taskkills a descendant pid recycled while another Windows cleanup is awaited", async () => { + const session = createFakeSession(); + Object.assign(session, { + processCleanup: { + status: "tracked", + target: { + root: { pid: 123, identity: "start-123" }, + descendants: [ + { pid: 124, identity: "start-124" }, + { pid: 125, identity: "start-125" }, + ], + }, + }, + }); + let firstDescendantAlive = true; + let secondDescendantIdentity = "start-124"; + const taskkillProcessTree = vi.fn(async (pid: number) => { + if (pid !== 125) { + throw new Error("attempted to terminate a recycled pid"); + } + firstDescendantAlive = false; + secondDescendantIdentity = "start-reused"; + }); + setChromeMcpProcessCleanupDepsForTest({ + platform: "win32", + listProcesses: async () => [ + processSnapshot(124, 1, secondDescendantIdentity), + ...(firstDescendantAlive ? [processSnapshot(125, 1)] : []), + ], + taskkillProcessTree, + sleep: vi.fn().mockResolvedValue(undefined), + }); + setChromeMcpSessionFactoryForTest(async () => session); + + await ensureChromeMcpAvailable("chrome-live", undefined, { ephemeral: true }); + + expect(secondDescendantIdentity).toBe("start-reused"); + expect(taskkillProcessTree).toHaveBeenCalledExactlyOnceWith(125); + expect(taskkillProcessTree).not.toHaveBeenCalledWith(124); + }); + it("surfaces a surviving Chrome MCP process and retries its exact retained handle", async () => { const session = createFakeSession(); Object.assign(session, { processCleanup: { status: "open" } }); 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 a496f2015010..3ee62f871b4e 100644 --- a/extensions/browser/src/browser/routes/basic.existing-session.test.ts +++ b/extensions/browser/src/browser/routes/basic.existing-session.test.ts @@ -32,6 +32,8 @@ function createExistingSessionProfileState(params?: { options?: { ephemeral?: boolean; signal?: AbortSignal }, ) => Promise; }) { + const isTransportAvailable = params?.isTransportAvailable ?? (async () => true); + const isReachable = params?.isReachable ?? (async () => true); return { resolved: { enabled: true, @@ -54,8 +56,27 @@ function createExistingSessionProfileState(params?: { attachOnly: true, }, isHttpReachable: params?.isHttpReachable ?? (async () => true), - isTransportAvailable: params?.isTransportAvailable ?? (async () => true), - isReachable: params?.isReachable ?? (async () => true), + isTransportAvailable: async ( + timeoutMs?: number, + signal?: AbortSignal, + pageProbe?: { timeoutMs?: () => number; onResult: (tabCount: number | null) => void }, + ) => { + const available = await isTransportAvailable(timeoutMs, signal); + if (available && pageProbe) { + try { + const ready = await isReachable(pageProbe.timeoutMs?.() ?? timeoutMs, { + ephemeral: true, + signal, + }); + pageProbe.onResult(ready ? 1 : null); + } catch { + signal?.throwIfAborted(); + pageProbe.onResult(null); + } + } + return available; + }, + isReachable, }) as never, }; } diff --git a/extensions/browser/src/browser/routes/basic.ts b/extensions/browser/src/browser/routes/basic.ts index e7bcb21ce908..830df1e53e40 100644 --- a/extensions/browser/src/browser/routes/basic.ts +++ b/extensions/browser/src/browser/routes/basic.ts @@ -41,28 +41,6 @@ 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, - signal: AbortSignal, -) { - 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: AbortSignal.any([signal, abort.signal]), - }); - } catch { - signal.throwIfAborted(); - return false; - } finally { - clearTimeout(timer); - } -} - function handleBrowserRouteError(res: BrowserResponse, err: unknown) { if (isProfileRestartRequiredError(err)) { throw err; @@ -153,20 +131,14 @@ async function buildBrowserStatus( const [cdpHttp, cdpReady, pageReady] = capabilities.usesChromeMcp ? await (async () => { const statusStartedAtMs = Date.now(); + let pageReachable = false; const transportReady = await profileCtx.isTransportAvailable( STATUS_CHROME_MCP_TRANSPORT_TIMEOUT_MS, signal, - ); - if (!transportReady) { - return [false, false, false] as const; - } - // 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), - signal, + { + timeoutMs: () => remainingChromeMcpStatusTimeoutMs(statusStartedAtMs), + onResult: (tabCount) => (pageReachable = tabCount !== null), + }, ); return [transportReady, transportReady, pageReachable] as const; })() diff --git a/extensions/browser/src/browser/server-context.availability.ts b/extensions/browser/src/browser/server-context.availability.ts index dfc778210a40..d3eb12b31a17 100644 --- a/extensions/browser/src/browser/server-context.availability.ts +++ b/extensions/browser/src/browser/server-context.availability.ts @@ -50,6 +50,7 @@ import { import type { BrowserServerState, ContextOptions, + ProfileContext, ProfileRuntimeState, } from "./server-context.types.js"; @@ -63,7 +64,7 @@ type AvailabilityDeps = { type AvailabilityOps = { isHttpReachable: (timeoutMs?: number, signal?: AbortSignal) => Promise; - isTransportAvailable: (timeoutMs?: number, signal?: AbortSignal) => Promise; + isTransportAvailable: ProfileContext["isTransportAvailable"]; isReachable: ( timeoutMs?: number, options?: { ephemeral?: boolean; signal?: AbortSignal }, @@ -196,17 +197,11 @@ export function createProfileAvailability({ // but do not seed a new persistent session as a side effect of read-only status calls. assertChromeMcpCdpTransportAllowed(profile, getCdpReachabilityPolicy()); const { countChromeMcpTabs } = await getChromeMcpModule(); - 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 countChromeMcpTabs(profile.name, profile, callOptions); + await countChromeMcpTabs(profile.name, profile, { + ...(timeoutMs != null ? { timeoutMs } : {}), + ...(options?.ephemeral ? { ephemeral: true } : {}), + ...(options?.signal ? { signal: options.signal } : {}), + }); return true; } const { httpTimeoutMs, wsTimeoutMs } = resolveTimeouts(timeoutMs); @@ -218,7 +213,11 @@ export function createProfileAvailability({ ); }; - const isTransportAvailable = async (timeoutMs?: number, signal?: AbortSignal) => { + const isTransportAvailable: AvailabilityOps["isTransportAvailable"] = async ( + timeoutMs, + signal, + pageProbe, + ) => { if (capabilities.usesChromeMcp) { assertChromeMcpCdpTransportAllowed(profile, getCdpReachabilityPolicy()); const { ensureChromeMcpAvailable } = await getChromeMcpModule(); @@ -226,6 +225,7 @@ export function createProfileAvailability({ ephemeral: true, timeoutMs, signal, + ...(pageProbe ? { pageProbe } : {}), }); return true; } diff --git a/extensions/browser/src/browser/server-context.existing-session.test.ts b/extensions/browser/src/browser/server-context.existing-session.test.ts index 30c107222ad2..dc04bf8072a5 100644 --- a/extensions/browser/src/browser/server-context.existing-session.test.ts +++ b/extensions/browser/src/browser/server-context.existing-session.test.ts @@ -17,8 +17,37 @@ afterAll(() => { const chromeMcpMock = vi.hoisted(() => ({ closeChromeMcpSession: vi.fn(async () => true), - countChromeMcpTabs: vi.fn(async () => 1), - ensureChromeMcpAvailable: vi.fn(async () => {}), + countChromeMcpTabs: vi.fn( + async ( + _profileName: string, + _profile: unknown, + _options?: { ephemeral?: boolean; signal?: AbortSignal }, + ) => 1, + ), + ensureChromeMcpAvailable: vi.fn( + async ( + profileName: string, + profile: unknown, + options?: { + signal?: AbortSignal; + pageProbe?: { onResult: (tabCount: number | null) => void }; + }, + ) => { + if (!options?.pageProbe) { + return; + } + try { + options.pageProbe.onResult( + await chromeMcpMock.countChromeMcpTabs(profileName, profile, { + ephemeral: true, + signal: options.signal, + }), + ); + } catch { + options.pageProbe.onResult(null); + } + }, + ), focusChromeMcpTab: vi.fn(async () => {}), listChromeMcpTabs: vi.fn(async () => [ { targetId: "7", title: "", url: "https://example.com", type: "page" }, @@ -185,7 +214,6 @@ describe("browser server-context existing-session profile", () => { const state = makeState(); const ctx = createBrowserRouteContext({ getState: () => state }); - vi.mocked(chromeMcp.ensureChromeMcpAvailable).mockResolvedValueOnce(); vi.mocked(chromeMcp.countChromeMcpTabs).mockRejectedValueOnce(new Error("No page selected")); const profiles = await ctx.listProfiles(); @@ -208,6 +236,7 @@ describe("browser server-context existing-session profile", () => { ephemeral: true, timeoutMs: 300, signal: expect.any(AbortSignal), + pageProbe: { onResult: expect.any(Function) }, }); const [, countedProfile, countOptions] = ( diff --git a/extensions/browser/src/browser/server-context.list-profiles.test.ts b/extensions/browser/src/browser/server-context.list-profiles.test.ts index c3de453b1c5c..3247e3a3149d 100644 --- a/extensions/browser/src/browser/server-context.list-profiles.test.ts +++ b/extensions/browser/src/browser/server-context.list-profiles.test.ts @@ -1,17 +1,271 @@ // Browser tests cover server context.list profiles plugin behavior. +import { ErrorCode, McpError } from "@modelcontextprotocol/sdk/types.js"; import { afterEach, describe, expect, it, vi } from "vitest"; import "./server-context.chrome-test-harness.js"; +import { + listChromeMcpTabs, + resetChromeMcpSessionsForTest, + setChromeMcpProcessCleanupDepsForTest, + setChromeMcpSessionFactoryForTest, +} from "./chrome-mcp.js"; import * as chromeModule from "./chrome.js"; +import { registerBrowserBasicRoutes } from "./routes/basic.js"; +import { createBrowserRouteApp, createBrowserRouteResponse } from "./routes/test-helpers.js"; import { createBrowserRouteContext } from "./server-context.js"; import { beginProfileTransition } from "./server-context.lifecycle.js"; import { makeBrowserProfile, makeBrowserServerState } from "./server-context.test-harness.js"; -afterEach(() => { +afterEach(async () => { + await resetChromeMcpSessionsForTest(); vi.clearAllMocks(); vi.restoreAllMocks(); }); +function createExistingSessionProcessFixture( + profileCount = 1, + pageFailure = false, + options: { attachElapsedMs?: number; hangingPage?: boolean; instantProcessScans?: boolean } = {}, +) { + const profiles = Array.from({ length: profileCount }, (_, index) => + makeBrowserProfile({ + name: `chrome-live-${index + 1}`, + driver: "existing-session", + attachOnly: true, + cdpUrl: "", + cdpPort: 0, + userDataDir: `/tmp/openclaw-browser-status-${index + 1}`, + }), + ); + const profile = profiles[0]; + if (!profile) { + throw new Error("expected browser profile"); + } + const state = makeBrowserServerState({ + profile, + resolvedOverrides: { + defaultProfile: profile.name, + profiles: Object.fromEntries(profiles.map((current) => [current.name, current])), + }, + }); + const alive = new Set(); + let nextPid = 40_000; + const listProcesses = vi.fn(async () => { + if (!options.instantProcessScans) { + await new Promise((resolve) => { + setTimeout(resolve, 5); + }); + } + return [...alive].map((pid) => ({ pid, ppid: 1, identity: `fixture:${pid}` })); + }); + setChromeMcpProcessCleanupDepsForTest({ + platform: "linux", + listProcesses, + sleep: async () => {}, + killProcess: (pid) => alive.delete(pid), + }); + const callTool = vi.fn( + async ( + _request: { name: string; arguments?: Record }, + _resultSchema?: unknown, + requestOptions?: { signal?: AbortSignal; timeout?: number }, + ) => { + if (options.hangingPage) { + return await new Promise((_resolve, reject) => { + const timer = setTimeout( + () => reject(new McpError(ErrorCode.RequestTimeout, "Request timed out")), + requestOptions?.timeout ?? 60_000, + ); + requestOptions?.signal?.addEventListener( + "abort", + () => { + clearTimeout(timer); + reject(new McpError(ErrorCode.RequestTimeout, "Request cancelled")); + }, + { once: true }, + ); + }); + } + if (pageFailure) { + throw new Error("page unavailable"); + } + return { + content: [{ type: "text", text: "## Pages\n1: https://example.com [selected]" }], + }; + }, + ); + const factory = vi.fn(async () => { + if (options.attachElapsedMs) { + vi.setSystemTime(Date.now() + options.attachElapsedMs); + } + const pid = nextPid++; + alive.add(pid); + const transport: { pid: number | null } = { pid }; + return { + transport, + processCleanup: { status: "open" as const }, + ready: Promise.resolve(), + client: { + callTool, + close: vi.fn(async () => { + alive.delete(pid); + transport.pid = null; + }), + }, + } as never; + }); + setChromeMcpSessionFactoryForTest(factory); + return { callTool, factory, listProcesses, profile, profiles, state }; +} + describe("browser server-context listProfiles", () => { + it.each([1, 3])( + "uses one temporary MCP session and only authority-required process scans for %i cold profiles", + async (profileCount) => { + const fixture = createExistingSessionProcessFixture(profileCount); + const started = performance.now(); + const profiles = await createBrowserRouteContext({ + getState: () => fixture.state, + }).listProfiles(); + const elapsedMs = performance.now() - started; + + console.info( + `[browser-status-process-scans] profiles=${profileCount} scans=${fixture.listProcesses.mock.calls.length} elapsedMs=${elapsedMs.toFixed(1)}`, + ); + expect(profiles.map(({ name, running, tabCount }) => ({ name, running, tabCount }))).toEqual( + fixture.profiles.map(({ name }) => ({ name, running: true, tabCount: 1 })), + ); + expect(fixture.factory).toHaveBeenCalledTimes(profileCount); + expect(fixture.listProcesses).toHaveBeenCalledTimes(profileCount * 2); + }, + ); + + it("does not enumerate processes when profile status reuses a warm MCP session", async () => { + const fixture = createExistingSessionProcessFixture(); + const ctx = createBrowserRouteContext({ getState: () => fixture.state }); + const profile = ctx.forProfile(fixture.profile.name).profile; + await listChromeMcpTabs(profile.name, profile); + fixture.listProcesses.mockClear(); + + const profiles = await ctx.listProfiles(); + + expect(profiles[0]).toMatchObject({ running: true, tabCount: 1 }); + expect(fixture.factory).toHaveBeenCalledOnce(); + expect(fixture.listProcesses).not.toHaveBeenCalled(); + }); + + it("bounds a shared profile-list page probe with its inherited transport timeout", async () => { + const fixture = createExistingSessionProcessFixture(); + + const profiles = await createBrowserRouteContext({ + getState: () => fixture.state, + }).listProfiles(); + + expect(profiles[0]).toMatchObject({ running: true, tabCount: 1 }); + expect(fixture.callTool).toHaveBeenCalledWith( + { name: "list_pages", arguments: {} }, + undefined, + { signal: expect.any(AbortSignal), timeout: 300 }, + ); + }); + + it("keeps process discovery scoped to each independent cold profile request", async () => { + const fixture = createExistingSessionProcessFixture(); + const ctx = createBrowserRouteContext({ getState: () => fixture.state }); + + await ctx.listProfiles(); + await ctx.listProfiles(); + + expect(fixture.factory).toHaveBeenCalledTimes(2); + expect(fixture.listProcesses).toHaveBeenCalledTimes(4); + }); + + it("reuses one temporary MCP session for the real browser status route", async () => { + const fixture = createExistingSessionProcessFixture(); + const ctx = createBrowserRouteContext({ getState: () => fixture.state }); + const { app, getHandlers } = createBrowserRouteApp(); + registerBrowserBasicRoutes(app, ctx); + const response = createBrowserRouteResponse(); + const started = performance.now(); + + await getHandlers.get("/")?.( + { params: {}, query: { profile: fixture.profile.name } }, + response.res, + ); + + console.info( + `[browser-status-process-scans] route=status scans=${fixture.listProcesses.mock.calls.length} elapsedMs=${(performance.now() - started).toFixed(1)}`, + ); + expect(response.statusCode).toBe(200); + expect(response.body).toMatchObject({ + profile: fixture.profile.name, + running: true, + cdpReady: true, + pageReady: true, + }); + expect(fixture.factory).toHaveBeenCalledOnce(); + expect(fixture.listProcesses).toHaveBeenCalledTimes(2); + }); + + it("preserves healthy transport status when the shared page probe fails", async () => { + const fixture = createExistingSessionProcessFixture(1, true); + const ctx = createBrowserRouteContext({ getState: () => fixture.state }); + const { app, getHandlers } = createBrowserRouteApp(); + registerBrowserBasicRoutes(app, ctx); + const response = createBrowserRouteResponse(); + + await getHandlers.get("/")?.( + { params: {}, query: { profile: fixture.profile.name } }, + response.res, + ); + + expect(response.body).toMatchObject({ running: true, cdpReady: true, pageReady: false }); + expect(fixture.factory).toHaveBeenCalledOnce(); + + const profiles = await ctx.listProfiles(); + expect(profiles[0]).toMatchObject({ running: true, tabCount: 0 }); + expect(fixture.factory).toHaveBeenCalledTimes(2); + }); + + it("times out a stuck status page probe within the budget remaining after attach", async () => { + vi.useFakeTimers(); + vi.setSystemTime(1_000); + try { + const fixture = createExistingSessionProcessFixture(1, false, { + attachElapsedMs: 3_000, + hangingPage: true, + instantProcessScans: true, + }); + const ctx = createBrowserRouteContext({ getState: () => fixture.state }); + const { app, getHandlers } = createBrowserRouteApp(); + registerBrowserBasicRoutes(app, ctx); + const response = createBrowserRouteResponse(); + + const pending = getHandlers.get("/")?.( + { params: {}, query: { profile: fixture.profile.name } }, + response.res, + ); + await vi.advanceTimersByTimeAsync(0); + + expect(fixture.callTool).toHaveBeenCalledWith( + { name: "list_pages", arguments: {} }, + undefined, + { signal: expect.any(AbortSignal), timeout: 4_000 }, + ); + await vi.advanceTimersByTimeAsync(3_999); + expect(response.body).toBeUndefined(); + + await vi.advanceTimersByTimeAsync(1); + await pending; + + expect(response.statusCode).toBe(200); + expect(response.body).toMatchObject({ running: true, cdpReady: true, pageReady: false }); + expect(fixture.factory).toHaveBeenCalledOnce(); + expect(fixture.listProcesses).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } + }); + it("reads running state only after an in-flight profile transition settles", async () => { const state = makeBrowserServerState(); const ctx = createBrowserRouteContext({ getState: () => state }); diff --git a/extensions/browser/src/browser/server-context.ts b/extensions/browser/src/browser/server-context.ts index 68b0e518f8e0..9de0697d6d0c 100644 --- a/extensions/browser/src/browser/server-context.ts +++ b/extensions/browser/src/browser/server-context.ts @@ -8,7 +8,6 @@ import { } from "./cdp-reachability-policy.js"; import { usesFastLoopbackCdpProbeClass } from "./cdp-timeouts.js"; import { redactCdpUrl } from "./cdp.helpers.js"; -import { countChromeMcpTabs } from "./chrome-mcp.js"; import { isChromeReachable, resolveOpenClawUserDataDir } from "./chrome.js"; import { getOwnBrowserProfile, resolveProfile, type ResolvedBrowserProfile } from "./config.js"; import { @@ -174,10 +173,10 @@ function createProfileContext( callerSignal, async (signal) => await rawAvailability.isHttpReachable(timeoutMs, signal), ), - isTransportAvailable: async (timeoutMs, callerSignal) => + isTransportAvailable: async (timeoutMs, callerSignal, pageProbe) => await withLease( callerSignal, - async (signal) => await rawAvailability.isTransportAvailable(timeoutMs, signal), + async (signal) => await rawAvailability.isTransportAvailable(timeoutMs, signal, pageProbe), ), isReachable: async (timeoutMs, options) => await withLease( @@ -285,13 +284,9 @@ export function createBrowserRouteContext(opts: ContextOptions): BrowserRouteCon if (capabilities.usesChromeMcp) { try { - activeRunning = await profileCtx.isTransportAvailable(300); - if (activeRunning) { - activeTabCount = await countChromeMcpTabs(activeProfile.name, activeProfile, { - ephemeral: true, - signal, - }).catch(() => 0); - } + activeRunning = await profileCtx.isTransportAvailable(300, signal, { + onResult: (observedTabCount) => (activeTabCount = observedTabCount ?? 0), + }); } catch { activeRunning = false; } diff --git a/extensions/browser/src/browser/server-context.types.ts b/extensions/browser/src/browser/server-context.types.ts index 4222c22286b6..9988c39df6f1 100644 --- a/extensions/browser/src/browser/server-context.types.ts +++ b/extensions/browser/src/browser/server-context.types.ts @@ -3,6 +3,7 @@ * operation factories. */ import type { Server } from "node:http"; +import type { ChromeMcpPageProbe } from "./chrome-mcp-contracts.js"; import type { RunningChrome } from "./chrome.js"; import type { BrowserOpenResult, BrowserTab, BrowserTransport } from "./client.types.js"; import type { ResolvedBrowserConfig, ResolvedBrowserProfile } from "./config.js"; @@ -64,7 +65,11 @@ type BrowserProfileActions = { options?: EnsureTabAvailableOptions, ) => Promise; isHttpReachable: (timeoutMs?: number, signal?: AbortSignal) => Promise; - isTransportAvailable: (timeoutMs?: number, signal?: AbortSignal) => Promise; + isTransportAvailable: ( + timeoutMs?: number, + signal?: AbortSignal, + pageProbe?: ChromeMcpPageProbe, + ) => Promise; isReachable: ( timeoutMs?: number, options?: { ephemeral?: boolean; signal?: AbortSignal },