refactor(cdp): extract attachTo, getWindowId, evaluateJson helpers Target.attachToTarget was decoded at 5 sites, Browser.getWindowForTarget at 5, and Runtime.evaluate unwrapped at another. Each is now one typed helper. Deletes the dead cdp/window-binding.ts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@@ -0,0 +1,13 @@
+import { map, type Result } from "../util/result"; +import type { CdpError } from "./errors"; +import type { CdpSession } from "./session"; + +/** Target.attachToTarget, unwrapped to the new session id. */ +export const attachTo = async ( + session: CdpSession, + targetId: string, +): Promise<Result<string, CdpError>> => + map( + await session.callBrowser("Target.attachToTarget", { targetId, flatten: true }), + (d) => d.sessionId, + );
@@ -0,0 +1,31 @@
+import { type Result, err, ok } from "../util/result"; +import { type CdpError, cdpError } from "./errors"; +import type { CdpSession } from "./session"; + +/** + * Runtime.evaluate, unwrapped and narrowed. Runs `expression`, surfaces a page + * exception as an error, then applies `check` to the returned value — the + * sanctioned way to turn `unknown` evaluate output into a typed `T` without a + * cast. + */ +export const evaluateJson = async <T>( + session: CdpSession, + expression: string, + check: (v: unknown) => v is T, +): Promise<Result<T, CdpError>> => { + const r = await session.call("Runtime.evaluate", { + expression, + returnByValue: true, + awaitPromise: true, + }); + if (!r.success) return r; + const { result, exceptionDetails } = r.data; + if (exceptionDetails !== undefined) { + return err(cdpError("remote_error", exceptionDetails.text, "Runtime.evaluate")); + } + const value = result.value; + if (!check(value)) { + return err(cdpError("invalid_response", `unexpected evaluate shape for: ${expression.slice(0, 60)}`, "Runtime.evaluate")); + } + return ok(value); +};
@@ -6,6 +6,8 @@ import type { CdpTransport } from "./transport";
import { type CdpMethod, type ParamsOf, type ResultOf, decodeResult } from "./commands"; import { createNetworkBuffer, type DrainResult, type NetworkFilter } from "./network-buffer"; import { createConsoleBuffer, type ConsoleDrainResult, type ConsoleFilter } from "./console-buffer"; +import { attachTo } from "./attach"; +import { getWindowId } from "./window"; // Per-tab state. One TabSession exists per known targetId. // Dialog, page-info dirty flag, and CDP buffers are per-tab — switching@@ -200,7 +202,7 @@ if (!raw.success) return raw;
return decodeResult(method, raw.data); }; - return { + const session: CdpSession = { async attachFirstPage() { // Subscribe to Target.* events so we can react to targetDestroyed. // Best-effort: failing to enable discovery is not fatal for attach.@@ -231,20 +233,20 @@ // Ownership is thus DERIVED from the real window, never from the bare
// persisted integer (which Chrome may reassign to a user window on // restart — anchoring on a survivor makes that misfire impossible). const anchorId = survivors[0]!; - const anchorWin = await req("Browser.getWindowForTarget", { targetId: anchorId }, null); + const anchorWin = await getWindowId(session, anchorId); if (anchorWin.success) { - const anchorWindowId = anchorWin.data.windowId; + const anchorWindowId = anchorWin.data; ownership.setHarnessWindowId(anchorWindowId); // refresh to the live id const windows = await Promise.all( allPages.map((p) => p.targetId === anchorId ? Promise.resolve(anchorWin) - : req("Browser.getWindowForTarget", { targetId: p.targetId }, null), + : getWindowId(session, p.targetId), ), ); allPages.forEach((p, i) => { const r = windows[i]; - if (r?.success && r.data.windowId === anchorWindowId) { + if (r?.success && r.data === anchorWindowId) { ownership.add(p.targetId); } });@@ -290,19 +292,19 @@ ownership.setHarnessWindow(created.data.targetId);
ownership.add(created.data.targetId); // Capture the real Chrome windowId — the durable identity of the // window this session initialized (survives seed-tab closure). - const win = await req("Browser.getWindowForTarget", { targetId: created.data.targetId }, null); - if (win.success) ownership.setHarnessWindowId(win.data.windowId); + const win = await getWindowId(session, created.data.targetId); + if (win.success) ownership.setHarnessWindowId(win.data); } } - const attached = await req("Target.attachToTarget", { targetId: pickTargetId, flatten: true }, null); + const attached = await attachTo(session, pickTargetId); if (!attached.success) return attached; - const a = attached.data; - sessionId = a.sessionId; + const attachedSessionId = attached.data; + sessionId = attachedSessionId; targetId = pickTargetId; - await enableDomains(a.sessionId); + await enableDomains(attachedSessionId); tabs.set(pickTargetId, { - sessionId: a.sessionId, + sessionId: attachedSessionId, targetId: pickTargetId, dialog: null, pageInfoDirty: false,@@ -311,19 +313,19 @@ consoleBuffer: createConsoleBuffer(),
refMap: new Map(), refSig: new Map(), }); - sessionIdToTargetId.set(a.sessionId, pickTargetId); - return ok({ targetId: pickTargetId, sessionId: a.sessionId }); + sessionIdToTargetId.set(attachedSessionId, pickTargetId); + return ok({ targetId: pickTargetId, sessionId: attachedSessionId }); }, async switchTo(tid) { const activated = await req("Target.activateTarget", { targetId: tid }, null); if (!activated.success) return activated; - const attached = await req("Target.attachToTarget", { targetId: tid, flatten: true }, null); + const attached = await attachTo(session, tid); if (!attached.success) return attached; - const a = attached.data; + const attachedSessionId = attached.data; // Reuse existing TabSession or create one on first visit const existing = tabs.get(tid); const tab: TabSession = existing ?? { - sessionId: a.sessionId, + sessionId: attachedSessionId, targetId: tid, dialog: null, pageInfoDirty: true,@@ -335,12 +337,12 @@ };
if (existing) { // Each Target.attachToTarget produces a new sessionId — update it. sessionIdToTargetId.delete(existing.sessionId); - existing.sessionId = a.sessionId; - sessionIdToTargetId.set(a.sessionId, tid); + existing.sessionId = attachedSessionId; + sessionIdToTargetId.set(attachedSessionId, tid); } else { tabs.set(tid, tab); - await enableDomains(a.sessionId); - sessionIdToTargetId.set(a.sessionId, tid); + await enableDomains(attachedSessionId); + sessionIdToTargetId.set(attachedSessionId, tid); } // Update global pointers to point at the new active tab sessionId = tab.sessionId;@@ -398,4 +400,5 @@ if (!tab) return { records: [], total: 0, bufferOverflowed: false };
return tab.consoleBuffer.drain(filter); }, }; + return session; };
@@ -30,7 +30,9 @@
import { randomUUID } from "node:crypto"; import type { BrowserClient } from "../client"; import { type Result, err, ok } from "../util/result"; +import { attachTo } from "./attach"; import { type CdpError, cdpError } from "./errors"; +import { getWindowId } from "./window"; /** How long to wait for a window.open-spawned target to appear. */ const SPAWN_DEADLINE_MS = 8_000;@@ -63,9 +65,7 @@ /** A CDP session id for driving `targetId`, reusing the active one when possible. */
const sessionForTarget = async (client: BrowserClient, targetId: string): Promise<Result<string, CdpError>> => { const current = client.current(); if (current?.targetId === targetId) return ok(current.sessionId); - const attached = await client.session().callBrowser("Target.attachToTarget", { targetId, flatten: true }); - if (!attached.success) return attached; - return ok(attached.data.sessionId); + return attachTo(client.session(), targetId); }; /**@@ -189,8 +189,8 @@ const { targetId } = created.data;
const ownership = client.ownership(); ownership.setHarnessWindow(targetId); ownership.add(targetId); - const win = await client.session().callBrowser("Browser.getWindowForTarget", { targetId }); - if (win.success) ownership.setHarnessWindowId(win.data.windowId); + const win = await getWindowId(client.session(), targetId); + if (win.success) ownership.setHarnessWindowId(win.data); return ok(targetId); };
@@ -1,15 +0,0 @@
-import type { BrowserClient } from "../client"; - -/** - * Capture the real Chrome windowId of a freshly-created harness window and bind - * it as the session's durable window identity. Best-effort: a failed CDP call - * leaves the previous binding untouched. Call this on every code path that - * spawns a NEW dedicated window (newWindow:true), so the windowId never points - * at a window that has since been replaced. - */ -export const bindHarnessWindowId = async (client: BrowserClient, targetId: string): Promise<void> => { - const r = await client.session().callBrowser("Browser.getWindowForTarget", { targetId }); - if (!r.success) return; - const wid = (r.data as { windowId?: number }).windowId; - if (typeof wid === "number") client.ownership().setHarnessWindowId(wid); -};
@@ -0,0 +1,10 @@
+import { map, type Result } from "../util/result"; +import type { CdpError } from "./errors"; +import type { CdpSession } from "./session"; + +/** Browser.getWindowForTarget, unwrapped to the windowId. */ +export const getWindowId = async ( + session: CdpSession, + targetId: string, +): Promise<Result<number, CdpError>> => + map(await session.callBrowser("Browser.getWindowForTarget", { targetId }), (d) => d.windowId);
@@ -244,11 +244,10 @@ const r = sessionId
? await session.callOnTarget("Runtime.evaluate", { expression: wrapped, returnByValue: true, awaitPromise: true }, sessionId) : await session.call("Runtime.evaluate", { expression: wrapped, returnByValue: true, awaitPromise: true }); if (!r.success) return r; - const data = r.data as { result?: { value?: unknown }; exceptionDetails?: unknown }; - if (data.exceptionDetails) { - return err(cdpError("remote_error", `JS evaluation failed: ${JSON.stringify(data.exceptionDetails)}`, "Runtime.evaluate")); + if (r.data.exceptionDetails !== undefined) { + return err(cdpError("remote_error", `JS evaluation failed: ${JSON.stringify(r.data.exceptionDetails)}`, "Runtime.evaluate")); } - return ok(data.result?.value); + return ok(r.data.result.value); }; const readPageInfo = async (): Promise<Result<PageInfo, CdpError>> => {
@@ -19,7 +19,9 @@ * mints new context ids on every browser run.
*/ import type { BrowserClient } from "../client"; +import { attachTo } from "../cdp/attach"; import { type CdpError, cdpError } from "../cdp/errors"; +import { getWindowId } from "../cdp/window"; import { type Result, err, ok } from "../util/result"; import { detectRunningBrowser, resolveBrowserExecutable } from "./browser-process"; import { openProfileWindow } from "./launch";@@ -116,22 +118,15 @@
const ownership = client.ownership(); ownership.setHarnessWindow(seed.targetId); ownership.add(seed.targetId); - const win = await client.session().callBrowser("Browser.getWindowForTarget", { targetId: seed.targetId }); - if (win.success) { - const windowId = (win.data as { windowId?: number }).windowId; - if (typeof windowId === "number") ownership.setHarnessWindowId(windowId); - } + const win = await getWindowId(client.session(), seed.targetId); + if (win.success) ownership.setHarnessWindowId(win.data); // Blank the handshake page so the seed tab behaves like any other fresh tab, // then remove the sentinel file. Best-effort: a failure here costs nothing // beyond the placeholder page staying visible. - const attached = await client.session().callBrowser("Target.attachToTarget", { - targetId: seed.targetId, - flatten: true, - }); + const attached = await attachTo(client.session(), seed.targetId); if (attached.success) { - const sessionId = (attached.data as { sessionId: string }).sessionId; - await client.session().callOnTarget("Page.navigate", { url: "about:blank" }, sessionId); + await client.session().callOnTarget("Page.navigate", { url: "about:blank" }, attached.data); } await cleanup();
@@ -0,0 +1,39 @@
+import { test, describe } from "node:test"; +import assert from "node:assert/strict"; +import { evaluateJson } from "../../src/cdp/evaluate"; +import type { CdpSession } from "../../src/cdp/session"; +import { ok } from "../../src/util/result"; + +const isNumberArray = (v: unknown): v is ReadonlyArray<number> => + Array.isArray(v) && v.every((x) => typeof x === "number"); + +const fakeSession = (payload: unknown): CdpSession => + ({ + call: async () => ok(payload), + }) as unknown as CdpSession; + +describe("evaluateJson", () => { + test("unwraps result.value and applies the guard", async () => { + const s = fakeSession({ result: { type: "object", value: [1, 2, 3] } }); + const r = await evaluateJson(s, "[1,2,3]", isNumberArray); + assert.equal(r.success, true); + if (r.success) assert.deepEqual(r.data, [1, 2, 3]); + }); + + test("surfaces a page exception as an error", async () => { + const s = fakeSession({ + result: { type: "undefined" }, + exceptionDetails: { text: "ReferenceError: x is not defined" }, + }); + const r = await evaluateJson(s, "x", isNumberArray); + assert.equal(r.success, false); + if (!r.success) assert.match(r.error.message, /ReferenceError/); + }); + + test("rejects a value that fails the guard", async () => { + const s = fakeSession({ result: { type: "string", value: "nope" } }); + const r = await evaluateJson(s, "'nope'", isNumberArray); + assert.equal(r.success, false); + if (!r.success) assert.equal(r.error.kind, "invalid_response"); + }); +});