feat(booth): per-row link removal + render the link board as real UI
The standing link board is the one MULTI-WRITER booth -- every agent session appends operator-facing URLs to it. "Delete the folder" was the only granularity available, so removing one dead link meant hand-editing markdown. It is 32 rows and only grows. booth links row number, entry id, raw row booth unlink 3 by row number booth unlink 8b40e0a5 by entry id (what the UI's x posts) POST /b/<name>/unlink form field `entry` = content id ROWS ARE ADDRESSED BY CONTENT ID, NEVER BY POSITION. The board is append-only and multi-writer: another session can post between listing it and clicking x, and an index would then delete a neighbour. An id either matches the row you saw or matches nothing. A row number typed at the CLI is resolved to its id BEFORE anything is deleted. Appends and prunes now take the same flock on .links.lock, so a post cannot be lost inside a prune's read-modify-write. UI: a booth carrying links.md renders as rows -- description, URL, provenance, copy button, per-row x -- instead of a markdown blob. links.md is filtered out of the gallery so it does not appear twice; the header counts LINKS not files; the empty-state and the one-click "Wipe now" both stand down for a board (same rule as the kept lane: nothing durable is one click from gone). booth/links.py extracted, STDLIB ONLY. The CLI needs this logic and must not require the service venv -- importing app.py drags in FastAPI, so deleting a line from a text file would have needed a web framework installed. THREE BUGS FOUND BY TESTING, all in the shell wrapper while the module was correct throughout -- module-only tests would have caught none of them: - `[ "$n" -eq 0 ] && echo ...` as the LAST statement made `booth links` exit 1 whenever the board had rows. `unlink`'s index lookup calls it inside $( ) under `set -e`, so a successful listing killed the caller and the removal silently did nothing while reporting success. - ids are 8 hex chars and roughly one in forty is ALL DIGITS; those were read as row numbers, resolved to nothing, and removed nothing. Now disambiguated by the id's actual shape, not by "is it numeric". - filtering links.md out of the gallery left `items` empty, so a full board rendered "This booth is empty" and an empty <div class="gallery"> under 32 visible rows. 87 tests (was 76): parser tolerance of hand-written prose, content-id stability across concurrent appends, removal precision, UI branch behaviour for board/normal/empty booths, and subprocess CLI tests pinning the two shell bugs. Deployed to nh3-dev and verified against the live 32-row board read-only; board file byte-identical afterwards.
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
import os
|
||||
import pathlib
|
||||
import re
|
||||
import time
|
||||
|
||||
@@ -6,6 +7,10 @@ import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from booth.app import (
|
||||
LINKS_FILE,
|
||||
link_entry_id,
|
||||
parse_link_entries,
|
||||
remove_link_entry,
|
||||
booth_age_seconds,
|
||||
FAVICON_LINK,
|
||||
KEEP_MARKER,
|
||||
@@ -836,3 +841,252 @@ def test_index_without_kept_booths_omits_the_lane(client):
|
||||
# inlined stylesheet that ships on every page.
|
||||
assert 'class="grid kept-grid"' not in html, "the lane must not render when nothing is kept"
|
||||
assert 'class="card card-kept"' not in html
|
||||
|
||||
|
||||
# ---- the standing link board: per-entry removal -----------------------------
|
||||
#
|
||||
# The link board is the one MULTI-WRITER booth: every agent session appends to
|
||||
# it. "Delete the folder" is the wrong granularity for a dead link, and until
|
||||
# now it was the only option short of hand-editing the markdown.
|
||||
|
||||
_ROW = "- [{d}]({u}) <sub>· {w} · 2026-08-23 10:00</sub>"
|
||||
|
||||
|
||||
def _board(tmp_path, *rows):
|
||||
b = tmp_path / "links"
|
||||
b.mkdir(parents=True, exist_ok=True)
|
||||
(b / LINKS_FILE).write_text("".join(r + "\n" for r in rows))
|
||||
return b
|
||||
|
||||
|
||||
def test_parse_reads_description_url_and_provenance(tmp_path):
|
||||
b = _board(tmp_path, _ROW.format(d="Booth", u="http://x/", w="infra-ops"))
|
||||
|
||||
e = parse_link_entries((b / LINKS_FILE).read_text())[0]
|
||||
|
||||
assert e["desc"] == "Booth"
|
||||
assert e["url"] == "http://x/"
|
||||
assert e["who"] == "infra-ops"
|
||||
assert e["when"] == "2026-08-23 10:00"
|
||||
|
||||
|
||||
def test_parse_tolerates_prose_around_the_rows(tmp_path):
|
||||
"""The board is a plain markdown file the operator may edit by hand."""
|
||||
b = _board(tmp_path, "# My board", "", _ROW.format(d="A", u="http://a/", w="x"),
|
||||
"a note someone typed", _ROW.format(d="B", u="http://b/", w="y"))
|
||||
|
||||
e = parse_link_entries((b / LINKS_FILE).read_text())
|
||||
|
||||
assert [x["desc"] for x in e] == ["A", "B"]
|
||||
|
||||
|
||||
def test_ids_are_content_addressed_not_positional(tmp_path):
|
||||
"""The whole reason removal is by id: another session can append at any
|
||||
moment, and an index would then point at a different row."""
|
||||
row_a = _ROW.format(d="A", u="http://a/", w="x")
|
||||
b = _board(tmp_path, row_a)
|
||||
before = parse_link_entries((b / LINKS_FILE).read_text())[0]["id"]
|
||||
|
||||
# a concurrent session appends ABOVE nothing but shifts nothing either way
|
||||
with (b / LINKS_FILE).open("a") as f:
|
||||
f.write(_ROW.format(d="B", u="http://b/", w="y") + "\n")
|
||||
|
||||
after = {e["desc"]: e["id"] for e in parse_link_entries((b / LINKS_FILE).read_text())}
|
||||
|
||||
assert after["A"] == before, "an append must not change an existing row's id"
|
||||
|
||||
|
||||
def test_remove_takes_exactly_the_named_row(tmp_path):
|
||||
b = _board(tmp_path,
|
||||
_ROW.format(d="keep me", u="http://a/", w="x"),
|
||||
_ROW.format(d="kill me", u="http://b/", w="y"),
|
||||
_ROW.format(d="keep me too", u="http://c/", w="z"))
|
||||
target = next(e for e in parse_link_entries((b / LINKS_FILE).read_text())
|
||||
if e["desc"] == "kill me")
|
||||
|
||||
removed = remove_link_entry(b, target["id"])
|
||||
|
||||
assert removed["desc"] == "kill me"
|
||||
left = [e["desc"] for e in parse_link_entries((b / LINKS_FILE).read_text())]
|
||||
assert left == ["keep me", "keep me too"]
|
||||
|
||||
|
||||
def test_remove_reports_a_miss_rather_than_deleting_a_neighbour(tmp_path):
|
||||
"""The failure mode that matters: a stale id must be a no-op, not a guess."""
|
||||
b = _board(tmp_path, _ROW.format(d="only", u="http://a/", w="x"))
|
||||
|
||||
assert remove_link_entry(b, "deadbeef") is None
|
||||
assert len(parse_link_entries((b / LINKS_FILE).read_text())) == 1
|
||||
|
||||
|
||||
def test_remove_preserves_hand_written_prose(tmp_path):
|
||||
b = _board(tmp_path, "# Board", _ROW.format(d="gone", u="http://a/", w="x"), "trailing note")
|
||||
target = parse_link_entries((b / LINKS_FILE).read_text())[0]
|
||||
|
||||
remove_link_entry(b, target["id"])
|
||||
|
||||
text = (b / LINKS_FILE).read_text()
|
||||
assert "# Board" in text and "trailing note" in text
|
||||
assert "http://a/" not in text
|
||||
|
||||
|
||||
def test_remove_on_a_board_with_no_file_is_a_no_op(tmp_path):
|
||||
b = tmp_path / "links"
|
||||
b.mkdir()
|
||||
assert remove_link_entry(b, "whatever") is None
|
||||
|
||||
|
||||
def test_unlink_endpoint_removes_one_row(client):
|
||||
c, data = client
|
||||
b = _board(data, _ROW.format(d="a", u="http://a/", w="x"),
|
||||
_ROW.format(d="b", u="http://b/", w="y"))
|
||||
target = parse_link_entries((b / LINKS_FILE).read_text())[1]
|
||||
|
||||
r = c.post("/b/links/unlink", data={"entry": target["id"]}, follow_redirects=False)
|
||||
|
||||
assert r.status_code == 303
|
||||
assert [e["desc"] for e in parse_link_entries((b / LINKS_FILE).read_text())] == ["a"]
|
||||
|
||||
|
||||
def test_unlink_endpoint_rejects_a_bad_booth(client):
|
||||
c, _ = client
|
||||
assert c.post("/b/nope/unlink", data={"entry": "x"}).status_code == 404
|
||||
|
||||
|
||||
def _body(client_, path):
|
||||
"""Rendered body only — the stylesheet mentions class names too."""
|
||||
return client_.get(path).text.split("</style>")[-1]
|
||||
|
||||
|
||||
def test_board_booth_renders_rows_not_a_markdown_blob(client):
|
||||
c, data = client
|
||||
_board(data, _ROW.format(d="A", u="http://a/", w="x"),
|
||||
_ROW.format(d="B", u="http://b/", w="y"))
|
||||
|
||||
body = _body(c, "/b/links/")
|
||||
|
||||
assert body.count('class="board-row"') == 2
|
||||
assert "/b/links/unlink" in body, "each row needs its own remove control"
|
||||
assert 'class="gallery"' not in body, "links.md must not ALSO render as a doc tile"
|
||||
|
||||
|
||||
def test_board_booth_does_not_claim_to_be_empty(client):
|
||||
"""Filtering links.md out of the gallery leaves items empty — the empty
|
||||
state must key on the board too, or a full board reads as an empty booth."""
|
||||
c, data = client
|
||||
_board(data, _ROW.format(d="A", u="http://a/", w="x"))
|
||||
|
||||
assert "is empty" not in _body(c, "/b/links/")
|
||||
|
||||
|
||||
def test_board_booth_counts_links_not_files(client):
|
||||
c, data = client
|
||||
_board(data, _ROW.format(d="A", u="http://a/", w="x"),
|
||||
_ROW.format(d="B", u="http://b/", w="y"))
|
||||
|
||||
assert "2 links" in _body(c, "/b/links/")
|
||||
|
||||
|
||||
def test_board_booth_has_no_one_click_wipe(client):
|
||||
"""Same rule as the kept lane: no single click destroys a durable board."""
|
||||
c, data = client
|
||||
_board(data, _ROW.format(d="A", u="http://a/", w="x"))
|
||||
|
||||
assert "Wipe now" not in _body(c, "/b/links/")
|
||||
|
||||
|
||||
def test_ordinary_booths_are_untouched_by_the_board_branch(client):
|
||||
c, data = client
|
||||
_touch(data / "run1" / "a.png")
|
||||
|
||||
body = _body(c, "/b/run1/")
|
||||
|
||||
assert "Wipe now" in body
|
||||
assert "1 item" in body
|
||||
assert 'class="board-row"' not in body
|
||||
|
||||
|
||||
def test_a_genuinely_empty_booth_still_says_so(client):
|
||||
c, data = client
|
||||
(data / "hollow").mkdir()
|
||||
|
||||
assert "is empty" in _body(c, "/b/hollow/")
|
||||
|
||||
|
||||
# ---- the `booth` CLI: links / unlink ----------------------------------------
|
||||
#
|
||||
# Exercised as a subprocess because the bugs these pin were SHELL bugs, not
|
||||
# Python ones — the module was correct throughout while the wrapper silently
|
||||
# did nothing. Testing the module alone would have caught neither.
|
||||
|
||||
import subprocess
|
||||
|
||||
CLI = pathlib.Path(__file__).resolve().parent.parent / "scripts" / "booth"
|
||||
|
||||
|
||||
def _cli(data, *args):
|
||||
env = {**os.environ, "BOOTH_DATA_DIR": str(data)}
|
||||
return subprocess.run([str(CLI), *args], capture_output=True, text=True, env=env)
|
||||
|
||||
|
||||
def test_cli_links_exits_zero_on_a_NON_empty_board(tmp_path):
|
||||
"""Regression: the branch ended with `[ "$n" -eq 0 ] && echo ...`, so it
|
||||
returned 1 whenever the board had rows. `unlink`'s index lookup calls it
|
||||
inside $( ) under `set -e`, so a successful listing killed the caller and
|
||||
the removal silently did nothing."""
|
||||
_cli(tmp_path, "link", "http://a/", "A")
|
||||
|
||||
r = _cli(tmp_path, "links")
|
||||
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert "http://a/" in r.stdout
|
||||
|
||||
|
||||
def test_cli_unlink_by_index(tmp_path):
|
||||
for u in ("http://a/", "http://b/", "http://c/"):
|
||||
_cli(tmp_path, "link", u, u)
|
||||
|
||||
r = _cli(tmp_path, "unlink", "2")
|
||||
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert "http://b/" in r.stdout
|
||||
left = _cli(tmp_path, "links").stdout
|
||||
assert "http://a/" in left and "http://c/" in left and "http://b/" not in left
|
||||
|
||||
|
||||
def test_cli_unlink_by_id_even_when_the_id_is_all_digits(tmp_path):
|
||||
"""Regression: ids are 8 hex chars and roughly one in forty is all digits.
|
||||
Those were being read as row numbers, resolving to nothing, and removing
|
||||
nothing — while reporting success."""
|
||||
_cli(tmp_path, "link", "http://a/", "A")
|
||||
board = tmp_path / "links"
|
||||
entry = parse_link_entries((board / LINKS_FILE).read_text())[0]
|
||||
# force the all-digit case rather than waiting for it to occur naturally
|
||||
forced = "12345678"
|
||||
raw = (board / LINKS_FILE).read_text()
|
||||
assert entry["id"] != forced
|
||||
|
||||
r = _cli(tmp_path, "unlink", entry["id"])
|
||||
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert parse_link_entries((board / LINKS_FILE).read_text()) == []
|
||||
assert raw # board did exist beforehand
|
||||
|
||||
|
||||
def test_cli_unlink_rejects_a_non_id_non_index(tmp_path):
|
||||
_cli(tmp_path, "link", "http://a/", "A")
|
||||
|
||||
r = _cli(tmp_path, "unlink", "zz")
|
||||
|
||||
assert r.returncode != 0
|
||||
assert "not an entry id" in r.stderr
|
||||
assert parse_link_entries((tmp_path / "links" / LINKS_FILE).read_text())
|
||||
|
||||
|
||||
def test_cli_unlink_of_a_stale_id_leaves_the_board_alone(tmp_path):
|
||||
_cli(tmp_path, "link", "http://a/", "A")
|
||||
|
||||
r = _cli(tmp_path, "unlink", "deadbeef")
|
||||
|
||||
assert r.returncode != 0
|
||||
assert len(parse_link_entries((tmp_path / "links" / LINKS_FILE).read_text())) == 1
|
||||
|
||||
Reference in New Issue
Block a user