Vuong Hoang
95beede3c3
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.
27 KiB
contract_version, module, purpose, depends_on, language, complexity, estimated_loc, confidence, used_by, touches, assumptions, open_questions
| contract_version | module | purpose | depends_on | language | complexity | estimated_loc | confidence | used_by | touches | assumptions | open_questions | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 1.0 | booth.manifest | A booth that says what it IS and who posted it. Today the index card shows a name, an item count and a countdown -- nothing about provenance or purpose -- so an agent that wants the operator to look at something has no way to make the booth say so, and posts a URL to the link board instead. That is job 5 (`Announce`), the job nobody named, and its absence is the measured cause of 145 dead link rows (69% of the board pointing at booths that no longer exist). This unit gives job 5 a home: each booth carries `.booth.json` -- `{handle, title, why, created}`, written by the CLI from `$ALTHING_HANDLE` -- and the index card and the booth page header render it. Enforcing the link rule WITHOUT giving job 5 a home first just makes it homeless; this is the home. |
|
python | low | 150 | 0.85 |
|
|
|
|
U5 — self-announcing booths
The defect, stated precisely
The index card is the only thing an agent can put in front of the operator, and it carries no information the agent chose. Name, item count, countdown, a thumbnail. Everything about why this exists has to travel some other way.
So it travelled some other way. booth link exists because a session with
something to show had no way to make the booth itself say "look at this", and
the link board absorbed job 5 until 145 of its 210 rows (69%) pointed at
booths that had already been swept. The rot is not a link-board bug. The board
was doing a job it was never shaped for, because the shaped thing did not exist.
The lesson the measurement carries, and the reason this unit comes before any link-board enforcement: enforcing the link rule without giving job 5 a home just makes it homeless.
The record
@dataclass(frozen=True)
class Manifest:
handle: str # an althing handle, or "booth" for one the service made
title: str # display name; falls back to the directory name
why: str # ONE line: what the operator is looking at and why
created: str # ISO-8601 with offset, from the FIRST announcement
error: str | None = None # a read-time verdict; never stored
.booth.json on disk is the same four fields, no error.
Every field on an error-carrying record has a stated value, because the
templates render the record and a careless fill would re-raise the outage in
the renderer: handle and why and created are "", title is the
normalized directory name, and error says which of the six refusals fired.
created being "" is what makes write_manifest treat a damaged prior as
having no stamp to preserve (INV-3).
Caps, all applied at the write and again at the read: handle 64, title 120,
why 200, created 64. Each is a display budget, not a storage limit —
they exist because these strings land in a card's sub-line.
Signatures
MANIFEST_FILE = ".booth.json"
HANDLE_MAX, TITLE_MAX, WHY_MAX = 64, 120, 200
MANIFEST_MAX_BYTES = 64 * 1024
QUARANTINE_FILE = ".booth.json.broken"
def read_manifest(booth: Path) -> Manifest | None:
"""This booth's announcement, or None if it never made one.
LENIENT, and never raises. `list_booths` calls this once per booth on every
index page load, so a damaged file must cost that booth's provenance and
nothing else — the same posture `marks_for` takes, for the reason v0.2.2
made expensive: a read that can raise, called in a loop over every booth,
is a service-wide outage wearing a single-booth bug's clothes.
"NEVER RAISES" IS BOUNDED, NOT MERELY CAUGHT. An earlier draft of this
contract named a 4 GB file as a tested case and constrained only the RETURN
— which is letter-compliant and purpose-defeating: reading four gigabytes
per booth per index load recreates the same outage in slow motion. The size
is checked by `stat` BEFORE the bytes are touched, and the two exception
classes that are neither `OSError` nor `ValueError` — `MemoryError` from a
huge document, `RecursionError` from a deeply nested one — are caught as
well, so that raising the bound one day cannot quietly re-open the hole.
REGULAR-FILE FIRST, THEN SIZE — and the order is the whole point. `st_size`
is 0 for a FIFO and 0 for a symlink to `/dev/zero`, so both sail under any
byte cap and then the read either blocks forever with no EOF or allocates
until the kernel intervenes. The bound is what made this reachable: a cap
that trusts `st_size` inherits everything `st_size` does not mean. One such
file stalls every `GET /` and `/healthz`, with no error and no recovery
short of a restart.
Absent -> None. Present but too large, unreadable, unparseable, not an
object, or missing `handle` -> a Manifest carrying `error`, so the card can
say `unreadable` rather than quietly showing the same thing as a booth that
never announced.
"""
def write_manifest(booth: Path, handle: str, *, title: str | None = None,
why: str | None = None) -> Manifest:
"""Announce a booth. Atomic per CLAUDE.md invariant 5: temp file +
os.replace, onto a temp name no other writer will pick.
OMITTED MEANS UNCHANGED; `""` MEANS CLEAR. `title` and `why` default to
None. The ordinary sequence is `booth new x --why "..."` then
`booth add x out/*.png`, and while omission meant `""` the second command
silently erased the sentence the first one existed to record. The shell
carries the distinction by leaving the environment variable UNSET rather
than empty.
Re-announcing PRESERVES the original `created` — `created` is when the
booth appeared, and saying something more about it later is not a second
appearance. A prior record carrying `error`, or one whose `created` is
`""`, is treated as having no stamp to preserve and gets `now()`: a stamp
that is silently wrong is worse than one that is silently new.
A WRITE THAT CHANGES NOTHING IS NOT ACTIVITY and does not touch the file,
so it cannot reset the booth's TTL — the rule marks learned in v0.2.0,
needed here because `booth link` re-announces the standing board on every
single post to it.
BYTES THAT COULD NOT BE READ ARE KEPT, not replaced. See INV-6.
A FAILED WRITE LEAVES NOTHING BEHIND. The temp name carries a random suffix
so two writers cannot share it — which also means nothing ever overwrites an
orphan, and `.booth.json.<hex>.tmp` is not a `.lock`, so `_newest_mtime`
counts it and a leak would keep a dead booth alive forever. Cleaned up on
every exit path.
`title` falls back to the directory name, THROUGH the same normalizer the
explicit value gets — a directory name may legally carry a newline on POSIX
and may run to 255 bytes, and the fallback used to hand either straight
into a card's sub-line.
Every stored string is collapsed to a single line — all runs of whitespace,
not only newlines, because a tab or a forty-space indent renders as badly
in a sub-line as a newline does — and truncated to its cap.
An empty `handle` becomes `"booth"` rather than being refused: a manifest
naming no handle does not read back at all, and an unreadable file is the
worse outcome. Unreachable from the CLI, whose fallback chain always yields
something; a direct caller should pass a real one.
"""
What renders
One line, on both surfaces, driven by the same record. The example booth below
is the directory r18-ab, announced by the handle booth-dev:
| state | the provenance line, on an index card AND on the booth header |
|---|---|
| announced, with a why | booth-dev · pick the winning denoiser |
| announced, no why | booth-dev |
| no manifest | unannounced (muted) |
| damaged manifest | unreadable (muted, warning tint, title= carries the reason) |
title renders too, and on exactly one surface. An earlier draft stored it,
surfaced a --title flag for it, and rendered it nowhere — a promise of a
display name with no display, caught 4-of-4 and ranked first independently by
every arm. It lands on the booth page heading, where there is room:
<h1>R18 A/B <span class=h1-slug>r18-ab</span></h1>. The index card keeps
the directory name alone, because that is the identity the operator navigates
by and refers to positionally, and CLAUDE.md invariant 6 is about exactly that
kind of reference surviving a re-render. When title equals the directory name
— the default — the heading is unchanged from today.
Both index lanes get it. The kept lane renders first and is a separate block
in index.html; patching only the ephemeral lane would leave the 15 kept booths
— the durable, most-looked-at ones — with exactly the defect this closes. This
is the blurtoggle lesson (three item branches, one macro) applied to two lanes.
The booth page header gets it too, and that is deliberate scope, not creep: a booth URL handed to the operator lands on the booth page, never on the index. Job 5 is "operator, look at this", and the page he actually opens is where the answer has to be.
The CLI surface
Operator decision, 2026-09-22 — flags on the existing verbs, not a second verb:
booth new r18-ab --why "pick the winning denoiser"
booth add r18-ab out/*.png --why "second pass, sharper" --title "R18 A/B"
booth new scratch # still legal — handle + created, no why
handle comes from $ALTHING_HANDLE, falling back to $BOOTH_SOURCE then
hostname -s — the same resolution booth link already uses for its rows, so
provenance means the same thing on the board and on the card.
Nothing existing breaks. A bare booth new x / booth add x f.png keeps
working; the flags are optional and may sit on either side of the file
arguments, because a glob is usually last and a flag usually after it and
nothing enforces that. The alternative — a separate booth announce verb — was
rejected because a second step is the step that gets forgotten, which is the
69% rot's own mechanism.
A bare re-announce does not wipe what the last one said. On a booth that has
never announced, a bare new/add writes {handle, created} with no why. On
one that HAS, an omitted flag leaves the stored value alone and only a supplied
one overwrites — --why "" still clears, which is a different intention. This
distinction is load-bearing rather than polite: booth new x --why "…" followed
by booth add x out/*.png is the ordinary sequence, and the naive reading
erases the sentence on the second command.
The handle is the CLI's three-step chain, not $ALTHING_HANDLE alone:
${ALTHING_HANDLE:-${BOOTH_SOURCE:-$(hostname -s)}}, identical to the one
booth link already uses for its rows, so provenance means the same thing on
the board and on the card. A session with no handle set still announces, as its
host.
Scope — the blast-radius pass
Graphify + grep, both run, because neither is sufficient alone (graphify is blind to function-local and DI-injected imports; grep misses transitive reach).
Every site that creates a booth directory:
| site | gets a manifest? |
|---|---|
scripts/booth new (line 97) |
yes — $ALTHING_HANDLE |
scripts/booth add (line 103) |
yes — $ALTHING_HANDLE |
scripts/booth link (line 178) |
yes — handle: booth, the standing board |
app.upload (app.py:1087) |
yes — handle: booth, a pickup booth |
marks._Locked.__enter__ (marks.py:267) |
no — mkdir(exist_ok=True) on the write path; a mark written to a booth that does not exist is not an announcement, and manifest.py must not be imported by marks.py (INV-1 cross-import) |
rsync from another host |
no — no CLI runs; this is why unannounced exists |
Every reader of a booth's facts: list_booths (app.py:251) and booth_view
— confirmed by graphify explain list_booths (15 edges, 4 test consumers) and
by grep for data_dir.iterdir (two sites, both in app.py, both enumerating
booths for exactly these two surfaces).
Sites that already exclude the new file and need no change, each verified
rather than assumed: items.booth_items (items.py:182), app.zip_booth
(app.py:351), marks.import_legacy_asks (marks.py:656).
One site the first draft of this contract got WRONG, corrected by the seam
review (SR-1, below): the upload path's used: set = {UPLOAD_MARKER} filename
dedupe set does not need to gain MANIFEST_FILE. The implementation adds it
anyway, as consistency with the equally-unreachable entry already there, and
says so in a comment rather than claiming it prevents anything.
⚠ Line numbers in this section are the PRE-CHANGE coordinates the
blast-radius pass was run against, kept because that is what makes the pass
auditable. They have moved; grep the symbol, do not trust the number.
Seam review — what the real sibling surfaces said
Caller-side pass against the actual modules, not against their prose. Run after the cold contract panel was dispatched and before any code.
SR-1 — the upload-collision change is unnecessary, and so is the one already
there. safe_upload_name (app.py) does base = base.lstrip(".") with the
comment "a leading dot would hide the file from every listing", so an uploaded
file can never be named .booth.json — or .uploaded, which means the existing
UPLOAD_MARKER entry in that set has never been able to matter either. Adding
MANIFEST_FILE alongside it is consistency with a redundant guard, not a fix
for a reachable collision. Do it or don't; what the contract may not do is claim
it prevents something. This is the exact class the seam review exists for: a
scope item the contract asserted from its own reasoning and the sibling's real
surface refutes.
SR-2 — the atomic-write pattern transfers cleanly to a dotfile, verified not
assumed. marks._write_raw derives its temp name as
path.with_suffix(path.suffix + ".tmp"). For a dotfile with an extension that
is not obviously safe — Path(".booth.json").stem is ".booth", which looks
alarming — but .suffix is ".json" and the result is .booth.json.tmp.
Checked against the interpreter. The temp file is itself a dotfile, so
booth_items and zip_booth skip it and no reader can see it mid-write.
SR-3 — the dotfile skips are on p.name, and all three use rglob or
iterdir over the booth. items.booth_items (items.py:182), app.zip_booth
(app.py:351) and marks.import_legacy_asks (marks.py:656) each test
p.name.startswith("."). A manifest at the booth root is skipped by every one
of them. Confirmed by reading the three loops, not by trusting the claim.
SR-4 — test_stdlib_only is parametrized ["marks", "asks", "links"]
(tests/test_marks.py:279) and gains "manifest" as a fourth entry. The test's
docstring calls this INV-5 while CLAUDE.md calls it invariant 1; that
inconsistency predates this unit and is left alone.
SR-5 — .booth.json is reachable over HTTP at /b/<name>/.booth.json.
booth_file refuses only path escapes and non-files, not dotfiles, so a remote
session with no filesystem access can read a booth's announcement the same way
it already polls /b/<n>/marks.json. That is a feature and it is now written
down; there is no secret in a manifest, and the Booth has no auth by design.
SR-6 — list_booths returns plain dicts and the templates read them by key.
b.manifest resolves through Jinja's getitem fallback. A None manifest must be
guarded with an explicit {% if %} rather than relying on b.manifest.handle
rendering as Undefined, because the two lanes' cards differ and a silent
Undefined in one of them is how the kept lane would quietly keep the old defect.
Out of scope
Deliberately deferred or never. Divergence here is not drift.
- A second index ordering keyed on
created— a "what landed" feed. Operator decision, 2026-09-22: parked for v1.1. It is a new ordered collection needing its own stated rule, it competes with the existing order for what "the third one" means, and it has nothing to sort the 26 manifest-less booths by. whyin the zip manifest, or abooth lscolumn. One-liners over the same record, neither on the v1 path.- Enforcing that a booth MUST announce itself.
rsyncis the documented path for every host that is not nh3-dev and never runs the CLI, so a refusal would break the documented workflow. The marker is the whole mechanism. - Deleting, expiring or migrating anything based on the manifest. U4 owns lifetime; this unit only describes.
- Any change to how items, marks, blur, keep or the link board work. The manifest is a dotfile and every existing listing already skips it.
- Auth, or treating a manifest as trusted. Standing non-goal; the Booth is
LAN-internal and a hand-written
.booth.jsonis a supported input. - Provenance ON a verbatim-
index.htmlbooth's own page. Five live booths serve the author's HTML raw, and the Booth owns no header there to put a line into — it currently reaches those pages through six regexes injected into arbitrary markup, which is precisely the defect U3 exists to fix. Their INDEX cards carry provenance like everything else; the page itself waits for U3's declared embed seam. Verified onpewpew-ui-brief: page renders 200, card readsunannounced.
Invariants
Numbered INV-1..5 and local to this unit. Where a repo-wide rule is meant it is
named in words — "CLAUDE.md invariant 5", "CLAUDE.md invariant 6" — never by a
bare number, because an earlier draft used INV-5 for both the repo's
atomic-write rule and this unit's render rule and the collision was caught
3-of-4.
INV-1 — one module knows the filename. booth/manifest.py is the only
module that names MANIFEST_FILE. No route body, template or CLI verb opens or
parses .booth.json; write_manifest reads it back inside that module, which
is what INV-3 requires and is not an exception to this rule. Falsifiable and
tested: no other file under booth/ contains the literal .booth.json.
INV-2 — the read cannot raise, AND cannot cost the caller unboundedly.
read_manifest returns for every input: an absent directory, a .booth.json
that is a list, a string, null, empty, not UTF-8, wrong-typed, missing its
handle, nested deeply enough to overflow the parser's stack, and one larger
than MANIFEST_MAX_BYTES — which is refused by stat before a byte is read,
because a bound that only constrains the RETURN recreates the outage in slow
motion. Tested per case, the size and depth cases included.
INV-3 — created survives re-announcement. A second write_manifest on the
same booth preserves the first created. A prior record carrying error, or
one whose created is "", has no stamp to preserve and gets now(). Tested
against a stamp that could not have come from now() — _now() is whole-second
resolution, so back-to-back writes share a timestamp and a naive test passes
against an implementation that regenerates it every time.
INV-4 — stdlib-only, and sibling-free (this is CLAUDE.md invariant 1
extended by one clause). booth/manifest.py imports nothing outside the
standard library and nothing from booth.* — a cross-import between two
stdlib-only modules is a second way for the repo rule to break. Relative
imports count; the AST walk sees them.
INV-6 — bytes that could not be read are never destroyed. When
write_manifest replaces a manifest whose read returned error, the old bytes
move to QUARANTINE_FILE first. This is the doctrine marks made explicit in
v0.2.1 — reads lenient, writes strict, damaged bytes stay on disk — and this
unit contradicted it by replacing outright, so a file 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, and the divergence is the
interesting part. Marks REFUSE the write 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 mid-way through
copying — and a booth's own description is something its poster can say again.
One fixed quarantine name rather than a timestamped series: nothing prunes a
booth but the sweep, and the most recent damage is the only copy anyone opens.
INV-5 — unannounced and unreadable render DIFFERENT TEXT. Not merely
different styling: the words differ (unannounced / unreadable), so the
distinction survives a stylesheet change and a reader who cannot see colour. A
one-pixel difference would satisfy a looser wording and encode nothing, and the
point is that one of the two states is something somebody has to go and fix.