diff --git a/booth/app.py b/booth/app.py index 6ae0623..3c9d8b7 100644 --- a/booth/app.py +++ b/booth/app.py @@ -1204,10 +1204,20 @@ def create_app( return _pick_fragments(name, mark) except Exception as exc: # noqa: BLE001 - deliberate broken = replace(mark, error=f"this question could not be rendered: {exc}") + try: + whole = str(_frag.whole(broken, ask_form_id(mark.id), + quote(name, safe=""))) + except Exception: # noqa: BLE001 - deliberate + # THE HANDLER MUST SURVIVE THE FAILURE IT IS HANDLING. The + # fallback re-rendered through the SAME macro module that had + # just raised, so when `whole` itself was the broken thing this + # guard re-raised and took the report anyway — a guard that + # only works when the failure is somewhere else. Found while + # building a falsifier for the guard: the falsifier tripped it. + # Plain text, escaped by the caller, no macro involved. + whole = "" return {"id": mark.id, "error": broken.error, - "whole": str(_frag.whole(broken, ask_form_id(mark.id), - quote(name, safe=""))), - "submit": "", "questions": []} + "whole": whole, "submit": "", "questions": []} @app.get("/b/{name}/embed.json") def booth_embed_json(name: str): diff --git a/booth/marks.py b/booth/marks.py index c9b0807..04b4bf2 100644 --- a/booth/marks.py +++ b/booth/marks.py @@ -488,6 +488,40 @@ def _hydrate(entry: dict) -> Mark: norm = normalize_ask(decl, mid) except AskError as exc: return Mark(**base, declaration=decl, answer=answer, error=str(exc)) + # THE ANSWER'S SHAPE IS VALIDATED HERE, at the ONE boundary every + # surface crosses — not at the three render sites that happen to draw + # it today, and not defensively in the template, which would hide that + # anything is wrong. + # + # `{"answer": {"answers": [], "notes": ""}}` is well-formed JSON with a + # wrong-shaped value. It passed `_entry_type_error`, passed the + # `isinstance(answer, dict)` check above, and `marks_for` and + # `hold_read` both reported the mark HEALTHY with no read error — and + # then `_ask_inline.html` did `a.answer.answers.get(q.key)`, Jinja asked + # a LIST for `.get`, and the gallery page and the marks page returned + # 500. Measured at 42ea67f, so it predates U3; U3 guarded only its own + # surface with `_safe_fragments` and left these two by scope. + # + # This is the v0.2.2 lesson finished rather than half-done. That outage + # was a file that could not be PARSED and the reader was made lenient; + # this one parses perfectly and breaks one layer further in, at render, + # where no leniency exists. `read_error` was answering a narrower + # question than every caller assumed. + # + # ONLY the multi case is checked, because only the multi case indexes: + # a single-question pick's answer IS the record, with no `answers` key + # to get wrong. Requiring one unconditionally would break every single + # pick, which is the direction a too-eager guard fails in. + if norm["multi"] and isinstance(answer, dict) and \ + not isinstance(answer.get("answers"), dict): + return Mark(**base, declaration=decl, answer=None, + prompt=norm["prompt"], title=norm["title"], + multi=norm["multi"], questions=norm["questions"], + options=norm.get("options", []), + notes_enabled=norm["notes"], notes_label=norm["notes_label"], + error="this pick's answer is stored in a shape the page " + "cannot render; the answer was dropped and the " + "question is unanswered") return Mark( **base, declaration=decl, diff --git a/docs/contracts/u3_declared_embed_seam.contract.md b/docs/contracts/u3_declared_embed_seam.contract.md index 013e8f8..44f49cb 100644 --- a/docs/contracts/u3_declared_embed_seam.contract.md +++ b/docs/contracts/u3_declared_embed_seam.contract.md @@ -436,7 +436,7 @@ arms flagged the staleness themselves. | **BH-2** | **A submit anchor inside the author's own `
` loses ours** — the HTML parser drops a nested form outright. Every control's `form=` then points at nothing, and the code recorded the pick as submitted so the tail added no fallback. The operator fills it in and the button does nothing. | 1 | **Genuine add.** A submit anchor counts as submitted only if the form actually survived (`hasForm`); otherwise the tail supplies one at body level, where no form encloses it. | | **BH-3** | **A broken pick's diagnostic never rendered from a submit-only anchor.** An errored pick's `submit` is empty; mounting that and marking it placed made the tail skip it, so the "broken ask" box vanished from the one surface built to show it. | 3 of 4 | **Genuine add.** A submit anchor for an errored pick is left alone, exactly as an anchor naming no mark is, and the tail mounts the diagnostic. | | **BH-4** | **An author's own element can hijack the chip.** `
` satisfies any id-prefix rule — the hyphen boundary from CR-7 included. | 4 of 4 | **Genuine add, and it supersedes CR-7's fix.** The chip now searches only the elements THIS SCRIPT MOUNTED, which is the identity the deleted `bk-ask--top` anchor used to guarantee, and takes the earliest of those by `compareDocumentPosition`. | -| **BH-5** | **No error boundary around fragment rendering.** A `.marks.json` that is well-formed JSON with a wrong-shaped `answer` hydrates with no error and then raises in the macro. | 1, `needs-repro` | **Genuine add — reproduced before building for it.** `_safe_fragments` returns a per-mark error record, the same leniency `_hydrate_safe` applies one layer down. ⚠ **The gallery and marks pages still 500 on it, and that is PRE-EXISTING** — measured at `42ea67f`. Out of scope here and recorded rather than quietly widened: `persistent-memory.d/2026-09-22-a-wrong-shaped-answer-500s-the-gallery.md`. | +| **BH-5** | **No error boundary around fragment rendering.** A `.marks.json` that is well-formed JSON with a wrong-shaped `answer` hydrates with no error and then raises in the macro. | 1, `needs-repro` | **Genuine add — reproduced before building for it.** `_safe_fragments` returns a per-mark error record, the same leniency `_hydrate_safe` applies one layer down. ⚠ **The gallery and marks pages still 500 on it, and that is PRE-EXISTING** — measured at `42ea67f`. Out of scope here and recorded rather than quietly widened: `persistent-memory.d/2026-09-22-a-wrong-shaped-answer-500s-the-gallery.md`. **CLOSED 2026-09-22**, after U6, at the hydration boundary rather than by a third copy of this guard — so `_safe_fragments` no longer has a reachable natural trigger and is now a pure backstop, falsified synthetically. Hardening the falsifier found that this guard's own fallback re-rendered through the macro module that had just raised, so it re-raised whenever `whole` was the broken thing; fixed in the same pass. | | **BH-6** | Prototype pollution in the placement maps (`toString` as a mark id, `constructor` as a question key). | 1 | **Already fixed this round** as CR-13, from the code-review panel. Two panels, two lenses, the same defect independently — the strongest signal of the evening that the lenses are not redundant. | | **BH-7** | Bare-substring declaration suppresses the chrome. | 4 of 4 | **Already fixed** as CR-3, before the reply landed. | diff --git a/persistent-memory.d/2026-09-22-a-wrong-shaped-answer-500s-the-gallery.md b/persistent-memory.d/2026-09-22-a-wrong-shaped-answer-500s-the-gallery.md index 66ccf13..3a036e4 100644 --- a/persistent-memory.d/2026-09-22-a-wrong-shaped-answer-500s-the-gallery.md +++ b/persistent-memory.d/2026-09-22-a-wrong-shaped-answer-500s-the-gallery.md @@ -1,4 +1,4 @@ -# A wrong-shaped answer 500s the gallery and the marks page — PRE-EXISTING, NOT U3 +# A wrong-shaped answer 500s the gallery and the marks page — CLOSED 2026-09-22 _2026-09-22 · booth_ @@ -72,3 +72,49 @@ whose closing comment points back here. Related: [[2026-09-21-marks-write-wiped-judgment]], [[2026-09-22-lenient-reader-blast-radius]], [[2026-09-22-u3-declared-embed-seam-landed]]. + + +--- + +## CLOSED — 2026-09-22, after U6, at option (1) + +Fixed in `_hydrate`, the option this entry argued for: **one predicate, one +place, every surface inherits it.** The operator was asked three times where the +guard belonged and did not answer; the placement was taken under the stated +assumption, and it is cheap to move if he disagrees — the whole fix is one +condition in one function. + +**Only the MULTI case is checked**, because only the multi case indexes: a +single-question pick's answer IS the record, with no `answers` key to get wrong. +Requiring one unconditionally would break every single pick — the direction a +too-eager guard fails in, and it has its own test. + +Measured before and after, on the gallery booth (no `index.html`): + + before /b/g/ 500 /b/g/marks 500 / 200 /healthz 200 + after /b/g/ 200 /b/g/marks 200 / 200 /healthz 200 + and the error is VISIBLE on the page, and the booth's + OTHER, healthy pick still renders + +**Two things fell out of it that are worth more than the fix.** + +1. **`_safe_fragments` lost its natural trigger.** Probed every wrong answer + shape reachable from a `.marks.json`: `answers` as a list, a string or null + all become hydration errors now, and a wrong-typed VALUE inside `answers` + renders without raising because Jinja absorbs attribute access on a + non-mapping. So U3's guard is now a pure backstop with **no reachable + natural input**. Its test was rewritten to a synthetic trigger that says so — + patching the shared macro module through `app.state.templates` — rather than + left asserting a path nothing reaches. An untested guard and a guard tested + by an unreachable input are the same thing. + +2. **The guard's own handler could not survive the failure it was handling.** + Building that falsifier tripped it: `_safe_fragments` caught a raising + `_pick_fragments` and then rebuilt the broken-ask box **through the same + macro module that had just raised**, so when `whole` itself was broken the + handler re-raised and took the whole report. Fixed, with its own test. Found + by accident, which is the usual way. + +Both new falsifiers were **verified RED against their defeating change** rather +than assumed — the discipline from [[2026-09-22-vacuous-falsifiers]], applied to +the fix for the entry that names it. diff --git a/persistent-memory.md b/persistent-memory.md index 972bf38..2a44423 100644 --- a/persistent-memory.md +++ b/persistent-memory.md @@ -36,12 +36,15 @@ _As of 2026-09-22:_ is `miranda-is` at 92 items. Two of 19 booths have subfolders and both are reports. Sections buy close to nothing; the rail, filters and grid keyboard are the unit. -- ⚠ **ONE DEFECT IS OPEN, AND THE OPERATOR HAS NOW BEEN ASKED TWICE.** A - `.marks.json` that is well-formed JSON with a wrong-shaped `answer` 500s **the - gallery page and the marks page**. Predates U3 (measured at `42ea67f`). No - issue, no tracking surface — untracked by operator choice. Session - recommendation is `_hydrate` (one predicate, one place, every surface - inherits it). Read +- **THE LAST OPEN DEFECT IS CLOSED.** The wrong-shaped `answer` that 500'd the + gallery and marks pages (pre-existing, measured at `42ea67f`) is fixed at + `_hydrate` — the placement the session recommended three times and the + operator never ruled on, **taken under a stated assumption and cheap to move** + (one condition in one function) if he disagrees. Measured before/after: both + pages 500 → 200, error visible, the booth's other pick untouched. Two things + fell out of it that matter more than the fix — U3's `_safe_fragments` lost its + natural trigger and is now a synthetically-falsified backstop, and that guard's + own handler could not survive the failure it was handling. Read `persistent-memory.d/2026-09-22-a-wrong-shaped-answer-500s-the-gallery.md` before touching marks rendering anywhere. - ⚠ **TWO OPERATOR DECISIONS ARE OUTSTANDING AND BOTH ARE DELIBERATELY NOT @@ -75,6 +78,7 @@ _As of 2026-09-22:_ ## Recent decisions +- `[2026-09-22]` **The last open defect closed, and building its falsifier found another** — the wrong-shaped answer fixed at `_hydrate`; `_safe_fragments` lost its natural trigger and its handler could not survive the failure it handled → `persistent-memory.d/2026-09-22-a-wrong-shaped-answer-500s-the-gallery.md` - `[2026-09-22]` **U6 released as `v0.6.0` — benches, and the number that was two defects** — six of seven v1 units landed, NOT PUSHED → `persistent-memory.d/2026-09-22-u6-benches-released.md` - `[2026-09-22]` **Three cold panels on one unit, and what each lens could only see alone** — READ BEFORE DECIDING TO SKIP A GATE; all five passes found something the others structurally could not → `persistent-memory.d/2026-09-22-three-cold-panels-on-one-unit.md` - `[2026-09-22]` **U6 landed — three surfaces, three jobs, one predicate** — the seam review caught three real contract defects incl. a per-row `resolve_booth` that would have 404'd the board → `persistent-memory.d/2026-09-22-u6-benches-landed.md` diff --git a/tests/test_embed.py b/tests/test_embed.py index 6cacdcc..67679bd 100644 --- a/tests/test_embed.py +++ b/tests/test_embed.py @@ -307,18 +307,107 @@ def test_a_wrongly_shaped_answer_costs_its_pick_not_the_report(client): r = c.get("/b/b/embed.json") assert r.status_code == 200 by = {m["id"]: m for m in r.json()["marks"]} - assert by["batch"]["error"] and "could not be rendered" in by["batch"]["error"] + # THE PROMISE, NOT THE LAYER. This used to pin the string + # `_safe_fragments` produces ("could not be rendered"), which made the test + # an assertion about WHICH guard fired. As of the `_hydrate` answer-shape + # check, this input is caught one layer earlier and never reaches + # `_pick_fragments` at all — the endpoint's promise is unchanged and the + # error is better (it names what is wrong with the stored answer instead of + # reporting a render failure), so the assertion moved to the promise. + # `_safe_fragments` is still the backstop and is still falsified, by + # `test_safe_fragments_still_catches_what_hydration_cannot` below. + assert by["batch"]["error"], "a wrong-shaped answer reported no error" assert "broken ask" in by["batch"]["whole"] # and the booth's other pick is untouched — one bad entry costs one entry assert by["healthy"]["error"] is None assert "Which render wins?" in by["healthy"]["whole"] assert c.get("/b/b/").status_code == 200 - # ⚠ THE GALLERY AND MARKS PAGES STILL 500 ON THIS ENTRY, and that is NOT - # U3's doing — measured at 42ea67f, the commit before this unit. They render - # the same macro without this guard. Out of scope here (the gallery is named - # out of scope in the contract) and recorded rather than quietly widened: - # see persistent-memory.d/2026-09-22-a-wrong-shaped-answer-500s-the-gallery.md + # ⚠ THE GALLERY AND MARKS PAGES USED TO 500 ON THIS ENTRY, and that was NOT + # U3's doing — measured at 42ea67f, the commit before that unit. CLOSED + # 2026-09-22 at the hydration boundary rather than by a third copy of this + # guard: see tests/test_marks.py + # ::test_a_wrong_shaped_answer_is_an_error_at_hydration_not_a_500 and + # persistent-memory.d/2026-09-22-a-wrong-shaped-answer-500s-the-gallery.md + + +def test_safe_fragments_still_catches_what_hydration_cannot(client): + """U3's `_safe_fragments` guard, kept falsifiable after `_hydrate` took its + natural trigger away. + + The answer-shape check in `_hydrate` now catches every wrong answer shape + reachable from a `.marks.json` — probed 2026-09-22: `answers` as a list, a + string or null all become hydration errors, and a wrong-typed VALUE inside + `answers` renders without raising, because Jinja absorbs attribute access + on a non-mapping. **No natural input reaches `_safe_fragments` by this + route any more**, and a test that kept pretending one did would assert + nothing — which is the failure this suite has now paid for twice. + + So the trigger is synthetic and says so: the shared `_ask_inline` macro + module is made to raise. `_pick_fragments` resolves `whole` off that object + per call, and `create_app` stashes the environment on `app.state`, so this + reaches the very object the closure captured. What it pins is the guard + itself — one raising pick costs that pick, never the report. + + Defeating change: removing the try/except in `_safe_fragments`, under which + this returns 500. + """ + c, data = client + b = data / "b" + b.mkdir(parents=True, exist_ok=True) + declare_pick(b, "batch", {"prompt": "Which?", "options": ["x", "y"]}) + (b / "index.html").write_text(DECLARED) + + frag = c.app.state.templates.env.get_template("_ask_inline.html").module + real_submit = frag.submit + + def explode(*a, **k): + raise RuntimeError("synthetic render failure") + + object.__setattr__(frag, "submit", explode) + try: + assert frag.submit is explode, "the patch did not take; this test is vacuous" + r = c.get("/b/b/embed.json") + assert r.status_code == 200, "a raising fragment renderer took the whole report" + by = {m["id"]: m for m in r.json()["marks"]} + assert by["batch"]["error"] and "could not be rendered" in by["batch"]["error"] + assert by["batch"]["whole"], "the fallback rendered nothing at all" + finally: + object.__setattr__(frag, "submit", real_submit) + + # and the guard is not sticky — with the macro restored, the pick is fine + assert c.get("/b/b/embed.json").json()["marks"][0]["error"] is None + + +def test_the_handler_survives_the_failure_it_is_handling(client): + """`_safe_fragments` caught a raising `_pick_fragments` and then rebuilt the + broken-ask box THROUGH THE SAME MACRO MODULE that had just raised. So when + `whole` itself was the broken thing, the handler re-raised and took the + whole report — a guard that only worked when the failure was somewhere + else. + + Found by accident: the first draft of the falsifier above patched `whole`, + and the guard failed rather than caught. Defeating change: removing the + inner try/except, under which this returns 500.""" + c, data = client + b = data / "b" + b.mkdir(parents=True, exist_ok=True) + declare_pick(b, "batch", {"prompt": "Which?", "options": ["x", "y"]}) + (b / "index.html").write_text(DECLARED) + + frag = c.app.state.templates.env.get_template("_ask_inline.html").module + real_whole = frag.whole + + def explode(*a, **k): + raise RuntimeError("even the fallback macro is broken") + + object.__setattr__(frag, "whole", explode) + try: + r = c.get("/b/b/embed.json") + assert r.status_code == 200, "the handler re-raised through the broken macro" + assert r.json()["marks"][0]["error"] + finally: + object.__setattr__(frag, "whole", real_whole) def test_no_regex_touches_author_html(): diff --git a/tests/test_marks.py b/tests/test_marks.py index c15d92f..0db2e62 100644 --- a/tests/test_marks.py +++ b/tests/test_marks.py @@ -1374,3 +1374,57 @@ def test_a_clock_restore_that_fails_does_not_take_the_route_down(tmp_path): assert (booth / MARKS_LOCK).exists() assert [m.target for m in marks_for(booth)] == ["a.png"] + + +# ---- the wrong-shaped answer, closed at the hydration boundary -------------- + + +def test_a_wrong_shaped_answer_is_an_error_at_hydration_not_a_500(tmp_path): + """A `.marks.json` that is well-formed JSON with a wrong-shaped `answer` + passed every reader and then raised in the TEMPLATE: `_hydrate` checked only + that `answer` was a dict, never that `answer["answers"]` was one, so + `marks_for` and `hold_read` both reported the mark healthy with no read + error — and `_ask_inline.html` asked a list for `.get`. + + Measured at `42ea67f`, so it predates U3. U3 guarded its own surface with + `_safe_fragments` and left the gallery and marks pages alone by scope. This + closes it at the boundary the rest of the module already argues for: ONE + predicate, ONE place, every surface inherits it. + + Defeating change: restoring the bare `isinstance(answer, dict)` check — + under which `error` is None here and both pages 500. + """ + declare_pick(tmp_path, "batch", {"title": "T", "questions": [ + {"key": "r1", "prompt": "A?", "options": ["x", "y"]}, + {"key": "r2", "prompt": "B?", "options": ["x", "y"]}]}) + raw = json.loads((tmp_path / ".marks.json").read_text()) + for e in raw["marks"]: + if e["id"] == "batch": + e["answer"] = {"answers": [], "notes": ""} + (tmp_path / ".marks.json").write_text(json.dumps(raw)) + + mark = {m.id: m for m in marks_for(tmp_path)}["batch"] + assert mark.error, "a wrong-shaped answer hydrated as healthy" + assert "answer" in mark.error + # AND the mark is not silently emptied — the declaration survives, so the + # operator can still see WHICH question broke rather than a bare error. + assert mark.declaration is not None + + +def test_a_healthy_multi_answer_still_hydrates(tmp_path): + """The other direction, so the guard cannot be satisfied by rejecting + everything. Defeating change: requiring `answers` unconditionally, which + would break every single-question pick.""" + declare_pick(tmp_path, "multi", {"title": "T", "questions": [ + {"key": "r1", "prompt": "A?", "options": ["x", "y"]}]}) + declare_pick(tmp_path, "single", {"prompt": "Which?", "options": ["x", "y"]}) + raw = json.loads((tmp_path / ".marks.json").read_text()) + for e in raw["marks"]: + if e["id"] == "multi": + e["answer"] = {"answers": {"r1": {"choice": "x", "notes": ""}}, "notes": ""} + if e["id"] == "single": + e["answer"] = {"choice": "x", "notes": ""} + (tmp_path / ".marks.json").write_text(json.dumps(raw)) + by = {m.id: m for m in marks_for(tmp_path)} + assert by["multi"].error is None, by["multi"].error + assert by["single"].error is None, by["single"].error