Timmy_Foundation/hermes-agent
15
0
Fork 0
Code Issues 247 Pull Requests 14 Actions 1 Packages Projects Releases Wiki Activity

fix(doctor): only check the active memory provider, not all providers unconditionally (#6285)

Browse Source 
* fix(tools): skip camofox auto-cleanup when managed persistence is enabled

When managed_persistence is enabled, cleanup_browser() was calling
camofox_close() which destroys the server-side browser context via
DELETE /sessions/{userId}, killing login sessions across cron runs.

Add camofox_soft_cleanup() — a public wrapper that drops only the
in-memory session entry when managed persistence is on, returning True.
When persistence is off it returns False so the caller falls back to
the full camofox_close().  The inactivity reaper still handles idle
resource cleanup.

Also surface a logger.warning() when _managed_persistence_enabled()
fails to load config, replacing a silent except-and-return-False.

Salvaged from #6182 by el-analista (Eduardo Perea Fernandez).
Added public API wrapper to avoid cross-module private imports,
and test coverage for both persistence paths.

Co-authored-by: Eduardo Perea Fernandez <el-analista@users.noreply.github.com>

* fix(doctor): only check the active memory provider, not all providers unconditionally

hermes doctor had hardcoded Honcho Memory and Mem0 Memory sections that
always ran regardless of the user's memory.provider config setting. After
the swappable memory provider update (#4623), users with leftover Honcho
config but no active provider saw false 'broken' errors.

Replaced both sections with a single Memory Provider section that reads
memory.provider from config.yaml and only checks the configured provider.
Users with no external provider see a green 'Built-in memory active' check.

Reported by community user michaelruiz001, confirmed by Eri (Honcho).

---------

Co-authored-by: Eduardo Perea Fernandez <el-analista@users.noreply.github.com>
This commit is contained in:
Teknium
2026-04-08 13:44:58 -07:00
committed by GitHub
parent ff6a86cb52
commit 4f467700d4
6 changed files with 269 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

126
hermes_cli/doctor.py
View File

@@ -812,69 +812,83 @@ def run_doctor(args):
check_warn("No GITHUB_TOKEN", f"(60 req/hr rate limit — set in {_DHH}/.env for better rates)")
# =========================================================================
# Honcho memory
# Memory Provider (only check the active provider, if any)
# =========================================================================
print()
print(color("◆ Honcho Memory", Colors.CYAN, Colors.BOLD))
print(color("◆ Memory Provider", Colors.CYAN, Colors.BOLD))
_active_memory_provider = ""
try:
from plugins.memory.honcho.client import HonchoClientConfig, resolve_config_path
hcfg = HonchoClientConfig.from_global_config()
_honcho_cfg_path = resolve_config_path()
import yaml as _yaml
_mem_cfg_path = HERMES_HOME / "config.yaml"
if _mem_cfg_path.exists():
with open(_mem_cfg_path) as _f:
_raw_cfg = _yaml.safe_load(_f) or {}
_active_memory_provider = (_raw_cfg.get("memory") or {}).get("provider", "")
except Exception:
pass
if not _honcho_cfg_path.exists():
check_warn("Honcho config not found", "run: hermes memory setup")
elif not hcfg.enabled:
check_info(f"Honcho disabled (set enabled: true in {_honcho_cfg_path} to activate)")
elif not (hcfg.api_key or hcfg.base_url):
check_fail("Honcho API key or base URL not set", "run: hermes memory setup")
issues.append("No Honcho API key — run 'hermes memory setup'")
else:
from plugins.memory.honcho.client import get_honcho_client, reset_honcho_client
reset_honcho_client()
try:
get_honcho_client(hcfg)
check_ok(
"Honcho connected",
f"workspace={hcfg.workspace_id} mode={hcfg.recall_mode} freq={hcfg.write_frequency}",
)
except Exception as _e:
check_fail("Honcho connection failed", str(_e))
issues.append(f"Honcho unreachable: {_e}")
except ImportError:
check_warn("honcho-ai not installed", "pip install honcho-ai")
except Exception as _e:
check_warn("Honcho check failed", str(_e))
if not _active_memory_provider:
check_ok("Built-in memory active", "(no external provider configured — this is fine)")
elif _active_memory_provider == "honcho":
try:
from plugins.memory.honcho.client import HonchoClientConfig, resolve_config_path
hcfg = HonchoClientConfig.from_global_config()
_honcho_cfg_path = resolve_config_path()
# =========================================================================
# Mem0 memory
# =========================================================================
print()
print(color("◆ Mem0 Memory", Colors.CYAN, Colors.BOLD))
try:
from plugins.memory.mem0 import _load_config as _load_mem0_config
mem0_cfg = _load_mem0_config()
mem0_key = mem0_cfg.get("api_key", "")
if mem0_key:
check_ok("Mem0 API key configured")
check_info(f"user_id={mem0_cfg.get('user_id', '?')} agent_id={mem0_cfg.get('agent_id', '?')}")
# Check if mem0.json exists but is missing api_key (the bug we fixed)
mem0_json = HERMES_HOME / "mem0.json"
if mem0_json.exists():
if not _honcho_cfg_path.exists():
check_warn("Honcho config not found", "run: hermes memory setup")
elif not hcfg.enabled:
check_info(f"Honcho disabled (set enabled: true in {_honcho_cfg_path} to activate)")
elif not (hcfg.api_key or hcfg.base_url):
check_fail("Honcho API key or base URL not set", "run: hermes memory setup")
issues.append("No Honcho API key — run 'hermes memory setup'")
else:
from plugins.memory.honcho.client import get_honcho_client, reset_honcho_client
reset_honcho_client()
try:
import json as _json
file_cfg = _json.loads(mem0_json.read_text())
if not file_cfg.get("api_key") and mem0_key:
check_info("api_key from .env (not in mem0.json) — this is fine")
except Exception:
pass
else:
check_warn("Mem0 not configured", "(set MEM0_API_KEY in .env or run hermes memory setup)")
except ImportError:
check_warn("Mem0 plugin not loadable", "(optional)")
except Exception as _e:
check_warn("Mem0 check failed", str(_e))
get_honcho_client(hcfg)
check_ok(
"Honcho connected",
f"workspace={hcfg.workspace_id} mode={hcfg.recall_mode} freq={hcfg.write_frequency}",
)
except Exception as _e:
check_fail("Honcho connection failed", str(_e))
issues.append(f"Honcho unreachable: {_e}")
except ImportError:
check_fail("honcho-ai not installed", "pip install honcho-ai")
issues.append("Honcho is set as memory provider but honcho-ai is not installed")
except Exception as _e:
check_warn("Honcho check failed", str(_e))
elif _active_memory_provider == "mem0":
try:
from plugins.memory.mem0 import _load_config as _load_mem0_config
mem0_cfg = _load_mem0_config()
mem0_key = mem0_cfg.get("api_key", "")
if mem0_key:
check_ok("Mem0 API key configured")
check_info(f"user_id={mem0_cfg.get('user_id', '?')} agent_id={mem0_cfg.get('agent_id', '?')}")
else:
check_fail("Mem0 API key not set", "(set MEM0_API_KEY in .env or run hermes memory setup)")
issues.append("Mem0 is set as memory provider but API key is missing")
except ImportError:
check_fail("Mem0 plugin not loadable", "pip install mem0ai")
issues.append("Mem0 is set as memory provider but mem0ai is not installed")
except Exception as _e:
check_warn("Mem0 check failed", str(_e))
else:
# Generic check for other memory providers (openviking, hindsight, etc.)
try:
from plugins.memory import load_memory_provider
_provider = load_memory_provider(_active_memory_provider)
if _provider and _provider.is_available():
check_ok(f"{_active_memory_provider} provider active")
elif _provider:
check_warn(f"{_active_memory_provider} configured but not available", "run: hermes memory status")
else:
check_warn(f"{_active_memory_provider} plugin not found", "run: hermes memory setup")
except Exception as _e:
check_warn(f"{_active_memory_provider} check failed", str(_e))
# =========================================================================
# Profiles

70
tests/hermes_cli/test_doctor.py
View File

@@ -136,3 +136,73 @@ def test_check_gateway_service_linger_skips_when_service_not_installed(monkeypat
out = capsys.readouterr().out
assert out == ""
assert issues == []
# ── Memory provider section (doctor should only check the *active* provider) ──
class TestDoctorMemoryProviderSection:
"""The ◆ Memory Provider section should respect memory.provider config."""
def _make_hermes_home(self, tmp_path, provider=""):
"""Create a minimal HERMES_HOME with config.yaml."""
home = tmp_path / ".hermes"
home.mkdir(parents=True, exist_ok=True)
import yaml
config = {"memory": {"provider": provider}} if provider else {"memory": {}}
(home / "config.yaml").write_text(yaml.dump(config))
return home
def _run_doctor_and_capture(self, monkeypatch, tmp_path, provider=""):
"""Run doctor and capture stdout."""
home = self._make_hermes_home(tmp_path, provider)
monkeypatch.setattr(doctor_mod, "HERMES_HOME", home)
monkeypatch.setattr(doctor_mod, "PROJECT_ROOT", tmp_path / "project")
monkeypatch.setattr(doctor_mod, "_DHH", str(home))
(tmp_path / "project").mkdir(exist_ok=True)
# Stub tool availability (returns empty) so doctor runs past it
fake_model_tools = types.SimpleNamespace(
check_tool_availability=lambda *a, **kw: ([], []),
TOOLSET_REQUIREMENTS={},
)
monkeypatch.setitem(sys.modules, "model_tools", fake_model_tools)
# Stub auth checks to avoid real API calls
try:
from hermes_cli import auth as _auth_mod
monkeypatch.setattr(_auth_mod, "get_nous_auth_status", lambda: {})
monkeypatch.setattr(_auth_mod, "get_codex_auth_status", lambda: {})
except Exception:
pass
import io, contextlib
buf = io.StringIO()
with contextlib.redirect_stdout(buf):
doctor_mod.run_doctor(Namespace(fix=False))
return buf.getvalue()
def test_no_provider_shows_builtin_ok(self, monkeypatch, tmp_path):
out = self._run_doctor_and_capture(monkeypatch, tmp_path, provider="")
assert "Memory Provider" in out
assert "Built-in memory active" in out
# Should NOT mention Honcho or Mem0 errors
assert "Honcho API key" not in out
assert "Mem0" not in out
def test_honcho_provider_not_installed_shows_fail(self, monkeypatch, tmp_path):
# Make honcho import fail
monkeypatch.setitem(
sys.modules, "plugins.memory.honcho.client", None
)
out = self._run_doctor_and_capture(monkeypatch, tmp_path, provider="honcho")
assert "Memory Provider" in out
# Should show failure since honcho is set but not importable
assert "Built-in memory active" not in out
def test_mem0_provider_not_installed_shows_fail(self, monkeypatch, tmp_path):
# Make mem0 import fail
monkeypatch.setitem(sys.modules, "plugins.memory.mem0", None)
out = self._run_doctor_and_capture(monkeypatch, tmp_path, provider="mem0")
assert "Memory Provider" in out
assert "Built-in memory active" not in out

48
tests/tools/test_browser_camofox_persistence.py
View File

@@ -16,6 +16,7 @@ from tools.browser_camofox import (
_managed_persistence_enabled,
camofox_close,
camofox_navigate,
camofox_soft_cleanup,
check_camofox_available,
cleanup_all_camofox_sessions,
get_vnc_url,
@@ -240,3 +241,50 @@ class TestVncUrlDiscovery:
assert result["vnc_url"] == "http://localhost:6080"
assert "vnc_hint" in result
class TestCamofoxSoftCleanup:
"""camofox_soft_cleanup drops local state only when managed persistence is on."""
def test_returns_true_and_drops_session_when_enabled(self, tmp_path, monkeypatch):
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
monkeypatch.setenv("CAMOFOX_URL", "http://localhost:9377")
with _enable_persistence():
_get_session("task-1")
result = camofox_soft_cleanup("task-1")
assert result is True
# Session should have been dropped from in-memory store
import tools.browser_camofox as mod
with mod._sessions_lock:
assert "task-1" not in mod._sessions
def test_returns_false_when_disabled(self, tmp_path, monkeypatch):
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
monkeypatch.setenv("CAMOFOX_URL", "http://localhost:9377")
_get_session("task-1")
config = {"browser": {"camofox": {"managed_persistence": False}}}
with patch("tools.browser_camofox.load_config", return_value=config):
result = camofox_soft_cleanup("task-1")
assert result is False
# Session should still be present — not dropped
import tools.browser_camofox as mod
with mod._sessions_lock:
assert "task-1" in mod._sessions
def test_does_not_call_server_delete(self, tmp_path, monkeypatch):
"""Soft cleanup must never hit the Camofox /sessions DELETE endpoint."""
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
monkeypatch.setenv("CAMOFOX_URL", "http://localhost:9377")
with (
_enable_persistence(),
patch("tools.browser_camofox.requests.delete") as mock_delete,
):
_get_session("task-1")
camofox_soft_cleanup("task-1")
mock_delete.assert_not_called()

56
tests/tools/test_browser_cleanup.py
View File

@@ -65,6 +65,62 @@ class TestBrowserCleanup:
mock_stop.assert_called_once_with("task-1")
mock_run.assert_called_once_with("task-1", "close", [], timeout=10)
def test_cleanup_camofox_managed_persistence_skips_close(self):
"""When camofox mode + managed persistence, soft_cleanup fires instead of close."""
browser_tool = self.browser_tool
browser_tool._active_sessions["task-1"] = {
"session_name": "sess-1",
"bb_session_id": None,
}
browser_tool._session_last_activity["task-1"] = 123.0
with (
patch("tools.browser_tool._is_camofox_mode", return_value=True),
patch("tools.browser_tool._maybe_stop_recording") as mock_stop,
patch(
"tools.browser_tool._run_browser_command",
return_value={"success": True},
),
patch("tools.browser_tool.os.path.exists", return_value=False),
patch(
"tools.browser_camofox.camofox_soft_cleanup",
return_value=True,
) as mock_soft,
patch("tools.browser_camofox.camofox_close") as mock_close,
):
browser_tool.cleanup_browser("task-1")
mock_soft.assert_called_once_with("task-1")
mock_close.assert_not_called()
def test_cleanup_camofox_no_persistence_calls_close(self):
"""When camofox mode but managed persistence is off, camofox_close fires."""
browser_tool = self.browser_tool
browser_tool._active_sessions["task-1"] = {
"session_name": "sess-1",
"bb_session_id": None,
}
browser_tool._session_last_activity["task-1"] = 123.0
with (
patch("tools.browser_tool._is_camofox_mode", return_value=True),
patch("tools.browser_tool._maybe_stop_recording") as mock_stop,
patch(
"tools.browser_tool._run_browser_command",
return_value={"success": True},
),
patch("tools.browser_tool.os.path.exists", return_value=False),
patch(
"tools.browser_camofox.camofox_soft_cleanup",
return_value=False,
) as mock_soft,
patch("tools.browser_camofox.camofox_close") as mock_close,
):
browser_tool.cleanup_browser("task-1")
mock_soft.assert_called_once_with("task-1")
mock_close.assert_called_once_with("task-1")
def test_emergency_cleanup_clears_all_tracking_state(self):
browser_tool = self.browser_tool
browser_tool._cleanup_done = False

19
tools/browser_camofox.py
View File

@@ -101,7 +101,8 @@ def _managed_persistence_enabled() -> bool:
"""
try:
camofox_cfg = load_config().get("browser", {}).get("camofox", {})
except Exception:
except Exception as exc:
logger.warning("managed_persistence check failed, defaulting to disabled: %s", exc)
return False
return bool(camofox_cfg.get("managed_persistence"))
@@ -172,6 +173,22 @@ def _drop_session(task_id: Optional[str]) -> Optional[Dict[str, Any]]:
return _sessions.pop(task_id, None)
def camofox_soft_cleanup(task_id: Optional[str] = None) -> bool:
"""Release the in-memory session without destroying the server-side context.
When managed persistence is enabled the browser profile (and its cookies)
must survive across agent tasks. This helper drops only the local tracking
entry and returns ``True``. When managed persistence is *not* enabled it
does nothing and returns ``False`` so the caller can fall back to
:func:`camofox_close`.
"""
if _managed_persistence_enabled():
_drop_session(task_id)
logger.debug("Camofox soft cleanup for task %s (managed persistence)", task_id)
return True
return False
# ---------------------------------------------------------------------------
# HTTP helpers
# ---------------------------------------------------------------------------

10
tools/browser_tool.py
View File

@@ -1935,11 +1935,15 @@ def cleanup_browser(task_id: Optional[str] = None) -> None:
if task_id is None:
task_id = "default"
# Also clean up Camofox session if running in Camofox mode
# Also clean up Camofox session if running in Camofox mode.
# Skip full close when managed persistence is enabled — the browser
# profile (and its session cookies) must survive across agent tasks.
# The inactivity reaper still frees idle resources.
if _is_camofox_mode():
try:
from tools.browser_camofox import camofox_close
camofox_close(task_id)
from tools.browser_camofox import camofox_close, camofox_soft_cleanup
if not camofox_soft_cleanup(task_id):
camofox_close(task_id)
except Exception as e:
logger.debug("Camofox cleanup for task %s: %s", task_id, e)