From eeb220d40c2fb6cb33d61a9b792ca68811408b3a Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 21 Sep 2026 15:22:33 -0700 Subject: [PATCH] fix: model-provider plugins resolve per bound profile home (#88143) Symptom: a model-provider plugin installed with `hermes plugins install` (e.g. claude-subscription-directsdk) worked from a terminal but the Desktop app failed every session build with "Unknown provider ''". Cause: `providers/__init__.py` scanned `$HERMES_HOME/plugins` exactly once per process, under whichever profile home happened to be bound at the first lookup, and the registry was global. The Desktop backend and the multiplex gateway serve several profiles from one process, so any profile other than the first-discovered one never saw its own plugins, and a plugin installed while the process ran was invisible until a restart. Change: bundled, pip and legacy providers stay process-wide; `$HERMES_HOME` plugins load into a per-home layer keyed by `hermes_home_key()` at lookup time (`get_provider_profile`, `list_providers`, `provider_source`). The layer rescans when the plugin directories' mtimes change, so a fresh install is found on the next lookup. The registration target is a ContextVar so two turn threads scanning two homes cannot cross-register, and no lock is held across plugin imports (a lock there could deadlock against a thread mid-`import hermes_cli.auth`). Per-home module names let two profiles carry the same plugin. `plugin_dev` reuses the module-name helper. Live repro (tui_gateway `session.create` on a secondary profile whose config selects a plugin installed only there): base -> agent_error "Unknown provider 'fakeprov-b'"; fixed -> provider resolves and the build proceeds to the plugin's own runtime check. --- hermes_cli/plugin_dev.py | 2 +- providers/__init__.py | 218 ++++++++++++------ tests/providers/test_profile_home_layers.py | 92 ++++++++ .../developer-guide/model-provider-plugin.md | 4 +- 4 files changed, 246 insertions(+), 70 deletions(-) create mode 100644 tests/providers/test_profile_home_layers.py diff --git a/hermes_cli/plugin_dev.py b/hermes_cli/plugin_dev.py index ed03cea55d..ec19804610 100644 --- a/hermes_cli/plugin_dev.py +++ b/hermes_cli/plugin_dev.py @@ -126,7 +126,7 @@ def _load_model_provider(copied: Path, manifest): # The live install may already have imported this very plugin (same directory name) during # startup discovery; import the copy fresh and put the live module/profiles back afterwards. - module_name = f"_hermes_user_provider_{copied.name.replace('-', '_')}" + module_name = providers._user_module_name(copied, "") prior_module = sys.modules.pop(module_name, None) before = dict(providers._REGISTRY) before_aliases = dict(providers._ALIASES) diff --git a/providers/__init__.py b/providers/__init__.py index c2b38b8167..66b276770b 100644 --- a/providers/__init__.py +++ b/providers/__init__.py @@ -12,10 +12,12 @@ Each plugin directory contains: - ``plugin.yaml`` — manifest (name, kind: model-provider, version, description) Discovery is lazy: the first call to ``get_provider_profile()`` or -``list_providers()`` scans both locations and imports every plugin. User -plugins override bundled plugins on name collision (last-writer-wins), so -third parties can monkey-patch or replace any built-in profile without -editing the repo. +``list_providers()`` imports the bundled and pip-installed plugins once per +process; the ``$HERMES_HOME`` plugins of the profile home bound at lookup time +load into that home's own layer, so one process serving several profiles +(multiplex gateway, Desktop ``serve``) resolves each profile's installs. User +plugins override bundled plugins on name collision, so third parties can +monkey-patch or replace any built-in profile without editing the repo. For backward compatibility, ``providers/*.py`` files (other than ``base.py`` and ``__init__.py``) are still discovered via ``pkgutil.iter_modules``. @@ -32,16 +34,22 @@ Usage:: from __future__ import annotations +import hashlib import importlib import importlib.util import logging +import os import sys +import threading +from contextvars import ContextVar +from dataclasses import dataclass, field from pathlib import Path from providers.base import ProviderProfile logger = logging.getLogger(__name__) +# Process-wide layer: bundled plugins, pip entry points, legacy ``providers/.py``. _REGISTRY: dict[str, ProviderProfile] = {} _ALIASES: dict[str, str] = {} # Where the CURRENT registration of each name came from: "bundled" / "user" (a @@ -52,6 +60,31 @@ _PROVIDER_LIST_CACHE: list[ProviderProfile] | None = None _discovered = False _discovering = False + +@dataclass +class _HomeLayer: + """The ``$HERMES_HOME/plugins`` providers of ONE profile home. + + One process serves many profiles (multiplex gateway, Desktop ``serve``) and each profile installs + its own plugins, so user plugins are keyed by the home bound at lookup time instead of the home + that happened to be bound at first discovery — that made a plugin installed in a secondary + profile ``Unknown provider`` in Desktop while the same profile worked in a terminal (#88143). + ``stamps`` are the plugin dirs' mtimes: a directory added by ``hermes plugins install`` while the + process runs changes them, and the next lookup imports it without a restart. + """ + registry: dict[str, ProviderProfile] = field(default_factory=dict) + aliases: dict[str, str] = field(default_factory=dict) + stamps: tuple = () + + +_HOME_LAYERS: dict[str, _HomeLayer] = {} +_HOME_LAYERS_LOCK = threading.Lock() +# The layer a ``$HERMES_HOME`` plugin import registers into. A ContextVar, not a module global: +# two turn threads scanning two profile homes at once must not cross-register. Never a lock held +# across the import itself — a thread mid-``import hermes_cli.auth`` (whose import calls +# ``list_providers()``) would block on it while the scanning thread waits on that module's import lock. +_REGISTRATION_TARGET: ContextVar[_HomeLayer | None] = ContextVar("_provider_registration_target", default=None) + # Repo-root ``plugins/model-providers/`` — populated at discovery time. _BUNDLED_PLUGINS_DIR = ( Path(__file__).resolve().parent.parent / "plugins" / "model-providers" @@ -83,14 +116,21 @@ def register_provider(profile: ProviderProfile) -> None: Later registrations with the same name replace earlier ones — so user plugins under ``$HERMES_HOME/plugins/model-providers/`` can override - bundled profiles without editing repo code. + bundled profiles without editing repo code. A registration made while a + ``$HERMES_HOME`` plugin is being imported lands in that home's layer. """ global _PROVIDER_LIST_CACHE - _REGISTRY[profile.name] = profile - _SOURCES[profile.name] = _current_source or "runtime" - for alias in profile.aliases: - _ALIASES[alias] = profile.name - _PROVIDER_LIST_CACHE = None + layer = _REGISTRATION_TARGET.get() + if layer is not None: + layer.registry[profile.name] = profile + for alias in profile.aliases: + layer.aliases[alias] = profile.name + else: + _REGISTRY[profile.name] = profile + _SOURCES[profile.name] = _current_source or "runtime" + for alias in profile.aliases: + _ALIASES[alias] = profile.name + _PROVIDER_LIST_CACHE = None if _discovered and not _discovering: # post-discovery registration: mirror it immediately _sync_auth_registry() @@ -101,7 +141,11 @@ def provider_source(name: str) -> str | None: ``"user"`` is what lets a ``$HERMES_HOME`` plugin re-registering a bundled name win in ``hermes_cli.auth.PROVIDER_REGISTRY`` too — a bundled profile never rewrites a built-in row. """ - return _SOURCES.get(_ALIASES.get(name, name)) + layer = _home_layer() + canonical = layer.aliases.get(name) or _ALIASES.get(name, name) + if canonical in layer.registry: + return "user" + return _SOURCES.get(canonical) def get_provider_profile(name: str) -> ProviderProfile | None: @@ -111,12 +155,13 @@ def get_provider_profile(name: str) -> ProviderProfile | None: """ if not _discovered: _discover_providers() - canonical = _ALIASES.get(name, name) - profile = _REGISTRY.get(canonical) + layer = _home_layer() + canonical = layer.aliases.get(name) or _ALIASES.get(name, name) + profile = layer.registry.get(canonical) or _REGISTRY.get(canonical) # Named custom routes share the generic wire policy unless a plugin # explicitly registered that route. Other names retain exact lookup. if profile is None and isinstance(name, str) and name.lower().startswith("custom:"): - profile = _REGISTRY.get("custom") + profile = layer.registry.get("custom") or _REGISTRY.get("custom") return profile @@ -148,22 +193,55 @@ def routed_model_rejects_vision_tool_messages(provider: str, model: str) -> bool def list_providers() -> list[ProviderProfile]: - """Return all registered provider profiles (one per canonical name).""" + """Return all registered provider profiles (one per canonical name); the bound home's + ``$HERMES_HOME`` plugins shadow process-wide profiles of the same name.""" global _PROVIDER_LIST_CACHE if not _discovered: _discover_providers() - if _PROVIDER_LIST_CACHE is not None: - return list(_PROVIDER_LIST_CACHE) - # Deduplicate: _REGISTRY has canonical names; _ALIASES points to same objects - seen: set[int] = set() - result: list[ProviderProfile] = [] - for profile in _REGISTRY.values(): - pid = id(profile) - if pid not in seen: - seen.add(pid) - result.append(profile) - _PROVIDER_LIST_CACHE = result - return list(result) + layer = _home_layer() + if _PROVIDER_LIST_CACHE is None: + # Deduplicate: _REGISTRY has canonical names; _ALIASES points to same objects + seen: set[int] = set() + cache: list[ProviderProfile] = [] + for profile in _REGISTRY.values(): + if id(profile) not in seen: + seen.add(id(profile)) + cache.append(profile) + _PROVIDER_LIST_CACHE = cache + result = [p for p in _PROVIDER_LIST_CACHE if p.name not in layer.registry] + result.extend({id(p): p for p in layer.registry.values()}.values()) + return result + + +def _home_layer() -> _HomeLayer: + """The layer for the home bound right now, importing plugin dirs it has not seen yet.""" + try: + from hermes_constants import get_hermes_home, hermes_home_key + + home = get_hermes_home() + key = hermes_home_key(home) + except Exception: + home, key = None, "" + with _HOME_LAYERS_LOCK: + layer = _HOME_LAYERS.get(key) + if layer is None: + layer = _HOME_LAYERS[key] = _HomeLayer() + # Stamps are read before the scan: a plugin that lands mid-scan changes them and the next lookup + # picks it up. Two threads scanning the same home at once only re-import idempotently. + if home is not None and (stamps := _plugin_dir_stamps(home)) != layer.stamps: + _scan_home_layer(layer, key) + layer.stamps = stamps + return layer + + +def _plugin_dir_stamps(home: Path) -> tuple: + """mtimes of ``plugins/`` and ``plugins/model-providers/``: they change when a child is added.""" + def stamp(path: Path): + try: + return os.stat(path).st_mtime_ns + except OSError: + return None + return (stamp(home / "plugins"), stamp(home / "plugins" / "model-providers")) def _user_plugins_dir() -> Path | None: @@ -228,7 +306,43 @@ def _declares_model_provider_kind(plugin_dir: Path) -> bool: return False -def _import_plugin_dir(plugin_dir: Path, source: str) -> None: +def _scan_home_layer(layer: _HomeLayer, key: str) -> None: + """Import the bound home's not-yet-imported provider plugins into *layer*. + + ``$HERMES_HOME/plugins/model-providers//`` first, then plugins cloned flat by + ``hermes plugins install`` into ``$HERMES_HOME/plugins//`` that declare + ``kind: model-provider`` (PluginManager owns every other kind there). Per-home module names + let two profiles carry the same plugin without aliasing each other's registrations. + """ + global _discovering + token, prior_discovering = _REGISTRATION_TARGET.set(layer), _discovering + _discovering = True + try: + user_dir = _user_plugins_dir() + if user_dir is not None: + for child in sorted(user_dir.iterdir()): + if child.is_dir() and not child.name.startswith(("_", ".")): + _import_plugin_dir(child, "user", home_key=key) + installed_dir = _installed_plugins_dir() + if installed_dir is not None: + for child in sorted(installed_dir.iterdir()): + if not child.is_dir() or child.name.startswith(("_", ".")) or child.name == "model-providers": + continue + if _declares_model_provider_kind(child): + _import_plugin_dir(child, "user", home_key=key) + finally: + _REGISTRATION_TARGET.reset(token) + _discovering = prior_discovering + if _discovered and not _discovering: + _sync_auth_registry() + + +def _user_module_name(plugin_dir: Path, home_key: str) -> str: + digest = hashlib.sha1(home_key.encode("utf-8")).hexdigest()[:10] + return f"_hermes_user_provider_{digest}_{plugin_dir.name.replace('-', '_')}" + + +def _import_plugin_dir(plugin_dir: Path, source: str, *, home_key: str = "") -> None: """Import a single plugin directory so it self-registers. ``source`` is "bundled" or "user"; it is recorded per registered profile (``_SOURCES``). @@ -240,13 +354,12 @@ def _import_plugin_dir(plugin_dir: Path, source: str) -> None: # Give bundled plugins a stable import path (``plugins.model_providers.``) # so relative imports within the plugin work. User plugins load via - # ``importlib.util.spec_from_file_location`` with a unique module name so + # ``importlib.util.spec_from_file_location`` under a per-home module name so # multiple HERMES_HOME profiles don't alias each other. - safe_name = plugin_dir.name.replace("-", "_") if source == "bundled": - module_name = f"plugins.model_providers.{safe_name}" + module_name = f"plugins.model_providers.{plugin_dir.name.replace('-', '_')}" else: - module_name = f"_hermes_user_provider_{safe_name}" + module_name = _user_module_name(plugin_dir, home_key) if module_name in sys.modules: return # already imported @@ -393,17 +506,15 @@ def _requires_arguments(fn) -> bool: def _discover_providers() -> None: - """Populate the registry by importing every provider plugin. + """Populate the process-wide registry by importing every provider plugin. Order: 1. Bundled plugins at ``/plugins/model-providers//`` - 2. User plugins at ``$HERMES_HOME/plugins/model-providers//`` - 2b. Plugins installed by ``hermes plugins install`` at - ``$HERMES_HOME/plugins//`` that declare ``kind: model-provider`` - 3. Legacy per-file modules at ``providers/.py`` (back-compat) + 2. Legacy per-file modules at ``providers/.py`` (back-compat) Each step imports its plugins, which call ``register_provider()`` at - module-level. Later steps win on name collision. + module-level. Later steps win on name collision. ``$HERMES_HOME`` plugins are + per profile home and load through :func:`_home_layer` at lookup time. """ global _discovered, _discovering if _discovered: @@ -444,36 +555,7 @@ def _run_discovery_steps() -> None: continue _import_plugin_dir(child, "bundled") - # 2. User plugins — under $HERMES_HOME/plugins/model-providers//. - # These can override any bundled profile of the same name (last-writer-wins - # in register_provider()). - user_dir = _user_plugins_dir() - if user_dir is not None: - for child in sorted(user_dir.iterdir()): - if not child.is_dir() or child.name.startswith(("_", ".")): - continue - _import_plugin_dir(child, "user") - - # 2b. Plugins installed by ``hermes plugins install`` / the plugin index. - # Those clone into $HERMES_HOME/plugins// — flat, NOT under - # model-providers/ — so step 2 never sees them. PluginManager does not - # import them either: it classifies ``kind: model-provider`` and routes - # it here on purpose. Without this step the documented install path - # silently half-works — the CLI reports success and the provider does - # not exist. Only manifests declaring that kind are imported; every - # other plugin in this directory belongs to PluginManager. - installed_dir = _installed_plugins_dir() - if installed_dir is not None: - for child in sorted(installed_dir.iterdir()): - if not child.is_dir() or child.name.startswith(("_", ".")): - continue - if child.name == "model-providers": - continue # handled by step 2 - if not _declares_model_provider_kind(child): - continue - _import_plugin_dir(child, "user") - - # 3. Legacy single-file profiles at providers/.py. Kept for + # 2. Legacy single-file profiles at providers/.py. Kept for # back-compat — if someone drops a ``providers/foo.py`` into an # editable install, it still works without the plugin layout. try: diff --git a/tests/providers/test_profile_home_layers.py b/tests/providers/test_profile_home_layers.py new file mode 100644 index 0000000000..186c456e69 --- /dev/null +++ b/tests/providers/test_profile_home_layers.py @@ -0,0 +1,92 @@ +"""``$HERMES_HOME`` model-provider plugins resolve for the profile home bound at lookup time (#88143). + +One process serves several profiles (multiplex gateway, Desktop ``serve``); discovery used to read the +plugins of whichever home was bound first and never look again, so a plugin installed in a secondary +profile was ``Unknown provider`` from Desktop while ``hermes -p `` in a terminal worked. +""" + +from __future__ import annotations + +import sys +import textwrap +from pathlib import Path + +import pytest + +from hermes_constants import reset_hermes_home_override, set_hermes_home_override + +_PLUGIN = textwrap.dedent( + """ + from providers import register_provider + from providers.base import ProviderProfile + + register_provider(ProviderProfile(name="{name}", aliases=("{name}-alias",), auth_type="external_process", + base_url="process://{name}", api_mode="chat_completions")) + """ +) + + +def _install(home: Path, name: str) -> None: + plugin = home / "plugins" / name + plugin.mkdir(parents=True) + (plugin / "plugin.yaml").write_text(f"name: {name}\nkind: model-provider\n", encoding="utf-8") + (plugin / "__init__.py").write_text(_PLUGIN.format(name=name), encoding="utf-8") + + +@pytest.fixture +def homes(tmp_path, monkeypatch): + import providers + + launch = tmp_path / "launch" + secondary = tmp_path / "profiles" / "scaleup" + launch.mkdir() + secondary.mkdir(parents=True) + monkeypatch.setenv("HERMES_HOME", str(launch)) + monkeypatch.setattr(providers, "_REGISTRY", dict(providers._REGISTRY)) + monkeypatch.setattr(providers, "_ALIASES", dict(providers._ALIASES)) + monkeypatch.setattr(providers, "_PROVIDER_LIST_CACHE", None) + monkeypatch.setattr(providers, "_HOME_LAYERS", {}, raising=False) + yield launch, secondary + for mod in [m for m in sys.modules if m.startswith("_hermes_user_provider")]: + del sys.modules[mod] + + +def _bound(home: Path, fn): + token = set_hermes_home_override(home) + try: + return fn() + finally: + reset_hermes_home_override(token) + + +def test_secondary_profile_plugin_resolves_for_its_home_only(homes): + import providers + from hermes_cli.auth import resolve_provider + + launch, secondary = homes + _install(secondary, "scaleup-only") + + assert providers.get_provider_profile("scaleup-only") is None # launch home discovers first + + assert _bound(secondary, lambda: providers.get_provider_profile("scaleup-only")) is not None + assert _bound(secondary, lambda: providers.get_provider_profile("scaleup-only-alias")) is not None + assert _bound(secondary, lambda: providers.provider_source("scaleup-only")) == "user" + assert "scaleup-only" in _bound(secondary, lambda: {p.name for p in providers.list_providers()}) + # The agent-build gate Desktop hits (``Unknown provider`` came from here). + assert _bound(secondary, lambda: resolve_provider("scaleup-only")) == "scaleup-only" + + # Profiles are islands: the launch home still does not see the secondary's install. + assert providers.get_provider_profile("scaleup-only") is None + assert "scaleup-only" not in {p.name for p in providers.list_providers()} + + +def test_plugin_installed_after_discovery_is_found_without_a_restart(homes): + import providers + + launch, _ = homes + assert providers.get_provider_profile("late-install") is None + + _install(launch, "late-install") + + assert providers.get_provider_profile("late-install") is not None + assert "late-install" in {p.name for p in providers.list_providers()} diff --git a/website/docs/developer-guide/model-provider-plugin.md b/website/docs/developer-guide/model-provider-plugin.md index 777e5285ab..850fc5ce11 100644 --- a/website/docs/developer-guide/model-provider-plugin.md +++ b/website/docs/developer-guide/model-provider-plugin.md @@ -17,10 +17,12 @@ Model provider plugins are the third kind of **provider plugin**. The others are `providers/__init__.py._discover_providers()` runs lazily the first time any code calls `get_provider_profile()` or `list_providers()`. Discovery order: 1. **Bundled plugins** — `/plugins/model-providers//` — ship with Hermes -2. **User plugins** — `$HERMES_HOME/plugins/model-providers//` — drop in a directory; restart an already-running Hermes process to discover it +2. **User plugins** — `$HERMES_HOME/plugins/model-providers//` — drop in a directory; a running process picks it up on its next provider lookup (no restart) 3. **Installed plugins** — `$HERMES_HOME/plugins//` (where `hermes plugins install owner/repo` clones) — imported only when `plugin.yaml` declares `kind: model-provider`; every other kind there belongs to the general PluginManager 4. **Legacy single-file** — `/providers/.py` — back-compat for out-of-tree editable installs +Steps 2 and 3 are **per profile home**: one process that serves several profiles (the multiplex gateway, the Desktop app's `hermes serve`) resolves the plugins of whichever profile's `$HERMES_HOME` is bound at lookup time, and a plugin installed in one profile is not visible from another. Install the plugin in every profile that should use it (`hermes -p plugins install ...`). + **User plugins override bundled plugins of the same name** because `register_provider()` is last-writer-wins. Drop a `$HERMES_HOME/plugins/model-providers/gmi/` directory to replace the built-in GMI profile without touching the repo. ## Directory structure