Address feedback

ericstj · ericstj · commit c3df5c5cb761 · 2026-04-03T13:36:33.000-07:00
diff --git a/src/ModelContextProtocol.Core/Client/StreamClientSessionTransport.cs b/src/ModelContextProtocol.Core/Client/StreamClientSessionTransport.cs
@@ -15,7 +15,7 @@ internal class StreamClientSessionTransport : TransportBase
     private readonly TextReader _serverOutput;
     private readonly Stream _serverInputStream;
     private readonly SemaphoreSlim _sendLock = new(1, 1);
-    private CancellationTokenSource? _shutdownCts = new();
+    private readonly CancellationTokenSource _shutdownCts = new();
     private Task? _readTask;
 
     /// <summary>
@@ -182,24 +182,14 @@ private async Task ProcessMessageAsync(string line, CancellationToken cancellati
     /// </summary>
     protected void CancelShutdown()
     {
-        try
-        {
-            _shutdownCts?.Cancel();
-        }
-        catch (ObjectDisposedException)
-        {
-        }
+        _shutdownCts.Cancel();
     }
 
     protected virtual async ValueTask CleanupAsync(Exception? error = null, CancellationToken cancellationToken = default)
     {
         LogTransportShuttingDown(Name);
 
-        if (Interlocked.Exchange(ref _shutdownCts, null) is { } shutdownCts)
-        {
-            await shutdownCts.CancelAsync().ConfigureAwait(false);
-            shutdownCts.Dispose();
-        }
+        await _shutdownCts.CancelAsync().ConfigureAwait(false);
 
         if (Interlocked.Exchange(ref _readTask, null) is Task readTask)
         {
diff --git a/tests/ModelContextProtocol.Tests/Configuration/McpServerOptionsSetupTests.cs b/tests/ModelContextProtocol.Tests/Configuration/McpServerOptionsSetupTests.cs
@@ -164,7 +164,7 @@ public void Configure_WithManualResourceSubscribeCapability_AndWithResources_Pre
     }
 
     [Fact]
-    public async Task ServerCapabilities_WithManualResourceSubscribeCapability_AndWithResources_ExposesSubscribeCapability()
+    public void ServerCapabilities_WithManualResourceSubscribeCapability_AndWithResources_ExposesSubscribeCapability()
     {
         // This test would require a full client-server setup, so we'll test via options validation instead
         var services = new ServiceCollection();
@@ -183,7 +183,7 @@ public async Task ServerCapabilities_WithManualResourceSubscribeCapability_AndWi
         .WithResources<SimpleResourceType>()
         .WithStreamServerTransport(Stream.Null, Stream.Null);
 
-        await using var sp = services.BuildServiceProvider();
+        using var sp = services.BuildServiceProvider();
         var options = sp.GetRequiredService<IOptions<McpServerOptions>>().Value;
         
         // The options should preserve the user's manually set capabilities
diff --git a/tests/ModelContextProtocol.Tests/Configuration/McpServerResourceCapabilityIntegrationTests.cs b/tests/ModelContextProtocol.Tests/Configuration/McpServerResourceCapabilityIntegrationTests.cs
@@ -85,7 +85,7 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer
     }
 
     [Fact]
-    public async Task Resources_AreExposed_WhenSubscribeCapabilitySetInAddMcpServerOptions()
+    public void Resources_AreExposed_WhenSubscribeCapabilitySetInAddMcpServerOptions()
     {
         // Create a fresh service collection to test the exact scenario from the issue
         var services = new ServiceCollection();
@@ -108,7 +108,7 @@ public async Task Resources_AreExposed_WhenSubscribeCapabilitySetInAddMcpServerO
             .WithResources<LiveResources>()
             .WithStreamServerTransport(Stream.Null, Stream.Null);
 
-        await using var serviceProvider = services.BuildServiceProvider();
+        using var serviceProvider = services.BuildServiceProvider();
         var mcpOptions = serviceProvider.GetRequiredService<IOptions<McpServerOptions>>().Value;
 
         // Verify capabilities are preserved
@@ -122,15 +122,15 @@ public async Task Resources_AreExposed_WhenSubscribeCapabilitySetInAddMcpServerO
     }
 
     [Fact]
-    public async Task ResourcesCapability_IsCreated_WhenOnlyResourcesAreProvided()
+    public void ResourcesCapability_IsCreated_WhenOnlyResourcesAreProvided()
     {
         // Test that ResourcesCapability is created even without handlers or manual setting
         var services = new ServiceCollection();
         var builder = services.AddMcpServer()
             .WithResources<LiveResources>()
             .WithStreamServerTransport(Stream.Null, Stream.Null);
 
-        await using var serviceProvider = services.BuildServiceProvider();
+        using var serviceProvider = services.BuildServiceProvider();
         var mcpOptions = serviceProvider.GetRequiredService<IOptions<McpServerOptions>>().Value;
 
         // Resources are registered
diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerLoggingLevelTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerLoggingLevelTests.cs
@@ -15,29 +15,29 @@ public McpServerLoggingLevelTests()
     }
 
     [Fact]
-    public async Task CanCreateServerWithLoggingLevelHandler()
+    public void CanCreateServerWithLoggingLevelHandler()
     {
         var services = new ServiceCollection();
 
         services.AddMcpServer()
             .WithStreamServerTransport(Stream.Null, Stream.Null)
             .WithSetLoggingLevelHandler(async (ctx, ct) => new EmptyResult());
 
-        await using var provider = services.BuildServiceProvider();
+        using var provider = services.BuildServiceProvider();
 
         provider.GetRequiredService<McpServer>();
     }
 
     [Fact]
-    public async Task AddingLoggingLevelHandlerSetsLoggingCapability()
+    public void AddingLoggingLevelHandlerSetsLoggingCapability()
     {
         var services = new ServiceCollection();
 
         services.AddMcpServer()
             .WithStreamServerTransport(Stream.Null, Stream.Null)
             .WithSetLoggingLevelHandler(async (ctx, ct) => new EmptyResult());
 
-        await using var provider = services.BuildServiceProvider();
+        using var provider = services.BuildServiceProvider();
 
         var server = provider.GetRequiredService<McpServer>();
 
@@ -46,12 +46,12 @@ public async Task AddingLoggingLevelHandlerSetsLoggingCapability()
     }
 
     [Fact]
-    public async Task ServerWithoutCallingLoggingLevelHandlerDoesNotSetLoggingCapability()
+    public void ServerWithoutCallingLoggingLevelHandlerDoesNotSetLoggingCapability()
     {
         var services = new ServiceCollection();
         services.AddMcpServer()
             .WithStreamServerTransport(Stream.Null, Stream.Null);
-        await using var provider = services.BuildServiceProvider();
+        using var provider = services.BuildServiceProvider();
         var server = provider.GetRequiredService<McpServer>();
         Assert.Null(server.ServerOptions.Capabilities?.Logging);
     }
diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerResourceTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerResourceTests.cs
@@ -29,7 +29,7 @@ public McpServerResourceTests()
     }
 
     [Fact]
-    public async Task CanCreateServerWithResource()
+    public void CanCreateServerWithResource()
     {
         var services = new ServiceCollection();
 
@@ -58,14 +58,14 @@ public async Task CanCreateServerWithResource()
                 };
             });
 
-        await using var provider = services.BuildServiceProvider();
+        using var provider = services.BuildServiceProvider();
 
         provider.GetRequiredService<McpServer>();
     }
 
 
     [Fact]
-    public async Task CanCreateServerWithResourceTemplates()
+    public void CanCreateServerWithResourceTemplates()
     {
         var services = new ServiceCollection();
 
@@ -94,13 +94,13 @@ public async Task CanCreateServerWithResourceTemplates()
                 };
             });
 
-        await using var provider = services.BuildServiceProvider();
+        using var provider = services.BuildServiceProvider();
 
         provider.GetRequiredService<McpServer>();
     }
 
     [Fact]
-    public async Task CreatingReadHandlerWithNoListHandlerSucceeds()
+    public void CreatingReadHandlerWithNoListHandlerSucceeds()
     {
         var services = new ServiceCollection();
         services.AddMcpServer()
@@ -117,7 +117,7 @@ public async Task CreatingReadHandlerWithNoListHandlerSucceeds()
                     }]
                 };
             });
-        await using var sp = services.BuildServiceProvider();
+        using var sp = services.BuildServiceProvider();
 
         sp.GetRequiredService<McpServer>();
     }

Original file line number	Diff line number	Diff line change
`@@ -164,7 +164,7 @@ public void Configure_WithManualResourceSubscribeCapability_AndWithResources_Pre`
`164`	`164`	`}`
`165`	`165`
`166`	`166`	`[Fact]`
`167`		`- public async Task ServerCapabilities_WithManualResourceSubscribeCapability_AndWithResources_ExposesSubscribeCapability()`
	`167`	`+ public void ServerCapabilities_WithManualResourceSubscribeCapability_AndWithResources_ExposesSubscribeCapability()`
`168`	`168`	`{`
`169`	`169`	`// This test would require a full client-server setup, so we'll test via options validation instead`
`170`	`170`	`var services = new ServiceCollection();`
`@@ -183,7 +183,7 @@ public async Task ServerCapabilities_WithManualResourceSubscribeCapability_AndWi`
`183`	`183`	`.WithResources<SimpleResourceType>()`
`184`	`184`	`.WithStreamServerTransport(Stream.Null, Stream.Null);`
`185`	`185`
`186`		`- await using var sp = services.BuildServiceProvider();`
	`186`	`+ using var sp = services.BuildServiceProvider();`
`187`	`187`	`var options = sp.GetRequiredService<IOptions<McpServerOptions>>().Value;`
`188`	`188`
`189`	`189`	`// The options should preserve the user's manually set capabilities`
Original file line number	Diff line number	Diff line change
`@@ -15,29 +15,29 @@ public McpServerLoggingLevelTests()`
`15`	`15`	`}`
`16`	`16`
`17`	`17`	`[Fact]`
`18`		`- public async Task CanCreateServerWithLoggingLevelHandler()`
	`18`	`+ public void CanCreateServerWithLoggingLevelHandler()`
`19`	`19`	`{`
`20`	`20`	`var services = new ServiceCollection();`
`21`	`21`
`22`	`22`	`services.AddMcpServer()`
`23`	`23`	`.WithStreamServerTransport(Stream.Null, Stream.Null)`
`24`	`24`	`.WithSetLoggingLevelHandler(async (ctx, ct) => new EmptyResult());`
`25`	`25`
`26`		`- await using var provider = services.BuildServiceProvider();`
	`26`	`+ using var provider = services.BuildServiceProvider();`
`27`	`27`
`28`	`28`	`provider.GetRequiredService<McpServer>();`
`29`	`29`	`}`
`30`	`30`
`31`	`31`	`[Fact]`
`32`		`- public async Task AddingLoggingLevelHandlerSetsLoggingCapability()`
	`32`	`+ public void AddingLoggingLevelHandlerSetsLoggingCapability()`
`33`	`33`	`{`
`34`	`34`	`var services = new ServiceCollection();`
`35`	`35`
`36`	`36`	`services.AddMcpServer()`
`37`	`37`	`.WithStreamServerTransport(Stream.Null, Stream.Null)`
`38`	`38`	`.WithSetLoggingLevelHandler(async (ctx, ct) => new EmptyResult());`
`39`	`39`
`40`		`- await using var provider = services.BuildServiceProvider();`
	`40`	`+ using var provider = services.BuildServiceProvider();`
`41`	`41`
`42`	`42`	`var server = provider.GetRequiredService<McpServer>();`
`43`	`43`
`@@ -46,12 +46,12 @@ public async Task AddingLoggingLevelHandlerSetsLoggingCapability()`
`46`	`46`	`}`
`47`	`47`
`48`	`48`	`[Fact]`
`49`		`- public async Task ServerWithoutCallingLoggingLevelHandlerDoesNotSetLoggingCapability()`
	`49`	`+ public void ServerWithoutCallingLoggingLevelHandlerDoesNotSetLoggingCapability()`
`50`	`50`	`{`
`51`	`51`	`var services = new ServiceCollection();`
`52`	`52`	`services.AddMcpServer()`
`53`	`53`	`.WithStreamServerTransport(Stream.Null, Stream.Null);`
`54`		`- await using var provider = services.BuildServiceProvider();`
	`54`	`+ using var provider = services.BuildServiceProvider();`
`55`	`55`	`var server = provider.GetRequiredService<McpServer>();`
`56`	`56`	`Assert.Null(server.ServerOptions.Capabilities?.Logging);`
`57`	`57`	`}`
Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ public McpServerResourceTests()`
`29`	`29`	`}`
`30`	`30`
`31`	`31`	`[Fact]`
`32`		`- public async Task CanCreateServerWithResource()`
	`32`	`+ public void CanCreateServerWithResource()`
`33`	`33`	`{`
`34`	`34`	`var services = new ServiceCollection();`
`35`	`35`
`@@ -58,14 +58,14 @@ public async Task CanCreateServerWithResource()`
`58`	`58`	`};`
`59`	`59`	`});`
`60`	`60`
`61`		`- await using var provider = services.BuildServiceProvider();`
	`61`	`+ using var provider = services.BuildServiceProvider();`
`62`	`62`
`63`	`63`	`provider.GetRequiredService<McpServer>();`
`64`	`64`	`}`
`65`	`65`
`66`	`66`
`67`	`67`	`[Fact]`
`68`		`- public async Task CanCreateServerWithResourceTemplates()`
	`68`	`+ public void CanCreateServerWithResourceTemplates()`
`69`	`69`	`{`
`70`	`70`	`var services = new ServiceCollection();`
`71`	`71`
`@@ -94,13 +94,13 @@ public async Task CanCreateServerWithResourceTemplates()`
`94`	`94`	`};`
`95`	`95`	`});`
`96`	`96`
`97`		`- await using var provider = services.BuildServiceProvider();`
	`97`	`+ using var provider = services.BuildServiceProvider();`
`98`	`98`
`99`	`99`	`provider.GetRequiredService<McpServer>();`
`100`	`100`	`}`
`101`	`101`
`102`	`102`	`[Fact]`
`103`		`- public async Task CreatingReadHandlerWithNoListHandlerSucceeds()`
	`103`	`+ public void CreatingReadHandlerWithNoListHandlerSucceeds()`
`104`	`104`	`{`
`105`	`105`	`var services = new ServiceCollection();`
`106`	`106`	`services.AddMcpServer()`
`@@ -117,7 +117,7 @@ public async Task CreatingReadHandlerWithNoListHandlerSucceeds()`
`117`	`117`	`}]`
`118`	`118`	`};`
`119`	`119`	`});`
`120`		`- await using var sp = services.BuildServiceProvider();`
	`120`	`+ using var sp = services.BuildServiceProvider();`
`121`	`121`
`122`	`122`	`sp.GetRequiredService<McpServer>();`
`123`	`123`	`}`