-
Notifications
You must be signed in to change notification settings - Fork 299
Implementation of Custom Tool in MCP #3048
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
base: main
Are you sure you want to change the base?
Conversation
- Add EntityMcpOptions class with custom-tool and dml-tools properties - Add JSON converter supporting boolean and object formats - Add CLI support for --mcp.dml-tools and --mcp.custom-tool flags - Add schema validation restricting custom-tool to stored procedures - Entity.Mcp property is optional (default null) to avoid test cascade Only 9 files changed in this minimal implementation.
- Update EntityMcpOptions documentation to clarify custom-tool behavior in boolean mode - Replace if-else with switch-case in converter for better extensibility - Remove unnecessary null writes in serializer - Change CustomToolEnabled and DmlToolEnabled from nullable to non-nullable bool - Fix boolean shorthand deserialization to not mark custom-tool as user-provided - Add consistent else block in constructor for symmetry All 530 tests passing. Functionality verified with manual testing.
…/Azure/data-api-builder into Usr/sogh/entity-level-mcp-config
…/Azure/data-api-builder into Usr/sogh/entity-level-mcp-config
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces dynamic custom MCP tool support, allowing stored procedures configured with "custom-tool" enabled to be automatically exposed as dedicated MCP tools. The implementation adds a factory pattern for generating tools from configuration and integrates them into the service registration pipeline.
Changes:
- Added
DynamicCustomToolclass to dynamically generate MCP tools from stored procedure configurations - Implemented
CustomMcpToolFactoryto scan runtime configuration and create custom tools - Updated service registration in
McpServiceCollectionExtensionsto exclude dynamic tools from auto-discovery and register custom tools from configuration
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 14 comments.
| File | Description |
|---|---|
| src/Azure.DataApiBuilder.Mcp/Core/McpServiceCollectionExtensions.cs | Added RegisterCustomTools method and modified auto-discovery to exclude DynamicCustomTool |
| src/Azure.DataApiBuilder.Mcp/Core/DynamicCustomTool.cs | New class implementing IMcpTool for dynamically generated stored procedure tools |
| src/Azure.DataApiBuilder.Mcp/Core/CustomMcpToolFactory.cs | New factory class for creating custom tools from runtime configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| using System.Data.Common; | ||
| using System.Text.Json; | ||
| using System.Text.RegularExpressions; | ||
| using Azure.DataApiBuilder.Auth; | ||
| using Azure.DataApiBuilder.Config.DatabasePrimitives; | ||
| using Azure.DataApiBuilder.Config.ObjectModel; | ||
| using Azure.DataApiBuilder.Core.Configurations; | ||
| using Azure.DataApiBuilder.Core.Models; | ||
| using Azure.DataApiBuilder.Core.Resolvers; | ||
| using Azure.DataApiBuilder.Core.Resolvers.Factories; | ||
| using Azure.DataApiBuilder.Core.Services; | ||
| using Azure.DataApiBuilder.Mcp.Model; | ||
| using Azure.DataApiBuilder.Mcp.Utils; | ||
| using Azure.DataApiBuilder.Service.Exceptions; | ||
| using Microsoft.AspNetCore.Http; | ||
| using Microsoft.AspNetCore.Mvc; | ||
| using Microsoft.Data.SqlClient; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
| using Microsoft.Extensions.Logging; | ||
| using ModelContextProtocol.Protocol; | ||
| using static Azure.DataApiBuilder.Mcp.Model.McpEnums; | ||
|
|
||
| namespace Azure.DataApiBuilder.Mcp.Core | ||
| { | ||
| /// <summary> | ||
| /// Dynamic custom MCP tool generated from stored procedure entity configuration. | ||
| /// Each custom tool represents a single stored procedure exposed as a dedicated MCP tool. | ||
| /// </summary> | ||
| public class DynamicCustomTool : IMcpTool | ||
| { | ||
| private readonly string _entityName; | ||
| private readonly Entity _entity; | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of DynamicCustomTool. | ||
| /// </summary> | ||
| /// <param name="entityName">The entity name from configuration.</param> | ||
| /// <param name="entity">The entity configuration object.</param> | ||
| public DynamicCustomTool(string entityName, Entity entity) | ||
| { | ||
| _entityName = entityName ?? throw new ArgumentNullException(nameof(entityName)); | ||
| _entity = entity ?? throw new ArgumentNullException(nameof(entity)); | ||
|
|
||
| // Validate that this is a stored procedure | ||
| if (_entity.Source.Type != EntitySourceType.StoredProcedure) | ||
| { | ||
| throw new ArgumentException( | ||
| $"Custom tools can only be created for stored procedures. Entity '{entityName}' is of type '{_entity.Source.Type}'.", | ||
| nameof(entity)); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets the type of the tool, which is Custom for dynamically generated tools. | ||
| /// </summary> | ||
| public ToolType ToolType { get; } = ToolType.Custom; | ||
|
|
||
| /// <summary> | ||
| /// Gets the metadata for this custom tool, including name, description, and input schema. | ||
| /// </summary> | ||
| public Tool GetToolMetadata() | ||
| { | ||
| string toolName = ConvertToToolName(_entityName); | ||
| string description = _entity.Description ?? $"Execute {_entityName} stored procedure"; | ||
|
|
||
| // Build input schema based on parameters | ||
| JsonElement inputSchema = BuildInputSchema(); | ||
|
|
||
| return new Tool | ||
| { | ||
| Name = toolName, | ||
| Description = description, | ||
| InputSchema = inputSchema | ||
| }; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Executes the stored procedure represented by this custom tool. | ||
| /// </summary> | ||
| public async Task<CallToolResult> ExecuteAsync( | ||
| JsonDocument? arguments, | ||
| IServiceProvider serviceProvider, | ||
| CancellationToken cancellationToken = default) | ||
| { | ||
| ILogger<DynamicCustomTool>? logger = serviceProvider.GetService<ILogger<DynamicCustomTool>>(); | ||
| string toolName = GetToolMetadata().Name; | ||
|
|
||
| try | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
|
|
||
| // 1) Resolve required services & configuration | ||
| RuntimeConfigProvider runtimeConfigProvider = serviceProvider.GetRequiredService<RuntimeConfigProvider>(); | ||
| RuntimeConfig config = runtimeConfigProvider.GetConfig(); | ||
|
|
||
| // 2) Parse arguments from the request | ||
| Dictionary<string, object?> parameters = new(); | ||
| if (arguments != null) | ||
| { | ||
| foreach (JsonProperty property in arguments.RootElement.EnumerateObject()) | ||
| { | ||
| parameters[property.Name] = GetParameterValue(property.Value); | ||
| } | ||
| } | ||
|
|
||
| // 3) Validate entity still exists in configuration | ||
| if (!config.Entities.TryGetValue(_entityName, out Entity? entityConfig)) | ||
| { | ||
| return McpResponseBuilder.BuildErrorResult(toolName, "EntityNotFound", $"Entity '{_entityName}' not found in configuration.", logger); | ||
| } | ||
|
|
||
| if (entityConfig.Source.Type != EntitySourceType.StoredProcedure) | ||
| { | ||
| return McpResponseBuilder.BuildErrorResult(toolName, "InvalidEntity", $"Entity {_entityName} is not a stored procedure.", logger); | ||
| } | ||
|
|
||
| // 4) Resolve metadata | ||
| if (!McpMetadataHelper.TryResolveMetadata( | ||
| _entityName, | ||
| config, | ||
| serviceProvider, | ||
| out ISqlMetadataProvider sqlMetadataProvider, | ||
| out DatabaseObject dbObject, | ||
| out string dataSourceName, | ||
| out string metadataError)) | ||
| { | ||
| return McpResponseBuilder.BuildErrorResult(toolName, "EntityNotFound", metadataError, logger); | ||
| } | ||
|
|
||
| // 5) Authorization check | ||
| IAuthorizationResolver authResolver = serviceProvider.GetRequiredService<IAuthorizationResolver>(); | ||
| IHttpContextAccessor httpContextAccessor = serviceProvider.GetRequiredService<IHttpContextAccessor>(); | ||
| HttpContext? httpContext = httpContextAccessor.HttpContext; | ||
|
|
||
| if (!McpAuthorizationHelper.ValidateRoleContext(httpContext, authResolver, out string roleError)) | ||
| { | ||
| return McpErrorHelpers.PermissionDenied(toolName, _entityName, "execute", roleError, logger); | ||
| } | ||
|
|
||
| if (!McpAuthorizationHelper.TryResolveAuthorizedRole( | ||
| httpContext!, | ||
| authResolver, | ||
| _entityName, | ||
| EntityActionOperation.Execute, | ||
| out string? effectiveRole, | ||
| out string authError)) | ||
| { | ||
| return McpErrorHelpers.PermissionDenied(toolName, _entityName, "execute", authError, logger); | ||
| } | ||
|
|
||
| // 6) Build request payload | ||
| JsonElement? requestPayloadRoot = null; | ||
| if (parameters.Count > 0) | ||
| { | ||
| string jsonPayload = JsonSerializer.Serialize(parameters); | ||
| using JsonDocument doc = JsonDocument.Parse(jsonPayload); | ||
| requestPayloadRoot = doc.RootElement.Clone(); | ||
| } | ||
|
|
||
| // 7) Build stored procedure execution context | ||
| StoredProcedureRequestContext context = new( | ||
| entityName: _entityName, | ||
| dbo: dbObject, | ||
| requestPayloadRoot: requestPayloadRoot, | ||
| operationType: EntityActionOperation.Execute); | ||
|
|
||
| // Add user-provided parameters | ||
| if (requestPayloadRoot != null) | ||
| { | ||
| foreach (JsonProperty property in requestPayloadRoot.Value.EnumerateObject()) | ||
| { | ||
| context.FieldValuePairsInBody[property.Name] = GetParameterValue(property.Value); | ||
| } | ||
| } | ||
|
|
||
| // Add default parameters from configuration if not provided | ||
| if (entityConfig.Source.Parameters != null) | ||
| { | ||
| foreach (ParameterMetadata param in entityConfig.Source.Parameters) | ||
| { | ||
| if (!context.FieldValuePairsInBody.ContainsKey(param.Name)) | ||
| { | ||
| context.FieldValuePairsInBody[param.Name] = param.Default; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Populate resolved parameters | ||
| context.PopulateResolvedParameters(); | ||
|
|
||
| // 8) Execute stored procedure | ||
| DatabaseType dbType = config.GetDataSourceFromDataSourceName(dataSourceName).DatabaseType; | ||
| IQueryEngineFactory queryEngineFactory = serviceProvider.GetRequiredService<IQueryEngineFactory>(); | ||
| IQueryEngine queryEngine = queryEngineFactory.GetQueryEngine(dbType); | ||
|
|
||
| IActionResult? queryResult = null; | ||
|
|
||
| try | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| queryResult = await queryEngine.ExecuteAsync(context, dataSourceName).ConfigureAwait(false); | ||
| } | ||
| catch (DataApiBuilderException dabEx) | ||
| { | ||
| logger?.LogError(dabEx, "Error executing custom tool {ToolName} for entity {Entity}", toolName, _entityName); | ||
| return McpResponseBuilder.BuildErrorResult(toolName, "ExecutionError", dabEx.Message, logger); | ||
| } | ||
| catch (SqlException sqlEx) | ||
| { | ||
| logger?.LogError(sqlEx, "SQL error executing custom tool {ToolName}", toolName); | ||
| return McpResponseBuilder.BuildErrorResult(toolName, "DatabaseError", $"Database error: {sqlEx.Message}", logger); | ||
| } | ||
| catch (DbException dbEx) | ||
| { | ||
| logger?.LogError(dbEx, "Database error executing custom tool {ToolName}", toolName); | ||
| return McpResponseBuilder.BuildErrorResult(toolName, "DatabaseError", $"Database error: {dbEx.Message}", logger); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| logger?.LogError(ex, "Unexpected error executing custom tool {ToolName}", toolName); | ||
| return McpResponseBuilder.BuildErrorResult(toolName, "UnexpectedError", "An error occurred during execution.", logger); | ||
| } | ||
|
|
||
| // 9) Build success response | ||
| return BuildExecuteSuccessResponse(toolName, _entityName, parameters, queryResult, logger); | ||
| } | ||
| catch (OperationCanceledException) | ||
| { | ||
| return McpResponseBuilder.BuildErrorResult(toolName, "OperationCanceled", "The operation was canceled.", logger); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| logger?.LogError(ex, "Unexpected error in DynamicCustomTool for {EntityName}", _entityName); | ||
| return McpResponseBuilder.BuildErrorResult(toolName, "UnexpectedError", "An unexpected error occurred.", logger); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Converts entity name to tool name format (lowercase with underscores). | ||
| /// </summary> | ||
| private static string ConvertToToolName(string entityName) | ||
| { | ||
| // Convert PascalCase to snake_case | ||
| string result = Regex.Replace(entityName, "([a-z0-9])([A-Z])", "$1_$2"); | ||
| return result.ToLowerInvariant(); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Builds the input schema for the tool based on entity parameters. | ||
| /// </summary> | ||
| private JsonElement BuildInputSchema() | ||
| { | ||
| var schema = new Dictionary<string, object> | ||
| { | ||
| ["type"] = "object", | ||
| ["properties"] = new Dictionary<string, object>() | ||
| }; | ||
|
|
||
| if (_entity.Source.Parameters != null && _entity.Source.Parameters.Any()) | ||
| { | ||
| var properties = (Dictionary<string, object>)schema["properties"]; | ||
|
|
||
| foreach (var param in _entity.Source.Parameters) | ||
| { | ||
| properties[param.Name] = new Dictionary<string, object> | ||
| { | ||
| ["type"] = "string", | ||
| ["description"] = param.Description ?? $"Parameter {param.Name}" | ||
| }; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| schema["properties"] = new Dictionary<string, object>(); | ||
| } | ||
|
|
||
| return JsonSerializer.SerializeToElement(schema); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Converts a JSON element to its appropriate CLR type. | ||
| /// </summary> | ||
| private static object? GetParameterValue(JsonElement element) | ||
| { | ||
| return element.ValueKind switch | ||
| { | ||
| JsonValueKind.String => element.GetString(), | ||
| JsonValueKind.Number => | ||
| element.TryGetInt64(out long longValue) ? longValue : | ||
| element.TryGetDecimal(out decimal decimalValue) ? decimalValue : | ||
| element.GetDouble(), | ||
| JsonValueKind.True => true, | ||
| JsonValueKind.False => false, | ||
| JsonValueKind.Null => null, | ||
| _ => element.ToString() | ||
| }; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Builds a successful response for the execute operation. | ||
| /// </summary> | ||
| private static CallToolResult BuildExecuteSuccessResponse( | ||
| string toolName, | ||
| string entityName, | ||
| Dictionary<string, object?>? parameters, | ||
| IActionResult? queryResult, | ||
| ILogger? logger) | ||
| { | ||
| Dictionary<string, object?> responseData = new() | ||
| { | ||
| ["entity"] = entityName, | ||
| ["message"] = "Execution successful" | ||
| }; | ||
|
|
||
| if (parameters?.Count > 0) | ||
| { | ||
| responseData["parameters"] = parameters; | ||
| } | ||
|
|
||
| // Handle different result types | ||
| if (queryResult is OkObjectResult okResult && okResult.Value != null) | ||
| { | ||
| if (okResult.Value is JsonDocument jsonDoc) | ||
| { | ||
| JsonElement root = jsonDoc.RootElement; | ||
| responseData["value"] = root.ValueKind == JsonValueKind.Array ? root : JsonSerializer.SerializeToElement(new[] { root }); | ||
| } | ||
| else if (okResult.Value is JsonElement jsonElement) | ||
| { | ||
| responseData["value"] = jsonElement.ValueKind == JsonValueKind.Array ? jsonElement : JsonSerializer.SerializeToElement(new[] { jsonElement }); | ||
| } | ||
| else | ||
| { | ||
| JsonElement serialized = JsonSerializer.SerializeToElement(okResult.Value); | ||
| responseData["value"] = serialized; | ||
| } | ||
| } | ||
| else if (queryResult is BadRequestObjectResult badRequest) | ||
| { | ||
| return McpResponseBuilder.BuildErrorResult( | ||
| toolName, | ||
| "BadRequest", | ||
| badRequest.Value?.ToString() ?? "Bad request", | ||
| logger); | ||
| } | ||
| else if (queryResult is UnauthorizedObjectResult) | ||
| { | ||
| return McpErrorHelpers.PermissionDenied(toolName, entityName, "execute", "Unauthorized", logger); | ||
| } | ||
| else | ||
| { | ||
| responseData["value"] = JsonSerializer.SerializeToElement(Array.Empty<object>()); | ||
| } | ||
|
|
||
| return McpResponseBuilder.BuildSuccessResult( | ||
| responseData, | ||
| logger, | ||
| $"Custom tool {toolName} executed successfully for entity {entityName}." | ||
| ); | ||
| } | ||
| } | ||
| } |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description indicates that both Integration Tests and Unit Tests are unchecked. Given the complexity of the new DynamicCustomTool class, which includes authorization, parameter parsing, stored procedure execution, and error handling, comprehensive test coverage is critical. Tests should cover: tool creation from configuration, parameter schema generation, execution with various parameter types, authorization checks, error scenarios, and tool name conversion edge cases.
| private static void RegisterCustomTools(IServiceCollection services, RuntimeConfig config) | ||
| { | ||
| // Create custom tools and register each as a singleton | ||
| foreach (IMcpTool customTool in CustomMcpToolFactory.CreateCustomTools(config)) | ||
| { | ||
| services.AddSingleton<IMcpTool>(customTool); | ||
| } |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When multiple custom tools have the same converted name, the McpToolRegistry will silently overwrite previously registered tools (the dictionary assignment at line 22 in McpToolRegistry). This makes the tool name collision issue more severe, as it could result in tools being silently lost without any warning. This issue should be addressed in conjunction with adding collision detection in the RegisterCustomTools method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is already captured as a subsequent work item- #3049
| public Tool GetToolMetadata() | ||
| { | ||
| string toolName = ConvertToToolName(_entityName); | ||
| string description = _entity.Description ?? $"Execute {_entityName} stored procedure"; |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the entity description is null, a default description is provided that includes the entity name. However, entity names might not be user-friendly (e.g., "sp_GetUserData" becomes "Execute sp_GetUserData stored procedure"). Consider using the converted tool name in the default description instead for consistency, or provide guidance in documentation that descriptions should be set for better user experience.
| string description = _entity.Description ?? $"Execute {_entityName} stored procedure"; | |
| string description = _entity.Description ?? $"Execute {toolName} stored procedure"; |
| { | ||
| properties[param.Name] = new Dictionary<string, object> | ||
| { | ||
| ["type"] = "string", |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The input schema sets all parameters with type "string", but the GetParameterValue method in the same class can return various types (long, decimal, double, bool, null, string). This inconsistency between the declared schema and the actual parameter parsing could lead to type mismatch issues when clients use the schema. Consider using a more accurate type mapping based on the actual parameter types from the stored procedure metadata, or document that all parameters should be passed as strings if that's the intended behavior.
| ["type"] = "string", | |
| // Allow all JSON primitive types that GetParameterValue can produce. | |
| ["type"] = new[] { "string", "number", "boolean", "null" }, |
| customToolCount++; | ||
|
|
||
| logger?.LogInformation( | ||
| "Created custom MCP tool '{ToolName}' for stored procedure entity '{EntityName}'", | ||
| tool.GetToolMetadata().Name, | ||
| entityName); | ||
|
|
||
| customTools.Add(tool); |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable customToolCount is incremented before adding the tool to the list, but if the subsequent customTools.Add(tool) were to throw an exception, the count would be incorrect. While this is unlikely, consider incrementing the counter after the add operation, or simply use customTools.Count in the final log message for accuracy.
| return element.ValueKind switch | ||
| { | ||
| JsonValueKind.String => element.GetString(), | ||
| JsonValueKind.Number => | ||
| element.TryGetInt64(out long longValue) ? longValue : | ||
| element.TryGetDecimal(out decimal decimalValue) ? decimalValue : | ||
| element.GetDouble(), | ||
| JsonValueKind.True => true, | ||
| JsonValueKind.False => false, | ||
| JsonValueKind.Null => null, | ||
| _ => element.ToString() | ||
| }; |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GetParameterValue method duplicates logic from McpArgumentParser.GetExecuteParameterValue. This code duplication violates the DRY principle and makes maintenance harder. Consider extracting this to a shared utility method that both classes can use.
| return element.ValueKind switch | |
| { | |
| JsonValueKind.String => element.GetString(), | |
| JsonValueKind.Number => | |
| element.TryGetInt64(out long longValue) ? longValue : | |
| element.TryGetDecimal(out decimal decimalValue) ? decimalValue : | |
| element.GetDouble(), | |
| JsonValueKind.True => true, | |
| JsonValueKind.False => false, | |
| JsonValueKind.Null => null, | |
| _ => element.ToString() | |
| }; | |
| return McpArgumentParser.GetExecuteParameterValue(element); |
| if (requestPayloadRoot != null) | ||
| { | ||
| foreach (JsonProperty property in requestPayloadRoot.Value.EnumerateObject()) | ||
| { | ||
| context.FieldValuePairsInBody[property.Name] = GetParameterValue(property.Value); |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code processes parameters twice in the ExecuteAsync method: once at lines 103-107 to build the parameters dictionary, and again at lines 171-177 when adding them to the StoredProcedureRequestContext. This is inefficient. Consider building the parameters dictionary directly into the context's FieldValuePairsInBody dictionary to avoid the redundant iteration.
| if (requestPayloadRoot != null) | |
| { | |
| foreach (JsonProperty property in requestPayloadRoot.Value.EnumerateObject()) | |
| { | |
| context.FieldValuePairsInBody[property.Name] = GetParameterValue(property.Value); | |
| if (parameters.Count > 0) | |
| { | |
| foreach (KeyValuePair<string, object?> parameter in parameters) | |
| { | |
| context.FieldValuePairsInBody[parameter.Key] = parameter.Value; |
| public Tool GetToolMetadata() | ||
| { | ||
| string toolName = ConvertToToolName(_entityName); | ||
| string description = _entity.Description ?? $"Execute {_entityName} stored procedure"; | ||
|
|
||
| // Build input schema based on parameters | ||
| JsonElement inputSchema = BuildInputSchema(); | ||
|
|
||
| return new Tool | ||
| { | ||
| Name = toolName, | ||
| Description = description, | ||
| InputSchema = inputSchema | ||
| }; | ||
| } |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stored Entity object (_entity) captured during construction may become stale if the RuntimeConfig is reloaded at runtime. The ExecuteAsync method correctly re-validates the entity from the current config (lines 110-118), but other methods like GetToolMetadata use the stored _entity which could contain outdated information. Consider either re-fetching the entity in GetToolMetadata or adding a comment documenting this potential staleness and why it's acceptable for metadata.
| foreach (IMcpTool customTool in CustomMcpToolFactory.CreateCustomTools(config)) | ||
| { | ||
| services.AddSingleton<IMcpTool>(customTool); | ||
| } |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tool name collisions are not handled. If two entities have names that convert to the same tool name (e.g., "GetUser" and "get_user" would both become "get_user"), the second tool would silently override the first in the service collection. Consider adding validation to detect and log/throw on tool name collisions during registration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is already captured as a subsequent work item- #3049
| foreach (ParameterMetadata param in entityConfig.Source.Parameters) | ||
| { | ||
| if (!context.FieldValuePairsInBody.ContainsKey(param.Name)) | ||
| { | ||
| context.FieldValuePairsInBody[param.Name] = param.Default; | ||
| } | ||
| } |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
Why make this change?
What is this change?
This pull request introduces a new system for dynamically generating and registering custom MCP tools based on stored procedure entity configurations in the runtime configuration. The main changes are the implementation of the
DynamicCustomToolclass, a factory to create these tools from configuration, and the necessary service registration logic to ensure these custom tools are available at runtime.Dynamic custom MCP tool support:
DynamicCustomToolclass, which implementsIMcpTooland provides logic for generating tool metadata, validating configuration, handling authorization, executing the underlying stored procedure, and formatting the response. This enables each stored procedure entity withcustom-toolenabled to be exposed as a dedicated MCP tool.CustomMcpToolFactoryclass, which scans the runtime configuration for stored procedure entities marked withcustom-toolenabled and creates correspondingDynamicCustomToolinstances.Dependency injection and service registration:
AddDabMcpServer) to register custom tools generated from configuration by calling a newRegisterCustomToolsmethod after auto-discovering static tools.RegisterAllMcpToolsmethod to excludeDynamicCustomToolfrom auto-discovery (since these are created dynamically per configuration) and added theRegisterCustomToolsmethod to register each generated custom tool as a singleton service.How was this tested?
Sample Request(s)
Error Scenarios
6. Missing Required Parameter
Edge Cases
9. SQL Injection Attempt