fix(openviking): refresh client from env on every access

initialize() snapshots OPENVIKING_* into the provider once, so /reload
(which only updates os.environ) leaves viking_* tools running against
stale auth — users have to restart hermes to pick up keys added to
~/.hermes/.env after startup.

Add _ensure_client(), which re-resolves the connection settings via the
same _resolve_connection_settings/_load_hermes_openviking_config path
initialize() uses and rebuilds + health-checks the client only when an
OPENVIKING_* value actually changed; otherwise it reuses the cached
client so the hot path stays at one dict comparison with no network
calls. Every `if not self._client:` guard in system_prompt_block,
queue_prefetch, sync_turn, on_session_end, on_memory_write and
handle_tool_call now goes through it.

Refreshing is gated behind a flag set at the end of initialize() so the
baseline is established before any env re-resolution happens — callers
that wire up a client directly keep the existing client untouched.

Refs #21130
This commit is contained in:
0xDevNinja 2026-06-22 17:11:58 +05:30
parent b4cb33cd42
commit b694d21b7c
2 changed files with 176 additions and 6 deletions

View file

@ -1697,6 +1697,11 @@ class OpenVikingMemoryProvider(MemoryProvider):
self._api_key = ""
self._session_id = ""
self._turn_count = 0
# Set once initialize() has resolved the connection baseline. Until then
# _ensure_client() must not re-resolve from the environment — callers
# that wire up a client directly (e.g. tests) would otherwise have it
# discarded. See _ensure_client() / #21130.
self._env_refresh_enabled = False
# Guards the (_session_id, _turn_count) pair. sync_turn runs on the
# MemoryManager's background sync executor while on_session_end /
# on_session_switch run on the caller's thread, so the snapshot+reset
@ -2031,8 +2036,72 @@ class OpenVikingMemoryProvider(MemoryProvider):
global _last_active_provider
_last_active_provider = self
# Baseline established — subsequent accesses may refresh from env (#21130).
self._env_refresh_enabled = True
def _ensure_client(self) -> Optional["_VikingClient"]:
"""Return the active client, rebuilding it if the resolved config changed.
``/reload`` only refreshes ``os.environ`` the existing provider
instance is not re-initialized so OPENVIKING_* values added to
``~/.hermes/.env`` after startup never reach the live client and tools
keep running against stale auth until the user restarts hermes (#21130).
Re-resolve the connection settings on each access (same layering as
``initialize``) and rebuild + health-check only when a value actually
changed; otherwise reuse the cached client so the hot path stays at one
dict comparison with zero network calls.
"""
# Before initialize() runs there is no env baseline to refresh against;
# return whatever client the caller wired up (matches legacy behavior).
if not self._env_refresh_enabled:
return self._client
settings = _resolve_connection_settings(_load_hermes_openviking_config())
endpoint = settings["endpoint"]
api_key = settings["api_key"]
account = settings["account"]
user = settings["user"]
agent = settings["agent"]
config_unchanged = (
endpoint == getattr(self, "_endpoint", None)
and api_key == getattr(self, "_api_key", None)
and account == getattr(self, "_account", None)
and user == getattr(self, "_user", None)
and agent == getattr(self, "_agent", None)
)
if config_unchanged and self._client is not None:
return self._client
self._endpoint = endpoint
self._api_key = api_key
self._account = account
self._user = user
self._agent = agent
try:
client = _VikingClient(
endpoint, api_key, account=account, user=user, agent=agent,
)
except ImportError:
logger.warning("httpx not installed — OpenViking plugin disabled")
self._client = None
return None
health_state, health_message = _classify_runtime_openviking_health(client, endpoint)
if health_state == "healthy":
self._client = client
return self._client
if health_state == "responded":
logger.warning("%s OpenViking memory disabled until config changes.", health_message)
else: # unreachable
logger.warning("OpenViking server at %s is not reachable", endpoint)
self._client = None
return None
def system_prompt_block(self) -> str:
if not self._client:
if not self._ensure_client():
return ""
# Provide brief info about the knowledge base
try:
@ -2072,7 +2141,7 @@ class OpenVikingMemoryProvider(MemoryProvider):
def queue_prefetch(self, query: str, *, session_id: str = "") -> None:
"""Fire a background search to pre-load relevant context."""
query = _derive_openviking_user_text(query)
if not self._client or not query:
if not query or not self._ensure_client():
return
# Drop prefetch results from older switch generations.
@ -2602,7 +2671,7 @@ class OpenVikingMemoryProvider(MemoryProvider):
messages: Optional[List[Dict[str, Any]]] = None,
) -> None:
"""Record the conversation turn in OpenViking's session (non-blocking)."""
if not self._client:
if not self._ensure_client():
return
user_content = _derive_openviking_user_text(user_content)
@ -2698,7 +2767,7 @@ class OpenVikingMemoryProvider(MemoryProvider):
OpenViking automatically extracts 6 categories of memories:
profile, preferences, entities, events, cases, and patterns.
"""
if not self._client:
if not self._ensure_client():
return
# Snapshot sid + turn count atomically against a concurrent sync_turn
@ -2807,7 +2876,7 @@ class OpenVikingMemoryProvider(MemoryProvider):
metadata: Optional[Dict[str, Any]] = None,
) -> None:
"""Mirror built-in memory writes to OpenViking via content/write."""
if not self._client or action != "add" or not content:
if action != "add" or not content or not self._ensure_client():
return
subdir = _MEMORY_WRITE_TARGET_SUBDIR_MAP.get(target, _DEFAULT_MEMORY_SUBDIR)
@ -2834,7 +2903,7 @@ class OpenVikingMemoryProvider(MemoryProvider):
return [SEARCH_SCHEMA, READ_SCHEMA, BROWSE_SCHEMA, REMEMBER_SCHEMA, ADD_RESOURCE_SCHEMA]
def handle_tool_call(self, tool_name: str, args: dict, **kwargs) -> str:
if not self._client:
if not self._ensure_client():
return tool_error("OpenViking server not connected")
try:

View file

@ -869,3 +869,104 @@ class TestOpenVikingMemoryUriBuilder:
assert f"/memories/{subdir}/mem_" in uri, (
f"subdir '{subdir}' not placed correctly in URI: {uri}"
)
# ===================================================================
# Issue #21130 — OPENVIKING_* not reloaded after /reload
# ===================================================================
class TestEnsureClientReloadsEnv:
"""Verify /reload picks up new OPENVIKING_* values without a restart (#21130)."""
def test_ensure_client_rebuilds_when_api_key_changes(self, monkeypatch):
constructions = []
class _StubClient:
def __init__(self, endpoint, api_key, account="", user="", agent="hermes"):
constructions.append({"endpoint": endpoint, "api_key": api_key,
"account": account, "user": user, "agent": agent})
self.endpoint, self.api_key = endpoint, api_key
self.account, self.user, self.agent = account, user, agent
def health(self):
return True
monkeypatch.setattr("plugins.memory.openviking._VikingClient", _StubClient)
monkeypatch.setenv("OPENVIKING_ENDPOINT", "http://srv:31933")
monkeypatch.setenv("OPENVIKING_API_KEY", "")
provider = OpenVikingMemoryProvider()
provider._env_refresh_enabled = True
first = provider._ensure_client()
assert first is not None
assert first.api_key == ""
assert len(constructions) == 1
# Same env on second call — must reuse cached client (no rebuild).
assert provider._ensure_client() is first
assert len(constructions) == 1
# Simulate /reload: env now carries the new API key.
monkeypatch.setenv("OPENVIKING_API_KEY", "sk-fresh")
rebuilt = provider._ensure_client()
assert rebuilt is not None
assert rebuilt is not first
assert rebuilt.api_key == "sk-fresh"
assert len(constructions) == 2
def test_ensure_client_rebuilds_when_endpoint_changes(self, monkeypatch):
builds = []
class _StubClient:
def __init__(self, endpoint, api_key, **kw):
builds.append(endpoint)
self.endpoint = endpoint
def health(self):
return True
monkeypatch.setattr("plugins.memory.openviking._VikingClient", _StubClient)
monkeypatch.setenv("OPENVIKING_ENDPOINT", "http://a")
monkeypatch.setenv("OPENVIKING_API_KEY", "key")
provider = OpenVikingMemoryProvider()
provider._env_refresh_enabled = True
provider._ensure_client()
provider._ensure_client() # cached
monkeypatch.setenv("OPENVIKING_ENDPOINT", "http://b")
provider._ensure_client() # rebuilds
assert builds == ["http://a", "http://b"]
def test_ensure_client_returns_none_when_health_fails(self, monkeypatch):
class _StubClient:
def __init__(self, *a, **kw):
pass
def health(self):
return False
monkeypatch.setattr("plugins.memory.openviking._VikingClient", _StubClient)
monkeypatch.setenv("OPENVIKING_ENDPOINT", "http://dead")
monkeypatch.setenv("OPENVIKING_API_KEY", "")
provider = OpenVikingMemoryProvider()
provider._env_refresh_enabled = True
assert provider._ensure_client() is None
assert provider._client is None
def test_handle_tool_call_uses_ensure_client(self, monkeypatch):
provider = OpenVikingMemoryProvider()
provider._env_refresh_enabled = True
class _StubClient:
def __init__(self, *a, **kw):
pass
def health(self):
return False
monkeypatch.setattr("plugins.memory.openviking._VikingClient", _StubClient)
out = provider.handle_tool_call("viking_search", {"query": "x"})
assert "not connected" in out.lower()