diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-09-19 17:13:17 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-09-19 17:13:17 +0200 |
| commit | 2eaf6887295614509f8d0bc24a9b77ccd915ef88 (patch) | |
| tree | c713fec411770da9250a44f9b1e08c4165c8d439 /packages | |
| parent | d2495a2c4b89fbbfc18cefec83ae96cabdd745e2 (diff) | |
| download | meshbay-2eaf6887295614509f8d0bc24a9b77ccd915ef88.tar.gz | |
fix(hub): a reconnect re-reads the index, not just the handshake
_reconnectLoop re-did the handshake and nothing else, so a page kept
whatever it last saw until someone reloaded it. That is invisible until
the node restarts: it rebuilds its index from index_cache.db, which
holds no enrichment, and Music is the one app whose enrichment is
persisted nowhere — for the length of the re-read pass it serves tracks
with no artist, and the album grid drew nothing.
onReconnected was one slot the video player took on open and cleared on
close; it is a listener set now, and carries the fresh ack.
docs/MESHBAY_DESIGN.md §15.3 records the two defects found alongside and
not fixed here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'packages')
9 files changed, 306 insertions, 42 deletions
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/group-page.js b/packages/meshbay-hub/src/meshbay_hub/static/group-page.js index 26519da..4fdf5f8 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/group-page.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/group-page.js @@ -256,6 +256,44 @@ function GroupPage({ groupId, group, token, username, userId, userPrefs, } }, [passInput, username]); + // Everything one handshake ack tells this page, applied in one place. + // + // Called by the first connect and again by every automatic reconnect: a node + // that restarted is a different process, and its answers are not the ones the + // first handshake got. Written once because the two paths drifting is how + // `helloworld`'s directories went missing from one of them. + const applyAck = useCallback((ack) => { + if (!ack) return; + setIsNodeAdmin(!!ack.is_node_admin); + setEnabledApps(ack.enabled_apps || null); + setScanSettings(ack.scan_settings || null); + setTmdbConfig({ + // Per-group (2026-08-24, used to be node-wide). + enabled: ack.tmdb_enabled !== false, + // Node-wide — one shared credential/cache. + tokenCustomized: !!ack.tmdb_token_customized, + language: ack.tmdb_language || '', + }); + // Every `<app>_directories` the ack carries, keyed by the app's own + // name — read off the ack rather than from a list of app names held + // here, so an application the node knows about is one this page already + // handles. Three names were hardcoded until 2026-09-10 and `helloworld` + // was not among them, so the app that exists to prove a new one needs + // no special-casing had its directories dropped on arrival. The live + // path below (`onAppDirectories`) was always generic; this was the half + // that was not. + setAppDirectories(Object.fromEntries( + Object.keys(ack) + .filter((k) => k.endsWith('_directories')) + .map((k) => [k.slice(0, -'_directories'.length), ack[k] || []]))); + setChatDirectory(ack.chat_directory || ''); + setChatLinkPreview(ack.chat_link_preview !== false); + setSearchListed(ack.search_listed !== false); + setMusicbrainzConfig({ + enabled: ack.musicbrainz_enabled !== false, + }); + }, []); + // One place that takes an index from the node and puts it everywhere it has to // go. Deleting a file used to refresh the table and leave the cache alone, so // the search page went on offering a file that no longer existed until the @@ -298,6 +336,9 @@ function GroupPage({ groupId, group, token, username, userId, userPrefs, useEffect(() => { let cancelled = false; + // Dropped by the teardown below, so a transport handed on to a running + // download (`releaseWhenIdle`) stops driving a page that is gone. + let offReconnect = null; // The cache is written here and read only by the search page. It used to // seed this list too, which put a stale index on screen and then raced the @@ -398,34 +439,7 @@ function GroupPage({ groupId, group, token, username, userId, userPrefs, if (!transport) throw (lastErr || new Error('no node served this group')); session.pendingJoinCode = null; if (cancelled) return; - setIsNodeAdmin(!!ack.is_node_admin); - setEnabledApps(ack.enabled_apps || null); - setScanSettings(ack.scan_settings || null); - setTmdbConfig({ - // Per-group (2026-08-24, used to be node-wide). - enabled: ack.tmdb_enabled !== false, - // Node-wide — one shared credential/cache. - tokenCustomized: !!ack.tmdb_token_customized, - language: ack.tmdb_language || '', - }); - // Every `<app>_directories` the ack carries, keyed by the app's own - // name — read off the ack rather than from a list of app names held - // here, so an application the node knows about is one this page already - // handles. Three names were hardcoded until 2026-09-10 and `helloworld` - // was not among them, so the app that exists to prove a new one needs - // no special-casing had its directories dropped on arrival. The live - // path below (`onAppDirectories`) was always generic; this was the half - // that was not. - setAppDirectories(Object.fromEntries( - Object.keys(ack) - .filter((k) => k.endsWith('_directories')) - .map((k) => [k.slice(0, -'_directories'.length), ack[k] || []]))); - setChatDirectory(ack.chat_directory || ''); - setChatLinkPreview(ack.chat_link_preview !== false); - setSearchListed(ack.search_listed !== false); - setMusicbrainzConfig({ - enabled: ack.musicbrainz_enabled !== false, - }); + applyAck(ack); transport.onAppsEnabled = (apps) => setEnabledApps(apps); // Two independent acks now (tmdb_config_ack: token/language, // node-wide; tmdb_enabled_ack: the per-group switch) — each merges @@ -513,6 +527,49 @@ function GroupPage({ groupId, group, token, username, userId, userPrefs, if (onPresence) onPresence(groupId, 'online'); }; + // An automatic reconnect (transport.js's _reconnectLoop) re-does the + // handshake and nothing else: no index is fetched, and any push sent + // while the old channel was dying is simply lost. That was survivable + // while a node that came back came back with the same answers — and a + // restarted one does not. It rebuilds its index from + // `index_cache.db`, which stores path/mtime/size/hash/type and no + // enrichment at all, so for the ~25s its re-enrichment pass takes + // (measured: 6176 audio files, 6.4s of tag reads plus cover work) the + // index it serves has no artist and no album on any track. A client + // that reconnected inside that window kept exactly that view for as + // long as the page stayed open: Files and Videos looked right — one + // needs no enrichment, the other's is restored from `media_cache.db` + // — and Music, whose grouping *is* the enrichment, drew nothing. + // + // So the reconnect asks again, for the ack and the index both. The + // full fetch rather than a delta: this session was never told what it + // missed, and a delta is computed against a snapshot only the node + // has. + offReconnect = transport.addReconnectListener((reack) => { + if (cancelled) return; + (async () => { + try { + applyAck(reack); + // Re-imported, not kept: a chat epoch or a re-key while we were + // away means the handshake just handed us a different GEK, and + // gekRef is what every decrypt on this page reads. + if (transport.gekRaw && window.MeshBayCrypto) { + gekRef.current = await window.MeshBayCrypto.importGEK( + window.MeshBayCrypto.b64encode(transport.gekRaw)); + } + const msg = await transport.fetchIndex(); + if (cancelled) return; + applyIndex(msg); + } catch (e) { + // The connection went again mid-refresh: the next reconnect + // runs this same handler. Saying so beats a view that is + // quietly one node-restart old. + console.warn('[MeshBay] index refresh after reconnect failed:', + e.message); + } + })(); + }); + // We are in: an invitation to this group has served its purpose. if (onJoined) onJoined(groupId); @@ -591,6 +648,7 @@ function GroupPage({ groupId, group, token, username, userId, userPrefs, return () => { cancelled = true; + if (offReconnect) { offReconnect(); offReconnect = null; } // Nothing will update this group's dock row once the page lets go of it. reportIndexPush(groupId, null); if (transportRef.current) { diff --git a/packages/meshbay-hub/src/meshbay_hub/static/transport.js b/packages/meshbay-hub/src/meshbay_hub/static/transport.js index f4979f4..39e0ccf 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/transport.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/transport.js @@ -487,7 +487,12 @@ class MeshBayTransport { // "a reconnect to wait for" and stall every handshake step for the full // 6s gate below before ever sending it. this._inReconnectAttempt = false; - this._onReconnected = null; + // A set, not one slot. Two consumers want this at once — the video + // player, to re-ask for the stream it was watching, and the group + // page, to re-read an index the node rebuilt while we were away — + // and a single setter meant the second to arrive silently replaced + // the first, then cleared it on the way out. + this._reconnectListeners = new Set(); this._onNeedToken = null; // Which device key THIS connection has identified itself to the node with. // Empty means "not identified": nothing can be sealed, so nothing can be @@ -548,10 +553,19 @@ class MeshBayTransport { // Fired when a message that must open under the group key does not — // see _failSession. The session is over by the time this runs. set onSessionFailed(fn) { this._onSessionFailed = fn; } - // Fired once an automatic reconnect (see _reconnectLoop) lands a fresh - // handshake, so a consumer with something mid-flight on the old channel — - // today only the video player — can pick back up rather than sit dead. - set onReconnected(fn) { this._onReconnected = fn; } + /** + * Told once an automatic reconnect (see _reconnectLoop) lands a fresh + * handshake, with that handshake's ack. + * + * Returns its own unsubscribe, because the caller that stops listening + * must not be able to stop anyone else listening: `onReconnected` was a + * setter, the video player took it on open and set it back to `null` on + * close, and any other consumer's handler went with it. + */ + addReconnectListener(fn) { + this._reconnectListeners.add(fn); + return () => this._reconnectListeners.delete(fn); + } /** * Told whenever this connection's device identity changes — including to @@ -1213,10 +1227,11 @@ class MeshBayTransport { const token = this._onNeedToken ? await this._onNeedToken() : this._lastToken; trace('reconnect_attempt', { attempt: this._reconnectAttempts }); this._inReconnectAttempt = true; + let ack; try { - await this.connect(args.nodeId, token, args.groupId, args.gekRaw, - this._sessionKeys, args.bundleKey, args.username, - args.userId, args.joinCode); + ack = await this.connect(args.nodeId, token, args.groupId, args.gekRaw, + this._sessionKeys, args.bundleKey, args.username, + args.userId, args.joinCode); } finally { this._inReconnectAttempt = false; } @@ -1226,9 +1241,14 @@ class MeshBayTransport { // have asked for its slot back first, or its next `file_req` carries a // `tr` the node has never heard of. this._reopenTransfers(); - if (this._onReconnected) { - try { this._onReconnected(); } catch (e) { - console.error('[MeshBay] onReconnected handler threw:', e); + // The ack goes with it: this is a *new* session against whatever the + // node is running now, and everything the first handshake taught the + // page — the folders each app reads, which apps are on, the roots — + // was answered by a process that may since have restarted. One + // listener throwing must not rob the next of the notification. + for (const fn of [...this._reconnectListeners]) { + try { fn(ack); } catch (e) { + console.error('[MeshBay] reconnect listener threw:', e); } } return; diff --git a/packages/meshbay-hub/src/meshbay_hub/static/video-player.js b/packages/meshbay-hub/src/meshbay_hub/static/video-player.js index a07558e..66099de 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/video-player.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/video-player.js @@ -521,6 +521,9 @@ function VideoPlayer({ entry, transportRef, gekRef, onClose, onDownload }) { useEffect(() => { let cancelled = false; + // Held so the teardown below can drop *this* listener and no one else's + // (transport.js's addReconnectListener). + let offReconnect = null; // Reset here, not in the teardown of the run before: switching video while // an append was in flight left `appendingRef` true, and flushQueue bails // out on it. The new SourceBuffer then never appended anything, so no @@ -730,7 +733,7 @@ function VideoPlayer({ entry, transportRef, gekRef, onClose, onDownload }) { // exactly what dragging the scrubber does — so reusing it here means a // screen-lock reconnect looks like a seek to where the film already // was, not a reload. - transport.onReconnected = () => { + offReconnect = transport.addReconnectListener(() => { if (cancelled) return; const v = videoRef.current; const seek = requestSeekRef.current; @@ -738,7 +741,7 @@ function VideoPlayer({ entry, transportRef, gekRef, onClose, onDownload }) { console.log('[MeshBay] transport reconnected — resuming stream at', v.currentTime.toFixed(1)); seek(v.currentTime); - }; + }); transport.onStreamInit = (msg) => { if (cancelled) return; @@ -1121,8 +1124,8 @@ function VideoPlayer({ entry, transportRef, gekRef, onClose, onDownload }) { transport.onStreamData = null; transport.onStreamEnd = null; transport.onStreamError = null; - transport.onReconnected = null; } + if (offReconnect) { offReconnect(); offReconnect = null; } // The queue can hold several megabytes of decrypted video. queueRef.current = []; const ms = msRef.current; diff --git a/packages/meshbay-hub/tests/harness/group_tab_probe.py b/packages/meshbay-hub/tests/harness/group_tab_probe.py index 473bcdc..730ba99 100644 --- a/packages/meshbay-hub/tests/harness/group_tab_probe.py +++ b/packages/meshbay-hub/tests/harness/group_tab_probe.py @@ -69,6 +69,10 @@ window.MeshBayTransport = class { async fetchIndex() { return { entries: [], dirs: [], roots: [] }; } async fetchChatHistory() { return { messages: [], hasMore: false }; } async fetchLinkPreview() { return { ok: false }; } + // GroupPage subscribes to reconnects and drops the subscription on + // unmount; a stub without this throws inside connect() and the page + // renders its error state instead of a tab bar. + addReconnectListener() { return () => {}; } close() {} }; </script> diff --git a/packages/meshbay-hub/tests/harness/music_grid_probe.py b/packages/meshbay-hub/tests/harness/music_grid_probe.py index 365e028..da73b1f 100644 --- a/packages/meshbay-hub/tests/harness/music_grid_probe.py +++ b/packages/meshbay-hub/tests/harness/music_grid_probe.py @@ -78,6 +78,9 @@ window.MeshBayTransport = function () { }, async fetchChatHistory() { return { messages: [], hasMore: false }; }, async fetchLinkPreview() { return { ok: false }; }, + // Real, not left to the Proxy below: that would hand back a promise + // where an unsubscribe belongs, and the page calls it on unmount. + addReconnectListener() { return () => {}; }, close() {}, }; return new Proxy(self, { diff --git a/packages/meshbay-hub/tests/harness/music_queue_probe.py b/packages/meshbay-hub/tests/harness/music_queue_probe.py index a9146ce..483c093 100755 --- a/packages/meshbay-hub/tests/harness/music_queue_probe.py +++ b/packages/meshbay-hub/tests/harness/music_queue_probe.py @@ -77,6 +77,9 @@ window.MeshBayTransport = function () { }, async fetchChatHistory() { return { messages: [], hasMore: false }; }, async fetchLinkPreview() { return { ok: false }; }, + // Real, not left to the Proxy below: that would hand back a promise + // where an unsubscribe belongs, and the page calls it on unmount. + addReconnectListener() { return () => {}; }, close() {}, }; return new Proxy(self, { diff --git a/packages/meshbay-hub/tests/harness/playlist_ui_probe.py b/packages/meshbay-hub/tests/harness/playlist_ui_probe.py index f0fc9c3..2defed9 100644 --- a/packages/meshbay-hub/tests/harness/playlist_ui_probe.py +++ b/packages/meshbay-hub/tests/harness/playlist_ui_probe.py @@ -73,6 +73,9 @@ window.MeshBayTransport = function () { }, async fetchChatHistory() { return { messages: [], hasMore: false }; }, async fetchLinkPreview() { return { ok: false }; }, + // Real, not left to the Proxy below: that would hand back a promise + // where an unsubscribe belongs, and the page calls it on unmount. + addReconnectListener() { return () => {}; }, close() {}, }; return new Proxy(self, { diff --git a/packages/meshbay-hub/tests/harness/sticky_header_probe.py b/packages/meshbay-hub/tests/harness/sticky_header_probe.py index ee1e61b..6f084d6 100755 --- a/packages/meshbay-hub/tests/harness/sticky_header_probe.py +++ b/packages/meshbay-hub/tests/harness/sticky_header_probe.py @@ -238,6 +238,9 @@ window.MeshBayTransport = function () { }, async fetchChatHistory() { return { messages: [], hasMore: false }; }, async fetchLinkPreview() { return { ok: false }; }, + // Real, not left to the Proxy below: that would hand back a promise + // where an unsubscribe belongs, and the page calls it on unmount. + addReconnectListener() { return () => {}; }, close() {}, }; return new Proxy(self, { diff --git a/packages/meshbay-hub/tests/test_reconnect_refresh.py b/packages/meshbay-hub/tests/test_reconnect_refresh.py new file mode 100644 index 0000000..c93e94d --- /dev/null +++ b/packages/meshbay-hub/tests/test_reconnect_refresh.py @@ -0,0 +1,167 @@ +""" +What an automatic reconnect has to re-read, and who is allowed to stop +listening for one. + +Both defects here produce a plausible screen rather than an error, so they read +the source — the only evidence available for the SPA (CLAUDE.md), and the right +kind for a fault whose whole symptom is a page that looks fine and is stale. + + - `_reconnectLoop` re-did the handshake and nothing else. Nobody asked for an + index again, and a push sent while the old channel was dying reached + nobody, so the page stayed frozen at whatever it last saw until someone + reloaded it. Survivable while a node that came back came back with the same + answers; a *restarted* one does not. It rebuilds its index from + `index_cache.db` — path/mtime/size/hash/type, no enrichment — so during its + re-enrichment pass the index it serves carries no artist and no album on + any track, and a client that reconnected inside that window drew an empty + Music grid for as long as it stayed open. + + - `onReconnected` was one setter. The video player took it on open and set it + back to `null` on close, which silently disabled every other consumer's + handler — including, once the group page had one, the refresh above. +""" + +import re +from pathlib import Path + +import pytest + +STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" +TRANSPORT = STATIC / "transport.js" +GROUP_PAGE = STATIC / "group-page.js" +VIDEO_PLAYER = STATIC / "video-player.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 group_page(): + return GROUP_PAGE.read_text() + + +@pytest.fixture(scope="module") +def video_player(): + return VIDEO_PLAYER.read_text() + + +def _reconnect_loop(transport: str) -> str: + body = transport[transport.index("async _reconnectLoop()"):] + return body[:body.index("\n /**")] + + +def test_a_reconnect_is_announced_to_every_listener(transport): + """One consumer unsubscribing must not silence the others.""" + assert "addReconnectListener(fn)" in transport + assert "set onReconnected(" not in transport, ( + "a single slot is what let the video player unset the group page's handler") + assert "this._reconnectListeners = new Set()" in transport + + +def test_addReconnectListener_hands_back_its_own_unsubscribe(transport): + body = transport[transport.index("addReconnectListener(fn) {"):] + body = body[:body.index("\n }")] + assert "this._reconnectListeners.add(fn)" in body + assert "return () => this._reconnectListeners.delete(fn)" in body, ( + "without it a caller can only stop listening by clearing the whole set") + + +def test_the_reconnect_carries_the_fresh_ack(transport): + """ + The page's whole view of the node — which folders each app reads, which + apps are on, the roots — was answered by a process that may since have + restarted. + """ + loop = _reconnect_loop(transport) + assert re.search(r"ack\s*=\s*await this\.connect\(", loop), ( + "the reconnect's own handshake answer was thrown away") + assert re.search(r"fn\(ack\)", loop) + + +def test_one_listener_throwing_does_not_rob_the_next(transport): + loop = _reconnect_loop(transport) + notify = loop[loop.index("_reconnectListeners"):] + assert "try {" in notify and "catch" in notify + + +def test_nothing_assigns_the_old_setter(): + """`grep onReconnected =` is what this is, spelled so it cannot rot.""" + offenders = [p.name for p in STATIC.glob("*.js") + if re.search(r"\.onReconnected\s*=", p.read_text())] + assert offenders == [], ( + f"{offenders} still assign a slot that no longer exists") + + +def test_the_group_page_refetches_the_index_on_reconnect(group_page): + assert "addReconnectListener(" in group_page, ( + "a reconnect that re-reads nothing leaves the page a node-restart old") + listener = group_page[group_page.index("addReconnectListener("):] + listener = listener[:listener.index("\n });")] + assert "applyAck(" in listener, "the ack is a restarted node's answers, not the old one's" + assert "transport.fetchIndex()" in listener, ( + "a delta is computed against a snapshot only the node has, and this " + "session was never told what it missed") + assert "applyIndex(" in listener + + +def test_the_group_page_reimports_the_gek_on_reconnect(group_page): + """A chat epoch or a re-key while we were away hands back a different GEK.""" + listener = group_page[group_page.index("addReconnectListener("):] + listener = listener[:listener.index("\n });")] + assert "importGEK" in listener + + +def test_the_ack_is_applied_by_one_implementation(group_page): + """ + Two copies drifting is how `helloworld`'s directories went missing from one + of them; the connect path and the reconnect path read the same function. + """ + assert group_page.count("const applyAck = useCallback(") == 1 + assert group_page.count("applyAck(ack);") == 1 + assert group_page.count("applyAck(reack);") == 1 + body = group_page[group_page.index("const applyAck = useCallback("):] + body = body[:body.index("\n }, [")] + for setter in ("setEnabledApps", "setAppDirectories", "setMusicbrainzConfig", + "setTmdbConfig", "setChatDirectory", "setSearchListed"): + assert setter in body, f"{setter} is not re-read on a reconnect" + + +def test_every_listener_is_dropped_by_whoever_registered_it(group_page, video_player): + """ + A transport handed on to a running download (`releaseWhenIdle`) outlives + the page that opened it, and would go on driving a component that is gone. + """ + for name, source in (("group-page.js", group_page), + ("video-player.js", video_player)): + assert re.search(r"=\s*transport\.addReconnectListener\(", source), ( + f"{name} must keep the unsubscribe it is handed") + assert "offReconnect()" in source, ( + f"{name} registers a reconnect listener it never drops") + + +def test_every_harness_that_renders_the_group_page_stubs_the_subscription(): + """ + The fast half of a guard `test_group_tab_fallback` already provides slowly. + + GroupPage subscribes on connect and calls the unsubscribe on unmount, so a + stub node without this throws inside `connect()` and the page renders its + error state — a whole harness reporting a layout it never drew. Three of + these stubs answer an unknown method through a Proxy, which hands back a + promise where an unsubscribe belongs and defers the same failure to the + teardown. Nine minutes of browser tests found it; this finds it in a + fraction of a second. + """ + harness = Path(__file__).parent / "harness" + stubs = [p for p in harness.glob("*.py") + if "window.MeshBayTransport" in p.read_text() + and "GroupPage" in p.read_text()] + assert stubs, "no harness stubs the transport any more — has this moved?" + missing = [p.name for p in stubs + if "addReconnectListener" not in p.read_text()] + assert missing == [], ( + f"{missing} render GroupPage against a node that cannot be subscribed to") |