feat(settings): choose where extracted stems are stored - #355
Merged
Conversation
Closes #354 Reported by a user whose cloud storage grew by 2.5 GB without them asking: the desktop app writes stems to ~/Documents/StemDeck/jobs, and Documents is the folder most people have syncing to iCloud or OneDrive. A stem library is six WAVs plus a video track per song, so it adds up fast, and there was no way to change it short of editing the environment by hand. Settings -> General now shows where stems live, how much is there, and a native folder picker to change it. The move is the feature, not the preference. The registry lives inside the stems folder, so writing the setting alone would strand the library and bring the app back empty. Changing the location moves the files, and the preference is written only after that succeeds -- a failure leaves the app still reading the folder the files are actually in. Validation is deliberately strict, because this path becomes the root of a directory the app creates and deletes inside: absolute paths only, no folder inside the current one (a directory cannot be moved into itself), and an existing folder is accepted only when it is empty or already ours. Merging a stem library into someone's Desktop is not a recoverable mistake. It also refuses while any job is queued or running, since moving files out from under a separation would corrupt it. Desktop only. Docker and Unraid get their storage from a mounted volume that the operator decides and that is still the mount on the next start, so both endpoints 403 outside the desktop shell and the UI removes the row when it is not running under Tauri. STEMDECK_JOBS_DIR keeps winning over everything, which is what makes a pinned deployment immune. For that to work the launcher now passes the Documents path as STEMDECK_DEFAULT_JOBS_DIR rather than STEMDECK_JOBS_DIR: a default the setting can override, instead of a pin that would silently defeat it.
…move The row was shown or hidden by sniffing for Tauri in the frontend, which put the judgement in two places and got it wrong in one: the backend is the thing that knows a deployment pins its storage. The read now answers everywhere and carries an `editable` flag; only the move is refused outside the desktop shell. Probing a 403 to decide, which was the first attempt, logged a failed request in the console every time Settings was opened. Shortens the label to "StemData location". The description explained why Documents is a bad place for tens of gigabytes, which is true but is three lines of a settings panel to say something the user already knows by the time they are looking for this. The one part that mattered -- that it takes a restart -- is in the message shown after the move, where it is actually needed.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Flagged by the code-quality bot: the fixtures imported app.main and app.core.settings as module objects (monkeypatch needs those) and then pulled names out of the same modules with 'from'. Now module-only.
Four cases in a subprocess, because JOBS_DIR is bound at import time across the app. The first is the one that matters: an existing desktop user with no stored choice must keep reading their Documents folder after an update. Get it wrong and the app opens to an empty library with the stems still on disk. Also covers a corrupt settings.json falling back to the default rather than relocating the library, and an explicit pin (Docker, Unraid) beating a stray setting in the image.
Two bugs found reviewing this branch, both introduced by it. JOBS_DIR is bound at import time across the app, so after a move this process still writes to the folder the files just left. An import accepted in that window landed in the old folder and vanished on the next start -- stems on disk, in a directory nothing looks at any more. Reproduced against a live server: the import returned a job id and wrote a fresh registry.json into the folder that had just been emptied. Imports are now refused with a 409 that says to restart, from the moment the move begins. Starting there rather than after it also closes the race against an import arriving mid-move. The flag is cleared only if the move fails, since then nothing moved and the app is still right about where the library is; success is cleared by the restart, which is the point. Second: a configured folder that no longer exists is no longer honoured. That means an external disk that is not mounted, and ensure_runtime_dirs would have happily created the path -- on macOS that puts a real directory at the mount point on the boot disk and can stop the drive mounting under its own name later, while the user stares at an empty library. Falling back to the default leaves everything findable again once the disk is plugged in. Also stubs the queue in the endpoint fixture. One of the new tests submits a real URL, and letting the worker pick it up sent the suite to YouTube: 121 s down to 1.3 s.
…e real folder Two more from a second read of this branch. The guard against imports went up after the busy check and after validate_target, which touches the disk -- it creates the target folder and lists it. An import accepted during that window would have had its directory moved out from under it moments later. It now goes up first, and every early exit reopens it, including the busy 409: leaving it closed there would block imports until a restart for a move that never happened. And the panel reported JOBS_DIR, which after a move is the folder the user just moved away from -- reopening Settings made a successful move look like it had failed. It now reports where the stems actually are, with the size read from there, and says a restart is still pending.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Settings -> General now has a StemData location row: where extracted stems live, how much is there, and a native folder picker to change it.
Closes #354
Why
From the report:
Documents is the right default -- the library is the user's work, it should be visible in Finder and survive a reinstall -- but it is also the folder most people have syncing to iCloud or OneDrive. Six WAVs plus a video track per song adds up fast, and there was no way to change it short of editing the environment by hand.
The move is the feature
The registry lives inside the stems folder, so writing the preference alone would strand the library and bring the app back empty with the files orphaned. Changing the location moves the files, and the preference is written only after that succeeds -- a failure leaves the app still reading the folder the files are actually in.
Validation is deliberately strict, because this path becomes the root of a directory the app creates and deletes inside:
A move interrupted partway can be re-run: entries already at the target are skipped rather than colliding.
Desktop only
Docker and Unraid get their storage from a mounted volume the operator chose, and it is still the mount on the next start. So the move endpoint 403s outside the desktop shell, and the UI hides the row.
Which of those two decides was worth getting right. The first version sniffed for Tauri in the frontend, putting the judgement in two places; the backend is the thing that knows whether a deployment pins its storage. The read now answers everywhere with an
editableflag and only the write is refused. (Probing a 403 to decide -- the second attempt -- logged a failed request in the console every time Settings was opened.)STEMDECK_JOBS_DIRstill wins over everything, which is what makes a pinned deployment immune. For that to hold, the desktop launcher now passes the Documents path asSTEMDECK_DEFAULT_JOBS_DIRrather thanSTEMDECK_JOBS_DIR: a default the setting can override, instead of a pin that would silently defeat it -- the move would happen and the app would come back empty.Resolution order is now:
STEMDECK_JOBS_DIR-- a deployment that pinned itsettings.json-- what the user chose hereSTEMDECK_DEFAULT_JOBS_DIR-- the desktop defaultDATA_DIR/jobs, then<repo>/jobsTesting
23 new tests, 515 total. Most of them are about refusing to destroy a library rather than about moving one.
Verified against a live backend and a real browser: a 1.1 MB library moves, the setting persists, and after a restart the backend reads the new location. Also checked that a Docker-style pin beats the setting, and that a server-mode instance hides the row with no console noise.
Not verified
The native picker and the launcher change need a real desktop build. The Rust compiles and links and adds no new clippy findings, but I have not seen the Finder dialog open, so that is the one thing worth a manual check before this merges.
Also in the report, not addressed here
Worth answering for the reporter, and worth its own issue if the update flow really does need that.