[codex] expose moq-rtc session runner

[kixelated]

Summary

• Change moq_rtc::server::Response into an owned session runner with Response::run().
• Keep WHIP/WHEP accept functions focused on SDP negotiation and resource registration, while bundled routers explicitly spawn the returned runner after preparing the HTTP answer.
• Move session unregister and DELETE cancellation handling into the runner so embedders can await the exact WebRTC session lifetime.
• Build WHIP/WHEP Location headers from OriginalUri so nested routers preserve their mount prefix.

Root Cause

whip::accept and whep::accept previously spawned Session::run() internally and returned only the SDP response data. The WebRTC task knew when the peer disconnected, but embedders had no handle to await, so external session accounting had to guess.

The bundled handlers also built Location from the wildcard path, which dropped route mount prefixes such as /whip or /whep.

Public API Changes

• moq_rtc::server::Response is no longer Clone.
• moq_rtc::server::Response adds public async fn run(self) -> Result<()>.
• whip::accept and whep::accept now return a Response that must be run by the caller to drive the negotiated media session.

This is a breaking API change in rs/moq-rtc, but moq-rtc is currently version 0.0.1 and not listed in the branch-targeting breaking-change set.

Validation

• cargo test -p moq-rtc
• cargo check -p moq-rtc
• git diff --check

(Written by Claude)

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In AcceptedSession::run, the early Ok(()) returns when any of session, registration, or cancel is None will silently hide logic bugs; consider treating missing state as an internal error (or at least logging/warning) so accidental double-runs or construction issues surface instead of being silently ignored.
• In the WHIP/WHEP HTTP handlers, cloning resource_id and answer from Response just to move the original into the spawned task adds extra allocations; you could instead restructure the API (e.g., a method that consumes Response and returns (resource_id, answer, runner) or splits out a SessionRunner type) to avoid these clones while still allowing the session to be driven asynchronously.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `AcceptedSession::run`, the early `Ok(())` returns when any of `session`, `registration`, or `cancel` is `None` will silently hide logic bugs; consider treating missing state as an internal error (or at least logging/warning) so accidental double-runs or construction issues surface instead of being silently ignored.
- In the WHIP/WHEP HTTP handlers, cloning `resource_id` and `answer` from `Response` just to move the original into the spawned task adds extra allocations; you could instead restructure the API (e.g., a method that consumes `Response` and returns `(resource_id, answer, runner)` or splits out a `SessionRunner` type) to avoid these clones while still allowing the session to be driven asynchronously.

## Individual Comments

### Comment 1
<location path="rs/moq-rtc/src/server/mod.rs" line_range="87-94" />
<code_context>
+}
+
+impl AcceptedSession {
+	async fn run(mut self) -> Result<()> {
+		let Some(session) = self.session.take() else {
+			return Ok(());
+		};
+		let Some(registration) = self.registration.take() else {
+			return Ok(());
+		};
+		let Some(cancel) = self.cancel.take() else {
+			return Ok(());
+		};
</code_context>
<issue_to_address>
**issue (bug_risk):** Silently returning Ok(()) when session, registration, or cancel are None can hide broken invariants.

Given these fields should always be `Some` after construction, hitting any of these `None` branches indicates a logic error elsewhere. Instead of silently returning, please at least log a debug message or use `expect` with a clear message so unexpected states are surfaced and session lifecycle issues are easier to diagnose.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[coderabbitai]

Walkthrough

The PR changes the session lifecycle so Response objects are run explicitly instead of being started inside accept(). WHIP and WHEP handlers now spawn session.run(), while accept() returns a Response::new(...) value carrying the negotiated session state. Session-end logging now borrows the result, and client code and crate docs were updated to match the new flow.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: exposing the moq-rtc session runner.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
Description check	✅ Passed	The description matches the code changes: owned session runner, accept flow refactor, cancellation cleanup, and mount-preserving Location headers.

✨ Finishing Touches

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch codex/fix-moqrtc-session-end-status

---

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.}

[coderabbitai]

Actionable comments posted: 2

🤖 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-rtc/src/server/whep.rs`:
- Around line 27-41: The WHEP handler builds the Location header from the
extracted `path`, which loses the mounted `/whep` prefix when the router is
nested. Update `handle` in `whep.rs` to derive the session URI from the original
request target instead of `path.0`, using `OriginalUri` or another preserved
full request URI source. Keep the existing `accept_offer` flow and
`response.resource_id` usage, but construct the Location value from the actual
mounted request path so it points to the correct session resource.

In `@rs/moq-rtc/src/server/whip.rs`:
- Around line 35-46: The Location header is being built from the wildcard `path`
in `whip.rs`, which loses the router mount prefix when the handler is mounted
under a route like `/whip`. Update the response construction in `whip::handle`
to derive the Location value from the original request URI path instead of
`path`, and apply the same change in the matching WHEP handler so both return
the full mounted path.

🪄 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: fdf2d00b-6e4d-413b-badf-5b50d3250ba8

📥 Commits

Reviewing files that changed from the base of the PR and between 3f355a1 and 9fe9e95.

📒 Files selected for processing (7)

• rs/moq-rtc/src/client/whep.rs
• rs/moq-rtc/src/client/whip.rs
• rs/moq-rtc/src/lib.rs
• rs/moq-rtc/src/server/mod.rs
• rs/moq-rtc/src/server/whep.rs
• rs/moq-rtc/src/server/whip.rs
• rs/moq-rtc/src/session.rs