fix(tailwind): stop the dev server rewriting a tracked file (#911) (#918)

The runtime stylesheet moves to CONFIG_DIR. static/tailwind.min.css is never written again.

━━━ TWO DIFFERENT THINGS WERE SHARING ONE PATH ━━━

    static/tailwind.min.css   a BUILD ARTEFACT. Committed, image-baked, generated by scanning
                              the in-tree plugins only. CI's tailwind-fresh check verifies it.
    the RUNTIME sheet         PER-INSTALL STATE. Additionally scans whatever the user installed
                              into FEEDBACK_PLUGINS_DIR, so it differs machine to machine.

Writing the second over the first meant that MERELY RUNNING THE DEV SERVER from a git checkout
silently modified a tracked file. `git add -A` then swept a 100KB reshuffle of minified CSS
into the commit and ci/tailwind-fresh went red with a diff that explains nothing — on a PR
whose real change touched no Tailwind classes at all. It also wrote app state into the app
directory, which is read-only in some deploys.

A new route serves the runtime sheet when there is one and falls back to the committed one
otherwise. It is registered BEFORE the /static mount, which would otherwise swallow the path.

━━━ A PERSISTED SHEET MUST NOT OUTLIVE ITS REASON (Codex [P2] x2) ━━━

1. THE USER REMOVES THEIR PLUGINS. Startup only rebuilds when user plugins exist, so nothing
   would ever overwrite the stale sheet — and it still carries classes for plugins that are
   gone. With no user plugins the COMMITTED sheet is complete by definition. Guarded.

2. THE APP IS UPGRADED, and my first guard for this was WRONG. I compared mtimes. Codex: that
   is not a freshness signal across install methods — archives and container images routinely
   PRESERVE SOURCE MTIMES, so a just-shipped stylesheet can carry an OLDER timestamp than a
   runtime sheet a user built days ago. The mtime check then calls the stale one FRESH and it
   masks the new core CSS indefinitely — permanently, if no Tailwind toolchain is present to
   trigger a rebuild.

   Freshness is decided by CONTENT now. Each runtime build stamps a sidecar with the sha256 of
   the committed sheet it was made from. Core ships new CSS -> that file changes -> the hash
   changes -> the runtime sheet is correctly judged stale. Timestamps only gesture at the
   question that hashing answers.

Falling back to the committed sheet is always safe: at worst it lacks a just-installed plugin's
classes for the seconds until the async rebuild lands.

VERIFIED END TO END. Ran the real dev server with 3 plugins installed: it rebuilt Tailwind over
them (123,291 bytes), wrote the sheet + sidecar to CONFIG_DIR, still served /static/
tailwind.min.css at 200 — and `git diff` on the tracked file came back CLEAN.

8 tests. Bite-tested: reverting to the shared path fails 3, dropping the staleness guards fails
2 more.

pytest 2425, pyflakes 0, Codex 0.

Closes #911

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

lib/tailwind_rebuild.py

@@ -1,4 +1,4 @@
-"""Regenerate ``static/tailwind.min.css`` over the full installed-plugin set.
+"""Regenerate the runtime stylesheet over the full installed-plugin set.
 
 Core's committed (and image-baked) stylesheet is built scanning only the
 in-tree plugins. A plugin installed at runtime — into ``FEEDBACK_PLUGINS_DIR``
@@ -15,6 +15,7 @@ on a missing optional engine.
 
 from __future__ import annotations
 
+import hashlib
 import json
 import logging
 import os
@@ -40,12 +41,84 @@ _lock = threading.Lock()
 # in-flight build re-runs once more to pick up the newer plugin set instead of
 # every concurrent trigger stacking its own redundant build.
 _rerun = threading.Event()
+_fingerprint_cache: dict = {}
 
 # lib/ lives at ``<app>/lib``; the app root (static/, tailwind.config.js) is its
 # grandparent.
 APP_DIR = Path(__file__).resolve().parent.parent
 
 
+def _committed_css_fingerprint() -> str:
+    """Content hash of the SHIPPED stylesheet, cached on (mtime, size).
+
+    This is the marker that says WHICH CORE the runtime sheet was built against. Any change to
+    core's CSS regenerates static/tailwind.min.css, which changes this hash.
+    """
+    committed = APP_DIR / "static" / "tailwind.min.css"
+    try:
+        st = committed.stat()
+    except OSError:
+        return ""
+    key = (st.st_mtime_ns, st.st_size)
+    cached = _fingerprint_cache.get("k")
+    if cached == key:
+        return _fingerprint_cache["v"]
+    h = hashlib.sha256(committed.read_bytes()).hexdigest()
+    _fingerprint_cache["k"] = key
+    _fingerprint_cache["v"] = h
+    return h
+
+
+def runtime_meta_path() -> Path:
+    """Sidecar recording which core the runtime sheet was built against."""
+    return runtime_css_path().with_suffix(".meta.json")
+
+
+def runtime_css_is_current() -> bool:
+    """True when the runtime sheet was built against the core we are running NOW.
+
+    WHY NOT mtime. Codex [P2] on the second cut of #911, and it was right: filesystem
+    timestamps are not a freshness signal across install methods. Archives and container images
+    routinely PRESERVE SOURCE MTIMES, so a just-shipped stylesheet can carry an OLDER mtime than
+    a runtime sheet a user built days ago. The mtime comparison then reports the stale sheet as
+    fresh and it masks the new core CSS indefinitely — permanently, if no Tailwind toolchain is
+    present to trigger a rebuild.
+
+    Content answers the question timestamps only gesture at: the sidecar records the hash of the
+    committed sheet this runtime build was made from. Core ships new CSS -> that file changes ->
+    the hash changes -> the runtime sheet is correctly judged stale.
+    """
+    try:
+        meta = json.loads(runtime_meta_path().read_text())
+    except (OSError, ValueError):
+        return False
+    return bool(meta.get("committed_sha256")) and meta["committed_sha256"] == _committed_css_fingerprint()
+
+
+def runtime_css_path() -> Path:
+    """Where the RUNTIME-augmented stylesheet is written.
+
+    NOT ``static/tailwind.min.css``. That file is a BUILD ARTEFACT: committed, image-baked,
+    and generated by scanning only the in-tree plugins. This one is PER-INSTALL STATE — it
+    additionally scans whatever the user has installed into FEEDBACK_PLUGINS_DIR, so it differs
+    from machine to machine. They are different things and must not share a path.
+
+    Writing the runtime sheet over the committed one had two costs:
+
+      * IN A GIT CHECKOUT it silently modifies a TRACKED file. `git add -A` then sweeps a
+        100KB reshuffle of minified CSS into the commit and `ci/tailwind-fresh` goes red with a
+        diff that explains nothing. That is issue #911, and it cost a red run on a PR whose
+        real diff touched no Tailwind classes at all.
+      * IN A DEPLOY the app directory may be read-only. Writing app state into it is wrong on
+        principle and fatal in practice.
+
+    CONFIG_DIR is where per-install state already lives.
+    """
+    cfg = (getenv_compat("CONFIG_DIR", "") or "").strip()
+    base = Path(cfg) if cfg else (Path.home() / ".local" / "share" / "feedback")
+    return base / "tailwind.min.css"
+
+
 def _user_plugins_dir() -> Path | None:
     raw = (getenv_compat("FEEDBACK_PLUGINS_DIR", "") or "").strip()
     if not raw:
@@ -136,6 +209,14 @@ def _run_build(cmd_prefix: list[str], out: Path, src: Path) -> bool:
                 cwd=str(APP_DIR), timeout=120,
             )
             os.replace(staged, out)
+            # Stamp WHICH CORE this was built against. Without it, an upgraded app cannot tell a
+            # current runtime sheet from one that predates its new CSS.
+            try:
+                runtime_meta_path().write_text(json.dumps({
+                    "committed_sha256": _committed_css_fingerprint(),
+                }))
+            except OSError:
+                log.warning("tailwind: could not write the runtime sheet's meta sidecar")
             return True
         except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as e:
             stderr = (getattr(e, "stderr", "") or "")[-500:]
@@ -153,7 +234,7 @@ def _run_build(cmd_prefix: list[str], out: Path, src: Path) -> bool:
 
 
 def rebuild(reason: str = "") -> bool:
-    """Regenerate ``static/tailwind.min.css`` over baked-in + user plugins.
+    """Regenerate the RUNTIME stylesheet (see runtime_css_path) over baked-in + user plugins.
 
     Returns ``True`` on a successful rebuild, ``False`` on any skip/failure.
     Never raises — callers treat CSS freshness as best-effort. Concurrent
@@ -166,8 +247,13 @@ def rebuild(reason: str = "") -> bool:
         log.info("tailwind rebuild skipped — engine/inputs unavailable%s", tag)
         return False
 
-    out = APP_DIR / "static" / "tailwind.min.css"
+    out = runtime_css_path()
     src = APP_DIR / "static" / "_tailwind.src.css"
+    try:
+        out.parent.mkdir(parents=True, exist_ok=True)
+    except OSError:
+        log.warning("tailwind rebuild skipped — cannot create %s%s", out.parent, tag)
+        return False
 
     # If a rebuild is already running, flag a rerun and return instead of
     # queueing a redundant build behind it.

server.py

@@ -18,7 +18,7 @@ configure_logging()
 
 log = logging.getLogger("feedBack.server")
 
-from fastapi import FastAPI, File
+from fastapi import FastAPI, File, Response
 from fastapi.staticfiles import StaticFiles
 from fastapi.responses import FileResponse, JSONResponse, StreamingResponse
 
@@ -47,6 +47,7 @@ import appstate
 import builtin_content
 import demo_mode
 import scan
+import tailwind_rebuild
 # 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
 from routers import tunings as tunings_router
@@ -1639,6 +1640,69 @@ class _RevalidatedStaticFiles(StaticFiles):
         return response
 
 
+# ── The Tailwind stylesheet: runtime-augmented if there is one, else the committed build ──
+#
+# Two different things used to share one path (#911):
+#
+#   static/tailwind.min.css   a BUILD ARTEFACT. Committed, image-baked, generated from the
+#                             in-tree plugins only. CI (`tailwind-fresh`) verifies it.
+#   the RUNTIME sheet         PER-INSTALL STATE. Additionally scans whatever the user installed
+#                             into FEEDBACK_PLUGINS_DIR, so it differs machine to machine.
+#
+# Writing the second over the first meant that merely RUNNING THE DEV SERVER from a git
+# checkout silently modified a tracked file. `git add -A` then swept a 100KB reshuffle of
+# minified CSS into the commit and ci/tailwind-fresh went red with a diff that explained
+# nothing — on a PR whose real change touched no Tailwind classes at all. It also meant writing
+# app state into the app directory, which is read-only in some deploys.
+#
+# The runtime sheet lives in CONFIG_DIR now. This route serves it when it exists and otherwise
+# falls through to the committed one. It MUST be registered BEFORE the /static mount: routes
+# are matched in order, and the mount would otherwise swallow the path.
+def _runtime_css_if_usable() -> Path | None:
+    """The runtime sheet, but ONLY when it is actually the right answer.
+
+    Codex [P2] on the first cut of #911, and it was right: a persisted sheet can outlive the
+    reason it existed and then MASK newer core CSS indefinitely. Two ways:
+
+      * THE USER REMOVED THEIR PLUGINS. Startup only rebuilds when there are user plugins, so
+        nothing would ever overwrite the old sheet — and it still carries classes for plugins
+        that are gone, while missing nothing. With no user plugins the COMMITTED sheet is by
+        definition complete and authoritative.
+      * THE APP WAS UPGRADED. A new release ships new core classes in static/tailwind.min.css.
+        The runtime sheet on disk predates them. Serving it hides the new CSS until something
+        happens to trigger a rebuild — which, if the toolchain is absent (no node), is never.
+
+    Freshness is decided by CONTENT, not mtime. Codex [P2] again, and again correct: archives
+    and container images routinely PRESERVE SOURCE MTIMES, so a just-shipped stylesheet can
+    carry an older timestamp than a runtime sheet built days ago — and an mtime check would call
+    the stale one fresh. tailwind_rebuild stamps each runtime build with the hash of the
+    committed sheet it was made from; a core upgrade changes that file, hence that hash.
+
+    Falling back to the committed sheet is always safe: at worst it lacks a just-installed
+    plugin's classes for the seconds until the async rebuild lands.
+    """
+    runtime = tailwind_rebuild.runtime_css_path()
+    if not runtime.is_file():
+        return None
+    if tailwind_rebuild.user_plugin_count() == 0:
+        return None
+    if not tailwind_rebuild.runtime_css_is_current():
+        return None   # built against a different core — see runtime_css_is_current()
+    return runtime
+
+
+@app.get("/static/tailwind.min.css")
+def tailwind_css(request: Request):
+    target = _runtime_css_if_usable() or (STATIC_DIR / "tailwind.min.css")
+    if not target.is_file():
+        return Response("", status_code=404)
+    # Same cache contract the /static mount applies (_RevalidatedStaticFiles): no-cache, so the
+    # browser always revalidates and picks up a rebuild without a hard refresh.
+    resp = FileResponse(str(target), media_type="text/css")
+    resp.headers["Cache-Control"] = "no-cache"
+    return resp
+
+
 app.mount("/static", _RevalidatedStaticFiles(directory=str(STATIC_DIR)), name="static")
 
 

tests/test_tailwind_runtime_path.py

@@ -0,0 +1,149 @@
+"""The runtime stylesheet must NOT be written over the committed one. (#911)
+
+Two different things used to share `static/tailwind.min.css`:
+
+    the committed file   a BUILD ARTEFACT — image-baked, generated from the in-tree plugins
+                         only, and verified by CI's `tailwind-fresh` check.
+    the runtime sheet    PER-INSTALL STATE — additionally scans whatever the user installed
+                         into FEEDBACK_PLUGINS_DIR, so it differs machine to machine.
+
+Writing the second over the first meant that merely RUNNING THE DEV SERVER from a git checkout
+silently modified a tracked file. `git add -A` then swept a 100KB reshuffle of minified CSS
+into the commit and `ci/tailwind-fresh` went red with a diff that explained nothing — on a PR
+whose real change touched no Tailwind classes at all. It also meant writing app state into the
+app directory, which is read-only in some deploys.
+
+These tests pin the separation. The first is the one that matters: it is the exact failure that
+shipped.
+"""
+
+import importlib
+import sys
+from pathlib import Path
+
+import pytest
+
+
+@pytest.fixture()
+def tw(monkeypatch, tmp_path):
+    monkeypatch.setenv("CONFIG_DIR", str(tmp_path))
+    sys.modules.pop("tailwind_rebuild", None)
+    return importlib.import_module("tailwind_rebuild")
+
+
+def test_the_runtime_sheet_is_not_the_committed_one(tw, tmp_path):
+    """THE REGRESSION. The runtime build must never target the tracked file."""
+    runtime = tw.runtime_css_path()
+    committed = tw.APP_DIR / "static" / "tailwind.min.css"
+
+    assert runtime != committed, (
+        "the runtime stylesheet is being written over the COMMITTED one — running the dev "
+        "server in a checkout will silently dirty a tracked file and red-light ci/tailwind-fresh"
+    )
+    assert committed not in runtime.parents
+    assert runtime.parent == tmp_path, "the runtime sheet belongs in CONFIG_DIR"
+
+
+def test_runtime_path_follows_CONFIG_DIR(monkeypatch, tmp_path):
+    """It is per-install state, so it lives wherever this install keeps its state."""
+    other = tmp_path / "elsewhere"
+    monkeypatch.setenv("CONFIG_DIR", str(other))
+    sys.modules.pop("tailwind_rebuild", None)
+    tw = importlib.import_module("tailwind_rebuild")
+    assert tw.runtime_css_path() == other / "tailwind.min.css"
+
+
+def test_runtime_path_falls_back_when_CONFIG_DIR_is_unset(monkeypatch):
+    monkeypatch.delenv("CONFIG_DIR", raising=False)
+    monkeypatch.delenv("SLOPSMITH_CONFIG_DIR", raising=False)
+    sys.modules.pop("tailwind_rebuild", None)
+    tw = importlib.import_module("tailwind_rebuild")
+    p = tw.runtime_css_path()
+    assert p.name == "tailwind.min.css"
+    assert "static" not in p.parts, "must not fall back into the app's static/ dir"
+
+
+def test_rebuild_never_touches_the_committed_file(tw, tmp_path, monkeypatch):
+    """Belt and braces: drive rebuild() and assert the tracked file is byte-identical.
+
+    This is the assertion that would actually have caught #911 in CI.
+    """
+    committed = tw.APP_DIR / "static" / "tailwind.min.css"
+    before = committed.read_bytes() if committed.is_file() else None
+
+    tw.rebuild("test")   # best-effort; may skip if node/tailwind is absent — that is fine
+
+    after = committed.read_bytes() if committed.is_file() else None
+    assert after == before, (
+        "rebuild() modified the COMMITTED static/tailwind.min.css — this is #911: it dirties a "
+        "tracked file in any git checkout and red-lights ci/tailwind-fresh"
+    )
+
+
+# ── Codex [P2]: a persisted sheet must not outlive its reason ──────────────────
+#
+# A runtime sheet can survive the thing that justified it and then MASK newer core CSS —
+# possibly forever, because startup only rebuilds when user plugins exist and skips entirely
+# when the toolchain is absent.
+
+def _server(monkeypatch, tmp_path):
+    monkeypatch.setenv("CONFIG_DIR", str(tmp_path))
+    for m in ("server", "tailwind_rebuild"):
+        sys.modules.pop(m, None)
+    return importlib.import_module("server")
+
+
+def test_runtime_sheet_is_ignored_when_the_user_has_no_plugins(monkeypatch, tmp_path):
+    """Remove your plugins and the committed sheet is authoritative again — it is complete by
+    definition. A leftover runtime sheet would carry classes for plugins that are gone."""
+    srv = _server(monkeypatch, tmp_path)
+    (tmp_path / "tailwind.min.css").write_text("/* stale runtime sheet */")
+    monkeypatch.setattr(srv.tailwind_rebuild, "user_plugin_count", lambda: 0)
+    assert srv._runtime_css_if_usable() is None
+
+
+def _stamp(srv, tmp_path, *, matching: bool):
+    """Write the sidecar that records WHICH CORE the runtime sheet was built against."""
+    import json
+    h = srv.tailwind_rebuild._committed_css_fingerprint() if matching else "0" * 64
+    # ask for the real path rather than hardcoding it — with_suffix('.meta.json') on
+    # tailwind.min.css yields tailwind.min.meta.json, not tailwind.meta.json
+    srv.tailwind_rebuild.runtime_meta_path().write_text(json.dumps({"committed_sha256": h}))
+
+
+def test_runtime_sheet_is_ignored_when_it_was_built_against_a_DIFFERENT_core(monkeypatch, tmp_path):
+    """An upgrade ships new core classes. A runtime sheet built against the OLD core would hide
+    them — and with no Tailwind toolchain present, nothing would ever rebuild it.
+
+    Freshness is decided by CONTENT, not mtime. Codex [P2] on the mtime version, and correct:
+    archives and container images routinely PRESERVE SOURCE MTIMES, so a just-shipped stylesheet
+    can carry an OLDER timestamp than a runtime sheet built days ago — and an mtime check would
+    then call the stale one fresh, masking the new CSS forever.
+    """
+    srv = _server(monkeypatch, tmp_path)
+    (tmp_path / "tailwind.min.css").write_text("/* built against the old core */")
+    monkeypatch.setattr(srv.tailwind_rebuild, "user_plugin_count", lambda: 1)
+    _stamp(srv, tmp_path, matching=False)
+
+    assert srv._runtime_css_if_usable() is None, (
+        "a runtime sheet built against a different core must not mask the shipped CSS"
+    )
+
+
+def test_runtime_sheet_is_ignored_when_it_has_no_stamp_at_all(monkeypatch, tmp_path):
+    """A sheet from before this mechanism existed. Unknown provenance -> do not trust it."""
+    srv = _server(monkeypatch, tmp_path)
+    (tmp_path / "tailwind.min.css").write_text("/* no sidecar */")
+    monkeypatch.setattr(srv.tailwind_rebuild, "user_plugin_count", lambda: 1)
+    assert srv._runtime_css_if_usable() is None
+
+
+def test_runtime_sheet_IS_used_when_it_matches_this_core_and_plugins_exist(monkeypatch, tmp_path):
+    """The case it exists for."""
+    srv = _server(monkeypatch, tmp_path)
+    runtime = tmp_path / "tailwind.min.css"
+    runtime.write_text("/* fresh, with plugin classes */")
+    monkeypatch.setattr(srv.tailwind_rebuild, "user_plugin_count", lambda: 2)
+    _stamp(srv, tmp_path, matching=True)
+
+    assert srv._runtime_css_if_usable() == runtime