From 0fe64593bcc3379032840b35833ea39f23aa9322 Mon Sep 17 00:00:00 2001 From: Aman Kumar <145339995+amankumarsingh77@users.noreply.github.com> Date: Sun, 11 Oct 2026 23:33:25 +0530 Subject: fix(daemon): stop importing typebox in the daemon process (#38) pi installs packages with dev and peer dependencies omitted and supplies typebox only to the extension host. The daemon is a separate Node process whose import graph reached typebox via src/cdp/discovery.ts, so from a pi-managed install it crashed with "Cannot find module 'typebox'" before binding its socket. Parse /json/version with the package's own guards, and add a test that keeps every package reachable from the daemon a runtime dependency. Fixes #34 Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 1 + src/cdp/discovery.ts | 27 +++++++++++---------- test/cdp/discovery-test.ts | 19 ++++++++++++++- test/daemon/entry-imports-test.ts | 50 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 13 deletions(-) create mode 100644 test/daemon/entry-imports-test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index ac990d6..5f0778b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ All notable changes to pi-browser-harness will be documented in this file. ### Fixed - **Browser state is no longer appended to the session on every callback.** Every ownership change, every `session_tree` event (even one that merely restored the entry already on the branch) and every shutdown appended a `browser-harness-state` entry, so the append-only session file and Pi Web's `+N custom` chain grew without bound. A snapshot is now written only when it differs from the last one persisted or restored; tab order does not count as a difference. (#26) +- **The daemon starts from a pi-managed install without `typebox`.** pi installs packages with dev and peer dependencies omitted and maps host-provided packages such as `typebox` only inside the extension host, but the daemon is a separate Node process that resolved `typebox` through `src/cdp/discovery.ts` and died with `Cannot find module 'typebox'` before binding its socket, so `/browser-setup` reported `Could not start the browser daemon`. The daemon's `/json/version` parsing no longer uses `typebox`, and a test walks the daemon's import graph to keep every reachable package a runtime dependency. (#34) - **The daemon starts from a Bun-compiled pi binary.** Inside `pi.exe` (and any other `bun build --compile` host), `createRequire().resolve("tsx/cli")` is rooted at the binary's embedded virtual filesystem and cannot see the extension's real `node_modules`, so `browser_setup` failed instantly with `Could not start the browser daemon`. The resolver result is now checked on disk and, when it fails, the tsx CLI is found by walking up the `node_modules` chain the way Node does. Under Bun the daemon is launched with `node` from `PATH` instead of `process.execPath` (which is the pi binary itself); `PI_BROWSER_NODE` overrides the executable. (#27) ## 0.11.1 — 2026-08-12 diff --git a/src/cdp/discovery.ts b/src/cdp/discovery.ts index 60fb14f..3de6e97 100644 --- a/src/cdp/discovery.ts +++ b/src/cdp/discovery.ts @@ -2,23 +2,27 @@ import { readFile, stat } from "node:fs/promises"; import { join } from "node:path"; import { connect as netConnect } from "node:net"; import { request as httpRequest } from "node:http"; -import { Type } from "typebox"; -import { Compile } from "typebox/compile"; -import { parseJson } from "../schemas/parse"; import { type Result, err, ok } from "../util/result"; import { userDataDirCandidates } from "../profile/paths"; import { type CdpError, cdpError } from "./errors"; -import { errnoCode } from "../util/guards"; +import { asString, errnoCode, isRecord } from "../util/guards"; const PORT_PROBE_DEADLINE_MS = 30_000; const PORT_PROBE_INTERVAL_MS = 1_000; -const VersionResponse = Type.Object( - { webSocketDebuggerUrl: Type.Optional(Type.String()) }, - { additionalProperties: true }, -); - -const versionValidator = Compile(VersionResponse); +// Hand-rolled on purpose: this file runs inside the daemon, a separate Node process that pi does not +// supply host-provided packages (typebox) to. Everything the daemon imports must be a runtime +// dependency or a builtin; test/daemon/entry-imports-test.ts enforces that. +export const parseVersionResponse = (raw: string): string | null => { + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return null; + } + if (!isRecord(parsed)) return null; + return asString(parsed["webSocketDebuggerUrl"]) ?? null; +}; export type CdpEndpoint = { readonly wsUrl: string; @@ -72,8 +76,7 @@ const queryLiveWsUrl = (port: number, timeoutMs = 1_500): Promise const chunks: Buffer[] = []; res.on("data", (c: Buffer) => chunks.push(c)); res.on("end", () => { - const parsed = parseJson(Buffer.concat(chunks).toString("utf8"), versionValidator); - resolve(parsed.success ? parsed.data.webSocketDebuggerUrl ?? null : null); + resolve(parseVersionResponse(Buffer.concat(chunks).toString("utf8"))); }); }, ); diff --git a/test/cdp/discovery-test.ts b/test/cdp/discovery-test.ts index 7fc18fb..61b42b7 100644 --- a/test/cdp/discovery-test.ts +++ b/test/cdp/discovery-test.ts @@ -1,6 +1,6 @@ import { test, describe } from "node:test"; import assert from "node:assert/strict"; -import { endpointFromEnv } from "../../src/cdp/discovery"; +import { endpointFromEnv, parseVersionResponse } from "../../src/cdp/discovery"; describe("BU_CDP_WS endpoint override", () => { test("an unset variable leaves discovery to run normally", () => { @@ -27,3 +27,20 @@ describe("BU_CDP_WS endpoint override", () => { assert.equal(endpointFromEnv({ BU_CDP_WS: " " }), undefined); }); }); + +describe("/json/version parsing", () => { + test("extracts webSocketDebuggerUrl from a well-formed response", () => { + const raw = JSON.stringify({ Browser: "Chrome/154.0", webSocketDebuggerUrl: "ws://127.0.0.1:9222/devtools/browser/abc" }); + assert.equal(parseVersionResponse(raw), "ws://127.0.0.1:9222/devtools/browser/abc"); + }); + + test("a response without the url yields null", () => { + assert.equal(parseVersionResponse(JSON.stringify({ Browser: "Chrome/154.0" })), null); + }); + + test("a non-string url, a non-object body, and invalid JSON all yield null", () => { + assert.equal(parseVersionResponse(JSON.stringify({ webSocketDebuggerUrl: 42 })), null); + assert.equal(parseVersionResponse(JSON.stringify([1, 2])), null); + assert.equal(parseVersionResponse("not json"), null); + }); +}); diff --git a/test/daemon/entry-imports-test.ts b/test/daemon/entry-imports-test.ts new file mode 100644 index 0000000..24119a6 --- /dev/null +++ b/test/daemon/entry-imports-test.ts @@ -0,0 +1,50 @@ +import { test, describe } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +// The daemon is spawned as a separate Node process that resolves modules from the package directory +// upward. pi installs packages with dev and peer dependencies omitted and maps host-provided packages +// (typebox) only inside the extension host, so anything the daemon imports must be a runtime +// dependency of this package or a Node builtin. Issue #34 was `Cannot find module 'typebox'`. +const RUNTIME_DEPENDENCIES: ReadonlySet = new Set( + Object.keys(JSON.parse(readFileSync(new URL("../../package.json", import.meta.url), "utf8")).dependencies ?? {}), +); + +const srcRoot = resolve(dirname(fileURLToPath(import.meta.url)), "..", "..", "src"); + +const importSpecifiers = (file: string): ReadonlyArray => + [...readFileSync(file, "utf8").matchAll(/^(?:import|export)[^'"\n]*from\s*["']([^'"]+)["']/gm)].map((m) => m[1] ?? ""); + +const externalImportsReachableFrom = (entry: string): ReadonlyMap => { + const external = new Map(); + const seen = new Set(); + const walk = (file: string): void => { + if (seen.has(file)) return; + seen.add(file); + for (const spec of importSpecifiers(file)) { + if (spec.startsWith(".")) { + let target = resolve(dirname(file), spec); + if (!target.endsWith(".ts")) target += ".ts"; + walk(target); + } else if (!spec.startsWith("node:")) { + external.set(spec, file.slice(srcRoot.length + 1)); + } + } + }; + walk(entry); + return external; +}; + +describe("daemon entrypoint imports", () => { + test("the daemon only reaches runtime dependencies, never host-provided or dev packages", () => { + const external = externalImportsReachableFrom(join(srcRoot, "daemon", "index.ts")); + const offenders = [...external].filter(([spec]) => !RUNTIME_DEPENDENCIES.has(spec.split("/")[0] ?? spec)); + assert.deepEqual( + offenders, + [], + `daemon imports packages that are not in "dependencies": ${offenders.map(([s, f]) => `${s} (via ${f})`).join(", ")}`, + ); + }); +}); -- cgit v1.2.3