diff options
26 files changed, 1128 insertions, 77 deletions
@@ -493,6 +493,42 @@ do. Read them before writing anything that touches the same mechanism. viewport from the layout one), which was plausible, cost a round trip, and was wrong; the screenshot showing no keyboard was already in hand. +- **Derived data that nothing persists is re-derived where somebody is + watching.** `video_meta`, `photo_meta` and `thumbs` were durable; the audio + tags were not, so every node start re-read every audio file it serves and, + until that pass landed, served an index with no artist on any track — an + index the Music app cannot group. Nobody noticed for months because the + re-read is cheap and the node is usually up. `media_cache.audio_meta` holds + it now: the pass over a real 6176-file library costs 27.8s cold and 6.4s + cached. **What goes in such a cache is what the file's bytes decided, never + the field as the app finally sees it** — `display_title`, `track_no`, artist + and album can also come from the filename and the folder, and a rename has to + re-derive those; storing the resolved field would hand a renamed file the old + name's answer, which is the fault `_reenrich_renamed_audio_entries` exists to + prevent. The sibling-cover scan stays live for the mirror-image reason: it + reads the *folder*, so caching "no cover" would make a cover dropped in + afterwards unfindable. And a read that *failed* is not an answer — caching + "this file says nothing" for a drive that did not respond makes one bad read + permanent + +- **A view that draws one shape of unit, beside an emptiness test that counts + every shape.** The Music grid's unit is an album; a track whose artist tag is + empty belongs to none, so it was drawn nowhere — while `empty` counted it and + therefore stayed false. Neither "no music" nor anything else appeared: a + toolbar over a blank page, with every track one mode-switch away and nothing + saying so. Both halves read as correct alone, which is why no amount of + re-reading the file found it; what found it was rendering the component + against entries with no artist and looking (`music_untagged_probe.py`). + **Whenever a mode filters the units it draws, the emptiness test has to be + asked of that mode, not of the library** — or the two disagree and the + disagreement is silent. Fixed by giving the untagged pile a card of its own, + the same shape the singleton folding already minted, so the two agree by + construction rather than by a second condition someone has to maintain. That + card also stopped a cover lookup going out for a release name the browser + itself wrote: a third-party request, on the operator's connection, that + cannot match anything, seen in a live log naming a placeholder as both artist + and release + - **Three headers decide whether a page may frame itself, and they must agree.** The same streamed download navigates a hidden iframe to `/_mbdl/<id>`. `frame-src` was reCAPTCHA's two origins with no `'self'`, `frame-ancestors` 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/locales/de.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js index 2d925e1..aea1196 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js @@ -302,6 +302,7 @@ export default { 'music.mode_flat': 'Flache Liste', 'music.empty': 'Keine Musik gefunden.', 'music.unknown_album': 'Unbekanntes Album', + 'music.unknown_artist': 'Unbekannter Künstler', 'music.various': 'Verschiedenes', 'music.play_all': 'Alle abspielen', 'music.menu_more': 'Mehr…', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js index 690a70b..d9172a6 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js @@ -300,6 +300,7 @@ export default { 'music.mode_flat': 'Flat list', 'music.empty': 'No music found.', 'music.unknown_album': 'Unknown album', + 'music.unknown_artist': 'Unknown artist', 'music.various': 'Various', 'music.play_all': 'Play all', 'music.menu_more': 'More…', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js index c48f942..fc0e639 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js @@ -300,6 +300,7 @@ export default { 'music.mode_flat': 'Lista plana', 'music.empty': 'No se encontró música.', 'music.unknown_album': 'Álbum desconocido', + 'music.unknown_artist': 'Artista desconocido', 'music.various': 'Varios', 'music.play_all': 'Reproducir todo', 'music.menu_more': 'Más…', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js index 6d148d6..5eb8d60 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js @@ -301,6 +301,7 @@ export default { 'music.mode_flat': 'Liste à plat', 'music.empty': 'Aucune musique trouvée.', 'music.unknown_album': 'Album inconnu', + 'music.unknown_artist': 'Artiste inconnu', 'music.various': 'Divers', 'music.play_all': 'Tout lire', 'music.menu_more': 'Plus…', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js index c619d61..6d12cb3 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js @@ -301,6 +301,7 @@ export default { 'music.mode_flat': 'Elenco semplice', 'music.empty': 'Nessuna musica trovata.', 'music.unknown_album': 'Album sconosciuto', + 'music.unknown_artist': 'Artista sconosciuto', 'music.various': 'Vari', 'music.play_all': 'Riproduci tutto', 'music.menu_more': 'Altro…', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js index eac90dc..f4ced88 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js @@ -298,6 +298,7 @@ export default { 'music.mode_flat': 'フラットリスト', 'music.empty': '音楽が見つかりません。', 'music.unknown_album': '不明なアルバム', + 'music.unknown_artist': '不明なアーティスト', 'music.various': 'その他', 'music.play_all': 'すべて再生', 'music.menu_more': 'その他…', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js index 65bf4f5..31a2321 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js @@ -302,6 +302,7 @@ export default { 'music.mode_flat': 'Platte lijst', 'music.empty': 'Geen muziek gevonden.', 'music.unknown_album': 'Onbekend album', + 'music.unknown_artist': 'Onbekende artiest', 'music.various': 'Diversen', 'music.play_all': 'Alles afspelen', 'music.menu_more': 'Meer…', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js index 264d442..463a6a9 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js @@ -309,6 +309,7 @@ export default { 'music.mode_flat': 'Lista płaska', 'music.empty': 'Nie znaleziono muzyki.', 'music.unknown_album': 'Nieznany album', + 'music.unknown_artist': 'Nieznany wykonawca', 'music.various': 'Różne', 'music.play_all': 'Odtwórz wszystko', 'music.menu_more': 'Więcej…', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js index 0fbfb1e..20822ee 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js @@ -302,6 +302,7 @@ export default { 'music.mode_flat': 'Lista simples', 'music.empty': 'Nenhuma música encontrada.', 'music.unknown_album': 'Álbum desconhecido', + 'music.unknown_artist': 'Artista desconhecido', 'music.various': 'Diversos', 'music.play_all': 'Reproduzir tudo', 'music.menu_more': 'Mais…', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js index ab75b11..b2d7591 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js @@ -295,6 +295,7 @@ export default { 'music.mode_flat': '平铺列表', 'music.empty': '未找到音乐。', 'music.unknown_album': '未知专辑', + 'music.unknown_artist': '未知艺术家', 'music.various': '其他', 'music.play_all': '全部播放', 'music.menu_more': '更多…', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/music-app.js b/packages/meshbay-hub/src/meshbay_hub/static/music-app.js index e0308cb..1dbecbb 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/music-app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/music-app.js @@ -256,7 +256,13 @@ function AlbumCard({ album, transportRef, gekRef, musicbrainzEnabled, onOpen, on const repTrack = album.tracks.find((tr) => tr.thumb_hash) || album.tracks[0]; const tRef = repTrack._tRef || transportRef; const gRef = repTrack._gRef || gekRef; - const needsLookup = musicbrainzEnabled && !repTrack.thumb_hash; + // Never for an album this file minted itself (`isUnknown`): the release + // name is one we wrote -- '<artist> - Various', or the untagged pile + // below -- so the lookup is a third-party request, on the operator's + // connection, that cannot match anything. Confirmed in a node's log: + // queries went out naming a placeholder as both the artist and the + // release, once per card. + const needsLookup = musicbrainzEnabled && !repTrack.thumb_hash && !album.isUnknown; const meta = useMusicMeta(tRef, repTrack.id, needsLookup); const coverHash = repTrack.thumb_hash || (meta && meta.cover_thumb_hash) || null; @@ -287,7 +293,7 @@ function MusicDetailModal({ album, transportRef, gekRef, musicbrainzEnabled, onC const repTrack = album.tracks.find((tr) => tr.thumb_hash) || album.tracks[0]; const tRef = repTrack._tRef || transportRef; const gRef = repTrack._gRef || gekRef; - const needsLookup = musicbrainzEnabled && !repTrack.thumb_hash; + const needsLookup = musicbrainzEnabled && !repTrack.thumb_hash && !album.isUnknown; const meta = useMusicMeta(tRef, repTrack.id, needsLookup); const coverHash = repTrack.thumb_hash || (meta && meta.cover_thumb_hash) || null; @@ -672,17 +678,39 @@ function MusicApp({ const filteredTracks = useMemo(() => (!needle ? tracks : tracks.filter( (tr) => (tr.display_title || tr.name).toLowerCase().includes(needle))), [tracks, needle]); + // A track whose artist tag is empty *and* whose folder gave nothing to fall + // back on. The flat list has always drawn these as its own top-level rows; + // the grid, whose unit is an album, drew them nowhere at all -- and `empty` + // below counts them, so it stayed false and no message appeared either. A + // library nothing has tagged therefore rendered a toolbar over a blank page, + // with every one of its tracks one mode-switch away and nothing saying so. + // Reported after a node restart, where the index is briefly served without + // the tags it re-reads at start-up, and true of a genuinely untagged library + // with no restart involved. + // + // One card, the same shape the singleton folding above already mints for an + // artist's leftovers: it names what it is, and the tracks are playable from + // it. Not a card each -- that is the wall of one-track tiles this file + // exists to avoid -- and not sorted in among the artists, because it is not + // a name anybody chose and the alphabet is no place for it. + const untagged = useMemo(() => (filteredTracks.length + ? { artist: t('music.unknown_artist'), album: t('music.unknown_album'), + isUnknown: true, tracks: filteredTracks } + : null), [filteredTracks]); + // What this mode draws, in drawing order: albums under their artists in the // grid, and in the flat list loose tracks and artist folders sorted together. const units = useMemo(() => { if (mode === 'grid') { - return filteredArtists.flatMap((a) => a.albums.map((album) => ({ artist: a.artist, album }))); + const byArtist = filteredArtists.flatMap( + (a) => a.albums.map((album) => ({ artist: a.artist, album }))); + return untagged ? [...byArtist, { artist: untagged.artist, album: untagged }] : byArtist; } return [ ...filteredTracks.map((tr) => ({ key: tr.display_title || tr.name, kind: 'track', track: tr })), ...filteredArtists.map((a) => ({ key: a.artist, kind: 'artist', artist: a })), ].sort((a, b) => a.key.localeCompare(b.key)); - }, [mode, filteredArtists, filteredTracks]); + }, [mode, filteredArtists, filteredTracks, untagged]); const pager = usePager(units.length, pageSizeFrom(userPrefs), `${groupId}|${mode}|${needle}|${pageResetKey || ''}`); @@ -690,6 +718,9 @@ function MusicApp({ return units.slice(pager.start, pager.end); }, [units, pager.start, pager.end]); + // True exactly when the library has nothing, and — now that every track + // reaches a card — exactly when the grid has nothing to draw either. The two + // used to disagree, which is the whole of the defect above. const empty = albums.length === 0 && tracks.length === 0; return html` 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 0cf65bc..4f5b0ce 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 9138db9..9be440b 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/music_untagged_probe.py b/packages/meshbay-hub/tests/harness/music_untagged_probe.py new file mode 100644 index 0000000..f4c484c --- /dev/null +++ b/packages/meshbay-hub/tests/harness/music_untagged_probe.py @@ -0,0 +1,283 @@ +#!/usr/bin/env python3 +""" +What the Music tab draws when a track carries no artist tag. + +The grid's unit is an album, and a track with no artist at all belongs to +none — so it was drawn nowhere, while `empty` counted it and therefore stayed +false. A library nothing has tagged rendered a toolbar over a blank page with +no message, every one of its tracks reachable only by switching to the flat +list and nothing on screen saying so. + +Reading `music-app.js` does not show this: both halves are correct on their own +and the fault is that they disagree about what "nothing" means. So this renders +the shipped `GroupPage` against a stub node and reads back what is actually on +the page — cards, messages, and the rows the flat list draws for the same +entries. + + music_untagged_probe.py +""" + +import http.server +import json +import socketserver +import subprocess +import sys +import tempfile +import threading +import time +from pathlib import Path + +STATIC = Path(__file__).resolve().parents[2] / "src" / "meshbay_hub" / "static" +PORT = 8759 +RECORDS = [] +socketserver.TCPServer.allow_reuse_address = True + +# (name, tagged albums of three tracks, tracks with no artist at all, +# one-track albums under a single artist) +CASES = [ + # The reported shape: a node serving an index it has not re-read the tags + # for yet, and a genuinely untagged library, are the same page. + ("nothing tagged", 0, 7, 0), + # The ordinary shape — the pile must not displace real albums. + ("some tagged", 3, 4, 0), + # Nothing at all: the one case that really is empty, and must say so. + ("no audio", 0, 0, 0), + # The *other* album this file invents: one artist's one-track albums, folded + # into a single "<artist> - Various" card. A real release name nowhere, so + # a cover lookup for it cannot match — and this case is the only one that + # can tell that gate from a card that was simply never drawn. + ("singletons only", 0, 0, 3), +] + +FRAME = r"""<!doctype html><html><head><meta charset=utf-8> +<link rel="stylesheet" href="/style.css"></head><body> +<nav class="nav"><div class="nav-left"><a class="nav-brand" href="#/">MeshBay</a></div></nav> +<div class="layout"><main class="main"><div id="root"></div></main></div> +<script> +const ENTRIES = []; +let n = 0; +// Tagged: three tracks per album, so none of them folds into the singleton +// pile and each is a card of its own. +for (let a = 1; a <= %(albums)d; a++) { + for (let i = 0; i < 3; i++) { + ENTRIES.push({ id: 'a' + (++n), name: (i + 1) + ' - t.flac', + display_title: 'disque ' + a + ' ' + (i + 1), + path: 'musique/artiste ' + a + '/disque ' + a, type: 'audio', + artist: 'artiste ' + a, album: 'disque ' + a, track_no: i + 1, + duration: 200, size: 1024, added_at: 1750000000 + n }); + } +} +// One artist, several albums of one track each: the folding above turns these +// into a single "<artist> - Various" card whose name this file wrote. +for (let i = 1; i <= %(singles)d; i++) { + ENTRIES.push({ id: 's' + (++n), name: 'unique ' + i + '.flac', + display_title: 'unique ' + i, path: 'musique/solo/album ' + i, type: 'audio', + artist: 'solo', album: 'album ' + i, track_no: 1, + duration: 200, size: 1024, added_at: 1750000000 + n }); +} +// Untagged: no artist, no album, and sitting straight under the configured +// folder so the node's own ancestor walk would have had nothing to offer +// either. `display_title` is what the flat list shows. +for (let i = 0; i < %(loose)d; i++) { + ENTRIES.push({ id: 'u' + (++n), name: 'piste ' + (i + 1) + '.flac', + display_title: 'piste ' + (i + 1), path: 'musique', type: 'audio', + duration: 200, size: 1024, added_at: 1750000000 + n }); +} + +const ACK = { + is_node_admin: false, + enabled_apps: ['files', 'music'], + tmdb_enabled: false, musicbrainz_enabled: true, + video_directories: [], music_directories: ['musique'], photo_directories: [], +}; + +// Counted, not stubbed away: a cover lookup for an album this page invented is +// a third-party request on the operator's connection that cannot match +// anything, and the count is the only way to say it did not happen. +let musicMetaCalls = 0; +window.MeshBayTransport = function () { + const self = { + connected: false, memberRole: 'member', supportsAppOps: true, + sessionKeys: null, gekRaw: null, + newNodeBundle: null, newNodeBundleRecovery: null, + async connect() { self.connected = true; return ACK; }, + async fetchIndex() { + return { entries: ENTRIES, dirs: ['musique'], + roots: [{ name: 'musique', available: true, writable: false, + removable: false }] }; + }, + async fetchMusicMeta() { musicMetaCalls += 1; return { confidence: 0 }; }, + async fetchChatHistory() { return { messages: [], hasMore: false }; }, + async fetchLinkPreview() { return { ok: false }; }, + addReconnectListener() { return () => {}; }, + close() {}, + }; + return new Proxy(self, { + get(target, prop) { + if (prop in target) return target[prop]; + if (typeof prop === 'string' && prop.startsWith('on')) return undefined; + if (typeof prop === 'symbol') return undefined; + return () => new Promise(() => {}); + }, + set(target, prop, value) { target[prop] = value; return true; }, + }); +}; +</script> +<script type="module"> +import { html, render } from '/vendor/htm-preact.js'; +import { initLocale } from '/i18n.js'; +import { GroupPage } from '/group-page.js'; + +const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); + +// The view mode is a per-device convenience in localStorage; a case that +// inherits the previous one's would measure whichever ran first. +try { localStorage.removeItem('meshbay_music_view_mode'); } catch {} + +const read = () => ({ + cards: [...document.querySelectorAll('.music-card')].map((c) => ({ + title: c.querySelector('.music-card-title').textContent, + sub: c.querySelector('.music-card-sub').textContent, + })), + // Every message the panel can put up instead of content. + messages: [...document.querySelectorAll('.page-message')].map((p) => p.textContent.trim()), + toolbar: !!document.querySelector('.video-toolbar'), +}); + +(async () => { + const fail = (why) => parent.postMessage( + { case: %(index)d, error: why, + text: (document.getElementById('root').textContent || '').slice(0, 400) }, '*'); + try { + await initLocale(); + render(html`<${GroupPage} groupId="g1" token="t" username="me" userId="u1" + group=${{ id: 'g1', name: 'un groupe', owner_username: 'me', is_admin: false }} + userPrefs=${{ default_tab: 'music', media_page_size: '50' }} />`, + document.getElementById('root')); + + // Tiles mount on intersection; walk the page as a reader would. + for (let i = 0; i < 40; i++) { + scrollTo(0, document.documentElement.scrollHeight); + await sleep(60); + if (document.querySelector('.music-card') || document.querySelector('.page-message')) break; + } + scrollTo(0, 0); + await sleep(400); + const grid = read(); + + // The same entries in the flat list, which is where these tracks were + // always reachable — the point is that the grid now reaches them too, not + // that the list stopped. + const buttons = [...document.querySelectorAll('.video-toolbar .tb-btn')]; + if (buttons[1]) { buttons[1].click(); await sleep(400); } + const flatRows = document.querySelectorAll('.video-flat-list .video-flat-row').length; + // An untagged track is a top-level row in the flat list, not a folder. + const flatTracks = document.querySelectorAll('.video-flat-list .music-flat-track').length; + + parent.postMessage({ + case: %(index)d, + ...grid, + flatRows, + flatTracks, + musicMetaCalls, + }, '*'); + } catch (err) { + fail(String((err && err.stack) || err)); + } +})(); +</script></body></html>""" + +PAGE = r"""<!doctype html><html><head><meta charset=utf-8></head> +<body style="margin:0"><div id="frames"></div><script> +const N = %(count)d; +const seen = []; +addEventListener('message', (e) => { + seen.push(e.data); + if (seen.length === N) fetch('/log', { method: 'POST', body: JSON.stringify(seen) }); +}); +// One at a time: every case clears the same localStorage key on start-up, and +// two frames doing that at once measure each other. +let i = 0; +const next = () => { + if (i >= N) return; + const f = document.createElement('iframe'); + f.src = '/case?n=' + (i++); + f.style.cssText = 'width:1100px;height:800px;border:0;display:block'; + document.getElementById('frames').appendChild(f); +}; +addEventListener('message', () => next()); +next(); +</script></body></html>""" + + +class H(http.server.BaseHTTPRequestHandler): + def log_message(self, *a): + pass + + def do_POST(self): + length = int(self.headers.get("Content-Length") or 0) + if self.path == "/log": + RECORDS.extend(json.loads(self.rfile.read(length).decode())) + else: + self.rfile.read(length) + self.send_response(204) + self.end_headers() + + def _send(self, body: bytes, ctype: str) -> None: + self.send_response(200) + self.send_header("Content-Type", ctype) + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def do_GET(self): + path = self.path.split("?")[0] + if path == "/": + self._send((PAGE % {"count": len(CASES)}).encode(), "text/html; charset=utf-8") + elif path == "/case": + n = int(self.path.split("n=")[1]) + _, albums, loose, singles = CASES[n] + body = FRAME % {"index": n, "albums": albums, "loose": loose, + "singles": singles} + self._send(body.encode(), "text/html; charset=utf-8") + elif path == "/v1/groups/g1/nodes": + self._send(b'{"nodes": [{"node_id": "n1"}]}', "application/json") + else: + asset = (STATIC / path.lstrip("/")).resolve() + if not str(asset).startswith(str(STATIC)) or not asset.is_file(): + self.send_response(404) + self.end_headers() + return + self._send(asset.read_bytes(), + "text/css" if asset.suffix == ".css" + else "text/javascript" if asset.suffix == ".js" + else "application/octet-stream") + + +def main() -> int: + with socketserver.TCPServer(("127.0.0.1", PORT), H) as srv: + threading.Thread(target=srv.serve_forever, daemon=True).start() + with tempfile.TemporaryDirectory(ignore_cleanup_errors=True) as profile: + proc = subprocess.Popen( + ["google-chrome", "--headless=new", "--disable-gpu", "--no-sandbox", + f"--user-data-dir={profile}", "--window-size=1100,900", + f"http://127.0.0.1:{PORT}/"], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + deadline = time.time() + 120 + while len(RECORDS) < len(CASES) and time.time() < deadline: + time.sleep(0.2) + proc.terminate() + proc.wait(timeout=20) + if len(RECORDS) < len(CASES): + print(f"only {len(RECORDS)} of {len(CASES)} cases reported", file=sys.stderr) + print(json.dumps(RECORDS, indent=1), file=sys.stderr) + return 1 + out = [] + for rec in sorted(RECORDS, key=lambda r: r["case"]): + out.append({**rec, "name": CASES[rec["case"]][0]}) + print(json.dumps(out)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/packages/meshbay-hub/tests/harness/playlist_ui_probe.py b/packages/meshbay-hub/tests/harness/playlist_ui_probe.py index b60baf6..e6f99f7 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_music_untagged.py b/packages/meshbay-hub/tests/test_music_untagged.py new file mode 100644 index 0000000..b27b6a3 --- /dev/null +++ b/packages/meshbay-hub/tests/test_music_untagged.py @@ -0,0 +1,93 @@ +""" +A track with no artist tag is still music, and the grid has to say so. + +The album grid drew `albums` and nothing else, so a track with no artist at all +— which belongs to no album — appeared nowhere in it. `empty` counted those +tracks, so it stayed false and no message was drawn either: a library nothing +has tagged rendered a toolbar over a blank page, with every one of its tracks a +mode-switch away and nothing on screen saying so. Found after a node restart, +where the index is briefly served before its tags have been re-read, and true +of a genuinely untagged library with no restart involved. + +Measured rather than read: both halves of `music-app.js` are correct on their +own, and the fault is that they disagree about what "nothing" means. +""" +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +HARNESS = Path(__file__).parent / "harness" / "music_untagged_probe.py" +STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" + +pytestmark = pytest.mark.skipif( + shutil.which("google-chrome") is None or not (STATIC / "music-app.js").exists(), + reason="Chrome or the SPA sources are not available") + + +@pytest.fixture(scope="module") +def cases(): + run = subprocess.run(["python3", str(HARNESS)], capture_output=True, timeout=240) + assert run.returncode == 0, run.stderr.decode()[-2000:] + return {c["name"]: c for c in json.loads(run.stdout.decode())} + + +def test_no_case_draws_a_blank_panel(cases): + """The bug, stated once: a toolbar with nothing under it and nothing said.""" + for name, case in cases.items(): + assert "error" not in case, f"{name}: {case.get('error')}" + assert case["cards"] or case["messages"], ( + f"{name}: the panel drew neither a card nor a message") + + +def test_untagged_tracks_reach_the_grid(cases): + case = cases["nothing tagged"] + assert len(case["cards"]) == 1, "seven untagged tracks are one card, not seven" + assert case["messages"] == [], "there is music here — saying otherwise is the old lie" + + +def test_the_card_names_what_it_is(cases): + """Not a real release, and it must not read like one.""" + card = cases["nothing tagged"]["cards"][0] + assert card["title"] and card["sub"] + assert card["title"] != card["sub"] + + +def test_the_pile_does_not_displace_real_albums(cases): + case = cases["some tagged"] + assert len(case["cards"]) == 4, "three tagged albums and one pile" + # Last, not sorted in among the artists: it is not a name anybody chose. + assert case["cards"][-1]["sub"] == cases["nothing tagged"]["cards"][0]["sub"] + assert [c["title"] for c in case["cards"][:3]] == ["disque 1", "disque 2", "disque 3"] + + +def test_an_empty_library_still_says_so(cases): + """The one case that really is empty. Widening `empty` would have broken this.""" + case = cases["no audio"] + assert case["cards"] == [] + assert len(case["messages"]) == 1 + + +def test_the_flat_list_still_lists_every_untagged_track(cases): + """The grid reaching them must not cost the list what it always drew.""" + assert cases["nothing tagged"]["flatTracks"] == 7 + assert cases["some tagged"]["flatTracks"] == 4 + assert cases["some tagged"]["flatRows"] == 3, "the three tagged artists, as folders" + + +def test_no_cover_is_looked_up_for_an_album_this_page_invented(cases): + """ + A third-party request, on the operator's connection, that cannot match + anything: the release name is one the browser wrote. Seen in a live node's + log, going out with a placeholder as both artist and release. + """ + # The discriminating one: this card is drawn with or without the fix, so it + # is the only case where the count says anything about the gate itself. + assert cases["singletons only"]["cards"], "the folded card must still be drawn" + assert cases["singletons only"]["musicMetaCalls"] == 0 + + assert cases["nothing tagged"]["musicMetaCalls"] == 0 + assert cases["some tagged"]["musicMetaCalls"] == 3, ( + "the three real albums are still looked up — only the invented ones are not") 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") diff --git a/packages/meshbay-node/src/meshbay_node/indexer/enrich_audio.py b/packages/meshbay-node/src/meshbay_node/indexer/enrich_audio.py index d484e35..dd6b2c0 100644 --- a/packages/meshbay-node/src/meshbay_node/indexer/enrich_audio.py +++ b/packages/meshbay-node/src/meshbay_node/indexer/enrich_audio.py @@ -320,16 +320,30 @@ def _read_tags_and_cover( tags["title"] = title_parse.strip_track_prefix(tags["title"]) or tags["title"] if cover is None and not skip_cover: - sibling = _find_sibling_cover(path.parent) - if sibling is not None: - try: - cover = sibling.read_bytes() - except OSError: - cover = None + cover = _read_sibling_cover(path) return tags, duration, cover +def _read_sibling_cover(path: Path) -> bytes | None: + """ + The cover image sitting beside the track, as bytes. + + Its own function because it is the one part of the read that depends on the + *folder* rather than on the file's bytes: `audio_meta` remembers what the + bytes said and lets `AudioEnricher._run` skip opening the file at all, and + this still has to run on top of that, or a cover dropped in after the first + pass could never be found again. Blocking; called via asyncio.to_thread. + """ + sibling = _find_sibling_cover(path.parent) + if sibling is None: + return None + try: + return sibling.read_bytes() + except OSError: + return None + + # A folder name used as a last-resort artist/album, cleaned of the # punctuation-as-separator and release-tag noise this era of rip is full of # (underscores standing in for spaces, a bitrate/quality tag still attached @@ -450,33 +464,53 @@ class AudioEnricher: async with self._sem: fields: dict = {} - # entry.id is the file's own content hash — a cover already - # cached under it means this exact content's cover was already - # extracted (this run, an earlier one, even a previous daemon - # process), so the second file open (embedded APIC/covr scan) - # and the sibling-directory disk read are both skippable. + # entry.id is the file's own content hash, so everything a read of + # those bytes produced is cacheable under it — the tags, the + # duration, and whether the embedded-art scan has already run + # (media_cache.audio_meta, plus get_thumb_hash_by_file_id for the + # cover itself). With both in hand the file is not opened at all, + # which is the whole point: this pass used to re-read every audio + # file on the node at every start, and until it landed the index + # the node served carried no artist on any track. # - # Tags themselves are *not* skipped this way, deliberately: - # unlike a cover, artist/album/title/track_no can fall back to - # the filename or the folder name (_artist_album_from_ancestors - # above) when no tag is present, which is exactly what a rename - # needs re-derived — _reenrich_renamed_audio_entries exists for - # precisely that. Caching the *result* the same way enrich.py's - # duration/width/height is cached doesn't apply cleanly here: - # mutagen reads tags and duration in the same call as cover, so - # skipping that call to save time would also skip the - # rename-sensitive fields, and skipping only the parts that are - # safe to skip needs the cover check below, not a separate - # cache of the tag-derived fields. + # What is *not* cached, and must not be: the fallbacks below. + # artist/album/title/track_no can come from the filename or the + # folder name (_artist_album_from_ancestors above) when no tag is + # present, and those a rename has to re-derive — + # _reenrich_renamed_audio_entries exists for precisely that. The + # raw tag is not rename-sensitive, so the tag is what is stored and + # the chain still runs live on top of it. The sibling-image scan is + # the other one: it reads the folder, not the file, so a cover + # dropped in afterwards must still be found. cached_thumb_hash = await self._media_cache.get_thumb_hash_by_file_id(entry.id) - try: - tags, duration, cover = await asyncio.wait_for( - asyncio.to_thread( - _read_tags_and_cover, file_path, skip_cover=bool(cached_thumb_hash)), - timeout=READ_TIMEOUT_SECS) - except Exception as e: - log.warning("Tag read failed for %s: %s", file_path, e) - tags, duration, cover = {}, None, None + cached = await self._media_cache.get_audio_meta(entry.id) + cover_settled = bool(cached_thumb_hash) or bool(cached and cached["cover_seen"]) + if cached and cover_settled: + tags = {k: cached[k] for k in ("title", "artist", "album", "track_no") + if cached[k] is not None} + duration = cached["duration"] + cover = None if cached_thumb_hash else await asyncio.to_thread( + _read_sibling_cover, file_path) + else: + try: + tags, duration, cover = await asyncio.wait_for( + asyncio.to_thread( + _read_tags_and_cover, file_path, skip_cover=bool(cached_thumb_hash)), + timeout=READ_TIMEOUT_SECS) + except Exception as e: + # Not cached. A file that genuinely carries no tags reads + # fine and returns an empty dict, which is an answer worth + # keeping; this is a drive that did not answer, and writing + # "says nothing" for it would make one bad read permanent. + log.warning("Tag read failed for %s: %s", file_path, e) + tags, duration, cover = {}, None, None + else: + # `cover_seen` is false when the scan was skipped, so a file + # whose cover was cached and has since been evicted is + # looked at again rather than left without one for good. + await self._media_cache.put_audio_meta( + entry.id, tags, int(duration) if duration else None, + cover_seen=not cached_thumb_hash) if duration: fields["duration"] = int(duration) diff --git a/packages/meshbay-node/src/meshbay_node/media_cache.py b/packages/meshbay-node/src/meshbay_node/media_cache.py index 9dfdf73..9c692ca 100644 --- a/packages/meshbay-node/src/meshbay_node/media_cache.py +++ b/packages/meshbay-node/src/meshbay_node/media_cache.py @@ -107,6 +107,40 @@ CREATE TABLE IF NOT EXISTS mbid_meta ( -- hit had already proven unnecessary. thumb_hash is not duplicated here — -- get_thumb_hash_by_file_id(file_id) already answers that, and a second copy -- would just be one more place for the two to drift. +-- Music app (docs/MESHBAY_DESIGN.md §9.8): what a read of the file's own bytes +-- produced. Music was the one app whose index-time enrichment survived +-- nothing: `video_meta` and `photo_meta` are here, `thumbs` is here, and +-- artist/album/track_no/title lived only in the in-memory IndexEntry. So every +-- node start re-read the tags of every audio file it serves, and until that +-- pass landed the index it served carried no artist on any track — which is an +-- index the Music app cannot group, and looks from the outside like a tab that +-- lost its content. Measured over a real 6176-file library: the pass costs +-- 27.8s with nothing cached and 6.4s with this table populated. It does not +-- make the node's start-up window vanish — video enrichment and re-sealing an +-- 8845-entry index dominate that — it makes the artist and the album come back +-- in seconds rather than in tens of them. +-- +-- Only what the bytes decide, for the reason video_meta states: `display_title` +-- and `track_no` as the app finally sees them can also come from the *filename* +-- (title_parse, and _artist_album_from_ancestors for artist/album), and a +-- rename has to re-derive those — _reenrich_renamed_audio_entries exists for +-- it. The raw tag is not rename-sensitive, so it is what is stored, and the +-- fallback chain still runs live on top of it. +-- +-- `cover_seen` records that the *embedded* art scan ran for this content; +-- combined with get_thumb_hash_by_file_id it says whether the file has to be +-- opened again at all. The sibling-image scan is deliberately not covered by +-- it — that one reads the *folder*, so a cover dropped in later must still be +-- found, and it costs 0.8s across the same library. +CREATE TABLE IF NOT EXISTS audio_meta ( + file_id TEXT PRIMARY KEY, + title TEXT, + artist TEXT, + album TEXT, + track_no INTEGER, + duration INTEGER, + cover_seen INTEGER NOT NULL DEFAULT 0 +); CREATE TABLE IF NOT EXISTS photo_meta ( file_id TEXT PRIMARY KEY, width INTEGER, @@ -460,6 +494,34 @@ class MediaCache: ) await self._db.commit() + # ── audio tags (Music app) ─────────────────────────────────────────────── + # + # The tag as read, never the field as the app finally sees it — see the + # schema comment on why the filename-derived half stays out. + + async def get_audio_meta(self, file_id: str) -> dict | None: + async with self._db.execute( + "SELECT title, artist, album, track_no, duration, cover_seen " + "FROM audio_meta WHERE file_id = ?", + (file_id,), + ) as cur: + row = await cur.fetchone() + if not row: + return None + return {"title": row[0], "artist": row[1], "album": row[2], + "track_no": row[3], "duration": row[4], "cover_seen": bool(row[5])} + + async def put_audio_meta(self, file_id: str, tags: dict, duration: int | None, + cover_seen: bool) -> None: + await self._db.execute( + "INSERT OR REPLACE INTO audio_meta " + "(file_id, title, artist, album, track_no, duration, cover_seen) " + "VALUES (?, ?, ?, ?, ?, ?, ?)", + (file_id, tags.get("title"), tags.get("artist"), tags.get("album"), + tags.get("track_no"), duration, 1 if cover_seen else 0), + ) + await self._db.commit() + # ── video technical fields (Videos app) ────────────────────────────────── # # duration/width/height only — see the schema comment on why @@ -498,6 +560,8 @@ class MediaCache: await self._db.execute("DELETE FROM thumbs WHERE file_id = ?", (file_id,)) await self._db.execute("DELETE FROM photo_meta WHERE file_id = ?", (file_id,)) await self._db.execute("DELETE FROM video_meta WHERE file_id = ?", (file_id,)) + await self._db.execute( + "DELETE FROM audio_meta WHERE file_id = ?", (file_id,)) await self._db.execute("DELETE FROM file_tmdb WHERE file_id = ?", (file_id,)) await self._db.execute("DELETE FROM tmdb_override WHERE file_id = ?", (file_id,)) await self._db.execute("DELETE FROM file_mbid WHERE file_id = ?", (file_id,)) diff --git a/packages/meshbay-node/tests/test_audio_meta_cache.py b/packages/meshbay-node/tests/test_audio_meta_cache.py new file mode 100644 index 0000000..e6ee315 --- /dev/null +++ b/packages/meshbay-node/tests/test_audio_meta_cache.py @@ -0,0 +1,236 @@ +""" +The Music app's index-time enrichment survives a restart. + +`video_meta`, `photo_meta` and `thumbs` were all durable; the audio tags were +not, so every node start re-read the tags of every audio file it serves, and +until that pass landed the index it served carried no artist on any track. A +client that asked in that window got an index the Music app cannot group — a +toolbar over a blank page, for as long as the page stayed open. + +What is cached is what the file's *bytes* said. The filename and folder +fallbacks on top of it are not, and these tests are mostly about that line: +a cache that also remembered the derived fields would hand a renamed file the +old name's answer, which is the fault `_reenrich_renamed_audio_entries` exists +to prevent. +""" +import asyncio +import subprocess +from pathlib import Path + +import pytest +from meshbay_common.protocol import IndexEntry +from meshbay_node.indexer import enrich_audio +from meshbay_node.indexer.enrich_audio import AudioEnricher +from meshbay_node.media_cache import MediaCache + +_HAVE_FFMPEG = ( + subprocess.run(["which", "ffmpeg"], capture_output=True).returncode == 0) + +pytestmark = pytest.mark.skipif(not _HAVE_FFMPEG, reason="ffmpeg not installed") + + +def _make_clip(path: Path, *, title=None, artist=None, album=None, track=None) -> None: + subprocess.run( + ["ffmpeg", "-hide_banner", "-loglevel", "error", "-y", + "-f", "lavfi", "-i", "sine=frequency=440:duration=1", + "-c:a", "libmp3lame", "-b:a", "64k", + *(["-metadata", f"title={title}"] if title else []), + *(["-metadata", f"artist={artist}"] if artist else []), + *(["-metadata", f"album={album}"] if album else []), + *(["-metadata", f"track={track}"] if track else []), + str(path)], + check=True, capture_output=True, + ) + + +@pytest.fixture +async def media_cache(tmp_path): + c = MediaCache(db_path=tmp_path / "media_cache.db") + await c.open() + yield c + await c.close() + + +async def _enrich(enricher, entry, clip, root_path=None): + done = asyncio.get_event_loop().create_future() + + async def on_done(file_id, fields): + done.set_result(fields) + + enricher.spawn(entry, clip, on_done, root_path) + return await asyncio.wait_for(done, timeout=30) + + +@pytest.fixture +def counted_reads(monkeypatch): + """How many times the audio file itself was opened and parsed.""" + calls = [] + real = enrich_audio._read_tags_and_cover + + def counting(path, skip_cover=False): + calls.append(Path(path).name) + return real(path, skip_cover=skip_cover) + + monkeypatch.setattr(enrich_audio, "_read_tags_and_cover", counting) + return calls + + +@pytest.mark.asyncio +async def test_a_second_pass_over_the_same_content_does_not_open_the_file( + tmp_path, media_cache, counted_reads): + """The restart case, in one process: same bytes, no second read.""" + clip = tmp_path / "01 - a track.mp3" + _make_clip(clip, title="A Title", artist="An Act", album="A Record", track=2) + entry = IndexEntry(id="content1", name=clip.name, path=clip.name, + size=clip.stat().st_size, type="audio", added_at=0) + + first = await _enrich(AudioEnricher(media_cache), entry, clip) + assert len(counted_reads) == 1 + + # A different enricher, as a restarted daemon would build — only the cache + # on disk is shared. + second = await _enrich(AudioEnricher(media_cache), entry, clip) + assert len(counted_reads) == 1, "the file was opened again despite a cache hit" + assert second == first, "a cache hit must produce the fields the read produced" + + +@pytest.mark.asyncio +async def test_the_cached_answer_is_the_tags_and_the_duration(tmp_path, media_cache): + clip = tmp_path / "01 - a track.mp3" + _make_clip(clip, title="A Title", artist="An Act", album="A Record", track=2) + entry = IndexEntry(id="content2", name=clip.name, path=clip.name, + size=clip.stat().st_size, type="audio", added_at=0) + await _enrich(AudioEnricher(media_cache), entry, clip) + + row = await media_cache.get_audio_meta("content2") + assert row["title"] == "A Title" + assert row["artist"] == "An Act" + assert row["album"] == "A Record" + assert row["track_no"] == 2 + assert row["duration"] == 1 + assert row["cover_seen"] is True + + +@pytest.mark.asyncio +async def test_a_file_that_says_nothing_is_remembered_as_saying_nothing( + tmp_path, media_cache, counted_reads): + """ + The commonest row in a real library, and the one worth caching most: an + untagged file costs exactly the same open as a tagged one to learn nothing + from. + """ + folder = tmp_path / "Some Act" / "Some Record" + folder.mkdir(parents=True) + clip = folder / "05 - Filename Title.mp3" + _make_clip(clip) + entry = IndexEntry(id="content3", name=clip.name, + path=str(clip.relative_to(tmp_path)), + size=clip.stat().st_size, type="audio", added_at=0) + + first = await _enrich(AudioEnricher(media_cache), entry, clip, tmp_path) + assert await media_cache.get_audio_meta("content3") is not None + + second = await _enrich(AudioEnricher(media_cache), entry, clip, tmp_path) + assert len(counted_reads) == 1 + # Still resolved, and from the folder rather than from any tag. + assert second["artist"] == "Some Act" + assert second["album"] == "Some Record" + assert second == first + + +@pytest.mark.asyncio +async def test_a_renamed_file_re_derives_what_the_name_decides( + tmp_path, media_cache, counted_reads): + """ + The line the cache must not cross. Same content, so the tags are reused — + and `display_title`/`track_no`/artist/album still come from the new name + and the new folder, which is what `_reenrich_renamed_audio_entries` asks + for when it discards the attempt. + """ + first_folder = tmp_path / "Old Act" / "Old Record" + first_folder.mkdir(parents=True) + clip = first_folder / "05 - Old Name.mp3" + _make_clip(clip) # no tags at all: the name decides + entry = IndexEntry(id="content4", name=clip.name, + path=str(clip.relative_to(tmp_path)), + size=clip.stat().st_size, type="audio", added_at=0) + before = await _enrich(AudioEnricher(media_cache), entry, clip, tmp_path) + assert before["display_title"] == "Old Name" + assert before["artist"] == "Old Act" + + moved_folder = tmp_path / "New Act" / "New Record" + moved_folder.mkdir(parents=True) + moved = moved_folder / "07 - New Name.mp3" + clip.rename(moved) + renamed = IndexEntry(id="content4", name=moved.name, + path=str(moved.relative_to(tmp_path)), + size=moved.stat().st_size, type="audio", added_at=0) + + after = await _enrich(AudioEnricher(media_cache), renamed, moved, tmp_path) + assert len(counted_reads) == 1, "the bytes did not change; the file need not be reopened" + assert after["display_title"] == "New Name", "the cache answered for the old name" + assert after["track_no"] == 7 + assert after["artist"] == "New Act" + assert after["album"] == "New Record" + + +@pytest.mark.asyncio +async def test_a_cover_dropped_in_afterwards_is_still_found(tmp_path, media_cache): + """ + The sibling scan reads the *folder*, so it stays live on top of the cache. + Caching it would have made "no cover" permanent for content that later got + one — and the scan costs 0.8s across a 6000-file library, measured. + """ + folder = tmp_path / "An Act" / "A Record" + folder.mkdir(parents=True) + clip = folder / "01 - a track.mp3" + _make_clip(clip) + entry = IndexEntry(id="content5", name=clip.name, + path=str(clip.relative_to(tmp_path)), + size=clip.stat().st_size, type="audio", added_at=0) + + first = await _enrich(AudioEnricher(media_cache), entry, clip, tmp_path) + assert first.get("thumb_hash") is None + + (folder / "cover.jpg").write_bytes(b"\xff\xd8\xff\xe0 not really a jpeg") + second = await _enrich(AudioEnricher(media_cache), entry, clip, tmp_path) + assert second.get("thumb_hash"), "the cache made a missing cover permanent" + + +@pytest.mark.asyncio +async def test_an_unreadable_file_is_not_remembered_as_empty(tmp_path, media_cache): + """ + A drive that did not answer is not a file that carries no tags. Writing + "says nothing" for it would make one bad read permanent. + """ + clip = tmp_path / "01 - a track.mp3" + _make_clip(clip, title="A Title", artist="An Act") + entry = IndexEntry(id="content6", name=clip.name, path=clip.name, + size=clip.stat().st_size, type="audio", added_at=0) + + def boom(path, skip_cover=False): + raise OSError("the drive said no") + + real = enrich_audio._read_tags_and_cover + enrich_audio._read_tags_and_cover = boom + try: + await _enrich(AudioEnricher(media_cache), entry, clip) + finally: + enrich_audio._read_tags_and_cover = real + assert await media_cache.get_audio_meta("content6") is None + + fields = await _enrich(AudioEnricher(media_cache), entry, clip) + assert fields["artist"] == "An Act", "the failed read was cached and never retried" + + +@pytest.mark.asyncio +async def test_a_file_leaving_the_index_takes_its_row_with_it(tmp_path, media_cache): + clip = tmp_path / "01 - a track.mp3" + _make_clip(clip, artist="An Act") + entry = IndexEntry(id="content7", name=clip.name, path=clip.name, + size=clip.stat().st_size, type="audio", added_at=0) + await _enrich(AudioEnricher(media_cache), entry, clip) + assert await media_cache.get_audio_meta("content7") is not None + + await media_cache.prune_file("content7") + assert await media_cache.get_audio_meta("content7") is None |