Comfy-Org / Comfy-Org/Comfy-Desktop
Decouple desktop download identity from URL (composite download IDs) for concurrent same-URL/different-destination downloads
- Dominant language
- TypeScript
- Stars
- 458
- Forks
- 59
- Avg merge
- 22h 18m
- Merged PRs (30d)
- 45
Description
## Summary
The desktop download manager identifies every in-flight download **solely by its source URL**. This is baked into the entire IPC surface — progress events, pause/resume/cancel/retry, and the `window.__comfyDesktop2.downloadModel` bridge all key on `url`. As a result, a single URL cannot represent two concurrent downloads with **different destinations**.
This surfaced while fixing #1045 (honor per-install model paths when shared models are off, PR #1058). With per-install model paths, two installations can legitimately want to download the same URL into two different directories at the same time. Today they collide on the URL key.
## Current behavior / limitation
In `src/main/lib/comfyDownloadManager.ts`:
- `pendingDownloads` is a `Map`.
- `pauseModelDownload(url)`, `resumeModelDownload(url)`, `cancelModelDownload(url)`, `retryDownload(url)` all look up by URL.
- Progress is broadcast as `desktop2-download-progress` / `model-download-progress` with `{ url, ... }`, and `findPendingForItem` matches by URL chain.
- `retryParamsByUrl`, `createdAtByUrl`, and the recent-downloads buffer are all URL-keyed.
If install A is already downloading `https://.../model.safetensors` into its own models dir and install B (different destination) requests the same URL, B cannot get its own download. PR #1058 added a **fail-closed guard** as a stopgap: if a pending download for the URL targets a different directory, the second request returns `false` instead of subscribing and reporting a completion that lands where B's ComfyUI can't see it. This avoids the wrong-success symptom but does not actually let both downloads proceed.
## Proposed fix
Introduce an opaque per-download identifier (download ID) as the primary key, decoupled from the URL:
- Key `pendingDownloads` (and `retryParamsByUrl`, `createdAtByUrl`, recent buffer) by download ID.
- Thread the ID through every IPC call: progress events carry `{ id, url, ... }`; pause/resume/cancel/retry take an ID.
- `findPendingForItem` resolves the originating pending entry by ID rather than URL chain.
- Allow multiple concurrent entries for the same URL with different destinations.
## Cross-repo impact: ComfyUI_frontend must change too
The download contract is shared with `ComfyUI_frontend`, so this is **not** a desktop-only change. The frontend code that landed to support Desktop 2.0 downloads also keys on URL and would need to be updated in lockstep:
- `src/platform/missingModel/missingModelDownload.ts` — the `ComfyDesktop2Bridge` interface (`downloadModel(url, filename, directory) => Promise`) and the `window.__comfyDesktop2` typing. The bridge should return/accept a download ID.
- `src/stores/electronDownloadStore.ts` — tracks download state keyed by URL; would need to key by ID.
- `src/platform/missingModel/missingModelStore.ts` and the missing-model UI — progress/status correlation per model would move from URL to ID.
On the desktop side, the injected content script that intercepts download clicks also routes through the URL-based bridge:
- `src/main/lib/comfyContentScript.ts` — the `window.__comfyDesktop2.downloadModel(...)` interception and the title-bar downloads tray.
## Acceptance criteria
- Two installs with different destinations can download the same URL concurrently, each landing in (and being visible to) its own install.
- Pause/resume/cancel/retry and progress correctly target one download without affecting the other.
- The `ComfyUI_frontend` bridge + stores are updated to the ID-based contract and remain backward-compatible (or are versioned) with older desktop builds.
- The fail-closed guard added in #1058 can be removed once the ID-based contract lands.
## References
- Issue #1045
- PR #1058 (introduced the fail-closed guard noting this limitation)
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.