fix(r2): the heid bug-hunt panel (round "Nyx", 4/4) — triaged and folded

In-place client (base.html): - Saves are serialized: POST, re-fetch and swap complete before the next save starts, so an older snapshot can no longer land after a newer one. - A form already queued or in flight ignores another submit; a double-click writes one note. - Dirty controls (drafts, unsent radio choices) and disclosures carry by identity (form action + hidden ask/target/mark/f + name), not position. - Any non-tile structural difference, or a page with no region to swap, reloads instead of patching. Server and templates: - .seen is a JSON array read without following links or blocking, regular files of at most 1 MiB only; malformed, nested-too-deep or planted markers read as nothing seen. - landed_at reads symlinks by lstat and skips one unreadable entry instead of pinning the booth in "new". - The Desk counts flags on current items only; orphan flags are listed under the tray with an unmark form. - Agent-written bench and bookmark URLs link only when http(s). - Audio and video tiles carry a review link. - A rel the filesystem cannot represent is a 404, not a 500. - A non-finite Accept q-value fails to parse. - The standalone marks page has regions and updates in place. - The review's next arrow sits at the edge at phone width. Contract amended for each, plus an accepted-risks section (unlocked .seen read-modify-write, a planted .viewed symlink, Item.ordinal with no default). 741 passed. Each new browser test was mutation-checked against its fix; the serialization test forces the race with a held first refresh, since localhost alone never lost it.
2026-09-23 10:22:51 -07:00
parent fa5d46443d
commit 77833dc6d4
11 changed files with 622 additions and 82 deletions
@@ -113,7 +113,7 @@ def test_a_full_size_look_is_recorded_as_seen_and_a_non_item_is_not(tmp_path):
    for f in ("a.png", "c.png", "a.png", ".marks.lock"):
        c.get(f"/b/g/view?f={f}")
    assert read_seen(b) == {"a.png", "c.png"}
-    assert (b / ".seen").read_text() == "a.png\nc.png\n"      # sorted, deduplicated
+    assert (b / ".seen").read_text() == '["a.png", "c.png"]'   # sorted, deduplicated, JSON


 def test_seen_is_pruned_to_live_items_at_the_next_write(tmp_path):
@@ -673,3 +673,127 @@ def test_a_full_size_look_also_counts_as_looking_at_the_booth(tmp_path):
    assert not (b / ".viewed").exists()
    _client(tmp_path).get("/b/g/view?f=a.png")
    assert (b / ".viewed").exists() and (b / ".seen").exists()
+
+
+def test_a_flag_on_a_file_that_is_gone_stays_visible_and_withdrawable(tmp_path):
+    """Nyx N3 (2/4): the tray only shows live items, and `tray` being always
+    defined killed the old list fallback — so a flag whose file was deleted
+    rendered NOWHERE on the booth page while the Desk still counted it. It is
+    now listed apart, with its withdraw control; the Desk counts live items."""
+    b = _booth(tmp_path, "g", {"a.png": PNG, "b.png": PNG})
+    set_flag(b, "a.png", True)
+    set_flag(b, "b.png", True)
+    (b / "b.png").unlink()
+    c = _client(tmp_path)
+    aside = _region(c.get("/b/g/").text, "verdict")
+    orphans = re.search(r'class="orphan-flags".*?</ul>', aside, re.S)
+    assert orphans and "b.png" in orphans.group(0)
+    assert 'action="/b/g/unmark"' in orphans.group(0)
+    row = re.search(r'data-booth="g".*?</article>', c.get("/").text, re.S).group(0)
+    assert "1 flagged" in row
+
+
+def test_a_planted_fifo_or_device_seen_marker_cannot_hang_the_review(tmp_path):
+    """Nyx N4 (3/4): `.seen` was read with an unbounded, symlink-following
+    read_text(). A FIFO with no writer blocked the worker forever; a symlink to
+    /dev/zero read until memory ran out. The read now refuses anything that is
+    not a small regular file, without following a link."""
+    import os
+    import threading
+    b = _booth(tmp_path, "g", {"a.png": PNG})
+    os.mkfifo(b / ".seen")
+    out = {}
+    t = threading.Thread(target=lambda: out.setdefault(
+        "r", _client(tmp_path).get("/b/g/view?f=a.png")), daemon=True)
+    t.start()
+    t.join(timeout=5)
+    assert "r" in out, "the review hung on a FIFO .seen"
+    assert out["r"].status_code == 200
+    h = _booth(tmp_path, "h", {"a.png": PNG})
+    (h / ".seen").symlink_to("/dev/zero")
+    assert _client(tmp_path).get("/b/h/view?f=a.png").status_code == 200
+
+
+def test_seen_round_trips_names_with_spaces_and_newlines(tmp_path):
+    """Nyx (groa, hulda): one stripped line per rel lost ` a.png` and split a
+    name holding a newline into two identities. The marker is a JSON array."""
+    from booth.items import read_seen
+    b = _booth(tmp_path, "g", {"a.png": PNG, " a.png": PNG, "x\ny.png": PNG})
+    c = _client(tmp_path)
+    c.get("/b/g/view", params={"f": " a.png"})
+    c.get("/b/g/view", params={"f": "x\ny.png"})
+    assert read_seen(b) == {" a.png", "x\ny.png"}
+
+
+
+def test_a_deeply_nested_seen_marker_reads_as_nothing_seen(tmp_path):
+    """A JSON array nested past the parser's recursion limit raises
+    RecursionError, which is not a ValueError: a 100 KB file of `[` planted as
+    `.seen` escaped the never-raises read and 500'd every review of the booth.
+    It reads as nothing seen, and the next look rewrites it."""
+    from booth.items import read_seen
+    b = _booth(tmp_path, "g", {"a.png": PNG})
+    (b / ".seen").write_text("[" * 100_000)
+    assert read_seen(b) == set()
+    assert _client(tmp_path).get("/b/g/view?f=a.png").status_code == 200
+    assert read_seen(b) == {"a.png"}
+
+def test_the_content_clock_reads_the_booth_not_what_its_links_point_at(tmp_path):
+    """Nyx (groa, regin): stat() followed a symlink, so a link to a busy file
+    outside the booth made the booth read as newly delivered on every load; and
+    one unreadable entry (a symlink loop) made the whole booth read as landed
+    NOW, forever. The link's own mtime counts; an unreadable entry is skipped."""
+    import os
+    t0 = time.time() - 10_000    # in the PAST: a future stamp outranks every real write and hides the bug
+    outside = tmp_path / "busy.log"
+    outside.write_text("x")
+    b = _booth(tmp_path, "g", {"a.png": PNG})
+    (b / "linked.png").symlink_to(outside)
+    (b / "loop.png").symlink_to(b / "loop.png")
+    for p in (b / "a.png", b / "linked.png", b / "loop.png"):
+        os.utime(p, (t0, t0), follow_symlinks=False)
+    _at(b, t0)
+    c = _client(tmp_path)
+    c.get("/b/g/")                      # look at it
+    os.utime(outside, None)             # the outside file keeps moving
+    assert _desk(c.get("/").text).get("rest") == ["g"]
+
+
+def test_a_nul_in_the_review_path_is_a_404_not_a_500(tmp_path):
+    """Nyx (groa, seat-probed): Path raises ValueError on an embedded NUL, and
+    the route caught only OSError. Every other hostile `f` is a 404."""
+    _booth(tmp_path, "g", {"a.png": PNG})
+    assert _client(tmp_path).get("/b/g/view?f=a%00.png").status_code == 404
+
+
+@pytest.mark.parametrize("q", ["inf", "1e999", "nan", "-inf"])
+def test_a_non_finite_q_is_malformed(q):
+    """Nyx (regin): float() parses inf and 1e999, and inf > 0 — a malformed
+    header slipped through to the 204. Non-finite q is malformed: False."""
+    from booth.app import wants_json
+    assert wants_json(f"application/json;q={q}") is False
+
+
+def test_a_sound_only_booth_can_open_the_review(tmp_path):
+    """Nyx (groa): only the image tile linked to view?f=, so a booth of tracks
+    had no way into the review, the tape or `.seen`. Every media tile links in
+    (and Enter on the grid cursor follows that link)."""
+    _booth(tmp_path, "g", {"a.mp3": b"ID3", "b.webm": b"\x1aE"})
+    body = _client(tmp_path).get("/b/g/").text
+    for rel in ("a.mp3", "b.webm"):
+        fig = re.search(r'<figure[^>]*data-item="%s".*?</figure>' % re.escape(rel), body, re.S).group(0)
+        assert f'href="view?f={rel}"' in fig, rel
+
+
+def test_the_desk_never_makes_a_non_web_url_clickable(tmp_path):
+    """Nyx (kimi): bookmark and bench URLs are agent-written and land in href.
+    Autoescape does nothing about a `javascript:` scheme. The Desk links only
+    http(s) and shows anything else as plain text."""
+    rows = (_link("evil", "javascript:alert`1`")
+            + _link("fine", "https://example.test/"))
+    board = _booth(tmp_path, "links", {"links.md": rows.encode()})
+    (board / ".forever").write_bytes(b"")
+    body = _client(tmp_path).get("/").text
+    panel = re.search(r'data-panel="bookmarks".*?</section>', body, re.S).group(0)
+    assert 'href="javascript:' not in panel
+    assert 'href="https://example.test/"' in panel and "evil" in panel
@@ -250,3 +250,127 @@ def test_on_a_narrow_screen_flags_and_notes_fold_and_on_a_wide_one_they_show(bro
            page.locator(".verdict summary.v-fold-head").first.click()
            assert tray.is_visible()
        ctx.close()
+
+
+# ---- fixups from the heid bug-hunt panel (round "Nyx") ------------------------
+
+def test_the_link_board_still_confirms_before_removing_a_row(browser, live):
+    """Nyx N1 (2/4): the R2 rewrite of booth.html's scripts deleted the board's
+    multi-select + confirmation script along with the handlers it replaced.
+    Removing a row is destructive; the confirm naming it must still stand in
+    front of the POST, and select-all must still select."""
+    base, root = live
+    board = root / "links"
+    board.mkdir()
+    (board / "links.md").write_text(
+        "- [one](http://x/1) <sub>· a · 2026-09-01 10:00</sub>\n"
+        "- [two](http://x/2) <sub>· a · 2026-09-01 10:01</sub>\n")
+    page = browser.new_page()
+    page.goto(f"{base}/b/links/", wait_until="networkidle")
+    dialogs = []
+    page.on("dialog", lambda d: (dialogs.append(d.message), d.dismiss()))
+    page.locator(".board-rm-btn").first.click()
+    page.wait_for_timeout(300)
+    page.locator("#board-selall").check()
+    ticked = page.eval_on_selector_all(".board-check", "els => els.filter(e => e.checked).length")
+    page.close()
+    assert dialogs and "Remove this link?" in dialogs[0]
+    assert ticked == 2
+    assert (board / "links.md").read_text().count("- [") == 2, "a dismissed confirm removed nothing"
+
+
+def test_an_unsaved_choice_survives_a_save_elsewhere_and_a_double_click_writes_once(browser, live):
+    """Nyx N5 (hulda, regin, groa): a picked-but-unsent radio was reset by any
+    other in-place save, drafts were matched by POSITION, and a double-click on
+    Add note wrote two notes. Now: dirty controls carry by identity, and a form
+    already in flight ignores a second submit."""
+    from booth.marks import declare_pick, marks_for
+    base, root = live
+    b = _set(root, 3)
+    declare_pick(b, "q", {"prompt": "Which?", "options": ["x", "y"]})
+    page = browser.new_page(viewport={"width": 1400, "height": 900})
+    page.goto(f"{base}/b/g/", wait_until="networkidle")
+    page.locator('.verdict input[type=radio][value="y"]').check()
+    page.locator('figure.item[data-item="02.png"] .flagtoggle button').click()
+    page.wait_for_selector('figure.item.is-flagged[data-item="02.png"]', timeout=10000)
+    assert page.locator('.verdict input[type=radio][value="y"]').is_checked()
+
+    page.locator(".verdict .mark-add textarea").fill("once")
+    page.locator(".verdict .mark-add button").dblclick()
+    page.wait_for_timeout(1500)
+    page.close()
+    assert [m.text for m in marks_for(b) if m.shape == "note"] == ["once"]
+
+
+# The first page refresh after a save is held back 1s in the CLIENT: the server
+# renders it at once (so it carries only the first flag) and the browser sees
+# it late. Localhost alone never loses that race, so without the hold the test
+# passed with sequencing deleted — it has to be forced to be a control.
+_HOLD_FIRST_REFRESH = """
+(function () {
+  var real = window.fetch, n = 0;
+  window.fetch = function (u, o) {
+    var p = real.apply(this, arguments);
+    if ((!o || !o.method || o.method === 'GET') && n++ === 0) {
+      return p.then(function (r) {
+        return new Promise(function (res) { setTimeout(function () { res(r); }, 1000); });
+      });
+    }
+    return p;
+  };
+})();
+"""
+
+
+def test_quick_successive_flags_all_show(browser, live):
+    """Nyx (hulda): with no sequencing, an older refresh landing after a newer
+    one showed the newer flag as gone. Saves are serialized."""
+    base, root = live
+    _set(root, 4)
+    page = browser.new_page(viewport={"width": 1400, "height": 900})
+    page.add_init_script(_HOLD_FIRST_REFRESH)
+    page.goto(f"{base}/b/g/", wait_until="networkidle")
+    for rel in ("01.png", "02.png", "03.png"):
+        page.locator(f'figure.item[data-item="{rel}"] .flagtoggle button').click()
+        page.wait_for_timeout(150)
+    page.wait_for_function(
+        "document.querySelectorAll('figure.item.is-flagged').length === 3", timeout=10000)
+    page.wait_for_timeout(1500)
+    n = page.locator("figure.item.is-flagged").count()
+    page.close()
+    assert n == 3
+
+
+def test_the_standalone_marks_page_updates_in_place(browser, live):
+    """Nyx (kimi): the marks page's forms are in-place, but the page had no
+    region, so an answer saved and the page never showed it."""
+    from booth.marks import declare_pick
+    base, root = live
+    b = _set(root, 1)
+    declare_pick(b, "q", {"prompt": "Which?", "options": ["x", "y"]})
+    page = browser.new_page()
+    page.goto(f"{base}/b/g/marks", wait_until="networkidle")
+    page.evaluate("window.__same_page = 1")
+    page.locator('input[type=radio][value="x"]').check()
+    page.locator(".mark-submit").click()
+    page.wait_for_selector(".mark.is-answered", timeout=10000)
+    # In place, not the reload fallback: the page's own window survived.
+    same = page.evaluate("window.__same_page === 1")
+    page.close()
+    assert same
+
+
+def test_the_next_arrow_clears_the_rail_only_beside_it(browser, live):
+    """Nyx: view.html's bare `.vnext{right:360px}` came later in the page than
+    base.html's narrow override and won it, parking the arrow 360px in from
+    the edge of a phone. Wide: it clears the rail. Narrow: it sits at the edge."""
+    base, root = live
+    _set(root, 3)
+    rights = {}
+    for w in (1400, 390):
+        page = browser.new_page(viewport={"width": w, "height": 900})
+        page.goto(f"{base}/b/g/view?f=01.png", wait_until="networkidle")
+        rights[w] = page.evaluate(
+            "getComputedStyle(document.querySelector('.vnav.vnext')).right")
+        page.close()
+    assert rights == {1400: "360px", 390: "0px"}