[kixelated]

Finishes quest/m1/mock-clock-tests/manual-drive-host-io.md, a child of the mocked-clock epic.

Summary

Manual.drive stops at the deadline the driven work reaches, even when the work first waits on a mocked transport delivery. js/net's driveUntil is deleted. Every caller now passes drive a promise.

No clock change was needed. The quest assumed a mock delivery resolves on a later host task that the clock doesn't count. I reproduced each driveUntil caller on plain drive and found this isn't so. MockTransport is built on WHATWG streams, and their deliveries are promise reactions. The drain() that drive runs before each deadline already includes them. A probe confirmed it: a mock bidi round trip and a pipeThrough both settle within one drain. Two other things caused the early deadlines that driveUntil worked around:

1. A cold dynamic import(). The IETF handshake loads ietf/connection.ts on first use. Bun resolves a cold import outside the task queue (a probe spinning MessageChannel tasks never saw it settle). A warm import settles in one drain. The quest puts this out of scope, so reload.test.ts now imports the module up front.
2. Bun's expect(promise).rejects. For a pending promise, it spins the event loop nested inside the call, ahead of microtasks the caller already queued. The connect-timeout failure test(net): js/net tests run on a mocked clock #8165 described ("the client needs a second host turn to read the server's SETUP") is this: under await expect(clock.drive(connecting)).rejects, the 30 s connect timeout fired before the client decoded SERVER_SETUP. In a minimal repro, gated work rejected at 110 instead of at 10. connect.test.ts now calls drive first and asserts on the settled promise. Manual.drive's doc says to await it directly.

Changes

• js/net/src/mock.ts: driveUntil is replaced by waitFor(getter, ready), which resolves once a signal holds a value that passes ready. Identical copies in auth.test.ts and integration.test.ts now import it.
• Callers that passed a predicate (reload.test.ts) now pass drive a promise that resolves on the event the predicate read. This follows the epic's "await the event it means" rule. The reload.error, reload.established, watched.active, and watching.synced checks wait on those signals directly. Counters and dial lists became Signals. The origins test watches its route streams through a small present(routes) set instead of polling wireOf(...).routes(path). The watcher test's sessions.length > 1 && guard is dropped: it waits for !synced and then synced, which already means a new sync.
• Callers that passed a promise (tail, ietf/tail, lite/datagram-tracks, connect) use clock.drive. Where a test awaited the same promise again after driving it, it now asserts on drive's return value.
• The three required tests, in js/net/src/mock.test.ts:
  • a queued round trip (the peer echoes once the request reaches it) lands before its 1 s timeout, and time stops at the 10 ms sleep the work reaches next, with a 60 s deadline left unfired;
  • a read with nothing in flight times out with Elapsed exactly at 1 s;
  • a peer that refills the stream's queue on every host turn doesn't starve a 1 s deadline.
• js/clock/src/manual.ts: two doc lines on drive. Stream I/O lands before each deadline; a dynamic import may not; and await it directly rather than inside bun's expect(...).rejects.

I checked that the tests catch real regressions. If drive skips its drain (await Promise.resolve() instead of await drain()), the first test fails. If each mock write is counted as a clock task (await yieldNow() in the sink), which is the approach planned in earlier drafts of this quest, the third test starves and times out. An earlier version of that test wrote one chunk per turn, and the counting mutation slipped past it. Keeping the queue full is what makes it catch the mutation.

Public API: none. Manual.drive gains doc lines only. mock.ts is test support that the package doesn't export: driveUntil is removed and waitFor added, and js/crypto imports neither. Wire: none.

Choices made without asking

• Leaving Manual unchanged instead of adding a mechanism. The quest left the mechanism to this PR. The probes found no mocked I/O for drive to wait on, and any form of counting continuous deliveries starves a deadline, which the third required test now rules out.
• waitFor is shared in mock.ts, not local to reload.test.ts. Two test files already had identical copies.
• The tail tests still assert elapsed < GRACE, not elapsed == 0. That is the property they claim ("nothing waited out the grace"). An exact zero would break on any unrelated short timer.

Validation

• bun test in js/net: 3131 pass, 0 fail, after merging origin/main (cb3d764410). js/clock: 37 pass.
• Under load: three parallel quest gate bun test --rerun-each 10 runs over the changed files other than connect.test.ts (reload, tail, ietf/tail, lite/datagram-tracks, auth, integration, mock) passed 3420 of 3420 each. Five plain connect.test.ts runs passed. That file fails under --rerun-each on main too, because of its module-level WebSocket preference cache.
• mock.test.ts --rerun-each 20: 140 pass.
• bun biome check --error-on-warnings js/net js/clock passes, and so does bun run check in js/net and js/clock.
• quest check --base origin/main passes. git grep driveUntil finds nothing.

Models

Size: M
Implementer: Claude Code, claude-opus-5-5, high effort

Follow-ups

• A guard against expect(clock.drive(...)).rejects / .resolves (plan). Bun's nested event loop silently breaks drive's ordering, and the failure looks like a clock bug. A biome noRestrictedSyntax rule or a check in the @moq/clock preload could reject the pattern. Today the only warning is the doc line.

(Written by Claude Opus 5.5 with high)

🤖 Generated with Claude Code

Review

Rewritten by quest review after each round.

• Reviewer: Codex, gpt-6.1-sol, xhigh effort
• Session: 01a1b8ad-7213-78a3-947d-72398e2d3d4a, round 1
• Head: 859d52a
• Base: origin/main at 4a86602
• Score: 9/10, The promise-driven waits match the signal and mock-stream implementations with no actionable regressions found; builds and tests were not run, as instructed.
• Findings: none

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 6 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9755593c-5673-4ec1-a1c3-8b79dac9c9f2

Commits

Reviewing files that changed from the base of the PR and between cb3d764 and 859d52a.

Files selected for processing: 12

• js/clock/src/manual.ts
• js/net/src/auth.test.ts
• js/net/src/connection/connect.test.ts
• js/net/src/connection/reload.test.ts
• js/net/src/ietf/tail.test.ts
• js/net/src/integration.test.ts
• js/net/src/lite/datagram-tracks.test.ts
• js/net/src/mock.test.ts
• js/net/src/mock.ts
• js/net/src/tail.test.ts
• quest/m1/mock-clock-tests/README.md
• quest/m1/mock-clock-tests/manual-drive-host-io.md

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[kixelated]

Automated review by review (OpenAI)

Reviewed 859d52a.

No actionable findings. Replacing millisecond polling with promises for the actual signals and route events preserves the reconnect and delivery assertions while letting the existing clock drain control deadline order. Loading the IETF module before driving and asserting rejection only after the driven promise settles keeps those host-runtime concerns out of the clock implementation. The new regressions distinguish queued delivery, an idle read timeout, and continuous traffic without adding task accounting that would prevent time from advancing.

Direction: keep the existing clock mechanism and the event-driven test helpers. The documented Bun assertion caveat and proposed guard remain useful follow-up work, not a reason to add another clock workaround.

Verification limits: GitHub-only static review of the complete diff and relevant Manual/drain, mock streams, signal notifications, reconnect and tail-test context. No commands, tests, runtime probes or mutation tests run; the reported Bun scheduling and stress results were not independently reproduced. Rechecked open/non-draft state, full head and published reviews immediately before this COMMENT.

[pullfrog]

✅ No new issues found.

Reviewed changes

• Manual.drive documentation — two doc lines in js/clock/src/manual.ts: promise-based I/O lands before each deadline, a cold import() may not, and callers must await drive directly rather than inside bun's expect(...).rejects. No mechanism added; the PR's proof that MockTransport deliveries are promise reactions covered by drain() holds up under the new tests.
• driveUntil replaced by waitFor in js/net/src/mock.ts — the helper now resolves once a Getter passes ready, with the two duplicated copies in auth.test.ts and integration.test.ts importing it instead.
• Predicate callers await the event instead of polling — reload.error, reload.established, watched.active, watching.synced, and test-local counters/lists (attempts, dialed, servers) become Signals; a new present(routes) helper tracks a Routes stream into a path set, replacing wireOf(...).routes(path) polling.
• Promise callers now clock.drive the work — tail, ietf/tail, lite/datagram-tracks, and connect assert on drive's return value instead of awaiting the same promise again; connect.test.ts drives connecting.catch(() => {}) and keeps the rejection assertion on the original promise.
• Eager ../ietf/connection.ts import in reload.test.ts — a documented workaround for Bun resolving a cold dynamic import outside the task queue.
• Three new mock.test.ts tests — a queued round trip before its deadline with time stopping at the next sleep (60 s timer left unfired), a read timing out exactly at 1 s, and a peer refilling the stream queue every host turn not starving a 1 s deadline.
• Quest bookkeeping — the completed child quest file and its README ## Required entry are removed, consistent with how other quests land.

Verification

Ran bun test on every touched file plus integration.test.ts and js/clock: all pass (mock 7, reload/tail/ietf-tail 64, connect/datagram-tracks/auth 222, integration 106 pass 1 skip, clock 37). tsc --noEmit in js/net and biome check --error-on-warnings js/net js/clock are clean, and git grep driveUntil finds nothing. A mutation check confirms the first new test guards the drain: replacing await drain() with await Promise.resolve() in Manual.#advance fails it (at jumps to 60 001 ms). The dropped sessions.length > 1 && guard in the watcher test is also safe — re-adding expect(sessions.length).toBeGreaterThan(1) after the two synced waits passed under --rerun-each, so !synced→synced still implies a fresh session's initial set.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}