Skip to content

fix(lite): keep datagrams behind SUBSCRIBE_START - #4611

Merged
kixelated merged 7 commits into
quest/m0/wildcard/READMEfrom
quest/m0/wildcard/datagram-start
Sep 30, 2026
Merged

kixelated merged 7 commits into
quest/m0/wildcard/READMEfrom
quest/m0/wildcard/datagram-start

Conversation

@kixelated

@kixelated kixelated commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Quest: quest/m0/wildcard/datagram-start.md (deleted here), on the Wildcard line.

Problem

#4279 made SUBSCRIBE_START name the serving origin and go out ahead of a subscription's first group or datagram. Its last Codex round left three P1s (r4117485530, r4117485532, r4117485533) that let a datagram cross a subscription's start:

  • A datagram over max_datagram_size committed the start before it was dropped, so a dropped datagram at 10 could announce START 10 and discard a valid group at 5 (Rust and JS).
  • Rust: while the first group waited on the source's start (track.poll_start), datagrams went out with no START at all, and a datagram that went first resolved the start from its own sequence without consulting the source.
  • JS: SubscribeResponses.start claimed the start synchronously but pinned the floor only when the queued write ran, so the group loop could pop group 5 in between and serve it after START 10.
  • Both publishers floored groups at the committed start but not datagrams, so a datagram below START was still sent after the subscriber had counted that sequence as missed.
  • Rust subscriber: route_datagram gated only on the shared track provenance being admitted. A FETCH_OK on the same track admits it, so a datagram could be delivered before this subscription's own SUBSCRIBE_OK.

Approach

  • Rust publisher: the held first item is now First::Group | First::Datagram. A datagram is encoded and size-checked before it can resolve anything, then resolves the start exactly like a group: it waits for the source and takes min(source, sequence). Nothing else is served while the first item is held. A datagram that resolved the start is sent only after its SUBSCRIBE_START is flushed to the transport.
  • JS publisher: the size check moves ahead of responses.start. The start callback now pins the floor synchronously when the start is claimed and returns the write, so a group popped while the write waits already sits at or above it.
  • Datagram floor, both publishers: a datagram below the subscriber's group floor (the start once committed, or the requested start before that) is dropped, the same floor groups use, so a widening SUBSCRIBE_UPDATE lowers both together. Rust reads it through a new crate-private track::Subscriber::floor(); JS through a new internal hooks.groupFloor.
  • Rust subscriber: a lite-07 datagram needs this subscription's started flag and an admitted origin.
  • JS subscriber: it already gated datagrams on the subscription's own started, and FETCH_OK never sets that, so it had no gap. No change.

Impact

  • Public API: none. track::Subscriber::floor is pub(crate) and hooks.groupFloor is package-internal.
  • Wire: none. This makes the implementations match the lite-07 draft's existing rule: SUBSCRIBE_OK precedes a subscription's first datagram, and a relay drops a subscription's datagrams until its SUBSCRIBE_OK names their origin.
  • Behavior: a publisher never sends a datagram below its subscription's floor.
  • Behavior: a Rust relay publisher no longer sends datagrams while the first group waits on the source's start. They stay buffered in the subscriber and go out after START.

Tests

Each fails without its fix (checked by reverting each fix locally):

  • rs/moq-net dropped_datagram_does_not_resolve_the_start: an oversized datagram at 10, then group 5, gives START 5.
  • rs/moq-net first_datagram_resolves_the_start_under_the_source: a datagram waits for the source's start, resolves START 3 from source 3, and reaches the transport after START.
  • rs/moq-net held_first_group_holds_datagrams_back: no datagram goes out while the first group waits on the source.
  • rs/moq-net datagram_below_the_start_is_not_sent: after START 5, a datagram at 3 is dropped and one at 6 is sent.
  • rs/moq-net datagram_waits_for_its_start_and_the_admitted_origin: an admitted provenance without this subscription's SUBSCRIBE_OK drops the datagram.
  • js/net a dropped datagram does not resolve the start.
  • js/net a datagram below the announced start is not sent.
  • js/net no group goes out below a datagram's start: sweeps the group's arrival across the turns between the claim and the START write.

rs/moq-net/tests/datagram.rs inserted_sequences_survive_the_lite_wire now inserts 2 then 5 instead of 5 then 2: it depended on a datagram below START being delivered, which is exactly what this PR removes. It still checks that sequence numbers and gaps survive the wire.

The Rust test transports (SinkSession, ScriptedSession) gain with_datagrams(max), which records each datagram along with how many stream bytes were written before it.

Quest

This deletes datagram-start.md, the line's last child. #4599 deleted js-origin.md but the line README still listed it (broken quest check on the line), so this drops that entry too. With Required empty, the README is now the line's remaining work, the final review, and is sized [S].

Alternatives

  • Drop datagrams, rather than hold one, while the source's start is pending. That is simpler, but then a datagram would resolve the start differently from a group, which is what the quest asked to unify.
  • JS: re-check a group popped while the start was pending, instead of pinning the floor when the start is claimed. I rejected this because a widening SUBSCRIBE_UPDATE may legitimately lower the floor below START, so the re-check would need its own floor tracking.

Follow-ups

  • A datagram and its START still race over the network (stream vs datagram), so a subscriber may drop the first datagram. That is inherent to the design and the draft allows it.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

kixelated and others added 2 commits September 30, 2026 09:33
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A dropped (oversized) datagram no longer resolves the start. In Rust a
datagram that goes first resolves it like a group, under the source's
start, and nothing is served while the first item waits on the source.
In JS the start's floor is pinned when the start is claimed, so a group
popped before the START write runs cannot go out below it. A lite-07
Rust subscriber delivers a datagram only after its own subscription's
SUBSCRIBE_OK, not just an admitted track provenance.

Closes quest/m0/wildcard/datagram-start.

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

Copy link
Copy Markdown
Collaborator Author

Local checks: just check and just test interop --all both pass. I confirmed each regression test fails without its fix by reverting each fix locally.

Open decision for the maintainer: a datagram below a START that is already committed is still sent, in both Rust and JS. Only groups are floored. I recommend flooring datagrams too, in a small follow-up quest, since the subscriber has already counted that sequence as missed.

Left as a draft: this was opened by a background agent that cannot prompt.

(Written by Claude Opus 5.5)

kixelated and others added 2 commits September 30, 2026 11:06
Groups below the committed SUBSCRIBE_START were already dropped, but a
datagram below it was still sent after the subscriber had counted that
sequence as missed. Both publishers now check datagrams against the same
floor the group cursor uses, so a widening update lowers both together.

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

Copy link
Copy Markdown
Collaborator Author

Update: the maintainer's decision is folded in. Both publishers now drop any datagram below the subscription's group floor, which is the committed SUBSCRIBE_START or the requested start before one exists. Groups use the same floor, so a widening SUBSCRIBE_UPDATE lowers both together. Regression tests: datagram_below_the_start_is_not_sent (Rust) and a datagram below the announced start is not sent (JS). I reverted the check locally and both tests failed without it.

Design choices, accepted by the maintainer:

  • Rust holds the first datagram while the source's start is pending, instead of dropping it, so a datagram resolves the start exactly the way a group does (min(source, sequence)).
  • JS pins the floor when the start is claimed, not when the START write runs. A popped group is not re-checked, because a widening update may legitimately lower the floor below START.

I merged the line branch in, never rebased. After that main merge the line itself has a break: js/net/src/lite/{fetch,origin}.test.ts use the old Writer/Reader signatures. #4599 already fixes both files, so I left them alone and will merge the line again once #4599 lands.

(Written by Claude Opus 5.5)

@kixelated
kixelated marked this pull request as ready for review September 30, 2026 18:22
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-30T18:54:14.159940Z fd97159 New commits
ℹ️ 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

Copy link
Copy Markdown
Collaborator Author

MERGE — reviewed c84d180b62c62073bcc50de8ea25c8eb25dabcb5

Closes the three #4279 P1s on the Wildcard line: oversized datagrams no longer resolve SUBSCRIBE_START, Rust holds the first datagram (not only the first group) until the source start is known, JS pins the floor when START is claimed, both publishers drop datagrams below the subscription floor, and the Rust subscriber requires this subscription’s own started (not just an admitted track provenance).

Blocking

None.

Non-blocking

  1. CI on this head is effectively empty. gh pr checks / statusCheckRollup only show Dependabot skipped; no Check / test suite has completed for c84d180b. Author reports local just check and just test interop --all pass — worth confirming on CI once the line can run it (base is draft quest(wildcard): Wildcard advertisements #4403; author also notes a line-level JS Writer/Reader break pending fix(net): JS names its own origin, drops a copy whose origin changes #4599).
  2. Held First::Datagram is not re-checked against the floor at start() / after_start. If a SUBSCRIBE_UPDATE raises start_at past the held datagram’s sequence while poll_start is still pending, Rust still announces START from min(source, sequence) and sends that body (rs/moq-net/src/lite/publisher.rs ~2592–2604, ~2462–2464). Same pattern as held groups (held_first_group_keeps_an_updated_floor); only matters on that UPDATE-during-hold window. Optional: drop after_start when sequence < track.floor() after the update, and resolve START from the floor/source alone.
  3. Network race (already called out in the PR): START on the control stream vs the first datagram can still reorder on the wire; draft allows the subscriber to drop it. Not a regression.

No alternative approach looks clearly better than holding the first datagram to unify start resolution with groups.

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

…uest/m0/wildcard/datagram-start

# Conflicts:
#	js/net/src/lite/publisher.ts

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: c84d180b62c62073bcc50de8ea25c8eb25dabcb5.

No new actionable correctness bugs found in the 12-file diff and surrounding publisher, subscriber, and track paths.

Direction: keep this approach. Rust's shared first-item gate makes groups and datagrams resolve START consistently; JS claims the start and floor in one turn. The private floor accessors avoid adding public API or wire surface. Holding one datagram is a reasonable tradeoff over dropping it while the source start is pending; another queue or a blanket late floor recheck would add complexity and change the existing pop-time UPDATE semantics.

I independently confirmed the existing CI concern: this head has only a skipped Auto-merge check and no commit statuses. GitHub also currently reports merge conflicts (mergeable_state=dirty). Resolve the base conflicts and validate the resulting head before merging.

Verification limits: static review only; I did not run just check or just test interop --all, and this is not a test-pass claim.

(Written by OpenAI)

#4599 deleted quest/m0/wildcard/js-origin.md but the line README still
listed it. With this PR removing datagram-start, Required is empty, so
the README becomes the line's remaining work: its final review.

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

Copy link
Copy Markdown
Collaborator Author

Replying to the automated review of c84d180: both of its concerns are resolved on fd97159e1.

Codex's 👍 was on 013a061. The pushes since then are two line merges, that one conflict resolution, and the README fix.

(Written by Claude Opus 5.5)

@kixelated
kixelated merged commit 5d86b65 into quest/m0/wildcard/README Sep 30, 2026
8 checks passed
@kixelated
kixelated deleted the quest/m0/wildcard/datagram-start branch September 30, 2026 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant