perf(ui): cache busting — HTML no-cache + versioned asset URLs (?v=) with immutable 1y asset caching

Phase 33 (story: .agent/user_stories/cache-busting.md). - app/core/caching.py: asset_version() — git short SHA (a commit is a deploy), stable content-hash fallback for non-git checkouts, "dev" for a missing static dir; computed once per process. CachingMiddleware — the five HTML pages revalidate (no-cache) with ?v=<token> asset refs rewritten in flight; /assets/* is public, max-age=31536000, immutable; everything else (all /api/*, the SSE chat stream in particular) passes through byte-identical. - tests/e2e/test_cache_busting.py: fresh-Chromium wire assertions — document no-cache, versioned CSS/JS request URLs sharing one token, immutable asset headers, /api/health baseline headers, SSE chat to done (mock LLM). - README 'Caching / deploys' section + story file. Also fixed two prod-image defects surfaced by this phase's podman smoke (the full app would not boot): - Containerfile: ship the scripts/ package — app/api/sync.py (phase 32) imports scripts.git_sync / scripts.import_docs at module level, so the container crashed on boot (ModuleNotFoundError: No module named 'scripts'). - compose.yaml: pass BOR_ADMIN_PASSWORD / BOR_SESSION_SECRET through to the app service (:- defaults keep 'podman compose up -d db' working; the app's own fail-loud gate still names missing admin auth). Smoke: podman compose --profile prod up -d on a fresh image + a fresh Chromium profile — /, /sources.html and /login.html all served Cache-Control: no-cache; all 8 asset requests versioned with one shared token (content-hash fallback inside the image — no .git there); /assets/* immutable for a year.
2026-08-25 22:42:10 -04:00
parent 52136fe307
commit 8fabb7efda
12 changed files with 988 additions and 0 deletions
@@ -0,0 +1,362 @@
+"""Unit tests: the frontend asset version token (phase 33, app/core/caching.py).
+
+Covers the four token paths:
+* git repo  -> ``git rev-parse --short HEAD`` (stable, cached).
+* fallback  -> 12-hex content hash; stable for an unchanged tree, flips on
+  a size or mtime change once the per-process cache is cleared.
+* failure   -> a ``.git`` present but git broken (missing / timeout /
+  non-zero exit) falls back to the content hash without raising.
+* empty     -> a missing or empty static dir yields ``"dev"``.
+
+Plus the asset-reference rewrite (``rewrite_asset_refs``) and the
+CachingMiddleware fallback branches (task 02).
+"""
+from __future__ import annotations
+
+import asyncio
+import os
+import re
+import subprocess
+from collections.abc import AsyncIterator, Iterator
+from pathlib import Path
+
+import pytest
+from fastapi import FastAPI
+from fastapi.responses import HTMLResponse, JSONResponse
+from fastapi.testclient import TestClient
+
+import app.core.caching as caching
+from app.config import Settings
+from app.core.caching import asset_version, rewrite_asset_refs
+
+TOKEN = "abc123"
+
+
+@pytest.fixture(autouse=True)
+def _clear_token_cache() -> Iterator[None]:
+    # The token is lru_cached per process — every case starts from a clean
+    # slate and never leaks its entry into the next case.
+    asset_version.cache_clear()
+    yield
+    asset_version.cache_clear()
+
+
+def _git(repo: Path, *args: str) -> None:
+    subprocess.run(["git", "-C", str(repo), *args], check=True, capture_output=True)
+
+
+def _make_git_repo(base: Path) -> tuple[Path, str]:
+    """A repo with a committed ``frontend/`` file; returns (frontend, short SHA)."""
+    repo = base / "proj"
+    frontend = repo / "frontend"
+    frontend.mkdir(parents=True)
+    (frontend / "styles.css").write_text("body { margin: 0 }\n")
+    _git(repo, "init", "-q")
+    _git(repo, "add", "frontend")
+    _git(
+        repo,
+        "-c",
+        "user.name=test",
+        "-c",
+        "user.email=test@example.com",
+        "commit",
+        "-q",
+        "-m",
+        "init",
+    )
+    proc = subprocess.run(
+        ["git", "-C", str(repo), "rev-parse", "--short", "HEAD"],
+        check=True,
+        capture_output=True,
+        text=True,
+    )
+    return frontend, proc.stdout.strip()
+
+
+def _make_plain_frontend(tmp_path: Path) -> Path:
+    frontend = tmp_path / "frontend"
+    frontend.mkdir()
+    (frontend / "app.js").write_text("console.log('hi')\n")
+    (frontend / "styles.css").write_text("body { margin: 0 }\n")
+    return frontend
+
+
+def test_git_repo_token_matches_short_sha_and_is_cached(tmp_path) -> None:
+    frontend, short_sha = _make_git_repo(tmp_path)
+
+    assert asset_version(str(frontend)) == short_sha
+    # Second call: same value, served from the per-process cache.
+    assert asset_version(str(frontend)) == short_sha
+    assert asset_version.cache_info().hits >= 1
+
+
+def test_default_static_dir_comes_from_settings(tmp_path, monkeypatch) -> None:
+    frontend, short_sha = _make_git_repo(tmp_path)
+    monkeypatch.setattr(
+        caching, "get_settings", lambda: Settings(static_dir=str(frontend))
+    )
+    assert asset_version() == short_sha  # no argument -> settings default
+
+
+def test_fallback_token_is_12_hex_and_stable_for_unchanged_tree(tmp_path) -> None:
+    frontend = _make_plain_frontend(tmp_path)
+
+    token = asset_version(str(frontend))
+    assert re.fullmatch(r"[0-9a-f]{12}", token)
+    # Unchanged tree: same token even after the cache entry is dropped
+    # (i.e. the hash itself is deterministic, not just the cache).
+    asset_version.cache_clear()
+    assert asset_version(str(frontend)) == token
+
+
+def test_fallback_token_changes_on_size_change(tmp_path) -> None:
+    frontend = _make_plain_frontend(tmp_path)
+    before = asset_version(str(frontend))
+
+    (frontend / "app.js").write_text("console.log('a longer payload')\n")
+    asset_version.cache_clear()
+    assert asset_version(str(frontend)) != before
+
+
+def test_fallback_token_changes_on_mtime_only(tmp_path) -> None:
+    frontend = _make_plain_frontend(tmp_path)
+    before = asset_version(str(frontend))
+
+    path = frontend / "app.js"
+    now = os.stat(path).st_mtime_ns
+    os.utime(path, ns=(now + 5_000, now + 5_000))  # mtime change, same size
+    asset_version.cache_clear()
+    assert asset_version(str(frontend)) != before
+
+
+@pytest.mark.parametrize(
+    ("error",),
+    [
+        (FileNotFoundError("git not found"),),
+        (subprocess.TimeoutExpired(cmd="git", timeout=5),),
+        (subprocess.CalledProcessError(returncode=128, cmd="git"),),
+    ],
+    ids=["git-missing", "git-timeout", "git-nonzero-exit"],
+)
+def test_git_failure_falls_back_to_content_hash(tmp_path, monkeypatch, error: Exception) -> None:
+    # A ``.git`` exists, but git itself is broken -> content hash, no raise.
+    repo = tmp_path / "proj"
+    frontend = repo / "frontend"
+    frontend.mkdir(parents=True)
+    (repo / ".git").mkdir()
+    (frontend / "styles.css").write_text("body { margin: 0 }\n")
+
+    def _boom(*args: object, **kwargs: object) -> subprocess.CompletedProcess[str]:
+        raise error
+
+    monkeypatch.setattr(caching.subprocess, "run", _boom)
+    token = asset_version(str(frontend))
+    assert re.fullmatch(r"[0-9a-f]{12}", token)
+
+
+def test_git_success_with_empty_output_falls_back(tmp_path, monkeypatch) -> None:
+    # git exits 0 but prints nothing (defensive guard) -> content hash.
+    repo = tmp_path / "proj"
+    frontend = repo / "frontend"
+    frontend.mkdir(parents=True)
+    (repo / ".git").mkdir()
+    (frontend / "styles.css").write_text("body { margin: 0 }\n")
+
+    monkeypatch.setattr(
+        caching.subprocess,
+        "run",
+        lambda *a, **k: subprocess.CompletedProcess(
+            args=["git"], returncode=0, stdout="  \n", stderr=""
+        ),
+    )
+    token = asset_version(str(frontend))
+    assert re.fullmatch(r"[0-9a-f]{12}", token)
+
+
+def test_missing_static_dir_is_dev(tmp_path) -> None:
+    assert asset_version(str(tmp_path / "does-not-exist")) == "dev"
+
+
+def test_empty_static_dir_is_dev(tmp_path) -> None:
+    empty = tmp_path / "frontend"
+    empty.mkdir()
+    assert asset_version(str(empty)) == "dev"
+
+
+# ---------------------------------------------------------------------------
+# rewrite_asset_refs (task 02)
+# ---------------------------------------------------------------------------
+
+
+def test_rewrite_versions_href_with_leading_slash() -> None:
+    html = '<link rel="stylesheet" href="/assets/styles.css">'
+    assert rewrite_asset_refs(html, TOKEN) == (
+        '<link rel="stylesheet" href="/assets/styles.css?v=abc123">'
+    )
+
+
+def test_rewrite_versions_src_without_leading_slash() -> None:
+    html = '<script src="assets/markdown.js"></script>'
+    assert rewrite_asset_refs(html, TOKEN) == (
+        '<script src="assets/markdown.js?v=abc123"></script>'
+    )
+
+
+def test_rewrite_versions_module_script_src() -> None:
+    html = '<script type="module" src="/assets/app.js"></script>'
+    assert rewrite_asset_refs(html, TOKEN) == (
+        '<script type="module" src="/assets/app.js?v=abc123"></script>'
+    )
+
+
+def test_rewrite_versions_every_ref_in_one_pass() -> None:
+    html = (
+        '<link rel="stylesheet" href="/assets/styles.css">'
+        "<script src=\"assets/markdown.js\"></script>"
+        '<script type="module" src="/assets/app.js"></script>'
+    )
+    assert rewrite_asset_refs(html, TOKEN) == (
+        '<link rel="stylesheet" href="/assets/styles.css?v=abc123">'
+        '<script src="assets/markdown.js?v=abc123"></script>'
+        '<script type="module" src="/assets/app.js?v=abc123"></script>'
+    )
+
+
+def test_rewrite_is_idempotent_for_already_versioned_refs() -> None:
+    html = '<link rel="stylesheet" href="/assets/styles.css?v=abc123">'
+    assert rewrite_asset_refs(html, "deadbeef") == html
+
+
+def test_rewrite_leaves_query_and_fragment_refs_alone() -> None:
+    html = '<script src="/assets/app.js?x=1"></script><img src="/assets/logo.svg#frag">'
+    assert rewrite_asset_refs(html, TOKEN) == html
+
+
+def test_rewrite_leaves_non_asset_refs_untouched() -> None:
+    html = (
+        '<a href="/sources.html">sources</a>'
+        '<img src="data:image/png;base64,AAA">'
+        '<a href="/login.html?next=/sources.html">login</a>'
+    )
+    assert rewrite_asset_refs(html, TOKEN) == html
+
+
+def test_rewrite_returns_html_unchanged_when_no_asset_refs() -> None:
+    html = "<p>no assets here</p>"
+    assert rewrite_asset_refs(html, TOKEN) == html
+
+
+# ---------------------------------------------------------------------------
+# CachingMiddleware fallback branches (task 02)
+# ---------------------------------------------------------------------------
+
+
+def _page_app() -> FastAPI:
+    """A bare app with a ``text/html`` route at ``/`` + the middleware."""
+    app = FastAPI()
+
+    @app.get("/", response_class=HTMLResponse)
+    def index() -> str:
+        return '<html><head><link rel="stylesheet" href="/assets/styles.css"></head></html>'
+
+    @app.get("/api/health")
+    def health() -> JSONResponse:
+        return JSONResponse({"status": "ok"})
+
+    caching.configure_caching(app)
+    return app
+
+
+def test_middleware_success_path_rewrites_page_and_sets_no_cache() -> None:
+    client = TestClient(_page_app())
+    r = client.get("/")
+    assert r.status_code == 200
+    assert r.headers["cache-control"] == "no-cache"
+    token = caching.asset_version()
+    assert f'href="/assets/styles.css?v={token}"' in r.text
+    assert 'href="/assets/styles.css">' not in r.text
+
+
+def test_middleware_buffer_failure_keeps_body_and_sets_no_cache(monkeypatch) -> None:
+    """If buffering/token resolution fails, the ORIGINAL streaming body is
+    served unmodified — with ``no-cache`` — never an empty page."""
+
+    def _boom() -> str:
+        raise RuntimeError("token blew up")
+
+    monkeypatch.setattr(caching, "asset_version", _boom)
+    client = TestClient(_page_app())
+    r = client.get("/")
+    assert r.status_code == 200
+    assert r.headers["cache-control"] == "no-cache"
+    assert r.text == (
+        "<html><head><link rel=\"stylesheet\" href=\"/assets/styles.css\"></head></html>"
+    )
+
+
+def test_middleware_non_html_body_on_page_path_gets_no_cache_only() -> None:
+    """A page path whose response is not ``text/html`` (e.g. the 404 JSON
+    when the static dir is missing) is revalidated but never rewritten."""
+    app = FastAPI()
+
+    @app.get("/")
+    def not_found() -> JSONResponse:
+        return JSONResponse({"detail": "Not Found"}, status_code=404)
+
+    caching.configure_caching(app)
+    client = TestClient(app)
+    r = client.get("/")
+    assert r.status_code == 404
+    assert r.headers["cache-control"] == "no-cache"
+    assert r.json() == {"detail": "Not Found"}  # body untouched
+
+
+def test_middleware_rewrite_failure_reserves_original_bytes(monkeypatch) -> None:
+    """If the body buffered fine but the rewrite itself fails, the ORIGINAL
+    bytes are re-served (never an empty page) with ``no-cache``."""
+
+    def _boom(html: str, token: str) -> str:
+        raise ValueError("rewrite blew up")
+
+    monkeypatch.setattr(caching, "rewrite_asset_refs", _boom)
+    client = TestClient(_page_app())
+    r = client.get("/")
+    assert r.status_code == 200
+    assert r.headers["cache-control"] == "no-cache"
+    assert r.text == (
+        "<html><head><link rel=\"stylesheet\" href=\"/assets/styles.css\"></head></html>"
+    )
+
+
+def test_middleware_leaves_api_responses_byte_identical() -> None:
+    """``/api/*`` gets no injected headers at all (no cache-control)."""
+    client = TestClient(_page_app())
+    r = client.get("/api/health")
+    assert r.status_code == 200
+    assert "cache-control" not in r.headers
+    assert r.json() == {"status": "ok"}
+
+
+def test_read_body_buffers_plain_response_bytes() -> None:
+    from starlette.responses import Response
+
+    resp = Response(content=b"<html>hi</html>", media_type="text/html")
+    assert asyncio.run(caching._read_body(resp)) == b"<html>hi</html>"
+
+
+def test_read_body_copies_plain_response_memoryview() -> None:
+    from starlette.responses import Response
+
+    resp = Response(content=memoryview(b"<html>hi</html>"), media_type="text/html")
+    assert asyncio.run(caching._read_body(resp)) == b"<html>hi</html>"
+
+
+def test_read_body_drains_streaming_response() -> None:
+    from starlette.responses import StreamingResponse
+
+    async def gen() -> AsyncIterator[bytes]:
+        yield b"<a>"
+        yield b"</a>"
+
+    resp = StreamingResponse(content=gen(), media_type="text/html")
+    assert asyncio.run(caching._read_body(resp)) == b"<a></a>"