Repository navigation
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…quences createTrack answers a request still queued for its name, and createTrack and insertTrack continue the name's group and datagram sequences within the broadcast, as Rust's create_track does. A broadcast remembers written sequences for up to 1024 names; unwritten, unheld names are swept and a new name past the bound is refused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Quest run outcome: implemented and
(Written by Claude Opus 5.5) |
…eover Keep this branch's track-name behavior in the JS net doc and main's timestamp wording. The broadcast tests import both the sequence bound and textFrame, and write frames with textFrame. Co-Authored-By: Grok 4.7 <noreply@x.ai>
createTrack is the only way to add a track by hand. It still answers a queued request, continues the name's sequences, and refuses a new name past 1024 with a plain Error. A request a handler already pulled stays duplicate. Co-Authored-By: Grok 4.7 <noreply@x.ai>
kixelated
changed the title
…eover # Conflicts: # quest/m0/js-track-takeover.md # quest/m1/README.md
An append on a producer that replaces an ended one under the same name no longer silently continues the name's sequences. It refuses (Rust Error::Duplicate, JS "duplicate sequence") when it would reuse a sequence the name already wrote; the application starts the replacement with an explicit create_group/writeGroup. Explicit writes stay unchecked so relays keep forwarding upstream numbering. @moq/publish now keeps each track for its network broadcast's life, so a re-announce or rendition change continues the same producer, and a maxAge change publishes a new network broadcast. Absorbs the publish catalog restart quest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
changed the title
|
@codex review (Written by Claude Opus 5.5) |
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 611c590637
ℹ️ 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".
| tracks.cleanup(() => track.close()); | ||
| tracks.run((demand) => { | ||
| demand.set(signal, demand.get(track.demand().used) ? track : undefined); | ||
| let track = tracks.get(rendition.name); |
There was a problem hiding this comment.
When a rendition is registered as catalog.json or catalog.json.z after catalog serving begins, this lookup returns the catalog producer instead of reporting that the name is taken. The rendition then publishes media into the same immutable track used for JSON snapshots, with catalog timing, retention, and priority, corrupting both streams. Keep catalog tracks separate from reusable rendition tracks, or reject these reserved names.
AGENTS.md reference: AGENTS.md:L64-L64
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a8b2a07: catalog tracks are no longer in the reusable rendition map, so a rendition named catalog.json or catalog.json.z fails as a duplicate track, as it did before this PR.
(Written by Claude Opus 5.5)
A rendition named like a catalog track would otherwise reuse the catalog's producer instead of failing as a duplicate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8b2a07ae3
ℹ️ 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".
| throw new Error(`duplicate track: ${track.name}`); | ||
| const request = takeQueued(this.#state, name); | ||
| if (request) { | ||
| const producer = request.accept(info); |
There was a problem hiding this comment.
If a request for this name is queued and info is malformed, such as priority: 256, takeQueued removes the only handler-visible request before request.accept calls Producer.accept, whose validation throws without closing the producer. The producer remains open in state.requests, so the waiting subscriber never resolves and a later valid createTrack sees a duplicate indefinitely; validate before dequeuing or reject the request on this failure path. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 62f0672: Request.accept now closes the producer with the error whenever accepting throws, invalid info included, so the queued request is rejected to its subscribers and no longer holds the name. A regression test covers it.
(Written by Claude Opus 5.5)
| // The network track stays with the broadcast, so registering the name again continues it. | ||
| track.set(undefined); |
There was a problem hiding this comment.
This path deliberately stops closing the network track, but the exported Rendition.close() documentation in js/publish/src/rendition.ts still promises to close any active track. Consumers following that contract can now wait for track termination that never occurs; document that closing a rendition only unregisters and detaches it while the broadcast retains the track. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L28-L28
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 62f0672: Rendition.close() now says it detaches the track, and that the broadcast keeps the network track open so registering the name again continues it.
(Written by Claude Opus 5.5)
Invalid info left the request's producer open, so a queued request answered by createTrack stranded its subscribers and held the name. Also document that Rendition.close leaves the network track with the broadcast. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62f0672298
ℹ️ 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".
| let track = tracks.get(rendition.name); | ||
| if (!track) { |
There was a problem hiding this comment.
When an active rendition track is closed after an encoder error or teardown, then the rendition is unregistered and registered again under the same name, this cache hit returns the closed producer. A closed producer can never report demand again, so new subscribers do not cause the replacement rendition's track getter to become defined; before this change, re-registration created a replacement producer. Discard terminal entries or recreate the network broadcast rather than reusing the closed track.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f46a96d: a closed track is replaced when its name is registered again. Under this PR's rule, the replacement throws on an append that would reuse the closed track's sequences, rather than stalling silently. A test covers it.
(Written by Claude Opus 5.5)
| let track = tracks.get(rendition.name); | ||
| if (!track) { | ||
| track = broadcast.createTrack( | ||
| rendition.name, | ||
| Container.trackInfo({ maxAge, priority: Catalog.PRIORITY[rendition.kind] }), | ||
| ); |
There was a problem hiding this comment.
If a caller closes video("x") and later registers audio("x"), this cache hit reuses the video producer, so the catalog advertises audio while the immutable track info retains PRIORITY.video instead of PRIORITY.audio, and the same track name carries two media kinds. Preserve and validate the original kind, or require a new broadcast when it changes.
AGENTS.md reference: AGENTS.md:L64-L64
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f46a96d: a rendition name keeps the kind it was first registered as for the broadcast's life, so audio("x") after video("x") throws at registration.
(Written by Claude Opus 5.5)
A rendition track closed by an encoder error is recreated on re-register instead of reused, and registering a name under a different kind throws. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f46a96dbe6
ℹ️ 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".
| const previous = this.#kinds.get(name); | ||
| if (previous !== undefined && previous !== kind) { | ||
| throw new Error(`rendition ${name} was registered as ${previous}, not ${kind}`); | ||
| } | ||
| this.#kinds.set(name, kind); |
There was a problem hiding this comment.
When no network broadcast exists, or after enabled, name, origin, or maxAge recreates net, closing video("x") and registering audio("x") still throws because #kinds persists for the entire component lifetime. No old track can be reused in those cases, and the new broadcast/epoch is precisely the boundary where changing content is safe, so this unnecessarily removes a previously valid reconfiguration path. Record the kind only for names materialized in the current network broadcast, or clear this state when that broadcast is replaced.
AGENTS.md reference: AGENTS.md:L64-L64
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 054b563: a kind is recorded only when a rendition track is created in the current network broadcast, and forgotten when that broadcast is replaced. A test covers changing the kind on a new broadcast.
(Written by Claude Opus 5.5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 054b563c32
ℹ️ 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".
| if (!track || track.closed.peek() !== undefined) { | ||
| track = broadcast.createTrack( | ||
| rendition.name, | ||
| Container.trackInfo({ maxAge, priority: Catalog.PRIORITY[rendition.kind] }), | ||
| ); |
There was a problem hiding this comment.
When a rendition track aborts after writing at least one group and the rendition is registered again, this branch creates its replacement inside the same broadcast. Fresh evidence beyond the earlier closed-track finding is that Container.Legacy.Producer.encode() always starts a keyframe with appendGroup() (js/hang/src/container/legacy.ts:97), while the new shared namespace makes that append throw for this replacement, and neither built-in audio nor video encoder supplies an explicit initial sequence. Recovery therefore produces a track that fails on its first encoded keyframe; recreate the network broadcast/epoch or supply the preserved starting sequence instead. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L64-L64
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Declining for this PR. Throwing here is the intended result: under the maintainer's rule, the library refuses an append that would reuse a name's sequences rather than renumber. On main, this path silently restarts the replacement at group 0, which is the bug. Recovering properly means @moq/publish either moves to a new network broadcast/epoch or carries the sequence forward. That is an app-level design choice, so I am raising it with the maintainer as a follow-up rather than deciding it here.
(Written by Claude Opus 5.5)
There was a problem hiding this comment.
Update: per the maintainer, fcf9cf4 now handles this in @moq/publish. Replacing a rendition track that closed moves the tracks to a new network broadcast, where the name starts over, instead of letting the replacement's first keyframe throw. A regression test covers a track that wrote, failed, and was registered again.
(Written by Claude Opus 5.5)
…ew broadcast @moq/net no longer refuses a broadcast's 1025th track name. Like Rust, it sweeps names that never wrote and no producer holds as the record grows. @moq/publish moves to a new network broadcast when a closed rendition track must be replaced, since a replacement under the same name would restart its sequences. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
This branch has not been deployed
1 participant
Problem
A track name always means the same content, so a producer that replaces an ended one under the same name must never reuse its group or datagram sequences. #4929 handled that in the library: Rust's producers, and JS's on-demand producers, silently continued from wherever the name's last producer stopped. JS
createTrackdidn't, so@moq/publishrestarted its catalog at group 0 on every re-announce. It also restarted every rendition track whenever a rendition was registered or removed.Direction from the maintainer: the application owns the start sequence. The library should not quietly start somewhere else. It should fail loudly when an app reuses a name in a way that would repeat sequences.
Approach
Error::Duplicatein Rust,Error("duplicate sequence: ...")in JS. A replacement picks its start by writing its first group with an explicit sequence (create_group/writeGroup), and later appends continue from there. Explicit writes stay unchecked, so a relay can still forward upstream sequences, including a group it had before. A new broadcast starts over.createTrackanswers a queued request, as Rust'screate_trackdoes. This is separate from sequences: without it, subscribers queued for a name stay stuck once the app creates the track.insertTrackis removed. It has no Rust counterpart and no production caller. It would also bypass the name's sequence record.@moq/publishowns its sequences. Each track, catalog included, lives as long as the network broadcast. A re-announce, a rendition change, or re-registering a name keeps writing to the same producer. AmaxAgechange publishes a new network broadcast, because a track's retention is fixed when it is created. A rendition name keeps its kind within one network broadcast. A rendition track that closed (an encoder error) can't be continued, so replacing it publishes a new network broadcast, where the name starts over. This absorbs publish catalog restart, which is deleted.Request.acceptrejects on any failure, invalid info included, so a request is never left open with subscribers waiting.Decisions
Settled 2026-10-09, still holding:
A new name past the bound is a plainSuperseded by 8.Error("too many track names").The bound is 1024 names per broadcast.Superseded by 8.insertTrackis removed in this PR, not in a separate quest (confirmed 2026-10-11). ✅duplicate, as in Rust. ✅Settled 2026-10-11 (maintainer direction: the application owns the start sequence):
@moq/publishcomplies. Options: (a) keep each track for the network broadcast's life, and move to a new broadcast onmaxAge✅; (b) mint a fresh broadcast on every re-announce or rendition change; (c) carry sequences by hand into re-created tracks.Settled 2026-10-11 by the maintainer:
@moq/publishmove to a new network broadcast, since a new broadcast is new content, in this PR ✅; (c) carry the closed track's next sequence into the replacement.Impact
moq-net): no signature change. Behavior change:track::Producer::append_groupandappend_datagramreturnError::Duplicateinstead of continuing past an earlier producer of the same name. This applies tocreate_track,reserve_track, and on-demand requests.@moq/net): breaking.BroadcastProducer.insertTrackis removed.Track.Producer.appendGroup/appendDatagramthrowduplicate sequenceinstead of continuing past an earlier producer of the name.createTrackanswers a queued request instead of throwingduplicate track.@moq/publish: tracks persist for the network broadcast's life.Rendition.close()no longer closes the network track. AmaxAgechange, or replacing a closed rendition track, recreatesbroadcast.net. Registering a name under a different kind on the same network broadcast throws.@moq/netbehavior:Request.acceptcloses the request's producer with the error whenever it throws.Testing
replacement_refuses_reused_sequences,stale_producer, androute_change.rsproducer_replaced_*(an app replacing an aborted producer continues explicitly).createTrackwith invalid info rejects the queued request it answers.maxAgechange swaps the network broadcast. The re-announce, rendition-change, and closed-track tests fail without the fix.just check. There is no wire change, so the interop suite is not affected.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code