diff --git a/apps/memos-local-plugin/adapters/hermes/memos_provider/__init__.py b/apps/memos-local-plugin/adapters/hermes/memos_provider/__init__.py index 25546dfff..47392b8a7 100644 --- a/apps/memos-local-plugin/adapters/hermes/memos_provider/__init__.py +++ b/apps/memos-local-plugin/adapters/hermes/memos_provider/__init__.py @@ -65,13 +65,11 @@ if str(_PLUGIN_DIR) not in sys.path: sys.path.insert(0, str(_PLUGIN_DIR)) -from bridge_client import BridgeError, MemosBridgeClient, MemosHttpClient # noqa: E402 +from bridge_client import BridgeError, MemosBridgeClient # noqa: E402 from daemon_manager import ( # noqa: E402 ensure_bridge_running, ensure_viewer_daemon, kill_zombie_bridges, - probe_viewer_status, - startup_lock_active, ) @@ -279,7 +277,7 @@ class MemTensorProvider(MemoryProvider): """ def __init__(self) -> None: - self._bridge: MemosBridgeClient | MemosHttpClient | None = None + self._bridge: MemosBridgeClient | None = None self._reconnect_lock = threading.Lock() self._session_id: str = "" self._episode_id: str = "" @@ -329,23 +327,6 @@ def is_available(self) -> bool: # type: ignore[override] # ─── Lifecycle ──────────────────────────────────────────────────────── - def _connect_http_bridge(self, session_id: str, *, timeout: float = 60.0) -> bool: - """Try to connect via HTTP bridge. Sets self._bridge on success.""" - http_bridge: MemosHttpClient | None = None - try: - http_bridge = MemosHttpClient() - http_bridge.register_host_handler("host.llm.complete", self._handle_host_llm_complete) - self._bridge = http_bridge - self._open_session(session_id, timeout=timeout) - return True - except Exception as err: - logger.warning("MemOS: HTTP bridge failed, falling back to stdio — %s", err) - if http_bridge is not None: - with contextlib.suppress(Exception): - http_bridge.close() - self._bridge = None - return False - def initialize(self, session_id: str, **kwargs: Any) -> None: # type: ignore[override] """Called once at agent startup. @@ -414,32 +395,13 @@ def initialize(self, session_id: str, **kwargs: Any) -> None: # type: ignore[ov except Exception: pass - # If the daemon is already running on the viewer port, connect - # to it over HTTP instead of spawning a new stdio bridge. This - # eliminates zombie bridge accumulation. - viewer_status = probe_viewer_status() - if viewer_status == "running_memos": - if self._connect_http_bridge(session_id): - logger.info( - "MemOS: bridge ready (HTTP) session=%s platform=%s (episode deferred)", - self._session_id, - self._platform, - ) - else: - viewer_status = "free" # force stdio fallback below - elif viewer_status == "free": - # Re-probe after a short wait only when another process may be - # mid-startup (startup lock is held). On a cold first-launch the - # lock doesn't exist, so we skip the delay entirely. - if startup_lock_active(): - time.sleep(1.0) - viewer_status = probe_viewer_status() - if viewer_status == "running_memos" and self._connect_http_bridge(session_id): - logger.info( - "MemOS: bridge ready (HTTP, late probe) session=%s platform=%s (episode deferred)", - self._session_id, - self._platform, - ) + # NOTE: An HTTP bridge path used to live here that connected to a + # running viewer daemon over HTTP instead of spawning a stdio + # subprocess. It depended on ``MemosHttpClient`` which was never + # committed — the class was referenced by name only. Issue #2096 + # reverts the half-merged HTTP feature; the stdio path below is + # the sole connection mechanism until the HTTP client lands as a + # complete change. if self._bridge is None: try: @@ -1958,13 +1920,9 @@ def _reconnect_bridge(self, session_id: str = "", *, timeout: float = 30.0) -> N logger.info("MemOS: old bridge closed (pid=%s)", old_pid) ensure_bridge_running() - # Try HTTP first if daemon is running - viewer_status = probe_viewer_status() - if viewer_status == "running_memos" and self._connect_http_bridge( - session_id, timeout=timeout - ): - logger.info("MemOS: reconnected via HTTP") - return + # NOTE: HTTP bridge reconnect path was removed alongside issue + # #2096. See ``initialize`` for the rationale. Reconnect always + # spawns a fresh stdio bridge. try: ensure_viewer_daemon() diff --git a/apps/memos-local-plugin/tests/python/test_hermes_provider_pipeline.py b/apps/memos-local-plugin/tests/python/test_hermes_provider_pipeline.py index 1c8cb6a6c..71e3b347b 100644 --- a/apps/memos-local-plugin/tests/python/test_hermes_provider_pipeline.py +++ b/apps/memos-local-plugin/tests/python/test_hermes_provider_pipeline.py @@ -68,6 +68,51 @@ def request(self, method: str, params: dict | None = None, **_kwargs: object) -> class HermesProviderPipelineTests(unittest.TestCase): + def test_module_imports_cleanly(self) -> None: + """Regression guard for #2096: asserts that ``MemosHttpClient`` is + NOT present in ``memos_provider``, since the class was referenced + before it was ever committed (see issue #2096). + + Note: the import itself is already validated at collection time — + the ``import memos_provider`` at the top of this file will raise + ``ImportError`` if a dangling reference is reintroduced, causing + the entire test file to fail to load. This test body only adds: + + * the explicit negative guard on ``MemosHttpClient`` below (unique + to this test), which covers both the ``memos_provider`` + re-export surface *and* ``bridge_client`` itself so a partial + re-add of the class only in ``bridge_client`` (with no + matching re-export) still fails the guard, and + * positive checks on ``MemTensorProvider`` (the class the Hermes + host actually instantiates) and on ``bridge_client``'s real + contract (``MemosBridgeClient`` / ``BridgeError``), rather than + on their incidental re-exports through ``memos_provider`` — the + latter only appear on the package namespace because + ``__init__.py`` uses a bare ``from bridge_client import ...``, + which is an implementation detail we don't want the test to + lock in. + """ + import importlib + + # MemTensorProvider is the class hermes-agent host instantiates. + self.assertTrue(hasattr(memos_provider, "MemTensorProvider")) + + # Assert the actual contract on bridge_client directly rather + # than on its re-exports through memos_provider. + bc = importlib.import_module("bridge_client") + self.assertTrue(hasattr(bc, "MemosBridgeClient")) + self.assertTrue(hasattr(bc, "BridgeError")) + + # ``MemosHttpClient`` was referenced by name in a half-merged HTTP + # bridge feature (see #2096). It must not reappear until the class + # itself is committed in ``bridge_client``. Guard both the + # ``memos_provider`` re-export (which is what the original + # ImportError travelled through) and ``bridge_client`` itself — + # otherwise a partial re-add of the class in ``bridge_client`` + # without a matching re-export would slip past this test. + self.assertFalse(hasattr(memos_provider, "MemosHttpClient")) + self.assertFalse(hasattr(bc, "MemosHttpClient")) + def test_lifecycle_persists_turn_and_closes_real_episode(self) -> None: bridge = FakeBridge() with (