Improve hosting

[twenzel]

This PR adds a new way to host and configure McpServer which is more aligned to the Microsoft hosting principles.

See the new TestServerWithHosting project for details.

Summary:

• Adds a new AddMcpServer extension for DI
• Adds a hosted service to host/run McpServer
• Adds couple of extension methods to configure the McpServer

[PederHP]

This is a great change! It aligns perfectly with modern .NET idiomatic constructs, providing a more convenient and elegant way of creating servers.

It does deserve some documentation, ie. an example of how to use it, but given there isn't any documentation yet, that's a task for later when the docs are written. And the sample serves as a perfectly usable guide until then.

[stephentoub]

This adds a dependency on Microsoft.Extensions.Hosting.Abstractions. Is that worth it for this small helper?

[twenzel]

Good question. Is it bad in anyway? Or should we try to have less dependencies as possible?

[twenzel]

We also might provide this functionality in a dedicated mcpdotnet.Extensions.Hosting package..

[stephentoub]

The package is fine and a key dependency of ASP.NET. It's just that in general it's better to not have extraneous dependencies, as it makes them required for anyone who might want to use this library, making the package graph larger, etc. Personally I don't think it's worth it, but it's not a problem right now. Have you gotten requests for this, or was it added just because it's interesting and possible? I'd be curious to understand the use case and whether this is sufficient or whether devs would actually want more code in the background service, at which point saving them one or two lines isn't all that much value.

[halter73]

I don't think that taking a Hosting.Abstractions dependency is a big deal in a package that already has a Logging.Abstractions dependency. Hosting.Abstractions does bring in a lot more transitive dependencies than Logging.Abstractions, but they naturally go together.

I understand the appeal of a core package that brings in no Microsoft.Extensions dependencies, not even M.E.D.I, but Hosting.Abstractions is a weird place to draw the line imo.

[stephentoub]

a weird place to draw the line imo.

Not a line :) I'm just questioning each individual dependency. And in this case, it seems to be in support of a class:

public class McpServerHostedService(IMcpServer server) : BackgroundService
{
    protected override Task ExecuteAsync(CancellationToken stoppingToken) => server.StartAsync(stoppingToken);
}

which anyone could write. If this is something that's been requested and that will be used as-is, then fine. But if it's being added just because it's possible, then it doesn't seem worth it, especially since I'd wonder if this is actually sufficient as the background service. If the developer would be deriving from McpServerHostedService to add their own additional logic or state as well, then this isn't saving anything except them writing await server.StartAsync(stoppingToken) somewhere in their own class, and presumably even then that would just be in place of them doing await base.ExecuteAsync(stoppingToken).

Hosting.Abstractions does bring in a lot more transitive dependencies than Logging.Abstractions

Right, it also brings in Microsoft.Extensions.Options, Microsoft.Extensions.Configuration.Abstractions, Microsoft.Extensions.Diagnostics.Abstractions, and Microsoft.Extensions.FileProviders.Abstractions, beyond what's already being referenced.

[halter73]

Even without the BackgroundService, this change still requires an Options dependency now that the McpServerFactory is registered as part of AddMcpServer. Do you think AddMcpServer should move into a separate package? I like the hosting package idea, but then I wonder if we shouldn't move logging and DI into the hosting package as well.

I do have a functional issue with how AddMcpServer adds this background service by default. I don't think it makes sense to always start the IMcpServer when the host starts just because I wanted to add MCP services to a service collection.

I think it's very likely that the host might be an ASP.NET Core application that is going to want to handle the incoming request and pass off an SSE stream to the IMcpServerFactory. This means IMcpServerFactory.CreateServer should probably take an ITransport argument or something similar, so something like ASP.NET Core can handle the auth, routing, and stuff like that, but delegate the MCP protocol layer logic to an IMcpServer.

I can submit a PR that does this in a separate hosting package if there's interest. I'd leave this project mostly untouched for now, but I'd have to change the IMcpServerFactory interface of course. And I'd probably update AddMcpServer to not register this McpServerHostedService by default.

[stephentoub]

I realize I started this conversation with my question, but maybe for now we leave the dependency as it is and @halter73 you could make your proposed changes in this lib? I suggest we subsequently think about what the right layering for packages is holistically, e.g. maybe there should be three: a core library, a client library, and a server library. But if/how it's split could be based on what the dependencies look like after the library has evolved a bit more?