diff --git a/src/main.ts b/src/main.ts index 7b8efe6..285ee00 100644 --- a/src/main.ts +++ b/src/main.ts @@ -52,7 +52,7 @@ import { registerTrustedIpcHandler } from './main/domain/privileged-ipc'; import { createRendererQueueItem, getMergeGroupCleanupPaths } from './main/domain/renderer-queue-input'; import { createAppStateStore, type AppStateStore } from './main/domain/app-state-store'; import { createExportableConfig } from './main/domain/config-export'; -import { persistStateChange } from './main/domain/persistence-commit'; +import { commitQueueMutation, persistStateChange } from './main/domain/persistence-commit'; import { resolveSecretInputUpdate } from './main/domain/secret-input'; import { createSecretStore, type SecretStore } from './main/domain/secret-store'; import { createElectronSecureStorage } from './main/infra/secure-storage'; @@ -7424,25 +7424,29 @@ registerTrustedIpcHandler(ipcMain, 'add-to-queue', isTrustedRendererEvent, () => registerTrustedIpcHandler(ipcMain, 'remove-from-queue', isTrustedRendererEvent, () => Promise.resolve(downloadQueue), async (_, id: string) => { if (typeof id !== 'string' || !id) return downloadQueue; const wasActiveItem = activeQueueItemId === id || activeDownloads.has(id) || queueProcessRegistry.activeItemIds().includes(id); + const removedItem = downloadQueue.find((item) => item.id === id); - if (wasActiveItem) { - cancelledItemIds.add(id); - await queueProcessRegistry.cancelItem(id); - activeDownloads.delete(id); - const nextActiveId = queueProcessRegistry.activeItemIds()[0] || null; - activeQueueItemId = nextActiveId; - runtimeMetrics.activeItemId = nextActiveId; - runtimeMetrics.activeItemTitle = nextActiveId ? downloadQueue.find((item) => item.id === nextActiveId)?.title || null : null; - appendDebugLog('queue-item-removed-active-cancelled', { id }); - } - - // Clean up merge-group temp files (must run for any merge group, not just active) - const removedItem = downloadQueue.find(item => item.id === id); - for (const cleanupPath of getMergeGroupCleanupPaths(removedItem)) { - try { if (fs.existsSync(cleanupPath)) fs.unlinkSync(cleanupPath); } catch { } - } - - downloadQueue = persistStateChange(downloadQueue, (current) => current.filter((item) => item.id !== id), saveQueue); + await commitQueueMutation( + downloadQueue, + (current) => current.filter((item) => item.id !== id), + saveQueue, + (nextQueue) => { downloadQueue = nextQueue; }, + async () => { + if (wasActiveItem) { + cancelledItemIds.add(id); + await queueProcessRegistry.cancelItem(id); + activeDownloads.delete(id); + const nextActiveId = queueProcessRegistry.activeItemIds()[0] || null; + activeQueueItemId = nextActiveId; + runtimeMetrics.activeItemId = nextActiveId; + runtimeMetrics.activeItemTitle = nextActiveId ? downloadQueue.find((item) => item.id === nextActiveId)?.title || null : null; + appendDebugLog('queue-item-removed-active-cancelled', { id }); + } + for (const cleanupPath of getMergeGroupCleanupPaths(removedItem)) { + try { if (fs.existsSync(cleanupPath)) fs.unlinkSync(cleanupPath); } catch { } + } + }, + ); emitQueueUpdated(); return downloadQueue; }); @@ -7631,7 +7635,6 @@ ipcMain.handle('pause-download', async (event) => { if (!isTrustedRendererEvent(event)) return false; if (!isDownloading || queuePaused) return false; - await Promise.all(queueProcessRegistry.activeItemIds().map((id) => queueProcessRegistry.pauseItem(id))); const nextQueue = downloadQueue.map((item) => item.status === 'downloading' ? { ...item, status: 'paused' as const, @@ -7639,8 +7642,18 @@ ipcMain.handle('pause-download', async (event) => { eta: '', progressStatus: tBackend('downloadPaused') } : item); - downloadQueue = persistStateChange(downloadQueue, () => nextQueue, saveQueue); - queuePaused = true; + await commitQueueMutation( + downloadQueue, + () => nextQueue, + saveQueue, + (candidate) => { + downloadQueue = candidate; + queuePaused = true; + }, + async () => { + await Promise.all(queueProcessRegistry.activeItemIds().map((id) => queueProcessRegistry.pauseItem(id))); + }, + ); emitQueueUpdated(true); mainWindow?.webContents.send('download-paused'); return true; diff --git a/src/main/domain/persistence-commit.test.ts b/src/main/domain/persistence-commit.test.ts index 36d349e..726b34d 100644 --- a/src/main/domain/persistence-commit.test.ts +++ b/src/main/domain/persistence-commit.test.ts @@ -1,10 +1,10 @@ -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { openDatabase, type DbHandle } from '../infra/db'; import { createAppStateStore } from './app-state-store'; -import { persistStateChange } from './persistence-commit'; +import { commitQueueMutation, persistStateChange } from './persistence-commit'; let directory: string; let db: DbHandle; @@ -59,4 +59,63 @@ describe('persistStateChange', () => { expect(runtime).toEqual(previous); expect(createAppStateStore(db).loadQueue()).toEqual(previous); }); + + it('does not cancel a process or delete a file when queue removal cannot persist', async () => { + const previous = [{ id: 'q1', status: 'pending' }]; + const temporaryFile = path.join(directory, 'q1.partial'); + fs.writeFileSync(temporaryFile, 'keep'); + createAppStateStore(db).saveQueue(previous); + const failingDb: DbHandle = { + ...db, + run(sql, params) { + if (sql.includes('DELETE FROM queue_items')) throw new Error('SQLITE_IOERR remove'); + db.run(sql, params); + }, + }; + const cancelProcess = vi.fn(); + let runtime = previous; + + await expect(commitQueueMutation( + runtime, + () => [], + (candidate) => createAppStateStore(failingDb).saveQueue(candidate), + (candidate) => { runtime = candidate; }, + async () => { + cancelProcess('q1'); + fs.rmSync(temporaryFile); + }, + )).rejects.toThrow('SQLITE_IOERR remove'); + + expect(cancelProcess).not.toHaveBeenCalled(); + expect(fs.existsSync(temporaryFile)).toBe(true); + expect(runtime).toEqual(previous); + expect(createAppStateStore(db).loadQueue()).toEqual(previous); + }); + + it('does not pause a process when a paused queue snapshot cannot persist', async () => { + const previous = [{ id: 'q1', status: 'downloading' }]; + const paused = [{ id: 'q1', status: 'paused' }]; + createAppStateStore(db).saveQueue(previous); + const failingDb: DbHandle = { + ...db, + run(sql, params) { + if (sql.includes('DELETE FROM queue_items')) throw new Error('SQLITE_IOERR pause'); + db.run(sql, params); + }, + }; + const pauseProcess = vi.fn(); + let runtime = previous; + + await expect(commitQueueMutation( + runtime, + () => paused, + (candidate) => createAppStateStore(failingDb).saveQueue(candidate), + (candidate) => { runtime = candidate; }, + async () => { pauseProcess('q1'); }, + )).rejects.toThrow('SQLITE_IOERR pause'); + + expect(pauseProcess).not.toHaveBeenCalled(); + expect(runtime).toEqual(previous); + expect(createAppStateStore(db).loadQueue()).toEqual(previous); + }); }); diff --git a/src/main/domain/persistence-commit.ts b/src/main/domain/persistence-commit.ts index e523988..fe1af18 100644 --- a/src/main/domain/persistence-commit.ts +++ b/src/main/domain/persistence-commit.ts @@ -3,3 +3,17 @@ export function persistStateChange(current: T, createNext: (current: T) => T, persist(next); return next; } + +export async function commitQueueMutation( + current: T, + createNext: (current: T) => T, + persist: (next: T) => void, + apply: (next: T) => void, + effects: () => Promise, +): Promise { + const next = createNext(current); + persist(next); + apply(next); + await effects(); + return next; +} diff --git a/src/renderer-settings-autosave.test.ts b/src/renderer-settings-autosave.test.ts new file mode 100644 index 0000000..e287d8d --- /dev/null +++ b/src/renderer-settings-autosave.test.ts @@ -0,0 +1,83 @@ +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as vm from 'node:vm'; +import * as ts from 'typescript'; +import { describe, expect, it } from 'vitest'; + +type Input = { + value: string; + checked: boolean; +}; + +const inputIds = [ + 'clientId', 'clientSecret', 'sidebarSplitViewToggle', 'downloadMode', 'partMinutes', 'parallelDownloads', + 'performanceMode', 'smartSchedulerToggle', 'duplicatePreventionToggle', 'persistQueueToggle', + 'autoResumeQueueToggle', 'notifyEachCompletionToggle', 'streamlinkDisableAdsToggle', 'downloadChatReplayToggle', + 'captureLiveChatToggle', 'logStreamEventsToggle', 'autoResumeLiveRecordingToggle', 'autoMergeResumedPartsToggle', + 'deletePartsAfterMergeToggle', 'discordWebhookUrl', 'discordNotifyLiveStartToggle', 'discordNotifyLiveEndToggle', + 'discordNotifyVodCompleteToggle', 'discordNotifyVodAutoQueuedToggle', 'autoVodPollMinutes', 'autoVodMaxAgeHours', + 'autoCleanupEnabledToggle', 'autoCleanupDays', 'autoCleanupTarget', 'autoCleanupAction', 'streamlinkQuality', + 'metadataCacheMinutes', 'vodFilenameTemplate', 'partsFilenameTemplate', 'defaultClipFilenameTemplate' +]; + +function createInput(value = '', checked = false): Input { + return { value, checked }; +} + +describe('renderer settings autosave orchestration', () => { + it('persists a newer secret after an earlier asynchronous save settles', async () => { + const inputs = new Map(inputIds.map((id) => [id, createInput()])); + inputs.get('clientSecret')!.value = 'A'; + let resolveFirstSecret: ((status: unknown) => void) | undefined; + const setClientSecretCalls: string[] = []; + const saveConfigCalls: unknown[] = []; + const window = { + api: { + setClientSecret(value: string) { + setClientSecretCalls.push(value); + if (value === 'A') { + return new Promise((resolve) => { resolveFirstSecret = resolve; }); + } + return Promise.resolve({ encryptionAvailable: true, clientSecretConfigured: true, discordWebhookConfigured: false }); + }, + clearClientSecret: () => Promise.resolve({ encryptionAvailable: true, clientSecretConfigured: false, discordWebhookConfigured: false }), + setDiscordWebhook: () => Promise.resolve({ encryptionAvailable: true, clientSecretConfigured: false, discordWebhookConfigured: true }), + clearDiscordWebhook: () => Promise.resolve({ encryptionAvailable: true, clientSecretConfigured: false, discordWebhookConfigured: false }), + saveConfig(payload: unknown) { + saveConfigCalls.push(payload); + return Promise.resolve(payload); + }, + }, + }; + const sandbox = { + window, + config: {}, + UI_TEXT: { status: {}, static: {}, streamers: {} }, + byId: (id: string) => inputs.get(id) ?? createInput(), + collectUnknownTemplatePlaceholders: () => [], + document: { hidden: false, querySelector: () => null, getElementById: () => null }, + setTimeout, + clearTimeout, + console, + }; + const context = vm.createContext(sandbox); + const source = fs.readFileSync(path.join(process.cwd(), 'src', 'renderer-settings.ts'), 'utf8'); + const compiled = ts.transpileModule(source, { + compilerOptions: { target: ts.ScriptTarget.ES2022, module: ts.ModuleKind.None }, + }).outputText; + vm.runInContext(compiled, context); + + const firstSave = vm.runInContext('flushSettingsAutoSave(false)', context) as Promise; + expect(setClientSecretCalls).toEqual(['A']); + expect(vm.runInContext('settingsAutoSaveInFlight', context)).toBe(true); + + vm.runInContext("byId('clientSecret').value = 'B'; secretInputGenerations.clientSecret += 1; if (typeof settingsInputGeneration === 'number') settingsInputGeneration += 1; void flushSettingsAutoSave(false);", context); + expect(vm.runInContext('secretInputGenerations.clientSecret', context)).toBe(1); + resolveFirstSecret?.({ encryptionAvailable: true, clientSecretConfigured: true, discordWebhookConfigured: false }); + await firstSave; + await new Promise((resolve) => setImmediate(resolve)); + + expect(setClientSecretCalls).toEqual(['A', 'B']); + expect(saveConfigCalls).toHaveLength(2); + }); +}); diff --git a/src/renderer-settings.ts b/src/renderer-settings.ts index 32592a3..788cc9e 100644 --- a/src/renderer-settings.ts +++ b/src/renderer-settings.ts @@ -6,6 +6,7 @@ let pendingSettingsAutoSave = false; let settingsAutoSaveTimer: number | null = null; let pendingCredentialsReconnect = false; let lastPersistedSettingsFingerprint = ''; +let settingsInputGeneration = 0; const SECRET_INPUT_MASK = '••••••••'; let secretStatus: SecretStatus = { encryptionAvailable: false, @@ -26,6 +27,10 @@ function canRunSettingsAutoRefresh(): boolean { return document.querySelector('.tab-content.active')?.id === 'settingsTab'; } +function markSettingsInputChanged(): void { + settingsInputGeneration += 1; +} + async function connect(): Promise { const hasCredentials = Boolean((config.client_id ?? '').toString().trim() && secretStatus.clientSecretConfigured); if (!hasCredentials) { @@ -99,6 +104,7 @@ function applyTemplatePreset(preset: string): void { byId('partsFilenameTemplate').value = selected.parts; byId('defaultClipFilenameTemplate').value = selected.clip; validateFilenameTemplates(); + markSettingsInputChanged(); // Programmatic .value = ... does not trigger the 'input' event the // template inputs listen on for debounced save, so the preset click // would otherwise look applied but never persist until the user @@ -831,6 +837,7 @@ async function flushSettingsAutoSave(reconnectAfterSave = false): Promise const payload = collectAutoSavePayload(); const fingerprint = getSettingsFingerprint(payload); + const inputGeneration = settingsInputGeneration; if (fingerprint === lastPersistedSettingsFingerprint) { if (reconnectAfterSave && pendingCredentialsReconnect) { @@ -849,7 +856,11 @@ async function flushSettingsAutoSave(reconnectAfterSave = false): Promise try { await persistSecretInputs(); config = await window.api.saveConfig(payload); - lastPersistedSettingsFingerprint = getSettingsFingerprint({}); + if (settingsInputGeneration === inputGeneration) { + lastPersistedSettingsFingerprint = getSettingsFingerprint({}); + } else { + pendingSettingsAutoSave = true; + } if (reconnectAfterSave && pendingCredentialsReconnect) { pendingCredentialsReconnect = false; await connect(); @@ -928,13 +939,17 @@ function initSettingsAutoSave(): void { for (const id of immediateSaveIds) { const element = byId(id); - element.addEventListener('change', triggerImmediateSave); + element.addEventListener('change', () => { + markSettingsInputChanged(); + triggerImmediateSave(); + }); element.addEventListener('blur', triggerImmediateSave); } for (const id of debouncedSaveIds) { const element = byId(id); element.addEventListener('input', () => { + markSettingsInputChanged(); scheduleSettingsAutoSave(); }); element.addEventListener('blur', () => { @@ -945,6 +960,7 @@ function initSettingsAutoSave(): void { for (const id of credentialIds) { const element = byId(id); element.addEventListener('input', () => { + markSettingsInputChanged(); pendingCredentialsReconnect = true; scheduleSettingsAutoSave(); });