-
Notifications
You must be signed in to change notification settings - Fork 15
Make MCP client initialization timeout configurable via ToolOptions #212
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
Merged
Grant Harris (gwharris7)
merged 7 commits into
microsoft:main
from
matiazo:feature/configurable-mcp-timeout
Jun 8, 2026
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
5462fe3
Make MCP client initialization timeout configurable via ToolOptions
64bb70c
Address PR review feedback: validation, null behavior, tests
21f76df
Merge remote-tracking branch 'origin/main' into feature/configurable-…
35ab17d
Merge branch 'main' into feature/configurable-mcp-timeout
gwharris7 1a16a76
Address PR 212 review comments: refactor timeout helper and harden tests
gwharris7 86b8242
Potential fix for pull request finding
gwharris7 546e8c6
chore: retrigger CodeQL checks
gwharris7 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
181 changes: 181 additions & 0 deletions
181
...Tests/Microsoft.Agents.A365.Tooling.Tests/Services/McpClientInitializationTimeoutTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,181 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| using FluentAssertions; | ||
| using Microsoft.Agents.A365.Tooling.Models; | ||
| using Microsoft.Agents.A365.Tooling.Services; | ||
| using Microsoft.Agents.Builder; | ||
| using Microsoft.Extensions.Configuration; | ||
| using Microsoft.Extensions.Logging; | ||
| using Moq; | ||
| using Xunit; | ||
|
|
||
| namespace Microsoft.Agents.A365.Tooling.Tests.Services | ||
| { | ||
| /// <summary> | ||
| /// Unit tests for McpClientInitializationTimeoutSeconds in ToolOptions | ||
| /// and its application in McpToolServerConfigurationService. | ||
| /// </summary> | ||
| public class McpClientInitializationTimeoutTests | ||
| { | ||
| private readonly Mock<ILogger<IMcpToolServerConfigurationService>> _loggerMock; | ||
| private readonly Mock<IServiceProvider> _serviceProviderMock; | ||
| private readonly Mock<IHttpClientFactory> _httpClientFactoryMock; | ||
|
|
||
| public McpClientInitializationTimeoutTests() | ||
| { | ||
| _loggerMock = new Mock<ILogger<IMcpToolServerConfigurationService>>(); | ||
| _serviceProviderMock = new Mock<IServiceProvider>(); | ||
| _httpClientFactoryMock = new Mock<IHttpClientFactory>(); | ||
|
|
||
| _httpClientFactoryMock.Setup(f => f.CreateClient(It.IsAny<string>())) | ||
| .Returns(new HttpClient()); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void ToolOptions_McpClientInitializationTimeoutSeconds_DefaultsToNull() | ||
| { | ||
| // Arrange & Act | ||
| var options = new ToolOptions(); | ||
|
|
||
| // Assert | ||
| options.McpClientInitializationTimeoutSeconds.Should().BeNull(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void ToolOptions_McpClientInitializationTimeoutSeconds_CanBeSetToValidValue() | ||
| { | ||
| // Arrange & Act | ||
| var options = new ToolOptions | ||
| { | ||
| McpClientInitializationTimeoutSeconds = 180 | ||
| }; | ||
|
|
||
| // Assert | ||
| options.McpClientInitializationTimeoutSeconds.Should().Be(180); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void ToolOptions_McpClientInitializationTimeoutSeconds_CanBeSetToNull() | ||
| { | ||
| // Arrange | ||
| var options = new ToolOptions | ||
| { | ||
| McpClientInitializationTimeoutSeconds = 120 | ||
| }; | ||
|
|
||
| // Act | ||
| options.McpClientInitializationTimeoutSeconds = null; | ||
|
|
||
| // Assert | ||
| options.McpClientInitializationTimeoutSeconds.Should().BeNull(); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(0)] | ||
| [InlineData(-1)] | ||
| [InlineData(-100)] | ||
| [InlineData(601)] | ||
| [InlineData(int.MaxValue)] | ||
| public async Task GetMcpClientToolsAsync_InvalidTimeoutValue_ThrowsArgumentOutOfRangeException(int invalidTimeout) | ||
| { | ||
| // Arrange | ||
| var configMock = new Mock<IConfiguration>(); | ||
| configMock.Setup(c => c["ASPNETCORE_ENVIRONMENT"]).Returns("Development"); | ||
|
|
||
| var service = new McpToolServerConfigurationService( | ||
| _loggerMock.Object, | ||
| configMock.Object, | ||
| _serviceProviderMock.Object, | ||
| _httpClientFactoryMock.Object); | ||
|
|
||
| var serverConfig = new MCPServerConfig | ||
| { | ||
| mcpServerName = "test_server", | ||
| url = "https://localhost:52856/agents/servers/test_server", | ||
| id = "test-id", | ||
| scope = "test-scope", | ||
| audience = "test-audience", | ||
| publisher = "test-publisher", | ||
| }; | ||
|
|
||
| var toolOptions = new ToolOptions | ||
| { | ||
| McpClientInitializationTimeoutSeconds = invalidTimeout | ||
| }; | ||
|
|
||
| var turnContextMock = new Mock<ITurnContext>(); | ||
|
|
||
| // Act | ||
| Func<Task> act = async () => await service.GetMcpClientToolsAsync( | ||
| turnContextMock.Object, serverConfig, "test-token", toolOptions); | ||
|
|
||
| // Assert - invalid timeout should surface an ArgumentOutOfRangeException either directly | ||
| // or wrapped by a higher-level InvalidOperationException depending on the call path. | ||
| var exception = await Record.ExceptionAsync(act); | ||
| exception.Should().NotBeNull(); | ||
| (exception is ArgumentOutOfRangeException | ||
| || exception is InvalidOperationException { InnerException: ArgumentOutOfRangeException }) | ||
| .Should().BeTrue("an invalid timeout should result in an ArgumentOutOfRangeException, either directly or wrapped"); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void GetValidatedInitializationTimeout_NullInput_ReturnsNull() | ||
| { | ||
| // Act | ||
| var result = McpToolServerConfigurationService.GetValidatedInitializationTimeout(null); | ||
|
|
||
| // Assert | ||
| result.Should().BeNull(); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(1)] | ||
| [InlineData(60)] | ||
| [InlineData(120)] | ||
| [InlineData(300)] | ||
| [InlineData(600)] | ||
| public void GetValidatedInitializationTimeout_ValidInput_ReturnsMatchingTimeSpan(int seconds) | ||
| { | ||
| // Act | ||
| var result = McpToolServerConfigurationService.GetValidatedInitializationTimeout(seconds); | ||
|
|
||
| // Assert | ||
| result.Should().Be(TimeSpan.FromSeconds(seconds)); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(0)] | ||
| [InlineData(-1)] | ||
| [InlineData(-100)] | ||
| [InlineData(601)] | ||
| [InlineData(int.MaxValue)] | ||
| public void GetValidatedInitializationTimeout_InvalidInput_ThrowsArgumentOutOfRangeException(int invalidSeconds) | ||
| { | ||
| // Act | ||
| Action act = () => McpToolServerConfigurationService.GetValidatedInitializationTimeout(invalidSeconds); | ||
|
|
||
| // Assert | ||
| act.Should().Throw<ArgumentOutOfRangeException>() | ||
| .Which.ParamName.Should().Be(nameof(ToolOptions.McpClientInitializationTimeoutSeconds)); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(1)] | ||
| [InlineData(60)] | ||
| [InlineData(120)] | ||
| [InlineData(300)] | ||
| [InlineData(600)] | ||
| public void ToolOptions_McpClientInitializationTimeoutSeconds_AcceptsValidValues(int validTimeout) | ||
| { | ||
| // Arrange & Act | ||
| var options = new ToolOptions | ||
| { | ||
| McpClientInitializationTimeoutSeconds = validTimeout | ||
| }; | ||
|
|
||
| // Assert | ||
| options.McpClientInitializationTimeoutSeconds.Should().Be(validTimeout); | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.