Open
Conversation
Azure SDKs do not throw UnauthorizedAccessException for authorization failures. They throw Azure.RequestFailedException with Status == 403. This aligns Service Bus error handling with the standard Azure SDK exception pattern used across all other toolsets and provides more specific error messages mentioning RBAC permissions. Changes: - Replace UnauthorizedAccessException with Azure.RequestFailedException (Status 403) - Update error messages to mention RBAC permissions - Update test method names from HandlesUnauthorizedAccess to HandlesAuthorizationFailure - Update test assertions to verify new error messages
Remove Azure.RequestFailedException handling since it's not thrown by Azure.Identity. Azure.Identity only throws AuthenticationFailedException for token acquisition failures. Service-level authorization errors (403 Forbidden) would come from ServiceBusException, not RequestFailedException. This simplifies error handling to focus only on Identity-specific exceptions: - Azure.Identity.AuthenticationFailedException → 401 Unauthorized - ServiceBusException (entity not found) → 404 NotFound - All other exceptions → handled by base class Changes: - Remove Azure.RequestFailedException handling from all commands - Remove HandlesAuthorizationFailure tests (no longer applicable) - Keep only AuthenticationFailedException tests (3 tests, all passing)
Azure.Identity SDK throws two exception types: 1. AuthenticationFailedException - credentials exist but authentication fails 2. CredentialUnavailableException - no credentials available Previously, only AuthenticationFailedException was handled. This adds CredentialUnavailableException handling to provide clearer error messages when no credential sources are available (e.g., Azure CLI not logged in, VS Code not signed in). Benefits: - Better error messages distinguishing 'no credentials' from 'authentication failed' - More actionable guidance for users (lists all authentication methods) - Consistent with internal credential chaining in CustomChainedCredential.cs Status code: Both map to 401 Unauthorized Applies to: All commands inheriting from GlobalCommand (automatically benefits all Azure toolsets)
Service Bus commands were duplicating AuthenticationFailedException and CredentialUnavailableException handling that already exists in GlobalCommand base class.
Since Service Bus commands inherit from SubscriptionCommand -> GlobalCommand, they automatically get Identity exception handling through the fallback case: _ => base.GetErrorMessage(ex).
Changes:
- Remove AuthenticationFailedException handling from all 5 Service Bus commands
- Let exceptions fall through to GlobalCommand for consistent error messages
- Update tests to verify GlobalCommand message format ('az login' guidance)
- Service Bus commands now only handle ServiceBusException (entity not found)
Benefits:
- Eliminates code duplication
- Consistent error messages across all Azure toolsets
- Automatic CredentialUnavailableException support
- Single source of truth for Identity errors in GlobalCommand
All tests passing (3/3)
Previously, when commands reported 'Missing Required options: --subscription', the ServerToolLoader and NamespaceToolLoader would replace the specific error with a generic 'The command is missing required parameters' message, making it unclear which parameter was actually missing. This was particularly problematic for subscription parameter errors because: - Subscription is often auto-detected from AZURE_SUBSCRIPTION_ID or Azure CLI - When auto-detection fails (e.g., resource in different tenant), validation fails - Users see 'missing required parameters' but don't know it's --subscription - The MCP schema shows subscription as optional, adding to confusion Changes: - ServerToolLoader: Use actual error text instead of generic message - NamespaceToolLoader: Extract commandResponse.Message for specific details - Both still provide helpful guidance and command spec - Original detailed error now visible to users Example improvement: Before: 'The servicebus_topic_details command is missing required parameters.' After: 'Missing Required options: --subscription' + helpful guidance Fixes the root cause reported in the original bug where users couldn't tell that subscription parameter was needed when resources were in different tenants.
Contributor
There was a problem hiding this comment.
Pull request overview
Improves error propagation to the LLM when tool invocations fail due to missing required parameters, and adds Service Bus command unit coverage for authentication failures.
Changes:
- Update tool-loader “missing required options” handling to surface the underlying error text (instead of a generic message).
- Add Service Bus unit tests to validate
AuthenticationFailedExceptionmaps to401 Unauthorizedwith the expected guidance message.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tools/Azure.Mcp.Tools.ServiceBus/tests/Azure.Mcp.Tools.ServiceBus.UnitTests/Topic/TopicDetailsCommandTests.cs | Adds auth-failure unit test for topic details command response/status/message. |
| tools/Azure.Mcp.Tools.ServiceBus/tests/Azure.Mcp.Tools.ServiceBus.UnitTests/Topic/SubscriptionDetailsCommandTests.cs | Adds auth-failure unit test for subscription details command response/status/message. |
| tools/Azure.Mcp.Tools.ServiceBus/tests/Azure.Mcp.Tools.ServiceBus.UnitTests/Queue/QueueDetailsCommandTests.cs | Adds auth-failure unit test for queue details command response/status/message. |
| core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/ServerToolLoader.cs | Uses the child tool’s missing-options text in the guidance response. |
| core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/NamespaceToolLoader.cs | Uses commandResponse.Message (with fallback) in the guidance response. |
Comments suppressed due to low confidence (3)
core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/ServerToolLoader.cs:296
- This change is intended to propagate the specific “Missing required options” message to the LLM, but there doesn’t appear to be unit coverage that asserts the final
CallToolResultcontains that propagated message (and only once). Please add/extend tool-loader tests (e.g., incore/Azure.Mcp.Core/.../ServerToolLoaderTests.cs) to simulate a child tool returning aTextContentBlockwith “Missing required options …” and verify the constructed response includes that exact text and the tool spec guidance.
if (textContent.Text.Contains("Missing required options", StringComparison.OrdinalIgnoreCase))
{
var childToolSpecJson = await GetChildToolJsonAsync(request, tool, command, cancellationToken);
_logger.LogWarning("Tool {Tool} command {Command} requires additional parameters.", tool, command);
var finalResponse = new CallToolResult
{
Content =
[
new TextContentBlock {
Text = $"""
{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.
- Wrap all command arguments into the root "parameters" argument.
- If required data is missing infer the data from your context or prompt the user as needed.
- Run the tool again with the "command" and root "parameters" object.
Command Spec:
{childToolSpecJson}
"""
core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/NamespaceToolLoader.cs:406
- The new
errorMessagepropagation path should be covered by a unit test to ensurecommandResponse.Message(the specific missing-parameter details) is surfaced in the first text block returned to the LLM. Consider adding aNamespaceToolLoaderTestscase that executes a command with missing required options and asserts the resultingCallToolResult.Content[0]starts with the originalcommandResponse.Message(not the generic fallback).
_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)
? $"The '{command}' command is missing required parameters."
: commandResponse.Message;
var finalResponse = new CallToolResult
{
Content =
[
new TextContentBlock {
Text = $"""
{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.
- Wrap all command arguments into the root "parameters" argument.
- If required data is missing infer the data from your context or prompt the user as needed.
- Run the tool again with the "command" and root "parameters" object.
Command Spec:
{childToolSpecJson}
"""
}
core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/ServerToolLoader.cs:304
- The new prompt prepends
textContent.Text, but the code then appends alltoolCallResponse.Contentblocks (including the sameTextContentBlock) tofinalResponse.Content. This will duplicate the “Missing required options …” text in the final response content. Consider either not re-adding the original text block, or keep the append but change the prepended text to a short summary (or dedupe by reference/type).
var finalResponse = new CallToolResult
{
Content =
[
new TextContentBlock {
Text = $"""
{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.
- Wrap all command arguments into the root "parameters" argument.
- If required data is missing infer the data from your context or prompt the user as needed.
- Run the tool again with the "command" and root "parameters" object.
Command Spec:
{childToolSpecJson}
"""
}
]
};
foreach (var contentBlock in toolCallResponse.Content)
{
finalResponse.Content.Add(contentBlock);
}
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes the issue where missing parameter wasn't being propagated in the response to the LLM.