From e287a8c6e2a4c75ab29e1acac4fca391ed89686f Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 14 Aug 2026 16:29:41 +0000 Subject: [PATCH] fix(browser): probe CDP endpoints over raw TCP to bypass HTTP_PROXY The shared-browser CDP liveness probes (probeEndpoint, waitForCdp, probeCdpAt) used a bare fetch() against the loopback DevTools endpoint. Bun's fetch honors HTTP_PROXY/HTTPS_PROXY and forwards even 127.0.0.1 requests to the proxy unless NO_PROXY covers them, so a local proxy (e.g. Clash) that 502s internal addresses made a healthy daemon look dead and ensureSharedBrowser tore it down. Replace the three probes with probeCdpStatus(), a raw-TCP HTTP/1.1 GET that never routes through a proxy and resolves to the response status (or null on unreachable/aborted/timeout). Fixes #8567 --- packages/coding-agent/CHANGELOG.md | 4 + .../coding-agent/src/tools/browser/attach.ts | 105 +++++++++++++----- .../src/tools/browser/shared-daemon.ts | 10 +- .../test/tools/browser-attach.test.ts | 55 +++++++++ 4 files changed, 142 insertions(+), 32 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 5597d222b..fe127c92c 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `browser open` failing with "Shared browser daemon unavailable" when `HTTP_PROXY`/`HTTPS_PROXY` is set (e.g. a local Clash proxy), because the shared-browser CDP liveness probes routed loopback `127.0.0.1` requests through the proxy, which 502'd them and killed the healthy daemon. The probes now talk to the endpoint over raw TCP and never touch a proxy ([#8567](https://github.com/can1357/oh-my-pi/issues/8567)). + ## [17.3.4] - 2026-08-14 ### Changed diff --git a/packages/coding-agent/src/tools/browser/attach.ts b/packages/coding-agent/src/tools/browser/attach.ts index 15330be6e..b1db5d764 100644 --- a/packages/coding-agent/src/tools/browser/attach.ts +++ b/packages/coding-agent/src/tools/browser/attach.ts @@ -1,5 +1,6 @@ import * as net from "node:net"; import { Process, ProcessStatus } from "@oh-my-pi/pi-natives"; +import type { Socket } from "bun"; import type { Browser, Page } from "puppeteer-core"; import { ToolError, throwIfAborted } from "../tool-errors"; @@ -29,31 +30,92 @@ export async function findFreeCdpPort(): Promise { return promise; } +/** + * Loopback HTTP/1.1 GET that never routes through a proxy, resolving to the + * response status code (or null when the endpoint is unreachable, aborted, + * malformed, or slow past `timeoutMs`). + * + * Chrome's DevTools endpoint listens on loopback and speaks plain HTTP/1.1. + * Both `fetch` and Bun's `node:http` honor `HTTP_PROXY`/`HTTPS_PROXY` and + * forward even `127.0.0.1` requests to the proxy unless `NO_PROXY` covers them, + * so a local proxy that 502s internal addresses makes a healthy daemon look + * dead and the CDP readiness checks tear it down (issue #8567). Talking to the + * socket over raw TCP sidesteps proxy env entirely. + */ +export async function probeCdpStatus( + url: string, + opts: { timeoutMs: number; signal?: AbortSignal }, +): Promise { + let target: URL; + try { + target = new URL(url); + } catch { + return null; + } + if (opts.signal?.aborted) return null; + const port = target.port ? Number(target.port) : 80; + const requestPath = `${target.pathname}${target.search}` || "/"; + const { promise, resolve } = Promise.withResolvers(); + let socket: Socket | undefined; + let settled = false; + const finish = (status: number | null) => { + if (settled) return; + settled = true; + clearTimeout(timer); + opts.signal?.removeEventListener("abort", onAbort); + try { + socket?.end(); + } catch { + // socket already torn down + } + resolve(status); + }; + const onAbort = () => finish(null); + const timer = setTimeout(() => finish(null), opts.timeoutMs); + opts.signal?.addEventListener("abort", onAbort, { once: true }); + let buffered = ""; + try { + socket = await Bun.connect({ + hostname: target.hostname, + port, + socket: { + open(s) { + s.write(`GET ${requestPath} HTTP/1.1\r\nHost: ${target.hostname}:${port}\r\nConnection: close\r\n\r\n`); + }, + data(_s, chunk) { + buffered += chunk.toString("latin1"); + const match = /^HTTP\/\d(?:\.\d)? (\d{3})/.exec(buffered); + if (match) finish(Number(match[1])); + }, + error() { + finish(null); + }, + close() { + finish(null); + }, + }, + }); + } catch { + finish(null); + } + return promise; +} + /** Poll `${cdpUrl}/json/version` until it responds with 200, with abort + timeout support. */ export async function waitForCdp(cdpUrl: string, timeoutMs: number, signal?: AbortSignal): Promise { const deadline = Date.now() + timeoutMs; - let lastErr: unknown; const probeUrl = `${cdpUrl.replace(/\/+$/, "")}/json/version`; + let lastStatus: number | null = null; while (Date.now() < deadline) { throwIfAborted(signal); - const probeTimeout = AbortSignal.timeout(2000); - const probeSignal = signal ? AbortSignal.any([signal, probeTimeout]) : probeTimeout; - try { - const res = await fetch(probeUrl, { signal: probeSignal }); - if (res.ok) { - await res.body?.cancel(); - return; - } - lastErr = new Error(`HTTP ${res.status}`); - await res.body?.cancel(); - } catch (err) { - if (signal?.aborted) throwIfAborted(signal); - lastErr = err; - } + const status = await probeCdpStatus(probeUrl, { timeoutMs: 2000, signal }); + if (status !== null && status >= 200 && status < 300) return; + lastStatus = status; await Bun.sleep(150); } + throwIfAborted(signal); throw new ToolError( - `Timed out waiting for CDP endpoint ${cdpUrl}${lastErr instanceof Error ? `: ${lastErr.message}` : ""}`, + `Timed out waiting for CDP endpoint ${cdpUrl}${lastStatus !== null ? `: HTTP ${lastStatus}` : ""}`, ); } @@ -81,15 +143,8 @@ function findCdpPortInArgs(args: string[]): number | null { /** One-shot probe: returns true when `/json/version` answers 200 within the timeout. */ async function probeCdpAt(port: number, signal?: AbortSignal): Promise { - const probeTimeout = AbortSignal.timeout(1500); - const probeSignal = signal ? AbortSignal.any([signal, probeTimeout]) : probeTimeout; - try { - const res = await fetch(`http://127.0.0.1:${port}/json/version`, { signal: probeSignal }); - await res.body?.cancel(); - return res.ok; - } catch { - return false; - } + const status = await probeCdpStatus(`http://127.0.0.1:${port}/json/version`, { timeoutMs: 1500, signal }); + return status !== null && status >= 200 && status < 300; } /** diff --git a/packages/coding-agent/src/tools/browser/shared-daemon.ts b/packages/coding-agent/src/tools/browser/shared-daemon.ts index fc98b488f..c26fd05ba 100644 --- a/packages/coding-agent/src/tools/browser/shared-daemon.ts +++ b/packages/coding-agent/src/tools/browser/shared-daemon.ts @@ -16,6 +16,7 @@ import { describeQuietly, stopQuietly, waitReady } from "../../launch/ensure"; import { daemonRuntimeDir } from "../../launch/paths"; import type { DaemonSnapshot } from "../../launch/protocol"; import { throwIfAborted } from "../tool-errors"; +import { probeCdpStatus } from "./attach"; import { resolveSharedBrowserLaunchSpec } from "./launch"; /** Chrome prints this on stderr once the CDP listener is up; the broker's ready probe captures the line. */ @@ -50,13 +51,8 @@ async function probeEndpoint(wsEndpoint: string): Promise { } 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; - } + const status = await probeCdpStatus(`http://${host}/json/version`, { timeoutMs: PROBE_TIMEOUT_MS }); + return status !== null && status >= 200 && status < 300; } /** diff --git a/packages/coding-agent/test/tools/browser-attach.test.ts b/packages/coding-agent/test/tools/browser-attach.test.ts index 2a59cf5cb..8df76ec5c 100644 --- a/packages/coding-agent/test/tools/browser-attach.test.ts +++ b/packages/coding-agent/test/tools/browser-attach.test.ts @@ -1,6 +1,8 @@ import { afterAll, beforeAll, describe, expect, test } from "bun:test"; import { + findFreeCdpPort, pickElectronTarget, + probeCdpStatus, shouldPreserveConnectedBrowserFocus, } from "@oh-my-pi/pi-coding-agent/tools/browser/attach"; import { @@ -191,3 +193,56 @@ describe("pickElectronTarget", () => { 30_000, ); }); + +describe("probeCdpStatus", () => { + // Regression for #8567: a local proxy (Clash, corporate) 502s internal + // loopback addresses, so a bare fetch()/node:http probe misreports a healthy + // CDP daemon as dead. The raw-TCP probe must ignore HTTP_PROXY entirely. + test("returns the loopback status even when HTTP_PROXY 502s the request", async () => { + const cdp = Bun.serve({ port: 0, fetch: () => new Response("{}", { status: 200 }) }); + const proxy = Bun.serve({ port: 0, fetch: () => new Response("Bad Gateway", { status: 502 }) }); + const saved = { HTTP_PROXY: process.env.HTTP_PROXY, http_proxy: process.env.http_proxy }; + process.env.HTTP_PROXY = `http://127.0.0.1:${proxy.port}`; + process.env.http_proxy = `http://127.0.0.1:${proxy.port}`; + try { + const status = await probeCdpStatus(`http://127.0.0.1:${cdp.port}/json/version`, { timeoutMs: 1500 }); + expect(status).toBe(200); + } finally { + process.env.HTTP_PROXY = saved.HTTP_PROXY; + process.env.http_proxy = saved.http_proxy; + if (saved.HTTP_PROXY === undefined) delete process.env.HTTP_PROXY; + if (saved.http_proxy === undefined) delete process.env.http_proxy; + await cdp.stop(true); + await proxy.stop(true); + } + }); + + test("surfaces a non-2xx status from a live endpoint", async () => { + const server = Bun.serve({ port: 0, fetch: () => new Response("nope", { status: 503 }) }); + try { + const status = await probeCdpStatus(`http://127.0.0.1:${server.port}/json/version`, { timeoutMs: 1500 }); + expect(status).toBe(503); + } finally { + await server.stop(true); + } + }); + + test("returns null when the endpoint is unreachable", async () => { + const port = await findFreeCdpPort(); + const status = await probeCdpStatus(`http://127.0.0.1:${port}/json/version`, { timeoutMs: 500 }); + expect(status).toBeNull(); + }); + + test("returns null when the request is already aborted", async () => { + const server = Bun.serve({ port: 0, fetch: () => new Response("{}", { status: 200 }) }); + try { + const status = await probeCdpStatus(`http://127.0.0.1:${server.port}/json/version`, { + timeoutMs: 1500, + signal: AbortSignal.abort(), + }); + expect(status).toBeNull(); + } finally { + await server.stop(true); + } + }); +});