From 289eb84501a761958fd8723707493ccd61154e50 Mon Sep 17 00:00:00 2001 From: MemOS AutoDev Date: Fri, 10 Jul 2026 12:32:34 +0800 Subject: [PATCH 1/3] fix(memos-local-plugin): revert half-merged MemosHttpClient usages (#2096) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hermes adapter's __init__.py imports MemosHttpClient from bridge_client, but the class was never committed — every plugin python test fails at collection and any real Hermes host loading memos_provider hits ImportError at module load. Reverting the four usages and the two HTTP-first branches in initialize/_reconnect_bridge restores the adapter to its pre-half-merge stdio-only behavior. A dedicated test_module_imports_cleanly regression test asserts the module surface (MemTensorProvider / MemosBridgeClient / BridgeError present, MemosHttpClient absent) so this class of import-level breakage cannot silently return. Fixes #2096 --- .../hermes/memos_provider/__init__.py | 66 ++++--------------- .../python/test_hermes_provider_pipeline.py | 19 ++++++ 2 files changed, 31 insertions(+), 54 deletions(-) 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..3f88c9f30 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,25 @@ def request(self, method: str, params: dict | None = None, **_kwargs: object) -> class HermesProviderPipelineTests(unittest.TestCase): + def test_module_imports_cleanly(self) -> None: + """Regression for #2096: ``memos_provider`` must import without + pulling in symbols that don't exist in ``bridge_client``. + + The module is already loaded at the top of this file, so if any + dangling reference is reintroduced, ``import memos_provider`` + raises ``ImportError`` and the whole file fails to collect. + This test additionally asserts the surface Hermes actually reads: + the ``MemTensorProvider`` class and the ``BridgeError`` / + ``MemosBridgeClient`` names from ``bridge_client``. + """ + self.assertTrue(hasattr(memos_provider, "MemTensorProvider")) + self.assertTrue(hasattr(memos_provider, "MemosBridgeClient")) + self.assertTrue(hasattr(memos_provider, "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``. + self.assertFalse(hasattr(memos_provider, "MemosHttpClient")) + def test_lifecycle_persists_turn_and_closes_real_episode(self) -> None: bridge = FakeBridge() with ( From 6272765b80384817162c88d3ff13cca85fee6d01 Mon Sep 17 00:00:00 2001 From: MemOS AutoDev Date: Fri, 10 Jul 2026 12:43:33 +0800 Subject: [PATCH 2/3] test(memos-local-plugin): tighten module-import regression test per OCR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address two Open Code Review findings on the test_module_imports_cleanly regression test added in #2097: * The positive hasattr assertions on memos_provider.MemosBridgeClient and memos_provider.BridgeError were locking in an implementation detail — those names only appear on the memos_provider namespace because __init__.py uses a bare 'from bridge_client import ...'. If __init__.py ever switched to 'import bridge_client', the test would fail even though the module is correct. Assert the real contract on bridge_client directly via importlib. * The docstring claimed a role the test body does not uniquely fill: since test_module_imports_cleanly is the first test in the suite, a collection-level ImportError would already prevent every test from running. Rewrite the docstring to spell out that the only unique guard the body provides is the negative assertion that MemosHttpClient is absent (plus a positive MemTensorProvider check for the class Hermes actually instantiates). Refs #2096. --- .../python/test_hermes_provider_pipeline.py | 40 ++++++++++++++----- 1 file changed, 29 insertions(+), 11 deletions(-) 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 3f88c9f30..87c2ac00e 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 @@ -69,19 +69,37 @@ def request(self, method: str, params: dict | None = None, **_kwargs: object) -> class HermesProviderPipelineTests(unittest.TestCase): def test_module_imports_cleanly(self) -> None: - """Regression for #2096: ``memos_provider`` must import without - pulling in symbols that don't exist in ``bridge_client``. - - The module is already loaded at the top of this file, so if any - dangling reference is reintroduced, ``import memos_provider`` - raises ``ImportError`` and the whole file fails to collect. - This test additionally asserts the surface Hermes actually reads: - the ``MemTensorProvider`` class and the ``BridgeError`` / - ``MemosBridgeClient`` names from ``bridge_client``. + """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), 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")) - self.assertTrue(hasattr(memos_provider, "MemosBridgeClient")) - self.assertTrue(hasattr(memos_provider, "BridgeError")) + + # 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``. From 4e7f07c00960c064da3e2cecad4d5ad7b0b49f2e Mon Sep 17 00:00:00 2001 From: MemOS AutoDev Date: Fri, 10 Jul 2026 12:49:25 +0800 Subject: [PATCH 3/3] test(memos-local-plugin): also guard MemosHttpClient absence on bridge_client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2 OCR fix on PR #2097. The prior negative guard only checked memos_provider — but memos_provider only sees MemosHttpClient through a bare 'from bridge_client import ...' in __init__.py. If the class were re-added to bridge_client without a matching re-export (e.g. an aliased import, or used internally), the assertion would still pass and the half-merged feature could silently re-enter the codebase. Add the complementary assertFalse(hasattr(bc, 'MemosHttpClient')) so the class cannot land in bridge_client at all until it is intentionally committed as a complete feature. Update the docstring to note the guard now covers both surfaces. Refs #2096. --- .../tests/python/test_hermes_provider_pipeline.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) 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 87c2ac00e..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 @@ -79,7 +79,10 @@ def test_module_imports_cleanly(self) -> None: the entire test file to fail to load. This test body only adds: * the explicit negative guard on ``MemosHttpClient`` below (unique - to this test), and + 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 @@ -102,8 +105,13 @@ def test_module_imports_cleanly(self) -> None: # ``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``. + # 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()