Fix 'Record all streams' joystick button intermittent failure

[ArturoManzoli]

• Primary issue was that recordStream() would exit early if a WebRTC stream was not attached at the exact moment the "record all streams" button was pressed.

• Additionally, startRecordingAllStreams() is an async function that was being called without an await (fire-and-forget). This caused it to incorrectly add dropped streams to streamsThatStarted and leave isRecordingAllStreams set to true.

--

• startRecording() now waits for the WebRTC mediaStream to be ready before proceeding, and then returns a boolean indicating success.

• startRecordingAllStreams() now awaits the results for each stream, only counts the streams that successfully started, and reverts isRecordingAllStreams to false if none did.

Fix #2688

[github-actions]

Automated PR Review (Claude)

0. Summary

Verdict: MINOR SUGGESTIONS

Minor items to consider: 1.1, 1.2, 6.1

This PR fixes an intermittent failure when using the joystick button to record all streams. The root cause was that startRecording() would bail out if the WebRTC media stream wasn't attached at the exact moment recording was triggered, and startRecordingAllStreams() was fire-and-forget (sync calling async without await), causing it to incorrectly count streams as started. The fix adds a polling loop (up to 5 s) in startRecording() to wait for the media stream, changes the return type to Promise<boolean>, and makes startRecordingAllStreams() properly async with Promise.all to await all results. It also correctly resets isRecordingAllStreams to false when no streams started.

---

1. Correctness & Implementation Bugs

1.1 (minor) — toggleRecordingAllStreams still fire-and-forgets the now-async startRecordingAllStreams.

src/stores/video.ts:1084-1090 (base ref): toggleRecordingAllStreams has signature (): void and calls startRecordingAllStreams() without await. While the internal logic of startRecordingAllStreams is now correct (it awaits its own work), there is a race-window: if the user triggers the toggle action again after the 3 s throttle expires but before the 5 s polling timeout completes, isRecordingAllStreams.value is already true, so stopRecordingAllStreams fires. But since recording hasn't actually started yet (still polling), isRecording(streamName) returns false for every stream, producing the "No streams were being recorded" error alert — leaving isRecordingAllStreams as false while recordings may start moments later with no toggle-aware flag tracking them.

This is an edge-case, and the 3 s throttle mitigates it for most users, but it could be addressed by making toggleRecordingAllStreams async or by guarding with a "starting" state.

1.2 (minor) — Multiple error dialogs when several streams time out.

When startRecordingAllStreams calls startRecording for N streams concurrently via Promise.all, each timed-out stream will independently call showDialog(...) with "Media stream not defined." This could produce a rapid cascade of modal error dialogs. Consider aggregating failures and showing a single summary dialog, or suppressing per-stream dialogs when called from the "all streams" path.

---

2. AGENTS.md Adherence

No findings.

The PR correctly:

• Uses optional chaining (?.) on the new polling checks (diff line 27-28).
• Uses sleep from existing @/libs/utils rather than introducing a new dependency.
• Updates the JSDoc @returns tag with the new return type.
• Comments explain "why" (the WebRTC readiness window), not "what".

---

3. Security

3.1 No obfuscated or intentionally unreadable code.

3.2 No base64/hex/encoded blobs or binary-like strings.

3.3 No hidden Unicode, zero-width characters, or homoglyph attacks.

3.4 No unexpected network calls or exfiltration patterns. The change only adds a local polling loop using sleep.

3.5 No changes to build scripts, CI workflows, Dockerfiles, or Electron main-process code.

3.6 No changes to secret handling, CORS/CSP, eval, Function(), v-html, or similar.

3.7 No new dependencies added.

3.8 No other suspicious patterns detected.

No findings.

---

4. Performance

No findings.

The polling loop (while with sleep(100) up to 5 s) is reasonable. It only runs when recording is initiated and is bounded. Promise.all for concurrent stream startup is efficient.

---

5. UI / UX

No findings beyond what is noted in 1.2 (potential dialog cascade, which was pre-existing behavior).

---

6. Code Quality & Style

6.1 (nit) — Inconsistent use of !. vs ?. on adjacent lines.

In the diff, the "Media stream not defined" guard (diff context line ~35) is changed from activeStreams.value[streamName]!.mediaStream to activeStreams.value[streamName]?.mediaStream (good). However, the very next guard at diff context line ~40 retains activeStreams.value[streamName]!.mediaStream!.active. While this is safe at runtime (guarded by the previous check), it is inconsistent with the AGENTS.md rule to "Use optional chaining (?.) when possible in typescript." Consider activeStreams.value[streamName]?.mediaStream?.active for consistency, since the falsy short-circuit would behave identically here.

---

7. Tests

No findings.

There are no existing unit tests for startRecording or startRecordingAllStreams in the base ref, so no test coverage is being weakened. The logic change is well-suited for integration/E2E testing, but the absence of new unit tests is consistent with the current test coverage pattern.

---

8. Documentation

No findings.

The in-code JSDoc is updated appropriately with the new @returns annotation. No README changes are needed as this is a bug fix with no feature-parity implications.

---

9. Nitpicks / Optional

No additional nitpicks beyond 6.1.

---

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

[rafaellehmkuhl]

The startRecording function is an action and so it should succeed or fail. It it succeed, it should return early (undefined), as it was doing. If it fails it should throw.

Returning booleans on action functions is an anti-pattern from languages were throws didn't exist and one couldn't properly stack an error.

[ArturoManzoli]

Fixed

[rafaellehmkuhl]

Problem still happening here, only took 5 seconds to appear (as opposed to master where it appeared immediately):

Logs:

Cockpit (May 22, 2026 - 07꞉23꞉08 GMT-3).syslog.zip

[rafaellehmkuhl]

@ArturoManzoli I'm planning to add this one to the v.1.18.3 patch release.

[ArturoManzoli]

@ArturoManzoli I'm planning to add this one to the v.1.18.3 patch release.

I'm not sure if the same problem that happens in your local machine is happening here.
I wasn't able to reproduce exactly what was described on your screenshot/logs.
Even though, I've improved the fix to overcome the problem as you described. Please, test it again on your local setup.