From e35b1af73101ddfb28ba50a96937b7ed348da2c6 Mon Sep 17 00:00:00 2001 From: NimBold Date: Wed, 15 Jul 2026 01:51:04 +0330 Subject: [PATCH] fix(downloads): close audit lifecycle gaps Prevent duplicate Add Window submissions and preserve discard confirmation. Reject duplicate backend primary-path ownership and clear stale lifecycle progress. --- src-tauri/src/db.rs | 32 +++++++++++++++++++++ src/components/AddDownloadsModal.tsx | 20 ++++++++++--- src/store/downloadStore.test.ts | 43 ++++++++++++++++++++++++++++ src/store/downloadStore.ts | 12 +++++--- 4 files changed, 99 insertions(+), 8 deletions(-) diff --git a/src-tauri/src/db.rs b/src-tauri/src/db.rs index 6be0bf1..31758d5 100644 --- a/src-tauri/src/db.rs +++ b/src-tauri/src/db.rs @@ -923,6 +923,20 @@ pub fn load_ownership(connection: &Connection) -> Result, } pub fn set_ownership(connection: &Connection, id: &str, path: &str) -> Result<(), String> { + let existing_owner = connection + .query_row( + "SELECT id FROM download_ownership + WHERE primary_path = ?1 AND id <> ?2 + LIMIT 1", + params![path, id], + |row| row.get::<_, String>(0), + ) + .optional() + .map_err(|error| format!("failed to check download ownership path: {error}"))?; + if existing_owner.is_some() { + return Err("Download destination is already owned by another Firelink download".to_string()); + } + connection .execute( "INSERT INTO download_ownership (id, primary_path) VALUES (?1, ?2) @@ -1668,4 +1682,22 @@ mod tests { acknowledge_pairing_token_notice(&connection).unwrap(); assert!(!has_pending_notice(&connection, TOKEN_CHANGED_NOTICE).unwrap()); } + + #[test] + fn rejects_two_download_ids_from_claiming_the_same_primary_path() { + let temp = TempDir::new().unwrap(); + let state = init_at_path(temp.path()).unwrap(); + let connection = state.lock().unwrap(); + + set_ownership(&connection, "first", "/downloads/file.bin").unwrap(); + let error = set_ownership(&connection, "second", "/downloads/file.bin") + .expect_err("a primary path must have one live owner"); + + assert!(error.contains("already owned")); + assert_eq!(load_ownership(&connection).unwrap(), vec![( + "first".to_string(), + "/downloads/file.bin".to_string() + )]); + set_ownership(&connection, "first", "/downloads/renamed.bin").unwrap(); + } } diff --git a/src/components/AddDownloadsModal.tsx b/src/components/AddDownloadsModal.tsx index b5240dd..3163327 100644 --- a/src/components/AddDownloadsModal.tsx +++ b/src/components/AddDownloadsModal.tsx @@ -101,6 +101,7 @@ export const AddDownloadsModal = () => { const [resolvedLocation, setResolvedLocation] = useState(''); const [isQueueMenuOpen, setIsQueueMenuOpen] = useState(false); const [isSubmitting, setIsSubmitting] = useState(false); + const isSubmittingRef = useRef(false); const actionMenuRef = useRef(null); // Right Form @@ -155,7 +156,7 @@ export const AddDownloadsModal = () => { }; const closeModalFromDismissAction = useCallback(() => { - if (isSubmitting) return; + if (isSubmitting || isSubmittingRef.current) return; const hasPendingInput = Boolean( urls.trim() || pendingAddUrls.trim() || parsedItems.length || headers.trim() || cookies.trim() ); @@ -206,6 +207,7 @@ export const AddDownloadsModal = () => { cookiesManuallyEditedRef.current = false; setMirrors(''); setIsQueueMenuOpen(false); + isSubmittingRef.current = false; setIsSubmitting(false); }, [ isAddModalOpen, @@ -496,13 +498,14 @@ export const AddDownloadsModal = () => { }; const handleAction = async (action: AddDownloadAction) => { - if (isSubmitting || !canSubmitMetadataRows(parsedItems)) { + if (isSubmitting || isSubmittingRef.current || !canSubmitMetadataRows(parsedItems)) { return; } if (speedLimitEnabled && (!Number.isFinite(Number(speedLimit)) || Number(speedLimit) <= 0)) { addToast({ message: 'Speed limit must be greater than zero', variant: 'error', isActionable: true }); return; } + isSubmittingRef.current = true; setIsSubmitting(true); let finalLocation = saveLocation; let useSharedDestination = isSaveLocationManual; @@ -525,11 +528,13 @@ export const AddDownloadsModal = () => { const approvedPath = await useSettingsStore.getState().approveDownloadRoot(selected); destinationOverrides[index] = approvedPath; } else { + isSubmittingRef.current = false; setIsSubmitting(false); return; } } catch (e) { console.error("Failed to select folder:", e); + isSubmittingRef.current = false; setIsSubmitting(false); return; } @@ -622,6 +627,7 @@ export const AddDownloadsModal = () => { setPendingUseSharedDestination(useSharedDestination); setPendingDestinationOverrides(destinationOverrides); setShowingDuplicates(true); + isSubmittingRef.current = false; setIsSubmitting(false); return; } @@ -629,6 +635,7 @@ export const AddDownloadsModal = () => { try { await executeAddDownloads(action, finalLocation, useSharedDestination, undefined, destinationOverrides); } finally { + isSubmittingRef.current = false; setIsSubmitting(false); } }; @@ -886,6 +893,8 @@ export const AddDownloadsModal = () => { { + if (isSubmittingRef.current) return; + isSubmittingRef.current = true; setShowingDuplicates(false); setIsSubmitting(true); void executeAddDownloads( @@ -902,7 +911,10 @@ export const AddDownloadsModal = () => { isActionable: true }); }) - .finally(() => setIsSubmitting(false)); + .finally(() => { + isSubmittingRef.current = false; + setIsSubmitting(false); + }); }} onCancel={() => setShowingDuplicates(false)} /> @@ -1238,7 +1250,7 @@ export const AddDownloadsModal = () => { {metadataSummaryMessage(parsedItems)}
-
diff --git a/src/store/downloadStore.test.ts b/src/store/downloadStore.test.ts index e65fb20..e8a45ca 100644 --- a/src/store/downloadStore.test.ts +++ b/src/store/downloadStore.test.ts @@ -113,4 +113,47 @@ describe('useDownloadProgressStore', () => { expect(useDownloadProgressStore.getState().progressMap).toEqual({}); release(); }); + + it('drops stale progress when a download returns to a queued lifecycle', async () => { + const handlers: Record void> = {}; + vi.mocked(ipc.listenEvent).mockImplementation((event, handler) => { + handlers[event] = handler as (event: any) => void; + return Promise.resolve(vi.fn()); + }); + useDownloadStore.setState({ + downloads: [{ + id: 'reused', + url: 'https://example.com/file', + fileName: 'file.bin', + status: 'downloading', + category: 'Other', + dateAdded: '' + }] + }); + + const release = await initDownloadListener(); + handlers['download-progress']({ payload: { + id: 'reused', + fraction: 0.8, + speed: '1 MB/s', + eta: '2s', + size: '8 MB', + size_is_final: false + } }); + handlers['download-state']({ payload: { + id: 'reused', + status: 'queued' + } }); + handlers['download-progress']({ payload: { + id: 'reused', + fraction: 0.9, + speed: '2 MB/s', + eta: '1s', + size: '9 MB', + size_is_final: false + } }); + + expect(useDownloadProgressStore.getState().progressMap).toEqual({}); + release(); + }); }); diff --git a/src/store/downloadStore.ts b/src/store/downloadStore.ts index 5dcca1d..26edab3 100644 --- a/src/store/downloadStore.ts +++ b/src/store/downloadStore.ts @@ -38,9 +38,10 @@ const startDownloadListeners = async () => { return; } // A sidecar can flush one last progress chunk after a pause, failure, - // or completion event. Do not let that stale chunk repopulate the live - // progress map or overwrite a later lifecycle's first frame. - if (current && ['completed', 'failed', 'paused'].includes(current.status)) { + // completion, or lifecycle reset. Do not let that stale chunk repopulate + // the live progress map or overwrite a later lifecycle's first frame. + if (!['downloading', 'processing'].includes(current.status)) { + useDownloadProgressStore.getState().clearDownloadProgress(payload.id); return; } useDownloadProgressStore.getState().updateDownloadProgress(payload.id, payload); @@ -74,6 +75,9 @@ const startDownloadListeners = async () => { return; } + if (['queued', 'retrying', 'completed', 'failed', 'paused'].includes(status)) { + useDownloadProgressStore.getState().clearDownloadProgress(payload.id); + } const progress = useDownloadProgressStore.getState().progressMap[payload.id]; const updates: Partial = { status, @@ -109,7 +113,7 @@ const startDownloadListeners = async () => { } else if (status === 'completed' || status === 'failed') { mainStore.unregisterBackendIds([payload.id]); } - if (status === 'completed' || status === 'failed' || status === 'paused') { + if (['queued', 'retrying', 'completed', 'failed', 'paused'].includes(status)) { useDownloadProgressStore.getState().clearDownloadProgress(payload.id); } }),