diff --git a/.agents/skills/json-serialization/SKILL.md b/.agents/skills/json-serialization/SKILL.md index 9242a90..7f76cf8 100644 --- a/.agents/skills/json-serialization/SKILL.md +++ b/.agents/skills/json-serialization/SKILL.md @@ -21,6 +21,10 @@ Tool arguments, tool results, resources and prompt attachments all use one set o Results are trees. A review inside a movie carries the movie's id and title, not the movie, so there's no cycle handling. +Tool schemas are generated from the C# types, and a nullable field comes out as `"type": ["string", "null"]`. That's valid JSON Schema, but clients that map schemas onto a single-type dialect, such as Gemini's function declarations, can reject it or drop its constraints. `PortableSchemaFilter` rewrites each one to an `anyOf` of single types when tools are listed, keeping `description`, `title` and `default` on the outer node. It runs for every tool, so a new tool needs nothing extra. `ListTools_ShouldWrite_PortableSchemas` fails if a type array gets through. + +The filter is a workaround for those clients, and nothing else needs it. Delete it, with that test, once they accept type arrays. To check, take it out and confirm the Inspector shows no schema portability warnings on any tool. + ## Steps 1. Add every new result model, and the `ResultPage` you return, to `AppJsonSerializerContext` with `[JsonSerializable(typeof(...))]`. @@ -47,6 +51,7 @@ options.MakeReadOnly(); - Add `ReferenceHandler.IgnoreCycles` or `Preserve` to hide a cycle. Change the result shape. - List Application entities in the context unless a resource returns one directly. - Use Newtonsoft.Json. +- Hand-write a tool schema to get around a portability warning. Change `PortableSchemaFilter`. ## Check diff --git a/src/Presentation/Extensions/WebApplicationBuilderExtensions.cs b/src/Presentation/Extensions/WebApplicationBuilderExtensions.cs index ceeca86..0da07f8 100644 --- a/src/Presentation/Extensions/WebApplicationBuilderExtensions.cs +++ b/src/Presentation/Extensions/WebApplicationBuilderExtensions.cs @@ -192,6 +192,7 @@ so pass nextOffset back as the offset to continue rather than raising the limit. .WithCompleteHandler(CompletionHandler.Handle) .WithRequestFilters(filters => { + filters.AddListToolsFilter(PortableSchemaFilter.Filter); filters.AddCallToolFilter(ToolErrorFilter.Filter); filters.AddReadResourceFilter(ProtocolErrorFilter.Filter); filters.AddGetPromptFilter(ProtocolErrorFilter.Filter); diff --git a/src/Presentation/Serialization/PortableSchemaFilter.cs b/src/Presentation/Serialization/PortableSchemaFilter.cs new file mode 100644 index 0000000..98cf4b6 --- /dev/null +++ b/src/Presentation/Serialization/PortableSchemaFilter.cs @@ -0,0 +1,95 @@ +namespace CleanModelContextProtocol.Presentation.Serialization; + +using System.Text.Json; +using System.Text.Json.Nodes; +using ModelContextProtocol.Protocol; +using ModelContextProtocol.Server; + +/// +/// Rewrites nullable fields in tool schemas from "type": ["string", "null"] to an anyOf of single types. +/// Both are valid JSON Schema, but clients that map schemas onto a single-type dialect, such as Gemini's function +/// declarations, can reject the first form or drop its constraints. +/// +/// +/// This is a workaround for those single-type dialects, and nothing else needs it. Remove it once they accept JSON +/// Schema type arrays. +/// +public static class PortableSchemaFilter +{ + private static readonly HashSet OuterKeywords = ["default", "description", "title"]; + + public static McpRequestHandler Filter(McpRequestHandler next) + { + return async (request, cancellationToken) => + { + var result = await next(request, cancellationToken); + + foreach (var tool in result.Tools) + { + tool.InputSchema = ToPortable(tool.InputSchema); + + if (tool.OutputSchema is { } outputSchema) + { + tool.OutputSchema = ToPortable(outputSchema); + } + } + + return result; + }; + } + + public static JsonElement ToPortable(JsonElement schema) + { + var node = JsonNode.Parse(schema.GetRawText()); + + Rewrite(node); + + using var document = JsonDocument.Parse(node?.ToJsonString() ?? "null"); + + return document.RootElement.Clone(); + } + + private static void Rewrite(JsonNode? node) + { + if (node is JsonArray array) + { + foreach (var item in array) + { + Rewrite(item); + } + } + + if (node is JsonObject schema) + { + foreach (var property in schema) + { + Rewrite(property.Value); + } + + if (schema["type"] is JsonArray types && types.Count == 2 && types.Any(IsNull)) + { + SplitNullable(schema, types.First(t => !IsNull(t))!.GetValue()); + } + } + } + + private static void SplitNullable(JsonObject schema, string type) + { + var branch = new JsonObject { ["type"] = type }; + + _ = schema.Remove("type"); + + foreach (var name in schema.Select(p => p.Key).Where(k => !OuterKeywords.Contains(k)).ToList()) + { + _ = schema.Remove(name, out var value); + branch[name] = value; + } + + schema["anyOf"] = new JsonArray(branch, new JsonObject { ["type"] = "null" }); + } + + private static bool IsNull(JsonNode? type) + { + return type?.GetValue() == "null"; + } +} diff --git a/tests/Presentation.Tests.Integration/Tools/ToolListTests.cs b/tests/Presentation.Tests.Integration/Tools/ToolListTests.cs index 1c86ba8..e052529 100644 --- a/tests/Presentation.Tests.Integration/Tools/ToolListTests.cs +++ b/tests/Presentation.Tests.Integration/Tools/ToolListTests.cs @@ -1,5 +1,6 @@ namespace CleanModelContextProtocol.Presentation.Tests.Integration.Tools; +using System.Text.Json; using Shouldly; using Xunit; @@ -54,6 +55,20 @@ public async Task ListTools_ShouldDescribe_Tool(string name, string title, bool } } + [Fact] + public async Task ListTools_ShouldWrite_PortableSchemas() + { + // Act + var tools = await this.Client.ListToolsAsync(); + + // Assert + foreach (var tool in tools.Select(t => t.ProtocolTool)) + { + HasTypeArray(tool.InputSchema).ShouldBeFalse(tool.Name); + HasTypeArray(tool.OutputSchema.ShouldNotBeNull()).ShouldBeFalse(tool.Name); + } + } + [Fact] public async Task ServerInfo_ShouldDescribe_Server() { @@ -66,4 +81,15 @@ public async Task ServerInfo_ShouldDescribe_Server() await Task.CompletedTask; } + + private static bool HasTypeArray(JsonElement schema) + { + if (schema.ValueKind == JsonValueKind.Array) + { + return schema.EnumerateArray().Any(HasTypeArray); + } + + return schema.ValueKind == JsonValueKind.Object && schema.EnumerateObject().Any(p => + (p.Name == "type" && p.Value.ValueKind == JsonValueKind.Array) || HasTypeArray(p.Value)); + } } diff --git a/tests/Presentation.Tests.Unit/Serialization/PortableSchemaFilterTests.cs b/tests/Presentation.Tests.Unit/Serialization/PortableSchemaFilterTests.cs new file mode 100644 index 0000000..096d5ee --- /dev/null +++ b/tests/Presentation.Tests.Unit/Serialization/PortableSchemaFilterTests.cs @@ -0,0 +1,105 @@ +namespace CleanModelContextProtocol.Presentation.Tests.Unit.Serialization; + +using System.Text.Json; +using ModelContextProtocol.Protocol; +using Presentation.Serialization; +using Shouldly; +using Xunit; + +public class PortableSchemaFilterTests +{ + [Fact] + public void ToPortable_ShouldSplit_NullableString() + { + // Arrange + var schema = Parse(/*lang=json,strict*/ """{"description":"An id.","type":["string","null"],"format":"uuid","default":null}"""); + + // Act + var result = PortableSchemaFilter.ToPortable(schema); + + // Assert + result.GetProperty("description").GetString().ShouldBe("An id."); + result.GetProperty("default").ValueKind.ShouldBe(JsonValueKind.Null); + result.TryGetProperty("type", out _).ShouldBeFalse(); + result.TryGetProperty("format", out _).ShouldBeFalse(); + + var anyOf = result.GetProperty("anyOf"); + + anyOf[0].GetProperty("type").GetString().ShouldBe("string"); + anyOf[0].GetProperty("format").GetString().ShouldBe("uuid"); + anyOf[1].GetProperty("type").GetString().ShouldBe("null"); + } + + [Fact] + public void ToPortable_ShouldSplit_NullableNumber() + { + // Arrange + var schema = Parse(/*lang=json,strict*/ """{"type":["number","null"]}"""); + + // Act + var result = PortableSchemaFilter.ToPortable(schema); + + // Assert + result.GetProperty("anyOf")[0].GetProperty("type").GetString().ShouldBe("number"); + } + + [Fact] + public void ToPortable_ShouldSplit_NullableArray_AndItsItems() + { + // Arrange + var schema = Parse(/*lang=json,strict*/ """ + {"type":"object","properties":{"reviews":{"type":["array","null"],"items":{"type":"object","properties":{"title":{"type":["string","null"]}}}}}} + """); + + // Act + var result = PortableSchemaFilter.ToPortable(schema); + + // Assert + var branch = result.GetProperty("properties").GetProperty("reviews").GetProperty("anyOf")[0]; + + branch.GetProperty("type").GetString().ShouldBe("array"); + + var title = branch.GetProperty("items").GetProperty("properties").GetProperty("title"); + + title.GetProperty("anyOf")[0].GetProperty("type").GetString().ShouldBe("string"); + } + + [Fact] + public void ToPortable_ShouldLeave_NonNullableFieldUnchanged() + { + // Arrange + var schema = Parse(/*lang=json,strict*/ """{"type":"object","properties":{"stars":{"description":"Stars.","type":"integer"}},"required":["stars"]}"""); + + // Act + var result = PortableSchemaFilter.ToPortable(schema); + + // Assert + result.GetRawText().ShouldBe(schema.GetRawText()); + } + + [Fact] + public async Task Filter_ShouldRewrite_InputAndOutputSchemas() + { + // Arrange + var nullable = Parse(/*lang=json,strict*/ """{"type":"object","properties":{"id":{"type":["string","null"]}}}"""); + var withOutput = new Tool { Name = "with_output", InputSchema = nullable, OutputSchema = nullable }; + var withoutOutput = new Tool { Name = "without_output", InputSchema = nullable }; + var handler = PortableSchemaFilter.Filter((request, cancellationToken) => + ValueTask.FromResult(new ListToolsResult { Tools = [withOutput, withoutOutput] })); + + // Act + var result = await handler(null!, CancellationToken.None); + + // Assert + result.Tools[0].InputSchema.GetRawText().ShouldNotContain("[\"string\",\"null\"]"); + result.Tools[0].OutputSchema.ShouldNotBeNull().GetRawText().ShouldNotContain("[\"string\",\"null\"]"); + result.Tools[1].OutputSchema.ShouldBeNull(); + } + + private static JsonElement Parse(string json) + { + using var document = JsonDocument.Parse(json); + + return document.RootElement.Clone(); + } +}