Repository navigation
Conversation
Quest-Claim: c4347869a7b639b1
A session now establishes the reconnect policy only once it stays up past Backoff::healthy. Until then a closed session ends the policy like a failed walk, so a relay refusing a token right after ALPN (moq-lite 03+) reports the refusal instead of redialing forever. Rust Reconnect and js/net Reload agree; Handover.tla models the new rule. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…outage Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Also adds a follow-up quest for a socket close flake seen in the py tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou'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 12 minutes. View limit details
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. |
There was a problem hiding this comment.
ℹ️ No critical issues. One documentation gap across the bindings.
Reviewed changes
I reviewed the full diff at 7e93c67: the Reconnect and Reload fail-fast change, the Handover.tla update, and the test and doc updates across Rust, JS, Go, Python, C, FFI and GStreamer.
- Rust
Reconnect:establishednow comes from session tenure inend()and fromsettleinprobed(), not fromconnected().closed()ends the policy while the policy is not yet established, and it does this before the predecessor migrate-back. - JS
Reload:#establishedHrefis set only by#settlewhen a session ends. That covers a close, a GOAWAY, a probe, and run teardown on a page hide or disable.connectedmoved to the run scope so the cleanup can read it. A close before the healthy bar now calls#giveUp. - Parity checked: a GOAWAY before the healthy bar still migrates or backs off on both sides, and the failure after it then ends the policy on both sides. Under
Restart::Wait/Handoff, a session drained before it is healthy keeps serving until the peer closes it. Its tenure includes that wait, so it still establishes the policy unless the drain deadline is under 1 s. - Model:
establishedanddrainedare now real variables, andFirstWalkFailsFastallows only a GOAWAY's backoff. The retargeted drill mutation patch still applies cleanly. - Tests: tests that relied on an immediate redial now wait past the healthy bar first. The real-clock waits begin after the client reports
Connected, sotenure >= healthyholds when the close lands.
ℹ️ The Go, Swift, Kotlin and Dart wrappers still describe the old fail-fast rule
The PR updates the Backoff docs for Python (_records.py), moq-ffi, moq-tokio and moq-c to say retrying starts only after a session stays up for 1 s. The matching Backoff and connect docs in the other bindings, and the user-facing doc/lib pages, still say only "a first dial that reaches no session ends the connection". An app developer reading those docs would expect a token refused after connecting to be retried, which is the behavior this PR removes. AGENTS.md asks for the same functionality across all languages and wrappers, and doc/ is what end users read.
Technical details
# Wrapper and user docs still describe the pre-#8817 fail-fast rule
## Affected sites
- go/wrapper/client.go:209 — `Backoff` doc: "applies only once a session has been established: a first dial that reaches no session…"
- go/wrapper/client.go:275-276 — `Connect` doc: "If that first dial reaches no session… only an established session reconnects"
- swift/Sources/Moq/Aliases.swift:93-94 — `Backoff` doc, same wording
- swift/Sources/Moq/Client.swift:68 — connect doc
- kt/moq/src/jvmAndAndroidMain/kotlin/dev/moq/Aliases.kt:389 — `Backoff` doc
- dart/moq/lib/src/aliases.dart:276-277 — `Backoff` doc
- dart/moq/lib/src/client.dart:64 — connect doc
- doc/lib/index.md:69 — "a relay that never answered with a session ends the connection"
- doc/lib/{go,kt,swift,dart,cpp,py}/index.md — Stats bullets: "A first dial that reaches no session ends the session with that error rather than retrying."
- py/moq-rs/README.md:163 — same sentence
## Required outcome
- Each binding's `Backoff`/connect docs match the wording already used in `py/moq-rs/moq/_records.py` and `rs/moq-ffi/src/session.rs`: until a session has stayed up past 1 s, a failed dial or a closed session (such as a refused credential) ends the connection with its error rather than retrying.
- The `doc/lib` pages state the same rule.claude-opus-5-5 | 𝕏

Completes quest/m1/first-dial-refusal.md.
Problem
A client whose first session is refused right after ALPN (a rejected token on moq-lite 03 or later, which has no server SETUP to refuse in) redialed forever:
Reconnectmarked the policy established as soon as the transport connected, so the relay'sMalformedAuthTokenclose went throughclosed()and backed off underBackoff::timeout: Noneindefinitely. moq-dev/moq#8694 saw one refusal a second in the relay log and changed the interop test instead. The gst docs promisedfailedon a rejected token, which had been false since moq-dev/moq#8618.Approach
A session establishes the policy only once it stays up past
Backoff::healthy; no error kind is read.rs/moq-net/src/reconnect.rs:connected()no longer setsestablished;end()sets it from the session's tenure (so a long session that ends in a redirect loop still counts) andprobed()from the fallback'ssettle.closed()ends the policy with the close reason while not established, aswalk_failedalready did for a failed walk.js/net/src/connection/reload.ts:#establishedHrefis set by#settleat each session's end (close, GOAWAY, fallback probe, and the run's teardown on a page hide) once it lived pasthealthy; a close before then gives up the URL with that error.Handover.tla:establishedis a variable set where the code sets it,EndClosedends the policy until then, andFirstWalkFailsFastnow also covers closes (a GOAWAY's backoff, which the peer asked for, is tracked asdrainedand exempt). All three configs pass (1.77M, 389k, 401k distinct states); revertingEndClosedto always back off violatesFirstWalkFailsFast. Skill table, counts, and sync commit updated.doc/bin/gstreamer.mdandrs/moq-gst/src/sink/session.rsnow promisefailedfor a first session that fails or is refused before it is healthy; the stale "refusal of since-replaced tokens redials" claims inmoq-ffiand the verify-reconnect skill are gone (no code did that since feat(reconnect)!: the reconnect loop stops judging errors by kind #8618);moq-tokio,moq-c,moq-ffi, and PythonBackoffdocs say when retrying starts. Dart bindings regenerated for the ffi doc change.FASTwait where it runs real sockets).test/drill/mutations/reconnect-stops-after-session-loss.patchretargeted to the new context.Decisions
Taken while the maintainer was away; each is reversible in review.
a_first_settled_answer_fails_fastalready settled that for a refused dial, and keeping an exception only for the after-ALPN path would make the same refusal behave differently by version.tokens::a_refused_credential_redials_with_its_replacementand the JS counterpart became tests of the fail-fast instead. Alternative: retry once when the connection credential changed since the dial (a smallTokens::replacedcheck), for both paths.main, as feat(moq-relay): restart beside the draining relay without dropping sessions #8596 did.Impact
Reconnect/moq_tokio::Connection, JSConnection/Reload, and every binding throughmoq-ffi) whose first session closes beforehealthy(default 1 s) ends with that close's error instead of redialing. After a healthy session, every failure retries as before.Validation
just test interop --all: passes, including the browserrefused sessioncase, now asserting a WebSocketConnectionstops after exactly one refused session against a real relay process with real auth (it failed with "gave up after 1 session(s) instead of redialing" before the case was updated, which is the new behavior on the real surface).reconnect::tests::a_first_session_closed_before_healthy_fails_fastdeadlocks redialing, andreload.test.ts"a first session refused before it is healthy stops the URL" times out.moq-tokioreconnect_ends_on_a_first_session_rejection(real QUIC server rejecting with each of Unauthorized, MalformedAuthToken, Expired: one accept, then the loop ends) andreconnect_retries_a_rejection_after_a_healthy_session;moq-relaywebsocket_unauthorized_ends_a_first_session;moq-cserver_rejects_a_session(terminal snapshot carries the rejection).just checkpasses except onepy/moq-rs/tests/test_socket.py::test_abort_and_endpoint_close_are_errorsrun (unroutableinstead ofClosed); the socket model does not use the reconnect policy, and a rerun passed. Follow-up below.Caused by
closed()whileconnected()still marked the policy established, so a refused first session redialed forever (found by reviewer; the feature review).Follow-ups
unroutableinstead ofClosedonce after an endpoint close. No open decisions.Models
Size: S
Implementer: Claude, claude-opus-5-5, unknown effort (arm A)
🤖 Generated with Claude Code
Review
Rewritten by
quest reviewafter each round.Head: 7e93c67
Base: origin/main at ca88fc1
Risk: medium,
the base has no RISK.md to name what is always high or customer-facing, so every change is mediumClassified: medium by default
Reviewer: Codex, gpt-6.1-sol, medium effort
Session:
01a1dfae-be9b-7371-b057-98b45fcd8185, round 1Score: 8/10,
The healthy-session threshold is applied consistently in Rust and JS with corresponding model, documentation, and test updates; no actionable defects were found by source inspection, but CI confirmation remains pending.Rounds: 1; first round 0; findings 0: fixed 0, refuted 0, open 0
Findings: none