Skip to content
42 changes: 42 additions & 0 deletions src/Azure.DataApiBuilder.Mcp/Core/DynamicCustomTool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -378,6 +386,11 @@ private JsonElement BuildInputSchema()
["properties"] = properties
};

if (requiredParameters.Count > 0)
{
schema["required"] = requiredParameters;
}

return JsonSerializer.SerializeToElement(schema);
}

Expand All @@ -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)
{
Expand All @@ -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;
}
Comment thread
Copilot marked this conversation as resolved.

/// <summary>
/// Maps a .NET System.Type to the appropriate JSON Schema type string.
/// </summary>
Expand Down
77 changes: 77 additions & 0 deletions src/Service.Tests/Mcp/DynamicCustomToolMsSqlIntegrationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
148 changes: 148 additions & 0 deletions src/Service.Tests/Mcp/DynamicCustomToolTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
};
Comment thread
Copilot marked this conversation as resolved.

// 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()
Comment thread
souvikghosh04 marked this conversation as resolved.
{
// 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.
Expand Down