fix(browser): make waitFor options-only and race-safe

[Alezander9]

Summary

Replaces #111. Fixes #110.

waitFor's second parameter is now an options object — waitFor(method, { predicate?, timeoutMs? }) — and the positional (method, predicate?, timeoutMs?) form is removed. The skill's navigation examples register the load waiter before Page.navigate so a fast load event can't be missed.

Why this shape instead of #111

Both real bugs from #110 are fixed here; the difference is how much API we own afterwards.

The evidence that settled the signature: in observed agent traces, a model that wanted a navigation timeout wrote waitFor("Page.loadEventFired", { timeoutMs: 15000 }).catch(() => {}) — under the positional signature the object became the predicate, the predicate throw was swallowed, the waiter ran to the default 30s, and the defensive .catch masked the timeout. A silent flat 30s tax on every navigation. The options object is the shape models assume from priors (Puppeteer/Playwright), so we make the one invented signature in session.ts match the prior instead of asking models to learn ours.

Given that, the overload machinery in #111 (positional + options forms, argument-shape validation ladder) isn't needed — there is exactly one call shape, and the only guard is a synchronous TypeError when a function is passed where options belong, so stragglers fail loud at the call site instead of stalling silently. Sync-throw also can't be swallowed by a .catch on the returned promise.

Also intentionally not carried over from #111:

• Snippet-side void loaded.catch(() => {}) incantation — waitFor now pre-observes its own promise, so an abandoned waiter times out without an unhandled rejection. Fixed once in the session layer instead of taught in the skill forever.
• loaderId / same-document branching in the skill examples — the canonical example stays the 3-line happy path; Page.navigate's errorText behavior is noted in one line (that's real CDP knowledge worth teaching, the branching isn't).
• Active-session event filtering in waitFor — real question, but it's a semantics change beyond fix(browser): make waitFor options explicit and race-safe #110's scope; should be its own decision if traces show it biting.
• navigate() wrapper — session.ts stays a transport shim; navigation composition belongs to the snippet, guided by the skill.

Predicate-throw → reject (with unsubscribe) is kept from #111, as is the mock-WS test structure (credited in the test file).

Validation

• bun test test/cdp-session.test.ts — 4 pass
• bun test (package) — 16 pass, 8 env-gated skips, 1 pre-existing unrelated failure (edit + cache-bust workspace import; fails identically on main)
• bun typecheck from packages/bcode-browser — clean

---

Summary by cubic

Make waitFor options-only and race-safe. Prevents silent 30s stalls and ensures navigation waiters catch fast load events.

• Bug Fixes

  • waitFor(method, { predicate?, timeoutMs? }) is the only supported form; passing a function as the second arg or a positional third-arg timeout now throws synchronously.
  • Abandoned waiters no longer create unhandled rejections; predicate errors reject immediately and unsubscribe.
  • Examples/tests now register the load waiter before Page.navigate to avoid missing a fast Page.loadEventFired.

• Migration

  • Replace waitFor(method, predicate?, timeoutMs?) with waitFor(method, { predicate, timeoutMs }).
  • Register const loaded = session.waitFor("Page.loadEventFired", { timeoutMs }) before Page.navigate({ url }), then await loaded.
  • Remove waitFor(...).catch(() => {}) that was only used to silence unhandled rejections.

^{Written for commit c15d706. Summary will update on new commits.}

[cubic-dev-ai]

All reported issues were addressed across 5 files

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[Alezander9]

Pushed two commits:

c15d706 — closes the last silent 30s path. The existing guard only caught waitFor(m, fn, ...) (function in the options slot). It did not catch the other positional shape, waitFor(m, undefined, 10_000), which quietly dropped the third argument and reverted to the 30s default — verified against this branch: that call was still pending after 600ms while { timeoutMs: 60 } rejected at 61ms. Since the entire point of the PR is removing a silent 30s stall, leaving one open defeated it. Now an extra argument is a ...rest: never[] — a compile error for TS callers and a synchronous TypeError for runtime-written snippets. Test added covering the positional-timeout throw plus the supported form actually honouring its timeout.

a427292 — merged origin/main. This branch was 925 commits behind, cut before the v1.18.4 sync (#116) and rebrand (#117). Clean merge, no conflicts. Worth knowing: on the old base the filtered root typecheck failed on @opencode-ai/effect-drizzle-sqlite (examples/basic.ts(28,26) Effect Defect/Top drift) — pre-existing on that base, fixed in main, so the pre-push gate blocked until the merge. Now 16/16 tasks pass.

Package suite: 17 pass, 8 env-gated skips, 1 pre-existing unrelated edit + cache-bust failure that reproduces on bare main.