diff --git a/hermes_cli/doctor.py b/hermes_cli/doctor.py index 876ab15d5..361e81d21 100644 --- a/hermes_cli/doctor.py +++ b/hermes_cli/doctor.py @@ -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 diff --git a/tests/hermes_cli/test_doctor.py b/tests/hermes_cli/test_doctor.py index d91cf3f64..f30fb4839 100644 --- a/tests/hermes_cli/test_doctor.py +++ b/tests/hermes_cli/test_doctor.py @@ -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 diff --git a/tests/tools/test_browser_camofox_persistence.py b/tests/tools/test_browser_camofox_persistence.py index 0fa5723c6..0e9c86372 100644 --- a/tests/tools/test_browser_camofox_persistence.py +++ b/tests/tools/test_browser_camofox_persistence.py @@ -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() diff --git a/tests/tools/test_browser_cleanup.py b/tests/tools/test_browser_cleanup.py index df21f3a0e..817927903 100644 --- a/tests/tools/test_browser_cleanup.py +++ b/tests/tools/test_browser_cleanup.py @@ -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 diff --git a/tools/browser_camofox.py b/tools/browser_camofox.py index 226e99b56..3a305bbcb 100644 --- a/tools/browser_camofox.py +++ b/tools/browser_camofox.py @@ -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 # --------------------------------------------------------------------------- diff --git a/tools/browser_tool.py b/tools/browser_tool.py index 012b8eb02..e62a586c1 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -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)