diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index e59f9467ac236..bce1a423a1e6c 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -1332,7 +1332,8 @@ def _runtime_openviking_timeout_message(endpoint: str) -> str: 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." + "OpenViking memory disabled; will retry on a later access or when " + "the config changes." ) @@ -2187,8 +2188,9 @@ class OpenVikingMemoryProvider(MemoryProvider): return if not healthy: warning_message = ( - f"OpenViking server at {endpoint} is still not reachable after auto-start; " - "OpenViking memory disabled for this Hermes run." + f"OpenViking server at {endpoint} is still not reachable after auto-start. " + "OpenViking memory disabled; will retry on a later access or when " + "the config changes." ) else: self._client = client @@ -2206,7 +2208,8 @@ class OpenVikingMemoryProvider(MemoryProvider): except Exception as e: warning_message = ( f"OpenViking server at {endpoint} could not be attached after auto-start: {e}. " - "OpenViking memory disabled for this Hermes run." + "OpenViking memory disabled; will retry on a later access or when " + "the config changes." ) if warning_message: @@ -2230,8 +2233,9 @@ class OpenVikingMemoryProvider(MemoryProvider): 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. " + f"Remote OpenViking server at {endpoint} is not reachable. " + "OpenViking memory disabled; will retry on a later access or when " + "the config changes. " "Check the configured endpoint and network connectivity.", warning_callback, ) @@ -2256,7 +2260,8 @@ class OpenVikingMemoryProvider(MemoryProvider): self._runtime_start_pending = False warning_message = ( f"Local OpenViking server at {endpoint} is not reachable. {start_message} " - "OpenViking memory disabled for this Hermes run." + "OpenViking memory disabled; will retry on a later access or when " + "the config changes." ) self._client = None else: @@ -2333,7 +2338,8 @@ class OpenVikingMemoryProvider(MemoryProvider): ) elif health_state != "healthy": _emit_runtime_warning( - f"{health_message} OpenViking memory disabled for this Hermes run.", + f"{health_message} OpenViking memory disabled; will retry on a " + "later access or when the config changes.", warning_callback, ) self._client = None diff --git a/tests/openviking_plugin/test_openviking.py b/tests/openviking_plugin/test_openviking.py index c6edbc0105455..f1eff441c46a3 100644 --- a/tests/openviking_plugin/test_openviking.py +++ b/tests/openviking_plugin/test_openviking.py @@ -2002,3 +2002,169 @@ class TestEnsureClientFailureHardening: assert provider._conn_snapshot == healthy_snapshot built = provider._new_client() assert built.endpoint == "https://up.example" + + +class TestUnavailableWarningsPromiseRetry: + """Every "OpenViking is unavailable" warning must describe what actually + happens next. + + ``_ensure_client()`` rebuilds and re-probes the client whenever the + resolved config changes or the failed-config cooldown has elapsed, so no + warning may tell the user memory is off for the rest of the run — that + reads as "it never recovers" and sends people restarting hermes for + nothing (#5721). + """ + + @staticmethod + def _assert_promises_retry(message: str) -> None: + assert "for this Hermes run" not in message, message + assert "will retry on a later access" in message, message + assert "when the config changes" in message, message + + @staticmethod + def _stub_client(health_result): + class _StubClient: + def __init__(self, endpoint, api_key="", account="", user="", agent=""): + self.endpoint = endpoint + + def health(self): + return health_result + + return _StubClient + + def test_local_autostart_timeout_warning(self): + self._assert_promises_retry( + openviking_plugin._runtime_openviking_timeout_message("http://127.0.0.1:1934") + ) + + def test_remote_unreachable_warning(self): + provider = OpenVikingMemoryProvider() + provider._endpoint = "https://remote.example" + warnings: list[str] = [] + + provider._handle_runtime_openviking_unreachable(warning_callback=warnings.append) + + assert provider._client is None + assert len(warnings) == 1 + self._assert_promises_retry(warnings[0]) + + def test_local_autostart_refused_warning(self, monkeypatch): + monkeypatch.setattr( + openviking_plugin, + "_start_local_openviking_server", + lambda endpoint: (False, "openviking-server was not found on PATH."), + ) + provider = OpenVikingMemoryProvider() + provider._endpoint = "http://127.0.0.1:1934" + warnings: list[str] = [] + + provider._handle_runtime_openviking_unreachable(warning_callback=warnings.append) + + assert provider._client is None + assert len(warnings) == 1 + self._assert_promises_retry(warnings[0]) + + def test_still_unhealthy_after_autostart_warning(self, monkeypatch): + monkeypatch.setattr(openviking_plugin, "_VikingClient", self._stub_client(False)) + monkeypatch.setattr( + openviking_plugin, "_wait_for_openviking_health", lambda endpoint, **kwargs: True + ) + provider = OpenVikingMemoryProvider() + provider._endpoint = "http://127.0.0.1:1934" + warnings: list[str] = [] + + provider._finish_runtime_openviking_start(warning_callback=warnings.append) + + assert provider._client is None + assert len(warnings) == 1 + self._assert_promises_retry(warnings[0]) + + def test_attach_failure_after_autostart_warning(self, monkeypatch): + def _explode(*args, **kwargs): + raise RuntimeError("connection reset by peer") + + monkeypatch.setattr(openviking_plugin, "_VikingClient", _explode) + monkeypatch.setattr( + openviking_plugin, "_wait_for_openviking_health", lambda endpoint, **kwargs: True + ) + provider = OpenVikingMemoryProvider() + provider._endpoint = "http://127.0.0.1:1934" + warnings: list[str] = [] + + provider._finish_runtime_openviking_start(warning_callback=warnings.append) + + assert provider._client is None + assert len(warnings) == 1 + self._assert_promises_retry(warnings[0]) + + def test_initialize_responded_unhealthy_warning(self, monkeypatch, tmp_path): + class _UnhealthyClient: + def __init__(self, endpoint, api_key="", account="", user="", agent=""): + self.endpoint = endpoint + + def health_payload(self): + return {"healthy": False} + + def health(self): + return False + + monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes")) + monkeypatch.setenv("OPENVIKING_ENDPOINT", "https://sick.example") + monkeypatch.setattr(openviking_plugin, "_VikingClient", _UnhealthyClient) + provider = OpenVikingMemoryProvider() + warnings: list[str] = [] + + provider.initialize("session-1", platform="cli", warning_callback=warnings.append) + + assert provider._client is None + assert len(warnings) == 1 + self._assert_promises_retry(warnings[0]) + + def test_ensure_client_responded_unhealthy_warning(self, monkeypatch, caplog): + class _UnhealthyClient: + def __init__(self, endpoint, api_key="", account="", user="", agent=""): + self.endpoint = endpoint + + def health_payload(self): + return {"healthy": False} + + monkeypatch.setenv("OPENVIKING_ENDPOINT", "https://sick.example") + monkeypatch.setattr(openviking_plugin, "_VikingClient", _UnhealthyClient) + provider = OpenVikingMemoryProvider() + provider._env_refresh_enabled = True + + with caplog.at_level("WARNING", logger=openviking_plugin.__name__): + assert provider._ensure_client() is None + + self._assert_promises_retry(caplog.text) + + def test_startup_failure_really_does_reconnect_on_a_later_access( + self, monkeypatch, tmp_path + ): + """The warnings promise a retry — prove the provider delivers one.""" + probes: list[str] = [] + + class _FlakyClient: + def __init__(self, endpoint, api_key="", account="", user="", agent=""): + self.endpoint = endpoint + + def health(self): + probes.append(self.endpoint) + return len(probes) > 1 # down at startup, up on the next access + + monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes")) + monkeypatch.setenv("OPENVIKING_ENDPOINT", "https://remote.example") + monkeypatch.setattr(openviking_plugin, "_VikingClient", _FlakyClient) + provider = OpenVikingMemoryProvider() + warnings: list[str] = [] + + provider.initialize("session-1", platform="cli", warning_callback=warnings.append) + assert provider._client is None + assert len(warnings) == 1 + self._assert_promises_retry(warnings[0]) + + # A startup failure arms no cooldown, so the very next access re-probes. + client = provider._ensure_client() + assert client is not None + assert client.endpoint == "https://remote.example" + assert len(probes) == 2 diff --git a/tests/plugins/memory/test_openviking_provider.py b/tests/plugins/memory/test_openviking_provider.py index 914dc0621248a..1cee5ef90c36b 100644 --- a/tests/plugins/memory/test_openviking_provider.py +++ b/tests/plugins/memory/test_openviking_provider.py @@ -779,8 +779,9 @@ def test_https_local_endpoint_is_not_runtime_autostart_eligible(monkeypatch): assert provider._client is None assert warnings == [ - "Remote OpenViking server at https://localhost:1934 is not reachable; " - "OpenViking memory disabled for this Hermes run. " + "Remote OpenViking server at https://localhost:1934 is not reachable. " + "OpenViking memory disabled; will retry on a later access or when " + "the config changes. " "Check the configured endpoint and network connectivity." ] @@ -813,7 +814,7 @@ def test_runtime_does_not_autostart_when_local_server_reports_unhealthy(monkeypa 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." + "OpenViking memory disabled; will retry on a later access or when the config changes." ] @@ -1104,7 +1105,8 @@ def test_runtime_openviking_waiter_warns_when_background_start_times_out(monkeyp 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." + "within 60 seconds. OpenViking memory disabled; will retry on a later access " + "or when the config changes." ] @@ -1197,7 +1199,7 @@ def test_initialize_emits_cli_warning_when_local_runtime_autostart_fails(monkeyp 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." + "OpenViking memory disabled; will retry on a later access or when the config changes." ]