fix(static): request raw stereo audio for the loopback capture track

Chromium treats a getDisplayMedia audio track as a voice call by
default: echo cancellation, noise suppression, auto gain control and
mono downmix. Music through that pipeline is the tester-reported
"tin can" sound on ASIO/exclusive outputs.

Request the raw path explicitly (EC/NS/AGC off, stereo, 48 kHz) —
all constraints are best-effort so unsupported ones degrade silently
instead of failing the capture. The [asio-diag] loopback line now
dumps track.getSettings() so logs prove which processing actually
applied.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

lib/appstate.py

@@ -117,6 +117,16 @@ invalidate_song_caches = None
 stat_for_cache = None
 scan_status = None
 
+# The directory containing server.py: the repo root in dev, resources/feedBack when
+# bundled — the tree that actually holds docs/ and data/.
+#
+# It is published HERE, by server.py, precisely so no module under lib/ ever computes it.
+# `Path(__file__).resolve().parent` is correct in server.py and silently WRONG anywhere in
+# lib/ (it yields lib/, which has no docs/ or data/), and it fails by finding nothing
+# rather than by raising — the builtin-content seeds would just quietly never run. See
+# lib/builtin_content.py's header. Read it; never re-derive it.
+server_root = None
+
 _SLOTS = frozenset({
     "meta_db", "audio_effect_mappings", "tuning_providers",
     "library_providers", "local_library_provider",
@@ -127,6 +137,7 @@ _SLOTS = frozenset({
     "art_cache_dir", "song_pack_art_exists", "art_override_paths", "art_safe_name",
     "default_settings",
     "kick_scan", "invalidate_song_caches", "stat_for_cache", "scan_status",
+    "server_root",
 })
 
 

lib/builtin_content.py

@@ -0,0 +1,378 @@
+"""Builtin content seeding: the calibration/diagnostic sloppaks and the starter library.
+
+Carved VERBATIM out of server.py (R3b) — with ONE deliberate signature change, and it is
+the whole reason this module is safe.
+
+━━━ WHY THE ROOT IS A PARAMETER ━━━
+
+server.py had `_feedBack_server_root()` = `Path(__file__).resolve().parent`. That is
+correct *in server.py*: the repo root in dev, resources/feedBack when bundled — the tree
+that actually holds docs/ and data/.
+
+Move that body here unchanged and it keeps working, silently, and returns `lib/`. There is
+no docs/diagnostics under lib/, so every seed would quietly find nothing and log "source
+missing" — a verbatim move whose meaning changed because `__file__` did. Nothing would
+fail; the starter library would just never appear.
+
+So this module CANNOT compute a root: it takes `server_root` as a parameter, and server.py
+— the only place that legitimately knows where it lives — passes it in. The trap is now
+structurally impossible rather than merely avoided. (_copy_builtin_packs already took the
+root this way; the two seed helpers now do too.)
+
+Everything else is byte-identical. `log` is this module's own logger under the same
+`feedBack.` hierarchy, and CONFIG_DIR is read late as `appstate.config_dir` — see appstate.py
+for why those reads must be late-bound (tests monkeypatch it).
+"""
+import logging
+import os
+import secrets
+import shutil
+import stat
+import tempfile
+from pathlib import Path
+
+import appstate
+from dlc_paths import _get_dlc_dir
+
+log = logging.getLogger("feedBack.builtin_content")
+
+
+BUILTIN_DIAGNOSTIC_SUBDIR = "diagnostics-builtin"
+
+
+BUILTIN_DIAGNOSTIC_SOURCES: list[tuple[str, str]] = [
+    (
+        "feedBack-diagnostic-basic-guitar.sloppak",
+        "docs/diagnostics/feedBack-diagnostic-basic-guitar.sloppak",
+    ),
+]
+
+
+def builtin_diagnostic_filename() -> str:
+    """Library filename (DLC-relative POSIX path) of the calibration sloppak —
+    the onboarding challenge target (spec 010)."""
+    return f"{BUILTIN_DIAGNOSTIC_SUBDIR}/{BUILTIN_DIAGNOSTIC_SOURCES[0][0]}"
+
+
+def _copy_builtin_packs(
+    root: Path,
+    dest_dir: Path,
+    sources: list[tuple[str, str]],
+    label: str,
+    update_existing: bool = True,
+) -> int:
+    """Symlink-safe, mtime-aware copy of bundled packs into ``dest_dir``.
+
+    ``sources`` is a list of ``(dest_name, rel_source)`` pairs; each source is
+    resolved under ``root`` (the repo root in dev, ``resources/feedBack`` when
+    bundled). A pack is copied when its destination is missing. Never deletes
+    user files; refuses to follow a symlinked seed directory or destination and
+    refuses to clobber a non-regular destination (any would let a copy escape
+    ``dest_dir`` or destroy user data). Logs and continues on error. ``label``
+    prefixes every log line.
+
+    ``update_existing`` controls what happens when a *regular* destination file
+    already exists: when True (diagnostic seed) a bundle copy newer than the
+    destination refreshes it; when False (one-time starter content) an existing
+    file is always left as-is so the user's copy is never overwritten.
+
+    Returns the number of ``sources`` that are present at their destination
+    afterwards (freshly seeded, refreshed, or already current) — so callers can
+    tell whether every pack made it. A skip (missing source, symlink/non-regular
+    refusal, copy error) does not count.
+    """
+    # Refuse a symlinked seed directory: mkdir(exist_ok=True) would accept it
+    # and copies would land at the link target, outside the DLC tree. The
+    # per-file symlink guard below cannot catch this.
+    if dest_dir.is_symlink():
+        log.warning("%s: %s is a symlink, skipping all seeding", label, dest_dir.name)
+        return 0
+    dest_dir.mkdir(parents=True, exist_ok=True)
+
+    # Pin the seed directory by an O_NOFOLLOW fd so a symlink swapped in for
+    # dest_dir *after* the check above cannot redirect the per-file stat /
+    # temp-create / replace outside the DLC tree (parent-directory TOCTOU).
+    # os.replace accepts dir_fd on POSIX even though it isn't listed in
+    # os.supports_dir_fd, so gate on os.rename (the reliable proxy); platforms
+    # without dir_fd/O_NOFOLLOW (e.g. Windows) fall back to path-based ops.
+    dir_fd = None
+    if (
+        hasattr(os, "O_NOFOLLOW")
+        and hasattr(os, "O_DIRECTORY")
+        and os.open in os.supports_dir_fd
+        and os.rename in os.supports_dir_fd
+    ):
+        try:
+            dir_fd = os.open(dest_dir, os.O_RDONLY | os.O_NOFOLLOW | os.O_DIRECTORY)
+        except OSError as exc:
+            log.warning("%s: cannot open seed dir %s: %s", label, dest_dir, exc)
+            return 0
+
+    try:
+        present = 0
+        for dest_name, rel_source in sources:
+            source = root / rel_source
+            if not source.is_file():
+                log.warning("%s: source missing, skipping %s (%s)", label, dest_name, source)
+                continue
+
+            # lstat the destination without following symlinks. Pinned by dir_fd
+            # this resolves within the real seed dir, immune to a parent swap.
+            try:
+                if dir_fd is not None:
+                    dstat = os.lstat(dest_name, dir_fd=dir_fd)
+                else:
+                    dstat = os.lstat(dest_dir / dest_name)
+                dest_exists = True
+                dest_islink = stat.S_ISLNK(dstat.st_mode)
+            except FileNotFoundError:
+                dest_exists = False
+                dest_islink = False
+            except OSError as exc:
+                log.warning("%s: cannot stat %s: %s", label, dest_name, exc)
+                continue
+
+            # Refuse to seed through a symlink at the destination name.
+            if dest_islink:
+                log.warning("%s: destination is a symlink, skipping %s", label, dest_name)
+                continue
+
+            # A non-regular destination (directory, fifo, …) the user placed
+            # there: never clobber it, and never count it as present — otherwise
+            # a one-time seed would mark itself done without a real pack on disk.
+            if dest_exists and not stat.S_ISREG(dstat.st_mode):
+                log.warning("%s: destination is not a regular file, skipping %s", label, dest_name)
+                continue
+
+            if dest_exists:
+                # A regular file is already there. One-time seeds (starter
+                # content) must never overwrite the user's copy; refreshing
+                # seeds (diagnostics) replace it only when the bundle is newer.
+                if not update_existing:
+                    log.info("%s: already present %s", label, dest_name)
+                    present += 1
+                    continue
+                try:
+                    src_mtime = source.stat().st_mtime
+                except OSError as exc:
+                    log.warning("%s: cannot stat source %s: %s", label, source, exc)
+                    continue
+                if src_mtime <= dstat.st_mtime:
+                    log.info("%s: already present %s", label, dest_name)
+                    present += 1
+                    continue
+                action = "updated"
+            else:
+                action = "seeded"
+
+            if _write_builtin_pack(source, dest_dir, dest_name, dir_fd):
+                present += 1
+                log.info("%s: %s %s -> %s", label, action, source.name, dest_name)
+            else:
+                log.warning("%s: failed to copy %s -> %s/%s", label, source, dest_dir.name, dest_name)
+
+        return present
+    finally:
+        if dir_fd is not None:
+            os.close(dir_fd)
+
+
+def _write_builtin_pack(
+    source: Path,
+    dest_dir: Path,
+    dest_name: str,
+    dir_fd: int | None,
+) -> bool:
+    """Atomically write ``source`` to ``dest_name`` inside ``dest_dir``.
+
+    Writes to a temp file then ``os.replace()``s onto the final name so a
+    symlink raced in at the destination is overwritten (rename semantics), not
+    followed, and a crash never leaves a half-written pack. When ``dir_fd`` is
+    given, every step is anchored to that fd (O_NOFOLLOW temp create + dir_fd
+    replace), closing the parent-directory TOCTOU; otherwise falls back to
+    path-based temp+replace. Returns True on success. Never raises.
+    """
+    # Unique per-attempt name (O_EXCL create) so a crash that orphans a temp
+    # can't permanently block later seeds via an EEXIST collision.
+    tmp_name = f".seed-{dest_name}.{os.getpid()}.{secrets.token_hex(4)}.tmp"
+    try:
+        src_stat = source.stat()
+    except OSError as exc:
+        log.debug("builtin pack: cannot stat source %s: %s", source, exc)
+        return False
+    if dir_fd is not None:
+        tmp_fd = None
+        try:
+            tmp_fd = os.open(
+                tmp_name,
+                os.O_CREAT | os.O_EXCL | os.O_WRONLY | os.O_NOFOLLOW,
+                0o644,
+                dir_fd=dir_fd,
+            )
+            with open(source, "rb") as sf, os.fdopen(tmp_fd, "wb") as tf:
+                tmp_fd = None  # fdopen now owns the descriptor
+                shutil.copyfileobj(sf, tf)
+            os.replace(tmp_name, dest_name, src_dir_fd=dir_fd, dst_dir_fd=dir_fd)
+            # Preserve the bundle mtime (copyfileobj doesn't) so the mtime-based
+            # refresh check matches the shutil.copy2 fallback path. Best-effort.
+            try:
+                os.utime(
+                    dest_name,
+                    ns=(src_stat.st_atime_ns, src_stat.st_mtime_ns),
+                    dir_fd=dir_fd,
+                    follow_symlinks=False,
+                )
+            except OSError as exc:
+                log.debug("builtin pack: could not set mtime on %s: %s", dest_name, exc)
+            return True
+        except OSError as exc:
+            log.debug("builtin pack write (dir_fd) failed for %s: %s", dest_name, exc)
+            if tmp_fd is not None:
+                try:
+                    os.close(tmp_fd)
+                except OSError:
+                    pass
+            try:
+                os.unlink(tmp_name, dir_fd=dir_fd)
+            except OSError:
+                pass
+            return False
+
+    tmp = None
+    try:
+        fd, tmp = tempfile.mkstemp(dir=dest_dir, prefix=".seed-", suffix=".tmp")
+        os.close(fd)
+        shutil.copy2(source, tmp)
+        os.replace(tmp, dest_dir / dest_name)
+        tmp = None
+        return True
+    except OSError as exc:
+        log.debug("builtin pack write failed for %s: %s", dest_name, exc)
+        return False
+    finally:
+        if tmp is not None:
+            try:
+                os.unlink(tmp)
+            except OSError:
+                pass
+
+
+def seed_builtin_diagnostic_sloppaks(server_root: Path, dlc: Path | None = None) -> None:
+    """Copy bundled diagnostic sloppaks into DLC before library scan.
+
+    Creates ``DLC_DIR/diagnostics-builtin/`` and copies each bundled sloppak
+    when the destination is missing or older than the repo/bundle source.
+    Never deletes user files or touches manually copied paths (e.g.
+    ``diagnostics-test/``). Re-seeds whenever the destination is missing so the
+    diagnostic target is always available. Logs and continues on errors.
+    """
+    try:
+        if dlc is None:
+            dlc = _get_dlc_dir()
+        if dlc is None:
+            log.debug("Builtin diagnostic seed: no DLC folder configured, skipping")
+            return
+        _copy_builtin_packs(
+            server_root,
+            dlc / BUILTIN_DIAGNOSTIC_SUBDIR,
+            BUILTIN_DIAGNOSTIC_SOURCES,
+            "Builtin diagnostic seed",
+        )
+    except Exception:
+        log.warning("Builtin diagnostic seed: unexpected error", exc_info=True)
+
+
+# Starter content: bundled songs copied into ``DLC_DIR/starter/`` exactly ONCE,
+# on first run, as a welcome library so a fresh install isn't empty. Unlike the
+# diagnostic seed this is one-time — guarded by a marker in CONFIG_DIR — so if
+# the user deletes the starter song it stays gone. ``starter/`` is NOT in the
+# library scan carve-out (unlike diagnostics-builtin/ / tutorials-builtin/), so
+# seeded packs surface as ordinary library songs.
+BUILTIN_STARTER_SUBDIR = "starter"
+
+
+BUILTIN_STARTER_SOURCES: list[tuple[str, str]] = [
+    (
+        "beethoven-fur_elise.feedpak",
+        "content/starter/beethoven-fur_elise.feedpak",
+    ),
+    (
+        "star_spangled_banner.feedpak",
+        "content/starter/star_spangled_banner.feedpak",
+    ),
+    (
+        "the_adicts-ode-to-joy_vst_cover.feedpak",
+        "content/starter/the_adicts-ode-to-joy_vst_cover.feedpak",
+    ),
+]
+
+
+STARTER_SEED_MARKER = ".starter-content-seeded"
+
+
+def seed_builtin_starter_content(server_root: Path, dlc: Path | None = None) -> None:
+    """Copy bundled starter songs into ``DLC_DIR/starter/`` exactly once.
+
+    Guarded by ``CONFIG_DIR/.starter-content-seeded``: the first run with a DLC
+    folder configured seeds the packs and writes the marker; subsequent runs are
+    no-ops, so a user who deletes the starter song does not get it back on the
+    next launch. Symlink-safe; never deletes user files. Logs, never raises.
+    """
+    try:
+        marker = appstate.config_dir / STARTER_SEED_MARKER
+        # Already seeded? The marker is a sentinel: any existing path there
+        # (regular file, or a symlink/dir a user deliberately planted to opt
+        # out) means "done" — lstat so we detect it without following a symlink.
+        # Worst case of a planted marker is simply no starter content, never a
+        # data write; the O_EXCL|O_NOFOLLOW create below refuses to write
+        # *through* a symlink regardless.
+        try:
+            os.lstat(marker)
+            return
+        except FileNotFoundError:
+            pass
+        except OSError as exc:
+            log.warning("Starter content seed: cannot stat marker %s: %s", marker, exc)
+            return
+        if dlc is None:
+            dlc = _get_dlc_dir()
+        if dlc is None:
+            # No DLC yet — leave the marker unwritten so we retry once a
+            # library folder is configured.
+            log.debug("Starter content seed: no DLC folder configured, skipping")
+            return
+        present = _copy_builtin_packs(
+            server_root,
+            dlc / BUILTIN_STARTER_SUBDIR,
+            BUILTIN_STARTER_SOURCES,
+            "Starter content seed",
+            update_existing=False,
+        )
+        # Only mark seeding complete once every starter pack is actually in
+        # place. If a source was missing or a copy failed, leave the marker
+        # unwritten so the next launch retries rather than permanently skipping.
+        if present < len(BUILTIN_STARTER_SOURCES):
+            log.info(
+                "Starter content seed: %d/%d packs present, will retry next launch",
+                present,
+                len(BUILTIN_STARTER_SOURCES),
+            )
+            return
+        # Record completion with an exclusive, no-follow create so a planted or
+        # raced symlink at the marker path can't redirect the write outside
+        # CONFIG_DIR. O_EXCL fails (EEXIST) on any existing path including a
+        # symlink, so we never write through one.
+        try:
+            appstate.config_dir.mkdir(parents=True, exist_ok=True)
+            flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY | getattr(os, "O_NOFOLLOW", 0)
+            fd = os.open(marker, flags, 0o644)
+            try:
+                os.write(fd, b"1\n")
+            finally:
+                os.close(fd)
+        except FileExistsError:
+            pass  # already marked (or a non-regular path is squatting) — fine
+        except OSError as exc:
+            log.warning("Starter content seed: could not write marker %s: %s", marker, exc)
+    except Exception:
+        log.warning("Starter content seed: unexpected error", exc_info=True)

lib/demo_mode.py

@@ -0,0 +1,342 @@
+"""Demo mode: the read-only request guard and the hourly session janitor.
+
+Carved VERBATIM out of server.py (R3b). Bodies are byte-identical — including a bug, see
+below.
+
+━━━ THE MIDDLEWARE NEEDS `app`, SO THIS MODULE TAKES IT ━━━
+
+`_demo_mode_guard` is an @app.middleware("http"), and a middleware has to be attached to an
+app object. Rather than reach for a global, this module exposes install(app): server.py
+owns the app and hands it over. Same direction as every other seam here — server.py knows
+things lib/ must not have to guess.
+
+The janitor is symmetrical: start_janitor() / stop_janitor(), called from server.py's
+startup and shutdown hooks, which is where the process lifecycle actually lives.
+
+━━━ register_demo_janitor_hook IS PART OF THE PLUGIN CONTRACT ━━━
+
+It is a key in plugin_context, so plugins hold it as a LIVE REFERENCE from setup(). Moving
+the function is fine; wrapping or renaming it is not. server.py imports this exact object
+and puts it in the dict unchanged, so callable identity is preserved —
+tests/test_plugin_context_contract.py (#898) fails if that ever stops being true.
+
+━━━ A BUG MOVED VERBATIM, ON PURPOSE ━━━
+
+The janitor start guard in server.py reads:
+
+    if getenv_compat("FEEDBACK_DEMO_MODE") or getenv_compat("FEEDBACK_DEMO_MODE") == "1" \
+            and not _DEMO_JANITOR_STARTED:
+
+`and` binds tighter than `or`, so that is `A or (B and C)` — the `not _DEMO_JANITOR_STARTED`
+re-entry guard is DEAD whenever the env var is truthy, which is the only case that runs. A
+second startup leaks a janitor thread (the handle is overwritten, so shutdown joins only
+the last). Preserved exactly as-is here and filed as issue #902: a carve whose value is
+being provably behaviour-neutral is not the place to change behaviour.
+"""
+import inspect
+import logging
+import re
+import threading
+import uuid
+import warnings
+
+from fastapi import Request
+from fastapi.responses import JSONResponse
+from env_compat import getenv_compat
+
+log = logging.getLogger("feedBack.demo_mode")
+
+
+# Plugins that maintain session stores can register a cleanup callback here.
+# The demo-mode janitor calls every registered hook once per hour so stale
+# sessions are swept without the core needing to know plugin internals.
+_DEMO_JANITOR_HOOKS: list = []
+
+
+_DEMO_JANITOR_HOOKS_LOCK = threading.Lock()
+
+
+_DEMO_JANITOR_STARTED = False
+
+
+_DEMO_JANITOR_STOP = threading.Event()
+
+
+_DEMO_JANITOR_THREAD: threading.Thread | None = None
+
+
+def register_demo_janitor_hook(fn) -> None:
+    """Register a zero-argument callable to be invoked hourly by the demo
+    janitor.  Plugins call this from their ``setup(app, context)`` when they
+    want to participate in session cleanup under demo mode.
+
+    The callable must accept no required arguments.  Async (coroutine)
+    functions are rejected: the janitor runs in a plain thread and cannot
+    await coroutines.
+    """
+    if not callable(fn):
+        raise TypeError(
+            f"register_demo_janitor_hook expects a callable, got {type(fn).__name__!r}"
+        )
+    # Reject coroutine functions — check both the callable itself and its
+    # __call__ method so objects with an async __call__ (e.g. class instances,
+    # functools.partial wrappers around async functions) are also caught.
+    _call = getattr(fn, "__call__", None)
+    if inspect.iscoroutinefunction(fn) or (
+        _call is not None and inspect.iscoroutinefunction(_call)
+    ):
+        raise TypeError(
+            "register_demo_janitor_hook does not accept async functions; "
+            "the janitor runs in a plain thread and cannot await coroutines"
+        )
+    # Validate that the callable accepts zero required arguments so it won't
+    # crash at sweep time (hourly, far from the registration site).
+    try:
+        sig = inspect.signature(fn)
+    except ValueError:
+        # inspect.signature() raises ValueError for built-in C callables whose
+        # signature cannot be determined.  Accept them as-is; if they fail at
+        # runtime the janitor will catch and log the exception.
+        pass
+    else:
+        required = [
+            p for p in sig.parameters.values()
+            if p.default is inspect.Parameter.empty
+            and p.kind not in (
+                inspect.Parameter.VAR_POSITIONAL,
+                inspect.Parameter.VAR_KEYWORD,
+            )
+        ]
+        if required:
+            raise TypeError(
+                f"register_demo_janitor_hook expects a zero-argument callable; "
+                f"{fn!r} has {len(required)} required parameter(s): "
+                + ", ".join(p.name for p in required)
+            )
+    with _DEMO_JANITOR_HOOKS_LOCK:
+        _DEMO_JANITOR_HOOKS.append(fn)
+
+
+def _run_janitor_hook(hook) -> None:
+    """Run a single janitor hook inline, swallowing and logging any exception.
+
+    If the hook returns an awaitable (e.g. a coroutine slipped through the
+    async-function guard), the coroutine is closed immediately to avoid
+    ``RuntimeWarning: coroutine was never awaited`` noise, and a warning is
+    emitted so the plugin author knows to fix their hook.
+    """
+    try:
+        result = hook()
+    except Exception:
+        log.exception("janitor hook %r raised", hook)
+        return
+    if inspect.iscoroutine(result):
+        # A coroutine slipped through the async-function guard (e.g. via a
+        # wrapper/partial).  Close it to suppress "coroutine never awaited",
+        # then warn so the plugin author knows to fix their hook.
+        try:
+            result.close()
+        except Exception:
+            log.exception("error closing coroutine from janitor hook %r", hook)
+        warnings.warn(
+            f"janitor hook {hook!r} returned a coroutine; "
+            "hooks must be plain synchronous callables — "
+            "register_demo_janitor_hook does not accept async functions",
+            RuntimeWarning,
+            stacklevel=1,
+        )
+    elif inspect.isawaitable(result):
+        # Future/Task: no .close() method; just warn and leave it alone.
+        warnings.warn(
+            f"janitor hook {hook!r} returned an awaitable (Future/Task); "
+            "hooks must be plain synchronous callables",
+            RuntimeWarning,
+            stacklevel=1,
+        )
+
+
+_DEMO_BLOCKED: list[tuple[str, re.Pattern]] = [
+    ("POST",   re.compile(r"^/api/settings$")),
+    ("POST",   re.compile(r"^/api/settings/import$")),
+    ("POST",   re.compile(r"^/api/settings/reset$")),
+    ("POST",   re.compile(r"^/api/rescan$")),
+    ("POST",   re.compile(r"^/api/rescan/full$")),
+    ("POST",   re.compile(r"^/api/songs/upload$")),
+    ("DELETE", re.compile(r"^/api/song/.+$")),
+    ("POST",   re.compile(r"^/api/favorites/toggle$")),
+    ("POST",   re.compile(r"^/api/loops$")),
+    ("DELETE", re.compile(r"^/api/loops/[^/]+$")),
+    ("POST",   re.compile(r"^/api/audio-effects/mappings$")),
+    ("DELETE", re.compile(r"^/api/audio-effects/mappings/[^/]+$")),
+    ("POST",   re.compile(r"^/api/audio-effects/mappings/[^/]+/activate$")),
+    ("DELETE", re.compile(r"^/api/audio-effects/active-mapping$")),
+    ("POST",   re.compile(r"^/api/song/.*/meta$")),
+    ("POST",   re.compile(r"^/api/song/.*/art/upload$")),
+    ("PUT",    re.compile(r"^/api/song/.+/overrides$")),
+    ("GET",    re.compile(r"^/api/plugins/updates$")),
+    ("POST",   re.compile(r"^/api/plugins/[^/]+/update$")),
+    ("POST",   re.compile(r"^/api/plugins/editor/save$")),
+    ("POST",   re.compile(r"^/api/plugins/editor/build$")),
+    ("POST",   re.compile(r"^/api/plugins/editor/upload-art$")),
+    ("POST",   re.compile(r"^/api/plugins/editor/upload-audio$")),
+    ("POST",   re.compile(r"^/api/plugins/editor/youtube-audio$")),
+    ("POST",   re.compile(r"^/api/plugins/editor/import-gp$")),
+    ("POST",   re.compile(r"^/api/plugins/editor/import-midi$")),
+    ("POST",   re.compile(r"^/api/plugins/lyrics_karaoke/align$")),
+    ("POST",   re.compile(r"^/api/plugins/lyrics_karaoke/generate-pitch$")),
+    ("POST",   re.compile(r"^/api/plugins/lyrics_karaoke/save-lyrics$")),
+    ("POST",   re.compile(r"^/api/plugins/lyrics_sync/align$")),
+    ("POST",   re.compile(r"^/api/plugins/lyrics_sync/save$")),
+    ("POST",   re.compile(r"^/api/plugins/studio/sessions/[^/]+/extract-drums$")),
+    ("POST",   re.compile(r"^/api/diagnostics/export$")),
+    ("GET",    re.compile(r"^/api/diagnostics/preview$")),
+    ("GET",    re.compile(r"^/api/diagnostics/hardware$")),
+    # Bundled core plugin — video background upload/delete
+    ("POST",   re.compile(r"^/api/plugins/highway_3d/files$")),
+    ("DELETE", re.compile(r"^/api/plugins/highway_3d/files$")),
+    # fee[dB]ack v0.3.0 write endpoints — demo mode is read-only, so block the
+    # new profile / XP / stats / playlists / saved mutators too.
+    ("POST",   re.compile(r"^/api/profile$")),
+    ("POST",   re.compile(r"^/api/profile/avatar$")),
+    ("POST",   re.compile(r"^/api/xp/award$")),
+    ("POST",   re.compile(r"^/api/stats$")),
+    ("POST",   re.compile(r"^/api/playlists$")),
+    ("PATCH",  re.compile(r"^/api/playlists/[^/]+$")),
+    ("DELETE", re.compile(r"^/api/playlists/[^/]+$")),
+    ("POST",   re.compile(r"^/api/playlists/[^/]+/songs$")),
+    ("DELETE", re.compile(r"^/api/playlists/[^/]+/songs/.+$")),
+    ("POST",   re.compile(r"^/api/playlists/[^/]+/reorder$")),
+    ("POST",   re.compile(r"^/api/playlists/[^/]+/cover$")),
+    ("DELETE", re.compile(r"^/api/playlists/[^/]+/cover$")),
+    ("POST",   re.compile(r"^/api/saved/toggle$")),
+    # Progression (spec 010) write endpoints — demo mode stays read-only.
+    ("POST",   re.compile(r"^/api/progression/paths$")),
+    ("POST",   re.compile(r"^/api/progression/onboarding$")),
+    ("POST",   re.compile(r"^/api/progression/events$")),
+    ("POST",   re.compile(r"^/api/shop/buy$")),
+    ("POST",   re.compile(r"^/api/shop/equip$")),
+    # Enrichment (P8): review writes mutate the local match cache, and the
+    # search proxy / manual kick relay to MusicBrainz — none of it belongs to
+    # anonymous demo visitors (they'd spend the shared rate limit).
+    ("POST",   re.compile(r"^/api/enrichment/review/.+$")),
+    ("POST",   re.compile(r"^/api/enrichment/kick$")),
+    ("POST",   re.compile(r"^/api/enrichment/cancel$")),
+    ("POST",   re.compile(r"^/api/enrichment/rematch$")),
+    ("GET",    re.compile(r"^/api/enrichment/search$")),
+    # AcoustID audio fingerprinting: both identify endpoints run fpcalc (CPU)
+    # and spend the shared AcoustID rate budget on the caller's behalf — same
+    # rule as the search/kick relays above; not for anonymous demo visitors.
+    ("POST",   re.compile(r"^/api/enrichment/identify$")),
+    ("POST",   re.compile(r"^/api/enrichment/identify/.+$")),
+    # Context menus (R2): the per-song re-match mutates the cache + spends
+    # rate limit; Get-info exposes filesystem paths.
+    ("POST",   re.compile(r"^/api/enrichment/refresh/.+$")),
+    ("GET",    re.compile(r"^/api/chart/.+/fileinfo$")),
+    # Gap-fill (R4a) rewrites pack files on disk — never for demo visitors.
+    ("POST",   re.compile(r"^/api/song/.+/gap-fill$")),
+    # Art layer (R3): all three mutate server state / touch the network on a
+    # visitor's behalf — the base64 upload writes files, the URL fetch makes the
+    # server request arbitrary images, and the override delete removes files.
+    ("POST",   re.compile(r"^/api/song/.+/art/upload$")),
+    ("POST",   re.compile(r"^/api/song/.+/art/url$")),
+    ("DELETE", re.compile(r"^/api/art/.+/override$")),
+    # Cover picker (PR-C): read-only, but a cache-miss open spends 1-3
+    # throttled Cover Art Archive calls — anonymous demo visitors don't get
+    # to spend the shared rate budget (same rule as enrichment search/kick).
+    ("GET",    re.compile(r"^/api/song/.+/art/candidates$")),
+    # Artist pages (PR-B): the links GET lazily fetches from MusicBrainz on a
+    # visitor's behalf AND writes the artist_enrichment cache; refresh
+    # re-spends the shared rate limit. The /page route stays open (all-local
+    # read). Same rationale as /api/enrichment/search above.
+    ("GET",    re.compile(r"^/api/artist/.+/links$")),
+    ("POST",   re.compile(r"^/api/artist/.+/links/refresh$")),
+]
+
+
+async def _demo_mode_guard(request: Request, call_next):
+    if getenv_compat("FEEDBACK_DEMO_MODE") or getenv_compat("FEEDBACK_DEMO_MODE") == "1":
+        path = request.url.path
+        for method, pattern in _DEMO_BLOCKED:
+            if request.method == method and pattern.match(path):
+                return JSONResponse({"error": "demo mode: read-only"}, status_code=403)
+        response = await call_next(request)
+        if request.method == "GET" and path == "/" and "feedBack_demo_session" not in request.cookies:
+            forwarded_proto = (request.headers.get("x-forwarded-proto") or "").split(",")[0].strip()
+            is_secure = request.url.scheme == "https" or forwarded_proto.lower() == "https"
+            response.set_cookie(
+                "feedBack_demo_session", str(uuid.uuid4()),
+                max_age=86400, httponly=True, samesite="lax",
+                secure=is_secure,
+            )
+        return response
+    return await call_next(request)
+
+
+def install(app) -> None:
+    """Attach the demo-mode request guard to `app`.
+
+    Called by server.py, which owns the app. A middleware cannot exist without one, and a
+    module under lib/ should not be reaching for a global to find it.
+    """
+    app.middleware("http")(_demo_mode_guard)
+
+
+def demo_mode_enabled() -> bool:
+    """True when demo mode is on. Read at CALL time, never captured — tests set and unset
+    FEEDBACK_DEMO_MODE with monkeypatch, so a value cached at import pins the wrong one."""
+    return bool(getenv_compat("FEEDBACK_DEMO_MODE"))
+
+
+def start_janitor() -> None:
+    """Start the hourly session janitor. Called from server.py's startup hook.
+
+    NB the caller's guard is the buggy one described in this module's header (issue #902).
+    Behaviour is preserved verbatim: this starts a thread every time it is called.
+    """
+    global _DEMO_JANITOR_STARTED, _DEMO_JANITOR_THREAD
+    _DEMO_JANITOR_STARTED = True
+    _DEMO_JANITOR_STOP.clear()
+
+    def _janitor():
+        while not _DEMO_JANITOR_STOP.wait(timeout=3600):
+            with _DEMO_JANITOR_HOOKS_LOCK:
+                hooks = list(_DEMO_JANITOR_HOOKS)
+            for hook in hooks:
+                _run_janitor_hook(hook)
+
+    _DEMO_JANITOR_THREAD = threading.Thread(target=_janitor, daemon=True, name="demo-janitor")
+    _DEMO_JANITOR_THREAD.start()
+
+
+def janitor_started() -> bool:
+    return _DEMO_JANITOR_STARTED
+
+
+def stop_janitor(timeout: float = 5) -> bool:
+    """Signal the janitor to stop, join it, and drop the registered hooks.
+
+    Returns True if it stopped, False if it outlived the join (the caller warns).
+
+    THE ORDER HERE IS LOAD-BEARING and preserved exactly from server.py. When the thread
+    does NOT die within the timeout we return WITHOUT clearing _DEMO_JANITOR_STARTED and
+    WITHOUT dropping the thread handle — deliberately — so a subsequent startup does not
+    spawn a SECOND janitor alongside the one still running. Clearing the flag first (the
+    obvious way to write this) would quietly reintroduce exactly the double-janitor leak
+    the flag exists to prevent.
+    """
+    global _DEMO_JANITOR_STARTED, _DEMO_JANITOR_THREAD
+    if not _DEMO_JANITOR_STARTED:
+        return True
+    _DEMO_JANITOR_STOP.set()
+    thread = _DEMO_JANITOR_THREAD
+    if thread is not None:
+        thread.join(timeout=timeout)
+        if thread.is_alive():
+            # Leave _DEMO_JANITOR_STARTED True so a new janitor is not spawned by a
+            # subsequent startup while the old one is alive.
+            return False
+        _DEMO_JANITOR_THREAD = None
+    _DEMO_JANITOR_STARTED = False
+    with _DEMO_JANITOR_HOOKS_LOCK:
+        _DEMO_JANITOR_HOOKS.clear()
+    return True

lib/scan.py

@@ -0,0 +1,326 @@
+"""The library scanner: the background scan, its process pool, and the kick/runner
+plumbing that serialises passes.
+
+Carved VERBATIM out of server.py (R3b) except the seam reads. Everything shared is read
+LATE off appstate — the same contract every module in lib/routers/ uses, and it is not
+cosmetic: tests monkeypatch CONFIG_DIR and swap meta_db, so a value captured at import
+time would pin the wrong one for the life of the process.
+
+    CONFIG_DIR        -> appstate.config_dir
+    meta_db           -> appstate.meta_db
+    _default_settings -> appstate.default_settings()
+    _stat_for_cache   -> appstate.stat_for_cache()
+    _feedBack_server_root() -> appstate.server_root      <- see below
+
+━━━ THE SCAN STATUS IS REBOUND, NOT MUTATED ━━━
+
+`_background_scan` does `global _scan_status; _scan_status = {**INIT, ...}` at every stage
+transition. It REPLACES the dict; it does not update it in place. So nothing may hold the
+dict by value — a reference captured once goes permanently stale at the first stage change,
+and would report "listing" forever while the scan ran to completion.
+
+That is why this module exports `status()`, a getter, and why appstate publishes
+`scan_status` as a CALLABLE rather than a dict. appstate.py already says so in a comment;
+this is the code that makes it true.
+
+━━━ AND WHY THE SERVER ROOT IS READ, NEVER DERIVED ━━━
+
+`_background_scan` seeds the builtin content, which needs the directory holding server.py.
+`Path(__file__).resolve().parent` is correct in server.py and silently WRONG here (it
+yields lib/, which has no docs/ or data/) — and it fails by finding nothing rather than by
+raising, so the seeds would just quietly never run. server.py publishes the root once, as
+appstate.server_root. Read it; never re-derive it.
+"""
+import concurrent.futures
+import logging
+import multiprocessing
+import os
+import sys
+import threading
+from pathlib import Path
+
+import appstate
+import builtin_content
+import enrichment
+import loosefolder as loosefolder_mod
+import sloppak as sloppak_mod
+from appconfig import _load_config
+from dlc_paths import _get_dlc_dir
+from env_compat import getenv_compat
+from scan_worker import _relpath, _scan_one
+
+log = logging.getLogger("feedBack.scan")
+
+
+_SCAN_STATUS_INIT = {"running": False, "stage": "idle", "total": 0, "done": 0, "current": "", "error": None, "is_first_scan": False, "added": 0, "removed": 0}
+
+
+_scan_status = dict(_SCAN_STATUS_INIT)
+
+
+def _make_scan_executor():
+    """Build the executor for the background metadata scan.
+
+    A `spawn` ProcessPoolExecutor in production. `spawn` (not the platform
+    default) is mandatory: _background_scan runs on a non-main daemon
+    thread, and forking a multithreaded process from a non-main thread can
+    deadlock on locks held by other threads at fork time (the default on
+    Linux). `spawn` boots a clean interpreter that imports only scan_worker
+    (+ its pure lib deps) to unpickle the worker — never this module — so
+    workers don't re-run server.py's import-time side effects (reopening
+    SQLite, attaching a second RotatingFileHandler, re-registering routes).
+
+    Tests monkeypatch this to a ThreadPoolExecutor so the scan runs
+    in-process and metadata extraction can be mocked.
+    """
+    mp_ctx = multiprocessing.get_context("spawn")
+    # Default to one worker per core so CPU-bound metadata parsing uses the
+    # whole machine (the point of moving to processes).
+    # FEEDBACK_MAX_SCAN_WORKERS (set by the Desktop launcher to cap memory
+    # usage on low-RAM machines — e.g. 8 GB M2 MacBook Air) takes priority;
+    # SCAN_MAX_WORKERS is a legacy override for Docker/bare installs.
+    # A malformed override falls back to the core count rather than crashing.
+    try:
+        max_workers = int(
+            getenv_compat("FEEDBACK_MAX_SCAN_WORKERS")
+            or os.environ.get("SCAN_MAX_WORKERS")
+            or (os.cpu_count() or 1)
+        )
+    except ValueError:
+        max_workers = os.cpu_count() or 1
+    # ProcessPoolExecutor raises ValueError on Windows when max_workers > 61
+    # (the WaitForMultipleObjects handle limit), so clamp there — otherwise
+    # a high-core Windows host can't construct the pool and the scan never
+    # starts.
+    if sys.platform == "win32":
+        max_workers = min(max_workers, 61)
+    return concurrent.futures.ProcessPoolExecutor(
+        max_workers=max(1, max_workers), mp_context=mp_ctx,
+    )
+
+
+def background_scan():
+    """Scan the library and cache song metadata on startup. Uses a process pool to bypass the GIL for CPU-bound metadata parsing.
+
+    Never sets `_scan_status["running"] = False` — ownership of that flag
+    lives in `_scan_runner` so a `kick_scan()` racing this function's
+    terminal write cannot observe a stale False and start a second runner.
+    """
+    global _scan_status
+    _scan_status = {**_SCAN_STATUS_INIT, "running": True, "stage": "listing"}
+
+    # Load config once so both the DLC-dir lookup and the platform filter
+    # read from the same snapshot, avoiding a redundant parse of config.json.
+    _cfg = _load_config(appstate.config_dir / "config.json") or appstate.default_settings()
+    dlc = _get_dlc_dir(_cfg)
+    if not dlc:
+        _scan_status = {**_SCAN_STATUS_INIT, "running": True, "stage": "idle", "error": "DLC folder not configured"}
+        log.warning("Scan: no DLC folder configured")
+        return
+
+    builtin_content.seed_builtin_diagnostic_sloppaks(appstate.server_root, dlc)
+    builtin_content.seed_builtin_starter_content(appstate.server_root, dlc)
+
+    # Listing can fail on macOS without Full Disk Access, or on Docker if the
+    # path isn't shared. Report the failure explicitly rather than silently
+    # appearing to scan nothing.
+    try:
+        # Generated-content sloppaks that the highway WS must resolve by path
+        # but that are NOT library songs. Two conventions share this carve-out:
+        #   - tutorials-builtin/  — lesson drills seeded by the tutorials plugin
+        #     (see plugins/tutorials/routes.py::_seed_builtin_packs).
+        #   - minigames-builtin/  — exercise charts generated on demand by
+        #     minigame plugins (e.g. Chord Sprint writes alternating-chord
+        #     drills here). Cached/reused per exercise, never browsed.
+        # Both are kept out of the scan; _resolve_dlc_path still loads them by
+        # path for playback.
+        def _is_excluded_from_library(p: Path) -> bool:
+            return "tutorials-builtin" in p.parts or "minigames-builtin" in p.parts
+        # Sloppaks: match both file (zip) and directory form, across both the
+        # `.feedpak` and legacy `.sloppak` suffixes.
+        _cands = sorted(p for ext in sloppak_mod.SONG_EXTS for p in dlc.rglob(f"*{ext}"))
+        sloppaks = [f for f in _cands
+                    if sloppak_mod.is_sloppak(f)
+                    and not _is_excluded_from_library(f)]
+
+        # Loose song folders: any directory containing a non-preview *.wem + *.xml.
+        # Skip directories that are actually sloppak bundles — those are
+        # already in `sloppaks`; the dispatcher's sloppak-first precedence
+        # would route them to the sloppak path anyway, but adding them
+        # here would inflate the scan queue and over-count the total.
+        loose_songs = []
+        seen_loose = set()
+        sloppak_dirs = {p for p in sloppaks if p.is_dir()}
+        for wem in sorted(dlc.rglob("*.wem")):
+            if "preview" in wem.stem.lower():
+                continue
+            if _is_excluded_from_library(wem):
+                continue
+            d = wem.parent
+            if d in sloppak_dirs or d.name.lower().endswith(sloppak_mod.SONG_EXTS):
+                continue
+            if d not in seen_loose and loosefolder_mod.is_loose_song(d):
+                loose_songs.append(d)
+                seen_loose.add(d)
+    except PermissionError as e:
+        msg = (f"Permission denied reading {dlc}. "
+               "On macOS: grant Full Disk Access to the app in System Settings → Privacy & Security. "
+               "With Docker: share this path in Docker Desktop → Settings → Resources → File Sharing.")
+        log.error("Scan failed: %s (%s)", msg, e)
+        _scan_status = {**_SCAN_STATUS_INIT, "running": True, "stage": "error", "error": msg}
+        return
+    except OSError as e:
+        log.error("Scan failed listing %s: %s", dlc, e)
+        _scan_status = {**_SCAN_STATUS_INIT, "running": True, "stage": "error", "error": f"Unable to list {dlc}: {e}"}
+        return
+
+    all_songs = sloppaks + loose_songs
+    log.info("Scan: listed %d sloppaks and %d loose folders in %s",
+             len(sloppaks), len(loose_songs), dlc)
+
+    current_files = {_relpath(f, dlc) for f in all_songs}
+
+    # Clean up stale DB entries. delete_missing reports both deltas (rows pruned
+    # + genuinely-new files) so the scan can surface an added/removed summary.
+    _delta = appstate.meta_db.delete_missing(current_files)
+    removed, added = _delta["removed"], _delta["added"]
+    if removed:
+        log.info("Removed %d stale DB entries", removed)
+
+    # Figure out which need scanning
+    to_scan = []
+    for f in all_songs:
+        # Skip entries that vanish or become unreadable between listing
+        # and stat. Without this, one concurrent move/delete in DLC_DIR
+        # would crash the scan thread and leave `_scan_status["running"]`
+        # stuck true with no path to recover.
+        try:
+            mtime, size = appstate.stat_for_cache(f)
+        except OSError as e:
+            log.debug("scan: skipping %s (%s)", f, e)
+            continue
+        cache_key = _relpath(f, dlc)
+        try:
+            cached = appstate.meta_db.get(cache_key, mtime, size)
+        except Exception as e:
+            # Keep scanning even if a single metadata lookup fails.
+            # The file will be re-scanned and cache repaired by put().
+            log.warning("scan cache lookup failed for %s: %s", cache_key, e)
+            cached = None
+        if not cached:
+            to_scan.append((f, mtime, size, dlc))
+        elif cached.get("arrangements") and any(
+            "smart_name" not in a for a in cached["arrangements"]
+        ):
+            # Row was scanned before smart naming was introduced — force a
+            # rescan so the DB picks up authoritative path flags from the
+            # manifest JSON and stores correct smart_name values. Don't
+            # re-queue rows where smart_name is explicitly null: the writer
+            # only emits that when compute_smart_names truly can't classify
+            # the arrangement (e.g. a name outside the recognised set with
+            # zero path flags), so rescanning would produce the same null
+            # forever and never converge.
+            to_scan.append((f, mtime, size, dlc))
+
+    if not to_scan:
+        _scan_status = {**_SCAN_STATUS_INIT, "running": True, "stage": "complete", "added": added, "removed": removed}
+        log.info("Scan: nothing new to scan (%d songs, all cached)", len(all_songs))
+        return
+
+    # Refine: all discovered songs need scanning → treat as first-time import
+    # (covers moved DLC folder / fully-stale DB as well as a genuinely empty DB).
+    is_first_scan = bool(all_songs) and len(to_scan) == len(all_songs)
+    _scan_status = {**_SCAN_STATUS_INIT, "running": True, "stage": "scanning", "total": len(to_scan),
+                    "is_first_scan": is_first_scan}
+    log.info("Library: %d sloppaks + %d loose folders, %d cached, %d to scan",
+             len(sloppaks), len(loose_songs), len(all_songs) - len(to_scan), len(to_scan))
+
+    with _make_scan_executor() as executor:
+        futures = {executor.submit(_scan_one, item): item[0].name for item in to_scan}
+        for future in concurrent.futures.as_completed(futures):
+            fname = futures[future]
+            try:
+                name, mtime, size, meta = future.result()
+                appstate.meta_db.put(name, mtime, size, meta)
+            except Exception as e:
+                log.warning("scan failed for %s: %s", fname, e)
+            _scan_status["done"] += 1
+            _scan_status["current"] = fname
+
+    log.info("Scan complete: %d songs cached", len(to_scan))
+    _scan_status = {**_SCAN_STATUS_INIT, "running": True, "stage": "complete", "added": added, "removed": removed}
+
+
+_scan_kick_lock = threading.Lock()
+
+
+_scan_rescan_pending = False
+
+
+# Handles to the running scan / enrichment worker threads. Both use the shared
+# MetadataDB connection, so teardown/shutdown MUST join them before closing that
+# connection — a daemon thread mid-query on a closed SQLite conn is a native
+# use-after-free that segfaults the process (seen flaky in CI). Set by
+# _kick_scan / _kick_enrich; joined by _join_background_db_threads().
+_scan_thread: threading.Thread | None = None
+
+
+def kick_scan() -> bool:
+    """Request a library rescan, single-flight + coalescing.
+
+    Returns True if a new scan thread was started, False if one was already
+    running. In the latter case a follow-up pass is queued and runs as soon
+    as the current scan finishes so files landing mid-scan (e.g. an upload
+    that finalizes after the scan has already listed DLC_DIR) are not lost
+    until the next periodic pass. Multiple late-arriving requests coalesce
+    into a single follow-up.
+    """
+    global _scan_rescan_pending, _scan_thread
+    with _scan_kick_lock:
+        if _scan_status["running"]:
+            _scan_rescan_pending = True
+            return False
+        # Mark running synchronously so a parallel kick_scan() observes it
+        # before the worker thread has a chance to reassign _scan_status.
+        _scan_status["running"] = True
+    _scan_thread = threading.Thread(target=_scan_runner, daemon=True)
+    _scan_thread.start()
+    return True
+
+
+def _scan_runner():
+    """Run _background_scan, then re-run if requests arrived mid-scan."""
+    global _scan_rescan_pending
+    while True:
+        try:
+            background_scan()
+        except Exception:
+            log.exception("background scan failed unexpectedly")
+
+        with _scan_kick_lock:
+            if not _scan_rescan_pending:
+                _scan_status["running"] = False
+                break
+            _scan_rescan_pending = False
+            _scan_status["running"] = True
+    # Enrichment rides scan completion (library-metadata design §6): the scan
+    # pool is a side-effect-free, no-network process pool by design, so
+    # enrichment is a SEPARATE post-scan pass — non-blocking, the library is
+    # usable immediately. The 5-minute periodic rescan re-kicks it, which is
+    # the natural low-priority retry hook.
+    enrichment._kick_enrich()
+
+
+def status() -> dict:
+    """The live scan status.
+
+    A GETTER, deliberately. `_scan_status` is REBOUND on every stage transition, so a
+    caller holding the dict would be reading a snapshot frozen at whatever stage it
+    happened to grab — see the module header.
+    """
+    return _scan_status
+
+
+def scan_thread():
+    """The background scan thread, or None. Read by shutdown to join it."""
+    return _scan_thread

plugins/__init__.py

@@ -6,6 +6,7 @@ import json
 import logging
 import mimetypes
 import os
+import re
 import subprocess
 import sys
 import threading
@@ -2384,6 +2385,53 @@ def register_plugin_api(app: FastAPI):
                     return _plugin_file_response(request, script_file, "application/javascript")
         return Response("", status_code=404)
 
+    # ── Module-graph cache busting (#879) ────────────────────────────────
+    #
+    # ES modules are evaluated ONCE PER URL PER DOCUMENT. Re-inserting a
+    # <script type="module"> whose src the module map has already seen fires
+    # `load` but does NOT re-run the body. So re-loading a plugin — a rollback,
+    # and (see below) an upgrade too — silently kept the OLD module live while
+    # the loader recorded success: a no-op that reported it worked.
+    #
+    # Busting the ENTRY url does not help. A module plugin's screen.js is a
+    # one-line `import './src/main.js'`, and a relative specifier resolves
+    # against the base URL WITH THE QUERY DROPPED — so a ?v= token never reaches
+    # the graph. Driving a real browser through install -> upgrade -> rollback and
+    # counting evaluations of src/main.js gives ONE. The upgrade re-runs the shim
+    # at its new ?v= URL; the shim imports './src/main.js'; that resolves to the
+    # same URL; the module map returns the already-evaluated old module.
+    #
+    # So the token goes in the PATH: /api/plugins/<id>/g/<n>/screen.js. Every
+    # relative import inherits it at every depth — for free, with no
+    # import-specifier rewriting (which could never see `import(expr)` anyway).
+    #
+    # WHY A PATH REWRITE AND NOT TWO MIRRORED ROUTES. The token shifts the base
+    # URL, so EVERYTHING a module resolves relatively moves with it — not just
+    # imports. `new URL('../assets/worklet.js', import.meta.url)` from
+    # /api/plugins/x/g/1/src/main.js resolves to /api/plugins/x/g/1/assets/... .
+    # Mirroring only screen.js and src/ would fix imports and 404 every asset,
+    # worklet and wasm file the graph reaches — and would silently break again the
+    # next time someone adds a plugin route. Stripping the segment before routing
+    # makes every plugin route, present and future, work under the prefix.
+    #
+    # The token is opaque: it is never joined into a filesystem path (and is gone
+    # by the time any handler runs), so containment still rests entirely on the
+    # same safe_join the un-prefixed routes use.
+    _GEN_PREFIX = re.compile(r"^(/api/plugins/[^/]+)/g/[^/]+(/.+)$")
+
+    @app.middleware("http")
+    async def _strip_plugin_generation_prefix(request: Request, call_next):
+        m = _GEN_PREFIX.match(request.scope.get("path", ""))
+        if m:
+            # Starlette routes on scope["path"] alone. raw_path is deliberately left
+            # ALONE: it is informational, and re-encoding the rewritten str back to
+            # bytes would have to guess a codec — `.encode("latin-1")` raises
+            # UnicodeEncodeError on a perfectly valid plugin file like src/工具.js,
+            # 500ing a request the un-prefixed route serves fine. Leaving raw_path as
+            # the client actually sent it is also simply more truthful for logs.
+            request.scope["path"] = m.group(1) + m.group(2)
+        return await call_next(request)
+
     @app.get("/api/plugins/{plugin_id}/settings.html")
     def plugin_settings_html(plugin_id: str):
         with PLUGINS_LOCK:

static/js/count-in.js

@@ -0,0 +1,389 @@
+// Count-in — the 1-2-3-4 click before playback, plus the song-credits overlay that
+// shares its lifecycle and timers.
+//
+// The third slice out of app.js's strongly-connected core, and the first that had to
+// WRITE shared state rather than just read it. It starts and stops playback, so it sets
+// `isPlaying` and `lastAudioTime`. An imported binding is read-only — `isPlaying = true`
+// throws — which is exactly why those two scalars were lifted onto the container in
+// ./player-state.js. Every earlier slice only READ what it shared, so a getter hook
+// sufficed; this one could not.
+//
+// It imports the loop module directly (setLoop / loopA / loopB — a count-in that starts
+// inside an A-B loop must begin at A). Nothing imports count-in back: app.js and
+// section-practice both reach it through the host seam, so the graph stays acyclic.
+//
+// app.js's autoplay path used to reach IN and set the credits timers itself. It cannot
+// now, and it should not have to — so the module exports the OPERATIONS instead
+// (armCreditsHideOnPlay, scheduleCreditsHide, holdCreditsThen, isCountingIn) and owns
+// its own timer invariants. Same reason section-practice grew resetSelection().
+//
+// See ./host.js: reading an unwired hook THROWS, and tests/js/host_contract.test.js
+// fails CI if the hooks used here and the hooks app.js wires ever drift apart.
+import { audio } from './audio-el.js';
+import { _audioSeek, _songEventPayload, jucePlayer, setPlayButtonState, togglePlay } from './transport.js';
+import { loopA, loopB, setLoop } from './loops.js';
+import { S } from './player-state.js';
+
+// ── Count-in click sound (Web Audio API) ────────────────────────────────
+let _audioCtx = null;
+export function playClick(high = false) {
+    if (!_audioCtx) _audioCtx = new (window.AudioContext || window.webkitAudioContext)();
+    const osc = _audioCtx.createOscillator();
+    const gain = _audioCtx.createGain();
+    osc.connect(gain);
+    gain.connect(_audioCtx.destination);
+    osc.frequency.value = high ? 1200 : 800;
+    osc.type = 'sine';
+    gain.gain.setValueAtTime(0.5, _audioCtx.currentTime);
+    gain.gain.exponentialRampToValueAtTime(0.001, _audioCtx.currentTime + 0.08);
+    osc.start(_audioCtx.currentTime);
+    osc.stop(_audioCtx.currentTime + 0.08);
+}
+
+let _countingIn = false;
+let _countOverlay = null;
+// Generation token so teardown can cancel an in-progress count-in. Each
+// startCountIn() captures the gen at entry; rewindStep, the loop-wrap
+// then-callback, and beginCount's tick all bail when their captured gen
+// no longer matches. Bumped by _cancelCountIn().
+let _countInGen = 0;
+let _countInTimer = null;
+let _countInRaf = 0;
+// Feedpak credits overlay (manifest `authors:`, spec §5.4): shown on the
+// highway when a song is loaded, alongside the count-in. Torn down together
+// with the count-in via _cancelCountIn().
+let _creditsOverlay = null;
+let _creditsTimer = null;
+let _creditsHideOnPlay = null;
+let _creditsMaxTimer = null;
+const _CREDITS_HOLD_MS = 3000;
+// Backstop: the overlay's primary dismiss is song:play, but playback can fail
+// to start without emitting it (HTML5 autoplay rejection, JUCE start failure,
+// a count-in handoff that never plays). This hard cap guarantees the credits
+// never linger over the highway. Generous enough to outlast a normal count-in.
+const _CREDITS_MAX_MS = 12000;
+export function _cancelCountIn() {
+    _countInGen++;
+    _countingIn = false;
+    hideCountOverlay();
+    // The credits overlay rides the count-in lifecycle (and its no-count-in
+    // hold timer), so a teardown — leaving the player, loading another song —
+    // must clear it too, or it lingers on the next screen.
+    hideSongCreditsOverlay();
+    if (_countInTimer) { clearTimeout(_countInTimer); _countInTimer = null; }
+    if (_countInRaf) { cancelAnimationFrame(_countInRaf); _countInRaf = 0; }
+}
+
+export function showCountOverlay(n) {
+    if (!_countOverlay) {
+        _countOverlay = document.createElement('div');
+        _countOverlay.className = 'fixed inset-0 z-[100] flex items-center justify-center pointer-events-none';
+        document.body.appendChild(_countOverlay);
+    }
+    _countOverlay.innerHTML = `<span class="text-9xl font-black text-white/30">${n}</span>`;
+}
+
+export function hideCountOverlay() {
+    if (_countOverlay) { _countOverlay.remove(); _countOverlay = null; }
+}
+
+// Map a feedpak author `role` to a friendly "<verb> by" credit line. The
+// recommended vocabulary is from feedpak spec §5.4; unknown roles are
+// title-cased ("foo" → "Foo by"); a missing role shows the bare name.
+const _CREDIT_ROLE_VERBS = {
+    charter: 'Charted by',
+    transcriber: 'Transcribed by',
+    arranger: 'Arranged by',
+    editor: 'Edited by',
+    mixer: 'Mixed by',
+    engineer: 'Engineered by',
+    proofreader: 'Proofread by',
+};
+
+function _creditLineLabel(role) {
+    if (!role) return '';
+    const key = String(role).trim().toLowerCase();
+    if (_CREDIT_ROLE_VERBS[key]) return _CREDIT_ROLE_VERBS[key];
+    return key.charAt(0).toUpperCase() + key.slice(1) + ' by';
+}
+
+// Show the feedpak contributor credits over the highway. `authors` is the
+// sanitized [{name, role}] list from window.feedBack.currentSong.authors.
+// Anchored to the lower third (bottom-center) so it never collides with the
+// vertically-centered count-in number, and pointer-events-none so it never
+// intercepts clicks. No-op when there are no contributors to show.
+export function showSongCreditsOverlay(authors) {
+    if (!Array.isArray(authors) || authors.length === 0) return;
+    if (!_creditsOverlay) {
+        _creditsOverlay = document.createElement('div');
+        _creditsOverlay.className = 'song-credits-overlay';
+        document.body.appendChild(_creditsOverlay);
+    }
+    // Build via DOM + textContent — author names are untrusted pack data and
+    // must never be interpolated as HTML.
+    _creditsOverlay.replaceChildren();
+    const card = document.createElement('div');
+    card.className = 'song-credits-card';
+
+    const eyebrow = document.createElement('div');
+    eyebrow.className = 'song-credits-eyebrow';
+    eyebrow.textContent = 'Credits';
+    card.appendChild(eyebrow);
+
+    const title = (window.feedBack && window.feedBack.currentSong
+        && window.feedBack.currentSong.title) || '';
+    if (title) {
+        const heading = document.createElement('div');
+        heading.className = 'song-credits-heading';
+        heading.textContent = title;
+        card.appendChild(heading);
+    }
+
+    for (const a of authors) {
+        if (!a || !a.name) continue;
+        const row = document.createElement('div');
+        row.className = 'song-credits-line';
+        const label = _creditLineLabel(a.role);
+        if (label) {
+            const lab = document.createElement('span');
+            lab.className = 'song-credits-role';
+            lab.textContent = label + ' ';
+            row.appendChild(lab);
+        }
+        const nm = document.createElement('span');
+        nm.className = 'song-credits-name';
+        nm.textContent = a.name;
+        row.appendChild(nm);
+        card.appendChild(row);
+    }
+    _creditsOverlay.appendChild(card);
+    // Arm the backstop so the overlay self-clears even if playback never starts
+    // / never emits song:play. song:play (or any teardown) clears it earlier.
+    if (_creditsMaxTimer) clearTimeout(_creditsMaxTimer);
+    _creditsMaxTimer = setTimeout(hideSongCreditsOverlay, _CREDITS_MAX_MS);
+}
+
+export function hideSongCreditsOverlay() {
+    if (_creditsTimer) { clearTimeout(_creditsTimer); _creditsTimer = null; }
+    if (_creditsMaxTimer) { clearTimeout(_creditsMaxTimer); _creditsMaxTimer = null; }
+    if (_creditsHideOnPlay) {
+        window.feedBack.off('song:play', _creditsHideOnPlay);
+        _creditsHideOnPlay = null;
+    }
+    if (_creditsOverlay) { _creditsOverlay.remove(); _creditsOverlay = null; }
+}
+
+export async function startCountIn(opts = {}) {
+    if (_countingIn) return;
+    _countingIn = true;
+    // Snapshot the current gen so every delayed callback (rewind frames,
+    // post-seek then, count-in ticks, post-count play) can bail if a
+    // teardown bumped the gen mid-flight via _cancelCountIn().
+    const gen = _countInGen;
+    const immediate = !!opts.immediate;
+    if (window._juceMode) {
+        await jucePlayer.pause().catch((err) => console.error('[app] jucePlayer.pause error in count-in:', err));
+    } else {
+        audio.pause();
+    }
+    if (gen !== _countInGen) return; // teardown during pause
+
+    // Section-practice entry: already at loop A after setLoop(); skip the
+    // B→A rewind animation used on loop wrap and go straight to clicks.
+    if (immediate) {
+        if (loopA === null || loopB === null) {
+            _countingIn = false;
+            return;
+        }
+        S.lastAudioTime = loopA;
+        highway.setTime(loopA);
+        if (window.feedBack) {
+            window.feedBack.emit('loop:restart', { loopA, loopB, time: loopA });
+        }
+        beginCount();
+        return;
+    }
+
+    // Rewind animation: sweep highway time from B to A
+    const rewindDuration = 400; // ms
+    const rewindStart = performance.now();
+    const fromTime = loopB;
+    const toTime = loopA;
+
+    function rewindStep(now) {
+        if (gen !== _countInGen) return; // teardown mid-rewind
+        const elapsed = now - rewindStart;
+        const t = Math.min(elapsed / rewindDuration, 1);
+        // Ease out quad
+        const eased = 1 - (1 - t) * (1 - t);
+        const currentT = fromTime + (toTime - fromTime) * eased;
+        highway.setTime(currentT);
+        if (t < 1) {
+            _countInRaf = requestAnimationFrame(rewindStep);
+        } else {
+            _countInRaf = 0;
+            // Rewind done — set final position and start count.
+            // Await the JUCE seek so the engine has repositioned before
+            // we start the click track (HTML5 path is synchronous).
+            _audioSeek(loopA, 'loop-wrap').then((r) => {
+                if (gen !== _countInGen) return; // teardown during seek
+                // Abort the loop restart in two cases:
+                //   1. Cancelled (player torn down): don't beginCount on a
+                //      new session.
+                //   2. Off-target landing (JUCE rollback / clamp far from
+                //      loopA): proceeding would emit loop:restart and start
+                //      a count-in from the wrong position. Audio is at
+                //      r.from / r.to, which is not where the loop wants to
+                //      resume — better to drop this iteration than play out
+                //      of sync.
+                // 50 ms tolerance: well within JUCE's normal seek precision
+                // but tight enough to catch a real rollback or no-op.
+                if (!r.completed || Math.abs(r.to - loopA) > 0.05) {
+                    // startCountIn paused audio at entry but left isPlaying
+                    // alone — beginCount would have set it on resume. On
+                    // abort, sync the transport: audio is paused, so
+                    // isPlaying must reflect that and the button + plugin
+                    // host must agree.
+                    _countingIn = false;
+                    if (S.isPlaying) {
+                        S.isPlaying = false;
+                        setPlayButtonState(false);
+                        if (window.feedBack) {
+                            window.feedBack.isPlaying = false;
+                            window.feedBack.emit('song:pause', _songEventPayload());
+                        }
+                    }
+                    return;
+                }
+                // Use the verified post-seek clock for the chart so audio
+                // and chart stay in sync if JUCE clamped to slightly
+                // before/after loopA. The loop:restart event keeps `time:
+                // loopA` because subscribers treat that as the semantic
+                // marker for "new iteration starts at A", not the actual
+                // audio position.
+                S.lastAudioTime = r.to;
+                highway.setTime(r.to);
+                window.feedBack.emit('loop:restart', { loopA, loopB, time: loopA });
+                beginCount();
+            });
+        }
+    }
+    _countInRaf = requestAnimationFrame(rewindStep);
+
+    function beginCount() {
+        const bpm = highway.getBPM(loopA);
+        const beatInterval = 60 / bpm;
+        let count = 0;
+
+        function tick() {
+            if (gen !== _countInGen) return; // teardown mid-count
+            count++;
+            if (count > 4) {
+                hideCountOverlay();
+                _countingIn = false;
+                if (window._juceMode) {
+                    jucePlayer.play().then((started) => {
+                        if (gen !== _countInGen) return; // teardown during play start
+                        if (!started) return;
+                        S.isPlaying = true;
+                        setPlayButtonState(true);
+                        window.feedBack.isPlaying = true;
+                        const payload = _songEventPayload();
+                        window.feedBack.emit('song:play', payload);
+                        window.feedBack.emit('song:resume', payload);
+                    }).catch((err) => console.error('[app] jucePlayer.play error:', err));
+                } else {
+                    audio.play().then(() => {
+                        if (gen !== _countInGen) return;
+                        S.isPlaying = true;
+                        setPlayButtonState(true);
+                    }).catch((err) => {
+                        if (gen !== _countInGen) return;
+                        // An engine reroute's deliberate pause aborts this play()
+                        // while playback continues on JUCE — don't reset the
+                        // button (mirrors the togglePlay guard).
+                        if (window._juceRerouteInProgress) return;
+                        // Same rationale as togglePlay: don't claim playback
+                        // started if the Promise rejected.
+                        console.error('[app] audio.play() rejected after count-in:', err);
+                        S.isPlaying = false;
+                        setPlayButtonState(false);
+                    });
+                }
+                return;
+            }
+            showCountOverlay(count);
+            playClick(count === 1);
+            _countInTimer = setTimeout(tick, beatInterval * 1000);
+        }
+        _countInTimer = setTimeout(tick, 500);
+    }
+}
+
+// Start-of-song count-in: a 4-beat click before playback begins, gated by the
+// "Countdown before song" setting (Gameplay tab). Mirrors the loop count-in's
+// overlay + click + gen-token cancellation, but counts from the song's current
+// position (0 at song start) with no loop A/B rewind. startCountIn() is loop-
+// coupled (early-returns when loopA/loopB are null), so this is a sibling
+// rather than an overload. Hands off to togglePlay() once the count completes.
+export async function startSongCountIn() {
+    if (_countingIn) return;
+    _countingIn = true;
+    // Snapshot the gen so a teardown (showScreen/playSong calls _cancelCountIn)
+    // bumps it and every delayed callback below bails.
+    const gen = _countInGen;
+    if (window._juceMode) {
+        await jucePlayer.pause().catch((err) => console.error('[app] jucePlayer.pause error in song count-in:', err));
+    } else {
+        audio.pause();
+    }
+    if (gen !== _countInGen) return; // teardown during pause
+    const startT = S.lastAudioTime || 0;
+    let bpm = highway.getBPM(startT);
+    // Pre-chart / malformed-tempo fallback: 4 beats at 120 BPM (500 ms each).
+    if (!Number.isFinite(bpm) || bpm <= 0) bpm = 120;
+    const beatInterval = 60 / bpm;
+    let count = 0;
+    function tick() {
+        if (gen !== _countInGen) return; // teardown mid-count
+        count++;
+        if (count > 4) {
+            hideCountOverlay();
+            _countingIn = false;
+            // Hand off to the normal play path — togglePlay() flips isPlaying,
+            // updates the button, and emits song:play/resume for plugins.
+            Promise.resolve(togglePlay()).catch((err) => console.warn('[app] play after count-in failed:', err));
+            return;
+        }
+        showCountOverlay(count);
+        playClick(count === 1);
+        _countInTimer = setTimeout(tick, beatInterval * 1000);
+    }
+    // First beat after a short lead-in, matching the loop count-in's 500 ms.
+    _countInTimer = setTimeout(tick, 500);
+}
+
+// ── Operations app.js's autoplay path used to perform by reaching in ────────
+// It used to assign _creditsTimer / _creditsHideOnPlay directly. Imported bindings are
+// read-only, and the module should own its own timer invariants anyway.
+
+/** Is a count-in running? app.js's timeupdate handler suppresses highway sync during one. */
+export function isCountingIn() {
+    return _countingIn;
+}
+
+/** Dismiss the credits the moment real playback begins. Fires once. */
+export function armCreditsHideOnPlay() {
+    _creditsHideOnPlay = () => { _creditsHideOnPlay = null; hideSongCreditsOverlay(); };
+    window.feedBack.on('song:play', _creditsHideOnPlay, { once: true });
+}
+
+/** Let the credits dwell, then clear them. Used when autoplay-exit is disabled. */
+export function scheduleCreditsHide() {
+    _creditsTimer = setTimeout(hideSongCreditsOverlay, _CREDITS_HOLD_MS);
+}
+
+/** Let the credits dwell, then run `then` (the autoplay start). */
+export function holdCreditsThen(then) {
+    _creditsTimer = setTimeout(() => { _creditsTimer = null; then(); }, _CREDITS_HOLD_MS);
+}

static/js/format.js

@@ -0,0 +1,17 @@
+// Display formatters. A LEAF module: imports nothing.
+//
+// WHY THIS EXISTS FOR ONE FUNCTION. formatTime was a HOST HOOK — loops.js and
+// section-practice.js both reached back through the seam for it. It was also, by pure
+// accident of who calls it, inside the dependency closure of the library carve. Leaving
+// it there would have made loops.js and section-practice.js import the LIBRARY to format
+// a timestamp, which is nonsense, and a cycle waiting to happen.
+//
+// A hook is a cycle you agreed to live with. This one has a real owner — it just isn't
+// app.js, and it certainly isn't the library. Give it a home of its own and both
+// consumers import it directly.
+//
+// It is a leaf on purpose. Anything else that turns out to be a shared pure formatter
+// belongs here too; nothing does yet, so nothing else is here.
+
+/** Seconds -> `M:SS`. */
+export function formatTime(s) { return `${Math.floor(s / 60)}:${String(Math.floor(s % 60)).padStart(2, '0')}`; }

static/js/library-state.js

@@ -0,0 +1,29 @@
+// Shared, MUTABLE library state.
+//
+// WHY A CONTAINER AND NOT PLAIN EXPORTS. An imported binding is READ-ONLY:
+// `import { _treeStats }; _treeStats = x` throws. Of the library module's 28 outward
+// bindings, 23 are only ever READ from outside, so they stay plain exports. These five
+// are genuinely WRITTEN from outside — by showScreen (session teardown bumps the epoch,
+// resets the page), deleteSongFromModal, and syncLibrarySong, none of which can move into
+// the library module because they reach the playSong/showScreen core.
+//
+// So exactly these five move onto an object, and no more. `L.treeStats = x` is a property
+// write, which works from any module holding the same `L`. Same shape as ./player-state.js.
+//
+// Add to it when a carve actually needs it, not before — a container is a shared mutable
+// global with better manners, and every field on it is a coupling you have to keep true.
+export const L = {
+    /** Library tree stats (artist -> counts), cached from /api/library/tree-stats. */
+    treeStats: null,
+    /** Same, for the favourites tree. */
+    favTreeStats: null,
+    /** Tuning names, cached from /api/library/tuning-names. */
+    tuningNames: null,
+    /**
+     * Session generation for the library. Bumped on teardown so an in-flight page fetch
+     * that resolves against a stale library can't render into the new one.
+     */
+    libEpoch: 0,
+    /** Current grid page (0-based). */
+    currentPage: 0,
+};

static/js/loops.js

@@ -19,6 +19,8 @@
 // See ./host.js: reading an unwired hook THROWS, and tests/js/host_contract.test.js
 // fails CI if the hooks used here and the hooks app.js wires ever drift apart.
 import { esc, uiPrompt } from './dom.js';
+import { _audioSeek, _audioTime } from './transport.js';
+import { formatTime } from './format.js';
 import { host } from './host.js';
 import {
     _setSectionPracticeMode,
@@ -39,14 +41,14 @@ export let loopB = null;
 export let _loopMutationGen = 0;
 
 export function setLoopStart() {
-    loopA = host._audioTime();
+    loopA = _audioTime();
     document.getElementById('btn-loop-a').className = 'px-3 py-1.5 bg-green-900/50 rounded-lg text-xs text-green-300 transition';
     updateLoopUI();
 }
 
 export function setLoopEnd() {
     if (loopA === null) return;
-    loopB = host._audioTime();
+    loopB = _audioTime();
     if (loopB <= loopA) { loopB = null; return; }
     document.getElementById('btn-loop-b').className = 'px-3 py-1.5 bg-green-900/50 rounded-lg text-xs text-green-300 transition';
     updateLoopUI();
@@ -72,7 +74,7 @@ export function clearLoop(options) {
     document.getElementById('loop-label').textContent = '';
     document.getElementById('saved-loops').value = '';
     resetSelection();
-    _updateSectionPracticeHighlight(host._audioTime());
+    _updateSectionPracticeHighlight(_audioTime());
     if (hadLoop && emitTransportEvent && typeof window !== 'undefined') {
         window.feedBack?.playback?.transportEvent?.('loop-cleared', {
             requesterId: 'core.loop',
@@ -125,7 +127,7 @@ export async function setLoop(a, b, options) {
     // Don't arm loopA/loopB before the seek lands — the 60Hz tick's wrap
     // detector (`ct >= loopB`) would trigger startCountIn against
     // half-applied state.
-    const r = await host._audioSeek(aNum, 'loop-set');
+    const r = await _audioSeek(aNum, 'loop-set');
     if (!r.completed || Math.abs(r.to - aNum) > 0.05) return false;
     // Caller-owned staleness gate, re-checked after the awaited seek and before
     // we commit loopA/loopB. practiceSection() passes this so a superseded retry
@@ -166,11 +168,11 @@ export function updateLoopUI() {
     const label = document.getElementById('loop-label');
     const hasLoop = loopA !== null && loopB !== null;
     if (hasLoop) {
-        label.textContent = `${host.formatTime(loopA)} → ${host.formatTime(loopB)}`;
+        label.textContent = `${formatTime(loopA)} → ${formatTime(loopB)}`;
         document.getElementById('btn-loop-clear').classList.remove('hidden');
         document.getElementById('btn-loop-save').classList.remove('hidden');
     } else if (loopA !== null) {
-        label.textContent = `${host.formatTime(loopA)} → ?`;
+        label.textContent = `${formatTime(loopA)} → ?`;
         document.getElementById('btn-loop-clear').classList.add('hidden');
         document.getElementById('btn-loop-save').classList.add('hidden');
     } else {
@@ -189,7 +191,7 @@ export async function loadSavedLoops() {
 
     sel.innerHTML = '<option value="">Saved Loops</option>';
     for (const l of loops) {
-        sel.innerHTML += `<option value="${l.id}" data-start="${l.start}" data-end="${l.end}">${esc(l.name)} (${host.formatTime(l.start)}→${host.formatTime(l.end)})</option>`;
+        sel.innerHTML += `<option value="${l.id}" data-start="${l.start}" data-end="${l.end}">${esc(l.name)} (${formatTime(l.start)}→${formatTime(l.end)})</option>`;
     }
     if (loops.length > 0) {
         sel.classList.remove('hidden');

static/js/player-controls.js

@@ -0,0 +1,229 @@
+// Player controls — the speed and mastery sliders, and the four playback preference
+// reads (autoplay-exit, up-next, countdown-before-song, confirm-exit).
+//
+// The fourth slice out of app.js's strongly-connected core, and by far the easiest:
+// ONE hook and NO shared mutable state. It is here because these three groups are the
+// same surface (the controls under the highway) and all three reach the same helper.
+//
+// The preference reads are one-line localStorage lookups that half of app.js consults
+// before deciding whether to auto-start, show the Up Next pill, run a count-in, or
+// confirm on exit. They travel with the controls that set them.
+//
+// See ./host.js: reading an unwired hook THROWS, and tests/js/host_contract.test.js
+// fails CI if the hooks used here and the hooks app.js wires ever drift apart.
+import { audio } from './audio-el.js';
+import { host } from './host.js';
+
+// ── Autoplay & auto-exit (global option, default ON) ──────────────────
+// One toggle (`autoplayExit` in localStorage) that (a) auto-starts a song
+// once it's ready and (b) returns to the launching menu when the song
+// ends. Absence of the key means enabled. The behaviour lives in core
+// (app.js, shared by the v3 + classic UIs); the end-of-song *score*
+// screen, when present, is a plugin and hooks the contract below.
+export function _autoplayExitEnabled() {
+    try { return localStorage.getItem('autoplayExit') !== '0'; } catch (_) { return true; }
+}
+
+// ── "Up Next" pill (global option, default ON) ────────────────────────
+// Gates the v3 player chrome's persistent upcoming-section pill
+// (#v3-upnext, driven by player-chrome.js's updateUpNext). Client-only
+// localStorage pref (`showUpNext`); absence of the key means enabled.
+// player-chrome.js reads window.feedBack.showUpNext each tick and hides
+// the pill when off.
+export function _showUpNextEnabled() {
+    try { return localStorage.getItem('showUpNext') !== '0'; } catch (_) { return true; }
+}
+
+// "Countdown before song" (Gameplay tab). Mirrored to localStorage by
+// loadSettings so the song-start path can read it synchronously here — no
+// async /api/settings fetch on the play hot path. Defaults off.
+export function _countdownBeforeSongEnabled() {
+    try { return localStorage.getItem('countdownBeforeSong') === '1'; } catch (_) { return false; }
+}
+
+export function _curPlaybackSpeed() {
+    try {
+        return window._juceMode
+            ? ((window.jucePlayer && window.jucePlayer._speed) || 1)
+            : (document.getElementById('audio')?.playbackRate || 1);
+    } catch (_) { return 1; }
+}
+
+// ── "Ask before leaving a song" (Gameplay tab, default OFF) ────────────────
+// Client-only localStorage pref (`confirmExitSong`); absence = OFF. When ON, a
+// *user-initiated* exit (Escape, or the player ✕) opens a small confirm instead
+// of leaving immediately. Auto-exit on song-end and a results screen's own
+// Close never prompt — they call closeCurrentSong() directly, which stays the
+// unguarded actual-exit.
+export function _exitConfirmEnabled() {
+    try { return localStorage.getItem('confirmExitSong') === '1'; } catch (_) { return false; }
+}
+
+const SPEED_PRESET_PCTS = [100, 90, 80, 75, 70, 60, 50];
+const SPEED_SNAP_THRESHOLD = 0.02;
+let _speedPresetsWired = false;
+
+function _speedPresetPctFromActive(activePctOrRate) {
+    if (!Number.isFinite(activePctOrRate)) return null;
+    const rate = activePctOrRate <= 1.5 ? activePctOrRate : activePctOrRate / 100;
+    for (const pct of SPEED_PRESET_PCTS) {
+        if (Math.abs(rate - pct / 100) <= SPEED_SNAP_THRESHOLD) return pct;
+    }
+    return null;
+}
+
+function _updateSpeedPresetButtons(activePctOrRate) {
+    const wrap = document.getElementById('speed-presets');
+    if (!wrap) return;
+    const target = _speedPresetPctFromActive(activePctOrRate);
+    for (const btn of wrap.querySelectorAll('[data-speed-preset]')) {
+        const pct = Number(btn.dataset.speedPreset);
+        btn.classList.toggle('v3-speed-preset-active', target !== null && pct === target);
+    }
+}
+
+export function applySpeedPreset(percent) {
+    const slider = document.getElementById('speed-slider');
+    if (!slider) return;
+    const pct = Math.max(
+        Number(slider.min) || 15,
+        Math.min(Number(slider.max) || 150, Number(percent)),
+    );
+    if (!Number.isFinite(pct)) return;
+    slider.value = String(pct);
+    host.handleSliderInput(slider);
+    slider.dispatchEvent(new Event('input', { bubbles: true }));
+}
+
+export function _wireSpeedPresetsOnce() {
+    if (_speedPresetsWired) return;
+    const presets = document.getElementById('speed-presets');
+    if (!presets) return;
+    _speedPresetsWired = true;
+    presets.addEventListener('click', (e) => {
+        const btn = e.target.closest('[data-speed-preset]');
+        if (!btn) return;
+        applySpeedPreset(Number(btn.dataset.speedPreset));
+    });
+}
+
+export function setSpeed(v) {
+    const speedSlider = document.getElementById('speed-slider');
+    const rate = Number(v);
+    if (!Number.isFinite(rate)) {
+        return;
+    }
+    if (window._juceMode) {
+        window.jucePlayer?.setRate(rate);
+        const juceAudio = window.feedBackDesktop?.audio;
+        Promise.resolve()
+            .then(() => juceAudio?.setBackingSpeed(rate))
+            // Match the HTML5 path: preserve pitch on the JUCE backing track too.
+            // Optional-chained call is a no-op on desktop builds that predate
+            // setBackingPreservePitch, so this is safe to ship unconditionally.
+            .then(() => juceAudio?.setBackingPreservePitch?.(true))
+            .catch(err => console.warn('[setSpeed] backing speed/preserve-pitch failed:', err));
+    } else {
+        audio.playbackRate = rate;
+    }
+    const speedLabel = document.getElementById('speed-label');
+    if (speedLabel) speedLabel.textContent = rate.toFixed(2) + 'x';
+    host.handleSliderInput(speedSlider);
+    _updateSpeedPresetButtons(rate);
+}
+
+export function _resetPlaybackSpeedForNewSong() {
+    // Reset the *actual* playback rate to 1x, not just the visible slider/label
+    // (feedBack#615). The HTML5 <audio> element and the desktop JUCE/backing
+    // engine each retain their own rate, and which one drives the next song
+    // isn't decided until later in the load, so reset all paths unconditionally.
+    // Every setter is idempotent and optional-chained, so this is safe in web
+    // and desktop builds alike — no need to branch on window._juceMode.
+    const speedSlider = document.getElementById('speed-slider');
+    if (speedSlider) speedSlider.value = 100;
+    audio.playbackRate = 1;
+    window.jucePlayer?.setRate?.(1);
+    const juceAudio = window.feedBackDesktop?.audio;
+    Promise.resolve()
+        .then(() => juceAudio?.setBackingSpeed?.(1))
+        .then(() => juceAudio?.setBackingPreservePitch?.(true))
+        .catch(err => console.warn('[resetSpeed] backing speed/preserve-pitch failed:', err));
+    // Mirror setSpeed's UI side-effects (label text + slider fill styling).
+    const speedLabel = document.getElementById('speed-label');
+    if (speedLabel) speedLabel.textContent = (1).toFixed(2) + 'x';
+    host.handleSliderInput(speedSlider);
+    _updateSpeedPresetButtons(100);
+}
+// Master-difficulty slider (feedBack#48). Persists partial via
+// /api/settings — the POST handler merges only the keys present, so
+// this fire-and-forget call doesn't clobber dlc_dir or other settings.
+//
+// Debounced trailing-edge (300ms) so dragging the slider — which fires
+// oninput per pixel — doesn't flood the server with concurrent writes
+// to config.json. highway.setMastery() still fires every oninput so
+// the chart re-filters in real time; only disk persistence waits.
+let _masteryPersistTimer = null;
+function _persistMastery(pct) {
+    if (_masteryPersistTimer) clearTimeout(_masteryPersistTimer);
+    _masteryPersistTimer = setTimeout(() => {
+        _masteryPersistTimer = null;
+        fetch('/api/settings', {
+            method: 'POST',
+            headers: { 'Content-Type': 'application/json' },
+            body: JSON.stringify({ master_difficulty: pct }),
+        }).catch(() => { /* best-effort — next setMastery() will retry */ });
+    }, 300);
+}
+export function setMastery(v) {
+    _applyMastery(v);
+}
+// Shared mastery applier. Master difficulty has two controls that write the
+// same master_difficulty key: the player-popover slider (#mastery-slider) and
+// the Gameplay-tab "Note highway speed" slider (#setting-highway-speed). Route
+// both — and loadSettings' hydration — through here so their positions,
+// labels, and track fills stay in sync regardless of which the user touches,
+// plus the live highway re-filter and the debounced persist. All element reads
+// are null-guarded since either control may be absent (follower window, or the
+// settings markup not yet rendered).
+export function _applyMastery(v, opts = {}) {
+    // Guard + clamp: v might be a slider string, a programmatic call from a
+    // plugin, or a restored settings value with a bad shape. Don't let NaN
+    // reach a label (would show "NaN%") or the POST.
+    const parsed = parseInt(v, 10);
+    if (!Number.isFinite(parsed)) return;
+    const pct = Math.max(0, Math.min(100, parsed));
+    const popLabel = document.getElementById('mastery-label');
+    if (popLabel) popLabel.textContent = pct + '%';
+    const popSlider = document.getElementById('mastery-slider');
+    if (popSlider) {
+        if (String(popSlider.value) !== String(pct)) popSlider.value = pct;
+        host.handleSliderInput(popSlider);
+    }
+    const setSlider = document.getElementById('setting-highway-speed');
+    if (setSlider) {
+        if (String(setSlider.value) !== String(pct)) setSlider.value = pct;
+        host.handleSliderInput(setSlider);
+    }
+    // The Gameplay-tab label markup appends a literal "%" after this span
+    // (matching the av-offset "ms" pattern), so write the number alone here —
+    // unlike #mastery-label above, whose markup carries no trailing unit.
+    const setLabel = document.getElementById('setting-highway-speed-val');
+    if (setLabel) setLabel.textContent = pct;
+    highway.setMastery(pct / 100);
+    if (!opts.skipPersist) _persistMastery(pct);
+}
+// Reflect phrase-data availability on the slider after every `ready`.
+// The server omits the `phrases` message entirely for single-level
+// sources (GP imports, legacy sloppak), so hasPhraseData() is the
+// right signal to enable/disable the slider.
+export function _applyMasteryAvailability(hasPhraseData) {
+    const slider = document.getElementById('mastery-slider');
+    if (!slider) return;
+    if (hasPhraseData) {
+        slider.disabled = false;
+        slider.title = 'Master difficulty — low = simpler chart, high = full';
+    } else {
+        slider.disabled = true;
+        slider.title = 'Source chart has a single difficulty level — slider disabled';
+    }
+}

static/js/player-state.js

@@ -31,4 +31,12 @@ export const S = {
      * land where it was asked to (JUCE can clamp; HTML5 can round).
      */
     lastAudioTime: 0,
+
+    /**
+     * A resume request armed by playSong({ resume }) and consumed on song:ready.
+     * Written by app.js (playSong, and the song:ready listener that consumes it) and
+     * read by the resume-session module — so, like the two above, it cannot be a plain
+     * export.
+     */
+    pendingResume: null,
 };

static/js/plugin-loader.js

@@ -654,7 +654,8 @@ export async function loadPlugins() {
                         // of a cached copy keyed only by path (matches the art
                         // URL ?v=mtime convention elsewhere in this file).
                         const v = encodeURIComponent(wantedVersion);
-                        script.src = `/api/plugins/${plugin.id}/screen.js${v ? `?v=${v}` : ''}`;
+                        const query = v ? `?v=${v}` : '';
+                        script.src = _pluginScriptUrl(plugin, wantedVersion, query);
                         // Module-migration (R0): a migrated plugin declares
                         // scriptType:"module" and its screen.js is `import
                         // './src/main.js'`. A <script type="module"> fires load
@@ -844,6 +845,56 @@ export async function checkPluginUpdates() {
     btn.textContent = 'Check for Updates';
 }
 
+// ── Module re-evaluation (#879) ─────────────────────────────────────────────
+//
+// ES modules are evaluated ONCE PER URL PER DOCUMENT. Re-inserting a
+// <script type="module"> whose src the module map has already seen fires `load` but
+// does NOT re-run the body. So a ROLLBACK — reloading a version already evaluated
+// this session — silently kept the OLD module live, while onload fired and
+// loadedScripts recorded the rollback as applied. A no-op that reported success.
+// (Upgrades were fine: a new version means a new ?v=, hence a new URL.)
+//
+// Busting the ENTRY url alone does NOT fix it. A module plugin's screen.js is a
+// one-line `import './src/main.js'`, and a relative specifier resolves against the
+// base URL WITH THE QUERY STRING DROPPED — so ?v= never reaches the graph, and
+// src/main.js (where the plugin actually lives) stays cached no matter what we hang
+// off screen.js.
+//
+// So the token goes in the PATH. From /api/plugins/x/g/7/screen.js, './src/main.js'
+// resolves to /api/plugins/x/g/7/src/main.js — every relative import in the graph
+// inherits it, at every depth, with no import-specifier rewriting (which could not
+// see `import(expr)` anyway). The server ignores the token and serves identical
+// bytes.
+//
+// ─── AND THE UPGRADE PATH WAS BROKEN TOO ────────────────────────────────────
+//
+// #879 says "upgrades are fine — a new version yields a new URL". That is true of
+// screen.js and FALSE of the plugin. Driving a real browser through
+// install(1.0.0) -> upgrade(1.1.0) -> rollback(1.0.0) and counting evaluations of
+// src/main.js gives ONE. Not two, not three: ONE. The upgrade re-evaluates the
+// one-line screen.js shim at its new ?v= URL, that shim imports './src/main.js',
+// that resolves to the same URL as before, and the module map hands back the
+// ALREADY-EVALUATED v1.0.0 module. The plugin's actual code never re-ran.
+//
+// So the generation token is not a rollback special case. EVERY re-load of a module
+// plugin needs it — the key is the plugin id, NOT id@version. Only the first load of
+// a given plugin in this document takes the stable URL, which is what keeps the
+// ETag/304 live-edit contract the R0 rails depend on.
+const _evaluatedModules = new Set();   // plugin ids whose module graph is live in this document
+let _moduleReloadSeq = 0;
+
+function _pluginScriptUrl(plugin, wantedVersion, query) {
+    const base = `/api/plugins/${plugin.id}/screen.js${query}`;
+    if (plugin.script_type !== 'module') return base;   // classic scripts always re-run
+    if (!_evaluatedModules.has(plugin.id)) {
+        _evaluatedModules.add(plugin.id);
+        return base;                                    // first load: stable URL, 304-able
+    }
+    // Re-load of a module plugin — upgrade OR rollback. Its graph is already in the
+    // module map, so it needs an entirely fresh path or nothing below screen.js re-runs.
+    return `/api/plugins/${plugin.id}/g/${++_moduleReloadSeq}/screen.js${query}`;
+}
+
 export async function updatePlugin(pluginId, btn) {
     btn.disabled = true;
     btn.textContent = 'Updating...';

static/js/resume-session.js

@@ -0,0 +1,157 @@
+// Resume last session — the snapshot taken when you leave a song, and the pill that
+// offers it back.
+//
+// The fifth slice out of app.js's strongly-connected core. Small and self-contained:
+// ONE hook (playSong) plus a currentFilename getter.
+//
+// The armed resume request itself lives on the shared container as S.pendingResume,
+// not here, because app.js WRITES it — playSong({ resume }) arms it and the song:ready
+// listener consumes it — while this module reads it. An imported binding is read-only,
+// so shared mutable state has to live on the container. Same reason isPlaying does.
+//
+// See ./host.js: reading an unwired hook THROWS, and tests/js/host_contract.test.js
+// fails CI if the hooks used here and the hooks app.js wires ever drift apart.
+import { host } from './host.js';
+import { _curPlaybackSpeed } from './player-controls.js';
+import { S } from './player-state.js';
+
+// ── Resume last session ────────────────────────────────────────────────────
+// Leaving a song snapshots where you were — song, arrangement, position, and
+// speed — so an exit (especially an accidental one, now that Escape reliably
+// leaves regardless of focus) is recoverable instead of restarting from bar 1.
+// The snapshot is offered back through a non-blocking "Resume" pill; it never
+// gates, blocks, or auto-acts. Cleared on natural song-end and once consumed.
+// (This is the player-session slice; the broader nav/state-resume work — e.g.
+// returning to a song after wandering into Settings → Tone Builder — is a
+// separate, larger track.)
+const _RESUME_KEY = 'feedBack.resumeSession';
+const _RESUME_MAX_AGE_MS = 24 * 60 * 60 * 1000;   // a day-old snapshot is stale
+const _RESUME_MIN_POSITION_S = 3;                  // ignore barely-started songs
+const _RESUME_END_GUARD_S = 5;                      // ignore basically-finished songs
+let _resumePillDismissed = false;                   // per-session: user waved off the current snapshot
+
+// Snapshot the live session. Called from showScreen()'s teardown before
+// highway.stop()/audio unload, while getSongInfo() + position are still valid.
+export function _snapshotResumeSession(position) {
+    try {
+        if (!host.currentFilename()) return;
+        const si = (window.highway && typeof highway.getSongInfo === 'function')
+            ? (highway.getSongInfo() || {}) : {};
+        const dur = Number(si.duration) || 0;
+        const pos = Number(position) || 0;
+        // Only worth resuming a song you were genuinely mid-way through — not a
+        // glance at the first seconds, and not one that already basically ended.
+        if (pos < _RESUME_MIN_POSITION_S) { _clearResumeSession(); return; }
+        if (dur && pos > dur - _RESUME_END_GUARD_S) { _clearResumeSession(); return; }
+        const snap = {
+            f: host.currentFilename(),
+            a: (typeof si.arrangement_index === 'number' && si.arrangement_index >= 0)
+                ? si.arrangement_index : undefined,
+            t: pos,
+            sp: _curPlaybackSpeed(),
+            title: si.title || '',
+            artist: si.artist || '',
+            ts: Date.now(),
+        };
+        localStorage.setItem(_RESUME_KEY, JSON.stringify(snap));
+        // A fresh snapshot earns one offer — undo any earlier dismissal.
+        _resumePillDismissed = false;
+    } catch (_) { /* storage unavailable — resume is best-effort */ }
+}
+
+export function _readResumeSession() {
+    try {
+        const raw = localStorage.getItem(_RESUME_KEY);
+        if (!raw) return null;
+        const snap = JSON.parse(raw);
+        if (!snap || !snap.f || !(Number(snap.t) > 0)) return null;
+        if (!snap.ts || Date.now() - snap.ts > _RESUME_MAX_AGE_MS) { _clearResumeSession(); return null; }
+        return snap;
+    } catch (_) { return null; }
+}
+
+export function _clearResumeSession() {
+    try { localStorage.removeItem(_RESUME_KEY); } catch (_) {}
+}
+
+// Re-enter the snapshotted song and restore arrangement + position + speed.
+export async function resumeLastSession() {
+    const snap = _readResumeSession();
+    if (!snap) { _hideResumePill(); return false; }
+    _hideResumePill();
+    try {
+        await host.playSong(snap.f, snap.a, {
+            resume: { position: Number(snap.t) || 0, speed: Number(snap.sp) || 1 },
+        });
+    } catch (err) {
+        // A transient load/connect failure must not strand the user: keep the
+        // snapshot so the pill can re-offer it on the next non-player screen,
+        // rather than consuming the only copy before the song actually loaded.
+        console.warn('[app] resume failed to load; keeping snapshot:', err);
+        S.pendingResume = null;
+        return false;
+    }
+    _clearResumeSession();   // consumed only after a successful load
+    return true;
+}
+
+// ── Resume pill (non-blocking "continue where you left off") ────────────────
+// Self-contained, inline-styled, body-appended so it works identically in the
+// classic (v2) and v3 shells with no Tailwind rebuild. It only ever appears off
+// the player screen, never blocks, and a dismiss forgets the current snapshot
+// for the session.
+export function _hideResumePill() {
+    const el = document.getElementById('fb-resume-pill');
+    if (el) el.remove();
+}
+
+export function _maybeShowResumePill() {
+    const active = document.querySelector('.screen.active');
+    if (active && active.id === 'player') { _hideResumePill(); return; }
+    if (_resumePillDismissed) return;
+    const snap = _readResumeSession();
+    if (!snap) { _hideResumePill(); return; }
+    if (document.getElementById('fb-resume-pill')) return;   // already shown
+
+    const label = (snap.title || decodeURIComponent(snap.f || 'your last song')).toString();
+    const pill = document.createElement('div');
+    pill.id = 'fb-resume-pill';
+    pill.setAttribute('role', 'status');
+    pill.style.cssText = [
+        'position:fixed', 'left:16px', 'bottom:16px', 'z-index:120',
+        'display:flex', 'align-items:center', 'gap:10px',
+        'max-width:min(90vw,360px)', 'padding:10px 12px',
+        'background:rgba(17,24,39,0.96)', 'color:#e5e7eb',
+        'border:1px solid rgba(148,163,184,0.25)', 'border-radius:10px',
+        'box-shadow:0 6px 24px rgba(0,0,0,0.4)',
+        'font:13px/1.3 system-ui,-apple-system,"Segoe UI",Roboto,sans-serif',
+    ].join(';');
+
+    const text = document.createElement('div');
+    text.style.cssText = 'flex:1;min-width:0';
+    const t1 = document.createElement('div');
+    t1.textContent = 'Resume practice';
+    t1.style.cssText = 'font-weight:600;color:#fff';
+    const t2 = document.createElement('div');
+    t2.textContent = label;
+    t2.style.cssText = 'opacity:0.7;white-space:nowrap;overflow:hidden;text-overflow:ellipsis';
+    text.appendChild(t1); text.appendChild(t2);
+
+    const resumeBtn = document.createElement('button');
+    resumeBtn.type = 'button';
+    resumeBtn.textContent = 'Resume ▸';
+    resumeBtn.style.cssText = 'flex:none;padding:6px 10px;border:0;border-radius:7px;background:#4080e0;color:#fff;font-weight:600;cursor:pointer';
+    resumeBtn.addEventListener('click', () => { resumeLastSession(); });
+
+    const dismissBtn = document.createElement('button');
+    dismissBtn.type = 'button';
+    dismissBtn.setAttribute('aria-label', 'Dismiss');
+    dismissBtn.textContent = '✕';
+    dismissBtn.style.cssText = 'flex:none;padding:4px 6px;border:0;border-radius:7px;background:transparent;color:#9ca3af;cursor:pointer;font-size:14px';
+    dismissBtn.addEventListener('click', () => { _resumePillDismissed = true; _hideResumePill(); });
+
+    pill.appendChild(text);
+    pill.appendChild(resumeBtn);
+    pill.appendChild(dismissBtn);
+    (document.body || document.documentElement).appendChild(pill);
+}

static/js/section-practice.js

@@ -27,6 +27,8 @@
 // layer that catches it on the paths a smoke test never runs.
 import { audio } from './audio-el.js';
 import { esc } from './dom.js';
+import { _audioDuration, _audioTime, audioSeekGen } from './transport.js';
+import { formatTime } from './format.js';
 import { host } from './host.js';
 
 export function _sectionPracticeBarContains(el) {
@@ -85,7 +87,7 @@ export function _setSectionPracticeMode(on, opts = {}) {
         if (opts.defaultWholeOn) {
             _sectionPracticeWholeSection = true;
         }
-        _updateSectionPracticeHighlight(host._audioTime());
+        _updateSectionPracticeHighlight(_audioTime());
         if (opts.defaultWholeOn) {
             _syncSectionPracticePieceUi();
         }
@@ -104,7 +106,7 @@ export function _setSectionPracticeMode(on, opts = {}) {
         _sectionPracticeSelected = -1;
         _sectionPracticeWholeSection = false;
         _sectionPracticeSavedPartIndex = 0;
-        _updateSectionPracticeHighlight(host._audioTime());
+        _updateSectionPracticeHighlight(_audioTime());
         if (!opts.skipClearLoop && (host.loopA() !== null || host.loopB() !== null)) {
             host.clearLoop();
         }
@@ -129,7 +131,7 @@ function _sectionPracticeHighway() {
 }
 
 function _sectionPracticeDuration() {
-    const d = host._audioDuration();
+    const d = _audioDuration();
     if (d && Number.isFinite(d) && d > 0) return d;
     const cd = window.feedBack?.currentSong?.duration;
     return (cd && Number.isFinite(cd) && cd > 0) ? cd : 0;
@@ -901,7 +903,7 @@ export function renderSectionPracticeBar() {
     _showSectionPracticeBar(bar);
     scroll.innerHTML = parents.map((p, i) => {
         const label = _formatSectionPracticeName(p.name);
-        const tip = `${label} (${host.formatTime(p.start)}–${host.formatTime(p.end)})`;
+        const tip = `${label} (${formatTime(p.start)}–${formatTime(p.end)})`;
         const kindClass = _sectionPracticeChipKindClass(p.name, i);
         return `<button type="button" class="section-practice-chip${kindClass}" data-parent-idx="${i}" title="${esc(tip)}" onclick="onSectionParentClick(${i})">${esc(label)}</button>`;
     }).join('');
@@ -914,7 +916,7 @@ export function renderSectionPracticeBar() {
     // matching one; run it before the piece UI so that reflects the result.
     _syncSectionPracticeFromLoop();
     _syncSectionPracticePieceUi();
-    _updateSectionPracticeHighlight(host._audioTime());
+    _updateSectionPracticeHighlight(_audioTime());
 }
 
 export async function onSectionParentClick(parentIdx) {
@@ -927,7 +929,7 @@ export async function onSectionParentClick(parentIdx) {
     _sectionPracticeSavedPartIndex = 0;
     _sectionPracticeWholeSection = true;
     _syncSectionPracticePieceUi();
-    _updateSectionPracticeHighlight(host._audioTime());
+    _updateSectionPracticeHighlight(_audioTime());
     if (_sectionPracticeActiveParentRange() || _sectionPracticeRanges.length) {
         await practiceSection(0, { whole: true });
     }
@@ -1019,7 +1021,7 @@ function _blurSectionPracticeFocusIfNeeded() {
 
 export async function practiceSection(index, opts = {}) {
     const requestGen = ++_sectionPracticeRequestGen;
-    const seekGen = host._audioSeekGen();
+    const seekGen = audioSeekGen();
     const loopGen = host._loopMutationGen();
     const whole = !!opts.whole;
     const r = _sectionPracticeResolveLoopTarget(index, opts);
@@ -1045,7 +1047,7 @@ export async function practiceSection(index, opts = {}) {
     let ok = false;
     for (let attempt = 0; attempt < 5; attempt++) {
         // A newer click or a song/arrangement change supersedes this retry.
-        if (requestGen !== _sectionPracticeRequestGen || seekGen !== host._audioSeekGen() || loopGen !== host._loopMutationGen()) return;
+        if (requestGen !== _sectionPracticeRequestGen || seekGen !== audioSeekGen() || loopGen !== host._loopMutationGen()) return;
         try {
             // skipSectionSync: this function owns the section-practice state and
             // applies it below under the request-gen guard, so a stale retry
@@ -1055,7 +1057,7 @@ export async function practiceSection(index, opts = {}) {
             // after its internal seek await, so a stale loop is never armed.
             ok = await host.setLoop(start, end, {
                 skipSectionSync: true,
-                commitGuard: () => requestGen === _sectionPracticeRequestGen && seekGen === host._audioSeekGen() && loopGen === host._loopMutationGen(),
+                commitGuard: () => requestGen === _sectionPracticeRequestGen && seekGen === audioSeekGen() && loopGen === host._loopMutationGen(),
             });
         } catch (err) {
             ok = false;
@@ -1064,7 +1066,7 @@ export async function practiceSection(index, opts = {}) {
         await new Promise(res => setTimeout(res, 60 + attempt * 90));
     }
     // Re-check after the awaited retries before applying any loop/count-in state.
-    if (requestGen !== _sectionPracticeRequestGen || seekGen !== host._audioSeekGen() || loopGen !== host._loopMutationGen()) return;
+    if (requestGen !== _sectionPracticeRequestGen || seekGen !== audioSeekGen() || loopGen !== host._loopMutationGen()) return;
 
     if (ok) {
         _sectionPracticeWholeSection = whole;
@@ -1073,7 +1075,7 @@ export async function practiceSection(index, opts = {}) {
             _sectionPracticeSavedPartIndex = index;
         }
         _blurSectionPracticeFocusIfNeeded();
-        _updateSectionPracticeHighlight(host._audioTime());
+        _updateSectionPracticeHighlight(_audioTime());
         host.startCountIn({ immediate: true });
     } else {
         _setSectionPracticeMode(false, { skipClearLoop: true });
@@ -1120,7 +1122,7 @@ export function _syncSectionPracticeFromLoop() {
     } else if (_sectionPracticeMode) {
         _setSectionPracticeMode(false, { skipClearLoop: true });
     }
-    _updateSectionPracticeHighlight(host._audioTime());
+    _updateSectionPracticeHighlight(_audioTime());
 }
 
 function _sectionPracticeIndexAtTime(t) {

static/js/transport.js

@@ -0,0 +1,377 @@
+// The playback transport — the play/pause/seek core, and the two clocks it reads.
+//
+// WHY THIS IS A MODULE AND NOT A HOOK BUNDLE. Every carve before this one ADDED host
+// hooks: a module pulled out of app.js still had to call back into it. This one SUBTRACTS
+// them. count-in, juce-audio, loops, and section-practice were all reaching through the
+// seam for the same handful of names — _audioSeek, _audioTime, setPlayButtonState,
+// _songEventPayload, jucePlayer. Those names have an owner, and it isn't app.js. Give
+// them one and the four consumers import them directly:
+//
+//     count-in.js    5 hooks -> 0        juce-audio.js  4 hooks -> 0
+//     loops.js       6 hooks -> 4        section-practice.js  10 hooks -> 7
+//
+// A hook is a cycle you agreed to live with. An import is a dependency you actually have.
+// Prefer the import whenever the name has a real owner.
+//
+// TWO THINGS DELIBERATELY LEFT IN app.js, both for the same reason — they would close a
+// cycle, and app.js is the root, so it can import from both sides for free:
+//
+//   * _currentPlaybackSnapshot  reads loopA/loopB from ./loops.js, and loops.js imports
+//                               this module. The dependency scan MISSED this at first: it
+//                               only walked app.js's own top-level decls, and loopA stopped
+//                               being one the moment loops.js was carved out. Any scan of a
+//                               partly-carved monolith has to resolve the imports too.
+//   * restartCurrentSong        calls _cancelCountIn() from ./count-in.js, which imports
+//                               this module.
+//
+// The seek generation (_audioSeekGen) stays PRIVATE. It has exactly one writer —
+// _resetAudioSeekState(), right here — so readers get audioSeekGen() and nobody outside
+// can desync it. That is strictly better than the host hook it replaces, which handed out
+// a getter and left the writer in app.js.
+import { audio } from './audio-el.js';
+import { S } from './player-state.js';
+
+// Sync the play/pause button's icon and accessible state in one place so
+// screen readers, tooltips, and aria-pressed stay aligned with playback.
+// Updates the existing <img> child's src in place rather than rewriting
+// innerHTML, so any future children (fallback label, loading spinner, …)
+// survive state changes.
+export function setPlayButtonState(isPlaying) {
+    const btn = document.getElementById('btn-play');
+    if (!btn) return;
+    const label = isPlaying ? 'Pause' : 'Play';
+    const icon = isPlaying ? 'pause' : 'play';
+    let img = btn.querySelector('img.button-icon-svg');
+    if (!img) {
+        img = document.createElement('img');
+        img.className = 'button-icon-svg';
+        img.alt = '';
+        img.setAttribute('aria-hidden', 'true');
+        btn.appendChild(img);
+    }
+    img.src = `/static/svg/${icon}.svg`;
+    btn.setAttribute('aria-label', label);
+    btn.setAttribute('aria-pressed', isPlaying ? 'true' : 'false');
+    btn.title = label;
+}
+
+// ── Player ───────────────────────────────────────────────────────────────
+// `audio` now lives in ./js/audio-el.js so carved-out modules can reach the
+// player without importing app.js back (which would close a cycle). Same
+// element, same handle, same lookup — just imported instead of declared here.
+let _lastSongPositionEventAt = 0;
+
+export function _emitSongPositionChanged(time, duration) {
+    const now = Date.now();
+    if (now - _lastSongPositionEventAt < 250) return;
+    _lastSongPositionEventAt = now;
+    const payload = (typeof _songEventPayload === 'function') ? _songEventPayload() : { time };
+    window.feedBack.emit('song:position-changed', Object.assign(payload, { duration }));
+}
+
+export const jucePlayer = {
+    _timer: null,
+    _pos: 0,
+    _dur: 0,
+    _pollAt: 0,    // performance.now() when _pos was last set
+    _polling: false,
+    _speed: 1,
+    get currentTime() {
+        if (!this._polling) return this._pos;
+        // Interpolate between IPC polls so highway motion is smooth at 60fps
+        // Scale by _speed so at 0.7x the interpolated clock advances 0.7s/s
+        const elapsed = (performance.now() - this._pollAt) / 1000;
+        return Math.min(this._pos + elapsed * this._speed, this._dur > 0 ? this._dur : Infinity);
+    },
+    get duration() { return this._dur; },
+    async play() {
+        try {
+            await window.feedBackDesktop.audio.startBacking();
+        } catch (err) {
+            console.warn('[jucePlayer] startBacking failed:', err);
+            return false;
+        }
+        this._startPolling();
+        return true;
+    },
+    async pause() {
+        // Snapshot the interpolated position before stopping the poll so
+        // _pos stays at the visible pause point rather than jumping back
+        // to the last raw IPC sample (which can be up to 100ms behind).
+        this._pos = this.currentTime;
+        this._pollAt = performance.now();
+        this._stopPolling();
+        try {
+            await window.feedBackDesktop.audio.stopBacking();
+        } catch (err) {
+            console.warn('[jucePlayer] stopBacking failed:', err);
+        }
+    },
+    async seek(s) {
+        const prev = this._pos;
+        this._pos = s;
+        this._pollAt = performance.now();
+        try {
+            await window.feedBackDesktop.audio.seekBacking(s);
+        } catch (err) {
+            console.warn('[jucePlayer] seekBacking failed:', err);
+            this._pos = prev;
+            this._pollAt = performance.now();
+        }
+    },
+    _startPolling() {
+        this._stopPolling();
+        this._polling = true;
+        this._pollAt = performance.now();
+        const self = this;
+        function scheduleNext() {
+            self._timer = setTimeout(async () => {
+                if (!self._polling) return;
+                try {
+                    self._pos = await window.feedBackDesktop.audio.getBackingPosition();
+                    self._pollAt = performance.now();
+                    _emitSongPositionChanged(self.currentTime, self.duration || null);
+                } catch (err) {
+                    console.warn('[jucePlayer] position poll failed:', err);
+                } finally {
+                    if (self._polling) scheduleNext();
+                }
+            }, 100);
+        }
+        scheduleNext();
+    },
+    _stopPolling() {
+        this._polling = false;
+        if (this._timer) { clearTimeout(this._timer); this._timer = null; }
+    },
+    setRate(rate) {
+        this._pos = this.currentTime;
+        this._pollAt = performance.now();
+        this._speed = rate;
+    },
+    async stop() {
+        await this.pause();
+        this._pos = 0;
+        this._dur = 0;
+        this._pollAt = 0;
+        this._speed = 1;
+    },
+};
+
+export function _audioTime() { return window._juceMode ? jucePlayer.currentTime : audio.currentTime; }
+
+export function _audioDuration() { return window._juceMode ? jucePlayer.duration : audio.duration; }
+
+// Canonical payload for song:play/song:pause/song:ended. Plugins anchor
+// their own clocks against `perfNow` (a monotonic timestamp at the same
+// moment audio reports `audioT`) so they don't have to chase the chart
+// clock with a follow-up call. `time` is kept as an alias for `audioT`
+// because pre-existing plugins read e.detail.time.
+export function _songEventPayload() {
+    const audioT = _audioTime();
+    return {
+        time: audioT,
+        audioT,
+        chartT: highway.getTime(),
+        perfNow: performance.now(),
+    };
+}
+
+export function _markPlaybackPaused() {
+    S.isPlaying = false;
+    setPlayButtonState(false);
+    if (window.feedBack) {
+        window.feedBack.isPlaying = false;
+        window.feedBack.emit('song:pause', _songEventPayload());
+    }
+}
+
+export function _markPlaybackResumed() {
+    S.isPlaying = true;
+    setPlayButtonState(true);
+    if (window.feedBack) {
+        window.feedBack.isPlaying = true;
+        const payload = _songEventPayload();
+        window.feedBack.emit('song:play', payload);
+        window.feedBack.emit('song:resume', payload);
+    }
+}
+
+export function _emitPlaybackStopped(time, screen = 'playback-command') {
+    if (window.feedBack) window.feedBack.emit('song:stop', { time: time || 0, screen });
+}
+
+export function _waitForSongReady(expectedSeekGen, timeoutMs = 10000) {
+    if (!window.feedBack || typeof window.feedBack.on !== 'function') return Promise.resolve(false);
+    return new Promise(resolve => {
+        let timer = null;
+        const done = value => {
+            if (timer !== null) clearTimeout(timer);
+            window.feedBack.off('song:ready', onReady);
+            resolve(value);
+        };
+        const onReady = () => done(expectedSeekGen == null || expectedSeekGen === _audioSeekGen);
+        window.feedBack.on('song:ready', onReady);
+        timer = setTimeout(() => done(false), timeoutMs);
+    });
+}
+
+// Serializes seeks so concurrent callers (e.g. user ⏪ during a loop wrap)
+// don't interleave their from/to reads — each call captures `from` only
+// once the previous seek + emit have completed. The generation token
+// lets session teardown invalidate queued seeks so they don't run against
+// the new player and emit a stale song:seek.
+let _audioSeekChain = Promise.resolve();
+
+let _audioSeekGen = 0;
+
+export function _resetAudioSeekState() {
+    // Bump the generation — in-flight chain callbacks see the mismatch on
+    // their next guard check and short-circuit (no emit, no further state
+    // mutation by us). Don't reset the chain head: new seeks must still
+    // queue behind the in-flight old seek's IPC so two `jucePlayer.seek()`
+    // calls can't race in the JUCE backing engine. The queue drains
+    // quickly because each subsequent old-gen step bails on the first
+    // guard the moment its predecessor resolves.
+    _audioSeekGen++;
+}
+
+// Time-box the JUCE IPC so a single hung seek can't block the global
+// _audioSeekChain forever (which would freeze every subsequent reposition
+// path: seekBy, loop-wrap, jump-fix, shimmed audio.currentTime).
+const _JUCE_SEEK_TIMEOUT_MS = 2000;
+
+function _juceSeekWithTimeout(s) {
+    let timer;
+    const seekP = jucePlayer.seek(s);
+    const timeoutP = new Promise((_, reject) => {
+        timer = setTimeout(() => reject(new Error('JUCE seek timed out')), _JUCE_SEEK_TIMEOUT_MS);
+    });
+    // Clear the timer once the race settles either way; without this the
+    // pending timeout keeps the event loop alive (and eventually rejects
+    // an unawaited promise) even after a successful seek.
+    return Promise.race([seekP, timeoutP]).finally(() => clearTimeout(timer));
+}
+
+// Resolves to `{ completed, from, to }`:
+//   - completed: true if the seek ran to completion and emitted song:seek;
+//                false if cancelled by a teardown gen bump (or threw).
+//   - from: chart clock just before the seek (NaN on cancel before from-read).
+//   - to:   verified post-seek clock (NaN on cancel/throw).
+// Callers that fire follow-up work after the seek (count-in, arrangement
+// restore, etc.) should check `completed` so they don't act on a torn-down
+// session. Callers that need the actual landed position (because JUCE may
+// clamp or HTML5 may snap to the seekable range) should read `to` rather
+// than re-using the requested `s`.
+export async function _audioSeek(s, reason) {
+    // Single funnel for every audio repositioning. Emits song:seek so
+    // plugins (notedetect detection-suppression during seek transients,
+    // practice-journal segment tracking) can react to any chart-time
+    // jump regardless of which UI path triggered it. `reason` is a
+    // free-form short string ('seek-by', 'loop-wrap', 'loop-set',
+    // 'arrangement-restore', 'jump-fix') so subscribers can filter.
+    const gen = _audioSeekGen;
+    _audioSeekChain = _audioSeekChain.then(async () => {
+        if (gen !== _audioSeekGen) return { completed: false, from: NaN, to: NaN };
+        const from = _audioTime();
+        if (window._juceMode) await _juceSeekWithTimeout(s);
+        else audio.currentTime = s;
+        if (gen !== _audioSeekGen) return { completed: false, from, to: NaN };
+        // Read the verified post-seek position rather than the requested `s`
+        // so plugins observe the actual clock — JUCE may clamp or roll back,
+        // and HTML5 may snap to the nearest seekable range.
+        const to = _audioTime();
+        // Sync the jump-fix tracker so the next 60Hz tick doesn't see a
+        // legitimate far seek (e.g. saved-loop jump > 30s) as a browser
+        // bug and revert it.
+        S.lastAudioTime = to;
+        // Sync the chart clock too so any song:* emit fired right after
+        // _audioSeek resolves (e.g. the auto-resume song:play in
+        // changeArrangement) sees an in-sync chartT via _songEventPayload.
+        // Without this, chartT lags by one 60Hz tick after a seek.
+        if (typeof highway !== 'undefined' && highway && typeof highway.setTime === 'function') {
+            highway.setTime(to);
+        }
+        window.feedBack.emit('song:seek', { from, to, reason: reason || null });
+        return { completed: true, from, to };
+    }).catch((err) => {
+        // Don't let one failed seek poison subsequent ones.
+        console.warn('[_audioSeek]', err);
+        return { completed: false, from: NaN, to: NaN };
+    });
+    return _audioSeekChain;
+}
+
+// Per-attempt counter for HTML5 audio.play() invocations. Bumped on
+// every play branch entry so a slow rejection from attempt N can't
+// clobber the UI of a newer attempt N+1 within the same session.
+let _playAttemptGen = 0;
+
+export async function togglePlay() {
+    if (window._juceMode) {
+        if (S.isPlaying) {
+            await jucePlayer.pause();
+            S.isPlaying = false;
+            setPlayButtonState(false);
+            window.feedBack.isPlaying = false;
+            window.feedBack.emit('song:pause', _songEventPayload());
+        } else {
+            const started = await jucePlayer.play();
+            if (!started) return; // startBacking() failed — IPC error already logged
+            S.isPlaying = true;
+            setPlayButtonState(true);
+            window.feedBack.isPlaying = true;
+            const payload = _songEventPayload();
+            window.feedBack.emit('song:play', payload);
+            window.feedBack.emit('song:resume', payload);
+        }
+        return;
+    }
+    if (S.isPlaying) {
+        audio.pause(); S.isPlaying = false;
+        setPlayButtonState(false);
+    } else {
+        // Flip the UI optimistically before awaiting the play() Promise so
+        // a quick second click during a slow start (buffering, device
+        // wake, etc.) still enters the pause branch above. Two stale-
+        // resolution guards:
+        //   - _audioSeekGen: bumped in showScreen() teardown and
+        //     playSong(), so a rejection from a torn-down session can't
+        //     touch new-session UI. Survives same-URL reloads.
+        //   - _playAttemptGen: bumped on every play branch entry, so
+        //     within a single session a slow rejection from attempt N
+        //     can't clobber a faster attempt N+1 (Play → Pause → Play).
+        const sessionGen = _audioSeekGen;
+        const attempt = ++_playAttemptGen;
+        S.isPlaying = true;
+        setPlayButtonState(true);
+        try {
+            await audio.play();
+        } catch (err) {
+            if (sessionGen !== _audioSeekGen) return;
+            if (attempt !== _playAttemptGen) return;
+            // An engine reroute (HTML5 -> JUCE) deliberately pauses the <audio>
+            // element mid-migration, which rejects this in-flight play() with an
+            // AbortError even though playback continues on the JUCE transport.
+            // The reroute owns isPlaying / the button while it runs (same guard
+            // the <audio> 'play'/'pause' listeners use); resetting here would
+            // leave the button showing Play while the song keeps playing — the
+            // "two clicks to pause on the first song after a fresh load" bug.
+            if (window._juceRerouteInProgress) return;
+            console.error('[app] audio.play() rejected:', err);
+            S.isPlaying = false;
+            setPlayButtonState(false);
+        }
+    }
+}
+
+export async function seekBy(s) {
+    await _audioSeek(Math.max(0, _audioTime() + s), 'seek-by');
+}
+
+/**
+ * Read-only view of the seek generation. Bumped by _resetAudioSeekState() on session
+ * teardown; callers capture it before an await and compare after, so a resolution from a
+ * torn-down session can't touch new-session state.
+ */
+export function audioSeekGen() { return _audioSeekGen; }

tests/conftest.py

@@ -1,6 +1,8 @@
 """Shared pytest fixtures for the feedBack test suite."""
 
+import importlib
 import logging
+import sys
 
 import pytest
 import structlog
@@ -76,3 +78,92 @@ def isolate_logging():
         lg.setLevel(original_level)
         lg.propagate = original_propagate
     structlog.reset_defaults()
+
+
+# ── Plugin-loader isolation ─────────────────────────────────────────────────────
+#
+# Lifted verbatim out of tests/test_plugins.py so more than one test module can drive
+# the real plugins.load_plugins(). It has to be ONE fixture, not a copy per file:
+# load_plugins() mutates sys.path, sys.modules, PENDING_PLUGINS and LOADED_PLUGINS, and a
+# partial restore makes the suite order- and environment-dependent (Codex [P2] on
+# test_plugin_context_contract.py — it was right).
+
+# Bare module names that this test module pre-populates into
+# sys.modules to simulate the bare-import path. Saved/restored by
+# the reset_plugin_state fixture so they don't leak to other test
+# files. Codex / Copilot review on PR for feedBack#33.
+_BARE_NAMES_USED = ("util", "extractor")
+
+
+@pytest.fixture()
+def reset_plugin_state(monkeypatch):
+    """Clear loader module-level state and restore on teardown.
+
+    Saves and restores:
+      * `plugins.LOADED_PLUGINS`
+      * any `plugin_*` keys we add to `sys.modules`
+      * the bare names this module simulates (`util`, `extractor`)
+      * `sys.path` — `plugins.load_plugins()` mutates it
+    Also unsets `FEEDBACK_PLUGINS_DIR` for the test's duration
+    (via monkeypatch) so a CI env that pre-sets it can't leak
+    real user plugins into a tmp_path-driven test. Per-module
+    locks are owned by the standard import system
+    (`importlib._bootstrap._module_locks`) and are not our
+    responsibility to reset.
+    """
+    monkeypatch.delenv("FEEDBACK_PLUGINS_DIR", raising=False)
+    plugins = importlib.import_module("plugins")
+    saved_loaded = list(plugins.LOADED_PLUGINS)
+    saved_pending = dict(plugins.PENDING_PLUGINS)
+    saved_modules = {k: v for k, v in sys.modules.items() if k.startswith("plugin_")}
+    saved_bare = {k: sys.modules[k] for k in _BARE_NAMES_USED if k in sys.modules}
+    saved_path = list(sys.path)
+    plugins.LOADED_PLUGINS.clear()
+    plugins.PENDING_PLUGINS.clear()
+    for k in list(sys.modules):
+        if k.startswith("plugin_") or k in _BARE_NAMES_USED:
+            del sys.modules[k]
+    try:
+        yield plugins
+    finally:
+        plugins.LOADED_PLUGINS.clear()
+        plugins.LOADED_PLUGINS.extend(saved_loaded)
+        plugins.PENDING_PLUGINS.clear()
+        plugins.PENDING_PLUGINS.update(saved_pending)
+        for k in list(sys.modules):
+            if k.startswith("plugin_") or k in _BARE_NAMES_USED:
+                del sys.modules[k]
+        sys.modules.update(saved_modules)
+        sys.modules.update(saved_bare)
+        sys.path[:] = saved_path
+
+
+# ── Scanner isolation ───────────────────────────────────────────────────────────
+#
+# lib/scan.py holds MODULE-LEVEL state (_scan_status, and the kick/runner bookkeeping),
+# and `scan` is NOT re-imported by the fixtures that re-import `server` — so unlike the
+# old server-globals arrangement, that state now outlives a test.
+#
+# It matters because of a deliberate asymmetry in the scanner: background_scan() never
+# sets `running` back to False. Ownership of that flag lives in _scan_runner, so that a
+# kick_scan() racing the terminal write cannot see a stale False and start a second runner.
+# Correct in production — but a test that calls background_scan() DIRECTLY skips the runner
+# entirely and therefore leaves the scanner marked "running" forever. Every later scan or
+# rescan then returns "already in progress" and quietly does nothing.
+#
+# The suite passed anyway, on ordering luck. Codex [P2] caught it. So: snapshot and restore.
+@pytest.fixture()
+def reset_scan_state():
+    """Restore lib/scan.py's module-level state around a test that drives it directly."""
+    import scan
+
+    saved_status = scan._scan_status
+    saved_thread = scan._scan_thread
+    saved_pending = scan._scan_rescan_pending
+    scan._scan_status = dict(scan._SCAN_STATUS_INIT)
+    try:
+        yield scan
+    finally:
+        scan._scan_status = saved_status
+        scan._scan_thread = saved_thread
+        scan._scan_rescan_pending = saved_pending

tests/js/autoplay_exit.test.js

@@ -14,10 +14,16 @@ const vm = require('node:vm');
 const { extractFunction } = require('./test_utils');
 
 const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js');
+// _autoplayExitEnabled was carved out into static/js/player-controls.js (R3a); the
+// auto-exit machinery around it (_clearAutoExit, holdAutoExit, _resolvePlayerOrigin)
+// stayed in app.js.
+const CONTROLS_JS = path.join(__dirname, '..', '..', 'static', 'js', 'player-controls.js');
 const SRC = fs.readFileSync(APP_JS, 'utf8');
+// the module is ESM; these sandboxes evaluate plain script text
+const CONTROLS_SRC = fs.readFileSync(CONTROLS_JS, 'utf8').replace(/^export /gm, '');
 
 function runEnabled(stored) {
-    const fnSrc = extractFunction(SRC, 'function _autoplayExitEnabled(');
+    const fnSrc = extractFunction(CONTROLS_SRC, 'function _autoplayExitEnabled(');
     const sandbox = {
         localStorage: {
             getItem: () => {

tests/js/juce_engine_reroute.test.js

@@ -1,4 +1,4 @@
-// Behavioral tests for the JUCE engine-reroute watcher in static/app.js.
+// Behavioral tests for the JUCE engine-reroute watcher in static/js/juce-audio.js.
 //
 // The watcher (an IIFE, `_installJuceEngineRoutingWatcher`) migrates a loaded
 // song between the HTML5 <audio> element and the native JUCE backing transport
@@ -14,14 +14,15 @@ const fs = require('node:fs');
 const path = require('node:path');
 const vm = require('node:vm');
 
-const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js');
+// The JUCE audio shims were carved out of app.js into their own module (R3a).
+const APP_JS = path.join(__dirname, '..', '..', 'static', 'js', 'juce-audio.js');
 
 // Brace-balanced extraction of the watcher IIFE, starting at its `(function`
 // and ending after the matching `})();`.
 function extractWatcherIIFE(src) {
     const marker = '(function _installJuceEngineRoutingWatcher() {';
     const start = src.indexOf(marker);
-    assert.ok(start !== -1, 'watcher IIFE not found in app.js');
+    assert.ok(start !== -1, 'watcher IIFE not found in static/js/juce-audio.js');
     const openBrace = src.indexOf('{', start);
     let depth = 1;
     let i = openBrace + 1;
@@ -100,6 +101,17 @@ function makeSandbox({ isAudioRunning, loadBackingTrack, outputType = 'Windows A
 
     const src = fs.readFileSync(APP_JS, 'utf8');
     const iife = extractWatcherIIFE(src);
+    // The shims reach back into app.js through the host seam (static/js/host.js).
+    // Route it at the SAME stubs this sandbox already had — a fresh `() => {}` would
+    // swallow the calls and the assertions below would pass vacuously.
+    sandbox.host = {
+        jucePlayer: () => sandbox.jucePlayer,
+        playSong: (...a) => (sandbox.playSong ? sandbox.playSong(...a) : undefined),
+        _audioSeek: (...a) => (sandbox._audioSeek ? sandbox._audioSeek(...a) : Promise.resolve({ completed: true })),
+        setPlayButtonState: (...a) => (sandbox.setPlayButtonState ? sandbox.setPlayButtonState(...a) : undefined),
+        _songEventPayload: (...a) => (sandbox._songEventPayload ? sandbox._songEventPayload(...a) : ({})),
+        showScreen: (...a) => (sandbox.showScreen ? sandbox.showScreen(...a) : undefined),
+    };
     vm.createContext(sandbox);
     vm.runInContext(iife, sandbox);
     return sandbox;

tests/js/legacy_shim_hits.test.js

@@ -77,6 +77,11 @@ const PLUGIN_LOADER_JS = path.join(ROOT, 'static', 'js', 'plugin-loader.js');
 // The viz layer was carved out of app.js too (R3a).
 const VIZ_JS = path.join(ROOT, 'static', 'js', 'viz.js');
 const LIBRARY_JS = path.join(ROOT, 'static', 'capabilities', 'library.js');
+// The library itself was carved out of app.js into ./static/js/library.js (R3a). Note the
+// two are DIFFERENT files: LIBRARY_JS above is the capability; this is the UI module.
+// syncLibrarySong deliberately stayed behind in app.js — it reaches showScreen/playSong,
+// and moving it would have dragged the whole playback core into the library module.
+const LIBRARY_MODULE_JS = path.join(ROOT, 'static', 'js', 'library.js');
 
 function source(file) {
     // Normalize CRLF: region() slices fixed CHARACTER windows, so on a
@@ -93,16 +98,20 @@ function region(src, needle, length = 1200) {
 
 test('plugin script hydration exposes the current plugin id for legacy registrations', () => {
     const src = source(PLUGIN_LOADER_JS);
-    const block = region(src, 'script.src = `/api/plugins/${plugin.id}/screen.js');
+    // Anchored on the ASSIGNMENT, not the URL literal: the URL is built in
+    // _pluginScriptUrl() now (#879 — a rollback needs a fresh module URL for the whole
+    // import graph), so the old literal no longer appears at the injection site.
+    const block = region(src, 'script.src = _pluginScriptUrl(');
     assert.match(block, /window\.feedBack\._loadingPluginId\s*=\s*plugin\.id/);
     assert.match(block, /delete\s+window\.feedBack\._loadingPluginId/);
 });
 
 test('library providers route through native library capability', () => {
     const src = source(APP_JS);
+    const libModule = source(LIBRARY_MODULE_JS);
     const librarySrc = source(LIBRARY_JS);
-    const loader = region(src, 'async function loadLibraryProviders', 1800);
-    const selector = region(src, 'async function setLibraryProvider(providerId, options = {})', 1600);
+    const loader = region(libModule, 'async function loadLibraryProviders', 1800);
+    const selector = region(libModule, 'async function setLibraryProvider(providerId, options = {})', 1600);
     const sync = region(src, 'async function syncLibrarySong(providerId, songId', 1600);
 
     assert.match(librarySrc, /capabilities\.registerOwner\(['"]library['"]/);

tests/js/loop_restart.test.js

@@ -14,7 +14,8 @@ const fs = require('node:fs');
 const path = require('node:path');
 const vm = require('node:vm');
 
-const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js');
+// startCountIn was carved out of app.js into its own module (R3a).
+const APP_JS = path.join(__dirname, '..', '..', 'static', 'js', 'count-in.js');
 
 // Pull a function body by declaration prefix (e.g. `async function startCountIn`)
 // and brace-matching to the closing brace. Skips an optional `( ... )` param
@@ -111,12 +112,23 @@ function buildSandbox() {
         __emitCalls: emitCalls,
         queueMicrotask,
     };
+    // startCountIn was carved into static/js/count-in.js and now reaches back into
+    // app.js through the host seam (static/js/host.js). Point the seam at the SAME
+    // stubs the sandbox already had: the assertions below are unchanged, they just
+    // travel through the indirection the real code now uses.
+    sandbox.host = {
+        _audioSeek: (...a) => sandbox._audioSeek(...a),
+        setPlayButtonState: () => {},
+        _songEventPayload: () => ({}),
+        togglePlay: () => {},
+        jucePlayer: () => sandbox.jucePlayer,
+    };
     vm.createContext(sandbox);
     return sandbox;
 }
 
 test('loop:restart fires once when wrap path runs', async () => {
-    const src = fs.readFileSync(APP_JS, 'utf8');
+    const src = fs.readFileSync(APP_JS, 'utf8').replace(/^export /gm, '');
     const startCountInSrc = extractFunction(src, 'async function startCountIn');
 
     // Sanity check: the change under test is present at all. Catches
@@ -167,7 +179,7 @@ test('loop:restart aborts when seek lands far from loopA (JUCE rollback)', async
     // _audioSeek resolves with completed:true but r.to !== loopA. The
     // wrap handler must abort instead of running beginCount on the wrong
     // position and emitting a misleading loop:restart.
-    const src = fs.readFileSync(APP_JS, 'utf8');
+    const src = fs.readFileSync(APP_JS, 'utf8').replace(/^export /gm, '');
     const startCountInSrc = extractFunction(src, 'async function startCountIn');
 
     const sandbox = buildSandbox();
@@ -202,7 +214,7 @@ test('count-in cancellation token bails delayed callbacks (rewindStep + tick)',
     // teardown can interrupt an in-flight count-in. Behavioral simulation
     // of timer cancellation is out of scope for the static extractor; this
     // verifies the contract is wired into the source.
-    const src = fs.readFileSync(APP_JS, 'utf8');
+    const src = fs.readFileSync(APP_JS, 'utf8').replace(/^export /gm, '');
     const fn = extractFunction(src, 'async function startCountIn');
     // Captures gen at entry
     assert.match(fn, /const gen = _countInGen/, 'startCountIn must capture _countInGen at entry');
@@ -218,7 +230,7 @@ test('loop:restart fires after highway.setTime, before beginCount', () => {
     // Source-order assertion on the A-B wrap path only. Section-practice
     // `opts.immediate` also emits loop:restart but is a separate entry path;
     // the wrap handler lives inside the `_audioSeek(loopA, 'loop-wrap')` then.
-    const src = fs.readFileSync(APP_JS, 'utf8');
+    const src = fs.readFileSync(APP_JS, 'utf8').replace(/^export /gm, '');
     const fn = extractFunction(src, 'async function startCountIn');
     const wrapMarker = "_audioSeek(loopA, 'loop-wrap')";
     const wrapStart = fn.indexOf(wrapMarker);

tests/js/play_button_reroute_guard.test.js

@@ -18,7 +18,7 @@ const vm = require('node:vm');
 
 const { extractFunction } = require('./test_utils');
 
-const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js');
+const APP_JS = path.join(__dirname, '..', '..', 'static', 'js', 'transport.js');
 const SRC = fs.readFileSync(APP_JS, 'utf8');
 const TOGGLE_PLAY_SRC = extractFunction(SRC, 'async function togglePlay(');
 

tests/js/playback_app_adapter.test.js

@@ -5,6 +5,10 @@ const path = require('node:path');
 const vm = require('node:vm');
 
 const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js');
+// SPLIT. _installPlaybackTransportAdapter stayed in app.js — it reads loopA/loopB from
+// ./js/loops.js, and loops.js imports transport, so moving it would close a cycle.
+// _waitForSongReady went with the rest of the seek machinery.
+const TRANSPORT_JS = path.join(__dirname, '..', '..', 'static', 'js', 'transport.js');
 
 function extractFunction(src, signature) {
     const start = src.indexOf(signature);
@@ -54,7 +58,7 @@ function loadReadyHelper(sandbox, src) {
 }
 
 test('_waitForSongReady rejects a ready event from a different audio generation', async () => {
-    const src = fs.readFileSync(APP_JS, 'utf8');
+    const src = fs.readFileSync(TRANSPORT_JS, 'utf8');
     const sandbox = buildReadySandbox();
     loadReadyHelper(sandbox, src);
 
@@ -72,7 +76,7 @@ test('playback adapter scopes startTime readiness and validates seek targets', (
     const src = fs.readFileSync(APP_JS, 'utf8');
     const fn = extractFunction(src, 'function _installPlaybackTransportAdapter()');
 
-    assert.match(fn, /const expectedSeekGen\s*=\s*_audioSeekGen\s*\+\s*1;/);
+    assert.match(fn, /const expectedSeekGen\s*=\s*audioSeekGen\(\)\s*\+\s*1;/);
     assert.match(fn, /_waitForSongReady\(expectedSeekGen\)/);
     assert.match(fn, /const seconds\s*=\s*Number\(time\);/);
     assert.match(fn, /!Number\.isFinite\(seconds\)\s*\|\|\s*seconds\s*<\s*0/);

tests/js/plugin_loader_script_type.test.js

@@ -19,13 +19,19 @@ const path = require('node:path');
 const PLUGIN_LOADER_JS = path.join(__dirname, '..', '..', 'static', 'js', 'plugin-loader.js');
 const src = fs.readFileSync(PLUGIN_LOADER_JS, 'utf8');
 
-// Isolate the screen.js <script> injection block: from where its src is built
-// to where the element is appended.
+// Isolate the screen.js <script> injection block: from where its src is assigned to
+// where the element is appended.
+//
+// Anchored on the ASSIGNMENT, not on the URL literal. The URL is built in
+// _pluginScriptUrl() now (#879 — a rollback needs a fresh module URL), so the literal
+// '/api/plugins/${plugin.id}/screen.js' appears FURTHER DOWN the file than the block
+// that uses it, and slicing from it ran off the end of the injection block entirely.
+const SRC_ASSIGN = 'script.src = _pluginScriptUrl(';
 function injectionBlock() {
-    const start = src.indexOf('/api/plugins/${plugin.id}/screen.js');
-    assert.ok(start !== -1, 'screen.js injection src not found — loader moved?');
+    const start = src.indexOf(SRC_ASSIGN);
+    assert.ok(start !== -1, 'screen.js src assignment not found — loader moved?');
     const end = src.indexOf('document.body.appendChild(script)', start);
-    assert.ok(end !== -1, 'appendChild(script) not found after screen.js src');
+    assert.ok(end !== -1, 'appendChild(script) not found after the src assignment');
     return src.slice(start, end);
 }
 
@@ -52,7 +58,7 @@ test('the module type is gated, never set unconditionally', () => {
 
 test('the module guard sits before appendChild, after the src assignment', () => {
     const guardAt = src.indexOf('script.type = \'module\'');
-    const srcAt = src.indexOf('/api/plugins/${plugin.id}/screen.js');
+    const srcAt = src.indexOf(SRC_ASSIGN);
     const appendAt = src.indexOf('document.body.appendChild(script)', srcAt);
     assert.ok(guardAt > srcAt && guardAt < appendAt,
         'the module guard must live inside the screen.js injection block');

tests/js/plugin_module_rollback.test.js

@@ -0,0 +1,83 @@
+// #879 — a plugin ROLLBACK must actually re-evaluate a module plugin.
+//
+// ES modules are evaluated once per URL per document. Re-inserting a
+// <script type="module"> whose src the module map has already seen fires `load` but
+// does NOT re-run the body — so rolling back to a version already evaluated this
+// session left the OLD module live while the loader recorded success.
+//
+// The fix puts a generation token in the PATH (/api/plugins/x/g/7/screen.js), not the
+// query, because a relative specifier resolves against the base URL with the query
+// DROPPED — so './src/main.js' would otherwise keep resolving to the same cached URL
+// and the plugin's actual code would never re-run.
+
+const { test } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+const vm = require('node:vm');
+
+const { extractFunction } = require('./test_utils');
+const LOADER = path.join(__dirname, '..', '..', 'static', 'js', 'plugin-loader.js');
+
+function makeUrlBuilder() {
+    const src = fs.readFileSync(LOADER, 'utf8');
+    const sandbox = { _evaluatedModules: new Set(), _moduleReloadSeq: 0 };
+    vm.createContext(sandbox);
+    vm.runInContext(`
+        ${extractFunction(src, 'function _pluginScriptUrl(')}
+        globalThis.url = _pluginScriptUrl;
+    `, sandbox);
+    return sandbox.url;
+}
+
+const MOD = { id: 'editor', script_type: 'module' };
+const CLASSIC = { id: 'legacy', script_type: 'classic' };
+
+test('a module plugin first load uses the stable ?v= URL (ETag/304 stays intact)', () => {
+    const url = makeUrlBuilder();
+    assert.equal(url(MOD, '1.0.0', '?v=1.0.0'), '/api/plugins/editor/screen.js?v=1.0.0');
+});
+
+// An UPGRADE has to bust the graph too, and this is the part #879 got wrong. It says
+// "upgrades are fine — a new version yields a new URL". True of screen.js; FALSE of the
+// plugin. Driving a real browser through install -> upgrade -> rollback and counting
+// evaluations of src/main.js gives ONE: the upgrade re-runs the one-line screen.js shim
+// at its new ?v= URL, the shim imports './src/main.js', that resolves to the SAME url,
+// and the module map hands back the already-evaluated old module. So the key here is the
+// plugin ID, not id@version — every re-load of a module plugin needs a fresh path.
+test('an UPGRADE also gets a fresh /g/<n>/ path — a new ?v= does NOT reach the graph', () => {
+    const url = makeUrlBuilder();
+    url(MOD, '1.0.0', '?v=1.0.0');
+    assert.equal(url(MOD, '1.1.0', '?v=1.1.0'), '/api/plugins/editor/g/1/screen.js?v=1.1.0');
+});
+
+test('a ROLLBACK to an already-evaluated version gets a fresh /g/<n>/ PATH', () => {
+    const url = makeUrlBuilder();
+    url(MOD, '1.0.0', '?v=1.0.0');                        // installed
+    url(MOD, '1.1.0', '?v=1.1.0');                        // upgraded   -> /g/1/
+    const back = url(MOD, '1.0.0', '?v=1.0.0');           // rolled back -> /g/2/
+    assert.equal(back, '/api/plugins/editor/g/2/screen.js?v=1.0.0');
+
+    // The token must be in the PATH so a relative import INHERITS it — the whole point.
+    // A query token is dropped by URL resolution and never reaches src/main.js.
+    const resolved = new URL('./src/main.js', `http://h${back}`).pathname;
+    assert.equal(resolved, '/api/plugins/editor/g/2/src/main.js',
+        'the token must reach the module GRAPH, not just the entry point');
+});
+
+test('every re-load gets a distinct URL (no reuse across a bounce)', () => {
+    const url = makeUrlBuilder();
+    url(MOD, '1.0.0', '?v=1.0.0');
+    const seen = new Set();
+    for (const v of ['1.1.0', '1.0.0', '1.1.0', '1.0.0']) seen.add(url(MOD, v, `?v=${v}`));
+    assert.equal(seen.size, 4, 'each re-load must be a URL the module map has never seen');
+});
+
+test('classic-script plugins are untouched — they always re-run on re-insert', () => {
+    const url = makeUrlBuilder();
+    const first = url(CLASSIC, '1.0.0', '?v=1.0.0');
+    url(CLASSIC, '1.1.0', '?v=1.1.0');
+    const back = url(CLASSIC, '1.0.0', '?v=1.0.0');
+    assert.equal(first, '/api/plugins/legacy/screen.js?v=1.0.0');
+    assert.equal(back, first, 'a classic script needs no cache-busting and must not get a /g/ path');
+});

tests/js/renderer_bus_feeder.test.js

@@ -1,4 +1,4 @@
-// Behavioral tests for the renderer-audio bus feeder in static/app.js.
+// Behavioral tests for the renderer-audio bus feeder in static/js/juce-audio.js.
 //
 // The feeder (an IIFE, `_installRendererBusFeeder`) captures renderer-side
 // song audio (stems-plugin WebAudio master, or the core <audio> element) and
@@ -16,12 +16,13 @@ const fs = require('node:fs');
 const path = require('node:path');
 const vm = require('node:vm');
 
-const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js');
+// The JUCE audio shims were carved out of app.js into their own module (R3a).
+const APP_JS = path.join(__dirname, '..', '..', 'static', 'js', 'juce-audio.js');
 
 function extractFeederIIFE(src) {
     const marker = '(function _installRendererBusFeeder() {';
     const start = src.indexOf(marker);
-    assert.ok(start !== -1, 'feeder IIFE not found in app.js');
+    assert.ok(start !== -1, 'feeder IIFE not found in static/js/juce-audio.js');
     const openBrace = src.indexOf('{', start);
     let depth = 1;
     let i = openBrace + 1;
@@ -123,6 +124,17 @@ function makeSandbox({ isAudioRunning = () => true, exclusive = () => true, disp
     sandbox.globalThis = sandbox;
 
     const src = fs.readFileSync(APP_JS, 'utf8');
+    // The shims reach back into app.js through the host seam (static/js/host.js).
+    // Route it at the SAME stubs this sandbox already had — a fresh `() => {}` would
+    // swallow the calls and the assertions below would pass vacuously.
+    sandbox.host = {
+        jucePlayer: () => sandbox.jucePlayer,
+        playSong: (...a) => (sandbox.playSong ? sandbox.playSong(...a) : undefined),
+        _audioSeek: (...a) => (sandbox._audioSeek ? sandbox._audioSeek(...a) : Promise.resolve({ completed: true })),
+        setPlayButtonState: (...a) => (sandbox.setPlayButtonState ? sandbox.setPlayButtonState(...a) : undefined),
+        _songEventPayload: (...a) => (sandbox._songEventPayload ? sandbox._songEventPayload(...a) : ({})),
+        showScreen: (...a) => (sandbox.showScreen ? sandbox.showScreen(...a) : undefined),
+    };
     vm.createContext(sandbox);
     vm.runInContext(extractFeederIIFE(src), sandbox);
     assert.equal(typeof sandbox.window._reevaluateRendererBus, 'function',

tests/js/song_credits_overlay.test.js

@@ -14,8 +14,9 @@ const vm = require('node:vm');
 
 const { extractFunction } = require('./test_utils');
 
-const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js');
-const SRC = fs.readFileSync(APP_JS, 'utf8');
+// the song-credits overlay was carved out of app.js into its own module (R3a).
+const APP_JS = path.join(__dirname, '..', '..', 'static', 'js', 'count-in.js');
+const SRC = fs.readFileSync(APP_JS, 'utf8').replace(/^export /gm, '');
 
 // Minimal fake DOM element: records className, children, and textContent.
 // Setting textContent clears children (matching real DOM) so we can assert

tests/js/song_event_payload.test.js

@@ -12,7 +12,7 @@ const fs = require('node:fs');
 const path = require('node:path');
 const vm = require('node:vm');
 
-const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js');
+const APP_JS = path.join(__dirname, '..', '..', 'static', 'js', 'transport.js');
 
 function extractFunction(src, signature) {
     const start = src.indexOf(signature);
@@ -129,11 +129,24 @@ test('every song:play/pause/ended emit uses _songEventPayload', () => {
     );
 });
 
+// CENSUS over the WHOLE frontend, not one file. This test counts call/emit sites, and the
+// carve keeps moving them between app.js and static/js/*.js — point it at a single file
+// and the count silently shrinks as code leaves, which reads as "someone deleted an emit"
+// (or, worse, passes while genuinely missing sites). Read every source that can hold one.
+function allFrontendSources() {
+    const jsDir = path.join(__dirname, '..', '..', 'static', 'js');
+    const parts = [fs.readFileSync(path.join(__dirname, '..', '..', 'static', 'app.js'), 'utf8')];
+    for (const f of fs.readdirSync(jsDir).sort()) {
+        if (f.endsWith('.js')) parts.push(fs.readFileSync(path.join(jsDir, f), 'utf8'));
+    }
+    return parts.join('\n');
+}
+
 test('there are at least 8 song:* emit sites threaded through the helper', () => {
     // Sanity-check that the helper actually got wired everywhere. If the
     // count drops, someone removed an emit (regression) or refactored an
     // event away (intentional — this test then needs updating).
-    const src = fs.readFileSync(APP_JS, 'utf8');
+    const src = allFrontendSources();
     const matches = src.match(/(?:window\.feedBack|\w+)\.emit\(\s*['"]song:(play|pause|ended)['"][^)]*\)/g) || [];
     assert.ok(
         matches.length >= 8,

tests/js/song_seek.test.js

@@ -1,4 +1,4 @@
-// Verify static/app.js emits `song:seek` for every audio repositioning,
+// Verify static/js/transport.js emits `song:seek` for every audio repositioning,
 // with `{ from, to, reason }` payload. Plugins (notedetect detection-
 // suppression during seek transients) consume this contract.
 //
@@ -11,7 +11,7 @@ const fs = require('node:fs');
 const path = require('node:path');
 const vm = require('node:vm');
 
-const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js');
+const APP_JS = path.join(__dirname, '..', '..', 'static', 'js', 'transport.js');
 
 function extractFunction(src, signature) {
     const start = src.indexOf(signature);
@@ -287,13 +287,26 @@ test('seekBy floors at zero (does not seek to negative time)', async () => {
     assert.equal(seek.detail.to, 0);
 });
 
+// CENSUS over the WHOLE frontend, not one file. This test counts call/emit sites, and the
+// carve keeps moving them between app.js and static/js/*.js — point it at a single file
+// and the count silently shrinks as code leaves, which reads as "someone deleted an emit"
+// (or, worse, passes while genuinely missing sites). Read every source that can hold one.
+function allFrontendSources() {
+    const jsDir = path.join(__dirname, '..', '..', 'static', 'js');
+    const parts = [fs.readFileSync(path.join(__dirname, '..', '..', 'static', 'app.js'), 'utf8')];
+    for (const f of fs.readdirSync(jsDir).sort()) {
+        if (f.endsWith('.js')) parts.push(fs.readFileSync(path.join(jsDir, f), 'utf8'));
+    }
+    return parts.join('\n');
+}
+
 test('every documented seek callsite passes a reason', () => {
     // Source-order assertion: every _audioSeek call outside the
     // implementation must pass a kebab-case reason string. Catches a
     // future contributor adding a new seek path without threading the
     // reason. Line-based — regex argument capture can't balance parens
     // through Math.max/_audioTime calls.
-    const src = fs.readFileSync(APP_JS, 'utf8');
+    const src = allFrontendSources();
     const fnSrc = extractFunction(src, 'async function _audioSeek(');
     const withoutImpl = src.replace(fnSrc, '');
     const callLines = withoutImpl.split('\n').filter((l) => /_audioSeek\(/.test(l));

tests/js/speed_reset.test.js

@@ -5,6 +5,9 @@ const path = require('node:path');
 const vm = require('node:vm');
 
 const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js');
+// The speed controls were carved out into static/js/player-controls.js (R3a); playSong,
+// which resets them on a new song, stayed in app.js. This test spans both.
+const CONTROLS_JS = path.join(__dirname, '..', '..', 'static', 'js', 'player-controls.js');
 
 function extractFunction(src, signature) {
     const start = src.indexOf(signature);
@@ -130,15 +133,17 @@ function extractConstLine(src, name) {
 
 function loadPlaySong(sandbox) {
     const src = fs.readFileSync(APP_JS, 'utf8');
-    const resetHelper = src.includes('function _resetPlaybackSpeedForNewSong')
-        ? extractFunction(src, 'function _resetPlaybackSpeedForNewSong')
+    // the module is ESM; the vm sandbox evaluates plain script text
+    const controls = fs.readFileSync(CONTROLS_JS, 'utf8').replace(/^export /gm, '');
+    const resetHelper = controls.includes('function _resetPlaybackSpeedForNewSong')
+        ? extractFunction(controls, 'function _resetPlaybackSpeedForNewSong')
         : '';
-    const speedPresetHelpers = src.includes('function _updateSpeedPresetButtons')
+    const speedPresetHelpers = controls.includes('function _updateSpeedPresetButtons')
         ? `
-        ${extractConstLine(src, 'SPEED_PRESET_PCTS')}
-        ${extractConstLine(src, 'SPEED_SNAP_THRESHOLD')}
-        ${extractFunction(src, 'function _speedPresetPctFromActive')}
-        ${extractFunction(src, 'function _updateSpeedPresetButtons')}
+        ${extractConstLine(controls, 'SPEED_PRESET_PCTS')}
+        ${extractConstLine(controls, 'SPEED_SNAP_THRESHOLD')}
+        ${extractFunction(controls, 'function _speedPresetPctFromActive')}
+        ${extractFunction(controls, 'function _updateSpeedPresetButtons')}
         `
         : '';
     const code = `
@@ -147,6 +152,11 @@ function loadPlaySong(sandbox) {
         // WRITE it (an imported binding is read-only). NB window.feedBack.isPlaying — the
         // public mirror stubbed above — is a different thing and is unchanged.
         var S = { isPlaying: true, lastAudioTime: 0 };
+        // The speed controls reach app.js through the host seam (static/js/host.js).
+        // Route it at the sandbox's EXISTING handleSliderInput spy — a fresh stub would
+        // swallow the call and the assertion below (which checks the slider was actually
+        // refreshed) would pass vacuously.
+        var host = { handleSliderInput: (el) => handleSliderInput(el) };
         var currentFilename = null;
         var _playerOriginScreen = null;
         var _pendingAutostart = false;
@@ -166,7 +176,7 @@ function loadPlaySong(sandbox) {
         function _scheduleSectionPracticeRetries() {}
         function loadSavedLoops() {}
         function _songEventPayload() { return { time: 7, audioT: 7, chartT: 7, perfNow: 7 }; }
-        ${extractFunction(src, 'function setSpeed')}
+        ${extractFunction(controls, 'function setSpeed')}
         ${speedPresetHelpers}
         ${resetHelper}
         ${extractFunction(src, 'async function playSong')}

tests/js/v3_library_refresh.test.js

@@ -16,7 +16,11 @@ const path = require('node:path');
 
 const root = path.join(__dirname, '..', '..');
 const SONGS = fs.readFileSync(path.join(root, 'static', 'v3', 'songs.js'), 'utf8');
-const APP = fs.readFileSync(path.join(root, 'static', 'app.js'), 'utf8');
+// The rescan path moved into ./static/js/library.js with the rest of the library (R3a).
+// Read BOTH: this asserts the emit exists SOMEWHERE in the app, and pinning it to one file
+// just means the test starts lying the next time the code moves.
+const APP = fs.readFileSync(path.join(root, 'static', 'app.js'), 'utf8')
+    + '\n' + fs.readFileSync(path.join(root, 'static', 'js', 'library.js'), 'utf8');
 
 test('app.js emits library:changed when a Settings rescan completes', () => {
     assert.match(APP, /emit\(\s*['"]library:changed['"]/,

tests/test_builtin_diagnostic_seed.py

@@ -7,6 +7,7 @@ import os
 import sys
 import time
 
+import builtin_content
 import pytest
 
 
@@ -23,13 +24,13 @@ def test_seed_creates_builtin_diagnostic_sloppak(tmp_path, server_mod):
     """First seed copies the bundled sloppak into diagnostics-builtin/."""
     dlc = tmp_path / "dlc"
     dlc.mkdir()
-    source = server_mod._feedBack_server_root() / server_mod._BUILTIN_DIAGNOSTIC_SOURCES[0][1]
+    source = server_mod._feedBack_server_root() / builtin_content.BUILTIN_DIAGNOSTIC_SOURCES[0][1]
     if not source.is_file():
         pytest.skip(f"source sloppak not present in checkout: {source}")
 
-    server_mod._seed_builtin_diagnostic_sloppaks(dlc)
+    builtin_content.seed_builtin_diagnostic_sloppaks(server_mod._feedBack_server_root(), dlc)
 
-    dest = dlc / server_mod._BUILTIN_DIAGNOSTIC_SUBDIR / server_mod._BUILTIN_DIAGNOSTIC_SOURCES[0][0]
+    dest = dlc / builtin_content.BUILTIN_DIAGNOSTIC_SUBDIR / builtin_content.BUILTIN_DIAGNOSTIC_SOURCES[0][0]
     assert dest.is_file()
     assert dest.stat().st_size == source.stat().st_size
 
@@ -38,16 +39,16 @@ def test_seed_is_idempotent_when_destination_exists(tmp_path, server_mod):
     """Second seed leaves an up-to-date destination unchanged."""
     dlc = tmp_path / "dlc"
     dlc.mkdir()
-    source = server_mod._feedBack_server_root() / server_mod._BUILTIN_DIAGNOSTIC_SOURCES[0][1]
+    source = server_mod._feedBack_server_root() / builtin_content.BUILTIN_DIAGNOSTIC_SOURCES[0][1]
     if not source.is_file():
         pytest.skip(f"source sloppak not present in checkout: {source}")
 
-    server_mod._seed_builtin_diagnostic_sloppaks(dlc)
-    dest = dlc / server_mod._BUILTIN_DIAGNOSTIC_SUBDIR / server_mod._BUILTIN_DIAGNOSTIC_SOURCES[0][0]
+    builtin_content.seed_builtin_diagnostic_sloppaks(server_mod._feedBack_server_root(), dlc)
+    dest = dlc / builtin_content.BUILTIN_DIAGNOSTIC_SUBDIR / builtin_content.BUILTIN_DIAGNOSTIC_SOURCES[0][0]
     first_mtime = dest.stat().st_mtime_ns
     first_size = dest.stat().st_size
 
-    server_mod._seed_builtin_diagnostic_sloppaks(dlc)
+    builtin_content.seed_builtin_diagnostic_sloppaks(server_mod._feedBack_server_root(), dlc)
 
     assert dest.stat().st_mtime_ns == first_mtime
     assert dest.stat().st_size == first_size
@@ -57,18 +58,18 @@ def test_seed_skips_when_destination_is_newer(tmp_path, server_mod):
     """An existing newer destination is not overwritten."""
     dlc = tmp_path / "dlc"
     dlc.mkdir()
-    source = server_mod._feedBack_server_root() / server_mod._BUILTIN_DIAGNOSTIC_SOURCES[0][1]
+    source = server_mod._feedBack_server_root() / builtin_content.BUILTIN_DIAGNOSTIC_SOURCES[0][1]
     if not source.is_file():
         pytest.skip(f"source sloppak not present in checkout: {source}")
-    dest_dir = dlc / server_mod._BUILTIN_DIAGNOSTIC_SUBDIR
+    dest_dir = dlc / builtin_content.BUILTIN_DIAGNOSTIC_SUBDIR
     dest_dir.mkdir(parents=True)
-    dest_name = server_mod._BUILTIN_DIAGNOSTIC_SOURCES[0][0]
+    dest_name = builtin_content.BUILTIN_DIAGNOSTIC_SOURCES[0][0]
     dest = dest_dir / dest_name
     dest.write_bytes(b"user-owned diagnostic copy")
     future = time.time() + 3600
     os.utime(dest, (future, future))
 
-    server_mod._seed_builtin_diagnostic_sloppaks(dlc)
+    builtin_content.seed_builtin_diagnostic_sloppaks(server_mod._feedBack_server_root(), dlc)
 
     assert dest.read_bytes() == b"user-owned diagnostic copy"
 
@@ -77,18 +78,18 @@ def test_seed_refuses_to_follow_symlink_destination(tmp_path, server_mod):
     """A symlink at the destination is skipped, not written through."""
     dlc = tmp_path / "dlc"
     dlc.mkdir()
-    source = server_mod._feedBack_server_root() / server_mod._BUILTIN_DIAGNOSTIC_SOURCES[0][1]
+    source = server_mod._feedBack_server_root() / builtin_content.BUILTIN_DIAGNOSTIC_SOURCES[0][1]
     if not source.is_file():
         pytest.skip(f"source sloppak not present in checkout: {source}")
 
     outside = tmp_path / "outside.txt"
     outside.write_bytes(b"do not overwrite me")
-    dest_dir = dlc / server_mod._BUILTIN_DIAGNOSTIC_SUBDIR
+    dest_dir = dlc / builtin_content.BUILTIN_DIAGNOSTIC_SUBDIR
     dest_dir.mkdir(parents=True)
-    dest = dest_dir / server_mod._BUILTIN_DIAGNOSTIC_SOURCES[0][0]
+    dest = dest_dir / builtin_content.BUILTIN_DIAGNOSTIC_SOURCES[0][0]
     dest.symlink_to(outside)
 
-    server_mod._seed_builtin_diagnostic_sloppaks(dlc)
+    builtin_content.seed_builtin_diagnostic_sloppaks(server_mod._feedBack_server_root(), dlc)
 
     # The symlink target must be untouched and the link left as-is.
     assert outside.read_bytes() == b"do not overwrite me"
@@ -101,11 +102,11 @@ def test_seed_refuses_symlinked_seed_directory(tmp_path, server_mod):
     dlc.mkdir()
     outside_dir = tmp_path / "outside_dir"
     outside_dir.mkdir()
-    (dlc / server_mod._BUILTIN_DIAGNOSTIC_SUBDIR).symlink_to(
+    (dlc / builtin_content.BUILTIN_DIAGNOSTIC_SUBDIR).symlink_to(
         outside_dir, target_is_directory=True
     )
 
-    server_mod._seed_builtin_diagnostic_sloppaks(dlc)
+    builtin_content.seed_builtin_diagnostic_sloppaks(server_mod._feedBack_server_root(), dlc)
 
     # Nothing was written through the directory symlink into the link target.
     assert list(outside_dir.iterdir()) == []
@@ -116,11 +117,11 @@ def test_seed_missing_source_does_not_crash(tmp_path, server_mod, monkeypatch):
     dlc = tmp_path / "dlc"
     dlc.mkdir()
     monkeypatch.setattr(
-        server_mod,
-        "_BUILTIN_DIAGNOSTIC_SOURCES",
+        builtin_content,
+        "BUILTIN_DIAGNOSTIC_SOURCES",
         [("missing.sloppak", "docs/diagnostics/does-not-exist.sloppak")],
     )
 
-    server_mod._seed_builtin_diagnostic_sloppaks(dlc)
+    builtin_content.seed_builtin_diagnostic_sloppaks(server_mod._feedBack_server_root(), dlc)
 
-    assert not (dlc / server_mod._BUILTIN_DIAGNOSTIC_SUBDIR / "missing.sloppak").exists()
+    assert not (dlc / builtin_content.BUILTIN_DIAGNOSTIC_SUBDIR / "missing.sloppak").exists()

tests/test_builtin_starter_seed.py

@@ -5,6 +5,7 @@ from __future__ import annotations
 import importlib
 import sys
 
+import builtin_content
 import pytest
 
 
@@ -21,15 +22,15 @@ def server_mod(tmp_path, monkeypatch, isolate_logging):
 def _source(server_mod):
     return (
         server_mod._feedBack_server_root()
-        / server_mod._BUILTIN_STARTER_SOURCES[0][1]
+        / builtin_content.BUILTIN_STARTER_SOURCES[0][1]
     )
 
 
 def _dest(server_mod, dlc):
     return (
         dlc
-        / server_mod._BUILTIN_STARTER_SUBDIR
-        / server_mod._BUILTIN_STARTER_SOURCES[0][0]
+        / builtin_content.BUILTIN_STARTER_SUBDIR
+        / builtin_content.BUILTIN_STARTER_SOURCES[0][0]
     )
 
 
@@ -41,12 +42,12 @@ def test_seed_creates_starter_content_and_marker(tmp_path, server_mod):
     if not source.is_file():
         pytest.skip(f"starter source not present in checkout: {source}")
 
-    server_mod._seed_builtin_starter_content(dlc)
+    builtin_content.seed_builtin_starter_content(server_mod._feedBack_server_root(), dlc)
 
     dest = _dest(server_mod, dlc)
     assert dest.is_file()
     assert dest.stat().st_size == source.stat().st_size
-    assert (server_mod.CONFIG_DIR / server_mod._STARTER_SEED_MARKER).is_file()
+    assert (server_mod.CONFIG_DIR / builtin_content.STARTER_SEED_MARKER).is_file()
 
 
 def test_seed_preserves_source_mtime(tmp_path, server_mod):
@@ -58,7 +59,7 @@ def test_seed_preserves_source_mtime(tmp_path, server_mod):
     if not source.is_file():
         pytest.skip(f"starter source not present in checkout: {source}")
 
-    server_mod._seed_builtin_starter_content(dlc)
+    builtin_content.seed_builtin_starter_content(server_mod._feedBack_server_root(), dlc)
 
     assert _dest(server_mod, dlc).stat().st_mtime_ns == source.stat().st_mtime_ns
 
@@ -78,7 +79,7 @@ def test_seed_runs_only_once_and_respects_deletion(tmp_path, server_mod):
     if not source.is_file():
         pytest.skip(f"starter source not present in checkout: {source}")
 
-    server_mod._seed_builtin_starter_content(dlc)
+    builtin_content.seed_builtin_starter_content(server_mod._feedBack_server_root(), dlc)
     dest = _dest(server_mod, dlc)
     assert dest.is_file()
 
@@ -86,7 +87,7 @@ def test_seed_runs_only_once_and_respects_deletion(tmp_path, server_mod):
     dest.unlink()
 
     # A subsequent launch must not re-seed it.
-    server_mod._seed_builtin_starter_content(dlc)
+    builtin_content.seed_builtin_starter_content(server_mod._feedBack_server_root(), dlc)
     assert not dest.exists()
 
 
@@ -98,15 +99,15 @@ def test_seed_deferred_until_dlc_configured(tmp_path, server_mod):
         pytest.skip(f"starter source not present in checkout: {source}")
 
     # dlc is None and DLC_DIR unset -> _get_dlc_dir() returns None.
-    server_mod._seed_builtin_starter_content(None)
-    assert not (server_mod.CONFIG_DIR / server_mod._STARTER_SEED_MARKER).exists()
+    builtin_content.seed_builtin_starter_content(server_mod._feedBack_server_root(), None)
+    assert not (server_mod.CONFIG_DIR / builtin_content.STARTER_SEED_MARKER).exists()
 
     # Now a DLC is configured: the deferred seed runs.
     dlc = tmp_path / "dlc"
     dlc.mkdir()
-    server_mod._seed_builtin_starter_content(dlc)
+    builtin_content.seed_builtin_starter_content(server_mod._feedBack_server_root(), dlc)
     assert _dest(server_mod, dlc).is_file()
-    assert (server_mod.CONFIG_DIR / server_mod._STARTER_SEED_MARKER).is_file()
+    assert (server_mod.CONFIG_DIR / builtin_content.STARTER_SEED_MARKER).is_file()
 
 
 def test_seed_refuses_symlinked_seed_directory(tmp_path, server_mod):
@@ -119,13 +120,13 @@ def test_seed_refuses_symlinked_seed_directory(tmp_path, server_mod):
 
     outside_dir = tmp_path / "outside"
     outside_dir.mkdir()
-    (dlc / server_mod._BUILTIN_STARTER_SUBDIR).symlink_to(outside_dir)
+    (dlc / builtin_content.BUILTIN_STARTER_SUBDIR).symlink_to(outside_dir)
 
-    server_mod._seed_builtin_starter_content(dlc)
+    builtin_content.seed_builtin_starter_content(server_mod._feedBack_server_root(), dlc)
 
     assert list(outside_dir.iterdir()) == []
     # An incomplete seed must NOT write the marker, so a later launch retries.
-    assert not (server_mod.CONFIG_DIR / server_mod._STARTER_SEED_MARKER).exists()
+    assert not (server_mod.CONFIG_DIR / builtin_content.STARTER_SEED_MARKER).exists()
 
 
 def test_seed_never_overwrites_an_existing_user_file(tmp_path, server_mod):
@@ -140,11 +141,11 @@ def test_seed_never_overwrites_an_existing_user_file(tmp_path, server_mod):
     dest.write_bytes(b"user's own edited pack")
     _os.utime(dest, (1_000_000, 1_000_000))  # far older than the bundled source
 
-    server_mod._seed_builtin_starter_content(dlc)
+    builtin_content.seed_builtin_starter_content(server_mod._feedBack_server_root(), dlc)
 
     assert dest.read_bytes() == b"user's own edited pack"  # untouched
     # counted as already-present, so the one-time seed considers itself done
-    assert (server_mod.CONFIG_DIR / server_mod._STARTER_SEED_MARKER).is_file()
+    assert (server_mod.CONFIG_DIR / builtin_content.STARTER_SEED_MARKER).is_file()
 
 
 def test_seed_does_not_mark_when_destination_is_a_directory(tmp_path, server_mod):
@@ -160,10 +161,10 @@ def test_seed_does_not_mark_when_destination_is_a_directory(tmp_path, server_mod
     bogus.parent.mkdir(parents=True, exist_ok=True)
     bogus.mkdir()  # user (or junk) placed a directory where the pack goes
 
-    server_mod._seed_builtin_starter_content(dlc)
+    builtin_content.seed_builtin_starter_content(server_mod._feedBack_server_root(), dlc)
 
     assert bogus.is_dir()  # untouched
-    assert not (server_mod.CONFIG_DIR / server_mod._STARTER_SEED_MARKER).exists()
+    assert not (server_mod.CONFIG_DIR / builtin_content.STARTER_SEED_MARKER).exists()
 
 
 def test_seed_does_not_mark_when_source_missing(tmp_path, server_mod, monkeypatch):
@@ -172,15 +173,15 @@ def test_seed_does_not_mark_when_source_missing(tmp_path, server_mod, monkeypatc
     dlc = tmp_path / "dlc"
     dlc.mkdir()
     monkeypatch.setattr(
-        server_mod,
-        "_BUILTIN_STARTER_SOURCES",
+        builtin_content,
+        "BUILTIN_STARTER_SOURCES",
         [("missing.feedpak", "content/starter/does-not-exist.feedpak")],
     )
 
-    server_mod._seed_builtin_starter_content(dlc)
+    builtin_content.seed_builtin_starter_content(server_mod._feedBack_server_root(), dlc)
 
-    assert not (dlc / server_mod._BUILTIN_STARTER_SUBDIR / "missing.feedpak").exists()
-    assert not (server_mod.CONFIG_DIR / server_mod._STARTER_SEED_MARKER).exists()
+    assert not (dlc / builtin_content.BUILTIN_STARTER_SUBDIR / "missing.feedpak").exists()
+    assert not (server_mod.CONFIG_DIR / builtin_content.STARTER_SEED_MARKER).exists()
 
 
 def test_every_starter_source_file_is_present(server_mod):
@@ -190,7 +191,7 @@ def test_every_starter_source_file_is_present(server_mod):
     the checkout is clean, so "on disk" == committed."""
     root = server_mod._feedBack_server_root()
     missing = [
-        rel for _, rel in server_mod._BUILTIN_STARTER_SOURCES
+        rel for _, rel in builtin_content.BUILTIN_STARTER_SOURCES
         if not (root / rel).is_file()
     ]
     assert not missing, f"listed starter sources missing on disk: {missing}"
@@ -199,18 +200,18 @@ def test_every_starter_source_file_is_present(server_mod):
 def test_seed_lands_every_listed_starter_pack(tmp_path, server_mod):
     """A real seed run copies every listed pack into starter/ and marks done."""
     root = server_mod._feedBack_server_root()
-    for _, rel in server_mod._BUILTIN_STARTER_SOURCES:
+    for _, rel in builtin_content.BUILTIN_STARTER_SOURCES:
         if not (root / rel).is_file():
             pytest.skip(f"starter source not present in checkout: {rel}")
 
     dlc = tmp_path / "dlc"
     dlc.mkdir()
-    server_mod._seed_builtin_starter_content(dlc)
+    builtin_content.seed_builtin_starter_content(server_mod._feedBack_server_root(), dlc)
 
-    for dest_name, _ in server_mod._BUILTIN_STARTER_SOURCES:
-        dest = dlc / server_mod._BUILTIN_STARTER_SUBDIR / dest_name
+    for dest_name, _ in builtin_content.BUILTIN_STARTER_SOURCES:
+        dest = dlc / builtin_content.BUILTIN_STARTER_SUBDIR / dest_name
         assert dest.is_file(), f"pack not seeded: {dest_name}"
-    assert (server_mod.CONFIG_DIR / server_mod._STARTER_SEED_MARKER).is_file()
+    assert (server_mod.CONFIG_DIR / builtin_content.STARTER_SEED_MARKER).is_file()
 
 
 def test_no_unlisted_starter_pack_on_disk(server_mod):
@@ -220,7 +221,7 @@ def test_no_unlisted_starter_pack_on_disk(server_mod):
     main before being wired up. In CI the checkout is clean, so this flags any
     stray/committed pack that isn't listed."""
     root = server_mod._feedBack_server_root()
-    listed = {rel for _, rel in server_mod._BUILTIN_STARTER_SOURCES}
+    listed = {rel for _, rel in builtin_content.BUILTIN_STARTER_SOURCES}
     if not listed:
         pytest.skip("no starter sources declared")
     content_dir = (root / next(iter(listed))).parent  # all sources share this dir

tests/test_demo_mode.py

@@ -16,6 +16,7 @@ Covers:
 import importlib
 import sys
 
+import demo_mode
 import pytest
 from fastapi.testclient import TestClient
 
@@ -50,14 +51,14 @@ def _cleanup(server, client):
     client.close()
     # Stop the demo-mode janitor thread (if started) so daemon threads don't
     # accumulate across tests.
-    server._DEMO_JANITOR_STOP.set()
-    thread = server._DEMO_JANITOR_THREAD
+    demo_mode._DEMO_JANITOR_STOP.set()
+    thread = demo_mode._DEMO_JANITOR_THREAD
     if thread is not None:
         thread.join(timeout=2)
-    server._DEMO_JANITOR_STARTED = False
-    server._DEMO_JANITOR_THREAD = None
-    with server._DEMO_JANITOR_HOOKS_LOCK:
-        server._DEMO_JANITOR_HOOKS.clear()
+    demo_mode._DEMO_JANITOR_STARTED = False
+    demo_mode._DEMO_JANITOR_THREAD = None
+    with demo_mode._DEMO_JANITOR_HOOKS_LOCK:
+        demo_mode._DEMO_JANITOR_HOOKS.clear()
     conn = getattr(getattr(server, "meta_db", None), "conn", None)
     if conn is not None:
         getattr(__import__("sys").modules.get("server"), "_join_background_db_threads", lambda: None)()
@@ -221,15 +222,15 @@ def test_register_demo_janitor_hook_is_callable(tmp_path, monkeypatch):
     server, client = _make_client(tmp_path, monkeypatch, demo=True)
     try:
         called = []
-        server.register_demo_janitor_hook(lambda: called.append(1))
+        demo_mode.register_demo_janitor_hook(lambda: called.append(1))
         # Manually invoke the registered hooks (simulating a janitor sweep).
-        for hook in list(server._DEMO_JANITOR_HOOKS):
+        for hook in list(demo_mode._DEMO_JANITOR_HOOKS):
             hook()
         assert 1 in called
     finally:
         # Clean up our test hook so it doesn't leak into other tests.
-        with server._DEMO_JANITOR_HOOKS_LOCK:
-            server._DEMO_JANITOR_HOOKS.clear()
+        with demo_mode._DEMO_JANITOR_HOOKS_LOCK:
+            demo_mode._DEMO_JANITOR_HOOKS.clear()
         _cleanup(server, client)
 
 
@@ -238,7 +239,7 @@ def test_register_demo_janitor_hook_rejects_non_callable(tmp_path, monkeypatch):
     server, client = _make_client(tmp_path, monkeypatch, demo=True)
     try:
         with pytest.raises(TypeError):
-            server.register_demo_janitor_hook("not a function")
+            demo_mode.register_demo_janitor_hook("not a function")
     finally:
         _cleanup(server, client)
 
@@ -251,7 +252,7 @@ def test_register_demo_janitor_hook_rejects_async_callable(tmp_path, monkeypatch
             pass
 
         with pytest.raises(TypeError, match="async"):
-            server.register_demo_janitor_hook(_async_hook)
+            demo_mode.register_demo_janitor_hook(_async_hook)
     finally:
         _cleanup(server, client)
 
@@ -264,7 +265,7 @@ def test_register_demo_janitor_hook_rejects_non_zero_arg_callable(tmp_path, monk
             pass
 
         with pytest.raises(TypeError, match="zero-argument"):
-            server.register_demo_janitor_hook(_needs_arg)
+            demo_mode.register_demo_janitor_hook(_needs_arg)
     finally:
         _cleanup(server, client)
 
@@ -276,10 +277,10 @@ def test_register_demo_janitor_hook_accepts_default_arg_callable(tmp_path, monke
         def _optional_arg(x=None):
             pass
 
-        server.register_demo_janitor_hook(_optional_arg)
+        demo_mode.register_demo_janitor_hook(_optional_arg)
     finally:
-        with server._DEMO_JANITOR_HOOKS_LOCK:
-            server._DEMO_JANITOR_HOOKS.clear()
+        with demo_mode._DEMO_JANITOR_HOOKS_LOCK:
+            demo_mode._DEMO_JANITOR_HOOKS.clear()
         _cleanup(server, client)
 
 
@@ -308,21 +309,21 @@ def test_register_demo_janitor_hook_in_plugin_context(tmp_path, monkeypatch):
         assert "register_demo_janitor_hook" in captured, (
             "register_demo_janitor_hook was not passed in the plugin context"
         )
-        assert captured["register_demo_janitor_hook"] is server.register_demo_janitor_hook
+        assert captured["register_demo_janitor_hook"] is demo_mode.register_demo_janitor_hook
 
     conn = getattr(getattr(server, "meta_db", None), "conn", None)
     if conn is not None:
         getattr(__import__("sys").modules.get("server"), "_join_background_db_threads", lambda: None)()
         conn.close()
     # Clean up janitor state so it doesn't bleed into other tests.
-    server._DEMO_JANITOR_STOP.set()
-    thread = server._DEMO_JANITOR_THREAD
+    demo_mode._DEMO_JANITOR_STOP.set()
+    thread = demo_mode._DEMO_JANITOR_THREAD
     if thread is not None:
         thread.join(timeout=2)
-    server._DEMO_JANITOR_STARTED = False
-    server._DEMO_JANITOR_THREAD = None
-    with server._DEMO_JANITOR_HOOKS_LOCK:
-        server._DEMO_JANITOR_HOOKS.clear()
+    demo_mode._DEMO_JANITOR_STARTED = False
+    demo_mode._DEMO_JANITOR_THREAD = None
+    with demo_mode._DEMO_JANITOR_HOOKS_LOCK:
+        demo_mode._DEMO_JANITOR_HOOKS.clear()
 
 
 

tests/test_feedpak_extension.py

@@ -82,7 +82,7 @@ def test_is_sloppak_rejects_other_suffixes(name):
 # ── 2. _background_scan discovery glob (DLC scan) ────────────────────────────
 
 @pytest.fixture()
-def scan_server(tmp_path, monkeypatch, isolate_logging):
+def scan_server(tmp_path, monkeypatch, isolate_logging, reset_scan_state):
     """Fresh server import with the background scan forced in-process.
 
     Mirrors tests/test_settings_api.py::scan_module — the production scan uses
@@ -94,8 +94,13 @@ def scan_server(tmp_path, monkeypatch, isolate_logging):
     monkeypatch.delenv("DLC_DIR", raising=False)
     sys.modules.pop("server", None)
     mod = importlib.import_module("server")
+    # The scanner is lib/scan.py now (R3b). Patch it THERE — `mod` (server) re-imports
+    # per-test, but `scan` stays cached in sys.modules, so this is the same module object
+    # server calls into. That it still works is the point of the late-bound appstate
+    # reads: scan picks up the fresh CONFIG_DIR without being re-imported itself.
+    import scan as scan_mod
     monkeypatch.setattr(
-        mod, "_make_scan_executor",
+        scan_mod, "_make_scan_executor",
         lambda: concurrent.futures.ThreadPoolExecutor(max_workers=4),
     )
     yield mod
@@ -125,7 +130,7 @@ def test_background_scan_discovers_both_suffixes(tmp_path, scan_server):
         return {"title": f.name, "artist": "", "album": ""}
 
     with mock.patch("scan_worker._extract_meta_for_file", new=mock_extract):
-        scan_server._background_scan()
+        importlib.import_module("scan").background_scan()
 
     assert "new.feedpak" in seen
     assert "legacy.sloppak" in seen

tests/test_plugin_context_contract.py

@@ -0,0 +1,205 @@
+"""The plugin context is a THIRD-PARTY CONTRACT. Pin it.
+
+`context` is handed to every plugin's `setup()`. Plugins — including ones we don't ship
+and can't grep — read keys out of it and hold the callables as live references. Issue #48
+flagged this while planning the server.py split and asked for exactly this assertion:
+
+    "Plugin context[...] are passed as live references into already-loaded plugins.
+     Refactoring must preserve the exact callables — moving them to a new module is
+     fine, but renaming or wrapping them breaks third-party plugins. We'd want a
+     'plugin context unchanged' assertion in CI."
+
+It doesn't exist yet, and server.py is about to be carved apart around the code that
+builds it. This is the guard that makes the carve safe: a key silently dropped or
+renamed by a move is invisible to every other test in the suite (nothing in-tree reads
+most of these) and would break plugins at runtime, in the field.
+
+Same lesson the frontend carve learned the hard way: a contract that only external code
+reads cannot be found by a call-graph scan, so it has to be pinned by name.
+
+WHY A LITERAL LIST AND NOT A DERIVED ONE. Deriving the expected set from the source would
+assert the code equals itself. The whole point is that a human has to look at a diff and
+consciously agree to change the contract.
+"""
+
+import ast
+from pathlib import Path
+
+import pytest
+
+SERVER_PY = Path(__file__).resolve().parents[1] / "server.py"
+PLUGINS_PY = Path(__file__).resolve().parents[1] / "plugins" / "__init__.py"
+
+# The keys server.py puts in the shared context handed to register_plugin_api().
+BASE_CONTEXT_KEYS = {
+    "config_dir",
+    "get_dlc_dir",
+    "extract_meta",
+    "meta_db",
+    "get_scan_status",
+    "get_art_cache_dir",
+    "library_providers",
+    "register_library_provider",
+    "unregister_library_provider",
+    "register_tuning_provider",
+    "unregister_tuning_provider",
+    "get_sloppak_cache_dir",
+    "register_demo_janitor_hook",
+    "award_xp",
+    "get_xp_progress",
+    "seed_xp",
+    "reset_xp",
+    "record_progression_event",
+}
+
+# Added PER PLUGIN by plugins/__init__.py on top of the base — so the surface a plugin
+# actually sees is the union. Real shipped plugins read `log` and `load_sibling`, and
+# neither is in server.py's dict; a test that pinned only the base would miss them.
+PER_PLUGIN_KEYS = {"load_sibling", "log"}
+
+FULL_CONTEXT = BASE_CONTEXT_KEYS | PER_PLUGIN_KEYS
+
+
+def _plugin_context_keys() -> set:
+    """The literal keys of server.py's `plugin_context = {...}`, read from the AST.
+
+    AST, not a regex: the dict spans ~40 lines and is dense with comments, lambdas and
+    nested calls, and the values contain braces of their own.
+    """
+    tree = ast.parse(SERVER_PY.read_text(encoding="utf-8"))
+    for node in ast.walk(tree):
+        if (
+            isinstance(node, ast.Assign)
+            and node.targets
+            and isinstance(node.targets[0], ast.Name)
+            and node.targets[0].id == "plugin_context"
+            and isinstance(node.value, ast.Dict)
+        ):
+            keys = set()
+            for k in node.value.keys:
+                assert isinstance(k, ast.Constant), (
+                    "plugin_context must be built from literal string keys — a computed "
+                    "key makes this contract un-reviewable"
+                )
+                keys.add(k.value)
+            return keys
+    pytest.fail(
+        "server.py no longer builds a literal `plugin_context = {...}` dict. If it moved "
+        "to another module, point this test at that module — do NOT delete it."
+    )
+
+
+def test_plugin_context_keys_are_exactly_the_pinned_contract():
+    actual = _plugin_context_keys()
+
+    missing = BASE_CONTEXT_KEYS - actual
+    added = actual - BASE_CONTEXT_KEYS
+
+    assert not missing, (
+        f"plugin_context lost {sorted(missing)}. Every one of these is read by plugins we "
+        "do not control and cannot grep. Dropping one breaks them at runtime, in the "
+        "field, with nothing else in this suite failing."
+    )
+    assert not added, (
+        f"plugin_context gained {sorted(added)}. That's fine — but it is a PUBLIC API "
+        "addition, so add the key to BASE_CONTEXT_KEYS here deliberately, and document it "
+        "in docs/. This test exists to make that a conscious act rather than a side effect."
+    )
+
+
+def test_per_plugin_keys_are_still_layered_on_top():
+    """`log` and `load_sibling` are added per-plugin in plugins/__init__.py, not by
+    server.py — so they're invisible to the check above. Real plugins read both."""
+    src = PLUGINS_PY.read_text(encoding="utf-8")
+    for key in sorted(PER_PLUGIN_KEYS):
+        assert f'plugin_context["{key}"]' in src, (
+            f"plugins/__init__.py no longer sets plugin_context[{key!r}] — shipped plugins "
+            "read it"
+        )
+
+
+def test_context_values_reach_a_REAL_plugin_by_identity(tmp_path, reset_plugin_state):
+    """The contract is CALLABLE IDENTITY, not just key names.
+
+    A carve that moves these into a module and re-exports them through a wrapper (a
+    property, a functools.partial, a lazily-bound getter) keeps every key name intact and
+    STILL breaks plugins that stored the reference at setup() time.
+
+    Codex [P2] on the first cut of this test, and it was right: I originally built a dict
+    locally and called setup() on it, which asserts `dict(x)['k'] is x['k']` — trivially
+    true, and blind to everything plugins/__init__.py does. It has to go through the REAL
+    loader, because the real loader is exactly what copies and re-binds the context.
+
+    (That is not hypothetical: `register_library_provider` IS deliberately wrapped by the
+    loader, per-plugin, to force owner attribution. Pinned below so the one intentional
+    exception can't quietly become two.)
+    """
+    from fastapi import FastAPI
+
+    # reset_plugin_state (tests/conftest.py) is the ONLY safe way to drive the real
+    # load_plugins(): it also mutates sys.path, sys.modules and PENDING_PLUGINS, and a
+    # hand-rolled partial restore makes the suite order- and environment-dependent.
+    # Codex [P2] on the first cut of this, and it was right.
+    plugins_mod = reset_plugin_state
+
+    plugin_dir = tmp_path / "ctxprobe"
+    plugin_dir.mkdir()
+    (plugin_dir / "plugin.json").write_text(
+        '{"id": "ctxprobe", "name": "ctx probe", "routes": "routes.py"}'
+    )
+    # A backend plugin's entry point is routes.py's `setup(app, ctx)` — the same shape
+    # tests/test_plugins.py::_make_plugin uses. The probe hands the context BACK through a
+    # sink in the context itself: importing the probe module by name does not work (the
+    # loader namespaces plugin modules), and a file/JSON channel would lose the object
+    # IDENTITY that is the entire point of this test.
+    (plugin_dir / "routes.py").write_text(
+        "def setup(app, ctx):\n"
+        "    ctx['_probe_sink'].append(ctx)\n"
+    )
+
+    sentinel_db = object()
+
+    def sentinel_extract(_p):
+        return {}
+
+    def sentinel_register_library_provider(provider, *a, **kw):
+        return None
+
+    sink = []
+    context = {
+        "_probe_sink": sink,
+        "meta_db": sentinel_db,
+        "extract_meta": sentinel_extract,
+        "config_dir": tmp_path,
+        "register_library_provider": sentinel_register_library_provider,
+    }
+
+    app = FastAPI()
+    saved_dir = plugins_mod.PLUGINS_DIR
+    plugins_mod.PLUGINS_DIR = tmp_path
+    try:
+        plugins_mod.load_plugins(app, context)
+    finally:
+        plugins_mod.PLUGINS_DIR = saved_dir
+
+    assert sink, "the probe plugin's setup() never ran — the harness is not exercising the loader"
+    seen = sink[0]
+
+    assert seen["meta_db"] is sentinel_db, "meta_db must reach a real plugin BY IDENTITY"
+    assert seen["extract_meta"] is sentinel_extract, (
+        "extract_meta must reach a real plugin BY IDENTITY — wrapping it (partial, "
+        "property, re-binding getter) breaks plugins that stored the reference at setup()"
+    )
+    assert seen["config_dir"] is context["config_dir"]
+
+    # The loader adds these per-plugin; shipped plugins read both.
+    assert callable(seen["load_sibling"])
+    assert seen["log"].name == "feedBack.plugin.ctxprobe"
+
+    # THE ONE DELIBERATE WRAPPER. register_library_provider is scoped per-plugin so a
+    # plugin cannot forge owner attribution and impersonate another. Pinned so that the
+    # single intentional exception to identity cannot quietly become two.
+    assert seen["register_library_provider"] is not sentinel_register_library_provider, (
+        "register_library_provider is supposed to be wrapped per-plugin for owner "
+        "attribution — if that wrapper is gone, a plugin can impersonate another"
+    )

tests/test_plugin_runtime_idempotence.py

@@ -189,9 +189,13 @@ def test_app_event_bus_dispatches_locally_and_preserves_juce_stop_state():
     # `isPlaying` moved onto the shared player-state container (static/js/player-state.js)
     # so a carved module can WRITE it — an imported binding is read-only.
     assert "const hadPlayableSong = !!audio.src || !!window._juceAudioUrl || S.isPlaying" in source
-    assert "sm.emit('song:resume', payload)" in source
     assert "window.feedBack.emit('song:resume', payload)" in source
 
+    # The JUCE audio-element shim — which re-emits song:resume through the session
+    # manager when JUCE owns the transport — was carved out into its own module (R3a).
+    juce = (ROOT / "static" / "js" / "juce-audio.js").read_text(encoding="utf-8")
+    assert "sm.emit('song:resume', payload)" in juce
+
 
 def test_nam_and_stems_use_owner_claim_dispatch_semantics():
     nam_source = _sibling_text("feedBack-plugin-nam-tone", "screen.js", "NAM_STEM_CLAIM_ID = 'nam.amp-active'")

tests/test_plugin_src_route.py

@@ -138,3 +138,119 @@ def test_unready_plugin_src_is_404(client):
     c, _ = client
     plugins.LOADED_PLUGINS[0]["status"] = "installing"
     assert c.get(f"/api/plugins/{PLUGIN_ID}/src/main.js").status_code == 404
+
+
+# ── #879: the /g/<token>/ generation prefix ────────────────────────────────────
+#
+# A plugin ROLLBACK must actually re-evaluate a module plugin. ES modules are
+# evaluated once per URL per document, so re-inserting a <script type="module">
+# whose src the module map has already seen fires `load` without re-running the
+# body. Busting the ENTRY url alone does not help — screen.js is a one-line
+# `import './src/main.js'`, and a relative specifier resolves against the base URL
+# with the QUERY DROPPED, so a ?v= token never reaches the graph.
+#
+# Hence a token in the PATH: every relative import inherits it, at every depth,
+# with no import-specifier rewriting. These routes must serve the SAME bytes and
+# keep the SAME containment.
+
+def test_generation_prefix_serves_identical_screen_js(client):
+    c, _ = client
+    plain = c.get(f"/api/plugins/{PLUGIN_ID}/screen.js")
+    gen = c.get(f"/api/plugins/{PLUGIN_ID}/g/7/screen.js")
+    assert gen.status_code == 200
+    assert gen.content == plain.content
+    assert "import './src/main.js'" in gen.text
+
+
+def test_generation_prefix_serves_the_whole_module_graph(client):
+    """The point of the path token: a relative import from a /g/7/ entry resolves
+    to a /g/7/ URL, so the graph is fetched fresh — not just the entry."""
+    c, _ = client
+    main = c.get(f"/api/plugins/{PLUGIN_ID}/g/7/src/main.js")
+    assert main.status_code == 200
+    assert main.text == c.get(f"/api/plugins/{PLUGIN_ID}/src/main.js").text
+    # and one level deeper, which is where a query-string token would already have
+    # been lost twice over
+    nested = c.get(f"/api/plugins/{PLUGIN_ID}/g/7/src/util/x.js")
+    assert nested.status_code == 200
+    assert "export const x = 42" in nested.text
+
+
+def test_generation_token_is_opaque(client):
+    """Any token serves the same bytes — it exists only to vary the URL."""
+    c, _ = client
+    a = c.get(f"/api/plugins/{PLUGIN_ID}/g/1/src/main.js")
+    b = c.get(f"/api/plugins/{PLUGIN_ID}/g/999999/src/main.js")
+    assert a.status_code == b.status_code == 200
+    assert a.text == b.text
+
+
+def test_generation_prefix_does_not_widen_containment(client):
+    """The token is never joined into a path, so containment must be EXACTLY what the
+    un-prefixed route already gives. Asserted as parity rather than as a flat 404:
+    `../screen.js` legitimately 200s on BOTH, because the URL normalises to
+    /api/plugins/<id>/screen.js before routing ever happens — it never leaves the
+    plugin dir. Pinning an absolute expectation here would have encoded my guess
+    about the existing route instead of testing the thing that matters, which is
+    that /g/ changes nothing."""
+    c, _ = client
+    for bad in ("../screen.js", "../../etc/passwd", "..%2f..%2fetc%2fpasswd",
+                "..%5c..%5cwindows%5cwin.ini", "/etc/passwd"):
+        plain = c.get(f"/api/plugins/{PLUGIN_ID}/src/{bad}")
+        gen = c.get(f"/api/plugins/{PLUGIN_ID}/g/1/src/{bad}")
+        assert gen.status_code == plain.status_code, f"/g/ diverged on {bad!r}"
+        assert gen.content == plain.content, f"/g/ served different bytes for {bad!r}"
+        assert "root:" not in gen.text and "[extensions]" not in gen.text
+
+    # and the real traversals are genuinely rejected, on both
+    for bad in ("../../etc/passwd", "..%2f..%2fetc%2fpasswd"):
+        assert c.get(f"/api/plugins/{PLUGIN_ID}/g/1/src/{bad}").status_code == 404
+
+
+def test_generation_prefix_404s_for_unknown_plugin(client):
+    c, _ = client
+    assert c.get("/api/plugins/nope/g/1/screen.js").status_code == 404
+    assert c.get("/api/plugins/nope/g/1/src/main.js").status_code == 404
+
+
+def test_generation_prefix_serves_ASSETS_too(client):
+    """Codex [P2] on the first cut of this fix, and it was right.
+
+    The path token shifts the BASE URL, so everything a module resolves relatively moves
+    with it — not just imports. `new URL('../assets/worklet.js', import.meta.url)` from
+    /api/plugins/<id>/g/1/src/main.js resolves to /api/plugins/<id>/g/1/assets/worklet.js.
+    Mirroring only screen.js and src/ would have fixed imports and 404'd every asset,
+    worklet and wasm file the graph reaches. Hence a path REWRITE, so every plugin route
+    — present and future — works under the prefix."""
+    c, _ = client
+    plain = c.get(f"/api/plugins/{PLUGIN_ID}/assets/worklet.js")
+    gen = c.get(f"/api/plugins/{PLUGIN_ID}/g/1/assets/worklet.js")
+    assert plain.status_code == 200
+    assert gen.status_code == 200, "an asset reached relatively from a reloaded module graph 404'd"
+    assert gen.content == plain.content
+
+
+def test_generation_prefix_covers_every_plugin_route(client):
+    """The rewrite is generic, so this holds for routes nobody thought about — which is
+    the point. Any plugin route added later works under /g/ with no extra wiring."""
+    c, _ = client
+    for route in ("screen.js", "src/main.js", "src/util/x.js", "src/theme.css",
+                  "assets/worklet.js", "settings.html"):
+        plain = c.get(f"/api/plugins/{PLUGIN_ID}/{route}")
+        gen = c.get(f"/api/plugins/{PLUGIN_ID}/g/42/{route}")
+        assert gen.status_code == plain.status_code, f"/g/ diverged on {route}"
+        assert gen.content == plain.content, f"/g/ served different bytes for {route}"
+
+
+def test_generation_prefix_handles_non_ascii_filenames(client):
+    """Codex [P3] on the second cut. A plugin file named e.g. src/工具.js is perfectly
+    valid, and the middleware must not 500 on it — which an eager
+    raw_path.encode("latin-1") did, making the prefixed route LESS capable than the
+    plain one. raw_path is informational; Starlette routes on scope["path"]."""
+    c, tmp = client
+    (tmp / "src" / "工具.js").write_text("export const t = 1;\n")
+    plain = c.get(f"/api/plugins/{PLUGIN_ID}/src/工具.js")
+    gen = c.get(f"/api/plugins/{PLUGIN_ID}/g/3/src/工具.js")
+    assert plain.status_code == 200
+    assert gen.status_code == 200, "non-ASCII module path 500'd or 404'd under /g/"
+    assert gen.content == plain.content

tests/test_plugins.py

@@ -46,56 +46,6 @@ def capture_logger(caplog, logger_name, level=logging.WARNING):
         logger.propagate = orig_propagate
 
 
-# Bare module names that this test module pre-populates into
-# sys.modules to simulate the bare-import path. Saved/restored by
-# the reset_plugin_state fixture so they don't leak to other test
-# files. Codex / Copilot review on PR for feedBack#33.
-_BARE_NAMES_USED = ("util", "extractor")
-
-
-@pytest.fixture()
-def reset_plugin_state(monkeypatch):
-    """Clear loader module-level state and restore on teardown.
-
-    Saves and restores:
-      * `plugins.LOADED_PLUGINS`
-      * any `plugin_*` keys we add to `sys.modules`
-      * the bare names this module simulates (`util`, `extractor`)
-      * `sys.path` — `plugins.load_plugins()` mutates it
-    Also unsets `FEEDBACK_PLUGINS_DIR` for the test's duration
-    (via monkeypatch) so a CI env that pre-sets it can't leak
-    real user plugins into a tmp_path-driven test. Per-module
-    locks are owned by the standard import system
-    (`importlib._bootstrap._module_locks`) and are not our
-    responsibility to reset.
-    """
-    monkeypatch.delenv("FEEDBACK_PLUGINS_DIR", raising=False)
-    plugins = importlib.import_module("plugins")
-    saved_loaded = list(plugins.LOADED_PLUGINS)
-    saved_pending = dict(plugins.PENDING_PLUGINS)
-    saved_modules = {k: v for k, v in sys.modules.items() if k.startswith("plugin_")}
-    saved_bare = {k: sys.modules[k] for k in _BARE_NAMES_USED if k in sys.modules}
-    saved_path = list(sys.path)
-    plugins.LOADED_PLUGINS.clear()
-    plugins.PENDING_PLUGINS.clear()
-    for k in list(sys.modules):
-        if k.startswith("plugin_") or k in _BARE_NAMES_USED:
-            del sys.modules[k]
-    try:
-        yield plugins
-    finally:
-        plugins.LOADED_PLUGINS.clear()
-        plugins.LOADED_PLUGINS.extend(saved_loaded)
-        plugins.PENDING_PLUGINS.clear()
-        plugins.PENDING_PLUGINS.update(saved_pending)
-        for k in list(sys.modules):
-            if k.startswith("plugin_") or k in _BARE_NAMES_USED:
-                del sys.modules[k]
-        sys.modules.update(saved_modules)
-        sys.modules.update(saved_bare)
-        sys.path[:] = saved_path
-
-
 def _make_plugin(plugin_root, plugin_id, *, sibling_files=None, routes_body=None):
     """Create a minimal plugin directory under `plugin_root`.
 

tests/test_progression_api.py

@@ -5,6 +5,7 @@ import importlib
 import json
 import sys
 
+import builtin_content
 import pytest
 from fastapi.testclient import TestClient
 
@@ -192,7 +193,7 @@ def test_low_accuracy_play_does_not_complete_gated_challenge(client):
 
 
 def test_diagnostic_at_100_completes_calibration(client, server):
-    diag = server._builtin_diagnostic_filename()
+    diag = builtin_content.builtin_diagnostic_filename()
     # A near-miss leaves calibration pending.
     _scored_play(client, filename=diag, accuracy=0.97, score=500)
     assert client.get("/api/progression").json()["onboarding"]["calibration_status"] == "pending"
@@ -207,7 +208,7 @@ def test_diagnostic_play_does_not_feed_challenges_or_quests(client, server):
     # The calibration run is a perfect guitar play — it must yield rank 1
     # EXACTLY, advancing neither the guitar path nor the daily song quest.
     client.post("/api/progression/paths", json={"add": ["guitar"]})
-    r = _scored_play(client, filename=server._builtin_diagnostic_filename(),
+    r = _scored_play(client, filename=builtin_content.builtin_diagnostic_filename(),
                      accuracy=1.0, score=500)
     summary = r.json()["progression"]
     assert summary["calibration_completed"] is True
@@ -228,7 +229,7 @@ def test_pathless_diagnostic_run_still_completes_calibration(client, server):
     run is an earned achievement and must count even before any path is
     selected (e.g. a pre-progression profile playing the diagnostic as a
     hardware test) — yielding a valid pathless rank-1 state."""
-    _scored_play(client, filename=server._builtin_diagnostic_filename(),
+    _scored_play(client, filename=builtin_content.builtin_diagnostic_filename(),
                  accuracy=1.0, score=500)
     data = client.get("/api/progression").json()
     assert data["onboarding"]["calibration_status"] == "completed"
@@ -240,7 +241,7 @@ def test_diagnostic_upgrades_skipped_without_rank_change(client, server):
     client.post("/api/progression/paths", json={"add": ["guitar"]})
     r = client.post("/api/progression/onboarding", json={"action": "skip"})
     assert r.json()["onboarding"]["calibration_status"] == "skipped"
-    _scored_play(client, filename=server._builtin_diagnostic_filename(), accuracy=1.0, score=500)
+    _scored_play(client, filename=builtin_content.builtin_diagnostic_filename(), accuracy=1.0, score=500)
     data = client.get("/api/progression").json()
     assert data["onboarding"]["calibration_status"] == "completed"
     assert data["mastery_rank"] == 1

tests/test_settings_api.py

@@ -429,11 +429,11 @@ def test_get_dlc_dir_ignores_nonexistent_config_dir(tmp_path, server_module):
 # ── library scan fixtures ────────────────────────────────────────────────────
 
 @pytest.fixture()
-def scan_module(tmp_path, monkeypatch, isolate_logging):
+def scan_module(tmp_path, monkeypatch, isolate_logging, reset_scan_state):
     """Import server with CONFIG_DIR and DLC_DIR isolated in tmp_path.
 
     The background scan uses a `spawn` ProcessPoolExecutor in production
-    (see server._make_scan_executor), whose workers run in fresh
+    (see scan._make_scan_executor), whose workers run in fresh
     interpreters that an in-process mock.patch() can't reach. Override it
     with an in-process ThreadPoolExecutor so these tests can mock metadata
     extraction (on scan_worker, where the worker resolves it) and observe
@@ -444,8 +444,13 @@ def scan_module(tmp_path, monkeypatch, isolate_logging):
     monkeypatch.delenv("DLC_DIR", raising=False)
     sys.modules.pop("server", None)
     mod = importlib.import_module("server")
+    # The scanner is lib/scan.py now (R3b). Patch it THERE — `mod` (server) re-imports
+    # per-test, but `scan` stays cached in sys.modules, so this is the same module object
+    # server calls into. That it still works is the point of the late-bound appstate
+    # reads: scan picks up the fresh CONFIG_DIR without being re-imported itself.
+    import scan as scan_mod
     monkeypatch.setattr(
-        mod, "_make_scan_executor",
+        scan_mod, "_make_scan_executor",
         lambda: concurrent.futures.ThreadPoolExecutor(max_workers=4),
     )
     yield mod
@@ -485,11 +490,11 @@ def test_is_first_scan_true_when_all_songs_unscanned(tmp_path, scan_module):
     def mock_extract(f, dlc):
         # Capture the scan status on the first call (during the scanning phase)
         if not captured_status:
-            captured_status.update(scan_module._scan_status)
+            captured_status.update(importlib.import_module("scan").status())
         return {"title": f.name, "artist": "", "album": ""}
 
     with mock.patch("scan_worker._extract_meta_for_file", new=mock_extract):
-        scan_module._background_scan()
+        importlib.import_module("scan").background_scan()
 
     assert captured_status.get("is_first_scan") is True
 
@@ -514,11 +519,11 @@ def test_is_first_scan_false_when_some_songs_cached(tmp_path, scan_module):
 
     def mock_extract(f, dlc):
         if not captured_status:
-            captured_status.update(scan_module._scan_status)
+            captured_status.update(importlib.import_module("scan").status())
         return {"title": f.name, "artist": "", "album": ""}
 
     with mock.patch("scan_worker._extract_meta_for_file", new=mock_extract):
-        scan_module._background_scan()
+        importlib.import_module("scan").background_scan()
 
     assert captured_status.get("is_first_scan") is False
 

tests/test_startup_status.py

@@ -11,6 +11,7 @@ import time
 import asyncio
 
 import httpx
+import demo_mode
 import pytest
 from fastapi.testclient import TestClient
 
@@ -120,12 +121,12 @@ def startup_harness(tmp_path, monkeypatch, isolate_logging):
 
     yield server, phases
 
-    server._DEMO_JANITOR_STOP.set()
-    thread = server._DEMO_JANITOR_THREAD
+    demo_mode._DEMO_JANITOR_STOP.set()
+    thread = demo_mode._DEMO_JANITOR_THREAD
     if thread is not None:
         thread.join(timeout=2)
-    server._DEMO_JANITOR_STARTED = False
-    server._DEMO_JANITOR_THREAD = None
+    demo_mode._DEMO_JANITOR_STARTED = False
+    demo_mode._DEMO_JANITOR_THREAD = None
     conn = getattr(getattr(server, "meta_db", None), "conn", None)
     if conn is not None:
         getattr(__import__("sys").modules.get("server"), "_join_background_db_threads", lambda: None)()
@@ -686,12 +687,12 @@ def test_startup_status_e2e_real_plugin_loader(tmp_path, monkeypatch, isolate_lo
             assert sentinel.status_code == 200
             assert sentinel.json() == {"ok": True}
     finally:
-        server._DEMO_JANITOR_STOP.set()
-        thread = server._DEMO_JANITOR_THREAD
+        demo_mode._DEMO_JANITOR_STOP.set()
+        thread = demo_mode._DEMO_JANITOR_THREAD
         if thread is not None:
             thread.join(timeout=2)
-        server._DEMO_JANITOR_STARTED = False
-        server._DEMO_JANITOR_THREAD = None
+        demo_mode._DEMO_JANITOR_STARTED = False
+        demo_mode._DEMO_JANITOR_THREAD = None
         conn = getattr(getattr(server, "meta_db", None), "conn", None)
         if conn is not None:
             getattr(__import__("sys").modules.get("server"), "_join_background_db_threads", lambda: None)()
@@ -777,12 +778,12 @@ def test_startup_status_endpoint_background_thread_path(tmp_path, monkeypatch, i
             # actually executed the sentinel — proves the main-loop handoff path ran.
             assert _route_setup_called, "route_setup_fn was never called; call_soon_threadsafe path was not exercised"
     finally:
-        server._DEMO_JANITOR_STOP.set()
-        thread = server._DEMO_JANITOR_THREAD
+        demo_mode._DEMO_JANITOR_STOP.set()
+        thread = demo_mode._DEMO_JANITOR_THREAD
         if thread is not None:
             thread.join(timeout=2)
-        server._DEMO_JANITOR_STARTED = False
-        server._DEMO_JANITOR_THREAD = None
+        demo_mode._DEMO_JANITOR_STARTED = False
+        demo_mode._DEMO_JANITOR_THREAD = None
         conn = getattr(getattr(server, "meta_db", None), "conn", None)
         if conn is not None:
             getattr(__import__("sys").modules.get("server"), "_join_background_db_threads", lambda: None)()
@@ -826,12 +827,12 @@ def test_startup_status_endpoint_background_thread_failure(tmp_path, monkeypatch
             assert data["phase"] == "error"
             assert _BG_ERROR in data["error"]
     finally:
-        server._DEMO_JANITOR_STOP.set()
-        thread = server._DEMO_JANITOR_THREAD
+        demo_mode._DEMO_JANITOR_STOP.set()
+        thread = demo_mode._DEMO_JANITOR_THREAD
         if thread is not None:
             thread.join(timeout=2)
-        server._DEMO_JANITOR_STARTED = False
-        server._DEMO_JANITOR_THREAD = None
+        demo_mode._DEMO_JANITOR_STARTED = False
+        demo_mode._DEMO_JANITOR_THREAD = None
         conn = getattr(getattr(server, "meta_db", None), "conn", None)
         if conn is not None:
             getattr(__import__("sys").modules.get("server"), "_join_background_db_threads", lambda: None)()

tests/test_tuning_provider_isolation.py

@@ -0,0 +1,72 @@
+"""A raising tuning provider must not take down get_merged() for everyone. (#899)
+
+`TuningProviderRegistry.get_merged()` wraps each provider in a try/except precisely so one
+misbehaving plugin cannot break tunings for the rest. The handler called `logger.exception`
+— and there is no `logger` in server.py; the module logger is `log`. So the handler MEANT
+to swallow-and-report instead raised NameError, which propagated out of get_merged().
+
+The net effect was the exact opposite of the handler's purpose: one bad provider took the
+whole merged-tunings call down, and the traceback named the wrong problem.
+
+Nothing exercised the failure path, which is why it survived. This is that path.
+"""
+
+import importlib
+import logging
+import sys
+
+import pytest
+
+
+@pytest.fixture()
+def registry(monkeypatch, tmp_path):
+    monkeypatch.setenv("CONFIG_DIR", str(tmp_path))
+    sys.modules.pop("server", None)
+    mod = importlib.import_module("server")
+    yield mod.TuningProviderRegistry()
+
+
+def test_a_raising_provider_does_not_break_the_others(registry, caplog):
+    """The whole point of the try/except. Before the fix this raised NameError."""
+    def boom():
+        raise RuntimeError("provider exploded")
+
+    def good():
+        return {"guitar": {"My Tuning": [82.41, 110.0, 146.83, 196.0, 246.94, 329.63]}}
+
+    registry.register("bad-plugin", boom)
+    registry.register("good-plugin", good)
+
+    merged = registry.get_merged()          # must NOT raise
+
+    assert "My Tuning" in merged["guitar"], (
+        "the healthy provider's tuning is missing — one raising provider took down the "
+        "merged result for everyone"
+    )
+    # and the default tunings survive
+    assert merged["guitar"], "default tunings were lost"
+
+
+def test_the_failure_is_actually_logged(registry, caplog):
+    """Swallowing is only acceptable if it is reported. A NameError in the handler meant
+    nothing was ever logged — the failure was both fatal AND silent about its real cause."""
+    def boom():
+        raise RuntimeError("provider exploded")
+
+    registry.register("bad-plugin", boom)
+
+    # The feedBack logger sets propagate=False, so pytest's root-logger capture sees
+    # NOTHING from it. Attach caplog's handler directly. (test_plugins.py has a
+    # capture_logger() context manager for this, but it is not importable from here:
+    # pyproject pins pythonpath to [".", "lib"], so `tests` is not a package.)
+    lg = logging.getLogger("feedBack")
+    lg.addHandler(caplog.handler)
+    lg.setLevel(logging.ERROR)
+    try:
+        registry.get_merged()
+    finally:
+        lg.removeHandler(caplog.handler)
+
+    assert any("bad-plugin" in r.getMessage() for r in caplog.records), (
+        "the raising provider was never named in the logs"
+    )