[kixelated]

Finishes quest/m1/websocket-quic-upgrade.md and deletes it, with its link.

Problem

A short UDP outage makes a reconnecting link redial. WebSocket wins that race, and the link then stays on WebSocket for good: nothing re-probes QUIC while a session runs, and the race memory (WEBSOCKET_WON in moq-tokio, websocketWon in @moq/net) drops QUIC's head start on every later redial until QUIC wins once. One blip leaves a link on the degraded transport indefinitely.

Changes

• moq-net Reconnect probes the preferred transport. A probe previews its tokens (Tokens::preview) and commits them (Tokens::commit, replacing Tokens::dial) only once it gets through, so a failing probe never disturbs the fallback session's grants. A driver whose session landed on a fallback calls Reconnect::fallback(now). The policy then emits Action::Probe(attempt) 5 to 10 s later, then on a doubling, jittered backoff capped at one every 5 minutes, each bounded by the connect timeout (Action::Abandon on expiry). The driver reports with Reconnect::probed(now, Result<(), E>).
• Make-before-break through the GOAWAY handover. A probe reaches the transport only, with no MoQ handshake. Once it gets through, the fallback session sends its own GOAWAY naming no URI and drains as the predecessor (queued Action::Migrating). The probe then becomes the dial in flight: the driver finishes the MoQ handshake over the transport it reached and reports connected/failed as usual. The fallback drains first because an auth handle backs one active session at a time. It keeps serving until the replacement does, and closes once idle or at the handover cap. A failed handshake falls into the usual migration retry while the predecessor serves.
• moq-tokio marks a session as on the fallback when WebSocket won the race for an http(s):// URL with the QUIC backend compiled. Its probe is a QUIC-only dial (Client::probe), and the probed connection carries the replacement session, so a WebSocket win in a second race is impossible.
• The WebSocket race win is provisional. moq-tokio forgets WEBSOCKET_WON when the WebSocket session that set it ends: on close, GOAWAY, or a probe moving off it. A post-outage redial therefore gives QUIC its head start again. This needs no clock.
• @moq/net mirrors it. websocketWon is cleared when the winning WebSocket session closes. While the reconnecting Connection (Reload) runs on WebSocket, it probes with a bare WebTransport session (reachable in connect.ts, internal) on the same backoff. Once one opens, the session sends its own GOAWAY through a new internal wire hook, leave() (a leaving phase on the lite and IETF connections), drains as the predecessor, and the replacement finishes its MoQ handshake over the probed WebTransport session (internal establish) with no second race, as in moq-tokio.
  • The IETF compatibility layer in JS only drains locally on leave() and sends no client GOAWAY. Lite sends one on lite-04+.
• Docs: the moq-tokio::websocket module docs and doc/concept/transport.md no longer promise that a WebSocket session stays put.

Tests

• moq-net (simulated clock): moving off the fallback (a GOAWAY reaches the peer, the predecessor retires once idle, nothing goes offline); a target that never reaches the preferred transport probes no faster than the jittered doubling bound and reaches the 5 minute cap; a hung probe is abandoned at the connect timeout; probing stops with the fallback session; connecting once never probes.
• moq-tokio (paused clock, in-memory sessions): UDP blocked → WebSocket, unblock, the link is on WebTransport within the probe bound with the subscription still delivering the next group, and the race memory is cleared. A WebSocket session ending clears the race memory.
• @moq/net (manual clock, in-memory WebSocketStream and WebTransport): the fallback moves to WebTransport after WebTransport becomes reachable over the probe's own session (one WebTransport dial), the old peer receives the GOAWAY, and the status never reports disconnected.
• moq-net auth::tokens: previewing a probe's credential leaves the live session's in-band grants alone until the dial is committed.

Choices made without asking

• Probe pacing is a private constant (10 s doubling to 5 min, jittered 50 to 100%) in both languages. I added no [connect.websocket] probe setting and no MoqWebSocketConfig field, since no consumer needs to tune it yet.
• A probe is transport-only rather than a full MoQ dial, because a second active session on the same auth handle is refused (Error::Duplicate).
• A peer GOAWAY that arrives with a successful probe wins, in both languages, so its redirect is never lost; the probe is dropped and retried on its backoff.
• Moving off the fallback earns the healthy-session reset when the fallback served past the healthy bar, but skips the redirect-loop and restart bookkeeping, since it is no peer's redirect.
• The client sends a GOAWAY when moving off the fallback. moq-lite allows either endpoint to send one, and it tells the relay to rank the old session's routes below the replacement's.
• Connecting once (with_reconnect(false)) never probes: moving would be a second session.

Decisions

• Action::Probe is a new variant on the exhaustive moq_net::reconnect::Action, a semver break for any out-of-tree driver of Reconnect (the quest's plan had called the milestone additive): ✅ accept the break and mark the title ! (rejected: a separate Reconnect::poll_probe that drivers could skip, a second poll loop every driver must remember to run). moq-tokio is the only in-tree driver, Action is exhaustive so drivers handle new actions, and main allows breaking changes. Recorded here.

Public API / wire impact

• moq-net (breaking): adds Action::Probe, Reconnect::fallback, and Reconnect::probed. Action::Abandon now also covers a probe.

Impact

Migration for an out-of-tree driver of moq_net::reconnect::Reconnect: the match on Action gains an Action::Probe(attempt) arm. A driver that never calls Reconnect::fallback never receives it and can match Action::Probe(_) => unreachable!(). A driver that does dials the preferred transport only, then reports with Reconnect::probed(now, result).

• moq-tokio: no API change. Behavior: a reconnecting connection on the WebSocket fallback probes QUIC and moves back, and sends a GOAWAY to its peer when it does.
• @moq/net: no public API change (reachable, Reached, establish, and the wire view's leave are internal). Same behavior change.
• Wire: none. The client GOAWAY uses the existing message.

Caused by

No single PR: a WebSocket session has stayed put since the fallback was added.

Follow-ups

• moq-ffi gets this through moq-tokio unchanged, so no SDK work is needed. No follow-up recommended.

Merge with main

• Merged origin/main after moq-dev/moq#8266 reshaped the dial credential. Tokens::preview/commit keep the probe's read-only preview on main's shape (Dial { token: Option<String>, .. }, no URL or versions), Reconnect builds main's Attempt { target, token, auth } for dials and probes alike, and moq-tokio's Client::attempt uses Attempt::client.
• test/interop/interop.sh now expects the native JS catalog reader's catalog compression=none|deflate line. moq-dev/moq#8266 renamed that output but left the harness matching compress=, which fails just test interop --all on main too; no open PR fixed it.
• just check and just test interop --all pass on the merged head.

Models

Size: M
Implementer: Claude Code, Opus 5.5, medium effort
Implementer: Claude Code, claude-opus-5-5, medium effort (iterate, M)
Implementer: Claude Code, claude-opus-5-5, medium effort (iterate, S)

(Written by Opus 5.5 with medium effort)

Review

mechanical fix by codex, gpt-6.1-sol, medium effort: Merged base, preserving its optional timeout handling and the PR shared jitter helper; adapted probe timeout uses and tests; ran just ci base, just test interop --all, and git diff --check base. (head 465c215)

Rewritten by quest review after each round.

• Reviewer: Codex, gpt-6.1-sol, medium effort
• Session: 01a1be6f-8eaa-7560-be09-b0b0df4632bb, round 5
• Head: 2a2f7e5
• Base: origin/main at 6724807
• Score: 10/10, The incremental changes correctly adapt probe credential handling to the upstream API and align the interop expectation with the reader’s output, with no new findings.
• Findings: none

[kixelated]

Maintainer decision (via /quest-complete):

• Action::Probe on the exhaustive moq_net::reconnect::Action: ✅ (a) accept the break, mark the title ! and add the Impact note (rejected: (b) a separate Reconnect::poll_probe).

(Written by Claude Opus 5.5 with high effort)

[coderabbitai]

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 20 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

• 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: eed0c6c4-7ccd-4911-8a1a-ccd0108eec16

Commits

Reviewing files that changed from the base of the PR and between 6ba771b and 215b32a.

Files selected for processing: 18

• doc/concept/transport.md
• js/net/api/index.api.md
• js/net/bench/forward.ts
• js/net/src/connection/connect.ts
• js/net/src/connection/forward.test.ts
• js/net/src/connection/reload.test.ts
• js/net/src/connection/reload.ts
• js/net/src/ietf/connection.ts
• js/net/src/lite/connection.ts
• js/net/src/wire.ts
• quest/m1/README.md
• quest/m1/websocket-quic-upgrade.md
• rs/moq-net/src/auth/tokens.rs
• rs/moq-net/src/reconnect.rs
• rs/moq-net/src/reconnect/tests.rs
• rs/moq-tokio/src/client.rs
• rs/moq-tokio/src/connection.rs
• rs/moq-tokio/src/websocket.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

• X
• Mastodon
• Reddit
• LinkedIn

[kixelated]

Automated review by review (OpenAI)

Reviewed head f378a39.

P2 — Close a successful probe that loses the session race (js/net/src/connection/reload.ts:621–624). A probe can pass its final abort check and return Reached just as the fallback closes, sends GOAWAY, or the effect is cancelled. If that competing event wins effect.race, the fulfilled probe value is discarded. The finally block's probing.abort() is too late to dispose it: reachable has removed its abort listener and #probe has already returned. This leaves an open WebTransport session neither reused nor closed. Retain the probe promise and close any successful transport whose result was not selected, analogous to connect.ts's loser cleanup. Add a test that resolves the probe concurrently with fallback close/GOAWAY or effect cancellation and checks the losing transport closes.

Otherwise checked the Rust probe policy/backoff and timeout, token preview/commit separation, auth handover, QUIC transport reuse, fallback race-memory cleanup, and JS leaving phases. No additional actionable findings.

Verification limits: GitHub-only static source/diff review; no tests, network fault simulation, browser runs, or builds executed. The current PR reports merge conflicts, so any eventual conflict resolution remains unreviewed.

[pullfrog]

ℹ️ No critical issues — three narrow race/parity findings inline.

Reviewed changes

• moq-net probe policy — Action::Probe, Reconnect::fallback/probed, and the Probe/Probing state machine that paces a preferred-transport probe on a jittered 10 s → 300 s backoff and turns a successful one into the dial in flight through the existing GOAWAY handover.
• Token preview/commit — Tokens::dial split into preview/commit, so a failed probe leaves the serving session's in-band grants untouched and the credential is only committed once the probe gets through.
• moq-tokio driver — Client::on_fallback/Client::probe, the driver's probe/fallback bookkeeping, and clearing the provisional WEBSOCKET_WON win on Migrating/Offline/Done; Action::Abandon now also clears a dial in non-QUIC builds and tells a probe abandon from a failed dial.
• js/net probe path — internal Reached/reachable/establish, Reload.#probe raced against close/drain, websocketWon clearing on a probe win and on session close, and the new internal wire leave() (lite sends a client GOAWAY on lite-04+, ietf drains locally) with a leaving phase that frees the shared auth handle.
• Docs and quest — the transport concept doc, the moq-tokio::websocket module docs, and the completed quest/m1/websocket-quic-upgrade.md deletion with its milestone README line (no stale references remain).

The design reuses the existing handover rather than adding a second migration path, and the mock-clock tests in both languages cover the move, the backoff bound, a hung probe, the once case, and the provisional race win. The three findings below are all narrow (same-wake races and a parity gap); none blocks the normal path.

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

[pullfrog]

ℹ️ No new issues in this increment — the only new commit merges origin/main, and its conflict resolutions preserve every behavior already under review.

Reviewed changes

• Upstream renames applied everywhere — @moq/qmux's class is imported as Qmux, so js/net's connection interface is now Session (was Established) across connect.ts, reload.ts, the lite/ietf connections, wire.ts, the benchmark, and the tests, with no stale established.ts import left behind.
• main's status derivation adopted in the reconnect loop — reload.ts's direct #dialing/established writes became #serve/#setDialing/#report, and the probe handover still reports connected while the draining predecessor serves, so moving off the fallback introduces no disconnected flicker.
• Merge fallout in moq-tokio — websocket.rs imports tokio_tungstenite directly (a declared, feature-gated dependency) and connection.rs drops the now-redundant moq_net::kio import, following upstream's dependency move.

Verified locally: bunx tsc --noEmit -p net/tsconfig.json is clean, bun test net/src/connection/reload.test.ts passes 46 including both fallback-return cases, and cargo check -p moq-tokio --features websocket succeeds.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

Important

The three findings from review #5822095073 were resolved as "Fixed in 215b32a30a532bc09c87d9a7d058a6aa909a96e1", but that commit is not on quest/m1/websocket-quic-upgrade: the PR head is still 9f27a8935 (a merge of origin/main whose first parent is f378a3999), the PR's commit list does not contain it, and the GitHub commits API returns No commit found for SHA: 215b32a.... The head tree is identical to the version that review covered, so all three findings still hold and are re-anchored below rather than reopened as new.

Reviewed changes

• No commits landed — the head is unchanged since the prior pullfrog review (9f27a89); only the three thread resolutions moved, and they cite a commit that is not on the branch. The code paths they describe as fixed are still the versions the prior review flagged.

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

[kixelated]

Automated review by review (OpenAI)

Reviewed head 215b32a incrementally, accounting for the intervening main merge. The prior P2 successful-probe leak is fixed: Reload retains the probe promise and closes every fulfilled transport whose result was not selected, including effect cancellation and peer-GOAWAY precedence.

No new actionable findings. Both policies now give an already-arrived peer GOAWAY precedence over a successful probe, and JS resets retry pacing after a healthy fallback session as Rust already does. The new Rust regression covers simultaneous peer GOAWAY/probe completion. A deterministic JS regression for a successful probe losing to close/GOAWAY/cancellation would help preserve the cleanup guarantee.

Verification limits: GitHub-only static source/diff review. No tests, builds, browser runs or network-fault simulations executed.

[pullfrog]

✅ No new issues found.

Reviewed changes

• 215b32a3 lands the three fixes — the probe path now gives an already-arrived peer GOAWAY precedence over a successful probe in both languages, and JS resets the retry window when a healthy fallback is left.
• JS probe ownership — Reload retains the probe promise and closes every Reached the effect.race did not select, instead of inferring ownership from signal.aborted, so a losing or cancelled probe can no longer leak a WebTransport session.
• JS healthy reset — moving off the fallback resets #delay/#deadline when the fallback served past #pacing().healthy, matching Rust's retry.settle, while still skipping the redirect-loop and restart bookkeeping.
• Rust peer precedence — Reconnect::probed(Ok) now polls live.goaway with a no-op waiter before replacing the phase; a ready GOAWAY drops the probe onto its backoff and the next poll follows the redirect. The non-consuming poll leaves the message for that poll.
• Regression coverage — a_peer_goaway_wins_over_a_probe sends a GOAWAY with a successful probe, asserts probed returns false, and asserts the next action is not a probe, so a lost GOAWAY fails the test.

The three open findings from the prior review are addressed and their threads are resolved. Residual, not a defect: a peer GOAWAY that only becomes readable after the probe result was committed as the dial is not re-consulted (same in both languages), but the fallback is already draining at that point.

Verified locally: bun test js/net/src/connection/reload.test.ts (46 pass) and cargo test -p moq-net --lib --features async reconnect::tests (28 pass).

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

• No new commits — the head is unchanged at 215b32a3 since the prior Pullfrog approval, so this synchronize event carries no delta; the run re-verified the resolved findings at head rather than re-reviewing new code.
• Peer GOAWAY precedence — Reconnect::probed(Ok) still yields to a ready live.goaway before replacing the phase, a_peer_goaway_wins_over_a_probe still covers it in reconnect/tests.rs, and Reload still prefers an already-settled draining over a Reached.
• Probe ownership and healthy reset — Reload still closes every Reached the race did not select and resets #delay/#deadline after a fallback session served past the healthy bar, matching Rust's retry.settle.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[moq-bot]

quest land watch: the merge queue removed this pull request at 2a2f7e5 because it conflicts with main (merge_conflict).

The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.

(written by quest land watch)

[moq-bot]

quest land watch: armed auto-merge on this pull request at 465c215, so it joins the back of the merge queue once its required checks pass, with the land fixer's fix (codex, gpt-6.1-sol, medium effort): Merged base, preserving its optional timeout handling and the PR shared jitter helper; adapted probe timeout uses and tests; ran just ci base, just test interop --all, and git diff --check base..

(written by quest land watch)

[kixelated]

Automated review by review (OpenAI)

Reviewed 465c215 incrementally from 215b32a, accounting for main merges. No new actionable findings. The prior P2 successful-probe leak remains fixed; the JS loser-cleanup changes are preserved.

Checked the adaptation to main's credential API: preview remains read-only, commit stays after fallback draining, and both normal dials and transport probes use Attempt::client for the URL, token and auth handle. Probe and replacement-handshake deadlines now honor the optional timeout, while the merged retry-window checks and shared jitter helper are retained. The hung-probe regression uses an explicit timeout.

Verification limits: GitHub-only static source/diff review; no builds, tests, browser runs or network-fault simulations performed.

[pullfrog]

✅ No new issues found.

Reviewed changes

• Merged origin/main credential API — Tokens::preview/commit now carry main's Dial { token: Option<String>, connection }, and Reconnect::attempt builds main's Attempt { target, token, auth } so moq-tokio's dial and probe paths use Attempt::client for the URL, SETUP credential, and auth handle. The preview stays read-only and is committed only after the fallback starts draining.
• Optional connect timeout — the probe and probe-handover deadlines use Config::timeout: Option<Duration> directly, dropping the zero-means-none filter and leaving a probe unbounded when no timeout is configured, matching the ordinary dial path.
• Retry backoff merge — Retry::wait() keeps main's window-less signature via the PR's shared jitter helper, with back_off applying the retry window exactly as origin/main does.
• Probe test configuration — the hung-probe and probe-stops-with-fallback tests now pass Some(..) timeouts.
• Interop expectation — the catalog fanout harness now matches the native JS reader's renamed compression=none|deflate line.

The delta is a mechanical adaptation to base with no behavior change, and its files are byte-identical to the squash commit 2311ef4cc now on main. All three prior Pullfrog threads remain resolved and their fixes are intact.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}