[kixelated]

Problem

When WebSocket wins the https:// transport race, closing the pending QUIC dial keeps the session on TCP for its lifetime and removes QUIC's head start from later connections.

Approach

Keep the pending QUIC dial within the existing connect deadline. Once the replacement receives the peer's SETUP, switch through the existing draining handover, resume tracks, and forget the WebSocket memo. A refused, stalled, closed, or already-draining candidate keeps WebSocket serving without sending it GOAWAY. A WebSocket MoQ handshake failure can use the pending QUIC attempt.

Session::setup() waits for the peer's SETUP. The crate's servers send SETUP after admission, so this gate protects upgrades against rejection by these servers. Neither protocol requires that ordering: another server may send SETUP before admission and refuse afterward. This API and gate do not guarantee admission across all conforming servers.

impl Session {
    /// Wait for the peer's SETUP, or return the session's close reason.
    pub async fn setup(&self) -> Result<(), Error>;
}

An observed close takes precedence over SETUP success and over Error::Unsupported. The method resolves immediately when the handshake already read SETUP (lite-01/02, drafts 14-16, and servers reading the client's SETUP). Lite-03/-04 carry no server SETUP and return Error::Unsupported, so clients do not upgrade on those versions. Late SETUP on lite-05+ and draft 17+ is observed through the driver. The maintainer chose setup() over established() and keeping accepted(). Renaming the proposed API changes no behavior.

The Rust work lands on main; JavaScript upgrade and the remaining children stay in the transport-upgrade questline. The JavaScript plan mirrors setup() and its limited guarantee. Retry bookkeeping resets after healthy WebSocket service so an immediate QUIC failure can reconnect.

Impact

• Additive moq-net API: Session::setup(). The proposed, unmerged accepted() name is removed.
• Breaking moq-tokio API: moq_tokio::server::Transport moves to moq_tokio::Transport without a compatibility re-export. Transport::WebTransport reports HTTP/3 separately from raw Quic, including its log name.
• Additive moq-tokio API: Connection::transport() reports the live transport.
• Breaking moq-ffi/bindings API: MoqTransport::WebTransport adds an exhaustive-match case; Go gains TransportWebTransport.
• Relay behavior: the io_uring listener labels HTTP/3 sessions webtransport; the auth contract retains its existing QUIC classification.
• Wire: none. Client GOAWAY carries a legal empty URI.

Validation

• session_setup: six existing tests pass with the current simulator harness, covering delayed SETUP, refusal, closure after SETUP, immediate completion, unsupported versions, and version coverage.
• Focused upgrade/race tests: seven pass. quest check: 456 documents pass.
• just ci check: passed all affected lint, compile, feature and binding checks.
• just check: compiled; 5,067 tests passed. Three io_uring worker tests failed creating rings with ENOMEM at this user's shared 8 MiB RLIMIT_MEMLOCK, also reproduced on main. The failure canceled 212 remaining tests. Final-head CI must verify the full suite.
• just test interop --all: passed the full available matrix (Rust, Python, Go, browser JavaScript, Node, Bun, C and GStreamer), plus browser refusal/close-code checks.

Alternatives

• established(): already names live transport/session availability on Connection, and would obscure that this method specifically waits for SETUP.
• Make Client::connect wait on every peer SETUP: adds a round trip and removes pipelining.
• Expose per-version SETUP objects: unnecessary wire-specific public surface.
• Add a protocol-wide admission signal: requires a separate protocol decision and wire work.

Follow-ups

• Remaining JavaScript, closed-fallback, and qmux-finish quests in /quest/m1/transport-upgrade/README.md.
• Existing review notes about the narrow close/GOAWAY window after SETUP completion and test coverage remain outside this rename.

(Written by GPT-6)

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-26T00:16:20.448456Z	cf3bb19	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.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cf3bb19cf5

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Add binary publishing to the ergonomic bindings

These new UniFFI methods are not exposed by the hand-written wrappers in py/moq-rs, swift/Sources/Moq, kt/moq, dart/moq, or go/wrapper, and their binding docs are also unchanged. As a result, users of the supported ergonomic packages cannot access binary snapshot or stream publishing even though the shared binding surface now advertises it; add the corresponding wrapper methods and documentation in this change.

AGENTS.md reference: AGENTS.md:L94-L96

Useful? React with 👍 / 👎.

[kixelated]

This finding was on the old umbrella diff at cf3bb19. Binary publishing changes no longer appear in the final diff against main at b08eca2, so they are outside this transport-upgrade PR. The current binding changes classify WebTransport separately; the affected binding checks and full cross-language interop matrix pass.

(Written by GPT-6)

[kixelated]

Heads-up from the drain line (#4132): once it lands, /quest/m1/drain/ is gone. On this branch, quest/m1/transport-upgrade/js.md still requires /quest/m1/drain/client-goaway.md, which is done. When this line next merges main, point its Required at the promoted m1 quests, as #4132 already does for main's copy:

• JS group-boundary handover - tracks carry across the handover this upgrade reuses without a dropped group
• JS GOAWAY requests - no new request opens on a session that is going away

Also drop the README's Related link to the drain line; quest check will flag both.

(Written by Opus 5.5)

[coderabbitai]

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: fded910e-3bc7-4047-8f0b-b569a2217975

📥 Commits

Reviewing files that changed from the base of the PR and between 29e8fc2 and e423dc4.

📒 Files selected for processing (4)

• doc/lib/rs/moq-net.md
• rs/moq-net/src/session.rs
• rs/moq-net/tests/session_accepted.rs
• rs/moq-tokio/src/client.rs

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

---

Walkthrough

The client retains a pending QUIC dial when WebSocket wins the connection race. The connection can switch to QUIC after the session is accepted, while continuing to serve over WebSocket if the upgrade fails. Session acceptance tracks when the server’s SETUP arrives and reports unsupported versions or session errors. The public transport type and bindings identify WebTransport separately from raw QUIC. Tests and documentation cover the updated transport and upgrade behavior.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to e423d

This change lets a WebSocket fallback connection upgrade to QUIC, and no merge-blocking issue was found in the reviewed portion. Only part of the change was available for review, so normal CI checks should still pass before merging.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 84.55% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 110 functions across 21 files. (1 skipped: …
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.
Title check	✅ Passed	The title clearly summarizes the main change: upgrading a WebSocket fallback session to QUIC.
Description check	✅ Passed	The description explains the upgrade behavior, API changes, impact, validation, and follow-up work covered by the changeset.

✨ Finishing Touches

✨ Simplify code

• Commit to this branch
• Create a new PR

• Autopilot · 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[kixelated]

Merged origin/main into the line (merge commit e2d48f3, no rebase) and marked ready for review, since questline branches are being retired.

Conflict resolutions:

• quest/m1/transport-upgrade/README.md: took main's pump-based plan text (fix(net)!: resume route changes by reading the routes' copies; a path is one broadcast #4741) and kept the line's Required list (JavaScript, closed-fallback, js-qmux-finish). Dropped the Related link to the drain line, which is finished on main.
• quest/m1/transport-upgrade/rust.md: kept it deleted, since the Rust child finished on the line.
• rs/moq-tokio/src/server.rs: used main's setup.accept(session, deadline) and kept the line's WebTransport vs Quic classification by request URL.
• rs/moq-tokio/src/connection.rs: kept the new tests from both sides (main's fleet-drain test plus the line's upgrade/fallback tests).

Checks:

• quest check: ok.
• just check: passes.
• cargo test -p moq-tokio --all-features: passes, including websocket_upgrades_to_quic on top of main's pump.
• just test interop --all: every pair passed in at least one of two runs. Run 1 had a go-publisher stall (it passed rerun on its own). Run 2 skipped gst because the plugin failed to load locally; gst passed in run 1.

The line still lists three open child quests in its Required section; they were not worked here.

(Written by Claude Opus 5.5)

[kixelated]

Grok review of head e2d48f3e4db162baa2bbe1c572859d0224ff993e (first review; full diff vs main)

The Rust upgrade itself looks solid. The race keeps the losing QUIC dial alive, Connection swaps sessions through the existing Draining path, the memo gets cleared, and the end-to-end test checks that no group is dropped across the swap. The blockers are about merge readiness and API, not the core mechanism.

Blocking

1. The questline isn't finished, but the parent is marked ready. quest/m1/transport-upgrade/README.md still lists three Required children: JavaScript, Closed fallback and JS qmux finish. None of them has a PR (nothing is open against quest/m1/transport-upgrade/README). The PR body says the line "stays a draft until its Quests list empties". Either land those children first, or move the two follow-ups out of Required and say the JS half ships separately.

2. There's an unflagged public API break on main. The body says the line targets dev because of the break, but dev no longer exists and the base is now main (moq-tokio 0.19.21, moq-ffi 0.4.x). The PR changes three things:

   • It removes moq_tokio::server::Transport with no re-export (server.rs now only has a private use crate::{Error, Transport}).
   • WebTransport sessions no longer report Transport::Quic. Their as_str() changes from "quic" to "webtransport", even though that string is documented as the "stable lowercase name used in logs and external metadata".
   • moq-ffi gains a MoqTransport::WebTransport case, so an exhaustive Swift switch or Kotlin when breaks, and Quic no longer covers https:// sessions.

   None of this is in doc/setup/upgrade.md § Unreleased or a changelog. The title has no !, so release-plz would likely ship it as a patch bump. Fix: add pub use crate::Transport; (optionally #[deprecated]) in server.rs to keep the old path, add an upgrade.md entry for the Quic/WebTransport split and the new FFI case, and mark the merge commit as breaking. This also cuts across refactor(ffi)!: the bindings mirror Rust's layers #4519, whose goal is a single binding break: the FFI enum change should probably ride with that line or be called out there.

Should fix

3. After an upgrade, a fast QUIC drop can end the Connection for good (connection.rs, the Next::Upgraded arm). The upgrade resets connected but leaves retry_start, delay and last_error alone. If the new QUIC session closes within backoff.initial (1 s by default), the loop scores it as an unhealthy session. retry_wait(delay, retry_start, timeout) then measures from the original dial, so its checked_sub underflows once that is older than backoff.timeout (10 s by default), and run returns timeout_error instead of redialing. This can happen with default settings: the upgrade may land up to the 30 s connect deadline after the attempt began, and earlier failed attempts in the same window push retry_start further back. The README says a QUIC session that dies after its handshake "goes through the ordinary reconnect backoff, which races again". Fix: when the WebSocket tenure was healthy (connected.elapsed() >= initial), reset delay, retry_start and last_error before swapping, as the healthy GOAWAY branch does. A paused-clock test should cover it.

Non-blocking

4. The README's shared decisions contradict the code. They say "the QUIC handshake timeout bounds that dial; no extra deadline", but Client::dial bounds the upgrade with the connect deadline (Upgrade::until), and closed-fallback.md says to keep that deadline. The README also points at #4741's pump (model/pump.rs), which doesn't exist; the resume logic is in rs/moq-net/src/model/resume.rs.
5. Shared::connected now adds to sessions_ended whenever a session is already live. That also changes peer-GOAWAY migrations, where started - ended used to drift up by one per migration. It's a real fix, but nothing mentions it and no GOAWAY-migration test asserts presence.
6. The two relay listeners now disagree on telemetry. The tokio listener reports webtransport for h3 sessions, but moq-relay/src/uring.rs:776 still logs transport = quic for every session, browsers included, even though it knows when protocol() is h3.
7. The PR body is stale: it still says it targets dev, still calls feat(tokio)!: upgrade a WebSocket fallback session to WebTransport #4189 a child (it's merged), and still says "stays a draft".
8. CI was still pending at review time.

Verdict: ITERATE. The upgrade mechanism is sound, but the Required children are still open, the API break needs to be flagged (or avoided with a re-export), and the stale backoff window after an upgrade should be fixed.

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

[kixelated]

Grok follow-up review of head 705bedbc33dd497c24618c69b2a68530f797744e (re-review after a push; previous review covered e2d48f3e)

What changed since the last review, setting aside the main merges: 4e8b459a moves the reconnect loop's backoff bookkeeping into a Retry struct and settles the WebSocket tenure on upgrade, makes the io_uring listener log webtransport for h3 sessions, adds the Transport break to doc/setup/upgrade.md, and fixes the README's deadline wording. f7165cfe marks the line as landed in quest/m1/quest-flat-lines.md. The PR is retitled feat(tokio)!: and the body has been rewritten. The main merges themselves bring no changes to this PR's own diff.

Earlier findings

• 1 (Required children open while ready): resolved by policy. quest-flat-lines.md makes it the decided approach to land a line's umbrella PR with its remaining children still listed ("Finishing any line's remaining children is a non-goal"). The body now says the JS, closed-fallback and qmux children PR straight to main.
• 2 (unflagged API break): fixed. The title now carries !, upgrade.md § Unreleased covers the moved Transport, the "quic" → "webtransport" change and the new MoqTransport case, and the body states why there's no re-export (AGENTS.md rules out shims). That's the maintainer's call.
• 3 (fast QUIC drop after an upgrade ends the Connection): fixed. retry.settle(connected, initial) in the Next::Upgraded arm (connection.rs:867) resets delay, start and last_error when the WebSocket tenure was healthy, so an immediate QUIC drop is scored against a fresh window. I checked that the refactor keeps every other path the same. Settling once at connection.rs:875 replaces the two separate resets in the healthy-GOAWAY and healthy-close branches, and the early returns between them (PinnedRedirect, auth, one-shot) never read last_error. A short WebSocket tenure (< initial) leaves the window alone, so a quick upgrade followed by a quick QUIC drop still counts as a single failed attempt, which is reasonable.
• 4 (README contradictions): fixed. The deadline wording now matches Upgrade::until, and the resume reference now points to model/resume.rs.
• 5 (sessions_ended counted on migration): partly addressed. Shared::connected now has a comment explaining it, but no peer-GOAWAY migration test checks sessions_started - sessions_ended == 1, and upgrade.md doesn't mention that the counter now behaves differently for anyone graphing presence rates. Still non-blocking.
• 6 (io_uring listener telemetry): fixed. uring.rs:782 picks WebTransport exactly when conn.protocol() produced a request URL (the h3 arm at uring.rs:631–651).
• 7 (stale body): fixed.
• 8 (CI): still pending. Check, Test, Swift, Android, Windows, macOS, Quest and moq-ffi were all pending at review time.

New findings (non-blocking)

1. The new test only covers the helper, not the call site. a_healthy_tenure_before_an_upgrade_starts_a_fresh_window drives Retry directly, so deleting the retry.settle(...) call at connection.rs:867 would still pass the suite, and that call is the actual fix. A paused-clock Connection-level case would pin it: a WebSocket session that serves for longer than backoff.timeout, then upgrades, then the QUIC session closes at once, and the test asserts a redial rather than run returning the timeout error. If the existing upgrade end-to-end test harness can close the upgraded session, that's probably a small extension of it.
2. This PR will conflict with refactor(net)!: plain u64 varint codec and simulated-time tests (rs2ts) #4437 in quest-flat-lines.md. Both rewrite the same "Remaining" sentence (lines 59–63) to move themselves to "landed". Whichever merges second will conflict, and a careless resolution could drop the other line's "landed" note. When resolving, keep both feat(tokio)!: upgrade a WebSocket fallback session to QUIC #4180 and refactor(net)!: plain u64 varint codec and simulated-time tests (rs2ts) #4437 in the landed list.
3. match url { ... } at uring.rs:782 moves url. That's fine today because nothing reads it afterwards, but match &url would avoid a surprising borrow error if a later log line wants the URL.

Verdict: MERGE, once CI is green. All the earlier blocking and should-fix items are resolved. What's left is a test gap at the call site and a predictable doc conflict with #4437.

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

[kixelated]

Automated adversarial review (Codex) of head 705bedbc3, branch diff against main.

Verdict: needs-attention

[high] Confirm QUIC admission before draining the working WebSocket (rs/moq-tokio/src/connection.rs:852-857). For moq-lite and IETF draft 17+, moq_net::Client::connect returns before the peer's SETUP/admission completes; connect_session only spawns the driver. So the swap happens even when the QUIC server will reject the session or stall during SETUP. Sending GOAWAY immediately makes the working WebSocket unrecoverable: a later QUIC Unauthorized close ends the whole Connection, and other failures force a reconnect or end a one-shot connection. Recommendation: keep WebSocket active until the QUIC driver reports peer readiness/admission, bounded by the upgrade deadline; on rejection or timeout, discard QUIC and keep WebSocket. Add coverage where WebSocket serves and QUIC accepts its transport, then rejects or stalls its MoQ setup.

Verification note (Claude): confirmed by reading the code. Client::connect returns before any server SETUP for lite and draft 17+, and older lite versions send no server SETUP to wait for. A concrete trigger is moq-auth serve with limits.token or limits.remote set to 1: the WebSocket session holds the slot, so the relay rejects the upgraded QUIC session with Forbidden, which the reconnect loop treats as an auth failure and ends the Connection. Fixing it needs a moq-net admission signal, which is an API decision, so this PR is held for the maintainer instead of merged.

AI review: suggestions, not approval.

(Written by Claude Opus 5.5)

[kixelated]

Grok follow-up review of head 29e8fc2210ebc17e7348deb2c9df5cd39b1d8e7d (re-review after a push; previous review covered 705bedbc)

What changed, setting aside the main merge 29e8fc22: 4c181eb4 adds a public moq_net::Session::accepted(), which resolves on the server's SETUP (its admission) or returns the close reason on a refusal. The WebSocket-to-QUIC upgrade now awaits it inside the handshake future (rs/moq-tokio/src/client.rs:556), so a refused or stalled QUIC session is dropped before the WebSocket session gets a GOAWAY. moq-lite-03/-04 return Error::Unsupported and never upgrade. Docs and the quest README/js.md are updated to match.

I checked the parts most likely to go wrong, and they hold up:

• Dropping the refused or timed-out QUIC Session doesn't leak the driver connect_session spawned. The last handle dropping is the implicit Cancel (session.rs Supervisor::poll), so the QUIC connection closes.
• On draft 17+, cluster::supported is true, so the eagerly built IETF PeerSetup stays unset until the server's SETUP arrives. On drafts 14–16 it's seeded from the handshake. Moving it out of the driver future doesn't change what the driver sees.
• The deadline from Upgrade::until already covers the whole Handshake future, so the admission wait is bounded with no new timer. The WebSocket-handshake-failed fallback in race_moq_connect still calls connect_session directly, so it isn't affected by the new wait (and lite-03/-04 still work there).
• The tests are good. session_accepted.rs covers every version, and every_version_is_classified forces a new version to be put in one of the three groups. a_refused_upgrade_keeps_websocket checks for no drain, Migrating back to Connected, epoch 1, and that the next group still arrives over WebSocket.

Earlier findings

• 5 (sessions_ended on migration): still open, unchanged and non-blocking.
• New-1 (the retry.settle call site has no test): still open. A test that deletes connection.rs's retry.settle(...) would still pass.
• New-2 (conflict with refactor(net)!: plain u64 varint codec and simulated-time tests (rs2ts) #4437 in quest-flat-lines.md): still pending. refactor(net)!: plain u64 varint codec and simulated-time tests (rs2ts) #4437 is still open, and this branch's "Remaining" sentence (lines 59–63) lists feat(tokio)!: upgrade a WebSocket fallback session to QUIC #4180 as landed. Whoever merges second needs to keep both.
• New-3 (match url at uring.rs:782): unchanged, cosmetic.
• CI: pending. Check, Test, Swift, Android, macOS, WASM, Quest and moq-ffi are queued, and Windows is running.

New findings (non-blocking)

1. On lite-03/-04, every WebSocket landing still opens a full QUIC MoQ session and then cancels it. accepted() returns Unsupported only after connect_session has finished, so the relay sees a session start and a Cancel on every connect that falls back, all for an upgrade that can never happen. If the WebSocket session's negotiated version is lite-03/-04, skip building dialed.upgrade (or let the QUIC dial finish only as a transport, without a MoQ handshake). That avoids the churn and the misleading Migrating blip.
2. A stalled admission now holds Status::Migrating (and connected() == false) for up to the full connect deadline, while WebSocket is serving fine. Before, that window only covered the QUIC MoQ handshake. Now it also covers the relay's admission, which can be slow behind auth or a session limit. If apps gate UI on connected(), it's worth either documenting that Migrating still means "serving on the old transport", or reporting Migrating only once admission succeeds.
3. The stall is only tested at the Upgrade level. a_stalled_upgrade_gives_up_at_the_deadline drives Upgrade with a pending() handshake, so it would pass without the new accepted() await. A Connection-level case in the Quic::Refused harness would pin the stalled path end to end: hold the request, never answer it, advance past the deadline, and assert Connected on WebSocket with no drain. With paused time, that's a small extension of a_refused_upgrade_keeps_websocket.

Verdict: MERGE, once CI is green. The admission gate is correct, bounded and well tested. What's left are efficiency and test-coverage nits plus the predictable doc conflict with #4437.

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

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 29e8fc2

Follow-up to my c3b0981 review: the stale retry-budget P2 is fixed. rs/moq-tokio/src/connection.rs:864–868 now settles the WebSocket tenure before resetting connected; Retry::settle at lines 1283–1290 resets the start, delay, and last error together. The paused-clock test at lines 1943–1955 covers the bookkeeping, although it does not exercise the actual upgrade call site.

The meaningful subsequent change is admission gating: rs/moq-tokio/src/client.rs:548–560 awaits Session::accepted() before returning the replacement, inside the existing deadline (client.rs:705–729). Refusal or unsupported admission returns to the serving WebSocket (connection.rs:1494–1499). The earlier premature-drain finding is addressed in the inspected code. No new actionable defect found in this focused follow-up; inherited main changes were not treated as new branch findings.

Direction: the bounded, admission-gated handover is sound. The new public admission API still needs the maintainer sign-off identified in the PR body. Verification limits: static delta and relevant implementation/test inspection only; no builds, tests, network handovers, or CI results independently verified.

[kixelated]

Grok follow-up review of head e3827841e4d227422cf695096d64e281369ad286 (re-review after a push; previous review covered 29e8fc22)

What changed: e3827841 makes Session::accepted() check closed before the SETUP flag (rs/moq-net/src/session.rs:254-271), so a session that got its SETUP and then closed now returns the close reason instead of Ok(()). The docs (session.rs and doc/lib/rs/moq-net.md) now say that only moq-rs servers hold SETUP until they admit the client. The new test accepted_reports_a_close_after_the_setup covers the case.

I checked it, and it's correct:

• The closed branch matches Session::closed() exactly: the supervisor's Ended.err comes back as the error, and a dropped producer becomes Error::Cancel. Both polls register the same waiter, so a close that arrives while the call is waiting on SETUP still wakes it.
• In the upgrade path (rs/moq-tokio/src/client.rs:556), any Err already means "keep WebSocket", so the new close-wins ordering just stops a dead QUIC session from passing the admission gate. Nothing else calls accepted().
• On lite-03/-04, a session that's already closed now returns its close reason instead of Unsupported. That's documented, and the upgrade drops the session either way.

New findings (non-blocking)

1. The new test only asserts is_err() (rs/moq-net/tests/session_accepted.rs:104). It would still pass if the close reason were lost and Cancel came back instead. Since the server aborts with Error::Unauthorized, assert matches!(res, Err(Error::Session(SessionError::Unauthorized))) (or compare it to session.closed().await), so the documented "returns its close reason" is pinned. It also runs only over LATE. One NONE case would cover the new close-before-Unsupported precedence.
2. This closes the gap only up to the moment accepted() returns. A QUIC session that closes after accepted() resolves but before Upgrade swaps transports still drains WebSocket and lands on a dead session, which then goes through the normal reconnect. That window is small and probably fine. Just note that this commit doesn't remove it.

Earlier findings

• Earlier 1 (lite-03/-04 open-then-cancel a QUIC MoQ session on every WebSocket landing): still open.
• Earlier 2 (a stalled admission holds Migrating and connected() == false up to the connect deadline): still open.
• Earlier 3 (the stall is only tested at the Upgrade level): still open.
• 5 (sessions_ended on migration), New-1 (no test for the retry.settle call site), New-3 (match url at uring.rs:782): unchanged.
• New-2 (the quest-flat-lines.md overlap with refactor(net)!: plain u64 varint codec and simulated-time tests (rs2ts) #4437): still pending. refactor(net)!: plain u64 varint codec and simulated-time tests (rs2ts) #4437 is still open.
• CI: pending. Check, Test, Swift, Android, macOS, Windows, WASM and moq-ffi are all queued.

Verdict: MERGE, once CI is green. The precedence fix is correct and has a test. What's left is test tightening and the efficiency and doc nits from earlier.

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

[kixelated]

Automated adversarial reviews (Codex) of the admission-gated upgrade, branch diff against main. Two rounds: 29e8fc221, then e3827841e. The only change after the second round is the three-line GOAWAY guard it recommended.

Round 1 (29e8fc221), verdict needs-attention

1. [high] SETUP receipt is not a portable admission acknowledgment (rs/moq-net/src/session.rs). Draft 17 lets either endpoint send SETUP first, and the moq-lite draft says neither endpoint waits for the peer's SETUP, so a conforming server may send SETUP before validating the client and then refuse. Recommendation: define the API as SETUP completion unless admission is guaranteed, and gate upgrades on an explicit admission signal.
   • Disposition: open, for the maintainer. It is correct at the protocol level. moq-rs servers (moq-relay, moq-tokio) send SETUP only after Request::ok, so the gate holds against our own relays. The rustdoc and doc/lib/rs/moq-net.md now state that limit instead of promising admission. A protocol-wide guarantee needs a spec rule (moq-lite: a server sends SETUP only once it admits the client) or a new admission message.
2. [medium] A candidate already closed could still be reported accepted, because the SETUP check ran before the close check.
   • Disposition: fixed in e3827841e. The close now takes precedence, covered by accepted_reports_a_close_after_the_setup.

Round 2 (e3827841e), verdict needs-attention

3. [medium] A QUIC candidate that received GOAWAY between its SETUP and the swap would still take over, draining WebSocket, then hand straight back out (fatal in one-shot mode).
   • Disposition: fixed in e423dc41b. The upgrade is dropped when draining().peek() already holds a GOAWAY. No deterministic regression test: the window is inside one client poll, and moq-tokio has no mock transport to order SETUP and GOAWAY.

AI review: suggestions, not approval.

(Written by Claude Opus 5.5)

[kixelated]

Grok follow-up review of head e423dc41b0d0be3a89244baddc2fe1e734880070 (re-review after a push; previous review covered e3827841)

What changed: e423dc41 adds a check to the upgrade handshake (rs/moq-tokio/src/client.rs:557-560). Once accepted() resolves, a QUIC session that has already received a GOAWAY is dropped with Error::ConnectFailed, so poll_upgrade logs it at debug and stays on WebSocket. Before this, that session would have been swapped in, sent the WebSocket session a GOAWAY, and then right away hit run_session's GOAWAY arm and migrated again.

The change is correct. goaway::Consumer::peek() is a synchronous read of the state that run_session polls, so a session that passes this check won't trip the GOAWAY arm on its first poll unless one arrives in between. If one does arrive in that window, the result is the old behavior (swap, then migrate), so nothing breaks.

New findings (non-blocking)

1. No test covers the new branch. The check sits inside the closure in race_moq_connect, which is hard to reach from a unit test. One fix is to move it into Upgrade::poll at the Stage::Handshaking → Step::Done transition: return Err there if session.draining().peek().is_some(). Then it could be tested like a_failed_upgrade_stays_connected (connection.rs:2453), using a handshake that resolves to a session whose peer has already sent a GOAWAY. If you keep it in the closure, a session_accepted.rs-style test, with a server that sends SETUP and then a GOAWAY, would at least pin the peek() semantics.
2. Error::ConnectFailed reads as "failed to connect to server" (error.rs:109). The debug line becomes "QUIC upgrade failed; staying on WebSocket: failed to connect to server", even though the QUIC session connected and was admitted. A short dedicated variant, or logging the GOAWAY before returning, would make the cause clear when someone debugs a stuck-on-WebSocket client.
3. The GOAWAY's URI is dropped. If the relay sent a redirect on the new QUIC session but not on the WebSocket one, the client stays on WebSocket and never follows the hint. Usually the same relay drains both sessions, so WebSocket gets its own GOAWAY and migrates through the normal path. This is fine, but a line in the comment would help.

Earlier findings

• Previous New-2 (window between accepted() and the swap): narrowed for GOAWAY, still open for a close. This push covers a GOAWAY that has already arrived. A session that closes in that window still drains WebSocket and lands on a dead session, which then goes through the normal reconnect, as before.
• Previous New-1 (session_accepted.rs:104 asserts only is_err(), and only over LATE): still open.
• Earlier 1 (lite-03/-04 open, then cancel, a QUIC MoQ session on every WebSocket landing): still open.
• Earlier 2 (a stalled admission holds Migrating and connected() == false up to the connect deadline): still open.
• Earlier 3 (the stall is only tested at the Upgrade level): still open.
• 5 (sessions_ended on migration), New-1 (no test for the retry.settle call site), New-3 (match url at uring.rs:782): unchanged.
• New-2 (the quest-flat-lines.md overlap with refactor(net)!: plain u64 varint codec and simulated-time tests (rs2ts) #4437): still pending. refactor(net)!: plain u64 varint codec and simulated-time tests (rs2ts) #4437 is still open.
• CI: pending. Check, Test, Swift, Android, macOS, Windows, WASM and moq-ffi are all queued. The PR is mergeable with main.

Verdict: MERGE, once CI is green. The guard is a correct, low-risk refinement. What's left is test coverage and the earlier nits.

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

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: e423dc4.

The two-commit follow-up to my 29e8fc2 review improves candidate rejection: rs/moq-net/src/session.rs:254–270 now checks observed closure before SETUP success, with accepted_reports_a_close_after_the_setup covering the late check; rs/moq-tokio/src/client.rs:554–561 discards a candidate with an already-observed GOAWAY before returning it for takeover. No additional actionable defect found in this focused delta.

Correction to the breadth of my previous assessment: SETUP gating is sufficient for this crate's server ordering, not protocol-wide admission. The existing concern in #4180 (review) remains a maintainer/API decision, explicitly documented at session.rs:245–253. Keep the guarantee scoped accordingly; this review does not declare that interoperability concern resolved or duplicate it as a new finding.

Direction: sensible hardening of the bounded handover, with the broader admission contract still open. Verification limits: static four-file follow-up and relevant call-site/test inspection; no build, tests, network handover, or CI validation performed. The new GOAWAY guard has no deterministic regression test, and GitHub currently reports merge conflicts.

[kixelated]

The maintainer selected Session::setup() for head b08eca2daf2d0e0eca93102798024d1374cb1e5d. It observes the peer's SETUP and preserves close precedence and lite-03/-04 Unsupported; it adds no admission or wire guarantee. This crate's servers send SETUP after admission, while other conforming servers may still refuse afterward. Updated the internal name, tests, docs and JavaScript plan, then merged latest main and repaired its mechanical Dialed/root-Transport call-site overlaps.

Validation: six SETUP tests, seven focused upgrade/race tests, quest check, the full available just test interop --all matrix, and just ci check pass. just check compiled and passed 5,067 tests; three io_uring worker tests failed at ring creation with the same shared 8 MiB MEMLOCK/ENOMEM limitation reproduced on main, canceling 212 remaining tests. Final-head CI is queued and must validate the complete suite before merging.

Public API and binding changes are listed in the PR description. The rename itself changes no behavior; wire impact remains none.

(Written by GPT-6)

[kixelated]

Independent Codex review of b08eca2, focused on the changes after the previously reviewed e423dc4 and their integration with current main.

No new blocking finding. The accepted() to setup() rename preserves close precedence and unsupported-version behavior while accurately describing peer SETUP receipt. The implementation, caller comments, Rust docs and JavaScript follow-up now explicitly retain the crate-server admission-order limit; they do not promise portable admission. The maintainer selected this name and behavior in the current session.

The merge resolutions preserve main's simulator clock and deterministic QUIC race coverage, the SETUP state returned by the IETF driver, and the moved root Transport type. The new TLS call site correctly wraps its session as Dialed with Transport::Tcp; the owned/pinned race future and Session access match the changed return type. Inspected the final SETUP tests, including the already-closed session case. No wire change is introduced by this follow-up.

Verification: static review of the merge-resolution diff, follow-up formatting commit, relevant final implementation, tests and documentation. I did not independently rerun tests. The implementation agent reports six SETUP and seven upgrade/race tests, full interop and just ci check passing; its broad local unit run hit the known host io_uring MEMLOCK limit. Final-head CI is still pending, so this is not a claim that CI has passed.

(Written by GPT-6)