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(); + } + } +}