Skip to content

feat!: one Route handle owns its broadcast - #7806

Merged
kixelated merged 28 commits into
mainfrom
quest/m1/api-audit/route/handle
Nov 5, 2026
Merged

kixelated merged 28 commits into
mainfrom
quest/m1/api-audit/route/handle

Conversation

@kixelated

@kixelated kixelated commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Implements One Route handle and deletes the quest. The Hop/Hops/HopLimit privatization, plus hiding JS RouteInfo's epoch and auth, is split into a new quest, Transit facts stay read-only. That work is mechanical and lands independently, and it would have added roughly 100 more call sites to an already large diff.

On the publishing side, ownership now runs origin → route → broadcast. An application creates a Route on the origin, and the route creates and owns its broadcast. Publication, Prepared, Requests, the broadcast's own announcer and Producer::{create_broadcast, publish, requests} are gone. This covers Rust, JS, moq-ffi, moq-c and every wrapper.

let route = origin.route("live/alice", RouteOptions::hidden())?;
route.broadcast().create_track("catalog.json", info)?;
route.announce()?;  // visible now, same instance
route.set_cost(5)?; // re-prices in place
route.withdraw();   // hidden; announce() again continues the same stream

let claim = origin.route_prefix("transcode", RouteOptions::default())?;
while let Some(request) = claim.requested().await {
    let job = request.route(RouteOptions::hidden())?; // fresh instance; answers on announce()
    spawn(run(job));
}

Changes

Rust (moq-net)

  • origin::Route is the handle, for both exact and prefix routes. It is Clone, and dropping the last clone retracts the route. Closing its broadcast also retracts it.
    • Methods: broadcast(), instance(), announce(), withdraw(), is_announced(), set_cost(), set_auth(), requested() and poll_requested().
    • The route mints its instance at creation and keeps it across withdraw and re-announce. RouteOptions covers hidden(), with_cost, with_auth and with_instance(instance, epoch).
  • Request::route(options) replaces prepare/Prepared::publish. It mints a fresh instance by default, as decided on moq-dev/moq#7770.
    • Visible options announce at once and answer the waiting requesters. Hidden options answer them on announce().
    • Dropping the route before it announces refuses them. A released request makes announce() fail Unroutable, and that route can never announce.
  • A prefix route is priced, credentialed and hidden like an exact one.
  • The broadcast knows nothing of routes. Its announcer slot is now a generic close hook that the owning route uses to retract.
  • The old metadata origin::Route is now origin::RouteInfo, with read-only getters: instance() -> &Instance, cost(), hops(), is_anonymous() and source(). It has no public builders.
  • Sessions use a crate-private Producer::forward(prefix, info, predecessor) and Route::update(info) in place of requests_route/update_route.
  • moq_net::fuzz gains route_info(hops, cost, instance) for benches and tests that need relay metadata.

JS (@moq/net)

  • New API: origin.route(path, options?), origin.routePrefix(prefix, options?), class Route and Request.route(options?).
    • class Route has broadcast (throws on a prefix route), instance, announced, announce(), withdraw(), setCost(), setAuth(), requested(), close() and [Symbol.dispose].
    • RouteOptions is { hidden, cost, auth, instance, epoch }.
    • RouteInfo is the read-only received metadata.
  • Removed: createBroadcast, requests, requestsRoute, updateRoute, Publication, Prepared and Requests.
  • No longer exported: RequestSlot and RouteEntry. The orphaned @internal blocks are deleted, and sessions reach routes through a non-exported wire view.
  • @moq/publish's on/off toggle is withdraw()/announce() on one hidden-created route, so viewers continue on the same instance.
  • @moq/crypto's signer takes the route's instance ({ path, instance }) instead of minting its own.

Bindings

  • moq-ffi:
    • MoqOriginProducer::{route, route_prefix} with MoqRouteOptions { hidden, cost, instance, epoch }, and MoqBroadcastRequest::route(options).
    • MoqRoute has broadcast(), instance_id(), announce(), withdraw(), set_cost(), async requested() and cancel().
    • The facts record MoqRoute is now MoqRouteInfo.
  • moq-c:
    • moq_origin_route, moq_origin_route_prefix (takes the request callback), moq_route_{broadcast,instance,announce,withdraw,set_cost,free} and moq_broadcast_request_route.
    • moq_announce_route is now moq_route_options (adds hidden), and moq_route is now moq_route_info.
  • Wrappers: Python, Go, Kotlin, Swift, Dart and C/C++ (generated) expose the route handle. The OBS output holds its route, and the dock shows the route's instance again.

Callers

  • Every crate that published is migrated: moq-mux, moq-stats, moq-tokio, moq-uring, moq-room, moq-crypto, hang, moq-boy, moq-bench, moq-transcode, moq-cli, moq-relay, moq-hls, moq-rtc, moq-gst, moq-rtmp, moq-rtsp, moq-srt and the fanout bench. So are the interop clients (Rust, JS, Go, Python, C++), carla, and the JS packages and tests.
  • Callers that fill a catalog first create the route hidden and announce it when ready: moq-mux ts/programs.rs, the moq-cli imports, and WHIP after SDP.
  • Every site that used to drop the publication and keep only a broadcast clone now holds its route: moq-rtc WHIP (IngestSink::routed), moq-cli transcode/hls/archive/rtc/rtsp/publish, rtmp, srt, the gst sink, the mux TS programs and the bench connections.

Review fixes:

  • Withdrawing a prefix route releases its pending requests (Rust), and closing a JS origin ends hidden or withdrawn prefix routes' request iterators.
  • A job route's answer is admitted only while its prefix route is still in the table, so one racing the prefix route's retraction can't answer a request that retraction is about to release (Rust).
  • A broadcast closing retracts its route under the route's control lock, so a racing announce() can't advertise after the close (Rust).
  • The ingress announce stats guard is set and cleared under the table lock (Rust).
  • A JS prefix route notices its origin closed: announce() and setCost() throw "origin is closed", as Rust and the JS exact route do.
  • moq-ffi and moq-c create an exact route hidden, attach the built-in catalog track, and only then announce it when asked visible. A reader that finds the route always finds catalog.json.

Docs: doc/concept/identity.md (lazy jobs), doc/lib/rs, doc/lib/js, doc/lib/{c,cpp,py,kt,swift,dart}, binding READMEs, doc/bin/obs.md, and MIGRATE entries for moq-net, @moq/net, moq-ffi, moq-c, c/moq, cpp/moq, moq-crypto and the wrappers.

Impact

  • Public API: breaking across moq-net, @moq/net, @moq/crypto, moq-crypto, moq-ffi, moq-c and every wrapper. The rs/moq-net/MIGRATE.md and js/net/MIGRATE.md entries give the full mapping:
    • publish(path, Route::default()) → route(path, RouteOptions::default())
    • create_broadcast + announce → route(path, RouteOptions::hidden()) + announce()
    • unannounce → withdraw
    • re-price → set_cost
    • requests(prefix) → route_prefix(prefix, ..)
    • prepare/publish → request.route(..)/announce()
  • Behaviour: a broadcast producer clone no longer keeps its path announced. Hold the Route.
  • moq.pro:
    • The .dash claim moves to route_prefix + request.route(RouteOptions::hidden()) + announce(). A refused announce means the request was released, which replaces the prepare/publish workaround.
    • The sync-ingest mirror becomes route_prefix(prefix, RouteOptions::default().with_instance(forwarded, epoch)) + request.serve(held), all public now. Only the hop chain stays crate-private.
  • Wire: none.

Choices made without asking

  • One Route type for exact and prefix routes, per the epic decision. On a prefix route, broadcast() panics in Rust, throws in JS and errors in the bindings. On an exact route, requested() yields nothing.
  • The raw FFI accessor is instance_id(), not instance(). uniffi-bindgen-cpp keeps every generated C/C++ object's handle in a private field named instance, so a method with that name does not compile. Kotlin renames it back to instance(). The Python, Go and Swift wrappers expose instance. Only the generated C/C++ and Dart surfaces show instance_id. Fixing the generator is a suggested follow-up.
  • No binding exposes set_auth, as before.
  • moq-c takes the prefix route's request callback at moq_origin_route_prefix, and keeps the moq_broadcast_request_* names to mirror moq-ffi's MoqBroadcastRequest.
  • Dart keeps the handle named MoqRoute, because Route clashes with Flutter. Swift takes labeled arguments, per its binding conventions.
  • moq-crypto's sign::broadcast::Producer::new(inner, key, path, instance) takes the route's instance and mints none, mirroring JS's { path, instance }.
  • moq_mux::Linger still takes broadcasts. Its owner must hold the routes, which the doc now says. Taking routes would leave moq-rtc's WHEP ingest, which has no route, with nothing to pass.
  • Smaller divergences:
    • JS routePrefix on a closed origin returns an already-retracted route instead of throwing.
    • A JS job route whose request was released also closes its broadcast.
    • set_cost/set_auth on a hidden route keep the value for the next announce.
  • Tests that covered removed behaviour were rewritten to the new behaviour, and none of their assertions were weakened.
    • standalone_broadcast_cannot_announce and moq-c's admission_must_match_the_broadcast_it_filled are deleted, because the API they guarded no longer exists.
    • "Dropping a publication keeps its route" tests now assert the reverse, which is the new ownership.
    • Hop-chain tests that needed relay metadata moved to moq_net::fuzz forwarded routes. Two of them moved to rs/moq-net-tests/tests/integration/hop_limit.rs.

Checks

On head 01e7a80 (the merge of origin/main a14995f): quest gate nix develop --command just ci origin/main exited 0, and every module passed, including check js, test js, test rs, py, c, cpp, kt, go and dart. check swift skipped itself because this host has no Swift toolchain (swift check: no swift toolchain on PATH, skipping). The merge changes no Swift, Kotlin, Go, Python, Dart, C or C++ binding source relative to ad72053, so the Swift surface is still the one reviewed earlier, and it is still not compiled locally.

Earlier runs:
On head ad72053 (the merge of origin/main f4bccd1 plus two fixes):

  • quest gate just ci origin/main on c26cd13: every module passed except three. check rs failed only on rustfmt of the new front fix. check kt failed on one AnnounceEventEnd the merge left behind. test js timed out once in the PROBE stream sends the viewers' target to an Increase peer (5 s), which passes 3/3 run alone and touches no routes. ad72053 fixes the first two. On it, just rs check, just kt check and just js test pass. The Rust tests (test rs) passed in that run, and the run before it passed every other module (py, c, cpp, obs, go, dart selection, markdown, bench, flake, quest).
  • cargo check -p moq-net (poll-only, no async) and cargo check of the out-of-workspace bench/fanout/session harness: clean.
  • moq-ffi follow_hands_out_the_instance_again_after_an_end failed 7/40 runs before the front fix below and passed 60/60 after it.
  • Swift is still not compiled: this host has no Swift toolchain.

Merges

The 2026-11-05 merge of origin/main (8022bf6) collided with moq-dev/moq#7678 (announcements are a path listing; follow reports offline), moq-dev/moq#7607 (generated async wrappers, poll-machine sessions) and moq-dev/moq#7846 (split JS datagram tracks), across 66 files:

  • The ietf publisher and subscriber take main's poll machines with this branch's renames reapplied (RouteInfo, forward, update, poll_requested).
  • Route::requested is now generated by #[cfg_attr(feature = "async", kio::wrapper)] on poll_requested, per feat!: generate async wrappers behind default-off async features #7607.
  • Relay-level tests and the moved hop-limit tests read routes(). Main's new tests (follow offline/refusal, the listing filter in announced.rs, flights, moq-boy, moq-room, moq-cli echo/archive, the JS datagram tracks, the Go, Kotlin, Python, Swift and Dart follower tests) publish through routes.
  • Re-price checks in moq-ffi and moq-c no longer expect an Update event, which apps no longer see. A fresh listing reads the new cost under the same instance instead. "A new route restarts its readers" tests now follow the path: the path stays listed, the follower is handed the new route's broadcast, and the listing next reports Absent once both routes withdraw.
  • MIGRATE files keep both sides' entries, main's first.

Two fixes on top:

  • c26cd13, the automated review's finding: a failed job admission retracts under the route's control lock.
  • c26cd13, found by the merge: a job route takes over its prefix route under the table lock before its answer wakes the front. So a front could resolve under the prefix route's instance and hand a follower an already superseded stream. Main's follow exposed this through follow_hands_out_the_instance_again_after_an_end. The front's first resolution now applies a takeover that already landed, under the table lock, as select() and FrontWatch already did once a front had resolved.

A second merge of origin/main (a14995f), after the merge queue dropped the PR on a conflict:

  • rs/moq-hls/benches/include.rs and rs/moq-hls/src/server/include_tests.rs conflicted with main's rename of contributions to include. They keep main's include names and URLs, and this branch's route.broadcast(), route.announce(), route.withdraw() and route_prefix().
  • Main's new call sites to the deleted publishing API now publish through a hidden route that announces once filled: widened_max_age.rs's legacy live-edge test, the copy-fill test in js/net/src/lite/copy_fill.test.ts (a visible route), and a_pending_subscribe_joins_the_newest_group_once_resolved in rs/moq-net/src/lite/publisher.rs. This matches pullfrog's candidate on pullfrog/7806-merge-tests-fix, written independently here. A tree-wide grep finds no other use of create_broadcast, createBroadcast, requested_broadcast or unannounce outside docs, changelogs, MIGRATE entries and the smoke and compat clients that drive released packages.

Suggested follow-ups

  • Rename the private handle field in kixelated/uniffi-bindgen-cpp's generated objects, and bump the pin. moq-ffi could then name the accessor instance() everywhere.
  • Check the Swift wrapper on a host with a Swift toolchain (kixtop).

Review

Head 9319c47352 adds two merges onto the reviewed 01e7a80:

Codex gpt-6.1-sol, medium effort, via quest review --base origin/main, session 01a1adb8, round 1: reviewed head 01e7a80 and reported no findings.

Earlier rounds:

Codex gpt-6.1-sol, medium effort, via quest review --base origin/main, session 01a1aa3b, round 4: reviewed head ad72053 and reported no findings. Round 4 is the design checkpoint: the approach is unchanged, and the merge only adapted it to main's listing, so continuing as is.

The automated review and pullfrog's shared finding on edbfb67 (a failed job admission released control before retracting) is fixed in c26cd13 and answered in this comment.

Earlier rounds:

Codex gpt-6.1-sol, medium effort, via quest review --base origin/main, session 01a1aa3b. It reviewed head 08aa622 in round 3 and reported no findings. The pushed head adds only a conflict-free merge of origin/main.

Round 1 (head d61dbdc), fixed:

  • rs/moq-ffi/src/origin.rs MoqOriginProducer::route: a visible route announced before the binding attached its catalog track, so a reader finding it at once got NotFound for catalog.json. The same held for MoqBroadcastRequest::route and moq-c. Fixed in d03c388: both bindings create the route hidden, attach the catalog, then announce.

Round 2 (head d03c388): no findings. It covered the reworked retraction fix (a0e908f).

Pullfrog's review of 05f1964 (review), all fixed:

  • AnnounceProducer::retract released the prefix route's requests after dropping the table lock, so a job route could announce in between. Fixed in a0e908f: a job answer is admitted only while its prefix route is still in the table.
  • RouteState::ended released the control lock before retracting. Fixed in d61dbdc: it retracts under that lock.
  • The ingress guard was installed after the table lock dropped. Fixed in d61dbdc.
  • js/net/src/origin.ts insert: a prefix route stayed "inserted" after origin.close(). Fixed in d61dbdc, with a test.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

@kixelated kixelated changed the title One Route handle feat!: one Route handle owns its broadcast Nov 4, 2026
@kixelated
kixelated marked this pull request as ready for review November 4, 2026 07:14

@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.

Important

Two narrow ordering gaps in the new retraction path and one JS/Rust parity break. Each fix is small, but the first can violate the documented "announcing nothing once its prefix route retracted" guarantee under concurrency.

Reviewed changes

  • Route owns its broadcast (Rust): new origin::Route handle for exact and prefix routes, RouteOptions, Request::route, crate-private forward/update; Publication, Prepared, Requests, Producer::{create_broadcast, publish, requests, requests_route, update_route} removed, and the old metadata Route renamed RouteInfo.
  • JS parity: Producer.route/routePrefix, class Route, Request.route, read-only RouteInfo; the removed exports and the non-exported wire view.
  • Bindings and callers: moq-ffi/moq-c route handles plus all wrappers, and every publishing crate migrated (hidden-until-ready for catalog fills, route handles held where a broadcast clone used to be).
  • Docs/tests: MIGRATE entries across packages, doc updates, and tests rewritten to the new ownership semantics.

⚠️ retract drains the served route after releasing the table lock

AnnounceProducer::retract removes the prefix route's table entries under the table lock, then drop(shared) before setting server.closed and draining (rs/moq-net/src/model/origin.rs:3382-3398). A concurrent job Route::announce enters update_if, whose entries check covers only that job route's own entries, then runs admit, which takes the request out of the still-open ServeState (origin.rs:3213-3232, 6170-6176). If that lands in the window after the prefix entry is removed but before the drain, the job route advertises and answers a request its prefix route already retracted, and the drain afterwards finds nothing to refuse.

The withdraw path already does this correctly by draining under the table lock (origin.rs:3337-3339), which is why the sequential tests pass; this only shows under concurrency.

Technical details
# Retraction is not atomic with releasing pending requests

## Affected sites
- `rs/moq-net/src/model/origin.rs:3355-3399` — `AnnounceProducer::retract`: entries removed under `shared`, then `drop(shared)` at 3382, then `server.closed = true` / `drain_all` at 3387-3398.
- `rs/moq-net/src/model/origin.rs:2977-3010` — `Route::announce` job branch; admit closure takes from `request.home` at 2993-2998.
- `rs/moq-net/src/model/origin.rs:3213-3232` — `update_if`: only checks `self.entries`, then calls `admit()`.
- `rs/moq-net/src/model/origin.rs:6170-6176` — `ServeState::take` guards on `self.closed` only, not on the owning prefix route still being live.
- Same class: `update_if` installs the ingress guard after releasing the table lock (`3279-3280`) while `withdraw`/`retract` clear it before taking it (`3326`, `3356`).

## Required outcome
- Once a prefix route's table entries are gone, no request handed off from its `ServeState` (job `announce`, `serve`, or `reject`) may resolve or advertise.
- `Route::announce` must return `Err(Unroutable)` for a request whose prefix route was retracted, in every interleaving, matching `origin.rs:2973-2976` and `6245-6248`.

## Suggested approach
- In `retract`, set `closed` and collect `drain_all()` while still holding the table lock (the `server` lock is taken after the table lock here, same order as `update_if`'s `admit`), then write the refusals after `drop(shared)`. Move the ingress-guard clear under the lock too, or clear it after taking `shared`.

⚠️ RouteState::ended releases the control lock before retracting

ended takes the job's pending request under control but drops that guard at the end of the statement, then calls announcement.retract (rs/moq-net/src/model/origin.rs:2936-2941). A concurrent Route::announce acquires control, sees request == None, and takes the non-job branch, which can still find the entries and re-advertise (origin.rs:2977-2982). That advertises a route whose broadcast already closed, which the docs say must fail Error::Closed.

Technical details
# Close hook is not serialized with announce

## Affected sites
- `rs/moq-net/src/model/origin.rs:2933-2942` — `RouteState::ended`.
- `rs/moq-net/src/model/origin.rs:2406-2411` — `broadcast::Producer::on_close` hook calls `ended`.
- `rs/moq-net/src/model/origin.rs:2977-2982` — `Route::announce` non-job branch.

## Required outcome
- Once the broadcast close hook starts, no `announce` may succeed on that route.

## Suggested approach
- Hold the `control` guard across `retract` in `ended`, so a racing `announce` blocks and then finds the entries gone (`Err(Closed)`). Alternatively check `broadcast.is_closed()` while holding `control` in `announce` and `change`.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

Comment thread js/net/src/origin.ts Resolved
@pullfrog

pullfrog Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Fixed the check js failure and pushed to the PR.

Cause: test/interop/clients/js/open-gop/main.ts was added on main after the PR branched and still used the removed origin.createBroadcast API, so @moq/interop-browser check failed with TS2339 (and a follow-on TS18048 for group).

Fix (commit 0eb7228d4): migrated the fixture to the new route handle, origin.route(name) + route.broadcast.createTrack(...), holding the route for its lifetime.

Verified with the package's own check (tsc --noEmit in test/interop/clients/js) and biome check --error-on-warnings on the file; both pass. The diff is a single file, 2 insertions / 4 deletions.

Task list (6/6 completed)

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

kixelated and others added 8 commits November 4, 2026 21:30
Resolves the conflicts with main's zero max-age default and datagram tracks, and
migrates code main added since the last merge to the Route handle: socket
endpoints, HLS contributions and their bench, the follow refusal tests, the
stitched TS republish test, and the new range-fill, socket, fetch and interop
clients.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rust: retracting a prefix route now closes its request queue under the table
lock that removes its entries, so a job route announcing concurrently either
lands first or finds its request released, never answers one its prefix route
already retracted. A route's broadcast closing retracts it under the route's
control lock, so a racing announce cannot advertise after the close. The
ingress announce guard is set and cleared under the table lock too.

JS: a prefix route notices its origin closed, so announce() and setCost()
throw "origin is closed" instead of silently succeeding, matching Rust and the
exact route.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Closing the retracted prefix route's queue under the table lock woke its
waiters there, where an inline executor can re-enter the table. Retraction
closes the queue after the table unlocks again, and a job route's answer is
admitted only while its prefix route is still in the table, which the
retraction leaves first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A route created visible announced before the binding attached the catalog
track every FFI broadcast carries, so a reader finding it at once could ask
for catalog.json and get NotFound. The bindings create the native route
hidden, attach the catalog, then announce it when asked visible.

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: 01e7a80

No new actionable findings in the merge-resolution delta since ad72053, checked against base a14995f.

The HLS conflict resolutions preserve main's include names/URLs and the Route ownership lifecycle. The three newly added test setups now use the replacement API with the intended visibility and retained route handles. The earlier failed-admission and front-resolution ordering fixes remain intact. The single-Route direction still fits; no change of approach suggested.

Verification: GitHub-only static review of the integration changes, not a fresh audit of all 460 files. I did not run tests. The current-head Check workflow is in progress. The PR reports its local CI gate passed with Swift skipped for lack of a toolchain.

@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

  • Merged origin/main (a14995f) onto the route handle: main's include rename, per-track Sync timelines, lite-07 route-key proofs, the C pre-encoded simulcast API, and PeerSetup::set becoming fallible now sit on the branch with its Route/RouteOptions/requested renames intact.
  • Ported main's new call sites to the route API: widened_max_age's legacy live-edge test, copy_fill's new fill test, and the lite publisher's pending-subscribe test publish through a hidden route created before the tracks and announced once filled, matching the old create_broadcast + announce order, and hold that route for the test's lifetime.
  • moq-hls and @moq/watch tests follow main's contribution → include rename while keeping the branch's route.broadcast(), route.announce(), route.withdraw() and route_prefix() calls; unannounce became withdraw and requested_broadcast became requested.
  • Re-exported RouteKey from moq_net::origin for main's lite-07 setup proof.

I read all nine conflict files against both parents. The auto-merged rs/moq-net/src/model/origin.rs keeps main's Config::route_key / OriginState::route_key plumbing intact (the field, the OriginState::new argument, and the Producer::config/route_key accessors), and no code call to the removed publishing API survives the merge. cargo check -p moq-net --all-features is clean.

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

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

pullfrog Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Fixed the failing moq-ffi test and pushed da3f1b09e to the PR branch.

Diagnosis: origin::supervision_tests::container_importers_keep_the_origin_failure_after_other_owners_are_released failed with assertion failed: matches!(completion.finished().await, Err(MoqError::Task { .. })). The test was migrated to origin.route(...).unwrap().broadcast().unwrap(), which drops the MoqRoute as a temporary and keeps only broadcast handles. Under the new ownership the route is the origin's owner (RouteState::_keepalive); a broadcast producer is not. Once the origin producer and the route were both gone, the driver could poll Closed and finish cleanly before the injected panic, so the test raced the driver and only lost under CI load.

Confirmed deterministically: dropping the route and waiting 200 ms reproduces the exact assertion failure, while holding the route passes 15/15 runs.

Fix:

  • Hold the route through the failure check, then drop it before the importer assertions so only the importers still carry the origin's failure — the test's original intent, not weakened.
  • Clarified the Route doc in moq-net: the route, not the broadcast, keeps the origin's Driver running.

Verification: cargo test -p moq-ffi --lib -- origin::supervision_tests (7 passed), cargo fmt --all -- --check, and cargo clippy -p moq-ffi --lib --locked -- -D warnings all pass.

Note: the branch was briefly locked in the merge queue, so the fix was first proposed as #7989; that landed on the PR branch and #7806 now points at da3f1b09e.

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

The container-importer supervision test dropped the route it created as a
temporary, leaving only broadcast handles. Under the new ownership the
route is the origin's owner, so the driver could observe every owner gone
and finish cleanly before the injected panic, making the test race the
driver and fail under load. Hold the route until the failure is recorded,
then drop it before the importer assertions.

Also spell out that the route, not the broadcast, keeps the driver
running.
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Nov 5, 2026
@moq-bot

moq-bot Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

quest land watch: the merge queue removed this pull request at 01e7a80 because it conflicts with main (merge_conflict).

The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.

(written by quest land watch)

Drop the muted-rendition hold test that #7942 removed, and keep this
branch's deletion of the completed route/handle quest.

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: da3f1b0

No new actionable findings in the two-file delta since 01e7a80.

Holding the route until completion.finished() records the injected failure prevents the origin driver from ending cleanly first. Dropping it before the importer assertions preserves the test's failure-retention coverage. The clarified lifetime documentation matches RouteState::_keepalive. This is consistent with the existing Route ownership direction; production behavior is unchanged.

Verification: GitHub-only static review of the patch, surrounding supervision tests and driver completion logic; I did not run tests. GitHub currently reports merge conflicts and the current-head Check runs are skipped, so this is not a verified CI pass.

@moq-bot

moq-bot Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

codex / Codex land fixer note:

The PR deletes a planning file modified on base and migrates a playback test deleted on base. Both conflicts have mechanical resolutions: retain the planning-file deletion and retain base's test deletion. The trial merge was aborted and the PR branch remains at its pinned head because validation is blocked.

Failure log evidence:

18: CONFLICT (modify/delete): quest/m1/api-audit/route/handle.md deleted in 01e7a80 and modified in HEAD. Version HEAD of quest/m1/api-audit/route/handle.md left in tree.
25: CONFLICT (content): Merge conflict in rs/moq-cli/src/play/media.rs

Both cargo build -p moq-cli --features play and cargo test -p moq-cli --features play --bin moq failed before tests could run:

The system library alsa required by crate alsa-sys was not found.

Provide ALSA development libraries and rerun validation after merging base.

(written by codex / Codex)

@moq-bot

moq-bot Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

quest land watch: the land fixer's result for 01e7a80 was not applied: its head moved to da3f1b0 while the fixer ran, and a push by anyone but the land fixer ends the pick.

(written by quest land watch)

…ndle

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated added this pull request to the merge queue Nov 5, 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

  • Merged origin/main at f3ddaee5b onto the route handle: main's #7975 quest audit and #7942 push-to-talk change (enabled: false now means "not selectable", so held disabled renditions are gone from moq-cli, moq-gst, moq-ffi, moq-c and the JS watch decoders) now sit on the branch with its Route/RouteOptions/requested names intact.
  • Kept the branch's route migration through the merge: all 28 files both sides touched retain the branch's changes relative to main's parent — checked per file against both parents rather than by eye. The catalog-before-announce, set_cost, withdraw and route_prefix/request migrations all survive.
  • Took main's moq-cli media test rewrite and re-applied the route migration to the surviving tests: main's stop_disabled coverage keeps the branch's route helpers, and the muted-rendition hold test the branch had migrated is deleted because #7942 removed that behaviour.
  • Merged #7989: the moq-ffi supervision test holds the Route through the injected panic and drops it before the importer assertions, so the origin driver cannot finish clean before the failure is recorded; the Route doc now credits the route rather than a bare Producer with keeping the Driver running, matching RouteState::_keepalive.
  • Quest cleanup: quest/m1/api-audit/route/handle.md stays deleted against #7975's edit, and routes/session.md drops the landed lite-07 wire quest link.

I re-checked the merge resolutions structurally: for each file touched by both main and the branch, the merge differs from main by exactly the branch's migration counts, and a tree-wide grep finds no surviving call to the removed publishing API. Holding the route in the FFI test is the right fix — the route, not the broadcast, is the origin driver's remaining owner there — and dropping it before the importer assertions preserves the failure-retention coverage the test is about.

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

Merged via the queue into main with commit d6fcf6d Nov 5, 2026
9 of 10 checks passed
@kixelated
kixelated deleted the quest/m1/api-audit/route/handle branch November 5, 2026 23:56
kixelated added a commit that referenced this pull request Nov 6, 2026
#7806 replaced OriginProducer.create_broadcast with route().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated added a commit that referenced this pull request Nov 6, 2026
#7806 replaced origin::Producer::publish with route().

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant