diff --git a/docs/tools/browser.md b/docs/tools/browser.md index 748d0b71e..e1c7d1cfc 100644 --- a/docs/tools/browser.md +++ b/docs/tools/browser.md @@ -251,7 +251,7 @@ The tool returns one result per call; no streaming partial output is emitted fro - Use `read` for static URLs; use `browser` when JavaScript execution, authentication, or interaction is required. A tab must be opened before `run`, and named tabs persist until closed. - `run` code has full Node/Bun and session-tool access; it is not sandboxed. - `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 resolves its executable in this order: `PUPPETEER_EXECUTABLE_PATH` always wins; otherwise, on macOS the isolated Chrome for Testing binary (`com.google.chrome.for.testing`) is preferred over a detected system Chrome and downloaded on first use, falling back to system Chrome only when Chrome for Testing cannot be obtained (a headless daemon launched from a system `Google Chrome.app` bundle shares its `com.google.Chrome` LaunchServices identity, so macOS can route the user's link clicks to the daemon — #8673). On other platforms a detected system Chrome/Chromium is preferred, then a downloaded Chrome for Testing. - 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 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. diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 52f26cb93..fb217e456 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -45,6 +45,9 @@ - Fixed transient Anthropic failures (`overloaded_error`, `rate_limit_error`, 429/500/502/503/529) aborting or silently degrading side-effect-free oneshot LLM calls. Session title generation, TTS speech enhancement, commit-message generation, the auto-thinking and unexpected-stop classifiers, memory extraction/consolidation, the commit analysis/summary/changelog/map/reduce passes, and the mnemopi LLM callback now retry with backoff that honors `retry-after`, instead of failing on the first blip or returning `null` — which made a transient overload indistinguishable from a legitimate empty result. - Fixed the commit analysis, summary, changelog and reduce passes feeding a provider error message straight into their response parsers: they never checked `stopReason`, so a failed request produced garbage output instead of surfacing the error. - Fixed the commit map phase never retrying transient failures: its retry helper only caught thrown errors, so a `stopReason: "error"` response bypassed it entirely. +### Fixed + +- Fixed the shared headless browser daemon launching from the macOS system Google Chrome bundle (`com.google.Chrome`), which let macOS LaunchServices route the user's link clicks to the automation daemon and silently swallow them; the daemon now prefers the isolated Chrome for Testing binary (`com.google.chrome.for.testing`) on macOS and falls back to system Chrome only when Chrome for Testing cannot be obtained ([#8673](https://github.com/can1357/oh-my-pi/issues/8673)). ## [17.3.4] - 2026-08-14 diff --git a/packages/coding-agent/src/tools/browser/launch.ts b/packages/coding-agent/src/tools/browser/launch.ts index 4c10635e1..31fbf32d7 100644 --- a/packages/coding-agent/src/tools/browser/launch.ts +++ b/packages/coding-agent/src/tools/browser/launch.ts @@ -121,23 +121,36 @@ async function loadBrowsers(): Promise { } /** - * Resolve the Chromium executable puppeteer will launch, honoring - * PUPPETEER_EXECUTABLE_PATH before system browser detection and lazily - * downloading Chromium otherwise. The browser is cached under - * ~/.omp/puppeteer (getPuppeteerDir). Returns undefined when platform - * detection fails (puppeteer default resolution takes over). Exported so - * real-browser tests can probe launchability and skip on hosts missing - * Chrome's system libraries. + * Resolve the Chromium executable puppeteer will launch. + * + * `PUPPETEER_EXECUTABLE_PATH` always wins. On macOS the isolated Chrome for + * Testing binary is preferred over a detected system Chrome: a headless + * daemon launched from a system `Google Chrome.app` bundle shares its + * LaunchServices bundle identity (`com.google.Chrome`), so macOS can deliver + * the user's open-URL Apple Events to the daemon and silently swallow their + * link clicks (#8673). Chrome for Testing uses a dedicated bundle id + * (`com.google.chrome.for.testing`) that is never a user's default handler; + * system Chrome is used on macOS only when Chrome for Testing cannot be + * obtained. Other platforms keep the download-avoiding system Chrome + * preference and fall back to Chrome for Testing. The managed browser is + * cached under ~/.omp/puppeteer (getPuppeteerDir). Returns undefined when + * platform detection fails (puppeteer default resolution takes over). + * Exported so real-browser tests can probe launchability and skip on hosts + * missing Chrome's system libraries. */ let chromiumExecutablePromise: Promise | undefined; export async function ensureChromiumExecutable(): Promise { const envPath = process.env.PUPPETEER_EXECUTABLE_PATH; if (envPath) return envPath; - const sysChrome = await resolveSystemChromium(); - if (sysChrome) return sysChrome; - if (chromiumExecutablePromise) return chromiumExecutablePromise; + // macOS: never route a background daemon through the user's GUI Chrome + // bundle; prefer the isolated Chrome for Testing binary instead (#8673). + const preferManagedChromium = process.platform === "darwin"; + if (!preferManagedChromium) { + const sysChrome = await resolveSystemChromium(); + if (sysChrome) return sysChrome; + } - chromiumExecutablePromise = (async () => { + chromiumExecutablePromise ??= (async () => { const browsers = await loadBrowsers(); const platform = browsers.detectBrowserPlatform(); if (!platform) { @@ -185,7 +198,23 @@ export async function ensureChromiumExecutable(): Promise { "Set PUPPETEER_EXECUTABLE_PATH to use an existing Chrome/Chromium binary, or install one manually.", ); }); - return chromiumExecutablePromise; + + try { + return await chromiumExecutablePromise; + } catch (err) { + if (!preferManagedChromium) throw err; + // Chrome for Testing could not be obtained on macOS; degrade to the + // system Chrome bundle rather than leaving the browser tool unusable. + const sysChrome = await resolveSystemChromium(); + if (!sysChrome) throw err; + logger.warn( + "Chrome for Testing unavailable; falling back to the system Chrome bundle. On macOS this can let the " + + "headless browser daemon capture your link clicks (#8673). Set PUPPETEER_EXECUTABLE_PATH to a " + + "dedicated Chromium to avoid this.", + { path: sysChrome, error: (err as Error).message }, + ); + return sysChrome; + } } let resolvedChromium: string | null | undefined; // undefined = unchecked; null = not found diff --git a/packages/coding-agent/test/tools/browser-launch.test.ts b/packages/coding-agent/test/tools/browser-launch.test.ts index c168a7517..fafa5eaea 100644 --- a/packages/coding-agent/test/tools/browser-launch.test.ts +++ b/packages/coding-agent/test/tools/browser-launch.test.ts @@ -8,6 +8,9 @@ import { systemChromiumCandidatesForTest, } from "@oh-my-pi/pi-coding-agent/tools/browser/launch"; import { TempDir } from "@oh-my-pi/pi-utils"; +import { Browser, computeExecutablePath, detectBrowserPlatform, resolveBuildId } from "@oh-my-pi/pi-utils/browsers"; +import { APP_NAME } from "@oh-my-pi/pi-utils/dirs"; +import { PUPPETEER_REVISIONS } from "puppeteer-core/internal/revisions.js"; const EXECUTABLE_PROBE = path.resolve(import.meta.dir, "../fixtures/browser-executable-probe.ts"); @@ -192,4 +195,54 @@ describe("browser executable selection", () => { await tempDir.remove(); } }); + + it("prefers Chrome for Testing over a detected system Chrome on macOS (#8673)", async () => { + const tempDir = TempDir.createSync("@browser-macos-cft-"); + try { + const home = path.join(tempDir.path(), "home"); + const xdgCache = path.join(tempDir.path(), "cache"); + // resolveIf() (packages/utils/src/dirs.ts) only redirects to an XDG + // root when its `/omp` dir already exists, so create them to pin + // the child's puppeteer cache to this isolated location. + for (const xdg of [xdgCache, path.join(tempDir.path(), "data"), path.join(tempDir.path(), "state")]) { + fs.mkdirSync(path.join(xdg, APP_NAME), { recursive: true }); + } + const env = { + ...process.env, + HOME: home, + XDG_CACHE_HOME: xdgCache, + XDG_DATA_HOME: path.join(tempDir.path(), "data"), + XDG_STATE_HOME: path.join(tempDir.path(), "state"), + OMP_BROWSER_PROBE_PLATFORM: "darwin", + PUPPETEER_EXECUTABLE_PATH: "", + }; + + // System Google Chrome bundle (com.google.Chrome) — the LaunchServices + // hijacker the fix must avoid selecting. + const systemChrome = path.join(home, "Applications/Google Chrome.app/Contents/MacOS/Google Chrome"); + await Bun.write(systemChrome, "#!/bin/sh\necho 'Google Chrome 151'\n"); + fs.chmodSync(systemChrome, 0o755); + + // Seed the isolated Chrome for Testing binary in the child's cache so the + // probe resolves it without a network download. getPuppeteerDir() resolves + // to `/omp/puppeteer` given the dirs created above. + const cacheDir = path.join(xdgCache, APP_NAME, "puppeteer"); + const platform = detectBrowserPlatform(); + if (!platform) throw new Error("unsupported host platform for Chrome-for-Testing selection test"); + const buildId = await resolveBuildId(Browser.CHROME, platform, PUPPETEER_REVISIONS.chrome); + const chromeForTesting = computeExecutablePath({ browser: Browser.CHROME, buildId, cacheDir, platform }); + await Bun.write(chromeForTesting, "#!/bin/sh\necho 'Chrome for Testing'\n"); + fs.chmodSync(chromeForTesting, 0o755); + + const result = Bun.spawnSync([process.execPath, EXECUTABLE_PROBE], { env, stdout: "pipe", stderr: "pipe" }); + const stderr = new TextDecoder().decode(result.stderr); + + expect(result.exitCode, stderr).toBe(0); + const selected = new TextDecoder().decode(result.stdout); + expect(selected).toBe(chromeForTesting); + expect(selected).not.toBe(systemChrome); + } finally { + await tempDir.remove(); + } + }); });