From 9688b5c20142cdfcb7af58135f5e4b0a0ff9d860 Mon Sep 17 00:00:00 2001 From: Stephen Walsh <868017+stphnwlsh@users.noreply.github.com> Date: Fri, 6 Nov 2026 09:12:26 -0600 Subject: [PATCH 1/3] Write nullable tool schema fields as anyOf so single-type clients accept them --- .agents/skills/json-serialization/SKILL.md | 3 + .../WebApplicationBuilderExtensions.cs | 1 + .../Serialization/PortableSchemaFilter.cs | 91 +++++++++++++++ .../Tools/ToolListTests.cs | 26 +++++ .../PortableSchemaFilterTests.cs | 105 ++++++++++++++++++ 5 files changed, 226 insertions(+) create mode 100644 src/Presentation/Serialization/PortableSchemaFilter.cs create mode 100644 tests/Presentation.Tests.Unit/Serialization/PortableSchemaFilterTests.cs diff --git a/.agents/skills/json-serialization/SKILL.md b/.agents/skills/json-serialization/SKILL.md index d6e0cdd..4ff62e9 100644 --- a/.agents/skills/json-serialization/SKILL.md +++ b/.agents/skills/json-serialization/SKILL.md @@ -21,6 +21,8 @@ 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. + ## Steps 1. Add every new result model, and the `ResultPage` you return, to `AppJsonSerializerContext` with `[JsonSerializable(typeof(...))]`. @@ -47,6 +49,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..605eedf --- /dev/null +++ b/src/Presentation/Serialization/PortableSchemaFilter.cs @@ -0,0 +1,91 @@ +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. +/// +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(); + } +} From b61a4dfc577d355dd06c12d9e05dc40473a3ba28 Mon Sep 17 00:00:00 2001 From: Stephen Walsh <868017+stphnwlsh@users.noreply.github.com> Date: Fri, 6 Nov 2026 09:21:06 -0600 Subject: [PATCH 2/3] Link each movie, author and review a tool returns to its resource --- .agents/skills/api-conventions/SKILL.md | 5 + .../WebApplicationBuilderExtensions.cs | 2 + src/Presentation/Tools/ResourceLinkFilter.cs | 60 ++++++++ .../Tools/ResourceLinkTests.cs | 136 ++++++++++++++++++ 4 files changed, 203 insertions(+) create mode 100644 src/Presentation/Tools/ResourceLinkFilter.cs create mode 100644 tests/Presentation.Tests.Integration/Tools/ResourceLinkTests.cs diff --git a/.agents/skills/api-conventions/SKILL.md b/.agents/skills/api-conventions/SKILL.md index f3fe170..7c427db 100644 --- a/.agents/skills/api-conventions/SKILL.md +++ b/.agents/skills/api-conventions/SKILL.md @@ -42,6 +42,10 @@ Anything else is unexpected. `ToolErrorFilter` logs it as `Error` and returns `E 5. When a tool, resource or prompt is added, renamed or changes behaviour, update `ServerInstructions` in the same change. 6. Test the annotations in `ToolListTests` and each error message in the tool's integration tests (`test-design`). +## Resource links + +Every tool that returns a movie, author or review also returns a `resource_link` block for it, as well as its structured content. The link's URI is the matching resource template, its name is the item's title or name, and its mime type is `application/json`. `ResourceLinkFilter` adds them, because a tool that returns a typed result has no way to add a content block itself. It takes the scheme from the start of the tool's name, so a new tool only has to follow the `movies_get` naming and return an `id`. + ## Example ```csharp @@ -59,6 +63,7 @@ NotFoundException => $"Not Found: {exception.Message} Check the ids: movies_sear - Throw from a tool to report a problem the model can fix. Let the filter return an error result. - Return an unbounded list. +- Build a `CallToolResult` by hand in a tool to add a link. `ResourceLinkFilter` does that for every tool. - Name tools after Application classes (`GetMovieByIdQuery`) or use camelCase or dashes. - Leave `ServerInstructions` describing tools that changed. - Put internal detail (stack traces, SQL) in an error message. diff --git a/src/Presentation/Extensions/WebApplicationBuilderExtensions.cs b/src/Presentation/Extensions/WebApplicationBuilderExtensions.cs index 0da07f8..65e167b 100644 --- a/src/Presentation/Extensions/WebApplicationBuilderExtensions.cs +++ b/src/Presentation/Extensions/WebApplicationBuilderExtensions.cs @@ -27,6 +27,7 @@ namespace CleanModelContextProtocol.Presentation.Extensions; using Presentation.Errors; using Presentation.Serialization; using Presentation.Telemetry; +using Presentation.Tools; using Serilog; [ExcludeFromCodeCoverage] @@ -194,6 +195,7 @@ so pass nextOffset back as the offset to continue rather than raising the limit. { filters.AddListToolsFilter(PortableSchemaFilter.Filter); filters.AddCallToolFilter(ToolErrorFilter.Filter); + filters.AddCallToolFilter(ResourceLinkFilter.Filter); filters.AddReadResourceFilter(ProtocolErrorFilter.Filter); filters.AddGetPromptFilter(ProtocolErrorFilter.Filter); }); diff --git a/src/Presentation/Tools/ResourceLinkFilter.cs b/src/Presentation/Tools/ResourceLinkFilter.cs new file mode 100644 index 0000000..f3882a0 --- /dev/null +++ b/src/Presentation/Tools/ResourceLinkFilter.cs @@ -0,0 +1,60 @@ +namespace CleanModelContextProtocol.Presentation.Tools; + +using System.Text.Json; +using ModelContextProtocol.Protocol; +using ModelContextProtocol.Server; + +/// +/// Adds a resource link for each movie, author or review a tool returns, so a client can open it as a resource. A tool +/// returns a typed result that the SDK turns into structured content, which leaves no room for a link there. The +/// resource's scheme is the start of the tool's name: movies_get links movies://{id}. +/// +public static class ResourceLinkFilter +{ + private const string MimeType = "application/json"; + + public static McpRequestHandler Filter(McpRequestHandler next) + { + return async (request, cancellationToken) => + { + var result = await next(request, cancellationToken); + + if (result.IsError != true && result.StructuredContent is { } content) + { + var scheme = request.Params!.Name.Split('_')[0]; + + foreach (var link in GetItems(content).Select(i => ToLink(scheme, i)).OfType()) + { + result.Content.Add(link); + } + } + + return result; + }; + } + + private static List GetItems(JsonElement content) + { + return content.TryGetProperty("items", out var items) ? [.. items.EnumerateArray()] : [content]; + } + + private static ResourceLinkBlock? ToLink(string scheme, JsonElement item) + { + // A deleted review has no id, because there's nothing left to open. + return GetText(item, "id") is { } id + ? new ResourceLinkBlock { Uri = $"{scheme}://{id}", Name = GetName(item), MimeType = MimeType } + : null; + } + + private static string GetName(JsonElement item) + { + return GetText(item, "title") + ?? GetText(item, "name") + ?? $"Review of {GetText(item, "movieTitle")} by {GetText(item, "authorName")}"; + } + + private static string? GetText(JsonElement item, string name) + { + return item.TryGetProperty(name, out var value) ? value.GetString() : null; + } +} diff --git a/tests/Presentation.Tests.Integration/Tools/ResourceLinkTests.cs b/tests/Presentation.Tests.Integration/Tools/ResourceLinkTests.cs new file mode 100644 index 0000000..5ae7d04 --- /dev/null +++ b/tests/Presentation.Tests.Integration/Tools/ResourceLinkTests.cs @@ -0,0 +1,136 @@ +namespace CleanModelContextProtocol.Presentation.Tests.Integration.Tools; + +using ModelContextProtocol.Protocol; +using Presentation.Models; +using Shouldly; +using Xunit; + +public class ResourceLinkTests : BaseToolTests +{ + [Fact] + public async Task GetMovie_ShouldLink_Movie() + { + // Arrange + var movie = (await this.CallAsync>("movies_search")).Items[0]; + + // Act + var links = await this.GetLinksAsync("movies_get", new Dictionary { ["movieId"] = movie.Id }); + + // Assert + var link = links.ShouldHaveSingleItem(); + + link.Uri.ShouldBe($"movies://{movie.Id}"); + link.Name.ShouldBe(movie.Title); + link.MimeType.ShouldBe("application/json"); + await this.ShouldResolveAsync(links); + } + + [Fact] + public async Task SearchMovies_ShouldLink_EveryMovie() + { + // Act + var links = await this.GetLinksAsync("movies_search"); + + // Assert + links.Count.ShouldBe(10); + links.ShouldAllBe(l => l.Uri.StartsWith("movies://", StringComparison.Ordinal)); + await this.ShouldResolveAsync(links); + } + + [Fact] + public async Task SearchAuthors_ShouldLink_EveryAuthor() + { + // Act + var links = await this.GetLinksAsync("authors_search"); + + // Assert + links.Count.ShouldBe(10); + links.ShouldAllBe(l => l.Uri.StartsWith("authors://", StringComparison.Ordinal)); + await this.ShouldResolveAsync(links); + } + + [Fact] + public async Task ListReviews_ShouldLink_EveryReview() + { + // Act + var links = await this.GetLinksAsync("reviews_list"); + + // Assert + links.Count.ShouldBe(10); + links.ShouldAllBe(l => l.Uri.StartsWith("reviews://", StringComparison.Ordinal)); + links.ShouldAllBe(l => l.Name.StartsWith("Review of ", StringComparison.Ordinal)); + await this.ShouldResolveAsync(links); + } + + [Fact] + public async Task WriteReview_ShouldLink_Review() + { + // Arrange + var movie = (await this.CallAsync>("movies_search")).Items[0]; + var author = (await this.CallAsync>("authors_search")).Items[0]; + + // Act + var links = await this.GetLinksAsync("reviews_write", + new Dictionary { ["movieId"] = movie.Id, ["authorId"] = author.Id, ["stars"] = 5 }); + + // Assert + var link = links.ShouldHaveSingleItem(); + + link.Name.ShouldBe($"Review of {movie.Title} by {author.Name}"); + await this.ShouldResolveAsync(links); + } + + [Fact] + public async Task DeleteReview_ShouldNotLink_DeletedReview() + { + // Arrange + var review = (await this.CallAsync>("reviews_list")).Items[0]; + + // Act + var links = await this.GetLinksAsync("reviews_delete", new Dictionary { ["reviewId"] = review.Id }); + + // Assert + links.ShouldBeEmpty(); + } + + [Fact] + public async Task SearchMovies_ShouldKeep_StructuredContentAndText() + { + // Act + var result = await this.Client.CallToolAsync("movies_search"); + + // Assert + result.StructuredContent.ShouldNotBeNull().GetProperty("items").GetArrayLength().ShouldBe(10); + _ = result.Content[0].ShouldBeOfType(); + } + + [Fact] + public async Task GetMovie_NotFound_ShouldNotLink() + { + // Act + var result = await this.Client.CallToolAsync("movies_get", new Dictionary { ["movieId"] = Guid.NewGuid() }); + + // Assert + result.IsError.ShouldBe(true); + result.Content.OfType().ShouldBeEmpty(); + } + + private async Task> GetLinksAsync(string name, Dictionary? arguments = null) + { + var result = await this.Client.CallToolAsync(name, arguments); + + result.IsError.ShouldNotBe(true); + + return [.. result.Content.OfType()]; + } + + private async Task ShouldResolveAsync(List links) + { + foreach (var link in links) + { + var resource = await this.Client.ReadResourceAsync(link.Uri); + + resource.Contents.ShouldNotBeEmpty(); + } + } +} From 7e6b0c23c90426a5491a0e9de6c9813433aa9916 Mon Sep 17 00:00:00 2001 From: Stephen Walsh <868017+stphnwlsh@users.noreply.github.com> Date: Fri, 6 Nov 2026 09:22:56 -0600 Subject: [PATCH 3/3] Say the schema filter is a workaround and when to remove it --- .agents/skills/json-serialization/SKILL.md | 2 ++ src/Presentation/Serialization/PortableSchemaFilter.cs | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/.agents/skills/json-serialization/SKILL.md b/.agents/skills/json-serialization/SKILL.md index 4ff62e9..7b43b24 100644 --- a/.agents/skills/json-serialization/SKILL.md +++ b/.agents/skills/json-serialization/SKILL.md @@ -23,6 +23,8 @@ Results are trees. A review inside a movie carries the movie's id and title, not 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(...))]`. diff --git a/src/Presentation/Serialization/PortableSchemaFilter.cs b/src/Presentation/Serialization/PortableSchemaFilter.cs index 605eedf..98cf4b6 100644 --- a/src/Presentation/Serialization/PortableSchemaFilter.cs +++ b/src/Presentation/Serialization/PortableSchemaFilter.cs @@ -10,6 +10,10 @@ namespace CleanModelContextProtocol.Presentation.Serialization; /// 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"];