Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ModelContextProtocol.slnx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
13 changes: 12 additions & 1 deletion src/ModelContextProtocol.Core/Protocol/CallToolRequestParams.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Diagnostics.CodeAnalysis;
using System.Text.Json;
using System.Text.Json.Serialization;

Expand Down Expand Up @@ -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; }
}
13 changes: 12 additions & 1 deletion src/ModelContextProtocol.Core/Protocol/CallToolResult.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Diagnostics.CodeAnalysis;
using System.Text.Json.Nodes;
using System.Text.Json.Serialization;

Expand Down Expand Up @@ -65,6 +66,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; }
}
13 changes: 12 additions & 1 deletion src/ModelContextProtocol.Core/Protocol/ClientCapabilities.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.ComponentModel;
using System.Diagnostics.CodeAnalysis;
using System.Text.Json.Serialization;
using ModelContextProtocol.Client;
using ModelContextProtocol.Server;
Expand Down Expand Up @@ -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; }
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Diagnostics.CodeAnalysis;
using System.Text.Json;
using System.Text.Json.Serialization;

Expand Down Expand Up @@ -128,6 +129,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; }
}
12 changes: 11 additions & 1 deletion src/ModelContextProtocol.Core/Protocol/ElicitRequestParams.cs
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,18 @@ public string Mode
/// 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
Expand Down
13 changes: 12 additions & 1 deletion src/ModelContextProtocol.Core/Protocol/ServerCapabilities.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.ComponentModel;
using System.Diagnostics.CodeAnalysis;
using System.Text.Json.Serialization;
using ModelContextProtocol.Server;

Expand Down Expand Up @@ -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; }
}
11 changes: 10 additions & 1 deletion src/ModelContextProtocol.Core/Protocol/Tool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,17 @@ public JsonElement? OutputSchema
/// 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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we make sure this is being adequately advertised to users? The fact that we're silently dropping experimental properties only for externally defined source generators (but not externally defined reflection-based JSOs) seems to violate the principle of least astonishment, particularly in the face of us documenting that STJ has the same behavior regardless of what contract resolver is being used.

Given how STJ is being used typically in AOT apps, users could be seeing regressions trigger in unexpected manner, consider the following example:

JsonSerializerOptions options = new()
{
    TypeInfoResolverchain = { MyContext.Default, McpJsonUtilities.DefaultOptions.Resolver }
};

[JsonSerializable(typeof(MyCustomType)]
//[JsonSerializable(typeof(CallToolRequestParams)]
partial class MyContext : JsonSerializerContext;

Either uncommenting CallToolRequestParams or otherwise including it in the transitive type graph of MyContext will result in the experimental properties on CallToolRequestParams getting skipped silently. This seems like a very brittle arrangement to me.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's the unfortunate footgun that this approach has. The brittle configuration is the main tradeoff this PR makes compared to #1260.

I would think that at a minimum, we'd need to document somewhere that anyone serializing MCP protocol types via JsonSerializerContext needs to configure their TypeInfoResolverChain with McpJsonUtilities.DefaultOptions.Resolver being the first entry.

We could also add some kind of McpJsonUtilities.CreateOptions(MyContext.Default) helper that sets up the TypeInfoResolverChain correctly, but customers still need to know to do that.

A more involved option could be to emit a compiler diagnostic if we detect that any protocol types are either directly or transitively included in a JsonSerializerContext. The warning would suggest adding an [McpJsonContext] attribute to MyContext, and another source generator would detect that attribute and add a correctly-configured McpOptions property to MyContext. I haven't explored this though, so the implementation might be prohibitively complex for the benefit it brings.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only other thing that's coming to mind for me is for us to implement an analyzer that detects if the JSON source generator is being used, understand this pattern we have, and raise diagnostics for the footgun.

But I suggest we start by documenting this adequately and explore the analyzer separately (not blocking 1.0.0 GA).

/// 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);
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -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;