Repository navigation
test(net): model failover along a relay chain, and pin two fetch bugs it found - #8482
Conversation
Quest-Claim: 870e6ba70794c290
…fetch bugs it found FailoverChain.tla composes route changes, resume switches, and hop delivery across a client, two relays deep, with frames and Recover. The chain passes; with a mesh hop it finds that an ended relay cannot move a fetch off a dying route. Replaying its paths on real sessions pins two code bugs where a fetch for the rest of a half-read group fails or never leaves the relay once the track ends. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pre-merge checks:
|
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed 006ed54, including all nine changed files and the referenced recovery/front and integration-harness paths. No additional actionable findings.
Direction: the separate stay/leave configurations avoid letting an available unsubscribe hide a playback deadlock. Frame recovery and clean-end checks complement the existing whole-group models. The control replay and two explicitly named residual tests distinguish successful recovery from the known end-of-track fetch failures; these are documented existing defects, not fixes delivered here. The mesh counterexample remains explicitly unreplayed.
Verification limits: GitHub-only static review. No Rust tests or TLC runs performed, and reported state counts, timings, and failures were not independently reproduced. The model uses bounded, abstract routes and deadlock checks, not an unrestricted temporal-liveness proof; the documented model/code differences remain.
There was a problem hiding this comment.
Important
The model's rule table claims coverage the spec does not have: FailsOnlyUnroutable and NoSkipWhileHeld are entailed by the step definitions and can never fail, in any config. The replay tests themselves are sound — 3/3 pass, stable across five simulator seeds.
Reviewed changes
I read the whole diff, ran cargo test -p moq-net --test integration --features async failover_chain (3/3 pass, 0.02 s, and again across MOQ_SIM_SEED 1/7/42/123/999), and checked the new files against the sh/text.sh gate.
- Failover chain model —
FailoverChain.tlacomposes route, resume, and subscribe as one step each and adds frames, so a half-read group continues at the same frame from another copy or by fetching from the serving route. - Three configs —
FailoverChain.cfg(client stays, so a deadlock is a playback hang) andLeave.cfg(client may leave, so a deadlock is a retained copy) are green;EndedRelay.cfgis committed known-red on the ended-relay finding. - Replay tests —
failover_chain.rsbuilds P→A/B→E over the memory transport with a simulated clock, aborts the A↔E session, and pins the two fetch bugs asresidual_*tests, with a control test for the same path without the track end. - Skill —
verify-failoverdocuments the rules, run times, open findings, coverage, and scope in the shape of the four sibling models; the finished quest file and its README line are removed.
The residual_* naming, the weak is_err() assertion (AGENTS forbids matching on errors), and the 500 ms settle() all hold up: the control test discriminates the setup, the abort forces the same switch as the price change, and 30 s cannot be reached by the Old grace, so the second residual can only pass by hanging.
Reviewed the complete diff at 006ed54; 9 files, 3 commits. Tests in this run: failover_chain 3/3 across five seeds.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏
|
Heads-up from moq-dev/moq#8483. It runs every TLA+ config in Nightly through (Written by Opus 5.5 with medium) |
|
One more from moq-dev/moq#8483. (Written by Opus 5.5 with medium) |
…in-model # Conflicts: # quest/m1/README.md
Abort no longer needs the whole track to have failed, so NoSkipWhileHeld can break: EndedRelay.cfg now fails on it instead of deadlocking. Drop FailsOnlyUnroutable, which FrontFail's guard entails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the delta since the prior pullfrog review at 006ed54: the Abort guard, the dropped rule, and the matching doc updates.
Abortcan fail a group while its track lives — a short group now fails only when its source is dead or gave up, no other copy can continue it, and no fetch can reach the rest (the front has no route, ended with its serving copy gone, or its live serving copy gave up on the group). This mirrorsRecover::poll's give-up paths and makesNoSkipWhileHeldfalsifiable.FailsOnlyUnroutableremoved — it was entailed byFrontFail's own guard in every config; the skill's Not covered section now records that the client's track fails only through that step.EndedRelay.cfgnow fails onNoSkipWhileHeld— the mesh counterexample reports the rule instead of a deadlock, so the skill's attribution of the first residual to that rule holds.- Docs updated to match — the
Abortcomment, the rules table, the open-finding text, and the run notes (depth 31, about 50M distinct states).
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

Finishes quest/m1/failover-chain-model.md.
Problem
Each model from moq-dev/moq#8447 checks one layer alone: routing (
verify-route), a front switching copies (verify-resume), and a subscription's accounting (verify-subscribe). Resume failover keeps breaking where those layers meet: a relay's front switches routes while the relay above it is resuming too, and a half-read group has to continue across both.Approach
rs/moq-net/verify/failover/FailoverChain.tlamodels one track read by a clientCthrough relayE.Estarts on relayAand may fail over to relayB; both pull from the originO. WithMesh,Bmay also route throughA. Each layer is cut down to the steps its own model checked: a route change is one step, a switch asks the new copy from the reader's newest group or lowest gap, and a handed-out group crosses a hop unless its session dies. What the chain adds is frames. A group whose copy fails or stalls mid-read continues at the same frame (Recover): from another copy, from the serving copy, or by fetching the rest from the serving route, which asks its own upstream in turn.Rules:
CleanEndIsComplete,CleanEndIsDeclared,NoSkipWhileHeld,SettledIsWhole, and TLC's deadlock check asSettles.Settlesmeans no hang, and once the client leaves, no relay keeps a copy. The newverify-failoverskill documents them, with run times, the open findings, a Coverage table, and the scope.FailoverChain.cfgpasses: about 2.0M states, with one death and one switch per relay. The client stays, so a deadlock there is a playback hang.Leave.cfgpasses: about 2.2M states. The client may leave at any point, so a deadlock there is a relay still holding a copy. Each takes under 30 s on 24 cores.EndedRelay.cfgadds the mesh hop and failsNoSkipWhileHeld, at depth 31 after about 50M distinct states (11 min on 8 workers).Bfinishes its track, then its route throughAdies withE's fetch for the rest of group 0 still pending.B's front no longer switches, so the fetch fails althoughBstill reachesOdirectly, andEcuts the client's group short.Mutation checks show the rules catch real breaks: dropping
backfill_gapsbreaksCleanEndIsComplete, disablingRecoverdeadlocks, and disabling a relay's park deadlocks once the client leaves.Found on the real stack
Replaying those paths over real sessions (
rs/moq-net/tests/integration/failover_chain.rs, memory transport, simulated clock) found two bugs. In both, the code falls short of what the model allows. Each is pinned as aresidual_*test that passes while the bug stands. A control test,a_half_read_group_continues_through_the_new_route, runs the same path without the end and passes.residual_ending_the_track_fails_a_fetch_in_flight:E's fetch for the rest of group 0 is in flight throughBwhen the publisher ends the track.B's copy, asked from group 1, completes, andB's fetch from the publisher is cancelled.E's read of group 0 then fails withNotFound, even though every relay still reaches the publisher and the publisher writes the rest.residual_a_fetch_from_a_completed_copy_never_leaves_the_relay: the same group, read further only afterE's serving copy fromBcompleted.Recover::poll_fetchcallsfetch_groupon that copy, but no FETCH goes on the wire, so the read waits out the subscriber's budget.The mesh finding has no replay of its own yet: the real stack breaks the same rule earlier, on the first bug.
Impact
Validation
just check: every test passed exceptmoq-relay config::tests::repository_configs_load. That test fails becausetest/audio-quality/congestion.tomlonmainhas a field the relay config rejects (unknown field down). This branch touches neither file, so the failure predates it, and moq-dev/moq#8486 fixes it.failover_chainintegration tests passed 5 runs out of 5. TLC passesFailoverChain.cfgandLeave.cfg, and failsEndedRelay.cfgon its open finding.Decisions
Abort): when no other copy holds it and no fetch can reach the rest, because the front has no route, ended with its serving copy gone, or its live serving copy gave up on the group. Before,Abortneeded the whole track to have failed, soNoSkipWhileHeldcould never break (pullfrog's finding). ✅ makeAbortmatchRecover::pollgiving up (rejected: keep the guard and document the rules as out of reach).FailsOnlyUnroutableis dropped: the client's track fails only throughFrontFail, whose guard is the rule, so it could not break. ✅ drop it and say so under Not covered (rejected: keep it as a tautology).Choices made without asking
verify-failoverskill, matching the four sibling models.Related
Follow-ups
EndedRelay.cfg's path and fix how an ended relay moves a fetch off a dying route. Recommend: plan one m1 quest for all three; they share the fetch path and the replay harness.EndedRelay.cfgtakes 11 minutes. Recommend: no separate quest; the nightly models quest picks it up as an open-finding config.Models
Size: L
Implementer: Claude Code, claude-opus-5-5, high effort
Implementer: Claude Code, claude-opus-5-5, medium effort (iterate, S)
🤖 Generated with Claude Code
Review
Rewritten by
quest reviewafter each round.01a1c852-f40c-78c0-8b58-1532e5ec8a6e, round 3The incremental changes let group failures surface independently of track failure and update the documented counterexample consistently, with no new findings from source-only review; no builds or tests were run.(Written by Claude Opus 5.5 with medium)