From ffb96cd69e3bbab55be40857c68baedb84549907 Mon Sep 17 00:00:00 2001 From: Thales Pereira <31625914+thcp@users.noreply.github.com> Date: Tue, 11 Aug 2026 21:52:50 +0100 Subject: [PATCH 1/6] feat(settings): choose where extracted stems are stored 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. --- app/core/config.py | 40 ++- app/core/settings.py | 25 ++ app/core/stems_location.py | 152 ++++++++++ app/main.py | 99 +++++++ desktop/src-tauri/capabilities/default.json | 2 +- desktop/src-tauri/src/main.rs | 29 +- static/css/daw.css | 32 +++ static/js/catalog.js | 127 +++++++++ tests/test_stems_location.py | 298 ++++++++++++++++++++ 9 files changed, 799 insertions(+), 5 deletions(-) create mode 100644 app/core/stems_location.py create mode 100644 tests/test_stems_location.py diff --git a/app/core/config.py b/app/core/config.py index 1b237f07..f41af75d 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -1,3 +1,4 @@ +import json import os import re import sys @@ -59,9 +60,46 @@ def detect_torch_device() -> str: # folder. PORTABLE_DATA_DIR_ENABLED = bool(os.environ.get("STEMDECK_DATA_DIR", "").strip()) DATA_DIR = _env_path("STEMDECK_DATA_DIR", ROOT) + + +def _stored_jobs_dir() -> Path | None: + """The stems location the user picked in Settings, if any. + + Read straight out of settings.json rather than through app.core.settings, + which imports this module -- and it has to happen here because JOBS_DIR is + bound at import time across the app. Any problem reading it falls through to + the default: a library that quietly moves is far worse than one that ignores + a corrupt preference. + """ + try: + raw = json.loads((DATA_DIR / "settings.json").read_text(encoding="utf-8")) + except (OSError, ValueError): + return None + value = raw.get("jobs_dir") if isinstance(raw, dict) else None + if not isinstance(value, str) or not value.strip(): + return None + return Path(value).expanduser() + + +# Where extracted stems live. Precedence, most explicit first: +# 1. STEMDECK_JOBS_DIR -- a deployment that pinned it (Docker, Unraid, +# CI, tests). Wins over everything: a mounted +# volume is not the user's to relocate. +# 2. settings.json -- what the user chose in Settings (#354). +# Desktop only; the endpoint that writes it +# refuses to run anywhere else. +# 3. STEMDECK_DEFAULT_JOBS_DIR -- the desktop shell's default +# (~/Documents/StemDeck/jobs). Passed as a +# default rather than a pin, so 2 can win. +# 4. DATA_DIR/jobs -- portable default +# 5. /jobs -- plain dev checkout JOBS_DIR = _env_path( "STEMDECK_JOBS_DIR", - (DATA_DIR / "jobs") if PORTABLE_DATA_DIR_ENABLED else (ROOT / "jobs"), + _stored_jobs_dir() + or _env_path( + "STEMDECK_DEFAULT_JOBS_DIR", + (DATA_DIR / "jobs") if PORTABLE_DATA_DIR_ENABLED else (ROOT / "jobs"), + ), ) CACHE_DIR = _env_path("STEMDECK_CACHE_DIR", DATA_DIR / "cache") DOWNLOADS_DIR = _env_path("STEMDECK_DOWNLOADS_DIR", DATA_DIR / "downloads") diff --git a/app/core/settings.py b/app/core/settings.py index 90746dab..21ae06ef 100644 --- a/app/core/settings.py +++ b/app/core/settings.py @@ -5,6 +5,7 @@ - `allow_network` — whether StemDeck answers requests from other devices. - `max_duration_sec` — longest track accepted for processing. +- `jobs_dir` — where extracted stems are written (needs a restart). - `playlist_max_items` — how many tracks one playlist import may queue. - `video_max_height` — max video resolution for MP4 export / YouTube pulls. - `export_sample_rate` — sample rate for exported mixes/regions (WAV/FLAC/MP3). @@ -21,6 +22,7 @@ import logging import os import threading +from pathlib import Path from app.core.config import ( DATA_DIR, @@ -125,6 +127,29 @@ def set_max_duration_sec(value: int) -> int: return clamped +# ── jobs_dir ── +# Where extracted stems are written. Read by config.py at import time (straight +# from settings.json, to avoid importing this module from there), so a change +# only takes effect on the next start -- which is also when the move that goes +# with it has finished. Absent means "wherever the default puts it". +def get_jobs_dir() -> str | None: + with _LOCK: + value = _ensure().get("jobs_dir") + return value if isinstance(value, str) and value.strip() else None + + +def set_jobs_dir(value: str | None) -> str | None: + with _LOCK: + if value is None or not str(value).strip(): + _ensure().pop("jobs_dir", None) + _save() + return None + resolved = str(Path(str(value)).expanduser().resolve()) + _ensure()["jobs_dir"] = resolved + _save() + return resolved + + # ── playlist_max_items ── # How many tracks one playlist import may queue. A waiting link costs a registry # record, so the ceiling is generous; the real reason to keep this adjustable is diff --git a/app/core/stems_location.py b/app/core/stems_location.py new file mode 100644 index 00000000..a567ba55 --- /dev/null +++ b/app/core/stems_location.py @@ -0,0 +1,152 @@ +"""Moving the stems library to a different folder (#354). + +Documents is a sensible default -- the library is the user's work, it belongs +somewhere visible that survives a reinstall -- but on macOS and Windows it is +also the folder most people have syncing to iCloud or OneDrive, and a stem +library is tens of gigabytes nobody agreed to spend on cloud storage. + +Changing the location is not just a preference write. The registry lives inside +the stems folder, so leaving the existing library behind would make the app come +back empty with the files stranded. The move is the feature; the setting is +bookkeeping. +""" + +from __future__ import annotations + +import logging +import os +import shutil +from dataclasses import dataclass +from pathlib import Path + +logger = logging.getLogger("stemdeck.stems_location") + + +class StemsLocationError(ValueError): + """Rejected before anything on disk was touched.""" + + +@dataclass(frozen=True) +class MoveResult: + source: str + target: str + moved_entries: int + bytes_moved: int + same_filesystem: bool + + +def directory_size(path: Path) -> int: + """Best-effort total size. Used to tell the user what they are about to move, + so a failure to stat one file must not sink the whole answer.""" + total = 0 + for root, _dirs, files in os.walk(path, onerror=lambda _e: None): + for name in files: + try: + total += (Path(root) / name).stat().st_size + except OSError: + continue + return total + + +def validate_target(target: Path, current: Path) -> Path: + """Check a candidate location before anything is moved into it. + + Deliberately strict. This path becomes the root of a directory the app + creates and deletes inside, and a careless answer here -- the user's home + directory, or a folder already full of their own files -- turns a settings + change into data loss. + """ + if not str(target).strip(): + raise StemsLocationError("Pick a folder to store stems in.") + + target = Path(target).expanduser() + if not target.is_absolute(): + raise StemsLocationError("The stems folder must be an absolute path.") + target = target.resolve() + current = current.expanduser().resolve() + + if target == current: + raise StemsLocationError("Stems are already stored there.") + if current in target.parents: + # Moving a directory inside itself would recurse forever. + raise StemsLocationError("Pick a folder outside the current stems folder.") + + if target.exists(): + if not target.is_dir(): + raise StemsLocationError("That path is a file, not a folder.") + # An existing folder is fine only if it is empty or already ours: merging + # a stem library into someone's Desktop is not a recoverable mistake. + entries = list(target.iterdir()) + ours = {"registry.json", "failed"} + if entries and not all(e.name in ours or _looks_like_job_dir(e) for e in entries): + raise StemsLocationError("Pick an empty folder, or one StemDeck already uses.") + else: + try: + target.mkdir(parents=True, exist_ok=True) + except OSError as e: + raise StemsLocationError(f"Cannot create that folder: {e.strerror or e}") from e + + if not os.access(target, os.W_OK): + raise StemsLocationError("That folder is not writable.") + return target + + +def _looks_like_job_dir(entry: Path) -> bool: + from app.core.config import JOB_ID_RE + + return entry.is_dir() and bool(JOB_ID_RE.match(entry.name)) + + +def move_library(current: Path, target: Path) -> MoveResult: + """Move every stem directory from `current` into `target`. + + Entry by entry rather than moving the folder itself: the folder may be one + the user picked in a native dialog and expects to keep, and on desktop the + parent (~/Documents/StemDeck) also holds the library index, which stays put. + + On a failure partway, whatever has already moved stays moved and the error + names the entry that stopped it. Rolling back a half-finished multi-gigabyte + copy has its own failure modes, and the app can see a library in either + place -- it is the setting, written only on success, that decides which one + it reads. + """ + current = current.expanduser().resolve() + target = target.expanduser().resolve() + target.mkdir(parents=True, exist_ok=True) + + same_fs = _same_filesystem(current, target) + moved = 0 + moved_bytes = 0 + + if not current.exists(): + return MoveResult(str(current), str(target), 0, 0, same_fs) + + for entry in sorted(current.iterdir()): + destination = target / entry.name + if destination.exists(): + # A previous, interrupted attempt already brought this one over. + logger.info("stems move: %s already at the target, skipping", entry.name) + continue + size = directory_size(entry) if entry.is_dir() else entry.stat().st_size + try: + shutil.move(str(entry), str(destination)) + except OSError as e: + raise StemsLocationError( + f"Could not move {entry.name}: {e.strerror or e}. " + f"Anything already moved is in the new folder." + ) from e + moved += 1 + moved_bytes += size + + return MoveResult(str(current), str(target), moved, moved_bytes, same_fs) + + +def _same_filesystem(a: Path, b: Path) -> bool: + """Whether the move is a rename or a copy. Only used to set expectations in + the UI, so an unanswerable question is answered with "assume the slow one".""" + try: + probe = a if a.exists() else a.parent + other = b if b.exists() else b.parent + return probe.stat().st_dev == other.stat().st_dev + except OSError: + return False diff --git a/app/main.py b/app/main.py index 3bcc6859..1f398854 100644 --- a/app/main.py +++ b/app/main.py @@ -42,6 +42,7 @@ get_demucs_device, get_demucs_device_choice, get_export_sample_rate, + get_jobs_dir, get_max_duration_sec, get_playlist_max_items, get_port, @@ -50,12 +51,19 @@ set_allow_network, set_demucs_device, set_export_sample_rate, + set_jobs_dir, set_max_duration_sec, set_playlist_max_items, set_port, set_separation_quality, set_video_max_height, ) +from app.core.stems_location import ( + StemsLocationError, + directory_size, + move_library, + validate_target, +) from app.pipeline.collect import sweep_failed_jobs, sweep_old_jobs # Set the stemdeck logger level (Python's default root level of WARNING would @@ -345,6 +353,97 @@ async def update_settings(request: Request) -> dict[str, object]: _ACTIVE_JOB_STATUSES = ("queued", "downloading", "analyzing", "separating", "processing") +def _require_desktop_shell() -> None: + """Relocating the stem library is a desktop-app feature only (#354). + + A server, Docker or Unraid deployment gets its storage from a mounted volume + or an explicit STEMDECK_JOBS_DIR, decided by whoever runs it. Moving files + from inside the app there would fight the deployment: the mount would still + be the mount on the next start, and the library would be somewhere the + container no longer looks. + """ + if os.environ.get("STEMDECK_DESKTOP") != "1": + raise HTTPException( + status_code=403, + detail=( + "The stems folder is set by this deployment. " + "Change the mounted volume or STEMDECK_JOBS_DIR instead." + ), + ) + + +@app.get("/api/settings/stems-location", tags=["settings"]) +def get_stems_location() -> dict[str, object]: + """Where stems are stored now, and how much is there. + + The size is what makes the setting actionable -- "2.5 GB in your Documents + folder" is the thing the user is trying to fix (#354).""" + _require_desktop_shell() + return { + "path": str(JOBS_DIR), + "bytes": directory_size(JOBS_DIR) if JOBS_DIR.exists() else 0, + "is_default": get_jobs_dir() is None, + "busy": bool([j for j in registry_all_jobs().values() if j.status in _ACTIVE_JOB_STATUSES]), + } + + +@app.post("/api/settings/stems-location", tags=["settings"]) +async def set_stems_location(request: Request) -> dict[str, object]: + """Move the stem library somewhere else and remember the choice. + + The move is the point: the registry lives inside this folder, so changing + the setting alone would strand the library and show an empty app. The + preference is written only after the move succeeds, so a failure leaves the + app still reading the folder the files are actually in. + + Takes effect fully on the next start -- JOBS_DIR is bound at import time + across the app -- which is why the response says so. + """ + _require_desktop_shell() + try: + body = await request.json() + except Exception: + body = {} + raw = body.get("path") + if not isinstance(raw, str): + raise HTTPException(status_code=422, detail="path is required") + + active = [j for j in registry_all_jobs().values() if j.status in _ACTIVE_JOB_STATUSES] + if active: + # Moving files out from under a running separation would corrupt it. + raise HTTPException( + status_code=409, + detail="Finish or cancel the imports in the queue before moving the stems folder", + ) + + try: + target = validate_target(Path(raw), JOBS_DIR) + except StemsLocationError as e: + raise HTTPException(status_code=422, detail=str(e)) from None + + try: + result = await asyncio.to_thread(move_library, JOBS_DIR, target) + except StemsLocationError as e: + raise HTTPException(status_code=500, detail=str(e)) from None + except Exception: + _log.exception("moving the stems library failed") + raise HTTPException(status_code=500, detail="Could not move the stems folder") from None + + set_jobs_dir(str(target)) + _log.info( + "stems library moved to %s (%d entries, %d bytes)", + target, + result.moved_entries, + result.bytes_moved, + ) + return { + "path": str(target), + "moved_entries": result.moved_entries, + "bytes_moved": result.bytes_moved, + "restart_required": True, + } + + @app.post("/api/reset", tags=["settings"]) def reset_app_data() -> dict[str, object]: """Factory reset (Settings -> General -> "Reset app data"): delete every diff --git a/desktop/src-tauri/capabilities/default.json b/desktop/src-tauri/capabilities/default.json index 29dff59a..7e6c3e42 100644 --- a/desktop/src-tauri/capabilities/default.json +++ b/desktop/src-tauri/capabilities/default.json @@ -3,5 +3,5 @@ "identifier": "default", "description": "Default StemDeck desktop window permissions", "windows": ["main"], - "permissions": ["core:default", "dialog:allow-save"] + "permissions": ["core:default", "dialog:allow-save", "dialog:allow-open"] } diff --git a/desktop/src-tauri/src/main.rs b/desktop/src-tauri/src/main.rs index 9575c180..add7803c 100644 --- a/desktop/src-tauri/src/main.rs +++ b/desktop/src-tauri/src/main.rs @@ -237,6 +237,7 @@ fn main() { build_target, open_url, save_audio_file, + pick_stems_folder, store_get, store_set, reset_user_data, @@ -272,8 +273,13 @@ fn documents_store_path(app: &tauri::AppHandle) -> Result { Ok(documents_stemdeck_dir(app)?.join("user-data.json")) } -/// Returns ~/Documents/StemDeck/jobs/ (stem audio files). -/// Falls back to data_dir/jobs if document_dir is unavailable. +/// The DEFAULT stems folder: ~/Documents/StemDeck/jobs/. Falls back to +/// data_dir/jobs if document_dir is unavailable. +/// +/// Handed to the backend as STEMDECK_DEFAULT_JOBS_DIR, not STEMDECK_JOBS_DIR: +/// the latter means "this deployment pins the location" and would override the +/// folder the user picked in Settings (#354). The backend owns that choice; it +/// is the one that has to move the library when it changes. fn documents_dir_for_jobs(app: &tauri::AppHandle) -> PathBuf { match documents_stemdeck_dir(app) { Ok(dir) => { @@ -287,6 +293,23 @@ fn documents_dir_for_jobs(app: &tauri::AppHandle) -> PathBuf { } } +/// Native folder picker for the stems location. Returns None when the user +/// cancels, which the UI treats as "leave it where it is". +#[tauri::command] +async fn pick_stems_folder(app: tauri::AppHandle) -> Result, String> { + use tauri_plugin_dialog::DialogExt; + + let (tx, rx) = std::sync::mpsc::channel(); + app.dialog() + .file() + .set_title("Choose where StemDeck stores extracted stems") + .pick_folder(move |path| { + let _ = tx.send(path); + }); + let picked = rx.recv().map_err(|e| e.to_string())?; + Ok(picked.map(|p| p.to_string())) +} + /// Get a value from the persistent user-data store. #[tauri::command] fn store_get(app: tauri::AppHandle, key: String) -> Result, String> { @@ -653,7 +676,7 @@ fn start_backend( cmd.current_dir(&backend_dir) .env("STEMDECK_DATA_DIR", &data_dir) - .env("STEMDECK_JOBS_DIR", &jobs_dir) + .env("STEMDECK_DEFAULT_JOBS_DIR", &jobs_dir) .env("STEMDECK_DESKTOP", "1") .env("STEMDECK_PARENT_PID", std::process::id().to_string()) .env("PYTHONUNBUFFERED", "1") diff --git a/static/css/daw.css b/static/css/daw.css index da14a689..4f0df53b 100644 --- a/static/css/daw.css +++ b/static/css/daw.css @@ -2921,6 +2921,38 @@ input, textarea { font-family: inherit; } .settings-export-logs:hover:not(:disabled) { background: var(--panel-3); color: var(--fg); } .settings-export-logs:disabled { opacity: 0.5; cursor: default; } +/* Stems location (#354). The path is long, so this row stacks instead of + sitting on one line with the label. */ +.settings-row-stack { display: block; } +.settings-btn { + padding: 6px 12px; + background: var(--panel-2); border: 1px solid var(--border-strong); + border-radius: 7px; color: var(--fg-2); cursor: pointer; + font-family: inherit; font-size: 12px; font-weight: 600; white-space: nowrap; + transition: background var(--t-fast), color var(--t-fast); +} +.settings-btn:hover:not(:disabled) { background: var(--panel-3); color: var(--fg); } +.settings-btn:disabled { opacity: 0.5; cursor: default; } +.stems-location { + display: flex; align-items: center; gap: 8px; margin-top: 8px; +} +.stems-location-path { + flex: 1; min-width: 0; + padding: 6px 8px; border-radius: 6px; + background: var(--panel); border: 1px solid var(--border); + color: var(--fg-2); font-family: var(--font-mono); font-size: 11px; + white-space: nowrap; overflow: hidden; text-overflow: ellipsis; +} +.stems-location-size { + flex-shrink: 0; font-size: 11px; color: var(--muted); + font-variant-numeric: tabular-nums; +} +.stems-location-msg { + margin-top: 6px; font-size: 11px; line-height: 1.4; color: var(--muted); +} +.stems-location-msg.error { color: var(--danger); } +.stems-location-msg.ok { color: var(--accent); } + /* Logs sub-navigation: Location / Application / Setup. */ .settings-subtabs { display: flex; gap: 2px; margin: 0 0 10px; diff --git a/static/js/catalog.js b/static/js/catalog.js index 38e2ed1e..60dfbb9d 100644 --- a/static/js/catalog.js +++ b/static/js/catalog.js @@ -2493,6 +2493,117 @@ function networkSettingsHtml() { `; } +/** Long paths are truncated from the LEFT: the folder name is what the user + * needs to see, and the leading /Users/... is the part they already know. + * Done here rather than with CSS -- the direction:rtl trick that gives a + * leading ellipsis also moves the path's leading slash to the far end, so + * /private/tmp/x renders as tmp/x/ and reads like a different path. */ +export function shortenPath(path, max = 52) { + const text = String(path ?? ""); + if (text.length <= max) return text; + return "…" + text.slice(text.length - (max - 1)); +} + +function formatSize(bytes) { + if (!bytes) return ""; + const units = ["B", "KB", "MB", "GB", "TB"]; + let value = bytes; + let i = 0; + while (value >= 1024 && i < units.length - 1) { + value /= 1024; + i += 1; + } + return `${value >= 10 || i === 0 ? Math.round(value) : value.toFixed(1)} ${units[i]}`; +} + +// Where extracted stems live (#354). Documents is a fine default until you +// notice it is syncing tens of gigabytes to iCloud. +async function wireStemsLocation(overlay) { + const pathEl = overlay.querySelector(".stems-location-path"); + const sizeEl = overlay.querySelector(".stems-location-size"); + const btn = overlay.querySelector(".set-stems-location"); + const msg = overlay.querySelector(".stems-location-msg"); + if (!pathEl || !btn) return; + + // Desktop only. A server, Docker or Unraid deployment gets its storage from a + // mounted volume decided by whoever runs it -- moving files from inside the + // app would fight the mount, and the browser cannot pick a folder on a + // machine it is not running on. The backend refuses there too. + const invoke = window.__TAURI__?.core?.invoke; + if (!invoke) { + overlay.querySelector(".stems-location")?.closest(".settings-row")?.remove(); + return; + } + + let current = null; + + const setMessage = (text, kind = "") => { + if (!msg) return; + msg.textContent = text || ""; + msg.className = `stems-location-msg${kind ? " " + kind : ""}`; + }; + + const apply = (d) => { + current = d.path; + pathEl.textContent = shortenPath(d.path); + pathEl.title = d.path; + if (sizeEl) sizeEl.textContent = formatSize(d.bytes); + }; + + try { + const r = await fetch("/api/settings/stems-location", { cache: "no-store" }); + if (r.ok) apply(await r.json()); + } catch (e) { + console.warn("[settings] could not read the stems location:", e); + } + + btn.addEventListener("click", async () => { + let picked = null; + try { + picked = await invoke("pick_stems_folder"); + } catch (e) { + console.warn("[settings] folder picker failed:", e); + setMessage("Could not open the folder picker.", "error"); + return; + } + if (!picked) return; // cancelled + + btn.disabled = true; + setMessage("Moving stems… this can take a while for a large library."); + try { + const r = await fetch("/api/settings/stems-location", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ path: picked }), + }); + const data = await r.json(); + if (!r.ok) { + setMessage(data.detail || "Could not move the stems folder.", "error"); + return; + } + apply({ path: data.path, bytes: 0 }); + setMessage( + `Moved ${data.moved_entries} item${data.moved_entries === 1 ? "" : "s"}. ` + + "Restart StemDeck to finish switching over.", + "ok", + ); + // The size we had is meaningless now; re-read it from the new location. + try { + const again = await fetch("/api/settings/stems-location", { cache: "no-store" }); + if (again.ok) { + const d = await again.json(); + if (sizeEl) sizeEl.textContent = formatSize(d.bytes); + } + } catch (e) { console.warn("[settings] size refresh failed:", e); } + } catch (e) { + console.warn("[settings] move failed:", e); + setMessage("Could not reach the server.", "error"); + } finally { + btn.disabled = false; + } + }); +} + // General settings: max track length (minutes), playlist import limit, and // MP4 video quality. Read live // and POSTed on change to /api/settings (same runtime store as the toggle). @@ -2920,6 +3031,21 @@ function openLibraryEditor() { +
+
+
Where stems are stored
+
+ Extracted stems are large. Keep them out of a folder that syncs to iCloud or OneDrive + unless you want them backed up. Moving takes effect after a restart. +
+
+
+ + + +
+
+
@@ -3115,6 +3241,7 @@ function openLibraryEditor() { refreshLibrarySyncSummary(); const isDesktop = Boolean(window.__TAURI__?.core?.invoke); wireGeneralSettings(overlay); + wireStemsLocation(overlay); wireNetworkSetting(overlay); if (!isDesktop) { overlay.querySelector(".net-access-input")?.setAttribute("disabled", ""); diff --git a/tests/test_stems_location.py b/tests/test_stems_location.py new file mode 100644 index 00000000..351537f2 --- /dev/null +++ b/tests/test_stems_location.py @@ -0,0 +1,298 @@ +"""Moving the stem library to a different folder (#354). + +The setting is bookkeeping; the move is the feature, and it is the part that can +destroy someone's library. Most of what follows is about refusing to. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from fastapi.testclient import TestClient + +from app.core.models import Job +from app.core.registry import _jobs +from app.core.stems_location import ( + StemsLocationError, + directory_size, + move_library, + validate_target, +) + + +@pytest.fixture(autouse=True) +def _isolate_registry(): + _jobs.clear() + yield + _jobs.clear() + + +def _job_dir(root: Path, job_id: str, *, size: int = 32) -> Path: + d = root / job_id / "stems" + d.mkdir(parents=True, exist_ok=True) + (d / "vocals.wav").write_bytes(b"R" * size) + return root / job_id + + +# ── validation ─────────────────────────────────────────────────────────────── + + +def test_accepts_an_empty_folder(tmp_path): + current = tmp_path / "old" + current.mkdir() + target = tmp_path / "new" + target.mkdir() + assert validate_target(target, current) == target.resolve() + + +def test_creates_the_folder_when_it_does_not_exist(tmp_path): + current = tmp_path / "old" + current.mkdir() + target = tmp_path / "brand" / "new" + assert validate_target(target, current) == target.resolve() + assert target.is_dir() + + +def test_rejects_the_current_location(tmp_path): + current = tmp_path / "old" + current.mkdir() + with pytest.raises(StemsLocationError, match="already stored"): + validate_target(current, current) + + +def test_rejects_a_folder_inside_the_current_one(tmp_path): + """Moving a directory into itself would recurse forever.""" + current = tmp_path / "old" + (current / "inner").mkdir(parents=True) + with pytest.raises(StemsLocationError, match="outside"): + validate_target(current / "inner", current) + + +def test_rejects_a_folder_full_of_the_users_own_files(tmp_path): + """Merging a stem library into someone's Desktop is not recoverable.""" + current = tmp_path / "old" + current.mkdir() + target = tmp_path / "desktop" + target.mkdir() + (target / "tax return.pdf").write_bytes(b"%PDF") + with pytest.raises(StemsLocationError, match="empty folder"): + validate_target(target, current) + + +def test_accepts_a_folder_stemdeck_already_uses(tmp_path): + """Retrying an interrupted move must not be blocked by its own progress.""" + current = tmp_path / "old" + current.mkdir() + target = tmp_path / "new" + _job_dir(target, "abcdefabcdef") + (target / "registry.json").write_text("{}", encoding="utf-8") + assert validate_target(target, current) == target.resolve() + + +def test_rejects_a_file(tmp_path): + current = tmp_path / "old" + current.mkdir() + target = tmp_path / "notafolder.txt" + target.write_text("x", encoding="utf-8") + with pytest.raises(StemsLocationError, match="not a folder"): + validate_target(target, current) + + +def test_rejects_a_relative_path(tmp_path): + current = tmp_path / "old" + current.mkdir() + with pytest.raises(StemsLocationError, match="absolute"): + validate_target(Path("stems"), current) + + +def test_rejects_an_empty_path(tmp_path): + current = tmp_path / "old" + current.mkdir() + with pytest.raises(StemsLocationError): + validate_target(Path(""), current) + + +# ── the move ───────────────────────────────────────────────────────────────── + + +def test_moves_every_job_and_the_registry(tmp_path): + current = tmp_path / "old" + _job_dir(current, "aaaaaaaaaaaa") + _job_dir(current, "bbbbbbbbbbbb") + (current / "registry.json").write_text('{"jobs": []}', encoding="utf-8") + target = tmp_path / "new" + + result = move_library(current, target) + + assert result.moved_entries == 3 + assert (target / "aaaaaaaaaaaa" / "stems" / "vocals.wav").is_file() + assert (target / "bbbbbbbbbbbb" / "stems" / "vocals.wav").is_file() + assert (target / "registry.json").is_file() + assert list(current.iterdir()) == [], "nothing should be left behind" + + +def test_the_registry_moves_with_the_stems(tmp_path): + """It lives inside the stems folder, so leaving it behind would bring the + app back empty with the files stranded.""" + current = tmp_path / "old" + current.mkdir() + (current / "registry.json").write_text('{"jobs": [{"id": "x"}]}', encoding="utf-8") + target = tmp_path / "new" + + move_library(current, target) + + assert not (current / "registry.json").exists() + assert '"id": "x"' in (target / "registry.json").read_text(encoding="utf-8") + + +def test_an_interrupted_move_can_be_resumed(tmp_path): + """Entries already at the target are skipped rather than colliding.""" + current = tmp_path / "old" + _job_dir(current, "aaaaaaaaaaaa") + _job_dir(current, "bbbbbbbbbbbb") + target = tmp_path / "new" + _job_dir(target, "aaaaaaaaaaaa") # a previous attempt got this far + + result = move_library(current, target) + + assert result.moved_entries == 1 + assert (target / "bbbbbbbbbbbb").is_dir() + + +def test_moving_an_empty_library_is_not_an_error(tmp_path): + current = tmp_path / "old" + current.mkdir() + result = move_library(current, tmp_path / "new") + assert result.moved_entries == 0 + + +def test_moving_from_a_folder_that_never_existed(tmp_path): + result = move_library(tmp_path / "never", tmp_path / "new") + assert result.moved_entries == 0 + + +def test_directory_size_adds_up(tmp_path): + _job_dir(tmp_path, "aaaaaaaaaaaa", size=100) + _job_dir(tmp_path, "bbbbbbbbbbbb", size=50) + assert directory_size(tmp_path) == 150 + + +# ── the endpoint ───────────────────────────────────────────────────────────── + + +@pytest.fixture +def client(tmp_path, monkeypatch): + monkeypatch.setenv("STEMDECK_DESKTOP", "1") + jobs = tmp_path / "current" + jobs.mkdir() + import app.core.settings as settings_mod + import app.main as main_mod + + monkeypatch.setattr(main_mod, "JOBS_DIR", jobs) + monkeypatch.setattr(settings_mod, "_SETTINGS_PATH", tmp_path / "settings.json") + settings_mod._state = None + from app.main import app + + with TestClient(app) as c: + c.jobs_dir = jobs + yield c + settings_mod._state = None + + +def test_reports_the_current_location_and_size(client, tmp_path): + _job_dir(client.jobs_dir, "aaaaaaaaaaaa", size=200) + body = client.get("/api/settings/stems-location").json() + assert body["path"] == str(client.jobs_dir) + assert body["bytes"] == 200 + assert body["is_default"] is True + assert body["busy"] is False + + +def test_moving_writes_the_setting_and_asks_for_a_restart(client, tmp_path): + _job_dir(client.jobs_dir, "aaaaaaaaaaaa") + target = tmp_path / "elsewhere" + + body = client.post("/api/settings/stems-location", json={"path": str(target)}).json() + + assert body["path"] == str(target.resolve()) + assert body["moved_entries"] == 1 + assert body["restart_required"] is True + assert (target / "aaaaaaaaaaaa").is_dir() + + from app.core.settings import get_jobs_dir + + assert get_jobs_dir() == str(target.resolve()) + + +def test_refuses_while_a_job_is_running(client, tmp_path): + """Moving files out from under a separation would corrupt it.""" + job = Job(id="aaaaaaaaaaaa") + job.status = "separating" + _jobs[job.id] = job + _job_dir(client.jobs_dir, "bbbbbbbbbbbb") + + r = client.post("/api/settings/stems-location", json={"path": str(tmp_path / "elsewhere")}) + + assert r.status_code == 409 + assert (client.jobs_dir / "bbbbbbbbbbbb").is_dir(), "nothing may move while busy" + + +def test_refuses_while_a_job_is_merely_queued(client, tmp_path): + job = Job(id="aaaaaaaaaaaa") + job.status = "queued" + _jobs[job.id] = job + r = client.post("/api/settings/stems-location", json={"path": str(tmp_path / "elsewhere")}) + assert r.status_code == 409 + + +def test_a_rejected_target_leaves_the_setting_alone(client, tmp_path): + """The preference is only written after the move succeeds, so a failure + leaves the app reading the folder the files are actually in.""" + from app.core.settings import get_jobs_dir + + occupied = tmp_path / "someone-elses" + occupied.mkdir() + (occupied / "holiday.jpg").write_bytes(b"\xff\xd8") + + r = client.post("/api/settings/stems-location", json={"path": str(occupied)}) + + assert r.status_code == 422 + assert get_jobs_dir() is None + + +def test_missing_path_is_a_422(client): + assert client.post("/api/settings/stems-location", json={}).status_code == 422 + + +# ── desktop only ───────────────────────────────────────────────────────────── + + +@pytest.fixture +def server_client(tmp_path, monkeypatch): + """The same app without the desktop shell: a self-hosted server, Docker or + Unraid deployment.""" + monkeypatch.delenv("STEMDECK_DESKTOP", raising=False) + jobs = tmp_path / "current" + jobs.mkdir() + import app.main as main_mod + + monkeypatch.setattr(main_mod, "JOBS_DIR", jobs) + from app.main import app + + with TestClient(app) as c: + c.jobs_dir = jobs + yield c + + +def test_not_offered_outside_the_desktop_app(server_client): + """Docker and Unraid mount their storage; the location is the operator's + decision, and it is still the mount on the next start.""" + assert server_client.get("/api/settings/stems-location").status_code == 403 + + +def test_cannot_be_moved_outside_the_desktop_app(server_client, tmp_path): + _job_dir(server_client.jobs_dir, "aaaaaaaaaaaa") + r = server_client.post("/api/settings/stems-location", json={"path": str(tmp_path / "new")}) + assert r.status_code == 403 + assert (server_client.jobs_dir / "aaaaaaaaaaaa").is_dir(), "nothing may move" From fbcb522d9e929397f58c8ef62954b483d2c321df Mon Sep 17 00:00:00 2001 From: Thales Pereira <31625914+thcp@users.noreply.github.com> Date: Tue, 11 Aug 2026 22:11:20 +0100 Subject: [PATCH 2/6] refactor(settings): let the backend say whether the stems folder can 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. --- app/main.py | 16 ++++++++--- static/js/catalog.js | 54 +++++++++++++++++++++++------------- tests/test_stems_location.py | 9 ++++-- 3 files changed, 53 insertions(+), 26 deletions(-) diff --git a/app/main.py b/app/main.py index 1f398854..1fc48534 100644 --- a/app/main.py +++ b/app/main.py @@ -353,7 +353,7 @@ async def update_settings(request: Request) -> dict[str, object]: _ACTIVE_JOB_STATUSES = ("queued", "downloading", "analyzing", "separating", "processing") -def _require_desktop_shell() -> None: +def _stems_location_editable() -> bool: """Relocating the stem library is a desktop-app feature only (#354). A server, Docker or Unraid deployment gets its storage from a mounted volume @@ -362,7 +362,11 @@ def _require_desktop_shell() -> None: be the mount on the next start, and the library would be somewhere the container no longer looks. """ - if os.environ.get("STEMDECK_DESKTOP") != "1": + return os.environ.get("STEMDECK_DESKTOP") == "1" + + +def _require_desktop_shell() -> None: + if not _stems_location_editable(): raise HTTPException( status_code=403, detail=( @@ -377,11 +381,15 @@ def get_stems_location() -> dict[str, object]: """Where stems are stored now, and how much is there. The size is what makes the setting actionable -- "2.5 GB in your Documents - folder" is the thing the user is trying to fix (#354).""" - _require_desktop_shell() + folder" is the thing the user is trying to fix (#354). + + Answers everywhere, including deployments that cannot change it: `editable` + is how the UI knows whether to offer the control at all. Probing a 403 + instead would log a failed request every time Settings is opened.""" return { "path": str(JOBS_DIR), "bytes": directory_size(JOBS_DIR) if JOBS_DIR.exists() else 0, + "editable": _stems_location_editable(), "is_default": get_jobs_dir() is None, "busy": bool([j for j in registry_all_jobs().values() if j.status in _ACTIVE_JOB_STATUSES]), } diff --git a/static/js/catalog.js b/static/js/catalog.js index 60dfbb9d..37c57b21 100644 --- a/static/js/catalog.js +++ b/static/js/catalog.js @@ -2525,15 +2525,8 @@ async function wireStemsLocation(overlay) { const msg = overlay.querySelector(".stems-location-msg"); if (!pathEl || !btn) return; - // Desktop only. A server, Docker or Unraid deployment gets its storage from a - // mounted volume decided by whoever runs it -- moving files from inside the - // app would fight the mount, and the browser cannot pick a folder on a - // machine it is not running on. The backend refuses there too. - const invoke = window.__TAURI__?.core?.invoke; - if (!invoke) { + const hideRow = () => overlay.querySelector(".stems-location")?.closest(".settings-row")?.remove(); - return; - } let current = null; @@ -2550,21 +2543,46 @@ async function wireStemsLocation(overlay) { if (sizeEl) sizeEl.textContent = formatSize(d.bytes); }; + // The backend decides whether this setting exists at all -- it is false on a + // server, Docker or Unraid deployment, where storage comes from a mounted + // volume the operator chose and moving it from inside the app would fight the + // mount. Asking it, rather than sniffing for Tauri, keeps that judgement in + // one place and means the row is testable in a browser against a desktop + // backend. try { const r = await fetch("/api/settings/stems-location", { cache: "no-store" }); - if (r.ok) apply(await r.json()); + if (!r.ok) { + hideRow(); + return; + } + const data = await r.json(); + if (!data.editable) { + hideRow(); + return; + } + apply(data); } catch (e) { console.warn("[settings] could not read the stems location:", e); + hideRow(); + return; } btn.addEventListener("click", async () => { let picked = null; - try { - picked = await invoke("pick_stems_folder"); - } catch (e) { - console.warn("[settings] folder picker failed:", e); - setMessage("Could not open the folder picker.", "error"); - return; + const invoke = window.__TAURI__?.core?.invoke; + if (invoke) { + try { + picked = await invoke("pick_stems_folder"); + } catch (e) { + console.warn("[settings] folder picker failed:", e); + setMessage("Could not open the folder picker.", "error"); + return; + } + } else { + // No native picker outside the desktop shell. Only reachable when a + // desktop-mode backend is being driven from a browser, which is a + // development setup -- in the shipped app invoke is always there. + picked = window.prompt("Full path to the folder for extracted stems:", current || ""); } if (!picked) return; // cancelled @@ -3033,11 +3051,7 @@ function openLibraryEditor() {
-
Where stems are stored
-
- Extracted stems are large. Keep them out of a folder that syncs to iCloud or OneDrive - unless you want them backed up. Moving takes effect after a restart. -
+
StemData location
diff --git a/tests/test_stems_location.py b/tests/test_stems_location.py index 351537f2..b04db266 100644 --- a/tests/test_stems_location.py +++ b/tests/test_stems_location.py @@ -207,6 +207,7 @@ def test_reports_the_current_location_and_size(client, tmp_path): assert body["bytes"] == 200 assert body["is_default"] is True assert body["busy"] is False + assert body["editable"] is True def test_moving_writes_the_setting_and_asks_for_a_restart(client, tmp_path): @@ -287,8 +288,12 @@ def server_client(tmp_path, monkeypatch): def test_not_offered_outside_the_desktop_app(server_client): """Docker and Unraid mount their storage; the location is the operator's - decision, and it is still the mount on the next start.""" - assert server_client.get("/api/settings/stems-location").status_code == 403 + decision, and it is still the mount on the next start. + + The read still answers -- that flag is how the UI knows to hide the control. + Refusing it would log a failed request every time Settings is opened.""" + body = server_client.get("/api/settings/stems-location").json() + assert body["editable"] is False def test_cannot_be_moved_outside_the_desktop_app(server_client, tmp_path): From 063fae304e99c228f1bcc30ab10deefa639ef9fb Mon Sep 17 00:00:00 2001 From: Thales Pereira <31625914+thcp@users.noreply.github.com> Date: Tue, 11 Aug 2026 22:18:17 +0100 Subject: [PATCH 3/6] test: use one import style per module in the stems-location tests 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. --- tests/test_stems_location.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/test_stems_location.py b/tests/test_stems_location.py index b04db266..3f4fc47f 100644 --- a/tests/test_stems_location.py +++ b/tests/test_stems_location.py @@ -192,9 +192,8 @@ def client(tmp_path, monkeypatch): monkeypatch.setattr(main_mod, "JOBS_DIR", jobs) monkeypatch.setattr(settings_mod, "_SETTINGS_PATH", tmp_path / "settings.json") settings_mod._state = None - from app.main import app - with TestClient(app) as c: + with TestClient(main_mod.app) as c: c.jobs_dir = jobs yield c settings_mod._state = None @@ -221,9 +220,9 @@ def test_moving_writes_the_setting_and_asks_for_a_restart(client, tmp_path): assert body["restart_required"] is True assert (target / "aaaaaaaaaaaa").is_dir() - from app.core.settings import get_jobs_dir + import app.core.settings as settings_mod - assert get_jobs_dir() == str(target.resolve()) + assert settings_mod.get_jobs_dir() == str(target.resolve()) def test_refuses_while_a_job_is_running(client, tmp_path): @@ -250,7 +249,7 @@ def test_refuses_while_a_job_is_merely_queued(client, tmp_path): def test_a_rejected_target_leaves_the_setting_alone(client, tmp_path): """The preference is only written after the move succeeds, so a failure leaves the app reading the folder the files are actually in.""" - from app.core.settings import get_jobs_dir + import app.core.settings as settings_mod occupied = tmp_path / "someone-elses" occupied.mkdir() @@ -259,7 +258,7 @@ def test_a_rejected_target_leaves_the_setting_alone(client, tmp_path): r = client.post("/api/settings/stems-location", json={"path": str(occupied)}) assert r.status_code == 422 - assert get_jobs_dir() is None + assert settings_mod.get_jobs_dir() is None def test_missing_path_is_a_422(client): @@ -279,9 +278,8 @@ def server_client(tmp_path, monkeypatch): import app.main as main_mod monkeypatch.setattr(main_mod, "JOBS_DIR", jobs) - from app.main import app - with TestClient(app) as c: + with TestClient(main_mod.app) as c: c.jobs_dir = jobs yield c From 99cb378bc389994be1c82ca571f96b8ea484d0ea Mon Sep 17 00:00:00 2001 From: Thales Pereira <31625914+thcp@users.noreply.github.com> Date: Wed, 12 Aug 2026 09:18:35 +0100 Subject: [PATCH 4/6] test: pin how JOBS_DIR is resolved 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. --- tests/test_stems_location.py | 94 ++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/tests/test_stems_location.py b/tests/test_stems_location.py index 3f4fc47f..6a28e3b7 100644 --- a/tests/test_stems_location.py +++ b/tests/test_stems_location.py @@ -299,3 +299,97 @@ def test_cannot_be_moved_outside_the_desktop_app(server_client, tmp_path): r = server_client.post("/api/settings/stems-location", json={"path": str(tmp_path / "new")}) assert r.status_code == 403 assert (server_client.jobs_dir / "aaaaaaaaaaaa").is_dir(), "nothing may move" + + +# ── where JOBS_DIR comes from ──────────────────────────────────────────────── +# +# Resolved at import time across the app, so these run in a subprocess. Worth +# the awkwardness: get this wrong and every existing desktop user opens the app +# to an empty library after an update, with their stems still on disk. + + +def _resolve_jobs_dir(tmp_path: Path, env_extra: dict, settings: dict | None) -> str: + import json + import os + import subprocess + import sys + + data = tmp_path / "data" + data.mkdir(exist_ok=True) + settings_file = data / "settings.json" + if settings is None: + settings_file.unlink(missing_ok=True) + else: + settings_file.write_text(json.dumps(settings), encoding="utf-8") + + env = {**os.environ, "STEMDECK_DATA_DIR": str(data)} + env.pop("STEMDECK_JOBS_DIR", None) + env.pop("STEMDECK_DEFAULT_JOBS_DIR", None) + env.update(env_extra) + out = subprocess.run( + [sys.executable, "-c", "from app.core.config import JOBS_DIR; print(JOBS_DIR)"], + env=env, + capture_output=True, + text=True, + cwd=str(Path(__file__).resolve().parent.parent), + check=True, + ) + return out.stdout.strip() + + +def test_the_desktop_default_is_used_until_the_user_chooses(tmp_path): + """The upgrade path. The launcher passes its Documents folder as a DEFAULT, + and with no stored choice that is exactly where the library must stay.""" + default = tmp_path / "documents-jobs" + got = _resolve_jobs_dir(tmp_path, {"STEMDECK_DEFAULT_JOBS_DIR": str(default)}, None) + assert got == str(default) + + +def test_a_stored_choice_beats_the_desktop_default(tmp_path): + chosen = tmp_path / "chosen" + got = _resolve_jobs_dir( + tmp_path, + {"STEMDECK_DEFAULT_JOBS_DIR": str(tmp_path / "documents-jobs")}, + {"jobs_dir": str(chosen)}, + ) + assert got == str(chosen) + + +def test_an_explicit_pin_beats_everything(tmp_path): + """Docker and Unraid mount their storage. A stray setting in the image must + not send the library somewhere the container no longer looks.""" + mount = tmp_path / "mount" + got = _resolve_jobs_dir( + tmp_path, + { + "STEMDECK_JOBS_DIR": str(mount), + "STEMDECK_DEFAULT_JOBS_DIR": str(tmp_path / "documents-jobs"), + }, + {"jobs_dir": str(tmp_path / "chosen")}, + ) + assert got == str(mount) + + +def test_a_corrupt_setting_falls_back_rather_than_moving_the_library(tmp_path): + """A library that quietly relocates because a JSON file got mangled is far + worse than one that ignores the preference.""" + default = tmp_path / "documents-jobs" + (tmp_path / "data").mkdir(exist_ok=True) + (tmp_path / "data" / "settings.json").write_text("{not json", encoding="utf-8") + + import os + import subprocess + import sys + + env = {**os.environ, "STEMDECK_DATA_DIR": str(tmp_path / "data")} + env.pop("STEMDECK_JOBS_DIR", None) + env["STEMDECK_DEFAULT_JOBS_DIR"] = str(default) + out = subprocess.run( + [sys.executable, "-c", "from app.core.config import JOBS_DIR; print(JOBS_DIR)"], + env=env, + capture_output=True, + text=True, + cwd=str(Path(__file__).resolve().parent.parent), + check=True, + ) + assert out.stdout.strip() == str(default) From 7fa5bf5d68705732645b130b2156946e9cfc1079 Mon Sep 17 00:00:00 2001 From: Thales Pereira <31625914+thcp@users.noreply.github.com> Date: Wed, 12 Aug 2026 09:57:51 +0100 Subject: [PATCH 5/6] fix(settings): do not accept imports between the move and the restart 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. --- app/api/jobs.py | 8 +++++ app/api/playlist.py | 7 ++++ app/core/config.py | 9 +++++- app/core/stems_location.py | 27 ++++++++++++++++ app/main.py | 8 +++++ tests/test_stems_location.py | 62 ++++++++++++++++++++++++++++++++++++ 6 files changed, 120 insertions(+), 1 deletion(-) diff --git a/app/api/jobs.py b/app/api/jobs.py index 65882bf7..1e781418 100644 --- a/app/api/jobs.py +++ b/app/api/jobs.py @@ -31,6 +31,7 @@ from app.core.registry import register_if_capacity as registry_register_if_capacity from app.core.registry import remove as registry_remove from app.core.settings import get_max_duration_sec +from app.core.stems_location import is_relocating from app.pipeline import jobqueue from app.pipeline.download import InvalidYouTubeURL, validate_youtube_url @@ -121,6 +122,13 @@ class JobRequest(BaseModel): async def create_job(request: Request) -> dict[str, str]: """Submit a YouTube URL (JSON body) or upload an audio file (multipart/form-data) to start a stem-separation job. Returns the new job ID.""" + if is_relocating(): + # The stems folder just moved. This process still writes to the old one, + # so anything accepted now would be orphaned by the restart. + raise HTTPException( + status_code=409, + detail="Restart StemDeck to finish moving your stems folder before importing", + ) ct = request.headers.get("content-type", "") if "multipart/form-data" in ct: return await _create_local_job(request) diff --git a/app/api/playlist.py b/app/api/playlist.py index bc7cbdb7..bd0ac63b 100644 --- a/app/api/playlist.py +++ b/app/api/playlist.py @@ -24,6 +24,7 @@ from app.core.registry import persist as registry_persist from app.core.registry import register_if_capacity as registry_register_if_capacity from app.core.settings import get_max_duration_sec, get_playlist_max_items +from app.core.stems_location import is_relocating from app.pipeline import jobqueue from app.pipeline.download import InvalidPlaylistURL, expand_playlist @@ -121,6 +122,12 @@ async def create_playlist_jobs(request: Request) -> dict[str, Any]: except Exception as e: raise HTTPException(status_code=422, detail=str(e)) from e + if is_relocating(): + raise HTTPException( + status_code=409, + detail="Restart StemDeck to finish moving your stems folder before importing", + ) + selected = [s for s in payload.stems if s in STEM_NAMES] if payload.stems else list(STEM_NAMES) if not selected: selected = list(STEM_NAMES) diff --git a/app/core/config.py b/app/core/config.py index f41af75d..65692556 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -78,7 +78,14 @@ def _stored_jobs_dir() -> Path | None: value = raw.get("jobs_dir") if isinstance(raw, dict) else None if not isinstance(value, str) or not value.strip(): return None - return Path(value).expanduser() + path = Path(value).expanduser() + # Only honour a folder that is actually there. It existed when the user + # picked it, so a missing one means the disk holding it is not mounted -- + # and ensure_runtime_dirs would otherwise happily mkdir it, which on macOS + # creates a real directory at the mount point on the boot disk and can stop + # the drive mounting under its own name later. Falling back to the default + # leaves the library findable again as soon as the disk is plugged in. + return path if path.is_dir() else None # Where extracted stems live. Precedence, most explicit first: diff --git a/app/core/stems_location.py b/app/core/stems_location.py index a567ba55..3366eca9 100644 --- a/app/core/stems_location.py +++ b/app/core/stems_location.py @@ -26,6 +26,33 @@ class StemsLocationError(ValueError): """Rejected before anything on disk was touched.""" +# JOBS_DIR is bound at import time across the app, so a move leaves this process +# still writing to the folder the files just left. An import accepted in that +# window lands in the old folder and disappears from the library on the next +# start -- the stems are on disk, in a directory nothing looks at any more. +# +# Set before the move begins (which also closes the race against an import +# arriving mid-move) and never cleared on success: the restart is what clears +# it, and the restart is the point. +_relocating = False + + +def is_relocating() -> bool: + return _relocating + + +def begin_relocation() -> None: + global _relocating + _relocating = True + + +def abandon_relocation() -> None: + """The move failed, so the library is still where this process thinks it is + and the app stays usable.""" + global _relocating + _relocating = False + + @dataclass(frozen=True) class MoveResult: source: str diff --git a/app/main.py b/app/main.py index 1fc48534..043f2c6f 100644 --- a/app/main.py +++ b/app/main.py @@ -60,6 +60,8 @@ ) from app.core.stems_location import ( StemsLocationError, + abandon_relocation, + begin_relocation, directory_size, move_library, validate_target, @@ -429,11 +431,17 @@ async def set_stems_location(request: Request) -> dict[str, object]: except StemsLocationError as e: raise HTTPException(status_code=422, detail=str(e)) from None + # Stop accepting imports before a single file moves. An import arriving + # mid-move, or after it while this process still points at the old folder, + # would write stems somewhere the next start does not look. + begin_relocation() try: result = await asyncio.to_thread(move_library, JOBS_DIR, target) except StemsLocationError as e: + abandon_relocation() raise HTTPException(status_code=500, detail=str(e)) from None except Exception: + abandon_relocation() _log.exception("moving the stems library failed") raise HTTPException(status_code=500, detail="Could not move the stems folder") from None diff --git a/tests/test_stems_location.py b/tests/test_stems_location.py index 6a28e3b7..477c7759 100644 --- a/tests/test_stems_location.py +++ b/tests/test_stems_location.py @@ -23,9 +23,13 @@ @pytest.fixture(autouse=True) def _isolate_registry(): + from app.core import stems_location + _jobs.clear() + stems_location.abandon_relocation() yield _jobs.clear() + stems_location.abandon_relocation() def _job_dir(root: Path, job_id: str, *, size: int = 32) -> Path: @@ -188,6 +192,11 @@ def client(tmp_path, monkeypatch): jobs.mkdir() import app.core.settings as settings_mod import app.main as main_mod + from app.pipeline import jobqueue + + # Accepting a submit is the whole assertion here; letting the worker pick it + # up would send the suite to YouTube. + monkeypatch.setattr(jobqueue, "enqueue", lambda job_id, **kw: None) monkeypatch.setattr(main_mod, "JOBS_DIR", jobs) monkeypatch.setattr(settings_mod, "_SETTINGS_PATH", tmp_path / "settings.json") @@ -347,6 +356,7 @@ def test_the_desktop_default_is_used_until_the_user_chooses(tmp_path): def test_a_stored_choice_beats_the_desktop_default(tmp_path): chosen = tmp_path / "chosen" + chosen.mkdir() # it exists in reality: the move creates it before the setting is written got = _resolve_jobs_dir( tmp_path, {"STEMDECK_DEFAULT_JOBS_DIR": str(tmp_path / "documents-jobs")}, @@ -393,3 +403,55 @@ def test_a_corrupt_setting_falls_back_rather_than_moving_the_library(tmp_path): check=True, ) assert out.stdout.strip() == str(default) + + +def test_a_configured_folder_that_is_gone_falls_back(tmp_path): + """An external disk that is not mounted. Creating the folder instead would + put a phantom directory at the mount point on the boot disk, and the user's + library would look empty with no hint why.""" + default = tmp_path / "documents-jobs" + got = _resolve_jobs_dir( + tmp_path, + {"STEMDECK_DEFAULT_JOBS_DIR": str(default)}, + {"jobs_dir": str(tmp_path / "unplugged-drive" / "StemDeck")}, + ) + assert got == str(default) + assert not (tmp_path / "unplugged-drive").exists(), "must not create the missing path" + + +# ── the window between the move and the restart ────────────────────────────── + + +def test_importing_is_refused_until_the_restart(client, tmp_path): + """JOBS_DIR is bound at import time, so this process still writes to the old + folder after a move. A track accepted here would be orphaned by the restart: + on disk, in a directory nothing looks at any more.""" + _job_dir(client.jobs_dir, "aaaaaaaaaaaa") + client.post("/api/settings/stems-location", json={"path": str(tmp_path / "elsewhere")}) + + r = client.post("/api/jobs", json={"url": "https://youtu.be/dQw4w9WgXcQ"}) + + assert r.status_code == 409 + assert "Restart" in r.json()["detail"] + assert not list(client.jobs_dir.glob("*/")), "nothing new may appear in the old folder" + + +def test_a_playlist_import_is_refused_too(client, tmp_path): + client.post("/api/settings/stems-location", json={"path": str(tmp_path / "elsewhere")}) + r = client.post("/api/playlist", json={"url": "https://www.youtube.com/playlist?list=PLabc"}) + assert r.status_code == 409 + + +def test_a_failed_move_leaves_the_app_usable(client, tmp_path): + """Nothing moved, so this process is still right about where the library is + and there is no reason to make the user restart.""" + from app.core import stems_location + + occupied = tmp_path / "someone-elses" + occupied.mkdir() + (occupied / "holiday.jpg").write_bytes(b"\xff\xd8") + + client.post("/api/settings/stems-location", json={"path": str(occupied)}) + + assert stems_location.is_relocating() is False + assert client.post("/api/jobs", json={"url": "https://youtu.be/dQw4w9WgXcQ"}).status_code == 200 From faf2bff55472193e0b7d784b922c07d97f6ebae7 Mon Sep 17 00:00:00 2001 From: Thales Pereira <31625914+thcp@users.noreply.github.com> Date: Wed, 12 Aug 2026 11:12:19 +0100 Subject: [PATCH 6/6] fix(settings): close the import door before validating, and report the 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. --- app/main.py | 44 +++++++++++++++++++++++------------- static/js/catalog.js | 21 +++++++++-------- tests/test_stems_location.py | 36 +++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 25 deletions(-) diff --git a/app/main.py b/app/main.py index 043f2c6f..38f4d5d9 100644 --- a/app/main.py +++ b/app/main.py @@ -387,12 +387,21 @@ def get_stems_location() -> dict[str, object]: Answers everywhere, including deployments that cannot change it: `editable` is how the UI knows whether to offer the control at all. Probing a 403 - instead would log a failed request every time Settings is opened.""" + instead would log a failed request every time Settings is opened. + + After a move the stored choice and this process disagree until the restart: + the files are at the new path, JOBS_DIR still points at the old one. Report + where the stems actually are, or reopening Settings would show the folder + the user just moved away from.""" + configured = get_jobs_dir() + pending = bool(configured and configured != str(JOBS_DIR)) + path = Path(configured) if configured else JOBS_DIR return { - "path": str(JOBS_DIR), - "bytes": directory_size(JOBS_DIR) if JOBS_DIR.exists() else 0, + "path": str(path), + "bytes": directory_size(path) if path.exists() else 0, "editable": _stems_location_editable(), - "is_default": get_jobs_dir() is None, + "is_default": configured is None, + "restart_required": pending, "busy": bool([j for j in registry_all_jobs().values() if j.status in _ACTIVE_JOB_STATUSES]), } @@ -418,23 +427,26 @@ async def set_stems_location(request: Request) -> dict[str, object]: if not isinstance(raw, str): raise HTTPException(status_code=422, detail="path is required") - active = [j for j in registry_all_jobs().values() if j.status in _ACTIVE_JOB_STATUSES] - if active: - # Moving files out from under a running separation would corrupt it. - raise HTTPException( - status_code=409, - detail="Finish or cancel the imports in the queue before moving the stems folder", - ) - + # Close the door first. Validation touches the disk (it creates the target + # and lists it), and an import accepted during that would have its directory + # moved out from under it moments later. Every early exit below reopens it. + begin_relocation() try: + active = [j for j in registry_all_jobs().values() if j.status in _ACTIVE_JOB_STATUSES] + if active: + # Moving files out from under a running separation would corrupt it. + raise HTTPException( + status_code=409, + detail="Finish or cancel the imports in the queue before moving the stems folder", + ) target = validate_target(Path(raw), JOBS_DIR) except StemsLocationError as e: + abandon_relocation() raise HTTPException(status_code=422, detail=str(e)) from None + except Exception: + abandon_relocation() + raise - # Stop accepting imports before a single file moves. An import arriving - # mid-move, or after it while this process still points at the old folder, - # would write stems somewhere the next start does not look. - begin_relocation() try: result = await asyncio.to_thread(move_library, JOBS_DIR, target) except StemsLocationError as e: diff --git a/static/js/catalog.js b/static/js/catalog.js index 37c57b21..d0f1e960 100644 --- a/static/js/catalog.js +++ b/static/js/catalog.js @@ -2541,6 +2541,8 @@ async function wireStemsLocation(overlay) { pathEl.textContent = shortenPath(d.path); pathEl.title = d.path; if (sizeEl) sizeEl.textContent = formatSize(d.bytes); + // Reopening Settings after a move, before the restart, should still say so. + if (d.restart_required) setMessage("Restart StemDeck to finish switching over.", "ok"); }; // The backend decides whether this setting exists at all -- it is false on a @@ -2599,20 +2601,21 @@ async function wireStemsLocation(overlay) { setMessage(data.detail || "Could not move the stems folder.", "error"); return; } - apply({ path: data.path, bytes: 0 }); + // Re-read rather than trust the POST: the GET reports where the stems + // actually are now, including the size at the new location. + try { + const again = await fetch("/api/settings/stems-location", { cache: "no-store" }); + if (again.ok) apply(await again.json()); + else apply({ path: data.path, bytes: 0 }); + } catch (e) { + console.warn("[settings] refresh failed:", e); + apply({ path: data.path, bytes: 0 }); + } setMessage( `Moved ${data.moved_entries} item${data.moved_entries === 1 ? "" : "s"}. ` + "Restart StemDeck to finish switching over.", "ok", ); - // The size we had is meaningless now; re-read it from the new location. - try { - const again = await fetch("/api/settings/stems-location", { cache: "no-store" }); - if (again.ok) { - const d = await again.json(); - if (sizeEl) sizeEl.textContent = formatSize(d.bytes); - } - } catch (e) { console.warn("[settings] size refresh failed:", e); } } catch (e) { console.warn("[settings] move failed:", e); setMessage("Could not reach the server.", "error"); diff --git a/tests/test_stems_location.py b/tests/test_stems_location.py index 477c7759..a1b82112 100644 --- a/tests/test_stems_location.py +++ b/tests/test_stems_location.py @@ -455,3 +455,39 @@ def test_a_failed_move_leaves_the_app_usable(client, tmp_path): assert stems_location.is_relocating() is False assert client.post("/api/jobs", json={"url": "https://youtu.be/dQw4w9WgXcQ"}).status_code == 200 + + +def test_the_panel_shows_the_new_folder_before_the_restart(client, tmp_path): + """After a move the stored choice and this process disagree until the + restart. Reporting JOBS_DIR would show the folder the user just moved away + from, which reads as the move having failed.""" + _job_dir(client.jobs_dir, "aaaaaaaaaaaa", size=500) + target = tmp_path / "elsewhere" + client.post("/api/settings/stems-location", json={"path": str(target)}) + + body = client.get("/api/settings/stems-location").json() + + assert body["path"] == str(target.resolve()) + assert body["restart_required"] is True + assert body["bytes"] == 500, "size should come from where the files now are" + assert body["is_default"] is False + + +def test_no_restart_is_advertised_when_nothing_moved(client): + body = client.get("/api/settings/stems-location").json() + assert body["restart_required"] is False + + +def test_a_refused_move_reopens_the_door(client, tmp_path): + """A busy queue stops the move, and the app has to stay usable afterwards -- + the guard is closed before the busy check, so it must be reopened.""" + from app.core import stems_location + + job = Job(id="aaaaaaaaaaaa") + job.status = "separating" + _jobs[job.id] = job + + r = client.post("/api/settings/stems-location", json={"path": str(tmp_path / "elsewhere")}) + + assert r.status_code == 409 + assert stems_location.is_relocating() is False, "imports would be blocked forever"