Remove InputOutputStreams and clean up StdioServerTransport ctors (#71)

- There doesn't appear to be a meaningful scenario for injecting different readers/writers than stdin/stdout, and having InputOutputStreams complicates the API.
- Logically folks talk about "input and output", "readers and writers", etc., and "stdio" is "standard input and output", so having input specified first is less likely to be a source of bugs.
- Logger should be optional, not required, to simplify the common and getting-started cases.

Author: stephentoub

src/mcpdotnet/Configuration/McpServerServiceCollectionExtension.cs

@@ -39,7 +39,6 @@ public static IMcpServerBuilder AddMcpServer(this IServiceCollection services, M
         services.AddSingleton<IMcpServerFactory, McpServerFactory>();
         services.AddHostedService<McpServerHostedService>();
         services.AddOptions();
-        services.AddSingleton(InputOutputStreams.Default);
 
         services.AddSingleton<IMcpServer>(sp => sp.GetRequiredService<IMcpServerFactory>().CreateServer());
 

src/mcpdotnet/Protocol/Transport/InputOutputStreams.cs

@@ -1,15 +1,18 @@
 using System.Text.Json;
+using McpDotNet.Configuration;
 using McpDotNet.Logging;
 using McpDotNet.Protocol.Messages;
 using McpDotNet.Server;
 using McpDotNet.Utils.Json;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Abstractions;
 
+#pragma warning disable CA2208 // Instantiate argument exceptions correctly
+
 namespace McpDotNet.Protocol.Transport;
 
 /// <summary>
-/// Implements the MCP transport protocol over standard input/output streams.
+/// Provides an implementation of the MCP transport protocol over standard input/output streams.
 /// </summary>
 public sealed class StdioServerTransport : TransportBase, IServerTransport
 {
@@ -24,47 +27,90 @@ public sealed class StdioServerTransport : TransportBase, IServerTransport
     private string EndpointName => $"Server (stdio) ({_serverName})";
 
     /// <summary>
-    /// Initializes a new instance of the StdioServerTransport class.
+    /// Initializes a new instance of the <see cref="StdioServerTransport"/> class, using
+    /// <see cref="Console.In"/> and <see cref="Console.Out"/> for input and output streams.
     /// </summary>
     /// <param name="serverOptions">The server options.</param>
-    /// <param name="loggerFactory">A logger factory for creating loggers.</param>
-    /// <param name="inputOutputStreams">Container for the input and output streams</param>
-    public StdioServerTransport(McpServerOptions serverOptions, ILoggerFactory? loggerFactory, InputOutputStreams inputOutputStreams)
-        : this(
-              serverOptions is not null ? serverOptions.ServerInfo.Name : throw new ArgumentNullException(nameof(serverOptions)),
-              loggerFactory,
-              inputOutputStreams is not null ? inputOutputStreams.Output : throw new ArgumentNullException(nameof(inputOutputStreams)),
-              inputOutputStreams.Input
-              )
+    /// <param name="loggerFactory">Optional logger factory used for logging employed by the transport.</param>
+    /// <exception cref="ArgumentNullException"><paramref name="serverOptions"/> is <see langword="null"/> or contains a null name.</exception>
+    /// <remarks>
+    /// <para>
+    /// By default, no logging is performed. If a <paramref name="loggerFactory"/> is supplied, it must not log
+    /// to <see cref="Console.Out"/>, as that will interfere with the transport's output.
+    /// </para>
+    /// </remarks>
+    public StdioServerTransport(McpServerOptions serverOptions, ILoggerFactory? loggerFactory = null)
+        : this(GetServerName(serverOptions), loggerFactory)
     {
     }
 
     /// <summary>
-    /// Initializes a new instance of the StdioServerTransport class.
+    /// Initializes a new instance of the <see cref="StdioServerTransport"/> class, using
+    /// <see cref="Console.In"/> and <see cref="Console.Out"/> for input and output streams.
     /// </summary>
     /// <param name="serverName">The name of the server.</param>
-    /// <param name="loggerFactory">A logger factory for creating loggers.</param>
-    public StdioServerTransport(string serverName, ILoggerFactory? loggerFactory)
-        : this(serverName, loggerFactory, Console.Out, Console.In)
+    /// <param name="loggerFactory">Optional logger factory used for logging employed by the transport.</param>
+    /// <exception cref="ArgumentNullException"><paramref name="serverName"/> is <see langword="null"/>.</exception>
+    /// <remarks>
+    /// <para>
+    /// By default, no logging is performed. If a <paramref name="loggerFactory"/> is supplied, it must not log
+    /// to <see cref="Console.Out"/>, as that will interfere with the transport's output.
+    /// </para>
+    /// </remarks>
+    public StdioServerTransport(string serverName, ILoggerFactory? loggerFactory = null)
+        : this(serverName ?? throw new ArgumentNullException(nameof(serverName)), Console.In, Console.Out, loggerFactory)
     {
+    }
 
+    /// <summary>
+    /// Initializes a new instance of the <see cref="StdioServerTransport"/> class using the specified
+    /// <paramref name="input"/> and <paramref name="output"/> streams.
+    /// </summary>
+    /// <param name="serverOptions">The server options.</param>
+    /// <param name="input">The reader, from which all input is read.</param>
+    /// <param name="output">The writer, to which all output is written.</param>
+    /// <param name="loggerFactory">Optional logger factory used for logging employed by the transport.</param>
+    /// <exception cref="ArgumentNullException"><paramref name="serverOptions"/> is <see langword="null"/> or contains a null name.</exception>
+    /// <exception cref="ArgumentNullException"><paramref name="input"/> is <see langword="null"/>.</exception>
+    /// <exception cref="ArgumentNullException"><paramref name="output"/> is <see langword="null"/>.</exception>
+    /// <remarks>
+    /// <para>
+    /// By default, no logging is performed. If a <paramref name="loggerFactory"/> is supplied, it must not log
+    /// to <see cref="Console.Out"/>, as that will interfere with the transport's output.
+    /// </para>
+    /// </remarks>
+    public StdioServerTransport(McpServerOptions serverOptions, TextReader input, TextWriter output, ILoggerFactory? loggerFactory = null)
+        : this(GetServerName(serverOptions), input, output, loggerFactory)
+    {
     }
 
     /// <summary>
-    /// Initializes a new instance of the StdioServerTransport class.
+    /// Initializes a new instance of the <see cref="StdioServerTransport"/> class using the specified
+    /// <paramref name="input"/> and <paramref name="output"/> streams.
     /// </summary>
     /// <param name="serverName">The name of the server.</param>
-    /// <param name="loggerFactory">A logger factory for creating loggers.</param>
-    /// <param name="output">The output stream writer</param>
-    /// <param name="input">The input stream reader</param>
-    public StdioServerTransport(string serverName, ILoggerFactory? loggerFactory, TextWriter output, TextReader input)
+    /// <param name="input">The reader, from which all input is read.</param>
+    /// <param name="output">The writer, to which all output is written.</param>
+    /// <param name="loggerFactory">Optional logger factory used for logging employed by the transport.</param>
+    /// <exception cref="ArgumentNullException"><paramref name="serverName"/> is <see langword="null"/>.</exception>
+    /// <exception cref="ArgumentNullException"><paramref name="input"/> is <see langword="null"/>.</exception>
+    /// <exception cref="ArgumentNullException"><paramref name="output"/> is <see langword="null"/>.</exception>
+    /// <remarks>
+    /// <para>
+    /// By default, no logging is performed. If a <paramref name="loggerFactory"/> is supplied, it must not log
+    /// to <see cref="Console.Out"/>, as that will interfere with the transport's output.
+    /// </para>
+    /// </remarks>
+    public StdioServerTransport(string serverName, TextReader input, TextWriter output, ILoggerFactory? loggerFactory = null)
         : base(loggerFactory)
     {
-        _serverName = serverName;
-        _logger = loggerFactory is not null ? loggerFactory.CreateLogger<StdioClientTransport>() : NullLogger.Instance;
-        _jsonOptions = JsonSerializerOptionsExtensions.DefaultOptions;
+        _serverName = serverName ?? throw new ArgumentNullException(nameof(serverName));
         _input = input ?? throw new ArgumentNullException(nameof(input));
         _output = output ?? throw new ArgumentNullException(nameof(output));
+
+        _logger = loggerFactory is not null ? loggerFactory.CreateLogger<StdioClientTransport>() : NullLogger.Instance;
+
+        _jsonOptions = JsonSerializerOptionsExtensions.DefaultOptions;
     }
 
     /// <inheritdoc/>
@@ -224,4 +270,20 @@ private async Task CleanupAsync(CancellationToken cancellationToken)
         SetConnected(false);
         _logger.TransportCleanedUp(EndpointName);
     }
+
+    /// <summary>Validates the <paramref name="serverOptions"/> and extracts from it the server name to use.</summary>
+    private static string GetServerName(McpServerOptions serverOptions)
+    {
+        if (serverOptions is null)
+        {
+            throw new ArgumentNullException(nameof(serverOptions));
+        }
+
+        if (serverOptions.ServerInfo is null)
+        {
+            throw new ArgumentNullException($"{nameof(serverOptions)}.{nameof(serverOptions.ServerInfo)}");
+        }
+
+        return serverOptions.ServerInfo.Name;
+    }
 }

src/mcpdotnet/Protocol/Transport/StdioServerTransport.cs

@@ -31,7 +31,7 @@ public StdioServerTransportTests()
     public async Task Constructor_Should_Initialize_With_Valid_Parameters()
     {
         // Act
-        await using var transport = new StdioServerTransport(_serverOptions, NullLoggerFactory.Instance, InputOutputStreams.Default);
+        await using var transport = new StdioServerTransport(_serverOptions);
 
         // Assert
         Assert.NotNull(transport);
@@ -40,21 +40,27 @@ public async Task Constructor_Should_Initialize_With_Valid_Parameters()
     [Fact]
     public void Constructor_Throws_For_Null_Options()
     {
-        var exception = Assert.Throws<ArgumentNullException>(() => new StdioServerTransport(serverOptions: null!, NullLoggerFactory.Instance, InputOutputStreams.Default));
-        Assert.Equal("serverOptions", exception.ParamName);
+        Assert.Throws<ArgumentNullException>("serverName", () => new StdioServerTransport((string)null!));
+
+        Assert.Throws<ArgumentNullException>("serverOptions", () => new StdioServerTransport((McpServerOptions)null!));
+        Assert.Throws<ArgumentNullException>("serverOptions.ServerInfo", () => new StdioServerTransport(new McpServerOptions() { ServerInfo = null! }));
+        Assert.Throws<ArgumentNullException>("serverName", () => new StdioServerTransport(new McpServerOptions() { ServerInfo = new() { Name = null!, Version = "" } }));
     }
 
     [Fact]
     public void Constructor_Throws_For_Null_Streams()
     {
-        var exception = Assert.Throws<ArgumentNullException>(() => new StdioServerTransport(_serverOptions, NullLoggerFactory.Instance, null!));
-        Assert.Equal("inputOutputStreams", exception.ParamName);
+        Assert.Throws<ArgumentNullException>("input", () => new StdioServerTransport(_serverOptions, null!, Console.Out));
+        Assert.Throws<ArgumentNullException>("input", () => new StdioServerTransport("name", null!, Console.Out));
+
+        Assert.Throws<ArgumentNullException>("output", () => new StdioServerTransport("name", Console.In, null!));
+        Assert.Throws<ArgumentNullException>("output", () => new StdioServerTransport(_serverOptions, Console.In, null!));
     }
 
     [Fact]
     public async Task StartListeningAsync_Should_Set_Connected_State()
     {
-        await using var transport = new StdioServerTransport(_serverOptions, NullLoggerFactory.Instance, InputOutputStreams.Default);
+        await using var transport = new StdioServerTransport(_serverOptions);
 
         await transport.StartListeningAsync();
 
@@ -64,16 +70,18 @@ public async Task StartListeningAsync_Should_Set_Connected_State()
     [Fact]
     public async Task SendMessageAsync_Should_Send_Message()
     {
-        var streams = new InputOutputStreams { Output = new StringWriter(), Input = new StringReader("") };
-        await using var transport = new StdioServerTransport(_serverOptions, NullLoggerFactory.Instance, streams);
+        var input = new StringReader("");
+        var output = new StringWriter();
+
+        await using var transport = new StdioServerTransport(_serverOptions, input, output, NullLoggerFactory.Instance);
         await transport.StartListeningAsync();
 
         var message = new JsonRpcRequest { Method = "test", Id = RequestId.FromNumber(44) };
 
 
         await transport.SendMessageAsync(message);
 
-        var result = streams.Output.ToString()?.Trim();
+        var result = output.ToString()?.Trim();
         var expected = JsonSerializer.Serialize(message, JsonSerializerOptionsExtensions.DefaultOptions);
 
         Assert.Equal(expected, result);
@@ -82,7 +90,7 @@ public async Task SendMessageAsync_Should_Send_Message()
     [Fact]
     public async Task SendMessageAsync_Throws_Exception_If_Not_Connected()
     {
-        await using var transport = new StdioServerTransport(_serverOptions, NullLoggerFactory.Instance, InputOutputStreams.Default);
+        await using var transport = new StdioServerTransport(_serverOptions);
 
         var message = new JsonRpcRequest { Method = "test" };
 
@@ -92,7 +100,7 @@ public async Task SendMessageAsync_Throws_Exception_If_Not_Connected()
     [Fact]
     public async Task DisposeAsync_Should_Dispose_Resources()
     {
-        await using var transport = new StdioServerTransport(_serverOptions, NullLoggerFactory.Instance, InputOutputStreams.Default);
+        await using var transport = new StdioServerTransport(_serverOptions);
 
         await transport.DisposeAsync();
 
@@ -105,10 +113,10 @@ public async Task ReadMessagesAsync_Should_Read_Messages()
         var message = new JsonRpcRequest { Method = "test", Id = RequestId.FromNumber(44) };
         var json = JsonSerializer.Serialize(message, JsonSerializerOptionsExtensions.DefaultOptions);
 
-        using var sr = new StringReader(json);
+        var input = new StringReader(json);
+        var output = new StringWriter();
 
-        var streams = new InputOutputStreams { Output = new StringWriter(), Input = new StringReader(json) };
-        await using var transport = new StdioServerTransport(_serverOptions, NullLoggerFactory.Instance, streams);
+        await using var transport = new StdioServerTransport(_serverOptions, input, output);
         await transport.StartListeningAsync();
 
         var canRead = await transport.MessageReader.WaitToReadAsync();
@@ -123,7 +131,7 @@ public async Task ReadMessagesAsync_Should_Read_Messages()
     [Fact]
     public async Task CleanupAsync_Should_Cleanup_Resources()
     {
-        var transport = new StdioServerTransport(_serverOptions, NullLoggerFactory.Instance, InputOutputStreams.Default);
+        var transport = new StdioServerTransport(_serverOptions);
         await transport.StartListeningAsync();
 
         await transport.DisposeAsync();