MS Cin-1 + Cin-2: Explicit contract inheritance + generic accessors

Apply the Cin-1 / Cin-2 pattern from the download refactor PR to the
media server engine PR before review.

Cin-1 — explicit inheritance:
- PlexClient, JellyfinClient, NavidromeClient, SoulSyncClient now
  explicitly inherit MediaServerClient instead of relying on
  structural typing alone. Pre-change a reader of plex_client.py
  had no way to know the class was supposed to satisfy the contract.
- Removed the engine + registry re-exports from
  core/media_server/__init__.py to break the circular import that
  the inheritance change introduced (importing the package now
  triggered a chain that loaded clients before their base class
  resolved). Submodules import directly: from
  core.media_server.engine import MediaServerEngine, etc.
- Conformance test now also asserts isinstance() / issubclass()
  against MediaServerClient — drift in any class fails at the test
  boundary instead of at runtime.

Cin-2 — generic accessors + singleton:
- engine.configured_clients() — replaces the legacy per-server
  `if X and X.is_connected(): clients[name] = X` chains in
  web_server.py.
- engine.reload_config(name=None) — generic dispatch, so callers
  pass the server name instead of reaching for plex_client.reload_config()
  directly.
- get_media_server_engine() / set_media_server_engine() singleton
  factory matching the get_metadata_engine() / get_download_orchestrator()
  shape. web_server.py boots via set_media_server_engine(...) so
  factory + global handle share state.
- 7 new tests pin the accessors + singleton behaviour.

core/jellyfin_client.py

@@ -146,7 +146,10 @@ class JellyfinTrack:
             return self._client.get_album_by_id(self._album_id)
         return None
 
-class JellyfinClient:
+from core.media_server.contract import MediaServerClient
+
+
+class JellyfinClient(MediaServerClient):
     def __init__(self):
         self.base_url: Optional[str] = None
         self.api_key: Optional[str] = None

core/media_server/__init__.py

@@ -15,15 +15,19 @@ API, SoulSync's filesystem walk). The engine just routes by
 
 See ``docs/media-server-engine-refactor-plan.md`` for the full
 phased plan.
+
+Note: only ``MediaServerClient`` is re-exported here. The engine +
+registry are NOT — importing the registry triggers eager imports
+of every per-server client class, and those clients now inherit
+``MediaServerClient`` (Cin-1), so re-exporting them here would
+form a circular import the moment a client tried to resolve its
+base class. Import them directly from their submodules:
+    from core.media_server.engine import MediaServerEngine
+    from core.media_server.registry import build_default_registry
 """
 
 from core.media_server.contract import MediaServerClient
-from core.media_server.engine import MediaServerEngine
-from core.media_server.registry import MediaServerRegistry, build_default_registry
 
 __all__ = [
     "MediaServerClient",
-    "MediaServerEngine",
-    "MediaServerRegistry",
-    "build_default_registry",
 ]

core/media_server/engine.py

@@ -234,3 +234,73 @@ class MediaServerEngine:
                 )
                 return []
         return []
+
+    # ------------------------------------------------------------------
+    # Generic accessors — replace per-server attribute reaches in
+    # callers (Cin's standard from the download refactor).
+    # ------------------------------------------------------------------
+
+    def configured_clients(self) -> Dict[str, MediaServerClient]:
+        """Return ``{name: client}`` for every server that's both
+        registered AND reports ``is_connected() == True``. Replaces
+        the legacy per-server `if X and X.is_connected(): ...`
+        chains in web_server.py."""
+        result: Dict[str, MediaServerClient] = {}
+        for name, client in self.registry.all_clients():
+            try:
+                if not hasattr(client, 'is_connected') or client.is_connected():
+                    result[name] = client
+            except Exception as exc:
+                logger.debug("%s is_connected raised in configured_clients: %s", name, exc)
+        return result
+
+    def reload_config(self, name: Optional[str] = None) -> bool:
+        """Reload config on a single server (or every server when
+        ``name`` is None). Generic dispatch — caller passes the name
+        instead of reaching for ``plex_client.reload_config()``
+        / ``jellyfin_client.reload_config()`` directly. Servers
+        without a ``reload_config`` method are silently skipped.
+        """
+        names = [name] if name else list(self.registry.names())
+        ok = True
+        for n in names:
+            client = self.client(n)
+            if client is None or not hasattr(client, 'reload_config'):
+                continue
+            try:
+                client.reload_config()
+            except Exception as exc:
+                logger.warning("%s reload_config failed: %s", n, exc)
+                ok = False
+        return ok
+
+
+# ---------------------------------------------------------------------------
+# Singleton accessor — mirrors the get_metadata_engine() /
+# get_download_orchestrator() pattern so callers that don't need a
+# custom registry use this instead of instantiating MediaServerEngine
+# directly. web_server.py constructs the singleton at startup and
+# installs it via ``set_media_server_engine`` so the factory + the
+# global handle share state.
+# ---------------------------------------------------------------------------
+
+_default_engine: Optional['MediaServerEngine'] = None
+
+
+def get_media_server_engine() -> 'MediaServerEngine':
+    """Return (lazily creating) the process-wide MediaServerEngine
+    singleton. Mirrors the ``get_metadata_engine()`` /
+    ``get_download_orchestrator()`` shape."""
+    global _default_engine
+    if _default_engine is None:
+        _default_engine = MediaServerEngine()
+    return _default_engine
+
+
+def set_media_server_engine(engine: Optional['MediaServerEngine']) -> None:
+    """Set the process-wide singleton. Used by web_server.py at boot
+    to install the engine it constructs (with the pre-built per-client
+    instances) as the default for callers reaching via
+    ``get_media_server_engine()``."""
+    global _default_engine
+    _default_engine = engine

core/navidrome_client.py

@@ -150,7 +150,10 @@ class NavidromeTrack:
             return self._client.get_album_by_id(self._album_id)
         return None
 
-class NavidromeClient:
+from core.media_server.contract import MediaServerClient
+
+
+class NavidromeClient(MediaServerClient):
     def __init__(self):
         self.base_url: Optional[str] = None
         self.username: Optional[str] = None

core/plex_client.py

@@ -74,7 +74,10 @@ class PlexPlaylistInfo:
             tracks=tracks
         )
 
-class PlexClient:
+from core.media_server.contract import MediaServerClient
+
+
+class PlexClient(MediaServerClient):
     def __init__(self):
         self.server: Optional[PlexServer] = None
         self.music_library: Optional[MusicSection] = None

core/soulsync_client.py

@@ -190,7 +190,10 @@ class SoulSyncArtist:
         return self._albums
 
 
-class SoulSyncClient:
+from core.media_server.contract import MediaServerClient
+
+
+class SoulSyncClient(MediaServerClient):
     """Filesystem-based media server client for standalone SoulSync operation.
 
     Scans the Transfer folder recursively, reads audio file tags, and

tests/media_server/test_conformance.py

@@ -32,7 +32,7 @@ def _import_server_classes():
 def test_default_registry_registers_all_four_servers():
     """Smoke check that the foundation registry knows about every
     server SoulSync historically dispatched to."""
-    from core.media_server import build_default_registry
+    from core.media_server.registry import build_default_registry
 
     registry = build_default_registry()
     expected = {'plex', 'jellyfin', 'navidrome', 'soulsync'}
@@ -52,3 +52,21 @@ def test_server_class_has_all_required_methods(server_name):
     assert not missing, (
         f"{server_name} ({cls.__name__}) missing required methods: {missing}"
     )
+
+
+@pytest.mark.parametrize('server_name', ['plex', 'jellyfin', 'navidrome', 'soulsync'])
+def test_server_class_explicitly_inherits_contract(server_name):
+    """Per Cin's standard from the download refactor: clients must
+    explicitly inherit ``MediaServerClient`` so the contract conformance
+    is obvious from reading the class declaration. Structural
+    typing alone (which would still pass `hasattr` checks) leaves
+    the contract invisible to anyone reading the code — drift in a
+    future client class wouldn't fail at the contract boundary."""
+    from core.media_server.contract import MediaServerClient
+
+    classes = _import_server_classes()
+    cls = classes[server_name]
+    assert issubclass(cls, MediaServerClient), (
+        f"{cls.__name__} must explicitly inherit MediaServerClient — "
+        f"structural typing isn't enough"
+    )

tests/media_server/test_engine.py

@@ -6,8 +6,8 @@ from unittest.mock import MagicMock
 
 import pytest
 
-from core.media_server import MediaServerEngine, MediaServerRegistry
+from core.media_server.engine import MediaServerEngine
-from core.media_server.registry import ServerSpec
+from core.media_server.registry import MediaServerRegistry, ServerSpec
 
 
 class _FakeClient:
@@ -190,3 +190,107 @@ def test_is_library_scanning_returns_false_when_client_lacks_method(make_engine)
 def test_get_library_stats_returns_empty_dict_when_client_lacks_method(make_engine):
     engine = make_engine({'soulsync': _MinimalClient()}, active='soulsync')
     assert engine.get_library_stats() == {}
+
+
+# ---------------------------------------------------------------------------
+# Generic accessors (Cin's standard from the download refactor)
+# ---------------------------------------------------------------------------
+
+
+def test_configured_clients_only_returns_connected_servers(make_engine):
+    """Replaces the legacy per-server `if X and X.is_connected(): ...`
+    chains in web_server.py. Single call returns the dict."""
+    plex = _FakeClient('plex', connected=True)
+    jelly = _FakeClient('jellyfin', connected=False)
+    soulsync = _FakeClient('soulsync', connected=True)
+    engine = make_engine(
+        {'plex': plex, 'jellyfin': jelly, 'soulsync': soulsync},
+        active='plex',
+    )
+    result = engine.configured_clients()
+    assert set(result.keys()) == {'plex', 'soulsync'}
+    assert result['plex'] is plex
+    assert result['soulsync'] is soulsync
+
+
+def test_configured_clients_skips_clients_whose_is_connected_raises(make_engine):
+    """Defensive: a single broken is_connected() must not crash the
+    iteration. Healthy clients still come back."""
+    healthy = _FakeClient('plex', connected=True)
+    broken = _FakeClient('jellyfin')
+    broken.is_connected = MagicMock(side_effect=RuntimeError("boom"))
+    engine = make_engine({'plex': healthy, 'jellyfin': broken}, active='plex')
+    result = engine.configured_clients()
+    assert 'plex' in result
+    assert 'jellyfin' not in result
+
+
+def test_reload_config_dispatches_to_named_server(make_engine):
+    """Generic dispatch — caller passes server name instead of
+    reaching for plex_client.reload_config() directly."""
+
+    class _ReloadablePlex(_FakeClient):
+        def __init__(self):
+            super().__init__('plex')
+            self.reload_called = False
+
+        def reload_config(self):
+            self.reload_called = True
+
+    plex = _ReloadablePlex()
+    soulsync = _FakeClient('soulsync')  # No reload_config method
+    engine = make_engine({'plex': plex, 'soulsync': soulsync}, active='plex')
+
+    assert engine.reload_config('plex') is True
+    assert plex.reload_called is True
+
+
+def test_reload_config_skips_clients_without_method(make_engine):
+    """Servers that don't expose reload_config are skipped silently
+    (return True)."""
+    soulsync = _FakeClient('soulsync')
+    engine = make_engine({'soulsync': soulsync}, active='soulsync')
+    assert engine.reload_config('soulsync') is True
+
+
+def test_reload_config_with_no_args_reloads_every_server(make_engine):
+    """When called with no name, hits every registered server that
+    exposes reload_config."""
+
+    class _ReloadableClient(_FakeClient):
+        def __init__(self, name):
+            super().__init__(name)
+            self.reload_called = False
+
+        def reload_config(self):
+            self.reload_called = True
+
+    plex = _ReloadableClient('plex')
+    jelly = _ReloadableClient('jellyfin')
+    engine = make_engine({'plex': plex, 'jellyfin': jelly}, active='plex')
+
+    engine.reload_config()
+    assert plex.reload_called is True
+    assert jelly.reload_called is True
+
+
+# ---------------------------------------------------------------------------
+# Singleton factory (matches get_metadata_engine() / get_download_orchestrator())
+# ---------------------------------------------------------------------------
+
+
+def test_get_media_server_engine_returns_set_singleton(make_engine):
+    """When set_media_server_engine has been called (web_server.py
+    does this at boot), get_media_server_engine returns the installed
+    instance instead of building a fresh one with the default registry."""
+    from core.media_server.engine import (
+        get_media_server_engine,
+        set_media_server_engine,
+    )
+
+    engine = make_engine({'plex': _FakeClient('plex')}, active='plex')
+    set_media_server_engine(engine)
+    try:
+        assert get_media_server_engine() is engine
+    finally:
+        set_media_server_engine(None)

web_server.py

@@ -608,13 +608,18 @@ except Exception as e:
 # ``engine.method()`` dispatch in place of the historic
 # ``if active_server == 'plex' / 'jellyfin' / ...`` chains.
 try:
-    from core.media_server import MediaServerEngine
+    from core.media_server.engine import MediaServerEngine, set_media_server_engine
     media_server_engine = MediaServerEngine(clients={
         'plex':      plex_client,
         'jellyfin':  jellyfin_client,
         'navidrome': navidrome_client,
         'soulsync':  soulsync_library_client,
     })
+    # Install as process-wide singleton so callers reaching via
+    # get_media_server_engine() see the same instance web_server.py
+    # constructs at boot. Matches the metadata + download engine
+    # patterns.
+    set_media_server_engine(media_server_engine)
     logger.info("  Media server engine initialized")
 except Exception as e:
     logger.error(f"  Media server engine failed to initialize: {e}")