fix: stop direct/mirror download race from clobbering destination file

[rainxchzed]

Summary

Users with a custom download mirror configured were hitting Error: file not ready after download: <path> on installs.

MultiSourceDownloaderImpl races a direct download against a mirror download, both calling Downloader.download(url, suggestedFileName) with the same suggestedFileName. Both AndroidDownloader and DesktopDownloader resolved that name to the same destination File and wrote to it directly. Two consequences:

1. The non-atomic containsKey + put guard in AndroidDownloader let both jobs proceed concurrently and write to the same path.
2. When the loser was cancelled, its catch block ran destination.delete() — which removed the winner's just-written file before the winner's post-write destination.exists() && length() > 0 check, throwing "File not ready after download".

This change makes each attempt write to its own per-downloadId temp file (<safeName>.part-<downloadId>), then atomic-rename to the final destination on success. Loser cleanup deletes only its own temp; the winner's destination is never touched by the other job.

While here:

• Drop the "already in progress" guard in both downloaders — MultiSourceDownloader legitimately runs two parallel attempts under the same logical name; the temp-file uniqueness now makes the guard unnecessary.
• Switch the per-fileName tracking map to hold a set of in-flight downloadIds. cancelDownload(fileName) now cancels every active call for that name (it previously cancelled only one of the two race participants).
• Stop cancelDownload from blindly deleting the destination file — with atomic-rename, the destination only exists when a download has already succeeded; deleting it would wipe a perfectly valid prior download.
• Tighten the empty-file check error to include contentLength so future zero-byte responses are easier to diagnose.

Test plan

• With a custom download mirror configured (Profile → Proxy → Download mirror), install several apps. Previously failed with "File not ready after download"; should now succeed.
• Without a mirror configured, regular installs still work.
• Cancel an in-progress download from the UI; the partially-written temp file (*.part-*) is removed and a prior successful download with the same filename is retained.
• Re-install/update an app that already exists in the downloads dir; new bytes replace the old file via atomic move.
• Desktop builds (Windows/macOS/Linux): downloads still work end-to-end.

Summary by CodeRabbit

Bug Fixes

• Improved download reliability with atomic file operations and validation to prevent incomplete files
• Enhanced support for concurrent downloads of the same file
• Better cleanup of failed download attempts

[coderabbitai]

Walkthrough

Both Android and Desktop downloader implementations now support concurrent downloads with atomic file writes. Download attempts write to temporary per-download files, validate non-empty results, and atomically move to final destinations. Concurrent tracking changed from single-ID per filename to per-name ID sets, and cancellation now terminates all active downloads for a given filename.

Changes

Concurrent Download Support with Atomic Writes

Layer / File(s)	Summary
Concurrent Download Tracking core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidDownloader.kt (line 35), core/data/src/jvmMain/kotlin/zed/rainxch/core/data/services/DesktopDownloader.kt (lines 29–33)	Replace single ID-per-filename mappings with idsByName: Map<String, MutableSet<String>> to track multiple concurrent download IDs per logical filename.
Download Implementation & Temp File Handling core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidDownloader.kt (lines 117–186), core/data/src/jvmMain/kotlin/zed/rainxch/core/data/services/DesktopDownloader.kt (lines 35–171)	Each download attempt writes to a per-attempt temp file ($safeName.part-$downloadId). After download completes, temp file is validated as non-empty, then atomically moved to the final destination. Progress emission uses final file size. Cleanup on failure deletes the temp file instead of the destination.
Atomic Move Helper core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidDownloader.kt (lines 188–203), core/data/src/jvmMain/kotlin/zed/rainxch/core/data/services/DesktopDownloader.kt (lines 173–184)	New private moveAtomic(source, target) helper performs atomic file move with fallback to non-atomic move when AtomicMoveNotSupportedException is thrown.
Cancellation & Lifecycle core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidDownloader.kt (lines 247–266), core/data/src/jvmMain/kotlin/zed/rainxch/core/data/services/DesktopDownloader.kt (lines 221–237)	cancelDownload(fileName) now cancels all in-flight download IDs associated with that filename via idsByName lookup. Cleanup no longer deletes the destination file on cancellation; temp file cleanup is delegated to error/catch handlers.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

• Replace Android DownloadManager with OkHttp and refactor downloader i… #316: Directly modified the same downloader implementations with single-ID-per-filename tracking and OkHttp integration; this PR extends that work with atomic writes and multi-ID concurrent tracking.
• refactor: update download directory to include app subfolder in deskt… #298: Both PRs modify Android and Desktop downloader logic; this PR's temp-file and atomic-move changes affect the skip-download and error-handling paths.
• refactor(download): Skip download if file already exists and improve … #154: Both PRs refactor downloader file-handling and cleanup lifecycle; the temp-file validation and deletion patterns introduced here interact with any skip-logic added elsewhere.

Poem

🐰 Atomic hops across the file,
Temp parts dance in granular style,
No more clashing, safe and sound,
Each concurrent download's found.
Cleanup whispers, "temp goes bye,"
Finally lands where downloads lie! 🏁

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main fix: preventing race conditions between concurrent downloads from overwriting the destination file.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/multi-source-download-race

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

core/data/src/jvmMain/kotlin/zed/rainxch/core/data/services/DesktopDownloader.kt (1)

204-207: 💤 Low value

Consider: pre-download deletion creates a brief unavailability window.

saveToFile deletes the existing file before starting the download, then download() writes to a temp file and atomically renames. This creates a window where the file doesn't exist at all. If another component checks for the file during download, it won't find it.

If this is intentional (ensuring a completely fresh download), the current behavior is fine. If availability during re-download matters, consider letting the atomic rename handle replacement instead.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@core/data/src/jvmMain/kotlin/zed/rainxch/core/data/services/DesktopDownloader.kt`
around lines 204 - 207, The current pre-download deletion in saveToFile (the
file.delete() block) creates a brief absence window before download() writes to
a temp file and atomically renames; remove that pre-delete and instead rely on
the temp-file->atomic rename to replace the target (or, if you need explicit
replacement, perform the final move with an atomic REPLACE_EXISTING/ATOMIC_MOVE
operation) so the file remains available until the new file is ready; update
saveToFile to stop calling file.delete() and ensure the final rename/move used
by download() overwrites atomically.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@core/data/src/jvmMain/kotlin/zed/rainxch/core/data/services/DesktopDownloader.kt`:
- Around line 204-207: The current pre-download deletion in saveToFile (the
file.delete() block) creates a brief absence window before download() writes to
a temp file and atomically renames; remove that pre-delete and instead rely on
the temp-file->atomic rename to replace the target (or, if you need explicit
replacement, perform the final move with an atomic REPLACE_EXISTING/ATOMIC_MOVE
operation) so the file remains available until the new file is ready; update
saveToFile to stop calling file.delete() and ensure the final rename/move used
by download() overwrites atomically.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 109ffaa3-3d46-49d9-bb23-eeb269bce748

📥 Commits

Reviewing files that changed from the base of the PR and between 25a848f and eb5a3b1.

📒 Files selected for processing (2)

• core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidDownloader.kt
• core/data/src/jvmMain/kotlin/zed/rainxch/core/data/services/DesktopDownloader.kt