diff --git a/Tools/SqlTools.cs b/Tools/SqlTools.cs index 5275399..b79dc17 100644 --- a/Tools/SqlTools.cs +++ b/Tools/SqlTools.cs @@ -1,4 +1,4 @@ -using ModelContextProtocol; +using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; using SqlMcp.Models; using SqlMcp.Services; @@ -23,7 +23,7 @@ public sealed class SqlTools(QueryService svc) [McpServerTool(Name = "list_tables", ReadOnly = true, Idempotent = true)] [Description("List the tables and views the database login can read, as JSON. System schemas are left out unless named.")] - public async Task ListTables( + public async Task ListTables( [Description("Database name from list_databases.")] string dbName, [Description("Only list this schema (on MySQL, the database). Optional.")] string? schema = null, CancellationToken cancellationToken = default) => @@ -31,7 +31,7 @@ public async Task ListTables( [McpServerTool(Name = "describe_table", ReadOnly = true, Idempotent = true)] [Description("Describe a table or view's columns: name, data type, nullability and primary key, as JSON.")] - public async Task DescribeTable( + public async Task DescribeTable( [Description("Database name from list_databases.")] string dbName, [Description("Table or view name, optionally schema-qualified (\"sales.orders\"). Case-insensitive.")] string table, [Description("Schema containing the table. Optional; needed only when the name exists in more than one schema.")] string? schema = null, @@ -42,7 +42,7 @@ public async Task DescribeTable( [Description("Execute a read-only SELECT query on the specified database and return the columns and rows as JSON. " + "Pass values through parameters rather than writing them into the SQL: they are bound by the database, " + "so quotes in values need no escaping and can't change the query.")] - public async Task QueryDatabase( + public async Task QueryDatabase( [Description("Database name from list_databases.")] string dbName, [Description("SELECT query to execute. Refer to parameters as @name on SQL Server, Postgres and MySQL, and as :name on Oracle.")] string sql, [Description("Values for the query's placeholders, by name without the @ or :, e.g. {\"customer\": \"O'Brien\", \"minTotal\": 100}. " + @@ -50,12 +50,11 @@ public async Task QueryDatabase( CancellationToken cancellationToken = default) => Serialize(await svc.ExecuteAsync(new QueryRequest(dbName, sql, parameters), cancellationToken)); - private static string Serialize(QueryService.Result result) + // An expected failure (unknown database, rejected statement, refused login) is an isError result, not an + // exception: the SDK would log a thrown one as unhandled, with a stack trace, and prefix its message. + private static CallToolResult Serialize(QueryService.Result result) => new() { - // McpException messages are returned to the client as an isError tool result. - if (result.IsError) - throw new McpException(result.Error!); - - return JsonSerializer.Serialize(result.Value, JsonOptions); - } + Content = [new TextContentBlock { Text = result.IsError ? result.Error! : JsonSerializer.Serialize(result.Value, JsonOptions) }], + IsError = result.IsError, + }; } diff --git a/tests/SqlMcp.Tests/McpToolsTests.cs b/tests/SqlMcp.Tests/McpToolsTests.cs index cdaf9df..5a0a192 100644 --- a/tests/SqlMcp.Tests/McpToolsTests.cs +++ b/tests/SqlMcp.Tests/McpToolsTests.cs @@ -6,23 +6,26 @@ namespace SqlMcp.Tests; public class McpToolsTests { - private static async Task ListToolsAsync() + private static async Task ListToolsAsync() => + (await PostAsync("""{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{}}""")).GetProperty("tools"); + + private static async Task PostAsync(string body) { using var client = new WebApplicationFactory().CreateClient(); var request = new HttpRequestMessage(HttpMethod.Post, "/mcp") { - Content = new StringContent("""{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{}}""", Encoding.UTF8, "application/json") + Content = new StringContent(body, Encoding.UTF8, "application/json") }; request.Headers.Accept.ParseAdd("application/json"); request.Headers.Accept.ParseAdd("text/event-stream"); var response = await client.SendAsync(request); response.EnsureSuccessStatusCode(); - var body = await response.Content.ReadAsStringAsync(); + var text = await response.Content.ReadAsStringAsync(); // The response is either plain JSON or a single server-sent event. - var json = body.Split('\n').FirstOrDefault(l => l.StartsWith("data: "))?["data: ".Length..] ?? body; - return JsonDocument.Parse(json).RootElement.GetProperty("result").GetProperty("tools"); + var json = text.Split('\n').FirstOrDefault(l => l.StartsWith("data: "))?["data: ".Length..] ?? text; + return JsonDocument.Parse(json).RootElement.GetProperty("result"); } [Theory] @@ -40,4 +43,14 @@ public async Task Advertises_read_only_tools_with_their_required_arguments(strin var actual = schema.TryGetProperty("required", out var r) ? r.EnumerateArray().Select(e => e.GetString()!).ToArray() : []; Assert.Equal(required.Order(), actual.Order()); } + + [Fact] + public async Task A_refused_call_is_an_error_result_with_just_the_reason() + { + var result = await PostAsync("""{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"query_database","arguments":{"dbName":"nope","sql":"SELECT 1"}}}"""); + + Assert.True(result.GetProperty("isError").GetBoolean()); + var text = result.GetProperty("content")[0].GetProperty("text").GetString()!; + Assert.StartsWith("Unknown database 'nope'", text); + } }