From 84e778d5cdd1bb5cded8a7c0238797c17d48666c Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Sun, 16 Aug 2026 15:29:11 +0200 Subject: feat(hub): chat, presence, a Profile page, and downloads that do not freeze MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Chat opens on the newest hundred messages, loads fifty older on demand with the reading position anchored — the distance from the *bottom*, since everything above the viewport just grew — and follows new messages only when the reader was already at the end. Day separators, sender grouping, an unread marker, and a jump-to-latest pill. Messages are keyed by id: index keys plus prepending makes Preact reuse the wrong bubbles. A presence dot per group in the sidebar, three states, each backed by something: the hub's registry, or a connection this browser made or failed to make. Never colour alone — red and green are the pair colour-blind readers cannot separate — so each dot carries a title and an aria-label. Profile is split out of Settings: identity, node link, pinned node identities and account deletion. Mixing them put an irreversible button two scrolls under a theme picker. The create-group page loses its centred 520 px card, which left 190 px of margin either side, and its two button panels become a radio group — a button conveys no chosen state to a screen reader, and side by side they read as two independent actions rather than one either/or. The Files toolbar shows its actions as icon buttons the moment Select is on, disabled when they do not apply rather than appearing and vanishing. On a phone the right-hand group could not wrap and ran 130 px off the screen. Streamed downloads no longer freeze after one chunk. `registration.active` says a worker exists, not that this page is controlled by it — and an uncontrolled page's requests never reach its fetch handler, so the worker took the stream and was never asked for it, leaving `writer.write()` waiting on backpressure that would never lift. The page now requires control and the worker confirms it actually served the request before the sink is trusted. Fixed on the way: `setActionsOpen` outlived the state it belonged to and threw on every Files action; the chat scrollbar stopped short of the bottom; the owner's row sat lower than the rest; About showed a version hardcoded two releases ago. Co-Authored-By: Claude Opus 5 --- packages/meshbay-hub/tests/test_downloads.py | 41 +++- .../meshbay-hub/tests/test_layout_responsive.py | 159 +++++++++++++++ packages/meshbay-hub/tests/test_spa_ordering.py | 7 +- .../meshbay-hub/tests/test_transport_contracts.py | 218 +++++++++++++++++++++ .../meshbay-hub/tests/test_video_stream_switch.py | 160 +++++++++++++++ 5 files changed, 582 insertions(+), 3 deletions(-) create mode 100644 packages/meshbay-hub/tests/test_layout_responsive.py create mode 100644 packages/meshbay-hub/tests/test_transport_contracts.py create mode 100644 packages/meshbay-hub/tests/test_video_stream_switch.py (limited to 'packages/meshbay-hub/tests') diff --git a/packages/meshbay-hub/tests/test_downloads.py b/packages/meshbay-hub/tests/test_downloads.py index 41d61bf..85bf488 100644 --- a/packages/meshbay-hub/tests/test_downloads.py +++ b/packages/meshbay-hub/tests/test_downloads.py @@ -161,6 +161,45 @@ def test_backpressure_is_real(tmp_path): src = DOWNLOADS.read_text() fn = src[src.index("export async function openStreamedDownload"):] assert "new TransformStream()" in fn - assert "[readable]" in fn, "the readable half must be transferred, not copied" + # The transfer list may carry more than the stream — a reply port rides + # along now — so this asserts that `readable` is transferred, not the exact + # shape of the list. + transfer = fn[fn.index("worker.postMessage("):] + transfer = transfer[transfer.index("["):transfer.index("]") + 1] + assert "readable" in transfer, "the readable half must be transferred, not copied" assert "writer.write(bytes)" in fn assert "return null" in fn, "a browser that cannot transfer streams must say so" + + +def test_the_streamed_path_gives_up_rather_than_blocking_for_ever(): + """Reported 2026-08-16: a download frozen at exactly one chunk. + + The writable half applies real backpressure, which is the whole point — and + the trap. If nothing ever reads the readable half, `writer.write()` waits + for room that never comes, and the transfer stops dead after the stream's + internal queue fills. Two ways that happens on a phone: the page is not yet + *controlled* by the worker, so the iframe's request is never handed to its + fetch handler; or the browser refuses a download started from a hidden + iframe. Both are silent. + + So the worker confirms that it actually answered, and this path reports + failure instead of returning a sink nobody drains. + """ + src = DOWNLOADS.read_text() + fn = src[src.index("export async function openStreamedDownload"):] + assert "mbdl-serving" in fn, "the worker has to confirm it served the request" + assert "Promise.race" in fn, "the confirmation needs a deadline" + assert "writable.abort" in fn, "give up cleanly so the caller can fall back" + + sw = (DOWNLOADS.parent / "sw.js").read_text() + assert "mbdl-serving" in sw, "and the worker has to send that confirmation" + + +def test_an_uncontrolled_page_is_not_treated_as_ready(): + """`registration.active` says a worker exists, not that it will see our fetch.""" + src = DOWNLOADS.read_text() + fn = src[src.index("async function serviceWorker()"):] + fn = fn[:fn.index("\n}")] + assert "navigator.serviceWorker.controller" in fn + assert "controllerchange" in fn, ( + "control can arrive a tick after registration; waiting beats refusing") diff --git a/packages/meshbay-hub/tests/test_layout_responsive.py b/packages/meshbay-hub/tests/test_layout_responsive.py new file mode 100644 index 0000000..741e728 --- /dev/null +++ b/packages/meshbay-hub/tests/test_layout_responsive.py @@ -0,0 +1,159 @@ +""" +The rules that keep the Files toolbar inside a phone screen. + +Reported from a real handset: the toolbar ran off the right edge, the download +button worst of all. The cause was arithmetic rather than subtle. At a 360 px +viewport the toolbar has 310 px of usable width, and its right-hand group asked +for 440 px: + + filter 172 + Select 90 + five 30 px actions 166 + gaps 12 = 440 + +`.toolbar-group` had no `flex-wrap`, so that group could not break, and +`margin-left: auto` pushed the excess off the right-hand side rather than the +left — which is exactly how it was seen. + +These assertions read the stylesheet. That is weak evidence and it is what is +available: there is no browser in this suite, so a layout cannot be measured +here, only its inputs pinned. What they buy is that the four rules holding the +toolbar together cannot be removed without something saying so. +""" + +import re +from pathlib import Path + +import pytest + +STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" +CSS = STATIC / "style.css" + +pytestmark = pytest.mark.skipif( + not CSS.exists(), reason="the SPA sources are not available") + + +@pytest.fixture(scope="module") +def css(): + return CSS.read_text() + + +def _rule(css: str, selector: str) -> str: + """The body of the first rule whose selector list starts with `selector`.""" + m = re.search(r"^" + re.escape(selector) + r"[^{]*\{([^}]*)\}", css, re.M) + assert m, f"no rule found for {selector}" + return m.group(1) + + +@pytest.fixture(scope="module") +def mobile(css): + """The body of the max-width: 768px block.""" + i = css.index("@media (max-width: 768px)") + depth, j = 0, css.index("{", i) + for k in range(j, len(css)): + if css[k] == "{": + depth += 1 + elif css[k] == "}": + depth -= 1 + if depth == 0: + return css[j:k] + pytest.fail("the mobile media query is not closed") + + +def test_a_toolbar_group_can_break(css): + """Without this the right-hand group is a single unbreakable 440 px row.""" + assert "flex-wrap: wrap" in _rule(css, ".toolbar-group") + + +def test_the_filter_can_shrink(css): + """A fixed 150 px input keeps its width and pushes everything after it out.""" + assert "min-width: 0" in _rule(css, ".tb-search") + assert "min-width: 0" in _rule(css, ".tb-search input") + + +def test_the_action_buttons_wrap(css): + assert "flex-wrap: wrap" in _rule(css, ".tb-actions") + + +def test_breadcrumbs_wrap(css): + """A deep path is the other way this row grows without limit.""" + assert "flex-wrap: wrap" in _rule(css, ".breadcrumbs") + + +def test_the_right_hand_group_stops_being_pushed_right_on_a_phone(mobile): + """`margin-left: auto` is what sent the overflow off-screen to the right.""" + assert "margin-left: 0" in mobile + assert ".toolbar-group.right" in mobile + + +def test_the_groups_take_a_line_each_on_a_phone(mobile): + assert "width: 100%" in mobile + + +def test_the_toolbar_still_fits_a_360px_screen(css): + """The measurements the rules above are chosen against. + + Recomputed from the stylesheet rather than restated, so a change to the + button height or the toolbar padding is caught here instead of on a phone. + """ + icon_btn = _rule(css, ".tb-icon-btn") + size = int(re.search(r"width:\s*(\d+)px", icon_btn).group(1)) + gap = int(re.search(r"gap:\s*(\d+)px", _rule(css, ".tb-actions")).group(1)) + + usable = 360 - 2 * 16 - 2 * 8 - 2 # viewport − .main − .file-toolbar − borders + actions = 5 * size + 4 * gap # play, view, download, zip, delete + assert actions <= usable, ( + f"five actions need {actions}px and the toolbar offers {usable}px on a " + "360px screen — they no longer fit on their own line") + + +# ── The chat panel's height ─────────────────────────────────────────────────── + +APP = STATIC / "app.js" + + +@pytest.fixture(scope="module") +def app(): + return APP.read_text() + + +def test_the_chat_panel_is_measured_not_guessed(app): + """`calc(100vh - 220px)` was wrong twice over on a phone. + + `100vh` is the viewport with the URL bar *hidden*, so with it showing the + panel is already taller than the screen. And 220px is a guess at the group + header, which carries a title, a description of any length, an edit link, + a delete button and the tabs. Between them the composer ended up below the + fold and the whole page scrolled to reach it. + """ + assert "el.getBoundingClientRect().top + window.scrollY" in app, ( + "the height must come from where the panel actually sits") + assert "window.visualViewport?.height || window.innerHeight" in app, ( + "innerHeight alone ignores the on-screen keyboard on Android") + + +def test_the_measurement_survives_a_scrolled_page(app): + """Document-relative, so the answer does not depend on the scroll offset.""" + fit = app[app.index("const fit = () => {"):] + fit = fit[:fit.index("};")] + assert "window.scrollY" in fit + + +def test_the_panel_refits_when_the_viewport_changes(app): + for event in ("resize", "orientationchange"): + assert f"addEventListener('{event}', fit)" in app + assert "visualViewport?.addEventListener('resize', fit)" in app + assert "removeEventListener('resize', fit)" in app, "the listener must be released" + + +def test_the_css_floor_does_not_fight_the_measurement(css, app): + """A min-height above the computed value would put the scrollbar back.""" + panel = _rule(css, ".chat-panel") + css_floor = int(re.search(r"min-height:\s*(\d+)px", panel).group(1)) + js_floor = int(re.search(r"const CHAT_MIN_HEIGHT = (\d+)", app).group(1)) + assert css_floor == js_floor, ( + f"CSS floor {css_floor}px and JS floor {js_floor}px disagree — the " + "larger one silently wins and the page scrolls again") + + +def test_the_fallback_height_uses_dvh(css): + """The value before the measurement runs, and if it never does.""" + panel = _rule(css, ".chat-panel") + assert "dvh" in panel, "vh is the URL-bar-hidden viewport and overshoots" diff --git a/packages/meshbay-hub/tests/test_spa_ordering.py b/packages/meshbay-hub/tests/test_spa_ordering.py index 79d7c9a..9d02d42 100644 --- a/packages/meshbay-hub/tests/test_spa_ordering.py +++ b/packages/meshbay-hub/tests/test_spa_ordering.py @@ -210,8 +210,11 @@ def test_a_multi_file_download_waits_for_each_picker(): selected, one file downloaded. """ app = APP.read_text() - block = app[app.index("${selectedFiles.length > 0 && html`"):] - block = block[:block.index("`}")] + # Anchored on the loop rather than on the markup around it: the toolbar + # moved from a dropdown to icon buttons and took the old wrapper with it, + # while the property under test — one picker at a time — did not change. + block = app[app.index("for (const e of selectedFiles)"):] + block = block[:block.index("\n")] assert "await downloadFile(e)" in block, ( "downloads are fired without awaiting again; only the first will ask " "for a save location and the others will be rejected") diff --git a/packages/meshbay-hub/tests/test_transport_contracts.py b/packages/meshbay-hub/tests/test_transport_contracts.py new file mode 100644 index 0000000..fb92fcd --- /dev/null +++ b/packages/meshbay-hub/tests/test_transport_contracts.py @@ -0,0 +1,218 @@ +""" +Two wire contracts in transport.js that fail quietly when broken. + +Neither can be reached from Python, and `QE/deploy/e2e.py` is a second +implementation of the client rather than a test of this one, so these read the +source. That is weak evidence in general, and the right kind here: both defects +below produce a plausible screen rather than an error. + + - History paged forward from the oldest message, so a group with more than + 200 of them opened on its first screen and the recent conversation could + not be reached. Nothing threw; the wrong messages were simply shown. + - A reply that is not routed falls through to "resolve the oldest pending + request". Adding a ping made that dangerous: a pong handed to a waiting + history request satisfies it with a message that has no `messages` field, + and the conversation renders empty. +""" + +import re +from pathlib import Path + +import pytest + +STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" +TRANSPORT = STATIC / "transport.js" +APP = STATIC / "app.js" + +pytestmark = pytest.mark.skipif( + not TRANSPORT.exists(), reason="the SPA sources are not available") + + +@pytest.fixture(scope="module") +def transport(): + return TRANSPORT.read_text() + + +@pytest.fixture(scope="module") +def app(): + return APP.read_text() + + +def test_chat_history_pages_backwards(transport): + body = transport[transport.index("async fetchChatHistory"):] + body = body[:body.index("\n }")] + assert "before" in body, "the request must carry a backward cursor" + assert "since" not in body, ( + "`since` pages forward from the oldest message — that was the bug") + + +def test_chat_history_reports_whether_more_exists(transport): + body = transport[transport.index("async fetchChatHistory"):] + body = body[:body.index("\n }")] + assert "has_more" in body, ( + "without it the 'load older' control cannot know when to stop offering") + + +def test_the_browser_asks_for_the_newest_page_first(app): + """A group opens on the newest messages, not the oldest.""" + assert "fetchChatHistory({ limit: CHAT_PAGE })" in app + assert re.search(r"const CHAT_PAGE\s*=\s*100", app) + assert re.search(r"const CHAT_OLDER_PAGE\s*=\s*50", app) + + +def test_older_pages_are_requested_with_a_cursor_not_an_offset(app): + assert "before: messages[0].id" in app, ( + "paging by offset repeats or skips messages when one arrives mid-scroll") + + +def test_pong_is_routed_by_its_echoed_token(transport): + """Not left to the oldest-pending fallback, which would empty a chat.""" + assert "if (msg.type === 'pong')" in transport + routing = transport[transport.index("if (msg.type === 'pong')"):] + routing = routing[:routing.index("const oldest")] + assert "handler._key === key" in routing + assert "return;" in routing, ( + "a pong for a timed-out probe must stop here, not fall through") + + +def test_ping_requests_are_keyed(transport): + assert "`ping:${obj.token}`" in transport, ( + "an unkeyed ping cannot be matched to its pong") + + +def test_ping_can_time_out_sooner_than_a_transfer(transport): + """30 s is right for a chunk and useless for a liveness probe.""" + assert "_sendAndWait(obj, timeoutMs = 30000)" in transport + body = transport[transport.index(" async ping("):] + body = body[:body.index("\n }")] + assert "timeoutMs" in body + + +def test_scroll_position_is_anchored_when_older_messages_are_prepended(app): + """Everything above the viewport grows, so scrollTop alone is not enough.""" + assert "scrollHeight - list.scrollTop" in app, "the anchor is measured from the bottom" + assert "list.scrollTop = list.scrollHeight - anchorRef.current" in app + assert "useLayoutEffect" in app, ( + "correcting after paint shows the jump it is meant to prevent") + + +def test_the_view_only_follows_new_messages_when_already_at_the_bottom(app): + assert "if (atBottomRef.current) list.scrollTop = list.scrollHeight" in app, ( + "scrolling unconditionally fights someone reading back through history") + + +def test_the_bottom_is_reached_by_scrollTop_not_a_sentinel(app): + """scrollIntoView on a zero-height marker stops short of the true bottom. + + The list has padding and a flex gap below the last bubble, so aligning an + empty div to the viewport bottom left the bar a few pixels from the end — + visible on opening a group and again after sending a message. + """ + # The call, not the word: a comment explaining why it is gone should not + # be able to fail this. + assert ".scrollIntoView(" not in app + assert "list.scrollTo({ top: list.scrollHeight" in app, ( + "jumping to the latest should also land on the real bottom") + + +def test_messages_are_keyed_by_id_not_index(app): + """Index keys plus prepending makes Preact reuse the wrong bubbles.""" + assert "key=${m.id}" in app + assert "key=${i}" not in app.split("function ChatPanel")[1].split("\n}")[0] + + +def test_presence_has_three_states_and_a_label_for_each(app): + for state in ("online", "offline", "unknown"): + assert f"presence-{state}" in (STATIC / "style.css").read_text() + assert "t('presence.' + state)" in app, ( + "red and green are the pair colour-blind readers cannot separate, so " + "the dot needs a title and an aria-label, not just a colour") + + +def test_a_refusal_from_the_node_counts_as_present(app): + """The node answering "no" proves it is up; only silence proves nothing.""" + assert "err.reason ? 'online' : 'offline'" in app + + +# ── The create-group form ───────────────────────────────────────────────────── + +def test_choosing_public_settles_the_admission_question(app): + """Public implies open, so the policy selector has nothing left to ask. + + Enforced twice on purpose: the API refuses public+invite with a 422, and the + form never offers the combination. A form that can build a request the server + rejects is a form that produces an error message instead of a group. + """ + assert "setVisibility('public'); setJoinPolicy('open');" in app, ( + "picking Public must settle the policy, not leave the previous one") + assert "setVisibility('private'); setJoinPolicy('invite');" in app, ( + "going back to Private must not leave the group open by accident") + + form = app[app.index("function CreateGroupPage"):] + form = form[:form.index("\n}\n")] + selector = form.index("t('create_group.join_policy')") + guard = form.rindex("visibility === 'public'", 0, selector) + assert guard != -1, "the policy selector must sit behind a visibility guard" + assert "create_group.public_is_open" in form[guard:selector], ( + "a public group should say why there is nothing to choose") + + +def test_the_form_starts_on_a_combination_the_api_accepts(app): + form = app[app.index("function CreateGroupPage"):] + assert "useState('private')" in form[:form.index("return html")] + assert "useState('invite')" in form[:form.index("return html")] + + +# ── Dead references in the SPA ──────────────────────────────────────────────── + +def test_no_setter_survives_the_state_it_belonged_to(app): + """A removed useState leaves its setter behind, and nothing complains. + + `setActionsOpen` outlived `actionsOpen` when the Actions dropdown became a + row of buttons, and shipped: every action in the Files panel threw + ReferenceError on click. No Python test could see it and e2e.py does not + drive the SPA, so the shape is checked here instead. + + A grep for the state name does not find it — `setActionsOpen` does not + contain `actionsOpen`, the capital breaks the match. That is exactly how it + got through. + """ + import re + declared = set(re.findall(r"const \[\s*\w+\s*,\s*(set\w+)\s*\]\s*=\s*useState", app)) + # Names brought in from another module are defined, just not here. + imported = set() + for names in re.findall(r"import\s*\{([^}]*)\}\s*from", app): + imported.update(n.strip().split(" as ")[-1].strip() for n in names.split(",")) + # A bare call only: `downloads.setMode(...)` and `view.setUint32(...)` belong + # to their object, not to this component. + called = set(re.findall(r"(? str: + i = app.index("function VideoPlayer(") + return app[i:app.index("\nfunction ", i + 1)] + + +# ── The flag that stalled everything ────────────────────────────────────────── + +def test_a_new_stream_starts_from_a_clean_slate(app): + """The reset must be where the stream starts, not where the last one ended. + + Teardown is skippable — an unmount that races, an effect that re-runs — and + a stale flag costs the whole player. Starting from a known state cannot be + skipped. + """ + player = _player(app) + effect = player[player.index("useEffect(() => {\n let cancelled = false;"):] + effect = effect[:effect.index("transport.requestStream")] + for ref in ("appendingRef", "endedRef"): + assert f"{ref}.current = false" in effect, ( + f"{ref} is not reset before the stream starts") + + +def test_the_stall_is_reproduced_and_the_reset_clears_it(tmp_path): + """The guard and the reset, run for real rather than read. + + Models flushQueue's first line and the append/updateend cycle. Without the + reset the second stream appends nothing at all; with it, it drains. + """ + script = tmp_path / "case.mjs" + script.write_text(""" + // flushQueue's guard, and the part that makes it a trap: `updateend` + // belongs to the SourceBuffer, which teardown throws away, while + // `appending` is a ref on the component, which survives. + const makePlayer = (resetOnStart) => { + const st = { appending: false, appended: 0, queue: [], owed: 0 }; + return { + st, + startStream() { if (resetOnStart) st.appending = false; }, + push(seg) { st.queue.push(seg); this.flush(); }, + flush() { + if (st.appending || st.queue.length === 0) return; + st.appending = true; + st.queue.shift(); + st.appended++; + st.owed++; // the browser will fire updateend for this + }, + fireUpdateend() { // only for appends that really happened + while (st.owed > 0) { + st.owed--; + st.appending = false; + this.flush(); + } + }, + teardown() { + st.queue = []; + st.owed = 0; // the SourceBuffer and its listener are gone + }, + }; + }; + + const out = {}; + for (const [name, reset] of [["without_reset", false], ["with_reset", true]]) { + const p = makePlayer(reset); + // First video: an append is in flight and its updateend has not arrived + // when the viewer moves on. That is the whole scenario. + p.startStream(); + p.push("A1"); + p.teardown(); + const afterFirst = p.st.appended; + // Second video. + p.startStream(); + p.push("B1"); p.fireUpdateend(); + p.push("B2"); p.fireUpdateend(); + out[name] = p.st.appended - afterFirst; + } + console.log(JSON.stringify(out)); + """) + proc = subprocess.run(["node", str(script)], capture_output=True, text=True) + assert proc.returncode == 0, proc.stderr + got = json.loads(proc.stdout) + + assert got["without_reset"] == 0, ( + "the scenario no longer reproduces the stall, so this test proves nothing") + assert got["with_reset"] == 2, ( + "the second video still cannot append — the reset does not clear the stall") + + +# ── Segments from the abandoned stream ──────────────────────────────────────── + +@pytest.mark.parametrize("handler", ["onStreamInit", "onStreamData", "onStreamEnd"]) +def test_stream_messages_are_matched_to_the_file_they_belong_to(app, handler): + player = _player(app) + body = player[player.index(f"transport.{handler} = "):] + body = body[:body.index("\n };")] + assert "file_id !== entry.id" in body, ( + f"{handler} accepts a message from any stream — on an ordered channel " + "the film you just left is still arriving") + + +def test_the_node_actually_stamps_those_messages(): + """The guard above is worth nothing if the field is not sent.""" + server = (Path(__file__).resolve().parents[2] / "meshbay-node" / "src" + / "meshbay_node" / "transport" / "webrtc_server.py") + if not server.exists(): + pytest.skip("the node sources are not available") + text = server.read_text() + for const in ("STREAM_INIT", "STREAM_DATA", "STREAM_END"): + i = text.index(f"MNP.{const}") + block = text[i:i + 400] + assert re.search(r'"file_id":\s*file_id', block), ( + f"{const} carries no file_id, so the client cannot tell streams apart") -- cgit v1.2.3