diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 58161975c415..8b55fc17df03 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -7713,6 +7713,11 @@ def _update_via_zip(args): ) _install_python_dependencies_with_optional_fallback(pip_cmd) + # ZIP path parity: heal the active memory provider's bridge packages + # after the dependency reinstall, same as the git-pull path (#53272, + # #70636). + _refresh_active_memory_provider_dependencies() + node_failures = _update_node_dependencies() _build_web_ui(PROJECT_ROOT / "web") @@ -9541,6 +9546,55 @@ def _refresh_active_lazy_features( return False +def _refresh_active_memory_provider_dependencies() -> None: + """Refresh pip dependencies for the configured external memory provider. + + Memory-provider bridge packages are declared in each provider's + ``plugin.yaml`` (plus mode-dependent extras like Hindsight's + ``hindsight-all``), NOT in Hermes' editable-install extras or + ``LAZY_DEPS`` alone — so the core dependency reinstall above can strip + or downgrade them (#53272 mem0ai, #70636 hindsight-embed). Re-run the + provider's declared install for the ACTIVE provider only, after the + core install and lazy refresh, so the last write to any shared package + is the one the active provider needs. + + Never raises. A failure here must not block the rest of the update. + """ + try: + from hermes_cli.config import load_config + + cfg = load_config() + except Exception as exc: + logger.debug("Memory provider refresh skipped (config load failed): %s", exc) + return + + provider = "" + if isinstance(cfg, dict): + memory_cfg = cfg.get("memory") + if isinstance(memory_cfg, dict): + if memory_cfg.get("enabled") is False: + return + provider = str(memory_cfg.get("provider") or "").strip() + + # "default" / empty is the built-in file-backed store — no pip deps. + if not provider or provider in {"default", "builtin", "none"}: + return + + try: + from hermes_cli.memory_setup import _install_dependencies + except Exception as exc: + logger.debug("Memory provider refresh skipped (import failed): %s", exc) + return + + print() + print(f"→ Refreshing active memory provider dependencies ({provider})...") + + try: + _install_dependencies(provider, force=True) + except Exception as exc: + print(f" ⚠ {provider} dependencies failed to refresh: {exc}") + + def _install_python_dependencies_with_optional_fallback( install_cmd_prefix: list[str], *, @@ -12199,6 +12253,11 @@ def _cmd_update_impl(args, gateway_mode: bool): "to finish import-based venv repair." ) + # Heal the active memory provider's bridge packages last — the core + # reinstall + lazy refresh above may have stripped or downgraded + # plugin.yaml-declared deps that aren't in extras (#53272, #70636). + _refresh_active_memory_provider_dependencies() + node_failures = _update_node_dependencies() _build_web_ui(PROJECT_ROOT / "web") diff --git a/hermes_cli/memory_setup.py b/hermes_cli/memory_setup.py index c1858fc832ff..3d8e12d572e4 100644 --- a/hermes_cli/memory_setup.py +++ b/hermes_cli/memory_setup.py @@ -8,6 +8,7 @@ the provider's config schema. Writes config to config.yaml + .env. from __future__ import annotations import os +import re import sys import shlex from pathlib import Path @@ -18,6 +19,32 @@ from hermes_cli.secret_prompt import masked_secret_prompt _CANCELLED = -1 +def _provider_pip_dependencies(provider_name: str, declared: list) -> list: + """Return the pip deps a provider actually needs on THIS install. + + ``plugin.yaml`` declares the provider's baseline bridge packages, but + some providers install mode-dependent extras at setup time that the + manifest can't express. Hindsight's ``local_embedded`` mode installs + ``hindsight-all`` (daemon + embedder + client) during + ``hermes memory setup`` — if the update-time refresh only reinstalled + the declared ``hindsight-client``, the embedded daemon would stay + broken after a venv rebuild stripped ``hindsight-embed`` (#70636). + """ + deps = list(declared or []) + if provider_name == "hindsight": + try: + import json + cfg_path = get_hermes_home() / "hindsight" / "config.json" + cfg = json.loads(cfg_path.read_text(encoding="utf-8")) if cfg_path.exists() else {} + mode = cfg.get("mode", "") + # "local" is a legacy alias for "local_embedded" + if mode in {"local", "local_embedded"}: + deps.append("hindsight-all") + except Exception: + pass + return deps + + # --------------------------------------------------------------------------- # Curses-based interactive picker (same pattern as hermes tools) # --------------------------------------------------------------------------- @@ -77,8 +104,16 @@ def _prompt(label: str, default: str | None = None, secret: bool = False) -> str # Provider discovery # --------------------------------------------------------------------------- -def _install_dependencies(provider_name: str) -> None: - """Install pip dependencies declared in plugin.yaml.""" +def _install_dependencies(provider_name: str, *, force: bool = False) -> None: + """Install pip dependencies declared in ``plugin.yaml``. + + When ``force`` is true, every declared dependency is handed to the + installer even if its import currently succeeds — the resolver then + reinstalls anything missing or version-drifted and no-ops on satisfied + ranges. This is how ``hermes update`` heals the active memory provider + after a venv rebuild/sync removed or downgraded its bridge packages + (#53272, #70636). + """ import subprocess from plugins.memory import find_provider_dir @@ -96,7 +131,7 @@ def _install_dependencies(provider_name: str) -> None: except Exception: return - pip_deps = meta.get("pip_dependencies", []) + pip_deps = _provider_pip_dependencies(provider_name, meta.get("pip_dependencies", [])) if not pip_deps: return @@ -108,10 +143,15 @@ def _install_dependencies(provider_name: str) -> None: "hindsight-all": "hindsight", } - # Check which packages are missing + # Check which packages need installation. missing = [] for dep in pip_deps: - import_name = _IMPORT_NAMES.get(dep, dep.replace("-", "_").split("[")[0]) + if force: + missing.append(dep) + continue + dep_name = re.match(r"^[A-Za-z0-9_][A-Za-z0-9_.\-]*", dep) + base = dep_name.group(0) if dep_name else dep + import_name = _IMPORT_NAMES.get(base, base.replace("-", "_").split("[")[0]) try: __import__(import_name) except ImportError: diff --git a/tests/hermes_cli/test_update_autostash.py b/tests/hermes_cli/test_update_autostash.py index d89d830b140a..94fd27949f16 100644 --- a/tests/hermes_cli/test_update_autostash.py +++ b/tests/hermes_cli/test_update_autostash.py @@ -480,6 +480,94 @@ def test_cmd_update_succeeds_with_extras(monkeypatch, tmp_path): assert ".[all]" in install_cmds[0] +def test_refresh_active_memory_provider_dependencies_reinstalls_active_provider(monkeypatch): + """#53272/#70636: update must re-run the active provider's dep install.""" + recorded = [] + + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"memory": {"provider": "mem0"}}, + ) + monkeypatch.setattr( + "hermes_cli.memory_setup._install_dependencies", + lambda provider_name, force=False: recorded.append((provider_name, force)), + ) + + hermes_main._refresh_active_memory_provider_dependencies() + + assert recorded == [("mem0", True)] + + +@pytest.mark.parametrize( + "memory_cfg", + [ + {}, # no provider configured + {"provider": ""}, # empty provider + {"provider": "default"}, # built-in store + {"provider": "mem0", "enabled": False}, # memory disabled + ], +) +def test_refresh_active_memory_provider_dependencies_skips_inactive(monkeypatch, memory_cfg): + recorded = [] + + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"memory": memory_cfg}, + ) + monkeypatch.setattr( + "hermes_cli.memory_setup._install_dependencies", + lambda provider_name, force=False: recorded.append((provider_name, force)), + ) + + hermes_main._refresh_active_memory_provider_dependencies() + + assert recorded == [] + + +def test_refresh_active_memory_provider_dependencies_never_raises(monkeypatch): + """A provider install failure must not block the rest of the update.""" + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"memory": {"provider": "hindsight"}}, + ) + + def boom(provider_name, force=False): + raise RuntimeError("pip exploded") + + monkeypatch.setattr("hermes_cli.memory_setup._install_dependencies", boom) + + hermes_main._refresh_active_memory_provider_dependencies() # must not raise + + +def test_cmd_update_refreshes_active_memory_provider_dependencies(monkeypatch, tmp_path): + """The git-pull update path must invoke the memory-provider refresh.""" + _setup_update_mocks(monkeypatch, tmp_path) + monkeypatch.setattr("shutil.which", lambda name: "/usr/bin/uv" if name == "uv" else None) + monkeypatch.setattr(hermes_main, "_is_termux_env", lambda env=None: False) + + refresh_calls = [] + monkeypatch.setattr( + hermes_main, + "_refresh_active_memory_provider_dependencies", + lambda: refresh_calls.append(True), + ) + + def fake_run(cmd, **kwargs): + if cmd == ["git", "rev-parse", "--abbrev-ref", "HEAD"]: + return SimpleNamespace(stdout="main\n", stderr="", returncode=0) + if cmd == ["git", "rev-list", "HEAD..origin/main", "--count"]: + return SimpleNamespace(stdout="1\n", stderr="", returncode=0) + if cmd == ["git", "pull", "--ff-only", "origin", "main"]: + return SimpleNamespace(stdout="Updating\n", stderr="", returncode=0) + return SimpleNamespace(returncode=0, stdout="", stderr="") + + monkeypatch.setattr(hermes_main.subprocess, "run", fake_run) + + hermes_main.cmd_update(SimpleNamespace()) + + assert refresh_calls == [True] + + def test_install_with_optional_fallback_honors_custom_group(monkeypatch): """Termux update path should target .[termux-all] when requested.""" calls = []