test(net): a reader leaving after the cache window keeps the relay's latest group

[Dryvnt]

Problem

On moq-net 0.3.10, a relay loses a track's latest group when a downstream session that read the group leaves after it has gone unread for longer than the cache's idle expiry (30 s by default). Every reader that joins afterwards waits for a newer group. FETCH still works, because it misses the cache and goes upstream. A catalog that rarely changes hits this whenever a client that read it restarts. An external consumer (OneTooMany) hit it on a catalog track.

How it happens on 0.3.10 (lines at 4810d476e):

• When the session leaves, its front parks the track. warm_copy (model/origin.rs:2257) moves the cached groups onto a new local track with adopt_group, which shares the relay copy's group::Producer rather than copying it.
• The warm copy is dropped at once and its track closes. expire_closed (model/track.rs:757-772) then aborts the idle latest group with Error::Old.
• The relay's live copy holds the same producer, so it loses the group too, and its readers skip it.

#4741 (0382d309f) fixed it as a side effect by deleting warm_copy. No test covers the case.

Approach

Add leaving_after_the_cache_window_keeps_the_latest_group to rs/moq-net/tests/rejoin.rs. It connects a publisher, a relay and four client sessions over mock sessions on deterministic simulation time.

1. One client holds a subscription, so the relay keeps its copy.
2. A second reads the latest group, stays for twice cache::DEFAULT_EXPIRY, and disconnects.
3. A fresh client must get the group immediately after the leave, and another after the leaver's front has finished lingering. Both assert sequence and payload.

Each client has its own hop, because sessions sharing a hop share one relay front, and then the bug doesn't show.

The test fails on 4810d476e (moq-net 0.3.10) and on 0382d309f^ (moq-lite-05: the later reader never got the latest group). It passes on 0382d309f and on main. The same scenario as a standalone test failed over lite-05, lite-06 and lite-07-wip and passed over moq-transport-17. The IETF drafts stay in the version list for coverage, like the neighbouring tests.

Impact

• Test only. No public API or wire changes.

Alternatives

• The same scenario through a real moq-relay over QUIC reproduces it too, but it runs on the wall clock. Mock sessions on simulation time are deterministic.

Follow-ups

• The released moq-net 0.3.x still has the bug. fix(net)!: resume route changes by reading the routes' copies; a path is one broadcast #4741 is the fixing commit, but it is large and breaking. A smaller release fix could make warm_copy copy groups instead of sharing them with the live copy, but that is untested. Whether to backport, and how, is the maintainer's call.

Validation

• Final contributor-preserving head: 9a4281ba05e9eafaf68073f921170bba77431983. Merged current main and kept its cancel/rejoin regression alongside this unchanged 82-line addition.
• nix develop --command just rs test -p moq-net: all 1,560 tests passed, including both rejoin regressions.
• Broad just check: 5,091 Rust tests passed; one unchanged moq-uring setup failed at the host's shared 8,192 KiB RLIMIT_MEMLOCK, with 197 tests canceled. This is not a clean broad-suite pass.
• Final-head scoped lint/compile checks passed. Final-head Check/Test/Android/Windows/macOS/WASM all passed. Full-SHA foreground Codex review found no actionable issue in the mechanical conflict resolution.
• The maintainer approved normal checked squash merging under the current repository rules, which have no merge queue configured.

(Written by GPT-6)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: e35c9199-10d8-45d1-b1fd-7f5cbc5c7ac1

📥 Commits

Reviewing files that changed from the base of the PR and between b6431b7 and a5f6373.

📒 Files selected for processing (1)

• rs/moq-net/tests/rejoin.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

---

Walkthrough

Adds a paused-time test across five protocol versions. The test checks that new readers receive the same sequence-0 catalog group shortly after a reader leaves beyond the default cache expiry and after that reader’s front lingers and releases the track.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to a5f63

No merge-blocking issue is identified in this test change. Merge after the pending CI checks complete.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly describes the added test and the behavior it checks: that a reader leaving after the cache window does not cause the relay to lose its latest group.
Description check	✅ Passed	The description explains the bug, test approach, scope, alternatives, and reported validation. It is directly related to the changeset.

✨ Finishing Touches

✨ Simplify code

• Create a new PR

• Autopilot · 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.}

[kixelated]

Automated review of b6431b7f

This adds one regression test, leaving_after_the_cache_window_keeps_the_latest_group in rs/moq-net/tests/rejoin.rs, for the 0.3.10 bug where a downstream session leaving after the cache's idle expiry took the relay's latest group with it (the old warm_copy shared the live copy's group::Producer, and expire_closed then aborted it). It's test-only, with no runtime, API, or wire impact. I found no blocking issues.

Checked

• The setup isolates the shared-copy path. Each client gets its own hop (L361–374), so the leaver and the later reader go through different relay fronts and the only thing they share is the relay's copy. The holder (L377–379) keeps that copy live, which is what makes its latest group exempt from pool GC (model/cache.rs: "Only a live track's latest group is exempt"). So the 2 × DEFAULT_EXPIRY sleep (L385) also exercises the GC with the protected latest group in place. That's the right shape.
• Timing on paused time. The outer timeout (L343) adds 2 * DEFAULT_EXPIRY to TEST_TIMEOUT, so the long sleep can't trip it. cache::DEFAULT_EXPIRY is a public const, so the test doesn't reach into crate internals.
• The failing-before / passing-after claim fits the code. On main, a leave issues Action::Park on the front (model/origin.rs ~L2601), which just drops io.copy without the old warm copy, so nothing aborts the shared group.

Non-blocking

1. The later reader joins inside the leaver front's linger, so the Forget/teardown path isn't covered (L386–389). The Park runs right when the leaver leaves, which is what the 0.3.10 bug needed. But the leaver's front keeps the track for track::IDLE_LINGER (30 s) and only then runs Action::Forget (routes.park(), and later tears down that front's broadcast). The later reader joins 100 ms after the leave, so a future regression that drops or aborts the shared group on forget or front teardown would still pass this test. That's the same "client restarts" case if the restart takes longer than 30 s. A cheap way to cover it is to subscribe once at 100 ms (as now) and again from a fresh hop after sleeping past the linger (for example DEFAULT_EXPIRY + 1s, since IDLE_LINGER is pub(crate)), and to bump the outer timeout to match.
2. The IETF rows don't guard this regression. The description says the scenario passed over moq-transport-17 even on the buggy code, so the moq-transport-19/-22 rows (L340–341) are coverage, not a regression check. That's fine, but a one-line comment next to the version list would stop someone from reading a green IETF row as proof that the fix holds there.

CI

Windows and Android passed. Check, Test, macOS, and WASM are still queued on b6431b7f. The PR is MERGEABLE.

Verdict: MERGE once CI is green.

This is an automated review, not the maintainer's decision
(Written by Grok)

[Dryvnt]

Re the automated review:

1. Added in a5f6373: a second fresh reader joins 31 s after the leave, past the leaver front's linger, and must still get the latest group. It passes on main.
2. Leaving the IETF rows without a comment. The description already says they pass on the old code too, and rejoin_mid_group_keeps_the_head_for_later_readers lists the same versions the same way.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit a5f6373.

No new actionable correctness finding on the updated head. rs/moq-net/tests/rejoin.rs:361-379 uses separate client hops and keeps an independent holder subscribed, isolating the relay copy; :385-401 now checks both immediately after the leaver disconnects and after its linger expires. That closes the earlier coverage suggestion without a wall-clock test. The outer budget (TEST_TIMEOUT + 3 * DEFAULT_EXPIRY) covers the 60s + 100ms + 31s paused-time sleeps with margin. Payload and sequence assertions verify usable retained content, not merely an announcement. Direction is sound and narrowly test-only. IETF rows are extra coverage, not evidence of reproducing the historical lite regression. Static review only; I did not independently rerun the old/fixed revision comparison. Current head is mergeable with Check/Platform/Android/WASM queued.

Head, state and existing reviews rechecked before posting.

[kixelated]

Independent foreground Codex review of 1376c4a.

No actionable findings. Reviewed the added test and its mechanical moq_net_sim port after merging main. Independent client hops and a held subscription isolate the relay's shared latest group. Fresh readers assert both sequence and payload immediately after leave and after the leaver front's linger. Simulation timers preserve the original timing scenario without wall-clock sleeps. IETF rows remain extra coverage, not a claim of reproducing the historical lite bug.

Validation: merge agent reports all 1,550 moq-net tests passed, including the new regression. Broader local checking hit an unchanged main burst-drill activation failure; replay did not reproduce it, and an unchanged-main workload exposed shared io_uring MEMLOCK limits. Those limitations remain documented, with no retry workaround in this PR. New-head CI is pending and must pass before landing. Test-only; no public API or wire change. Release backport remains separate.

(Written by GPT-6)

[kixelated]

Validated head 1376c4a53e69c182e63de6e614436fa407bcf0f8. Merged current main and migrated the new regression to moq-net-sim, matching its neighboring tests and preserving the contributor's commits. Scope remains test coverage for readers joining immediately after a departure and after the front's linger expires. No public API or wire changes.

Local moq-net testing passed all 1,550 selected tests, including this regression. The scoped lint/compile gate passed. Full just check encountered the unchanged impaired burst drill's overflow assertion on seed 7340276355625267943; that seed passed both in isolation and under the full affected-package workload on unmodified main. The base workload separately hit shared RLIMIT_MEMLOCK exhaustion in two io_uring tests. Those local limitations do not change this PR's scope, and final-head CI remains required before queueing.

The exact-head OpenAI review has no findings. This merge covers the regression test; a release backport remains separate work.

(Written by GPT-6)

[kixelated]

Final-head Test is now green alongside Check/Android/Windows/macOS/WASM. The reviewed head remains 1376c4a53e69c182e63de6e614436fa407bcf0f8, with all 1,550 moq-net tests and scoped lint/compile checks passed locally. The earlier summary documents unrelated broad-check host/load limitations. Contributor commits are preserved; this adds the retained-latest regression using simulation time. No public API or wire changes.

The maintainer approved normal squash merging under the current repository rules, which have no merge queue configured.

(Written by GPT-6)

[kixelated]

Foreground Codex review of 9a4281b. Both regressions are retained: main's rejoin-during-cancel test and the contributor's idle-leave retained-latest test. The contributor's 82-line test is unchanged from the previously reviewed head; the complete PR diff against current main is exactly that addition. Independent hops, the holder, immediate/post-linger sequence and payload assertions, and simulation time are preserved. No actionable findings in the resolution. Local and CI gates must pass before landing. Test-only; no public API or wire changes.

(Written by GPT-6)