diff --git a/booth/app.py b/booth/app.py index 918e07d..b29fc6b 100644 --- a/booth/app.py +++ b/booth/app.py @@ -932,6 +932,14 @@ def create_app( # every other booth, so this pair is read only when it renders. # `read_benches` never raises; a damaged registry costs its own # panel and says so, which is the v0.2.2 lesson. + # `is_board` is PAGE IDENTITY, not page content. Gating the + # panel on `board or benches` hid it — and its registration + # form — exactly when the board was empty and the registry + # absent, which is the state a new deployment starts in and the + # one where "no benches registered yet" is most worth saying. + # A panel that disappears when it has nothing to show is the + # same defect as a damaged panel rendering as an absent one. + "is_board": (booth / LINKS_FILE).is_file(), **dict(zip(("benches", "benches_error"), read_benches(data_dir) if (booth / LINKS_FILE).is_file() else ([], None))), diff --git a/booth/benches.py b/booth/benches.py index a06a34f..bb8fc8e 100644 --- a/booth/benches.py +++ b/booth/benches.py @@ -136,6 +136,14 @@ def normalize_bench_url(url: str) -> str: if not host: raise ValueError("that URL has no host") + # RE-WRAP A BRACKETED IPv6 LITERAL. `urlsplit().hostname` strips the + # brackets, and rebuilding the netloc from it produces `http://::1:8080/a` + # — not a different spelling of the same URL but a BROKEN one, so a re-post + # never matches the row the operator thinks they are updating. The bracket + # is part of the authority's syntax, not decoration. Detected by the colon, + # which cannot appear in a hostname or an IPv4 literal. + if ":" in host: + host = f"[{host}]" default = {"http": 80, "https": 443}[scheme] netloc = host if port in (None, default) else f"{host}:{port}" # A bare "/" is the same resource as no path at all; a trailing slash on a @@ -240,6 +248,12 @@ def read_benches(root: Path) -> tuple[list[Bench], str | None]: return [], str(exc) except OSError as exc: return [], f"registry could not be read: {exc}" + except RecursionError: + # Deeply nested JSON (`[[[[...`) blows the stack inside json.loads, and + # RecursionError is neither ValueError nor OSError — so it escaped the + # pair above and 500'd the page this function exists to protect. The + # byte cap does not help: 200k open brackets is 200 KB. + return [], "registry is nested too deeply to parse" def _write_all(root: Path, benches: dict[str, Bench]) -> None: diff --git a/booth/templates/base.html b/booth/templates/base.html index 737544d..6a1c448 100644 --- a/booth/templates/base.html +++ b/booth/templates/base.html @@ -511,6 +511,7 @@ .bench-row.is-promoted .bench-state{background:rgba(130,170,240,.18)} .bench-main{flex:1;min-width:0} .bench-url{font-size:.78em;opacity:.55;overflow:hidden;text-overflow:ellipsis;white-space:nowrap} + .bench-meta{display:flex;flex-direction:column;align-items:flex-end;font-size:.75em;opacity:.6} .bench-acts{display:flex;gap:.3rem} .bench-to,.bench-rm{font-size:.75em;padding:.15rem .4rem;cursor:pointer} .bench-add{display:flex;gap:.4rem;padding:.5rem .7rem;border-top:1px solid var(--line,#2a2a2a)} diff --git a/booth/templates/booth.html b/booth/templates/booth.html index 0060efc..f4f76f0 100644 --- a/booth/templates/booth.html +++ b/booth/templates/booth.html @@ -66,7 +66,7 @@ {% else %}

{{ name }}

{% endif %} - {% if uploaded %}⬆ pickup {% endif %}{% if board or benches or benches_error %} + {% if uploaded %}⬆ pickup {% endif %}{% if is_board %} {# THE BENCH REGISTRY. A bench is a running thing — jackdaw's current bench, talk's current bench, the things that get promoted to Homepage when they are fully deployed. NOT a booth (a booth announces itself and is swept) and @@ -102,6 +102,10 @@
{% if b.owner %}{{ b.owner }}{% endif %} + {# The date it was REGISTERED, not the date it was last touched: `added` + survives re-registration and `updated` does not, so `added` is the + one that answers "how long has this been around". #} + {% if b.added %}{{ b.added[:10] }}{% endif %}
diff --git a/docs/contracts/u6_benches.contract.md b/docs/contracts/u6_benches.contract.md index e08dfa5..b133892 100644 --- a/docs/contracts/u6_benches.contract.md +++ b/docs/contracts/u6_benches.contract.md @@ -4,7 +4,7 @@ module: "booth.benches" purpose: "A bench is a running thing, registered -- not a booth, and not a bookmark. The standing link board absorbed all three jobs because only one of them had a surface, and it now carries 221 rows of which 178 (80%) are booth announcements and 156 (71% of the whole board) point at booths that were swept. U5 gave the booth announcement a home; this unit gives the RUNNING SERVICE one, and closes the loop by refusing the one shape that now has somewhere better to go. Identity is the normalized URL, so re-announcing a bench UPDATES its row instead of appending a fifth -- `talk` is on the board five times and Peedlar's root three. Nothing on the board is deleted by this unit: the dead rows are MARKED so the operator can see and remove them with the bulk control that already exists." depends_on: - "booth.links (`booth_target` is DEFINED here and consumed there -- see INV-2. The board's existing parse/remove/pin machinery is untouched: rows keep their content-hash identity, `links.md` stays an O_APPEND multi-writer log, and no row is rewritten by anything this unit adds.)" - - "booth.app (the dead-row marker needs a booth-exists predicate. IT CANNOT USE `resolve_booth`: that is a CLOSURE inside `create_app` (app.py:798), not importable, and it RAISES HTTPException(404) -- calling it per row would turn one swept booth into a 404 for the whole board page, which is the opposite of the marker's purpose. The marker gets its own non-raising predicate carrying the SAME name-safety rules (no leading dot, no separator, no `..`) and returning False where `resolve_booth` raises. A row is dead when its target directory is absent, not when its target is nearly expired -- no new lifetime arithmetic. Verified against the real function, not assumed: seam review SR-2.)" + - "booth.app (the dead-row marker needs a booth-exists predicate. IT CANNOT USE `resolve_booth`: that is a CLOSURE inside `create_app`, not importable, and it RAISES HTTPException(404) -- calling it per row would turn one swept booth into a 404 for the whole board page, which is the opposite of the marker's purpose. The marker gets its own non-raising predicate carrying the SAME name-safety rules (no leading dot, no separator, no `..`) and returning False where `resolve_booth` raises. A row is dead when its target directory is absent, not when its target is nearly expired -- no new lifetime arithmetic. Verified against the real function, not assumed: seam review SR-2.)" language: "python" complexity: "medium" estimated_loc: 320 @@ -117,9 +117,23 @@ and got a 404" and is never traced back here. U5's `created`, and for the same reason: an upsert is the same bench saying something new about itself, not a new bench. -Caps, applied at the write and again at the read: `name` 120, `owner` 64, `id` -and `url` 2048, `state` one of three. Each is a display budget, not a storage -limit. +**`updated` means the last MUTATION of the record, not the last upsert** — +`set_bench_state` bumps it too. Amended after the cold panel read "most recent +upsert" literally and found the code bumping on a state change: the code is +right (a promotion is a change to the record and "last touched" should say so) +and the earlier wording was narrower than what anyone wants the field to mean. + +Caps: `name` 120, `owner` 64, `url` 2048, `state` one of three — applied at the +write and again at the read. Each is a display budget, not a storage limit. + +**`id` is capped at the WRITE ONLY, and that asymmetry is deliberate.** +`normalize_bench_url` refuses an input over `URL_MAX`, so nothing this code +writes can exceed it. On the read the id is the dict KEY and it is the locator +every control posts back — `bench state`, `bench rm`, and the panel's remove +button all address by it. Truncating a hand-edited over-long key on read would +produce a row the operator can see and cannot act on, which is strictly worse +than a long one. Amended after the cold panel found the code and the contract +disagreeing here; the code was right. ## Signatures @@ -234,10 +248,18 @@ A registry that cannot be read renders as a panel carrying its error, never as an absent panel and never as a 500 — the v0.2.2 lesson, which this repo learned by returning 500 for `/` and `/healthz` across all 25 booths. +**The panel is gated on PAGE IDENTITY — the booth carries a `links.md` — and +never on content.** A content gate (`board or benches`) hides the panel AND its +registration form exactly when the board is empty and the registry absent, +which is the state a fresh deployment starts in and the one where "no benches +registered yet" is most worth saying. That is the same defect as a damaged +panel rendering as an absent one, one level up. Amended after the cold panel +found the content gate shipped. + ## The CLI surface ``` -booth bench add register or update; prints created/updated +booth bench add register or update; prints registered/updated booth bench ls list, in the rendered order, with ids booth bench state live | promoted | retired booth bench rm remove one @@ -362,7 +384,14 @@ byte. **INV-8 — nothing this unit ships removes a board row.** The dead marker is a render-time stamp; `import` without `--apply` writes nothing anywhere; `import` -with `--apply` writes only `.benches.json`. +with `--apply` writes only the registry and its lock sidecar (`.benches.json`, +`.benches.lock`) and never touches `links.md`. + +*(Amended: this said "writes only `.benches.json`", which contradicted the +unit's own assumption that every read-modify-write is held under an flock on a +sidecar. The cold panel caught the contract arguing with itself. The +load-bearing half — `links.md` is not touched — is unchanged and is what the +test hashes.)* *Falsifiable:* the defeating change is `import --apply` "tidying up" the rows it consumed. The test snapshots `links.md` byte for byte, runs the full unit's CLI surface against it — refusal, import, import --apply, bench add, bench rm — and @@ -374,12 +403,12 @@ outside the standard library and nothing from `booth.*`. which is the natural thing to write, since `booth_target` is the predicate this unit's CLI branch also needs. -**The existing parametrized `test_stdlib_only` (tests/test_marks.py:280) DOES +**The existing parametrized `test_stdlib_only` in tests/test_marks.py DOES NOT CATCH THAT, and an earlier draft of this contract claimed it did.** Its failure set is `{r for r in roots if r != "booth" and r not in sys.stdlib_module_names}` — it exempts `booth` explicitly, so a sibling import -passes it clean. The sibling-free clause exists only in the stricter copy at -tests/test_manifest.py:209. Adding `benches` to the parametrized list therefore +passes it clean. The sibling-free clause exists only in the stricter copy in +tests/test_manifest.py. Adding `benches` to the parametrized list therefore buys stdlib-only and NOT sibling-free. So: `benches` joins that list AND `tests/test_benches.py` carries its own stricter copy, mirroring `manifest`'s, which fails on a `booth` root. Verified by reading the real test — seam review @@ -395,13 +424,13 @@ this file and are forbidden the siblings it borrows from. | # | seam | what the real surface said | disposition | |---|---|---|---| -| **SR-1** | `test_stdlib_only` (tests/test_marks.py:280) | **The contract was wrong.** It claimed the parametrized test "already carries" the sibling-free clause. It does not — its failure set is `{r for r in roots if r != "booth" and ...}`, which exempts `booth` on purpose. Only tests/test_manifest.py:209 has the strict copy. | **Fixed.** INV-9 now requires both: the parametrize entry AND a stricter copy in `tests/test_benches.py`. Without this the unit would have shipped with its own INV-9 untested. | -| **SR-2** | `resolve_booth` (app.py:798) | **The contract invited an outage.** It named `resolve_booth` as the existence check for the dead marker. That function is a closure inside `create_app` (not importable) and **raises HTTPException(404)** — called per row, one swept booth would 404 the entire board page. It also calls `.resolve()`, a syscall per row, 178 of them on this board. | **Fixed.** `depends_on` now forbids it explicitly and specifies an own non-raising predicate with the same name-safety rules. Cost stated below. | +| **SR-1** | `test_stdlib_only` (tests/test_marks.py) | **The contract was wrong.** It claimed the parametrized test "already carries" the sibling-free clause. It does not — its failure set is `{r for r in roots if r != "booth" and ...}`, which exempts `booth` on purpose. Only tests/test_manifest.py:209 has the strict copy. | **Fixed.** INV-9 now requires both: the parametrize entry AND a stricter copy in `tests/test_benches.py`. Without this the unit would have shipped with its own INV-9 untested. | +| **SR-2** | `resolve_booth` (booth/app.py) | **The contract invited an outage.** It named `resolve_booth` as the existence check for the dead marker. That function is a closure inside `create_app` (not importable) and **raises HTTPException(404)** — called per row, one swept booth would 404 the entire board page. It also calls `.resolve()`, a syscall per row, 178 of them on this board. | **Fixed.** `depends_on` now forbids it explicitly and specifies an own non-raising predicate with the same name-safety rules. Cost stated below. | | **SR-7** | `quote(name, safe="")` (app.py, booth link emission) | **The contract was silent on encoding.** Booth links are emitted percent-encoded. A `booth_target` comparing the raw path segment to a directory name marks every encoded-name booth permanently dead and echoes the encoded form back in the refusal. | **Fixed.** `booth_target` decodes, and applies `resolve_booth`'s own addressability rules so the two cannot disagree. | | **SR-6** | `scripts/booth` dispatch (flat `case "$cmd"`, 13 single-word verbs) | Not a defect — a gap. **`bench add` would be the first two-word verb in this script.** Nothing about the existing dispatch anticipates one, and `booth bench` with no sub-verb must not fall through into the generic usage in a way that hides which word was wrong. | **Recorded.** A nested `case` under `bench)`, and a bare `bench` prints the bench verbs specifically. Named so the implementer does not invent a third pattern. | -| **SR-3** | `data_dir` (app.py:704) vs `DATA` (scripts/booth:120) | The service resolves and expands its root in `create_app`; the CLI derives it from `$BOOTH_DATA_DIR`. Two independent derivations of one path. | **No change.** This is already true of `links.md`, `.marks.json` and `.booth.json` — pre-existing and out of this unit's scope. Recorded so it is a known property rather than a discovery. | -| **SR-4** | `list_booths` (app.py:418) | **Confirmed, not assumed.** `if not child.is_dir() or child.name.startswith("."): continue` — `.benches.json` fails both guards. The index cannot see the registry. | **Verified.** The assumption stands on read code. | -| **SR-5** | `sweep_once` (app.py:386) | **Confirmed, not assumed — and this was the dangerous one.** The sweeper iterates the data root and could in principle delete the registry. It cannot: the same `is_dir()` + leading-dot pair guards it, and `shutil.rmtree` is reached only past both. | **Verified.** Had either guard been absent this unit would have shipped a design that eats its own registry on the first tick. | +| **SR-3** | `data_dir` (booth/app.py) vs `DATA` (scripts/booth) | The service resolves and expands its root in `create_app`; the CLI derives it from `$BOOTH_DATA_DIR`. Two independent derivations of one path. | **No change.** This is already true of `links.md`, `.marks.json` and `.booth.json` — pre-existing and out of this unit's scope. Recorded so it is a known property rather than a discovery. | +| **SR-4** | `list_booths` (booth/app.py) | **Confirmed, not assumed.** `if not child.is_dir() or child.name.startswith("."): continue` — `.benches.json` fails both guards. The index cannot see the registry. | **Verified.** The assumption stands on read code. | +| **SR-5** | `sweep_once` (booth/app.py) | **Confirmed, not assumed — and this was the dangerous one.** The sweeper iterates the data root and could in principle delete the registry. It cannot: the same `is_dir()` + leading-dot pair guards it, and `shutil.rmtree` is reached only past both. | **Verified.** Had either guard been absent this unit would have shipped a design that eats its own registry on the first tick. | **The per-render cost, stated because SR-2 surfaced it.** The dead marker runs once per board row: 221 rows today, 178 of which parse as booth links and cost @@ -411,3 +440,36 @@ per booth on the index, so it is not a new order of magnitude. It is bounded by the row count, it touches no network, and it is confined to the ONE booth that carries a `links.md`. If the board ever grows past a few thousand rows this becomes worth caching; at 221 it would be premature. + +## Code review — what the cold panel found + +`/heid-code-review` panel `01M35CK8YKEKMV7T15JXEF6A8N`, four arms, verdict +**NOT drift-zero**. Folded in full. Three findings were independently reported +by **all four arms**, which is the signature of a contract clause that was +written as prose and never converted into an assertion. + +| # | finding | arms | disposition | +|---|---|---|---| +| **A** | **The panel dropped the added date.** *What renders* says "the date it was added"; `b.added` appeared nowhere in the template and no test asked for it. | 4/4 | **Fixed** — rendered, and pinned by a test. | +| **B** | **`bench ls` printed no ids**, and the truncated URL it printed was not pasteable into `bench state\|rm`. Worse: the test's own docstring *claimed* it printed ids while asserting nothing — a claim standing in for evidence, which is how the drift would have survived CI. | 4/4 | **Fixed** — the id prints whole and last; the test now round-trips what `ls` prints back through `bench state`. | +| **C** | **`bench import` printed the description, not the raw URL**, beside each id — hiding the five-rows-of-talk collapse the clause exists to expose. | 4/4 | **Fixed** — raw URL beside the id, description demoted to a continuation line. | +| **D** | **An IPv6 literal lost its brackets.** `http://[::1]:8080/a` normalized to `http://::1:8080/a` — not another spelling but a BROKEN identity, so a re-post never matches the row. | 3/4 | **Fixed** — bracketed literals are re-wrapped; an *unbracketed* one is refused with a reason rather than guessed at. | +| **H** | **INV-4's tie-break falsifier could not fail.** `_write_all` serializes with `sort_keys=True`, so both insertion orders came back off disk already id-sorted and removing the tie-break left the test green. | 1/4 | **Fixed** — the test now calls `order_benches` directly with records that tie on both prior keys. A vacuous falsifier of exactly the class `persistent-memory.d/2026-09-22-vacuous-falsifiers.md` names, found by a cold reader and not by us. | +| **I** | **An empty board hid the whole panel**, registration form included — the state a fresh deployment starts in. | 1/4 | **Fixed** — gated on page identity. | +| **J** | **The `booth link` refusal could fail OPEN** on a name bash's `$()` erases, because it classified by captured-text emptiness. | 1/4 | **Fixed** — the predicate answers with a `B:`/`N` sentinel, so no name can be mistaken for "not a booth". | +| **K** | A FIFO at the registry path blocked in `open()`; a deeply-nested JSON `RecursionError` escaped the `except (ValueError, OSError)` pair. | 1/4 | **The FIFO half was already fixed** by our own pass before the reply landed. **The RecursionError half was not** — 200k open brackets is 200 KB, well inside the byte cap, and it 500'd the page the function exists to protect. Fixed. | +| **E** | The read does not apply the `id` cap the contract promised. | 3/4 | **Contract amended, code kept.** The id is the locator every control posts back; truncating a hand-edited over-long key would make a row visible and unactionable. | +| **F, G** | INV-5's render test covered 5 of 6 cases and asserted only status 200; INV-2's URL table never ran through the dead-marker render. | 4/4, 3/4 | **Both fixed** — the render test now covers oversized, unreadable and FIFO and asserts the error is *visible*; the full table runs through the marker. | + +**Also folded from the per-invariant vacuity pass** (the arms' "what would still +pass" section, which is the single most useful thing the panel produced): +INV-6 had no vector asserting a non-default port is part of the identity, so +"always omit the port" passed every row; INV-3 asserted only that `links/` was +absent, so a refusal touching any other sidecar passed; INV-8's hashed sequence +omitted `bench ls`; INV-9's AST walk is defeated by `__import__("booth.links")`. +All four closed. + +**Declined:** nothing. **Amended rather than fixed:** E, `updated`'s meaning, +INV-8's file list, the `registered`/`created` wording, and every line number in +this document's prose — the panel found two already stale, which is the whole +argument against putting them in prose at all. diff --git a/scripts/booth b/scripts/booth index 8cc8383..885bafa 100755 --- a/scripts/booth +++ b/scripts/booth @@ -218,12 +218,20 @@ booth_src() { # the board's dead-row marker and `bench import` use that same function and a # second implementation in the shell would classify the host-agnostic and # percent-encoded cases differently (INV-2). +# Prints `B:` for a booth URL and `N` for anything else. +# +# A SENTINEL, NOT AN EMPTY STRING. Command substitution strips trailing +# newlines, so a predicate that answers with the bare name cannot distinguish +# "not a booth" from "a booth whose name bash just erased" — and the guard +# then fails OPEN on that edge, which is the one direction a guard must never +# fail. The prefix makes the answer unambiguous whatever the name contains. booth_target_of() { BOOTH_SRC="$(booth_src)" BOOTH_Q="$1" python3 -c ' import os, sys sys.path.insert(0, os.environ["BOOTH_SRC"]) from booth.links import booth_target # stdlib only — no venv needed -sys.stdout.write(booth_target(os.environ["BOOTH_Q"]) or "") +name = booth_target(os.environ["BOOTH_Q"]) +sys.stdout.write("N" if name is None else "B:" + name) ' } @@ -344,6 +352,13 @@ case "$cmd" in } >&2 exit 3 fi + case "$refused_name" in + N) refused_name="" ;; + B:*) refused_name="${refused_name#B:}" ;; + *) + echo "booth link: the booth check answered something unrecognised; nothing was posted." >&2 + exit 3 ;; + esac if [ -n "$refused_name" ]; then { echo "booth link: that is a booth, and a booth announces itself now." @@ -463,7 +478,12 @@ def die(msg, code=2): def row(b): # ONE LINE PER BENCH, in the rendered order — state first, so a retired # bench sinks, then name, then id as a total tie-break (INV-4). - return "%-9s %-28s %-52s %s" % (b.state, b.name[:28], b.url[:52], b.owner) + # THE ID IS PRINTED WHOLE AND UNTRUNCATED, because it is the locator + # `bench state` and `bench rm` take: a truncated one is not an id, it is a + # string that looks like one and silently addresses nothing. The name and + # the added date are the truncatable columns. + return "%-9s %-10s %-16s %-24s %s" % ( + b.state, b.added[:10], b.owner[:16], b.name[:24], b.id) if sub == "add": if len(argv) < 2: die("bench add ") @@ -478,6 +498,9 @@ elif sub == "ls": die("the registry could not be read: %s" % err, 3) if not benches: print("no benches registered yet") + else: + print("%-9s %-10s %-16s %-24s %s" + % ("STATE", "ADDED", "OWNER", "NAME", "ID (pass to state|rm)")) for b in benches: print(row(b)) elif sub in ("state", "rm"): @@ -518,7 +541,13 @@ elif sub == "import": print("CANDIDATES — would be registered (%d rows, %d distinct):" % (len(candidates), len({i for i, _ in candidates}))) for i, e in candidates: - print(" %-52s %s" % (i, e["desc"][:60])) + # THE NORMALIZED ID BESIDE THE RAW URL, which is the whole point of the + # proposal: five rows of `talk` collapsing to one is only visible if you + # can see which five raw URLs produced the one id. The description is + # the thing to drop here, not the URL. + print(" %-52s %s" % (i, e["url"])) + if e["desc"]: + print(" %-52s %s" % ("", e["desc"][:70])) print() print("REFUSED — normalization said no (%d):" % len(refused)) for e, why in refused: diff --git a/tests/test_benches.py b/tests/test_benches.py index eb4cc39..47f3e2b 100644 --- a/tests/test_benches.py +++ b/tests/test_benches.py @@ -97,6 +97,8 @@ def test_query_is_part_of_the_identity(tmp_path): ("https://x.test:443/p", "https://x.test/p"), ("http://x.test/p#frag", "http://x.test/p"), # fragment dropped (" http://x.test/p ", "http://x.test/p"), # whitespace + ("http://[::1]:80/a", "http://[::1]/a"), # default port, bracketed + ("http://[::1]/A", "http://[::1]/A"), # bracket round-trips ]) def test_these_pairs_are_one_bench(a, b): """INV-6. Each pair is the SAME resource reached two ways.""" @@ -108,6 +110,11 @@ def test_these_pairs_are_one_bench(a, b): ("http://x.test/p", "http://x.test/P"), # path case ("http://x.test/?a=1&b=2", "http://x.test/?b=2&a=1"), # query order is opaque ("http://x.test:8092/", "https://x.test:8092/"), # scheme + # A NON-DEFAULT PORT IS PART OF THE IDENTITY. Without this vector, "always + # omit the port" passes every other row in this file — caught by the cold + # panel's per-invariant "what would still pass" pass, not by us. + ("http://x.test:8092/p", "http://x.test/p"), + ("https://x.test:8443/p", "https://x.test/p"), ]) def test_these_pairs_are_two_benches(a, b): """INV-6, the other direction. Each pair MAY be two different resources, and @@ -150,18 +157,32 @@ def test_the_stored_url_is_the_raw_string(tmp_path): # ---- INV-4: the rendered order is TOTAL and stated -------------------------- -def test_same_name_benches_do_not_swap(tmp_path): - """INV-4. Defeating change: dropping the `id` tie-break. Two benches with - the SAME name, registered in both orders, must render identically — a test - over distinct names passes with no tie-break at all.""" - def build(order): - root = tmp_path / f"r{order}" - root.mkdir() - for u in (["http://a.test/", "http://b.test/"] if order else - ["http://b.test/", "http://a.test/"]): - upsert_bench(root, u, "same name", "o") - return [b.id for b in read_benches(root)[0]] - assert build(0) == build(1) +def test_same_name_benches_do_not_swap(): + """INV-4. Defeating change: dropping the `id` tie-break. + + THIS TEST USED TO GO THROUGH THE REGISTRY AND COULD NOT FAIL. `_write_all` + serializes with `sort_keys=True`, so whatever order two benches were + inserted in, they came back off disk already id-sorted — and removing the + tie-break from `order_benches` left it green. A vacuous falsifier of + exactly the shape persistent-memory.d/2026-09-22-vacuous-falsifiers.md + describes: it asserted the outcome the author had in mind rather than the + discriminator the invariant names. Caught by the cold panel (hulda, solo), + not by us. + + So it calls `order_benches` DIRECTLY, with records that tie on both prior + keys, presented in both orders. Nothing upstream can pre-sort them. + """ + def recs(order): + pair = [ + Bench(id="http://a.test/", url="http://a.test/", name="same name", + owner="o", state="live", added="", updated=""), + Bench(id="http://b.test/", url="http://b.test/", name="same name", + owner="o", state="live", added="", updated=""), + ] + return pair if order else list(reversed(pair)) + assert [b.id for b in order_benches(recs(0))] == \ + [b.id for b in order_benches(recs(1))] + assert [b.id for b in order_benches(recs(1))] == ["http://a.test/", "http://b.test/"] def test_state_ranks_before_name(tmp_path): @@ -276,6 +297,14 @@ def test_benches_is_stdlib_only_and_imports_no_sibling(): roots.add("booth" if node.level else (node.module or "").split(".")[0]) outside = {r for r in roots if r and r not in sys.stdlib_module_names} assert not outside, f"booth/benches.py imports outside the stdlib (booth.* included): {sorted(outside)}" + # A STRING IMPORT IS INVISIBLE TO THE WALK ABOVE. `__import__("booth.links")` + # or `importlib.import_module(...)` inside a function defeats it entirely, + # and that is the exact shape someone reaches for when a sibling import is + # refused by review. Caught by the cold panel's per-invariant vacuity pass. + called = {n.func.id for n in ast.walk(tree) + if isinstance(n, ast.Call) and isinstance(n.func, ast.Name)} + assert "__import__" not in called, "benches.py imports by string, defeating the AST walk" + assert "importlib" not in roots, "benches.py can import anything at runtime via importlib" # ---- INV-2: ONE predicate decides what a booth URL is ----------------------- @@ -485,6 +514,9 @@ def test_the_anchor_href_is_the_raw_url_not_the_id(tmp_path): ("[]", "wrong top-level shape"), ('{"http://a/": {"name": [], "owner": "o", "state": "live"}}', "wrong-typed field"), ('{"http://a/": {"name": "n", "owner": "o", "state": "invented"}}', "unknown state"), + ("OVERSIZED", "over the size cap"), + ("UNREADABLE", "chmod 000"), + ("FIFO", "a named pipe"), ]) def test_a_damaged_registry_costs_its_panel_and_never_the_page(tmp_path, payload, label): """INV-5, at the render. The v0.2.2 lesson: a poisoned sidecar returned 500 @@ -492,12 +524,34 @@ def test_a_damaged_registry_costs_its_panel_and_never_the_page(tmp_path, payload the shape currently 500ing the gallery elsewhere in this service, so it is the one that matters most.""" _board(tmp_path, ROW_REF) - if payload is not None: - (tmp_path / BENCHES_FILE).write_text(payload) - c = _client(tmp_path) - assert c.get("/b/links/").status_code == 200, label - assert c.get("/").status_code == 200, label - assert c.get("/healthz").status_code == 200, label + reg = tmp_path / BENCHES_FILE + if payload == "OVERSIZED": + from booth.benches import BENCHES_MAX_BYTES + reg.write_text('{"http://a/": {"name": "' + "x" * BENCHES_MAX_BYTES + '"}}') + elif payload == "UNREADABLE": + if os.geteuid() == 0: + pytest.skip("root ignores the mode bit") + reg.write_text("{}") + reg.chmod(0o000) + elif payload == "FIFO": + os.mkfifo(reg) + elif payload is not None: + reg.write_text(payload) + try: + c = _client(tmp_path) + body = c.get("/b/links/") + assert body.status_code == 200, label + assert c.get("/").status_code == 200, label + assert c.get("/healthz").status_code == 200, label + # AND THE ERROR IS VISIBLE. Asserting only 200 was the gap: a render + # that swallowed the failure and drew an empty panel passed every case + # here while telling the operator nothing needed fixing. Absent is the + # one case that must NOT show an error. + shown = "the bench registry could not be read" in body.text + assert shown is (payload is not None), label + finally: + if payload == "UNREADABLE": + reg.chmod(0o644) def test_damaged_and_absent_render_different_text(tmp_path): @@ -612,3 +666,66 @@ def test_a_failed_write_leaves_no_scratch_file(tmp_path, monkeypatch): assert not strays, strays # and the prior registry is intact — a failed write destroys nothing assert [b.name for b in read_benches(tmp_path)[0]] == ["one"] + + +def test_an_ipv6_literal_keeps_its_brackets(): + """`urlsplit().hostname` strips them, and a netloc rebuilt from it is not + another spelling of the URL — it is a broken one, so a re-post never + matches the row the operator means to update. Defeating change: rebuilding + netloc from `hostname` with no re-wrap, which is what this shipped as.""" + assert normalize_bench_url("http://[::1]:8080/a") == "http://[::1]:8080/a" + assert normalize_bench_url("http://[2001:DB8::1]/p") == "http://[2001:db8::1]/p" + assert normalize_bench_url("HTTP://[::1]:80/p") == "http://[::1]/p" + # An UNBRACKETED IPv6 netloc is refused with a reason, not repaired: + # `urlsplit(...).port` raises on `::1:8080` because it cannot tell the + # address from the port — which is precisely why the brackets exist. The + # refusal is the honest answer; guessing where the address ends would be + # inventing an identity out of an ambiguous string. + with pytest.raises(ValueError): + normalize_bench_url("http://::1:8080/a") + + +def test_deeply_nested_json_does_not_escape_the_read(tmp_path): + """RecursionError is neither ValueError nor OSError, so it went straight + past `read_benches`'s except pair and 500'd the page the function exists to + protect. The byte cap does not help: 200k open brackets is 200 KB, well + inside it. Defeating change: dropping the RecursionError arm.""" + (tmp_path / BENCHES_FILE).write_text("[" * 200_000) + benches, err = read_benches(tmp_path) + assert benches == [] and err + + +def test_the_panel_renders_on_an_EMPTY_board(tmp_path): + """The panel is gated on PAGE IDENTITY, not page content. Gating on + `board or benches` hid the panel and its registration form exactly when the + board was empty and the registry absent — the state a new deployment starts + in, and the one where "no benches registered yet" is most worth saying. + Defeating change: any content-derived gate.""" + _board(tmp_path, "") + body = _client(tmp_path).get("/b/links/").text + assert "no benches registered yet" in body + assert "bench-add" in body, "the registration form vanished with the panel" + + +def test_the_panel_shows_when_a_bench_was_added(tmp_path): + """INV-N/What renders: the contract says the panel shows the date it was + added; `b.added` appeared nowhere in the template and no test asked. All + four cold arms found this independently.""" + _board(tmp_path, ROW_REF) + upsert_bench(tmp_path, "https://talk.test/", "talk", "o") + added = read_benches(tmp_path)[0][0].added[:10] + assert added in _client(tmp_path).get("/b/links/").text + + +@pytest.mark.parametrize("url,expected", BOOTH_URL_TABLE) +def test_the_dead_marker_classifies_the_SAME_table(tmp_path, url, expected): + """INV-2 names RENDER-LEVEL agreement, and the marker tests never ran the + table — three hand-written rows with no query between them, so a marker + that stopped calling `booth_target` and treated `?q=1` as "not a booth" + stayed green while disagreeing with the CLI. Caught by the cold panel. + + Every row whose target does not exist on disk must be marked dead; every + non-booth row must not be.""" + _board(tmp_path, f"- [r]({url}) · x · 2026-09-01 00:00\n") + marked = bool(_dead_rows(_client(tmp_path).get("/b/links/").text)) + assert marked is (expected is not None), (url, expected) diff --git a/tests/test_cli.py b/tests/test_cli.py index 20bcf1e..c516e77 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -358,6 +358,7 @@ REFUSED = 2 # decides what a booth URL is, and these are the rows the CLI must agree on. # A second `/b/` check inlined in the shell for speed goes red HERE. from test_benches import BOOTH_URL_TABLE # noqa: E402 +from booth.benches import normalize_bench_url as normalize_bench_url_cli # noqa: E402 @pytest.mark.parametrize("url,is_booth", [(u, e is not None) for u, e in BOOTH_URL_TABLE]) @@ -378,9 +379,14 @@ def test_a_refused_link_writes_nothing_at_all(booth): data, _ = booth board = data / "links" assert not board.exists() + before = sorted(p.name for p in data.iterdir()) r = run(data, "link", "http://10.100.10.50:8090/b/some-booth/", "nope") assert r.returncode == REFUSED - assert not board.exists(), f"a refused link left {sorted(p.name for p in board.iterdir())}" + assert not board.exists(), "a refused link created the board directory" + # NOTHING AT ALL, not just no board. Asserting only `links/`'s absence let + # a refusal that touched `.benches.lock` (or any other sidecar) on its way + # out stay green — the cold panel's vacuity pass named exactly that. + assert sorted(p.name for p in data.iterdir()) == before, "a refused link wrote something" def test_the_refusal_names_the_alternative(booth): @@ -410,6 +416,18 @@ def test_bench_add_is_an_upsert(booth): r = run(data, "bench", "ls") assert r.returncode == OK, r.stderr assert r.stdout.count("talk v") == 1 and "talk v2" in r.stdout + # THE ID, WHOLE AND UNTRUNCATED, because it is the locator `bench state` + # and `bench rm` take. An earlier version of this test had a docstring + # claiming `bench ls` prints ids and asserted nothing of the kind, while + # the code printed a url truncated to 52 columns — a claim standing in for + # evidence, which is how the drift would have survived CI. Found by all + # four cold arms independently. + bid = normalize_bench_url_cli("https://talk.nh3.phasefinal.com:8092/") + assert bid in r.stdout, r.stdout + # and what ls prints is addressable, end to end + line = [l for l in r.stdout.splitlines() if "talk v2" in l][0] + printed_id = line.split()[-1] + assert run(data, "bench", "state", printed_id, "promoted").returncode == OK def test_bench_verbs_round_trip(booth): @@ -476,6 +494,12 @@ def test_import_classifies_into_three_groups(booth): assert "gone" in out # the booth row, skipped assert "talk" in out # a candidate assert "ftp://x.test/f" in out # refused, with its reason + # THE RAW URL BESIDE THE NORMALIZED ID, which is the entire point of the + # proposal: five rows of `talk` collapsing to one is only checkable if you + # can see which raw URLs produced the one id. This printed the description + # instead, so the collapse was invisible in the one place it had to be + # visible. All four cold arms found it. + assert out.count("https://talk.nh3.phasefinal.com:8092/") >= 2, out def test_import_apply_collapses_the_repost(booth): @@ -503,6 +527,7 @@ def test_nothing_in_the_unit_touches_links_md(booth): run(data, "bench", "import", "--apply") run(data, "bench", "add", "http://new.test/", "new") run(data, "bench", "state", "http://new.test/", "retired") + run(data, "bench", "ls") # a read verb can truncate too run(data, "bench", "rm", "http://new.test/") after = hashlib.sha256((board / "links.md").read_bytes()).hexdigest() assert before == after