diff options
| author | amankumarsingh77 <amankumarsingh7702@gmail.com> | 2026-07-28 21:48:35 +0530 |
|---|---|---|
| committer | Aman Kumar <145339995+amankumarsingh77@users.noreply.github.com> | 2026-08-02 19:12:46 +0530 |
| commit | 6d300099a86cfdb92b7869dc2b4f33094554b24c (patch) | |
| tree | 56ea6be73ac8b92d4ae4a2731e8d41dd54e876f7 | |
| parent | aedbb88a8025851067be9f77ec406b29d65599a8 (diff) | |
fix: make --browser-debug-clicks and BU_CDP_WS actually work
Both were documented and inert.
--browser-debug-clicks was registered as a flag and never read; the only
real switch was the undocumented BH_DEBUG_CLICKS env var. The flag now
feeds util/debug.ts, which the click tool consults, and the env var keeps
working as an equivalent.
BU_CDP_WS was read in the client and handed to transport.connect(), but
the daemon transport discards that argument and always dials the daemon
socket — and the daemon's own discovery never looked at the variable, so
setting it did nothing. The override now lives in discoverEndpoint, which
is what both processes go through, and the client's redundant read is
gone. The variable must be set before the daemon spawns.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| -rw-r--r-- | src/cdp/discovery.ts | 9 | ||||
| -rw-r--r-- | src/client.ts | 3 | ||||
| -rw-r--r-- | src/domains/click.ts | 5 | ||||
| -rw-r--r-- | src/index.ts | 4 | ||||
| -rw-r--r-- | src/util/debug.ts | 8 | ||||
| -rw-r--r-- | test/cdp/discovery-test.ts | 29 | ||||
| -rw-r--r-- | test/util/debug-test.ts | 32 |
7 files changed, 84 insertions, 6 deletions
diff --git a/src/cdp/discovery.ts b/src/cdp/discovery.ts index f33adc6..60fb14f 100644 --- a/src/cdp/discovery.ts +++ b/src/cdp/discovery.ts @@ -101,7 +101,16 @@ type Candidate = { readonly userDataDir: string; }; +// Read here rather than in the client: the daemon owns the only socket to Chrome, so an override the daemon cannot see has no effect. +export const endpointFromEnv = (env: NodeJS.ProcessEnv = process.env): CdpEndpoint | undefined => { + const wsUrl = env["BU_CDP_WS"]?.trim(); + return wsUrl ? { wsUrl } : undefined; +}; + export const discoverEndpoint = async (): Promise<Result<CdpEndpoint, CdpError>> => { + const override = endpointFromEnv(); + if (override) return ok(override); + const dirs = userDataDirCandidates(); const candidates: Candidate[] = []; diff --git a/src/client.ts b/src/client.ts index 84a98ee..5367489 100644 --- a/src/client.ts +++ b/src/client.ts @@ -140,12 +140,9 @@ export const createBrowserClient = (opts: BrowserClientOptions): BrowserClient = const start = async (): Promise<Result<void, CdpError>> => { if (transport.state() === "open" && session.current()) return ok(undefined); - const envUrl = process.env["BU_CDP_WS"]; let wsUrl: string; if (remote?.cdpUrl) { wsUrl = remote.cdpUrl; - } else if (envUrl) { - wsUrl = envUrl; } else { const discovered = await discoverEndpoint(); if (!discovered.success) return discovered; diff --git a/src/domains/click.ts b/src/domains/click.ts index cb4b007..41c3024 100644 --- a/src/domains/click.ts +++ b/src/domains/click.ts @@ -3,6 +3,7 @@ import { Text } from "@mariozechner/pi-tui"; import type { BrowserClient } from "../client"; import { Coords, MouseButton } from "../schemas/common"; import { type Result, ok } from "../util/result"; +import { debugClicksEnabled } from "../util/debug"; import { defineBrowserTool, type ToolErr } from "../util/tool"; import { cdpCall } from "./cdp-call"; import { captureWithCrosshair } from "./screenshot"; @@ -62,7 +63,7 @@ export const clickTool = defineBrowserTool({ "A 'ref is stale' error means the page changed — re-run browser_snapshot to get fresh refs.", "After clicking, read the appended page-changes diff to confirm the action landed before moving on (no separate snapshot needed for a quick check).", "Coordinates are viewport CSS pixels (not device pixels). Compositor-level clicks pass through iframes, shadow DOM, and cross-origin content.", - "If a click doesn't register, set BH_DEBUG_CLICKS=1 to get annotated screenshots (debug only). For React/Vue components ignoring clicks, try browser_dispatch_key.", + "If a click doesn't register, run pi with --browser-debug-clicks (or set BH_DEBUG_CLICKS=1) to get annotated screenshots. For React/Vue components ignoring clicks, try browser_dispatch_key.", ], parameters: ClickArgs, concurrency: "serialized", @@ -79,7 +80,7 @@ export const clickTool = defineBrowserTool({ if (!clicked.success) return clicked; const diff = await interactiveDiff(client); - if (process.env["BH_DEBUG_CLICKS"]) { + if (debugClicksEnabled()) { const debug = await captureWithCrosshair(client, { x, y }); if (debug.success) { return ok({ diff --git a/src/index.ts b/src/index.ts index 1573605..6097881 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,7 +8,8 @@ import { type BrowserState, defaultState, persistState, restoreState } from "./s import { registerAllTools } from "./registry"; import { cleanupTempDirs } from "./util/truncate"; import { createDaemonTransport } from "./cdp/daemon-transport"; -import { asString } from "./util/guards"; +import { setDebugClicks } from "./util/debug"; +import { asBoolean, asString } from "./util/guards"; export default function browserHarnessExtension(pi: ExtensionAPI): void { const flagNs = asString(pi.getFlag("browser-namespace")); @@ -82,6 +83,7 @@ export default function browserHarnessExtension(pi: ExtensionAPI): void { pi.on("session_start", async (_event, ctx) => { state = restoreState(ctx, state.namespace); + setDebugClicks(asBoolean(pi.getFlag("browser-debug-clicks")) ?? false); // Re-read the on-disk profile pin every session start: another pi session may have changed it since this client was created. const profilePin = await readPin(); diff --git a/src/util/debug.ts b/src/util/debug.ts new file mode 100644 index 0000000..e3b3a62 --- /dev/null +++ b/src/util/debug.ts @@ -0,0 +1,8 @@ +let flagEnabled = false; + +export const setDebugClicks = (on: boolean): void => { + flagEnabled = on; +}; + +export const debugClicksEnabled = (env: NodeJS.ProcessEnv = process.env): boolean => + flagEnabled || Boolean(env["BH_DEBUG_CLICKS"]); diff --git a/test/cdp/discovery-test.ts b/test/cdp/discovery-test.ts new file mode 100644 index 0000000..7fc18fb --- /dev/null +++ b/test/cdp/discovery-test.ts @@ -0,0 +1,29 @@ +import { test, describe } from "node:test"; +import assert from "node:assert/strict"; +import { endpointFromEnv } from "../../src/cdp/discovery"; + +describe("BU_CDP_WS endpoint override", () => { + test("an unset variable leaves discovery to run normally", () => { + assert.equal(endpointFromEnv({}), undefined); + }); + + test("a set variable becomes the endpoint", () => { + const e = endpointFromEnv({ BU_CDP_WS: "ws://127.0.0.1:9333/devtools/browser/abc" }); + assert.equal(e?.wsUrl, "ws://127.0.0.1:9333/devtools/browser/abc"); + }); + + test("no userDataDir is claimed for a remote endpoint", () => { + const e = endpointFromEnv({ BU_CDP_WS: "ws://example.test/x" }); + assert.equal(e?.userDataDir, undefined); + }); + + test("surrounding whitespace is trimmed", () => { + const e = endpointFromEnv({ BU_CDP_WS: " ws://127.0.0.1:9222/x " }); + assert.equal(e?.wsUrl, "ws://127.0.0.1:9222/x"); + }); + + test("an empty or whitespace-only variable is ignored, not treated as an endpoint", () => { + assert.equal(endpointFromEnv({ BU_CDP_WS: "" }), undefined); + assert.equal(endpointFromEnv({ BU_CDP_WS: " " }), undefined); + }); +}); diff --git a/test/util/debug-test.ts b/test/util/debug-test.ts new file mode 100644 index 0000000..08236dd --- /dev/null +++ b/test/util/debug-test.ts @@ -0,0 +1,32 @@ +import { test, describe, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import { debugClicksEnabled, setDebugClicks } from "../../src/util/debug"; + +describe("debug click overlay switch", () => { + afterEach(() => { + setDebugClicks(false); + }); + + test("off when neither the flag nor the env var is set", () => { + assert.equal(debugClicksEnabled({}), false); + }); + + test("the --browser-debug-clicks flag turns it on", () => { + setDebugClicks(true); + assert.equal(debugClicksEnabled({}), true); + }); + + test("BH_DEBUG_CLICKS still turns it on without the flag", () => { + assert.equal(debugClicksEnabled({ BH_DEBUG_CLICKS: "1" }), true); + }); + + test("an empty BH_DEBUG_CLICKS does not turn it on", () => { + assert.equal(debugClicksEnabled({ BH_DEBUG_CLICKS: "" }), false); + }); + + test("the flag can be turned back off", () => { + setDebugClicks(true); + setDebugClicks(false); + assert.equal(debugClicksEnabled({}), false); + }); +}); |
