diff --git a/hermes_cli/memory_setup.py b/hermes_cli/memory_setup.py index e6a61316a..88186b8ec 100644 --- a/hermes_cli/memory_setup.py +++ b/hermes_cli/memory_setup.py @@ -58,9 +58,11 @@ def _prompt(label: str, default: str | None = None, secret: bool = False) -> str def _install_dependencies(provider_name: str) -> None: """Install pip dependencies declared in plugin.yaml.""" 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" if not yaml_path.exists(): return diff --git a/plugins/memory/__init__.py b/plugins/memory/__init__.py index cd583e6d8..0ae65a25d 100644 --- a/plugins/memory/__init__.py +++ b/plugins/memory/__init__.py @@ -1,18 +1,22 @@ """Memory provider plugin discovery. -Scans ``plugins/memory//`` directories for memory provider plugins. -Each subdirectory must contain ``__init__.py`` with a class implementing -the MemoryProvider ABC. +Scans two directories for memory provider plugins: -Memory providers are separate from the general plugin system — they live -in the repo and are always available without user installation. Only ONE -can be active at a time, selected via ``memory.provider`` in config.yaml. +1. Bundled providers: ``plugins/memory//`` (shipped with hermes-agent) +2. User-installed providers: ``$HERMES_HOME/plugins//`` + +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: from plugins.memory import discover_memory_providers, load_memory_provider 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 @@ -29,24 +33,101 @@ logger = logging.getLogger(__name__) _MEMORY_PLUGINS_DIR = Path(__file__).parent +# --------------------------------------------------------------------------- +# Directory helpers +# --------------------------------------------------------------------------- + +def _get_user_plugins_dir() -> Optional[Path]: + """Return ``$HERMES_HOME/plugins/`` or None if unavailable.""" + 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. + """ + init_file = path / "__init__.py" + if not init_file.exists(): + 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//) + if _MEMORY_PLUGINS_DIR.is_dir(): + for child in sorted(_MEMORY_PLUGINS_DIR.iterdir()): + if not child.is_dir() or child.name.startswith(("_", ".")): + continue + if not (child / "__init__.py").exists(): + continue + seen.add(child.name) + dirs.append((child.name, child)) + + # 2. User-installed providers ($HERMES_HOME/plugins//) + 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 plugins/memory/ for available providers. + """Scan bundled and user-installed directories for available providers. Returns list of (name, description, is_available) tuples. - Does NOT import the providers — just reads plugin.yaml for metadata - and does a lightweight availability check. + Bundled providers take precedence on name collisions. """ results = [] - if not _MEMORY_PLUGINS_DIR.is_dir(): - return results - - for child in sorted(_MEMORY_PLUGINS_DIR.iterdir()): - if not child.is_dir() or child.name.startswith(("_", ".")): - continue - init_file = child / "__init__.py" - if not init_file.exists(): - continue + for name, child in _iter_provider_dirs(): # Read description from plugin.yaml if available desc = "" yaml_file = child / "plugin.yaml" @@ -70,7 +151,7 @@ def discover_memory_providers() -> List[Tuple[str, str, bool]]: except Exception: available = False - results.append((child.name, desc, available)) + results.append((name, desc, available)) return results @@ -78,11 +159,15 @@ def discover_memory_providers() -> List[Tuple[str, str, bool]]: def load_memory_provider(name: str) -> Optional["MemoryProvider"]: """Load and return a MemoryProvider instance by name. + Checks both bundled (``plugins/memory//``) and user-installed + (``$HERMES_HOME/plugins//``) directories. Bundled takes + precedence on name collisions. + Returns None if the provider is not found or fails to load. """ - provider_dir = _MEMORY_PLUGINS_DIR / name - if not provider_dir.is_dir(): - logger.debug("Memory provider '%s' not found in %s", name, _MEMORY_PLUGINS_DIR) + provider_dir = find_provider_dir(name) + if not provider_dir: + logger.debug("Memory provider '%s' not found in bundled or user plugins", name) return None 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 """ 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" if not init_file.exists(): @@ -257,15 +345,16 @@ def discover_plugin_cli_commands() -> List[dict]: return results # Only look at the active provider's directory - plugin_dir = _MEMORY_PLUGINS_DIR / active_provider - if not plugin_dir.is_dir(): + plugin_dir = find_provider_dir(active_provider) + if not plugin_dir: return results cli_file = plugin_dir / "cli.py" if not cli_file.exists(): 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: # Import the CLI module (lightweight — no SDK needed) if module_name in sys.modules: diff --git a/tests/agent/test_memory_provider.py b/tests/agent/test_memory_provider.py index 7e07d2f33..db2a70c2f 100644 --- a/tests/agent/test_memory_provider.py +++ b/tests/agent/test_memory_provider.py @@ -396,6 +396,108 @@ class TestPluginMemoryDiscovery: 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 # ---------------------------------------------------------------------------