From ad78b6a84e16424d59306d7c851d178d783acd87 Mon Sep 17 00:00:00 2001 From: can1357 Date: Sat, 1 Aug 2026 06:22:49 +0200 Subject: [PATCH] feat(coding-agent/tools): implemented shared browser daemon management - Added `ensureSharedBrowser` and shared browser acquisition to manage project-shared broker-owned Chromium instances. - Implemented concurrent duplicate daemon start prevention and single-flight `pendingOpens` deduplication. - Updated browser handle disposal to disconnect from shared daemons rather than closing them. - Updated browser documentation and launch specifications to support shared and local headless runs. --- .gitignore | 1 + docs/tools/browser.md | 7 +- packages/coding-agent/CHANGELOG.md | 8 + packages/coding-agent/src/launch/broker.ts | 100 +++++---- packages/coding-agent/src/tools/browser.ts | 2 +- .../coding-agent/src/tools/browser/launch.ts | 63 +++++- .../src/tools/browser/registry.ts | 153 +++++++++++--- .../src/tools/browser/shared-daemon.ts | 190 ++++++++++++++++++ 8 files changed, 440 insertions(+), 84 deletions(-) create mode 100644 packages/coding-agent/src/tools/browser/shared-daemon.ts diff --git a/.gitignore b/.gitignore index 3266d9634..bec1aacf3 100644 --- a/.gitignore +++ b/.gitignore @@ -64,6 +64,7 @@ packages/natives/npm/ packages/coding-agent/src/utils/mupdf-wasm.wasm /runs/ python/omp-rpc/src/omp_rpc.egg-info/ +python/omp-rpc/build/ # parallel-agent worktrees .wt/ CPU*.md diff --git a/docs/tools/browser.md b/docs/tools/browser.md index 2189c2e75..d320ed577 100644 --- a/docs/tools/browser.md +++ b/docs/tools/browser.md @@ -11,6 +11,7 @@ - `packages/coding-agent/src/tools/browser/tab-worker-entry.ts` — worker-thread transport bootstrap. - `packages/coding-agent/src/tools/browser/registry.ts` — browser-handle registry keyed by browser kind. - `packages/coding-agent/src/tools/browser/launch.ts` — Puppeteer loading, Chromium resolution/download, headless launch, stealth injection. + - `packages/coding-agent/src/tools/browser/shared-daemon.ts` — project-shared broker-owned Chromium (ensure/attach over the daemon broker). - `packages/coding-agent/src/tools/browser/attach.ts` — CDP attach/reuse, target picking, spawned-app process handling. - `packages/coding-agent/src/tools/browser/tab-protocol.ts` — worker init/run/result message schema. - `packages/coding-agent/src/tools/browser/readable.ts` — `tab.extract()` readability extraction. @@ -99,7 +100,7 @@ The tool returns one result per call; no streaming partial output is emitted fro 4. `open` acquires a browser handle through `acquireBrowser()` (`packages/coding-agent/src/tools/browser/registry.ts`): - existing connected handle is reused by browser-kind key; - stale disconnected handles are disposed and recreated; - - headless launches via `launchHeadlessBrowser()`; + - headless attaches to the project-shared broker-owned Chromium (`ensureSharedBrowser()`); in a CLI-host process a broker failure is a hard error, while non-CLI hosts (`bun test`, SDK embedding) launch a process-local Chromium via `launchHeadlessBrowser()`; - `connected` waits for `${cdpUrl}/json/version`, then `puppeteer.connect()`; - `spawned` first tries `findReusableCdp()`, else kills same-path processes, allocates a free loopback port, spawns the executable with `--remote-debugging-port=`, waits for CDP, then connects. - `cmux` connects a `CmuxSocketClient` to the cmux unix socket; existing cmux handles are reused unconditionally (no connection-liveness recheck). @@ -162,7 +163,7 @@ The tool returns one result per call; no streaming partial output is emitted fro - `close` — release one tab or all tabs. - `run` — execute JS inside the tab worker. - **Browser kind** - - **Headless**: launches local Chromium with Puppeteer, applies stealth patches, and creates a fresh page per tab. + - **Headless**: attaches to one project-shared Chromium supervised by the daemon broker (`omp.browser.headless` / `omp.browser.headed` in `hub ps`), applies stealth patches, and creates a fresh page per tab. The daemon stops with the last omp client in the project. Non-CLI hosts launch a private local Chromium instead. - **Spawned app (`app.path`)**: reuses an existing CDP-enabled process for that executable when possible; otherwise kills same-path processes, spawns the executable with remote debugging enabled, then attaches. No stealth patches are injected. - **Connected browser (`app.cdp_url`, or the `browser.cdpUrl` setting when the call carries no `app`)**: attaches to an already-running CDP endpoint. No process ownership; close only disconnects. - **Cmux surface (`browser.cmux`)**: with no `app` and a cmux socket available (`CMUX_SOCKET_PATH`, enabled by the `browser.cmux` setting / `PI_BROWSER_CMUX` override), drives a cmux WKWebView surface over a unix-socket JSON-RPC client instead of Puppeteer. No Bun worker and no stealth patches; `open` opens a split (owning that surface), `run` executes via `runCmuxCode()`, and `close` issues `surface.close` for surfaces it owns (leaving the workspace's last surface open). @@ -241,7 +242,7 @@ The tool returns one result per call; no streaming partial output is emitted fro - `loadPuppeteer()` and `loadPuppeteerInWorker()` temporarily redirect `cwd` to a safe Puppeteer directory before importing `puppeteer-core`, because Puppeteer probes the current working directory during module load. - Headless launch prefers a detected system Chrome/Chromium, then `PUPPETEER_EXECUTABLE_PATH`, and only then downloads Chromium. - Headless launch always passes `--no-sandbox`, `--disable-setuid-sandbox`, `--disable-blink-features=AutomationControlled`, and a `--window-size=...` matching the initial viewport. It also ignores Puppeteer default args `--disable-extensions`, `--disable-default-apps`, and `--disable-component-extensions-with-background-pages`. -- Proxy-related env vars only affect headless launch: `PUPPETEER_PROXY`, `PUPPETEER_PROXY_BYPASS_LOOPBACK`, and `PUPPETEER_PROXY_IGNORE_CERT_ERRORS`. +- Proxy-related env vars only affect headless launch argv (shared and local): `PUPPETEER_PROXY`, `PUPPETEER_PROXY_BYPASS_LOOPBACK`, and `PUPPETEER_PROXY_IGNORE_CERT_ERRORS`. For the shared daemon they are baked in at first launch and take effect again after the daemon's next cold start. - Stealth patches are applied only in headless mode. Spawned or externally connected browsers are intentionally left untouched. - `applyStealthPatches()` also strips Puppeteer's `//# sourceURL=__puppeteer_evaluation_script__` suffix from CDP `Runtime.evaluate` / `Runtime.callFunctionOn` payloads. - `tab.extract()` reads `page.content()`, runs Readability first, then falls back to the first non-empty of `[data-pagefind-body]`/`main article`/`article`/`main`/`[role='main']`/`body`, and returns `null` if neither extraction path yields content. diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 3f76ce653..4575d72ff 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,14 @@ ## [Unreleased] +### Changed + +- Tightened the system prompt notation: the legend now defines `⟺`, `≠`, `∉`/`∌`, and operator binding order; replaced undefined symbols (`⊭`, `≢`) in prompt bodies; removed delegation guidance duplicated between the eager-tasks preamble and the delegation gates. + +### Fixed + +- Fixed headless browser launch storms and orphaned Chromium process trees: omp processes now attach to one project-shared Chromium owned by the daemon broker (tabs per session; Chrome dies with the last omp client in the project), concurrent browser opens in one process share a single launch, and concurrent daemon `start` requests for one name can no longer spawn duplicate untracked processes. + ## [17.2.2] - 2026-07-31 ### Added diff --git a/packages/coding-agent/src/launch/broker.ts b/packages/coding-agent/src/launch/broker.ts index 53f4cf666..6cae0dacf 100644 --- a/packages/coding-agent/src/launch/broker.ts +++ b/packages/coding-agent/src/launch/broker.ts @@ -342,6 +342,15 @@ class DaemonBroker { readonly #token: string; readonly #idleGraceMs: number; readonly #records = new Map(); + /** + * Names reserved by an in-flight `start` before its record lands in + * `#records`. Requests dispatch concurrently, and `#start` awaits (cwd stat, + * log open) between the duplicate check and the record insert; without a + * synchronous reservation two clients can both pass the check and spawn + * duplicate processes — one exits on a held resource (e.g. a Chromium + * profile lock) or keeps running untracked. + */ + readonly #startingNames = new Set(); readonly #clients = new Set(); readonly #finished = Promise.withResolvers(); readonly #sockets = new Set(); @@ -500,50 +509,59 @@ class DaemonBroker { ) { throw new Error('Windows batch files require application "cmd.exe" with the batch path after "/c"'); } - const existing = this.#records.get(spec.name); - if (existing) await this.#refreshDetached(existing); - if (existing && !terminalState(existing.snapshot.state)) { - throw new Error(`Daemon ${spec.name} is already ${existing.snapshot.state}`); + if (this.#startingNames.has(spec.name)) { + throw new Error(`Daemon ${spec.name} is already starting`); } - if (spec.ready?.log) { - try { - new RegExp(spec.ready.log, "u"); - } catch (error) { - throw new Error(`Invalid readiness regex: ${error instanceof Error ? error.message : String(error)}`); + this.#startingNames.add(spec.name); + let record: ManagedDaemon; + try { + const existing = this.#records.get(spec.name); + if (existing) await this.#refreshDetached(existing); + if (existing && !terminalState(existing.snapshot.state)) { + throw new Error(`Daemon ${spec.name} is already ${existing.snapshot.state}`); } + if (spec.ready?.log) { + try { + new RegExp(spec.ready.log, "u"); + } catch (error) { + throw new Error(`Invalid readiness regex: ${error instanceof Error ? error.message : String(error)}`); + } + } + const stat = await fs.stat(spec.cwd); + if (!stat.isDirectory()) throw new Error(`Daemon cwd is not a directory: ${spec.cwd}`); + const dir = path.join(this.#runtimeDir, "daemons", spec.name); + const now = Date.now(); + record = { + spec, + snapshot: { + name: spec.name, + id: crypto.randomUUID(), + state: "starting", + createdAt: now, + startedAt: now, + restartCount: 0, + outputBytes: 0, + owner, + persist: spec.persist, + detached: spec.detached, + }, + dir, + log: await DaemonLog.open(dir), + generation: 0, + stopRequested: false, + logReady: !spec.ready?.log, + portReady: spec.ready?.port === undefined, + readinessBuffer: "", + outputOffset: 0, + readyPattern: spec.ready?.log ? new RegExp(spec.ready.log, "u") : undefined, + consecutiveFailures: 0, + persistQueue: Promise.resolve(), + }; + syncReadyPending(record); + this.#records.set(spec.name, record); + } finally { + this.#startingNames.delete(spec.name); } - const stat = await fs.stat(spec.cwd); - if (!stat.isDirectory()) throw new Error(`Daemon cwd is not a directory: ${spec.cwd}`); - const dir = path.join(this.#runtimeDir, "daemons", spec.name); - const now = Date.now(); - const record: ManagedDaemon = { - spec, - snapshot: { - name: spec.name, - id: crypto.randomUUID(), - state: "starting", - createdAt: now, - startedAt: now, - restartCount: 0, - outputBytes: 0, - owner, - persist: spec.persist, - detached: spec.detached, - }, - dir, - log: await DaemonLog.open(dir), - generation: 0, - stopRequested: false, - logReady: !spec.ready?.log, - portReady: spec.ready?.port === undefined, - readinessBuffer: "", - outputOffset: 0, - readyPattern: spec.ready?.log ? new RegExp(spec.ready.log, "u") : undefined, - consecutiveFailures: 0, - persistQueue: Promise.resolve(), - }; - syncReadyPending(record); - this.#records.set(spec.name, record); await this.#launch(record); let readyTimedOut = false; if (spec.ready && !terminalState(record.snapshot.state)) { diff --git a/packages/coding-agent/src/tools/browser.ts b/packages/coding-agent/src/tools/browser.ts index eda99d7ef..2908b6e80 100644 --- a/packages/coding-agent/src/tools/browser.ts +++ b/packages/coding-agent/src/tools/browser.ts @@ -422,7 +422,7 @@ function describeBrowser(handle: BrowserHandle): string { } switch (handle.kind.kind) { case "headless": - return `headless browser (${handle.kind.headless ? "hidden" : "visible"})`; + return `headless browser (${handle.kind.headless ? "hidden" : "visible"}${handle.sharedDaemon ? ", shared" : ""})`; case "spawned": return `spawned ${handle.kind.path} (pid ${handle.pid ?? "?"})`; case "connected": diff --git a/packages/coding-agent/src/tools/browser/launch.ts b/packages/coding-agent/src/tools/browser/launch.ts index 565296a98..c3bb19b05 100644 --- a/packages/coding-agent/src/tools/browser/launch.ts +++ b/packages/coding-agent/src/tools/browser/launch.ts @@ -296,19 +296,17 @@ export interface LaunchHeadlessResult { userDataDir?: string; } -export async function launchHeadlessBrowser(opts: LaunchHeadlessOptions): Promise { - const vp = opts.viewport ?? DEFAULT_VIEWPORT; - const initialViewport = { - width: vp.width, - height: vp.height, - deviceScaleFactor: vp.deviceScaleFactor ?? DEFAULT_VIEWPORT.deviceScaleFactor, - }; - const puppeteer = await loadPuppeteer(); +/** + * Base Chromium argv shared by process-local puppeteer launches and the + * broker-owned shared browser: sandbox/stealth flags, window size, and + * PUPPETEER_PROXY* env-derived proxy flags. + */ +export function buildHeadlessLaunchArgs(viewport: { width: number; height: number }): string[] { const launchArgs = [ "--no-sandbox", "--disable-setuid-sandbox", "--disable-blink-features=AutomationControlled", - `--window-size=${initialViewport.width},${initialViewport.height}`, + `--window-size=${viewport.width},${viewport.height}`, ]; const proxy = process.env.PUPPETEER_PROXY; if (proxy) { @@ -324,6 +322,18 @@ export async function launchHeadlessBrowser(opts: LaunchHeadlessOptions): Promis if (ignoreCert === "true" || ignoreCert === "1" || ignoreCert === "yes" || ignoreCert === "on") { launchArgs.push("--ignore-certificate-errors"); } + return launchArgs; +} + +export async function launchHeadlessBrowser(opts: LaunchHeadlessOptions): Promise { + const vp = opts.viewport ?? DEFAULT_VIEWPORT; + const initialViewport = { + width: vp.width, + height: vp.height, + deviceScaleFactor: vp.deviceScaleFactor ?? DEFAULT_VIEWPORT.deviceScaleFactor, + }; + const puppeteer = await loadPuppeteer(); + const launchArgs = buildHeadlessLaunchArgs(initialViewport); for (const arg of opts.args ?? []) { if (!launchArgs.includes(arg)) launchArgs.push(arg); } @@ -357,6 +367,41 @@ export async function launchHeadlessBrowser(opts: LaunchHeadlessOptions): Promis } } +/** Fully resolved executable and argv for a broker-spawned shared Chromium. */ +export interface SharedBrowserLaunchSpec { + executablePath: string; + args: string[]; +} + +/** + * Resolve the executable and complete argv for a shared Chromium the daemon + * broker spawns directly (no puppeteer inside the broker). Mirrors + * `launchHeadlessBrowser` flag assembly — puppeteer's default args minus the + * stealth-suppressed set — plus `--remote-debugging-port=0` so every client + * attaches over CDP. Returns null when no Chromium executable resolves; + * callers fall back to a process-local launch. + */ +export async function resolveSharedBrowserLaunchSpec(opts: { + headless: boolean; + userDataDir: string; + viewport?: { width: number; height: number }; +}): Promise { + const executablePath = await ensureChromiumExecutable(); + if (!executablePath) return null; + const puppeteer = await loadPuppeteer(); + const vp = opts.viewport ?? DEFAULT_VIEWPORT; + const ignored = new Set(stealthIgnoreDefaultArgs(executablePath)); + const defaults = await puppeteer.defaultArgs({ + headless: opts.headless, + args: buildHeadlessLaunchArgs(vp), + userDataDir: opts.userDataDir, + }); + return { + executablePath, + args: [...defaults.filter(arg => !ignored.has(arg)), "--remote-debugging-port=0"], + }; +} + /** * Remove an OMP-owned headless Chromium profile directory, tolerating the brief * window on Windows in which Chromium (or an orphaned browser subprocess) still diff --git a/packages/coding-agent/src/tools/browser/registry.ts b/packages/coding-agent/src/tools/browser/registry.ts index a7864fb85..7d6d893e6 100644 --- a/packages/coding-agent/src/tools/browser/registry.ts +++ b/packages/coding-agent/src/tools/browser/registry.ts @@ -1,5 +1,5 @@ import * as path from "node:path"; -import { logger, withTimeout } from "@oh-my-pi/pi-utils"; +import { isCompiledBinary, logger, withTimeout, workerHostEntry } from "@oh-my-pi/pi-utils"; import type { Subprocess } from "bun"; import type { Browser, CDPSession } from "puppeteer-core"; import { ToolAbortError, ToolError } from "../tool-errors"; @@ -8,11 +8,13 @@ import type { CmuxKind } from "./cmux/rpc"; import { CmuxSocketClient } from "./cmux/socket-client"; import { BROWSER_PROTOCOL_TIMEOUT_MS, + DEFAULT_VIEWPORT, launchHeadlessBrowser, loadPuppeteer, removeUserDataDir, type UserAgentOverride, } from "./launch"; +import { ensureSharedBrowser } from "./shared-daemon"; export type PuppeteerBrowserKind = | { kind: "headless"; headless: boolean } @@ -41,8 +43,10 @@ export interface PuppeteerBrowserHandle extends BrowserHandleCommon { browser: Browser; cdpUrl?: string; pid?: number; - /** OMP-owned temp Chromium profile directory removed on dispose (headless launches). */ + /** OMP-owned temp Chromium profile directory removed on dispose (process-local headless launches). */ userDataDir?: string; + /** Broker daemon backing this handle; dispose disconnects instead of closing, kill routes to the broker. */ + sharedDaemon?: { name: string; projectDir: string }; subprocess?: Subprocess; stealth: { browserSession: CDPSession | null; override: UserAgentOverride | null }; } @@ -63,6 +67,8 @@ export interface ReleaseBrowserOptions { } const browsers = new Map(); +/** In-flight opens by browser key, so concurrent acquisitions share one launch instead of storming Chromium. */ +const pendingOpens = new Map>(); function browserKey(kind: BrowserKind): string { switch (kind.kind) { @@ -86,37 +92,51 @@ export interface AcquireBrowserOptions { export async function acquireBrowser(kind: BrowserKind, opts: AcquireBrowserOptions): Promise { const key = browserKey(kind); - const existing = browsers.get(key); - if (existing) { - if ("client" in existing) return existing; - if (existing.browser.connected) return existing; - browsers.delete(key); - await disposeBrowserHandle(existing, { kill: false }); - } - // Short-circuit before launching: the tool wrapper's `untilAborted` only - // rejects its outer promise on abort; without this check `openBrowserHandle` - // would still fire and its result would land in `browsers` below. - if (opts.signal?.aborted) throw new ToolAbortError("Browser open aborted"); + for (;;) { + const existing = browsers.get(key); + if (existing) { + if ("client" in existing) return existing; + if (existing.browser.connected) return existing; + browsers.delete(key); + await disposeBrowserHandle(existing, { kill: false }); + continue; + } + // Short-circuit before launching: the tool wrapper's `untilAborted` only + // rejects its outer promise on abort; without this check `openBrowserHandle` + // would still fire and its result would land in `browsers` below. + if (opts.signal?.aborted) throw new ToolAbortError("Browser open aborted"); - const handle = await openBrowserHandle(kind, opts); - // The launch may resolve AFTER the caller has already aborted (the outer - // `untilAborted` rejects immediately on abort but does not cancel the - // inner promise, and `launchHeadlessBrowser` does not accept a signal). - // Without this branch the completed handle sits in `browsers` at - // refCount:0 forever — no tab ever takes a hold, `releaseBrowser` never - // fires, and `releaseAllTabs` walks `tabs`, not `browsers`, so the - // orphaned Chromium/app process / puppeteer handle survives to process - // exit. (Issue #3963.) - if (opts.signal?.aborted) { - await disposeBrowserHandle(handle, { kill: kind.kind === "spawned" }).catch(err => { - logger.debug("Failed to dispose orphan browser after abort", { - error: err instanceof Error ? err.message : String(err), + // Single-flight per key: a concurrent caller already opening this browser + // wins; everyone else waits and re-reads the registry. Without this, N + // simultaneous opens each launch a Chromium and the last write wins, + // leaking the rest as unreferenced process trees. + const pending = pendingOpens.get(key); + if (pending) { + await pending.catch(() => undefined); + continue; + } + const open = openBrowserHandle(kind, opts).finally(() => pendingOpens.delete(key)); + pendingOpens.set(key, open); + const handle = await open; + // The launch may resolve AFTER the caller has already aborted (the outer + // `untilAborted` rejects immediately on abort but does not cancel the + // inner promise, and `launchHeadlessBrowser` does not accept a signal). + // Without this branch the completed handle sits in `browsers` at + // refCount:0 forever — no tab ever takes a hold, `releaseBrowser` never + // fires, and `releaseAllTabs` walks `tabs`, not `browsers`, so the + // orphaned Chromium/app process / puppeteer handle survives to process + // exit. (Issue #3963.) + if (opts.signal?.aborted) { + await disposeBrowserHandle(handle, { kill: kind.kind === "spawned" }).catch(err => { + logger.debug("Failed to dispose orphan browser after abort", { + error: err instanceof Error ? err.message : String(err), + }); }); - }); - throw new ToolAbortError("Browser open aborted"); + throw new ToolAbortError("Browser open aborted"); + } + browsers.set(key, handle); + return handle; } - browsers.set(key, handle); - return handle; } export function normalizeConnectedCdpUrl(rawCdpUrl: string): string { @@ -142,6 +162,14 @@ async function openBrowserHandle(kind: BrowserKind, opts: AcquireBrowserOptions) }; } if (kind.kind === "headless") { + // Every real omp process (session, subagent, worker — anything with a CLI + // worker host) MUST go through the project-shared broker-owned Chromium: + // per-process launches are what produced launch storms and orphaned + // process trees. The process-local launch survives only for hosts that + // cannot spawn the broker (bun test, SDK embedding without a CLI entry). + if (isCompiledBinary() || workerHostEntry() !== null) { + return await openSharedHeadlessHandle(kind, opts); + } const { browser, userDataDir } = await launchHeadlessBrowser({ headless: kind.headless, viewport: opts.viewport, @@ -257,6 +285,21 @@ async function disposeBrowserHandle(handle: BrowserHandle, opts: ReleaseBrowserO return; } if (handle.kind.kind === "headless") { + if (handle.sharedDaemon) { + // The broker owns the Chromium; this process only drops its CDP + // connection. `kill` is scoped to spawned-app browsers — stopping the + // shared daemon here would tear down every other session's tabs. The + // daemon dies with the last omp client in the project (broker idle + // teardown), or via an explicit hub stop. + if (handle.browser.connected) { + try { + handle.browser.disconnect(); + } catch (err) { + logger.debug("Failed to disconnect from shared browser", { error: (err as Error).message }); + } + } + return; + } if (handle.browser.connected) { // Puppeteer's `browser.close()` resolves only once the Chromium // process fully exits. A wedged Chromium (a known Windows failure @@ -297,6 +340,56 @@ async function disposeBrowserHandle(handle: BrowserHandle, opts: ReleaseBrowserO if (opts.kill && handle.pid !== undefined) await gracefulKillTreeOnce(handle.pid); } +/** + * Attach to the project-shared broker-owned Chromium. Failures surface as + * `ToolError` — a CLI-host process never silently falls back to a private + * Chromium, so a broken broker cannot quietly recreate per-process launch + * storms. + */ +async function openSharedHeadlessHandle( + kind: Extract, + opts: AcquireBrowserOptions, +): Promise { + const vp = opts.viewport ?? DEFAULT_VIEWPORT; + try { + const shared = await ensureSharedBrowser({ + projectDir: opts.cwd, + headless: kind.headless, + viewport: vp, + signal: opts.signal, + }); + if (!shared) { + throw new ToolError( + "Shared browser daemon unavailable (broker start or Chromium launch failed); check `hub ps` for omp.browser.* daemons and ~/.omp/logs for details", + ); + } + const puppeteer = await loadPuppeteer(); + const browser = await puppeteer.connect({ + browserWSEndpoint: shared.wsEndpoint, + defaultViewport: kind.headless + ? { + width: vp.width, + height: vp.height, + deviceScaleFactor: vp.deviceScaleFactor ?? DEFAULT_VIEWPORT.deviceScaleFactor, + } + : null, + protocolTimeout: BROWSER_PROTOCOL_TIMEOUT_MS, + }); + return { + key: browserKey(kind), + kind, + browser, + sharedDaemon: { name: shared.daemonName, projectDir: shared.projectDir }, + refCount: 0, + stealth: { browserSession: null, override: null }, + }; + } catch (err) { + if (err instanceof ToolAbortError || err instanceof ToolError) throw err; + if (opts.signal?.aborted) throw new ToolAbortError("Browser open aborted"); + throw new ToolError(`Shared browser attach failed: ${err instanceof Error ? err.message : String(err)}`); + } +} + /** Test-only accessor for the module-global browsers map. */ export function getBrowsersMapForTest(): ReadonlyMap { return browsers; diff --git a/packages/coding-agent/src/tools/browser/shared-daemon.ts b/packages/coding-agent/src/tools/browser/shared-daemon.ts new file mode 100644 index 000000000..c9cd40703 --- /dev/null +++ b/packages/coding-agent/src/tools/browser/shared-daemon.ts @@ -0,0 +1,190 @@ +/** + * Shared automation Chromium owned by the per-project daemon broker. + * + * Instead of every omp process launching (and sometimes orphaning) a private + * Chromium, the headless browser kind attaches to one broker-supervised Chrome + * per project directory — sessions and subagents each open their own tabs in + * it. The broker stops the daemon when the last omp client in the project + * exits, so Chrome can never outlive omp, and concurrent acquisitions across + * processes converge on a single launch instead of a launch storm. + */ +import * as fs from "node:fs/promises"; +import * as path from "node:path"; +import { logger } from "@oh-my-pi/pi-utils"; +import { type DaemonBrokerClient, daemonClientForProject } from "../../launch/client"; +import { daemonRuntimeDir } from "../../launch/paths"; +import type { DaemonSnapshot } from "../../launch/protocol"; +import { throwIfAborted } from "../tool-errors"; +import { resolveSharedBrowserLaunchSpec } from "./launch"; + +/** Chrome prints this on stderr once the CDP listener is up; the broker's ready probe captures the line. */ +const READY_LOG_PATTERN = String.raw`DevTools listening on ws://\S+`; +const READY_TIMEOUT_MS = 30_000; +const STOP_TIMEOUT_MS = 5_000; +const PROBE_TIMEOUT_MS = 1_500; +/** describe→start rounds before giving up; bounds cross-process start races and wedged-Chrome replacement. */ +const ENSURE_ATTEMPTS = 3; + +/** Broker-owned browser endpoint one omp process can attach to. */ +export interface SharedBrowserEndpoint { + wsEndpoint: string; + daemonName: string; + /** Canonical project directory owning the broker (used to address later stop requests). */ + projectDir: string; +} + +/** Stable broker daemon name for the shared automation browser. */ +export function sharedBrowserDaemonName(headless: boolean): string { + return headless ? "omp.browser.headless" : "omp.browser.headed"; +} + +function wsEndpointOf(snapshot: DaemonSnapshot | undefined): string | undefined { + return snapshot?.readyMatch?.match(/ws:\/\/\S+/)?.[0]; +} + +/** CDP liveness probe: the ws endpoint host must answer /json/version. */ +async function probeEndpoint(wsEndpoint: string): Promise { + let host: string; + try { + host = new URL(wsEndpoint).host; + } catch { + return false; + } + try { + const res = await fetch(`http://${host}/json/version`, { signal: AbortSignal.timeout(PROBE_TIMEOUT_MS) }); + await res.body?.cancel(); + return res.ok; + } catch { + return false; + } +} + +/** Snapshot the daemon, treating "unknown daemon" as absent. */ +async function describeQuietly( + client: DaemonBrokerClient, + name: string, + signal?: AbortSignal, +): Promise { + try { + const result = await client.request({ op: "describe", name }, signal); + return result.op === "describe" ? result.daemon : undefined; + } catch (error) { + throwIfAborted(signal); + logger.debug("Shared browser describe failed", { + name, + error: error instanceof Error ? error.message : String(error), + }); + return undefined; + } +} + +/** Block until the daemon reports ready; undefined on timeout or pre-ready exit. */ +async function waitReady( + client: DaemonBrokerClient, + name: string, + signal?: AbortSignal, +): Promise { + try { + const result = await client.request({ op: "wait", name, for: "ready", timeoutMs: READY_TIMEOUT_MS }, signal); + if (result.op !== "wait" || result.timedOut) return undefined; + return result.daemon; + } catch (error) { + throwIfAborted(signal); + logger.debug("Shared browser ready wait failed", { + name, + error: error instanceof Error ? error.message : String(error), + }); + return undefined; + } +} + +/** Best-effort stop before replacing a wedged or endpoint-less daemon. */ +async function stopQuietly(client: DaemonBrokerClient, name: string, signal?: AbortSignal): Promise { + try { + await client.request({ op: "stop", name, timeoutMs: STOP_TIMEOUT_MS }, signal); + } catch (error) { + throwIfAborted(signal); + logger.debug("Shared browser stop failed", { + name, + error: error instanceof Error ? error.message : String(error), + }); + } +} + +/** + * Ensure the project-shared automation Chromium is running and reachable, + * launching it under the daemon broker when needed. Idempotent across + * processes: losers of the start race adopt the winner's endpoint on the next + * describe round. Returns null when the shared path is unavailable (no + * resolvable Chromium, broker failure, or a daemon that never becomes + * reachable); callers fall back to a process-local launch. + */ +export async function ensureSharedBrowser(opts: { + projectDir: string; + headless: boolean; + viewport?: { width: number; height: number }; + signal?: AbortSignal; +}): Promise { + const client = await daemonClientForProject(opts.projectDir); + const name = sharedBrowserDaemonName(opts.headless); + // Stable profile under the broker's runtime dir: reused across launches, and + // never contended by pre-daemon Chromiums that used throwaway temp profiles. + const userDataDir = path.join(daemonRuntimeDir(client.projectDir), `${name}.profile`); + const launch = await resolveSharedBrowserLaunchSpec({ + headless: opts.headless, + userDataDir, + viewport: opts.viewport, + }); + if (!launch) return null; + await fs.mkdir(userDataDir, { recursive: true }); + for (let attempt = 0; attempt < ENSURE_ATTEMPTS; attempt++) { + throwIfAborted(opts.signal); + const existing = await describeQuietly(client, name, opts.signal); + if (existing && existing.state !== "exited" && existing.state !== "failed") { + const settled = existing.readyAt !== undefined ? existing : await waitReady(client, name, opts.signal); + const wsEndpoint = wsEndpointOf(settled); + if (wsEndpoint && (await probeEndpoint(wsEndpoint))) { + return { wsEndpoint, daemonName: name, projectDir: client.projectDir }; + } + // Live record but unreachable Chrome (wedged, or readiness never + // matched): replace it rather than handing out a dead endpoint. + await stopQuietly(client, name, opts.signal); + continue; + } + try { + const started = await client.request( + { + op: "start", + spec: { + name, + application: launch.executablePath, + args: launch.args, + env: {}, + cwd: client.projectDir, + pty: false, + ready: { log: READY_LOG_PATTERN, timeoutMs: READY_TIMEOUT_MS }, + restart: "no", + persist: false, + detached: false, + }, + }, + opts.signal, + ); + if (started.op !== "start") continue; + const wsEndpoint = started.readyTimedOut ? undefined : wsEndpointOf(started.daemon); + if (wsEndpoint && (await probeEndpoint(wsEndpoint))) { + return { wsEndpoint, daemonName: name, projectDir: client.projectDir }; + } + await stopQuietly(client, name, opts.signal); + } catch (error) { + throwIfAborted(opts.signal); + // Lost a cross-process start race ("already starting/ready"); the next + // describe round adopts the winner's endpoint. + logger.debug("Shared browser start contention", { + name, + error: error instanceof Error ? error.message : String(error), + }); + } + } + return null; +}