From bb1e3cfcd7449e93f0cbe23c0d40d26802d9ee04 Mon Sep 17 00:00:00 2001 From: Vuong Hoang Date: Mon, 21 Sep 2026 23:44:37 -0700 Subject: [PATCH] fix(booth): templates were hot-reloading into a live service running older Python MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 19 of 25 live booths returned 500 with `UndefinedError: 'item_marks' is undefined`. Neither the old code nor the new code was broken — the service was running both at once. `booth.service` sets WorkingDirectory to this repo, so the repo IS the deployment root: no build step, no staging copy, the live service imports these files. Python is read once when the process starts. Jinja's FileSystemLoader re-reads a template on EVERY render. So the two halves of the service had different staleness rules, and editing booth.html deployed it instantly against Python from 22:03 that had never heard of the context the new markup wanted. The failure mode is worth naming precisely, because it is invisible to the suite by construction: the skew exists between a running process and the disk underneath it, so every test can pass against a tree that is simultaneously serving 500s. No amount of green catches this. The operator found it. Fixed at the source rather than with a reminder to restart. The template Environment is built here with auto_reload=False, so templates are cached at startup exactly like the Python, and there is ONE rule: nothing takes effect until you restart. The price is that template work needs a restart to see — that price is the entire point, and it is cheaper than a page of 500s while someone is reviewing. Building the Environment by hand means autoescape no longer comes from the Jinja2Templates constructor, so it is explicit and load-bearing: booth names, item names and mark text are all agent- or operator-authored strings that land in HTML. Verified escaped, not merely configured. Two tests hold the line — one on the snapshot property, one on the `dur` filter that is no longer incidental to the constructor. The environment is reachable at app.state.templates because a promise about the deployed service needs an assertion, and an assertion needs the env the app actually renders with. Also records the foot-gun in CLAUDE.md and persistent-memory: anyone editing this repo while the operator may be using the service is editing production. 244 tests. No version bump — the release tier for U2 is still the operator's call, and this rides with it. --- CLAUDE.md | 36 ++++++++++++++++++++++++++++++++++++ booth/app.py | 33 +++++++++++++++++++++++++++++++-- persistent-memory.md | 44 +++++++++++++++++++++++++++++++------------- tests/test_booth.py | 43 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 141 insertions(+), 15 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 409541b..4c44623 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -152,6 +152,42 @@ Ask which one you have before choosing a storage shape. - **Nothing deletes the operator's data on a timer** beyond the documented 24h TTL. Liveness is *flagged*, not enforced. +## ⚠ The repo IS the deployment root + +`booth.service` runs uvicorn with `WorkingDirectory=/home/lkraven/development/booth`. +There is no build step, no staging copy, and no separate deploy artifact: the +live service on `:8090` imports **these files**. Two consequences, and the second +one caused an outage. + +1. **A Python edit does nothing until you restart.** Expected, and documented + below. +2. **A template edit used to take effect INSTANTLY.** Jinja's `FileSystemLoader` + re-reads a template from disk on every render. So the two halves of the + service had different staleness rules, and editing `booth.html` deployed it + immediately against Python that had never heard of the context it wanted. + + On 2026-09-21 that put **19 of 25 live booths at 500** — + `UndefinedError: 'item_marks' is undefined` — with the Python from 22:03 and + the templates from 23:40. Neither version was broken; the service was running + both. The operator found it, not the suite, because no test can see a skew + that only exists between a process and the disk under it. + + Fixed at the source: the template `Environment` is now built with + `auto_reload=False`, so templates are cached at startup exactly like the + Python. **One rule now — nothing takes effect until you restart.** The price + is that template work needs a restart to see, and that price is the point. + `test_templates_do_not_hot_reload_from_disk` holds the line. + +**So: after ANY edit here — Python or template — the live service is stale until +you restart it.** If you are touching this repo while the operator may be using +the service, either restart promptly or expect him to be looking at the old +version. Never leave the tree in a state where a restart would 500. + +⚠ The `Environment` is hand-built now, which means `autoescape` is explicit +rather than inherited from the `Jinja2Templates` constructor. It is on +(`select_autoescape(["html", "xml"])`) and it is load-bearing: booth names, item +names and mark text are all agent- or operator-authored and land in HTML. + ## Working in here ```sh diff --git a/booth/app.py b/booth/app.py index 592c58f..2c99508 100644 --- a/booth/app.py +++ b/booth/app.py @@ -45,6 +45,7 @@ from fastapi.responses import ( Response, ) from fastapi.templating import Jinja2Templates +from jinja2 import Environment, FileSystemLoader, select_autoescape try: import markdown as _markdown @@ -522,8 +523,32 @@ def create_app( ttl_seconds = ttl_hours * 3600.0 max_upload_bytes = int(max_upload_mb * 1024 * 1024) - templates = Jinja2Templates(directory=str(TEMPLATES_DIR)) - templates.env.filters["dur"] = human_dur + # TEMPLATES ARE CACHED AT STARTUP, DELIBERATELY — `auto_reload=False`. + # + # `booth.service` runs uvicorn with WorkingDirectory set to this repo, so the + # repo IS the deployment root: there is no build step and no staging copy. + # Jinja's default FileSystemLoader re-reads a template from disk on every + # render, while the Python stays as it was when the process started. That + # gives the two halves of the service different staleness rules, and editing + # a template deploys it INSTANTLY against Python that may know nothing about + # the context it wants. + # + # It cost an outage on 2026-09-21: 19 of 25 live booths returned 500 with + # `UndefinedError: 'item_marks' is undefined` — new markup, old context, both + # running at once, and neither version broken on its own. The Python had + # started at 22:03 and the templates were from 23:40. + # + # With reload off there is ONE rule — nothing takes effect until you restart + # — so the running process is always a coherent snapshot of one commit. The + # price is that template work needs a `systemctl --user restart + # booth.service` to see; that price is the whole point. + env = Environment( + loader=FileSystemLoader(str(TEMPLATES_DIR)), + autoescape=select_autoescape(["html", "xml"]), + auto_reload=False, + ) + env.filters["dur"] = human_dur + templates = Jinja2Templates(env=env) @asynccontextmanager async def lifespan(app: FastAPI): @@ -548,6 +573,10 @@ def create_app( task.cancel() app = FastAPI(title="The Booth", lifespan=lifespan) + # The template environment, reachable for assertion: the snapshot property + # above is a promise about the DEPLOYED service, so it needs a test, and a + # test needs a handle on the env that the app actually renders with. + app.state.templates = templates ttl_display = int(ttl_hours) if float(ttl_hours).is_integer() else ttl_hours base_ctx = { diff --git a/persistent-memory.md b/persistent-memory.md index 278c022..b71d82e 100644 --- a/persistent-memory.md +++ b/persistent-memory.md @@ -28,19 +28,16 @@ _As of 2026-09-21:_ re-exports are asserted by a test. 192 tests green, `0.1.15`. - **U2 (marks) has landed** — `booth/marks.py`, contract at `docs/contracts/u2_marks.contract.md`, 242 tests green. Not yet deployed. -- **Two things are outstanding on U2 and both need the operator:** - 1. **Deploy + migrate, in that order.** The live service still runs the old - code, and four unanswered `*.ask.json` sidecars are live - (`dfa-concepts`, `sc-iso-spread`, `sindra-voice-1`, `run07-decisions`). - Migrating BEFORE deploying is the hazard: the old code would keep serving - the sidecar, an answer written there would land in the sidecar, and - `import_legacy_asks` skips a stem it has already imported — so that answer - would be lost. Restart the service first, then `booth marks-import ` - on each of the four. - 2. **The release tier.** U2 changes the CLI surface for 17 consuming handles - (`booth asks` → `booth marks`, new `marks-import`) and is a v1 unit, so it - reads minor-worthy — which needs explicit operator approval per the SemVer - rule. Nothing is bumped or tagged; the work is committed as SHAs. +- **U2 is DEPLOYED and the migration is done.** The service was restarted + 2026-09-21 23:41 and again after the `auto_reload` fix; all four legacy + sidecars imported (`dfa-concepts/dfa`, `run07-decisions/decisions`, + `sc-iso-spread/spread`, `sindra-voice-1/anchor`, all still open) with the + sidecars left on disk. Verified live: index + 25 booths x {booth page, marks + page, marks.json} all 200, plus zoom views on five booths. +- **Still needs the operator: the release tier.** U2 changes the CLI surface for + 17 consuming handles (`booth asks` -> `booth marks`, new `marks-import`) and is + a v1 unit, so it reads minor-worthy — which needs explicit approval per the + SemVer rule. Nothing is bumped or tagged; the work is committed as SHAs. - **`/heid-contract-review` on the U2 contract is still in flight** (panel mode, posted 2026-09-21, redacted copy at `/tmp/heid-contract-review/booth-20260922-061015/`). Triage it when it lands — @@ -152,6 +149,27 @@ _As of 2026-09-21:_ ## Tried and abandoned +- `[2026-09-21]` **Letting Jinja hot-reload templates while the repo is the + deployment root** — the cause of a live outage the same day U2 landed, and the + sharpest foot-gun in the repo. `booth.service` sets `WorkingDirectory` to this + repo, so the running service imports these files with no build step and no + staging copy. Python is read once at process start; Jinja's `FileSystemLoader` + re-reads a template **on every render**. Editing `booth.html` therefore + deployed it instantly against Python from 22:03 that knew nothing about + `item_marks`, and **19 of 25 live booths returned 500** with + `UndefinedError: 'item_marks' is undefined`. Neither the old code nor the new + code was broken — the service was running both at once. + **The lesson that generalises:** a skew between a process and the disk under it + is invisible to the test suite by construction, so no amount of green tests + would have caught it; the operator found it. Fixed at the source rather than + with a reminder — the `Environment` is hand-built with `auto_reload=False`, so + there is now ONE staleness rule (nothing takes effect until you restart) and + the running process is always a coherent snapshot of one commit. Asserted by + `test_templates_do_not_hot_reload_from_disk`. Watch the second-order risk the + fix introduces: a hand-built `Environment` does not inherit `autoescape` from + the `Jinja2Templates` constructor, and booth names, item names and mark text + are all agent-authored strings landing in HTML. + - `[2026-09-21]` **Five separate mechanisms to get one question next to one artifact** — `.forever`, the link board, `inline.py`'s placeholder DSL, `wrap_verbatim_html`'s six regexes, and the floating amber asks chip plus diff --git a/tests/test_booth.py b/tests/test_booth.py index 47ab88c..02c0a6b 100644 --- a/tests/test_booth.py +++ b/tests/test_booth.py @@ -1579,3 +1579,46 @@ def test_kept_lane_offers_a_direct_wipe_beside_release(client): # ...and it actually wipes. c.post("/b/bo/delete", follow_redirects=False) assert not d.exists() + + +# ---- the running service is a coherent snapshot ------------------------------ +# +# Outage, 2026-09-21: 19 of 25 live booths returned 500 with +# `UndefinedError: 'item_marks' is undefined`. Nothing was wrong with either the +# old code or the new code — the service was running BOTH. `booth.service` sets +# WorkingDirectory to the repo, so the repo IS the deployment root, and Jinja's +# FileSystemLoader re-reads a template from disk on every render while the Python +# stays as it was at process start. Editing a template therefore deployed it +# INSTANTLY, against Python that had never heard of the context it wanted. +# +# The fix is not "remember to restart" — it is to make the two halves fail the +# same way, so the running process is always the code as of its start time. + + +def test_templates_do_not_hot_reload_from_disk(tmp_path): + """Templates must be cached at startup, exactly like the Python is. + + With auto_reload on, the two halves of the service have DIFFERENT staleness + rules — Python needs a restart, templates do not — and any edit to a + template puts a live service into a state that was never tested: new markup + against old context. One consistent rule ("nothing takes effect until you + restart") turns a silent 500 storm into a change that simply has not + happened yet. + """ + from booth.app import create_app + + app = create_app(tmp_path, ttl_hours=24, start_sweeper=False) + env = app.state.templates.env + assert env.auto_reload is False, ( + "templates hot-reload from disk while the Python does not — " + "editing one deploys it to the live service instantly" + ) + + +def test_the_dur_filter_survives_the_custom_environment(tmp_path): + """The env is hand-built now, so the filter registration is no longer + incidental to the constructor.""" + from booth.app import create_app + + app = create_app(tmp_path, ttl_hours=24, start_sweeper=False) + assert app.state.templates.env.filters["dur"](3600) == "1h"