[kixelated]

Rust resume has no counterpart to the JS mirror-release bug that moq-dev/moq#6947 fixes. A new test pins that down.

Quest: quest/m1/resume-release-rust.md (deleted here, since it is done)

What I found

In JS, letting go of a replaced copy released the group mirror for every group that copy had cached for the sink. That included a group that recovery had already moved to another copy, so the group ended cleanly with frames missing.

Rust has no equivalent path:

• A Rust group has no per-subscriber mirror. track::Subscriber and track::Consumer have no Drop hook that reaches the groups they handed out.
• When Recover::adopt moves a group, the group's consumer switches to the serving copy's own group state. Its lease also moves to the serving copy, so the replaced copy is free to go.
• After that, dropping the replaced copy (Reader::let_go) leaves the moved group untouched.

Changes

• rs/moq-net/src/model/resume.rs: adds the test a_moved_group_outlives_the_replaced_copy_it_was_handed_out_from. It uses the fixed case from the JS fix, in both arrival and sequence order:
  1. The reader gets group 8 from the replaced copy.
  2. The replaced route gives the group up.
  3. Recovery fetches the rest from the serving route, starting at frame 1.
  4. The serving copy's first group bounds the replaced copy, and the test asserts the reader let it go (one copy left).
  5. The group still reads its remaining frames and ends cleanly.
• Deletes the quest file and its link in quest/m1/README.md.

Checks

• I added the four JS-only regression cases from test(net): check resume against a model of one logical track #6947 (including the mirror-release one) to a scratch copy of test(net): check resume against a model of one logical track #6947's Rust resume_model::regressions and ran them. All pass.
• just check origin/main passes: clippy, lint, and nextest (8962 tests passed, 96 skipped).

Choices made without asking

• The test is a direct unit test in resume.rs, not a case in the model harness, because that harness exists only in test(net): check resume against a model of one logical track #6947, which hasn't merged yet. This PR can land on its own.
• The serving copy's first group is 9, not 0 as in the JS case, so an ordered reader takes it too. In both cases the replaced copy is bounded at group 8.

Follow-ups

• Once test(net): check resume against a model of one logical track #6947 merges, copy the four JS-only regression cases into Rust's resume_model::regressions too. The two lists then cover the same cases, and a random sweep can't drift from the fixed ones. All four pass in Rust today.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[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-28T13:39:34.242469Z	e07330c	Draft marked ready

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

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: e07330c

No actionable bugs found. The focused regression is a good fit: it checks recovery starts at frame 1, verifies the replaced copy is actually released, then requires both remaining payloads and a clean end in arrival and sequence order. This directly exercises the cursor/lease handoff without adding production complexity or depending on the unmerged model harness.

Verification: static review of the changed test, its helpers, and the surrounding recovery/adoption and copy-retention paths. I did not run tests.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

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: a42fedfe-6c67-4518-9bb2-ac74d5c85c42

Commits

Reviewing files that changed from the base of the PR and between 270395a and e07330c.

Files selected for processing: 3

• quest/m1/README.md
• quest/m1/resume-release-rust.md
• rs/moq-net/src/model/resume.rs

Files with no reviewable changes: 2

• quest/m1/README.md
• quest/m1/resume-release-rust.md

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

Walkthrough

The change adds an async regression test for ordered and arrival-order reads. The test checks that a group handed out by a replaced copy remains readable after the serving route fetches its remaining frames and the replaced copy is released. The Rust resume release-path quest entry and its document are removed.

Priority: Normal

Merge Risk: Minimal · up to e0733

The regression test covers the release behavior, and no merge-blocking risk was identified.

Pre-merge checks: 5

Check name	Status	Explanation
Description check		The description clearly explains the new Rust resume regression test, its coverage, related quest cleanup, validation results, and follow-up work.
Title check		The title clearly and concisely identifies the main change: adding a Rust resume test for a moved group that outlives its replaced copy.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

Finishing Touches

✨ Simplify code

• Commit to this branch
• Create a new PR

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

[kixelated]

Merge summary: no changes made during merge. Head e07330c4aa0d3dbb0cfdec99f520db9aa6f2fe10 has green CI, a 👍 from Codex, and a clean review from the OpenAI review bot. quest check --base origin/main passes, and the PR deletes the finished quest. Landing as is.

Follow-up for /quest-plan: once moq-dev/moq#6947 merges, port its four JS-only regression cases into Rust's resume_model::regressions.

(Written by Claude Opus 5.5)