Skip to content

Add ModelContextProtocol.Core package #428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 2, 2025

Conversation

halter73
Copy link
Contributor

@halter73 halter73 commented May 17, 2025

I started this change trying to keep the IMcpServerBuilder in the ModelContextProtocol.Core 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.Core 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.

The current iteration of this PR introduces a ModelContextProtocol.Core package that trims out the hosting and options dependencies from the existing ModelContextProtocol package. The new ModelContextProtocol depends on the Core package and exposes the same IMcpServerBuilder builder APIs as before. 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.Core
    • Depends on Microsoft.Extensions.AI.Abstractions
    • Depends on Microsoft.Extensions.Logging.Abstractions (and thereby M.E.DI)
    • Depends on System.Net.ServerSentEvents
  • ModelContextProtocol
    • Depends on ModelContextProtocol.Core
    • 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 #369 @KrzysztofCwalina.

@eiriktsarpalis
Copy link
Contributor

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.

@halter73 halter73 marked this pull request as ready for review May 28, 2025 21:17
@halter73 halter73 changed the title Remove the hosting and options dependencies from the ModelContextProtocol package Add ModelContextProtocol.Hosting package May 28, 2025
@halter73
Copy link
Contributor Author

Alright. I updated this PR to add a ModelContextProtocol.Hosting package for the code that was previously moved directly up to the ModelContextProtocol.AspNetCore package. ModelContextProtocol.Hosting includes everything that used to be in the core ModelContextProtocol package.

The markdown link failure is because the new README points to https://www.nuget.org/packages/ModelContextProtocol.Hosting which doesn't exist yet.

Copy link
Contributor

@eiriktsarpalis eiriktsarpalis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might also want to update the root README.md so that the new structure is documented.

Copilot AI and others added 3 commits May 29, 2025 12:26
- ModelContextProtocol -> ModelContextProtocol.Core: The core package now clearly indicates it provides fundamental MCP functionality without hosting features
- ModelContextProtocol.Hosting -> ModelContextProtocol: The main package now includes hosting and dependency injection extensions, making it the primary entry point for most users
@halter73 halter73 changed the title Add ModelContextProtocol.Hosting package Add ModelContextProtocol.Core package May 29, 2025
@halter73
Copy link
Contributor Author

You might also want to update the root README.md so that the new structure is documented.

Done.

Copy link
Contributor

@eiriktsarpalis eiriktsarpalis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@stephentoub
Copy link
Contributor

This seems reasonable. I keep waffling about either moving more stuff from MCP.Core up into MCP, but based on the previous experiments in #369, doing so saves very little size, and the real win is just in removing the extra dependencies, which then means the goal is to put into MCP only the minimum amount that requires the dependencies, and I think that's what you have, yes?

halter73 added 2 commits June 2, 2025 15:10
# Conflicts:
#	ModelContextProtocol.sln
#	src/ModelContextProtocol.Core/Protocol/ElicitRequestParams.cs
#	src/ModelContextProtocol.Core/Protocol/ElicitResult.cs
#	src/ModelContextProtocol.Core/Protocol/ElicitationCapability.cs
@halter73
Copy link
Contributor Author

halter73 commented Jun 2, 2025

which then means the goal is to put into MCP only the minimum amount that requires the dependencies, and I think that's what you have, yes?

Exactly. The goal here was not to reduce the assembly size of the Core package but to remove the extra dependencies.

@halter73 halter73 merged commit 703b0fe into modelcontextprotocol:main Jun 2, 2025
7 of 8 checks passed
@@ -29,6 +29,7 @@
<Folder Name="/src/">
<File Path="src/Directory.Build.props" />
<Project Path="src/ModelContextProtocol.AspNetCore/ModelContextProtocol.AspNetCore.csproj" />
<Project Path="src/ModelContextProtocol.Core/ModelContextProtocol.Core.csproj" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the way diffs work in slnx

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.

3 participants