Skip to content

Commit 1449f7d

Browse files
Copilotstephentoub
andcommitted
Fix spec compliance: Tool.Name required, CancelledNotificationParams.RequestId optional, LoggingMessageNotificationParams.Data required
- Tool.Name: Add `required` modifier to match Prompt.Name, Resource.Name, ResourceTemplate.Name. An empty string is not a valid tool name per spec. - CancelledNotificationParams.RequestId: Make optional (RequestId?) per 2025-11-25 spec. Required for request cancellation, must not be used for task cancellation. - LoggingMessageNotificationParams.Data: Make required non-nullable (JsonElement instead of JsonElement?) per spec where `data: unknown` is required. - Update McpSessionHandler to use pattern matching for nullable RequestId. - Update tests to reflect the new type constraints. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
1 parent 9f62045 commit 1449f7d

7 files changed

Lines changed: 29 additions & 18 deletions

File tree

src/ModelContextProtocol.Core/McpSessionHandler.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -380,11 +380,11 @@ private async Task HandleNotificationAsync(JsonRpcNotification notification, Can
380380
{
381381
try
382382
{
383-
if (GetCancelledNotificationParams(notification.Params) is CancelledNotificationParams cn &&
384-
_handlingRequests.TryGetValue(cn.RequestId, out var cts))
383+
if (GetCancelledNotificationParams(notification.Params) is CancelledNotificationParams { RequestId: { } requestId } cn &&
384+
_handlingRequests.TryGetValue(requestId, out var cts))
385385
{
386386
await cts.CancelAsync().ConfigureAwait(false);
387-
LogRequestCanceled(EndpointName, cn.RequestId, cn.Reason);
387+
LogRequestCanceled(EndpointName, requestId, cn.Reason);
388388
}
389389
}
390390
catch
@@ -625,8 +625,8 @@ await _outgoingMessageFilter(async (msg, ct) =>
625625
// server won't be sending a response, or per the specification, the response should be ignored. There are inherent
626626
// race conditions here, so it's possible and allowed for the operation to complete before we get to this point.
627627
if (msg is JsonRpcNotification { Method: NotificationMethods.CancelledNotification } notification &&
628-
GetCancelledNotificationParams(notification.Params) is CancelledNotificationParams cn &&
629-
_pendingRequests.TryRemove(cn.RequestId, out var tcs))
628+
GetCancelledNotificationParams(notification.Params) is CancelledNotificationParams { RequestId: { } requestId } &&
629+
_pendingRequests.TryRemove(requestId, out var tcs))
630630
{
631631
tcs.TrySetCanceled(default);
632632
}

src/ModelContextProtocol.Core/Protocol/CancelledNotificationParams.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,16 @@ public sealed class CancelledNotificationParams : NotificationParams
1717
/// Gets or sets the ID of the request to cancel.
1818
/// </summary>
1919
/// <remarks>
20-
/// This value must match the ID of an in-flight request that the sender wishes to cancel.
20+
/// <para>
21+
/// This value must correspond to the ID of a request previously issued in the same direction.
22+
/// It must be provided for cancelling non-task requests.
23+
/// </para>
24+
/// <para>
25+
/// This must not be used for cancelling tasks; use the <c>tasks/cancel</c> request instead.
26+
/// </para>
2127
/// </remarks>
2228
[JsonPropertyName("requestId")]
23-
public required RequestId RequestId { get; set; }
29+
public RequestId? RequestId { get; set; }
2430

2531
/// <summary>
2632
/// Gets or sets an optional string describing the reason for the cancellation request.

src/ModelContextProtocol.Core/Protocol/LoggingMessageNotificationParams.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,11 @@ public sealed class LoggingMessageNotificationParams : NotificationParams
4545
public string? Logger { get; set; }
4646

4747
/// <summary>
48-
/// Gets or sets the data to be logged, such as a string message.
48+
/// Gets or sets the data to be logged, such as a string message or an object.
4949
/// </summary>
50+
/// <remarks>
51+
/// Any JSON serializable type is allowed here. This property is required by the MCP specification.
52+
/// </remarks>
5053
[JsonPropertyName("data")]
51-
public JsonElement? Data { get; set; }
54+
public required JsonElement Data { get; set; }
5255
}

src/ModelContextProtocol.Core/Protocol/Tool.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public sealed class Tool : IBaseMetadata
1515
{
1616
/// <inheritdoc />
1717
[JsonPropertyName("name")]
18-
public string Name { get; set; } = string.Empty;
18+
public required string Name { get; set; }
1919

2020
/// <inheritdoc />
2121
[JsonPropertyName("title")]

tests/ModelContextProtocol.Tests/Client/McpClientTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -539,11 +539,11 @@ public async Task AsClientLoggerProvider_MessagesSentToClient()
539539
{
540540
var m = await channel.Reader.ReadAsync(TestContext.Current.CancellationToken);
541541
Assert.NotNull(m);
542-
Assert.NotNull(m.Data);
542+
Assert.NotEqual(JsonValueKind.Undefined, m.Data.ValueKind);
543543

544544
Assert.Equal("TestLogger", m.Logger);
545545

546-
string? s = JsonSerializer.Deserialize<string>(m.Data.Value, McpJsonUtilities.DefaultOptions);
546+
string? s = JsonSerializer.Deserialize<string>(m.Data, McpJsonUtilities.DefaultOptions);
547547
Assert.NotNull(s);
548548

549549
if (s.Contains("Information"))

tests/ModelContextProtocol.Tests/Protocol/LoggingMessageNotificationParamsTests.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ public static void LoggingMessageNotificationParams_SerializationRoundTrip_Prese
2323
Assert.NotNull(deserialized);
2424
Assert.Equal(LoggingLevel.Warning, deserialized.Level);
2525
Assert.Equal("MyApp.Services", deserialized.Logger);
26-
Assert.NotNull(deserialized.Data);
27-
Assert.Equal("Something went wrong", deserialized.Data.Value.GetString());
26+
Assert.NotEqual(JsonValueKind.Undefined, deserialized.Data.ValueKind);
27+
Assert.Equal("Something went wrong", deserialized.Data.GetString());
2828
Assert.NotNull(deserialized.Meta);
2929
Assert.Equal("value", (string)deserialized.Meta["key"]!);
3030
}
@@ -34,7 +34,8 @@ public static void LoggingMessageNotificationParams_SerializationRoundTrip_WithM
3434
{
3535
var original = new LoggingMessageNotificationParams
3636
{
37-
Level = LoggingLevel.Error
37+
Level = LoggingLevel.Error,
38+
Data = JsonDocument.Parse("null").RootElement.Clone(),
3839
};
3940

4041
string json = JsonSerializer.Serialize(original, McpJsonUtilities.DefaultOptions);
@@ -43,7 +44,7 @@ public static void LoggingMessageNotificationParams_SerializationRoundTrip_WithM
4344
Assert.NotNull(deserialized);
4445
Assert.Equal(LoggingLevel.Error, deserialized.Level);
4546
Assert.Null(deserialized.Logger);
46-
Assert.Null(deserialized.Data);
47+
Assert.Equal(JsonValueKind.Null, deserialized.Data.ValueKind);
4748
Assert.Null(deserialized.Meta);
4849
}
4950
}

tests/ModelContextProtocol.Tests/Protocol/ToolTests.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public static void Tool_HasCorrectJsonPropertyNames()
9595
[Fact]
9696
public static void ToolInputSchema_HasValidDefaultSchema()
9797
{
98-
var tool = new Tool();
98+
var tool = new Tool { Name = "test" };
9999
JsonElement jsonElement = tool.InputSchema;
100100

101101
Assert.Equal(JsonValueKind.Object, jsonElement.ValueKind);
@@ -119,7 +119,7 @@ public static void ToolInputSchema_HasValidDefaultSchema()
119119
public static void ToolInputSchema_RejectsInvalidSchemaDocuments(string invalidSchema)
120120
{
121121
using var document = JsonDocument.Parse(invalidSchema);
122-
var tool = new Tool();
122+
var tool = new Tool { Name = "test" };
123123

124124
Assert.Throws<ArgumentException>(() => tool.InputSchema = document.RootElement);
125125
}
@@ -133,6 +133,7 @@ public static void ToolInputSchema_AcceptsValidSchemaDocuments(string validSchem
133133
using var document = JsonDocument.Parse(validSchema);
134134
Tool tool = new()
135135
{
136+
Name = "test",
136137
InputSchema = document.RootElement
137138
};
138139

0 commit comments

Comments
 (0)