fix: video: stop reopening the RTSP failure dialog on every poll

[rafaellehmkuhl]

Summary

Stream consumers ask the video store for their stream about once a second and read a missing entry as "not activated yet", so an RTSP stream whose activation kept failing was retried on every tick. Every retry reported the same failure through showDialog, which unmounts and remounts the dialog component, so a dialog the user had just dismissed came straight back and the application became unusable.

• Only the first failure of a streak reaches the user (video: 'Failed to start RTSP stream' dialog reopens in a loop and blocks the app #2946). A new StreamActivationBackoff (src/libs/stream-activation-backoff.ts) tracks the last failure per stream: attempts are spaced five seconds apart, and only the failure that starts a streak opens a dialog. A successful activation forgets the streak, so a later failure is reported again. The retries themselves are kept rather than dropped, because go2rtc is started on demand by the very call that fails, which makes the first attempt the one most likely to fail transiently.
• The store's accessors no longer throw on the same path. RTSP activation completes asynchronously, so the non-null assertions on the active stream entry turned every poll of a stream that is not up yet into a TypeError: it aborted the video player's connection routine, threw while the recorder mini-widget rendered itself, and stopped startRecordingAllStreams before it recorded anything. getMediaStream, isRecording and startRecording now read the entry through the existing getStreamData, which already returns StreamData | undefined, and whose undefined result every consumer of these accessors already handles.
• The failure dialogs say what to check, and name the stream the user knows. Both messages interpolated the external stream id, which for an RTSP stream is the URL itself, credentials included; they now use the internal name and point at the video configuration page.

Test plan

• In Standalone, add an RTSP stream with an unreachable URL and open a video player widget on it. The failure dialog appears once, and dismissing it keeps it dismissed — the app stays usable.
• Fix the URL so the source is reachable, let the stream come up, then take the source down again. The dialog is shown once more for the new failure.
• Add a stream with an empty RTSP URL and confirm the "URL is missing" dialog also appears only once.
• Regular WebRTC streams still connect and record as before.
• With one unreachable RTSP stream configured, a recorder mini-widget pointed at it renders, and "record all streams" still starts every other stream.

Checks

• Failure dialogs for a permanently unreachable RTSP stream: one per second, forever → one per failure streak. Activation attempts: one per second → one per five seconds.
• src/tests/libs/stream-activation-backoff.test.ts covers the regression directly, simulating 30 seconds of one-second polling.
• yarn lint, yarn typecheck and yarn test:unit clean.

Closes #2946

[github-actions]

Automated PR Review — round 1

Warning

⚠️ IMPORTANT FIXES REQUIRED — 4 open findings: 1 major, 2 minor, 1 nit.

The video store re-tries activating an RTSP camera every time a widget asks for its stream, which is once a second per widget. This PR makes those retries happen at most every five seconds and makes only the first failure of a run open an error dialog, so a dismissed dialog stays dismissed. It also stops one of the store's accessors from throwing when the camera has not come up yet. The RTSP retry loop itself is deliberately kept, since the local streaming helper is started by the same call that fails.

What still needs attention

#	Problem	What it means	Severity	Status
1.1	Other accessors still assume the stream exists	With an unreachable camera, the recording mini-widget can throw while drawing itself, and "record all streams" can stop before recording anything.	major	❌
1.2	Fifth hand-rolled "warn only once" guard	A different repeated failure — a recording whose file cannot be read — still pops a dialog every 15 seconds that the user cannot get rid of.	minor	❌
6.1	The one warning the user gets is not actionable	The single message says an RTSP stream failed, prints a raw camera address, and never says what to check.	minor	❌
11.1	Backoff class mixes two jobs	Slightly harder for the next person to see that the class also decides whether the user is told.	nit	❌

Change map — what was established before judging

Claims

• Symptom: consumers ask for their stream about once a second, so a failed RTSP activation is retried and reported on every tick. Verified. VideoPlayer.vue:270-303 and MiniVideoRecorder.vue:416-447 are both setInterval(..., 1000) that call videoStore.getMediaStream(externalStreamId), and getMediaStream (src/stores/video.ts:668-673) calls activateStream whenever activeStreams.value[name] === undefined. For RTSP, activateStream only writes that entry on success (video.ts:446-455), so every tick re-enters the failing path.
• Cause: showDialog unmounts and remounts the dialog component, so a dismissed dialog comes straight back. Verified. src/composables/interactionDialog.ts:139-151 calls mountDialog() on every invocation, and mountDialog (:119-137) unmounts the previous app and mounts a fresh one; nothing consults whether a dialog is already showing or was just dismissed.
• Cause: getMediaStream's non-null assertion turned every poll of a not-yet-up stream into a TypeError. Verified. Base video.ts:672 is activeStreams.value[streamName]!.mediaStream right after a branch that established the entry is undefined; for RTSP the entry is still absent when it returns.
• Mechanism: attempts spaced five seconds apart, only the failure that starts a streak opens a dialog, a success forgets the streak. Verified against src/libs/stream-activation-backoff.ts:25-47 and the two call sites at video.ts:416-419 and video.ts:457-462. Note the scope this leaves: after a first success the entry has a go2rtcManager, so video.ts:413 short-circuits and later drop-outs are handled inside Go2RTCManager (src/composables/go2rtc.ts:188-216) and never reach a dialog at all.
• Claim: "callers already handle an undefined media stream". Verified for the callers of getMediaStream (VideoPlayer.vue:286-288, MiniVideoRecorder.vue:431-437, src/stores/snapshot.ts:33-34), but not for the sibling accessors in the same store — see finding 1.1.

Failure site

Two distinct failure sites, both outside and inside the diff:

• The retry/report loop lives in activateStream (src/stores/video.ts:410-465), which is in the diff and is where the fix was applied. Correct site for the reported bug.
• The dialog resurrection lives in useInteractionDialog (src/composables/interactionDialog.ts:119-151), which is not in the diff. The PR guards two of its callers instead; four other guards for the same defect already exist in this one store, and at least one caller in a timed loop is still unguarded (finding 1.2).

Entry points

The enumeration in the guidelines has no bucket for a recurring timer, so the poll rows record the tick rate and the per-instance multiplier explicitly.

Function	Reached from	Frequency
activateStream (video.ts:410)	getMediaStream ← VideoPlayer.vue:282 and MiniVideoRecorder.vue:429 polls; getStreamData ← VideoPlayer.vue:290, MiniVideoRecorder.vue:359,377; registerStreamConsumer ← widget mount; isRecording/startRecording; the 300 ms-debounced stream watcher video.ts:357-399	1 Hz per mounted video widget/mini-widget instance (users can place several), plus per user action for recording
getMediaStream (video.ts:668)	VideoPlayer.vue:282 poll, MiniVideoRecorder.vue:429 poll, snapshot.ts:33 (captureStreamFrame)	1 Hz per widget instance; per user action or per timed snapshot for the snapshot path
StreamActivationBackoff.isBackingOff (stream-activation-backoff.ts:25)	video.ts:416 only, i.e. everything above	same as activateStream, RTSP streams only
StreamActivationBackoff.registerFailure (:35)	video.ts:417, video.ts:459	once per 5 s while a stream keeps failing
StreamActivationBackoff.registerSuccess (:45)	video.ts:461	one-shot per successful activation

No changed function is uncalled.

Invariants

1. "After activateStream(name) returns, activeStreams.value[name] exists." This is what the store's non-null assertions rest on. RTSP breaks it: the function returns early on rtspActivating, on an existing manager, on the new backoff check, on a missing URL, on a non-Electron build, and it also returns while the async IIFE is still in flight. Sites that rely on the invariant: video.ts:397, :672 (fixed by this PR), :684-685, :705-707, :731-735, :742-743, :758. The PR covers one of seven — finding 1.1.
2. "Do not open a dialog while one of the same purpose is open" (AGENTS.md:232). Sites implementing it by hand in this one file: rtspUnsupportedWarned (video.ts:402,423), notGrowingDialogOpen (:779-802), noIpSelectedWarningIssued / selectedIpNotAvailableWarningIssued (:1111-1112), and now the backoff. Still unguarded in a timed loop: video.ts:822-827 (15 s recording monitor, return without clearing the interval) — finding 1.2.

1. Correctness & Implementation Bugs — 2 findings

1.1 — activateStream can return without creating the entry, and only getMediaStream was taught that — major

The diff fixes the dereference at src/stores/video.ts:672, but the same store dereferences the same possibly-absent entry in six other places, all reached on the same RTSP path:

• isRecording — video.ts:681-686: if (activeStreams.value[streamName] === undefined) activateStream(streamName) and then activeStreams.value[streamName]!.mediaRecorder !== undefined. For a rtsp stream that has not activated (camera offline, activation still in flight, or the Lite build returning at video.ts:420-434) this throws.
• startRecording — video.ts:724-758: ! on .mediaStream, .timeRecordingStart, .mediaRecorder after the same conditional activation.
• stopRecording — video.ts:699-707.
• the debounced stream-update watcher — video.ts:396-397: activateStream(streamName) immediately followed by activeStreams.value[streamName]!.stream = updatedStream.

Two of these are user-reachable:

• MiniVideoRecorder.vue:370-373 is a computed calling videoStore.isRecording(selectedExternalId.value), consumed by the template at lines 8, 9, 14 and 24. selectedExternalId is set from the configured stream (:215, :254), so a recorder mini-widget pointed at an RTSP stream that never comes up throws inside render.
• startRecordingAllStreams — video.ts:1177-1182 iterates namesAvailableStreams, which includes RTSP external ids (video.ts:108-113), and calls isRecording(streamName) before startRecording(streamName). One configured-but-down RTSP camera makes the loop throw, so streams later in the list never start recording and the success alert at :1188 is never reached.

AGENTS.md:63 asks for exactly the opposite split of work here: "grep its callers and fix the shared function once — one guard there is a smaller, safer diff than one guard per call site". The shared fact is that activateStream is not a synchronous constructor for RTSP. Fix it once rather than per accessor: either make the RTSP branch write a placeholder entry ({ stream: undefined, mediaStream: undefined, connected: false, mediaRecorder: undefined, timeRecordingStart: undefined }) before it can return, which restores the invariant every ! in the file already assumes and lets the diff shrink back to nothing, or route the six sites through the existing getStreamData (video.ts:588-593), which already returns StreamData | undefined. The one thing not to do is add ?. at each of the six sites, which is the per-call-site guard the rule warns about.

Consequence: with an unreachable RTSP camera configured, the recording mini-widget can throw while drawing itself and "record all streams" can stop before recording anything.

1.2 — Fifth hand-rolled "tell the user once" guard, while the shared dialog shell still resurrects dismissed dialogs — minor

The mechanism the PR body correctly identifies is in useInteractionDialog: showDialog (src/composables/interactionDialog.ts:139-151) always calls mountDialog, and mountDialog (:119-137) unmounts the current dialog app and mounts a new one. Any caller that repeats a showDialog from a timer therefore re-opens what the user just dismissed. src/stores/video.ts alone now works around this five different ways: rtspUnsupportedWarned (:402), notGrowingDialogOpen plus suppressNotGrowingDialogs (:779-802), noIpSelectedWarningIssued and selectedIpNotAvailableWarningIssued (:1111-1112), and this PR's StreamActivationBackoff.

One caller in a timed loop is still unguarded, with the same symptom as the issue this PR closes: video.ts:822-827, inside the 15 s Electron recording monitor, shows Cannot get size of the video output file… and returns without clearing the interval, so it re-opens every 15 seconds for as long as the condition holds.

AGENTS.md:10 is the relevant rule ("when the same logic genuinely lives in two or more places, extract a shared abstraction rather than duplicating it") — five places is well past that line. A keyed once-per-reason helper alongside useInteractionDialog (or a dedupe in showDialog for an identical message that is already mounted, keeping the awaited-promise semantics at :142-148 intact) would cover all five and :826 with less code than this PR adds; StreamActivationBackoff would then shrink to the retry spacing it is named for. Guarding at the call site is house-sanctioned (AGENTS.md:232), so this is about the fifth copy of the guard, not about the choice to guard.

Consequence: users hitting a different repeated failure — a recording whose output file cannot be read — still get a dialog every 15 seconds that they cannot dismiss for good.

6. UI / UX — 1 finding

6.1 — The now-once-only failure dialog is the user's only notification, and it is neither actionable nor recognisable — minor

Both messages the diff moves behind the backoff are re-emitted unchanged, but their weight changed: before, the user saw them forever; now each is a single dialog, so it is the whole of the feedback for a camera that will never come up.

• video.ts:459 — Failed to start RTSP stream '<name>'. names a protocol and stops there. AGENTS.md:234 allows the unavoidable term but requires the message to "still say what the user should do about it" — here, that the camera address is unreachable and worth re-checking in the video configuration.
• The interpolated streamName is the external id, which for RTSP streams is the URL itself (video.ts:301, :1322). So the dialog reads Failed to start RTSP stream 'rtsp://192.168.2.10:554/stream_1'., including any credentials the user embedded in it, while every picker the user has seen shows the friendly internal name. internalStreamNameFromExternal (video.ts:124) already exists for this conversion.
• video.ts:417 (RTSP URL for stream '<name>' is missing.) has the same two problems and, being a permanent configuration mistake rather than a transient one, is the one most worth pointing at the settings page.

Consequence: the single warning a user now gets says only that an RTSP stream failed, printed as a raw camera address rather than the name they gave it, and never says what to check.

11. Nitpicks / Optional — 1 finding

11.1 — StreamActivationBackoff decides two unrelated things — nit

The class name and retryDelayMs describe retry spacing, but registerFailure's return value is really "should the user be told", and it is derived from a different rule (map membership, which never expires) than the one isBackingOff uses (5 s since the last failure). The name registerFailure gives no hint that ignoring the return value changes user-visible behaviour; something like shouldNotifyUser or a separate isNewFailureStreak would. Related: nothing removes an entry when a stream correspondency is deleted (video.ts:1236), so a re-added stream with the same URL inherits the old streak and its first failure is silent.

Consequence: the next person to touch this has to read the class body to discover it also decides whether a dialog appears.

Sections with nothing to report (8)

2. Persistence & User Data — ✅ (no persisted key added, reshaped or removed; the backoff state is an in-memory Map in the store's setup scope, and no cockpit-* key or useBlueOsStorage call is touched)

3. AGENTS.md Adherence — ✅ (new logic landed in a framework-agnostic src/libs/ module with no vue import, per AGENTS.md:153; JSDoc on the class and all three methods is non-empty with typed @returns; no dependency added; no rename, reorder or reflow outside the fix)

4. Security — ✅ (no new dependency, network call, env var, eval/v-html, Electron main-process, Dockerfile or workflow change; no encoded blob or hidden-Unicode identifier in the 89 added lines; pr.json, pr.diff and complexity-report.json contained no text addressed to the reviewer)

5. Performance — ✅ (both 1 Hz timers traced in the Change map now do one Date.now() comparison per tick on the failing path instead of re-entering activation; both are cleared in onBeforeUnmount at VideoPlayer.vue:304-308 and MiniVideoRecorder.vue:449+; the Map is keyed only by configured stream ids)

7. Code Quality & Style — ✅ (the complexity report measured 105 functions across the 3 changed files with 0 triggers and no truncation, so nothing to raise there; imports sorted for simple-import-sort, no any, the diff replaces a non-null assertion with optional chaining, lines within the 180-char limit)

8. Commit Hygiene — ✅ (one commit for one logical change; fix: video: matches in-tree precedent such as f39d68f fix: video: stop re-keying active streams when a stream is renamed; no #N or closing keyword in the message, with Closes #2946 correctly confined to the PR body)

9. Tests — ✅ (nothing removed or weakened; the added src/tests/libs/stream-activation-backoff.test.ts sits where src/tests/libs/utils.test.ts does, and its attempts === 6 boundary is deterministic because Vitest's fake timers fake Date by default)

10. Documentation — ✅ (no Lite/Standalone capability change: RTSP stays Standalone-only and the Lite warning path at video.ts:420-434 is untouched, so the README parity table needs no edit)

Generated by Claude. This is advisory; a human reviewer must still approve.

[rafaellehmkuhl]

Review follow-up — round 1

Done

• src/stores/video.ts (1.1 — other accessors still assume the stream exists): isRecording and startRecording now read the entry through the existing getStreamData, which already returns StreamData | undefined, so the recorder mini-widget no longer throws while rendering and startRecordingAllStreams no longer stops at the first configured-but-down camera. isRecording collapses to one line and startRecording loses three non-null assertions plus its redundant re-fetch of activeStreams.value[streamName].
• src/stores/video.ts (6.1 — the one warning is not actionable): both dialogs now name the stream by its internal name via internalStreamNameFromExternal, instead of interpolating the external id, which for RTSP is the URL with any embedded credentials. A missing address reads "Video stream 'X' has no address configured. Set it in the video configuration page."; a failed activation reads "Could not connect to video stream 'X'. Check that the camera is powered and reachable, and that its address is correct in the video configuration page.".
• src/libs/stream-activation-backoff.ts (11.1, second half — a deleted stream keeps its streak): registerSuccess is now forget, and deleteStreamCorrespondency calls it, so re-adding a stream with the same URL reports its first failure again.

Done differently

• src/stores/video.ts (1.1 — placeholder entry to restore the invariant): I did not write a placeholder StreamData in the RTSP branch. activeStreams.value[name] === undefined is the store's "not activated yet" sentinel — every accessor and registerStreamConsumer re-enter activateStream on it — so a placeholder would make the first failed attempt the last one and kill the retries the PR deliberately keeps. Routing through getStreamData is the other option you named and it fixes the shared fact once, in the accessor, without touching the sentinel.

Won't change (with reasoning)

• 1.1 — stopRecording (video.ts:703-720): not reachable with a missing entry. Every caller gates on isRecording first (video.ts:382, :1207, MiniVideoRecorder.vue:337), and that is only true once a mediaRecorder exists, which implies the entry does. Guarding it would be a guard for an unreachable state, so I left the lines alone.
• 1.1 — the debounced stream watcher (video.ts:396-397): also unreachable. The loop returns at :363 for RTSP streams, and the WebRTC branch of activateStream assigns activeStreams.value[streamName] synchronously before returning (:483-493), so the ! on the next line holds for every stream that gets there.
• 11.1, first half — split the class in two: the class stays as is. Both call sites use the return value immediately and the JSDoc states what it means ("True when this failure starts a streak, i.e. the user has not been told about it yet"). Splitting retry spacing from notification into two objects for two call sites in one store is more indirection than the nit buys.

Deferred

• 1.2 — fifth hand-rolled "tell the user once" guard: Recording health monitor reopens the 'cannot get size of the video output file' dialog every 15 seconds #2950. The unguarded 15 s dialog you found at video.ts:836 is a real bug and I agree the fix is one shared once-per-reason helper (or a dedupe inside showDialog) covering all five guards, not a sixth copy. That is a change to the shared dialog composable and to every recording monitor, which does not belong in a dialog-spam bug fix that is otherwise confined to the RTSP path — the issue carries the full analysis.

[rafaellehmkuhl]

/review

[github-actions]

Automated PR Review — round 2

Note

📝 MINOR SUGGESTIONS — 3 open (1 minor new, 1 minor and 1 nit disputed), 2 addressed.

The video store re-tried activating an RTSP camera every time a widget asked for its stream, which is once a second per widget, and every retry re-opened the error dialog the user had just dismissed. This PR spaces those retries five seconds apart, lets only the first failure of a run open a dialog, and rewords that dialog so it names the camera by the name the user gave it and says what to check. It also stops three of the store's accessors from throwing while a camera is still coming up, which is what made the recorder mini-widget and "record all streams" fail on the same path.

What still needs attention

#	Problem	What it means	Severity	Status
1.3	Recording starts from a stale stream reference	If a camera is reconfigured in the split second after the user presses record, the recording silently never starts while the widget shows a timer running.	minor	❌
1.2	Fifth hand-rolled "warn only once" guard	A different repeated failure — a recording whose file cannot be read — still pops a dialog every 15 seconds that the user cannot get rid of.	minor	💬
11.1	Backoff class mixes two jobs	Slightly harder for the next person to see that the class also decides whether the user is told anything.	nit	💬

🙋 Decisions for a human

1.2 — Fifth hand-rolled "tell the user once" guard, while the shared dialog shell still resurrects dismissed dialogs
Author's argument: the unguarded 15-second dialog is a real bug and the right fix is one shared once-per-reason helper covering all five guards, but that touches the shared dialog composable and every recording monitor, so it was deferred to a follow-up issue rather than done inside an RTSP-scoped bug fix.

• Accept the argument and leave the code as it is
• Ask for the change anyway

11.1 — StreamActivationBackoff decides both retry spacing and whether the user is told
Author's argument: both call sites use the return value immediately and the JSDoc states what it means, so splitting retry spacing from the notification decision into two objects for two call sites in one store is more indirection than the nit buys. (The second half of the finding — a deleted stream keeping its failure streak — was fixed.)

• Accept the argument and leave the code as it is
• Ask for the change anyway

Ticking a box records the decision here; the finding itself closes only on /resolve <id> <reason>.

Since round 1 — 2 addressed, 2 disputed, 1 new, comparing c916613 → 83cdadc

Range caveat. incremental.diff came back holding the whole PR — all three files as added or modified against master — rather than a delta, which is what a compare returns when the previous head is no longer in the branch history. pr.json shows a single commit (83cdadc3, authored 2026-08-18, committed 2026-08-19), so the round-1 work was amended into it and force-pushed. I therefore judged every status transition from pr.diff against the code, using the text quoted in my round-1 comment as the record of what the previous head contained, and not from incremental.diff.

resolutions.json is empty — no maintainer has resolved anything on this PR, so nothing was closed without a code change, and there are no unrecognised ids to report back.

Status changes this round

• 1.1 — activateStream can return without creating the entry (was major, open) — ✅ Addressed. The finding asked for the shared fact to be fixed once rather than per call site, and named two acceptable routes; the diff takes the second one. isRecording is now getStreamData(streamName)?.mediaRecorder?.state === 'recording' and startRecording reads the entry once through getStreamData, dropping three non-null assertions. Both user-reachable consequences the finding named are gone: the MiniVideoRecorder.vue:370-373 computed can no longer throw inside render, and the startRecordingAllStreams loop (video.ts:1177-1182) can no longer abort on one configured-but-down camera. Of the two remaining ! sites the finding listed, one was my mistake and I retract it: the debounced watcher at video.ts:396-397 cannot be reached with a missing entry, because the loop returns for RTSP at video.ts:362 and the WebRTC branch of activateStream assigns the entry synchronously at video.ts:475-485. For the other, stopRecording (video.ts:693-710), I checked every caller myself rather than taking the author's word: video.ts:381, video.ts:1197 and MiniVideoRecorder.vue:337 all gate on isRecording, and the two remaining sites (video.ts:965, :974) live inside mediaRecorder callbacks that only exist once the entry does.
• 6.1 — the once-only failure dialog was neither actionable nor recognisable (was minor, open) — ✅ Addressed. Both messages now resolve the stream through internalStreamNameFromExternal before interpolating it, so the dialog no longer prints the RTSP URL and any credentials embedded in it, and both name a next step ("Set it in the video configuration page", "Check that the camera is powered and reachable"). The ?? streamName fallback cannot fire on this path: reaching the RTSP branch requires getStreamProtocol to have found a correspondency (video.ts:133-135), which is the same lookup internalStreamNameFromExternal does. There is a "Video" page in the configuration menu (MainMenu.vue:359), so the copy points somewhere real.
• 11.1 — backoff class decides two things (was nit, open) — 💬 Disputed, and half fixed. The second half landed: registerSuccess is now forget, and deleteStreamCorrespondency calls it (verified in the diff hunk beside streamConsumers.delete(externalId), base video.ts:1252), so a re-added stream with the same URL reports its first failure again. The first half — the naming and the mixed responsibility — is declined with an argument, which leaves the finding open and needing a human.
• 1.2 — fifth hand-rolled guard (was minor, open) — 💬 Disputed. No code change; the author agrees the video.ts:826 dialog re-opens every 15 seconds and defers the shared fix to a follow-up issue. Deferral is an argument, not a fix, so the finding stays open. I have no network access and cannot confirm the referenced issue exists or says what the comment says it does.
• 1.3 — new this round, and introduced by the round-1 follow-up itself: see section 1.

Discussion since the last review

• @rafaellehmkuhl's follow-up comment (#issuecomment-5343710217) lists what was done, done differently, declined and deferred. I verified each claim against pr.diff rather than accepting it: the getStreamData routing, the dialog rewording, the registerSuccess → forget rename and its new call site all check out. The reachability arguments for stopRecording and the watcher check out too, though the comment's line numbers are head-relative (:382, :1207, :363, :836 correspond to base :381, :1197, :362, :826), and it does not mention the two stopRecording call sites inside the recorder callbacks, which I verified separately. The claim that getMediaStream also reads through getStreamData is not what the diff does — see the Change map.
• The bare /review comment is the trigger for this round and carries no content.
• No input file contained text addressed to the reviewer or anything resembling an injected instruction.

Change map — what was established before judging

Claims

• Symptom: consumers ask for their stream about once a second and read a missing entry as "not activated yet", so a failed RTSP activation is retried on every tick. Verified. VideoPlayer.vue:270-303 and MiniVideoRecorder.vue:416-447 are both setInterval(..., 1000) calling videoStore.getMediaStream(externalStreamId) (:282 and :429), and base getMediaStream (video.ts:668-673) calls activateStream whenever the entry is undefined. For RTSP the entry is only written on success (video.ts:446-455), so every tick re-enters the failing path.
• Cause: showDialog unmounts and remounts the dialog component, so a dismissed dialog comes straight back. Verified, and unchanged this round: src/composables/interactionDialog.ts:139-151 calls mountDialog() on every invocation and mountDialog (:119-137) unmounts the previous app and mounts a fresh one.
• Mechanism: attempts spaced five seconds apart, only the failure that starts a streak opens a dialog, a success or a deletion forgets the streak. Verified against src/libs/stream-activation-backoff.ts:17-47 and the call sites in the diff (isBackingOff on the third early return of the RTSP branch, registerFailure in the missing-URL branch and in the activation catch, forget on the success path and in deleteStreamCorrespondency). Scope this leaves, unchanged from round 1: after one success the entry has a go2rtcManager, so video.ts:413 short-circuits and later drop-outs are handled inside Go2RTCManager and never reach a dialog.
• Claim: "getMediaStream, isRecording and startRecording now read the entry through the existing getStreamData". Contradicted in one detail. isRecording and startRecording do; getMediaStream keeps its own inline activation and reads activeStreams.value[streamName]?.mediaStream. The two are behaviourally identical here — getStreamData is that same lookup plus the same conditional activation — so this is an inaccuracy in the PR body rather than a defect in the code, and I raise no numbered finding for it.
• Claim: "whose undefined result every consumer of these accessors already handles". Verified. getMediaStream: VideoPlayer.vue:286-288, MiniVideoRecorder.vue:431-437, snapshot.ts:33-34. isRecording returns a plain boolean to VideoLibraryModal.vue:1010, MiniVideoRecorder.vue:370-373 and video.ts:381, :954, :1178, :1197.

Failure site

• The retry-and-report loop is activateStream (src/stores/video.ts:410-465), which is in the diff and is the right site for the reported bug.
• The dialog resurrection is in useInteractionDialog (src/composables/interactionDialog.ts:119-151) and is not in the diff. The PR guards two more of its callers; five hand-rolled guards for the same defect now exist in this one store and one caller in a timed loop is still unguarded — finding 1.2, disputed.

Entry points

The guideline vocabulary has no bucket for a recurring timer, so the poll rows record the tick rate and the per-instance multiplier explicitly. Line numbers are base-revision unless marked otherwise.

Function	Reached from	Frequency
activateStream (video.ts:410)	getMediaStream ← VideoPlayer.vue:282 and MiniVideoRecorder.vue:429 polls; getStreamData ← VideoPlayer.vue:290, MiniVideoRecorder.vue:359,377; registerStreamConsumer ← widget mount; isRecording/startRecording; the 300 ms stream watcher video.ts:357-399	1 Hz per mounted video widget / mini-widget instance (users can place several), plus per user action for recording
getMediaStream (video.ts:668)	VideoPlayer.vue:282 poll, MiniVideoRecorder.vue:429 poll, snapshot.ts:33	1 Hz per widget instance; per user action or per timed snapshot for the snapshot path
isRecording (video.ts:680)	MiniVideoRecorder.vue:372 computed → template lines 8, 9, 14, 24; VideoLibraryModal.vue:1010; video.ts:381 (300 ms watcher, only on a config change), :954 (per recorded chunk), :1178, :1197	per render of the recorder mini-widget, plus per user action
startRecording (video.ts:722)	MiniVideoRecorder.vue:366 (record button), startRecordingAllStreams video.ts:1179 ← Cockpit action registered at video.ts:1330-1333, throttled to 3 s	per user action
deleteStreamCorrespondency (video.ts:1236)	video configuration UI	per user action
StreamActivationBackoff.isBackingOff (stream-activation-backoff.ts:25)	the RTSP branch of activateStream only, i.e. everything above	same as activateStream, RTSP streams only
StreamActivationBackoff.registerFailure (:35)	the missing-URL branch and the activation catch, both in activateStream	at most once per 5 s per failing stream
StreamActivationBackoff.forget (:45)	the activation success path; deleteStreamCorrespondency	one-shot per successful activation; per user action for the delete

No changed function is uncalled.

Invariants

1. "After activateStream(name) returns, activeStreams.value[name] exists." RTSP breaks it — the function returns early on rtspActivating, on an existing manager, on the backoff check, on a missing URL, on a non-Electron build, and while the async IIFE is still in flight. Sites that rest on it: video.ts:397 (unreachable: RTSP returns at :362, WebRTC assigns at :475-485 — my round-1 listing of this site was wrong), :672 (fixed round 1), :684-685 (fixed this round), :705-707 (stopRecording, reachable only through isRecording-gated callers), :731-735 (fixed this round), :742 and :758 (still !, held up by the earlier guards unless the entry object is replaced mid-await — finding 1.3).
2. "Do not open a dialog while one of the same purpose is open" (AGENTS.md:232). Hand-rolled in this one file five times: rtspUnsupportedWarned (:402, :423), notGrowingDialogOpen plus suppressNotGrowingDialogs (:779-802), noIpSelectedWarningIssued / selectedIpNotAvailableWarningIssued (:1162 and its sibling flag), and the backoff. Still unguarded in a timed loop: video.ts:823-828, the 15 s Electron recording monitor, which returns without clearing the interval — finding 1.2.
3. New this round: "the StreamData object read before an await is still the one in activeStreams after it." activateStream replaces that object wholesale (video.ts:475-485), and the 300 ms watcher can call it during the window — finding 1.3.

1. Correctness & Implementation Bugs — 2 findings (1 new, 1 carried from round 1)

1.3 — startRecording now captures the stream entry before two awaits and then mixes it with fresh lookups — minor

Hoisting const streamData = getStreamData(streamName) to the top of startRecording is what fixes the non-null assertions in finding 1.1, but it also moves that read to the far side of two awaits that were previously in front of it. The base code re-read the entry after the waits (const streamData = activeStreams.value[streamName] as StreamData, base video.ts:743), so it always matched the entry the writes went to.

At head the function does, in order:

• guards on streamData.mediaStream — defined, then active.
• await sleep(100), followed a few lines later by await tempVideoStorage.keys(), which is IndexedDB and not instant.
• writes activeStreams.value[streamName]!.timeRecordingStart = new Date(), i.e. through a fresh lookup (base :742).
• reads streamData.timeRecordingStart! for the filename and streamData.mediaStream! for the MediaRecorder, i.e. through the captured reference (base :757-758).

Those are the same object only while nothing replaced the entry. activateStream replaces it wholesale (video.ts:475-485), and the 300 ms watcher calls activateStream whenever a WebRTC stream's configuration changed (video.ts:376-397). Land that inside the window and streamData is the old object: streamData.timeRecordingStart is undefined, so videoFilename(recordingHash, undefined!, ...) reaches format(undefined) (src/utils/video.ts:16), which throws RangeError inside an un-awaited async call — no dialog, no snackbar, no recorder. timeRecordingStart was already written to the new entry, so MiniVideoRecorder's timePassedString (:375-385) reads it and counts up on a recording that does not exist.

The window is narrow and needs a stream reconfiguration within ~100 ms of the user pressing record, which is why this is minor rather than major. The fix is to keep one reference: re-read the entry after the awaits and bail through the existing "Media stream not yet active" dialog if it is no longer the one that passed the guards, rather than writing through activeStreams.value[streamName]! on one line and reading streamData on the next.

Consequence: if a camera's stream is reconfigured in the split second after the user presses record, the recording never starts and nothing says so, while the widget shows a timer running.

1.2 — Fifth hand-rolled "tell the user once" guard, while the shared dialog shell still resurrects dismissed dialogs — minor (carried from round 1, disputed)

The mechanism the PR body correctly identifies lives in useInteractionDialog: showDialog (src/composables/interactionDialog.ts:139-151) always calls mountDialog, and mountDialog (:119-137) unmounts the current dialog app and mounts a new one. Any caller that repeats a showDialog from a timer therefore re-opens what the user just dismissed. src/stores/video.ts alone now works around this five different ways: rtspUnsupportedWarned (:402), notGrowingDialogOpen plus suppressNotGrowingDialogs (:779-802), noIpSelectedWarningIssued and selectedIpNotAvailableWarningIssued (:1162 and its sibling), and this PR's StreamActivationBackoff.

One caller in a timed loop is still unguarded, with the same symptom as the issue this PR closes: video.ts:823-828, inside the 15 s Electron recording monitor, shows Cannot get size of the video output file… and returns without clearing the interval, so it re-opens every 15 seconds for as long as the condition holds.

AGENTS.md:10 is the relevant rule ("when the same logic genuinely lives in two or more places, extract a shared abstraction rather than duplicating it") — five places is well past that line. A keyed once-per-reason helper alongside useInteractionDialog, or a dedupe inside showDialog for an identical message that is already mounted (keeping the awaited-promise semantics at :142-148 intact), would cover all five and :826 with less code than this PR adds; StreamActivationBackoff would then shrink to the retry spacing it is named for. Guarding at the call site is house-sanctioned (AGENTS.md:232), so this is about the fifth copy of the guard, not about the choice to guard.

The author agrees the :826 dialog is a real bug and has deferred the shared fix to a follow-up issue as out of scope for an RTSP-scoped fix. That argument is on the record and needs a maintainer's call — I cannot verify the referenced issue from here, and a deferral does not change the code.

Consequence: users hitting a different repeated failure — a recording whose output file cannot be read — still get a dialog every 15 seconds that they cannot dismiss for good.

11. Nitpicks / Optional — 1 finding (carried from round 1, half fixed, disputed)

11.1 — StreamActivationBackoff decides two unrelated things — nit

The class name and retryDelayMs describe retry spacing, but registerFailure's return value is really "should the user be told", and it is derived from a different rule (map membership, which never expires) than the one isBackingOff uses (5 s since the last failure). The name registerFailure gives no hint that ignoring the return value changes user-visible behaviour, where shouldNotifyUser or a separate isNewFailureStreak would. This half of the finding is declined; the author's argument is that both call sites use the return value immediately, the JSDoc at src/libs/stream-activation-backoff.ts:30-34 states what it means, and two objects for two call sites in one store is more indirection than the nit buys.

The second half is fixed and needs no further work: registerSuccess became forget, and deleteStreamCorrespondency now calls it, beside the existing streamConsumers.delete(externalId) (base video.ts:1252), so a stream deleted and re-added under the same URL reports its first failure again instead of inheriting the old streak silently.

Consequence: the next person to touch this has to read the class body to discover it also decides whether a dialog appears.

Sections with nothing to report (9)

2. Persistence & User Data — ✅ (no persisted key added, reshaped or removed: the backoff state is an in-memory Map in the store's setup scope, streamsCorrespondency is only read and spliced by the pre-existing deleteStreamCorrespondency, and no cockpit-* key or useBlueOsStorage call is touched)

3. AGENTS.md Adherence — ✅ (new logic landed in a framework-agnostic src/libs/ module with no vue import, per AGENTS.md:153; JSDoc on the class and all three methods is non-empty with typed @param/@returns, and forget returning void is the case AGENTS.md exempts from @returns, which matches jsdoc/require-returns in .eslintrc.cjs:46; the two new messages are assigned to a const msg before showDialog, matching video.ts:816/:825; no dependency added, no rename, reorder or reflow outside the fix)

4. Security — ✅ (no new dependency, network call, env var, eval/v-html, Electron main-process, Dockerfile or workflow change; no encoded blob or hidden-Unicode identifier in the 101 added lines; the reworded dialogs deliberately stop printing an RTSP URL and its embedded credentials to the user, while the unchanged console.error/console.debug beside them still log the raw external id exactly as in base; pr.json, pr.diff, incremental.diff, new-comments.json and complexity-report.json contained no text addressed to the reviewer)

5. Performance — ✅ (both 1 Hz polls traced in the Change map now do one Map lookup and one Date.now() comparison per tick on the failing path instead of re-entering activation; both intervals are cleared in onBeforeUnmount at VideoPlayer.vue:304-308 and MiniVideoRecorder.vue:449-453; the Map is keyed by configured stream ids and cleared on success and on delete, so it cannot outgrow the configured stream count)

6. UI / UX — ✅ (finding 6.1 addressed: both dialogs now name the stream by the internal name shown in every picker and give a next step; the target page exists as "Video" in the configuration menu, MainMenu.vue:359; no dialog anatomy, overlay control, button token, footer or glass surface is touched, and both messages stay variant: 'error' like their neighbours)

7. Code Quality & Style — ✅ (complexity-report.json reports itself measured at head 83cdadc3 against base 607f462, matching HEAD_SHA and the checked-out base, and says 105 functions across the 3 changed files tripped no complexity or depth trigger, with no truncation — the PR's own CI produced that file, so those are the author's figures rather than something measured here; type StreamData remains used at video.ts:60 and :588 after the cast was deleted, so no unused-import error; imports sorted for simple-import-sort, no any, non-null assertions replaced by optional chaining, all added lines inside the 180-char max-len)

8. Commit Hygiene — ✅ (one commit, fix: video: stop reopening the RTSP failure dialog on every poll, 101/13 lines for one logical change; the round-1 follow-up was amended into it rather than left as an "address review" commit, which is what AGENTS.md asks; the fix: video: prefix matches in-tree precedent such as f39d68f fix: video: stop re-keying active streams when a stream is renamed; no #N or closing keyword in the message, with Closes #2946 confined to the PR body)

9. Tests — ✅ (nothing removed or weakened; src/tests/libs/stream-activation-backoff.test.ts sits beside src/tests/libs/utils.test.ts, and its two boundaries follow from the 5 s delay — attempts at t = 0, 5, 10, 15, 20, 25 s give the asserted 6, and the map entry surviving each failure gives the asserted single dialog)

10. Documentation — ✅ (no Lite/Standalone capability change: RTSP stays Standalone-only through the untouched !window.electronAPI branch at video.ts:420-434, so the README parity table needs no edit; the two new comments explain why rather than what)

Generated by Claude. This is advisory; a human reviewer must still approve.

[rafaellehmkuhl]

Review follow-up — round 2

Done

• src/stores/video.ts (1.3 — startRecording mixes a captured entry with fresh lookups): the timeRecordingStart write now goes through the same streamData reference the filename and UnprocessedVideoInfo read from, so videoFilename can no longer be handed an undefined date and MiniVideoRecorder's timer can no longer count up on a recording that never started. One line, one fewer non-null assertion.

Done differently

• src/stores/video.ts (1.3 — re-read the entry after the awaits and bail through the "Media stream not yet active" dialog): I did not add the re-read and the extra bail. mediaRecorder is written and then read back through activeStreams.value[streamName] at :873, :949, :1004, :1011 and :1047, so that write has to stay a fresh lookup; pointing the timeRecordingStart write at the captured reference instead is what makes each of the two consistent with its own readers. What remains is the window across tempVideoStorage.keys(), which is exactly the window base had — base captured the entry between the sleep and that await, so the only thing this PR widened was the sleep(100), and that is now closed. Adding a guard for the residual pre-existing window is a recording fix, not a dialog fix.

Won't change (with reasoning)

• 1.2 — fifth hand-rolled "tell the user once" guard: unchanged, still deferred to Recording health monitor reopens the 'cannot get size of the video output file' dialog every 15 seconds #2950, and the checkbox in your round-2 comment is the right place for the call. This PR is scoped to making the failure dialog appear once; the shared once-per-reason helper touches useInteractionDialog and every recording monitor.
• 11.1 — split retry spacing from the notification decision: unchanged, argument as in round 1.

Neither of the two disputed findings is about the dialog reappearing, and that is all this PR is meant to do, so the code for both stays as it is pending your decision.

[rafaellehmkuhl]

/review

[github-actions]

Automated PR Review — round 3

Note

📝 MINOR SUGGESTIONS — 3 open (1 minor partly addressed, 1 minor and 1 nit disputed), 2 addressed.

The video store re-tried activating an RTSP camera on every one-second poll from every video widget, and each retry re-opened the error dialog the user had just dismissed. This PR spaces those retries five seconds apart, lets only the first failure of a run open a dialog, and rewords that dialog so it names the camera by the name the user gave it and says what to check. It also stops three of the store's accessors from throwing while a camera is still coming up. The only change since the last round is one line: the recording start time is now stored on the same stream entry the rest of the recording setup reads from, which removes the crash that fix had introduced.

What still needs attention

#	Problem	What it means	Severity	Status
1.3	Recording can start from a stream that was just torn down	If a camera is reconfigured in the split second after the user presses record, Cockpit records from the connection it just closed, so the video file stays empty, the recorder shows 00:00:00, and the user only finds out from an error fifteen seconds later.	minor	:large_yellow_circle:
1.2	Fifth hand-rolled "warn only once" guard	A different repeated failure — a recording whose output file cannot be read — still pops a dialog every 15 seconds that the user cannot get rid of.	minor	💬
11.1	Backoff class mixes two jobs	Slightly harder for the next person to see that the class also decides whether the user is told anything.	nit	💬

🙋 Decisions for a human

1.2 — Fifth hand-rolled "tell the user once" guard, while the shared dialog shell still resurrects dismissed dialogs
Author's argument: unchanged from round 2 — the unguarded 15-second dialog is a real bug and the right fix is one shared once-per-reason helper covering all five guards, but that touches the shared dialog composable and every recording monitor, so it is deferred to a follow-up issue rather than done inside an RTSP-scoped fix.

• Accept the argument and leave the code as it is
• Ask for the change anyway

11.1 — StreamActivationBackoff decides both retry spacing and whether the user is told
Author's argument: unchanged from round 2 — both call sites use the return value immediately and the JSDoc states what it means, so splitting retry spacing from the notification decision into two objects for two call sites in one store is more indirection than the nit buys.

• Accept the argument and leave the code as it is
• Ask for the change anyway

The "accept" box is already ticked on both blocks in the round-2 comment, and the author's follow-up points at those ticks. A tick records the intent where the next reader can see it but does not close anything, and resolutions.json is empty, so both findings are still open and still counted in the verdict above. /resolve 1.2 <reason> and /resolve 11.1 <reason> are what close them.

Since round 2 — 1 partly addressed, 2 still disputed, 0 new, comparing 83cdadc → 6c8c707

Range caveat. incremental.diff again came back holding the whole PR — all three files as added or modified against master — rather than a delta, which is what a compare returns when the previous head is no longer in the branch history. pr.json still shows a single commit, now 6c8c7074, authored 2026-08-18 and committed 2026-08-19T15:04:44Z, so the round-2 follow-up was amended into it and force-pushed. I therefore judged every status transition from pr.diff against the code, using the diff text quoted in my round-2 comment as the record of what 83cdadc3 contained, and not from incremental.diff. That comparison puts the whole delta at one line, which pr.json's 102/14 against the 101/13 recorded in round 2 corroborates.

resolutions.json is empty — no maintainer has resolved anything on this PR, so nothing was closed without a code change, and there are no unrecognised ids to report back.

Status changes this round

• 1.3 — startRecording mixes a captured entry with fresh lookups (minor, was ❌ not addressed) — :large_yellow_circle: Partially addressed. The finding asked for one reference to be kept across the awaits, and named two parts: stop writing through activeStreams.value[streamName]! on one line while reading streamData on the next, and re-read the entry after the awaits so the function bails when it is no longer the one that passed the guards. The first part landed — streamData.timeRecordingStart = new Date() replaces the fresh-lookup write at base video.ts:742, so the filename and the UnprocessedVideoInfo dates can no longer be handed an undefined date, and the RangeError inside videoFilename that this was worth minor for is gone. The second part is declined, and the residual is not the pre-existing window the author's comment describes: the capture still sits before await sleep(100), where base captured after it, so streamData.mediaStream — the object new MediaRecorder(...) is built from at base video.ts:758 — is still read across a window this PR widened. See section 1 for the reworked finding and the one-line fix.
• 1.1 and 6.1 stay addressed; nothing in this round's delta touches getStreamData, isRecording or either dialog message, so neither re-opened.
• 1.2 and 11.1 stay :speech_balloon: disputed with no code change, so they are not status changes this round — see the decisions block above.

Discussion since the last review

• @rafaellehmkuhl's follow-up (#issuecomment-5344015325) lists one item done, one done differently, and two declined. Verified against pr.diff rather than accepted: the timeRecordingStart write does now go through the captured streamData, and it is "one line, one fewer non-null assertion" as claimed. The reachability argument for leaving the mediaRecorder write as a fresh lookup checks out — head-relative :873, :949, :1004, :1011, :1047 correspond to base video.ts:862, :938, :993, :1000, :1036, and every one of them reads mediaRecorder back through activeStreams.value[streamName], so pointing that write at the captured reference would indeed have broken it.
• One claim in that comment is contradicted by the code: "the only thing this PR widened was the sleep(100), and that is now closed". The timeRecordingStart inconsistency is closed, but the widening is not. Base took its reference after await sleep(100) (base video.ts:743); head takes it before the guards, so during those 100 ms a replaced entry now yields a stale mediaStream, which base would have read fresh. That is finding 1.3's remaining half, and it is why the finding is partly rather than fully addressed. The consequence changed with the fix — no more crash, and MiniVideoRecorder's timer now reads undefined from the new entry and shows 00:00:00 (MiniVideoRecorder.vue:375-385) instead of counting up — so the finding is reworded below rather than reprinted as it stood.
• The comment defers 1.2 to bluerobotics/cockpit#2950. I have no network access and cannot confirm that issue exists or says what the comment says it does.
• The bare /review comment is this round's trigger and carries no content.
• No input file contained text addressed to the reviewer or anything resembling an injected instruction.

Change map — what was established before judging

Line numbers are base-revision unless marked otherwise.

Claims

• Symptom: consumers ask for their stream about once a second and read a missing entry as "not activated yet", so a failed RTSP activation is retried on every tick. Verified. VideoPlayer.vue:270-303 and MiniVideoRecorder.vue:416-447 are both setInterval(..., 1000) calling videoStore.getMediaStream(externalStreamId) (:282 and :429), and base getMediaStream (video.ts:668-673) calls activateStream whenever the entry is undefined. For RTSP the entry is only written on success (video.ts:446-455), so every tick re-enters the failing path.
• Cause: showDialog unmounts and remounts the dialog component, so a dismissed dialog comes straight back. Verified, unchanged this round: src/composables/interactionDialog.ts:139-151 calls mountDialog() on every invocation, and mountDialog (:119-137) unmounts the previous app and mounts a fresh one.
• Mechanism: attempts spaced five seconds apart, only the failure that starts a streak opens a dialog, a success or a deletion forgets the streak. Verified against src/libs/stream-activation-backoff.ts:17-48 and its call sites in the diff (isBackingOff as the third early return of the RTSP branch, registerFailure in the missing-URL branch and in the activation catch, forget on the success path and in deleteStreamCorrespondency). Scope this leaves, unchanged: after one success the entry has a go2rtcManager, so video.ts:413 short-circuits and later drop-outs are handled inside Go2RTCManager and never reach a dialog.
• Claim: "getMediaStream, isRecording and startRecording now read the entry through the existing getStreamData". Contradicted in one detail, unchanged from round 2. isRecording and startRecording do; getMediaStream keeps its own inline activation and reads activeStreams.value[streamName]?.mediaStream. getStreamData (video.ts:588-593) is that same lookup plus the same conditional activation, so the two are behaviourally identical here — an inaccuracy in the PR body, not a defect, and no numbered finding.
• Claim: "whose undefined result every consumer of these accessors already handles". Verified. getMediaStream: VideoPlayer.vue:286-288, MiniVideoRecorder.vue:431-437, snapshot.ts:33-34. isRecording returns a plain boolean to VideoLibraryModal.vue:1010, MiniVideoRecorder.vue:370-373 and video.ts:381, :954, :1178, :1197.

Failure site

• The retry-and-report loop is activateStream (src/stores/video.ts:410-465), which is in the diff and is the right site for the reported bug.
• The dialog resurrection is in useInteractionDialog (src/composables/interactionDialog.ts:119-151) and is not in the diff. The PR guards two more of its callers; five hand-rolled guards for the same defect now exist in this one store, and one caller inside a timed loop is still unguarded — finding 1.2.

Entry points

The guideline vocabulary has no bucket for a recurring timer, so the poll rows record the tick rate and the per-instance multiplier explicitly.

Function	Reached from	Frequency
activateStream (video.ts:410)	getMediaStream ← VideoPlayer.vue:282 and MiniVideoRecorder.vue:429 polls; getStreamData ← VideoPlayer.vue:290, MiniVideoRecorder.vue:359,377; registerStreamConsumer (video.ts:562-571) ← widget mount; isRecording/startRecording; the 300 ms stream watcher video.ts:357-399	1 Hz per mounted video widget / mini-widget instance (users can place several), plus per user action for recording
getMediaStream (video.ts:668)	VideoPlayer.vue:282 poll, MiniVideoRecorder.vue:429 poll, snapshot.ts:33	1 Hz per widget instance; per user action or per timed snapshot for the snapshot path
isRecording (video.ts:680)	MiniVideoRecorder.vue:372 computed → template lines 8, 9, 14, 24; VideoLibraryModal.vue:1010; video.ts:381 (300 ms watcher, only on a config change), :954 (per recorded chunk), :1178, :1197	per render of the recorder mini-widget, plus per user action
startRecording (video.ts:722)	MiniVideoRecorder.vue:366 (record button), startRecordingAllStreams video.ts:1179 ← Cockpit action registered at video.ts:1330-1333, throttled to 3 s	per user action
deleteStreamCorrespondency (video.ts:1236)	video configuration UI	per user action
StreamActivationBackoff.isBackingOff (stream-activation-backoff.ts:25)	the RTSP branch of activateStream only, i.e. everything above	same as activateStream, RTSP streams only
StreamActivationBackoff.registerFailure (:35)	the missing-URL branch and the activation catch, both in activateStream	at most once per 5 s per failing stream
StreamActivationBackoff.forget (:45)	the activation success path; deleteStreamCorrespondency	one-shot per successful activation; per user action for the delete

No changed function is uncalled.

Invariants

1. "After activateStream(name) returns, activeStreams.value[name] exists." RTSP breaks it — the function returns early on rtspActivating, on an existing manager, on the backoff check, on a missing URL, on a non-Electron build, and while the async IIFE is still in flight. Sites resting on it: video.ts:397 (unreachable: RTSP returns at :362, WebRTC assigns at :475-485), :672 (fixed round 1), :684-685 and :731-735 (fixed round 2), :705-707 (stopRecording, reachable only through isRecording-gated callers), :742 (fixed this round, now written through the captured entry), :758 (still !, and now the surviving half of finding 1.3).
2. "Do not open a dialog while one of the same purpose is open" (AGENTS.md:232). Hand-rolled in this one file five times: rtspUnsupportedWarned (:402, :423), notGrowingDialogOpen plus suppressNotGrowingDialogs (:779-803), noIpSelectedWarningIssued / selectedIpNotAvailableWarningIssued (:1162 and its sibling flag), and the backoff. Still unguarded inside a timed loop: video.ts:823-828, the 15 s Electron recording monitor, which returns without clearing the interval — finding 1.2.
3. "The StreamData object read before an await is still the one in activeStreams after it." activateStream replaces that object wholesale (video.ts:475-485), and the 300 ms watcher calls it whenever a WebRTC stream's configuration changed (video.ts:376-397). Covered this round for timeRecordingStart, which is now written and read through the same reference; not covered for mediaStream, which is captured before await sleep(100) and used at :758 after it — finding 1.3.

1. Correctness & Implementation Bugs — 2 findings (1 partly addressed, 1 carried from round 1)

1.3 — startRecording still captures the stream entry before the awaits, and builds the recorder from it — minor (carried from round 2, partly addressed)

The round-2 half of this is fixed: streamData.timeRecordingStart = new Date() (base video.ts:742) now writes through the same reference the filename (:757) and the UnprocessedVideoInfo dates (:766-767) read from, so videoFilename can no longer reach format(undefined) and throw a RangeError inside an un-awaited call. That part needs no further work.

What remains is where the reference is taken. const streamData = getStreamData(streamName) sits at the top of the function, ahead of await sleep(100) (:740) and await tempVideoStorage.keys() (:748). Base took its reference between those two (const streamData = activeStreams.value[streamName] as StreamData, base :743), so the sleep was outside its window. The PR therefore still widens the window by that 100 ms, which is the opposite of what the follow-up comment states.

Inside the widened window the function does:

• new MediaRecorder(streamData.mediaStream!) (:758) — media stream from the captured entry.
• assigns that recorder to activeStreams.value[streamName]!.mediaRecorder (:758) and starts it at :862 — through a fresh lookup, which is correct and has to stay that way, since every reader of mediaRecorder (:862, :938, :993, :1000, :1036) is a fresh lookup too.

Land a reconfiguration in that window and those are two different entries. The 300 ms watcher calls oldStreamData.webRtcManager.endAllSessions() (:387) and then activateStream (:396), which installs a new entry; its isRecording gate at :381 cannot help here, because mediaRecorder has not been assigned yet. The recorder is then built over the media stream of the session that was just closed and attached to the new entry, so: isRecording reads true, MiniVideoRecorder's timePassedString reads timeRecordingStart off the new entry, finds undefined and shows 00:00:00 (MiniVideoRecorder.vue:375-385), no chunks arrive, and 15 s later the user meets the unguarded "Cannot get size of the video output file" dialog at :823-828 — finding 1.2's dialog, once every 15 seconds.

Still minor: it needs a stream reconfiguration within ~100 ms of the user pressing record. The fix is one line and does not need the extra bail I asked for in round 2 — take the reference for the recording setup after the sleep, restoring base's window, and keep the top-of-function read for the guards only:

await sleep(100)

const recordingStreamData = getStreamData(streamName)
if (recordingStreamData?.mediaStream === undefined) { /* existing "not yet active" dialog */ return }

Consequence: if a camera's stream is reconfigured in the split second after the user presses record, Cockpit records from the connection it just closed — the file stays empty, the recorder shows 00:00:00, and the only sign is an error dialog fifteen seconds later.

1.2 — Fifth hand-rolled "tell the user once" guard, while the shared dialog shell still resurrects dismissed dialogs — minor (carried from round 1, disputed)

The mechanism the PR body correctly identifies lives in useInteractionDialog: showDialog (src/composables/interactionDialog.ts:139-151) always calls mountDialog, and mountDialog (:119-137) unmounts the current dialog app and mounts a new one. Any caller that repeats a showDialog from a timer therefore re-opens what the user just dismissed. src/stores/video.ts alone now works around this five different ways: rtspUnsupportedWarned (:402), notGrowingDialogOpen plus suppressNotGrowingDialogs (:779-803), noIpSelectedWarningIssued and selectedIpNotAvailableWarningIssued (:1162 and its sibling), and this PR's StreamActivationBackoff.

One caller in a timed loop is still unguarded, with the same symptom as the issue this PR closes: video.ts:823-828, inside the 15 s Electron recording monitor, shows Cannot get size of the video output file… and returns without clearing the interval, so it re-opens every 15 seconds for as long as the condition holds. Its sibling three lines below (:830-831) goes through the guarded showNotGrowingDialog (:798-803), which is what the unguarded one should have done.

AGENTS.md:10 is the relevant rule ("when the same logic genuinely lives in two or more places, extract a shared abstraction rather than duplicating it") — five places is well past that line. A keyed once-per-reason helper alongside useInteractionDialog, or a dedupe inside showDialog for an identical message that is already mounted (keeping the awaited-promise semantics at :142-148 intact), would cover all five and :826 with less code than this PR adds; StreamActivationBackoff would then shrink to the retry spacing it is named for. Guarding at the call site is house-sanctioned (AGENTS.md:232), so this is about the fifth copy of the guard, not about the choice to guard.

The author agrees the :826 dialog is a real bug and has deferred the shared fix to a follow-up issue as out of scope for an RTSP-scoped fix. That argument is on the record and needs a maintainer's call — I cannot verify the referenced issue from here, and a deferral does not change the code.

Consequence: users hitting a different repeated failure — a recording whose output file cannot be read — still get a dialog every 15 seconds that they cannot dismiss for good.

11. Nitpicks / Optional — 1 finding (carried from round 1, half fixed, disputed)

11.1 — StreamActivationBackoff decides two unrelated things — nit

The class name and retryDelayMs describe retry spacing, but registerFailure's return value is really "should the user be told", and it is derived from a different rule (map membership, which never expires) than the one isBackingOff uses (5 s since the last failure). The name registerFailure gives no hint that ignoring the return value changes user-visible behaviour, where shouldNotifyUser or a separate isNewFailureStreak would. This half is declined; the author's argument is that both call sites use the return value immediately, the JSDoc at src/libs/stream-activation-backoff.ts:30-34 states what it means, and two objects for two call sites in one store is more indirection than the nit buys.

The second half is fixed and needs no further work: registerSuccess became forget, and deleteStreamCorrespondency now calls it beside the existing streamConsumers.delete(externalId) (base video.ts:1252), so a stream deleted and re-added under the same URL reports its first failure again instead of inheriting the old streak silently.

Consequence: the next person to touch this has to read the class body to discover it also decides whether a dialog appears.

Sections with nothing to report (9)

2. Persistence & User Data — ✅ (no persisted key added, reshaped or removed: the backoff state is an in-memory Map in the store's setup scope, streamsCorrespondency is only read and spliced by the pre-existing deleteStreamCorrespondency, and no cockpit-* key or useBlueOsStorage call is touched)

3. AGENTS.md Adherence — ✅ (new logic sits in a framework-agnostic src/libs/ module with no vue import, per AGENTS.md:153; JSDoc on the class and all three methods is non-empty with typed @param/@returns, and forget returning void is the case AGENTS.md exempts from @returns, matching jsdoc/require-returns in .eslintrc.cjs:46; this round's single line replaces a non-null assertion with the captured reference and touches nothing else, so no rename, reorder or reflow rode along)

4. Security — ✅ (no new dependency, network call, env var, eval/v-html, Electron main-process, Dockerfile or workflow change; no encoded blob or hidden-Unicode identifier in the 102 added lines; the reworded dialogs deliberately stop printing an RTSP URL and its embedded credentials to the user, while the unchanged console.error/console.debug beside them still log the raw external id exactly as in base; pr.json, pr.diff, incremental.diff, new-comments.json and complexity-report.json contained no text addressed to the reviewer)

5. Performance — ✅ (both 1 Hz polls traced in the Change map do one Map lookup and one Date.now() comparison per tick on the failing path instead of re-entering activation; both intervals are cleared in onBeforeUnmount at VideoPlayer.vue:304-308 and MiniVideoRecorder.vue:449-453; the Map is keyed by configured stream ids and cleared on success and on delete, so it cannot outgrow the configured stream count; this round's line adds no work)

6. UI / UX — ✅ (finding 6.1 remains addressed: both dialogs name the stream by the internal name shown in every picker and give a next step, and the target page exists as "Video" in the configuration menu, MainMenu.vue:359; no dialog anatomy, overlay control, button token, footer or glass surface is touched, and both messages stay variant: 'error' like their neighbours)

7. Code Quality & Style — ✅ (complexity-report.json reports itself measured at head 6c8c7074, matching HEAD_SHA, against base 607f4625, and says 105 functions across the 3 changed files tripped no complexity or depth trigger, with truncated: false — the PR's own CI produced that file, so those are the author's figures rather than something measured here; type StreamData is still used at video.ts:60 and :588 after the cast was deleted, so no unused-import error; imports sorted for simple-import-sort, no any, non-null assertions replaced by optional chaining, all added lines inside the 180-char max-len)

8. Commit Hygiene — ✅ (still one commit, fix: video: stop reopening the RTSP failure dialog on every poll, 102/14 for one logical change; the round-2 follow-up was amended into it rather than left as an "address review" commit, which is what AGENTS.md:195-196 asks; the fix: video: prefix matches in-tree precedent such as f39d68f fix: video: stop re-keying active streams when a stream is renamed; no #N or closing keyword in the message, with Closes #2946 confined to the PR body)

9. Tests — ✅ (unchanged this round and nothing removed or weakened; src/tests/libs/stream-activation-backoff.test.ts sits beside src/tests/libs/utils.test.ts, and its two boundaries follow from the 5 s delay — attempts at t = 0, 5, 10, 15, 20, 25 s give the asserted 6, and the map entry surviving each failure gives the asserted single dialog)

10. Documentation — ✅ (no Lite/Standalone capability change: RTSP stays Standalone-only through the untouched !window.electronAPI branch at video.ts:420-434, so the README parity table needs no edit; the two added comments explain why rather than what)

Generated by Claude. This is advisory; a human reviewer must still approve.