fix(integrations): prevent Hermes bm mcp process leaks

integrations/hermes/__init__.py

@@ -817,6 +817,17 @@ def is_available(self) -> bool:
 
     # ---- Lifecycle ----
     def initialize(self, session_id: str, **kwargs: Any) -> None:
+        # Hermes may initialize memory providers repeatedly in long-lived
+        # gateway/dashboard processes. Each initialization starts a bm MCP
+        # stdio child; if we overwrite _actor without shutting it down, those
+        # children accumulate and retain hundreds of MB each. Make re-init
+        # idempotent from a process-lifecycle perspective.
+        if self._actor is not None or self._initialized:
+            try:
+                self.shutdown()
+            except Exception as e:
+                logger.debug("basic-memory: shutdown before reinitialize failed: %s", e)
+
         self._session_id = session_id or ""
         self._hermes_home = kwargs.get("hermes_home") or os.path.expanduser("~/.hermes")
         cfg = _load_config(self._hermes_home)
@@ -955,6 +966,15 @@ def shutdown(self) -> None:
                 logger.debug("actor shutdown: %s", e)
         self._actor = None
         self._initialized = False
+        # Clear session-scoped state so the next initialize() starts fresh.
+        # Without this, reusing a provider singleton across sessions causes
+        # the first turn of the next session to append to the prior session's
+        # transcript (_session_note_id) and lose the opening user message
+        # (_first_user_msg), matching the Codex review on PR #1046.
+        self._session_note_id = None
+        self._first_user_msg = None
+        self._session_started_at = None
+        self._session_id = ""
 
     # ---- Tool surface ----
     def system_prompt_block(self) -> str:
@@ -1806,6 +1826,8 @@ def _register_via_plugin_manager(
 # ---------------------------------------------------------------------------
 
 _active_providers: list[BasicMemoryProvider] = []
+_provider_singleton: BasicMemoryProvider | None = None
+_provider_singleton_lock = threading.Lock()
 
 
 def _atexit_cleanup() -> None:
@@ -1825,9 +1847,22 @@ def _atexit_cleanup() -> None:
 
 
 def register(ctx: Any) -> None:
-    """Register basic-memory as a memory provider plugin."""
-    provider = BasicMemoryProvider()
-    _active_providers.append(provider)
+    """Register basic-memory as a memory provider plugin.
+
+    Hermes can call plugin register() more than once inside the same
+    long-lived process (gateway/dashboard command discovery, session startup,
+    cron runs). A fresh provider owns a fresh bm MCP subprocess, so creating
+    a new provider on every register leaks bm mcp children until process exit.
+    Reuse one provider per process and let initialize() refresh its session
+    state.
+    """
+    global _provider_singleton
+    with _provider_singleton_lock:
+        if _provider_singleton is None:
+            _provider_singleton = BasicMemoryProvider()
+            _active_providers.append(_provider_singleton)
+        provider = _provider_singleton
+
     ctx.register_memory_provider(provider)
 
     # Bundle the user-facing skill so `hermes plugins install` wires it up

integrations/hermes/tests/conftest.py

@@ -58,6 +58,20 @@ def bm():
     return _plugin
 
 
+@pytest.fixture(autouse=True)
+def reset_plugin_lifecycle_state():
+    """Keep module-level provider singletons isolated between tests."""
+    if hasattr(_plugin, "_active_providers"):
+        _plugin._active_providers.clear()
+    if hasattr(_plugin, "_provider_singleton"):
+        setattr(_plugin, "_provider_singleton", None)
+    yield
+    if hasattr(_plugin, "_active_providers"):
+        _plugin._active_providers.clear()
+    if hasattr(_plugin, "_provider_singleton"):
+        setattr(_plugin, "_provider_singleton", None)
+
+
 # ---- Synthetic MCP CallToolResult objects (no MCP SDK needed at test time) ----
 
 

integrations/hermes/tests/test_provider.py

@@ -3,7 +3,6 @@
 from __future__ import annotations
 
 import json
-from pathlib import Path
 from unittest.mock import MagicMock
 
 
@@ -107,7 +106,7 @@ def test_handle_tool_call_dispatches(bm):
     actor.call.return_value = json.dumps({"results": []})
     p._actor = actor
 
-    out = p.handle_tool_call("bm_search", {"query": "hi", "limit": 3})
+    p.handle_tool_call("bm_search", {"query": "hi", "limit": 3})
     actor.call.assert_called_once()
     bm_tool, bm_args = actor.call.call_args[0][:2]
     assert bm_tool == "search_notes"
@@ -269,13 +268,108 @@ def test_get_config_schema_shape(bm):
     }.issubset(keys)
 
 
-def test_register_appends_to_active_providers(bm):
-    fake_ctx = MagicMock()
-    bm._active_providers.clear()
-    bm.register(fake_ctx)
-    fake_ctx.register_memory_provider.assert_called_once()
-    assert len(bm._active_providers) == 1
-    bm._active_providers.clear()
+def test_register_reuses_single_provider_per_process(bm):
+    """Repeated plugin registration must not create providers that each own a bm mcp child."""
+    first_ctx = MagicMock()
+    second_ctx = MagicMock()
+
+    bm.register(first_ctx)
+    bm.register(second_ctx)
+
+    first_provider = first_ctx.register_memory_provider.call_args.args[0]
+    second_provider = second_ctx.register_memory_provider.call_args.args[0]
+    assert first_provider is second_provider
+    assert bm._provider_singleton is first_provider
+    assert bm._active_providers == [first_provider]
+
+
+def test_initialize_shuts_down_existing_actor_before_reinitializing(bm, monkeypatch, tmp_path):
+    """Repeated initialize() calls must not orphan the previous bm mcp actor."""
+    actors = []
+
+    class _FakeActor:
+        def __init__(self, argv):
+            self.argv = argv
+            self.shutdown_calls = 0
+            actors.append(self)
+
+        def start(self, timeout=25.0):
+            return None
+
+        def shutdown(self, timeout=5.0):
+            self.shutdown_calls += 1
+
+        def list_tools(self):
+            return [{"name": name} for name in bm._HERMES_TO_BM.values()]
+
+    monkeypatch.setattr(bm, "_MCP_AVAILABLE", True)
+    monkeypatch.setattr(bm, "_bm_binary_path", lambda: "/fake/bm")
+    monkeypatch.setattr(bm, "_BmMcpActor", _FakeActor)
+    monkeypatch.setattr(bm.BasicMemoryProvider, "_ensure_local_project", lambda self: None)
+    monkeypatch.setattr(bm.BasicMemoryProvider, "_verify_project_registered", lambda self: True)
+    monkeypatch.setattr(bm.BasicMemoryProvider, "_server_argv", lambda self: ["/fake/bm", "mcp"])
+
+    p = bm.BasicMemoryProvider()
+    p.initialize(session_id="first", hermes_home=str(tmp_path))
+    first_actor = p._actor
+    p.initialize(session_id="second", hermes_home=str(tmp_path))
+
+    assert len(actors) == 2
+    assert first_actor is actors[0]
+    assert actors[0].shutdown_calls == 1
+    assert p._actor is actors[1]
+    assert p._initialized is True
+
+
+def test_shutdown_clears_session_scoped_state(bm, monkeypatch, tmp_path):
+    """shutdown() must clear _session_note_id, _first_user_msg, and
+    _session_id so the next initialize() does not append turns into the
+    previous session's transcript.
+
+    Regression test for the Codex review finding on PR #1046:
+    without this, reusing a provider singleton causes the next session
+    to write into the prior session note and lose its opening message.
+    """
+
+    class _FakeActor:
+        def __init__(self, argv):
+            self.argv = argv
+
+        def start(self, timeout=25.0):
+            return None
+
+        def shutdown(self, timeout=5.0):
+            pass
+
+        def list_tools(self):
+            return [{"name": name} for name in bm._HERMES_TO_BM.values()]
+
+    monkeypatch.setattr(bm, "_MCP_AVAILABLE", True)
+    monkeypatch.setattr(bm, "_bm_binary_path", lambda: "/fake/bm")
+    monkeypatch.setattr(bm, "_BmMcpActor", _FakeActor)
+    monkeypatch.setattr(bm.BasicMemoryProvider, "_ensure_local_project", lambda self: None)
+    monkeypatch.setattr(bm.BasicMemoryProvider, "_verify_project_registered", lambda self: True)
+    monkeypatch.setattr(bm.BasicMemoryProvider, "_server_argv", lambda self: ["/fake/bm", "mcp"])
+
+    p = bm.BasicMemoryProvider()
+    p.initialize(session_id="first", hermes_home=str(tmp_path))
+
+    # Set session-scoped state (as would happen during a real session)
+    p._session_note_id = "test/hermes-sessions/session-old"
+    p._first_user_msg = "old first message"
+    p._session_started_at = None  # doesn't matter — shutdown clears the important fields
+    p._session_id = "first"
+
+    p.shutdown()
+
+    # After shutdown, session-scoped state must be cleared
+    assert p._session_note_id is None, (
+        "_session_note_id not cleared — next session would append, not create"
+    )
+    assert p._first_user_msg is None, (
+        "_first_user_msg not cleared — next session would lose its opening message"
+    )
+    assert p._session_id == "", "_session_id not cleared"
 
 
 def test_register_also_registers_bundled_skill(bm):