moq-net: add Lite05Wip version variant (unadvertised)#1518
Conversation
Introduces a work-in-progress placeholder for moq-lite-05 so future feature work can gate on the version without yet exposing it on the wire. The ALPN string and wire code are reserved, but lite-05 is deliberately omitted from the default ALPNS list and Versions::all(), so servers/clients won't advertise or accept it unless explicitly opted in via with_versions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR adds work-in-progress support for MoQ Lite Draft05 WIP version across JavaScript and Rust implementations. It introduces a new 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rs/moq-net/src/setup.rs`:
- Around line 79-81: In SetupVersion::from_version, replace the explicit match
arm that lists Lite03, Lite04, Lite05Wip with a forward-default arm so future
Lite variants are treated as unsupported; keep the explicit arms for Lite01 and
Lite02 and change the other arm from Version::Lite(lite::Version::Lite03 |
lite::Version::Lite04 | lite::Version::Lite05Wip) => Self::Unsupported to a
catch-all Version::Lite(_) => Self::Unsupported so unrecognized Lite variants
default to Unsupported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 30cb03e3-d8d8-4c29-86f8-402b49e79f05
📒 Files selected for processing (6)
js/net/src/lite/version.tsrs/moq-net/src/client.rsrs/moq-net/src/lite/version.rsrs/moq-net/src/server.rsrs/moq-net/src/setup.rsrs/moq-net/src/version.rs
Per the CLAUDE.md convention, list only the older Lite variants explicitly and default future ones forward with Version::Lite(_) => Self::Unsupported. Lite03+ all share the same property (no legacy SETUP message) and any future variant will too, so the catch-all expresses the intent without forcing edits on every new draft. Addresses CodeRabbit feedback on #1518. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per the review on PR #1513: - moq-net: replace `Versions::qmux_alpns()` / `qmux_alpn_strings()` (the per-instance Vec-returning methods) with module-level consts `QMUX_ALPNS: &[(QmuxVersion, &str)]` and `QMUX_ALPN_STRINGS: &[&str]`. The permutation table is small and baked in; no need to recompute. A unit test pins it against the typed list to catch drift. - moq-net: add `Lite05Wip` arm to `Version::qmux_versions` (pinned to qmux-01, opt-in only — excluded from QMUX_ALPNS just like main keeps it out of ALPNS / Versions::all). This is the merge-from-main fix for the CI break introduced by #1518. - moq-native (client): drop the unused `versions` field and switch the websocket helper to take `&[(QmuxVersion, &str)]` (typed pair list). Recover the qmux version after the handshake by index lookup in the pair list rather than re-parsing the prefix. - moq-relay: bundle handle_socket's seven arguments into a `Handler` struct with a `run()` method. axum filters on `QMUX_ALPN_STRINGS.iter().copied()`; the negotiated subprotocol is also resolved by index lookup so the relay no longer has its own prefix-parsing helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Lite05Wipvariant tolite::Version(Rust) andDRAFT_05_WIPto the JS liteVersionso future feature work can branch on moq-lite-05.0xff0dad05and the ALPN stringmoq-lite-05-wipin both Rust and JS, plus name/FromStr/from_code/from_alpn/alpnround-trips.SetupVersion::from_versionmatch inrs/moq-net/src/setup.rsto putLite05WipinUnsupported(same as Lite03/Lite04: no legacy SETUP message).ALPN_LITE_05_WIParms inrs/moq-net/src/{server,client}.rsso explicit opt-in works end-to-end if someone bumps the ALPN list.Why
We want to start landing moq-lite-05 features (gated
match versionarms in encoders/decoders) without exposing the version on the wire yet. Keeping the variant in the codebase but excluding it fromALPNSandVersions::all()lets that work compile and run in tests without changing default server/client behavior.Not advertised
Deliberately omitted (per the "don't advertise as ALPN" intent):
pub const ALPNSinrs/moq-net/src/version.rs(the QUIC/TLS ALPN list)Versions::all()(the default version set used for SETUP/ALPN negotiation)protocolslist injs/net/src/connection/connect.tsA doc comment on
ALPNScalls out the omission so the next reader doesn't "fix" it.All existing per-version
matcharms inrs/moq-net/src/lite/andjs/net/src/lite/already use_/default:fallthroughs that target the newest behavior, soLite05Wipnaturally inherits Lite04-style probe/goaway/hop chain semantics until divergence starts.Test plan
cargo test -p moq-net— 327 passedcargo test -p moq-native— 51 passed (ALPN integration tests still negotiate the existing versions, no new lite-05 path leaks into defaults)just check— green (rustfmt, clippy-D warnings, rustdoc-D warnings, biome, JS tests)(Written by Claude)