Skip to content

Conversation

@halter73
Copy link
Owner

I started this change trying to keep the IMcpServerBuilder in the ModelContextProtocol package even after removing these dependencies, and while this is technically possible, it comes with too many downsides. Some of these we already knew, like the fact that anyone using AddMcpServer would still have to manually run the server (possibly via their own hosted service) after they built the ServiceProvider/Host.

However, the biggest issue, and the one that made me step back and reevaluate the approach was that the IMcpServer that gets registered by methods like WithStdioServerTransport via AddSingleSessionServerDependencies would not get any options configured using the options pattern unless someone knows to manually set up a new IMcpServer registration to read from IOptions<McpServerOptions>.

I considered keeping AddMcpServer(this IServiceCollection services, Action<McpServerOptions>? configureOptions = null) in the ModelContextProtocol package and just removing the configureOptions parameter. It'd then be possible to just add an overload in the AspNetCore package that does take configureOptions, but that would make things worse in my opinion. This would mean that builder.Services.Configure<McpServerOptions>(options => { }) would only work if you called the overload in the AspNetCore package which I think would be extremely confusing.

As ModelContextProtocol.TestServer demonstrates, you can still write a stdio server with just the ModelContextProtocol package, so I do like this change over removing the server completely from the core package. However, I do realize that it's far less convenient to configure a stdio server without the IMcpServerBuilder, and people writing stdio-only servers likely won't want the AspNetCore framework reference.

I think the best solution would be to introduce a ModelContextProtocol.Hosting package that is basically the same as what ModelContextProtocol was previously and depends on the ModelContextProtocol package as it is in this PR. So we'd have three packages:

  • ModelContextProtocol
    • Depends on Microsoft.Extensions.AI.Abstractions
    • Depends on Microsoft.Extensions.Logging.Abstractions (and thereby M.E.DI)
    • Depends on System.Net.ServerSentEvents
  • ModelContextProtocol.Hosting
    • Depends on ModelContextProtocol
    • Depends on Microsoft.Extensions.Hosting.Abstractions (and thereby Options, Diagnostic.Abstractions, etc...)
  • ModelContextProtocol.AspNetCore
    • Depends on ModelContextProtocol.Hosting
    • Depends on Microsoft.AspNetCore.App FrameworkReference

This possibly helps address some concerns about dependency bloat for clients raised in modelcontextprotocol#369 @KrzysztofCwalina.

@halter73
Copy link
Owner Author

I think the best solution would be to introduce a ModelContextProtocol.Hosting package that is basically the same as what ModelContextProtocol was previously and depends on the ModelContextProtocol package as it is in this PR.

It sounds like that this might make it easier for a particular server to switch from stdio to http and vice versa, while also ensuring that the core protocol is contained in a leaner package. @copilot can you do this?

@halter73
Copy link
Owner Author

Sorry for the @-mention. I was trying to test if I could have copilot continue a PR in progress because it can only submit PRs to the default branch and I already have @copilot working in parallel on #2. I'll try this again with an issue after I can update the default branch of my fork to contain changes in my half-completed at modelcontextprotocol#428.

@halter73 halter73 closed this May 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants