From 2d144d76cee55cf8faaacf196e716a0930dfd7e9 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Wed, 26 Aug 2026 01:08:58 +0200 Subject: fix(node,hub): key music/media metadata lookups by file_id, not path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_013XSohfUQQiaE77qyFLgSv3 --- .../meshbay-node/tests/test_media_meta_request.py | 130 +++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 packages/meshbay-node/tests/test_media_meta_request.py (limited to 'packages/meshbay-node/tests/test_media_meta_request.py') 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"}] -- cgit v1.2.3