Skip to content

feat(net)!: own broadcast discovery during resolution - #6445

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

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

Conversation

@kixelated

@kixelated kixelated commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Requesting an unwatched path can return Unroutable before a watch-driven peer discovers it. Epoch-pinned announcement followers also close across a gap instead of handing out their instance when it returns.

Approach

  • Make Rust request_broadcast own scoped discovery, report table absence after sync, and retry route-generation churn. The immediate table lookup becomes the private Lookup used by sessions; lite and IETF publishers answer peers through it, so serving a peer never starts discovery.
  • Give every JS request its discovery watch from creation. An announced follower requires discovery and resumes its pinned instance after a gap. Plain requests retain synchronous peeks and can use peers without discovery; a pending one fails Unroutable once every session discovers and none still owes an initial set covering its path. That check runs when a snapshot lands or the session mix changes, coalesced into one microtask, and walks only the watch trie beneath the landed prefix, so settlement stays path-local. A follower sharing a slot with a plain request never takes the slot's blind answer. A session releases its snapshot on a failed announce stream together with the downgrade, so a refusal never reads as synced absence.
  • Record selected-handler refusal at the existing private refusal boundary, so a dropped Prepared answer stays terminal even when it changes the table. Route withdrawal and coalesced same-epoch end/start events remain retryable.
  • Replace the stats reader's is_queued retry heuristic with the shared request. The mux binding keeps main's pinned routed_broadcast wait (moq-dev/moq#6254), now typed as Requesting.
  • Preserve the existing routed_broadcast route-change retry policy for CLI, gateway, mux binding, relay HTTP, and benchmark consumers. Its removal belongs to the remaining rollout.

Merge with main (2026-10-28)

Main moved 580 commits, so this round merged it and re-applied the change onto main's renames (Path, requests()/Requests). Per the MIGRATE.md-at-cut policy (moq-dev/moq#6610), the per-package MIGRATE.md edits are dropped and their guidance moved into Impact. Main removed JS Requesting.unroutable (moq-dev/moq#6470), so this PR's synced-absence extension of that getter, its absence benchmark, and the tests reading it are gone; synced absence now settles closed instead (decision 2). Main's new IETF resolve path (moq-dev/moq#6964) now uses the private lookup.

Impact

  • Rust (breaking, moq-net): request_broadcast and routed_broadcast return Pending<origin::Requesting>, replacing public origin::Routing. request_broadcast owns discovery of its path and fails Unroutable only once discovery has synced without a route for the requested instance; drop any watch held only to make it resolve. Requesting::poll_ok needs a mutable receiver (keep the task mut when polling; awaiting is unchanged). Requesting::is_queued is removed: delete manual Unroutable retries, since the request watches route changes itself. A selected handler's explicit refusal is returned directly by request_broadcast and announced_broadcast; routed_broadcast keeps retrying refusals only after its covering routes change. Paths discovery cannot name fail with InvalidPath.
  • JS (@moq/net, behavior): Origin.Consumer.request watches its path from creation until close. Paths with wildcard characters are refused before registering. { announced: true, epoch } follows that instance through gaps and returns it when announced again. Announced requests no longer fall back to blind answers on a session without discovery; use a plain request there. With a discovering session, a plain request waits for the path's announcement instead of taking a blind answer, and fails closed with Unroutable once that discovery has synced with no route for it (or none for its pinned epoch). Handle that error where main left the request pending.
  • Bindings (behavior): request_broadcast in moq-ffi, moq_origin_request in C, and every wrapper inherit owned discovery and refusal behavior; calling it straight after connecting no longer races the session's announcements. C ABI and UniFFI method signatures are unchanged; the Dart checksums change with the corrected docs.
  • Wire: none. No package version bumps.

Alternatives

Inferring refusal from route identity or watch generation loses provenance when preparing an answer changes the table or an end/start is coalesced. A private marker at the existing refusal decision preserves both explicit refusal and genuine churn. Existing legacy route-wait behavior remains local to routed_broadcast rather than adding another public policy or compatibility API.

Decisions

The earlier prerequisite, finishing reference announcements (Source::follow) before the rollout, has landed: moq-dev/moq#6254 and moq-dev/moq#6649 moved moq play, moqsrc, and the mux follower onto moq_mux::Source::follow. The quest's Required link is removed. The maintainer settled both remaining decisions on 2026-10-28:

  1. Scope of this PR.
    1. ✅ Land it as the behavioral checkpoint; the resolve quest keeps the request/follow naming rollout, deleting routed/routed_broadcast, and caller migrations. (The diff already crosses Rust, JS, and every binding, and the rollout breaks every wrapper's names, so it reviews better on its own.) quest/m1/api-audit/resolve.md is rewritten to what remains.
    2. Extend this PR to the full rollout.
  2. JS plain-request absence. With main's Requesting.unroutable gone, a plain JS request on a discovering session that found no route stayed pending with no signal.
    1. ✅ Settle closed with Unroutable once scoped discovery syncs with no covering route, in this PR. (Matches Rust and the quest goal, and closes the silent wait the merge exposed.) Implemented, with a test that fails without it.
    2. Leave it pending here and make it terminal in the rollout.
    3. Keep main's blind fallback for plain requests on discovering sessions until the rollout.

Follow-ups

Continue the resolve quest: method naming and optional-epoch bindings, removing routed/routed_broadcast and the route-level Follow (remaining consumer: the moq-hls server's RESOLVE_TIMEOUT resolve), and the WASM, CLI, SRT, transcode, bench, relay HTTP, e2ee, and demo callers it lists.

Validation

Round of 2026-10-28 (head after merging main): nix develop --command just ci origin/main passed (all modules), and nix develop --command just test interop --all passed. The new JS test a pending plain request fails Unroutable once its discovery syncs without a route fails on the previous head. After the automated review, fdb3d53 fixed the follower blind-answer leak (the extended only a plain request can use a session without discovery fails without it) and made settlement path-local; the new pending,landed,settle_us section of js/net/bench/requests.ts drops from 865 to 38 µs at 8,192 pending requests and one landing. just ci origin/main passed again on that head. Releasing the snapshot only while the announce stream is open fixes three integration tests and two origin tests that otherwise read a refused discovery stream as synced absence.

Earlier rounds:

Before the fix, the unwatched native request reproduction failed with Unroutable and the JS pinned follower failed across a gap. Coverage includes scoped discovery sync and release over lite-05/06/07-wip, route churn, pinned return, unsupported discovery and scopes, non-discovery peers, and explicit/dropped-Prepared refusal.

On head after the merge:

  • nix develop --command just ci origin/main: passed (Rust 8,968 + 1,236 + 126 + others, Python 108, JS, C, C++, Go, Kotlin, Dart, quest).
  • nix develop --command just test interop --all: passed.
  • cargo clippy -p moq-net -p moq-mux -p moq-stats -p moq-ffi -p moq-c --all-targets --all-features -D warnings and a workspace cargo check --all-features: passed.
  • Two main-side JS tests changed: a derived-handle test now expects the request's own +blind/-blind interest, and the coalesced-wakeup forward test downgrades discovery so it still exercises blind answers.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

Keep one-instance requests scoped to synchronized discovery, follow pinned instances through gaps, and preserve explicit refusal provenance. Use the shared native resolver for mux references and stats while recording the remaining public API rollout.

Co-Authored-By: GPT-6 <noreply@openai.com>
@kixelated kixelated changed the title Two broadcast resolutions feat(net): own broadcast discovery during resolution Oct 23, 2026
kixelated and others added 2 commits October 28, 2026 08:55
Resolve against main's renames (Path, requests(), Requests) and its
MIGRATE.md-at-cut policy: drop per-PR MIGRATE.md edits, keep the mux
binding's pinned routed_broadcast wait, and drop the JS synced-absence
extension of Requesting.unroutable, which main removed. Sessions keep the
private immediate lookup, including the new IETF resolve path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title feat(net): own broadcast discovery during resolution feat(net)!: own broadcast discovery during resolution Oct 28, 2026
kixelated and others added 2 commits October 28, 2026 10:03
…thout a route

A pending plain request now settles `closed` with `Unroutable` once every
attached session discovers and none still owes an initial set covering its
path, matching Rust's `request_broadcast`. The check is slot-scoped: snapshot
releases and session changes recheck only the requests under the affected
prefix, in one microtask. A session releases its snapshot on a failed announce
stream together with the downgrade, so a refusal never reads as synced
absence.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review October 28, 2026 17:31
@coderabbitai

coderabbitai Bot commented Oct 28, 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 18 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: 100ae6b1-fb16-45c4-abff-5282751e7bb7
Commits

Reviewing files that changed from the base of the PR and between c3543bf and fdb3d53.

Files selected for processing: 25
  • dart/moq_ffi/lib/src/moq.dart
  • doc/lib/js/net.md
  • doc/lib/rs/moq-net.md
  • js/net/bench/requests.ts
  • js/net/src/connection/forward.test.ts
  • js/net/src/connection/forward.ts
  • js/net/src/integration.test.ts
  • js/net/src/origin-request.test.ts
  • js/net/src/origin.test.ts
  • js/net/src/origin.ts
  • py/moq-rs/README.md
  • quest/m1/api-audit/resolve.md
  • rs/moq-c/src/api.rs
  • rs/moq-ffi/src/origin.rs
  • rs/moq-ffi/src/test.rs
  • rs/moq-mux/src/source.rs
  • rs/moq-net/benches/origin.rs
  • rs/moq-net/src/ietf/publisher.rs
  • rs/moq-net/src/lite/publisher.rs
  • rs/moq-net/src/model/follow.rs
  • rs/moq-net/src/model/mod.rs
  • rs/moq-net/src/model/origin.rs
  • rs/moq-net/tests/integration/lazy_discovery.rs
  • rs/moq-net/tests/integration/model.rs
  • rs/moq-stats/src/aggregate.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

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 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-28T17:46:49.935721Z fdb3d53 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: 384668ddb0

ℹ️ 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/origin.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: 384668d

  • P2 — Keep blind answers out of announced handles sharing a slot (js/net/src/origin.ts:1880–1885, 1903–1905). With a non-discovering session, create both request(path, { announced: true }) and a plain request(path). The plain handle makes blind(slot) true, and #answer installs its response in the shared slot. route() then returns slot.answer, and own() filters only by epoch, so the unpinned announced follower also becomes active despite no announcement. The same happens when it joins an already answered plain request. Enforce discovery eligibility per handle (while retaining shared transport work), and add a mixed plain/announced regression that actually delivers the blind answer; the new test currently only checks the slot's blind flag.
  • The independent quadratic discovery-settlement finding also applies; no duplicate inline comment added.

Direction: owning discovery in the request and separating peer lookup is sound; retaining refusal provenance avoids brittle generation heuristics. The behavioral checkpoint is a reasonable boundary before the naming rollout. The shared-slot policy leak and table-wide settlement work should be addressed without another public policy API.

Verification: static inspection of changed Rust/JS resolution, forwarding, bindings, callers and regression tests; I did not run tests or independently verify the reported CI results.

…llowers

Synced-absence settlement walks the watch trie beneath each landed prefix
instead of scanning every request slot, and a resolved handle drops its
check. A follower sharing a slot with a plain request no longer takes the
slot's blind answer. A new requests bench sweeps pending requests against
initial sets landing at once.

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

Copy link
Copy Markdown
Collaborator Author

Addressed the automated review in fdb3d53:

  • Blind answers in a shared slot: agreed, this was a real leak. A follower now takes only a front something announced (one with a source), so a slot's blind answer reaches only its plain handles. only a plain request can use a session without discovery now delivers a blind answer and checks that both an existing and a joining follower stay inactive; it fails without the fix.
  • Quadratic settlement: fixed path-locally, with a benchmark; see the thread.

(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: fdb3d53

Compared the single follow-up commit against my prior review at 384668d (base unchanged).

  • Fixed: the shared-slot blind-answer leak. own() now excludes source-less answers for announced handles, and the regression delivers a blind answer while checking both existing and newly joining followers.
  • Fixed: the independent quadratic settlement finding. Settlement walks affected watch-trie subtrees with deduplicated nodes; resolved handles remove their absence checks. The added benchmark varies unrelated pending requests and landed snapshots and checks settlement isolation.

No new actionable bugs found in this update. Reusing the existing watch trie and filtering eligibility per handle are appropriately small fixes that preserve transport sharing without expanding the public API.

Verification: static review of the three changed JS files and surrounding ownership/settlement code. Tests and benchmarks were not run; reported timings were not independently verified.

@kixelated

Copy link
Copy Markdown
Collaborator Author

Merge summary for fdb3d53:

  • Change: Rust request_broadcast now owns scoped discovery and returns Unroutable only after discovery syncs with no route. JS requests watch their path from creation: announced followers resume their pinned instance after a gap, and once discovery syncs with no route, plain requests close with Unroutable.
  • Decisions (settled by the maintainer 2026-10-28): land this as the behavioral checkpoint and keep the naming rollout and routed_broadcast removal in quest/m1/api-audit/resolve.md. JS plain-request absence settles closed with Unroutable in this PR.
  • Reviews: both findings from the first automated review (the shared-slot blind-answer leak and the quadratic settlement) were fixed in fdb3d53. The follow-up review found nothing new, and Codex gave a 👍 on that head.
  • Checks: CI passed on the head, and quest check --base origin/main passed (617 documents ok). Landing through the merge queue.

(Written by Claude Opus 5.5)

@kixelated
kixelated added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit d506916 Oct 28, 2026
11 checks passed
@kixelated
kixelated deleted the quest/m1/api-audit/resolve branch October 28, 2026 20:04
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