Skip to content

feat(tokio): accept_with and connect_with hand an established session to moq-net's policy - #8085

Merged
kixelated merged 4 commits into
mainfrom
quest/m1/session-policy/tokio-handoff
Nov 7, 2026
Merged

kixelated merged 4 commits into
mainfrom
quest/m1/session-policy/tokio-handoff

Conversation

@kixelated

@kixelated kixelated commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Completes moq-tokio becomes a thin shell, part of the session policy epic. Its prerequisites moq-dev/moq#8003 (accept policy) and moq-dev/moq#7904 (connect policy as moq_net::Reconnect) already moved the policy itself into moq-net and deleted Deadline and friends. This PR adds the handoffs and removes the per-transport duplication that was left:

  • Listener::accept_with(transport, session, link) reads SETUP on a qmux session established somewhere else and returns the usual Request. moq-net's accept deadline covers SETUP, admission and the reply, counted from the call.
  • Client::connect_with(session) runs moq_net::Client::connect over an established qmux session, bounded by the connect timeout. A handed-over transport can't be redialed, so there's no Reconnect.
  • WebRTC goes through them. accept_webrtc and connect_webrtc now only bring up ICE, the channel and qmux (bounded by the listen or connect timeout), then hand the session off. That lets the WebRTC rename move them into moq-webrtc without touching moq-tokio's internals.
  • One Request constructor. Five hand-built Request { .. } literals (QUIC/WebTransport, WebSocket, the stream listeners, WebRTC, and a test) are now one private Request::accept over a transport's Incoming facts. The two cfg-gated quic/qmux fields became one Under enum naming the connection ok() widens.
  • Quest: deletes tokio-handoff.md. It updates the epic's decision line, and adds a note to the WebRTC rename that moq-webrtc needs its own establishment bound once it owns ICE.

Impact

Public API

  • moq-tokio (additive): Listener::accept_with and Client::connect_with, available with websocket, tcp, or webrtc. No public item was removed or changed: the internals this would have dropped (Deadline, qmux_limits, accept_request) were already removed by feat(net)!: accept policy lives in moq-net #8003, and Backoff is now only the CLI knobs that resolve into moq_net::reconnect::Backoff.
  • moq-net: unchanged, so the sibling io_uring quest builds on the same API.
  • Behavior (WebRTC only, webrtc is #[doc(hidden)]): the MoQ handshake's deadline now starts at the handoff, not at the accept_webrtc / connect_webrtc call. Each phase is still bounded by the same timeout, so the worst case goes from one timeout to two.
  • Wire: none.

Migration

Nothing to migrate: the change is additive, and the WebRTC entry points keep their signatures. For the release notes: "moq-tokio: Listener::accept_with and Client::connect_with run a MoQ session over a qmux session established outside moq-tokio, such as a WebRTC data channel."

Choices made without asking

  • The handoffs take a qmux::Session, not any moq_net transport. The one consumer, WebRTC, is qmux. Taking qmux keeps Request::with_quic_limits working in full: it applies stream counts through qmux's own set_limits, and moq-net's transport::Limits only carries the receive window. Making it generic would mean either silently ignoring those limits or widening moq-net's Limits, and the coordinator asked to leave moq-net's API alone. qmux::Session is already part of moq-tokio's public API (tcp::Listener::accept, websocket::Listener::accept).
  • accept_with takes the Transport kind as well as the Link, so Request::transport() reports it. That's three arguments instead of the quest's sketch of (transport, link).
  • The handoff's deadline counts from the call. The other option was a public deadline accessor plus a deadline argument or builder, so one deadline could also cover a caller's own establishment. That's two more pieces of API with no consumer once moq-webrtc owns ICE; the rename quest now records that moq-webrtc bounds its own establishment.
  • accept_webrtc holds a drain guard while establishing and drops it only after accept_with has taken its own. A drain therefore waits for a peer that is still coming up and still admits it (Codex round 1).

Tests

  • New: webrtc::tests::a_drain_waits_for_an_establishing_peer. It fails with the establishing guard removed and passes with it.
  • webrtc::tests::transfer also checks that the handed-off request reports the channel's ALPN.
  • The existing WebRTC, SETUP-deadline, reply-deadline and receive-window tests pass on the shared constructor.

Validation

  • quest gate nix develop --command just check origin/main passed on 2785505a06, with every module passing. The earlier run on 85d7543 ran 5,890 Rust tests, all passing.
  • cargo clippy -p moq-tokio --no-default-features --features {quic,websocket,tcp,webrtc,uds} --lib -D warnings is clean. Under --profile test, the single-feature builds still warn about dead test helpers in connection.rs and worker/group.rs, which this PR doesn't touch. Those warnings are also on main, and the integration tests already needed websocket there.
  • Browser checks left to CI: dispatch.sh check --query selected only the Rust lanes, so no Playwright or browser lane ran locally, and none was started.

Review

Codex, gpt-6.1-sol, medium effort (quest review --base origin/main), session 01a1b305-640b-73b0-95f5-b41cde8904b6:

  • Round 2 on 2785505a06: no findings.
  • Round 1 on 85d7543cd5 found one issue, now fixed: a WebRTC accept dropped out of the drain while it was establishing. accept_webrtc now holds a guard until accept_with takes over, and there's a regression test.

Suggested follow-ups

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

kixelated and others added 4 commits November 6, 2026 13:30
…ux session to moq-net's policy

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title moq-tokio becomes a thin shell feat(tokio): accept_with and connect_with hand an established session to moq-net's policy Nov 6, 2026
@kixelated
kixelated marked this pull request as ready for review November 6, 2026 22:54
@coderabbitai

coderabbitai Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

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 1 minute.

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: 12719f4d-27eb-4b1c-866a-87b2a07a366b
Commits

Reviewing files that changed from the base of the PR and between f2f9bc9 and 2785505.

Files selected for processing: 7

  • quest/m1/session-policy/README.md
  • quest/m1/session-policy/tokio-handoff.md
  • quest/m2/webrtc-rename.md
  • rs/moq-tokio/src/client.rs
  • rs/moq-tokio/src/server.rs
  • rs/moq-tokio/src/tcp.rs
  • rs/moq-tokio/src/webrtc/tests.rs
  • 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

@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 2785505.

No actionable findings in the changed code. I traced the new accept/connect handoffs, the shared Request constructor and limit application, and the WebRTC establishment guard against the surrounding moq-net activity/deadline logic, qmux session lifetime, and runtime clock implementation. The two-phase WebRTC timeout behavior is documented, and the establishing guard remains held until the handoff has acquired its own activity guard.

Direction: no code changes requested from this static review; await CI before treating validation as complete.

Verification limits: GitHub-only source review of all seven changed files and relevant surrounding code. I did not run builds, tests, quest commands, or browser checks. The Check workflow for this SHA was still in progress when inspected.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ The handoffs themselves check out; one gap worth recording for the WebRTC rename.

Reviewed changes

  • Listener::accept_with / Client::connect_with — new handoffs that read SETUP, or run the connect policy, over an established qmux session, each bounding only its own half.
  • WebRTC through them — accept_webrtc now holds an Activity guard across ICE and channel setup before handing off, and connect_webrtc establishes then delegates. Each phase keeps a full timeout, so the worst case doubles (documented).
  • One Request constructor — five hand-built literals collapse into Request::accept / accept_qmux over Incoming, with the cfg-gated quic/qmux fields replaced by Under.
  • Quest bookkeeping — tokio-handoff.md deleted with its epic ## Required line, and the WebRTC rename gains a note about owning its establishment bound.

The refactor is faithful to the old paths (guard ordering, url/authority/identity plumbing, Under dispatch in quic()/qmux_window()/ok()), and the new drain test genuinely pins the invariant: _establishing is bound from accept_webrtc's first poll and the handoff into accept_with has no suspension point where the count could reach zero.

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

Comment thread quest/m2/webrtc-rename.md

- Move `rs/moq-rtc` to `rs/moq-webrtc` and merge `rs/moq-ice` in as a module. Keep the gateway's `server` feature; add a feature for the data-channel transport so WHIP/WHEP-only builds skip SCTP.
- Move moq-tokio's `webrtc` module (`rs/moq-tokio/src/webrtc.rs`, host gathering, `.local` resolution) into moq-webrtc, built on moq-tokio's public session types. Its connect and accept go through moq-tokio's `connect_with`/`accept_with` handoffs, which run moq-net's session policy over the established data channel.
- Move moq-tokio's `webrtc` module (`rs/moq-tokio/src/webrtc.rs`, host gathering, `.local` resolution) into moq-webrtc, built on moq-tokio's public session types. Its connect and accept go through moq-tokio's `connect_with`/`accept_with` handoffs, which run moq-net's session policy over the established data channel. The handoffs take the qmux session and bound only the MoQ handshake from the call; bounding ICE and the channel coming up (moq-tokio's wrappers use the listen and connect timeouts today) moves with the transport, so moq-webrtc needs its own establishment bound.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The note records the establishment bound that moves out with ICE, but accept_webrtc also does two things before the handoff that a Listener handle cannot express: it holds a moq_net::server::Active guard across establishment so a drain waits for a peer still coming up (the round-1 fix, pinned by a_drain_waits_for_an_establishing_peer), and it refuses a data-channel protocol the listener's Versions does not speak. Listener exposes neither its Activity nor its Versions, and accept_with counts the connection only from the handoff and never checks link.alpn, so the rename either plumbs both in from the app's server::Config/listen::Config or silently loses those guarantees. Recording that here (or folding the checks into accept_with) keeps the follow-up from regressing them.

Technical details
# WebRTC handoff leaves the listener-identity checks behind

## Affected sites
- `quest/m2/webrtc-rename.md:14` — the new note covers only the establishment deadline.
- `rs/moq-tokio/src/server.rs:861` — `Listener::accept_with` enters `Activity` only at the handoff, and copies `link.alpn` through unchanged.
- `rs/moq-tokio/src/server.rs:887` — `_establishing` guard, on the private `Server::activity`.
- `rs/moq-tokio/src/server.rs:888` — `versions.alpns()` gate, on the private `Server::versions`.

## Required outcome
- Either the rename quest records that moq-webrtc also needs the listener's `Activity`
  (to hold `Active` across ICE/channel setup) and its `Versions` (to refuse a non-MoQ
  data-channel protocol), or `accept_with` owns both checks so a consumer cannot drop them.

## Suggested approach (optional)
- Add `Listener::activity()` / `Listener::versions()` accessors, or have `accept_with`
  validate `link.alpn` against the server's versions and accept a pre-acquired `Active`
  for the establishment window.
- The same gap applies to the `connect_with` side: `Client::versions` and `Client::policy`
  are `pub(crate)`, so a crate handed a `Client` cannot re-run `connect_webrtc`'s ALPN gate.

@kixelated
kixelated added this pull request to the merge queue Nov 6, 2026
Merged via the queue into main with commit a0138fa Nov 7, 2026
10 checks passed
@kixelated
kixelated deleted the quest/m1/session-policy/tokio-handoff branch November 7, 2026 02: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