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 dca57915b9)
This commit is contained in:
parent
f94914f773
commit
5396dd8f02
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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-<profile> > "default"
|
||||
# Project resolution: RETAINDB_PROJECT > config.yaml project > hermes-<profile> > "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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
Loading…
Reference in New Issue