fix(loops): take the DB lock for the count+insert and the list read

Two pre-existing races in the loops routes, flagged by CodeRabbit on #839 (the
verbatim extraction moved them unchanged from server.py, so they correctly
weren't fixed there):

1. save_loop computed `COUNT(*)` OUTSIDE meta_db._lock, then inserted inside it.
   Two simultaneous unnamed POSTs read the same count and both mint "Loop N".
   Fix: one lock scope around COUNT + INSERT.
2. list_loops read the shared single connection (check_same_thread=False) with
   no lock, so it could overlap a POST/DELETE commit. Fix: read under the lock,
   like every writer.

Low severity in context — FeedBack is single-user (Principle I), so concurrent
unnamed-loop POSTs essentially can't happen — but each fix is one lock scope.

tests/test_loops_concurrency.py pins both with a threading.Barrier that releases
16 workers into save_loop at once. Negative-checked: reverting the COUNT back
outside the lock fails the uniqueness assertion 5/5 runs; the fix passes 3/3.
pytest 2400 passed; on-device two unnamed POSTs -> ['Loop 1','Loop 2'].

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

CHANGELOG.md

@@ -26,7 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   next root-level module can't ship broken.
 
 ### Added
-- **`routers/` — extracting `server.py`'s route layer, cheapest-first (R3).** Each PR moves a cohesive route group into a `fastapi.APIRouter` under `lib/routers/`, mounted with `app.include_router(...)` at its original site (FastAPI matches in registration order; the full route table stays byte-identical). Bodies are verbatim — only the decorator receiver (`@app` → `@router`) and singleton reads (`meta_db` → `appstate.meta_db`, resolved at call time) change. So far: `audio_effects` (5), `artist_aliases` (5), `loops` (3), `playlists` (12 + covers), `ws_highway` (the 902-line highway chart WebSocket — the single largest handler). The DLC library-path resolution (`_get_dlc_dir`, pure `_resolve_dlc_path`) moved to `lib/dlc_paths.py`, reading paths through the seam; `config_dir`/`dlc_dir`/`dlc_dir_env` now ride the `appstate` seam (env-derived, so the pop-and-reimport fixtures reconfigure it for free), and the shared request-field sanitizer `_clean_str` moved to `lib/reqfields.py`. The next cut is picked by a dependency-closure scan that ranks groups by how many `monkeypatch.setattr(server, …)` targets they'd drag along.
+- **`routers/` — extracting `server.py`'s route layer, cheapest-first (R3).** Each PR moves a cohesive route group into a `fastapi.APIRouter` under `lib/routers/`, mounted with `app.include_router(...)` at its original site (FastAPI matches in registration order; the full route table stays byte-identical). Bodies are verbatim — only the decorator receiver (`@app` → `@router`) and singleton reads (`meta_db` → `appstate.meta_db`, resolved at call time) change. So far: `audio_effects` (5), `artist_aliases` (5), `loops` (3 — saved A/B practice regions). The next cut is picked by a dependency-closure scan that ranks groups by how many `monkeypatch.setattr(server, …)` targets they'd drag along.
 - **`routers/` — the first extracted route module (R3).** The five audio-effects mapping
   endpoints move out of `server.py` into `lib/routers/audio_effects.py` as a
   `fastapi.APIRouter`, mounted with `app.include_router(...)` **at the point in the file

docs/size-exemptions.md

@@ -55,8 +55,8 @@ without a *signed* exemption" is unenforceable.
 ## Planned, NOT exempt (owned by split plans — listed so nothing falls between states)
 
 core `static/app.js` (11,852) · `static/highway.js` (4,168, whole file) · `server.py`
-(8,003 — was 14,037; ratcheted by the R3 `MetadataDB` + `AudioEffectsMappingDB`
-extractions and five `routers/` modules) ·
+(9,302 — was 14,037; ratcheted by the R3 `MetadataDB` + `AudioEffectsMappingDB`
+extractions and the first three `routers/` modules) ·
 `lib/metadata_db.py` (4,373 — new in R3; the `MetadataDB` class alone is 4,018 lines
 and is a monolith in its own right, to be split per-table once the router train
 lands) · `static/v3/songs.js` (4,134) · `static/capabilities/audio-session.js`

lib/appstate.py

@@ -62,31 +62,10 @@ copies a hardcoded file list — that regression is what moved this file here.
 meta_db = None
 audio_effect_mappings = None
 
-# Config paths. server.py derives these from the environment (fresh on every
-# import, so the ~49 pop-and-reimport fixtures keep working) and injects them
-# here. Routers read them as `appstate.config_dir` etc. — a module attribute at
-# call time. NOTE: config_dir/dlc_dir are env-derived, so a `setenv`+reimport
-# test reconfigures them for free; STATIC_DIR/SLOPPAK_CACHE_DIR are patched via
-# `setattr(server, …)` in a few tests, so those slots (when added) need their
-# tests retargeted to appstate in the same PR.
-config_dir = None
-dlc_dir = None          # the DLC_DIR env value as a Path (Path("") if unset)
-dlc_dir_env = None      # the raw DLC_DIR env string, "" if unset — distinguishes
-                        # "unset" from Path("")→"." (see dlc_paths._get_dlc_dir)
-# Cache/asset dirs. static_dir + sloppak_cache_dir are patched via
-# `setattr(server, …)` in a few tests, so a router reading them here needs those
-# setattr sites retargeted to `setattr(appstate, …)` in the same PR (ws_highway
-# retargets the 3 test_highway_ws_* SLOPPAK sites). config_dir-derived dirs are
-# reconfigured for free on a setenv+reimport.
-static_dir = None
-sloppak_cache_dir = None
-audio_cache_dir = None
-
-_SLOTS = frozenset({
-    "meta_db", "audio_effect_mappings",
-    "config_dir", "dlc_dir", "dlc_dir_env",
-    "static_dir", "sloppak_cache_dir", "audio_cache_dir",
-})
+# Declared up front so `configure()` can reject a typo'd or stale keyword
+# instead of silently creating a new global that nothing ever reads. A seam
+# whose wiring can no-op undetected is worse than no seam.
+_SLOTS = frozenset({"meta_db", "audio_effect_mappings"})
 
 
 def configure(**kwargs) -> None:

lib/dlc_paths.py

@@ -1,99 +0,0 @@
-"""DLC library path resolution — where the song files live, plus safe containment.
-
-Extracted from ``server.py`` (R3). ``_resolve_dlc_path`` is pure and moved
-verbatim. ``_get_dlc_dir`` reads the env-derived paths through the ``appstate``
-seam (``server.py`` configures ``dlc_dir``/``dlc_dir_env``/``config_dir`` at
-import, fresh on every re-import), so this module does no import-time IO and the
-pop-and-reimport fixtures keep working. ``server.py`` re-exports both names, so
-existing ``server._get_dlc_dir`` / ``server._resolve_dlc_path`` references
-(tests, other handlers) resolve unchanged.
-"""
-
-import json
-import os
-from pathlib import Path
-
-import appstate
-
-
-def _get_dlc_dir(cfg: dict | None = None) -> Path | None:
-    # Only consider DLC_DIR if the env var was non-empty. `Path("")` collapses
-    # to `.` and reports `.is_dir() == True`, which would silently shadow the
-    # config.json fallback. Checking the raw env string preserves
-    # `DLC_DIR=.` as a valid opt-in for cwd while keeping unset/empty out.
-    if appstate.dlc_dir_env and appstate.dlc_dir.is_dir():
-        return appstate.dlc_dir
-    if cfg is None:
-        config_file = appstate.config_dir / "config.json"
-        if config_file.exists():
-            try:
-                cfg = json.loads(config_file.read_text(encoding="utf-8"))
-            except Exception:
-                pass
-    if isinstance(cfg, dict):
-        raw = str(cfg.get("dlc_dir", "")).strip()
-        if raw:
-            p = Path(raw)
-            if p.is_dir():
-                return p
-    return None
-
-
-def _resolve_dlc_path(dlc: Path, filename: str) -> Path | None:
-    """Resolve `filename` under DLC_DIR and refuse anything that escapes.
-
-    `filename` arrives from `:path` route params and can contain `..`
-    segments. The Sloppak and archive paths happen to fail safely later
-    because their loaders raise on missing/invalid files, but loose-
-    folder format detection (`is_loose_song`) globs and parses XML on
-    disk first, which lets a crafted path trigger filesystem reads
-    outside DLC_DIR before any guard fires. Centralise the containment
-    check so every filename-bound handler validates before touching the
-    filesystem.
-
-    Containment here is LEXICAL (normalize `.`/`..` WITHOUT following
-    symlinks), not `safe_join`'s `.resolve()`-based check — because users
-    commonly mount their song library through a directory JUNCTION/symlink
-    (a library shared across app installs; the desktop app's own mounts).
-    `.resolve()` follows that junction to its real target, sees it sits
-    outside DLC_DIR, and wrongly rejects every song reached through it — the
-    scanner's `rglob` indexes those songs, but art/load then 403/404s (broken
-    covers, unplayable songs). Lexical normalization still rejects the only
-    escapes a `:path` filename can express — `..` traversal and absolute
-    paths — which the traversal tests pin. `safe_join` stays strict (it is
-    the zip-slip / plugin-asset guard, where following a symlink out IS the
-    defense); the loose-folder art handler keeps its own per-file symlink
-    re-check for defence-in-depth.
-
-    Returns the validated Path (not necessarily link-resolved), or None if
-    the filename is empty, contains a NUL, or escapes the DLC root.
-    """
-    if not filename:
-        return None
-    # Backslashes → forward slashes so a Windows-style `..\\x` traversal is
-    # rejected identically on POSIX (mirrors safe_join's normalisation).
-    safe = filename.replace("\\", "/")
-    if "\x00" in safe:
-        return None
-    # Reject drive-letter / absolute paths in BOTH conventions. A POSIX "/x" is
-    # caught by the containment check below (the `/` operator discards `root`),
-    # but a Windows drive-absolute "C:/x" is treated as a relative "C:" dir on
-    # POSIX and would otherwise slip in as `<root>/C:/x` — so the contract must
-    # hold cross-platform (a shared library is reached from either OS).
-    from pathlib import PurePosixPath, PureWindowsPath
-    if (PurePosixPath(safe).is_absolute()
-            or PureWindowsPath(safe).is_absolute()
-            or PureWindowsPath(safe).drive):
-        return None
-    try:
-        root = dlc.resolve()
-        # normpath collapses `.`/`..`/duplicate separators purely lexically —
-        # it never touches the filesystem, so an in-library junction component
-        # is preserved (allowed) while `..`/absolute segments still escape and
-        # get caught by the containment check below.
-        candidate = Path(os.path.normpath(root / safe))
-        if not candidate.is_relative_to(root):
-            return None
-    except (ValueError, OSError):
-        return None
-    return candidate

lib/reqfields.py

@@ -1,13 +0,0 @@
-"""Request-field coercion helpers shared by the raw-`dict` POST handlers.
-
-Extracted verbatim from ``server.py`` (R3). Pure — no IO, no globals — so it
-imports cleanly from both ``server`` and any ``routers/`` module.
-"""
-
-
-def _clean_str(value) -> str:
-    """Trim a request field to a string; non-strings (or missing) → ''.
-    Lets the raw-`dict` POST handlers treat wrong-typed JSON (an int/list/etc.
-    where a string was expected) as "empty" and answer 400, instead of raising
-    AttributeError/TypeError → 500 on a later .strip()/`in`."""
-    return value.strip() if isinstance(value, str) else ""

lib/routers/playlists.py

@@ -1,267 +0,0 @@
-"""Playlists + custom playlist covers (fee[dB]ack v0.3.0).
-
-Extracted verbatim from ``server.py`` (R3). Edits: ``@app`` -> ``@router``,
-``meta_db`` -> ``appstate.meta_db``, ``CONFIG_DIR`` -> ``appstate.config_dir``
-(both read at call time through the seam), and ``_clean_str`` now imports from
-``reqfields``. See ``appstate.py``.
-"""
-
-import logging
-import os
-import tempfile
-from pathlib import Path
-
-from fastapi import APIRouter
-from fastapi.responses import FileResponse, JSONResponse
-
-import appstate
-from reqfields import _clean_str
-
-log = logging.getLogger("feedBack.server")
-
-router = APIRouter()
-
-# Cache policy for the custom-cover file response: revalidate every time so a
-# replaced cover is never served stale (pairs with the mtime-ns URL token).
-_ART_CACHE_HEADERS = {"Cache-Control": "no-cache"}
-
-
-def _playlist_cover_path(pid) -> Path | None:
-    """Filesystem path of a playlist's optional custom cover image (PNG),
-    stored under CONFIG_DIR. Returns None for a non-integer id."""
-    try:
-        pid = int(pid)
-    except (TypeError, ValueError):
-        return None
-    return appstate.config_dir / "playlist_covers" / f"{pid}.png"
-
-
-def _playlist_cover_url(pid) -> str | None:
-    cover = _playlist_cover_path(pid)
-    if not cover or not cover.exists():
-        return None
-    try:
-        # Nanosecond mtime so a same-second replace/remove/re-upload still
-        # changes the cache-bust token (int seconds could collide → stale image).
-        mt = cover.stat().st_mtime_ns
-    except OSError:
-        mt = 0
-    return f"/api/playlists/{pid}/cover?v={mt}"
-
-
-@router.get("/api/playlists")
-def api_list_playlists():
-    lists = appstate.meta_db.list_playlists()
-    for pl in lists:
-        pl["cover_url"] = _playlist_cover_url(pl["id"])
-    return lists
-
-
-@router.post("/api/playlists")
-def api_create_playlist(data: dict):
-    name = _clean_str(data.get("name"))
-    if not (1 <= len(name) <= 100):
-        return JSONResponse({"error": "Playlist name must be 1–100 characters."}, status_code=400)
-    # kind='album' = a curated album (§7.2): hand-picked works, a chosen chart
-    # per slot, played front-to-back on the queue. Absent/None = a regular mix.
-    kind = _clean_str(data.get("kind")) or None
-    if kind not in (None, "album"):
-        return JSONResponse({"error": "kind must be 'album' or omitted"}, status_code=400)
-    return appstate.meta_db.create_playlist(name, kind=kind)
-
-
-@router.get("/api/playlists/{pid}")
-def api_get_playlist(pid: int):
-    pl = appstate.meta_db.get_playlist(pid)
-    if pl is None:
-        return JSONResponse({"error": "not found"}, status_code=404)
-    pl["cover_url"] = _playlist_cover_url(pid)
-    return pl
-
-
-@router.patch("/api/playlists/{pid}")
-def api_rename_playlist(pid: int, data: dict):
-    pl = appstate.meta_db.get_playlist(pid)
-    if pl is None:
-        return JSONResponse({"error": "not found"}, status_code=404)
-    if pl["system_key"]:
-        return JSONResponse({"error": "System playlists cannot be renamed."}, status_code=400)
-    name = _clean_str(data.get("name"))
-    if not (1 <= len(name) <= 100):
-        return JSONResponse({"error": "Playlist name must be 1–100 characters."}, status_code=400)
-    appstate.meta_db.rename_playlist(pid, name)
-    return appstate.meta_db.get_playlist(pid)
-
-
-@router.delete("/api/playlists/{pid}")
-def api_delete_playlist(pid: int):
-    pl = appstate.meta_db.get_playlist(pid)
-    if pl is None:
-        return JSONResponse({"error": "not found"}, status_code=404)
-    if pl["system_key"]:
-        return JSONResponse({"error": "System playlists cannot be deleted."}, status_code=400)
-    if not appstate.meta_db.delete_playlist(pid):   # vanished under us (concurrent delete)
-        return JSONResponse({"error": "not found"}, status_code=404)
-    cover = _playlist_cover_path(pid)       # drop any custom cover with the playlist
-    if cover and cover.exists():
-        try:
-            cover.unlink()
-        except OSError:
-            pass
-    return {"ok": True}
-
-
-@router.post("/api/playlists/{pid}/songs")
-def api_add_playlist_song(pid: int, data: dict):
-    if appstate.meta_db.get_playlist(pid) is None:
-        return JSONResponse({"error": "not found"}, status_code=404)
-    filename = _clean_str(data.get("filename"))
-    if not filename:
-        return JSONResponse({"error": "filename required"}, status_code=400)
-    if appstate.meta_db.add_playlist_song(pid, filename) is None:   # playlist vanished under us
-        return JSONResponse({"error": "not found"}, status_code=404)
-    pl = appstate.meta_db.get_playlist(pid)
-    return pl if pl is not None else JSONResponse({"error": "not found"}, status_code=404)
-
-
-@router.patch("/api/playlists/{pid}/songs/{filename:path}")
-def api_update_playlist_slot(pid: int, filename: str, data: dict):
-    """Edit one curated-album slot: {"arrangement": name|null} pins/clears the
-    slot's arrangement; {"chart_filename": fn} swaps the slot to another chart
-    of the same work (position + pin kept). Albums only — a mix has no slots."""
-    pl = appstate.meta_db.get_playlist(pid)
-    if pl is None:
-        return JSONResponse({"error": "not found"}, status_code=404)
-    if pl.get("kind") != "album":
-        return JSONResponse({"error": "Slot editing is for albums."}, status_code=400)
-    kwargs = {}
-    if "chart_filename" in data:
-        new_fn = _clean_str(data.get("chart_filename"))
-        if not new_fn:
-            return JSONResponse({"error": "chart_filename must be a filename"}, status_code=400)
-        kwargs["new_filename"] = new_fn
-    if "arrangement" in data:
-        arr = data.get("arrangement")
-        if arr is not None and not (isinstance(arr, str) and 1 <= len(arr.strip()) <= 100):
-            return JSONResponse({"error": "arrangement must be a name or null"}, status_code=400)
-        kwargs["arrangement"] = arr.strip() if isinstance(arr, str) else None
-    if not kwargs:
-        return JSONResponse({"error": "nothing to update"}, status_code=400)
-    if appstate.meta_db.update_playlist_slot(pid, filename, **kwargs) is None:
-        return JSONResponse(
-            {"error": "no such slot, or the chart isn't a version of this song"},
-            status_code=400)
-    return appstate.meta_db.get_playlist(pid)
-
-
-@router.delete("/api/playlists/{pid}/songs/{filename:path}")
-def api_remove_playlist_song(pid: int, filename: str):
-    if appstate.meta_db.get_playlist(pid) is None:
-        return JSONResponse({"error": "not found"}, status_code=404)
-    appstate.meta_db.remove_playlist_song(pid, filename)
-    pl = appstate.meta_db.get_playlist(pid)
-    return pl if pl is not None else JSONResponse({"error": "not found"}, status_code=404)
-
-
-@router.post("/api/playlists/{pid}/reorder")
-def api_reorder_playlist(pid: int, data: dict):
-    pl = appstate.meta_db.get_playlist(pid)
-    if pl is None:
-        return JSONResponse({"error": "not found"}, status_code=404)
-    order = data.get("order")
-    if not isinstance(order, list) or not all(isinstance(f, str) for f in order):
-        return JSONResponse({"error": "order must be a list of filenames"}, status_code=400)
-    # Require an exact permutation of the playlist's current songs: a list with
-    # duplicates, omissions, or extras would otherwise produce duplicate
-    # positions / a partial reorder while still returning 200.
-    current = [s["filename"] for s in pl["songs"]]
-    if len(order) != len(current) or sorted(order) != sorted(current):
-        return JSONResponse(
-            {"error": "order must be a permutation of the playlist's current songs"},
-            status_code=400,
-        )
-    appstate.meta_db.reorder_playlist(pid, order)
-    return appstate.meta_db.get_playlist(pid)
-
-
-@router.post("/api/playlists/{pid}/cover")
-async def api_set_playlist_cover(pid: int, data: dict):
-    """Set a playlist's custom cover from a base64 / data-URL image (PNG/JPG).
-    Overrides the content-dependent (song-art) cover. Stored as a small PNG
-    thumbnail under CONFIG_DIR/playlist_covers/."""
-    if appstate.meta_db.get_playlist(pid) is None:
-        return JSONResponse({"error": "not found"}, status_code=404)
-    import base64
-    import io
-    b64 = data.get("image", "")
-    # Guard the type before the `","` membership test — a non-string image
-    # (e.g. {"image": 123} / null) would otherwise raise TypeError → 500.
-    # Mirrors the avatar/song-art upload guard.
-    if not isinstance(b64, str) or not b64:
-        return JSONResponse({"error": "No image data"}, status_code=400)
-    if "," in b64:
-        b64 = b64.split(",", 1)[1]
-    if not b64:
-        return JSONResponse({"error": "No image data"}, status_code=400)
-    try:
-        img_data = base64.b64decode(b64)
-    except Exception:
-        return JSONResponse({"error": "Invalid base64"}, status_code=400)
-    cover = _playlist_cover_path(pid)
-    cover.parent.mkdir(parents=True, exist_ok=True)
-    # Decode/validate the image — a bad payload is a CLIENT error (400), and the
-    # message stays generic so it can't echo internals.
-    try:
-        from PIL import Image
-        img = Image.open(io.BytesIO(img_data)).convert("RGB")
-        img.thumbnail((640, 640))                  # covers stay small
-    except Exception:
-        return JSONResponse({"error": "Invalid image"}, status_code=400)
-    # Persist. A save/replace failure is a SERVER error (500, logged, no
-    # filesystem detail leaked) — the pre-split handler mislabeled these as 400
-    # and echoed the exception. A unique temp name in the cover dir (not a shared
-    # `{pid}.png.tmp`) means two concurrent uploads can't clobber each other's
-    # temp file; the atomic replace publishes. Re-check the playlist still exists
-    # just before publishing so a delete that raced the decode above can't leave
-    # an orphan cover — cheap belt-and-braces; FeedBack is single-user
-    # (Principle I), so a full per-playlist lock would be for a race the
-    # deployment model precludes.
-    tmp = None
-    try:
-        # mkstemp is inside the try too: an unwritable dir / full disk raises
-        # here, and that's the same class of persistence failure as save/replace.
-        fd, tmp_name = tempfile.mkstemp(prefix=f".{pid}.", suffix=".png.tmp", dir=str(cover.parent))
-        tmp = Path(tmp_name)
-        with os.fdopen(fd, "wb") as f:
-            img.save(f, "PNG")
-        if appstate.meta_db.get_playlist(pid) is None:
-            tmp.unlink(missing_ok=True)
-            return JSONResponse({"error": "not found"}, status_code=404)
-        tmp.replace(cover)
-    except Exception:
-        if tmp is not None:
-            tmp.unlink(missing_ok=True)
-        log.exception("playlist cover save failed (pid=%s)", pid)
-        return JSONResponse({"error": "could not save cover"}, status_code=500)
-    return {"ok": True, "cover_url": _playlist_cover_url(pid)}
-
-
-@router.get("/api/playlists/{pid}/cover")
-def api_get_playlist_cover(pid: int):
-    cover = _playlist_cover_path(pid)
-    if not cover or not cover.exists():
-        return JSONResponse({"error": "not found"}, status_code=404)
-    # no-cache (revalidate) like song art, so a replaced cover is never served
-    # stale — pairs with the mtime-ns cache-bust token on the URL.
-    return FileResponse(str(cover), media_type="image/png", headers=_ART_CACHE_HEADERS)
-
-
-@router.delete("/api/playlists/{pid}/cover")
-def api_delete_playlist_cover(pid: int):
-    cover = _playlist_cover_path(pid)
-    if cover and cover.exists():
-        try:
-            cover.unlink()
-        except OSError:
-            pass
-    return {"ok": True}

tests/test_highway_ws_authors.py

@@ -16,8 +16,6 @@ import pytest
 import yaml
 from fastapi.testclient import TestClient
 
-from routers.ws_highway import _sanitize_authors
-
 
 # ── _sanitize_authors unit tests ────────────────────────────────────────────
 
@@ -37,7 +35,7 @@ def server_mod(monkeypatch, tmp_path):
 
 
 def test_sanitize_authors_valid(server_mod):
-    out = _sanitize_authors(
+    out = server_mod._sanitize_authors(
         {
             "authors": [
                 {"name": "Azure", "role": "charter", "email": "a@b.c", "url": "x"},
@@ -55,7 +53,7 @@ def test_sanitize_authors_valid(server_mod):
 
 
 def test_sanitize_authors_skips_malformed(server_mod):
-    out = _sanitize_authors(
+    out = server_mod._sanitize_authors(
         {
             "authors": [
                 {"name": ""},          # blank name → skipped
@@ -71,7 +69,7 @@ def test_sanitize_authors_skips_malformed(server_mod):
 
 @pytest.mark.parametrize("manifest", [None, {}, {"authors": None}, {"authors": "x"}, "nope"])
 def test_sanitize_authors_absent_or_nonlist(server_mod, manifest):
-    assert _sanitize_authors(manifest) == []
+    assert server_mod._sanitize_authors(manifest) == []
 
 
 # ── song_info WS integration ────────────────────────────────────────────────
@@ -120,9 +118,6 @@ def make_client(tmp_path, monkeypatch):
         monkeypatch.setattr(server, "load_plugins", lambda *a, **kw: None)
         monkeypatch.setattr(server, "startup_scan", lambda: None)
         monkeypatch.setattr(server, "SLOPPAK_CACHE_DIR", tmp_path / "cache")
-        # ws_highway reads the cache dir through the appstate seam now.
-        import appstate as _appstate
-        monkeypatch.setattr(_appstate, "sloppak_cache_dir", tmp_path / "cache")
         return server
 
     (tmp_path / "dlc").mkdir()

tests/test_highway_ws_instrument_routing.py

@@ -96,9 +96,6 @@ def make_client(tmp_path, monkeypatch):
         monkeypatch.setattr(server, "load_plugins", lambda *a, **kw: None)
         monkeypatch.setattr(server, "startup_scan", lambda: None)
         monkeypatch.setattr(server, "SLOPPAK_CACHE_DIR", tmp_path / "cache")
-        # ws_highway reads the cache dir through the appstate seam now.
-        import appstate as _appstate
-        monkeypatch.setattr(_appstate, "sloppak_cache_dir", tmp_path / "cache")
         return server
 
     (tmp_path / "dlc").mkdir()

tests/test_highway_ws_notation.py

@@ -123,9 +123,6 @@ def make_client(tmp_path, monkeypatch):
         monkeypatch.setattr(server, "load_plugins", lambda *a, **kw: None)
         monkeypatch.setattr(server, "startup_scan", lambda: None)
         monkeypatch.setattr(server, "SLOPPAK_CACHE_DIR", tmp_path / "cache")
-        # ws_highway reads the cache dir through the appstate seam now.
-        import appstate as _appstate
-        monkeypatch.setattr(_appstate, "sloppak_cache_dir", tmp_path / "cache")
         return server
 
     (tmp_path / "dlc").mkdir()

tests/test_playlist_cover_errors.py

@@ -1,107 +0,0 @@
-"""Playlist cover upload: client vs server errors, no temp litter, no leak.
-
-The pre-split handler caught decode AND persistence failures in one `except`,
-returned 400 for both, and echoed the exception (`Invalid image: {e}`) — so a
-disk/permission failure was mislabeled as a client error and could leak a
-filesystem path. These pin the split.
-"""
-
-import base64
-import io
-
-import pytest
-from fastapi.testclient import TestClient
-from PIL import Image
-
-import appstate
-from metadata_db import MetadataDB
-from routers import playlists
-
-
-@pytest.fixture()
-def client(tmp_path):
-    prev = (appstate.meta_db, appstate.config_dir)
-    db = MetadataDB(tmp_path)
-    appstate.configure(meta_db=db, config_dir=tmp_path)
-    app_ = __import__("fastapi").FastAPI()
-    app_.include_router(playlists.router)
-    try:
-        yield TestClient(app_), tmp_path
-    finally:
-        db.conn.close()
-        appstate.configure(meta_db=prev[0], config_dir=prev[1])
-
-
-def _png_b64():
-    buf = io.BytesIO()
-    Image.new("RGB", (8, 8), (10, 20, 30)).save(buf, "PNG")
-    return base64.b64encode(buf.getvalue()).decode()
-
-
-def _make_playlist(client):
-    return client.post("/api/playlists", json={"name": "P"}).json()["id"]
-
-
-def test_valid_cover_saves_and_leaves_no_temp(client):
-    c, tmp_path = client
-    pid = _make_playlist(c)
-    r = c.post(f"/api/playlists/{pid}/cover", json={"image": _png_b64()})
-    assert r.status_code == 200
-    cover_dir = tmp_path / "playlist_covers"
-    assert (cover_dir / f"{pid}.png").exists()
-    # The atomic-publish temp file must not linger.
-    assert not list(cover_dir.glob("*.tmp"))
-
-
-def test_undecodable_image_is_a_400_without_leaking(client):
-    c, _ = client
-    pid = _make_playlist(c)
-    # valid base64, not a valid image
-    r = c.post(f"/api/playlists/{pid}/cover", json={"image": base64.b64encode(b"not an image").decode()})
-    assert r.status_code == 400
-    body = r.json()["error"]
-    assert body == "Invalid image"          # generic — no exception detail echoed
-    assert "playlist_covers" not in body    # no filesystem path leak
-
-
-def test_save_failure_is_a_500_not_a_400(client, monkeypatch):
-    """A persistence failure (here: Image.save raising) must be a logged 500,
-    not a 400 — the whole point of the decode/persist split. Negative-checks
-    against the pre-fix behavior, which returned 400 for exactly this."""
-    c, tmp_path = client
-    pid = _make_playlist(c)
-    payload = _png_b64()                     # build BEFORE patching save
-
-    def boom(self, fp, *a, **k):
-        raise OSError("disk full")
-
-    monkeypatch.setattr(Image.Image, "save", boom)
-    r = c.post(f"/api/playlists/{pid}/cover", json={"image": payload})
-
-    assert r.status_code == 500
-    assert "disk full" not in r.json()["error"]           # no internal detail
-    assert not list((tmp_path / "playlist_covers").glob("*.tmp"))  # temp cleaned up
-
-
-def test_temp_creation_failure_is_a_500(client, monkeypatch):
-    """mkstemp raising (unwritable dir / full disk) must hit the same logged 500
-    path as a save failure, not escape as an unhandled server error."""
-    import tempfile as _tempfile
-
-    c, _ = client
-    pid = _make_playlist(c)
-    payload = _png_b64()
-
-    def boom(*a, **k):
-        raise OSError("read-only file system")
-
-    monkeypatch.setattr(_tempfile, "mkstemp", boom)
-    r = c.post(f"/api/playlists/{pid}/cover", json={"image": payload})
-    assert r.status_code == 500
-    assert "read-only" not in r.json()["error"]
-
-
-def test_upload_to_missing_playlist_is_404(client):
-    c, _ = client
-    r = c.post("/api/playlists/9999/cover", json={"image": _png_b64()})
-    assert r.status_code == 404

tests/test_playlists_api.py

@@ -6,10 +6,6 @@ import sys
 import pytest
 from fastapi.testclient import TestClient
 
-# Moved to routers/playlists in R3; reads appstate.config_dir, which the
-# `server` fixture configures via CONFIG_DIR before this is called.
-from routers.playlists import _playlist_cover_path
-
 
 @pytest.fixture()
 def server(tmp_path, monkeypatch, isolate_logging):
@@ -172,6 +168,6 @@ def test_cover_rejects_non_string_image_with_400_not_500(client):
 def test_deleting_playlist_removes_custom_cover(client, server):
     pid = client.post("/api/playlists", json={"name": "Doomed"}).json()["id"]
     client.post(f"/api/playlists/{pid}/cover", json={"image": _png_b64()})
-    assert _playlist_cover_path(pid).exists()
+    assert server._playlist_cover_path(pid).exists()
     client.delete(f"/api/playlists/{pid}")
-    assert not _playlist_cover_path(pid).exists()
+    assert not server._playlist_cover_path(pid).exists()

tests/test_ws_highway_disconnect.py

@@ -1,110 +0,0 @@
-"""A client that disconnects mid-stream is routine, not a logged error.
-
-The highway WS streams ~15 message batches before its keep-alive loop. A
-disconnect during that streaming used to fall through to the blanket
-`except Exception` and log `highway_ws unhandled error`. The dedicated
-`except WebSocketDisconnect: return` makes it quiet.
-"""
-
-import asyncio
-import importlib
-import json
-import logging
-import sys
-
-import pytest
-import yaml
-from fastapi import WebSocketDisconnect
-
-
-def _write_sloppak(dlc_root):
-    pak = dlc_root / "disc.sloppak"
-    pak.mkdir(parents=True)
-    (pak / "arrangements").mkdir()
-    (pak / "arrangements" / "lead.json").write_text(json.dumps({
-        "notes": [], "chords": [], "anchors": [], "handshapes": [],
-        "templates": [], "beats": [{"time": 0.0, "measure": 1}],
-        "sections": [{"name": "intro", "number": 1, "time": 0.0}],
-    }))
-    (pak / "manifest.yaml").write_text(yaml.safe_dump({
-        "title": "Disc", "artist": "T", "album": "", "year": 2026, "duration": 10.0,
-        "arrangements": [{"id": "lead", "name": "Lead", "file": "arrangements/lead.json"}],
-        "stems": [],
-    }, sort_keys=False))
-    return pak
-
-
-class _DisconnectingWS:
-    """Accepts, then raises WebSocketDisconnect on the first streamed send —
-    i.e. a client that drops mid-stream."""
-    def __init__(self):
-        self.sends = 0
-
-    async def accept(self):
-        pass
-
-    async def send_json(self, data):
-        self.sends += 1
-        # Raise only on the FIRST send. If the handler wrongly kept streaming
-        # after catching the disconnect, later sends would succeed and `sends`
-        # would climb past 1 — the exactly-one assertion catches that.
-        if self.sends == 1:
-            raise WebSocketDisconnect(code=1001)
-
-    async def receive_text(self):
-        raise WebSocketDisconnect(code=1001)
-
-    async def close(self):
-        pass
-
-
-@pytest.fixture()
-def server(tmp_path, monkeypatch):
-    (tmp_path / "dlc").mkdir()
-    monkeypatch.setenv("CONFIG_DIR", str(tmp_path / "config"))
-    monkeypatch.setenv("DLC_DIR", str(tmp_path / "dlc"))
-    monkeypatch.setenv("FEEDBACK_SKIP_STARTUP_TASKS", "1")
-    sys.modules.pop("server", None)
-    mod = importlib.import_module("server")
-    yield mod, tmp_path / "dlc"
-    conn = getattr(getattr(mod, "meta_db", None), "conn", None)
-    if conn is not None:
-        getattr(mod, "_join_background_db_threads", lambda: None)()
-        conn.close()
-    sys.modules.pop("server", None)
-
-
-class _Capture(logging.Handler):
-    def __init__(self):
-        super().__init__()
-        self.messages = []
-
-    def emit(self, record):
-        self.messages.append(record.getMessage())
-
-
-def test_midstream_disconnect_is_not_logged_as_error(server):
-    """The first streamed send (`loading`) raises WebSocketDisconnect; the
-    handler must return quietly, not log `highway_ws unhandled error`.
-
-    A raw handler on `feedBack.server` is used rather than pytest's `caplog`
-    because `configure_logging()` (run at server import) reroutes that logger
-    through the structlog pipeline, which caplog's fixture doesn't observe.
-    """
-    _server, dlc = server
-    _write_sloppak(dlc)
-    from routers.ws_highway import highway_ws
-
-    cap = _Capture()
-    cap.setLevel(logging.ERROR)
-    lg = logging.getLogger("feedBack.server")
-    lg.addHandler(cap)
-    try:
-        ws = _DisconnectingWS()
-        asyncio.run(highway_ws(ws, "disc.sloppak", arrangement=0))  # must not raise
-    finally:
-        lg.removeHandler(cap)
-
-    assert ws.sends == 1, f"handler kept streaming after the disconnect ({ws.sends} sends)"
-    unhandled = [m for m in cap.messages if "highway_ws unhandled error" in m]
-    assert not unhandled, f"disconnect logged as unhandled error: {unhandled}"