Cin review: alias resolution, atomic terminal write, generic accessors

Three correctness fixes from kettui's PR review plus the web_server
migration to generic accessors.

- Engine alias map: register_plugin accepts aliases tuple; get_plugin
  + cancel_download resolve through it. Fixes deezer_dl cancels
  silently routing to soulseek.
- Orchestrator hybrid_order normalization: _resolve_source_chain
  routes raw config names through registry.get_spec() so legacy
  deezer_dl entries don't drop deezer from hybrid mode.
- Atomic update_record_unless_state on the engine: holds state_lock
  across the check + write. Both _mark_terminal AND the success path
  use it now so a Cancelled state set mid-impl can't be clobbered.
- web_server.py: 30 soulseek_client.<source> reaches migrated to
  client("<source>"); shutdown-check setup migrated to generic
  registry iteration; 4 hifi reload sites use reload_instances('hifi').
- 18 new tests pin every fix.

core/download_engine/engine.py

@@ -65,11 +65,15 @@ class DownloadEngine:
         # holding the lock for its own update.
         self._records: Dict[Tuple[str, str], DownloadRecord] = {}
         # Plugins that have registered with the engine. Source name
-        # → plugin instance. The engine itself doesn't use plugins
-        # until later phases, but holding the references here keeps
-        # plugin lookup local to the engine instead of forcing every
-        # caller to also touch the registry.
+        # → plugin instance.
         self._plugins: Dict[str, Any] = {}
+        # Alias → canonical-name map. Lets engine resolve legacy
+        # source-name strings (e.g. ``'deezer_dl'`` for Deezer) to
+        # the canonical key in ``_plugins``. Cin's review caught
+        # that engine.cancel_download(source_hint='deezer_dl')
+        # silently fell through to Soulseek because alias resolution
+        # only existed at the registry, not on the engine.
+        self._aliases: Dict[str, str] = {}
         # Background download worker — lives on the engine because
         # it owns the cross-source state the worker mutates. Lazy
         # import keeps the engine module standalone.
@@ -80,11 +84,17 @@ class DownloadEngine:
     # Plugin registration
     # ------------------------------------------------------------------
 
-    def register_plugin(self, source_name: str, plugin: Any) -> None:
+    def register_plugin(self, source_name: str, plugin: Any,
+                        aliases: Tuple[str, ...] = ()) -> None:
         """Register a plugin under its canonical source name. Called
         once per source by the orchestrator after the registry's
         ``initialize`` builds the client instances.
 
+        ``aliases`` is the list of legacy source-name strings that
+        should resolve to this plugin (e.g. ``'deezer_dl'`` for
+        Deezer). Without alias resolution the engine couldn't route
+        cancel/lookup calls that came in with the legacy name.
+
         If the plugin exposes ``set_engine(engine)``, the engine
         passes a self-reference so the plugin can dispatch into
         ``engine.worker`` / read state / etc. Plugins that haven't
@@ -101,6 +111,8 @@ class DownloadEngine:
         if source_name in self._plugins:
             logger.warning("Plugin %s already registered with engine — overwriting", source_name)
         self._plugins[source_name] = plugin
+        for alias in aliases:
+            self._aliases[alias] = source_name
 
         # Apply the plugin's rate-limit policy BEFORE set_engine so
         # set_engine callbacks can override per-source if they need
@@ -120,7 +132,23 @@ class DownloadEngine:
                 )
 
     def get_plugin(self, source_name: str) -> Optional[Any]:
-        return self._plugins.get(source_name)
+        """Return the plugin instance for the given source name.
+        Resolves through aliases — e.g. ``get_plugin('deezer_dl')``
+        returns the same instance as ``get_plugin('deezer')``."""
+        if source_name in self._plugins:
+            return self._plugins[source_name]
+        canonical = self._aliases.get(source_name)
+        if canonical:
+            return self._plugins.get(canonical)
+        return None
+
+    def _resolve_canonical(self, source_name: str) -> Optional[str]:
+        """Return the canonical source name for an input that may be
+        an alias. Returns None if the input matches neither a
+        canonical name nor an alias."""
+        if source_name in self._plugins:
+            return source_name
+        return self._aliases.get(source_name)
 
     def registered_sources(self) -> List[str]:
         return list(self._plugins.keys())
@@ -148,6 +176,29 @@ class DownloadEngine:
                 return
             existing.update(patch)
 
+    def update_record_unless_state(self, source_name: str, download_id: str,
+                                   patch: DownloadRecord,
+                                   skip_if_state_in: Tuple[str, ...] = ()) -> bool:
+        """Atomically check the record's state and apply ``patch`` only
+        if the current state is NOT in ``skip_if_state_in``. Returns
+        True if the patch was applied, False if it was skipped (or
+        the record didn't exist).
+
+        Used by the background download worker's ``_mark_terminal``
+        to avoid the read-then-write race Cin flagged: a cancel
+        landing between the snapshot and update could be overwritten
+        back to Errored / Completed. Holding ``state_lock`` across
+        the check + write closes the window.
+        """
+        with self.state_lock:
+            existing = self._records.get((source_name, download_id))
+            if existing is None:
+                return False
+            if existing.get('state') in skip_if_state_in:
+                return False
+            existing.update(patch)
+            return True
+
     def remove_record(self, source_name: str, download_id: str) -> Optional[DownloadRecord]:
         """Delete a record (cancellation cleanup). Returns the
         removed record or None if not found."""
@@ -226,23 +277,31 @@ class DownloadEngine:
                               source_hint: Optional[str] = None,
                               remove: bool = False) -> bool:
         """Cancel a download. ``source_hint`` is the source name (or
-        legacy username string like ``'deezer_dl'``) — when provided,
-        routes directly to that plugin. When omitted, every plugin
-        is asked in turn until one accepts the cancel."""
+        legacy alias like ``'deezer_dl'``, or a real Soulseek peer
+        username) — when provided, routes directly to that plugin.
+        When omitted, every plugin is asked in turn until one accepts.
+
+        Cin's review caught a bug here: legacy alias strings like
+        ``'deezer_dl'`` weren't resolved to the canonical ``'deezer'``
+        plugin name, so the cancel silently fell through to Soulseek.
+        Resolution now goes through ``_resolve_canonical`` first.
+        """
         # Direct routing when the caller knows the source.
         if source_hint:
-            # Streaming source names ARE the username. Soulseek
-            # uses a real peer username (anything not in our plugin
-            # registry), so route those to the soulseek plugin.
-            target_plugin = self._plugins.get(source_hint)
-            if target_plugin is not None and source_hint != 'soulseek':
-                try:
-                    return await target_plugin.cancel_download(
-                        download_id, source_hint, remove,
-                    )
-                except Exception as exc:
-                    logger.debug("%s cancel_download failed: %s", source_hint, exc)
-                    return False
+            canonical = self._resolve_canonical(source_hint)
+            # Streaming source names (or aliases) resolve to a
+            # registered plugin. Anything else (real Soulseek peer
+            # name not in our registry) routes to Soulseek.
+            if canonical and canonical != 'soulseek':
+                target_plugin = self._plugins.get(canonical)
+                if target_plugin is not None:
+                    try:
+                        return await target_plugin.cancel_download(
+                            download_id, source_hint, remove,
+                        )
+                    except Exception as exc:
+                        logger.debug("%s cancel_download failed: %s", canonical, exc)
+                        return False
             soulseek = self._plugins.get('soulseek')
             if soulseek is not None:
                 try:

core/download_engine/worker.py

@@ -256,11 +256,18 @@ class BackgroundDownloadWorker:
                 self._last_download_at[source_name] = time.time()
 
                 if file_path:
-                    self._engine.update_record(source_name, download_id, {
-                        'state': 'Completed, Succeeded',
-                        'progress': 100.0,
-                        'file_path': file_path,
-                    })
+                    # Atomic write — preserve Cancelled if user cancelled
+                    # between impl returning and this write. Same guard
+                    # _mark_terminal uses; Cin flagged both split sites.
+                    self._engine.update_record_unless_state(
+                        source_name, download_id,
+                        {
+                            'state': 'Completed, Succeeded',
+                            'progress': 100.0,
+                            'file_path': file_path,
+                        },
+                        skip_if_state_in=('Cancelled',),
+                    )
                     logger.info(
                         "%s download %s completed: %s",
                         source_name, download_id, file_path,
@@ -291,15 +298,18 @@ class BackgroundDownloadWorker:
         'Cancelled' state set by the user via cancel_download.
         Mirrors the legacy per-client guard
         (``if state != 'Cancelled': state = 'Errored'``) every
-        client used to hand-roll inside its thread worker."""
-        record = self._engine.get_record(source_name, download_id)
-        if record is None:
-            return  # already removed (e.g. cancel + remove)
-        if record.get('state') == 'Cancelled':
-            return  # user explicitly cancelled — leave it
+        client used to hand-roll inside its thread worker.
+
+        Uses ``update_record_unless_state`` so the check + write are
+        atomic under the engine's state_lock. Cin caught a race
+        where a cancel landing between the read-snapshot + write
+        could overwrite Cancelled back to Errored / Completed.
+        """
         patch: Dict[str, Any] = {
             'state': 'Completed, Succeeded' if success else 'Errored',
         }
         if error is not None:
             patch['error'] = error
-        self._engine.update_record(source_name, download_id, patch)
+        self._engine.update_record_unless_state(
+            source_name, download_id, patch, skip_if_state_in=('Cancelled',),
+        )

core/download_orchestrator.py

@@ -76,7 +76,9 @@ class DownloadOrchestrator:
         # can route through it without further orchestrator changes.
         self.engine = engine if engine is not None else DownloadEngine()
         for source_name, plugin in self.registry.all_plugins():
-            self.engine.register_plugin(source_name, plugin)
+            spec = self.registry.get_spec(source_name)
+            aliases = spec.aliases if spec else ()
+            self.engine.register_plugin(source_name, plugin, aliases=aliases)
 
         if self._init_failures:
             logger.warning(f"Download clients failed to initialize: {', '.join(self._init_failures)}")
@@ -246,17 +248,42 @@ class DownloadOrchestrator:
 
         return False
 
+    def _normalize_source_name(self, name: str) -> Optional[str]:
+        """Convert a possibly-aliased source name (e.g. legacy
+        ``'deezer_dl'``) to the canonical registry name (``'deezer'``).
+        Returns None if the input matches neither a canonical name
+        nor an alias.
+
+        Cin's review caught a bug where legacy alias values from
+        config (hybrid_order containing ``'deezer_dl'``) silently
+        dropped Deezer from hybrid mode because the canonical-name
+        membership check rejected the alias.
+        """
+        spec = self.registry.get_spec(name) if name else None
+        return spec.name if spec else None
+
     def _resolve_source_chain(self) -> List[str]:
         """Order the configured sources for hybrid mode. Prefers
         ``hybrid_order`` config; falls back to legacy
-        primary/secondary pair when no order set."""
-        registry_names = set(self.registry.names())
+        primary/secondary pair when no order set. Normalizes alias
+        names through the registry so legacy ``deezer_dl`` config
+        values resolve correctly to the canonical ``deezer`` plugin."""
         if self.hybrid_order:
-            return [s for s in self.hybrid_order if s in registry_names]
-        primary = self.hybrid_primary if self.hybrid_primary in registry_names else 'soulseek'
-        secondary = self.hybrid_secondary if self.hybrid_secondary in registry_names else 'soulseek'
+            chain = []
+            seen = set()
+            for raw in self.hybrid_order:
+                canonical = self._normalize_source_name(raw)
+                if canonical and canonical not in seen:
+                    chain.append(canonical)
+                    seen.add(canonical)
+            return chain
+        primary = self._normalize_source_name(self.hybrid_primary) or 'soulseek'
+        secondary = self._normalize_source_name(self.hybrid_secondary) or 'soulseek'
         if secondary == primary:
-            secondary = next((name for name in registry_names if name != primary), 'soulseek')
+            secondary = next(
+                (name for name in self.registry.names() if name != primary),
+                'soulseek',
+            )
         chain = [primary, secondary]
         if not chain:
             chain = ['soulseek']

tests/downloads/test_background_download_worker.py

@@ -190,6 +190,40 @@ def test_worker_preserves_cancelled_when_impl_returns_none():
     )
 
 
+def test_worker_preserves_cancelled_when_impl_returns_success():
+    """Cin's bug 3 follow-up: the success path also has a read-then-write
+    race. If the user cancels between the impl returning a valid file
+    path and the worker writing 'Completed, Succeeded', the cancel is
+    overwritten. The success-path write must use the same atomic
+    Cancelled-preserve guard as _mark_terminal."""
+    engine = DownloadEngine()
+
+    def impl(download_id, target_id, display_name):
+        # User cancels mid-impl, then impl finishes successfully.
+        engine.update_record('youtube', download_id, {'state': 'Cancelled'})
+        return '/tmp/file.flac'
+
+    download_id = engine.worker.dispatch(
+        source_name='youtube',
+        target_id='vid',
+        display_name='X',
+        original_filename='vid||X',
+        impl_callable=impl,
+    )
+
+    deadline = time.time() + 2.0
+    while time.time() < deadline:
+        record = engine.get_record('youtube', download_id)
+        if record and record['state'] in ('Cancelled', 'Completed, Succeeded'):
+            break
+        time.sleep(0.01)
+
+    record = engine.get_record('youtube', download_id)
+    assert record['state'] == 'Cancelled', (
+        f"Worker clobbered user's Cancelled with {record['state']}"
+    )
+
+
 def test_worker_preserves_cancelled_when_impl_raises():
     """Same Cancelled-preserve guard, but for the impl-raises path."""
     engine = DownloadEngine()

tests/downloads/test_download_engine.py

@@ -513,3 +513,80 @@ def test_download_with_fallback_continues_past_exception():
         'youtube', 'v||t', 0, ['youtube', 'tidal'],
     ))
     assert result == 'td-id'
+
+
+# ---------------------------------------------------------------------------
+# Cin bug 1: alias resolution on cancel_download
+# ---------------------------------------------------------------------------
+
+
+def test_register_plugin_records_aliases():
+    """Aliases passed to register_plugin resolve to the canonical plugin
+    via get_plugin. Cin caught engine.cancel_download routing 'deezer_dl'
+    to soulseek because the alias never made it to the engine."""
+    engine = DownloadEngine()
+    deezer = _FakePlugin('deezer')
+    engine.register_plugin('deezer', deezer, aliases=('deezer_dl',))
+
+    assert engine.get_plugin('deezer') is deezer
+    assert engine.get_plugin('deezer_dl') is deezer
+
+
+def test_cancel_download_resolves_alias_to_canonical_plugin():
+    """The legacy 'deezer_dl' source_hint must route to the deezer
+    plugin, not fall through to soulseek. This was Cin's bug 1 —
+    cancel of a Deezer download silently no-op'd."""
+    engine = DownloadEngine()
+    soulseek = _FakePlugin('soulseek')
+    deezer = _FakePlugin('deezer')
+    engine.register_plugin('soulseek', soulseek)
+    engine.register_plugin('deezer', deezer, aliases=('deezer_dl',))
+
+    _run_async(engine.cancel_download('dl-1', 'deezer_dl', remove=False))
+    assert deezer.cancel_calls == [('dl-1', 'deezer_dl', False)]
+    assert soulseek.cancel_calls == []
+
+
+# ---------------------------------------------------------------------------
+# Cin bug 3: atomic update_record_unless_state
+# ---------------------------------------------------------------------------
+
+
+def test_update_record_unless_state_applies_when_state_not_blocked():
+    engine = DownloadEngine()
+    engine.add_record('youtube', 'dl-1', {'state': 'InProgress, Downloading'})
+
+    applied = engine.update_record_unless_state(
+        'youtube', 'dl-1',
+        {'state': 'Completed, Succeeded', 'progress': 100.0},
+        skip_if_state_in=('Cancelled',),
+    )
+    assert applied is True
+    assert engine.get_record('youtube', 'dl-1')['state'] == 'Completed, Succeeded'
+    assert engine.get_record('youtube', 'dl-1')['progress'] == 100.0
+
+
+def test_update_record_unless_state_skips_when_state_blocked():
+    """A worker-thread terminal write must NOT clobber a Cancelled
+    state set by the user. Returns False so caller knows the patch
+    was skipped."""
+    engine = DownloadEngine()
+    engine.add_record('youtube', 'dl-1', {'state': 'Cancelled'})
+
+    applied = engine.update_record_unless_state(
+        'youtube', 'dl-1',
+        {'state': 'Completed, Succeeded'},
+        skip_if_state_in=('Cancelled',),
+    )
+    assert applied is False
+    assert engine.get_record('youtube', 'dl-1')['state'] == 'Cancelled'
+
+
+def test_update_record_unless_state_returns_false_for_missing_record():
+    engine = DownloadEngine()
+    applied = engine.update_record_unless_state(
+        'youtube', 'never-existed',
+        {'state': 'Completed, Succeeded'},
+        skip_if_state_in=('Cancelled',),
+    )
+    assert applied is False

tests/downloads/test_download_orchestrator.py

@@ -190,6 +190,53 @@ def test_reload_instances_with_no_args_reloads_every_source():
 # ---------------------------------------------------------------------------
 
 
+# ---------------------------------------------------------------------------
+# Cin bug 2: hybrid_order alias normalization
+# ---------------------------------------------------------------------------
+
+
+def test_resolve_source_chain_normalizes_legacy_aliases():
+    """Cin's bug 2: hybrid_order config containing the legacy alias
+    'deezer_dl' was silently dropped because the canonical-name
+    membership check rejected it. Orchestrator must normalize via
+    the registry alias map first."""
+    orch = _build_orchestrator(
+        soulseek=_FakeClient(),
+        deezer_dl=_FakeClient(),
+        youtube=_FakeClient(),
+    )
+    orch.hybrid_order = ['deezer_dl', 'soulseek', 'youtube']
+    orch.hybrid_primary = None
+    orch.hybrid_secondary = None
+
+    chain = orch._resolve_source_chain()
+    assert chain == ['deezer', 'soulseek', 'youtube']
+
+
+def test_resolve_source_chain_dedupes_alias_and_canonical():
+    """If both 'deezer' and 'deezer_dl' appear, dedupe to single entry."""
+    orch = _build_orchestrator(
+        soulseek=_FakeClient(),
+        deezer_dl=_FakeClient(),
+    )
+    orch.hybrid_order = ['deezer_dl', 'deezer', 'soulseek']
+    orch.hybrid_primary = None
+    orch.hybrid_secondary = None
+
+    chain = orch._resolve_source_chain()
+    assert chain == ['deezer', 'soulseek']
+
+
+def test_resolve_source_chain_drops_unknown_names():
+    orch = _build_orchestrator(soulseek=_FakeClient(), youtube=_FakeClient())
+    orch.hybrid_order = ['nonsense', 'soulseek', 'also_fake', 'youtube']
+    orch.hybrid_primary = None
+    orch.hybrid_secondary = None
+
+    chain = orch._resolve_source_chain()
+    assert chain == ['soulseek', 'youtube']
+
+
 def test_get_download_orchestrator_returns_set_singleton():
     """When set_download_orchestrator has been called (web_server.py
     does this at boot), get_download_orchestrator returns the

web_server.py

@@ -627,23 +627,17 @@ try:
 except Exception as e:
     logger.error(f"  Playlist sync service failed to initialize: {e}")
 
-# Inject shutdown check callback into YouTube and Tidal clients (avoids circular imports)
-if soulseek_client:
-    if hasattr(soulseek_client, 'youtube'):
-         soulseek_client.youtube.set_shutdown_check(lambda: IS_SHUTTING_DOWN)
-         logger.info("  Configured YouTube client shutdown callback")
-    if hasattr(soulseek_client, 'tidal'):
-         soulseek_client.tidal.set_shutdown_check(lambda: IS_SHUTTING_DOWN)
-         logger.info("  Configured Tidal download client shutdown callback")
-    if hasattr(soulseek_client, 'qobuz'):
-         soulseek_client.qobuz.set_shutdown_check(lambda: IS_SHUTTING_DOWN)
-         logger.info("  Configured Qobuz client shutdown callback")
-    if hasattr(soulseek_client, 'hifi'):
-         soulseek_client.hifi.set_shutdown_check(lambda: IS_SHUTTING_DOWN)
-         logger.info("  Configured HiFi client shutdown callback")
-    if hasattr(soulseek_client, 'soundcloud') and soulseek_client.soundcloud:
-         soulseek_client.soundcloud.set_shutdown_check(lambda: IS_SHUTTING_DOWN)
-         logger.info("  Configured SoundCloud client shutdown callback")
+# Inject shutdown check callback into every download source that
+# accepts one. Generic dispatch via the registry — no per-source
+# attribute reaches needed.
+if soulseek_client and hasattr(soulseek_client, 'registry'):
+    for _src_name, _src_client in soulseek_client.registry.all_plugins():
+        if _src_client is not None and hasattr(_src_client, 'set_shutdown_check'):
+            try:
+                _src_client.set_shutdown_check(lambda: IS_SHUTTING_DOWN)
+                logger.info("  Configured %s client shutdown callback", _src_name)
+            except Exception as _exc:
+                logger.warning("  %s set_shutdown_check failed: %s", _src_name, _exc)
 
 # Initialize web scan manager for automatic post-download scanning
 try:
@@ -2459,7 +2453,7 @@ def get_cached_transfer_data():
 
             # First, get Soulseek downloads from API
             transfers_data = None
-            if not soulseek_known_down and soulseek_client and getattr(soulseek_client, 'soulseek', None) and soulseek_client.soulseek.base_url:
+            if not soulseek_known_down and soulseek_client and getattr(soulseek_client, 'soulseek', None) and soulseek_client.client("soulseek").base_url:
                 transfers_data = run_async(soulseek_client._make_request('GET', 'transfers/downloads'))
             if transfers_data:
                 all_transfers = []
@@ -2481,14 +2475,13 @@ def get_cached_transfer_data():
             # stays at 0 even when the underlying client knows the real percent.
             try:
                 all_downloads = []
-                for _dl_client in [soulseek_client.youtube, soulseek_client.tidal, soulseek_client.qobuz,
-                                   soulseek_client.hifi, soulseek_client.deezer_dl, soulseek_client.lidarr,
-                                   getattr(soulseek_client, 'soundcloud', None)]:
-                    if _dl_client:
-                        try:
-                            all_downloads.extend(run_async(_dl_client.get_all_downloads()))
-                        except Exception:
-                            pass
+                # Generic dispatch — engine returns active downloads
+                # across every plugin in one call, no per-source iteration.
+                if soulseek_client and hasattr(soulseek_client, 'engine'):
+                    try:
+                        all_downloads = run_async(soulseek_client.engine.get_all_downloads())
+                    except Exception:
+                        pass
                 for download in all_downloads:
                         key = _make_context_key(download.username, download.filename)
                         # Convert DownloadStatus to transfer dict format
@@ -4152,7 +4145,7 @@ def handle_settings():
                 soulseek_client.reload_settings()
                 # Reload YouTube client settings (rate limiting, cookies)
                 if hasattr(soulseek_client, 'youtube'):
-                    soulseek_client.youtube.reload_settings()
+                    soulseek_client.client("youtube").reload_settings()
             # FIX: Re-instantiate the global tidal_client to pick up new settings
             try:
                 tidal_client = TidalClient()
@@ -6816,7 +6809,7 @@ def enhanced_search_source(source_name):
 
     When the requested source's client isn't available (Spotify unauthed,
     Discogs missing token, Hydrabase disconnected, MusicBrainz import
-    failure, soulseek_client.youtube missing), returns plain JSON
+    failure, soulseek_client.client("youtube") missing), returns plain JSON
     `{"artists":[],"albums":[],"tracks":[],"available":false}` to match
     the original endpoint contract.
     """
@@ -7040,7 +7033,7 @@ def download_music_video():
             def _progress(pct):
                 _music_video_downloads[video_id]['progress'] = round(pct, 1)
 
-            final_path = soulseek_client.youtube.download_music_video(video_url, output_path, progress_callback=_progress)
+            final_path = soulseek_client.client("youtube").download_music_video(video_url, output_path, progress_callback=_progress)
 
             if final_path and os.path.exists(final_path):
                 _music_video_downloads[video_id]['status'] = 'completed'
@@ -14155,7 +14148,7 @@ def _enhance_file_metadata(file_path: str, context: dict, artist: dict, album_in
             genius_worker=genius_worker,
             spotify_enrichment_worker=spotify_enrichment_worker,
             itunes_enrichment_worker=itunes_enrichment_worker,
-            hifi_client=soulseek_client.hifi if soulseek_client else None,
+            hifi_client=soulseek_client.client("hifi") if soulseek_client else None,
         ),
     )
 
@@ -14260,7 +14253,7 @@ def _post_process_matched_download_with_verification(context_key, context, file_
             genius_worker=genius_worker,
             spotify_enrichment_worker=spotify_enrichment_worker,
             itunes_enrichment_worker=itunes_enrichment_worker,
-            hifi_client=soulseek_client.hifi if soulseek_client else None,
+            hifi_client=soulseek_client.client("hifi") if soulseek_client else None,
         ),
     )
 
@@ -14384,7 +14377,7 @@ def _post_process_matched_download(context_key, context, file_path):
             genius_worker=genius_worker,
             spotify_enrichment_worker=spotify_enrichment_worker,
             itunes_enrichment_worker=itunes_enrichment_worker,
-            hifi_client=soulseek_client.hifi if soulseek_client else None,
+            hifi_client=soulseek_client.client("hifi") if soulseek_client else None,
         ),
     )
 
@@ -17084,7 +17077,7 @@ def _try_source_reuse(task_id, batch_id, track):
     # Sort by confidence, filter by quality preference
     candidates.sort(key=lambda c: c.confidence, reverse=True)
     _sr.info(f"Found {len(candidates)} candidates above 0.70, best={candidates[0].confidence:.3f} ({candidates[0].filename})")
-    slsk = soulseek_client.soulseek if hasattr(soulseek_client, 'soulseek') else soulseek_client
+    slsk = soulseek_client.client("soulseek") if hasattr(soulseek_client, 'soulseek') else soulseek_client
     filtered = slsk.filter_results_by_quality_preference(candidates)
     if not filtered:
         _sr.info(f"Quality filter rejected all candidates for task {task_id}")
@@ -17164,7 +17157,7 @@ def _store_batch_source(batch_id, username, filename):
 
     try:
         # Access SoulseekClient directly (soulseek_client is DownloadOrchestrator)
-        slsk = soulseek_client.soulseek if hasattr(soulseek_client, 'soulseek') else soulseek_client
+        slsk = soulseek_client.client("soulseek") if hasattr(soulseek_client, 'soulseek') else soulseek_client
         _sr.info(f"Browsing {username}:{folder_path}...")
         files = run_async(slsk.browse_user_directory(username, folder_path))
         if not files:
@@ -19732,7 +19725,7 @@ def get_discover_album(source, album_id):
 def hifi_status():
     """Check if HiFi API instances are reachable."""
     try:
-        hifi = soulseek_client.hifi
+        hifi = soulseek_client.client("hifi")
         available = hifi.is_available()
         version = hifi.get_version() if available else None
         return jsonify({
@@ -19757,7 +19750,7 @@ def soundcloud_status():
     try:
         sc = None
         if soulseek_client and hasattr(soulseek_client, 'soundcloud'):
-            sc = soulseek_client.soundcloud
+            sc = soulseek_client.client("soundcloud")
         if not sc:
             return jsonify({
                 "available": False,
@@ -19785,7 +19778,7 @@ def hifi_instances():
     """Check availability of all HiFi API instances."""
     import requests as req
     try:
-        hifi = soulseek_client.hifi
+        hifi = soulseek_client.client("hifi")
         instances = list(hifi._instances)
         results = []
         for url in instances:
@@ -19840,9 +19833,9 @@ def hifi_add_instance():
         added = db.add_hifi_instance(url, priority)
         if not added:
             return jsonify({'success': False, 'error': 'Instance already exists'}), 400
-        # Reload the client
-        if soulseek_client and hasattr(soulseek_client, 'hifi') and soulseek_client.hifi:
-            soulseek_client.hifi.reload_instances()
+        # Reload the HiFi client
+        if soulseek_client:
+            soulseek_client.reload_instances('hifi')
         return jsonify({'success': True, 'url': url})
     except Exception as e:
         logger.error(f"Error adding HiFi instance: {e}")
@@ -19862,9 +19855,9 @@ def hifi_remove_instance():
         removed = db.remove_hifi_instance(url)
         if not removed:
             return jsonify({'success': False, 'error': 'Instance not found'}), 404
-        # Reload the client
-        if soulseek_client and hasattr(soulseek_client, 'hifi') and soulseek_client.hifi:
-            soulseek_client.hifi.reload_instances()
+        # Reload the HiFi client
+        if soulseek_client:
+            soulseek_client.reload_instances('hifi')
         return jsonify({'success': True, 'url': url})
     except Exception as e:
         logger.error(f"Error removing HiFi instance: {e}")
@@ -19884,8 +19877,8 @@ def hifi_toggle_instance():
         from database.music_database import get_database
         db = get_database()
         db.toggle_hifi_instance(url, enabled)
-        if soulseek_client and hasattr(soulseek_client, 'hifi') and soulseek_client.hifi:
-            soulseek_client.hifi.reload_instances()
+        if soulseek_client:
+            soulseek_client.reload_instances('hifi')
         return jsonify({'success': True})
     except Exception as e:
         logger.error(f"Error toggling HiFi instance: {e}")
@@ -19905,9 +19898,9 @@ def hifi_reorder_instances():
         db = get_database()
         if not db.reorder_hifi_instances(urls):
             return jsonify({'success': False, 'error': 'One or more URLs not found'}), 400
-        # Reload the client
-        if soulseek_client and hasattr(soulseek_client, 'hifi') and soulseek_client.hifi:
-            soulseek_client.hifi.reload_instances()
+        # Reload the HiFi client
+        if soulseek_client:
+            soulseek_client.reload_instances('hifi')
         return jsonify({'success': True})
     except Exception as e:
         logger.error(f"Error reordering HiFi instances: {e}")
@@ -20063,7 +20056,7 @@ def _get_tidal_download_client():
         raise RuntimeError("Download orchestrator not initialized — check startup logs for errors")
     if not hasattr(soulseek_client, 'tidal') or not soulseek_client.tidal:
         raise RuntimeError("Tidal download client not available — ensure tidalapi is installed")
-    return soulseek_client.tidal
+    return soulseek_client.client("tidal")
 
 @app.route('/api/tidal/download/auth/start', methods=['POST'])
 def tidal_download_auth_start():
@@ -20132,7 +20125,7 @@ def qobuz_auth_login():
         if not email or not password:
             return jsonify({"success": False, "error": "Email and password required"}), 400
 
-        qobuz = soulseek_client.qobuz
+        qobuz = soulseek_client.client("qobuz")
         result = qobuz.login(email, password)
 
         if result['status'] == 'success':
@@ -20155,7 +20148,7 @@ def qobuz_auth_token():
         if not token:
             return jsonify({"success": False, "error": "Auth token required"}), 400
 
-        qobuz = soulseek_client.qobuz
+        qobuz = soulseek_client.client("qobuz")
         result = qobuz.login_with_token(token)
 
         if result['status'] == 'success':
@@ -20172,7 +20165,7 @@ def qobuz_auth_token():
 def qobuz_auth_status():
     """Check if Qobuz client is authenticated."""
     try:
-        qobuz = soulseek_client.qobuz
+        qobuz = soulseek_client.client("qobuz")
         authenticated = qobuz.is_authenticated()
         user_info = {}
         if authenticated and qobuz.user_info:
@@ -20189,7 +20182,7 @@ def qobuz_auth_status():
 def qobuz_auth_logout():
     """Logout from Qobuz."""
     try:
-        soulseek_client.qobuz.logout()
+        soulseek_client.client("qobuz").logout()
         _sync_qobuz_credentials_to_worker()
         return jsonify({"success": True})
     except Exception as e:
@@ -21133,7 +21126,7 @@ def get_deezer_arl_status():
     try:
         deezer_dl = None
         if soulseek_client and hasattr(soulseek_client, 'deezer_dl') and soulseek_client.deezer_dl:
-            deezer_dl = soulseek_client.deezer_dl
+            deezer_dl = soulseek_client.client("deezer_dl")
         if deezer_dl and deezer_dl.is_authenticated():
             user_data = deezer_dl._user_data or {}
             return jsonify({
@@ -21152,7 +21145,7 @@ def get_deezer_arl_playlists():
     try:
         deezer_dl = None
         if soulseek_client and hasattr(soulseek_client, 'deezer_dl') and soulseek_client.deezer_dl:
-            deezer_dl = soulseek_client.deezer_dl
+            deezer_dl = soulseek_client.client("deezer_dl")
         if not deezer_dl or not deezer_dl.is_authenticated():
             return jsonify({'error': 'Deezer ARL not authenticated. Configure your ARL token in Settings > Downloads.'}), 401
 
@@ -21182,7 +21175,7 @@ def get_deezer_arl_playlist_tracks(playlist_id):
     try:
         deezer_dl = None
         if soulseek_client and hasattr(soulseek_client, 'deezer_dl') and soulseek_client.deezer_dl:
-            deezer_dl = soulseek_client.deezer_dl
+            deezer_dl = soulseek_client.client("deezer_dl")
         if not deezer_dl or not deezer_dl.is_authenticated():
             return jsonify({'error': 'Deezer ARL not authenticated.'}), 401
 
@@ -27374,8 +27367,8 @@ def get_your_artists_sources():
         try:
             deezer_cl = _get_deezer_client()
             deezer_oauth = deezer_cl and hasattr(deezer_cl, 'is_user_authenticated') and deezer_cl.is_user_authenticated()
-            deezer_arl = (hasattr(soulseek_client, 'deezer_dl') and soulseek_client.deezer_dl
-                          and soulseek_client.deezer_dl.is_authenticated())
+            deezer_arl = (hasattr(soulseek_client, 'deezer_dl') and soulseek_client.client("deezer_dl")
+                          and soulseek_client.client("deezer_dl").is_authenticated())
             if deezer_oauth or deezer_arl:
                 connected.append('deezer')
         except Exception:
@@ -27496,10 +27489,10 @@ def _fetch_and_match_liked_artists(profile_id: int):
             if deezer_cl and hasattr(deezer_cl, 'is_user_authenticated') and deezer_cl.is_user_authenticated():
                 logger.info("[Your Artists] Fetching favorite artists from Deezer (OAuth)...")
                 artists = deezer_cl.get_user_favorite_artists(limit=200)
-            elif (hasattr(soulseek_client, 'deezer_dl') and soulseek_client.deezer_dl
-                  and soulseek_client.deezer_dl.is_authenticated()):
+            elif (hasattr(soulseek_client, 'deezer_dl') and soulseek_client.client("deezer_dl")
+                  and soulseek_client.client("deezer_dl").is_authenticated()):
                 logger.info("[Your Artists] Fetching favorite artists from Deezer (ARL)...")
-                artists = soulseek_client.deezer_dl.get_user_favorite_artists(limit=200)
+                artists = soulseek_client.client("deezer_dl").get_user_favorite_artists(limit=200)
             for a in artists:
                 database.upsert_liked_artist(
                     artist_name=a['name'], source_service='deezer',
@@ -27646,8 +27639,8 @@ def get_your_albums_sources():
         try:
             deezer_cl = _get_deezer_client()
             deezer_oauth = deezer_cl and hasattr(deezer_cl, 'is_user_authenticated') and deezer_cl.is_user_authenticated()
-            deezer_arl = (hasattr(soulseek_client, 'deezer_dl') and soulseek_client.deezer_dl
-                          and soulseek_client.deezer_dl.is_authenticated())
+            deezer_arl = (hasattr(soulseek_client, 'deezer_dl') and soulseek_client.client("deezer_dl")
+                          and soulseek_client.client("deezer_dl").is_authenticated())
             if deezer_oauth or deezer_arl:
                 connected.append('deezer')
         except Exception:
@@ -27754,10 +27747,10 @@ def _fetch_liked_albums(profile_id: int):
             if deezer_cl and hasattr(deezer_cl, 'is_user_authenticated') and deezer_cl.is_user_authenticated():
                 logger.info("[Your Albums] Fetching favorite albums from Deezer (OAuth)...")
                 albums = deezer_cl.get_user_favorite_albums(limit=500)
-            elif (hasattr(soulseek_client, 'deezer_dl') and soulseek_client.deezer_dl
-                  and soulseek_client.deezer_dl.is_authenticated()):
+            elif (hasattr(soulseek_client, 'deezer_dl') and soulseek_client.client("deezer_dl")
+                  and soulseek_client.client("deezer_dl").is_authenticated()):
                 logger.info("[Your Albums] Fetching favorite albums from Deezer (ARL)...")
-                albums = soulseek_client.deezer_dl.get_user_favorite_albums(limit=500)
+                albums = soulseek_client.client("deezer_dl").get_user_favorite_albums(limit=500)
             for a in albums:
                 database.upsert_liked_album(
                     album_name=a['album_name'], artist_name=a['artist_name'],

webui/static/helper.js

@@ -3432,6 +3432,7 @@ const WHATS_NEW = {
     '2.4.2': [
         // --- post-2.4.1 dev work — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
         { date: 'Unreleased — 2.4.2 dev cycle' },
+        { title: 'Internal: Download Engine Review Followup', desc: 'internal — three correctness fixes on top of the download engine refactor, all flagged in cin\'s pr review. (1) `engine.cancel_download(source_hint=\'deezer_dl\')` was silently routing deezer cancels to soulseek because the legacy alias never made it to the engine\'s plugin map — only the registry knew about it. fix: aliases now flow through `register_plugin` and `get_plugin` / `cancel_download` resolve them to the canonical name. (2) `_resolve_source_chain` filtered hybrid_order against canonical registry names only, so any user with `deezer_dl` in their config quietly dropped deezer from hybrid mode. fix: orchestrator normalizes through `registry.get_spec()` first. (3) the worker\'s terminal write was a read-then-write split — a cancel landing between the snapshot and the update could be overwritten back to errored / completed. fix: new atomic `update_record_unless_state` on the engine holds `state_lock` across the check + write; both `_mark_terminal` AND the success path use it now. also added generic `client(name)` / `configured_clients()` / `reload_instances(name?)` accessors on the orchestrator + a `get/set_download_orchestrator()` singleton matching cin\'s `get_metadata_engine()` shape, and migrated 30 external `soulseek_client.<source>` reaches in web_server.py to `client("<source>")`. 18 new tests pin every fix.' },
         { title: 'Internal: Typed Metadata Foundation', desc: 'internal — first step of a multi-pr migration to give the metadata pipeline a real contract. the codebase historically grew duck-typed extractors (`_extract_lookup_value(album_data, "id", "album_id", "collectionId", "release_id", default=...)`) at every consumer site because each provider returns its own response shape. ~150 of those across the codebase. new `core/metadata/types.py` defines canonical typed `Album` / `Track` / `Artist` dataclasses with strict required fields. per-source classmethod converters (from_spotify_dict, from_itunes_dict, from_deezer_dict, from_discogs_dict, from_musicbrainz_dict, from_hydrabase_dict) are the SINGLE place that knows each provider\'s wire shape. zero behavior changes in this pr — pure additive foundation. follow-up prs migrate consumers one at a time. full migration plan documented at docs/metadata-types-migration.md.', page: 'library' },
         { title: 'Internal: Migrate Album-Info Builders to Typed Path', desc: 'internal — steps 2+3 of the typed metadata migration in one pr. two album-info builders now route through `Album.from_<source>_dict()` when the caller passes a known source: `_build_album_info` (used by every album-tracks lookup) and the embedded album section of `_build_single_import_context_payload` (used by single-track import context resolution). legacy duck-typed extraction stays as the fallback when source is empty/unknown, raw input isn\'t a dict, or the typed converter raises — so a converter bug can\'t break album resolution or import context. caller-provided album_id / album_name / artist_name fallbacks apply on the typed path the same way they did on legacy. zero behavior change for existing callers since they don\'t pass a source yet — opt-in only. 22 new tests pin the typed path, the legacy fallback, and parametrized coverage across registered providers.' },
         { title: 'Internal: Migrate Discography + Quality Scanner to Typed Path', desc: 'internal — next round of the typed metadata migration. three more album-shape consumers now route through `Album.from_<source>_dict()` when the caller passes a known source: `_build_discography_release_dict` (artist discography release cards), `_build_artist_detail_release_card` (artist detail page release cards), and `_normalize_track_album` (quality scanner result normalization). legacy duck-typed extraction stays as the fallback when source is empty/unknown, raw input isn\'t a dict, or the typed converter raises — same safety contract as the prior migration steps. 20 new tests pin the typed path + legacy fallback + parametrized coverage across registered providers.' },