Skip to content

fix(studio): catalogs finish when the studio quits - #95

Merged
kixelated merged 3 commits into
mainfrom
quest/m0/catalog-finish
Nov 7, 2026
Merged

kixelated merged 3 commits into
mainfrom
quest/m0/catalog-finish

Conversation

@kixelated

@kixelated kixelated commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Claims and closes quest/m0/catalog-finish.md.

Problem

On quit, catalogs were dropped instead of finished. A reproduction on nixos (lavapipe, RUST_LOG=warn, the two quit tests below extended to read each catalog to its end) showed what a reader sees today:

  • Program catalog, through the relay: the viewer's catalog subscription is cut with Moq(Session(Cancel)). The live catalog track also kept the relay session from draining: the relay session closed without draining err=timeout.
  • Recording catalog, from the local broadcast: it never ends (Elapsed after 30 s).

A backtrace on each track::Producer dropped without finish() or abort() warning found two more sources of it. Neither is the program's or the recording's catalog, and both fire in the app:

  • A source's own catalog (moq_scene::publish::Listing): dropped whenever a source stops, which every source does on quit. This is most likely the "catalog.json and catalog, twice" seen on quit (two sources). In the tests the engine is never dropped, so the program's and recording's catalogs never logged the warning there.
  • A guest's return-feed catalog (feed::Running): dropped whenever a feed ends, when a guest leaves or on quit.

Approach

Each catalog now finishes once the tracks it lists have ended:

  • Engine::stop finishes the program's catalog and the recording's (Program::finish). This runs after the program clock and audio threads are joined, which have already ended every video and audio track. Then Engine::close drains the relay session, so the relay receives the end of the catalog.
  • Engine::close finishes the sources' local catalog (new moq_scene::publish::Publish::finish) once every source thread has ended and left it.
  • A source's Listing finishes its own catalog when dropped, after the source's tracks have ended.
  • A guest's feed finishes its catalog after its audio, or aborts it with the audio on failure. Once the program catalog finishes, the feed's list ends too, so a feed now ends when the program does.

Tests, reusing #91's served and ended:

  • the_program_reaches_a_relay reads the program's audio and its catalog, in both Hang and MSF formats, to a clean end through the relay after quitting.
  • the_program_records_without_a_relay reads the recording's catalog and the sources' local catalog to a clean end after quitting while recording.
  • feed::each_guest_hears_the_program_without_themselves reads a dropped guest's feed catalog to a clean end. Before this change it failed with Moq(Dropped).

Impact

  • Viewers on a relay, and recordings, see the program's catalog end cleanly on quit instead of a cut.
  • Quitting no longer waits out the relay drain timeout.
  • Quitting no longer logs track::Producer dropped without finish() or abort().
  • Public API: moq_scene::publish::Publish::finish is new. No wire change.

Alternatives

  • Finish in Program::drop, as the quest's Plan suggested. Rejected: the engine is dropped only after close has already closed the relay session, so a relay viewer would still get a cut. In the tests the engine is never dropped at all.
  • Finish the sources' local catalog in stop with the others. Rejected: sources are still stopping at that point, and each removes its entry as it goes. Finishing in close, after their threads end, keeps the last catalog accurate.

Validation

On nixos2, at 66d36a4, and just ci again at 30106be after merging main:

  • nix develop --command quest gate just ci: passes (281 tests) at both heads.
  • The nightly engine set (just render-sw's second command): 6 of 6 pass.
  • Repro: before the fix, both quit tests failed as described above and logged warnings. After the fix, both pass, and the feed test and both quit tests log no dropped without finish() warning.
  • Not run: macOS, and the CEF and PipeWire sets, which this change does not touch.

Follow-ups

  • Upstream, moq-dev/moq (moq-net): in 2 of about 12 runs of the_program_reaches_a_relay, the same warning appeared once more at teardown, for catalog.json in one run and 1.avc3 in the other. The backtrace points to the subscriber side of a lite session: lite::subscriber::SourceServe drops a track::Requests when its session ends, with neither finish nor abort. That is the relay's ingest of the studio's broadcast in this in-process test, not the studio's publishing. A session ending should abort its subscriber-side tracks with the session's error. I found no matching moq issue or open PR. Worth a moq quest via /quest-plan.

Review

Codex (quest review), gpt-6.1-sol, medium effort, session 01a1b6b7:

  • Round 1 on 66d36a463032a1157e21bf05af6a66e3668152b2 against origin/main at 73c2c7b: no findings.
  • Head 30106be0b5dbaa8b1ed06d13f5289505a5e8fcd3 only adds a conflict-free merge of origin/main (73c2c7b), so round 1 still covers it.

(Written by Claude Opus 5.5)

Quest-Claim: 05a842c4b0abf435
@pullfrog

pullfrog Bot commented Nov 7, 2026

Copy link
Copy Markdown

Pullfrog  | Review this ➔

Quitting finishes the program's catalog and the recording's once their
tracks end, then the sources' local catalog once the sources stop. A
source's own catalog and a guest's return feed catalog finish with them.
Viewers on the relay and recordings read each catalog to a clean end, the
relay session drains instead of timing out, and quitting logs no
`track::Producer dropped without finish() or abort()` warning.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title Catalogs finish when the studio quits fix(studio): catalogs finish when the studio quits Nov 7, 2026
@coderabbitai

coderabbitai Bot commented Nov 7, 2026

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Publishing, studio shutdown, and feed lifecycle code now finishes catalogs at defined completion points. Feed errors abort the catalog. Tests check catalog completion after studio shutdown and after a guest is no longer renewed. The quest documentation for the catalog-finishing work was removed.

Merge Risk: Minimal · up to 30106

The changes finish catalogs during studio and feed shutdown and add completion checks. No concrete merge-blocking issue is established; merge remains subject to normal validation.

Pre-merge checks: Passed 5

Check name Status Explanation
Title check Passed The title clearly describes the catalog shutdown change.
Description check Passed The description explains the catalog changes and related tests.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 10 functions across 3 files.

Finishing Touches

Simplify code

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

Clippy (1.99.1)

Clippy execution failed

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.

Share

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 719f30ba-91bc-4531-8462-fec670ab98e2
Commits

Reviewing files that changed from the base of the PR and between 73c2c7b and 30106be.

Files selected for processing: 5

  • quest/m0/README.md
  • quest/m0/catalog-finish.md
  • rs/moq-scene/src/publish.rs
  • rs/moq-studio/src/engine.rs
  • rs/moq-studio/src/feed.rs
Files with no reviewable changes: 1

  • quest/m0/catalog-finish.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

@kixelated kixelated left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed head 30106be0b5dbaa8b1ed06d13f5289505a5e8fcd3 against base 73c2c7b16e00e2b3d68ff4934561d3e988729210.

No actionable findings. The program and recording catalogs now finish after their media workers join and before relay draining. The local sources' catalog finishes after source workers leave it. Finishing a cloned catalog handle closes the shared state in the pinned moq implementation; source listings and guest feeds also explicitly finish their catalogs. The extended tests check catalog termination through the relay in both formats and on the local recording/source paths.

Direction: keep the shutdown fix at these existing lifecycle boundaries. No changes requested.

Verification limits: GitHub-only static review of all changed files, shutdown callers, source/feed lifecycles, recorder behavior, and relevant pinned moq APIs. I read the pinned quest context as source but did not execute quest context, builds, tests, or the reported reproduction. The reported test results and upstream subscriber teardown warning were not independently reproduced. COMMENT only, not approval.

(Written by review (OpenAI))

@kixelated

Copy link
Copy Markdown
Contributor Author

Pick (/quest-complete, maintainer away under /afk, recommended option): ✅ Merge, keeping the choice to finish in Engine::stop and Engine::close rather than Program::drop, since the engine drops only after the relay session has closed. Check passed and the OpenAI review found nothing. The moq-net teardown warning went into moq-dev/moq#8152's update to quest/m3/interop-shutdown-warnings.md, which already covers it.

(Written by Claude Opus 5.5)

@kixelated
kixelated merged commit a8ca56f into main Nov 7, 2026
4 checks passed
@kixelated
kixelated deleted the quest/m0/catalog-finish branch November 7, 2026 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant