fix(downloads): reconcile partial cleanup completion

CLAUDE.md

@@ -959,10 +959,12 @@ engine` (restart required) or
   stalled native work. Only `ENOENT` and `ENOTDIR` prove absence; permission,
   I/O, and other filesystem errors remain unknown and cannot clear a completed
   row. Before a completed-missing, failed, or canceled row clears its retained
-  path, the start IPC asynchronously removes any `.part` through a separate
-  one-second, same-path-coalesced, four-operation cap. Timeout or non-absence
-  errors keep the row's ownership intact; `ENOENT` and `ENOTDIR` safely proceed.
-  Episode and season download actions require an authoritative global list. A
+  path, the start IPC asynchronously removes any `.part` through a separate,
+  same-path-coalesced, four-operation cap. A one-second admission deadline
+  rejects queued work before unlink starts; started work is awaited so it cannot
+  mutate after a failure response. Non-absence errors keep the row's ownership
+  intact; `ENOENT` and `ENOTDIR` safely proceed. Episode and season download
+  actions require an authoritative global list. A
   successful snapshot remains authoritative while a later background refresh
   is in flight; a latest refresh failure leaves
   loading/empty-state resolution intact but disables starts until another

apps/electron-backend/src/app/events/database/download-partial-cleanup.spec.ts

@@ -1,6 +1,6 @@
 import {
     createPartialDownloadCleanup,
-    removePartialDownloadFileWithTimeoutAsync,
+    removePartialDownloadFileAsync,
 } from './download-partial-cleanup';
 
 describe('bounded partial download cleanup', () => {
@@ -19,7 +19,7 @@ describe('bounded partial download cleanup', () => {
             const cleanup = createPartialDownloadCleanup(unlink);
 
             await expect(
-                removePartialDownloadFileWithTimeoutAsync(
+                removePartialDownloadFileAsync(
                     '/downloads/episode.mp4',
                     25,
                     cleanup
@@ -34,7 +34,7 @@ describe('bounded partial download cleanup', () => {
         const cleanup = createPartialDownloadCleanup(unlink);
 
         await expect(
-            removePartialDownloadFileWithTimeoutAsync(
+            removePartialDownloadFileAsync(
                 '/downloads/episode.mp4',
                 25,
                 cleanup
@@ -43,7 +43,7 @@ describe('bounded partial download cleanup', () => {
         expect(unlink).toHaveBeenCalledWith('/downloads/episode.mp4.part');
     });
 
-    it('bounds each caller while retaining and coalescing the raw cleanup', async () => {
+    it('waits for a started unlink and coalesces same-path callers', async () => {
         jest.useFakeTimers();
         try {
             let finish: (() => void) | undefined;
@@ -54,26 +54,67 @@ describe('bounded partial download cleanup', () => {
                     })
             );
             const cleanup = createPartialDownloadCleanup(unlink, 1);
-            const first = removePartialDownloadFileWithTimeoutAsync(
+            const first = removePartialDownloadFileAsync(
                 '/downloads/episode.mp4',
                 25,
                 cleanup
             );
-            const second = removePartialDownloadFileWithTimeoutAsync(
+            const second = removePartialDownloadFileAsync(
                 '/downloads/episode.mp4',
                 50,
                 cleanup
             );
+            let firstSettled = false;
+            void first.then(() => {
+                firstSettled = true;
+            });
 
             await jest.advanceTimersByTimeAsync(25);
-            await expect(first).resolves.toBe('unknown');
+            expect(firstSettled).toBe(false);
             expect(unlink).toHaveBeenCalledTimes(1);
 
             finish?.();
+            await expect(first).resolves.toBe('removed');
             await expect(second).resolves.toBe('removed');
             expect(unlink).toHaveBeenCalledTimes(1);
         } finally {
             jest.useRealTimers();
         }
     });
+
+    it('times out queued cleanup without unlinking it later', async () => {
+        jest.useFakeTimers();
+        try {
+            let finishFirst: (() => void) | undefined;
+            const unlink = jest.fn((partialPath: string) =>
+                partialPath.includes('first')
+                    ? new Promise<void>((resolve) => {
+                          finishFirst = resolve;
+                      })
+                    : Promise.resolve()
+            );
+            const cleanup = createPartialDownloadCleanup(unlink, 1);
+            const first = removePartialDownloadFileAsync(
+                '/downloads/first.mp4',
+                25,
+                cleanup
+            );
+            const queued = removePartialDownloadFileAsync(
+                '/downloads/queued.mp4',
+                25,
+                cleanup
+            );
+
+            await jest.advanceTimersByTimeAsync(25);
+            await expect(queued).resolves.toBe('unknown');
+            expect(unlink).toHaveBeenCalledTimes(1);
+
+            finishFirst?.();
+            await expect(first).resolves.toBe('removed');
+            await Promise.resolve();
+            expect(unlink).toHaveBeenCalledTimes(1);
+        } finally {
+            jest.useRealTimers();
+        }
+    });
 });

apps/electron-backend/src/app/events/database/download-partial-cleanup.ts

@@ -6,7 +6,8 @@ export type DownloadPartialCleanupResult = 'removed' | 'missing' | 'unknown';
 export type DownloadAsyncUnlink = (filePath: string) => Promise<void>;
 
 export type DownloadPartialCleanup = (
-    filePath: string
+    filePath: string,
+    admissionTimeoutMs?: number
 ) => Promise<DownloadPartialCleanupResult>;
 
 const DEFAULT_MAX_CONCURRENT_PARTIAL_CLEANUPS = 4;
@@ -26,15 +27,39 @@ export function createPartialDownloadCleanup(
 ): DownloadPartialCleanup {
     const concurrency = Math.max(1, Math.floor(maxConcurrent));
     const pending: Array<() => void> = [];
+    // Only admission can time out. Once unlink starts, every coalesced caller
+    // awaits its authoritative result so no late deletion can race a retry.
     const inFlight = new Map<string, Promise<DownloadPartialCleanupResult>>();
     let active = 0;
 
-    const acquire = (): Promise<void> => {
+    const acquire = (timeoutMs: number): Promise<boolean> => {
         if (active < concurrency) {
             active += 1;
-            return Promise.resolve();
+            return Promise.resolve(true);
         }
-        return new Promise((resolve) => pending.push(resolve));
+        return new Promise((resolve) => {
+            let settled = false;
+            const start = () => {
+                if (settled) {
+                    return;
+                }
+                settled = true;
+                clearTimeout(timeout);
+                resolve(true);
+            };
+            const timeout = setTimeout(() => {
+                if (settled) {
+                    return;
+                }
+                settled = true;
+                const index = pending.indexOf(start);
+                if (index >= 0) {
+                    pending.splice(index, 1);
+                }
+                resolve(false);
+            }, timeoutMs);
+            pending.push(start);
+        });
     };
 
     const release = (): void => {
@@ -47,9 +72,12 @@ export function createPartialDownloadCleanup(
     };
 
     const remove = async (
-        filePath: string
+        filePath: string,
+        admissionTimeoutMs: number
     ): Promise<DownloadPartialCleanupResult> => {
-        await acquire();
+        if (!(await acquire(admissionTimeoutMs))) {
+            return 'unknown';
+        }
         try {
             await asyncUnlink(getPartialDownloadPath(filePath));
             return 'removed';
@@ -60,13 +88,16 @@ export function createPartialDownloadCleanup(
         }
     };
 
-    return (filePath: string) => {
+    return (
+        filePath: string,
+        admissionTimeoutMs = DEFAULT_PARTIAL_CLEANUP_TIMEOUT_MS
+    ) => {
         const existing = inFlight.get(filePath);
         if (existing) {
             return existing;
         }
 
-        const cleanup = remove(filePath);
+        const cleanup = remove(filePath, admissionTimeoutMs);
         inFlight.set(filePath, cleanup);
         const clear = () => {
             if (inFlight.get(filePath) === cleanup) {
@@ -80,31 +111,23 @@ export function createPartialDownloadCleanup(
 
 const cleanupPartialDownloadFile = createPartialDownloadCleanup();
 
-export async function removePartialDownloadFileWithTimeoutAsync(
+export async function removePartialDownloadFileAsync(
     filePath: string | null | undefined,
-    timeoutMs = DEFAULT_PARTIAL_CLEANUP_TIMEOUT_MS,
+    admissionTimeoutMs = DEFAULT_PARTIAL_CLEANUP_TIMEOUT_MS,
     cleanup: DownloadPartialCleanup = cleanupPartialDownloadFile
 ): Promise<DownloadPartialCleanupResult> {
     if (!filePath) {
         return 'missing';
     }
 
-    const boundedTimeoutMs =
-        Number.isFinite(timeoutMs) && timeoutMs >= 0
-            ? timeoutMs
+    const boundedAdmissionTimeoutMs =
+        Number.isFinite(admissionTimeoutMs) && admissionTimeoutMs >= 0
+            ? admissionTimeoutMs
             : DEFAULT_PARTIAL_CLEANUP_TIMEOUT_MS;
-    let timeout: ReturnType<typeof setTimeout> | undefined;
-    const timedOut = new Promise<'unknown'>((resolve) => {
-        timeout = setTimeout(() => resolve('unknown'), boundedTimeoutMs);
-    });
 
     try {
-        return await Promise.race([cleanup(filePath), timedOut]);
+        return await cleanup(filePath, boundedAdmissionTimeoutMs);
     } catch {
         return 'unknown';
-    } finally {
-        if (timeout !== undefined) {
-            clearTimeout(timeout);
-        }
     }
 }

apps/electron-backend/src/app/events/database/download-requests.spec.ts

@@ -921,7 +921,7 @@ describe('download requests resume', () => {
                 update: jest.fn(() => ({ set })),
             };
             const enqueueDownload = jest.fn();
-            const removePartialDownloadFileWithTimeoutAsync = jest.fn(
+            const removePartialDownloadFileAsync = jest.fn(
                 async () => 'removed' as const
             );
             const authorizer = {
@@ -937,7 +937,7 @@ describe('download requests resume', () => {
                 assertRemoteUrlAllowed: jest.fn().mockResolvedValue(undefined),
             }));
             jest.doMock('./download-partial-cleanup', () => ({
-                removePartialDownloadFileWithTimeoutAsync,
+                removePartialDownloadFileAsync,
             }));
             jest.doMock('./download-runtime', () => ({
                 enqueueDownload,
@@ -960,9 +960,9 @@ describe('download requests resume', () => {
                 )
             ).resolves.toEqual({ id: 42, success: true });
 
-            expect(
-                removePartialDownloadFileWithTimeoutAsync
-            ).toHaveBeenCalledWith('/downloads/movie.mp4');
+            expect(removePartialDownloadFileAsync).toHaveBeenCalledWith(
+                '/downloads/movie.mp4'
+            );
             expect(set).toHaveBeenCalledWith(
                 expect.objectContaining({
                     filePath: null,
@@ -1002,7 +1002,7 @@ describe('download requests resume', () => {
             update: jest.fn(() => ({ set })),
         };
         const enqueueDownload = jest.fn();
-        const removePartialDownloadFileWithTimeoutAsync = jest.fn(
+        const removePartialDownloadFileAsync = jest.fn(
             async () => 'unknown' as const
         );
         const authorizer = {
@@ -1016,7 +1016,7 @@ describe('download requests resume', () => {
             assertRemoteUrlAllowed: jest.fn().mockResolvedValue(undefined),
         }));
         jest.doMock('./download-partial-cleanup', () => ({
-            removePartialDownloadFileWithTimeoutAsync,
+            removePartialDownloadFileAsync,
         }));
         jest.doMock('./download-runtime', () => ({
             enqueueDownload,

apps/electron-backend/src/app/events/database/download-requests.ts

@@ -11,7 +11,7 @@ import * as schema from '../../database/schema';
 import { assertRemoteUrlAllowed } from '../url-safety';
 import { DownloadDirectoryAuthorizer } from './download-directory-authorization';
 import { getDownloadFileAvailabilityWithTimeoutAsync } from './download-file-availability';
-import { removePartialDownloadFileWithTimeoutAsync } from './download-partial-cleanup';
+import { removePartialDownloadFileAsync } from './download-partial-cleanup';
 import { resolveExistingDownloadIdentity } from './download-request-identity';
 import { resolveStoredDownloadHeaders } from './download-request-headers';
 import {
@@ -182,9 +182,7 @@ export async function startDownloadRequest(
             // A terminal row can still reference a retained .part; delete it
             // before the restart clears filePath, or the file is orphaned.
             // An unavailable or slow .part must keep its database owner.
-            const cleanup = await removePartialDownloadFileWithTimeoutAsync(
-                item.filePath
-            );
+            const cleanup = await removePartialDownloadFileAsync(item.filePath);
             if (cleanup === 'unknown') {
                 console.error(
                     '[Downloads] Could not verify retained partial cleanup'

docs/architecture/download-manager.md

@@ -111,11 +111,13 @@ variants, contextual buttons, and theme-aware styling.
   absence; permission, I/O, and other probe errors remain unknown, so
   `DOWNLOADS_START` leaves the completed row and file path untouched. Before a
   completed-missing, failed, or canceled row clears its path, the same start IPC
-  asynchronously removes any retained `.part`. This cleanup also has a
-  one-second per-caller deadline, coalesces same-path work, and allows at most
-  four underlying unlinks; timeout, permission, and I/O failures keep the row's
-  ownership intact, while `ENOENT` and `ENOTDIR` safely proceed. The coordinator
-  counts both stable duplicate reasons as skipped. There is no batch IPC,
+  asynchronously removes any retained `.part`. Cleanup coalesces same-path work
+  and allows at most four underlying unlinks. A one-second admission deadline
+  rejects queued work before it can mutate the filesystem; once an unlink
+  starts, the request awaits its authoritative result so no late side effect can
+  race a retry. Permission and I/O failures keep the row's ownership intact,
+  while `ENOENT` and `ENOTDIR` safely proceed. The coordinator counts both stable
+  duplicate reasons as skipped. There is no batch IPC,
   parallel transfer, or queue reordering: destination authorization, persisted
   header handling, and the backend's one-active-transfer FIFO semantics remain
   unchanged.