diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index b88114def1..8ea4a4bedc 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -545,6 +545,29 @@ class OpenVikingMemoryProvider(MemoryProvider): return uri[: -len(suffix)] or "viking://" return uri + def _is_directory_uri(self, uri: str) -> bool | None: + """Probe fs/stat to decide if a URI is a directory. + + Returns True/False when the server answers cleanly, and None when the + probe itself fails (network error, unexpected shape). Callers should + treat None as "unknown" and fall back to the exception-based path. + """ + try: + resp = self._client.get("/api/v1/fs/stat", params={"uri": uri}) + except Exception: + return None + result = self._unwrap_result(resp) + if isinstance(result, dict): + if "isDir" in result: + return bool(result.get("isDir")) + if "is_dir" in result: + return bool(result.get("is_dir")) + if result.get("type") == "dir": + return True + if result.get("type") == "file": + return False + return None + def _tool_search(self, args: dict) -> str: query = args.get("query", "") if not query: @@ -600,19 +623,32 @@ class OpenVikingMemoryProvider(MemoryProvider): resolved_uri = self._normalize_summary_uri(uri) if summary_level else uri used_fallback = False + # abstract/overview endpoints are directory-only on OpenViking + # (v0.3.x returns 500/412 for file URIs). When the caller asks for a + # summary level on a non-pseudo URI, probe fs/stat first and route + # file URIs straight to /content/read instead of eating a failing + # round-trip. The pseudo-URI path already points at a directory, so + # skip the probe there. + if summary_level and resolved_uri == uri: + is_dir = self._is_directory_uri(uri) + if is_dir is False: + resolved_uri = uri + used_fallback = True + # Map our level names to OpenViking GET endpoints. endpoint = "/api/v1/content/read" - if level == "abstract": - endpoint = "/api/v1/content/abstract" - elif level == "overview": - endpoint = "/api/v1/content/overview" + if not used_fallback: + if level == "abstract": + endpoint = "/api/v1/content/abstract" + elif level == "overview": + endpoint = "/api/v1/content/overview" try: resp = self._client.get(endpoint, params={"uri": resolved_uri}) except Exception: # OpenViking may return HTTP 500 for abstract/overview reads on normal # file URIs (mem_*.md). For those, gracefully fallback to full read. - if not summary_level or resolved_uri != uri: + if not summary_level or resolved_uri != uri or used_fallback: raise resp = self._client.get("/api/v1/content/read", params={"uri": uri}) used_fallback = True diff --git a/scripts/release.py b/scripts/release.py index d9ea666e5f..baeb7dbe1c 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -58,6 +58,11 @@ AUTHOR_MAP = { "nbot@liizfq.top": "liizfq", "274096618+hermes-agent-dhabibi@users.noreply.github.com": "dhabibi", "dejie.guo@gmail.com": "JayGwod", + # OpenViking viking_read salvage (April 2026) + "hitesh@gmail.com": "htsh", + "pty819@outlook.com": "pty819", + "pty819@users.noreply.github.com": "pty819", + "517024110@qq.com": "chennest", "aamirjawaid@microsoft.com": "heyitsaamir", "johnnncenaaa77@gmail.com": "johnncenae", "thomasjhon6666@gmail.com": "ThomassJonax", diff --git a/tests/openviking_plugin/test_openviking.py b/tests/openviking_plugin/test_openviking.py index 81dcfb0cf1..6848afc475 100644 --- a/tests/openviking_plugin/test_openviking.py +++ b/tests/openviking_plugin/test_openviking.py @@ -71,11 +71,91 @@ class TestOpenVikingRead: {"uri": "viking://user/hermes/memories/profile.md"}, )] - def test_overview_file_uri_falls_back_to_content_read_on_summary_error(self): + def test_overview_file_uri_routes_straight_to_content_read_via_stat_probe(self): + """Pre-check via fs/stat: file URIs skip the directory-only endpoint entirely.""" provider = OpenVikingMemoryProvider() file_uri = "viking://user/hermes/memories/entities/mem_abc.md" provider._client = FakeVikingClient( { + ( + "/api/v1/fs/stat", + (("uri", file_uri),), + ): {"result": {"isDir": False}}, + ( + "/api/v1/content/read", + (("uri", file_uri),), + ): {"result": {"content": "full content"}}, + } + ) + + result = json.loads(provider._tool_read({"uri": file_uri, "level": "overview"})) + + assert result["uri"] == file_uri + assert result["resolved_uri"] == file_uri + assert result["level"] == "overview" + assert result["fallback"] == "content/read" + assert result["content"] == "full content" + assert provider._client.calls == [ + ("/api/v1/fs/stat", {"uri": file_uri}), + ("/api/v1/content/read", {"uri": file_uri}), + ] + + def test_overview_dir_uri_skips_stat_when_pseudo_summary(self): + """Pseudo-URI path already resolves to dir, so no stat probe needed.""" + provider = OpenVikingMemoryProvider() + provider._client = FakeVikingClient( + { + ( + "/api/v1/content/overview", + (("uri", "viking://user/hermes"),), + ): {"result": "overview"}, + } + ) + + result = json.loads(provider._tool_read({"uri": "viking://user/hermes/.overview.md", "level": "overview"})) + + assert result["content"] == "overview" + # No fs/stat call — normalization already determined it's a directory. + assert provider._client.calls == [ + ("/api/v1/content/overview", {"uri": "viking://user/hermes"}), + ] + + def test_overview_directory_uri_uses_stat_probe_then_overview(self): + """Non-pseudo directory URI: stat → isDir=True → summary endpoint.""" + provider = OpenVikingMemoryProvider() + dir_uri = "viking://user/hermes/memories" + provider._client = FakeVikingClient( + { + ( + "/api/v1/fs/stat", + (("uri", dir_uri),), + ): {"result": {"isDir": True}}, + ( + "/api/v1/content/overview", + (("uri", dir_uri),), + ): {"result": "dir overview"}, + } + ) + + result = json.loads(provider._tool_read({"uri": dir_uri, "level": "overview"})) + + assert result["content"] == "dir overview" + assert "fallback" not in result + assert provider._client.calls == [ + ("/api/v1/fs/stat", {"uri": dir_uri}), + ("/api/v1/content/overview", {"uri": dir_uri}), + ] + + def test_overview_file_uri_falls_back_via_exception_when_stat_indeterminate(self): + """If fs/stat raises or returns unknown shape, legacy exception fallback still kicks in.""" + provider = OpenVikingMemoryProvider() + file_uri = "viking://user/hermes/memories/entities/mem_abc.md" + provider._client = FakeVikingClient( + { + ( + "/api/v1/fs/stat", + (("uri", file_uri),), + ): RuntimeError("stat unavailable"), ( "/api/v1/content/overview", (("uri", file_uri),), @@ -90,11 +170,11 @@ class TestOpenVikingRead: result = json.loads(provider._tool_read({"uri": file_uri, "level": "overview"})) assert result["uri"] == file_uri - assert result["resolved_uri"] == file_uri assert result["level"] == "overview" assert result["fallback"] == "content/read" assert result["content"] == "fallback full content" assert provider._client.calls == [ + ("/api/v1/fs/stat", {"uri": file_uri}), ("/api/v1/content/overview", {"uri": file_uri}), ("/api/v1/content/read", {"uri": file_uri}), ]