refactor(server): carve demo mode into lib/demo_mode.py (R3b) (#903)

lib/demo_mode.py (342). server.py 1,870 -> 1,649.

The read-only request guard (its 96-entry blocked-route table + the middleware) and the
hourly session janitor (registry, hook runner, thread). Bodies VERBATIM.

THE MIDDLEWARE NEEDS `app`, SO THE MODULE TAKES IT. _demo_mode_guard is an
@app.middleware("http") and cannot exist without an app object. Rather than have a module
under lib/ reach for a global, it exposes install(app) and server.py — which owns the app —
hands it over. The janitor is symmetrical: start_janitor() / stop_janitor(), called from
server.py's startup and shutdown hooks, 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(). server.py imports this exact object
and puts it in the dict unchanged — identity preserved, and
tests/test_plugin_context_contract.py (#898, merged) fails if that ever stops being true.
This is the first carve that guard has actually protected.

━━━ stop_janitor()'s ORDER IS LOAD-BEARING ━━━

The obvious way to write it — clear the "started" flag, then join — is WRONG, and I wrote
it that way first. server.py's original deliberately returns EARLY, leaving
_DEMO_JANITOR_STARTED True and the thread handle intact, when the thread outlives the join:

    # Leave _DEMO_JANITOR_STARTED True so a new janitor is not
    # spawned by a subsequent startup while the old one is alive.

Clearing the flag first quietly reintroduces exactly the double-janitor leak the flag
exists to prevent. Preserved byte-for-byte, and the reason is now written down at the
function rather than only at its single call site.

━━━ A BUG MOVED VERBATIM, ON PURPOSE ━━━

    if getenv_compat("FEEDBACK_DEMO_MODE") or getenv_compat("FEEDBACK_DEMO_MODE") == "1" \
            and not _DEMO_JANITOR_STARTED:

`and` binds tighter than `or`, so this is `A or (B and C)` — the not-already-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). Verified. Preserved exactly and filed as issue #902: a carve whose whole value
is being provably behaviour-neutral is not the place to change behaviour.

pyflakes caught three more missing imports on the way in (uuid, warnings x2). Five carves,
ten missing imports, every one a NameError on a live path.

pytest 2399, pyflakes 0, Codex 0.

Refs #48

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

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

server.py

@@ -45,6 +45,7 @@ from dlc_paths import _get_dlc_dir, _resolve_dlc_path
 # Lives in lib/ because that is the one core dir every packaging path copies.
 import appstate
 import builtin_content
+import demo_mode
 import scan
 # Extracted route modules. They import `appstate`, never `server` — one-way graph.
 from routers import audio_effects, artist_aliases, loops, playlists, ws_highway, chart, wanted, library_extras, shop, progression, profile, stats, version, diagnostics
@@ -81,222 +82,18 @@ from fastapi import Request
 
 app = FastAPI(title="FeedBack")
 
-# Plugins that maintain session stores can register a cleanup callback here.
+# Demo mode lives in lib/demo_mode.py now. The guard is a middleware, so it needs the app —
-# The demo-mode janitor calls every registered hook once per hour so stale
+# server.py owns it and hands it over rather than making lib/ reach for a global.
-# sessions are swept without the core needing to know plugin internals.
+demo_mode.install(app)
-_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$")),
-]
 
 
-@app.middleware("http")
-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)
 
 from asgi_correlation_id import CorrelationIdMiddleware
 
@@ -915,7 +712,7 @@ async def startup_events():
         "register_tuning_provider": register_tuning_provider,
         "unregister_tuning_provider": unregister_tuning_provider,
         "get_sloppak_cache_dir": lambda: SLOPPAK_CACHE_DIR,
-        "register_demo_janitor_hook": register_demo_janitor_hook,
+        "register_demo_janitor_hook": demo_mode.register_demo_janitor_hook,
         # Unified XP service (fee[dB]ack v0.3.0). Plugins that award XP
         # (minigames, tutorials, …) should feed the single core store via these
         # instead of keeping a private XP curve. `award_xp` returns the new
@@ -1173,18 +970,13 @@ async def startup_events():
     else:
         threading.Thread(target=_load_plugins_background, daemon=True).start()
 
-    global _DEMO_JANITOR_STARTED, _DEMO_JANITOR_THREAD
+    # NB the `or ... == "1" and not started` shape below is PRESERVED VERBATIM: `and` binds
-    if getenv_compat("FEEDBACK_DEMO_MODE") or getenv_compat("FEEDBACK_DEMO_MODE") == "1" and not _DEMO_JANITOR_STARTED:
+    # tighter than `or`, so the re-entry guard is dead whenever the env var is truthy, and a
-        _DEMO_JANITOR_STARTED = True
+    # second startup leaks a janitor thread. That is issue #902 — not fixed here, because a
-        _DEMO_JANITOR_STOP.clear()
+    # carve whose value is being provably behaviour-neutral is not the place to change
-        def _janitor():
+    # behaviour.
-            while not _DEMO_JANITOR_STOP.wait(timeout=3600):
+    if getenv_compat("FEEDBACK_DEMO_MODE") or getenv_compat("FEEDBACK_DEMO_MODE") == "1" and not demo_mode.janitor_started():
-                with _DEMO_JANITOR_HOOKS_LOCK:
+        demo_mode.start_janitor()
-                    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()
 
     # Start background metadata scan
     startup_scan()
@@ -1193,28 +985,15 @@ async def startup_events():
 @app.on_event("shutdown")
 def shutdown_events():
     """Stop the demo-mode janitor thread (if running) on server shutdown."""
-    global _DEMO_JANITOR_STARTED, _DEMO_JANITOR_THREAD, _event_loop
+    global _event_loop
     _event_loop = None  # prevent stale loop reference after shutdown
-    if _DEMO_JANITOR_STARTED:
+    if not demo_mode.stop_janitor(timeout=5):
-        _DEMO_JANITOR_STOP.set()
+        warnings.warn(
-        thread = _DEMO_JANITOR_THREAD
+            "demo-janitor thread did not stop within 5 s; "
-        if thread is not None:
+            "a registered hook may be blocking",
-            thread.join(timeout=5)
+            RuntimeWarning,
-            if thread.is_alive():
+            stacklevel=1,
-                import warnings
+        )
-                warnings.warn(
-                    "demo-janitor thread did not stop within 5 s; "
-                    "a registered hook may be blocking",
-                    RuntimeWarning,
-                    stacklevel=1,
-                )
-                # Leave _DEMO_JANITOR_STARTED True so a new janitor is not
-                # spawned by a subsequent startup while the old one is alive.
-                return
-            _DEMO_JANITOR_THREAD = None
-        _DEMO_JANITOR_STARTED = False
-        with _DEMO_JANITOR_HOOKS_LOCK:
-            _DEMO_JANITOR_HOOKS.clear()
 
 
 def startup_scan():

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()
+    demo_mode._DEMO_JANITOR_STOP.set()
-    thread = server._DEMO_JANITOR_THREAD
+    thread = demo_mode._DEMO_JANITOR_THREAD
     if thread is not None:
         thread.join(timeout=2)
-    server._DEMO_JANITOR_STARTED = False
+    demo_mode._DEMO_JANITOR_STARTED = False
-    server._DEMO_JANITOR_THREAD = None
+    demo_mode._DEMO_JANITOR_THREAD = None
-    with server._DEMO_JANITOR_HOOKS_LOCK:
+    with demo_mode._DEMO_JANITOR_HOOKS_LOCK:
-        server._DEMO_JANITOR_HOOKS.clear()
+        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:
+        with demo_mode._DEMO_JANITOR_HOOKS_LOCK:
-            server._DEMO_JANITOR_HOOKS.clear()
+            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:
+        with demo_mode._DEMO_JANITOR_HOOKS_LOCK:
-            server._DEMO_JANITOR_HOOKS.clear()
+            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()
+    demo_mode._DEMO_JANITOR_STOP.set()
-    thread = server._DEMO_JANITOR_THREAD
+    thread = demo_mode._DEMO_JANITOR_THREAD
     if thread is not None:
         thread.join(timeout=2)
-    server._DEMO_JANITOR_STARTED = False
+    demo_mode._DEMO_JANITOR_STARTED = False
-    server._DEMO_JANITOR_THREAD = None
+    demo_mode._DEMO_JANITOR_THREAD = None
-    with server._DEMO_JANITOR_HOOKS_LOCK:
+    with demo_mode._DEMO_JANITOR_HOOKS_LOCK:
-        server._DEMO_JANITOR_HOOKS.clear()
+        demo_mode._DEMO_JANITOR_HOOKS.clear()
 
 
 

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()
+    demo_mode._DEMO_JANITOR_STOP.set()
-    thread = server._DEMO_JANITOR_THREAD
+    thread = demo_mode._DEMO_JANITOR_THREAD
     if thread is not None:
         thread.join(timeout=2)
-    server._DEMO_JANITOR_STARTED = False
+    demo_mode._DEMO_JANITOR_STARTED = False
-    server._DEMO_JANITOR_THREAD = None
+    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()
+        demo_mode._DEMO_JANITOR_STOP.set()
-        thread = server._DEMO_JANITOR_THREAD
+        thread = demo_mode._DEMO_JANITOR_THREAD
         if thread is not None:
             thread.join(timeout=2)
-        server._DEMO_JANITOR_STARTED = False
+        demo_mode._DEMO_JANITOR_STARTED = False
-        server._DEMO_JANITOR_THREAD = None
+        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()
+        demo_mode._DEMO_JANITOR_STOP.set()
-        thread = server._DEMO_JANITOR_THREAD
+        thread = demo_mode._DEMO_JANITOR_THREAD
         if thread is not None:
             thread.join(timeout=2)
-        server._DEMO_JANITOR_STARTED = False
+        demo_mode._DEMO_JANITOR_STARTED = False
-        server._DEMO_JANITOR_THREAD = None
+        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()
+        demo_mode._DEMO_JANITOR_STOP.set()
-        thread = server._DEMO_JANITOR_THREAD
+        thread = demo_mode._DEMO_JANITOR_THREAD
         if thread is not None:
             thread.join(timeout=2)
-        server._DEMO_JANITOR_STARTED = False
+        demo_mode._DEMO_JANITOR_STARTED = False
-        server._DEMO_JANITOR_THREAD = None
+        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)()