[eyevinn-developer]

Closes #624

Summary

• Supersedes Add opt-in commentator preview: undelayed per-source WHEP output for remote commentators #542 by @markusnygard. This is his commit 2cbadb3, cherry-picked onto current main, with the git author kept as markusnygard. Thanks, Markus. The design, code, tests and docs are all his.
• Why: Add opt-in commentator preview: undelayed per-source WHEP output for remote commentators #542 conflicts with main in src/routes/returns.ts since fix(returns): keep the return-picture session binding on a revision conflict #621 merged (353e25d). update-branch returns 422, and the branch is on a fork we can't push to. On Finish #542: verify commentator preview against live Strom on openlivedev and re-review #624, @svensson00 ruled that if there was no push by 2026-11-09, we should carry Add opt-in commentator preview: undelayed per-source WHEP output for remote commentators #542 forward on an Eyevinn/open-live branch with Markus's authorship kept, close Add opt-in commentator preview: undelayed per-source WHEP output for remote commentators #542 as superseded, and then do the live-Strom verification and re-review here. There was no push (the head is still 271d2bf).
• What it adds (unchanged from Add opt-in commentator preview: undelayed per-source WHEP output for remote commentators #542): an opt-in commentatorPreview flag on SRT, EFP and clip source assignments. A flagged source gets an undelayed builtin.videoenc plus a builtin.whep_output, tapped before the time_offset blocks. It also adds crew and guest WHEP routes at /previews/:mixerInput/whep, and previews on the guest join/slot responses. See Add opt-in commentator preview: undelayed per-source WHEP output for remote commentators #542's description for the full write-up.

Changes

• One commit (0dd4612): Markus's 2cbadb3, cherry-picked onto main.
• src/routes/returns.ts: the conflict is resolved exactly as posted on Add opt-in commentator preview: undelayed per-source WHEP output for remote commentators #542 (2026-11-05 09:22Z). It keeps Add opt-in commentator preview: undelayed per-source WHEP output for remote commentators #542's side (proxyScopedWhep parameterisation) and takes fix(returns): keep the return-picture session binding on a revision conflict #621's shorter call-site comment. bindWhepSession, MAX_BIND_ATTEMPTS and isDocConflictError exist once, from fix(returns): keep the return-picture session binding on a revision conflict #621. They are not duplicated.
• src/lib/db-errors.ts and src/__tests__/returns-guest-auth.test.ts dropped out of the diff, because Add opt-in commentator preview: undelayed per-source WHEP output for remote commentators #542 carried fix(returns): keep the return-picture session binding on a revision conflict #621's changes byte for byte. The diff against main is now only the commentator-preview change: 15 files, +1474/−46.
• One deviation from 2cbadb3: the TEST_API_KEY fixture in src/__tests__/commentator-preview-routes.test.ts gets a : string annotation. The team's commit guard flags the literal TEST_API_KEY = '...' as a possible secret. It is the same test-only value used in many test files on main. Behaviour is unchanged.

Maintainer decisions (#622), as resolved by the team (#624 step 6)

1. Who may watch: production-wide guest access for v1. A per-invite allow-list or role is deferred.
2. Always-on cost: per-source opt-in, off by default, with no cap in v1.
3. Contract shape: previews is a required, possibly empty array on join and slot. The routes are /previews/:mixerInput/whep.

Test Plan

• pnpm install --frozen-lockfile, pnpm run typecheck, pnpm run build: clean
• npx vitest run: 177 files, 2042/2042 tests pass
• Live-Strom verification on openlivedev (Finish #542: verify commentator preview against live Strom on openlivedev and re-review #624 steps 2–4): a flagged SRT/EFP source and a flagged clip source activate and reach playing. A WHEP client receives video and audio on the crew and guest preview routes. Evidence (or why it couldn't be gathered) is posted as a comment on this PR.
• Re-review against the 2026-10-19 REQUEST_CHANGES findings on Add opt-in commentator preview: undelayed per-source WHEP output for remote commentators #542 (concurrent binding, stale target after a source swap, rebind behaviour documented, log-string nit), by pr-reviewer

Checklist

• Code-quality checks passed. Lint doesn't apply: open-live has no lint script or ESLint config.
• No hardcoded SRT ports
• GStreamer pipelines have watchdogs: not applicable, since the flow is Strom JSON and no new pipelines are started here
• EFP sequence validation: not applicable
• docs/repo-patterns.md: no update needed

🤖 Generated with Claude Code

[eyevinn-developer]

Live-Strom verification (#624 steps 2–4): not done in this run. No evidence was gathered, so none is claimed here.

What I tried, on 2026-11-09:

• The agent-test Strom https://strom-se-sto01.osaas.io:8082 is reachable: GET / returns 200. Its API is not reachable without a token: GET /api/flows returns 401.
• This task's environment has no STROM_AUTH_TOKEN/STROM_TOKEN, so this branch's backend can't be run locally against that Strom (STROM_AUTH_MODE=direct). There is also no CouchDB for a local backend.
• The shared openlivedev beta and the production Strom (:8080) were not touched. Nothing was redeployed, restarted or reconfigured.

Needed to run it:

1. A STROM_AUTH_TOKEN valid on :8082, plus a CouchDB URL, in the task env. Alternatively, a deploy of this branch to a non-shared instance.
2. A live SRT or EFP feed and a clip, so the check can confirm that a WHEP client on /api/v1/productions/:id/previews/video_in_N/whep and /api/v1/guests/:inviteId/previews/:mixerInput/whep receives both video and audio.

Local checks on 0dd4612: typecheck and build are clean, and npx vitest run passes 2042/2042 tests (Strom and CouchDB mocked).

[eyevinn-pr-reviewer]

PR Review: feat(previews): opt-in commentator preview, undelayed per-source WHEP output (carries #542)

Prior findings

Sources read: the 2026-10-19 CHANGES_REQUESTED review on #542 (a11354b), every issue and inline comment on #542 and #626, and the issue comments on #624 and #622. There are no earlier reviews or inline comments on #626. Each row below is judged against the code at 0dd4612.

#	Source	Finding	Status
1	#542 review 2026-10-19, critical	A guest's concurrent preview/return binding is lost on a CouchDB 409	FIXED: 0dd4612, src/routes/returns.ts:133 (bindWhepSession, which re-reads, requires state !== 'left' and retries, from #621/353e25d) and src/routes/returns.ts:549 (merges only its own key into the re-read doc's previewWhepSessionIds). The session mock enforces _rev (src/__tests__/commentator-preview-routes.test.ts:74-86). Tests at :428 (two previews plus the return picture at once) and :474 (guest leaves mid-offer)
2	#542 review 2026-10-19, critical	The Strom assumptions need live verification before merge: a flagged SRT/EFP and a flagged clip source reach playing, and a WHEP client gets video and audio	OPEN: no evidence has been gathered. The #626 comment of 2026-11-09 16:22Z says plainly that it was not done (no Strom token, no CouchDB). No human has waived it
3	#542 review 2026-10-19, needs maintainer decision	Who may watch, always-on cost, and contract shape	FIXED: the contributor opened #622 as asked. The resolutions are recorded on #622 (2026-11-04 16:19Z) and in this PR's body (#624 step 6). I checked the code against them: guests are scoped production-wide through the invite (returns.ts guestPreviewScope), the flag is per source and absent by default (productions.ts assignment schema), previews is always present on join/slot (guests.ts), and the routes are /previews/:mixerInput/whep
4	#542 review 2026-10-19, warning	Stale target after a live source swap	FIXED: 0dd4612, src/lib/commentator-preview.ts:51 (the entry's sourceId must match the current assignment), with sourceId stored at activation (productions.ts, previewWhepUrls). Test at commentator-preview-routes.test.ts:499. Documented in docs/specs/guest-calling-intercom.md:295-298
5	#542 review 2026-10-19, warning	Rebinding the same input overwrites the earlier session id (should be documented)	FIXED: 0dd4612, docs/specs/guest-calling-intercom.md:323-326
6	#542 review 2026-10-19, nit	Return-picture log strings changed to Return WHEP: …	FIXED: 0dd4612, src/routes/returns.ts:213 ('return picture WHEP'). Previews log 'preview WHEP' (:360, :539)
7	#624 step 1	Deploy the #542 head to the openlivedev beta, or run it against its Strom	OPEN: not done (#626 comment 2026-11-09 16:22Z)
8	#624 steps 2–3	Production with a flagged SRT/EFP source and a flagged clip source: activation reaches playing, and a WHEP client gets video and audio ahead of the time_offset delay on both the crew route and the guest route	OPEN: not done. Same evidence gap as row 2
9	#624 step 4	Attach the evidence (to #542, now to #626 per @svensson00's ruling on #624)	OPEN: no evidence attached
10	#624 step 5	Re-review against the 2026-10-19 findings	Done in this review: rows 1 and 3–6 are FIXED, row 2 is OPEN
11	#624 step 6	Record the team's resolutions of #622's three decisions	FIXED: listed in the PR body under "Maintainer decisions", and matching the code (row 3)

Verdict: REQUEST CHANGES

The code is in good shape, and I found no new defects. The verdict rests only on the live-Strom requirement (rows 2 and 7–9). #624 makes "If all checks pass, approve and merge" conditional on live-Strom evidence, the 2026-10-19 review made that evidence a merge blocker, and @svensson00's ruling on #624 says to "run the live-Strom verification and re-review here". Nobody with authority has waived it. Mocked-Strom tests and green CI do not replace it: this change edits the generated flow of every production with a flagged source, and if Strom rejects that topology, activation fails.

Critical issues (must fix before merge)

• Live-Strom verification evidence is missing (#624 steps 1–4). Please run the checks and post the evidence on this PR:

  • A production with a flagged SRT or EFP source and a flagged clip source activates and reaches playing.
  • A WHEP client gets video and audio, ahead of the time_offset delay, on POST /api/v1/productions/:id/previews/video_in_N/whep and on POST /api/v1/guests/:inviteId/previews/:mixerInput/whep.

  This needs either a STROM_AUTH_TOKEN for the :8082 agent-test Strom plus a CouchDB, or a deploy to a non-shared instance, as listed in the PR comment. If no agent holds that access, escalate it under CLAUDE.md as a missing credential. Do not merge without it.

Warnings

• None.

Nits (optional, not blocking)

• Neither the guest preview DELETE nor the return-picture DELETE clears previewWhepSessionIds[mixerInput] after teardown. That is harmless, because the id is dead upstream and a later POST overwrites it, and it matches the existing return-picture behaviour.

What was verified

• Carry-forward fidelity: I diffed 0dd4612 against #542's head 271d2bf. Only two hunks differ: the returns.ts call-site comment, resolved exactly as posted on #542 on 2026-11-05 09:22Z, and the TEST_API_KEY: string annotation in the test. The git author is markusnygard, and the commit has the co-author trailer. The branch is based on current main (353e25d). There is one commit, 15 files, no dependency or lockfile changes, and no eval or child_process. The only "secret" is the shared test literal test-secret-key used across the suite.
• Guest route auth (/api/v1/guests/:inviteId/previews/:mixerInput/whep):
  • The server.ts isGuestTokenAuthedPath exemption is widened only by previews/[^/]+/whep(?:/[^/]+)?, anchored, and only POST and DELETE are registered there.
  • guestPreviewScope checks isGuestCallingEnabled(), then calls resolveGuestSession (HMAC and expiry, stored hash and revocation, persisted expiry, live session), then requires who.invite._id === :inviteId. The production comes from invite.productionId, never from the request.
  • DELETE requires previewWhepSessionIds[mixerInput] === :sessionId, otherwise it returns 403.
  • Tests cover 401 with no token, with the shared API key, and after the guest has left.
• Crew route: it is not in guest-scope.ts's guest-eligible families (WHIP_PATH_RE and RETURN_PICTURE_PATH_RE are unchanged), so it stays behind the shared API key.
• SSRF:
  • The POST target comes only from server-built previewWhepUrls (${config.stromUrl}/whep/${endpointId}), gated by previewTarget, which requires the flag and the same sourceId.
  • The target is re-checked with assertSameStromOrigin.
  • Teardown rebuilds origin/whep/:sessionId from the stored URL, never from client input.
• Validation: the commentatorPreview flag is z.boolean().optional(), stored only when true, and rejected with 400 for source types other than SRT, EFP and clip (503 on a DB error). The flow generator checks PREVIEW_STREAM_TYPES again.
• Refactor: proxyReturnPictureWhep still passes returnStromUrl as the target, 'Return' as the origin label, 'return picture WHEP' as the log name, and the same returnWhepSessionId apply. Teardown uses the same base.
• Flow generator, checked against main:
  • The builtin.whep_output properties (low_latency, mode: 'audio_video', num_audio_tracks: 1) match the existing return outputs at flow-generator.ts:1499.
  • videoenc uses video_in → encoded_out with bitrate: 4000.
  • The taps reuse the same pads that feed the time_offset blocks: inputId:video_out and the selected audioPad, or the clip player's video_out and audio_out.
  • The previews are emitted after the endpoint uniquification, with endpointSuffix.
  • Activation writes previewWhepUrls unconditionally, and deactivatedDoc clears it.
• Local checks on 0dd4612 (fresh clone): pnpm install --frozen-lockfile ✔, pnpm run typecheck ✔, pnpm run build ✔, npx vitest run ✔ (177 files, 2042/2042 tests).
• CI: build-and-test passed (run 42654004403).

[svensson00]

Waiving the live-Strom evidence for this PR: approve and merge on code review and green CI. I will validate the commentator preview end to end on a fresh beta instance (flagged SRT and clip sources, crew and guest preview WHEP URLs, preview ahead of the delayed program) before the next Open Live release, and report back on #624.

[eyevinn-developer]

First re-review: @svensson00 waived the live-Strom evidence (2026-11-09 17:29Z), which was the only open finding of the 16:28Z REQUEST_CHANGES on 0dd4612.

[eyevinn-pr-reviewer]

PR Review: feat(previews): opt-in commentator preview, undelayed per-source WHEP output (carries #542)

The code was reviewed at 0dd4612. The only commit since then is c9c3ac6, a Merge branch 'main' from update-branch that brings in #627. That merge was also checked locally: it is clean, and typecheck, build and vitest pass (2055/2055). CI build-and-test is green on c9c3ac6.

Prior findings

This is the first re-review. The head was unchanged since the 2026-11-09 16:28Z REQUEST_CHANGES review, so each code point was re-checked in source rather than reviewed from scratch.

Waiver: @svensson00 (PM_DECISION_AUTHORITY), comment 6746824591 on #626, 2026-11-09 17:29Z: "Waiving the live-Strom evidence for this PR: approve and merge on code review and green CI." He owns the end-to-end beta validation before the next release and will report on #624.

#	Source	Finding	Status
1	#542 review 2026-10-19, critical	A guest's concurrent preview/return binding is lost on a CouchDB 409	FIXED: 0dd4612, src/routes/returns.ts:132-175 (bindWhepSession: re-read, state !== 'left', bounded retries) and :546-550 (merges only its own key). Tests are in commentator-preview-routes.test.ts
2	#542 review 2026-10-19, critical	Live-Strom verification of the flagged SRT/EFP and clip topology	DROPPED: waived by @svensson00, comment 6746824591
3	#542 review 2026-10-19, maintainer decision	Who may watch, cost, contract shape	FIXED: decided on #622 and recorded in the PR body. The code matches (returns.ts:518, productions.ts:1202, /previews/:mixerInput/whep)
4	#542 review 2026-10-19, warning	Stale target after a source swap	FIXED: src/lib/commentator-preview.ts:51-56; test :499
5	#542 review 2026-10-19, warning	Rebind overwrites the earlier session id	FIXED: documented in docs/specs/guest-calling-intercom.md
6	#542 review 2026-10-19, nit	Return-picture log strings	FIXED: returns.ts:213, :360, :539
7	#624 step 1	Deploy or run against the beta Strom	DROPPED: waived, comment 6746824591
8	#624 steps 2–3	Activation reaches playing; WHEP video and audio arrive ahead of the delay on both routes	DROPPED: waived, comment 6746824591
9	#624 step 4	Attach the evidence	DROPPED: waived, comment 6746824591
10	#624 step 5	Re-review against the 2026-10-19 findings	Done (rows 1–6)
11	#624 step 6	Record #622's resolutions	FIXED: in the PR body, and the code matches

No row is OPEN.

Verdict: APPROVE

Re-verified in source

• Guest route auth:
  • the server.ts:112 exemption is anchored;
  • guestPreviewScope checks the feature flag, then the guest session, then that the invite id matches, and takes the production from the invite;
  • DELETE requires the stored session id;
  • tests cover 401 with no token, with the API key, and after the guest has left.
• Crew route: stays behind the shared API key.
• SSRF: the POST target is built server-side only (previewWhepUrls), gated by previewTarget (flag plus same sourceId) and assertSameStromOrigin. Teardown is built from stored state.
• Validation: the flag is z.boolean().optional(), stored only when true, and limited to SRT, EFP and clip. deactivatedDoc clears previewWhepUrls, and since #627 that covers the idle stop too.

Nit (carried over, not blocking)

• The preview and return-picture DELETE routes do not clear previewWhepSessionIds[mixerInput]. This is harmless.