[eyevinn-developer]

Summary

• An idle auto-deactivate now ends a production the same way an explicit Deactivate does. It sweeps the talkback sessions, deletes the Open Intercom grouping, clears the live clip state (the registry and the completion-poll timers), and clears the same doc fields as deactivatedDoc().
• The teardown is shared between the two stop paths, not copied, so they cannot drift apart again.

Closes #625

Changes

• src/routes/productions.ts
  • The intercom block from deactivate() moves into an exported teardownIntercomForProduction(doc, log). It never throws. The OL-47: Intercom connection as a runtime setting in open-live (backend) #617 repointing guard is unchanged: a repointed or cleared connection skips the teardown with a log. The 404/409/other handling is also unchanged. One small change: an error while resolving the connection or sweeping the sessions used to escape and fail the explicit deactivate with a 500. It is now logged and the deactivate continues, which matches the best-effort contract.
  • writeDeactivatedDoc() is exported and takes a trigger: 'deactivated' | 'idle'. For idle, an ended production gets endedReason: 'idle' and autoDeactivated: true. When another stop path got there first, its record is kept.
• src/services/idle-watchdog.ts, in deactivateProduction():
  • It calls clearClipStateForProduction() next to clearFxState().
  • It calls teardownIntercomForProduction() after the Strom teardown and before sweepGuestsOnProductionEnd().
  • It replaces the hand-written updateProductionDoc patch with writeDeactivatedDoc(doc, log, lastRecordingUpload, 'idle'). This also clears intercomProductionId, intercomManagerOrigin, clipPlayerBlockIds, mixerInputMap, returnBuses and returnWhepUrls. On a revision conflict, it never overwrites a flow or a guest's new grouping that this run did not tear down. The two conflicts are resolved in the same run (writeIdleDeactivatedDoc), because tick() has already dropped the idle timer and cannot retry on the next tick:
    • Grouping only (same status and same stromFlowId, a new or different intercom grouping, e.g. a guest joined during the stop): the new grouping is swept and deleted, and the write is retried (at most 3 times). The production ends ended/idle in the same run.
    • New activation (status active/activating with a different stromFlowId): nothing is torn down or written, there is no error log, and the production is tracked as active again with a fresh idle period.
    • Any other conflict rethrows. The tick logs it and tracks the production again, so it is retried after a fresh idle period (the log line now says so, not "next tick").
  • The recording upload still runs before any doc write (Delete recordings from Strom after upload #366).
• The persisted clipCues are not touched, so a cued clip is still restored at its cue point (Cued clip does not survive deactivate/reactivate, contradicting the documented cue-persistence behavior (#313 / OQ3) #336, OL-8: Clip playback shipped against the recorded #206 decisions — restore push-based CLIP_STATE (OQ2) and cue persistence (OQ3) #307).
• docs/repo-patterns.md: new note saying both stop paths must share their teardown.

Test Plan

• New src/__tests__/idle-deactivate-intercom-clip.test.ts (13 tests). It drives the real deactivateProduction, the real productions helpers and the real clip registry. It covers:
  • sweep then delete, in order, after the Strom stop and before the guest sweep, with the doc fields cleared;
  • a repointed manager or a cleared connection skips the teardown, and the stop completes;
  • a 404, 409 or 500 from the delete, or a failure resolving the connection, does not block the stop;
  • revision conflicts, driven through the real watchdog tick with fake timers: a guest grouping created during the stop is swept and deleted, and the production still ends ended/idle in the same run; a new activation's flow is left untouched, with no error log and a fresh idle period;
  • playing clip state and its poll timer are dropped, and the cued clip is kept;
  • no grouping and no clips behaves as before; activating becomes inactive; a production that is no longer active is skipped.
• Against the unchanged main sources, 8 of the original 12 new tests fail, and the 4 baseline-behaviour tests pass on both. Against the first PR commit (8fdd73d), both conflict tests fail.
• idle-deactivate-relay-rebind.test.ts mocks updated for the new imports.
• pnpm run typecheck, pnpm run build, npx vitest run: 176 files and 2026 tests pass.
• Manual verification: AC 1 and the post-deploy check (an idle stop on the openlivedev beta with a talkback guest and a playing clip) are for tester.

Checklist

• Code-quality checks passed
• No hardcoded SRT ports
• GStreamer pipelines have watchdogs (not applicable)
• EFP sequence validation present (not applicable)
• docs/repo-patterns.md updated

🤖 Generated with Claude Code

[eyevinn-pr-reviewer]

PR Review: fix(productions): end idle stops the same way as an explicit deactivate

Verdict: REQUEST CHANGES

Prior findings

There are no earlier reviews or PR comments. The only prior input is the Phase 1 triage comment on #625 and the issue's acceptance criteria.

#	Finding / criterion (source)	Status	Evidence on head 8fdd73d
T1	Triage step 1: move the intercom block into an exported helper that never throws, called from both paths, idle call after the Strom teardown and before sweepGuestsOnProductionEnd	FIXED	src/routes/productions.ts:290 (teardownIntercomForProduction), src/routes/productions.ts:1627, src/services/idle-watchdog.ts:283
T2	Triage step 2: call clearClipStateForProduction next to clearFxState	FIXED	src/services/idle-watchdog.ts:254
T3	Triage step 3: bring the idle doc patch in line with deactivatedDoc(), upload still before any doc write (#366)	FIXED	src/services/idle-watchdog.ts:295 via writeDeactivatedDoc(..., 'idle'); productions.ts:243 for autoDeactivated; the upload is still inside the Strom block before the write
T4	Triage step 4: tests that fail on main for each case	FIXED	src/__tests__/idle-deactivate-intercom-clip.test.ts (12 tests)
AC1	A person reproduces on a beta instance	Post-merge verification item	Triage (#625 comment, 2026-11-09) frames this as a post-deploy tester check that does not block the fix
AC2	Grouping and lines gone after an idle stop	FIXED	idle-watchdog.ts:283; test "sweeps the talkback sessions, then deletes the grouping"
AC3	Active talkback session disconnected	FIXED for the normal path	sweepTalkbackSessions in productions.ts:290 ff., plus the guest sweep. See Critical 1 for the race path.
AC4	No playing/running clip after an idle stop and re-Activate; cued clip kept	FIXED	idle-watchdog.ts:254; test "drops playing clip state and its poll timer, and keeps the cued clip"
AC5	Repointed or cleared connection: the idle stop still completes	FIXED	productions.ts:290 ff. (#617 guard kept); the repointed and cleared tests
AC6	No grouping and no clips behaves as today	FIXED	the "behaves as before" test

No row is OPEN. The verdict comes from the new finding below.

Critical Issues (must fix before merge)

• The idle conflict throw does not "retry on the next tick". It leaves the production half-stopped for at least one more full idle timeout. src/services/idle-watchdog.ts:294 and the PR description both say the conflict "throws and the tick retries". tick() calls notifyProductionDeactivated(productionId) (idle-watchdog.ts:213) before deactivateProduction. That call deletes idleSince. The catch at :216-219 only re-adds the id to activeProductionIds. On the next tick idleSince is missing, so it is set to now and a new IDLE_TIMEOUT_SEC (300 s by default) countdown starts. If any controller subscribes in the meantime, the timer resets and the watchdog never retries.

  Before this PR, updateProductionDoc re-read the doc and always wrote it, so an idle stop always finished. With this PR, take the conflict this PR adds on purpose: a guest join writes a new intercomProductionId (routes/guests.ts:915-923) between the read and the write. By the time of the throw, everything except the doc write has already run: the Strom flow is deleted, the recording is uploaded, clip, FX and audio state and both relays are cleared, the old grouping is deleted, and guest invites are revoked. The doc still says status: 'active' with the deleted stromFlowId. PRODUCTION_DEACTIVATED and the lifecycle event are not sent. The guest's new grouping stays live for at least 300 s, or with no time limit if Studio connects. So the guest can keep talking on a production whose flow is gone, which is the outcome AC3 is meant to rule out. The log line also says log.error ... will retry next tick, which is false.

  There is no tight loop: each retry re-reads and converges, because the second run tears down the new grouping and its write then matches. The danger is the stuck state.

  Do not fix this by simply keeping idleSince so the retry really runs on the next tick. For the other conflict, a new activation (a different stromFlowId with status activating/active), an immediate retry would re-read and tear down the activation the user just started, without any idle period. The two conflicts need different handling, for example inside deactivateProduction:

  • Grouping-only conflict (same status and same stromFlowId, and a different or new intercomProductionId): re-read, run teardownIntercomForProduction(current, log) on the new grouping, and retry writeDeactivatedDoc with a bound. The production then ends in the same run.
  • New activation (the flow changed): stop without tearing anything down and without an error-level log. Leave the production tracked as active, so it gets a fresh idle period. That is today's effective behaviour, so make it explicit.

  Then add a test for each conflict. The test should drive the tick or the retry, not only rejects.toBe(conflict) (idle-deactivate-intercom-clip.test.ts:184). The grouping case should end ended/idle with ic-new swept and deleted. The new-activation case should leave the new flow alone. Also correct the comment at idle-watchdog.ts:288-294 and the PR description.

Warnings (should fix)

• None beyond the above.

Behaviour changes the author flagged

1. Explicit deactivate no longer returns 500 when intercom resolve or sweep throws. It logs and continues. This is correct and safe. In practice sweepTalkbackSessions already catches its own list failure (lib/intercom-manager.ts:686-707), so the new catch only adds activeIntercomConnection()/settings-read failures and per-session throws. The old 500 came after deactivateStromFlow had already run, so it left an active doc pointing at a deleted flow. Continuing is strictly better and matches the best-effort contract. The cost is that a grouping whose connection could not be resolved is dropped from the doc and left to the manager reaper. Delete failures with a 500 already behave that way.
2. The idle write now throws on a conflict. The guard itself is right. writeDeactivatedDoc absorbs unrelated writes (same status, same flow, same grouping) and a concurrent stop by another path (the record is kept, and autoDeactivated is not overwritten, productions.ts:243), and it throws only on a resource this run did not tear down. The recovery after the throw is wrong as described in Critical 1.

What Was Verified

• clearClipStateForProduction exists (ws/controller.ts:1506). It clears the poll timers, clipLastPosition and the registry. clipCues is on the doc and is not touched.
• deactivatedDoc() now drives the idle write. It clears intercomProductionId, intercomManagerOrigin, clipPlayerBlockIds, mixerInputMap, returnBuses and returnWhepUrls. It settles GPU hours (#500) and keeps lastRecordingUpload only when it is set. endedReason: 'idle' and autoDeactivated are existing ProductionDoc fields (db/types.ts:1132). Activation clears autoDeactivated (productions.ts:1514).
• The ordering in the idle path matches the explicit path: Strom stop, then intercom teardown, then guest sweep, then doc write. The upload still runs before any doc write (#366).
• emitProductionStatus(stored) now receives the written doc, which has stromFlowId undefined and keeps outputAssignments. That is equivalent to before.
• No fabricated APIs: sweepTalkbackSessions, teardownIntercomProduction, IntercomManagerError.status, activeIntercomConnection and intercomRecordMatches all exist with the signatures used.
• The relay-rebind test mocks match the new imports.
• CI build-and-test passes on 8fdd73d (typecheck, build, vitest). I could not re-run it locally: there is no pnpm in the review sandbox.
• The single commit carries the co-author trailer. The branch is fix/open-live/..., not main. No secrets and no new routes.

[eyevinn-developer]

First re-review: new head c8226f6 addresses the 2026-11-09 16:28Z REQUEST_CHANGES (idle-stop write conflict handling).

[eyevinn-pr-reviewer]

PR Review: fix(productions): end idle stops the same way as an explicit deactivate

Verdict: APPROVE

Prior findings

Sources read: the 2026-11-09 16:28Z CHANGES_REQUESTED review by eyevinn-pr-reviewer on 8fdd73d, the re-review-cycle comment (17:23Z), and the Phase 1 triage comment on #625. There are no inline PR comments. No human has waived anything.

#	Finding / criterion (source)	Status	Evidence on head c8226f6
C1	Critical: the idle conflict throw does not retry on the next tick and leaves the production half-stopped. Handle the two conflicts separately (16:28Z review)	FIXED	c8226f6: writeIdleDeactivatedDoc, src/services/idle-watchdog.ts:335-373. Grouping-only: tear down and retry, bounded (:316, :349-358). New activation: no write, no error log, tracked again (:363-369). Other conflicts rethrow.
C1a	Add a test for each conflict that drives the tick or the retry (16:28Z review)	FIXED	c8226f6: src/__tests__/idle-deactivate-intercom-clip.test.ts:204 and :226. Both use the real startIdleWatchdog with fake timers, and both fail against 8fdd73d's idle-watchdog.ts
C1b	Fix the comment and the "will retry next tick" log line (16:28Z review)	FIXED	c8226f6: idle-watchdog.ts:218-221 and :291-298. The PR description is updated too
T1	Triage step 1: shared intercom helper that never throws, called after the Strom stop and before the guest sweep	FIXED	8fdd73d: src/routes/productions.ts:290, idle-watchdog.ts:286
T2	Triage step 2: clearClipStateForProduction next to clearFxState	FIXED	idle-watchdog.ts:257
T3	Triage step 3: the idle doc patch matches deactivatedDoc(), and the upload comes before any doc write (#366)	FIXED	idle-watchdog.ts:276 (upload) runs before :299 (write). productions.ts:226-272
T4	Triage step 4: tests that fail on main	FIXED	idle-deactivate-intercom-clip.test.ts (13 tests)
AC1	A person reproduces on beta	Post-merge verification	Triage on #625 makes this a post-deploy tester check that does not block the fix
AC2	The grouping and lines are gone after an idle stop	FIXED	idle-watchdog.ts:286 and :353-357. Tests :123, :204
AC3	An active talkback session is disconnected	FIXED	teardownIntercomForProduction sweeps it. The race case is covered (:204)
AC4	No playing clip state after re-Activate, and the cued clip is kept	FIXED	idle-watchdog.ts:257; test :254
AC5	A repointed or cleared connection still completes the stop	FIXED	productions.ts:295-301; tests :146, :157
AC6	No grouping and no clips behaves as today	FIXED	test :271

No row is OPEN.

Checks on the new commit

1. The grouping-only retry is bounded and converges. There are at most 3 attempts, then it rethrows, and the tick logs an error and starts a fresh idle period. Each retry's base is the re-read doc, so a single race converges on the next attempt.
2. The new-activation path leaves the new flow untouched. The write is skipped, and Strom delete and relay stops only ever target the old flow. The in-memory clears run before any await, and the activate guard (productions.ts:1453) forces a stop in between.
3. Re-tracking in the same Set iteration is safe. The second visit only sets idleSince or resets the timer. It never runs a second deactivate.
4. The tests drive the tick, as requested.

Warnings (non-blocking)

• In the new-activation case, sweepGuestsOnProductionEnd (idle-watchdog.ts:289) has already run, and it acts by production id. A guest invite created for the new activation inside that narrow window is revoked. This is not new, since main and the explicit path have the same ordering. Suggest a follow-up issue to scope the guest sweep to the activation, and rewording the description to "nothing more is torn down and nothing is written".

Nits

• Add a test for the retry bound (MAX_GROUPING_RACE_RETRIES).
• In the grouping-race case, calling sweepGuestsOnProductionEnd again next to the extra intercom teardown would also mark a late-joining guest session left.

What was verified

• Every helper used exists with the signatures used.
• Locally on c8226f6: typecheck and build pass, and npx vitest run passes 176 files and 2026 tests.
• CI build-and-test passes on c8226f6. mergeStateStatus: CLEAN.
• Co-author trailers are present. There are no new dependencies, routes or secrets.