- 
                Notifications
    You must be signed in to change notification settings 
- Fork 555
Add configurable JsonSerializerOptions and SchemaCreateOptions to McpServerOptions #925
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
…lder extensions Co-authored-by: eiriktsarpalis <[email protected]>
Co-authored-by: eiriktsarpalis <[email protected]>
Co-authored-by: eiriktsarpalis <[email protected]>
        
          
                tests/ModelContextProtocol.Tests/Configuration/McpServerJsonSerializerOptionsTests.cs
          
            Show resolved
            Hide resolved
        
      …cking schema naming policy Co-authored-by: eiriktsarpalis <[email protected]>
| Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
 If you need me to access, download, or install something from one of these locations, you can either: 
 | 
Co-authored-by: eiriktsarpalis <[email protected]>
| Assert.Equal(JsonNumberHandling.AllowNamedFloatingPointLiterals, options.JsonSerializerOptions.NumberHandling); | ||
| } | ||
|  | ||
| [Fact] | 
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 please add corresponding tests for prompts and resources.
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 corresponding tests for prompts and resources. Both tests verify that server-wide JsonSerializerOptions with snake_case naming policy are applied correctly. See commit dfcd1ec.
…ptions Co-authored-by: eiriktsarpalis <[email protected]>
| Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
 If you need me to access, download, or install something from one of these locations, you can either: 
 | 
| /// and <see cref="double.NaN"/>. | ||
| /// </para> | ||
| /// </remarks> | ||
| public JsonSerializerOptions? JsonSerializerOptions { get; set; } | 
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 let's see if we could also address #795 here. Please add a second AIJsonSchemaCreateOptions property that similarly acts as the default for all tools, resources and prompts. Per that issue's description, also add relevant parameters to tool, resource, and prompt methods that currently do not accept an AIJsonSchemaCreateOptions parameter.
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 SchemaCreateOptions property to McpServerOptions in commit e62aae8.
I'm working on updating all WithTools*, WithPrompts*, and WithResources* methods to:
- Add AIJsonSchemaCreateOptions? schemaCreateOptions = nullparameter
- Use the server-wide default when null
- Add proper XML documentation
This is a large change affecting 12+ methods. Would you prefer I complete this in the current PR or should this be split into a follow-up PR to keep the changes more focused?
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 Please do, the new property doesn't do anything unless it can be mapped to the corresponding overloads in the tool methods.
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.
Completed! Added AIJsonSchemaCreateOptions parameter to all 15 WithTools*, WithPrompts*, and WithResources* methods. Each method now:
- Accepts optional schemaCreateOptionsparameter
- Falls back to McpServerOptions.SchemaCreateOptionswhen not provided
- Has proper XML documentation
See commit d56b8e5. The implementation is now complete and addresses both issues #636 and #795.
Co-authored-by: eiriktsarpalis <[email protected]>
| Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
 If you need me to access, download, or install something from one of these locations, you can either: 
 | 
…urces methods Co-authored-by: eiriktsarpalis <[email protected]>
| Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
 If you need me to access, download, or install something from one of these locations, you can either: 
 | 
| @copilot your latest changes are causing a stack overflow in testing. Please investigate. | 
…ing IOptions during tool creation Co-authored-by: eiriktsarpalis <[email protected]>
| 
 Investigated and fixed! The stack overflow was caused by a circular dependency: resolving  Fixed by introducing  Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
 If you need me to access, download, or install something from one of these locations, you can either: 
 | 
Work in Progress: Fixing Stack Overflow Issue
The previous implementation caused a stack overflow because resolving
IOptions<McpServerOptions>from within tool/prompt/resource factories created a circular dependency:IOptions<McpServerOptions>depends onMcpServerOptionsSetupMcpServerOptionsSetupdepends onIEnumerable<McpServerTool>IOptions<McpServerOptions>→ infinite recursionFix Applied:
Created
McpServerDefaultOptionsclass that captures JsonSerializerOptions and SchemaCreateOptions values whenAddMcpServer(configureOptions)is called, avoiding circular dependencies during service resolution.Status:
The stack overflow is fixed but only the first WithTools method is fully updated. All other methods need the same pattern applied.
Original prompt
This section details on the original issue you should resolve
<issue_title>Cannot Configure JSON Serialization for MCPServer</issue_title>
<issue_description>Describe the bug
It seems is not possible to configure JsonSerialisation used by MCPServer
To Reproduce
Steps to reproduce the behavior:
System.ArgumentException: .NET number values such as positive and negative infinity cannot be written as valid JSON. To make it work when using 'JsonSerializer', consider specifying 'JsonNumberHandling.AllowNamedFloatingPointLiterals' (see https://learn.microsoft.com/dotnet/api/system.text.json.serialization.jsonnumberhandling).
Expected behavior
I can configure json serialisation for the MCP server with something like that : builder.Services.AddMcpServer(opt=>opt.JsonSerializerOptions.NumberHandling = JsonNumberHandling.AllowNamedFloatingPointLiterals)
Logs
Server (MCP.Server 1.0.0.0), Client (Visual Studio Code 1.102.1) method 'tools/call' request handler failed.
System.ArgumentException: .NET number values such as positive and negative infinity cannot be written as valid JSON. To make it work when using 'JsonSerializer', consider specifying 'JsonNumberHandling.AllowNamedFloatingPointLiterals' (see https://learn.microsoft.com/dotnet/api/system.text.json.serialization.jsonnumberhandling).
Additional context
I tried these :
builder.Services.ConfigureHttpJsonOptions(options =>
{
options.SerializerOptions.NumberHandling = JsonNumberHandling.AllowNamedFloatingPointLiterals;
});
builder.Services.Configure(options =>
{
options.SerializerOptions.NumberHandling = JsonNumberHandling.AllowNamedFloatingPointLiterals;
});
// Configure System.Text.Json defaults
builder.Services.PostConfigure(options =>
{
options.NumberHandling = JsonNumberHandling.AllowNamedFloatingPointLiterals;
});
builder.Services.AddSingleton(sp =>
{
Console.Error.WriteLine("✅ JsonOptions created with NumberHandling=" +
JsonNumberHandling.AllowNamedFloatingPointLiterals);
return new JsonSerializerOptions { NumberHandling = JsonNumberHandling.AllowNamedFloatingPointLiterals };
});
</issue_description>
<agent_instructions>Per the issue description, add an
AddMcpServeroverload that accepts aJsonSerializerOptionsparameter that becomes the default for all configured tools, resources, etc. WithTools and WithResources calls not specifying aJsonSerializerOptionsparameter should default to that app-wideJsonSerializerOptionswhich would otherwise beMcpJsonUtilities.DefaultOptions.</agent_instructions>Comments on the Issue (you are @copilot in this section)
@eiriktsarpalis You can configure serialization by passing a `JsonSerializerOptions` parameter in the individual `WithTools*` methods. Hope this helps. @eiriktsarpalis It's something we could consider, however the main MCP package cannot be made to depend on the `JsonOptions` type. Instead, I think parameterizing on `AddMcpServer()` seems preferable. @halter73 opinions? @halter73 I definitely don't mind adding a JsonSerializerOptions parameter to AddMcpServer(). I was curious about why we split up the options for each tool/prompt registration method rather than just allowing you to configure JsonSerializerOptions in one place. I figured it was for the increased flexibility, but I wonder if anyone really needs it.Rather than adding it as a direct parameter to the AddMcpServer() method, I'd probably add it as a property to
McpServerOptions. This would allow us to easily default it toJsonOptions.SerializerOptionsbut still let you override it.If we did this, would we continue to take a
JsonSerializerOptionsparameter in theWithTools*/WithPrompts*methods? If so, would we just prefer the more local serializer options?Also, would we want to add the same property to
McpClientOptions? It seems like it could be useful for the same reasons. That way you wouldn't always have to manually pass in options to CallToolAsync, ListToolAsync, GetPromptAsync, etc.</comment_new><comment_new>@eiriktsarpalis
I think it's a remnant from when you would only have one
WithToolsFromAssembly()call requiring serialization configuration. There's still value in being able to ...✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.