Repository navigation
feat!: announcements are a path listing; follow reports offline - #7678
Conversation
Quest-Claim: 2e76cee834ff4e3a
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>
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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 73bac21
- [P2] Preserve compatible instance matching when following a rendition (
rs/moq-mux/src/source.rs:392). Passing the catalog's instance directly to the network follower requires an exact route ID. Lite versions before 07 and moq-transport mint route IDs because the wire carries none, so a valid pinned sibling rendition can wait indefinitely instead of playing; the previous unpinned follow pluscheck_instancedeliberately accepted minted IDs. Direct pinning also lets a late follower resolve an older, still-routable instance after another has won, whereas the previous code waited for the named instance to serve again. Follow the winning route and check the returned instance with the existing compatibility rules, or provide equivalent behavior in the network follower. - [P2] Keep watching a listed participant after a resolution refusal (
rs/moq-room/src/room.rs:137). If a handler rejects the initial request, this permanently clears the follower. An actual publisher can then replace that claim without the path becoming absent; the listing suppresses its restart and the room never emits the recovered camera. Previously the restart caused another request. Retain or re-establish a watcher with bounded retry while the path is present. The same stop-on-error pattern also affectsrs/moq-cli/src/archive.rs:139anddemo/carla/sim/moq_carla_sim/publish.py:362. - [P3] Reset the aggregate baseline on a node restart (
demo/web/src/stats.ts:197). The follower clears only that node's history, while the sampler identifies the cluster solely by its unchanged node names. A restarted node's counters therefore splice into the old aggregate history, producing false zeroes or spikes in cluster rates. Invalidate the aggregate history or membership baseline along with the node's history.
Verification: static inspection through GitHub only; I did not run tests or builds.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This reworks origin announcements into a path listing (each path present or absent) and moves instance/restart/offline observation to follow, breaking in every language.
moq-netlisting model:origin::Consumer::announced()yields newannounce::Event::{Present, Absent}(model/announced.rs), a filter over the route stream. The old five variants survive only as a hiddenorigin::Routes/RouteEventreached viaConsumer::routes(); the crate-privateroutes(path)becamepath_routes(path).Followingreports offline:available()is nowResult<Option<broadcast::Consumer>>, withNonewhen the path goes offline (or a pinned instance is taken over). Newlive/ended/end()state machine.moq-muxfold:follow::Instances/follow::Eventfold intoFollowing;Source::followkeeps a thinInstancesfor the catalog's one-shot hand-out.- JS + bindings + C ABI:
Announce.Event.kindis"present" | "absent", internalroutes()/list();MoqAnnounceEvent::{Present, Absent}; optionalavailable()across all wrappers;MOQ_FOLLOW_OFFLINE(INT32_MAX) plus an ID allocator capped atINT32_MAX - 1. - Callers/docs: relay, stats, CLI, gst, room, boy, demo and interop clients migrated; MIGRATE notes and concept docs updated;
quest/m1/api-audit/route/events.mddeleted.
I read the full diff and verified the three highest-risk surfaces: the Following end state machine (no missing/double end, no None on first call; the lone Ok(None)-after-still-served case is the deliberate Follow gap end), the AnnounceConsumer::list filter (a faithful per-prefix projection with no same-prefix duplicate or spurious absence), and offline-None handling across every wrapper (offline is consistently distinguished from error and from "still waiting"), including the C sentinel and the allocator bound.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
|
Fixed the failing Task list (5/5 completed)
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ute/events # Conflicts: # cpp/moq/MIGRATE.md # dart/moq/MIGRATE.md # go/wrapper/MIGRATE.md # kt/moq/MIGRATE.md # py/moq-rs/MIGRATE.md # rs/moq-c/MIGRATE.md # rs/moq-ffi/MIGRATE.md # swift/MIGRATE.md
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head 6f3ad66 against base 151ed38.
The substantive follow-up replaces the C callback's overloaded status/sentinel with explicit online, offline and terminal events, moves follows into a separate cancellation slab, and restores the full positive handle range. I found no new actionable issue in that change or its merge integration.
The three findings in the earlier review remain open: pinned sibling renditions still pass the instance directly to the network follower, a refused participant follower still stops while its listing remains present, and node restart still clears only per-node history rather than the aggregate baseline.
Static GitHub-only review of the C callback lifecycle, cancellation, affected callers and prior findings. I did not run builds, tests or interop.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
ℹ️ No critical issues — one minor test cleanup inline.
Reviewed changes
This increment replaces the C follow sentinel with an explicit event stream and integrates main.
- Explicit C follow events:
moq_origin_follownow takesmoq_follow_callbackovermoq_follow_event(ONLINE/OFFLINE/END) instead of overloading a status code withMOQ_FOLLOW_OFFLINE; follows moved to their ownTask<OnFollow>slab with a generic terminal callback. - Full handle range restored:
id.rsno longer reservesINT32_MAX, so the C ID allocator wraps the whole positivei32range again. - Callers and docs: the C interop subscriber,
rs/moq-c/c/moqMIGRATE notes,doc/lib/c, anddoc/concept/identity.mdmoved to the event API; the merge re-applied main's#7697Delaydocs and fixedtest/max-age/client.tsto wait onpresent.
Verified locally: cargo check and clippy -p moq-c pass, cbindgen emits the moq_follow_event enum and callback typedef, and test/interop/clients/c/subscribe.c compiles against the generated header. The follow tests could not run in this environment because libpipewire-0.3 is missing.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Archive export fails on a refused listed path:
follownow returnsanyhow::Resultandexportpropagates a follower error fromJoinSet::join_next, ignoring only the abort that follows anAbsent; a handler refusing a listed path now fails the export instead of silently dropping that follower. The newa_refused_path_fails_the_exporttest covers it.
Error::Closed still ends the follower cleanly, and path-gone aborts are filtered via JoinError::is_cancelled. No new issues in this increment; the earlier inline test cleanup is still open, so this stays a comment review.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏
…ter stats on restart Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main's #7836 retries a refused follower on any serving-route change. Drop its test that duplicated this branch's refusal-after-takeover test, and say the follower waits for a route change, not another instance. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The listing reports nothing new when a publisher takes over a path that stays present, so a reader that quit on a refusal never saw the takeover. Only an origin close ends them now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nsumer Main's #7736 has moq.json.SnapshotConsumer take the broadcast and track name; publish.py and its test fake still used the subscribed track. 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>
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run covers d2b04d0..a18da85: two merges of main, the author's refusal alignment and caller fixes, and the adaptations main's new code needed.
- Kept main's
Followingrefusal rule and dropped its duplicate test: therefusedflag now re-asks on any serving-route change (Update/Takeover, not just another instance), andannounced_waits_after_refusing_a_closed_instances_successoris gone becauseannounced_does_not_retry_a_refusal_after_a_takeovercovers the same closed-instance path and also asserts the owedNone. Dropping main's!refusedguard on the closed-broadcast check is correct — a refusal already clearslast. - Refused followers stay alive in
moq-boyandmoq export echo: both now select onfollowing.available()with a boxed reader, returning only onError::Closedand logging other refusals while the reader keeps running. New tests cover a publisher taking over after a refusal. - Carla reads driver control through the self-subscribing snapshot consumer:
SnapshotConsumer(broadcast, "control")plus matching test fakes, following main's#7736. - Ported main's new callers to the listing and optional result: main's datagram-track, range-fill, stream-credit, warmup and JSON-snapshot work now uses
routes()or theOptionfromavailable()across the model, relay, tokio, HLS and bench tests;socket::Step'sexpectis safe becauseResolvingis only the first state. - Fixed the fan-out bench harness to read the optional
available().
MIGRATE ordering is intact (#7678 stays on top, including the hand-written C ABI section). I could not run the Rust suite here (libpipewire-0.3 is missing), and the head's Check was still in progress.
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: a18da85
The two findings from my earlier review remain open, rather than new regressions:
- Carla still continues on a replacement refusal before cancelling the active driver (
demo/carla/sim/moq_carla_sim/publish.py:356–360). Existing subscriptions survive replacement, so the old driver can retain its lease and continue commands. - Archive still treats a handler's
Error::Closedrefusal as origin termination (rs/moq-cli/src/archive.rs:149–153), although a handler can reject with that error while the listing remains. The new refusal-recovery paths inrs/moq-boy/src/input.rs:114andrs/moq-cli/src/report.rs:574have the same closed-refusal hole and can miss a later takeover. Distinguish refusal from actual origin termination.
The new retry-on-serving-route-change policy otherwise fits the follower model, and the earlier sibling-instance and room-retention fixes remain present.
Verification: focused GitHub-only static review of the incremental lifecycle paths. I did not run builds, tests or interop.
Ports #7678's route listing and Following changes into the IETF namespace machines and moq-hls's follow machines, takes its Activity::poll_close, and links Following::poll_available so poll-only docs resolve. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ports main's announcement listing and follow (#7678), split JS datagram tracks (#7846) and generated async wrappers (#7607) onto the Route handle: the ietf session layer takes main's poll machines with this branch's route renames, main's new tests publish through routes, and re-price checks read a fresh listing now that apps no longer see updates. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Implements Listing and follow and deletes the quest.
Applications now read announcements as a listing: each path is present or absent. A restart, takeover, or route re-price no longer surfaces as an announce event; readers that need instance changes hold a
follow. The five-variant stream survives only as a hidden, relay-levelroutes()that the sessions, moq-relay, and moq-stats use.Changes
Rust (
moq-net)origin::Consumer::announced()returnsannounce::Consumer, yieldingannounce::Event::{Present, Absent}(newmodel/announced.rs, a thin filter over the route stream).origin::Consumer::routes()(#[doc(hidden)]) returnsorigin::Routesoforigin::RouteEvent(the oldStart/Update/End/Restart/Takeover). Every session, relay and stats caller moved to it. The old crate-privateroutes(path)is nowpath_routes(path).Following::available()returnsResult<Option<broadcast::Consumer>>:Noneonce the path goes offline, or, when pinned, once another instance takes it over (the pinned instance is handed out again when it returns). The first call never returnsNone. After a resolution failure, such as a handler refusing the path, the next call waits for another instance instead of asking the same handler again, so callers keep their follower.moq_mux::follow::Instancesfolds intoFollowing:Instances::newandfollow::Eventare gone.Source::followkeeps a thinInstancesfor its one-shot hand-out of the catalog's own broadcast, whichmoq-gstneeds, and for a named sibling instance: that is followed unpinned and checked withcheck_instance, since a pin never matches an instance an older hop minted.Restartmove to a follower per listed path:moq-boyviewers,moq-room(Roomholds a follower per path, as room-bench expects),moq export echo, the archive recorder, and thehangsubscribe example. The rest only merged variants and now readPresent/Absent.JS (
@moq/net)Announce.Eventis{ kind: "present" | "absent" } & Announce. Connections and origins keep an@internalroutes()(stripped from the API report) thatconnection/forward.tsand the pool pump use.Announce.Produceris@internaltoo, since it was never exported.@moq/watch,@moq/room,@moq/boy, the demo dashboard (which now follows each stats node) and carla's web player move topresent/absent.Bindings
MoqAnnounceEventisPresent { announce }/Absent { announce };Takeover'spredecessoris gone.MoqFollowing::available()returns an optional broadcast: none once the path goes offline (or, when pinned, once another instance takes it over), mirroring Rust and JS. Python returnsNone, Go(nil, nil)(asNextGroupdoes at track end), Kotlin and Dartnull, Swiftnil, and C++ a nullshared_ptr(uniffi-bindgen-cpp mapsOption<Arc<T>>that way).moq_announce_kindisMOQ_ANNOUNCE_KIND_PRESENT(0) /_ABSENT(1);moq_announce_eventlosespredecessor/predecessor_len.moq_origin_followtakes a newmoq_follow_callback(void *user_data, moq_follow_event event, int32_t code):MOQ_FOLLOW_EVENT_ONLINEwith the broadcast handle,MOQ_FOLLOW_EVENT_OFFLINEwith0, and one terminalMOQ_FOLLOW_EVENT_ENDwith0or a negative error thatmoq_error()explains inside the callback, which is moq-c's existing terminal-code convention. Internally, follow tasks get their own slab and aTerminaltrait delivers each callback type's end.moq_origin_follow: a null broadcast logs the offline and waits for the next one, holding the last frame as before.moq_ffi/lib/src/moq.dart) and Swift wrappers, their READMEs, tests anddoc/lib/*samples. Restart tests become "a follower gets the new instance while the path stays listed".publish.pyuse the listing plusfollow. The Go client is gofmt-clean; the Python one already passedruff format.Docs:
doc/concept/identity.md(followers),doc/setup/redundancy.md,MIGRATE.mdentries for moq-net, moq-mux, moq-ffi, moq-c, the C header, @moq/net and every wrapper.Impact
announce::EventisPresent/Absent. MapStarttoPresent,EndtoAbsent, dropUpdate, and replace "request again onRestart" withconsumer.follow(path, None).Following::available()returns anOption.moq_mux::follow::Instances::newandfollow::Eventare removed. Relays use the hiddenconsumer.routes().Announce.Event.kindis"present" | "absent"; followorigin.follow(path).activefor instance changes.MoqAnnounceEvent::{Present, Absent}; holdfollow(path)for restarts.available()may return none: handle it as offline. Go keeps its signature but can return(nil, nil).MOQ_ANNOUNCE_KIND_PRESENT/_ABSENT,predecessorfields removed.moq_origin_followtakes amoq_follow_callback: read the handle onMOQ_FOLLOW_EVENT_ONLINE(where a positive code was), and releaseuser_dataonMOQ_FOLLOW_EVENT_END(where a code<= 0was).Decisions
Answered by the maintainer on this PR, 2026-11-03 (the chosen option is marked ✅):
Followingnow reports? Keep skipping ends; ✅ report it:MoqFollowing.available()returns an optional broadcast in moq-ffi and every wrapper, and C'smoq_origin_followcallback gets a distinct offline code.Present/Absent(JS"present"/"absent");Active/Ended;Announced/Unannounced.Following::available() -> Result<Option<broadcast::Consumer>>; afollow::Event { Start, End }enum.Answered after the maintainer's API review, 2026-11-03:
moq_origin_follow's callback takes amoq_follow_event(ONLINE, OFFLINE, END) beside its code; keep theMOQ_FOLLOW_OFFLINEsentinel; rethink more.MOQ_FOLLOW_OFFLINEand the ID allocator cap are removed, and END carries errors through moq-c's negative-code convention.Choices made without asking
routes()rather than to the listing: moq-net's own tests and benches, moq-relay, moq-stats, and the moq-cli, moq-gst, moq-rtmp, moq-tokio and moq-net-tests tests that assert restarts, updates or hop changes.code, nothandle, since END carries the terminal status there.moq export archivefails when a handler refuses a listed path, as the recording request it replaced did. moq-room and carla instead keep following past a refusal.BroadcastConsumerexposes no instance; a follow'sNonenow ends the driver's run.Updateevents reached apps (Pythontest_route_update_observes_restart, carla's "keeps the lease across route updates"). Apps no longer see re-prices.origin/mainseveral times, taking main's poll-native moq-ffi (moq-dev/moq#7497) and reapplying this PR's changes on top. Later merges joined MIGRATE entries, kept a JS integration test main deleted deleted, and moved main's new relay-level tests (route_dies_across_instances, the lite subscriber sync test,cluster_fresh_port) toroutes().origin/main(after the merge queue dropped this PR on a staleannounced()call in main's newmesh_withdraw.rs):Followingrefusal rule from moq-dev/moq#7836: after a refusal, any change to the serving route asks again, even the same instance over another route. This replaces this PR's narrower "wait for another instance". This PR already forgets the broadcast handed out last on a refusal, so main's extra!refusedguard on the closed-broadcast check was redundant and is dropped. Main'sannounced_waits_after_refusing_a_closed_instances_successoris dropped too: this PR'sannounced_does_not_retry_a_refusal_after_a_takeovercovers the same case and also asserts the owed end.mesh_withdraw.rsand other relay-level tests toroutes(); the socket endpoint step (model/socket.rs, whose only call is the first, which is neverNone), the HLS contributions follower (follow::EventtoOption), the moq-gst, moq-audio, moq-ffi, moq-relay, moq-tokio and moq-net tests, and the fan-out session bench harness to the optionalavailable(). That harness builds against each base revision, so a base from before this PR now reports it unsupported, asbench/run.shalready does for an API a revision lacks.moq export echo, the demo stats page, and carla. carla'spublish.pystill used the oldSnapshotConsumer(track)on main as well, which fails its pyright check; it and its test fake now pass(broadcast, "control").Checks
On the final head, a18da85:
just ci origin/main(the landing gate): every module passes, including 10857 Rust tests, JS, Python, carla, C, C++, OBS compile, Go, Kotlin, Dart and Swift selection, markdown andquest check.cargo checkof the out-of-workspacebench/fanout/sessionharness: clean.Earlier on this branch, and unaffected by the later merges:
just py check,just py test,just go check,just kt check,just cpp check,just obs test, andjust test interop --all(185 checks). Swift was edited but not compiled: there is no Swift toolchain on this Linux host.Suggested follow-ups
MoqBroadcastConsumerexposes no instance in moq-ffi, so carla's Python lease uses a local counter. Aninstance()accessor would let it key on the real instance.rs/moq-ctest::raw_track_subscription_options_and_updatefailed once (-27from a close) and passed four reruns: a flake unrelated to announcements.cpp/obstest/moq-output-test.cpp:909("stop drains the session") failed once under a load average near 33 and passed on rerun; it does not touch the follow path.test/smokeclients (Kotlin, Swift, Python) install published packages and still call the pre-renameannouncedBroadcast(...).available(); after this ships they need the optional result. Nothing builds them from this tree.rs/moq-pattern/benches/matching.rsfailscargo clippy --all-targetson main, with default features too: overlapping match ranges (..64,..16384) from moq-dev/moq#7536. Main's Check does not catch it because its clippy gate rides the test build, which compiles no bench targets. Out of scope here.Review
Codex
gpt-6.1-sol, medium effort, viaquest review --base origin/main, a new session on this branch:rs/moq-boy/src/input.rs:105: a viewer whose handler refused it stopped being read, and a publisher taking over the still-listed path was never picked up.follow_viewernow ends only onError::Closed. Test:input::tests::a_refused_viewer_is_read_once_a_publisher_takes_over.rs/moq-cli/src/report.rs:577: the same formoq export echo. Test:report::tests::a_refused_feedback_path_is_read_once_a_publisher_takes_over.Head a18da85 adds a conflict-free merge of
origin/main(cfed1ec) and one mechanical line inbench/fanout/session/src/main.rs(.expecton the now-optionalavailable()), skipped as too trivial for another round.Earlier sessions (6 rounds, ending with no findings on 3582fd9) fixed three findings, each with a test: a refused moq-room participant dropped (
reports_the_absence_of_a_refused_participant), the archive swallowing a refusal (a_refused_path_fails_the_export), and a refusal re-armed by a closed broadcast's movement (announced_does_not_retry_a_refusal_after_a_takeover). The automated GitHub review and pullfrog's two inline threads are fixed and answered (comment).Overlap
moq-dev/moq#7846 (open, split JavaScript datagram tracks) touches 10 of the same
js/netfiles:MIGRATE.md,api/index.api.md,ietf/connection.ts,ietf/subscriber.tsand its test,integration.test.ts,lite/compression.test.ts,lite/connection.ts,lite/subscriber.tsand its test. Whichever lands second merges the other.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code