diff --git a/AGENTS.md b/AGENTS.md index aac3c4f..8029222 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,13 +62,16 @@ When adding new code: ## Test-First Requirements -Before implementing behavior changes: +Always define expected behavior and acceptance criteria before implementing. Treat "what behavior changed?" as a required design question before coding. -- Add unit tests for pure logic. -- Add integration tests for filesystem, IPC, or service coordination. -- Add or extend e2e coverage for critical user flows if the change affects runtime behavior. -- For every user-visible behavior change, add at least one proving test that would fail without the change. -- Treat "what behavior changed?" as a required design question before coding. +How to prove the behavior depends on the layer — apply tests-first where it earns its keep, and be honest where it cannot: + +- Pure logic and main-process services (`src/shared/**`, `src/main/services/**`): write the proving unit/integration tests first. These paths test cheaply against temp directories and fakes, and this is where tests-first pays off most. +- Filesystem, IPC, or service coordination: add integration tests alongside the change. +- OS-, device-, or `MediaRecorder`-level behavior (real disk-full, device disconnect, app quit ordering, capture edge cases): unit tests with fakes are NOT sufficient proof. Add e2e/smoke coverage where feasible, and document explicit manual verification steps for what automation cannot reach. Never claim durability or capture reliability is proven from helper-only tests. +- Renderer UI glue (banners, overlays, status text): prefer behavior-level e2e/smoke checks or documented manual verification over brittle DOM-assertion unit tests written first. Extract decision logic into a pure helper and unit test that instead. + +For every user-visible behavior change, add at least one proving test that would fail without the change — at the most honest layer available, not merely the most convenient one. Minimum expectation by change type: diff --git a/docs/production/feature-inventory.md b/docs/production/feature-inventory.md index 6564b99..75c9d39 100644 --- a/docs/production/feature-inventory.md +++ b/docs/production/feature-inventory.md @@ -119,6 +119,22 @@ Acceptance criteria: - Recovered take is not duplicated if already present. - Recovery file is cleared on successful append completion. +### D2. Quit guard while recording + +- Closing the window mid-recording is intercepted; the renderer prompts + "Recording in progress — stop and save before quitting?". +- On confirm, the recording is stopped and finalized to disk before the + window actually closes; on cancel, recording continues. + +Acceptance criteria: + +- Window close is prevented only while a recording is active (renderer keeps + main informed via `recording:set-active`). +- A stop/finalize failure still allows the close (bytes remain in `.part` + files and are recoverable on next launch). +- If the renderer is unreachable, the close proceeds instead of wedging the + window open. + ## E. Timeline Editor ### E1. Enter/exit timeline diff --git a/src/main.ts b/src/main.ts index 65f9139..e1976df 100644 --- a/src/main.ts +++ b/src/main.ts @@ -11,6 +11,7 @@ import { systemPreferences } from 'electron'; +import { createCloseGuard } from './main/app/close-guard'; import { createWindow } from './main/app/create-window'; import { registerDisplayMediaHandler, @@ -30,6 +31,10 @@ let win: BrowserWindow | null = null; const projectService = createProjectService({ app }); +// Quit guard: prevents closing the window mid-recording until the renderer +// stops/finalizes (or the user confirms the close anyway). +const closeGuard = createCloseGuard(); + registerIpcHandlers({ ipcMain, app, @@ -38,6 +43,7 @@ registerIpcHandlers({ shell, systemPreferences, getWindow: () => win, + closeGuard, projectService, renderComposite, exportPremiereProject, @@ -52,12 +58,16 @@ registerIpcHandlers({ function createMainWindow(): void { win = createWindow({ BrowserWindow, + closeGuard, onConsoleMessage: ({ level, message, line, sourceId }) => { console.log(`[renderer:${level}] ${message} (${sourceId}:${line})`); } }); win.on('closed', () => { win = null; + // The renderer that owned the recording flag is gone; reset so a window + // reopened via app 'activate' does not inherit a stale guard state. + closeGuard.setRecordingActive(false); }); } @@ -69,6 +79,17 @@ app.whenReady().then(() => { createMainWindow(); }); +app.on('before-quit', () => { + // Shutdown safety: flush and close any recording fds still open so their + // .part files survive intact on disk for orphan recovery on next launch. + // Best-effort and synchronous; never throws (and must never block quit). + try { + recordingService.closeAllRecordingHandlesForShutdown(); + } catch (error) { + console.warn('[recording] shutdown flush failed:', error); + } +}); + app.on('window-all-closed', () => { if (process.platform !== 'darwin') app.quit(); }); diff --git a/src/main/app/close-guard.ts b/src/main/app/close-guard.ts new file mode 100644 index 0000000..62534d5 --- /dev/null +++ b/src/main/app/close-guard.ts @@ -0,0 +1,53 @@ +/** + * Close guard: pure decision state for "can the window close right now?". + * + * The renderer keeps main informed via the fire-and-forget + * `recording:set-active` IPC channel whenever recording starts/stops, so the + * BrowserWindow 'close' handler can decide synchronously (no renderer + * round-trip) whether to intercept the close. + * + * When a close is intercepted, the renderer is asked (via + * `app:close-requested`) to stop/finalize the recording and then confirm the + * real close with `app:confirm-close`, which arms a one-shot bypass before + * `win.close()` re-fires the 'close' event. + */ + +export type CloseRequestDecision = 'allow' | 'prevent'; + +export interface CloseGuard { + /** Renderer-driven recording state; coerced to a strict boolean. */ + setRecordingActive(active: boolean): void; + isRecordingActive(): boolean; + /** + * Arm a one-shot bypass so the next close request is allowed even if the + * recording flag is still set (e.g. stopRecording threw — the bytes are in + * the .part file and recoverable, so the user must never be trapped). + */ + confirmClose(): void; + /** Decide the fate of a window 'close' event. Consumes the bypass. */ + handleCloseRequest(): CloseRequestDecision; +} + +export function createCloseGuard(): CloseGuard { + let recordingActive = false; + let closeConfirmed = false; + + return { + setRecordingActive(active: boolean): void { + recordingActive = Boolean(active); + }, + isRecordingActive(): boolean { + return recordingActive; + }, + confirmClose(): void { + closeConfirmed = true; + }, + handleCloseRequest(): CloseRequestDecision { + if (closeConfirmed) { + closeConfirmed = false; + return 'allow'; + } + return recordingActive ? 'prevent' : 'allow'; + } + }; +} diff --git a/src/main/app/create-window.ts b/src/main/app/create-window.ts index 254a7bf..8e240c8 100644 --- a/src/main/app/create-window.ts +++ b/src/main/app/create-window.ts @@ -6,6 +6,15 @@ import type { Event } from 'electron'; +import type { CloseGuard } from './close-guard'; + +/** + * Sent to the renderer when a window close was intercepted because a + * recording is active. The renderer prompts, stops/finalizes, and then calls + * `app:confirm-close` to trigger the real close. + */ +export const CLOSE_REQUESTED_CHANNEL = 'app:close-requested'; + interface ConsoleMessagePayload { event: Event; level: number; @@ -21,11 +30,13 @@ export type BrowserWindowConstructor = new ( export function createWindow({ BrowserWindow, onConsoleMessage, - appRootDir = path.join(__dirname, '..', '..') + appRootDir = path.join(__dirname, '..', '..'), + closeGuard }: { BrowserWindow: BrowserWindowConstructor; onConsoleMessage?: (payload: ConsoleMessagePayload) => void; appRootDir?: string; + closeGuard?: CloseGuard; }): ElectronBrowserWindow { const win = new BrowserWindow({ width: 960, @@ -72,6 +83,30 @@ export function createWindow({ } }); + // Quit guard: while a recording is active (renderer keeps the guard + // informed via 'recording:set-active'), intercept the close and ask the + // renderer to stop + finalize first. The renderer confirms the real close + // via 'app:confirm-close', which arms a one-shot bypass on the guard. + if (closeGuard) { + win.on('close', (event) => { + if (closeGuard.handleCloseRequest() !== 'prevent') return; + + // If the renderer is already gone there is nobody left to finalize and + // confirm; the streamed bytes are safe in the .part files (recovered on + // next launch), so let the close proceed instead of wedging the window. + if (win.webContents.isDestroyed()) return; + + event.preventDefault(); + try { + win.webContents.send(CLOSE_REQUESTED_CHANNEL); + } catch (error) { + console.warn('close-requested notification failed; closing anyway:', error); + closeGuard.confirmClose(); + win.close(); + } + }); + } + win.loadFile(path.join(appRootDir, 'index.html')); return win; } diff --git a/src/main/ipc/register-handlers.ts b/src/main/ipc/register-handlers.ts index d322265..960c0bb 100644 --- a/src/main/ipc/register-handlers.ts +++ b/src/main/ipc/register-handlers.ts @@ -13,6 +13,7 @@ import type { SystemPreferences } from 'electron'; +import type { CloseGuard } from '../app/close-guard'; import type { createProjectService } from '../services/project-service'; import type { renderComposite } from '../services/render-service'; import type { exportPremiereProject } from '../services/premiere-export-service'; @@ -44,6 +45,7 @@ export function registerIpcHandlers({ shell, systemPreferences, getWindow, + closeGuard, projectService, renderComposite, exportPremiereProject, @@ -61,6 +63,7 @@ export function registerIpcHandlers({ shell: Shell; systemPreferences?: Pick; getWindow: () => BrowserWindow | null; + closeGuard: CloseGuard; projectService: ProjectService; renderComposite: RenderComposite; exportPremiereProject: ExportPremiereProject; @@ -437,6 +440,24 @@ export function registerIpcHandlers({ // renderer crash or timeout never drops captured bytes. Each recorder // (screen, camera) opens an independent write handle keyed by // (takeId, suffix) and finalizes with an atomic rename. + // Quit-guard bookkeeping: the renderer flips this flag on recording + // start/stop (fire-and-forget) so the window 'close' handler can decide + // synchronously in main whether a recording is at risk. + ipcMain.on('recording:set-active', (_event, active: unknown) => { + closeGuard.setRecordingActive(Boolean(active)); + }); + + // The renderer calls this after the close prompt (recording stopped and + // finalized, or stop failed and the user still wants out). Arms the + // one-shot bypass BEFORE close() so the guard lets the close through. + ipcMain.handle('app:confirm-close', async () => { + closeGuard.confirmClose(); + const win = getWindow(); + if (!win || win.isDestroyed()) return false; + win.close(); + return true; + }); + ipcMain.handle('recording:begin', async (_event, payload: unknown) => { const opts = (payload || {}) as { takeId?: string; diff --git a/src/main/services/recording-service.ts b/src/main/services/recording-service.ts index 6b37706..9361dbe 100644 --- a/src/main/services/recording-service.ts +++ b/src/main/services/recording-service.ts @@ -47,6 +47,12 @@ export interface RecordingHandle { fd: number; bytesWritten: number; closed: boolean; + /** Timestamp (durability.now()) of the last periodic fsync attempt. */ + lastFsyncAt: number; + /** True while an async periodic fsync is in flight for this fd. */ + fsyncInFlight: boolean; + /** First periodic fsync failure, recorded once and surfaced at finalize. */ + fsyncWarning: string | null; } export interface BeginRecordingResult { @@ -57,6 +63,13 @@ export interface BeginRecordingResult { export interface FinalizeRecordingResult { path: string; bytesWritten: number; + /** + * Present when captured bytes were saved but durability could not be fully + * guaranteed (an fsync failed at finalize or during recording). Callers + * should surface this prominently instead of treating the save as fully + * durable. + */ + warning?: string; } export interface AppendChunkResult { @@ -65,6 +78,43 @@ export interface AppendChunkResult { const handles = new Map(); +/** + * Durability knobs, injectable so tests can drive periodic fsync behavior + * deterministically without real clocks or real fsync failures. + */ +export interface RecordingDurabilityConfig { + /** Minimum gap between periodic fsyncs while a recording is appending. */ + fsyncIntervalMs: number; + now: () => number; + fsyncAsync: (fd: number) => Promise; + fsyncSync: (fd: number) => void; +} + +function defaultFsyncAsync(fd: number): Promise { + return new Promise((resolve, reject) => { + fs.fsync(fd, (err) => (err ? reject(err) : resolve())); + }); +} + +const DEFAULT_DURABILITY_CONFIG: RecordingDurabilityConfig = { + fsyncIntervalMs: 5000, + now: () => Date.now(), + fsyncAsync: defaultFsyncAsync, + fsyncSync: (fd) => fs.fsyncSync(fd) +}; + +let durability: RecordingDurabilityConfig = { ...DEFAULT_DURABILITY_CONFIG }; + +export function configureRecordingDurability( + overrides: Partial +): void { + durability = { ...durability, ...overrides }; +} + +export function resetRecordingDurabilityConfig(): void { + durability = { ...DEFAULT_DURABILITY_CONFIG }; +} + function handleKey(takeId: string, suffix: string): string { return `${takeId}::${suffix}`; } @@ -96,6 +146,24 @@ export function computeRecordingPaths( return { tempPath, finalPath }; } +/** + * Never rename over an existing final recording. If the deterministic final + * path is already occupied (crash -> recover orphan -> record again with the + * same takeId), pick the next free "-2", "-3", ... variant so the previous + * bytes survive. + */ +function resolveNonClobberingFinalPath(finalPath: string): string { + if (!fs.existsSync(finalPath)) return finalPath; + const dir = path.dirname(finalPath); + const ext = path.extname(finalPath); + const base = path.basename(finalPath, ext); + for (let n = 2; n < 10_000; n += 1) { + const candidate = path.join(dir, `${base}-${n}${ext}`); + if (!fs.existsSync(candidate)) return candidate; + } + throw new Error(`Could not find a non-conflicting final name for ${finalPath}`); +} + function toBuffer(data: Buffer | Uint8Array | ArrayBuffer): Buffer { if (Buffer.isBuffer(data)) return data; if (data instanceof Uint8Array) return Buffer.from(data.buffer, data.byteOffset, data.byteLength); @@ -132,7 +200,10 @@ export function beginRecording(opts: BeginRecordingOptions): BeginRecordingResul finalPath, fd, bytesWritten: 0, - closed: false + closed: false, + lastFsyncAt: durability.now(), + fsyncInFlight: false, + fsyncWarning: null }); return { tempPath, finalPath }; } @@ -155,9 +226,36 @@ export async function appendRecordingChunk(opts: AppendChunkOptions): Promise { + if (!handle.fsyncWarning) { + const message = error instanceof Error ? error.message : String(error); + handle.fsyncWarning = `Periodic fsync failed during recording: ${message}`; + console.warn(`[recording] periodic fsync failed for ${key}:`, error); + } + }) + .finally(() => { + handle.fsyncInFlight = false; + }); +} + export function finalizeRecording(opts: FinalizeRecordingOptions): FinalizeRecordingResult { const takeId = typeof opts.takeId === 'string' ? opts.takeId.trim() : ''; const suffix = typeof opts.suffix === 'string' ? opts.suffix.trim() : ''; @@ -170,12 +268,20 @@ export function finalizeRecording(opts: FinalizeRecordingOptions): FinalizeRecor return { path: handle.finalPath, bytesWritten: handle.bytesWritten }; } + const warnings: string[] = []; + if (handle.fsyncWarning) warnings.push(handle.fsyncWarning); + try { // Flush buffers to disk before rename so a crash immediately after the - // rename cannot leave us with a zero-length final file. + // rename cannot leave us with a zero-length final file. If the fsync + // fails we still proceed with the rename — bytes under a final name beat + // a stuck .part — but the durability gap must not be silently swallowed, + // so it is surfaced to the caller as a warning. try { - fs.fsyncSync(handle.fd); + durability.fsyncSync(handle.fd); } catch (error) { + const message = error instanceof Error ? error.message : String(error); + warnings.push(`fsync failed at finalize; bytes may not be fully flushed: ${message}`); console.warn(`[recording] fsync failed for ${key}:`, error); } fs.closeSync(handle.fd); @@ -193,11 +299,14 @@ export function finalizeRecording(opts: FinalizeRecordingOptions): FinalizeRecor throw new Error(`Recording produced no data for ${suffix}`); } - fs.renameSync(handle.tempPath, handle.finalPath); + const finalPath = resolveNonClobberingFinalPath(handle.finalPath); + fs.renameSync(handle.tempPath, finalPath); + handle.finalPath = finalPath; const result: FinalizeRecordingResult = { - path: handle.finalPath, + path: finalPath, bytesWritten: handle.bytesWritten }; + if (warnings.length > 0) result.warning = warnings.join('; '); handles.delete(key); return result; } @@ -376,7 +485,12 @@ export function recoverOrphanRecording( const entry = candidate[suffix]; if (!entry) continue; if (!fs.existsSync(entry.partPath)) continue; - const { finalPath } = computeRecordingPaths(folder, takeId, suffix); + // Never rename over an existing final file: a previous recovery (or a new + // recording reusing the same takeId) may already own the deterministic + // name, and recovering an orphan must not destroy it. + const finalPath = resolveNonClobberingFinalPath( + computeRecordingPaths(folder, takeId, suffix).finalPath + ); try { fs.renameSync(entry.partPath, finalPath); if (suffix === 'screen') recovered.screenPath = finalPath; @@ -418,6 +532,29 @@ export function discardOrphanRecording( return { discarded }; } +/** + * Shutdown safety: flush and close every open recording fd, leaving the .part + * files on disk so orphan recovery can pick them up on next launch. Wired to + * Electron's `before-quit`; synchronous, best-effort, and never throws — a + * failing fsync/close on one handle must not block the others (or the quit). + */ +export function closeAllRecordingHandlesForShutdown(): void { + for (const [key, handle] of handles) { + if (handle.closed) continue; + try { + durability.fsyncSync(handle.fd); + } catch (error) { + console.warn(`[recording] shutdown fsync failed for ${key}:`, error); + } + try { + fs.closeSync(handle.fd); + } catch (error) { + console.warn(`[recording] shutdown close failed for ${key}:`, error); + } + handle.closed = true; + } +} + /** * Reset service state. Test-only helper; in production, finalize/cancel own * every handle. @@ -434,4 +571,5 @@ export function _resetForTests(): void { safeUnlink(handle.tempPath); } handles.clear(); + resetRecordingDurabilityConfig(); } diff --git a/src/preload.ts b/src/preload.ts index e332957..a85cced 100644 --- a/src/preload.ts +++ b/src/preload.ts @@ -78,6 +78,16 @@ const electronApi: ElectronApi = { recordingAppend: (opts) => ipcRenderer.invoke('recording:append', opts), recordingFinalize: (opts) => ipcRenderer.invoke('recording:finalize', opts), recordingCancel: (opts) => ipcRenderer.invoke('recording:cancel', opts), + recordingSetActive: (active) => { + ipcRenderer.send('recording:set-active', Boolean(active)); + }, + confirmClose: () => ipcRenderer.invoke('app:confirm-close'), + onCloseRequested: (listener) => { + if (typeof listener !== 'function') return () => {}; + const handler = () => listener(); + ipcRenderer.on('app:close-requested', handler); + return () => ipcRenderer.removeListener('app:close-requested', handler); + }, recordingListOrphans: (folder) => ipcRenderer.invoke('recording:list-orphans', folder), recordingScanOrphans: (folder) => ipcRenderer.invoke('recording:scan-orphans', folder), recordingRecoverOrphan: (opts) => ipcRenderer.invoke('recording:recover-orphan', opts), diff --git a/src/renderer/app.ts b/src/renderer/app.ts index aae11a6..945e286 100644 --- a/src/renderer/app.ts +++ b/src/renderer/app.ts @@ -34,15 +34,19 @@ import { import { getTakePlaybackSources } from './features/timeline/take-playback-sources'; import { getWaveformDecodeSources } from './features/timeline/waveform-sources'; import { + buildMicrophoneConstraints, + classifyRecorderFailure, finalizeStreamedRecording, getRecorderFinalizeTimeoutMs, getRecorderOptions, getRecorderTimesliceMs, + isOverconstrainedError, shouldRenderPreviewFrame, createAudioOnlyRecordingStream, createCameraRecordingStream, createScreenRecordingStream } from './features/recording/recorder-utils'; +import { CLOSE_PROMPT_MESSAGE, getCloseRequestedAction } from './features/recording/close-request'; import { resolveTakeAudio } from '../shared/domain/take-audio'; import { drawMirroredImage, getCenteredSquareCropRect } from './features/camera/camera-render'; import { cleanupAllMedia } from './features/media-cleanup'; @@ -143,6 +147,13 @@ let cameraStream = null; let audioStream = null; let recorders = []; let recording = false; +// Guards stopRecording against re-entry: a mid-capture failure may auto-stop +// while the user also presses Stop (or a track-ended monitor fires). Only the +// first caller runs the finalize flow. +let stopRecordingInFlight = false; +// True while the "System audio unavailable" notice is showing so a later +// successful loopback capture can clear it instead of leaving a stale warning. +let systemAudioFallbackNoticeShown = false; let pendingRecordingTakeId = null; // System-audio metering + activity detection. We sample the screen stream's // audio tracks during recording and emit "keep" segments (aligned with the @@ -200,8 +211,23 @@ function clearMediaIdleTimer() { mediaIdleTimer = null; } +/** + * Keep the main-process quit guard in sync with the renderer's recording + * state. Fire-and-forget: a failure to notify must never affect the + * recording pipeline itself. + */ +function notifyRecordingActive(active) { + if (typeof window.electronAPI.recordingSetActive !== 'function') return; + try { + window.electronAPI.recordingSetActive(Boolean(active)); + } catch (error) { + console.warn('Failed to sync recording-active state to main:', error); + } +} + function resetMediaRefsAfterCleanup() { recording = false; + notifyRecordingActive(false); screenStream = null; cameraStream = null; audioStream = null; @@ -656,11 +682,6 @@ function cleanupVideoPool() { activePlaybackSection = null; } -function invalidateTakeWaveformCache(takeId) { - takeAudioBufferCache.delete(takeId); - takeSystemAudioBufferCache.delete(takeId); -} - function resolveTimeToSource(timelineTime) { const section = findSectionForTime(timelineTime); if (!section) return null; @@ -2361,11 +2382,24 @@ async function updateScreenStream() { video: true, audio: true }); + // Loopback worked this time: retire any stale "system audio + // unavailable" warning from a previous attempt so the UI reflects the + // stream the user will actually record. + if (systemAudioFallbackNoticeShown) { + systemAudioFallbackNoticeShown = false; + if (!recording) { + setTranscriptStatus('', 'neutral'); + transcriptPanel?.classList.add('hidden'); + } + } } catch (error) { console.warn( '[Recorder] System audio capture failed; falling back to video-only screen stream:', error ); + const decision = classifyRecorderFailure('system-audio', 'screen'); + showRecordingNotice(decision.userMessage, 'warning'); + systemAudioFallbackNoticeShown = true; screenStream = await navigator.mediaDevices.getUserMedia({ audio: false, video: { @@ -2403,11 +2437,14 @@ async function updateCameraStream() { const deviceId = cameraSelect.value; if (!deviceId) return; + // Allow up to 4K camera capture. Frame rate deliberately stays capped at + // 30fps: camera WebM is VFR and this app has a documented history of + // camera/audio sync drift, so higher frame rates risk regressing sync. cameraStream = await navigator.mediaDevices.getUserMedia({ video: { deviceId: { exact: deviceId }, - width: { ideal: 1920, max: 1920 }, - height: { ideal: 1080, max: 1080 }, + width: { ideal: 3840, max: 3840 }, + height: { ideal: 2160, max: 2160 }, frameRate: { ideal: 30, max: 30 }, aspectRatio: { ideal: 16 / 9 } }, @@ -2431,10 +2468,24 @@ async function updateAudioStream() { const deviceId = audioSelect.value; if (!deviceId) return; - audioStream = await navigator.mediaDevices.getUserMedia({ - audio: { deviceId: { exact: deviceId } }, - video: false - }); + try { + audioStream = await navigator.mediaDevices.getUserMedia({ + audio: buildMicrophoneConstraints(deviceId), + video: false + }); + } catch (err) { + // Capture beats quality: never fail a recording because a device cannot + // satisfy the quality constraints. Retry once with only the device id. + if (!isOverconstrainedError(err)) throw err; + console.warn( + '[Recorder] Microphone quality constraints rejected (OverconstrainedError) — retrying with device id only.', + err + ); + audioStream = await navigator.mediaDevices.getUserMedia({ + audio: buildMicrophoneConstraints(deviceId, { includeQualityConstraints: false }), + video: false + }); + } startAudioMeter(audioStream); } @@ -2710,9 +2761,17 @@ async function createRecorder(stream, suffix, takeId) { // surfaced on finalize. let appendChain = Promise.resolve(); + // Feed the actual captured dimensions (from track.getSettings()) into the + // options so the camera bitrate scales with what the device delivered + // (12/20/30 Mbps for <=1080p/1440p/4K). Screen bitrate is fixed and + // ignores these. + const [videoTrack] = typeof stream?.getVideoTracks === 'function' ? stream.getVideoTracks() : []; + const videoSettings = videoTrack?.getSettings?.() || {}; const recorderOptions = getRecorderOptions({ suffix, - hasAudio: typeof stream?.getAudioTracks === 'function' && stream.getAudioTracks().length > 0 + hasAudio: typeof stream?.getAudioTracks === 'function' && stream.getAudioTracks().length > 0, + videoWidth: videoSettings.width, + videoHeight: videoSettings.height }); // Open the on-disk write handle BEFORE starting the recorder. If this fails @@ -2760,17 +2819,24 @@ async function createRecorder(stream, suffix, takeId) { bytesWritten += buffer.byteLength; } } catch (err) { - if (!recorderError) { + // Only the FIRST failure drives the user-facing reaction: once an + // append fails every later chunk is also lost, so surface it and + // (for critical recorders) auto-stop instead of waiting for Stop. + const firstFailure = !recorderError; + if (firstFailure) { recorderError = err instanceof Error ? err.message : String(err); } console.error(`[Recorder] ${suffix} chunk append failed:`, err); + if (firstFailure) handleRecorderFailure('append', suffix); } }); }; recorder.onerror = (event) => { + const firstFailure = !recorderError; recorderError = event?.error?.message || `${suffix} recorder failed`; console.error(`[Recorder] ${suffix} error`, event?.error || event); + if (firstFailure) handleRecorderFailure('recorder-error', suffix); }; // blobPromise resolves with { path, error, suffix, bytesWritten } after the @@ -2895,6 +2961,37 @@ function applyScribeStatus(status) { setTranscriptStatus(status.text, status.tone); } +/** + * Surface a recording notice on the existing transcript status line. The + * status line lives inside the transcript panel (hidden outside recording), + * so un-hide the panel when a notice must be visible before recording starts + * (e.g. the system-audio fallback during source selection). + */ +function showRecordingNotice(text, tone = 'warning') { + const hasText = typeof text === 'string' && text.trim().length > 0; + if (hasText && transcriptPanel) transcriptPanel.classList.remove('hidden'); + setTranscriptStatus(text, tone); +} + +/** + * React to a mid-capture recorder failure: show a visible, non-blocking + * notice immediately and, for critical recorders (screen/mic), auto-stop the + * whole recording exactly once so everything already streamed to disk is + * finalized instead of silently recording into a black hole. + */ +function handleRecorderFailure(kind, suffix) { + const decision = classifyRecorderFailure(kind, suffix); + showRecordingNotice(decision.userMessage, decision.shouldAutoStop ? 'error' : 'warning'); + if (decision.shouldAutoStop && recording && !stopRecordingInFlight) { + console.error( + `[Recorder] ${suffix} ${kind} failure, auto-stopping to preserve what is on disk` + ); + stopRecording().catch((err) => { + console.error('[Recorder] Auto-stop after recorder failure crashed:', err); + }); + } +} + async function startRecording() { if (!activeProjectPath) return; recorders = []; @@ -3034,6 +3131,7 @@ async function startRecording() { const recorderTimesliceMs = getRecorderTimesliceMs(); recorders.forEach((r) => r.start(recorderTimesliceMs)); recording = true; + notifyRecordingActive(true); updateWorkspaceHeader(); recordBtn.textContent = 'Stop'; recordBtn.classList.replace('bg-red-600', 'bg-neutral-700'); @@ -3897,6 +3995,18 @@ function appendTakeToTimeline({ } async function stopRecording() { + // Stop may be requested twice: a mid-capture failure auto-stops while the + // user clicks Stop (or a critical track ends). Run the finalize flow once. + if (stopRecordingInFlight) return; + stopRecordingInFlight = true; + try { + await stopRecordingImpl(); + } finally { + stopRecordingInFlight = false; + } +} + +async function stopRecordingImpl() { const projectSession = getActiveProjectSession(); const recordedDuration = (Date.now() - startTime) / 1000; clearInterval(timerInterval); @@ -3982,6 +4092,7 @@ async function stopRecording() { recorders = []; recording = false; + notifyRecordingActive(false); updateWorkspaceHeader(); recordBtn.textContent = 'Record'; recordBtn.classList.replace('bg-neutral-700', 'bg-red-600'); @@ -6001,3 +6112,56 @@ window.addEventListener('beforeunload', () => { console.warn('Failed to flush project save on exit:', error); }); }); + +// Quit guard: main intercepts the window close while a recording is active +// and asks us to stop + save first. On confirm, stop/finalize the recording, +// then trigger the real close; on cancel, keep recording. If stopping fails, +// still allow the close — every streamed chunk is already in the .part files +// and recoverable on next launch, so the user must never be trapped. +let closeRequestInFlight = false; + +async function requestConfirmedClose() { + if (typeof window.electronAPI.confirmClose !== 'function') return; + try { + await window.electronAPI.confirmClose(); + } catch (error) { + console.error('Failed to confirm window close:', error); + } +} + +async function handleCloseRequested() { + if (closeRequestInFlight) return; + closeRequestInFlight = true; + try { + const action = getCloseRequestedAction({ + recording, + hasActiveRecorders: hasActiveRecorders() + }); + if (action === 'close-immediately') { + // Main's flag was stale (recording already fully stopped); nothing to + // save, so let the close proceed without prompting. + await requestConfirmedClose(); + return; + } + + if (!window.confirm(CLOSE_PROMPT_MESSAGE)) return; + + try { + await stopRecording(); + } catch (error) { + console.error( + 'Failed to stop recording before close; captured bytes remain recoverable in the .part files:', + error + ); + } + await requestConfirmedClose(); + } finally { + closeRequestInFlight = false; + } +} + +if (typeof window.electronAPI.onCloseRequested === 'function') { + window.electronAPI.onCloseRequested(() => { + handleCloseRequested(); + }); +} diff --git a/src/renderer/features/recording/close-request.ts b/src/renderer/features/recording/close-request.ts new file mode 100644 index 0000000..5eb22f7 --- /dev/null +++ b/src/renderer/features/recording/close-request.ts @@ -0,0 +1,23 @@ +/** + * Decision logic for the main-process "window close requested" event. + * + * Main only asks the renderer when its recording flag is set, but the + * renderer re-checks its own live state: the flag can be stale (recording + * finished a beat earlier) or recorders can still be draining/finalizing + * after `recording` flipped false. + */ + +export const CLOSE_PROMPT_MESSAGE = + 'Recording in progress — stop and save before quitting?'; + +export type CloseRequestedAction = 'close-immediately' | 'confirm-stop-then-close'; + +export function getCloseRequestedAction({ + recording, + hasActiveRecorders +}: { + recording: boolean; + hasActiveRecorders: boolean; +}): CloseRequestedAction { + return recording || hasActiveRecorders ? 'confirm-stop-then-close' : 'close-immediately'; +} diff --git a/src/renderer/features/recording/recorder-utils.ts b/src/renderer/features/recording/recorder-utils.ts index 6c7a013..d95595d 100644 --- a/src/renderer/features/recording/recorder-utils.ts +++ b/src/renderer/features/recording/recorder-utils.ts @@ -1,7 +1,9 @@ +// Ordered by preference: VP9 gives noticeably better quality-per-bit than +// VP8 at the bitrates we record at, so try it first and step down. export const RECORDER_MIME_CANDIDATES = [ + 'video/webm; codecs=vp9', 'video/webm; codecs=vp8', - 'video/webm', - 'video/webm; codecs=vp9' + 'video/webm' ] as const; export const RECORDER_TIMESLICE_MS = 1000; @@ -25,29 +27,64 @@ export interface FinalizedRecordingResult { path: string | null; suffix: string; bytesWritten: number; + /** Durability warning from the main process (e.g. fsync failed); the file + * was still saved, but the caller should surface the degraded guarantee. */ + warning?: string; } export function getSupportedRecorderMimeType( mediaRecorderCtor: MediaRecorderCtorLike | undefined = globalThis.MediaRecorder ): string { if (!mediaRecorderCtor || typeof mediaRecorderCtor.isTypeSupported !== 'function') { + console.warn( + '[Recorder] MediaRecorder.isTypeSupported is unavailable — recording with the browser default codec.' + ); return ''; } - return ( - RECORDER_MIME_CANDIDATES.find((mimeType) => mediaRecorderCtor.isTypeSupported?.(mimeType)) || '' + const supported = RECORDER_MIME_CANDIDATES.find((mimeType) => + mediaRecorderCtor.isTypeSupported?.(mimeType) ); + if (!supported) { + console.warn( + '[Recorder] No preferred WebM codec (vp9/vp8/webm) is supported — recording with the browser default codec.' + ); + return ''; + } + return supported; +} + +// Camera bitrate tiers by captured resolution. The camera opens at up to 4K +// now, so the bitrate has to scale with what the device actually delivered +// (read from track.getSettings() after getUserMedia) or 4K footage would be +// starved at a 1080p-era bitrate. +export const CAMERA_VIDEO_BITS_PER_SECOND_4K = 30_000_000; +export const CAMERA_VIDEO_BITS_PER_SECOND_1440P = 20_000_000; +export const CAMERA_VIDEO_BITS_PER_SECOND_DEFAULT = 12_000_000; + +export function computeCameraVideoBitsPerSecond(width?: unknown, height?: unknown): number { + const w = typeof width === 'number' && Number.isFinite(width) && width > 0 ? width : 0; + const h = typeof height === 'number' && Number.isFinite(height) && height > 0 ? height : 0; + + if (w >= 3840 && h >= 2160) return CAMERA_VIDEO_BITS_PER_SECOND_4K; + if (w >= 2560 && h >= 1440) return CAMERA_VIDEO_BITS_PER_SECOND_1440P; + return CAMERA_VIDEO_BITS_PER_SECOND_DEFAULT; } export function getRecorderOptions( - { suffix, hasAudio = true }: { suffix?: string; hasAudio?: boolean } = {}, + { + suffix, + hasAudio = true, + videoWidth, + videoHeight + }: { suffix?: string; hasAudio?: boolean; videoWidth?: number; videoHeight?: number } = {}, mediaRecorderCtor: MediaRecorderCtorLike | undefined = globalThis.MediaRecorder ): MediaRecorderOptions { const mimeType = getSupportedRecorderMimeType(mediaRecorderCtor); const options: MediaRecorderOptions = mimeType ? { mimeType } : {}; if (suffix === 'camera') { - options.videoBitsPerSecond = 10000000; + options.videoBitsPerSecond = computeCameraVideoBitsPerSecond(videoWidth, videoHeight); if (hasAudio) options.audioBitsPerSecond = 192000; } else if (suffix === 'screen') { options.videoBitsPerSecond = 30000000; @@ -61,6 +98,40 @@ export function getRecorderOptions( return options; } +/** + * Constraints for opening the microphone. Quality knobs (sample rate, + * channel count, disabled processing) use `ideal`/plain values rather than + * `exact` so a constrained device can still open; only the device id itself + * is exact. The device-id-only fallback exists because capture beats + * quality: if a device rejects the quality constraints with + * OverconstrainedError, retry with just the id instead of losing the mic. + */ +export function buildMicrophoneConstraints( + deviceId: string, + { includeQualityConstraints = true }: { includeQualityConstraints?: boolean } = {} +): MediaTrackConstraints { + if (!includeQualityConstraints) { + return { deviceId: { exact: deviceId } }; + } + + return { + deviceId: { exact: deviceId }, + sampleRate: { ideal: 48000 }, + channelCount: { ideal: 2 }, + echoCancellation: false, + noiseSuppression: false, + autoGainControl: false + }; +} + +export function isOverconstrainedError(error: unknown): boolean { + return ( + typeof error === 'object' && + error !== null && + (error as { name?: unknown }).name === 'OverconstrainedError' + ); +} + export function getRecorderTimesliceMs(): number { return RECORDER_TIMESLICE_MS; } @@ -139,11 +210,69 @@ export function createScreenRecordingStream( return new MediaStreamCtor([...videoTracks, ...screenAudioTracks, ...extraAudioTracks]); } +export type RecorderFailureKind = 'append' | 'recorder-error' | 'system-audio'; +export type RecorderFailureSuffix = 'screen' | 'camera' | 'audio'; + +export interface RecorderFailureDecision { + shouldAutoStop: boolean; + userMessage: string; +} + +const RECORDER_SUFFIX_LABELS: Record = { + screen: 'Screen', + camera: 'Camera', + audio: 'Microphone' +}; + +/** + * Decide how the recording UI should react to a mid-capture failure. + * + * Policy (mirrors the critical/non-critical track policy used for + * track-ended monitoring): + * - screen or dedicated mic recorder failing to append (disk/IPC) or hitting + * an encoder error means every later chunk is also lost, so auto-stop the + * whole recording to finalize what is already on disk. + * - camera-only failure keeps the recording going: partial success must still + * save the screen file, so just warn. + * - system-audio loopback fallback never stops anything; it only informs that + * the screen is being recorded without desktop audio. + */ +export function classifyRecorderFailure( + kind: RecorderFailureKind, + suffix: RecorderFailureSuffix +): RecorderFailureDecision { + if (kind === 'system-audio') { + return { + shouldAutoStop: false, + userMessage: 'System audio unavailable — recording screen without it' + }; + } + + if (suffix === 'camera') { + return { + shouldAutoStop: false, + userMessage: + kind === 'append' + ? 'Camera recording can no longer save to disk — continuing with screen only.' + : 'Camera recorder failed — continuing with screen only.' + }; + } + + const label = RECORDER_SUFFIX_LABELS[suffix] || suffix; + return { + shouldAutoStop: true, + userMessage: + kind === 'append' + ? `${label} recording can no longer save to disk — stopping to keep what has been saved.` + : `${label} recorder failed — stopping to keep what has been saved.` + }; +} + export interface FinalizeStreamedRecordingDeps { finalize: (opts: { takeId: string; suffix: string; - }) => Promise<{ path: string; bytesWritten: number }>; + }) => Promise<{ path: string; bytesWritten: number; warning?: string }>; cancel?: (opts: { takeId: string; suffix: string }) => Promise<{ cancelled: boolean }>; } @@ -184,11 +313,20 @@ export async function finalizeStreamedRecording({ if (!result?.path) { throw new Error(`${suffix} recording could not be saved`); } + if (result.warning) { + // Durability warning from the main process: the bytes were saved, but + // an fsync failed somewhere along the way. Log loudly so degraded + // durability is never silently swallowed, and carry it on the result. + console.error( + `[Recorder] DURABILITY WARNING for ${suffix} recording at ${result.path}: ${result.warning}` + ); + } return { error: null, path: result.path, suffix, - bytesWritten: result.bytesWritten ?? bytesWritten + bytesWritten: result.bytesWritten ?? bytesWritten, + ...(result.warning ? { warning: result.warning } : {}) }; } catch (error) { return { diff --git a/src/renderer/features/timeline/take-playback-sources.ts b/src/renderer/features/timeline/take-playback-sources.ts index 6f51104..63871a3 100644 --- a/src/renderer/features/timeline/take-playback-sources.ts +++ b/src/renderer/features/timeline/take-playback-sources.ts @@ -11,7 +11,7 @@ function normalizePath(value: unknown): string | null { export function getTakePlaybackSources( take: - | Pick + | Partial> | null | undefined ): TakePlaybackSources { diff --git a/src/shared/electron-api.ts b/src/shared/electron-api.ts index 518d231..1cfa51a 100644 --- a/src/shared/electron-api.ts +++ b/src/shared/electron-api.ts @@ -61,6 +61,12 @@ export interface RecordingFinalizeOpts { export interface RecordingFinalizeResult { path: string; bytesWritten: number; + /** + * Present when the recording was saved but an fsync failed (at finalize or + * during recording), so full durability could not be guaranteed. Callers + * should surface this prominently. + */ + warning?: string; } export interface RecordingCancelResult { @@ -162,6 +168,22 @@ export interface ElectronApi { recordingAppend: (opts: RecordingAppendOpts) => Promise; recordingFinalize: (opts: RecordingFinalizeOpts) => Promise; recordingCancel: (opts: RecordingFinalizeOpts) => Promise; + /** + * Fire-and-forget notification that a recording started/stopped, so main + * can guard window close synchronously (no renderer round-trip). + */ + recordingSetActive: (active: boolean) => void; + /** + * Trigger the real window close after a guarded close was confirmed + * (recording stopped/finalized, or the user chose to close anyway). + * Resolves true when a live window was told to close. + */ + confirmClose: () => Promise; + /** + * Fired by main when a window close was intercepted mid-recording; the + * renderer should prompt, stop/finalize, then call confirmClose(). + */ + onCloseRequested: (listener: () => void) => () => void; recordingListOrphans: (folder: string) => Promise; recordingScanOrphans: (folder: string) => Promise; recordingRecoverOrphan: (opts: { diff --git a/tests/integration/recording-service.test.ts b/tests/integration/recording-service.test.ts index 03ac79f..ff995f2 100644 --- a/tests/integration/recording-service.test.ts +++ b/tests/integration/recording-service.test.ts @@ -9,15 +9,24 @@ import { appendRecordingChunk, beginRecording, cancelRecording, + closeAllRecordingHandlesForShutdown, computeRecordingPaths, + configureRecordingDurability, discardOrphanRecording, finalizeRecording, findOrphanRecordingParts, getActiveRecordingCount, + listActiveRecordings, recoverOrphanRecording, scanOrphanRecordings } from '../../src/main/services/recording-service'; +/** Let fire-and-forget promise chains (periodic fsync) settle. */ +async function flushAsyncWork(): Promise { + await new Promise((resolve) => setImmediate(resolve)); + await new Promise((resolve) => setImmediate(resolve)); +} + function createSandbox() { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'video-recording-service-')); return { @@ -241,6 +250,240 @@ describe('main/services/recording-service', () => { expect(recoverOrphanRecording(sandbox.root, 'take-does-not-exist')).toBeNull(); }); + test('finalize never renames over an existing final recording file', async () => { + const { finalPath } = computeRecordingPaths(sandbox.root, 'take-clobber', 'screen'); + // Simulate a previous recording (e.g. recovered after a crash) already + // occupying the deterministic final path for this takeId. + fs.writeFileSync(finalPath, 'previous-take-bytes'); + + beginRecording({ takeId: 'take-clobber', suffix: 'screen', folder: sandbox.root }); + await appendRecordingChunk({ + takeId: 'take-clobber', + suffix: 'screen', + data: Buffer.from('new-take-bytes') + }); + const result = finalizeRecording({ takeId: 'take-clobber', suffix: 'screen' }); + + expect(result.path).toBe(path.join(sandbox.root, 'recording-take-clobber-screen-2.webm')); + // Both recordings survive with their own bytes. + expect(fs.readFileSync(finalPath, 'utf8')).toBe('previous-take-bytes'); + expect(fs.readFileSync(result.path, 'utf8')).toBe('new-take-bytes'); + }); + + test('finalize keeps uniquifying (-3, ...) when earlier suffixes are taken', async () => { + const { finalPath } = computeRecordingPaths(sandbox.root, 'take-clobber2', 'screen'); + const secondPath = path.join(sandbox.root, 'recording-take-clobber2-screen-2.webm'); + fs.writeFileSync(finalPath, 'first'); + fs.writeFileSync(secondPath, 'second'); + + beginRecording({ takeId: 'take-clobber2', suffix: 'screen', folder: sandbox.root }); + await appendRecordingChunk({ + takeId: 'take-clobber2', + suffix: 'screen', + data: Buffer.from('third') + }); + const result = finalizeRecording({ takeId: 'take-clobber2', suffix: 'screen' }); + + expect(result.path).toBe(path.join(sandbox.root, 'recording-take-clobber2-screen-3.webm')); + expect(fs.readFileSync(finalPath, 'utf8')).toBe('first'); + expect(fs.readFileSync(secondPath, 'utf8')).toBe('second'); + expect(fs.readFileSync(result.path, 'utf8')).toBe('third'); + }); + + test('recoverOrphanRecording never renames over an existing final recording file', () => { + const existingFinal = path.join( + sandbox.root, + 'recording-take-1700000000000-screen.webm' + ); + fs.writeFileSync(existingFinal, 'already-recovered-bytes'); + const screenPart = path.join( + sandbox.root, + '.recording-take-1700000000000-screen-aaaaaa.webm.part' + ); + fs.writeFileSync(screenPart, 'orphan-screen-bytes'); + + const result = recoverOrphanRecording(sandbox.root, 'take-1700000000000'); + expect(result).not.toBeNull(); + expect(result!.screenPath).toBe( + path.join(sandbox.root, 'recording-take-1700000000000-screen-2.webm') + ); + // Both the pre-existing recording and the recovered orphan survive. + expect(fs.readFileSync(existingFinal, 'utf8')).toBe('already-recovered-bytes'); + expect(fs.readFileSync(result!.screenPath!, 'utf8')).toBe('orphan-screen-bytes'); + expect(fs.existsSync(screenPart)).toBe(false); + }); + + test('finalize surfaces a durability warning when fsync fails but still saves the file', async () => { + configureRecordingDurability({ + fsyncSync: () => { + throw new Error('EIO: fsync exploded'); + } + }); + + beginRecording({ takeId: 'take-fsync', suffix: 'screen', folder: sandbox.root }); + await appendRecordingChunk({ + takeId: 'take-fsync', + suffix: 'screen', + data: Buffer.from('bytes-we-must-keep') + }); + const result = finalizeRecording({ takeId: 'take-fsync', suffix: 'screen' }); + + // The rename still happens: bytes under a final name beat a stuck .part. + expect(fs.readFileSync(result.path, 'utf8')).toBe('bytes-we-must-keep'); + expect(result.bytesWritten).toBe('bytes-we-must-keep'.length); + expect(result.warning).toMatch(/fsync/i); + }); + + test('finalize reports no warning when fsync succeeds', async () => { + beginRecording({ takeId: 'take-ok', suffix: 'screen', folder: sandbox.root }); + await appendRecordingChunk({ + takeId: 'take-ok', + suffix: 'screen', + data: Buffer.from('ok') + }); + const result = finalizeRecording({ takeId: 'take-ok', suffix: 'screen' }); + expect(result.warning).toBeUndefined(); + }); + + test('appendRecordingChunk fsyncs periodically based on the configured interval', async () => { + let nowMs = 0; + const fsyncCalls: number[] = []; + configureRecordingDurability({ + fsyncIntervalMs: 5000, + now: () => nowMs, + fsyncAsync: async (fd) => { + fsyncCalls.push(fd); + } + }); + + beginRecording({ takeId: 'take-periodic', suffix: 'screen', folder: sandbox.root }); + const fd = listActiveRecordings()[0].fd; + + nowMs = 1000; + await appendRecordingChunk({ + takeId: 'take-periodic', + suffix: 'screen', + data: Buffer.from('a') + }); + await flushAsyncWork(); + expect(fsyncCalls).toHaveLength(0); + + nowMs = 6000; + await appendRecordingChunk({ + takeId: 'take-periodic', + suffix: 'screen', + data: Buffer.from('b') + }); + await flushAsyncWork(); + expect(fsyncCalls).toEqual([fd]); + + // Within the interval of the last fsync: no additional fsync. + nowMs = 7000; + await appendRecordingChunk({ + takeId: 'take-periodic', + suffix: 'screen', + data: Buffer.from('c') + }); + await flushAsyncWork(); + expect(fsyncCalls).toHaveLength(1); + + // Past the interval again: a second fsync fires. + nowMs = 12000; + await appendRecordingChunk({ + takeId: 'take-periodic', + suffix: 'screen', + data: Buffer.from('d') + }); + await flushAsyncWork(); + expect(fsyncCalls).toEqual([fd, fd]); + + const result = finalizeRecording({ takeId: 'take-periodic', suffix: 'screen' }); + expect(fs.readFileSync(result.path, 'utf8')).toBe('abcd'); + }); + + test('a failed periodic fsync is reported once and does not kill the recording', async () => { + let nowMs = 0; + let fsyncAttempts = 0; + configureRecordingDurability({ + fsyncIntervalMs: 5000, + now: () => nowMs, + fsyncAsync: async () => { + fsyncAttempts += 1; + throw new Error('ENOSPC: no space left on device'); + } + }); + + beginRecording({ takeId: 'take-badfsync', suffix: 'screen', folder: sandbox.root }); + + nowMs = 6000; + await appendRecordingChunk({ + takeId: 'take-badfsync', + suffix: 'screen', + data: Buffer.from('one') + }); + await flushAsyncWork(); + + // Appends keep working after the failed fsync. + nowMs = 12000; + await appendRecordingChunk({ + takeId: 'take-badfsync', + suffix: 'screen', + data: Buffer.from('two') + }); + await flushAsyncWork(); + expect(fsyncAttempts).toBeGreaterThanOrEqual(1); + + const result = finalizeRecording({ takeId: 'take-badfsync', suffix: 'screen' }); + expect(fs.readFileSync(result.path, 'utf8')).toBe('onetwo'); + expect(result.warning).toMatch(/fsync/i); + // Reported once: the warning does not repeat itself per failed attempt. + const mentions = result.warning!.split(/periodic fsync/i).length - 1; + expect(mentions).toBe(1); + }); + + test('closeAllRecordingHandlesForShutdown fsyncs, closes fds, and leaves .part files recoverable', async () => { + beginRecording({ + takeId: 'take-1700000000000', + suffix: 'screen', + folder: sandbox.root + }); + await appendRecordingChunk({ + takeId: 'take-1700000000000', + suffix: 'screen', + data: Buffer.from('shutdown-safe-bytes') + }); + const handle = listActiveRecordings()[0]; + const { fd, tempPath } = handle; + + expect(() => closeAllRecordingHandlesForShutdown()).not.toThrow(); + + // The fd is really closed... + expect(() => fs.fstatSync(fd)).toThrow(); + expect(handle.closed).toBe(true); + // ...and the .part file stays on disk with every byte for recovery. + expect(fs.existsSync(tempPath)).toBe(true); + expect(fs.readFileSync(tempPath, 'utf8')).toBe('shutdown-safe-bytes'); + + // Appends after shutdown fail loudly instead of writing to a dead fd. + await expect( + appendRecordingChunk({ + takeId: 'take-1700000000000', + suffix: 'screen', + data: Buffer.from('late') + }) + ).rejects.toThrow(/finalized|closed/i); + + // Orphan recovery sees the .part file exactly like a post-crash scan. + const candidates = scanOrphanRecordings(sandbox.root); + expect(candidates).toHaveLength(1); + expect(candidates[0].takeId).toBe('take-1700000000000'); + expect(candidates[0].screen?.partPath).toBe(tempPath); + expect(candidates[0].screen?.bytes).toBe('shutdown-safe-bytes'.length); + + // Best-effort and idempotent: calling again never throws. + expect(() => closeAllRecordingHandlesForShutdown()).not.toThrow(); + }); + test('discardOrphanRecording removes every .part file for a takeId', () => { const screenPart = path.join( sandbox.root, diff --git a/tests/unit/close-guard.test.ts b/tests/unit/close-guard.test.ts new file mode 100644 index 0000000..43681b9 --- /dev/null +++ b/tests/unit/close-guard.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, test } from 'vitest'; + +import { createCloseGuard } from '../../src/main/app/close-guard'; + +describe('main/app/close-guard', () => { + test('allows close when no recording is active', () => { + const guard = createCloseGuard(); + + expect(guard.isRecordingActive()).toBe(false); + expect(guard.handleCloseRequest()).toBe('allow'); + }); + + test('prevents close while a recording is active', () => { + const guard = createCloseGuard(); + + guard.setRecordingActive(true); + + expect(guard.isRecordingActive()).toBe(true); + expect(guard.handleCloseRequest()).toBe('prevent'); + // The decision is repeatable: a prevented close does not mutate the flag. + expect(guard.handleCloseRequest()).toBe('prevent'); + }); + + test('allows close again after recording stops', () => { + const guard = createCloseGuard(); + + guard.setRecordingActive(true); + guard.setRecordingActive(false); + + expect(guard.isRecordingActive()).toBe(false); + expect(guard.handleCloseRequest()).toBe('allow'); + }); + + test('coerces non-boolean recording state to a boolean', () => { + const guard = createCloseGuard(); + + guard.setRecordingActive(1 as unknown as boolean); + expect(guard.isRecordingActive()).toBe(true); + + guard.setRecordingActive(0 as unknown as boolean); + expect(guard.isRecordingActive()).toBe(false); + }); + + test('confirmClose bypasses the guard exactly once, even while recording', () => { + const guard = createCloseGuard(); + + guard.setRecordingActive(true); + guard.confirmClose(); + + // The confirmed close goes through even though the recording flag is + // still set (resilience: if stopRecording failed, the user can still + // close — bytes are in the .part file and recoverable). + expect(guard.handleCloseRequest()).toBe('allow'); + // The bypass is one-shot: a later close request is guarded again. + expect(guard.handleCloseRequest()).toBe('prevent'); + }); + + test('confirmClose on an idle guard does not leave a stale bypass behind', () => { + const guard = createCloseGuard(); + + guard.confirmClose(); + expect(guard.handleCloseRequest()).toBe('allow'); + + guard.setRecordingActive(true); + expect(guard.handleCloseRequest()).toBe('prevent'); + }); +}); diff --git a/tests/unit/close-request.test.ts b/tests/unit/close-request.test.ts new file mode 100644 index 0000000..849aea5 --- /dev/null +++ b/tests/unit/close-request.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, test } from 'vitest'; + +import { + CLOSE_PROMPT_MESSAGE, + getCloseRequestedAction +} from '../../src/renderer/features/recording/close-request'; + +describe('renderer/features/recording/close-request', () => { + test('requests confirm-stop-then-close while recording', () => { + expect(getCloseRequestedAction({ recording: true, hasActiveRecorders: true })).toBe( + 'confirm-stop-then-close' + ); + }); + + test('requests confirm-stop-then-close when recorders are still draining', () => { + // recording flips false at the end of stop, but recorders can still be + // finalizing; the close must wait for them too. + expect(getCloseRequestedAction({ recording: false, hasActiveRecorders: true })).toBe( + 'confirm-stop-then-close' + ); + }); + + test('closes immediately when nothing is recording (stale main-side flag)', () => { + expect(getCloseRequestedAction({ recording: false, hasActiveRecorders: false })).toBe( + 'close-immediately' + ); + }); + + test('prompt copy explains that stopping saves the recording', () => { + expect(CLOSE_PROMPT_MESSAGE).toMatch(/recording in progress/i); + expect(CLOSE_PROMPT_MESSAGE).toMatch(/stop and save/i); + }); +}); diff --git a/tests/unit/create-window.test.ts b/tests/unit/create-window.test.ts index 34dc94a..6510e43 100644 --- a/tests/unit/create-window.test.ts +++ b/tests/unit/create-window.test.ts @@ -2,7 +2,51 @@ import path from 'node:path'; import { describe, expect, test, vi } from 'vitest'; -import { createWindow, type BrowserWindowConstructor } from '../../src/main/app/create-window'; +import { + CLOSE_REQUESTED_CHANNEL, + createWindow, + type BrowserWindowConstructor +} from '../../src/main/app/create-window'; +import type { CloseGuard } from '../../src/main/app/close-guard'; + +type CloseHandler = (event: { preventDefault: () => void }) => void; + +function createCloseGuardStub(decision: 'allow' | 'prevent') { + const guard = { + setRecordingActive: vi.fn((_active: boolean) => {}), + isRecordingActive: vi.fn(() => decision === 'prevent'), + confirmClose: vi.fn(() => {}), + handleCloseRequest: vi.fn(() => decision) + }; + return guard satisfies CloseGuard; +} + +function createGuardedWindow(closeGuard: CloseGuard, webContentsOverrides: object = {}) { + const closeHandlers: CloseHandler[] = []; + const webContents = { + on: vi.fn(), + setWindowOpenHandler: vi.fn(), + send: vi.fn(), + isDestroyed: vi.fn(() => false), + ...webContentsOverrides + }; + const browserWindowInstance = { + webContents, + loadFile: vi.fn(), + setContentProtection: vi.fn(), + close: vi.fn(), + on: vi.fn((name: string, handler: CloseHandler) => { + if (name === 'close') closeHandlers.push(handler); + }) + }; + const BrowserWindow = vi.fn(function BrowserWindow() { + return browserWindowInstance; + }) as unknown as BrowserWindowConstructor; + + createWindow({ BrowserWindow, appRootDir: path.join('/tmp', 'loop-dist'), closeGuard }); + + return { closeHandlers, webContents, browserWindowInstance }; +} describe('main/app/create-window', () => { test('uses the provided app root and applies hardened webPreferences', () => { @@ -103,4 +147,80 @@ describe('main/app/create-window', () => { expect(captured).not.toBeNull(); expect(captured!()).toEqual({ action: 'deny' }); }); + + test('close is prevented and the renderer notified while recording is active', () => { + const closeGuard = createCloseGuardStub('prevent'); + const { closeHandlers, webContents, browserWindowInstance } = createGuardedWindow(closeGuard); + + expect(closeHandlers).toHaveLength(1); + const event = { preventDefault: vi.fn() }; + closeHandlers[0](event); + + expect(closeGuard.handleCloseRequest).toHaveBeenCalled(); + expect(event.preventDefault).toHaveBeenCalled(); + expect(webContents.send).toHaveBeenCalledWith(CLOSE_REQUESTED_CHANNEL); + // The window itself is not force-closed; the renderer drives the real + // close after stop/finalize via app:confirm-close. + expect(browserWindowInstance.close).not.toHaveBeenCalled(); + }); + + test('close proceeds untouched when the guard allows it', () => { + const closeGuard = createCloseGuardStub('allow'); + const { closeHandlers, webContents } = createGuardedWindow(closeGuard); + + const event = { preventDefault: vi.fn() }; + closeHandlers[0](event); + + expect(event.preventDefault).not.toHaveBeenCalled(); + expect(webContents.send).not.toHaveBeenCalled(); + }); + + test('close proceeds when the renderer cannot be notified (destroyed webContents)', () => { + const closeGuard = createCloseGuardStub('prevent'); + const { closeHandlers, webContents } = createGuardedWindow(closeGuard, { + isDestroyed: vi.fn(() => true) + }); + + const event = { preventDefault: vi.fn() }; + closeHandlers[0](event); + + // Nobody can finalize + confirm the close, and the bytes are already in + // the .part files, so the close must not be blocked forever. + expect(event.preventDefault).not.toHaveBeenCalled(); + expect(webContents.send).not.toHaveBeenCalled(); + }); + + test('falls back to a confirmed close when notifying the renderer throws', () => { + const closeGuard = createCloseGuardStub('prevent'); + const send = vi.fn(() => { + throw new Error('render frame disposed'); + }); + const { closeHandlers, browserWindowInstance } = createGuardedWindow(closeGuard, { send }); + + const event = { preventDefault: vi.fn() }; + closeHandlers[0](event); + + expect(event.preventDefault).toHaveBeenCalled(); + expect(closeGuard.confirmClose).toHaveBeenCalled(); + expect(browserWindowInstance.close).toHaveBeenCalled(); + }); + + test('windows created without a close guard do not intercept close', () => { + const closeHandlers: CloseHandler[] = []; + const browserWindowInstance = { + webContents: { on: vi.fn(), setWindowOpenHandler: vi.fn() }, + loadFile: vi.fn(), + setContentProtection: vi.fn(), + on: vi.fn((name: string, handler: CloseHandler) => { + if (name === 'close') closeHandlers.push(handler); + }) + }; + const BrowserWindow = vi.fn(function BrowserWindow() { + return browserWindowInstance; + }) as unknown as BrowserWindowConstructor; + + createWindow({ BrowserWindow, appRootDir: path.join('/tmp', 'loop-dist') }); + + expect(closeHandlers).toHaveLength(0); + }); }); diff --git a/tests/unit/preload.test.ts b/tests/unit/preload.test.ts index 163b47c..d369c63 100644 --- a/tests/unit/preload.test.ts +++ b/tests/unit/preload.test.ts @@ -6,6 +6,7 @@ const { mockContextBridge, mockIpcRenderer, mockWebUtils } = vi.hoisted(() => ({ mockContextBridge: { exposeInMainWorld: vi.fn() }, mockIpcRenderer: { invoke: vi.fn(), + send: vi.fn(), on: vi.fn(), removeListener: vi.fn() }, @@ -150,4 +151,42 @@ describe('preload', () => { takeId: 'take-1' }); }); + + test('recordingSetActive sends a coerced boolean over the fire-and-forget channel', () => { + electronAPI.recordingSetActive(true); + expect(mockIpcRenderer.send).toHaveBeenCalledWith('recording:set-active', true); + + electronAPI.recordingSetActive(0 as unknown as boolean); + expect(mockIpcRenderer.send).toHaveBeenLastCalledWith('recording:set-active', false); + }); + + test('confirmClose invokes the matching IPC channel', () => { + electronAPI.confirmClose(); + expect(mockIpcRenderer.invoke).toHaveBeenCalledWith('app:confirm-close'); + }); + + test('onCloseRequested subscribes with unsubscribe support', () => { + const listener = vi.fn(); + const unsubscribe = electronAPI.onCloseRequested(listener); + + expect(mockIpcRenderer.on).toHaveBeenCalledWith('app:close-requested', expect.any(Function)); + + const handler = mockIpcRenderer.on.mock.calls.find( + (call) => call[0] === 'app:close-requested' + )?.[1] as (_event: unknown) => void; + handler({}); + expect(listener).toHaveBeenCalledTimes(1); + + unsubscribe(); + expect(mockIpcRenderer.removeListener).toHaveBeenCalledWith('app:close-requested', handler); + }); + + test('onCloseRequested ignores non-function listeners', () => { + const unsubscribe = electronAPI.onCloseRequested(undefined as unknown as () => void); + expect(typeof unsubscribe).toBe('function'); + expect(mockIpcRenderer.on).not.toHaveBeenCalledWith( + 'app:close-requested', + expect.any(Function) + ); + }); }); diff --git a/tests/unit/recorder-utils.test.ts b/tests/unit/recorder-utils.test.ts index 0468312..fe82adf 100644 --- a/tests/unit/recorder-utils.test.ts +++ b/tests/unit/recorder-utils.test.ts @@ -3,6 +3,9 @@ import { describe, expect, test } from 'vitest'; import { vi } from 'vitest'; import { + buildMicrophoneConstraints, + classifyRecorderFailure, + computeCameraVideoBitsPerSecond, createAudioOnlyRecordingStream, createCameraRecordingStream, createScreenRecordingStream, @@ -11,6 +14,7 @@ import { getRecorderFinalizeTimeoutMs, getRecorderTimesliceMs, getSupportedRecorderMimeType, + isOverconstrainedError, PREVIEW_FPS_IDLE, PREVIEW_FPS_RECORDING, RECORDER_FINALIZE_TIMEOUT_MS, @@ -20,23 +24,115 @@ import { } from '../../src/renderer/features/recording/recorder-utils'; describe('recorder-utils', () => { - test('prefers vp8 recorder support before heavier codecs', () => { - const mediaRecorderCtor = { + test('prefers vp9 over vp8, with plain webm as the last candidate', () => { + expect(RECORDER_MIME_CANDIDATES).toEqual([ + 'video/webm; codecs=vp9', + 'video/webm; codecs=vp8', + 'video/webm' + ]); + + const bothSupported = { isTypeSupported: (mimeType: string) => mimeType === 'video/webm; codecs=vp8' || mimeType === 'video/webm; codecs=vp9' }; + expect(getSupportedRecorderMimeType(bothSupported)).toBe('video/webm; codecs=vp9'); + }); - expect(RECORDER_MIME_CANDIDATES[0]).toBe('video/webm; codecs=vp8'); - expect(getSupportedRecorderMimeType(mediaRecorderCtor)).toBe('video/webm; codecs=vp8'); + test('falls back to vp8 when vp9 is unsupported, then plain webm', () => { + const vp8Only = { + isTypeSupported: (mimeType: string) => mimeType === 'video/webm; codecs=vp8' + }; + expect(getSupportedRecorderMimeType(vp8Only)).toBe('video/webm; codecs=vp8'); + + const plainOnly = { + isTypeSupported: (mimeType: string) => mimeType === 'video/webm' + }; + expect(getSupportedRecorderMimeType(plainOnly)).toBe('video/webm'); }); test('falls back to empty mime type when MediaRecorder support is unavailable', () => { - expect(getSupportedRecorderMimeType(undefined)).toBe(''); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + try { + expect(getSupportedRecorderMimeType(undefined)).toBe(''); + expect(warnSpy).toHaveBeenCalledWith(expect.stringMatching(/browser default codec/i)); + } finally { + warnSpy.mockRestore(); + } + }); + + test('warns that the browser default codec is used when no candidate is supported', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + try { + const nothingSupported = { isTypeSupported: () => false }; + expect(getSupportedRecorderMimeType(nothingSupported)).toBe(''); + expect(warnSpy).toHaveBeenCalledWith(expect.stringMatching(/browser default codec/i)); + } finally { + warnSpy.mockRestore(); + } + }); + + test('does not warn when a preferred codec is supported', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + try { + const supported = { isTypeSupported: () => true }; + expect(getSupportedRecorderMimeType(supported)).toBe('video/webm; codecs=vp9'); + expect(warnSpy).not.toHaveBeenCalled(); + } finally { + warnSpy.mockRestore(); + } + }); + + describe('computeCameraVideoBitsPerSecond', () => { + test('uses 30 Mbps at 4K and above', () => { + expect(computeCameraVideoBitsPerSecond(3840, 2160)).toBe(30_000_000); + expect(computeCameraVideoBitsPerSecond(4096, 2160)).toBe(30_000_000); + }); + + test('uses 20 Mbps at 1440p and above (below 4K)', () => { + expect(computeCameraVideoBitsPerSecond(2560, 1440)).toBe(20_000_000); + // Wide-but-short frames drop to the tier both dimensions satisfy. + expect(computeCameraVideoBitsPerSecond(3840, 2158)).toBe(20_000_000); + }); + + test('uses the 12 Mbps default at 1080p and below', () => { + expect(computeCameraVideoBitsPerSecond(1920, 1080)).toBe(12_000_000); + expect(computeCameraVideoBitsPerSecond(2559, 1440)).toBe(12_000_000); + expect(computeCameraVideoBitsPerSecond(1280, 720)).toBe(12_000_000); + }); + + test('treats missing or invalid dimensions as the 12 Mbps default', () => { + expect(computeCameraVideoBitsPerSecond(undefined, undefined)).toBe(12_000_000); + expect(computeCameraVideoBitsPerSecond(Number.NaN, 2160)).toBe(12_000_000); + expect(computeCameraVideoBitsPerSecond(3840, Number.NaN)).toBe(12_000_000); + expect( + computeCameraVideoBitsPerSecond(Number.POSITIVE_INFINITY, Number.POSITIVE_INFINITY) + ).toBe(12_000_000); + expect(computeCameraVideoBitsPerSecond(-3840, -2160)).toBe(12_000_000); + expect(computeCameraVideoBitsPerSecond(0, 0)).toBe(12_000_000); + expect( + computeCameraVideoBitsPerSecond('3840' as unknown as number, '2160' as unknown as number) + ).toBe(12_000_000); + }); + }); + + test('camera recorder options scale video bitrate with the captured resolution', () => { + expect( + getRecorderOptions( + { suffix: 'camera', hasAudio: false, videoWidth: 3840, videoHeight: 2160 }, + undefined + ) + ).toEqual({ videoBitsPerSecond: 30_000_000 }); + expect( + getRecorderOptions( + { suffix: 'camera', hasAudio: false, videoWidth: 2560, videoHeight: 1440 }, + undefined + ) + ).toEqual({ videoBitsPerSecond: 20_000_000 }); }); test('omits audio bitrate for camera recordings without audio tracks', () => { expect(getRecorderOptions({ suffix: 'camera', hasAudio: false }, undefined)).toEqual({ - videoBitsPerSecond: 10000000 + videoBitsPerSecond: 12_000_000 }); }); @@ -47,6 +143,47 @@ describe('recorder-utils', () => { }); }); + test('screen bitrate stays fixed at 30 Mbps regardless of captured dimensions', () => { + expect( + getRecorderOptions( + { suffix: 'screen', hasAudio: false, videoWidth: 5120, videoHeight: 2880 }, + undefined + ) + ).toEqual({ videoBitsPerSecond: 30000000 }); + }); + + describe('microphone constraints', () => { + test('requests explicit quality constraints with ideal (not exact) sample rate and channels', () => { + expect(buildMicrophoneConstraints('mic-1')).toEqual({ + deviceId: { exact: 'mic-1' }, + sampleRate: { ideal: 48000 }, + channelCount: { ideal: 2 }, + echoCancellation: false, + noiseSuppression: false, + autoGainControl: false + }); + }); + + test('falls back to device-id-only constraints so capture beats quality', () => { + expect(buildMicrophoneConstraints('mic-1', { includeQualityConstraints: false })).toEqual({ + deviceId: { exact: 'mic-1' } + }); + }); + + test('detects OverconstrainedError rejections and nothing else', () => { + expect(isOverconstrainedError({ name: 'OverconstrainedError' })).toBe(true); + const domLike = new Error('constraints not satisfied'); + domLike.name = 'OverconstrainedError'; + expect(isOverconstrainedError(domLike)).toBe(true); + + expect(isOverconstrainedError(new Error('NotAllowedError'))).toBe(false); + expect(isOverconstrainedError({ name: 'NotReadableError' })).toBe(false); + expect(isOverconstrainedError(null)).toBe(false); + expect(isOverconstrainedError(undefined)).toBe(false); + expect(isOverconstrainedError('OverconstrainedError')).toBe(false); + }); + }); + test('flushes recorder data on a steady interval during capture', () => { expect(RECORDER_TIMESLICE_MS).toBe(1000); expect(getRecorderTimesliceMs()).toBe(1000); @@ -237,6 +374,32 @@ describe('recorder-utils', () => { }); }); + test('finalizeStreamedRecording carries a durability warning through and logs it prominently', async () => { + const finalize = vi.fn(async () => ({ + path: '/tmp/screen.webm', + bytesWritten: 2048, + warning: 'fsync failed at finalize; bytes may not be fully flushed: EIO' + })); + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); + + try { + const result = await finalizeStreamedRecording({ + takeId: 'take-warn', + suffix: 'screen', + bytesWritten: 2048, + deps: { finalize } + }); + + // The recording still succeeded — a durability warning is not an error. + expect(result.error).toBeNull(); + expect(result.path).toBe('/tmp/screen.webm'); + expect(result.warning).toMatch(/fsync/i); + expect(errorSpy).toHaveBeenCalledWith(expect.stringMatching(/durability warning/i)); + } finally { + errorSpy.mockRestore(); + } + }); + test('finalizeStreamedRecording reports an error and cancels when no bytes were written', async () => { const finalize = vi.fn(); const cancel = vi.fn(async () => ({ cancelled: true })); @@ -285,4 +448,54 @@ describe('recorder-utils', () => { expect(result.path).toBeNull(); expect(result.error).toMatch(/could not be saved/i); }); + + describe('classifyRecorderFailure', () => { + test('auto-stops when the screen recorder can no longer append chunks to disk', () => { + const decision = classifyRecorderFailure('append', 'screen'); + expect(decision.shouldAutoStop).toBe(true); + expect(decision.userMessage).toMatch(/screen/i); + expect(decision.userMessage).toMatch(/save/i); + expect(decision.userMessage).toMatch(/stopping/i); + }); + + test('auto-stops on a screen recorder (encoder) error to finalize what is on disk', () => { + const decision = classifyRecorderFailure('recorder-error', 'screen'); + expect(decision.shouldAutoStop).toBe(true); + expect(decision.userMessage).toMatch(/screen/i); + expect(decision.userMessage).toMatch(/stopping/i); + }); + + test('auto-stops when the dedicated mic recorder fails (append or encoder error)', () => { + const appendDecision = classifyRecorderFailure('append', 'audio'); + expect(appendDecision.shouldAutoStop).toBe(true); + expect(appendDecision.userMessage).toMatch(/microphone/i); + expect(appendDecision.userMessage).toMatch(/stopping/i); + + const errorDecision = classifyRecorderFailure('recorder-error', 'audio'); + expect(errorDecision.shouldAutoStop).toBe(true); + expect(errorDecision.userMessage).toMatch(/microphone/i); + expect(errorDecision.userMessage).toMatch(/stopping/i); + }); + + test('camera append failure warns but keeps the recording going for partial success', () => { + const decision = classifyRecorderFailure('append', 'camera'); + expect(decision.shouldAutoStop).toBe(false); + expect(decision.userMessage).toMatch(/camera/i); + expect(decision.userMessage).toMatch(/continuing/i); + expect(decision.userMessage).toMatch(/screen/i); + }); + + test('camera recorder error warns but keeps the recording going for partial success', () => { + const decision = classifyRecorderFailure('recorder-error', 'camera'); + expect(decision.shouldAutoStop).toBe(false); + expect(decision.userMessage).toMatch(/camera/i); + expect(decision.userMessage).toMatch(/continuing/i); + }); + + test('system-audio fallback never stops the recording and only informs', () => { + const decision = classifyRecorderFailure('system-audio', 'screen'); + expect(decision.shouldAutoStop).toBe(false); + expect(decision.userMessage).toBe('System audio unavailable — recording screen without it'); + }); + }); }); diff --git a/tests/unit/register-handlers.test.ts b/tests/unit/register-handlers.test.ts index 3a8e839..a3cae6c 100644 --- a/tests/unit/register-handlers.test.ts +++ b/tests/unit/register-handlers.test.ts @@ -44,20 +44,35 @@ function createRecordingServiceStub() { }; } +function createCloseGuardStub() { + return { + setRecordingActive: vi.fn(), + isRecordingActive: vi.fn(() => false), + confirmClose: vi.fn(), + handleCloseRequest: vi.fn(() => 'allow' as const) + }; +} + function registerWithHandlers( opts: { systemPreferences?: { askForMediaAccess: ReturnType; getMediaAccessStatus: ReturnType; }; + getWindow?: () => unknown; } = {} ) { const handlers = new Map unknown>(); + const listeners = new Map unknown>(); const ipcMain = { handle(channel: string, handler: (event: unknown, payload: unknown) => unknown) { handlers.set(channel, handler); + }, + on(channel: string, listener: (event: unknown, payload: unknown) => unknown) { + listeners.set(channel, listener); } }; + const closeGuard = createCloseGuardStub(); const renderComposite = vi.fn( async (_opts: unknown, deps: { onProgress?: (u: unknown) => void; signal?: AbortSignal }) => { deps.onProgress?.({ phase: 'rendering', percent: 0.5, status: 'Rendering 50%' }); @@ -84,7 +99,8 @@ function registerWithHandlers( desktopCapturer: { getSources: vi.fn() }, shell: { openPath: vi.fn() }, systemPreferences: opts.systemPreferences, - getWindow: () => null, + getWindow: opts.getWindow || (() => null), + closeGuard, projectService: createProjectServiceStub(), renderComposite, exportPremiereProject, @@ -95,7 +111,15 @@ function registerWithHandlers( setPendingDisplayMediaSource: vi.fn() } as unknown as Parameters[0]); - return { handlers, renderComposite, exportPremiereProject, proxyService, recordingService }; + return { + handlers, + listeners, + closeGuard, + renderComposite, + exportPremiereProject, + proxyService, + recordingService + }; } describe('main/ipc/register-handlers', () => { @@ -525,4 +549,49 @@ describe('main/ipc/register-handlers', () => { }); expect(await handlers.get('recording:scan-orphans')!({ sender }, '')).toEqual([]); }); + + test('recording:set-active forwards a coerced boolean to the close guard', () => { + const { listeners, closeGuard } = registerWithHandlers(); + const listener = listeners.get('recording:set-active'); + expect(listener).toBeDefined(); + + listener!({}, true); + expect(closeGuard.setRecordingActive).toHaveBeenLastCalledWith(true); + + listener!({}, false); + expect(closeGuard.setRecordingActive).toHaveBeenLastCalledWith(false); + + // A malformed payload must never flip the guard on by accident. + listener!({}, undefined); + expect(closeGuard.setRecordingActive).toHaveBeenLastCalledWith(false); + + listener!({}, 1); + expect(closeGuard.setRecordingActive).toHaveBeenLastCalledWith(true); + }); + + test('app:confirm-close arms the bypass before closing the window', async () => { + const win = { close: vi.fn(), isDestroyed: vi.fn(() => false) }; + const { handlers, closeGuard } = registerWithHandlers({ getWindow: () => win }); + + await expect(handlers.get('app:confirm-close')!({}, undefined)).resolves.toBe(true); + + expect(closeGuard.confirmClose).toHaveBeenCalledTimes(1); + expect(win.close).toHaveBeenCalledTimes(1); + // Ordering matters: the bypass must be armed before close() re-triggers + // the window 'close' event, otherwise the guard prevents its own close. + const confirmOrder = closeGuard.confirmClose.mock.invocationCallOrder[0]; + const closeOrder = win.close.mock.invocationCallOrder[0]; + expect(confirmOrder).toBeLessThan(closeOrder); + }); + + test('app:confirm-close tolerates a missing or destroyed window', async () => { + const { handlers, closeGuard } = registerWithHandlers(); + await expect(handlers.get('app:confirm-close')!({}, undefined)).resolves.toBe(false); + expect(closeGuard.confirmClose).toHaveBeenCalled(); + + const destroyed = { close: vi.fn(), isDestroyed: vi.fn(() => true) }; + const { handlers: handlers2 } = registerWithHandlers({ getWindow: () => destroyed }); + await expect(handlers2.get('app:confirm-close')!({}, undefined)).resolves.toBe(false); + expect(destroyed.close).not.toHaveBeenCalled(); + }); });