Repository navigation
feat: downloads UX, failed-download cleanup, live progress and uploads view - #104
khaosdoctor wants to merge 17 commits into
Conversation
 [High impact] The migration split appears safe to merge; the earlier stale-speed issue remains non-blocking.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ing errors, summarize uploads in SQL
There was a problem hiding this comment.
🟡 Changes recommended
Uncaught SQLite write failures can disrupt transfers or terminate the server, the reaper can continue retrying or deleting after being disabled, and dashboard updates have throttling gaps and costly full-history queries.
5 open issues
What changed in this PR
This PR combines the earlier downloads improvements with upload tracking,
extending jack's management UI and backend transfer lifecycle.
Changes:
- Adds bulk download actions, filename wrapping, live progress and optional
retry-then-delete cleanup for failed downloads. - Records peer uploads without replacing zero-copy file serving, exposes upload
history in the UI, and displays the build version.
| File | Description |
|---|---|
| website/reference/configuration.md | Documents failed-download cleanup settings. |
| website/guide/management-ui.md | Documents uploads and live settings. |
| apps/ui/nuxt.config.ts | Resolves the build version from environment or git. |
| apps/ui/Dockerfile | Accepts the version build argument. |
| apps/ui/app/utils/format.ts | Adds transfer-rate and version formatting. |
| apps/ui/app/utils/format.test.ts | Tests release and branch version labels. |
| apps/ui/app/types/management.ts | Adds upload, progress and cleanup types. |
| apps/ui/app/pages/uploads.vue | Adds a filterable upload-history page. |
| apps/ui/app/pages/settings.vue | Updates version display and adds repository link. |
| apps/ui/app/pages/index.vue | Shows download rates and upload activity. |
| apps/ui/app/pages/downloads.vue | Adds bulk actions, wrapping and transfer rates. |
| apps/ui/app/layouts/default.vue | Adds uploads navigation and sidebar version. |
| apps/ui/app/components/DownloadsSection.vue | Exposes failed-download cleanup controls. |
| apps/backend/src/modules/uploads/uploads.service.ts | Coordinates upload tracking and receipts. |
| apps/backend/src/modules/uploads/uploads.repository.ts | Persists upload history and state transitions. |
| apps/backend/src/modules/status/status.router.ts | Adds upload list and stream endpoints. |
| apps/backend/src/modules/status/status.controller.ts | Exposes upload summaries and live download metrics. |
| apps/backend/src/modules/peer/peer.router.ts | Tracks file requests and accepts receipts. |
| apps/backend/src/modules/peer/peer.controller.ts | Explains zero-copy tracking in comments. |
| apps/backend/src/modules/downloads/live-progress.ts | Tracks in-memory download speed and bytes. |
| apps/backend/src/modules/downloads/failed-download-reaper.ts | Retries and removes failed downloads. |
| apps/backend/src/modules/downloads/downloads.service.ts | Publishes live progress and guards automated deletion. |
| apps/backend/src/modules/config/config.service.ts | Clarifies which settings apply live. |
| apps/backend/src/modules/config/config.router.ts | Updates configuration API documentation. |
| apps/backend/src/middleware/require-auth.ts | Exposes key IDs for receipt ownership checks. |
| apps/backend/src/management-app.ts | Connects upload tracking to management endpoints. |
| apps/backend/src/lib/servers/peer.ts | Reports live bytes and sends upload receipts. |
| apps/backend/src/lib/openapi.ts | Adds the uploads documentation tag. |
| apps/backend/src/lib/config.ts | Defines cleanup settings and validation. |
| apps/backend/src/index.ts | Starts upload maintenance and failed-download cleanup. |
| apps/backend/src/database/schema.ts | Defines upload records and indexes. |
| apps/backend/src/app.ts | Injects upload tracking into peer routes. |
| apps/backend/src/__tests__/uploads-repository.test.ts | Tests upload persistence and ownership. |
| apps/backend/src/__tests__/upload-tracking.test.ts | Tests tracking failures and receipt routes. |
| apps/backend/src/__tests__/peer-download.test.ts | Tests outgoing upload receipts. |
| apps/backend/src/__tests__/live-progress.test.ts | Tests speed calculation and status enrichment. |
| apps/backend/src/__tests__/failed-download-reaper.test.ts | Tests retry backoff and cleanup eligibility. |
| apps/backend/src/__tests__/downloads-service.test.ts | Extends configuration fixtures. |
| apps/backend/drizzle/meta/0014_snapshot.json | Records the uploads schema snapshot. |
| apps/backend/drizzle/meta/_journal.json | Registers migration 0014. |
| apps/backend/drizzle/0014_uploads.sql | Creates the uploads table and indexes. |
| .github/workflows/publish.yml | Passes the image version into UI builds. |
Review effort: Balanced
|
@greptileai review |
|
@greptileai review |
1 similar comment
|
@greptileai review |
|
@greptileai review |
|
|
||
| // Record the upload; the body still goes out zero-copy. A HEAD request sends | ||
| // no body, so it is not an upload. The master key sets no name. | ||
| const uploadId = uploads && c.req.method !== 'HEAD' |
There was a problem hiding this comment.
Done in 785a9ff, the transfer is its own resource now:
POST /peer/items/:itemId/downloadstakes{ clientTransferId }and returnsdownloadId(a UUID),filename,totalBytesandurl. It's idempotent per API key: the same id returns the existing transfer with a 200 instead of a 201, and reusing it for another item is a 409.GET /peer/downloads/:downloadId/filestreams with Range and only records the lowest byte served, no new row. A dropped request leaves the transfer open.POST /peer/downloads/:downloadId/receiptcompletes the same row. Bytes and duration are computed on the serving side (total minus the lowest range start, creation to receipt), so the receipt has no body.
The downloading jack uses download-<rowId>-<startedAt> as the client id, so every retry of one download hits the same transfer, restarts included. Protocol goes to 0.3.0. GET /items/:itemId/file stays for older peers, but it no longer records anything, and a newer jack only falls back to it for older peers or when creating the transfer fails.
| * (each write also fsyncs the .part), so this is what lets the UI move the bar | ||
| * and show a speed in between. Nothing here is persisted. | ||
| */ | ||
| export class LiveProgressTracker { |
There was a problem hiding this comment.
Agreed, removed the tracker and the bytes event in 785a9ff. Speed now comes from the previous progress sample in the same attempt and is stored as downloads.speed_bps, so it goes out through the repository and the existing SSE; etaSeconds is derived from it in enrichDownload.
The trade-off is the cadence: progress, speed and ETA now update every 10 s / 64 MiB instead of every second, so the bar moves in steps and the speed lags up to 10 s behind. I kept that interval because each progress write also fsyncs the .part, and a write per second would mean an fsync per second per transfer. If one-second updates matter to you, I can bump the cadence of the existing progress writes, maybe with the fsync kept on the 10 s / 64 MiB rhythm, rather than bring back a second source.
…oad speed from progress samples
|
|
||
| // Record the upload; the body still goes out zero-copy. A HEAD request sends | ||
| // no body, so it is not an upload. The master key sets no name. | ||
| const uploadId = uploads && c.req.method !== 'HEAD' |
There was a problem hiding this comment.
Done in 785a9ff, the transfer is its own resource now:
POST /peer/items/:itemId/downloadstakes{ clientTransferId }and returnsdownloadId(a UUID),filename,totalBytesandurl. It's idempotent per API key: the same id returns the existing transfer with a 200 instead of a 201, and reusing it for another item is a 409.GET /peer/downloads/:downloadId/filestreams with Range and only records the lowest byte served, no new row. A dropped request leaves the transfer open.POST /peer/downloads/:downloadId/receiptcompletes the same row. Bytes and duration are computed on the serving side (total minus the lowest range start, creation to receipt), so the receipt has no body.
The downloading jack uses download-<rowId>-<startedAt> as the client id, so every retry of one download hits the same transfer, restarts included. Protocol goes to 0.3.0. GET /items/:itemId/file stays for older peers, but it no longer records anything, and a newer jack only falls back to it for older peers or when creating the transfer fails.
| * (each write also fsyncs the .part), so this is what lets the UI move the bar | ||
| * and show a speed in between. Nothing here is persisted. | ||
| */ | ||
| export class LiveProgressTracker { |
There was a problem hiding this comment.
Agreed, removed the tracker and the bytes event in 785a9ff. Speed now comes from the previous progress sample in the same attempt and is stored as downloads.speed_bps, so it goes out through the repository and the existing SSE; etaSeconds is derived from it in enrichDownload.
The trade-off is the cadence: progress, speed and ETA now update every 10 s / 64 MiB instead of every second, so the bar moves in steps and the speed lags up to 10 s behind. I kept that interval because each progress write also fsyncs the .part, and a write per second would mean an fsync per second per transfer. If one-second updates matter to you, I can bump the cadence of the existing progress writes, maybe with the fsync kept on the 10 s / 64 MiB rhythm, rather than bring back a second source.
| // Measured between progress writes (every 10 s / 64 MiB); only while running. | ||
| const speedBps = d.status === 'downloading' | ||
| ? d.speedBps | ||
| : null |
There was a problem hiding this comment.
enrichDownload() now returns the saved speedBps without checking when bytes last arrived. After a peer stops sending, the UI keeps showing the old speed and ETA through the idle timeout and retry wait. Removing the live tracker also makes progress updates wait for the 10-second / 64-MiB writes.
Keep a separate live byte count and clear its speed when samples stop.
There was a problem hiding this comment.
Fixed in ffca03e without bringing back a second byte count: a failed attempt clears speed_bps (and notifies) before its retry wait, and enrichDownload drops a speed whose row hasn't been written for 30 s, so a stall shows no speed past that. The 10 s / 64 MiB cadence is discussed in the maintainer's thread above.
…ount uploads of replaced files
roziscoding
left a comment
roziscoding
left a comment
There was a problem hiding this comment.
Re-reviewed the current head after the follow-up fixes. The transfer lifecycle now uses an explicit idempotent resource, and speed/ETA build on the existing progress path. Backend tests, UI typecheck, targeted lint, and diff checks pass.
What does this PR do?
Replaces #100, #101, #102 and #103 with one PR, and fixes the review feedback left on #102. One commit per change, so they can still be read separately. The dashboard redesign is a separate PR.
downloads.removeFailedDownloads, off by default, applies live). A failed download is retried up tofailedDownloadRetriestimes (minimum and default 3, since one network error is often enough to fail an attempt), with the delay doubling fromfailedRetryBaseDelayMsand capped atretryMaxDelayMs. If it keeps failing, the row and its files are deleted the way Delete does it.apps/ui/package.json, which stays at0.1.0. CI now passes the image'sorg.opencontainers.image.versionlabel value asAPP_VERSION. Release images showv1.2.3, branch and PR images showmain/pr-<n>. Local builds fall back to the latest git tag, thendev.downloads.speed_bps, and the status API derivesetaSecondsfrom it. A new attempt or a resume reset clears it.uploadstable, one row per transfer. Migration0014is the table as this PR first shipped it, unchanged;0015converts it to transfers (existing rows kept,abortedones becomeunconfirmed) and addsdownloads.speed_bps, so a database that ran an earlier build of this PR still starts. Peer protocol goes to0.3.0:POST /peer/items/:itemId/downloadswith aclientTransferIdcreates the transfer and returns an opaquedownloadId, the file metadata and its URL. It's idempotent per API key, so a create whose response got lost returns the same transfer. The downloading jack uses a client id derived from its download row, so every retry and restart of one download reuses one transfer.GET /peer/downloads/:downloadId/filestreams it with Range support, as often as needed, without new rows. A dropped request leaves the transfer open to resume.POST /peer/downloads/:downloadId/receiptcompletes it once the file is in place. Bun gives no signal when a zero-copy response finishes, so the receipt is how jack knows. The bytes (total size minus the lowest range start served) and the duration (creation to receipt) are computed on the serving side, so a peer can't skew them.unconfirmed; a late one still completes it.GET /peer/items/:itemId/filestays for older peers and still serves the file, but records nothing. A newer jack falls back to it for older peers, or when creating the transfer fails (a full disk on the other side, say).Related issue
Supersedes #100, #101, #102 and #103.
Checklist
mise run lint:fixwas run and its output is committedmise run testpasses (andmise run test:e2eif transfer/request paths changed)