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