From c4de26b0fd46cfbc1ad079f7e615178c13099e22 Mon Sep 17 00:00:00 2001 From: Brett Grane <24548810+scarabone@users.noreply.github.com> Date: Sun, 30 Aug 2026 20:42:22 -0400 Subject: [PATCH 1/3] feat(hermes): support existing Basic Memory HTTP daemon Signed-off-by: Brett Grane <24548810+scarabone@users.noreply.github.com> --- integrations/hermes/CHANGELOG.md | 10 + integrations/hermes/README.md | 36 +- integrations/hermes/__init__.py | 472 ++++++++++++++---- .../hermes/tests/test_http_integration.py | 135 +++++ .../hermes/tests/test_http_provider.py | 293 +++++++++++ integrations/hermes/tests/test_http_review.py | 387 ++++++++++++++ 6 files changed, 1238 insertions(+), 95 deletions(-) create mode 100644 integrations/hermes/tests/test_http_integration.py create mode 100644 integrations/hermes/tests/test_http_provider.py create mode 100644 integrations/hermes/tests/test_http_review.py diff --git a/integrations/hermes/CHANGELOG.md b/integrations/hermes/CHANGELOG.md index 375499adb..ed67b6e95 100644 --- a/integrations/hermes/CHANGELOG.md +++ b/integrations/hermes/CHANGELOG.md @@ -4,6 +4,16 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Added +- **Existing Streamable HTTP server mode.** Set `server_url` to connect the + provider to a running Basic Memory MCP endpoint (for example, + `http://127.0.0.1:8766/mcp`) without installing/spawning `bm` or creating a + local project. Existing tool, prefetch, capture, cleanup, reconnect, and + per-call project-routing behavior is retained. A reconnect retries only + read/idempotent operations; ambiguous note mutations are never replayed. + ## [0.3.2] — 2026-05-23 ### Fixed diff --git a/integrations/hermes/README.md b/integrations/hermes/README.md index 727329982..6d014814e 100644 --- a/integrations/hermes/README.md +++ b/integrations/hermes/README.md @@ -13,7 +13,7 @@ hermes plugins install basicmachines-co/basic-memory/integrations/hermes ``` Hermes does not install a plugin's Python dependencies (it only prints them), so put the -`mcp` package into the Hermes venv yourself: +`mcp>=2,<3` package into the Hermes venv yourself: ```bash uv pip install --python ~/.hermes/hermes-agent/venv/bin/python "mcp>=2,<3" @@ -28,7 +28,32 @@ memory: If you run the gateway, restart it (`hermes gateway restart`). Done. -The plugin self-installs the `basic-memory` CLI on first init via `uv tool install basic-memory --prerelease=allow` (one-time ~10s pause if it isn't already present). The bm binary lands at `~/.local/bin/bm` — the same location a manual `uv tool install basic-memory` would produce, so a later manual install or upgrade is a no-op rather than a second install. +In the default local/cloud modes, the plugin self-installs the `basic-memory` CLI on first init via `uv tool install basic-memory --prerelease=allow` (one-time ~10s pause if it isn't already present). The bm binary lands at `~/.local/bin/bm` — the same location a manual `uv tool install basic-memory` would produce, so a later manual install or upgrade is a no-op rather than a second install. + +### Use an existing Basic Memory MCP server + +If Basic Memory is already running as a shared MCP Streamable HTTP server, set +`server_url` in `~/.hermes/basic-memory.json`. This mode connects directly to +that endpoint and does not install or spawn `bm`, create a project, or modify +Basic Memory's project registry. An explicit non-empty `project` is required +in this mode (the provider fails closed with an actionable error if omitted), +and is sent on each read/write call. Optional per-call `project` / `project_id` +overrides continue to work. + +```json +{ + "server_url": "http://127.0.0.1:8766/mcp", + "project": "main", + "capture_per_turn": true, + "capture_session_end": true, + "capture_folder": "hermes-sessions" +} +``` + +`server_url` takes precedence over `mode`; omit it to retain the existing +local/cloud behavior and CLI bootstrap. If an HTTP session drops, the provider +reconnects once and retries only read/idempotent operations; an ambiguous note +write or mutation is not replayed, so its original failure is surfaced. ### Prerequisites @@ -38,7 +63,7 @@ The plugin self-installs the `basic-memory` CLI on first init via `uv tool insta builds may lack repository-subdirectory plugin sources, and the `/bm-*` slash commands need Hermes ≥ v0.11.0 - [`uv`](https://docs.astral.sh/uv/) on PATH (used for the bootstrap install) -- The `mcp` Python package in the Hermes venv. Hermes never installs plugin Python +- The `mcp>=2,<3` Python package in the Hermes venv. Hermes never installs plugin Python dependencies — it prints the ones declared in `plugin.yaml` — so run: ```bash uv pip install --python ~/.hermes/hermes-agent/venv/bin/python "mcp>=2,<3" @@ -136,7 +161,8 @@ Defaults are reasonable for local use: | Key | Default | Notes | |---|---|---| | `mode` | `local` | `local` (in-process) or `cloud` (route through BM Cloud API) | -| `project` | `hermes-memory` | BM project name | +| `server_url` | empty | Existing Basic Memory MCP Streamable HTTP endpoint. When set, connects directly and skips CLI/project setup. | +| `project` | `hermes-memory` (local/cloud); required with `server_url` | BM project name | | `project_path` | `~/hermes-memory/` | Local mode only — where session notes land | | `capture_folder` | `hermes-sessions` | Folder within the project for session notes | | `capture_per_turn` | `true` | Append every turn to a session transcript | @@ -238,7 +264,7 @@ BM_INTEGRATION=1 uv run --with pytest --with mcp pytest tests/test_integration.p The unit suite stubs out Hermes-internal imports (`agent.memory_provider`, `tools.registry`) so it runs without a Hermes install. `mcp` is optional at unit-test time — its absence just makes `is_available()` return False, which the tests verify. -Integration tests require `BM_INTEGRATION=1`, `bm` CLI on PATH, and `mcp` Python package importable. Each session creates a unique throwaway BM project (under `tempfile.mkdtemp`) and removes it on teardown, so they never touch your real BM projects. +Integration tests require `BM_INTEGRATION=1`, `bm` CLI on PATH, and `mcp>=2,<3` importable. Each session creates a unique throwaway BM project (under `tempfile.mkdtemp`) and removes it on teardown, so they never touch your real BM projects. ## License diff --git a/integrations/hermes/__init__.py b/integrations/hermes/__init__.py index 11e273639..2236830b4 100644 --- a/integrations/hermes/__init__.py +++ b/integrations/hermes/__init__.py @@ -1,8 +1,9 @@ """ basic-memory — Hermes Memory Provider plugin -Wraps the basic-memory MCP server (`bm mcp`) to provide knowledge-graph-backed -memory for Hermes. Analog of openclaw-basic-memory. +Wraps the basic-memory MCP server to provide knowledge-graph-backed memory for +Hermes. By default the server is `bm mcp`; `server_url` can instead attach to +an existing Streamable HTTP daemon. Analog of openclaw-basic-memory. Architecture: - `_BmMcpActor` owns a long-lived asyncio loop in a daemon thread that holds @@ -30,15 +31,25 @@ import threading import time from datetime import datetime, timezone +from ipaddress import ip_address from pathlib import Path from shutil import which from typing import Any, Callable +from urllib.parse import urlsplit # Hermes ABC + helpers — these resolve because Hermes adds its tree to sys.path # when loading plugins (same pattern as plugins/memory/mem0/__init__.py:21). from agent.memory_provider import MemoryProvider from tools.registry import tool_error +try: + # Hermes keeps profile/home resolution in this dependency-free module. It + # is optional here because the provider's unit tests intentionally load + # without a Hermes checkout on sys.path. + from hermes_constants import get_hermes_home # type: ignore +except Exception: # pragma: no cover - exercised only outside Hermes + get_hermes_home = None # type: ignore[assignment] + __version__ = "0.23.2" logger = logging.getLogger("hermes.memory.basic-memory") @@ -50,10 +61,21 @@ _MCP_AVAILABLE = False _MCP_IMPORT_ERROR: BaseException | None = None +streamable_http_client: Any = None try: from mcp import ClientSession, StdioServerParameters # type: ignore from mcp.client.stdio import stdio_client # type: ignore + # Streamable HTTP was added after the original stdio-only provider. Keep + # this import soft so old MCP SDKs continue to support local/cloud mode. + # The released MCP 2.x API is ``streamable_http_client`` (with an + # underscore); its context manager yields the read/write streams used by + # ClientSession. + try: + from mcp.client.streamable_http import streamable_http_client # type: ignore + except ImportError: + streamable_http_client = None + _MCP_AVAILABLE = True except Exception as _e: # pragma: no cover _MCP_IMPORT_ERROR = _e @@ -79,6 +101,20 @@ "bm_workspaces": "list_workspaces", } +# Only these operations are safe to replay after an ambiguous transport +# failure. Writes and note mutations may have reached the server before the +# connection dropped, so they are deliberately never retried. +_HTTP_RETRYABLE_TOOLS: frozenset[str] = frozenset( + { + "search_notes", + "read_note", + "build_context", + "recent_activity", + "list_memory_projects", + "list_workspaces", + } +) + # Discovery tools that operate across all projects/workspaces. They don't # accept project/project_id args (no per-call routing) and the user-facing # schemas omit those properties. @@ -425,6 +461,59 @@ def _config_path(hermes_home: str) -> Path: return Path(hermes_home) / "basic-memory.json" +def _active_hermes_home() -> Path: + """Resolve the active Hermes home/profile without assuming ``~/.hermes``.""" + if get_hermes_home is not None: + try: + return Path(get_hermes_home()).expanduser() + except Exception as e: + logger.debug("basic-memory: Hermes home resolver unavailable: %s", e) + configured = os.environ.get("HERMES_HOME", "").strip() + return Path(configured or "~/.hermes").expanduser() + + +def _validate_server_url(value: Any) -> str | None: + """Validate and normalize an MCP Streamable HTTP endpoint. + + URL credentials, query strings, and fragments are intentionally rejected: + they are unnecessary for the daemon endpoint and could leak through status + or error output. Non-loopback HTTP is rejected so a remote endpoint uses + TLS; loopback HTTP is the intended local-daemon configuration. + """ + if value is None: + return None + if not isinstance(value, str): + raise ValueError("server_url must be an http:// or https:// URL") + url = value.strip() + if not url: + return None + try: + parsed = urlsplit(url) + hostname = parsed.hostname + # Accessing .port validates malformed port values (e.g. ':abc'). + _ = parsed.port + except ValueError: + raise ValueError("server_url is malformed") from None + if parsed.scheme.lower() not in {"http", "https"}: + raise ValueError("server_url must use http:// or https://") + if not hostname: + raise ValueError("server_url must include a hostname") + if parsed.username is not None or parsed.password is not None: + raise ValueError("server_url must not contain username or password") + if parsed.query or parsed.fragment or "?" in url or "#" in url: + raise ValueError("server_url must not contain a query or fragment") + host = hostname.lower().rstrip(".") + is_loopback = host == "localhost" or host.endswith(".localhost") + if not is_loopback: + try: + is_loopback = ip_address(host).is_loopback + except ValueError: + pass + if parsed.scheme.lower() == "http" and not is_loopback: + raise ValueError("non-loopback server_url endpoints must use https://") + return url + + def _bm_config_path() -> Path: """Location of bm's own project registry.""" return Path.home() / ".basic-memory" / "config.json" @@ -593,8 +682,17 @@ class _BmMcpActor: daemon thread and ferry calls in via run_coroutine_threadsafe. """ - def __init__(self, server_argv: list[str], env: dict[str, str] | None = None): - self._server_argv = list(server_argv) + def __init__( + self, + server_argv: list[str] | None = None, + env: dict[str, str] | None = None, + *, + server_url: str | None = None, + ): + if not server_argv and not server_url: + raise ValueError("either server_argv or server_url is required") + self._server_argv = list(server_argv or []) + self._server_url = server_url # Sanitized so `bm mcp` resolves imports against its own interpreter, # not Hermes's — see _clean_child_env. self._env = _clean_child_env(env) @@ -648,41 +746,63 @@ def _run(self) -> None: logger.exception("basic-memory MCP actor terminated with error") finally: self._running = False + # A transport reconnect must never expose the prior MCP session + # object (and its server-assigned session ID) to a later call. + self._session = None self._main_task = None try: loop.close() except Exception: pass + async def _run_session(self, read: Any, write: Any) -> None: + """Run one ClientSession over either stdio or Streamable HTTP.""" + async with ClientSession(read, write) as session: + await session.initialize() + self._session = session + self._stop_future = asyncio.get_running_loop().create_future() + try: + listing = await session.list_tools() + # Same unpinned-`mcp`-SDK reason as parse_result above: + # ListToolsResult / Tool field shapes vary across SDK versions, + # so read them defensively rather than by direct attribute access. + self._tools_cache = [ + { + "name": getattr(t, "name", ""), + "description": getattr(t, "description", "") or "", + } + for t in getattr(listing, "tools", []) or [] + ] + except Exception as e: + logger.warning("list_tools failed: %s", e) + self._tools_cache = [] + self._ready.set() + await self._stop_future # blocks until shutdown + async def _main(self) -> None: - params = StdioServerParameters( - command=self._server_argv[0], - args=self._server_argv[1:], - env=self._env, - ) try: - async with stdio_client(params) as (read, write): - async with ClientSession(read, write) as session: - await session.initialize() - self._session = session - self._stop_future = asyncio.get_running_loop().create_future() - try: - listing = await session.list_tools() - # Same unpinned-`mcp`-SDK reason as parse_result above: - # ListToolsResult / Tool field shapes vary across SDK versions, - # so read them defensively rather than by direct attribute access. - self._tools_cache = [ - { - "name": getattr(t, "name", ""), - "description": getattr(t, "description", "") or "", - } - for t in getattr(listing, "tools", []) or [] - ] - except Exception as e: - logger.warning("list_tools failed: %s", e) - self._tools_cache = [] - self._ready.set() - await self._stop_future # blocks until shutdown + if self._server_url: + if streamable_http_client is None: + raise RuntimeError( + "MCP SDK does not provide streamable_http_client; " + "install mcp>=1.24 or use local/cloud mode" + ) + # MCP 1.x yielded (read, write, get_session_id), while MCP + # 2.x yields (read, write). Only the first two are consumed. + async with streamable_http_client(self._server_url) as streams: + if len(streams) < 2: + raise RuntimeError( + "streamable_http_client returned fewer than two streams" + ) + await self._run_session(streams[0], streams[1]) + else: + params = StdioServerParameters( + command=self._server_argv[0], + args=self._server_argv[1:], + env=self._env, + ) + async with stdio_client(params) as (read, write): + await self._run_session(read, write) except BaseException as e: if self._init_error is None: self._init_error = e @@ -862,6 +982,7 @@ def __init__(self) -> None: self._actor: _BmMcpActor | None = None self._project: str = _default_project() self._mode: str = "local" + self._server_url: str | None = None self._project_path: str = _default_project_path() self._capture_per_turn: bool = True self._capture_session_end: bool = True @@ -879,6 +1000,10 @@ def __init__(self) -> None: self._failure_pause_until: float = 0.0 self._initialized: bool = False self._first_user_msg: str | None = None + # Serializes HTTP actor replacement when concurrent prefetch/capture + # threads observe the same dead transport. Normal calls do not hold + # this lock while waiting on MCP, so independent calls remain safe. + self._actor_lock = threading.RLock() # ---- Identity ---- @property @@ -887,10 +1012,18 @@ def name(self) -> str: def is_available(self) -> bool: # Discovery hot path. NEVER make network calls or spawn subprocesses here. + # A configured Streamable HTTP endpoint needs neither the bm CLI nor uv. # We report available when either bm is present already OR uv is present # (we bootstrap-install bm via `uv tool install` at initialize() time). if not _MCP_AVAILABLE: return False + configured = _load_config(str(_active_hermes_home())).get("server_url") + if isinstance(configured, str) and configured.strip(): + try: + _validate_server_url(configured) + except ValueError: + return False + return streamable_http_client is not None if _bm_binary_path(): return True if _uv_binary_path(): @@ -900,10 +1033,34 @@ def is_available(self) -> bool: # ---- Lifecycle ---- def initialize(self, session_id: str, **kwargs: Any) -> None: requested_session_id = session_id or "" - self._hermes_home = kwargs.get("hermes_home") or os.path.expanduser("~/.hermes") + self._hermes_home = kwargs.get("hermes_home") or str(_active_hermes_home()) cfg = _load_config(self._hermes_home) - self._mode = cfg.get("mode") or "local" - self._project = cfg.get("project") or _default_project() + try: + self._server_url = _validate_server_url(cfg.get("server_url")) + except ValueError as e: + # Do not include the configured value: it may have contained + # credentials before validation rejected it. + self._server_url = None + self._stop_actor() + self._reset_session_state() + logger.error("basic-memory: invalid server_url: %s", e) + return + # server_url is an explicit transport mode. Keep mode local/cloud + # behavior exactly as it was when this key is absent. + self._mode = "server_url" if self._server_url else (cfg.get("mode") or "local") + configured_project = cfg.get("project") + if self._server_url: + if not isinstance(configured_project, str) or not configured_project.strip(): + self._stop_actor() + self._reset_session_state() + logger.error( + "basic-memory: server_url mode requires an explicit non-empty " + "'project' (for example 'main'); provider will not initialize" + ) + return + self._project = configured_project.strip() + else: + self._project = configured_project or _default_project() self._project_path = os.path.expanduser(cfg.get("project_path") or _default_project_path()) self._capture_per_turn = bool(_coerce_bool(cfg.get("capture_per_turn", True))) self._capture_session_end = bool(_coerce_bool(cfg.get("capture_session_end", True))) @@ -917,46 +1074,65 @@ def initialize(self, session_id: str, **kwargs: Any) -> None: ) return - # Bootstrap-install bm via uv if it's not already on disk. One-time cost - # on a fresh machine; idempotent no-op once basic-memory is installed. - if not _bm_binary_path(): - if _uv_binary_path() is None: - logger.error( - "basic-memory: bm CLI not found and uv is not installed. " - "Install uv (https://docs.astral.sh/uv/) or run " - "`pip install basic-memory` manually. Provider will not initialize." - ) - return - logger.info( - "basic-memory: bm CLI not found — installing basic-memory via " - "`uv tool install` (one-time bootstrap)" - ) - if _install_bm_via_uv() is None: - logger.error( - "basic-memory: auto-install via uv failed. Run " - "`uv tool install basic-memory --prerelease=allow` manually to debug. " - "Provider will not initialize." + if not self._server_url: + # Bootstrap-install bm via uv if it's not already on disk. One-time + # cost on a fresh machine; idempotent no-op once installed. + if not _bm_binary_path(): + if _uv_binary_path() is None: + logger.error( + "basic-memory: bm CLI not found and uv is not installed. " + "Install uv (https://docs.astral.sh/uv/) or run " + "`pip install basic-memory` manually. Provider will not initialize." + ) + return + logger.info( + "basic-memory: bm CLI not found — installing basic-memory via " + "`uv tool install` (one-time bootstrap)" ) + if _install_bm_via_uv() is None: + logger.error( + "basic-memory: auto-install via uv failed. Run " + "`uv tool install basic-memory --prerelease=allow` manually to debug. " + "Provider will not initialize." + ) + return + + if self._mode == "local": + self._ensure_local_project() + + if not self._verify_project_registered(): + self._log_missing_project() return - if self._mode == "local": - self._ensure_local_project() - - if not self._verify_project_registered(): - self._log_missing_project() + if self._server_url and streamable_http_client is None: + self._stop_actor() + self._reset_session_state() + logger.error( + "basic-memory: server_url requires an MCP SDK with " + "streamable_http_client; provider will not initialize" + ) return # Trigger: initialize() called while a previous actor is still running # (Hermes re-initializes providers per session; _ensure_slash_ready's # lazy init can also precede a later session initialize). - # Why: every _BmMcpActor owns one `bm mcp` child process. Allocating a - # fresh actor without stopping the old one orphans that child — issue - # #1017 saw 18 idle `bm mcp` processes (~2.3 GB) accumulate this way. - # Outcome: a healthy actor is reused — its argv is always `bm mcp` and - # project routing happens per call, so the config re-read above never - # invalidates the connection. A dead actor is shut down first (joining - # its thread reaps the child) and then replaced below. - if self._actor is not None and self._actor.is_alive(): + # Why: every _BmMcpActor owns one transport (and local actors own one + # `bm mcp` child process). Allocating a fresh actor without stopping the + # old one can orphan that child — issue #1017 saw 18 idle `bm mcp` + # processes (~2.3 GB) accumulate this way. + # Outcome: a healthy actor is reused when its transport is unchanged; + # project routing happens per call, so the config re-read above does + # not invalidate the connection. A dead actor (or one whose transport + # changed between modes) is shut down first and then replaced below. + actor_transport_matches = False + if self._actor is not None: + actor_url = getattr(self._actor, "_server_url", None) + actor_transport_matches = ( + actor_url == self._server_url + if self._server_url + else actor_url is None + ) + if self._actor is not None and self._actor.is_alive() and actor_transport_matches: self._mark_session_ready(requested_session_id) logger.info( "basic-memory provider ready (reusing running MCP actor): mode=%s project=%s", @@ -972,7 +1148,7 @@ def initialize(self, session_id: str, **kwargs: Any) -> None: self._actor = None try: - argv = self._server_argv() + argv = [] if self._server_url else self._server_argv() except Exception as e: logger.error("basic-memory: cannot determine server argv: %s", e) return @@ -983,7 +1159,11 @@ def initialize(self, session_id: str, **kwargs: Any) -> None: # its freshly spawned `bm mcp` child if a signal lands mid-start. # Callers can't observe the half-started actor: every tool/capture # path gates on _initialized, which stays False until start() returns. - actor = _BmMcpActor(argv) + actor = ( + _BmMcpActor(argv, server_url=self._server_url) + if self._server_url + else _BmMcpActor(argv) + ) self._actor = actor try: actor.start(timeout=25.0) @@ -1017,9 +1197,10 @@ def initialize(self, session_id: str, **kwargs: Any) -> None: self._mark_session_ready(requested_session_id) logger.info( - "basic-memory provider ready: mode=%s project=%s tools=%d", + "basic-memory provider ready: mode=%s project=%s%s tools=%d", self._mode, self._project, + f" server_url={self._server_url}" if self._server_url else "", len(tools), ) @@ -1091,21 +1272,118 @@ def _server_argv(self) -> list[str]: return [bm, "mcp"] def shutdown(self) -> None: - if self._actor is not None: + self._stop_actor() + + def _reset_session_state(self) -> None: + """Discard capture identity after a fail-closed initialization.""" + self._session_id = "" + self._session_note_id = None + self._first_user_msg = None + self._session_started_at = None + with self._prefetch_lock: + self._pending_prefetch = "" + + def _stop_actor(self) -> None: + """Detach and close the current actor, tolerating shutdown failures.""" + with self._actor_lock: + actor = self._actor + self._actor = None + self._initialized = False + if actor is not None: try: - self._actor.shutdown(timeout=5.0) + actor.shutdown(timeout=5.0) except Exception as e: logger.debug("actor shutdown: %s", e) + + def _reconnect_http_locked(self) -> _BmMcpActor: + """Replace a failed HTTP actor; caller must hold ``_actor_lock``.""" + if not self._server_url: + raise RuntimeError("HTTP reconnect requested without server_url") + old = self._actor self._actor = None - self._initialized = False + if old is not None: + try: + old.shutdown(timeout=5.0) + except Exception as e: + logger.debug("basic-memory: failed HTTP actor shutdown: %s", e) + + actor = _BmMcpActor([], server_url=self._server_url) + # Publish before start, matching initialize(), so process shutdown can + # clean up an actor while its handshake is still in progress. + self._actor = actor + try: + actor.start(timeout=25.0) + except Exception: + if self._actor is actor: + self._actor = None + try: + actor.shutdown(timeout=5.0) + except Exception as e: + logger.debug("basic-memory: failed HTTP actor cleanup: %s", e) + raise + if self._actor is not actor: + try: + actor.shutdown(timeout=5.0) + except Exception as e: + logger.debug("basic-memory: interrupted HTTP actor cleanup: %s", e) + raise RuntimeError("basic-memory HTTP actor was replaced during reconnect") + return actor + + def _call_actor( + self, tool_name: str, arguments: dict[str, Any], timeout: float = 30.0 + ) -> str: + """Call the active MCP actor, retrying once after an HTTP failure. + + The first failure is never retried for legacy stdio/cloud mode. HTTP + retries create a fresh MCP transport and ClientSession, preventing a + stale server-assigned session from being reused after a disconnect. + """ + with self._actor_lock: + actor = self._actor + if actor is None: + raise RuntimeError("basic-memory MCP actor not running") + try: + return actor.call(tool_name, arguments, timeout=timeout) + except Exception as original_failure: + if not self._server_url: + raise + try: + with self._actor_lock: + # Another concurrent caller may already have replaced this + # failed actor. Reuse that fresh actor instead of + # reconnecting twice, but still bound this call to one + # retry. + if self._actor is actor: + retry_actor = self._reconnect_http_locked() + else: + retry_actor = self._actor + except Exception as reconnect_error: + logger.warning("basic-memory: HTTP reconnect failed: %s", reconnect_error) + raise original_failure + if retry_actor is None: + raise original_failure + # Never replay a mutation after an ambiguous transport failure: + # the server may already have committed it. Reconnect above keeps + # the next call healthy, while this call reports its original + # failure to the caller. + if tool_name not in _HTTP_RETRYABLE_TOOLS: + raise original_failure + return retry_actor.call(tool_name, arguments, timeout=timeout) # ---- Tool surface ---- def system_prompt_block(self) -> str: if not self._initialized: return "" + transport_note = ( + f"Connected to the existing MCP server at `{self._server_url}`; " + "the provider does not install or run a local `bm` process.\n" + if self._server_url + else "" + ) return ( "## Basic Memory Knowledge Graph\n" f"Active project: `{self._project}` ({self._mode}).\n" + f"{transport_note}" "\n" "**Use the `bm_*` tools below directly — do not shell out to the `bm` CLI.** " "These tools route through a persistent MCP connection " @@ -1165,7 +1443,7 @@ def handle_tool_call(self, tool_name: str, args: dict[str, Any], **kwargs: Any) except KeyError as e: return tool_error(f"{tool_name}: missing required arg {e}") try: - return self._actor.call(bm_tool, bm_args, timeout=30.0) + return self._call_actor(bm_tool, bm_args, timeout=30.0) except Exception as e: self._record_failure(e) logger.exception("bm tool call failed: %s", tool_name) @@ -1188,7 +1466,7 @@ def prefetch(self, query: str, *, session_id: str = "") -> str: # under cold-start or load. Prefetch is a recall hot path with a # 3s budget and the queries are usually keyword-like — FTS-only is # both faster and more deterministic. - raw = self._actor.call( + raw = self._call_actor( "search_notes", { "project": self._project, @@ -1215,7 +1493,7 @@ def _bg() -> None: # search_type="text" mirrors prefetch() — see note there. The # background path can afford a longer timeout but the # async-vector-indexing race still applies. - raw = self._actor.call( # type: ignore[union-attr] + raw = self._call_actor( "search_notes", { "project": self._project, @@ -1310,7 +1588,7 @@ def _capture_turn(self, user: str, asst: str) -> None: "tags": ["hermes-session", _hostname()], "output_format": "json", } - raw = self._actor.call("write_note", args, timeout=15.0) # type: ignore[union-attr] + raw = self._call_actor("write_note", args, timeout=15.0) self._session_note_id = _extract_permalink(raw, fallback=title) else: args = { @@ -1319,7 +1597,7 @@ def _capture_turn(self, user: str, asst: str) -> None: "operation": "append", "content": "\n" + block, } - self._actor.call("edit_note", args, timeout=15.0) # type: ignore[union-attr] + self._call_actor("edit_note", args, timeout=15.0) def _session_note_title(self) -> str: ts = (self._session_started_at or datetime.now(timezone.utc)).strftime("%Y-%m-%d %H%M") @@ -1392,7 +1670,7 @@ def _write_summary(self, messages: list[dict[str, Any]]) -> None: "output_format": "json", } try: - self._actor.call("write_note", args, timeout=15.0) # type: ignore[union-attr] + self._call_actor("write_note", args, timeout=15.0) except Exception as e: logger.warning("basic-memory: summary write failed: %s", e) @@ -1405,9 +1683,20 @@ def get_config_schema(self) -> list[dict[str, Any]]: "default": "local", "choices": ["local", "cloud"], }, + { + "key": "server_url", + "description": ( + "Existing Basic Memory MCP Streamable HTTP endpoint; when set, " + "connect here instead of spawning/installing bm" + ), + "default": "", + }, { "key": "project", - "description": "Basic Memory project name", + "description": ( + "Basic Memory project name (required when server_url is set; " + "otherwise defaults to hermes-memory)" + ), "default": _default_project(), }, { @@ -1568,7 +1857,7 @@ def _ensure_slash_ready(provider: "BasicMemoryProvider", cmd: str) -> str | None try: provider.initialize( session_id=f"slash:{cmd}:{int(time.time())}", - hermes_home=os.path.expanduser("~/.hermes"), + hermes_home=str(_active_hermes_home()), ) except Exception as e: logger.warning("basic-memory: slash init for /%s failed: %s", cmd, e) @@ -1603,7 +1892,7 @@ def _bm_search(raw_args: str) -> str: if err := _ensure_slash_ready(provider, "bm-search"): return err try: - raw = provider._actor.call( + raw = provider._call_actor( "search_notes", { "project": provider._project, @@ -1630,7 +1919,7 @@ def _bm_read(raw_args: str) -> str: if err := _ensure_slash_ready(provider, "bm-read"): return err try: - raw = provider._actor.call( + raw = provider._call_actor( "read_note", {"project": provider._project, "identifier": args}, timeout=15.0, @@ -1649,7 +1938,7 @@ def _bm_context(raw_args: str) -> str: if err := _ensure_slash_ready(provider, "bm-context"): return err try: - raw = provider._actor.call( + raw = provider._call_actor( "build_context", {"project": provider._project, "url": args, "depth": 1}, timeout=15.0, @@ -1669,7 +1958,7 @@ def _bm_recent(raw_args: str) -> str: if err := _ensure_slash_ready(provider, "bm-recent"): return err try: - raw = provider._actor.call( + raw = provider._call_actor( "recent_activity", { "project": provider._project, @@ -1709,10 +1998,13 @@ def _bm_status(raw_args: str) -> str: f" Mode: {provider._mode}", f" Project: {provider._project}", ] - if provider._mode == "local": + if provider._server_url: + lines.append(f" Server URL: {provider._server_url}") + elif provider._mode == "local": lines.append(f" Path: {provider._project_path}") - bm_bin = _bm_binary_path() - lines.append(f" bm CLI: {bm_bin or '(not found)'}") + bm_bin = _bm_binary_path() if not provider._server_url else None + cli_status = bm_bin or ("(not used)" if provider._server_url else "(not found)") + lines.append(f" bm CLI: {cli_status}") lines.append(f" MCP module: {'available' if _MCP_AVAILABLE else 'missing'}") lines.append(f" Initialized: {'yes' if provider._initialized else 'no'}") lines.append( @@ -1735,7 +2027,7 @@ def _bm_remember(raw_args: str) -> str: title = _remember_title(text) folder = provider._remember_folder or "bm-remember" try: - raw = provider._actor.call( + raw = provider._call_actor( "write_note", { "project": provider._project, @@ -1758,7 +2050,7 @@ def _bm_project(raw_args: str) -> str: if err := _ensure_slash_ready(provider, "bm-project"): return err try: - raw = provider._actor.call( + raw = provider._call_actor( "list_memory_projects", {"output_format": "json"}, timeout=15.0, @@ -1800,7 +2092,7 @@ def _bm_workspace(raw_args: str) -> str: f"This plugin is in '{provider._mode}' mode — no workspaces to list." ) try: - raw = provider._actor.call( + raw = provider._call_actor( "list_workspaces", {"output_format": "json"}, timeout=15.0, diff --git a/integrations/hermes/tests/test_http_integration.py b/integrations/hermes/tests/test_http_integration.py new file mode 100644 index 000000000..376aef5f7 --- /dev/null +++ b/integrations/hermes/tests/test_http_integration.py @@ -0,0 +1,135 @@ +"""Gated live tests for a running Basic Memory Streamable HTTP endpoint. + +Set ``BM_SERVER_URL`` to run the read-only path, for example:: + + BM_SERVER_URL=http://127.0.0.1:8766/mcp \ + uv run --with pytest --with mcp pytest tests/test_http_integration.py + +No test in this module writes by default. The opt-in write test additionally +requires ``BM_HTTP_WRITE_TEST=1`` and an explicit ``BM_HTTP_PROJECT``. +""" + +from __future__ import annotations + +import json +import os +import uuid + +import pytest + + +_SERVER_URL = os.environ.get("BM_SERVER_URL", "").strip() +_PROJECT = os.environ.get("BM_HTTP_PROJECT", "main").strip() or "main" +_WRITE_TEST = os.environ.get("BM_HTTP_WRITE_TEST") == "1" +_WRITE_PROJECT = os.environ.get("BM_HTTP_PROJECT", "").strip() + +try: + import mcp # noqa: F401 + + _MCP_OK = True +except Exception: + _MCP_OK = False + +pytestmark = [ + pytest.mark.skipif(not _SERVER_URL, reason="set BM_SERVER_URL to a running MCP endpoint"), + pytest.mark.skipif(not _MCP_OK, reason="mcp Python package not installed"), +] + + +@pytest.fixture +def http_provider(bm, tmp_path, monkeypatch): + """Initialize HTTP mode without touching the local BM CLI registry.""" + monkeypatch.setattr(bm, "_bm_binary_path", lambda: None) + monkeypatch.setattr( + bm, "_uv_binary_path", lambda: pytest.fail("HTTP mode attempted to locate uv") + ) + monkeypatch.setattr( + bm, "_install_bm_via_uv", lambda: pytest.fail("HTTP mode attempted to install bm") + ) + + config = { + "server_url": _SERVER_URL, + "project": _PROJECT, + # Read-only by default: no capture write can happen during a smoke run. + "capture_per_turn": False, + "capture_session_end": False, + } + (tmp_path / "basic-memory.json").write_text(json.dumps(config)) + + provider = bm.BasicMemoryProvider() + provider.initialize( + session_id=f"http-integration-{uuid.uuid4().hex[:8]}", + hermes_home=str(tmp_path), + platform="cli", + ) + if not provider._initialized: + pytest.fail("Provider failed to initialize against the live Streamable HTTP server") + try: + yield provider + finally: + provider.shutdown() + + +def test_live_http_lists_tools_and_uses_main_project(http_provider, bm): + """Handshake and discovery work through the configured HTTP daemon.""" + names = {tool["name"] for tool in http_provider._actor.list_tools()} + assert set(bm._HERMES_TO_BM.values()) <= names + raw = http_provider.handle_tool_call( + "bm_search", {"project": _PROJECT, "query": "Hermes", "limit": 1} + ) + assert raw and "error" not in raw.lower() + + +def test_live_http_prefetch_is_read_only(http_provider): + """Prefetch reaches the HTTP server without enabling capture writes.""" + result = http_provider.prefetch("Hermes", session_id="http-prefetch") + assert isinstance(result, str) + assert http_provider._capture_per_turn is False + assert http_provider._capture_session_end is False + + +def test_live_http_clean_shutdown_and_reconnect(http_provider, tmp_path): + """A clean close can reconnect to the same daemon URL without bm setup.""" + first_actor = http_provider._actor + http_provider.shutdown() + assert http_provider._initialized is False + assert http_provider._actor is None + + http_provider.initialize( + session_id="http-reconnect", + hermes_home=str(tmp_path), + platform="cli", + ) + try: + assert http_provider._initialized is True + assert http_provider._actor is not first_actor + assert http_provider._project == _PROJECT + finally: + http_provider.shutdown() + + +def test_live_http_optional_read_identifier(http_provider): + """An explicitly supplied identifier enables a narrow live note read.""" + identifier = os.environ.get("BM_HTTP_IDENTIFIER", "").strip() + if not identifier: + pytest.skip("set BM_HTTP_IDENTIFIER for an explicit live note read") + raw = http_provider.handle_tool_call("bm_read", {"identifier": identifier}) + assert raw and "error" not in raw.lower() + + +def test_live_http_opt_in_write_requires_marker_and_project(http_provider): + """Writes require both an explicit marker and an explicit target project.""" + if not (_WRITE_TEST and _WRITE_PROJECT): + pytest.skip("set BM_HTTP_WRITE_TEST=1 and BM_HTTP_PROJECT for write coverage") + marker = f"HERMES-HTTP-WRITE-{uuid.uuid4().hex}" + raw = http_provider.handle_tool_call( + "bm_write", + { + "project": _WRITE_PROJECT, + "title": marker, + "content": f"# {marker}\n\nExplicit HTTP integration test marker.\n", + "folder": "hermes-http-tests", + "tags": ["integration", "hermes-http-test"], + }, + ) + assert "error" not in raw.lower(), raw[:300] diff --git a/integrations/hermes/tests/test_http_provider.py b/integrations/hermes/tests/test_http_provider.py new file mode 100644 index 000000000..4ba132125 --- /dev/null +++ b/integrations/hermes/tests/test_http_provider.py @@ -0,0 +1,293 @@ +"""Unit coverage for the daemon-backed Streamable HTTP provider mode. + +The HTTP mode is deliberately tested without a ``bm`` executable, subprocess, +or project registry. The actor is a small synchronous test double so these +tests exercise provider routing, prefetch, capture, and reconnect behavior +without requiring an MCP server. +""" + +from __future__ import annotations + +import json +from datetime import datetime, timezone +from types import SimpleNamespace + +import pytest + + +SERVER_URL = "http://127.0.0.1:8766/mcp" + + +class HttpActor: + """Minimal actor double that records the URL and every MCP tool call.""" + + instances: list["HttpActor"] = [] + + def __init__(self, *args, **kwargs): + self.args = args + self.kwargs = kwargs + self.server_url = kwargs.get("server_url") + if self.server_url is None: + self.server_url = next( + (a for a in args if isinstance(a, str) and a.startswith("http")), None + ) + self._server_url = self.server_url + self.calls: list[tuple[str, dict]] = [] + self.started = False + self.alive = False + self.shutdown_calls: list[float] = [] + type(self).instances.append(self) + + def start(self, timeout: float = 25.0) -> None: + self.started = True + self.alive = True + + def is_alive(self) -> bool: + return self.alive + + def list_tools(self) -> list[dict[str, str]]: + return [ + {"name": name, "description": ""} + for name in ( + "search_notes", + "read_note", + "write_note", + "edit_note", + "build_context", + "delete_note", + "move_note", + "recent_activity", + "list_memory_projects", + "list_workspaces", + ) + ] + + def call(self, tool_name: str, arguments: dict, timeout: float = 30.0) -> str: + self.calls.append((tool_name, dict(arguments))) + if tool_name == "search_notes": + return json.dumps({"results": [{"title": "main note", "permalink": "main/note"}]}) + return json.dumps({"permalink": "main/hermes-sessions/session", "ok": True}) + + def shutdown(self, timeout: float = 5.0) -> None: + self.shutdown_calls.append(timeout) + self.alive = False + + +def _enable_mcp_for_unit_test(bm, monkeypatch): + """The test double replaces the SDK transport, so no MCP package is needed.""" + monkeypatch.setattr(bm, "_MCP_AVAILABLE", True) + monkeypatch.setattr(bm, "streamable_http_client", object()) + # Keep this compatible with implementations that expose a separate HTTP + # availability flag while the transport import remains optional. + for name in ("_HTTP_AVAILABLE", "_STREAMABLE_HTTP_AVAILABLE"): + monkeypatch.setattr(bm, name, True, raising=False) + + +def test_server_url_is_available_without_bm_or_uv(bm, monkeypatch): + """Availability for URL mode must not depend on local CLI installation.""" + _enable_mcp_for_unit_test(bm, monkeypatch) + monkeypatch.setattr(bm, "streamable_http_client", object()) + monkeypatch.setattr(bm, "_load_config", lambda _: {"server_url": SERVER_URL}) + monkeypatch.setattr(bm, "_bm_binary_path", lambda: pytest.fail("looked up bm")) + monkeypatch.setattr(bm, "_uv_binary_path", lambda: pytest.fail("looked up uv")) + assert bm.BasicMemoryProvider().is_available() is True + + +def test_config_schema_advertises_server_url(bm): + """The new key is discoverable without changing legacy mode choices.""" + entries = {entry["key"]: entry for entry in bm.BasicMemoryProvider().get_config_schema()} + assert "server_url" in entries + assert entries["server_url"]["default"] == "" + + +def test_actor_server_url_uses_streamable_http_transport(bm, monkeypatch): + """The URL constructor selects Streamable HTTP, not the stdio client.""" + entered_urls: list[str] = [] + + class HttpTransport: + def __init__(self, url): + entered_urls.append(url) + + async def __aenter__(self): + return (object(), object()) + + async def __aexit__(self, exc_type, exc, tb): + return False + + class Session: + def __init__(self, read, write): + self.read = read + self.write = write + + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc, tb): + return False + + async def initialize(self): + return None + + async def list_tools(self): + return SimpleNamespace(tools=[SimpleNamespace(name="search_notes", description="")]) + + # An async generator is not the SDK's context-manager shape; use a regular + # factory so the fake mirrors ``streamable_http_client(url)`` exactly. + def transport_factory(url): + return HttpTransport(url) + + _enable_mcp_for_unit_test(bm, monkeypatch) + monkeypatch.setattr(bm, "streamable_http_client", transport_factory) + monkeypatch.setattr(bm, "ClientSession", Session, raising=False) + monkeypatch.setattr( + bm, "StdioServerParameters", lambda **kwargs: pytest.fail("used stdio"), raising=False + ) + monkeypatch.setattr( + bm, "stdio_client", lambda *args: pytest.fail("used stdio"), raising=False + ) + + actor = bm._BmMcpActor(server_url=SERVER_URL) + actor.start(timeout=2.0) + try: + assert actor._server_url == SERVER_URL + assert entered_urls == [SERVER_URL] + assert actor.list_tools()[0]["name"] == "search_notes" + finally: + actor.shutdown(timeout=2.0) + + +def _http_config(tmp_path, *, capture_per_turn=False, capture_session_end=False): + config = { + "server_url": SERVER_URL, + "project": "main", + "project_path": str(tmp_path / "must-not-be-created"), + "capture_per_turn": capture_per_turn, + "capture_session_end": capture_session_end, + } + (tmp_path / "basic-memory.json").write_text(json.dumps(config)) + + +def test_server_url_initialization_skips_bm_bootstrap_and_project_registration( + bm, monkeypatch, tmp_path +): + """A daemon URL is self-contained: no bm lookup, install, or project add.""" + _enable_mcp_for_unit_test(bm, monkeypatch) + _http_config(tmp_path) + HttpActor.instances = [] + + monkeypatch.setattr(bm, "_bm_binary_path", lambda: pytest.fail("HTTP mode looked up bm")) + monkeypatch.setattr(bm, "_uv_binary_path", lambda: pytest.fail("HTTP mode looked up uv")) + monkeypatch.setattr( + bm, "_install_bm_via_uv", lambda: pytest.fail("HTTP mode installed bm") + ) + monkeypatch.setattr( + bm.BasicMemoryProvider, + "_ensure_local_project", + lambda self: pytest.fail("HTTP mode created a local project"), + ) + monkeypatch.setattr( + bm.BasicMemoryProvider, + "_verify_project_registered", + lambda self: pytest.fail("HTTP mode inspected project registration"), + ) + monkeypatch.setattr(bm, "_BmMcpActor", HttpActor) + + provider = bm.BasicMemoryProvider() + provider.initialize(session_id="http-session", hermes_home=str(tmp_path)) + + try: + assert provider._initialized is True + assert provider._mode == "server_url" + assert provider._project == "main" + assert not (tmp_path / "must-not-be-created").exists() + assert len(HttpActor.instances) == 1 + assert HttpActor.instances[0].server_url == SERVER_URL + assert HttpActor.instances[0].started is True + finally: + provider.shutdown() + + +def test_server_url_preserves_main_project_routing_and_prefetch(bm, monkeypatch, tmp_path): + """HTTP calls still carry the configured ``main`` project on scoped tools.""" + _enable_mcp_for_unit_test(bm, monkeypatch) + _http_config(tmp_path) + HttpActor.instances = [] + monkeypatch.setattr(bm, "_BmMcpActor", HttpActor) + monkeypatch.setattr(bm, "_bm_binary_path", lambda: None) + + provider = bm.BasicMemoryProvider() + provider.initialize(session_id="http-session", hermes_home=str(tmp_path)) + actor = HttpActor.instances[0] + try: + raw = provider.handle_tool_call("bm_search", {"query": "main"}) + assert json.loads(raw)["results"] + assert actor.calls[0] == ("search_notes", {"project": "main", "query": "main"}) + + recall = provider.prefetch("main") + assert "Basic Memory Recall" in recall + assert actor.calls[1][0] == "search_notes" + assert actor.calls[1][1]["project"] == "main" + assert actor.calls[1][1]["search_type"] == "text" + finally: + provider.shutdown() + + +def test_server_url_capture_uses_same_actor_and_project(bm, monkeypatch, tmp_path): + """Per-turn and end-of-session capture remain ordinary MCP calls over HTTP.""" + _enable_mcp_for_unit_test(bm, monkeypatch) + _http_config(tmp_path, capture_per_turn=True, capture_session_end=True) + HttpActor.instances = [] + monkeypatch.setattr(bm, "_BmMcpActor", HttpActor) + monkeypatch.setattr(bm, "_bm_binary_path", lambda: None) + + provider = bm.BasicMemoryProvider() + provider.initialize(session_id="http-session", hermes_home=str(tmp_path)) + provider._session_started_at = datetime(2026, 5, 10, 12, 34, tzinfo=timezone.utc) + actor = HttpActor.instances[0] + try: + provider.sync_turn("user over HTTP", "assistant over HTTP") + assert provider._sync_thread is not None + provider._sync_thread.join(timeout=2) + assert not provider._sync_thread.is_alive() + assert actor.calls[0][0] == "write_note" + assert actor.calls[0][1]["project"] == "main" + + provider.on_session_end( + [ + {"role": "user", "content": "user over HTTP"}, + {"role": "assistant", "content": "done"}, + ] + ) + assert any(name == "write_note" and args["project"] == "main" for name, args in actor.calls) + finally: + provider.shutdown() + + +def test_server_url_reconnects_after_dead_actor_without_cli_or_project_work( + bm, monkeypatch, tmp_path +): + """A dead HTTP actor is replaced with another URL-backed actor cleanly.""" + _enable_mcp_for_unit_test(bm, monkeypatch) + _http_config(tmp_path) + HttpActor.instances = [] + monkeypatch.setattr(bm, "_BmMcpActor", HttpActor) + monkeypatch.setattr(bm, "_bm_binary_path", lambda: None) + monkeypatch.setattr( + bm.BasicMemoryProvider, + "_verify_project_registered", + lambda self: pytest.fail("HTTP reconnect checked project registry"), + ) + + provider = bm.BasicMemoryProvider() + provider.initialize(session_id="http-session", hermes_home=str(tmp_path)) + first = HttpActor.instances[0] + first.alive = False + provider.initialize(session_id="http-session", hermes_home=str(tmp_path)) + + try: + assert first.shutdown_calls + assert len(HttpActor.instances) == 2 + assert HttpActor.instances[1].server_url == SERVER_URL + assert provider._initialized is True + finally: + provider.shutdown() diff --git a/integrations/hermes/tests/test_http_review.py b/integrations/hermes/tests/test_http_review.py new file mode 100644 index 000000000..6bcf681e6 --- /dev/null +++ b/integrations/hermes/tests/test_http_review.py @@ -0,0 +1,387 @@ +"""Regression tests for the review-hardening of the Streamable HTTP mode.""" + +from __future__ import annotations + +import asyncio +import json +from types import SimpleNamespace + +import pytest + +from tests.test_http_provider import SERVER_URL, _enable_mcp_for_unit_test + + +class RetryActor: + """HTTP actor double that fails one real call and then permits one retry.""" + + instances: list["RetryActor"] = [] + fail_next_call = True + + def __init__(self, *args, **kwargs): + self._server_url = kwargs.get("server_url") + self.calls: list[tuple[str, dict]] = [] + self.alive = False + self.shutdown_calls = 0 + type(self).instances.append(self) + + def start(self, timeout: float = 25.0): + self.alive = True + + def is_alive(self): + return self.alive + + def list_tools(self): + return [{"name": name, "description": ""} for name in ( + "search_notes", + "read_note", + "write_note", + "edit_note", + "build_context", + "delete_note", + "move_note", + "recent_activity", + "list_memory_projects", + "list_workspaces", + )] + + def call(self, name, arguments, timeout=30.0): + self.calls.append((name, dict(arguments))) + if type(self).fail_next_call: + type(self).fail_next_call = False + self.alive = False + raise RuntimeError("simulated HTTP actor disconnect") + if name == "search_notes": + return json.dumps({"results": [{"title": "retry note", "permalink": "main/retry"}]}) + return json.dumps({"permalink": "main/hermes-sessions/retry", "ok": True}) + + def shutdown(self, timeout: float = 5.0): + self.shutdown_calls += 1 + self.alive = False + + +def _write_config(tmp_path, *, url=SERVER_URL, project="main", capture=False): + (tmp_path / "basic-memory.json").write_text( + json.dumps( + { + "server_url": url, + "project": project, + "capture_per_turn": capture, + "capture_session_end": False, + } + ) + ) + + +def _patch_http_provider(bm, monkeypatch, tmp_path, *, capture=False): + _enable_mcp_for_unit_test(bm, monkeypatch) + _write_config(tmp_path, capture=capture) + RetryActor.instances = [] + RetryActor.fail_next_call = True + monkeypatch.setattr(bm, "_BmMcpActor", RetryActor) + monkeypatch.setattr(bm, "_bm_binary_path", lambda: None) + monkeypatch.setattr( + bm.BasicMemoryProvider, + "_verify_project_registered", + lambda self: pytest.fail("HTTP mode consulted the local project registry"), + ) + + +def test_profile_aware_hermes_home_is_used_for_availability(bm, monkeypatch, tmp_path): + """Discovery reads the selected Hermes profile, not only ~/.hermes.""" + profile = tmp_path / "profile" + profile.mkdir() + (profile / "basic-memory.json").write_text(json.dumps({"server_url": SERVER_URL})) + _enable_mcp_for_unit_test(bm, monkeypatch) + monkeypatch.setenv("HERMES_HOME", str(profile)) + monkeypatch.setattr(bm, "_bm_binary_path", lambda: pytest.fail("looked up bm")) + monkeypatch.setattr(bm, "_uv_binary_path", lambda: pytest.fail("looked up uv")) + assert bm.BasicMemoryProvider().is_available() is True + + +def test_server_url_requires_explicit_project_name(bm, monkeypatch, tmp_path): + """An HTTP daemon cannot silently route to a local default project.""" + _enable_mcp_for_unit_test(bm, monkeypatch) + (tmp_path / "basic-memory.json").write_text(json.dumps({"server_url": SERVER_URL})) + monkeypatch.setattr(bm, "_BmMcpActor", lambda *a, **k: pytest.fail("missing project connected")) + monkeypatch.setattr(bm, "_bm_binary_path", lambda: pytest.fail("looked up bm")) + provider = bm.BasicMemoryProvider() + provider.initialize(session_id="missing-project", hermes_home=str(tmp_path)) + assert provider._initialized is False + + +@pytest.mark.parametrize( + ("url", "secret"), + [ + ("ftp://127.0.0.1:8766/mcp", ""), + ("http://", ""), + ("http://127.0.0.1:8766/mcp?token=TOKENVALUE123", "TOKENVALUE123"), + ("http://127.0.0.1:8766/mcp#s3cr3tfrag", "s3cr3tfrag"), + ("http://user:UltraSecret99@127.0.0.1:8766/mcp", "UltraSecret99"), + ], +) +def test_invalid_server_url_is_rejected_without_secret_logging( + bm, monkeypatch, tmp_path, caplog, url, secret +): + """Only a bare HTTP(S) endpoint is accepted; credentials never enter logs.""" + _enable_mcp_for_unit_test(bm, monkeypatch) + _write_config(tmp_path, url=url) + monkeypatch.setattr(bm, "_BmMcpActor", lambda *a, **k: pytest.fail("invalid URL connected")) + monkeypatch.setattr(bm, "_bm_binary_path", lambda: pytest.fail("invalid URL looked up bm")) + monkeypatch.setattr(bm, "_uv_binary_path", lambda: pytest.fail("invalid URL looked up uv")) + provider = bm.BasicMemoryProvider() + with caplog.at_level("WARNING"): + provider.initialize(session_id="invalid-url", hermes_home=str(tmp_path)) + assert provider._initialized is False + if secret: + assert secret not in caplog.text + + +@pytest.mark.parametrize("operation", ["tool", "prefetch"]) +def test_http_call_reconnects_once_and_retries_common_paths(bm, monkeypatch, tmp_path, operation): + """A dead HTTP actor gets exactly one bounded reconnect/retry.""" + _patch_http_provider(bm, monkeypatch, tmp_path, capture=operation == "capture") + provider = bm.BasicMemoryProvider() + provider.initialize(session_id="retry-session", hermes_home=str(tmp_path)) + try: + if operation == "tool": + raw = provider.handle_tool_call("bm_search", {"query": "retry"}) + assert "error" not in raw.lower() + elif operation == "prefetch": + assert "Basic Memory Recall" in provider.prefetch("retry") + else: + provider.sync_turn("retry user", "retry assistant") + assert provider._sync_thread is not None + provider._sync_thread.join(timeout=2) + assert not provider._sync_thread.is_alive() + + assert len(RetryActor.instances) == 2 + assert RetryActor.instances[0].shutdown_calls == 1 + assert len(RetryActor.instances[0].calls) == 1 + assert len(RetryActor.instances[1].calls) == 1 + finally: + provider.shutdown() + + +@pytest.mark.parametrize( + ("hermes_tool", "arguments"), + [ + ( + "bm_write", + {"title": "ambiguous", "content": "do not duplicate", "folder": "tests"}, + ), + ( + "bm_edit", + { + "identifier": "main/tests/existing", + "operation": "append", + "content": "do not duplicate", + }, + ), + ], +) +def test_http_failure_reconnects_but_does_not_replay_mutation( + bm, monkeypatch, tmp_path, hermes_tool, arguments +): + """An ambiguous write failure reconnects, but never duplicates the write.""" + _patch_http_provider(bm, monkeypatch, tmp_path) + provider = bm.BasicMemoryProvider() + provider.initialize(session_id="mutation-session", hermes_home=str(tmp_path)) + try: + raw = provider.handle_tool_call(hermes_tool, arguments) + assert "error" in raw.lower() + assert len(RetryActor.instances) == 2 + assert len(RetryActor.instances[0].calls) == 1 + assert RetryActor.instances[0].calls[0][0] == { + "bm_write": "write_note", + "bm_edit": "edit_note", + }[hermes_tool] + assert RetryActor.instances[1].calls == [] + + # The next independent call uses the fresh transport and can retry-free + # continue operating after the ambiguous mutation result. + recovered = provider.handle_tool_call("bm_search", {"query": "after-write"}) + assert "results" in recovered + assert RetryActor.instances[1].calls[0][0] == "search_notes" + finally: + provider.shutdown() + + +def test_http_capture_failure_reconnects_but_does_not_replay_capture( + bm, monkeypatch, tmp_path +): + """Automatic capture treats a failed write as ambiguous, never as retryable.""" + _patch_http_provider(bm, monkeypatch, tmp_path, capture=True) + provider = bm.BasicMemoryProvider() + provider.initialize(session_id="capture-session", hermes_home=str(tmp_path)) + try: + provider.sync_turn("captured user", "captured assistant") + assert provider._sync_thread is not None + provider._sync_thread.join(timeout=2) + assert not provider._sync_thread.is_alive() + assert len(RetryActor.instances) == 2 + assert RetryActor.instances[0].calls[0][0] == "write_note" + assert RetryActor.instances[1].calls == [] + + # A subsequent explicit read proves the reconnect was retained. + assert "results" in provider.handle_tool_call("bm_search", {"query": "after-capture"}) + assert RetryActor.instances[1].calls[0][0] == "search_notes" + finally: + provider.shutdown() + + +def test_invalid_reinitialize_clears_actor_and_session_capture_state( + bm, monkeypatch, tmp_path +): + """A bad config cannot leave an old actor or note ID for a later session.""" + _patch_http_provider(bm, monkeypatch, tmp_path) + provider = bm.BasicMemoryProvider() + provider.initialize(session_id="before-invalid", hermes_home=str(tmp_path)) + old_actor = RetryActor.instances[0] + provider._session_note_id = "main/hermes-sessions/old-session" + provider._first_user_msg = "old session opener" + provider._session_started_at = object() # type: ignore[assignment] + (tmp_path / "basic-memory.json").write_text( + json.dumps({"server_url": "https://user:secret@example.test/mcp", "project": "main"}) + ) + + try: + provider.initialize(session_id="after-invalid", hermes_home=str(tmp_path)) + + assert old_actor.shutdown_calls == 1 + assert provider._actor is None + assert provider._initialized is False + assert provider._server_url is None + assert provider._session_id == "" + assert provider._session_note_id is None + assert provider._first_user_msg is None + assert provider._session_started_at is None + + # A subsequent valid initialization must start capture with write_note, + # never edit_note against the old session permalink. + _write_config(tmp_path, capture=True) + RetryActor.fail_next_call = False + provider.initialize(session_id="after-valid", hermes_home=str(tmp_path)) + provider.sync_turn("new user", "new assistant") + assert provider._sync_thread is not None + provider._sync_thread.join(timeout=2) + assert not provider._sync_thread.is_alive() + fresh_actor = RetryActor.instances[-1] + assert fresh_actor.calls[0][0] == "write_note" + finally: + provider.shutdown() + + +def test_stdio_call_failure_is_not_retried(bm, monkeypatch, tmp_path): + """Automatic reconnect is scoped to server_url and cannot alter stdio.""" + class StdioFailActor(RetryActor): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._server_url = None + + def call(self, name, arguments, timeout=30.0): + self.calls.append((name, dict(arguments))) + self.alive = False + raise RuntimeError("simulated stdio failure") + + _enable_mcp_for_unit_test(bm, monkeypatch) + (tmp_path / "basic-memory.json").write_text( + json.dumps({"mode": "local", "project": "main", "capture_per_turn": False}) + ) + StdioFailActor.instances = [] + monkeypatch.setattr(bm, "_BmMcpActor", StdioFailActor) + monkeypatch.setattr(bm, "_bm_binary_path", lambda: "/fake/bm") + monkeypatch.setattr(bm.BasicMemoryProvider, "_ensure_local_project", lambda self: None) + monkeypatch.setattr(bm.BasicMemoryProvider, "_verify_project_registered", lambda self: True) + provider = bm.BasicMemoryProvider() + provider.initialize(session_id="stdio-failure", hermes_home=str(tmp_path)) + try: + raw = provider.handle_tool_call("bm_search", {"query": "no retry"}) + assert "error" in raw + assert len(StdioFailActor.instances) == 1 + assert len(StdioFailActor.instances[0].calls) == 1 + finally: + provider.shutdown() + + +def test_http_transport_and_session_contexts_both_close_on_shutdown(bm, monkeypatch): + """Shutdown unwinds ClientSession before the Streamable HTTP transport.""" + exits: list[str] = [] + transport_calls: list[tuple[str, dict]] = [] + streams = (object(), object()) + + class Transport: + def __init__(self, url, **kwargs): + transport_calls.append((url, kwargs)) + + async def __aenter__(self): + # MCP SDK 1.x also yielded a session-id callback as a third item. + # The plugin intentionally consumes only read/write; the SDK owns + # Mcp-Session-Id headers and DELETE-on-close semantics. + return (*streams, lambda: "server-session-123") + + async def __aexit__(self, exc_type, exc, tb): + exits.append("transport") + + class Session: + def __init__(self, read, write): + assert (read, write) == streams + + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc, tb): + exits.append("session") + + async def initialize(self): + return None + + async def list_tools(self): + return SimpleNamespace(tools=[]) + + _enable_mcp_for_unit_test(bm, monkeypatch) + monkeypatch.setattr(bm, "streamable_http_client", lambda url: Transport(url)) + monkeypatch.setattr(bm, "ClientSession", Session, raising=False) + actor = bm._BmMcpActor(server_url=SERVER_URL) + actor.start(timeout=2) + actor.shutdown(timeout=2) + assert transport_calls == [(SERVER_URL, {})] + assert exits == ["session", "transport"] + assert actor._thread is not None and not actor._thread.is_alive() + + +def test_http_start_timeout_unwinds_contexts_and_stops_thread(bm, monkeypatch): + """Cancellation during HTTP session startup is bounded and leak-free.""" + exits: list[str] = [] + + class Transport: + async def __aenter__(self): + return (object(), object()) + + async def __aexit__(self, exc_type, exc, tb): + exits.append("transport") + + class HangingSession: + def __init__(self, read, write): + self.read = read + self.write = write + + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc, tb): + exits.append("session") + + async def initialize(self): + await asyncio.sleep(60) + + async def list_tools(self): + return SimpleNamespace(tools=[]) + + _enable_mcp_for_unit_test(bm, monkeypatch) + monkeypatch.setattr(bm, "streamable_http_client", lambda url: Transport()) + monkeypatch.setattr(bm, "ClientSession", HangingSession, raising=False) + actor = bm._BmMcpActor(server_url=SERVER_URL) + with pytest.raises(TimeoutError): + actor.start(timeout=0.05) + assert exits == ["session", "transport"] + assert actor._thread is not None and not actor._thread.is_alive() From 187cf61b14902479ec22bbe7064cfc1e9814e711 Mon Sep 17 00:00:00 2001 From: Brett Grane <24548810+scarabone@users.noreply.github.com> Date: Sun, 30 Aug 2026 20:43:16 -0400 Subject: [PATCH 2/3] test(hermes): isolate provider availability checks Signed-off-by: Brett Grane <24548810+scarabone@users.noreply.github.com> --- integrations/hermes/tests/test_provider.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/integrations/hermes/tests/test_provider.py b/integrations/hermes/tests/test_provider.py index 7fe0ccc43..7b7ef230a 100644 --- a/integrations/hermes/tests/test_provider.py +++ b/integrations/hermes/tests/test_provider.py @@ -13,9 +13,10 @@ def test_is_available_no_mcp(bm, monkeypatch): assert p.is_available() is False -def test_is_available_no_bm_no_uv(bm, monkeypatch): +def test_is_available_no_bm_no_uv(bm, monkeypatch, tmp_path): """No bm AND no uv → can't install, can't operate → unavailable.""" monkeypatch.setattr(bm, "_MCP_AVAILABLE", True) + monkeypatch.setattr(bm, "_active_hermes_home", lambda: tmp_path) monkeypatch.setattr(bm, "_bm_binary_path", lambda: None) monkeypatch.setattr(bm, "_uv_binary_path", lambda: None) p = bm.BasicMemoryProvider() From de485c82dccda68c39ac69da045254fb7338f29e Mon Sep 17 00:00:00 2001 From: Brett Grane <24548810+scarabone@users.noreply.github.com> Date: Tue, 1 Sep 2026 17:44:52 -0400 Subject: [PATCH 3/3] fix(hermes): skip vault capture for cron sessions Cron runs (session ids prefixed cron_) are not conversations. A 2-hourly watch answering [SILENT] wrote two notes per tick, and Basic Memory 0.23.2 dropped the first of the near-simultaneous pair, leaving a file-less index row every run. Interactive sessions are still captured per turn and at end. Co-Authored-By: Claude --- integrations/hermes/__init__.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/integrations/hermes/__init__.py b/integrations/hermes/__init__.py index 2236830b4..ce1165079 100644 --- a/integrations/hermes/__init__.py +++ b/integrations/hermes/__init__.py @@ -1540,10 +1540,19 @@ def _format_prefetch(self, raw_json: str) -> str: lines.append(f"- **{title}** (`{permalink}`) — {preview}") return "\n".join(lines) + # ---- Cron exclusion ---- + # Hermes cron runs (session ids prefixed "cron_") are not conversations: + # a 2-hourly watch that answers [SILENT] wrote two vault notes per tick, + # and Basic Memory 0.23.2 dropped the first of the near-simultaneous pair + # (index row, no file) every time. Interactive sessions are still captured. + def _is_cron_session(self, session_id: str = "") -> bool: + return (session_id or self._session_id or "").startswith("cron_") + # ---- Per-turn capture (extract step) ---- def sync_turn(self, user_content: str, assistant_content: str, *, session_id: str = "") -> None: if ( not self._capture_per_turn + or self._is_cron_session(session_id) or not self._initialized or self._actor is None or self._is_circuit_open() @@ -1612,7 +1621,12 @@ def _session_note_title(self) -> str: # ---- Session-end summary ---- def on_session_end(self, messages: list[dict[str, Any]]) -> None: - if not self._capture_session_end or not self._initialized or self._actor is None: + if ( + not self._capture_session_end + or self._is_cron_session() + or not self._initialized + or self._actor is None + ): return try: self._write_summary(messages)