From 026a1fc39220b937e3a3021ea507fb81715d43e7 Mon Sep 17 00:00:00 2001 From: Vuong Hoang Date: Tue, 22 Sep 2026 00:20:47 -0700 Subject: [PATCH] =?UTF-8?q?fix(marks):=20v0.2.2=20=E2=80=94=20nine=20findi?= =?UTF-8?q?ngs=20from=20the=20cross-frontier=20bug-hunt=20panel?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `/heid-bug-hunt` on U2's diff, four arms, artifact-only. Eight findings were real against live code; a ninth was already closed by v0.2.1 and is recorded as declined. Full triage in persistent-memory.d/2026-09-22-bug-hunt-panel.md. THE LOCK LIFECYCLE (4/4 convergent, and two defects in one place) `_Locked.__exit__` unlinked `.marks.lock` on the no-op path so a booth that had never been marked was left exactly as it was found. `flock` binds to an INODE: unlinking it under a blocked waiter leaves that waiter holding an exclusive lock on a deleted file while the next writer creates a fresh lock and takes it immediately. Two processes then run the read-modify-write concurrently, the later os.replace drops the earlier one's mark, and both obeyed the protocol. The cleanup existed to protect the booth's TTL, and was failing at that too: creating or removing a directory entry bumps the DIRECTORY's mtime, which is what `_newest_mtime` seeds from. The guard's comment reasons about the lock file's own mtime and misses that the directory moved underneath it. One fix: never unlink the lock, exempt `..lock` dotfiles from `_newest_mtime`, and restore the directory's mtime after creating one. THE READ PATH'S BLAST RADIUS `_clean_text` did `(text or "").replace(...)` and `marks_for` sorts on `(created, id)`, so a stored `text` that was a dict or a `created` that was a number raised out of the read path. `list_booths` reads every booth's marks on every index load, so one hand-edited file returned 500 for `/` and `/healthz` across all 25 booths. Guarded in two layers — a named type check and a `_hydrate_safe` backstop that cannot raise — and an unreadable mark now renders as ⚠ broken rather than as an empty note. ALSO - import_legacy_asks stamped `created` at whole-second resolution, so two sidecars from the same second lost the ordering the importer had just established and re-sorted alphabetically. Microseconds, per the stated `(mtime, name)` rule. - The five mark-write routes ran a blocking flock on the event loop; they now dispatch through run_in_threadpool, asserted structurally like INV-1. - `/answer` 500'd on a non-string `notes` form value where `/note` handled it. - The inline-doc tile had a flag control and no note field. - The marks panel was suppressed on any booth carrying a links.md. - The viewer's arrow keys and Escape threw away a note being typed. CLI `booth marks` printed a traceback and exited 0 on a failed read, and `--wait` emitted a whole JSON document per poll. `booth answer --wait` read a damaged file as "not yet" and spun the full hour. Both now use real exit codes — 0 ok, 1 unanswered/timed-out, 2 no such pick, 3 unreadable — and `--wait` prints once. `marks.read_error()` lets the CLI ask what the page must not: the browser stays lenient, the machine consumer gets the truth. `scripts/booth` had no tests; it has five now, run against the real script under the system python3, which also makes them a live check on INV-1. 275 tests (253 before). Live service restarted, 25/25 booth pages verified 200. --- booth/app.py | 58 +++- booth/marks.py | 106 +++++- booth/templates/_marks.html | 28 +- booth/templates/booth.html | 12 +- booth/templates/doc.html | 16 +- booth/templates/view.html | 10 + .../2026-09-22-bug-hunt-panel.md | 79 +++++ persistent-memory.md | 62 +++- pyproject.toml | 2 +- scripts/booth | 91 ++++- tests/test_cli.py | 121 +++++++ tests/test_marks.py | 310 ++++++++++++++++++ 12 files changed, 842 insertions(+), 53 deletions(-) create mode 100644 persistent-memory.d/2026-09-22-bug-hunt-panel.md create mode 100644 tests/test_cli.py diff --git a/booth/app.py b/booth/app.py index 95f7c5a..6396980 100644 --- a/booth/app.py +++ b/booth/app.py @@ -45,6 +45,7 @@ from fastapi.responses import ( Response, ) from fastapi.templating import Jinja2Templates +from starlette.concurrency import run_in_threadpool from jinja2 import Environment, FileSystemLoader, select_autoescape try: @@ -169,12 +170,27 @@ def human_dur(seconds: float) -> str: def _newest_mtime(path: Path) -> float: - """Newest mtime among a folder and everything under it.""" + """Newest mtime among a folder and everything under it — OUR LOCKS EXCEPT. + + A booth's age is how long since somebody touched it, and a lock sidecar is + machinery: `marks.py` and `links.py` each create one on the way into a + read-modify-write, including one that turns out to change nothing. Counting + it made reading-through-a-write-path look like activity, and a no-op mark + POST on a dead booth reset its clock. + + The exclusion is `..lock` — a DOTfile, which is the Booth's own + namespace. An agent that posts a real artifact called `build.lock` still + gets its clock counted. Everything else counts too, dotfiles included, + because `.marks.json`, `.blurred` and `.pins` are the operator doing + something. + """ try: newest = path.stat().st_mtime except OSError: return 0.0 for p in path.rglob("*"): + if p.name.startswith(".") and p.name.endswith(".lock"): + continue try: m = p.stat().st_mtime except OSError: @@ -478,6 +494,18 @@ PICKUP_WORDS = ( ).split() +def _form_text(form, key: str) -> str: + """One form field as text, or "" for anything that is not text. + + A multipart FILE part named `notes` parses to an UploadFile, not a str, and + every downstream cleaner calls `.replace` on what it is handed. Coercing + here keeps that decision in one place instead of one `isinstance` per call + site — which is how `/note` came to have the guard and `/answer` not to. + """ + value = form.get(key) + return value if isinstance(value, str) else "" + + def safe_upload_name(name: str, fallback: str) -> str: """Reduce a client-supplied filename to a safe basename (no path, no hidden).""" base = (name or "").replace("\\", "/").split("/")[-1].strip() @@ -760,13 +788,22 @@ def create_app( if spec.error is not None: raise HTTPException(status_code=400, detail=spec.error) who = request.client.host if request.client else "" + # `notes` is whatever the form parser yielded. A multipart FILE part + # named `notes` is an UploadFile, and `_clean_notes` calls `.replace` on + # it — a 500 on hostile-but-legal input, where the sibling `/note` route + # returns 400 for exactly the same class of value. Same parser, same + # question, one answer. + notes = _form_text(form, "notes") try: if spec.multi: choice = {q["key"]: form.get(f"choice.{q['key']}") for q in spec.questions} - qnotes = {q["key"]: form.get(f"notes.{q['key']}") for q in spec.questions} - answer_pick(booth, mark_id, choice, form.get("notes", ""), who=who, qnotes=qnotes) + qnotes = {q["key"]: _form_text(form, f"notes.{q['key']}") + for q in spec.questions} + await run_in_threadpool(answer_pick, booth, mark_id, choice, notes, + who=who, qnotes=qnotes) else: - answer_pick(booth, mark_id, form.get("choice"), form.get("notes", ""), who=who) + await run_in_threadpool(answer_pick, booth, mark_id, + form.get("choice"), notes, who=who) except AskError as exc: raise HTTPException(status_code=400, detail=str(exc)) return _mark_redirect(name, form, f"mark-{quote(mark_id, safe='')}") @@ -785,8 +822,9 @@ def create_app( target = raw_target if isinstance(raw_target, str) and raw_target else None text = form.get("text") try: - mark = write_note(booth, target, text if isinstance(text, str) else "", - who=request.client.host if request.client else "") + mark = await run_in_threadpool( + write_note, booth, target, text if isinstance(text, str) else "", + who=request.client.host if request.client else "") except AskError as exc: raise HTTPException(status_code=400, detail=str(exc)) return _mark_redirect(name, form, f"mark-{quote(mark.id, safe='')}") @@ -806,7 +844,9 @@ def create_app( raise HTTPException(status_code=400, detail="a flag needs a target") on = str(form.get("on", "1")) not in ("0", "", "false", "off") try: - set_flag(booth, target, on, who=request.client.host if request.client else "") + await run_in_threadpool( + set_flag, booth, target, on, + who=request.client.host if request.client else "") except AskError as exc: raise HTTPException(status_code=400, detail=str(exc)) return _mark_redirect(name, form, f"item-{quote(target, safe='')}") @@ -820,7 +860,7 @@ def create_app( mark_id = form.get("mark") if not isinstance(mark_id, str) or not mark_id: raise HTTPException(status_code=400, detail="which mark?") - delete_mark(booth, mark_id) + await run_in_threadpool(delete_mark, booth, mark_id) return _mark_redirect(name, form, "marks") @app.post("/b/{name}/import-asks") @@ -832,7 +872,7 @@ def create_app( migrated from the page you are already looking at. """ booth = resolve_booth(name) - import_legacy_asks(booth) + await run_in_threadpool(import_legacy_asks, booth) form = await request.form() return _mark_redirect(name, form, "marks") diff --git a/booth/marks.py b/booth/marks.py index 380a0c0..17c8e34 100644 --- a/booth/marks.py +++ b/booth/marks.py @@ -218,6 +218,24 @@ def _read_raw_strict(booth: Path) -> list[dict]: return entries +def read_error(booth: Path) -> str | None: + """Why this booth's marks cannot be read, or None if they can. + + `marks_for` is lenient on purpose — a review page that will not load is + worse than one missing an annotation — and that leniency turns an + unreadable file into "no marks". For a BROWSER that is the right trade. For + the CLI it is not: a session that asked a question and is told "no such + pick" will conclude the question was never posted, when in fact the file + holding it is damaged. A machine consumer can act on the difference, so it + gets to ask. + """ + try: + _read_raw_strict(booth) + except MarksCorrupt as exc: + return str(exc) + return None + + def _write_raw(booth: Path, entries: list[dict]) -> None: """Atomic replace, so a reader never sees a half-written document and a crash mid-write cannot truncate the file into a shorter — and therefore @@ -249,12 +267,20 @@ class _Locked: self.booth.mkdir(parents=True, exist_ok=True) lock = self.booth / MARKS_LOCK # `touch(exist_ok=True)` on an EXISTING file bumps its mtime, and a - # booth's TTL is measured from its newest mtime including dotfiles — so - # an unconditional touch would keep a booth alive just for being read - # through a write path. Create it only when it is not there. + # booth's TTL is measured from its newest mtime — so an unconditional + # touch would keep a booth alive just for being read through a write + # path. Create it only when it is not there. + # + # ONCE CREATED, THE LOCK FILE IS NEVER REMOVED (see __exit__). if not lock.exists(): + # Creating a directory entry bumps the DIRECTORY's mtime, which is + # what `_newest_mtime` seeds from — so making our own lock file + # would itself read as activity. Put the clock back: the lock is + # machinery, and machinery is not the operator touching the booth. + before = self.booth.stat() lock.touch() self._made_lock = True + os.utime(self.booth, (before.st_atime, before.st_mtime)) self._lf = lock.open("r+") fcntl.flock(self._lf, fcntl.LOCK_EX) try: @@ -264,8 +290,6 @@ class _Locked: fcntl.flock(self._lf, fcntl.LOCK_UN) self._lf.close() self._lf = None - if self._made_lock: - lock.unlink(missing_ok=True) raise self._before = _fingerprint(self.entries) return self @@ -284,10 +308,17 @@ class _Locked: # would otherwise keep a dead booth alive forever. if exc_type is None and _fingerprint(self.entries) != self._before: _write_raw(self.booth, self.entries) - elif self._made_lock and not (self.booth / MARKS_FILE).exists(): - # Nothing was written and this booth had no marks before: do not - # leave a lock file behind as the only trace of a no-op. - (self.booth / MARKS_LOCK).unlink(missing_ok=True) + # THE LOCK FILE IS NEVER UNLINKED. It used to be, on the no-op path, + # so a booth that had never been marked was left exactly as it was + # found. That tidiness cost mutual exclusion outright: `flock` binds + # to an INODE, so unlinking the lock while a second writer is blocked + # on it leaves that writer holding an exclusive lock on a deleted + # file, and the NEXT writer creates a fresh lock and takes it at + # once. Two processes then run the read-modify-write concurrently, + # the later `os.replace` drops the earlier one's mark, and both of + # them obeyed the protocol. A zero-byte dotfile is the cheaper + # thing to leave behind — `booth_items` skips it, the zip skips it, + # and `_newest_mtime` exempts it so it cannot hold a booth open. finally: fcntl.flock(lf, fcntl.LOCK_UN) lf.close() @@ -301,6 +332,25 @@ class _Locked: # ---- read ------------------------------------------------------------------- +def _entry_type_error(entry: dict) -> str | None: + """The stored scalars this module refuses to guess at. + + `_clean_text` did `(text or "").replace(...)` and `marks_for` sorts on + `(created, id)` — so a stored `text` that is a dict, or a `created` that is a + number, raised AttributeError or TypeError out of the READ path. That is not + a marks bug, it is an INDEX bug: `list_booths` reads every booth's marks on + every page load and `/healthz` does the same, so one hand-edited or + foreign-written file took down the front page for every booth on the + service. A wrong type is a broken mark, and this module already knows how to + render one of those. + """ + for name in ("created", "by", "text", "error"): + value = entry.get(name) + if value is not None and not isinstance(value, str): + return f"{name} is {type(value).__name__}, not a string" + return None + + def _hydrate(entry: dict) -> Mark: """One stored entry -> one Mark, declarations normalized. @@ -312,6 +362,15 @@ def _hydrate(entry: dict) -> Mark: """ mid = entry["id"] shape = entry.get("shape") if entry.get("shape") in SHAPES else NOTE + bad = _entry_type_error(entry) + if bad is not None: + # `created` is dropped rather than coerced, which sorts the entry to the + # TOP of the booth's marks: a mark nobody can read is the one that wants + # looking at, and burying it under 270 items' worth of notes is how it + # stays unnoticed. Deterministic, and stated — `("", id)` against + # `(created, id)`. + return Mark(id=mid, shape=shape, target=None, created="", + error=f"unreadable mark: {bad}") target = entry.get("target") if not _valid_target(target): target = None @@ -359,11 +418,26 @@ def _hydrate(entry: dict) -> Mark: return Mark(**base, text=_clean_text(entry.get("text"))) +def _hydrate_safe(entry: dict) -> Mark: + """`_hydrate`, with the promise that it cannot raise. + + `_entry_type_error` covers the shapes we know how to name; this is the + backstop for the ones we do not, and it exists because of WHERE this runs. + One unreadable mark must cost that mark, never the page — and on the index + it is not even that booth's page, it is all of them. + """ + try: + return _hydrate(entry) + except Exception as exc: # noqa: BLE001 - deliberate + return Mark(id=str(entry.get("id", "")), shape=NOTE, target=None, + created="", error=f"unreadable mark: {exc}") + + def marks_for(booth: Path) -> list[Mark]: """Every mark in a booth, oldest first, declarations normalized and answers folded in. ONE file read — which is the whole point of the storage shape.""" entries = _read_raw(booth) - marks = [_hydrate(e) for e in entries] + marks = [_hydrate_safe(e) for e in entries] # (created, id) rather than created alone: two marks written in the same # second would otherwise order by however json listed them. marks.sort(key=lambda m: (m.created, m.id)) @@ -642,7 +716,17 @@ def import_legacy_asks(booth: Path) -> list[Mark]: "id": stem, "shape": PICK, "target": None, - "created": datetime.fromtimestamp(mtime).astimezone().isoformat(timespec="seconds"), + # MICROSECONDS, not seconds. `found` is ordered by fractional + # mtime and `marks_for` re-sorts on this string, so truncating + # to the whole second threw away the only thing distinguishing + # two sidecars written in the same second — and the `(created, + # id)` tie-break then silently re-sorted them alphabetically, + # reversing the order the importer had just established. The + # ROADMAP states this import's order is `(mtime, name)`; an + # order that is stated and not kept is worse than one never + # claimed. + "created": datetime.fromtimestamp(mtime).astimezone().isoformat( + timespec="microseconds"), "declaration": decl, "answer": answer, } diff --git a/booth/templates/_marks.html b/booth/templates/_marks.html index cd2c5bc..34beeb3 100644 --- a/booth/templates/_marks.html +++ b/booth/templates/_marks.html @@ -13,11 +13,35 @@ Works with JS off — plain form POST, every shape. An answered pick shows the recorded judgment and a collapsed "change" form, because the mark is the CURRENT judgment and not a log. #} +{# A mark carrying `error` is sorted out FIRST, whatever shape it claims. A + pick keeps its own ⚠ broken rendering below (richer — it has a declaration to + show); a broken note would otherwise render as an empty
 with a withdraw
+   button, indistinguishable from a note the operator wrote and then cleared,
+   and a broken flag would link to a target that is not there. Unreadable state
+   is visible state — the rule `_hydrate` states for picks, applied to all
+   three. #}
+{% set broken = marks | selectattr('error') | rejectattr('shape', 'equalto', 'pick') | list %}
 {% set picks = marks | selectattr('shape', 'equalto', 'pick') | list %}
-{% set notes = marks | selectattr('shape', 'equalto', 'note') | list %}
-{% set flags = marks | selectattr('shape', 'equalto', 'flag') | list %}
+{% set notes = marks | selectattr('shape', 'equalto', 'note') | rejectattr('error') | list %}
+{% set flags = marks | selectattr('shape', 'equalto', 'flag') | rejectattr('error') | list %}
 
+{% for a in broken %} +
+
+ ⚠ broken + {{ a.id }} + +
+ + {% if marks_page %}{% endif %} + +
+
+

This mark could not be read: {{ a.error }}

+
+{% endfor %} + {% for a in picks %}
diff --git a/booth/templates/booth.html b/booth/templates/booth.html index 203c192..bf96406 100644 --- a/booth/templates/booth.html +++ b/booth/templates/booth.html @@ -94,7 +94,11 @@ back to the flagged items. Always rendered on a gallery booth — the add-note field is a control, not a result, so it has to be there before the first mark exists. #} -{% if not board %} +{# `marks or not board`: the standing link board renders as a board rather than + a gallery, and the add-note control would be noise on it — but the + suppression was unconditional, so a pick declared on a booth that happens to + carry a links.md had no form to answer it and nothing said so. #} +{% if marks or not board %} {% include "_marks.html" %} {% endif %} @@ -187,6 +191,12 @@ {% else %}
{{ it.rendered }}
{% endif %} + {# The doc branch had `markcontrols` and not `marknotes`, so the + operator could point at a report and not write down why — on the + one item kind whose whole content is prose. Exactly the + "patched two of three" failure the blurtoggle macro above was + written to prevent, recurring on the macro written to prevent it. #} + {{ marknotes(name_url, it, item_marks.get(it.name, [])) }} {% else %} diff --git a/booth/templates/doc.html b/booth/templates/doc.html index 0c96321..d2ded30 100644 --- a/booth/templates/doc.html +++ b/booth/templates/doc.html @@ -33,8 +33,18 @@ white-space:pre-wrap} {% endblock %} diff --git a/booth/templates/view.html b/booth/templates/view.html index a99e866..0493738 100644 --- a/booth/templates/view.html +++ b/booth/templates/view.html @@ -99,7 +99,17 @@ img.addEventListener('load', evaluate); window.addEventListener('resize', evaluate); if (img.complete) evaluate(); + + /* An arrow key inside the note field is a CARET move, not a navigation. + The handler is on `document` and the note textarea shipped into this same + page, so typing a note and reaching for ← threw the draft away; Escape + did it in one keystroke. Anything editable keeps its own keys. */ + function isEditable(el) { + return !!(el && (el.isContentEditable || + /^(input|textarea|select)$/i.test(el.tagName || ''))); + } document.addEventListener('keydown', function (e) { + if (isEditable(e.target)) return; if (e.key === 'Escape') window.location.href = BACK; else if (e.key === 'ArrowLeft' && PREV) window.location.href = PREV; else if (e.key === 'ArrowRight' && NEXT) window.location.href = NEXT; diff --git a/persistent-memory.d/2026-09-22-bug-hunt-panel.md b/persistent-memory.d/2026-09-22-bug-hunt-panel.md new file mode 100644 index 0000000..a5d345e --- /dev/null +++ b/persistent-memory.d/2026-09-22-bug-hunt-panel.md @@ -0,0 +1,79 @@ +# The U2 bug-hunt panel — full triage + +**Date:** 2026-09-22 · **Thread:** `01M33XEC1H0298C0D968FWBN7A` · +**Reply:** `01M33YZZ1VYGZ04JGNXNTBXDKS` · **Shipped as:** `v0.2.2` + +`/heid-bug-hunt` on U2's diff (+2251/−632, 20 sections, 18 post-change +snapshots). Four arms — Gróa (Grok), Hulda (Codex), Regin (GLM-5.2), Kimi +(kimi-k3) — artifact-only, 4/4 clean transport. Heid adjudicated **9 findings +(6 bug / 3 robustness)**. Staleness was disclosed at build: `app.py` was edited +after the 06:38:52Z capture. + +## Triage, five-category + +### Category 1 — genuine add (8 taken, all shipped) + +| # | finding | where | why it was real | +|---|---|---|---| +| 1 | Lock-inode split on the no-op unlink (**4/4 convergent**) | `marks._Locked` | `flock` binds to an inode; unlinking under a waiter destroys mutual exclusion silently | +| 2 | No-op lock churn resets the TTL via **directory** mtime | `marks._Locked` + `app._newest_mtime` | the guard's own comment reasons about the lock FILE's mtime; the directory is what the sweeper reads | +| 3 | Non-string `text` / `created` raise out of the read path | `marks._clean_text`, `marks_for` sort | `list_booths` reads every booth per page load → one bad file 500s `/` and `/healthz` | +| 4 | Legacy import stamped `created` at whole-second resolution | `marks.import_legacy_asks` | same-second sidecars re-sorted alphabetically, reversing the order the importer had just set — violates the stated `(mtime, name)` rule | +| 5 | `/answer` 500s on a non-string `notes` form value | `app.booth_answer` | the sibling `/note` guards it; same parser, same class of value, two answers | +| 6 | All five mark-write routes hold a blocking `flock` on the event loop | `app.py` | a contended lock freezes every route, not just the one request | +| 7 | CLI conflates a reader crash with "open" / "unanswered" | `scripts/booth` | `marks` printed a traceback and exited 0; `answer --wait` spun the full hour on a damaged file | +| 8 | The inline-doc tile had `markcontrols` and not `marknotes` | `booth.html` | flag a report, cannot say why — on the one item kind that is prose | + +Two more taken on the same sweep, found while fixing the above rather than by +the panel: a broken mark of any shape now renders **⚠ broken** instead of as an +empty note (the rule `_hydrate` states for picks, applied to all three shapes), +and the marks panel is no longer suppressed on a booth that carries a +`links.md` *and* has marks. + +### Category 3 — restatement of a settled prior (1, no change) + +**Corrupt read → filtered writeback → silent deletion** (hulda F2, kimi F3, +gróa F4; Heid ranked it #3). **Already fixed in `v0.2.1`** by +`_read_raw_strict` + `MarksCorrupt` — reads lenient, writes strict. The panel +reviewed the pre-fix capture and the staleness was disclosed up front. Verified +against the current source before declining, not assumed. + +This is the exact case the cross-frontier triage discipline warns about: a +confident, well-argued, four-arm-corroborated finding against code that no +longer exists. **Check what the peer actually read before treating an omission +or a defect claim as new.** + +### Category 4 — out of place, parked (2) + +- **Note-id recycling** (`note-1` reused after a withdrawal) lets a stale tab + delete a newer note. Real mechanism; needs two tabs and an interleaving, and + the Booth has one viewer. Non-reused ids are a schema change, not a patch. +- **Unvalidated flag / note targets** accumulate orphan marks. Targets come + from rendered items; the operator is the only writer through the browser. + +### Category 5 — wrong-grounding (1) + +**`delete_mark` can remove a pick, not only a note.** Framed as an +access-control divergence. There is no auth by design, and restricting it would +remove the only way to withdraw a pick that hydrates broken. Declined; the +docstring is the thing that was imprecise, not the behaviour. + +## What the round is worth remembering for + +1. **The two review gates stayed complementary a second time.** The contract + panel (2026-09-21) found three defects; this bug-hunt found eight more, with + **no overlap**. Both ran on the same unit. Neither substitutes. +2. **The panel beat the code's own comments three times.** The bundle's comments + are unusually honest and still wrong about what protected the TTL, and + "written atomically" sat next to a filter-then-replace. **A comment is a + claim, and a claim can be tested.** +3. **The headline bug class shipped with zero guard coverage, and both mutation + tables said so.** `test_a_no_op_write_does_not_touch_the_booth` asserted only + that `.marks.json` was absent — so removing the lock unlink, removing the + whole lock lifecycle, or bumping the directory clock all **SURVIVED** it. The + test asserted an artifact of the property instead of the property. The + replacement asserts `booth_age_seconds` directly, with a positive control (a + real mark still resets the clock) so the fix cannot overshoot into "marking + is never activity". +4. **`scripts/booth` had no tests at all** and two findings lived there. It has + five now, running the real script under the system `python3`. diff --git a/persistent-memory.md b/persistent-memory.md index cec0697..a014512 100644 --- a/persistent-memory.md +++ b/persistent-memory.md @@ -1,6 +1,6 @@ # Persistent memory — booth -_Last updated: 2026-09-21_ +_Last updated: 2026-09-22_ > **Always check for `/tmp/booth-dev-handoff.md`** — if it exists and its > `Written:` stamp is under 8 hours old, read it (it carries the in-flight @@ -17,13 +17,15 @@ loop it turned out to actually be. ## Current state / in-flight -_As of 2026-09-21 (late):_ +_As of 2026-09-22:_ - **v1 is gated on seven units** in `ROADMAP.md`, dependency-ordered **U1 → U2 → {U3, U4, U5} → U7**, with **U6 independent**. -- **U1 and U2 are landed and released.** Current version `0.2.1`, deployed to the - live service, 253 tests green, tree clean. U1 `ce598b3`; U2 `c7f9437` released as - `v0.2.0`, then `5e41108` as `v0.2.1` carrying four cross-frontier panel findings. +- **U1 and U2 are landed and released.** Current version `0.2.2`, deployed to the + live service, 275 tests green, tree clean, 25/25 booth pages verified 200 after + the deploy. U1 `ce598b3`; U2 `c7f9437` released as `v0.2.0`, then `5e41108` as + `v0.2.1` (four contract-panel findings), then `v0.2.2` carrying the + **bug-hunt panel's** nine (below). - **U5 is the next unit** (operator, 2026-09-21): **self-announcing booths.** `.booth.json` carrying `{handle, title, why, created}`, written by the CLI from `$ALTHING_HANDLE`; the index card gains provenance and a one-line purpose, and @@ -38,16 +40,58 @@ _As of 2026-09-21 (late):_ and that rule must stay stated. Also worth knowing before scoping: enforcing the link rule without giving job 5 a home first just makes it homeless — that is the lesson from the 69% rot, and U5 is the home. -- **One heid dispatch is still outstanding**: the `/heid-bug-hunt` on U2's diff, - msg `01M33XEC1H0298C0D968FWBN7A`, thread same. Triage it the same five-category - way when it lands. **Do not assume it is ceremony** — the contract-review panel - that preceded it found three real defects in already-released code. +- **No heid dispatch is outstanding.** The `/heid-bug-hunt` on U2's diff landed + 2026-09-22 and shipped as `v0.2.2`; see the dated entry below. - Live service `active` on `:8090`, 25 booths, verified 25 × 3 page types after the last deploy. The booth set churns: `sindra20-engines` and `sindra-finalists` were swept during the session, `cr123a-to-d-sleeve` and `sindra` appeared. ## Recent decisions +- `[2026-09-22]` **The U2 bug-hunt panel landed and it was not ceremony — + `v0.2.2`.** Nine adopted findings across four arms; eight were real against + live code and one was already fixed. The headline was **4/4 convergent from + four different angles**: `_Locked.__exit__` unlinked `.marks.lock` on the no-op + path, and `flock` binds to an INODE — so a writer blocked on the old inode + proceeds while the next writer creates a fresh lock file and takes it at once. + Two processes then run the read-modify-write concurrently and the later + `os.replace` drops a mark, with both of them obeying the protocol. **The + cleanup existed to protect the booth's TTL and it was failing at that too**: + creating and removing a directory entry bumps the DIRECTORY's mtime, which is + what `_newest_mtime` actually seeds from, so a no-op reset the clock it was + written to leave alone. Same code region, two defects, one fix — never unlink + the lock, exempt `..lock` dotfiles from `_newest_mtime`, and put the + directory's mtime back after creating one. Full triage in + `persistent-memory.d/2026-09-22-bug-hunt-panel.md`. +- `[2026-09-22]` **The lenient reader's blast radius was the whole service, not + one booth.** `_clean_text` did `(text or "").replace(...)` and `marks_for` + sorts on `(created, id)`, so a stored `text` that was a dict or a `created` + that was a number raised out of the READ path — and `list_booths` reads every + booth's marks on every index load. One hand-edited file 500'd `/` and + `/healthz` for all 25 booths. Fixed in two layers, matching the house posture: + a named type check (`_entry_type_error`) plus a `_hydrate_safe` backstop that + cannot raise, and the panel now RENDERS an unreadable mark as ⚠ broken instead + of as an empty note. **The general shape: a lenient reader is only lenient if + the leniency is bounded by where it runs.** `marks_for` was written for one + booth's page and is called in a loop over every booth. +- `[2026-09-22]` **`booth marks` / `booth answer` got real exit codes**, because + a read that CRASHED was indistinguishable from a read that said no. `marks` + printed a traceback and exited 0 (a caller's `jq` saw success and got + nothing); `answer --wait` read a damaged file as "not yet" and spun for the + full hour before blaming the operator. Now `0 ok · 1 unanswered/timed-out · + 2 no such pick · 3 unreadable`, and `read_error()` was added to `marks.py` so + the CLI can ask the question the browser must not: the page stays lenient, the + machine consumer gets the truth. Also `--wait` now prints ONCE — it was + emitting a whole JSON document per poll, so a captured `--wait` held several + concatenated values and parsed as none of them. +- `[2026-09-22]` **`scripts/booth` had zero tests and now has five** + (`tests/test_cli.py`). The panel's guard-strength tables returned UNVERIFIED + for every CLI claim because nothing in the suite executed the script — two of + the round's findings lived in exactly that gap. The new tests run the real + script under the system `python3`, which makes them a live check on INV-1 + (stdlib-only) as a side effect: a third-party import in `marks.py` now fails + in the suite the same way it would fail on a fleet host. + - `[2026-09-21]` **v0.2.0 cut and announced; v0.2.1 fixed what the announcement was already wrong about.** Operator approved the minor (a v1 unit closed plus a CLI surface change for 17 consuming handles clears the release-note bar). The diff --git a/pyproject.toml b/pyproject.toml index c154943..9c6f4f8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "booth" -version = "0.2.1" +version = "0.2.2" description = "The Booth — a dead-simple standing web server that scans a data dir of drop-folders and renders each as an ephemeral media 'booth' (image/webm/audio auto-gallery, or a folder's own index.html verbatim). Also accepts browser/curl uploads for pickup under a human-readable id. 24h TTL, then the folder is wiped. Fleet tool for CC sessions to surface A/B and smoke results to the operator." requires-python = ">=3.11" dependencies = [ diff --git a/scripts/booth b/scripts/booth index a3af48c..1534227 100755 --- a/scripts/booth +++ b/scripts/booth @@ -22,6 +22,13 @@ # booth answer [--wait [SECS]] # print ONE pick's answer (exit 1 if unanswered); # --wait polls until it lands (default 3600 s) +# +# EXIT CODES for the two reading verbs. A read that FAILED gets its own code so +# a caller can tell "not yet" from "the file is damaged" — conflating them is +# how a broken `.marks.json` used to look like an unanswered question and wait +# out the full hour. +# marks 0 read ok · 1 --wait timed out with picks open · 3 unreadable +# answer 0 answered · 1 unanswered · 2 no such pick · 3 unreadable # booth marks-import import legacy *.ask.json into .marks.json # booth asks alias for `marks` (deprecated) # @@ -270,6 +277,15 @@ except MarksCorrupt as exc: ;; marks|asks) # booth marks [--wait [SECS]] (`asks` is the deprecated alias) + # + # EXIT CODES. 0 = the read succeeded and the document is on stdout; 1 = + # --wait gave up with picks still open (the document is still printed); 3 = + # the marks could not be read at all. A reader that CRASHED must never look + # like an answer — the old shape printed a traceback and exited 0, so a + # caller piping to `jq` saw success and got nothing. + # + # Whether anything is still open is in the payload's `open` list. The read + # verb does not encode it in its status: a successful read is a success. [ $# -ge 1 ] || usage name="$1"; shift wait_s=0 @@ -278,19 +294,41 @@ except MarksCorrupt as exc: # os.replace, and a 2 s cadence is plenty for a human clicking a radio. deadline=$(( $(date +%s) + wait_s )) while :; do - BOOTH_SRC="$(cd "$(dirname -- "$(readlink -f -- "$0")")/.." && pwd)" python3 -c ' + # CAPTURED, not streamed. Printing inside the loop wrote one whole JSON + # document per poll, so `booth marks b --wait | jq` got several values + # concatenated and could parse none of them. The wait is a wait; the + # print is the result, and it happens once. + rc=0 + out="$(BOOTH_SRC="$(cd "$(dirname -- "$(readlink -f -- "$0")")/.." && pwd)" python3 -c ' import json, os, pathlib, sys sys.path.insert(0, os.environ["BOOTH_SRC"]) -from booth.marks import as_dict, marks_for, open_marks -marks = marks_for(pathlib.Path(sys.argv[1])) -print(json.dumps({"marks": [as_dict(m) for m in marks], - "open": [m.id for m in open_marks(marks)]}, - ensure_ascii=False, indent=2)) -sys.exit(1 if open_marks(marks) else 0) -' "$DATA/$name" && exit 0 - # exit 1 from the reader means at least one pick is still open - if [ "$wait_s" -eq 0 ]; then exit 0; fi +try: + from booth.marks import as_dict, marks_for, open_marks, read_error + booth = pathlib.Path(sys.argv[1]) + # Ask FIRST whether the file is readable. `marks_for` answers "no marks" + # for a damaged file, which is the right answer for a page and the wrong + # one for a session that wants to know whether its question survived. + broken = read_error(booth) + if broken: + print(f"booth: {broken}", file=sys.stderr) + sys.exit(3) + marks = marks_for(booth) + doc = json.dumps({"marks": [as_dict(m) for m in marks], + "open": [m.id for m in open_marks(marks)]}, + ensure_ascii=False, indent=2) +except Exception as exc: + print(f"booth: cannot read marks: {exc}", file=sys.stderr) + sys.exit(3) +print(doc) +sys.exit(2 if open_marks(marks) else 0) +' "$DATA/$name")" || rc=$? + case "$rc" in + 0) printf '%s\n' "$out"; exit 0 ;; # read ok, nothing open + 2) if [ "$wait_s" -eq 0 ]; then printf '%s\n' "$out"; exit 0; fi ;; + *) echo "cannot read marks in $name" >&2; exit 3 ;; + esac if [ "$(date +%s)" -ge "$deadline" ]; then + printf '%s\n' "$out" echo "timed out after ${wait_s}s with marks still open in $name" >&2; exit 1 fi sleep 2 @@ -304,20 +342,39 @@ sys.exit(1 if open_marks(marks) else 0) if [ "${1:-}" = "--wait" ]; then wait_s="${2:-3600}"; fi deadline=$(( $(date +%s) + wait_s )) while :; do - BOOTH_SRC="$(cd "$(dirname -- "$(readlink -f -- "$0")")/.." && pwd)" python3 -c ' + rc=0 + out="$(BOOTH_SRC="$(cd "$(dirname -- "$(readlink -f -- "$0")")/.." && pwd)" python3 -c ' import json, os, pathlib, sys sys.path.insert(0, os.environ["BOOTH_SRC"]) -from booth.marks import marks_for -booth, mid = sys.argv[1:3] -m = next((x for x in marks_for(pathlib.Path(booth)) if x.id == mid), None) +try: + from booth.marks import marks_for, read_error + booth, mid = sys.argv[1:3] + broken = read_error(pathlib.Path(booth)) + if broken: + print(f"booth: {broken}", file=sys.stderr) + sys.exit(3) + # id AND shape, matching the web route. Matching on id alone reported a + # note id as "unanswered" and then polled it for an hour — a question that + # could never be answered because it was never a question. + m = next((x for x in marks_for(pathlib.Path(booth)) + if x.id == mid and x.shape == "pick"), None) +except Exception as exc: + print(f"booth: cannot read marks: {exc}", file=sys.stderr) + sys.exit(3) if m is None: sys.exit(2) if m.answer is None: sys.exit(1) print(json.dumps(m.answer, ensure_ascii=False, indent=2)) -' "$DATA/$name" "$mid" && exit 0 - rc=$? - if [ "$rc" -eq 2 ]; then echo "no such pick: $name/$mid" >&2; exit 1; fi +' "$DATA/$name" "$mid")" || rc=$? + case "$rc" in + 0) printf '%s\n' "$out"; exit 0 ;; + 2) echo "no such pick: $name/$mid" >&2; exit 2 ;; + # A read that FAILED is not "not yet". Conflating them sent --wait + # spinning for the full hour on a broken file and then blamed the + # operator for not answering. + 3) echo "cannot read marks in $name" >&2; exit 3 ;; + esac if [ "$wait_s" -eq 0 ]; then echo "unanswered: $URL/b/$name/#mark-$mid" >&2; exit 1; fi if [ "$(date +%s)" -ge "$deadline" ]; then echo "timed out after ${wait_s}s waiting on $name/$mid" >&2; exit 1 diff --git a/tests/test_cli.py b/tests/test_cli.py new file mode 100644 index 0000000..6f5f301 --- /dev/null +++ b/tests/test_cli.py @@ -0,0 +1,121 @@ +"""`scripts/booth` — the surface every fleet session actually calls. + +It had no tests at all, which the 2026-09-22 bug-hunt panel found the hard way: +its guard-strength table returned UNVERIFIED for every CLI claim because nothing +in the suite executes the script. Two of that round's findings live in here. + +These run the real script under the real system `python3` with no venv, which +also makes them a live check on INV-1 (stdlib-only): a third-party import in +`marks.py` fails here the same way it fails on a fleet host. +""" +import json +import os +import pathlib +import subprocess + +import pytest + +SCRIPT = pathlib.Path(__file__).parent.parent / "scripts" / "booth" + +# Exit codes the verbs promise. 0 is a successful read; a reader that CRASHED +# must never be one of the meaningful codes, or a caller cannot tell "no" from +# "broken" — which is the whole finding. +OK, UNANSWERED, NO_SUCH_PICK, READER_FAILED = 0, 1, 2, 3 + + +def run(data, *args, **kw): + env = {**os.environ, "BOOTH_DATA_DIR": str(data), "BOOTH_URL": "http://booth.invalid"} + return subprocess.run([str(SCRIPT), *args], capture_output=True, text=True, + env=env, timeout=30, **kw) + + +@pytest.fixture +def booth(tmp_path): + b = tmp_path / "b" + b.mkdir() + return tmp_path, b + + +def _declare(booth_dir, mark_id="winner"): + import sys + sys.path.insert(0, str(pathlib.Path(__file__).parent.parent)) + from booth.marks import declare_pick + declare_pick(booth_dir, mark_id, + {"prompt": "Which one?", "options": ["A", "B"]}) + + +def test_marks_prints_one_json_document(booth): + """`booth marks ` is a read. Its stdout is parsed by the session that + called it, so it has to be ONE document — and exit 0, because the read + succeeded. Whether a pick is open is in the payload's `open` list, which is + where a caller should read it from.""" + data, b = booth + _declare(b) + r = run(data, "marks", "b") + assert r.returncode == OK, r.stderr + doc = json.loads(r.stdout) + assert doc["open"] == ["winner"] + + +def test_marks_wait_prints_once_not_once_per_poll(booth): + """`--wait` polls every 2 s and printed the whole document on every pass, so + a capture held several concatenated JSON values and `jq` could not read any + of them. The wait is a wait; the print is the result.""" + data, b = booth + _declare(b) + import sys + sys.path.insert(0, str(pathlib.Path(__file__).parent.parent)) + from booth.marks import answer_pick + + # Answer it after the first poll so --wait genuinely loops at least once. + r = subprocess.Popen([str(SCRIPT), "marks", "b", "--wait", "20"], + stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, + env={**os.environ, "BOOTH_DATA_DIR": str(data), + "BOOTH_URL": "http://booth.invalid"}) + import time + time.sleep(3) + answer_pick(b, "winner", "A") + out, err = r.communicate(timeout=30) + assert r.returncode == OK, err + json.loads(out) # ONE document, or this raises + + +def test_marks_reports_a_reader_failure_instead_of_printing_garbage(booth): + """A traceback on stdout with exit 0 is the worst of both: the caller's `jq` + sees success and gets nothing. A read that could not happen is its own + answer and gets its own code.""" + data, b = booth + (b / ".marks.json").write_bytes(b"\xff\xfe not utf-8 at all") + r = run(data, "marks", "b") + assert r.returncode == READER_FAILED, f"rc={r.returncode} out={r.stdout!r}" + + +def test_answer_distinguishes_a_crash_from_an_unanswered_pick(booth): + """`answer` funnelled a reader crash and "not yet answered" through the same + exit 1, so `--wait` spun for the full hour on a broken file and then blamed + the operator for not answering.""" + data, b = booth + _declare(b) + r = run(data, "answer", "b", "winner") + assert r.returncode == UNANSWERED + + (b / ".marks.json").write_bytes(b"\xff\xfe not utf-8 at all") + r = run(data, "answer", "b", "winner", "--wait", "6") + assert r.returncode == READER_FAILED, ( + "a crash was read as 'unanswered' and waited out the timeout" + ) + + +def test_answer_on_a_note_id_says_no_such_pick(booth): + """`answer` matched on id alone while the web route filters on shape, so a + note id was reported 'unanswered' and polled forever — a question that could + never be answered because it was never a question.""" + data, b = booth + import sys + sys.path.insert(0, str(pathlib.Path(__file__).parent.parent)) + from booth.marks import write_note + write_note(b, "a.png", "just a note") + + r = run(data, "answer", "b", "note-1") + assert r.returncode == NO_SUCH_PICK + assert "no such pick" in r.stderr diff --git a/tests/test_marks.py b/tests/test_marks.py index 9134445..4d814ce 100644 --- a/tests/test_marks.py +++ b/tests/test_marks.py @@ -881,3 +881,313 @@ def test_a_corrupt_marks_file_gives_the_browser_a_409_not_a_500(client): # the page still renders, so the operator can see the booth at all assert c.get("/b/b/").status_code == 200 assert c.get("/b/b/marks.json").status_code == 200 + + +# ---- findings from the cross-frontier BUG-HUNT panel, 2026-09-22 ------------- +# +# Heid panel (thread 01M33XEC1H0298C0D968FWBN7A). Four arms, artifact-only, +# diff-scoped. The headline was 4/4 convergent and none of it had a guard: the +# panel's own mutation tables showed the lock lifecycle SURVIVED every existing +# test, because `test_a_no_op_write_does_not_touch_the_booth` asserts only that +# `.marks.json` is absent and never looks at the lock or at the clock the +# sweeper actually reads. + + +def test_the_lock_file_is_never_unlinked(tmp_path): + """The lock must outlive the operation that created it. + + `flock` binds to an INODE, not to a path. Unlinking `.marks.lock` while a + second writer is blocked on it leaves that writer holding an exclusive lock + on a deleted inode — and the next writer along creates a FRESH lock file and + takes it immediately. Two processes then run the read-modify-write + concurrently and the later `os.replace` drops the earlier one's mark, with + no error anywhere. Both of them obeyed the protocol. + + The cleanup existed to keep a no-op from leaving a lock file as its only + trace. That is a tidiness goal, and it bought a lost-update race. + """ + from booth.marks import MARKS_LOCK, set_flag + + booth = tmp_path / "b" + booth.mkdir() + assert set_flag(booth, "ghost.png", False) is None # a no-op + assert (booth / MARKS_LOCK).exists(), "the no-op path unlinked the lock file" + + +def test_a_no_op_does_not_reset_the_ttl_clock(tmp_path): + """The property the no-op guard actually exists for, asserted against the + clock the sweeper reads instead of against one file's absence. + + Creating or removing a directory entry bumps the DIRECTORY's mtime, and + `_newest_mtime` seeds from exactly that. So `touch` + `unlink` of the lock + reset the booth's age to zero while leaving no trace behind — the comment on + the create-only guard reasons about the lock FILE's mtime and misses that + the directory moved underneath it. Repeated, it kept a dead booth alive + forever, which is the precise outcome the guard was written to prevent. + """ + import os + + from booth.app import booth_age_seconds + from booth.marks import delete_mark, set_flag + + booth = tmp_path / "b" + booth.mkdir() + old = 1_000_000_000 + os.utime(booth, (old, old)) + + set_flag(booth, "ghost.png", False) # no-op: never flagged + delete_mark(booth, "nothing") # no-op: no such mark + + age = booth_age_seconds(booth, now=old + 90_000) + assert age > 86_400, f"a no-op reset the TTL clock (age fell to {age:.0f}s)" + + +def test_a_real_mark_still_resets_the_ttl_clock(tmp_path): + """The other half of the same rule, so the fix cannot overshoot into + 'marking is never activity'. Marking IS activity and must reset the clock; + only a write that changes nothing must not.""" + import os + + from booth.app import booth_age_seconds + from booth.marks import set_flag + + booth = tmp_path / "b" + booth.mkdir() + old = 1_000_000_000 + os.utime(booth, (old, old)) + + set_flag(booth, "a.png", True) # a real mark + + assert booth_age_seconds(booth, now=old + 90_000) < 86_400 + + +def test_a_non_string_note_text_does_not_crash_the_read(tmp_path): + """`_clean_text` did `(text or "").replace(...)`, so a stored `text` that is + valid JSON but not a string raised AttributeError out of the READ path. + + That is not a marks bug, it is an INDEX bug: `list_booths` reads every + booth's marks on every page load, so one poisoned file took down `/` and + `/healthz` for all 25 booths. The module's stated posture is that a mark it + cannot read renders as broken, never as a 500. + """ + booth = tmp_path / "b" + booth.mkdir() + (booth / MARKS_FILE).write_text(json.dumps({ + "version": 1, + "marks": [{"id": "n1", "shape": "note", "text": 7, + "created": "2026-09-21T00:00:00+00:00"}], + })) + + marks = marks_for(booth) + assert len(marks) == 1 + assert marks[0].error, "a poisoned note read clean instead of reading broken" + + +def test_a_non_string_created_does_not_crash_the_sort(tmp_path): + """`marks_for` sorts on `(created, id)`. A stored `created` of the wrong type + made that comparison raise TypeError — same blast radius as the note above, + reached through the sort rather than through hydration.""" + booth = tmp_path / "b" + booth.mkdir() + (booth / MARKS_FILE).write_text(json.dumps({ + "version": 1, + "marks": [ + {"id": "a", "shape": "note", "text": "fine", + "created": "2026-09-21T00:00:00+00:00"}, + {"id": "b", "shape": "note", "text": "also fine", "created": 17}, + ], + })) + + marks = marks_for(booth) + assert len(marks) == 2 + # An unreadable mark loses its `created` and so sorts FIRST — the stated + # rule is `("", id)` against `(created, id)`. A mark nobody can read is the + # one that wants looking at, and the alternative is it landing at an + # arbitrary position in the middle of the panel. + assert [m.id for m in marks] == ["b", "a"] + assert marks[0].error and not marks[1].error + + +def test_legacy_import_order_survives_same_second_mtimes(tmp_path): + """ROADMAP states the legacy import's order is `(mtime, name)`. It was + stamping `created` at whole-second resolution, so two sidecars written in + the same second lost the fractional part that distinguished them and + `marks_for`'s `(created, id)` tie-break silently re-sorted them into + alphabetical order — reversing the pair the importer had just ordered. + + Deterministic order is a v1 invariant precisely because the operator refers + to things positionally. An order that is stated and not kept is worse than + one that was never claimed. + """ + import os + + from booth.marks import import_legacy_asks + + booth = tmp_path / "b" + booth.mkdir() + for stem in ("zeta", "alpha"): + (booth / f"{stem}{ASK_SUFFIX}").write_text(json.dumps(_single())) + # Same whole second, different fractions: `zeta` is OLDER and must come first. + os.utime(booth / f"zeta{ASK_SUFFIX}", (1_700_000_000.10, 1_700_000_000.10)) + os.utime(booth / f"alpha{ASK_SUFFIX}", (1_700_000_000.90, 1_700_000_000.90)) + + imported = [m.id for m in import_legacy_asks(booth)] + assert imported == ["zeta", "alpha"], "the importer's own order is wrong" + assert [m.id for m in marks_for(booth)] == imported, ( + "the read path re-sorted what the importer ordered" + ) + + +def test_the_index_survives_a_poisoned_marks_file(client): + """The blast radius, asserted where it actually hurts. + + `list_booths` reads every booth's marks on every index load and `/healthz` + does the same. One hand-edited or foreign-written `.marks.json` therefore + took down the front page for all 25 booths — the single-booth failure the + lenient reader exists to contain, escaping the booth it belongs to. + """ + c, data = client + good = data / "good" + good.mkdir() + _png(good / "a.png") + bad = data / "bad" + bad.mkdir() + (bad / MARKS_FILE).write_text(json.dumps({ + "version": 1, + "marks": [{"id": "n1", "shape": "note", "text": {"oops": True}, "created": 3}], + })) + + assert c.get("/").status_code == 200 + assert c.get("/healthz").status_code == 200 + assert c.get("/b/bad/").status_code == 200 + + +def test_answer_treats_a_non_string_notes_field_as_no_notes(client): + """`booth_note` guards `text` with `isinstance(..., str)`; `booth_answer` + passed `notes` straight to `_clean_notes`, which calls `.replace` on it. A + multipart FILE part named `notes` is a str to nobody, so the route 500'd on + hostile-but-legal input where its sibling handled the same class of value. + + Both routes now read the field the same way: a value that is not text is no + value. The CHOICE is the judgment and it still lands — throwing the whole + answer away over a junk optional field would be the wrong trade.""" + c, data = client + b = data / "b" + b.mkdir() + declare_pick(b, "winner", _single()) + + r = c.post( + "/b/b/answer", + data={"ask": "winner", "choice": "A — baseline"}, + files={"notes": ("n.txt", b"surprise", "text/plain")}, + follow_redirects=False, + ) + assert r.status_code == 303 + mark = next(m for m in marks_for(b) if m.id == "winner") + assert mark.answer["choice"] == "A — baseline" + assert not mark.answer.get("notes") + + +def test_an_inline_doc_tile_offers_a_note_control(client): + """Three item branches, two of them call `marknotes`. The doc branch got the + flag button and not the note field, so the operator could point at a report + and not write down why — on the one item kind whose whole purpose is prose. + + This is the exact failure the `blurtoggle` macro comment names ("patched two + of three"), recurring on the macro that was written to prevent it. + """ + c, data = client + b = data / "b" + b.mkdir() + (b / "report.md").write_text("# report\n\nprose here\n") + + html = c.get("/b/b/").text + assert 'value="report.md"' in html, "the doc tile has no mark controls at all" + # `marknotes`' add-field, which only that macro emits. The booth-level panel + # has its own note form, so the presence of /note on the page proves nothing. + assert 'placeholder="a note on this item"' in html, ( + "an inline doc tile has no way to add a note" + ) + + +def test_the_marks_panel_survives_a_booth_that_also_has_a_link_board(client): + """The board booth renders as a board instead of a gallery, which is right — + but the suppression was unconditional, so a pick declared on a booth that + happens to carry a `links.md` had no form to answer it and no way to say so.""" + c, data = client + b = data / "b" + b.mkdir() + (b / "links.md").write_text("- [a thing](http://example.invalid) · who · when\n") + declare_pick(b, "winner", _single()) + + html = c.get("/b/b/").text + assert "Which render wins?" in html, "a pick on a board booth was unanswerable" + + +def test_the_zoom_view_does_not_navigate_away_from_a_note_being_typed(client): + """The viewer's arrow keys move between images and Escape goes back. The + note textarea landed in the same page, and the handler is on `document`, so + an arrow key meant for the caret threw away the draft instead of moving it. + + Asserted structurally: the handler must bail on events from an editable + target. There is no browser in this suite, and a guard nobody can test is + exactly how this shipped.""" + c, data = client + b = data / "b" + b.mkdir() + _png(b / "a.png") + + js = c.get("/b/b/view?f=a.png").text + assert "isEditable" in js, "the viewer's key handler has no editing guard" + + +@pytest.mark.parametrize("route", ["booth_answer", "booth_note", "booth_flag", + "booth_unmark", "booth_import_asks"]) +def test_mark_writes_do_not_block_the_event_loop(route): + """Every mark write takes a blocking `flock` and does synchronous disk I/O. + In an `async def` handler that runs ON the event loop, so a lock held by + another process — the CLI mid-`marks-import`, a second browser tab — freezes + every other request, including the index and `/healthz`. + + Structural, like `test_stdlib_only`, and for the same reason: the failure is + a property of where the call runs, which no single-process response + assertion can see. The rule is that an async mark-write handler hands the + locked section to a worker thread and never calls the writer inline. + """ + src = pathlib.Path(__file__).parent.parent / "booth" / "app.py" + fn = next( + n for n in ast.walk(ast.parse(src.read_text())) + if isinstance(n, ast.AsyncFunctionDef) and n.name == route + ) + writers = {"answer_pick", "write_note", "set_flag", "delete_mark", + "import_legacy_asks"} + for node in ast.walk(fn): + if not isinstance(node, ast.Call): + continue + name = getattr(node.func, "id", None) or getattr(node.func, "attr", None) + if name in writers: + pytest.fail(f"{route} calls {name}() on the event loop; " + "dispatch it through run_in_threadpool") + + +def test_an_unreadable_mark_is_visible_on_the_page(client): + """Surviving the poisoned file is half of it. A note whose stored `text` is + unreadable hydrates with empty text, and the panel rendered that as an empty + `
` with a withdraw button beside it — which looks exactly like a note
+    the operator wrote and then cleared.
+
+    `_hydrate`'s own docstring forbids this for picks ("a broken question the
+    session believes it posted has to be visible — silently hiding it is the one
+    outcome nobody can debug"). It is the same argument for every shape."""
+    c, data = client
+    b = data / "b"
+    b.mkdir()
+    (b / MARKS_FILE).write_text(json.dumps({
+        "version": 1,
+        "marks": [{"id": "n1", "shape": "note", "text": {"oops": True},
+                   "created": "2026-09-21T00:00:00+00:00"}],
+    }))
+
+    html = c.get("/b/b/").text
+    assert "⚠ broken" in html, "an unreadable mark rendered as an empty note"
+    assert "n1" in html