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/7] 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/7] 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/7] 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"]; From 6488248d865eb8878951992cacbdd53c00d16932 Mon Sep 17 00:00:00 2001 From: Stephen Walsh <868017+stphnwlsh@users.noreply.github.com> Date: Fri, 6 Nov 2026 09:58:06 -0600 Subject: [PATCH 4/7] Suggest ids for resource templates as people type --- .agents/skills/expose-a-use-case/SKILL.md | 2 +- .../Completions/CompletionHandler.cs | 84 ++++++++++++- .../Completions/CompletionTests.cs | 114 +++++++++++++++++- 3 files changed, 195 insertions(+), 5 deletions(-) diff --git a/.agents/skills/expose-a-use-case/SKILL.md b/.agents/skills/expose-a-use-case/SKILL.md index 0ce704a..6a0ff0f 100644 --- a/.agents/skills/expose-a-use-case/SKILL.md +++ b/.agents/skills/expose-a-use-case/SKILL.md @@ -29,7 +29,7 @@ The SDK finds every class in the assembly (`WithToolsFromAssembly`, `WithResourc 4. Write the method with `sender` and `cancellationToken` first, then required inputs, then optional ones with defaults. 5. Document it: `` saying what it does, what it returns and which tool to call first; a `` for every input except `sender` and the token, with allowed values and where to find ids. Put the XML comment above the attributes. 6. Return a result model from `src/Presentation/Models/` (records with `[property: Description]`, because the output schema reads attributes), mapped in `ResultMapper`. Add new models to `AppJsonSerializerContext` (`json-serialization`). -7. A prompt argument that names a movie or author gets suggestions from `Completions/CompletionHandler.cs`; add its name there. +7. A prompt argument that names a movie or author gets suggestions from `Completions/CompletionHandler.cs`; add its name there. So does the id in a resource template, which suggests ids by their first characters or the title or name they belong to. 8. Update the server instructions in `WebApplicationBuilderExtensions` if the model needs to know the new thing exists (`api-conventions`). 9. Test it twice: a unit test calling the static method with a faked `ISender` (`tests/Presentation.Tests.Unit/Tools/`), and an integration test through the MCP client (`presentation-integration-tests`). Add a tool to the list in `ToolListTests`. diff --git a/src/Presentation/Completions/CompletionHandler.cs b/src/Presentation/Completions/CompletionHandler.cs index 0bd0ec0..849c8b8 100644 --- a/src/Presentation/Completions/CompletionHandler.cs +++ b/src/Presentation/Completions/CompletionHandler.cs @@ -7,9 +7,11 @@ namespace CleanModelContextProtocol.Presentation.Completions; using ModelContextProtocol.Server; using AuthorQueries = Application.Authors.Queries; using MovieQueries = Application.Movies.Queries; +using ReviewQueries = Application.Reviews.Queries; /// -/// Suggests movie titles and author names as the user types a prompt argument. +/// Suggests movie titles and author names as the user types a prompt argument, and ids as they type a resource +/// template's id. An id can be found by its first characters, or by the title or name of what it belongs to. /// public static class CompletionHandler { @@ -24,11 +26,21 @@ public static async Task Complete(ISender sender, CompleteReques { var argument = request?.Argument; - if (request?.Ref is not PromptReference || argument is null || argument.Value?.Length > MaxQueryLength) + if (argument is null || argument.Value?.Length > MaxQueryLength) { return Empty(); } + return request?.Ref switch + { + PromptReference => await CompletePromptArgument(sender, argument, cancellationToken), + ResourceTemplateReference => await CompleteResourceId(sender, argument, cancellationToken), + _ => Empty() + }; + } + + private static async Task CompletePromptArgument(ISender sender, Argument argument, CancellationToken cancellationToken) + { switch (argument.Name) { case "movieTitle" or "firstMovieTitle" or "secondMovieTitle": @@ -58,6 +70,74 @@ public static async Task Complete(ISender sender, CompleteReques } } + private static async Task CompleteResourceId(ISender sender, Argument argument, CancellationToken cancellationToken) + { + switch (argument.Name) + { + case "movieId": + { + var page = await sender.Send(new MovieQueries.GetMovies.GetMoviesQuery + { + Query = argument.Value, + Limit = Paging.MaxLimit + }, cancellationToken); + + if (page.Items.Count > 0) + { + return Result([.. page.Items.Select(m => m.Id.ToString())], page.TotalCount); + } + + var movies = await sender.Send(new MovieQueries.GetMovies.GetMoviesQuery { Limit = Paging.MaxLimit }, cancellationToken); + + return IdsStartingWith(movies.Items.Select(m => m.Id), argument.Value); + } + + case "authorId": + { + var page = await sender.Send(new AuthorQueries.GetAuthors.GetAuthorsQuery + { + Query = argument.Value, + Limit = Paging.MaxLimit + }, cancellationToken); + + if (page.Items.Count > 0) + { + return Result([.. page.Items.Select(a => a.Id.ToString())], page.TotalCount); + } + + var authors = await sender.Send(new AuthorQueries.GetAuthors.GetAuthorsQuery { Limit = Paging.MaxLimit }, cancellationToken); + + return IdsStartingWith(authors.Items.Select(a => a.Id), argument.Value); + } + + case "reviewId": + { + var reviews = await sender.Send(new ReviewQueries.GetReviews.GetReviewsQuery { Limit = Paging.MaxLimit }, cancellationToken); + var value = argument.Value ?? string.Empty; + var matches = reviews.Items + .Where(r => r.Id.ToString().StartsWith(value, StringComparison.OrdinalIgnoreCase) + || (r.ReviewedMovie?.Title.Contains(value, StringComparison.OrdinalIgnoreCase) ?? false)) + .Select(r => r.Id.ToString()) + .ToList(); + + return Result(matches, matches.Count); + } + + default: + return Empty(); + } + } + + private static CompleteResult IdsStartingWith(IEnumerable ids, string? value) + { + var matches = ids + .Select(i => i.ToString()) + .Where(i => i.StartsWith(value ?? string.Empty, StringComparison.OrdinalIgnoreCase)) + .ToList(); + + return Result(matches, matches.Count); + } + private static CompleteResult Result(List values, int total) { return new CompleteResult diff --git a/tests/Presentation.Tests.Integration/Completions/CompletionTests.cs b/tests/Presentation.Tests.Integration/Completions/CompletionTests.cs index 8f23eae..9d18ebb 100644 --- a/tests/Presentation.Tests.Integration/Completions/CompletionTests.cs +++ b/tests/Presentation.Tests.Integration/Completions/CompletionTests.cs @@ -63,10 +63,120 @@ public async Task Complete_WithUnsupportedArgument_ShouldReturn_Nothing(string p } [Fact] - public async Task Complete_ResourceTemplate_ShouldReturn_Nothing() + public async Task CompleteMovieId_ShouldMatch_Title() + { + // Arrange + var movie = (await this.CallAsync>("movies_search")).Items[0]; + + // Act + var result = await this.Client.CompleteAsync(new ResourceTemplateReference { Uri = "movies://{movieId}" }, "movieId", movie.Title); + + // Assert + result.Completion.Values.ShouldContain(movie.Id.ToString()); + } + + [Fact] + public async Task CompleteMovieId_ShouldMatch_IdPrefix() + { + // Arrange + var movie = (await this.CallAsync>("movies_search")).Items[0]; + + // Act + var result = await this.Client.CompleteAsync(new ResourceTemplateReference { Uri = "movies://{movieId}" }, "movieId", movie.Id.ToString()[..8]); + + // Assert + result.Completion.Values.ShouldBe([movie.Id.ToString()]); + } + + [Fact] + public async Task CompleteAuthorId_ShouldMatch_Name() + { + // Arrange + var author = (await this.CallAsync>("authors_search")).Items[0]; + + // Act + var result = await this.Client.CompleteAsync(new ResourceTemplateReference { Uri = "authors://{authorId}" }, "authorId", author.Name); + + // Assert + result.Completion.Values.ShouldContain(author.Id.ToString()); + } + + [Fact] + public async Task CompleteAuthorId_ShouldMatch_IdPrefix() + { + // Arrange + var author = (await this.CallAsync>("authors_search")).Items[0]; + + // Act + var result = await this.Client.CompleteAsync(new ResourceTemplateReference { Uri = "authors://{authorId}" }, "authorId", author.Id.ToString()[..8]); + + // Assert + result.Completion.Values.ShouldBe([author.Id.ToString()]); + } + + [Fact] + public async Task CompleteReviewId_ShouldMatch_IdPrefix() + { + // Arrange + var review = (await this.CallAsync>("reviews_list")).Items[0]; + + // Act + var result = await this.Client.CompleteAsync(new ResourceTemplateReference { Uri = "reviews://{reviewId}" }, "reviewId", review.Id.ToString()[..8]); + + // Assert + result.Completion.Values.ShouldContain(review.Id.ToString()); + } + + [Fact] + public async Task CompleteReviewId_ShouldMatch_MovieTitle() + { + // Arrange + var review = (await this.CallAsync>("reviews_list")).Items[0]; + + // Act + var result = await this.Client.CompleteAsync(new ResourceTemplateReference { Uri = "reviews://{reviewId}" }, "reviewId", review.MovieTitle!); + + // Assert + result.Completion.Values.ShouldContain(review.Id.ToString()); + } + + [Theory] + [InlineData("movies://{movieId}", "movieId")] + [InlineData("authors://{authorId}", "authorId")] + [InlineData("reviews://{reviewId}", "reviewId")] + public async Task CompleteResourceId_WithEmptyValue_ShouldReturn_Ids(string uri, string argument) + { + // Act + var result = await this.Client.CompleteAsync(new ResourceTemplateReference { Uri = uri }, argument, string.Empty); + + // Assert + result.Completion.Values.ShouldNotBeEmpty(); + + foreach (var value in result.Completion.Values) + { + Guid.TryParse(value, out _).ShouldBeTrue(); + } + } + + [Theory] + [InlineData("movies://{movieId}", "movieId")] + [InlineData("authors://{authorId}", "authorId")] + [InlineData("reviews://{reviewId}", "reviewId")] + public async Task CompleteResourceId_WithNoMatch_ShouldReturn_Nothing(string uri, string argument) + { + // Act + var result = await this.Client.CompleteAsync(new ResourceTemplateReference { Uri = uri }, argument, "zzz-no-match"); + + // Assert + result.Completion.Values.ShouldBeEmpty(); + result.Completion.Total.ShouldBe(0); + } + + [Fact] + public async Task CompleteUnknownResourceArgument_ShouldReturn_Nothing() { // Act - var result = await this.Client.CompleteAsync(new ResourceTemplateReference { Uri = "movies://{movieId}" }, "movieId", "a"); + var result = await this.Client.CompleteAsync(new ResourceTemplateReference { Uri = "movies://{movieId}" }, "unknown", "a"); // Assert result.Completion.Values.ShouldBeEmpty(); From 2ad50ebab23fba967ed49ade841a096dd7957a78 Mon Sep 17 00:00:00 2001 From: Stephen Walsh <868017+stphnwlsh@users.noreply.github.com> Date: Fri, 6 Nov 2026 11:25:48 -0600 Subject: [PATCH 5/7] Give every tool, prompt and resource an icon --- .agents/skills/api-conventions/SKILL.md | 4 ++++ src/Presentation/Prompts/MoviePrompts.cs | 7 ++++--- src/Presentation/Resources/AuthorResources.cs | 3 ++- src/Presentation/Resources/MovieResources.cs | 3 ++- src/Presentation/Resources/ReviewResources.cs | 3 ++- src/Presentation/Resources/VersionResources.cs | 3 ++- src/Presentation/Tools/AuthorTools.cs | 2 +- src/Presentation/Tools/ItemIcons.cs | 15 +++++++++++++++ src/Presentation/Tools/MovieTools.cs | 4 ++-- src/Presentation/Tools/ReviewTools.cs | 6 +++--- .../Tools/ToolListTests.cs | 16 ++++++++++++++++ 11 files changed, 53 insertions(+), 13 deletions(-) create mode 100644 src/Presentation/Tools/ItemIcons.cs diff --git a/.agents/skills/api-conventions/SKILL.md b/.agents/skills/api-conventions/SKILL.md index 7c427db..e876f5b 100644 --- a/.agents/skills/api-conventions/SKILL.md +++ b/.agents/skills/api-conventions/SKILL.md @@ -46,6 +46,10 @@ Anything else is unexpected. `ToolErrorFilter` logs it as `Error` and returns `E 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`. +## Icons + +Every tool, prompt and resource sets `IconSource` to a constant in `ItemIcons`, which points at a pinned brand release like the server's icon. Use the icon for what the item is about, not what it does. A new item needs one, or `EveryItem_ShouldHave_AnIcon` fails. + ## Example ```csharp diff --git a/src/Presentation/Prompts/MoviePrompts.cs b/src/Presentation/Prompts/MoviePrompts.cs index 696f8cc..760f1a3 100644 --- a/src/Presentation/Prompts/MoviePrompts.cs +++ b/src/Presentation/Prompts/MoviePrompts.cs @@ -5,6 +5,7 @@ namespace CleanModelContextProtocol.Presentation.Prompts; using ModelContextProtocol.Server; using Presentation.Models; using Presentation.Serialization; +using Presentation.Tools; using Queries = Application.Movies.Queries; [McpServerPromptType] @@ -12,7 +13,7 @@ public static partial class MoviePrompts { /// Summarise how reviewers have received a movie: its average stars, how the ratings spread, and who liked it most and least. /// The movie's title, or part of it. - [McpServerPrompt(Name = "summarise_movie_reception", Title = "Summarise a movie's reception")] + [McpServerPrompt(Name = "summarise_movie_reception", Title = "Summarise a movie's reception", IconSource = ItemIcons.Movie)] public static async partial Task SummariseMovieReception( ISender sender, string movieTitle, @@ -44,7 +45,7 @@ public static async partial Task SummariseMovieReception( /// Compare how reviewers rated two movies, and say which one they preferred and by how much. /// The first movie's title, or part of it. /// The second movie's title, or part of it. - [McpServerPrompt(Name = "compare_movies", Title = "Compare two movies")] + [McpServerPrompt(Name = "compare_movies", Title = "Compare two movies", IconSource = ItemIcons.Movie)] public static async partial Task CompareMovies( ISender sender, string firstMovieTitle, @@ -76,7 +77,7 @@ public static async partial Task CompareMovies( /// Write a review of a movie as one of the authors, checking the movie and author before saving it. /// The movie's title, or part of it. /// The name of the author writing the review. - [McpServerPrompt(Name = "review_movie", Title = "Review a movie")] + [McpServerPrompt(Name = "review_movie", Title = "Review a movie", IconSource = ItemIcons.Review)] public static partial GetPromptResult ReviewMovie( string movieTitle, string authorName) diff --git a/src/Presentation/Resources/AuthorResources.cs b/src/Presentation/Resources/AuthorResources.cs index 726102b..cb23e09 100644 --- a/src/Presentation/Resources/AuthorResources.cs +++ b/src/Presentation/Resources/AuthorResources.cs @@ -4,6 +4,7 @@ namespace CleanModelContextProtocol.Presentation.Resources; using ModelContextProtocol.Server; using Presentation.Models; using Presentation.Serialization; +using Presentation.Tools; using Queries = Application.Authors.Queries; [McpServerResourceType] @@ -11,7 +12,7 @@ public static partial class AuthorResources { /// An author with their review count, average stars and every review they have written, including each movie's title. /// The author's id (a GUID). - [McpServerResource(UriTemplate = "authors://{authorId}", Name = "author", Title = "Author", MimeType = "application/json")] + [McpServerResource(UriTemplate = "authors://{authorId}", Name = "author", Title = "Author", MimeType = "application/json", IconSource = ItemIcons.Author)] public static async partial Task GetAuthor( ISender sender, Guid authorId, diff --git a/src/Presentation/Resources/MovieResources.cs b/src/Presentation/Resources/MovieResources.cs index 35bbf3a..1e78985 100644 --- a/src/Presentation/Resources/MovieResources.cs +++ b/src/Presentation/Resources/MovieResources.cs @@ -4,6 +4,7 @@ namespace CleanModelContextProtocol.Presentation.Resources; using ModelContextProtocol.Server; using Presentation.Models; using Presentation.Serialization; +using Presentation.Tools; using Queries = Application.Movies.Queries; [McpServerResourceType] @@ -11,7 +12,7 @@ public static partial class MovieResources { /// A movie with its review count, average stars and every review, including each author's name. /// The movie's id (a GUID). - [McpServerResource(UriTemplate = "movies://{movieId}", Name = "movie", Title = "Movie", MimeType = "application/json")] + [McpServerResource(UriTemplate = "movies://{movieId}", Name = "movie", Title = "Movie", MimeType = "application/json", IconSource = ItemIcons.Movie)] public static async partial Task GetMovie( ISender sender, Guid movieId, diff --git a/src/Presentation/Resources/ReviewResources.cs b/src/Presentation/Resources/ReviewResources.cs index a876371..e264e32 100644 --- a/src/Presentation/Resources/ReviewResources.cs +++ b/src/Presentation/Resources/ReviewResources.cs @@ -4,6 +4,7 @@ namespace CleanModelContextProtocol.Presentation.Resources; using ModelContextProtocol.Server; using Presentation.Models; using Presentation.Serialization; +using Presentation.Tools; using Queries = Application.Reviews.Queries; [McpServerResourceType] @@ -11,7 +12,7 @@ public static partial class ReviewResources { /// A review with its star rating, the movie's title and the author's name. /// The review's id (a GUID). - [McpServerResource(UriTemplate = "reviews://{reviewId}", Name = "review", Title = "Review", MimeType = "application/json")] + [McpServerResource(UriTemplate = "reviews://{reviewId}", Name = "review", Title = "Review", MimeType = "application/json", IconSource = ItemIcons.Review)] public static async partial Task GetReview( ISender sender, Guid reviewId, diff --git a/src/Presentation/Resources/VersionResources.cs b/src/Presentation/Resources/VersionResources.cs index 415adb1..bd9cc5c 100644 --- a/src/Presentation/Resources/VersionResources.cs +++ b/src/Presentation/Resources/VersionResources.cs @@ -3,13 +3,14 @@ namespace CleanModelContextProtocol.Presentation.Resources; using Mediator; using ModelContextProtocol.Server; using Presentation.Serialization; +using Presentation.Tools; using Queries = Application.Versions.Queries; [McpServerResourceType] public static partial class VersionResources { /// The version of this server: the file version and the informational version, which includes the commit it was built from. - [McpServerResource(UriTemplate = "server://version", Name = "server_version", Title = "Server version", MimeType = "application/json")] + [McpServerResource(UriTemplate = "server://version", Name = "server_version", Title = "Server version", MimeType = "application/json", IconSource = ItemIcons.Server)] public static async partial Task GetVersion(ISender sender, CancellationToken cancellationToken) { return McpSerializer.Serialize(await sender.Send(new Queries.GetVersion.GetVersionQuery(), cancellationToken)); diff --git a/src/Presentation/Tools/AuthorTools.cs b/src/Presentation/Tools/AuthorTools.cs index 28431fa..93f3add 100644 --- a/src/Presentation/Tools/AuthorTools.cs +++ b/src/Presentation/Tools/AuthorTools.cs @@ -18,7 +18,7 @@ public static partial class AuthorTools /// The number of results to skip, 0 for the first page. Pass the nextOffset from a previous call to get the next page. /// The most results to return, from 1 to 50. /// Also return every review each author has written. Leave false unless you need the individual reviews. - [McpServerTool(Name = "authors_search", Title = "Search authors", ReadOnly = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true)] + [McpServerTool(Name = "authors_search", Title = "Search authors", ReadOnly = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true, IconSource = ItemIcons.Author)] public static async partial Task> SearchAuthors( ISender sender, CancellationToken cancellationToken, diff --git a/src/Presentation/Tools/ItemIcons.cs b/src/Presentation/Tools/ItemIcons.cs new file mode 100644 index 0000000..e7d702a --- /dev/null +++ b/src/Presentation/Tools/ItemIcons.cs @@ -0,0 +1,15 @@ +namespace CleanModelContextProtocol.Presentation.Tools; + +/// +/// The icons on tools, prompts and resources, from a pinned brand release like the server's own icon. They point at +/// the brand repository, not this one, because dotnet new renames the template's name wherever it appears. +/// +public static class ItemIcons +{ + public const string Movie = Source + "movie-64.png"; + public const string Author = Source + "author-64.png"; + public const string Review = Source + "review-64.png"; + public const string Server = Source + "server-64.png"; + + private const string Source = "https://raw.githubusercontent.com/LeanMeanCleanApps/brand/2026.11.06/assets/icons/mcp/png/"; +} diff --git a/src/Presentation/Tools/MovieTools.cs b/src/Presentation/Tools/MovieTools.cs index 9c16add..061dc87 100644 --- a/src/Presentation/Tools/MovieTools.cs +++ b/src/Presentation/Tools/MovieTools.cs @@ -18,7 +18,7 @@ public static partial class MovieTools /// The number of results to skip, 0 for the first page. Pass the nextOffset from a previous call to get the next page. /// The most results to return, from 1 to 50. /// Also return every review of each movie. Leave false unless you need the individual reviews. - [McpServerTool(Name = "movies_search", Title = "Search movies", ReadOnly = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true)] + [McpServerTool(Name = "movies_search", Title = "Search movies", ReadOnly = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true, IconSource = ItemIcons.Movie)] public static async partial Task> SearchMovies( ISender sender, CancellationToken cancellationToken, @@ -50,7 +50,7 @@ public static async partial Task> SearchMovies( /// Get one movie with its review count, average stars and every review, including each author's name. /// The movie's id (a GUID). Use movies_search to find it. - [McpServerTool(Name = "movies_get", Title = "Get movie", ReadOnly = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true)] + [McpServerTool(Name = "movies_get", Title = "Get movie", ReadOnly = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true, IconSource = ItemIcons.Movie)] public static async partial Task GetMovie( ISender sender, Guid movieId, diff --git a/src/Presentation/Tools/ReviewTools.cs b/src/Presentation/Tools/ReviewTools.cs index 8670f41..3955842 100644 --- a/src/Presentation/Tools/ReviewTools.cs +++ b/src/Presentation/Tools/ReviewTools.cs @@ -18,7 +18,7 @@ public static partial class ReviewTools /// Get this one review instead of searching. The other filters are ignored. /// The number of results to skip, 0 for the first page. Pass the nextOffset from a previous call to get the next page. /// The most results to return, from 1 to 50. - [McpServerTool(Name = "reviews_list", Title = "List reviews", ReadOnly = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true)] + [McpServerTool(Name = "reviews_list", Title = "List reviews", ReadOnly = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true, IconSource = ItemIcons.Review)] public static async partial Task> ListReviews( ISender sender, CancellationToken cancellationToken, @@ -58,7 +58,7 @@ public static async partial Task> ListReviews( /// The id of the movie being reviewed, needed to create a review. Use movies_search to find it. /// The id of the author writing the review, needed to create a review. Use authors_search to find it. /// The id of an existing review to change the stars of. Leave empty to create a new review. - [McpServerTool(Name = "reviews_write", Title = "Write review", ReadOnly = false, Destructive = true, Idempotent = false, OpenWorld = false, UseStructuredContent = true)] + [McpServerTool(Name = "reviews_write", Title = "Write review", ReadOnly = false, Destructive = true, Idempotent = false, OpenWorld = false, UseStructuredContent = true, IconSource = ItemIcons.Review)] public static async partial Task WriteReview( ISender sender, int stars, @@ -90,7 +90,7 @@ public static async partial Task WriteReview( /// Permanently delete a review. This can't be undone, so confirm with the user first. /// The id of the review to delete. Use reviews_list to find it. - [McpServerTool(Name = "reviews_delete", Title = "Delete review", ReadOnly = false, Destructive = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true)] + [McpServerTool(Name = "reviews_delete", Title = "Delete review", ReadOnly = false, Destructive = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true, IconSource = ItemIcons.Review)] public static async partial Task DeleteReview( ISender sender, Guid reviewId, diff --git a/tests/Presentation.Tests.Integration/Tools/ToolListTests.cs b/tests/Presentation.Tests.Integration/Tools/ToolListTests.cs index e052529..1638d03 100644 --- a/tests/Presentation.Tests.Integration/Tools/ToolListTests.cs +++ b/tests/Presentation.Tests.Integration/Tools/ToolListTests.cs @@ -69,6 +69,22 @@ public async Task ListTools_ShouldWrite_PortableSchemas() } } + [Fact] + public async Task EveryItem_ShouldHave_AnIcon() + { + // Act + var tools = await this.Client.ListToolsAsync(); + var prompts = await this.Client.ListPromptsAsync(); + var resources = await this.Client.ListResourcesAsync(); + var templates = await this.Client.ListResourceTemplatesAsync(); + + // Assert + tools.ShouldAllBe(t => t.ProtocolTool.Icons!.Count == 1); + prompts.ShouldAllBe(p => p.ProtocolPrompt.Icons!.Count == 1); + resources.ShouldAllBe(r => r.ProtocolResource.Icons!.Count == 1); + templates.ShouldAllBe(t => t.ProtocolResourceTemplate.Icons!.Count == 1); + } + [Fact] public async Task ServerInfo_ShouldDescribe_Server() { From 1459b827ff7287858f2f7a0833859981f3c49fa3 Mon Sep 17 00:00:00 2001 From: Stephen Walsh <868017+stphnwlsh@users.noreply.github.com> Date: Fri, 6 Nov 2026 11:30:44 -0600 Subject: [PATCH 6/7] Add a browsable top rated movies resource --- .agents/skills/expose-a-use-case/SKILL.md | 2 +- .../WebApplicationBuilderExtensions.cs | 3 +- src/Presentation/Resources/MovieResources.cs | 38 +++++++++++++++++++ .../Resources/ResourceTests.cs | 20 ++++++++-- 4 files changed, 57 insertions(+), 6 deletions(-) diff --git a/.agents/skills/expose-a-use-case/SKILL.md b/.agents/skills/expose-a-use-case/SKILL.md index 6a0ff0f..4c62022 100644 --- a/.agents/skills/expose-a-use-case/SKILL.md +++ b/.agents/skills/expose-a-use-case/SKILL.md @@ -24,7 +24,7 @@ The SDK finds every class in the assembly (`WithToolsFromAssembly`, `WithResourc ## Steps 1. Add the use case first (`add-a-use-case`). -2. Pick the kind: a tool when the model should decide to call it, a resource when the client reads a known thing by id, a prompt when a person starts a task. +2. Pick the kind: a tool when the model should decide to call it, a resource when the client reads a known thing by id or a fixed list such as `movies://top-rated`, a prompt when a person starts a task. 3. Choose the name, title and annotations for a tool (`api-conventions`). 4. Write the method with `sender` and `cancellationToken` first, then required inputs, then optional ones with defaults. 5. Document it: `` saying what it does, what it returns and which tool to call first; a `` for every input except `sender` and the token, with allowed values and where to find ids. Put the XML comment above the attributes. diff --git a/src/Presentation/Extensions/WebApplicationBuilderExtensions.cs b/src/Presentation/Extensions/WebApplicationBuilderExtensions.cs index 65e167b..df1ce91 100644 --- a/src/Presentation/Extensions/WebApplicationBuilderExtensions.cs +++ b/src/Presentation/Extensions/WebApplicationBuilderExtensions.cs @@ -183,7 +183,8 @@ so pass nextOffset back as the offset to continue rather than raising the limit. reviews_write creates a review, or changes the stars of one when you pass its reviewId. A review's movie and author can't change. reviews_delete can't be undone, so confirm with the user before calling it. Each movie, author and review can also be read as a resource at movies://{id}, authors://{id} or - reviews://{id}, and server://version gives this server's version. + reviews://{id}. movies://top-rated lists the ten best rated movies, and server://version gives this + server's version. """; }) .WithHttpTransport(options => options.Stateless = true) diff --git a/src/Presentation/Resources/MovieResources.cs b/src/Presentation/Resources/MovieResources.cs index 1e78985..26d64fe 100644 --- a/src/Presentation/Resources/MovieResources.cs +++ b/src/Presentation/Resources/MovieResources.cs @@ -1,5 +1,7 @@ namespace CleanModelContextProtocol.Presentation.Resources; +using Application.Common.Paging; +using Application.Movies.Entities; using Mediator; using ModelContextProtocol.Server; using Presentation.Models; @@ -10,6 +12,42 @@ namespace CleanModelContextProtocol.Presentation.Resources; [McpServerResourceType] public static partial class MovieResources { + private const int TopRatedCount = 10; + + /// The ten best rated movies, highest average stars first, with each one's review count. + [McpServerResource(UriTemplate = "movies://top-rated", Name = "top_rated_movies", Title = "Top rated movies", MimeType = "application/json", IconSource = ItemIcons.Movie)] + public static async partial Task GetTopRatedMovies( + ISender sender, + CancellationToken cancellationToken) + { + var movies = new List(); + Page page; + + // The query sorts by title, so ranking needs every movie. + do + { + page = await sender.Send(new Queries.GetMovies.GetMoviesQuery + { + Offset = movies.Count, + Limit = Paging.MaxLimit + }, cancellationToken); + + movies.AddRange(page.Items); + } + while (movies.Count < page.TotalCount); + + var topRated = movies + .Where(m => m.ReviewCount > 0) + .OrderByDescending(m => m.AverageStars) + .ThenByDescending(m => m.ReviewCount) + .ThenBy(m => m.Title, StringComparer.Ordinal) + .Take(TopRatedCount) + .Select(m => m.ToResult()) + .ToList(); + + return McpSerializer.Serialize(new ResultPage(topRated, topRated.Count, null)); + } + /// A movie with its review count, average stars and every review, including each author's name. /// The movie's id (a GUID). [McpServerResource(UriTemplate = "movies://{movieId}", Name = "movie", Title = "Movie", MimeType = "application/json", IconSource = ItemIcons.Movie)] diff --git a/tests/Presentation.Tests.Integration/Resources/ResourceTests.cs b/tests/Presentation.Tests.Integration/Resources/ResourceTests.cs index 83c8afc..2973747 100644 --- a/tests/Presentation.Tests.Integration/Resources/ResourceTests.cs +++ b/tests/Presentation.Tests.Integration/Resources/ResourceTests.cs @@ -22,15 +22,27 @@ public async Task ListResourceTemplates_ShouldReturn_EveryTemplate() } [Fact] - public async Task ListResources_ShouldReturn_ServerVersion() + public async Task ListResources_ShouldReturn_EveryResource() { // Act var resources = await this.Client.ListResourcesAsync(); // Assert - var resource = resources.ShouldHaveSingleItem(); - resource.Uri.ShouldBe("server://version"); - resource.MimeType.ShouldBe("application/json"); + resources.Select(r => r.Uri).ShouldBe(["server://version", "movies://top-rated"], ignoreOrder: true); + resources.ShouldAllBe(r => r.MimeType == "application/json"); + } + + [Fact] + public async Task ReadTopRated_ShouldReturn_BestMoviesFirst() + { + // Act + var page = await this.ReadAsync>("movies://top-rated"); + + // Assert + page.Items.Count.ShouldBe(10); + page.NextOffset.ShouldBeNull(); + page.Items.ShouldAllBe(m => m.ReviewCount > 0); + page.Items.Select(m => m.AverageStars).ShouldBeInOrder(SortDirection.Descending); } [Fact] From c67034a8df0599ea5775749c705b6de3d5726e80 Mon Sep 17 00:00:00 2001 From: Stephen Walsh <868017+stphnwlsh@users.noreply.github.com> Date: Fri, 6 Nov 2026 11:36:21 -0600 Subject: [PATCH 7/7] Suggest ids from every page and tidy the completion checks --- .../Completions/CompletionHandler.cs | 73 +++++++++++++------ .../Completions/CompletionTests.cs | 26 +++++++ .../Completions/CompletionHandlerTests.cs | 47 ++++++++++++ 3 files changed, 124 insertions(+), 22 deletions(-) create mode 100644 tests/Presentation.Tests.Unit/Completions/CompletionHandlerTests.cs diff --git a/src/Presentation/Completions/CompletionHandler.cs b/src/Presentation/Completions/CompletionHandler.cs index 849c8b8..d0cec4b 100644 --- a/src/Presentation/Completions/CompletionHandler.cs +++ b/src/Presentation/Completions/CompletionHandler.cs @@ -17,6 +17,9 @@ public static class CompletionHandler { private const int MaxQueryLength = 100; + // The most values the protocol allows in one completion. + private const int MaxValues = 100; + public static async ValueTask Handle(RequestContext context, CancellationToken cancellationToken) { return await Complete(context.Services!.GetRequiredService(), context.Params, cancellationToken); @@ -24,19 +27,14 @@ public static async ValueTask Handle(RequestContext Complete(ISender sender, CompleteRequestParams? request, CancellationToken cancellationToken) { - var argument = request?.Argument; - - if (argument is null || argument.Value?.Length > MaxQueryLength) + if (request?.Argument is not { } argument || argument.Value.Length > MaxQueryLength) { return Empty(); } - return request?.Ref switch - { - PromptReference => await CompletePromptArgument(sender, argument, cancellationToken), - ResourceTemplateReference => await CompleteResourceId(sender, argument, cancellationToken), - _ => Empty() - }; + return request.Ref is PromptReference + ? await CompletePromptArgument(sender, argument, cancellationToken) + : await CompleteResourceId(sender, argument, cancellationToken); } private static async Task CompletePromptArgument(ISender sender, Argument argument, CancellationToken cancellationToken) @@ -87,9 +85,13 @@ private static async Task CompleteResourceId(ISender sender, Arg return Result([.. page.Items.Select(m => m.Id.ToString())], page.TotalCount); } - var movies = await sender.Send(new MovieQueries.GetMovies.GetMoviesQuery { Limit = Paging.MaxLimit }, cancellationToken); + var movies = await GetAll(offset => sender.Send(new MovieQueries.GetMovies.GetMoviesQuery + { + Offset = offset, + Limit = Paging.MaxLimit + }, cancellationToken)); - return IdsStartingWith(movies.Items.Select(m => m.Id), argument.Value); + return IdsStartingWith(movies.Select(m => m.Id), argument.Value); } case "authorId": @@ -105,22 +107,29 @@ private static async Task CompleteResourceId(ISender sender, Arg return Result([.. page.Items.Select(a => a.Id.ToString())], page.TotalCount); } - var authors = await sender.Send(new AuthorQueries.GetAuthors.GetAuthorsQuery { Limit = Paging.MaxLimit }, cancellationToken); + var authors = await GetAll(offset => sender.Send(new AuthorQueries.GetAuthors.GetAuthorsQuery + { + Offset = offset, + Limit = Paging.MaxLimit + }, cancellationToken)); - return IdsStartingWith(authors.Items.Select(a => a.Id), argument.Value); + return IdsStartingWith(authors.Select(a => a.Id), argument.Value); } case "reviewId": { - var reviews = await sender.Send(new ReviewQueries.GetReviews.GetReviewsQuery { Limit = Paging.MaxLimit }, cancellationToken); - var value = argument.Value ?? string.Empty; - var matches = reviews.Items - .Where(r => r.Id.ToString().StartsWith(value, StringComparison.OrdinalIgnoreCase) - || (r.ReviewedMovie?.Title.Contains(value, StringComparison.OrdinalIgnoreCase) ?? false)) + var reviews = await GetAll(offset => sender.Send(new ReviewQueries.GetReviews.GetReviewsQuery + { + Offset = offset, + Limit = Paging.MaxLimit + }, cancellationToken)); + var matches = reviews + .Where(r => r.Id.ToString().StartsWith(argument.Value, StringComparison.OrdinalIgnoreCase) + || (r.ReviewedMovie?.Title.Contains(argument.Value, StringComparison.OrdinalIgnoreCase) ?? false)) .Select(r => r.Id.ToString()) .ToList(); - return Result(matches, matches.Count); + return Capped(matches); } default: @@ -128,14 +137,34 @@ private static async Task CompleteResourceId(ISender sender, Arg } } - private static CompleteResult IdsStartingWith(IEnumerable ids, string? value) + private static CompleteResult IdsStartingWith(IEnumerable ids, string value) { var matches = ids .Select(i => i.ToString()) - .Where(i => i.StartsWith(value ?? string.Empty, StringComparison.OrdinalIgnoreCase)) + .Where(i => i.StartsWith(value, StringComparison.OrdinalIgnoreCase)) .ToList(); - return Result(matches, matches.Count); + return Capped(matches); + } + + private static async Task> GetAll(Func>> getPage) + { + var items = new List(); + Page page; + + do + { + page = await getPage(items.Count); + items.AddRange(page.Items); + } + while (items.Count < page.TotalCount); + + return items; + } + + private static CompleteResult Capped(List matches) + { + return Result([.. matches.Take(MaxValues)], matches.Count); } private static CompleteResult Result(List values, int total) diff --git a/tests/Presentation.Tests.Integration/Completions/CompletionTests.cs b/tests/Presentation.Tests.Integration/Completions/CompletionTests.cs index 9d18ebb..4105250 100644 --- a/tests/Presentation.Tests.Integration/Completions/CompletionTests.cs +++ b/tests/Presentation.Tests.Integration/Completions/CompletionTests.cs @@ -62,6 +62,32 @@ public async Task Complete_WithUnsupportedArgument_ShouldReturn_Nothing(string p result.Completion.HasMore.ShouldBe(false); } + [Fact] + public async Task CompleteReviewId_ShouldFind_ReviewBeyondTheFirstPage() + { + // Arrange + var review = (await this.CallAsync>("reviews_list", + new Dictionary { ["offset"] = 120, ["limit"] = 1 })).Items[0]; + + // Act + var result = await this.Client.CompleteAsync(new ResourceTemplateReference { Uri = "reviews://{reviewId}" }, "reviewId", review.Id.ToString()); + + // Assert + result.Completion.Values.ShouldContain(review.Id.ToString()); + } + + [Fact] + public async Task CompleteReviewId_WithEmptyValue_ShouldCap_AtTheProtocolLimit() + { + // Act + var result = await this.Client.CompleteAsync(new ResourceTemplateReference { Uri = "reviews://{reviewId}" }, "reviewId", string.Empty); + + // Assert + result.Completion.Values.Count.ShouldBe(100); + result.Completion.Total.ShouldBe(150); + result.Completion.HasMore.ShouldBe(true); + } + [Fact] public async Task CompleteMovieId_ShouldMatch_Title() { diff --git a/tests/Presentation.Tests.Unit/Completions/CompletionHandlerTests.cs b/tests/Presentation.Tests.Unit/Completions/CompletionHandlerTests.cs new file mode 100644 index 0000000..aee1e2e --- /dev/null +++ b/tests/Presentation.Tests.Unit/Completions/CompletionHandlerTests.cs @@ -0,0 +1,47 @@ +namespace CleanModelContextProtocol.Presentation.Tests.Unit.Completions; + +using Application.Common.Paging; +using Application.Reviews.Entities; +using Application.Reviews.Queries.GetReviews; +using Mediator; +using ModelContextProtocol.Protocol; +using NSubstitute; +using Presentation.Completions; +using Shouldly; +using Xunit; + +public class CompletionHandlerTests +{ + [Fact] + public async Task Complete_WithoutRequest_ShouldReturn_Nothing() + { + // Act + var result = await CompletionHandler.Complete(Substitute.For(), null, CancellationToken.None); + + // Assert + result.Completion.Values.ShouldBeEmpty(); + } + + [Fact] + public async Task Complete_ReviewWithoutMovie_ShouldNot_Match() + { + // Arrange + var review = new Review(Guid.NewGuid(), 5); + var sender = Substitute.For(); + + _ = sender.Send(Arg.Any(), Arg.Any()) + .Returns(new Page([review], 0, Paging.MaxLimit, 1)); + + var request = new CompleteRequestParams + { + Ref = new ResourceTemplateReference { Uri = "reviews://{reviewId}" }, + Argument = new Argument { Name = "reviewId", Value = "no-match" } + }; + + // Act + var result = await CompletionHandler.Complete(sender, request, CancellationToken.None); + + // Assert + result.Completion.Values.ShouldBeEmpty(); + } +}