fix(api): resolve the programme the request names, in sixteen more places - #598
Merged
Conversation
…aces Closes #527 #528 #539 #540 #541 #542 #543 #544 #545 #546 #547 #548 #574 #576 #578 #579. Records #575 as not real. s.eng() is mgr.Default() is Engines()[0] -- correct only on a single-source install, which is every development box and every CI runner. That is why five instances of this shipped before anyone saw one. #527 was the sharpest: handleCreateDestination validated the programme with requireNamedSource and then threw the answer away, compiling the "would carry no audio" refusal against the DEFAULT programme's track layout. Its three sibling routes all resolve through sourceForDestination. So a Studio B destination was either stored while streaming silence -- the guard's own text calls silence "the one failure this product exists to prevent" -- or refused 400 naming a feed it had not read. #528 finished what an earlier fix started: destination series were scoped, while ingest_up, ingest_bitrate, ingest_restarts_total and every relay_* stayed unlabelled scalars off the default engine. A missing Prometheus series is indistinguishable from a destination nobody configured, so the alert a dead programme should raise never evaluates. THE REGISTER HAD STOPPED REGISTERING. Its AST scan matched the selector `eng` only, so engOrNil() and bare mgr.Default() were invisible to it, and it scanned internal/api alone -- which is how the scheduler instance landed outside it entirely. It now matches all three spellings, carries a sentinel per spelling so a silent scan cannot pass, and walks the whole repository refusing any Manager.Default() outside internal/api. Four unrecorded reaches surfaced immediately. NOT REAL -- #575. Its consequence, "programme 1 has no engine so s.eng() is nil while programme 2 runs", does not follow: Default() is Engines()[0] over engines that actually STARTED, so it answers programme 2. The reviewer read it as the first source ROW, which is db.DefaultSourceID() -- and defaultSourceID exists precisely because those two differ. internal/metrics/metrics.go is edited here despite being outside the lane's assignment: #528's defect IS that Snapshot and Render have nowhere to put a source label, so it cannot be fixed anywhere else. Claude-Session: https://claude.ai/code/session_01A8N3W5ct9SZtHK9sCDD9cL
Closes #577. recordings.source_id has been on the table since sources existed and db.Recording never read it, so every consumer had to guess -- and clipTracks guessed the DEFAULT programme for both halves of a clip's track labelling: the count came from engOrNil().SourceKnown(), and the names from settings.Ingest.Annotations, which handlePutAnnotations writes for the default programme only. The issue rated itself LOW on the grounds that "clips from another programme cannot currently exist" -- which was true because handleCaptureClip captured from the default programme's rolling buffer. Fixing that in this same branch is what makes this live: clips from other programmes now exist, and without this they would be labelled with Main's track names. An operator reading "Presenter mic" over somebody else's channel is a worse answer than no label at all. SourceID is a pointer and the column is nullable, because a row written before sources existed has no answer and inventing one is the bug this exists to end. For those rows the default IS the honest answer: there was only ever one programme to have recorded them. Claude-Session: https://claude.ai/code/session_01A8N3W5ct9SZtHK9sCDD9cL
Closes #550. The playout settings are a SINGLETON -- one intent for the box -- while every engine runs its own playout manager. playoutManager resolved that asymmetry by taking engOrNil(), so on a two-programme install the page shared with an AUDIENCE served programme 1 whatever the operator was broadcasting. It is the one surface here with viewers on it. It cannot be fixed the way the authenticated routes were. A viewer has no session, and a public URL has nowhere to carry a ?source= -- so the answer cannot come from the request. It has to be stated, and PlayoutSettings.SourceID is where it is stated. Nil means the default programme: every single-source install, and the behaviour this has always had. Nothing changes for them. What changes is that on the installs where it was a coin toss it is now an answer somebody gave. A named programme that is not running falls back rather than 404ing. An audience staring at a dead page because a source was renamed is worse than one watching the install's other feed, and the console shows which is on air. Three of this repo's registries refused the field until it was accounted for: the stored-leaf classifier (is it a credential -- no, a source id), the reload rules (ClassOnDemand: resolved per request, no child restarts), and the route ledger. That is three separate devices catching one three-line struct change, which is the standard this codebase holds itself to. NOTE: TestAHaltedRecorderOnAnyProgrammeIsReported failed once during this work and passed on re-run and in isolation. Not root-caused, and recorded here rather than reported as clean. Claude-Session: https://claude.ai/code/session_01A8N3W5ct9SZtHK9sCDD9cL
|
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
1 participant
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.
DRAFT — do not merge until the UI companion lands. See "What this breaks" below.
Closes #527 #528 #539 #540 #541 #542 #543 #544 #545 #546 #547 #548 #574 #576 #578 #579. Records #575 as not real.
What this breaks, and why it is still the right change
Nine routes now answer
400 source_requiredwhen two or more programmes exist and the request names none.ui/src/lib/api.tssends?source=on five endpoints and none of them are these — in factsourceQueryexists but no caller supplies an id at all. So on a multi-source install the console would go dark:/status,/source,/levelsand/wsare polled on every tick.Single-source installs are unaffected, which is exactly why the Go suite passed unchanged — and is the same blindness that let five instances of this bug ship.
I tried softening the three UI-polled reads to a labelled default (the trade
handleSystemalready makes) and reverted it. It weakens a correct design to work around a missing feature, and/wsis in the same set, so the softening would have had to spread until it meant nothing. The gap is that the UI has no concept of a current programme. That is #516, and it is what I am building next.The findings
#527 —
handleCreateDestinationvalidated the programme withrequireNamedSourcethen threw the answer away, compiling the "would carry no audio" refusal against the default programme's tracks. Its three siblings all usesourceForDestination. A Studio B destination was either stored while streaming silence — which the guard's own text calls "the one failure this product exists to prevent" — or refused 400 naming a feed it never read.#528 — finishes what an earlier fix started. Destination series were scoped;
ingest_up,ingest_bitrate,ingest_restarts_totaland everyrelay_*stayed unlabelled scalars. A missing series is indistinguishable from a destination nobody configured, so the alert a dead programme should raise never evaluates.#539 — the register had stopped registering. Its AST scan matched the selector
engonly, soengOrNil()and baremgr.Default()were invisible, and it scannedinternal/apialone — which is how the scheduler instance landed outside it entirely. Now matches all three spellings, carries a sentinel per spelling so a silent scan cannot pass, and walks the whole repo refusingManager.Default()outsideinternal/api. Four unrecorded reaches surfaced at once.Not real — #575
Its stated consequence doesn't follow:
Default()isEngines()[0]over engines that actually started, so it answers programme 2. The reviewer read it as the first source row — which isdb.DefaultSourceID(), anddefaultSourceIDexists precisely because those differ.Out of assignment, deliberately
internal/metrics/metrics.gois edited here: #528's defect is thatSnapshotandRenderhave nowhere to put a source label.Still open: #577 (needs
db.Recordingto exposesource_id), #550 (public playout URL has nowhere to put?source=), #549 (scheduler, done in #594).docs/MONITORING.md:69needsand on()— docs lane's file.https://claude.ai/code/session_01A8N3W5ct9SZtHK9sCDD9cL