mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-27 17:58:07 +00:00
fix(update): refresh active memory provider deps after venv rebuild
Surgical reapply of #53505 by @LeonSGP43 onto current main (stale-base cherry-pick conflicted in main.py): - _refresh_active_memory_provider_dependencies() in hermes_cli/main.py, wired into BOTH the git-pull update path (after lazy refresh) and the ZIP update path — the provider's plugin.yaml bridge packages are not in extras or LAZY_DEPS, so the core reinstall could strip/downgrade them and the update flow never healed them (#53272 mem0ai). - _install_dependencies(force=True) in memory_setup.py: hand every declared spec to the resolver on update so missing AND version-drifted packages are restored (no-op when satisfied). - Skip guards: no provider / builtin store / memory.enabled=false. Widened beyond the original PR for the #70636 half: - _provider_pip_dependencies(): mode-aware expansion — Hindsight in local/local_embedded mode needs hindsight-all (daemon + embedder), not just the declared hindsight-client; setup installs it but plugin.yaml can't express it, so update-time healing previously missed hindsight-embed and the daemon stayed broken. - Spec-aware import probing: version ranges in pip_dependencies (mem0ai>=2.0.10,<3) no longer break the pip-name -> import-name mapping. Fixes #53272. Fixes the hindsight-embed half of #70636.
This commit is contained in:
parent
ab2c9289ca
commit
5645169c8f
3 changed files with 192 additions and 5 deletions
|
|
@ -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")
|
||||
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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 = []
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue