fix(openviking): pre-check fs/stat to route file URIs before hitting directory-only endpoints
Adds a deterministic pre-check on top of htsh's exception-based fallback: before calling /content/abstract or /content/overview on a non-pseudo URI, probe /api/v1/fs/stat. If the server says the URI is a file, route straight to /content/read instead of eating a failing 500 round-trip. This is the same idea pty819 and chennest independently landed in PRs #12757 and #12937 — merged here on top of htsh's broader fix so we keep pseudo-URI normalization and v0.3.3 browse-shape handling while avoiding the slow exception path on servers that return a raised 500 every time. The exception fallback from #5886 stays in place for environments where fs/stat is unavailable or returns an unfamiliar shape. Also credits pty819, chennest, and htsh in AUTHOR_MAP so future release notes attribute them correctly.
This commit is contained in:
parent
10e43edc09
commit
5d253e65b7
@ -545,6 +545,29 @@ class OpenVikingMemoryProvider(MemoryProvider):
|
|||||||
return uri[: -len(suffix)] or "viking://"
|
return uri[: -len(suffix)] or "viking://"
|
||||||
return uri
|
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:
|
def _tool_search(self, args: dict) -> str:
|
||||||
query = args.get("query", "")
|
query = args.get("query", "")
|
||||||
if not query:
|
if not query:
|
||||||
@ -600,19 +623,32 @@ class OpenVikingMemoryProvider(MemoryProvider):
|
|||||||
resolved_uri = self._normalize_summary_uri(uri) if summary_level else uri
|
resolved_uri = self._normalize_summary_uri(uri) if summary_level else uri
|
||||||
used_fallback = False
|
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.
|
# Map our level names to OpenViking GET endpoints.
|
||||||
endpoint = "/api/v1/content/read"
|
endpoint = "/api/v1/content/read"
|
||||||
if level == "abstract":
|
if not used_fallback:
|
||||||
endpoint = "/api/v1/content/abstract"
|
if level == "abstract":
|
||||||
elif level == "overview":
|
endpoint = "/api/v1/content/abstract"
|
||||||
endpoint = "/api/v1/content/overview"
|
elif level == "overview":
|
||||||
|
endpoint = "/api/v1/content/overview"
|
||||||
|
|
||||||
try:
|
try:
|
||||||
resp = self._client.get(endpoint, params={"uri": resolved_uri})
|
resp = self._client.get(endpoint, params={"uri": resolved_uri})
|
||||||
except Exception:
|
except Exception:
|
||||||
# OpenViking may return HTTP 500 for abstract/overview reads on normal
|
# OpenViking may return HTTP 500 for abstract/overview reads on normal
|
||||||
# file URIs (mem_*.md). For those, gracefully fallback to full read.
|
# 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
|
raise
|
||||||
resp = self._client.get("/api/v1/content/read", params={"uri": uri})
|
resp = self._client.get("/api/v1/content/read", params={"uri": uri})
|
||||||
used_fallback = True
|
used_fallback = True
|
||||||
|
|||||||
@ -58,6 +58,11 @@ AUTHOR_MAP = {
|
|||||||
"nbot@liizfq.top": "liizfq",
|
"nbot@liizfq.top": "liizfq",
|
||||||
"274096618+hermes-agent-dhabibi@users.noreply.github.com": "dhabibi",
|
"274096618+hermes-agent-dhabibi@users.noreply.github.com": "dhabibi",
|
||||||
"dejie.guo@gmail.com": "JayGwod",
|
"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",
|
"aamirjawaid@microsoft.com": "heyitsaamir",
|
||||||
"johnnncenaaa77@gmail.com": "johnncenae",
|
"johnnncenaaa77@gmail.com": "johnncenae",
|
||||||
"thomasjhon6666@gmail.com": "ThomassJonax",
|
"thomasjhon6666@gmail.com": "ThomassJonax",
|
||||||
|
|||||||
@ -71,11 +71,91 @@ class TestOpenVikingRead:
|
|||||||
{"uri": "viking://user/hermes/memories/profile.md"},
|
{"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()
|
provider = OpenVikingMemoryProvider()
|
||||||
file_uri = "viking://user/hermes/memories/entities/mem_abc.md"
|
file_uri = "viking://user/hermes/memories/entities/mem_abc.md"
|
||||||
provider._client = FakeVikingClient(
|
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",
|
"/api/v1/content/overview",
|
||||||
(("uri", file_uri),),
|
(("uri", file_uri),),
|
||||||
@ -90,11 +170,11 @@ class TestOpenVikingRead:
|
|||||||
result = json.loads(provider._tool_read({"uri": file_uri, "level": "overview"}))
|
result = json.loads(provider._tool_read({"uri": file_uri, "level": "overview"}))
|
||||||
|
|
||||||
assert result["uri"] == file_uri
|
assert result["uri"] == file_uri
|
||||||
assert result["resolved_uri"] == file_uri
|
|
||||||
assert result["level"] == "overview"
|
assert result["level"] == "overview"
|
||||||
assert result["fallback"] == "content/read"
|
assert result["fallback"] == "content/read"
|
||||||
assert result["content"] == "fallback full content"
|
assert result["content"] == "fallback full content"
|
||||||
assert provider._client.calls == [
|
assert provider._client.calls == [
|
||||||
|
("/api/v1/fs/stat", {"uri": file_uri}),
|
||||||
("/api/v1/content/overview", {"uri": file_uri}),
|
("/api/v1/content/overview", {"uri": file_uri}),
|
||||||
("/api/v1/content/read", {"uri": file_uri}),
|
("/api/v1/content/read", {"uri": file_uri}),
|
||||||
]
|
]
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user