diff --git a/CODE-AUDIT.md b/CODE-AUDIT.md index 8aef6e8..70b8315 100644 --- a/CODE-AUDIT.md +++ b/CODE-AUDIT.md @@ -309,8 +309,19 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n preload + handler + mock, zero callers). - [x] **M3 — Remove duplicated completion side-effect in `store/downloads.ts`** (history write + `markDownloaded` in both `applyEvent('done')` and the preview ticker). Subsumed by C2. -- [ ] **M4 — Decide queue persistence explicitly.** Scheduler/queue is renderer-memory only; - `saved`/scheduled items don't survive a quit. Persist or document as a non-goal in code. +- [x] **M4 — Decide queue persistence explicitly.** Scheduler/queue is renderer-memory only; + `saved`/scheduled items don't survive a quit. Persist or document as a non-goal in code. *Fixed + (persist, per the run decision): a main [queue.ts](src/main/queue.ts) store (the proven cached-atomic + `createJsonStore`, `queue.json`) plus `queue:list`/`queue:save` IPC. The renderer mirrors its durable + items on every queue change (a `PersistedQueueItem` = the stable subset of `DownloadItem`; runtime-only + progress/speed/eta are dropped, and a signature over the persistable subset means progress ticks don't + re-save) and rehydrates on launch via an `App` `useEffect`. On restore a `downloading` item (its process + died) normalizes back to `queued` (yt-dlp continues the `.part`), `saved`+`scheduledFor` survives so the + promoter fires it when due, `paused`/`error` return actionable; `completed`/`canceled` are never persisted. + A `queueHydrated` gate stops a launch-time store change wiping the file before it's read. Pure mappers + extracted to [queuePersist.ts](src/renderer/src/store/queuePersist.ts) and unit-tested (`test/queuePersist.test.ts`); + plumbing is typecheck-verified. **Live-verify note:** the OS-level quit→relaunch cycle itself wasn't + smoke-tested here — worth a manual confirm (schedule/park an item, quit, relaunch).* - [x] **M5 — Command-preview feature is fully built but unwired.** `CommandPreviewResult` type + `command:preview` channel + `previewCommand` (download.ts) + `formatCommandLine`/ `quoteForDisplay` (buildArgs) + preload method all exist, but no renderer component calls diff --git a/ROADMAP.md b/ROADMAP.md index 5abb98b..9ccdaba 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -342,8 +342,10 @@ Today `src/renderer/src/store/downloads.ts` has cancel + retry only. `datetime-local` input in `DownloadBar.tsx` (future time parks the new download as scheduled), plus per-row **Save for later** / **Add to queue** actions in `QueueItem.tsx`; saved items survive "Clear finished". Distinct from the Phase J `--sync` schedule (which is - for *sources*). **Limitation: the queue isn't persisted, so a schedule only fires while - AeroFetch is running** — noted in the UI hint and the code.* + for *sources*). The queue is now persisted across restarts (audit M4 — `queue.json` via + `src/main/queue.ts`), so a parked/scheduled item survives a quit and its schedule fires on the + next launch once due. (The app must still be running at the scheduled moment for an immediate + fire; a schedule that came due while closed fires at the next launch.)* ## Phase N — Power-user surface ✅ COMPLETE diff --git a/src/main/constants.ts b/src/main/constants.ts index fbc2fd9..fbae5fe 100644 --- a/src/main/constants.ts +++ b/src/main/constants.ts @@ -63,3 +63,5 @@ export const ERRORLOG_MAX_ENTRIES = 200 export const TEMPLATES_MAX = 100 /** Indexed media items across all sources (media-items.json). */ export const MEDIA_ITEMS_MAX = 20_000 +/** Persisted download-queue rows (queue.json, M4) — generous; a whole channel can be queued. */ +export const QUEUE_MAX = 10_000 diff --git a/src/main/ipc.ts b/src/main/ipc.ts index a279b66..0584a63 100644 --- a/src/main/ipc.ts +++ b/src/main/ipc.ts @@ -21,7 +21,8 @@ import { type CommandTemplate, type YtdlpUpdateChannel, type SystemThemeInfo, - type TaskbarProgress + type TaskbarProgress, + type PersistedQueueItem } from '@shared/ipc' import { PAGE_BACKGROUND } from '@shared/theme' import { getYtdlpVersion, updateYtdlp } from './ytdlp' @@ -36,6 +37,7 @@ import { listTemplates, saveTemplate, removeTemplate } from './templates' import { safeOpenPath, safeShowInFolder } from './reveal' import { openCookieLoginWindow, getCookiesStatus, clearCookies } from './cookies' import { listErrorLog, addErrorLog, clearErrorLog } from './errorlog' +import { listQueue, saveQueue } from './queue' import { exportBackup, importBackup } from './backup' import { listSources, @@ -208,6 +210,11 @@ export function registerIpcHandlers(getMainWindow: () => BrowserWindow | null): ipcMain.handle(IpcChannels.errorLogList, () => listErrorLog()) ipcMain.handle(IpcChannels.errorLogClear, () => clearErrorLog()) + // Persisted download queue (M4): the renderer mirrors its durable items here and + // rehydrates them on launch, so saved/scheduled/pending downloads survive a quit. + ipcMain.handle(IpcChannels.queueList, () => listQueue()) + ipcMain.handle(IpcChannels.queueSave, (_e, items: PersistedQueueItem[]) => saveQueue(items)) + ipcMain.handle(IpcChannels.backupExport, (e) => exportBackup(BrowserWindow.fromWebContents(e.sender) ?? undefined) ) diff --git a/src/main/queue.ts b/src/main/queue.ts new file mode 100644 index 0000000..0a0d863 --- /dev/null +++ b/src/main/queue.ts @@ -0,0 +1,46 @@ +import { createJsonStore } from './jsonStore' +import { QUEUE_MAX } from './constants' +import type { PersistedQueueItem, PersistedQueueStatus } from '@shared/ipc' + +/** + * Persistence for the download queue (M4). The queue lived only in renderer memory, + * so `saved` / scheduled and other pending downloads were silently lost on quit. + * The renderer now mirrors its durable items here (via `queueSave`) and rehydrates + * them on launch (via `queueList`), so a parked/scheduled download returns. + * + * Reuses the shared cached-atomic `createJsonStore` (R1/R2/R3), so writes are atomic + * and coalesced — the renderer can call `queueSave` freely on queue changes. + */ + +const PERSISTED_STATUSES: readonly PersistedQueueStatus[] = [ + 'queued', + 'downloading', + 'paused', + 'saved', + 'error' +] + +/** Defensive row validator (a hand-edited or partially-written file can't crash a read). */ +function isValidQueueItem(o: unknown): o is PersistedQueueItem { + if (typeof o !== 'object' || o === null) return false + const it = o as Record + return ( + typeof it.id === 'string' && + typeof it.url === 'string' && + typeof it.title === 'string' && + (it.kind === 'video' || it.kind === 'audio') && + typeof it.quality === 'string' && + typeof it.status === 'string' && + (PERSISTED_STATUSES as readonly string[]).includes(it.status) + ) +} + +const store = createJsonStore('queue.json', isValidQueueItem, QUEUE_MAX) + +export function listQueue(): PersistedQueueItem[] { + return store.read() +} + +export function saveQueue(items: PersistedQueueItem[]): void { + store.write(items) +} diff --git a/src/preload/index.ts b/src/preload/index.ts index f406e8b..8bd09f6 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -31,7 +31,8 @@ import { type ScheduledSyncStatus, type TerminalEvent, type TerminalRunResult, - type TaskbarProgress + type TaskbarProgress, + type PersistedQueueItem } from '@shared/ipc' // The surface exposed to the renderer. Keep this thin: it only forwards to IPC. @@ -139,6 +140,13 @@ const api = { clearErrorLog: (): Promise => ipcRenderer.invoke(IpcChannels.errorLogClear), + /** Load the persisted download queue on launch (M4). */ + listQueue: (): Promise => ipcRenderer.invoke(IpcChannels.queueList), + + /** Mirror the renderer's durable queue items to disk so they survive a quit (M4). */ + saveQueue: (items: PersistedQueueItem[]): Promise => + ipcRenderer.invoke(IpcChannels.queueSave, items), + /** Opens a save dialog and writes settings + templates to the chosen JSON file. */ exportBackup: (): Promise => ipcRenderer.invoke(IpcChannels.backupExport), diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 5681979..49de5a5 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -98,11 +98,17 @@ function App(): React.JSX.Element { return useDownloads.subscribe((st) => push(st.items)) }, []) + // Rehydrate the persisted download queue on launch (M4) so saved/scheduled and + // other pending items return from the previous session. + useEffect(() => { + useDownloads.getState().hydrate() + }, []) + // Promote scheduled ('saved' + a due scheduledFor) items when their time arrives. - // Session-only: the queue isn't persisted, so a schedule only fires while the app - // runs (noted in ROADMAP.md). A 15s tick is plenty for minute-granularity times. - // Lives here (not at the downloads store's module load) so importing the store in - // tests/preview doesn't start a stray timer, and the interval is cleared cleanly + // The queue is persisted (M4), so a scheduled item survives a quit and fires on the + // next launch once its time has passed. A 15s tick is plenty for minute-granularity + // times. Lives here (not at the downloads store's module load) so importing the store + // in tests/preview doesn't start a stray timer, and the interval is cleared cleanly // on unmount (L1). useEffect(() => { const tick = setInterval(() => { diff --git a/src/renderer/src/mockApi.ts b/src/renderer/src/mockApi.ts index eaac20d..2b69add 100644 --- a/src/renderer/src/mockApi.ts +++ b/src/renderer/src/mockApi.ts @@ -184,6 +184,8 @@ export const mockApi: Api = { onYtdlpAutoUpdateStatus: () => () => {}, listErrorLog: async () => [], clearErrorLog: async () => [], + listQueue: async () => [], + saveQueue: async () => {}, exportBackup: async () => ({ ok: true, path: 'C:\\Users\\you\\Downloads\\aerofetch-backup.json' diff --git a/src/renderer/src/store/downloadTypes.ts b/src/renderer/src/store/downloadTypes.ts index c5de272..1e2a215 100644 --- a/src/renderer/src/store/downloadTypes.ts +++ b/src/renderer/src/store/downloadTypes.ts @@ -124,4 +124,6 @@ export interface DownloadState { pump: () => void /** internal: apply a main-process download event */ applyEvent: (ev: DownloadEvent) => void + /** Load the persisted queue from main on launch (M4); no-op in preview. */ + hydrate: () => void } diff --git a/src/renderer/src/store/downloads.ts b/src/renderer/src/store/downloads.ts index cb81ea6..cc35900 100644 --- a/src/renderer/src/store/downloads.ts +++ b/src/renderer/src/store/downloads.ts @@ -1,10 +1,12 @@ import { create } from 'zustand' import { type MediaKind } from '@shared/ipc' import { isPreview as PREVIEW } from '../isPreview' +import { logError } from '../reportError' import { useSettings } from './settings' import { useHistory } from './history' import { emit, on } from './coordinator' import { RESOLVING, buildItem } from './downloadItem' +import { fromPersisted, persistableItems } from './queuePersist' import { seed } from './downloadSeed' import { type ChosenFormat, @@ -19,6 +21,11 @@ import { // `import { … } from '../store/downloads'` sites keep working. export type { MediaKind, ChosenFormat, DownloadItem, DownloadStatus, AddOptions, AddEntry } +// M4: gate the persist subscription until the initial hydrate has read the saved +// queue, so a launch-time store mutation can't overwrite queue.json with [] before +// we've loaded it back. +let queueHydrated = false + // Side-effects to run exactly once when a download completes: record it to // history (unless private) and mark its source MediaItem downloaded. Shared by // the real `applyEvent('done')` path and the preview ticker so the two can't @@ -154,6 +161,29 @@ export const useDownloads = create((set, get) => { items: seed, pump, + hydrate: () => { + if (PREVIEW) return + window.api + .listQueue() + .then((persisted) => { + if (persisted.length === 0) return + // Merge ahead of anything already present (normally nothing at launch), + // de-duping by id so a double-hydrate can't double-insert. + set((s) => { + const have = new Set(s.items.map((i) => i.id)) + const add = persisted.filter((p) => !have.has(p.id)).map(fromPersisted) + return add.length ? { items: [...s.items, ...add] } : {} + }) + pump() + }) + .catch(logError('listQueue')) + .finally(() => { + // Enable persistence once the read has completed (success or failure), so + // subsequent queue changes are saved but the pre-hydrate window can't wipe. + queueHydrated = true + }) + }, + addFromUrl: (url, kind, quality, opts) => { set((s) => ({ items: [buildItem(url, kind, quality, opts), ...s.items] })) pump() @@ -409,6 +439,24 @@ export const useDownloads = create((set, get) => { // no direct import from sources, so the two stores stay acyclic. on('enqueueDownloads', (entries) => useDownloads.getState().addMany(entries)) +// M4: mirror the durable queue to main whenever it changes, so saved/scheduled and +// other pending items survive a quit. Skipped in preview (no IPC bridge). A +// progress-only update doesn't churn this — toPersisted drops progress/speed/eta, so +// the persistable snapshot is unchanged by a tick and no redundant save fires. Main's +// jsonStore coalesces the actual disk writes. +if (!PREVIEW) { + let lastSig = '' + useDownloads.subscribe((state) => { + // Don't persist until the initial hydrate has read the saved file (above). + if (!queueHydrated) return + const persistable = persistableItems(state.items) + const sig = JSON.stringify(persistable) + if (sig === lastSig) return + lastSig = sig + window.api.saveQueue(persistable).catch(logError('saveQueue')) + }) +} + // The scheduled-download promoter tick lives in App (a useEffect), not here, so // importing this store in tests/preview doesn't start a stray never-cleared timer // on module load (L1). See SCHEDULE_TICK_MS / App.tsx. diff --git a/src/renderer/src/store/queuePersist.ts b/src/renderer/src/store/queuePersist.ts new file mode 100644 index 0000000..90623e5 --- /dev/null +++ b/src/renderer/src/store/queuePersist.ts @@ -0,0 +1,63 @@ +import { type PersistedQueueItem, type PersistedQueueStatus } from '@shared/ipc' +import { type DownloadItem, type DownloadStatus } from './downloadTypes' + +/** + * Pure mapping between the live `DownloadItem` and the durable `PersistedQueueItem` + * main stores across restarts (M4). Kept free of any store/zustand import so it can + * be unit-tested in isolation (mirrors queueStats / formatters). Only stable fields + * persist — runtime-only progress/speed/eta/sizeLabel/finishing are dropped and + * re-derived on restore. + */ + +// Statuses worth surviving a quit. 'completed'/'canceled' are transient results +// (completions already live in history) and are never persisted. +export const PERSISTED_STATUSES: readonly DownloadStatus[] = [ + 'queued', + 'downloading', + 'paused', + 'saved', + 'error' +] + +export function toPersisted(i: DownloadItem): PersistedQueueItem { + return { + id: i.id, + url: i.url, + title: i.title, + channel: i.channel, + durationLabel: i.durationLabel, + thumbnail: i.thumbnail, + kind: i.kind, + quality: i.quality, + status: i.status as PersistedQueueStatus, + filePath: i.filePath, + error: i.error, + formatId: i.formatId, + formatHasAudio: i.formatHasAudio, + options: i.options, + extraArgs: i.extraArgs, + trim: i.trim, + scheduledFor: i.scheduledFor, + incognito: i.incognito, + probedMeta: i.probedMeta, + collection: i.collection, + mediaItemId: i.mediaItemId + } +} + +export function fromPersisted(p: PersistedQueueItem): DownloadItem { + return { + ...p, + // A download in flight when we quit lost its process; re-queue it (yt-dlp + // continues the .part on relaunch). Progress restarts from 0. + status: p.status === 'downloading' ? 'queued' : p.status, + progress: 0 + } +} + +/** The persistable subset of the live queue, in durable form. */ +export function persistableItems(items: DownloadItem[]): PersistedQueueItem[] { + return items + .filter((i) => (PERSISTED_STATUSES as readonly string[]).includes(i.status)) + .map(toPersisted) +} diff --git a/src/shared/ipc.ts b/src/shared/ipc.ts index 014022e..96cf986 100644 --- a/src/shared/ipc.ts +++ b/src/shared/ipc.ts @@ -46,6 +46,9 @@ export const IpcChannels = { ytdlpAutoUpdateStatus: 'ytdlp:auto-update-status', errorLogList: 'errorlog:list', errorLogClear: 'errorlog:clear', + /** Persisted download queue (M4): saved/scheduled/pending items survive a quit. */ + queueList: 'queue:list', + queueSave: 'queue:save', backupExport: 'backup:export', backupImport: 'backup:import', systemThemeGet: 'system-theme:get', @@ -501,6 +504,41 @@ export type DownloadEvent = | { type: 'done'; id: string; filePath?: string } | { type: 'error'; id: string; error: string } +/** The subset of a persisted queue item's status worth surviving a quit. */ +export type PersistedQueueStatus = 'queued' | 'downloading' | 'paused' | 'saved' | 'error' + +/** + * A queue item persisted across restarts (M4) so saved/scheduled and other pending + * downloads return after a quit. This is the *durable* subset of the renderer's + * `DownloadItem`: runtime-only fields (progress/speed/eta/sizeLabel/finishing) are + * dropped since a restored item re-derives them. Transient results ('completed' / + * 'canceled') are never persisted; on load a 'downloading' item is normalized back + * to 'queued' because its process died with the previous session. + */ +export interface PersistedQueueItem { + id: string + url: string + title: string + channel?: string + durationLabel?: string + thumbnail?: string + kind: MediaKind + quality: string + status: PersistedQueueStatus + filePath?: string + error?: string + formatId?: string + formatHasAudio?: boolean + options?: DownloadOptions + extraArgs?: string + trim?: string + scheduledFor?: number + incognito?: boolean + probedMeta?: DownloadMeta + collection?: CollectionContext + mediaItemId?: string +} + /** Persisted user settings (electron-store). */ export interface Settings { /** where video downloads are saved; empty string = the default Documents\Video folder */ diff --git a/test/queuePersist.test.ts b/test/queuePersist.test.ts new file mode 100644 index 0000000..63b0d74 --- /dev/null +++ b/test/queuePersist.test.ts @@ -0,0 +1,75 @@ +import { describe, it, expect } from 'vitest' +import { + toPersisted, + fromPersisted, + persistableItems +} from '../src/renderer/src/store/queuePersist' +import type { DownloadItem } from '../src/renderer/src/store/downloadTypes' + +function item(over: Partial): DownloadItem { + return { + id: 'i1', + url: 'https://example.com/1', + title: 'Title', + kind: 'video', + quality: '1080p', + status: 'queued', + progress: 0, + ...over + } +} + +describe('queuePersist (M4)', () => { + it('keeps pending states and drops transient completed/canceled', () => { + const items = [ + item({ id: 'q', status: 'queued' }), + item({ id: 'd', status: 'downloading' }), + item({ id: 'p', status: 'paused' }), + item({ id: 's', status: 'saved', scheduledFor: 123 }), + item({ id: 'e', status: 'error', error: 'boom' }), + item({ id: 'c', status: 'completed' }), + item({ id: 'x', status: 'canceled' }) + ] + expect(persistableItems(items).map((i) => i.id)).toEqual(['q', 'd', 'p', 's', 'e']) + }) + + it('drops runtime-only fields when persisting', () => { + const p = toPersisted( + item({ + progress: 0.5, + speedBytesPerSec: 1000, + etaSeconds: 10, + sizeLabel: '5 MB', + finishing: true + }) + ) + expect(p).not.toHaveProperty('progress') + expect(p).not.toHaveProperty('speedBytesPerSec') + expect(p).not.toHaveProperty('etaSeconds') + expect(p).not.toHaveProperty('sizeLabel') + expect(p).not.toHaveProperty('finishing') + }) + + it('normalizes a downloading item back to queued with progress 0 on restore', () => { + const restored = fromPersisted(toPersisted(item({ status: 'downloading', progress: 0.7 }))) + expect(restored.status).toBe('queued') + expect(restored.progress).toBe(0) + }) + + it('preserves saved + scheduledFor across a round-trip (the core M4 promise)', () => { + const restored = fromPersisted(toPersisted(item({ status: 'saved', scheduledFor: 999 }))) + expect(restored.status).toBe('saved') + expect(restored.scheduledFor).toBe(999) + }) + + it('round-trips durable fields (formatId/trim/incognito)', () => { + const restored = fromPersisted( + toPersisted( + item({ formatId: '137', formatHasAudio: false, trim: '1:00-2:00', incognito: true }) + ) + ) + expect(restored.formatId).toBe('137') + expect(restored.trim).toBe('1:00-2:00') + expect(restored.incognito).toBe(true) + }) +})