From 2dace37f6b55a6aca189cf0c2562a7c06ffc8356 Mon Sep 17 00:00:00 2001 From: Hao Zhe Date: Wed, 13 May 2026 20:42:18 +0800 Subject: [PATCH 01/11] feat(memory): improve OpenViking setup UX Support linking, copying, and creating ovcli.conf during OpenViking memory setup. Make setup cancellation write nothing and cover OpenViking/Hindsight picker cancellation paths. --- hermes_cli/memory_setup.py | 34 +- plugins/memory/hindsight/__init__.py | 40 ++- plugins/memory/openviking/README.md | 7 + plugins/memory/openviking/__init__.py | 340 ++++++++++++++++-- plugins/memory/openviking/plugin.yaml | 3 +- tests/hermes_cli/test_memory_setup.py | 109 ++++++ .../plugins/memory/test_hindsight_provider.py | 56 +++ .../memory/test_openviking_provider.py | 284 ++++++++++++++- 8 files changed, 818 insertions(+), 55 deletions(-) create mode 100644 tests/hermes_cli/test_memory_setup.py diff --git a/hermes_cli/memory_setup.py b/hermes_cli/memory_setup.py index 2707c77f4ff..8b076da2884 100644 --- a/hermes_cli/memory_setup.py +++ b/hermes_cli/memory_setup.py @@ -15,24 +15,40 @@ from pathlib import Path from hermes_constants import get_hermes_home from hermes_cli.secret_prompt import masked_secret_prompt +_CANCELLED = -1 + # --------------------------------------------------------------------------- # Curses-based interactive picker (same pattern as hermes tools) # --------------------------------------------------------------------------- -def _curses_select(title: str, items: list[tuple[str, str]], default: int = 0) -> int: +def _curses_select( + title: str, + items: list[tuple[str, str]], + default: int = 0, + *, + cancel_returns: int | None = None, +) -> int: """Interactive single-select with arrow keys. items: list of (label, description) tuples. - Returns selected index, or default on escape/quit. + Returns selected index, or cancel_returns/default on escape/quit. """ from hermes_cli.curses_ui import curses_radiolist + + if cancel_returns is None: + cancel_returns = default + # Format (label, desc) tuples into display strings display_items = [ f"{label} {desc}" if desc else label for label, desc in items ] - return curses_radiolist(title, display_items, selected=default, cancel_returns=default) + return curses_radiolist(title, display_items, selected=default, cancel_returns=cancel_returns) + + +def _print_cancelled_setup() -> None: + print("\n Cancelled. No changes saved.\n") def _prompt(label: str, default: str | None = None, secret: bool = False) -> str: @@ -241,14 +257,17 @@ def cmd_setup(args) -> None: items.append(("Built-in only", "— MEMORY.md / USER.md (default)")) builtin_idx = len(items) - 1 - selected = _curses_select("Memory provider setup", items, default=builtin_idx) + selected = _curses_select("Memory provider setup", items, default=builtin_idx, cancel_returns=_CANCELLED) + if selected == _CANCELLED: + _print_cancelled_setup() + return config = load_config() if not isinstance(config.get("memory"), dict): config["memory"] = {} # Built-in only - if selected >= len(providers) or selected < 0: + if selected >= len(providers): config["memory"]["provider"] = "" save_config(config) print("\n ✓ Memory provider: built-in only") @@ -309,7 +328,10 @@ def cmd_setup(args) -> None: current_idx = 0 if current and current in choices: current_idx = choices.index(current) - sel = _curses_select(f" {desc}", choice_items, default=current_idx) + sel = _curses_select(f" {desc}", choice_items, default=current_idx, cancel_returns=_CANCELLED) + if sel == _CANCELLED: + _print_cancelled_setup() + return provider_config[key] = choices[sel] elif is_secret: # Prompt for secret diff --git a/plugins/memory/hindsight/__init__.py b/plugins/memory/hindsight/__init__.py index c26e45a0e11..03ebda28eca 100644 --- a/plugins/memory/hindsight/__init__.py +++ b/plugins/memory/hindsight/__init__.py @@ -702,7 +702,7 @@ class HindsightMemoryProvider(MemoryProvider): from hermes_cli.config import save_config from hermes_cli.secret_prompt import masked_secret_prompt - from hermes_cli.memory_setup import _curses_select + from hermes_cli.memory_setup import _CANCELLED, _curses_select, _print_cancelled_setup print("\n Configuring Hindsight memory:\n") @@ -719,7 +719,10 @@ class HindsightMemoryProvider(MemoryProvider): ] existing_mode = existing_config.get("mode") mode_default_idx = mode_values.index(existing_mode) if existing_mode in mode_values else 0 - mode_idx = _curses_select(" Select mode", mode_items, default=mode_default_idx) + mode_idx = _curses_select(" Select mode", mode_items, default=mode_default_idx, cancel_returns=_CANCELLED) + if mode_idx == _CANCELLED: + _print_cancelled_setup() + return mode = mode_values[mode_idx] provider_config: dict = dict(existing_config) @@ -737,6 +740,27 @@ class HindsightMemoryProvider(MemoryProvider): else: deps_to_install = [cloud_dep] + llm_provider = "" + if mode == "local_embedded": + providers_list = list(_PROVIDER_DEFAULT_MODELS.keys()) + llm_items = [ + (p, f"default model: {_PROVIDER_DEFAULT_MODELS[p]}") + for p in providers_list + ] + existing_llm_provider = provider_config.get("llm_provider") + llm_default_idx = providers_list.index(existing_llm_provider) if existing_llm_provider in providers_list else 0 + llm_idx = _curses_select( + " Select LLM provider", + llm_items, + default=llm_default_idx, + cancel_returns=_CANCELLED, + ) + if llm_idx == _CANCELLED: + _print_cancelled_setup() + return + llm_provider = providers_list[llm_idx] + provider_config["llm_provider"] = llm_provider + print("\n Checking dependencies...") uv_path = shutil.which("uv") if not uv_path: @@ -785,18 +809,6 @@ class HindsightMemoryProvider(MemoryProvider): env_writes["HINDSIGHT_API_KEY"] = api_key else: # local_embedded - providers_list = list(_PROVIDER_DEFAULT_MODELS.keys()) - llm_items = [ - (p, f"default model: {_PROVIDER_DEFAULT_MODELS[p]}") - for p in providers_list - ] - existing_llm_provider = provider_config.get("llm_provider") - llm_default_idx = providers_list.index(existing_llm_provider) if existing_llm_provider in providers_list else 0 - llm_idx = _curses_select(" Select LLM provider", llm_items, default=llm_default_idx) - llm_provider = providers_list[llm_idx] - - provider_config["llm_provider"] = llm_provider - if llm_provider == "openai_compatible": existing_base_url = provider_config.get("llm_base_url", "") prompt = " LLM endpoint URL (e.g. http://192.168.1.10:8080/v1)" diff --git a/plugins/memory/openviking/README.md b/plugins/memory/openviking/README.md index 07e9484d4dd..0b6be37c0a7 100644 --- a/plugins/memory/openviking/README.md +++ b/plugins/memory/openviking/README.md @@ -14,6 +14,10 @@ Context database by Volcengine (ByteDance) with filesystem-style knowledge hiera hermes memory setup # select "openviking" ``` +The setup can link to an existing `~/.openviking/ovcli.conf`, copy its current +connection values into Hermes, or create a minimal `ovcli.conf` when one does +not exist. + Or manually: ```bash hermes config set memory.provider openviking @@ -28,6 +32,9 @@ All config via environment variables in `.env`: |---------|---------|-------------| | `OPENVIKING_ENDPOINT` | `http://127.0.0.1:1933` | Server URL | | `OPENVIKING_API_KEY` | (none) | API key (optional) | +| `OPENVIKING_ACCOUNT` | (none) | Tenant account override | +| `OPENVIKING_USER` | (none) | Tenant user override | +| `OPENVIKING_AGENT` | `hermes` | Tenant agent namespace | ## Tools diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index 810f2db43e0..07df0e7d888 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -7,12 +7,13 @@ automatic memory extraction, and session management. Original PR #3369 by Mibayy, rewritten to use the full OpenViking session lifecycle instead of read-only search endpoints. -Config via environment variables (profile-scoped via each profile's .env): +Config via environment variables (profile-scoped via each profile's .env) +or a linked OpenViking CLI config: OPENVIKING_ENDPOINT — Server URL (default: http://127.0.0.1:1933) OPENVIKING_API_KEY — API key (required for authenticated servers) - OPENVIKING_ACCOUNT — Tenant account (default: default) - OPENVIKING_USER — Tenant user (default: default) - OPENVIKING_AGENT — Tenant agent (default: hermes) + OPENVIKING_ACCOUNT — Optional tenant account override + OPENVIKING_USER — Optional tenant user override + OPENVIKING_AGENT — Tenant agent (default: hermes) Capabilities: - Automatic memory extraction on session commit (6 categories) @@ -44,6 +45,18 @@ from tools.registry import tool_error logger = logging.getLogger(__name__) _DEFAULT_ENDPOINT = "http://127.0.0.1:1933" +_DEFAULT_ACCOUNT = "" +_DEFAULT_USER = "" +_DEFAULT_AGENT = "hermes" +_OVCLI_CONFIG_ENV = "OPENVIKING_CLI_CONFIG_FILE" +_OVCLI_DEFAULT_RELATIVE_PATH = ".openviking/ovcli.conf" +_OPENVIKING_ENV_KEYS = ( + "OPENVIKING_ENDPOINT", + "OPENVIKING_API_KEY", + "OPENVIKING_ACCOUNT", + "OPENVIKING_USER", + "OPENVIKING_AGENT", +) _TIMEOUT = 30.0 _REMOTE_RESOURCE_PREFIXES = ("http://", "https://", "git@", "ssh://", "git://") @@ -108,27 +121,21 @@ class _VikingClient: """Thin HTTP client for the OpenViking REST API.""" def __init__(self, endpoint: str, api_key: str = "", - account: str = "", user: str = "", agent: str = ""): + account: Optional[str] = None, user: Optional[str] = None, + agent: Optional[str] = None): self._endpoint = endpoint.rstrip("/") self._api_key = api_key - self._account = account or os.environ.get("OPENVIKING_ACCOUNT", "default") - self._user = user or os.environ.get("OPENVIKING_USER", "default") - self._agent = agent or os.environ.get("OPENVIKING_AGENT", "hermes") + self._account = account if account is not None else os.environ.get("OPENVIKING_ACCOUNT", _DEFAULT_ACCOUNT) + self._user = user if user is not None else os.environ.get("OPENVIKING_USER", _DEFAULT_USER) + self._agent = agent if agent is not None else os.environ.get("OPENVIKING_AGENT", _DEFAULT_AGENT) self._httpx = _get_httpx() if self._httpx is None: raise ImportError("httpx is required for OpenViking: pip install httpx") def _headers(self) -> dict: - # Always send tenant headers when account/user are configured. - # OpenViking 0.3.x requires X-OpenViking-Account and X-OpenViking-User - # for ROOT API key requests to tenant-scoped APIs — omitting them - # causes INVALID_ARGUMENT errors even when account="default". - # User-level keys can omit them (server derives tenancy from the key), - # but ROOT keys must always include them explicitly. - h = { - "Content-Type": "application/json", - "X-OpenViking-Agent": self._agent, - } + h = {"Content-Type": "application/json"} + if self._agent: + h["X-OpenViking-Agent"] = self._agent if self._account: h["X-OpenViking-Account"] = self._account if self._user: @@ -405,6 +412,156 @@ def _path_from_file_uri(uri: str) -> Path | str: return Path(url2pathname(parsed.path)).expanduser() +def _clean_config_value(value: Any) -> str: + return value.strip() if isinstance(value, str) else "" + + +def _default_ovcli_config_path() -> Path: + return Path.home() / _OVCLI_DEFAULT_RELATIVE_PATH + + +def _resolve_ovcli_config_path(config_path: str = "") -> Path: + if config_path: + return Path(config_path).expanduser() + env_path = os.environ.get(_OVCLI_CONFIG_ENV, "").strip() + if env_path: + return Path(env_path).expanduser() + return _default_ovcli_config_path() + + +def _load_ovcli_config(path: Optional[Path] = None) -> dict: + config_path = path or _resolve_ovcli_config_path() + if not config_path.exists(): + return {} + with config_path.open(encoding="utf-8") as f: + data = json.load(f) + if not isinstance(data, dict): + raise ValueError(f"OpenViking CLI config must be a JSON object: {config_path}") + return data + + +def _connection_values_from_ovcli(data: dict) -> dict: + return { + "endpoint": _clean_config_value(data.get("url")) or _DEFAULT_ENDPOINT, + "api_key": _clean_config_value(data.get("api_key")), + "account": _clean_config_value(data.get("account") or data.get("account_id")), + "user": _clean_config_value(data.get("user") or data.get("user_id")), + "agent": _clean_config_value(data.get("agent_id")), + } + + +def _load_hermes_openviking_config() -> dict: + try: + from hermes_cli.config import load_config + + config = load_config() + memory_config = config.get("memory", {}) if isinstance(config, dict) else {} + provider_config = memory_config.get("openviking", {}) if isinstance(memory_config, dict) else {} + return dict(provider_config) if isinstance(provider_config, dict) else {} + except Exception: + return {} + + +def _env_value(name: str) -> Optional[str]: + return os.environ[name].strip() if name in os.environ else None + + +def _first_nonempty(*values: Optional[str], default: str = "") -> str: + for value in values: + if value: + return value + return default + + +def _resolve_connection_settings(provider_config: Optional[dict] = None) -> dict: + provider_config = dict(provider_config or {}) + ovcli_values: dict = {} + if provider_config.get("use_ovcli_config"): + ovcli_path = _resolve_ovcli_config_path(str(provider_config.get("ovcli_config_path") or "")) + ovcli_values = _connection_values_from_ovcli(_load_ovcli_config(ovcli_path)) + + endpoint_env = _env_value("OPENVIKING_ENDPOINT") + api_key_env = _env_value("OPENVIKING_API_KEY") + account_env = _env_value("OPENVIKING_ACCOUNT") + user_env = _env_value("OPENVIKING_USER") + agent_env = _env_value("OPENVIKING_AGENT") + + return { + "endpoint": _first_nonempty(endpoint_env, ovcli_values.get("endpoint"), default=_DEFAULT_ENDPOINT), + "api_key": api_key_env if api_key_env is not None else ovcli_values.get("api_key", ""), + "account": account_env if account_env is not None else ovcli_values.get("account", ""), + "user": user_env if user_env is not None else ovcli_values.get("user", ""), + "agent": _first_nonempty(agent_env, ovcli_values.get("agent"), default=_DEFAULT_AGENT), + } + + +def _env_writes_from_connection_values(values: dict) -> dict: + writes = {} + mapping = { + "OPENVIKING_ENDPOINT": "endpoint", + "OPENVIKING_API_KEY": "api_key", + "OPENVIKING_ACCOUNT": "account", + "OPENVIKING_USER": "user", + "OPENVIKING_AGENT": "agent", + } + for env_key, value_key in mapping.items(): + value = _clean_config_value(values.get(value_key)) + if value: + writes[env_key] = value + return writes + + +def _write_env_vars(env_path: Path, env_writes: dict, remove_keys: tuple[str, ...] = ()) -> None: + env_path.parent.mkdir(parents=True, exist_ok=True) + remove_set = set(remove_keys) - set(env_writes) + existing_lines = env_path.read_text(encoding="utf-8").splitlines() if env_path.exists() else [] + updated_keys = set() + new_lines = [] + for line in existing_lines: + key_match = line.split("=", 1)[0].strip() if "=" in line else "" + if key_match in remove_set: + continue + if key_match in env_writes: + new_lines.append(f"{key_match}={env_writes[key_match]}") + updated_keys.add(key_match) + else: + new_lines.append(line) + for key, val in env_writes.items(): + if key not in updated_keys: + new_lines.append(f"{key}={val}") + env_path.write_text("\n".join(new_lines) + ("\n" if new_lines else ""), encoding="utf-8") + + +def _remember_ovcli_path(provider_config: dict, ovcli_path: Path) -> None: + default_path = _default_ovcli_config_path().expanduser() + if os.environ.get(_OVCLI_CONFIG_ENV, "").strip() or ovcli_path.expanduser() != default_path: + provider_config["ovcli_config_path"] = str(ovcli_path) + else: + provider_config.pop("ovcli_config_path", None) + + +def _ovcli_data_from_connection_values(values: dict) -> dict: + data = {"url": _clean_config_value(values.get("endpoint")) or _DEFAULT_ENDPOINT} + api_key = _clean_config_value(values.get("api_key")) + account = _clean_config_value(values.get("account")) + user = _clean_config_value(values.get("user")) + agent = _clean_config_value(values.get("agent")) or _DEFAULT_AGENT + if api_key: + data["api_key"] = api_key + if account: + data["account"] = account + if user: + data["user"] = user + if agent: + data["agent_id"] = agent + return data + + +def _write_ovcli_config(path: Path, values: dict) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(_ovcli_data_from_connection_values(values), indent=2) + "\n", encoding="utf-8") + + # --------------------------------------------------------------------------- # MemoryProvider implementation # --------------------------------------------------------------------------- @@ -429,7 +586,16 @@ class OpenVikingMemoryProvider(MemoryProvider): def is_available(self) -> bool: """Check if OpenViking endpoint is configured. No network calls.""" - return bool(os.environ.get("OPENVIKING_ENDPOINT")) + if os.environ.get("OPENVIKING_ENDPOINT"): + return True + provider_config = _load_hermes_openviking_config() + if not provider_config.get("use_ovcli_config"): + return False + try: + ovcli_path = _resolve_ovcli_config_path(str(provider_config.get("ovcli_config_path") or "")) + return bool(_connection_values_from_ovcli(_load_ovcli_config(ovcli_path)).get("endpoint")) + except Exception: + return False def get_config_schema(self): return [ @@ -448,14 +614,12 @@ class OpenVikingMemoryProvider(MemoryProvider): }, { "key": "account", - "description": "OpenViking tenant account ID ([default], used when local mode, OPENVIKING_API_KEY is empty)", - "default": "default", + "description": "OpenViking tenant account ID (blank for user API keys)", "env_var": "OPENVIKING_ACCOUNT", }, { "key": "user", - "description": "OpenViking user ID within the account ([default], used when local mode, OPENVIKING_API_KEY is empty)", - "default": "default", + "description": "OpenViking user ID within the account (blank for user API keys)", "env_var": "OPENVIKING_USER", }, { @@ -466,12 +630,132 @@ class OpenVikingMemoryProvider(MemoryProvider): }, ] + def post_setup(self, hermes_home: str, config: dict) -> None: + """Custom setup that can reuse OpenViking's shared CLI config.""" + from hermes_cli.config import save_config + from hermes_cli.memory_setup import _CANCELLED, _curses_select, _print_cancelled_setup, _prompt + + hermes_home_path = Path(hermes_home) + env_path = hermes_home_path / ".env" + if not isinstance(config.get("memory"), dict): + config["memory"] = {} + provider_config = config["memory"].get("openviking", {}) + if not isinstance(provider_config, dict): + provider_config = {} + + ovcli_path = _resolve_ovcli_config_path(str(provider_config.get("ovcli_config_path") or "")) + + print("\n Configuring OpenViking memory:\n") + + if ovcli_path.exists(): + try: + ovcli_values = _connection_values_from_ovcli(_load_ovcli_config(ovcli_path)) + except Exception as e: + print(f"\n Could not read OpenViking CLI config: {e}") + print(" No changes saved.\n") + return + + setup_options = [ + ("Link to ovcli.conf", "Hermes follows the active OpenViking CLI config"), + ("Copy once", "Hermes won't follow future ovcli.conf changes"), + ] + choice = _curses_select( + " OpenViking config source", + setup_options, + default=0, + cancel_returns=_CANCELLED, + ) + if choice == _CANCELLED: + _print_cancelled_setup() + return + + if choice == 0: + provider_config["use_ovcli_config"] = True + _remember_ovcli_path(provider_config, ovcli_path) + _write_env_vars(env_path, {}, remove_keys=_OPENVIKING_ENV_KEYS) + config["memory"]["provider"] = "openviking" + config["memory"]["openviking"] = provider_config + save_config(config) + print(f"\n Memory provider: openviking") + print(f" Linked config: {ovcli_path}") + print(" Start a new session to activate.\n") + return + + provider_config["use_ovcli_config"] = False + provider_config.pop("ovcli_config_path", None) + config["memory"]["provider"] = "openviking" + config["memory"]["openviking"] = provider_config + save_config(config) + _write_env_vars( + env_path, + _env_writes_from_connection_values(ovcli_values), + remove_keys=_OPENVIKING_ENV_KEYS, + ) + print(f"\n Memory provider: openviking") + print(" Connection saved to .env") + print(" Start a new session to activate.\n") + return + + setup_options = [ + ("Create ovcli.conf and link", "Recommended"), + ("Configure Hermes only", "Write OpenViking values to Hermes .env"), + ] + choice = _curses_select( + " OpenViking config source", + setup_options, + default=0, + cancel_returns=_CANCELLED, + ) + if choice == _CANCELLED: + _print_cancelled_setup() + return + + defaults = { + "endpoint": _DEFAULT_ENDPOINT, + "api_key": "", + "account": "", + "user": "", + "agent": _DEFAULT_AGENT, + } + values = { + "endpoint": _prompt("OpenViking server URL", default=defaults["endpoint"]), + "api_key": _prompt("OpenViking API key", secret=True), + "account": _prompt("OpenViking account", default=defaults["account"]), + "user": _prompt("OpenViking user", default=defaults["user"]), + "agent": _prompt("OpenViking agent", default=defaults["agent"]), + } + + config["memory"]["provider"] = "openviking" + if choice == 0: + _write_ovcli_config(ovcli_path, values) + provider_config["use_ovcli_config"] = True + _remember_ovcli_path(provider_config, ovcli_path) + config["memory"]["openviking"] = provider_config + save_config(config) + _write_env_vars(env_path, {}, remove_keys=_OPENVIKING_ENV_KEYS) + print(f"\n Memory provider: openviking") + print(f" Created config: {ovcli_path}") + else: + provider_config["use_ovcli_config"] = False + provider_config.pop("ovcli_config_path", None) + config["memory"]["openviking"] = provider_config + save_config(config) + _write_env_vars( + env_path, + _env_writes_from_connection_values(values), + remove_keys=_OPENVIKING_ENV_KEYS, + ) + print(f"\n Memory provider: openviking") + print(" Connection saved to .env") + print(" Start a new session to activate.\n") + def initialize(self, session_id: str, **kwargs) -> None: - self._endpoint = os.environ.get("OPENVIKING_ENDPOINT", _DEFAULT_ENDPOINT) - self._api_key = os.environ.get("OPENVIKING_API_KEY", "") - self._account = os.environ.get("OPENVIKING_ACCOUNT", "default") - self._user = os.environ.get("OPENVIKING_USER", "default") - self._agent = os.environ.get("OPENVIKING_AGENT", "hermes") + settings = _resolve_connection_settings(_load_hermes_openviking_config()) + self._endpoint = settings["endpoint"] + self._api_key = settings["api_key"] + self._account = settings["account"] + self._user = settings["user"] + self._agent = settings["agent"] self._session_id = session_id self._turn_count = 0 diff --git a/plugins/memory/openviking/plugin.yaml b/plugins/memory/openviking/plugin.yaml index 714877f9763..18b8ea78741 100644 --- a/plugins/memory/openviking/plugin.yaml +++ b/plugins/memory/openviking/plugin.yaml @@ -3,7 +3,6 @@ version: 2.0.0 description: "OpenViking context database — session-managed memory with automatic extraction, tiered retrieval, and filesystem-style knowledge browsing." pip_dependencies: - httpx -requires_env: - - OPENVIKING_ENDPOINT +requires_env: [] hooks: - on_session_end diff --git a/tests/hermes_cli/test_memory_setup.py b/tests/hermes_cli/test_memory_setup.py new file mode 100644 index 00000000000..b458a1d2d6a --- /dev/null +++ b/tests/hermes_cli/test_memory_setup.py @@ -0,0 +1,109 @@ +from types import SimpleNamespace +from unittest.mock import MagicMock + +import hermes_cli.memory_setup as memory_setup +from hermes_cli.memory_setup import _CANCELLED, _curses_select + + +def test_curses_select_cancel_defaults_to_selected(monkeypatch): + captured = {} + + def fake_radiolist(title, items, selected=0, *, cancel_returns=None): + captured.update({ + "title": title, + "items": items, + "selected": selected, + "cancel_returns": cancel_returns, + }) + return cancel_returns + + monkeypatch.setattr("hermes_cli.curses_ui.curses_radiolist", fake_radiolist) + + result = _curses_select("Pick one", [("first", "desc"), ("second", "")], default=1) + + assert result == 1 + assert captured == { + "title": "Pick one", + "items": ["first desc", "second"], + "selected": 1, + "cancel_returns": 1, + } + + +def test_curses_select_accepts_explicit_cancel_value(monkeypatch): + captured = {} + + def fake_radiolist(title, items, selected=0, *, cancel_returns=None): + captured["cancel_returns"] = cancel_returns + return cancel_returns + + monkeypatch.setattr("hermes_cli.curses_ui.curses_radiolist", fake_radiolist) + + result = _curses_select("Pick one", [("first", "")], default=0, cancel_returns=_CANCELLED) + + assert result == _CANCELLED + assert captured["cancel_returns"] == _CANCELLED + + +def test_cmd_setup_top_level_cancel_writes_nothing(monkeypatch): + save_config = MagicMock() + load_config = MagicMock(side_effect=AssertionError("cancel should not load config")) + + monkeypatch.setattr(memory_setup, "_get_available_providers", lambda: [("fake", "local", object())]) + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: kwargs["cancel_returns"]) + monkeypatch.setattr("hermes_cli.config.load_config", load_config) + monkeypatch.setattr("hermes_cli.config.save_config", save_config) + + memory_setup.cmd_setup(SimpleNamespace()) + + load_config.assert_not_called() + save_config.assert_not_called() + + +def test_cmd_setup_builtin_selection_still_saves_builtin(monkeypatch): + save_config = MagicMock() + config = {"memory": {"provider": "openviking"}} + providers = [("fake", "local", object())] + + monkeypatch.setattr(memory_setup, "_get_available_providers", lambda: providers) + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: len(providers)) + monkeypatch.setattr("hermes_cli.config.load_config", lambda: config) + monkeypatch.setattr("hermes_cli.config.save_config", save_config) + + memory_setup.cmd_setup(SimpleNamespace()) + + assert config["memory"]["provider"] == "" + save_config.assert_called_once_with(config) + + +def test_cmd_setup_generic_choice_cancel_writes_nothing(tmp_path, monkeypatch): + class ChoiceProvider: + def __init__(self): + self.save_config = MagicMock() + + def get_config_schema(self): + return [{ + "key": "mode", + "description": "Mode", + "default": "one", + "choices": ["one", "two"], + }] + + provider = ChoiceProvider() + selections = iter([0, _CANCELLED]) + save_config = MagicMock() + install_dependencies = MagicMock() + + monkeypatch.setattr(memory_setup, "_get_available_providers", lambda: [("fake", "local", provider)]) + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: next(selections)) + monkeypatch.setattr(memory_setup, "_install_dependencies", install_dependencies) + monkeypatch.setattr(memory_setup, "get_hermes_home", lambda: tmp_path) + monkeypatch.setattr("hermes_cli.config.load_config", lambda: {"memory": {}}) + monkeypatch.setattr("hermes_cli.config.save_config", save_config) + + memory_setup.cmd_setup(SimpleNamespace()) + + install_dependencies.assert_called_once_with("fake") + save_config.assert_not_called() + provider.save_config.assert_not_called() + assert not (tmp_path / ".env").exists() diff --git a/tests/plugins/memory/test_hindsight_provider.py b/tests/plugins/memory/test_hindsight_provider.py index b121a2bb20b..bbcb151baa9 100644 --- a/tests/plugins/memory/test_hindsight_provider.py +++ b/tests/plugins/memory/test_hindsight_provider.py @@ -15,6 +15,7 @@ from unittest.mock import AsyncMock, MagicMock import pytest +from hermes_cli.memory_setup import _CANCELLED from plugins.memory.hindsight import ( HindsightMemoryProvider, RECALL_SCHEMA, @@ -376,6 +377,61 @@ class TestConfig: class TestPostSetup: + def test_setup_cancel_at_mode_picker_writes_nothing(self, tmp_path, monkeypatch): + hermes_home = tmp_path / "hermes-home" + user_home = tmp_path / "user-home" + user_home.mkdir() + monkeypatch.setenv("HOME", str(user_home)) + monkeypatch.setattr("plugins.memory.hindsight.get_hermes_home", lambda: hermes_home) + + save_config = MagicMock() + which = MagicMock(return_value="/usr/bin/uv") + run = MagicMock() + monkeypatch.setattr("hermes_cli.memory_setup._curses_select", lambda *args, **kwargs: _CANCELLED) + monkeypatch.setattr("shutil.which", which) + monkeypatch.setattr("subprocess.run", run) + monkeypatch.setattr("builtins.input", MagicMock(side_effect=AssertionError("prompt should not run"))) + monkeypatch.setattr("getpass.getpass", MagicMock(side_effect=AssertionError("prompt should not run"))) + monkeypatch.setattr("hermes_cli.config.save_config", save_config) + + provider = HindsightMemoryProvider() + provider.post_setup(str(hermes_home), {"memory": {"provider": "builtin"}}) + + save_config.assert_not_called() + which.assert_not_called() + run.assert_not_called() + assert not (hermes_home / ".env").exists() + assert not (hermes_home / "hindsight" / "config.json").exists() + assert not (user_home / ".hindsight" / "profiles" / "hermes.env").exists() + + def test_local_embedded_setup_cancel_at_llm_picker_writes_nothing(self, tmp_path, monkeypatch): + hermes_home = tmp_path / "hermes-home" + user_home = tmp_path / "user-home" + user_home.mkdir() + monkeypatch.setenv("HOME", str(user_home)) + monkeypatch.setattr("plugins.memory.hindsight.get_hermes_home", lambda: hermes_home) + + selections = iter([1, _CANCELLED]) # local_embedded, then cancel LLM picker + save_config = MagicMock() + which = MagicMock(return_value="/usr/bin/uv") + run = MagicMock() + monkeypatch.setattr("hermes_cli.memory_setup._curses_select", lambda *args, **kwargs: next(selections)) + monkeypatch.setattr("shutil.which", which) + monkeypatch.setattr("subprocess.run", run) + monkeypatch.setattr("builtins.input", MagicMock(side_effect=AssertionError("prompt should not run"))) + monkeypatch.setattr("getpass.getpass", MagicMock(side_effect=AssertionError("prompt should not run"))) + monkeypatch.setattr("hermes_cli.config.save_config", save_config) + + provider = HindsightMemoryProvider() + provider.post_setup(str(hermes_home), {"memory": {"provider": "builtin"}}) + + save_config.assert_not_called() + which.assert_not_called() + run.assert_not_called() + assert not (hermes_home / ".env").exists() + assert not (hermes_home / "hindsight" / "config.json").exists() + assert not (user_home / ".hindsight" / "profiles" / "hermes.env").exists() + def test_local_embedded_setup_materializes_profile_env(self, tmp_path, monkeypatch): hermes_home = tmp_path / "hermes-home" user_home = tmp_path / "user-home" diff --git a/tests/plugins/memory/test_openviking_provider.py b/tests/plugins/memory/test_openviking_provider.py index 3f609cd1d67..754f7ff617c 100644 --- a/tests/plugins/memory/test_openviking_provider.py +++ b/tests/plugins/memory/test_openviking_provider.py @@ -8,6 +8,281 @@ import pytest from plugins.memory.openviking import OpenVikingMemoryProvider, _VikingClient +def _clear_openviking_env(monkeypatch): + for key in ( + "OPENVIKING_ENDPOINT", + "OPENVIKING_API_KEY", + "OPENVIKING_ACCOUNT", + "OPENVIKING_USER", + "OPENVIKING_AGENT", + "OPENVIKING_CLI_CONFIG_FILE", + ): + monkeypatch.delenv(key, raising=False) + + +def test_linked_ovcli_config_is_read_at_runtime(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + ovcli_path = tmp_path / "ovcli.conf" + ovcli_path.write_text( + json.dumps({ + "url": "http://openviking-one.local", + "api_key": "key-one", + "account": "acct-one", + "user": "alice", + "agent_id": "agent-one", + }), + encoding="utf-8", + ) + provider_config = {"use_ovcli_config": True, "ovcli_config_path": str(ovcli_path)} + + settings = openviking_module._resolve_connection_settings(provider_config) + + assert settings == { + "endpoint": "http://openviking-one.local", + "api_key": "key-one", + "account": "acct-one", + "user": "alice", + "agent": "agent-one", + } + + ovcli_path.write_text( + json.dumps({ + "url": "http://openviking-two.local", + "api_key": "key-two", + "agent_id": "agent-two", + }), + encoding="utf-8", + ) + + settings = openviking_module._resolve_connection_settings(provider_config) + + assert settings == { + "endpoint": "http://openviking-two.local", + "api_key": "key-two", + "account": "", + "user": "", + "agent": "agent-two", + } + + +def test_openviking_env_overrides_linked_ovcli_config(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + ovcli_path = tmp_path / "ovcli.conf" + ovcli_path.write_text( + json.dumps({ + "url": "http://openviking.local", + "api_key": "file-key", + "account": "file-account", + "user": "file-user", + "agent_id": "file-agent", + }), + encoding="utf-8", + ) + monkeypatch.setenv("OPENVIKING_ENDPOINT", "http://env.local") + monkeypatch.setenv("OPENVIKING_API_KEY", "env-key") + monkeypatch.setenv("OPENVIKING_ACCOUNT", "env-account") + monkeypatch.setenv("OPENVIKING_USER", "env-user") + monkeypatch.setenv("OPENVIKING_AGENT", "env-agent") + + settings = openviking_module._resolve_connection_settings({ + "use_ovcli_config": True, + "ovcli_config_path": str(ovcli_path), + }) + + assert settings == { + "endpoint": "http://env.local", + "api_key": "env-key", + "account": "env-account", + "user": "env-user", + "agent": "env-agent", + } + + +def test_post_setup_link_existing_ovcli_clears_hermes_env(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + env_path = hermes_home / ".env" + env_path.write_text( + "OPENVIKING_ENDPOINT=http://old.local\n" + "OPENVIKING_ACCOUNT=old-account\n" + "OTHER_KEY=keep\n", + encoding="utf-8", + ) + ovcli_path = tmp_path / "ovcli.conf" + ovcli_path.write_text(json.dumps({"url": "http://openviking.local"}), encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + + from hermes_cli import memory_setup + + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: 0) + config = {"memory": {}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + assert config["memory"]["provider"] == "openviking" + assert config["memory"]["openviking"]["use_ovcli_config"] is True + assert config["memory"]["openviking"]["ovcli_config_path"] == str(ovcli_path) + env_text = env_path.read_text(encoding="utf-8") + assert "OPENVIKING_" not in env_text + assert "OTHER_KEY=keep" in env_text + + +def test_post_setup_copy_existing_ovcli_writes_hermes_env(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + ovcli_path = tmp_path / "ovcli.conf" + ovcli_path.write_text( + json.dumps({ + "url": "http://openviking.local", + "api_key": "test-key", + "account": "acct", + "user": "alice", + "agent_id": "agent", + }), + encoding="utf-8", + ) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + + from hermes_cli import memory_setup + + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: 1) + config = {"memory": {}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + assert config["memory"]["provider"] == "openviking" + assert config["memory"]["openviking"]["use_ovcli_config"] is False + env_text = (hermes_home / ".env").read_text(encoding="utf-8") + assert "OPENVIKING_ENDPOINT=http://openviking.local" in env_text + assert "OPENVIKING_API_KEY=test-key" in env_text + assert "OPENVIKING_ACCOUNT=acct" in env_text + assert "OPENVIKING_USER=alice" in env_text + assert "OPENVIKING_AGENT=agent" in env_text + + +def test_post_setup_cancel_existing_ovcli_writes_nothing(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + env_path = hermes_home / ".env" + original_env = "OPENVIKING_ENDPOINT=http://old.local\nOTHER_KEY=keep\n" + env_path.write_text(original_env, encoding="utf-8") + ovcli_path = tmp_path / "ovcli.conf" + ovcli_path.write_text(json.dumps({"url": "http://openviking.local"}), encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + + from hermes_cli import config as hermes_config + from hermes_cli import memory_setup + + save_config = MagicMock() + monkeypatch.setattr(hermes_config, "save_config", save_config) + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: -1) + config = {"memory": {"provider": "builtin"}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + save_config.assert_not_called() + assert config == {"memory": {"provider": "builtin"}} + assert env_path.read_text(encoding="utf-8") == original_env + + +def test_post_setup_invalid_existing_ovcli_writes_nothing(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + env_path = hermes_home / ".env" + original_env = "OPENVIKING_ENDPOINT=http://old.local\nOTHER_KEY=keep\n" + env_path.write_text(original_env, encoding="utf-8") + ovcli_path = tmp_path / "ovcli.conf" + ovcli_path.write_text("{", encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + + from hermes_cli import config as hermes_config + from hermes_cli import memory_setup + + save_config = MagicMock() + monkeypatch.setattr(hermes_config, "save_config", save_config) + monkeypatch.setattr( + memory_setup, + "_curses_select", + MagicMock(side_effect=AssertionError("picker should not open for invalid ovcli.conf")), + ) + config = {"memory": {"provider": "builtin"}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + save_config.assert_not_called() + assert config == {"memory": {"provider": "builtin"}} + assert env_path.read_text(encoding="utf-8") == original_env + + +def test_post_setup_creates_minimal_ovcli_and_links(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + ovcli_path = tmp_path / "missing" / "ovcli.conf" + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + + from hermes_cli import memory_setup + + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: 0) + monkeypatch.setattr( + memory_setup, + "_prompt", + lambda label, default=None, secret=False: default or "", + ) + config = {"memory": {}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + assert config["memory"]["provider"] == "openviking" + assert config["memory"]["openviking"]["use_ovcli_config"] is True + data = json.loads(ovcli_path.read_text(encoding="utf-8")) + assert data == { + "url": "http://127.0.0.1:1933", + "agent_id": "hermes", + } + env_path = hermes_home / ".env" + if env_path.exists(): + assert env_path.read_text(encoding="utf-8") == "" + + +def test_post_setup_cancel_missing_ovcli_does_not_prompt_or_create(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + ovcli_path = tmp_path / "missing" / "ovcli.conf" + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + + from hermes_cli import config as hermes_config + from hermes_cli import memory_setup + + save_config = MagicMock() + monkeypatch.setattr(hermes_config, "save_config", save_config) + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: -1) + monkeypatch.setattr( + memory_setup, + "_prompt", + MagicMock(side_effect=AssertionError("prompts should not run after cancel")), + ) + config = {"memory": {"provider": "builtin"}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + save_config.assert_not_called() + assert config == {"memory": {"provider": "builtin"}} + assert not ovcli_path.exists() + assert not (hermes_home / ".env").exists() + + def test_tool_search_sorts_by_raw_score_across_buckets(): provider = OpenVikingMemoryProvider() provider._client = MagicMock() @@ -371,9 +646,7 @@ def test_viking_client_headers_send_tenant_when_default(): assert headers["Authorization"] == "Bearer test-key" -def test_viking_client_headers_send_tenant_when_empty_falls_back_to_default(): - # Empty account/user strings fall back to "default" via the constructor. - # Headers are sent even for the default value — ROOT API keys need them. +def test_viking_client_headers_omit_tenant_when_empty(): client = _VikingClient( "https://example.com", api_key="", @@ -382,8 +655,9 @@ def test_viking_client_headers_send_tenant_when_empty_falls_back_to_default(): agent="hermes", ) headers = client._headers() - assert headers["X-OpenViking-Account"] == "default" - assert headers["X-OpenViking-User"] == "default" + assert "X-OpenViking-Account" not in headers + assert "X-OpenViking-User" not in headers + assert headers["X-OpenViking-Agent"] == "hermes" assert "Authorization" not in headers assert "X-API-Key" not in headers From b0e25c9cb29517a4cd829a50182289f1e7c261ff Mon Sep 17 00:00:00 2001 From: Hao Zhe Date: Wed, 20 May 2026 13:25:29 +0800 Subject: [PATCH 02/11] fix(memory): restrict OpenViking setup file permissions --- plugins/memory/openviking/__init__.py | 10 ++++++++ .../memory/test_openviking_provider.py | 23 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index 07df0e7d888..92775810e87 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -30,6 +30,7 @@ import json import logging import mimetypes import os +import stat import tempfile import threading import uuid @@ -511,6 +512,13 @@ def _env_writes_from_connection_values(values: dict) -> dict: return writes +def _restrict_secret_file_permissions(path: Path) -> None: + try: + path.chmod(stat.S_IRUSR | stat.S_IWUSR) + except OSError: + pass + + def _write_env_vars(env_path: Path, env_writes: dict, remove_keys: tuple[str, ...] = ()) -> None: env_path.parent.mkdir(parents=True, exist_ok=True) remove_set = set(remove_keys) - set(env_writes) @@ -530,6 +538,7 @@ def _write_env_vars(env_path: Path, env_writes: dict, remove_keys: tuple[str, .. if key not in updated_keys: new_lines.append(f"{key}={val}") env_path.write_text("\n".join(new_lines) + ("\n" if new_lines else ""), encoding="utf-8") + _restrict_secret_file_permissions(env_path) def _remember_ovcli_path(provider_config: dict, ovcli_path: Path) -> None: @@ -560,6 +569,7 @@ def _ovcli_data_from_connection_values(values: dict) -> dict: def _write_ovcli_config(path: Path, values: dict) -> None: path.parent.mkdir(parents=True, exist_ok=True) path.write_text(json.dumps(_ovcli_data_from_connection_values(values), indent=2) + "\n", encoding="utf-8") + _restrict_secret_file_permissions(path) # --------------------------------------------------------------------------- diff --git a/tests/plugins/memory/test_openviking_provider.py b/tests/plugins/memory/test_openviking_provider.py index 754f7ff617c..ce6f7515507 100644 --- a/tests/plugins/memory/test_openviking_provider.py +++ b/tests/plugins/memory/test_openviking_provider.py @@ -1,4 +1,6 @@ import json +import os +import stat import zipfile from types import SimpleNamespace from unittest.mock import MagicMock @@ -20,6 +22,27 @@ def _clear_openviking_env(monkeypatch): monkeypatch.delenv(key, raising=False) +@pytest.mark.skipif(os.name == "nt", reason="POSIX file modes") +def test_openviking_env_writer_restricts_file_permissions(tmp_path): + env_path = tmp_path / ".env" + + openviking_module._write_env_vars(env_path, {"OPENVIKING_API_KEY": "secret"}) + + assert stat.S_IMODE(env_path.stat().st_mode) == 0o600 + + +@pytest.mark.skipif(os.name == "nt", reason="POSIX file modes") +def test_ovcli_config_writer_restricts_file_permissions(tmp_path): + config_path = tmp_path / "ovcli.conf" + + openviking_module._write_ovcli_config( + config_path, + {"endpoint": "http://remote.example", "api_key": "secret"}, + ) + + assert stat.S_IMODE(config_path.stat().st_mode) == 0o600 + + def test_linked_ovcli_config_is_read_at_runtime(tmp_path, monkeypatch): _clear_openviking_env(monkeypatch) ovcli_path = tmp_path / "ovcli.conf" From 7f76cf719557d699840593e5a8a3f6c2866cb1ba Mon Sep 17 00:00:00 2001 From: Hao Zhe Date: Wed, 20 May 2026 14:16:35 +0800 Subject: [PATCH 03/11] fix(memory): smooth setup transition after provider selection --- hermes_cli/memory_setup.py | 10 ++++++++++ tests/hermes_cli/test_memory_setup.py | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/hermes_cli/memory_setup.py b/hermes_cli/memory_setup.py index 8b076da2884..86e5e1cd6de 100644 --- a/hermes_cli/memory_setup.py +++ b/hermes_cli/memory_setup.py @@ -51,6 +51,14 @@ def _print_cancelled_setup() -> None: print("\n Cancelled. No changes saved.\n") +def _clear_interactive_transition() -> None: + """Clear stale curses content before entering a follow-up setup screen.""" + if not sys.stdout.isatty(): + return + sys.stdout.write("\033[2J\033[H") + sys.stdout.flush() + + def _prompt(label: str, default: str | None = None, secret: bool = False) -> str: """Prompt for a value with optional default and secret masking.""" suffix = f" [{default}]" if default else "" @@ -276,6 +284,8 @@ def cmd_setup(args) -> None: name, _, provider = providers[selected] + _clear_interactive_transition() + # Install pip dependencies if declared in plugin.yaml _install_dependencies(name) diff --git a/tests/hermes_cli/test_memory_setup.py b/tests/hermes_cli/test_memory_setup.py index b458a1d2d6a..689dc670cb4 100644 --- a/tests/hermes_cli/test_memory_setup.py +++ b/tests/hermes_cli/test_memory_setup.py @@ -76,6 +76,25 @@ def test_cmd_setup_builtin_selection_still_saves_builtin(monkeypatch): save_config.assert_called_once_with(config) +def test_cmd_setup_clears_interactive_picker_before_provider_post_setup(monkeypatch): + events = [] + + class PostSetupProvider: + def post_setup(self, hermes_home, config): + events.append("post_setup") + + monkeypatch.setattr(memory_setup, "_get_available_providers", lambda: [("openviking", "local", PostSetupProvider())]) + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: events.append("select") or 0) + monkeypatch.setattr(memory_setup, "_clear_interactive_transition", lambda: events.append("clear"), raising=False) + monkeypatch.setattr(memory_setup, "_install_dependencies", lambda name: events.append("install")) + monkeypatch.setattr(memory_setup, "get_hermes_home", lambda: "/tmp/hermes-test") + monkeypatch.setattr("hermes_cli.config.load_config", lambda: {"memory": {}}) + + memory_setup.cmd_setup(SimpleNamespace()) + + assert events == ["select", "clear", "install", "post_setup"] + + def test_cmd_setup_generic_choice_cancel_writes_nothing(tmp_path, monkeypatch): class ChoiceProvider: def __init__(self): From 70f53f36cb1c2af69c834b931b1fd5680008dd5d Mon Sep 17 00:00:00 2001 From: Hao Zhe Date: Tue, 26 May 2026 12:33:01 +0800 Subject: [PATCH 04/11] feat(memory): add manual OpenViking setup path --- plugins/memory/openviking/__init__.py | 139 +++++++++- .../memory/test_openviking_provider.py | 248 +++++++++++++++++- 2 files changed, 365 insertions(+), 22 deletions(-) diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index 92775810e87..c5bda3e5d0d 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -79,6 +79,8 @@ _MEMORY_WRITE_TARGET_SUBDIR_MAP = { "user": "preferences", "memory": "patterns", } +_LOCAL_OPENVIKING_HOSTS = {"localhost", "127.0.0.1", "::1"} +_SETUP_CANCELLED = object() # --------------------------------------------------------------------------- @@ -451,6 +453,16 @@ def _connection_values_from_ovcli(data: dict) -> dict: } +def _is_local_openviking_url(value: str) -> bool: + candidate = _clean_config_value(value) + if not candidate: + return False + if "://" not in candidate: + candidate = f"//{candidate}" + parsed = urlparse(candidate) + return (parsed.hostname or "").lower() in _LOCAL_OPENVIKING_HOSTS + + def _load_hermes_openviking_config() -> dict: try: from hermes_cli.config import load_config @@ -552,11 +564,17 @@ def _remember_ovcli_path(provider_config: dict, ovcli_path: Path) -> None: def _ovcli_data_from_connection_values(values: dict) -> dict: data = {"url": _clean_config_value(values.get("endpoint")) or _DEFAULT_ENDPOINT} api_key = _clean_config_value(values.get("api_key")) + api_key_type = _clean_config_value(values.get("api_key_type")) + root_api_key = _clean_config_value(values.get("root_api_key")) account = _clean_config_value(values.get("account")) user = _clean_config_value(values.get("user")) agent = _clean_config_value(values.get("agent")) or _DEFAULT_AGENT if api_key: data["api_key"] = api_key + if root_api_key: + data["root_api_key"] = root_api_key + elif api_key and api_key_type == "root": + data["root_api_key"] = api_key if account: data["account"] = account if user: @@ -572,6 +590,58 @@ def _write_ovcli_config(path: Path, values: dict) -> None: _restrict_secret_file_permissions(path) +def _prompt_manual_connection_values(prompt, select, cancelled): + endpoint = _clean_config_value( + prompt("OpenViking server URL", default=_DEFAULT_ENDPOINT) + ) or _DEFAULT_ENDPOINT + is_local = _is_local_openviking_url(endpoint) + api_key_label = ( + "OpenViking API key (optional for local)" + if is_local + else "OpenViking API key" + ) + api_key = _clean_config_value(prompt(api_key_label, secret=True)) + if not api_key and not is_local: + print("\n Remote OpenViking servers require an API key.") + print(" No changes saved.\n") + return None + + values = { + "endpoint": endpoint, + "api_key": api_key, + "account": "", + "user": "", + "agent": "", + } + if api_key: + key_type = select( + " OpenViking API key type", + [ + ("User API key", "server derives account/user automatically"), + ("Root API key", "requires account and user IDs"), + ], + default=0, + cancel_returns=cancelled, + ) + if key_type == cancelled: + return _SETUP_CANCELLED + if key_type == 1: + values["api_key_type"] = "root" + values["account"] = _clean_config_value(prompt("OpenViking account")) + values["user"] = _clean_config_value(prompt("OpenViking user")) + if not values["account"] or not values["user"]: + print("\n Root API keys require both OpenViking account and user.") + print(" No changes saved.\n") + return None + else: + values["api_key_type"] = "user" + + values["agent"] = _clean_config_value( + prompt("OpenViking agent", default=_DEFAULT_AGENT) + ) or _DEFAULT_AGENT + return values + + # --------------------------------------------------------------------------- # MemoryProvider implementation # --------------------------------------------------------------------------- @@ -668,6 +738,7 @@ class OpenVikingMemoryProvider(MemoryProvider): setup_options = [ ("Link to ovcli.conf", "Hermes follows the active OpenViking CLI config"), ("Copy once", "Hermes won't follow future ovcli.conf changes"), + ("Manual Setup", "Enter a new URL/API key"), ] choice = _curses_select( " OpenViking config source", @@ -691,18 +762,64 @@ class OpenVikingMemoryProvider(MemoryProvider): print(" Start a new session to activate.\n") return - provider_config["use_ovcli_config"] = False - provider_config.pop("ovcli_config_path", None) - config["memory"]["provider"] = "openviking" - config["memory"]["openviking"] = provider_config - save_config(config) - _write_env_vars( - env_path, - _env_writes_from_connection_values(ovcli_values), - remove_keys=_OPENVIKING_ENV_KEYS, + if choice == 1: + provider_config["use_ovcli_config"] = False + provider_config.pop("ovcli_config_path", None) + config["memory"]["provider"] = "openviking" + config["memory"]["openviking"] = provider_config + save_config(config) + _write_env_vars( + env_path, + _env_writes_from_connection_values(ovcli_values), + remove_keys=_OPENVIKING_ENV_KEYS, + ) + print(f"\n Memory provider: openviking") + print(" Connection saved to .env") + print(" Start a new session to activate.\n") + return + + values = _prompt_manual_connection_values(_prompt, _curses_select, _CANCELLED) + if values is _SETUP_CANCELLED: + _print_cancelled_setup() + return + if values is None: + return + + save_choice = _curses_select( + " Save OpenViking config", + [ + ("Write ovcli.conf and link", "Hermes and ov use this config"), + ("Keep within Hermes", "Write values only to Hermes .env"), + ], + default=1, + cancel_returns=_CANCELLED, ) - print(f"\n Memory provider: openviking") - print(" Connection saved to .env") + if save_choice == _CANCELLED: + _print_cancelled_setup() + return + + config["memory"]["provider"] = "openviking" + if save_choice == 0: + _write_ovcli_config(ovcli_path, values) + provider_config["use_ovcli_config"] = True + _remember_ovcli_path(provider_config, ovcli_path) + config["memory"]["openviking"] = provider_config + save_config(config) + _write_env_vars(env_path, {}, remove_keys=_OPENVIKING_ENV_KEYS) + print(f"\n Memory provider: openviking") + print(f" Updated config: {ovcli_path}") + else: + provider_config["use_ovcli_config"] = False + provider_config.pop("ovcli_config_path", None) + config["memory"]["openviking"] = provider_config + save_config(config) + _write_env_vars( + env_path, + _env_writes_from_connection_values(values), + remove_keys=_OPENVIKING_ENV_KEYS, + ) + print(f"\n Memory provider: openviking") + print(" Connection saved to .env") print(" Start a new session to activate.\n") return diff --git a/tests/plugins/memory/test_openviking_provider.py b/tests/plugins/memory/test_openviking_provider.py index ce6f7515507..b4e42093e00 100644 --- a/tests/plugins/memory/test_openviking_provider.py +++ b/tests/plugins/memory/test_openviking_provider.py @@ -22,6 +22,17 @@ def _clear_openviking_env(monkeypatch): monkeypatch.delenv(key, raising=False) +def _prompt_from_values(values: dict[str, str], *, forbidden: set[str] | None = None): + forbidden = forbidden or set() + + def _prompt(label, default=None, secret=False): + if label in forbidden: + raise AssertionError(f"{label} should not be prompted") + return values.get(label, default or "") + + return _prompt + + @pytest.mark.skipif(os.name == "nt", reason="POSIX file modes") def test_openviking_env_writer_restricts_file_permissions(tmp_path): env_path = tmp_path / ".env" @@ -133,7 +144,8 @@ def test_post_setup_link_existing_ovcli_clears_hermes_env(tmp_path, monkeypatch) encoding="utf-8", ) ovcli_path = tmp_path / "ovcli.conf" - ovcli_path.write_text(json.dumps({"url": "http://openviking.local"}), encoding="utf-8") + original_ovcli = json.dumps({"url": "http://openviking.local"}) + ovcli_path.write_text(original_ovcli, encoding="utf-8") monkeypatch.setenv("HERMES_HOME", str(hermes_home)) monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) @@ -150,6 +162,7 @@ def test_post_setup_link_existing_ovcli_clears_hermes_env(tmp_path, monkeypatch) env_text = env_path.read_text(encoding="utf-8") assert "OPENVIKING_" not in env_text assert "OTHER_KEY=keep" in env_text + assert ovcli_path.read_text(encoding="utf-8") == original_ovcli def test_post_setup_copy_existing_ovcli_writes_hermes_env(tmp_path, monkeypatch): @@ -157,16 +170,14 @@ def test_post_setup_copy_existing_ovcli_writes_hermes_env(tmp_path, monkeypatch) hermes_home = tmp_path / "hermes" hermes_home.mkdir() ovcli_path = tmp_path / "ovcli.conf" - ovcli_path.write_text( - json.dumps({ - "url": "http://openviking.local", - "api_key": "test-key", - "account": "acct", - "user": "alice", - "agent_id": "agent", - }), - encoding="utf-8", - ) + original_ovcli = json.dumps({ + "url": "http://openviking.local", + "api_key": "test-key", + "account": "acct", + "user": "alice", + "agent_id": "agent", + }) + ovcli_path.write_text(original_ovcli, encoding="utf-8") monkeypatch.setenv("HERMES_HOME", str(hermes_home)) monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) @@ -185,6 +196,221 @@ def test_post_setup_copy_existing_ovcli_writes_hermes_env(tmp_path, monkeypatch) assert "OPENVIKING_ACCOUNT=acct" in env_text assert "OPENVIKING_USER=alice" in env_text assert "OPENVIKING_AGENT=agent" in env_text + assert ovcli_path.read_text(encoding="utf-8") == original_ovcli + + +def test_post_setup_manual_remote_root_writes_ovcli_and_links(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + env_path = hermes_home / ".env" + env_path.write_text("OPENVIKING_ENDPOINT=http://old.local\n", encoding="utf-8") + ovcli_path = tmp_path / "ovcli.conf" + ovcli_path.write_text(json.dumps({"url": "http://old.local"}), encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + + from hermes_cli import memory_setup + + choices = iter([2, 1, 0]) + monkeypatch.setattr( + memory_setup, + "_curses_select", + lambda *args, **kwargs: next(choices), + ) + monkeypatch.setattr( + memory_setup, + "_prompt", + _prompt_from_values({ + "OpenViking server URL": "https://openviking.example", + "OpenViking API key": "root-secret", + "OpenViking account": "acct", + "OpenViking user": "alice", + "OpenViking agent": "agent", + }), + ) + config = {"memory": {}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + assert config["memory"]["provider"] == "openviking" + assert config["memory"]["openviking"]["use_ovcli_config"] is True + assert config["memory"]["openviking"]["ovcli_config_path"] == str(ovcli_path) + assert env_path.read_text(encoding="utf-8") == "" + data = json.loads(ovcli_path.read_text(encoding="utf-8")) + assert data == { + "url": "https://openviking.example", + "api_key": "root-secret", + "root_api_key": "root-secret", + "account": "acct", + "user": "alice", + "agent_id": "agent", + } + + +def test_post_setup_manual_remote_user_keeps_only_hermes_env(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + ovcli_path = tmp_path / "ovcli.conf" + original_ovcli = json.dumps({"url": "http://old.local"}) + ovcli_path.write_text(original_ovcli, encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + + from hermes_cli import memory_setup + + choices = iter([2, 0, 1]) + monkeypatch.setattr( + memory_setup, + "_curses_select", + lambda *args, **kwargs: next(choices), + ) + monkeypatch.setattr( + memory_setup, + "_prompt", + _prompt_from_values( + { + "OpenViking server URL": "https://openviking.example", + "OpenViking API key": "user-secret", + "OpenViking agent": "agent", + }, + forbidden={"OpenViking account", "OpenViking user"}, + ), + ) + config = {"memory": {}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + assert config["memory"]["provider"] == "openviking" + assert config["memory"]["openviking"]["use_ovcli_config"] is False + assert ovcli_path.read_text(encoding="utf-8") == original_ovcli + env_text = (hermes_home / ".env").read_text(encoding="utf-8") + assert "OPENVIKING_ENDPOINT=https://openviking.example" in env_text + assert "OPENVIKING_API_KEY=user-secret" in env_text + assert "OPENVIKING_AGENT=agent" in env_text + assert "OPENVIKING_ACCOUNT" not in env_text + assert "OPENVIKING_USER" not in env_text + + +def test_post_setup_manual_remote_requires_api_key(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + ovcli_path = tmp_path / "ovcli.conf" + original_ovcli = json.dumps({"url": "http://old.local"}) + ovcli_path.write_text(original_ovcli, encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + + from hermes_cli import config as hermes_config + from hermes_cli import memory_setup + + save_config = MagicMock() + monkeypatch.setattr(hermes_config, "save_config", save_config) + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: 2) + monkeypatch.setattr( + memory_setup, + "_prompt", + _prompt_from_values({ + "OpenViking server URL": "https://openviking.example", + "OpenViking API key": "", + }), + ) + config = {"memory": {"provider": "builtin"}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + save_config.assert_not_called() + assert config == {"memory": {"provider": "builtin"}} + assert ovcli_path.read_text(encoding="utf-8") == original_ovcli + assert not (hermes_home / ".env").exists() + + +def test_post_setup_manual_root_requires_account_and_user(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + ovcli_path = tmp_path / "ovcli.conf" + original_ovcli = json.dumps({"url": "http://old.local"}) + ovcli_path.write_text(original_ovcli, encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + + from hermes_cli import config as hermes_config + from hermes_cli import memory_setup + + save_config = MagicMock() + choices = iter([2, 1]) + monkeypatch.setattr(hermes_config, "save_config", save_config) + monkeypatch.setattr( + memory_setup, + "_curses_select", + lambda *args, **kwargs: next(choices), + ) + monkeypatch.setattr( + memory_setup, + "_prompt", + _prompt_from_values({ + "OpenViking server URL": "https://openviking.example", + "OpenViking API key": "root-secret", + "OpenViking account": "", + "OpenViking user": "alice", + }), + ) + config = {"memory": {"provider": "builtin"}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + save_config.assert_not_called() + assert config == {"memory": {"provider": "builtin"}} + assert ovcli_path.read_text(encoding="utf-8") == original_ovcli + assert not (hermes_home / ".env").exists() + + +def test_post_setup_manual_local_allows_blank_api_key(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + ovcli_path = tmp_path / "ovcli.conf" + original_ovcli = json.dumps({"url": "http://old.local"}) + ovcli_path.write_text(original_ovcli, encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + + from hermes_cli import memory_setup + + choices = iter([2, 1]) + monkeypatch.setattr( + memory_setup, + "_curses_select", + lambda *args, **kwargs: next(choices), + ) + monkeypatch.setattr( + memory_setup, + "_prompt", + _prompt_from_values( + { + "OpenViking server URL": "http://localhost:1933", + "OpenViking API key": "", + "OpenViking agent": "agent", + }, + forbidden={"OpenViking account", "OpenViking user"}, + ), + ) + config = {"memory": {}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + assert config["memory"]["provider"] == "openviking" + assert config["memory"]["openviking"]["use_ovcli_config"] is False + assert ovcli_path.read_text(encoding="utf-8") == original_ovcli + env_text = (hermes_home / ".env").read_text(encoding="utf-8") + assert "OPENVIKING_ENDPOINT=http://localhost:1933" in env_text + assert "OPENVIKING_AGENT=agent" in env_text + assert "OPENVIKING_API_KEY" not in env_text + assert "OPENVIKING_ACCOUNT" not in env_text + assert "OPENVIKING_USER" not in env_text def test_post_setup_cancel_existing_ovcli_writes_nothing(tmp_path, monkeypatch): From 94523764fca8b94c4c4f32abc514c0fd5cce1764 Mon Sep 17 00:00:00 2001 From: Hao Zhe Date: Tue, 26 May 2026 12:37:28 +0800 Subject: [PATCH 05/11] fix(memory): choose OpenViking key type before prompting --- plugins/memory/openviking/__init__.py | 68 ++++++++++++------- .../memory/test_openviking_provider.py | 31 ++++++--- 2 files changed, 66 insertions(+), 33 deletions(-) diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index c5bda3e5d0d..29b3e5ad7c4 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -595,26 +595,35 @@ def _prompt_manual_connection_values(prompt, select, cancelled): prompt("OpenViking server URL", default=_DEFAULT_ENDPOINT) ) or _DEFAULT_ENDPOINT is_local = _is_local_openviking_url(endpoint) - api_key_label = ( - "OpenViking API key (optional for local)" - if is_local - else "OpenViking API key" - ) - api_key = _clean_config_value(prompt(api_key_label, secret=True)) - if not api_key and not is_local: - print("\n Remote OpenViking servers require an API key.") - print(" No changes saved.\n") - return None values = { "endpoint": endpoint, - "api_key": api_key, + "api_key": "", "account": "", "user": "", "agent": "", } - if api_key: - key_type = select( + if is_local: + credential_choice = select( + " OpenViking credential", + [ + ("No API key", "local dev mode"), + ("User API key", "server derives account/user automatically"), + ("Root API key", "requires account and user IDs"), + ], + default=0, + cancel_returns=cancelled, + ) + if credential_choice == cancelled: + return _SETUP_CANCELLED + if credential_choice == 0: + values["agent"] = _clean_config_value( + prompt("OpenViking agent", default=_DEFAULT_AGENT) + ) or _DEFAULT_AGENT + return values + api_key_type = "root" if credential_choice == 2 else "user" + else: + credential_choice = select( " OpenViking API key type", [ ("User API key", "server derives account/user automatically"), @@ -623,18 +632,29 @@ def _prompt_manual_connection_values(prompt, select, cancelled): default=0, cancel_returns=cancelled, ) - if key_type == cancelled: + if credential_choice == cancelled: return _SETUP_CANCELLED - if key_type == 1: - values["api_key_type"] = "root" - values["account"] = _clean_config_value(prompt("OpenViking account")) - values["user"] = _clean_config_value(prompt("OpenViking user")) - if not values["account"] or not values["user"]: - print("\n Root API keys require both OpenViking account and user.") - print(" No changes saved.\n") - return None - else: - values["api_key_type"] = "user" + api_key_type = "root" if credential_choice == 1 else "user" + + values["api_key_type"] = api_key_type + api_key_label = ( + "OpenViking root API key" + if api_key_type == "root" + else "OpenViking user API key" + ) + values["api_key"] = _clean_config_value(prompt(api_key_label, secret=True)) + if not values["api_key"]: + print(f"\n {api_key_label} is required.") + print(" No changes saved.\n") + return None + + if api_key_type == "root": + values["account"] = _clean_config_value(prompt("OpenViking account")) + values["user"] = _clean_config_value(prompt("OpenViking user")) + if not values["account"] or not values["user"]: + print("\n Root API keys require both OpenViking account and user.") + print(" No changes saved.\n") + return None values["agent"] = _clean_config_value( prompt("OpenViking agent", default=_DEFAULT_AGENT) diff --git a/tests/plugins/memory/test_openviking_provider.py b/tests/plugins/memory/test_openviking_provider.py index b4e42093e00..2ca648d3228 100644 --- a/tests/plugins/memory/test_openviking_provider.py +++ b/tests/plugins/memory/test_openviking_provider.py @@ -223,7 +223,7 @@ def test_post_setup_manual_remote_root_writes_ovcli_and_links(tmp_path, monkeypa "_prompt", _prompt_from_values({ "OpenViking server URL": "https://openviking.example", - "OpenViking API key": "root-secret", + "OpenViking root API key": "root-secret", "OpenViking account": "acct", "OpenViking user": "alice", "OpenViking agent": "agent", @@ -272,10 +272,14 @@ def test_post_setup_manual_remote_user_keeps_only_hermes_env(tmp_path, monkeypat _prompt_from_values( { "OpenViking server URL": "https://openviking.example", - "OpenViking API key": "user-secret", + "OpenViking user API key": "user-secret", "OpenViking agent": "agent", }, - forbidden={"OpenViking account", "OpenViking user"}, + forbidden={ + "OpenViking account", + "OpenViking root API key", + "OpenViking user", + }, ), ) config = {"memory": {}} @@ -308,13 +312,18 @@ def test_post_setup_manual_remote_requires_api_key(tmp_path, monkeypatch): save_config = MagicMock() monkeypatch.setattr(hermes_config, "save_config", save_config) - monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: 2) + choices = iter([2, 0]) + monkeypatch.setattr( + memory_setup, + "_curses_select", + lambda *args, **kwargs: next(choices), + ) monkeypatch.setattr( memory_setup, "_prompt", _prompt_from_values({ "OpenViking server URL": "https://openviking.example", - "OpenViking API key": "", + "OpenViking user API key": "", }), ) config = {"memory": {"provider": "builtin"}} @@ -353,7 +362,7 @@ def test_post_setup_manual_root_requires_account_and_user(tmp_path, monkeypatch) "_prompt", _prompt_from_values({ "OpenViking server URL": "https://openviking.example", - "OpenViking API key": "root-secret", + "OpenViking root API key": "root-secret", "OpenViking account": "", "OpenViking user": "alice", }), @@ -380,7 +389,7 @@ def test_post_setup_manual_local_allows_blank_api_key(tmp_path, monkeypatch): from hermes_cli import memory_setup - choices = iter([2, 1]) + choices = iter([2, 0, 1]) monkeypatch.setattr( memory_setup, "_curses_select", @@ -392,10 +401,14 @@ def test_post_setup_manual_local_allows_blank_api_key(tmp_path, monkeypatch): _prompt_from_values( { "OpenViking server URL": "http://localhost:1933", - "OpenViking API key": "", "OpenViking agent": "agent", }, - forbidden={"OpenViking account", "OpenViking user"}, + forbidden={ + "OpenViking account", + "OpenViking root API key", + "OpenViking user", + "OpenViking user API key", + }, ), ) config = {"memory": {}} From a893d77d8d0bb542b710ccc41425efc09569a73c Mon Sep 17 00:00:00 2001 From: Hao Zhe Date: Tue, 26 May 2026 12:40:39 +0800 Subject: [PATCH 06/11] fix(memory): separate setup option descriptions --- hermes_cli/memory_setup.py | 2 +- tests/hermes_cli/test_memory_setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hermes_cli/memory_setup.py b/hermes_cli/memory_setup.py index 86e5e1cd6de..cd2a1b4406f 100644 --- a/hermes_cli/memory_setup.py +++ b/hermes_cli/memory_setup.py @@ -41,7 +41,7 @@ def _curses_select( # Format (label, desc) tuples into display strings display_items = [ - f"{label} {desc}" if desc else label + f"{label} - {desc}" if desc else label for label, desc in items ] return curses_radiolist(title, display_items, selected=default, cancel_returns=cancel_returns) diff --git a/tests/hermes_cli/test_memory_setup.py b/tests/hermes_cli/test_memory_setup.py index 689dc670cb4..1e75a5a2add 100644 --- a/tests/hermes_cli/test_memory_setup.py +++ b/tests/hermes_cli/test_memory_setup.py @@ -24,7 +24,7 @@ def test_curses_select_cancel_defaults_to_selected(monkeypatch): assert result == 1 assert captured == { "title": "Pick one", - "items": ["first desc", "second"], + "items": ["first - desc", "second"], "selected": 1, "cancel_returns": 1, } From 2b972472cee873032f48e51c26e85a0badf36caa Mon Sep 17 00:00:00 2001 From: Hao Zhe Date: Tue, 26 May 2026 14:30:45 +0800 Subject: [PATCH 07/11] fix(memory): validate OpenViking manual setup steps --- plugins/memory/openviking/__init__.py | 285 ++++++++--- .../memory/test_openviking_provider.py | 460 +++++++++++++++++- 2 files changed, 671 insertions(+), 74 deletions(-) diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index 29b3e5ad7c4..c3180305fb0 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -223,6 +223,14 @@ class _VikingClient: except Exception: return False + def validate_auth(self) -> dict: + """Validate authenticated OpenViking access without mutating state.""" + return self.get("/api/v1/system/status") + + def validate_root_access(self) -> dict: + """Validate ROOT access against a read-only admin endpoint.""" + return self.get("/api/v1/admin/accounts") + # --------------------------------------------------------------------------- # Tool schemas @@ -564,17 +572,11 @@ def _remember_ovcli_path(provider_config: dict, ovcli_path: Path) -> None: def _ovcli_data_from_connection_values(values: dict) -> dict: data = {"url": _clean_config_value(values.get("endpoint")) or _DEFAULT_ENDPOINT} api_key = _clean_config_value(values.get("api_key")) - api_key_type = _clean_config_value(values.get("api_key_type")) - root_api_key = _clean_config_value(values.get("root_api_key")) account = _clean_config_value(values.get("account")) user = _clean_config_value(values.get("user")) agent = _clean_config_value(values.get("agent")) or _DEFAULT_AGENT if api_key: data["api_key"] = api_key - if root_api_key: - data["root_api_key"] = root_api_key - elif api_key and api_key_type == "root": - data["root_api_key"] = api_key if account: data["account"] = account if user: @@ -590,76 +592,217 @@ def _write_ovcli_config(path: Path, values: dict) -> None: _restrict_secret_file_permissions(path) -def _prompt_manual_connection_values(prompt, select, cancelled): - endpoint = _clean_config_value( - prompt("OpenViking server URL", default=_DEFAULT_ENDPOINT) - ) or _DEFAULT_ENDPOINT - is_local = _is_local_openviking_url(endpoint) +def _validate_openviking_reachability(endpoint: str) -> tuple[bool, str]: + endpoint = _clean_config_value(endpoint) or _DEFAULT_ENDPOINT + try: + client = _VikingClient(endpoint) + if client.health(): + return True, "" + except Exception as e: + return False, f"OpenViking server is not reachable at {endpoint}: {e}" + return False, f"OpenViking server is not reachable at {endpoint}." - values = { - "endpoint": endpoint, - "api_key": "", - "account": "", - "user": "", - "agent": "", - } - if is_local: - credential_choice = select( - " OpenViking credential", - [ - ("No API key", "local dev mode"), - ("User API key", "server derives account/user automatically"), - ("Root API key", "requires account and user IDs"), - ], - default=0, - cancel_returns=cancelled, - ) - if credential_choice == cancelled: - return _SETUP_CANCELLED - if credential_choice == 0: - values["agent"] = _clean_config_value( - prompt("OpenViking agent", default=_DEFAULT_AGENT) - ) or _DEFAULT_AGENT - return values - api_key_type = "root" if credential_choice == 2 else "user" - else: - credential_choice = select( - " OpenViking API key type", - [ - ("User API key", "server derives account/user automatically"), - ("Root API key", "requires account and user IDs"), - ], - default=0, - cancel_returns=cancelled, - ) - if credential_choice == cancelled: - return _SETUP_CANCELLED - api_key_type = "root" if credential_choice == 1 else "user" - values["api_key_type"] = api_key_type - api_key_label = ( - "OpenViking root API key" - if api_key_type == "root" - else "OpenViking user API key" +def _validate_openviking_auth(values: dict) -> tuple[bool, str]: + endpoint = _clean_config_value(values.get("endpoint")) or _DEFAULT_ENDPOINT + try: + client = _VikingClient( + endpoint, + _clean_config_value(values.get("api_key")), + account=_clean_config_value(values.get("account")), + user=_clean_config_value(values.get("user")), + agent=_clean_config_value(values.get("agent")) or _DEFAULT_AGENT, + ) + client.validate_auth() + except Exception as e: + return False, f"OpenViking authentication validation failed: {e}" + return True, "" + + +def _validate_openviking_root_access(values: dict) -> tuple[bool, str]: + endpoint = _clean_config_value(values.get("endpoint")) or _DEFAULT_ENDPOINT + try: + client = _VikingClient( + endpoint, + _clean_config_value(values.get("api_key")), + agent=_clean_config_value(values.get("agent")) or _DEFAULT_AGENT, + ) + client.validate_root_access() + except Exception as e: + return False, f"OpenViking root API key validation failed: {e}" + return True, "" + + +def _validate_openviking_user_key_scope(values: dict) -> tuple[bool, str]: + root_ok, _message = _validate_openviking_root_access(values) + if not root_ok: + return True, "" + return ( + False, + "That key has ROOT access. Choose Root API key and provide account/user, " + "or enter a user API key.", ) - values["api_key"] = _clean_config_value(prompt(api_key_label, secret=True)) - if not values["api_key"]: - print(f"\n {api_key_label} is required.") - print(" No changes saved.\n") - return None - if api_key_type == "root": - values["account"] = _clean_config_value(prompt("OpenViking account")) - values["user"] = _clean_config_value(prompt("OpenViking user")) - if not values["account"] or not values["user"]: - print("\n Root API keys require both OpenViking account and user.") - print(" No changes saved.\n") - return None - values["agent"] = _clean_config_value( - prompt("OpenViking agent", default=_DEFAULT_AGENT) - ) or _DEFAULT_AGENT - return values +def _retry_or_cancel_manual_setup(select, title: str, message: str, cancelled): + print(f" {message}") + choice = select( + title, + [ + ("Retry", "try this step again"), + ("Cancel setup", "no changes saved"), + ], + default=0, + cancel_returns=cancelled, + ) + if choice == 0: + return True + return _SETUP_CANCELLED + + +def _prompt_manual_connection_values(prompt, select, cancelled): + while True: + endpoint = _clean_config_value( + prompt("OpenViking server URL", default=_DEFAULT_ENDPOINT) + ) or _DEFAULT_ENDPOINT + reachable, message = _validate_openviking_reachability(endpoint) + if reachable: + print(" OpenViking server is reachable.") + break + retry = _retry_or_cancel_manual_setup( + select, + " OpenViking server unreachable", + message, + cancelled, + ) + if retry is _SETUP_CANCELLED: + return _SETUP_CANCELLED + + is_local = _is_local_openviking_url(endpoint) + while True: + values = { + "endpoint": endpoint, + "api_key": "", + "account": "", + "user": "", + "agent": "", + } + if is_local: + credential_choice = select( + " OpenViking credential", + [ + ("No API key", "local dev mode"), + ("User API key", "server derives account/user automatically"), + ("Root API key", "requires account and user IDs"), + ], + default=0, + cancel_returns=cancelled, + ) + if credential_choice == cancelled: + return _SETUP_CANCELLED + if credential_choice == 0: + values["agent"] = _clean_config_value( + prompt("OpenViking agent", default=_DEFAULT_AGENT) + ) or _DEFAULT_AGENT + authenticated, message = _validate_openviking_auth(values) + if authenticated: + print(" OpenViking local dev access validated.") + return values + retry = _retry_or_cancel_manual_setup( + select, + " OpenViking credential failed", + message, + cancelled, + ) + if retry is _SETUP_CANCELLED: + return _SETUP_CANCELLED + continue + api_key_type = "root" if credential_choice == 2 else "user" + else: + credential_choice = select( + " OpenViking API key type", + [ + ("User API key", "server derives account/user automatically"), + ("Root API key", "requires account and user IDs"), + ], + default=0, + cancel_returns=cancelled, + ) + if credential_choice == cancelled: + return _SETUP_CANCELLED + api_key_type = "root" if credential_choice == 1 else "user" + + values["api_key_type"] = api_key_type + api_key_label = ( + "OpenViking root API key" + if api_key_type == "root" + else "OpenViking user API key" + ) + values["api_key"] = _clean_config_value(prompt(api_key_label, secret=True)) + if not values["api_key"]: + retry = _retry_or_cancel_manual_setup( + select, + " OpenViking API key required", + f"{api_key_label} is required.", + cancelled, + ) + if retry is _SETUP_CANCELLED: + return _SETUP_CANCELLED + continue + + if api_key_type == "root": + root_ok, message = _validate_openviking_root_access(values) + if not root_ok: + retry = _retry_or_cancel_manual_setup( + select, + " OpenViking root API key failed", + message, + cancelled, + ) + if retry is _SETUP_CANCELLED: + return _SETUP_CANCELLED + continue + print(" OpenViking root API key validated.") + values["account"] = _clean_config_value(prompt("OpenViking account")) + values["user"] = _clean_config_value(prompt("OpenViking user")) + if not values["account"] or not values["user"]: + retry = _retry_or_cancel_manual_setup( + select, + " OpenViking tenant identity required", + "Root API keys require both OpenViking account and user.", + cancelled, + ) + if retry is _SETUP_CANCELLED: + return _SETUP_CANCELLED + continue + + values["agent"] = _clean_config_value( + prompt("OpenViking agent", default=_DEFAULT_AGENT) + ) or _DEFAULT_AGENT + authenticated, message = _validate_openviking_auth(values) + if authenticated: + if api_key_type == "user": + user_key_ok, message = _validate_openviking_user_key_scope(values) + if not user_key_ok: + retry = _retry_or_cancel_manual_setup( + select, + " OpenViking user API key is root key", + message, + cancelled, + ) + if retry is _SETUP_CANCELLED: + return _SETUP_CANCELLED + continue + print(" OpenViking API access validated.") + return values + retry = _retry_or_cancel_manual_setup( + select, + " OpenViking API access failed", + message, + cancelled, + ) + if retry is _SETUP_CANCELLED: + return _SETUP_CANCELLED # --------------------------------------------------------------------------- diff --git a/tests/plugins/memory/test_openviking_provider.py b/tests/plugins/memory/test_openviking_provider.py index 2ca648d3228..af03fba0552 100644 --- a/tests/plugins/memory/test_openviking_provider.py +++ b/tests/plugins/memory/test_openviking_provider.py @@ -7,6 +7,7 @@ from unittest.mock import MagicMock import pytest +import plugins.memory.openviking as openviking_module from plugins.memory.openviking import OpenVikingMemoryProvider, _VikingClient @@ -33,6 +34,27 @@ def _prompt_from_values(values: dict[str, str], *, forbidden: set[str] | None = return _prompt +def _allow_setup_validation(monkeypatch, *, root_access: bool = False): + monkeypatch.setattr( + openviking_module, + "_validate_openviking_reachability", + lambda endpoint: (True, ""), + raising=False, + ) + monkeypatch.setattr( + openviking_module, + "_validate_openviking_auth", + lambda values: (True, ""), + raising=False, + ) + monkeypatch.setattr( + openviking_module, + "_validate_openviking_root_access", + lambda values: (root_access, "" if root_access else "Requires role: root"), + raising=False, + ) + + @pytest.mark.skipif(os.name == "nt", reason="POSIX file modes") def test_openviking_env_writer_restricts_file_permissions(tmp_path): env_path = tmp_path / ".env" @@ -209,6 +231,7 @@ def test_post_setup_manual_remote_root_writes_ovcli_and_links(tmp_path, monkeypa ovcli_path.write_text(json.dumps({"url": "http://old.local"}), encoding="utf-8") monkeypatch.setenv("HERMES_HOME", str(hermes_home)) monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + _allow_setup_validation(monkeypatch, root_access=True) from hermes_cli import memory_setup @@ -241,7 +264,6 @@ def test_post_setup_manual_remote_root_writes_ovcli_and_links(tmp_path, monkeypa assert data == { "url": "https://openviking.example", "api_key": "root-secret", - "root_api_key": "root-secret", "account": "acct", "user": "alice", "agent_id": "agent", @@ -257,6 +279,7 @@ def test_post_setup_manual_remote_user_keeps_only_hermes_env(tmp_path, monkeypat ovcli_path.write_text(original_ovcli, encoding="utf-8") monkeypatch.setenv("HERMES_HOME", str(hermes_home)) monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + _allow_setup_validation(monkeypatch) from hermes_cli import memory_setup @@ -297,6 +320,294 @@ def test_post_setup_manual_remote_user_keeps_only_hermes_env(tmp_path, monkeypat assert "OPENVIKING_USER" not in env_text +def test_post_setup_manual_validation_failure_writes_nothing(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + ovcli_path = tmp_path / "ovcli.conf" + original_ovcli = json.dumps({"url": "http://old.local"}) + ovcli_path.write_text(original_ovcli, encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + _allow_setup_validation(monkeypatch) + monkeypatch.setattr( + openviking_module, + "_validate_openviking_auth", + lambda values: (False, "OpenViking authentication validation failed: bad key"), + raising=False, + ) + + from hermes_cli import config as hermes_config + from hermes_cli import memory_setup + + save_config = MagicMock() + choices = iter([2, 0, 1]) + monkeypatch.setattr(hermes_config, "save_config", save_config) + monkeypatch.setattr( + memory_setup, + "_curses_select", + lambda *args, **kwargs: next(choices), + ) + monkeypatch.setattr( + memory_setup, + "_prompt", + _prompt_from_values({ + "OpenViking server URL": "https://openviking.example", + "OpenViking user API key": "bad-key", + "OpenViking agent": "agent", + }), + ) + config = {"memory": {"provider": "builtin"}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + save_config.assert_not_called() + assert config == {"memory": {"provider": "builtin"}} + assert ovcli_path.read_text(encoding="utf-8") == original_ovcli + assert not (hermes_home / ".env").exists() + + +def test_post_setup_manual_retries_base_url_until_reachable(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + ovcli_path = tmp_path / "ovcli.conf" + ovcli_path.write_text(json.dumps({"url": "http://old.local"}), encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + monkeypatch.setattr(openviking_module, "_validate_openviking_auth", lambda values: (True, "")) + + reachability_calls = [] + + def validate_reachability(endpoint): + reachability_calls.append(endpoint) + if endpoint == "http://bad.local:1933": + return False, "OpenViking server is not reachable at http://bad.local:1933." + return True, "" + + monkeypatch.setattr(openviking_module, "_validate_openviking_reachability", validate_reachability) + monkeypatch.setattr(openviking_module, "_validate_openviking_root_access", lambda values: (False, "Requires role: root")) + + from hermes_cli import memory_setup + + prompts = { + "OpenViking server URL": iter(["http://bad.local:1933", "http://localhost:1933"]), + "OpenViking agent": iter(["agent"]), + } + + def fake_prompt(label, default=None, secret=False): + return next(prompts[label]) + + choices = iter([2, 0, 0, 1]) + monkeypatch.setattr( + memory_setup, + "_curses_select", + lambda *args, **kwargs: next(choices), + ) + monkeypatch.setattr(memory_setup, "_prompt", fake_prompt) + config = {"memory": {}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + assert reachability_calls == ["http://bad.local:1933", "http://localhost:1933"] + assert config["memory"]["provider"] == "openviking" + env_text = (hermes_home / ".env").read_text(encoding="utf-8") + assert "OPENVIKING_ENDPOINT=http://localhost:1933" in env_text + + +def test_post_setup_manual_retries_user_key_until_status_valid(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + ovcli_path = tmp_path / "ovcli.conf" + ovcli_path.write_text(json.dumps({"url": "http://old.local"}), encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + monkeypatch.setattr(openviking_module, "_validate_openviking_reachability", lambda endpoint: (True, "")) + monkeypatch.setattr(openviking_module, "_validate_openviking_root_access", lambda values: (False, "Requires role: root")) + + auth_calls = [] + + def validate_auth(values): + auth_calls.append(dict(values)) + if values["api_key"] == "bad-key": + return False, "OpenViking authentication validation failed: bad key" + return True, "" + + monkeypatch.setattr(openviking_module, "_validate_openviking_auth", validate_auth) + + from hermes_cli import memory_setup + + prompts = { + "OpenViking server URL": iter(["https://openviking.example"]), + "OpenViking user API key": iter(["bad-key", "good-key"]), + "OpenViking agent": iter(["agent", "agent"]), + } + + def fake_prompt(label, default=None, secret=False): + return next(prompts[label]) + + choices = iter([2, 0, 0, 0, 1]) + monkeypatch.setattr( + memory_setup, + "_curses_select", + lambda *args, **kwargs: next(choices), + ) + monkeypatch.setattr(memory_setup, "_prompt", fake_prompt) + config = {"memory": {}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + assert [call["api_key"] for call in auth_calls] == ["bad-key", "good-key"] + env_text = (hermes_home / ".env").read_text(encoding="utf-8") + assert "OPENVIKING_API_KEY=good-key" in env_text + + +def test_post_setup_manual_user_key_rejects_root_key(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + ovcli_path = tmp_path / "ovcli.conf" + ovcli_path.write_text(json.dumps({"url": "http://old.local"}), encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + monkeypatch.setattr(openviking_module, "_validate_openviking_reachability", lambda endpoint: (True, "")) + monkeypatch.setattr(openviking_module, "_validate_openviking_auth", lambda values: (True, "")) + + root_checks = [] + + def validate_root(values): + root_checks.append(values["api_key"]) + if values["api_key"] == "root-secret": + return True, "" + return False, "Requires role: root" + + monkeypatch.setattr(openviking_module, "_validate_openviking_root_access", validate_root) + + from hermes_cli import memory_setup + + prompts = { + "OpenViking server URL": iter(["https://openviking.example"]), + "OpenViking user API key": iter(["root-secret", "user-secret"]), + "OpenViking agent": iter(["agent", "agent"]), + } + + def fake_prompt(label, default=None, secret=False): + return next(prompts[label]) + + choices = iter([2, 0, 0, 0, 1]) + monkeypatch.setattr( + memory_setup, + "_curses_select", + lambda *args, **kwargs: next(choices), + ) + monkeypatch.setattr(memory_setup, "_prompt", fake_prompt) + config = {"memory": {}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + assert root_checks == ["root-secret", "user-secret"] + env_text = (hermes_home / ".env").read_text(encoding="utf-8") + assert "OPENVIKING_API_KEY=user-secret" in env_text + assert "OPENVIKING_API_KEY=root-secret" not in env_text + + +def test_post_setup_manual_root_key_requires_root_only_validation(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + ovcli_path = tmp_path / "ovcli.conf" + ovcli_path.write_text(json.dumps({"url": "http://old.local"}), encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + monkeypatch.setattr(openviking_module, "_validate_openviking_reachability", lambda endpoint: (True, "")) + monkeypatch.setattr(openviking_module, "_validate_openviking_auth", lambda values: (True, "")) + + root_calls = [] + + def validate_root(values): + root_calls.append(dict(values)) + return True, "" + + monkeypatch.setattr(openviking_module, "_validate_openviking_root_access", validate_root) + + from hermes_cli import memory_setup + + monkeypatch.setattr( + memory_setup, + "_prompt", + _prompt_from_values({ + "OpenViking server URL": "https://openviking.example", + "OpenViking root API key": "root-secret", + "OpenViking account": "acct", + "OpenViking user": "alice", + "OpenViking agent": "agent", + }), + ) + choices = iter([2, 1, 1]) + monkeypatch.setattr( + memory_setup, + "_curses_select", + lambda *args, **kwargs: next(choices), + ) + config = {"memory": {}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + assert [call["api_key"] for call in root_calls] == ["root-secret"] + assert config["memory"]["provider"] == "openviking" + + +def test_post_setup_manual_retries_root_key_before_account_prompts(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + ovcli_path = tmp_path / "ovcli.conf" + ovcli_path.write_text(json.dumps({"url": "http://old.local"}), encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + monkeypatch.setattr(openviking_module, "_validate_openviking_reachability", lambda endpoint: (True, "")) + monkeypatch.setattr(openviking_module, "_validate_openviking_auth", lambda values: (True, "")) + + def validate_root(values): + if values["api_key"] == "bad-root": + return False, "OpenViking root API key validation failed: bad key" + return True, "" + + monkeypatch.setattr(openviking_module, "_validate_openviking_root_access", validate_root) + + from hermes_cli import memory_setup + + prompt_events = [] + prompts = { + "OpenViking server URL": iter(["https://openviking.example"]), + "OpenViking root API key": iter(["bad-root", "good-root"]), + "OpenViking account": iter(["acct"]), + "OpenViking user": iter(["alice"]), + "OpenViking agent": iter(["agent"]), + } + + def fake_prompt(label, default=None, secret=False): + prompt_events.append(label) + return next(prompts[label]) + + choices = iter([2, 1, 0, 1, 1]) + monkeypatch.setattr( + memory_setup, + "_curses_select", + lambda *args, **kwargs: next(choices), + ) + monkeypatch.setattr(memory_setup, "_prompt", fake_prompt) + config = {"memory": {}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + assert prompt_events.index("OpenViking account") > prompt_events.index("OpenViking root API key") + assert prompt_events.count("OpenViking account") == 1 + env_text = (hermes_home / ".env").read_text(encoding="utf-8") + assert "OPENVIKING_API_KEY=good-root" in env_text + + def test_post_setup_manual_remote_requires_api_key(tmp_path, monkeypatch): _clear_openviking_env(monkeypatch) hermes_home = tmp_path / "hermes" @@ -312,7 +623,7 @@ def test_post_setup_manual_remote_requires_api_key(tmp_path, monkeypatch): save_config = MagicMock() monkeypatch.setattr(hermes_config, "save_config", save_config) - choices = iter([2, 0]) + choices = iter([2, 0, 1]) monkeypatch.setattr( memory_setup, "_curses_select", @@ -345,12 +656,13 @@ def test_post_setup_manual_root_requires_account_and_user(tmp_path, monkeypatch) ovcli_path.write_text(original_ovcli, encoding="utf-8") monkeypatch.setenv("HERMES_HOME", str(hermes_home)) monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + _allow_setup_validation(monkeypatch, root_access=True) from hermes_cli import config as hermes_config from hermes_cli import memory_setup save_config = MagicMock() - choices = iter([2, 1]) + choices = iter([2, 1, 1]) monkeypatch.setattr(hermes_config, "save_config", save_config) monkeypatch.setattr( memory_setup, @@ -386,6 +698,7 @@ def test_post_setup_manual_local_allows_blank_api_key(tmp_path, monkeypatch): ovcli_path.write_text(original_ovcli, encoding="utf-8") monkeypatch.setenv("HERMES_HOME", str(hermes_home)) monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + _allow_setup_validation(monkeypatch) from hermes_cli import memory_setup @@ -956,3 +1269,144 @@ def test_viking_client_health_sends_auth_headers(monkeypatch): assert client.health() is True assert captured["url"] == "https://example.com/health" assert captured["headers"]["Authorization"] == "Bearer test-key" + + +def test_viking_client_validate_auth_uses_authenticated_system_status(monkeypatch): + client = _VikingClient( + "https://example.com", + api_key="test-key", + account="acct", + user="alice", + agent="hermes", + ) + captured = {} + + def capture_get(url, **kwargs): + captured["url"] = url + captured["headers"] = kwargs.get("headers") or {} + return SimpleNamespace( + status_code=200, + text="", + json=lambda: {"status": "ok", "result": {"initialized": True}}, + raise_for_status=lambda: None, + ) + + monkeypatch.setattr(client._httpx, "get", capture_get) + + assert client.validate_auth() == { + "status": "ok", + "result": {"initialized": True}, + } + assert captured["url"] == "https://example.com/api/v1/system/status" + assert captured["headers"]["Authorization"] == "Bearer test-key" + assert captured["headers"]["X-OpenViking-Account"] == "acct" + assert captured["headers"]["X-OpenViking-User"] == "alice" + + +def test_viking_client_validate_root_access_uses_admin_accounts(monkeypatch): + client = _VikingClient( + "https://example.com", + api_key="root-key", + account="", + user="", + agent="hermes", + ) + captured = {} + + def capture_get(url, **kwargs): + captured["url"] = url + captured["headers"] = kwargs.get("headers") or {} + return SimpleNamespace( + status_code=200, + text="", + json=lambda: {"status": "ok", "result": []}, + raise_for_status=lambda: None, + ) + + monkeypatch.setattr(client._httpx, "get", capture_get) + + assert client.validate_root_access() == {"status": "ok", "result": []} + assert captured["url"] == "https://example.com/api/v1/admin/accounts" + assert captured["headers"]["Authorization"] == "Bearer root-key" + assert "X-OpenViking-Account" not in captured["headers"] + assert "X-OpenViking-User" not in captured["headers"] + + +def test_validate_openviking_reachability_uses_health_only(monkeypatch): + events = [] + + class FakeVikingClient: + def __init__(self, endpoint, api_key="", account="", user="", agent=""): + assert endpoint == "https://openviking.example" + assert api_key == "" + + def health(self): + events.append("health") + return True + + monkeypatch.setattr(openviking_module, "_VikingClient", FakeVikingClient) + + ok, message = openviking_module._validate_openviking_reachability( + "https://openviking.example" + ) + + assert ok is True + assert message == "" + assert events == ["health"] + + +def test_validate_openviking_auth_uses_status_without_health(monkeypatch): + events = [] + + class FakeVikingClient: + def __init__(self, endpoint, api_key="", account="", user="", agent=""): + assert endpoint == "https://openviking.example" + assert api_key == "test-key" + assert account == "acct" + assert user == "alice" + assert agent == "hermes" + + def validate_auth(self): + events.append("status") + return {"status": "ok"} + + monkeypatch.setattr(openviking_module, "_VikingClient", FakeVikingClient) + + ok, message = openviking_module._validate_openviking_auth({ + "endpoint": "https://openviking.example", + "api_key": "test-key", + "account": "acct", + "user": "alice", + "agent": "hermes", + }) + + assert ok is True + assert message == "" + assert events == ["status"] + + +def test_validate_openviking_root_access_uses_admin_endpoint(monkeypatch): + events = [] + + class FakeVikingClient: + def __init__(self, endpoint, api_key="", account="", user="", agent=""): + assert endpoint == "https://openviking.example" + assert api_key == "root-key" + assert account == "" + assert user == "" + assert agent == "hermes" + + def validate_root_access(self): + events.append("admin") + return {"status": "ok"} + + monkeypatch.setattr(openviking_module, "_VikingClient", FakeVikingClient) + + ok, message = openviking_module._validate_openviking_root_access({ + "endpoint": "https://openviking.example", + "api_key": "root-key", + }) + + assert ok is True + assert message == "" + assert events == ["admin"] From 3c76dac4fdbf3d20417dde39890443c638f5d2c9 Mon Sep 17 00:00:00 2001 From: Hao Zhe Date: Tue, 26 May 2026 16:26:28 +0800 Subject: [PATCH 08/11] fix(memory): log OpenViking chmod failures --- plugins/memory/openviking/__init__.py | 4 ++-- tests/plugins/memory/test_openviking_provider.py | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index c3180305fb0..1bd1dc1262d 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -535,8 +535,8 @@ def _env_writes_from_connection_values(values: dict) -> dict: def _restrict_secret_file_permissions(path: Path) -> None: try: path.chmod(stat.S_IRUSR | stat.S_IWUSR) - except OSError: - pass + except OSError as e: + logger.debug("Could not restrict permissions on %s: %s", path, e) def _write_env_vars(env_path: Path, env_writes: dict, remove_keys: tuple[str, ...] = ()) -> None: diff --git a/tests/plugins/memory/test_openviking_provider.py b/tests/plugins/memory/test_openviking_provider.py index af03fba0552..190f8ba1b78 100644 --- a/tests/plugins/memory/test_openviking_provider.py +++ b/tests/plugins/memory/test_openviking_provider.py @@ -76,6 +76,22 @@ def test_ovcli_config_writer_restricts_file_permissions(tmp_path): assert stat.S_IMODE(config_path.stat().st_mode) == 0o600 +def test_secret_permission_restriction_logs_chmod_failure(tmp_path, monkeypatch, caplog): + env_path = tmp_path / ".env" + env_path.write_text("OPENVIKING_API_KEY=secret\n", encoding="utf-8") + + def fail_chmod(self, mode): + raise OSError("read-only filesystem") + + monkeypatch.setattr(type(env_path), "chmod", fail_chmod) + + with caplog.at_level("DEBUG", logger=openviking_module.__name__): + openviking_module._restrict_secret_file_permissions(env_path) + + assert "Could not restrict permissions" in caplog.text + assert "read-only filesystem" in caplog.text + + def test_linked_ovcli_config_is_read_at_runtime(tmp_path, monkeypatch): _clear_openviking_env(monkeypatch) ovcli_path = tmp_path / "ovcli.conf" From 2c2ca0443bbaad1f30752064d23716b927b783ea Mon Sep 17 00:00:00 2001 From: Hao Zhe Date: Wed, 17 Jun 2026 01:00:48 +0800 Subject: [PATCH 09/11] feat(memory): improve OpenViking setup UX --- agent/agent_init.py | 3 + hermes_cli/memory_setup.py | 70 +- hermes_cli/secret_prompt.py | 8 +- plugins/memory/openviking/__init__.py | 1149 +++++++++++--- tests/hermes_cli/test_memory_setup.py | 70 + tests/hermes_cli/test_secret_prompt.py | 6 +- .../memory/test_openviking_provider.py | 1347 ++++++++++------- tests/run_agent/test_memory_provider_init.py | 33 + 8 files changed, 1930 insertions(+), 756 deletions(-) diff --git a/agent/agent_init.py b/agent/agent_init.py index 2c2ded871e5..14311d8c0db 100644 --- a/agent/agent_init.py +++ b/agent/agent_init.py @@ -1153,6 +1153,9 @@ def init_agent( "hermes_home": str(get_hermes_home()), "agent_context": "primary", } + if _init_kwargs["platform"] == "cli": + _init_kwargs["warning_callback"] = agent._emit_warning + _init_kwargs["status_callback"] = agent._emit_status # Thread session title for memory provider scoping # (e.g. honcho uses this to derive chat-scoped session keys) if agent._session_db: diff --git a/hermes_cli/memory_setup.py b/hermes_cli/memory_setup.py index cd2a1b4406f..c1b058adaeb 100644 --- a/hermes_cli/memory_setup.py +++ b/hermes_cli/memory_setup.py @@ -44,7 +44,9 @@ def _curses_select( f"{label} - {desc}" if desc else label for label, desc in items ] - return curses_radiolist(title, display_items, selected=default, cancel_returns=cancel_returns) + result = curses_radiolist(title, display_items, selected=default, cancel_returns=cancel_returns) + _clear_interactive_transition() + return result def _print_cancelled_setup() -> None: @@ -229,6 +231,8 @@ def cmd_setup_provider(provider_name: str) -> None: name, _, provider = match + _clear_interactive_transition() + _install_dependencies(name) config = load_config() @@ -439,43 +443,53 @@ def cmd_status(args) -> None: print(f" Built-in: always active") print(f" Provider: {provider_name or '(none — built-in only)'}") + providers = _get_available_providers() + provider = None + for pname, _, candidate in providers: + if pname == provider_name: + provider = candidate + break + if provider_name: provider_config = mem_config.get(provider_name, {}) - if provider_config: + display_config = provider_config + if provider and hasattr(provider, "get_status_config"): + try: + display_config = provider.get_status_config(provider_config) + except Exception as e: + display_config = dict(provider_config) if isinstance(provider_config, dict) else provider_config + if isinstance(display_config, dict): + display_config["status_config_error"] = str(e) + + if display_config: print(f"\n {provider_name} config:") - for key, val in provider_config.items(): + for key, val in display_config.items(): print(f" {key}: {val}") - providers = _get_available_providers() - found = any(name == provider_name for name, _, _ in providers) - if found: + if provider: print(f"\n Plugin: installed ✓") - for pname, _, p in providers: - if pname == provider_name: - if p.is_available(): - print(f" Status: available ✓") - else: - print(f" Status: not available ✗") - schema = p.get_config_schema() if hasattr(p, "get_config_schema") else [] - # Check all fields that have env_var (both secret and non-secret) - required_fields = [f for f in schema if f.get("env_var")] - if required_fields: - print(f" Missing:") - for f in required_fields: - env_var = f.get("env_var", "") - url = f.get("url", "") - is_set = bool(os.environ.get(env_var)) - mark = "✓" if is_set else "✗" - line = f" {mark} {env_var}" - if url and not is_set: - line += f" → {url}" - print(line) - break + if provider.is_available(): + print(f" Status: available ✓") + else: + print(f" Status: not available ✗") + schema = provider.get_config_schema() if hasattr(provider, "get_config_schema") else [] + # Check all fields that have env_var (both secret and non-secret) + required_fields = [f for f in schema if f.get("env_var")] + if required_fields: + print(f" Missing:") + for f in required_fields: + env_var = f.get("env_var", "") + url = f.get("url", "") + is_set = bool(os.environ.get(env_var)) + mark = "✓" if is_set else "✗" + line = f" {mark} {env_var}" + if url and not is_set: + line += f" → {url}" + print(line) else: print(f"\n Plugin: NOT installed ✗") print(f" Install the '{provider_name}' memory plugin to ~/.hermes/plugins/") - providers = _get_available_providers() if providers: print(f"\n Installed plugins:") for pname, desc, _ in providers: diff --git a/hermes_cli/secret_prompt.py b/hermes_cli/secret_prompt.py index d1cffc34c5e..1f8a4df485a 100644 --- a/hermes_cli/secret_prompt.py +++ b/hermes_cli/secret_prompt.py @@ -27,16 +27,16 @@ def _collect_masked_input( while True: ch = read_char() if ch == "": - write("\n") + write("\r\n") raise EOFError if ch in _ENTER_CHARS: - write("\n") + write("\r\n") return "".join(value) if ch == "\x03": - write("\n") + write("\r\n") raise KeyboardInterrupt if ch in _EOF_CHARS: - write("\n") + write("\r\n") raise EOFError if ch in _BACKSPACE_CHARS: if value: diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index 1bd1dc1262d..07dd3317957 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -30,11 +30,16 @@ import json import logging import mimetypes import os +import re +import shutil import stat +import subprocess import tempfile import threading +import time import uuid import zipfile +from dataclasses import dataclass, replace from pathlib import Path from typing import Any, Dict, List, Optional from urllib.parse import urlparse @@ -46,11 +51,13 @@ from tools.registry import tool_error logger = logging.getLogger(__name__) _DEFAULT_ENDPOINT = "http://127.0.0.1:1933" +_OPENVIKING_SERVICE_ENDPOINT = "https://api.vikingdb.cn-beijing.volces.com/openviking" _DEFAULT_ACCOUNT = "" _DEFAULT_USER = "" _DEFAULT_AGENT = "hermes" _OVCLI_CONFIG_ENV = "OPENVIKING_CLI_CONFIG_FILE" _OVCLI_DEFAULT_RELATIVE_PATH = ".openviking/ovcli.conf" +_OVCLI_SAVED_PREFIX = "ovcli.conf." _OPENVIKING_ENV_KEYS = ( "OPENVIKING_ENDPOINT", "OPENVIKING_API_KEY", @@ -80,9 +87,57 @@ _MEMORY_WRITE_TARGET_SUBDIR_MAP = { "memory": "patterns", } _LOCAL_OPENVIKING_HOSTS = {"localhost", "127.0.0.1", "::1"} +_LOCAL_OPENVIKING_AUTOSTART_TIMEOUT = 60.0 _SETUP_CANCELLED = object() +@dataclass(frozen=True) +class _OvcliProfile: + source: str + name: str + path: Path + data: dict + values: dict + is_active: bool = False + + +class _OpenVikingHTTPError(RuntimeError): + def __init__(self, message: str, status_code: Optional[int] = None): + super().__init__(message) + self.status_code = status_code + + +def _sanitize_openviking_error_message(message: str, status_code: Optional[int] = None) -> str: + text = (message or "").strip() + status = f"HTTP {status_code}" if status_code else "HTTP error" + looks_like_html = bool(re.search(r"^\s*<(!doctype|html|head|body)\b", text, flags=re.IGNORECASE)) + if looks_like_html: + title_match = re.search(r"]*>(.*?)", text, flags=re.IGNORECASE | re.DOTALL) + if title_match: + title = re.sub(r"\s+", " ", title_match.group(1)).strip() + if "|" in title: + title = title.split("|", 1)[1].strip() + if status_code and title.startswith(f"{status_code}:"): + title = title.split(":", 1)[1].strip() + if title: + return f"{status}: {title}" + return f"{status}: OpenViking endpoint returned an HTML error page." + + if len(text) > 300: + return text[:297].rstrip() + "..." + return text or status + + +def _format_openviking_exception(error: Exception) -> str: + status_code = None + if isinstance(error, _OpenVikingHTTPError): + status_code = error.status_code + else: + response = getattr(error, "response", None) + status_code = getattr(response, "status_code", None) + return _sanitize_openviking_error_message(str(error), status_code) + + # --------------------------------------------------------------------------- # Process-level atexit safety net — ensures pending sessions are committed # even if shutdown_memory_provider is never called (e.g. gateway crash, @@ -138,6 +193,7 @@ class _VikingClient: def _headers(self) -> dict: h = {"Content-Type": "application/json"} if self._agent: + h["X-OpenViking-Actor-Peer"] = self._agent h["X-OpenViking-Agent"] = self._agent if self._account: h["X-OpenViking-Account"] = self._account @@ -163,15 +219,19 @@ class _VikingClient: data = None if resp.status_code >= 400: + message = _sanitize_openviking_error_message( + getattr(resp, "text", ""), + resp.status_code, + ) if isinstance(data, dict): error = data.get("error") if isinstance(error, dict): code = error.get("code", "HTTP_ERROR") - message = error.get("message", resp.text) - raise RuntimeError(f"{code}: {message}") + message = f"{code}: {error.get('message', message)}" + raise _OpenVikingHTTPError(message, resp.status_code) if data.get("status") == "error": - raise RuntimeError(str(data)) - resp.raise_for_status() + raise _OpenVikingHTTPError(str(data), resp.status_code) + raise _OpenVikingHTTPError(message or f"HTTP {resp.status_code}", resp.status_code) if isinstance(data, dict) and data.get("status") == "error": error = data.get("error") @@ -223,6 +283,12 @@ class _VikingClient: except Exception: return False + def health_payload(self) -> dict: + resp = self._httpx.get( + self._url("/health"), headers=self._headers(), timeout=3.0 + ) + return self._parse_response(resp) + def validate_auth(self) -> dict: """Validate authenticated OpenViking access without mutating state.""" return self.get("/api/v1/system/status") @@ -432,14 +498,18 @@ def _default_ovcli_config_path() -> Path: def _resolve_ovcli_config_path(config_path: str = "") -> Path: - if config_path: - return Path(config_path).expanduser() env_path = os.environ.get(_OVCLI_CONFIG_ENV, "").strip() if env_path: return Path(env_path).expanduser() + if config_path: + return Path(config_path).expanduser() return _default_ovcli_config_path() +def _ovcli_config_dir() -> Path: + return _default_ovcli_config_path().parent + + def _load_ovcli_config(path: Optional[Path] = None) -> dict: config_path = path or _resolve_ovcli_config_path() if not config_path.exists(): @@ -452,17 +522,143 @@ def _load_ovcli_config(path: Optional[Path] = None) -> dict: def _connection_values_from_ovcli(data: dict) -> dict: + api_key = _clean_config_value(data.get("api_key")) or _clean_config_value(data.get("root_api_key")) + root_api_key = _clean_config_value(data.get("root_api_key")) + send_identity = not api_key or api_key == root_api_key + account = _clean_config_value(data.get("account") or data.get("account_id")) + user = _clean_config_value(data.get("user") or data.get("user_id")) return { - "endpoint": _clean_config_value(data.get("url")) or _DEFAULT_ENDPOINT, - "api_key": _clean_config_value(data.get("api_key")), - "account": _clean_config_value(data.get("account") or data.get("account_id")), - "user": _clean_config_value(data.get("user") or data.get("user_id")), - "agent": _clean_config_value(data.get("agent_id")), + "endpoint": _normalize_openviking_url(data.get("url")), + "api_key": api_key, + "root_api_key": root_api_key, + "account": account if send_identity else "", + "user": user if send_identity else "", + "agent": _clean_config_value(data.get("actor_peer_id") or data.get("agent_id")), } +def _is_valid_ovcli_profile_name(name: str) -> bool: + if not name or name.strip() != name or name.startswith("."): + return False + if "/" in name or "\\" in name: + return False + return all(ch.isascii() and (ch.isalnum() or ch in {"-", "_"}) for ch in name) + + +def _validate_openviking_identity_value(value: str, *, field: str) -> tuple[bool, str, str]: + label = "Account ID" if field == "account" else "User ID" + identifier = "account_id" if field == "account" else "user_id" + trimmed = value.strip() + if not trimmed: + return False, f"{label} cannot be empty.", "" + if trimmed != value: + return False, f"{label} cannot start or end with whitespace.", "" + if field == "account" and trimmed.startswith("_"): + return False, "Account ID cannot start with '_'.", "" + if not all(ch.isascii() and (ch.isalnum() or ch in {"_", "-", ".", "@"}) for ch in trimmed): + return False, f"{label} can only contain letters, numbers, '_', '-', '.', and '@'.", "" + if trimmed.count("@") > 1: + return False, f"{identifier} must have at most one '@'.", "" + return True, "", trimmed + + +def _normalize_openviking_url(url: str) -> str: + trimmed = _clean_config_value(url).rstrip("/") + if not trimmed: + return _DEFAULT_ENDPOINT + lower = trimmed.lower() + if lower in {"::1", "[::1]"}: + return "http://[::1]:1933" + if lower.startswith("[::1]:"): + return f"http://[::1]:{trimmed.rsplit(':', 1)[1]}" + if lower.startswith("::1:"): + return f"http://[::1]:{trimmed.rsplit(':', 1)[1]}" + if "://" in trimmed: + return trimmed + host, _sep, port = trimmed.partition(":") + if host.lower() in {"localhost", "127.0.0.1"}: + return f"http://{host}:{port or '1933'}" + return trimmed + + +def _load_profile(path: Path, *, source: str, name: str) -> Optional[_OvcliProfile]: + try: + data = _load_ovcli_config(path) + except Exception as e: + logger.debug("Skipping invalid OpenViking CLI config %s: %s", path, e) + return None + return _OvcliProfile( + source=source, + name=name, + path=path, + data=data, + values=_connection_values_from_ovcli(data), + ) + + +def _profile_identity(path: Path) -> str: + try: + return str(path.expanduser().resolve()) + except OSError: + return str(path.expanduser()) + + +def _profiles_equivalent(left: _OvcliProfile, right: _OvcliProfile) -> bool: + return left.values == right.values + + +def _discover_ovcli_profiles() -> list[_OvcliProfile]: + profiles: list[_OvcliProfile] = [] + seen_paths: set[str] = set() + + def add(path: Path, *, source: str, name: str) -> None: + if not path.exists() or not path.is_file(): + return + identity = _profile_identity(path) + if identity in seen_paths: + return + profile = _load_profile(path, source=source, name=name) + if profile is None: + return + seen_paths.add(identity) + profiles.append(profile) + + env_path = os.environ.get(_OVCLI_CONFIG_ENV, "").strip() + if env_path: + add(Path(env_path).expanduser(), source="env", name=_OVCLI_CONFIG_ENV) + + active_path = _default_ovcli_config_path() + active_profile = _load_profile(active_path, source="active", name="active") if active_path.exists() else None + + config_dir = _ovcli_config_dir() + saved_start = len(profiles) + if config_dir.exists(): + for path in sorted(config_dir.iterdir(), key=lambda item: item.name): + if not path.is_file(): + continue + name = path.name.removeprefix(_OVCLI_SAVED_PREFIX) + if name == path.name or name == "bak" or not _is_valid_ovcli_profile_name(name): + continue + add(path, source="saved", name=name) + + if active_profile is not None: + marked_active = False + for idx in range(saved_start, len(profiles)): + if profiles[idx].source == "saved" and _profiles_equivalent(profiles[idx], active_profile): + profiles[idx] = replace(profiles[idx], is_active=True) + marked_active = True + break + has_env_profile = any(profile.source == "env" for profile in profiles) + has_saved_profile = any(profile.source == "saved" for profile in profiles) + active_identity = _profile_identity(active_profile.path) + if not marked_active and not has_env_profile and not has_saved_profile and active_identity not in seen_paths: + profiles.append(active_profile) + + return profiles + + def _is_local_openviking_url(value: str) -> bool: - candidate = _clean_config_value(value) + candidate = _normalize_openviking_url(value) if not candidate: return False if "://" not in candidate: @@ -570,19 +766,22 @@ def _remember_ovcli_path(provider_config: dict, ovcli_path: Path) -> None: def _ovcli_data_from_connection_values(values: dict) -> dict: - data = {"url": _clean_config_value(values.get("endpoint")) or _DEFAULT_ENDPOINT} + data = {"url": _normalize_openviking_url(_clean_config_value(values.get("endpoint")) or _DEFAULT_ENDPOINT)} api_key = _clean_config_value(values.get("api_key")) + root_api_key = _clean_config_value(values.get("root_api_key")) account = _clean_config_value(values.get("account")) user = _clean_config_value(values.get("user")) agent = _clean_config_value(values.get("agent")) or _DEFAULT_AGENT if api_key: data["api_key"] = api_key + if root_api_key: + data["root_api_key"] = root_api_key if account: data["account"] = account if user: data["user"] = user if agent: - data["agent_id"] = agent + data["actor_peer_id"] = agent return data @@ -593,18 +792,24 @@ def _write_ovcli_config(path: Path, values: dict) -> None: def _validate_openviking_reachability(endpoint: str) -> tuple[bool, str]: - endpoint = _clean_config_value(endpoint) or _DEFAULT_ENDPOINT + endpoint = _normalize_openviking_url(endpoint) try: client = _VikingClient(endpoint) - if client.health(): + if hasattr(client, "health_payload"): + payload = client.health_payload() + if payload.get("healthy") is False: + return False, "OpenViking server responded but reported unhealthy status." + if payload: + return True, "" + elif client.health(): return True, "" except Exception as e: - return False, f"OpenViking server is not reachable at {endpoint}: {e}" + return False, f"OpenViking server is not reachable at {endpoint}: {_format_openviking_exception(e)}" return False, f"OpenViking server is not reachable at {endpoint}." def _validate_openviking_auth(values: dict) -> tuple[bool, str]: - endpoint = _clean_config_value(values.get("endpoint")) or _DEFAULT_ENDPOINT + endpoint = _normalize_openviking_url(values.get("endpoint")) try: client = _VikingClient( endpoint, @@ -615,12 +820,12 @@ def _validate_openviking_auth(values: dict) -> tuple[bool, str]: ) client.validate_auth() except Exception as e: - return False, f"OpenViking authentication validation failed: {e}" + return False, f"OpenViking authentication validation failed: {_format_openviking_exception(e)}" return True, "" def _validate_openviking_root_access(values: dict) -> tuple[bool, str]: - endpoint = _clean_config_value(values.get("endpoint")) or _DEFAULT_ENDPOINT + endpoint = _normalize_openviking_url(values.get("endpoint")) try: client = _VikingClient( endpoint, @@ -629,7 +834,7 @@ def _validate_openviking_root_access(values: dict) -> tuple[bool, str]: ) client.validate_root_access() except Exception as e: - return False, f"OpenViking root API key validation failed: {e}" + return False, f"OpenViking root API key validation failed: {_format_openviking_exception(e)}" return True, "" @@ -644,6 +849,68 @@ def _validate_openviking_user_key_scope(values: dict) -> tuple[bool, str]: ) +def _status_code_from_error(error: Exception) -> Optional[int]: + if isinstance(error, _OpenVikingHTTPError): + return error.status_code + response = getattr(error, "response", None) + return getattr(response, "status_code", None) + + +def _admin_probe_means_regular_key(error: Exception) -> bool: + return _status_code_from_error(error) in {401, 403, 404} + + +def _should_probe_openviking_auth(health: dict, *, require_api_key: bool, has_api_key: bool) -> bool: + if require_api_key or has_api_key: + return True + auth_mode = health.get("auth_mode") + if auth_mode == "dev": + return False + if auth_mode in {"api_key", "trusted", None}: + return True + return False + + +def _validate_openviking_setup_values( + values: dict, + *, + require_api_key: bool = False, +) -> tuple[bool, str, Optional[str]]: + endpoint = _normalize_openviking_url(values.get("endpoint")) + api_key = _clean_config_value(values.get("api_key")) + if require_api_key and not api_key: + return False, "Remote OpenViking configs require an API key.", None + + try: + client = _VikingClient( + endpoint, + api_key, + account=_clean_config_value(values.get("account")), + user=_clean_config_value(values.get("user")), + agent=_clean_config_value(values.get("agent")) or _DEFAULT_AGENT, + ) + health = client.health_payload() + if health.get("healthy") is False: + return False, "OpenViking server responded but reported unhealthy status.", None + if _should_probe_openviking_auth( + health, + require_api_key=require_api_key, + has_api_key=bool(api_key), + ): + client.validate_auth() + if not api_key: + return True, "", None + try: + client.validate_root_access() + return True, "", "root" + except Exception as e: + if _admin_probe_means_regular_key(e): + return True, "", "user" + raise + except Exception as e: + return False, f"OpenViking validation failed: {_format_openviking_exception(e)}", None + + def _retry_or_cancel_manual_setup(select, title: str, message: str, cancelled): print(f" {message}") choice = select( @@ -660,34 +927,188 @@ def _retry_or_cancel_manual_setup(select, title: str, message: str, cancelled): return _SETUP_CANCELLED -def _prompt_manual_connection_values(prompt, select, cancelled): +def _print_validation_progress(message: str) -> None: + print(f" {message}", flush=True) + + +def _local_openviking_bind(endpoint: str) -> tuple[str, int]: + normalized = _normalize_openviking_url(endpoint) + parsed = urlparse(normalized) + host = parsed.hostname or "127.0.0.1" + port = parsed.port or 1933 + return host, port + + +def _start_local_openviking_server(endpoint: str) -> tuple[bool, str]: + server_cmd = shutil.which("openviking-server") + if not server_cmd: + return False, "openviking-server was not found on PATH. Start it manually, then retry." + try: + host, port = _local_openviking_bind(endpoint) + except ValueError as e: + return False, f"Could not parse local OpenViking URL: {e}" + try: + subprocess.Popen( + [server_cmd, "--host", host, "--port", str(port)], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + stdin=subprocess.DEVNULL, + start_new_session=True, + ) + except Exception as e: + return False, f"Could not start openviking-server: {e}" + return True, f"Started openviking-server on {host}:{port} in the background." + + +def _wait_for_openviking_health(endpoint: str, *, timeout_seconds: float = 15.0) -> bool: + deadline = time.monotonic() + timeout_seconds + while time.monotonic() < deadline: + ok, _message = _validate_openviking_reachability(endpoint) + if ok: + return True + time.sleep(0.5) + return False + + +def _handle_unreachable_endpoint(endpoint: str, message: str, select, cancelled): + if _is_local_openviking_url(endpoint): + print(f" {message}") + choice = select( + " Local OpenViking server is down", + [ + ("Start local OpenViking", "run openviking-server and retry"), + ("Retry URL", "enter the server URL again"), + ("Cancel setup", "no changes saved"), + ], + default=0, + cancel_returns=cancelled, + ) + if choice == 0: + started, start_message = _start_local_openviking_server(endpoint) + print(f" {start_message}") + if not started: + return False + print(" Waiting for OpenViking server to become reachable...", flush=True) + if _wait_for_openviking_health( + endpoint, + timeout_seconds=_LOCAL_OPENVIKING_AUTOSTART_TIMEOUT, + ): + print(" OpenViking server is reachable.") + return True + print(" OpenViking server did not become reachable.") + return False + if choice == 1: + return False + return _SETUP_CANCELLED + + return _retry_or_cancel_manual_setup( + select, + " OpenViking server unreachable", + message, + cancelled, + ) + + +def _emit_runtime_warning(message: str, warning_callback=None) -> None: + logger.warning("%s", message) + if warning_callback: + try: + warning_callback(message) + except Exception: + logger.debug("OpenViking runtime warning callback failed", exc_info=True) + + +def _emit_runtime_status(message: str, status_callback=None) -> None: + logger.info("%s", message) + if status_callback: + try: + status_callback(message) + except Exception: + logger.debug("OpenViking runtime status callback failed", exc_info=True) + + +def _runtime_openviking_timeout_message(endpoint: str) -> str: + return ( + f"Local OpenViking server at {endpoint} is not reachable. " + "Tried to start openviking-server, but it did not become reachable " + f"within {_LOCAL_OPENVIKING_AUTOSTART_TIMEOUT:.0f} seconds. " + "OpenViking memory disabled for this Hermes run." + ) + + +def _prompt_profile_name(prompt, select, cancelled) -> str | object: while True: - endpoint = _clean_config_value( - prompt("OpenViking server URL", default=_DEFAULT_ENDPOINT) - ) or _DEFAULT_ENDPOINT - reachable, message = _validate_openviking_reachability(endpoint) - if reachable: - print(" OpenViking server is reachable.") - break + name = _clean_config_value(prompt("OpenViking profile name")) + if _is_valid_ovcli_profile_name(name): + return name retry = _retry_or_cancel_manual_setup( select, - " OpenViking server unreachable", - message, + " Invalid OpenViking profile name", + "Profile names can only contain letters, numbers, '-' and '_'.", cancelled, ) if retry is _SETUP_CANCELLED: return _SETUP_CANCELLED + +def _confirm_replace_existing_profile(path: Path, values: dict, select, cancelled): + if not path.exists(): + return True + try: + existing_data = _load_ovcli_config(path) + except Exception: + existing_data = {} + if existing_data == _ovcli_data_from_connection_values(values): + return True + choice = select( + " OpenViking profile already exists", + [ + ("Choose another name", "leave the existing profile unchanged"), + ("Replace profile", "overwrite this saved OpenViking profile"), + ("Cancel setup", "no changes saved"), + ], + default=0, + cancel_returns=cancelled, + ) + if choice == 1: + return True + if choice == 0: + return False + return _SETUP_CANCELLED + + +def _prompt_manual_connection_values(prompt, select, cancelled, *, service: bool = False): + if service: + endpoint = _OPENVIKING_SERVICE_ENDPOINT + print(f" OpenViking Service endpoint: {endpoint}") + else: + while True: + endpoint = _normalize_openviking_url(prompt("OpenViking server URL", default=_DEFAULT_ENDPOINT)) + _print_validation_progress("Checking OpenViking server...") + reachable, message = _validate_openviking_reachability(endpoint) + if reachable: + print(" OpenViking server is reachable.") + break + retry = _handle_unreachable_endpoint(endpoint, message, select, cancelled) + if retry is True: + break + if retry is _SETUP_CANCELLED: + return _SETUP_CANCELLED + is_local = _is_local_openviking_url(endpoint) + api_key_type = "user" if service else "" + prefilled_api_key = "" + prefilled_agent = "" while True: values = { "endpoint": endpoint, "api_key": "", + "root_api_key": "", "account": "", "user": "", "agent": "", } - if is_local: + if not api_key_type and is_local: credential_choice = select( " OpenViking credential", [ @@ -704,8 +1125,9 @@ def _prompt_manual_connection_values(prompt, select, cancelled): values["agent"] = _clean_config_value( prompt("OpenViking agent", default=_DEFAULT_AGENT) ) or _DEFAULT_AGENT - authenticated, message = _validate_openviking_auth(values) - if authenticated: + _print_validation_progress("Validating OpenViking local dev access...") + valid, message, _role = _validate_openviking_setup_values(values) + if valid: print(" OpenViking local dev access validated.") return values retry = _retry_or_cancel_manual_setup( @@ -718,7 +1140,7 @@ def _prompt_manual_connection_values(prompt, select, cancelled): return _SETUP_CANCELLED continue api_key_type = "root" if credential_choice == 2 else "user" - else: + elif not api_key_type: credential_choice = select( " OpenViking API key type", [ @@ -733,12 +1155,19 @@ def _prompt_manual_connection_values(prompt, select, cancelled): api_key_type = "root" if credential_choice == 1 else "user" values["api_key_type"] = api_key_type - api_key_label = ( - "OpenViking root API key" - if api_key_type == "root" - else "OpenViking user API key" - ) - values["api_key"] = _clean_config_value(prompt(api_key_label, secret=True)) + if service: + api_key_label = "OpenViking API key" + else: + api_key_label = ( + "OpenViking root API key" + if api_key_type == "root" + else "OpenViking user API key" + ) + if prefilled_api_key: + values["api_key"] = prefilled_api_key + prefilled_api_key = "" + else: + values["api_key"] = _clean_config_value(prompt(api_key_label, secret=True)) if not values["api_key"]: retry = _retry_or_cancel_manual_setup( select, @@ -751,8 +1180,30 @@ def _prompt_manual_connection_values(prompt, select, cancelled): continue if api_key_type == "root": - root_ok, message = _validate_openviking_root_access(values) + _print_validation_progress("Validating OpenViking root API key...") + valid, message, role = _validate_openviking_setup_values(values, require_api_key=True) + root_ok = valid and role == "root" if not root_ok: + if valid and role == "user": + print(" That key is valid, but it is a user API key.") + route_choice = select( + " OpenViking key is a user key", + [ + ("Use as User API key", "server derives account/user automatically"), + ("Re-enter Root API key", "try another root key"), + ("Cancel setup", "no changes saved"), + ], + default=0, + cancel_returns=cancelled, + ) + if route_choice == 0: + prefilled_api_key = values["api_key"] + api_key_type = "user" + continue + if route_choice == 1: + api_key_type = "root" + continue + return _SETUP_CANCELLED retry = _retry_or_cancel_manual_setup( select, " OpenViking root API key failed", @@ -763,36 +1214,75 @@ def _prompt_manual_connection_values(prompt, select, cancelled): return _SETUP_CANCELLED continue print(" OpenViking root API key validated.") - values["account"] = _clean_config_value(prompt("OpenViking account")) - values["user"] = _clean_config_value(prompt("OpenViking user")) - if not values["account"] or not values["user"]: + values["root_api_key"] = values["api_key"] + account_ok, account_message, account = _validate_openviking_identity_value( + prompt("OpenViking account"), + field="account", + ) + user_ok, user_message, user = _validate_openviking_identity_value( + prompt("OpenViking user"), + field="user", + ) + values["account"] = account + values["user"] = user + if not account_ok or not user_ok: + message = account_message if not account_ok else user_message retry = _retry_or_cancel_manual_setup( select, " OpenViking tenant identity required", - "Root API keys require both OpenViking account and user.", + message, + cancelled, + ) + if retry is _SETUP_CANCELLED: + return _SETUP_CANCELLED + prefilled_api_key = values["api_key"] + continue + + if prefilled_agent: + values["agent"] = prefilled_agent + prefilled_agent = "" + else: + values["agent"] = _clean_config_value( + prompt("OpenViking agent", default=_DEFAULT_AGENT) + ) or _DEFAULT_AGENT + _print_validation_progress("Validating OpenViking API access...") + valid, message, role = _validate_openviking_setup_values( + values, + require_api_key=service or not is_local, + ) + if valid: + if api_key_type == "user": + if role == "root": + print(" That key is valid, but it has root access.") + route_choice = select( + " OpenViking user API key is root key", + [ + ("Configure as Root API key", "provide account and user IDs"), + ("Re-enter User API key", "try another user key"), + ("Cancel setup", "no changes saved"), + ], + default=0, + cancel_returns=cancelled, + ) + if route_choice == 0: + prefilled_api_key = values["api_key"] + prefilled_agent = values["agent"] + api_key_type = "root" + continue + if route_choice == 1: + api_key_type = "user" + continue + return _SETUP_CANCELLED + if api_key_type == "root" and role != "root": + retry = _retry_or_cancel_manual_setup( + select, + " OpenViking root API key failed", + "The supplied key was not accepted as a root API key.", cancelled, ) if retry is _SETUP_CANCELLED: return _SETUP_CANCELLED continue - - values["agent"] = _clean_config_value( - prompt("OpenViking agent", default=_DEFAULT_AGENT) - ) or _DEFAULT_AGENT - authenticated, message = _validate_openviking_auth(values) - if authenticated: - if api_key_type == "user": - user_key_ok, message = _validate_openviking_user_key_scope(values) - if not user_key_ok: - retry = _retry_or_cancel_manual_setup( - select, - " OpenViking user API key is root key", - message, - cancelled, - ) - if retry is _SETUP_CANCELLED: - return _SETUP_CANCELLED - continue print(" OpenViking API access validated.") return values retry = _retry_or_cancel_manual_setup( @@ -805,6 +1295,223 @@ def _prompt_manual_connection_values(prompt, select, cancelled): return _SETUP_CANCELLED +def _set_openviking_provider(config: dict, provider_config: dict) -> None: + config["memory"]["provider"] = "openviking" + config["memory"]["openviking"] = provider_config + + +def _link_ovcli_profile( + *, + config: dict, + provider_config: dict, + env_path: Path, + ovcli_path: Path, +) -> None: + for key in ("endpoint", "api_key", "root_api_key", "account", "user", "agent", "api_key_type"): + provider_config.pop(key, None) + provider_config["use_ovcli_config"] = True + _remember_ovcli_path(provider_config, ovcli_path) + _set_openviking_provider(config, provider_config) + _write_env_vars(env_path, {}, remove_keys=_OPENVIKING_ENV_KEYS) + for key in _OPENVIKING_ENV_KEYS: + os.environ.pop(key, None) + + +def _save_hermes_only_config( + *, + config: dict, + provider_config: dict, + env_path: Path, + values: dict, +) -> None: + provider_config["use_ovcli_config"] = False + provider_config.pop("ovcli_config_path", None) + _set_openviking_provider(config, provider_config) + _write_env_vars( + env_path, + _env_writes_from_connection_values(values), + remove_keys=_OPENVIKING_ENV_KEYS, + ) + + +def _profile_display_name(profile: _OvcliProfile) -> str: + if profile.source == "env": + return _OVCLI_CONFIG_ENV + if profile.source == "active": + return "ovcli.conf" + return profile.name + + +def _profile_description(profile: _OvcliProfile) -> str: + endpoint = _clean_config_value(profile.values.get("endpoint")) or _DEFAULT_ENDPOINT + return f"{endpoint} ({profile.path})" + + +def _validate_profile_for_setup(profile: _OvcliProfile) -> tuple[bool, str, Optional[str]]: + require_api_key = not _is_local_openviking_url(profile.values.get("endpoint", "")) + return _validate_openviking_setup_values(profile.values, require_api_key=require_api_key) + + +def _print_openviking_ready(message: str, path: Optional[Path] = None) -> None: + print("\n OpenViking memory is ready") + print(f" {message}") + if path is not None: + print(f" Config file: {path}") + print(" Start a new Hermes session to activate.\n") + + +def _run_existing_profile_setup( + *, + profiles: list[_OvcliProfile], + select, + cancelled, + config: dict, + provider_config: dict, + env_path: Path, +) -> bool | object: + while True: + choice = select( + " OpenViking profile", + [(_profile_display_name(profile), _profile_description(profile)) for profile in profiles], + default=0, + cancel_returns=cancelled, + ) + if choice == cancelled: + return _SETUP_CANCELLED + if choice < 0 or choice >= len(profiles): + return _SETUP_CANCELLED + + profile = profiles[choice] + _print_validation_progress("Validating OpenViking profile...") + ok, message, _role = _validate_profile_for_setup(profile) + if ok: + _link_ovcli_profile( + config=config, + provider_config=provider_config, + env_path=env_path, + ovcli_path=profile.path, + ) + _print_openviking_ready(f"Linked profile: {_profile_display_name(profile)}", profile.path) + return True + + print(f" {message}") + retry = select( + " OpenViking profile validation failed", + [ + ("Choose another profile", "select a different OpenViking profile"), + ("Retry validation", "try this profile again"), + ("Cancel setup", "no changes saved"), + ], + default=0, + cancel_returns=cancelled, + ) + if retry == 0: + continue + if retry == 1: + _print_validation_progress("Validating OpenViking profile...") + ok, message, _role = _validate_profile_for_setup(profile) + if ok: + _link_ovcli_profile( + config=config, + provider_config=provider_config, + env_path=env_path, + ovcli_path=profile.path, + ) + _print_openviking_ready(f"Linked profile: {_profile_display_name(profile)}", profile.path) + return True + print(f" {message}") + continue + return _SETUP_CANCELLED + + +def _mirror_manual_config_to_openviking_store( + *, + prompt, + select, + cancelled, + values: dict, +) -> Path | object: + while True: + name = _prompt_profile_name(prompt, select, cancelled) + if name is _SETUP_CANCELLED: + return _SETUP_CANCELLED + path = _ovcli_config_dir() / f"{_OVCLI_SAVED_PREFIX}{name}" + replace = _confirm_replace_existing_profile(path, values, select, cancelled) + if replace is _SETUP_CANCELLED: + return _SETUP_CANCELLED + if replace is False: + continue + _write_ovcli_config(path, values) + return path + + +def _run_create_profile_setup( + *, + prompt, + select, + cancelled, + config: dict, + provider_config: dict, + env_path: Path, +) -> bool | object: + source_choice = select( + " OpenViking connection", + [ + ("OpenViking Service (VolcEngine Cloud)", "use the managed OpenViking endpoint"), + ("Custom", "use a local, VPS, or self-hosted OpenViking server"), + ], + default=0, + cancel_returns=cancelled, + ) + if source_choice == cancelled: + return _SETUP_CANCELLED + + values = _prompt_manual_connection_values(prompt, select, cancelled, service=(source_choice == 0)) + if values is _SETUP_CANCELLED: + return _SETUP_CANCELLED + if values is None: + return False + + save_choice = select( + " Save OpenViking config", + [ + ("Keep in Hermes only", "write values only to Hermes .env"), + ("Mirror to OpenViking store", "write ~/.openviking/ovcli.conf. and link it"), + ], + default=1, + cancel_returns=cancelled, + ) + if save_choice == cancelled: + return _SETUP_CANCELLED + + if save_choice == 1: + ovcli_path = _mirror_manual_config_to_openviking_store( + prompt=prompt, + select=select, + cancelled=cancelled, + values=values, + ) + if ovcli_path is _SETUP_CANCELLED: + return _SETUP_CANCELLED + _link_ovcli_profile( + config=config, + provider_config=provider_config, + env_path=env_path, + ovcli_path=ovcli_path, + ) + _print_openviking_ready("Created and linked OpenViking profile.", ovcli_path) + return True + + _save_hermes_only_config( + config=config, + provider_config=provider_config, + env_path=env_path, + values=values, + ) + _print_openviking_ready("Connection saved to Hermes .env.") + return True + + # --------------------------------------------------------------------------- # MemoryProvider implementation # --------------------------------------------------------------------------- @@ -822,6 +1529,8 @@ class OpenVikingMemoryProvider(MemoryProvider): self._prefetch_result = "" self._prefetch_lock = threading.Lock() self._prefetch_thread: Optional[threading.Thread] = None + self._runtime_start_lock = threading.Lock() + self._runtime_start_thread: Optional[threading.Thread] = None @property def name(self) -> str: @@ -873,6 +1582,40 @@ class OpenVikingMemoryProvider(MemoryProvider): }, ] + def get_status_config(self, provider_config: dict) -> dict: + provider_config = dict(provider_config or {}) + if provider_config.get("use_ovcli_config"): + ovcli_path = _resolve_ovcli_config_path(str(provider_config.get("ovcli_config_path") or "")) + try: + settings = _resolve_connection_settings(provider_config) + except Exception as e: + return { + "use_ovcli_config": True, + "ovcli_config_path": str(ovcli_path), + "error": _format_openviking_exception(e), + } + + display = { + "use_ovcli_config": True, + "ovcli_config_path": str(ovcli_path), + "endpoint": settings.get("endpoint") or _DEFAULT_ENDPOINT, + "agent": settings.get("agent") or _DEFAULT_AGENT, + } + if settings.get("account"): + display["account"] = settings["account"] + if settings.get("user"): + display["user"] = settings["user"] + env_overrides = [key for key in _OPENVIKING_ENV_KEYS if _env_value(key) is not None] + if env_overrides: + display["env_overrides"] = ", ".join(env_overrides) + return display + + display = dict(provider_config) + for key in ("api_key", "root_api_key"): + if key in display: + display[key] = "(set)" + return display + def post_setup(self, hermes_home: str, config: dict) -> None: """Custom setup that can reuse OpenViking's shared CLI config.""" from hermes_cli.config import save_config @@ -886,22 +1629,13 @@ class OpenVikingMemoryProvider(MemoryProvider): if not isinstance(provider_config, dict): provider_config = {} - ovcli_path = _resolve_ovcli_config_path(str(provider_config.get("ovcli_config_path") or "")) - - print("\n Configuring OpenViking memory:\n") - - if ovcli_path.exists(): - try: - ovcli_values = _connection_values_from_ovcli(_load_ovcli_config(ovcli_path)) - except Exception as e: - print(f"\n Could not read OpenViking CLI config: {e}") - print(" No changes saved.\n") - return + print("\n OpenViking memory setup\n") + profiles = _discover_ovcli_profiles() + if profiles: setup_options = [ - ("Link to ovcli.conf", "Hermes follows the active OpenViking CLI config"), - ("Copy once", "Hermes won't follow future ovcli.conf changes"), - ("Manual Setup", "Enter a new URL/API key"), + ("Use existing OpenViking profile", "choose from detected ovcli.conf profiles"), + ("Create new OpenViking profile", "enter a new URL/API key"), ] choice = _curses_select( " OpenViking config source", @@ -914,130 +1648,143 @@ class OpenVikingMemoryProvider(MemoryProvider): return if choice == 0: - provider_config["use_ovcli_config"] = True - _remember_ovcli_path(provider_config, ovcli_path) - _write_env_vars(env_path, {}, remove_keys=_OPENVIKING_ENV_KEYS) - config["memory"]["provider"] = "openviking" - config["memory"]["openviking"] = provider_config - save_config(config) - print(f"\n Memory provider: openviking") - print(f" Linked config: {ovcli_path}") - print(" Start a new session to activate.\n") - return - - if choice == 1: - provider_config["use_ovcli_config"] = False - provider_config.pop("ovcli_config_path", None) - config["memory"]["provider"] = "openviking" - config["memory"]["openviking"] = provider_config - save_config(config) - _write_env_vars( - env_path, - _env_writes_from_connection_values(ovcli_values), - remove_keys=_OPENVIKING_ENV_KEYS, + result = _run_existing_profile_setup( + profiles=profiles, + select=_curses_select, + cancelled=_CANCELLED, + config=config, + provider_config=provider_config, + env_path=env_path, ) - print(f"\n Memory provider: openviking") - print(" Connection saved to .env") - print(" Start a new session to activate.\n") + if result is _SETUP_CANCELLED: + _print_cancelled_setup() + return + if result: + save_config(config) return - values = _prompt_manual_connection_values(_prompt, _curses_select, _CANCELLED) - if values is _SETUP_CANCELLED: - _print_cancelled_setup() - return - if values is None: - return + else: + print(" No existing OpenViking CLI profiles found. Creating a new config.") - save_choice = _curses_select( - " Save OpenViking config", - [ - ("Write ovcli.conf and link", "Hermes and ov use this config"), - ("Keep within Hermes", "Write values only to Hermes .env"), - ], - default=1, - cancel_returns=_CANCELLED, - ) - if save_choice == _CANCELLED: - _print_cancelled_setup() - return - - config["memory"]["provider"] = "openviking" - if save_choice == 0: - _write_ovcli_config(ovcli_path, values) - provider_config["use_ovcli_config"] = True - _remember_ovcli_path(provider_config, ovcli_path) - config["memory"]["openviking"] = provider_config - save_config(config) - _write_env_vars(env_path, {}, remove_keys=_OPENVIKING_ENV_KEYS) - print(f"\n Memory provider: openviking") - print(f" Updated config: {ovcli_path}") - else: - provider_config["use_ovcli_config"] = False - provider_config.pop("ovcli_config_path", None) - config["memory"]["openviking"] = provider_config - save_config(config) - _write_env_vars( - env_path, - _env_writes_from_connection_values(values), - remove_keys=_OPENVIKING_ENV_KEYS, - ) - print(f"\n Memory provider: openviking") - print(" Connection saved to .env") - print(" Start a new session to activate.\n") - return - - setup_options = [ - ("Create ovcli.conf and link", "Recommended"), - ("Configure Hermes only", "Write OpenViking values to Hermes .env"), - ] - choice = _curses_select( - " OpenViking config source", - setup_options, - default=0, - cancel_returns=_CANCELLED, + result = _run_create_profile_setup( + prompt=_prompt, + select=_curses_select, + cancelled=_CANCELLED, + config=config, + provider_config=provider_config, + env_path=env_path, ) - if choice == _CANCELLED: + if result is _SETUP_CANCELLED: _print_cancelled_setup() return - - defaults = { - "endpoint": _DEFAULT_ENDPOINT, - "api_key": "", - "account": "", - "user": "", - "agent": _DEFAULT_AGENT, - } - values = { - "endpoint": _prompt("OpenViking server URL", default=defaults["endpoint"]), - "api_key": _prompt("OpenViking API key", secret=True), - "account": _prompt("OpenViking account", default=defaults["account"]), - "user": _prompt("OpenViking user", default=defaults["user"]), - "agent": _prompt("OpenViking agent", default=defaults["agent"]), - } - - config["memory"]["provider"] = "openviking" - if choice == 0: - _write_ovcli_config(ovcli_path, values) - provider_config["use_ovcli_config"] = True - _remember_ovcli_path(provider_config, ovcli_path) - config["memory"]["openviking"] = provider_config + if result: save_config(config) - _write_env_vars(env_path, {}, remove_keys=_OPENVIKING_ENV_KEYS) - print(f"\n Memory provider: openviking") - print(f" Created config: {ovcli_path}") - else: - provider_config["use_ovcli_config"] = False - provider_config.pop("ovcli_config_path", None) - config["memory"]["openviking"] = provider_config - save_config(config) - _write_env_vars( - env_path, - _env_writes_from_connection_values(values), - remove_keys=_OPENVIKING_ENV_KEYS, + + def _start_runtime_openviking_waiter( + self, + *, + status_callback=None, + warning_callback=None, + ) -> None: + with self._runtime_start_lock: + if self._runtime_start_thread and self._runtime_start_thread.is_alive(): + return + self._runtime_start_thread = threading.Thread( + target=self._finish_runtime_openviking_start, + kwargs={ + "status_callback": status_callback, + "warning_callback": warning_callback, + }, + daemon=True, + name="openviking-runtime-start", ) - print(f"\n Memory provider: openviking") - print(" Connection saved to .env") - print(" Start a new session to activate.\n") + self._runtime_start_thread.start() + + def _finish_runtime_openviking_start( + self, + *, + status_callback=None, + warning_callback=None, + ) -> None: + endpoint = self._endpoint + if not _wait_for_openviking_health( + endpoint, + timeout_seconds=_LOCAL_OPENVIKING_AUTOSTART_TIMEOUT, + ): + _emit_runtime_warning( + _runtime_openviking_timeout_message(endpoint), + warning_callback, + ) + return + + try: + client = _VikingClient( + endpoint, + self._api_key, + account=self._account, + user=self._user, + agent=self._agent, + ) + if not client.health(): + _emit_runtime_warning( + f"OpenViking server at {endpoint} is still not reachable after auto-start; " + "OpenViking memory disabled for this Hermes run.", + warning_callback, + ) + return + except ImportError: + logger.warning("httpx not installed — OpenViking plugin disabled") + return + except Exception as e: + _emit_runtime_warning( + f"OpenViking server at {endpoint} could not be attached after auto-start: {e}. " + "OpenViking memory disabled for this Hermes run.", + warning_callback, + ) + return + + self._client = client + _emit_runtime_status( + f"Local OpenViking server at {endpoint} is reachable; OpenViking memory is active for later turns.", + status_callback, + ) + + def _handle_runtime_openviking_unreachable( + self, + *, + status_callback=None, + warning_callback=None, + ) -> None: + endpoint = self._endpoint + if not _is_local_openviking_url(endpoint): + _emit_runtime_warning( + f"Remote OpenViking server at {endpoint} is not reachable; " + "OpenViking memory disabled for this Hermes run. " + "Check the configured endpoint and network connectivity.", + warning_callback, + ) + self._client = None + return + + started, start_message = _start_local_openviking_server(endpoint) + if not started: + _emit_runtime_warning( + f"Local OpenViking server at {endpoint} is not reachable. {start_message} " + "OpenViking memory disabled for this Hermes run.", + warning_callback, + ) + self._client = None + return + + self._client = None + _emit_runtime_status( + f"{start_message} OpenViking memory is starting in the background and will attach when ready.", + status_callback, + ) + self._start_runtime_openviking_waiter( + status_callback=status_callback, + warning_callback=warning_callback, + ) def initialize(self, session_id: str, **kwargs) -> None: settings = _resolve_connection_settings(_load_hermes_openviking_config()) @@ -1048,6 +1795,16 @@ class OpenVikingMemoryProvider(MemoryProvider): self._agent = settings["agent"] self._session_id = session_id self._turn_count = 0 + warning_callback = ( + kwargs.get("warning_callback") + if kwargs.get("platform") == "cli" + else None + ) + status_callback = ( + kwargs.get("status_callback") + if kwargs.get("platform") == "cli" + else None + ) try: self._client = _VikingClient( @@ -1055,8 +1812,10 @@ class OpenVikingMemoryProvider(MemoryProvider): account=self._account, user=self._user, agent=self._agent, ) if not self._client.health(): - logger.warning("OpenViking server at %s is not reachable", self._endpoint) - self._client = None + self._handle_runtime_openviking_unreachable( + status_callback=status_callback, + warning_callback=warning_callback, + ) except ImportError: logger.warning("httpx not installed — OpenViking plugin disabled") self._client = None diff --git a/tests/hermes_cli/test_memory_setup.py b/tests/hermes_cli/test_memory_setup.py index 1e75a5a2add..b5b574230c7 100644 --- a/tests/hermes_cli/test_memory_setup.py +++ b/tests/hermes_cli/test_memory_setup.py @@ -45,6 +45,22 @@ def test_curses_select_accepts_explicit_cancel_value(monkeypatch): assert captured["cancel_returns"] == _CANCELLED +def test_curses_select_clears_after_picker_returns(monkeypatch): + events = [] + + def fake_radiolist(title, items, selected=0, *, cancel_returns=None): + events.append("picker") + return selected + + monkeypatch.setattr("hermes_cli.curses_ui.curses_radiolist", fake_radiolist) + monkeypatch.setattr(memory_setup, "_clear_interactive_transition", lambda: events.append("clear")) + + result = _curses_select("Pick one", [("first", "")], default=0) + + assert result == 0 + assert events == ["picker", "clear"] + + def test_cmd_setup_top_level_cancel_writes_nothing(monkeypatch): save_config = MagicMock() load_config = MagicMock(side_effect=AssertionError("cancel should not load config")) @@ -95,6 +111,60 @@ def test_cmd_setup_clears_interactive_picker_before_provider_post_setup(monkeypa assert events == ["select", "clear", "install", "post_setup"] +def test_cmd_setup_provider_clears_before_provider_post_setup(monkeypatch): + events = [] + + class PostSetupProvider: + def post_setup(self, hermes_home, config): + events.append("post_setup") + + monkeypatch.setattr(memory_setup, "_get_available_providers", lambda: [("openviking", "local", PostSetupProvider())]) + monkeypatch.setattr(memory_setup, "_clear_interactive_transition", lambda: events.append("clear"), raising=False) + monkeypatch.setattr(memory_setup, "_install_dependencies", lambda name: events.append("install")) + monkeypatch.setattr(memory_setup, "get_hermes_home", lambda: "/tmp/hermes-test") + monkeypatch.setattr("hermes_cli.config.load_config", lambda: {"memory": {}}) + + memory_setup.cmd_setup_provider("openviking") + + assert events == ["clear", "install", "post_setup"] + + +def test_cmd_status_prefers_provider_status_config(monkeypatch, capsys): + class StatusProvider: + def get_status_config(self, provider_config): + assert provider_config["endpoint"] == "http://stale.local" + return { + "use_ovcli_config": True, + "ovcli_config_path": "/tmp/ovcli.conf.VPS_ROOT", + "endpoint": "https://vps.example", + "account": "acct", + "user": "alice", + "agent": "hermes", + } + + def is_available(self): + return True + + config = { + "memory": { + "provider": "openviking", + "openviking": { + "use_ovcli_config": True, + "ovcli_config_path": "/tmp/ovcli.conf.VPS_ROOT", + "endpoint": "http://stale.local", + }, + } + } + monkeypatch.setattr("hermes_cli.config.load_config", lambda: config) + monkeypatch.setattr(memory_setup, "_get_available_providers", lambda: [("openviking", "API key / local", StatusProvider())]) + + memory_setup.cmd_status(SimpleNamespace()) + + output = capsys.readouterr().out + assert "endpoint: https://vps.example" in output + assert "http://stale.local" not in output + + def test_cmd_setup_generic_choice_cancel_writes_nothing(tmp_path, monkeypatch): class ChoiceProvider: def __init__(self): diff --git a/tests/hermes_cli/test_secret_prompt.py b/tests/hermes_cli/test_secret_prompt.py index 50aec43cd88..d33bb07ea4d 100644 --- a/tests/hermes_cli/test_secret_prompt.py +++ b/tests/hermes_cli/test_secret_prompt.py @@ -25,7 +25,7 @@ def test_collect_masked_input_shows_feedback_without_echoing_secret(): value, output = _run_collect("secret\n") assert value == "secret" - assert output == "API key: ******\n" + assert output == "API key: ******\r\n" assert "secret" not in output @@ -33,7 +33,7 @@ def test_collect_masked_input_handles_backspace(): value, output = _run_collect("sec\x7fret\r") assert value == "seret" - assert output == "API key: ***\b \b***\n" + assert output == "API key: ***\b \b***\r\n" assert "secret" not in output @@ -47,7 +47,7 @@ def test_collect_masked_input_raises_keyboard_interrupt(): "API key: ", ) - assert "".join(output) == "API key: \n" + assert "".join(output) == "API key: \r\n" def test_masked_secret_prompt_falls_back_to_getpass_for_non_tty(monkeypatch): diff --git a/tests/plugins/memory/test_openviking_provider.py b/tests/plugins/memory/test_openviking_provider.py index 190f8ba1b78..36e0658f338 100644 --- a/tests/plugins/memory/test_openviking_provider.py +++ b/tests/plugins/memory/test_openviking_provider.py @@ -11,6 +11,12 @@ import plugins.memory.openviking as openviking_module from plugins.memory.openviking import OpenVikingMemoryProvider, _VikingClient +@pytest.fixture(autouse=True) +def _isolate_openviking_home(tmp_path, monkeypatch): + home = tmp_path / "home" + monkeypatch.setattr(openviking_module.Path, "home", staticmethod(lambda: home)) + + def _clear_openviking_env(monkeypatch): for key in ( "OPENVIKING_ENDPOINT", @@ -53,6 +59,16 @@ def _allow_setup_validation(monkeypatch, *, root_access: bool = False): lambda values: (root_access, "" if root_access else "Requires role: root"), raising=False, ) + monkeypatch.setattr( + openviking_module, + "_validate_openviking_setup_values", + lambda values, *, require_api_key=False: ( + True, + "", + "root" if root_access else ("user" if values.get("api_key") else None), + ), + raising=False, + ) @pytest.mark.skipif(os.name == "nt", reason="POSIX file modes") @@ -112,8 +128,8 @@ def test_linked_ovcli_config_is_read_at_runtime(tmp_path, monkeypatch): assert settings == { "endpoint": "http://openviking-one.local", "api_key": "key-one", - "account": "acct-one", - "user": "alice", + "account": "", + "user": "", "agent": "agent-one", } @@ -170,101 +186,222 @@ def test_openviking_env_overrides_linked_ovcli_config(tmp_path, monkeypatch): } -def test_post_setup_link_existing_ovcli_clears_hermes_env(tmp_path, monkeypatch): +def test_openviking_cli_config_env_overrides_saved_profile_path(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + saved_path = tmp_path / "ovcli.conf.saved" + env_path = tmp_path / "ovcli.conf.env" + saved_path.write_text( + json.dumps({"url": "http://saved.local", "api_key": "saved-key"}), + encoding="utf-8", + ) + env_path.write_text( + json.dumps({"url": "http://env-profile.local", "api_key": "env-profile-key"}), + encoding="utf-8", + ) + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(env_path)) + + settings = openviking_module._resolve_connection_settings({ + "use_ovcli_config": True, + "ovcli_config_path": str(saved_path), + }) + + assert settings["endpoint"] == "http://env-profile.local" + assert settings["api_key"] == "env-profile-key" + + +def test_connection_values_omit_stale_identity_for_user_key_with_root_key(): + values = openviking_module._connection_values_from_ovcli({ + "url": "https://openviking.example", + "api_key": "user-key", + "root_api_key": "root-key", + "account": "stale-account", + "user": "stale-user", + }) + + assert values["api_key"] == "user-key" + assert values["account"] == "" + assert values["user"] == "" + + +def test_discover_ovcli_profiles_lists_saved_profiles_without_active_label(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + openviking_home = tmp_path / ".openviking" + openviking_home.mkdir() + env_path = tmp_path / "custom-ovcli.conf" + env_path.write_text(json.dumps({"url": "http://env.local"}), encoding="utf-8") + (openviking_home / "ovcli.conf").write_text( + json.dumps({"url": "https://vps.example", "api_key": "secret"}), + encoding="utf-8", + ) + (openviking_home / "ovcli.conf.VPS").write_text( + json.dumps({"url": "https://vps.example", "api_key": "secret"}), + encoding="utf-8", + ) + (openviking_home / "ovcli.conf.bak").write_text( + json.dumps({"url": "http://backup.local"}), + encoding="utf-8", + ) + (openviking_home / "ovcli.conf.bad").write_text("{", encoding="utf-8") + monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(env_path)) + monkeypatch.setattr(openviking_module.Path, "home", staticmethod(lambda: tmp_path)) + + profiles = openviking_module._discover_ovcli_profiles() + + assert [(profile.source, profile.name, profile.path) for profile in profiles] == [ + ("env", "OPENVIKING_CLI_CONFIG_FILE", env_path), + ("saved", "VPS", openviking_home / "ovcli.conf.VPS"), + ] + assert profiles[1].is_active is True + assert openviking_module._profile_display_name(profiles[1]) == "VPS" + assert "active" not in openviking_module._profile_description(profiles[1]).lower() + + +def test_link_ovcli_profile_removes_stale_inline_config(tmp_path): + env_path = tmp_path / ".env" + env_path.write_text("OPENVIKING_ENDPOINT=http://old.local\nOTHER_KEY=keep\n", encoding="utf-8") + config = {"memory": {}} + provider_config = { + "use_ovcli_config": False, + "endpoint": "http://stale.local", + "api_key": "stale-key", + "account": "default", + "user": "default", + "agent": "stale-agent", + "api_key_type": "root", + } + ovcli_path = tmp_path / "ovcli.conf.VPS_ROOT" + + openviking_module._link_ovcli_profile( + config=config, + provider_config=provider_config, + env_path=env_path, + ovcli_path=ovcli_path, + ) + + assert config["memory"]["openviking"] == { + "use_ovcli_config": True, + "ovcli_config_path": str(ovcli_path), + } + assert "OPENVIKING_ENDPOINT" not in env_path.read_text(encoding="utf-8") + assert "OTHER_KEY=keep" in env_path.read_text(encoding="utf-8") + + +def test_post_setup_existing_profile_picker_validates_and_links_saved_profile(tmp_path, monkeypatch): _clear_openviking_env(monkeypatch) hermes_home = tmp_path / "hermes" hermes_home.mkdir() env_path = hermes_home / ".env" - env_path.write_text( - "OPENVIKING_ENDPOINT=http://old.local\n" - "OPENVIKING_ACCOUNT=old-account\n" - "OTHER_KEY=keep\n", + env_path.write_text("OPENVIKING_ENDPOINT=http://old.local\nOTHER_KEY=keep\n", encoding="utf-8") + openviking_home = tmp_path / ".openviking" + openviking_home.mkdir() + active_path = openviking_home / "ovcli.conf" + saved_path = openviking_home / "ovcli.conf.VPS" + active_path.write_text(json.dumps({"url": "http://active.local"}), encoding="utf-8") + saved_path.write_text( + json.dumps({"url": "https://vps.example", "api_key": "user-key"}), encoding="utf-8", ) - ovcli_path = tmp_path / "ovcli.conf" - original_ovcli = json.dumps({"url": "http://openviking.local"}) - ovcli_path.write_text(original_ovcli, encoding="utf-8") monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + monkeypatch.setattr(openviking_module.Path, "home", staticmethod(lambda: tmp_path)) from hermes_cli import memory_setup - monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: 0) + validate_calls = [] + + def validate_values(values, *, require_api_key=False): + validate_calls.append(dict(values)) + return True, "", "user" + + monkeypatch.setattr( + openviking_module, + "_validate_openviking_setup_values", + validate_values, + raising=False, + ) + choices = iter([0, 0]) + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: next(choices)) config = {"memory": {}} OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + assert validate_calls == [{ + "endpoint": "https://vps.example", + "api_key": "user-key", + "root_api_key": "", + "account": "", + "user": "", + "agent": "", + }] assert config["memory"]["provider"] == "openviking" - assert config["memory"]["openviking"]["use_ovcli_config"] is True - assert config["memory"]["openviking"]["ovcli_config_path"] == str(ovcli_path) + assert config["memory"]["openviking"] == { + "use_ovcli_config": True, + "ovcli_config_path": str(saved_path), + } env_text = env_path.read_text(encoding="utf-8") assert "OPENVIKING_" not in env_text assert "OTHER_KEY=keep" in env_text - assert ovcli_path.read_text(encoding="utf-8") == original_ovcli -def test_post_setup_copy_existing_ovcli_writes_hermes_env(tmp_path, monkeypatch): +def test_post_setup_create_remote_user_profile_can_mirror_to_openviking_store(tmp_path, monkeypatch): _clear_openviking_env(monkeypatch) hermes_home = tmp_path / "hermes" hermes_home.mkdir() - ovcli_path = tmp_path / "ovcli.conf" - original_ovcli = json.dumps({ - "url": "http://openviking.local", - "api_key": "test-key", - "account": "acct", - "user": "alice", - "agent_id": "agent", - }) - ovcli_path.write_text(original_ovcli, encoding="utf-8") monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + monkeypatch.setattr(openviking_module.Path, "home", staticmethod(lambda: tmp_path)) + _allow_setup_validation(monkeypatch) from hermes_cli import memory_setup - monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: 1) - config = {"memory": {}} - - OpenVikingMemoryProvider().post_setup(str(hermes_home), config) - - assert config["memory"]["provider"] == "openviking" - assert config["memory"]["openviking"]["use_ovcli_config"] is False - env_text = (hermes_home / ".env").read_text(encoding="utf-8") - assert "OPENVIKING_ENDPOINT=http://openviking.local" in env_text - assert "OPENVIKING_API_KEY=test-key" in env_text - assert "OPENVIKING_ACCOUNT=acct" in env_text - assert "OPENVIKING_USER=alice" in env_text - assert "OPENVIKING_AGENT=agent" in env_text - assert ovcli_path.read_text(encoding="utf-8") == original_ovcli - - -def test_post_setup_manual_remote_root_writes_ovcli_and_links(tmp_path, monkeypatch): - _clear_openviking_env(monkeypatch) - hermes_home = tmp_path / "hermes" - hermes_home.mkdir() - env_path = hermes_home / ".env" - env_path.write_text("OPENVIKING_ENDPOINT=http://old.local\n", encoding="utf-8") - ovcli_path = tmp_path / "ovcli.conf" - ovcli_path.write_text(json.dumps({"url": "http://old.local"}), encoding="utf-8") - monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) - _allow_setup_validation(monkeypatch, root_access=True) - - from hermes_cli import memory_setup - - choices = iter([2, 1, 0]) - monkeypatch.setattr( - memory_setup, - "_curses_select", - lambda *args, **kwargs: next(choices), - ) + choices = iter([1, 0, 1]) + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: next(choices)) monkeypatch.setattr( memory_setup, "_prompt", _prompt_from_values({ "OpenViking server URL": "https://openviking.example", - "OpenViking root API key": "root-secret", - "OpenViking account": "acct", - "OpenViking user": "alice", + "OpenViking user API key": "user-secret", + "OpenViking agent": "hermes", + "OpenViking profile name": "VPS", + }), + ) + config = {"memory": {}} + + OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + + mirrored_path = tmp_path / ".openviking" / "ovcli.conf.VPS" + assert mirrored_path.exists() + assert json.loads(mirrored_path.read_text(encoding="utf-8")) == { + "url": "https://openviking.example", + "api_key": "user-secret", + "actor_peer_id": "hermes", + } + assert config["memory"]["provider"] == "openviking" + assert config["memory"]["openviking"] == { + "use_ovcli_config": True, + "ovcli_config_path": str(mirrored_path), + } + env_path = hermes_home / ".env" + if env_path.exists(): + assert "OPENVIKING_" not in env_path.read_text(encoding="utf-8") + + +def test_post_setup_create_remote_user_can_keep_hermes_only(tmp_path, monkeypatch): + _clear_openviking_env(monkeypatch) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + _allow_setup_validation(monkeypatch) + + from hermes_cli import memory_setup + + choices = iter([1, 0, 0]) + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: next(choices)) + monkeypatch.setattr( + memory_setup, + "_prompt", + _prompt_from_values({ + "OpenViking server URL": "https://openviking.example", + "OpenViking user API key": "user-secret", "OpenViking agent": "agent", }), ) @@ -273,378 +410,83 @@ def test_post_setup_manual_remote_root_writes_ovcli_and_links(tmp_path, monkeypa OpenVikingMemoryProvider().post_setup(str(hermes_home), config) assert config["memory"]["provider"] == "openviking" - assert config["memory"]["openviking"]["use_ovcli_config"] is True - assert config["memory"]["openviking"]["ovcli_config_path"] == str(ovcli_path) - assert env_path.read_text(encoding="utf-8") == "" - data = json.loads(ovcli_path.read_text(encoding="utf-8")) - assert data == { - "url": "https://openviking.example", - "api_key": "root-secret", - "account": "acct", - "user": "alice", - "agent_id": "agent", - } + assert config["memory"]["openviking"] == {"use_ovcli_config": False} + env_text = (hermes_home / ".env").read_text(encoding="utf-8") + assert "OPENVIKING_ENDPOINT=https://openviking.example" in env_text + assert "OPENVIKING_API_KEY=user-secret" in env_text + assert "OPENVIKING_AGENT=agent" in env_text + assert not (tmp_path / "home" / ".openviking").exists() -def test_post_setup_manual_remote_user_keeps_only_hermes_env(tmp_path, monkeypatch): +def test_post_setup_create_openviking_service_validates_after_api_key(tmp_path, monkeypatch): _clear_openviking_env(monkeypatch) hermes_home = tmp_path / "hermes" hermes_home.mkdir() - ovcli_path = tmp_path / "ovcli.conf" - original_ovcli = json.dumps({"url": "http://old.local"}) - ovcli_path.write_text(original_ovcli, encoding="utf-8") monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) - _allow_setup_validation(monkeypatch) from hermes_cli import memory_setup - choices = iter([2, 0, 1]) + validation_calls = [] + + def validate_values(values, *, require_api_key=False): + validation_calls.append((dict(values), require_api_key)) + return True, "", "user" + monkeypatch.setattr( - memory_setup, - "_curses_select", - lambda *args, **kwargs: next(choices), + openviking_module, + "_validate_openviking_reachability", + MagicMock(side_effect=AssertionError("service setup validates only after API key entry")), ) + monkeypatch.setattr(openviking_module, "_validate_openviking_setup_values", validate_values) + choices = iter([0, 0]) + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: next(choices)) monkeypatch.setattr( memory_setup, "_prompt", _prompt_from_values( { - "OpenViking server URL": "https://openviking.example", - "OpenViking user API key": "user-secret", + "OpenViking API key": "service-secret", "OpenViking agent": "agent", }, - forbidden={ - "OpenViking account", - "OpenViking root API key", - "OpenViking user", - }, + forbidden={"OpenViking server URL", "OpenViking user API key", "OpenViking root API key"}, ), ) config = {"memory": {}} OpenVikingMemoryProvider().post_setup(str(hermes_home), config) - assert config["memory"]["provider"] == "openviking" - assert config["memory"]["openviking"]["use_ovcli_config"] is False - assert ovcli_path.read_text(encoding="utf-8") == original_ovcli + assert validation_calls == [( + { + "endpoint": "https://api.vikingdb.cn-beijing.volces.com/openviking", + "api_key": "service-secret", + "root_api_key": "", + "account": "", + "user": "", + "agent": "agent", + "api_key_type": "user", + }, + True, + )] env_text = (hermes_home / ".env").read_text(encoding="utf-8") - assert "OPENVIKING_ENDPOINT=https://openviking.example" in env_text - assert "OPENVIKING_API_KEY=user-secret" in env_text + assert "OPENVIKING_ENDPOINT=https://api.vikingdb.cn-beijing.volces.com/openviking" in env_text + assert "OPENVIKING_API_KEY=service-secret" in env_text assert "OPENVIKING_AGENT=agent" in env_text - assert "OPENVIKING_ACCOUNT" not in env_text - assert "OPENVIKING_USER" not in env_text -def test_post_setup_manual_validation_failure_writes_nothing(tmp_path, monkeypatch): +def test_post_setup_remote_blank_api_key_cancels_without_saving(tmp_path, monkeypatch): _clear_openviking_env(monkeypatch) hermes_home = tmp_path / "hermes" hermes_home.mkdir() - ovcli_path = tmp_path / "ovcli.conf" - original_ovcli = json.dumps({"url": "http://old.local"}) - ovcli_path.write_text(original_ovcli, encoding="utf-8") monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) - _allow_setup_validation(monkeypatch) - monkeypatch.setattr( - openviking_module, - "_validate_openviking_auth", - lambda values: (False, "OpenViking authentication validation failed: bad key"), - raising=False, - ) - - from hermes_cli import config as hermes_config - from hermes_cli import memory_setup - - save_config = MagicMock() - choices = iter([2, 0, 1]) - monkeypatch.setattr(hermes_config, "save_config", save_config) - monkeypatch.setattr( - memory_setup, - "_curses_select", - lambda *args, **kwargs: next(choices), - ) - monkeypatch.setattr( - memory_setup, - "_prompt", - _prompt_from_values({ - "OpenViking server URL": "https://openviking.example", - "OpenViking user API key": "bad-key", - "OpenViking agent": "agent", - }), - ) - config = {"memory": {"provider": "builtin"}} - - OpenVikingMemoryProvider().post_setup(str(hermes_home), config) - - save_config.assert_not_called() - assert config == {"memory": {"provider": "builtin"}} - assert ovcli_path.read_text(encoding="utf-8") == original_ovcli - assert not (hermes_home / ".env").exists() - - -def test_post_setup_manual_retries_base_url_until_reachable(tmp_path, monkeypatch): - _clear_openviking_env(monkeypatch) - hermes_home = tmp_path / "hermes" - hermes_home.mkdir() - ovcli_path = tmp_path / "ovcli.conf" - ovcli_path.write_text(json.dumps({"url": "http://old.local"}), encoding="utf-8") - monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) - monkeypatch.setattr(openviking_module, "_validate_openviking_auth", lambda values: (True, "")) - - reachability_calls = [] - - def validate_reachability(endpoint): - reachability_calls.append(endpoint) - if endpoint == "http://bad.local:1933": - return False, "OpenViking server is not reachable at http://bad.local:1933." - return True, "" - - monkeypatch.setattr(openviking_module, "_validate_openviking_reachability", validate_reachability) - monkeypatch.setattr(openviking_module, "_validate_openviking_root_access", lambda values: (False, "Requires role: root")) - - from hermes_cli import memory_setup - - prompts = { - "OpenViking server URL": iter(["http://bad.local:1933", "http://localhost:1933"]), - "OpenViking agent": iter(["agent"]), - } - - def fake_prompt(label, default=None, secret=False): - return next(prompts[label]) - - choices = iter([2, 0, 0, 1]) - monkeypatch.setattr( - memory_setup, - "_curses_select", - lambda *args, **kwargs: next(choices), - ) - monkeypatch.setattr(memory_setup, "_prompt", fake_prompt) - config = {"memory": {}} - - OpenVikingMemoryProvider().post_setup(str(hermes_home), config) - - assert reachability_calls == ["http://bad.local:1933", "http://localhost:1933"] - assert config["memory"]["provider"] == "openviking" - env_text = (hermes_home / ".env").read_text(encoding="utf-8") - assert "OPENVIKING_ENDPOINT=http://localhost:1933" in env_text - - -def test_post_setup_manual_retries_user_key_until_status_valid(tmp_path, monkeypatch): - _clear_openviking_env(monkeypatch) - hermes_home = tmp_path / "hermes" - hermes_home.mkdir() - ovcli_path = tmp_path / "ovcli.conf" - ovcli_path.write_text(json.dumps({"url": "http://old.local"}), encoding="utf-8") - monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) monkeypatch.setattr(openviking_module, "_validate_openviking_reachability", lambda endpoint: (True, "")) - monkeypatch.setattr(openviking_module, "_validate_openviking_root_access", lambda values: (False, "Requires role: root")) - - auth_calls = [] - - def validate_auth(values): - auth_calls.append(dict(values)) - if values["api_key"] == "bad-key": - return False, "OpenViking authentication validation failed: bad key" - return True, "" - - monkeypatch.setattr(openviking_module, "_validate_openviking_auth", validate_auth) - - from hermes_cli import memory_setup - - prompts = { - "OpenViking server URL": iter(["https://openviking.example"]), - "OpenViking user API key": iter(["bad-key", "good-key"]), - "OpenViking agent": iter(["agent", "agent"]), - } - - def fake_prompt(label, default=None, secret=False): - return next(prompts[label]) - - choices = iter([2, 0, 0, 0, 1]) - monkeypatch.setattr( - memory_setup, - "_curses_select", - lambda *args, **kwargs: next(choices), - ) - monkeypatch.setattr(memory_setup, "_prompt", fake_prompt) - config = {"memory": {}} - - OpenVikingMemoryProvider().post_setup(str(hermes_home), config) - - assert [call["api_key"] for call in auth_calls] == ["bad-key", "good-key"] - env_text = (hermes_home / ".env").read_text(encoding="utf-8") - assert "OPENVIKING_API_KEY=good-key" in env_text - - -def test_post_setup_manual_user_key_rejects_root_key(tmp_path, monkeypatch): - _clear_openviking_env(monkeypatch) - hermes_home = tmp_path / "hermes" - hermes_home.mkdir() - ovcli_path = tmp_path / "ovcli.conf" - ovcli_path.write_text(json.dumps({"url": "http://old.local"}), encoding="utf-8") - monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) - monkeypatch.setattr(openviking_module, "_validate_openviking_reachability", lambda endpoint: (True, "")) - monkeypatch.setattr(openviking_module, "_validate_openviking_auth", lambda values: (True, "")) - - root_checks = [] - - def validate_root(values): - root_checks.append(values["api_key"]) - if values["api_key"] == "root-secret": - return True, "" - return False, "Requires role: root" - - monkeypatch.setattr(openviking_module, "_validate_openviking_root_access", validate_root) - - from hermes_cli import memory_setup - - prompts = { - "OpenViking server URL": iter(["https://openviking.example"]), - "OpenViking user API key": iter(["root-secret", "user-secret"]), - "OpenViking agent": iter(["agent", "agent"]), - } - - def fake_prompt(label, default=None, secret=False): - return next(prompts[label]) - - choices = iter([2, 0, 0, 0, 1]) - monkeypatch.setattr( - memory_setup, - "_curses_select", - lambda *args, **kwargs: next(choices), - ) - monkeypatch.setattr(memory_setup, "_prompt", fake_prompt) - config = {"memory": {}} - - OpenVikingMemoryProvider().post_setup(str(hermes_home), config) - - assert root_checks == ["root-secret", "user-secret"] - env_text = (hermes_home / ".env").read_text(encoding="utf-8") - assert "OPENVIKING_API_KEY=user-secret" in env_text - assert "OPENVIKING_API_KEY=root-secret" not in env_text - - -def test_post_setup_manual_root_key_requires_root_only_validation(tmp_path, monkeypatch): - _clear_openviking_env(monkeypatch) - hermes_home = tmp_path / "hermes" - hermes_home.mkdir() - ovcli_path = tmp_path / "ovcli.conf" - ovcli_path.write_text(json.dumps({"url": "http://old.local"}), encoding="utf-8") - monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) - monkeypatch.setattr(openviking_module, "_validate_openviking_reachability", lambda endpoint: (True, "")) - monkeypatch.setattr(openviking_module, "_validate_openviking_auth", lambda values: (True, "")) - - root_calls = [] - - def validate_root(values): - root_calls.append(dict(values)) - return True, "" - - monkeypatch.setattr(openviking_module, "_validate_openviking_root_access", validate_root) - - from hermes_cli import memory_setup - - monkeypatch.setattr( - memory_setup, - "_prompt", - _prompt_from_values({ - "OpenViking server URL": "https://openviking.example", - "OpenViking root API key": "root-secret", - "OpenViking account": "acct", - "OpenViking user": "alice", - "OpenViking agent": "agent", - }), - ) - choices = iter([2, 1, 1]) - monkeypatch.setattr( - memory_setup, - "_curses_select", - lambda *args, **kwargs: next(choices), - ) - config = {"memory": {}} - - OpenVikingMemoryProvider().post_setup(str(hermes_home), config) - - assert [call["api_key"] for call in root_calls] == ["root-secret"] - assert config["memory"]["provider"] == "openviking" - - -def test_post_setup_manual_retries_root_key_before_account_prompts(tmp_path, monkeypatch): - _clear_openviking_env(monkeypatch) - hermes_home = tmp_path / "hermes" - hermes_home.mkdir() - ovcli_path = tmp_path / "ovcli.conf" - ovcli_path.write_text(json.dumps({"url": "http://old.local"}), encoding="utf-8") - monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) - monkeypatch.setattr(openviking_module, "_validate_openviking_reachability", lambda endpoint: (True, "")) - monkeypatch.setattr(openviking_module, "_validate_openviking_auth", lambda values: (True, "")) - - def validate_root(values): - if values["api_key"] == "bad-root": - return False, "OpenViking root API key validation failed: bad key" - return True, "" - - monkeypatch.setattr(openviking_module, "_validate_openviking_root_access", validate_root) - - from hermes_cli import memory_setup - - prompt_events = [] - prompts = { - "OpenViking server URL": iter(["https://openviking.example"]), - "OpenViking root API key": iter(["bad-root", "good-root"]), - "OpenViking account": iter(["acct"]), - "OpenViking user": iter(["alice"]), - "OpenViking agent": iter(["agent"]), - } - - def fake_prompt(label, default=None, secret=False): - prompt_events.append(label) - return next(prompts[label]) - - choices = iter([2, 1, 0, 1, 1]) - monkeypatch.setattr( - memory_setup, - "_curses_select", - lambda *args, **kwargs: next(choices), - ) - monkeypatch.setattr(memory_setup, "_prompt", fake_prompt) - config = {"memory": {}} - - OpenVikingMemoryProvider().post_setup(str(hermes_home), config) - - assert prompt_events.index("OpenViking account") > prompt_events.index("OpenViking root API key") - assert prompt_events.count("OpenViking account") == 1 - env_text = (hermes_home / ".env").read_text(encoding="utf-8") - assert "OPENVIKING_API_KEY=good-root" in env_text - - -def test_post_setup_manual_remote_requires_api_key(tmp_path, monkeypatch): - _clear_openviking_env(monkeypatch) - hermes_home = tmp_path / "hermes" - hermes_home.mkdir() - ovcli_path = tmp_path / "ovcli.conf" - original_ovcli = json.dumps({"url": "http://old.local"}) - ovcli_path.write_text(original_ovcli, encoding="utf-8") - monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) from hermes_cli import config as hermes_config from hermes_cli import memory_setup save_config = MagicMock() monkeypatch.setattr(hermes_config, "save_config", save_config) - choices = iter([2, 0, 1]) - monkeypatch.setattr( - memory_setup, - "_curses_select", - lambda *args, **kwargs: next(choices), - ) + choices = iter([1, 0, 1]) + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: next(choices)) monkeypatch.setattr( memory_setup, "_prompt", @@ -659,219 +501,504 @@ def test_post_setup_manual_remote_requires_api_key(tmp_path, monkeypatch): save_config.assert_not_called() assert config == {"memory": {"provider": "builtin"}} - assert ovcli_path.read_text(encoding="utf-8") == original_ovcli assert not (hermes_home / ".env").exists() -def test_post_setup_manual_root_requires_account_and_user(tmp_path, monkeypatch): +def test_post_setup_user_key_path_can_route_detected_root_key_to_root_setup(tmp_path, monkeypatch): _clear_openviking_env(monkeypatch) hermes_home = tmp_path / "hermes" hermes_home.mkdir() - ovcli_path = tmp_path / "ovcli.conf" - original_ovcli = json.dumps({"url": "http://old.local"}) - ovcli_path.write_text(original_ovcli, encoding="utf-8") monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) - _allow_setup_validation(monkeypatch, root_access=True) - from hermes_cli import config as hermes_config from hermes_cli import memory_setup - save_config = MagicMock() - choices = iter([2, 1, 1]) - monkeypatch.setattr(hermes_config, "save_config", save_config) - monkeypatch.setattr( - memory_setup, - "_curses_select", - lambda *args, **kwargs: next(choices), - ) - monkeypatch.setattr( - memory_setup, - "_prompt", - _prompt_from_values({ + def validate_values(values, *, require_api_key=False): + assert values["api_key"] == "root-secret" + return True, "", "root" + + monkeypatch.setattr(openviking_module, "_validate_openviking_reachability", lambda endpoint: (True, "")) + monkeypatch.setattr(openviking_module, "_validate_openviking_setup_values", validate_values) + choices = iter([1, 0, 0, 0]) + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: next(choices)) + prompt_events = [] + + def fake_prompt(label, default=None, secret=False): + if label == "OpenViking root API key": + raise AssertionError("OpenViking root API key should not be re-prompted") + prompt_events.append(label) + values = { "OpenViking server URL": "https://openviking.example", - "OpenViking root API key": "root-secret", - "OpenViking account": "", + "OpenViking user API key": "root-secret", + "OpenViking account": "acct", "OpenViking user": "alice", - }), - ) - config = {"memory": {"provider": "builtin"}} + "OpenViking agent": "agent", + } + return values.get(label, default or "") + + monkeypatch.setattr(memory_setup, "_prompt", fake_prompt) + config = {"memory": {}} OpenVikingMemoryProvider().post_setup(str(hermes_home), config) - save_config.assert_not_called() - assert config == {"memory": {"provider": "builtin"}} - assert ovcli_path.read_text(encoding="utf-8") == original_ovcli - assert not (hermes_home / ".env").exists() + assert prompt_events.count("OpenViking agent") == 1 + env_text = (hermes_home / ".env").read_text(encoding="utf-8") + assert "OPENVIKING_API_KEY=root-secret" in env_text + assert "OPENVIKING_ACCOUNT=acct" in env_text + assert "OPENVIKING_USER=alice" in env_text + assert "OPENVIKING_AGENT=agent" in env_text -def test_post_setup_manual_local_allows_blank_api_key(tmp_path, monkeypatch): +def test_post_setup_root_key_path_can_route_detected_user_key_to_user_setup(tmp_path, monkeypatch): _clear_openviking_env(monkeypatch) hermes_home = tmp_path / "hermes" hermes_home.mkdir() - ovcli_path = tmp_path / "ovcli.conf" - original_ovcli = json.dumps({"url": "http://old.local"}) - ovcli_path.write_text(original_ovcli, encoding="utf-8") monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) - _allow_setup_validation(monkeypatch) from hermes_cli import memory_setup - choices = iter([2, 0, 1]) - monkeypatch.setattr( - memory_setup, - "_curses_select", - lambda *args, **kwargs: next(choices), - ) + def validate_values(values, *, require_api_key=False): + assert values["api_key"] == "user-secret" + return True, "", "user" + + monkeypatch.setattr(openviking_module, "_validate_openviking_reachability", lambda endpoint: (True, "")) + monkeypatch.setattr(openviking_module, "_validate_openviking_setup_values", validate_values) + choices = iter([1, 1, 0, 0]) + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: next(choices)) monkeypatch.setattr( memory_setup, "_prompt", _prompt_from_values( { - "OpenViking server URL": "http://localhost:1933", + "OpenViking server URL": "https://openviking.example", + "OpenViking root API key": "user-secret", "OpenViking agent": "agent", }, - forbidden={ - "OpenViking account", - "OpenViking root API key", - "OpenViking user", - "OpenViking user API key", - }, + forbidden={"OpenViking user API key", "OpenViking account", "OpenViking user"}, ), ) config = {"memory": {}} OpenVikingMemoryProvider().post_setup(str(hermes_home), config) - assert config["memory"]["provider"] == "openviking" - assert config["memory"]["openviking"]["use_ovcli_config"] is False - assert ovcli_path.read_text(encoding="utf-8") == original_ovcli env_text = (hermes_home / ".env").read_text(encoding="utf-8") - assert "OPENVIKING_ENDPOINT=http://localhost:1933" in env_text + assert "OPENVIKING_API_KEY=user-secret" in env_text assert "OPENVIKING_AGENT=agent" in env_text - assert "OPENVIKING_API_KEY" not in env_text assert "OPENVIKING_ACCOUNT" not in env_text assert "OPENVIKING_USER" not in env_text -def test_post_setup_cancel_existing_ovcli_writes_nothing(tmp_path, monkeypatch): +def test_manual_root_key_flow_prints_validation_progress(monkeypatch, capsys): _clear_openviking_env(monkeypatch) - hermes_home = tmp_path / "hermes" - hermes_home.mkdir() - env_path = hermes_home / ".env" - original_env = "OPENVIKING_ENDPOINT=http://old.local\nOTHER_KEY=keep\n" - env_path.write_text(original_env, encoding="utf-8") - ovcli_path = tmp_path / "ovcli.conf" - ovcli_path.write_text(json.dumps({"url": "http://openviking.local"}), encoding="utf-8") - monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) - from hermes_cli import config as hermes_config - from hermes_cli import memory_setup + monkeypatch.setattr(openviking_module, "_validate_openviking_reachability", lambda endpoint: (True, "")) - save_config = MagicMock() - monkeypatch.setattr(hermes_config, "save_config", save_config) - monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: -1) - config = {"memory": {"provider": "builtin"}} + validate_calls = [] - OpenVikingMemoryProvider().post_setup(str(hermes_home), config) + def validate_values(values, *, require_api_key=False): + validate_calls.append(dict(values)) + return True, "", "root" - save_config.assert_not_called() - assert config == {"memory": {"provider": "builtin"}} - assert env_path.read_text(encoding="utf-8") == original_env + monkeypatch.setattr(openviking_module, "_validate_openviking_setup_values", validate_values) + choices = iter([1]) - -def test_post_setup_invalid_existing_ovcli_writes_nothing(tmp_path, monkeypatch): - _clear_openviking_env(monkeypatch) - hermes_home = tmp_path / "hermes" - hermes_home.mkdir() - env_path = hermes_home / ".env" - original_env = "OPENVIKING_ENDPOINT=http://old.local\nOTHER_KEY=keep\n" - env_path.write_text(original_env, encoding="utf-8") - ovcli_path = tmp_path / "ovcli.conf" - ovcli_path.write_text("{", encoding="utf-8") - monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) - - from hermes_cli import config as hermes_config - from hermes_cli import memory_setup - - save_config = MagicMock() - monkeypatch.setattr(hermes_config, "save_config", save_config) - monkeypatch.setattr( - memory_setup, - "_curses_select", - MagicMock(side_effect=AssertionError("picker should not open for invalid ovcli.conf")), + values = openviking_module._prompt_manual_connection_values( + _prompt_from_values({ + "OpenViking server URL": "https://openviking.example", + "OpenViking root API key": "root-secret", + "OpenViking account": "acct", + "OpenViking user": "alice", + "OpenViking agent": "agent", + }), + lambda *args, **kwargs: next(choices), + -1, ) - config = {"memory": {"provider": "builtin"}} - OpenVikingMemoryProvider().post_setup(str(hermes_home), config) - - save_config.assert_not_called() - assert config == {"memory": {"provider": "builtin"}} - assert env_path.read_text(encoding="utf-8") == original_env + assert values["root_api_key"] == "root-secret" + assert len(validate_calls) == 2 + output = capsys.readouterr().out + assert "Checking OpenViking server..." in output + assert "Validating OpenViking root API key..." in output + assert "Validating OpenViking API access..." in output -def test_post_setup_creates_minimal_ovcli_and_links(tmp_path, monkeypatch): +def test_start_local_openviking_server_uses_endpoint_host_and_port(monkeypatch): + popen_calls = [] + + def fake_popen(args, **kwargs): + popen_calls.append((args, kwargs)) + return object() + + monkeypatch.setattr(openviking_module.shutil, "which", lambda name: "/usr/local/bin/openviking-server") + monkeypatch.setattr(openviking_module.subprocess, "Popen", fake_popen) + + started, message = openviking_module._start_local_openviking_server("http://127.0.0.1:1934") + + assert started is True + assert "127.0.0.1:1934" in message + args, kwargs = popen_calls[0] + assert args == ["/usr/local/bin/openviking-server", "--host", "127.0.0.1", "--port", "1934"] + assert kwargs["start_new_session"] is True + + +def test_handle_unreachable_endpoint_does_not_wait_when_autostart_command_missing(monkeypatch, capsys): + monkeypatch.setattr( + openviking_module, + "_start_local_openviking_server", + lambda endpoint: (False, "openviking-server was not found on PATH."), + ) + monkeypatch.setattr( + openviking_module, + "_wait_for_openviking_health", + MagicMock(side_effect=AssertionError("should not wait when server did not start")), + ) + + result = openviking_module._handle_unreachable_endpoint( + "http://127.0.0.1:1934", + "OpenViking server is not reachable.", + lambda *args, **kwargs: 0, + -1, + ) + + assert result is False + output = capsys.readouterr().out + assert "openviking-server was not found on PATH." in output + assert "did not become reachable" not in output + + +def test_handle_unreachable_endpoint_waits_long_enough_after_autostart(monkeypatch, capsys): + wait_calls = [] + + monkeypatch.setattr( + openviking_module, + "_start_local_openviking_server", + lambda endpoint: (True, "Started openviking-server on 127.0.0.1:1934 in the background."), + ) + monkeypatch.setattr( + openviking_module, + "_wait_for_openviking_health", + lambda endpoint, *, timeout_seconds=0: wait_calls.append((endpoint, timeout_seconds)) or True, + ) + + result = openviking_module._handle_unreachable_endpoint( + "http://127.0.0.1:1934", + "OpenViking server is not reachable.", + lambda *args, **kwargs: 0, + -1, + ) + + assert result is True + assert wait_calls == [("http://127.0.0.1:1934", 60.0)] + output = capsys.readouterr().out + assert "Waiting for OpenViking server to become reachable..." in output + + +def test_initialize_autostarts_local_openviking_in_background_when_runtime_health_fails(monkeypatch): + _clear_openviking_env(monkeypatch) + monkeypatch.setenv("OPENVIKING_ENDPOINT", "http://127.0.0.1:1934") + health_calls = [] + start_calls = [] + waiter_calls = [] + + class FakeVikingClient: + def __init__(self, endpoint, api_key="", account="", user="", agent=""): + assert endpoint == "http://127.0.0.1:1934" + + def health(self): + health_calls.append("health") + return False + + monkeypatch.setattr(openviking_module, "_VikingClient", FakeVikingClient) + monkeypatch.setattr( + openviking_module, + "_start_local_openviking_server", + lambda endpoint: start_calls.append(endpoint) or (True, "started"), + ) + monkeypatch.setattr( + openviking_module, + "_wait_for_openviking_health", + MagicMock(side_effect=AssertionError("runtime init should not wait synchronously")), + ) + + provider = OpenVikingMemoryProvider() + monkeypatch.setattr( + provider, + "_start_runtime_openviking_waiter", + lambda **kwargs: waiter_calls.append(kwargs), + raising=False, + ) + statuses = [] + provider.initialize("session-1", platform="cli", status_callback=statuses.append) + + assert provider._client is None + assert health_calls == ["health"] + assert start_calls == ["http://127.0.0.1:1934"] + assert len(waiter_calls) == 1 + assert waiter_calls[0]["status_callback"] == statuses.append + assert any("starting in the background" in message for message in statuses) + + +def test_runtime_openviking_waiter_attaches_client_after_health_recovers(monkeypatch): + _clear_openviking_env(monkeypatch) + wait_calls = [] + + class FakeVikingClient: + def __init__(self, endpoint, api_key="", account="", user="", agent=""): + self.endpoint = endpoint + self.api_key = api_key + self.account = account + self.user = user + self.agent = agent + + def health(self): + return True + + monkeypatch.setattr(openviking_module, "_VikingClient", FakeVikingClient) + monkeypatch.setattr( + openviking_module, + "_wait_for_openviking_health", + lambda endpoint, **kwargs: wait_calls.append((endpoint, kwargs)) or True, + ) + + provider = OpenVikingMemoryProvider() + provider._endpoint = "http://127.0.0.1:1934" + provider._api_key = "secret" + provider._account = "acct" + provider._user = "alice" + provider._agent = "hermes" + statuses = [] + + provider._finish_runtime_openviking_start( + status_callback=statuses.append, + warning_callback=None, + ) + + assert provider._client is not None + assert provider._client.endpoint == "http://127.0.0.1:1934" + assert provider._client.api_key == "secret" + assert wait_calls == [( + "http://127.0.0.1:1934", + {"timeout_seconds": openviking_module._LOCAL_OPENVIKING_AUTOSTART_TIMEOUT}, + )] + assert any("OpenViking memory is active" in message for message in statuses) + + +def test_runtime_openviking_waiter_warns_when_background_start_times_out(monkeypatch): + _clear_openviking_env(monkeypatch) + monkeypatch.setattr( + openviking_module, + "_wait_for_openviking_health", + lambda endpoint, **kwargs: False, + ) + monkeypatch.setattr( + openviking_module, + "_VikingClient", + MagicMock(side_effect=AssertionError("client should not be rebuilt before health recovers")), + ) + + provider = OpenVikingMemoryProvider() + provider._endpoint = "http://127.0.0.1:1934" + warnings = [] + + provider._finish_runtime_openviking_start( + status_callback=None, + warning_callback=warnings.append, + ) + + assert provider._client is None + assert warnings == [ + "Local OpenViking server at http://127.0.0.1:1934 is not reachable. " + "Tried to start openviking-server, but it did not become reachable " + "within 60 seconds. OpenViking memory disabled for this Hermes run." + ] + + +def test_initialize_does_not_autostart_remote_openviking(monkeypatch, caplog): + _clear_openviking_env(monkeypatch) + monkeypatch.setenv("OPENVIKING_ENDPOINT", "https://openviking.example") + + class FakeVikingClient: + def __init__(self, endpoint, api_key="", account="", user="", agent=""): + assert endpoint == "https://openviking.example" + + def health(self): + return False + + monkeypatch.setattr(openviking_module, "_VikingClient", FakeVikingClient) + monkeypatch.setattr( + openviking_module, + "_start_local_openviking_server", + MagicMock(side_effect=AssertionError("remote endpoint should not auto-start")), + ) + monkeypatch.setattr( + openviking_module, + "_wait_for_openviking_health", + MagicMock(side_effect=AssertionError("remote endpoint should not wait")), + ) + + with caplog.at_level("WARNING", logger=openviking_module.__name__): + provider = OpenVikingMemoryProvider() + provider.initialize("session-1") + + assert provider._client is None + assert "Remote OpenViking server at https://openviking.example is not reachable" in caplog.text + + +def test_initialize_warns_clearly_when_local_runtime_autostart_fails(monkeypatch, caplog): + _clear_openviking_env(monkeypatch) + monkeypatch.setenv("OPENVIKING_ENDPOINT", "http://localhost:1934") + + class FakeVikingClient: + def __init__(self, endpoint, api_key="", account="", user="", agent=""): + assert endpoint == "http://localhost:1934" + + def health(self): + return False + + monkeypatch.setattr(openviking_module, "_VikingClient", FakeVikingClient) + monkeypatch.setattr( + openviking_module, + "_start_local_openviking_server", + lambda endpoint: (False, "openviking-server was not found on PATH."), + ) + monkeypatch.setattr( + openviking_module, + "_wait_for_openviking_health", + MagicMock(side_effect=AssertionError("should not wait when server did not start")), + ) + + with caplog.at_level("WARNING", logger=openviking_module.__name__): + provider = OpenVikingMemoryProvider() + provider.initialize("session-1") + + assert provider._client is None + assert "Local OpenViking server at http://localhost:1934 is not reachable" in caplog.text + assert "openviking-server was not found on PATH" in caplog.text + + +def test_initialize_emits_cli_warning_when_local_runtime_autostart_fails(monkeypatch): + _clear_openviking_env(monkeypatch) + monkeypatch.setenv("OPENVIKING_ENDPOINT", "http://localhost:1934") + + class FakeVikingClient: + def __init__(self, endpoint, api_key="", account="", user="", agent=""): + assert endpoint == "http://localhost:1934" + + def health(self): + return False + + warnings = [] + monkeypatch.setattr(openviking_module, "_VikingClient", FakeVikingClient) + monkeypatch.setattr( + openviking_module, + "_start_local_openviking_server", + lambda endpoint: (False, "openviking-server was not found on PATH."), + ) + + provider = OpenVikingMemoryProvider() + provider.initialize("session-1", platform="cli", warning_callback=warnings.append) + + assert provider._client is None + assert warnings == [ + "Local OpenViking server at http://localhost:1934 is not reachable. " + "openviking-server was not found on PATH. " + "OpenViking memory disabled for this Hermes run." + ] + + +def test_initialize_does_not_emit_cli_warning_when_callback_absent(monkeypatch): + _clear_openviking_env(monkeypatch) + monkeypatch.setenv("OPENVIKING_ENDPOINT", "http://localhost:1934") + + class FakeVikingClient: + def __init__(self, endpoint, api_key="", account="", user="", agent=""): + assert endpoint == "http://localhost:1934" + + def health(self): + return False + + monkeypatch.setattr(openviking_module, "_VikingClient", FakeVikingClient) + monkeypatch.setattr( + openviking_module, + "_start_local_openviking_server", + lambda endpoint: (False, "openviking-server was not found on PATH."), + ) + + provider = OpenVikingMemoryProvider() + provider.initialize("session-1", platform="gateway") + + assert provider._client is None + + +def test_post_setup_local_server_down_can_offer_autostart(tmp_path, monkeypatch): _clear_openviking_env(monkeypatch) hermes_home = tmp_path / "hermes" hermes_home.mkdir() - ovcli_path = tmp_path / "missing" / "ovcli.conf" monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + monkeypatch.setattr(openviking_module, "_validate_openviking_setup_values", lambda values, *, require_api_key=False: (True, "", None)) from hermes_cli import memory_setup - monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: 0) + reachability_calls = [] + + def validate_reachability(endpoint): + reachability_calls.append(endpoint) + return False, "OpenViking server is not reachable." if len(reachability_calls) == 1 else "" + + started = [] + monkeypatch.setattr(openviking_module, "_validate_openviking_reachability", validate_reachability) + monkeypatch.setattr(openviking_module, "_start_local_openviking_server", lambda endpoint: (started.append(endpoint) or True, "started")) + monkeypatch.setattr(openviking_module, "_wait_for_openviking_health", lambda endpoint, **kwargs: True) + choices = iter([1, 0, 0, 0]) + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: next(choices)) monkeypatch.setattr( memory_setup, "_prompt", - lambda label, default=None, secret=False: default or "", + _prompt_from_values({ + "OpenViking server URL": "localhost", + "OpenViking agent": "agent", + }), ) config = {"memory": {}} OpenVikingMemoryProvider().post_setup(str(hermes_home), config) - assert config["memory"]["provider"] == "openviking" - assert config["memory"]["openviking"]["use_ovcli_config"] is True - data = json.loads(ovcli_path.read_text(encoding="utf-8")) - assert data == { - "url": "http://127.0.0.1:1933", - "agent_id": "hermes", - } - env_path = hermes_home / ".env" - if env_path.exists(): - assert env_path.read_text(encoding="utf-8") == "" + assert started == ["http://localhost:1933"] + assert reachability_calls == ["http://localhost:1933"] + env_text = (hermes_home / ".env").read_text(encoding="utf-8") + assert "OPENVIKING_ENDPOINT=http://localhost:1933" in env_text + assert "OPENVIKING_API_KEY" not in env_text -def test_post_setup_cancel_missing_ovcli_does_not_prompt_or_create(tmp_path, monkeypatch): +def test_post_setup_invalid_env_profile_can_create_new_config(tmp_path, monkeypatch): _clear_openviking_env(monkeypatch) hermes_home = tmp_path / "hermes" hermes_home.mkdir() - ovcli_path = tmp_path / "missing" / "ovcli.conf" + ovcli_path = tmp_path / "broken" / "ovcli.conf" + ovcli_path.parent.mkdir() + ovcli_path.write_text("{", encoding="utf-8") monkeypatch.setenv("HERMES_HOME", str(hermes_home)) monkeypatch.setenv("OPENVIKING_CLI_CONFIG_FILE", str(ovcli_path)) + _allow_setup_validation(monkeypatch) - from hermes_cli import config as hermes_config from hermes_cli import memory_setup - save_config = MagicMock() - monkeypatch.setattr(hermes_config, "save_config", save_config) - monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: -1) + choices = iter([1, 0, 0]) + monkeypatch.setattr(memory_setup, "_curses_select", lambda *args, **kwargs: next(choices)) monkeypatch.setattr( memory_setup, "_prompt", - MagicMock(side_effect=AssertionError("prompts should not run after cancel")), + _prompt_from_values({ + "OpenViking server URL": "https://openviking.example", + "OpenViking user API key": "user-secret", + "OpenViking agent": "agent", + }), ) - config = {"memory": {"provider": "builtin"}} + config = {"memory": {}} OpenVikingMemoryProvider().post_setup(str(hermes_home), config) - save_config.assert_not_called() - assert config == {"memory": {"provider": "builtin"}} - assert not ovcli_path.exists() - assert not (hermes_home / ".env").exists() + assert ovcli_path.read_text(encoding="utf-8") == "{" + assert config["memory"]["openviking"] == {"use_ovcli_config": False} def test_tool_search_sorts_by_raw_score_across_buckets(): @@ -1181,6 +1308,7 @@ def test_viking_client_upload_temp_file_uses_multipart_identity_headers(tmp_path headers = captured_kwargs["headers"] assert headers["X-OpenViking-Account"] == "test-account" assert headers["X-OpenViking-User"] == "test-user" + assert headers["X-OpenViking-Actor-Peer"] == "test-agent" assert headers["X-OpenViking-Agent"] == "test-agent" assert headers["X-API-Key"] == "test-key" assert "Content-Type" not in headers @@ -1205,6 +1333,28 @@ def test_viking_client_raises_structured_server_error(): client._parse_response(response) +def test_viking_client_sanitizes_html_error_body(): + client = _VikingClient.__new__(_VikingClient) + response = SimpleNamespace( + status_code=523, + text=""" + +tosaki.top | 523: Origin is unreachable +large Cloudflare error page +""", + json=lambda: (_ for _ in ()).throw(ValueError("not json")), + ) + + with pytest.raises(openviking_module._OpenVikingHTTPError) as exc_info: + client._parse_response(response) + + message = str(exc_info.value) + assert "HTTP 523" in message + assert "Origin is unreachable" in message + assert " Date: Wed, 17 Jun 2026 01:23:05 +0800 Subject: [PATCH 10/11] fix(memory): tighten OpenViking local autostart --- plugins/memory/openviking/__init__.py | 64 +++++++++++--- .../memory/test_openviking_provider.py | 87 +++++++++++++++++++ 2 files changed, 141 insertions(+), 10 deletions(-) diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index 07dd3317957..452c543d04d 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -88,6 +88,7 @@ _MEMORY_WRITE_TARGET_SUBDIR_MAP = { } _LOCAL_OPENVIKING_HOSTS = {"localhost", "127.0.0.1", "::1"} _LOCAL_OPENVIKING_AUTOSTART_TIMEOUT = 60.0 +_OPENVIKING_SERVER_LOG_RELATIVE_PATH = Path("logs") / "openviking-server.log" _SETUP_CANCELLED = object() @@ -664,7 +665,8 @@ def _is_local_openviking_url(value: str) -> bool: if "://" not in candidate: candidate = f"//{candidate}" parsed = urlparse(candidate) - return (parsed.hostname or "").lower() in _LOCAL_OPENVIKING_HOSTS + scheme = (parsed.scheme or "http").lower() + return scheme == "http" and (parsed.hostname or "").lower() in _LOCAL_OPENVIKING_HOSTS def _load_hermes_openviking_config() -> dict: @@ -939,6 +941,15 @@ def _local_openviking_bind(endpoint: str) -> tuple[str, int]: return host, port +def _openviking_server_log_path() -> Path: + try: + from hermes_constants import get_hermes_home + home = get_hermes_home() + except Exception: + home = Path(os.environ.get("HERMES_HOME", "")).expanduser() if os.environ.get("HERMES_HOME") else Path.home() / ".hermes" + return home / _OPENVIKING_SERVER_LOG_RELATIVE_PATH + + def _start_local_openviking_server(endpoint: str) -> tuple[bool, str]: server_cmd = shutil.which("openviking-server") if not server_cmd: @@ -947,17 +958,20 @@ def _start_local_openviking_server(endpoint: str) -> tuple[bool, str]: host, port = _local_openviking_bind(endpoint) except ValueError as e: return False, f"Could not parse local OpenViking URL: {e}" + log_path = _openviking_server_log_path() try: - subprocess.Popen( - [server_cmd, "--host", host, "--port", str(port)], - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, - stdin=subprocess.DEVNULL, - start_new_session=True, - ) + log_path.parent.mkdir(parents=True, exist_ok=True) + with log_path.open("ab") as log_file: + subprocess.Popen( + [server_cmd, "--host", host, "--port", str(port)], + stdout=log_file, + stderr=log_file, + stdin=subprocess.DEVNULL, + start_new_session=True, + ) except Exception as e: return False, f"Could not start openviking-server: {e}" - return True, f"Started openviking-server on {host}:{port} in the background." + return True, f"Started openviking-server on {host}:{port} in the background. Logs: {log_path}" def _wait_for_openviking_health(endpoint: str, *, timeout_seconds: float = 15.0) -> bool: @@ -1036,6 +1050,29 @@ def _runtime_openviking_timeout_message(endpoint: str) -> str: ) +def _classify_runtime_openviking_health(client: _VikingClient, endpoint: str) -> tuple[str, str]: + """Classify runtime health without treating every false result as server absence.""" + try: + if hasattr(client, "health_payload"): + payload = client.health_payload() + if payload.get("healthy") is False: + return ( + "responded", + f"OpenViking server at {endpoint} responded but reported unhealthy status.", + ) + return "healthy", "" + if client.health(): + return "healthy", "" + except _OpenVikingHTTPError as e: + return ( + "responded", + f"OpenViking server at {endpoint} responded with {_format_openviking_exception(e)}.", + ) + except Exception: + return "unreachable", "" + return "unreachable", "" + + def _prompt_profile_name(prompt, select, cancelled) -> str | object: while True: name = _clean_config_value(prompt("OpenViking profile name")) @@ -1811,11 +1848,18 @@ class OpenVikingMemoryProvider(MemoryProvider): self._endpoint, self._api_key, account=self._account, user=self._user, agent=self._agent, ) - if not self._client.health(): + health_state, health_message = _classify_runtime_openviking_health(self._client, self._endpoint) + if health_state == "unreachable": self._handle_runtime_openviking_unreachable( status_callback=status_callback, warning_callback=warning_callback, ) + elif health_state != "healthy": + _emit_runtime_warning( + f"{health_message} OpenViking memory disabled for this Hermes run.", + warning_callback, + ) + self._client = None except ImportError: logger.warning("httpx not installed — OpenViking plugin disabled") self._client = None diff --git a/tests/plugins/memory/test_openviking_provider.py b/tests/plugins/memory/test_openviking_provider.py index 36e0658f338..d9aba21ca95 100644 --- a/tests/plugins/memory/test_openviking_provider.py +++ b/tests/plugins/memory/test_openviking_provider.py @@ -640,6 +640,93 @@ def test_start_local_openviking_server_uses_endpoint_host_and_port(monkeypatch): assert kwargs["start_new_session"] is True +def test_start_local_openviking_server_writes_output_to_log(tmp_path, monkeypatch): + hermes_home = tmp_path / "hermes" + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + popen_calls = [] + + class FakeProcess: + pass + + def fake_popen(args, **kwargs): + popen_calls.append((args, kwargs)) + assert kwargs["stdout"] is kwargs["stderr"] + assert kwargs["stdout"].name == str(hermes_home / "logs" / "openviking-server.log") + assert not kwargs["stdout"].closed + return FakeProcess() + + monkeypatch.setattr(openviking_module.shutil, "which", lambda name: "/usr/local/bin/openviking-server") + monkeypatch.setattr(openviking_module.subprocess, "Popen", fake_popen) + + started, message = openviking_module._start_local_openviking_server("http://127.0.0.1:1934") + + assert started is True + assert str(hermes_home / "logs" / "openviking-server.log") in message + assert popen_calls + + +def test_https_local_endpoint_is_not_runtime_autostart_eligible(monkeypatch): + _clear_openviking_env(monkeypatch) + monkeypatch.setenv("OPENVIKING_ENDPOINT", "https://localhost:1934") + + class FakeVikingClient: + def __init__(self, endpoint, api_key="", account="", user="", agent=""): + assert endpoint == "https://localhost:1934" + + def health(self): + return False + + monkeypatch.setattr(openviking_module, "_VikingClient", FakeVikingClient) + monkeypatch.setattr( + openviking_module, + "_start_local_openviking_server", + MagicMock(side_effect=AssertionError("https localhost endpoint should not auto-start")), + ) + + warnings = [] + provider = OpenVikingMemoryProvider() + provider.initialize("session-1", platform="cli", warning_callback=warnings.append) + + assert provider._client is None + assert warnings == [ + "Remote OpenViking server at https://localhost:1934 is not reachable; " + "OpenViking memory disabled for this Hermes run. " + "Check the configured endpoint and network connectivity." + ] + + +def test_runtime_does_not_autostart_when_local_server_reports_unhealthy(monkeypatch): + _clear_openviking_env(monkeypatch) + monkeypatch.setenv("OPENVIKING_ENDPOINT", "http://localhost:1934") + + class FakeVikingClient: + def __init__(self, endpoint, api_key="", account="", user="", agent=""): + assert endpoint == "http://localhost:1934" + + def health(self): + return False + + def health_payload(self): + return {"healthy": False} + + monkeypatch.setattr(openviking_module, "_VikingClient", FakeVikingClient) + monkeypatch.setattr( + openviking_module, + "_start_local_openviking_server", + MagicMock(side_effect=AssertionError("responding unhealthy server should not auto-start another process")), + ) + + warnings = [] + provider = OpenVikingMemoryProvider() + provider.initialize("session-1", platform="cli", warning_callback=warnings.append) + + assert provider._client is None + assert warnings == [ + "OpenViking server at http://localhost:1934 responded but reported unhealthy status. " + "OpenViking memory disabled for this Hermes run." + ] + + def test_handle_unreachable_endpoint_does_not_wait_when_autostart_command_missing(monkeypatch, capsys): monkeypatch.setattr( openviking_module, From 166d2457b292e10d347331016b440ebbfa2fb66e Mon Sep 17 00:00:00 2001 From: Hao Zhe Date: Wed, 17 Jun 2026 01:32:43 +0800 Subject: [PATCH 11/11] fix(memory): avoid setup autostart for unhealthy OpenViking --- plugins/memory/openviking/__init__.py | 28 +++++++++++-- .../memory/test_openviking_provider.py | 40 +++++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index 452c543d04d..7b626cf7a53 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -89,6 +89,7 @@ _MEMORY_WRITE_TARGET_SUBDIR_MAP = { _LOCAL_OPENVIKING_HOSTS = {"localhost", "127.0.0.1", "::1"} _LOCAL_OPENVIKING_AUTOSTART_TIMEOUT = 60.0 _OPENVIKING_SERVER_LOG_RELATIVE_PATH = Path("logs") / "openviking-server.log" +_OPENVIKING_RESPONDED_FAILURE_PREFIX = "OpenViking server responded" _SETUP_CANCELLED = object() @@ -806,6 +807,8 @@ def _validate_openviking_reachability(endpoint: str) -> tuple[bool, str]: elif client.health(): return True, "" except Exception as e: + if _status_code_from_error(e) is not None: + return False, f"OpenViking server responded with {_format_openviking_exception(e)}." return False, f"OpenViking server is not reachable at {endpoint}: {_format_openviking_exception(e)}" return False, f"OpenViking server is not reachable at {endpoint}." @@ -984,8 +987,19 @@ def _wait_for_openviking_health(endpoint: str, *, timeout_seconds: float = 15.0) return False -def _handle_unreachable_endpoint(endpoint: str, message: str, select, cancelled): - if _is_local_openviking_url(endpoint): +def _reachability_failure_allows_local_autostart(message: str) -> bool: + return not (message or "").startswith(_OPENVIKING_RESPONDED_FAILURE_PREFIX) + + +def _handle_unreachable_endpoint( + endpoint: str, + message: str, + select, + cancelled, + *, + allow_local_autostart: bool = True, +): + if _is_local_openviking_url(endpoint) and allow_local_autostart: print(f" {message}") choice = select( " Local OpenViking server is down", @@ -1017,7 +1031,7 @@ def _handle_unreachable_endpoint(endpoint: str, message: str, select, cancelled) return _retry_or_cancel_manual_setup( select, - " OpenViking server unreachable", + " OpenViking server unhealthy" if _is_local_openviking_url(endpoint) else " OpenViking server unreachable", message, cancelled, ) @@ -1126,7 +1140,13 @@ def _prompt_manual_connection_values(prompt, select, cancelled, *, service: bool if reachable: print(" OpenViking server is reachable.") break - retry = _handle_unreachable_endpoint(endpoint, message, select, cancelled) + retry = _handle_unreachable_endpoint( + endpoint, + message, + select, + cancelled, + allow_local_autostart=_reachability_failure_allows_local_autostart(message), + ) if retry is True: break if retry is _SETUP_CANCELLED: diff --git a/tests/plugins/memory/test_openviking_provider.py b/tests/plugins/memory/test_openviking_provider.py index d9aba21ca95..518b2e37ded 100644 --- a/tests/plugins/memory/test_openviking_provider.py +++ b/tests/plugins/memory/test_openviking_provider.py @@ -779,6 +779,46 @@ def test_handle_unreachable_endpoint_waits_long_enough_after_autostart(monkeypat assert "Waiting for OpenViking server to become reachable..." in output +def test_manual_setup_does_not_offer_autostart_when_local_server_is_unhealthy(monkeypatch): + _clear_openviking_env(monkeypatch) + + class FakeVikingClient: + def __init__(self, endpoint, api_key="", account="", user="", agent=""): + assert endpoint == "http://localhost:1933" + + def health_payload(self): + return {"healthy": False} + + select_calls = [] + + def select(title, options, **kwargs): + select_calls.append((title, options)) + assert all(label != "Start local OpenViking" for label, _description in options) + return 1 + + monkeypatch.setattr(openviking_module, "_VikingClient", FakeVikingClient) + monkeypatch.setattr( + openviking_module, + "_start_local_openviking_server", + MagicMock(side_effect=AssertionError("unhealthy local server should not offer auto-start")), + ) + + result = openviking_module._prompt_manual_connection_values( + _prompt_from_values({"OpenViking server URL": "localhost"}), + select, + -1, + ) + + assert result is openviking_module._SETUP_CANCELLED + assert select_calls == [( + " OpenViking server unhealthy", + [ + ("Retry", "try this step again"), + ("Cancel setup", "no changes saved"), + ], + )] + + def test_initialize_autostarts_local_openviking_in_background_when_runtime_health_fails(monkeypatch): _clear_openviking_env(monkeypatch) monkeypatch.setenv("OPENVIKING_ENDPOINT", "http://127.0.0.1:1934")