test(cdp): cover call/callOnTarget/callBrowser through a stub transport; collapse decode into one path

session-call-test.ts previously only exercised decodeResult (Task 3's
surface, and mostly duplicating commands-test.ts) — call, callOnTarget,
callBrowser, and the session's internal req helper had zero coverage.
Add a stub CdpTransport (no socket, no browser) and test: a valid payload
decodes to typed data, a garbage payload becomes invalid_response without
throwing, a transport-level error passes through with its original kind
(not relabeled), call/callOnTarget/callBrowser route the session id
correctly, and opts.timeoutMs reaches the transport only when provided.

Also collapse the four near-identical transport-then-decode bodies (req,
call, callOnTarget, callBrowser) into one: the three public methods now
delegate to req, which is the sole place unknown ends for every CDP call
the session makes, internal or public. Public signatures are unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

2 files changed, 135 insertions, 38 deletions

diff --git a/src/cdp/session.ts b/src/cdp/session.ts
index 96b980f..dd5a9b3 100644
--- a/src/cdp/session.ts
+++ b/src/cdp/session.ts
@@ -180,16 +180,22 @@ export const createCdpSession = (
     }
   };
 
-  // Internal helper: the session's own CDP calls (attach/switch bookkeeping)
-  // go through the same decode-on-call path as the public call/callOnTarget/
-  // callBrowser — the session is where `unknown` ends, including for its own
-  // requests, not just the ones exposed to callers.
+  // The one place `unknown` ends for every CDP call this session makes — its
+  // own internal bookkeeping calls (attach/switch) and the public
+  // call/callOnTarget/callBrowser all funnel through here. A single
+  // transport-then-decode implementation means a future change (tracing,
+  // error wrapping, the short-circuit on failure) lands in one place, and a
+  // new internal call site can't forget to decode.
   const req = async <M extends CdpMethod>(
     method: M,
-    params: ParamsOf<M>,
+    params: ParamsOf<M> | undefined,
     sid: string | null,
+    opts: { timeoutMs?: number } = {},
   ): Promise<Result<ResultOf<M>, CdpError>> => {
-    const raw = await transport.request(method, { ...params }, { sessionId: sid });
+    const raw = await transport.request(method, { ...(params ?? {}) }, {
+      sessionId: sid,
+      ...(opts.timeoutMs !== undefined ? { timeoutMs: opts.timeoutMs } : {}),
+    });
     if (!raw.success) return raw;
     return decodeResult(method, raw.data);
   };
@@ -358,29 +364,14 @@ export const createCdpSession = (
       const tab = targetId ? tabs.get(targetId) : undefined;
       return tab?.refSig ?? new Map();
     },
-    async call(method, params, opts = {}) {
-      const raw = await transport.request(method, { ...(params ?? {}) }, {
-        sessionId,
-        ...(opts.timeoutMs !== undefined ? { timeoutMs: opts.timeoutMs } : {}),
-      });
-      if (!raw.success) return raw;
-      return decodeResult(method, raw.data);
+    call(method, params, opts) {
+      return req(method, params, sessionId, opts);
     },
-    async callOnTarget(method, params, sid, opts = {}) {
-      const raw = await transport.request(method, { ...params }, {
-        sessionId: sid,
-        ...(opts.timeoutMs !== undefined ? { timeoutMs: opts.timeoutMs } : {}),
-      });
-      if (!raw.success) return raw;
-      return decodeResult(method, raw.data);
+    callOnTarget(method, params, sid, opts) {
+      return req(method, params, sid, opts);
     },
-    async callBrowser(method, params, opts = {}) {
-      const raw = await transport.request(method, { ...(params ?? {}) }, {
-        sessionId: null,
-        ...(opts.timeoutMs !== undefined ? { timeoutMs: opts.timeoutMs } : {}),
-      });
-      if (!raw.success) return raw;
-      return decodeResult(method, raw.data);
+    callBrowser(method, params, opts) {
+      return req(method, params, null, opts);
     },
     takeDialog() {
       const tab = targetId ? tabs.get(targetId) : undefined;
diff --git a/test/cdp/session-call-test.ts b/test/cdp/session-call-test.ts
index b318eb4..7b1386b 100644
--- a/test/cdp/session-call-test.ts
+++ b/test/cdp/session-call-test.ts
@@ -1,28 +1,134 @@
 import { test, describe } from "node:test";
 import assert from "node:assert/strict";
 import { decodeResult } from "../../src/cdp/commands";
+import { createCdpSession } from "../../src/cdp/session";
+import type { CdpTransport } from "../../src/cdp/transport";
+import type { CdpEvent } from "../../src/cdp/types";
+import { type Result, ok, err } from "../../src/util/result";
+import { type CdpError, cdpError } from "../../src/cdp/errors";
+
+type RequestOpts = { readonly sessionId?: string | null; readonly timeoutMs?: number };
+type RecordedCall = {
+  readonly method: string;
+  readonly params: Record<string, unknown>;
+  readonly opts: RequestOpts | undefined;
+};
+
+/**
+ * A stub transport: no socket, no browser. `request()` looks up a canned
+ * Result by method name (defaulting to `ok({})` for methods the test doesn't
+ * care about — e.g. the six domain-enable calls switchTo fires as a side
+ * effect). Every call is recorded so tests can assert on exactly what the
+ * session sent, including the sessionId routing and the timeoutMs option.
+ */
+const makeStubTransport = (
+  responses: Record<string, Result<unknown, CdpError>> = {},
+): { transport: CdpTransport; calls: RecordedCall[] } => {
+  const calls: RecordedCall[] = [];
+  const transport: CdpTransport = {
+    connect: () => Promise.resolve(ok(undefined)),
+    close: () => Promise.resolve(),
+    request: (method, params, opts) => {
+      calls.push({ method, params, opts });
+      return Promise.resolve(responses[method] ?? ok({}));
+    },
+    // No real events ever arrive; the session's background consumer just
+    // awaits forever. No timers involved, so it doesn't keep the process alive.
+    events: (): AsyncIterable<CdpEvent> => (async function* (): AsyncGenerator<CdpEvent> {
+      await new Promise<never>(() => {});
+    })(),
+    state: () => "open",
+    onClose: () => () => {},
+  };
+  return { transport, calls };
+};
 
 describe("typed call decoding", () => {
-  test("a valid transport payload decodes to typed data", () => {
-    const fromTransport: unknown = { sessionId: "abc123" };
-    const r = decodeResult("Target.attachToTarget", fromTransport);
+  test("extra Chrome fields on a result are tolerated", () => {
+    const r = decodeResult("Target.getTargetInfo", {
+      targetInfo: { targetId: "t", type: "page", title: "T", url: "u", futureField: 1 },
+    });
+    assert.equal(r.success, true);
+  });
+});
+
+describe("CdpSession.call / callOnTarget / callBrowser", () => {
+  test("(a) a valid stub payload decodes to typed data", async () => {
+    const { transport } = makeStubTransport({
+      "Target.getTargetInfo": ok({ targetInfo: { targetId: "t1", type: "page", title: "T", url: "u" } }),
+    });
+    const session = createCdpSession(transport);
+    const r = await session.call("Target.getTargetInfo", {});
     assert.equal(r.success, true);
     if (r.success) {
-      const sid: string = r.data.sessionId;
-      assert.equal(sid, "abc123");
+      const targetId: string = r.data.targetInfo.targetId;
+      assert.equal(targetId, "t1");
     }
   });
 
-  test("a garbage transport payload becomes invalid_response", () => {
-    const r = decodeResult("Target.attachToTarget", { sessionId: 42 });
+  test("(b) a garbage stub payload becomes invalid_response, without throwing", async () => {
+    const { transport } = makeStubTransport({
+      "Target.getTargetInfo": ok({ targetInfo: { targetId: 42 } }),
+    });
+    const session = createCdpSession(transport);
+    const r = await session.call("Target.getTargetInfo", {});
     assert.equal(r.success, false);
     if (!r.success) assert.equal(r.error.kind, "invalid_response");
   });
 
-  test("extra Chrome fields on a result are tolerated", () => {
-    const r = decodeResult("Target.getTargetInfo", {
-      targetInfo: { targetId: "t", type: "page", title: "T", url: "u", futureField: 1 },
+  test("(c) a transport-level error passes through with its original kind", async () => {
+    const { transport } = makeStubTransport({
+      "Target.getTargetInfo": err(cdpError("timeout", "socket dead", "Target.getTargetInfo")),
     });
-    assert.equal(r.success, true);
+    const session = createCdpSession(transport);
+    const r = await session.call("Target.getTargetInfo", {});
+    assert.equal(r.success, false);
+    if (!r.success) {
+      // Must stay "timeout" — decodeResult must never run on a failed
+      // transport call and relabel it "invalid_response".
+      assert.equal(r.error.kind, "timeout");
+      assert.equal(r.error.message, "socket dead");
+    }
+  });
+
+  test("(d) call/callOnTarget/callBrowser route the session id correctly", async () => {
+    const { transport, calls } = makeStubTransport({
+      "Target.attachToTarget": ok({ sessionId: "session-xyz" }),
+    });
+    const session = createCdpSession(transport);
+
+    // Establish an internal sessionId distinct from both null and the
+    // explicit sid used below, via the public switchTo() — no attachFirstPage
+    // bookkeeping required.
+    const switched = await session.switchTo("target-1");
+    assert.equal(switched.success, true);
+
+    calls.length = 0; // discard switchTo's own requests (activate/attach/enable*6)
+
+    await session.call("Page.navigate", { url: "http://example.com" });
+    await session.callOnTarget("Page.navigate", { url: "http://example.com" }, "explicit-sid");
+    await session.callBrowser("Target.getTargets");
+
+    const navigateCalls = calls.filter((c) => c.method === "Page.navigate");
+    assert.equal(navigateCalls[0]?.opts?.sessionId, "session-xyz"); // call() uses the active session
+    assert.equal(navigateCalls[1]?.opts?.sessionId, "explicit-sid"); // callOnTarget() uses the given sid, not the active one
+
+    const browserCall = calls.find((c) => c.method === "Target.getTargets");
+    assert.equal(browserCall?.opts?.sessionId, null); // callBrowser() forces null even though a session is active
+  });
+
+  test("opts.timeoutMs reaches the transport only when provided (exactOptionalPropertyTypes)", async () => {
+    const { transport, calls } = makeStubTransport();
+    const session = createCdpSession(transport);
+
+    await session.callBrowser("Target.getTargets", {}, { timeoutMs: 5_000 });
+    const withTimeout = calls.find((c) => c.method === "Target.getTargets");
+    assert.equal(withTimeout?.opts?.timeoutMs, 5_000);
+
+    await session.callBrowser("Target.getTargets");
+    const withoutTimeout = calls.filter((c) => c.method === "Target.getTargets")[1];
+    // The key itself must be absent, not present-with-undefined — that's the
+    // distinction exactOptionalPropertyTypes exists to enforce at the call site.
+    assert.equal(withoutTimeout?.opts && "timeoutMs" in withoutTimeout.opts, false);
   });
 });