Skip to content

fix(net): a relay answers a SUBSCRIBE on a copy it kept after a fetch - #7631

Merged
kixelated merged 9 commits into
mainfrom
quest/m1/relay-fetched-copy-subscribe
Nov 2, 2026
Merged

kixelated merged 9 commits into
mainfrom
quest/m1/relay-fetched-copy-subscribe

Conversation

@kixelated

@kixelated kixelated commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

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:

  1. Since feat(net)!: end bounded reads and owe cached backfill (fetch as a range, part 1) #7545: the relay's IETF publisher waits for the copy's live edge (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.
  2. Older (why the probe also hung on main before 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_live then parks the copy in Feed::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

  • A wire SUBSCRIBE asks for the live feed. track::Subscription gains a crate-private feed flag. The flag is aggregated as "any": a counter in Subscriptions, OR'd in the test-only Combined, and carried through clamped.
  • A feed that ends before the answer's Largest makes the copy current from the answer alone. TrackState::set_live now delegates to answered(largest, start, reaches), and the new set_live_ahead passes reaches: false. The copy then goes Live { edge: from }, so relative starts count back from the upstream's edge and never from the cache.
    • The subscriber's resume gate uses set_live_ahead when 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 keeps set_live.
  • A draft-18 join leaves a bounded end open instead of refusing it. Since a wire SUBSCRIBE now resumes a lingering copy whatever its range, a relay with a draft-18 upstream refused a history SUBSCRIBE's end in subscribe_join, and the resumed Target::fail aborted 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_join is infallible.
  • A resumed feed starts where it asked. The resume gate now passes the subscription's own absolute start (sent_bounds.0, or none for a live join, which every draft-18 join is) to set_live and set_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

  • Public API: none. Subscription is #[non_exhaustive] and the field is pub(crate). Aggregated track::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.
  • Wire: none. A relay now holds an upstream subscription for as long as a downstream moq-transport SUBSCRIBE lasts, even a history one, as it did before feat(net)!: end bounded reads and owe cached backfill (fetch as a range, part 1) #7545.
  • Tests in 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 the set_live_ahead fix.
    • older_drafts_always_join_live now covers a bounded end; older_drafts_refuse_a_bounded_end is deleted.
    • a_resumed_bounded_subscription_keeps_its_cached_start (draft 22, direct): the review's scenario. It fails with the start forced back to None.
  • Deletes the finished quest and its link from quest/m1/README.md.

Choices made without asking

  • A draft-18 bounded end is left open rather than refused. This reverses the refusal from moq-dev/moq#3727, which was written when a draft-18 join still sent an absolute joining FETCH. Since moq-dev/moq#6348 every draft-18 join is live, so an open end costs only the live groups past the reader's end until it leaves, while a refusal kills the copy for everyone. A fresh draft-18 subscription with an end now joins live instead of failing with Unsupported.
  • The direct bounded-resume tests stay draft 22 only. On draft 18 they hang for reasons that predate this PR (see Follow-ups): the range below the live join needs a multi-group fill, which our publisher refuses, and a live join's start raises the floor past a cached group.
  • A bounded read through an origin front still parks past its end (documented in MIGRATE.md), so the relay history test reads group 1 and stops, as a_bounded_read_of_history_fetches_without_subscribing does.
  • One narrow edge case is left unhandled: a resumed open feed narrowed to a bounded range after it already entered 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 ci on head 9162adb80f (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.
  • Each fix was checked by reverting it: the relay history test fails without the feed flag, both bounded-resume tests fail without set_live_ahead or the start fix. The live-SUBSCRIBE relay test passes either way; it is regression coverage.

Review

  • Reviewer: Codex (quest review), gpt-6.1-sol, medium effort.
  • Head 9162adb80f1bf37e9f21794092a5b670ac56e8df: no findings. This was round 1 of a new session, which quest review started because the merged base changed the reviewer's instructions.
  • Earlier session, rounds 1 and 2 (heads 42d579f903 and ba9f79ddb6): one finding, the bounded resume's cached start (model/track.rs:2857). It was fixed in ba9f79ddb6, with a_resumed_bounded_subscription_keeps_its_cached_start.
  • Online findings on ba9f79ddb6: the CodeRabbit thread on ietf/subscriber.rs:767 and the automated OpenAI review's P2 (a draft-18 bounded resume aborts the shared copy). Both were fixed in 58a8b4f837; the thread is replied to and resolved. Pullfrog reported no issues.

Follow-ups

  • A draft-18 resume with an earlier absolute start can hang. This also happens on main with an open range (with_range(1..)). Two causes:
    • The groups below the live join come from one fill of the run, and our moq-transport publisher answers only a one-group FETCH, so a run of two or more groups never arrives. A one-group range works.
    • The live join's start raises live_floor past a cached group (TrackState::answered), so that group is skipped.
  • Check the lite publisher too. It may wait the same way when it serves a bounded SUBSCRIBE from a moq-transport fetch-only copy. This PR covers only the moq-transport publisher.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

kixelated and others added 2 commits November 2, 2026 04:01
@kixelated kixelated changed the title A relay answers a SUBSCRIBE on a copy it kept after a fetch chore(quest): checkpoint retained-copy validation blocker Nov 2, 2026
kixelated and others added 4 commits November 2, 2026 09:40
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 kixelated changed the title chore(quest): checkpoint retained-copy validation blocker fix(net): a relay answers a SUBSCRIBE on a copy it kept after a fetch Nov 2, 2026
@kixelated
kixelated marked this pull request as ready for review November 2, 2026 18:47

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: ba9f79d

  • P2 — Contain unsupported bounded resumes to the requesting reader. rs/moq-net/src/ietf/publisher.rs:38 now 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 reaches subscribe_join, which rejects its finite end (ietf/subscriber.rs:12138–12140); Target::Resume::fail then 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)

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Recent review info
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: b4433d80-0cbf-409f-ac1a-047b9607626f
Commits

Reviewing files that changed from the base of the PR and between 998721a and ba9f79d.

Files selected for processing: 8
  • quest/m1/README.md
  • quest/m1/relay-fetched-copy-subscribe.md
  • rs/moq-net/src/ietf/publisher.rs
  • rs/moq-net/src/ietf/subscriber.rs
  • rs/moq-net/src/model/subscription.rs
  • rs/moq-net/src/model/track.rs
  • rs/moq-net/src/model/track/demand.rs
  • rs/moq-net/tests/integration/fetch_only.rs
Files with no reviewable changes: 2
  • quest/m1/README.md
  • quest/m1/relay-fetched-copy-subscribe.md

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 subscription model now carries and aggregates a live-feed preference. Subscription handling uses that preference and distinguishes upstream answers that reach the reported largest group from answers ahead of it. New integration tests cover fetched-copy history and live subscriptions in transport versions 18 and 22. The related quest entry and planning document were removed.

Merge Risk: Moderate · up to ba9f7

A relay serving a retained copy backed by a draft-18 upstream can now abort that copy when a downstream client requests a bounded range. This interrupts other readers of the same track. The change fixes the main fetched-copy SUBSCRIBE cases, but this mixed-version bounded case should be handled, or explicitly accepted, before merge.

Pre-merge checks: Passed 4 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Docstring Coverage Inconclusive Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: fixing relay responses for SUBSCRIBE requests on copies retained after a fetch.
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.
Description check Passed The pull request description directly explains the relay SUBSCRIBE issue, the implemented fixes, affected behavior, tests, and remaining follow-ups. It is clearly related to the changeset.
Full details: Docstring Coverage

Explanation

Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 9 functions across 3 files. (3 skipped: 3 too large.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
Simplify code
  • Commit to this branch
  • Create a new PR
  • 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

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Fix comments: Fix CodeRabbit comments on this PR

Comment thread rs/moq-net/src/ietf/subscriber.rs Resolved

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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::Subscription gains a crate-private feed flag, aggregated as "any" in Subscriptions/Combined, set by the IETF publisher's serving_subscription, and honored by wants_feed so a lingering copy resubscribes upstream whatever its ranges.
  • A feed ending before the answer's Largest makes the copy current: TrackState::set_live delegates to answered(largest, start, reaches), and the new set_live_ahead passes reaches: false, going Feed::Live at once instead of parking in Feed::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 selects set_live_ahead when 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.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

kixelated and others added 3 commits November 2, 2026 11:55
…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>
@kixelated

Copy link
Copy Markdown
Collaborator Author

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 kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

@kixelated
kixelated added this pull request to the merge queue Nov 2, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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_join is now infallible; a non-draft20 join returns the live NextObject+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 and wanted_bounds carries it.
  • The resume gate distinguishes a live join's open feed: set_live_ahead is now guarded by !live, with live = start.is_none() || join.fetch. Because live is exactly "the wire feed is open-ended" (always for draft-18; draft-22 start=None), a live join takes set_live and waits for the cache to show the answer's Largest, while a bounded draft-22 feed still goes Feed::Live at 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_live covers a bounded end and older_drafts_refuse_a_bounded_end is 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).

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Merged via the queue into main with commit d43ff43 Nov 2, 2026
9 of 10 checks passed
@kixelated
kixelated deleted the quest/m1/relay-fetched-copy-subscribe branch November 2, 2026 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant