diff --git a/CHANGELOG.md b/CHANGELOG.md index a39408c..a7e860c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 0.3.15 + +- Reported ambiguous MCP resource URIs instead of reading from the first matching server when `server` is omitted. + ## 0.3.14 - Detected colliding MCP tool names after sanitization and skipped ambiguous tools instead of exposing duplicate provider-facing names. diff --git a/rv.toml b/rv.toml index 1b671e3..0e17a21 100644 --- a/rv.toml +++ b/rv.toml @@ -1,6 +1,6 @@ [package] name = "rook" -version = "0.3.14" +version = "0.3.15" edition = "v2" [dependencies] diff --git a/src/mcp/manager.rv b/src/mcp/manager.rv index 9f89b17..8b9ea1f 100644 --- a/src/mcp/manager.rv +++ b/src/mcp/manager.rv @@ -45,6 +45,12 @@ struct Route { tool: String, } +struct ResourceRoute { + ok: Bool, + server: String, + error: String, +} + // Connect to every configured server once. Safe to call repeatedly; only the // first call connects (the handshakes run outside the lock). fun ensure_started() { @@ -260,16 +266,13 @@ fun read_resource(server: String, uri: String) -> String { } ensure_started() let clients = _clients() - return match _resource_owner(clients, server, uri) { - Some(c) -> { - let resp = c.call("resources/read", "{\"uri\":${enc_str(uri)}}", CALL_TIMEOUT) - if resp.ok { - format_resource(resp.result) - } else { - "error: ${resp.error}" - } - }, - None -> "error: no connected MCP server exposes the resource '${uri}'", + let route = resource_route(clients, server, uri) + if !route.ok { + return route.error + } + return match _find(clients, route.server) { + Some(c) -> _read_resource_from(c, uri), + None -> "error: MCP server '${route.server}' is not connected", } } @@ -348,26 +351,91 @@ fun _msg_text(m: JsonValue) -> String { return as_str(content) } -fun _resource_owner(clients: List, server: String, uri: String) -> Option { +fun resource_route(clients: List, server: String, uri: String) -> ResourceRoute { + if uri.trim() == "" { + return ResourceRoute { ok: false, server: "", error: "error: a resource uri is required" } + } + if server != "" { + return match _find(clients, server) { + Some(c) -> { + if _has_resource(c, uri) { + ResourceRoute { ok: true, server: c.name, error: "" } + } else { + ResourceRoute { + ok: false, + server: "", + error: "error: MCP server '${server}' does not expose the resource '${uri}'", + } + } + }, + None -> ResourceRoute { + ok: false, + server: "", + error: "error: MCP server '${server}' is not connected", + }, + } + } + let owners = _resource_owners(clients, uri) + if owners.len() == 1 { + return ResourceRoute { ok: true, server: owners[0].name, error: "" } + } + if owners.len() > 1 { + return ResourceRoute { + ok: false, + server: "", + error: "error: MCP resource '${uri}' is ambiguous across servers ${_client_names(owners)}; retry with the server argument", + } + } + return ResourceRoute { + ok: false, + server: "", + error: "error: no connected MCP server exposes the resource '${uri}'", + } +} + +fun _read_resource_from(c: McpClient, uri: String) -> String { + let resp = c.call("resources/read", "{\"uri\":${enc_str(uri)}}", CALL_TIMEOUT) + if resp.ok { + return format_resource(resp.result) + } + return "error: ${resp.error}" +} + +fun _resource_owners(clients: List, uri: String) -> List { + let out: List = [] let i = 0 while i < clients.len() { let c = clients[i] - if server != "" { - if c.name == server { - return Some(c) - } - } else { - let j = 0 - while j < c.resources.len() { - if c.resources[j].uri == uri { - return Some(c) - } - j = j + 1 - } + if c.ok && _has_resource(c, uri) { + out.push(c) } i = i + 1 } - return None + return out +} + +fun _has_resource(c: McpClient, uri: String) -> Bool { + let j = 0 + while j < c.resources.len() { + if c.resources[j].uri == uri { + return true + } + j = j + 1 + } + return false +} + +fun _client_names(clients: List) -> String { + let out = "" + let i = 0 + while i < clients.len() { + if i > 0 { + out = out.concat(", ") + } + out = out.concat(clients[i].name) + i = i + 1 + } + return out } fun _any_resources(clients: List) -> Bool { diff --git a/src/mcp/manager_test.rv b/src/mcp/manager_test.rv index 287e803..ac4e539 100644 --- a/src/mcp/manager_test.rv +++ b/src/mcp/manager_test.rv @@ -3,7 +3,7 @@ import std/json { parse, JsonValue } import std/test { assert_eq_str, assert_eq_int, assert_true } import "./client" { McpClient, McpTool, McpResource, McpPrompt } import "./config" { ServerSpec } -import "./manager" { Route, exposed_name, tools_from, route_in, format_tool_result, is_mcp_tool, format_resource, format_prompt, status_from } +import "./manager" { Route, exposed_name, tools_from, route_in, format_tool_result, is_mcp_tool, format_resource, format_prompt, status_from, resource_route } fun _client() -> McpClient { let tools = [ @@ -42,6 +42,23 @@ fun _tool_client(server: String, tool: String) -> McpClient { } } +fun _resource_client(server: String, uri: String) -> McpClient { + let tools: List = [] + let resources = [McpResource { uri, name: "doc", description: "", mime: "text/plain" }] + let prompts: List = [] + return McpClient { + name: server, + server_name: "", + server_version: "", + tools, + resources, + prompts, + ok: true, + error: "", + conn: None, + } +} + fun test_exposed_name_namespaces_and_sanitizes() { assert_eq_str(exposed_name("fs", "read_file"), "mcp__fs__read_file") // A space and punctuation become underscores so the name is valid. @@ -98,6 +115,52 @@ fun test_format_resource_joins_text_contents() { assert_true(format_resource(_json("{\"contents\": []}")).contains("no text contents")) } +fun test_resource_route_allows_unique_uri_without_server() { + let route = resource_route( + [_resource_client("docs", "file:///README.md")], + "", + "file:///README.md", + ) + assert_true(route.ok) + assert_eq_str(route.server, "docs") +} + +fun test_resource_route_reports_missing_uri() { + let route = resource_route( + [_resource_client("docs", "file:///README.md")], + "", + "file:///missing.md", + ) + assert_true(!route.ok) + assert_true(route.error.contains("no connected MCP server exposes")) +} + +fun test_resource_route_reports_duplicate_uri_without_server() { + let route = resource_route( + [ + _resource_client("docs", "file:///README.md"), + _resource_client("cache", "file:///README.md"), + ], + "", + "file:///README.md", + ) + assert_true(!route.ok) + assert_true(route.error.contains("ambiguous")) + assert_true(route.error.contains("docs")) + assert_true(route.error.contains("cache")) + assert_true(route.error.contains("server argument")) +} + +fun test_resource_route_validates_explicit_server_uri() { + let route = resource_route( + [_resource_client("docs", "file:///README.md")], + "docs", + "file:///missing.md", + ) + assert_true(!route.ok) + assert_true(route.error.contains("does not expose")) +} + fun test_status_from_shows_each_server_state() { let no_res: List = [] let no_pr: List = []