WithSubscribeToResourcesHandler is ignored if you do not configure ListResourcesHandler or ReadResourceHandler

[halter73]

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.

@mikekistler @PederHP

[PederHP]

I might be misremembering but I think back when I made my change, there'd always be a handler configured. So it didn't make sense to register a subscription handler without those. But and here I'm still rusty on the SDK after having been away for a bit - this is no longer the only way to register resources, and hence the old assumption is now causing a bug.

So I think this is simply defensive programming from a time when WithResources worked differently. But I'll dig further into it, as it's an excellent place to re-familiarize myself with the codebase as more than an SDK consumer. (I have dogfooded plenty in my absence :) ).

Addendum: Looking at McpServer.cs - there's a commented out // subscribe = true and a TODO note. I think the todo is dated - the subscription logic is there earlier in the file. Will dig further, but one way or another the TODO should probably go.

[halter73]

Fixed by #676