From 632600ec0a0a68d9327e19bd419a1ae72047955f Mon Sep 17 00:00:00 2001 From: amankumarsingh77 Date: Tue, 28 Jul 2026 21:55:55 +0530 Subject: refactor: one door from a tool to the browser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four routes coexisted: cdpCall, a raw client.session().call, a session held in a local and called directly, and client.evaluateJs — and five files mixed two or three of them. The CdpError-to-ToolErr mapping was hand-copied at nine sites, plus a private copy in isolated-tab.ts. domains/cdp-call.ts now owns all of it: cdpErrToToolErr, cdpCall, cdpCallOnTarget, cdpCallBrowser, evalJs. Every other file under src/domains goes through them, which also removes a pile of `if (!r.success) return err({ kind: "cdp_error", ... })` boilerplate in favour of returning the failed Result as-is. Two scanner rules keep it that way, scoped to src/domains/ so the layer that legitimately owns the session — cdp/ and client.ts — is unaffected. Scoping is new to the rule model; without it the rules fired on the transport itself. One deliberate behaviour change: a CDP timeout now surfaces as kind "timeout" rather than "cdp_error". isolated-tab.ts already did this and the other 13 sites flattened it; the shared mapper uses the honest label rather than preserving the mislabel. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/boundary-allowlist.ts | 13 +++++++++++ scripts/check-boundaries.ts | 9 ++++++-- src/domains/cdp-call.ts | 45 ++++++++++++++++++++++++++++++++++++-- src/domains/files.ts | 14 ++++++------ src/domains/form.ts | 12 +++++----- src/domains/isolated-tab.ts | 27 +++++++++++------------ src/domains/js.ts | 5 +++-- src/domains/keyboard.ts | 8 +++---- src/domains/navigate.ts | 11 +++++----- src/domains/network.ts | 3 ++- src/domains/page.ts | 5 +++-- src/domains/readpage/read-page.ts | 9 ++++---- src/domains/ref-resolve.ts | 5 +++-- src/domains/screenshot.ts | 4 ++-- src/domains/snapshot.ts | 12 +++++----- src/domains/tabs.ts | 5 +++-- test/util/check-boundaries-test.ts | 34 ++++++++++++++++++++++++++++ 17 files changed, 160 insertions(+), 61 deletions(-) diff --git a/scripts/boundary-allowlist.ts b/scripts/boundary-allowlist.ts index bb34139..512aa40 100644 --- a/scripts/boundary-allowlist.ts +++ b/scripts/boundary-allowlist.ts @@ -41,6 +41,19 @@ export const ALLOWLIST: ReadonlyArray = [ reason: "renderCall is stored under a per-tool Static but invoked through the erased TSchema form; the generic-correlation limit makes this unavoidable and the value is the tool's own handler, created with the same S", }, + { + path: "src/domains/cdp-call.ts", + rule: "raw-cdp-call", + count: 3, + reason: + "the one door from a tool to CDP: cdpCall, cdpCallOnTarget and cdpCallBrowser each reach the session once and map CdpError to ToolErr, so no other file under src/domains needs to", + }, + { + path: "src/domains/cdp-call.ts", + rule: "raw-evaluate", + count: 1, + reason: "evalJs is the single wrapper over client.evaluateJs, for the same reason", + }, { path: "src/domains/js.ts", rule: "trailing-cast", diff --git a/scripts/check-boundaries.ts b/scripts/check-boundaries.ts index c438b83..7dd97e2 100644 --- a/scripts/check-boundaries.ts +++ b/scripts/check-boundaries.ts @@ -11,7 +11,8 @@ export type Violation = { readonly text: string; }; -const RULES: ReadonlyArray<{ readonly rule: string; readonly re: RegExp }> = [ +// `scope` restricts a rule to one layer: the tool layer must go through domains/cdp-call.ts, while cdp/ and client.ts are the layer that owns the session. +const RULES: ReadonlyArray<{ readonly rule: string; readonly re: RegExp; readonly scope?: RegExp }> = [ { rule: "as-any", re: /\bas\s+any\b/ }, { rule: "ts-ignore", re: /@ts-(ignore|expect-error|nocheck)/ }, { rule: "non-null-assertion", re: /[A-Za-z_$)\]]!\s*(?:[.[(;,)\]}]|$)/ }, @@ -20,6 +21,8 @@ const RULES: ReadonlyArray<{ readonly rule: string; readonly re: RegExp }> = [ { rule: "named-cast", re: /\bas\s+(?:new\s+)?[A-Z][A-Za-z0-9_]*/ }, { rule: "function-cast", re: /\bas\s+\(/ }, { rule: "trailing-cast", re: /\bas\s*$/ }, + { rule: "raw-cdp-call", re: /(?:session\(\)|\bsession)\.call(?:OnTarget|Browser)?\(/, scope: /^src\/domains\// }, + { rule: "raw-evaluate", re: /\.evaluateJs\(/, scope: /^src\/domains\// }, ]; export type SourceFile = { readonly path: string; readonly text: string }; @@ -37,8 +40,10 @@ const budgets = (exemptions: ReadonlyArray): Map => { const rawHits = (files: ReadonlyArray): ReadonlyArray => { const out: Violation[] = []; for (const file of files) { + const path = normalise(file.path); for (const [i, line] of file.text.split("\n").entries()) { - for (const { rule, re } of RULES) { + for (const { rule, re, scope } of RULES) { + if (scope && !scope.test(path)) continue; if (re.test(line)) out.push({ path: file.path, line: i + 1, rule, text: line.trim() }); } } diff --git a/src/domains/cdp-call.ts b/src/domains/cdp-call.ts index 3316633..eee2e0a 100644 --- a/src/domains/cdp-call.ts +++ b/src/domains/cdp-call.ts @@ -1,14 +1,55 @@ import type { BrowserClient } from "../client"; import type { CdpMethod, ParamsOf, ResultOf } from "../cdp/commands"; +import type { CdpError } from "../cdp/errors"; import { type Result, err } from "../util/result"; import type { ToolErr } from "../util/tool"; +export const cdpErrToToolErr = (e: CdpError, method: string): ToolErr => ({ + kind: e.kind === "timeout" ? "timeout" : "cdp_error", + message: e.message, + details: { method }, +}); + export const cdpCall = async ( client: BrowserClient, method: M, params: ParamsOf, + opts?: { timeoutMs?: number }, +): Promise, ToolErr>> => { + const r = await client.session().call(method, params, opts); + if (!r.success) return err(cdpErrToToolErr(r.error, method)); + return r; +}; + +export const cdpCallOnTarget = async ( + client: BrowserClient, + method: M, + params: ParamsOf, + sessionId: string, + opts?: { timeoutMs?: number }, +): Promise, ToolErr>> => { + const r = await client.session().callOnTarget(method, params, sessionId, opts); + if (!r.success) return err(cdpErrToToolErr(r.error, method)); + return r; +}; + +export const cdpCallBrowser = async ( + client: BrowserClient, + method: M, + params?: ParamsOf, + opts?: { timeoutMs?: number }, ): Promise, ToolErr>> => { - const r = await client.session().call(method, params); - if (!r.success) return err({ kind: "cdp_error", message: r.error.message, details: { method } }); + const r = await client.session().callBrowser(method, params, opts); + if (!r.success) return err(cdpErrToToolErr(r.error, method)); + return r; +}; + +export const evalJs = async ( + client: BrowserClient, + expression: string, + sessionId?: string, +): Promise> => { + const r = await client.evaluateJs(expression, sessionId); + if (!r.success) return err(cdpErrToToolErr(r.error, "Runtime.evaluate")); return r; }; diff --git a/src/domains/files.ts b/src/domains/files.ts index 534a674..89fdaf9 100644 --- a/src/domains/files.ts +++ b/src/domains/files.ts @@ -6,7 +6,7 @@ import { type Result, err, ok } from "../util/result"; import { defineBrowserTool, type ToolErr, type ToolOk } from "../util/tool"; import { safeJs } from "../util/js-template"; import { pdfPath } from "../util/paths"; -import { cdpCall } from "./cdp-call"; +import { cdpCall, cdpCallBrowser, evalJs } from "./cdp-call"; import { resolveRefToBackendId } from "./ref-resolve"; const UploadArgs = Type.Object({ @@ -52,8 +52,8 @@ const tryCdpUpload = async ( if (!nodeId) return err({ kind: "invalid_state", message: `Selector matched 0 file inputs: ${selector}` }); const set = await cdpCall(client, "DOM.setFileInputFiles", { files: [filePath], nodeId }); if (!set.success) return set; - const verify = await client.evaluateJs(safeJs`document.querySelector(${selector})?.files?.length || 0`); - if (!verify.success) return err({ kind: "cdp_error", message: verify.error.message }); + const verify = await evalJs(client, safeJs`document.querySelector(${selector})?.files?.length || 0`); + if (!verify.success) return verify; if (Number(verify.data ?? 0) === 0) { return err({ kind: "invalid_state", message: "CDP upload reported success but file count is 0" }); } @@ -92,8 +92,8 @@ const jsFallbackUpload = async ( return input.files.length; })() `; - const r = await client.evaluateJs(expr); - if (!r.success) return err({ kind: "cdp_error", message: r.error.message }); + const r = await evalJs(client, expr); + if (!r.success) return r; if (!Number(r.data ?? 0)) return err({ kind: "invalid_state", message: "JS fallback set 0 files" }); return ok(undefined); }; @@ -181,12 +181,12 @@ export const downloadTool = defineBrowserTool({ message: `Download path unusable: ${e instanceof Error ? e.message : String(e)}`, }); } - const r = await client.session().callBrowser("Browser.setDownloadBehavior", { + const r = await cdpCallBrowser(client, "Browser.setDownloadBehavior", { behavior: "allow", downloadPath: args.downloadPath, eventsEnabled: true, }); - if (!r.success) return err({ kind: "cdp_error", message: r.error.message }); + if (!r.success) return r; return ok({ text: `Downloads will save to: ${args.downloadPath}`, details: { downloadPath: args.downloadPath }, diff --git a/src/domains/form.ts b/src/domains/form.ts index b97b22d..8af43c3 100644 --- a/src/domains/form.ts +++ b/src/domains/form.ts @@ -3,7 +3,7 @@ import type { BrowserClient } from "../client"; import { safeJs } from "../util/js-template"; import { type Result, err, ok } from "../util/result"; import { defineBrowserTool, type ToolErr, type ToolOk } from "../util/tool"; -import { cdpCall } from "./cdp-call"; +import { cdpCall, evalJs } from "./cdp-call"; import { fillBody } from "./fill-engine"; import { interactiveDiff, resolveRefToObjectId } from "./ref-resolve"; @@ -58,8 +58,8 @@ const runOnElement = async ( const __arg = ${opts.arg}; return (function (arg) {`; const expr = `${prelude} ${opts.fnBody} }).call(el, __arg); })()`; - const r = await client.evaluateJs(expr); - if (!r.success) return err({ kind: "cdp_error", message: r.error.message }); + const r = await evalJs(client, expr); + if (!r.success) return r; return ok(r.data); }; @@ -321,14 +321,14 @@ export const resolveAndCall = async ( if (!objectId.success) return objectId; const handle = objectId.data; try { - const called = await client.session().call("Runtime.callFunctionOn", { + const called = await cdpCall(client, "Runtime.callFunctionOn", { objectId: handle, functionDeclaration, arguments: args.map((v) => ({ value: v })), returnByValue: true, awaitPromise: true, }); - if (!called.success) return err({ kind: "cdp_error", message: called.error.message, details: { ref } }); + if (!called.success) return err({ ...called.error, details: { ref } }); if (called.data.exceptionDetails !== undefined) { return err({ kind: "cdp_error", message: `page function threw: ${JSON.stringify(called.data.exceptionDetails)}`, details: { ref } }); } @@ -338,7 +338,7 @@ export const resolveAndCall = async ( } return ok(value); } finally { - await client.session().call("Runtime.releaseObject", { objectId: handle }); + await cdpCall(client, "Runtime.releaseObject", { objectId: handle }); } }; diff --git a/src/domains/isolated-tab.ts b/src/domains/isolated-tab.ts index ba00756..103c9cc 100644 --- a/src/domains/isolated-tab.ts +++ b/src/domains/isolated-tab.ts @@ -4,6 +4,7 @@ import { safeJs } from "../util/js-template"; import { ensureHarnessWindow, openHarnessTab } from "../cdp/target-factory"; import { evaluateJson } from "../cdp/evaluate"; import type { ToolErr } from "../util/tool"; +import { cdpCallBrowser, cdpCallOnTarget, cdpErrToToolErr, evalJs } from "./cdp-call"; export type IsolatedTab = { readonly targetId: string; @@ -13,32 +14,30 @@ export type IsolatedTab = { const EVAL_TIMEOUT_MS = 15_000; const READY_POLL_INTERVAL_MS = 50; -const toToolErr = (message: string, kind: ToolErr["kind"] = "cdp_error"): ToolErr => ({ kind, message }); - export const openIsolatedTab = async (client: BrowserClient): Promise> => { // Routed through the target factory so an isolated tab lands in the pinned profile, not another profile's cookies. const window = await ensureHarnessWindow(client); - if (!window.success) return err(toToolErr(window.error.message)); + if (!window.success) return err(cdpErrToToolErr(window.error, "Target.createTarget")); let targetId: string; if (window.data.freshlyCreated) { targetId = window.data.targetId; } else { const opened = await openHarnessTab(client, window.data.targetId); - if (!opened.success) return err(toToolErr(opened.error.message)); + if (!opened.success) return err(cdpErrToToolErr(opened.error, "Target.createTarget")); targetId = opened.data; } - const attached = await client.session().callBrowser("Target.attachToTarget", { targetId, flatten: true }); + const attached = await cdpCallBrowser(client, "Target.attachToTarget", { targetId, flatten: true }); if (!attached.success) { await client.closeTab(targetId); - return err(toToolErr(attached.error.message)); + return attached; } const { sessionId } = attached.data; - const enabled = await client.session().callOnTarget("Page.enable", {}, sessionId); + const enabled = await cdpCallOnTarget(client, "Page.enable", {}, sessionId); if (!enabled.success) { await client.closeTab(targetId); - return err(toToolErr(enabled.error.message)); + return enabled; } return ok({ targetId, sessionId }); }; @@ -48,10 +47,10 @@ export const navigateIsolatedTab = async ( tab: IsolatedTab, url: string, ): Promise> => { - const nav = await client.session().callOnTarget("Page.navigate", { url }, tab.sessionId, { + const nav = await cdpCallOnTarget(client, "Page.navigate", { url }, tab.sessionId, { timeoutMs: EVAL_TIMEOUT_MS, }); - if (!nav.success) return err(toToolErr(nav.error.message)); + if (!nav.success) return nav; return ok(undefined); }; @@ -63,12 +62,12 @@ export const waitForIsolatedLoad = async ( ): Promise> => { const start = Date.now(); while (Date.now() - start < timeoutMs) { - if (signal?.aborted) return err(toToolErr("aborted", "internal")); - const r = await client.evaluateJs(safeJs`document.readyState`, tab.sessionId); + if (signal?.aborted) return err({ kind: "internal", message: "aborted" }); + const r = await evalJs(client, safeJs`document.readyState`, tab.sessionId); if (r.success && r.data === "complete") return ok(undefined); await new Promise((res) => setTimeout(res, READY_POLL_INTERVAL_MS)); } - return err(toToolErr(`page did not finish loading in ${Math.round(timeoutMs / 1000)}s`, "timeout")); + return err({ kind: "timeout", message: `page did not finish loading in ${Math.round(timeoutMs / 1000)}s` }); }; export const isJsonText = (v: unknown): v is string => typeof v === "string"; @@ -83,7 +82,7 @@ export const evalInIsolatedTab = async ( sessionId: tab.sessionId, timeoutMs: EVAL_TIMEOUT_MS, }); - if (!r.success) return err(toToolErr(r.error.message, r.error.kind === "timeout" ? "timeout" : "cdp_error")); + if (!r.success) return err(cdpErrToToolErr(r.error, "Runtime.evaluate")); return ok(r.data); }; diff --git a/src/domains/js.ts b/src/domains/js.ts index a64b4ac..b43387c 100644 --- a/src/domains/js.ts +++ b/src/domains/js.ts @@ -7,6 +7,7 @@ import { Markdown, Text } from "@mariozechner/pi-tui"; import { getMarkdownTheme, keyHint } from "@mariozechner/pi-coding-agent"; import { type Result, err, ok } from "../util/result"; import { defineBrowserTool, type ToolErr, type ToolOk } from "../util/tool"; +import { evalJs } from "./cdp-call"; import { applyTruncation } from "../util/truncate"; import { asNumber, asRecord, asString, isRecord } from "../util/guards"; @@ -47,8 +48,8 @@ export const executeJsTool = defineBrowserTool({ parameters: ExecuteJsArgs, concurrency: "parallel", async handler(args, { client }): Promise> { - const r = await client.evaluateJs(args.expression, args.targetId); - if (!r.success) return err({ kind: "cdp_error", message: r.error.message }); + const r = await evalJs(client, args.expression, args.targetId); + if (!r.success) return r; const valueStr = r.data === undefined ? "undefined" : JSON.stringify(r.data); const truncated = await applyTruncation(valueStr, "js"); let pretty: string | undefined; diff --git a/src/domains/keyboard.ts b/src/domains/keyboard.ts index f0f1a6d..50bee62 100644 --- a/src/domains/keyboard.ts +++ b/src/domains/keyboard.ts @@ -3,7 +3,7 @@ import { safeJs } from "../util/js-template"; import { virtualKeyCode } from "../util/keycodes"; import { type Result, err, ok } from "../util/result"; import { defineBrowserTool, type ToolErr, type ToolOk } from "../util/tool"; -import { cdpCall } from "./cdp-call"; +import { cdpCall, evalJs } from "./cdp-call"; import { resolveRefToObjectId } from "./ref-resolve"; const TypeArgs = Type.Object({ @@ -29,7 +29,7 @@ export const typeTool = defineBrowserTool({ parameters: TypeArgs, concurrency: "serialized", async handler(args, { client }): Promise> { - const focused = await client.evaluateJs(safeJs` + const focused = await evalJs(client, safeJs` (() => { const el = document.activeElement; const ok = !!el && el !== document.body && @@ -171,8 +171,8 @@ export const dispatchKeyTool = defineBrowserTool({ return els.length; })() `; - const r = await client.evaluateJs(expr); - if (!r.success) return err({ kind: "cdp_error", message: r.error.message }); + const r = await evalJs(client, expr); + if (!r.success) return r; const matched = Number(r.data ?? 0); if (matched === 0) { return err({ diff --git a/src/domains/navigate.ts b/src/domains/navigate.ts index 86cc272..8afea6f 100644 --- a/src/domains/navigate.ts +++ b/src/domains/navigate.ts @@ -6,7 +6,7 @@ import { applyTruncation } from "../util/truncate"; import { safeJs } from "../util/js-template"; import { attachTo } from "../cdp/attach"; import { ensureHarnessWindow, openHarnessTab } from "../cdp/target-factory"; -import { cdpCall } from "./cdp-call"; +import { cdpCall, cdpCallBrowser, cdpCallOnTarget, evalJs } from "./cdp-call"; const NavigateArgs = Type.Object({ url: Type.String({ description: "Full URL to navigate to (e.g. https://github.com)" }), @@ -110,11 +110,12 @@ export const openUrlsTool = defineBrowserTool({ const attached = await attachTo(client.session(), tab.targetId); if (!attached.success) return { ...tab, ok: false, error: attached.error.message }; const sid = attached.data; - const enabled = await client.session().callOnTarget("Page.enable", {}, sid); + const enabled = await cdpCallOnTarget(client, "Page.enable", {}, sid); if (!enabled.success) return { ...tab, ok: false, error: enabled.error.message }; - const nav = await client.session().callOnTarget("Page.navigate", { url: tab.url }, sid); + const nav = await cdpCallOnTarget(client, "Page.navigate", { url: tab.url }, sid); if (!nav.success) return { ...tab, ok: false, error: nav.error.message }; - client.evaluateJs( + evalJs( + client, safeJs`if(!document.title.startsWith('🟢'))document.title='🟢 '+document.title`, sid ).catch(() => {}); @@ -135,7 +136,7 @@ export const openUrlsTool = defineBrowserTool({ if (okTabs.length > 0) { const last = okTabs[okTabs.length - 1]; if (last) { - await client.session().callBrowser("Target.activateTarget", { targetId: last.targetId }); + await cdpCallBrowser(client, "Target.activateTarget", { targetId: last.targetId }); } } const lines: string[] = []; diff --git a/src/domains/network.ts b/src/domains/network.ts index fc54eb2..5520f40 100644 --- a/src/domains/network.ts +++ b/src/domains/network.ts @@ -3,6 +3,7 @@ import { Markdown, Text } from "@mariozechner/pi-tui"; import { getMarkdownTheme, keyHint } from "@mariozechner/pi-coding-agent"; import { type Result, err, ok } from "../util/result"; import { defineBrowserTool, type ToolErr, type ToolOk } from "../util/tool"; +import { cdpCall } from "./cdp-call"; import { applyTruncation } from "../util/truncate"; import { asArrayOf, asBoolean, asNumber, asRecord, asString } from "../util/guards"; import type { NetworkRecord } from "../cdp/network-buffer"; @@ -250,7 +251,7 @@ export const networkRequestsTool = defineBrowserTool({ const timer = setTimeout(() => ac.abort(), BODY_BUDGET_MS); const fills = drained.records.map(async (r) => { if (ac.signal.aborted) return; - const bodyRes = await session.call("Network.getResponseBody", { requestId: r.requestId }); + const bodyRes = await cdpCall(client, "Network.getResponseBody", { requestId: r.requestId }); if (!bodyRes.success) { r.body = null; return; diff --git a/src/domains/page.ts b/src/domains/page.ts index 0d7aa8b..c202ef1 100644 --- a/src/domains/page.ts +++ b/src/domains/page.ts @@ -1,6 +1,7 @@ import { Type } from "typebox"; import { type Result, err, ok } from "../util/result"; import { defineBrowserTool, type ToolErr, type ToolOk } from "../util/tool"; +import { evalJs } from "./cdp-call"; import { sleep } from "../util/time"; import { safeJs } from "../util/js-template"; @@ -79,7 +80,7 @@ export const waitForLoadTool = defineBrowserTool({ const start = Date.now(); while (Date.now() - start < timeoutMs) { if (signal?.aborted) return err({ kind: "internal", message: "aborted" }); - const r = await client.evaluateJs(safeJs`document.readyState`); + const r = await evalJs(client, safeJs`document.readyState`); if (r.success && r.data === "complete") { const ms = Date.now() - start; return ok({ text: `Page loaded in ${Math.round(ms / 100) / 10}s`, details: { ms } }); @@ -123,7 +124,7 @@ export const waitForTool = defineBrowserTool({ const start = Date.now(); while (Date.now() - start < timeoutMs) { if (signal?.aborted) return err({ kind: "internal", message: "aborted" }); - const r = await client.evaluateJs(expr); + const r = await evalJs(client, expr); if (r.success && r.data === true) { const ms = Date.now() - start; const what = args.selector !== undefined ? `${args.selector}${gone ? " gone" : ""}` : `text "${args.text}"`; diff --git a/src/domains/readpage/read-page.ts b/src/domains/readpage/read-page.ts index 0a6fadc..9d46c7e 100644 --- a/src/domains/readpage/read-page.ts +++ b/src/domains/readpage/read-page.ts @@ -4,6 +4,7 @@ import { parseJson } from "../../schemas/parse"; import { type Result, err, ok } from "../../util/result"; import { applyTruncation } from "../../util/truncate"; import { defineBrowserTool, type ToolErr, type ToolOk } from "../../util/tool"; +import { cdpCallBrowser, evalJs } from "../cdp-call"; import { closeIsolatedTab, evalInIsolatedTab, @@ -107,11 +108,11 @@ const readOwnedTab = async ( if (!client.owns(targetId)) { return err({ kind: "invalid_state", message: `Tab ${targetId} is not owned by this session.` }); } - const attached = await client.session().callBrowser("Target.attachToTarget", { targetId, flatten: true }); - if (!attached.success) return err({ kind: "cdp_error", message: attached.error.message }); + const attached = await cdpCallBrowser(client, "Target.attachToTarget", { targetId, flatten: true }); + if (!attached.success) return attached; const sessionId = attached.data.sessionId; - const captured = await client.evaluateJs(buildPageCaptureExpr(), sessionId); - if (!captured.success) return err({ kind: "cdp_error", message: captured.error.message }); + const captured = await evalJs(client, buildPageCaptureExpr(), sessionId); + if (!captured.success) return captured; return captureToResult(captured.data); }; diff --git a/src/domains/ref-resolve.ts b/src/domains/ref-resolve.ts index ece308e..0f509a0 100644 --- a/src/domains/ref-resolve.ts +++ b/src/domains/ref-resolve.ts @@ -2,6 +2,7 @@ import type { BrowserClient } from "../client"; import { type Result, err, ok } from "../util/result"; import type { ToolErr } from "../util/tool"; import { type Box, boxOf } from "./box"; +import { cdpCall } from "./cdp-call"; import { buildTree, collectInteractiveTargets, type SlimNode } from "./snapshot"; const staleErr = (ref: string): ToolErr => ({ @@ -16,7 +17,7 @@ export const resolveRefToObjectId = async ( ): Promise> => { const backendId = client.session().resolveRef(ref); if (backendId === undefined) return err(staleErr(ref)); - const resolved = await client.session().call("DOM.resolveNode", { backendNodeId: backendId }); + const resolved = await cdpCall(client, "DOM.resolveNode", { backendNodeId: backendId }); if (!resolved.success) { return err(staleErr(ref)); } @@ -49,7 +50,7 @@ export const interactiveDiff = async (client: BrowserClient): Promise => const session = client.session(); const prev = session.refSignatures(); - const axRes = await session.call("Accessibility.getFullAXTree", {}); + const axRes = await cdpCall(client, "Accessibility.getFullAXTree", {}); if (!axRes.success) return ""; const rawNodes = axRes.data.nodes; const slim = buildTree(rawNodes, { interestingOnly: true, maxNodes: 1000 }); diff --git a/src/domains/screenshot.ts b/src/domains/screenshot.ts index 04f0fa9..4b5732d 100644 --- a/src/domains/screenshot.ts +++ b/src/domains/screenshot.ts @@ -9,7 +9,7 @@ import { screenshotPath } from "../util/paths"; import { loadSharp } from "../util/sharp-shim"; import { asRecord, asString } from "../util/guards"; import { safeJs } from "../util/js-template"; -import { cdpCall } from "./cdp-call"; +import { cdpCall, evalJs } from "./cdp-call"; const ScreenshotArgs = Type.Object({ fullPage: Type.Optional(Type.Boolean({ default: false, description: "Capture beyond viewport" })), @@ -133,7 +133,7 @@ export const captureWithCrosshair = async ( const load = await loadSharp(); if (load.kind !== "ok") return ok({ path: cap.data.path }); try { - const dprR = await client.evaluateJs(safeJs`window.devicePixelRatio`); + const dprR = await evalJs(client, safeJs`window.devicePixelRatio`); const dpr = dprR.success && typeof dprR.data === "number" ? dprR.data : 1; const meta = await load.sharp(cap.data.path).metadata(); const w = meta.width ?? 0; diff --git a/src/domains/snapshot.ts b/src/domains/snapshot.ts index b33c8f9..848ebb1 100644 --- a/src/domains/snapshot.ts +++ b/src/domains/snapshot.ts @@ -2,7 +2,7 @@ import { readFileSync, writeFileSync } from "node:fs"; import { Type } from "typebox"; import { Container, Image, type ImageTheme, Markdown, Text, truncateToWidth, visibleWidth } from "@mariozechner/pi-tui"; import { getMarkdownTheme, keyHint } from "@mariozechner/pi-coding-agent"; -import type { CdpSession } from "../cdp/session"; +import type { BrowserClient } from "../client"; import type { AxNodeResult } from "../cdp/commands"; import { type Box, boxOf } from "./box"; import { cdpCall } from "./cdp-call"; @@ -229,8 +229,8 @@ export const collectInteractiveTargets = ( return out; }; -const liveValue = async (session: CdpSession, backendId: number): Promise => { - const resolved = await session.call("DOM.resolveNode", { backendNodeId: backendId }); +const liveValue = async (client: BrowserClient, backendId: number): Promise => { + const resolved = await cdpCall(client, "DOM.resolveNode", { backendNodeId: backendId }); if (!resolved.success) return undefined; const objectId = resolved.data.object.objectId; if (!objectId) return undefined; @@ -239,7 +239,7 @@ const liveValue = async (session: CdpSession, backendId: number): Promise> { const cur = client.current(); if (!cur) return err({ kind: "invalid_state", message: "No tab attached." }); - const ti = await client.session().callBrowser("Target.getTargetInfo", { targetId: cur.targetId }); - if (!ti.success) return err({ kind: "cdp_error", message: ti.error.message }); + const ti = await cdpCallBrowser(client, "Target.getTargetInfo", { targetId: cur.targetId }); + if (!ti.success) return ti; const info = ti.data.targetInfo; const owned = client.owns(info.targetId); return ok({ diff --git a/test/util/check-boundaries-test.ts b/test/util/check-boundaries-test.ts index b42baa0..f9eb7eb 100644 --- a/test/util/check-boundaries-test.ts +++ b/test/util/check-boundaries-test.ts @@ -97,6 +97,40 @@ describe("boundary check", () => { assert.equal(checkSource([{ path: "a.ts", text }], noExemptions).length, 0); }); + test("flags a tool reaching the session directly", () => { + const forms = [ + "const r = await client.session().call(\"DOM.resolveNode\", { backendNodeId: id });", + "const r = await client.session().callBrowser(\"Target.getTargets\");", + "const r = await client.session().callOnTarget(\"Page.enable\", {}, sid);", + "const r = await session.call(\"Accessibility.getFullAXTree\", {});", + "const r = await session.callBrowser(\"Browser.getWindowForTarget\", { targetId });", + ]; + for (const text of forms) { + const v = checkSource([{ path: "src/domains/thing.ts", text }], noExemptions); + assert.equal(v.some((x) => x.rule === "raw-cdp-call"), true, text); + } + }); + + test("flags a tool calling evaluateJs directly", () => { + const v = checkSource( + [{ path: "src/domains/thing.ts", text: "const r = await client.evaluateJs(expr);" }], + noExemptions, + ); + assert.equal(v.some((x) => x.rule === "raw-evaluate"), true); + }); + + test("does not flag the session layer, which owns the session", () => { + const text = 'const r = await session.callBrowser("Target.getTargets");\nawait client.evaluateJs(expr);'; + for (const path of ["src/client.ts", "src/cdp/session.ts", "src/cdp/target-factory.ts"]) { + assert.equal(checkSource([{ path, text }], noExemptions).length, 0, path); + } + }); + + test("does not mistake Function.prototype.call in an evaluated snippet for a CDP call", () => { + const text = "const expr = `${prelude} ${opts.fnBody} }).call(el, __arg); })()`;"; + assert.equal(checkSource([{ path: "src/domains/form.ts", text }], noExemptions).length, 0); + }); + test("reports the 1-indexed line number and the trimmed text", () => { const v = checkSource([{ path: "a.ts", text: "const a = 1;\n const b = c as any;" }], noExemptions); assert.equal(v[0]?.line, 2); -- cgit v1.2.3