diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index f8687eb2bd05..8ea4a4bedcca 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -528,6 +528,46 @@ def shutdown(self) -> None: # -- Tool implementations ------------------------------------------------ + @staticmethod + def _unwrap_result(resp: Any) -> Any: + """Return OpenViking payload body regardless of wrapped/unwrapped shape.""" + if isinstance(resp, dict) and "result" in resp: + return resp.get("result") + return resp + + @staticmethod + def _normalize_summary_uri(uri: str) -> str: + """Map pseudo summary files to their parent directory URI for L0/L1 reads.""" + if not uri: + return uri + for suffix in ("/.abstract.md", "/.overview.md", "/.read.md", "/.full.md"): + if uri.endswith(suffix): + 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: @@ -576,27 +616,72 @@ def _tool_read(self, args: dict) -> str: return tool_error("uri is required") level = args.get("level", "overview") - # Map our level names to OpenViking GET endpoints - if level == "abstract": - resp = self._client.get("/api/v1/content/abstract", params={"uri": uri}) - elif level == "full": - resp = self._client.get("/api/v1/content/read", params={"uri": uri}) - else: # overview - resp = self._client.get("/api/v1/content/overview", params={"uri": uri}) - result = resp.get("result", "") - # result is a plain string from the content endpoints - content = result if isinstance(result, str) else result.get("content", "") + summary_level = level in ("abstract", "overview") + # OpenViking expects directory URIs for pseudo summary files + # (e.g. viking://user/hermes/.overview.md). + 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 not used_fallback: + if level == "abstract": + endpoint = "/api/v1/content/abstract" + elif level == "overview": + endpoint = "/api/v1/content/overview" - # Truncate very long content to avoid flooding the context - if len(content) > 8000: - content = content[:8000] + "\n\n[... truncated, use a more specific URI or abstract level]" - - return json.dumps({ + 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 or used_fallback: + raise + resp = self._client.get("/api/v1/content/read", params={"uri": uri}) + used_fallback = True + + result = self._unwrap_result(resp) + # Content endpoints may return either plain strings or objects. + if isinstance(result, str): + content = result + elif isinstance(result, dict): + content = result.get("content", "") or result.get("text", "") + else: + content = "" + + # Truncate long content to avoid flooding context. + max_len = 8000 + if level == "overview": + max_len = 4000 + elif level == "abstract": + max_len = 1200 + + if len(content) > max_len: + content = content[:max_len] + "\n\n[... truncated, use a more specific URI or full level]" + + payload = { "uri": uri, + "resolved_uri": resolved_uri, "level": level, "content": content, - }, ensure_ascii=False) + } + if used_fallback: + payload["fallback"] = "content/read" + + return json.dumps(payload, ensure_ascii=False) def _tool_browse(self, args: dict) -> str: action = args.get("action", "list") @@ -606,19 +691,27 @@ def _tool_browse(self, args: dict) -> str: endpoint_map = {"tree": "/api/v1/fs/tree", "list": "/api/v1/fs/ls", "stat": "/api/v1/fs/stat"} endpoint = endpoint_map.get(action, "/api/v1/fs/ls") resp = self._client.get(endpoint, params={"uri": path}) - result = resp.get("result", {}) + result = self._unwrap_result(resp) # Format list/tree results for readability - if action in ("list", "tree") and isinstance(result, list): - entries = [] - for e in result[:50]: # cap at 50 entries - entries.append({ - "name": e.get("rel_path", e.get("name", "")), - "uri": e.get("uri", ""), - "type": "dir" if e.get("isDir") else "file", - "abstract": e.get("abstract", ""), - }) - return json.dumps({"path": path, "entries": entries}, ensure_ascii=False) + if action in ("list", "tree"): + raw_entries = result + if isinstance(result, dict): + raw_entries = result.get("entries") or result.get("items") or result.get("children") or [] + + if isinstance(raw_entries, list): + entries = [] + for e in raw_entries[:50]: # cap at 50 entries + uri = e.get("uri", "") + name = e.get("rel_path") or e.get("name") or (uri.rsplit("/", 1)[-1] if uri else "") + is_dir = bool(e.get("isDir") or e.get("is_dir") or e.get("type") == "dir") + entries.append({ + "name": name, + "uri": uri, + "type": "dir" if is_dir else "file", + "abstract": e.get("abstract", ""), + }) + return json.dumps({"path": path, "entries": entries}, ensure_ascii=False) return json.dumps(result, ensure_ascii=False) diff --git a/scripts/release.py b/scripts/release.py index d9ea666e5f61..baeb7dbe1c7b 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -58,6 +58,11 @@ "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 new file mode 100644 index 000000000000..6848afc4759c --- /dev/null +++ b/tests/openviking_plugin/test_openviking.py @@ -0,0 +1,233 @@ +"""Tests for plugins/memory/openviking/__init__.py — URI normalization and payload handling.""" + +import json + +from plugins.memory.openviking import OpenVikingMemoryProvider + + +class FakeVikingClient: + def __init__(self, responses): + self.responses = responses + self.calls = [] + + def get(self, path, params=None, **kwargs): + self.calls.append((path, params or {})) + response = self.responses[(path, tuple(sorted((params or {}).items())))] + if isinstance(response, Exception): + raise response + return response + + +class TestOpenVikingSummaryUriNormalization: + def test_normalize_summary_uri_maps_pseudo_files_to_parent_directory(self): + assert OpenVikingMemoryProvider._normalize_summary_uri("viking://user/hermes/.overview.md") == "viking://user/hermes" + assert OpenVikingMemoryProvider._normalize_summary_uri("viking://resources/.abstract.md") == "viking://resources" + assert OpenVikingMemoryProvider._normalize_summary_uri("viking://") == "viking://" + assert OpenVikingMemoryProvider._normalize_summary_uri("viking://user/hermes/memories/profile.md") == "viking://user/hermes/memories/profile.md" + + +class TestOpenVikingRead: + def test_overview_read_normalizes_uri_and_unwraps_result(self): + provider = OpenVikingMemoryProvider() + provider._client = FakeVikingClient( + { + ( + "/api/v1/content/overview", + (("uri", "viking://user/hermes"),), + ): {"result": {"content": "overview text"}}, + } + ) + + result = json.loads(provider._tool_read({"uri": "viking://user/hermes/.overview.md", "level": "overview"})) + + assert result["uri"] == "viking://user/hermes/.overview.md" + assert result["resolved_uri"] == "viking://user/hermes" + assert result["level"] == "overview" + assert result["content"] == "overview text" + assert provider._client.calls == [( + "/api/v1/content/overview", + {"uri": "viking://user/hermes"}, + )] + + def test_full_read_keeps_original_uri(self): + provider = OpenVikingMemoryProvider() + provider._client = FakeVikingClient( + { + ( + "/api/v1/content/read", + (("uri", "viking://user/hermes/memories/profile.md"),), + ): {"result": "full text"}, + } + ) + + result = json.loads(provider._tool_read({"uri": "viking://user/hermes/memories/profile.md", "level": "full"})) + + assert result["uri"] == "viking://user/hermes/memories/profile.md" + assert result["resolved_uri"] == "viking://user/hermes/memories/profile.md" + assert result["level"] == "full" + assert result["content"] == "full text" + assert provider._client.calls == [( + "/api/v1/content/read", + {"uri": "viking://user/hermes/memories/profile.md"}, + )] + + 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),), + ): RuntimeError("500 Internal Server Error"), + ( + "/api/v1/content/read", + (("uri", file_uri),), + ): {"result": {"content": "fallback full content"}}, + } + ) + + result = json.loads(provider._tool_read({"uri": file_uri, "level": "overview"})) + + assert result["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}), + ] + + def test_summary_uri_error_does_not_fallback_and_raises(self): + provider = OpenVikingMemoryProvider() + provider._client = FakeVikingClient( + { + ( + "/api/v1/content/overview", + (("uri", "viking://user/hermes"),), + ): RuntimeError("500 Internal Server Error"), + } + ) + + try: + provider._tool_read({"uri": "viking://user/hermes/.overview.md", "level": "overview"}) + assert False, "Expected summary endpoint error to be raised" + except RuntimeError: + pass + + assert provider._client.calls == [ + ("/api/v1/content/overview", {"uri": "viking://user/hermes"}), + ] + + +class TestOpenVikingBrowse: + def test_list_browse_unwraps_and_normalizes_entry_shapes(self): + provider = OpenVikingMemoryProvider() + provider._client = FakeVikingClient( + { + ( + "/api/v1/fs/ls", + (("uri", "viking://user/hermes"),), + ): { + "result": { + "entries": [ + {"name": "memories", "uri": "viking://user/hermes/memories", "type": "dir"}, + {"rel_path": "profile.md", "uri": "viking://user/hermes/memories/profile.md", "isDir": False, "abstract": "Profile"}, + ] + } + }, + } + ) + + result = json.loads(provider._tool_browse({"action": "list", "path": "viking://user/hermes"})) + + assert result["path"] == "viking://user/hermes" + assert result["entries"] == [ + {"name": "memories", "uri": "viking://user/hermes/memories", "type": "dir", "abstract": ""}, + {"name": "profile.md", "uri": "viking://user/hermes/memories/profile.md", "type": "file", "abstract": "Profile"}, + ] + assert provider._client.calls == [( + "/api/v1/fs/ls", + {"uri": "viking://user/hermes"}, + )]