Repository navigation
Conversation
Quest-Claim: 0e6b5d7717bf8fa5
`a_published_scene_has_its_own_broadcast` failed with `the video finishes: Unroutable` when its last viewer subscribed and the studio quit before the relay had the subscription's answer. The relay refuses a subscription still unanswered once the scene's broadcast is withdrawn, since the program's broadcast still covers its path there. A viewer the relay already serves ends cleanly. Each viewer now waits for its first group. Closes quest/m0/unpublish-clean.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Pick (/quest-complete, maintainer away under /afk, recommended option): ✅ Land the test-only fix. Viewers the relay is already serving end cleanly on unpublish, rename, remove and quit, and (Written by Claude Opus 5.5) |
|
Warning Review limit reachedYou'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 5 minutes. View limit details
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. |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head 4d2de7007d8eaafb35e2cfef8fd456669afe4ba4 against base b87cc76a5384768d671d57197e40d47ad0b21ab3.
[P2] Keep the first group in the clean-end assertion (rs/moq-studio/src/engine.rs:3472–3476)
served() discards the group it receives, so ended() cannot check that group's frames or terminal error. On the immediate-quit path this can be the only group produced before shutdown: a cut tail in that group can now go unnoticed while video.recv_group() reports a clean track end. Those are independent outcomes in the pinned transport; a_cut_group_reaches_its_readers explicitly exercises a group returning an error followed by the track returning Ok(None).
Preserve the first group and pass it into the teardown reader, which should drain it after unpublish/stop before reading subsequent groups. This keeps the useful subscription-readiness barrier without removing the existing clean-tail assertion.
Direction: retain the narrow test-only fix, but keep checking every group already received. No production drain machinery is needed for this finding.
Verification limits: GitHub-only static review of the test, teardown paths, quest changes and pinned transport sources. I read the pinned quest context as source but did not execute quest context, builds, tests or the contention reproduction; the reported pass counts are not independently verified. COMMENT only, not approval.
(Written by review (OpenAI))

Claims and closes quest/m0/unpublish-clean.md.
Problem
engine::tests::a_published_scene_has_its_own_broadcastsometimes fails withthe video finishes: Unroutable.The quest blamed unpublishing: the studio closes a scene's broadcast before the relay has the end of a track. A reproduction showed a different cause. Every failure happened when the test quit, and every one was the same race:
studio/tall/0.avc3. The studio answered and served group 0.studio/tall. The unannounce reached the relay before the SUBSCRIBE's answer.studiowas still announced, and on the relay a session's route covers the paths beneath it. So the relay sawstudio/tallmove to another instance. It refused the subscription that was still unanswered withUnroutable:The quest's diagnosis does not hold. Once the relay has answered a subscription, withdrawing the broadcast leaves it alone. The track runs to its own end (
moq_net::model::origin,Action::End: "Tracks in flight carry on"). In this test, the unpublish half's viewer is already being served, and it ended cleanly in every run, before and after this change. Only a subscription the relay has not answered yet can endUnroutable. Its subscriber has received nothing from the track, andUnroutableis the relay's normal answer for a request that races the removal of its route.Approach
Test only. A new helper,
served, subscribes and waits for the first group before the test unpublishes or quits. A real viewer is past that point too. Both subscriptions in the test use it. The test's doc now says that the clean end applies to a viewer the relay is serving.I did not add the planned wait before closing a scene's broadcast. See Decision: it narrows the race but cannot close it.
This deletes the quest and its link in the m0 README.
Impact
None for users. Test only. No public API or wire change.
Validation
taskset -c 24-31), on lavapipe, withRUST_LOG=info,moq_net=debug. Before: 2 of 48 runs failed, both on quit, both with the log above. After: 0 of 120.nix develop --command quest gate just cion nixos atf9b8980: passes (279 tests). Again at4d2de70, after merging main: passes (281 tests).just render-sw's second command) atf9b8980: 6 of 6 passed.quest check: passes (part ofjust ci).Decision
The quest's Plan picked "unpublishing waits the same way" as quitting before the studio closes the broadcast. That pick rested on the diagnosis above, which turned out wrong. Picked on this PR on 2026-11-07 (/quest-complete, maintainer away under /afk, recommended option):
Unroutableis the correct answer for a subscription still being set up when its path goes away.moq_net::broadcast::Demanddrops. It is about 100 lines and still leaves gaps: a re-SUBSCRIBE without TRACK, since demand drops once the studio writes the tail rather than once the relay reads the answer, and a SUBSCRIBE still on the wire.Follow-ups
None.
Review
Codex (
quest review), gpt-6.1-sol, medium effort. Round 1 on head4d2de7007d8eaafb35e2cfef8fd456669afe4ba4againstorigin/mainatb87cc76: no findings.🤖 Generated with Claude Code
(Written by Claude Opus 5.5)