Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions packages/bcode-browser/skills/browser-execute/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,12 @@ For unknown param shapes, call with `{}` and inspect the thrown `CdpError` — `
Common moves:

```js
// Navigate.
// Navigate. Register the load waiter BEFORE navigate so a fast load isn't missed.
await session.Page.enable()
const loaded = session.waitFor("Page.loadEventFired", { timeoutMs: 15_000 })
await session.Page.navigate({ url: "https://example.com" })
await session.waitFor("Page.loadEventFired")
await loaded
// Page.navigate resolves even on network errors — its result carries `errorText` when the load failed.

// Evaluate JS in the page.
const r = await session.Runtime.evaluate({
Expand Down Expand Up @@ -153,8 +155,9 @@ export async function scrapeTitles(session: any, urls: string[]) {
const titles: string[] = []
await session.Page.enable()
for (const url of urls) {
const loaded = session.waitFor("Page.loadEventFired", { timeoutMs: 15_000 })
await session.Page.navigate({ url })
await session.waitFor("Page.loadEventFired")
await loaded
const r = await session.Runtime.evaluate({ expression: "document.title", returnByValue: true })
titles.push(r.result.value)
}
Expand Down
31 changes: 26 additions & 5 deletions packages/bcode-browser/src/cdp/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,21 +188,42 @@ 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) => {
/**
* Wait for the next event matching `method` (and optional predicate).
* Register the waiter before the call that triggers the event:
* const loaded = session.waitFor("Page.loadEventFired", { timeoutMs: 15_000 })
* await session.Page.navigate({ url })
* await loaded
*/
waitFor<T = unknown>(method: string, opts: { predicate?: (params: T) => boolean; timeoutMs?: number } = {}): Promise<T> {
if (typeof opts === 'function') {

@cubic-dev-ai cubic-dev-ai Bot Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The removed positional form with an omitted predicate is not rejected: waitFor(method, undefined, timeoutMs) falls through the function-only guard, defaults to {}, and silently ignores timeoutMs, restoring the 30-second default. This makes JavaScript/snippet callers fail with an unexpectedly long timeout instead of getting the documented migration error; checking for extra arguments (for example arguments.length > 2) would make the old form consistently fail.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/bcode-browser/src/cdp/session.ts, line 199:

<comment>The removed positional form with an omitted predicate is not rejected: `waitFor(method, undefined, timeoutMs)` falls through the function-only guard, defaults to `{}`, and silently ignores `timeoutMs`, restoring the 30-second default. This makes JavaScript/snippet callers fail with an unexpectedly long timeout instead of getting the documented migration error; checking for extra arguments (for example `arguments.length > 2`) would make the old form consistently fail.</comment>

<file context>
@@ -188,21 +188,42 @@ export class Session implements Transport {
+   *   await loaded
+   */
+  waitFor<T = unknown>(method: string, opts: { predicate?: (params: T) => boolean; timeoutMs?: number } = {}): Promise<T> {
+    if (typeof opts === 'function') {
+      throw new TypeError('waitFor(method, { predicate, timeoutMs }) — pass the predicate in the options object');
+    }
</file context>
Suggested change
if (typeof opts === 'function') {
if (arguments.length > 2 || typeof opts === 'function') {
Fix with cubic

throw new TypeError('waitFor(method, { predicate, timeoutMs }) — pass the predicate in the options object');
}
const p = new Promise<T>((resolve, reject) => {
const timer = setTimeout(() => {
unsub();
reject(new Error(`Timeout waiting for ${method}`));
}, timeoutMs);
}, opts.timeoutMs ?? 30_000);
const unsub = this.onEvent((m, params) => {
if (m !== method) return;
if (predicate && !predicate(params as T)) return;
try {
if (opts.predicate && !opts.predicate(params as T)) return;
} catch (e) {
clearTimeout(timer);
unsub();
reject(e);
return;
}
clearTimeout(timer);
unsub();
resolve(params as T);
});
});
// Pre-observe so an abandoned waiter (snippet returned or threw before
// awaiting it) times out without an unhandled rejection. Awaiting
// callers still see the rejection.
p.catch(() => {});
return p;
}

// Transport implementation. Called by the generated domain bindings.
Expand Down
6 changes: 4 additions & 2 deletions packages/bcode-browser/test/browser-execute.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,9 @@ test.skipIf(!enabled)("workspace import inside a snippet", async () => {
await session.use(page.targetId)
}
await session.Page.enable()
const loaded = session.waitFor("Page.loadEventFired", { timeoutMs: 5000 })
await session.Page.navigate({ url: "data:text/html,<title>bcode-be</title>" })
await session.waitFor("Page.loadEventFired", undefined, 5000)
await loaded
const r = await session.Runtime.evaluate({ expression: "document.title", returnByValue: true })
return r.result.value
}`,
Expand Down Expand Up @@ -125,8 +126,9 @@ test.skipIf(!enabled)("Page.captureScreenshot is collected into result.screensho
{
description: "Capture two screenshots",
code: `await session.Page.enable();
const loaded = session.waitFor("Page.loadEventFired", { timeoutMs: 5000 });
await session.Page.navigate({ url: "data:text/html,<title>shot</title><body>hi" });
await session.waitFor("Page.loadEventFired", undefined, 5000);
await loaded;
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 };`,
Expand Down
66 changes: 66 additions & 0 deletions packages/bcode-browser/test/cdp-session.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// waitFor semantics against a bare WebSocket server (no Chrome needed).
// Test structure adapted from PR #111 by @MagMueller.
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() {},
},
})
const session = new Session()
const emit = (method: string, params: unknown) => {
server.publish(channel, JSON.stringify({ method, params }))
}

beforeAll(async () => {
await session.connect({ wsUrl: `ws://127.0.0.1:${server.port}/` })
})

afterAll(() => {
session.close()
server.stop(true)
})

test("waitFor resolves on a matching event, respecting the predicate", async () => {
const waiting = session.waitFor<{ ready: boolean }>("Test.event", {
predicate: (params) => params.ready,
timeoutMs: 1_000,
})
emit("Test.event", { ready: false })
emit("Test.event", { ready: true })
expect(await waiting).toEqual({ ready: true })
})

test("waitFor honors timeoutMs", async () => {
await expect(session.waitFor("Test.timeout", { timeoutMs: 20 })).rejects.toThrow("Timeout waiting for Test.timeout")
})

test("waitFor rejects and unsubscribes when a predicate throws", async () => {
let calls = 0
const waiting = session.waitFor("Test.bad", {
predicate: () => {
calls++
throw new Error("predicate failed")
},
timeoutMs: 1_000,
})
emit("Test.bad", {})
await expect(waiting).rejects.toThrow("predicate failed")
emit("Test.bad", {})
await Bun.sleep(10)
expect(calls).toBe(1)
})

test("waitFor throws synchronously on the removed positional-predicate form", () => {
// @ts-expect-error old signature: waitFor(method, predicate, timeoutMs)
expect(() => session.waitFor("Test.positional", () => true, 1_000)).toThrow(TypeError)
})
3 changes: 2 additions & 1 deletion packages/bcode-browser/test/cdp-smoke.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ test.skipIf(!enabled)("Session connects, navigates, reads title", async () => {
}

await session.domains.Page.enable()
const loaded = session.waitFor("Page.loadEventFired", { timeoutMs: 5000 })
await session.domains.Page.navigate({ url: "data:text/html,<title>bcode-smoke</title>" })
await session.waitFor("Page.loadEventFired", undefined, 5000)
await loaded

const r = (await session.domains.Runtime.evaluate({
expression: "document.title",
Expand Down
Loading