From c5f964f5043493e7f23edfbb59c891960d6ce708 Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Wed, 1 Apr 2026 19:23:43 -0700 Subject: [PATCH 01/15] fix: prevent PostHog analytics data loss MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Gate /api/version ping to interactive-only (skip CI/agent), send machine ID as query param for correlation with PostHog distinctId - Fix captureImmediate: Effect.sync → Effect.tryPromise so the HTTP promise is properly awaited instead of fire-and-forget - Fix flush: remove Effect.ignore, handle errors at Analytics service layer with logging instead of silent swallow - Fix session-analytics: use ManagedRuntime so machineId() runs once and distinctId stays stable across calls, remove redundant error swallowing --- apps/cli/src/index.tsx | 11 ++++++++--- apps/cli/src/utils/session-analytics.ts | 15 ++++++-------- packages/shared/src/analytics/analytics.ts | 23 ++++++++++++++-------- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/apps/cli/src/index.tsx b/apps/cli/src/index.tsx index 30b71e225..606a2c661 100644 --- a/apps/cli/src/index.tsx +++ b/apps/cli/src/index.tsx @@ -21,9 +21,14 @@ import { prompts } from "./utils/prompts"; import { highlighter } from "./utils/highlighter"; import { logger } from "./utils/logger"; -try { - fetch(`${VERSION_API_URL}?source=cli&t=${Date.now()}`).catch(() => {}); -} catch {} +if (!isRunningInAgent() && !isHeadless()) { + import("node-machine-id") + .then((module) => module.machineId()) + .catch(() => "unknown") + .then((mid) => { + fetch(`${VERSION_API_URL}?source=cli&mid=${mid}&t=${Date.now()}`).catch(() => {}); + }); +} const DEFAULT_INSTRUCTION = "Test all changes from main in the browser and verify they work correctly."; diff --git a/apps/cli/src/utils/session-analytics.ts b/apps/cli/src/utils/session-analytics.ts index f7c857254..7d3a4fd88 100644 --- a/apps/cli/src/utils/session-analytics.ts +++ b/apps/cli/src/utils/session-analytics.ts @@ -1,14 +1,14 @@ -import { Effect } from "effect"; +import { Effect, ManagedRuntime } from "effect"; import { Analytics, type EventMap } from "@expect/shared/observability"; import { usePreferencesStore } from "../stores/use-preferences"; -const analyticsLayer = Analytics.layerPostHog; +const analyticsRuntime = ManagedRuntime.make(Analytics.layerPostHog); export const trackEvent = ( eventName: K, ...[properties]: EventMap[K] extends undefined ? [] : [EventMap[K]] ) => - Effect.runPromise( + analyticsRuntime.runPromise( Effect.gen(function* () { const analytics = yield* Analytics; const captureEffect: Effect.Effect = (analytics.capture as Function).call( @@ -17,10 +17,7 @@ export const trackEvent = ( ...(properties !== undefined ? [properties] : []), ); yield* captureEffect; - }).pipe( - Effect.catchCause(() => Effect.void), - Effect.provide(analyticsLayer), - ), + }), ); export const trackSessionStarted = () => @@ -31,12 +28,12 @@ export const trackSessionStarted = () => }); export const flushSession = (sessionStartedAt: number) => - Effect.runPromise( + analyticsRuntime.runPromise( Effect.gen(function* () { const analytics = yield* Analytics; yield* analytics.capture("session:ended", { session_ms: Date.now() - sessionStartedAt, }); yield* analytics.flush; - }).pipe(Effect.provide(analyticsLayer)), + }), ); diff --git a/packages/shared/src/analytics/analytics.ts b/packages/shared/src/analytics/analytics.ts index 6fc66a547..3efd542b5 100644 --- a/packages/shared/src/analytics/analytics.ts +++ b/packages/shared/src/analytics/analytics.ts @@ -37,13 +37,13 @@ export class AnalyticsProvider extends ServiceMap.Service< >()("@expect/AnalyticsProvider") { static layerPostHog = Layer.succeed(this)({ capture: (event) => - Effect.sync(() => { + Effect.tryPromise(() => posthogClient.captureImmediate({ event: event.eventName, properties: event.properties, distinctId: event.distinctId, - }); - }), + }), + ), identify: (params) => Effect.sync(() => { posthogClient.identify({ @@ -54,10 +54,7 @@ export class AnalyticsProvider extends ServiceMap.Service< }, }); }), - flush: Effect.tryPromise({ - try: () => posthogClient.flush(), - catch: (cause) => cause, - }).pipe(Effect.ignore), + flush: Effect.tryPromise(() => posthogClient.flush()), }); static layerDev = Layer.succeed(this)({ @@ -158,7 +155,17 @@ export class Analytics extends ServiceMap.Service()("@expect/Analytic ); })) as never; - return { capture, track, flush: telemetryDisabled ? Effect.void : provider.flush } as const; + const flush = telemetryDisabled + ? Effect.void + : provider.flush.pipe( + Effect.catchCause((cause) => + Effect.logWarning("Analytics flush failed", { cause }).pipe( + Effect.annotateLogs({ module: "Analytics" }), + ), + ), + ); + + return { capture, track, flush } as const; }), }) { static layerPostHog = Layer.effect(this)(this.make).pipe( From 4555adc9ef24439218355f5e8c99bb58c1f73bfa Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Wed, 1 Apr 2026 19:39:18 -0700 Subject: [PATCH 02/15] fix --- apps/cli/src/commands/init.ts | 4 +- apps/cli/tests/add-skill.test.ts | 4 +- packages/browser/package.json | 2 + packages/browser/src/browser.ts | 41 ++- packages/browser/src/chrome-launcher.ts | 267 ++++++++++++++++++ packages/browser/src/constants.ts | 5 + packages/browser/src/errors.ts | 19 ++ packages/browser/src/index.ts | 3 + packages/browser/src/mcp/mcp-session.ts | 10 + packages/browser/src/mcp/server.ts | 12 +- packages/browser/src/types.ts | 1 + .../browser/tests/chrome-launcher.test.ts | 184 ++++++++++++ pnpm-lock.yaml | 22 +- 13 files changed, 550 insertions(+), 24 deletions(-) create mode 100644 packages/browser/src/chrome-launcher.ts create mode 100644 packages/browser/tests/chrome-launcher.test.ts diff --git a/apps/cli/src/commands/init.ts b/apps/cli/src/commands/init.ts index d7d912901..62864a78d 100644 --- a/apps/cli/src/commands/init.ts +++ b/apps/cli/src/commands/init.ts @@ -122,9 +122,7 @@ export const runInit = async (options: InitOptions = {}) => { `Installed! ${highlighter.info("expect-cli")} is now available globally.`, ); } else { - globalSpinner.warn( - `Installed, but ${highlighter.info("expect-cli")} is not on your PATH.`, - ); + globalSpinner.warn(`Installed, but ${highlighter.info("expect-cli")} is not on your PATH.`); const globalPrefix = spawnSync("npm", ["prefix", "-g"], { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"], diff --git a/apps/cli/tests/add-skill.test.ts b/apps/cli/tests/add-skill.test.ts index db25f2311..799752b26 100644 --- a/apps/cli/tests/add-skill.test.ts +++ b/apps/cli/tests/add-skill.test.ts @@ -92,9 +92,7 @@ describe("extractTarEntries", () => { }); it("creates nested directories", () => { - const tar = buildTar([ - { name: "prefix/sub/dir/file.txt", content: "nested" }, - ]); + const tar = buildTar([{ name: "prefix/sub/dir/file.txt", content: "nested" }]); extractTarEntries(tar, "prefix/", destDir); diff --git a/packages/browser/package.json b/packages/browser/package.json index 9b030d68b..60d1c0956 100644 --- a/packages/browser/package.json +++ b/packages/browser/package.json @@ -30,10 +30,12 @@ "effect": "4.0.0-beta.35", "playwright": "^1.52.0", "rrweb": "^2.0.0-alpha.18", + "which": "^6.0.1", "zod": "^4.3.6" }, "devDependencies": { "@types/node": "^22.15.0", + "@types/which": "^3.0.4", "esbuild": "^0.25.0", "typescript": "^5.7.0" } diff --git a/packages/browser/src/browser.ts b/packages/browser/src/browser.ts index ecc74dec0..68e607012 100644 --- a/packages/browser/src/browser.ts +++ b/packages/browser/src/browser.ts @@ -24,6 +24,7 @@ import { NavigationError, SnapshotTimeoutError, } from "./errors"; +import { launchSystemChrome, killChromeProcess } from "./chrome-launcher"; import { toActionError } from "./utils/action-error"; import { compactTree } from "./utils/compact-tree"; import { createLocator } from "./utils/create-locator"; @@ -194,13 +195,33 @@ export class Browser extends ServiceMap.Service()("@browser/Browser", { options: CreatePageOptions = {}, ) { const engine = options.browserType ?? "chromium"; + const useSystemChrome = options.systemChrome && engine === "chromium"; yield* Effect.annotateCurrentSpan({ url: url ?? "about:blank", cdp: Boolean(options.cdpUrl), + systemChrome: Boolean(useSystemChrome), browserType: engine, }); - const cdpEndpoint = engine === "chromium" ? options.cdpUrl : undefined; + const chromeProcess = + useSystemChrome && !options.cdpUrl + ? yield* launchSystemChrome({ + headless: !options.headed, + }).pipe( + Effect.tap((launched) => + Effect.logInfo("Connected to system Chrome via CDP", { wsUrl: launched.wsUrl }), + ), + Effect.catchTags({ + ChromeNotFoundError: Effect.die, + ChromeLaunchTimeoutError: Effect.die, + }), + ) + : undefined; + + const cdpEndpoint = + chromeProcess?.wsUrl ?? (engine === "chromium" ? options.cdpUrl : undefined); + const cleanup = chromeProcess ? killChromeProcess(chromeProcess) : Effect.void; + const browserType = resolveBrowserType(engine); const browser = cdpEndpoint ? yield* Effect.tryPromise({ @@ -301,16 +322,20 @@ export class Browser extends ServiceMap.Service()("@browser/Browser", { }); } - return { browser, context, page }; + return { browser, context, page, cleanup }; }); return yield* setupPage.pipe( - Effect.tapError(() => { - if (cdpEndpoint) return Effect.void; - return Effect.tryPromise(() => browser.close()).pipe( - Effect.catchTag("UnknownError", () => Effect.void), - ); - }), + Effect.tapError(() => + cleanup.pipe( + Effect.andThen(() => { + if (cdpEndpoint) return Effect.void; + return Effect.tryPromise(() => browser.close()).pipe( + Effect.catchTag("UnknownError", () => Effect.void), + ); + }), + ), + ), ); }); diff --git a/packages/browser/src/chrome-launcher.ts b/packages/browser/src/chrome-launcher.ts new file mode 100644 index 000000000..a6f25fff1 --- /dev/null +++ b/packages/browser/src/chrome-launcher.ts @@ -0,0 +1,267 @@ +import { spawn, type ChildProcess } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import which from "which"; +import { Effect } from "effect"; +import { ChromeNotFoundError, ChromeLaunchTimeoutError } from "./errors"; +import { + CDP_LAUNCH_TIMEOUT_MS, + CDP_POLL_INTERVAL_MS, + HEADLESS_CHROME_WINDOW_HEIGHT_PX, + HEADLESS_CHROME_WINDOW_WIDTH_PX, +} from "./constants"; + +interface ChromeProcess { + readonly process: ChildProcess; + readonly wsUrl: string; + readonly userDataDir: string; + readonly tempUserDataDir: string | undefined; +} + +const SYSTEM_CHROME_PATHS_DARWIN = [ + "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome", + "/Applications/Google Chrome Canary.app/Contents/MacOS/Google Chrome Canary", + "/Applications/Chromium.app/Contents/MacOS/Chromium", + "/Applications/Brave Browser.app/Contents/MacOS/Brave Browser", + "/Applications/Microsoft Edge.app/Contents/MacOS/Microsoft Edge", + "/Applications/Arc.app/Contents/MacOS/Arc", +] as const; + +const SYSTEM_CHROME_NAMES_LINUX = [ + "google-chrome", + "google-chrome-stable", + "chromium-browser", + "chromium", + "brave-browser", + "brave-browser-stable", + "microsoft-edge", +] as const; + +export const findSystemChrome = Effect.fn("findSystemChrome")(function* () { + const platform = os.platform(); + + if (platform === "darwin") { + for (const candidate of SYSTEM_CHROME_PATHS_DARWIN) { + if (fs.existsSync(candidate)) { + yield* Effect.logDebug("Found system Chrome", { path: candidate }); + return candidate; + } + } + } + + if (platform === "linux") { + for (const name of SYSTEM_CHROME_NAMES_LINUX) { + const resolved = which.sync(name, { nothrow: true }); + if (resolved) { + yield* Effect.logDebug("Found system Chrome", { path: resolved }); + return resolved; + } + } + } + + if (platform === "win32") { + // HACK: process.env is used for Windows system path discovery — these are + // OS-level paths, not app configuration + const localAppData = process.env["LOCALAPPDATA"]; + const programFiles = process.env["PROGRAMFILES"]; + const programFilesX86 = process.env["PROGRAMFILES(X86)"]; + + const candidates = [ + localAppData && path.join(localAppData, "Google", "Chrome", "Application", "chrome.exe"), + localAppData && + path.join(localAppData, "BraveSoftware", "Brave-Browser", "Application", "brave.exe"), + programFiles && path.join(programFiles, "Google", "Chrome", "Application", "chrome.exe"), + programFilesX86 && + path.join(programFilesX86, "Google", "Chrome", "Application", "chrome.exe"), + ].filter(Boolean) as string[]; + + for (const candidate of candidates) { + if (fs.existsSync(candidate)) { + yield* Effect.logDebug("Found system Chrome", { path: candidate }); + return candidate; + } + } + } + + return yield* new ChromeNotFoundError(); +}); + +const readDevToolsActivePort = ( + userDataDir: string, +): { port: number; wsPath: string } | undefined => { + const filePath = path.join(userDataDir, "DevToolsActivePort"); + try { + const content = fs.readFileSync(filePath, "utf-8"); + const lines = content.trim().split("\n"); + const portStr = lines[0]?.trim(); + if (!portStr) return undefined; + const port = Number.parseInt(portStr, 10); + if (Number.isNaN(port)) return undefined; + const wsPath = lines[1]?.trim() ?? "/devtools/browser"; + return { port, wsPath }; + } catch { + return undefined; + } +}; + +const buildLaunchArgs = (options: { headless: boolean; userDataDir: string }): string[] => { + const args = [ + "--remote-debugging-port=0", + "--no-first-run", + "--no-default-browser-check", + "--disable-background-networking", + "--disable-backgrounding-occluded-windows", + "--disable-component-update", + "--disable-default-apps", + "--disable-hang-monitor", + "--disable-popup-blocking", + "--disable-prompt-on-repost", + "--disable-sync", + "--disable-features=Translate", + "--enable-features=NetworkService,NetworkServiceInProcess", + "--metrics-recording-only", + "--password-store=basic", + "--use-mock-keychain", + `--user-data-dir=${options.userDataDir}`, + ]; + + if (options.headless) { + args.push( + "--headless=new", + "--enable-unsafe-swiftshader", + `--window-size=${HEADLESS_CHROME_WINDOW_WIDTH_PX},${HEADLESS_CHROME_WINDOW_HEIGHT_PX}`, + ); + } + + if (isContainerEnvironment()) { + args.push("--no-sandbox", "--disable-dev-shm-usage"); + } + + return args; +}; + +// HACK: process.env["CI"] is used for container detection — runtime environment +// introspection, not app configuration +const isContainerEnvironment = (): boolean => { + if (process.env["CI"]) return true; + if (os.platform() !== "linux") return false; + if (process.getuid?.() === 0) return true; + if (fs.existsSync("/.dockerenv")) return true; + if (fs.existsSync("/run/.containerenv")) return true; + try { + const cgroup = fs.readFileSync("/proc/1/cgroup", "utf-8"); + return cgroup.includes("docker") || cgroup.includes("kubepods") || cgroup.includes("lxc"); + } catch { + return false; + } +}; + +const cleanupFailedLaunch = (child: ChildProcess, tempDir: string | undefined) => + Effect.gen(function* () { + yield* Effect.sync(() => child.kill()).pipe( + Effect.catchCause((cause) => + Effect.logDebug("Failed to kill Chrome process during cleanup", { cause }), + ), + ); + if (tempDir) { + yield* Effect.sync(() => fs.rmSync(tempDir, { recursive: true, force: true })).pipe( + Effect.catchCause((cause) => + Effect.logDebug("Failed to remove temp dir during cleanup", { cause }), + ), + ); + } + }); + +export const launchSystemChrome = Effect.fn("launchSystemChrome")(function* (options: { + headless: boolean; + profilePath?: string; +}) { + const chromePath = yield* findSystemChrome(); + + const tempDir = options.profilePath + ? undefined + : fs.mkdtempSync(path.join(os.tmpdir(), "expect-chrome-")); + + const userDataDir = options.profilePath ?? tempDir!; + + fs.rmSync(path.join(userDataDir, "DevToolsActivePort"), { force: true }); + + const args = buildLaunchArgs({ headless: options.headless, userDataDir }); + + yield* Effect.logInfo("Launching system Chrome", { chromePath, userDataDir }); + + const child = yield* Effect.try({ + try: (): ChildProcess => + spawn(chromePath, args, { + stdio: ["ignore", "ignore", "pipe"], + detached: false, + }), + catch: () => new ChromeNotFoundError(), + }); + + const wsUrl = yield* Effect.callback((resume) => { + const deadline = Date.now() + CDP_LAUNCH_TIMEOUT_MS; + + const poll = () => { + if (Date.now() > deadline) { + resume( + Effect.fail( + new ChromeLaunchTimeoutError({ + timeoutMs: CDP_LAUNCH_TIMEOUT_MS, + cause: "Timed out waiting for DevToolsActivePort", + }), + ), + ); + return; + } + + const result = readDevToolsActivePort(userDataDir); + if (result) { + resume(Effect.succeed(`ws://127.0.0.1:${result.port}${result.wsPath}`)); + return; + } + + if (child.exitCode !== null) { + resume( + Effect.fail( + new ChromeLaunchTimeoutError({ + timeoutMs: CDP_LAUNCH_TIMEOUT_MS, + cause: `Chrome exited with code ${child.exitCode} before providing CDP URL`, + }), + ), + ); + return; + } + + setTimeout(poll, CDP_POLL_INTERVAL_MS); + }; + + poll(); + }).pipe(Effect.tapError(() => cleanupFailedLaunch(child, tempDir))); + + yield* Effect.logInfo("System Chrome launched, CDP available", { wsUrl }); + + return { + process: child, + wsUrl, + userDataDir, + tempUserDataDir: tempDir, + } satisfies ChromeProcess; +}); + +export const killChromeProcess = (chrome: ChromeProcess) => + Effect.gen(function* () { + yield* Effect.sync(() => chrome.process.kill()).pipe( + Effect.catchCause((cause) => Effect.logDebug("Failed to kill Chrome process", { cause })), + ); + if (chrome.tempUserDataDir) { + yield* Effect.sync(() => + fs.rmSync(chrome.tempUserDataDir!, { recursive: true, force: true }), + ).pipe( + Effect.catchCause((cause) => + Effect.logDebug("Failed to remove temp user data dir", { cause }), + ), + ); + } + }); diff --git a/packages/browser/src/constants.ts b/packages/browser/src/constants.ts index 10516037f..7b846134e 100644 --- a/packages/browser/src/constants.ts +++ b/packages/browser/src/constants.ts @@ -57,3 +57,8 @@ export const REPLAY_PLAYER_HEIGHT_PX = 540; export const CDP_DISCOVERY_TIMEOUT_MS = 2_000; export const CDP_PORT_PROBE_TIMEOUT_MS = 500; export const CDP_COMMON_PORTS = [9222, 9229] as const; +export const CDP_LAUNCH_TIMEOUT_MS = 30_000; +export const CDP_POLL_INTERVAL_MS = 50; + +export const HEADLESS_CHROME_WINDOW_WIDTH_PX = 1280; +export const HEADLESS_CHROME_WINDOW_HEIGHT_PX = 720; diff --git a/packages/browser/src/errors.ts b/packages/browser/src/errors.ts index 144569b81..64fec9211 100644 --- a/packages/browser/src/errors.ts +++ b/packages/browser/src/errors.ts @@ -116,6 +116,25 @@ export class CdpConnectionError extends Schema.ErrorClass("C message = `Failed to connect to CDP endpoint ${this.endpointUrl}: ${this.cause}`; } +export class ChromeNotFoundError extends Schema.ErrorClass( + "ChromeNotFoundError", +)({ + _tag: Schema.tag("ChromeNotFoundError"), +}) { + message = + "No system Chrome installation found. Install Google Chrome or pass an explicit executable path."; +} + +export class ChromeLaunchTimeoutError extends Schema.ErrorClass( + "ChromeLaunchTimeoutError", +)({ + _tag: Schema.tag("ChromeLaunchTimeoutError"), + timeoutMs: Schema.Number, + cause: Schema.String, +}) { + message = `Chrome launch failed (timeout ${this.timeoutMs}ms): ${this.cause}`; +} + export type ActionError = | RefAmbiguousError | RefBlockedError diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index 40742be02..d97d2b099 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -3,6 +3,7 @@ export { buildReplayViewerHtml } from "./replay-viewer"; export { diffSnapshots } from "./diff"; export { collectEvents, collectAllEvents, loadSession } from "./recorder"; export { autoDiscoverCdp, discoverCdpUrl } from "./cdp-discovery"; +export { launchSystemChrome, killChromeProcess } from "./chrome-launcher"; export { RrVideo, RrVideoConvertError } from "./rrvideo"; export type { Browser as BrowserProfile, @@ -16,6 +17,8 @@ export { BrowserLaunchError, CdpConnectionError, CdpDiscoveryError, + ChromeNotFoundError, + ChromeLaunchTimeoutError, NavigationError, RecorderInjectionError, RefAmbiguousError, diff --git a/packages/browser/src/mcp/mcp-session.ts b/packages/browser/src/mcp/mcp-session.ts index 31ec141df..27f55d372 100644 --- a/packages/browser/src/mcp/mcp-session.ts +++ b/packages/browser/src/mcp/mcp-session.ts @@ -44,6 +44,7 @@ export interface BrowserSessionData { readonly browser: PlaywrightBrowser; readonly context: BrowserContext; readonly page: Page; + readonly cleanup: Effect.Effect; readonly consoleMessages: ConsoleEntry[]; readonly networkRequests: NetworkEntry[]; readonly replayOutputPath: string | undefined; @@ -58,6 +59,7 @@ export interface OpenOptions { waitUntil?: "load" | "domcontentloaded" | "networkidle" | "commit"; cdpUrl?: string; browserType?: BrowserEngine; + systemChrome?: boolean; } export interface OpenResult { @@ -225,12 +227,14 @@ export class McpSession extends ServiceMap.Service()("@browser/McpSe videoOutputDir, cdpUrl: options.cdpUrl ?? defaultCdpUrl, browserType: options.browserType, + systemChrome: options.systemChrome, }); const sessionData: BrowserSessionData = { browser: pageResult.browser, context: pageResult.context, page: pageResult.page, + cleanup: pageResult.cleanup, consoleMessages: [], networkRequests: [], replayOutputPath: Option.getOrUndefined(replayOutputPath), @@ -512,6 +516,12 @@ export class McpSession extends ServiceMap.Service()("@browser/McpSe Effect.catchCause((cause) => Effect.logDebug("Failed to close browser", { cause })), ); + yield* activeSession.cleanup.pipe( + Effect.catchCause((cause) => + Effect.logDebug("Failed to clean up Chrome process", { cause }), + ), + ); + if (pageVideo) { videoPath = yield* Effect.tryPromise(() => pageVideo.path()).pipe( Effect.catchCause((cause) => diff --git a/packages/browser/src/mcp/server.ts b/packages/browser/src/mcp/server.ts index fb8bc6be9..e79d98099 100644 --- a/packages/browser/src/mcp/server.ts +++ b/packages/browser/src/mcp/server.ts @@ -79,9 +79,15 @@ export const createBrowserMcpServer = ( .describe( "Browser engine to launch (default: chromium). Use 'webkit' for Safari-like testing or 'firefox' for Firefox testing. CDP connections are only supported with chromium.", ), + systemChrome: z + .boolean() + .optional() + .describe( + "Launch the system-installed Chrome (Google Chrome, Brave, Edge) instead of Playwright's bundled Chromium. Connects via CDP. Useful for testing with the real browser the user has installed. Ignored when 'cdp' is also provided.", + ), }, }, - ({ url, headed, cookies, waitUntil, cdp, browser: browserType }) => + ({ url, headed, cookies, waitUntil, cdp, browser: browserType, systemChrome }) => runMcp( Effect.gen(function* () { const session = yield* McpSession; @@ -105,11 +111,13 @@ export const createBrowserMcpServer = ( waitUntil, cdpUrl, browserType, + systemChrome, }); const engineSuffix = browserType && browserType !== "chromium" ? ` [${browserType}]` : ""; const cdpSuffix = cdpUrl ? ` (connected via CDP: ${cdpUrl})` : ""; + const chromeSuffix = systemChrome ? " (system Chrome)" : ""; return textResult( - `Opened ${url}${engineSuffix}${cdpSuffix}` + + `Opened ${url}${engineSuffix}${cdpSuffix}${chromeSuffix}` + (result.injectedCookieCount > 0 ? ` (${result.injectedCookieCount} cookies synced from local browser)` : ""), diff --git a/packages/browser/src/types.ts b/packages/browser/src/types.ts index 209d7b990..fbbcab8cb 100644 --- a/packages/browser/src/types.ts +++ b/packages/browser/src/types.ts @@ -54,6 +54,7 @@ export interface CreatePageOptions { videoOutputDir?: string; cdpUrl?: string; browserType?: BrowserEngine; + systemChrome?: boolean; } export interface AnnotatedScreenshotOptions extends SnapshotOptions { diff --git a/packages/browser/tests/chrome-launcher.test.ts b/packages/browser/tests/chrome-launcher.test.ts new file mode 100644 index 000000000..d5c5a5081 --- /dev/null +++ b/packages/browser/tests/chrome-launcher.test.ts @@ -0,0 +1,184 @@ +import { beforeEach, describe, expect, it, vi } from "vite-plus/test"; +import type { ChildProcess } from "node:child_process"; + +const { platformMock, existsSyncMock, rmSyncMock, whichSyncMock } = vi.hoisted(() => ({ + platformMock: vi.fn(), + existsSyncMock: vi.fn(), + rmSyncMock: vi.fn(), + whichSyncMock: vi.fn(), +})); + +vi.mock("node:os", async (importOriginal) => { + const original = await importOriginal(); + return { default: { ...original, platform: platformMock } }; +}); + +vi.mock("node:fs", async (importOriginal) => { + const original = await importOriginal(); + return { default: { ...original, existsSync: existsSyncMock, rmSync: rmSyncMock } }; +}); + +vi.mock("which", () => ({ + default: { sync: whichSyncMock }, +})); + +import { Effect } from "effect"; +import { findSystemChrome, killChromeProcess } from "../src/chrome-launcher"; +import { ChromeNotFoundError } from "../src/errors"; + +describe("findSystemChrome", () => { + beforeEach(() => { + vi.clearAllMocks(); + existsSyncMock.mockReturnValue(false); + whichSyncMock.mockReturnValue(null); + }); + + it("finds Google Chrome on macOS", async () => { + platformMock.mockReturnValue("darwin"); + existsSyncMock.mockImplementation( + (filePath: string) => + filePath === "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome", + ); + + const result = await Effect.runPromise(findSystemChrome()); + + expect(result).toBe("/Applications/Google Chrome.app/Contents/MacOS/Google Chrome"); + }); + + it("returns first available browser on macOS", async () => { + platformMock.mockReturnValue("darwin"); + existsSyncMock.mockImplementation( + (filePath: string) => + filePath === "/Applications/Brave Browser.app/Contents/MacOS/Brave Browser", + ); + + const result = await Effect.runPromise(findSystemChrome()); + + expect(result).toBe("/Applications/Brave Browser.app/Contents/MacOS/Brave Browser"); + }); + + it("finds Chrome on Linux via which", async () => { + platformMock.mockReturnValue("linux"); + whichSyncMock.mockImplementation((name: string) => + name === "google-chrome" ? "/usr/bin/google-chrome" : null, + ); + + const result = await Effect.runPromise(findSystemChrome()); + + expect(result).toBe("/usr/bin/google-chrome"); + expect(whichSyncMock).toHaveBeenCalledWith("google-chrome", { nothrow: true }); + }); + + it("tries candidates in priority order on Linux", async () => { + platformMock.mockReturnValue("linux"); + whichSyncMock.mockImplementation((name: string) => + name === "chromium" ? "/usr/bin/chromium" : null, + ); + + const result = await Effect.runPromise(findSystemChrome()); + + expect(result).toBe("/usr/bin/chromium"); + expect(whichSyncMock).toHaveBeenCalledWith("google-chrome", { nothrow: true }); + expect(whichSyncMock).toHaveBeenCalledWith("google-chrome-stable", { nothrow: true }); + expect(whichSyncMock).toHaveBeenCalledWith("chromium-browser", { nothrow: true }); + expect(whichSyncMock).toHaveBeenCalledWith("chromium", { nothrow: true }); + }); + + it("fails with ChromeNotFoundError when no browser is found on macOS", async () => { + platformMock.mockReturnValue("darwin"); + + const error = await Effect.runPromise(Effect.flip(findSystemChrome())); + + expect(error).toBeInstanceOf(ChromeNotFoundError); + }); + + it("fails with ChromeNotFoundError when no browser is found on Linux", async () => { + platformMock.mockReturnValue("linux"); + + const error = await Effect.runPromise(Effect.flip(findSystemChrome())); + + expect(error).toBeInstanceOf(ChromeNotFoundError); + }); + + it("fails with ChromeNotFoundError on unsupported platform", async () => { + platformMock.mockReturnValue("freebsd"); + + const error = await Effect.runPromise(Effect.flip(findSystemChrome())); + + expect(error).toBeInstanceOf(ChromeNotFoundError); + }); +}); + +describe("killChromeProcess", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("kills process and removes temp dir", async () => { + const killMock = vi.fn(() => true); + const chrome = { + process: { kill: killMock } as unknown as ChildProcess, + wsUrl: "ws://127.0.0.1:9222/devtools/browser/abc", + userDataDir: "/tmp/expect-chrome-abc123", + tempUserDataDir: "/tmp/expect-chrome-abc123", + }; + + await Effect.runPromise(killChromeProcess(chrome)); + + expect(killMock).toHaveBeenCalledOnce(); + expect(rmSyncMock).toHaveBeenCalledWith("/tmp/expect-chrome-abc123", { + recursive: true, + force: true, + }); + }); + + it("skips temp dir removal when no temp dir exists", async () => { + const killMock = vi.fn(() => true); + const chrome = { + process: { kill: killMock } as unknown as ChildProcess, + wsUrl: "ws://127.0.0.1:9222/devtools/browser/abc", + userDataDir: "/home/user/.config/google-chrome", + tempUserDataDir: undefined, + }; + + await Effect.runPromise(killChromeProcess(chrome)); + + expect(killMock).toHaveBeenCalledOnce(); + expect(rmSyncMock).not.toHaveBeenCalled(); + }); + + it("continues gracefully when process.kill throws", async () => { + const killMock = vi.fn(() => { + throw new Error("No such process"); + }); + const chrome = { + process: { kill: killMock } as unknown as ChildProcess, + wsUrl: "ws://127.0.0.1:9222/devtools/browser/abc", + userDataDir: "/tmp/expect-chrome-abc123", + tempUserDataDir: "/tmp/expect-chrome-abc123", + }; + + await Effect.runPromise(killChromeProcess(chrome)); + + expect(killMock).toHaveBeenCalledOnce(); + expect(rmSyncMock).toHaveBeenCalledOnce(); + }); + + it("continues gracefully when rmSync throws", async () => { + const killMock = vi.fn(() => true); + rmSyncMock.mockImplementation(() => { + throw new Error("EPERM"); + }); + const chrome = { + process: { kill: killMock } as unknown as ChildProcess, + wsUrl: "ws://127.0.0.1:9222/devtools/browser/abc", + userDataDir: "/tmp/expect-chrome-abc123", + tempUserDataDir: "/tmp/expect-chrome-abc123", + }; + + await Effect.runPromise(killChromeProcess(chrome)); + + expect(killMock).toHaveBeenCalledOnce(); + expect(rmSyncMock).toHaveBeenCalledOnce(); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d45db5d9e..40818909e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -310,6 +310,9 @@ importers: rrweb: specifier: ^2.0.0-alpha.18 version: 2.0.0-alpha.20 + which: + specifier: ^6.0.1 + version: 6.0.1 zod: specifier: ^4.3.6 version: 4.3.6 @@ -317,6 +320,9 @@ importers: '@types/node': specifier: ^22.15.0 version: 22.19.15 + '@types/which': + specifier: ^3.0.4 + version: 3.0.4 esbuild: specifier: ^0.25.0 version: 0.25.12 @@ -10539,8 +10545,8 @@ snapshots: '@next/eslint-plugin-next': 16.2.0 eslint: 9.39.4(jiti@2.6.1) eslint-import-resolver-node: 0.3.9 - eslint-import-resolver-typescript: 3.10.1(eslint-plugin-import@2.32.0)(eslint@9.39.4(jiti@2.6.1)) - eslint-plugin-import: 2.32.0(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-typescript@3.10.1)(eslint@9.39.4(jiti@2.6.1)) + eslint-import-resolver-typescript: 3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint@9.39.4(jiti@2.6.1)))(eslint@9.39.4(jiti@2.6.1)) + eslint-plugin-import: 2.32.0(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint@9.39.4(jiti@2.6.1)))(eslint@9.39.4(jiti@2.6.1)))(eslint@9.39.4(jiti@2.6.1)) eslint-plugin-jsx-a11y: 6.10.2(eslint@9.39.4(jiti@2.6.1)) eslint-plugin-react: 7.37.5(eslint@9.39.4(jiti@2.6.1)) eslint-plugin-react-hooks: 7.0.1(eslint@9.39.4(jiti@2.6.1)) @@ -10562,7 +10568,7 @@ snapshots: transitivePeerDependencies: - supports-color - eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0)(eslint@9.39.4(jiti@2.6.1)): + eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint@9.39.4(jiti@2.6.1)))(eslint@9.39.4(jiti@2.6.1)): dependencies: '@nolyfill/is-core-module': 1.0.39 debug: 4.4.3 @@ -10573,22 +10579,22 @@ snapshots: tinyglobby: 0.2.15 unrs-resolver: 1.11.1 optionalDependencies: - eslint-plugin-import: 2.32.0(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-typescript@3.10.1)(eslint@9.39.4(jiti@2.6.1)) + eslint-plugin-import: 2.32.0(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint@9.39.4(jiti@2.6.1)))(eslint@9.39.4(jiti@2.6.1)))(eslint@9.39.4(jiti@2.6.1)) transitivePeerDependencies: - supports-color - eslint-module-utils@2.12.1(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.10.1)(eslint@9.39.4(jiti@2.6.1)): + eslint-module-utils@2.12.1(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint@9.39.4(jiti@2.6.1)))(eslint@9.39.4(jiti@2.6.1)))(eslint@9.39.4(jiti@2.6.1)): dependencies: debug: 3.2.7 optionalDependencies: '@typescript-eslint/parser': 8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3) eslint: 9.39.4(jiti@2.6.1) eslint-import-resolver-node: 0.3.9 - eslint-import-resolver-typescript: 3.10.1(eslint-plugin-import@2.32.0)(eslint@9.39.4(jiti@2.6.1)) + eslint-import-resolver-typescript: 3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint@9.39.4(jiti@2.6.1)))(eslint@9.39.4(jiti@2.6.1)) transitivePeerDependencies: - supports-color - eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-typescript@3.10.1)(eslint@9.39.4(jiti@2.6.1)): + eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint@9.39.4(jiti@2.6.1)))(eslint@9.39.4(jiti@2.6.1)))(eslint@9.39.4(jiti@2.6.1)): dependencies: '@rtsao/scc': 1.1.0 array-includes: 3.1.9 @@ -10599,7 +10605,7 @@ snapshots: doctrine: 2.1.0 eslint: 9.39.4(jiti@2.6.1) eslint-import-resolver-node: 0.3.9 - eslint-module-utils: 2.12.1(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.10.1)(eslint@9.39.4(jiti@2.6.1)) + eslint-module-utils: 2.12.1(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.57.2(eslint@9.39.4(jiti@2.6.1))(typescript@5.9.3))(eslint@9.39.4(jiti@2.6.1)))(eslint@9.39.4(jiti@2.6.1)))(eslint@9.39.4(jiti@2.6.1)) hasown: 2.0.2 is-core-module: 2.16.1 is-glob: 4.0.3 From 0e7de5afa2b1b7648027c3791ca7597cc295517a Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Wed, 1 Apr 2026 20:10:52 -0700 Subject: [PATCH 03/15] fix: address PR review issues from bugbot - Fix systemChrome suffix showing when CDP URL was provided (system Chrome is ignored in that case) - Add cleanup finalizer to Effect.callback polling loop in chrome-launcher to prevent setTimeout leak on fiber interruption - Add Effect.catchCause error handling to analytics trackEvent/flush to prevent unhandled promise rejections from fire-and-forget calls --- apps/cli/src/utils/session-analytics.ts | 8 ++++++-- packages/browser/src/chrome-launcher.ts | 7 ++++++- packages/browser/src/mcp/server.ts | 2 +- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/apps/cli/src/utils/session-analytics.ts b/apps/cli/src/utils/session-analytics.ts index 7d3a4fd88..17dd7bbf9 100644 --- a/apps/cli/src/utils/session-analytics.ts +++ b/apps/cli/src/utils/session-analytics.ts @@ -17,7 +17,9 @@ export const trackEvent = ( ...(properties !== undefined ? [properties] : []), ); yield* captureEffect; - }), + }).pipe( + Effect.catchCause((cause) => Effect.logDebug("Analytics capture failed", { eventName, cause })), + ), ); export const trackSessionStarted = () => @@ -35,5 +37,7 @@ export const flushSession = (sessionStartedAt: number) => session_ms: Date.now() - sessionStartedAt, }); yield* analytics.flush; - }), + }).pipe( + Effect.catchCause((cause) => Effect.logDebug("Analytics flush failed", { cause })), + ), ); diff --git a/packages/browser/src/chrome-launcher.ts b/packages/browser/src/chrome-launcher.ts index a6f25fff1..de2620431 100644 --- a/packages/browser/src/chrome-launcher.ts +++ b/packages/browser/src/chrome-launcher.ts @@ -202,6 +202,7 @@ export const launchSystemChrome = Effect.fn("launchSystemChrome")(function* (opt const wsUrl = yield* Effect.callback((resume) => { const deadline = Date.now() + CDP_LAUNCH_TIMEOUT_MS; + let pendingTimer: ReturnType | undefined; const poll = () => { if (Date.now() > deadline) { @@ -234,10 +235,14 @@ export const launchSystemChrome = Effect.fn("launchSystemChrome")(function* (opt return; } - setTimeout(poll, CDP_POLL_INTERVAL_MS); + pendingTimer = setTimeout(poll, CDP_POLL_INTERVAL_MS); }; poll(); + + return Effect.sync(() => { + if (pendingTimer !== undefined) clearTimeout(pendingTimer); + }); }).pipe(Effect.tapError(() => cleanupFailedLaunch(child, tempDir))); yield* Effect.logInfo("System Chrome launched, CDP available", { wsUrl }); diff --git a/packages/browser/src/mcp/server.ts b/packages/browser/src/mcp/server.ts index e79d98099..c22e2cfbd 100644 --- a/packages/browser/src/mcp/server.ts +++ b/packages/browser/src/mcp/server.ts @@ -115,7 +115,7 @@ export const createBrowserMcpServer = ( }); const engineSuffix = browserType && browserType !== "chromium" ? ` [${browserType}]` : ""; const cdpSuffix = cdpUrl ? ` (connected via CDP: ${cdpUrl})` : ""; - const chromeSuffix = systemChrome ? " (system Chrome)" : ""; + const chromeSuffix = systemChrome && !cdpUrl ? " (system Chrome)" : ""; return textResult( `Opened ${url}${engineSuffix}${cdpSuffix}${chromeSuffix}` + (result.injectedCookieCount > 0 From cbde4b74de2b5243e34520e667fc6104782e1fde Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Wed, 1 Apr 2026 20:26:29 -0700 Subject: [PATCH 04/15] fix: add Windows Edge paths to chrome-launcher discovery Add Microsoft Edge executable paths (LOCALAPPDATA, PROGRAMFILES, PROGRAMFILES(X86)) to Windows system Chrome discovery, matching parity with macOS and Linux which already include Edge. --- apps/cli/src/utils/session-analytics.ts | 8 ++++---- packages/browser/src/chrome-launcher.ts | 4 ++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/apps/cli/src/utils/session-analytics.ts b/apps/cli/src/utils/session-analytics.ts index 17dd7bbf9..2d7bc58a9 100644 --- a/apps/cli/src/utils/session-analytics.ts +++ b/apps/cli/src/utils/session-analytics.ts @@ -18,7 +18,9 @@ export const trackEvent = ( ); yield* captureEffect; }).pipe( - Effect.catchCause((cause) => Effect.logDebug("Analytics capture failed", { eventName, cause })), + Effect.catchCause((cause) => + Effect.logDebug("Analytics capture failed", { eventName, cause }), + ), ), ); @@ -37,7 +39,5 @@ export const flushSession = (sessionStartedAt: number) => session_ms: Date.now() - sessionStartedAt, }); yield* analytics.flush; - }).pipe( - Effect.catchCause((cause) => Effect.logDebug("Analytics flush failed", { cause })), - ), + }).pipe(Effect.catchCause((cause) => Effect.logDebug("Analytics flush failed", { cause }))), ); diff --git a/packages/browser/src/chrome-launcher.ts b/packages/browser/src/chrome-launcher.ts index de2620431..96f7fd910 100644 --- a/packages/browser/src/chrome-launcher.ts +++ b/packages/browser/src/chrome-launcher.ts @@ -71,9 +71,13 @@ export const findSystemChrome = Effect.fn("findSystemChrome")(function* () { localAppData && path.join(localAppData, "Google", "Chrome", "Application", "chrome.exe"), localAppData && path.join(localAppData, "BraveSoftware", "Brave-Browser", "Application", "brave.exe"), + localAppData && path.join(localAppData, "Microsoft", "Edge", "Application", "msedge.exe"), programFiles && path.join(programFiles, "Google", "Chrome", "Application", "chrome.exe"), programFilesX86 && path.join(programFilesX86, "Google", "Chrome", "Application", "chrome.exe"), + programFiles && path.join(programFiles, "Microsoft", "Edge", "Application", "msedge.exe"), + programFilesX86 && + path.join(programFilesX86, "Microsoft", "Edge", "Application", "msedge.exe"), ].filter(Boolean) as string[]; for (const candidate of candidates) { From 123ec36bd6da49c9bc0f8c7960dbb883cfda16e5 Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Wed, 1 Apr 2026 20:42:29 -0700 Subject: [PATCH 05/15] fix: close Playwright browser on error for system Chrome The error cleanup path skipped browser.close() whenever cdpEndpoint was truthy, which was designed for user-provided CDP endpoints (externally managed Chrome). System Chrome sets cdpEndpoint from its own launch, so the Playwright CDP client was never released on failure. Now only skip browser.close() for user-provided CDP (when no chromeProcess). --- packages/browser/src/browser.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser/src/browser.ts b/packages/browser/src/browser.ts index 68e607012..e465ef2f9 100644 --- a/packages/browser/src/browser.ts +++ b/packages/browser/src/browser.ts @@ -329,7 +329,7 @@ export class Browser extends ServiceMap.Service()("@browser/Browser", { Effect.tapError(() => cleanup.pipe( Effect.andThen(() => { - if (cdpEndpoint) return Effect.void; + if (cdpEndpoint && !chromeProcess) return Effect.void; return Effect.tryPromise(() => browser.close()).pipe( Effect.catchTag("UnknownError", () => Effect.void), ); From 03d098b6e3ed5a1c30fde9f5f4ad9262622c9952 Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Wed, 1 Apr 2026 23:14:09 -0700 Subject: [PATCH 06/15] fix: add testTimeout to browser E2E tests The @expect/browser package had no testTimeout config, defaulting to 5000ms. Cross-browser E2E tests (webkit, firefox launches) and cookie injection tests exceed this on Windows CI. Set testTimeout: 0 to match the project convention documented in AGENTS.md. --- packages/browser/vite.config.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/browser/vite.config.ts b/packages/browser/vite.config.ts index 5a74a86b4..fba575c3f 100644 --- a/packages/browser/vite.config.ts +++ b/packages/browser/vite.config.ts @@ -7,4 +7,7 @@ export default defineConfig({ dts: true, sourcemap: true, }, + test: { + testTimeout: 0, + }, }); From b30eee0394a63a716a648525fddb831796a86ba1 Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Wed, 1 Apr 2026 23:16:06 -0700 Subject: [PATCH 07/15] 0.0.23 --- apps/website/app/llms.txt/route.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/apps/website/app/llms.txt/route.ts b/apps/website/app/llms.txt/route.ts index 4dae41ea7..8845b26fc 100644 --- a/apps/website/app/llms.txt/route.ts +++ b/apps/website/app/llms.txt/route.ts @@ -2,13 +2,21 @@ import { readFileSync } from "fs"; import { NextResponse } from "next/server"; import { join } from "path"; +const root = join(process.cwd(), "..", ".."); + +const readme = readFileSync(join(root, "README.md"), "utf-8") + .replace(/^# ]*\/>\s*/m, "# ") + .replace(/\[!\[.*?\]\(.*?\)\]\(.*?\)\n?/g, ""); + const skill = readFileSync( - join(process.cwd(), "..", "..", "packages", "expect-skill", "SKILL.md"), + join(root, "packages", "expect-skill", "SKILL.md"), "utf-8", ).replace(/^---[\s\S]*?---\n+/, ""); +const content = `${readme.trim()}\n\n---\n\n${skill.trim()}\n`; + export const GET = () => - new NextResponse(skill, { + new NextResponse(content, { headers: { "Content-Type": "text/markdown; charset=utf-8", "Cache-Control": "public, max-age=86400", From 71ed39fcc246cd65a77729e77c6cbc78af42ee3f Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Wed, 1 Apr 2026 23:21:08 -0700 Subject: [PATCH 08/15] fix --- apps/cli/src/utils/session-analytics.ts | 15 +++++++------- packages/shared/src/analytics/analytics.ts | 23 ++++++++-------------- 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/apps/cli/src/utils/session-analytics.ts b/apps/cli/src/utils/session-analytics.ts index 2d7bc58a9..f7c857254 100644 --- a/apps/cli/src/utils/session-analytics.ts +++ b/apps/cli/src/utils/session-analytics.ts @@ -1,14 +1,14 @@ -import { Effect, ManagedRuntime } from "effect"; +import { Effect } from "effect"; import { Analytics, type EventMap } from "@expect/shared/observability"; import { usePreferencesStore } from "../stores/use-preferences"; -const analyticsRuntime = ManagedRuntime.make(Analytics.layerPostHog); +const analyticsLayer = Analytics.layerPostHog; export const trackEvent = ( eventName: K, ...[properties]: EventMap[K] extends undefined ? [] : [EventMap[K]] ) => - analyticsRuntime.runPromise( + Effect.runPromise( Effect.gen(function* () { const analytics = yield* Analytics; const captureEffect: Effect.Effect = (analytics.capture as Function).call( @@ -18,9 +18,8 @@ export const trackEvent = ( ); yield* captureEffect; }).pipe( - Effect.catchCause((cause) => - Effect.logDebug("Analytics capture failed", { eventName, cause }), - ), + Effect.catchCause(() => Effect.void), + Effect.provide(analyticsLayer), ), ); @@ -32,12 +31,12 @@ export const trackSessionStarted = () => }); export const flushSession = (sessionStartedAt: number) => - analyticsRuntime.runPromise( + Effect.runPromise( Effect.gen(function* () { const analytics = yield* Analytics; yield* analytics.capture("session:ended", { session_ms: Date.now() - sessionStartedAt, }); yield* analytics.flush; - }).pipe(Effect.catchCause((cause) => Effect.logDebug("Analytics flush failed", { cause }))), + }).pipe(Effect.provide(analyticsLayer)), ); diff --git a/packages/shared/src/analytics/analytics.ts b/packages/shared/src/analytics/analytics.ts index 3efd542b5..6fc66a547 100644 --- a/packages/shared/src/analytics/analytics.ts +++ b/packages/shared/src/analytics/analytics.ts @@ -37,13 +37,13 @@ export class AnalyticsProvider extends ServiceMap.Service< >()("@expect/AnalyticsProvider") { static layerPostHog = Layer.succeed(this)({ capture: (event) => - Effect.tryPromise(() => + Effect.sync(() => { posthogClient.captureImmediate({ event: event.eventName, properties: event.properties, distinctId: event.distinctId, - }), - ), + }); + }), identify: (params) => Effect.sync(() => { posthogClient.identify({ @@ -54,7 +54,10 @@ export class AnalyticsProvider extends ServiceMap.Service< }, }); }), - flush: Effect.tryPromise(() => posthogClient.flush()), + flush: Effect.tryPromise({ + try: () => posthogClient.flush(), + catch: (cause) => cause, + }).pipe(Effect.ignore), }); static layerDev = Layer.succeed(this)({ @@ -155,17 +158,7 @@ export class Analytics extends ServiceMap.Service()("@expect/Analytic ); })) as never; - const flush = telemetryDisabled - ? Effect.void - : provider.flush.pipe( - Effect.catchCause((cause) => - Effect.logWarning("Analytics flush failed", { cause }).pipe( - Effect.annotateLogs({ module: "Analytics" }), - ), - ), - ); - - return { capture, track, flush } as const; + return { capture, track, flush: telemetryDisabled ? Effect.void : provider.flush } as const; }), }) { static layerPostHog = Layer.effect(this)(this.make).pipe( From 2b761ab0412e830393d8e07ca576232d6a936b8d Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Wed, 1 Apr 2026 23:26:38 -0700 Subject: [PATCH 09/15] fix --- apps/cli/src/index.tsx | 4 ++-- packages/shared/package.json | 3 ++- packages/shared/src/analytics/analytics.ts | 2 +- packages/shared/src/machine-id.ts | 10 ++++++++++ 4 files changed, 15 insertions(+), 4 deletions(-) create mode 100644 packages/shared/src/machine-id.ts diff --git a/apps/cli/src/index.tsx b/apps/cli/src/index.tsx index 606a2c661..12bd05c7b 100644 --- a/apps/cli/src/index.tsx +++ b/apps/cli/src/index.tsx @@ -19,11 +19,11 @@ import { renderApp } from "./program"; import { CI_EXECUTION_TIMEOUT_MS, VERSION, VERSION_API_URL } from "./constants"; import { prompts } from "./utils/prompts"; import { highlighter } from "./utils/highlighter"; +import { machineId } from "@expect/shared/machine-id"; import { logger } from "./utils/logger"; if (!isRunningInAgent() && !isHeadless()) { - import("node-machine-id") - .then((module) => module.machineId()) + machineId() .catch(() => "unknown") .then((mid) => { fetch(`${VERSION_API_URL}?source=cli&mid=${mid}&t=${Date.now()}`).catch(() => {}); diff --git a/packages/shared/package.json b/packages/shared/package.json index 46c4b0588..b4dce97c2 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -10,7 +10,8 @@ "./observability": "./src/observability/exports.ts", "./utils": "./src/utils.ts", "./launched-from": "./src/launched-from.ts", - "./is-command-available": "./src/is-command-available.ts" + "./is-command-available": "./src/is-command-available.ts", + "./machine-id": "./src/machine-id.ts" }, "scripts": { "lint": "vp lint && tsc --noEmit", diff --git a/packages/shared/src/analytics/analytics.ts b/packages/shared/src/analytics/analytics.ts index 6fc66a547..331688248 100644 --- a/packages/shared/src/analytics/analytics.ts +++ b/packages/shared/src/analytics/analytics.ts @@ -1,6 +1,6 @@ import { Config, Effect, Layer, Option, ServiceMap } from "effect"; import * as NodeServices from "@effect/platform-node/NodeServices"; -import { machineId } from "node-machine-id"; +import { machineId } from "../machine-id"; import { hash } from "ohash"; import { PostHog } from "posthog-node"; diff --git a/packages/shared/src/machine-id.ts b/packages/shared/src/machine-id.ts new file mode 100644 index 000000000..8cfdbd19b --- /dev/null +++ b/packages/shared/src/machine-id.ts @@ -0,0 +1,10 @@ +import { machineId as getRawMachineId } from "node-machine-id"; + +let cached: Promise | undefined; + +export const machineId = (): Promise => { + if (!cached) { + cached = getRawMachineId(); + } + return cached; +}; From d4d2419e3f340e0c43b94dadf9c7a70e274b2e80 Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Wed, 1 Apr 2026 23:29:59 -0700 Subject: [PATCH 10/15] fix: address PR review feedback - Early return in Analytics when telemetry disabled (skip machineId, capture, track) - Replace ternary with early return in Browser.createPage - Use async fs (fs.promises) in chrome-launcher instead of sync - Namespace fs/path imports in llms.txt route --- apps/website/app/llms.txt/route.ts | 16 ++++++------- packages/browser/src/browser.ts | 28 +++++++++++----------- packages/browser/src/chrome-launcher.ts | 13 ++++++---- packages/shared/src/analytics/analytics.ts | 14 ++++++----- 4 files changed, 39 insertions(+), 32 deletions(-) diff --git a/apps/website/app/llms.txt/route.ts b/apps/website/app/llms.txt/route.ts index 8845b26fc..2b0b451e2 100644 --- a/apps/website/app/llms.txt/route.ts +++ b/apps/website/app/llms.txt/route.ts @@ -1,17 +1,17 @@ -import { readFileSync } from "fs"; +import * as fs from "fs"; import { NextResponse } from "next/server"; -import { join } from "path"; +import * as path from "path"; -const root = join(process.cwd(), "..", ".."); +const root = path.join(process.cwd(), "..", ".."); -const readme = readFileSync(join(root, "README.md"), "utf-8") +const readme = fs + .readFileSync(path.join(root, "README.md"), "utf-8") .replace(/^# ]*\/>\s*/m, "# ") .replace(/\[!\[.*?\]\(.*?\)\]\(.*?\)\n?/g, ""); -const skill = readFileSync( - join(root, "packages", "expect-skill", "SKILL.md"), - "utf-8", -).replace(/^---[\s\S]*?---\n+/, ""); +const skill = fs + .readFileSync(path.join(root, "packages", "expect-skill", "SKILL.md"), "utf-8") + .replace(/^---[\s\S]*?---\n+/, ""); const content = `${readme.trim()}\n\n---\n\n${skill.trim()}\n`; diff --git a/packages/browser/src/browser.ts b/packages/browser/src/browser.ts index e465ef2f9..432bf7e3f 100644 --- a/packages/browser/src/browser.ts +++ b/packages/browser/src/browser.ts @@ -203,20 +203,20 @@ export class Browser extends ServiceMap.Service()("@browser/Browser", { browserType: engine, }); - const chromeProcess = - useSystemChrome && !options.cdpUrl - ? yield* launchSystemChrome({ - headless: !options.headed, - }).pipe( - Effect.tap((launched) => - Effect.logInfo("Connected to system Chrome via CDP", { wsUrl: launched.wsUrl }), - ), - Effect.catchTags({ - ChromeNotFoundError: Effect.die, - ChromeLaunchTimeoutError: Effect.die, - }), - ) - : undefined; + const chromeProcess = yield* Effect.gen(function* () { + if (!useSystemChrome || options.cdpUrl) return undefined; + return yield* launchSystemChrome({ + headless: !options.headed, + }).pipe( + Effect.tap((launched) => + Effect.logInfo("Connected to system Chrome via CDP", { wsUrl: launched.wsUrl }), + ), + Effect.catchTags({ + ChromeNotFoundError: Effect.die, + ChromeLaunchTimeoutError: Effect.die, + }), + ); + }); const cdpEndpoint = chromeProcess?.wsUrl ?? (engine === "chromium" ? options.cdpUrl : undefined); diff --git a/packages/browser/src/chrome-launcher.ts b/packages/browser/src/chrome-launcher.ts index 96f7fd910..05c9fb657 100644 --- a/packages/browser/src/chrome-launcher.ts +++ b/packages/browser/src/chrome-launcher.ts @@ -183,13 +183,18 @@ export const launchSystemChrome = Effect.fn("launchSystemChrome")(function* (opt }) { const chromePath = yield* findSystemChrome(); - const tempDir = options.profilePath - ? undefined - : fs.mkdtempSync(path.join(os.tmpdir(), "expect-chrome-")); + let tempDir: string | undefined; + if (!options.profilePath) { + tempDir = yield* Effect.tryPromise(() => + fs.promises.mkdtemp(path.join(os.tmpdir(), "expect-chrome-")), + ).pipe(Effect.catchTag("UnknownError", Effect.die)); + } const userDataDir = options.profilePath ?? tempDir!; - fs.rmSync(path.join(userDataDir, "DevToolsActivePort"), { force: true }); + yield* Effect.tryPromise(() => + fs.promises.rm(path.join(userDataDir, "DevToolsActivePort"), { force: true }), + ).pipe(Effect.catchTag("UnknownError", () => Effect.void)); const args = buildLaunchArgs({ headless: options.headless, userDataDir }); diff --git a/packages/shared/src/analytics/analytics.ts b/packages/shared/src/analytics/analytics.ts index 331688248..8ff0cd0b5 100644 --- a/packages/shared/src/analytics/analytics.ts +++ b/packages/shared/src/analytics/analytics.ts @@ -98,12 +98,15 @@ export class Analytics extends ServiceMap.Service()("@expect/Analytic }) || githubActionsValue !== ""; + if (telemetryDisabled) { + const capture = (() => Effect.void) as never; + const track = ((() => (self: Effect.Effect) => self) as never); + return { capture, track, flush: Effect.void } as const; + } + const projectId = hash(process.cwd()); - const distinctId = yield* Effect.tryPromise(async () => { - if (telemetryDisabled) return ""; - return machineId(); - }).pipe( + const distinctId = yield* Effect.tryPromise(() => machineId()).pipe( Effect.catchTag("UnknownError", (cause) => Effect.logWarning("Failed to get machine ID, using fallback", { cause }).pipe( Effect.as(globalThis.crypto.randomUUID()), @@ -116,7 +119,6 @@ export class Analytics extends ServiceMap.Service()("@expect/Analytic ...[properties]: EventMap[K] extends undefined ? [] : [EventMap[K]] ) => Effect.gen(function* () { - if (telemetryDisabled) return; const commonProperties = { timestamp: new Date().toISOString(), projectId, @@ -158,7 +160,7 @@ export class Analytics extends ServiceMap.Service()("@expect/Analytic ); })) as never; - return { capture, track, flush: telemetryDisabled ? Effect.void : provider.flush } as const; + return { capture, track, flush: provider.flush } as const; }), }) { static layerPostHog = Layer.effect(this)(this.make).pipe( From 1fb73f034d56442a703411da0d19155e51aaa921 Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Thu, 2 Apr 2026 00:27:18 -0700 Subject: [PATCH 11/15] fix --- packages/browser/src/browser.ts | 100 +++++++---- packages/browser/src/cdp-discovery.ts | 4 +- packages/browser/src/chrome-launcher.ts | 1 + packages/browser/src/mcp/mcp-session.ts | 20 ++- packages/browser/src/mcp/server.ts | 21 +-- packages/browser/tests/create-page.test.ts | 185 +++++++++++++++++++++ 6 files changed, 275 insertions(+), 56 deletions(-) diff --git a/packages/browser/src/browser.ts b/packages/browser/src/browser.ts index 432bf7e3f..0c75acaef 100644 --- a/packages/browser/src/browser.ts +++ b/packages/browser/src/browser.ts @@ -7,6 +7,7 @@ import { Array as Arr, Effect, Layer, Option, ServiceMap } from "effect"; const cookiesLayer = Layer.mergeAll(layerLive, Cookies.layer); import { + CDP_DISCOVERY_TIMEOUT_MS, CONTENT_ROLES, HEADLESS_CHROMIUM_ARGS, INTERACTIVE_ROLES, @@ -25,6 +26,7 @@ import { SnapshotTimeoutError, } from "./errors"; import { launchSystemChrome, killChromeProcess } from "./chrome-launcher"; +import { autoDiscoverCdp } from "./cdp-discovery"; import { toActionError } from "./utils/action-error"; import { compactTree } from "./utils/compact-tree"; import { createLocator } from "./utils/create-locator"; @@ -203,44 +205,72 @@ export class Browser extends ServiceMap.Service()("@browser/Browser", { browserType: engine, }); - const chromeProcess = yield* Effect.gen(function* () { - if (!useSystemChrome || options.cdpUrl) return undefined; - return yield* launchSystemChrome({ - headless: !options.headed, - }).pipe( - Effect.tap((launched) => - Effect.logInfo("Connected to system Chrome via CDP", { wsUrl: launched.wsUrl }), + const resolvedChrome = yield* Effect.gen(function* () { + if (!useSystemChrome || options.cdpUrl) + return { _tag: "none" as const }; + + const discovered = yield* autoDiscoverCdp().pipe( + Effect.map(Option.some), + Effect.catchTag("CdpDiscoveryError", () => Effect.succeed(Option.none())), + ); + + if (Option.isSome(discovered)) { + const connectedBrowser = yield* Effect.tryPromise(() => + chromium.connectOverCDP(discovered.value, { timeout: CDP_DISCOVERY_TIMEOUT_MS }), + ).pipe( + Effect.map(Option.some), + Effect.catchTag("UnknownError", () => Effect.succeed(Option.none())), + ); + + if (Option.isSome(connectedBrowser)) { + yield* Effect.logInfo("Connected to live Chrome", { wsUrl: discovered.value }); + return { _tag: "discovered" as const, browser: connectedBrowser.value }; + } + + yield* Effect.logDebug("Auto-discovered CDP endpoint failed to connect, launching new Chrome", { + wsUrl: discovered.value, + }); + } + + const launched = yield* launchSystemChrome({ headless: false }).pipe( + Effect.tap((process) => + Effect.logInfo("Launched system Chrome via CDP", { wsUrl: process.wsUrl }), ), Effect.catchTags({ ChromeNotFoundError: Effect.die, ChromeLaunchTimeoutError: Effect.die, }), ); + return { _tag: "launched" as const, process: launched }; }); + const chromeProcess = resolvedChrome._tag === "launched" ? resolvedChrome.process : undefined; + const isExternalBrowser = resolvedChrome._tag === "discovered"; const cdpEndpoint = chromeProcess?.wsUrl ?? (engine === "chromium" ? options.cdpUrl : undefined); const cleanup = chromeProcess ? killChromeProcess(chromeProcess) : Effect.void; const browserType = resolveBrowserType(engine); - const browser = cdpEndpoint - ? yield* Effect.tryPromise({ - try: () => chromium.connectOverCDP(cdpEndpoint), - catch: (cause) => - new CdpConnectionError({ - endpointUrl: cdpEndpoint, - cause: cause instanceof Error ? cause.message : String(cause), - }), - }) - : yield* Effect.tryPromise({ - try: () => - browserType.launch({ - headless: !options.headed, - executablePath: options.executablePath, - args: engine === "chromium" && !options.headed ? HEADLESS_CHROMIUM_ARGS : [], - }), - catch: toBrowserLaunchError, - }); + const browser = resolvedChrome._tag === "discovered" + ? resolvedChrome.browser + : cdpEndpoint + ? yield* Effect.tryPromise({ + try: () => chromium.connectOverCDP(cdpEndpoint), + catch: (cause) => + new CdpConnectionError({ + endpointUrl: cdpEndpoint, + cause: cause instanceof Error ? cause.message : String(cause), + }), + }) + : yield* Effect.tryPromise({ + try: () => + browserType.launch({ + headless: !options.headed, + executablePath: options.executablePath, + args: engine === "chromium" && !options.headed ? HEADLESS_CHROMIUM_ARGS : [], + }), + catch: toBrowserLaunchError, + }); const setupPage = Effect.gen(function* () { const defaultBrowserContext = @@ -264,7 +294,7 @@ export class Browser extends ServiceMap.Service()("@browser/Browser", { }; } - const isCdpConnected = Boolean(cdpEndpoint); + const isCdpConnected = isExternalBrowser || Boolean(cdpEndpoint); const existingContexts = isCdpConnected ? browser.contexts() : []; const context = existingContexts.length > 0 @@ -303,13 +333,13 @@ export class Browser extends ServiceMap.Service()("@browser/Browser", { } const existingPages = context.pages(); - const page = - isCdpConnected && existingPages.length > 0 - ? existingPages[0]! - : yield* Effect.tryPromise({ - try: () => context.newPage(), - catch: toBrowserLaunchError, - }); + const reuseExistingPage = isCdpConnected && existingPages.length > 0 && !isExternalBrowser; + const page = reuseExistingPage + ? existingPages[0]! + : yield* Effect.tryPromise({ + try: () => context.newPage(), + catch: toBrowserLaunchError, + }); if (url) { yield* Effect.tryPromise({ @@ -322,14 +352,14 @@ export class Browser extends ServiceMap.Service()("@browser/Browser", { }); } - return { browser, context, page, cleanup }; + return { browser, context, page, cleanup, isExternalBrowser }; }); return yield* setupPage.pipe( Effect.tapError(() => cleanup.pipe( Effect.andThen(() => { - if (cdpEndpoint && !chromeProcess) return Effect.void; + if (isExternalBrowser) return Effect.void; return Effect.tryPromise(() => browser.close()).pipe( Effect.catchTag("UnknownError", () => Effect.void), ); diff --git a/packages/browser/src/cdp-discovery.ts b/packages/browser/src/cdp-discovery.ts index da4dc0467..714eed474 100644 --- a/packages/browser/src/cdp-discovery.ts +++ b/packages/browser/src/cdp-discovery.ts @@ -105,9 +105,6 @@ export const discoverCdpUrl = Effect.fn("discoverCdpUrl")(function* (host: strin const listResult = yield* tryDiscover(discoverViaJsonList(host, port)); if (Option.isSome(listResult)) return listResult.value; - const reachable = yield* isPortReachable(host, port); - if (reachable) return `ws://${host}:${port}/devtools/browser`; - return yield* new CdpDiscoveryError({ cause: `All CDP discovery methods failed for ${host}:${port}`, }); @@ -126,6 +123,7 @@ const getChromeUserDataDirs = () => { path.join(base, "BraveSoftware", "Brave-Browser"), path.join(base, "Microsoft Edge"), path.join(base, "Arc", "User Data"), + path.join(base, "net.imput.helium"), ]; } diff --git a/packages/browser/src/chrome-launcher.ts b/packages/browser/src/chrome-launcher.ts index 05c9fb657..00ca84f22 100644 --- a/packages/browser/src/chrome-launcher.ts +++ b/packages/browser/src/chrome-launcher.ts @@ -26,6 +26,7 @@ const SYSTEM_CHROME_PATHS_DARWIN = [ "/Applications/Brave Browser.app/Contents/MacOS/Brave Browser", "/Applications/Microsoft Edge.app/Contents/MacOS/Microsoft Edge", "/Applications/Arc.app/Contents/MacOS/Arc", + "/Applications/Helium.app/Contents/MacOS/Helium", ] as const; const SYSTEM_CHROME_NAMES_LINUX = [ diff --git a/packages/browser/src/mcp/mcp-session.ts b/packages/browser/src/mcp/mcp-session.ts index 27f55d372..68cf4a481 100644 --- a/packages/browser/src/mcp/mcp-session.ts +++ b/packages/browser/src/mcp/mcp-session.ts @@ -45,6 +45,7 @@ export interface BrowserSessionData { readonly context: BrowserContext; readonly page: Page; readonly cleanup: Effect.Effect; + readonly isExternalBrowser: boolean; readonly consoleMessages: ConsoleEntry[]; readonly networkRequests: NetworkEntry[]; readonly replayOutputPath: string | undefined; @@ -64,6 +65,7 @@ export interface OpenOptions { export interface OpenResult { readonly injectedCookieCount: number; + readonly isExternalBrowser: boolean; } export interface CloseResult { @@ -235,6 +237,7 @@ export class McpSession extends ServiceMap.Service()("@browser/McpSe context: pageResult.context, page: pageResult.page, cleanup: pageResult.cleanup, + isExternalBrowser: pageResult.isExternalBrowser, consoleMessages: [], networkRequests: [], replayOutputPath: Option.getOrUndefined(replayOutputPath), @@ -305,7 +308,10 @@ export class McpSession extends ServiceMap.Service()("@browser/McpSe ), ); - return { injectedCookieCount } satisfies OpenResult; + return { + injectedCookieCount, + isExternalBrowser: pageResult.isExternalBrowser, + } satisfies OpenResult; }); const snapshot = Effect.fn("McpSession.snapshot")(function* ( @@ -512,9 +518,15 @@ export class McpSession extends ServiceMap.Service()("@browser/McpSe Effect.catchCause((cause) => Effect.logDebug("Failed during close cleanup", { cause })), ); - yield* Effect.tryPromise(() => activeSession.browser.close()).pipe( - Effect.catchCause((cause) => Effect.logDebug("Failed to close browser", { cause })), - ); + if (activeSession.isExternalBrowser) { + yield* Effect.tryPromise(() => activeSession.page.close()).pipe( + Effect.catchCause((cause) => Effect.logDebug("Failed to close page", { cause })), + ); + } else { + yield* Effect.tryPromise(() => activeSession.browser.close()).pipe( + Effect.catchCause((cause) => Effect.logDebug("Failed to close browser", { cause })), + ); + } yield* activeSession.cleanup.pipe( Effect.catchCause((cause) => diff --git a/packages/browser/src/mcp/server.ts b/packages/browser/src/mcp/server.ts index c22e2cfbd..c64ba8738 100644 --- a/packages/browser/src/mcp/server.ts +++ b/packages/browser/src/mcp/server.ts @@ -8,7 +8,6 @@ import { evaluateRuntime } from "../utils/evaluate-runtime"; import { runAccessibilityAudit } from "../accessibility"; import { formatPerformanceTrace } from "../performance-trace"; import { McpSession } from "./mcp-session"; -import { autoDiscoverCdp } from "../cdp-discovery"; import { DUPLICATE_REQUEST_WINDOW_MS } from "./constants"; const textResult = (text: string) => ({ @@ -71,7 +70,7 @@ export const createBrowserMcpServer = ( .string() .optional() .describe( - "CDP WebSocket endpoint URL to connect to an existing Chrome instance (e.g. 'ws://localhost:9222/devtools/browser/...'). Use 'auto' to auto-discover a running Chrome.", + "CDP WebSocket endpoint URL to connect to an existing Chrome instance (e.g. 'ws://localhost:9222/devtools/browser/...').", ), browser: z .enum(["chromium", "webkit", "firefox"]) @@ -83,7 +82,7 @@ export const createBrowserMcpServer = ( .boolean() .optional() .describe( - "Launch the system-installed Chrome (Google Chrome, Brave, Edge) instead of Playwright's bundled Chromium. Connects via CDP. Useful for testing with the real browser the user has installed. Ignored when 'cdp' is also provided.", + "Use the system-installed Chrome (Google Chrome, Brave, Edge) instead of Playwright's bundled Chromium. First tries to connect to an already-running Chrome with remote debugging enabled; if none is found, launches a new headed instance. Ignored when 'cdp' is provided.", ), }, }, @@ -97,25 +96,19 @@ export const createBrowserMcpServer = ( return textResult(`Navigated to ${url}`); } - let cdpUrl: string | undefined; - if (cdp === "auto") { - cdpUrl = yield* autoDiscoverCdp(); - yield* Effect.logInfo("Auto-discovered CDP endpoint", { cdpUrl }); - } else if (cdp) { - cdpUrl = cdp; - } - const result = yield* session.open(url, { headed, cookies, waitUntil, - cdpUrl, + cdpUrl: cdp, browserType, systemChrome, }); const engineSuffix = browserType && browserType !== "chromium" ? ` [${browserType}]` : ""; - const cdpSuffix = cdpUrl ? ` (connected via CDP: ${cdpUrl})` : ""; - const chromeSuffix = systemChrome && !cdpUrl ? " (system Chrome)" : ""; + const cdpSuffix = cdp ? ` (connected via CDP: ${cdp})` : ""; + const chromeSuffix = systemChrome + ? (result.isExternalBrowser ? " (live Chrome)" : " (system Chrome)") + : ""; return textResult( `Opened ${url}${engineSuffix}${cdpSuffix}${chromeSuffix}` + (result.injectedCookieCount > 0 diff --git a/packages/browser/tests/create-page.test.ts b/packages/browser/tests/create-page.test.ts index 1501e4619..3df524aba 100644 --- a/packages/browser/tests/create-page.test.ts +++ b/packages/browser/tests/create-page.test.ts @@ -5,6 +5,7 @@ const { browserListMock, cookieExtractMock, launchMock, + connectOverCDPMock, newContextMock, addCookiesMock, addInitScriptMock, @@ -12,11 +13,15 @@ const { newPageMock, gotoMock, closeMock, + autoDiscoverCdpMock, + launchSystemChromeMock, + killChromeProcessMock, } = vi.hoisted(() => ({ defaultBrowserMock: vi.fn(), browserListMock: vi.fn(), cookieExtractMock: vi.fn(), launchMock: vi.fn(), + connectOverCDPMock: vi.fn(), newContextMock: vi.fn(), addCookiesMock: vi.fn(), addInitScriptMock: vi.fn(), @@ -24,6 +29,9 @@ const { newPageMock: vi.fn(), gotoMock: vi.fn(), closeMock: vi.fn(), + autoDiscoverCdpMock: vi.fn(), + launchSystemChromeMock: vi.fn(), + killChromeProcessMock: vi.fn(), })); vi.mock("@expect/cookies", async () => { @@ -58,6 +66,7 @@ const firefoxLaunchMock = vi.hoisted(() => vi.fn()); vi.mock("playwright", () => ({ chromium: { launch: launchMock, + connectOverCDP: connectOverCDPMock, }, webkit: { launch: webkitLaunchMock, @@ -67,6 +76,15 @@ vi.mock("playwright", () => ({ }, })); +vi.mock("../src/cdp-discovery", () => ({ + autoDiscoverCdp: autoDiscoverCdpMock, +})); + +vi.mock("../src/chrome-launcher", () => ({ + launchSystemChrome: launchSystemChromeMock, + killChromeProcess: killChromeProcessMock, +})); + import { Effect, Option } from "effect"; import { runBrowser } from "../src/browser"; @@ -309,3 +327,170 @@ describe("Browser.createPage browserType", () => { expect(webkitLaunchMock).toHaveBeenCalledWith(expect.objectContaining({ args: [] })); }); }); + +describe("Browser.createPage systemChrome", () => { + const existingPage = { goto: vi.fn().mockResolvedValue(undefined), evaluate: vi.fn().mockResolvedValue(undefined) }; + + beforeEach(() => { + vi.clearAllMocks(); + + gotoMock.mockResolvedValue(undefined); + newPageMock.mockResolvedValue({ goto: gotoMock }); + addCookiesMock.mockResolvedValue(undefined); + addInitScriptMock.mockResolvedValue(undefined); + pagesMock.mockReturnValue([existingPage]); + newContextMock.mockResolvedValue({ + newPage: newPageMock, + addCookies: addCookiesMock, + addInitScript: addInitScriptMock, + pages: pagesMock, + }); + closeMock.mockResolvedValue(undefined); + + const mockCdpBrowser = { + contexts: () => [ + { + newPage: newPageMock, + addCookies: addCookiesMock, + addInitScript: addInitScriptMock, + pages: pagesMock, + }, + ], + newContext: newContextMock, + close: closeMock, + }; + connectOverCDPMock.mockResolvedValue(mockCdpBrowser); + + defaultBrowserMock.mockReturnValue(Effect.succeed(Option.none())); + browserListMock.mockReturnValue(Effect.succeed([])); + killChromeProcessMock.mockReturnValue(Effect.void); + }); + + it("connects to auto-discovered Chrome when systemChrome is true", async () => { + autoDiscoverCdpMock.mockReturnValue( + Effect.succeed("ws://127.0.0.1:9222/devtools/browser/abc"), + ); + + const result = await runBrowser((browser) => + browser.createPage("https://example.com", { systemChrome: true }), + ); + + expect(autoDiscoverCdpMock).toHaveBeenCalledOnce(); + expect(connectOverCDPMock).toHaveBeenCalledWith( + "ws://127.0.0.1:9222/devtools/browser/abc", + expect.objectContaining({ timeout: expect.any(Number) }), + ); + expect(launchSystemChromeMock).not.toHaveBeenCalled(); + expect(launchMock).not.toHaveBeenCalled(); + expect(result.isExternalBrowser).toBe(true); + }); + + it("falls back to launchSystemChrome when discovered CDP fails to connect", async () => { + autoDiscoverCdpMock.mockReturnValue( + Effect.succeed("ws://127.0.0.1:9222/devtools/browser/bad"), + ); + connectOverCDPMock.mockRejectedValueOnce(new Error("Timeout")); + launchSystemChromeMock.mockReturnValue( + Effect.succeed({ + process: { kill: vi.fn() }, + wsUrl: "ws://127.0.0.1:55555/devtools/browser/xyz", + userDataDir: "/tmp/test", + tempUserDataDir: "/tmp/test", + }), + ); + + const result = await runBrowser((browser) => + browser.createPage("https://example.com", { systemChrome: true }), + ); + + expect(autoDiscoverCdpMock).toHaveBeenCalledOnce(); + expect(launchSystemChromeMock).toHaveBeenCalledOnce(); + expect(result.isExternalBrowser).toBe(false); + }); + + it("falls back to launchSystemChrome when auto-discovery fails", async () => { + const { CdpDiscoveryError } = await import("../src/errors"); + autoDiscoverCdpMock.mockReturnValue( + new CdpDiscoveryError({ cause: "No running Chrome found" }).asEffect(), + ); + launchSystemChromeMock.mockReturnValue( + Effect.succeed({ + process: { kill: vi.fn() }, + wsUrl: "ws://127.0.0.1:55555/devtools/browser/xyz", + userDataDir: "/tmp/expect-chrome-test", + tempUserDataDir: "/tmp/expect-chrome-test", + }), + ); + + const result = await runBrowser((browser) => + browser.createPage("https://example.com", { systemChrome: true }), + ); + + expect(autoDiscoverCdpMock).toHaveBeenCalledOnce(); + expect(launchSystemChromeMock).toHaveBeenCalledOnce(); + expect(connectOverCDPMock).toHaveBeenCalledWith("ws://127.0.0.1:55555/devtools/browser/xyz"); + expect(launchMock).not.toHaveBeenCalled(); + expect(result.isExternalBrowser).toBe(false); + }); + + it("launches system Chrome headed when falling back", async () => { + const { CdpDiscoveryError } = await import("../src/errors"); + autoDiscoverCdpMock.mockReturnValue( + new CdpDiscoveryError({ cause: "No running Chrome" }).asEffect(), + ); + launchSystemChromeMock.mockReturnValue( + Effect.succeed({ + process: { kill: vi.fn() }, + wsUrl: "ws://127.0.0.1:55555/devtools/browser/xyz", + userDataDir: "/tmp/test", + tempUserDataDir: "/tmp/test", + }), + ); + + await runBrowser((browser) => + browser.createPage("https://example.com", { systemChrome: true }), + ); + + expect(launchSystemChromeMock).toHaveBeenCalledWith({ headless: false }); + }); + + it("opens a fresh tab for external Chrome instead of reusing existing pages", async () => { + autoDiscoverCdpMock.mockReturnValue( + Effect.succeed("ws://127.0.0.1:9222/devtools/browser/abc"), + ); + + await runBrowser((browser) => + browser.createPage("https://example.com", { systemChrome: true }), + ); + + expect(newPageMock).toHaveBeenCalledOnce(); + }); + + it("skips auto-discovery when cdpUrl is already provided", async () => { + await runBrowser((browser) => + browser.createPage("https://example.com", { + systemChrome: true, + cdpUrl: "ws://custom:1234/devtools/browser/manual", + }), + ); + + expect(autoDiscoverCdpMock).not.toHaveBeenCalled(); + expect(launchSystemChromeMock).not.toHaveBeenCalled(); + expect(connectOverCDPMock).toHaveBeenCalledWith("ws://custom:1234/devtools/browser/manual"); + }); + + it("ignores systemChrome when browserType is not chromium", async () => { + webkitLaunchMock.mockResolvedValue({ + newContext: newContextMock, + close: closeMock, + }); + + await runBrowser((browser) => + browser.createPage("https://example.com", { systemChrome: true, browserType: "webkit" }), + ); + + expect(autoDiscoverCdpMock).not.toHaveBeenCalled(); + expect(launchSystemChromeMock).not.toHaveBeenCalled(); + expect(webkitLaunchMock).toHaveBeenCalledOnce(); + }); +}); From 5ea0558d5f12d1a50823f1d04e1c039b3d2f2c63 Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Thu, 2 Apr 2026 00:29:32 -0700 Subject: [PATCH 12/15] fix: handle spawn error events on Chrome child process and format --- packages/browser/src/browser.ts | 53 ++++++++++++---------- packages/browser/src/chrome-launcher.ts | 30 ++++++++++-- packages/browser/src/mcp/server.ts | 4 +- packages/browser/tests/create-page.test.ts | 17 +++---- packages/shared/src/analytics/analytics.ts | 4 +- 5 files changed, 68 insertions(+), 40 deletions(-) diff --git a/packages/browser/src/browser.ts b/packages/browser/src/browser.ts index 0c75acaef..e25a07d1b 100644 --- a/packages/browser/src/browser.ts +++ b/packages/browser/src/browser.ts @@ -206,8 +206,7 @@ export class Browser extends ServiceMap.Service()("@browser/Browser", { }); const resolvedChrome = yield* Effect.gen(function* () { - if (!useSystemChrome || options.cdpUrl) - return { _tag: "none" as const }; + if (!useSystemChrome || options.cdpUrl) return { _tag: "none" as const }; const discovered = yield* autoDiscoverCdp().pipe( Effect.map(Option.some), @@ -227,9 +226,12 @@ export class Browser extends ServiceMap.Service()("@browser/Browser", { return { _tag: "discovered" as const, browser: connectedBrowser.value }; } - yield* Effect.logDebug("Auto-discovered CDP endpoint failed to connect, launching new Chrome", { - wsUrl: discovered.value, - }); + yield* Effect.logDebug( + "Auto-discovered CDP endpoint failed to connect, launching new Chrome", + { + wsUrl: discovered.value, + }, + ); } const launched = yield* launchSystemChrome({ headless: false }).pipe( @@ -251,26 +253,27 @@ export class Browser extends ServiceMap.Service()("@browser/Browser", { const cleanup = chromeProcess ? killChromeProcess(chromeProcess) : Effect.void; const browserType = resolveBrowserType(engine); - const browser = resolvedChrome._tag === "discovered" - ? resolvedChrome.browser - : cdpEndpoint - ? yield* Effect.tryPromise({ - try: () => chromium.connectOverCDP(cdpEndpoint), - catch: (cause) => - new CdpConnectionError({ - endpointUrl: cdpEndpoint, - cause: cause instanceof Error ? cause.message : String(cause), - }), - }) - : yield* Effect.tryPromise({ - try: () => - browserType.launch({ - headless: !options.headed, - executablePath: options.executablePath, - args: engine === "chromium" && !options.headed ? HEADLESS_CHROMIUM_ARGS : [], - }), - catch: toBrowserLaunchError, - }); + const browser = + resolvedChrome._tag === "discovered" + ? resolvedChrome.browser + : cdpEndpoint + ? yield* Effect.tryPromise({ + try: () => chromium.connectOverCDP(cdpEndpoint), + catch: (cause) => + new CdpConnectionError({ + endpointUrl: cdpEndpoint, + cause: cause instanceof Error ? cause.message : String(cause), + }), + }) + : yield* Effect.tryPromise({ + try: () => + browserType.launch({ + headless: !options.headed, + executablePath: options.executablePath, + args: engine === "chromium" && !options.headed ? HEADLESS_CHROMIUM_ARGS : [], + }), + catch: toBrowserLaunchError, + }); const setupPage = Effect.gen(function* () { const defaultBrowserContext = diff --git a/packages/browser/src/chrome-launcher.ts b/packages/browser/src/chrome-launcher.ts index 00ca84f22..5ad43f4f2 100644 --- a/packages/browser/src/chrome-launcher.ts +++ b/packages/browser/src/chrome-launcher.ts @@ -213,10 +213,32 @@ export const launchSystemChrome = Effect.fn("launchSystemChrome")(function* (opt const wsUrl = yield* Effect.callback((resume) => { const deadline = Date.now() + CDP_LAUNCH_TIMEOUT_MS; let pendingTimer: ReturnType | undefined; + let settled = false; + + const settle = (effect: Effect.Effect) => { + if (settled) return; + settled = true; + if (pendingTimer !== undefined) clearTimeout(pendingTimer); + child.removeListener("error", onSpawnError); + resume(effect); + }; + + const onSpawnError = (error: Error) => { + settle( + Effect.fail( + new ChromeLaunchTimeoutError({ + timeoutMs: CDP_LAUNCH_TIMEOUT_MS, + cause: `Chrome process error: ${error.message}`, + }), + ), + ); + }; + + child.on("error", onSpawnError); const poll = () => { if (Date.now() > deadline) { - resume( + settle( Effect.fail( new ChromeLaunchTimeoutError({ timeoutMs: CDP_LAUNCH_TIMEOUT_MS, @@ -229,12 +251,12 @@ export const launchSystemChrome = Effect.fn("launchSystemChrome")(function* (opt const result = readDevToolsActivePort(userDataDir); if (result) { - resume(Effect.succeed(`ws://127.0.0.1:${result.port}${result.wsPath}`)); + settle(Effect.succeed(`ws://127.0.0.1:${result.port}${result.wsPath}`)); return; } if (child.exitCode !== null) { - resume( + settle( Effect.fail( new ChromeLaunchTimeoutError({ timeoutMs: CDP_LAUNCH_TIMEOUT_MS, @@ -251,7 +273,9 @@ export const launchSystemChrome = Effect.fn("launchSystemChrome")(function* (opt poll(); return Effect.sync(() => { + settled = true; if (pendingTimer !== undefined) clearTimeout(pendingTimer); + child.removeListener("error", onSpawnError); }); }).pipe(Effect.tapError(() => cleanupFailedLaunch(child, tempDir))); diff --git a/packages/browser/src/mcp/server.ts b/packages/browser/src/mcp/server.ts index c64ba8738..87d718932 100644 --- a/packages/browser/src/mcp/server.ts +++ b/packages/browser/src/mcp/server.ts @@ -107,7 +107,9 @@ export const createBrowserMcpServer = ( const engineSuffix = browserType && browserType !== "chromium" ? ` [${browserType}]` : ""; const cdpSuffix = cdp ? ` (connected via CDP: ${cdp})` : ""; const chromeSuffix = systemChrome - ? (result.isExternalBrowser ? " (live Chrome)" : " (system Chrome)") + ? result.isExternalBrowser + ? " (live Chrome)" + : " (system Chrome)" : ""; return textResult( `Opened ${url}${engineSuffix}${cdpSuffix}${chromeSuffix}` + diff --git a/packages/browser/tests/create-page.test.ts b/packages/browser/tests/create-page.test.ts index 3df524aba..e03a89033 100644 --- a/packages/browser/tests/create-page.test.ts +++ b/packages/browser/tests/create-page.test.ts @@ -329,7 +329,10 @@ describe("Browser.createPage browserType", () => { }); describe("Browser.createPage systemChrome", () => { - const existingPage = { goto: vi.fn().mockResolvedValue(undefined), evaluate: vi.fn().mockResolvedValue(undefined) }; + const existingPage = { + goto: vi.fn().mockResolvedValue(undefined), + evaluate: vi.fn().mockResolvedValue(undefined), + }; beforeEach(() => { vi.clearAllMocks(); @@ -367,9 +370,7 @@ describe("Browser.createPage systemChrome", () => { }); it("connects to auto-discovered Chrome when systemChrome is true", async () => { - autoDiscoverCdpMock.mockReturnValue( - Effect.succeed("ws://127.0.0.1:9222/devtools/browser/abc"), - ); + autoDiscoverCdpMock.mockReturnValue(Effect.succeed("ws://127.0.0.1:9222/devtools/browser/abc")); const result = await runBrowser((browser) => browser.createPage("https://example.com", { systemChrome: true }), @@ -386,9 +387,7 @@ describe("Browser.createPage systemChrome", () => { }); it("falls back to launchSystemChrome when discovered CDP fails to connect", async () => { - autoDiscoverCdpMock.mockReturnValue( - Effect.succeed("ws://127.0.0.1:9222/devtools/browser/bad"), - ); + autoDiscoverCdpMock.mockReturnValue(Effect.succeed("ws://127.0.0.1:9222/devtools/browser/bad")); connectOverCDPMock.mockRejectedValueOnce(new Error("Timeout")); launchSystemChromeMock.mockReturnValue( Effect.succeed({ @@ -455,9 +454,7 @@ describe("Browser.createPage systemChrome", () => { }); it("opens a fresh tab for external Chrome instead of reusing existing pages", async () => { - autoDiscoverCdpMock.mockReturnValue( - Effect.succeed("ws://127.0.0.1:9222/devtools/browser/abc"), - ); + autoDiscoverCdpMock.mockReturnValue(Effect.succeed("ws://127.0.0.1:9222/devtools/browser/abc")); await runBrowser((browser) => browser.createPage("https://example.com", { systemChrome: true }), diff --git a/packages/shared/src/analytics/analytics.ts b/packages/shared/src/analytics/analytics.ts index 8ff0cd0b5..8521eddaf 100644 --- a/packages/shared/src/analytics/analytics.ts +++ b/packages/shared/src/analytics/analytics.ts @@ -100,7 +100,9 @@ export class Analytics extends ServiceMap.Service()("@expect/Analytic if (telemetryDisabled) { const capture = (() => Effect.void) as never; - const track = ((() => (self: Effect.Effect) => self) as never); + const track = (() => + (self: Effect.Effect) => + self) as never; return { capture, track, flush: Effect.void } as const; } From c1daf0521a69e75751573899c1379bb0dd145a8f Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Thu, 2 Apr 2026 00:36:16 -0700 Subject: [PATCH 13/15] refactor: replace systemChrome with liveChrome, remove launch fallback Simplifies browser connection to two clear modes: Playwright bundled Chromium (default) or connect to an already-running Chrome (liveChrome). Removes the confusing middle ground of launching a fresh system Chrome binary as a fallback. --- packages/browser/src/browser.ts | 114 +++++++++------------ packages/browser/src/index.ts | 1 - packages/browser/src/mcp/mcp-session.ts | 4 +- packages/browser/src/mcp/server.ts | 14 +-- packages/browser/src/types.ts | 2 +- packages/browser/tests/create-page.test.ts | 88 ++++------------ 6 files changed, 78 insertions(+), 145 deletions(-) diff --git a/packages/browser/src/browser.ts b/packages/browser/src/browser.ts index e25a07d1b..a27fa68f2 100644 --- a/packages/browser/src/browser.ts +++ b/packages/browser/src/browser.ts @@ -1,7 +1,7 @@ import { Browsers, Cookies, layerLive, browserKeyOf, Cookie } from "@expect/cookies"; import type { Browser as BrowserProfile } from "@expect/cookies"; import { chromium, webkit, firefox } from "playwright"; -import type { Locator, Page } from "playwright"; +import type { Browser as PlaywrightBrowser, Locator, Page } from "playwright"; import type { BrowserEngine } from "./types"; import { Array as Arr, Effect, Layer, Option, ServiceMap } from "effect"; @@ -25,7 +25,6 @@ import { NavigationError, SnapshotTimeoutError, } from "./errors"; -import { launchSystemChrome, killChromeProcess } from "./chrome-launcher"; import { autoDiscoverCdp } from "./cdp-discovery"; import { toActionError } from "./utils/action-error"; import { compactTree } from "./utils/compact-tree"; @@ -197,83 +196,72 @@ export class Browser extends ServiceMap.Service()("@browser/Browser", { options: CreatePageOptions = {}, ) { const engine = options.browserType ?? "chromium"; - const useSystemChrome = options.systemChrome && engine === "chromium"; + const useLiveChrome = options.liveChrome && engine === "chromium" && !options.cdpUrl; yield* Effect.annotateCurrentSpan({ url: url ?? "about:blank", cdp: Boolean(options.cdpUrl), - systemChrome: Boolean(useSystemChrome), + liveChrome: Boolean(useLiveChrome), browserType: engine, }); - const resolvedChrome = yield* Effect.gen(function* () { - if (!useSystemChrome || options.cdpUrl) return { _tag: "none" as const }; + const liveBrowser = yield* Effect.gen(function* () { + if (!useLiveChrome) return Option.none(); const discovered = yield* autoDiscoverCdp().pipe( Effect.map(Option.some), Effect.catchTag("CdpDiscoveryError", () => Effect.succeed(Option.none())), ); - if (Option.isSome(discovered)) { - const connectedBrowser = yield* Effect.tryPromise(() => - chromium.connectOverCDP(discovered.value, { timeout: CDP_DISCOVERY_TIMEOUT_MS }), - ).pipe( - Effect.map(Option.some), - Effect.catchTag("UnknownError", () => Effect.succeed(Option.none())), - ); + if (Option.isNone(discovered)) { + yield* Effect.logDebug("No running Chrome found, falling back to bundled Chromium"); + return Option.none(); + } - if (Option.isSome(connectedBrowser)) { - yield* Effect.logInfo("Connected to live Chrome", { wsUrl: discovered.value }); - return { _tag: "discovered" as const, browser: connectedBrowser.value }; - } + const connectedBrowser = yield* Effect.tryPromise(() => + chromium.connectOverCDP(discovered.value, { timeout: CDP_DISCOVERY_TIMEOUT_MS }), + ).pipe( + Effect.map(Option.some), + Effect.catchTag("UnknownError", () => Effect.succeed(Option.none())), + ); + if (Option.isNone(connectedBrowser)) { yield* Effect.logDebug( - "Auto-discovered CDP endpoint failed to connect, launching new Chrome", + "Failed to connect to discovered Chrome, falling back to bundled Chromium", { wsUrl: discovered.value, }, ); + return Option.none(); } - const launched = yield* launchSystemChrome({ headless: false }).pipe( - Effect.tap((process) => - Effect.logInfo("Launched system Chrome via CDP", { wsUrl: process.wsUrl }), - ), - Effect.catchTags({ - ChromeNotFoundError: Effect.die, - ChromeLaunchTimeoutError: Effect.die, - }), - ); - return { _tag: "launched" as const, process: launched }; + yield* Effect.logInfo("Connected to live Chrome", { wsUrl: discovered.value }); + return connectedBrowser; }); - const chromeProcess = resolvedChrome._tag === "launched" ? resolvedChrome.process : undefined; - const isExternalBrowser = resolvedChrome._tag === "discovered"; - const cdpEndpoint = - chromeProcess?.wsUrl ?? (engine === "chromium" ? options.cdpUrl : undefined); - const cleanup = chromeProcess ? killChromeProcess(chromeProcess) : Effect.void; + const isExternalBrowser = Option.isSome(liveBrowser); + const cdpEndpoint = engine === "chromium" ? options.cdpUrl : undefined; const browserType = resolveBrowserType(engine); - const browser = - resolvedChrome._tag === "discovered" - ? resolvedChrome.browser - : cdpEndpoint - ? yield* Effect.tryPromise({ - try: () => chromium.connectOverCDP(cdpEndpoint), - catch: (cause) => - new CdpConnectionError({ - endpointUrl: cdpEndpoint, - cause: cause instanceof Error ? cause.message : String(cause), - }), - }) - : yield* Effect.tryPromise({ - try: () => - browserType.launch({ - headless: !options.headed, - executablePath: options.executablePath, - args: engine === "chromium" && !options.headed ? HEADLESS_CHROMIUM_ARGS : [], - }), - catch: toBrowserLaunchError, - }); + const browser = Option.isSome(liveBrowser) + ? liveBrowser.value + : cdpEndpoint + ? yield* Effect.tryPromise({ + try: () => chromium.connectOverCDP(cdpEndpoint), + catch: (cause) => + new CdpConnectionError({ + endpointUrl: cdpEndpoint, + cause: cause instanceof Error ? cause.message : String(cause), + }), + }) + : yield* Effect.tryPromise({ + try: () => + browserType.launch({ + headless: !options.headed, + executablePath: options.executablePath, + args: engine === "chromium" && !options.headed ? HEADLESS_CHROMIUM_ARGS : [], + }), + catch: toBrowserLaunchError, + }); const setupPage = Effect.gen(function* () { const defaultBrowserContext = @@ -355,20 +343,16 @@ export class Browser extends ServiceMap.Service()("@browser/Browser", { }); } - return { browser, context, page, cleanup, isExternalBrowser }; + return { browser, context, page, cleanup: Effect.void, isExternalBrowser }; }); return yield* setupPage.pipe( - Effect.tapError(() => - cleanup.pipe( - Effect.andThen(() => { - if (isExternalBrowser) return Effect.void; - return Effect.tryPromise(() => browser.close()).pipe( - Effect.catchTag("UnknownError", () => Effect.void), - ); - }), - ), - ), + Effect.tapError(() => { + if (isExternalBrowser) return Effect.void; + return Effect.tryPromise(() => browser.close()).pipe( + Effect.catchTag("UnknownError", () => Effect.void), + ); + }), ); }); diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index d97d2b099..d9634d818 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -3,7 +3,6 @@ export { buildReplayViewerHtml } from "./replay-viewer"; export { diffSnapshots } from "./diff"; export { collectEvents, collectAllEvents, loadSession } from "./recorder"; export { autoDiscoverCdp, discoverCdpUrl } from "./cdp-discovery"; -export { launchSystemChrome, killChromeProcess } from "./chrome-launcher"; export { RrVideo, RrVideoConvertError } from "./rrvideo"; export type { Browser as BrowserProfile, diff --git a/packages/browser/src/mcp/mcp-session.ts b/packages/browser/src/mcp/mcp-session.ts index 68cf4a481..9191923ed 100644 --- a/packages/browser/src/mcp/mcp-session.ts +++ b/packages/browser/src/mcp/mcp-session.ts @@ -60,7 +60,7 @@ export interface OpenOptions { waitUntil?: "load" | "domcontentloaded" | "networkidle" | "commit"; cdpUrl?: string; browserType?: BrowserEngine; - systemChrome?: boolean; + liveChrome?: boolean; } export interface OpenResult { @@ -229,7 +229,7 @@ export class McpSession extends ServiceMap.Service()("@browser/McpSe videoOutputDir, cdpUrl: options.cdpUrl ?? defaultCdpUrl, browserType: options.browserType, - systemChrome: options.systemChrome, + liveChrome: options.liveChrome, }); const sessionData: BrowserSessionData = { diff --git a/packages/browser/src/mcp/server.ts b/packages/browser/src/mcp/server.ts index 87d718932..d296b5748 100644 --- a/packages/browser/src/mcp/server.ts +++ b/packages/browser/src/mcp/server.ts @@ -78,15 +78,15 @@ export const createBrowserMcpServer = ( .describe( "Browser engine to launch (default: chromium). Use 'webkit' for Safari-like testing or 'firefox' for Firefox testing. CDP connections are only supported with chromium.", ), - systemChrome: z + liveChrome: z .boolean() .optional() .describe( - "Use the system-installed Chrome (Google Chrome, Brave, Edge) instead of Playwright's bundled Chromium. First tries to connect to an already-running Chrome with remote debugging enabled; if none is found, launches a new headed instance. Ignored when 'cdp' is provided.", + "Connect to the user's already-running Chrome browser instead of launching Playwright's bundled Chromium. The user must have Chrome open with remote debugging enabled. Falls back to bundled Chromium if no running Chrome is found. Ignored when 'cdp' is provided.", ), }, }, - ({ url, headed, cookies, waitUntil, cdp, browser: browserType, systemChrome }) => + ({ url, headed, cookies, waitUntil, cdp, browser: browserType, liveChrome }) => runMcp( Effect.gen(function* () { const session = yield* McpSession; @@ -102,15 +102,11 @@ export const createBrowserMcpServer = ( waitUntil, cdpUrl: cdp, browserType, - systemChrome, + liveChrome, }); const engineSuffix = browserType && browserType !== "chromium" ? ` [${browserType}]` : ""; const cdpSuffix = cdp ? ` (connected via CDP: ${cdp})` : ""; - const chromeSuffix = systemChrome - ? result.isExternalBrowser - ? " (live Chrome)" - : " (system Chrome)" - : ""; + const chromeSuffix = liveChrome && result.isExternalBrowser ? " (live Chrome)" : ""; return textResult( `Opened ${url}${engineSuffix}${cdpSuffix}${chromeSuffix}` + (result.injectedCookieCount > 0 diff --git a/packages/browser/src/types.ts b/packages/browser/src/types.ts index fbbcab8cb..2b174f6e4 100644 --- a/packages/browser/src/types.ts +++ b/packages/browser/src/types.ts @@ -54,7 +54,7 @@ export interface CreatePageOptions { videoOutputDir?: string; cdpUrl?: string; browserType?: BrowserEngine; - systemChrome?: boolean; + liveChrome?: boolean; } export interface AnnotatedScreenshotOptions extends SnapshotOptions { diff --git a/packages/browser/tests/create-page.test.ts b/packages/browser/tests/create-page.test.ts index e03a89033..0de0eef32 100644 --- a/packages/browser/tests/create-page.test.ts +++ b/packages/browser/tests/create-page.test.ts @@ -14,8 +14,6 @@ const { gotoMock, closeMock, autoDiscoverCdpMock, - launchSystemChromeMock, - killChromeProcessMock, } = vi.hoisted(() => ({ defaultBrowserMock: vi.fn(), browserListMock: vi.fn(), @@ -30,8 +28,6 @@ const { gotoMock: vi.fn(), closeMock: vi.fn(), autoDiscoverCdpMock: vi.fn(), - launchSystemChromeMock: vi.fn(), - killChromeProcessMock: vi.fn(), })); vi.mock("@expect/cookies", async () => { @@ -80,11 +76,6 @@ vi.mock("../src/cdp-discovery", () => ({ autoDiscoverCdp: autoDiscoverCdpMock, })); -vi.mock("../src/chrome-launcher", () => ({ - launchSystemChrome: launchSystemChromeMock, - killChromeProcess: killChromeProcessMock, -})); - import { Effect, Option } from "effect"; import { runBrowser } from "../src/browser"; @@ -328,7 +319,7 @@ describe("Browser.createPage browserType", () => { }); }); -describe("Browser.createPage systemChrome", () => { +describe("Browser.createPage liveChrome", () => { const existingPage = { goto: vi.fn().mockResolvedValue(undefined), evaluate: vi.fn().mockResolvedValue(undefined), @@ -366,14 +357,13 @@ describe("Browser.createPage systemChrome", () => { defaultBrowserMock.mockReturnValue(Effect.succeed(Option.none())); browserListMock.mockReturnValue(Effect.succeed([])); - killChromeProcessMock.mockReturnValue(Effect.void); }); - it("connects to auto-discovered Chrome when systemChrome is true", async () => { + it("connects to auto-discovered Chrome when liveChrome is true", async () => { autoDiscoverCdpMock.mockReturnValue(Effect.succeed("ws://127.0.0.1:9222/devtools/browser/abc")); const result = await runBrowser((browser) => - browser.createPage("https://example.com", { systemChrome: true }), + browser.createPage("https://example.com", { liveChrome: true }), ); expect(autoDiscoverCdpMock).toHaveBeenCalledOnce(); @@ -381,84 +371,50 @@ describe("Browser.createPage systemChrome", () => { "ws://127.0.0.1:9222/devtools/browser/abc", expect.objectContaining({ timeout: expect.any(Number) }), ); - expect(launchSystemChromeMock).not.toHaveBeenCalled(); expect(launchMock).not.toHaveBeenCalled(); expect(result.isExternalBrowser).toBe(true); }); - it("falls back to launchSystemChrome when discovered CDP fails to connect", async () => { + it("falls back to Playwright when discovered CDP fails to connect", async () => { autoDiscoverCdpMock.mockReturnValue(Effect.succeed("ws://127.0.0.1:9222/devtools/browser/bad")); connectOverCDPMock.mockRejectedValueOnce(new Error("Timeout")); - launchSystemChromeMock.mockReturnValue( - Effect.succeed({ - process: { kill: vi.fn() }, - wsUrl: "ws://127.0.0.1:55555/devtools/browser/xyz", - userDataDir: "/tmp/test", - tempUserDataDir: "/tmp/test", - }), - ); + launchMock.mockResolvedValue({ + newContext: newContextMock, + close: closeMock, + }); const result = await runBrowser((browser) => - browser.createPage("https://example.com", { systemChrome: true }), + browser.createPage("https://example.com", { liveChrome: true }), ); expect(autoDiscoverCdpMock).toHaveBeenCalledOnce(); - expect(launchSystemChromeMock).toHaveBeenCalledOnce(); + expect(launchMock).toHaveBeenCalledOnce(); expect(result.isExternalBrowser).toBe(false); }); - it("falls back to launchSystemChrome when auto-discovery fails", async () => { + it("falls back to Playwright when auto-discovery fails", async () => { const { CdpDiscoveryError } = await import("../src/errors"); autoDiscoverCdpMock.mockReturnValue( new CdpDiscoveryError({ cause: "No running Chrome found" }).asEffect(), ); - launchSystemChromeMock.mockReturnValue( - Effect.succeed({ - process: { kill: vi.fn() }, - wsUrl: "ws://127.0.0.1:55555/devtools/browser/xyz", - userDataDir: "/tmp/expect-chrome-test", - tempUserDataDir: "/tmp/expect-chrome-test", - }), - ); + launchMock.mockResolvedValue({ + newContext: newContextMock, + close: closeMock, + }); const result = await runBrowser((browser) => - browser.createPage("https://example.com", { systemChrome: true }), + browser.createPage("https://example.com", { liveChrome: true }), ); expect(autoDiscoverCdpMock).toHaveBeenCalledOnce(); - expect(launchSystemChromeMock).toHaveBeenCalledOnce(); - expect(connectOverCDPMock).toHaveBeenCalledWith("ws://127.0.0.1:55555/devtools/browser/xyz"); - expect(launchMock).not.toHaveBeenCalled(); + expect(launchMock).toHaveBeenCalledOnce(); expect(result.isExternalBrowser).toBe(false); }); - it("launches system Chrome headed when falling back", async () => { - const { CdpDiscoveryError } = await import("../src/errors"); - autoDiscoverCdpMock.mockReturnValue( - new CdpDiscoveryError({ cause: "No running Chrome" }).asEffect(), - ); - launchSystemChromeMock.mockReturnValue( - Effect.succeed({ - process: { kill: vi.fn() }, - wsUrl: "ws://127.0.0.1:55555/devtools/browser/xyz", - userDataDir: "/tmp/test", - tempUserDataDir: "/tmp/test", - }), - ); - - await runBrowser((browser) => - browser.createPage("https://example.com", { systemChrome: true }), - ); - - expect(launchSystemChromeMock).toHaveBeenCalledWith({ headless: false }); - }); - it("opens a fresh tab for external Chrome instead of reusing existing pages", async () => { autoDiscoverCdpMock.mockReturnValue(Effect.succeed("ws://127.0.0.1:9222/devtools/browser/abc")); - await runBrowser((browser) => - browser.createPage("https://example.com", { systemChrome: true }), - ); + await runBrowser((browser) => browser.createPage("https://example.com", { liveChrome: true })); expect(newPageMock).toHaveBeenCalledOnce(); }); @@ -466,28 +422,26 @@ describe("Browser.createPage systemChrome", () => { it("skips auto-discovery when cdpUrl is already provided", async () => { await runBrowser((browser) => browser.createPage("https://example.com", { - systemChrome: true, + liveChrome: true, cdpUrl: "ws://custom:1234/devtools/browser/manual", }), ); expect(autoDiscoverCdpMock).not.toHaveBeenCalled(); - expect(launchSystemChromeMock).not.toHaveBeenCalled(); expect(connectOverCDPMock).toHaveBeenCalledWith("ws://custom:1234/devtools/browser/manual"); }); - it("ignores systemChrome when browserType is not chromium", async () => { + it("ignores liveChrome when browserType is not chromium", async () => { webkitLaunchMock.mockResolvedValue({ newContext: newContextMock, close: closeMock, }); await runBrowser((browser) => - browser.createPage("https://example.com", { systemChrome: true, browserType: "webkit" }), + browser.createPage("https://example.com", { liveChrome: true, browserType: "webkit" }), ); expect(autoDiscoverCdpMock).not.toHaveBeenCalled(); - expect(launchSystemChromeMock).not.toHaveBeenCalled(); expect(webkitLaunchMock).toHaveBeenCalledOnce(); }); }); From 9e910d46fa738aba04515cc1ec17b9f5f7d3f5ed Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Thu, 2 Apr 2026 02:01:29 -0700 Subject: [PATCH 14/15] revert: undo analytics refactor, keep machine-id import change only --- packages/shared/src/analytics/analytics.ts | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/packages/shared/src/analytics/analytics.ts b/packages/shared/src/analytics/analytics.ts index 8521eddaf..331688248 100644 --- a/packages/shared/src/analytics/analytics.ts +++ b/packages/shared/src/analytics/analytics.ts @@ -98,17 +98,12 @@ export class Analytics extends ServiceMap.Service()("@expect/Analytic }) || githubActionsValue !== ""; - if (telemetryDisabled) { - const capture = (() => Effect.void) as never; - const track = (() => - (self: Effect.Effect) => - self) as never; - return { capture, track, flush: Effect.void } as const; - } - const projectId = hash(process.cwd()); - const distinctId = yield* Effect.tryPromise(() => machineId()).pipe( + const distinctId = yield* Effect.tryPromise(async () => { + if (telemetryDisabled) return ""; + return machineId(); + }).pipe( Effect.catchTag("UnknownError", (cause) => Effect.logWarning("Failed to get machine ID, using fallback", { cause }).pipe( Effect.as(globalThis.crypto.randomUUID()), @@ -121,6 +116,7 @@ export class Analytics extends ServiceMap.Service()("@expect/Analytic ...[properties]: EventMap[K] extends undefined ? [] : [EventMap[K]] ) => Effect.gen(function* () { + if (telemetryDisabled) return; const commonProperties = { timestamp: new Date().toISOString(), projectId, @@ -162,7 +158,7 @@ export class Analytics extends ServiceMap.Service()("@expect/Analytic ); })) as never; - return { capture, track, flush: provider.flush } as const; + return { capture, track, flush: telemetryDisabled ? Effect.void : provider.flush } as const; }), }) { static layerPostHog = Layer.effect(this)(this.make).pipe( From f348d3eb2fd75880574bfee8b665299b4e903247 Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Thu, 2 Apr 2026 02:12:59 -0700 Subject: [PATCH 15/15] fix --- packages/browser/src/cdp-discovery.ts | 17 +++++------------ packages/browser/src/chrome-launcher.ts | 14 +++----------- .../src/utils/parse-devtools-active-port.ts | 14 ++++++++++++++ 3 files changed, 22 insertions(+), 23 deletions(-) create mode 100644 packages/browser/src/utils/parse-devtools-active-port.ts diff --git a/packages/browser/src/cdp-discovery.ts b/packages/browser/src/cdp-discovery.ts index 714eed474..a75618b95 100644 --- a/packages/browser/src/cdp-discovery.ts +++ b/packages/browser/src/cdp-discovery.ts @@ -5,6 +5,7 @@ import net from "node:net"; import { Effect, Option } from "effect"; import { CDP_DISCOVERY_TIMEOUT_MS, CDP_COMMON_PORTS, CDP_PORT_PROBE_TIMEOUT_MS } from "./constants"; import { CdpDiscoveryError } from "./errors"; +import { parseDevToolsActivePort } from "./utils/parse-devtools-active-port"; interface VersionInfo { readonly webSocketDebuggerUrl?: string; @@ -162,21 +163,13 @@ const readDevToolsActivePort = (userDataDir: string) => }), }).pipe( Effect.flatMap((content) => { - const lines = content.trim().split("\n"); - const portStr = lines[0]?.trim(); - if (!portStr) { + const parsed = parseDevToolsActivePort(content); + if (!parsed) { return new CdpDiscoveryError({ - cause: `Empty DevToolsActivePort in ${userDataDir}`, + cause: `Invalid DevToolsActivePort in ${userDataDir}`, }).asEffect(); } - const port = Number.parseInt(portStr, 10); - if (Number.isNaN(port)) { - return new CdpDiscoveryError({ - cause: `Invalid port in DevToolsActivePort: ${portStr}`, - }).asEffect(); - } - const wsPath = lines[1]?.trim() ?? "/devtools/browser"; - return Effect.succeed({ port, wsPath }); + return Effect.succeed(parsed); }), ); diff --git a/packages/browser/src/chrome-launcher.ts b/packages/browser/src/chrome-launcher.ts index 5ad43f4f2..9e1620c26 100644 --- a/packages/browser/src/chrome-launcher.ts +++ b/packages/browser/src/chrome-launcher.ts @@ -5,6 +5,7 @@ import path from "node:path"; import which from "which"; import { Effect } from "effect"; import { ChromeNotFoundError, ChromeLaunchTimeoutError } from "./errors"; +import { parseDevToolsActivePort } from "./utils/parse-devtools-active-port"; import { CDP_LAUNCH_TIMEOUT_MS, CDP_POLL_INTERVAL_MS, @@ -92,19 +93,10 @@ export const findSystemChrome = Effect.fn("findSystemChrome")(function* () { return yield* new ChromeNotFoundError(); }); -const readDevToolsActivePort = ( - userDataDir: string, -): { port: number; wsPath: string } | undefined => { +const readDevToolsActivePort = (userDataDir: string) => { const filePath = path.join(userDataDir, "DevToolsActivePort"); try { - const content = fs.readFileSync(filePath, "utf-8"); - const lines = content.trim().split("\n"); - const portStr = lines[0]?.trim(); - if (!portStr) return undefined; - const port = Number.parseInt(portStr, 10); - if (Number.isNaN(port)) return undefined; - const wsPath = lines[1]?.trim() ?? "/devtools/browser"; - return { port, wsPath }; + return parseDevToolsActivePort(fs.readFileSync(filePath, "utf-8")); } catch { return undefined; } diff --git a/packages/browser/src/utils/parse-devtools-active-port.ts b/packages/browser/src/utils/parse-devtools-active-port.ts new file mode 100644 index 000000000..e1f46cdb8 --- /dev/null +++ b/packages/browser/src/utils/parse-devtools-active-port.ts @@ -0,0 +1,14 @@ +interface DevToolsActivePort { + readonly port: number; + readonly wsPath: string; +} + +export const parseDevToolsActivePort = (content: string): DevToolsActivePort | undefined => { + const lines = content.trim().split("\n"); + const portStr = lines[0]?.trim(); + if (!portStr) return undefined; + const port = Number.parseInt(portStr, 10); + if (Number.isNaN(port)) return undefined; + const wsPath = lines[1]?.trim() ?? "/devtools/browser"; + return { port, wsPath }; +};