Merge branch 'main' into dev/rubencerna/fix-session-context-insertion-bug

Author: RubenCerna2079

src/Azure.DataApiBuilder.Mcp/Core/DynamicCustomTool.cs

@@ -361,6 +361,7 @@ private JsonElement BuildInputSchema()
             }
 
             Dictionary<string, object> properties = new();
+            List<string> requiredParameters = new();
             foreach ((string paramName, ParameterDefinition paramDef) in spDefinition.Parameters)
             {
                 Dictionary<string, object> paramSchema = new()
@@ -370,6 +371,13 @@ private JsonElement BuildInputSchema()
                 };
 
                 properties[paramName] = paramSchema;
+
+                // A DB metadata 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<string, object> 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<string, object> properties = (Dictionary<string, object>)schema["properties"];
+                List<string> requiredParameters = new();
 
                 foreach (ParameterMetadata param in _entity.Source.Parameters)
                 {
@@ -404,12 +418,40 @@ 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);
         }
 
+        /// <summary>
+        /// 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.
+        /// </summary>
+        private static bool IsParameterRequired(bool? configuredRequired, bool hasDefault)
+        {
+            // If the engine can supply a default when the caller omits the parameter,
+            // it should not be advertised as required.
+            if (hasDefault)
+            {
+                return false;
+            }
+
+            return configuredRequired ?? true;
+        }
+
         /// <summary>
         /// Maps a .NET System.Type to the appropriate JSON Schema type string.
         /// </summary>

src/Service.Tests/Mcp/DynamicCustomToolMsSqlIntegrationTests.cs

@@ -177,6 +177,83 @@ public void InitializeMetadata_DescriptionIncludesConfigDefaults(string entityNa
                 $"'{paramName}' description should mention config default '{expectedDefault}'.");
         }
 
+        /// <summary>
+        /// Validates that a DB-discovered parameter without a config default is advertised as required.
+        /// </summary>
+        [TestMethod]
+        public void InitializeMetadata_RequiredArray_IncludesParamWithoutDefault()
+        {
+            IServiceProvider serviceProvider = BuildQueryServiceProvider();
+            RuntimeConfigProvider configProvider = serviceProvider.GetRequiredService<RuntimeConfigProvider>();
+            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<string> 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.");
+        }
+
+        /// <summary>
+        /// Validates that parameters with config defaults (marked optional) are excluded from required.
+        /// </summary>
+        [TestMethod]
+        public void InitializeMetadata_RequiredArray_ExcludesParamsWithConfigDefaults()
+        {
+            IServiceProvider serviceProvider = BuildQueryServiceProvider();
+            RuntimeConfigProvider configProvider = serviceProvider.GetRequiredService<RuntimeConfigProvider>();
+            Entity entity = configProvider.GetConfig().Entities["InsertBook"];
+
+            DynamicCustomTool tool = new("InsertBook", entity);
+            tool.InitializeMetadata(serviceProvider);
+
+            JsonElement schema = tool.GetToolMetadata().InputSchema;
+
+            List<string> 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.");
+        }
+
+        /// <summary>
+        /// Validates that a zero-parameter SP does not emit a 'required' array.
+        /// </summary>
+        [TestMethod]
+        public void InitializeMetadata_ZeroParamSP_OmitsRequiredArray()
+        {
+            IServiceProvider serviceProvider = BuildQueryServiceProvider();
+            RuntimeConfigProvider configProvider = serviceProvider.GetRequiredService<RuntimeConfigProvider>();
+            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
 
         /// <summary>

src/Service.Tests/Mcp/DynamicCustomToolTests.cs

@@ -710,6 +710,154 @@ public void InitializeMetadata_ZeroParamSP_ReturnsEmptyProperties()
             Assert.AreEqual(0, props.EnumerateObject().Count());
         }
 
+        /// <summary>
+        /// DB-discovered parameters with no config default/override are advertised as required.
+        /// </summary>
+        [TestMethod]
+        public void InitializeMetadata_IncludesRequiredArray_ForParamsWithoutDefaults()
+        {
+            // Arrange
+            Dictionary<string, ParameterDefinition> 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.");
+        }
+
+        /// <summary>
+        /// Parameters that have a config default or are explicitly marked optional are excluded
+        /// from the required array, while parameters without defaults remain required.
+        /// </summary>
+        [TestMethod]
+        public void InitializeMetadata_ExcludesParamsWithDefaultsOrOptionalFlag_FromRequired()
+        {
+            // Arrange
+            Dictionary<string, ParameterDefinition> dbParams = new()
+            {
+                ["id"] = new() { SystemType = typeof(int) },
+                ["title"] = new() { SystemType = typeof(string), HasConfigDefault = true, ConfigDefaultValue = "randomX" },
+                ["category"] = new() { SystemType = typeof(string), Required = true, HasConfigDefault = true, ConfigDefaultValue = "defaultCategory" },
+                ["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.");
+        }
+
+        /// <summary>
+        /// A zero-parameter SP must not emit a 'required' array.
+        /// </summary>
+        [TestMethod]
+        public void InitializeMetadata_ZeroParamSP_OmitsRequiredArray()
+        {
+            // Arrange & Act
+            JsonElement schema = InitializeAndGetSchema(new Dictionary<string, ParameterDefinition>());
+
+            // Assert
+            Assert.IsFalse(schema.TryGetProperty("required", out _),
+                "Zero-param SP should not include a 'required' array.");
+        }
+
+        /// <summary>
+        /// When falling back to config-based schema, parameters without a default are required.
+        /// </summary>
+        [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.");
+        }
+
+        /// <summary>
+        /// Config fallback should exclude parameters with defaults from the required array even when
+        /// they are explicitly marked required.
+        /// </summary>
+        [TestMethod]
+        public void GetToolMetadata_ConfigFallback_ExcludesRequiredParamWhenDefaultExists()
+        {
+            // Arrange
+            ParameterMetadata[] parameters = new[]
+            {
+                new ParameterMetadata { Name = "userId", Required = true },
+                new ParameterMetadata { Name = "tenant", Required = true, Default = "contoso" }
+            };
+            Entity entity = CreateTestStoredProcedureEntity(parameters: parameters);
+            DynamicCustomTool tool = new("GetUser", entity);
+
+            // Act
+            JsonElement schema = tool.GetToolMetadata().InputSchema;
+
+            // Assert
+            Assert.IsTrue(schema.TryGetProperty("required", out JsonElement required));
+            CollectionAssert.AreEquivalent(
+                new[] { "userId" },
+                EnumerateStrings(required),
+                "Parameters with config defaults should not be required even when marked required.");
+        }
+
+        /// <summary>
+        /// Helper: Parses the "required" array values into a list of strings.
+        /// </summary>
+        private static List<string> EnumerateStrings(JsonElement array)
+        {
+            List<string> values = new();
+            foreach (JsonElement element in array.EnumerateArray())
+            {
+                values.Add(element.GetString()!);
+            }
+
+            return values;
+        }
+
+        /// <summary>
+        /// Helper: Creates a DynamicCustomTool, initializes it with mocked DB metadata, and returns
+        /// the full input schema element.
+        /// </summary>
+        private static JsonElement InitializeAndGetSchema(
+            Dictionary<string, ParameterDefinition> dbParameters,
+            string entityName = "TestSP")
+        {
+            Entity entity = CreateTestStoredProcedureEntity();
+            DynamicCustomTool tool = new(entityName, entity);
+            IServiceProvider sp = BuildServiceProviderForMetadata(entityName, dbParameters);
+
+            tool.InitializeMetadata(sp);
+            return tool.GetToolMetadata().InputSchema;
+        }
+
         /// <summary>
         /// Helper: Creates a DynamicCustomTool, initializes it with mocked DB metadata, and returns
         /// the "properties" element from the resulting input schema.