Skip to content

feat(web): keep downloads in a folder on disk - #231

Open
mia-riezebos wants to merge 6 commits into
feat/180-offline-progressfrom
feat/182-offline-folder
Open

mia-riezebos wants to merge 6 commits into
feat/180-offline-progressfrom
feat/182-offline-folder

Conversation

@mia-riezebos

@mia-riezebos mia-riezebos commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Downloads only lived in the browser's private storage, where the user can't see them, back them up, or free space outside the app. Chromium desktop browsers can write to a real folder. This is #182 of PRD #173, stacked on #227.

On Chromium desktop, the Downloads page can link a folder. Finished downloads move there under readable names, such as Arrival (2016).mkv or Severance · S1, E2 · Good News About Hell.mkv, and each row can move its file between the folder and browser storage.

  • Staged in browser storage, then moved. A download still streams into OPFS through the sync access handle, so it keeps resuming in place after a reload. Once finished, it copies into the folder. A folder file can only be written through a writable that copies the whole file each time it reopens, so writing there directly would lose everything since the last pause on a reload. The copy streams, is checked byte for byte, and is recorded in the index before the original goes. A failed copy keeps the original.
  • The folder describes itself. .thalia/downloads.json in the folder lists its downloads with their saved details, mirrored on every index write. When a folder is linked or allowed, Thalia adds the downloads it lists whose files are there whole. So a later session, or one whose browser data was cleared, finds them again.
  • Access. The folder handle is kept in IndexedDB. When the browser drops access after a restart, the folder row shows Needs access and an Allow access button. Its rows hide their actions until access comes back, and they still open offline from their saved details.
  • Unlink removes the folder's rows from the list and leaves the files and manifest, so linking it again brings them back. Changing to another folder does the same for the old one.
  • Other browsers. Without showDirectoryPicker (Safari, Firefox, Chrome on Android), the folder row isn't shown.
  • Plays offline from the folder. The offline player (feat(web): play a download offline by remuxing it in the browser #225) reads through downloads.file(name): the media by Version id, a saved subtitle (feat(web): show text and ASS subtitles in offline playback #233) by its name, from wherever the download is kept. Saved subtitles move with the media and sit beside it under its name, such as Arrival (2016).en.forced.srt, so media players find them too and a folder found again after the browser's data is cleared still has them. A name counts as free only when the media and every subtitle beside it are, so no file of the user's is overwritten.

Closes #182

Mia's call

  • Stage then move, or write straight to the folder? I chose staging (above) to keep resume after a reload. The cost is double disk use while a finished download copies, and if the tab closes during that copy, the download stays in browser storage until it's moved by hand. Writing straight to the folder would need neither, but a reload mid-download would restart from the last pause.
  • Pullfrog's questions on Offline: keep downloads in a folder on disk #182: yes to a hidden manifest (.thalia/downloads.json), and moves cover finished downloads only, copying first and removing the source only after verification.

Screenshots

Headless Chromium can't drive the real picker, so these use an OPFS folder named disk as a stand-in (hence In disk). The "Needs access" state is forced with a stubbed permission check.

1440 390
No folder unlinked 1440 light unlinked 390 dark
Folder linked linked 1440 dark linked 390 light
Needs access needs access 1440 needs access 390

Tests

  • apps/server/src/api/download-folder-browser.test.ts (new, headless Chromium, stand-in folder as above): links the folder, downloads through the button, and checks the file is in the folder as Arrival (2016).mkv with the right size, its stored forced subtitle beside it as Arrival (2016).en.forced.srt, and both gone from OPFS. It then deletes the browser's own index and stops the server; after a reload the download is listed again from the folder's manifest, its page opens offline, and Play on the Downloads page plays it from the folder with its subtitle showing. Move to browser storage brings both files back into OPFS. Without the subtitle move it fails on the folder listing. 10 of 10 passed with 12 cores busy. With showDirectoryPicker removed, the folder row doesn't render. Before the fix, 2 of 13 runs failed: one read the page mid-reload, and I didn't keep the other's output. The test now waits instead of reading once, and the 8 runs since all passed, 3 of them alongside the other three download browser tests.
  • lib/downloads/store.test.ts: a finished download lands in the folder under the Item's name and the manifest lists it; moving back restores the OPFS file and empties the manifest. A later session lists the folder's downloads from the manifest and skips one whose file is gone; file() reads it; unlinking drops the rows and leaves the file. A saved subtitle moves into the folder beside its media and back, file() reads it in both places, and a user's own Arrival (2016).en.srt already there is left alone: the download takes Arrival (2016) (2).mkv instead.
  • Web bun test: 308 pass. bun run check: clean. bun run lint: 0 errors, the 9 CSS warnings already on main.

Not covered automatically: the real picker, a real folder on disk, and the permission prompt after a browser restart. Those are in the manual checks.


Made with claude-opus-5.5 using Claude Code in T3 Code.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cbba45d3-ceca-4004-b1c6-3ae04b13da19

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

Comment @coderabbitai help to get the list of available commands.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Three reproduced concurrency failures can overwrite downloads, delete a file from a previously linked folder, or leave a completed download pointing at the wrong folder.

Reviewed changes across all nine files, covering folder-backed storage, recovery, permissions, and the Downloads UI.

  • Folder storage: Readable file names, streaming copies, a portable manifest, and an IndexedDB-persisted directory handle.
  • Download lifecycle: Automatic moves after completion, manual moves in both directions, manifest adoption, unlinking, and location-independent file() access.
  • Downloads UI: Folder selection and access controls, location captions, move actions, and unsupported-browser handling.
  • Verification: All 293 existing web unit tests pass and Svelte/TypeScript diagnostics are clean; three temporary regression tests reproduced the inline findings and were removed afterward. Real-picker and permission-restart behavior was not exercised.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Comment thread apps/web/src/lib/downloads/store.ts Outdated
Comment on lines +523 to +525
await locked(fileLock(versionId), async () => {
if (to === "folder") {
const name = await freeName(folder, fileNameFor(record));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Reserve destination names across Versions before copying

Two concurrent moves of different Versions with the same readable name can both select the same destination because these locks are per Version. I reproduced both records receiving the same fileName and both OPFS originals being deleted, leaving only one Version's bytes; the size check also passes when their sizes match.

Technical details
## Affected sites
- [store.ts:523-532](https://github.com/mia-cx/thalia/blob/a524c65347dd7370315d4488ba2c9b0700e1a6a9/apps/web/src/lib/downloads/store.ts#L523-L532): different Version locks do not serialize destination-name selection.
- [folder.ts:44-82](https://github.com/mia-cx/thalia/blob/a524c65347dd7370315d4488ba2c9b0700e1a6a9/apps/web/src/lib/downloads/folder.ts#L44-L82): freeName checks absence separately from getFileHandle(create:true), which can return the same existing file to both copies.

## Reproduction
Seed two completed OPFS records with distinct Version ids, identical titles/containers, and different same-sized bytes. With the existing FakeLocks fixture, await Promise.all of their move(id, "folder") calls. Both records end up with one identical filename rather than two unique destinations.

## Required outcome
Make destination-name selection and reservation atomic across Versions and tabs, so each copy owns its destination before writing or cleanup. Add concurrent same-name move coverage.

## API contracts
https://developer.mozilla.org/en-US/docs/Web/API/FileSystemDirectoryHandle/getFileHandle documents that create:true creates only when absent.
https://developer.mozilla.org/en-US/docs/Web/API/FileSystemFileHandle/createWritable documents that the default siloed mode permits multiple writers to one file.

Comment on lines +52 to +60
function loadLinked(): Promise<void> {
loaded ??= folderSupported()
? loadFolder()
.then((found) => {
handle = found;
})
.catch(() => {})
.then(publishFolder)
: Promise.resolve();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Refresh linked-folder identity across tabs

The folder handle is cached for the lifetime of each page, but other tabs receive the new folder's records through the shared index without updating that handle. After changing folders in one tab, I reproduced Delete in another tab removing a retained file from the old folder instead of the intended download in the new folder when their readable names matched.

Technical details
## Affected sites
- [index.ts:37-69](https://github.com/mia-cx/thalia/blob/a524c65347dd7370315d4488ba2c9b0700e1a6a9/apps/web/src/lib/downloads/index.ts#L37-L69): loaded and handle remain cached after another tab updates IndexedDB.
- [store.ts:364-367](https://github.com/mia-cx/thalia/blob/a524c65347dd7370315d4488ba2c9b0700e1a6a9/apps/web/src/lib/downloads/store.ts#L364-L367): changed messages synchronize records only.
- [store.ts:877-883](https://github.com/mia-cx/thalia/blob/a524c65347dd7370315d4488ba2c9b0700e1a6a9/apps/web/src/lib/downloads/store.ts#L877-L883): deletion interprets shared fileName against the stale local folder. Reads, moves, and manifest writes likewise use that local handle.

## Reproduction
Using two stores with shared OPFS, FakeLocks, and channelPair, initially link both to X. Change the active store to Y through release/adopt, leaving X's files intact. Download a new Version into Y with a filename also present in X. Once the other store receives its record, remove the new Version there: X's retained file is deleted and Y's intended file remains.

## Required outcome
Coordinate folder selection/unlink state across tabs and ensure file operations interpret records against the matching folder identity. Updating the displayed folder name alone is insufficient; stale tabs must not read, delete, move, or mirror records through an old handle.

Comment on lines +143 to +151
if (handle !== undefined && !(await handle.isSameEntry(picked))) {
// The old folder keeps its files and manifest; its rows leave the list.
handle = undefined;
await downloads.release();
}
await saveFolder(picked);
handle = picked;
await publishFolder();
await downloads.adopt();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Coordinate folder changes with in-flight moves

Change and Unlink remain available while a copy is running, but move() captures its destination before copying and commits its location using the current linked folder afterward. I reproduced changing from X to Y mid-copy: the move succeeded and deleted the OPFS original, while Y's manifest referenced a file that existed only in X and downloads.file() rejected.

Technical details
## Affected sites
- [index.ts:143-151](https://github.com/mia-cx/thalia/blob/a524c65347dd7370315d4488ba2c9b0700e1a6a9/apps/web/src/lib/downloads/index.ts#L143-L151): release and folder replacement do not wait for active moves; unlink has the same missing coordination.
- [store.ts:516-532](https://github.com/mia-cx/thalia/blob/a524c65347dd7370315d4488ba2c9b0700e1a6a9/apps/web/src/lib/downloads/store.ts#L516-L532): copy uses a captured folder, then setFileName changes a folder-relative record and removes its OPFS source.
- [store.ts:242-248](https://github.com/mia-cx/thalia/blob/a524c65347dd7370315d4488ba2c9b0700e1a6a9/apps/web/src/lib/downloads/store.ts#L242-L248): the manifest destination is resolved again at commit time.

## Reproduction
Gate the first destination write for an OPFS-to-X move. While it is blocked, clear the linked handle, await release(), link Y, and await adopt(), matching link()'s ordering. Then release the copy gate. The moving record survives release because its fileName is still null; afterward it points into Y although the only bytes are in X.

## Required outcome
Serialize folder changes/unlinking with active moves, or otherwise prevent a move from committing a location against a different folder and deleting its recoverable source. Protect the operation across tabs as well as within one page, and cover switching folders during a gated copy.

@pullfrog

pullfrog Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Pushed 2095bde to this PR: the folder test now uses in-app navigation and waits for the completed move, avoiding a reload that could interrupt index and manifest writes. Verification passed: 20 consecutive folder-test runs, all four download/offline browser tests together, three delayed-write trials, build, lint, and type checks. The full bun run test passed both originally failing tests but hit an unrelated password-change test timeout; that test passed on an isolated retry, and the working tree is clean.

Task list (1/3 completed)
  • Inspect the PR, CI failure logs, and repository guidance; confirm the failure is introduced by this PR.
  • Reproduce the failing CI command and apply the smallest targeted fix.
  • Run verification, inspect the diff, and push a signed commit to the existing PR.

Pullfrog  | View workflow run | via Pullfrog | Using gpt-6.1-sol | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

No new issues found in the incremental changes. The three previously reported concurrency findings remain unresolved, so the earlier request for changes still stands.

Reviewed changes since the previous Pullfrog review at a524c65, covering the index-write fix and browser-test updates.

  • Skipped redundant writes: Manifest adoption now ignores Versions already listed locally, avoiding unnecessary index and manifest rewrites on restore.
  • Added regression coverage: A new unit test verifies that restoring an already-populated folder index leaves the index untouched.
  • Preserved active downloads: The browser test navigates within the app while the automatic move finishes and waits for its controls before checking storage.
  • Updated recovery navigation: The manifest-recovery test uses page.goto() instead of a raw reload before checking the restored Downloads page.
  • Verified web checks: All 294 web unit tests passed, including 34 download tests; Svelte/TypeScript diagnostics reported zero errors and warnings. Browser integration and real-picker/permission-restart behavior were not exercised in this run.

Pullfrog  | Fix it ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

@mia-riezebos
mia-riezebos force-pushed the feat/182-offline-folder branch from 2c5980f to 7eb0f81 Compare October 21, 2026 08:07
@mia-riezebos
mia-riezebos force-pushed the feat/180-offline-progress branch from 7b2393d to 0ea0530 Compare October 21, 2026 08:07

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

No new issues found in the rebase integration. The three previously reported concurrency findings remain unresolved, so the earlier request for changes still stands.

Reviewed changes since the prior review at 2c5980f, focusing on the folder feature's integration with the updated offline-player base.

  • Preserved subtitle access: Kept saved-subtitle metadata and OPFS reads while resolving media files from their current storage location.
  • Adapted deletion: Removed folder-backed media from the linked folder and saved subtitle files from browser storage.
  • Retained playback UI: Combined folder-location captions with offline codec-support messages and preserved the base's Play action.
  • Verified integration: All 307 web unit tests passed; Svelte/TypeScript checks reported zero errors and warnings. Browser integration and real-picker/permission-restart behavior were not exercised.

Pullfrog  | Fix it ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

@mia-riezebos
mia-riezebos force-pushed the feat/182-offline-folder branch from 7eb0f81 to cfc4d76 Compare October 21, 2026 08:17

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

No new issues found in this rebase integration. The three previously reported concurrency issues remain unresolved, so the earlier request for changes still stands.

Reviewed changes since 7eb0f81, covering integration with the updated offline-progress base. A range-diff confirmed all five folder-feature patches are unchanged.

  • Integrated offline progress: Retained downloads.file() as the playback file resolver while the updated base records progress using the saved caller and Version duration.
  • Checked inherited test changes: Reviewed the base's fresh-page navigation and offline playback progress test changes for interaction with folder downloads.
  • Verified web checks: Passed all 307 web unit tests and Svelte/TypeScript diagnostics with zero errors or warnings. Browser integration and real-picker/permission-restart behavior were not exercised.

Pullfrog  | Fix it ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

@pullfrog

pullfrog Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

The failed CI job has one test failure: live transcode over HLS > an audio-only mismatch re-encodes the audio and passes every video packet untouched, returning 503 SERVICE_UNAVAILABLE: This Version is being prepared for streaming. The failing test and relevant server playback, indexing, and transcoder code are unchanged by this PR; the new folder-download browser test passed. No changes were pushed because the failure is outside this PR’s changes; the working tree is clean, and rerunning CI is the appropriate next step.

Task list (1/3 completed)
  • Inspect the PR and failed checks; confirm the failure is introduced by the PR.
  • Reproduce the CI failure and apply the smallest correct fix.
  • Run verification, review the diff, and push a signed fix to the existing PR.

Pullfrog  | View workflow run | via Pullfrog | Using gpt-6.1-sol | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

No new issues found in this incremental review. The three previously reported concurrency findings remain unresolved, so the earlier request for changes still stands.

Reviewed changes since cfc4d76, covering saved subtitles moving with folder-backed downloads and their offline playback integration.

  • Moved saved subtitles: Copied media and saved subtitles together in both directions, committed their location after all copies succeeded, and removed completed destination copies if a later copy failed.
  • Resolved folder-backed files: Routed saved-subtitle reads and deletion through the download's current location, using readable sidecar names beside the media.
  • Protected existing sidecars: Included subtitle destinations when choosing an available media filename, with a unit test covering a user's existing subtitle and the move back to browser storage.
  • Extended offline playback coverage: Added browser-test assertions for recovered folder downloads playing with their saved forced subtitle, then moving both files back to browser storage.
  • Verified web checks: Passed all 308 web unit tests and Svelte/TypeScript diagnostics with zero errors or warnings. Browser integration, the real picker, and permission restoration after a browser restart were not exercised in this run.

Pullfrog  | Fix it ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

@mia-riezebos
mia-riezebos force-pushed the feat/182-offline-folder branch from dfc96f0 to 830a0de Compare October 21, 2026 08:39
@mia-riezebos
mia-riezebos force-pushed the feat/180-offline-progress branch 2 times, most recently from 9086388 to d050798 Compare October 21, 2026 08:40
@mia-riezebos
mia-riezebos force-pushed the feat/182-offline-folder branch from 830a0de to 510da17 Compare October 21, 2026 08:40

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

No new issues found in this rebase integration. The three previously reported concurrency issues remain unresolved, so the earlier request for changes still stands.

Reviewed changes since dfc96f0, covering the updated base and its interaction with folder downloads. Range-diff confirmed all six folder-feature patches are unchanged.

  • Integrated browser-test waits: Inherited the base's asynchronous predicate handling and wait for offline artwork cleanup; checked compatibility with the folder browser test.
  • Preserved playback integration: Confirmed media and saved subtitles still resolve through downloads.file(), with offline progress recording unchanged.
  • Verified web checks: Passed all 308 web unit tests and Svelte/TypeScript diagnostics with zero errors or warnings after restoring missing dependencies. Browser integration, the real picker, and permission restoration after restart were not exercised.

Pullfrog  | Fix it ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

mia-riezebos and others added 6 commits October 21, 2026 12:11
On Chromium desktop browsers, the Downloads page can link a folder.
Finished downloads move there under readable names, such as
"Arrival (2016).mkv", and each row can move its file between the folder
and browser storage.

- Downloads still land in browser storage first, where they resume in
  place, then copy into the folder once finished. A copy is checked
  byte for byte before the original goes; a failed copy keeps it.
- The folder's handle is kept in IndexedDB; when the browser drops
  access, the folder row offers Allow access.
- .thalia/downloads.json in the folder lists its downloads, so a later
  session, or one without this browser's list, finds them again.
- Browsers without showDirectoryPicker don't offer the folder.
- downloads.file(versionId) reads a download's file wherever it is.

Closes #182

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… test

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Opening Downloads read the folder's manifest and rewrote the index even
when every download it lists was already there, so a write ran on after
the page showed its list.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Page.reload answers before the new document commits, so the "In disk"
wait could pass in the old page. The navigation sent next then raced the
reload, and under load it never answered and hung the test. Navigating
answers only once the new page commits.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The offline player reads a download through downloads.file(name), the
media by Version id and a saved subtitle by its name. Subtitles stayed in
browser storage when the media moved to the folder, so a folder found
again after the browser's data was cleared had none. They now move with
the media and sit beside it under its name, such as
Arrival (2016).en.forced.srt, where media players find them too, and
file() reads either from wherever the download is kept. A name is free
only when the media and every subtitle beside it are, so no file of the
user's is overwritten. A failed copy removes what it copied and keeps
the originals.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mia-riezebos
mia-riezebos force-pushed the feat/180-offline-progress branch from d050798 to 550c7f9 Compare October 21, 2026 10:58
@mia-riezebos
mia-riezebos force-pushed the feat/182-offline-folder branch from 510da17 to 60f6018 Compare October 21, 2026 10:58

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

No new issues found in this rebase integration. The three previously reported concurrency findings remain unresolved, so the earlier request for changes still stands.

Reviewed changes since 510da17, covering the inherited browser-test changes and their interaction with folder downloads.

  • Confirmed unchanged feature patches: Range-diff matched all six folder-feature commits to their previously reviewed versions; folder storage and playback behavior are unchanged.
  • Checked menu synchronization: Reviewed the inherited subtitle-test helper's wait for menu focus after opening, before keyboard selection. The folder playback test uses an automatically selected forced subtitle and is unaffected.
  • Checked assertion formatting: Verified the inherited subtitle assertion changes preserve their expectations.

Tests were not rerun in this review; the delta changes browser-test synchronization and formatting only. Real-picker and permission-restoration behavior remain unverified.

Pullfrog  | Fix it ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant