all repos — sz-chrome-remote-cdp @ c9d1605b1e3b8dbe88cd3674a9256aa0465f3e6d

pi-browser-harness, patched to drive a Chrome on another host over CDP, with castle_cdp_search web search

chore: address final-review minors — surface event-consumer crashes, document IIFE-wrap heuristic, log shutdown failures, fix CHANGELOG file-size claim

Five tweaks from the whole-implementation review:
- session.ts: log unexpected event-consumer rejections instead of silent swallow
- client.ts: comment the return-substring IIFE-wrap false-positive case
- transport.ts: comment that ws.onclose post-open settle is no-op-by-design
- index.ts: log client.stop() failures during shutdown
- CHANGELOG.md: correct largest-file claim (transport.ts and client.ts are ~220 each, not "client.ts at ~180")

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
amankumarsingh77 amankumarsingh7702@gmal.com
Sat, 02 May 2026 16:53:34 +0530
commit

c9d1605b1e3b8dbe88cd3674a9256aa0465f3e6d

parent

500dd206d300ab39599021ef9e80ed30d1871b9a

5 files changed, 22 insertions(+), 3 deletions(-)

jump to
M CHANGELOG.md → CHANGELOG.md

@@ -6,7 +6,7 @@ ## 0.3.0 — 2026-05-02

### Internal rewrite -- Per-domain module split: every tool now lives in its own `src/domains/<name>.ts` file. The 1140-line `daemon.ts` and 2277-line `tools.ts` are gone; the largest single file is now `src/client.ts` at ~180 LOC. +- Per-domain module split: every tool now lives in its own `src/domains/<name>.ts` file. The 1140-line `daemon.ts` and 2277-line `tools.ts` are gone; the three largest files are now `src/cdp/transport.ts` (~220 LOC), `src/client.ts` (~220 LOC), and `src/domains/js.ts` (~200 LOC). - New transport/session/client split: `BrowserDaemon` class replaced with `createBrowserClient()` factory composing a `CdpTransport` (factory) and `CdpSession` (factory). - All tool handlers now return `Result<T, E>`; one `defineBrowserTool` helper converts to pi's `ToolDefinition` and supplies a uniform `details` shape: `{ ok: true, ... }` on success, `{ ok: false, kind, message, ... }` on error. - Strict TypeScript flags enabled (`noUncheckedIndexedAccess`, `exactOptionalPropertyTypes`, `noPropertyAccessFromIndexSignature`, `noUnusedLocals`, `noUnusedParameters`). Zero `any` in the codebase. All boundary `as` casts documented.
M src/cdp/session.ts → src/cdp/session.ts

@@ -45,7 +45,13 @@ }

}; const restartConsumer = (): void => { - activeConsumer = activeConsumer.then(() => consumeEvents()).catch(() => undefined); + activeConsumer = activeConsumer.then(() => consumeEvents()).catch((e: unknown) => { + // The .then() chain calls consumeEvents() which iterates the transport's + // events() AsyncIterable. The iterable resolves cleanly on close (returns + // {done:true}); any rejection here is an unexpected bug in the event + // handler, not a normal termination. Surface it on stderr so it's not lost. + console.warn("[pi-browser-harness] CDP event consumer crashed:", e); + }); }; restartConsumer();
M src/cdp/transport.ts → src/cdp/transport.ts

@@ -156,6 +156,9 @@ ws.onclose = () => {

clearTimeout(timer); ws = null; cleanup("WebSocket closed"); + // If onopen already settled the connect promise, this settle is a no-op + // (guarded by `settled`). cleanup() above still runs unconditionally so + // pending requests are rejected and the event queue is rotated. settle(err(cdpError("transport_closed", "CDP WebSocket closed"))); }; });
M src/client.ts → src/client.ts

@@ -117,6 +117,10 @@ return start();

}; const evaluateJs = async (expression: string, sessionId?: string): Promise<Result<unknown, CdpError>> => { + // Heuristic IIFE wrap: legacy convenience that lets agents write + // `return foo` instead of `(() => foo)()`. False-positive on substring + // `"return "` inside string literals or comments — agents that hit this + // can pass an explicit IIFE instead. const wrapped = expression.includes("return ") && !expression.trim().startsWith("(") ? `(function(){${expression}})()` : expression;
M src/index.ts → src/index.ts

@@ -115,7 +115,13 @@

pi.on("session_shutdown", async () => { persistState(pi, state); if (client) { - try { await client.stop(); } catch { /* best-effort */ } + try { + await client.stop(); + } catch (e) { + // Shutdown is best-effort, but a stuck stop() points at a transport + // bug worth surfacing for debugging. + console.warn("[pi-browser-harness] client.stop() failed during shutdown:", e); + } client = null; } toolsRegistered = false;