From 69c8ad4e0c7e9789d382206f5b725ffb024efd45 Mon Sep 17 00:00:00 2001 From: Byron Gamatos Date: Mon, 6 Jul 2026 20:35:51 +0200 Subject: [PATCH] fix(settings): don't let a stale DLC path block saving the Demucs server address (#795) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v3 Settings "Save" button posts dlc_dir together with demucs_server_url, default_arrangement and av_offset_ms in one request. POST /api/settings validated dlc_dir first and early-returned "DLC directory not found" before it ever processed demucs_server_url, so on a machine whose DLC path doesn't resolve (fresh install, unplugged/network drive, a path carried over from another machine) setting the Demucs server address silently failed — reported in got-feedBack/feedBack-demucs-server#3 (macOS 07-05 nightly). - server: a non-resolving dlc_dir is now recorded as a warning and skipped rather than aborting the whole POST, so the co-submitted keys still persist. The bad path is surfaced via a new additive `warnings` field and folded into `message` so the settings status line still shows it. - client (v3): the Demucs input now autosaves on blur/enter via a single-key persistSetting POST, like every other v3 setting, so it never depends on the coupled Save button. - tests: cover the decoupling and the unchanged happy path. Co-authored-by: Claude Opus 4.8 (1M context) --- server.py | 24 ++++++++++++++++++++-- static/v3/index.html | 5 +++++ tests/test_settings_api.py | 41 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/server.py b/server.py index ecc38da..e183f08 100644 --- a/server.py +++ b/server.py @@ -10765,6 +10765,9 @@ def save_settings(data: dict): config_file = CONFIG_DIR / "config.json" updates: dict = {} messages: list[str] = [] + # Named dlc_warnings (not `warnings`) so it can't shadow the module-level + # `import warnings` used elsewhere in this file. + dlc_warnings: list[str] = [] if "dlc_dir" in data: dlc_path = data["dlc_dir"] @@ -10784,7 +10787,16 @@ def save_settings(data: dict): if f.suffix.lower() in sloppak_mod.SONG_EXTS) messages.append(f"DLC folder: {count} song files found") else: - return {"error": f"DLC directory not found: {dlc_path}"} + # A non-resolving DLC path (a stale value, an unplugged + # external/network drive, or a path carried over from another + # machine) must NOT abort the whole POST. saveSettings() bundles + # dlc_dir together with demucs_server_url / default_arrangement / + # av_offset_ms in a single request, so an early `return` here + # silently dropped every co-submitted key — this is the "can't + # set the Demucs server address" report (feedBack-demucs-server + # #3). Record it as a warning, skip persisting dlc_dir, and keep + # validating the rest so the other settings still save. + dlc_warnings.append(f"DLC directory not found: {dlc_path}") # Both of these are consumed downstream as strings (e.g. # demucs_server_url.rstrip('/')), so reject non-string shapes @@ -11041,7 +11053,15 @@ def save_settings(data: dict): return {"error": str(exc)} cfg = settings_with_instrument_profiles(cfg) _atomic_write_file(config_file, json.dumps(cfg, indent=2).encode("utf-8")) - return {"message": ". ".join(messages) if messages else "Settings saved"} + resp = {"message": ". ".join(messages) if messages else "Settings saved"} + if dlc_warnings: + # `warnings` is an additive response field (existing clients read + # `message || error`); fold the text into `message` too so the current + # settings status line still surfaces the bad DLC path even though the + # rest of the save succeeded. + resp["warnings"] = dlc_warnings + resp["message"] = resp["message"] + " — " + "; ".join(dlc_warnings) + return resp # Keys a client "Reset {category}" action may clear. Resetting removes the key diff --git a/static/v3/index.html b/static/v3/index.html index 2ac7e0e..0b699a5 100644 --- a/static/v3/index.html +++ b/static/v3/index.html @@ -641,7 +641,12 @@
+
diff --git a/tests/test_settings_api.py b/tests/test_settings_api.py index aae4a11..c97638e 100644 --- a/tests/test_settings_api.py +++ b/tests/test_settings_api.py @@ -219,6 +219,47 @@ def test_dlc_dir_empty_string_clears(client, tmp_path): assert _read_cfg(tmp_path)["dlc_dir"] == "" +def test_unresolvable_dlc_dir_does_not_block_other_keys(client, tmp_path): + # Regression (feedBack-demucs-server#3): the v3 "Save" button next to the + # Demucs field bundles dlc_dir with demucs_server_url in one POST. A DLC + # path that doesn't resolve on THIS machine (stale value / unplugged drive) + # must not abort the whole request — the co-submitted demucs_server_url has + # to persist, and the bad path is surfaced as a warning rather than a hard + # error that drops every other key. + missing = str(tmp_path / "does-not-exist") + r = client.post("/api/settings", json={ + "dlc_dir": missing, + "demucs_server_url": "http://demucs.example:7865", + "default_arrangement": "Lead", + }) + assert r.status_code == 200 + body = r.json() + # No hard error; the bad path is reported as a warning. + assert "error" not in body + assert any("does-not-exist" in w for w in body.get("warnings", [])) + assert "does-not-exist" in body["message"] + cfg = _read_cfg(tmp_path) + # The valid keys persisted... + assert cfg["demucs_server_url"] == "http://demucs.example:7865" + assert cfg["default_arrangement"] == "Lead" + # ...and the unresolvable path was NOT written. + assert cfg.get("dlc_dir", "") != missing + + +def test_valid_dlc_dir_still_reports_song_count(client, tmp_path): + # The happy path is unchanged: a resolvable DLC dir persists and the + # response message still carries the "N song files found" summary (no + # warnings key when nothing went wrong). + dlc = tmp_path / "dlc" + dlc.mkdir() + r = client.post("/api/settings", json={"dlc_dir": str(dlc)}) + assert r.status_code == 200 + body = r.json() + assert "warnings" not in body + assert "song files found" in body["message"] + assert _read_cfg(tmp_path)["dlc_dir"] == str(dlc) + + @pytest.mark.parametrize("key", ["default_arrangement", "demucs_server_url"]) def test_string_key_null_is_noop(client, tmp_path, key): # Match the dlc_dir contract: null preserves the on-disk value.