Skip to content

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

Open
terry197913 wants to merge 1 commit into
NousResearch:mainfrom
terry197913:luna/pr-28109-profile-isolation-fix
Open

fix: scope plugin manager by resolved hermes home (keyed cache)#63702
terry197913 wants to merge 1 commit into
NousResearch:mainfrom
terry197913:luna/pr-28109-profile-isolation-fix

Conversation

@terry197913

Copy link
Copy Markdown
Contributor

Summary

Fix plugin manager cache to be scoped by resolved HERMES_HOME path, preventing cross-profile plugin state bleed in multi-profile VPS environments.

Root cause: Plugin manager was a process-global singleton. When multiple Hermes profiles (Luna / Apollo / Hephaestus) ran on the same VPS, the first profile to load a plugin (e.g. hermes-lcm context engine) cached its instance. Subsequent profiles then received the wrong plugin instance, leading to incorrect lcm.db and other profile-specific state being shared.

Fix (3 parts):

  1. _plugin_home_key() + keyed _plugin_managers_by_home cache (ContextVar-aware) — multi-profile gateway isolation
  2. _make_plugin_dir() test helper now accepts home= for per-profile isolation
  3. _clear_plugin_submodules() evicts hermes_plugins.{slug}.* from sys.modules on reload and import failure

Adds regression tests:

  • test_profile_isolation_separate_managers_and_state
  • test_force_rediscover_clears_all_caches

Verification: 118 plugin tests pass.


Opened by Luna (kanban-assigned Hephaestus task). Branch: luna/pr-28109-profile-isolation-fix on terry197913/hermes-agent.

fix: remove .codegraph artifacts from commit
@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 13, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for identifying a real multi-profile seam: current main's gateway scopes each multiplexed turn with set_hermes_home_override() (gateway/run.py:1441-1472), while hermes_cli/plugins.py:2029-2037 still has a single manager.

Problems

  • The proposed cache does not isolate plugin registrations. PluginContext.register_tool() writes to the process-global tools.registry.registry (hermes_cli/plugins.py:428-442), which holds one entry per name (tools/registry.py:389-448); register_platform() also writes to the global platform registry (hermes_cli/plugins.py:963-978, gateway/platform_registry.py:231-248). Loading profile B can therefore replace or block profile A registrations despite separate managers.
  • The added regressions assert only context-engine/private-skill state (tests/hermes_cli/test_plugins.py:2074-2087, 2151-2157). They do not cover global tool/platform dispatch or return from B to A after module eviction.

Suggested changes

  • Scope or activate the registration lifecycle along with the manager, then add A→B→A integration coverage for same-named profile-local tools and platforms plus a runtime relative import.

Automated hermes-sweeper review.

Comment thread hermes_cli/plugins.py
# 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] = {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cache isolates only PluginManager-owned fields. PluginContext.register_tool() still mutates the process-global tools.registry.registry, and register_platform() mutates the global platform registry. Loading profile B can therefore overwrite or reject profile A registrations even when the managers differ; the registration lifecycle must be profile-safe too.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This proves distinct manager/engine objects only. Please add an A→B→A regression that registers and dispatches profile-specific tools/platforms through their real global registries; otherwise the remaining cross-profile registration bleed is untested.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants