mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(memory): discover user-installed memory providers from $HERMES_HOME/plugins/ (#10529)
Memory provider discovery (discover_memory_providers, load_memory_provider) only scanned the bundled plugins/memory/ directory. User-installed providers at $HERMES_HOME/plugins/<name>/ were invisible, forcing users to symlink into the repo source tree — which broke on hermes update and created a dual-registration path causing duplicate tool names (400 errors on strict providers like Xiaomi MiMo). Changes: - Add _get_user_plugins_dir(), _is_memory_provider_dir(), _iter_provider_dirs(), and find_provider_dir() helpers to plugins/memory/__init__.py - discover_memory_providers() now scans both bundled and user dirs - load_memory_provider() uses find_provider_dir() (bundled-first) - discover_plugin_cli_commands() uses find_provider_dir() - _install_dependencies() in memory_setup.py uses find_provider_dir() - User plugins use _hermes_user_memory namespace to avoid sys.modules collisions - Non-memory user plugins filtered via source text heuristic - Bundled providers always take precedence on name collisions Fixes #4956, #9099. Supersedes #4987, #9123, #9130, #9132, #9982.
This commit is contained in:
parent
22d22cd75c
commit
a9197f9bb1
3 changed files with 222 additions and 29 deletions
|
|
@ -58,9 +58,11 @@ def _prompt(label: str, default: str | None = None, secret: bool = False) -> str
|
||||||
def _install_dependencies(provider_name: str) -> None:
|
def _install_dependencies(provider_name: str) -> None:
|
||||||
"""Install pip dependencies declared in plugin.yaml."""
|
"""Install pip dependencies declared in plugin.yaml."""
|
||||||
import subprocess
|
import subprocess
|
||||||
from pathlib import Path as _Path
|
from plugins.memory import find_provider_dir
|
||||||
|
|
||||||
plugin_dir = _Path(__file__).parent.parent / "plugins" / "memory" / provider_name
|
plugin_dir = find_provider_dir(provider_name)
|
||||||
|
if not plugin_dir:
|
||||||
|
return
|
||||||
yaml_path = plugin_dir / "plugin.yaml"
|
yaml_path = plugin_dir / "plugin.yaml"
|
||||||
if not yaml_path.exists():
|
if not yaml_path.exists():
|
||||||
return
|
return
|
||||||
|
|
|
||||||
|
|
@ -1,18 +1,22 @@
|
||||||
"""Memory provider plugin discovery.
|
"""Memory provider plugin discovery.
|
||||||
|
|
||||||
Scans ``plugins/memory/<name>/`` directories for memory provider plugins.
|
Scans two directories for memory provider plugins:
|
||||||
Each subdirectory must contain ``__init__.py`` with a class implementing
|
|
||||||
the MemoryProvider ABC.
|
|
||||||
|
|
||||||
Memory providers are separate from the general plugin system — they live
|
1. Bundled providers: ``plugins/memory/<name>/`` (shipped with hermes-agent)
|
||||||
in the repo and are always available without user installation. Only ONE
|
2. User-installed providers: ``$HERMES_HOME/plugins/<name>/``
|
||||||
can be active at a time, selected via ``memory.provider`` in config.yaml.
|
|
||||||
|
Each subdirectory must contain ``__init__.py`` with a class implementing
|
||||||
|
the MemoryProvider ABC. On name collisions, bundled providers take
|
||||||
|
precedence.
|
||||||
|
|
||||||
|
Only ONE provider can be active at a time, selected via
|
||||||
|
``memory.provider`` in config.yaml.
|
||||||
|
|
||||||
Usage:
|
Usage:
|
||||||
from plugins.memory import discover_memory_providers, load_memory_provider
|
from plugins.memory import discover_memory_providers, load_memory_provider
|
||||||
|
|
||||||
available = discover_memory_providers() # [(name, desc, available), ...]
|
available = discover_memory_providers() # [(name, desc, available), ...]
|
||||||
provider = load_memory_provider("openviking") # MemoryProvider instance
|
provider = load_memory_provider("mnemosyne") # MemoryProvider instance
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
@ -29,24 +33,101 @@ logger = logging.getLogger(__name__)
|
||||||
_MEMORY_PLUGINS_DIR = Path(__file__).parent
|
_MEMORY_PLUGINS_DIR = Path(__file__).parent
|
||||||
|
|
||||||
|
|
||||||
def discover_memory_providers() -> List[Tuple[str, str, bool]]:
|
# ---------------------------------------------------------------------------
|
||||||
"""Scan plugins/memory/ for available providers.
|
# Directory helpers
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
Returns list of (name, description, is_available) tuples.
|
def _get_user_plugins_dir() -> Optional[Path]:
|
||||||
Does NOT import the providers — just reads plugin.yaml for metadata
|
"""Return ``$HERMES_HOME/plugins/`` or None if unavailable."""
|
||||||
and does a lightweight availability check.
|
try:
|
||||||
|
from hermes_constants import get_hermes_home
|
||||||
|
d = get_hermes_home() / "plugins"
|
||||||
|
return d if d.is_dir() else None
|
||||||
|
except Exception:
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
def _is_memory_provider_dir(path: Path) -> bool:
|
||||||
|
"""Heuristic: does *path* look like a memory provider plugin?
|
||||||
|
|
||||||
|
Checks for ``register_memory_provider`` or ``MemoryProvider`` in the
|
||||||
|
``__init__.py`` source. Cheap text scan — no import needed.
|
||||||
"""
|
"""
|
||||||
results = []
|
init_file = path / "__init__.py"
|
||||||
if not _MEMORY_PLUGINS_DIR.is_dir():
|
if not init_file.exists():
|
||||||
return results
|
return False
|
||||||
|
try:
|
||||||
|
source = init_file.read_text(errors="replace")[:8192]
|
||||||
|
return "register_memory_provider" in source or "MemoryProvider" in source
|
||||||
|
except Exception:
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
def _iter_provider_dirs() -> List[Tuple[str, Path]]:
|
||||||
|
"""Yield ``(name, path)`` for all discovered provider directories.
|
||||||
|
|
||||||
|
Scans bundled first, then user-installed. Bundled takes precedence
|
||||||
|
on name collisions (first-seen wins via ``seen`` set).
|
||||||
|
"""
|
||||||
|
seen: set = set()
|
||||||
|
dirs: List[Tuple[str, Path]] = []
|
||||||
|
|
||||||
|
# 1. Bundled providers (plugins/memory/<name>/)
|
||||||
|
if _MEMORY_PLUGINS_DIR.is_dir():
|
||||||
for child in sorted(_MEMORY_PLUGINS_DIR.iterdir()):
|
for child in sorted(_MEMORY_PLUGINS_DIR.iterdir()):
|
||||||
if not child.is_dir() or child.name.startswith(("_", ".")):
|
if not child.is_dir() or child.name.startswith(("_", ".")):
|
||||||
continue
|
continue
|
||||||
init_file = child / "__init__.py"
|
if not (child / "__init__.py").exists():
|
||||||
if not init_file.exists():
|
|
||||||
continue
|
continue
|
||||||
|
seen.add(child.name)
|
||||||
|
dirs.append((child.name, child))
|
||||||
|
|
||||||
|
# 2. User-installed providers ($HERMES_HOME/plugins/<name>/)
|
||||||
|
user_dir = _get_user_plugins_dir()
|
||||||
|
if user_dir:
|
||||||
|
for child in sorted(user_dir.iterdir()):
|
||||||
|
if not child.is_dir() or child.name.startswith(("_", ".")):
|
||||||
|
continue
|
||||||
|
if child.name in seen:
|
||||||
|
continue # bundled takes precedence
|
||||||
|
if not _is_memory_provider_dir(child):
|
||||||
|
continue # skip non-memory plugins
|
||||||
|
dirs.append((child.name, child))
|
||||||
|
|
||||||
|
return dirs
|
||||||
|
|
||||||
|
|
||||||
|
def find_provider_dir(name: str) -> Optional[Path]:
|
||||||
|
"""Resolve a provider name to its directory.
|
||||||
|
|
||||||
|
Checks bundled first, then user-installed.
|
||||||
|
"""
|
||||||
|
# Bundled
|
||||||
|
bundled = _MEMORY_PLUGINS_DIR / name
|
||||||
|
if bundled.is_dir() and (bundled / "__init__.py").exists():
|
||||||
|
return bundled
|
||||||
|
# User-installed
|
||||||
|
user_dir = _get_user_plugins_dir()
|
||||||
|
if user_dir:
|
||||||
|
user = user_dir / name
|
||||||
|
if user.is_dir() and _is_memory_provider_dir(user):
|
||||||
|
return user
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Public API
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
def discover_memory_providers() -> List[Tuple[str, str, bool]]:
|
||||||
|
"""Scan bundled and user-installed directories for available providers.
|
||||||
|
|
||||||
|
Returns list of (name, description, is_available) tuples.
|
||||||
|
Bundled providers take precedence on name collisions.
|
||||||
|
"""
|
||||||
|
results = []
|
||||||
|
|
||||||
|
for name, child in _iter_provider_dirs():
|
||||||
# Read description from plugin.yaml if available
|
# Read description from plugin.yaml if available
|
||||||
desc = ""
|
desc = ""
|
||||||
yaml_file = child / "plugin.yaml"
|
yaml_file = child / "plugin.yaml"
|
||||||
|
|
@ -70,7 +151,7 @@ def discover_memory_providers() -> List[Tuple[str, str, bool]]:
|
||||||
except Exception:
|
except Exception:
|
||||||
available = False
|
available = False
|
||||||
|
|
||||||
results.append((child.name, desc, available))
|
results.append((name, desc, available))
|
||||||
|
|
||||||
return results
|
return results
|
||||||
|
|
||||||
|
|
@ -78,11 +159,15 @@ def discover_memory_providers() -> List[Tuple[str, str, bool]]:
|
||||||
def load_memory_provider(name: str) -> Optional["MemoryProvider"]:
|
def load_memory_provider(name: str) -> Optional["MemoryProvider"]:
|
||||||
"""Load and return a MemoryProvider instance by name.
|
"""Load and return a MemoryProvider instance by name.
|
||||||
|
|
||||||
|
Checks both bundled (``plugins/memory/<name>/``) and user-installed
|
||||||
|
(``$HERMES_HOME/plugins/<name>/``) directories. Bundled takes
|
||||||
|
precedence on name collisions.
|
||||||
|
|
||||||
Returns None if the provider is not found or fails to load.
|
Returns None if the provider is not found or fails to load.
|
||||||
"""
|
"""
|
||||||
provider_dir = _MEMORY_PLUGINS_DIR / name
|
provider_dir = find_provider_dir(name)
|
||||||
if not provider_dir.is_dir():
|
if not provider_dir:
|
||||||
logger.debug("Memory provider '%s' not found in %s", name, _MEMORY_PLUGINS_DIR)
|
logger.debug("Memory provider '%s' not found in bundled or user plugins", name)
|
||||||
return None
|
return None
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
|
@ -104,7 +189,10 @@ def _load_provider_from_dir(provider_dir: Path) -> Optional["MemoryProvider"]:
|
||||||
- A top-level class that extends MemoryProvider — we instantiate it
|
- A top-level class that extends MemoryProvider — we instantiate it
|
||||||
"""
|
"""
|
||||||
name = provider_dir.name
|
name = provider_dir.name
|
||||||
module_name = f"plugins.memory.{name}"
|
# Use a separate namespace for user-installed plugins so they don't
|
||||||
|
# collide with bundled providers in sys.modules.
|
||||||
|
_is_bundled = _MEMORY_PLUGINS_DIR in provider_dir.parents or provider_dir.parent == _MEMORY_PLUGINS_DIR
|
||||||
|
module_name = f"plugins.memory.{name}" if _is_bundled else f"_hermes_user_memory.{name}"
|
||||||
init_file = provider_dir / "__init__.py"
|
init_file = provider_dir / "__init__.py"
|
||||||
|
|
||||||
if not init_file.exists():
|
if not init_file.exists():
|
||||||
|
|
@ -257,15 +345,16 @@ def discover_plugin_cli_commands() -> List[dict]:
|
||||||
return results
|
return results
|
||||||
|
|
||||||
# Only look at the active provider's directory
|
# Only look at the active provider's directory
|
||||||
plugin_dir = _MEMORY_PLUGINS_DIR / active_provider
|
plugin_dir = find_provider_dir(active_provider)
|
||||||
if not plugin_dir.is_dir():
|
if not plugin_dir:
|
||||||
return results
|
return results
|
||||||
|
|
||||||
cli_file = plugin_dir / "cli.py"
|
cli_file = plugin_dir / "cli.py"
|
||||||
if not cli_file.exists():
|
if not cli_file.exists():
|
||||||
return results
|
return results
|
||||||
|
|
||||||
module_name = f"plugins.memory.{active_provider}.cli"
|
_is_bundled = _MEMORY_PLUGINS_DIR in plugin_dir.parents or plugin_dir.parent == _MEMORY_PLUGINS_DIR
|
||||||
|
module_name = f"plugins.memory.{active_provider}.cli" if _is_bundled else f"_hermes_user_memory.{active_provider}.cli"
|
||||||
try:
|
try:
|
||||||
# Import the CLI module (lightweight — no SDK needed)
|
# Import the CLI module (lightweight — no SDK needed)
|
||||||
if module_name in sys.modules:
|
if module_name in sys.modules:
|
||||||
|
|
|
||||||
|
|
@ -396,6 +396,108 @@ class TestPluginMemoryDiscovery:
|
||||||
assert load_memory_provider("nonexistent_provider") is None
|
assert load_memory_provider("nonexistent_provider") is None
|
||||||
|
|
||||||
|
|
||||||
|
class TestUserInstalledProviderDiscovery:
|
||||||
|
"""Memory providers installed to $HERMES_HOME/plugins/ should be found.
|
||||||
|
|
||||||
|
Regression test for issues #4956 and #9099: load_memory_provider() and
|
||||||
|
discover_memory_providers() only scanned the bundled plugins/memory/
|
||||||
|
directory, ignoring user-installed plugins.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def _make_user_memory_plugin(self, tmp_path, name="myprovider"):
|
||||||
|
"""Create a minimal user memory provider plugin."""
|
||||||
|
plugin_dir = tmp_path / "plugins" / name
|
||||||
|
plugin_dir.mkdir(parents=True)
|
||||||
|
(plugin_dir / "__init__.py").write_text(
|
||||||
|
"from agent.memory_provider import MemoryProvider\n"
|
||||||
|
"class MyProvider(MemoryProvider):\n"
|
||||||
|
f" @property\n"
|
||||||
|
f" def name(self): return {name!r}\n"
|
||||||
|
" def is_available(self): return True\n"
|
||||||
|
" def initialize(self, **kw): pass\n"
|
||||||
|
" def sync_turn(self, *a, **kw): pass\n"
|
||||||
|
" def get_tool_schemas(self): return []\n"
|
||||||
|
" def handle_tool_call(self, *a, **kw): return '{}'\n"
|
||||||
|
)
|
||||||
|
(plugin_dir / "plugin.yaml").write_text(
|
||||||
|
f"name: {name}\ndescription: Test user provider\n"
|
||||||
|
)
|
||||||
|
return plugin_dir
|
||||||
|
|
||||||
|
def test_discover_finds_user_plugins(self, tmp_path, monkeypatch):
|
||||||
|
"""discover_memory_providers() includes user-installed plugins."""
|
||||||
|
from plugins.memory import discover_memory_providers, _get_user_plugins_dir
|
||||||
|
self._make_user_memory_plugin(tmp_path, "myexternal")
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"plugins.memory._get_user_plugins_dir",
|
||||||
|
lambda: tmp_path / "plugins",
|
||||||
|
)
|
||||||
|
providers = discover_memory_providers()
|
||||||
|
names = [n for n, _, _ in providers]
|
||||||
|
assert "myexternal" in names
|
||||||
|
assert "holographic" in names # bundled still found
|
||||||
|
|
||||||
|
def test_load_user_plugin(self, tmp_path, monkeypatch):
|
||||||
|
"""load_memory_provider() can load from $HERMES_HOME/plugins/."""
|
||||||
|
from plugins.memory import load_memory_provider
|
||||||
|
self._make_user_memory_plugin(tmp_path, "myexternal")
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"plugins.memory._get_user_plugins_dir",
|
||||||
|
lambda: tmp_path / "plugins",
|
||||||
|
)
|
||||||
|
p = load_memory_provider("myexternal")
|
||||||
|
assert p is not None
|
||||||
|
assert p.name == "myexternal"
|
||||||
|
assert p.is_available()
|
||||||
|
|
||||||
|
def test_bundled_takes_precedence(self, tmp_path, monkeypatch):
|
||||||
|
"""Bundled provider wins when user plugin has the same name."""
|
||||||
|
from plugins.memory import load_memory_provider, discover_memory_providers
|
||||||
|
# Create user plugin named "holographic" (same as bundled)
|
||||||
|
plugin_dir = tmp_path / "plugins" / "holographic"
|
||||||
|
plugin_dir.mkdir(parents=True)
|
||||||
|
(plugin_dir / "__init__.py").write_text(
|
||||||
|
"from agent.memory_provider import MemoryProvider\n"
|
||||||
|
"class Fake(MemoryProvider):\n"
|
||||||
|
" @property\n"
|
||||||
|
" def name(self): return 'holographic-FAKE'\n"
|
||||||
|
" def is_available(self): return True\n"
|
||||||
|
" def initialize(self, **kw): pass\n"
|
||||||
|
" def sync_turn(self, *a, **kw): pass\n"
|
||||||
|
" def get_tool_schemas(self): return []\n"
|
||||||
|
" def handle_tool_call(self, *a, **kw): return '{}'\n"
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"plugins.memory._get_user_plugins_dir",
|
||||||
|
lambda: tmp_path / "plugins",
|
||||||
|
)
|
||||||
|
# Load should return bundled (name "holographic"), not user (name "holographic-FAKE")
|
||||||
|
p = load_memory_provider("holographic")
|
||||||
|
assert p is not None
|
||||||
|
assert p.name == "holographic" # bundled wins
|
||||||
|
|
||||||
|
# discover should not duplicate
|
||||||
|
providers = discover_memory_providers()
|
||||||
|
holo_count = sum(1 for n, _, _ in providers if n == "holographic")
|
||||||
|
assert holo_count == 1
|
||||||
|
|
||||||
|
def test_non_memory_user_plugins_excluded(self, tmp_path, monkeypatch):
|
||||||
|
"""User plugins that don't reference MemoryProvider are skipped."""
|
||||||
|
from plugins.memory import discover_memory_providers
|
||||||
|
plugin_dir = tmp_path / "plugins" / "notmemory"
|
||||||
|
plugin_dir.mkdir(parents=True)
|
||||||
|
(plugin_dir / "__init__.py").write_text(
|
||||||
|
"def register(ctx):\n ctx.register_tool('foo', 'bar', {}, lambda: None)\n"
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"plugins.memory._get_user_plugins_dir",
|
||||||
|
lambda: tmp_path / "plugins",
|
||||||
|
)
|
||||||
|
providers = discover_memory_providers()
|
||||||
|
names = [n for n, _, _ in providers]
|
||||||
|
assert "notmemory" not in names
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Sequential dispatch routing tests
|
# Sequential dispatch routing tests
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue