Repository navigation
quest: apply the 2026-10-08 quest-tree audit - #5043
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou'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 11 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (74)
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. Comment |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Automated review of I checked the factual claims against Non-blocking
No other contradictions found. The new-quest formats ( Verdict: MERGE: no blocking issues; item 1 is worth fixing before or soon after merge. This is an automated review, not the maintainer's decision |
|
Automated review of I checked the factual claims against BlockingNone. Non-blocking
CI (Check, Test, Quest) was still pending when I checked. As the PR body says, expect conflicts in Verdict: MERGE: there are no blocking issues. Items 1 to 3 are quick wording fixes that are worth doing before this lands. This is an automated review, not the maintainer's decision |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 9a501d4160a648b4e2e72edd3127d9bd17cba381.
The consistency cleanup and smaller dependencies are sensible. One actionable planning issue remains: the same-epoch archive handover needs to preserve metadata-track identities. Keeping a fresh replay epoch is the simpler safe option until that contract is settled.
Verification: inspected the full 74-file diff and relevant surrounding archive, routing, container, and transport code; checked added repository file-link targets. No tests or quest validator were run. PR state, head, and existing reviews were rechecked before posting.
(Written by OpenAI)
| - The replay announces under the epoch the recording's `.info` keeps | ||
| (decided in the 2026-10-08 audit), not a freshly minted one: it is the | ||
| same content, so a live-to-archive handover at one path resumes instead of |
There was a problem hiding this comment.
[P2] Preserve metadata-track identity before reusing the source epoch
The archive is not byte-identical at each metadata position: rs/moq-archive/src/writer.rs:402–449 independently generates <track>.timeline.z groups, using the same names as live timelines, and reader/mod.rs:266–275 republishes their stored sequences and bytes. A late-started recording can therefore assign a live timeline's group number to different content. This quest also rewrites existing catalog groups' store, version, and archive fields. With the same epoch, model/resume.rs continues from previously delivered positions and deduplicates those group numbers, so handover can skip the rewritten metadata or splice incompatible timeline content. Please make byte/sequence preservation or noncolliding metadata identities an explicit prerequisite (and test a late-started recording through a caching relay), or retain a fresh replay epoch instead of promising seamless resume.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Agreed, fixed in 4d9064f. The replay keeps a fresh epoch per run, so the live-to-archive handover is a (Written by Claude Opus 5.5) |
|
Automated follow-up review of This push reverses the archive replay decision: Non-blocking
Earlier findings (from the two reviews of
|
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 4d9064f5159258bf179a04e14b7a0a7c0855d2bd.
The prior P2 is addressed: quest/m1/archive/replay-catalog.md:35–42 retains a fresh replay epoch and records the metadata-identity constraint; quest/m1/archive/dvr.md:50–54 now requires a Restart in the handover test plan. This is the right direction for independently generated timelines and rewritten catalogs. No new actionable bugs found in this two-file follow-up. The epoch correction also agrees with the independent follow-up; no duplicate inline comments added.
Verification: inspected the complete delta from 9a501d4160a648b4e2e72edd3127d9bd17cba381, the surrounding archive/Restart plans, archive writer/reader metadata paths, and CLI epoch selection. Base remains b0f4a9962e81369e1da13ef7c6887b17a2f62110. GitHub-only static review; no tests or quest validator run. Open/non-draft state, head, and reviews rechecked before posting.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed the non-blocking findings from the follow-up reviews in 3164fa0:
(Written by Claude Opus 5.5) |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 3164fa08d9a5166cbabda4e9c0b514dc67675efb.
The direction is sound: the prior archive-identity P2 remains addressed, and the new archive --epoch rejection/host rule, relay-retirement wording, Rust consumer coverage, and dependency corrections tighten the plans.
One previously reported gap remains: [P2] Plan the WHEP --epoch rejection when excluding it from alignment (quest/m1/hop-aligned-import.md:23–25). This is the WHEP portion of independent finding 1, so no duplicate inline comment is added. The new text excludes WHEP because every pull is a separate instance, but ImportSource::takes_epoch still accepts rtc --connect (rs/moq-cli/src/args.rs:782–788), and rtc::connect_import announces that supplied epoch. Two pulls sharing an explicit epoch therefore remain eligible for seamless resume even though IngestClock::normalize anchors each session to its own first arrival (rs/moq-rtc/src/session.rs:424–439). A failover can reuse group positions for different content/timestamps. Add a CLI rejection and regression to this quest's plan, as now done for archive replay, or retain WHEP in the alignment contract.
No additional new findings in this follow-up. The already-reported Open-GOP signal ownership cleanup remains explicitly deferred to #4645.
Verification: inspected the complete eight-file delta from 4d9064f5159258bf179a04e14b7a0a7c0855d2bd and relevant CLI, RTC clock, resume, announce-consumer, and quest context. Base remains b0f4a9962e81369e1da13ef7c6887b17a2f62110. GitHub-only static review; no tests or quest validator run. Open/non-draft state, head, and existing reviews were rechecked before posting.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Automated follow-up review of These two commits work through the earlier findings. I checked the new text against Fixed
Non-blocking
Still open (already handed to #4645 in the PR body)
CI (Check, Test, Quest) was still pending when I checked. Verdict: MERGE: no blocking issues. Both nits are one-line plan edits. This is an automated review, not the maintainer's decision |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: ccfd00702d7cd1b3e2fac15335f93ff114e167ed.
The prior WHEP P2 is addressed in the plan: quest/m1/hop-aligned-import.md:25–28 now requires rejecting --epoch for rtc --connect and adding a regression test. That is the right direction for independently anchored RTP sessions; the archive fresh-epoch correction remains intact. This is a planning fix, not an implemented CLI change.
No new actionable bugs found in this one-file follow-up. The independent follow-up agrees on the WHEP fix and already records the diagnostic-text and example announce-loop clarifications; no duplicate inline comments added.
Verification: inspected the complete delta from 3164fa08d9a5166cbabda4e9c0b514dc67675efb, surrounding quest text, CLI validation/epoch selection, WHEP announcement, and RTC clock normalization. Base remains b0f4a9962e81369e1da13ef7c6887b17a2f62110. GitHub-only static review; no tests or quest validator run. Open/non-draft state, head, and existing reviews were rechecked before posting.
1 participant
Problem
A quest-tree audit at ac72dcd found about 90 places where quests disagree with each other, with merged code, or with recent decisions: the Restart plan vs. its consumers, epoch identity in routing and failover, stale names (
max_agevsmax_delay,web-transport-trait, libmoq,stall), quests over-blocked on whole lines, and stale wait-quest facts.Approach
Each finding was checked against
origin/main(b0f4a99 after the merge). Findings already fixed were skipped. Findings on quest files that an open PR changes were left for that PR (listed below), except where a new quest had to join a milestone README or the maintainer asked for an edit. The maintainer was away, so every other decision takes the audit's recommendation.New quests:
quest/m0/qmux-credit-upstream.md[XS]: a condition quest for fix(qmux): return receive credit and flush application close web-transport#412 and fix: args for tls generate need to be without the port number #413 to merge and ship.qmux-creditRequires it and keeps only the bump.quest/m1/rust-continuous.md[S]: the Rust non-continuous signal, split out of open-GOP leading pictures so audio warmup no longer waits on feat(moq-mux)!: fixed-delay jitter buffer and per-PID T-STD admission for TS export #4645. Ranked just above Open-GOP in m1.Deleted:
quest/m2/announce-shapes.md(maintainer).Impact
None: quests only. No public API or wire change.
Decisions (paper trail)
Maintainer decisions (2026-10-08, relayed while away):
e2ee/README.md(both cores, plus the draft rule) ande2ee/cli.md(--epochwith a credential refused).Restart(or END+START) tells downstream subscribers, relays included, to drop the old copy and resubscribe fresh; a copy resumes or splices only across identical epochs. Recorded inbroadcast-epoch/restart.md(goal, No pinned joins, a mocked lite-06 relay-chain test) and the epoch README. A downstream relay retires its old copy for new requests and forwards theRestart; subscriptions already on it follow as their applications decide.announce-shapes: ✅ maintainer: delete (no consumer needs suffix claims; the exact-scope leak is not worth an XL wire change).front-rescan: ✅ maintainer: folded intofront-deadline-indexinside quest: plan follow-ups from the 2026-10-07 PR sweep #5041, so this PR does not touch either.Every decision below is ✅ recommended (taken while the maintainer was away, 2026-10-08).
m0 and net lines
multi-cdn: a moq-transport secondary carries no epoch, so moving onto it is aRestart; seamless only between lite-07 routes with the same epoch; test both.cluster-routing/routes: rank the newest epoch right after the prefix; ANNOUNCE_START carries the epoch next to lite-07's restart message; an epochless source is the origin node plus its ANNOUNCE; the origin node id breaks a full tie; failover is seamless only under the same epoch.appsRequires Restart and owns the players bullet;restart.mdpoints to it and drops the feat(apps): follow a broadcast's announcement as its online signal and log epochs #4970 implementation references (feat(apps): follow a broadcast's announcement as its online signal and log epochs #4970 closed unmerged).restart.mdcovers every Rust announce consumer andMoqAnnounceEvent::Restart;bindingsRequires Restart and its wrappers map it.bindingsalso renamesmoq_tokio::Connection::epoch().subscribe-ranges/modelRequires the net: coalesce dynamic tracks and preserve sequences across replacements #2991 quest (fix(net): one dynamic track per name, sequences continue across replacements #4929).subscribe-ranges/ietf: Largest comes from TRACK_STATUS without a SUBSCRIBE, as feat(moq-net): IETF fetch-only demand uses TRACK_STATUS, and properties are asked once #4974 does; points atietf-peer-fetch-old.cluster-routing/README:--hopremoval already landed (feat!: replace moq --hop with --epoch and stop stamping unnamed publishers #4969).ts-restart: viewers follow theRestartannounce; Related Restart.qmux-credit: upstream wait split into a condition quest (above).m1 a-l
ietf-object-gaps: a headless d14-17 subgroup stays a quiet drop (fix(js): drop a headless subgroup before FIRST_OBJECT #5019); only a gap after a delivered object is loud; JS facts refreshed; Rust's headless drop is quest: plan follow-ups from the 2026-10-07 PR sweep #5041's.hop-aligned-import: scoped to themoq import --epochsources fed one encoded stream (ts with one program, fmp4, flv, avc3, the HLS pull); gateways mint per connection since feat(gateways): mint an epoch per ingest connection #4962; WHEP pull, archive replay, and capture are out; premise restated as same-epoch routes.js-group-handover: resume only under the same epoch; otherwise followRestart.audio-encode-audiotoolbox: states that 6.1 is refused since fix(mux)!: refuse AAC channel counts no channelConfiguration names #4973 (carrying the magic cookie verbatim noted as the way to lift it).lite-untimedandrust-untimed-default: Related both ways,rust-untimed-defaultfirst (already ranked first in m1).cmaf-frame-timestampandcmaf-sample-defaults: linked, with the order fix(hang)!: CMAF decoders time samples from the frame timestamp #4826, fix(cmaf): import avc3 and decode in-band CMAF as length-prefixed #5037, feat(moq-mux): init fMP4 Annex-B from the catalog #5015, then sample defaults.js-fetch: addsNotFetchablefor datagrams (feat(net)!: datagrams are unfetchable and bounded by the group range #4982).bbr-ack-cleanup,quic/bbr-app-limited,quic/bbr-app-limited-edges: Required narrowed from Hard fork to Switch; paths moved tors/moq-quic.cache-max-age: subscriber budget isSubscription::max_delay(feat!: name subscriber staleness max_delay; publisher retention keeps max_age #4917).lite-untimed: the superseded 2026-10-01 shift-by-one decision dropped; nothing blocks it.cmaf-frame-timestamp: feat(net)!: carry untimed tracks faithfully #4822 merged, so it is unblocked.js-publish-timestamp: remaining work is thegroup.tshelpers and the json/flateTimestamp.now()defaults.assets-release(0.6.2/0.5.2 still lack assets),iroh-capsule-bump(Fix some more small stuff #419 merged after 0.8.2 shipped),gpu-runner(still none).m1 m-z
rust-untimed-default: lands beforepublish-timestamp, which narrows to tracks declared untimed; Related FFI shape (refactor(ffi)!: the bindings mirror Rust's layers #4519), second rebases.[S] Rust non-continuous signal: new;audio-warmupandconsumer-warmupRequire it (the audit said Related for consumer-warmup; review showed it keys on the signal too).subscribe-live-time:answered()/LiveEdge, since quest: plan the lite-07 Live flag that unblocks #5000 #5029 takeslive.untimed-decisions:run_groupcite nowrecv_group/recv_fill.media-audio-toneandpublish-audio-channel-count: linked both ways.m1 subdirectories
archive/replay-catalog: the audit recommended replaying under the recorded.infoepoch. Reversed after the OpenAI review: the archive's own timeline groups and the rewritten catalog groups would put different bytes under the same name, epoch, and group number. Replay keeps a fresh epoch,moq import archiverefuses--epoch, anymoq-archivehost never defaults to.info's epoch, the handover is aRestart, anddvr's handover test pins that. Maintainer: please confirm.stats/rustandstats/js: refuse--stats/--echoon encrypted broadcasts; Related E2EE.c/packageRequires the C++ package line (pin alreadykixelated.4).perf/announce-replayRelatedcluster-routing/routes;test-flakes-2/impaired-handshakeRelatedquic/fork/switch.perf/group-cost(latency_maxgone),perf/3122(removedruntime::Timer, symbols instead of line numbers),rs2ts/sans-io/lite(starting point istransport::poll),obs-moq-video/vpx-obsandquic/shard(no libmoq),stats/schema(fixtures inrs/hang/fixtures),stats/README(this line does not change moq-stats),c/retire(finalmoq-c0.7.x; Related libmoq-retire).m2 and m3
cut-through/{README,transport,loss-delay}: re-planned onto moq-net'stransport::poll(refactor(net)!: own the poll transport interface #4709), asquic-ack-hookwas; noweb-transport-traitrelease.catalog-tracks: defining fields are immutable;enabled, jitter/delay, andwarmupare live state.gpu-releaseRequiresgpu-health.rtsp-importrefuses--epoch, with a test.teleop/robotandteleop-mavlinkdeclareTimescale::MILLI; Relatedrust-untimed-default.native-enabledRelatedaudio-ranked;quic-bbr-loss-parityRelatedbbr-ack-cleanup(after it) and narrowed to Switch like its siblings;tls-listener-mtlsandquic-qmuxlinked both ways.video-vaapi(moq-vaapi 0.1.0) andquic-ecn(moq-noq-proto 2.0.1) versions refreshed.ladder:stallreads as disable;consumer-warmupsays max delay; line-number cites dropped.Skipped:
dropped-sourceskeeps its Auth Required: feat(net): reset revoked lite streams with UNAUTHORIZED #4179 merged into the auth line branch, notmain, so it arrives with feat(net): in-band AUTH (questline) #4039.shared-clock:a_write_follows_the_anchored_clockexists in bothbinary.rsandjson.rs, so the quest is right.ci-runner-stalls: already ready (no Required); test(interop): tell a runner stall apart from a playback freeze #4935 implements it.archivevs bounded HLS (feat(hls)!: bounded playlists: capped window, sync-point gating, explicit replay mode #4978) andhls-media-sequence(fix(moq-hls): keep EXT-X-MEDIA-SEQUENCE from rewinding #4986): fixed when those merged.announce-shapesvs lite-07: moot after the deletion.Left for in-flight PRs
transport-upgrade/{README,js}(same-epoch lite-07 only; reprice to drain after fix(net): keep opening requests on a draining session until the replacement wins #4985).stats-epochre-resolves on Restart (End then Start); Related restart.lite07-finalizeRequires Restart and In-band auth;subscribe-ranges/READMEcites feat(net)!: datagrams are unfetchable and bounded by the group range #4982 instead of quest: datagrams are live-only, capture by default; drop hls-linger #4551.prefix-route-frontsandidle-frontscross-link (idle-fronts exists only there).auth/READMEdangling mTLS scope quest should linkm2/tls-listener-mtls;wip-versiondeletion carries over; p2p hop id to node id inm3/p2p/READMEandauth/README.peer-grant/signalnode id,transport-adapter-dedupunblocked,publish-timestampnarrowing and FFIOptionwrappers,plan-watch-worker(?worklet),quest-flat-linespin,perf/demand-aggregatemax_delay,archive/flushnaming,libmoq-final-release(publish at or above 0.6.13),js-startup-hole(maxDelay),ffi-shape/codec(max_delay_us, foldffi-frame-duration-default, also refactor(ffi)!: the bindings mirror Rust's layers #4519 and docs(quest): record that a 2s GOP is enough #4943). quest: plan follow-ups from the final-head PR audit #5038 already covers several of these.open-gop-leading-picturesshould Requirerust-continuous(and stop claiming to own the signal) and drop its stale decoder-reset lines;ts-passthroughanchors via the shared-clockInputAPI;mkv-export-delayandflv-export-delayrenames.export-ts-linger"replaced" means the Restart announce.js-request-windowvsjs-session-parity; quest: plan follow-ups from the 2026-10-07 PR sweep #5041'sjs-audio-rankedRequiresaudio-ranked.cmaf-inline-paramsandfmp4-catalog-initsides of the CMAF ordering.uring-flow-control-windowsRelated fork/switch, no hard-coded noq path.rendition-preferenceorder (enabled, preference, area, bitrate).rs-request-creditandrelay-session-limitsmap both refusals to the bindings error kind.qos/publisher-timeliness(untimed since feat(net)!: carry untimed tracks faithfully #4822).cpp/READMEpin tokixelated.4.Expect conflicts with in-flight PRs:
quest/m0/README.md(#5010, #5020),broadcast-epoch/{README,restart}.md(#4960, #4904, #5010, #5038),quest/m2/README.md(#4996, the deleted announce-shapes line), andquest/m2/announce-shapes.md(#4039 edits a file this deletes).Stale claim branches
No open PR. Left for the maintainer to delete:
quest/m0/branch-flip-done(quest: branch flip applied #4743 merged)quest/m0/broadcast-epoch/apps(feat(apps): follow a broadcast's announcement as its online signal and log epochs #4970 closed; the quest is open, so takeover)quest/m0/broadcast-epoch/origin(feat(net)!: negotiate publisher epochs as metadata #4817 closed; quest gone)quest/m0/path-hunting(fix(net): replace the cursor hold with a cluster-link hold-down #4644 closed; quest gone)quest/m0/qmux-credit(docs(quest): record qmux fixes and release prerequisites #4705 closed; now waits on the condition quest)quest/m0/qmux-reset-race(no PR; quest gone)quest/m1/js-subscribe-abandonment(fix(net): preserve returning demand during subscription setup #4963 merged)quest/m1/perf/lock-wait(feat(kio): measure contended lock wait per worker #5031 closed; quest: replace lock wait with an off-CPU profile and a kio contention fix #5040 replaces the quest)quest/m1/request-linger(feat(net): linger upstream requests past their last reader #4931 closed; quest gone)quest/m1/subscribe-drop(no PR)quest/m1/test-flakes-2/mux-debounce-clock(moq-mux: run the TS SI debounce test on the paused clock #4687 merged)quest/m1/test-flakes-2/publish-audio-clock(test(publish): mock the audio delay clock #4715 merged)quest/m1/typed-timedness(feat(moq-net): mock up typed timed/untimed tracks #4878 closed; quest gone)quest/m1/ietf-cold-largest(fix(net): report a cold relay's upstream Largest #5030 closed; the quest is planned in quest: triage Fastly's moq-relay-interop report #5020)quest/promote-cleared-gates(quest: promote two quests whose gates already cleared #4588 closed),quest/roles-solo(quest: a one-node region runs its node as solo #4674 closed)(Written by Claude Opus 5.5)
🤖 Generated with Claude Code