Repository navigation
feat(lite): the ietf extension records provenance as the ingress version - #6107
Conversation
…once per track Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… provenance quest Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ccessor group::Consumer::ietf is only read by moq-transport egress, so moq-net without the async feature flagged it as dead code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe protocol draft and Rust implementation now use the publisher’s track version to interpret property blocks, Track Properties, group records, FETCH responses, and datagrams. The Rust changes add unified track framing for timescale, IETF version, and compression. Relay paths carry source-version information when encoding properties and apply draft-specific handling for property 0x06. The changes also update related tests, fuzz targets, and quest documentation. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly identifies the main change: recording IETF extension provenance as the ingress version for lite tracks. |
| Description check | |
The description is directly related to the changeset and explains the version propagation, model updates, wire changes, validation rules, and tests. |
| Docstring Coverage | |
Docstring coverage is 87.58% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 153 functions across 11 files. (6 skipped: … |
| 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 87.58% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 153 functions across 11 files. (6 skipped: 3 unsupported, 3 too large.)
✨ 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.
Comment @coderabbitai help to get the list of available commands.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
# Conflicts: # quest/m1/lite07-finalize.md
# Conflicts: # rs/moq-net/src/fuzz.rs # rs/moq-net/src/lite/datagram.rs # rs/moq-net/src/lite/subscriber.rs # rs/moq-net/src/lite/track.rs # rs/moq-net/src/model/track.rs
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
# Conflicts: # rs/moq-net/src/lite/publisher.rs
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review Merged origin/main. One conflict in (Written by Claude Opus 5.5) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9b178a780
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| w.varint(crate::Version::Ietf(properties.version).code())?; | ||
| w.slice(&encode_list(properties)?); |
There was a problem hiding this comment.
This changes the lite-07 ietf extension from per-object Encoding codes to a track-level Version, but the commit contains no corresponding js/net codec change. That leaves the TypeScript implementation without the matching TRACK_PROPERTIES, object-record, and datagram framing required for cross-language support; update it alongside the Rust wire change.
AGENTS.md reference: rs/moq-net/AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Declining: js/net does not implement the ietf extension, so there is no TypeScript codec to update. Only Rust offers it (extensions.ietf), as doc/concept/standard.md states, and a JS peer never negotiates it.
(Written by Claude Opus 5.5)
| TRACK_PROPERTIES Message { | ||
| Message Length (i) | ||
| Encoding (i) | ||
| Version (i) | ||
| Property List (..) |
There was a problem hiding this comment.
This replaces the TRACK_PROPERTIES Encoding field with Version and removes each object's Encoding field, but the draft changelog still contains only “Initial version.” Add a bullet under the in-progress version so revision readers can identify this wire-incompatible semantic change.
AGENTS.md reference: drafts/AGENTS.md:L18-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Declining: draft-lcurley-moq-ietf has never been published (no -00 on the datatracker), so the in-progress -00 is the initial version and "Initial version." already covers it. A changelog bullet would describe a change from a revision no reader has seen.
(Written by Claude Opus 5.5)
# Conflicts: # quest/m1/ietf-extension/README.md
|
@codex review Merged origin/main again. Only the quest tree conflicted: main added (Written by Claude Opus 5.5) |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Merge summary for head
(Written by Claude Opus 5.5) |
Adapt fills to main's error-carrying drops (#6143): a fill's local drop records its error, a cursor's given-up misses are named NOT_FOUND without overwriting a known reason, and a held legacy fill rejects the group it waited on with the error it was given up with. FetchServeRun takes the track info (#6107), and the C dynamic handler keeps main's finished broadcasts (#6157). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 participant
Summary
Implements quest/m1/ietf-extension/lite, as decided in moq-dev/moq#6072. A track that crosses a lite-07 hop with the
ietfextension now carries the moq-transport version it was published on once, in TRACK_PROPERTIES. Before this, every Property Block and TRACK_PROPERTIES carried its own Encoding code. A track published over moq-lite carries no version, and a peer cannot attach moq-transport state to it. Behavior across the hop is otherwise the same as in moq-dev/moq#6058.object::Encodingis deleted.object::Blockis{ bytes, immutable }.object::TrackPropertiesis{ version: ietf::Version, values }.track::Info::propertiesis nowSomefor every track a moq-transport publisher published, even one with no properties, andNonefor a moq-lite track. New crate-private accessors aretrack::Info::ietf()andgroup::Consumer::ietf().ietf::object::write(properties, source, ...). It applies the same single-relay rules from that version as before:ietf::object::{same_bytes, block, check, values}andWire::definedtake an exactietf::Versioninstead of an Encoding family.Version::ALL,drafts(),family()anddelivery_timeout()are deleted.ietf::Properties::unknownis now a plainVec<Property>in the message's own draft.accepted_info/refusedtake the session version.lite::Framing { timescale, ietf }, derived from the acceptedtrack::Info, replaces thetimescalethat was threaded throughTrackEntry,FrameIngest,ServeLoop,FetchServeRunandbegin_subscription/prepare_establish. The redundanttimescaleparameters are removed.FetchServeRunkeeps the whole track info, so a fetched group inherits the version too.Public API
None. Every changed item is crate-private (
pub(crate), with// private: passthroughmarkers).Wire
lite-07-wip only, and only when
ietfis negotiated. Not published.Version (i)followed by the Property List.0xff000012for draft 18, or 0 for a moq-lite track.fits()counts the Version's varint size.drafts/draft-lcurley-moq-ietf.mdis updated, andjust drafts checkpasses.Tests
ietf::subscriber::passthrough_keeps_0x06_from_crossing_draft_19: a draft-18 publisher's mutable 0x06 is dropped and a sealed one refused at draft 19, both directly and through a lite-07 hop. The harness requires identical bytes on both paths.ietf::publisher::track_property_0x06_does_not_cross_draft_19: the same check for track properties, directly and across the hop.plain_media_crosses_unchanged: a lite-published track crosses with no version.track_properties_refuse_what_ingress_would_not_keep)ietf_records_need_a_moq_transport_track)datagram_state_needs_a_moq_transport_track)Checks
On head
68a9bb96fd:just check: passed (7812 Rust tests, clippy, rustfmt, no-default-features moq-net, docs).68a9bb96fdfixed what the unverified WIP commit left: rustfmt diffs, andgroup::Consumer::ietfflagged as dead code without theasyncfeature.just checkrun hitexport::redundant_importers_switch_through_bufferbloat_on_lite_07, a known flake on main tracked byquest/m1/test-flakes-2/redundant-failover-drift.mdwith a fix in moq-dev/moq#6113. It does not touch theietfextension; the next run passed.just test interop --all: passed.quest checkandjust drafts check: passed.Decisions
Settled with the recommended option, and confirmed by the maintainer.
crate::Version::code(). Self-describing, and the cost is paid once per track.[L]: it needs a pinned moxygen build, a run per shared draft, a two-relay cluster, and possibly a routing decision for the 16-field namespace.[M].Follow-ups
🤖 Generated with Claude Code
(Written by Claude Opus 5.5)