diff options
| -rw-r--r-- | src/cdp/discovery.ts | 49 | ||||
| -rw-r--r-- | src/cdp/endpoint.ts | 84 | ||||
| -rw-r--r-- | src/index.ts | 28 | ||||
| -rw-r--r-- | test/cdp/endpoint-test.ts | 99 |
4 files changed, 237 insertions, 23 deletions
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<Result<void, CdpError>> => { const isPortLive = async (port: number): Promise<boolean> => (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<string | null> => +export const queryLiveWsUrl = (port: number, timeoutMs = 1_500, host = "127.0.0.1"): Promise<string | null> => new Promise<string | null>((resolve) => { const req = httpRequest( { host, port, path: "/json/version", method: "GET", timeout: timeoutMs }, @@ -114,33 +115,37 @@ export const discoverEndpoint = async (): Promise<Result<CdpEndpoint, CdpError>> 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(); |