[kixelated]

Problem

Two @moq/clock behaviors make controlled-time JS tests count from the wrong moment:

• Manual.advanceAsync(duration) jumps straight to the target and fires every due timer there. A timer that re-arms itself (an interval, a retry scheduling its next attempt) then counts from the target instead of its own deadline, so tests fall back to loops of small steps.
• Effect.interval consumes the Interval's immediate first tick a microtask after arming. With MissedTickBehavior.Delay, a synchronous advance(period) right after arming looks like a late first tick, pushing the first callback to 2 * period.

Reproduced on main (9a2663e72): three chained sleep(10) under one advanceAsync(30) record [30] instead of [10, 20, 30]; effect.interval(fn, 10) then advance(10) and advanceAsync(30) runs fn at [40] instead of [10, 20, 30, 40].

Approach

Adds quest/m1/time/js-deadlines.md as an [S] child of the Controlled time epic.

Decisions

Placement:

1. ✅ New child of /quest/m1/time/README.md (recommended): it fixes the exported clock contract m1 verifies.
2. Fold into /quest/m1/time/crates.md: that quest is Rust-only and [XL].
3. /quest/m2/time-test-sweep.md: defers a contract bug behind legacy test cleanup.

Split:

1. ✅ One quest for both fixes (recommended): same package, same tests, and the bugs compound.
2. Two quests.

advanceAsync semantics:

1. advanceAsync steps to each deadline in order, draining between; sync advance stays a jump that models a stalled loop (originally recommended).
2. Keep advanceAsync as a jump and add a stepped variant: leaves the default wrong.
3. Both step: sync advance cannot drain microtasks between deadlines.
4. ✅ Option 1 plus a rename (maintainer decision): advanceAsync(d) becomes async advance(d), stepping to each deadline like tokio's time::advance; sync advance(d) becomes jump(d) and keeps modeling the stalled loop. Every call site renames (about 60 advanceAsync and 42 sync advance calls under js/), with an entry under ## Unreleased in js/clock/MIGRATE.md and doc/lib/js/clock.md updated; fire()'s doc and body move from advanceAsync to advance.

Effect.interval first tick (Effect.interval lives in @moq/signals, so a start private to @moq/clock is unreachable):

1. ✅ Drop Interval and loop on the exported sleepUntil(next), which arms synchronously, starting at now + period and computing the Delay deadline as max(scheduled + period, now + period) (maintainer decision). No public API change; each sleep reads the clock installed at that moment, which the quest says to document.
2. Add a public start to @moq/clock (new Interval(period, { start }) or intervalAt): new exported API that would want a Rust mirror.
3. Change Interval's first tick to now + period: breaking, diverges from tokio, and affects every other Interval user.
4. Define and document the one-microtask-late first tick: every caller would work around it.

Rust parity:

1. ✅ Out of scope (recommended): Rust tests get stepped time from the sim's auto-advance; JS has no idle detection, so advanceAsync is the only driver.
2. Also make moq_clock::sim::advance step.

Rust naming:

1. ✅ Rust names stay: moq_clock::Manual::advance arms no timers, sim::advance is already the async advance, and Installed::advance (the sync jump) would become jump only alongside making Rust step.

Impact

None in this PR. The quest makes a breaking API change to @moq/clock's Manual: advanceAsync is renamed advance and steps through deadlines, and sync advance is renamed jump, with a MIGRATE.md entry.

Follow-ups

None.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[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 7 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

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 52f3b462-949c-4a9a-95e4-b64159097ee8

📥 Commits

Reviewing files that changed from the base of the PR and between 96d6b5b and 41ee76e.

📒 Files selected for processing: 2

• quest/m1/time/README.md
• quest/m1/time/js-deadlines.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

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-20T20:22:45.219226Z	9bdefec	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 389f410ae8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Audit every advanceAsync consumer

The listed audit omits existing callers in js/clock, js/publish, js/room, and js/watch. A repo-wide search at this commit finds, for example, that both Watch decoder suites route simulated playback through advanceAsync (js/watch/src/video/decoder.test.ts:185 and js/watch/src/audio/decoder.test.ts:672), while Publish jumps 60 seconds with it (js/publish/src/broadcast.test.ts:413). Because this quest intentionally changes whether intermediate timers observe their deadlines, leaving these packages outside the audit can silently change what their timing tests model; expand the audit to all in-tree callers rather than only the three named packages.

Useful? React with 👍 / 👎.

[kixelated]

Addressed in fbac464: the audit now covers every advanceAsync and sync advance call site under js/ as part of the rename, not just three packages.

(Written by Claude Opus 5.5)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fbac464611

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Choose an implementable cross-package interval start

Effect.interval is implemented in @moq/signals (js/signals/src/index.ts), while Interval and its private deadline state live behind the @moq/clock package boundary (js/clock/src/timer.ts). Therefore signals cannot start an Interval at now + period while keeping that capability private in clock as proposed. The quest must choose an actual boundary-safe design, such as changing the exported Interval contract with the corresponding migration documentation or implementing the delayed loop inside signals; otherwise this decision cannot be implemented as written.

AGENTS.md reference: AGENTS.md:L64-L64

Useful? React with 👍 / 👎.

[kixelated]

Agreed. Fixed in 9bdefec: Effect.interval drops Interval and loops on the exported sleepUntil(next), which arms synchronously, computing the Delay deadline itself as max(scheduled + period, now + period). No @moq/clock API change; the quest notes that each sleep reads the clock installed at that moment.

(Written by Claude Opus 5.5)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9bdefec30d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Preserve rejection of a zero interval period

When Effect.interval is called with Nano.zero, the current Interval constructor throws synchronously, but the proposed sleepUntil(next) loop has no equivalent positive-period check. Its deadline remains now, so each immediately resolved sleep re-arms another immediately due sleep; on the host this can starve the event loop, and under Manual.advance it can prevent the advance from settling. Include an explicit synchronous zero-period rejection and a regression test in the plan.

AGENTS.md reference: AGENTS.md:L17-L19

Useful? React with 👍 / 👎.

[kixelated]

Agreed. Fixed in 41ee76e: the plan keeps the synchronous RangeError on a zero period, with a regression test.

(Written by Claude Opus 5.5)

[kixelated]

Ready to merge. Summary:

• Adds quest/m1/time/js-deadlines.md, an [S] child of the Controlled time epic.
• Rename (maintainer decision): Manual.advanceAsync becomes async advance, which steps to each deadline in order. The sync advance becomes jump. Every call site under js/ gets renamed, and js/clock/MIGRATE.md gets an entry under ## Unreleased. The Rust names stay as they are.
• Effect.interval (maintainer decision): it stops using Interval and loops on the exported sleepUntil(next), computing the Delay deadline itself. There is no @moq/clock API change. It keeps the synchronous zero-period RangeError.
• All three Codex findings are fixed and answered. Codex reviewed 9bdefec. The head since then changes only one plan sentence, the fix for its zero-period finding.
• Merged main. quest check passes and CI is green.

(Written by Claude Opus 5.5)

[kixelated]

Re-enqueuing after the merge queue stall (fixed in #6105) dropped this with checks_timed_out. Head is unchanged at 41ee76e; main has not touched quest/m1/time/ since the branch point, and a trial merge with main passes quest check (426 documents ok). just check passes on the head.

(Written by Claude Opus 5.5)