fix(browser): harden waitFor event handling

packages/bcode-browser/skills/browser-execute/SKILL.md

@@ -114,9 +114,7 @@ Common moves:
 
 ```js
 // Navigate.
-await session.Page.enable()
-await session.Page.navigate({ url: "https://example.com" })
-await session.waitFor("Page.loadEventFired")
+await session.navigate("https://example.com")
 
 // Evaluate JS in the page.
 const r = await session.Runtime.evaluate({
@@ -151,10 +149,8 @@ Imports work at any depth; pick whatever layout makes the project easiest to nav
 // ./.bcode/agent-workspace/scrape_titles.ts (you write this with the `write` tool)
 export async function scrapeTitles(session: any, urls: string[]) {
   const titles: string[] = []
-  await session.Page.enable()
   for (const url of urls) {
-    await session.Page.navigate({ url })
-    await session.waitFor("Page.loadEventFired")
+    await session.navigate(url)
     const r = await session.Runtime.evaluate({ expression: "document.title", returnByValue: true })
     titles.push(r.result.value)
   }

packages/bcode-browser/src/cdp/session.ts

@@ -6,7 +6,7 @@
  * Target.sendMessageToTarget envelopes).
  */
 
-import { bindDomains, type Domains, type Transport } from './generated.ts';
+import { bindDomains, type Domains, type Page, type Transport } from './generated.ts';
 
 type Pending = {
   resolve: (v: unknown) => void;
@@ -25,6 +25,23 @@ export type ConnectOptions = {
   timeoutMs?: number;
 };
 
+export type WaitForOptions<T> = {
+  /** Only resolve when the event payload matches this predicate. */
+  predicate?: (params: T) => boolean;
+  /** Maximum wait in ms. Default 30000. */
+  timeoutMs?: number;
+};
+
+export type NavigateOptions = {
+  /** Maximum wait for a cross-document load event in ms. Default 30000. */
+  timeoutMs?: number;
+};
+
+type EventWaiter<T> = {
+  promise: Promise<T>;
+  cancel: () => void;
+};
+
 /** A Chromium-based browser detected as running on this machine. */
 export type DetectedBrowser = {
   /** Short label, e.g. 'Google Chrome', 'Brave', 'Comet'. */
@@ -189,20 +206,96 @@ export class Session implements Transport {
   }
 
   /** Wait for the next event matching `method` (and optional predicate). */
-  waitFor<T = unknown>(method: string, predicate?: (params: T) => boolean, timeoutMs = 30_000): Promise<T> {
-    return new Promise((resolve, reject) => {
-      const timer = setTimeout(() => {
+  waitFor<T = unknown>(method: string, options?: WaitForOptions<T>): Promise<T>;
+  waitFor<T = unknown>(method: string, predicate?: (params: T) => boolean, timeoutMs?: number): Promise<T>;
+  waitFor<T = unknown>(
+    method: string,
+    predicateOrOptions?: ((params: T) => boolean) | WaitForOptions<T>,
+    positionalTimeoutMs?: number,
+  ): Promise<T> {
+    if (
+      predicateOrOptions !== undefined &&
+      typeof predicateOrOptions !== 'function' &&
+      (typeof predicateOrOptions !== 'object' || predicateOrOptions === null || Array.isArray(predicateOrOptions))
+    ) {
+      throw new TypeError('waitFor expects a predicate function or an options object');
+    }
+    const options = typeof predicateOrOptions === 'object' ? predicateOrOptions : undefined;
+    if (options?.predicate !== undefined && typeof options.predicate !== 'function') {
+      throw new TypeError('waitFor options.predicate must be a function');
+    }
+    const predicate = typeof predicateOrOptions === 'function' ? predicateOrOptions : options?.predicate;
+    const timeoutMs = options?.timeoutMs ?? positionalTimeoutMs ?? 30_000;
+    const sessionId = isBrowserLevel(method) ? undefined : this.activeSessionId;
+    if (!Number.isFinite(timeoutMs) || timeoutMs < 0) {
+      throw new TypeError('waitFor timeoutMs must be a non-negative finite number');
+    }
+
+    return this.createEventWaiter(method, predicate, timeoutMs, sessionId).promise;
+  }
+
+  private createEventWaiter<T>(
+    method: string,
+    predicate: ((params: T) => boolean) | undefined,
+    timeoutMs: number,
+    sessionId: string | undefined,
+  ): EventWaiter<T> {
+    let cancel = () => {};
+    const promise = new Promise<T>((resolve, reject) => {
+      let settled = false;
+      const cleanup = () => {
+        if (settled) return;
+        settled = true;
+        clearTimeout(timer);
         unsub();
+      };
+      const timer = setTimeout(() => {
+        cleanup();
         reject(new Error(`Timeout waiting for ${method}`));
       }, timeoutMs);
-      const unsub = this.onEvent((m, params) => {
+      const unsub = this.onEvent((m, params, eventSessionId) => {
         if (m !== method) return;
-        if (predicate && !predicate(params as T)) return;
-        clearTimeout(timer);
-        unsub();
+        if (sessionId !== undefined && eventSessionId !== sessionId) return;
+        try {
+          if (predicate && !predicate(params as T)) return;
+        } catch (error) {
+          cleanup();
+          reject(error);
+          return;
+        }
+        cleanup();
         resolve(params as T);
       });
+      cancel = cleanup;
     });
+    return { promise, cancel };
+  }
+
+  /**
+   * Navigate the active page and wait for cross-document loads.
+   * Same-document and download navigations do not emit Page.loadEventFired.
+   */
+  async navigate(url: string, options: NavigateOptions = {}): Promise<Page.NavigateReturn> {
+    const timeoutMs = options.timeoutMs ?? 30_000;
+    if (!Number.isFinite(timeoutMs) || timeoutMs < 0) {
+      throw new TypeError('navigate timeoutMs must be a non-negative finite number');
+    }
+    await this.domains.Page.enable();
+    const waiter = this.createEventWaiter<unknown>(
+      'Page.loadEventFired',
+      undefined,
+      timeoutMs,
+      this.activeSessionId,
+    );
+    void waiter.promise.catch(() => {});
+    try {
+      const navigation = await this.domains.Page.navigate({ url });
+      if (navigation.errorText) throw new Error(`Navigation failed: ${navigation.errorText}`);
+      if (navigation.loaderId && !navigation.isDownload) await waiter.promise;
+      return navigation;
+    } finally {
+      waiter.cancel();
+    }
   }
 
   // Transport implementation. Called by the generated domain bindings.
@@ -423,4 +516,3 @@ async function tryReadDevToolsActivePort(
     return undefined;
   }
 }
-

packages/bcode-browser/test/browser-execute.test.ts

@@ -86,9 +86,7 @@ test.skipIf(!enabled)("workspace import inside a snippet", async () => {
        } else {
          await session.use(page.targetId)
        }
-       await session.Page.enable()
-       await session.Page.navigate({ url: "data:text/html,<title>bcode-be</title>" })
-       await session.waitFor("Page.loadEventFired", undefined, 5000)
+       await session.navigate("data:text/html,<title>bcode-be</title>", { timeoutMs: 5000 })
        const r = await session.Runtime.evaluate({ expression: "document.title", returnByValue: true })
        return r.result.value
      }`,
@@ -124,9 +122,7 @@ test.skipIf(!enabled)("Page.captureScreenshot is collected into result.screensho
         return yield* impl.execute(
           {
             description: "Capture two screenshots",
-            code: `await session.Page.enable();
-                   await session.Page.navigate({ url: "data:text/html,<title>shot</title><body>hi" });
-                   await session.waitFor("Page.loadEventFired", undefined, 5000);
+            code: `await session.navigate("data:text/html,<title>shot</title><body>hi", { timeoutMs: 5000 });
                    const a = await session.Page.captureScreenshot({ format: "png" });
                    const b = await session.Page.captureScreenshot({ format: "jpeg", quality: 50 });
                    return { aLen: a.data.length, bLen: b.data.length };`,

packages/bcode-browser/test/cdp-session.test.ts

@@ -0,0 +1,142 @@
+import { afterAll, beforeAll, expect, test } from "bun:test"
+import { Session } from "../src/cdp/session"
+
+const channel = "cdp-events"
+const server = Bun.serve({
+  port: 0,
+  fetch(req, srv) {
+    return srv.upgrade(req) ? undefined : new Response("nope", { status: 400 })
+  },
+  websocket: {
+    open(ws) {
+      ws.subscribe(channel)
+    },
+    message(ws, raw) {
+      const message: unknown = JSON.parse(String(raw))
+      if (typeof message !== "object" || message === null) return
+      const method = Reflect.get(message, "method")
+      const id = Reflect.get(message, "id")
+      if (typeof id !== "number") return
+      if (method === "Page.enable") {
+        ws.send(JSON.stringify({ id, result: {} }))
+        return
+      }
+      if (method !== "Page.navigate") return
+      const params = Reflect.get(message, "params")
+      const url = typeof params === "object" && params !== null ? Reflect.get(params, "url") : undefined
+      if (url === "https://example.com/#same-document") {
+        ws.send(JSON.stringify({ id, result: { frameId: "frame" } }))
+        return
+      }
+      if (url === "https://navigation-fails.example") {
+        ws.send(JSON.stringify({ id, result: { frameId: "frame", errorText: "net::ERR_FAILED" } }))
+        return
+      }
+      if (url === "https://download.example") {
+        ws.send(JSON.stringify({ id, result: { frameId: "frame", loaderId: "loader", isDownload: true } }))
+        return
+      }
+      ws.send(JSON.stringify({ method: "Page.loadEventFired", params: { timestamp: 1 } }))
+      ws.send(JSON.stringify({ id, result: { frameId: "frame", loaderId: "loader" } }))
+    },
+    close() {},
+  },
+})
+const session = new Session()
+
+beforeAll(async () => {
+  await session.connect({ wsUrl: `ws://127.0.0.1:${server.port}/` })
+})
+
+afterAll(() => {
+  session.close()
+  server.stop(true)
+})
+
+const emit = (method: string, params: unknown, sessionId?: string) => {
+  server.publish(channel, JSON.stringify({ method, params, sessionId }))
+}
+
+test("waitFor accepts predicate and timeout options", async () => {
+  const waiting = session.waitFor<{ ready: boolean }>("Test.options", {
+    predicate: (params) => params.ready,
+    timeoutMs: 1_000,
+  })
+  emit("Test.options", { ready: false })
+  emit("Test.options", { ready: true })
+  expect(await waiting).toEqual({ ready: true })
+})
+
+test("waitFor options timeout is honored", async () => {
+  const started = performance.now()
+  await expect(session.waitFor("Test.timeout", { timeoutMs: 20 })).rejects.toThrow("Timeout waiting for Test.timeout")
+  expect(performance.now() - started).toBeLessThan(500)
+})
+
+test("waitFor rejects and unsubscribes when a predicate throws", async () => {
+  let calls = 0
+  const waiting = session.waitFor("Test.predicate-error", {
+    predicate: () => {
+      calls++
+      throw new Error("predicate failed")
+    },
+    timeoutMs: 1_000,
+  })
+  emit("Test.predicate-error", {})
+  await expect(waiting).rejects.toThrow("predicate failed")
+  emit("Test.predicate-error", {})
+  await Bun.sleep(10)
+  expect(calls).toBe(1)
+})
+
+test("waitFor retains the positional signature", async () => {
+  const waiting = session.waitFor<{ ready: boolean }>("Test.positional", (params) => params.ready, 1_000)
+  emit("Test.positional", { ready: true })
+  expect(await waiting).toEqual({ ready: true })
+})
+
+test("a waiter registered before navigation catches an event emitted before the navigation response", async () => {
+  const navigation = await session.navigate("https://example.com", { timeoutMs: 1_000 })
+  expect(navigation.loaderId).toBe("loader")
+})
+
+test("navigate does not wait for same-document loads or downloads", async () => {
+  const sameDocument = await session.navigate("https://example.com/#same-document", { timeoutMs: 20 })
+  expect((session as any).eventListeners).toHaveLength(0)
+  const download = await session.navigate("https://download.example", { timeoutMs: 20 })
+  expect(sameDocument.loaderId).toBeUndefined()
+  expect(download.isDownload).toBe(true)
+  expect((session as any).eventListeners).toHaveLength(0)
+})
+
+test("navigate surfaces Page.navigate errorText", async () => {
+  await expect(
+    session.navigate("https://navigation-fails.example", { timeoutMs: 20 }),
+  ).rejects.toThrow("Navigation failed: net::ERR_FAILED")
+  expect((session as any).eventListeners).toHaveLength(0)
+})
+
+test("waitFor ignores matching events from another attached session", async () => {
+  session.setActiveSession("session-active")
+  try {
+    const waiting = session.waitFor<{ source: string }>("Page.loadEventFired", { timeoutMs: 1_000 })
+    emit("Page.loadEventFired", { source: "background" }, "session-background")
+    emit("Page.loadEventFired", { source: "active" }, "session-active")
+    expect(await waiting).toEqual({ source: "active" })
+  } finally {
+    session.setActiveSession(undefined)
+  }
+})
+
+test("waitFor and navigate reject invalid runtime arguments immediately", async () => {
+  expect(() =>
+    // @ts-expect-error Runtime callers can still pass invalid JavaScript.
+    session.waitFor("Test.invalid-predicate", { predicate: "not a function" }),
+  ).toThrow("waitFor options.predicate must be a function")
+  expect(() =>
+    session.waitFor("Test.invalid-timeout", { timeoutMs: Number.NaN }),
+  ).toThrow("waitFor timeoutMs must be a non-negative finite number")
+  await expect(
+    session.navigate("https://example.com", { timeoutMs: Number.POSITIVE_INFINITY }),
+  ).rejects.toThrow("navigate timeoutMs must be a non-negative finite number")
+})

packages/bcode-browser/test/cdp-smoke.test.ts

@@ -31,9 +31,7 @@ test.skipIf(!enabled)("Session connects, navigates, reads title", async () => {
       await session.use(page.targetId)
     }
 
-    await session.domains.Page.enable()
-    await session.domains.Page.navigate({ url: "data:text/html,<title>bcode-smoke</title>" })
-    await session.waitFor("Page.loadEventFired", undefined, 5000)
+    await session.navigate("data:text/html,<title>bcode-smoke</title>", { timeoutMs: 5000 })
 
     const r = (await session.domains.Runtime.evaluate({
       expression: "document.title",