From 5396dd8f02ac109d371e57966b858e88eb2e0a8a Mon Sep 17 00:00:00 2001 From: PRATHAMESH75 Date: Tue, 21 Jul 2026 02:14:35 +0530 Subject: [PATCH] fix(memory): read non-secret provider config from config.yaml for OpenViking and RetainDB OpenViking is_available() only consulted env vars and use_ovcli_config, so an endpoint saved to config.yaml (e.g. by the Dashboard) reported needs_config; _resolve_connection_settings() likewise never folded config.yaml's non-secret fields into its chain. RetainDB initialize() read base_url/project from the environment only, ignoring the values the Dashboard writes to config.yaml. Both now resolve non-secret fields as env -> (ovcli ->) config.yaml -> default; secrets still come from the environment. Adds regression tests for both. Fixes #68209 (cherry picked from commit dca57915b97b5705b30927a062e1d0f2f23d3841) --- plugins/memory/openviking/__init__.py | 30 +++++++-- plugins/memory/retaindb/__init__.py | 37 ++++++++++- .../memory/test_openviking_provider.py | 50 ++++++++++++++ .../plugins/memory/test_retaindb_provider.py | 66 +++++++++++++++++++ 4 files changed, 176 insertions(+), 7 deletions(-) diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index 28b6d71f7dc73..8e3abf656e90c 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -960,13 +960,31 @@ def _resolve_connection_settings(provider_config: Optional[dict] = None) -> dict user_env = _env_value("OPENVIKING_USER") agent_env = _env_value("OPENVIKING_AGENT") - endpoint = _first_nonempty(endpoint_env, ovcli_values.get("endpoint"), default=_DEFAULT_ENDPOINT) + # Non-secret fields fall back to config.yaml (e.g. the Dashboard writes + # ``memory.openviking.endpoint`` there) before the built-in default, so the + # full chain is env -> ovcli -> config.yaml -> default. The secret api_key is + # sourced from the environment (synced from .env), never from config.yaml. + endpoint = _first_nonempty( + endpoint_env, + ovcli_values.get("endpoint"), + _clean_config_value(provider_config.get("endpoint")), + default=_DEFAULT_ENDPOINT, + ) return { "endpoint": _normalize_openviking_url(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), + "account": account_env if account_env is not None else _first_nonempty( + ovcli_values.get("account"), _clean_config_value(provider_config.get("account")) + ), + "user": user_env if user_env is not None else _first_nonempty( + ovcli_values.get("user"), _clean_config_value(provider_config.get("user")) + ), + "agent": _first_nonempty( + agent_env, + ovcli_values.get("agent"), + _clean_config_value(provider_config.get("agent")), + default=_DEFAULT_AGENT, + ), } @@ -1983,6 +2001,10 @@ class OpenVikingMemoryProvider(MemoryProvider): if os.environ.get("OPENVIKING_ENDPOINT"): return True provider_config = _load_hermes_openviking_config() + # A non-secret endpoint saved to config.yaml (e.g. via the Dashboard) + # counts as configured even without an env var or ovcli config. + if _clean_config_value(provider_config.get("endpoint")): + return True if not provider_config.get("use_ovcli_config"): return False try: diff --git a/plugins/memory/retaindb/__init__.py b/plugins/memory/retaindb/__init__.py index 13ad0331501b3..c83a283a4099d 100644 --- a/plugins/memory/retaindb/__init__.py +++ b/plugins/memory/retaindb/__init__.py @@ -44,6 +44,29 @@ _DEFAULT_BASE_URL = "https://api.retaindb.com" _ASYNC_SHUTDOWN = object() +def _load_retaindb_config() -> Dict[str, Any]: + """Return the ``memory.retaindb`` block from config.yaml (empty on any error). + + Non-secret fields (``base_url``, ``project``) are persisted here by the + Dashboard; the runtime must read them back when the matching env var is + unset. The secret ``api_key`` continues to come from the environment. + """ + 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("retaindb", {}) if isinstance(memory_config, dict) else {} + return dict(provider_config) if isinstance(provider_config, dict) else {} + except Exception: + return {} + + +def _config_str(value: Any) -> str: + """Return a stripped string for a config value, else ``""``.""" + return value.strip() if isinstance(value, str) else "" + + # --------------------------------------------------------------------------- # Tool schemas # --------------------------------------------------------------------------- @@ -489,12 +512,20 @@ class RetainDBMemoryProvider(MemoryProvider): # ── Lifecycle ────────────────────────────────────────────────────────── def initialize(self, session_id: str, **kwargs) -> None: + # Non-secret fields fall back to config.yaml (written by the Dashboard) + # when the env var is unset: env -> config.yaml -> default. + provider_config = _load_retaindb_config() api_key = get_secret("RETAINDB_API_KEY", "") or "" - base_url = re.sub(r"/+$", "", os.environ.get("RETAINDB_BASE_URL", _DEFAULT_BASE_URL)) + base_url_raw = ( + os.environ.get("RETAINDB_BASE_URL") + or _config_str(provider_config.get("base_url")) + or _DEFAULT_BASE_URL + ) + base_url = re.sub(r"/+$", "", base_url_raw) - # Project resolution: RETAINDB_PROJECT > hermes- > "default" + # Project resolution: RETAINDB_PROJECT > config.yaml project > hermes- > "default" # If unset, the API auto-creates and uses the "default" project — no config required. - explicit = os.environ.get("RETAINDB_PROJECT") + explicit = os.environ.get("RETAINDB_PROJECT") or _config_str(provider_config.get("project")) if explicit: project = explicit else: diff --git a/tests/plugins/memory/test_openviking_provider.py b/tests/plugins/memory/test_openviking_provider.py index 3552d081874a9..b73a93b2a89ca 100644 --- a/tests/plugins/memory/test_openviking_provider.py +++ b/tests/plugins/memory/test_openviking_provider.py @@ -1149,3 +1149,53 @@ def test_in_place_compression_lifecycle_allows_a_later_commit(monkeypatch): "post-compression turns were never committed: " f"{provider._client.post.call_args_list}" ) + +def test_resolve_connection_settings_reads_config_yaml_non_secret_fields(monkeypatch): + """#68209: non-secret fields saved to config.yaml feed the resolution chain.""" + _clear_openviking_env(monkeypatch) + provider_config = { + "endpoint": "http://saved.local:1933", + "account": "cfg-account", + "user": "cfg-user", + "agent": "cfg-agent", + } + + settings = openviking_module._resolve_connection_settings(provider_config) + + assert settings["endpoint"] == "http://saved.local:1933" + assert settings["account"] == "cfg-account" + assert settings["user"] == "cfg-user" + assert settings["agent"] == "cfg-agent" + + +def test_env_overrides_config_yaml_non_secret_fields(monkeypatch): + """env still wins over config.yaml (env -> ovcli -> config.yaml -> default).""" + _clear_openviking_env(monkeypatch) + monkeypatch.setenv("OPENVIKING_ENDPOINT", "http://env.local") + monkeypatch.setenv("OPENVIKING_AGENT", "env-agent") + + settings = openviking_module._resolve_connection_settings( + {"endpoint": "http://saved.local", "agent": "cfg-agent"} + ) + + assert settings["endpoint"] == "http://env.local" + assert settings["agent"] == "env-agent" + + +def test_is_available_true_for_config_yaml_endpoint(monkeypatch): + """#68209: a config.yaml endpoint (no env, no ovcli) counts as available.""" + _clear_openviking_env(monkeypatch) + monkeypatch.setattr( + openviking_module, + "_load_hermes_openviking_config", + lambda: {"endpoint": "http://saved.local:1933"}, + ) + assert OpenVikingMemoryProvider().is_available() is True + + +def test_is_available_false_without_any_endpoint(monkeypatch): + _clear_openviking_env(monkeypatch) + monkeypatch.setattr( + openviking_module, "_load_hermes_openviking_config", lambda: {} + ) + assert OpenVikingMemoryProvider().is_available() is False diff --git a/tests/plugins/memory/test_retaindb_provider.py b/tests/plugins/memory/test_retaindb_provider.py index bc468980448db..922e7b47a16ff 100644 --- a/tests/plugins/memory/test_retaindb_provider.py +++ b/tests/plugins/memory/test_retaindb_provider.py @@ -38,3 +38,69 @@ def test_upload_file_allows_regular_file(tmp_path): provider._client.upload_file.assert_called_once() assert provider._client.upload_file.call_args.args[0] == note.read_bytes() assert result["file"]["id"] == "file-1" + + +def _capture_initialized_client(monkeypatch, tmp_path): + """Patch _Client/_WriteQueue/get_hermes_home; return a dict capturing args.""" + import hermes_constants + + import plugins.memory.retaindb as retaindb_module + + captured: dict = {} + + class _FakeClient: + def __init__(self, api_key, base_url, project): + captured["api_key"] = api_key + captured["base_url"] = base_url + captured["project"] = project + self.project = project + + monkeypatch.setattr(retaindb_module, "_Client", _FakeClient) + monkeypatch.setattr(retaindb_module, "_WriteQueue", lambda *a, **k: MagicMock()) + monkeypatch.setattr(hermes_constants, "get_hermes_home", lambda: tmp_path) + return retaindb_module, captured + + +def test_initialize_reads_base_url_and_project_from_config_yaml(tmp_path, monkeypatch): + """#68209: non-secret base_url/project come from config.yaml when env is unset.""" + for var in ("RETAINDB_API_KEY", "RETAINDB_BASE_URL", "RETAINDB_PROJECT"): + monkeypatch.delenv(var, raising=False) + retaindb_module, captured = _capture_initialized_client(monkeypatch, tmp_path) + monkeypatch.setattr( + retaindb_module, + "_load_retaindb_config", + lambda: {"base_url": "https://retaindb.example.com/", "project": "cfg-project"}, + ) + + RetainDBMemoryProvider().initialize("sess-1") + + assert captured["base_url"] == "https://retaindb.example.com" # trailing slash stripped + assert captured["project"] == "cfg-project" + + +def test_initialize_env_overrides_config_yaml(tmp_path, monkeypatch): + for var in ("RETAINDB_API_KEY", "RETAINDB_PROJECT"): + monkeypatch.delenv(var, raising=False) + monkeypatch.setenv("RETAINDB_BASE_URL", "https://env.example.com") + retaindb_module, captured = _capture_initialized_client(monkeypatch, tmp_path) + monkeypatch.setattr( + retaindb_module, + "_load_retaindb_config", + lambda: {"base_url": "https://cfg.example.com", "project": "cfg-project"}, + ) + + RetainDBMemoryProvider().initialize("sess-1") + + assert captured["base_url"] == "https://env.example.com" + + +def test_initialize_falls_back_to_default_base_url(tmp_path, monkeypatch): + for var in ("RETAINDB_API_KEY", "RETAINDB_BASE_URL", "RETAINDB_PROJECT"): + monkeypatch.delenv(var, raising=False) + retaindb_module, captured = _capture_initialized_client(monkeypatch, tmp_path) + monkeypatch.setattr(retaindb_module, "_load_retaindb_config", lambda: {}) + + RetainDBMemoryProvider().initialize("sess-1") + + assert captured["base_url"] == retaindb_module._DEFAULT_BASE_URL + assert captured["project"] == "default"