fix(memory): read non-secret provider config from config.yaml for OpenViking and RetainDB (#68209)

[PRATHAMESH75]

What does this PR do?

Two in-tree memory providers ignore the non-secret configuration the Dashboard writes to config.yaml, so they can't be enabled through the UI. The root cause is on the reading side — secrets already reach the runtime via save_env_value()'s os.environ sync; the non-secret fields (endpoint, base_url, project) are saved to config.yaml but never read back.

• OpenViking — is_available() only checked OPENVIKING_ENDPOINT and use_ovcli_config, so an endpoint saved to memory.openviking.endpoint reported needs_config. _resolve_connection_settings() likewise never folded config.yaml's non-secret fields into its resolution chain, so even past an availability check the runtime would fall back to the default endpoint.
• RetainDB — initialize() read base_url/project from os.environ only, ignoring the values the Dashboard persists to memory.retaindb.*.

Both now resolve non-secret fields as env → (ovcli →) config.yaml → default; secrets continue to come from the environment only. Scoped deliberately to these two reading bugs — the issue's broader "shared config interface for all 8 providers" refactor is intentionally left out.

Related Issue

Fixes #68209

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• plugins/memory/openviking/__init__.py
  • is_available() now returns True when a non-secret endpoint is present in config.yaml (not just env/ovcli).
  • _resolve_connection_settings() folds config.yaml's non-secret fields (endpoint, account, user, agent) into the chain before the built-in default; the secret api_key is unchanged (env-sourced).
• plugins/memory/retaindb/__init__.py
  • Added _load_retaindb_config() (reads the memory.retaindb block, empty on error) and a small _config_str() helper.
  • initialize() falls back to config.yaml for base_url and project when the env var is unset.
• tests/plugins/memory/test_openviking_provider.py, tests/plugins/memory/test_retaindb_provider.py
  • Regression tests: config.yaml values are read back, env still overrides config.yaml, and the default fallback is preserved.

How to Test

scripts/run_tests.sh tests/plugins/memory/test_openviking_provider.py tests/plugins/memory/test_retaindb_provider.py

Result: 144 tests passed, 0 failed. The new tests fail against upstream/main (OpenViking is_available() returns False for a config.yaml-only endpoint; RetainDB's initialize() never consults config.yaml) and pass with this change.

Manual: set memory.openviking.endpoint (or memory.retaindb.base_url/project) in config.yaml with no matching env var — OpenViking now reports available, and RetainDB connects to the configured base URL/project instead of the defaults.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run the affected test files and all tests pass (144 passed)
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A (docstrings updated inline; behavior documented in code comments)
• I've updated cli-config.yaml.example if I added/changed config keys — N/A (no new keys; reads existing memory.<provider>.*)
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
• I've considered cross-platform impact (Windows, macOS) — N/A (pure config-reading logic, no platform-specific paths)
• I've updated tool descriptions/schemas if I changed tool behavior — N/A

[trac3r00]

Checked out pr-68228 locally (Python 3.11) and verified the claims end-to-end:

Test evidence

• On the PR head: tests/plugins/memory/test_openviking_provider.py + tests/plugins/memory/test_retaindb_provider.py → 144 passed (matches the PR description).
• Cherry-picking only the new tests onto main (no source changes) → 5 failed, 139 passed, and the failures are exactly the regressions this PR claims to fix:
  • test_resolve_connection_settings_reads_config_yaml_non_secret_fields — OpenViking never folds memory.openviking.* non-secret fields into resolution.
  • test_is_available_true_for_config_yaml_endpoint — a config.yaml-only endpoint reports needs_config on main.
  • The three RetainDB initialize() tests fail on main (_load_retaindb_config doesn't exist there), confirming base_url/project were env-only.

So the tests are genuine failing-then-green regression tests, not change detectors.

Code review

• Resolution order (env → ovcli → config.yaml → default for OpenViking; env → config.yaml → default for RetainDB) matches the documented intent, and secrets (api_key) stay env-only in both providers — good scoping.
• Both call sites of _resolve_connection_settings() (lines ~1956 and ~2156) already pass _load_hermes_openviking_config(), so the new provider_config fallback is reachable from both paths.
• _load_retaindb_config() swallowing all exceptions and returning {} mirrors the existing _load_hermes_openviking_config() pattern, so behavior is consistent across the two providers.

One minor behavioral note (not blocking)

In RetainDB, os.environ.get("RETAINDB_BASE_URL") or _config_str(...) means an env var explicitly set to the empty string now falls through to config.yaml/default, whereas main would have used the empty string verbatim (producing a broken "" base URL). I'd call that an improvement, but it's technically a behavior change worth being aware of.

LGTM — focused fix, real regression coverage, consistent with the surrounding config-reading patterns.

[bob]

[teknium1]

Thanks for the focused fix. The reported behavior is still present on current main: OpenViking ignores memory.openviking.endpoint in plugins/memory/openviking/__init__.py:920-939 and :1927-1938; RetainDB reads URL/project only from environment variables at plugins/memory/retaindb/__init__.py:490-502.

Problems

• The added tests stub the new config-loading boundary or pass an in-memory config directly, so they do not verify the Dashboard's persisted config.yaml path. The real write path persists non-secret fields under memory.<provider> at hermes_cli/web_server.py:6436-6467.
• The proposed RetainDB helper uses load_config() at plugins/memory/retaindb/__init__.py:54 even though it is read-only. OpenViking deliberately uses load_config_readonly() at plugins/memory/openviking/__init__.py:897-905; its test enforces that at tests/plugins/memory/test_openviking_provider.py:86-116.

Suggested changes

• Switch RetainDB to load_config_readonly().
• Add temp-HERMES_HOME file-backed regression coverage for both providers, including env-over-config precedence.

This is an automated hermes-sweeper review.

[teknium1]

This helper only reads configuration, so use load_config_readonly() here. That matches OpenViking's runtime reader (plugins/memory/openviking/__init__.py:897-905) and avoids the defensive deepcopy performed by load_config().

[kshitijk4poor]

Merged via #77747 — your commits cherry-picked with authorship preserved (rebase merge).

This PR consolidated your config.yaml readback fix for OpenViking and RetainDB together with 5 other OpenViking fixes into one coherent integration so the complete runtime behavior could be validated as a unit.

Thanks for the contribution!