Skip to content
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

Make McpClientFactory accept an IClientTransport directly instead of McpServerConfig #190

Open
eiriktsarpalis opened this issue Apr 2, 2025 · 0 comments
Assignees
Labels
enhancement New feature or request

Comments

@eiriktsarpalis
Copy link
Contributor

eiriktsarpalis commented Apr 2, 2025

each transport implementation should define its own options object if necessary

That sounds reasonable.

Originally posted by @stephentoub in #181

Spinning off this conversation into a new issue. What I have in mind roughly is change the McpClientFactory signature to be the following:

public static class McpClientFactory
{
    public static async Task<IMcpClient> CreateAsync(
        IClientTransport transport,
        McpClientOptions? clientOptions = null,
        ILoggerFactory? loggerFactory = null,
        CancellationToken cancellationToken = default);
}

Then exposing bespoke configuration options for individual transports

public record StdioClientTransportOptions
{
     public string Executable { get; init; }
     public IReadOnlyList<string> Arguments { get; init; }
     /* etc etc */
}

public class StdioClientTransport : IClientTransport
{
     public StdioClientTransport(StdioClientTransportOptions options, ....); // No McpServerConfig argument anymore
}

This seems perfectly doable, the only blocker in my view is that ITransport instances are currently stateful. For this to work best we need to be able to offload all their state in session objects created by the client factory. cc @halter73

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants