feat: bound, retry, and resume peer downloads end-to-end (#13 5/5)

[roziscoding]

Stack 5/5 for #13 (top). Base: feat/harden-peer-downloads/4-attempts-schema (#19). Integrates PRs 2–4.

What this PR does

Wires the primitives together in DownloadsService and rewires startup, completing the hardened flow.

• runDownload() runs every download through a shared Semaphore(maxConcurrentDownloads) and a retry() loop: increments attempts per attempt, classifies transient vs permanent (PR 3), and persists the restart event via markResumeReset (PR 4).
• An active set (keyed by destPath) and a reenqueued set (torrent filenames) dedupe so the startup re-enqueue and the watcher never double-process the same file.
• resumeStaleDownloads() actively re-drives stale downloading rows from the DB (resuming from their .part) instead of failing them. index.ts calls it before the watcher scans; reconcileStaleDownloads() is the fallback only when downloads is unconfigured.
• The path-traversal guard on the peer-controlled release.filename and the atomic-rename completion are preserved.

Review-feedback fix (commit 5d38820)

A code review caught two edge cases in the re-enqueue dedupe, both fixed with regression tests:

• Stale rows sharing a destPath are deduped (they share one .part); superseded duplicates are marked failed instead of silently orphaning a downloading row.
• The reenqueued claim is released on successful resume (after the stub is unlinked, so no scan race), so a later legitimate re-drop of the same filename isn't silently skipped.

Files

• apps/backend/src/modules/downloads/downloads.service.ts (full rewrite)
• apps/backend/src/index.ts (startup wiring)
• apps/backend/src/__tests__/downloads-service.test.ts (full rewrite, 10 tests)

Testing

10 service tests: row lifecycle, no-row-on-metadata-fail, path traversal, permanent failure (no retry), transient retry-then-succeed, resume reset persisted, concurrency cap, stale re-drive, duplicate-destPath dedupe, re-enqueue-claim release. Full suite: 129/129 green, lint + typecheck clean.

Manual verification (post-merge)

Boot Jack with a leftover *.part + a stale downloading row and confirm it re-enqueues, issues a Range request, resumes, and renames into the completed folder.

Review focus

• The semaphore/retry/resume wiring in runDownload/downloadWithRetry, the active/reenqueued dedupe lifecycle, and the index.ts startup ordering (resumeStaleDownloads() before watcher.start()).

Greptile Summary

[Linus Torvalds Mode] Oh look, someone finally decided to wire together all the primitives they've been lovingly building for the last four stacked PRs — if you're going to take five PRs to ship a download manager, at least make the fifth one actually work, which, grudgingly, this does.

This PR completes the hardened peer-download flow by integrating Semaphore, retry, and markResumeReset into DownloadsService:

• runDownload / downloadWithRetry: every download acquires a Semaphore(maxConcurrentDownloads) permit, then runs inside retry() with exponential-backoff + jitter. Permanent errors (4xx non-429) are not retried; transient ones (5xx, 429, network errors, IncompleteDownloadError) are.
• active + reenqueued dedup sets: active prevents duplicate rows/writers for concurrent live drops; reenqueued gates the watcher away from stubs owned by startup re-enqueue, cleared only on successful resume.
• resumeStaleDownloads: dedupes downloading rows by destPath, marks superseded rows failed, fires background runDownload calls before watcher.start().
• index.ts startup ordering: correct — resumeStaleDownloads() before watcher when configured; reconcileStaleDownloads() fallback when not.

Confidence Score: 5/5

[Linus Torvalds Mode] Five stacked PRs to ship a download manager — I've seen glaciers move faster. That said, this final piece holds together: semaphore, retry, and dedup logic are correct, startup ordering is right, and the tests cover the real edge cases. Safe to merge.

The single finding is a P2 type annotation in a test helper. No functional bugs found. The active.add synchronicity guarantee, reenqueued lifecycle, semaphore fairness, retry policy, and startup ordering are all correct. 10-test suite validates every meaningful invariant.

Only downloads-service.test.ts needs the minor type fix on the downloadsConfig helper; service and startup wiring are solid.

Important Files Changed

Filename	Overview
apps/backend/src/modules/downloads/downloads.service.ts	Full rewrite wiring Semaphore concurrency, retry loop, active/reenqueued dedup sets, and resumeStaleDownloads; all edge cases handled correctly.
apps/backend/src/index.ts	Startup wiring correctly calls resumeStaleDownloads() before watcher.start(); fallback reconcileStaleDownloads() path is correct.
apps/backend/src/tests/downloads-service.test.ts	10 well-targeted tests covering full lifecycle, retry, concurrency cap, stale re-drive, dedupe, and reenqueue-claim release; one minor type issue in the downloadsConfig helper.

Sequence Diagram

sequenceDiagram
    participant idx as index.ts
    participant svc as DownloadsService
    participant sem as Semaphore
    participant repo as DownloadsRepository
    participant peer as PeerConnector

    idx->>svc: resumeStaleDownloads()
    svc->>repo: listStaleDownloads()
    repo-->>svc: stale DownloadRecord[]
    note over svc: Dedupe by destPath, mark superseded failed, add filenames to reenqueued
    svc-)svc: void runDownload(record) fire-and-forget
    svc-->>idx: resumed count

    idx->>svc: BlackholeWatcher triggers processTorrentFile
    note over svc: reenqueued.has? skip. active.has? skip.
    svc->>peer: getRelease(itemId)
    peer-->>svc: Release validated
    svc->>repo: create(downloadInput)
    repo-->>svc: DownloadRecord
    svc->>svc: runDownload(record)
    note over svc: active.add(destPath) synchronous
    svc->>sem: acquire()
    sem-->>svc: permit

    loop retry up to maxDownloadAttempts
        svc->>repo: incrementAttempts(id)
        svc->>peer: downloadFile(itemId, destPath, partPath, onProgress)
        peer->>svc: onProgress headers
        svc->>repo: setExpectedBytes
        peer->>svc: onProgress restart optional
        svc->>repo: markResumeReset
        peer->>svc: onProgress completed
        svc->>repo: markCompleted
        peer-->>svc: resolved or transient error
    end

    svc->>svc: unlink stub
    svc->>svc: triggerImport
    svc->>repo: markImportQueued
    svc->>svc: reenqueued.delete
    svc->>sem: release
    note over svc: active.delete in finally block

Loading

_{Reviews (1): Last reviewed commit: "fix: harden startup re-enqueue dedupe (r..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.