Merge salvage/5461-plugin-catalog

api/config.py

@@ -35,6 +35,7 @@ import api.paths as _paths
 from api.plugin_providers import (
     effective_provider_display_name as _effective_provider_display_name,
     is_plugin_model_provider as _is_plugin_model_provider,
+    plugin_model_provider_profiles as _plugin_model_provider_profiles,
 )
 
 HOME = _paths.HOME
@@ -2959,6 +2960,16 @@ def model_with_provider_context(model_id: str, model_provider: str | None = None
     if provider in _ACP_SUBPROCESS_PROVIDERS:
         return f"@{provider}:{model}"
 
+    # Plugin-only model providers (e.g. 9router, and other model plugins whose
+    # slugs are not in the static provider tables) route through the plugin, not
+    # the default provider. This MUST come before the `provider == config_provider`
+    # bare-passthrough below: when a plugin provider is ALSO the configured
+    # provider, returning a bare model would drop the '@plugin:' hint and the model
+    # would be sent to the wrong backend. Emit the explicit hint so it stays
+    # routable to the plugin that surfaced it. (#5909 gate finding)
+    if _is_plugin_model_provider(provider):
+        return f"@{provider}:{model}"
+
     # If the selected provider is already the configured provider, leaving the
     # model bare preserves provider-specific base_url/proxy settings.
     if provider == config_provider:
@@ -2978,6 +2989,9 @@ def model_with_provider_context(model_id: str, model_provider: str | None = None
     if isinstance(providers_cfg, dict) and provider in providers_cfg:
         return f"@{provider}:{model}"
 
+    # (Plugin-only provider routing handled above, before the config_provider
+    # bare-passthrough.)
+
     # For non-OpenRouter slash IDs without an explicit configured provider,
     # keep the ID intact so existing custom/proxy base_url routing and
     # portal-provider handling remain in charge.
@@ -4863,6 +4877,23 @@ def _static_models_catalog_without_live_probes() -> dict:
             if canonical and _provider_has_key(canonical):
                 detected_providers.add(canonical)
 
+        # Plugin-only providers (e.g. 9router) are not in the static
+        # _PROVIDER_MODELS / _PROVIDER_DISPLAY tables and are detected above
+        # only when the user puts them in `providers.<slug>`.  Plugins ship
+        # with their own env-var wiring, so an installed-and-keyed plugin
+        # provider should also enter the static catalog even without a
+        # `providers:` block — otherwise the picker silently drops the
+        # group when the live-rebuild cache is cold.
+        try:
+            for _plugin_pid in list(_plugin_model_provider_profiles().keys()):
+                if not _plugin_pid or not _provider_has_key(_plugin_pid):
+                    continue
+                _canonical = _canonicalise_provider_id(_plugin_pid) or _plugin_pid
+                if _canonical:
+                    detected_providers.add(_canonical)
+        except Exception:
+            logger.debug("Plugin provider detection failed in static catalog", exc_info=True)
+
         fallback_cfg = cfg.get("fallback_providers", []) if isinstance(cfg, dict) else []
         if isinstance(fallback_cfg, list):
             for entry in fallback_cfg:
@@ -4996,12 +5027,32 @@ def _static_models_catalog_without_live_probes() -> dict:
                             raw_models.append({"id": item, "label": item})
             if not raw_models:
                 raw_models = copy.deepcopy(_PROVIDER_MODELS.get(pid, []))
+            # Plugin-only providers (e.g. 9router) are not in _PROVIDER_MODELS
+            # and rarely ship a `models:` allowlist in providers.<slug>, so
+            # the static catalog above would render them as empty groups that
+            # the picker filters out. Fall back to the plugin's own
+            # ProviderProfile.fallback_models so the provider surfaces a
+            # curated, network-free subset on the cold path. The live
+            # rebuild (_build_available_models_uncached) does a full
+            # /v1/models fetch and supersedes this view on the next call.
+            if not raw_models and _is_plugin_model_provider(pid):
+                _plugin_profile = _plugin_model_provider_profiles().get(
+                    (pid or "").strip().lower()
+                )
+                if _plugin_profile is not None:
+                    _fallback = getattr(_plugin_profile, "fallback_models", ()) or ()
+                    raw_models = [{"id": str(mid), "label": str(mid)} for mid in _fallback]
             for model_id in configured_model_ids.get(pid, []):
                 if model_id and not any(m.get("id") == model_id for m in raw_models):
                     raw_models.append(
                         {"id": model_id, "label": _get_label_for_model(model_id, groups)}
                     )
-            if raw_models:
+            # Plugin-only providers (e.g. 9router) must enter `groups` even
+            # when `raw_models` is empty so the post-loop filter sees them.
+            # Without this, the earlier plugin-fallback pass only seeds
+            # `raw_models` when `fallback_models` is non-empty; the cold-cache
+            # picker still silently drops a keyed plugin with no models yet.
+            if raw_models or _is_plugin_model_provider(pid):
                 groups.append(
                     {
                         "provider": provider_name,
@@ -5037,7 +5088,14 @@ def _static_models_catalog_without_live_probes() -> dict:
         groups = [
             group
             for group in groups
-            if group.get("models") or str(group.get("provider_id") or "").startswith("custom:")
+            if group.get("models")
+            or str(group.get("provider_id") or "").startswith("custom:")
+            # Keep plugin-only providers visible even when no models surfaced
+            # yet (e.g. plugin's fallback_models is empty and live rebuild
+            # hasn't completed). Otherwise they silently drop from the
+            # picker and look "not installed" — the same 9router-empty-group
+            # regression this branch was added to fix.
+            or _is_plugin_model_provider(str(group.get("provider_id") or ""))
         ]
 
         providers_with_keys: set[str] = set()

tests/test_plugin_model_providers.py

@@ -223,3 +223,252 @@ class TestPluginModelProvidersPicker:
             config.cfg.update(old_cfg)
             config._cfg_mtime = old_mtime
             config.invalidate_models_cache()
+
+
+class TestPluginFallbackModelsInStaticCatalog:
+    """Regression tests for the network-free /api/models catalog path.
+
+    ``_static_models_catalog_without_live_probes()`` is used for ``prefer_cache``
+    lookups and as the warm-cache payload.  Plugin-only providers (e.g. 9router)
+    are not in ``_PROVIDER_MODELS`` and rarely ship a ``models:`` allowlist in
+    ``providers.<slug>``, so without a fallback the static catalog would render
+    them as empty groups that the picker filters out.  The fix is to surface
+    the ``ProviderProfile.fallback_models`` declared by the plugin itself on
+    this cold path.
+    """
+
+    def test_static_catalog_surfaces_plugin_fallback_models(
+        self, monkeypatch, tmp_path
+    ):
+        _install_fake_yandex_plugin(monkeypatch)
+        _install_fake_hermes_cli(monkeypatch, authenticated=True, model_ids=[])
+        monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
+        monkeypatch.setenv("HERMES_HOME", str(tmp_path))
+        # _provider_has_key reads the env var directly, not the .env file.
+        # Set it so the static catalog's plugin-detection branch fires.
+        monkeypatch.setenv("YANDEX_API_KEY", "test-y...n")
+
+        env_path = tmp_path / ".env"
+        env_path.write_text("YANDEX_API_KEY=test-y...n", encoding="utf-8")
+
+        old_cfg = dict(config.cfg)
+        old_mtime = config._cfg_mtime
+        config.cfg.clear()
+        # No `providers.yandex.models:` allowlist, no `models:` in cfg at all.
+        # Active provider set to something else so yandex only enters via the
+        # plugin discovery path.
+        config.cfg["model"] = {"provider": "gemini", "default": "gemini-2.5-flash"}
+        config.cfg["providers"] = {}
+        try:
+            config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime
+        except Exception:
+            config._cfg_mtime = 0.0
+
+        config.invalidate_models_cache()
+        try:
+            catalog = config._static_models_catalog_without_live_probes()
+            yandex_group = next(
+                (
+                    g
+                    for g in catalog.get("groups", [])
+                    if g.get("provider_id") == "yandex"
+                ),
+                None,
+            )
+            assert yandex_group is not None, (
+                "plugin provider must appear in network-free static catalog "
+                "even without providers.<slug>.models allowlist"
+            )
+            # yandex fake plugin declares no fallback_models in this test's
+            # profile, so the group is allowed to be empty — but the provider
+            # itself must be present (not filtered out as an empty group).
+            assert yandex_group.get("models") == [] or yandex_group.get("models")
+        finally:
+            config.cfg.clear()
+            config.cfg.update(old_cfg)
+            config._cfg_mtime = old_mtime
+            config.invalidate_models_cache()
+
+    def test_static_catalog_uses_provider_fallback_models_when_declared(
+        self, monkeypatch, tmp_path
+    ):
+        """When the plugin declares ``fallback_models``, the static catalog
+        surfaces them as the group's model list."""
+        fallback = (
+            "gh/claude-sonnet-4.5",
+            "ag/claude-sonnet-4-6",
+            "ds/deepseek-chat",
+        )
+        profile = SimpleNamespace(
+            name="myplugin",
+            display_name="My Plugin",
+            env_vars=("MYPLUGIN_API_KEY",),
+            auth_type="api_key",
+            aliases=(),
+            fallback_models=fallback,
+        )
+
+        def _fake_list_providers():
+            return [profile]
+
+        fake_providers = types.ModuleType("providers")
+        fake_providers.list_providers = _fake_list_providers
+        monkeypatch.setitem(sys.modules, "providers", fake_providers)
+
+        from api.plugin_providers import invalidate_plugin_model_provider_cache
+
+        invalidate_plugin_model_provider_cache()
+
+        monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
+        monkeypatch.setenv("HERMES_HOME", str(tmp_path))
+        # _provider_has_key reads the env var directly.
+        monkeypatch.setenv("MYPLUGIN_API_KEY", "test-m...n")
+        (tmp_path / ".env").write_text(
+            "MYPLUGIN_API_KEY=test-m...n", encoding="utf-8"
+        )
+
+        # Mock the plugin provider's API key as authenticated via hermes_cli.auth
+        fake_pkg = types.ModuleType("hermes_cli")
+        fake_pkg.__path__ = []
+        fake_models = types.ModuleType("hermes_cli.models")
+        fake_models.list_available_providers = lambda: []
+        fake_models.provider_model_ids = lambda pid: []
+        fake_auth = types.ModuleType("hermes_cli.auth")
+        fake_auth.get_auth_status = lambda pid: (
+            {"logged_in": True, "configured": True, "key_source": "env_file"}
+            if pid == "myplugin"
+            else {}
+        )
+        monkeypatch.setitem(sys.modules, "hermes_cli", fake_pkg)
+        monkeypatch.setitem(sys.modules, "hermes_cli.models", fake_models)
+        monkeypatch.setitem(sys.modules, "hermes_cli.auth", fake_auth)
+
+        old_cfg = dict(config.cfg)
+        old_mtime = config._cfg_mtime
+        config.cfg.clear()
+        config.cfg["model"] = {"provider": "gemini", "default": "gemini-2.5-flash"}
+        config.cfg["providers"] = {}
+        try:
+            config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime
+        except Exception:
+            config._cfg_mtime = 0.0
+
+        config.invalidate_models_cache()
+        try:
+            catalog = config._static_models_catalog_without_live_probes()
+            myplugin_group = next(
+                (
+                    g
+                    for g in catalog.get("groups", [])
+                    if g.get("provider_id") == "myplugin"
+                ),
+                None,
+            )
+            assert myplugin_group is not None, (
+                "plugin provider with fallback_models must appear in static catalog"
+            )
+            model_ids = [m.get("id") for m in myplugin_group.get("models", [])]
+            assert model_ids == list(fallback), (
+                f"static catalog should use plugin's fallback_models, got {model_ids}"
+            )
+        finally:
+            config.cfg.clear()
+            config.cfg.update(old_cfg)
+            config._cfg_mtime = old_mtime
+            config.invalidate_models_cache()
+
+
+class TestPluginModelProviderRouting:
+    """Regression test for routing surfaced plugin-only models.
+
+    Surfacing a plugin-only model in the picker is only half the fix: the
+    selected ``(model, model_provider)`` pair must also *route* to the plugin.
+    ``model_with_provider_context()`` historically let any slash-bearing model
+    ID fall through to a bare passthrough, which made a surfaced plugin model
+    such as ``gh/claude-sonnet-4.5`` inherit the default provider instead of
+    the plugin that surfaced it.  The fix emits an explicit ``@plugin:model``
+    hint for plugin-only providers before that bare passthrough.
+    """
+
+    def test_plugin_only_model_routes_to_plugin_provider(self, monkeypatch):
+        profile = SimpleNamespace(
+            name="myplugin",
+            display_name="My Plugin",
+            env_vars=("MYPLUGIN_API_KEY",),
+            auth_type="api_key",
+            aliases=(),
+            fallback_models=("gh/claude-sonnet-4.5",),
+        )
+
+        def _fake_list_providers():
+            return [profile]
+
+        fake_providers = types.ModuleType("providers")
+        fake_providers.list_providers = _fake_list_providers
+        monkeypatch.setitem(sys.modules, "providers", fake_providers)
+        invalidate_plugin_model_provider_cache()
+
+        old_cfg = dict(config.cfg)
+        config.cfg.clear()
+        # Default provider is something else; plugin has no `providers:` block.
+        # Without the plugin-routing branch the slash-bearing model ID would
+        # fall through to the bare passthrough and inherit the default.
+        config.cfg["model"] = {"provider": "gemini", "default": "gemini-2.5-flash"}
+        config.cfg["providers"] = {}
+        try:
+            assert config._is_plugin_model_provider("myplugin"), (
+                "test setup: fake plugin provider must be recognised"
+            )
+            routed = config.model_with_provider_context(
+                "gh/claude-sonnet-4.5", "myplugin"
+            )
+            assert routed == "@myplugin:gh/claude-sonnet-4.5", (
+                "surfaced plugin-only model must route to its plugin, "
+                f"got {routed!r}"
+            )
+        finally:
+            config.cfg.clear()
+            config.cfg.update(old_cfg)
+            invalidate_plugin_model_provider_cache()
+
+    def test_plugin_provider_routes_even_when_it_is_the_configured_provider(self, monkeypatch):
+        # #5909 gate finding: the plugin-routing branch must run BEFORE the
+        # `provider == config_provider` bare-passthrough. When the ACTIVE plugin
+        # provider is also the configured default, returning a bare model would
+        # drop the '@plugin:' hint and route to the wrong backend.
+        profile = SimpleNamespace(
+            name="gmi",
+            display_name="GMI",
+            env_vars=("GMI_API_KEY",),
+            auth_type="api_key",
+            aliases=(),
+            fallback_models=("anthropic/claude-sonnet-4.6",),
+        )
+
+        def _fake_list_providers():
+            return [profile]
+
+        fake_providers = types.ModuleType("providers")
+        fake_providers.list_providers = _fake_list_providers
+        monkeypatch.setitem(sys.modules, "providers", fake_providers)
+        invalidate_plugin_model_provider_cache()
+
+        old_cfg = dict(config.cfg)
+        config.cfg.clear()
+        # The plugin IS the configured provider now.
+        config.cfg["model"] = {"provider": "gmi", "default": "anthropic/claude-sonnet-4.6"}
+        config.cfg["providers"] = {}
+        try:
+            assert config._is_plugin_model_provider("gmi")
+            routed = config.model_with_provider_context(
+                "anthropic/claude-sonnet-4.6", "gmi"
+            )
+            assert routed == "@gmi:anthropic/claude-sonnet-4.6", (
+                "an active plugin provider must keep its '@plugin:' routing hint "
+                "even when it equals the configured provider, got "
+                f"{routed!r}"
+            )
+        finally:
+            config.cfg.clear()
+            config.cfg.update(old_cfg)
+            invalidate_plugin_model_provider_cache()