diff --git a/src/Azure.DataApiBuilder.Mcp/Core/DynamicCustomTool.cs b/src/Azure.DataApiBuilder.Mcp/Core/DynamicCustomTool.cs index 5a94acd4cb..e1a48a6e0d 100644 --- a/src/Azure.DataApiBuilder.Mcp/Core/DynamicCustomTool.cs +++ b/src/Azure.DataApiBuilder.Mcp/Core/DynamicCustomTool.cs @@ -361,6 +361,7 @@ private JsonElement BuildInputSchema() } Dictionary properties = new(); + List requiredParameters = new(); foreach ((string paramName, ParameterDefinition paramDef) in spDefinition.Parameters) { Dictionary paramSchema = new() @@ -370,6 +371,13 @@ private JsonElement BuildInputSchema() }; properties[paramName] = paramSchema; + + // A parameter is required unless config marks it optional or supplies a default + // the engine applies when the caller omits it. + if (IsParameterRequired(paramDef.Required, paramDef.HasConfigDefault)) + { + requiredParameters.Add(paramName); + } } Dictionary schema = new() @@ -378,6 +386,11 @@ private JsonElement BuildInputSchema() ["properties"] = properties }; + if (requiredParameters.Count > 0) + { + schema["required"] = requiredParameters; + } + return JsonSerializer.SerializeToElement(schema); } @@ -396,6 +409,7 @@ private JsonElement BuildInputSchemaFromConfig() if (_entity.Source.Parameters != null && _entity.Source.Parameters.Any()) { Dictionary properties = (Dictionary)schema["properties"]; + List requiredParameters = new(); foreach (ParameterMetadata param in _entity.Source.Parameters) { @@ -404,12 +418,39 @@ private JsonElement BuildInputSchemaFromConfig() ["type"] = new[] { "string", "number", "boolean", "null" }, ["description"] = param.Description ?? $"Parameter {param.Name}" }; + + // A parameter is required unless config marks it optional or supplies a default. + if (IsParameterRequired(param.Required, param.Default is not null)) + { + requiredParameters.Add(param.Name); + } + } + + if (requiredParameters.Count > 0) + { + schema["required"] = requiredParameters; } } return JsonSerializer.SerializeToElement(schema); } + /// + /// Determines whether a stored procedure parameter should be advertised as required in the + /// tool input schema. A parameter is required when configuration does not mark it optional + /// and does not provide a default value the engine can apply when the caller omits it. + /// + private static bool IsParameterRequired(bool? configuredRequired, bool hasDefault) + { + if (configuredRequired is not null) + { + return configuredRequired.Value; + } + + // No explicit config: a parameter is required unless a default is available. + return !hasDefault; + } + /// /// Maps a .NET System.Type to the appropriate JSON Schema type string. /// diff --git a/src/Service.Tests/Mcp/DynamicCustomToolMsSqlIntegrationTests.cs b/src/Service.Tests/Mcp/DynamicCustomToolMsSqlIntegrationTests.cs index 9ebadeacdd..d6cf1bf2b6 100644 --- a/src/Service.Tests/Mcp/DynamicCustomToolMsSqlIntegrationTests.cs +++ b/src/Service.Tests/Mcp/DynamicCustomToolMsSqlIntegrationTests.cs @@ -177,6 +177,83 @@ public void InitializeMetadata_DescriptionIncludesConfigDefaults(string entityNa $"'{paramName}' description should mention config default '{expectedDefault}'."); } + /// + /// Validates that a DB-discovered parameter without a config default is advertised as required. + /// + [TestMethod] + public void InitializeMetadata_RequiredArray_IncludesParamWithoutDefault() + { + IServiceProvider serviceProvider = BuildQueryServiceProvider(); + RuntimeConfigProvider configProvider = serviceProvider.GetRequiredService(); + Entity entity = configProvider.GetConfig().Entities["GetBook"]; + + DynamicCustomTool tool = new("GetBook", entity); + tool.InitializeMetadata(serviceProvider); + + JsonElement schema = tool.GetToolMetadata().InputSchema; + + Assert.IsTrue(schema.TryGetProperty("required", out JsonElement required), + "GetBook schema should expose a 'required' array."); + + List requiredParams = new(); + foreach (JsonElement element in required.EnumerateArray()) + { + requiredParams.Add(element.GetString()!); + } + + CollectionAssert.Contains(requiredParams, "id", + "'id' has no config default and should be required."); + } + + /// + /// Validates that parameters with config defaults (marked optional) are excluded from required. + /// + [TestMethod] + public void InitializeMetadata_RequiredArray_ExcludesParamsWithConfigDefaults() + { + IServiceProvider serviceProvider = BuildQueryServiceProvider(); + RuntimeConfigProvider configProvider = serviceProvider.GetRequiredService(); + Entity entity = configProvider.GetConfig().Entities["InsertBook"]; + + DynamicCustomTool tool = new("InsertBook", entity); + tool.InitializeMetadata(serviceProvider); + + JsonElement schema = tool.GetToolMetadata().InputSchema; + + List requiredParams = new(); + if (schema.TryGetProperty("required", out JsonElement required)) + { + foreach (JsonElement element in required.EnumerateArray()) + { + requiredParams.Add(element.GetString()!); + } + } + + CollectionAssert.DoesNotContain(requiredParams, "title", + "'title' has a config default and must not be required."); + CollectionAssert.DoesNotContain(requiredParams, "publisher_id", + "'publisher_id' has a config default and must not be required."); + } + + /// + /// Validates that a zero-parameter SP does not emit a 'required' array. + /// + [TestMethod] + public void InitializeMetadata_ZeroParamSP_OmitsRequiredArray() + { + IServiceProvider serviceProvider = BuildQueryServiceProvider(); + RuntimeConfigProvider configProvider = serviceProvider.GetRequiredService(); + Entity entity = configProvider.GetConfig().Entities["GetBooks"]; + + DynamicCustomTool tool = new("GetBooks", entity); + tool.InitializeMetadata(serviceProvider); + + JsonElement schema = tool.GetToolMetadata().InputSchema; + + Assert.IsFalse(schema.TryGetProperty("required", out _), + "Zero-param SP should not include a 'required' array."); + } + #endregion /// diff --git a/src/Service.Tests/Mcp/DynamicCustomToolTests.cs b/src/Service.Tests/Mcp/DynamicCustomToolTests.cs index bb67aec5a0..30b0970092 100644 --- a/src/Service.Tests/Mcp/DynamicCustomToolTests.cs +++ b/src/Service.Tests/Mcp/DynamicCustomToolTests.cs @@ -710,6 +710,126 @@ public void InitializeMetadata_ZeroParamSP_ReturnsEmptyProperties() Assert.AreEqual(0, props.EnumerateObject().Count()); } + /// + /// DB-discovered parameters with no config default/override are advertised as required. + /// + [TestMethod] + public void InitializeMetadata_IncludesRequiredArray_ForParamsWithoutDefaults() + { + // Arrange + Dictionary dbParams = new() + { + ["productId"] = new() { SystemType = typeof(int) } + }; + + // Act + JsonElement schema = InitializeAndGetSchema(dbParams); + + // Assert + Assert.IsTrue(schema.TryGetProperty("required", out JsonElement required), + "Schema should expose a 'required' array for parameters without defaults."); + CollectionAssert.AreEquivalent( + new[] { "productId" }, + EnumerateStrings(required), + "'productId' should be listed as required."); + } + + /// + /// Parameters that have a config default or are explicitly marked optional are excluded + /// from the required array, while parameters without defaults remain required. + /// + [TestMethod] + public void InitializeMetadata_ExcludesParamsWithDefaultsOrOptionalFlag_FromRequired() + { + // Arrange + Dictionary dbParams = new() + { + ["id"] = new() { SystemType = typeof(int) }, + ["title"] = new() { SystemType = typeof(string), HasConfigDefault = true, ConfigDefaultValue = "randomX" }, + ["publisher_id"] = new() { SystemType = typeof(int), Required = false } + }; + + // Act + JsonElement schema = InitializeAndGetSchema(dbParams); + + // Assert + Assert.IsTrue(schema.TryGetProperty("required", out JsonElement required)); + CollectionAssert.AreEquivalent( + new[] { "id" }, + EnumerateStrings(required), + "Only 'id' (no default, not marked optional) should be required."); + } + + /// + /// A zero-parameter SP must not emit a 'required' array. + /// + [TestMethod] + public void InitializeMetadata_ZeroParamSP_OmitsRequiredArray() + { + // Arrange & Act + JsonElement schema = InitializeAndGetSchema(new Dictionary()); + + // Assert + Assert.IsFalse(schema.TryGetProperty("required", out _), + "Zero-param SP should not include a 'required' array."); + } + + /// + /// When falling back to config-based schema, parameters without a default are required. + /// + [TestMethod] + public void GetToolMetadata_ConfigFallback_MarksParamsWithoutDefaultsRequired() + { + // Arrange - config declares one required param and one with a default + ParameterMetadata[] parameters = new[] + { + new ParameterMetadata { Name = "userId" }, + new ParameterMetadata { Name = "tenant", Default = "contoso" } + }; + Entity entity = CreateTestStoredProcedureEntity(parameters: parameters); + DynamicCustomTool tool = new("GetUser", entity); + + // Act - no InitializeMetadata call, so the config-based schema is used + JsonElement schema = tool.GetToolMetadata().InputSchema; + + // Assert + Assert.IsTrue(schema.TryGetProperty("required", out JsonElement required)); + CollectionAssert.AreEquivalent( + new[] { "userId" }, + EnumerateStrings(required), + "Only 'userId' (no default) should be required in the config-based schema."); + } + + /// + /// Helper: Parses the "required" array values into a list of strings. + /// + private static List EnumerateStrings(JsonElement array) + { + List values = new(); + foreach (JsonElement element in array.EnumerateArray()) + { + values.Add(element.GetString()!); + } + + return values; + } + + /// + /// Helper: Creates a DynamicCustomTool, initializes it with mocked DB metadata, and returns + /// the full input schema element. + /// + private static JsonElement InitializeAndGetSchema( + Dictionary dbParameters, + string entityName = "TestSP") + { + Entity entity = CreateTestStoredProcedureEntity(); + DynamicCustomTool tool = new(entityName, entity); + IServiceProvider sp = BuildServiceProviderForMetadata(entityName, dbParameters); + + tool.InitializeMetadata(sp); + return tool.GetToolMetadata().InputSchema; + } + /// /// Helper: Creates a DynamicCustomTool, initializes it with mocked DB metadata, and returns /// the "properties" element from the resulting input schema.