Skip to content

feat(net)!: name the Rust resolutions request and follow - #7172

Merged
kixelated merged 7 commits into
mainfrom
quest/m1/api-audit/resolve/rust
Oct 29, 2026
Merged

kixelated merged 7 commits into
mainfrom
quest/m1/api-audit/resolve/rust

Conversation

@kixelated

@kixelated kixelated commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Closes the Rust request and follow quest under Two broadcast resolutions. The behavior landed in moq-dev/moq#6445; this PR renames it.

Problem

moq-net had four ways to resolve a path: request_broadcast, announced_broadcast, routed (wait for a route), and routed_broadcast (wait, then resolve). The route-level announce::Follow was public too. Callers paired routed with request_broadcast by hand, which is the racy two-step resolve the epic wants gone.

Approach

  • Consumer::request(path, epoch) is the old request_broadcast. The immediate table lookup that sessions use is now the private lookup.
  • Consumer::follow(path, epoch) is the old announced_broadcast. It returns origin::Following (was AnnouncedBroadcast).
  • routed and routed_broadcast are deleted, along with Requesting's refusal-retry mode that only routed_broadcast used. A selected handler's refusal is now always terminal.
  • announce::Follow is crate-private, behind the private Consumer::routes.
  • Callers move off the old helpers:
    • routed + request pairs collapse into follow(p, None)?.available(), which waits for the announcement and then resolves.
    • Tests that read route events or need the Route use a literal-scoped announced() cursor, or a support::announced helper in moq-net's integration tests.
    • The moq-hls server resolves with request, so absence comes from discovery sync with no hand-rolled sync check. It then follows later instances, skipping the instance it already serves.
    • The mux Binding holds a Following and keeps its failure, so repeated reads return the same answer. A ready answer from its eager first poll is pinned right away.
  • moq-ffi and the C ABI keep their public names (request_broadcast, announced_broadcast). The bindings quest renames those.

Impact

  • Breaking (moq-net): origin::Consumer::request_broadcast(path, epoch) is now request(path, epoch). Rename the call; nothing else changes.
  • Breaking (moq-net): origin::Consumer::announced_broadcast(path, epoch) is now follow(path, epoch), and origin::AnnouncedBroadcast is now origin::Following.
  • Breaking (moq-net): origin::Consumer::routed is removed. To wait for an announcement and then resolve it, use consumer.follow(path, None)?.available().await. To read the route itself, use consumer.announced(), scoped to the path with Pattern::literal if needed.
  • Breaking (moq-net): origin::Consumer::routed_broadcast is removed. Use consumer.follow(path, None)?.available().await. It also waits for a route, but it returns a selected handler's refusal instead of waiting for the routes to change.
  • Breaking (moq-net): announce::Follow and Consumer::follow(path) -> announce::Follow are no longer public. Read events from consumer.announced() on a consumer scoped to the path with Pattern::literal.
  • moq-mux: the Binding for a sibling reference returns a handler's refusal instead of waiting for the routes to move.
  • Wire: none.

Choices made without asking

  • Deleted the origin/unroutable_churn bench. It measured requests that had been refused and woke up when a covering route was repriced, and only routed_broadcast could still do that.
  • moq-hls kept RESOLVE_TIMEOUT, now only as a limit on a dynamic handler that never answers. Absence no longer waits on it. Whether to keep that limit is listed for the callers quest.
  • CLI waits that only blocked until a route appeared (stdout, RTMP and WHIP exports) now resolve with follow and drop the result before the real resolve. This is the smallest change that keeps their waiting behavior. Rewriting them belongs to the callers quest, whose Plan this PR updates. The redundant route check in subscribe's TS path is removed, because Subscribe::new has already resolved the broadcast by then.
  • In an_unpublish_lingers_and_a_hangup_does_not (moq-rtmp), a hangup ends the instance before any follower would hand it out, so that test waits for the raw announcement and then requests, as routed did.

Decisions

  • ✅ follow returns origin::Following (maintainer's pick). It pairs with Requesting, the owned task request returns. The JS sibling (moq-dev/moq#7173) deliberately keeps returning Origin.Requesting, so no JS change follows from this.

Alternatives

  • Keeping a crate-private test helper routed for the ~150 test waits. Rejected: tests read the public announced() cursor or use follow, so no shim is left behind.

Checks

  • just ci: pass.
  • just test interop --all: all checks passed.
  • just rs test across every touched crate, no fail-fast: pass, after fixing the two failures found (moq-hls rendition refusal and the moq-rtmp hangup test above).

Follow-ups

  • The callers quest now lists the leftover resolve-and-drop CLI waits, moq-wasm's discarded error, and the moq-hls stall limit.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

@kixelated kixelated changed the title Rust request and follow feat(net)!: name the Rust resolutions request and follow Oct 29, 2026
@kixelated
kixelated marked this pull request as ready for review October 29, 2026 01:53
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 29, 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-29T02:24:32.395458Z 55c9040 New commits
ℹ️ 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: f409707d83

ℹ️ 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 rs/moq-mux/src/source.rs Resolved
Comment thread rs/moq-hls/src/server/mod.rs 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: f409707

P2 — Preserve the initially served instance's end while starting the HLS follower (rs/moq-hls/src/server/mod.rs:273–280). The first instance now comes from a separate request, but instances has never been polled. If that publisher disappears after the request resolves and before the spawned run_follow first runs, Following folds its queued start/end into current absence and waits for another publisher. Instances.current is still None, so it cannot emit End for the already-served instance; forget_closed only prunes by_id, not current or the outer broadcasts entry. A path that never returns therefore retains its cached broadcaster, task, and path watch indefinitely. Seed the follower's current-instance tracking with the initial answer, or explicitly observe that initial answer's closure while awaiting the first Start. Add a regression that closes/retracts the publisher before the first follow-task poll and asserts the entry is removed without a replacement publication.

Direction: the request/follow split and removal of refusal-retry mode simplify the API appropriately; the mux binding's retained terminal failure and eager pin are consistent with that contract. The HLS handoff needs the lifecycle fix above.

Verification: static inspection of the changed core resolution, HLS/mux lifecycle, caller migrations, and relevant tests, with an independent core/race cross-check. I did not run builds, tests, or interop checks; the PR's reported results were not independently verified.

kixelated and others added 2 commits October 28, 2026 18:59
…solve/rust

# Conflicts:
#	doc/lib/rs/moq-net.md
#	quest/m1/api-audit/resolve/js.md
The mux Binding now holds its follower locked until the answer is
pinned. The follower hands each instance out once, so a concurrent read
that polled it between the two would wait for the next instance.

The moq-hls follow task ends when the served instance closes before the
follower hands it out. The follower folds away an instance that started
and ended before its first poll, so nothing else would end the follow.

Drops callers.md's now-empty Required section, since its JS blocker
merged in main.

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

Copy link
Copy Markdown
Collaborator Author

Pushed 55c9040:

  • HLS follower handoff (the P2 in the OpenAI review, also raised by Codex): run_follow now ends when the served instance closes before the follower hands it out. The new an_instance_gone_before_the_follower_runs_is_forgotten test drops the publisher before the follow task first runs and checks that the entry is removed. It fails without the fix.
  • mux Binding concurrent polls (Codex): the follower stays locked until its answer is pinned.
  • Merged main. js.md was deleted there because moq-dev/moq#7173 merged, and with it gone callers.md's ## Required was empty, so I removed that heading.

just ci passes except for the quest check, which failed on that empty heading and passes now. cargo test -p moq-mux -p moq-hls and clippy also pass.

(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: 55c9040

The prior HLS lifecycle P2 is fixed: run_follow now observes the initially served broadcast's closure while waiting for its first follower event, with a regression covering closure before the task's first poll. Polling the follower first preserves replacement priority. The independent mux pinning race reported in discussion_r4407952453 is also addressed by holding the following lock through pin lookup, polling, and insertion.

No new actionable bugs found in this update. Direction: these small, targeted lifecycle and serialization fixes fit the request/follow split without adding another coordination mechanism.

Verification: base-aware static comparison against the previous reviewed head, inspection of both fixes and the added regression, plus an independent race/lifecycle cross-check. I did not run builds or tests; reported test results were not independently verified.

@kixelated

Copy link
Copy Markdown
Collaborator Author

Landing 55c9040 with the maintainer's approval.

  • Reviews: Codex gave 55c9040 a 👍. The automated OpenAI review of that head found nothing new and confirmed the two earlier findings are fixed: the mux Binding pin race and the moq-hls follower not seeing the served instance close. Both threads are resolved.
  • Checks: all 11 required checks passed. quest check --base origin/main passes on this head (661 documents).
  • Decisions: none still open. follow returns origin::Following (the maintainer's pick). The leftover CLI waits, moq-wasm's discarded error, and the moq-hls stall limit are already in the callers quest.

(Written by Claude Opus 5.5)

@kixelated
kixelated added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit 80cb517 Oct 29, 2026
11 checks passed
@kixelated
kixelated deleted the quest/m1/api-audit/resolve/rust branch October 29, 2026 04:00
kixelated added a commit that referenced this pull request Oct 29, 2026
#7173 replaced `request(path, { announced: true })` with `follow(path)` in JS,
and #7172 renamed Rust's `request_broadcast` to `request`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated added a commit that referenced this pull request Oct 29, 2026
main renamed request_broadcast to request in #7172.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated added a commit that referenced this pull request Oct 29, 2026
Main renamed OriginConsumer::request_broadcast to request (#7172).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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