-
Notifications
You must be signed in to change notification settings - Fork 270
Separate client and server APIs into two separate packages #369
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
Comments
For ASP.NET Core server scenarios, we already have the ModelContextProtocol.AspNetCore package. The primary ModelContextProtocol does not have a Microsoft.AspNetCore.App framework dependency. What it does have is dependencies on Logging.Abstractions and Hosting.Abstractions which can be very useful even for both stdio clients and servers There was some previous discussion as to whether or not the Hosting.Abstractions dependency was necessary in PederHP/mcpdotnet#30 (comment) back when this project was hosted in another repo and the ModelContextProtocol.AspNetCore package didn't exist yet. And while it would be easier to remove that than Logging.Abstractions, I don't think we're super interested in doing either at this point. |
I think @KrzysztofCwalina is also wondering about Microsoft.Extensions.DependencyInjection.Abstractions. We use DI in server configuration. Do we expect we'll use DI in client configuration? If not, and if we split, we could probably also get away without that dependency for core and client packages? |
You can write the server without DI too, right? You wouldn't get AddMcpServer, IMcpServerBuilder or its extension methods, but I don't think it's that hard to create a simple server without those. The bottom sample of our README shows how to configure a server without using IMcpServerBuilder, IServiceCollection or M.E.DI at all. And even if you didn't want to define your own CallToolHandler, you could easily call McpServerTool.Create and add it to McpServerOptions.ServerCapabilities.Tools.ToolCollection. Splitting the existing ModelContextProtocol package into .Client and .Server packages makes no sense to me since the client and servers are peers and share so much code. If we do decide that we need to a third package with an even smaller dependency graph, the new package should probably be something like ModelContetProtocol.Core. I don't particularly love putting "Core" in a package name, so I'm open to suggestions there, but it should support client and server scenarios. Although, it'd be a lot simpler not to create a third package. @KrzysztofCwalina Can you explain the reasons for wanting to exclude Microsoft.Extensions.DependencyInjection.Abstractions? It's the abstractions package, so it's not nearly as big as most other packages, and it doesn't have many of its own dependencies. |
It's a death by 1000 papercuts issue. Not just DI. To me the server and client scenarios are mainly disjoint, and so the server specific code seems like unnecessary overhead in client scenarios. Are there any other examples in the .NET platform (BCL) where we mix clients with servers? BTW, I can also imagine the server parts evolving in the future such that you want to add dependency on asp core itself. At which point you'd have a massive problem |
Can you elaborate? The ASP.NET pieces are already in a separate package. What are the server parts you're imagining evolving where the server parts would need a dependency on ASP.NET separate from where we've already aggregated the ASP.NET dependency?
There are very few protocols in the core libraries where both client and servers are provided, regardless of packaging. But you have things like Socket, which support both connecting and accepting, WebSocket.CreateFromStream which accepts a bool isServer for specifying whether it's the client side or server side, System.Net.Quic which supports both clients connecting and servers listening, etc. |
I don't have specifics. But what I meant is if an existing type in the current package wants to have an ASP.NET type in the signature, it won't matter that you have a separate ASP.NET specific package. It will be too late. Anyway, your call. I was just thinking the architecture would be cleaner if we had: if you are writing an MCP server app, here is a library for you. If you are writing an app that calls an MCP server, here is a library for you that has no overhead (dependencies, size, complexity) related the scenario of writing a server app. I do see the point @halter73 made that it would result in 3 packages; one additional for the shared parts. |
I did a quick-and-dirty spike to see what it would look like if instead of:
we instead had:
For the net9.0 assets, based on what's currently in main, before this refactoring:
and after this refactoring:
The sum of the parts is measurably larger than the original in large part because some internal implementation is now compiled into both .Client/.Server rather than into the core assembly. Changes in public API would also be necessary to tease apart some things. For example, the capability objects are shared at the protocol level, but today they have some client or server-specific types on them. We would need to change these APIs to avoid that. I hacked it just by replacing the types with object to get it to build, but there would be meaningful changes required. I was also curious about the impact of trimming. Using the ModelContextProtocol library as it’s currently published on nuget, I wrote a simple console app that created an MCP client, listed tools, and invoked a tool, then published a trimmed app. The resulting ModelContextProtocol library was 386KB. Moreover, the resulting app completely trimmed away all of the cited dependencies except for Microsoft.Extensions.AI.Abstractions and Microsoft.Extensions.Logging.Extensions; the others weren’t in the app at all. Regardless of trimming, for an ASP.NET app, all of the dependencies are in the ASP.NET shared framework, except for Microsoft.Extensions.AI.Abstractions, so none of those dependencies are carried with the app. For net10.0, System.Net.ServerSentEvents is also in the netcoreapp shared framework. |
I think the library should be refactored into low level APIs that don't depend on the abstractions and then a framework on top of it that does. Such low level MCP Client would be very light weight, and could support scenarios where the abstractions are present and where they are not. I suspect low level MCP protocol will be more broadly applicable than what these abstractions represent. For example, see https://github.com/openai/openai-dotnet/tree/de89187f0637b99297f3f038e96d6d084723232f/src/Utility/MCP for such low level MCP client that simply returns JSON representation of the tool descriptions. This JSON can then be parsed to arbitrary AI library, and/or to the abstractions. For OpenAI library, parsing tool descriptions into M.E.AI.A types is not useful as the library would then have to "marshal" to its representation (as it does not, and cannot, use MEAI types directly. |
Maybe your "ModelContextProtocol (contains only shared types, for protocol layer, and core interfaces)" could contain such low level JSON based MCP client? |
We count our assembly loads in VS because each one impacts our startup performance. Breaking up this one assembly into several (such that as an MCP client, we'd need to load two instead of one) would be a perf hit for us. Any design that keeps all client functionality in a single assembly would be preferable for us from a perf perspective. |
@AArnott, would it be better for you to use low level client that returns JSON? It would be just one dll. |
Probably not better from a functionality/ease perspective. But if I had to choose between higher level APIs vs. load one less DLL, our perf team may direct me to use the lower-level APIs. |
I experimented with the suggested split, with the no-frills protocol-level support (protocol implementation, DTOs, etc.) in a ModelContextProtocol.Protocol assembly and all the extra goodness (attribute-based tool/prompt/resource creation, helpful extension methods, DI integration, etc.) in ModelContextProtocol, which references ModelContextProtocol.Protocol.
For a client that doesn't care about the server bits, the previous split (core+client+server) would mean a size reduction of ~125K, and for this split (protocol+helpers) ~90K. I'm not convinced the size reduction really moves any needles. However, I'd entertain either of the splits to reduce the dependency graph if it was really meaningful, and both the core+client+server and protocol+helpers split remove the transitive dependencies on the same exact assemblies. Most real-world apps we see either already have these dependencies, or get them for free (they're in the framework they use), or are trimming, or don't care about size on disk. But if this would really be helpful, we could do it, as long as it doesn't negatively impact others, e.g. I need @AArnott to be happy. @KrzysztofCwalina, if we refactor like this, I'll assume you'll use it. I also looked a bit into what's composing that 500KB. More than half of the size is from the JSON source generator, for all of the ~80 DTOs that make up the MCP specification. |
Thanks for exploring options, @stephentoub. If you can send me the nupkg's for each of the splits you've prototyped, I can try them in VS to see how much trouble the reduced API from using just one assembly causes (if any) and report back here. |
Right now, the NuGet package (ModelContextProtocol) contains both the client and the server APIs. There are scenarios where just the client is needed, but that pulls-in server specific dependencies, e.g. dependencies on the ASP.NET DI container abstractions which are not used in many client apps (non ASP.NET).
It would be great to break the package into two, e.g.
ModelContextProtocol.Client
ModelContextProtocol.Server
The text was updated successfully, but these errors were encountered: