From 5a43ee09e8388131ae7c74806375c958b0ceaab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=BD=98=E6=B0=B8=E6=98=8E?= <206003912@nbu.edu.cn> Date: Wed, 25 Feb 2026 20:59:12 +0800 Subject: [PATCH 1/3] fix(mcp): reject boolean values in integer parameters - Update _optional_int to exclude bool type (isinstance bool is int subclass) - Add parametrized tests for limit/offset with true/false values - Add MCP server module and CLI mcp command - Update README with MCP server documentation --- README.md | 49 +++++ README_CN.md | 49 +++++ openviking/mcp/__init__.py | 13 ++ openviking/mcp/server.py | 98 +++++++++ openviking/mcp/tools.py | 263 ++++++++++++++++++++++++ openviking_cli/cli/commands/__init__.py | 2 + openviking_cli/cli/commands/mcp.py | 44 ++++ pyproject.toml | 3 + tests/cli/conftest.py | 40 +++- tests/cli/test_cli.py | 29 ++- tests/mcp/test_server.py | 45 ++++ tests/mcp/test_tools.py | 140 +++++++++++++ 12 files changed, 770 insertions(+), 5 deletions(-) create mode 100644 openviking/mcp/__init__.py create mode 100644 openviking/mcp/server.py create mode 100644 openviking/mcp/tools.py create mode 100644 openviking_cli/cli/commands/mcp.py create mode 100644 tests/mcp/test_server.py create mode 100644 tests/mcp/test_tools.py diff --git a/README.md b/README.md index 9ef267d0..3ec26ac5 100644 --- a/README.md +++ b/README.md @@ -416,6 +416,55 @@ Congratulations! You have successfully run OpenViking 🎉 --- +### 5. MCP Server (MVP) + +OpenViking provides an embedded MCP server for local agent integration. + +Install MCP dependency: + +```bash +pip install "openviking[mcp]" +``` + +Start MCP server (stdio transport): + +```bash +openviking mcp --path ./data +``` + +Current MVP scope: +- Transport: `stdio` only +- Runtime mode: embedded local path (`--path`) +- Tools: `openviking_find`, `openviking_read`, `openviking_ls`, `openviking_abstract`, `openviking_overview` + +OpenCode example configuration: + +```json +{ + "mcp": { + "openviking": { + "command": "openviking", + "args": ["mcp", "--path", "./data"] + } + } +} +``` + +Codex example configuration: + +```json +{ + "mcpServers": { + "openviking": { + "command": "openviking", + "args": ["mcp", "--path", "./data"] + } + } +} +``` + +--- + ## Server Deployment For production environments, we recommend running OpenViking as a standalone HTTP service to provide persistent, high-performance context support for your AI Agents. diff --git a/README_CN.md b/README_CN.md index 3c55e6bb..2c706330 100644 --- a/README_CN.md +++ b/README_CN.md @@ -292,6 +292,55 @@ Search results: --- +### 5. MCP Server（MVP） + +OpenViking 提供了可嵌入的 MCP Server，便于本地 Agent 工具接入。 + +安装 MCP 依赖： + +```bash +pip install "openviking[mcp]" +``` + +启动 MCP Server（stdio 传输）： + +```bash +openviking mcp --path ./data +``` + +当前 MVP 范围： +- 传输协议：仅 `stdio` +- 运行模式：仅本地嵌入 `--path` +- 工具集合：`openviking_find`、`openviking_read`、`openviking_ls`、`openviking_abstract`、`openviking_overview` + +OpenCode 配置示例： + +```json +{ + "mcp": { + "openviking": { + "command": "openviking", + "args": ["mcp", "--path", "./data"] + } + } +} +``` + +Codex 配置示例： + +```json +{ + "mcpServers": { + "openviking": { + "command": "openviking", + "args": ["mcp", "--path", "./data"] + } + } +} +``` + +--- + ## 服务端部署 在生产环境中，我们推荐将 OpenViking 作为独立 HTTP 服务运行，以便为您的 AI Agent 提供持久化、高性能的上下文支持。 diff --git a/openviking/mcp/__init__.py b/openviking/mcp/__init__.py new file mode 100644 index 00000000..f51435bc --- /dev/null +++ b/openviking/mcp/__init__.py @@ -0,0 +1,13 @@ +# Copyright (c) 2026 Beijing Volcano Engine Technology Co., Ltd. +# SPDX-License-Identifier: Apache-2.0 +"""MCP integration for OpenViking.""" + +from openviking.mcp.server import OpenVikingMCPAdapter, run_stdio_server +from openviking.mcp.tools import TOOL_DEFINITIONS, dispatch_tool + +__all__ = [ + "TOOL_DEFINITIONS", + "dispatch_tool", + "OpenVikingMCPAdapter", + "run_stdio_server", +] diff --git a/openviking/mcp/server.py b/openviking/mcp/server.py new file mode 100644 index 00000000..81906433 --- /dev/null +++ b/openviking/mcp/server.py @@ -0,0 +1,98 @@ +# Copyright (c) 2026 Beijing Volcano Engine Technology Co., Ltd. +# SPDX-License-Identifier: Apache-2.0 +"""OpenViking MCP server runtime.""" + +from __future__ import annotations + +import os +from typing import Any, Dict, List, Optional + +from openviking_cli.utils.logger import get_logger + +from .tools import TOOL_DEFINITIONS, dispatch_tool + +logger = get_logger(__name__) + + +class OpenVikingMCPAdapter: + """Thin adapter exposing OpenViking methods to MCP tool handlers.""" + + def __init__(self, client: Any): + self.client = client + + def list_tools(self) -> List[Dict[str, Any]]: + return [dict(tool) for tool in TOOL_DEFINITIONS] + + def call_tool(self, name: str, arguments: Optional[Dict[str, Any]] = None) -> str: + return dispatch_tool(name, arguments or {}, self.client) + + +def run_stdio_server(path: str, config: Optional[str] = None, transport: str = "stdio") -> None: + """Run OpenViking MCP server in stdio mode.""" + if transport != "stdio": + raise ValueError("Only stdio transport is supported in MVP") + if not path: + raise ValueError("path is required") + if config: + os.environ["OPENVIKING_CONFIG_FILE"] = config + + try: + from mcp.server.fastmcp import FastMCP + except ImportError as exc: + raise RuntimeError( + "MCP dependency is missing. Install it with: pip install \"openviking[mcp]\"" + ) from exc + + from openviking.sync_client import SyncOpenViking + + client = SyncOpenViking(path=path) + adapter = OpenVikingMCPAdapter(client) + client.initialize() + logger.info("[MCP] OpenViking client initialized (path=%s)", path) + + mcp = FastMCP("openviking") + + @mcp.tool(description="Semantic search in OpenViking context database.") + def openviking_find( + query: str, + uri: str = "", + limit: int = 10, + threshold: float | None = None, + ) -> str: + return adapter.call_tool( + "openviking_find", + { + "query": query, + "uri": uri, + "limit": limit, + "threshold": threshold, + }, + ) + + @mcp.tool(description="Read content from OpenViking (L2).") + def openviking_read(uri: str, offset: int = 0, limit: int = 200) -> str: + return adapter.call_tool( + "openviking_read", + {"uri": uri, "offset": offset, "limit": limit}, + ) + + @mcp.tool(description="List directory contents in OpenViking.") + def openviking_ls(uri: str = "viking://", simple: bool = False, recursive: bool = False) -> str: + return adapter.call_tool( + "openviking_ls", + {"uri": uri, "simple": simple, "recursive": recursive}, + ) + + @mcp.tool(description="Read L0 abstract (.abstract.md) for a directory URI.") + def openviking_abstract(uri: str) -> str: + return adapter.call_tool("openviking_abstract", {"uri": uri}) + + @mcp.tool(description="Read L1 overview (.overview.md) for a directory URI.") + def openviking_overview(uri: str) -> str: + return adapter.call_tool("openviking_overview", {"uri": uri}) + + try: + mcp.run(transport="stdio") + finally: + client.close() + logger.info("[MCP] OpenViking client closed") diff --git a/openviking/mcp/tools.py b/openviking/mcp/tools.py new file mode 100644 index 00000000..c71b382d --- /dev/null +++ b/openviking/mcp/tools.py @@ -0,0 +1,263 @@ +# Copyright (c) 2026 Beijing Volcano Engine Technology Co., Ltd. +# SPDX-License-Identifier: Apache-2.0 +"""Tool definitions and dispatcher for OpenViking MCP server.""" + +from __future__ import annotations + +import json +from dataclasses import asdict, is_dataclass +from enum import Enum +from typing import Any, Dict + +MAX_READ_LIMIT = 2000 +DEFAULT_READ_LIMIT = 200 +MAX_FIND_LIMIT = 50 +DEFAULT_FIND_LIMIT = 10 + +TOOL_DEFINITIONS = [ + { + "name": "openviking_find", + "description": "Semantic search in OpenViking context database.", + "inputSchema": { + "type": "object", + "properties": { + "query": {"type": "string", "description": "Search query."}, + "uri": { + "type": "string", + "description": "Target URI scope. Default is global search.", + "default": "", + }, + "limit": { + "type": "integer", + "description": f"Max number of results (1-{MAX_FIND_LIMIT}).", + "default": DEFAULT_FIND_LIMIT, + "minimum": 1, + "maximum": MAX_FIND_LIMIT, + }, + "threshold": { + "type": "number", + "description": "Optional score threshold.", + }, + }, + "required": ["query"], + }, + }, + { + "name": "openviking_read", + "description": "Read content from OpenViking (L2).", + "inputSchema": { + "type": "object", + "properties": { + "uri": {"type": "string", "description": "Resource URI."}, + "offset": { + "type": "integer", + "description": "Starting line number, 0-indexed.", + "default": 0, + "minimum": 0, + }, + "limit": { + "type": "integer", + "description": ( + f"Number of lines to read. " + f"Default {DEFAULT_READ_LIMIT}, max {MAX_READ_LIMIT}." + ), + "default": DEFAULT_READ_LIMIT, + "minimum": 1, + "maximum": MAX_READ_LIMIT, + }, + }, + "required": ["uri"], + }, + }, + { + "name": "openviking_ls", + "description": "List directory contents in OpenViking.", + "inputSchema": { + "type": "object", + "properties": { + "uri": { + "type": "string", + "description": "Directory URI. Default is viking://.", + "default": "viking://", + }, + "simple": { + "type": "boolean", + "description": "Whether to return simple path list.", + "default": False, + }, + "recursive": { + "type": "boolean", + "description": "Whether to list subdirectories recursively.", + "default": False, + }, + }, + "required": [], + }, + }, + { + "name": "openviking_abstract", + "description": "Read L0 abstract (.abstract.md) for a directory URI.", + "inputSchema": { + "type": "object", + "properties": {"uri": {"type": "string", "description": "Directory URI."}}, + "required": ["uri"], + }, + }, + { + "name": "openviking_overview", + "description": "Read L1 overview (.overview.md) for a directory URI.", + "inputSchema": { + "type": "object", + "properties": {"uri": {"type": "string", "description": "Directory URI."}}, + "required": ["uri"], + }, + }, +] + + +class ToolArgumentError(ValueError): + """Raised when MCP tool arguments are invalid.""" + + +def _to_jsonable(value: Any) -> Any: + """Convert values into JSON-serializable structures.""" + if value is None or isinstance(value, (str, int, float, bool)): + return value + if isinstance(value, Enum): + return value.value + if isinstance(value, dict): + return {str(k): _to_jsonable(v) for k, v in value.items()} + if isinstance(value, (list, tuple, set)): + return [_to_jsonable(item) for item in value] + if hasattr(value, "to_dict") and callable(value.to_dict): + return _to_jsonable(value.to_dict()) + if hasattr(value, "model_dump") and callable(value.model_dump): + return _to_jsonable(value.model_dump()) + if is_dataclass(value): + return _to_jsonable(asdict(value)) + if hasattr(value, "__dict__"): + data = {k: v for k, v in vars(value).items() if not k.startswith("_")} + return _to_jsonable(data) + return str(value) + + +def _json_ok(result: Any) -> str: + return json.dumps({"ok": True, "result": _to_jsonable(result)}, ensure_ascii=False) + + +def _json_error(code: str, message: str, details: Dict[str, Any] | None = None) -> str: + payload: Dict[str, Any] = {"ok": False, "error": {"code": code, "message": message}} + if details: + payload["error"]["details"] = _to_jsonable(details) + return json.dumps(payload, ensure_ascii=False) + + +def _expect_dict(arguments: Any) -> Dict[str, Any]: + if arguments is None: + return {} + if not isinstance(arguments, dict): + raise ToolArgumentError("arguments must be a JSON object") + return arguments + + +def _require_str(arguments: Dict[str, Any], key: str) -> str: + value = arguments.get(key) + if not isinstance(value, str) or not value.strip(): + raise ToolArgumentError(f"'{key}' must be a non-empty string") + return value + + +def _optional_str(arguments: Dict[str, Any], key: str, default: str) -> str: + value = arguments.get(key, default) + if value is None: + return default + if not isinstance(value, str): + raise ToolArgumentError(f"'{key}' must be a string") + return value + + +def _optional_int(arguments: Dict[str, Any], key: str, default: int) -> int: + value = arguments.get(key, default) + if not isinstance(value, int) or isinstance(value, bool): + raise ToolArgumentError(f"'{key}' must be an integer") + return value + + +def _optional_bool(arguments: Dict[str, Any], key: str, default: bool) -> bool: + value = arguments.get(key, default) + if not isinstance(value, bool): + raise ToolArgumentError(f"'{key}' must be a boolean") + return value + + +def _optional_float(arguments: Dict[str, Any], key: str) -> float | None: + value = arguments.get(key) + if value is None: + return None + if not isinstance(value, (int, float)): + raise ToolArgumentError(f"'{key}' must be a number") + return float(value) + + +def dispatch_tool(name: str, arguments: Any, client: Any) -> str: + """Dispatch an MCP tool call and return a JSON payload string.""" + try: + args = _expect_dict(arguments) + + if name == "openviking_find": + query = _require_str(args, "query") + target_uri = _optional_str(args, "uri", "") + limit = _optional_int(args, "limit", DEFAULT_FIND_LIMIT) + if limit < 1 or limit > MAX_FIND_LIMIT: + raise ToolArgumentError( + f"'limit' must be between 1 and {MAX_FIND_LIMIT} for openviking_find" + ) + threshold = _optional_float(args, "threshold") + result = client.find( + query=query, + target_uri=target_uri, + limit=limit, + score_threshold=threshold, + ) + return _json_ok(result) + + if name == "openviking_read": + uri = _require_str(args, "uri") + offset = _optional_int(args, "offset", 0) + if offset < 0: + raise ToolArgumentError("'offset' must be >= 0") + limit = _optional_int(args, "limit", DEFAULT_READ_LIMIT) + if limit < 1 or limit > MAX_READ_LIMIT: + raise ToolArgumentError( + f"'limit' must be between 1 and {MAX_READ_LIMIT} for openviking_read" + ) + result = client.read(uri=uri, offset=offset, limit=limit) + return _json_ok(result) + + if name == "openviking_ls": + uri = _optional_str(args, "uri", "viking://") + simple = _optional_bool(args, "simple", False) + recursive = _optional_bool(args, "recursive", False) + result = client.ls(uri=uri, simple=simple, recursive=recursive, output="agent") + return _json_ok(result) + + if name == "openviking_abstract": + uri = _require_str(args, "uri") + result = client.abstract(uri=uri) + return _json_ok(result) + + if name == "openviking_overview": + uri = _require_str(args, "uri") + result = client.overview(uri=uri) + return _json_ok(result) + + return _json_error("TOOL_NOT_FOUND", f"Unknown tool: {name}") + + except ToolArgumentError as exc: + return _json_error("INVALID_ARGUMENT", str(exc)) + except Exception as exc: # noqa: BLE001 + return _json_error( + "INTERNAL", + "Tool execution failed", + details={"tool": name, "exception": type(exc).__name__, "message": str(exc)}, + ) diff --git a/openviking_cli/cli/commands/__init__.py b/openviking_cli/cli/commands/__init__.py index c6dfc638..1a71a555 100644 --- a/openviking_cli/cli/commands/__init__.py +++ b/openviking_cli/cli/commands/__init__.py @@ -9,6 +9,7 @@ content, debug, filesystem, + mcp, observer, pack, relations, @@ -23,6 +24,7 @@ def register_commands(app: typer.Typer) -> None: """Register all supported commands into the root CLI app.""" serve.register(app) + mcp.register(app) admin.register(app) resources.register(app) filesystem.register(app) diff --git a/openviking_cli/cli/commands/mcp.py b/openviking_cli/cli/commands/mcp.py new file mode 100644 index 00000000..5b399d4b --- /dev/null +++ b/openviking_cli/cli/commands/mcp.py @@ -0,0 +1,44 @@ +# Copyright (c) 2026 Beijing Volcano Engine Technology Co., Ltd. +# SPDX-License-Identifier: Apache-2.0 +"""`mcp` command implementation.""" + +from typing import Optional + +import typer + +from openviking_cli.cli.context import get_cli_context +from openviking_cli.cli.errors import handle_command_error + + +def register(app: typer.Typer) -> None: + """Register `mcp` command.""" + + @app.command("mcp") + def mcp_command( + ctx: typer.Context, + path: str = typer.Option( + ..., + "--path", + help="Local workspace path for embedded OpenViking storage", + ), + config: Optional[str] = typer.Option( + None, + "--config", + help="Path to ov.conf config file", + ), + transport: str = typer.Option( + "stdio", + "--transport", + help="MCP transport mode. MVP supports only 'stdio'.", + ), + ) -> None: + """Run OpenViking MCP server (stdio, embedded mode).""" + cli_ctx = get_cli_context(ctx) + try: + if transport != "stdio": + raise ValueError("Only stdio transport is supported in MVP") + from openviking.mcp.server import run_stdio_server + + run_stdio_server(path=path, config=config, transport=transport) + except Exception as exc: # noqa: BLE001 + handle_command_error(cli_ctx, exc) diff --git a/pyproject.toml b/pyproject.toml index ac30a11f..1af556a1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -81,6 +81,9 @@ eval = [ "datasets>=2.0.0", "pandas>=2.0.0", ] +mcp = [ + "mcp", +] [project.urls] Homepage = "https://github.com/volcengine/openviking" diff --git a/tests/cli/conftest.py b/tests/cli/conftest.py index a28f9968..7676dbf8 100644 --- a/tests/cli/conftest.py +++ b/tests/cli/conftest.py @@ -14,6 +14,17 @@ import httpx import pytest +LOCAL_NO_PROXY_ENV = { + "NO_PROXY": "127.0.0.1,localhost", + "no_proxy": "127.0.0.1,localhost", + "HTTP_PROXY": "", + "HTTPS_PROXY": "", + "http_proxy": "", + "https_proxy": "", + "ALL_PROXY": "", + "all_proxy": "", +} + def _get_free_port() -> int: """Reserve a free port for the test server.""" @@ -30,7 +41,7 @@ def _wait_for_health(url: str, timeout_s: float = 20.0) -> None: last_error = None while time.time() < deadline: try: - response = httpx.get(f"{url}/health", timeout=1.0) + response = httpx.get(f"{url}/health", timeout=1.0, trust_env=False) if response.status_code == 200: return except Exception as exc: # noqa: BLE001 @@ -39,15 +50,37 @@ def _wait_for_health(url: str, timeout_s: float = 20.0) -> None: raise RuntimeError(f"OpenViking server failed to start: {last_error}") +def _resolve_base_conf_path() -> Path: + """Resolve base example config path with backward-compatible fallback.""" + candidates = (Path("examples/ov.conf"), Path("examples/ov.conf.example")) + for candidate in candidates: + if candidate.exists(): + return candidate.resolve() + raise FileNotFoundError( + "No base config found, expected one of: " + + ", ".join(str(path) for path in candidates) + ) + + +def _ensure_agfs_binary_available() -> None: + """Skip integration tests when AGFS local binary is unavailable.""" + binary_name = "agfs-server.exe" if os.name == "nt" else "agfs-server" + binary_path = (Path("openviking") / "bin" / binary_name).resolve() + if not binary_path.exists(): + pytest.skip(f"AGFS binary not found: {binary_path}") + + @pytest.fixture(scope="session") def openviking_server(tmp_path_factory: pytest.TempPathFactory) -> Generator[str, None, None]: """Start a real OpenViking server for CLI tests.""" + _ensure_agfs_binary_available() + storage_dir = tmp_path_factory.mktemp("openviking_cli_data") port = _get_free_port() # Load the base example config and override storage path + server port - base_conf_path = Path("examples/ov.conf").resolve() - with open(base_conf_path) as f: + base_conf_path = _resolve_base_conf_path() + with open(base_conf_path, encoding="utf-8") as f: conf_data = json.load(f) conf_data.setdefault("server", {}) @@ -69,6 +102,7 @@ def openviking_server(tmp_path_factory: pytest.TempPathFactory) -> Generator[str env = os.environ.copy() env["OPENVIKING_CONFIG_FILE"] = str(tmp_conf) + env.update(LOCAL_NO_PROXY_ENV) cmd = [ sys.executable, diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 8edf7b48..4f7c0188 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -12,6 +12,17 @@ runner = CliRunner() +LOCAL_NO_PROXY_ENV = { + "NO_PROXY": "127.0.0.1,localhost", + "no_proxy": "127.0.0.1,localhost", + "HTTP_PROXY": "", + "HTTPS_PROXY": "", + "http_proxy": "", + "https_proxy": "", + "ALL_PROXY": "", + "all_proxy": "", +} + def _make_ovcli_conf(url: str, tmp_dir: str) -> str: """Create a temporary ovcli.conf and return its path.""" @@ -36,7 +47,10 @@ def _run_cli(args, server_url, env=None, expected_exit_code=0): """ with tempfile.TemporaryDirectory() as tmp_dir: conf_path = _make_ovcli_conf(server_url, tmp_dir) - merged_env = {"OPENVIKING_CLI_CONFIG_FILE": conf_path} + merged_env = { + "OPENVIKING_CLI_CONFIG_FILE": conf_path, + **LOCAL_NO_PROXY_ENV, + } if env: merged_env.update(env) result = runner.invoke(app, ["-o", "json", *args], env=merged_env) @@ -106,6 +120,8 @@ def test_cli_help_smoke(): ["search", "--help"], ["grep", "--help"], ["glob", "--help"], + # mcp + ["mcp", "--help"], # serve ["serve", "--help"], # system @@ -138,7 +154,10 @@ def test_cli_connection_refused(): result = runner.invoke( app, ["-o", "json", "health"], - env={"OPENVIKING_CLI_CONFIG_FILE": conf_path}, + env={ + "OPENVIKING_CLI_CONFIG_FILE": conf_path, + **LOCAL_NO_PROXY_ENV, + }, ) assert result.exit_code == 3 payload = json.loads(result.output) @@ -241,3 +260,9 @@ def test_cli_nonexistent_resource(openviking_server): expected_exit_code=1, ) assert result.exit_code == 1 + + +def test_cli_mcp_requires_path(): + result = runner.invoke(app, ["mcp"], env={}) + assert result.exit_code != 0 + assert "--path" in result.output diff --git a/tests/mcp/test_server.py b/tests/mcp/test_server.py new file mode 100644 index 00000000..216ce004 --- /dev/null +++ b/tests/mcp/test_server.py @@ -0,0 +1,45 @@ +# Copyright (c) 2026 Beijing Volcano Engine Technology Co., Ltd. +# SPDX-License-Identifier: Apache-2.0 +"""Tests for OpenViking MCP server adapters.""" + +import json + +from openviking.mcp.server import OpenVikingMCPAdapter + + +class _StubClient: + def find(self, query, target_uri="", limit=10, score_threshold=None): + return {"memories": [], "resources": [], "skills": [], "total": 0} + + def read(self, uri, offset=0, limit=-1): + return "hello" + + def ls(self, uri="viking://", simple=False, recursive=False, output="agent"): + return [{"uri": "viking://resources"}] + + def abstract(self, uri): + return "abs" + + def overview(self, uri): + return "ov" + + +def test_list_tools_returns_mvp_definitions(): + adapter = OpenVikingMCPAdapter(_StubClient()) + tools = adapter.list_tools() + names = {tool["name"] for tool in tools} + + assert "openviking_find" in names + assert "openviking_read" in names + assert "openviking_ls" in names + assert "openviking_abstract" in names + assert "openviking_overview" in names + + +def test_call_tool_returns_json_text_payload(): + adapter = OpenVikingMCPAdapter(_StubClient()) + payload = adapter.call_tool("openviking_read", {"uri": "viking://resources/readme.md"}) + body = json.loads(payload) + + assert body["ok"] is True + assert body["result"] == "hello" diff --git a/tests/mcp/test_tools.py b/tests/mcp/test_tools.py new file mode 100644 index 00000000..0a8c496f --- /dev/null +++ b/tests/mcp/test_tools.py @@ -0,0 +1,140 @@ +# Copyright (c) 2026 Beijing Volcano Engine Technology Co., Ltd. +# SPDX-License-Identifier: Apache-2.0 +"""Tests for OpenViking MCP tool definitions and dispatch.""" + +import json + +import pytest + +from openviking.mcp.tools import TOOL_DEFINITIONS, dispatch_tool + + +class _FakeClient: + def __init__(self): + self.calls = [] + + def find(self, query, target_uri="", limit=10, score_threshold=None): + self.calls.append(("find", query, target_uri, limit, score_threshold)) + return {"memories": [], "resources": [], "skills": [], "total": 0} + + def read(self, uri, offset=0, limit=-1): + self.calls.append(("read", uri, offset, limit)) + return "hello" + + def ls(self, uri="viking://", simple=False, recursive=False, output="agent"): + self.calls.append(("ls", uri, simple, recursive, output)) + return [{"uri": "viking://resources"}] + + def abstract(self, uri): + self.calls.append(("abstract", uri)) + return "abs" + + def overview(self, uri): + self.calls.append(("overview", uri)) + return "ov" + + +def _tool_names(): + return {tool["name"] for tool in TOOL_DEFINITIONS} + + +def test_tool_definitions_have_minimum_mvp_set(): + names = _tool_names() + assert "openviking_find" in names + assert "openviking_read" in names + assert "openviking_ls" in names + assert "openviking_abstract" in names + assert "openviking_overview" in names + + +def test_dispatch_find_success(): + client = _FakeClient() + payload = dispatch_tool("openviking_find", {"query": "what is openviking"}, client) + body = json.loads(payload) + + assert body["ok"] is True + assert body["result"]["total"] == 0 + assert client.calls[0][0] == "find" + + +def test_dispatch_read_applies_default_limit_cap(): + client = _FakeClient() + dispatch_tool("openviking_read", {"uri": "viking://resources/a.md"}, client) + assert client.calls[0] == ("read", "viking://resources/a.md", 0, 200) + + +def test_dispatch_read_rejects_oversized_limit(): + client = _FakeClient() + payload = dispatch_tool("openviking_read", {"uri": "viking://resources/a.md", "limit": 5000}, client) + body = json.loads(payload) + + assert body["ok"] is False + assert body["error"]["code"] == "INVALID_ARGUMENT" + + +@pytest.mark.parametrize( + "args,key", + [ + ({"uri": "viking://resources/a.md", "limit": True}, "limit"), + ({"uri": "viking://resources/a.md", "offset": False}, "offset"), + ], +) +def test_dispatch_read_rejects_boolean_integer_fields(args, key): + client = _FakeClient() + payload = dispatch_tool("openviking_read", args, client) + body = json.loads(payload) + + assert body["ok"] is False + assert body["error"]["code"] == "INVALID_ARGUMENT" + assert body["error"]["message"] == f"'{key}' must be an integer" + + +def test_dispatch_find_requires_query(): + client = _FakeClient() + payload = dispatch_tool("openviking_find", {}, client) + body = json.loads(payload) + + assert body["ok"] is False + assert body["error"]["code"] == "INVALID_ARGUMENT" + + +def test_dispatch_unknown_tool_returns_error(): + client = _FakeClient() + payload = dispatch_tool("no_such_tool", {}, client) + body = json.loads(payload) + + assert body["ok"] is False + assert body["error"]["code"] == "TOOL_NOT_FOUND" + + +def test_dispatch_client_exception_is_wrapped(): + class _BrokenClient: + def read(self, *args, **kwargs): + raise RuntimeError("boom") + + payload = dispatch_tool("openviking_read", {"uri": "viking://resources/a.md"}, _BrokenClient()) + body = json.loads(payload) + + assert body["ok"] is False + assert body["error"]["code"] == "INTERNAL" + + +@pytest.mark.parametrize( + "tool_name,args,expected_call", + [ + ( + "openviking_ls", + {"uri": "viking://resources", "simple": True, "recursive": True}, + ("ls", "viking://resources", True, True, "agent"), + ), + ("openviking_abstract", {"uri": "viking://resources"}, ("abstract", "viking://resources")), + ("openviking_overview", {"uri": "viking://resources"}, ("overview", "viking://resources")), + ], +) +def test_dispatch_other_mvp_tools(tool_name, args, expected_call): + client = _FakeClient() + payload = dispatch_tool(tool_name, args, client) + body = json.loads(payload) + + assert body["ok"] is True + assert client.calls[0] == expected_call From 14dcb3251068925c965c90a8edececb15a9b889a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=BD=98=E6=B0=B8=E6=98=8E?= <206003912@nbu.edu.cn> Date: Wed, 25 Feb 2026 22:01:28 +0800 Subject: [PATCH 2/3] feat(mcp): extend V1 with additional tools and read/write gating --- README.md | 46 ++++- README_CN.md | 46 ++++- openviking/mcp/__init__.py | 3 +- openviking/mcp/server.py | 108 ++++++++++- openviking/mcp/tools.py | 299 ++++++++++++++++++++++++++++- openviking_cli/cli/commands/mcp.py | 14 +- tests/cli/test_cli.py | 17 ++ tests/mcp/test_protocol_stdio.py | 76 ++++++++ tests/mcp/test_server.py | 42 +++- tests/mcp/test_tools.py | 227 ++++++++++++++++++---- 10 files changed, 819 insertions(+), 59 deletions(-) create mode 100644 tests/mcp/test_protocol_stdio.py diff --git a/README.md b/README.md index 3ec26ac5..43504a85 100644 --- a/README.md +++ b/README.md @@ -416,7 +416,7 @@ Congratulations! You have successfully run OpenViking 🎉 --- -### 5. MCP Server (MVP) +### 5. MCP Server (V1) OpenViking provides an embedded MCP server for local agent integration. @@ -426,16 +426,24 @@ Install MCP dependency: pip install "openviking[mcp]" ``` -Start MCP server (stdio transport): +Start MCP server (stdio transport, readonly by default): ```bash openviking mcp --path ./data ``` -Current MVP scope: +Enable write tools explicitly when needed: + +```bash +openviking mcp --path ./data --enable-write +``` + +Current V1 scope: - Transport: `stdio` only - Runtime mode: embedded local path (`--path`) -- Tools: `openviking_find`, `openviking_read`, `openviking_ls`, `openviking_abstract`, `openviking_overview` +- Default mode: readonly (write tools disabled unless `--enable-write` is set) +- Read tools: `openviking_find`, `openviking_search`, `openviking_read`, `openviking_ls`, `openviking_abstract`, `openviking_overview`, `openviking_wait_processed`, `openviking_stat`, `openviking_tree`, `openviking_grep`, `openviking_glob`, `openviking_status`, `openviking_health` +- Write tools (optional): `openviking_add_resource` OpenCode example configuration: @@ -450,6 +458,19 @@ OpenCode example configuration: } ``` +OpenCode writable configuration: + +```json +{ + "mcp": { + "openviking": { + "command": "openviking", + "args": ["mcp", "--path", "./data", "--enable-write"] + } + } +} +``` + Codex example configuration: ```json @@ -463,6 +484,23 @@ Codex example configuration: } ``` +Codex writable configuration: + +```json +{ + "mcpServers": { + "openviking": { + "command": "openviking", + "args": ["mcp", "--path", "./data", "--enable-write"] + } + } +} +``` + +Production recommendation: +- Prefer readonly MCP instances by default. +- Run writable instances separately with explicit access control. + --- ## Server Deployment diff --git a/README_CN.md b/README_CN.md index 2c706330..de913fb4 100644 --- a/README_CN.md +++ b/README_CN.md @@ -292,7 +292,7 @@ Search results: --- -### 5. MCP Server（MVP） +### 5. MCP Server（V1） OpenViking 提供了可嵌入的 MCP Server，便于本地 Agent 工具接入。 @@ -302,16 +302,24 @@ OpenViking 提供了可嵌入的 MCP Server，便于本地 Agent 工具接入。 pip install "openviking[mcp]" ``` -启动 MCP Server（stdio 传输）： +启动 MCP Server（stdio 传输，默认只读）： ```bash openviking mcp --path ./data ``` -当前 MVP 范围： +需要写入能力时，显式开启写模式： + +```bash +openviking mcp --path ./data --enable-write +``` + +当前 V1 范围： - 传输协议：仅 `stdio` - 运行模式：仅本地嵌入 `--path` -- 工具集合：`openviking_find`、`openviking_read`、`openviking_ls`、`openviking_abstract`、`openviking_overview` +- 默认模式：只读（除非设置 `--enable-write`，否则不暴露写工具） +- 读取工具：`openviking_find`、`openviking_search`、`openviking_read`、`openviking_ls`、`openviking_abstract`、`openviking_overview`、`openviking_wait_processed`、`openviking_stat`、`openviking_tree`、`openviking_grep`、`openviking_glob`、`openviking_status`、`openviking_health` +- 写入工具（可选）：`openviking_add_resource` OpenCode 配置示例： @@ -326,6 +334,19 @@ OpenCode 配置示例： } ``` +OpenCode 可写配置示例： + +```json +{ + "mcp": { + "openviking": { + "command": "openviking", + "args": ["mcp", "--path", "./data", "--enable-write"] + } + } +} +``` + Codex 配置示例： ```json @@ -339,6 +360,23 @@ Codex 配置示例： } ``` +Codex 可写配置示例： + +```json +{ + "mcpServers": { + "openviking": { + "command": "openviking", + "args": ["mcp", "--path", "./data", "--enable-write"] + } + } +} +``` + +生产环境建议： +- 默认使用只读 MCP 实例。 +- 写入实例建议独立部署并做明确访问控制。 + --- ## 服务端部署 diff --git a/openviking/mcp/__init__.py b/openviking/mcp/__init__.py index f51435bc..a460d08e 100644 --- a/openviking/mcp/__init__.py +++ b/openviking/mcp/__init__.py @@ -3,10 +3,11 @@ """MCP integration for OpenViking.""" from openviking.mcp.server import OpenVikingMCPAdapter, run_stdio_server -from openviking.mcp.tools import TOOL_DEFINITIONS, dispatch_tool +from openviking.mcp.tools import TOOL_DEFINITIONS, dispatch_tool, get_tool_definitions __all__ = [ "TOOL_DEFINITIONS", + "get_tool_definitions", "dispatch_tool", "OpenVikingMCPAdapter", "run_stdio_server", diff --git a/openviking/mcp/server.py b/openviking/mcp/server.py index 81906433..fc5d9545 100644 --- a/openviking/mcp/server.py +++ b/openviking/mcp/server.py @@ -9,7 +9,7 @@ from openviking_cli.utils.logger import get_logger -from .tools import TOOL_DEFINITIONS, dispatch_tool +from .tools import dispatch_tool, get_tool_definitions logger = get_logger(__name__) @@ -17,20 +17,26 @@ class OpenVikingMCPAdapter: """Thin adapter exposing OpenViking methods to MCP tool handlers.""" - def __init__(self, client: Any): + def __init__(self, client: Any, allow_write: bool = False): self.client = client + self.allow_write = allow_write def list_tools(self) -> List[Dict[str, Any]]: - return [dict(tool) for tool in TOOL_DEFINITIONS] + return get_tool_definitions(include_write=self.allow_write) def call_tool(self, name: str, arguments: Optional[Dict[str, Any]] = None) -> str: - return dispatch_tool(name, arguments or {}, self.client) + return dispatch_tool(name, arguments or {}, self.client, allow_write=self.allow_write) -def run_stdio_server(path: str, config: Optional[str] = None, transport: str = "stdio") -> None: +def run_stdio_server( + path: str, + config: Optional[str] = None, + transport: str = "stdio", + enable_write: bool = False, +) -> None: """Run OpenViking MCP server in stdio mode.""" if transport != "stdio": - raise ValueError("Only stdio transport is supported in MVP") + raise ValueError("Only stdio transport is supported in V1") if not path: raise ValueError("path is required") if config: @@ -46,9 +52,9 @@ def run_stdio_server(path: str, config: Optional[str] = None, transport: str = " from openviking.sync_client import SyncOpenViking client = SyncOpenViking(path=path) - adapter = OpenVikingMCPAdapter(client) + adapter = OpenVikingMCPAdapter(client, allow_write=enable_write) client.initialize() - logger.info("[MCP] OpenViking client initialized (path=%s)", path) + logger.info("[MCP] OpenViking client initialized (path=%s, enable_write=%s)", path, enable_write) mcp = FastMCP("openviking") @@ -69,6 +75,25 @@ def openviking_find( }, ) + @mcp.tool(description="Context-aware retrieval in OpenViking.") + def openviking_search( + query: str, + uri: str = "", + session_id: str | None = None, + limit: int = 10, + threshold: float | None = None, + ) -> str: + return adapter.call_tool( + "openviking_search", + { + "query": query, + "uri": uri, + "session_id": session_id, + "limit": limit, + "threshold": threshold, + }, + ) + @mcp.tool(description="Read content from OpenViking (L2).") def openviking_read(uri: str, offset: int = 0, limit: int = 200) -> str: return adapter.call_tool( @@ -91,6 +116,73 @@ def openviking_abstract(uri: str) -> str: def openviking_overview(uri: str) -> str: return adapter.call_tool("openviking_overview", {"uri": uri}) + @mcp.tool(description="Wait until queued async processing completes.") + def openviking_wait_processed(timeout: float | None = None) -> str: + return adapter.call_tool("openviking_wait_processed", {"timeout": timeout}) + + @mcp.tool(description="Get resource metadata and status.") + def openviking_stat(uri: str) -> str: + return adapter.call_tool("openviking_stat", {"uri": uri}) + + @mcp.tool(description="Get directory tree in agent-friendly format.") + def openviking_tree( + uri: str, + abs_limit: int = 128, + show_all_hidden: bool = False, + node_limit: int = 1000, + ) -> str: + return adapter.call_tool( + "openviking_tree", + { + "uri": uri, + "abs_limit": abs_limit, + "show_all_hidden": show_all_hidden, + "node_limit": node_limit, + }, + ) + + @mcp.tool(description="Search text pattern in files under a URI.") + def openviking_grep(pattern: str, uri: str = "viking://", ignore_case: bool = False) -> str: + return adapter.call_tool( + "openviking_grep", + {"pattern": pattern, "uri": uri, "ignore_case": ignore_case}, + ) + + @mcp.tool(description="Search files by glob pattern under a URI.") + def openviking_glob(pattern: str, uri: str = "viking://") -> str: + return adapter.call_tool("openviking_glob", {"pattern": pattern, "uri": uri}) + + @mcp.tool(description="Get OpenViking component status.") + def openviking_status() -> str: + return adapter.call_tool("openviking_status", {}) + + @mcp.tool(description="Get quick health check result.") + def openviking_health() -> str: + return adapter.call_tool("openviking_health", {}) + + if enable_write: + + @mcp.tool(description="Add local path or URL as resource into OpenViking.") + def openviking_add_resource( + path: str, + to: str | None = None, + reason: str = "", + instruction: str = "", + wait: bool = False, + timeout: float | None = None, + ) -> str: + return adapter.call_tool( + "openviking_add_resource", + { + "path": path, + "to": to, + "reason": reason, + "instruction": instruction, + "wait": wait, + "timeout": timeout, + }, + ) + try: mcp.run(transport="stdio") finally: diff --git a/openviking/mcp/tools.py b/openviking/mcp/tools.py index c71b382d..ecdc6b59 100644 --- a/openviking/mcp/tools.py +++ b/openviking/mcp/tools.py @@ -4,17 +4,22 @@ from __future__ import annotations +import copy import json from dataclasses import asdict, is_dataclass from enum import Enum -from typing import Any, Dict +from typing import Any, Dict, List MAX_READ_LIMIT = 2000 DEFAULT_READ_LIMIT = 200 MAX_FIND_LIMIT = 50 DEFAULT_FIND_LIMIT = 10 +MAX_TREE_ABS_LIMIT = 4096 +DEFAULT_TREE_ABS_LIMIT = 128 +MAX_TREE_NODE_LIMIT = 5000 +DEFAULT_TREE_NODE_LIMIT = 1000 -TOOL_DEFINITIONS = [ +READ_TOOL_DEFINITIONS = [ { "name": "openviking_find", "description": "Semantic search in OpenViking context database.", @@ -42,6 +47,37 @@ "required": ["query"], }, }, + { + "name": "openviking_search", + "description": "Context-aware retrieval in OpenViking.", + "inputSchema": { + "type": "object", + "properties": { + "query": {"type": "string", "description": "Search query."}, + "uri": { + "type": "string", + "description": "Target URI scope. Default is global search.", + "default": "", + }, + "session_id": { + "type": "string", + "description": "Optional session ID for context-aware retrieval.", + }, + "limit": { + "type": "integer", + "description": f"Max number of results (1-{MAX_FIND_LIMIT}).", + "default": DEFAULT_FIND_LIMIT, + "minimum": 1, + "maximum": MAX_FIND_LIMIT, + }, + "threshold": { + "type": "number", + "description": "Optional score threshold.", + }, + }, + "required": ["query"], + }, + }, { "name": "openviking_read", "description": "Read content from OpenViking (L2).", @@ -112,13 +148,161 @@ "required": ["uri"], }, }, + { + "name": "openviking_wait_processed", + "description": "Wait until queued async processing completes.", + "inputSchema": { + "type": "object", + "properties": { + "timeout": { + "type": "number", + "description": "Optional timeout in seconds.", + } + }, + "required": [], + }, + }, + { + "name": "openviking_stat", + "description": "Get resource metadata and status.", + "inputSchema": { + "type": "object", + "properties": {"uri": {"type": "string", "description": "Resource URI."}}, + "required": ["uri"], + }, + }, + { + "name": "openviking_tree", + "description": "Get directory tree in agent-friendly format.", + "inputSchema": { + "type": "object", + "properties": { + "uri": {"type": "string", "description": "Directory URI."}, + "abs_limit": { + "type": "integer", + "description": f"Abstract content limit (0-{MAX_TREE_ABS_LIMIT}).", + "default": DEFAULT_TREE_ABS_LIMIT, + "minimum": 0, + "maximum": MAX_TREE_ABS_LIMIT, + }, + "show_all_hidden": { + "type": "boolean", + "description": "Whether to include all hidden entries.", + "default": False, + }, + "node_limit": { + "type": "integer", + "description": f"Maximum nodes in output (1-{MAX_TREE_NODE_LIMIT}).", + "default": DEFAULT_TREE_NODE_LIMIT, + "minimum": 1, + "maximum": MAX_TREE_NODE_LIMIT, + }, + }, + "required": ["uri"], + }, + }, + { + "name": "openviking_grep", + "description": "Search text pattern in files under a URI.", + "inputSchema": { + "type": "object", + "properties": { + "pattern": {"type": "string", "description": "Search pattern."}, + "uri": { + "type": "string", + "description": "Search root URI.", + "default": "viking://", + }, + "ignore_case": { + "type": "boolean", + "description": "Case-insensitive matching.", + "default": False, + }, + }, + "required": ["pattern"], + }, + }, + { + "name": "openviking_glob", + "description": "Search files by glob pattern under a URI.", + "inputSchema": { + "type": "object", + "properties": { + "pattern": {"type": "string", "description": "Glob pattern."}, + "uri": { + "type": "string", + "description": "Search root URI.", + "default": "viking://", + }, + }, + "required": ["pattern"], + }, + }, + { + "name": "openviking_status", + "description": "Get OpenViking component status.", + "inputSchema": {"type": "object", "properties": {}, "required": []}, + }, + { + "name": "openviking_health", + "description": "Get quick health check result.", + "inputSchema": {"type": "object", "properties": {}, "required": []}, + }, ] +WRITE_TOOL_DEFINITIONS = [ + { + "name": "openviking_add_resource", + "description": "Add local path or URL as resource into OpenViking.", + "inputSchema": { + "type": "object", + "properties": { + "path": { + "type": "string", + "description": "Local path or URL to import.", + }, + "to": { + "type": "string", + "description": "Optional target URI.", + }, + "reason": { + "type": "string", + "description": "Optional import reason.", + "default": "", + }, + "instruction": { + "type": "string", + "description": "Optional additional instruction.", + "default": "", + }, + "wait": { + "type": "boolean", + "description": "Wait until processing completes.", + "default": False, + }, + "timeout": { + "type": "number", + "description": "Optional wait timeout in seconds.", + }, + }, + "required": ["path"], + }, + } +] + +TOOL_DEFINITIONS = READ_TOOL_DEFINITIONS + WRITE_TOOL_DEFINITIONS + class ToolArgumentError(ValueError): """Raised when MCP tool arguments are invalid.""" +def get_tool_definitions(include_write: bool = False) -> List[Dict[str, Any]]: + """Return MCP tool definitions, optionally including write tools.""" + definitions = READ_TOOL_DEFINITIONS + WRITE_TOOL_DEFINITIONS if include_write else READ_TOOL_DEFINITIONS + return [copy.deepcopy(tool) for tool in definitions] + + def _to_jsonable(value: Any) -> Any: """Convert values into JSON-serializable structures.""" if value is None or isinstance(value, (str, int, float, bool)): @@ -176,6 +360,17 @@ def _optional_str(arguments: Dict[str, Any], key: str, default: str) -> str: return value +def _optional_nullable_str(arguments: Dict[str, Any], key: str) -> str | None: + if key not in arguments: + return None + value = arguments.get(key) + if value is None: + return None + if not isinstance(value, str): + raise ToolArgumentError(f"'{key}' must be a string or null") + return value + + def _optional_int(arguments: Dict[str, Any], key: str, default: int) -> int: value = arguments.get(key, default) if not isinstance(value, int) or isinstance(value, bool): @@ -191,15 +386,17 @@ def _optional_bool(arguments: Dict[str, Any], key: str, default: bool) -> bool: def _optional_float(arguments: Dict[str, Any], key: str) -> float | None: + if key not in arguments: + return None value = arguments.get(key) if value is None: return None - if not isinstance(value, (int, float)): + if isinstance(value, bool) or not isinstance(value, (int, float)): raise ToolArgumentError(f"'{key}' must be a number") return float(value) -def dispatch_tool(name: str, arguments: Any, client: Any) -> str: +def dispatch_tool(name: str, arguments: Any, client: Any, allow_write: bool = False) -> str: """Dispatch an MCP tool call and return a JSON payload string.""" try: args = _expect_dict(arguments) @@ -221,6 +418,25 @@ def dispatch_tool(name: str, arguments: Any, client: Any) -> str: ) return _json_ok(result) + if name == "openviking_search": + query = _require_str(args, "query") + target_uri = _optional_str(args, "uri", "") + session_id = _optional_nullable_str(args, "session_id") + limit = _optional_int(args, "limit", DEFAULT_FIND_LIMIT) + if limit < 1 or limit > MAX_FIND_LIMIT: + raise ToolArgumentError( + f"'limit' must be between 1 and {MAX_FIND_LIMIT} for openviking_search" + ) + threshold = _optional_float(args, "threshold") + result = client.search( + query=query, + target_uri=target_uri, + session_id=session_id, + limit=limit, + score_threshold=threshold, + ) + return _json_ok(result) + if name == "openviking_read": uri = _require_str(args, "uri") offset = _optional_int(args, "offset", 0) @@ -251,6 +467,81 @@ def dispatch_tool(name: str, arguments: Any, client: Any) -> str: result = client.overview(uri=uri) return _json_ok(result) + if name == "openviking_wait_processed": + timeout = _optional_float(args, "timeout") + result = client.wait_processed(timeout=timeout) + return _json_ok(result) + + if name == "openviking_stat": + uri = _require_str(args, "uri") + result = client.stat(uri=uri) + return _json_ok(result) + + if name == "openviking_tree": + uri = _require_str(args, "uri") + abs_limit = _optional_int(args, "abs_limit", DEFAULT_TREE_ABS_LIMIT) + if abs_limit < 0 or abs_limit > MAX_TREE_ABS_LIMIT: + raise ToolArgumentError( + f"'abs_limit' must be between 0 and {MAX_TREE_ABS_LIMIT} for openviking_tree" + ) + show_all_hidden = _optional_bool(args, "show_all_hidden", False) + node_limit = _optional_int(args, "node_limit", DEFAULT_TREE_NODE_LIMIT) + if node_limit < 1 or node_limit > MAX_TREE_NODE_LIMIT: + raise ToolArgumentError( + f"'node_limit' must be between 1 and {MAX_TREE_NODE_LIMIT} for openviking_tree" + ) + result = client.tree( + uri=uri, + output="agent", + abs_limit=abs_limit, + show_all_hidden=show_all_hidden, + node_limit=node_limit, + ) + return _json_ok(result) + + if name == "openviking_grep": + pattern = _require_str(args, "pattern") + uri = _optional_str(args, "uri", "viking://") + ignore_case = _optional_bool(args, "ignore_case", False) + result = client.grep(uri=uri, pattern=pattern, case_insensitive=ignore_case) + return _json_ok(result) + + if name == "openviking_glob": + pattern = _require_str(args, "pattern") + uri = _optional_str(args, "uri", "viking://") + result = client.glob(pattern=pattern, uri=uri) + return _json_ok(result) + + if name == "openviking_status": + result = client.get_status() + return _json_ok(result) + + if name == "openviking_health": + healthy = bool(client.is_healthy()) + return _json_ok({"healthy": healthy}) + + if name == "openviking_add_resource": + if not allow_write: + return _json_error( + "PERMISSION_DENIED", + "Tool 'openviking_add_resource' is disabled in readonly mode", + ) + path = _require_str(args, "path") + to = _optional_nullable_str(args, "to") + reason = _optional_str(args, "reason", "") + instruction = _optional_str(args, "instruction", "") + wait = _optional_bool(args, "wait", False) + timeout = _optional_float(args, "timeout") + result = client.add_resource( + path=path, + target=to, + reason=reason, + instruction=instruction, + wait=wait, + timeout=timeout, + ) + return _json_ok(result) + return _json_error("TOOL_NOT_FOUND", f"Unknown tool: {name}") except ToolArgumentError as exc: diff --git a/openviking_cli/cli/commands/mcp.py b/openviking_cli/cli/commands/mcp.py index 5b399d4b..08513889 100644 --- a/openviking_cli/cli/commands/mcp.py +++ b/openviking_cli/cli/commands/mcp.py @@ -31,14 +31,24 @@ def mcp_command( "--transport", help="MCP transport mode. MVP supports only 'stdio'.", ), + enable_write: bool = typer.Option( + False, + "--enable-write/--readonly", + help="Enable write tools (default is readonly mode).", + ), ) -> None: """Run OpenViking MCP server (stdio, embedded mode).""" cli_ctx = get_cli_context(ctx) try: if transport != "stdio": - raise ValueError("Only stdio transport is supported in MVP") + raise ValueError("Only stdio transport is supported in V1") from openviking.mcp.server import run_stdio_server - run_stdio_server(path=path, config=config, transport=transport) + run_stdio_server( + path=path, + config=config, + transport=transport, + enable_write=enable_write, + ) except Exception as exc: # noqa: BLE001 handle_command_error(cli_ctx, exc) diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 4f7c0188..c0f9b6a6 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -266,3 +266,20 @@ def test_cli_mcp_requires_path(): result = runner.invoke(app, ["mcp"], env={}) assert result.exit_code != 0 assert "--path" in result.output + + +def test_cli_mcp_help_mentions_write_toggle(): + result = runner.invoke(app, ["mcp", "--help"], env={}) + assert result.exit_code == 0 + assert "--enable-write" in result.output + assert "--readonly" in result.output + + +def test_cli_mcp_rejects_non_stdio(): + result = runner.invoke( + app, + ["mcp", "--path", "./data", "--transport", "sse"], + env={}, + ) + assert result.exit_code == 1 + assert "Only stdio transport is supported in V1" in result.output diff --git a/tests/mcp/test_protocol_stdio.py b/tests/mcp/test_protocol_stdio.py new file mode 100644 index 00000000..c9c80c4f --- /dev/null +++ b/tests/mcp/test_protocol_stdio.py @@ -0,0 +1,76 @@ +# Copyright (c) 2026 Beijing Volcano Engine Technology Co., Ltd. +# SPDX-License-Identifier: Apache-2.0 +"""Protocol-level MCP stdio tests.""" + +from __future__ import annotations + +import json +import os +import sys +import tempfile +from pathlib import Path + +import pytest + +pytest.importorskip("mcp") +from mcp.client.session import ClientSession +from mcp.client.stdio import StdioServerParameters, stdio_client + + +def _ensure_agfs_binary_available() -> None: + binary_name = "agfs-server.exe" if os.name == "nt" else "agfs-server" + binary_path = (Path("openviking") / "bin" / binary_name).resolve() + if not binary_path.exists(): + pytest.skip(f"AGFS binary not found: {binary_path}") + + +def _repo_root() -> str: + return str(Path(__file__).resolve().parents[2]) + + +def _extract_payload(call_result) -> dict: + texts = [item.text for item in call_result.content if getattr(item, "type", "") == "text"] + assert texts, "No text content returned by MCP tool" + return json.loads(texts[0]) + + +@pytest.mark.anyio +async def test_stdio_readonly_hides_add_resource_and_health_works(): + _ensure_agfs_binary_available() + with tempfile.TemporaryDirectory(dir=_repo_root()) as data_path: + server = StdioServerParameters( + command=sys.executable, + args=["-m", "openviking", "mcp", "--path", data_path], + cwd=_repo_root(), + ) + + async with stdio_client(server) as (read_stream, write_stream): + async with ClientSession(read_stream, write_stream) as session: + await session.initialize() + tools_result = await session.list_tools() + tool_names = {tool.name for tool in tools_result.tools} + assert "openviking_add_resource" not in tool_names + + health_result = await session.call_tool("openviking_health", {}) + assert health_result.isError is False + payload = _extract_payload(health_result) + assert payload["ok"] is True + assert isinstance(payload["result"]["healthy"], bool) + + +@pytest.mark.anyio +async def test_stdio_writable_includes_add_resource_tool(): + _ensure_agfs_binary_available() + with tempfile.TemporaryDirectory(dir=_repo_root()) as data_path: + server = StdioServerParameters( + command=sys.executable, + args=["-m", "openviking", "mcp", "--path", data_path, "--enable-write"], + cwd=_repo_root(), + ) + + async with stdio_client(server) as (read_stream, write_stream): + async with ClientSession(read_stream, write_stream) as session: + await session.initialize() + tools_result = await session.list_tools() + tool_names = {tool.name for tool in tools_result.tools} + assert "openviking_add_resource" in tool_names diff --git a/tests/mcp/test_server.py b/tests/mcp/test_server.py index 216ce004..7ae35af1 100644 --- a/tests/mcp/test_server.py +++ b/tests/mcp/test_server.py @@ -11,6 +11,9 @@ class _StubClient: def find(self, query, target_uri="", limit=10, score_threshold=None): return {"memories": [], "resources": [], "skills": [], "total": 0} + def search(self, query, target_uri="", session_id=None, limit=10, score_threshold=None): + return {"memories": [], "resources": [], "skills": [], "total": 0} + def read(self, uri, offset=0, limit=-1): return "hello" @@ -23,17 +26,45 @@ def abstract(self, uri): def overview(self, uri): return "ov" + def add_resource( + self, + path, + target=None, + reason="", + instruction="", + wait=False, + timeout=None, + ): + return {"root_uri": "viking://resources/demo"} + -def test_list_tools_returns_mvp_definitions(): +def test_list_tools_returns_v1_read_definitions(): adapter = OpenVikingMCPAdapter(_StubClient()) tools = adapter.list_tools() names = {tool["name"] for tool in tools} assert "openviking_find" in names + assert "openviking_search" in names assert "openviking_read" in names assert "openviking_ls" in names assert "openviking_abstract" in names assert "openviking_overview" in names + assert "openviking_wait_processed" in names + assert "openviking_stat" in names + assert "openviking_tree" in names + assert "openviking_grep" in names + assert "openviking_glob" in names + assert "openviking_status" in names + assert "openviking_health" in names + assert "openviking_add_resource" not in names + + +def test_list_tools_includes_write_tool_when_enabled(): + adapter = OpenVikingMCPAdapter(_StubClient(), allow_write=True) + tools = adapter.list_tools() + names = {tool["name"] for tool in tools} + + assert "openviking_add_resource" in names def test_call_tool_returns_json_text_payload(): @@ -43,3 +74,12 @@ def test_call_tool_returns_json_text_payload(): assert body["ok"] is True assert body["result"] == "hello" + + +def test_call_tool_write_is_denied_in_readonly_mode(): + adapter = OpenVikingMCPAdapter(_StubClient(), allow_write=False) + payload = adapter.call_tool("openviking_add_resource", {"path": "./data"}) + body = json.loads(payload) + + assert body["ok"] is False + assert body["error"]["code"] == "PERMISSION_DENIED" diff --git a/tests/mcp/test_tools.py b/tests/mcp/test_tools.py index 0a8c496f..a89d78e8 100644 --- a/tests/mcp/test_tools.py +++ b/tests/mcp/test_tools.py @@ -6,7 +6,7 @@ import pytest -from openviking.mcp.tools import TOOL_DEFINITIONS, dispatch_tool +from openviking.mcp.tools import dispatch_tool, get_tool_definitions class _FakeClient: @@ -17,6 +17,10 @@ def find(self, query, target_uri="", limit=10, score_threshold=None): self.calls.append(("find", query, target_uri, limit, score_threshold)) return {"memories": [], "resources": [], "skills": [], "total": 0} + def search(self, query, target_uri="", session_id=None, limit=10, score_threshold=None): + self.calls.append(("search", query, target_uri, session_id, limit, score_threshold)) + return {"memories": [], "resources": [], "skills": [], "total": 0} + def read(self, uri, offset=0, limit=-1): self.calls.append(("read", uri, offset, limit)) return "hello" @@ -33,90 +37,181 @@ def overview(self, uri): self.calls.append(("overview", uri)) return "ov" + def wait_processed(self, timeout=None): + self.calls.append(("wait_processed", timeout)) + return {"pending": 0} + + def stat(self, uri): + self.calls.append(("stat", uri)) + return {"uri": uri} + + def tree(self, uri, output="agent", abs_limit=128, show_all_hidden=False, node_limit=1000): + self.calls.append(("tree", uri, output, abs_limit, show_all_hidden, node_limit)) + return {"uri": uri, "children": []} + + def grep(self, uri, pattern, case_insensitive=False): + self.calls.append(("grep", uri, pattern, case_insensitive)) + return {"matches": []} + + def glob(self, pattern, uri="viking://"): + self.calls.append(("glob", pattern, uri)) + return {"matches": []} + + def get_status(self): + self.calls.append(("status",)) + return {"healthy": True} + + def is_healthy(self): + self.calls.append(("health",)) + return True + + def add_resource( + self, + path, + target=None, + reason="", + instruction="", + wait=False, + timeout=None, + ): + self.calls.append(("add_resource", path, target, reason, instruction, wait, timeout)) + return {"root_uri": "viking://resources/demo"} -def _tool_names(): - return {tool["name"] for tool in TOOL_DEFINITIONS} +def _tool_names(include_write: bool = False): + return {tool["name"] for tool in get_tool_definitions(include_write=include_write)} -def test_tool_definitions_have_minimum_mvp_set(): + +def _payload(tool_name, arguments, client, allow_write=False): + return json.loads(dispatch_tool(tool_name, arguments, client, allow_write=allow_write)) + + +def test_tool_definitions_have_v1_read_set(): names = _tool_names() assert "openviking_find" in names + assert "openviking_search" in names assert "openviking_read" in names assert "openviking_ls" in names assert "openviking_abstract" in names assert "openviking_overview" in names + assert "openviking_wait_processed" in names + assert "openviking_stat" in names + assert "openviking_tree" in names + assert "openviking_grep" in names + assert "openviking_glob" in names + assert "openviking_status" in names + assert "openviking_health" in names + assert "openviking_add_resource" not in names + + +def test_get_tool_definitions_respects_write_flag(): + read_only_names = _tool_names(include_write=False) + writable_names = _tool_names(include_write=True) + + assert "openviking_add_resource" not in read_only_names + assert "openviking_add_resource" in writable_names def test_dispatch_find_success(): client = _FakeClient() - payload = dispatch_tool("openviking_find", {"query": "what is openviking"}, client) - body = json.loads(payload) + body = _payload("openviking_find", {"query": "what is openviking"}, client) assert body["ok"] is True assert body["result"]["total"] == 0 - assert client.calls[0][0] == "find" + assert client.calls[0] == ("find", "what is openviking", "", 10, None) + + +def test_dispatch_search_success(): + client = _FakeClient() + body = _payload( + "openviking_search", + {"query": "design", "uri": "viking://resources", "session_id": "s1", "threshold": 0.5}, + client, + ) + + assert body["ok"] is True + assert client.calls[0] == ("search", "design", "viking://resources", "s1", 10, 0.5) def test_dispatch_read_applies_default_limit_cap(): client = _FakeClient() - dispatch_tool("openviking_read", {"uri": "viking://resources/a.md"}, client) + _payload("openviking_read", {"uri": "viking://resources/a.md"}, client) assert client.calls[0] == ("read", "viking://resources/a.md", 0, 200) def test_dispatch_read_rejects_oversized_limit(): client = _FakeClient() - payload = dispatch_tool("openviking_read", {"uri": "viking://resources/a.md", "limit": 5000}, client) - body = json.loads(payload) + body = _payload("openviking_read", {"uri": "viking://resources/a.md", "limit": 5000}, client) assert body["ok"] is False assert body["error"]["code"] == "INVALID_ARGUMENT" @pytest.mark.parametrize( - "args,key", + "args,key,message", [ - ({"uri": "viking://resources/a.md", "limit": True}, "limit"), - ({"uri": "viking://resources/a.md", "offset": False}, "offset"), + ({"uri": "viking://resources/a.md", "limit": True}, "limit", "'limit' must be an integer"), + ({"uri": "viking://resources/a.md", "offset": False}, "offset", "'offset' must be an integer"), + ( + {"query": "x", "threshold": True}, + "threshold", + "'threshold' must be a number", + ), ], ) -def test_dispatch_read_rejects_boolean_integer_fields(args, key): +def test_dispatch_rejects_boolean_for_numeric_fields(args, key, message): client = _FakeClient() - payload = dispatch_tool("openviking_read", args, client) - body = json.loads(payload) + body = _payload("openviking_search" if key == "threshold" else "openviking_read", args, client) assert body["ok"] is False assert body["error"]["code"] == "INVALID_ARGUMENT" - assert body["error"]["message"] == f"'{key}' must be an integer" + assert body["error"]["message"] == message def test_dispatch_find_requires_query(): client = _FakeClient() - payload = dispatch_tool("openviking_find", {}, client) - body = json.loads(payload) + body = _payload("openviking_find", {}, client) assert body["ok"] is False assert body["error"]["code"] == "INVALID_ARGUMENT" -def test_dispatch_unknown_tool_returns_error(): +def test_dispatch_add_resource_is_denied_in_readonly_mode(): client = _FakeClient() - payload = dispatch_tool("no_such_tool", {}, client) - body = json.loads(payload) + body = _payload("openviking_add_resource", {"path": "./demo"}, client, allow_write=False) assert body["ok"] is False - assert body["error"]["code"] == "TOOL_NOT_FOUND" - + assert body["error"]["code"] == "PERMISSION_DENIED" + assert client.calls == [] -def test_dispatch_client_exception_is_wrapped(): - class _BrokenClient: - def read(self, *args, **kwargs): - raise RuntimeError("boom") - payload = dispatch_tool("openviking_read", {"uri": "viking://resources/a.md"}, _BrokenClient()) - body = json.loads(payload) +def test_dispatch_add_resource_success_when_write_enabled(): + client = _FakeClient() + body = _payload( + "openviking_add_resource", + { + "path": "./demo", + "to": "viking://resources/target", + "reason": "import", + "instruction": "keep structure", + "wait": True, + "timeout": 30.5, + }, + client, + allow_write=True, + ) - assert body["ok"] is False - assert body["error"]["code"] == "INTERNAL" + assert body["ok"] is True + assert body["result"]["root_uri"] == "viking://resources/demo" + assert client.calls[0] == ( + "add_resource", + "./demo", + "viking://resources/target", + "import", + "keep structure", + True, + 30.5, + ) @pytest.mark.parametrize( @@ -129,12 +224,74 @@ def read(self, *args, **kwargs): ), ("openviking_abstract", {"uri": "viking://resources"}, ("abstract", "viking://resources")), ("openviking_overview", {"uri": "viking://resources"}, ("overview", "viking://resources")), + ("openviking_wait_processed", {"timeout": 5.0}, ("wait_processed", 5.0)), + ("openviking_stat", {"uri": "viking://resources/a.md"}, ("stat", "viking://resources/a.md")), + ( + "openviking_tree", + { + "uri": "viking://resources", + "abs_limit": 256, + "show_all_hidden": True, + "node_limit": 50, + }, + ("tree", "viking://resources", "agent", 256, True, 50), + ), + ( + "openviking_grep", + {"uri": "viking://resources", "pattern": "OpenViking", "ignore_case": True}, + ("grep", "viking://resources", "OpenViking", True), + ), + ( + "openviking_glob", + {"pattern": "*.md", "uri": "viking://resources"}, + ("glob", "*.md", "viking://resources"), + ), + ("openviking_status", {}, ("status",)), + ("openviking_health", {}, ("health",)), ], ) -def test_dispatch_other_mvp_tools(tool_name, args, expected_call): +def test_dispatch_v1_tools_success(tool_name, args, expected_call): client = _FakeClient() - payload = dispatch_tool(tool_name, args, client) - body = json.loads(payload) + body = _payload(tool_name, args, client) assert body["ok"] is True assert client.calls[0] == expected_call + if tool_name == "openviking_health": + assert body["result"]["healthy"] is True + + +@pytest.mark.parametrize( + "args,field", + [ + ({"uri": "viking://resources", "abs_limit": -1}, "abs_limit"), + ({"uri": "viking://resources", "abs_limit": 5000}, "abs_limit"), + ({"uri": "viking://resources", "node_limit": 0}, "node_limit"), + ({"uri": "viking://resources", "node_limit": 6000}, "node_limit"), + ], +) +def test_dispatch_tree_rejects_out_of_range_limits(args, field): + client = _FakeClient() + body = _payload("openviking_tree", args, client) + + assert body["ok"] is False + assert body["error"]["code"] == "INVALID_ARGUMENT" + assert field in body["error"]["message"] + + +def test_dispatch_unknown_tool_returns_error(): + client = _FakeClient() + body = _payload("no_such_tool", {}, client) + + assert body["ok"] is False + assert body["error"]["code"] == "TOOL_NOT_FOUND" + + +def test_dispatch_client_exception_is_wrapped(): + class _BrokenClient: + def read(self, *args, **kwargs): + raise RuntimeError("boom") + + body = _payload("openviking_read", {"uri": "viking://resources/a.md"}, _BrokenClient()) + + assert body["ok"] is False + assert body["error"]["code"] == "INTERNAL" From 132d4ff1a70f4e8f4cefc66e2bf0d8bfdc157662 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=BD=98=E6=B0=B8=E6=98=8E?= <206003912@nbu.edu.cn> Date: Thu, 26 Feb 2026 11:41:46 +0800 Subject: [PATCH 3/3] feat(mcp): extend V1 tools and add access-level gating - Add four-tier access level model: readonly < ingest < mutate < admin - Expose session, relation, filesystem, and pack tools gated by level - Register backward-compatible alias openviking_add_resource -> openviking_resource_add - CLI: --enable-write maps to --access-level mutate for compatibility - Tests: all 45 MCP tests pass (3 skipped, 0 failed) --- README.md | 22 +- README_CN.md | 20 +- openviking/mcp/__init__.py | 12 + openviking/mcp/permissions.py | 55 ++ openviking/mcp/server.py | 180 ++++++- openviking/mcp/tools.py | 776 +++++++++++++++++++++-------- openviking_cli/cli/commands/mcp.py | 20 +- tests/cli/test_cli.py | 21 + tests/mcp/test_protocol_stdio.py | 36 +- tests/mcp/test_server.py | 118 +++-- tests/mcp/test_tools.py | 318 ++++++------ 11 files changed, 1171 insertions(+), 407 deletions(-) create mode 100644 openviking/mcp/permissions.py diff --git a/README.md b/README.md index 43504a85..56d59fbb 100644 --- a/README.md +++ b/README.md @@ -432,18 +432,22 @@ Start MCP server (stdio transport, readonly by default): openviking mcp --path ./data ``` -Enable write tools explicitly when needed: +Enable access level explicitly when needed: ```bash -openviking mcp --path ./data --enable-write +openviking mcp --path ./data --access-level mutate ``` Current V1 scope: - Transport: `stdio` only - Runtime mode: embedded local path (`--path`) -- Default mode: readonly (write tools disabled unless `--enable-write` is set) -- Read tools: `openviking_find`, `openviking_search`, `openviking_read`, `openviking_ls`, `openviking_abstract`, `openviking_overview`, `openviking_wait_processed`, `openviking_stat`, `openviking_tree`, `openviking_grep`, `openviking_glob`, `openviking_status`, `openviking_health` -- Write tools (optional): `openviking_add_resource` +- Access levels: `readonly`, `ingest`, `mutate`, `admin` +- Compatibility: `--enable-write` maps to `--access-level mutate` +- Readonly tools: `openviking_find`, `openviking_search`, `openviking_read`, `openviking_ls`, `openviking_abstract`, `openviking_overview`, `openviking_wait_processed`, `openviking_stat`, `openviking_tree`, `openviking_grep`, `openviking_glob`, `openviking_status`, `openviking_health`, `openviking_session_list`, `openviking_session_get`, `openviking_relation_list` +- Ingest tools: `openviking_session_create`, `openviking_session_add_message`, `openviking_session_commit`, `openviking_resource_add`, `openviking_resource_add_skill` +- Mutate tools: `openviking_relation_link`, `openviking_relation_unlink`, `openviking_fs_mkdir`, `openviking_fs_mv` +- Admin tools: `openviking_session_delete`, `openviking_fs_rm`, `openviking_pack_export`, `openviking_pack_import` +- Compatibility alias: `openviking_add_resource` is accepted as alias of `openviking_resource_add` OpenCode example configuration: @@ -465,7 +469,7 @@ OpenCode writable configuration: "mcp": { "openviking": { "command": "openviking", - "args": ["mcp", "--path", "./data", "--enable-write"] + "args": ["mcp", "--path", "./data", "--access-level", "mutate"] } } } @@ -491,7 +495,7 @@ Codex writable configuration: "mcpServers": { "openviking": { "command": "openviking", - "args": ["mcp", "--path", "./data", "--enable-write"] + "args": ["mcp", "--path", "./data", "--access-level", "mutate"] } } } @@ -499,7 +503,9 @@ Codex writable configuration: Production recommendation: - Prefer readonly MCP instances by default. -- Run writable instances separately with explicit access control. +- Use `ingest` for content ingestion workflows. +- Use `mutate` only when relation or filesystem mutation is required. +- Restrict `admin` to trusted environments and operators only. --- diff --git a/README_CN.md b/README_CN.md index de913fb4..42d17281 100644 --- a/README_CN.md +++ b/README_CN.md @@ -311,15 +311,19 @@ openviking mcp --path ./data 需要写入能力时，显式开启写模式： ```bash -openviking mcp --path ./data --enable-write +openviking mcp --path ./data --access-level mutate ``` 当前 V1 范围： - 传输协议：仅 `stdio` - 运行模式：仅本地嵌入 `--path` -- 默认模式：只读（除非设置 `--enable-write`，否则不暴露写工具） -- 读取工具：`openviking_find`、`openviking_search`、`openviking_read`、`openviking_ls`、`openviking_abstract`、`openviking_overview`、`openviking_wait_processed`、`openviking_stat`、`openviking_tree`、`openviking_grep`、`openviking_glob`、`openviking_status`、`openviking_health` -- 写入工具（可选）：`openviking_add_resource` +- 访问级别：`readonly`、`ingest`、`mutate`、`admin` +- 兼容开关：`--enable-write` 等价于 `--access-level mutate` +- 只读工具：`openviking_find`、`openviking_search`、`openviking_read`、`openviking_ls`、`openviking_abstract`、`openviking_overview`、`openviking_wait_processed`、`openviking_stat`、`openviking_tree`、`openviking_grep`、`openviking_glob`、`openviking_status`、`openviking_health`、`openviking_session_list`、`openviking_session_get`、`openviking_relation_list` +- ingest 工具：`openviking_session_create`、`openviking_session_add_message`、`openviking_session_commit`、`openviking_resource_add`、`openviking_resource_add_skill` +- mutate 工具：`openviking_relation_link`、`openviking_relation_unlink`、`openviking_fs_mkdir`、`openviking_fs_mv` +- admin 工具：`openviking_session_delete`、`openviking_fs_rm`、`openviking_pack_export`、`openviking_pack_import` +- 兼容别名：`openviking_add_resource` 可作为 `openviking_resource_add` 的别名继续调用 OpenCode 配置示例： @@ -341,7 +345,7 @@ OpenCode 可写配置示例： "mcp": { "openviking": { "command": "openviking", - "args": ["mcp", "--path", "./data", "--enable-write"] + "args": ["mcp", "--path", "./data", "--access-level", "mutate"] } } } @@ -367,7 +371,7 @@ Codex 可写配置示例： "mcpServers": { "openviking": { "command": "openviking", - "args": ["mcp", "--path", "./data", "--enable-write"] + "args": ["mcp", "--path", "./data", "--access-level", "mutate"] } } } @@ -375,7 +379,9 @@ Codex 可写配置示例： 生产环境建议： - 默认使用只读 MCP 实例。 -- 写入实例建议独立部署并做明确访问控制。 +- 内容导入场景建议使用 `ingest` 级别。 +- 需要关系/文件结构变更时再使用 `mutate`。 +- `admin` 仅建议在受信任环境中使用。 --- diff --git a/openviking/mcp/__init__.py b/openviking/mcp/__init__.py index a460d08e..978dbdaf 100644 --- a/openviking/mcp/__init__.py +++ b/openviking/mcp/__init__.py @@ -3,12 +3,24 @@ """MCP integration for OpenViking.""" from openviking.mcp.server import OpenVikingMCPAdapter, run_stdio_server +from openviking.mcp.permissions import ( + ACCESS_LEVEL_ORDER, + MCPAccessLevel, + access_level_name, + can_access, + parse_access_level, +) from openviking.mcp.tools import TOOL_DEFINITIONS, dispatch_tool, get_tool_definitions __all__ = [ "TOOL_DEFINITIONS", "get_tool_definitions", "dispatch_tool", + "MCPAccessLevel", + "ACCESS_LEVEL_ORDER", + "parse_access_level", + "access_level_name", + "can_access", "OpenVikingMCPAdapter", "run_stdio_server", ] diff --git a/openviking/mcp/permissions.py b/openviking/mcp/permissions.py new file mode 100644 index 00000000..5cb729e8 --- /dev/null +++ b/openviking/mcp/permissions.py @@ -0,0 +1,55 @@ +# Copyright (c) 2026 Beijing Volcano Engine Technology Co., Ltd. +# SPDX-License-Identifier: Apache-2.0 +"""Access-level helpers for OpenViking MCP tools.""" + +from __future__ import annotations + +from enum import IntEnum + + +class MCPAccessLevel(IntEnum): + """Ordered access levels for MCP tool authorization.""" + + READONLY = 0 + INGEST = 1 + MUTATE = 2 + ADMIN = 3 + + +ACCESS_LEVEL_ORDER = ("readonly", "ingest", "mutate", "admin") + +_ACCESS_LEVEL_BY_NAME = { + "readonly": MCPAccessLevel.READONLY, + "ingest": MCPAccessLevel.INGEST, + "mutate": MCPAccessLevel.MUTATE, + "admin": MCPAccessLevel.ADMIN, +} + + +def parse_access_level(value: MCPAccessLevel | str) -> MCPAccessLevel: + """Parse access-level input into MCPAccessLevel.""" + if isinstance(value, MCPAccessLevel): + return value + if isinstance(value, str): + normalized = value.strip().lower() + if normalized in _ACCESS_LEVEL_BY_NAME: + return _ACCESS_LEVEL_BY_NAME[normalized] + allowed = ", ".join(ACCESS_LEVEL_ORDER) + raise ValueError(f"Invalid access level '{value}'. Expected one of: {allowed}") + + +def access_level_name(value: MCPAccessLevel | str) -> str: + """Return canonical lowercase name of an access level.""" + level = parse_access_level(value) + for name, enum_value in _ACCESS_LEVEL_BY_NAME.items(): + if enum_value == level: + return name + return "readonly" + + +def can_access( + current: MCPAccessLevel | str, + required: MCPAccessLevel | str, +) -> bool: + """Return whether current level can access required level.""" + return parse_access_level(current) >= parse_access_level(required) diff --git a/openviking/mcp/server.py b/openviking/mcp/server.py index fc5d9545..77c4a0bb 100644 --- a/openviking/mcp/server.py +++ b/openviking/mcp/server.py @@ -9,6 +9,7 @@ from openviking_cli.utils.logger import get_logger +from .permissions import MCPAccessLevel, access_level_name, can_access, parse_access_level from .tools import dispatch_tool, get_tool_definitions logger = get_logger(__name__) @@ -17,22 +18,25 @@ class OpenVikingMCPAdapter: """Thin adapter exposing OpenViking methods to MCP tool handlers.""" - def __init__(self, client: Any, allow_write: bool = False): + def __init__(self, client: Any, access_level: MCPAccessLevel | str = MCPAccessLevel.READONLY): self.client = client - self.allow_write = allow_write + self.access_level = parse_access_level(access_level) def list_tools(self) -> List[Dict[str, Any]]: - return get_tool_definitions(include_write=self.allow_write) + return get_tool_definitions(access_level=self.access_level) def call_tool(self, name: str, arguments: Optional[Dict[str, Any]] = None) -> str: - return dispatch_tool(name, arguments or {}, self.client, allow_write=self.allow_write) + return dispatch_tool(name, arguments or {}, self.client, access_level=self.access_level) + + def can_access(self, required: MCPAccessLevel | str) -> bool: + return can_access(self.access_level, required) def run_stdio_server( path: str, config: Optional[str] = None, transport: str = "stdio", - enable_write: bool = False, + access_level: MCPAccessLevel | str = MCPAccessLevel.READONLY, ) -> None: """Run OpenViking MCP server in stdio mode.""" if transport != "stdio": @@ -51,10 +55,15 @@ def run_stdio_server( from openviking.sync_client import SyncOpenViking + resolved_access = parse_access_level(access_level) client = SyncOpenViking(path=path) - adapter = OpenVikingMCPAdapter(client, allow_write=enable_write) + adapter = OpenVikingMCPAdapter(client, access_level=resolved_access) client.initialize() - logger.info("[MCP] OpenViking client initialized (path=%s, enable_write=%s)", path, enable_write) + logger.info( + "[MCP] OpenViking client initialized (path=%s, access_level=%s)", + path, + access_level_name(resolved_access), + ) mcp = FastMCP("openviking") @@ -102,10 +111,26 @@ def openviking_read(uri: str, offset: int = 0, limit: int = 200) -> str: ) @mcp.tool(description="List directory contents in OpenViking.") - def openviking_ls(uri: str = "viking://", simple: bool = False, recursive: bool = False) -> str: + def openviking_ls( + uri: str = "viking://", + simple: bool = False, + recursive: bool = False, + output: str = "agent", + abs_limit: int = 256, + show_all_hidden: bool = False, + node_limit: int = 1000, + ) -> str: return adapter.call_tool( "openviking_ls", - {"uri": uri, "simple": simple, "recursive": recursive}, + { + "uri": uri, + "simple": simple, + "recursive": recursive, + "output": output, + "abs_limit": abs_limit, + "show_all_hidden": show_all_hidden, + "node_limit": node_limit, + }, ) @mcp.tool(description="Read L0 abstract (.abstract.md) for a directory URI.") @@ -160,9 +185,72 @@ def openviking_status() -> str: def openviking_health() -> str: return adapter.call_tool("openviking_health", {}) - if enable_write: + @mcp.tool(description="List sessions.") + def openviking_session_list() -> str: + return adapter.call_tool("openviking_session_list", {}) + + @mcp.tool(description="Get session details.") + def openviking_session_get(session_id: str) -> str: + return adapter.call_tool("openviking_session_get", {"session_id": session_id}) + + @mcp.tool(description="List relations of a resource.") + def openviking_relation_list(uri: str) -> str: + return adapter.call_tool("openviking_relation_list", {"uri": uri}) + + if adapter.can_access("ingest"): + + @mcp.tool(description="Create a new session.") + def openviking_session_create() -> str: + return adapter.call_tool("openviking_session_create", {}) + + @mcp.tool(description="Add a message to a session.") + def openviking_session_add_message( + session_id: str, + role: str, + content: str | None = None, + parts: list[dict] | None = None, + ) -> str: + return adapter.call_tool( + "openviking_session_add_message", + { + "session_id": session_id, + "role": role, + "content": content, + "parts": parts, + }, + ) + + @mcp.tool(description="Commit a session (archive and extract memories).") + def openviking_session_commit(session_id: str) -> str: + return adapter.call_tool("openviking_session_commit", {"session_id": session_id}) @mcp.tool(description="Add local path or URL as resource into OpenViking.") + def openviking_resource_add( + path: str, + to: str | None = None, + reason: str = "", + instruction: str = "", + wait: bool = False, + timeout: float | None = None, + ) -> str: + return adapter.call_tool( + "openviking_resource_add", + { + "path": path, + "to": to, + "reason": reason, + "instruction": instruction, + "wait": wait, + "timeout": timeout, + }, + ) + + @mcp.tool( + description=( + "[Deprecated alias] Add local path or URL as resource into OpenViking. " + "Use openviking_resource_add instead." + ) + ) def openviking_add_resource( path: str, to: str | None = None, @@ -172,7 +260,7 @@ def openviking_add_resource( timeout: float | None = None, ) -> str: return adapter.call_tool( - "openviking_add_resource", + "openviking_resource_add", { "path": path, "to": to, @@ -183,6 +271,76 @@ def openviking_add_resource( }, ) + @mcp.tool(description="Add a skill into OpenViking.") + def openviking_resource_add_skill( + data: str, + wait: bool = False, + timeout: float | None = None, + ) -> str: + return adapter.call_tool( + "openviking_resource_add_skill", + {"data": data, "wait": wait, "timeout": timeout}, + ) + + if adapter.can_access("mutate"): + + @mcp.tool(description="Create relation links from one URI to one or more targets.") + def openviking_relation_link( + from_uri: str, + uris: str | list[str], + reason: str = "", + ) -> str: + return adapter.call_tool( + "openviking_relation_link", + {"from_uri": from_uri, "uris": uris, "reason": reason}, + ) + + @mcp.tool(description="Remove a relation link.") + def openviking_relation_unlink(from_uri: str, uri: str) -> str: + return adapter.call_tool( + "openviking_relation_unlink", + {"from_uri": from_uri, "uri": uri}, + ) + + @mcp.tool(description="Create a directory.") + def openviking_fs_mkdir(uri: str) -> str: + return adapter.call_tool("openviking_fs_mkdir", {"uri": uri}) + + @mcp.tool(description="Move or rename a resource.") + def openviking_fs_mv(from_uri: str, to_uri: str) -> str: + return adapter.call_tool("openviking_fs_mv", {"from_uri": from_uri, "to_uri": to_uri}) + + if adapter.can_access("admin"): + + @mcp.tool(description="Delete a session.") + def openviking_session_delete(session_id: str) -> str: + return adapter.call_tool("openviking_session_delete", {"session_id": session_id}) + + @mcp.tool(description="Remove a resource.") + def openviking_fs_rm(uri: str, recursive: bool = False) -> str: + return adapter.call_tool("openviking_fs_rm", {"uri": uri, "recursive": recursive}) + + @mcp.tool(description="Export context as .ovpack.") + def openviking_pack_export(uri: str, to: str) -> str: + return adapter.call_tool("openviking_pack_export", {"uri": uri, "to": to}) + + @mcp.tool(description="Import .ovpack into target URI.") + def openviking_pack_import( + file_path: str, + target_uri: str, + force: bool = False, + vectorize: bool = True, + ) -> str: + return adapter.call_tool( + "openviking_pack_import", + { + "file_path": file_path, + "target_uri": target_uri, + "force": force, + "vectorize": vectorize, + }, + ) + try: mcp.run(transport="stdio") finally: diff --git a/openviking/mcp/tools.py b/openviking/mcp/tools.py index ecdc6b59..aeb2b016 100644 --- a/openviking/mcp/tools.py +++ b/openviking/mcp/tools.py @@ -10,6 +10,8 @@ from enum import Enum from typing import Any, Dict, List +from .permissions import MCPAccessLevel, access_level_name, can_access, parse_access_level + MAX_READ_LIMIT = 2000 DEFAULT_READ_LIMIT = 200 MAX_FIND_LIMIT = 50 @@ -18,12 +20,28 @@ DEFAULT_TREE_ABS_LIMIT = 128 MAX_TREE_NODE_LIMIT = 5000 DEFAULT_TREE_NODE_LIMIT = 1000 +ALLOWED_SESSION_ROLES = {"user", "assistant", "system", "tool"} + + +def _tool( + name: str, + description: str, + input_schema: Dict[str, Any], + min_access_level: str = "readonly", +) -> Dict[str, Any]: + return { + "name": name, + "description": description, + "inputSchema": input_schema, + "minAccessLevel": min_access_level, + } + READ_TOOL_DEFINITIONS = [ - { - "name": "openviking_find", - "description": "Semantic search in OpenViking context database.", - "inputSchema": { + _tool( + "openviking_find", + "Semantic search in OpenViking context database.", + { "type": "object", "properties": { "query": {"type": "string", "description": "Search query."}, @@ -39,18 +57,15 @@ "minimum": 1, "maximum": MAX_FIND_LIMIT, }, - "threshold": { - "type": "number", - "description": "Optional score threshold.", - }, + "threshold": {"type": "number", "description": "Optional score threshold."}, }, "required": ["query"], }, - }, - { - "name": "openviking_search", - "description": "Context-aware retrieval in OpenViking.", - "inputSchema": { + ), + _tool( + "openviking_search", + "Context-aware retrieval in OpenViking.", + { "type": "object", "properties": { "query": {"type": "string", "description": "Search query."}, @@ -70,18 +85,15 @@ "minimum": 1, "maximum": MAX_FIND_LIMIT, }, - "threshold": { - "type": "number", - "description": "Optional score threshold.", - }, + "threshold": {"type": "number", "description": "Optional score threshold."}, }, "required": ["query"], }, - }, - { - "name": "openviking_read", - "description": "Read content from OpenViking (L2).", - "inputSchema": { + ), + _tool( + "openviking_read", + "Read content from OpenViking (L2).", + { "type": "object", "properties": { "uri": {"type": "string", "description": "Resource URI."}, @@ -104,11 +116,11 @@ }, "required": ["uri"], }, - }, - { - "name": "openviking_ls", - "description": "List directory contents in OpenViking.", - "inputSchema": { + ), + _tool( + "openviking_ls", + "List directory contents in OpenViking.", + { "type": "object", "properties": { "uri": { @@ -126,55 +138,74 @@ "description": "Whether to list subdirectories recursively.", "default": False, }, + "output": { + "type": "string", + "description": "Output format, either 'agent' or 'original'.", + "default": "agent", + }, + "abs_limit": { + "type": "integer", + "description": f"Abstract content limit (0-{MAX_TREE_ABS_LIMIT}).", + "default": 256, + "minimum": 0, + "maximum": MAX_TREE_ABS_LIMIT, + }, + "show_all_hidden": { + "type": "boolean", + "description": "Whether to include all hidden entries.", + "default": False, + }, + "node_limit": { + "type": "integer", + "description": f"Maximum nodes in output (1-{MAX_TREE_NODE_LIMIT}).", + "default": DEFAULT_TREE_NODE_LIMIT, + "minimum": 1, + "maximum": MAX_TREE_NODE_LIMIT, + }, }, "required": [], }, - }, - { - "name": "openviking_abstract", - "description": "Read L0 abstract (.abstract.md) for a directory URI.", - "inputSchema": { + ), + _tool( + "openviking_abstract", + "Read L0 abstract (.abstract.md) for a directory URI.", + { "type": "object", "properties": {"uri": {"type": "string", "description": "Directory URI."}}, "required": ["uri"], }, - }, - { - "name": "openviking_overview", - "description": "Read L1 overview (.overview.md) for a directory URI.", - "inputSchema": { + ), + _tool( + "openviking_overview", + "Read L1 overview (.overview.md) for a directory URI.", + { "type": "object", "properties": {"uri": {"type": "string", "description": "Directory URI."}}, "required": ["uri"], }, - }, - { - "name": "openviking_wait_processed", - "description": "Wait until queued async processing completes.", - "inputSchema": { + ), + _tool( + "openviking_wait_processed", + "Wait until queued async processing completes.", + { "type": "object", - "properties": { - "timeout": { - "type": "number", - "description": "Optional timeout in seconds.", - } - }, + "properties": {"timeout": {"type": "number", "description": "Optional timeout in seconds."}}, "required": [], }, - }, - { - "name": "openviking_stat", - "description": "Get resource metadata and status.", - "inputSchema": { + ), + _tool( + "openviking_stat", + "Get resource metadata and status.", + { "type": "object", "properties": {"uri": {"type": "string", "description": "Resource URI."}}, "required": ["uri"], }, - }, - { - "name": "openviking_tree", - "description": "Get directory tree in agent-friendly format.", - "inputSchema": { + ), + _tool( + "openviking_tree", + "Get directory tree in agent-friendly format.", + { "type": "object", "properties": { "uri": {"type": "string", "description": "Directory URI."}, @@ -200,19 +231,15 @@ }, "required": ["uri"], }, - }, - { - "name": "openviking_grep", - "description": "Search text pattern in files under a URI.", - "inputSchema": { + ), + _tool( + "openviking_grep", + "Search text pattern in files under a URI.", + { "type": "object", "properties": { "pattern": {"type": "string", "description": "Search pattern."}, - "uri": { - "type": "string", - "description": "Search root URI.", - "default": "viking://", - }, + "uri": {"type": "string", "description": "Search root URI.", "default": "viking://"}, "ignore_case": { "type": "boolean", "description": "Case-insensitive matching.", @@ -221,55 +248,115 @@ }, "required": ["pattern"], }, - }, - { - "name": "openviking_glob", - "description": "Search files by glob pattern under a URI.", - "inputSchema": { + ), + _tool( + "openviking_glob", + "Search files by glob pattern under a URI.", + { "type": "object", "properties": { "pattern": {"type": "string", "description": "Glob pattern."}, - "uri": { - "type": "string", - "description": "Search root URI.", - "default": "viking://", - }, + "uri": {"type": "string", "description": "Search root URI.", "default": "viking://"}, }, "required": ["pattern"], }, - }, - { - "name": "openviking_status", - "description": "Get OpenViking component status.", - "inputSchema": {"type": "object", "properties": {}, "required": []}, - }, - { - "name": "openviking_health", - "description": "Get quick health check result.", - "inputSchema": {"type": "object", "properties": {}, "required": []}, - }, + ), + _tool("openviking_status", "Get OpenViking component status.", {"type": "object", "properties": {}, "required": []}), + _tool("openviking_health", "Get quick health check result.", {"type": "object", "properties": {}, "required": []}), + _tool("openviking_session_list", "List sessions.", {"type": "object", "properties": {}, "required": []}), + _tool( + "openviking_session_get", + "Get session details.", + { + "type": "object", + "properties": {"session_id": {"type": "string", "description": "Session ID."}}, + "required": ["session_id"], + }, + ), + _tool( + "openviking_relation_list", + "List relations of a resource.", + { + "type": "object", + "properties": {"uri": {"type": "string", "description": "Resource URI."}}, + "required": ["uri"], + }, + ), ] -WRITE_TOOL_DEFINITIONS = [ - { - "name": "openviking_add_resource", - "description": "Add local path or URL as resource into OpenViking.", - "inputSchema": { +INGEST_TOOL_DEFINITIONS = [ + _tool( + "openviking_session_create", + "Create a new session.", + {"type": "object", "properties": {}, "required": []}, + min_access_level="ingest", + ), + _tool( + "openviking_session_add_message", + "Add a message to a session.", + { "type": "object", "properties": { - "path": { + "session_id": {"type": "string", "description": "Session ID."}, + "role": {"type": "string", "description": "Message role: user, assistant, system, or tool."}, + "content": { "type": "string", - "description": "Local path or URL to import.", + "description": "Optional text content. Required when parts is absent.", }, - "to": { - "type": "string", - "description": "Optional target URI.", + "parts": { + "type": "array", + "description": "Optional message parts. Required when content is absent.", + "items": {"type": "object"}, }, - "reason": { + }, + "required": ["session_id", "role"], + }, + min_access_level="ingest", + ), + _tool( + "openviking_session_commit", + "Commit a session (archive and extract memories).", + { + "type": "object", + "properties": {"session_id": {"type": "string", "description": "Session ID."}}, + "required": ["session_id"], + }, + min_access_level="ingest", + ), + _tool( + "openviking_resource_add", + "Add local path or URL as resource into OpenViking.", + { + "type": "object", + "properties": { + "path": {"type": "string", "description": "Local path or URL to import."}, + "to": {"type": "string", "description": "Optional target URI."}, + "reason": {"type": "string", "description": "Optional import reason.", "default": ""}, + "instruction": { "type": "string", - "description": "Optional import reason.", + "description": "Optional additional instruction.", "default": "", }, + "wait": { + "type": "boolean", + "description": "Wait until processing completes.", + "default": False, + }, + "timeout": {"type": "number", "description": "Optional wait timeout in seconds."}, + }, + "required": ["path"], + }, + min_access_level="ingest", + ), + _tool( + "openviking_add_resource", + "[Deprecated alias] Add local path or URL as resource into OpenViking. Use openviking_resource_add.", + { + "type": "object", + "properties": { + "path": {"type": "string", "description": "Local path or URL to import."}, + "to": {"type": "string", "description": "Optional target URI."}, + "reason": {"type": "string", "description": "Optional import reason.", "default": ""}, "instruction": { "type": "string", "description": "Optional additional instruction.", @@ -280,26 +367,162 @@ "description": "Wait until processing completes.", "default": False, }, - "timeout": { - "type": "number", - "description": "Optional wait timeout in seconds.", - }, + "timeout": {"type": "number", "description": "Optional wait timeout in seconds."}, }, "required": ["path"], }, - } + min_access_level="ingest", + ), + _tool( + "openviking_resource_add_skill", + "Add a skill into OpenViking.", + { + "type": "object", + "properties": { + "data": {"type": "string", "description": "Skill directory, SKILL.md, or raw content."}, + "wait": {"type": "boolean", "description": "Wait until processing completes.", "default": False}, + "timeout": {"type": "number", "description": "Optional wait timeout in seconds."}, + }, + "required": ["data"], + }, + min_access_level="ingest", + ), ] -TOOL_DEFINITIONS = READ_TOOL_DEFINITIONS + WRITE_TOOL_DEFINITIONS +MUTATE_TOOL_DEFINITIONS = [ + _tool( + "openviking_relation_link", + "Create relation links from one URI to one or more targets.", + { + "type": "object", + "properties": { + "from_uri": {"type": "string", "description": "Source URI."}, + "uris": { + "anyOf": [{"type": "string"}, {"type": "array", "items": {"type": "string"}}], + "description": "Target URI or list of target URIs.", + }, + "reason": {"type": "string", "description": "Reason for linking.", "default": ""}, + }, + "required": ["from_uri", "uris"], + }, + min_access_level="mutate", + ), + _tool( + "openviking_relation_unlink", + "Remove a relation link.", + { + "type": "object", + "properties": { + "from_uri": {"type": "string", "description": "Source URI."}, + "uri": {"type": "string", "description": "Target URI to unlink."}, + }, + "required": ["from_uri", "uri"], + }, + min_access_level="mutate", + ), + _tool( + "openviking_fs_mkdir", + "Create a directory.", + { + "type": "object", + "properties": {"uri": {"type": "string", "description": "Directory URI to create."}}, + "required": ["uri"], + }, + min_access_level="mutate", + ), + _tool( + "openviking_fs_mv", + "Move or rename a resource.", + { + "type": "object", + "properties": { + "from_uri": {"type": "string", "description": "Source URI."}, + "to_uri": {"type": "string", "description": "Target URI."}, + }, + "required": ["from_uri", "to_uri"], + }, + min_access_level="mutate", + ), +] + +ADMIN_TOOL_DEFINITIONS = [ + _tool( + "openviking_session_delete", + "Delete a session.", + { + "type": "object", + "properties": {"session_id": {"type": "string", "description": "Session ID."}}, + "required": ["session_id"], + }, + min_access_level="admin", + ), + _tool( + "openviking_fs_rm", + "Remove a resource.", + { + "type": "object", + "properties": { + "uri": {"type": "string", "description": "Viking URI to remove."}, + "recursive": {"type": "boolean", "description": "Remove recursively.", "default": False}, + }, + "required": ["uri"], + }, + min_access_level="admin", + ), + _tool( + "openviking_pack_export", + "Export context as .ovpack.", + { + "type": "object", + "properties": { + "uri": {"type": "string", "description": "Source URI."}, + "to": {"type": "string", "description": "Output .ovpack file path."}, + }, + "required": ["uri", "to"], + }, + min_access_level="admin", + ), + _tool( + "openviking_pack_import", + "Import .ovpack into target URI.", + { + "type": "object", + "properties": { + "file_path": {"type": "string", "description": "Input .ovpack file path."}, + "target_uri": {"type": "string", "description": "Target parent URI."}, + "force": {"type": "boolean", "description": "Overwrite when conflicts exist.", "default": False}, + "vectorize": { + "type": "boolean", + "description": "Whether to trigger vectorization after import.", + "default": True, + }, + }, + "required": ["file_path", "target_uri"], + }, + min_access_level="admin", + ), +] + +TOOL_ALIASES = {"openviking_add_resource": "openviking_resource_add"} + +TOOL_DEFINITIONS = ( + READ_TOOL_DEFINITIONS + INGEST_TOOL_DEFINITIONS + MUTATE_TOOL_DEFINITIONS + ADMIN_TOOL_DEFINITIONS +) +TOOL_REQUIRED_LEVELS = { + tool["name"]: parse_access_level(tool["minAccessLevel"]) for tool in TOOL_DEFINITIONS +} class ToolArgumentError(ValueError): """Raised when MCP tool arguments are invalid.""" -def get_tool_definitions(include_write: bool = False) -> List[Dict[str, Any]]: - """Return MCP tool definitions, optionally including write tools.""" - definitions = READ_TOOL_DEFINITIONS + WRITE_TOOL_DEFINITIONS if include_write else READ_TOOL_DEFINITIONS +def get_tool_definitions(access_level: MCPAccessLevel | str = MCPAccessLevel.READONLY) -> List[Dict[str, Any]]: + """Return MCP tool definitions filtered by access level.""" + level = parse_access_level(access_level) + definitions = [ + tool for tool in TOOL_DEFINITIONS if can_access(level, parse_access_level(tool["minAccessLevel"])) + ] return [copy.deepcopy(tool) for tool in definitions] @@ -396,12 +619,70 @@ def _optional_float(arguments: Dict[str, Any], key: str) -> float | None: return float(value) -def dispatch_tool(name: str, arguments: Any, client: Any, allow_write: bool = False) -> str: +def _optional_parts(arguments: Dict[str, Any], key: str) -> list[dict] | None: + if key not in arguments: + return None + value = arguments.get(key) + if value is None: + return None + if not isinstance(value, list): + raise ToolArgumentError(f"'{key}' must be an array of objects") + if not value: + raise ToolArgumentError(f"'{key}' must not be an empty array") + for item in value: + if not isinstance(item, dict): + raise ToolArgumentError(f"'{key}' must be an array of objects") + return value + + +def _require_str_or_str_list(arguments: Dict[str, Any], key: str) -> str | list[str]: + value = arguments.get(key) + if isinstance(value, str): + if not value.strip(): + raise ToolArgumentError(f"'{key}' must be a non-empty string or non-empty string array") + return value + if isinstance(value, list): + if not value: + raise ToolArgumentError(f"'{key}' must be a non-empty string or non-empty string array") + normalized: list[str] = [] + for item in value: + if not isinstance(item, str) or not item.strip(): + raise ToolArgumentError(f"'{key}' must contain non-empty strings") + normalized.append(item) + return normalized + raise ToolArgumentError(f"'{key}' must be a non-empty string or non-empty string array") + + +def _permission_denied_error(name: str, required: MCPAccessLevel, current: MCPAccessLevel) -> str: + return _json_error( + "PERMISSION_DENIED", + f"Tool '{name}' requires access level '{access_level_name(required)}'", + details={ + "tool": name, + "required": access_level_name(required), + "current": access_level_name(current), + }, + ) + +def dispatch_tool( + name: str, + arguments: Any, + client: Any, + access_level: MCPAccessLevel | str = MCPAccessLevel.READONLY, +) -> str: """Dispatch an MCP tool call and return a JSON payload string.""" try: args = _expect_dict(arguments) + current_level = parse_access_level(access_level) + normalized_name = TOOL_ALIASES.get(name, name) + + required = TOOL_REQUIRED_LEVELS.get(normalized_name) + if required is None: + return _json_error("TOOL_NOT_FOUND", f"Unknown tool: {name}") + if not can_access(current_level, required): + return _permission_denied_error(name, required, current_level) - if name == "openviking_find": + if normalized_name == "openviking_find": query = _require_str(args, "query") target_uri = _optional_str(args, "uri", "") limit = _optional_int(args, "limit", DEFAULT_FIND_LIMIT) @@ -410,15 +691,16 @@ def dispatch_tool(name: str, arguments: Any, client: Any, allow_write: bool = Fa f"'limit' must be between 1 and {MAX_FIND_LIMIT} for openviking_find" ) threshold = _optional_float(args, "threshold") - result = client.find( - query=query, - target_uri=target_uri, - limit=limit, - score_threshold=threshold, + return _json_ok( + client.find( + query=query, + target_uri=target_uri, + limit=limit, + score_threshold=threshold, + ) ) - return _json_ok(result) - if name == "openviking_search": + if normalized_name == "openviking_search": query = _require_str(args, "query") target_uri = _optional_str(args, "uri", "") session_id = _optional_nullable_str(args, "session_id") @@ -428,16 +710,17 @@ def dispatch_tool(name: str, arguments: Any, client: Any, allow_write: bool = Fa f"'limit' must be between 1 and {MAX_FIND_LIMIT} for openviking_search" ) threshold = _optional_float(args, "threshold") - result = client.search( - query=query, - target_uri=target_uri, - session_id=session_id, - limit=limit, - score_threshold=threshold, + return _json_ok( + client.search( + query=query, + target_uri=target_uri, + session_id=session_id, + limit=limit, + score_threshold=threshold, + ) ) - return _json_ok(result) - if name == "openviking_read": + if normalized_name == "openviking_read": uri = _require_str(args, "uri") offset = _optional_int(args, "offset", 0) if offset < 0: @@ -447,37 +730,51 @@ def dispatch_tool(name: str, arguments: Any, client: Any, allow_write: bool = Fa raise ToolArgumentError( f"'limit' must be between 1 and {MAX_READ_LIMIT} for openviking_read" ) - result = client.read(uri=uri, offset=offset, limit=limit) - return _json_ok(result) + return _json_ok(client.read(uri=uri, offset=offset, limit=limit)) - if name == "openviking_ls": + if normalized_name == "openviking_ls": uri = _optional_str(args, "uri", "viking://") simple = _optional_bool(args, "simple", False) recursive = _optional_bool(args, "recursive", False) - result = client.ls(uri=uri, simple=simple, recursive=recursive, output="agent") - return _json_ok(result) + output = _optional_str(args, "output", "agent") + if output not in {"agent", "original"}: + raise ToolArgumentError("'output' must be either 'agent' or 'original'") + abs_limit = _optional_int(args, "abs_limit", 256) + if abs_limit < 0 or abs_limit > MAX_TREE_ABS_LIMIT: + raise ToolArgumentError( + f"'abs_limit' must be between 0 and {MAX_TREE_ABS_LIMIT} for openviking_ls" + ) + show_all_hidden = _optional_bool(args, "show_all_hidden", False) + node_limit = _optional_int(args, "node_limit", DEFAULT_TREE_NODE_LIMIT) + if node_limit < 1 or node_limit > MAX_TREE_NODE_LIMIT: + raise ToolArgumentError( + f"'node_limit' must be between 1 and {MAX_TREE_NODE_LIMIT} for openviking_ls" + ) + return _json_ok( + client.ls( + uri=uri, + simple=simple, + recursive=recursive, + output=output, + abs_limit=abs_limit, + show_all_hidden=show_all_hidden, + node_limit=node_limit, + ) + ) - if name == "openviking_abstract": - uri = _require_str(args, "uri") - result = client.abstract(uri=uri) - return _json_ok(result) + if normalized_name == "openviking_abstract": + return _json_ok(client.abstract(uri=_require_str(args, "uri"))) - if name == "openviking_overview": - uri = _require_str(args, "uri") - result = client.overview(uri=uri) - return _json_ok(result) + if normalized_name == "openviking_overview": + return _json_ok(client.overview(uri=_require_str(args, "uri"))) - if name == "openviking_wait_processed": - timeout = _optional_float(args, "timeout") - result = client.wait_processed(timeout=timeout) - return _json_ok(result) + if normalized_name == "openviking_wait_processed": + return _json_ok(client.wait_processed(timeout=_optional_float(args, "timeout"))) - if name == "openviking_stat": - uri = _require_str(args, "uri") - result = client.stat(uri=uri) - return _json_ok(result) + if normalized_name == "openviking_stat": + return _json_ok(client.stat(uri=_require_str(args, "uri"))) - if name == "openviking_tree": + if normalized_name == "openviking_tree": uri = _require_str(args, "uri") abs_limit = _optional_int(args, "abs_limit", DEFAULT_TREE_ABS_LIMIT) if abs_limit < 0 or abs_limit > MAX_TREE_ABS_LIMIT: @@ -490,57 +787,148 @@ def dispatch_tool(name: str, arguments: Any, client: Any, allow_write: bool = Fa raise ToolArgumentError( f"'node_limit' must be between 1 and {MAX_TREE_NODE_LIMIT} for openviking_tree" ) - result = client.tree( - uri=uri, - output="agent", - abs_limit=abs_limit, - show_all_hidden=show_all_hidden, - node_limit=node_limit, + return _json_ok( + client.tree( + uri=uri, + output="agent", + abs_limit=abs_limit, + show_all_hidden=show_all_hidden, + node_limit=node_limit, + ) ) - return _json_ok(result) - if name == "openviking_grep": - pattern = _require_str(args, "pattern") - uri = _optional_str(args, "uri", "viking://") - ignore_case = _optional_bool(args, "ignore_case", False) - result = client.grep(uri=uri, pattern=pattern, case_insensitive=ignore_case) - return _json_ok(result) + if normalized_name == "openviking_grep": + return _json_ok( + client.grep( + uri=_optional_str(args, "uri", "viking://"), + pattern=_require_str(args, "pattern"), + case_insensitive=_optional_bool(args, "ignore_case", False), + ) + ) - if name == "openviking_glob": - pattern = _require_str(args, "pattern") - uri = _optional_str(args, "uri", "viking://") - result = client.glob(pattern=pattern, uri=uri) - return _json_ok(result) - - if name == "openviking_status": - result = client.get_status() - return _json_ok(result) - - if name == "openviking_health": - healthy = bool(client.is_healthy()) - return _json_ok({"healthy": healthy}) - - if name == "openviking_add_resource": - if not allow_write: - return _json_error( - "PERMISSION_DENIED", - "Tool 'openviking_add_resource' is disabled in readonly mode", + if normalized_name == "openviking_glob": + return _json_ok( + client.glob( + pattern=_require_str(args, "pattern"), + uri=_optional_str(args, "uri", "viking://"), + ) + ) + + if normalized_name == "openviking_status": + return _json_ok(client.get_status()) + + if normalized_name == "openviking_health": + return _json_ok({"healthy": bool(client.is_healthy())}) + + if normalized_name == "openviking_session_create": + return _json_ok(client.create_session()) + + if normalized_name == "openviking_session_list": + return _json_ok(client.list_sessions()) + + if normalized_name == "openviking_session_get": + return _json_ok(client.get_session(session_id=_require_str(args, "session_id"))) + + if normalized_name == "openviking_session_delete": + session_id = _require_str(args, "session_id") + result = client.delete_session(session_id=session_id) + return _json_ok({"session_id": session_id} if result is None else result) + + if normalized_name == "openviking_session_add_message": + session_id = _require_str(args, "session_id") + role = _require_str(args, "role").strip().lower() + if role not in ALLOWED_SESSION_ROLES: + raise ToolArgumentError( + f"'role' must be one of: {', '.join(sorted(ALLOWED_SESSION_ROLES))}" + ) + content = _optional_nullable_str(args, "content") + if content is not None and not content.strip(): + raise ToolArgumentError("'content' must be a non-empty string when provided") + parts = _optional_parts(args, "parts") + if content is None and parts is None: + raise ToolArgumentError("either 'content' or 'parts' must be provided") + return _json_ok( + client.add_message( + session_id=session_id, + role=role, + content=content, + parts=parts, ) - path = _require_str(args, "path") - to = _optional_nullable_str(args, "to") + ) + + if normalized_name == "openviking_session_commit": + return _json_ok(client.commit_session(session_id=_require_str(args, "session_id"))) + + if normalized_name == "openviking_resource_add": + return _json_ok( + client.add_resource( + path=_require_str(args, "path"), + target=_optional_nullable_str(args, "to"), + reason=_optional_str(args, "reason", ""), + instruction=_optional_str(args, "instruction", ""), + wait=_optional_bool(args, "wait", False), + timeout=_optional_float(args, "timeout"), + ) + ) + + if normalized_name == "openviking_resource_add_skill": + return _json_ok( + client.add_skill( + data=_require_str(args, "data"), + wait=_optional_bool(args, "wait", False), + timeout=_optional_float(args, "timeout"), + ) + ) + + if normalized_name == "openviking_relation_list": + return _json_ok(client.relations(uri=_require_str(args, "uri"))) + + if normalized_name == "openviking_relation_link": + from_uri = _require_str(args, "from_uri") + uris = _require_str_or_str_list(args, "uris") reason = _optional_str(args, "reason", "") - instruction = _optional_str(args, "instruction", "") - wait = _optional_bool(args, "wait", False) - timeout = _optional_float(args, "timeout") - result = client.add_resource( - path=path, - target=to, - reason=reason, - instruction=instruction, - wait=wait, - timeout=timeout, + result = client.link(from_uri=from_uri, uris=uris, reason=reason) + return _json_ok({"from": from_uri, "to": uris, "reason": reason} if result is None else result) + + if normalized_name == "openviking_relation_unlink": + from_uri = _require_str(args, "from_uri") + uri = _require_str(args, "uri") + result = client.unlink(from_uri=from_uri, uri=uri) + return _json_ok({"from": from_uri, "to": uri} if result is None else result) + + if normalized_name == "openviking_fs_mkdir": + uri = _require_str(args, "uri") + result = client.mkdir(uri=uri) + return _json_ok({"uri": uri} if result is None else result) + + if normalized_name == "openviking_fs_mv": + from_uri = _require_str(args, "from_uri") + to_uri = _require_str(args, "to_uri") + result = client.mv(from_uri=from_uri, to_uri=to_uri) + return _json_ok({"from": from_uri, "to": to_uri} if result is None else result) + + if normalized_name == "openviking_fs_rm": + uri = _require_str(args, "uri") + recursive = _optional_bool(args, "recursive", False) + result = client.rm(uri=uri, recursive=recursive) + return _json_ok({"uri": uri, "recursive": recursive} if result is None else result) + + if normalized_name == "openviking_pack_export": + return _json_ok( + {"file": client.export_ovpack(uri=_require_str(args, "uri"), to=_require_str(args, "to"))} + ) + + if normalized_name == "openviking_pack_import": + return _json_ok( + { + "uri": client.import_ovpack( + file_path=_require_str(args, "file_path"), + target=_require_str(args, "target_uri"), + force=_optional_bool(args, "force", False), + vectorize=_optional_bool(args, "vectorize", True), + ) + } ) - return _json_ok(result) return _json_error("TOOL_NOT_FOUND", f"Unknown tool: {name}") diff --git a/openviking_cli/cli/commands/mcp.py b/openviking_cli/cli/commands/mcp.py index 08513889..207a24f2 100644 --- a/openviking_cli/cli/commands/mcp.py +++ b/openviking_cli/cli/commands/mcp.py @@ -6,6 +6,7 @@ import typer +from openviking.mcp.permissions import ACCESS_LEVEL_ORDER, access_level_name, parse_access_level from openviking_cli.cli.context import get_cli_context from openviking_cli.cli.errors import handle_command_error @@ -31,10 +32,19 @@ def mcp_command( "--transport", help="MCP transport mode. MVP supports only 'stdio'.", ), + access_level: str = typer.Option( + "readonly", + "--access-level", + help=( + "MCP tool access level: " + + ", ".join(ACCESS_LEVEL_ORDER) + + ". Default is readonly." + ), + ), enable_write: bool = typer.Option( False, "--enable-write/--readonly", - help="Enable write tools (default is readonly mode).", + help="Compatibility switch. --enable-write maps to --access-level mutate.", ), ) -> None: """Run OpenViking MCP server (stdio, embedded mode).""" @@ -42,13 +52,19 @@ def mcp_command( try: if transport != "stdio": raise ValueError("Only stdio transport is supported in V1") + if enable_write and access_level.strip().lower() != "readonly": + raise ValueError("Cannot use --enable-write together with --access-level") + + resolved_access_level = "mutate" if enable_write else access_level + resolved_access_level = access_level_name(parse_access_level(resolved_access_level)) + from openviking.mcp.server import run_stdio_server run_stdio_server( path=path, config=config, transport=transport, - enable_write=enable_write, + access_level=resolved_access_level, ) except Exception as exc: # noqa: BLE001 handle_command_error(cli_ctx, exc) diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index c0f9b6a6..66b8c698 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -271,6 +271,7 @@ def test_cli_mcp_requires_path(): def test_cli_mcp_help_mentions_write_toggle(): result = runner.invoke(app, ["mcp", "--help"], env={}) assert result.exit_code == 0 + assert "--access-level" in result.output assert "--enable-write" in result.output assert "--readonly" in result.output @@ -283,3 +284,23 @@ def test_cli_mcp_rejects_non_stdio(): ) assert result.exit_code == 1 assert "Only stdio transport is supported in V1" in result.output + + +def test_cli_mcp_rejects_conflicting_access_options(): + result = runner.invoke( + app, + ["mcp", "--path", "./data", "--enable-write", "--access-level", "admin"], + env={}, + ) + assert result.exit_code == 1 + assert "Cannot use --enable-write together with --access-level" in result.output + + +def test_cli_mcp_rejects_invalid_access_level(): + result = runner.invoke( + app, + ["mcp", "--path", "./data", "--access-level", "bad-level"], + env={}, + ) + assert result.exit_code == 1 + assert "Invalid access level" in result.output diff --git a/tests/mcp/test_protocol_stdio.py b/tests/mcp/test_protocol_stdio.py index c9c80c4f..07358db2 100644 --- a/tests/mcp/test_protocol_stdio.py +++ b/tests/mcp/test_protocol_stdio.py @@ -35,12 +35,12 @@ def _extract_payload(call_result) -> dict: @pytest.mark.anyio -async def test_stdio_readonly_hides_add_resource_and_health_works(): +async def test_stdio_readonly_hides_write_tools_and_health_works(): _ensure_agfs_binary_available() with tempfile.TemporaryDirectory(dir=_repo_root()) as data_path: server = StdioServerParameters( command=sys.executable, - args=["-m", "openviking", "mcp", "--path", data_path], + args=["-m", "openviking", "mcp", "--path", data_path, "--access-level", "readonly"], cwd=_repo_root(), ) @@ -49,7 +49,9 @@ async def test_stdio_readonly_hides_add_resource_and_health_works(): await session.initialize() tools_result = await session.list_tools() tool_names = {tool.name for tool in tools_result.tools} - assert "openviking_add_resource" not in tool_names + assert "openviking_resource_add" not in tool_names + assert "openviking_session_create" not in tool_names + assert "openviking_fs_mkdir" not in tool_names health_result = await session.call_tool("openviking_health", {}) assert health_result.isError is False @@ -59,12 +61,12 @@ async def test_stdio_readonly_hides_add_resource_and_health_works(): @pytest.mark.anyio -async def test_stdio_writable_includes_add_resource_tool(): +async def test_stdio_mutate_exposes_mutate_tools_but_not_admin_tools(): _ensure_agfs_binary_available() with tempfile.TemporaryDirectory(dir=_repo_root()) as data_path: server = StdioServerParameters( command=sys.executable, - args=["-m", "openviking", "mcp", "--path", data_path, "--enable-write"], + args=["-m", "openviking", "mcp", "--path", data_path, "--access-level", "mutate"], cwd=_repo_root(), ) @@ -73,4 +75,26 @@ async def test_stdio_writable_includes_add_resource_tool(): await session.initialize() tools_result = await session.list_tools() tool_names = {tool.name for tool in tools_result.tools} - assert "openviking_add_resource" in tool_names + assert "openviking_resource_add" in tool_names + assert "openviking_fs_mkdir" in tool_names + assert "openviking_fs_rm" not in tool_names + assert "openviking_pack_import" not in tool_names + + +@pytest.mark.anyio +async def test_stdio_admin_exposes_admin_tools(): + _ensure_agfs_binary_available() + with tempfile.TemporaryDirectory(dir=_repo_root()) as data_path: + server = StdioServerParameters( + command=sys.executable, + args=["-m", "openviking", "mcp", "--path", data_path, "--access-level", "admin"], + cwd=_repo_root(), + ) + + async with stdio_client(server) as (read_stream, write_stream): + async with ClientSession(read_stream, write_stream) as session: + await session.initialize() + tools_result = await session.list_tools() + tool_names = {tool.name for tool in tools_result.tools} + assert "openviking_fs_rm" in tool_names + assert "openviking_pack_import" in tool_names diff --git a/tests/mcp/test_server.py b/tests/mcp/test_server.py index 7ae35af1..798f11ec 100644 --- a/tests/mcp/test_server.py +++ b/tests/mcp/test_server.py @@ -17,7 +17,7 @@ def search(self, query, target_uri="", session_id=None, limit=10, score_threshol def read(self, uri, offset=0, limit=-1): return "hello" - def ls(self, uri="viking://", simple=False, recursive=False, output="agent"): + def ls(self, uri="viking://", simple=False, recursive=False, output="agent", **kwargs): return [{"uri": "viking://resources"}] def abstract(self, uri): @@ -26,45 +26,91 @@ def abstract(self, uri): def overview(self, uri): return "ov" - def add_resource( - self, - path, - target=None, - reason="", - instruction="", - wait=False, - timeout=None, - ): + def wait_processed(self, timeout=None): + return {"pending": 0} + + def stat(self, uri): + return {"uri": uri} + + def tree(self, uri, output="agent", abs_limit=128, show_all_hidden=False, node_limit=1000): + return {"uri": uri, "children": []} + + def grep(self, uri, pattern, case_insensitive=False): + return {"matches": []} + + def glob(self, pattern, uri="viking://"): + return {"matches": []} + + def get_status(self): + return {"healthy": True} + + def is_healthy(self): + return True + + def list_sessions(self): + return [{"session_id": "s1"}] + + def get_session(self, session_id): + return {"session_id": session_id} + + def relations(self, uri): + return [{"uri": uri, "reason": "ref"}] + + def create_session(self): + return {"session_id": "s1"} + + def add_message(self, session_id, role, content=None, parts=None): + return {"session_id": session_id} + + def commit_session(self, session_id): + return {"session_id": session_id} + + def add_resource(self, path, target=None, reason="", instruction="", wait=False, timeout=None): return {"root_uri": "viking://resources/demo"} + def add_skill(self, data, wait=False, timeout=None): + return {"root_uri": "viking://agent/skills/demo"} -def test_list_tools_returns_v1_read_definitions(): - adapter = OpenVikingMCPAdapter(_StubClient()) - tools = adapter.list_tools() - names = {tool["name"] for tool in tools} + def link(self, from_uri, uris, reason=""): + return None + + def unlink(self, from_uri, uri): + return None + + def mkdir(self, uri): + return None + + def mv(self, from_uri, to_uri): + return None + + def delete_session(self, session_id): + return None + + def rm(self, uri, recursive=False): + return None + + def export_ovpack(self, uri, to): + return to + + def import_ovpack(self, file_path, target, force=False, vectorize=True): + return "viking://resources/imported" - assert "openviking_find" in names - assert "openviking_search" in names - assert "openviking_read" in names - assert "openviking_ls" in names - assert "openviking_abstract" in names - assert "openviking_overview" in names - assert "openviking_wait_processed" in names - assert "openviking_stat" in names - assert "openviking_tree" in names - assert "openviking_grep" in names - assert "openviking_glob" in names - assert "openviking_status" in names - assert "openviking_health" in names - assert "openviking_add_resource" not in names +def test_list_tools_filters_by_access_level(): + readonly = OpenVikingMCPAdapter(_StubClient(), access_level="readonly") + mutate = OpenVikingMCPAdapter(_StubClient(), access_level="mutate") + admin = OpenVikingMCPAdapter(_StubClient(), access_level="admin") -def test_list_tools_includes_write_tool_when_enabled(): - adapter = OpenVikingMCPAdapter(_StubClient(), allow_write=True) - tools = adapter.list_tools() - names = {tool["name"] for tool in tools} + readonly_names = {tool["name"] for tool in readonly.list_tools()} + mutate_names = {tool["name"] for tool in mutate.list_tools()} + admin_names = {tool["name"] for tool in admin.list_tools()} - assert "openviking_add_resource" in names + assert "openviking_find" in readonly_names + assert "openviking_session_create" not in readonly_names + assert "openviking_fs_mkdir" in mutate_names + assert "openviking_fs_rm" not in mutate_names + assert "openviking_fs_rm" in admin_names + assert "openviking_pack_import" in admin_names def test_call_tool_returns_json_text_payload(): @@ -76,9 +122,9 @@ def test_call_tool_returns_json_text_payload(): assert body["result"] == "hello" -def test_call_tool_write_is_denied_in_readonly_mode(): - adapter = OpenVikingMCPAdapter(_StubClient(), allow_write=False) - payload = adapter.call_tool("openviking_add_resource", {"path": "./data"}) +def test_call_tool_write_is_denied_when_permission_insufficient(): + adapter = OpenVikingMCPAdapter(_StubClient(), access_level="ingest") + payload = adapter.call_tool("openviking_fs_rm", {"uri": "viking://resources/x"}) body = json.loads(payload) assert body["ok"] is False diff --git a/tests/mcp/test_tools.py b/tests/mcp/test_tools.py index a89d78e8..af405d44 100644 --- a/tests/mcp/test_tools.py +++ b/tests/mcp/test_tools.py @@ -6,6 +6,7 @@ import pytest +from openviking.mcp.permissions import MCPAccessLevel from openviking.mcp.tools import dispatch_tool, get_tool_definitions @@ -25,8 +26,19 @@ def read(self, uri, offset=0, limit=-1): self.calls.append(("read", uri, offset, limit)) return "hello" - def ls(self, uri="viking://", simple=False, recursive=False, output="agent"): - self.calls.append(("ls", uri, simple, recursive, output)) + def ls( + self, + uri="viking://", + simple=False, + recursive=False, + output="agent", + abs_limit=256, + show_all_hidden=False, + node_limit=1000, + ): + self.calls.append( + ("ls", uri, simple, recursive, output, abs_limit, show_all_hidden, node_limit) + ) return [{"uri": "viking://resources"}] def abstract(self, uri): @@ -65,6 +77,30 @@ def is_healthy(self): self.calls.append(("health",)) return True + def create_session(self): + self.calls.append(("create_session",)) + return {"session_id": "s1"} + + def list_sessions(self): + self.calls.append(("list_sessions",)) + return [{"session_id": "s1"}] + + def get_session(self, session_id): + self.calls.append(("get_session", session_id)) + return {"session_id": session_id} + + def delete_session(self, session_id): + self.calls.append(("delete_session", session_id)) + return None + + def add_message(self, session_id, role, content=None, parts=None): + self.calls.append(("add_message", session_id, role, content, parts)) + return {"session_id": session_id} + + def commit_session(self, session_id): + self.calls.append(("commit_session", session_id)) + return {"session_id": session_id} + def add_resource( self, path, @@ -77,205 +113,201 @@ def add_resource( self.calls.append(("add_resource", path, target, reason, instruction, wait, timeout)) return {"root_uri": "viking://resources/demo"} + def add_skill(self, data, wait=False, timeout=None): + self.calls.append(("add_skill", data, wait, timeout)) + return {"root_uri": "viking://agent/skills/demo"} -def _tool_names(include_write: bool = False): - return {tool["name"] for tool in get_tool_definitions(include_write=include_write)} + def relations(self, uri): + self.calls.append(("relations", uri)) + return [{"uri": "viking://resources/x", "reason": "ref"}] + def link(self, from_uri, uris, reason=""): + self.calls.append(("link", from_uri, uris, reason)) + return None -def _payload(tool_name, arguments, client, allow_write=False): - return json.loads(dispatch_tool(tool_name, arguments, client, allow_write=allow_write)) + def unlink(self, from_uri, uri): + self.calls.append(("unlink", from_uri, uri)) + return None + def mkdir(self, uri): + self.calls.append(("mkdir", uri)) + return None -def test_tool_definitions_have_v1_read_set(): - names = _tool_names() - assert "openviking_find" in names - assert "openviking_search" in names - assert "openviking_read" in names - assert "openviking_ls" in names - assert "openviking_abstract" in names - assert "openviking_overview" in names - assert "openviking_wait_processed" in names - assert "openviking_stat" in names - assert "openviking_tree" in names - assert "openviking_grep" in names - assert "openviking_glob" in names - assert "openviking_status" in names - assert "openviking_health" in names - assert "openviking_add_resource" not in names + def mv(self, from_uri, to_uri): + self.calls.append(("mv", from_uri, to_uri)) + return None + def rm(self, uri, recursive=False): + self.calls.append(("rm", uri, recursive)) + return None -def test_get_tool_definitions_respects_write_flag(): - read_only_names = _tool_names(include_write=False) - writable_names = _tool_names(include_write=True) + def export_ovpack(self, uri, to): + self.calls.append(("export_ovpack", uri, to)) + return to - assert "openviking_add_resource" not in read_only_names - assert "openviking_add_resource" in writable_names + def import_ovpack(self, file_path, target, force=False, vectorize=True): + self.calls.append(("import_ovpack", file_path, target, force, vectorize)) + return "viking://resources/imported" -def test_dispatch_find_success(): - client = _FakeClient() - body = _payload("openviking_find", {"query": "what is openviking"}, client) +def _tool_names(access_level=MCPAccessLevel.READONLY): + return {tool["name"] for tool in get_tool_definitions(access_level=access_level)} - assert body["ok"] is True - assert body["result"]["total"] == 0 - assert client.calls[0] == ("find", "what is openviking", "", 10, None) +def _payload(tool_name, arguments, client, access_level=MCPAccessLevel.READONLY): + return json.loads(dispatch_tool(tool_name, arguments, client, access_level=access_level)) -def test_dispatch_search_success(): - client = _FakeClient() - body = _payload( - "openviking_search", - {"query": "design", "uri": "viking://resources", "session_id": "s1", "threshold": 0.5}, - client, - ) - assert body["ok"] is True - assert client.calls[0] == ("search", "design", "viking://resources", "s1", 10, 0.5) +def test_get_tool_definitions_filters_by_access_level(): + readonly = _tool_names("readonly") + ingest = _tool_names("ingest") + mutate = _tool_names("mutate") + admin = _tool_names("admin") + + assert "openviking_find" in readonly + assert "openviking_session_list" in readonly + assert "openviking_resource_add" not in readonly + + assert "openviking_session_create" in ingest + assert "openviking_resource_add" in ingest + assert "openviking_fs_mkdir" not in ingest + + assert "openviking_fs_mkdir" in mutate + assert "openviking_relation_link" in mutate + assert "openviking_fs_rm" not in mutate + assert "openviking_fs_rm" in admin + assert "openviking_pack_import" in admin + assert "openviking_session_delete" in admin -def test_dispatch_read_applies_default_limit_cap(): + +def test_dispatch_alias_add_resource_works_with_ingest_access(): client = _FakeClient() - _payload("openviking_read", {"uri": "viking://resources/a.md"}, client) - assert client.calls[0] == ("read", "viking://resources/a.md", 0, 200) + body = _payload("openviking_add_resource", {"path": "./demo"}, client, access_level="ingest") + + assert body["ok"] is True + assert client.calls[0] == ("add_resource", "./demo", None, "", "", False, None) -def test_dispatch_read_rejects_oversized_limit(): +def test_dispatch_denies_when_access_level_is_insufficient(): client = _FakeClient() - body = _payload("openviking_read", {"uri": "viking://resources/a.md", "limit": 5000}, client) + body = _payload("openviking_session_delete", {"session_id": "s1"}, client, access_level="mutate") assert body["ok"] is False - assert body["error"]["code"] == "INVALID_ARGUMENT" + assert body["error"]["code"] == "PERMISSION_DENIED" + assert body["error"]["details"]["required"] == "admin" + assert body["error"]["details"]["current"] == "mutate" @pytest.mark.parametrize( - "args,key,message", + "tool_name,args,expected_call,level", [ - ({"uri": "viking://resources/a.md", "limit": True}, "limit", "'limit' must be an integer"), - ({"uri": "viking://resources/a.md", "offset": False}, "offset", "'offset' must be an integer"), + ("openviking_find", {"query": "what is openviking"}, ("find", "what is openviking", "", 10, None), "readonly"), + ("openviking_search", {"query": "design", "session_id": "s1"}, ("search", "design", "", "s1", 10, None), "readonly"), + ("openviking_read", {"uri": "viking://resources/a.md"}, ("read", "viking://resources/a.md", 0, 200), "readonly"), ( - {"query": "x", "threshold": True}, - "threshold", - "'threshold' must be a number", + "openviking_ls", + {"uri": "viking://resources", "output": "original", "abs_limit": 10, "node_limit": 20}, + ("ls", "viking://resources", False, False, "original", 10, False, 20), + "readonly", ), + ("openviking_abstract", {"uri": "viking://resources"}, ("abstract", "viking://resources"), "readonly"), + ("openviking_overview", {"uri": "viking://resources"}, ("overview", "viking://resources"), "readonly"), + ("openviking_wait_processed", {"timeout": 5.0}, ("wait_processed", 5.0), "readonly"), + ("openviking_stat", {"uri": "viking://resources/a.md"}, ("stat", "viking://resources/a.md"), "readonly"), + ( + "openviking_tree", + {"uri": "viking://resources", "abs_limit": 256, "show_all_hidden": True, "node_limit": 50}, + ("tree", "viking://resources", "agent", 256, True, 50), + "readonly", + ), + ("openviking_grep", {"uri": "viking://resources", "pattern": "OpenViking", "ignore_case": True}, ("grep", "viking://resources", "OpenViking", True), "readonly"), + ("openviking_glob", {"pattern": "*.md", "uri": "viking://resources"}, ("glob", "*.md", "viking://resources"), "readonly"), + ("openviking_status", {}, ("status",), "readonly"), + ("openviking_health", {}, ("health",), "readonly"), + ("openviking_session_create", {}, ("create_session",), "ingest"), + ("openviking_session_list", {}, ("list_sessions",), "readonly"), + ("openviking_session_get", {"session_id": "s1"}, ("get_session", "s1"), "readonly"), + ("openviking_session_add_message", {"session_id": "s1", "role": "user", "content": "hello"}, ("add_message", "s1", "user", "hello", None), "ingest"), + ("openviking_session_commit", {"session_id": "s1"}, ("commit_session", "s1"), "ingest"), + ("openviking_resource_add", {"path": "./demo", "to": "viking://resources/target"}, ("add_resource", "./demo", "viking://resources/target", "", "", False, None), "ingest"), + ("openviking_resource_add_skill", {"data": "./skills/demo"}, ("add_skill", "./skills/demo", False, None), "ingest"), + ("openviking_relation_list", {"uri": "viking://resources/a.md"}, ("relations", "viking://resources/a.md"), "readonly"), + ("openviking_relation_link", {"from_uri": "viking://a", "uris": ["viking://b"], "reason": "ref"}, ("link", "viking://a", ["viking://b"], "ref"), "mutate"), + ("openviking_relation_unlink", {"from_uri": "viking://a", "uri": "viking://b"}, ("unlink", "viking://a", "viking://b"), "mutate"), + ("openviking_fs_mkdir", {"uri": "viking://resources/newdir"}, ("mkdir", "viking://resources/newdir"), "mutate"), + ("openviking_fs_mv", {"from_uri": "viking://a", "to_uri": "viking://b"}, ("mv", "viking://a", "viking://b"), "mutate"), + ("openviking_fs_rm", {"uri": "viking://resources/newdir", "recursive": True}, ("rm", "viking://resources/newdir", True), "admin"), + ("openviking_pack_export", {"uri": "viking://resources/a", "to": "./a.ovpack"}, ("export_ovpack", "viking://resources/a", "./a.ovpack"), "admin"), + ("openviking_pack_import", {"file_path": "./a.ovpack", "target_uri": "viking://resources/import", "force": True, "vectorize": False}, ("import_ovpack", "./a.ovpack", "viking://resources/import", True, False), "admin"), ], ) -def test_dispatch_rejects_boolean_for_numeric_fields(args, key, message): +def test_dispatch_tools_success(tool_name, args, expected_call, level): client = _FakeClient() - body = _payload("openviking_search" if key == "threshold" else "openviking_read", args, client) + body = _payload(tool_name, args, client, access_level=level) - assert body["ok"] is False - assert body["error"]["code"] == "INVALID_ARGUMENT" - assert body["error"]["message"] == message + assert body["ok"] is True + assert client.calls[0] == expected_call -def test_dispatch_find_requires_query(): +def test_dispatch_add_message_requires_content_or_parts(): client = _FakeClient() - body = _payload("openviking_find", {}, client) - + body = _payload( + "openviking_session_add_message", + {"session_id": "s1", "role": "user"}, + client, + access_level="ingest", + ) assert body["ok"] is False assert body["error"]["code"] == "INVALID_ARGUMENT" -def test_dispatch_add_resource_is_denied_in_readonly_mode(): +@pytest.mark.parametrize( + "args,message", + [ + ({"session_id": "s1", "role": "bot", "content": "x"}, "'role' must be one of"), + ({"session_id": "s1", "role": "user", "parts": []}, "'parts' must not be an empty array"), + ({"session_id": "s1", "role": "user", "parts": ["x"]}, "'parts' must be an array of objects"), + ], +) +def test_dispatch_add_message_rejects_invalid_payload(args, message): client = _FakeClient() - body = _payload("openviking_add_resource", {"path": "./demo"}, client, allow_write=False) - + body = _payload("openviking_session_add_message", args, client, access_level="ingest") assert body["ok"] is False - assert body["error"]["code"] == "PERMISSION_DENIED" - assert client.calls == [] - - -def test_dispatch_add_resource_success_when_write_enabled(): - client = _FakeClient() - body = _payload( - "openviking_add_resource", - { - "path": "./demo", - "to": "viking://resources/target", - "reason": "import", - "instruction": "keep structure", - "wait": True, - "timeout": 30.5, - }, - client, - allow_write=True, - ) - - assert body["ok"] is True - assert body["result"]["root_uri"] == "viking://resources/demo" - assert client.calls[0] == ( - "add_resource", - "./demo", - "viking://resources/target", - "import", - "keep structure", - True, - 30.5, - ) + assert body["error"]["code"] == "INVALID_ARGUMENT" + assert message in body["error"]["message"] @pytest.mark.parametrize( - "tool_name,args,expected_call", + "args,message", [ - ( - "openviking_ls", - {"uri": "viking://resources", "simple": True, "recursive": True}, - ("ls", "viking://resources", True, True, "agent"), - ), - ("openviking_abstract", {"uri": "viking://resources"}, ("abstract", "viking://resources")), - ("openviking_overview", {"uri": "viking://resources"}, ("overview", "viking://resources")), - ("openviking_wait_processed", {"timeout": 5.0}, ("wait_processed", 5.0)), - ("openviking_stat", {"uri": "viking://resources/a.md"}, ("stat", "viking://resources/a.md")), - ( - "openviking_tree", - { - "uri": "viking://resources", - "abs_limit": 256, - "show_all_hidden": True, - "node_limit": 50, - }, - ("tree", "viking://resources", "agent", 256, True, 50), - ), - ( - "openviking_grep", - {"uri": "viking://resources", "pattern": "OpenViking", "ignore_case": True}, - ("grep", "viking://resources", "OpenViking", True), - ), - ( - "openviking_glob", - {"pattern": "*.md", "uri": "viking://resources"}, - ("glob", "*.md", "viking://resources"), - ), - ("openviking_status", {}, ("status",)), - ("openviking_health", {}, ("health",)), + ({"from_uri": "viking://a", "uris": []}, "non-empty string array"), + ({"from_uri": "viking://a", "uris": [""]}, "contain non-empty strings"), ], ) -def test_dispatch_v1_tools_success(tool_name, args, expected_call): +def test_dispatch_link_rejects_invalid_uris(args, message): client = _FakeClient() - body = _payload(tool_name, args, client) - - assert body["ok"] is True - assert client.calls[0] == expected_call - if tool_name == "openviking_health": - assert body["result"]["healthy"] is True + body = _payload("openviking_relation_link", args, client, access_level="mutate") + assert body["ok"] is False + assert body["error"]["code"] == "INVALID_ARGUMENT" + assert message in body["error"]["message"] @pytest.mark.parametrize( - "args,field", + "tool_name,args,error_message", [ - ({"uri": "viking://resources", "abs_limit": -1}, "abs_limit"), - ({"uri": "viking://resources", "abs_limit": 5000}, "abs_limit"), - ({"uri": "viking://resources", "node_limit": 0}, "node_limit"), - ({"uri": "viking://resources", "node_limit": 6000}, "node_limit"), + ("openviking_read", {"uri": "viking://resources/a.md", "limit": True}, "'limit' must be an integer"), + ("openviking_search", {"query": "x", "threshold": True}, "'threshold' must be a number"), + ("openviking_ls", {"output": "invalid"}, "'output' must be either 'agent' or 'original'"), ], ) -def test_dispatch_tree_rejects_out_of_range_limits(args, field): +def test_dispatch_rejects_invalid_argument_types(tool_name, args, error_message): client = _FakeClient() - body = _payload("openviking_tree", args, client) + body = _payload(tool_name, args, client) assert body["ok"] is False assert body["error"]["code"] == "INVALID_ARGUMENT" - assert field in body["error"]["message"] + assert error_message in body["error"]["message"] def test_dispatch_unknown_tool_returns_error():