Skip to content

feat(net): send SUBSCRIBE alongside TRACK - #7328

Merged
moq-bot[bot] merged 23 commits into
mainfrom
quest/m2/pipeline-requests/subscribe
Nov 1, 2026
Merged

moq-bot[bot] merged 23 commits into
mainfrom
quest/m2/pipeline-requests/subscribe

Conversation

@kixelated

@kixelated kixelated commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

On moq-lite 05 and later, a subscriber now opens its TRACK and SUBSCRIBE streams together instead of waiting for TRACK_INFO before it sends SUBSCRIBE. This holds in Rust and JS, at every hop: the viewer's session and each relay's upstream session. First data arrives one round trip sooner per hop. Lite-03/04 have no track stream and are unchanged.

Finishes quest/m2/pipeline-requests/subscribe.md, following its Plan (decided 2026-10-08). It is the epic's last child, since the first FETCH landed in moq-dev/moq#7332, so this PR also deletes quest/m2/pipeline-requests/README.md. The epic's own measurement is the table below plus fetch_pipeline::the_first_fetch_takes_one_round_trip_per_hop, now run over two relays as well.

Changes

Rust subscriber (rs/moq-net/src/lite/subscriber.rs)

  • TrackServeRun's Info phase opens the TRACK and then a Pipelined SUBSCRIBE, built from the request's demand. The subscription is registered under its id before it is written, with its copy pending (TrackEntry::copy: None).
  • A group stream that beats TRACK_INFO parks unread after its header (Parked), so flow control bounds it. On accept, the copy opens every parked group at once, in arrival order, so it receives them exactly as it would have without the wait. Each stream then fills its own group. Datagrams that beat TRACK_INFO are dropped.
  • Parked streams resume only after the responses already buffered on the SUBSCRIBE stream have been read, so a SUBSCRIBE_START still precedes its first group. Without this, a rejoin test fails.
  • A TRACK that fails or is reset while its SUBSCRIBE is live rejects the request, so the origin fails over, and resets the SUBSCRIBE. The same happens for revocation, abandonment and session end.
  • Accepting TRACK_INFO marks the pipelined demand as already sent (track::Producer::seen_subscription), so only a later change goes out as SUBSCRIBE_UPDATE.
  • On a closed session, streams parked on a TRACK_INFO are drained after their source reads the info. A source does not end a subscription while parked streams it already accepted remain unread.
  • The same holds when the upstream SUBSCRIBE fails (ServeMode::Failed). A lite-06 publisher resets the SUBSCRIBE with an errored end right after the cached groups, so that reset can land beside TRACK_INFO, ahead of the parked groups. The parked groups are now delivered before the track is given back. Before, errored_end::an_errored_end_keeps_its_history_across_a_hop lost the history once moq-dev/moq#7349 merged.
  • The failed drain waits until the subscription's tail has no stream still being read (Tail::is_reading), parked or not, before it gives the track back. Before, it stopped as soon as the last parked stream took its group, so a payload or FIN still in flight was cut short. Streams that never parked were cut the same way, as origin/main gives the track back on the error at once. The tail already holds this rule: a group ends on its own stream's FIN or reset, not because its track ended. The drain lasts only while someone holds the copy: once nobody does, it takes the idle teardown, as the tail does, so a stream the publisher never finishes cannot hold it.
  • The Info phase carries both the pipelined SUBSCRIBE and the fetches that moq-dev/moq#7332 sends alongside the TRACK. The TRACK still opens first. Failure, revocation, abandonment and drop each end all three. ServeLoop::adopt (feat(net): send the first FETCH alongside the track's info request #7332's constructor) builds the loop, and adopt_pipelined hands it the SUBSCRIBE.

Model (rs/moq-net/src/model/track.rs): the aggregate demand (Producer::subscription, Request::subscription, their change polls) is capped only by the local cache ceiling (demand_bound), not by TRACK_INFO's max age. Per-reader lateness and the drift check keep max_age_bound(). Docs reworded.

Rust publisher (rs/moq-net/src/lite/publisher.rs, coding/stream.rs)

  • TRACK_INFO goes out at TRACK_INFO_SEND_ORDER, one above every group stream. On grouping transports, ungrouped control already went first. This makes it strict where streams rank by send order alone.
  • On lite-05+, a SUBSCRIBE that arrives before its track resolves takes the latest group once the track resolves. Before, it fell back to 0, which made a pre-06 "no start" replay the whole cache.

Tail grace (model/track.rs Producer::tail_grace, lite/subscriber.rs; JS track.ts #grace, hooks.tailGrace): the aggregate asked upstream is no longer clamped to max age, but the session's tail (and in JS the track's arrival ledger) still waits for a late group no longer than the publisher keeps one. A pipelined SUBSCRIBE takes that bound once its copy is accepted.

JS publisher (js/net/src/lite/publisher.ts): a SUBSCRIBE that arrives before its track resolves, on any pre-06 draft, re-reads the newest group once it resolves, as the Rust publisher does on lite-05+. This finishes quest/m1/js-early-latest.md.

JS (js/net/src/lite/subscriber.ts, track.ts): the same changes. The SUBSCRIBE is written while the TRACK exchange is in flight. A group stream for a pending id waits on #pending unread, and promise order keeps arrival order. A pending id's datagrams are dropped. A failed TRACK fails the setup and resets the SUBSCRIBE. An answered TRACK is held as soon as TRACK_INFO lands, so a cancellation while the SUBSCRIBE create is still parked closes it. The aggregate is no longer clamped to maxAge. The JS publisher already sends TRACK_INFO first, since group streams use negative send orders.

Measured

Time to first frame in the simulated mock (10 ms each way, so a 20 ms RTT per hop), from subscribe_with_track::first_frame_takes_one_round_trip_per_hop, run on origin/main and on this branch:

hops main this PR
direct 40 ms 20 ms
1 relay 80 ms 40 ms
2 relays 120 ms 60 ms

The numbers are identical across lite-05, 06 and 07. The test asserts one RTT per hop.

Tests

  • Rust unit tests:
    • SUBSCRIBE is on the wire beside the TRACK, before TRACK_INFO.
    • Accepting TRACK_INFO (max age 1 s, subscriber 10 s) sends no SUBSCRIBE_UPDATE.
    • A reset TRACK rejects the request and resets its SUBSCRIBE.
    • Group streams that beat TRACK_INFO stay unread and open in arrival order on accept.
    • TRACK_INFO's send order outranks the top group.
    • The aggregate is not clamped to max age before or after accept.
  • Rust integration (subscribe_with_track.rs): lite-03 to 07 over 0, 1 and 2 relays. Arrivals at the first hop are together, serial (SUBSCRIBE one RTT after TRACK, as from a serial subscriber), and SUBSCRIBE-first (the publisher's groups beat its TRACK_INFO). Each reads a cached group, then a live one. Two new mock knobs drive the arrivals: release_oldest_bidi and release_bidis_reversed.
  • JS unit tests: the SUBSCRIBE goes out with its track and no update follows the accept; a group that beats TRACK_INFO stays untouched until it lands; a reset TRACK rejects and resets the SUBSCRIBE. These fail on the old code. A datagram is read with the entry it was decoded with. A withdrawal while the SUBSCRIBE create is parked closes the answered TRACK. A JS publisher serving a track that resolves after the SUBSCRIBE joins at the newest group (lite-01, 03, 05). The tail grace is bounded by max age (JS and Rust).
  • errored_end::an_errored_end_keeps_its_history_across_a_hop holds every group's frames and FIN until after the reset (split_unis), with the SUBSCRIBE arriving both beside and before the TRACK, and reads every frame. On lite-06 both orderings lost every group before the drain fix.
  • fetch_pipeline::the_first_fetch_takes_one_round_trip_per_hop now runs over 0, 1 and 2 relays and passes on each: one 100 ms RTT per hop, on lite-05, lite-07, moq-transport-18 and moq-transport-22.
  • lite::subscriber::tests::a_failed_subscription_drains_only_while_wanted: a failed subscription holds the track for a stream still being read while a reader holds the copy, and releases it once none does.
  • Checks: just check origin/main passes on f1e690d6d1: 9347 Rust tests, plus the JS tests and lints. One bench/relay.test.py case lost a port race on the busy host (Address already in use) and passed on rerun. just test interop --all passed on ff6aaede62 (Rust ↔ JS node/bun) and was not rerun; the changes since are Rust-subscriber-only.

Choices made without asking

  • Time anchors wait for the info. A lite-07 SUBSCRIBE with a time anchor carries it in the track's timescale, which only TRACK_INFO gives. The Plan's "no SUBSCRIBE field needs TRACK_INFO" misses this case. Such a subscription stays serial; the change is ordering only, with no wire change.
  • TRACK_INFO that already arrived survives a withdrawal. A withdrawn route now fails only a TRACK_INFO still owed. One already delivered is read, as one read before the withdrawal stays usable. This keeps finished_broadcast_mock delivering when the publisher ends the broadcast right after it sees the SUBSCRIBE.
  • Tail grace stays bounded by max age. The stopgap grace for owed groups reads the aggregate, bounded locally by the track's max age as before, so the wire demand alone changed. (Changed at merge after a local review finding: unbounded, a subscriber deferring to the max age waited forever for a group lost at the tail.)
  • The JS early-latest fix covers lite-01..05. Rust's publisher re-reads the newest group only on lite-05+. quest/m1/js-early-latest.md (maintainer, 2026-10-30) scoped the JS fix to every pre-06 draft, so JS follows it.
  • JS has no reorder tests. Its mock always delivers streams in creation order. The JS paths are covered by unit tests, and cross-language orderings by interop.
  • Tests updated for the new ordering:
    • JS "nothing past the TRACK reached the wire" tests now expect the SUBSCRIBE and its reset.
    • The request-deadline fake gives each open its own stream.
    • finished_broadcast_mock waits for its subscribe to resolve before ending.
    • A relay auth test reads until the live group, since the latest cached group may now come first.
  • Merged origin/main at 802cadc3cc by hand. Seven files conflicted. The Rust subscriber is covered above. The other resolutions:
    • The moq_clock → kio::clock rename is applied to this PR's code.
    • The datagram paths take main's padding None / undefined while reading the copy's framing.
    • The session-end and covered-tail paths take main's lite-05+ end rules while keeping this PR's tail_grace and unparked wait.
    • Both quest files are deleted, and so is the now-empty epic.

Decisions

  1. Early data can use up the connection window that TRACK_INFO needs (local Codex review, round 1). Suppose SUBSCRIBE reaches the publisher before TRACK, and the publisher replays more cached group data than the subscriber's connection receive window before TRACK arrives. The parked group streams then hold all the connection credit. They wait unread for TRACK_INFO, and TRACK_INFO cannot arrive without credit. Rust and JS both stall for as long as a reader waits. moq-quic's stuck-window eviction does not step in, because unread streams count as not yet looked at. origin/main already has the same dependency for the first FETCH (moq-dev/moq#7332). Both follow the epic Plan's rule that "streamed data that arrives before the info stays unread in QUIC until the info lands". It needs TRACK and SUBSCRIBE reordered on the network plus a replay larger than the window in about one RTO. Otherwise TRACK_INFO goes out first, at TRACK_INFO_SEND_ORDER. Decided by the maintainer on 2026-11-01:
    • ✅ (a) Land as is, and plan a follow-up quest that fixes FETCH and SUBSCRIBE together. The hazard is rare and already ships for FETCH. A real fix changes the shared design, so it does not belong in a merge.
    • (b) Bound the parked data here. Past a per-session budget, stop the newest parked stream. That frees its credit and loses its group, which the tail then waits out. This covers only SUBSCRIBE, and FETCH keeps the hazard.
    • (c) Publisher hold. A lite-05+ publisher holds a subscription's group streams until it has sent that track's TRACK_INFO on the session. This changes wire behavior and needs a draft note. It also only helps when TRACK arrives at all.
  2. Cached datagrams replayed to a pipelined hop are lost. The Plan drops datagrams that beat TRACK_INFO. For a datagram track the publisher has cached, the replay on SUBSCRIBE always beats TRACK_INFO, so a relay's first subscription loses all of it. ietf::subscriber::stitch_tests::passthrough_preserves_datagram_objects_through_dispatch (hop variant) lost all 3 objects. I made its hop resolve the track's info first, so it still covers dispatch. Decided by the maintainer on 2026-10-30:
    • ✅ (a) Accept it. Datagrams are best effort, and the Plan rejected buffering. Live datagram tracks lose only what precedes TRACK_INFO.
    • (b) Publisher holds back datagrams. A subscription's datagrams wait until its TRACK_INFO is sent on the session. This narrows the loss but does not close it: the receiver can still read the datagram before the stream data.
    • (c) Stay serial for datagram tracks. Not possible: the subscriber doesn't know a track carries datagrams before TRACK_INFO.

Follow-ups

  • The Rust publisher re-reads the newest group of an unresolved track only for lite-05+ subscribers (lite/publisher.rs, SubscribeServe). A Rust relay serving a lite-03/04 subscriber from a copy still resolving may still replay from 0, where JS no longer does.
  • TRACK_INFO always gets connection credit while FETCH and SUBSCRIBE data wait unread for it, in Rust and JS (Decision 1 above, maintainer 2026-11-01). Plan it as a quest.

Impact

  • Public API: none. track::Producer::subscription and Request::subscription (Rust) and Producer.subscription (JS) report the subscribers' max delay capped by the cache ceiling, no longer by the track's max age.
  • Wire: none. Ordering only, which the draft already permits.

Review

Codex gpt-6-astra, medium effort (quest review --deep), round 2 on head f1e690d6d11ce2600fbdd7210974255a4b4e5f7c: no findings.

  • Round 1, on 2b9b38f333 (new session, the fix for the online thread): observe abandonment while draining a failed subscription. A stream the publisher never finishes held the failed subscription, its copy and stream after every reader left. Fixed in 23c135ccb9: the drain polls the fetches and takes the idle teardown once the copy is unwanted, as the tail does. Covered by a_failed_subscription_drains_only_while_wanted.
  • Round 2, on f1e690d6d1 (that fix, a conflict-free origin/main merge, and formatting): no findings.

Rounds before the drain fix, on this merge:

  • Codex gpt-6-astra, round 2 on 1400eb58b5: no findings.
  • Round 1, on ff6aaede62 (new session for this merge): connection credit while waiting for TRACK_INFO. Group data parked unread can use up the connection window and keep TRACK_INFO out. Refuted for this PR, citing the recorded maintainer decision (the epic Plan: early stream data stays unread in QUIC until the info lands) and the identical dependency in the FETCH path already on main (moq-dev/moq#7332). The maintainer decided it as Decision 1 above: land as is, with a follow-up.
  • Round 2, on 1400eb58b5 (round 1's head plus a conflict-free origin/main merge), with that response: no findings.

Earlier rounds, before this merge:

  • The land fixer's mechanical merge, f19119509e: Merged base and mechanically reconciled subscriber, default-delay test, hook, and clock conflicts while preserving both sides intent.

  • Codex gpt-6.1-sol, medium effort, round 3 on eefbf7d429c585e910fd6fd12376fa9c07f77ad6: no findings.

  • Round 1, on 37e74667d8 (new session at merge): the tail grace and the JS arrival ledger read the aggregate, no longer bounded by max age, so a subscriber deferring to max age could wait forever for a group lost at the tail. Fixed in 8ed41a964c: track::Producer::tail_grace (Rust) and hooks.tailGrace (JS) bound it by max age, and a pipelined SUBSCRIBE takes the bound on adopt.

  • Round 2, on 8ed41a964c: no findings.

  • Round 3, on eefbf7d429: the origin/main merge, with a hand-resolved conflict (reject_unused renamed reject_unwanted), and the JS floor extended to lite-01..05. No findings.

  • Earlier session, round 1 on 8ce1d4edd8: a JS datagram decoded before its subscription registered could be delivered with default framing. Fixed in a1f37497e1. Round 2 on a1f37497e1: no findings.

Online review threads, all fixed and resolved:

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

kixelated and others added 7 commits October 30, 2026 02:58
A lite-05+ subscriber opens its SUBSCRIBE with the TRACK instead of a round
trip after TRACK_INFO. Group streams that beat the info park unread after
their header and open in arrival order on the accept. The aggregate demand is
capped only by the cache ceiling, so accepting the info sends no update, and
TRACK_INFO goes out ahead of every group stream.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A lite-05+ subscriber writes its SUBSCRIBE while its TRACK is in flight. A
group stream that beats the TRACK_INFO waits for it unread, a datagram is
dropped, and a failed TRACK rejects the request and resets the SUBSCRIBE. The
aggregate demand is no longer clamped to the track's maxAge, so accepting the
info sends no update.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title SUBSCRIBE goes out with TRACK feat(net): send SUBSCRIBE alongside TRACK Oct 30, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

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 14 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: 29730457-2c79-46aa-82d0-3d57c3b6e326
Commits

Reviewing files that changed from the base of the PR and between 3164bce and f1e690d.

Files selected for processing: 28
  • js/net/src/internal.ts
  • js/net/src/lite/publisher.test.ts
  • js/net/src/lite/publisher.ts
  • js/net/src/lite/subscriber.test.ts
  • js/net/src/lite/subscriber.ts
  • js/net/src/request-deadline.test.ts
  • js/net/src/track.test.ts
  • js/net/src/track.ts
  • quest/m1/README.md
  • quest/m1/js-early-latest.md
  • quest/m2/README.md
  • quest/m2/pipeline-requests/README.md
  • quest/m2/pipeline-requests/subscribe.md
  • rs/moq-net/src/coding/stream.rs
  • rs/moq-net/src/coding/writer.rs
  • rs/moq-net/src/ietf/subscriber.rs
  • rs/moq-net/src/lite/publisher.rs
  • rs/moq-net/src/lite/subscriber.rs
  • rs/moq-net/src/lite/test_transport.rs
  • rs/moq-net/src/model/track.rs
  • rs/moq-net/src/tail.rs
  • rs/moq-net/tests/integration/errored_end.rs
  • rs/moq-net/tests/integration/fetch_pipeline.rs
  • rs/moq-net/tests/integration/finished_broadcast_mock.rs
  • rs/moq-net/tests/integration/main.rs
  • rs/moq-net/tests/integration/subscribe_with_track.rs
  • rs/moq-net/tests/integration/support/mock.rs
  • rs/moq-relay/tests/integration/auth_lifetime.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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-30T20:15:04.100736Z a1f3749 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a1f37497e1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread js/net/src/lite/subscriber.ts Resolved

@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: a1f3749

One additional P2 finding below. The JS lite-05 join-floor regression is also still open; I agree with the existing independent finding, rather than duplicating it inline.

Direction: removing the extra per-hop round trip is worthwhile; keeping early groups unread is preferable to buffering their payloads. The Rust ordering/lifetime machinery is justified by that constraint, but the JS setup must retain ownership of each completed exchange independently.

Verification: static inspection of the changed Rust/JS paths, surrounding lifecycle/model code, and test changes via GitHub. I did not run tests or reproduce the timing case locally.

(Written by OpenAI)

Comment thread js/net/src/lite/subscriber.ts Resolved
kixelated and others added 5 commits October 30, 2026 22:34
A lite-05 SUBSCRIBE sent alongside its TRACK can reach a JS publisher before
an on-demand track resolves, when no group is known: re-read the latest group
once the track resolves, as the Rust publisher does, so a pre-06 "no start"
does not replay the cache.

The subscriber records the TRACK stream as soon as TRACK_INFO lands, so a
cancellation while the SUBSCRIBE still waits for a stream closes it instead
of leaving it open behind a create that may never finish.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The aggregate asked upstream is no longer clamped to TRACK_INFO's max age,
but the session's tail and the JS track's arrival ledger read their grace
from it. A subscriber deferring to the max age alone then waited out an
unbounded grace for a group lost at the tail. Bound the grace locally by the
max age, as before, without touching the demand sent upstream; a pipelined
SUBSCRIBE takes the bound once its copy is accepted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…uests/subscribe

# Conflicts:
#	rs/moq-net/src/lite/subscriber.rs
…raft

A SUBSCRIBE a JS relay serves from a copy still fetching upstream can beat the
copy resolving on any pre-06 draft, not only a lite-05 one sent beside its
TRACK. Re-read the newest group once the track resolves on all of them, and
finish quest/m1/js-early-latest.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Landing eefbf7d429c585e910fd6fd12376fa9c07f77ad6 via quest land.

Since a1f37497e1:

  • Both review threads fixed and resolved. The JS publisher re-reads the newest group once an unresolved track resolves, now on lite-01..05 per quest/m1/js-early-latest.md, which this PR deletes. The JS subscriber holds an answered TRACK as soon as TRACK_INFO lands, so a cancellation while the SUBSCRIBE create is still parked closes it.
  • Local review finding fixed: the session's tail grace is bounded by the track's max age again, in Rust and JS. Only the demand asked upstream stays unclamped.
  • Merged origin/main, with one hand-resolved conflict: reject_unused was renamed reject_unwanted.

just check passes against origin/main, and local Codex review round 3 on this head found nothing. Suggested follow-up: give the Rust publisher the same lite-03/04 early-latest floor.

(Written by Claude Opus 5.5)

@kixelated
kixelated enabled auto-merge (squash) October 31, 2026 09:17

@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: eefbf7d

Compared with my a1f3749 review, separating the main merge from the follow-up fixes. No new actionable bugs found in the substantive update.

Fixed: the answered TRACK cancellation leak now records ownership independently of SUBSCRIBE creation; the independent JS join-floor finding is addressed by recomputing the previously unknown floor after resolution, including pre-05 drafts. The added tests target both cases.

Direction: these are focused source-level fixes. Restoring max-age bounds for tail grace while leaving upstream demand stable is a useful separation, without new public API or wire fields.

Verification: GitHub-only static inspection of the incremental commits, current affected code and tests. Tests were not run; this is not a test-pass claim.

(Written by OpenAI)

@moq-bot
moq-bot Bot disabled auto-merge October 31, 2026 17:17
@moq-bot

moq-bot Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

codex / Codex land fixer: This maintainer pick already used its mechanical fi·x attempt. Run /quest-iterate to diagnose the repeated failure, then pick it again.

(written by codex / Codex)

@kixelated
kixelated enabled auto-merge (squash) November 1, 2026 12:17
@moq-bot
moq-bot Bot disabled auto-merge November 1, 2026 16:34
@moq-bot moq-bot Bot removed the quest:merge label Nov 1, 2026
@moq-bot

moq-bot Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

quest land removed this pull request from the queue: Conflicts with the base: js/net/src/lite/subscriber.ts, js/net/src/request-deadline.test.ts, quest/m2/pipeline-requests/fetch.md, quest/m2/pipeline-requests/subscribe.md, rs/moq-net/src/lite/subscriber.rs, rs/moq-net/tests/integration/finished_broadcast_mock.rs, rs/moq-relay/tests/integration/auth_lifetime.rs
.

@moq-bot

moq-bot Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

codex / Codex land fixer: Seven base conflicts; rs/moq-net/src/lite/subscriber.rs overlaps pending SUBSCRIBE with base's pending FETCH state machine and changes session-death handling while group streams are parked, requiring . Recommendation: Reconcile TrackRunState::Info polling, cancellation and adoption for simultaneous TRACK/SUBSCRIBE/FETCH, review parked-stream teardown against base's owed-tail abort semantics, and validate pipelining

		loop {
			match &mut self.state {
				TrackRunState::Info {
					request,
					info,
					pipelined,
				} => {
					// Abandonment was checked at the top of this poll, arming its wait.
					let pending = request.as_mut().expect("request pending");
					// Demand that arrives before the info goes upstream with it, the TRACK
					// first. One that needs the info to say what it asks for waits for it.
					let mut started = Ok(());
					if pipelined.is_none()
						&& let Poll::Ready(Some(pref)) = pending.poll_subscription_changed(waiter)
					{
						started = self.serve.pipeline(pref).map(|subscribing| *pipelined = subscribing);
					}
					let fetched = info.poll_fetch(&self.serve, waiter);
					let failed = match pipelined {
						Some(subscribing) => subscribing.poll(waiter),
						None => Poll::Pending,
					};
					let res = match (started, failed, fetched) {
						(Err(err), _, _) | (_, Poll::Ready(err), _) => Err(err),
						(_, _, Poll::Ready(res)) => res,
						(_, _, Poll::Pending) => return Poll::Pending,
					};
					let request = request.take().expect("request pending");
					let pipelined = pipelined.take();
					match res {
						Ok((mut info, held)) => {
							// Lite05/06 timestamps may be send times, so the copy is untimed
							// and the ingest reads past them (see `lite::Framing::new`).
							if !self.serve.subscriber.version.has_media_time() {
								info.timescale = None;
							}
							let mut serve_loop = ServeLoop::new(&self.serve, request, info);
							serve_loop.held = Some(held);
							if let Some(pipelined) = pipelined {
								serve_loop.adopt(&self.serve, pipelined);
							}
							self.state = TrackRunState::Serve(serve_loop);
						}
						Err(err) => {
							let err = lite::error::control(&mut self.serve.subscriber.session.clone(), err);
							let broadcast = self.serve.subscriber.log_path(&self.serve.path);
							match &err {
								// A request racing the route's removal: a normal outcome.
								Error::Unroutable | Error::Stream(crate::StreamError::Unroutable) => {
									tracing::debug!(%broadcast, track = %self.serve.name, %err, "track info failed")
								}
								err => tracing::warn!(%broadcast, track = %self.serve.name, %err, "track info failed"),
							}
							// Rejecting the request lets the origin retry (bounded) on
							// another source; waiting subscribers stall rather than error
							// meanwhile. The SUBSCRIBE sent alongside ends with it.
							if let Some(pipelined) = pipelined {
								pipelined.abort(&self.serve, &err);
							}
							request.reject(err);
							self.state = TrackRunState::Done;
							return Poll::Ready(());
						}
					}
				}
				TrackRunState::Serve(serve_loop) => {
					let teardown = ready!(serve_loop.poll(&self.serve, waiter));
					let finished = matches!(teardown, ServeEnd::Finished);
					let TrackRunState::Serve(mut serve_loop) = std::mem::replace(&mut self.state, TrackRunState::Done)
					else {
						unreachable!()

codex
The Rust conflicts require review of request lifecycle behavior. The PR adds pending SUBSCRIBE handling, while base adds pending FETCH handling and changes how tracks end when a session dies. Combining their polling, cancellation, and parked-stream cleanup goes beyond a mechanical resolution. I’ll hand PR 7328 back with that diagnosis and leave its branch unchanged.

exec
/bin/bash -lc "git -c core.hooksPath=/dev/null merge --abort; python3 - <<'PY'
import json
from pathlib import Path
result = [{\"number\":7328,\"action\":\"handback\",\"diagnosis\":\"Seven base conflicts; rs/moq-net/src/lite/subscriber.rs overlaps pending SUBSCRIBE with base's pending FETCH state machine and changes session-death handling while group streams are parked, requiring lifecycle review beyond mechanical resolution.\",\"recommendation\":\"Reconcile TrackRunState::Info polling, cancellation and adoption for simultaneous TRACK/SUBSCRIBE/FETCH, review parked-stream teardown against base's owed-tail abort semantics, and validate pipelining and session-death tests.\"}]
Path('quest-land-result.json').write_text(json.dumps(result, indent=2)+'\\n')
PY
 git rev-parse pr-7328 batch base; git -c core.fsmonitor=false status --short --untracked-files=normal" in /work
 succeeded in 1320ms:
Updating files:  56% (750/1321)
Updating files:  57% (753/1321)
Updating files:  58% (767/1321)
Updating files:  59% (780/1321)
Updating files:  60% (793/1321)
Updating files:  61% (806/1321)
Updating files:  62% (820/1321)
Updating files:  63% (833/1321)
Updating files:  64% (846/1321)
Updating files:  65% (859/1321)
Updating files:  66% (872/1321)
Updating files:  67% (886/1321)
Updating files:  68% (899/1321)
Updating files:  69% (912/1321)
Updating files:  70% (925/1321)
Updating files:  71% (938/1321)
Updating files:  72% (952/1321)
Updating files:  73% (965/1321)
Updating files:  74% (978/1321)
Updating files:  75% (991/1321)
Updating files:  76% (1004/1321)
Updating files:  77% (1018/1321)
Updating files:  78% (1031/1321)
Updating files:  79% (1044/1321)
Updating files:  80% (1057/1321)
Updating files:  81% (1071/1321)
Updating files:  82% (1084/1321)
Updating files:  83% (1097/1321)
Updating files:  84% (1110/1321)
Updating files:  85% (1123/1321)
Updating files:  86% (1137/1321)
Updating files:  87% (1150/1321)
Updating files:  88% (1163/1321)
Updating files:  89% (1176/1321)
Updating files:  90% (1189/1321)
Updating files:  91% (1203/1321)
Updating files:  92% (1216/1321)
Updating files:  93% (1229/1321)
Updating files:  94% (1242/1321)
Updating files:  95% (1255/1321)
Updating files:  96% (1269/1321)
Updating files:  97% (1282/1321)
Updating files:  98% (1295/1321)
Updating files:  99% (1308/1321)
Updating files: 100% (1321/1321)
Updating files: 100% (1321/1321), done.
f19119509e29d53bcbf2f56aca40462e5343816f
f19119509e29d53bcbf2f56aca40462e5343816f
802cadc3cc56cdee21bcf8b1ac161798132ae1a4
?? quest-land-result.json

codex
PR 7328 handed back: overlapping SUBSCRIBE/FETCH lifecycle and session-death changes require behavioral review.

Branch unchanged. Diagnosis and recommendation recorded in [quest-land-result.json](/work/quest-land-result.json), left uncommitted.
tokens used
33,459
PR 7328 handed back: overlapping SUBSCRIBE/FETCH lifecycle and session-death changes require behavioral review.

Branch unchanged. Diagnosis and recommendation recorded in [quest-land-result.json](/work/quest-land-result.json), left uncommitted.
fixer result written

(written by codex / Codex)

kixelated and others added 4 commits November 1, 2026 08:48
Resolve the subscriber conflicts with the pipelined first FETCH (#7332): the Info state carries both the pipelined SUBSCRIBE and the fetches sent alongside. Follow the moq_clock -> kio::clock rename. Delete the finished pipeline-requests epic and measure the first fetch over two relays.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ives the track back

A lite-06 publisher resets the SUBSCRIBE with an errored end right after the cached groups, so with the SUBSCRIBE pipelined the reset can land alongside the TRACK_INFO, ahead of the parked group streams. The serve loop gave the track back at once and lost that history (errored_end::an_errored_end_keeps_its_history_across_a_hop). It now unparks and waits for them first, as it already did for a closed session.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@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: 1400eb5

Compared with eefbf7d, separating base merges and mechanical adaptations from the combined FETCH/SUBSCRIBE setup and new failed-subscription drain. One P2 finding below: the drain stops at header adoption, not payload completion. The previously reviewed JS cancellation and join-floor fixes remain intact.

Direction: sharing the accepted-info/fetch lifecycle is sensible. Reuse the existing group-completion accounting for the failed-subscription drain rather than adding another header-level completion signal. The two-relay FETCH timing case is useful coverage.

Verification: GitHub-only static inspection of incremental patches, surrounding lifecycle/model code and tests. I did not run tests or reproduce the fragmented-stream case locally.

(Written by OpenAI)

Comment thread rs/moq-net/src/lite/subscriber.rs Resolved
kixelated and others added 4 commits November 1, 2026 09:49
…gives the track back

The failed drain stopped once the last parked stream took its group, before that stream had read its frames, so giving the track back truncated payloads still in flight. It now waits until the subscription's tail has no stream still being read, parked or not: a group ends on its own stream's FIN or reset, as the tail already holds.

The errored-end test now delays every group's frames and FIN past the reset, with the SUBSCRIBE arriving beside and before the TRACK, and reads every frame.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The failed drain waited only for its streams, so a stream the publisher never finished kept the subscription, its copy and its stream after every reader left. It now drives the fetches and takes the idle teardown once the copy is unwanted, as the tail does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@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: f1e690d

Compared with 1400eb5, accounting for the main merge. The parked-payload truncation finding is fixed: the failed drain now waits on stream-completion accounting, and releases the copy when no reader remains. The regression delays payloads/FIN past the reset and checks every frame, including SUBSCRIBE-first ordering. No new actionable bugs found in this update.

Direction: reusing the existing tail Reading lifetime is simpler and more accurate than treating header adoption as completion. The abandonment check keeps that drain tied to demand.

Verification: GitHub-only static inspection of the incremental code, surrounding ingest/tail lifecycle and tests. Tests were not run; this is not a test-pass claim.

(Written by OpenAI)

@kixelated
kixelated enabled auto-merge (squash) November 1, 2026 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant