From ab38125855258d7e3bdf301c080ba16f5e9bd362 Mon Sep 17 00:00:00 2001 From: r266-tech Date: Wed, 25 Feb 2026 16:34:33 +0800 Subject: [PATCH] fix: support short-format URI paths in VikingFS and VikingURI Previously, passing short-format paths like '/resources' or 'resources' (without the 'viking://' prefix) to VikingFS._uri_to_path() caused incorrect path conversion. For example, '/resources' was truncated to '/local/s' instead of '/local/resources', because the method blindly stripped the first 9 characters (len('viking://')) regardless of format. Similarly, VikingURI._parse() would raise ValueError for any URI not starting with 'viking://', even though the existing normalize() method already handled this conversion. Changes: - VikingFS._uri_to_path: Check URI format before stripping prefix; handle 'viking://', '/' prefix, and bare paths correctly - VikingURI._parse: Auto-normalize short-format paths using the existing normalize() static method instead of raising ValueError - Add comprehensive tests for both fixes Fixes #259 --- openviking/storage/viking_fs.py | 16 +++++- openviking_cli/utils/uri.py | 6 ++- tests/test_uri_short_format.py | 93 +++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 tests/test_uri_short_format.py diff --git a/openviking/storage/viking_fs.py b/openviking/storage/viking_fs.py index 28638083..1fed1cf1 100644 --- a/openviking/storage/viking_fs.py +++ b/openviking/storage/viking_fs.py @@ -704,8 +704,20 @@ def _shorten_component(component: str, max_bytes: int = 255) -> str: return f"{prefix}_{hash_suffix}" def _uri_to_path(self, uri: str) -> str: - """viking://user/memories/preferences/test -> /local/user/memories/preferences/test""" - remainder = uri[len("viking://") :].strip("/") + """Convert Viking URI or short-format path to internal AGFS path. + + Supports both full URIs and short-format paths: + viking://user/memories/preferences/test -> /local/user/memories/preferences/test + /resources -> /local/resources + resources -> /local/resources + """ + if uri.startswith("viking://"): + remainder = uri[len("viking://"):].strip("/") + elif uri.startswith("/"): + remainder = uri.lstrip("/") + else: + remainder = uri.strip("/") + if not remainder: return "/local" # Ensure each path component does not exceed filesystem filename limit diff --git a/openviking_cli/utils/uri.py b/openviking_cli/utils/uri.py index 144ae034..1afaf6bc 100644 --- a/openviking_cli/utils/uri.py +++ b/openviking_cli/utils/uri.py @@ -48,11 +48,15 @@ def _parse(self) -> Dict[str, str]: """ Parse Viking URI into components. + Accepts both full URIs (viking://...) and short-format paths + (/resources, resources). Short-format paths are auto-normalized. + Returns: Dictionary with URI components """ if not self.uri.startswith(f"{self.SCHEME}://"): - raise ValueError(f"URI must start with '{self.SCHEME}://'") + # Auto-normalize short-format paths (e.g., "/resources" or "resources") + self.uri = self.normalize(self.uri) # Remove scheme path = self.uri[len(f"{self.SCHEME}://") :] diff --git a/tests/test_uri_short_format.py b/tests/test_uri_short_format.py new file mode 100644 index 00000000..1c89640f --- /dev/null +++ b/tests/test_uri_short_format.py @@ -0,0 +1,93 @@ +# Copyright (c) 2026 Beijing Volcano Engine Technology Co., Ltd. +# SPDX-License-Identifier: Apache-2.0 +"""Tests for short-format URI parsing support. + +Verifies that both VikingURI and VikingFS._uri_to_path correctly handle +short-format paths (e.g., '/resources') in addition to full viking:// URIs. + +Related issue: https://github.com/volcengine/OpenViking/issues/259 +""" + +import pytest + +from openviking_cli.utils.uri import VikingURI + + +class TestVikingURIShortFormat: + """Test VikingURI accepts and normalizes short-format paths.""" + + def test_slash_resources(self): + uri = VikingURI("/resources") + assert uri.scope == "resources" + assert uri.uri == "viking://resources" + + def test_bare_resources(self): + uri = VikingURI("resources") + assert uri.scope == "resources" + assert uri.uri == "viking://resources" + + def test_slash_user_memories(self): + uri = VikingURI("/user/memories") + assert uri.scope == "user" + assert uri.full_path == "user/memories" + + def test_bare_agent_skills(self): + uri = VikingURI("agent/skills") + assert uri.scope == "agent" + assert uri.full_path == "agent/skills" + + def test_full_uri_unchanged(self): + uri = VikingURI("viking://resources/my_project") + assert uri.uri == "viking://resources/my_project" + assert uri.scope == "resources" + + def test_invalid_scope_still_raises(self): + with pytest.raises(ValueError, match="Invalid scope"): + VikingURI("/invalid_scope/path") + + def test_normalize_static_method(self): + assert VikingURI.normalize("/resources") == "viking://resources" + assert VikingURI.normalize("resources") == "viking://resources" + assert VikingURI.normalize("viking://resources") == "viking://resources" + assert VikingURI.normalize("/user/memories") == "viking://user/memories" + + +class TestUriToPathShortFormat: + """Test VikingFS._uri_to_path handles short-format paths.""" + + @pytest.fixture + def viking_fs(self): + """Create a minimal VikingFS-like object for testing _uri_to_path.""" + from openviking.storage.viking_fs import VikingFS + + class MinimalFS(VikingFS): + def __init__(self): + # Skip parent __init__ to avoid needing AGFS + pass + + return MinimalFS() + + def test_full_uri(self, viking_fs): + assert viking_fs._uri_to_path("viking://resources") == "/local/resources" + + def test_full_uri_nested(self, viking_fs): + assert viking_fs._uri_to_path("viking://user/memories/preferences") == "/local/user/memories/preferences" + + def test_slash_resources(self, viking_fs): + """The original bug: /resources was converted to /local/s instead of /local/resources.""" + assert viking_fs._uri_to_path("/resources") == "/local/resources" + + def test_slash_user_memories(self, viking_fs): + assert viking_fs._uri_to_path("/user/memories") == "/local/user/memories" + + def test_bare_path(self, viking_fs): + assert viking_fs._uri_to_path("resources/images") == "/local/resources/images" + + def test_root_uri(self, viking_fs): + assert viking_fs._uri_to_path("viking://") == "/local" + + def test_slash_only(self, viking_fs): + assert viking_fs._uri_to_path("/") == "/local" + + def test_empty_string(self, viking_fs): + assert viking_fs._uri_to_path("") == "/local"