fix(memory): register parent packages for user-installed provider imports
User-installed memory providers load under the synthetic
_hermes_user_memory.<name> package, but the loader never registered that
parent namespace in sys.modules (it only registers "plugins" and
"plugins.memory" for bundled providers). As a result any external provider
using a relative import failed to load:
from . import config
ModuleNotFoundError: No module named '_hermes_user_memory'
The same gap in discover_plugin_cli_commands() meant an external provider's
cli.py with a relative import could never be discovered, so the documented
"hermes <plugin>" CLI integration did not work for standalone plugins.
Register the synthetic parent namespace before loading user-installed
providers, mirror it for cli.py discovery (including the per-provider parent
package, without executing the plugin's __init__.py), and make
_load_provider_from_dir() reuse only modules actually loaded from disk so a
parent shell registered by CLI discovery is never mistaken for the loaded
provider.
Regressions cover: a flat provider with a sibling relative import, a provider
with its implementation in a nested subpackage (including a namespace
intermediate directory), cli.py discovery with a relative import, and
provider load after CLI discovery ran first.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
committed by
Teknium
co-authored by
Claude Opus 4.8
parent
4ae3c988b5
commit
de60bf40c6
@@ -22,6 +22,7 @@ Usage:
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import importlib
|
import importlib
|
||||||
|
import importlib.machinery
|
||||||
import importlib.util
|
import importlib.util
|
||||||
import logging
|
import logging
|
||||||
import sys
|
import sys
|
||||||
@@ -33,6 +34,28 @@ logger = logging.getLogger(__name__)
|
|||||||
|
|
||||||
_MEMORY_PLUGINS_DIR = Path(__file__).parent
|
_MEMORY_PLUGINS_DIR = Path(__file__).parent
|
||||||
|
|
||||||
|
# Synthetic parent package for user-installed providers, so they don't
|
||||||
|
# collide with bundled providers in sys.modules.
|
||||||
|
_USER_NAMESPACE = "_hermes_user_memory"
|
||||||
|
|
||||||
|
|
||||||
|
def _register_synthetic_package(name: str, search_locations: List[str]) -> None:
|
||||||
|
"""Register an empty package shell in sys.modules.
|
||||||
|
|
||||||
|
User-installed providers import as ``_hermes_user_memory.<name>``, a
|
||||||
|
dotted name whose parents exist nowhere on disk. Unless those parents
|
||||||
|
are present in ``sys.modules``, any relative import inside the plugin
|
||||||
|
(``from . import config``) fails with
|
||||||
|
``ModuleNotFoundError: No module named '_hermes_user_memory'`` — the
|
||||||
|
same reason the loader already registers ``plugins`` and
|
||||||
|
``plugins.memory`` for bundled providers.
|
||||||
|
"""
|
||||||
|
if name in sys.modules:
|
||||||
|
return
|
||||||
|
spec = importlib.machinery.ModuleSpec(name, None, is_package=True)
|
||||||
|
spec.submodule_search_locations = search_locations
|
||||||
|
sys.modules[name] = importlib.util.module_from_spec(spec)
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Directory helpers
|
# Directory helpers
|
||||||
@@ -193,15 +216,18 @@ def _load_provider_from_dir(provider_dir: Path) -> Optional["MemoryProvider"]:
|
|||||||
# Use a separate namespace for user-installed plugins so they don't
|
# Use a separate namespace for user-installed plugins so they don't
|
||||||
# collide with bundled providers in sys.modules.
|
# collide with bundled providers in sys.modules.
|
||||||
_is_bundled = _MEMORY_PLUGINS_DIR in provider_dir.parents or provider_dir.parent == _MEMORY_PLUGINS_DIR
|
_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}"
|
module_name = f"plugins.memory.{name}" if _is_bundled else f"{_USER_NAMESPACE}.{name}"
|
||||||
init_file = provider_dir / "__init__.py"
|
init_file = provider_dir / "__init__.py"
|
||||||
|
|
||||||
if not init_file.exists():
|
if not init_file.exists():
|
||||||
return None
|
return None
|
||||||
|
|
||||||
# Check if already loaded
|
# Check if already loaded. A synthetic package shell registered by
|
||||||
if module_name in sys.modules:
|
# discover_plugin_cli_commands() for relative-import support has no
|
||||||
mod = sys.modules[module_name]
|
# __file__; only reuse modules that were actually loaded from disk.
|
||||||
|
cached = sys.modules.get(module_name)
|
||||||
|
if cached is not None and getattr(cached, "__file__", None):
|
||||||
|
mod = cached
|
||||||
else:
|
else:
|
||||||
# Handle relative imports within the plugin
|
# Handle relative imports within the plugin
|
||||||
# First ensure the parent packages are registered
|
# First ensure the parent packages are registered
|
||||||
@@ -224,6 +250,11 @@ def _load_provider_from_dir(provider_dir: Path) -> Optional["MemoryProvider"]:
|
|||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
# User-installed plugins need their synthetic parent registered the
|
||||||
|
# same way, or relative imports inside the plugin cannot resolve.
|
||||||
|
if not _is_bundled:
|
||||||
|
_register_synthetic_package(_USER_NAMESPACE, [])
|
||||||
|
|
||||||
# Now load the provider module
|
# Now load the provider module
|
||||||
spec = importlib.util.spec_from_file_location(
|
spec = importlib.util.spec_from_file_location(
|
||||||
module_name, str(init_file),
|
module_name, str(init_file),
|
||||||
@@ -355,12 +386,24 @@ def discover_plugin_cli_commands() -> List[dict]:
|
|||||||
return results
|
return results
|
||||||
|
|
||||||
_is_bundled = _MEMORY_PLUGINS_DIR in plugin_dir.parents or plugin_dir.parent == _MEMORY_PLUGINS_DIR
|
_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"
|
module_name = f"plugins.memory.{active_provider}.cli" if _is_bundled else f"{_USER_NAMESPACE}.{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:
|
||||||
cli_mod = sys.modules[module_name]
|
cli_mod = sys.modules[module_name]
|
||||||
else:
|
else:
|
||||||
|
if not _is_bundled:
|
||||||
|
# cli.py imports as _hermes_user_memory.<name>.cli, usually
|
||||||
|
# before the provider itself is loaded. Register its parent
|
||||||
|
# packages so relative imports inside cli.py
|
||||||
|
# ("from . import config") resolve without executing the
|
||||||
|
# plugin's __init__.py. The package shell has no __file__,
|
||||||
|
# so _load_provider_from_dir() will still load the real
|
||||||
|
# module later instead of reusing the shell.
|
||||||
|
_register_synthetic_package(_USER_NAMESPACE, [])
|
||||||
|
_register_synthetic_package(
|
||||||
|
f"{_USER_NAMESPACE}.{active_provider}", [str(plugin_dir)]
|
||||||
|
)
|
||||||
spec = importlib.util.spec_from_file_location(
|
spec = importlib.util.spec_from_file_location(
|
||||||
module_name, str(cli_file)
|
module_name, str(cli_file)
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -533,6 +533,147 @@ class TestUserInstalledProviderDiscovery:
|
|||||||
names = [n for n, _, _ in providers]
|
names = [n for n, _, _ in providers]
|
||||||
assert "notmemory" not in names
|
assert "notmemory" not in names
|
||||||
|
|
||||||
|
def test_load_user_plugin_with_relative_import(self, tmp_path, monkeypatch):
|
||||||
|
"""User plugins may import sibling modules with relative imports.
|
||||||
|
|
||||||
|
Regression: _load_provider_from_dir() imports user plugins under the
|
||||||
|
synthetic ``_hermes_user_memory.<name>`` package but never registered
|
||||||
|
that parent namespace in sys.modules, so any relative import inside
|
||||||
|
the plugin raised
|
||||||
|
``ModuleNotFoundError: No module named '_hermes_user_memory'``.
|
||||||
|
"""
|
||||||
|
from plugins.memory import load_memory_provider
|
||||||
|
plugin_dir = tmp_path / "plugins" / "relimport"
|
||||||
|
plugin_dir.mkdir(parents=True)
|
||||||
|
(plugin_dir / "helper.py").write_text("PROVIDER_NAME = 'relimport'\n")
|
||||||
|
(plugin_dir / "__init__.py").write_text(
|
||||||
|
"from agent.memory_provider import MemoryProvider\n"
|
||||||
|
"from . import helper\n"
|
||||||
|
"class MyProvider(MemoryProvider):\n"
|
||||||
|
" @property\n"
|
||||||
|
" def name(self): return helper.PROVIDER_NAME\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",
|
||||||
|
)
|
||||||
|
p = load_memory_provider("relimport")
|
||||||
|
assert p is not None
|
||||||
|
assert p.name == "relimport"
|
||||||
|
|
||||||
|
def test_load_user_plugin_with_nested_subpackage(self, tmp_path, monkeypatch):
|
||||||
|
"""User plugins may keep their implementation in a nested subpackage.
|
||||||
|
|
||||||
|
Plugin repos that target several runtimes commonly expose a thin root
|
||||||
|
``__init__.py`` re-exporting from a deeper package, and the
|
||||||
|
intermediate directory may be a namespace package (no __init__.py).
|
||||||
|
Both must resolve through the synthetic parent namespace.
|
||||||
|
"""
|
||||||
|
from plugins.memory import load_memory_provider
|
||||||
|
plugin_dir = tmp_path / "plugins" / "nestedimpl"
|
||||||
|
impl_dir = plugin_dir / "adapters" / "hermes" # adapters/ has no __init__.py
|
||||||
|
impl_dir.mkdir(parents=True)
|
||||||
|
(impl_dir / "__init__.py").write_text(
|
||||||
|
"from agent.memory_provider import MemoryProvider\n"
|
||||||
|
"class MyProvider(MemoryProvider):\n"
|
||||||
|
" @property\n"
|
||||||
|
" def name(self): return 'nestedimpl'\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 / "__init__.py").write_text(
|
||||||
|
"from .adapters.hermes import MyProvider\n"
|
||||||
|
"def register(ctx):\n"
|
||||||
|
" ctx.register_memory_provider(MyProvider())\n"
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"plugins.memory._get_user_plugins_dir",
|
||||||
|
lambda: tmp_path / "plugins",
|
||||||
|
)
|
||||||
|
p = load_memory_provider("nestedimpl")
|
||||||
|
assert p is not None
|
||||||
|
assert p.name == "nestedimpl"
|
||||||
|
|
||||||
|
|
||||||
|
class TestUserInstalledProviderCli:
|
||||||
|
"""CLI commands of user-installed providers must be discoverable.
|
||||||
|
|
||||||
|
Mirror of the relative-import regression above:
|
||||||
|
discover_plugin_cli_commands() imports the active provider's cli.py as
|
||||||
|
``_hermes_user_memory.<name>.cli`` without registering the parent
|
||||||
|
packages, so a cli.py with a relative import could never load.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def _make_plugin_with_cli(self, tmp_path, name):
|
||||||
|
plugin_dir = tmp_path / "plugins" / name
|
||||||
|
plugin_dir.mkdir(parents=True)
|
||||||
|
(plugin_dir / "__init__.py").write_text(
|
||||||
|
"from agent.memory_provider import MemoryProvider\n"
|
||||||
|
"from . import config\n"
|
||||||
|
"class MyProvider(MemoryProvider):\n"
|
||||||
|
" @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"
|
||||||
|
"def register(ctx):\n"
|
||||||
|
" ctx.register_memory_provider(MyProvider())\n"
|
||||||
|
)
|
||||||
|
(plugin_dir / "config.py").write_text("STATUS = 'ok'\n")
|
||||||
|
(plugin_dir / "cli.py").write_text(
|
||||||
|
"from . import config\n"
|
||||||
|
"def register_cli(subparser):\n"
|
||||||
|
" subparser.add_argument('--status', action='store_true')\n"
|
||||||
|
)
|
||||||
|
return plugin_dir
|
||||||
|
|
||||||
|
def _activate(self, tmp_path, monkeypatch, name):
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"plugins.memory._get_user_plugins_dir",
|
||||||
|
lambda: tmp_path / "plugins",
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"plugins.memory._get_active_memory_provider",
|
||||||
|
lambda: name,
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_cli_discovered_for_user_plugin_with_relative_import(
|
||||||
|
self, tmp_path, monkeypatch
|
||||||
|
):
|
||||||
|
"""discover_plugin_cli_commands() loads a user provider's cli.py."""
|
||||||
|
from plugins.memory import discover_plugin_cli_commands
|
||||||
|
self._make_plugin_with_cli(tmp_path, "extcli")
|
||||||
|
self._activate(tmp_path, monkeypatch, "extcli")
|
||||||
|
commands = discover_plugin_cli_commands()
|
||||||
|
assert len(commands) == 1
|
||||||
|
assert commands[0]["name"] == "extcli"
|
||||||
|
assert callable(commands[0]["setup_fn"])
|
||||||
|
|
||||||
|
def test_provider_load_after_cli_discovery(self, tmp_path, monkeypatch):
|
||||||
|
"""The provider still loads after CLI discovery ran first.
|
||||||
|
|
||||||
|
CLI discovery registers a synthetic parent package shell for the
|
||||||
|
relative imports in cli.py; _load_provider_from_dir() must load the
|
||||||
|
real plugin module instead of reusing that shell.
|
||||||
|
"""
|
||||||
|
from plugins.memory import discover_plugin_cli_commands, load_memory_provider
|
||||||
|
self._make_plugin_with_cli(tmp_path, "extcliload")
|
||||||
|
self._activate(tmp_path, monkeypatch, "extcliload")
|
||||||
|
assert len(discover_plugin_cli_commands()) == 1
|
||||||
|
p = load_memory_provider("extcliload")
|
||||||
|
assert p is not None
|
||||||
|
assert p.name == "extcliload"
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Sequential dispatch routing tests
|
# Sequential dispatch routing tests
|
||||||
|
|||||||
Reference in New Issue
Block a user