hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 28 Packages Projects Releases Wiki Activity

fix: scope plugin manager by resolved hermes home (keyed cache)

Browse Source 
fix: remove .codegraph artifacts from commit
This commit is contained in:
terry197913
2026-07-13 09:02:35 +00:00
committed by Teknium
parent 22af80bcfd
commit 4e1b2e436c
4 changed files with 396 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

77
docs/ADR.md Normal file
View File

@@ -0,0 +1,77 @@
# Architecture Decision Records
## 2026-07-13: Scope plugin manager state by Hermes home/profile (keyed cache)
Status: Accepted
Context:
Hermes supports multiple profiles via different Hermes home directories.
Homes are switched two ways in a running process: the `HERMES_HOME`
environment variable (single-profile CLI/gateway processes), and the
context-local `set_hermes_home_override()` (`hermes_constants.py`), which
the multiplexed gateway worker (`gateway/run.py`'s `_profile_scope`) and
subagent/embedded callers use to serve several profiles from one
long-lived process. The override is a `ContextVar` and deliberately does
**not** mutate `os.environ`, since that would leak one profile's home
into every other concurrent task in the same process.
The plugin manager was a process-global single-slot singleton
(`_plugin_manager`). User-installed plugins are discovered from
`get_hermes_home() / "plugins"`, and context-engine plugins (e.g.
`hermes-lcm`) capture profile-scoped state — such as the LCM database
path — at registration time. A single-slot cache meant:
1. Switching homes via `set_hermes_home_override()` was invisible to a
naive "did `HERMES_HOME` change" check, so the singleton silently kept
serving the first profile's manager to every other profile in the
process.
2. Even when a fresh `PluginManager` *was* created for a new home, plugin
modules are imported into `sys.modules` as `hermes_plugins.<slug>` by
`_load_directory_module`, and only that top-level module was ever
replaced. A same-slug plugin's *relative* imports
(`from . import state`) are cached separately under
`hermes_plugins.<slug>.<submodule>`, and Python's import machinery
resolves those from `sys.modules` first — so a profile switch could
silently keep serving a previous profile's already-imported submodule
code/state instead of re-executing the new profile's plugin.
Decision:
- Replace the single-slot singleton with a cache keyed on the *resolved*
Hermes home path (`_plugin_managers_by_home: Dict[Path, PluginManager]`).
`get_plugin_manager()` resolves the current home via `get_hermes_home()`
(which itself already consults `get_hermes_home_override()` before
`os.environ`), so both the env-var and context-local override paths are
covered uniformly.
- `_plugin_manager` (the old single-slot name) is kept as a thin "last
manager returned" pointer purely for backward compatibility with
existing test code that does
`monkeypatch.setattr(plugins_mod, "_plugin_manager", some_manager)`.
When that name is monkeypatched to a manager the keyed cache doesn't
know about, `get_plugin_manager()` treats it as an explicit injection
and adopts it into the cache under the *current* resolved home, rather
than discarding it.
- Both `PluginManager._load_directory_module` (initial/`force=True`
reload within the same home) and the shared `_clear_plugin_submodules`
helper (profile switch / test teardown) evict `sys.modules[module_name]`
**and every name prefixed with `module_name + "."`** before a plugin
slug is (re-)imported, so relative-import submodules can never survive
a reload or a home switch.
- Test isolation (`tests/conftest.py`'s `_hermetic_environment` fixture)
calls a new `_reset_plugin_managers_for_tests()` helper that drops the
entire keyed cache and purges every plugin submodule from `sys.modules`
between tests, instead of only resetting the single-slot pointer.
Consequences:
- Per-profile LCM instances (and any other context-engine plugin) use
their own `{home}/lcm.db` regardless of whether the profile switch went
through `HERMES_HOME` or `set_hermes_home_override()`.
- Plugin discovery remains cached within a profile for normal
performance, and re-entering a previously-seen profile reuses its
cached manager instead of rebuilding from scratch.
- Sequential *and* interleaved profile switching — in tests, the gateway
multiplexer worker, or embedded callers using the context-local
override — no longer leaks context-engine state, plugin module state,
or stale relative-import submodules across profiles.
- Regression coverage exercises the real production path
(`set_hermes_home_override()`) rather than only the env-var path, and
includes a dedicated relative-import leak test.

142
hermes_cli/plugins.py
View File

@@ -3845,6 +3845,23 @@ class PluginManager:
key = manifest.key or manifest.name
slug = key.replace("/", "__").replace("-", "_")
module_name = f"{_NS_PARENT}.{slug}"
# Evict any stale sys.modules entries for this slug before
# (re-)importing. A same-slug module may already be cached here
# from a different Hermes home (profile switch reusing a slug
# like "hermes-lcm") or from an earlier force=True reload in the
# same home. Replacing only sys.modules[module_name] below is not
# enough: the plugin's own relative imports (`from . import foo`)
# are cached separately under "module_name + '.' + submodule",
# and Python's import system resolves those from sys.modules
# first — so a stale submodule would silently keep serving the
# previous load's code/state instead of the fresh one we're
# about to exec. Evict the package and everything nested under
# it so this import starts clean.
stale_prefix = f"{module_name}."
for name in [n for n in sys.modules if n == module_name or n.startswith(stale_prefix)]:
del sys.modules[name]
spec = importlib.util.spec_from_file_location(
module_name,
init_file,
@@ -3857,7 +3874,16 @@ class PluginManager:
module.__package__ = module_name
module.__path__ = [str(plugin_dir)] # type: ignore[attr-defined]
sys.modules[module_name] = module
spec.loader.exec_module(module)
try:
spec.loader.exec_module(module)
except BaseException:
# Don't leave a half-initialized module (or the partially
# imported relative submodules it pulled in before failing)
# cached in sys.modules — a retry or a same-slug plugin in a
# different profile would otherwise inherit broken state.
for name in [n for n in sys.modules if n == module_name or n.startswith(stale_prefix)]:
del sys.modules[name]
raise
return module
def _load_entrypoint_module(self, manifest: PluginManifest) -> types.ModuleType:
@@ -4370,15 +4396,121 @@ class PluginManager:
# Module-level singleton & convenience functions
# ---------------------------------------------------------------------------
# Legacy single-slot singleton. Kept as the storage for the "current"
# manager so existing test code that does
# ``monkeypatch.setattr(plugins_mod, "_plugin_manager", some_manager)``
# keeps working — ``get_plugin_manager()`` still reads/writes this name.
_plugin_manager: Optional[PluginManager] = None
# Keyed cache: resolved Hermes home -> PluginManager. Hermes supports
# multiple profiles via different HERMES_HOME directories, and a single
# long-lived process (gateway multiplexer, test session, embedder) can
# switch between them via ``set_hermes_home_override()`` — which is a
# ContextVar and deliberately does NOT touch os.environ (see
# hermes_constants.set_hermes_home_override). A process-wide single-slot
# cache leaks one profile's plugin/context-engine state into another. We
# key the cache by the *resolved* home path so re-entering a previously
# seen profile reuses its manager (and picks up any modules it already
# imported) instead of rebuilding from scratch every switch.
_plugin_managers_by_home: Dict[Path, PluginManager] = {}
def _plugin_home_key() -> Path:
"""Return the profile/home key for process-global plugin state.
Plugins are discovered from ``get_hermes_home() / "plugins"`` and some
plugins (notably context engines such as hermes-lcm) capture that home
at registration time for profile-scoped storage. A long-lived process
can temporarily switch Hermes home (env var *or* the context-local
``set_hermes_home_override()``) while serving another profile, so the
plugin manager must be scoped to the active Hermes home instead of
being one process-wide singleton.
"""
try:
return get_hermes_home().expanduser().resolve()
except Exception:
return get_hermes_home().expanduser()
def _clear_plugin_submodules(manager: Optional[PluginManager]) -> None:
"""Purge ``sys.modules`` entries for directory-loaded plugins.
``PluginManager._load_directory_module`` imports each plugin as
``hermes_plugins.<slug>`` and registers that top-level module in
``sys.modules``. Anything the plugin's ``__init__.py`` imports with a
*relative* import (``from . import foo``, ``from .sub import bar``)
ends up cached in ``sys.modules`` too, under
``hermes_plugins.<slug>.<submodule>``. When we swap in a fresh manager
for a new home, replacing only the parent module leaves those
submodules behind: if a same-named plugin in the new profile does a
relative import, Python resolves it from ``sys.modules`` first and
silently reuses the *previous* profile's already-imported submodule
(and any module-level state it captured), instead of re-executing the
new profile's code. We must evict the package itself and every module
whose name is prefixed with ``"<module_name>."`` before (or when)
discarding a manager, not just drop our reference to it.
"""
if manager is None:
return
for loaded in getattr(manager, "_plugins", {}).values():
module = getattr(loaded, "module", None)
module_name = getattr(module, "__name__", None)
if not module_name or not module_name.startswith(f"{_NS_PARENT}."):
continue
prefix = f"{module_name}."
for name in [n for n in sys.modules if n == module_name or n.startswith(prefix)]:
del sys.modules[name]
def get_plugin_manager() -> PluginManager:
"""Return (and lazily create) the global PluginManager singleton."""
"""Return the plugin manager for the active Hermes profile/home.
Managers are cached per resolved home so repeated calls within the
same profile reuse discovery state (normal performance), while a
profile switch — via ``HERMES_HOME`` or the context-local
``set_hermes_home_override()`` — gets its own manager with its own
plugin submodules, instead of silently inheriting another profile's
context engine or stale relative-import state.
"""
global _plugin_manager
if _plugin_manager is None:
_plugin_manager = PluginManager()
return _plugin_manager
current_home = _plugin_home_key()
# Tests and embedders historically monkeypatch ``_plugin_manager``
# directly (``monkeypatch.setattr(plugins_mod, "_plugin_manager", mgr)``).
# Detect that specifically by checking whether the single-slot pointer
# references a manager our keyed cache doesn't know about *at all*
# (i.e. it isn't the value cached for *any* home) — that can only
# happen via a direct assignment bypassing this function, not via a
# legitimate home switch (which always leaves ``_plugin_manager``
# pointing at a manager already stored in the cache). Comparing against
# only ``current_home``'s slot is wrong: it also matches an ordinary
# switch to a *new* home that simply hasn't been cached yet, which
# would incorrectly resurrect the previous home's manager here.
if _plugin_manager is not None and _plugin_manager not in _plugin_managers_by_home.values():
_plugin_managers_by_home[current_home] = _plugin_manager
return _plugin_manager
manager = _plugin_managers_by_home.get(current_home)
if manager is None:
manager = PluginManager()
_plugin_managers_by_home[current_home] = manager
_plugin_manager = manager
return manager
def _reset_plugin_managers_for_tests() -> None:
"""Test-only helper: drop every cached manager and its submodules.
Not used by production code paths — tests that want a fully clean
slate (rather than adopting/injecting a specific manager) can call
this instead of reaching into the module's private dict directly.
"""
global _plugin_manager
for manager in _plugin_managers_by_home.values():
_clear_plugin_submodules(manager)
_plugin_managers_by_home.clear()
_plugin_manager = None
def has_enabled_agent_plugin_mcp(raw_config: Mapping[str, Any]) -> bool:

7
tests/conftest.py
View File

@@ -517,6 +517,13 @@ def _hermetic_environment(tmp_path, monkeypatch):
try:
import hermes_cli.plugins as _plugins_mod
monkeypatch.setattr(_plugins_mod, "_plugin_manager", None)
# Also clear the keyed per-home manager cache (and any plugin
# submodules it left in sys.modules) so a manager built for a
# previous test's tmp_path HERMES_HOME can't leak forward. Paths
# are unique per test, so collisions are unlikely, but a full
# reset keeps this fixture the single source of plugin-state
# hygiene rather than relying on path uniqueness.
_plugins_mod._reset_plugin_managers_for_tests()
except Exception:
pass
# Explicitly clear provider-specific base URL overrides that don't match

182
tests/hermes_cli/test_plugins.py
View File

@@ -46,7 +46,8 @@ def test_portable_skill_namespace_is_ascii_safe():
def _make_plugin_dir(base: Path, name: str, *, register_body: str = "pass",
manifest_extra: dict | None = None,
auto_enable: bool = True) -> Path:
auto_enable: bool = True,
home: Path | None = None) -> Path:
"""Create a minimal plugin directory with plugin.yaml + __init__.py.
If *auto_enable* is True (default), also write the plugin's name into
@@ -56,7 +57,19 @@ def _make_plugin_dir(base: Path, name: str, *, register_body: str = "pass",
unenabled path.
*base* is expected to be ``<hermes_home>/plugins/``; we derive
``<hermes_home>`` from it by walking one level up.
``<hermes_home>`` from it by walking one level up unless *home* is
given explicitly.
Pass *home* explicitly whenever the target Hermes home for this
plugin isn't necessarily the current ``HERMES_HOME`` env var — e.g.
when writing fixtures for two profiles up front and only switching
``HERMES_HOME``/``set_hermes_home_override()`` per-profile afterwards
(as multi-profile regression tests do). Relying on the *current* env
var here is a bug: if the caller hasn't switched homes yet (or is
using the context-local override instead of the env var, which this
function can't see), the enable list gets written to the wrong
profile's config.yaml and the plugin never loads for its intended
home.
"""
plugin_dir = base / name
plugin_dir.mkdir(parents=True, exist_ok=True)
@@ -74,12 +87,15 @@ def _make_plugin_dir(base: Path, name: str, *, register_body: str = "pass",
# Write/merge plugins.enabled in <HERMES_HOME>/config.yaml.
# Config is always read from HERMES_HOME (not from the project
# dir for project plugins), so that's where we opt in.
import os
hermes_home_str = os.environ.get("HERMES_HOME")
if hermes_home_str:
hermes_home = Path(hermes_home_str)
if home is not None:
hermes_home = Path(home)
else:
hermes_home = base.parent
import os
hermes_home_str = os.environ.get("HERMES_HOME")
if hermes_home_str:
hermes_home = Path(hermes_home_str)
else:
hermes_home = base.parent
hermes_home.mkdir(parents=True, exist_ok=True)
cfg_path = hermes_home / "config.yaml"
cfg: dict = {}
@@ -1175,6 +1191,158 @@ class TestPluginCommands:
assert engine is not None
assert engine.name == "stub-engine"
def test_plugin_manager_scoped_by_hermes_home_override(self, tmp_path):
"""set_hermes_home_override() must get its own manager per profile.
This is the production path used by the gateway multiplexer
(``gateway/run.py``'s ``_profile_scope`` context manager) and by
subagent/embedded callers: it swaps ``HERMES_HOME`` via a
context-local ContextVar, which — per
``hermes_constants.set_hermes_home_override`` — deliberately does
NOT touch ``os.environ``. A regression test that only flips the
``HERMES_HOME`` env var never exercises this path.
"""
from hermes_constants import set_hermes_home_override, reset_hermes_home_override
import hermes_cli.plugins as plugins_mod
def write_engine_plugin(home: Path) -> None:
_make_plugin_dir(
home / "plugins",
"engine-plugin",
home=home,
manifest_extra={
"description": "Context engine plugin for profile-scope test",
},
register_body=(
"import os\n"
" from agent.context_engine import ContextEngine\n\n"
" class HomeEngine(ContextEngine):\n"
" def __init__(self):\n"
" self.home = os.environ.get('HERMES_HOME')\n\n"
" @property\n"
" def name(self):\n"
" return 'home-engine'\n\n"
" def update_from_response(self, usage):\n"
" return None\n\n"
" def should_compress(self, prompt_tokens=None):\n"
" return False\n\n"
" def compress(self, messages, current_tokens=None, focus_topic=None):\n"
" return messages\n\n"
" ctx.register_context_engine(HomeEngine())"
),
)
home_a = tmp_path / "profile-a"
home_b = tmp_path / "profile-b"
write_engine_plugin(home_a)
write_engine_plugin(home_b)
# Note: HomeEngine reads os.environ['HERMES_HOME'] itself (simulating
# a real plugin like hermes-lcm capturing its home at registration),
# so we set the env var to home_a as a baseline and only use the
# context-local override to *switch away* to home_b — proving the
# override, not the env var, is what get_plugin_manager() keys on.
token_a = set_hermes_home_override(str(home_a))
try:
manager_a = plugins_mod.get_plugin_manager()
manager_a.discover_and_load()
engine_a = manager_a._context_engine
finally:
reset_hermes_home_override(token_a)
token_b = set_hermes_home_override(str(home_b))
try:
manager_b = plugins_mod.get_plugin_manager()
manager_b.discover_and_load()
engine_b = manager_b._context_engine
finally:
reset_hermes_home_override(token_b)
assert engine_a is not None
assert engine_b is not None
assert manager_a is not manager_b
assert engine_a is not engine_b
# Re-entering home_a's override must return the SAME cached manager
# (and engine) rather than rebuilding — proves the cache is keyed,
# not last-write-wins.
token_a2 = set_hermes_home_override(str(home_a))
try:
manager_a2 = plugins_mod.get_plugin_manager()
finally:
reset_hermes_home_override(token_a2)
assert manager_a2 is manager_a
def test_relative_import_not_leaked_across_home_switch(self, tmp_path):
"""A same-slug plugin's relative import must not reuse the prior home's submodule.
``_load_directory_module`` imports each plugin as
``hermes_plugins.<slug>``, but a relative import inside the
plugin's ``__init__.py`` (``from .state import STATE``) is cached
separately in ``sys.modules`` under
``hermes_plugins.<slug>.state``. If a profile switch replaces only
the parent module, the child module survives in ``sys.modules``
and Python's import system serves it back unchanged — silently
leaking the previous profile's module-level state (and code) into
the new profile.
"""
from hermes_constants import set_hermes_home_override, reset_hermes_home_override
import hermes_cli.plugins as plugins_mod
def write_stateful_plugin(home: Path, marker: str) -> None:
plugin_dir = (home / "plugins" / "stateful-plugin")
plugin_dir.mkdir(parents=True, exist_ok=True)
(plugin_dir / "plugin.yaml").write_text(
yaml.dump({
"name": "stateful-plugin",
"version": "0.1.0",
"description": "Relative-import regression plugin",
})
)
# `state.py` is imported via a *relative* import from
# `__init__.py`, so it lands in sys.modules as
# `hermes_plugins.stateful_plugin.state`.
(plugin_dir / "state.py").write_text(f"MARKER = {marker!r}\n")
(plugin_dir / "__init__.py").write_text(
"from . import state\n\n"
"def register(ctx):\n"
" ctx._manager._plugin_skills['stateful-plugin::marker'] = "
"{'marker': state.MARKER}\n"
)
(home / "config.yaml").write_text(
yaml.safe_dump({"plugins": {"enabled": ["stateful-plugin"]}})
)
home_a = tmp_path / "profile-a"
home_b = tmp_path / "profile-b"
write_stateful_plugin(home_a, "marker-a")
write_stateful_plugin(home_b, "marker-b")
token_a = set_hermes_home_override(str(home_a))
try:
manager_a = plugins_mod.get_plugin_manager()
manager_a.discover_and_load()
finally:
reset_hermes_home_override(token_a)
assert "hermes_plugins.stateful_plugin.state" in sys.modules
assert sys.modules["hermes_plugins.stateful_plugin.state"].MARKER == "marker-a"
token_b = set_hermes_home_override(str(home_b))
try:
manager_b = plugins_mod.get_plugin_manager()
manager_b.discover_and_load()
finally:
reset_hermes_home_override(token_b)
# The submodule cached under sys.modules must now reflect profile
# b's code, not a leftover from profile a.
assert sys.modules["hermes_plugins.stateful_plugin.state"].MARKER == "marker-b"
assert (
manager_b._plugin_skills["stateful-plugin::marker"]["marker"] == "marker-b"
)
assert manager_a is not manager_b