From 8fabb7efda94d700f219f455740e8cf6cc3a4045 Mon Sep 17 00:00:00 2001 From: ducoterra Date: Tue, 25 Aug 2026 22:42:10 -0400 Subject: [PATCH] =?UTF-8?q?perf(ui):=20cache=20busting=20=E2=80=94=20HTML?= =?UTF-8?q?=20no-cache=20+=20versioned=20asset=20URLs=20(=3Fv=3D)=20with?= =?UTF-8?q?=20immutable=201y=20asset=20caching?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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= 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. --- .../01_asset_version_token.md | 0 .../33_cache_busting/02_caching_middleware.md | 0 .agent/user_stories/cache-busting.md | 91 +++++ Containerfile | 5 + README.md | 38 ++ app/core/caching.py | 262 +++++++++++++ app/main.py | 8 + compose.yaml | 7 + tests/e2e/test_cache_busting.py | 138 +++++++ tests/integration/test_api.py | 66 ++++ tests/unit/test_caching.py | 362 ++++++++++++++++++ tests/unit/test_main.py | 11 + 12 files changed, 988 insertions(+) rename .agent/phases/{todo => complete}/33_cache_busting/01_asset_version_token.md (100%) rename .agent/phases/{todo => complete}/33_cache_busting/02_caching_middleware.md (100%) create mode 100644 .agent/user_stories/cache-busting.md create mode 100644 app/core/caching.py create mode 100644 tests/e2e/test_cache_busting.py create mode 100644 tests/unit/test_caching.py diff --git a/.agent/phases/todo/33_cache_busting/01_asset_version_token.md b/.agent/phases/complete/33_cache_busting/01_asset_version_token.md similarity index 100% rename from .agent/phases/todo/33_cache_busting/01_asset_version_token.md rename to .agent/phases/complete/33_cache_busting/01_asset_version_token.md diff --git a/.agent/phases/todo/33_cache_busting/02_caching_middleware.md b/.agent/phases/complete/33_cache_busting/02_caching_middleware.md similarity index 100% rename from .agent/phases/todo/33_cache_busting/02_caching_middleware.md rename to .agent/phases/complete/33_cache_busting/02_caching_middleware.md diff --git a/.agent/user_stories/cache-busting.md b/.agent/user_stories/cache-busting.md new file mode 100644 index 0000000..ccbb0f6 --- /dev/null +++ b/.agent/user_stories/cache-busting.md @@ -0,0 +1,91 @@ +# Story: Cache Busting (Un-stick the Pages) + +**Phase:** `33_cache_busting/` · **E2E:** `tests/e2e/test_cache_busting.py` + +## Narrative + +As **anyone using Brain of Reese**, I want a new deploy to be **visible +without a hard refresh**. Today the five HTML pages reference their CSS/JS +with no version at all, so the browser keeps serving stale assets long +after the app has moved on — the "the pages are too sticky" report. I want +a deploy (a commit) to change what the browser fetches, automatically, +with no new services and no CDN. + +- **Given** the app has been redeployed (a new commit) +- **When** I open — or revisit — any page +- **Then** the page itself is always revalidated (never served from cache + unchecked), its assets are fetched from **versioned URLs** + (`?v=` — the git short SHA of the deploy, so each commit changes + them), and the API — the SSE chat stream in particular — is left + byte-for-byte alone. + +## Acceptance criteria +1. **HTML always revalidates.** All five pages (`/`, `/index.html`, + `/sources.html`, `/document.html`, `/login.html`, `/tuning.html`) ship + `Cache-Control: no-cache` (the page is the cheap redirector to the + long-cached assets). +2. **Assets are versioned.** Every local `href`/`src` asset reference on + those pages carries `?v=` — rewritten in flight by one + middleware (a small regex over `href`/`src` `assets/…` refs, leading + slash optional, idempotent: an existing query string or fragment is + never double-tagged, and non-asset references pass through). The token + is non-empty and **stable across requests** within a process (one token + per process, computed once). +3. **Immutable assets.** `/assets/*` responses ship + `Cache-Control: public, max-age=31536000, immutable` — safe precisely + because the URL carries the token (a deploy changes the token, hence + the URL). The unversioned path keeps resolving (the static mount + ignores the query string), so old tabs and direct links still work. +4. **The token is the deploy.** `app/core/caching.py::asset_version()`: + a git checkout (the project root next to `frontend/` has a `.git` — + the homelab reality) → `git rev-parse --short HEAD` with a 5 s timeout + (a deploy must not hang a boot); no `.git`, git missing, non-zero + exit, timeout, or empty output → the first **12 hex chars** of SHA-256 + over the sorted `relpath:mtime_ns:size` of every regular file under + `frontend/`; a missing/empty static dir → `"dev"`. Cached with + `functools.cache` (== `lru_cache(maxsize=None)`) — zero per-request + git/file cost. A new commit or a frontend content change flips the + token on the next process start. +5. **The API is untouched.** The middleware touches exactly two response + shapes (the page paths: body rewrite + `no-cache`; `/assets/*`: + header only). Everything else — all of `/api/*`, including the SSE + chat stream — passes through with no header changes and no body read + (SSE keeps the `no-cache` its endpoint sets itself). The no-CDN + integration test stays green (the rewritten references are + same-origin). +6. **Quality gates.** Unit (`tests/unit/test_caching.py`): the token's + git path, fallback path (stable across an unchanged tree, flips on a + touch/modify + `cache_clear()`), failure path (git raising → content + hash, no exception), empty-dir `"dev"`; the pure `rewrite_asset_refs` + (versioned `href`/`src`, no-leading-slash refs, module scripts, + idempotency, `#fragment`/existing-query left alone, non-asset refs + untouched). Integration (`tests/integration/test_api.py`): every page + `no-cache` + versioned references, asset headers, no `cache-control` + injected on `/api/health`, SSE chat tests unchanged and green. + Coverage: `app/core/caching.py` 100 %, `app/` > 90 % (TOTAL ≥ + pre-change). +7. **E2E (this story's gate).** `tests/e2e/test_cache_busting.py`, run in + isolation: a real (fresh-profile) Chromium asserts the **wire truth** + — the document responses are `no-cache`, the CSS/JS request URLs the + browser actually makes carry one shared token (matching this + checkout's `asset_version()`), the asset responses are immutable for a + year, and `/api/*` — including a live SSE chat turn that streams + deltas and completes with `done` — is unaffected (mock LLM, no live + aipi; `podman compose up -d db` for the chat check). + +## Playwright Mapping Rule +`tests/e2e/test_cache_busting.py` — run in isolation (Chromium + +`podman compose up -d db`; mock LLM via the shared `conftest.py` session +app; no per-module overrides needed): + +1. `test_html_pages_are_no_cache_and_versioned` → AC 1 + 2 + 3 (the `/` + document response is `no-cache`; the `styles.css` request URL carries + `?v=` and its response is `immutable` + `max-age=31536000`; + the `app.js` request URL carries the **same** token; the served HTML + carries no unversioned `"/assets/styles.css"` reference). +2. `test_other_pages_share_the_token` → AC 1 + 2 (the `/sources.html` and + `/login.html` document responses are `no-cache`; both pages' + stylesheet requests carry the same token). +3. `test_api_responses_unaffected` → AC 5 (`/api/health` has no + `cache-control` injected — the endpoint's baseline headers only; the + SSE chat POST still streams deltas and completes with `done`). diff --git a/Containerfile b/Containerfile index dff772b..a672e7d 100644 --- a/Containerfile +++ b/Containerfile @@ -43,6 +43,11 @@ RUN useradd --create-home --uid 10001 reese WORKDIR /app COPY --from=python /app/.venv /app/.venv COPY --from=python /app/app /app/app +# app/api/sync.py (phase 32) imports scripts.git_sync / scripts.import_docs +# at module level — the scripts package must ship in the image or the +# container crashes on boot (phase 33: ModuleNotFoundError: No module +# named 'scripts'). +COPY scripts ./scripts COPY --from=frontend /out /app/static COPY alembic ./alembic COPY alembic.ini ./alembic.ini diff --git a/README.md b/README.md index d7198de..fed4447 100644 --- a/README.md +++ b/README.md @@ -325,6 +325,44 @@ git-source refresh in one click, in-process: pull, hash skip, and the overview is left alone (its regeneration is change-gated). +## Caching / deploys + +A deploy is a commit — and the browser must see it **without a hard +refresh** (the "the pages are too sticky" problem, phase 33). One +Starlette middleware (`app/core/caching.py`) applies the rule at the +transport layer: + +- **HTML pages always revalidate.** Every page (`/`, `/sources.html`, + `/document.html`, `/login.html`, `/tuning.html`) ships + `Cache-Control: no-cache`, so each visit re-checks the page with the + server — a page never lingers in the browser's cache unchecked. +- **Assets are versioned and cached for a year.** The pages reference + their CSS/JS with a token (`/assets/styles.css?v=`), and every + `/assets/*` response ships + `Cache-Control: public, max-age=31536000, immutable`. The token is what + identifies the content, so long-term caching is safe: a new token means + a new URL, which the browser fetches fresh. +- **The token is the deploy.** In a git checkout (the normal case) it is + the short SHA of `HEAD` (`git rev-parse --short HEAD`), computed once + per process start — so **every commit/deploy flips the token** and the + versioned asset URLs change with it. A checkout without `.git` (or a git + failure) falls back to a stable content hash of the `frontend/` tree + (sorted path + mtime + size), so dev checkouts still bust; a missing + static dir gets the placeholder token `dev`. +- **The API is untouched.** Nothing under `/api/*` — the SSE chat stream + in particular — gains or loses a header or has its body read; the SSE + endpoint's own `Cache-Control: no-cache` is set by the endpoint itself. + +No CDN, no new services, no build-step change: the middleware rewrites +the asset references of the five known pages in flight. The unversioned +asset paths keep working too (the static mount ignores the query string), +so old tabs and direct links to `/assets/…` still resolve. + +> **Deploy note:** the very first deploy onto this scheme needs one +> normal page visit, so the browser revalidates the HTML once and starts +> requesting the versioned assets; every commit after that is picked up +> automatically. + ## Checking retrieval quality Ask the *real* pipeline (live aipi embeddings + the current KB) whether a diff --git a/app/core/caching.py b/app/core/caching.py new file mode 100644 index 0000000..879aa15 --- /dev/null +++ b/app/core/caching.py @@ -0,0 +1,262 @@ +"""Static-frontend cache busting (phase 33). + +Two layers, one module: + +* **Version token** (``asset_version()``) — the value the HTML pages + append to their asset URLs (``?v=``). +* **Response middleware** (``CachingMiddleware`` / ``configure_caching``) + — applies the caching behavior at the transport layer: the five known + HTML pages are always revalidated (``no-cache``) and their local asset + references are rewritten to carry ``?v=``; ``/assets/*`` is + served ``immutable`` for a year; everything else — all of ``/api/*``, + including the SSE chat stream — passes through byte-identical. + +The token is computed **once per process** (``functools.cache``, i.e. +``lru_cache(maxsize=None)``) — zero per-request git/file cost. It changes +when a new commit lands (git path) or the frontend tree's mtimes/sizes +change (fallback path); either way the change takes effect on the next +process start. Tests clear it with ``asset_version.cache_clear()``. + +Version-token sources: + +* **Git checkouts** (the homelab reality — the project root next to + ``frontend/`` has a ``.git``): the token is ``git rev-parse --short HEAD``. + A commit is a deploy, so the token flips on every deploy. +* **Plain directories** (dev checkouts without a ``.git``, or a git failure): + a stable content hash — first 12 hex chars of SHA-256 over the sorted + ``relpath:mtime_ns:size`` list of every regular file under the static dir. + Touching or editing a file (plus a process restart) busts the cache. +* **Missing/empty directory**: ``"dev"``. +""" +from __future__ import annotations + +import functools +import hashlib +import logging +import re +import subprocess +from pathlib import Path + +from fastapi import FastAPI +from starlette.middleware.base import BaseHTTPMiddleware, RequestResponseEndpoint +from starlette.requests import Request +from starlette.responses import Response + +from app.config import get_settings + +logger = logging.getLogger("app") + +#: Token for a missing/empty static dir (e.g. a build that ships the API +#: only — there is nothing on disk to version). +DEV_TOKEN = "dev" + +#: Hard cap for the ``git rev-parse`` call — a deploy must not hang a boot. +_GIT_TIMEOUT_S = 5 + + +def _static_files(static_dir: Path) -> list[str]: + """Sorted ``relpath:mtime_ns:size`` entries for every regular file.""" + entries: list[str] = [] + for path in static_dir.rglob("*"): + if path.is_file(): + st = path.stat() + rel = path.relative_to(static_dir).as_posix() + entries.append(f"{rel}:{st.st_mtime_ns}:{st.st_size}") + return sorted(entries) + + +def _git_short_sha(repo_root: Path) -> str: + """``git -C rev-parse --short HEAD`` (raises on any failure).""" + proc = subprocess.run( + ["git", "-C", str(repo_root), "rev-parse", "--short", "HEAD"], + capture_output=True, + text=True, + timeout=_GIT_TIMEOUT_S, + check=True, + ) + sha = proc.stdout.strip() + if not sha: + raise ValueError("git returned an empty short SHA") + return sha + + +@functools.cache # == lru_cache(maxsize=None): one token per process +def asset_version(static_dir: str | None = None) -> str: + """Version token for the frontend's asset URLs (see module docstring). + + ``static_dir`` defaults to ``get_settings().static_dir`` (``frontend``); + the git repo root is taken to be its parent. The result is cached for + the process lifetime — clear it with ``asset_version.cache_clear()``. + """ + target = get_settings().static_dir if static_dir is None else static_dir + directory = Path(target).expanduser().resolve() + files = _static_files(directory) if directory.is_dir() else [] + if not files: + return DEV_TOKEN + repo_root = directory.parent + if (repo_root / ".git").exists(): + try: + return _git_short_sha(repo_root) + except (OSError, subprocess.SubprocessError, ValueError): + # git missing (OSError), timeout or non-zero exit + # (SubprocessError — e.g. a repo with no commits), or empty + # output → the stable content-hash fallback keeps dev checkouts + # (and broken repos) cache-busting instead of breaking a boot. + pass + digest = hashlib.sha256("\n".join(files).encode("utf-8")).hexdigest() + return digest[:12] + + +# --------------------------------------------------------------------------- +# Response-layer cache busting (task 02) +# --------------------------------------------------------------------------- + +#: The five known HTML pages — the ONLY paths whose body is rewritten. +HTML_PAGES: tuple[str, ...] = ( + "/", + "/index.html", + "/sources.html", + "/document.html", + "/login.html", + "/tuning.html", +) + +#: Prefix of the versioned static assets (header-only caching; the body is +#: never read or modified). +ASSETS_PREFIX = "/assets/" + +#: ``/assets/*`` — the URL carries ``?v=``, so long-term caching is +#: safe (a deploy changes the token, hence the URL). +ASSET_CACHE_CONTROL = "public, max-age=31536000, immutable" + +#: HTML pages are always revalidated — the page is the cheap redirector to +#: the (long-cached) versioned assets. +HTML_CACHE_CONTROL = "no-cache" + +#: Matches ``href="…assets/…"`` / ``src="…assets/…"`` references — leading +#: slash optional (the pages mix ``/assets/…`` and ``assets/…``) — that do +#: NOT already carry a query string or a fragment: ``[^"?#]+`` stops at +#: ``?``/``#``, so an already-``?v=``-tagged reference can never match +#: again (idempotent rewrite, never a double ``?v=``). +_ASSET_REF_RE = re.compile(r'((?:href|src)="(?:/)?assets/[^"?#]+)(")') + + +def rewrite_asset_refs(html: str, token: str) -> str: + """Append ``?v=`` before the closing quote of every local + ``assets/…`` ``href``/``src`` reference. + + Pure and idempotent: references that already carry a query string or a + fragment (``?v=…``, ``#frag``) do not match the pattern and are left + alone, and non-asset references (``href="/sources.html"``, + ``href="data:…"``) pass through untouched. + """ + return _ASSET_REF_RE.sub(lambda m: f"{m.group(1)}?v={token}{m.group(2)}", html) + + +async def _read_body(response: Response) -> bytes: + """Buffer the whole response body. + + starlette 1.x's ``BaseHTTPMiddleware.call_next`` returns a streaming + response (no ``body()`` helper), so drain ``body_iterator``; a plain + buffered ``Response`` simply carries its ``body``. + """ + if hasattr(response, "body"): + raw = response.body # bytes | memoryview (Response.render's output) + if isinstance(raw, memoryview): + return bytes(raw) + return raw + # starlette 1.x's BaseHTTPMiddleware.call_next wraps the app response + # in a streaming shell (``body_iterator``, no ``body``) that is NOT a + # StreamingResponse subclass — drain the iterator instead. + iterator = response.body_iterator # pyright: ignore[reportAttributeAccessIssue] + return b"".join([chunk async for chunk in iterator]) + + +def _no_cache_headers(response: Response) -> dict[str, str]: + """Copy the original headers, drop the stale ``content-length`` (the + body size changes on rewrite), and force ``Cache-Control: no-cache``.""" + headers = {k: v for k, v in response.headers.items()} + headers.pop("content-length", None) + headers["Cache-Control"] = HTML_CACHE_CONTROL + return headers + + +class CachingMiddleware(BaseHTTPMiddleware): + """Transport-layer cache busting (phase 33). + + Touches exactly two response shapes: + + * ``/assets/*`` — ``Cache-Control: public, max-age=31536000, immutable`` + (header only — the body is never read). + * the five known HTML pages — ``Cache-Control: no-cache``, and (for + ``text/html`` bodies) every local asset reference gains + ``?v=``. + + Everything else — all of ``/api/*`` (including the SSE chat stream) — + passes through byte-identical: no header changes, the body stream is + never drained. + """ + + async def dispatch( + self, request: Request, call_next: RequestResponseEndpoint + ) -> Response: + response = await call_next(request) + path = request.url.path + + if path.startswith(ASSETS_PREFIX): + response.headers["Cache-Control"] = ASSET_CACHE_CONTROL + return response + + if path not in HTML_PAGES: + # /api/* (incl. SSE), /favicon.ico, unknown paths: untouched. + return response + + content_type = response.headers.get("content-type", "") + if content_type.split(";", 1)[0].strip().lower() != "text/html": + # A page path with a non-HTML body (e.g. the 404 JSON served + # when the static dir is missing): still revalidate, body as-is. + response.headers["Cache-Control"] = HTML_CACHE_CONTROL + return response + + # ``asset_version()`` is lru_cached, so only the very first call in + # the process can fail — resolve it BEFORE draining the body stream + # below. + try: + token = asset_version() + body = await _read_body(response) + except Exception: + logger.exception( + "cache busting: could not buffer %s — serving unmodified (no-cache)", + path, + ) + response.headers["Cache-Control"] = HTML_CACHE_CONTROL + return response + + try: + new_body = rewrite_asset_refs(body.decode("utf-8"), token).encode("utf-8") + except Exception: + # The body IS buffered — re-serve the ORIGINAL bytes so a + # rewrite hiccup never loses the page. + logger.exception("cache busting: rewrite of %s failed — serving raw (no-cache)", path) + return Response( + content=body, + status_code=response.status_code, + headers=_no_cache_headers(response), + ) + + return Response( + content=new_body, + status_code=response.status_code, + headers=_no_cache_headers(response), + ) + + +def configure_caching(app: FastAPI) -> None: + """Attach :class:`CachingMiddleware` to ``app`` (phase 33). + + Call inside ``create_app()`` after the routers are mounted; the + middleware wraps the whole app, so the ``/`` static catch-all is + covered too. ``/api/*`` responses (including the SSE chat stream) are + never touched by it. + """ + app.add_middleware(CachingMiddleware) diff --git a/app/main.py b/app/main.py index 0ce7dd8..f7fef01 100644 --- a/app/main.py +++ b/app/main.py @@ -28,6 +28,7 @@ from app.api.suggestions import router as suggestions_router from app.api.sync import router as sync_router from app.config import get_settings from app.core.auth import ensure_admin_configured +from app.core.caching import configure_caching from app.core.debugging import configure_debugging from app.core.logging import configure_logging @@ -67,6 +68,13 @@ def create_app() -> FastAPI: app.include_router(steering_router, prefix="/api") app.include_router(sync_router, prefix="/api") + # Cache busting (phase 33): the five HTML pages revalidate (no-cache) + # with ?v= asset refs; /assets/* becomes immutable for a year. + # Added after the session middleware, so it wraps the whole app + # (including the static catch-all below); /api/* — the SSE chat + # stream in particular — passes through untouched. + configure_caching(app) + static_dir = Path(settings.static_dir).resolve() if static_dir.is_dir(): app.mount("/", StaticFiles(directory=static_dir, html=True), name="static") diff --git a/compose.yaml b/compose.yaml index 16b0e57..9209c34 100644 --- a/compose.yaml +++ b/compose.yaml @@ -42,6 +42,13 @@ services: BOR_DATABASE_URL: postgresql+psycopg://reese:reese@db:5432/brain_of_reese BOR_LLM_BASE_URL: https://aipi.reeseapps.com/v1 # BOR_LLM_API_KEY: provide via shell env or your own env file — never commit it + # Single-admin auth (phase 16) is a fail-loud boot gate: set both via + # your shell env or an env file for the prod profile (the app refuses + # to boot without them). `:-` defaults keep `podman compose up -d db` + # (the dev workflow) parseable without them. Values are secrets: + # never commit them. + BOR_ADMIN_PASSWORD: ${BOR_ADMIN_PASSWORD:-} + BOR_SESSION_SECRET: ${BOR_SESSION_SECRET:-} ports: - "8000:8000" diff --git a/tests/e2e/test_cache_busting.py b/tests/e2e/test_cache_busting.py new file mode 100644 index 0000000..99b8fb9 --- /dev/null +++ b/tests/e2e/test_cache_busting.py @@ -0,0 +1,138 @@ +"""Phase 33 E2E (Playwright): cache busting — what the browser actually +receives and requests. + +Story: ``.agent/user_stories/cache-busting.md`` +Run in isolation (DB must be up: ``podman compose up -d db``): + + uv run pytest tests/e2e/test_cache_busting.py -v --no-cov + +The assertions are against the wire: the HTML document responses carry +``Cache-Control: no-cache``; every asset request URL the browser actually +makes carries ``?v=`` (one token per process — the git short SHA of +this checkout, i.e. the deploy); the asset responses are immutable for a +year; and the API — the SSE chat stream in particular — is untouched. The +mock LLM keeps the SSE check deterministic (no live aipi). +""" +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +import httpx +from playwright.sync_api import Page + +REPO = Path(__file__).resolve().parents[2] +CHAT_QUESTION = "How is my Kubernetes cluster set up?" + + +def _expected_token() -> str: + """The token the app process appends to its asset URLs. + + Computed exactly the way the app does (``asset_version`` over the same + static dir): the git short SHA of this checkout in a git repo (a commit + is a deploy), so the browser's asset requests must carry it. + """ + from app.core.caching import asset_version + + return asset_version(str(REPO / "frontend")) + + +def _version_token(url: str) -> str: + """Extract the ``?v=`` token from a versioned asset URL (asserts one).""" + assert "?v=" in url, f"asset request is not versioned: {url}" + return url.rsplit("?v=", 1)[1] + + +def _stream_chat_frames(app_url: str, message: str) -> list[dict[str, Any]]: + """Minimal SSE chat request (same pattern as ``test_chat_rag.py``): + POST /api/chat and collect the ``data:`` frames until the stream ends.""" + frames: list[dict[str, Any]] = [] + with httpx.stream( + "POST", f"{app_url}/api/chat", json={"message": message}, timeout=60.0 + ) as r: + assert r.status_code == 200 + assert r.headers["content-type"].startswith("text/event-stream") + buf = "" + for part in r.iter_text(): + buf += part + while "\n\n" in buf: + frame, buf = buf.split("\n\n", 1) + if frame.strip().startswith("data:"): + frames.append( + json.loads(frame.strip().removeprefix("data:").strip()) + ) + return frames + + +def test_html_pages_are_no_cache_and_versioned(page: Page, app_url: str) -> None: + """`/`: the document revalidates (no-cache); the CSS/JS request URLs + the browser actually makes carry the process token; the asset + responses are immutable for a year; the served HTML carries no + unversioned asset references.""" + token = _expected_token() + assert token, "the version token must be non-empty" + + with ( + page.expect_response(lambda r: "/assets/styles.css" in r.url) as css_info, + page.expect_response(lambda r: "/assets/app.js" in r.url) as js_info, + ): + doc = page.goto(app_url) + + # The document: always revalidated, never served from cache unchecked. + assert doc is not None + assert doc.headers["cache-control"] == "no-cache" + + # CSS: versioned request URL + immutable-for-a-year response. + css = css_info.value + assert _version_token(css.url) == token + css_cc = css.headers["cache-control"] + assert "immutable" in css_cc + assert "max-age=31536000" in css_cc + + # JS: the SAME token (one per process — the URL identifies the + # content, which is what makes the 1-year cache safe). + assert _version_token(js_info.value.url) == _version_token(css.url) + + # The served HTML carries the versioned reference and no unversioned + # one (the "sticky" reference is gone from the page the browser sees). + html = page.content() + assert f'href="/assets/styles.css?v={token}"' in html + assert '/assets/styles.css"' not in html + + +def test_other_pages_share_the_token(page: Page, app_url: str) -> None: + """/sources.html and /login.html: each document revalidates, and both + pages' stylesheet requests carry the same process token.""" + token = _expected_token() + assert token + + def navigate(path: str) -> str: + with page.expect_response( + lambda r: "/assets/styles.css" in r.url + ) as css_info: + doc = page.goto(f"{app_url}{path}") + assert doc is not None + assert doc.headers["cache-control"] == "no-cache" + return _version_token(css_info.value.url) + + sources_token = navigate("/sources.html") + login_token = navigate("/login.html") + assert sources_token == login_token == token + + +def test_api_responses_unaffected(page: Page, app_url: str, db_ready: None) -> None: + """`/api/*` passes through untouched: no injected Cache-Control on the + health endpoint, and the SSE chat stream still streams to done.""" + r = page.request.get(f"{app_url}/api/health") + assert r.status == 200 + # Baseline (pre-middleware) behavior: FastAPI's JSON responses ship no + # Cache-Control header — the middleware must not inject one. + assert "cache-control" not in r.headers + + # The SSE contract (PLAN §4) survives the middleware: deltas, then a + # final done — the stream is neither read nor rewritten by it. + frames = _stream_chat_frames(app_url, CHAT_QUESTION) + assert frames, "the SSE stream must deliver events" + assert any(f["type"] == "delta" for f in frames), "answer must be streamed" + assert frames[-1]["type"] == "done", "the stream must complete with done" diff --git a/tests/integration/test_api.py b/tests/integration/test_api.py index b5cd865..d4f2d7d 100644 --- a/tests/integration/test_api.py +++ b/tests/integration/test_api.py @@ -71,6 +71,72 @@ def test_html_pages_served_locally_no_cdn(client, path: str, marker: str) -> Non assert 'href="https://' not in r.text +# Phase 33 (cache busting): the five HTML pages revalidate (no-cache) with +# ?v= asset refs; /assets/* is immutable for a year; /api/* is +# untouched. The token itself is unit-tested in tests/unit/test_caching.py. + + +def test_index_page_no_cache_with_versioned_asset_refs(client) -> None: + """GET / — always revalidated, and the stylesheet reference carries + the process version token (non-empty, matching asset_version()).""" + from app.core.caching import asset_version + + token = asset_version() + assert token # non-empty in every supported environment + + r = client.get("/") + assert r.status_code == 200 + assert r.headers["cache-control"] == "no-cache" + assert f'href="/assets/styles.css?v={token}"' in r.text + # The unversioned reference is gone from the served body. + assert 'href="/assets/styles.css">' not in r.text + + +@pytest.mark.parametrize( + "path", + ["/sources.html", "/document.html", "/login.html", "/tuning.html"], +) +def test_html_pages_no_cache_with_versioned_refs(client, path: str) -> None: + """Each of the other four pages revalidates and carries at least one + versioned asset reference.""" + from app.core.caching import asset_version + + r = client.get(path) + assert r.status_code == 200 + assert r.headers["cache-control"] == "no-cache" + assert f"?v={asset_version()}" in r.text + + +def test_index_html_variant_no_cache_versioned(client) -> None: + """/index.html is the same page as / — same caching treatment.""" + from app.core.caching import asset_version + + r = client.get("/index.html") + assert r.status_code == 200 + assert r.headers["cache-control"] == "no-cache" + assert f"?v={asset_version()}" in r.text + + +def test_assets_served_immutable_for_a_year(client) -> None: + r = client.get("/assets/styles.css") + assert r.status_code == 200 + cc = r.headers["cache-control"] + assert "public" in cc + assert "max-age=31536000" in cc + assert "immutable" in cc + # The asset body is untouched (header-only middleware). + assert client.get("/assets/app.js?v=whichever").status_code == 200 + + +def test_api_health_gets_no_cache_control_injected(client) -> None: + """Baseline (pre-middleware) behavior for /api/*: FastAPI's JSON + responses ship no Cache-Control header — the middleware must not + inject one.""" + r = client.get("/api/health") + assert r.status_code == 200 + assert "cache-control" not in r.headers + + def test_styles_and_js_served(client) -> None: assert client.get("/assets/styles.css").status_code == 200 assert client.get("/assets/app.js").status_code == 200 diff --git a/tests/unit/test_caching.py b/tests/unit/test_caching.py new file mode 100644 index 0000000..fb50934 --- /dev/null +++ b/tests/unit/test_caching.py @@ -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 = '' + assert rewrite_asset_refs(html, TOKEN) == ( + '' + ) + + +def test_rewrite_versions_src_without_leading_slash() -> None: + html = '' + assert rewrite_asset_refs(html, TOKEN) == ( + '' + ) + + +def test_rewrite_versions_module_script_src() -> None: + html = '' + assert rewrite_asset_refs(html, TOKEN) == ( + '' + ) + + +def test_rewrite_versions_every_ref_in_one_pass() -> None: + html = ( + '' + "" + '' + ) + assert rewrite_asset_refs(html, TOKEN) == ( + '' + '' + '' + ) + + +def test_rewrite_is_idempotent_for_already_versioned_refs() -> None: + html = '' + assert rewrite_asset_refs(html, "deadbeef") == html + + +def test_rewrite_leaves_query_and_fragment_refs_alone() -> None: + html = '' + assert rewrite_asset_refs(html, TOKEN) == html + + +def test_rewrite_leaves_non_asset_refs_untouched() -> None: + html = ( + 'sources' + '' + 'login' + ) + assert rewrite_asset_refs(html, TOKEN) == html + + +def test_rewrite_returns_html_unchanged_when_no_asset_refs() -> None: + html = "

no assets here

" + 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 '' + + @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 == ( + "" + ) + + +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 == ( + "" + ) + + +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"hi", media_type="text/html") + assert asyncio.run(caching._read_body(resp)) == b"hi" + + +def test_read_body_copies_plain_response_memoryview() -> None: + from starlette.responses import Response + + resp = Response(content=memoryview(b"hi"), media_type="text/html") + assert asyncio.run(caching._read_body(resp)) == b"hi" + + +def test_read_body_drains_streaming_response() -> None: + from starlette.responses import StreamingResponse + + async def gen() -> AsyncIterator[bytes]: + yield b"" + yield b"" + + resp = StreamingResponse(content=gen(), media_type="text/html") + assert asyncio.run(caching._read_body(resp)) == b"" diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index ba10aab..7683afe 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -19,3 +19,14 @@ def test_create_app_warns_and_serves_api_only_without_static_dir( assert client.get("/api/health").status_code == 200 # …but the static mount is absent (no index page). assert client.get("/").status_code == 404 + + +def test_create_app_wires_caching_middleware() -> None: + """Phase 33: the app factory always attaches the cache-busting + middleware (by name) — even when the static dir is missing, so the + /api/* no-touch guarantee holds in every environment.""" + app2 = main_mod.create_app() + # ``mw.cls`` is starlette's opaque ``_MiddlewareFactory`` protocol — + # reach for ``__name__`` the same way starlette's own __repr__ does. + middleware_names = [getattr(mw.cls, "__name__", "") for mw in app2.user_middleware] + assert "CachingMiddleware" in middleware_names