From cf655c50090b4457408fc9e59414ac2de41582e4 Mon Sep 17 00:00:00 2001
From: BoulderBadgeDad <batmanesignal@gmail.com>
Date: Thu, 4 Jun 2026 11:41:03 -0700
Subject: [PATCH] Fix #789: Navidrome library selection ignored (all libraries
 imported)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The saved music-folder selection was silently dropped on every reconnect.
_setup_client's restore step called the public get_music_folders(), which
starts with ensure_connection() — but we're already inside ensure_connection()
at that point (_is_connecting=True, _connection_attempted not yet set), so the
re-entrant call bailed and returned []. The restore matched nothing,
music_folder_id stayed None, and the per-call musicFolderId filters all
no-op'd → scans imported every library regardless of the user's choice.
Surfaces after any restart or settings save (reload_config resets the state).

Split get_music_folders() into the public method (does the connection check)
and a non-reentrant _fetch_music_folders() seam; the restore now calls the
seam directly (connection is already established + ping succeeded by then).

Regression + seam tests added.
---
 core/navidrome_client.py                     | 14 +++++-
 tests/media_server/test_navidrome_pinning.py | 47 ++++++++++++++++++++
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/core/navidrome_client.py b/core/navidrome_client.py
index 765153bc..fcf65bb6 100644
--- a/core/navidrome_client.py
+++ b/core/navidrome_client.py
@@ -192,6 +192,14 @@ class NavidromeClient(MediaServerClient):
         """Get available music folders from Navidrome."""
         if not self.ensure_connection():
             return []
+        return self._fetch_music_folders()
+
+    def _fetch_music_folders(self) -> list:
+        """Fetch + parse the Navidrome music folders. Assumes the connection is
+        already established (base_url/username set) and does NOT call
+        ensure_connection — so it is safe to call from inside _setup_client's
+        restore step without re-entering the connection guard (which would
+        otherwise bail with _is_connecting=True and return an empty list)."""
         try:
             response = self._make_request('getMusicFolders')
             if not response:
@@ -278,7 +286,11 @@ class NavidromeClient(MediaServerClient):
                     db = MusicDatabase()
                     saved_folder = db.get_preference('navidrome_music_folder')
                     if saved_folder:
-                        folders = self.get_music_folders()
+                        # Use the non-reentrant fetch: we're still inside
+                        # ensure_connection() here (_is_connecting=True), so the
+                        # public get_music_folders() would re-enter the guard and
+                        # return [], silently dropping the saved selection.
+                        folders = self._fetch_music_folders()
                         for folder in folders:
                             if folder['title'] == saved_folder:
                                 self.music_folder_id = folder['key']
diff --git a/tests/media_server/test_navidrome_pinning.py b/tests/media_server/test_navidrome_pinning.py
index 7850743f..76d15070 100644
--- a/tests/media_server/test_navidrome_pinning.py
+++ b/tests/media_server/test_navidrome_pinning.py
@@ -79,6 +79,53 @@ def test_get_all_album_ids_returns_set(nav_client):
     assert result == {'nav-1', 'nav-2'}
 
 
+def test_fetch_music_folders_parses_without_ensure_connection(nav_client):
+    """The seam: _fetch_music_folders does its own _make_request + parse and
+    does NOT gate on ensure_connection, so it is safe to call mid-connect."""
+    nav_client.base_url = 'http://nav'
+    nav_client.username = 'u'
+    nav_client.password = 'p'
+    folders_envelope = {'musicFolders': {'musicFolder': [
+        {'id': 1, 'name': 'Music'},
+        {'id': 2, 'name': 'Audiobooks'},
+    ]}}
+    with patch.object(nav_client, '_make_request', return_value=folders_envelope):
+        folders = nav_client._fetch_music_folders()
+    assert folders == [
+        {'title': 'Music', 'key': '1'},
+        {'title': 'Audiobooks', 'key': '2'},
+    ]
+
+
+def test_setup_client_restores_saved_music_folder(nav_client):
+    """Regression for #789: a saved music-folder selection must survive
+    _setup_client. The restore runs while still inside ensure_connection()
+    (_is_connecting=True); the old code called the public get_music_folders(),
+    which re-entered the guard, got [], and left music_folder_id=None — so
+    every scan imported all libraries regardless of the user's selection."""
+    def fake_request(endpoint, params=None):
+        if endpoint == 'ping':
+            return {'status': 'ok', 'version': '1.16.1'}
+        if endpoint == 'getMusicFolders':
+            return {'musicFolders': {'musicFolder': [
+                {'id': 1, 'name': 'Music'},
+                {'id': 2, 'name': 'Audiobooks'},
+            ]}}
+        return None
+
+    fake_db = MagicMock()
+    fake_db.get_preference.return_value = 'Audiobooks'
+
+    with patch('core.navidrome_client.config_manager.get_navidrome_config',
+               return_value={'base_url': 'http://nav', 'username': 'u', 'password': 'p'}), \
+         patch('database.music_database.MusicDatabase', return_value=fake_db), \
+         patch.object(nav_client, '_make_request', side_effect=fake_request):
+        assert nav_client.ensure_connection() is True
+
+    # Selection ('Audiobooks') was restored to its folder id, not left None.
+    assert nav_client.music_folder_id == '2'
+
+
 def test_navidrome_album_exposes_cover_art_url(nav_client):
     album = NavidromeAlbum({
         'id': 'album-1',