fix(openviking): prevent v0.3.3 HTTP 500s by normalizing /.overview.md and /.abstract.md pseudo-URIs before content reads - #5886
Conversation
OpenViking v0.3.3 expects directory URIs for abstract/overview reads. Passing pseudo-files like /.overview.md and /.abstract.md to /api/v1/content/overview|abstract triggers HTTP 500. This change normalizes those pseudo-URIs to their parent directory for abstract/overview requests, preserves full reads, and hardens parsing for wrapped/unwrapped result payloads and fs list response shapes.
ZaynJarvis
left a comment
There was a problem hiding this comment.
Review: URI Normalization for OpenViking v0.3.3
Verdict: Approve — solid defensive fix with good test coverage
Checklist Pass
-
_normalize_summary_uri(): strips/.abstract.md,/.overview.md,/.read.md,/.full.mdpseudo-file suffixes and returns the parent directory URI — correct for OV v0.3.3 which expects directory URIs for L0/L1 reads - Applied only for
level in ("abstract", "overview")—fullreads still use the original URI (correct: full read targets the actual file) -
_unwrap_result(): cleanly handles both{"result": ...}wrapped and raw unwrapped payloads - Browse response hardening: handles
{"entries": ...\}/{"items": ...\}/{"children": ...\}dict shapes AND bare list — covers both v0.3.2 (bare list) and v0.3.3 (wrapped object) API shapes - Browse
is_dirdetection: checksisDir,is_dir,type == "dir"— handles camelCase and snake_case server variants - Tests: 101 lines, covers normalization edge cases, read with resolved_uri tracking, browse with dict-wrapped entries
-
resolved_urifield added to_tool_readresponse — useful for debugging URI resolution
Minor Suggestion (not blocking)
_normalize_summary_uri and _unwrap_result are both @staticmethod — they could be module-level functions, but keeping them as static methods on the class is fine for discoverability.
Merge after #9167 since both touch _tool_read/_tool_browse.
OpenViking returns 500 for /content/abstract and /content/overview when URI points to mem_*.md files. Add resilient fallback to /content/read for non-pseudo summary file URIs while preserving pseudo summary normalization. Also add regression tests for fallback behavior.
|
Update from today's debugging (OpenViking v0.3.8): |
|
@ZaynJarvis thanks again — I pushed a follow-up commit after testing against OpenViking v0.3.8. The original normalization fix still stands, but I found |
|
lgtm, i would flag for merge with hermes agent team. Thx |
|
i do not mind which goes in |
…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.
|
Your 3 commits were cherry-picked onto current main with your authorship preserved (rebase merge, not squash):
On top of your work we added an |
…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 NousResearch#12757 and NousResearch#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 NousResearch#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.
…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 NousResearch#12757 and NousResearch#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 NousResearch#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.
…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 NousResearch#12757 and NousResearch#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 NousResearch#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.
…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 NousResearch#12757 and NousResearch#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 NousResearch#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.
…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 NousResearch#12757 and NousResearch#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 NousResearch#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.
…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 NousResearch#12757 and NousResearch#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 NousResearch#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.
…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 NousResearch#12757 and NousResearch#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 NousResearch#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.
…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 NousResearch#12757 and NousResearch#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 NousResearch#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.
…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 NousResearch#12757 and NousResearch#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 NousResearch#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.
Why this is necessary
I self host my own openviking setup and several hermes agents against it (local models + gpt 5.4) here and I kept running into this. This patch fixes it. Coded with hermes agent / gpt 5.3 codex.
OpenViking v0.3.3 content endpoints (
/api/v1/content/overview,/api/v1/content/abstract) expect directory URIs.The Hermes OpenViking plugin currently forwards pseudo-summary file URIs like:
viking://user/hermes/.overview.mdviking://resources/.abstract.mdThat mismatch causes server-side HTTP 500 responses, so
viking_read(level=overview|abstract)fails even though the backend is healthy and reachable.Reproduction (before this patch)
viking_read(uri="viking://user/hermes/.overview.md", level="overview")-> 500viking_read(uri="viking://resources/.abstract.md", level="abstract")-> 500What this PR changes
/.overview.md,/.abstract.md,/.read.md,/.full.md) to their parent directory URI for abstract/overview reads.level=full).listvsdict,isDirvsis_dir, missingname).Verification
Validated against a live OpenViking
v0.3.3server:viking://user/hermes/.overview.md->viking://user/hermesand succeedsviking://resources/.abstract.md->viking://resourcesand succeedsviking://user/hermes/memories/profile.mdstill succeedsImpact
This fixes a real integration breakage where normal user-facing
viking_readcalls fail with 500 due to URI shape mismatch, while preserving existing full-read behavior.