Merge PR #8677: fix(browser): prefer Chrome for Testing on macOS headless launch (@roboomp)

This commit is contained in:
can1357
2026-08-16 02:43:30 +02:00
4 changed files with 98 additions and 13 deletions
+1 -1
View File
@@ -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.
+3
View File
@@ -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
@@ -121,23 +121,36 @@ async function loadBrowsers(): Promise<typeof BrowsersNs> {
}
/**
* 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<string | undefined> | undefined;
export async function ensureChromiumExecutable(): Promise<string | undefined> {
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<string | undefined> {
"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
@@ -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 `<XDG>/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 `<XDG_CACHE_HOME>/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();
}
});
});