Skip to content

Commit 3e4435d

Browse files
Copilotstephentoub
Copilot
and
stephentoub
authored
Fix WithMeta+WithProgress by using DeepClone instead of JsonObject constructor
Replace `new JsonObject(meta)` with `(JsonObject)meta.DeepClone()` in both SendRequestWithProgressAsync and SendRequestWithProgressAsTaskAsync to prevent InvalidOperationException when JsonNode values already have a parent. Add tests covering progress with meta, InvokeAsync path, mutation safety, chaining all With* methods, and merging RequestOptions meta with progress. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> Agent-Logs-Url: https://github.com/modelcontextprotocol/csharp-sdk/sessions/d4ec72e6-6d91-429a-92ae-84f8eaa41bbc
1 parent 9812235 commit 3e4435d

2 files changed

Lines changed: 161 additions & 2 deletions

File tree

‎src/ModelContextProtocol.Core/Client/McpClient.Methods.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -877,7 +877,7 @@ async ValueTask<CallToolResult> SendRequestWithProgressAsync(
877877
return default;
878878
}).ConfigureAwait(false);
879879

880-
JsonObject metaWithProgress = meta is not null ? new(meta) : [];
880+
JsonObject metaWithProgress = meta is not null ? (JsonObject)meta.DeepClone() : [];
881881
metaWithProgress["progressToken"] = progressToken.ToString();
882882

883883
return await CallToolAsync(
@@ -1007,7 +1007,7 @@ async ValueTask<McpTask> SendTaskAugmentedCallToolRequestWithProgressAsync(
10071007
return default;
10081008
}).ConfigureAwait(false);
10091009

1010-
JsonObject metaWithProgress = meta is not null ? new(meta) : [];
1010+
JsonObject metaWithProgress = meta is not null ? (JsonObject)meta.DeepClone() : [];
10111011
metaWithProgress["progressToken"] = progressToken.ToString();
10121012

10131013
var result = await SendRequestAsync(

‎tests/ModelContextProtocol.Tests/Client/McpClientToolTests.cs‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -854,4 +854,163 @@ public async Task CallToolAsync_WithAnonymousTypeArguments_Works()
854854
var textBlock = Assert.IsType<TextContentBlock>(result.Content[0]);
855855
Assert.Contains("coordinates", textBlock.Text);
856856
}
857+
858+
[Fact]
859+
public async Task CallAsync_WithProgress_ProgressTokenInMeta()
860+
{
861+
await using McpClient client = await CreateMcpClientForServer();
862+
863+
var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
864+
var tool = tools.Single(t => t.Name == "metadata_echo_tool");
865+
866+
var progressValues = new List<ProgressNotificationValue>();
867+
var progress = new Progress<ProgressNotificationValue>(p => progressValues.Add(p));
868+
869+
// Pass progress directly to CallAsync
870+
var result = await tool.CallAsync(progress: progress, cancellationToken: TestContext.Current.CancellationToken);
871+
872+
Assert.NotNull(result);
873+
Assert.Single(result.Content);
874+
875+
var textBlock = Assert.IsType<TextContentBlock>(result.Content[0]);
876+
var receivedMetadata = JsonNode.Parse(textBlock.Text)?.AsObject();
877+
Assert.NotNull(receivedMetadata);
878+
Assert.NotNull(receivedMetadata["progressToken"]?.GetValue<string>());
879+
}
880+
881+
[Fact]
882+
public async Task CallAsync_WithMeta_WithProgress_BothMetaAndProgressTokenPresent()
883+
{
884+
await using McpClient client = await CreateMcpClientForServer();
885+
886+
var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
887+
var tool = tools.Single(t => t.Name == "metadata_echo_tool");
888+
889+
var progressValues = new List<ProgressNotificationValue>();
890+
var progress = new Progress<ProgressNotificationValue>(p => progressValues.Add(p));
891+
892+
// WithMeta on the tool, progress passed to CallAsync
893+
var result = await tool
894+
.WithMeta(new() { ["traceId"] = "trace-123" })
895+
.CallAsync(progress: progress, cancellationToken: TestContext.Current.CancellationToken);
896+
897+
Assert.NotNull(result);
898+
Assert.Single(result.Content);
899+
900+
var textBlock = Assert.IsType<TextContentBlock>(result.Content[0]);
901+
var receivedMetadata = JsonNode.Parse(textBlock.Text)?.AsObject();
902+
Assert.NotNull(receivedMetadata);
903+
Assert.Equal("trace-123", receivedMetadata["traceId"]?.GetValue<string>());
904+
Assert.NotNull(receivedMetadata["progressToken"]?.GetValue<string>());
905+
}
906+
907+
[Fact]
908+
public async Task InvokeAsync_WithMeta_WithProgress_BothMetaAndProgressTokenPresent()
909+
{
910+
await using McpClient client = await CreateMcpClientForServer();
911+
912+
var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
913+
var tool = tools.Single(t => t.Name == "metadata_echo_tool");
914+
915+
var progressValues = new List<ProgressNotificationValue>();
916+
var progress = new Progress<ProgressNotificationValue>(p => progressValues.Add(p));
917+
918+
// Both WithMeta and WithProgress on the tool, invoked via InvokeAsync (the AIFunction path)
919+
var result = await tool
920+
.WithMeta(new() { ["traceId"] = "trace-456" })
921+
.WithProgress(progress)
922+
.InvokeAsync(cancellationToken: TestContext.Current.CancellationToken);
923+
924+
// InvokeAsync returns a JsonElement for results with meta
925+
Assert.NotNull(result);
926+
}
927+
928+
[Fact]
929+
public async Task CallAsync_WithMeta_WithProgress_DoesNotMutateOriginalMeta()
930+
{
931+
await using McpClient client = await CreateMcpClientForServer();
932+
933+
var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
934+
var tool = tools.Single(t => t.Name == "metadata_echo_tool");
935+
936+
var progressValues = new List<ProgressNotificationValue>();
937+
var progress = new Progress<ProgressNotificationValue>(p => progressValues.Add(p));
938+
939+
JsonObject originalMeta = new()
940+
{
941+
["traceId"] = "trace-789"
942+
};
943+
944+
var toolWithMeta = tool.WithMeta(originalMeta);
945+
946+
// Call multiple times with progress to ensure original meta is not mutated
947+
await toolWithMeta.CallAsync(progress: progress, cancellationToken: TestContext.Current.CancellationToken);
948+
await toolWithMeta.CallAsync(progress: progress, cancellationToken: TestContext.Current.CancellationToken);
949+
950+
// Original meta should not contain progressToken
951+
Assert.Single(originalMeta);
952+
Assert.Equal("trace-789", originalMeta["traceId"]?.GetValue<string>());
953+
Assert.False(originalMeta.ContainsKey("progressToken"));
954+
}
955+
956+
[Fact]
957+
public async Task CallAsync_WithMeta_WithProgress_WithName_WithDescription_AllChained()
958+
{
959+
await using McpClient client = await CreateMcpClientForServer();
960+
961+
var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
962+
var tool = tools.Single(t => t.Name == "metadata_echo_tool");
963+
964+
var progressValues = new List<ProgressNotificationValue>();
965+
var progress = new Progress<ProgressNotificationValue>(p => progressValues.Add(p));
966+
967+
var modifiedTool = tool
968+
.WithName("custom_name")
969+
.WithDescription("Custom description")
970+
.WithMeta(new() { ["chainedKey"] = "chainedValue" });
971+
972+
Assert.Equal("custom_name", modifiedTool.Name);
973+
Assert.Equal("Custom description", modifiedTool.Description);
974+
975+
var result = await modifiedTool.CallAsync(progress: progress, cancellationToken: TestContext.Current.CancellationToken);
976+
977+
Assert.NotNull(result);
978+
var textBlock = Assert.IsType<TextContentBlock>(result.Content[0]);
979+
var receivedMetadata = JsonNode.Parse(textBlock.Text)?.AsObject();
980+
Assert.NotNull(receivedMetadata);
981+
Assert.Equal("chainedValue", receivedMetadata["chainedKey"]?.GetValue<string>());
982+
Assert.NotNull(receivedMetadata["progressToken"]?.GetValue<string>());
983+
}
984+
985+
[Fact]
986+
public async Task CallAsync_WithMeta_WithProgress_WithRequestOptionsMeta_AllMerged()
987+
{
988+
await using McpClient client = await CreateMcpClientForServer();
989+
990+
var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
991+
var tool = tools.Single(t => t.Name == "metadata_echo_tool");
992+
993+
var progressValues = new List<ProgressNotificationValue>();
994+
var progress = new Progress<ProgressNotificationValue>(p => progressValues.Add(p));
995+
996+
RequestOptions requestOptions = new()
997+
{
998+
Meta = new()
999+
{
1000+
["requestKey"] = "requestValue"
1001+
}
1002+
};
1003+
1004+
var result = await tool
1005+
.WithMeta(new() { ["toolKey"] = "toolValue" })
1006+
.CallAsync(progress: progress, options: requestOptions, cancellationToken: TestContext.Current.CancellationToken);
1007+
1008+
Assert.NotNull(result);
1009+
var textBlock = Assert.IsType<TextContentBlock>(result.Content[0]);
1010+
var receivedMetadata = JsonNode.Parse(textBlock.Text)?.AsObject();
1011+
Assert.NotNull(receivedMetadata);
1012+
Assert.Equal("toolValue", receivedMetadata["toolKey"]?.GetValue<string>());
1013+
Assert.Equal("requestValue", receivedMetadata["requestKey"]?.GetValue<string>());
1014+
Assert.NotNull(receivedMetadata["progressToken"]?.GetValue<string>());
1015+
}
8571016
}

0 commit comments

Comments
 (0)