-
Notifications
You must be signed in to change notification settings - Fork 541
Description
Describe the bug
@mikekistler pointed out when he registers a subscribe handler like this:
builder.Services.AddMcpServer()
.WithHttpTransport(options =>
// Configure session timeout
options.IdleTimeout = Timeout.InfiniteTimeSpan // Never timeout
)
.WithResources<LiveResources>()
.WithSubscribeToResourcesHandler((context, token) =>
{
if (context.Params?.Uri is string uri)
{
ResourceManager.Subscriptions.Add(uri, context.Server);
}
return ValueTask.FromResult(new EmptyResult());
}
);
And his client is sending a subscribe request like this:
await mcpClient.SubscribeToResourceAsync(resourceUri);
He can see in the server logs that this request is received and processed:
info: ModelContextProtocol.Server.McpServer[570385771]
Server (ResourceNotifications 1.0.0.0), Client (ResourceNotificationUpdateClient 1.0.0) method 'resources/subscribe' request handler called.
ModelContextProtocol.Server.McpServer: Information: Server (ResourceNotifications 1.0.0.0), Client (ResourceNotificationUpdateClient 1.0.0) method 'resources/subscribe' request handler completed.
info: ModelContextProtocol.Server.McpServer[1867955179]
Server (ResourceNotifications 1.0.0.0), Client (ResourceNotificationUpdateClient 1.0.0) method 'resources/subscribe' request handler completed.
But his handler never gets control.
Additional context
It appears that we only register the SubscribeToResourceHandler if you also register a ListResourcesHandler and/or ReadResourceHandler
csharp-sdk/src/ModelContextProtocol/McpServerHandlers.cs
Lines 181 to 195 in 650df63
if (ListResourcesHandler is not null || | |
ReadResourceHandler is not null) | |
{ | |
resourcesCapability ??= new(); | |
resourcesCapability.ListResourceTemplatesHandler = ListResourceTemplatesHandler ?? resourcesCapability.ListResourceTemplatesHandler; | |
resourcesCapability.ListResourcesHandler = ListResourcesHandler ?? resourcesCapability.ListResourcesHandler; | |
resourcesCapability.ReadResourceHandler = ReadResourceHandler ?? resourcesCapability.ReadResourceHandler; | |
if (SubscribeToResourcesHandler is not null || UnsubscribeFromResourcesHandler is not null) | |
{ | |
resourcesCapability.SubscribeToResourcesHandler = SubscribeToResourcesHandler ?? resourcesCapability.SubscribeToResourcesHandler; | |
resourcesCapability.UnsubscribeFromResourcesHandler = UnsubscribeFromResourcesHandler ?? resourcesCapability.UnsubscribeFromResourcesHandler; | |
resourcesCapability.Subscribe = true; | |
} | |
} |
I assume this is a bug. Prior to PederHP/mcpdotnet#89, we'd hook up the SubscribeToResourceHandler regardless of whether you configured ListResourcesHandler or ReadResourceHandler, but we wouldn't automatically set "subscribe": true
. I think we want to keep the "subscribe": true
bit while allowing you to configure just the SubscribeToResourceHandler as before. Even if we did want to require you to configure all the resource-related handlers to configure the subscribe-handler, we should raise an error rather than fail silently in this case.