From d1432a6c2a9a91c0ffd3e7897c6c74b2e5db5de6 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 13 Oct 2026 16:21:09 -0700 Subject: [PATCH 1/8] quest(m1): plan refusing timestamp rewinds and mocking time in the auth refresh tests Co-Authored-By: Claude Opus 5.5 --- quest/m1/README.md | 1 + quest/m1/refuse-rewinds.md | 87 +++++++++++++++++++ quest/m1/test-flakes-2/README.md | 2 +- quest/m1/test-flakes-2/auth-refresh-expiry.md | 27 ------ .../test-flakes-2/auth-refresh-mock-time.md | 51 +++++++++++ 5 files changed, 140 insertions(+), 28 deletions(-) create mode 100644 quest/m1/refuse-rewinds.md delete mode 100644 quest/m1/test-flakes-2/auth-refresh-expiry.md create mode 100644 quest/m1/test-flakes-2/auth-refresh-mock-time.md diff --git a/quest/m1/README.md b/quest/m1/README.md index fbbe8cef3d6..3e405301570 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -164,6 +164,7 @@ above them. - [Draft 14-16 updates](/quest/m1/ietf-legacy-updates.md) - Rust and JS apply and answer moq-transport 14-16 request updates without ending or leaking the request - [Tail arrivals](/quest/m1/tail-arrivals.md) - a track judges its pending tail from recorded arrivals, not a cache scan, and errors a reader whose tail ends short - [Resume reorder](/quest/m1/resume-reorder.md) - a group whose stream header arrives after its successor's still resumes on failover; only a dropped or long-gone group fails +- [Refuse rewinds](/quest/m1/refuse-rewinds.md) - a timed track refuses a group whose first frame is earlier than the previous group's, in Rust and js/net, on every hop - [Feed fetch index](/quest/m1/feed-fetch-index.md) - held feed fetches wake per group and are capped per peer session, on main then release - [JS pending tail](/quest/m1/js-pending-tail.md) - a JS reader holds for a track's pending tail like Rust - [Group demand after accept](/quest/m1/group-demand-accept.md) - an accepted group request's demand ends cleanly instead of failing with NotFound diff --git a/quest/m1/refuse-rewinds.md b/quest/m1/refuse-rewinds.md new file mode 100644 index 00000000000..039af113e1f --- /dev/null +++ b/quest/m1/refuse-rewinds.md @@ -0,0 +1,87 @@ +# [S] A track refuses a timestamp that rewinds + +## Goal + +A timed track never holds a group whose first frame is earlier than the +previous group's, in Rust and js/net. Writing one fails loudly instead of +being cached and served. Untimed tracks are exempt. + +## Plan + +Decided by the maintainer on 2026-10-13 +([#5446](https://github.com/moq-dev/moq/pull/5446)): never support timestamp +rewinds. Today the model accepts them, and its expiry logic works around +them. + +Where a group's start is stamped: + +- Rust: `GroupState::stamp` (`rs/moq-net/src/model/group.rs`), called from + `group::Producer::write_frame` and `write_frames`, sets a group's + start from its first frame. The track keeps groups by sequence in its + `lookup` map (`rs/moq-net/src/model/track.rs`). +- JS: `state.timestamp ??= frame.timestamp` in `js/net/src/group.ts` stamps + the start; the track keeps groups by sequence in its sorted `timeline` + (`js/net/src/track.ts`). + +The rule: when a group's first frame lands, its start must be at or after +the nearest cached, stamped, non-aborted group below it, and at or before +the nearest one above it. Both sides are checked because groups can be +created and received out of sequence order. An equal start is allowed, as +moq-mux's `container::Producer` already allows a keyframe at the previous +group's start. Groups no longer cached are not checked. The write fails +without writing; a session receiving such a group aborts the track with +that error and keeps the session. The check costs one neighbor lookup per +group, not per frame; confirm with the existing group benchmark. + +Recommended: every track producer refuses, local or received. The check +sits where the start is stamped, which publishers, relays, and Rust and JS +subscribers all share, so one rule covers every hop. A buggy or foreign +publisher (including a moq-transport peer) fails at the first hop instead +of being forwarded, and every hop's expiry and delivery logic (`reach`, the +drift budget, #5446's delivered floor) can rely on starts never falling. +Alternative: refuse only local writes (`append_group`, `create_group`, and +the JS equivalents) and let received groups through. It is cheaper on the +forwarding path, but relays forward a peer's rewind and every hop must still +tolerate one. + +Error: recommended, reuse `TimestampMismatch` (stream code 0x39 on the wire) +and broaden its doc, in Rust, js/net, and the code's description in +`drafts/draft-lcurley-moq-lite.md`, to "a frame's timestamp doesn't fit its +track". No new code, no new public variant. Alternative: a distinct +`TimestampRewind` error with its own stream code, a wire addition that +needs a draft entry. + +Reconcile the docs: + +- `TrackState::reach` in track.rs and `#reach` in js/net's track.ts say + "timestamps need not rise with sequence (a rewind reorders them)". Keep the + immediate-successor rule and drop the rewind rationale: the bound is the + immediate servable successor's start, unbounded while that successor is + unstamped. Tightening `reach` to the first stamped group above, which the + refusal makes safe, is out of scope; suggest a follow-up if a stall shows + it matters. +- Delete tests whose premise is a rewind, such as + `reach_follows_the_immediate_successor_not_a_later_rewind`. +- The container layers already refuse rewinds (moq-mux's + `container::Producer` returns `TimestampRewind`, and js/hang's `Consumer` + aborts one). Keep the producers' refusal, which fires before writing and + also floors every frame, not just a group's first. Delete only consumer + checks the model now makes unreachable. +- If #5446 lands first, update its note that late-group delivery after a + rewind is unspecified. + +Regression tests in both languages: a timed track refuses a group whose +first frame rewinds, whether appended or created out of order, and from a +received group; an equal start is accepted; an untimed track accepts groups +in any order. + +Public API: write paths can now fail with `TimestampMismatch` on a rewind. +Wire: none if `TimestampMismatch` is reused; the draft's description of +code 0x39 broadens. + +## Related + +- [#5446](https://github.com/moq-dev/moq/pull/5446) - where the maintainer decided rewinds are never supported +- [JS track handover](/quest/m1/js-group-handover.md) - mirrors Rust's resume contract, which relies on starts never falling +- [Same-epoch importers](/quest/m1/hop-aligned-import.md) - importers already refuse a rewind at the container layer +- [Epoch anchor](/quest/m1/epoch-anchor.md) - a rewind stays fatal to an importer, which restarts under a successor epoch diff --git a/quest/m1/test-flakes-2/README.md b/quest/m1/test-flakes-2/README.md index 74eea9ef3ac..6b6e95d71c7 100644 --- a/quest/m1/test-flakes-2/README.md +++ b/quest/m1/test-flakes-2/README.md @@ -34,7 +34,7 @@ Public API: none. Wire: none. - [Interop close code](/quest/m1/test-flakes-2/interop-close-code.md) - the browser close-code test reliably sees `unauthorized` for a refused session - [Handshake ACK](/quest/m1/test-flakes-2/handshake-ack.md) - the impaired cluster drill's connect survives a lost handshake ACK -- [Auth refresh expiry](/quest/m1/test-flakes-2/auth-refresh-expiry.md) - the auth refresh outage test measures the grant's life on one clock +- [Auth refresh mocks time](/quest/m1/test-flakes-2/auth-refresh-mock-time.md) - moq-relay's auth refresh and fixed-lease tests measure a grant's life on one mocked clock ## Related diff --git a/quest/m1/test-flakes-2/auth-refresh-expiry.md b/quest/m1/test-flakes-2/auth-refresh-expiry.md deleted file mode 100644 index 4f73db43611..00000000000 --- a/quest/m1/test-flakes-2/auth-refresh-expiry.md +++ /dev/null @@ -1,27 +0,0 @@ -# [XS] Auth refresh measures a grant's life on one clock - -## Goal - -moq-relay's `an_outage_refuses_transiently_and_never_extends_the_expiry` -(`rs/moq-relay/src/auth/refresh.rs`) passes under a loaded `just check`, -fixed at its cause. - -## Plan - -Seen 2026-10-13 on #5446's first `just check`: the session lived 28.862 s -against the test's 29 s floor, then the test passed alone and on rerun. - -The test's `grant()` stamps the expiry with `SystemTime::now() + 30s`, but -`Grant::deadline()` (`rs/moq-auth/src/grant.rs`) converts the remaining wall -time onto Tokio's paused clock only when the lease snapshots it. Wall time -keeps running under load while the paused clock does not, so a 1.14 s -scheduling delay before the snapshot leaves 28.86 s on the Tokio clock. -Moving `start` earlier does not fix that; `sessions()` uses an in-memory -duplex, so no real socket is involved. - -Measure against one clock: assert the session ends at the deadline the lease -captured, or build the grant's expiry from the moment that deadline is -taken, keeping the bounds. Reproduce first by delaying wall time between -`grant()` and admission. - -Public API: none. Wire: none. diff --git a/quest/m1/test-flakes-2/auth-refresh-mock-time.md b/quest/m1/test-flakes-2/auth-refresh-mock-time.md new file mode 100644 index 00000000000..a45c9a753c9 --- /dev/null +++ b/quest/m1/test-flakes-2/auth-refresh-mock-time.md @@ -0,0 +1,51 @@ +# [XS] moq-relay auth refresh tests mock time + +## Goal + +Three moq-relay auth tests that measure a grant's lifetime pass under any +load, because they read one mocked clock instead of the host's wall clock: + +- `an_outage_refuses_transiently_and_never_extends_the_expiry` and + `renewal_outlives_the_initial_credential_and_withdrawal_ends_only_its_grant` + (`rs/moq-relay/src/auth/refresh.rs`) +- `a_fixed_lease_still_expires` (`rs/moq-relay/tests/auth_lifetime.rs`) + +## Plan + +All three failed under load on 2026-10-13 (seen on #5148, #5462, and +#5446) and passed alone. One run reported the session outliving its grant +at 28.5 s against the outage test's 29 s floor. AGENTS.md requires unit +tests to mock time. + +Cause: a grant's `expires` is `SystemTime::now() + N`, but +`Grant::deadline()` (`rs/moq-auth/src/grant.rs`) converts the remaining wall +time onto Tokio's clock only when the lease snapshots it. The refresh tests +already run on paused Tokio time, which stops while wall time keeps running, +so a scheduling delay before the snapshot shortens the grant on the Tokio +clock. `a_fixed_lease_still_expires` runs on real sockets and real time with +a 1 s grant, so a slow connect and round trip races the expiry. + +Decided (maintainer, 2026-10-13): mock the clock so the tests are +deterministic, rather than widen a bound. + +- If moq-clock ([#5465](https://github.com/moq-dev/moq/pull/5465)) has + landed, read the grant's wall time through it (`Grant::deadline()` and + `Grant::valid()`), and the tests stamp `expires` from its simulated clock, + which moves wall and monotonic time together. Nothing reads the host clock. +- Otherwise stay on paused Tokio time and take the wall reading on the + paused clock's terms: bracket the snapshot the way + `an_outage_keeps_the_session_until_expires` already does, so the bounds hold + however long admission takes. +- `a_fixed_lease_still_expires` moves off real sockets onto + `memory_sessions()` and `moq_relay::supervise` under the same clock, like + `an_outage_keeps_the_session_until_expires`. If no other real-socket test + proves the tcp and ws paths hand their lease to `supervise`, keep one + real-socket case that asserts only the close, under a watchdog. + +Reproduce first by delaying wall time between `grant()` and admission. + +Public API: none. Wire: none. + +## Related + +- [Controlled time](/quest/m1/time/README.md) - the systemic fix: every crate on one controlled clock, and relay tests on the sim through [the relay seam](/quest/m1/time/relay-seam.md) From 178221847cd6be88e06796201962f1a7372c9329 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 13 Oct 2026 16:28:07 -0700 Subject: [PATCH 2/8] quest(m1): address review on the rewind and auth clock quests Co-Authored-By: Claude Opus 5.5 --- quest/m1/refuse-rewinds.md | 31 +++++++++++++--- .../test-flakes-2/auth-refresh-mock-time.md | 37 ++++++++++++------- 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/quest/m1/refuse-rewinds.md b/quest/m1/refuse-rewinds.md index 039af113e1f..c5a5a0d904c 100644 --- a/quest/m1/refuse-rewinds.md +++ b/quest/m1/refuse-rewinds.md @@ -30,8 +30,28 @@ created and received out of sequence order. An equal start is allowed, as moq-mux's `container::Producer` already allows a keyframe at the previous group's start. Groups no longer cached are not checked. The write fails without writing; a session receiving such a group aborts the track with -that error and keeps the session. The check costs one neighbor lookup per -group, not per frame; confirm with the existing group benchmark. +that error and keeps the session. + +Cases the rule must cover (from review): + +- Only an original start counts. A group received from a later frame + (`start_at(hdr.frame_start)` in `rs/moq-net/src/lite/subscriber.rs`, or a + partial fetch) stamps its first *received* frame, which can sit above a + valid successor's start, since moq-mux lets a keyframe overlap the previous + group's last frame. A group whose first held frame is not index 0 has no + known start: it is neither checked nor used as a neighbor's bound. +- The neighbor check and the first stamp are one atomic step under the + shared track state, taken in the existing track-before-group lock order. + Groups are written on independent streams, so two unstamped neighbors + checked under only their own locks could each accept a rewind. +- A JS `GroupProducer` can hold frames before `track.writeGroup` attaches + it, so its start is stamped with no timeline to check. Check an already + started group when it is attached, before it is cached or fanned out. + +The check costs one ordered neighbor lookup per group, not per frame. The +existing `group_write_frames` benchmark writes into an otherwise empty +track, so add a row that stamps groups swept over the retained group count, +where a scan of `lookup` would show as a slope. Recommended: every track producer refuses, local or received. The check sits where the start is stamped, which publishers, relays, and Rust and JS @@ -71,9 +91,10 @@ Reconcile the docs: rewind is unspecified. Regression tests in both languages: a timed track refuses a group whose -first frame rewinds, whether appended or created out of order, and from a -received group; an equal start is accepted; an untimed track accepts groups -in any order. +first frame rewinds, whether appended, created out of order, received, or +(JS) written before `writeGroup`; an equal start is accepted; a resumed +partial group of a valid overlapping pair is accepted; an untimed track +accepts groups in any order. Public API: write paths can now fail with `TimestampMismatch` on a rewind. Wire: none if `TimestampMismatch` is reused; the draft's description of diff --git a/quest/m1/test-flakes-2/auth-refresh-mock-time.md b/quest/m1/test-flakes-2/auth-refresh-mock-time.md index a45c9a753c9..e4774890599 100644 --- a/quest/m1/test-flakes-2/auth-refresh-mock-time.md +++ b/quest/m1/test-flakes-2/auth-refresh-mock-time.md @@ -2,13 +2,15 @@ ## Goal -Three moq-relay auth tests that measure a grant's lifetime pass under any +The moq-relay auth tests that measure a grant's lifetime pass under any load, because they read one mocked clock instead of the host's wall clock: - `an_outage_refuses_transiently_and_never_extends_the_expiry` and `renewal_outlives_the_initial_credential_and_withdrawal_ends_only_its_grant` (`rs/moq-relay/src/auth/refresh.rs`) -- `a_fixed_lease_still_expires` (`rs/moq-relay/tests/auth_lifetime.rs`) +- `a_fixed_lease_still_expires` and + `an_outage_keeps_the_session_until_expires` + (`rs/moq-relay/tests/auth_lifetime.rs`) ## Plan @@ -28,24 +30,31 @@ a 1 s grant, so a slow connect and round trip races the expiry. Decided (maintainer, 2026-10-13): mock the clock so the tests are deterministic, rather than widen a bound. -- If moq-clock ([#5465](https://github.com/moq-dev/moq/pull/5465)) has - landed, read the grant's wall time through it (`Grant::deadline()` and - `Grant::valid()`), and the tests stamp `expires` from its simulated clock, - which moves wall and monotonic time together. Nothing reads the host clock. -- Otherwise stay on paused Tokio time and take the wall reading on the - paused clock's terms: bracket the snapshot the way - `an_outage_keeps_the_session_until_expires` already does, so the bounds hold - however long admission takes. +- Read the grant's wall time through moq-clock + ([#5465](https://github.com/moq-dev/moq/pull/5465)) in `Grant::deadline()` + and `Grant::valid()`, and the tests stamp `expires` from its simulated + clock, which moves wall and monotonic time together. Nothing reads the host + clock. +- Paused Tokio time alone is not enough (review on #5474): it does not pause + `SystemTime`, so under load a short grant can still expire before admission, + and bracketing the bounds the way `an_outage_keeps_the_session_until_expires` + does cannot stop that. That test also unwraps a wall expiry that may + already be past, so it moves onto the mocked clock here too. Hence the + crate is Required, rather than a paused-Tokio fallback. - `a_fixed_lease_still_expires` moves off real sockets onto - `memory_sessions()` and `moq_relay::supervise` under the same clock, like - `an_outage_keeps_the_session_until_expires`. If no other real-socket test - proves the tcp and ws paths hand their lease to `supervise`, keep one - real-socket case that asserts only the close, under a watchdog. + `memory_sessions()` and `moq_relay::supervise` under the same clock. If no + other real-socket test proves the tcp and ws paths hand their lease to + `supervise`, keep one real-socket case that asserts only the close, under a + watchdog. Reproduce first by delaying wall time between `grant()` and admission. Public API: none. Wire: none. +## Required + +- [The controlled-clock crate](/quest/m1/time/crate.md) - the simulated clock that moves wall and monotonic time together + ## Related - [Controlled time](/quest/m1/time/README.md) - the systemic fix: every crate on one controlled clock, and relay tests on the sim through [the relay seam](/quest/m1/time/relay-seam.md) From 5813647799bdef09448663fc748474af0b9eb586 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 13 Oct 2026 16:28:25 -0700 Subject: [PATCH 3/8] quest(m1): check a rewind before the frame commits Co-Authored-By: Claude Opus 5.5 --- quest/m1/refuse-rewinds.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/quest/m1/refuse-rewinds.md b/quest/m1/refuse-rewinds.md index c5a5a0d904c..7dabc5ba9af 100644 --- a/quest/m1/refuse-rewinds.md +++ b/quest/m1/refuse-rewinds.md @@ -29,7 +29,9 @@ the nearest one above it. Both sides are checked because groups can be created and received out of sequence order. An equal start is allowed, as moq-mux's `container::Producer` already allows a keyframe at the previous group's start. Groups no longer cached are not checked. The write fails -without writing; a session receiving such a group aborts the track with +without writing: Rust's `write_frame` and `write_frames` commit the frame +before `stamp` today, so the check runs before any cache, frame, or index +change, and the start is stamped only once it passes. A session receiving such a group aborts the track with that error and keeps the session. Cases the rule must cover (from review): From 3bf63b634377ecdf66229b412a2a31109585e7d0 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 13 Oct 2026 16:33:37 -0700 Subject: [PATCH 4/8] quest(m1): cover streamed frames and eviction in the rewind check Co-Authored-By: Claude Opus 5.5 --- quest/m1/refuse-rewinds.md | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/quest/m1/refuse-rewinds.md b/quest/m1/refuse-rewinds.md index 7dabc5ba9af..0d9451478a3 100644 --- a/quest/m1/refuse-rewinds.md +++ b/quest/m1/refuse-rewinds.md @@ -15,10 +15,12 @@ them. Where a group's start is stamped: -- Rust: `GroupState::stamp` (`rs/moq-net/src/model/group.rs`), called from - `group::Producer::write_frame` and `write_frames`, sets a group's - start from its first frame. The track keeps groups by sequence in its - `lookup` map (`rs/moq-net/src/model/track.rs`). +- Rust: `GroupState::stamp` (`rs/moq-net/src/model/group.rs`) sets a + group's start from its first frame. Every write path calls it: + `group::Producer::write_frame`, `write_frames`, and `open_frame` behind + `create_frame` and `create_frame_owned`, which lite and IETF ingress use. + The track keeps groups by sequence in its `lookup` map + (`rs/moq-net/src/model/track.rs`). - JS: `state.timestamp ??= frame.timestamp` in `js/net/src/group.ts` stamps the start; the track keeps groups by sequence in its sorted `timeline` (`js/net/src/track.ts`). @@ -28,11 +30,11 @@ the nearest cached, stamped, non-aborted group below it, and at or before the nearest one above it. Both sides are checked because groups can be created and received out of sequence order. An equal start is allowed, as moq-mux's `container::Producer` already allows a keyframe at the previous -group's start. Groups no longer cached are not checked. The write fails -without writing: Rust's `write_frame` and `write_frames` commit the frame -before `stamp` today, so the check runs before any cache, frame, or index -change, and the start is stamped only once it passes. A session receiving such a group aborts the track with -that error and keeps the session. +group's start. The write fails without writing: every Rust write path +commits the frame (or installs the partial one) before `stamp` today, so +the check runs before any cache, frame, or index change, and the start is +stamped only once it passes. A session receiving such a group aborts the +track with that error and keeps the session. Cases the rule must cover (from review): @@ -49,6 +51,12 @@ Cases the rule must cover (from review): - A JS `GroupProducer` can hold frames before `track.writeGroup` attaches it, so its start is stamped with no timeline to check. Check an already started group when it is attached, before it is cached or fanned out. +- Eviction must not drop the bound. An unstamped group whose stamped + predecessor expires (Rust's `evict_expired_scan`, JS's `#prune`) would + otherwise have no lower neighbor and accept a rewind. The track keeps the + sequence and original start of the highest-sequence group it evicted, and + a group above it with no cached lower neighbor is checked against that. + One entry, so no history grows with the cache. The check costs one ordered neighbor lookup per group, not per frame. The existing `group_write_frames` benchmark writes into an otherwise empty @@ -93,8 +101,9 @@ Reconcile the docs: rewind is unspecified. Regression tests in both languages: a timed track refuses a group whose -first frame rewinds, whether appended, created out of order, received, or -(JS) written before `writeGroup`; an equal start is accepted; a resumed +first frame rewinds, whether appended, created out of order, received +through the streamed-frame path, written after its predecessor was evicted, +or (JS) written before `writeGroup`; an equal start is accepted; a resumed partial group of a valid overlapping pair is accepted; an untimed track accepts groups in any order. From f0d721ef9804525d2a311db3421794760c03a557 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 13 Oct 2026 16:37:35 -0700 Subject: [PATCH 5/8] quest(m1): late groups below the eviction watermark bound nothing Co-Authored-By: Claude Opus 5.5 --- quest/m1/refuse-rewinds.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/quest/m1/refuse-rewinds.md b/quest/m1/refuse-rewinds.md index 0d9451478a3..a15f5688b95 100644 --- a/quest/m1/refuse-rewinds.md +++ b/quest/m1/refuse-rewinds.md @@ -57,6 +57,13 @@ Cases the rule must cover (from review): sequence and original start of the highest-sequence group it evicted, and a group above it with no cached lower neighbor is checked against that. One entry, so no history grows with the cache. +- A late or refetched group at or below that watermark (JS `writeGroup` + accepts long-evicted sequences; Rust takes older groups after `finish`) + has lost its neighbors' starts. It is checked only against the watermark, + as an upper bound, and never serves as another group's bound, so a + refetched legitimate group is never refused because of it. The guarantee + is stated as such: a hop refuses a rewind against the starts it still + holds, not against history it has evicted. The check costs one ordered neighbor lookup per group, not per frame. The existing `group_write_frames` benchmark writes into an otherwise empty @@ -104,7 +111,8 @@ Regression tests in both languages: a timed track refuses a group whose first frame rewinds, whether appended, created out of order, received through the streamed-frame path, written after its predecessor was evicted, or (JS) written before `writeGroup`; an equal start is accepted; a resumed -partial group of a valid overlapping pair is accepted; an untimed track +partial group of a valid overlapping pair is accepted; a late group below +the eviction watermark does not cause a later refetch to be refused; an untimed track accepts groups in any order. Public API: write paths can now fail with `TimestampMismatch` on a rewind. From 03697d635dedba5bc68ca0486dcfaee964be3658 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 13 Oct 2026 16:41:30 -0700 Subject: [PATCH 6/8] quest(m1): late groups still check surviving cached neighbors Co-Authored-By: Claude Opus 5.5 --- quest/m1/refuse-rewinds.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/quest/m1/refuse-rewinds.md b/quest/m1/refuse-rewinds.md index a15f5688b95..ae6b8fea22f 100644 --- a/quest/m1/refuse-rewinds.md +++ b/quest/m1/refuse-rewinds.md @@ -59,8 +59,10 @@ Cases the rule must cover (from review): One entry, so no history grows with the cache. - A late or refetched group at or below that watermark (JS `writeGroup` accepts long-evicted sequences; Rust takes older groups after `finish`) - has lost its neighbors' starts. It is checked only against the watermark, - as an upper bound, and never serves as another group's bound, so a + may have lost its neighbors' starts. Eviction is not a sequence prefix + (each recently written group is retained on its own), so it is still + checked against its surviving cached neighbors, plus the watermark as an + upper bound. It never serves as another group's bound, so a refetched legitimate group is never refused because of it. The guarantee is stated as such: a hop refuses a rewind against the starts it still holds, not against history it has evicted. @@ -112,7 +114,8 @@ first frame rewinds, whether appended, created out of order, received through the streamed-frame path, written after its predecessor was evicted, or (JS) written before `writeGroup`; an equal start is accepted; a resumed partial group of a valid overlapping pair is accepted; a late group below -the eviction watermark does not cause a later refetch to be refused; an untimed track +the eviction watermark does not cause a later refetch to be refused, but is +refused against a surviving cached neighbor after a non-prefix eviction; an untimed track accepts groups in any order. Public API: write paths can now fail with `TimestampMismatch` on a rewind. From 3fe4d2c9996c95b49065fd528b69e9738c4240e9 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 13 Oct 2026 16:46:14 -0700 Subject: [PATCH 7/8] quest(m1): combine eviction bounds, leave unprovable late groups open, name Grant::validate Co-Authored-By: Claude Opus 5.5 --- quest/m1/refuse-rewinds.md | 37 +++++++++++-------- .../test-flakes-2/auth-refresh-mock-time.md | 8 ++-- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/quest/m1/refuse-rewinds.md b/quest/m1/refuse-rewinds.md index ae6b8fea22f..85c53807899 100644 --- a/quest/m1/refuse-rewinds.md +++ b/quest/m1/refuse-rewinds.md @@ -54,18 +54,21 @@ Cases the rule must cover (from review): - Eviction must not drop the bound. An unstamped group whose stamped predecessor expires (Rust's `evict_expired_scan`, JS's `#prune`) would otherwise have no lower neighbor and accept a rewind. The track keeps the - sequence and original start of the highest-sequence group it evicted, and - a group above it with no cached lower neighbor is checked against that. - One entry, so no history grows with the cache. + sequence and original start of the highest-sequence group it evicted. A + group above it is checked against both its cached neighbors and that + watermark, never one in place of the other, since eviction is not a + sequence prefix (each recently written group is retained on its own). One + entry, so no history grows with the cache. - A late or refetched group at or below that watermark (JS `writeGroup` accepts long-evicted sequences; Rust takes older groups after `finish`) - may have lost its neighbors' starts. Eviction is not a sequence prefix - (each recently written group is retained on its own), so it is still - checked against its surviving cached neighbors, plus the watermark as an - upper bound. It never serves as another group's bound, so a - refetched legitimate group is never refused because of it. The guarantee - is stated as such: a hop refuses a rewind against the starts it still - holds, not against history it has evicted. + is checked against its surviving cached neighbors plus the watermark as an + upper bound, but its lower bound may be gone. Open, for the implementer to + confirm with the maintainer. Recommended: accept it unverified, keep it out + of every other group's bounds and out of timestamp-driven expiry, so + backfill below the watermark keeps working (fetched backfill is already + served by sequence only). Alternative: refuse a late group whose lower + bound was evicted, which keeps the Goal absolute but refuses legitimate + backfill of evicted history. The check costs one ordered neighbor lookup per group, not per frame. The existing `group_write_frames` benchmark writes into an otherwise empty @@ -111,12 +114,14 @@ Reconcile the docs: Regression tests in both languages: a timed track refuses a group whose first frame rewinds, whether appended, created out of order, received -through the streamed-frame path, written after its predecessor was evicted, -or (JS) written before `writeGroup`; an equal start is accepted; a resumed -partial group of a valid overlapping pair is accepted; a late group below -the eviction watermark does not cause a later refetch to be refused, but is -refused against a surviving cached neighbor after a non-prefix eviction; an untimed track -accepts groups in any order. +through the streamed-frame path, written after its predecessor was evicted +(including a delayed stamp above the watermark while an older group +survives a non-prefix eviction), or (JS) written before `writeGroup`; an +equal start is accepted; a resumed partial group of a valid overlapping +pair is accepted; a late group below the watermark is refused against a +surviving cached neighbor, follows the chosen rule otherwise, and never +causes a later refetch to be refused; an untimed track accepts groups in +any order. Public API: write paths can now fail with `TimestampMismatch` on a rewind. Wire: none if `TimestampMismatch` is reused; the draft's description of diff --git a/quest/m1/test-flakes-2/auth-refresh-mock-time.md b/quest/m1/test-flakes-2/auth-refresh-mock-time.md index e4774890599..604ab03181c 100644 --- a/quest/m1/test-flakes-2/auth-refresh-mock-time.md +++ b/quest/m1/test-flakes-2/auth-refresh-mock-time.md @@ -30,9 +30,11 @@ a 1 s grant, so a slow connect and round trip races the expiry. Decided (maintainer, 2026-10-13): mock the clock so the tests are deterministic, rather than widen a bound. -- Read the grant's wall time through moq-clock - ([#5465](https://github.com/moq-dev/moq/pull/5465)) in `Grant::deadline()` - and `Grant::valid()`, and the tests stamp `expires` from its simulated +- Read the grant's wall time through the controlled-clock crate (moq-clock + in [#5465](https://github.com/moq-dev/moq/pull/5465), planned as + moq-time) in both `Grant::deadline()` and `Grant::validate()` + (`rs/moq-auth/src/grant.rs`), which each compare `expires` with + `SystemTime::now()` today, and the tests stamp `expires` from its simulated clock, which moves wall and monotonic time together. Nothing reads the host clock. - Paused Tokio time alone is not enough (review on #5474): it does not pause From 1e91c2c10c1bf1b25f3e8457a81683f7f6eaefcc Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 13 Oct 2026 16:51:09 -0700 Subject: [PATCH 8/8] quest(m1): keep container rewind checks and route ingress refusals to the track Co-Authored-By: Claude Opus 5.5 --- quest/m1/refuse-rewinds.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/quest/m1/refuse-rewinds.md b/quest/m1/refuse-rewinds.md index 85c53807899..aa9110f902e 100644 --- a/quest/m1/refuse-rewinds.md +++ b/quest/m1/refuse-rewinds.md @@ -34,7 +34,12 @@ group's start. The write fails without writing: every Rust write path commits the frame (or installs the partial one) before `stamp` today, so the check runs before any cache, frame, or index change, and the start is stamped only once it passes. A session receiving such a group aborts the -track with that error and keeps the session. +track with that error and keeps the session. The ingress handlers must +route it there: today Rust's IETF subscriber (`rs/moq-net/src/ietf/subscriber.rs`) +sends `TimestampMismatch` down the generic branch that aborts only the +group, and js/net's (`js/net/src/ietf/subscriber.ts`) closes the track only +for `ProtocolViolation` or `MalformedTrack`. Check the lite subscribers the +same way, and test that the track, not just the group, closes. Cases the rule must cover (from review): @@ -105,10 +110,10 @@ Reconcile the docs: - Delete tests whose premise is a rewind, such as `reach_follows_the_immediate_successor_not_a_later_rewind`. - The container layers already refuse rewinds (moq-mux's - `container::Producer` returns `TimestampRewind`, and js/hang's `Consumer` - aborts one). Keep the producers' refusal, which fires before writing and - also floors every frame, not just a group's first. Delete only consumer - checks the model now makes unreachable. + `container::Producer` returns `TimestampRewind`; the Rust and js/hang + container consumers abort one). Keep all of them: they floor every frame + against the previous group's start, while the model checks only a group's + first frame, and a foreign wire publisher bypasses the producer's floor. - If #5446 lands first, update its note that late-group delivery after a rewind is unspecified.