fix: gate entry-point provider scan on plugins.enabled and skip register(ctx) targets
Follow-ups on salvaged #81419: - Honor the plugins.enabled allow-list / plugins.disabled deny-list (same opt-in contract as the general PluginManager) — installed != loaded. - Skip callables that require arguments: general plugins share the hermes_agent.plugins group with register(ctx) targets; invoking them zero-arg would TypeError-spam every startup. - Fix test docstring (entry points are discovered FIRST, lowest precedence) and docs mechanism wording; document the config gate. - New tests: opt-in gate, deny-list, register(ctx) never invoked. E2E-verified with a real pip-built package against a temp HERMES_HOME.
This commit is contained in:
@@ -160,6 +160,18 @@ def _discover_entry_point_providers() -> None:
|
||||
(``module`` — imported for its module-level ``register_provider`` side
|
||||
effect, mirroring the directory-plugin ``__init__.py`` contract).
|
||||
|
||||
Gating and safety:
|
||||
|
||||
* **Opt-in.** Entry-point plugins are subject to the same
|
||||
``plugins.enabled`` allow-list (and ``plugins.disabled`` deny-list) the
|
||||
general PluginManager enforces — a pip package is never imported just
|
||||
because it is installed. An entry point whose name is not enabled is
|
||||
skipped without loading.
|
||||
* **Provider targets only.** The ``hermes_agent.plugins`` group is shared
|
||||
with general plugins whose target is ``register(ctx)``. Callables that
|
||||
require arguments are skipped here (the PluginManager owns them);
|
||||
provider registration hooks take no arguments by contract.
|
||||
|
||||
Failures are swallowed per-entry (a broken third-party package must not
|
||||
break provider discovery) and logged at warning level. This scan runs
|
||||
first, so filesystem plugins (bundled + ``$HERMES_HOME``) keep their
|
||||
@@ -172,6 +184,18 @@ def _discover_entry_point_providers() -> None:
|
||||
except Exception: # pragma: no cover — importlib.metadata always present ≥3.8
|
||||
return
|
||||
|
||||
# Same opt-in gate as the general PluginManager: only entry points named
|
||||
# in ``plugins.enabled`` load, and ``plugins.disabled`` always wins.
|
||||
try:
|
||||
from hermes_cli.plugins import _get_disabled_plugins, _get_enabled_plugins
|
||||
|
||||
enabled = _get_enabled_plugins() # None = nothing enabled yet (opt-in default)
|
||||
disabled = _get_disabled_plugins()
|
||||
except Exception: # pragma: no cover — config layer unavailable
|
||||
enabled, disabled = None, set()
|
||||
if not enabled:
|
||||
return
|
||||
|
||||
group = "hermes_agent.plugins"
|
||||
try:
|
||||
eps = _md.entry_points()
|
||||
@@ -185,6 +209,11 @@ def _discover_entry_point_providers() -> None:
|
||||
return
|
||||
|
||||
for ep in group_eps:
|
||||
if ep.name not in enabled or ep.name in disabled:
|
||||
logger.debug(
|
||||
"entry-point provider %r skipped: not enabled in config", ep.name
|
||||
)
|
||||
continue
|
||||
try:
|
||||
loaded = ep.load()
|
||||
except Exception as exc:
|
||||
@@ -193,8 +222,18 @@ def _discover_entry_point_providers() -> None:
|
||||
)
|
||||
continue
|
||||
# ``module:func`` → callable we invoke; bare ``module`` → import side
|
||||
# effect already happened during load(). Only call when it's callable.
|
||||
# effect already happened during load(). Only call when it's callable
|
||||
# AND zero-arg: general plugins in this shared group expose
|
||||
# ``register(ctx)`` (requires an argument) and belong to the
|
||||
# PluginManager, not the provider registry.
|
||||
if callable(loaded):
|
||||
if _requires_arguments(loaded):
|
||||
logger.debug(
|
||||
"entry-point %r skipped by provider scan: target requires "
|
||||
"arguments (general plugin owned by PluginManager)",
|
||||
ep.name,
|
||||
)
|
||||
continue
|
||||
try:
|
||||
loaded()
|
||||
except Exception as exc:
|
||||
@@ -205,6 +244,30 @@ def _discover_entry_point_providers() -> None:
|
||||
)
|
||||
|
||||
|
||||
def _requires_arguments(fn) -> bool:
|
||||
"""True when ``fn`` cannot be called with zero arguments.
|
||||
|
||||
Used to distinguish provider registration hooks (zero-arg by contract)
|
||||
from general plugin hooks (``register(ctx)``) sharing the same entry-point
|
||||
group. Unintrospectable callables (C extensions) are treated as zero-arg
|
||||
and left to the per-entry exception guard.
|
||||
"""
|
||||
import inspect
|
||||
|
||||
try:
|
||||
sig = inspect.signature(fn)
|
||||
except (TypeError, ValueError): # pragma: no cover — builtins/C callables
|
||||
return False
|
||||
for param in sig.parameters.values():
|
||||
if param.kind in (
|
||||
inspect.Parameter.POSITIONAL_ONLY,
|
||||
inspect.Parameter.POSITIONAL_OR_KEYWORD,
|
||||
inspect.Parameter.KEYWORD_ONLY,
|
||||
) and param.default is inspect.Parameter.empty:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _discover_providers() -> None:
|
||||
"""Populate the registry by importing every provider plugin.
|
||||
|
||||
|
||||
@@ -60,6 +60,19 @@ class _FakeEP:
|
||||
return self._loader()
|
||||
|
||||
|
||||
def _enable(monkeypatch, *names, disabled=()):
|
||||
"""Gate helper: mark entry-point names enabled/disabled in config.
|
||||
|
||||
``_discover_entry_point_providers`` enforces the PluginManager's
|
||||
``plugins.enabled`` opt-in allow-list, so tests must enable their fake
|
||||
entry points explicitly.
|
||||
"""
|
||||
import hermes_cli.plugins as hp
|
||||
|
||||
monkeypatch.setattr(hp, "_get_enabled_plugins", lambda: set(names))
|
||||
monkeypatch.setattr(hp, "_get_disabled_plugins", lambda: set(disabled))
|
||||
|
||||
|
||||
class _FakeEntryPoints:
|
||||
def __init__(self, eps):
|
||||
self._eps = eps
|
||||
@@ -100,6 +113,7 @@ def test_entry_point_callable_and_module_targets(monkeypatch):
|
||||
import importlib.metadata as md
|
||||
|
||||
monkeypatch.setattr(md, "entry_points", lambda: fake_eps)
|
||||
_enable(monkeypatch, "ep-callable", "ep-module")
|
||||
_clear_provider_caches()
|
||||
try:
|
||||
assert providers.get_provider_profile("ep-callable") is not None
|
||||
@@ -109,6 +123,57 @@ def test_entry_point_callable_and_module_targets(monkeypatch):
|
||||
_clear_provider_caches()
|
||||
|
||||
|
||||
def test_entry_point_not_enabled_is_skipped(monkeypatch):
|
||||
"""Entry points honor the plugins.enabled opt-in gate — installed ≠ loaded."""
|
||||
fake_eps = _FakeEntryPoints([_FakeEP("ep-callable", _register_via_callable)])
|
||||
import importlib.metadata as md
|
||||
|
||||
monkeypatch.setattr(md, "entry_points", lambda: fake_eps)
|
||||
_enable(monkeypatch, "some-other-plugin") # ep-callable NOT enabled
|
||||
_clear_provider_caches()
|
||||
try:
|
||||
assert providers.get_provider_profile("ep-callable") is None
|
||||
finally:
|
||||
_clear_provider_caches()
|
||||
|
||||
|
||||
def test_entry_point_disabled_wins_over_enabled(monkeypatch):
|
||||
"""plugins.disabled is a deny-list that beats plugins.enabled."""
|
||||
fake_eps = _FakeEntryPoints([_FakeEP("ep-callable", _register_via_callable)])
|
||||
import importlib.metadata as md
|
||||
|
||||
monkeypatch.setattr(md, "entry_points", lambda: fake_eps)
|
||||
_enable(monkeypatch, "ep-callable", disabled=("ep-callable",))
|
||||
_clear_provider_caches()
|
||||
try:
|
||||
assert providers.get_provider_profile("ep-callable") is None
|
||||
finally:
|
||||
_clear_provider_caches()
|
||||
|
||||
|
||||
def test_general_plugin_register_ctx_not_invoked(monkeypatch):
|
||||
"""A register(ctx)-style general plugin sharing the group is never called."""
|
||||
calls = []
|
||||
|
||||
def _general_plugin_target():
|
||||
def register(ctx): # requires an argument — PluginManager contract
|
||||
calls.append(ctx)
|
||||
|
||||
return register
|
||||
|
||||
fake_eps = _FakeEntryPoints([_FakeEP("general-plugin", _general_plugin_target)])
|
||||
import importlib.metadata as md
|
||||
|
||||
monkeypatch.setattr(md, "entry_points", lambda: fake_eps)
|
||||
_enable(monkeypatch, "general-plugin")
|
||||
_clear_provider_caches()
|
||||
try:
|
||||
providers._discover_providers()
|
||||
assert calls == [] # never invoked (would have been a TypeError anyway)
|
||||
finally:
|
||||
_clear_provider_caches()
|
||||
|
||||
|
||||
def test_entry_point_failure_is_isolated(monkeypatch):
|
||||
def _boom():
|
||||
raise RuntimeError("broken plugin")
|
||||
@@ -122,6 +187,7 @@ def test_entry_point_failure_is_isolated(monkeypatch):
|
||||
import importlib.metadata as md
|
||||
|
||||
monkeypatch.setattr(md, "entry_points", lambda: fake_eps)
|
||||
_enable(monkeypatch, "broken", "ep-callable")
|
||||
_clear_provider_caches()
|
||||
try:
|
||||
# A broken entry point must not prevent the good one from registering.
|
||||
@@ -131,7 +197,9 @@ def test_entry_point_failure_is_isolated(monkeypatch):
|
||||
|
||||
|
||||
def test_filesystem_plugins_win_over_entry_points(monkeypatch):
|
||||
"""Entry points scan last, so a bundled/user profile of the same name wins."""
|
||||
"""Entry points are discovered FIRST (lowest precedence): last-writer-wins
|
||||
in register_provider() means a bundled/user profile of the same name
|
||||
overrides a pip impostor."""
|
||||
from providers.base import ProviderProfile
|
||||
|
||||
def _register_ep_openrouter():
|
||||
@@ -146,6 +214,7 @@ def test_filesystem_plugins_win_over_entry_points(monkeypatch):
|
||||
import importlib.metadata as md
|
||||
|
||||
monkeypatch.setattr(md, "entry_points", lambda: fake_eps)
|
||||
_enable(monkeypatch, "openrouter") # enabled, so precedence is what's tested
|
||||
_clear_provider_caches()
|
||||
try:
|
||||
p = providers.get_provider_profile("openrouter")
|
||||
|
||||
@@ -264,16 +264,33 @@ The target may be either:
|
||||
`register_provider(...)` side effect, mirroring the directory-plugin
|
||||
`__init__.py` contract.
|
||||
|
||||
`providers/__init__.py` discovers these entry points itself (the general
|
||||
`PluginManager` records model-provider manifests but never imports them, so it
|
||||
cannot register the profile). Entry-point plugins are discovered **before**
|
||||
filesystem plugins, giving them the lowest precedence: because
|
||||
`register_provider()` is last-writer-wins, a bundled or `$HERMES_HOME` profile
|
||||
of the same name always overrides a pip-installed one. A pip package can add a
|
||||
genuinely new provider, but cannot silently hijack a first-party provider name.
|
||||
`providers/__init__.py` discovers these entry points itself — the general
|
||||
`PluginManager` never invokes provider registration for pip packages (its
|
||||
entry-point path targets `register(ctx)`-style general plugins, gated by
|
||||
`plugins.enabled`), so the provider registry does its own scan. Two rules
|
||||
apply:
|
||||
|
||||
A broken entry point is isolated — it is logged at warning level and skipped,
|
||||
and never blocks discovery of the other providers.
|
||||
- **Opt-in required.** The same `plugins.enabled` allow-list (and
|
||||
`plugins.disabled` deny-list) from `config.yaml` governs this scan. A pip
|
||||
package is never imported just because it is installed — users must add the
|
||||
entry-point name to `plugins.enabled`:
|
||||
|
||||
```yaml
|
||||
plugins:
|
||||
enabled:
|
||||
- acme-inference
|
||||
```
|
||||
|
||||
- **Lowest precedence.** Entry-point plugins are discovered **before**
|
||||
filesystem plugins: because `register_provider()` is last-writer-wins, a
|
||||
bundled or `$HERMES_HOME` profile of the same name always overrides a
|
||||
pip-installed one. A pip package can add a genuinely new provider, but
|
||||
cannot silently hijack a first-party provider name.
|
||||
|
||||
Targets that require arguments (a general plugin's `register(ctx)`) are
|
||||
skipped by the provider scan — they belong to the `PluginManager`. A broken
|
||||
entry point is isolated — it is logged at warning level and skipped, and never
|
||||
blocks discovery of the other providers.
|
||||
|
||||
See [Building a Hermes Plugin](/developer-guide/plugins#distribute-via-pip) for the full entry-points setup.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user