From 1dfafb404d2581f36293b2398a335ed0c106a7ec Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 8 Sep 2026 05:16:27 +0200 Subject: [PATCH] fix(downloads): bind fresh archive reservations to owned files --- AGENTS.md | 2 + CLAUDE.md | 2 + .../database/download-catchup-output.ts | 4 +- .../database/download-catchup-removal.spec.ts | 9 ++++ .../download-catchup-reservation.spec.ts | 51 ++++++++++++++++++- .../database/download-catchup-reservation.ts | 39 ++++++++++++-- .../database/download-catchup-runtime.spec.ts | 10 ++++ .../download-catchup-transfer.spec.ts | 1 + .../database/download-runtime-reservation.ts | 9 +++- docs/architecture/download-manager.md | 5 +- 10 files changed, 123 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index dfe8e4dd4..e07beac69 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1045,6 +1045,8 @@ so Retry can reserve a fresh path. A synchronous completion-commit boundary rejects late pause/cancel commands before awaited cleanup and persistence. Archive ownership includes device, inode and positive creation time to reject reused inodes after unlink; old proofs without creation time remain untrusted. +Fresh reservations capture and journal ownership before the initial HTTP wait; +no preexisting partial is truncated without matching expected ownership. Private cleanup captures are journaled before relocation, keeping failed Remove/Clear/cancel cleanup retryable across restarts without hardlinks. Active failures, promotion and startup share that cleanup; Remove waits for active diff --git a/CLAUDE.md b/CLAUDE.md index f8a19125a..22095d486 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1856,6 +1856,8 @@ so Retry can reserve a fresh path. A synchronous completion-commit boundary rejects late pause/cancel commands before awaited cleanup and persistence. Archive ownership includes device, inode and positive creation time to reject reused inodes after unlink; old proofs without creation time remain untrusted. +Fresh reservations capture and journal ownership before the initial HTTP wait; +no preexisting partial is truncated without matching expected ownership. Private cleanup captures are journaled before relocation, keeping failed Remove/Clear/cancel cleanup retryable across restarts without hardlinks. Active failures, promotion and startup share that cleanup; Remove waits for active diff --git a/apps/electron-backend/src/app/events/database/download-catchup-output.ts b/apps/electron-backend/src/app/events/database/download-catchup-output.ts index edd0ecf39..dcb80d09a 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-output.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-output.ts @@ -67,8 +67,8 @@ export async function openCatchupOutput( current.nlink !== 1 || (before && !sameArchiveFileIdentity(current, before)) || (before && - expectedIdentity && - !sameArchiveFileIdentity(current, expectedIdentity)) + (!expectedIdentity || + !sameArchiveFileIdentity(current, expectedIdentity))) ) { throw new ArchivePartialReplacedError(); } 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 9f88a79ea..16a8b41da 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 @@ -278,3 +278,12 @@ it('does not recover a same-size final from a reused inode or a legacy proof wit ) ).toBeUndefined(); }); + +it('never truncates a preexisting file without expected ownership', async () => { + const record = jest.fn(); + await expect( + openCatchupOutput(filePath + '.part', undefined, record) + ).rejects.toThrow('changed'); + expect(record).not.toHaveBeenCalled(); + expect(readFileSync(filePath + '.part', 'utf8')).toBe('owned bytes'); +}); diff --git a/apps/electron-backend/src/app/events/database/download-catchup-reservation.spec.ts b/apps/electron-backend/src/app/events/database/download-catchup-reservation.spec.ts index 3c3666fea..3b43a19ab 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-reservation.spec.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-reservation.spec.ts @@ -1,3 +1,4 @@ +import { openCatchupOutput } from './download-catchup-output'; import { mkdtemp, lstat, @@ -12,6 +13,7 @@ import { reserveTarget } from './download-runtime-reservation'; import { reserveFreshCatchupTarget } from './download-catchup-reservation'; import { clearArchiveFinalization, + recordArchivePartial, readArchiveFinalizations, } from './download-catchup-journal'; import type { DownloadsDatabase, DownloadTask } from './download-task'; @@ -21,6 +23,7 @@ jest.mock('./download-catchup-journal', () => ({ clearArchiveFinalization: jest.fn().mockResolvedValue(undefined), readArchiveFinalizations: jest.fn(), recordArchiveCleanupPath: jest.fn(), + recordArchivePartial: jest.fn().mockResolvedValue(undefined), })); it.each([false, true])( @@ -83,7 +86,13 @@ it.each([false, true])( await expect(lstat(filePath + '.part')).rejects.toMatchObject({ code: 'ENOENT', }); - expect(task.catchupExpectedPartialIdentity).toBeUndefined(); + expect( + await lstat(join(directory, 'show (1).ts.part')) + ).toEqual( + expect.objectContaining( + task.catchupExpectedPartialIdentity! + ) + ); } expect(await readFile(filePath, 'utf8')).toBe('unrelated final'); } finally { @@ -131,3 +140,43 @@ it('removes a journaled incomplete final before retrying the retained archive', await rm(directory, { recursive: true, force: true }); } }); + +it('binds a fresh reservation before a replacement can arrive during the HTTP wait', async () => { + const directory = await mkdtemp(join(tmpdir(), 'archive-fresh-owner-')); + const task: DownloadTask = { + id: 19, + directory, + fileName: 'show.ts', + url: 'https://provider.test/archive.ts', + catchup: { + channelName: 'News', + startTimestamp: 100, + stopTimestamp: 200, + }, + }; + try { + const reservation = await reserveTarget({} as DownloadsDatabase, task); + expect(recordArchivePartial).toHaveBeenCalledWith( + expect.anything(), + task.id, + reservation.path, + task.catchupExpectedPartialIdentity + ); + await rename(reservation.partialPath, join(directory, 'original')); + await writeFile( + reservation.partialPath, + 'foreign file created before the response' + ); + await expect( + openCatchupOutput( + reservation.partialPath, + task.catchupExpectedPartialIdentity + ) + ).rejects.toThrow('changed'); + expect(await readFile(reservation.partialPath, 'utf8')).toBe( + 'foreign file created before the response' + ); + } finally { + await rm(directory, { recursive: true, force: true }); + } +}); diff --git a/apps/electron-backend/src/app/events/database/download-catchup-reservation.ts b/apps/electron-backend/src/app/events/database/download-catchup-reservation.ts index 72df40b67..8040e9b10 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-reservation.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-reservation.ts @@ -1,8 +1,13 @@ +import { closeSync, fstatSync, openSync } from 'node:fs'; import { lstat } from 'node:fs/promises'; import { cleanupStoredCatchupPartial } from './download-catchup-removal'; -import { clearArchiveFinalization } from './download-catchup-journal'; +import { + clearArchiveFinalization, + recordArchivePartial, +} from './download-catchup-journal'; import { ArchivePartialReplacedError, + archiveFileIdentity, sameArchiveFileIdentity, } from './download-catchup-output'; import { reserveAvailablePartialDownloadFile } from './download-file-path'; @@ -34,6 +39,34 @@ export async function reserveFreshCatchupTarget( } } await clearArchiveFinalization(db, task.id); - task.catchupExpectedPartialIdentity = undefined; - return reserveAvailablePartialDownloadFile(task.directory, task.fileName); + return reserveOwnedCatchupTarget(db, task); +} + +/** Capture ownership from the exclusive creation descriptor, before any network wait. */ +export async function reserveOwnedCatchupTarget( + db: DownloadsDatabase, + task: DownloadTask +) { + task.catchupExpectedPartialIdentity = undefined; + const reservation = reserveAvailablePartialDownloadFile( + task.directory, + task.fileName, + (partialPath) => { + const descriptor = openSync(partialPath, 'wx', 0o600); + try { + task.catchupExpectedPartialIdentity = archiveFileIdentity( + fstatSync(descriptor) + ); + } finally { + closeSync(descriptor); + } + } + ); + task.filePath = reservation.path; + task.fileName = reservation.filename; + const identity = task.catchupExpectedPartialIdentity; + if (!identity) + throw new Error('Archive reservation identity is unavailable'); + await recordArchivePartial(db, task.id, reservation.path, identity); + return reservation; } 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 a47101fba..dfac94785 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 @@ -49,6 +49,16 @@ function mockRememberCapture( } jest.mock('./download-catchup-journal', () => ({ ...jest.requireActual('./download-catchup-journal'), + recordArchivePartial: jest.fn( + async (_db, id, filePath, partialIdentity) => { + mockProofs.set(id, { + version: 1, + phase: 'transfer', + filePath, + partialIdentity, + }); + } + ), recordArchiveFinalization: jest.fn(async (_db, id, proof) => { mockProofs.set(id, proof); }), diff --git a/apps/electron-backend/src/app/events/database/download-catchup-transfer.spec.ts b/apps/electron-backend/src/app/events/database/download-catchup-transfer.spec.ts index 51482688e..52dac1736 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-transfer.spec.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-transfer.spec.ts @@ -123,6 +123,7 @@ describe('TS archive transfer', () => { }; try { await writeFile(path + '.part', 'old data'); + task.catchupExpectedPartialIdentity = await stat(path + '.part'); jest.mocked(recordArchivePartial).mockImplementationOnce( async (_db, id, filePath, identity) => { expect(filePath).toBe(path); diff --git a/apps/electron-backend/src/app/events/database/download-runtime-reservation.ts b/apps/electron-backend/src/app/events/database/download-runtime-reservation.ts index a853478a1..fd8743722 100644 --- a/apps/electron-backend/src/app/events/database/download-runtime-reservation.ts +++ b/apps/electron-backend/src/app/events/database/download-runtime-reservation.ts @@ -5,7 +5,10 @@ import { verifiedArchiveSize, } from './download-catchup-journal'; import { cleanupStoredCatchupFinal } from './download-catchup-removal'; -import { reserveFreshCatchupTarget } from './download-catchup-reservation'; +import { + reserveFreshCatchupTarget, + reserveOwnedCatchupTarget, +} from './download-catchup-reservation'; import { findAvailableFinalPath, getPartialDownloadPath, @@ -62,5 +65,7 @@ export async function reserveTarget( }; } - return reserveAvailablePartialDownloadFile(task.directory, task.fileName); + return task.catchup + ? reserveOwnedCatchupTarget(db, task) + : reserveAvailablePartialDownloadFile(task.directory, task.fileName); } diff --git a/docs/architecture/download-manager.md b/docs/architecture/download-manager.md index f05efb3bb..a59339f94 100644 --- a/docs/architecture/download-manager.md +++ b/docs/architecture/download-manager.md @@ -102,7 +102,10 @@ re-download and repeated programme submissions use journal-backed private capture for archive partial cleanup; unknown or replaced entries are preserved. Ownership includes a positive file creation timestamp as well as device/inode, so inode reuse after unlink cannot -bless a new entry; proofs lacking creation time remain untrusted. Before capture, a synchronous SQLite +bless a new entry; proofs lacking creation time remain untrusted. Fresh +reservations capture this identity from their exclusive creation descriptor and +journal it before the HTTP wait. Existing partials without matching expected +ownership are never truncated, including before the first response. Before capture, a synchronous SQLite write records `partialCleanupPath` (or `finalCleanupPath` for failed promotion) in the existing ownership proof. Active cancellation/failure, promotion and startup recovery use the same journal-backed cleanup as manual actions. Cancel,