-
Notifications
You must be signed in to change notification settings - Fork 587
Fix ToJsonObject serialization failure with anonymous types and add support for custom JsonSerializerOptions #1113
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
- Updated ToJsonObject to use JsonSerializer.SerializeToNode(object, Type, JsonSerializerOptions) instead of strongly-typed JsonTypeInfo - Added DefaultJsonTypeInfoResolver to McpJsonUtilities.DefaultOptions to enable reflection-based serialization for user-defined types - Fixed bug in FunctionResultContent serialization where it was serializing 'content' instead of 'resultContent.Result' - Simplified default case in ToContentBlock to avoid serializing unsupported AIContent types - Added comprehensive tests for anonymous types in AdditionalProperties - All tests passing on .NET 8, 9, and 10 Co-authored-by: stephentoub <[email protected]>
- Updated test to verify that DefaultOptions now supports anonymous types unconditionally - Test reflects that DefaultJsonTypeInfoResolver is always added to support user-defined types Co-authored-by: stephentoub <[email protected]>
- Use more efficient JsonSerializer.Serialize overload that takes Type parameter directly - Add suppression attributes to ToContentBlock method for trim/AOT warnings Co-authored-by: stephentoub <[email protected]>
…ntext properties - Changed ToJsonObject to use McpJsonUtilities.DefaultOptions.GetTypeInfo(typeof(IReadOnlyDictionary<string, object?>)) - Changed ToolUseContentBlock deserialization to use GetTypeInfo<IDictionary<string, object?>>() - Removed UnconditionalSuppressMessage attributes (not needed) - Reverted McpJsonUtilities to original (no DefaultJsonTypeInfoResolver) - Reverted FunctionResultContent and default case serialization to original code - Updated tests to skip when reflection is disabled (JsonSerializer.IsReflectionEnabledByDefault) This fix allows anonymous types to work when reflection is enabled (default on .NET 8/10 and opt-in on .NET 9) while maintaining AOT compatibility with source generators. Co-authored-by: stephentoub <[email protected]>
tests/ModelContextProtocol.Tests/AIContentExtensionsAnonymousTypeTests.cs
Outdated
Show resolved
Hide resolved
- Consolidated all anonymous type tests into AIContentExtensionsTests.cs - Removed AIContentExtensionsAnonymousTypeTests.cs and RegressionTests.cs - Tests are now colocated with other content block tests as requested Co-authored-by: stephentoub <[email protected]>
…JsonObject - Added optional JsonSerializerOptions parameter to ToContentBlock method - Added optional JsonSerializerOptions parameter to ToJsonObject helper method - Updated all serialization calls within ToContentBlock to use provided options - Updated XML documentation references in McpServerTool and McpServerToolAttribute - Added test to verify custom JsonSerializerOptions are properly used - Enables user-defined serialization options to be passed through for anonymous types and custom converters Co-authored-by: eiriktsarpalis <[email protected]>
- Added ToContentBlock_WithNamedUserDefinedTypeInAdditionalProperties_Works test - Test demonstrates that named types work with source generation regardless of reflection being enabled - Created TestCoordinates record and NamedTypeTestJsonContext for source generation support - Test passes on all frameworks including .NET 9 where reflection is disabled by default Co-authored-by: eiriktsarpalis <[email protected]>
|
@copilot, the concerns here apply not just to direct serialization and deserialization of dictionaries, but also types that contain dictionaries. For example, can you validate that anonymous types work as argument values iin a CallToolAsync call? Please review all serialization and deserialization calls that are using DefaultContext, looking for other places where user types might be in the object graph, test that there's indeed a problem with each, add tests for each (to existing files), and then fix each. |
- Added test for CallToolAsync with anonymous type arguments - Added test for ToChatMessage with anonymous types in ContentBlock.Meta - Added ArgumentEchoTool to McpClientToolTests to support testing - Tests verify that anonymous types work correctly when reflection is enabled - All tests pass, indicating AIJsonUtilities resolver chain handles anonymous types The tests demonstrate that user-defined types in dictionaries are properly serialized through the existing resolver chain that includes AIJsonUtilities.DefaultOptions. Co-authored-by: stephentoub <[email protected]>
Added tests for Investigation findings:
The existing implementation appears to handle user-defined types in object graphs correctly through the chained type resolvers. The |
|
@copilot, regarding:
Doesn't that mean there could be problems with ToChatMessage? JsonContext.Default.CallToolResult will not see the chained in default resolver, right? |
- Updated ToChatMessage to accept optional JsonSerializerOptions parameter - Changed from using JsonContext.Default.CallToolResult to options.GetTypeInfo<CallToolResult>() - Ensures user-defined serialization options are used throughout the call chain - Defaults to McpJsonUtilities.DefaultOptions when no options provided - All tests passing on .NET 8, 9, and 10 Co-authored-by: stephentoub <[email protected]>
You're correct. Using |
…ct method - Changed ToJsonObject parameter from optional to required (non-optional) - Updated all call sites to explicitly pass JsonSerializerOptions - Updated AIContentExtensions.cs line 78 to pass McpJsonUtilities.DefaultOptions - Updated McpServer.Methods.cs lines 151 and 175 to pass McpJsonUtilities.DefaultOptions - Ensures all locations explicitly specify serialization options - All tests passing on .NET 8, 9, and 10 Co-authored-by: eiriktsarpalis <[email protected]>
… and SampleAsync - Added optional serializerOptions parameter to CreateSamplingHandler - Added optional serializerOptions parameter to McpServer.SampleAsync(IEnumerable<ChatMessage>, ...) - Updated all hard-coded McpJsonUtilities.DefaultOptions calls to use the parameter - Updated call site in McpSamplingChatClient.GetResponseAsync to pass null explicitly - Updated XML documentation reference in McpServerOptions - Allows users to control serialization of user-provided objects (e.g., AdditionalProperties) - All tests passing on .NET 8, 9, and 10 Co-authored-by: stephentoub <[email protected]>
|
|
||
| ToolUseContentBlock toolUse => FunctionCallContent.CreateFromParsedArguments(toolUse.Input, toolUse.Id, toolUse.Name, | ||
| static json => JsonSerializer.Deserialize(json, McpJsonUtilities.JsonContext.Default.IDictionaryStringObject)), | ||
| static json => JsonSerializer.Deserialize(json, McpJsonUtilities.DefaultOptions.GetTypeInfo<IDictionary<string, object?>>())), |
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.
@copilot, ToAIContent needs to accept an optional JSO, and all callers in the library should be passing in a user supplied instance
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.
Added optional JsonSerializerOptions parameter to ToAIContent methods and updated all callers to pass user-supplied options in commit 46425b2.
| { | ||
| Throw.IfNull(chatClient); | ||
|
|
||
| serializerOptions ??= McpJsonUtilities.DefaultOptions; |
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.
@copilot, this serializer options needs to be passed into the constructed chart client instance and then used in that instance anywhere a JSO is needed, like the call to sampleasync
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.
Updated CreateSamplingHandler to pass serializer options through to ToChatClientArguments, which now uses them when calling ToAIContent in commit 46425b2.
| /// <inheritdoc/> | ||
| public Task<ChatResponse> GetResponseAsync(IEnumerable<ChatMessage> messages, ChatOptions? chatOptions = null, CancellationToken cancellationToken = default) => | ||
| _server.SampleAsync(messages, chatOptions, cancellationToken); | ||
| _server.SampleAsync(messages, chatOptions, serializerOptions: null, cancellationToken); |
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.
@copilot, anywhere in the libraries (not tests) where null or the default is being passed as the serializer options, it's a bug to be fixed. The user of the eventual public API must always be able to supply a JSO.
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.
Updated SamplingChatClient to accept and store serializer options instead of passing null, and updated AsSamplingChatClient to accept optional serializer options parameter in commit 46425b2. All library code now properly propagates user-supplied JSO through the call chain.
- Added optional JsonSerializerOptions parameter to ToAIContent(ContentBlock) and ToAIContent(ResourceContents) - Added optional JsonSerializerOptions parameter to ToAIContents and ToChatMessage(PromptMessage) - Updated CreateSamplingHandler to pass serializer options to ToChatClientArguments - Updated ToChatClientArguments to accept and use serializer options when calling ToAIContent - Updated AsSamplingChatClient to accept optional serializer options - Updated SamplingChatClient to store and use serializer options instead of passing null - Updated McpServer.SampleAsync to pass serializer options to ToAIContent - Updated McpClientTool to pass JsonSerializerOptions to ToAIContent calls - Updated test to use lambda instead of method group for AsSamplingChatClient - Updated XML documentation references to reflect new method signatures - All tests passing on .NET 8, 9, and 10 This ensures users can always supply custom JsonSerializerOptions for serialization operations throughout the library. Co-authored-by: stephentoub <[email protected]>
Fix ToJsonObject serialization failure with anonymous types
Fixes a regression in 0.5.0-preview.1 where
AIContent.AdditionalPropertiescontaining anonymous types (e.g., from Semantic Kernel sampling) would fail withNotSupportedExceptionduring serialization.Changes Made
Fixed serialization of anonymous types: Changed
ToJsonObjectto useMcpJsonUtilities.DefaultOptions.GetTypeInfo(typeof(IReadOnlyDictionary<string, object?>))instead of strongly-typedJsonContext.Default.IReadOnlyDictionaryStringObject, allowing the resolver chain to handle user-defined types through reflection when available.Added optional JsonSerializerOptions parameters throughout the SDK: Added optional
JsonSerializerOptionsparameter to all public APIs that serialize or deserialize user-provided objects, enabling users to pass custom serialization options (converters, naming policies, type resolvers):ToContentBlock- for convertingAIContentto MCP protocol typesToAIContent- for converting protocol types toAIContentToAIContents- for converting multiple content blocksToChatMessage- for converting protocol messages to chat messagesCreateSamplingHandler- for creating sampling handlers with custom serializationAsSamplingChatClient- for creating sampling chat clientsMcpServer.SampleAsync- for sampling operations with custom serializationToJsonObject(internal, required parameter) - for converting dictionaries toJsonObjectComplete serialization chain coverage: Updated all serialization and deserialization operations throughout the library to accept and properly pass through user-supplied
JsonSerializerOptions, ensuring no location in the library hard-codesMcpJsonUtilities.DefaultOptionswhen serializing arbitrary user-provided objects.Comprehensive testing: Added tests in
AIContentExtensionsTests.csandMcpClientToolTests.cscovering:AdditionalProperties(skipped when reflection is disabled)JsonSerializerOptionspropagationCallToolAsyncwith anonymous type argumentsToChatMessagewith anonymous types inContentBlock.MetaBehavior
Anonymous types in
AdditionalPropertieswork when reflection-based serialization is enabled:Named user-defined types work universally with source generation:
[JsonSerializable]attributesUsage Examples
Users can now pass custom serialization options to control serialization behavior at any public API entry point, and those options will be consistently used throughout the entire call chain:
For named types with source generation:
API Design
JsonSerializerOptions?parameters with default values for backward compatibilityJsonSerializerOptionsparameters to ensure explicit passing throughout the codebaseTesting
CallToolAsyncarguments andToChatMessageserializationThe fix enables comprehensive control over serialization throughout the MCP SDK while maintaining backward compatibility and proper handling of both anonymous and named user-defined types. Users can now supply custom
JsonSerializerOptionsat any public API entry point to control how their objects are serialized and deserialized throughout the entire SDK.Original prompt
This section details on the original issue you should resolve
<issue_title>ToJsonObject fails when dictionary values contain anonymous types (sampling pipeline regression in 0.5.0-preview.1)</issue_title>
<issue_description>Describe the bug
When using Semantic Kernel sampling via MCP, the internal payload passed from Semantic Kernel to MCP contains a dictionary whose values may include anonymous types (objects generated by Semantic Kernel and not controllable by the application).
In ModelContextProtocol 0.5.0-preview.1, calling
ModelContextProtocol.AIContentExtensions.ToJsonObject(IReadOnlyDictionary<string, object?> properties)
throws a System.NotSupportedException when any dictionary value is an anonymous type.
This previously worked in 0.4.0-preview.3 (stdio transport only) but now fails in 0.5.0-preview.1, including stdio, indicating that stdio and web transports now share the same serialization pipeline.
To Reproduce
Use Semantic Kernel with MCP sampling (AddSamplingChatClient)
Trigger a prompt invocation where Semantic Kernel generates arguments/metadata internally
Ensure the dictionary passed into
ModelContextProtocol.AIContentExtensions.ToJsonObject(...)
contains any anonymous type as a value (structure does not matter)
Run with ModelContextProtocol 0.5.0-preview.1
Observe serialization failure
Minimal conceptual example:
var dict = new Dictionary<string, object?>
{
["data"] = new { X = 1.0, Y = 2.0 }
};
AIContentExtensions.ToJsonObject(dict);
Expected behavior
ToJsonObject should be able to serialize dictionaries whose values are anonymous types, producing a valid JsonObject / JsonNode.
At minimum, anonymous types should be handled as plain object graphs rather than requiring source-generated JsonTypeInfo, which cannot be provided for anonymous types.
This is especially important because:
These anonymous types are generated internally by Semantic Kernel
Application developers cannot intercept or replace them
Logs
Additional context
In 0.4.0-preview.3:
stdio transport worked
web transport failed (likely due to different serialization pipelines)
In 0.5.0-preview.1:
Both stdio and web transports fail
Indicates a unified pipeline that now enforces the same resolver behavior
Request / suggestion:
Enhance ModelContextProtocol.AIContentExtensions.ToJsonObject to support
dictionary values containing anonymous types, either by:
Falling back to a non–source-generated resolver for such values, or
Detecting anonymous types and serializing them via a compatible JsonSerializerOptions
This would restore compatibility with Semantic Kernel sampling scenarios
where developers cannot control the generated object shapes.</issue_description>
Comments on the Issue (you are @copilot in this section)
@eiriktsarpalis Have you turned on `PublishAOT` in your application? Unfortunately, anonymous types cannot be (and probably never will be) supported by source generators. You could either turn off AOT or replace your anonymous types with equivalent named types (it should be straightforward nowadays with record types). @stephentoub @eiriktsarpalis, this fails without PublishAOT. Here's a minimal repro: ```csharp using Microsoft.Extensions.AI; using ModelContextProtocol;AIContent c = new()
{
AdditionalProperties = new()
{
["data"] = new { X = 1.0, Y = 2.0 }
}
};
c.ToContentBlock();