-
Notifications
You must be signed in to change notification settings - Fork 553
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?
Changes from 6 commits
958d1fd
2f84c3e
bb8d2a5
f7b9dee
eca0585
542fa91
dfcd1ec
e62aae8
d56b8e5
7507081
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| using Microsoft.Extensions.DependencyInjection; | ||
| using Microsoft.Extensions.Options; | ||
| using ModelContextProtocol.Protocol; | ||
| using ModelContextProtocol.Server; | ||
| using System.Text.Json; | ||
| using System.Text.Json.Serialization; | ||
|
|
||
| namespace ModelContextProtocol.Tests.Configuration; | ||
|
|
||
| public class McpServerJsonSerializerOptionsTests | ||
| { | ||
| [Fact] | ||
| public void McpServerOptions_JsonSerializerOptions_DefaultsToNull() | ||
| { | ||
| // Arrange & Act | ||
| var options = new McpServerOptions(); | ||
|
|
||
| // Assert | ||
| Assert.Null(options.JsonSerializerOptions); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void McpServerOptions_JsonSerializerOptions_CanBeSet() | ||
| { | ||
| // Arrange | ||
| var customOptions = new JsonSerializerOptions | ||
| { | ||
| NumberHandling = JsonNumberHandling.AllowNamedFloatingPointLiterals | ||
| }; | ||
| var options = new McpServerOptions(); | ||
|
|
||
| // Act | ||
| options.JsonSerializerOptions = customOptions; | ||
|
|
||
| // Assert | ||
| Assert.NotNull(options.JsonSerializerOptions); | ||
| Assert.Equal(JsonNumberHandling.AllowNamedFloatingPointLiterals, options.JsonSerializerOptions.NumberHandling); | ||
| } | ||
|
|
||
| [Fact] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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. |
||
| public void WithTools_UsesServerWideOptions_WhenNoExplicitOptionsProvided() | ||
| { | ||
| // Arrange | ||
| var services = new ServiceCollection(); | ||
| var customOptions = new JsonSerializerOptions(McpJsonUtilities.DefaultOptions) | ||
| { | ||
| PropertyNamingPolicy = JsonNamingPolicy.SnakeCaseLower | ||
| }; | ||
|
|
||
| services.Configure<McpServerOptions>(options => | ||
| { | ||
| options.JsonSerializerOptions = customOptions; | ||
| }); | ||
|
|
||
| var builder = services.AddMcpServer(); | ||
|
|
||
| // Act - WithTools should pick up the server-wide options with snake_case naming policy | ||
| builder.WithTools<TestTools>(); | ||
| var serviceProvider = services.BuildServiceProvider(); | ||
|
|
||
| // Assert - Verify the tool schema uses snake_case property naming | ||
| var tools = serviceProvider.GetServices<McpServerTool>().ToList(); | ||
eiriktsarpalis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Assert.Single(tools); | ||
|
|
||
| var tool = tools[0]; | ||
| Assert.Equal("ToolWithParameters", tool.ProtocolTool.Name); | ||
|
|
||
| // Check that the input schema uses snake_case for property names | ||
| var inputSchema = tool.ProtocolTool.InputSchema; | ||
|
|
||
| // The schema should have a "properties" object with snake_case property names | ||
| var propertiesElement = inputSchema.GetProperty("properties"); | ||
| Assert.True(propertiesElement.TryGetProperty("my_parameter", out _), "Schema should have 'my_parameter' property (snake_case)"); | ||
| Assert.False(propertiesElement.TryGetProperty("MyParameter", out _), "Schema should not have 'MyParameter' property (PascalCase)"); | ||
| } | ||
|
|
||
| [McpServerToolType] | ||
| private class TestTools | ||
| { | ||
| [McpServerTool] | ||
| public static string ToolWithParameters(string myParameter) => myParameter; | ||
| } | ||
| } | ||
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
AIJsonSchemaCreateOptionsproperty 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
SchemaCreateOptionsproperty toMcpServerOptionsin commit e62aae8.I'm working on updating all
WithTools*,WithPrompts*, andWithResources*methods to:AIJsonSchemaCreateOptions? schemaCreateOptions = nullparameterThis 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
AIJsonSchemaCreateOptionsparameter to all 15WithTools*,WithPrompts*, andWithResources*methods. Each method now:schemaCreateOptionsparameterMcpServerOptions.SchemaCreateOptionswhen not providedSee commit d56b8e5. The implementation is now complete and addresses both issues #636 and #795.