From e4e6207430702c09a1027177a660f1d51bc1d1e6 Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 8 Sep 2026 02:36:46 +0200 Subject: [PATCH] fix(downloads): journal private cleanup captures for recovery --- AGENTS.md | 2 + CLAUDE.md | 2 + .../src/xtream-catchup-timezone.e2e.ts | 58 +++++++++++++++ .../database/download-catchup-capture.ts | 33 +++++++++ .../database/download-catchup-cleanup.spec.ts | 5 +- .../database/download-catchup-cleanup.ts | 14 ---- .../database/download-catchup-journal.ts | 27 ++++++- .../download-catchup-recovery.spec.ts | 34 +++++++++ .../database/download-catchup-removal.spec.ts | 43 +++++++++--- .../database/download-catchup-removal.ts | 49 +++++++++---- .../database/download-catchup-runtime.spec.ts | 70 +++++++++++++++++++ .../database/download-redownload.spec.ts | 9 ++- .../events/database/download-redownload.ts | 9 ++- .../database/download-removal-requests.ts | 31 +++++++- .../app/events/database/download-runtime.ts | 16 +++-- .../events/database/downloads.events.spec.ts | 25 ++++++- .../events/database/downloads.test-helpers.ts | 3 + docs/architecture/download-manager.md | 10 ++- 18 files changed, 383 insertions(+), 57 deletions(-) create mode 100644 apps/electron-backend/src/app/events/database/download-catchup-capture.ts diff --git a/AGENTS.md b/AGENTS.md index 5a01cdb7c..4b5fe295e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1043,6 +1043,8 @@ The same journal stores transfer-phase descriptor identity before truncation; Resume checks it at open, and rejected replacements are preserved and detached so Retry can reserve a fresh path. A synchronous completion-commit boundary rejects late pause/cancel commands before awaited cleanup and persistence. +Private cleanup captures are journaled before relocation, keeping failed +Remove/Clear/cancel cleanup retryable across restarts without hardlinks. Archive transfers validate TS framing, restart from byte zero after interruption and check expiry again at transfer start. Completed cards play locally and never route to VOD details. Contract and EOF/duration limits: diff --git a/CLAUDE.md b/CLAUDE.md index 10e694dec..34386f6bc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1854,6 +1854,8 @@ The same journal stores transfer-phase descriptor identity before truncation; Resume checks it at open, and rejected replacements are preserved and detached so Retry can reserve a fresh path. A synchronous completion-commit boundary rejects late pause/cancel commands before awaited cleanup and persistence. +Private cleanup captures are journaled before relocation, keeping failed +Remove/Clear/cancel cleanup retryable across restarts without hardlinks. Archive transfers validate TS framing, restart from byte zero after interruption and check expiry again at transfer start. Completed cards play locally and never route to VOD details. Contract and EOF/duration limits: diff --git a/apps/electron-backend-e2e/src/xtream-catchup-timezone.e2e.ts b/apps/electron-backend-e2e/src/xtream-catchup-timezone.e2e.ts index ac80a84b0..97316eb23 100644 --- a/apps/electron-backend-e2e/src/xtream-catchup-timezone.e2e.ts +++ b/apps/electron-backend-e2e/src/xtream-catchup-timezone.e2e.ts @@ -4,6 +4,7 @@ import { } from './downloads.e2e-support'; import { mkdirSync, + linkSync, readFileSync, readdirSync, unlinkSync, @@ -540,6 +541,63 @@ test('@downloads @epg @xtream @electron downloads a completed archive into the l expect(readFileSync(redownloaded.filePath)).toEqual( readFileSync('apps/xtream-mock-server/src/fixtures/live.mpegts') ); + // Fail owned cleanup after private capture. Its recovery pointer must + // survive a process restart without relying on hardlink restoration. + linkSync(redownloaded.filePath, redownloaded.filePath + '.part'); + await app.electronApp.evaluate(() => { + const fs = process.getBuiltinModule('fs'); + const unlink = fs.unlinkSync; + fs.unlinkSync = (path) => { + if (String(path).includes('.iptvnator-cleanup-')) + throw Object.assign(new Error('simulated locked capture'), { + code: 'EACCES', + }); + return unlink(path); + }; + }); + expect( + await app.mainWindow.evaluate( + (id) => window.electron.downloadsRemove(id), + row.id + ) + ).toEqual({ + success: false, + error: 'Could not delete the partial file', + }); + const capturedPath = await app.electronApp.evaluate( + (_electron, { dependency, file, id }) => { + const Database = process + .getBuiltinModule('module') + .createRequire(dependency)(dependency); + const db = new Database(file); + try { + const record = db + .prepare( + 'SELECT proof FROM download_archive_finalizations WHERE download_id=?' + ) + .get(id) as { proof: string }; + return ( + JSON.parse(record.proof) as { + partialCleanupPath: string; + } + ).partialCleanupPath; + } finally { + db.close(); + } + }, + { + dependency: join(workspaceRoot, 'node_modules/better-sqlite3'), + file: join(dataDir, 'databases/iptvnator.db'), + id: row.id, + } + ); + expect(capturedPath).toContain('.iptvnator-cleanup-'); + expect(readFileSync(capturedPath)).toEqual( + readFileSync(redownloaded.filePath) + ); + app = await restartElectronApp(app, dataDir, { + env: { TZ: VIEWER_TIMEZONE }, + }); writeFileSync( redownloaded.filePath + '.part', 'unrelated terminal file' diff --git a/apps/electron-backend/src/app/events/database/download-catchup-capture.ts b/apps/electron-backend/src/app/events/database/download-catchup-capture.ts new file mode 100644 index 000000000..82be2aabd --- /dev/null +++ b/apps/electron-backend/src/app/events/database/download-catchup-capture.ts @@ -0,0 +1,33 @@ +import { lstatSync, rmdirSync, unlinkSync } from 'node:fs'; +import { dirname } from 'node:path'; +import type { ArchiveDownloadProof } from './download-catchup-journal'; + +/** Retry a journaled private capture without ever deleting a replacement. */ +export function cleanupArchiveCapture( + proof: ArchiveDownloadProof | undefined +): void { + const path = proof?.partialCleanupPath; + if (!path || !proof) return; + try { + const file = lstatSync(path); + if ( + file.isFile() && + file.dev === proof.partialIdentity.dev && + file.ino === proof.partialIdentity.ino + ) { + unlinkSync(path); + } else { + console.warn( + '[Downloads] Replaced file retained for recovery:', + path + ); + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + } + try { + rmdirSync(dirname(path)); + } catch { + /* never recursively delete captures */ + } +} diff --git a/apps/electron-backend/src/app/events/database/download-catchup-cleanup.spec.ts b/apps/electron-backend/src/app/events/database/download-catchup-cleanup.spec.ts index a02409e48..69dc1edc3 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-cleanup.spec.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-cleanup.spec.ts @@ -13,7 +13,6 @@ import { tmpdir } from 'node:os'; import { cleanupCatchupFile, cleanupCatchupPartial, - cleanupSelectedCatchupPartial, } from './download-catchup-cleanup'; jest.mock('node:fs/promises', () => { @@ -95,11 +94,11 @@ it.each(['EEXIST', 'ENOTSUP'])( } ); -it('preserves an unopened partial after failure but removes it on explicit queued cancellation', async () => { +it('preserves an unopened partial and removes it only with matching ownership', async () => { const { path } = await prepare(); const final = path.slice(0, -'.part'.length); expect(await cleanupCatchupPartial(final, undefined)).toBe(false); expect(await readFile(path, 'utf8')).toBe('archive'); - expect(await cleanupSelectedCatchupPartial(final)).toBe(true); + expect(await cleanupCatchupPartial(final, await lstat(path))).toBe(true); await expect(lstat(path)).rejects.toMatchObject({ code: 'ENOENT' }); }); diff --git a/apps/electron-backend/src/app/events/database/download-catchup-cleanup.ts b/apps/electron-backend/src/app/events/database/download-catchup-cleanup.ts index d4043e9b5..f22cfe8b7 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-cleanup.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-cleanup.ts @@ -55,17 +55,3 @@ export async function cleanupCatchupPartial( return (error as NodeJS.ErrnoException).code === 'ENOENT'; } } - -/** Explicit cancellation of a queued/paused archive owns the selected entry. */ -export async function cleanupSelectedCatchupPartial( - filePath: string | null | undefined -): Promise { - if (!filePath) return true; - try { - const stats = await lstat(filePath + '.part'); - if (!stats.isFile()) return false; - return cleanupCatchupPartial(filePath, stats); - } catch (error) { - return (error as NodeJS.ErrnoException).code === 'ENOENT'; - } -} diff --git a/apps/electron-backend/src/app/events/database/download-catchup-journal.ts b/apps/electron-backend/src/app/events/database/download-catchup-journal.ts index 1bb0144d1..bc57c79be 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-journal.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-journal.ts @@ -1,3 +1,4 @@ +import { cleanupArchiveCapture } from './download-catchup-capture'; import { eq, inArray } from 'drizzle-orm'; import { lstatSync } from 'node:fs'; import { isAbsolute } from 'node:path'; @@ -11,6 +12,7 @@ export interface ArchiveFinalizationProof { filePath: string; size: number; partialIdentity: ArchiveFileIdentity; + partialCleanupPath?: string; finalIdentity: ArchiveFileIdentity; } @@ -19,6 +21,7 @@ export interface ArchivePartialProof { phase: 'transfer'; filePath: string; partialIdentity: ArchiveFileIdentity; + partialCleanupPath?: string; } export type ArchiveDownloadProof = ArchiveFinalizationProof | ArchivePartialProof; @@ -74,11 +77,30 @@ async function writeArchiveProof( }); } +/** Commit the recovery pointer synchronously before a public entry is captured. */ +export function recordArchiveCleanupPath( + db: DownloadsDatabase, + downloadId: number, + proof: ArchiveDownloadProof, + path: string +): void { + const result = db + .update(schema.downloadArchiveFinalizations) + .set({ proof: JSON.stringify({ ...proof, partialCleanupPath: path }) }) + .where(eq(schema.downloadArchiveFinalizations.downloadId, downloadId)) + .run(); + if (result.changes !== 1) + throw new Error('Archive cleanup ownership is unavailable'); +} + /** Fresh reservations must never inherit an earlier attempt's proof. */ export async function clearArchiveFinalization( db: DownloadsDatabase, downloadId: number ): Promise { + cleanupArchiveCapture( + (await readArchiveFinalizations(db, [downloadId])).get(downloadId) + ); await db .delete(schema.downloadArchiveFinalizations) .where(eq(schema.downloadArchiveFinalizations.downloadId, downloadId)); @@ -103,7 +125,10 @@ export function parseArchiveFinalization( proof.version !== 1 || typeof proof.filePath !== 'string' || !isAbsolute(proof.filePath) || - !identity(proof.partialIdentity) + !identity(proof.partialIdentity) || + (proof.partialCleanupPath !== undefined && + (typeof proof.partialCleanupPath !== 'string' || + !isAbsolute(proof.partialCleanupPath))) ) return undefined; if (proof.phase === 'transfer') return proof; diff --git a/apps/electron-backend/src/app/events/database/download-catchup-recovery.spec.ts b/apps/electron-backend/src/app/events/database/download-catchup-recovery.spec.ts index f7d42f5ee..ce89cfc31 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-recovery.spec.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-recovery.spec.ts @@ -15,6 +15,7 @@ import { finalizeCatchupPartial } from './download-catchup-finalize'; import { recordArchiveFinalization, recordArchivePartial, + recordArchiveCleanupPath, parseArchiveFinalization, } from './download-catchup-journal'; import { resetStaleDownloads } from './download-recovery'; @@ -289,3 +290,36 @@ it.each([false, true])( ); } ); + +it.each([0, 1])( + 'requires a durable row for the write-ahead capture pointer (changes=%s)', + async (changes) => { + const { partial } = await prepare(); + const proof = { + version: 1 as const, + phase: 'transfer' as const, + filePath, + partialIdentity: partial, + }; + const run = jest.fn(() => ({ changes })); + const set = jest.fn((_value: { proof: string }) => ({ + where: () => ({ run }), + })); + const database = { + update: () => ({ set }), + } as unknown as DownloadsDatabase; + const capturePath = join(directory, '.iptvnator-cleanup-test/entry'); + const record = () => + recordArchiveCleanupPath(database, 1, proof, capturePath); + if (changes === 0) expect(record).toThrow('ownership'); + else expect(record).not.toThrow(); + const saved = JSON.parse(set.mock.calls[0][0].proof); + expect(saved).toEqual( + expect.objectContaining({ + phase: 'transfer', + partialCleanupPath: capturePath, + }) + ); + expect(run).toHaveBeenCalledTimes(1); + } +); diff --git a/apps/electron-backend/src/app/events/database/download-catchup-removal.spec.ts b/apps/electron-backend/src/app/events/database/download-catchup-removal.spec.ts index dd40fb453..e7d2a038b 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-removal.spec.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-removal.spec.ts @@ -6,6 +6,7 @@ import { rmSync, writeFileSync, unlinkSync, + linkSync, } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -17,6 +18,7 @@ jest.mock('node:fs', () => { ...actual, renameSync: jest.fn(actual.renameSync), unlinkSync: jest.fn(actual.unlinkSync), + linkSync: jest.fn(actual.linkSync), }; }); const actual = jest.requireActual('node:fs'); @@ -33,20 +35,24 @@ beforeEach(() => { }; jest.mocked(renameSync).mockReset().mockImplementation(actual.renameSync); jest.mocked(unlinkSync).mockReset().mockImplementation(actual.unlinkSync); + jest.mocked(linkSync).mockReset().mockImplementation(actual.linkSync); }); afterEach(() => rmSync(directory, { recursive: true, force: true })); +function recordCapture(path: string) { + proof = { ...proof, partialCleanupPath: path }; +} it('removes only the journaled partial', () => { - removeJournaledCatchupPartial(filePath, proof); + removeJournaledCatchupPartial(filePath, proof, recordCapture); expect(() => lstatSync(filePath + '.part')).toThrow(); }); it('preserves entries without proof', () => { - removeJournaledCatchupPartial(filePath, undefined); + removeJournaledCatchupPartial(filePath, undefined, recordCapture); expect(readFileSync(filePath + '.part', 'utf8')).toBe('owned bytes'); }); it('preserves a replaced regular partial in place', () => { renameSync(filePath + '.part', join(directory, 'original')); writeFileSync(filePath + '.part', 'unrelated bytes'); - removeJournaledCatchupPartial(filePath, proof); + removeJournaledCatchupPartial(filePath, proof, recordCapture); expect(readFileSync(filePath + '.part', 'utf8')).toBe('unrelated bytes'); }); it('restores a replacement captured at the cleanup boundary', () => { @@ -55,15 +61,36 @@ it('restores a replacement captured at the cleanup boundary', () => { writeFileSync(from, 'unrelated bytes'); actual.renameSync(from, to); }); - removeJournaledCatchupPartial(filePath, proof); + removeJournaledCatchupPartial(filePath, proof, recordCapture); expect(readFileSync(filePath + '.part', 'utf8')).toBe('unrelated bytes'); }); -it('restores an owned partial and reports an I/O error for retry', () => { +it('retries a durable capture after an I/O error without needing hardlinks', () => { jest.mocked(unlinkSync).mockImplementationOnce(() => { throw Object.assign(new Error('locked'), { code: 'EACCES' }); }); - expect(() => removeJournaledCatchupPartial(filePath, proof)).toThrow( - 'locked' - ); + jest.mocked(linkSync).mockImplementation(() => { + throw Object.assign(new Error('unsupported'), { code: 'ENOTSUP' }); + }); + expect(() => + removeJournaledCatchupPartial(filePath, proof, recordCapture) + ).toThrow('locked'); + expect(() => lstatSync(filePath + '.part')).toThrow(); + expect(proof.partialCleanupPath).toBeDefined(); + expect(readFileSync(proof.partialCleanupPath!, 'utf8')).toBe('owned bytes'); + removeJournaledCatchupPartial(filePath, proof, recordCapture); + expect(() => lstatSync(proof.partialCleanupPath!)).toThrow(); + expect(linkSync).not.toHaveBeenCalled(); +}); +it('does not capture the entry when write-ahead persistence fails', () => { + expect(() => + removeJournaledCatchupPartial(filePath, proof, (capture) => { + expect(readFileSync(filePath + '.part', 'utf8')).toBe( + 'owned bytes' + ); + expect(() => lstatSync(capture)).toThrow(); + throw new Error('SQLITE_BUSY'); + }) + ).toThrow('SQLITE_BUSY'); + expect(renameSync).not.toHaveBeenCalled(); expect(readFileSync(filePath + '.part', 'utf8')).toBe('owned bytes'); }); diff --git a/apps/electron-backend/src/app/events/database/download-catchup-removal.ts b/apps/electron-backend/src/app/events/database/download-catchup-removal.ts index f851b1fce..395d0a7eb 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-removal.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-removal.ts @@ -1,3 +1,4 @@ +import { cleanupArchiveCapture } from './download-catchup-capture'; import { lstatSync, type Stats, @@ -8,16 +9,23 @@ import { rmdirSync, } from 'node:fs'; import { dirname, join } from 'node:path'; -import type { ArchiveDownloadProof } from './download-catchup-journal'; +import { + readArchiveFinalizations, + recordArchiveCleanupPath, + type ArchiveDownloadProof, +} from './download-catchup-journal'; +import type { DownloadsDatabase } from './download-task'; import type { ArchiveFileIdentity } from './download-catchup-output'; /** IPC removal stays synchronous after its runtime guard, like VOD cleanup. */ export function removeJournaledCatchupPartial( filePath: string | null, - proof: ArchiveDownloadProof | undefined + proof: ArchiveDownloadProof | undefined, + recordCapture: (path: string) => void ): void { // No proof means no authority to remove the retained entry. if (!filePath || !proof || proof.filePath !== filePath) return; + cleanupArchiveCapture(proof); const path = `${filePath}.part`; const matches = (file: Stats, identity: ArchiveFileIdentity) => file.isFile() && file.dev === identity.dev && file.ino === identity.ino; @@ -30,6 +38,7 @@ export function removeJournaledCatchupPartial( const directory = mkdtempSync(join(dirname(path), '.iptvnator-cleanup-')); const captured = join(directory, 'entry'); try { + recordCapture(captured); renameSync(path, captured); if (matches(lstatSync(captured), proof.partialIdentity)) { unlinkSync(captured); @@ -44,18 +53,6 @@ export function removeJournaledCatchupPartial( ); } } - } catch (error) { - // Keep owned files reachable for another Remove attempt after I/O errors. - try { - linkSync(captured, path); - unlinkSync(captured); - } catch { - console.warn( - '[Downloads] Partial retained for recovery:', - captured - ); - } - throw error; } finally { try { rmdirSync(directory); @@ -64,3 +61,27 @@ export function removeJournaledCatchupPartial( } } } + +/** Queue/paused cancellation uses the same durable ownership as explicit Remove. */ +export async function cleanupStoredCatchupPartial( + db: DownloadsDatabase, + downloadId: number, + filePath: string | null | undefined +): Promise { + if (!filePath) return true; + try { + const proof = (await readArchiveFinalizations(db, [downloadId])).get( + downloadId + ); + removeJournaledCatchupPartial(filePath, proof, (path) => { + if (proof) recordArchiveCleanupPath(db, downloadId, proof, path); + }); + return true; + } catch (error) { + console.error( + '[Downloads] Failed to clean canceled archive partial:', + error + ); + return false; + } +} diff --git a/apps/electron-backend/src/app/events/database/download-catchup-runtime.spec.ts b/apps/electron-backend/src/app/events/database/download-catchup-runtime.spec.ts index f539353b9..943e68441 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-runtime.spec.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-runtime.spec.ts @@ -32,6 +32,7 @@ jest.mock('./download-catchup-cleanup', () => { }); jest.mock('./download-catchup-journal', () => ({ recordArchiveFinalization: jest.fn().mockResolvedValue(undefined), + recordArchiveCleanupPath: jest.fn(), clearArchiveFinalization: jest.fn().mockResolvedValue(undefined), readArchiveFinalizations: jest.fn().mockResolvedValue(new Map()), })); @@ -287,3 +288,72 @@ it('detaches a rejected replacement so Retry can reserve a fresh path', async () await rm(directory, { recursive: true, force: true }); } }); + +it.each([false, true])( + 'paused cancellation honors durable ownership (replaced=%s)', + async (replaced) => { + const directory = await mkdtemp( + join(tmpdir(), 'archive-cancel-owned-') + ); + const filePath = join(directory, 'show.ts'); + try { + await writeFile(filePath + '.part', 'owned archive'); + const original = await lstat(filePath + '.part'); + jest.mocked(readArchiveFinalizations).mockResolvedValueOnce( + new Map([ + [ + 995, + { + version: 1, + phase: 'transfer', + filePath, + partialIdentity: original, + }, + ], + ]) + ); + if (replaced) { + await rename(filePath + '.part', join(directory, 'original')); + await writeFile(filePath + '.part', 'unrelated file'); + } + const updates: Record[] = []; + const db = { + select: () => ({ + from: () => ({ + where: () => ({ + limit: async () => [ + { + filePath, + status: 'paused', + contentType: 'catchup', + }, + ], + }), + }), + }), + update: () => ({ + set: (value: Record) => ({ + where: async () => { + updates.push(value); + }, + }), + }), + }; + jest.mocked(getDatabase).mockResolvedValue(db as never); + await expect(cancelDownload(995)).resolves.toBe(true); + expect(updates).toContainEqual( + expect.objectContaining({ status: 'canceled', filePath: null }) + ); + if (replaced) + expect(await readFile(filePath + '.part', 'utf8')).toBe( + 'unrelated file' + ); + else + await expect(lstat(filePath + '.part')).rejects.toMatchObject({ + code: 'ENOENT', + }); + } finally { + await rm(directory, { recursive: true, force: true }); + } + } +); diff --git a/apps/electron-backend/src/app/events/database/download-redownload.spec.ts b/apps/electron-backend/src/app/events/database/download-redownload.spec.ts index cd12f9cf1..abd6fcb76 100644 --- a/apps/electron-backend/src/app/events/database/download-redownload.spec.ts +++ b/apps/electron-backend/src/app/events/database/download-redownload.spec.ts @@ -68,7 +68,10 @@ async function setup(options: SetupOptions = {}) { }); const enqueueDownload = jest.fn(); const proof = { version: 1, phase: 'transfer' }; - const removeJournaledCatchupPartial = jest.fn(); + const removeJournaledCatchupPartial = jest.fn((_path, _proof, record) => + record('/downloads/.iptvnator-cleanup-test/entry') + ); + const recordArchiveCleanupPath = jest.fn(); jest.doMock('node:fs', () => ({ ...jest.requireActual('node:fs'), @@ -81,6 +84,7 @@ async function setup(options: SetupOptions = {}) { jest.doMock('../url-safety', () => ({ assertRemoteUrlAllowed })); jest.doMock('./download-file-path', () => ({ removePartialDownloadFile })); jest.doMock('./download-catchup-journal', () => ({ + recordArchiveCleanupPath, readArchiveFinalizations: jest .fn() .mockResolvedValue(new Map([[42, proof]])), @@ -148,7 +152,8 @@ describe('redownload missing completed file', () => { }); expect(h.removeJournaledCatchupPartial).toHaveBeenCalledWith( '/downloads/movie.mp4', - h.proof + h.proof, + expect.any(Function) ); expect(h.removePartialDownloadFile).not.toHaveBeenCalled(); expect(h.set).toHaveBeenCalledWith( diff --git a/apps/electron-backend/src/app/events/database/download-redownload.ts b/apps/electron-backend/src/app/events/database/download-redownload.ts index a430fffd0..781988c32 100644 --- a/apps/electron-backend/src/app/events/database/download-redownload.ts +++ b/apps/electron-backend/src/app/events/database/download-redownload.ts @@ -1,4 +1,7 @@ -import { readArchiveFinalizations } from './download-catchup-journal'; +import { + readArchiveFinalizations, + recordArchiveCleanupPath, +} from './download-catchup-journal'; import { removeJournaledCatchupPartial } from './download-catchup-removal'; import { catchupForDownload } from './download-catchup'; import { and, eq, sql } from 'drizzle-orm'; @@ -72,7 +75,9 @@ export async function redownloadMissingRequest( const proof = (await readArchiveFinalizations(db, [item.id])).get( item.id ); - removeJournaledCatchupPartial(item.filePath, proof); + removeJournaledCatchupPartial(item.filePath, proof, (path) => { + if (proof) recordArchiveCleanupPath(db, item.id, proof, path); + }); } else removePartialDownloadFile(item.filePath); } catch (error) { console.error( diff --git a/apps/electron-backend/src/app/events/database/download-removal-requests.ts b/apps/electron-backend/src/app/events/database/download-removal-requests.ts index 794ced90c..9685203ad 100644 --- a/apps/electron-backend/src/app/events/database/download-removal-requests.ts +++ b/apps/electron-backend/src/app/events/database/download-removal-requests.ts @@ -1,7 +1,10 @@ import { and, eq, inArray } from 'drizzle-orm'; import { getDatabase } from '../../database/connection'; import * as schema from '../../database/schema'; -import { readArchiveFinalizations } from './download-catchup-journal'; +import { + readArchiveFinalizations, + recordArchiveCleanupPath, +} from './download-catchup-journal'; import { removeJournaledCatchupPartial } from './download-catchup-removal'; import { removePartialDownloadFile } from './download-file-path'; import { @@ -47,7 +50,19 @@ export async function removeDownloadRequest(downloadId: number) { if (row?.filePath && removablePartialStatuses.has(row.status)) { try { if (row.contentType === 'catchup') - removeJournaledCatchupPartial(row.filePath, proof); + removeJournaledCatchupPartial( + row.filePath, + proof, + (path) => { + if (proof) + recordArchiveCleanupPath( + db, + downloadId, + proof, + path + ); + } + ); else removePartialDownloadFile(row.filePath); } catch (cleanupError) { // Keep the row (and its runtime entry) so the .part is never @@ -115,7 +130,17 @@ export async function clearCompletedDownloadsRequest(playlistId?: string) { if (row.contentType === 'catchup') removeJournaledCatchupPartial( row.filePath, - proofs.get(row.id) + proofs.get(row.id), + (path) => { + const proof = proofs.get(row.id); + if (proof) + recordArchiveCleanupPath( + db, + row.id, + proof, + path + ); + } ); else removePartialDownloadFile(row.filePath); } catch (error) { diff --git a/apps/electron-backend/src/app/events/database/download-runtime.ts b/apps/electron-backend/src/app/events/database/download-runtime.ts index d306c6388..5b056472e 100644 --- a/apps/electron-backend/src/app/events/database/download-runtime.ts +++ b/apps/electron-backend/src/app/events/database/download-runtime.ts @@ -1,9 +1,10 @@ +import { cleanupArchiveCapture } from './download-catchup-capture'; import { reserveFreshCatchupTarget } from './download-catchup-reservation'; import { clearArchiveFinalization, readArchiveFinalizations, } from './download-catchup-journal'; -import { cleanupSelectedCatchupPartial } from './download-catchup-cleanup'; +import { cleanupStoredCatchupPartial } from './download-catchup-removal'; import { persistCancellation, persistPause, @@ -93,10 +94,14 @@ export async function cancelDownload(downloadId: number): Promise { ); if (queueIndex !== -1) { const [queuedTask] = downloadQueue.splice(queueIndex, 1); - const removed = queuedTask?.catchup - ? await cleanupSelectedCatchupPartial(queuedTask.filePath) - : removePartialFile(queuedTask?.filePath); const db = await getDatabase(); + const removed = queuedTask?.catchup + ? await cleanupStoredCatchupPartial( + db, + downloadId, + queuedTask.filePath + ) + : removePartialFile(queuedTask?.filePath); await persistQueuedCancellation( db, downloadId, @@ -123,7 +128,7 @@ export async function cancelDownload(downloadId: number): Promise { const removed = item.contentType === 'catchup' - ? await cleanupSelectedCatchupPartial(item.filePath) + ? await cleanupStoredCatchupPartial(db, downloadId, item.filePath) : removePartialFile(item.filePath); await persistQueuedCancellation( db, @@ -220,6 +225,7 @@ async function startDownload(task: DownloadTask): Promise { const proof = ( await readArchiveFinalizations(db, [task.id]) ).get(task.id); + cleanupArchiveCapture(proof); task.catchupExpectedPartialIdentity = proof?.partialIdentity; // No durable ownership evidence: preserve the old entry and // reserve a fresh destination instead of adopting it. diff --git a/apps/electron-backend/src/app/events/database/downloads.events.spec.ts b/apps/electron-backend/src/app/events/database/downloads.events.spec.ts index 0fdb47cc5..aa79414fa 100644 --- a/apps/electron-backend/src/app/events/database/downloads.events.spec.ts +++ b/apps/electron-backend/src/app/events/database/downloads.events.spec.ts @@ -8,6 +8,7 @@ import { mockHasRuntimeDownload, mockRemoveJournaledPartial, mockArchiveProofs, + mockRecordArchiveCleanupPath, mockRemovePartialDownloadFile, mockTerminalRows, setupDownloadsEventsHarness, @@ -52,15 +53,25 @@ describe('downloads events: partial-file cleanup', () => { contentType: 'catchup', }); const proof = { version: 1, phase: 'transfer' }; + mockRemoveJournaledPartial.mockImplementation((_path, _proof, record) => + record('/downloads/.iptvnator-cleanup-test/entry') + ); mockArchiveProofs.mockResolvedValue(new Map([[42, proof]])); await expect(getHandler('DOWNLOADS_REMOVE')(null, 42)).resolves.toEqual( { success: true } ); expect(mockRemoveJournaledPartial).toHaveBeenCalledWith( '/downloads/resume.mp4', - proof + proof, + expect.any(Function) ); expect(mockRemovePartialDownloadFile).not.toHaveBeenCalled(); + expect(mockRecordArchiveCleanupPath).toHaveBeenCalledWith( + expect.anything(), + 42, + proof, + '/downloads/.iptvnator-cleanup-test/entry' + ); expect(deleteWhere).toHaveBeenCalled(); }); @@ -69,13 +80,23 @@ describe('downloads events: partial-file cleanup', () => { { ...createDownloadRow('canceled'), contentType: 'catchup' }, ]); const proof = { version: 1, phase: 'transfer' }; + mockRemoveJournaledPartial.mockImplementation((_path, _proof, record) => + record('/downloads/.iptvnator-cleanup-test/entry') + ); mockArchiveProofs.mockResolvedValue(new Map([[1, proof]])); await expect( getHandler('DOWNLOADS_CLEAR_COMPLETED')(null) ).resolves.toEqual({ success: true }); expect(mockRemoveJournaledPartial).toHaveBeenCalledWith( '/downloads/resume.mp4', - proof + proof, + expect.any(Function) + ); + expect(mockRecordArchiveCleanupPath).toHaveBeenCalledWith( + expect.anything(), + 1, + proof, + '/downloads/.iptvnator-cleanup-test/entry' ); expect(mockRemovePartialDownloadFile).not.toHaveBeenCalled(); }); diff --git a/apps/electron-backend/src/app/events/database/downloads.test-helpers.ts b/apps/electron-backend/src/app/events/database/downloads.test-helpers.ts index dfc298fbd..c6aea7b09 100644 --- a/apps/electron-backend/src/app/events/database/downloads.test-helpers.ts +++ b/apps/electron-backend/src/app/events/database/downloads.test-helpers.ts @@ -14,6 +14,7 @@ export const mockRemoveDownloadFromRuntime = jest.fn(); export const mockIsDownloadCommitting = jest.fn(); export const mockHasRuntimeDownload = jest.fn(); export const mockArchiveProofs = jest.fn(); +export const mockRecordArchiveCleanupPath = jest.fn(); export const mockRemoveJournaledPartial = jest.fn(); export const mockBroadcastDownloadUpdate = jest.fn(); export const mockRemovePartialDownloadFile = jest.fn(); @@ -60,6 +61,7 @@ export async function setupDownloadsEventsHarness(): Promise { mockIsDownloadCommitting.mockReset().mockReturnValue(false); mockHasRuntimeDownload.mockReset().mockReturnValue(false); mockArchiveProofs.mockReset().mockResolvedValue(new Map()); + mockRecordArchiveCleanupPath.mockReset(); mockRemoveJournaledPartial.mockReset(); mockBroadcastDownloadUpdate.mockReset(); mockRemovePartialDownloadFile.mockReset(); @@ -121,6 +123,7 @@ export async function setupDownloadsEventsHarness(): Promise { })); jest.doMock('./download-catchup-journal', () => ({ readArchiveFinalizations: mockArchiveProofs, + recordArchiveCleanupPath: mockRecordArchiveCleanupPath, })); jest.doMock('./download-catchup-removal', () => ({ removeJournaledCatchupPartial: mockRemoveJournaledPartial, diff --git a/docs/architecture/download-manager.md b/docs/architecture/download-manager.md index c22df9cef..f0ed0fa12 100644 --- a/docs/architecture/download-manager.md +++ b/docs/architecture/download-manager.md @@ -89,7 +89,11 @@ overwritten by completion. Remove rejects this committing row before partial cleanup or deletion, and Clear completed skips it, preserving the cascading journal until completion finishes. Remove, Clear completed and missing-file re-download use journal-backed private capture for archive partial cleanup; -unknown or replaced entries are preserved. Cleanup remains synchronous after the +unknown or replaced entries are preserved. Before capture, a synchronous SQLite +write records `partialCleanupPath` in the existing ownership proof. A failed +unlink keeps that durable pointer; Remove/Clear, Retry/Resume and fresh +reservations retry identity-verified cleanup, including after restart and on +filesystems without hardlinks. Cleanup remains synchronous after the runtime guard, so a completion transition cannot interleave with unlink. Missing archive re-downloads claim a fresh reservation instead of inheriting the completed file's identity. @@ -100,8 +104,8 @@ numbered free destination. SQLite and filesystem creation cannot commit atomically, and portable rename cannot guarantee no-clobber publication on the filesystems that need this fallback. This bounded orphan is preferred to deleting or overwriting an unrelated file. -An explicit cancellation of a queued/paused archive captures the selected regular -partial using the same cleanup helper; symlink entries are preserved. +Cancellation of queued/paused archives uses the same journal-backed cleanup as +Remove; unproven or replaced entries, including symlinks, are preserved. A retained archive cannot use the VOD byte-count completion shortcut. Transfers have a 30-second idle timeout and a total deadline of twice programme duration plus ten minutes, capped at 24 hours. Transfers also stop at the smallest of a