From 15d5a3e97a8f0494c808033b32a839f7314454f5 Mon Sep 17 00:00:00 2001 From: Thales <> Date: Mon, 24 Aug 2026 09:36:58 +0100 Subject: [PATCH 1/2] fix: a new install no longer resets settings the user changed Reproduced against the published 0.14.0 package: relocate the stems folder in Settings, extract the same release into a fresh folder, and the new install reports the default folder again. The library looks empty even though every stem is still on disk where the user put it. Cause: portable packages keep their data in /data (#399), so settings.json lives INSIDE the install directory. A new install is a new folder, so it starts with an empty data/ and loses everything the user set -- not just the stems location but the port, compute device, separation quality and language too. The restore half of the fix was already in the working tree: migrate_persisted_files plus an ensure_workspace step that seeds a freshly extracted portable package from a per-user copy. What was missing was anything writing that copy. Nothing had, since #399 moved the data directory out of %LOCALAPPDATA% and no writer took over the old location, so the restore read a path that never existed. This adds the write half: - shared_settings_dir() names the per-user location once, for all three platforms. Deliberately the OS-standard data dir -- exactly what local_data_dir() returns for a NON-portable install -- so both layouts share one location and an install that switches between them keeps its settings. - start_backend passes it as STEMDECK_SETTINGS_MIRROR, so the writer and ensure_workspace's reader cannot drift apart. - settings._save() mirrors there after each successful write. Best-effort by construction: it is a redundant copy and must never fail the setting the user just changed. Written via a temp file and replace, because a torn write would be restored verbatim into the next install. Verified end to end against the source backend: relocate the folder in install A, seed install B the way ensure_workspace does, and B reports the user's folder rather than the default. Five tests cover the write half, including that a mirror which cannot be written still saves the setting. Also carries the clippy cleanups already sitting in the tree (derivable Default, match to if let, redundant trim, unneeded return, unused binding) -- audit item F12. Both platforms compile with no warnings; 45 Rust tests on Windows, 46 on Linux, and the Python suite has no new failures against main. --- app/core/settings.py | 37 ++++- desktop/src-tauri/src/main.rs | 254 +++++++++++++++++++++++++++++----- tests/test_settings_mirror.py | 80 +++++++++++ 3 files changed, 333 insertions(+), 38 deletions(-) create mode 100644 tests/test_settings_mirror.py diff --git a/app/core/settings.py b/app/core/settings.py index 0b6c3aa..fe9af47 100644 --- a/app/core/settings.py +++ b/app/core/settings.py @@ -94,10 +94,45 @@ def _save() -> bool: try: _SETTINGS_PATH.parent.mkdir(parents=True, exist_ok=True) _SETTINGS_PATH.write_text(json.dumps(_ensure()), encoding="utf-8") - return True except Exception: _log.warning("could not persist settings to %s", _SETTINGS_PATH, exc_info=True) return False + _mirror_settings() + return True + + +def _mirror_settings() -> None: + """Keep a per-user copy of settings.json outside the install directory. + + A Windows portable package keeps its data in `/data` (#399), so + settings.json lives *inside the install*. Upgrading by extracting the new + zip to a fresh folder therefore started that install with no settings at + all: the stems location, port, compute device, quality and language were + all silently back to defaults, and a relocated library looked empty. + + The desktop shell already restores from this copy -- `ensure_workspace` + seeds a fresh portable install from it before the backend ever reads + settings.json. Only the write half was missing, because #399 moved the + data directory and nothing took over writing the old location. + + Best-effort by definition: this is a redundant copy, and failing to write + it must never fail the setting the user just changed. The path comes from + the shell (STEMDECK_SETTINGS_MIRROR) so the platform logic stays in one + place and both halves cannot drift apart. + """ + target = os.environ.get("STEMDECK_SETTINGS_MIRROR", "").strip() + if not target: + return + try: + path = Path(target) + path.parent.mkdir(parents=True, exist_ok=True) + # Same-directory temp + replace: a torn write here would be restored + # verbatim into the user's next install. + tmp = path.with_suffix(".json.tmp") + tmp.write_text(json.dumps(_ensure()), encoding="utf-8") + tmp.replace(path) + except Exception: + _log.warning("could not mirror settings to %s", target, exc_info=True) def _num(v: object) -> int | None: diff --git a/desktop/src-tauri/src/main.rs b/desktop/src-tauri/src/main.rs index 3b7d36a..2760638 100644 --- a/desktop/src-tauri/src/main.rs +++ b/desktop/src-tauri/src/main.rs @@ -113,6 +113,7 @@ struct BackendHandles { url: String, } +#[derive(Default)] struct BackendStateInner { handles: Option, /// True while start_backend is executing; prevents concurrent starts (#145). @@ -139,18 +140,6 @@ struct BackendStateInner { /// process. const MAX_PENDING_SAVES: usize = 16; -impl Default for BackendStateInner { - fn default() -> Self { - BackendStateInner { - handles: None, - starting: false, - setup_child_pid: None, - pending_saves: HashMap::new(), - next_save_token: 0, - } - } -} - struct BackendState { inner: Mutex, } @@ -423,16 +412,16 @@ fn main() { ]) .build(tauri::generate_context!()) .expect("failed to build StemDeck desktop app") - .run(|app_handle, event| match event { - tauri::RunEvent::WindowEvent { + .run(|app_handle, event| { + if let tauri::RunEvent::WindowEvent { event: tauri::WindowEvent::CloseRequested { .. }, .. - } => { + } = event + { let state = app_handle.state::(); stop_backend(&state); app_handle.exit(0); } - _ => {} }); } @@ -1284,6 +1273,48 @@ fn apply_app_update( } } +/// The per-user directory holding the settings copy that survives reinstalling. +/// +/// A portable package keeps its data in `/data` (#399), which means +/// settings.json lives *inside the install*. Upgrading by extracting the new +/// zip to a fresh folder therefore lost every setting the user had changed: +/// stems location, port, compute device, quality, language. `ensure_workspace` +/// already restores from here, but nothing wrote it after #399 moved the data +/// directory — the backend mirrors to it now, via STEMDECK_SETTINGS_MIRROR. +/// +/// Deliberately the OS-standard data dir, i.e. exactly what `local_data_dir` +/// returns for a NON-portable install, so the two layouts share one location +/// and an install that switches between them keeps its settings either way. +fn shared_settings_dir() -> Option { + #[cfg(windows)] + { + env::var("LOCALAPPDATA") + .ok() + .map(|base| PathBuf::from(base).join("StemDeck")) + } + #[cfg(target_os = "macos")] + { + env::var("HOME").ok().map(|home| { + PathBuf::from(home) + .join("Library") + .join("Application Support") + .join("StemDeck") + }) + } + #[cfg(all(unix, not(target_os = "macos")))] + { + if let Ok(xdg) = env::var("XDG_DATA_HOME") { + return Some(PathBuf::from(xdg).join("stemdeck")); + } + env::var("HOME").ok().map(|home| { + PathBuf::from(home) + .join(".local") + .join("share") + .join("stemdeck") + }) + } +} + /// Creates required data directories and runs any pending data migrations. #[tauri::command] fn ensure_workspace() -> Result<(), String> { @@ -1301,7 +1332,16 @@ fn ensure_workspace() -> Result<(), String> { } } - migrate_legacy_data(&root, &data); + #[cfg(windows)] + if is_portable_package(&root) { + // Portable data moved from %LocalAppData% to /data in #399. A + // freshly extracted package has an empty data directory, so carry the + // user's prior choices forward before the backend reads settings.json. + if let Some(shared) = shared_settings_dir() { + migrate_persisted_files(&shared, &data, &["settings.json"])?; + } + } + migrate_legacy_data(&root, &data)?; fs::create_dir_all(&data).map_err(|e| format!("failed to create data dir: {e}"))?; for dir in ["cache", "downloads", "ffmpeg", "jobs", "logs", "models"] { fs::create_dir_all(data.join(dir)) @@ -1498,6 +1538,14 @@ fn start_backend( .env("STEMDECK_DATA_DIR", &data_dir) .env("STEMDECK_DEFAULT_JOBS_DIR", &jobs_dir) .env("STEMDECK_DESKTOP", "1") + // Where the backend keeps the per-user copy of settings.json that + // survives extracting a new package into a fresh folder. Computed + // here so the write half and ensure_workspace's restore half can + // never point at different places. + .envs( + shared_settings_dir() + .map(|dir| ("STEMDECK_SETTINGS_MIRROR", dir.join("settings.json"))), + ) .env("STEMDECK_PARENT_PID", std::process::id().to_string()) .env("PYTHONUNBUFFERED", "1") .env("XDG_CACHE_HOME", data_dir.join("cache")) @@ -1917,12 +1965,7 @@ fn parse_cuda_version(smi_output: &str) -> Option { for line in smi_output.lines() { if let Some(pos) = line.find("CUDA Version:") { let rest = &line[pos + "CUDA Version:".len()..]; - let v = rest - .trim() - .split_whitespace() - .next()? - .trim_matches('|') - .trim(); + let v = rest.split_whitespace().next()?.trim_matches('|').trim(); if !v.is_empty() && v != "N/A" { return Some(v.to_string()); } @@ -2567,7 +2610,7 @@ async fn save_audio_file( } fn stop_backend(state: &BackendState) { - let (handles, setup_child_pid) = match state.inner.lock() { + let (handles, _setup_child_pid) = match state.inner.lock() { Ok(mut guard) => (guard.handles.take(), guard.setup_child_pid.take()), Err(_) => return, }; @@ -2575,7 +2618,7 @@ fn stop_backend(state: &BackendState) { // Kill any in-progress setup-time subprocess (pip install, model warmup) // so it doesn't corrupt the venv/cache if the window is closed mid-setup (#140). #[cfg(unix)] - if let Some(pid) = setup_child_pid { + if let Some(pid) = _setup_child_pid { // SAFETY: pid was stored immediately after spawn; we send SIGTERM best-effort. unsafe { libc::kill(pid as libc::pid_t, libc::SIGTERM) }; } @@ -2671,30 +2714,86 @@ fn append_to_setup_log(data_dir: &Path, msg: &str) { /// One-time migration: move legacy data/models/jobs/ffmpeg from the install /// directory into the new per-user data directory on the user's first launch -/// after upgrading to a version that uses local_data_dir(). -fn migrate_legacy_data(root: &Path, data_dir: &Path) { +/// after upgrading to a version that uses local_data_dir(). User-owned settings +/// are copied as well so reinstalling cannot silently restore defaults. +fn migrate_legacy_data(root: &Path, data_dir: &Path) -> Result<(), String> { let old = root.join("data"); - // Only migrate if the old location exists and the new one doesn't yet. - if !old.is_dir() || data_dir.exists() { - return; + if !old.is_dir() || old == data_dir { + return Ok(()); } + let _ = fs::create_dir_all(data_dir); for name in ["models", "jobs", "ffmpeg", "logs", "cache"] { let src = old.join(name); - if src.is_dir() { + let destination = data_dir.join(name); + if src.is_dir() && !destination.exists() { // rename is a cheap move on the same volume; ignore errors silently // so a cross-volume failure doesn't block startup. - let _ = fs::rename(&src, data_dir.join(name)); + let _ = fs::rename(&src, destination); } } // NOTE: deliberately does NOT migrate `cpu-only` -- the marker is only // trusted in the app root (see is_cpu_only_package); carrying it into the // shared data dir poisoned later NVIDIA installs (#247). - { - let src = old.join("config.json"); - if src.exists() { - let _ = fs::copy(&src, data_dir.join("config.json")); + migrate_persisted_files(&old, data_dir, &["config.json", "settings.json"]) +} + +/// Copy persisted choices from an older data directory without ever replacing +/// state already written at the destination. +fn migrate_persisted_files( + source_dir: &Path, + data_dir: &Path, + names: &[&str], +) -> Result<(), String> { + if source_dir == data_dir || !source_dir.is_dir() { + return Ok(()); + } + fs::create_dir_all(data_dir) + .map_err(|e| format!("failed to prepare settings migration: {e}"))?; + for name in names { + let source = source_dir.join(name); + let destination = data_dir.join(name); + if source.is_file() && !destination.exists() { + let nonce = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|duration| duration.as_nanos()) + .unwrap_or(0); + let temporary = data_dir.join(format!( + ".{name}.migrate.{}.{nonce}.tmp", + std::process::id() + )); + let result = (|| -> Result<(), String> { + let mut input = fs::File::open(&source) + .map_err(|e| format!("failed to read existing {name}: {e}"))?; + let mut output = fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&temporary) + .map_err(|e| format!("failed to stage existing {name}: {e}"))?; + std::io::copy(&mut input, &mut output) + .map_err(|e| format!("failed to copy existing {name}: {e}"))?; + output + .flush() + .map_err(|e| format!("failed to flush existing {name}: {e}"))?; + output + .sync_all() + .map_err(|e| format!("failed to sync existing {name}: {e}"))?; + drop(output); + + // Another process may have completed migration while this copy + // was staged. Its destination wins; never replace it. + if destination.exists() { + return Ok(()); + } + fs::rename(&temporary, &destination) + .map_err(|e| format!("failed to preserve existing {name}: {e}")) + })(); + if temporary.exists() { + let _ = fs::remove_file(&temporary); + } + result?; } } + Ok(()) } fn runtime_dir(data_dir: &Path) -> PathBuf { @@ -3355,7 +3454,7 @@ fn ensure_ffmpeg(data_dir: &Path) -> Result { let portable = ffmpeg_path(data_dir).ok_or_else(|| "failed to resolve FFmpeg path".to_string())?; verify_ffmpeg(&portable)?; - return Ok(portable); + Ok(portable) } #[cfg(target_os = "macos")] @@ -4090,6 +4189,87 @@ mod tests { tempfile::tempdir().expect("failed to create temp dir") } + #[test] + fn legacy_migration_preserves_user_settings_when_data_dir_already_exists() { + // setup() creates the destination before ensure_workspace() invokes + // migration, which used to make migration return without copying any + // user state at all. + let root = make_tmp(); + let destination_parent = make_tmp(); + let destination = destination_parent.path().join("StemDeck"); + fs::create_dir_all(&destination).unwrap(); + fs::create_dir_all(root.path().join("data")).unwrap(); + let settings = br#"{"jobs_dir":"D:\\Audio\\StemDeck","separation_quality":"best"}"#; + fs::write(root.path().join("data/settings.json"), settings).unwrap(); + fs::write( + root.path().join("data/config.json"), + br#"{"torchDevice":"cuda"}"#, + ) + .unwrap(); + + super::migrate_legacy_data(root.path(), &destination).unwrap(); + + assert_eq!( + fs::read(destination.join("settings.json")).unwrap(), + settings + ); + assert!(destination.join("config.json").is_file()); + assert!(root.path().join("data/settings.json").is_file()); + assert!( + fs::read_dir(&destination).unwrap().all(|entry| !entry + .unwrap() + .file_name() + .to_string_lossy() + .contains(".migrate.")), + "successful migration must not leave staging files" + ); + } + + #[test] + fn legacy_migration_never_overwrites_newer_user_settings() { + let root = make_tmp(); + let destination_parent = make_tmp(); + let destination = destination_parent.path().join("StemDeck"); + fs::create_dir_all(root.path().join("data")).unwrap(); + fs::create_dir_all(&destination).unwrap(); + fs::write( + root.path().join("data/settings.json"), + br#"{"jobs_dir":"D:\\Old"}"#, + ) + .unwrap(); + let current = br#"{"jobs_dir":"E:\\Current"}"#; + fs::write(destination.join("settings.json"), current).unwrap(); + + super::migrate_legacy_data(root.path(), &destination).unwrap(); + + assert_eq!( + fs::read(destination.join("settings.json")).unwrap(), + current + ); + } + + #[test] + fn portable_migration_carries_preferences_but_not_install_readiness() { + let source = make_tmp(); + let destination_parent = make_tmp(); + let destination = destination_parent.path().join("data"); + fs::write( + source.path().join("settings.json"), + br#"{"jobs_dir":"D:\\Audio"}"#, + ) + .unwrap(); + fs::write( + source.path().join("config.json"), + br#"{"modelReady":true,"ffmpegReady":true}"#, + ) + .unwrap(); + + super::migrate_persisted_files(source.path(), &destination, &["settings.json"]).unwrap(); + + assert!(destination.join("settings.json").is_file()); + assert!(!destination.join("config.json").exists()); + } + // ── stale app-data cleanup (#356) ──────────────────────────────────────── fn seed_downloads(dir: &std::path::Path, names: &[(&str, usize)]) { diff --git a/tests/test_settings_mirror.py b/tests/test_settings_mirror.py new file mode 100644 index 0000000..dee7d52 --- /dev/null +++ b/tests/test_settings_mirror.py @@ -0,0 +1,80 @@ +"""The per-user settings copy that survives extracting a new package (#421). + +A Windows portable package keeps its data in `/data`, so settings.json +lives inside the install directory. Upgrading by extracting the new zip to a +fresh folder started that install with no settings at all -- stems location, +port, compute device, quality and language silently back to defaults, and a +relocated library looking empty. + +The desktop shell already restored from a per-user copy in `ensure_workspace`; +nothing had written it since the data directory moved. These cover the write +half. +""" + +from __future__ import annotations + +import json + +from app.core import settings as _settings + + +def test_settings_are_mirrored_when_the_shell_asks_for_it(tmp_path, monkeypatch): + mirror = tmp_path / "shared" / "settings.json" + monkeypatch.setenv("STEMDECK_SETTINGS_MIRROR", str(mirror)) + + _settings.set_max_duration_sec(11 * 60) + + assert mirror.is_file(), "a fresh install has nothing to restore from without this" + assert json.loads(mirror.read_text(encoding="utf-8"))["max_duration_sec"] == 11 * 60 + + +def test_the_mirror_tracks_later_changes(tmp_path, monkeypatch): + mirror = tmp_path / "shared" / "settings.json" + monkeypatch.setenv("STEMDECK_SETTINGS_MIRROR", str(mirror)) + + _settings.set_max_duration_sec(5 * 60) + _settings.set_max_duration_sec(9 * 60) + + # Restoring a stale copy would quietly hand back a setting the user had + # already changed, which is the same class of bug as losing it. + assert json.loads(mirror.read_text(encoding="utf-8"))["max_duration_sec"] == 9 * 60 + + +def test_no_mirror_is_written_when_the_shell_does_not_ask(tmp_path, monkeypatch): + # Docker and a source checkout keep their data outside the install already, + # so there is nothing to preserve and nothing should be created. + monkeypatch.delenv("STEMDECK_SETTINGS_MIRROR", raising=False) + stray = tmp_path / "shared" + + _settings.set_max_duration_sec(7 * 60) + + assert not stray.exists() + + +def test_a_failing_mirror_never_fails_the_setting(tmp_path, monkeypatch): + # The mirror is a redundant copy. If it cannot be written -- read-only disk, + # a path the user cannot reach -- the setting the user just changed must + # still be saved and reported as saved. + blocker = tmp_path / "not-a-dir" + blocker.write_text("", encoding="utf-8") + monkeypatch.setenv("STEMDECK_SETTINGS_MIRROR", str(blocker / "nested" / "settings.json")) + + assert _settings.set_max_duration_sec(6 * 60) is not None + assert _settings.get_max_duration_sec() == 6 * 60 + assert ( + json.loads(_settings._SETTINGS_PATH.read_text(encoding="utf-8"))["max_duration_sec"] + == 6 * 60 + ) + + +def test_mirror_holds_the_relocated_stems_folder(tmp_path, monkeypatch): + # The reported symptom: a new install went back to the default jobs folder + # because jobs_dir only existed inside the old install directory. + mirror = tmp_path / "shared" / "settings.json" + monkeypatch.setenv("STEMDECK_SETTINGS_MIRROR", str(mirror)) + chosen = tmp_path / "MyStems" + chosen.mkdir() + + _settings.set_jobs_dir(str(chosen)) + + assert json.loads(mirror.read_text(encoding="utf-8"))["jobs_dir"] == str(chosen) From b30057416cd7327ccfffee78914aa2660a9cf0a9 Mon Sep 17 00:00:00 2001 From: Thales <> Date: Mon, 24 Aug 2026 11:34:24 +0100 Subject: [PATCH 2/2] fix: preserve settings configured before the mirror existed Mirroring only on save left out the people most likely to be bitten: someone who relocated their stems folder in an earlier release and never opens Settings again never triggers a save, so no per-user copy is ever written and their next fresh extract still starts from defaults. Seed the copy on first load instead, from settings that are already on disk. Guarded so an empty state (a genuine first run) can never overwrite a good copy, which would destroy the very thing being preserved. Also pins the Unraid template at 0.14.1. --- app/core/settings.py | 9 +++++++++ templates/stemdeck.xml | 2 +- tests/test_settings_mirror.py | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/app/core/settings.py b/app/core/settings.py index fe9af47..18da525 100644 --- a/app/core/settings.py +++ b/app/core/settings.py @@ -81,6 +81,15 @@ def _ensure() -> dict: global _state if _state is None: _state = _load() + # Seed the per-user copy from settings that already exist. Mirroring + # only on _save() would protect nobody who configured StemDeck before + # this shipped and never opens Settings again -- their next install + # would still start empty. Safe against recursion (_state is assigned + # first) and against clobbering: an empty dict means a genuine first + # run, and overwriting a good copy with it is exactly the data loss + # this whole mechanism exists to prevent. + if _state: + _mirror_settings() return _state diff --git a/templates/stemdeck.xml b/templates/stemdeck.xml index d63c6b5..dff9c39 100644 --- a/templates/stemdeck.xml +++ b/templates/stemdeck.xml @@ -1,7 +1,7 @@ StemDeck - ghcr.io/stemdeckapp/stemdeck:0.14.0 + ghcr.io/stemdeckapp/stemdeck:0.14.1 https://github.com/stemdeckapp/stemdeck/pkgs/container/stemdeck bridge sh diff --git a/tests/test_settings_mirror.py b/tests/test_settings_mirror.py index dee7d52..55f0b82 100644 --- a/tests/test_settings_mirror.py +++ b/tests/test_settings_mirror.py @@ -67,6 +67,39 @@ def test_a_failing_mirror_never_fails_the_setting(tmp_path, monkeypatch): ) +def test_existing_settings_are_mirrored_without_waiting_for_a_change(tmp_path, monkeypatch): + # Someone who relocated their stems folder in an earlier release and never + # opens Settings again would never trigger a save, so mirroring on save + # alone would leave them exposed on their next install. + _settings._SETTINGS_PATH.write_text( + json.dumps({"jobs_dir": str(tmp_path / "MyStems")}), encoding="utf-8" + ) + _settings._state = None # a fresh process, reading what is already on disk + mirror = tmp_path / "shared" / "settings.json" + monkeypatch.setenv("STEMDECK_SETTINGS_MIRROR", str(mirror)) + + _settings.get_jobs_dir() + + assert mirror.is_file() + assert json.loads(mirror.read_text(encoding="utf-8"))["jobs_dir"] == str(tmp_path / "MyStems") + + +def test_a_first_run_never_clobbers_an_existing_mirror(tmp_path, monkeypatch): + # The restore is what seeds a new install, and it happens before the backend + # starts. If a run with no settings of its own overwrote the copy with its + # defaults, it would destroy the very thing being preserved. + mirror = tmp_path / "shared" / "settings.json" + mirror.parent.mkdir(parents=True) + mirror.write_text(json.dumps({"jobs_dir": str(tmp_path / "MyStems")}), encoding="utf-8") + monkeypatch.setenv("STEMDECK_SETTINGS_MIRROR", str(mirror)) + _settings._state = None + assert not _settings._SETTINGS_PATH.exists() + + _settings.get_jobs_dir() + + assert json.loads(mirror.read_text(encoding="utf-8"))["jobs_dir"] == str(tmp_path / "MyStems") + + def test_mirror_holds_the_relocated_stems_folder(tmp_path, monkeypatch): # The reported symptom: a new install went back to the default jobs folder # because jobs_dir only existed inside the old install directory.