Hide experimental types from external source generators using internal property pattern (#1301)
This commit is contained in:
parent
231e013160
commit
cc6ef5b926
12 changed files with 315 additions and 7 deletions
|
|
@ -78,5 +78,6 @@
|
|||
<Project Path="tests/ModelContextProtocol.Tests/ModelContextProtocol.Tests.csproj" />
|
||||
<Project Path="tests/ModelContextProtocol.TestServer/ModelContextProtocol.TestServer.csproj" />
|
||||
<Project Path="tests/ModelContextProtocol.TestSseServer/ModelContextProtocol.TestSseServer.csproj" />
|
||||
<Project Path="tests/ModelContextProtocol.ExperimentalApiRegressionTest/ModelContextProtocol.ExperimentalApiRegressionTest.csproj" />
|
||||
</Folder>
|
||||
</Solution>
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
using System.Diagnostics.CodeAnalysis;
|
||||
using System.Text.Json;
|
||||
using System.Text.Json.Serialization;
|
||||
|
||||
|
|
@ -33,6 +34,16 @@ public sealed class CallToolRequestParams : RequestParams
|
|||
/// When present, indicates that the requestor wants this operation executed as a task.
|
||||
/// The receiver must support task augmentation for this specific request type.
|
||||
/// </remarks>
|
||||
[Experimental(Experimentals.Tasks_DiagnosticId, UrlFormat = Experimentals.Tasks_Url)]
|
||||
[JsonIgnore]
|
||||
public McpTaskMetadata? Task
|
||||
{
|
||||
get => TaskCore;
|
||||
set => TaskCore = value;
|
||||
}
|
||||
|
||||
// See ExperimentalInternalPropertyTests.cs before modifying this property.
|
||||
[JsonInclude]
|
||||
[JsonPropertyName("task")]
|
||||
public McpTaskMetadata? Task { get; set; }
|
||||
internal McpTaskMetadata? TaskCore { get; set; }
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
using System.Diagnostics.CodeAnalysis;
|
||||
using System.Text.Json.Nodes;
|
||||
using System.Text.Json.Serialization;
|
||||
|
||||
|
|
@ -72,6 +73,16 @@ public sealed class CallToolResult : Result
|
|||
/// (<see cref="Content"/>, <see cref="StructuredContent"/>, <see cref="IsError"/>) may not be populated.
|
||||
/// The actual tool result can be retrieved later via <c>tasks/result</c>.
|
||||
/// </remarks>
|
||||
[Experimental(Experimentals.Tasks_DiagnosticId, UrlFormat = Experimentals.Tasks_Url)]
|
||||
[JsonIgnore]
|
||||
public McpTask? Task
|
||||
{
|
||||
get => TaskCore;
|
||||
set => TaskCore = value;
|
||||
}
|
||||
|
||||
// See ExperimentalInternalPropertyTests.cs before modifying this property.
|
||||
[JsonInclude]
|
||||
[JsonPropertyName("task")]
|
||||
public McpTask? Task { get; set; }
|
||||
internal McpTask? TaskCore { get; set; }
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
using System.ComponentModel;
|
||||
using System.Diagnostics.CodeAnalysis;
|
||||
using System.Text.Json.Serialization;
|
||||
using ModelContextProtocol.Client;
|
||||
using ModelContextProtocol.Server;
|
||||
|
|
@ -80,6 +81,16 @@ public sealed class ClientCapabilities
|
|||
/// See <see cref="McpTasksCapability"/> for details on configuring which operations support tasks.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
[Experimental(Experimentals.Tasks_DiagnosticId, UrlFormat = Experimentals.Tasks_Url)]
|
||||
[JsonIgnore]
|
||||
public McpTasksCapability? Tasks
|
||||
{
|
||||
get => TasksCore;
|
||||
set => TasksCore = value;
|
||||
}
|
||||
|
||||
// See ExperimentalInternalPropertyTests.cs before modifying this property.
|
||||
[JsonInclude]
|
||||
[JsonPropertyName("tasks")]
|
||||
public McpTasksCapability? Tasks { get; set; }
|
||||
internal McpTasksCapability? TasksCore { get; set; }
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
using System.Diagnostics.CodeAnalysis;
|
||||
using System.Text.Json;
|
||||
using System.Text.Json.Serialization;
|
||||
|
||||
|
|
@ -160,6 +161,16 @@ public sealed class CreateMessageRequestParams : RequestParams
|
|||
/// When present, indicates that the requestor wants this operation executed as a task.
|
||||
/// The receiver must support task augmentation for this specific request type.
|
||||
/// </remarks>
|
||||
[Experimental(Experimentals.Tasks_DiagnosticId, UrlFormat = Experimentals.Tasks_Url)]
|
||||
[JsonIgnore]
|
||||
public McpTaskMetadata? Task
|
||||
{
|
||||
get => TaskCore;
|
||||
set => TaskCore = value;
|
||||
}
|
||||
|
||||
// See ExperimentalInternalPropertyTests.cs before modifying this property.
|
||||
[JsonInclude]
|
||||
[JsonPropertyName("task")]
|
||||
public McpTaskMetadata? Task { get; set; }
|
||||
internal McpTaskMetadata? TaskCore { get; set; }
|
||||
}
|
||||
|
|
|
|||
|
|
@ -99,8 +99,18 @@ public sealed class ElicitRequestParams : RequestParams
|
|||
/// When present, indicates that the requestor wants this operation executed as a task.
|
||||
/// The receiver must support task augmentation for this specific request type.
|
||||
/// </remarks>
|
||||
[Experimental(Experimentals.Tasks_DiagnosticId, UrlFormat = Experimentals.Tasks_Url)]
|
||||
[JsonIgnore]
|
||||
public McpTaskMetadata? Task
|
||||
{
|
||||
get => TaskCore;
|
||||
set => TaskCore = value;
|
||||
}
|
||||
|
||||
// See ExperimentalInternalPropertyTests.cs before modifying this property.
|
||||
[JsonInclude]
|
||||
[JsonPropertyName("task")]
|
||||
public McpTaskMetadata? Task { get; set; }
|
||||
internal McpTaskMetadata? TaskCore { get; set; }
|
||||
|
||||
/// <summary>Represents a request schema used in a form mode elicitation request.</summary>
|
||||
public sealed class RequestSchema
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
using System.ComponentModel;
|
||||
using System.Diagnostics.CodeAnalysis;
|
||||
using System.Text.Json.Serialization;
|
||||
using ModelContextProtocol.Server;
|
||||
|
||||
|
|
@ -79,6 +80,16 @@ public sealed class ServerCapabilities
|
|||
/// See <see cref="McpTasksCapability"/> for details on configuring which operations support tasks.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
[Experimental(Experimentals.Tasks_DiagnosticId, UrlFormat = Experimentals.Tasks_Url)]
|
||||
[JsonIgnore]
|
||||
public McpTasksCapability? Tasks
|
||||
{
|
||||
get => TasksCore;
|
||||
set => TasksCore = value;
|
||||
}
|
||||
|
||||
// See ExperimentalInternalPropertyTests.cs before modifying this property.
|
||||
[JsonInclude]
|
||||
[JsonPropertyName("tasks")]
|
||||
public McpTasksCapability? Tasks { get; set; }
|
||||
internal McpTasksCapability? TasksCore { get; set; }
|
||||
}
|
||||
|
|
|
|||
|
|
@ -128,8 +128,17 @@ public sealed class Tool : IBaseMetadata
|
|||
/// regarding task augmentation support. See <see cref="ToolExecution"/> for details.
|
||||
/// </remarks>
|
||||
[Experimental(Experimentals.Tasks_DiagnosticId, UrlFormat = Experimentals.Tasks_Url)]
|
||||
[JsonIgnore]
|
||||
public ToolExecution? Execution
|
||||
{
|
||||
get => ExecutionCore;
|
||||
set => ExecutionCore = value;
|
||||
}
|
||||
|
||||
// See ExperimentalInternalPropertyTests.cs before modifying this property.
|
||||
[JsonInclude]
|
||||
[JsonPropertyName("execution")]
|
||||
public ToolExecution? Execution { get; set; }
|
||||
internal ToolExecution? ExecutionCore { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// Gets or sets an optional list of icons for this tool.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,17 @@
|
|||
using System.Text.Json.Serialization;
|
||||
using ModelContextProtocol.Protocol;
|
||||
|
||||
namespace ModelContextProtocol.ExperimentalApiRegressionTest;
|
||||
|
||||
/// <summary>
|
||||
/// This file validates that the System.Text.Json source generator does not produce
|
||||
/// MCPEXP001 diagnostics for MCP protocol types with experimental properties.
|
||||
/// </summary>
|
||||
[JsonSerializable(typeof(Tool))]
|
||||
[JsonSerializable(typeof(ServerCapabilities))]
|
||||
[JsonSerializable(typeof(ClientCapabilities))]
|
||||
[JsonSerializable(typeof(CallToolResult))]
|
||||
[JsonSerializable(typeof(CallToolRequestParams))]
|
||||
[JsonSerializable(typeof(CreateMessageRequestParams))]
|
||||
[JsonSerializable(typeof(ElicitRequestParams))]
|
||||
internal partial class ExperimentalPropertyRegressionContext : JsonSerializerContext;
|
||||
|
|
@ -0,0 +1,29 @@
|
|||
<Project Sdk="Microsoft.NET.Sdk">
|
||||
|
||||
<PropertyGroup>
|
||||
<TargetFrameworks>net10.0;net9.0;net8.0</TargetFrameworks>
|
||||
<ImplicitUsings>enable</ImplicitUsings>
|
||||
<Nullable>enable</Nullable>
|
||||
<IsPackable>false</IsPackable>
|
||||
|
||||
<!--
|
||||
This project validates that MCPEXP001 diagnostics are NOT emitted by the
|
||||
System.Text.Json source generator for types with experimental properties.
|
||||
tests/Directory.Build.props suppresses MCPEXP001 globally for all test projects,
|
||||
but this project must NOT suppress it: its successful compilation IS the test.
|
||||
-->
|
||||
<NoWarn>$(NoWarn.Replace('MCPEXP001',''))</NoWarn>
|
||||
|
||||
<!--
|
||||
[JsonInclude] internal members trigger this warning only because the MCP C# SDK
|
||||
is a project reference here. NuGet consumers won't see it since Roslyn doesn't import
|
||||
internal members from metadata.
|
||||
-->
|
||||
<NoWarn>$(NoWarn);SYSLIB1038</NoWarn>
|
||||
</PropertyGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<ProjectReference Include="..\..\src\ModelContextProtocol.Core\ModelContextProtocol.Core.csproj" />
|
||||
</ItemGroup>
|
||||
|
||||
</Project>
|
||||
|
|
@ -0,0 +1,69 @@
|
|||
using System.Reflection;
|
||||
using System.Text.Json.Serialization;
|
||||
using ModelContextProtocol.Protocol;
|
||||
|
||||
namespace ModelContextProtocol.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Validates the internal property pattern used for experimental MCP properties.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Experimental properties on stable protocol types must use an internal serialization
|
||||
/// property to hide the experimental type from external source generators. The public
|
||||
/// property is marked [Experimental][JsonIgnore] and delegates to an internal *Core
|
||||
/// property marked [JsonInclude][JsonPropertyName].
|
||||
/// </remarks>
|
||||
public class ExperimentalInternalPropertyTests
|
||||
{
|
||||
[Fact]
|
||||
public void ExperimentalProperties_MustBeHiddenFromSourceGenerator()
|
||||
{
|
||||
// [Experimental] properties on stable protocol types must use the internal property
|
||||
// pattern so the STJ source generator does not reference experimental types in
|
||||
// generated code (which would trigger MCPEXP001 for consumers).
|
||||
//
|
||||
// Required pattern:
|
||||
// 1. Mark the public property [Experimental][JsonIgnore]
|
||||
// 2. Add an internal *Core property with [JsonInclude][JsonPropertyName]
|
||||
// that the public property delegates to
|
||||
//
|
||||
// To stabilize:
|
||||
// 1. Remove [Experimental] and [JsonIgnore] from the public property
|
||||
// 2. Add [JsonPropertyName] to the public property
|
||||
// 3. Convert to auto-property
|
||||
// 4. Remove the internal *Core property
|
||||
|
||||
foreach (var (type, prop) in GetExperimentalPropertiesOnStableTypes())
|
||||
{
|
||||
Assert.True(
|
||||
prop.GetCustomAttribute<JsonIgnoreAttribute>() is not null,
|
||||
$"{type.Name}.{prop.Name} is [Experimental] but missing [JsonIgnore].");
|
||||
|
||||
Assert.True(
|
||||
prop.GetCustomAttribute<JsonPropertyNameAttribute>() is null,
|
||||
$"{type.Name}.{prop.Name} is [Experimental] and must not have [JsonPropertyName].");
|
||||
}
|
||||
}
|
||||
|
||||
private static IEnumerable<(Type Type, PropertyInfo Property)> GetExperimentalPropertiesOnStableTypes()
|
||||
{
|
||||
var protocolTypes = typeof(Tool).Assembly.GetTypes()
|
||||
.Where(t => t.Namespace == "ModelContextProtocol.Protocol" && t.IsClass && !t.IsAbstract);
|
||||
|
||||
foreach (var type in protocolTypes)
|
||||
{
|
||||
if (type.GetCustomAttributes().Any(a => a.GetType().Name == "ExperimentalAttribute"))
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
foreach (var prop in type.GetProperties(BindingFlags.Public | BindingFlags.Instance))
|
||||
{
|
||||
if (prop.GetCustomAttributes().Any(a => a.GetType().Name == "ExperimentalAttribute"))
|
||||
{
|
||||
yield return (type, prop);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,118 @@
|
|||
using System.Text.Json;
|
||||
using System.Text.Json.Serialization;
|
||||
using ModelContextProtocol.Protocol;
|
||||
|
||||
namespace ModelContextProtocol.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Validates that the internal property pattern used for experimental properties
|
||||
/// produces the expected serialization behavior for SDK consumers using source generators.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// Experimental properties (e.g. <see cref="Tool.Execution"/>, <see cref="ServerCapabilities.Tasks"/>)
|
||||
/// use an internal <c>*Core</c> property for serialization. A consumer's source-generated
|
||||
/// <see cref="JsonSerializerContext"/> cannot see internal members, so experimental data is
|
||||
/// silently dropped unless the consumer chains the SDK's resolver into their options.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// These tests depend on <see cref="Tool.Execution"/> and <see cref="ServerCapabilities.Tasks"/>
|
||||
/// being experimental. When those APIs stabilize, update these tests to reference whatever
|
||||
/// experimental properties exist at that time, or remove them entirely if no experimental
|
||||
/// APIs remain.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
public class ExperimentalPropertySerializationTests
|
||||
{
|
||||
[Fact]
|
||||
public void ExperimentalProperties_Dropped_WithConsumerContextOnly()
|
||||
{
|
||||
var options = new JsonSerializerOptions
|
||||
{
|
||||
TypeInfoResolverChain = { ConsumerJsonContext.Default }
|
||||
};
|
||||
|
||||
var tool = new Tool
|
||||
{
|
||||
Name = "test-tool",
|
||||
Execution = new ToolExecution { TaskSupport = ToolTaskSupport.Optional }
|
||||
};
|
||||
|
||||
string json = JsonSerializer.Serialize(tool, options);
|
||||
Assert.DoesNotContain("\"execution\"", json);
|
||||
Assert.Contains("\"name\"", json);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ExperimentalProperties_IgnoredOnDeserialize_WithConsumerContextOnly()
|
||||
{
|
||||
string json = JsonSerializer.Serialize(
|
||||
new Tool
|
||||
{
|
||||
Name = "test-tool",
|
||||
Execution = new ToolExecution { TaskSupport = ToolTaskSupport.Optional }
|
||||
},
|
||||
McpJsonUtilities.DefaultOptions);
|
||||
Assert.Contains("\"execution\"", json);
|
||||
|
||||
var options = new JsonSerializerOptions
|
||||
{
|
||||
TypeInfoResolverChain = { ConsumerJsonContext.Default }
|
||||
};
|
||||
var deserialized = JsonSerializer.Deserialize<Tool>(json, options)!;
|
||||
Assert.Equal("test-tool", deserialized.Name);
|
||||
Assert.Null(deserialized.Execution);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ExperimentalProperties_RoundTrip_WhenSdkResolverIsChained()
|
||||
{
|
||||
var options = new JsonSerializerOptions
|
||||
{
|
||||
TypeInfoResolverChain =
|
||||
{
|
||||
McpJsonUtilities.DefaultOptions.TypeInfoResolver!,
|
||||
ConsumerJsonContext.Default,
|
||||
}
|
||||
};
|
||||
|
||||
var tool = new Tool
|
||||
{
|
||||
Name = "test-tool",
|
||||
Execution = new ToolExecution { TaskSupport = ToolTaskSupport.Optional }
|
||||
};
|
||||
|
||||
string json = JsonSerializer.Serialize(tool, options);
|
||||
Assert.Contains("\"execution\"", json);
|
||||
Assert.Contains("\"name\"", json);
|
||||
|
||||
var deserialized = JsonSerializer.Deserialize<Tool>(json, options)!;
|
||||
Assert.Equal("test-tool", deserialized.Name);
|
||||
Assert.NotNull(deserialized.Execution);
|
||||
Assert.Equal(ToolTaskSupport.Optional, deserialized.Execution.TaskSupport);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ExperimentalProperties_RoundTrip_WithDefaultOptions()
|
||||
{
|
||||
var capabilities = new ServerCapabilities
|
||||
{
|
||||
Tasks = new McpTasksCapability()
|
||||
};
|
||||
|
||||
string json = JsonSerializer.Serialize(capabilities, McpJsonUtilities.DefaultOptions);
|
||||
Assert.Contains("\"tasks\"", json);
|
||||
|
||||
var deserialized = JsonSerializer.Deserialize<ServerCapabilities>(json, McpJsonUtilities.DefaultOptions)!;
|
||||
Assert.NotNull(deserialized.Tasks);
|
||||
}
|
||||
}
|
||||
|
||||
[JsonSerializable(typeof(Tool))]
|
||||
[JsonSerializable(typeof(ServerCapabilities))]
|
||||
[JsonSerializable(typeof(ClientCapabilities))]
|
||||
[JsonSerializable(typeof(CallToolResult))]
|
||||
[JsonSerializable(typeof(CallToolRequestParams))]
|
||||
[JsonSerializable(typeof(CreateMessageRequestParams))]
|
||||
[JsonSerializable(typeof(ElicitRequestParams))]
|
||||
internal partial class ConsumerJsonContext : JsonSerializerContext;
|
||||
Loading…
Reference in a new issue