From 34ac1683ee2b15f915465ead91325cc8991fd5ba Mon Sep 17 00:00:00 2001 From: Vuong Hoang Date: Fri, 25 Sep 2026 10:51:37 -0700 Subject: [PATCH] =?UTF-8?q?memory:=20snapshot=20for=20shutdown=20=E2=80=94?= =?UTF-8?q?=20nothing=20in=20flight;=20the=20r3=20seam=20pass=20and=20the?= =?UTF-8?q?=20upload-name=20lessons=20split=20to=20detail=20files?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...-24-r3-seam-pass-what-only-it-could-see.md | 40 ++++++++++++++ ...upload-names-two-crashes-then-two-holes.md | 55 +++++++++++++++++++ persistent-memory.md | 26 ++++----- 3 files changed, 106 insertions(+), 15 deletions(-) create mode 100644 persistent-memory.d/2026-09-24-r3-seam-pass-what-only-it-could-see.md create mode 100644 persistent-memory.d/2026-09-24-upload-names-two-crashes-then-two-holes.md diff --git a/persistent-memory.d/2026-09-24-r3-seam-pass-what-only-it-could-see.md b/persistent-memory.d/2026-09-24-r3-seam-pass-what-only-it-could-see.md new file mode 100644 index 0000000..08aac35 --- /dev/null +++ b/persistent-memory.d/2026-09-24-r3-seam-pass-what-only-it-could-see.md @@ -0,0 +1,40 @@ +# The r3 seam pass: what only it could see + +_2026-09-24. Contract `docs/contracts/r3_compare.contract.md` (design-dev); +althing thread `01M3952NCDRRJX5XDFSPMSP5HJ`._ + +design-dev's heid contract panel read r3 cold. Our seam pass read it against +the real `app.py`, `items.py`, `base.html` and `view.html`, plus small probes +on a scratch booth. It found 12 mismatches, all folded before a line of code. +The build then went through heid code-review and bug-hunt, and our own gate, +with nothing structural left. design-dev: "S4 and S8 would each have cost a +round." + +## The ones worth remembering, because they recur + +- **`data-region` ids must be unique on a page.** The in-place client's + `swap()` in `base.html` keeps only the FIRST fresh node for each id, then + replaces EVERY live node that has that id with a copy of it. Two per-side + regions sharing an id would make B's flag button a copy of A's after any + save, so pressing B flags A, with nothing to show it happened. Also: do not + key a region by rel (a pair with `a == b` duplicates it) and do not prefix + it `item-` (swap reads a missing `item-*` as a stale tile, not a reload). + This is a mechanical rule that belongs in CLAUDE.md; it is not there yet. +- **Moving code breaks mutation anchors.** 21 of r2c's 24 `view.html` rows + were anchored inside the inline script that r3 moved to `_stage_js.html`, + and `mutation_check.py` fails a row whose anchor is gone. A contract that + moves code must list the re-pointed rows in its "Assertions that change" + table, and gate on the table still falsifying, not only on the suite staying + green. +- **"404 the way view does" was two rules, and neither was view's.** A + missing `f` is a 422 from FastAPI (required `str`), not a 404; declare + `= ""` and 404 by hand. `booth_items` follows symlinks, so a link pointing + outside the booth is IN `review_chain`, while view 404s it through + resolve plus containment. Compare needed the conjunction. +- **Route order.** `/b/{name}/{filepath:path}` is a catch-all; a new + `/b/{name}/` route must register before it. A booth file literally named + `` becomes unreachable (accepted, as for view/marks/asks/embed.json). +- **A resolve done twice is a race.** After the merge, the review route checked + `f` and then resolved the whole ring again, so `cring.index(f)` could raise + (a 500) when a file vanished in between. design-dev's `d54bb04` judges each + rel once per request. diff --git a/persistent-memory.d/2026-09-24-upload-names-two-crashes-then-two-holes.md b/persistent-memory.d/2026-09-24-upload-names-two-crashes-then-two-holes.md new file mode 100644 index 0000000..68781df --- /dev/null +++ b/persistent-memory.d/2026-09-24-upload-names-two-crashes-then-two-holes.md @@ -0,0 +1,55 @@ +# Upload names: two crashes found, then two holes in the fix + +_2026-09-24. Commits `92c774e`, `225ba32`; heid bug-hunt thread `01M3AXG27KQDMA6P3RTAAYMEHP`._ + +## What was wrong + +design-dev's r3 bug hunt (hulda) found that `/upload` returned a 500 for a +multipart filename carrying a NUL: `safe_upload_name` stripped path, dots and +length but not NUL, and `(dest / name).open("wb")` raised ValueError. The +upload's `except Exception` tore the booth down and re-raised. + +Fixing it turned up a second 500 on the same line: the cap was +`base[:200]`, 200 CHARACTERS. NAME_MAX is 255 BYTES, so 200 two-byte +characters (`é`, or any CJK name) raised ENAMETOOLONG. + +## The first fix, and the two holes a single arm found in it + +`92c774e` stripped NUL first, then applied the dot rule, then capped at 200 +UTF-8 bytes via `encode("utf-8", "surrogatepass")[:200].decode("utf-8", +"ignore")`, taking the cut out of the stem so the extension survived. + +Heid's own review of that diff found nothing. hulda, reading the whole snapshot +against the declared invariants, found: + +1. **The surrogate was dropped LAST.** The final `decode("ignore")` removed a + lone surrogate after `lstrip(".")` had already run, so `"\ud800.forever"` + came out as `.forever`, which is the keep marker, and `"\ud800.."` as `..`. + The NUL had been moved first for exactly this reason; the same discipline + was not applied to the other droppable class. It was not reachable over HTTP: + Starlette decodes a multipart filename strictly (utf-8, else latin-1), so it + never yields a lone surrogate. The fix made the helper right by construction + anyway. +2. **A cut could manufacture a kind.** A suffix too long to keep (>16 bytes) + was cut like text, and the cut could land on a shorter suffix that means + something: `"a"*196 + ".png" + "x"*17` became `….png`, an image. +3. The 16-byte extension threshold was unguarded: every test suffix was 4 + bytes, so `<= 4` survived. A `.jpeg` case now pins it. + +`225ba32` does one pass first (NUL and everything unencodable), then basename +and the dot rule, then the byte cap. A cut whose `classify`/`doc_kind` differs +from the original's has its dots replaced with `_`. Falsifiers: +`tests/mutations/upload_names.toml`, 7/7. + +## The lessons + +- **A sanitiser drops everything droppable FIRST, then applies the structural + rules.** Anything dropped after a rule can defeat that rule. +- **A NUL test through httpx `files=` proves nothing.** httpx + percent-escapes the NUL, so the server sees a literal `%00`. Post a raw + multipart body. The first integration test passed pre-fix for this reason. +- **Truncating by length can change a file's meaning.** In the Booth the kind + comes from the extension, so a cut has to preserve the kind, not only the + byte count. +- **The single-arm hunt earned its cost.** Heid's own read traced only the + hunks; hulda read the declared invariants against the whole bundle. diff --git a/persistent-memory.md b/persistent-memory.md index aee59ba..26937d8 100644 --- a/persistent-memory.md +++ b/persistent-memory.md @@ -1,6 +1,6 @@ # Persistent memory — booth -_Last updated: 2026-09-24_ +_Last updated: 2026-09-25_ > **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,8 +17,12 @@ loop it turned out to actually be. ## Current state / in-flight -_As of 2026-09-24:_ +_As of 2026-09-25:_ +- 🟢 **NOTHING IS IN FLIGHT** (shutdown snapshot, 2026-09-25). The tree is + clean, no branch or worktree is open, and no peer is waiting on booth-dev. + main is ONE memory commit ahead of origin (this snapshot), unpushed: the + push is the operator's call. - ✅ **r3 COMPARE IS LIVE AND PUSHED** (operator: "merge r3 once the mutation check is clean", then "merge and push", 2026-09-24). origin/main is the r3 arc's tip: r3 (`f8d136a`), design-dev's race fix (`d54bb04`: one compare ring @@ -29,17 +33,7 @@ _As of 2026-09-24:_ contract caught 12 mismatches before code; the two that mattered were duplicate `data-region` ids (a save would have made B's flag button flag A) and 22 mutation-table rows anchored in the script that moved. -- ⚠ **THE UPLOAD ROUTE HAS THREE KNOWN LIFECYCLE GAPS, unfixed on purpose** - (hulda, 2026-09-24; the operator was told and did not ask for them): - - the pickup-id `mkdir` sits outside the try, so a racing writer that - creates the same id first gives a FileExistsError, a 500; - - `rmtree(ignore_errors=True)` hides its own failure, so a failed cleanup - can leave a half-written booth; - - `except Exception` misses CancelledError, so a cancelled upload skips - cleanup. - All three are rare. The helper next to them, `safe_upload_name`, is done: - unencodable characters go first, the cap is in bytes, and a cut never - manufactures a kind. + → `persistent-memory.d/2026-09-24-r3-seam-pass-what-only-it-could-see.md` - ✅ **Pushed on 09-23 (`d5ead3f`, 888 green).** Landed that night, in order: r2b merge 1 (`b92b002`, reveal all + booth fog), r2b merge 2 (`cce6a20`, Desk row + dates + theme toggle), the Desk sort (`64f6488`), the @@ -91,8 +85,10 @@ _As of 2026-09-24:_ ## Recent decisions -- `[2026-09-24]` ✅ **Upload names: two crashes found, then two holes in the fix** — a NUL and a name over NAME_MAX in BYTES both 500'd /upload. Our first fix then dropped a lone surrogate AFTER the dot rule (`\ud800.forever` came out as the keep marker) and let a cut manufacture `.png`. A single hulda arm found both after heid's own read found nothing. READ BEFORE WRITING A SANITISER: drop everything droppable FIRST, then apply the structural rules. A NUL test through httpx `files=` sends a literal `%00` and proves nothing. -- `[2026-09-24]` ✅ **r3 compare merged on the operator's word, gated by our own run** — fast-forward to `f8d136a` only after 918 green and 148/148 falsifiers on that exact commit, run here rather than taken from design-dev's report. The seam pass is what made the build clean: 12 contract-vs-code mismatches folded before a line was written. +- `[2026-09-24]` ⏸ **The upload route's three lifecycle gaps: DEFERRED** — the pickup-id `mkdir` sits outside the try (a FileExistsError race), `rmtree(ignore_errors=True)` hides its own failure, and `except Exception` misses CancelledError. All three are rare; the operator was told and merged without them. Tracked in `225ba32`'s commit message. +- `[2026-09-24]` ✅ **Upload names: two crashes found, then two holes in the fix** — READ BEFORE WRITING A SANITISER: drop everything droppable FIRST, then apply the structural rules; a NUL test through httpx `files=` proves nothing → `persistent-memory.d/2026-09-24-upload-names-two-crashes-then-two-holes.md` +- `[2026-09-24]` ✅ **The r3 seam pass: what only it could see** — 12 contract-vs-code mismatches folded before code. READ BEFORE A CONTRACT THAT ADDS `data-region`S, MOVES TEMPLATE CODE, OR ADDS A `/b/{name}/` ROUTE → `persistent-memory.d/2026-09-24-r3-seam-pass-what-only-it-could-see.md` +- `[2026-09-24]` ✅ **r3 compare merged and pushed on the operator's word, gated by our own run** — fast-forward only to SHAs we had gated ourselves (`f8d136a`, then `225ba32`), never to design-dev's reported numbers. - `[2026-09-24]` ✅ **r3 compare ruled: pick two, flag the winner** — operator, in design-dev's session. No `booth_items` or marks work; the URL is rel-keyed. The A/same/B pairwise verdict is **PARKED (deferred)**, with our ordered-pair-of-rels note attached. Tracked in design-dev's r3 contract parked entry, althing thread `01M3952NCDRRJX5XDFSPMSP5HJ`. - `[2026-09-23]` ✅ **The Desk's "Everything else" sorts by last UPDATE, not last activity** — the operator chose the simple fix over repairing `.viewed` from the access log. READ BEFORE CHECKING THE LIVE SERVICE → `persistent-memory.d/2026-09-23-desk-sorts-by-last-update.md` - `[2026-09-23]` ✅ **The blur round-trip, and the migration that recreated the bug it fixed** — three rounds, and the third was our own 09-21 marks lesson repeated. READ BEFORE ANY DOTFILE FORMAT CHANGE → `persistent-memory.d/2026-09-23-blur-round-trip-and-the-migration-that-recreated-it.md`