Repository navigation
test(interop): exercise clean media finish across clients - #6307
Merged
Merged
Conversation
Quest-Claim: f6c2ec75c4873e53
Assert natural media and catalog EOF, exact announcement withdrawal, and continued session use after the finished media lingers. Preflight the reader before publication and keep terminal callbacks owned through teardown. Co-Authored-By: GPT-6 <gpt-6@openai.com>
Add finish-only hooks around the existing finite media publishers, retain media and catalog readers through natural EOF, and require exact announcement END plus a raw health read over the same session. Completion ACK releases teardown only after the actual broadcast linger. Validated all four Go/Python publisher and subscriber combinations with ten nonempty frames and 30s linger. Four wrong-count controls reject eleven expected frames. Go build/vet and Python ruff/pyright pass. Required just ci stops on the unchanged Manual.advanceAsync type error addressed by PR6308. Co-Authored-By: GPT-6 <noreply@openai.com>
Co-Authored-By: GPT-6 <noreply@openai.com>
Require an untimed health payload, natural group EOF, and then track EOF. Track.ReadFrame only reads the first frame per group, so it cannot establish the group end or reject an extra frame in that group. Go build and vet pass against the current staged bindings without rebuilding FFI. Required just ci remains blocked by the unchanged Manual.advanceAsync type error covered by PR6308. Co-Authored-By: GPT-6 <noreply@openai.com>
Match the other finish readers by refusing a timestamp on the raw session health frame. Ruff, formatting and pyright pass; required just ci still stops on the existing Manual.advanceAsync error covered by PR6308. Co-Authored-By: GPT-6 <noreply@openai.com>
Co-Authored-By: GPT-6 <noreply@openai.com>
kixelated
changed the title
Resolve the audio-tone conflicts (#6284), check GStreamer's finished late tail with its end marked N/A, and retire the quest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…by prefix moq-transport's AUTH extension carries prefixes alone, so the finish fixture and its health sibling share a parent path. Late join, withdrawal, and same-session health after a finish do not yet interoperate on moq-transport; pinned passes record the case as skipped, tracked by quest/m2/ietf-finish.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
marked this pull request as ready for review
October 22, 2026 22:51
Collaborator
Author
|
@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. |
Contributor
|
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 29 minutes. View limit details
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 |
|
Codex Review: Didn't find any major issues. What shall we delve into next? 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 file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The interop matrix proves flowing media and raw track tails, but does not exercise a finished media broadcast's late tail, natural ends, withdrawal, and continued session use.
Approach
Add
case=finishto the existing runner and finish-only modes to its existing clients. Readers connect first and wait without media demand. After the publisher finishes ten H.264 frames and the catalog and obtains its normal finish handle, the runner releases readers. They require ten nonempty frames, natural media/catalog EOF, exact-path announcement END, and a new untimed health track with complete group/track EOF through the same session. Publishers await the real linger before health and require monotonic elapsed time from before handle creation to cover its returned nonzero duration (no hardcoded duration or tolerance); the FIFO ACK releases teardown only. New finish cells require successful terminal exits and publisher ACK.GStreamer checks the late tail alone.
moqsrckeeps watching a finished broadcast, since an announcement means online/offline, so it has no end, withdrawal, or session to report. Its reader joins once the broadcast has finished, requires exactly ten nonemptyvideo_0buffers, and stops on an interrupt; the cell passes with the end check marked N/A. Nomoqsrclifecycle API is added, since only this test would consume it.The fixture and its
.healthsibling share a parent path so one prefix grant covers both, which moq-transport's AUTH extension requires. Pinned moq-transport passes record the finish case asskip: late join, withdrawal, and same-session health after a finish do not yet interoperate there (see Follow-ups).Rust uses a private example over existing media/model APIs. Browser finish uses finite WebCodecs output and the existing catalog/container APIs; native JS shares its drain assertions. Normal tone, probe, contract, and playback behavior is retained. Finishes
quest/m1/interop-finish.md.Impact
interop.yml's full-matrix step (nightly, and on release PRs that touch the harness). The merge gate is unchanged.Validation
just test interop --all: 169/169 pass, including all 45 finish cells (the five GStreamer cells pass with the end marked N/A).just test interop --finish --all --versions auto,moq-transport-18,moq-transport-20,moq-transport-22after the prefix grant: lite 45/45; moq-transport fails on every draft, which is why those passes skip (--versions moq-transport-14then recordsskip, verified, and the report renders it).just fix, shellcheck, andquest check --base origin/mainpass.just checkpasses JS, Rust, and quest; its flake check cannot write/proc/self/uid_mapon this host (unshare: Operation not permitted).Alternatives
Preconnect readers instead of increasing retention or adding startup sleeps. Reuse a private Rust fixture instead of adding production CLI flags. Cancellation, transport close, synthetic EOS, or a second health connection cannot prove clean finish. A finite-watch
moqsrcmode would have no consumer besides this test. Leaving the moq-transport cells red would fail every nightly on known gaps.Follow-ups
(Written by Claude Opus 5.5)
🤖 Generated with Claude Code