From 95ff5e8eaefb35d79109f7ddfa1de5bf45a8daff Mon Sep 17 00:00:00 2001 From: Igor Soarez Date: Tue, 28 Jul 2026 22:08:44 +0100 Subject: feat(cdp): default to the shared browser, and say so in /browser-status Two follow-ups to BU_CDP_HTTP. Default the endpoint. The remote host is now a built-in default (DEFAULT_REMOTE_CDP), so nothing has to export BU_CDP_HTTP just to get the usual setup. The variable becomes an override rather than a requirement. Precedence is deliberately asymmetric, in src/cdp/endpoint.ts: BU_CDP_HTTP=host:port that endpoint; unreachable is a hard error BU_CDP_HTTP=local|off upstream behaviour, this machine's own browser unset the default, falling back to local when unreachable An explicitly configured endpoint is an instruction, so failing to reach it must not quietly automate a different browser - that would drive the operator's own signed-in Chrome instead of the shared one. The built-in default is only a preference, so a laptop away from the network still gets the local browser the extension was originally written for. Resolution is pure, with no I/O, so the status command can report the configured intent without touching the network and the daemon can resolve the same way in its own process. /browser-status no longer reports a bare "Disconnected". The connection is lazy - nothing dials the browser until the first browser_* tool runs - so a fresh session always showed a red light that reads as a fault. It now distinguishes "not connected yet, connects on first use" from "target unreachable", and names the target with a reachability check, so the common confusion answers itself. 26 tests for the resolution table, covering the aliases, malformed values that must be rejected rather than coerced, and that the default constant is itself well-formed - if it ever stopped parsing, every machine would silently fall back to local. --- src/cdp/discovery.ts | 49 ++++++++++++----------- src/cdp/endpoint.ts | 84 ++++++++++++++++++++++++++++++++++++++++ src/index.ts | 28 +++++++++++++- test/cdp/endpoint-test.ts | 99 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 237 insertions(+), 23 deletions(-) create mode 100644 src/cdp/endpoint.ts create mode 100644 test/cdp/endpoint-test.ts diff --git a/src/cdp/discovery.ts b/src/cdp/discovery.ts index 41bcfe5..53d4ad2 100644 --- a/src/cdp/discovery.ts +++ b/src/cdp/discovery.ts @@ -6,6 +6,7 @@ import { type Result, err, ok } from "../util/result"; import { userDataDirCandidates } from "../profile/paths"; import { type CdpError, cdpError } from "./errors"; import { asString, errnoCode, isRecord } from "../util/guards"; +import { describeCdpTarget, resolveCdpTarget } from "./endpoint"; const PORT_PROBE_DEADLINE_MS = 30_000; const PORT_PROBE_INTERVAL_MS = 1_000; @@ -63,7 +64,7 @@ const waitForPort = async (port: number): Promise> => { const isPortLive = async (port: number): Promise => (await probePort(port)).success; // /json/version is authoritative: a stale DevToolsActivePort file may name a browser that has since exited while another now owns the port. -const queryLiveWsUrl = (port: number, timeoutMs = 1_500, host = "127.0.0.1"): Promise => +export const queryLiveWsUrl = (port: number, timeoutMs = 1_500, host = "127.0.0.1"): Promise => new Promise((resolve) => { const req = httpRequest( { host, port, path: "/json/version", method: "GET", timeout: timeoutMs }, @@ -114,33 +115,37 @@ export const discoverEndpoint = async (): Promise> const override = endpointFromEnv(); if (override) return ok(override); - // LOCAL PATCH: direct remote browser support. + // FORK: drive a browser on another host. See src/cdp/endpoint.ts for the + // precedence rules and FORK.md for why this exists at all. // - // BU_CDP_HTTP="host:port" points at a CDP *HTTP* endpoint on another machine. - // We ask it for its canonical webSocketDebuggerUrl on every discovery call, - // so the browser UUID is re-read after the remote Chrome restarts. That is - // the difference from the stock BU_CDP_WS (checked just above, so an explicit - // WebSocket URL still wins), which pins a UUID that Chrome re-mints on every - // launch. + // The remote is asked for its canonical webSocketDebuggerUrl on every + // discovery call, so the browser UUID is re-read after the remote browser + // restarts. That is the difference from upstream's BU_CDP_WS (checked just + // above, so an explicit WebSocket URL still wins), which pins a UUID that + // Chrome re-mints on every launch. // - // Returns no userDataDir: the profile registry lives on the remote host, so - // profile enumeration correctly degrades to the pre-profile behaviour. - const remote = process.env["BU_CDP_HTTP"]?.trim(); - if (remote) { - const sep = remote.lastIndexOf(":"); - const host = sep === -1 ? remote : remote.slice(0, sep); - const port = sep === -1 ? 9222 : Number(remote.slice(sep + 1)); - if (!host || !Number.isInteger(port) || port <= 0 || port >= 65536) { - return err(cdpError("discovery_failed", `BU_CDP_HTTP is not a valid host:port — got "${remote}"`)); - } - const ws = await queryLiveWsUrl(port, 5_000, host); - if (!ws) { + // No userDataDir is returned: the profile registry lives on the remote host, + // so profile enumeration degrades to the pre-profile behaviour rather than + // reading the wrong machine's Local State. + const target = resolveCdpTarget(); + if (target.kind === "invalid") { + return err(cdpError("discovery_failed", describeCdpTarget(target))); + } + if (target.kind === "remote") { + const ws = await queryLiveWsUrl(target.port, 5_000, target.host); + if (ws) return ok({ wsUrl: ws }); + + // An explicitly configured endpoint is an instruction: fail rather than + // quietly automate a different browser than the one that was asked for. + if (target.source === "env") { return err(cdpError( "discovery_failed", - `No CDP endpoint answered at http://${host}:${port}/json/version (BU_CDP_HTTP). Is the remote browser running and reachable?`, + `No CDP endpoint answered at http://${target.host}:${target.port}/json/version (BU_CDP_HTTP). ` + + `Is the remote browser running and reachable? Set BU_CDP_HTTP=local to use this machine's browser instead.`, )); } - return ok({ wsUrl: ws }); + // The built-in default is only a preference — off the wire network, fall + // through to the local browser the extension was originally written for. } const dirs = userDataDirCandidates(); diff --git a/src/cdp/endpoint.ts b/src/cdp/endpoint.ts new file mode 100644 index 0000000..1be5322 --- /dev/null +++ b/src/cdp/endpoint.ts @@ -0,0 +1,84 @@ +/** + * Which browser this harness should drive. + * + * Upstream only ever drives a browser on localhost. This fork adds a remote + * target so pi can drive a shared Chrome on another host (see FORK.md). + * + * Resolution is pure — no I/O — so the status command can report the configured + * intent without touching the network, and the daemon can resolve the same way + * in its own process. + */ + +/** Default remote CDP endpoint: castle's Chrome, exposed on the wire network. */ +export const DEFAULT_REMOTE_CDP = "10.88.0.25:9223"; + +/** Values of BU_CDP_HTTP that mean "ignore the default, use this machine". */ +const LOCAL_ALIASES = new Set(["local", "localhost", "off", "none", "0", "no"]); + +export type CdpTarget = + | { + readonly kind: "remote"; + readonly host: string; + readonly port: number; + /** "env" was asked for explicitly; "default" is this fork's built-in. */ + readonly source: "env" | "default"; + } + | { readonly kind: "local"; readonly source: "env" } + | { readonly kind: "invalid"; readonly raw: string; readonly reason: string }; + +/** + * Precedence: + * BU_CDP_HTTP="host:port" -> that remote, and a failure to reach it is fatal + * BU_CDP_HTTP=local|off|… -> upstream behaviour, this machine's own browser + * unset -> DEFAULT_REMOTE_CDP, falling back to local when + * it isn't reachable + * + * The asymmetry is deliberate. An explicit endpoint is an instruction, so + * silently driving a different browser instead would be wrong. The built-in + * default is only a preference, so a laptop away from the wire network still + * gets the local browser the extension was originally written for. + */ +export const resolveCdpTarget = (env: NodeJS.ProcessEnv = process.env): CdpTarget => { + const raw = env["BU_CDP_HTTP"]?.trim(); + + if (raw === undefined || raw === "") { + const parsed = parseHostPort(DEFAULT_REMOTE_CDP); + // Unreachable in practice: the constant above is validated by its own test. + if (!parsed) return { kind: "local", source: "env" }; + return { kind: "remote", host: parsed.host, port: parsed.port, source: "default" }; + } + + if (LOCAL_ALIASES.has(raw.toLowerCase())) return { kind: "local", source: "env" }; + + const parsed = parseHostPort(raw); + if (!parsed) { + return { + kind: "invalid", + raw, + reason: `expected "host:port" or one of ${[...LOCAL_ALIASES].join(", ")}`, + }; + } + return { kind: "remote", host: parsed.host, port: parsed.port, source: "env" }; +}; + +const parseHostPort = (value: string): { host: string; port: number } | null => { + const sep = value.lastIndexOf(":"); + if (sep === -1) return null; + const host = value.slice(0, sep).trim(); + const port = Number(value.slice(sep + 1).trim()); + if (!host) return null; + if (!Number.isInteger(port) || port <= 0 || port >= 65536) return null; + return { host, port }; +}; + +/** One-line description for /browser-status and error messages. */ +export const describeCdpTarget = (target: CdpTarget): string => { + switch (target.kind) { + case "remote": + return `${target.host}:${target.port}` + (target.source === "default" ? " (built-in default)" : " (BU_CDP_HTTP)"); + case "local": + return "this machine's own browser (BU_CDP_HTTP=local)"; + case "invalid": + return `invalid BU_CDP_HTTP "${target.raw}" — ${target.reason}`; + } +}; diff --git a/src/index.ts b/src/index.ts index 5818a5d..2b95697 100644 --- a/src/index.ts +++ b/src/index.ts @@ -11,6 +11,8 @@ import { restoreDownloadBehavior } from "./domains/files"; import { createDaemonTransport } from "./daemon/transport"; import { setDebugClicks } from "./util/debug"; import { asBoolean, asString } from "./util/guards"; +import { describeCdpTarget, resolveCdpTarget } from "./cdp/endpoint"; +import { queryLiveWsUrl } from "./cdp/discovery"; export default function browserHarnessExtension(pi: ExtensionAPI): void { const flagNs = asString(pi.getFlag("browser-namespace")); @@ -40,8 +42,32 @@ export default function browserHarnessExtension(pi: ExtensionAPI): void { } const s = client.status(); const pin = client.profilePin(); + + // FORK: the connection is lazy — nothing dials the browser until the + // first browser_* tool runs. A plain "Disconnected" reads as a fault + // when it is really just "not needed yet", so say which it is, and show + // what we would connect to. + const target = resolveCdpTarget(); + const reachable = + target.kind === "remote" ? (await queryLiveWsUrl(target.port, 2_000, target.host)) !== null : null; + + const browserLine = s.alive + ? "🟢 Connected" + : reachable === false + ? "🔴 Not connected — target unreachable" + : "🟡 Not connected yet — connects on the first browser tool call"; + + const targetLine = + target.kind === "remote" + ? `${describeCdpTarget(target)}${reachable ? " — reachable ✓" : " — unreachable ✗"}` + + (reachable === false && target.source === "default" + ? "\n will fall back to this machine's browser" + : "") + : describeCdpTarget(target); + const lines = [ - `Browser: ${s.alive ? "🟢 Connected" : "🔴 Disconnected"}`, + `Browser: ${browserLine}`, + `Target: ${targetLine}`, `Profile: ${pin ? pin.label : "not selected (uses whichever window is focused) — run /browser-profile"}`, `Session: ${s.sessionId ?? "none"}`, ]; diff --git a/test/cdp/endpoint-test.ts b/test/cdp/endpoint-test.ts new file mode 100644 index 0000000..d4b696f --- /dev/null +++ b/test/cdp/endpoint-test.ts @@ -0,0 +1,99 @@ +/** + * Unit tests for CDP target resolution — no browser and no network required. + * + * This is fork-specific logic (see FORK.md): upstream only ever drives a + * browser on localhost. The precedence here is deliberately asymmetric — an + * explicitly configured endpoint must never silently fall back to a different + * browser, while the built-in default must, so a machine off the wire network + * still gets the local browser the extension was originally written for. + * + * Run: npx tsx test/cdp/endpoint-test.ts + */ +import { DEFAULT_REMOTE_CDP, describeCdpTarget, resolveCdpTarget } from "../../src/cdp/endpoint"; + +let passed = 0; +let failed = 0; +const check = (cond: boolean, label: string): void => { + if (cond) { + passed++; + console.log(` ✓ ${label}`); + } else { + failed++; + console.error(` ✗ ${label}`); + } +}; + +const env = (value?: string): NodeJS.ProcessEnv => (value === undefined ? {} : { BU_CDP_HTTP: value }); + +function main(): void { + // E1: the built-in default, used when nothing is configured. + { + const t = resolveCdpTarget(env()); + check(t.kind === "remote", "E1: unset BU_CDP_HTTP resolves to a remote target"); + if (t.kind === "remote") { + check(t.source === "default", "E1: it is marked as the built-in default, not an instruction"); + check(`${t.host}:${t.port}` === DEFAULT_REMOTE_CDP, "E1: it matches DEFAULT_REMOTE_CDP"); + } + } + + // E2: DEFAULT_REMOTE_CDP must itself be parseable, or the default silently + // degrades to local for everyone. + { + const t = resolveCdpTarget(env()); + check(t.kind !== "local", "E2: the built-in default constant is well-formed"); + } + + // E3: an explicit endpoint is an instruction — source must reflect that, so + // discovery fails loudly rather than falling back. + { + const t = resolveCdpTarget(env("10.0.0.9:9333")); + check(t.kind === "remote", "E3: an explicit host:port resolves to a remote target"); + if (t.kind === "remote") { + check(t.host === "10.0.0.9" && t.port === 9333, "E3: host and port are parsed"); + check(t.source === "env", "E3: it is marked as explicitly configured"); + } + } + + // E4: opting back into upstream behaviour. + for (const alias of ["local", "LOCAL", "off", "none", "0", "no"]) { + const t = resolveCdpTarget(env(alias)); + check(t.kind === "local", `E4: "${alias}" selects this machine's own browser`); + } + + // E5: empty or whitespace is treated as unset rather than as an error — a + // shell exporting an empty var should not break the browser. + for (const blank of ["", " "]) { + const t = resolveCdpTarget(env(blank)); + check(t.kind === "remote" && t.source === "default", `E5: ${JSON.stringify(blank)} falls back to the default`); + } + + // E6: malformed values are rejected, not coerced. Silently reinterpreting + // these would drive an unexpected browser. + for (const bad of ["nonsense", "host:", ":9222", "host:0", "host:70000", "host:abc"]) { + const t = resolveCdpTarget(env(bad)); + check(t.kind === "invalid", `E6: "${bad}" is rejected as malformed`); + } + + // E7: IPv6 and hostnames still work — the port is taken from the LAST colon. + { + const t = resolveCdpTarget(env("castle.local:9223")); + check(t.kind === "remote" && t.host === "castle.local", "E7: hostnames are accepted"); + const v6 = resolveCdpTarget(env("[::1]:9223")); + check(v6.kind === "remote" && v6.host === "[::1]" && v6.port === 9223, "E7: bracketed IPv6 keeps its host"); + } + + // E8: descriptions are for humans in /browser-status; they must distinguish + // a default from an instruction, since only one of them falls back. + { + const d = describeCdpTarget(resolveCdpTarget(env())); + const e = describeCdpTarget(resolveCdpTarget(env("10.0.0.9:9333"))); + check(d.includes("default"), "E8: the default is labelled as such"); + check(e.includes("BU_CDP_HTTP"), "E8: an explicit endpoint names the variable that set it"); + check(describeCdpTarget(resolveCdpTarget(env("local"))).includes("own browser"), "E8: local is described plainly"); + } + + console.log(`\n${passed} passed, ${failed} failed`); + if (failed > 0) process.exit(1); +} + +main(); -- cgit v1.2.3