diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-08-26 01:08:58 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-08-26 01:08:58 +0200 |
| commit | 2d144d76cee55cf8faaacf196e716a0930dfd7e9 (patch) | |
| tree | 1220d39cb30ccc8a58a80f85ba020b5b30bea9b7 /packages | |
| parent | 704cfe37506fc5316c997b025004fbc9c4d2a47b (diff) | |
| download | meshbay-2d144d76cee55cf8faaacf196e716a0930dfd7e9.tar.gz | |
fix(node,hub): key music/media metadata lookups by file_id, not path
IndexEntry.path is the *folder* a file is in (indexer.py's
_virtual_dir docstring: "the directory a file appears in"), not the
file itself. GroupIndex.get_entry_by_path() treated it as if it named
one file, and every one of its four callers did too:
_do_music_meta_request, _do_media_meta_request, _do_tmdb_override, and
_admin_exec_tmdb_override. Any two files sharing a folder — an album is
one folder with many tracks, a season is one folder with many episodes
— collided: a lookup by path silently returned whichever entry the
index happened to iterate to first, regardless of which file the
client actually asked about.
Found live (2026-08-25): three unrelated albums ("High Tone - Various",
two "Le Peuple de l'Herbe" albums) all showed the same MusicBrainz
cover, because all their representative tracks happened to sit in one
"high_tone" folder alongside a track that legitimately matched that
cover. A force-reload didn't help — the bug is server-side, not a
stale client state.
Fixed by keying these four request/response pairs by `file_id` (the
entry's own content hash — already unique, already how every other
lookup in the system identifies a file) instead of `path`, both in the
wire messages (music_meta_req/resp, media_meta_req/resp, tmdb_override)
and in music-app.js/video-app.js's own hooks. GroupIndex.get_entry_by_path
is now unused and removed — GroupIndex.get_entry(file_id) already did
the right thing.
No test previously exercised either handler with two entries sharing a
folder — the only existing coverage (test_tmdb_override_policy.py) gave
each entry its own folder, so the bug never had a chance to show up.
Added that scenario there and in two new test files, all confirmed
failing against the pre-fix code before being confirmed green against
the fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013XSohfUQQiaE77qyFLgSv3
Diffstat (limited to 'packages')
8 files changed, 413 insertions, 104 deletions
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 298e259..ae482ad 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/music-app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/music-app.js @@ -131,21 +131,21 @@ function groupMusicEntries(entries, audioRoot) { // -- album cover, MusicBrainz fetched lazily and only when actually needed -- -function useMusicMeta(transportRef, path, active) { +function useMusicMeta(transportRef, fileId, active) { const [meta, setMeta] = useState(null); useEffect(() => { - if (!active || !path) return; + if (!active || !fileId) return; let cancelled = false; (async () => { const transport = transportRef.current; if (!transport || !transport.connected) return; try { - const resp = await transport.fetchMusicMeta(path); + const resp = await transport.fetchMusicMeta(fileId); if (!cancelled) setMeta(resp); } catch { if (!cancelled) setMeta({ confidence: 0 }); } })(); return () => { cancelled = true; }; - }, [path, active]); + }, [fileId, active]); return meta; } @@ -196,7 +196,7 @@ function AlbumCard({ album, transportRef, gekRef, musicbrainzEnabled, onOpen }) // Only when nothing in the library already gives us a cover -- the common // case (a well-tagged rip with embedded art) needs no network call at all. const needsLookup = musicbrainzEnabled && !repTrack.thumb_hash; - const meta = useMusicMeta(transportRef, repTrack.path, needsLookup); + const meta = useMusicMeta(transportRef, repTrack.id, needsLookup); const coverHash = repTrack.thumb_hash || (meta && meta.cover_thumb_hash) || null; return html` @@ -218,7 +218,7 @@ function AlbumCard({ album, transportRef, gekRef, musicbrainzEnabled, onOpen }) function MusicDetailModal({ album, transportRef, gekRef, musicbrainzEnabled, onClose, onPlayQueue }) { const repTrack = album.tracks.find((tr) => tr.thumb_hash) || album.tracks[0]; const needsLookup = musicbrainzEnabled && !repTrack.thumb_hash; - const meta = useMusicMeta(transportRef, repTrack.path, needsLookup); + const meta = useMusicMeta(transportRef, repTrack.id, needsLookup); const coverHash = repTrack.thumb_hash || (meta && meta.cover_thumb_hash) || null; 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 065ccc0..7535594 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/transport.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/transport.js @@ -498,13 +498,17 @@ class MeshBayTransport { /** * TMDB metadata for one file (Videos app, docs/mediacenter.md §5.4). - * `path` is root+relpath, exactly what index_sync/index_delta already - * gave this browser — never a raw filesystem path constructed here. + * Keyed by the entry's own `id` (its content hash) — never a path: a + * path names the *folder* a file is in (indexer.py's `_virtual_dir`), so + * two files sharing a folder (any multi-episode season) would resolve to + * whichever entry the node's index happened to return first (found live + * via the Music app's identical bug, 2026-08-25 — see webrtc_server.py's + * `_do_media_meta_request`). * `confidence: 0` (no tmdb_id, no fields) means no confident match — * the caller falls back to a thumbnail-only card (§4.1), not an error. */ - async fetchMediaMeta(path) { - const msg = await this._sendAndWait({ type: 'media_meta_req', v: '0.5', path }); + async fetchMediaMeta(fileId) { + const msg = await this._sendAndWait({ type: 'media_meta_req', v: '0.6', file_id: fileId }); if (msg.type === 'error') throw new Error(msg.detail); return msg; } @@ -550,15 +554,16 @@ class MeshBayTransport { * override would let any member vandalize another show's metadata. * Applies to every file sharing the representative one's display_title, * not just the file the operator happened to be looking at (webrtc_ - * server.py's _admin_exec_tmdb_override). + * server.py's _admin_exec_tmdb_override). Keyed by `fileId`, not a path + * — same reasoning as fetchMediaMeta above. */ - async overrideTmdbMatch(path, tmdbId, mediaType, signFn) { + async overrideTmdbMatch(fileId, tmdbId, mediaType, signFn) { const msg = await this._sendAndWait({ - type: 'tmdb_override', v: '0.6', path, tmdb_id: tmdbId, media_type: mediaType, + type: 'tmdb_override', v: '0.7', file_id: fileId, tmdb_id: tmdbId, media_type: mediaType, }); if (msg.type === 'error') throw new Error(msg.detail); if (msg.type === 'admin_challenge') { - const subject = `path=${path},tmdb_id=${tmdbId},media_type=${mediaType}`; + const subject = `file_id=${fileId},tmdb_id=${tmdbId},media_type=${mediaType}`; return this._authorizeAdminOp(msg, 'tmdb_override', subject, signFn); } return msg; @@ -668,15 +673,19 @@ class MeshBayTransport { } /** - * MusicBrainz metadata for one track's path (Music app, docs/musicbay.md - * §4.3) — same shape as fetchMediaMeta, minus a season/episode concept: + * MusicBrainz metadata for one track (Music app, docs/musicbay.md §4.3) + * — same shape as fetchMediaMeta, minus a season/episode concept: * album-level (release), resolved from the track's own artist/album - * fields already in the index. `confidence: 0` means no confident match - * (or MusicBrainz off for this group, or nothing configured) — the caller - * falls back to the embedded/no cover it already had, not an error. + * fields already in the index. Keyed by the track's own `id` (content + * hash), not a path — a path names the *folder* a track is in, and an + * album is one folder with many tracks in it; three unrelated albums + * shared one folder's track's cover before this fix (found live, + * 2026-08-25). `confidence: 0` means no confident match (or MusicBrainz + * off for this group, or nothing configured) — the caller falls back to + * the embedded/no cover it already had, not an error. */ - async fetchMusicMeta(path) { - const msg = await this._sendAndWait({ type: 'music_meta_req', v: '0.8', path }); + async fetchMusicMeta(fileId) { + const msg = await this._sendAndWait({ type: 'music_meta_req', v: '0.9', file_id: fileId }); if (msg.type === 'error') throw new Error(msg.detail); return msg; } @@ -1467,10 +1476,10 @@ class MeshBayTransport { _key: obj.type === 'file_req' ? `chunk:${obj.file_id}:${obj.chunk_index}` : obj.type === 'ping' ? `ping:${obj.token}` - : obj.type === 'media_meta_req' ? `media_meta:${obj.path}` + : obj.type === 'media_meta_req' ? `media_meta:${obj.file_id}` // Same reordering hazard as media_meta_req: an album grid fires // one music_meta_req per visible tile, several at a time. - : obj.type === 'music_meta_req' ? `music_meta:${obj.path}` + : obj.type === 'music_meta_req' ? `music_meta:${obj.file_id}` // Same reordering hazard as media_meta_req: a season-tab bar or a // search box can have more than one of these in flight at once. : obj.type === 'season_meta_req' ? `season_meta:${obj.tmdb_id}:${obj.season}` @@ -1652,7 +1661,7 @@ class MeshBayTransport { // member_upload_ack/apps_enabled_ack above. if (msg.type === 'tmdb_override_ack' && this._onTmdbOverride) { this._onTmdbOverride({ - path: msg.path || '', tmdbId: msg.tmdb_id || '', mediaType: msg.media_type || '', + fileId: msg.file_id || '', tmdbId: msg.tmdb_id || '', mediaType: msg.media_type || '', }); } @@ -1774,21 +1783,21 @@ class MeshBayTransport { } if (msg.type === 'media_meta_resp') { - const key = `media_meta:${msg.path}`; + const key = `media_meta:${msg.file_id}`; for (const [, handler] of this._pending) { if (handler._key === key) { handler.resolve(msg); return; } } - // Nobody asked for this path any more (tile scrolled out and a fresh + // Nobody asked for this file any more (tile scrolled out and a fresh // request superseded it, most likely) — must not fall through to the // oldest pending request, which would hand a different tile's promise - // a TMDB result for a path it never asked about. + // a TMDB result for a file it never asked about. return; } // Same reasoning as media_meta_resp: keyed, not arrival-order, and // "nobody's waiting any more" must not fall through either. if (msg.type === 'music_meta_resp') { - const key = `music_meta:${msg.path}`; + const key = `music_meta:${msg.file_id}`; for (const [, handler] of this._pending) { if (handler._key === key) { handler.resolve(msg); return; } } diff --git a/packages/meshbay-hub/src/meshbay_hub/static/video-app.js b/packages/meshbay-hub/src/meshbay_hub/static/video-app.js index de61839..ce38b0e 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/video-app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/video-app.js @@ -190,43 +190,43 @@ function MediaThumb({ // ── TMDB metadata, fetched once per visible tile ──────────────────────────── // An operator correcting a wrong match (TmdbSearchOverlay below) changes -// what `media_meta_req` returns for a path every already-mounted tile/modal +// what `media_meta_req` returns for a file every already-mounted tile/modal // already has cached in its own useMediaMeta state — nothing would ever -// refetch otherwise, since path/active don't change. Bumping this and +// refetch otherwise, since fileId/active don't change. Bumping this and // telling every subscribed hook to redo its fetch is simpler than trying to -// know which paths a given override actually affects (that's server-side +// know which files a given override actually affects (that's server-side // knowledge — display_title grouping — this module doesn't have). const _mediaMetaListeners = new Set(); function bumpMediaMetaGeneration() { for (const fn of _mediaMetaListeners) fn(); } -function useMediaMeta(transportRef, path, active) { +function useMediaMeta(transportRef, fileId, active) { const [meta, setMeta] = useState(null); const [refetchToken, setRefetchToken] = useState(0); useEffect(() => { // Clears immediately (so the spinner shows right away, not only once // the new fetch resolves) and bumps the token, which re-runs the fetch - // effect below regardless of whether path/active changed at all. + // effect below regardless of whether fileId/active changed at all. const listener = () => { setMeta(null); setRefetchToken((n) => n + 1); }; _mediaMetaListeners.add(listener); return () => _mediaMetaListeners.delete(listener); }, []); useEffect(() => { - if (!active || !path) return; + if (!active || !fileId) return; let cancelled = false; (async () => { const transport = transportRef.current; if (!transport || !transport.connected) return; try { - const resp = await transport.fetchMediaMeta(path); + const resp = await transport.fetchMediaMeta(fileId); if (!cancelled) setMeta(resp); } catch { if (!cancelled) setMeta({ confidence: 0 }); } })(); return () => { cancelled = true; }; - }, [path, active, refetchToken]); + }, [fileId, active, refetchToken]); return meta; } @@ -266,7 +266,7 @@ function useSeasonMeta(transportRef, tmdbId, season, active) { // ── Mode A: poster grid ────────────────────────────────────────────────────── function PosterCard({ title, subtitle, repEntry, transportRef, gekRef, onOpen, groupKey, onMetaResolved }) { - const meta = useMediaMeta(transportRef, repEntry.path, true); + const meta = useMediaMeta(transportRef, repEntry.id, true); const confident = Boolean(meta && meta.confidence && meta.tmdb_id); const metaReady = meta !== null; @@ -366,7 +366,7 @@ function buildSignFn(transportRef) { } function TmdbSearchOverlay({ - initialQuery, mediaType, path, transportRef, gekRef, onClose, onApplied, + initialQuery, mediaType, fileId, transportRef, gekRef, onClose, onApplied, }) { const [query, setQuery] = useState(initialQuery || ''); const [results, setResults] = useState(null); // null = not searched yet @@ -398,14 +398,14 @@ function TmdbSearchOverlay({ setError(''); try { const signFn = buildSignFn(transportRef); - await transportRef.current.overrideTmdbMatch(path, tmdbId, mediaType, signFn); + await transportRef.current.overrideTmdbMatch(fileId, tmdbId, mediaType, signFn); bumpMediaMetaGeneration(); onApplied(); } catch (err) { setError(err.message); setApplying(false); } - }, [applying, path, mediaType, transportRef, onApplied]); + }, [applying, fileId, mediaType, transportRef, onApplied]); return html` <div class="video-overlay video-search-overlay" onClick=${(e) => { @@ -554,7 +554,7 @@ function VideoDetailModal({ </div> ${searching && html` <${TmdbSearchOverlay} initialQuery=${(confident && meta.title) || title} mediaType=${mediaType} - path=${repEntry.path} transportRef=${transportRef} gekRef=${gekRef} + fileId=${repEntry.id} transportRef=${transportRef} gekRef=${gekRef} onClose=${() => setSearching(false)} onApplied=${() => setSearching(false)} /> `} @@ -615,7 +615,7 @@ function PosterGrid({ movies, shows, transportRef, gekRef, onPreview, tmdbEnable }, [shows, metaByGroup]); const openDetail = (title, repEntry, show) => setDetail({ title, repEntry, show }); - const detailMeta = useMediaMeta(transportRef, detail ? detail.repEntry.path : null, !!detail); + const detailMeta = useMediaMeta(transportRef, detail ? detail.repEntry.id : null, !!detail); return html` <div class="video-grid"> diff --git a/packages/meshbay-node/src/meshbay_node/indexer/group_index.py b/packages/meshbay-node/src/meshbay_node/indexer/group_index.py index 1ce4e0a..25081ef 100644 --- a/packages/meshbay-node/src/meshbay_node/indexer/group_index.py +++ b/packages/meshbay-node/src/meshbay_node/indexer/group_index.py @@ -78,19 +78,6 @@ class GroupIndex: def get_entry(self, file_id: str) -> IndexEntry | None: return self._entries.get(file_id) - def get_entry_by_path(self, path: str) -> IndexEntry | None: - """ - Linear scan — entries are keyed by content id, not path, and nothing - before the Videos app needed to go the other way (a client always - already has the id from index_sync/index_delta). Fine for an - on-demand, per-tile lookup against a few thousand entries; revisit - if a future caller makes this hot. - """ - for entry in self._entries.values(): - if entry.path == path: - return entry - return None - @property def entries(self) -> list[IndexEntry]: return list(self._entries.values()) diff --git a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py index b4db051..6709fbc 100644 --- a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py +++ b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py @@ -2759,21 +2759,30 @@ class WebRTCPeerSession: async def _do_music_meta_request(self, msg: dict) -> None: """ - docs/musicbay.md §4.3: MusicBrainz metadata for one path, resolved - from the group's index. Album-level (release), the direct analogue - of Videos' show-level TMDB caching: one search per (artist, album) - pair serves cover art and canonical naming to every track of the - same release, keyed off the `artist`/`album` fields enrich_audio.py - already populated at index time (from tags, or the filename-parse - fallback) — never re-parsed here. + docs/musicbay.md §4.3: MusicBrainz metadata for one track, resolved + from the group's index by its content id. Album-level (release), the + direct analogue of Videos' show-level TMDB caching: one search per + (artist, album) pair serves cover art and canonical naming to every + track of the same release, keyed off the `artist`/`album` fields + enrich_audio.py already populated at index time (from tags, or the + filename-parse fallback) — never re-parsed here. + + Keyed by `file_id` (the entry's own content hash), not `path`: found + live (2026-08-25) — `IndexEntry.path` is the *folder* a file is in + (indexer.py's `_virtual_dir`), so any two tracks in the same folder + (routinely true — an album is one folder, many tracks) shared the + same `.path`, and looking a track up by it silently resolved to + whichever entry happened to be first in the index. Three unrelated + albums showed the same wrong cover before this fix, all sharing one + folder with the track that legitimately matched it. """ - path = msg.get("path") - log.debug("music_meta_req path=%r", path) - if not isinstance(path, str) or not path: - self._send({"type": "error", "detail": "Missing path"}) + file_id = msg.get("file_id") + log.debug("music_meta_req file_id=%r", file_id) + if not isinstance(file_id, str) or not file_id: + self._send({"type": "error", "detail": "Missing file_id"}) return ctx = self._group_ctx() - entry = ctx["index"].get_entry_by_path(path) + entry = ctx["index"].get_entry(file_id) if not entry: self._send({"type": "error", "detail": "File not found"}) return @@ -2789,7 +2798,7 @@ class WebRTCPeerSession: or not ctx.get("musicbrainz_enabled", True) or not entry.artist or not entry.album): self._send({"type": MNP.MUSIC_META_RESP, "v": MNP_VERSION, - "path": path, "confidence": 0}) + "file_id": file_id, "confidence": 0}) return mbid = await media_cache.get_file_mbid(entry.id) @@ -2799,7 +2808,7 @@ class WebRTCPeerSession: result, ratio = await musicbrainz_client.search_release(entry.artist, entry.album) if result is None or ratio < 0.6: self._send({"type": MNP.MUSIC_META_RESP, "v": MNP_VERSION, - "path": path, "confidence": 0}) + "file_id": file_id, "confidence": 0}) return mbid = result.get("id") artist_credit = result.get("artist-credit") or [] @@ -2814,11 +2823,11 @@ class WebRTCPeerSession: cover_thumb_hash = await self._fetch_and_cache_cover( media_cache, musicbrainz_client, mbid) - log.debug("music_meta_req path=%r: replying mbid=%s cover=%s", - path, mbid, cover_thumb_hash) + log.debug("music_meta_req file_id=%r: replying mbid=%s cover=%s", + file_id, mbid, cover_thumb_hash) self._send({ - "type": MNP.MUSIC_META_RESP, "v": MNP_VERSION, "path": path, + "type": MNP.MUSIC_META_RESP, "v": MNP_VERSION, "file_id": file_id, "mbid": mbid, "artist": meta.get("artist"), "album": meta.get("album"), @@ -2830,17 +2839,25 @@ class WebRTCPeerSession: async def _do_media_meta_request(self, msg: dict) -> None: """ - docs/mediacenter.md §5.4: TMDB metadata for one path, resolved from - the group's index (root+relpath the client already knows from - index_sync/index_delta — never a raw filesystem path off the wire). + docs/mediacenter.md §5.4: TMDB metadata for one file, resolved from + the group's index by its content id (root+relpath the client already + knows from index_sync/index_delta identify the entry; its own `id` + is what actually names one file — never a raw filesystem path off + the wire). + + Keyed by `file_id`, not `path`: `IndexEntry.path` is the *folder* a + file is in (indexer.py's `_virtual_dir`), so two files in the same + folder — any multi-episode season, routinely — shared the same + `.path`, and a lookup by it could silently resolve to the wrong + entry (found live via the Music app's identical bug, 2026-08-25). """ - path = msg.get("path") - log.debug("media_meta_req path=%r", path) - if not isinstance(path, str) or not path: - self._send({"type": "error", "detail": "Missing path"}) + file_id = msg.get("file_id") + log.debug("media_meta_req file_id=%r", file_id) + if not isinstance(file_id, str) or not file_id: + self._send({"type": "error", "detail": "Missing file_id"}) return ctx = self._group_ctx() - entry = ctx["index"].get_entry_by_path(path) + entry = ctx["index"].get_entry(file_id) if not entry: self._send({"type": "error", "detail": "File not found"}) return @@ -2853,7 +2870,7 @@ class WebRTCPeerSession: # TMDB match" as the ordinary case. if media_cache is None or tmdb_client is None or not ctx.get("tmdb_enabled", True): self._send({"type": MNP.MEDIA_META_RESP, "v": MNP_VERSION, - "path": path, "confidence": 0}) + "file_id": file_id, "confidence": 0}) return is_show = entry.season is not None and entry.episode is not None @@ -2870,7 +2887,7 @@ class WebRTCPeerSession: result, ratio = await self._tmdb_search(tmdb_client, entry, is_show) if result is None or ratio < 0.6: self._send({"type": MNP.MEDIA_META_RESP, "v": MNP_VERSION, - "path": path, "confidence": 0}) + "file_id": file_id, "confidence": 0}) return tmdb_id = str(result["id"]) meta = await self._tmdb_build_meta(tmdb_client, tmdb_id, media_type, result) @@ -2881,11 +2898,11 @@ class WebRTCPeerSession: media_cache, tmdb_client, meta.get("poster_path")) backdrop_thumb_hash = await self._fetch_and_cache_poster( media_cache, tmdb_client, meta.get("backdrop_path")) - log.debug("media_meta_req path=%r: replying tmdb_id=%s poster=%s backdrop=%s", - path, tmdb_id, poster_thumb_hash, backdrop_thumb_hash) + log.debug("media_meta_req file_id=%r: replying tmdb_id=%s poster=%s backdrop=%s", + file_id, tmdb_id, poster_thumb_hash, backdrop_thumb_hash) resp = { - "type": MNP.MEDIA_META_RESP, "v": MNP_VERSION, "path": path, + "type": MNP.MEDIA_META_RESP, "v": MNP_VERSION, "file_id": file_id, "tmdb_id": tmdb_id, "title": meta.get("title"), "original_title": meta.get("original_title"), "overview": meta.get("overview"), @@ -3013,25 +3030,28 @@ class WebRTCPeerSession: (§3.4/§V6) — not just the one file the operator happened to be looking at, so the correction actually sticks regardless of which episode a future render picks as representative. + + Keyed by `file_id`, not `path` — see `_do_media_meta_request`'s + docstring for why a folder-level path cannot name one file. """ - path = msg.get("path") + file_id = msg.get("file_id") tmdb_id = msg.get("tmdb_id") media_type = msg.get("media_type") - if not isinstance(path, str) or not path: - self._send({"type": "error", "detail": "Missing path"}) + if not isinstance(file_id, str) or not file_id: + self._send({"type": "error", "detail": "Missing file_id"}) return if not isinstance(tmdb_id, str) or not tmdb_id or media_type not in ("movie", "tv"): self._send({"type": "error", "detail": "Missing tmdb_id or media_type"}) return ctx = self._group_ctx() - entry = ctx["index"].get_entry_by_path(path) + entry = ctx["index"].get_entry(file_id) if not entry: self._send({"type": "error", "detail": "File not found"}) return if not self._has_admin_authority(): self._send({"type": "error", "detail": "No authorized key for this"}) return - subject = f"path={path},tmdb_id={tmdb_id},media_type={media_type}" + subject = f"file_id={file_id},tmdb_id={tmdb_id},media_type={media_type}" self._issue_admin_challenge(OP_TMDB_OVERRIDE, subject) async def _admin_exec_tmdb_override( @@ -3043,10 +3063,10 @@ class WebRTCPeerSession: self._audit("admin_auth_failed", f"tmdb_override:{subject}") return fields = dict(part.split("=", 1) for part in subject.split(",")) - path, tmdb_id, media_type = fields["path"], fields["tmdb_id"], fields["media_type"] + file_id, tmdb_id, media_type = fields["file_id"], fields["tmdb_id"], fields["media_type"] ctx = self._group_ctx() - entry = ctx["index"].get_entry_by_path(path) + entry = ctx["index"].get_entry(file_id) media_cache = self._ctx.get("media_cache") if entry is None or media_cache is None: self._send({"type": "error", "detail": "File or media cache not available"}) @@ -3058,7 +3078,7 @@ class WebRTCPeerSession: await media_cache.set_file_tmdb(e.id, tmdb_id, media_type) self._audit("tmdb_override", subject) - notice = {"type": MNP.TMDB_OVERRIDE_ACK, "v": MNP_VERSION, "path": path, + notice = {"type": MNP.TMDB_OVERRIDE_ACK, "v": MNP_VERSION, "file_id": file_id, "tmdb_id": tmdb_id, "media_type": media_type} for uid, session in list(self._peer_registry().items()): try: diff --git a/packages/meshbay-node/tests/test_media_meta_request.py b/packages/meshbay-node/tests/test_media_meta_request.py new file mode 100644 index 0000000..a7355e8 --- /dev/null +++ b/packages/meshbay-node/tests/test_media_meta_request.py @@ -0,0 +1,130 @@ +""" +`_do_media_meta_request`, keyed by `file_id` (2026-08-25 fix) — same +regression as test_music_meta_request.py, one app over: `IndexEntry.path` +is the *folder* a file is in, not the file itself, so a lookup by path +alone (the pre-fix `GroupIndex.get_entry_by_path`) silently resolved to +whichever entry the index happened to return first for that folder — a +real risk here too, since a season folder routinely holds many episodes. +""" + +import pytest +from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey +from meshbay_common.protocol import IndexEntry +from meshbay_node.indexer.group_index import GroupIndex +from meshbay_node.media_cache import MediaCache +from meshbay_node.transport.webrtc_server import WebRTCPeerSession + +pytestmark = pytest.mark.asyncio + + +class FakeTmdbClient: + """Returns a distinct, deterministic match per entry — real enough to + prove the server searched using the *right* entry's own fields.""" + + def __init__(self): + self.searched = [] + + async def search_movie(self, title): + self.searched.append(("movie", title)) + return {"id": 1000 + len(self.searched), "title": title, + "release_date": "2001-01-01"}, 1.0 + + async def search_tv(self, title): + self.searched.append(("tv", title)) + return {"id": 2000 + len(self.searched), "name": title, + "first_air_date": "2001-01-01"}, 1.0 + + async def fetch_image(self, url): + return f"image-bytes-for-{url}".encode() + + @staticmethod + def poster_url(path): + return f"https://image.tmdb.org/t/p/w500{path}" + + +def _entry(path: str, name: str, file_id: str, display_title: str) -> IndexEntry: + return IndexEntry( + id=file_id, name=name, path=path, size=1, type="video", added_at=0, + display_title=display_title, + ) + + +@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() + + +def _session(index, media_cache, tmdb_client): + session = WebRTCPeerSession.__new__(WebRTCPeerSession) + session._ctx = { + "index": index, + "media_cache": media_cache, + "tmdb_client": tmdb_client, + "tmdb_enabled": True, + } + session._group_id = None + session.sent = [] + session._send = session.sent.append + # _tmdb_search/_tmdb_build_meta are the real methods (not part of this + # regression) — stub the search ladder to a single direct call by + # display_title so this test is about routing, not TMDB matching. + async def _search(tmdb_client_, entry, is_show): + return await (tmdb_client_.search_tv(entry.display_title) if is_show + else tmdb_client_.search_movie(entry.display_title)) + session._tmdb_search = lambda *a: _search(*a) + + async def _build_meta(tmdb_client_, tmdb_id, media_type, result): + return { + "title": result.get("title") or result.get("name"), + "original_title": result.get("title") or result.get("name"), + "release_date": result.get("release_date"), + "first_air_date": result.get("first_air_date"), + "confidence": 1.0, + } + session._tmdb_build_meta = lambda *a: _build_meta(*a) + return session + + +async def test_two_episodes_in_the_same_season_folder_each_get_their_own_metadata(media_cache): + """The Videos-side analogue of the Music bug: two episodes share a + season folder, and each must resolve against its own entry — not + whichever one the index happens to return first for that folder.""" + index = GroupIndex(group_id="g" * 32, sk_node=Ed25519PrivateKey.generate()) + ep1 = _entry("shows/Show/Season 1", "s01e01.mkv", "id-1", "War of the Worlds") + ep2 = _entry("shows/Show/Season 1", "s01e02.mkv", "id-2", "A Different Show") + index.add_entry(ep1) + index.add_entry(ep2) + client = FakeTmdbClient() + session = _session(index, media_cache, client) + + await session._do_media_meta_request({"file_id": "id-1"}) + await session._do_media_meta_request({"file_id": "id-2"}) + + resp_1, resp_2 = session.sent + assert resp_1["file_id"] == "id-1" + assert resp_1["title"] == "War of the Worlds" + assert resp_2["file_id"] == "id-2" + assert resp_2["title"] == "A Different Show" + assert resp_1["tmdb_id"] != resp_2["tmdb_id"], ( + "two different shows sharing a season folder must not resolve to the same match") + + +async def test_missing_file_id_is_refused(media_cache): + index = GroupIndex(group_id="g" * 32, sk_node=Ed25519PrivateKey.generate()) + session = _session(index, media_cache, FakeTmdbClient()) + + await session._do_media_meta_request({}) + + assert session.sent == [{"type": "error", "detail": "Missing file_id"}] + + +async def test_unknown_file_id_is_refused(media_cache): + index = GroupIndex(group_id="g" * 32, sk_node=Ed25519PrivateKey.generate()) + session = _session(index, media_cache, FakeTmdbClient()) + + await session._do_media_meta_request({"file_id": "nope"}) + + assert session.sent == [{"type": "error", "detail": "File not found"}] diff --git a/packages/meshbay-node/tests/test_music_meta_request.py b/packages/meshbay-node/tests/test_music_meta_request.py new file mode 100644 index 0000000..96307f2 --- /dev/null +++ b/packages/meshbay-node/tests/test_music_meta_request.py @@ -0,0 +1,136 @@ +""" +`_do_music_meta_request`, keyed by `file_id` (2026-08-25 fix). + +Regression found live: `IndexEntry.path` is the *folder* a track is in +(indexer.py's `_virtual_dir`), not the track itself — an album is one +folder with many tracks in it, so looking a track up by `.path` alone +(the pre-fix behaviour, `GroupIndex.get_entry_by_path`) silently resolved +every track in that folder to whichever entry the index happened to +return first. Three unrelated albums showed the same wrong MusicBrainz +cover in production before this was found and fixed. +""" + +import pytest +from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey +from meshbay_common.protocol import MNP, IndexEntry +from meshbay_node.indexer.group_index import GroupIndex +from meshbay_node.media_cache import MediaCache +from meshbay_node.transport.webrtc_server import WebRTCPeerSession + +pytestmark = pytest.mark.asyncio + + +class FakeMusicBrainzClient: + """Returns a distinct, deterministic match per (artist, album) pair — + real enough to prove the server routed to the *right* track's own tags, + not a stand-in for musicbrainz.py's own search-quality tests.""" + + def __init__(self): + self.calls = [] + + async def search_release(self, artist, album): + self.calls.append((artist, album)) + return ( + {"id": f"mbid-for-{artist}-{album}", "title": album, + "artist-credit": [{"name": artist}]}, + 1.0, + ) + + async def fetch_cover_art(self, mbid): + return f"cover-bytes-for-{mbid}".encode() + + +def _entry(path: str, name: str, file_id: str, artist: str, album: str) -> IndexEntry: + return IndexEntry( + id=file_id, name=name, path=path, size=1, type="audio", added_at=0, + artist=artist, album=album, display_title=name, + ) + + +@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() + + +def _session(index, media_cache, musicbrainz_client): + session = WebRTCPeerSession.__new__(WebRTCPeerSession) + session._ctx = { + "index": index, + "media_cache": media_cache, + "musicbrainz_client": musicbrainz_client, + "musicbrainz_enabled": True, + } + session._group_id = None + session.sent = [] + session._send = session.sent.append + return session + + +async def test_two_tracks_in_the_same_folder_each_get_their_own_metadata(media_cache): + """The exact production scenario: two tracks share a folder (an album), + with different artist/album tags of their own (one mistagged, sitting + in the wrong physical folder — a real, if messy, real-world case). Each + must resolve against its *own* tags, not whichever track the index + happens to return first for that shared folder path.""" + index = GroupIndex(group_id="g" * 32, sk_node=Ed25519PrivateKey.generate()) + track_a = _entry("music/high_tone", "a.mp3", "id-a", "High Tone", "Future Dub (1)") + track_b = _entry("music/high_tone", "b.mp3", "id-b", "Le Peuple de l'Herbe", "Triple Zero") + index.add_entry(track_a) + index.add_entry(track_b) + client = FakeMusicBrainzClient() + session = _session(index, media_cache, client) + + await session._do_music_meta_request({"file_id": "id-a"}) + await session._do_music_meta_request({"file_id": "id-b"}) + + resp_a, resp_b = session.sent + assert resp_a["file_id"] == "id-a" + assert resp_a["artist"] == "High Tone" + assert resp_a["album"] == "Future Dub (1)" + assert resp_b["file_id"] == "id-b" + assert resp_b["artist"] == "Le Peuple de l'Herbe" + assert resp_b["album"] == "Triple Zero" + assert resp_a["cover_thumb_hash"] != resp_b["cover_thumb_hash"], ( + "two different tracks sharing a folder must not end up with the same cover") + assert set(client.calls) == {("High Tone", "Future Dub (1)"), + ("Le Peuple de l'Herbe", "Triple Zero")} + + +async def test_missing_file_id_is_refused(media_cache): + index = GroupIndex(group_id="g" * 32, sk_node=Ed25519PrivateKey.generate()) + session = _session(index, media_cache, FakeMusicBrainzClient()) + + await session._do_music_meta_request({}) + + assert session.sent == [{"type": "error", "detail": "Missing file_id"}] + + +async def test_unknown_file_id_is_refused(media_cache): + index = GroupIndex(group_id="g" * 32, sk_node=Ed25519PrivateKey.generate()) + session = _session(index, media_cache, FakeMusicBrainzClient()) + + await session._do_music_meta_request({"file_id": "nope"}) + + assert session.sent == [{"type": "error", "detail": "File not found"}] + + +async def test_no_confidence_below_threshold_still_answers_by_file_id(media_cache): + index = GroupIndex(group_id="g" * 32, sk_node=Ed25519PrivateKey.generate()) + index.add_entry(_entry("music/x", "a.mp3", "id-a", "Some Artist", "Some Album")) + + class LowConfidenceClient(FakeMusicBrainzClient): + async def search_release(self, artist, album): + return {"id": "mbid", "title": album, + "artist-credit": [{"name": artist}]}, 0.1 + + session = _session(index, media_cache, LowConfidenceClient()) + + await session._do_music_meta_request({"file_id": "id-a"}) + + assert session.sent == [{ + "type": MNP.MUSIC_META_RESP, "v": session.sent[0]["v"], + "file_id": "id-a", "confidence": 0, + }] diff --git a/packages/meshbay-node/tests/test_tmdb_override_policy.py b/packages/meshbay-node/tests/test_tmdb_override_policy.py index b8fa6f9..cd1cee1 100644 --- a/packages/meshbay-node/tests/test_tmdb_override_policy.py +++ b/packages/meshbay-node/tests/test_tmdb_override_policy.py @@ -65,7 +65,7 @@ def _entry(path: str, name: str, display_title: str) -> IndexEntry: # ── Refused before a challenge is even issued ─────────────────────────────── -async def test_missing_path_is_refused(tmp_path): +async def test_missing_file_id_is_refused(tmp_path): session = _session(tmp_path, "op", operator="op") session._has_admin_authority = lambda: True issued = [] @@ -79,24 +79,25 @@ async def test_missing_path_is_refused(tmp_path): async def test_missing_tmdb_id_is_refused(tmp_path): session = _session(tmp_path, "op", operator="op") - session._ctx["index"].add_entry(_entry("shared", "ep.mkv", "Show")) + entry = _entry("shared", "ep.mkv", "Show") + session._ctx["index"].add_entry(entry) session._has_admin_authority = lambda: True issued = [] session._issue_admin_challenge = lambda op, subject: issued.append((op, subject)) - session._do_tmdb_override({"path": "shared", "media_type": "tv"}) + session._do_tmdb_override({"file_id": entry.id, "media_type": "tv"}) assert not issued assert [m for m in session.sent if m.get("type") == "error"] -async def test_unknown_path_is_refused(tmp_path): +async def test_unknown_file_id_is_refused(tmp_path): session = _session(tmp_path, "op", operator="op") session._has_admin_authority = lambda: True issued = [] session._issue_admin_challenge = lambda op, subject: issued.append((op, subject)) - session._do_tmdb_override({"path": "nope", "tmdb_id": "123", "media_type": "tv"}) + session._do_tmdb_override({"file_id": "nope", "tmdb_id": "123", "media_type": "tv"}) assert not issued assert [m for m in session.sent if m.get("type") == "error"] @@ -104,25 +105,51 @@ async def test_unknown_path_is_refused(tmp_path): async def test_a_request_with_nobody_to_authorize_it_is_refused(tmp_path): session = _session(tmp_path, "member-1", operator="the-operator") - session._ctx["index"].add_entry(_entry("shared", "ep.mkv", "Show")) + entry = _entry("shared", "ep.mkv", "Show") + session._ctx["index"].add_entry(entry) session._has_admin_authority = lambda: False - session._do_tmdb_override({"path": "shared", "tmdb_id": "123", "media_type": "tv"}) + session._do_tmdb_override({"file_id": entry.id, "tmdb_id": "123", "media_type": "tv"}) assert [m for m in session.sent if m.get("type") == "error"] async def test_a_valid_request_is_signed(tmp_path): session = _session(tmp_path, "op", operator="op") - session._ctx["index"].add_entry(_entry("shared", "ep.mkv", "War of the Worlds")) + entry = _entry("shared", "ep.mkv", "War of the Worlds") + session._ctx["index"].add_entry(entry) session._has_admin_authority = lambda: True issued = [] session._issue_admin_challenge = lambda op, subject: issued.append((op, subject)) - session._do_tmdb_override({"path": "shared", "tmdb_id": "2255", "media_type": "tv"}) + session._do_tmdb_override({"file_id": entry.id, "tmdb_id": "2255", "media_type": "tv"}) assert issued == [(OP_TMDB_OVERRIDE, - "path=shared,tmdb_id=2255,media_type=tv")] + f"file_id={entry.id},tmdb_id=2255,media_type=tv")] + + +async def test_two_files_in_the_same_folder_are_told_apart(tmp_path): + """ + Regression (found live, 2026-08-25): `IndexEntry.path` is the *folder* a + file is in, not the file itself — two files in the same folder (any + multi-episode season) used to collide when looked up by path, silently + resolving to whichever entry the index happened to return first. Keyed + by `file_id` now, so two entries sharing a folder must resolve to their + own, distinct entries. + """ + session = _session(tmp_path, "op", operator="op") + e1 = _entry("shared/Season 1", "s01e01.mkv", "War of the Worlds") + e2 = _entry("shared/Season 1", "s01e02.mkv", "War of the Worlds") + session._ctx["index"].add_entry(e1) + session._ctx["index"].add_entry(e2) + session._has_admin_authority = lambda: True + issued = [] + session._issue_admin_challenge = lambda op, subject: issued.append((op, subject)) + + session._do_tmdb_override({"file_id": e2.id, "tmdb_id": "2255", "media_type": "tv"}) + + assert issued == [(OP_TMDB_OVERRIDE, + f"file_id={e2.id},tmdb_id=2255,media_type=tv")] # ── Applying the override ─────────────────────────────────────────────────── @@ -151,7 +178,7 @@ async def test_override_updates_every_entry_sharing_the_display_title(tmp_path): session._peer_registry = lambda: {"peer-1": peer} await session._admin_exec_tmdb_override( - {"subject": "path=shared/S1,tmdb_id=999,media_type=tv"}, + {"subject": f"file_id={s1.id},tmdb_id=999,media_type=tv"}, b"transcript", b"sig") for e in (s1, s2, s3): |