[kixelated]

Problem

A moq-lite-05/06 viewer joining a relay that already caches a track's history could wait forever for its first group. relay_session_join_broadcasts and relay_session_join_stale hung in their first lite-06 cell, so Nightly (Relay session benchmarks) could not finish.

The chain, reproduced with probes:

1. The last viewer leaves, so the relay drops its upstream subscription. The next viewer makes it subscribe again, and the publisher serves the cached group again.
2. The relay already holds that group whole and refuses the copy (Error::Duplicate).
3. A lite publisher before lite-07 reports every failed Group Stream as a final SUBSCRIBE_DROP, so the relay gets drop 0..1 for a group it holds.
4. track::Producer::give_up recorded that final drop over the cached group. It ended only groups still arriving and left this one whole, so the track contradicted itself.
5. Since moq-dev/moq#8476 a reader's drop scan and its served groups share one record. A new reader on the relay reported the drop before handing out the group, then refused the group as already told. On lite-06 its TrackRun waited for a later group to resolve its start, and none came.

Only the upstream hop's version matters: a lite-07 upstream never sent the drop, and the downstream version made no difference.

Approach

A final drop now leaves a live-feed group that already finished here, just as a stale drop already did and as an arriving group clears an earlier drop. The group stream wins over its drop. A group still arriving is still ended with the drop's error. The record follows the copy serving the group, so a fetched head that serves it whole beside an open or aborted live copy keeps it too, for stale and final drops alike. The rule sits in track::Producer::give_up (Rust) and #drop (js/net), so the track never holds both the group and a drop for it, whatever order a reader polls them in.

• rs/moq-net/tests/integration/history_join.rs: three viewers join a relay one after another, on lite-05, lite-06, lite-07 and moq-transport-22. Without the fix the third lite-05/06 viewer hangs.
• a_drop_leaves_a_group_a_fetched_head_serves (Rust): a final drop beside an open live copy and a stale drop beside an aborted one each leave the group a finished fetched head serves. Fails without the fix (found by review).
• a_final_drop_leaves_a_finished_group (Rust) and a final drop leaves a finished group (JS) pin the rule. Each fails without its fix.
• /verify-subscribe: OnDrop now keeps a whole copy's record on a final drop, matching give_up. SubscribeAccounting.cfg passes (13.3M distinct states). StreamWins.cfg still fails only on its documented open finding.

Impact

• No public API or wire change. Behavior change: a relay or subscriber that holds a group whole no longer records an upstream's final drop of it, and still serves that group to its readers.

Alternatives

• Have the relay stop a duplicate copy without an error the publisher reports as a drop. Rejected: a lite-06 publisher reports any failed stream, including a plain cancel, as a final drop, and production relays run that version. The track would still have to handle the contradiction.
• Have a reader prefer a cached group over a drop in its scan (DropCursor::next). Rejected: it only fixes one reader's order and leaves the contradiction in the track.

Caused by

• moq-dev/moq#8476: its shared per-reader record made the earlier contradiction (a final drop recorded over a group held whole) observable. A drop scanned before its cached group now marks that group told. The contradiction itself is older than fix(net): a failed fetch below the end no longer holds it open #8476.

Decisions made without asking

• Exempted only groups a copy still serves whole: one the live copy finished, or one a fetched head serves (Slot::serves_end, as DropCursor::cut already judges). A final drop still ends the live copy still arriving, as before.
• No js/net change for the fetched-head case: its cache holds one copy per sequence, so #drop already judges the copy readers get.
• Mirrored the fix in js/net, which had the same gap, per the Rust/JS sync rule. I adjusted lite draft-07: a final drop replaces an expired stream so its group 1 is still arriving rather than held whole, which keeps that test's intent under the new rule.
• Updated the drop-index test drop_changes_follow_split_coalesced_and_recreated_runs the same way (its recreated group is still open), so it still covers re-coalescing.
• Rewrote the quest to its remaining step, confirming a scheduled Nightly, instead of deleting it. Nightly is never dispatched.

Verification

• cargo bench -p moq-relay --features test-support --bench session -- --quick relay_session_join_: all 63 cells finish. The first lite-06 join_broadcasts cell takes about 0.23 ms per join.
• just check, just ci, cargo test -p moq-net --all-features, and bun test in js/net all pass.
• On 9ba1d52: just check and cargo test -p moq-net --all-features pass. js/net and the TLA rule are unchanged, so bun test and SubscribeAccounting.cfg were not rerun.

Follow-ups

• None new. The quest stays, waiting on the first scheduled Nightly after merge.

Review

Rewritten by quest review after each round.

• Reviewer: Codex, gpt-6.1-sol, medium effort
• Session: 01a1d073-1363-7e71-acda-b4b65712e1a0, round 2
• Head: 9ba1d52
• Base: origin/main at 8c8f0ef
• Score: 9/10, The incremental change preserves groups served by fetched heads while still aborting unfinished live copies, with no actionable defects found by source review; tests and model checks were not run as instructed.
• Findings: none

Models

Size: M
Implementer: Claude Code, claude-opus-5-5, medium effort
Implementer: Claude Code, claude-opus-5-5, medium effort (iterate, S)

🤖 Generated with Claude Code

(Written by Claude Opus 5.5 with medium)

[coderabbitai]

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 10 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: 74b63bfc-e9e8-47a5-94f0-7b1734e9fa4c

Commits

Reviewing files that changed from the base of the PR and between 4fa5fa5 and 9ba1d52.

Files selected for processing: 8

• js/net/src/cache.test.ts
• js/net/src/lite/publisher.test.ts
• js/net/src/track.ts
• quest/m0/nightly/lite06-join-hang.md
• rs/moq-net/src/model/track.rs
• rs/moq-net/tests/integration/history_join.rs
• rs/moq-net/tests/integration/main.rs
• rs/moq-net/verify/subscribe/SubscribeAccounting.tla

• 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

[pullfrog]

ℹ️ No critical issues — one narrow gap in the claimed invariant.

Reviewed changes

• The rule — track::Producer::give_up and #drop leave a group that finished here out of the recorded drop, while still ending a group still arriving with the drop's error.
• Test — history_join::each_joining_viewer_reads_the_held_group fails without the rule and passes with it, on four sim seeds.
• Coverage — the new Rust and JS unit tests pin the rule; two older tests keep their group open so they still exercise what they name.
• Model and quest — OnDrop leaves rec alone for a cached whole group, and the quest is reduced to confirming the next scheduled Nightly.
• Verified locally — full moq-net lib suite (3221 passed) and js/net cache/track/fill suites (299 passed).

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

Important

A final drop is still forgotten when the only copy serving the group is an unfinished fetched head and that head later dies, so the track ends up with neither the group nor the drop.

Reviewed changes

• Fetched-head rule — give_up now decides the abort from the live copy and the recorded run from the copy serving the slot, so a finished fetched head keeps the group for final and stale drops while an arriving live copy is still ended.
• Coverage — a_drop_leaves_a_group_a_fetched_head_serves builds the slot with a finished head for a final drop beside an arriving live copy and a stale drop beside an aborted one.
• Model comment — SubscribeAccounting.tla now says give_up keeps a group a copy serves to the end, the live one or a fetched head.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

serves_end() is true for a fetched head that is still arriving, so this exemption forgets the upstream's final verdict while the only copy holding the sequence is a fill in flight. If that fill then dies, the live copy this drop aborted is Fall::Abandoned for Old/Cancel/Evicted errors and stores no cut, and Slot::cut only reads the live copy, so the track ends up with neither the group nor the drop. A reader whose range resolves at that sequence then gets nothing to start from, which is the stall shape this PR fixes; the new test finishes its head, so it stops one step short.

Technical details

# A final drop forgotten when its serving head dies

## Affected sites
- `rs/moq-net/src/model/track.rs:4492` — `slot.serves_end()` is true for an unfinished head, so `left.remove` discards the final verdict.
- `rs/moq-net/src/model/track.rs:4483` — the live copy is aborted later with `Error::Stream(hole.error())`.
- `rs/moq-net/src/model/group.rs:2788` — `StreamError::Old` (also `Cancel`, `Evicted`, `DeliveryTimeout`, `TooFarBehind`) aborts as `Fall::Abandoned`, which stores no cut error.
- `rs/moq-net/src/model/track.rs:847` — `Slot::cut` reports only the live copy's cut, never the head's, so even a cut head leaves `given_up` empty.
- `rs/moq-net/src/model/track.rs:1596` — `given_up` then returns `None`; `landing` (2975) serves nothing.
- `rs/moq-net/src/lite/publisher.rs:5577` — the downstream cursor skips the sequence or parks, with no drop for the wire.

## Required outcome
- A final drop is never discarded while the copy that serves the sequence can still disappear without leaving either a cut error or a record.
- A head that does finish still wins over the drop: no recorded drop may sit beside a group readers are handed.

## Suggested approach (optional)
- Exempt a final drop only when the serving copy has finished (`Slot::is_settled()`), and forget the recorded run once a head becomes whole (the `insert_group` clearing at `track.rs:2714` has no head equivalent); then the group still wins when it lands, and the verdict survives until it does.
- Or hold the verdict in the slot and surface it through `Slot::cut`/`given_up` once no copy serves the end.

## Evidence
- Temporary test (reverted, tree clean): the new test's setup with an unfinished head, then `drop_groups(0..1, StreamError::Old)` and `head.abort(Error::Cancel)`. It holds today that `state.drops` is empty, `state.given_up(0)` is `None`, and `slot.is_read()` is false.
- Model gap: `SubscribeAccounting.tla:370` only keeps `rec` for `cached[g] = "whole"` on one copy that never dies, so `SubscribeAccounting.cfg` does not cover this window.