From 5e41108cd3c24cdb742fe691fb0d6759ce466828 Mon Sep 17 00:00:00 2001 From: Vuong Hoang Date: Mon, 21 Sep 2026 23:54:42 -0700 Subject: [PATCH] fix(marks): a write over a damaged mark file was wiping the booth's judgment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects and a missing test, all surfaced by the cross-frontier contract panel dispatched before implementation and triaged after it (heid, four arms, artifact-only, thread 01M33VSNFER4N1554G0Y0VC9C8). v0.2.0 was already tagged and announced to fifteen handles when they landed, which is the argument for running the gate at all. DATA LOSS. `marks_for` is deliberately lenient — an unparseable `.marks.json` reads as "no marks" so a review page still loads. The write path inherited that leniency through the same reader, so one flag click appended a single entry to an empty list and atomically replaced the file: every mark in the booth gone, silently, from a click. Reproduced first, then fixed. The fix is an asymmetry, not a retreat from leniency. Reads stay lenient; writes go strict through `_read_raw_strict`, which distinguishes bytes-present-but- unreadable from absent and valid-but-empty, and raises `MarksCorrupt`. The damaged bytes are left on disk. Routes answer 409 rather than 500 — the service is fine and the request was well-formed, the state on disk is not — and the body says what to do, because the alternative the operator reaches for otherwise is deleting the file, which is the thing being protected. The CLI says it in one line instead of a traceback. A PICK COULD NOT TARGET AN ITEM. `Mark.target` carried one, `marks_for_target` retrieved by it, and the panel already rendered "on " — but `declare_pick` had no parameter for it, so no session could produce one. A question about one artifact is the whole point of the 2026-09-09 inline-placement ruling; the door was simply missing. THE IMPORTER STRANDED AN ANSWER. A stem already present as a mark was skipped wholesale. If a session had re-declared that stem through marks while the operator's choice sat in the legacy sidecar, that choice was lost permanently — reads are forbidden from looking at sidecars. The declaration is still skipped (idempotence holds) but a legacy answer is now adopted when the existing mark is an unanswered pick, and an answer made through marks is never overwritten. INV-3 NAMED A SURFACE NOTHING TESTED. All four arms converged on it: the rule protects gallery tile, zoom view and doc view; the falsifiable check covered one. The doc view was implemented and untested, so shipping it unmarked would have passed. Three tests now, one per surface. The contract carries the full triage, including two findings accepted and NOT closed: INV-2's and INV-5's checks comply in letter — openness can be re-derived without spelling the grepped pattern, and importlib inside a function defeats the AST walk. Both describe a future careless change, and the honest statement is that these checks raise the cost of drifting rather than making it impossible. Recorded rather than papered over. Also pins the three prose ambiguities the panel found, normatively and once each: what counts as open, the three distinct broken-declaration cases, and INV-6, which had named a helper that does not exist and forbidden the calls that helper must make. 253 tests. --- README.md | 16 +++ ROADMAP.md | 2 +- booth/app.py | 20 ++++ booth/marks.py | 91 +++++++++++++-- docs/contracts/u2_marks.contract.md | 71 +++++++++++- pyproject.toml | 2 +- scripts/booth | 4 +- tests/test_marks.py | 168 ++++++++++++++++++++++++++++ 8 files changed, 359 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 5333f27..9cbaeab 100644 --- a/README.md +++ b/README.md @@ -258,6 +258,14 @@ declare_pick(booth, "batch", { # "unanswered", "complete", "notes", "answered_at", "answered_by"} ``` +**A pick can be about ONE item, not just the booth.** Pass `target` — an item's +booth-relative path — and the question renders beside that artifact: + +```python +declare_pick(booth, "which-crop", {"prompt": "Which crop?", "options": ["tight", "wide"]}, + target="v3/DSC03389.jpg") +``` + **A partial answer is recorded, not refused.** A question left blank is a deliberate outcome — "none of these", "not yet", "ask me later" — so it lands in `unanswered`, stays absent from `answers` unless it carried a note, and @@ -272,6 +280,14 @@ files (`.ask.json` / `.answer.json`) are imported, never deleted: booth marks-import r18-ab # idempotent; the sidecars stay on disk ``` +If the stem is already a mark the declaration is skipped, but a legacy answer +still gets adopted, so the operator's recorded choice is never stranded on disk. + +**If a booth's `.marks.json` is damaged**, reads degrade to "no marks" so the page +still loads, and every WRITE refuses with a 409 rather than replacing the file — +which would otherwise wipe every judgment in that booth. Repair or move the file +by hand; nothing deletes it for you. + ## Upload for pickup The reverse direction — put files in through the web, pick them up by id: diff --git a/ROADMAP.md b/ROADMAP.md index c4d3365..d460bf7 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1,7 +1,7 @@ # The Booth — roadmap Design: [`docs/design/information-architecture.md`](docs/design/information-architecture.md). -Current version: `0.2.0` (U1 + U2 landed; extracted from eshpfi 2026-09-21). +Current version: `0.2.1` (U1 + U2 landed; extracted from eshpfi 2026-09-21). ## v1 target diff --git a/booth/app.py b/booth/app.py index 2c99508..95f7c5a 100644 --- a/booth/app.py +++ b/booth/app.py @@ -123,6 +123,7 @@ from booth.asks import ( # noqa: E402 ) from booth.marks import ( # noqa: E402 MARKS_FILE, + MarksCorrupt, answer_pick, as_dict, declare_pick, @@ -578,6 +579,25 @@ def create_app( # test needs a handle on the env that the app actually renders with. app.state.templates = templates + @app.exception_handler(MarksCorrupt) + async def _marks_corrupt(request: Request, exc: MarksCorrupt): + """A write was refused because the booth's mark file is damaged. + + 409, not 500: the service is fine and the request was well-formed — the + state on disk is not, and the refusal is deliberate. Says what to do, + because the alternative the operator will otherwise reach for is + deleting the file, which is the thing being protected. + """ + return JSONResponse( + status_code=409, + content={ + "error": "this booth's .marks.json cannot be read, so nothing was written", + "detail": str(exc), + "why": "writing would replace every mark in the booth with just this one", + "fix": "repair or move the file by hand; the marks panel still renders as empty", + }, + ) + ttl_display = int(ttl_hours) if float(ttl_hours).is_integer() else ttl_hours base_ctx = { "ttl_hours": ttl_display, diff --git a/booth/marks.py b/booth/marks.py index 1f29338..380a0c0 100644 --- a/booth/marks.py +++ b/booth/marks.py @@ -59,6 +59,20 @@ from booth.asks import ( valid_stem, ) +class MarksCorrupt(RuntimeError): + """The mark file exists but cannot be parsed, and a WRITE was attempted. + + The read path is deliberately lenient — `marks_for` returns [] so a review + page still loads. The write path must not inherit that leniency: reading a + damaged file as "no marks" and then atomically replacing it destroys every + judgment in the booth from one click, silently. Shipped in v0.2.0 and found + by a cross-frontier contract panel, not by the suite. + + A page that renders without an annotation is recoverable. A file that + overwrote the operator's judgment is not. + """ + + MARKS_FILE = ".marks.json" MARKS_LOCK = ".marks.lock" SCHEMA_VERSION = 1 @@ -176,6 +190,34 @@ def _fingerprint(entries: list[dict]) -> str: return json.dumps(entries, sort_keys=True, ensure_ascii=False) +def _read_raw_strict(booth: Path) -> list[dict]: + """Like `_read_raw`, but RAISES `MarksCorrupt` on a file it cannot parse. + + Absent, empty and valid-but-empty are all "no marks yet" and are fine — the + distinction that matters is bytes-present-but-unreadable, because that is the + case where writing would destroy something. + """ + path = Path(booth) / MARKS_FILE + try: + text = path.read_text(encoding="utf-8") + except FileNotFoundError: + return [] + except (OSError, UnicodeDecodeError) as exc: + raise MarksCorrupt(f"{path} cannot be read: {exc}") from exc + if not text.strip(): + return [] + try: + raw = json.loads(text) + except ValueError as exc: + raise MarksCorrupt(f"{path} is not valid JSON: {exc}") from exc + if not isinstance(raw, dict) or not isinstance(raw.get("marks"), list): + raise MarksCorrupt(f"{path} is not a marks document") + entries = [e for e in raw["marks"] if isinstance(e, dict) and isinstance(e.get("id"), str)] + if len(entries) != len(raw["marks"]): + raise MarksCorrupt(f"{path} holds entries this version cannot read") + return entries + + 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 @@ -215,7 +257,16 @@ class _Locked: self._made_lock = True self._lf = lock.open("r+") fcntl.flock(self._lf, fcntl.LOCK_EX) - self.entries = _read_raw(self.booth) + try: + # STRICT here, lenient in marks_for — see MarksCorrupt. + self.entries = _read_raw_strict(self.booth) + except MarksCorrupt: + 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 @@ -357,16 +408,25 @@ def as_dict(mark: Mark) -> dict: # ---- write ------------------------------------------------------------------ -def declare_pick(booth: Path, mark_id: str, doc: dict) -> Mark: - """A session poses a pick. +def declare_pick(booth: Path, mark_id: str, doc: dict, target: str | None = None) -> Mark: + """A session poses a pick, about the booth or about ONE item in it. Validated through `normalize_ask` BEFORE anything is written, so a session cannot land a question the renderer would refuse. Re-declaring an existing id replaces the declaration and CLEARS its answer: the question changed, so - the old judgment is not an answer to it. + the old judgment is not an answer to it — and it may move the target, since + a re-declaration is a new question. + + `target` is an `Item.rel`, or None for the booth. It exists because the + 2026-09-09 ruling is that a question belongs WITH the artifact it is about: a + four-voice audition wants the radio group under that voice. The record and + the renderer both supported it before this parameter did, which meant a + session could not actually produce one. """ if not valid_stem(mark_id): raise AskError("bad mark id: letters, digits, . _ - only") + if not _valid_target(target): + raise AskError("a pick's target must be a path inside the booth") normalize_ask(doc, mark_id) # raises AskError; nothing written yet with _Locked(booth) as lk: existing = lk.find(mark_id) @@ -375,7 +435,7 @@ def declare_pick(booth: Path, mark_id: str, doc: dict) -> Mark: entry = { "id": mark_id, "shape": PICK, - "target": existing.get("target") if existing else None, + "target": target, "created": existing.get("created") if existing else now_stamp(), "declaration": doc, "answer": None, @@ -551,10 +611,8 @@ def import_legacy_asks(booth: Path) -> list[Mark]: created: list[dict] = [] with _Locked(booth) as lk: - have = {e.get("id") for e in lk.entries} + by_id = {e.get("id"): e for e in lk.entries} for mtime, stem, decl, err in found: - if stem in have: - continue answer = None ap = booth / f"{stem}{ANSWER_SUFFIX}" try: @@ -563,6 +621,23 @@ def import_legacy_asks(booth: Path) -> list[Mark]: answer = loaded except (OSError, ValueError, UnicodeDecodeError): pass + + prior = by_id.get(stem) + if prior is not None: + # The stem is already a mark, so the DECLARATION is not imported + # — that is the idempotence rule, and a mark declared since the + # sidecar outranks it. But a legacy ANSWER must not be stranded: + # if the existing mark is an unanswered pick and the sidecar + # holds the operator's choice, adopt it. Ordinary reads are + # forbidden from looking at sidecars, so a skip here would lose + # that judgment permanently. + if (answer is not None + and prior.get("shape") == PICK + and prior.get("answer") is None): + prior["answer"] = answer + created.append(prior) + continue + entry = { "id": stem, "shape": PICK, diff --git a/docs/contracts/u2_marks.contract.md b/docs/contracts/u2_marks.contract.md index 70e141f..5a33c63 100644 --- a/docs/contracts/u2_marks.contract.md +++ b/docs/contracts/u2_marks.contract.md @@ -208,10 +208,13 @@ def import_legacy_asks(booth: Path) -> list[Mark]: `asks.html:11` — and three in Python — `app.py:271` (the index badge), `app.py:750` and `app.py:751` (the verbatim-booth chip). - **INV-3 — the judgment travels, like the caption.** U1's rule, extended: - every surface that renders an item renders that item's marks. Gallery tile, - zoom view, doc view. *Falsifiable:* fetch `/b//view?f=` for a flagged - item carrying a note and assert both the flag state and the note text are in - the served HTML. + every surface that renders an item renders that item's marks. *Falsifiable, + once per surface* — the first draft named three surfaces and checked one, which + all four panel arms flagged as the document's strongest ambiguity: (a) the + **gallery tile** shows the flag control in its current state and the item's + notes; (b) the **zoom view** `/b//view?f=` carries the flag state and + the note text; (c) the **doc view** `/b//view?f=` carries the note + text. Three tests, not one. - **INV-4 — the pick semantics are byte-identical.** `build_answer` produces, for every input, the document `write_answer` produced. *Falsifiable:* the existing `test_asks.py` answer assertions pass against `build_answer` with @@ -275,6 +278,66 @@ out here because it is a visible change to what the index shows, it is the kind of thing that looks like a bug when it lands, and the operator should get to veto it rather than discover it. +## Cross-frontier contract panel — 2026-09-22, four arms, artifact-only + +`/heid-contract-review` panel (Gróa / Hulda / Regin / Kimi), thread +`01M33VSNFER4N1554G0Y0VC9C8`, dispatched before implementation and triaged after +it. Every quoted passage was verified verbatim by Heid; no arm fabricated an +identifier. Triaged per the five-category rule — what follows is the disposition, +not the reply. + +**Three of these were defects in shipped code, not ambiguities in prose.** v0.2.0 +was already tagged and announced to 15 handles when they landed. + +| finding | arms | category | disposition | +|---|---|---|---| +| **A write over a corrupt `.marks.json` silently replaced every mark in the booth.** The read path is deliberately lenient (unparseable → `[]` so the page loads); the write path inherited that through the same reader, so one flag click appended to an empty list and atomically replaced the file. | Kimi F2, Hulda F3 | **1 — genuine add** | **FIXED.** `MarksCorrupt`, raised by a strict `_read_raw_strict` used only by the write path. Read stays lenient, write goes strict; the damaged bytes are left on disk. Routes return 409, not 500. Reproduced first, then fixed. | +| **`declare_pick` had no `target`**, so a pick could not be attached to an item — though `Mark.target` carried one, `marks_for_target` retrieved it, and `_marks.html` already rendered "on \". | Hulda F1, Regin | **1 — genuine add** | **FIXED.** `declare_pick(..., target=None)`, validated like every other target. A re-declaration may move it. | +| **The importer stranded a legacy answer.** A stem already present as a mark was skipped wholesale, so a re-declared-but-unanswered pick with the operator's choice sitting in `.answer.json` lost that choice permanently — reads are forbidden from looking at sidecars. | Gróa F10 | **1 — genuine add** | **FIXED.** The declaration is still skipped (idempotence), but a legacy answer is ADOPTED when the existing mark is an unanswered pick. An answer made through marks is never overwritten. | +| **INV-3 names "doc view" as a protected surface; nothing tested it.** Shipping the doc view unmarked would have passed. | 4/4 — the panel's strongest convergence | **1 — genuine add** | **TEST ADDED.** The behaviour was already implemented; the gate caught that nothing held it. INV-3's falsifiable below now covers all three surfaces. | +| **The broken-declaration path is three different doors and none is written:** validate-before-write, stored-raw-with-read-time-error, and unparseable-file-yields-`[]`. | 4/4 | **1 — genuine add, prose only** | **PINNED below.** All three are real and distinct cases; the code always handled them separately. The contract conflated them. | +| **"What counts as open" is defined three ways** across assumptions, the signature comment and a test row. | Gróa F1, Regin F5, Hulda F4 | **1 — genuine add, prose only** | **PINNED below.** Code and tests were already correct (partial = open). | +| **INV-2 and INV-5's checks comply in letter:** openness can be re-derived as `(answer or {}).get("complete")` with the grep still green; `importlib` inside a function defeats the AST walk. | Gróa F5/F7, Kimi F4/F5 | **4 — out of place** | Accepted as true and NOT closed. Both describe a future careless change, and the honest statement is that these checks raise the cost of drifting rather than making it impossible. Recorded rather than papered over. | +| **INV-6 named `_with_marks(booth)`; the code has `_Locked`.** And its falsifiable makes the mandated helper unimplementable, since the helper must itself call `os.replace`. | Gróa F8, Kimi F8 | **2 — sharpening** | **FIXED below** — the name and the exemption. | +| `set_flag`'s annotation forbids a booth-level flag; never stated as a decision. | Regin F6 | **2 — sharpening** | It IS a decision: a flag means *this one*, so it needs an item. Stated in the signature. | +| "Cleaning" note text is defined by example only. | Kimi F6, Hulda F6 | **2 — sharpening** | `_clean_text` is CRLF-normalize, strip, truncate at `TEXT_MAX`. Documented at the function. | +| INV-1 self-conflict: the rule allows one function, the check and assumptions exempt `booth_items`' name check. | Gróa F6 | **3 — settled prior** | Already resolved by the seam review (SR-9): the exemption is a NAME check, never a content read. | + +**The methodology note the arms volunteered, which is worth more than any single +flag:** this contract's own frontmatter carries a plain-language narrative, so the +paraphrase half was partly re-reading the author's framing back to him. Regin and +Kimi both said the stronger shape for a narrative-heavy contract is the ambiguity +pass with the paraphrase cut to a drift-check. That is a finding about the +*mechanism*, not this document, and it belongs in the skill rather than here. + +### The three pinnings + +**Openness, normatively, once.** A mark is open when `shape == "pick"` **and** it +has no `error` **and** (`answer is None` **or** `answer["complete"]` is false). A +**partially answered pick is OPEN.** Every other sentence in this document about +openness is descriptive; this one governs, and `open_marks` is its only +implementation. + +**A broken declaration, normatively — three distinct cases, not one.** + +1. `declare_pick` validates through `normalize_ask` and **raises `AskError` + before writing anything.** A session cannot land a refused question. The + function never returns an invalid mark. +2. A declaration that is invalid **in the stored file** — reachable via the + importer, or a hand-edit — is hydrated with `error` set and is rendered, so a + question the session believes it posted is never silently hidden. It is not + open (it can never be answered), and it cannot be answered: `answer_pick` + re-validates and raises. +3. A **whole file** that cannot be parsed is not a broken declaration. `marks_for` + returns `[]` so the page loads; every WRITE refuses with `MarksCorrupt`. + +**INV-6, corrected.** Every writer goes through the one `_Locked(booth)` context +manager, which holds an exclusive flock on `/.marks.lock` across read, +mutate and atomic replace. *Falsifiable:* no function outside `_Locked` calls +`_write_raw` or `os.replace` on the mark file. (The first draft named a +`_with_marks` helper that does not exist, and forbade the very calls the helper +must make.) + ## Slices Vertical, each one shippable and green before the next starts. diff --git a/pyproject.toml b/pyproject.toml index edaac27..c154943 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "booth" -version = "0.2.0" +version = "0.2.1" 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 373b11f..97c3f18 100755 --- a/scripts/booth +++ b/scripts/booth @@ -255,7 +255,7 @@ print("removed: %s %s" % (removed["desc"], removed["url"])) import os, pathlib, sys sys.path.insert(0, os.environ["BOOTH_SRC"]) from booth.asks import AskError -from booth.marks import declare_pick +from booth.marks import MarksCorrupt, declare_pick booth, mid, prompt, *opts = sys.argv[1:] try: declare_pick(pathlib.Path(booth), mid, @@ -263,6 +263,8 @@ try: "notes": os.environ["ASK_NOTES"] == "1"}) except AskError as exc: sys.exit("bad pick: %s" % exc) +except MarksCorrupt as exc: + sys.exit("this booth'"'"'s .marks.json is damaged, so nothing was written: %s" % exc) ' "$DATA/$name" "$mid" "$prompt" "${opts[@]}" echo "$URL/b/$name/#mark-$mid" ;; diff --git a/tests/test_marks.py b/tests/test_marks.py index d5232b2..9134445 100644 --- a/tests/test_marks.py +++ b/tests/test_marks.py @@ -713,3 +713,171 @@ def test_a_real_write_then_a_no_op_leaves_the_file_alone(tmp_path): set_flag(booth, "a.png", True) # idempotent: already flagged assert path.stat().st_mtime == before, "an idempotent flag rewrote the file" + + +# ---- findings from the cross-frontier contract panel, 2026-09-22 ------------- +# +# Heid panel (thread 01M33VSNFER4N1554G0Y0VC9C8). Four arms, artifact-only. + + +def test_a_write_over_a_corrupt_marks_file_refuses_instead_of_replacing(tmp_path): + """DATA LOSS, shipped in v0.2.0. Found by Kimi (flag 2), converged with Hulda. + + `marks_for` is deliberately lenient — an unparseable file reads as "no marks" + so a review page still loads. The write path inherited that leniency through + the same reader, so the next flag toggle appended one entry to an empty list + and atomically replaced the file: every judgment in that booth gone, from one + click, silently. + + The read stays lenient and the WRITE goes strict. That asymmetry is the fix — + a page that renders without an annotation is recoverable, a file that + overwrote the operator's judgment is not, and this repo's standing rule is + that nothing deletes his data. + """ + from booth.marks import MarksCorrupt, set_flag, write_note + + booth = tmp_path / "b" + booth.mkdir() + write_note(booth, "a.png", "judgment one") + write_note(booth, "b.png", "judgment two") + raw = (booth / MARKS_FILE).read_text() + (booth / MARKS_FILE).write_text(raw[: len(raw) // 2]) # truncated mid-write + + with pytest.raises(MarksCorrupt): + set_flag(booth, "c.png", True) + + # The damaged bytes are still on disk — untouched, recoverable by hand. + assert (booth / MARKS_FILE).read_text() == raw[: len(raw) // 2] + # And the read path is still lenient, so the page renders rather than 500s. + assert marks_for(booth) == [] + + +def test_an_absent_or_empty_marks_file_is_not_corrupt(tmp_path): + """The strict write path must not mistake "nothing yet" for "damaged".""" + from booth.marks import set_flag + + booth = tmp_path / "b" + booth.mkdir() + assert set_flag(booth, "a.png", True) is not None # no file at all + (booth / MARKS_FILE).write_text("") + assert set_flag(booth, "b.png", True) is not None # zero bytes + (booth / MARKS_FILE).write_text('{"version": 1, "marks": []}') + assert set_flag(booth, "c.png", True) is not None # valid but empty + + +def test_a_pick_can_target_one_item(tmp_path): + """Found by Hulda (flag 1), converged with Regin. + + `Mark.target` carries an item rel, `marks_for_target` retrieves by it, and + the panel template already renders "on " for a pick — but + `declare_pick` had no target parameter, so a session could not actually + produce one. A question about ONE artifact is the 2026-09-09 ruling's whole + point; the record supported it and the door was missing. + """ + from booth.marks import marks_for_target + + booth = tmp_path / "b" + booth.mkdir() + declare_pick(booth, "which-crop", _single(), target="v3/DSC03389.jpg") + m = marks_for(booth)[0] + assert m.target == "v3/DSC03389.jpg" + assert [x.id for x in marks_for_target(marks_for(booth), "v3/DSC03389.jpg")] == ["which-crop"] + # and it still answers normally + answer_pick(booth, "which-crop", "A — baseline") + assert marks_for(booth)[0].answer["complete"] is True + + +def test_a_pick_target_cannot_escape_the_booth(tmp_path): + booth = tmp_path / "b" + booth.mkdir() + for bad in ("../outside.png", "/etc/passwd"): + with pytest.raises(AskError): + declare_pick(booth, "p", _single(), target=bad) + + +def test_redeclaring_a_pick_may_move_its_target(tmp_path): + booth = tmp_path / "b" + booth.mkdir() + declare_pick(booth, "p", _single(), target="a.png") + declare_pick(booth, "p", _single(), target="b.png") + assert marks_for(booth)[0].target == "b.png" + + +def test_import_adopts_a_legacy_answer_for_an_already_declared_pick(tmp_path): + """Found by Gróa (flag 10). + + The idempotence rule skipped any stem already present as a mark. If a + session had re-declared that stem through marks (so the mark exists, still + unanswered) while the operator's answer sat in the legacy sidecar, the import + skipped and that answer was stranded on disk forever — with the read path + forbidden from looking at sidecars. Adopting the answer preserves both rules: + idempotent, and never clobbers a NEWER judgment. + """ + from booth.asks import ANSWER_SUFFIX, build_answer, normalize_ask + from booth.marks import import_legacy_asks + + booth = tmp_path / "b" + _sidecar(booth, "winner", _single()) + doc = build_answer(normalize_ask(_single(), "winner"), "B — async", notes="from the sidecar") + (booth / f"winner{ANSWER_SUFFIX}").write_text(json.dumps(doc)) + declare_pick(booth, "winner", _single()) # re-declared, unanswered + assert marks_for(booth)[0].answer is None + + import_legacy_asks(booth) + got = marks_for(booth)[0] + assert got.answer is not None, "the legacy answer was stranded" + assert got.answer["choice"] == "B — async" + assert open_marks(marks_for(booth)) == [] + + +def test_import_never_overwrites_an_answer_made_through_marks(tmp_path): + """The other half of the same rule: a judgment recorded SINCE the sidecar + outranks it, and adoption must not reach back over it.""" + from booth.asks import ANSWER_SUFFIX, build_answer, normalize_ask + from booth.marks import import_legacy_asks + + booth = tmp_path / "b" + _sidecar(booth, "winner", _single()) + old = build_answer(normalize_ask(_single(), "winner"), "A — baseline") + (booth / f"winner{ANSWER_SUFFIX}").write_text(json.dumps(old)) + declare_pick(booth, "winner", _single()) + answer_pick(booth, "winner", "B — async") # the operator changed his mind + + import_legacy_asks(booth) + assert marks_for(booth)[0].answer["choice"] == "B — async" + + +def test_the_doc_view_carries_the_marks(client): + """INV-3's third surface — flagged 4/4 by the panel as named in the rule but + covered by no test, so shipping it unmarked would have passed.""" + from booth.marks import write_note + + c, data = client + b = data / "b" + b.mkdir() + (b / "notes.md").write_text("# report\n\nprose here\n") + write_note(b, "notes.md", "this section is wrong") + + html = c.get("/b/b/view?f=notes.md").text + assert "prose here" in html + assert "this section is wrong" in html + + +def test_a_corrupt_marks_file_gives_the_browser_a_409_not_a_500(client): + """The request was fine and the service is fine — the state on disk is not, + and the refusal is deliberate. A 500 would read as "the Booth is broken" and + send the operator looking for something to restart.""" + c, data = client + b = data / "b" + b.mkdir() + from booth.marks import write_note + write_note(b, "a.png", "keep me") + (b / MARKS_FILE).write_text("{truncated") + + r = c.post("/b/b/flag", data={"target": "a.png", "on": "1"}, follow_redirects=False) + assert r.status_code == 409 + body = r.json() + assert "cannot be read" in body["error"] and body["fix"] + # 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