Repository navigation
fix(net): a relay answers a SUBSCRIBE on a copy it kept after a fetch - #7631
Conversation
Quest-Claim: 1eca63ae47203bdd
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
A wire SUBSCRIBE reaching a relay's fetch-only copy hung with no SUBSCRIBE_OK, for two reasons: - The IETF publisher waits for the live edge before answering, but the lingering copy read a bounded history range as fetch-only and never subscribed upstream. A wire subscription now asks for the live feed (`Subscription::feed`, crate-private), so the relay subscribes again and answers from the upstream's SUBSCRIBE_OK. - A resumed subscription bounded below the answer's Largest Object waited forever for the cache to show that group. Such a feed now makes the copy current from the answer alone (`set_live_ahead`). Removes the finished quest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The answer's Largest stood in for where a resumed absolute feed starts, so a copy that went idle holding a group raised its floor past that group and a bounded resume of it could never read it. Pass the subscription's own start, or none for a live join. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: ba9f79d
- P2 — Contain unsupported bounded resumes to the requesting reader.
rs/moq-net/src/ietf/publisher.rs:38now forces bounded historical wire subscriptions to resume a retained copy. With a draft-22 downstream and draft-18 upstream, a range wholly below the reported edge now reachessubscribe_join, which rejects its finite end (ietf/subscriber.rs:12138–12140);Target::Resume::failthen aborts the shared track (:708–713). Thus a history request can destroy the retained copy for unrelated readers, rather than merely fail itself. This is the draft-18 abort already acknowledged in the PR's Follow-ups, not a separate duplicate finding; the mixed-version relay makes it reachable with our own draft-22 client too. Please reject only the unsupported reader, or satisfy the range without aborting the shared copy, and cover that mixed-version case with an independent history reader.
Direction: distinguishing wire answer demand from fetch-only demand and accepting an answered edge beyond a bounded feed are focused fixes. The earlier local-review cached-start finding appears fixed by passing sent_bounds.0 and its regression test. The shared-copy failure above should not be deferred as just missing draft-18 coverage.
Verification: static inspection of the diff and relevant publisher/subscriber/model paths; no tests executed or CI results independently verified.
(Written by OpenAI)
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run reviewed the fix for a relay failing to answer a SUBSCRIBE on a copy it retained after a FETCH, covering the new feed flag, the set_live_ahead resume path, and the added integration tests.
- A wire SUBSCRIBE requests the live feed:
track::Subscriptiongains a crate-privatefeedflag, aggregated as "any" inSubscriptions/Combined, set by the IETF publisher'sserving_subscription, and honored bywants_feedso a lingering copy resubscribes upstream whatever its ranges. - A feed ending before the answer's Largest makes the copy current:
TrackState::set_livedelegates toanswered(largest, start, reaches), and the newset_live_aheadpassesreaches: false, goingFeed::Liveat once instead of parking inFeed::Answered. - A resumed feed starts where it asked: the resume gate passes the subscription's own start (
sent_bounds.0, or none for a live join) and selectsset_live_aheadwhen the sent end is at or below the answer's Largest group. - Tests: history and live SUBSCRIBE through a relay on a fetched copy (drafts 18 and 22), plus direct bounded-resume coverage; deletes the finished quest and its README row with no stale references.
I traced the feed lifecycle end to end (aggregation, clamped, the with_* builders, Preferences, and the test-only Combined) and attacked the set_live_ahead/answered state machine with two independent specialists; neither found a reachable new fault. The only residual is a public-API subtlety the PR body already documents: Subscription derives PartialEq and now carries the hidden feed, so an aggregate returned by Producer::subscription() can differ from an application-built value when a wire subscriber is present. No in-tree consumer compares whole Subscription values, so nothing breaks today.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
…ng it A wire SUBSCRIBE now resumes a lingering copy whatever its range, so a relay speaking draft 18 upstream refused a downstream history SUBSCRIBE's end in subscribe_join and aborted the shared copy for every reader. A draft-18 join is live anyway: earlier groups come from fills and readers stop at their own end, so the end stays local, as a later start already does. subscribe_join is now infallible. The resume gate also answers a live join's feed with set_live, since its feed has no end on the wire and so reaches the answer's Largest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Re the automated OpenAI review (P2: contain unsupported bounded resumes to the requesting reader): fixed in 58a8b4f, the same root cause as the CodeRabbit thread. On drafts before 20, a join now leaves a bounded end open instead of refusing it. So a resume of a lingering copy over a draft-18 upstream no longer aborts it for other readers. Readers stop at their own end, and earlier groups come from fills. The relay history test now covers a draft-18 upstream with draft-22 and draft-18 downstreams, plus a second reader of the same copy. That also clears the "draft 18 aborts a lingering copy on a bounded resume" follow-up this PR used to list. (Written by Claude Opus 5.5) |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 9162adb
The P2 shared-copy abort in my previous review appears fixed: draft-18 bounded requests now use the existing open live join, retaining bounds locally instead of rejecting and aborting the shared copy. Restricting set_live_ahead to non-live joins matches that wire behavior. The expanded relay test covers 18→22 and 18→18 and checks another reader can still FETCH afterward.
No new actionable bugs found in this update. Reusing the existing join/fill path is a focused, simpler direction than adding separate resume machinery. Compared the substantive fix against the prior reviewed head, separating merged base changes; unchanged findings were not treated as new.
Verification: static inspection of the changed subscriber logic, relevant surrounding paths, and test assertions only. Tests were not executed and CI was not independently verified.
(Written by OpenAI)
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the delta since the prior pullfrog review (ba9f79ddb): commit 58a8b4f83, which leaves a draft-18 bounded end open instead of refusing it, plus the main merge.
- A draft-18 join no longer refuses a bounded end:
subscribe_joinis now infallible; a non-draft20 join returns the liveNextObject+fetch join whatever its start or end, so a mixed-version history SUBSCRIBE resumes the shared copy instead of aborting it for every reader (fixes the OpenAI P2). The end stays local: readers stop at their own range andwanted_boundscarries it. - The resume gate distinguishes a live join's open feed:
set_live_aheadis now guarded by!live, withlive = start.is_none() || join.fetch. Becauseliveis exactly "the wire feed is open-ended" (always for draft-18; draft-22start=None), a live join takesset_liveand waits for the cache to show the answer's Largest, while a bounded draft-22 feed still goesFeed::Liveat once. - Tests: the fetched-copy history test now runs 22→22, 18→22, and 18→18 and asserts the copy still serves its fetching reader;
older_drafts_always_join_livecovers a bounded end andolder_drafts_refuse_a_bounded_endis deleted.
I traced the live/subscribe_join/resume-gate paths and confirmed no other caller depended on the removed Error::Unsupported (the update-path End::Invalid arm was draft20-only, so it was dead). The remaining gaps are the author-documented ones (narrowing after Feed::Answered; a draft-18 resume with an earlier absolute start), out of scope here. Local test execution was not possible in this environment (a transative dev-dependency needs the libpipewire-0.3 system library, which is absent).
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Problem
Finishes the retained fetched-copy SUBSCRIBE quest, a follow-up of moq-dev/moq#7545.
A relay fetches group 2 of a live track, keeping a copy no subscription feeds. A fresh draft-22 session then sends SUBSCRIBE
with_range(1..2)through the relay and never gets SUBSCRIBE_OK. I reproduced it before changing anything. Two separate bugs cause the hang:poll_live) before answering. But the lingering copy judges a bounded range below the reported edge as history (wants_feed), so it never subscribes upstream and the copy never goes live.mainbefore feat(net)!: end bounded reads and owe cached backfill (fetch as a range, part 1) #7545): with the relay forced to subscribe upstream, it gets SUBSCRIBE_OK with Largest (2, 1).set_livethen parks the copy inFeed::Answered { from: 2 }until a live group 2 shows. The fetched group 2 is backfill, and the bounded upstream subscription never sends group 2, so the copy never goes live.Approach
track::Subscriptiongains a crate-privatefeedflag. The flag is aggregated as "any": a counter inSubscriptions, OR'd in the test-onlyCombined, and carried throughclamped.serving_subscriptionsets it, andwants_feedhonors it.a_relay_serves_a_bounded_read_of_history_without_subscribing_upstreamstill passes.TrackState::set_livenow delegates toanswered(largest, start, reaches), and the newset_live_aheadpassesreaches: false. The copy then goesLive { edge: from }, so relative starts count back from the upstream's edge and never from the cache.set_live_aheadwhen a non-live join's sent end is at or below the answer's Largest group. A live join's feed is open on the wire, so it reaches that group and keepsset_live.subscribe_join, and the resumedTarget::failaborted the shared copy for every reader (CodeRabbit and the automated OpenAI review). A draft-18 join is already live: earlier groups come from fills and a later start is skipped locally, so the end now stays local too.subscribe_joinis infallible.sent_bounds.0, or none for a live join, which every draft-18 join is) toset_liveandset_live_ahead. Before, the answer's Largest stood in, which raised the floor past a cached group the resume asked for. The local review found this.Impact
Subscriptionis#[non_exhaustive]and the field ispub(crate). Aggregatedtrack::Producer::subscription()values now also differ by the hidden flag when a wire subscriber is present, so whole-value equality against a value an application built can differ. Every in-tree comparison compares fields, and the whole suite passes.rs/moq-net/tests/integration/fetch_only.rs:a_relay_answers_a_history_subscribe_on_a_fetched_copy(upstream→downstream 22→22, 18→22, 18→18): the quest's repro. The edge moves past the fetch's report first. The test asserts that the relay subscribed upstream and that the copy still serves its other reader afterwards. 22→22 fails with either of the first two fixes reverted, and 18→22 fails with the old draft-18 end refusal, which aborts the copy.a_relay_answers_a_live_subscribe_on_a_fetched_copy(drafts 18 and 22): the live join starts at the upstream's current group, not the one the fetch reported. This is regression coverage and passed before the fix.a_resumed_bounded_subscription_reads_below_the_answered_edge(draft 22, direct): a bounded resume past the reported edge, with no relay. It fails without theset_live_aheadfix.older_drafts_always_join_livenow covers a bounded end;older_drafts_refuse_a_bounded_endis deleted.a_resumed_bounded_subscription_keeps_its_cached_start(draft 22, direct): the review's scenario. It fails with the start forced back toNone.quest/m1/README.md.Choices made without asking
Unsupported.MIGRATE.md), so the relay history test reads group 1 and stops, asa_bounded_read_of_history_fetches_without_subscribingdoes.Feed::Answered. The gate decides once, at the answer, so a narrowing that lands while the copy waits there would still wait. No test reaches that window.Validation
quest gate nix develop --command just cion head9162adb80f(main merged through moq-dev/moq#7489): passed (9474 tests, plus rs/markdown/gh/quest/shell/toml/nix/justfile checks).quest check --base origin/main: OK.feedflag, both bounded-resume tests fail withoutset_live_aheador the start fix. The live-SUBSCRIBE relay test passes either way; it is regression coverage.Review
quest review), gpt-6.1-sol, medium effort.9162adb80f1bf37e9f21794092a5b670ac56e8df: no findings. This was round 1 of a new session, whichquest reviewstarted because the merged base changed the reviewer's instructions.42d579f903andba9f79ddb6): one finding, the bounded resume's cached start (model/track.rs:2857). It was fixed inba9f79ddb6, witha_resumed_bounded_subscription_keeps_its_cached_start.ba9f79ddb6: the CodeRabbit thread onietf/subscriber.rs:767and the automated OpenAI review's P2 (a draft-18 bounded resume aborts the shared copy). Both were fixed in58a8b4f837; the thread is replied to and resolved. Pullfrog reported no issues.Follow-ups
with_range(1..)). Two causes:live_floorpast a cached group (TrackState::answered), so that group is skipped.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code