Skip to content

feat(p2p): carry MoQ over reliable data channels - #5595

Merged
kixelated merged 3 commits into
mainfrom
quest/m1/p2p/transport
Oct 16, 2026
Merged

kixelated merged 3 commits into
mainfrom
quest/m1/p2p/transport

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Problem

Browser applications cannot supply an RTCDataChannel to the existing MoQ connection API. This adds @moq/p2p, which carries the existing MoQ streams over a reliable ordered channel using @moq/qmux.

Approach

Transport.connect({peer, protocol}) creates the client channel; Transport.accept({peer, channel, protocol}) adopts the server channel. Both own their channel, while the application retains the peer connection and signaling. The adapter waits for SCTP negotiation, caps records at 16 KiB and the negotiated message size, bounds its receive queue, and applies event-driven send backpressure. It refuses partial reliability, unordered delivery, incompatible protocol labels, malformed messages, and limits below qmux's minimum.

A real Chromium test completes MoQ SETUP and transfers a 256 KiB frame through the public connection API. Unit tests cover both stream-ID roles, record limits including unbounded SCTP negotiation, close information, listener cleanup, backpressure, malformed input, and cancellation before channel opening. CI runs the browser test; nightly runs a benchmark swept across 1/8/64 streams and 1/16/256 KiB payloads.

Public API and wire impact

  • New @moq/p2p exports: Transport, ConnectProps, and AcceptProps. Its WebTransport-shaped handle supports streams and reports a zero datagram limit.
  • Connection transport labels add webrtc; the web demo displays that label.
  • The Lite 07 work-in-progress draft specifies the new reliable ordered RTCDataChannel binding: qmux draft-01 Frames payloads, one per binary message, without the byte-stream Record Size field. Existing published transport bindings are unchanged.
  • No signaling, ICE policy, peer admission, native WebRTC integration, or package release is added.

Alternatives

A separate dialer would duplicate MoQ negotiation; callers instead supply this transport to the existing connection factories. Owning the peer connection would prevent an application from sharing its connection with other channels, so ownership remains with the caller as agreed.

Validation

  • just check passed, including affected JS packages, package build/publint, drafts, CI configuration tests, and selected Rust checks.
  • just test interop --all passed the full available language matrix and close/tail checks.
  • Final focused adapter typecheck and seven unit tests passed, including two regressions reproduced before their fixes: positive-Infinity SCTP limits and early stream cancellation.
  • just test p2p passed in Chromium, including cleanup. One overlapping run passed its media assertions but exceeded Bun's default five-second browser teardown hook while interop builds were active; an isolated final run passed without changing timeouts.
  • just js p2p-bench passed all nine stream/payload combinations. These are coverage measurements on a shared host, not an optimization comparison.
  • just drafts check, quest check, and git diff --check passed.

Follow-ups

Existing P2P signaling and native WebRTC quests provide the next integration layers. No additional public API decisions remain for this adapter. The completed transport quest and its references are removed.

(Written by GPT-6)

kixelated and others added 2 commits October 15, 2026 10:53
Co-Authored-By: GPT-6 <noreply@openai.com>
Co-Authored-By: GPT-6 <noreply@openai.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Outcome: the browser data-channel adapter is implemented, documented, and validated. It owns only its reliable ordered channel, preserves caller ownership of RTCPeerConnection, and carries MoQ through qmux with bounded records and event-driven backpressure. Required checks, the full available language interop matrix, seven adapter unit tests, real Chromium SETUP/media transfer, and the nine-case benchmark sweep pass.

No unresolved API decisions remain. Keep this PR as a draft for review, then use /quest-merge once review and CI permit. Existing signaling and native WebRTC quests remain the recommended follow-ups; no new quest is needed for this scope.

(Written by GPT-6)

@kixelated
kixelated marked this pull request as ready for review October 15, 2026 23:37
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 15, 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-10-15T23:42:18.814017Z 8ae9b2b Draft marked ready
ℹ️ 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.

@coderabbitai

coderabbitai Bot commented Oct 15, 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 25 minutes.

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
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 66fc8443-585b-4466-b9fe-66ad63d1ba71
📥 Commits

Reviewing files that changed from the base of the PR and between 68c59cb and 8ae9b2b.

⛔ Files ignored due to path filters: 1
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing: 32
  • .github/workflows/interop.yml
  • demo/web/src/index.ts
  • demo/web/src/publish.ts
  • doc/concept/transport.md
  • drafts/draft-lcurley-moq-lite.md
  • js/justfile
  • js/net/src/connection/goaway.ts
  • js/net/src/connection/transport.ts
  • js/net/src/index.ts
  • js/p2p/README.md
  • js/p2p/bench/transport.ts
  • js/p2p/bench/tsconfig.json
  • js/p2p/package.json
  • js/p2p/src/channel.ts
  • js/p2p/src/index.ts
  • js/p2p/src/mock.ts
  • js/p2p/src/transport.test.ts
  • js/p2p/src/transport.ts
  • js/p2p/tsconfig.build.json
  • js/p2p/tsconfig.json
  • package.json
  • quest/m1/p2p/README.md
  • quest/m1/p2p/signal.md
  • quest/m1/p2p/transport.md
  • quest/m1/p2p/webrtc.md
  • sh/gh/nightly.sh
  • test/justfile
  • test/p2p/index.html
  • test/p2p/package.json
  • test/p2p/src/main.ts
  • test/p2p/transport.browser.ts
  • test/p2p/tsconfig.json
  • 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

Comment @coderabbitai help to get the list of available commands.

@kixelated

Copy link
Copy Markdown
Collaborator Author

Merge summary:

  • Merged main into the branch. The only conflict was js/justfile, where main added audio-warmup-bench next to this PR's p2p-bench; both recipes are kept.
  • The earlier red Interop was not caused by this PR. Its full-matrix step failed with "relay connection N (subscriber) idled out instead of closing" on WebSocket connections only. That happened because, on the PR's old base, the relay logged WebSocket closes under the moq_relay::websocket target, which the harness's close accounting did not see. fix(relay): log every connection's close from Connection::run #5602 (now on main) fixed it by logging every close from Connection::run. The PR only appeared in Interop at all because it adds js/p2p/** and test/p2p/** to the workflow paths. After the merge, Interop passes, including the new P2P data-channel step.
  • just check passes locally, and all CI is green on 8ae9b2b.
  • Codex reviewed 8ae9b2b with no findings. I also checked the bounded 64 KiB receive queue in Channel: qmux's read loop handles each record synchronously, so a slow stream consumer cannot back up that queue and kill the connection.

No open decisions. Enabling auto-merge.

(Written by Opus 5.5)

@kixelated
kixelated added this pull request to the merge queue Oct 16, 2026
Merged via the queue into main with commit a64e6f3 Oct 16, 2026
13 checks passed
@kixelated
kixelated deleted the quest/m1/p2p/transport branch October 16, 2026 00:34
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