Skip to content

Commit c5419c6

Browse files
committed
Refactor ElicitRequestParams and McpServerExtensions. #630
- Simplified "type" property handling in Converter. - Changed s_lazyElicitAllowedProperties to nullable type. - Updated ElicitAsync to use static lambda for clarity. - Refactored CreatePrimitiveSchema to handle nullable types. - Initialized allowed properties in TryValidateElicitationPrimitiveSchema. - Updated ElicitationTypedTests for naming conventions and added tests for nullable properties. - Enforced required properties in SampleForm and introduced NullablePropertyForm. - Added JSON source generation context for NullablePropertyForm.
1 parent 2f1dcf0 commit c5419c6

3 files changed

Lines changed: 74 additions & 28 deletions

File tree

src/ModelContextProtocol.Core/Protocol/ElicitRequestParams.cs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -126,14 +126,7 @@ public class Converter : JsonConverter<PrimitiveSchemaDefinition>
126126
switch (propertyName)
127127
{
128128
case "type":
129-
if (reader.TokenType == JsonTokenType.String)
130-
{
131-
type = reader.GetString();
132-
}
133-
else
134-
{
135-
throw new JsonException("The 'type' property must be a string.");
136-
}
129+
type = reader.GetString();
137130
break;
138131

139132
case "title":

src/ModelContextProtocol.Core/Server/McpServerExtensions.cs

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,7 @@ public static class McpServerExtensions
2121
/// </summary>
2222
private static readonly ConditionalWeakTable<JsonSerializerOptions, ConcurrentDictionary<Type, ElicitRequestParams.RequestSchema>> s_elicitResultSchemaCache = new();
2323

24-
private static Lazy<Dictionary<string, HashSet<string>>> s_lazyElicitAllowedProperties = new(()=> new()
25-
{
26-
["string"] = ["type", "title", "description", "minLength", "maxLength", "format", "enum", "enumNames"],
27-
["number"] = ["type", "title", "description", "minimum", "maximum"],
28-
["boolean"] = ["type", "title", "description", "default"]
29-
});
24+
private static Dictionary<string, HashSet<string>>? s_lazyElicitAllowedProperties = null;
3025

3126
/// <summary>
3227
/// Requests to sample an LLM via the client using the specified request parameters.
@@ -279,7 +274,7 @@ public static ValueTask<ElicitResult> ElicitAsync(
279274
var dict = s_elicitResultSchemaCache.GetValue(serializerOptions, _ => new());
280275

281276
#if NET
282-
var schema = dict.GetOrAdd<JsonSerializerOptions>(typeof(T), (t, s) => BuildRequestSchema(t, s), serializerOptions);
277+
var schema = dict.GetOrAdd(typeof(T), static (t, s) => BuildRequestSchema(t, s), serializerOptions);
283278
#else
284279
var schema = dict.GetOrAdd(typeof(T), type => BuildRequestSchema(type, serializerOptions));
285280
#endif
@@ -344,19 +339,21 @@ private static ElicitRequestParams.RequestSchema BuildRequestSchema(Type type, J
344339
/// <exception cref="McpException">Thrown when the type is not supported.</exception>
345340
private static ElicitRequestParams.PrimitiveSchemaDefinition CreatePrimitiveSchema(Type type, JsonSerializerOptions serializerOptions)
346341
{
347-
var originalType = type;
348-
var underlyingType = Nullable.GetUnderlyingType(originalType) ?? originalType;
349-
350-
var typeInfo = serializerOptions.GetTypeInfo(underlyingType);
342+
if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>))
343+
{
344+
throw new McpException($"Type '{type.FullName}' is not a supported property type for elicitation requests. Nullable types are not supported.");
345+
}
346+
347+
var typeInfo = serializerOptions.GetTypeInfo(type);
351348

352349
if (typeInfo.Kind != JsonTypeInfoKind.None)
353350
{
354-
throw new McpException($"Type '{originalType.FullName}' is not a supported property type for elicitation requests.");
351+
throw new McpException($"Type '{type.FullName}' is not a supported property type for elicitation requests.");
355352
}
356353

357-
var jsonElement = AIJsonUtilities.CreateJsonSchema(underlyingType, serializerOptions: serializerOptions);
354+
var jsonElement = AIJsonUtilities.CreateJsonSchema(type, serializerOptions: serializerOptions);
358355

359-
if (!TryValidateElicitationPrimitiveSchema(jsonElement, originalType, out var error))
356+
if (!TryValidateElicitationPrimitiveSchema(jsonElement, type, out var error))
360357
{
361358
throw new McpException(error);
362359
}
@@ -365,7 +362,7 @@ private static ElicitRequestParams.PrimitiveSchemaDefinition CreatePrimitiveSche
365362
jsonElement.Deserialize(McpJsonUtilities.JsonContext.Default.PrimitiveSchemaDefinition);
366363

367364
if (primitiveSchemaDefinition is null)
368-
throw new McpException($"Type '{originalType.FullName}' is not a supported property type for elicitation requests.");
365+
throw new McpException($"Type '{type.FullName}' is not a supported property type for elicitation requests.");
369366

370367
return primitiveSchemaDefinition;
371368
}
@@ -421,7 +418,14 @@ private static bool TryValidateElicitationPrimitiveSchema(JsonElement schema, Ty
421418
if (typeKeyword == "integer")
422419
typeKeyword = "number";
423420

424-
var allowed = s_lazyElicitAllowedProperties.Value[typeKeyword];
421+
s_lazyElicitAllowedProperties ??= new()
422+
{
423+
["string"] = ["type", "title", "description", "minLength", "maxLength", "format", "enum", "enumNames"],
424+
["number"] = ["type", "title", "description", "minimum", "maximum"],
425+
["boolean"] = ["type", "title", "description", "default"]
426+
};
427+
428+
var allowed = s_lazyElicitAllowedProperties[typeKeyword];
425429

426430
foreach (JsonProperty prop in schema.EnumerateObject())
427431
{

tests/ModelContextProtocol.Tests/Protocol/ElicitationTypedTests.cs

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer
3535
Assert.Equal(SampleRole.Admin, result.Content!.Role);
3636
Assert.Equal(99.5, result.Content!.Score);
3737
}
38-
else if (request.Params!.Name == "TestElicitationTypedCamel")
38+
else if (request.Params!.Name == "TestElicitationCamelForm")
3939
{
4040
var result = await request.Server.ElicitAsync<CamelForm>(
4141
message: "Please provide more information.",
@@ -48,6 +48,19 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer
4848
Assert.Equal(90210, result.Content!.ZipCode);
4949
Assert.False(result.Content!.IsAdmin);
5050
}
51+
else if (request.Params!.Name == "TestElicitationNullablePropertyForm")
52+
{
53+
var result = await request.Server.ElicitAsync<NullablePropertyForm>(
54+
message: "Please provide more information.",
55+
serializerOptions: ElicitationNullablePropertyJsonContext.Default.Options,
56+
cancellationToken: CancellationToken.None);
57+
58+
// Should be unreachable
59+
return new CallToolResult
60+
{
61+
Content = [new TextContentBlock { Text = "unexpected" }],
62+
};
63+
}
5164
else if (request.Params!.Name == "TestElicitationUnsupportedType")
5265
{
5366
await request.Server.ElicitAsync<UnsupportedForm>(
@@ -222,7 +235,7 @@ public async Task Elicit_Typed_Respects_NamingPolicy()
222235
},
223236
});
224237

225-
var result = await client.CallToolAsync("TestElicitationTypedCamel", cancellationToken: TestContext.Current.CancellationToken);
238+
var result = await client.CallToolAsync("TestElicitationCamelForm", cancellationToken: TestContext.Current.CancellationToken);
226239
Assert.Equal("success", (result.Content[0] as TextContentBlock)?.Text);
227240
}
228241

@@ -251,6 +264,29 @@ public async Task Elicit_Typed_With_Unsupported_Property_Type_Throws()
251264
Assert.Contains(typeof(UnsupportedForm.Nested).FullName!, ex.Message);
252265
}
253266

267+
[Fact]
268+
public async Task Elicit_Typed_With_Nullable_Property_Type_Throws()
269+
{
270+
await using IMcpClient client = await CreateMcpClientForServer(new McpClientOptions
271+
{
272+
Capabilities = new()
273+
{
274+
Elicitation = new()
275+
{
276+
// Handler should never be invoked because the exception occurs before the request is sent.
277+
ElicitationHandler = async (req, ct) =>
278+
{
279+
Assert.Fail("Elicitation handler should not be called for unsupported schema test.");
280+
return new ElicitResult { Action = "cancel" };
281+
},
282+
},
283+
},
284+
});
285+
286+
var ex = await Assert.ThrowsAsync<McpException>(async () =>
287+
await client.CallToolAsync("TestElicitationNullablePropertyForm", cancellationToken: TestContext.Current.CancellationToken));
288+
}
289+
254290
[Fact]
255291
public async Task Elicit_Typed_With_NonObject_Generic_Type_Throws()
256292
{
@@ -286,9 +322,9 @@ public enum SampleRole
286322

287323
public sealed class SampleForm
288324
{
289-
public string? Name { get; set; }
325+
public required string Name { get; set; }
290326
public int Age { get; set; }
291-
public bool? Active { get; set; }
327+
public bool Active { get; set; }
292328
public SampleRole Role { get; set; }
293329
public double Score { get; set; }
294330

@@ -297,6 +333,13 @@ public sealed class SampleForm
297333
}
298334

299335
public sealed class CamelForm
336+
{
337+
public required string FirstName { get; set; }
338+
public int ZipCode { get; set; }
339+
public bool IsAdmin { get; set; }
340+
}
341+
342+
public sealed class NullablePropertyForm
300343
{
301344
public string? FirstName { get; set; }
302345
public int ZipCode { get; set; }
@@ -313,6 +356,12 @@ internal partial class ElicitationTypedDefaultJsonContext : JsonSerializerContex
313356
[JsonSerializable(typeof(JsonElement))]
314357
internal partial class ElicitationTypedCamelJsonContext : JsonSerializerContext;
315358

359+
360+
[JsonSourceGenerationOptions(PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase)]
361+
[JsonSerializable(typeof(NullablePropertyForm))]
362+
[JsonSerializable(typeof(JsonElement))]
363+
internal partial class ElicitationNullablePropertyJsonContext : JsonSerializerContext;
364+
316365
public sealed class UnsupportedForm
317366
{
318367
public string? Name { get; set; }

0 commit comments

Comments
 (0)