summaryrefslogtreecommitdiffstats
path: root/packages/meshbay-node/tests
diff options
context:
space:
mode:
authorChristophe Besson <cbesson@gmail.com>2026-08-26 01:08:58 +0200
committerChristophe Besson <cbesson@gmail.com>2026-08-26 01:08:58 +0200
commit2d144d76cee55cf8faaacf196e716a0930dfd7e9 (patch)
tree1220d39cb30ccc8a58a80f85ba020b5b30bea9b7 /packages/meshbay-node/tests
parent704cfe37506fc5316c997b025004fbc9c4d2a47b (diff)
downloadmeshbay-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/meshbay-node/tests')
-rw-r--r--packages/meshbay-node/tests/test_media_meta_request.py130
-rw-r--r--packages/meshbay-node/tests/test_music_meta_request.py136
-rw-r--r--packages/meshbay-node/tests/test_tmdb_override_policy.py49
3 files changed, 304 insertions, 11 deletions
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):