Improve progress reporting (#145)
* Improve progress reporting - Enable injecting an `IProgress<>` into a tool call to report progress when the client has supplied a progress token - Add a custom type for ProgressToken and ProgressNotification to map to the schema - Simplify RequestId to match ProgressToken's shape - Serialize StdioClientTransport's SendMessageAsync implementation - Add strongly-typed names for all request methods - Change all request IDs created by McpJsonRpcEndpoint to include a guid for that endpoint * Address PR feedback
This commit is contained in:
parent
0294c574a6
commit
2c75acded8
6 changed files with 627 additions and 228 deletions
|
|
@ -6,9 +6,11 @@ using ModelContextProtocol.Client;
|
|||
using ModelContextProtocol.Configuration;
|
||||
using ModelContextProtocol.Protocol.Messages;
|
||||
using ModelContextProtocol.Protocol.Transport;
|
||||
using ModelContextProtocol.Protocol.Types;
|
||||
using ModelContextProtocol.Server;
|
||||
using ModelContextProtocol.Tests.Transport;
|
||||
using ModelContextProtocol.Tests.Utils;
|
||||
using System.Collections.Concurrent;
|
||||
using System.ComponentModel;
|
||||
using System.IO.Pipelines;
|
||||
using System.Text.Json;
|
||||
|
|
@ -90,7 +92,7 @@ public class McpServerBuilderExtensionsToolsTests : LoggedTest, IAsyncDisposable
|
|||
IMcpClient client = await CreateMcpClientForServer();
|
||||
|
||||
var tools = await client.ListToolsAsync(TestContext.Current.CancellationToken);
|
||||
Assert.Equal(11, tools.Count);
|
||||
Assert.Equal(12, tools.Count);
|
||||
|
||||
McpClientTool echoTool = tools.First(t => t.Name == "Echo");
|
||||
Assert.Equal("Echo", echoTool.Name);
|
||||
|
|
@ -137,7 +139,7 @@ public class McpServerBuilderExtensionsToolsTests : LoggedTest, IAsyncDisposable
|
|||
cancellationToken: TestContext.Current.CancellationToken))
|
||||
{
|
||||
var tools = await client.ListToolsAsync(TestContext.Current.CancellationToken);
|
||||
Assert.Equal(11, tools.Count);
|
||||
Assert.Equal(12, tools.Count);
|
||||
|
||||
McpClientTool echoTool = tools.First(t => t.Name == "Echo");
|
||||
Assert.Equal("Echo", echoTool.Name);
|
||||
|
|
@ -164,10 +166,10 @@ public class McpServerBuilderExtensionsToolsTests : LoggedTest, IAsyncDisposable
|
|||
IMcpClient client = await CreateMcpClientForServer();
|
||||
|
||||
var tools = await client.ListToolsAsync(TestContext.Current.CancellationToken);
|
||||
Assert.Equal(11, tools.Count);
|
||||
Assert.Equal(12, tools.Count);
|
||||
|
||||
Channel<JsonRpcNotification> listChanged = Channel.CreateUnbounded<JsonRpcNotification>();
|
||||
client.AddNotificationHandler("notifications/tools/list_changed", notification =>
|
||||
client.AddNotificationHandler(NotificationMethods.ToolListChangedNotification, notification =>
|
||||
{
|
||||
listChanged.Writer.TryWrite(notification);
|
||||
return Task.CompletedTask;
|
||||
|
|
@ -185,7 +187,7 @@ public class McpServerBuilderExtensionsToolsTests : LoggedTest, IAsyncDisposable
|
|||
await notificationRead;
|
||||
|
||||
tools = await client.ListToolsAsync(TestContext.Current.CancellationToken);
|
||||
Assert.Equal(12, tools.Count);
|
||||
Assert.Equal(13, tools.Count);
|
||||
Assert.Contains(tools, t => t.Name == "NewTool");
|
||||
|
||||
notificationRead = listChanged.Reader.ReadAsync(TestContext.Current.CancellationToken);
|
||||
|
|
@ -194,7 +196,7 @@ public class McpServerBuilderExtensionsToolsTests : LoggedTest, IAsyncDisposable
|
|||
await notificationRead;
|
||||
|
||||
tools = await client.ListToolsAsync(TestContext.Current.CancellationToken);
|
||||
Assert.Equal(11, tools.Count);
|
||||
Assert.Equal(12, tools.Count);
|
||||
Assert.DoesNotContain(tools, t => t.Name == "NewTool");
|
||||
}
|
||||
|
||||
|
|
@ -516,6 +518,49 @@ public class McpServerBuilderExtensionsToolsTests : LoggedTest, IAsyncDisposable
|
|||
Assert.Null(annotations.ReadOnlyHint);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task HandlesIProgressParameter()
|
||||
{
|
||||
ConcurrentQueue<ProgressNotification> notifications = new();
|
||||
|
||||
IMcpClient client = await CreateMcpClientForServer();
|
||||
client.AddNotificationHandler(NotificationMethods.ProgressNotification, notification =>
|
||||
{
|
||||
ProgressNotification pn = JsonSerializer.Deserialize<ProgressNotification>((JsonElement)notification.Params!)!;
|
||||
notifications.Enqueue(pn);
|
||||
return Task.CompletedTask;
|
||||
});
|
||||
|
||||
var tools = await client.ListToolsAsync(TestContext.Current.CancellationToken);
|
||||
Assert.NotNull(tools);
|
||||
Assert.NotEmpty(tools);
|
||||
|
||||
McpClientTool progressTool = tools.First(t => t.Name == nameof(EchoTool.SendsProgressNotifications));
|
||||
|
||||
var result = await client.SendRequestAsync<CallToolResponse>(new JsonRpcRequest()
|
||||
{
|
||||
Method = RequestMethods.ToolsCall,
|
||||
Params = new CallToolRequestParams()
|
||||
{
|
||||
Name = progressTool.ProtocolTool.Name,
|
||||
Meta = new() { ProgressToken = new("abc123") },
|
||||
},
|
||||
}, TestContext.Current.CancellationToken);
|
||||
|
||||
Assert.Contains("done", JsonSerializer.Serialize(result));
|
||||
SpinWait.SpinUntil(() => notifications.Count == 10, TimeSpan.FromSeconds(10));
|
||||
|
||||
ProgressNotification[] array = notifications.OrderBy(n => n.Progress.Progress).ToArray();
|
||||
Assert.Equal(10, array.Length);
|
||||
for (int i = 0; i < array.Length; i++)
|
||||
{
|
||||
Assert.Equal("\"abc123\"", array[i].ProgressToken.ToString());
|
||||
Assert.Equal(i, array[i].Progress.Progress);
|
||||
Assert.Equal(10, array[i].Progress.Total);
|
||||
Assert.Equal($"Progress {i}", array[i].Progress.Message);
|
||||
}
|
||||
}
|
||||
|
||||
[McpServerToolType]
|
||||
public sealed class EchoTool(ObjectWithId objectFromDI)
|
||||
{
|
||||
|
|
@ -583,6 +628,17 @@ public class McpServerBuilderExtensionsToolsTests : LoggedTest, IAsyncDisposable
|
|||
|
||||
[McpServerTool]
|
||||
public string GetCtorParameter() => $"{_randomValue}:{objectFromDI.Id}";
|
||||
|
||||
[McpServerTool]
|
||||
public string SendsProgressNotifications(IProgress<ProgressNotificationValue> progress)
|
||||
{
|
||||
for (int i = 0; i < 10; i++)
|
||||
{
|
||||
progress.Report(new() { Progress = i, Total = 10, Message = $"Progress {i}" });
|
||||
}
|
||||
|
||||
return "done";
|
||||
}
|
||||
}
|
||||
|
||||
[McpServerToolType]
|
||||
|
|
|
|||
|
|
@ -129,7 +129,7 @@ public class McpServerTests : LoggedTest
|
|||
Assert.NotNull(result);
|
||||
Assert.NotEmpty(transport.SentMessages);
|
||||
Assert.IsType<JsonRpcRequest>(transport.SentMessages[0]);
|
||||
Assert.Equal("sampling/createMessage", ((JsonRpcRequest)transport.SentMessages[0]).Method);
|
||||
Assert.Equal(RequestMethods.SamplingCreateMessage, ((JsonRpcRequest)transport.SentMessages[0]).Method);
|
||||
|
||||
await transport.DisposeAsync();
|
||||
await runTask;
|
||||
|
|
@ -162,7 +162,7 @@ public class McpServerTests : LoggedTest
|
|||
Assert.NotNull(result);
|
||||
Assert.NotEmpty(transport.SentMessages);
|
||||
Assert.IsType<JsonRpcRequest>(transport.SentMessages[0]);
|
||||
Assert.Equal("roots/list", ((JsonRpcRequest)transport.SentMessages[0]).Method);
|
||||
Assert.Equal(RequestMethods.RootsList, ((JsonRpcRequest)transport.SentMessages[0]).Method);
|
||||
|
||||
await transport.DisposeAsync();
|
||||
await runTask;
|
||||
|
|
@ -185,7 +185,7 @@ public class McpServerTests : LoggedTest
|
|||
{
|
||||
await Can_Handle_Requests(
|
||||
serverCapabilities: null,
|
||||
method: "ping",
|
||||
method: RequestMethods.Ping,
|
||||
configureOptions: null,
|
||||
assertResult: response =>
|
||||
{
|
||||
|
|
@ -198,7 +198,7 @@ public class McpServerTests : LoggedTest
|
|||
{
|
||||
await Can_Handle_Requests(
|
||||
serverCapabilities: null,
|
||||
method: "initialize",
|
||||
method: RequestMethods.Initialize,
|
||||
configureOptions: null,
|
||||
assertResult: response =>
|
||||
{
|
||||
|
|
@ -216,7 +216,7 @@ public class McpServerTests : LoggedTest
|
|||
{
|
||||
await Can_Handle_Requests(
|
||||
serverCapabilities: null,
|
||||
method: "completion/complete",
|
||||
method: RequestMethods.CompletionComplete,
|
||||
configureOptions: null,
|
||||
assertResult: response =>
|
||||
{
|
||||
|
|
@ -235,7 +235,7 @@ public class McpServerTests : LoggedTest
|
|||
{
|
||||
await Can_Handle_Requests(
|
||||
serverCapabilities: null,
|
||||
method: "completion/complete",
|
||||
method: RequestMethods.CompletionComplete,
|
||||
configureOptions: options =>
|
||||
{
|
||||
options.GetCompletionHandler = (request, ct) =>
|
||||
|
|
@ -287,7 +287,7 @@ public class McpServerTests : LoggedTest
|
|||
ReadResourceHandler = (request, ct) => throw new NotImplementedException(),
|
||||
}
|
||||
},
|
||||
"resources/templates/list",
|
||||
RequestMethods.ResourcesTemplatesList,
|
||||
configureOptions: null,
|
||||
assertResult: response =>
|
||||
{
|
||||
|
|
@ -318,7 +318,7 @@ public class McpServerTests : LoggedTest
|
|||
ReadResourceHandler = (request, ct) => throw new NotImplementedException(),
|
||||
}
|
||||
},
|
||||
"resources/list",
|
||||
RequestMethods.ResourcesList,
|
||||
configureOptions: null,
|
||||
assertResult: response =>
|
||||
{
|
||||
|
|
@ -334,7 +334,7 @@ public class McpServerTests : LoggedTest
|
|||
[Fact]
|
||||
public async Task Can_Handle_Resources_List_Requests_Throws_Exception_If_No_Handler_Assigned()
|
||||
{
|
||||
await Throws_Exception_If_No_Handler_Assigned(new ServerCapabilities { Resources = new() }, "resources/list", "ListResources handler not configured");
|
||||
await Throws_Exception_If_No_Handler_Assigned(new ServerCapabilities { Resources = new() }, RequestMethods.ResourcesList, "ListResources handler not configured");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
@ -355,7 +355,7 @@ public class McpServerTests : LoggedTest
|
|||
ListResourcesHandler = (request, ct) => throw new NotImplementedException(),
|
||||
}
|
||||
},
|
||||
method: "resources/read",
|
||||
method: RequestMethods.ResourcesRead,
|
||||
configureOptions: null,
|
||||
assertResult: response =>
|
||||
{
|
||||
|
|
@ -373,7 +373,7 @@ public class McpServerTests : LoggedTest
|
|||
[Fact]
|
||||
public async Task Can_Handle_Resources_Read_Requests_Throws_Exception_If_No_Handler_Assigned()
|
||||
{
|
||||
await Throws_Exception_If_No_Handler_Assigned(new ServerCapabilities { Resources = new() }, "resources/read", "ReadResource handler not configured");
|
||||
await Throws_Exception_If_No_Handler_Assigned(new ServerCapabilities { Resources = new() }, RequestMethods.ResourcesRead, "ReadResource handler not configured");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
@ -394,7 +394,7 @@ public class McpServerTests : LoggedTest
|
|||
GetPromptHandler = (request, ct) => throw new NotImplementedException(),
|
||||
},
|
||||
},
|
||||
method: "prompts/list",
|
||||
method: RequestMethods.PromptsList,
|
||||
configureOptions: null,
|
||||
assertResult: response =>
|
||||
{
|
||||
|
|
@ -410,7 +410,7 @@ public class McpServerTests : LoggedTest
|
|||
[Fact]
|
||||
public async Task Can_Handle_List_Prompts_Requests_Throws_Exception_If_No_Handler_Assigned()
|
||||
{
|
||||
await Throws_Exception_If_No_Handler_Assigned(new ServerCapabilities { Prompts = new() }, "prompts/list", "ListPrompts handler not configured");
|
||||
await Throws_Exception_If_No_Handler_Assigned(new ServerCapabilities { Prompts = new() }, RequestMethods.PromptsList, "ListPrompts handler not configured");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
@ -425,7 +425,7 @@ public class McpServerTests : LoggedTest
|
|||
ListPromptsHandler = (request, ct) => throw new NotImplementedException(),
|
||||
}
|
||||
},
|
||||
method: "prompts/get",
|
||||
method: RequestMethods.PromptsGet,
|
||||
configureOptions: null,
|
||||
assertResult: response =>
|
||||
{
|
||||
|
|
@ -439,7 +439,7 @@ public class McpServerTests : LoggedTest
|
|||
[Fact]
|
||||
public async Task Can_Handle_Get_Prompts_Requests_Throws_Exception_If_No_Handler_Assigned()
|
||||
{
|
||||
await Throws_Exception_If_No_Handler_Assigned(new ServerCapabilities { Prompts = new() }, "prompts/get", "GetPrompt handler not configured");
|
||||
await Throws_Exception_If_No_Handler_Assigned(new ServerCapabilities { Prompts = new() }, RequestMethods.PromptsGet, "GetPrompt handler not configured");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
@ -460,7 +460,7 @@ public class McpServerTests : LoggedTest
|
|||
CallToolHandler = (request, ct) => throw new NotImplementedException(),
|
||||
}
|
||||
},
|
||||
method: "tools/list",
|
||||
method: RequestMethods.ToolsList,
|
||||
configureOptions: null,
|
||||
assertResult: response =>
|
||||
{
|
||||
|
|
@ -475,7 +475,7 @@ public class McpServerTests : LoggedTest
|
|||
[Fact]
|
||||
public async Task Can_Handle_List_Tools_Requests_Throws_Exception_If_No_Handler_Assigned()
|
||||
{
|
||||
await Throws_Exception_If_No_Handler_Assigned(new ServerCapabilities { Tools = new() }, "tools/list", "ListTools handler not configured");
|
||||
await Throws_Exception_If_No_Handler_Assigned(new ServerCapabilities { Tools = new() }, RequestMethods.ToolsList, "ListTools handler not configured");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
@ -496,7 +496,7 @@ public class McpServerTests : LoggedTest
|
|||
ListToolsHandler = (request, ct) => throw new NotImplementedException(),
|
||||
}
|
||||
},
|
||||
method: "tools/call",
|
||||
method: RequestMethods.ToolsCall,
|
||||
configureOptions: null,
|
||||
assertResult: response =>
|
||||
{
|
||||
|
|
@ -511,7 +511,7 @@ public class McpServerTests : LoggedTest
|
|||
[Fact]
|
||||
public async Task Can_Handle_Call_Tool_Requests_Throws_Exception_If_No_Handler_Assigned()
|
||||
{
|
||||
await Throws_Exception_If_No_Handler_Assigned(new ServerCapabilities { Tools = new() }, "tools/call", "CallTool handler not configured");
|
||||
await Throws_Exception_If_No_Handler_Assigned(new ServerCapabilities { Tools = new() }, RequestMethods.ToolsCall, "CallTool handler not configured");
|
||||
}
|
||||
|
||||
private async Task Can_Handle_Requests(ServerCapabilities? serverCapabilities, string method, Action<McpServerOptions>? configureOptions, Action<object> assertResult)
|
||||
|
|
@ -528,7 +528,7 @@ public class McpServerTests : LoggedTest
|
|||
|
||||
transport.OnMessageSent = (message) =>
|
||||
{
|
||||
if (message is JsonRpcResponse response && response.Id.AsNumber == 55)
|
||||
if (message is JsonRpcResponse response && response.Id.ToString() == "55")
|
||||
receivedMessage.SetResult(response);
|
||||
};
|
||||
|
||||
|
|
@ -536,7 +536,7 @@ public class McpServerTests : LoggedTest
|
|||
new JsonRpcRequest
|
||||
{
|
||||
Method = method,
|
||||
Id = RequestId.FromNumber(55)
|
||||
Id = new RequestId(55)
|
||||
}
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -136,7 +136,7 @@ public class SseClientTransportTests : LoggedTest
|
|||
};
|
||||
|
||||
await using var session = await transport.ConnectAsync(TestContext.Current.CancellationToken);
|
||||
await session.SendMessageAsync(new JsonRpcRequest() { Method = "initialize", Id = RequestId.FromNumber(44) }, CancellationToken.None);
|
||||
await session.SendMessageAsync(new JsonRpcRequest() { Method = RequestMethods.Initialize, Id = new RequestId(44) }, CancellationToken.None);
|
||||
Assert.True(true);
|
||||
}
|
||||
|
||||
|
|
@ -181,7 +181,7 @@ public class SseClientTransportTests : LoggedTest
|
|||
|
||||
await using var session = await transport.ConnectAsync(TestContext.Current.CancellationToken);
|
||||
|
||||
await session.SendMessageAsync(new JsonRpcRequest() { Method = "initialize", Id = RequestId.FromNumber(44) }, CancellationToken.None);
|
||||
await session.SendMessageAsync(new JsonRpcRequest() { Method = RequestMethods.Initialize, Id = new RequestId(44) }, CancellationToken.None);
|
||||
Assert.True(true);
|
||||
eventSourcePipe.Writer.Complete();
|
||||
}
|
||||
|
|
@ -214,7 +214,7 @@ public class SseClientTransportTests : LoggedTest
|
|||
Assert.True(session.MessageReader.TryRead(out var message));
|
||||
Assert.NotNull(message);
|
||||
Assert.IsType<JsonRpcRequest>(message);
|
||||
Assert.Equal("44", ((JsonRpcRequest)message).Id.AsString);
|
||||
Assert.Equal("\"44\"", ((JsonRpcRequest)message).Id.ToString());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
|
|||
|
|
@ -77,7 +77,7 @@ public class StdioServerTransportTests : LoggedTest
|
|||
// Verify transport is connected
|
||||
Assert.True(transport.IsConnected, "Transport should be connected after StartListeningAsync");
|
||||
|
||||
var message = new JsonRpcRequest { Method = "test", Id = RequestId.FromNumber(44) };
|
||||
var message = new JsonRpcRequest { Method = "test", Id = new RequestId(44) };
|
||||
|
||||
await transport.SendMessageAsync(message, TestContext.Current.CancellationToken);
|
||||
|
||||
|
|
@ -100,7 +100,7 @@ public class StdioServerTransportTests : LoggedTest
|
|||
[Fact]
|
||||
public async Task ReadMessagesAsync_Should_Read_Messages()
|
||||
{
|
||||
var message = new JsonRpcRequest { Method = "test", Id = RequestId.FromNumber(44) };
|
||||
var message = new JsonRpcRequest { Method = "test", Id = new RequestId(44) };
|
||||
var json = JsonSerializer.Serialize(message, McpJsonUtilities.DefaultOptions);
|
||||
|
||||
// Use a reader that won't terminate
|
||||
|
|
@ -125,7 +125,7 @@ public class StdioServerTransportTests : LoggedTest
|
|||
Assert.True(transport.MessageReader.TryPeek(out var readMessage));
|
||||
Assert.NotNull(readMessage);
|
||||
Assert.IsType<JsonRpcRequest>(readMessage);
|
||||
Assert.Equal(44, ((JsonRpcRequest)readMessage).Id.AsNumber);
|
||||
Assert.Equal("44", ((JsonRpcRequest)readMessage).Id.ToString());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
@ -158,7 +158,7 @@ public class StdioServerTransportTests : LoggedTest
|
|||
var chineseMessage = new JsonRpcRequest
|
||||
{
|
||||
Method = "test",
|
||||
Id = RequestId.FromNumber(44),
|
||||
Id = new RequestId(44),
|
||||
Params = new Dictionary<string, JsonElement>
|
||||
{
|
||||
["text"] = JsonSerializer.SerializeToElement(chineseText)
|
||||
|
|
@ -180,7 +180,7 @@ public class StdioServerTransportTests : LoggedTest
|
|||
var emojiMessage = new JsonRpcRequest
|
||||
{
|
||||
Method = "test",
|
||||
Id = RequestId.FromNumber(45),
|
||||
Id = new RequestId(45),
|
||||
Params = new Dictionary<string, JsonElement>
|
||||
{
|
||||
["text"] = JsonSerializer.SerializeToElement(emojiText)
|
||||
|
|
|
|||
|
|
@ -191,7 +191,7 @@ public sealed class InMemoryTestSseServer : IAsyncDisposable
|
|||
string content = await reader.ReadToEndAsync(cancellationToken);
|
||||
|
||||
var jsonRpcNotification = JsonSerializer.Deserialize<JsonRpcNotification>(content);
|
||||
if (jsonRpcNotification != null && jsonRpcNotification.Method != "initialize")
|
||||
if (jsonRpcNotification != null && jsonRpcNotification.Method != RequestMethods.Initialize)
|
||||
{
|
||||
// Test server so just ignore notifications
|
||||
|
||||
|
|
@ -209,7 +209,7 @@ public sealed class InMemoryTestSseServer : IAsyncDisposable
|
|||
|
||||
if (jsonRpcRequest != null)
|
||||
{
|
||||
if (jsonRpcRequest.Method == "initialize")
|
||||
if (jsonRpcRequest.Method == RequestMethods.Initialize)
|
||||
{
|
||||
await HandleInitializationRequest(response, jsonRpcRequest);
|
||||
}
|
||||
|
|
@ -266,7 +266,7 @@ public sealed class InMemoryTestSseServer : IAsyncDisposable
|
|||
{
|
||||
var errorResponse = new JsonRpcError
|
||||
{
|
||||
Id = id ?? RequestId.FromString("error"),
|
||||
Id = id ?? new RequestId("error"),
|
||||
JsonRpc = "2.0",
|
||||
Error = new JsonRpcErrorDetail
|
||||
{
|
||||
|
|
|
|||
|
|
@ -39,9 +39,9 @@ public class TestServerTransport : ITransport
|
|||
SentMessages.Add(message);
|
||||
if (message is JsonRpcRequest request)
|
||||
{
|
||||
if (request.Method == "roots/list")
|
||||
if (request.Method == RequestMethods.RootsList)
|
||||
await ListRoots(request, cancellationToken);
|
||||
else if (request.Method == "sampling/createMessage")
|
||||
else if (request.Method == RequestMethods.SamplingCreateMessage)
|
||||
await Sampling(request, cancellationToken);
|
||||
else
|
||||
await WriteMessageAsync(request, cancellationToken);
|
||||
|
|
|
|||
Loading…
Reference in a new issue