Skip to content

Conversation

MackinnonBuck
Copy link
Collaborator

@MackinnonBuck MackinnonBuck commented Aug 21, 2025

Summary

Makes the following changes:

  • Replaces interfaces with abstract classes:
    • IMcpEndpoint -> McpSession
    • IMcpServer -> McpServer
    • IMcpClient -> McpClient
  • Deprecates the replaced interfaces
  • Renames internal types:
    • McpClient -> McpClientImpl
    • McpServer -> McpServerImpl
    • McpSession -> McpSessionHandler
  • Removes the internal McpEndpoint abstract class
  • Deprecates factory classes:
    • McpClientFactory
      • Factory method moved directly into the McpClient abstract class
    • McpServerFactory
      • Factory method moved directly into the McpServer abstract class
  • Deprecates extension methods for IMcpClient, IMcpServer, and IMcpEndpoint and places them directly inside their respective new abstract classes
  • Updates McpClientImpl and McpServerImpl to function without relying on a common base class implementing shared functionality

Note

See the edit history of this comment to view previous revisions

Description

This change generally aligns with the suggestions in this comment.

An alternative could be to remove McpClient factory methods altogether and do something similar to what was suggested here. However, this means that we're differentiating between a "client" and a "client session", and the MCP specification doesn't make such a distinction. In the future, we could still add new factory methods that make McpClient creation more concise. For example:

using var client = await McpClient.ConnectAsync("http://localhost:3001/sse");

Or:

using var client = await McpClient.LaunchAsync("npx", "-y", "@modelcontextprotocol/server-everything");

Fixes #355

@PederHP
Copy link
Collaborator

PederHP commented Aug 21, 2025

One downside to this renaming is that it distances the SDK from the TypeScript and Python SDKs, which makes it harder for developer to transition directly between. It's a very minor drift - adding postfixing -Session to McpClient. But I still think it's worth considering every time drift is introduced whether it is strictly necessary or a unified naming can be kept.

It is also preferable, I think, if McpClient aligns directly with the Client concept as described in: https://modelcontextprotocol.io/specification/2025-06-18/architecture.

I was never happy with McpClientFactory so I am glad to see it go, but definitely don't think it should be renamed to McpClient, as that was cause conceptual confusion with the Client concept in the specification. A client is a client session within a host.

Restructuring is definitely warranted, but I'd like to caution against drift from the other SDKs. There's a lot of polyglot development going in this space, and there are many upsides to having at least the specification concepts named identically in the SDKs.

With this change the SDK uses McpClient to mean something other than the specification and the other SDKs and uses McpClientSession to refer to the concept of a client. I can see the point of using the latter, but the former I am skeptical of. A client is a very well defined concept in the protocol spec and in the other SDKs.

@MackinnonBuck MackinnonBuck marked this pull request as ready for review August 29, 2025 17:10
@MackinnonBuck MackinnonBuck changed the title [DRAFT] Restructure IMcpEndpoint, IMcpClient, and IMcpServer Restructure IMcpEndpoint, IMcpClient, and IMcpServer Aug 29, 2025
@PederHP
Copy link
Collaborator

PederHP commented Aug 29, 2025

Really like the revised naming. It removes the Factory and Endpoint abstractions, and does so without drifting from the concepts used in the other SDKs and the specification docs. This version is easier to understand than the existing code for someone coming from another SDK or from the docs - in addition to accomplishing the needed restructuring.

@MackinnonBuck
Copy link
Collaborator Author

Really appreicate the feedback, @PederHP!

I agree that the previous naming could have created confusion since, as you mentioned, the distinction between a "client" and a "client session" doesn't really exist in the MCP spec. And developers might be surprised if "McpClient" means something different in the C# SDK than SDKs for other langs.

@MackinnonBuck
Copy link
Collaborator Author

We should also consider renaming SseClientTransport and related types.

My proposal would be to call it HttpClientTransport. This aligns with the existing HttpTransportMode type, and we already have HttpServerTransportOptions for the server side.

@PederHP
Copy link
Collaborator

PederHP commented Sep 2, 2025

We should also consider renaming SseClientTransport and related types.

My proposal would be to call it HttpClientTransport. This aligns with the existing HttpTransportMode type, and we already have HttpServerTransportOptions for the server side.

I agree that it should be renamed. I was drafting a PR at one point to align with the other SDKs but then got sidetracked and kinda forgot about it. There's a discrepancy where the C# SDK uses SseClientTransport as the entry point for both SSE and Streaming HTTP.

I've had to help multiple developers, online and my day job, on how to handle config-based clients, because the other SDKs have three different Transport types and just map http, sse, and stdio accordingly. I actually think it's more convenient to do like in the C# SDK, as there's less code duplication and it's cleaner, but the naming is kind of confusing. Changing it as you suggest, aligns with Sse being deprecated and not a name that's great to have hanging around, and it removes any confusion on where to find the transport for the http type (using the almost de facto config schema that many clients use).

I know that using the transport types directly is much less relevant after this change, but I still think renaming the transport and related types would be helpful in making the SDK easier to use for polyglot developers.

Copy link
Contributor

@halter73 halter73 left a comment

Choose a reason for hiding this comment

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

I think all the renames are clear improvements over the older names. I still think having "ConnectAsync" and "LaunchAsync" methods that don't require you to initialize a transport to get a client could be nice, but that could be a follow up.

I'm curious on how long we plan to keep around the obsoleted types and methods. I hope we can fully delete them by 1.0, but I see the value in keeping them for a few preview releases at least to make it easier to upgrade.

I'd also prefer to keep something like the McpEndpint._disposeLock SemaphoreSlim for similar reasons we have KestrelServerImple._stoppedTcs. It's not exactly the same because Kestrel waits for middleware to complete during shutdown while the _messageProcessingTask does not currently wait on any handlers, but I don't think it's much more complicated than the CompareExchange on _isDisposed, and it's arguably more correct especially if we do ever end up waiting on handlers.

Copy link
Member

@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.

Thanks, good change overall.

@MackinnonBuck MackinnonBuck merged commit 38b4a26 into modelcontextprotocol:main Sep 16, 2025
8 checks passed
@jeffhandley jeffhandley added the breaking-change This issue or PR introduces a breaking change label Sep 23, 2025
@MackinnonBuck
Copy link
Collaborator Author

Breaking Changes (Migration Summary)

Update your code for the following renamed and restructured APIs.

1. Interfaces replaced by abstract classes

Replace usages of the obsolete interfaces with the new abstract types:

Old New
IMcpClient McpClient
IMcpServer McpServer
IMcpEndpoint McpSession

Action: Change variable, field, and parameter types. (Optional but recommended: rename identifiers like endpointsession for clarity.) The obsolete interfaces remain temporarily with [Obsolete] warnings.

2. Factory classes deprecated

Old New
McpClientFactory.CreateAsync(...) McpClient.CreateAsync(...)
McpServerFactory.CreateAsync(...) McpServer.CreateAsync(...)

Action: Replace calls to the factory classes with the static factory methods on the abstract classes.

3. Transport rename

Old New
SseClientTransport HttpClientTransport
SseClientTransportOptions HttpClientTransportOptions

Action: Update usage of these types to utilize the new type names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This issue or PR introduces a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert IMcpEndpoint, IMcpClient, and IMcpServer types to classes.
6 participants