fix(net): retain resume bounds across route copies by kixelated · Pull Request #6224 · moq-dev/moq

kixelated · 2026-10-21T23:35:38Z

Problem

A Rust logical-track reader could forget a declared end after changing routes, retain another subscription on every flap between the same routes, and retain replaced copies for gaps outside its requested window.

Approach

Remember the first declared end across copies, allow the existing JS-compatible group-boundary refinement, and refuse contradictory ends, including a group or datagram at or past the end on any copy (as JS does since moq-dev/moq#6136); a mid-group end caps the group it falls in on every copy, and one learned late is refused below a frame or datagram already seen. The newest datagram handed out is kept past the dedup budget, mirroring JS's #newestDatagram. Reuse a returning route's retained subscription and leases, resetting its replacement bound and raising its floor to the reader's newest group, as a fresh copy would start. An in-flight group's recovery treats a flap back to the copy it reads as no route change. Intersect pending-group retention with the reader's sparse window. Mirror returning-route reuse in JS.

Add mocked-clock regressions that reproduce each Rust gap and the JS lease duplication, plus reader/route and reader/flap benchmark sweeps. Delete the completed quest and its links.

Decisions

✅ Late contradictory-end detection is best-effort (maintainer). A publisher must already contradict itself across routes to hit the remaining gap, and closing it adds per-consumer state for no real behavioral benefit.
- Kept: the permanent datagram high-water mark. It is one value per reader, and JS already keeps it as #newestDatagram, so Rust matches.
- Not done: pushing a mid-group end learned after a group was handed out into that consumer. The reader keeps no handle to groups it handed out; frames arriving on such a consumer later are not capped. JS enforces ends at group granularity only.

Impact

Consumer: logical-track readers used by relay and front connections that keep the same publisher epoch across route changes.
Public API: no signature or exported-symbol changes. Rust now preserves declared ends and reports contradictory copies, including content past the declared end, as ProtocolViolation; both readers bound repeated flaps by distinct retained routes.
Wire: no framing, values, negotiation, or draft changes.

Alternatives

Keeping a fresh subscription per flap loses shared leases and grows with flap count. Remembering only the serving copy's end loses a publisher declaration. Retaining the entire convex range holds copies for groups the sparse reader never requested.

Validation

All four added Rust regressions fail on the original implementation; the JS regression reproduces an extra mirror before the fix.
Review fixes, each with a regression that fails without it: a_returning_route_resumes_at_the_readers_newest (and its JS mirror), an_unpolled_flap_back_keeps_reading_a_sparse_group, content_past_the_declared_end_refuses_the_track, an_end_below_a_seen_group_refuses_the_track, a_mid_group_end_caps_another_copys_group, a_late_mid_group_end_below_a_seen_object_refuses_the_track, an_end_below_an_expired_datagram_refuses_the_track. Two older tests that finished a replaced copy while another route continued now end it without declaring an end.
Merged current main: Recover::wants keeps the delivery classification from moq-dev/moq#6251 and adds the returning-route check; moq-dev/moq#6136's JS declared end merged cleanly with the JS route reuse. Earlier merges took in moq-dev/moq#6223, moq-dev/moq#6206 and moq-dev/moq#6209.
After the merge: moq-net cargo test 3,048 passed, 13 ignored; full js/net 2,655 passed. just check passed its JS, Rust lint, and quest steps; only its flake step fails locally building the unchanged GitHub runner image (unshare: write failed /proc/self/uid_map), an environment limit.
JS 16-reader/64-flap sweep: 13.10 to 1.61 µs per flap per reader; small sweeps sometimes cost more. Rust reader/route matrix completed, including 32 readers/32 routes and 256 readers/eight routes. These are local measurements, not an acceptance threshold.

Follow-ups

None. Route-failure classification landed in moq-dev/moq#6251.

(Written by GPT-6, updated by Claude Opus 5.5)

Co-Authored-By: GPT-6 <noreply@openai.com>

Remember the first declared end and reject contradictory copies, reuse retained subscriptions when routes return, and restrict pending-copy retention to the reader window. Mirror returning-route reuse in JS, with mocked-clock regressions and fanout benchmark coverage. Co-Authored-By: GPT-6 <noreply@openai.com>

Co-Authored-By: GPT-6 <noreply@openai.com>

Place the flap sweep before replay benchmarks so the datagram history benchmark can append without a content conflict. Co-Authored-By: GPT-6 <noreply@openai.com>

Co-Authored-By: GPT-6 <noreply@openai.com>

kixelated · 2026-10-21T23:35:49Z

Implemented the settled declared-end, flapping-route, and sparse-window fixes, including the JS returning-route regression and benchmark sweeps. No new API or wire choice remains. Source/test changes compose with moq-dev/moq#6209 and moq-dev/moq#6136; route-failure policy stays with moq-dev/moq#6102.

Keep this PR draft. /quest-iterate should integrate moq-dev/moq#6223, clear the existing moq-dev/moq#6113 full-suite failure, and run the complete gate outside the runner-image user-namespace restriction before /quest-ready. The affected Rust package passed 2,867 tests with the temporary base fixture repair; all selected JS tests passed. No separate follow-up quest is proposed because these blockers already have owners.

(Written by GPT-6)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

coderabbitai · 2026-10-22T06:14:28Z

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 19 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: 756e0038-0046-4690-91cc-6dccec4a76a5

📥 Commits

Reviewing files that changed from the base of the PR and between 1c72130 and 2b654f2.

📒 Files selected for processing: 11

js/net/bench/resume.ts
js/net/src/resume.test.ts
js/net/src/resume.ts
quest/m1/README.md
quest/m1/resume-compress.md
quest/m1/resume-handed-out.md
quest/m1/resume-rust-end.md
rs/moq-net/benches/origin.rs
rs/moq-net/src/model/group.rs
rs/moq-net/src/model/resume.rs
rs/moq-net/src/model/track.rs

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

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

chatgpt-codex-connector · 2026-10-22T06:14:28Z

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-22T22:04:39.447351Z	`2b654f2`	Manual request

ℹ️ 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: 44d9b09407

ℹ️ 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".

A reused copy kept the floor and datagram watermark from its first subscription, so groups and datagrams it buffered while replaced, below what another route already handed out, were handed out again. Raise both to where a fresh copy would start, in Rust and JS, with regressions that fail without each half. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 80eea6b9a1

ℹ️ 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".

…ross a flap back A group or datagram at or past the declared end, handed out or on any copy, now refuses the resumed track, matching the JS reader in #6136. Two tests that finished a replaced copy while another route continued now end it without declaring an end. An in-flight group's recovery treats a route that returned to the copy it reads as no change, adopting the new generation, so an unpolled flap back neither refuses a sparse group nor fetches a dense one from itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4a88470739

ℹ️ 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".

A mid-group end now caps the group it falls in when another copy hands that group out, and a datagram at or past the end by object refuses the track, so a resumed reader never hands out frames its declared end says do not exist. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Adopt identity datagram dedup from #6209: a reused copy no longer takes a datagram watermark, and the declared-end check reads the newest datagram handed out from the identity set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e22ac137f6

ℹ️ 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".

A mid-group end learned after delivery now refuses the resumed track when any copy holds a frame at or past it in that group, or the reader handed out a datagram at or past it by object. The hand-out cap stays for frames that arrive after a group is handed out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 398118c7a0

ℹ️ 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".

Resolve Recover::wants against #6251's delivery classification, keeping the returning-route reuse. Follow #6111's create_object and #6222's async clock advance in the tests, and retire the quest now that #6136 has landed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A late declared end compares against the highest datagram ever handed out, as JS's #newestDatagram does, rather than the budgeted identity set that forgets it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

kixelated · 2026-10-22T22:00:10Z

@codex review

(Written by Claude Opus 5.5)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2b654f22d6

ℹ️ 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".

kixelated and others added 5 commits October 21, 2026 16:07

chore: claim Rust resume end quest

ecb7ae3

Co-Authored-By: GPT-6 <noreply@openai.com>

Merge origin/main into Rust resume bounds fix

938f889

Co-Authored-By: GPT-6 <noreply@openai.com>

test(net): keep resume benchmark additions independent

3dc5d7f

Place the flap sweep before replay benchmarks so the datagram history benchmark can append without a content conflict. Co-Authored-By: GPT-6 <noreply@openai.com>

Merge origin/main into Rust resume bounds fix

25e234e

Co-Authored-By: GPT-6 <noreply@openai.com>

This was referenced Oct 22, 2026

test(net): cover lagging replicas on epoch wires #6232

Merged

fix(net): refetch a resumed group only when its route failed delivering it #6251

Merged

Merge origin/main into Rust resume bounds fix

44d9b09

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

kixelated mentioned this pull request Oct 22, 2026

quest: audit the tree against main (2026-10-22) #6273

Merged

kixelated marked this pull request as ready for review October 22, 2026 06:14