fix(js/net): key the publisher's TRACK_INFO cache by broadcast, not path - #3422
Conversation
Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ity (#3377) Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…and ACME quests (#3402) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…ges (#3405) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
… lost picture (#3357) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Luke Curley <kixelated@gmail.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
… (#3407) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…a sidecars, and clock sync (#3416) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Claim UUID: cc3f0536-0789-44ea-8321-fb8e0f4d734e
The lite publisher memoized each track's immutable TRACK_INFO in a Map keyed by `broadcast\0track`. The key is a path, and a path outlives the broadcast on it: `Origin.publish` swaps the front under an existing key, so a republish left the entry in place and every later TRACK request was answered with the predecessor's priority, maxAge, and timescale. FETCH reads the same cache to pick the timescale it converts frame timestamps into, so the successor's frames were quantized onto the predecessor's grid: a 1234us timestamp on a MICRO track came back as 1000us once a MILLI predecessor had primed the cache. SUBSCRIBE resolves `track.info()` fresh, so the two disagreed about the same track on the same connection. Key the cache on the routing front instead. Immutability holds for one broadcast, which is exactly the lifetime a WeakMap keyed by the front gives it, and it is the same identity the announce loop already diffs on. `runFetch` now passes the front it already resolved, so the metadata and the frames come off one generation rather than two independent map lookups that a replacement can land between. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DLtoPv9B3779kGxLFTUGHR
The Track Stream section told a subscriber to cache TRACK_INFO keyed by broadcast path and track name for the life of the session, which is the exact shape that goes stale when a broadcast is replaced on that path. Say what the immutability is actually scoped to. The wire format is unchanged; this only qualifies existing caching advice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DLtoPv9B3779kGxLFTUGHR
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
#3415) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…car placement (#3427) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…3355) Co-authored-by: Luke Curley <kixelated@gmail.com> Co-authored-by: Codex <codex@openai.com>
# Conflicts: # CLAUDE.md # cpp/obs/buildspec.json # quest/m0/README.md # quest/m2/2278-watch-absolute-wall-clock-latency-target-for-synchronized.md # quest/m2/relay-memory/README.md # rs/moq-audio/src/encode/capture.rs # rs/moq-mux/src/container/ts/export.rs # rs/moq-mux/src/container/ts/export_test.rs # rs/moq-net/src/model/broadcast.rs # rs/moq-net/src/model/track.rs # rs/moq-video/src/decode/consumer.rs # test/ts/README.md # test/ts/run.sh
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Luke Curley <kixelated@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…er quest (#3430) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: GPT-5.6 Sol <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d44e19cd27
ℹ️ 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".
| The publisher replies with a single TRACK_INFO message and then FINs the stream, or resets the stream on error (e.g. the track does not exist). | ||
| The returned properties are fixed for the lifetime of the track, so the subscriber SHOULD cache TRACK_INFO keyed by broadcast path and track name, and reuse it across every SUBSCRIBE and FETCH of the same track over the session that served it. | ||
| The properties are fixed for one track, not for the path it arrived on: a path outlives the broadcast on it, and a different broadcast reaching the same path brings its own. | ||
| Anything cached against a path, on either side, is therefore scoped to the announcement that carried it and is discarded when that announcement is retracted. |
There was a problem hiding this comment.
Decouple cache invalidation from route retraction
When a persistent prefix route remains advertised while the application replaces a broadcast beneath it, no ANNOUNCE_END occurs: the Announce section defines routes as capability rather than inventory. A subscriber following this new rule therefore retains the predecessor's TRACK_INFO and can decode the successor's timestamps and retention metadata incorrectly. Define a generation signal or constrain cache validity independently of announcement lifetime instead. (Written by GPT-5.6 Sol)
Useful? React with 👍 / 👎.
| The subscriber sends a TRACK message containing the broadcast path and track name. | ||
| The publisher replies with a single TRACK_INFO message and then FINs the stream, or resets the stream on error (e.g. the track does not exist). | ||
| The returned properties are fixed for the lifetime of the track, so the subscriber SHOULD cache TRACK_INFO keyed by broadcast path and track name, and reuse it across every SUBSCRIBE and FETCH of the same track over the session that served it. | ||
| The properties are fixed for one track, not for the path it arrived on: a path outlives the broadcast on it, and a different broadcast reaching the same path brings its own. |
There was a problem hiding this comment.
Record the cache-lifetime change in the changelog
This adds semantic guidance governing when TRACK_INFO caches must be discarded, but the moq-lite-06 changelog has no corresponding entry, so revision readers cannot discover the behavior change. Add a concise factual bullet to the in-progress changelog section. (Written by GPT-5.6 Sol)
AGENTS.md reference: drafts/AGENTS.md:L77-L81
Useful? React with 👍 / 👎.
Adopts main's reorganized documentation site (#3426) and reconciles it with dev's API and CLI surface: moq-native -> moq-tokio, `--client-connect` -> `--connect`, `--latency-max` -> `--max-age`, `[server]`/`[client]` -> `[listen]`/`[connect]`, origin id -> Hop ID, the context-taking Go calls, and the origin-based JS and FFI publish APIs. Folds dev-only functionality into the new pages rather than the deleted ones: hang text and data tracks, the LAN cluster mesh, the `[runtime]` workers and io_uring counters, the wall-clock cache sweep, routes and max age on the moq-lite page, and the broadcast-level timeline the HLS gateway reads. Repoints the quest links main added at documents dev had already retired: uring-metrics (#3408), 2985 (#3422), and perf/session-micro (#3306). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Completes quest/m1/2985 and deletes the quest file.
Closes #2985
Summary
Root cause. The moq-lite publisher memoized each track's immutable
TRACK_INFOin aMapkeyed by`${broadcastPath}\0${trackName}`. That key is a path, and a path outlives the broadcast on it:Origin.publishswaps the front under an existing key, so a republish left the entry in place and every later TRACK request was answered with the predecessor'spriority,maxAge, andtimescale.runFetchreads the same cache to pick the timescale it converts frame timestamps into, so the successor's frames were quantized onto the predecessor's grid.runSubscriberesolvestrack.info()fresh, so the two paths disagreed about the same track on the same connection.Fix. Key the cache on the routing front (
broadcast.Consumeridentity) instead of the path, in aWeakMapso an entry's lifetime is exactly the broadcast's. That is the same identityrunAnnouncealready diffs on to emit ended-then-active for a republish, so discovery and metadata now agree on what a generation is.#resolveTrackInfotakes the front rather than re-looking it up, which also closes a smaller window inrunFetch: it resolved the front, then#resolveTrackInforesolved it again, so a replacement landing between the two lookups could pair one generation's metadata with the other's frames.This is the third of the three sites #2985 lists. The other two are already fixed on
dev:runAnnounceholdsMap<Path.Valid, broadcast.Consumer>and diffs on the front, andietf/publisher.tsrecords the front each refusal was about inoffered. The issue's suggested#2610epoch remedy is gone (#3225 removedEpochfrom the moq-lite draft as spec-only), so this is the local fix instead: no wire-level identity needed.Branch
Targets
devbecause the m1 questline is dev-based and the fix is written againstdev's origin/front model (broadcast.Consumer.resolveTrackInfo, the front-keyed announce diff).maincarries the same path-keyed cache and picks this up on the nextdevmerge.The same defect exists in rs/moq-net, and is NOT fixed here
Verified against a running
moq-relay(JS publisher -> relay -> JS subscriber). After the fix the JS publisher serves the successor's timescale on the wire, but the subscriber'strack("video").info()through the relay still returns the predecessor's{priority: 1, timescale: MILLI, maxAge: 1000}.The relay is not doing the path-keyed thing; it fails a different way in the same family.
resume::Consumer::poll_infoinrs/moq-net/src/model/resume.rsresolves info from segment zero and never from the newest, while every data read (fetch_group, subscribe) routes to the newest segment.origin.rs'srun_frontStep::Splicearm does re-resolvesource.track(&name).info()for the successor beforeresume.takeover(&track), so the relay learns the successor's info and then discards it. Reached viaTrackInfoServeinrs/moq-net/src/lite/publisher.rs->broadcast::Consumer::track(spliced) ->track::Querying::poll_ok->resume.poll_info. The relay's upstream side is correct.There is no open tracker for it: #2991 is about sequence continuity across replacements and says nothing about
track::Info, and #2610 (which spec'd the epoch answer) is closed as completed with no epoch inrs/. It wants its own issue againstresume::Consumer::poll_info, which I have not filed.Wire behavior changes
No encoding change. What a peer observes differently: after a broadcast is replaced at a path, a TRACK request now returns the successor's
TRACK_INFOrather than the predecessor's, and FETCH serves frames on the successor's timescale. Previously the publisher served the first generation's metadata for the rest of the session.The
drafts/draft-lcurley-moq-lite.mdcommit only qualifies existing caching advice: the Track Stream section told a subscriber to cacheTRACK_INFOkeyed by broadcast path and track name for the life of the session, which is the shape that goes stale here. It adds no normative requirement and no format change. Drop that commit if you would rather the spec text move separately.Cross-Package Sync
js/net<-rs/moq-net: this is the js side of a defect that exists on both. The Rust side is described above and deliberately not fixed here.drafts/: the wire format is unchanged, so no draft update was required; the one included is a clarification, not a format change.js/{watch,publish},demo/web: no API change, nothing to pair.Test plan
js/net/src/integration.test.ts, both failing ondevand passing with the fix:track info follows a republished broadcast:prioritycame back1instead of7.fetch uses the republished track's timescale: a1234ustimestamp came back as1000us, quantized onto the predecessor's millisecond grid.just fix,just check,just test: all pass. Rust and Python were skipped by the diff scoping (no crates affected), which is expected for a js + drafts diff.just drafts check: passes.moq-relayover the WebSocket/qmux transport, replacing a broadcast on a live path. With the fix reverted the fetched frame arrives as1000us; with the fix it arrives as1234000us, i.e. the publisher wrote1234ticks on the successor's MICRO grid and only the relay's own stale metadata rescaled them. That is what isolated the remaining staleness tors/moq-net.Not run
just test smoke-full: no wire,moq-ffi, or gateway change.just dev: nowatch/publish/UI surface changed.(Written by claude-opus-5[1m])
🤖 Generated with Claude Code
https://claude.ai/code/session_01DLtoPv9B3779kGxLFTUGHR