Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,39 @@ public async Task CallToolHandler_ThreadSafeLazyLoading()
});
}

[Fact]
public async Task CallToolHandler_PreservesSpecificErrorMessageForMissingParameters()
{
// Arrange
var loader = new NamespaceToolLoader(_commandFactory, _options, _serviceProvider, _logger);
// Use subscription list command which requires --subscription or AZURE_SUBSCRIPTION_ID
var toolName = "subscription";

// Create request without required subscription parameter
var request = CreateCallToolRequest(toolName, new Dictionary<string, object?>
{
["command"] = "list",
["parameters"] = new Dictionary<string, object?>()
});

// Act
var result = await loader.CallToolHandler(request, TestContext.Current.CancellationToken);

// Assert
Assert.NotNull(result);
Assert.True(result.IsError);

var textContent = result.Content[0] as TextContentBlock;
Assert.NotNull(textContent);

// Verify the specific error message is preserved (not replaced with generic message)
Assert.Contains("Missing Required options:", textContent.Text);

// Verify the command spec guidance is still included
Assert.Contains("Review the following command spec", textContent.Text);
Assert.Contains("Command Spec:", textContent.Text);
}

[Fact]
public async Task DisposeAsync_ClearsCaches()
{
Expand Down Expand Up @@ -556,6 +589,35 @@ await Assert.ThrowsAsync<ArgumentNullException>(async () =>
await options.Handlers.ElicitationHandler.Invoke(null!, TestContext.Current.CancellationToken));
}

[Fact]
public async Task CallToolHandler_WithMissingRequiredOptions_PreservesSpecificErrorMessage()
{
// Arrange
// Service Bus commands require --subscription parameter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are there any other tools that have a required --subscription parameter that you should add tests for at the same time?

Copy link
Contributor Author

@g2vinay g2vinay Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually, going to fix it for all the tools.
Servicebus just became the source to trace this bug.

// When calling without it, we should get a specific error message, not a generic one
const string command = "servicebus_namespace_list";
var args = new Dictionary<string, object>(); // Missing required --subscription parameter

var request = CreateCallToolRequest(command, args);

// Act
var response = await _loader.CallToolHandler(request, TestContext.Current.CancellationToken);

// Assert
Assert.NotNull(response);
Assert.True(response.IsError);
Assert.Single(response.Content);

var textContent = response.Content[0] as TextContent;
Assert.NotNull(textContent);

// Verify the specific error message is preserved (not the generic "missing required parameters")
Assert.Contains("Missing Required options: --subscription", textContent.Text);

// Verify it still includes the command spec guidance
Assert.Contains("Review the following command spec", textContent.Text);
}

// Helper methods

private string GetFirstAvailableNamespace()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,4 +136,47 @@ public async Task ListToolsHandler_WithRealRegistryDiscovery_ReturnsExpectedStru
Assert.True(tool.InputSchema.ValueKind != JsonValueKind.Undefined, "InputSchema should be defined");
}
}

[Fact]
public async Task CallToolHandler_PreservesSpecificErrorMessageForMissingParameters()
{
// Arrange - use real RegistryDiscoveryStrategy
var serviceProvider = new ServiceCollection().AddLogging().BuildServiceProvider();
var loggerFactory = serviceProvider.GetRequiredService<ILoggerFactory>();
var serviceStartOptions = Microsoft.Extensions.Options.Options.Create(new ServiceStartOptions());
var toolLoaderOptions = Microsoft.Extensions.Options.Options.Create(new ToolLoaderOptions());
var discoveryLogger = loggerFactory.CreateLogger<RegistryDiscoveryStrategy>();
var discoveryStrategy = RegistryDiscoveryStrategyHelper.CreateStrategy(serviceStartOptions.Value, discoveryLogger);
var logger = loggerFactory.CreateLogger<ServerToolLoader>();

var toolLoader = new ServerToolLoader(discoveryStrategy, toolLoaderOptions, logger);

// Create request for documentation tool with missing required parameters
var request = CreateCallToolRequest("documentation",
new Dictionary<string, JsonElement>
{
{ "command", JsonDocument.Parse("\"microsoft_docs_search\"").RootElement },
// Missing 'question' parameter which is required
{ "parameters", JsonDocument.Parse("{}").RootElement }
});

// Act
var result = await toolLoader.CallToolHandler(request, TestContext.Current.CancellationToken);

// Assert
Assert.NotNull(result);
Assert.NotNull(result.Content);
Assert.NotEmpty(result.Content);

var textContent = result.Content[0] as TextContentBlock;
Assert.NotNull(textContent);

// Verify the specific error message is preserved (not replaced with generic message)
// Should contain "Missing required options:" or similar specific error, not generic "missing required parameters"
Assert.Contains("Missing", textContent.Text);

// Verify the command spec guidance is still included
Assert.Contains("Review the following command spec", textContent.Text);
Assert.Contains("Command Spec:", textContent.Text);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -380,13 +380,19 @@ private async Task<CallToolResult> InvokeChildToolAsync(
var childToolSpecJson = GetChildToolJson(request, namespaceName, command);

_logger.LogWarning("Namespace {Namespace} command {Command} requires additional parameters.", namespaceName, command);

// Extract the specific error message from the response
var errorMessage = string.IsNullOrEmpty(commandResponse.Message)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs a CHANGELOG entry

? $"The '{command}' command is missing required parameters."
: commandResponse.Message;

var finalResponse = new CallToolResult
{
Content =
[
new TextContentBlock {
Text = $"""
The '{command}' command is missing required parameters.
{errorMessage}

- Review the following command spec and identify the required arguments from the input schema.
- Omit any arguments that are not required or do not apply to your use case.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ private async Task<CallToolResult> InvokeChildToolAsync(RequestContext<CallToolR
[
new TextContentBlock {
Text = $"""
The '{command}' command is missing required parameters.
{textContent.Text}

- Review the following command spec and identify the required arguments from the input schema.
- Omit any arguments that are not required or do not apply to your use case.
Expand Down
Loading