diff --git a/CHANGELOG.md b/CHANGELOG.md index 73bacd5..132cab7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,38 @@ # Changelog +## 0.3.3 + +### Update server + +- **New: "Check for update" in the managed-server card.** The server's `server.py` is + downloaded at *install* time and never touched again — so a bug fixed upstream could not + reach anyone who had already installed. The only route was to uninstall and re-download + several GB of wheels for a one-line change, which nobody does, so in practice the fix + simply never landed. + + It re-fetches the source only (a few hundred KB), re-applies the launcher and driver + bootstrap, and restarts the server if it was running. Dependencies and the model cache are + untouched, so nothing is re-downloaded. + + Checking hits GitHub, so it happens only on a click — the status poll stays offline. + The update runs only if there is genuinely something newer, so the button never restarts a + healthy server for nothing. + +### Fixes + +- **The status chips said "downloading" when nothing was being downloaded.** The server marks + a model as `downloading` while it *warms up* — even when it is only loading a cached file + from disk into VRAM. So a few-second RAM load was displayed as a download, and users + reasonably concluded their weights had been thrown away and re-fetched. (Reported exactly + that way, and the user was right to believe the UI.) + + `server_status()` now reports which weights are on disk, per model, and a warm-up of a file + we can already see reads **loading** (blue) rather than **downloading** (amber). A genuine + fetch still says downloading. + + Done plugin-side on purpose: a server-side fix would have to reach people through a source + refresh, and the plugin already knows what is on disk. + ## 0.3.2 ### Fixes — no more model re-downloads at launch diff --git a/demucs_server.py b/demucs_server.py index a718664..3335c80 100644 --- a/demucs_server.py +++ b/demucs_server.py @@ -647,6 +647,16 @@ def _emit(progress_cb: ProgressCB, line: str, pct: float, phase: str) -> None: progress_cb({"line": line, "pct": max(0.0, min(1.0, pct)), "phase": phase}) +def _norm_ref(ref: str | None) -> str: + """The one place a ref is normalized. + + check_update() trimmed and defaulted; update_server() passed the raw settings value straight + through. Same input, two behaviours — so a ref with stray whitespace could report an update + available and then fail to apply it, which reads as "the update button is broken". + """ + return (ref or DEFAULT_SOURCE_REF).strip() or DEFAULT_SOURCE_REF + + def source_meta(config_dir: Path) -> dict: """What source is actually installed (ref + commit).""" try: @@ -656,6 +666,274 @@ def source_meta(config_dir: Path) -> dict: return {} +def check_update(config_dir: Path, ref: str | None = None) -> dict: + """Is a newer server revision available? + + Hits the network (one GitHub API call), so it is NEVER called from the status poll — + only from an explicit click. `server_status()` stays offline and cheap. + """ + if not installed(config_dir): + return {"installed": False, "update_available": False} + ref = _norm_ref(ref) + meta = source_meta(config_dir) + have = str(meta.get("commit") or "") + latest = _resolve_commit(ref) + if not latest: + # `unknown` on every path that reports an install: a caller reading it to decide whether + # to offer an update to an install with no recorded commit must get the same signal here + # as everywhere else, not a missing key. + return {"installed": True, "ref": ref, "commit": have, "latest": None, + "update_available": False, "unknown": not have, + "reason": "Could not reach GitHub to check for a newer revision."} + return { + "installed": True, + "ref": ref, + "commit": have, + "latest": latest, + "update_available": bool(have) and have != latest, + # No recorded commit means the install predates commit-pinning: we cannot prove it's + # current, so offer the update rather than claiming it's fine. + "unknown": not have, + } + + +def _source_meta_file(config_dir: Path) -> Path: + return server_dir(config_dir) / "source.json" + + +def _snapshot_source(config_dir: Path) -> Path: + """Copy the installed source aside so a failed update can be undone. + + download_source() overwrites the tree in place, so without this the moment we discover the + new revision can't run here, the old working one is already gone — and the user is left with + a stopped server and a source that crash-loops. It's a few hundred KB; the insurance is free. + + **source.json goes in the snapshot too.** It records which commit is installed, and + download_source() rewrites it. Restoring the tree without it would leave the recorded commit + pointing at the revision we just REJECTED, so check_update() would report "up to date" and + stop offering the update — the old source running under a new name, and the user with no way + to reach the fix. The rollback has to be of the install, not of a directory. + + Raises if the snapshot can't be taken. An update with no way back is exactly the thing this + exists to prevent, so not being able to take it means not updating — the server is still + running and untouched at this point, so refusing costs nothing. + """ + src = src_dir(config_dir) + backup = src.with_name(src.name + ".bak") + try: + if backup.exists(): + shutil.rmtree(backup) + backup.mkdir(parents=True, exist_ok=True) + if src.is_dir(): + shutil.copytree(src, backup / "src") + meta = _source_meta_file(config_dir) + if meta.is_file(): + shutil.copy2(meta, backup / "source.json") + return backup + except OSError as e: + shutil.rmtree(backup, ignore_errors=True) + raise RuntimeError( + f"could not snapshot the installed server source ({e}), so an update could not be " + f"undone if it failed. Nothing was changed; the server is still running.") from e + + +def _restore_source(config_dir: Path, backup: Path | None) -> bool: + """Put the snapshot back — tree AND recorded commit. True if the old install is back.""" + if not backup or not backup.is_dir(): + return False + src, meta = src_dir(config_dir), _source_meta_file(config_dir) + try: + if (backup / "src").is_dir(): + if src.exists(): + shutil.rmtree(src) + shutil.copytree(backup / "src", src) + saved_meta = backup / "source.json" + if saved_meta.is_file(): + shutil.copy2(saved_meta, meta) + elif meta.exists(): + meta.unlink() # there was no source.json before; there must not be one now + return True + except OSError as e: + log.warning("stem_splitter: could not restore the previous server source: %s", e) + return False + + +def _discard_snapshot(backup: Path | None) -> None: + """Drop the snapshot. Only ever called once the install on disk is one we're happy with — + a failed restore KEEPS the backup, because at that point it is the only complete copy of a + working install the user has.""" + if backup: + shutil.rmtree(backup, ignore_errors=True) + + +def update_server(config_dir: Path, ref: str | None = None, port: int = DEFAULT_PORT, + device: str = "", model: str = DEFAULT_MODEL, + progress_cb: ProgressCB = None) -> dict: + """Re-fetch the server SOURCE at `ref` and restart it if it was running. + + Why this exists: server.py is downloaded at INSTALL time and never touched again, so a + bug fixed upstream cannot reach anyone who already installed — they would have to + uninstall and re-download several GB of wheels to pick up a one-line change. That is not + a thing anyone will do, so in practice the fix never lands. (The 24h cache sweeper that + deletes model weights daily is exactly this: fixed upstream, unreachable in the field.) + + This updates ONLY the source: a few hundred KB. It does not touch pylibs, and it does not + touch the model cache, so nothing is re-downloaded. + + But a new source revision can require a dependency the installed pylibs tree does not have. + So after refreshing, the imports are re-checked (verify_install). If they no longer hold, the + update is ROLLED BACK — old source, old recorded commit, server restarted — and we say so. + A server that cannot import its own dependencies would only crash-loop, and the user must + never end up worse off for having clicked a button meant to help them. + """ + if not installed(config_dir): + raise RuntimeError("the server isn't installed, so there is nothing to update") + + ref = _norm_ref(ref) # exactly as check_update() does it, or "check" and "apply" differ + + # BEFORE anything is touched, and before the server is stopped: no snapshot, no update. An + # in-place overwrite we cannot undo is precisely the failure the snapshot exists to prevent, + # so proceeding without one would be trading a certain small annoyance ("couldn't update") + # for a rare catastrophic one ("your server is gone and the source on disk can't run"). + # Nothing has been disturbed at this point, so refusing is free. + backup = _snapshot_source(config_dir) + + # The port it is ACTUALLY on, not just the one the settings say. Those disagree whenever the + # user edits the port without restarting — and putting the server back somewhere other than + # where we found it is precisely the failure this argument was threaded through to avoid. + was_running, live_port = is_running(config_dir) + port = _as_port(live_port, port) if was_running else port + before = str(source_meta(config_dir).get("commit") or "") # FULL sha, not a prefix + + if was_running: + _emit(progress_cb, "Stopping the server…", 0.05, "Updating") + stop_server(config_dir) + + try: + _emit(progress_cb, "Fetching the latest server source…", 0.2, "Updating") + # Scaled into [0.2, 0.75): download_source() reports its OWN 0→1, so forwarding our + # callback verbatim would slam the bar back to 2% one line after we said 20%. + download_source(config_dir, ref=ref, progress_cb=_scaled(progress_cb, 0.2, 0.55)) + + # The launcher and the driver bootstrap are generated from OUR templates, and a fresh + # source download overwrites the drivers — so both must be re-applied or the server + # comes back up unable to import its own dependencies. + write_launcher(config_dir) + patched = patch_driver_scripts(config_dir) + if patched: + _emit(progress_cb, f"Re-bootstrapped {', '.join(patched)}.", 0.8, "Updating") + except Exception as e: + # We stopped a server that was WORKING, and the update failed. Leaving it down means a + # click that was meant to fix something has instead taken the user's server away — a + # strictly worse position than before they clicked, over a network blip. + # + # Put the OLD source back first (the fetch may have left a half-extracted tree), then + # restart, then report. The launcher and drivers are regenerated from our own templates + # on every start, so the restored tree is startable. + # + # KEEP the backup if the restore fails: at that moment it is the only complete copy of a + # working install the user has, and deleting it would turn a bad day into an unrecoverable + # one. It sits next to src/ as src.bak and the next update overwrites it. + if _restore_source(config_dir, backup): + _discard_snapshot(backup) + else: + log.warning("stem_splitter: keeping %s - it is the only intact copy of the previous " + "server source", backup) + if was_running: + _emit(progress_cb, f"Update failed ({e}) — restarting the server as it was.", + 0.9, "Updating") + try: + start_server(config_dir, port=port, device=device, model=model, + warmup=models_downloaded(config_dir), + progress_cb=_scaled(progress_cb, 0.9, 0.1)) + except Exception as restart_error: + # The user's last message said we were putting their server back. If that ALSO + # failed, silence leaves them believing the old server is still up when it is + # actually down — the worst of the three possible states, and the only one they + # can't see. Every other failure branch here tells them; so does this one. + _emit(progress_cb, + f"The server could not be restarted either ({restart_error}). It is now " + f"stopped — use Start to bring it back.", + 1.0, "Failed") + log.warning("stem_splitter: update failed AND the server could not be " + "restarted: %s", restart_error) + raise + + # A newer source may need a dependency the installed tree does not have. Check before + # restarting, so a missing module surfaces as one clear sentence rather than a crash-loop. + try: + _emit(progress_cb, "Checking the updated server's dependencies…", 0.86, "Updating") + verify_install(config_dir, progress_cb=None) + except Exception as e: + # The new source can't run here. Don't leave it on disk: rolling forward would strand the + # user with a stopped server and the only source on disk being one that crash-loops. + # Put the old source back — tree AND recorded commit — and start it, so a failed update + # costs them nothing but time. + rolled_back = _restore_source(config_dir, backup) + restored = False + if rolled_back and was_running: + try: + start_server(config_dir, port=port, device=device, model=model, + warmup=models_downloaded(config_dir), progress_cb=None) + restored = True + except Exception as restart_error: + log.warning("stem_splitter: rolled back the source but could not restart: %s", + restart_error) + + if rolled_back: + msg = (f"The new server revision needs dependencies this install doesn't have ({e}). " + f"Rolled back to the previous revision" + + (" and restarted it — nothing was lost. " + if restored else " (it is stopped; use Start). ") + + "Click 'Install server + models' to update the dependencies and try again.") + else: + msg = (f"The new server revision needs dependencies this install doesn't have ({e}), " + f"and the previous source could not be restored. Click 'Install server + " + f"models' to repair the install.") + _emit(progress_cb, msg, 1.0, "Needs reinstall") + + # Keep the snapshot if the restore failed — it is then the only intact copy of a working + # install the user has. + if rolled_back: + _discard_snapshot(backup) + else: + log.warning("stem_splitter: keeping %s - it is the only intact copy of the previous " + "server source", backup) + + st = server_status(config_dir, model=model) + st["updated"] = not rolled_back # rolled back => the source is what it was + st["rolled_back"] = rolled_back + st["needs_reinstall"] = True + return st + else: + _discard_snapshot(backup) # the update stuck; the old source is no longer needed + + after = str(source_meta(config_dir).get("commit") or "") + # Compare FULL shas. Two different commits can share an 8-char prefix, and reporting + # "already up to date" for an update that actually happened is a lie the user cannot check. + changed = before != after + _emit(progress_cb, + f"Updated {before[:8] or '?'} → {after[:8] or '?'}." if changed + else f"Already up to date ({after[:8] or '?'}).", + 0.88, "Updating") # after the 0.86 dependency check — the bar must never go back + + if was_running: + _emit(progress_cb, "Restarting the server…", 0.92, "Updating") + # `port` is the live one (resolved from is_running above), falling back to the configured + # one. Restarting on DEFAULT_PORT would move a server the user had deliberately put + # elsewhere — or collide with whatever is on 7865 — leaving a working setup down or + # unreachable, after a click meant to help. Warmup only if the weights are already on + # disk: an update must never become a surprise multi-GB download. + start_server(config_dir, port=port, device=device, model=model, + warmup=models_downloaded(config_dir), + progress_cb=_scaled(progress_cb, 0.92, 0.08)) + + _emit(progress_cb, "Done.", 1.0, "Done") + st = server_status(config_dir, model=model) + st["updated"] = changed + return st + + def _resolve_commit(ref: str) -> str | None: """Resolve a ref to an immutable commit SHA. None if GitHub can't be reached (we then fall back to the branch archive rather than failing the install).""" @@ -684,7 +962,7 @@ def download_source(config_dir: Path, ref: str | None = None, sdir = src_dir(config_dir) sdir.mkdir(parents=True, exist_ok=True) - ref = (ref or DEFAULT_SOURCE_REF).strip() or DEFAULT_SOURCE_REF + ref = _norm_ref(ref) commit = _resolve_commit(ref) archive = commit or ref url = f"https://codeload.github.com/{SOURCE_REPO}/zip/{archive}" @@ -1294,7 +1572,7 @@ def setup_server(config_dir: Path, port: int = DEFAULT_PORT, device: str = "", progress_cb=_scaled(progress_cb, 0.55, 0.45)) _emit(progress_cb, "Server installed, running and warmed up.", 1.0, "Done") - return server_status(config_dir) + return server_status(config_dir, model=model) def uninstall_server(config_dir: Path) -> dict: @@ -1475,7 +1753,7 @@ def start_server(config_dir: Path, port: int = DEFAULT_PORT, device: str = "", running, live_port = is_running(config_dir, port) if running: _emit(progress_cb, f"Server already running on port {live_port}.", 1.0, "Running") - return server_status(config_dir) + return server_status(config_dir, model=model) if warmup is None: warmup = models_downloaded(config_dir) @@ -1574,7 +1852,7 @@ def _reader() -> None: ok, _payload = server_health(url, timeout=1.0) if ok: _emit(progress_cb, f"Server is up at {url}", 1.0, "Running") - return server_status(config_dir) + return server_status(config_dir, model=model) time.sleep(0.5) raise RuntimeError(f"server did not answer /health on {url} within 20s") @@ -1834,7 +2112,7 @@ def prepare_models(config_dir: Path, port: int = DEFAULT_PORT, device: str = "", raise RuntimeError(f"model warmup failed: {wu}") if _model_ready(wu, model): _emit(progress_cb, "Models ready.", 1.0, "Done") - return server_status(config_dir) + return server_status(config_dir, model=model) _emit(progress_cb, f"warmup: {wu}", 0.6, "Downloading models") time.sleep(5) @@ -1846,7 +2124,16 @@ def _model_ready(warmup: dict, model: str) -> bool: return str(v) in ("ready", "skipped") -def server_status(config_dir: Path) -> dict: +def server_status(config_dir: Path, model: str = DEFAULT_MODEL) -> dict: + """Offline, cheap, and polled every few seconds. + + `model` is the model the server was actually STARTED with. It defaults to DEFAULT_MODEL + for callers that don't know (nothing here can read the plugin's settings), but the route + passes the configured one — otherwise a user on a non-default model warms it fully and + models_ready stays false forever, because we'd be asking /health about a model the server + was never told to warm. The UI would never say "Warm · ready to split" and would keep + polling a server that is, in fact, ready. + """ st = _read_state(config_dir) port = _as_port(st.get("port"), DEFAULT_PORT) running, live_port = is_running(config_dir, port) @@ -1858,9 +2145,26 @@ def server_status(config_dir: Path) -> dict: if running: ok, health = server_health(url, timeout=2.0) if ok: - models_ready = _model_ready(health.get("warmup") or {}, DEFAULT_MODEL) + models_ready = _model_ready(health.get("warmup") or {}, model) manageable, manage_reason = can_manage(config_dir) + + # Which weights are ALREADY on disk, per model. The server reports a model as "downloading" + # while it warms up — even when it is only loading a cached file from disk into VRAM. So the + # UI showed "downloading" during a pure RAM load, the user saw a download that wasn't + # happening, and reasonably concluded their weights had been thrown away again. (Reported + # exactly that way.) With this, the UI can say "loading" when it knows the file is right there. + # + # Walk the cache ONCE. models_downloaded() is just the AND of these three, and this is the + # poll endpoint — hit every few seconds, and _has_whisper() rglob()s the whole snapshot tree. + # Calling both would do every check twice per poll. Deriving the flag from the dict also + # makes the two fields consistent by construction rather than by two walks agreeing. + cache = cache_dir(config_dir) + present = { + "bs_roformer_sw": _has_roformer(cache), + "whisperx": _has_whisper(cache), + "whisperx_aligners": _has_aligner(cache), + } return { "installed": installed(config_dir), "running": running, @@ -1868,7 +2172,8 @@ def server_status(config_dir: Path) -> dict: "port": port, "url": url if running else None, "health": health, - "models_downloaded": models_downloaded(config_dir), + "models_downloaded": all(present.values()), + "models_present": present, "models_ready": models_ready, "server_dir": str(server_dir(config_dir)), "disk_bytes": _server_disk_bytes(config_dir), diff --git a/plugin.json b/plugin.json index 32296ef..fbc5a23 100644 --- a/plugin.json +++ b/plugin.json @@ -1,7 +1,7 @@ { "id": "stem_splitter", "name": "Stem Splitter", - "version": "0.3.2", + "version": "0.3.3", "description": "Split feedpak stems and transcribe lyrics — remote server or opt-in local engine.", "bundled": true, "script": "screen.js", diff --git a/routes.py b/routes.py index 0854023..605e91c 100644 --- a/routes.py +++ b/routes.py @@ -660,11 +660,19 @@ def uninstall_engine(): return engine_install.uninstall_engine(mgr.config_dir) # ── managed demucs server ──────────────────────────────────────────────── - def _server_opts() -> tuple[int, str]: + def _server_opts() -> tuple[int, str, str]: + """The (port, device, model) EVERY server lifecycle path runs with. + + The model used to be threaded through Update only, so a user who changed the split model + got that model on a restart-after-update and DEFAULT_MODEL on a plain Start — the same + server warming a different model depending on which button they pressed. Returning the + whole triple from one place is what stops the next lifecycle path from forgetting one. + """ s = mgr.read_settings() port = _as_port(s.get("local_server_port")) device = str(s.get("local_server_device") or "") - return port, device + model = str(s.get("remote_model") or demucs_server.DEFAULT_MODEL) + return port, device, model def _want_gpu(body: dict | None = None) -> bool: """Whether to install the CUDA torch build. An explicit request wins; the @@ -689,12 +697,16 @@ def _op_in_flight() -> bool: @app.get(f"{P}/server_status") def get_server_status(): - return demucs_server.server_status(mgr.config_dir) + # The model the server was actually started with — see server_status(). Asking + # /health about DEFAULT_MODEL when the user configured another one leaves + # models_ready false on a server that is fully warm. + _p, _d, model = _server_opts() + return demucs_server.server_status(mgr.config_dir, model=model) @app.get(f"{P}/server/health") def get_server_health(): """Backend proxy for the 'Test status' button. /health needs no API key.""" - port, _ = _server_opts() + port, _, _ = _server_opts() url = mgr.local_server_url() or demucs_server.url_for(port) ok, payload = demucs_server.server_health(url, timeout=4.0) return {"ok": ok, "url": url, "health": payload} @@ -704,14 +716,14 @@ def post_server_install(body: dict | None = None): # One click = a server that actually works: dependencies AND model weights. # Installing without the weights leaves a server that can't split until a # second action, which is a confusing half-state. - port, device = _server_opts() + port, device, model = _server_opts() gpu = _want_gpu(body) st = mgr.read_settings() # Advanced overrides; blank means "use the default". ref = str((body or {}).get("ref") or st.get("local_server_ref") or "") or None cuda_tag = str((body or {}).get("cuda_tag") or st.get("local_server_cuda_tag") or "") or None if not mgr.run_server_op("install", lambda cb: demucs_server.setup_server( - mgr.config_dir, port=port, device=device, gpu=gpu, + mgr.config_dir, port=port, device=device, model=model, gpu=gpu, ref=ref, cuda_tag=cuda_tag, progress_cb=cb)): return _busy() return {"ok": True, "started": "install", "gpu": gpu, @@ -720,11 +732,12 @@ def post_server_install(body: dict | None = None): @app.post(f"{P}/server/start") def post_server_start(): - port, device = _server_opts() + port, device, model = _server_opts() # warmup=None -> warm up only if the weights are already on disk, so a # start can never trigger the big download. if not mgr.run_server_op("start", lambda cb: demucs_server.start_server( - mgr.config_dir, port=port, device=device, warmup=None, progress_cb=cb)): + mgr.config_dir, port=port, device=device, model=model, warmup=None, + progress_cb=cb)): return _busy() return {"ok": True, "started": "start"} @@ -734,6 +747,33 @@ def post_server_stop(): return _busy() return demucs_server.stop_server(mgr.config_dir) + @app.get(f"{P}/server/check_update") + def get_server_check_update(): + """Explicit only — this hits GitHub. server_status() stays offline and cheap, because + it is polled every few seconds and must never do network I/O.""" + s = mgr.read_settings() + return demucs_server.check_update(mgr.config_dir, ref=s.get("local_server_ref") or None) + + @app.post(f"{P}/server/update") + def post_server_update(): + """Re-fetch the server SOURCE (a few hundred KB) and restart it. + + server.py is downloaded at INSTALL time and never touched again, so a bug fixed + upstream cannot reach anyone who already installed — short of uninstalling and + re-downloading several GB of wheels for a one-line change, which nobody will do. In + practice the fix simply never lands. This is the path that lets it. + """ + s = mgr.read_settings() + ref = s.get("local_server_ref") or None + # The port/device the server is CONFIGURED for. Without these the restart would land on + # DEFAULT_PORT, moving a server the user deliberately put elsewhere. + port, device, model = _server_opts() + if not mgr.run_server_op("update", lambda cb: demucs_server.update_server( + mgr.config_dir, ref=ref, port=port, device=device, model=model, + progress_cb=cb)): + return _busy() + return {"ok": True} + # ── managed sibling container (Docker socket) ──────────────────────────── # # A containerized feedBack cannot start a process on its host - that is a namespace @@ -807,9 +847,9 @@ def post_sidecar_down(body: dict | None = _OPT_BODY): @app.post(f"{P}/server/prepare_models") def post_server_prepare_models(): - port, device = _server_opts() + port, device, model = _server_opts() if not mgr.run_server_op("prepare_models", lambda cb: demucs_server.prepare_models( - mgr.config_dir, port=port, device=device, progress_cb=cb)): + mgr.config_dir, port=port, device=device, model=model, progress_cb=cb)): return _busy() return {"ok": True, "started": "prepare_models"} @@ -975,14 +1015,15 @@ def _autostart() -> None: return if not demucs_server.installed(mgr.config_dir): return # nothing installed -> nothing to start - port, device = _server_opts() + port, device, model = _server_opts() running, _ = demucs_server.is_running(mgr.config_dir, port) if running: log.info("stem_splitter: demucs server already running on %s", port) return log.info("stem_splitter: auto-starting demucs server on port %s", port) mgr.run_server_op("start", lambda cb: demucs_server.start_server( - mgr.config_dir, port=port, device=device, warmup=None, progress_cb=cb)) + mgr.config_dir, port=port, device=device, model=model, warmup=None, + progress_cb=cb)) except Exception as e: log.warning("stem_splitter: demucs server auto-start failed: %s", e) diff --git a/settings.html b/settings.html index adfb952..41ec5f3 100644 --- a/settings.html +++ b/settings.html @@ -188,6 +188,7 @@

Local demucs server recommended LABEL is what clicking will do. --> + @@ -455,7 +456,9 @@

Local engine alternative — no server

return; } $('ss-srv-progress').style.display = 'block'; - setSrvBar(st.pct, st.phase || (st.active ? st.op : 'Done')); + // A snapshot is authoritative for whatever operation is live NOW (this fires on reconnect, + // possibly into a different op than the one whose floor we're carrying), so it resets. + setSrvBar(st.pct, st.phase || (st.active ? st.op : 'Done'), true); if (st.error) appendLog($('ss-srv-log'), '✗ ' + st.error); setServerButtons(!!st.active); } @@ -494,7 +497,7 @@

Local engine alternative — no server

loadDocker(); } else { appendLog($('ss-srv-log'), '✓ ' + m.op + ' done.'); - setSrvBar(1, 'Done'); + setSrvBar(1, 'Done', true); setServerButtons(false); if (m.status) renderServerStatus(m.status); else loadServerStatus(); } @@ -506,7 +509,7 @@

Local engine alternative — no server

loadDocker(); } else { appendLog($('ss-srv-log'), '✗ ' + m.error); - setSrvBar(0, 'Failed'); + setSrvBar(0, 'Failed', true); setServerButtons(false); loadServerStatus(); } @@ -598,7 +601,8 @@

Local engine alternative — no server

el.addEventListener('change', queueServerSave); if (el.tagName === 'INPUT' && el.type === 'text') el.addEventListener('input', queueServerSave); }); - var SERVER_BTNS = ['ss-srv-install', 'ss-srv-toggle', 'ss-srv-test', 'ss-srv-uninstall']; + var SERVER_BTNS = ['ss-srv-install', 'ss-srv-toggle', 'ss-srv-test', 'ss-srv-update', + 'ss-srv-uninstall']; var serverManageable = true; // false when the backend says it can't manage a server here var serverBusy = false; // an install/start/stop/prepare is in flight function setServerButtons(disabled) { @@ -606,9 +610,24 @@

Local engine alternative — no server

var d = disabled || !serverManageable; // never re-enable an unsupported section SERVER_BTNS.forEach(function (id) { var el = $(id); if (el) el.disabled = d; }); } - function setSrvBar(pct, phase) { - $('ss-srv-fill').style.width = Math.round((pct || 0) * 100) + '%'; - $('ss-srv-pct').textContent = Math.round((pct || 0) * 100) + '%'; + // The bar never runs backwards WITHIN an operation. + // + // The UI seeds a percentage for a step it is about to take (10% "Checking", 20% "Updating"), + // and then the server's own stream legitimately opens lower — update_server() emits 0.05 for + // "Stopping the server" when one is running. So the bar jumped forward and then back, which + // reads as the operation restarting. Enforcing it here rather than hand-tuning the seeds is + // the difference between fixing this and fixing it again the next time a step is inserted. + // + // `reset` starts a new operation (or reports a terminal state): that's the one time the bar + // is allowed to move down. + var _srvBarMax = 0; + function setSrvBar(pct, phase, reset) { + var p = pct || 0; + if (reset) _srvBarMax = p; + else if (p < _srvBarMax) p = _srvBarMax; + else _srvBarMax = p; + $('ss-srv-fill').style.width = Math.round(p * 100) + '%'; + $('ss-srv-pct').textContent = Math.round(p * 100) + '%'; if (phase) $('ss-srv-phase').textContent = phase; } function srvLog(line) { $('ss-srv-progress').style.display = 'block'; appendLog($('ss-srv-log'), line); } @@ -723,6 +742,25 @@

Local engine alternative — no server

var WARMUP_TONE = { ready: 'ok', skipped: 'ok', downloading: 'warn', pending: 'off', failed: 'bad', evicted: 'off' }; + // The server's warmup keys are NOT our models_present keys. It reports the default separator + // as `demucs` (the backend treats that as an alias for bs_roformer_sw — see _model_ready) and + // names the aligner differently again. A raw present[k] lookup misses, leaving the chip on + // "downloading" for a file sitting right there — which is the exact bug the loading-vs- + // downloading change exists to fix, reintroduced through a key mismatch. + var PRESENT_KEY = { + demucs: 'bs_roformer_sw', + bs_roformer_sw: 'bs_roformer_sw', + whisperx: 'whisperx', + whisperx_aligners: 'whisperx_aligners', + aligner: 'whisperx_aligners', + align: 'whisperx_aligners', + }; + + // An unmapped key falls back to the key itself rather than guessing. + function isPresent(present, key) { + return !!present[PRESENT_KEY[key] || key]; + } + function renderServerStatus(st) { lastServerStatus = st || null; var bar = $('ss-srv-chips'), note = $('ss-srv-note'); @@ -773,11 +811,21 @@

Local engine alternative — no server

// Per-model warmup detail (pending / downloading / ready / failed). var wu = (st.health || {}).warmup || {}; + var present = st.models_present || {}; Object.keys(wu).forEach(function (k) { var v = wu[k]; if (typeof v !== 'string') return; - chips.push(chip(k + ': ' + v, WARMUP_TONE[v] || 'off', - { live: v === 'downloading' , dot: v === 'downloading' })); + // The server says "downloading" while it warms a model — even when it is only loading + // a cached file from disk into VRAM. Shown verbatim, that tells the user their weights + // are being re-fetched when nothing is crossing the wire, which is exactly how a + // 1-second RAM load gets reported as a bug. If we can see the file, say "loading". + var label = v, tone = WARMUP_TONE[v] || 'off'; + if (v === 'downloading' && isPresent(present, k)) { + label = 'loading'; + tone = 'info'; + } + var busy = (v === 'downloading'); + chips.push(chip(k + ': ' + label, tone, { live: busy, dot: busy })); }); if (st.disk_bytes) chips.push(chip(gb(st.disk_bytes) + ' on disk', 'info')); @@ -885,20 +933,20 @@

Local engine alternative — no server

setServerButtons(true); $('ss-srv-progress').style.display = 'block'; $('ss-srv-log').textContent = label + '…\n'; - setSrvBar(0.02, label); + setSrvBar(0.02, label, true); connectWS(); return j(path, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body || {}) }) .then(function (res) { if (res && res.ok === false && res.busy) { srvLog('… ' + res.message); - setSrvBar(0, 'Busy: ' + res.busy); + setSrvBar(0, 'Busy: ' + res.busy, true); setServerButtons(false); } return res; }).catch(function (err) { srvLog('✗ ' + label + ' failed: ' + err); - setSrvBar(0, 'Failed'); + setSrvBar(0, 'Failed', true); setServerButtons(false); }); } @@ -910,14 +958,14 @@

Local engine alternative — no server

setServerButtons(true); $('ss-srv-progress').style.display = 'block'; $('ss-srv-log').textContent = label + '…\n'; - setSrvBar(0.02, label); + setSrvBar(0.02, label, true); connectWS(); return j(path, { method: 'POST' }).then(function (res) { // The backend serialises lifecycle ops; if one is already running it says so // rather than racing (which would corrupt the shared subprocess/state file). if (res && res.ok === false && res.busy) { srvLog('… ' + res.message); - setSrvBar(0, 'Busy: ' + res.busy); + setSrvBar(0, 'Busy: ' + res.busy, true); setServerButtons(false); return res; } @@ -925,13 +973,13 @@

Local engine alternative — no server

// stream over the WS and unlock on server_done/server_error. if (res && typeof res.installed !== 'undefined') { renderServerStatus(res); - setSrvBar(1, 'Done'); + setSrvBar(1, 'Done', true); setServerButtons(false); } return res; }).catch(function (err) { srvLog('✗ ' + label + ' failed: ' + err); - setSrvBar(0, 'Failed'); + setSrvBar(0, 'Failed', true); setServerButtons(false); }); } @@ -985,7 +1033,7 @@

Local engine alternative — no server

$('ss-srv-progress').style.display = 'block'; j('/server/health').then(function (r) { srvLog((r.ok ? '✓ ' : '✗ ') + r.url + ' → ' + JSON.stringify(r.health)); - setSrvBar(r.ok ? 1 : 0, r.ok ? 'Healthy' : 'No response'); + setSrvBar(r.ok ? 1 : 0, r.ok ? 'Healthy' : 'No response', true); return loadServerStatus(); }).catch(function (e) { srvLog('✗ ' + e); }) .finally(function () { setServerButtons(false); }); @@ -1149,6 +1197,65 @@

Local engine alternative — no server

.finally(function () { setDkButtons(false); }); }); + + // ── Update the server source ─────────────────────────────────────────────── + // + // server.py is downloaded at INSTALL time and never touched again, so a bug fixed upstream + // cannot reach anyone who already installed — short of uninstalling and re-downloading + // several GB of wheels for a one-line change, which nobody does. In practice the fix never + // lands. (The cache sweeper that deletes model weights daily is exactly that: fixed + // upstream, unreachable in the field.) + // + // Two steps on purpose: CHECK hits the network, so it only happens on a click — the status + // poll must stay offline. Then update only if there is actually something newer, so the + // button doesn't restart a healthy server for nothing. + $('ss-srv-update').addEventListener('click', function () { + setServerButtons(true); + $('ss-srv-progress').style.display = 'block'; + $('ss-srv-log').textContent = 'Checking for a newer server revision…\n'; + setSrvBar(0.1, 'Checking', true); + + j('/server/check_update').then(function (r) { + if (!r || !r.installed) { + srvLog('The server is not installed, so there is nothing to update.'); + setSrvBar(0, 'Not installed', true); + setServerButtons(false); + return; + } + if (r.latest == null) { + srvLog('✗ ' + (r.reason || 'Could not reach GitHub.')); + setSrvBar(0, 'Offline', true); + setServerButtons(false); + return; + } + var have = (r.commit || '').slice(0, 8) || 'unknown'; + var latest = (r.latest || '').slice(0, 8); + if (!r.update_available && !r.unknown) { + srvLog('✓ Already up to date (' + have + ').'); + setSrvBar(1, 'Up to date', true); + setServerButtons(false); + return; + } + srvLog(r.unknown + ? 'This install predates commit tracking, so its revision is unknown — updating to ' + + latest + '.' + : 'Update available: ' + have + ' → ' + latest + '. Updating…'); + setSrvBar(0.2, 'Updating'); + connectWS(); + return j('/server/update', { method: 'POST' }).then(function (res) { + if (res && res.ok === false && res.busy) { + srvLog('… ' + (res.message || 'another server operation is running')); + setSrvBar(0, 'Busy', true); + setServerButtons(false); + } + }); + }).catch(function (e) { + srvLog('✗ ' + e); + setSrvBar(0, 'Failed', true); + setServerButtons(false); + }); + }); + loadConfig(); loadDocker(); connectWS(); // connect on load so a snapshot recovers any install that finished while away diff --git a/tests/test_update_server.py b/tests/test_update_server.py new file mode 100644 index 0000000..2ac4562 --- /dev/null +++ b/tests/test_update_server.py @@ -0,0 +1,478 @@ +"""`update_server()` — the path that lets a fix reach an install that already exists. + +Every bug locked in here was found in review, not in use, and none of them would have shown up +in my own live test of the button: I run on the default port, so a dropped port looks fine, and +a progress bar that jumps backwards for one frame is not something you notice while watching a +server restart. They are exactly the bugs a test catches and a demo doesn't. +""" +import json +import os +import sys +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +from fastapi import FastAPI # noqa: E402 +from fastapi.testclient import TestClient # noqa: E402 + +import demucs_server as ds # noqa: E402 +import demucs_server as demucs_server_mod # noqa: E402 (the module routes.py patches against) + +P = "/api/plugins/stem_splitter" + +# The lifecycle routes only read these four. +SETTINGS = { + "remote_model": "bs_roformer_sw", + "local_server_port": 7865, + "local_server_device": "", + "local_server_ref": "", + "local_server_cuda_tag": "", + "local_server_gpu": None, + "local_server_autostart": False, +} + + +def _settle(seen, tries=50): + """run_server_op() runs the op on a daemon thread; give it a moment to land.""" + import time + for _ in range(tries): + if seen: + return + time.sleep(0.05) + + +class _Stubbed: + """update_server() with everything that touches the network, the disk, or a process stubbed + out — so what's under test is the ORCHESTRATION: what it emits, and what it hands to start.""" + + def __init__(self, td, was_running=True, before="a" * 40, after="b" * 40, live_port=None): + self.cfg = Path(td) + self.progress = [] # every (pct, phase) emitted, in order + self.started = [] # every start_server(**kwargs) + self._commits = [before, after] + self.patches = [ + mock.patch.object(ds, "installed", return_value=True), + mock.patch.object(ds, "is_running", return_value=(was_running, live_port)), + mock.patch.object(ds, "source_meta", side_effect=self._commit), + mock.patch.object(ds, "stop_server"), + mock.patch.object(ds, "download_source", side_effect=self._download), + mock.patch.object(ds, "write_launcher"), + mock.patch.object(ds, "patch_driver_scripts", return_value=["run_demucs.py"]), + mock.patch.object(ds, "verify_install"), + mock.patch.object(ds, "models_downloaded", return_value=False), + mock.patch.object(ds, "start_server", side_effect=self._start), + mock.patch.object(ds, "server_status", return_value={}), + ] + + def _commit(self, *_a, **_k): + return {"commit": self._commits.pop(0) if len(self._commits) > 1 else self._commits[0]} + + # download_source() and start_server() are whole operations with their OWN 0→1 progress. + # Stubbing them silent hid a real bug: update_server forwarded its callback into them + # verbatim, so the bar leapt back to 2% right after we reported 20%. A stub that emits + # nothing cannot see that — so these emit like the real thing. + def _sub_progress(self, cb): + if cb: + cb({"line": "starting…", "pct": 0.02, "phase": "sub"}) + cb({"line": "halfway…", "pct": 0.5, "phase": "sub"}) + cb({"line": "done", "pct": 1.0, "phase": "sub"}) + + def _download(self, config_dir, ref=None, progress_cb=None): + self._sub_progress(progress_cb) + + def _start(self, config_dir, progress_cb=None, **kw): + self.started.append(kw) + self._sub_progress(progress_cb) + return {} + + def _cb(self, ev): + self.progress.append((ev["pct"], ev["phase"])) + + def run(self, **kw): + for p in self.patches: + p.start() + try: + return ds.update_server(self.cfg, progress_cb=self._cb, **kw) + finally: + # LIFO. A test that patches the same attribute twice (the failure cases below do) + # has the second patch capture the FIRST MOCK as its original — so unwinding in + # start order restores a MagicMock onto the module and leaks it into every test + # that runs after. + for p in reversed(self.patches): + p.stop() + + +class TheProgressBarNeverGoesBackwards(unittest.TestCase): + def test_monotonic(self): + """It emitted 0.86 for the dependency check and then 0.85 for the result — so the bar + visibly ran backwards. Harmless, and precisely the kind of thing that creeps back in + every time someone inserts a step.""" + with tempfile.TemporaryDirectory() as td: + s = _Stubbed(td) + s.run() + pcts = [p for p, _ in s.progress] + self.assertEqual(pcts, sorted(pcts), + f"progress must never decrease, got {pcts}") + self.assertEqual(pcts[-1], 1.0, "and it must finish at 100%") + + +class TheServerComesBackWhereItWas(unittest.TestCase): + """The bug: update_server() took port/device/model and then restarted on the DEFAULTS. + + A user who moved the server off 7865 (or onto CUDA) clicked "Update server" and got it back + somewhere else — or colliding with whatever was already on 7865. Invisible to me: I run on + the default port, so my live test of the button could not have failed.""" + + def test_restarts_on_the_configured_port_device_and_model(self): + with tempfile.TemporaryDirectory() as td: + s = _Stubbed(td) + s.run(port=9123, device="cuda", model="htdemucs") + self.assertEqual(len(s.started), 1) + self.assertEqual(s.started[0]["port"], 9123) + self.assertEqual(s.started[0]["device"], "cuda") + self.assertEqual(s.started[0]["model"], "htdemucs") + + def test_a_failed_update_puts_the_server_back_on_its_own_port(self): + # We stopped a WORKING server. If the update dies, leaving it down is strictly worse + # than never having clicked — and bringing it back on the wrong port is barely better. + with tempfile.TemporaryDirectory() as td: + s = _Stubbed(td) + s.patches.append( + mock.patch.object(ds, "download_source", side_effect=RuntimeError("no network"))) + with self.assertRaises(RuntimeError): + s.run(port=9123, device="cuda") + self.assertEqual(len(s.started), 1, "the server must be restarted, not left down") + self.assertEqual(s.started[0]["port"], 9123) + self.assertEqual(s.started[0]["device"], "cuda") + + def test_a_stopped_server_is_not_started_by_an_update(self): + with tempfile.TemporaryDirectory() as td: + s = _Stubbed(td, was_running=False) + s.run() + self.assertEqual(s.started, [], "updating must not start a server the user had stopped") + + def test_a_new_source_that_cannot_import_is_never_started(self): + """The NEW source must not be started — it would only crash-loop, and "it keeps + restarting" is a far worse message than "the update needs new dependencies". + + What does get started is the OLD source, put back by the rollback (see + AFailedUpdateLeavesNothingBroken): the user clicked a button and ends up exactly where + they were, which is the whole point.""" + with tempfile.TemporaryDirectory() as td: + s = _Stubbed(td) + s.patches.append( + mock.patch.object(ds, "verify_install", side_effect=RuntimeError("no sphn"))) + s.patches.append(mock.patch.object(ds, "_restore_source", return_value=True)) + out = s.run() + self.assertTrue(out.get("needs_reinstall")) + self.assertTrue(out.get("rolled_back")) + self.assertEqual(len(s.started), 1, + "the rolled-back server must be running again — leaving it stopped is " + "the failure this rollback exists to prevent") + + + def test_the_live_port_beats_the_configured_one(self): + """The settings say 7865, the server is actually on 9001 (the user edited the port and + never restarted). Putting it back on 7865 moves a running server — or collides with + whatever is there — which is the exact failure the port argument exists to prevent.""" + with tempfile.TemporaryDirectory() as td: + s = _Stubbed(td, live_port=9001) + s.run(port=7865) + self.assertEqual(s.started[0]["port"], 9001, + "the server must come back where it was, not where the settings guess") + + +class RefNormalization(unittest.TestCase): + """check_update() trimmed the ref; update_server() passed the raw settings value through. + + Same input, two behaviours: a ref with stray whitespace reports an update available and then + fails to apply it — which the user reads as "the update button is broken".""" + + def test_check_and_apply_normalize_identically(self): + with tempfile.TemporaryDirectory() as td: + s = _Stubbed(td) + seen = {} + s.patches.append(mock.patch.object( + ds, "download_source", + side_effect=lambda cfg, ref=None, progress_cb=None: seen.setdefault("ref", ref))) + s.run(ref=" main\n") + + with mock.patch.object(ds, "installed", return_value=True), \ + mock.patch.object(ds, "source_meta", return_value={"commit": "x" * 40}), \ + mock.patch.object(ds, "_resolve_commit", return_value="y" * 40) as res: + ds.check_update(s.cfg, ref=" main\n") + + self.assertEqual(seen["ref"], "main") + self.assertEqual(res.call_args.args[0], "main") + + def test_an_empty_ref_falls_back_to_the_default(self): + with tempfile.TemporaryDirectory() as td: + s = _Stubbed(td) + seen = {} + s.patches.append(mock.patch.object( + ds, "download_source", + side_effect=lambda cfg, ref=None, progress_cb=None: seen.setdefault("ref", ref))) + s.run(ref=" ") + self.assertEqual(seen["ref"], ds.DEFAULT_SOURCE_REF) + + +class TheStubsDoNotLeak(unittest.TestCase): + """A patch stopped out of order restores a MagicMock onto the module — and every test that + runs afterwards silently exercises the mock instead of the code.""" + + def test_the_module_is_intact_after_a_double_patched_run(self): + real_download, real_verify = ds.download_source, ds.verify_install + with tempfile.TemporaryDirectory() as td: + s = _Stubbed(td) + s.patches.append( + mock.patch.object(ds, "verify_install", side_effect=RuntimeError("no sphn"))) + s.run() + self.assertIs(ds.download_source, real_download) + self.assertIs(ds.verify_install, real_verify) + + +class EveryLifecyclePathRunsTheSameServer(unittest.TestCase): + """The model was threaded through Update and nowhere else. + + So a user who changed the split model got that model after an Update and DEFAULT_MODEL after + a plain Start — the same server warming a different model depending on which button they + pressed, and a split whose behaviour depends on how the server happened to come up. Start, + Update, Install, Prepare models and autostart now all take their (port, device, model) from + one place.""" + + def _routes(self, td, settings): + """A real JobManager over a real settings file — the routes read it per request, so a + mock that only spans setup() would be gone by the time the button is pressed.""" + import routes + (Path(td) / "stem_splitter.json").write_text(json.dumps(settings), encoding="utf-8") + app = FastAPI() + # can_manage False keeps the autostart probe from doing anything on its own. + with mock.patch.object(demucs_server_mod, "can_manage", return_value=(False, "test")): + routes.setup(app, {"config_dir": td}) + return TestClient(app) + + def test_start_uses_the_configured_model_not_the_default(self): + seen = {} + settings = dict(SETTINGS, remote_model="htdemucs", local_server_port=9001, + local_server_device="cuda") + with tempfile.TemporaryDirectory() as td: + client = self._routes(td, settings) + with mock.patch.object(demucs_server_mod, "start_server", + side_effect=lambda cfg, **kw: seen.update(kw) or {}): + client.post(f"{P}/server/start") + _settle(seen) + + self.assertEqual(seen.get("model"), "htdemucs", + "Start ignored the configured model, so the same server warmed a " + "different model depending on which button started it") + self.assertEqual(seen.get("port"), 9001) + self.assertEqual(seen.get("device"), "cuda") + + def test_prepare_models_downloads_the_configured_model(self): + # Otherwise the explicit ~2 GB fetch grabs the weights for a model the user isn't using. + seen = {} + settings = dict(SETTINGS, remote_model="htdemucs") + with tempfile.TemporaryDirectory() as td: + client = self._routes(td, settings) + with mock.patch.object(demucs_server_mod, "prepare_models", + side_effect=lambda cfg, **kw: seen.update(kw) or {}): + client.post(f"{P}/server/prepare_models") + _settle(seen) + self.assertEqual(seen.get("model"), "htdemucs") + + +class AFailedUpdateLeavesNothingBroken(unittest.TestCase): + """download_source() overwrites the source tree IN PLACE. + + So by the time verify_install() discovers the new revision needs a dependency this install + doesn't have, the old — working — source is already gone. The user is left with a stopped + server and the only source on disk being one that cannot run: strictly worse than never + having clicked, and not recoverable from the button they clicked. Snapshot, then roll back. + """ + + def _install(self, td, body="OLD", commit="a" * 40): + src = ds.src_dir(Path(td)) + src.mkdir(parents=True, exist_ok=True) + (src / "server.py").write_text(body, encoding="utf-8") + ds._source_meta_file(Path(td)).write_text( + json.dumps({"repo": "x", "ref": "main", "commit": commit}), encoding="utf-8") + return src + + def _run(self, td, download, verify, was_running=True): + started = [] + # source_meta() is NOT stubbed: it reads source.json, which is exactly what has to roll + # back with the tree, so the test must see the real file. + with mock.patch.object(ds, "installed", return_value=True), \ + mock.patch.object(ds, "is_running", return_value=(was_running, 7865)), \ + mock.patch.object(ds, "stop_server"), \ + mock.patch.object(ds, "download_source", side_effect=download), \ + mock.patch.object(ds, "write_launcher"), \ + mock.patch.object(ds, "patch_driver_scripts", return_value=[]), \ + mock.patch.object(ds, "verify_install", side_effect=verify), \ + mock.patch.object(ds, "models_downloaded", return_value=False), \ + mock.patch.object(ds, "server_status", return_value={}), \ + mock.patch.object(ds, "start_server", + side_effect=lambda cfg, **kw: started.append(kw) or {}): + try: + out = ds.update_server(Path(td)) + except Exception as e: + out = e + return out, started + + def _overwrite(self, td, commit="b" * 40): + """What download_source() really does: rewrite the tree AND record the new commit.""" + def download(config_dir, ref=None, progress_cb=None): + (ds.src_dir(Path(td)) / "server.py").write_text("NEW", encoding="utf-8") + ds._source_meta_file(Path(td)).write_text( + json.dumps({"repo": "x", "ref": "main", "commit": commit}), encoding="utf-8") + return download + + def _boom(self, msg="no sphn"): + def verify(cfg, progress_cb=None): + raise RuntimeError(msg) + return verify + + def test_a_source_that_cannot_import_is_rolled_back(self): + with tempfile.TemporaryDirectory() as td: + src = self._install(td) + out, started = self._run(td, self._overwrite(td), verify=self._boom()) + + self.assertEqual((src / "server.py").read_text(encoding="utf-8"), "OLD", + "the working source must be back on disk — rolling forward strands " + "the user with a server that only crash-loops") + self.assertTrue(out["rolled_back"]) + self.assertTrue(out["needs_reinstall"]) + self.assertFalse(out["updated"], "nothing was updated: we put it back") + self.assertEqual(len(started), 1, "the server it was running must be running again") + + def test_the_recorded_commit_is_rolled_back_with_the_tree(self): + """source.json is part of the install, not decoration. + + download_source() rewrites it. Restore the tree but not the commit, and check_update() + believes the REJECTED revision is installed — so it reports "up to date" and stops + offering the update. The old source runs under a new name, and the fix the user was + trying to reach becomes unreachable from the button that exists to reach it.""" + with tempfile.TemporaryDirectory() as td: + self._install(td, commit="a" * 40) + self._run(td, self._overwrite(td, commit="b" * 40), verify=self._boom()) + + self.assertEqual(ds.source_meta(Path(td)).get("commit"), "a" * 40, + "the recorded commit must be the one actually on disk, or the " + "update we just rejected looks installed and is never offered again") + + def test_no_snapshot_means_no_update(self): + # An in-place overwrite we cannot undo is the exact failure the snapshot prevents. Better + # to refuse: nothing has been touched yet, so refusing costs only the click. + with tempfile.TemporaryDirectory() as td: + src = self._install(td) + with mock.patch.object(ds, "_snapshot_source", + side_effect=RuntimeError("disk full")), \ + mock.patch.object(ds, "installed", return_value=True), \ + mock.patch.object(ds, "stop_server") as stop, \ + mock.patch.object(ds, "download_source") as dl: + with self.assertRaises(RuntimeError): + ds.update_server(Path(td)) + stop.assert_not_called() + dl.assert_not_called() + self.assertEqual((src / "server.py").read_text(encoding="utf-8"), "OLD") + + def test_a_failed_restore_keeps_the_backup(self): + # At that moment the snapshot is the ONLY intact copy of a working install. Deleting it + # turns a bad day into an unrecoverable one. + with tempfile.TemporaryDirectory() as td: + src = self._install(td) + with mock.patch.object(ds, "_restore_source", return_value=False): + out, _ = self._run(td, self._overwrite(td), verify=self._boom()) + self.assertFalse(out["rolled_back"]) + self.assertTrue(src.with_name(src.name + ".bak").is_dir(), + "the backup must survive a failed restore — it is the only copy of " + "the user's working install left") + + def test_a_failed_fetch_is_rolled_back_too(self): + # A half-extracted tree is the same trap as a source that can't import. + def half_extract(config_dir, ref=None, progress_cb=None): + (ds.src_dir(config_dir) / "server.py").write_text("HALF", encoding="utf-8") + raise RuntimeError("connection reset") + + with tempfile.TemporaryDirectory() as td: + src = self._install(td) + out, started = self._run(td, half_extract, verify=lambda cfg, progress_cb=None: None) + self.assertIsInstance(out, RuntimeError) # the failure is still reported + self.assertEqual((src / "server.py").read_text(encoding="utf-8"), "OLD") + self.assertEqual(len(started), 1) + + def test_the_snapshot_is_cleaned_up_on_success(self): + with tempfile.TemporaryDirectory() as td: + src = self._install(td) + self._run(td, self._overwrite(td), verify=lambda cfg, progress_cb=None: None) + self.assertEqual((src / "server.py").read_text(encoding="utf-8"), "NEW", + "a successful update must actually update") + self.assertFalse(src.with_name(src.name + ".bak").exists(), + "the snapshot must not be left behind") + + +class CheckUpdateContract(unittest.TestCase): + def test_unknown_is_present_even_when_github_is_unreachable(self): + # A caller reading `unknown` to decide whether to offer an update to an install with no + # recorded commit must get the same signal on every path, not a missing key on one. + with tempfile.TemporaryDirectory() as td: + cfg = Path(td) + with mock.patch.object(ds, "installed", return_value=True), \ + mock.patch.object(ds, "source_meta", return_value={}), \ + mock.patch.object(ds, "_resolve_commit", return_value=None): + out = ds.check_update(cfg) + self.assertIn("unknown", out) + self.assertTrue(out["unknown"]) + self.assertFalse(out["update_available"]) + + +class StatusPollStaysCheap(unittest.TestCase): + """server_status() is polled every few seconds and must stay offline and cheap. + + It called models_downloaded() (which is the AND of the three presence checks) AND then each + of the three again for models_present — running every check twice per poll, including + _has_whisper()'s rglob() over the whole faster-whisper snapshot tree.""" + + def test_the_cache_is_walked_once_per_poll(self): + with tempfile.TemporaryDirectory() as td: + cfg = Path(td) + cache = ds.cache_dir(cfg) + with mock.patch.object(ds, "installed", return_value=True), \ + mock.patch.object(ds, "is_running", return_value=(False, None)), \ + mock.patch.object(ds, "_read_state", return_value={}), \ + mock.patch.object(ds, "_has_roformer", return_value=True) as rof, \ + mock.patch.object(ds, "_has_whisper", return_value=True) as whi, \ + mock.patch.object(ds, "_has_aligner", return_value=True) as ali: + st = ds.server_status(cfg) + + # Count only the calls for OUR cache. The routes tests leave an autostart/status poller + # on a daemon thread, and it calls these same patched functions with its own config dir — + # a bare call_count is a cross-test race that fails depending on file order. + for name, m in (("_has_roformer", rof), ("_has_whisper", whi), ("_has_aligner", ali)): + mine = [c for c in m.call_args_list if c.args and c.args[0] == cache] + self.assertEqual(len(mine), 1, + f"{name} ran {len(mine)}x in one poll — this is the hot path") + # and the flag must still agree with the per-model dict it is now derived from + self.assertTrue(st["models_downloaded"]) + self.assertEqual(st["models_present"], + {"bs_roformer_sw": True, "whisperx": True, "whisperx_aligners": True}) + + def test_the_flag_is_false_when_any_single_model_is_missing(self): + with tempfile.TemporaryDirectory() as td: + cfg = Path(td) + with mock.patch.object(ds, "installed", return_value=True), \ + mock.patch.object(ds, "is_running", return_value=(False, None)), \ + mock.patch.object(ds, "_read_state", return_value={}), \ + mock.patch.object(ds, "_has_roformer", return_value=True), \ + mock.patch.object(ds, "_has_whisper", return_value=True), \ + mock.patch.object(ds, "_has_aligner", return_value=False): + st = ds.server_status(cfg) + self.assertFalse(st["models_downloaded"], + "warming up with the aligner missing downloads it AT LAUNCH") + + +if __name__ == "__main__": + unittest.main()