moq-lite-05: SETUP message + negotiated Probe capability levels#26
Conversation
Re-introduce a SETUP message for per-hop capability negotiation, sent once on a new unidirectional Setup Stream (0x1) and FIN'd immediately. Endpoints keep exchanging non-Setup streams without waiting for SETUP, buffering only a stream whose encoding a negotiated extension would change; unknown stream types are still reset as a fallback. This replaces stream-probing for negotiation and better matches upstream moq-transport (version is still negotiated via ALPN, not SETUP). Add a SETUP `Probe` parameter advertising the publisher's capability level: None, Report (measure and report the estimated bitrate), or Increase (additionally pad to probe for bandwidth above the current sending rate). The levels are nested rather than independent, since probing for more bandwidth is meaningless without measuring it; a subscriber must not rely on a level the publisher did not advertise. moq-probe: mirror the same Report/Increase level negotiation and assign real, high, collision-resistant codepoints in place of TBD placeholders, following draft-ietf-moq-transport-18 conventions. moqt-18 removed bidirectional stream types in favor of negotiated request message types, so PROBE is now a PROBE_REQUEST (Request, First) / PROBE_RESPONSE pair of Message Types rather than a stream type. The Setup Option and PROBE_REQUEST share 0x950BE (independent registries); PROBE_RESPONSE is 0x950BF. Also refresh stale moqt section cross-references. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Complex PR? Review this PR in Change Stack to move by importance, not file order. Warning Review limit reached
More reviews will be available in 54 minutes and 23 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis pull request updates two standards drafts to replace stream-probing-based capability negotiation with explicit SETUP messages. In 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@draft-lcurley-moq-lite.md`:
- Around line 93-95: The spec currently conflicts about whether a SETUP is
mandatory; pick a single normative model and make both occurrences consistent:
either (A) require a SETUP at session start by changing the permissive language
in the later paragraph (lines ~438-443) to "MUST" and add text that an
empty/absent-capability list may be represented by an empty SETUP payload, or
(B) make the startup SETUP optional by changing the opening requirement (the
"Each endpoint opens a unidirectional Setup Stream..." sentence) to "MAY" and
explicitly state that when omitted peers MUST assume no optional capabilities;
update both the initial Setup Stream paragraph and the later paragraph about
omitting SETUP so they both use the chosen normative verb and include the single
agreed handling for empty/absent capabilities.
- Line 324: The Probe failure behavior is inconsistent between drafts: update
the text that currently says "If the publisher advertised no Probe capability
... it MUST reset the stream." to match the probe extension draft's normative
action (either change this to require session close with PROTOCOL_VIOLATION or
change the probe draft to require stream reset) so both documents are identical;
specifically, replace the sentence mentioning "reset the stream" with the agreed
outcome and use the exact token "PROTOCOL_VIOLATION" if you choose
session-close, and mirror the same wording in the other occurrence noted near
the later Probe rules so "Probe"/"PROBE"/"PROTOCOL_VIOLATION" semantics are
consistent across both drafts.
In `@draft-lcurley-moq-probe.md`:
- Line 69: The changed markdown uses inconsistent heading and code-block styles
causing MD003/MD040/MD046; convert any setext/underlined headings to ATX-style
(use leading # for headings like "Setup Negotiation" and the other affected
headings), ensure there is a blank line before and after each heading, and
replace indented or tilde-style code blocks with fenced triple-backtick code
blocks (include a language tag where appropriate) with blank lines surrounding
the fence to match the repo lint profile.
🪄 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: 750f35e1-0dc1-4408-9296-cc04e26a8c70
📒 Files selected for processing (2)
draft-lcurley-moq-lite.mddraft-lcurley-moq-probe.md
| In either case the publisher periodically replies with PROBE messages on the same bidirectional stream containing the current estimated bitrate and smoothed RTT. | ||
|
|
||
| If the publisher does not support PROBE (e.g., congestion controller is not exposed), it MUST reset the stream. | ||
| If the publisher advertised no Probe capability (e.g., the congestion controller is not exposed), it MUST reset the stream. |
There was a problem hiding this comment.
Probe failure behavior conflicts with the probe extension draft.
Here unsupported Probe results in stream reset, but the provided draft-lcurley-moq-probe.md snippet requires session close with PROTOCOL_VIOLATION when a subscriber opens PROBE without negotiated support. This cross-draft mismatch will cause interop splits.
Please align the normative outcome between drafts (either reset-stream or session-close) and keep both documents identical on this point.
Also applies to: 586-587
🤖 Prompt for 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.
In `@draft-lcurley-moq-lite.md` at line 324, The Probe failure behavior is
inconsistent between drafts: update the text that currently says "If the
publisher advertised no Probe capability ... it MUST reset the stream." to match
the probe extension draft's normative action (either change this to require
session close with PROTOCOL_VIOLATION or change the probe draft to require
stream reset) so both documents are identical; specifically, replace the
sentence mentioning "reset the stream" with the agreed outcome and use the exact
token "PROTOCOL_VIOLATION" if you choose session-close, and mirror the same
wording in the other occurrence noted near the later Probe rules so
"Probe"/"PROBE"/"PROTOCOL_VIOLATION" semantics are consistent across both
drafts.
There was a problem hiding this comment.
This divergence is intentional: the two drafts target different host protocols with different error models. moq-lite resets unknown/unsupported streams non-fatally (its core STREAM_TYPE rule), so stream reset is the correct, in-philosophy outcome here. moq-probe targets moq-transport, where Section 3.3 mandates closing the session with PROTOCOL_VIOLATION when a bidirectional stream begins with an un-negotiated message type. Making them identical would violate one host spec. Leaving this thread open for a maintainer call.
There was a problem hiding this comment.
@kixelated, thanks for the clear explanation — that makes sense. The error model is inherited from the host protocol in each case, so the divergence is load-bearing, not accidental. I'll note this for future reviews.
✏️ Learnings added
Learnt from: kixelated
Repo: moq-dev/drafts PR: 26
File: draft-lcurley-moq-lite.md:325-325
Timestamp: 2026-06-06T01:48:35.549Z
Learning: In the moq-lite + moq-probe drafts (moq-dev/drafts), the Probe failure behavior intentionally differs between the two documents:
- draft-lcurley-moq-lite.md: when a subscriber opens a Probe Stream without the publisher advertising Probe capability, the publisher MUST reset the stream (non-fatal). This follows moq-lite's core STREAM_TYPE rule where unknown/unsupported streams are reset non-fatally.
- draft-lcurley-moq-probe.md: targets moq-transport, where Section 3.3 mandates closing the session with PROTOCOL_VIOLATION when a bidirectional stream begins with an un-negotiated message type.
Aligning the two would violate one host specification. The divergence is intentional by design.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: kixelated
Repo: moq-dev/drafts PR: 25
File: draft-lcurley-moq-lite.md:0-0
Timestamp: 2026-06-03T03:14:11.765Z
Learning: In draft-lcurley-moq-lite.md (moq-lite protocol spec), the bidirectional stream type IDs are: Announce=0x1, Subscribe=0x2, Fetch=0x3, Probe=0x4, Goaway=0x5, Track=0x6. Subscribe is 0x2 and Fetch is 0x3 (not 0x3 and 0x4 as previously misstated).
Resolve the conflict between the Session section (which read as mandatory) and the Setup Stream section (which said MAY). SETUP is now required in both: each endpoint sends exactly one SETUP, using an empty parameter list when it has no optional capabilities. A guaranteed SETUP bounds the "buffer until the peer's SETUP arrives" rule, which would otherwise deadlock since QUIC offers no "stream will never open" signal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
What
Adds per-hop capability negotiation to moq-lite and reworks the moq-probe extension to negotiate the same Probe capabilities and use real codepoints.
moq-lite-05
0x1), sent once at session start and FIN'd immediately. Carries a genericParameter ID / Length / Valuelist (unknown IDs ignored) — the extension-negotiation mechanism, replacing stream-probing. Version is still negotiated via ALPN, not SETUP.Probesetup parameter advertising the publisher's capability level:0None — no probing1Report — measure & report estimated bitrate2Increase — additionally pad (or send redundant data) to probe above the current sending ratemoq-probe
TBDplaceholders with real codepoints followingdraft-ietf-moq-transport-18. moqt-18 removed bidirectional stream types in favor of negotiated request message types, so PROBE is now aPROBE_REQUEST(Request, First) /PROBE_RESPONSEMessage Type pair instead of a stream type.0x950BEandPROBE_REQUEST0x950BE(independent registries — shared value is unambiguous)PROBE_RESPONSE0x950BF0x7f*N + 0x9D).{{moqt}}section cross-references to -18 numbering (padding7.7→11.5, SETUP9.4→10.3).Why
The publisher's congestion controller knows/limits the send rate; the subscriber wants to learn it (for ABR) and optionally push it higher. These are per-hop, implementation-specific capabilities that must be negotiated so a peer doesn't rely on something the other side doesn't implement — e.g. a publisher that can report bitrate but not actively probe.
Reviewer notes
0x4) and the separate moq-probe extension still overlap; not reconciled in this PR.*.html,*.txt) are untouched per repo conventions.🤖 Generated with Claude Code