Address more feedback and further cleanup

stephentoub · stephentoub · commit 824d01d1dd71 · 2025-04-04T13:01:58.000-04:00
diff --git a/samples/QuickstartWeatherServer/Tools/WeatherTools.cs b/samples/QuickstartWeatherServer/Tools/WeatherTools.cs
@@ -1,7 +1,6 @@
 using ModelContextProtocol;
 using ModelContextProtocol.Server;
 using System.ComponentModel;
-using System.Net.Http.Json;
 using System.Text.Json;
 
 namespace QuickstartWeatherServer.Tools;
diff --git a/src/ModelContextProtocol/Client/McpClient.cs b/src/ModelContextProtocol/Client/McpClient.cs
@@ -5,17 +5,18 @@
 using ModelContextProtocol.Protocol.Types;
 using ModelContextProtocol.Shared;
 using ModelContextProtocol.Utils.Json;
-using System.Diagnostics;
-using System.Reflection;
 using System.Text.Json;
 
 namespace ModelContextProtocol.Client;
 
 /// <inheritdoc/>
 internal sealed class McpClient : McpEndpoint, IMcpClient
 {
-    /// <summary>Cached naming information used for client name/version when none is specified.</summary>
-    private static readonly AssemblyName s_asmName = (Assembly.GetEntryAssembly() ?? Assembly.GetExecutingAssembly()).GetName();
+    private static Implementation DefaultImplementation { get; } = new()
+    {
+        Name = DefaultAssemblyName.Name ?? nameof(McpClient),
+        Version = DefaultAssemblyName.Version?.ToString() ?? "1.0.0",
+    };
 
     private readonly IClientTransport _clientTransport;
     private readonly McpClientOptions _options;
@@ -37,17 +38,9 @@ internal sealed class McpClient : McpEndpoint, IMcpClient
     public McpClient(IClientTransport clientTransport, McpClientOptions? options, McpServerConfig serverConfig, ILoggerFactory? loggerFactory)
         : base(loggerFactory)
     {
-        _clientTransport = clientTransport;
+        options ??= new();
 
-        if (options?.ClientInfo is null)
-        {
-            options = options?.Clone() ?? new();
-            options.ClientInfo = new()
-            {
-                Name = s_asmName.Name ?? nameof(McpClient),
-                Version = s_asmName.Version?.ToString() ?? "1.0.0",
-            };
-        }
+        _clientTransport = clientTransport;
         _options = options;
 
         EndpointName = $"Client ({serverConfig.Id}: {serverConfig.Name})";
@@ -122,14 +115,13 @@ public async Task ConnectAsync(CancellationToken cancellationToken = default)
             try
             {
                 // Send initialize request
-                Debug.Assert(_options.ClientInfo is not null, "ClientInfo should be set by the constructor");
                 var initializeResponse = await this.SendRequestAsync(
                     RequestMethods.Initialize,
                     new InitializeRequestParams
                     {
                         ProtocolVersion = _options.ProtocolVersion,
                         Capabilities = _options.Capabilities ?? new ClientCapabilities(),
-                        ClientInfo = _options.ClientInfo!
+                        ClientInfo = _options.ClientInfo ?? DefaultImplementation,
                     },
                     McpJsonUtilities.JsonContext.Default.InitializeRequestParams,
                     McpJsonUtilities.JsonContext.Default.InitializeResult,
diff --git a/src/ModelContextProtocol/Client/McpClientFactory.cs b/src/ModelContextProtocol/Client/McpClientFactory.cs
@@ -5,7 +5,6 @@
 using ModelContextProtocol.Utils;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Abstractions;
-using System.Reflection;
 
 namespace ModelContextProtocol.Client;
 
diff --git a/src/ModelContextProtocol/Client/McpClientOptions.cs b/src/ModelContextProtocol/Client/McpClientOptions.cs
@@ -28,14 +28,4 @@ public class McpClientOptions
     /// Timeout for initialization sequence.
     /// </summary>
     public TimeSpan InitializationTimeout { get; set; } = TimeSpan.FromSeconds(60);
-
-    /// <summary>Creates a shallow clone of the options.</summary>
-    internal McpClientOptions Clone() =>
-        new()
-        {
-            ClientInfo = ClientInfo,
-            Capabilities = Capabilities,
-            ProtocolVersion = ProtocolVersion,
-            InitializationTimeout = InitializationTimeout
-        };
 }
diff --git a/src/ModelContextProtocol/Client/McpClientTool.cs b/src/ModelContextProtocol/Client/McpClientTool.cs
@@ -1,6 +1,5 @@
 ﻿using ModelContextProtocol.Protocol.Types;
 using ModelContextProtocol.Utils.Json;
-using ModelContextProtocol.Utils;
 using Microsoft.Extensions.AI;
 using System.Text.Json;
 
diff --git a/src/ModelContextProtocol/Configuration/McpServerOptionsSetup.cs b/src/ModelContextProtocol/Configuration/McpServerOptionsSetup.cs
@@ -1,5 +1,4 @@
-﻿using System.Reflection;
-using ModelContextProtocol.Server;
+﻿using ModelContextProtocol.Server;
 using Microsoft.Extensions.Options;
 using ModelContextProtocol.Utils;
 
@@ -25,18 +24,6 @@ public void Configure(McpServerOptions options)
     {
         Throw.IfNull(options);
 
-        // Configure the option's server information based on the current process,
-        // if it otherwise lacks server information.
-        if (options.ServerInfo is not { } serverInfo)
-        {
-            var assemblyName = Assembly.GetEntryAssembly()?.GetName();
-            options.ServerInfo = new()
-            {
-                Name = assemblyName?.Name ?? "McpServer",
-                Version = assemblyName?.Version?.ToString() ?? "1.0.0",
-            };
-        }
-
         // Collect all of the provided tools into a tools collection. If the options already has
         // a collection, add to it, otherwise create a new one. We want to maintain the identity
         // of an existing collection in case someone has provided their own derived type, wants
diff --git a/src/ModelContextProtocol/Protocol/Transport/StdioClientTransport.cs b/src/ModelContextProtocol/Protocol/Transport/StdioClientTransport.cs
@@ -2,7 +2,6 @@
 using Microsoft.Extensions.Logging.Abstractions;
 using ModelContextProtocol.Logging;
 using ModelContextProtocol.Utils;
-using System.ComponentModel;
 using System.Diagnostics;
 using System.Text;
 
diff --git a/src/ModelContextProtocol/Protocol/Transport/StdioServerTransport.cs b/src/ModelContextProtocol/Protocol/Transport/StdioServerTransport.cs
@@ -70,9 +70,7 @@ public StdioServerTransport(string serverName, ILoggerFactory? loggerFactory = n
     private static string GetServerName(McpServerOptions serverOptions)
     {
         Throw.IfNull(serverOptions);
-        Throw.IfNull(serverOptions.ServerInfo);
-        Throw.IfNull(serverOptions.ServerInfo.Name);
 
-        return serverOptions.ServerInfo.Name;
+        return serverOptions.ServerInfo?.Name ?? McpServer.DefaultImplementation.Name;
     }
 }
diff --git a/src/ModelContextProtocol/Protocol/Types/ListRootsRequestParams.cs b/src/ModelContextProtocol/Protocol/Types/ListRootsRequestParams.cs
@@ -1,6 +1,4 @@
-﻿using ModelContextProtocol.Protocol.Messages;
-
-namespace ModelContextProtocol.Protocol.Types;
+﻿namespace ModelContextProtocol.Protocol.Types;
 
 /// <summary>
 /// A request from the server to get a list of root URIs from the client.
diff --git a/src/ModelContextProtocol/Server/McpServer.cs b/src/ModelContextProtocol/Server/McpServer.cs
@@ -11,6 +11,12 @@ namespace ModelContextProtocol.Server;
 /// <inheritdoc />
 internal sealed class McpServer : McpEndpoint, IMcpServer
 {
+    internal static Implementation DefaultImplementation { get; } = new()
+    {
+        Name = DefaultAssemblyName.Name ?? nameof(McpServer),
+        Version = DefaultAssemblyName.Version?.ToString() ?? "1.0.0",
+    };
+
     private readonly EventHandler? _toolsChangedDelegate;
     private readonly EventHandler? _promptsChangedDelegate;
 
@@ -32,9 +38,11 @@ public McpServer(ITransport transport, McpServerOptions options, ILoggerFactory?
         Throw.IfNull(transport);
         Throw.IfNull(options);
 
+        options ??= new();
+
         ServerOptions = options;
         Services = serviceProvider;
-        _endpointName = $"Server ({options.ServerInfo.Name} {options.ServerInfo.Version})";
+        _endpointName = $"Server ({options.ServerInfo?.Name ?? DefaultImplementation.Name} {options.ServerInfo?.Version ?? DefaultImplementation.Version})";
 
         _toolsChangedDelegate = delegate
         {
@@ -158,7 +166,7 @@ private void SetInitializeHandler(McpServerOptions options)
                 {
                     ProtocolVersion = options.ProtocolVersion,
                     Instructions = options.ServerInstructions,
-                    ServerInfo = options.ServerInfo,
+                    ServerInfo = options.ServerInfo ?? DefaultImplementation,
                     Capabilities = ServerCapabilities ?? new(),
                 });
             },
diff --git a/src/ModelContextProtocol/Server/McpServerExtensions.cs b/src/ModelContextProtocol/Server/McpServerExtensions.cs
@@ -1,5 +1,4 @@
 ﻿using Microsoft.Extensions.AI;
-using ModelContextProtocol.Client;
 using ModelContextProtocol.Protocol.Messages;
 using ModelContextProtocol.Protocol.Types;
 using ModelContextProtocol.Utils;
diff --git a/src/ModelContextProtocol/Server/McpServerOptions.cs b/src/ModelContextProtocol/Server/McpServerOptions.cs
@@ -14,7 +14,7 @@ public class McpServerOptions
     /// <summary>
     /// Information about this server implementation.
     /// </summary>
-    public required Implementation ServerInfo { get; set; }
+    public Implementation? ServerInfo { get; set; }
 
     /// <summary>
     /// Server capabilities to advertise to the server.
diff --git a/src/ModelContextProtocol/Shared/McpEndpoint.cs b/src/ModelContextProtocol/Shared/McpEndpoint.cs
@@ -6,7 +6,7 @@
 using ModelContextProtocol.Server;
 using ModelContextProtocol.Utils;
 using System.Diagnostics.CodeAnalysis;
-using System.Text.Json.Serialization.Metadata;
+using System.Reflection;
 
 namespace ModelContextProtocol.Shared;
 
@@ -19,6 +19,9 @@ namespace ModelContextProtocol.Shared;
 /// </summary>
 internal abstract class McpEndpoint : IAsyncDisposable
 {
+    /// <summary>Cached naming information used for name/version when none is specified.</summary>
+    internal static AssemblyName DefaultAssemblyName { get; } = (Assembly.GetEntryAssembly() ?? Assembly.GetExecutingAssembly()).GetName();
+
     private McpSession? _session;
     private CancellationTokenSource? _sessionCts;
 
diff --git a/src/ModelContextProtocol/Shared/McpSession.cs b/src/ModelContextProtocol/Shared/McpSession.cs
@@ -3,7 +3,6 @@
 using ModelContextProtocol.Logging;
 using ModelContextProtocol.Protocol.Messages;
 using ModelContextProtocol.Protocol.Transport;
-using ModelContextProtocol.Server;
 using ModelContextProtocol.Utils;
 using ModelContextProtocol.Utils.Json;
 using System.Collections.Concurrent;
diff --git a/tests/ModelContextProtocol.TestServer/Program.cs b/tests/ModelContextProtocol.TestServer/Program.cs
@@ -38,7 +38,6 @@ private static async Task Main(string[] args)
 
         McpServerOptions options = new()
         {
-            ServerInfo = new Implementation() { Name = "TestServer", Version = "1.0.0" },
             Capabilities = new ServerCapabilities()
             {
                 Tools = ConfigureTools(),
diff --git a/tests/ModelContextProtocol.TestSseServer/Program.cs b/tests/ModelContextProtocol.TestSseServer/Program.cs
@@ -25,7 +25,6 @@ private static void ConfigureSerilog(ILoggingBuilder loggingBuilder)
 
     private static void ConfigureOptions(McpServerOptions options)
     {
-        options.ServerInfo = new Implementation() { Name = "TestServer", Version = "1.0.0" };
         options.Capabilities = new ServerCapabilities()
         {
             Tools = new(),
diff --git a/tests/ModelContextProtocol.Tests/Client/McpClientFactoryTests.cs b/tests/ModelContextProtocol.Tests/Client/McpClientFactoryTests.cs
@@ -11,29 +11,24 @@ namespace ModelContextProtocol.Tests.Client;
 
 public class McpClientFactoryTests
 {
-    private readonly McpClientOptions _defaultOptions = new()
-    {
-        ClientInfo = new() { Name = "TestClient", Version = "1.0.0" }
-    };
-
     [Fact]
     public async Task CreateAsync_WithInvalidArgs_Throws()
     {
-        await Assert.ThrowsAsync<ArgumentNullException>("serverConfig", () => McpClientFactory.CreateAsync((McpServerConfig)null!, _defaultOptions, cancellationToken: TestContext.Current.CancellationToken));
+        await Assert.ThrowsAsync<ArgumentNullException>("serverConfig", () => McpClientFactory.CreateAsync((McpServerConfig)null!, cancellationToken: TestContext.Current.CancellationToken));
 
         await Assert.ThrowsAsync<ArgumentException>("serverConfig", () => McpClientFactory.CreateAsync(new McpServerConfig()
             {
                 Name = "name",
                 Id = "id",
                 TransportType = "somethingunsupported",
-            }, _defaultOptions, cancellationToken: TestContext.Current.CancellationToken));
+            }, cancellationToken: TestContext.Current.CancellationToken));
 
         await Assert.ThrowsAsync<InvalidOperationException>(() => McpClientFactory.CreateAsync(new McpServerConfig()
             {
                 Name = "name",
                 Id = "id",
                 TransportType = TransportTypes.StdIo,
-            }, _defaultOptions, (_, __) => null!, cancellationToken: TestContext.Current.CancellationToken));
+            }, createTransportFunc: (_, __) => null!, cancellationToken: TestContext.Current.CancellationToken));
     }
 
     [Fact]
@@ -78,8 +73,7 @@ public async Task CreateAsync_WithValidStdioConfig_CreatesNewClient()
         // Act
         await using var client = await McpClientFactory.CreateAsync(
             serverConfig,
-            _defaultOptions,
-            (_, __) => new NopTransport(),
+            createTransportFunc: (_, __) => new NopTransport(),
             cancellationToken: TestContext.Current.CancellationToken);
 
         // Assert
@@ -102,8 +96,7 @@ public async Task CreateAsync_WithNoTransportOptions_CreatesNewClient()
         // Act
         await using var client = await McpClientFactory.CreateAsync(
             serverConfig,
-            _defaultOptions,
-            (_, __) => new NopTransport(),
+            createTransportFunc: (_, __) => new NopTransport(),
             cancellationToken: TestContext.Current.CancellationToken);
 
         // Assert
@@ -126,8 +119,7 @@ public async Task CreateAsync_WithValidSseConfig_CreatesNewClient()
         // Act
         await using var client = await McpClientFactory.CreateAsync(
             serverConfig,
-            _defaultOptions,
-            (_, __) => new NopTransport(),
+            createTransportFunc: (_, __) => new NopTransport(),
             cancellationToken: TestContext.Current.CancellationToken);
 
         // Assert
@@ -157,8 +149,7 @@ public async Task CreateAsync_WithSse_CreatesCorrectTransportOptions()
         // Act
         await using var client = await McpClientFactory.CreateAsync(
             serverConfig,
-            _defaultOptions,
-            (_, __) => new NopTransport(),
+            createTransportFunc: (_, __) => new NopTransport(),
             cancellationToken: TestContext.Current.CancellationToken);
 
         // Assert
@@ -186,7 +177,7 @@ public async Task McpFactory_WithInvalidTransportOptions_ThrowsFormatException(s
         };
 
         // act & assert
-        await Assert.ThrowsAsync<ArgumentException>(() => McpClientFactory.CreateAsync(config, _defaultOptions, cancellationToken: TestContext.Current.CancellationToken));
+        await Assert.ThrowsAsync<ArgumentException>(() => McpClientFactory.CreateAsync(config, cancellationToken: TestContext.Current.CancellationToken));
     }
 
     [Theory]
@@ -205,11 +196,6 @@ public async Task CreateAsync_WithCapabilitiesOptions(Type transportType)
 
         var clientOptions = new McpClientOptions
         {
-            ClientInfo = new Implementation 
-            {
-                Name = "TestClient", 
-                Version = "1.0.0.0"
-            },
             Capabilities = new ClientCapabilities
             {
                 Sampling = new SamplingCapability
diff --git a/tests/ModelContextProtocol.Tests/ClientIntegrationTests.cs b/tests/ModelContextProtocol.Tests/ClientIntegrationTests.cs
@@ -5,10 +5,7 @@
 using ModelContextProtocol.Protocol.Types;
 using ModelContextProtocol.Tests.Utils;
 using OpenAI;
-using System.Text.Encodings.Web;
 using System.Text.Json;
-using System.Text.Json.Serialization;
-using System.Text.Json.Serialization.Metadata;
 
 namespace ModelContextProtocol.Tests;
 
@@ -367,7 +364,6 @@ public async Task Sampling_Stdio(string clientId)
         int samplingHandlerCalls = 0;
         await using var client = await _fixture.CreateClientAsync(clientId, new()
         {
-            ClientInfo = new() { Name = "Sampling_Stdio", Version = "1.0.0" },
             Capabilities = new()
             {
                 Sampling = new()
@@ -532,7 +528,6 @@ public async Task SamplingViaChatClient_RequestResponseProperlyPropagated()
             .CreateSamplingHandler();
         await using var client = await McpClientFactory.CreateAsync(_fixture.EverythingServerConfig, new()
         {
-            ClientInfo = new() { Name = nameof(SamplingViaChatClient_RequestResponseProperlyPropagated), Version = "1.0.0" },
             Capabilities = new()
             {
                 Sampling = new()
diff --git a/tests/ModelContextProtocol.Tests/DiagnosticTests.cs b/tests/ModelContextProtocol.Tests/DiagnosticTests.cs
@@ -1,6 +1,5 @@
 ﻿using ModelContextProtocol.Client;
 using ModelContextProtocol.Protocol.Transport;
-using ModelContextProtocol.Protocol.Types;
 using ModelContextProtocol.Server;
 using OpenTelemetry.Trace;
 using System.Diagnostics;
@@ -49,7 +48,6 @@ private static async Task RunConnected(Func<IMcpClient, IMcpServer, Task> action
 
         await using (IMcpServer server = McpServerFactory.Create(serverTransport, new()
             {
-                ServerInfo = new Implementation { Name = "TestServer", Version = "1.0.0" },
                 Capabilities = new()
                 {
                     Tools = new()
diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerFactoryTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerFactoryTests.cs
@@ -1,5 +1,4 @@
-﻿using ModelContextProtocol.Protocol.Types;
-using ModelContextProtocol.Server;
+﻿using ModelContextProtocol.Server;
 using ModelContextProtocol.Tests.Utils;
 
 namespace ModelContextProtocol.Tests.Server;
@@ -13,7 +12,6 @@ public McpServerFactoryTests(ITestOutputHelper testOutputHelper)
     {
         _options = new McpServerOptions
         {
-            ServerInfo = new Implementation { Name = "TestServer", Version = "1.0" },
             ProtocolVersion = "1.0",
             InitializationTimeout = TimeSpan.FromSeconds(30)
         };
diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs
diff --git a/tests/ModelContextProtocol.Tests/SseIntegrationTests.cs b/tests/ModelContextProtocol.Tests/SseIntegrationTests.cs
diff --git a/tests/ModelContextProtocol.Tests/SseServerIntegrationTestFixture.cs b/tests/ModelContextProtocol.Tests/SseServerIntegrationTestFixture.cs
diff --git a/tests/ModelContextProtocol.Tests/SseServerIntegrationTests.cs b/tests/ModelContextProtocol.Tests/SseServerIntegrationTests.cs
diff --git a/tests/ModelContextProtocol.Tests/Transport/StdioServerTransportTests.cs b/tests/ModelContextProtocol.Tests/Transport/StdioServerTransportTests.cs

Original file line number	Diff line number	Diff line change
`@@ -70,9 +70,7 @@ public StdioServerTransport(string serverName, ILoggerFactory? loggerFactory = n`
`70`	`70`	`private static string GetServerName(McpServerOptions serverOptions)`
`71`	`71`	`{`
`72`	`72`	`Throw.IfNull(serverOptions);`
`73`		`- Throw.IfNull(serverOptions.ServerInfo);`
`74`		`- Throw.IfNull(serverOptions.ServerInfo.Name);`
`75`	`73`
`76`		`- return serverOptions.ServerInfo.Name;`
	`74`	`+ return serverOptions.ServerInfo?.Name ?? McpServer.DefaultImplementation.Name;`
`77`	`75`	`}`
`78`	`76`	`}`