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

* 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 NousResearch#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 (NousResearch#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>

Author: teknium1

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()
-
-        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))
-
-    # =========================================================================
-    # Mem0 memory
-    # =========================================================================
-    print()
-    print(color("◆ Mem0 Memory", Colors.CYAN, Colors.BOLD))
+        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
 
-    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 _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()
+
+            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

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

tests/tools/test_browser_camofox_persistence.py

@@ -16,6 +16,7 @@
     _managed_persistence_enabled,
     camofox_close,
     camofox_navigate,
+    camofox_soft_cleanup,
     check_camofox_available,
     cleanup_all_camofox_sessions,
     get_vnc_url,
@@ -240,3 +241,50 @@ def test_navigate_includes_vnc_hint(self, tmp_path, monkeypatch):
 
         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()

tests/tools/test_browser_cleanup.py

@@ -65,6 +65,62 @@ def test_cleanup_browser_clears_tracking_state(self):
         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

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
 # ---------------------------------------------------------------------------

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)