From e7fa6194f6902c1ab575050f998974473be5c912 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Tue, 18 Mar 2025 18:48:28 +0200 Subject: [PATCH] Turn on the AOT analyzer. (#92) * Turn on the AOT analyzer. * Address feedback * Add missing types. * Fix build and react to conflicts. * Fix failing test. * Address feedback. --- .../DynamicallyAccessedMemberTypes.cs | 164 ++++++++++++++++++ .../DynamicallyAccessedMembersAttribute.cs | 50 ++++++ .../RequiresDynamicCodeAttribute.cs | 38 ++++ .../CodeAnalysis/RequiresUnreferencedCode.cs | 39 +++++ .../UnconditionalSuppressMessageAttribute.cs | 84 +++++++++ .../McpDotNet.Extensions.AI.csproj | 8 + src/mcpdotnet/Client/McpClient.cs | 8 +- src/mcpdotnet/Client/McpClientExtensions.cs | 10 +- .../McpServerBuilderExtensions.Tools.cs | 50 ++++-- .../Protocol/Messages/IJsonRpcMessage.cs | 6 +- .../HttpListenerSseServerTransport.cs | 4 +- .../Protocol/Transport/SseClientTransport.cs | 6 +- .../Transport/StdioClientTransport.cs | 4 +- .../Transport/StdioServerTransport.cs | 4 +- src/mcpdotnet/Shared/McpJsonRpcEndpoint.cs | 16 +- .../Utils/Json/JsonRpcMessageConverter.cs | 24 +-- .../Json/JsonSerializerOptionsExtensions.cs | 62 +++++-- src/mcpdotnet/mcpdotnet.csproj | 4 + tests/mcpdotnet.TestSseServer/Program.cs | 1 - .../McpServerBuilderExtensionsToolsTests.cs | 2 +- .../mcpdotnet.Tests/Server/McpServerTests.cs | 4 +- .../SseServerIntegrationTestFixture.cs | 2 + 22 files changed, 522 insertions(+), 68 deletions(-) create mode 100644 src/Common/Polyfills/System/Diagnostics/CodeAnalysis/DynamicallyAccessedMemberTypes.cs create mode 100644 src/Common/Polyfills/System/Diagnostics/CodeAnalysis/DynamicallyAccessedMembersAttribute.cs create mode 100644 src/Common/Polyfills/System/Diagnostics/CodeAnalysis/RequiresDynamicCodeAttribute.cs create mode 100644 src/Common/Polyfills/System/Diagnostics/CodeAnalysis/RequiresUnreferencedCode.cs create mode 100644 src/Common/Polyfills/System/Diagnostics/CodeAnalysis/UnconditionalSuppressMessageAttribute.cs diff --git a/src/Common/Polyfills/System/Diagnostics/CodeAnalysis/DynamicallyAccessedMemberTypes.cs b/src/Common/Polyfills/System/Diagnostics/CodeAnalysis/DynamicallyAccessedMemberTypes.cs new file mode 100644 index 0000000..ee6fa51 --- /dev/null +++ b/src/Common/Polyfills/System/Diagnostics/CodeAnalysis/DynamicallyAccessedMemberTypes.cs @@ -0,0 +1,164 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics.CodeAnalysis; + +/// +/// Specifies the types of members that are dynamically accessed. +/// +/// This enumeration has a attribute that allows a +/// bitwise combination of its member values. +/// +[Flags] +internal enum DynamicallyAccessedMemberTypes +{ + /// + /// Specifies no members. + /// + None = 0, + + /// + /// Specifies the default, parameterless public constructor. + /// + PublicParameterlessConstructor = 0x0001, + + /// + /// Specifies all public constructors. + /// + PublicConstructors = 0x0002 | PublicParameterlessConstructor, + + /// + /// Specifies all non-public constructors. + /// + NonPublicConstructors = 0x0004, + + /// + /// Specifies all public methods. + /// + PublicMethods = 0x0008, + + /// + /// Specifies all non-public methods. + /// + NonPublicMethods = 0x0010, + + /// + /// Specifies all public fields. + /// + PublicFields = 0x0020, + + /// + /// Specifies all non-public fields. + /// + NonPublicFields = 0x0040, + + /// + /// Specifies all public nested types. + /// + PublicNestedTypes = 0x0080, + + /// + /// Specifies all non-public nested types. + /// + NonPublicNestedTypes = 0x0100, + + /// + /// Specifies all public properties. + /// + PublicProperties = 0x0200, + + /// + /// Specifies all non-public properties. + /// + NonPublicProperties = 0x0400, + + /// + /// Specifies all public events. + /// + PublicEvents = 0x0800, + + /// + /// Specifies all non-public events. + /// + NonPublicEvents = 0x1000, + + /// + /// Specifies all interfaces implemented by the type. + /// + Interfaces = 0x2000, + + /// + /// Specifies all non-public constructors, including those inherited from base classes. + /// + NonPublicConstructorsWithInherited = NonPublicConstructors | 0x4000, + + /// + /// Specifies all non-public methods, including those inherited from base classes. + /// + NonPublicMethodsWithInherited = NonPublicMethods | 0x8000, + + /// + /// Specifies all non-public fields, including those inherited from base classes. + /// + NonPublicFieldsWithInherited = NonPublicFields | 0x10000, + + /// + /// Specifies all non-public nested types, including those inherited from base classes. + /// + NonPublicNestedTypesWithInherited = NonPublicNestedTypes | 0x20000, + + /// + /// Specifies all non-public properties, including those inherited from base classes. + /// + NonPublicPropertiesWithInherited = NonPublicProperties | 0x40000, + + /// + /// Specifies all non-public events, including those inherited from base classes. + /// + NonPublicEventsWithInherited = NonPublicEvents | 0x80000, + + /// + /// Specifies all public constructors, including those inherited from base classes. + /// + PublicConstructorsWithInherited = PublicConstructors | 0x100000, + + /// + /// Specifies all public nested types, including those inherited from base classes. + /// + PublicNestedTypesWithInherited = PublicNestedTypes | 0x200000, + + /// + /// Specifies all constructors, including those inherited from base classes. + /// + AllConstructors = PublicConstructorsWithInherited | NonPublicConstructorsWithInherited, + + /// + /// Specifies all methods, including those inherited from base classes. + /// + AllMethods = PublicMethods | NonPublicMethodsWithInherited, + + /// + /// Specifies all fields, including those inherited from base classes. + /// + AllFields = PublicFields | NonPublicFieldsWithInherited, + + /// + /// Specifies all nested types, including those inherited from base classes. + /// + AllNestedTypes = PublicNestedTypesWithInherited | NonPublicNestedTypesWithInherited, + + /// + /// Specifies all properties, including those inherited from base classes. + /// + AllProperties = PublicProperties | NonPublicPropertiesWithInherited, + + /// + /// Specifies all events, including those inherited from base classes. + /// + AllEvents = PublicEvents | NonPublicEventsWithInherited, + + /// + /// Specifies all members. + /// + All = ~None +} diff --git a/src/Common/Polyfills/System/Diagnostics/CodeAnalysis/DynamicallyAccessedMembersAttribute.cs b/src/Common/Polyfills/System/Diagnostics/CodeAnalysis/DynamicallyAccessedMembersAttribute.cs new file mode 100644 index 0000000..2d01404 --- /dev/null +++ b/src/Common/Polyfills/System/Diagnostics/CodeAnalysis/DynamicallyAccessedMembersAttribute.cs @@ -0,0 +1,50 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics.CodeAnalysis; + +/// +/// Indicates that certain members on a specified are accessed dynamically, +/// for example through . +/// +/// +/// This allows tools to understand which members are being accessed during the execution +/// of a program. +/// +/// This attribute is valid on members whose type is or . +/// +/// When this attribute is applied to a location of type , the assumption is +/// that the string represents a fully qualified type name. +/// +/// When this attribute is applied to a class, interface, or struct, the members specified +/// can be accessed dynamically on instances returned from calling +/// on instances of that class, interface, or struct. +/// +/// If the attribute is applied to a method it's treated as a special case and it implies +/// the attribute should be applied to the "this" parameter of the method. As such the attribute +/// should only be used on instance methods of types assignable to System.Type (or string, but no methods +/// will use it there). +/// +[AttributeUsage( + AttributeTargets.Field | AttributeTargets.ReturnValue | AttributeTargets.GenericParameter | + AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.Method | + AttributeTargets.Class | AttributeTargets.Interface | AttributeTargets.Struct, + Inherited = false)] +internal sealed class DynamicallyAccessedMembersAttribute : Attribute +{ + /// + /// Initializes a new instance of the class + /// with the specified member types. + /// + /// The types of members dynamically accessed. + public DynamicallyAccessedMembersAttribute(DynamicallyAccessedMemberTypes memberTypes) + { + MemberTypes = memberTypes; + } + + /// + /// Gets the which specifies the type + /// of members dynamically accessed. + /// + public DynamicallyAccessedMemberTypes MemberTypes { get; } +} diff --git a/src/Common/Polyfills/System/Diagnostics/CodeAnalysis/RequiresDynamicCodeAttribute.cs b/src/Common/Polyfills/System/Diagnostics/CodeAnalysis/RequiresDynamicCodeAttribute.cs new file mode 100644 index 0000000..817ec6e --- /dev/null +++ b/src/Common/Polyfills/System/Diagnostics/CodeAnalysis/RequiresDynamicCodeAttribute.cs @@ -0,0 +1,38 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics.CodeAnalysis; + +/// +/// Indicates that the specified method requires the ability to generate new code at runtime, +/// for example through . +/// +/// +/// This allows tools to understand which methods are unsafe to call when compiling ahead of time. +/// +[AttributeUsage(AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Class, Inherited = false)] +internal sealed class RequiresDynamicCodeAttribute : Attribute +{ + /// + /// Initializes a new instance of the class + /// with the specified message. + /// + /// + /// A message that contains information about the usage of dynamic code. + /// + public RequiresDynamicCodeAttribute(string message) + { + Message = message; + } + + /// + /// Gets a message that contains information about the usage of dynamic code. + /// + public string Message { get; } + + /// + /// Gets or sets an optional URL that contains more information about the method, + /// why it requires dynamic code, and what options a consumer has to deal with it. + /// + public string? Url { get; set; } +} diff --git a/src/Common/Polyfills/System/Diagnostics/CodeAnalysis/RequiresUnreferencedCode.cs b/src/Common/Polyfills/System/Diagnostics/CodeAnalysis/RequiresUnreferencedCode.cs new file mode 100644 index 0000000..e4a5344 --- /dev/null +++ b/src/Common/Polyfills/System/Diagnostics/CodeAnalysis/RequiresUnreferencedCode.cs @@ -0,0 +1,39 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics.CodeAnalysis; + +/// +/// Indicates that the specified method requires dynamic access to code that is not referenced +/// statically, for example through . +/// +/// +/// This allows tools to understand which methods are unsafe to call when removing unreferenced +/// code from an application. +/// +[AttributeUsage(AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Class, Inherited = false)] +internal sealed class RequiresUnreferencedCodeAttribute : Attribute +{ + /// + /// Initializes a new instance of the class + /// with the specified message. + /// + /// + /// A message that contains information about the usage of unreferenced code. + /// + public RequiresUnreferencedCodeAttribute(string message) + { + Message = message; + } + + /// + /// Gets a message that contains information about the usage of unreferenced code. + /// + public string Message { get; } + + /// + /// Gets or sets an optional URL that contains more information about the method, + /// why it requires unreferenced code, and what options a consumer has to deal with it. + /// + public string? Url { get; set; } +} diff --git a/src/Common/Polyfills/System/Diagnostics/CodeAnalysis/UnconditionalSuppressMessageAttribute.cs b/src/Common/Polyfills/System/Diagnostics/CodeAnalysis/UnconditionalSuppressMessageAttribute.cs new file mode 100644 index 0000000..b06d9ed --- /dev/null +++ b/src/Common/Polyfills/System/Diagnostics/CodeAnalysis/UnconditionalSuppressMessageAttribute.cs @@ -0,0 +1,84 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics.CodeAnalysis; + +/// +/// /// Suppresses reporting of a specific rule violation, allowing multiple suppressions on a +/// single code artifact. +/// +/// +/// is different than +/// in that it doesn't have a +/// . So it is always preserved in the compiled assembly. +/// +[AttributeUsage(AttributeTargets.All, Inherited = false, AllowMultiple = true)] +internal sealed class UnconditionalSuppressMessageAttribute : Attribute +{ + /// + /// Initializes a new instance of the + /// class, specifying the category of the tool and the identifier for an analysis rule. + /// + /// The category for the attribute. + /// The identifier of the analysis rule the attribute applies to. + public UnconditionalSuppressMessageAttribute(string category, string checkId) + { + Category = category; + CheckId = checkId; + } + + /// + /// Gets the category identifying the classification of the attribute. + /// + /// + /// The property describes the tool or tool analysis category + /// for which a message suppression attribute applies. + /// + public string Category { get; } + + /// + /// Gets the identifier of the analysis tool rule to be suppressed. + /// + /// + /// Concatenated together, the and + /// properties form a unique check identifier. + /// + public string CheckId { get; } + + /// + /// Gets or sets the scope of the code that is relevant for the attribute. + /// + /// + /// The Scope property is an optional argument that specifies the metadata scope for which + /// the attribute is relevant. + /// + public string? Scope { get; set; } + + /// + /// Gets or sets a fully qualified path that represents the target of the attribute. + /// + /// + /// The property is an optional argument identifying the analysis target + /// of the attribute. An example value is "System.IO.Stream.ctor():System.Void". + /// Because it is fully qualified, it can be long, particularly for targets such as parameters. + /// The analysis tool user interface should be capable of automatically formatting the parameter. + /// + public string? Target { get; set; } + + /// + /// Gets or sets an optional argument expanding on exclusion criteria. + /// + /// + /// The property is an optional argument that specifies additional + /// exclusion where the literal metadata target is not sufficiently precise. For example, + /// the cannot be applied within a method, + /// and it may be desirable to suppress a violation against a statement in the method that will + /// give a rule violation, but not against all statements in the method. + /// + public string? MessageId { get; set; } + + /// + /// Gets or sets the justification for suppressing the code analysis message. + /// + public string? Justification { get; set; } +} \ No newline at end of file diff --git a/src/McpDotNet.Extensions.AI/McpDotNet.Extensions.AI.csproj b/src/McpDotNet.Extensions.AI/McpDotNet.Extensions.AI.csproj index 924d3f4..da22226 100644 --- a/src/McpDotNet.Extensions.AI/McpDotNet.Extensions.AI.csproj +++ b/src/McpDotNet.Extensions.AI/McpDotNet.Extensions.AI.csproj @@ -9,6 +9,10 @@ All + + true + + McpDotNet.Extensions.AI @@ -37,4 +41,8 @@ + + + + diff --git a/src/mcpdotnet/Client/McpClient.cs b/src/mcpdotnet/Client/McpClient.cs index c806d91..c5f6149 100644 --- a/src/mcpdotnet/Client/McpClient.cs +++ b/src/mcpdotnet/Client/McpClient.cs @@ -4,10 +4,9 @@ using McpDotNet.Protocol.Messages; using McpDotNet.Protocol.Transport; using McpDotNet.Protocol.Types; using McpDotNet.Shared; - +using McpDotNet.Utils.Json; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; - using System.Text.Json; namespace McpDotNet.Client; @@ -139,7 +138,10 @@ internal sealed class McpClient : McpJsonRpcEndpoint, IMcpClient initializationCts.Token).ConfigureAwait(false); // Store server information - _logger.ServerCapabilitiesReceived(EndpointName, JsonSerializer.Serialize(initializeResponse.Capabilities), JsonSerializer.Serialize(initializeResponse.ServerInfo)); + _logger.ServerCapabilitiesReceived(EndpointName, + capabilities: JsonSerializer.Serialize(initializeResponse.Capabilities, JsonSerializerOptionsExtensions.JsonContext.Default.ServerCapabilities), + serverInfo: JsonSerializer.Serialize(initializeResponse.ServerInfo, JsonSerializerOptionsExtensions.JsonContext.Default.Implementation)); + ServerCapabilities = initializeResponse.Capabilities; ServerInfo = initializeResponse.ServerInfo; ServerInstructions = initializeResponse.Instructions; diff --git a/src/mcpdotnet/Client/McpClientExtensions.cs b/src/mcpdotnet/Client/McpClientExtensions.cs index 1eeebb2..1f0da00 100644 --- a/src/mcpdotnet/Client/McpClientExtensions.cs +++ b/src/mcpdotnet/Client/McpClientExtensions.cs @@ -1,6 +1,7 @@ using McpDotNet.Protocol.Messages; using McpDotNet.Protocol.Types; using McpDotNet.Utils; +using McpDotNet.Utils.Json; using Microsoft.Extensions.AI; using System.Runtime.CompilerServices; using System.Text.Json; @@ -465,6 +466,8 @@ public static class McpClientExtensions /// Provides an AI function that calls a tool through . private sealed class McpAIFunction(IMcpClient client, Tool tool) : AIFunction { + private JsonElement? _jsonSchema; + /// public override string Name => tool.Name; @@ -472,7 +475,7 @@ public static class McpClientExtensions public override string Description => tool.Description ?? string.Empty; /// - public override JsonElement JsonSchema => + public override JsonElement JsonSchema => _jsonSchema ??= JsonSerializer.SerializeToElement(new Dictionary { ["type"] = "object", @@ -480,7 +483,7 @@ public static class McpClientExtensions ["description"] = tool.Description ?? string.Empty, ["properties"] = tool.InputSchema?.Properties ?? [], ["required"] = tool.InputSchema?.Required ?? [] - }); + }, JsonSerializerOptionsExtensions.JsonContext.Default.DictionaryStringObject); /// protected async override Task InvokeCoreAsync( @@ -498,8 +501,7 @@ public static class McpClientExtensions } CallToolResponse result = await client.CallToolAsync(tool.Name, argDict, cancellationToken).ConfigureAwait(false); - - return JsonSerializer.SerializeToElement(result); + return JsonSerializer.SerializeToElement(result, JsonSerializerOptionsExtensions.JsonContext.Default.CallToolResponse); } } } \ No newline at end of file diff --git a/src/mcpdotnet/Configuration/McpServerBuilderExtensions.Tools.cs b/src/mcpdotnet/Configuration/McpServerBuilderExtensions.Tools.cs index 872c550..a6cd52e 100644 --- a/src/mcpdotnet/Configuration/McpServerBuilderExtensions.Tools.cs +++ b/src/mcpdotnet/Configuration/McpServerBuilderExtensions.Tools.cs @@ -2,9 +2,9 @@ using McpDotNet.Protocol.Types; using McpDotNet.Server; using McpDotNet.Utils; - +using McpDotNet.Utils.Json; using Microsoft.Extensions.AI; - +using System.Diagnostics.CodeAnalysis; using System.Reflection; using System.Text.Json; @@ -15,21 +15,28 @@ namespace McpDotNet; /// public static partial class McpServerBuilderExtensions { + private const string RequiresUnreferencedCodeMessage = "This method requires dynamic lookup of method metadata and might not work in Native AOT."; + /// /// Adds a tool to the server. /// /// The tool type. /// The builder instance. /// is . - public static IMcpServerBuilder WithTool(this IMcpServerBuilder builder) + public static IMcpServerBuilder WithTool<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TTool>(this IMcpServerBuilder builder) { - return WithTools(builder, typeof(TTool)); + Throw.IfNull(builder); + List functions = []; + + PopulateFunctions(typeof(TTool), functions); + return WithTools(builder, functions); } /// /// Adds all tools marked with from the current assembly to the server. /// /// The builder instance. /// is . + [RequiresUnreferencedCode(RequiresUnreferencedCodeMessage)] public static IMcpServerBuilder WithTools(this IMcpServerBuilder builder) { return WithToolsFromAssembly(builder, Assembly.GetCallingAssembly()); @@ -42,6 +49,7 @@ public static partial class McpServerBuilderExtensions /// Types with marked methods to add as tools to the server. /// is . /// is . + [RequiresUnreferencedCode(RequiresUnreferencedCodeMessage)] public static IMcpServerBuilder WithTools(this IMcpServerBuilder builder, params IEnumerable toolTypes) { Throw.IfNull(builder); @@ -56,18 +64,7 @@ public static partial class McpServerBuilderExtensions throw new ArgumentNullException(nameof(toolTypes), $"A tool type provided by the enumerator was null."); } - foreach (var method in toolType.GetMethods(BindingFlags.Public | BindingFlags.Static)) - { - if (method.GetCustomAttribute() is not { } attribute) - { - continue; - } - - functions.Add(AIFunctionFactory.Create(method, target: null, new() - { - Name = attribute.Name ?? method.Name, - })); - } + PopulateFunctions(toolType, functions); } return WithTools(builder, functions); @@ -99,7 +96,7 @@ public static partial class McpServerBuilderExtensions { Name = function.Name, Description = function.Description, - InputSchema = JsonSerializer.Deserialize(function.JsonSchema), + InputSchema = JsonSerializer.Deserialize(function.JsonSchema, JsonSerializerOptionsExtensions.JsonContext.Default.JsonSchema), }); callbacks.Add(function.Name, async (request, cancellationToken) => @@ -155,6 +152,7 @@ public static partial class McpServerBuilderExtensions /// The builder instance. /// The assembly to load the types from. Null to get the current assembly /// is . + [RequiresUnreferencedCode(RequiresUnreferencedCodeMessage)] public static IMcpServerBuilder WithToolsFromAssembly(this IMcpServerBuilder builder, Assembly? assembly = null) { assembly ??= Assembly.GetCallingAssembly(); @@ -182,4 +180,22 @@ public static partial class McpServerBuilderExtensions WithTools(builder, toolTypes) : builder; } + + private static void PopulateFunctions( + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] Type toolType, + List functions) + { + foreach (var method in toolType.GetMethods(BindingFlags.Public | BindingFlags.Static)) + { + if (method.GetCustomAttribute() is not { } attribute) + { + continue; + } + + functions.Add(AIFunctionFactory.Create(method, target: null, new() + { + Name = attribute.Name ?? method.Name, + })); + } + } } diff --git a/src/mcpdotnet/Protocol/Messages/IJsonRpcMessage.cs b/src/mcpdotnet/Protocol/Messages/IJsonRpcMessage.cs index a5e8a76..9fb9fab 100644 --- a/src/mcpdotnet/Protocol/Messages/IJsonRpcMessage.cs +++ b/src/mcpdotnet/Protocol/Messages/IJsonRpcMessage.cs @@ -1,8 +1,12 @@ -namespace McpDotNet.Protocol.Messages; +using McpDotNet.Utils.Json; +using System.Text.Json.Serialization; + +namespace McpDotNet.Protocol.Messages; /// /// Base interface for all JSON-RPC messages in the MCP protocol. /// +[JsonConverter(typeof(JsonRpcMessageConverter))] public interface IJsonRpcMessage { /// diff --git a/src/mcpdotnet/Protocol/Transport/HttpListenerSseServerTransport.cs b/src/mcpdotnet/Protocol/Transport/HttpListenerSseServerTransport.cs index 52e6b96..6f73cde 100644 --- a/src/mcpdotnet/Protocol/Transport/HttpListenerSseServerTransport.cs +++ b/src/mcpdotnet/Protocol/Transport/HttpListenerSseServerTransport.cs @@ -77,7 +77,7 @@ public sealed class HttpListenerSseServerTransport : TransportBase, IServerTrans try { - var json = JsonSerializer.Serialize(message, _jsonOptions); + var json = JsonSerializer.Serialize(message, _jsonOptions.GetTypeInfo()); _logger.TransportSendingMessage(EndpointName, id, json); await _httpServerProvider.SendEvent(json, "message").ConfigureAwait(false); @@ -125,7 +125,7 @@ public sealed class HttpListenerSseServerTransport : TransportBase, IServerTrans try { - var message = JsonSerializer.Deserialize(request, _jsonOptions); + var message = JsonSerializer.Deserialize(request, _jsonOptions.GetTypeInfo()); if (message != null) { // Fire-and-forget the message to the message channel diff --git a/src/mcpdotnet/Protocol/Transport/SseClientTransport.cs b/src/mcpdotnet/Protocol/Transport/SseClientTransport.cs index 9759f3e..a45dc60 100644 --- a/src/mcpdotnet/Protocol/Transport/SseClientTransport.cs +++ b/src/mcpdotnet/Protocol/Transport/SseClientTransport.cs @@ -110,7 +110,7 @@ public sealed class SseClientTransport : TransportBase, IClientTransport throw new InvalidOperationException("Transport not connected"); using var content = new StringContent( - JsonSerializer.Serialize(message, _jsonOptions), + JsonSerializer.Serialize(message, _jsonOptions.GetTypeInfo()), Encoding.UTF8, "application/json" ); @@ -143,7 +143,7 @@ public sealed class SseClientTransport : TransportBase, IClientTransport } else { - JsonRpcResponse initializeResponse = JsonSerializer.Deserialize(responseContent, _jsonOptions) ?? + JsonRpcResponse initializeResponse = JsonSerializer.Deserialize(responseContent, _jsonOptions.GetTypeInfo()) ?? throw new McpTransportException("Failed to initialize client"); _logger.TransportReceivedMessageParsed(EndpointName, messageId); @@ -277,7 +277,7 @@ public sealed class SseClientTransport : TransportBase, IClientTransport try { - var message = JsonSerializer.Deserialize(data, _jsonOptions); + var message = JsonSerializer.Deserialize(data, _jsonOptions.GetTypeInfo()); if (message == null) { _logger.TransportMessageParseUnexpectedType(EndpointName, data); diff --git a/src/mcpdotnet/Protocol/Transport/StdioClientTransport.cs b/src/mcpdotnet/Protocol/Transport/StdioClientTransport.cs index d8613d5..225e1d2 100644 --- a/src/mcpdotnet/Protocol/Transport/StdioClientTransport.cs +++ b/src/mcpdotnet/Protocol/Transport/StdioClientTransport.cs @@ -134,7 +134,7 @@ public sealed class StdioClientTransport : TransportBase, IClientTransport try { - var json = JsonSerializer.Serialize(message, _jsonOptions); + var json = JsonSerializer.Serialize(message, _jsonOptions.GetTypeInfo()); _logger.TransportSendingMessage(EndpointName, id, json); // Write the message followed by a newline @@ -206,7 +206,7 @@ public sealed class StdioClientTransport : TransportBase, IClientTransport try { line=line.Trim();//Fixes an error when the service prefixes nonprintable characters - var message = JsonSerializer.Deserialize(line, _jsonOptions); + var message = JsonSerializer.Deserialize(line, _jsonOptions.GetTypeInfo()); if (message != null) { string messageId = "(no id)"; diff --git a/src/mcpdotnet/Protocol/Transport/StdioServerTransport.cs b/src/mcpdotnet/Protocol/Transport/StdioServerTransport.cs index 46785ca..2c3d87c 100644 --- a/src/mcpdotnet/Protocol/Transport/StdioServerTransport.cs +++ b/src/mcpdotnet/Protocol/Transport/StdioServerTransport.cs @@ -117,7 +117,7 @@ public sealed class StdioServerTransport : TransportBase, IServerTransport try { - var json = JsonSerializer.Serialize(message, _jsonOptions); + var json = JsonSerializer.Serialize(message, _jsonOptions.GetTypeInfo()); _logger.TransportSendingMessage(EndpointName, id, json); await _stdout.WriteLineAsync(json.AsMemory(), cancellationToken).ConfigureAwait(false); @@ -166,7 +166,7 @@ public sealed class StdioServerTransport : TransportBase, IServerTransport try { - var message = JsonSerializer.Deserialize(line, _jsonOptions); + var message = JsonSerializer.Deserialize(line, _jsonOptions.GetTypeInfo()); if (message != null) { string messageId = "(no id)"; diff --git a/src/mcpdotnet/Shared/McpJsonRpcEndpoint.cs b/src/mcpdotnet/Shared/McpJsonRpcEndpoint.cs index cd7647f..3084c1d 100644 --- a/src/mcpdotnet/Shared/McpJsonRpcEndpoint.cs +++ b/src/mcpdotnet/Shared/McpJsonRpcEndpoint.cs @@ -95,7 +95,7 @@ internal abstract class McpJsonRpcEndpoint : IAsyncDisposable } catch (Exception ex) { - var payload = JsonSerializer.Serialize(message); + var payload = JsonSerializer.Serialize(message, _jsonOptions.GetTypeInfo()); _logger.MessageHandlerError(EndpointName, message.GetType().Name, payload, ex); } } @@ -236,7 +236,7 @@ internal abstract class McpJsonRpcEndpoint : IAsyncDisposable // Expensive logging, use the logging framework to check if the logger is enabled if (_logger.IsEnabled(LogLevel.Debug)) { - _logger.SendingRequestPayload(EndpointName, JsonSerializer.Serialize(request)); + _logger.SendingRequestPayload(EndpointName, JsonSerializer.Serialize(request, _jsonOptions.GetTypeInfo())); } // Less expensive information logging @@ -256,8 +256,8 @@ internal abstract class McpJsonRpcEndpoint : IAsyncDisposable if (response is JsonRpcResponse success) { // Convert the Result object to JSON and back to get our strongly-typed result - var resultJson = JsonSerializer.Serialize(success.Result, _jsonOptions); - var resultObject = JsonSerializer.Deserialize(resultJson, _jsonOptions); + var resultJson = JsonSerializer.Serialize(success.Result, _jsonOptions.GetTypeInfo()); + var resultObject = JsonSerializer.Deserialize(resultJson, _jsonOptions.GetTypeInfo()); // Not expensive logging because we're already converting to JSON in order to get the result object _logger.RequestResponseReceivedPayload(EndpointName, resultJson); @@ -270,7 +270,7 @@ internal abstract class McpJsonRpcEndpoint : IAsyncDisposable // Result object was null, this is unexpected _logger.RequestResponseTypeConversionError(EndpointName, request.Method, typeof(TResult)); - throw new McpClientException($"Unexpected response type {JsonSerializer.Serialize(success.Result)}, expected {typeof(TResult)}"); + throw new McpClientException($"Unexpected response type {JsonSerializer.Serialize(success.Result, _jsonOptions.GetTypeInfo())}, expected {typeof(TResult)}"); } // Unexpected response type @@ -295,7 +295,7 @@ internal abstract class McpJsonRpcEndpoint : IAsyncDisposable if (_logger.IsEnabled(LogLevel.Debug)) { - _logger.SendingMessage(EndpointName, JsonSerializer.Serialize(message)); + _logger.SendingMessage(EndpointName, JsonSerializer.Serialize(message, _jsonOptions.GetTypeInfo())); } return _transport.SendMessageAsync(message, cancellationToken); @@ -339,8 +339,8 @@ internal abstract class McpJsonRpcEndpoint : IAsyncDisposable _requestHandlers[method] = async (request, cancellationToken) => { // Convert the params JsonElement to our type using the same options - var jsonString = JsonSerializer.Serialize(request.Params); - var typedRequest = JsonSerializer.Deserialize(jsonString, _jsonOptions); + var jsonString = JsonSerializer.Serialize(request.Params, _jsonOptions.GetTypeInfo()); + var typedRequest = JsonSerializer.Deserialize(jsonString, _jsonOptions.GetTypeInfo()); return await handler(typedRequest, cancellationToken).ConfigureAwait(false); }; diff --git a/src/mcpdotnet/Utils/Json/JsonRpcMessageConverter.cs b/src/mcpdotnet/Utils/Json/JsonRpcMessageConverter.cs index db84e4f..9b0c0e1 100644 --- a/src/mcpdotnet/Utils/Json/JsonRpcMessageConverter.cs +++ b/src/mcpdotnet/Utils/Json/JsonRpcMessageConverter.cs @@ -1,13 +1,15 @@ -using System.Text.Json; +using McpDotNet.Protocol.Messages; +using System.ComponentModel; +using System.Text.Json; using System.Text.Json.Serialization; -using McpDotNet.Protocol.Messages; namespace McpDotNet.Utils.Json; /// /// JSON converter for IJsonRpcMessage that handles polymorphic deserialization of different message types. /// -internal sealed class JsonRpcMessageConverter : JsonConverter +[EditorBrowsable(EditorBrowsableState.Never)] +public sealed class JsonRpcMessageConverter : JsonConverter { /// public override IJsonRpcMessage? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) @@ -40,13 +42,13 @@ internal sealed class JsonRpcMessageConverter : JsonConverter // Messages with an error property are error responses if (hasError) { - return JsonSerializer.Deserialize(rawText, options); + return JsonSerializer.Deserialize(rawText, options.GetTypeInfo()); } // Messages with a result property are success responses if (root.TryGetProperty("result", out _)) { - return JsonSerializer.Deserialize(rawText, options); + return JsonSerializer.Deserialize(rawText, options.GetTypeInfo()); } throw new JsonException("Response must have either result or error"); @@ -55,13 +57,13 @@ internal sealed class JsonRpcMessageConverter : JsonConverter // Messages with a method but no id are notifications if (hasMethod && !hasId) { - return JsonSerializer.Deserialize(rawText, options); + return JsonSerializer.Deserialize(rawText, options.GetTypeInfo()); } // Messages with both method and id are requests if (hasMethod && hasId) { - return JsonSerializer.Deserialize(rawText, options); + return JsonSerializer.Deserialize(rawText, options.GetTypeInfo()); } throw new JsonException("Invalid JSON-RPC message format"); @@ -73,16 +75,16 @@ internal sealed class JsonRpcMessageConverter : JsonConverter switch (value) { case JsonRpcRequest request: - JsonSerializer.Serialize(writer, request, options); + JsonSerializer.Serialize(writer, request, options.GetTypeInfo()); break; case JsonRpcNotification notification: - JsonSerializer.Serialize(writer, notification, options); + JsonSerializer.Serialize(writer, notification, options.GetTypeInfo()); break; case JsonRpcResponse response: - JsonSerializer.Serialize(writer, response, options); + JsonSerializer.Serialize(writer, response, options.GetTypeInfo()); break; case JsonRpcError error: - JsonSerializer.Serialize(writer, error, options); + JsonSerializer.Serialize(writer, error, options.GetTypeInfo()); break; default: throw new JsonException($"Unknown JSON-RPC message type: {value.GetType()}"); diff --git a/src/mcpdotnet/Utils/Json/JsonSerializerOptionsExtensions.cs b/src/mcpdotnet/Utils/Json/JsonSerializerOptionsExtensions.cs index 16e3047..83d34ca 100644 --- a/src/mcpdotnet/Utils/Json/JsonSerializerOptionsExtensions.cs +++ b/src/mcpdotnet/Utils/Json/JsonSerializerOptionsExtensions.cs @@ -1,12 +1,17 @@ -using System.Text.Json; +using McpDotNet.Protocol.Messages; +using McpDotNet.Protocol.Types; +using System.Diagnostics.CodeAnalysis; +using System.Text.Json; +using System.Text.Json.Nodes; using System.Text.Json.Serialization; +using System.Text.Json.Serialization.Metadata; namespace McpDotNet.Utils.Json; /// /// Extensions for configuring System.Text.Json serialization options for MCP. /// -internal static class JsonSerializerOptionsExtensions +internal static partial class JsonSerializerOptionsExtensions { public static JsonSerializerOptions DefaultOptions { get; } = CreateDefaultOptions(); @@ -14,19 +19,54 @@ internal static class JsonSerializerOptionsExtensions /// Creates default options to use for MCP-related serialization. /// /// The configured options. + [UnconditionalSuppressMessage("AotAnalysis", "IL3050", Justification = "DefaultJsonTypeInfoResolver is only used when reflection-based serialization is enabled")] + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026", Justification = "DefaultJsonTypeInfoResolver is only used when reflection-based serialization is enabled")] private static JsonSerializerOptions CreateDefaultOptions() { - JsonSerializerOptions options = new(); + // If reflection-based serialization is enabled by default, use it, as it's the most permissive in terms of what it can serialize, + // and we want to be flexible in terms of what can be put into the various collections in the object model. + // Otherwise, use the source-generated options to enable trimming and Native AOT. - // Add custom converters - options.Converters.Add(new JsonRpcMessageConverter()); - options.Converters.Add(new JsonStringEnumConverter(JsonNamingPolicy.CamelCase)); + if (JsonSerializer.IsReflectionEnabledByDefault) + { + // Keep in sync with the JsonSourceGenerationOptions attribute on JsonContext below. + JsonSerializerOptions options = new(JsonSerializerDefaults.Web) + { + TypeInfoResolver = new DefaultJsonTypeInfoResolver(), + Converters = { new JsonStringEnumConverter() }, + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, + NumberHandling = JsonNumberHandling.AllowReadingFromString + }; - // Configure general options - options.PropertyNameCaseInsensitive = true; - options.DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull; - options.NumberHandling = JsonNumberHandling.AllowReadingFromString; + options.MakeReadOnly(); + return options; + } - return options; + return JsonContext.Default.Options; } + + internal static JsonTypeInfo GetTypeInfo(this JsonSerializerOptions options) => + (JsonTypeInfo)options.GetTypeInfo(typeof(T)); + + // Keep in sync with CreateDefaultOptions above. + [JsonSourceGenerationOptions(JsonSerializerDefaults.Web, + UseStringEnumConverter = true, + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, + NumberHandling = JsonNumberHandling.AllowReadingFromString)] + [JsonSerializable(typeof(JsonDocument))] + [JsonSerializable(typeof(JsonElement))] + [JsonSerializable(typeof(JsonNode))] + [JsonSerializable(typeof(IJsonRpcMessage))] + [JsonSerializable(typeof(JsonRpcRequest))] + [JsonSerializable(typeof(JsonRpcNotification))] + [JsonSerializable(typeof(JsonRpcResponse))] + [JsonSerializable(typeof(JsonRpcError))] + [JsonSerializable(typeof(ServerCapabilities))] + [JsonSerializable(typeof(Implementation))] + [JsonSerializable(typeof(CreateMessageResult))] + [JsonSerializable(typeof(ListRootsResult))] + [JsonSerializable(typeof(InitializeResult))] + [JsonSerializable(typeof(JsonSchema))] + [JsonSerializable(typeof(CallToolResponse))] + internal sealed partial class JsonContext : JsonSerializerContext; } diff --git a/src/mcpdotnet/mcpdotnet.csproj b/src/mcpdotnet/mcpdotnet.csproj index 4e1b538..4e2ea9c 100644 --- a/src/mcpdotnet/mcpdotnet.csproj +++ b/src/mcpdotnet/mcpdotnet.csproj @@ -9,6 +9,10 @@ All + + true + + mcpdotnet diff --git a/tests/mcpdotnet.TestSseServer/Program.cs b/tests/mcpdotnet.TestSseServer/Program.cs index dd77be8..da44b6a 100644 --- a/tests/mcpdotnet.TestSseServer/Program.cs +++ b/tests/mcpdotnet.TestSseServer/Program.cs @@ -2,7 +2,6 @@ using McpDotNet.Protocol.Types; using McpDotNet.Server; using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Abstractions; using Serilog; using System.Text; diff --git a/tests/mcpdotnet.Tests/Configuration/McpServerBuilderExtensionsToolsTests.cs b/tests/mcpdotnet.Tests/Configuration/McpServerBuilderExtensionsToolsTests.cs index 5885a67..a9da91e 100644 --- a/tests/mcpdotnet.Tests/Configuration/McpServerBuilderExtensionsToolsTests.cs +++ b/tests/mcpdotnet.Tests/Configuration/McpServerBuilderExtensionsToolsTests.cs @@ -287,7 +287,7 @@ public class McpServerBuilderExtensionsToolsTests var tool = result.Tools.First(t => t.Name == "TestTool"); Assert.Equal("TestTool", tool.Name); - Assert.Empty(tool.Description); + Assert.Empty(tool.Description!); Assert.NotNull(tool.InputSchema); Assert.Equal("object", tool.InputSchema.Type); Assert.NotNull(tool.InputSchema.Properties); diff --git a/tests/mcpdotnet.Tests/Server/McpServerTests.cs b/tests/mcpdotnet.Tests/Server/McpServerTests.cs index 53579bf..68fe14d 100644 --- a/tests/mcpdotnet.Tests/Server/McpServerTests.cs +++ b/tests/mcpdotnet.Tests/Server/McpServerTests.cs @@ -603,7 +603,7 @@ public class McpServerTests supportsSampling ? new ClientCapabilities { Sampling = new SamplingCapability() } : null; - public async Task SendRequestAsync(JsonRpcRequest request, CancellationToken cancellationToken) where T : class + public Task SendRequestAsync(JsonRpcRequest request, CancellationToken cancellationToken) where T : class { CreateMessageRequestParams rp = Assert.IsType(request.Params); @@ -627,7 +627,7 @@ public class McpServerTests Role = "assistant", StopReason = "endTurn", }; - return (T)(object)result; + return Task.FromResult((T)(object)result); } public ValueTask DisposeAsync() => default; diff --git a/tests/mcpdotnet.Tests/SseServerIntegrationTestFixture.cs b/tests/mcpdotnet.Tests/SseServerIntegrationTestFixture.cs index daec16d..a82017e 100644 --- a/tests/mcpdotnet.Tests/SseServerIntegrationTestFixture.cs +++ b/tests/mcpdotnet.Tests/SseServerIntegrationTestFixture.cs @@ -3,6 +3,7 @@ using McpDotNet.Configuration; using McpDotNet.Protocol.Transport; using Microsoft.Extensions.Logging; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; namespace McpDotNet.Tests; @@ -37,6 +38,7 @@ public class SseServerIntegrationTestFixture : IDisposable Start(); } + [MemberNotNull(nameof(_process))] public void Start() { // Start the server (which is at TestSseServer.exe on windows and "dotnet TestSseServer.dll" on linux)