Commit Graph
3 Commits
Author SHA1 Message Date
Vuong Hoang 95beede3c3 fix(manifest)!: the size cap opened a service-wide hang; close it
The diff-scoped bug-hunt panel, four arms, artifact-only. Its strongest
finding is one I created two hours earlier while hardening the reader.

`stat` reports size 0 for a FIFO and 0 for a symlink to /dev/zero, so both
sail under the byte cap added for the RecursionError round — and then
`read_text` either blocks in read() with no EOF, so the except never runs,
or allocates until the kernel intervenes. `list_booths` reads every booth
on every GET / and /healthz, so ONE such file stalls the front page for the
whole service, with no error and no recovery short of a restart.
Reproduced before believing it (timeout returned 124). S_ISREG is checked
BEFORE the size in both modules now; verified against the live service with
two FIFOs planted, which answered 200 in 36ms.

The shape worth carrying: st_size answers a different question than "can
this be read", and a bound that trusts it inherits everything it does not
mean. A hardening fix opened a worse hole than the one it closed.

THE UPLOAD PATH WROTE ABOVE ITS OWN CLEANUP GUARD (4/4)

A failed manifest write orphaned a .uploaded half-booth with no files in
it — and because the temp name now carries a random suffix, nothing ever
overwrote the leak, and .booth.json.<hex>.tmp is not a .lock, so
_newest_mtime counted it and kept that empty booth past every sweep. The
uniqueness fix from the previous round is what made the leak permanent.
Both writes moved inside the guard; the temp is removed on every exit path.

DAMAGED BYTES ARE KEPT, NOT REPLACED (4/4, INV-6)

Marks made this explicit in v0.2.1 and this write path contradicted it: a
manifest that failed on ONE field lost the others with it, including a why
the re-announcer may never have kept anywhere. It diverges from marks in
HOW it honours the rule — marks refuse and answer 409 because the
operator's judgment is not restatable; a manifest quarantines and proceeds,
because refusing would fail `booth add` and lose the files it was copying.

ONE OPENNESS PREDICATE, AS U2 SAID (2/4)

`booth answer` spelled out `if m.answer is None` while `booth marks` asked
`open_marks`, so a partially-answered pick read as done to one verb and
open to the other — at the same instant, on the same booth. U2's INV-2 put
openness in one function precisely so they could not drift. The mirror case
is fixed too: a pick that hydrates broken is refused by the web route, so
`answer --wait` polled an hour on a form nothing could ever land.

ALSO

- now_stamp was whole-second while the importer had moved to microseconds,
  and '-' sorts before '.', so a later mark came out ahead of an earlier
  import inside the same second. One format; the previous round's ordering
  fix had opened this one.
- `_broken` was the third of three directory-name fallbacks and the one
  still handing a raw name into a card's sub-line.
- An identical re-announce rewrote the file and reset the TTL. `booth link`
  does this on every post to the standing board.
- The importer's return went through the bare _hydrate, not _hydrate_safe.
- A marks document could be written larger than it can be read back, and
  then read as no marks at all. Refused at the write instead.
- `choice` reached the answer builder raw while `notes` beside it did not.

AND ONE FINDING DELIBERATELY NOT FULLY CLOSED

The mtime-restore race is real. The clean fix — ignore a booth directory's
own mtime whenever the booth holds anything — also silently retires the
documented rule that releasing a kept board resets its clock, which the CLI
header, the README and a deliberately-written test all pin. That is a TTL
doctrine change, not a bug fix, and an existing test caught the attempt.
The concrete half is fixed (a failing os.utime escaped and 500'd the
route); the race is stated in the code where the next reader will meet it.

341 tests. Live service restarted, 24/24 booth pages verified.
2026-09-22 02:27:18 -07:00
Vuong Hoang c015a917ee fix(manifest): fold in both cross-frontier panels — and a live hole in v0.2.2
Two four-arm artifact-only rounds landed together: the contract paraphrase
(against the pre-seam-review capture) and the code-vs-contract conformance
review (against the amended one), correctly firewalled from each other.
The conformance round found ZERO drift in the strict sense — the code is a
clause-for-clause implementation of the contract — and the weight of both
rounds landed one layer down, in what green tests structurally cannot
report. Full triage in persistent-memory.d/.

A LIVE HOLE IN RELEASED CODE, FOUND ON THE SIBLING MODULE

v0.2.2 adopted the RecursionError finding from the bug-hunt round and
closed half of it: `_hydrate_safe` guards hydration, but `json.loads` runs
above it in `_read_raw`, whose catch list covers neither RecursionError nor
MemoryError. A 400 KB file of nothing but brackets in any ONE booth
therefore still returned 500 for `/` and `/healthz` across every booth on
the service. Confirmed by running it before believing it.

Both modules now bound the read by `stat` before touching the bytes and
catch both classes anyway, so raising a bound later cannot quietly re-open
the hole. The strict half of the marks asymmetry refuses everything the
lenient half tolerates, or a file that reads as "no marks" gets replaced by
a write that believed it.

THE WHY-WIPE

`booth new x --why "..."` then `booth add x out/*.png` erased the sentence
the first command existed to record. Omitted flags meant empty strings and
empty strings overwrote. Two arms predicted it from the contract's wording
alone; every test here passed --why on both calls and so could not see it.
Omitted now means unchanged and an explicit --why "" still clears — the
shell carries the distinction by leaving the variable UNSET, not empty.

--title WAS WRITE-ONLY

Stored, flag-surfaced, rendered nowhere. 4/4, and independently top-ranked
by every arm of the paraphrase round. It lands on the booth page heading
with the directory name beside it, because the directory name is the
identity the operator navigates by and refers to positionally.

THREE TESTS THAT COULD NOT FAIL

- test_the_write_is_atomic asserted no *.tmp survived, which a plain
  write_text passes. It asserts the inode changes now. (The first
  replacement was ALSO vacuous — it spied on os.open, which Path.write_text
  reaches through io.open in C and never touches. Recorded in the test,
  because writing a second vacuous test while fixing the first is exactly
  the failure this round is about.)
- The INV-3 preservation test passed against an implementation that
  regenerated `created` every time, because _now() is whole-second
  resolution and back-to-back writes share a stamp. Seeded from 2019 now.
- test_announcing_is_activity passed whether or not _newest_mtime counted
  the manifest, because writing it bumps the directory mtime either way.
  The directory's clock is put back, leaving the file as the only thing
  that can keep the booth alive.

ALSO

- The title fallback skipped the normalizer the explicit value gets; a
  directory name may legally carry a newline and run to 255 bytes.
- Every writer derived the same .booth.json.tmp. Marks are protected from
  that by their flock; the manifest has none, so uniqueness stands in.
- test_stdlib_only was blind to relative imports in all four modules.
- INV-1 had no guard at all; INV-5 named two different promises; the
  negative render states were asserted on the index only.

Contract amended throughout: the 4 GB case is a stat-checked bound rather
than a return constraint, every field of an error-carrying record has a
stated value, INV-1 no longer contradicts INV-3, repo-wide rules are named
in words instead of by a colliding number, and touches admits the macro
partial the implementation added.

329 tests.
2026-09-22 01:29:27 -07:00
Vuong Hoang a48ef83ef5 feat(manifest): U5 — booths that say who posted them and why
The index card showed a name, an item count and a countdown, and nothing
the poster chose. An agent with something to show therefore had no way to
make the booth say "look at this" and posted a URL to the link board
instead — which is why 145 of that board's 210 rows (69%) ended up
pointing at booths that had already been swept. The board was absorbing a
job it was never shaped for. This is the shape.

Each booth carries `.booth.json` — {handle, title, why, created} — written
by the CLI from $ALTHING_HANDLE, and the provenance line renders on both
index lanes and on the booth page header.

WHAT IS WHERE

- booth/manifest.py, stdlib-only and importing nothing from booth.* either:
  scripts/booth imports it under the system python3 with no venv, and a
  cross-import between two stdlib-only modules is a second way for that
  invariant to break. It joins the shared test_stdlib_only list and keeps
  a stricter copy of its own.
- The read is lenient and cannot raise. list_booths touches every booth on
  every index load, so a manifest that cannot be parsed costs that booth's
  provenance and nothing else. That is the v0.2.2 lesson applied before the
  same mistake rather than after it.
- Absent and damaged render differently — `unannounced` and `unreadable`.
  Folding "cannot be read" into "never said" would hide the one case
  somebody has to go and fix.
- Re-announcing preserves `created`. A second `booth add` sharpening the
  why is not a second appearance of the booth.
- The write is atomic (invariant 5); the temp file is itself a dotfile, so
  no listing can see it mid-write.

THREE OPERATOR CALLS, 2026-09-22

Flags on the existing new/add verbs rather than a separate `announce` verb
(a second step is the step that gets forgotten, which is the rot's own
mechanism). Unannounced booths get a quiet marker rather than nothing — the
convention is only adoptable if the gap is visible. U5 adds provenance only
and does NOT add a second index ordering keyed on announcement time; that
is a different surface needing its own stated rule, parked for v1.1.

NO EXEMPTION LIST

A pickup booth and the standing link board are created by the service, so
they announce themselves with handle `booth`, which is true rather than
manufactured. One rule — a booth with no manifest is unannounced — instead
of a growing set of special cases.

ALSO

tests/test_booth.py's keep/release assertion was slicing the page on the
bare word `boothhead`, which has lived in the stylesheet far longer than
the assertion has; it was reading CSS and passing on luck, and went red the
first time a new rule landed above the old one. Same assertion, aimed at
the markup. A U5 test had the mirror-image bug: pytest derives tmp_path
from the test name and the index renders data_dir, so a test named
`test_an_unannounced_booth_says_so` put the needle in the haystack itself
and passed against a template that did not yet exist.

310 tests (304 before this unit's CLI half). Live service restarted, 26/26
booth pages verified 200, end-to-end smoke through the real CLI.

NOT TAGGED. The cold contract-review panel is still in flight and the
code-review and bug-hunt gates have not run. Tagging with a gate
outstanding is what made v0.2.0 premature.
2026-09-22 00:48:41 -07:00