Files
booth/tests/test_cli.py
T
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

349 lines
15 KiB
Python

"""`scripts/booth` — the surface every fleet session actually calls.
It had no tests at all, which the 2026-09-22 bug-hunt panel found the hard way:
its guard-strength table returned UNVERIFIED for every CLI claim because nothing
in the suite executes the script. Two of that round's findings live in here.
These run the real script under the real system `python3` with no venv, which
also makes them a live check on INV-1 (stdlib-only): a third-party import in
`marks.py` fails here the same way it fails on a fleet host.
"""
import json
import os
import pathlib
import subprocess
import pytest
SCRIPT = pathlib.Path(__file__).parent.parent / "scripts" / "booth"
# Exit codes the verbs promise. 0 is a successful read; a reader that CRASHED
# must never be one of the meaningful codes, or a caller cannot tell "no" from
# "broken" — which is the whole finding.
OK, UNANSWERED, NO_SUCH_PICK, READER_FAILED = 0, 1, 2, 3
def run(data, *args, **kw):
env = {**os.environ, "BOOTH_DATA_DIR": str(data), "BOOTH_URL": "http://booth.invalid"}
return subprocess.run([str(SCRIPT), *args], capture_output=True, text=True,
env=env, timeout=30, **kw)
@pytest.fixture
def booth(tmp_path):
b = tmp_path / "b"
b.mkdir()
return tmp_path, b
def _declare(booth_dir, mark_id="winner"):
import sys
sys.path.insert(0, str(pathlib.Path(__file__).parent.parent))
from booth.marks import declare_pick
declare_pick(booth_dir, mark_id,
{"prompt": "Which one?", "options": ["A", "B"]})
def test_marks_prints_one_json_document(booth):
"""`booth marks <name>` is a read. Its stdout is parsed by the session that
called it, so it has to be ONE document — and exit 0, because the read
succeeded. Whether a pick is open is in the payload's `open` list, which is
where a caller should read it from."""
data, b = booth
_declare(b)
r = run(data, "marks", "b")
assert r.returncode == OK, r.stderr
doc = json.loads(r.stdout)
assert doc["open"] == ["winner"]
def test_marks_wait_prints_once_not_once_per_poll(booth):
"""`--wait` polls every 2 s and printed the whole document on every pass, so
a capture held several concatenated JSON values and `jq` could not read any
of them. The wait is a wait; the print is the result."""
data, b = booth
_declare(b)
import sys
sys.path.insert(0, str(pathlib.Path(__file__).parent.parent))
from booth.marks import answer_pick
# Answer it after the first poll so --wait genuinely loops at least once.
r = subprocess.Popen([str(SCRIPT), "marks", "b", "--wait", "20"],
stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True,
env={**os.environ, "BOOTH_DATA_DIR": str(data),
"BOOTH_URL": "http://booth.invalid"})
import time
time.sleep(3)
answer_pick(b, "winner", "A")
out, err = r.communicate(timeout=30)
assert r.returncode == OK, err
json.loads(out) # ONE document, or this raises
def test_marks_reports_a_reader_failure_instead_of_printing_garbage(booth):
"""A traceback on stdout with exit 0 is the worst of both: the caller's `jq`
sees success and gets nothing. A read that could not happen is its own
answer and gets its own code."""
data, b = booth
(b / ".marks.json").write_bytes(b"\xff\xfe not utf-8 at all")
r = run(data, "marks", "b")
assert r.returncode == READER_FAILED, f"rc={r.returncode} out={r.stdout!r}"
def test_answer_distinguishes_a_crash_from_an_unanswered_pick(booth):
"""`answer` funnelled a reader crash and "not yet answered" through the same
exit 1, so `--wait` spun for the full hour on a broken file and then blamed
the operator for not answering."""
data, b = booth
_declare(b)
r = run(data, "answer", "b", "winner")
assert r.returncode == UNANSWERED
(b / ".marks.json").write_bytes(b"\xff\xfe not utf-8 at all")
r = run(data, "answer", "b", "winner", "--wait", "6")
assert r.returncode == READER_FAILED, (
"a crash was read as 'unanswered' and waited out the timeout"
)
def test_answer_on_a_note_id_says_no_such_pick(booth):
"""`answer` matched on id alone while the web route filters on shape, so a
note id was reported 'unanswered' and polled forever — a question that could
never be answered because it was never a question."""
data, b = booth
import sys
sys.path.insert(0, str(pathlib.Path(__file__).parent.parent))
from booth.marks import write_note
write_note(b, "a.png", "just a note")
r = run(data, "answer", "b", "note-1")
assert r.returncode == NO_SUCH_PICK
assert "no such pick" in r.stderr
# ---- U5: self-announcing booths ---------------------------------------------
def _manifest(booth_dir):
import sys
sys.path.insert(0, str(pathlib.Path(__file__).parent.parent))
from booth.manifest import read_manifest
return read_manifest(booth_dir)
def test_new_announces_the_booth(tmp_path):
"""`$ALTHING_HANDLE` is the whole provenance story: the session already has
it, so the booth can say who made it without anybody typing a name."""
env = {**os.environ, "ALTHING_HANDLE": "shutter-dev"}
r = subprocess.run([str(SCRIPT), "new", "r18-ab", "--why", "pick the winner"],
capture_output=True, text=True, timeout=30,
env={**env, "BOOTH_DATA_DIR": str(tmp_path),
"BOOTH_URL": "http://booth.invalid"})
assert r.returncode == 0, r.stderr
m = _manifest(tmp_path / "r18-ab")
assert m.handle == "shutter-dev"
assert m.why == "pick the winner"
def test_new_without_a_why_is_still_legal(tmp_path):
"""The flags are optional and existing call sites keep working. A booth
that says only who made it is still a booth that said something."""
r = subprocess.run([str(SCRIPT), "new", "scratch"], capture_output=True,
text=True, timeout=30,
env={**os.environ, "ALTHING_HANDLE": "booth-dev",
"BOOTH_DATA_DIR": str(tmp_path),
"BOOTH_URL": "http://booth.invalid"})
assert r.returncode == 0, r.stderr
m = _manifest(tmp_path / "scratch")
assert m.handle == "booth-dev" and m.why == ""
def test_add_announces_and_still_copies_the_files(tmp_path):
"""`add` is the verb most sessions actually use — it creates the booth AND
fills it — so the why has to ride on it or it rides nowhere."""
src = tmp_path / "src"
src.mkdir()
(src / "a.txt").write_text("content")
r = subprocess.run([str(SCRIPT), "add", "r18-ab", str(src / "a.txt"),
"--why", "second pass", "--title", "R18 A/B"],
capture_output=True, text=True, timeout=30,
env={**os.environ, "ALTHING_HANDLE": "booth-dev",
"BOOTH_DATA_DIR": str(tmp_path),
"BOOTH_URL": "http://booth.invalid"})
assert r.returncode == 0, r.stderr
assert (tmp_path / "r18-ab" / "a.txt").read_text() == "content"
m = _manifest(tmp_path / "r18-ab")
assert m.why == "second pass" and m.title == "R18 A/B"
def test_add_re_announcing_keeps_the_original_created(tmp_path):
"""The common shape: `new` opens the booth, `add` drops the second batch and
sharpens the why. The booth appeared once."""
env = {**os.environ, "ALTHING_HANDLE": "booth-dev",
"BOOTH_DATA_DIR": str(tmp_path), "BOOTH_URL": "http://booth.invalid"}
src = tmp_path / "a.txt"
src.write_text("x")
subprocess.run([str(SCRIPT), "new", "b", "--why", "first"], check=True,
capture_output=True, timeout=30, env=env)
first = _manifest(tmp_path / "b").created
subprocess.run([str(SCRIPT), "add", "b", str(src), "--why", "sharper"],
check=True, capture_output=True, timeout=30, env=env)
after = _manifest(tmp_path / "b")
assert after.created == first
assert after.why == "sharper"
def test_the_link_board_announces_itself_as_the_booths_own(tmp_path):
"""No exemption list. The standing board is made by the service and posted
to by seventeen handles, so no single agent owns it — `booth` is the
truthful answer, and it keeps the rule to one line."""
r = subprocess.run([str(SCRIPT), "link", "http://example.invalid", "a thing"],
capture_output=True, text=True, timeout=30,
env={**os.environ, "ALTHING_HANDLE": "booth-dev",
"BOOTH_DATA_DIR": str(tmp_path),
"BOOTH_URL": "http://booth.invalid"})
assert r.returncode == 0, r.stderr
m = _manifest(tmp_path / "links")
assert m is not None and m.handle == "booth"
assert m.why
def test_the_flags_can_sit_on_either_side_of_the_files(tmp_path):
"""`booth add b *.png --why "..."` and `booth add b --why "..." *.png` both
work. A glob is usually last and a flag usually after it, but nothing
enforces that and a session should not have to remember which."""
src = tmp_path / "a.png"
src.write_bytes(b"x")
env = {**os.environ, "ALTHING_HANDLE": "booth-dev",
"BOOTH_DATA_DIR": str(tmp_path), "BOOTH_URL": "http://booth.invalid"}
for name, args in (("after", ["add", "after", str(src), "--why", "w"]),
("before", ["add", "before", "--why", "w", str(src)])):
r = subprocess.run([str(SCRIPT), *args], capture_output=True, text=True,
timeout=30, env=env)
assert r.returncode == 0, r.stderr
assert _manifest(tmp_path / name).why == "w"
assert (tmp_path / name / "a.png").exists(), "the files stopped being copied"
def test_a_why_survives_quotes_and_non_ascii_and_is_flattened(tmp_path):
"""The reason this goes through manifest.py instead of printf-ing JSON from
the shell: a why containing a quote, a backslash or a newline is not an edge
case, it is a sentence somebody wrote. Newlines flatten because the field
renders inside a card's sub-line."""
r = subprocess.run(
[str(SCRIPT), "new", "b", "--why", 'he said "pick v3" — line1\nline2 · ünï'],
capture_output=True, text=True, timeout=30,
env={**os.environ, "ALTHING_HANDLE": "booth-dev",
"BOOTH_DATA_DIR": str(tmp_path), "BOOTH_URL": "http://booth.invalid"})
assert r.returncode == 0, r.stderr
why = _manifest(tmp_path / "b").why
assert why == 'he said "pick v3" — line1 line2 · ünï'
def test_a_flag_with_no_value_does_not_eat_the_booth_name(tmp_path):
"""`booth new b --why` with nothing after it must not consume `b` as the
value and then create a booth called nothing. Usage, and no directory."""
r = subprocess.run([str(SCRIPT), "new", "b", "--why"], capture_output=True,
text=True, timeout=30,
env={**os.environ, "BOOTH_DATA_DIR": str(tmp_path),
"BOOTH_URL": "http://booth.invalid"})
assert r.returncode == 2
assert "usage:" in r.stderr
assert not (tmp_path / "b").exists()
def test_a_bare_add_does_not_wipe_the_why_the_new_set(tmp_path):
"""`booth new x --why "..."` then `booth add x out/*.png` is THE sequence,
and the second call must not erase the first one's sentence. The module
distinguishes omitted from empty; the shell has to carry that distinction
across, which means an UNSET variable, not an empty one."""
env = {**os.environ, "ALTHING_HANDLE": "booth-dev",
"BOOTH_DATA_DIR": str(tmp_path), "BOOTH_URL": "http://booth.invalid"}
src = tmp_path / "a.png"
src.write_bytes(b"x")
subprocess.run([str(SCRIPT), "new", "b", "--why", "pick the denoiser",
"--title", "R18 A/B"],
check=True, capture_output=True, timeout=30, env=env)
subprocess.run([str(SCRIPT), "add", "b", str(src)],
check=True, capture_output=True, timeout=30, env=env)
m = _manifest(tmp_path / "b")
assert m.why == "pick the denoiser", "a bare `booth add` wiped the why"
assert m.title == "R18 A/B"
def test_an_explicitly_empty_why_still_clears_it(tmp_path):
"""Omitted means unchanged; supplied-and-empty means the poster meant to
take it back. Both have to be reachable from the shell."""
env = {**os.environ, "ALTHING_HANDLE": "booth-dev",
"BOOTH_DATA_DIR": str(tmp_path), "BOOTH_URL": "http://booth.invalid"}
subprocess.run([str(SCRIPT), "new", "b", "--why", "wrong"], check=True,
capture_output=True, timeout=30, env=env)
subprocess.run([str(SCRIPT), "new", "b", "--why", ""], check=True,
capture_output=True, timeout=30, env=env)
assert _manifest(tmp_path / "b").why == ""
def test_answer_and_marks_agree_about_what_open_means(tmp_path):
"""U2 made `_is_open` THE openness predicate — "nothing else may spell this
out" — and `booth answer`'s reader spelled it out anyway, as
`if m.answer is None`. So a PARTIALLY answered pick read as done to
`answer` and still-open to `marks --wait`: one verb returns the half-filled
form and the other blocks on the same booth at the same instant.
Found 2/4. The two verbs are the session's whole view of the loop, and a
session that asks both gets two answers.
"""
import sys
sys.path.insert(0, str(pathlib.Path(__file__).parent.parent))
from booth.marks import answer_pick, declare_pick
b = tmp_path / "b"
b.mkdir()
declare_pick(b, "batch", {
"title": "R18",
"questions": [
{"key": "q1", "prompt": "One?", "options": ["keep", "drop"]},
{"key": "q2", "prompt": "Two?", "options": ["keep", "drop"]},
],
})
answer_pick(b, "batch", {"q1": "keep", "q2": None}) # partial
env = {**os.environ, "BOOTH_DATA_DIR": str(tmp_path),
"BOOTH_URL": "http://booth.invalid"}
marks = subprocess.run([str(SCRIPT), "marks", "b"], capture_output=True,
text=True, timeout=30, env=env)
answer = subprocess.run([str(SCRIPT), "answer", "b", "batch"],
capture_output=True, text=True, timeout=30, env=env)
still_open = "batch" in json.loads(marks.stdout)["open"]
assert still_open, "a partial answer stopped counting as open"
assert answer.returncode == UNANSWERED, (
"`answer` called a partially-answered pick done while `marks` called it open"
)
def test_answer_does_not_poll_forever_on_a_pick_that_cannot_be_answered(tmp_path):
"""The mirror failure. A pick whose declaration went bad hydrates with
`error` set, which makes it NOT open — so `marks --wait` returns at once
while `answer --wait` polled the full hour against a form the web route
refuses with a 400. Nothing was ever going to land."""
b = tmp_path / "b"
b.mkdir()
(b / ".marks.json").write_text(json.dumps({
"version": 1,
"marks": [{"id": "broken", "shape": "pick", "declaration": {},
"error": "pick has no declaration",
"created": "2026-09-21T00:00:00.000000+00:00"}],
}))
r = subprocess.run([str(SCRIPT), "answer", "b", "broken", "--wait", "8"],
capture_output=True, text=True, timeout=40,
env={**os.environ, "BOOTH_DATA_DIR": str(tmp_path),
"BOOTH_URL": "http://booth.invalid"})
assert r.returncode != 0
assert "broken" in r.stderr.lower() or "cannot" in r.stderr.lower()