refactor(net)!: return session drivers to callers

[kixelated]

Problem

The public Runtime trait couples timers, a transport associated type, and spawning a session state machine. Callers need an Inline adapter just to keep the driver in their own task.

Approach

Return (Session, Driver) from the session handshake and let callers poll or spawn the driver. Keep Timers independent of transport and execution. Move spawning into Tokio/browser integrations, remove Inline, and update callers, lifecycle tests, and documentation.

Impact

• Breaking Rust API: remove moq_net::Runtime and runtime::Machine; connect/accept (including lite-only variants) and Handshake::ok return a session and public Driver.
• Timer providers and test clocks no longer carry a transport type. Remove moq_tokio::runtime::Inline and the test runtime's implicit driver queue.
• No production task spawning inside moq-net. Dropping the driver cancels the session; dropping the last session requests closure on the next driver poll.
• No wire changes or changes to the higher-level client/server and foreign binding APIs. Targets main as explicitly requested by the maintainer.

Validation

• just check and just test passed, including 4,205 native tests and 64 Python tests; wasm checks and Tokio feature combinations passed. Swift was skipped because its toolchain is unavailable; macOS/Windows were not checked locally.
• just test smoke-full: 24/32 combinations passed. Native Node/Bun subscribers timed out with every publisher; both cases also reproduce against the unchanged base revision (f31ae9826).

Alternatives

Retaining a spawning runtime or Inline adapter preserves the coupling and makes task ownership harder to follow. Returning the driver makes ownership explicit with fewer abstractions.

Follow-ups

Investigate simplifying timers and clock APIs separately. Native Node/Bun smoke timeouts predate this refactor.

(written by GPT-6)

[moq-bot]

No issues found.
Verdict: approve
(Written by Muse Spark)

opencode session  |  github run

[coderabbitai]

Warning

Review limit reached

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 725bb78b-5ffe-44ac-951a-210382072f05

📥 Commits

Reviewing files that changed from the base of the PR and between 6015999 and 2ed147e.

📒 Files selected for processing (53)

• doc/lib/rs/moq-net.md
• rs/moq-e2ee/tests/support/harness.rs
• rs/moq-e2ee/tests/transport.rs
• rs/moq-ffi/src/origin.rs
• rs/moq-ffi/src/runtime.rs
• rs/moq-ffi/src/session.rs
• rs/moq-hls/src/export/mod.rs
• rs/moq-hls/src/export/upstream.rs
• rs/moq-hls/src/server/mod.rs
• rs/moq-mux/src/source.rs
• rs/moq-net/src/client.rs
• rs/moq-net/src/driver.rs
• rs/moq-net/src/goaway.rs
• rs/moq-net/src/ietf/control.rs
• rs/moq-net/src/ietf/publisher.rs
• rs/moq-net/src/ietf/session.rs
• rs/moq-net/src/ietf/subscriber.rs
• rs/moq-net/src/lib.rs
• rs/moq-net/src/lite/publisher.rs
• rs/moq-net/src/lite/session.rs
• rs/moq-net/src/model/origin.rs
• rs/moq-net/src/runtime.rs
• rs/moq-net/src/runtime/test.rs
• rs/moq-net/src/server.rs
• rs/moq-net/src/session.rs
• rs/moq-net/tests/datagram.rs
• rs/moq-net/tests/goaway.rs
• rs/moq-net/tests/support/harness.rs
• rs/moq-relay/src/uring.rs
• rs/moq-relay/src/websocket.rs
• rs/moq-rtc/src/egress.rs
• rs/moq-rtc/src/server/mod.rs
• rs/moq-srt/src/dial.rs
• rs/moq-srt/src/server.rs
• rs/moq-srt/src/ts.rs
• rs/moq-stats/src/aggregate.rs
• rs/moq-stats/src/consume.rs
• rs/moq-stats/src/produce.rs
• rs/moq-tokio/src/client.rs
• rs/moq-tokio/src/origin.rs
• rs/moq-tokio/src/runtime.rs
• rs/moq-tokio/src/server.rs
• rs/moq-uring/README.md
• rs/moq-uring/benches/session_lite.rs
• rs/moq-uring/src/lib.rs
• rs/moq-uring/src/quic/web.rs
• rs/moq-uring/src/worker.rs
• rs/moq-uring/tests/session.rs
• rs/moq-uring/tests/web.rs
• rs/moq-video/src/decode/consumer.rs
• rs/moq-wasm/README.md
• rs/moq-wasm/src/lib.rs
• rs/moq-wasm/src/runtime.rs

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: d4956280-ab53-4d68-9fa2-7d4f01329a0a

📥 Commits

Reviewing files that changed from the base of the PR and between 990a96a and 6015999.

📒 Files selected for processing (53)

• doc/lib/rs/moq-net.md
• rs/moq-e2ee/tests/support/harness.rs
• rs/moq-e2ee/tests/transport.rs
• rs/moq-ffi/src/origin.rs
• rs/moq-ffi/src/runtime.rs
• rs/moq-ffi/src/session.rs
• rs/moq-hls/src/export/mod.rs
• rs/moq-hls/src/export/upstream.rs
• rs/moq-hls/src/server/mod.rs
• rs/moq-mux/src/source.rs
• rs/moq-net/src/client.rs
• rs/moq-net/src/driver.rs
• rs/moq-net/src/goaway.rs
• rs/moq-net/src/ietf/control.rs
• rs/moq-net/src/ietf/publisher.rs
• rs/moq-net/src/ietf/session.rs
• rs/moq-net/src/ietf/subscriber.rs
• rs/moq-net/src/lib.rs
• rs/moq-net/src/lite/publisher.rs
• rs/moq-net/src/lite/session.rs
• rs/moq-net/src/model/origin.rs
• rs/moq-net/src/runtime.rs
• rs/moq-net/src/runtime/test.rs
• rs/moq-net/src/server.rs
• rs/moq-net/src/session.rs
• rs/moq-net/tests/datagram.rs
• rs/moq-net/tests/goaway.rs
• rs/moq-net/tests/support/harness.rs
• rs/moq-relay/src/uring.rs
• rs/moq-relay/src/websocket.rs
• rs/moq-rtc/src/egress.rs
• rs/moq-rtc/src/server/mod.rs
• rs/moq-srt/src/dial.rs
• rs/moq-srt/src/server.rs
• rs/moq-srt/src/ts.rs
• rs/moq-stats/src/aggregate.rs
• rs/moq-stats/src/consume.rs
• rs/moq-stats/src/produce.rs
• rs/moq-tokio/src/client.rs
• rs/moq-tokio/src/origin.rs
• rs/moq-tokio/src/runtime.rs
• rs/moq-tokio/src/server.rs
• rs/moq-uring/README.md
• rs/moq-uring/benches/session_lite.rs
• rs/moq-uring/src/lib.rs
• rs/moq-uring/src/quic/web.rs
• rs/moq-uring/src/worker.rs
• rs/moq-uring/tests/session.rs
• rs/moq-uring/tests/web.rs
• rs/moq-video/src/decode/consumer.rs
• rs/moq-wasm/README.md
• rs/moq-wasm/src/lib.rs
• rs/moq-wasm/src/runtime.rs

💤 Files with no reviewable changes (1)

• rs/moq-uring/src/worker.rs

Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review.

---

Walkthrough

The MoQ session lifecycle now uses a caller-driven Driver. Client and server APIs return (Session, Driver) pairs and accept timer-only runtimes with standalone transport types. Runtime implementations no longer spawn protocol machines. Tokio, wasm, FFI, relay, uring, tests, benchmarks, and documentation now explicitly poll or spawn drivers. Runtime construction no longer uses transport type parameters.

Priority: ➖ Normal

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 59.54% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 131 functions across 49 files. (3 skipped…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely identifies the primary change: returning session drivers to callers. It matches the PR changes and uses an appropriate breaking-change marker.
Description check	✅ Passed	The description directly explains the problem, approach, API impact, lifecycle behavior, validation results, and known validation limits. It is fully related to the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 59.54% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 131 functions across 49 files. (3 skipped: 3 unsupported.)

✨ Finishing Touches

✨ Simplify code

• Commit to this branch
• Create a new PR

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[moq-bot]

1. Breaking refactor(net)! targets main while moq-net is published at 0.2.22 (rs/moq-net/Cargo.toml:8, AGENTS.md#development). Retarget the PR to dev, set upstream to origin/dev, and rebase.

Verdict: request changes
(Written by Muse Spark)

opencode session  |  github run

[kixelated]

Code review: refactor(net)!: return session drivers to callers

Recommendation: MERGE

Criteria

1. Positive improvement? Yes. Decoupling timers from transport and execution, then returning (Session, Driver) from connect/accept/Handshake::ok, makes ownership and progress explicit. The old Runtime trait forced every dial path to name a transport on the timer handle and to either spawn or invent adapters (Inline) just to keep the machine in the caller's task.

2. Worth the complexity? Yes — and the net complexity goes down. Roughly −400 lines overall: Runtime / Machine / Inline / the test machine queue / PhantomData transport parameters go away; Timers stays as the only runtime dependency. That matches the existing origin (Producer, Driver) pattern and removes a whole class of “who is polling this?” indirection.

3. Better approach? Not really. Keeping a spawning Runtime or an Inline slot preserves the coupling and the surprising take() / panic-on-second-session API. Returning the driver is the honest interface; integrations (moq-tokio, moq-wasm, moq-ffi, relay, uring) can still spawn for convenience without baking spawn into moq-net.

What looks solid

• Ownership clarity: Driver is #[must_use], holds no Session clone, implements both Future and poll(&Waiter), and documents drop/cancel semantics clearly.
• Removing Inline: The relay WebSocket path now accepts with plain Timers and awaits the driver in the handler task. Same lifetime/teardown intent, without a mutex slot and a panic if you reuse the handle.
• Caller migration in-tree: Tokio connect_session / Request::ok, wasm/ffi microtask spawn, relay uring handle.spawn, and test harnesses all spawn or poll the returned driver. Lite !Send paths still work by letting the caller poll on the owning thread.
• Tests: Lifecycle coverage was rewritten around explicit driver.poll (last-handle close, abort, dropped driver → Error::Cancel, lite !Send). That is clearer than runtime.tick() / shutdown().
• Validation: Reported just check / just test green; smoke timeouts called out as pre-existing on base — good to record, not a regression signal for this change.

Risks / watch items (not merge blockers)

• Drop semantics for direct moq-net callers: Forgetting to poll/spawn the driver stalls the session; dropping the driver cancels with Error::Cancel; last Session drop only closes on the next driver poll. Docs + #[must_use] mitigate this; still the main footgun for anyone not going through moq-tokio/moq-wasm.
• Breaking surface: Public removal of moq_net::Runtime / runtime::Machine and the connect/accept return-type change. In-repo migration looks complete; external consumers will need the same (session, driver) + spawn/poll step.
• Minor doc leftover: Session::abort still says the close lands “once the runtime polls it” — should say driver for consistency. Harmless.

Verdict

This is the right shape for session execution: timers independent, driver caller-owned, spawning pushed to integrations. Complexity decreases and ownership gets clearer. MERGE.

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Not landing: this is superseded by #3825, which merged onto main at the same time this PR was closed.

• main already returns caller-owned (Session, Driver) from connect/accept with no Runtime/Machine/Inline, matching this PR's intent.
• refactor(net)!: drive time and cache cleanup explicitly #3825 goes further: Driver takes an explicit Clock/Instant instead of this PR's Driver<S, R: Timers> plus runtime argument, with additional time and cache-cleanup changes (68 files vs 53 here).
• A rebase of this branch onto current origin/main hits ~50 conflicting files and would regress those follow-ons.

Closing as superseded; no action needed on this branch.

(Written by Muse Spark)