feat: persist & restore screen/mic/webcam selection across sessions#532
feat: persist & restore screen/mic/webcam selection across sessions#532Reedaaz wants to merge 3 commits into
Conversation
feat(recording): persist webcam device + enabled across sessions Extends the existing per-user recording-preferences store (userData recordings-settings.json, never bundled) to also remember the selected webcam device id and webcam-enabled state, mirroring the proven mic persistence pattern: - get/set-recording-preferences handlers + types widened (webcamEnabled, webcamDeviceId, selectedSourceId/Name reserved for screen follow-up). - persistWebcamEnabled / persistWebcamDeviceId in useScreenRecorder, exposed as the public setters so all call sites persist automatically. - Hydrate webcam prefs on startup alongside mic. Screen-source persistence intentionally deferred (main-process, ephemeral desktopCapturer ids — separate follow-up). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> @
…ssions Save selected screen source (id, name, display_id, thumbnail, appIcon, type) to disk and restore it on startup. Fix mic device race where the init sync overwrote the saved deviceId with undefined before real devices enumerated. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
📝 WalkthroughWalkthroughRecording preferences are extended to persist and restore webcam settings and selected desktop source metadata. Type contracts, IPC handlers, and preload wiring add new optional fields. Source selection now updates recording preferences with metadata. The screen recorder hook loads persisted webcam settings and restores selected sources on startup, with new persistence callbacks wiring webcam changes back to preferences. A microphone sync fix prevents clearing device selection with the "default" value. ChangesWebcam and Source Selection Persistence
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
electron/preload.ts (1)
906-914: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winKeep preload
setRecordingPreferencestype in sync with the public renderer contract.This signature still omits
selectedSourceDisplayId,selectedSourceThumbnail,selectedSourceAppIcon, andselectedSourceType, which are exposed inwindow.electronAPItypings.Proposed fix
setRecordingPreferences: (prefs: { microphoneEnabled?: boolean; microphoneDeviceId?: string; systemAudioEnabled?: boolean; webcamEnabled?: boolean; webcamDeviceId?: string; selectedSourceId?: string; selectedSourceName?: string; + selectedSourceDisplayId?: string; + selectedSourceThumbnail?: string | null; + selectedSourceAppIcon?: string | null; + selectedSourceType?: "screen" | "window"; }) => ipcRenderer.invoke("set-recording-preferences", prefs),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/preload.ts` around lines 906 - 914, The preload export setRecordingPreferences currently types prefs without the fields exposed in the public renderer contract; update the prefs parameter type in setRecordingPreferences to include selectedSourceDisplayId, selectedSourceThumbnail, selectedSourceAppIcon, and selectedSourceType so it matches the window.electronAPI typings; adjust the anonymous prefs object signature in the setRecordingPreferences declaration to add those four optional properties (matching names and optionality used elsewhere) so the IPC invoke("set-recording-preferences", prefs) call accepts the full contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@electron/ipc/register/settings.ts`:
- Around line 150-154: The get-recording-preferences IPC handler currently only
returns selectedSourceId/selectedSourceName; update its return object (in the
block that builds the parsed preferences) to also include
selectedSourceDisplayId, selectedSourceThumbnail, selectedSourceAppIcon, and
selectedSourceType so the renderer receives full selected-source metadata;
perform the same typeof/string checks as used for
selectedSourceId/selectedSourceName (e.g., typeof parsed.selectedSourceDisplayId
=== 'string' ? parsed.selectedSourceDisplayId : undefined) and apply the
identical changes to the second similar block around lines 161-165 so both
handlers return the complete set of fields.
- Line 173: The IPC handler "set-recording-preferences" currently types the
prefs parameter without the new fields; update the prefs type in the
ipcMain.handle callback to include selectedSourceDisplayId,
selectedSourceThumbnail, selectedSourceAppIcon, and selectedSourceType (in
addition to the existing microphoneEnabled, microphoneDeviceId,
systemAudioEnabled, webcamEnabled, webcamDeviceId, selectedSourceId,
selectedSourceName) so the handler signature matches the expanded renderer API
contract; locate the handler function named "set-recording-preferences" and
extend its prefs parameter type accordingly and adjust any downstream usage
within that handler to consume the new fields.
In `@src/components/launch/hooks/useLaunchWindowActions.ts`:
- Around line 12-19: The call to window.electronAPI.setRecordingPreferences in
useLaunchWindowActions is fire-and-forget and currently can produce unhandled
promise rejections; update the source selection flow to append a .catch(...) on
the returned promise from setRecordingPreferences (inside the function handling
the source selection) to log the error and any useful diagnostics (e.g.,
source.id/name) using the existing logger or console.error so failures are
observed but the UI remains non-blocking.
---
Outside diff comments:
In `@electron/preload.ts`:
- Around line 906-914: The preload export setRecordingPreferences currently
types prefs without the fields exposed in the public renderer contract; update
the prefs parameter type in setRecordingPreferences to include
selectedSourceDisplayId, selectedSourceThumbnail, selectedSourceAppIcon, and
selectedSourceType so it matches the window.electronAPI typings; adjust the
anonymous prefs object signature in the setRecordingPreferences declaration to
add those four optional properties (matching names and optionality used
elsewhere) so the IPC invoke("set-recording-preferences", prefs) call accepts
the full contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e3c09e4b-4885-4bef-97ee-1af2403927a4
📒 Files selected for processing (6)
electron/electron-env.d.tselectron/ipc/register/settings.tselectron/preload.tssrc/components/launch/LaunchWindow.tsxsrc/components/launch/hooks/useLaunchWindowActions.tssrc/hooks/useScreenRecorder.ts
| webcamEnabled: parsed.webcamEnabled === true, | ||
| webcamDeviceId: typeof parsed.webcamDeviceId === 'string' ? parsed.webcamDeviceId : undefined, | ||
| selectedSourceId: typeof parsed.selectedSourceId === 'string' ? parsed.selectedSourceId : undefined, | ||
| selectedSourceName: typeof parsed.selectedSourceName === 'string' ? parsed.selectedSourceName : undefined, | ||
| } |
There was a problem hiding this comment.
Return full selected-source metadata from get-recording-preferences.
This handler currently restores only selectedSourceId/selectedSourceName, but the renderer restore flow also consumes selectedSourceDisplayId, selectedSourceThumbnail, selectedSourceAppIcon, and selectedSourceType. Missing these forces fallback values during startup restore.
Proposed fix
return {
success: true,
microphoneEnabled: parsed.microphoneEnabled === true,
microphoneDeviceId: typeof parsed.microphoneDeviceId === 'string' ? parsed.microphoneDeviceId : undefined,
systemAudioEnabled: parsed.systemAudioEnabled === true,
webcamEnabled: parsed.webcamEnabled === true,
webcamDeviceId: typeof parsed.webcamDeviceId === 'string' ? parsed.webcamDeviceId : undefined,
selectedSourceId: typeof parsed.selectedSourceId === 'string' ? parsed.selectedSourceId : undefined,
selectedSourceName: typeof parsed.selectedSourceName === 'string' ? parsed.selectedSourceName : undefined,
+ selectedSourceDisplayId:
+ typeof parsed.selectedSourceDisplayId === 'string' ? parsed.selectedSourceDisplayId : undefined,
+ selectedSourceThumbnail:
+ typeof parsed.selectedSourceThumbnail === 'string' || parsed.selectedSourceThumbnail === null
+ ? parsed.selectedSourceThumbnail
+ : undefined,
+ selectedSourceAppIcon:
+ typeof parsed.selectedSourceAppIcon === 'string' || parsed.selectedSourceAppIcon === null
+ ? parsed.selectedSourceAppIcon
+ : undefined,
+ selectedSourceType:
+ parsed.selectedSourceType === 'screen' || parsed.selectedSourceType === 'window'
+ ? parsed.selectedSourceType
+ : undefined,
}
} catch {
return {
success: true,
microphoneEnabled: false,
microphoneDeviceId: undefined,
systemAudioEnabled: false,
webcamEnabled: false,
webcamDeviceId: undefined,
selectedSourceId: undefined,
selectedSourceName: undefined,
+ selectedSourceDisplayId: undefined,
+ selectedSourceThumbnail: undefined,
+ selectedSourceAppIcon: undefined,
+ selectedSourceType: undefined,
}
}Also applies to: 161-165
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/ipc/register/settings.ts` around lines 150 - 154, The
get-recording-preferences IPC handler currently only returns
selectedSourceId/selectedSourceName; update its return object (in the block that
builds the parsed preferences) to also include selectedSourceDisplayId,
selectedSourceThumbnail, selectedSourceAppIcon, and selectedSourceType so the
renderer receives full selected-source metadata; perform the same typeof/string
checks as used for selectedSourceId/selectedSourceName (e.g., typeof
parsed.selectedSourceDisplayId === 'string' ? parsed.selectedSourceDisplayId :
undefined) and apply the identical changes to the second similar block around
lines 161-165 so both handlers return the complete set of fields.
| }) | ||
|
|
||
| ipcMain.handle('set-recording-preferences', async (_, prefs: { microphoneEnabled?: boolean; microphoneDeviceId?: string; systemAudioEnabled?: boolean }) => { | ||
| ipcMain.handle('set-recording-preferences', async (_, prefs: { microphoneEnabled?: boolean; microphoneDeviceId?: string; systemAudioEnabled?: boolean; webcamEnabled?: boolean; webcamDeviceId?: string; selectedSourceId?: string; selectedSourceName?: string }) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Align set-recording-preferences handler typing with the expanded contract.
The prefs type omits selectedSourceDisplayId, selectedSourceThumbnail, selectedSourceAppIcon, and selectedSourceType, which are now part of the renderer API contract.
Proposed fix
- ipcMain.handle('set-recording-preferences', async (_, prefs: { microphoneEnabled?: boolean; microphoneDeviceId?: string; systemAudioEnabled?: boolean; webcamEnabled?: boolean; webcamDeviceId?: string; selectedSourceId?: string; selectedSourceName?: string }) => {
+ ipcMain.handle('set-recording-preferences', async (_, prefs: {
+ microphoneEnabled?: boolean;
+ microphoneDeviceId?: string;
+ systemAudioEnabled?: boolean;
+ webcamEnabled?: boolean;
+ webcamDeviceId?: string;
+ selectedSourceId?: string;
+ selectedSourceName?: string;
+ selectedSourceDisplayId?: string;
+ selectedSourceThumbnail?: string | null;
+ selectedSourceAppIcon?: string | null;
+ selectedSourceType?: "screen" | "window";
+ }) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ipcMain.handle('set-recording-preferences', async (_, prefs: { microphoneEnabled?: boolean; microphoneDeviceId?: string; systemAudioEnabled?: boolean; webcamEnabled?: boolean; webcamDeviceId?: string; selectedSourceId?: string; selectedSourceName?: string }) => { | |
| ipcMain.handle('set-recording-preferences', async (_, prefs: { | |
| microphoneEnabled?: boolean; | |
| microphoneDeviceId?: string; | |
| systemAudioEnabled?: boolean; | |
| webcamEnabled?: boolean; | |
| webcamDeviceId?: string; | |
| selectedSourceId?: string; | |
| selectedSourceName?: string; | |
| selectedSourceDisplayId?: string; | |
| selectedSourceThumbnail?: string | null; | |
| selectedSourceAppIcon?: string | null; | |
| selectedSourceType?: "screen" | "window"; | |
| }) => { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/ipc/register/settings.ts` at line 173, The IPC handler
"set-recording-preferences" currently types the prefs parameter without the new
fields; update the prefs type in the ipcMain.handle callback to include
selectedSourceDisplayId, selectedSourceThumbnail, selectedSourceAppIcon, and
selectedSourceType (in addition to the existing microphoneEnabled,
microphoneDeviceId, systemAudioEnabled, webcamEnabled, webcamDeviceId,
selectedSourceId, selectedSourceName) so the handler signature matches the
expanded renderer API contract; locate the handler function named
"set-recording-preferences" and extend its prefs parameter type accordingly and
adjust any downstream usage within that handler to consume the new fields.
| void window.electronAPI.setRecordingPreferences({ | ||
| selectedSourceId: source.id, | ||
| selectedSourceName: source.name, | ||
| selectedSourceDisplayId: source.display_id, | ||
| selectedSourceThumbnail: source.thumbnail, | ||
| selectedSourceAppIcon: source.appIcon, | ||
| selectedSourceType: source.sourceType, | ||
| }); |
There was a problem hiding this comment.
Handle rejected preference writes in source selection flow.
The persistence call is intentionally not awaited, but it should still be .catch(...)-handled to avoid unhandled rejections and silent loss of diagnostics.
Proposed fix
- void window.electronAPI.setRecordingPreferences({
+ void window.electronAPI
+ .setRecordingPreferences({
selectedSourceId: source.id,
selectedSourceName: source.name,
selectedSourceDisplayId: source.display_id,
selectedSourceThumbnail: source.thumbnail,
selectedSourceAppIcon: source.appIcon,
selectedSourceType: source.sourceType,
- });
+ })
+ .catch((error) => {
+ console.warn("Failed to persist selected source preferences:", error);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void window.electronAPI.setRecordingPreferences({ | |
| selectedSourceId: source.id, | |
| selectedSourceName: source.name, | |
| selectedSourceDisplayId: source.display_id, | |
| selectedSourceThumbnail: source.thumbnail, | |
| selectedSourceAppIcon: source.appIcon, | |
| selectedSourceType: source.sourceType, | |
| }); | |
| void window.electronAPI | |
| .setRecordingPreferences({ | |
| selectedSourceId: source.id, | |
| selectedSourceName: source.name, | |
| selectedSourceDisplayId: source.display_id, | |
| selectedSourceThumbnail: source.thumbnail, | |
| selectedSourceAppIcon: source.appIcon, | |
| selectedSourceType: source.sourceType, | |
| }) | |
| .catch((error) => { | |
| console.warn("Failed to persist selected source preferences:", error); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/launch/hooks/useLaunchWindowActions.ts` around lines 12 - 19,
The call to window.electronAPI.setRecordingPreferences in useLaunchWindowActions
is fire-and-forget and currently can produce unhandled promise rejections;
update the source selection flow to append a .catch(...) on the returned promise
from setRecordingPreferences (inside the function handling the source selection)
to log the error and any useful diagnostics (e.g., source.id/name) using the
existing logger or console.error so failures are observed but the UI remains
non-blocking.
Electron desktopCapturer ids are not stable across reboots / display changes. Restore the saved source only if it still exists in the current source list (re-enumerated live), otherwise the app would silently capture a non-existent target. Drops the redundant persisted display_id/thumbnail/appIcon/type fields since the live source object carries them. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/launch/hooks/useLaunchWindowActions.ts (1)
12-15:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle rejected preference writes in the non-blocking persistence path.
This IPC write is intentionally fire-and-forget, but it should still be rejection-handled so persistence failures don’t go silent.
Proposed fix
- void window.electronAPI.setRecordingPreferences({ - selectedSourceId: source.id, - selectedSourceName: source.name, - }); + void window.electronAPI + .setRecordingPreferences({ + selectedSourceId: source.id, + selectedSourceName: source.name, + }) + .catch((error) => { + console.warn("Failed to persist selected source preferences:", { + error, + sourceId: source.id, + sourceName: source.name, + }); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/launch/hooks/useLaunchWindowActions.ts` around lines 12 - 15, The fire-and-forget IPC call to window.electronAPI.setRecordingPreferences can reject and currently fails silently; modify the call in useLaunchWindowActions (the setRecordingPreferences invocation) to attach a rejection handler (e.g. append .catch(...)) that logs the error (using the existing logger if available or console.error) and optionally surfaces a debug message, ensuring the call remains non-blocking but persistence failures are not silent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks/useScreenRecorder.ts`:
- Line 1220: The fire-and-forget IPC calls using window.electronAPI.selectSource
(and the other non-awaited electronAPI calls around lines 1245-1253) lack
rejection handling; update each invocation to append a .catch(...) that logs the
error (e.g., via console.error or the existing logger) and includes contextual
info (function name/selected source) so rejections surface. Locate usages of
window.electronAPI.selectSource and the similar electronAPI calls and add
.catch(err => /* log with context */) to each call.
---
Duplicate comments:
In `@src/components/launch/hooks/useLaunchWindowActions.ts`:
- Around line 12-15: The fire-and-forget IPC call to
window.electronAPI.setRecordingPreferences can reject and currently fails
silently; modify the call in useLaunchWindowActions (the setRecordingPreferences
invocation) to attach a rejection handler (e.g. append .catch(...)) that logs
the error (using the existing logger if available or console.error) and
optionally surfaces a debug message, ensuring the call remains non-blocking but
persistence failures are not silent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cfc93fc0-ec74-4c22-a49c-e2909d405fc9
📒 Files selected for processing (3)
electron/electron-env.d.tssrc/components/launch/hooks/useLaunchWindowActions.tssrc/hooks/useScreenRecorder.ts
| (source) => source.id === result.selectedSourceId, | ||
| ); | ||
| if (match) { | ||
| void window.electronAPI.selectSource(match); |
There was a problem hiding this comment.
Add rejection handling for new fire-and-forget IPC calls.
These new non-awaited calls can reject without diagnostics. Please attach .catch(...) handlers.
Proposed fix
- void window.electronAPI.selectSource(match);
+ void window.electronAPI.selectSource(match).catch((error) => {
+ console.warn("Failed to restore persisted capture source:", {
+ error,
+ sourceId: match.id,
+ });
+ });
const persistWebcamEnabled = useCallback((enabled: boolean) => {
setWebcamEnabled(enabled);
- void window.electronAPI.setRecordingPreferences({ webcamEnabled: enabled });
+ void window.electronAPI
+ .setRecordingPreferences({ webcamEnabled: enabled })
+ .catch((error) => {
+ console.warn("Failed to persist webcamEnabled preference:", error);
+ });
}, []);
const persistWebcamDeviceId = useCallback((deviceId: string | undefined) => {
setWebcamDeviceId(deviceId);
- void window.electronAPI.setRecordingPreferences({ webcamDeviceId: deviceId });
+ void window.electronAPI
+ .setRecordingPreferences({ webcamDeviceId: deviceId })
+ .catch((error) => {
+ console.warn("Failed to persist webcamDeviceId preference:", error);
+ });
}, []);Also applies to: 1245-1253
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/useScreenRecorder.ts` at line 1220, The fire-and-forget IPC calls
using window.electronAPI.selectSource (and the other non-awaited electronAPI
calls around lines 1245-1253) lack rejection handling; update each invocation to
append a .catch(...) that logs the error (e.g., via console.error or the
existing logger) and includes contextual info (function name/selected source) so
rejections surface. Locate usages of window.electronAPI.selectSource and the
similar electronAPI calls and add .catch(err => /* log with context */) to each
call.
What this adds
Persist and restore the last selected screen source, microphone, and webcam across app restarts. Today every launch starts from scratch and you re-pick all three devices before recording — annoying when you record often with the same setup.
Why
Pure quality-of-life. The settings file already had
selectedSourceId/selectedSourceNamefields that were never written; this finishes that wiring and fixes the mic side.What changed
desktopCapturerids are not stable across reboots / display changes, so we only re-select the saved source if it still exists, otherwise we skip silently instead of capturing a dead target. The existingonSelectedSourceChangedbroadcast syncs the HUD.undefinedbefore the real devices enumerated. Guard added so"default"no longer clobbers a persisted device.Design note
An earlier revision persisted the full source object (display_id/thumbnail/appIcon/type). I dropped that on review: the live-enumeration approach is strictly more robust (a stale persisted blob would happily restore a source that no longer exists), and the live source object already carries those fields. So this PR persists only the stable id+name and rehydrates the rest from the live list.
Testing
tsc --noEmitclean.Independent of #531 / #533. Targets
v1.3.0-beta.3.Context
Built with Claude Code as a tool (maintainers know I work this way); design and validation are mine.