Files
booth/persistent-memory.d/2026-09-22-bug-hunt-panel.md
T
Vuong Hoang 026a1fc392 fix(marks): v0.2.2 — nine findings from the cross-frontier bug-hunt panel
`/heid-bug-hunt` on U2's diff, four arms, artifact-only. Eight findings were
real against live code; a ninth was already closed by v0.2.1 and is recorded as
declined. Full triage in persistent-memory.d/2026-09-22-bug-hunt-panel.md.

THE LOCK LIFECYCLE (4/4 convergent, and two defects in one place)

`_Locked.__exit__` unlinked `.marks.lock` on the no-op path so a booth that had
never been marked was left exactly as it was found. `flock` binds to an INODE:
unlinking it under a blocked waiter leaves that waiter holding an exclusive
lock on a deleted file while the next writer creates a fresh lock and takes it
immediately. Two processes then run the read-modify-write concurrently, the
later os.replace drops the earlier one's mark, and both obeyed the protocol.

The cleanup existed to protect the booth's TTL, and was failing at that too:
creating or removing a directory entry bumps the DIRECTORY's mtime, which is
what `_newest_mtime` seeds from. The guard's comment reasons about the lock
file's own mtime and misses that the directory moved underneath it.

One fix: never unlink the lock, exempt `.<name>.lock` dotfiles from
`_newest_mtime`, and restore the directory's mtime after creating one.

THE READ PATH'S BLAST RADIUS

`_clean_text` did `(text or "").replace(...)` and `marks_for` sorts on
`(created, id)`, so a stored `text` that was a dict or a `created` that was a
number raised out of the read path. `list_booths` reads every booth's marks on
every index load, so one hand-edited file returned 500 for `/` and `/healthz`
across all 25 booths. Guarded in two layers — a named type check and a
`_hydrate_safe` backstop that cannot raise — and an unreadable mark now renders
as ⚠ broken rather than as an empty note.

ALSO

- import_legacy_asks stamped `created` at whole-second resolution, so two
  sidecars from the same second lost the ordering the importer had just
  established and re-sorted alphabetically. Microseconds, per the stated
  `(mtime, name)` rule.
- The five mark-write routes ran a blocking flock on the event loop; they now
  dispatch through run_in_threadpool, asserted structurally like INV-1.
- `/answer` 500'd on a non-string `notes` form value where `/note` handled it.
- The inline-doc tile had a flag control and no note field.
- The marks panel was suppressed on any booth carrying a links.md.
- The viewer's arrow keys and Escape threw away a note being typed.

CLI

`booth marks` printed a traceback and exited 0 on a failed read, and `--wait`
emitted a whole JSON document per poll. `booth answer --wait` read a damaged
file as "not yet" and spun the full hour. Both now use real exit codes —
0 ok, 1 unanswered/timed-out, 2 no such pick, 3 unreadable — and `--wait`
prints once. `marks.read_error()` lets the CLI ask what the page must not: the
browser stays lenient, the machine consumer gets the truth.

`scripts/booth` had no tests; it has five now, run against the real script
under the system python3, which also makes them a live check on INV-1.

275 tests (253 before). Live service restarted, 25/25 booth pages verified 200.
2026-09-22 00:20:58 -07:00

80 lines
5.0 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# The U2 bug-hunt panel — full triage
**Date:** 2026-09-22 · **Thread:** `01M33XEC1H0298C0D968FWBN7A` ·
**Reply:** `01M33YZZ1VYGZ04JGNXNTBXDKS` · **Shipped as:** `v0.2.2`
`/heid-bug-hunt` on U2's diff (+2251/−632, 20 sections, 18 post-change
snapshots). Four arms — Gróa (Grok), Hulda (Codex), Regin (GLM-5.2), Kimi
(kimi-k3) — artifact-only, 4/4 clean transport. Heid adjudicated **9 findings
(6 bug / 3 robustness)**. Staleness was disclosed at build: `app.py` was edited
after the 06:38:52Z capture.
## Triage, five-category
### Category 1 — genuine add (8 taken, all shipped)
| # | finding | where | why it was real |
|---|---|---|---|
| 1 | Lock-inode split on the no-op unlink (**4/4 convergent**) | `marks._Locked` | `flock` binds to an inode; unlinking under a waiter destroys mutual exclusion silently |
| 2 | No-op lock churn resets the TTL via **directory** mtime | `marks._Locked` + `app._newest_mtime` | the guard's own comment reasons about the lock FILE's mtime; the directory is what the sweeper reads |
| 3 | Non-string `text` / `created` raise out of the read path | `marks._clean_text`, `marks_for` sort | `list_booths` reads every booth per page load → one bad file 500s `/` and `/healthz` |
| 4 | Legacy import stamped `created` at whole-second resolution | `marks.import_legacy_asks` | same-second sidecars re-sorted alphabetically, reversing the order the importer had just set — violates the stated `(mtime, name)` rule |
| 5 | `/answer` 500s on a non-string `notes` form value | `app.booth_answer` | the sibling `/note` guards it; same parser, same class of value, two answers |
| 6 | All five mark-write routes hold a blocking `flock` on the event loop | `app.py` | a contended lock freezes every route, not just the one request |
| 7 | CLI conflates a reader crash with "open" / "unanswered" | `scripts/booth` | `marks` printed a traceback and exited 0; `answer --wait` spun the full hour on a damaged file |
| 8 | The inline-doc tile had `markcontrols` and not `marknotes` | `booth.html` | flag a report, cannot say why — on the one item kind that is prose |
Two more taken on the same sweep, found while fixing the above rather than by
the panel: a broken mark of any shape now renders **⚠ broken** instead of as an
empty note (the rule `_hydrate` states for picks, applied to all three shapes),
and the marks panel is no longer suppressed on a booth that carries a
`links.md` *and* has marks.
### Category 3 — restatement of a settled prior (1, no change)
**Corrupt read → filtered writeback → silent deletion** (hulda F2, kimi F3,
gróa F4; Heid ranked it #3). **Already fixed in `v0.2.1`** by
`_read_raw_strict` + `MarksCorrupt` — reads lenient, writes strict. The panel
reviewed the pre-fix capture and the staleness was disclosed up front. Verified
against the current source before declining, not assumed.
This is the exact case the cross-frontier triage discipline warns about: a
confident, well-argued, four-arm-corroborated finding against code that no
longer exists. **Check what the peer actually read before treating an omission
or a defect claim as new.**
### Category 4 — out of place, parked (2)
- **Note-id recycling** (`note-1` reused after a withdrawal) lets a stale tab
delete a newer note. Real mechanism; needs two tabs and an interleaving, and
the Booth has one viewer. Non-reused ids are a schema change, not a patch.
- **Unvalidated flag / note targets** accumulate orphan marks. Targets come
from rendered items; the operator is the only writer through the browser.
### Category 5 — wrong-grounding (1)
**`delete_mark` can remove a pick, not only a note.** Framed as an
access-control divergence. There is no auth by design, and restricting it would
remove the only way to withdraw a pick that hydrates broken. Declined; the
docstring is the thing that was imprecise, not the behaviour.
## What the round is worth remembering for
1. **The two review gates stayed complementary a second time.** The contract
panel (2026-09-21) found three defects; this bug-hunt found eight more, with
**no overlap**. Both ran on the same unit. Neither substitutes.
2. **The panel beat the code's own comments three times.** The bundle's comments
are unusually honest and still wrong about what protected the TTL, and
"written atomically" sat next to a filter-then-replace. **A comment is a
claim, and a claim can be tested.**
3. **The headline bug class shipped with zero guard coverage, and both mutation
tables said so.** `test_a_no_op_write_does_not_touch_the_booth` asserted only
that `.marks.json` was absent — so removing the lock unlink, removing the
whole lock lifecycle, or bumping the directory clock all **SURVIVED** it. The
test asserted an artifact of the property instead of the property. The
replacement asserts `booth_age_seconds` directly, with a positive control (a
real mark still resets the clock) so the fix cannot overshoot into "marking
is never activity".
4. **`scripts/booth` had no tests at all** and two findings lived there. It has
five now, running the real script under the system `python3`.