Skip to content

Adding an overload of McpClientExtensions::CallToolAsync taking JsonElement tool argument #641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
92 changes: 92 additions & 0 deletions src/ModelContextProtocol.Core/Client/McpClientExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,98 @@ public static Task UnsubscribeFromResourceAsync(this IMcpClient client, Uri uri,
return UnsubscribeFromResourceAsync(client, uri.ToString(), cancellationToken);
}

/// <summary>
/// Invokes a tool on the server.
/// </summary>
/// <param name="client">The client instance used to communicate with the MCP server.</param>
/// <param name="toolName">The name of the tool to call on the server.</param>
/// <param name="arguments">A dictionary of arguments to pass to the tool. Each key represents a parameter name,
/// and its associated value represents the argument value as a <see cref="JsonElement"/>.
/// </param>
/// <param name="progress">
/// An optional <see cref="IProgress{T}"/> to have progress notifications reported to it. Setting this to a non-<see langword="null"/>
/// value will result in a progress token being included in the call, and any resulting progress notifications during the operation
/// routed to this instance.
/// </param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to monitor for cancellation requests. The default is <see cref="CancellationToken.None"/>.</param>
/// <returns>
/// A task containing the <see cref="CallToolResult"/> from the tool execution. The response includes
/// the tool's output content, which may be structured data, text, or an error message.
/// </returns>
/// <exception cref="ArgumentNullException"><paramref name="client"/> is <see langword="null"/>.</exception>
/// <exception cref="ArgumentNullException"><paramref name="toolName"/> is <see langword="null"/>.</exception>
/// <exception cref="McpException">The server could not find the requested tool, or the server encountered an error while processing the request.</exception>
/// <example>
/// <code>
/// // Call a tool with JsonElement arguments
/// var arguments = new Dictionary&lt;string, JsonElement&gt;
/// {
/// ["message"] = JsonSerializer.SerializeToElement("Hello MCP!")
/// };
/// var result = await client.CallToolAsync("echo", arguments);
/// </code>
/// </example>
public static ValueTask<CallToolResult> CallToolAsync(
this IMcpClient client,
string toolName,
IReadOnlyDictionary<string, JsonElement> arguments,
Copy link
Member

@eiriktsarpalis eiriktsarpalis Jul 22, 2025

Choose a reason for hiding this comment

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

There are many ways in which you could represent tool arguments in STJ: IDictionary<string, JsonElement>, IDictionary<string, object?>, JsonElement, JsonObject, IReadOnlyDictionary<string, JsonNode?>, so singling out an overload for this particular representation feels oddly specific. Rather than exposing overloads for each and every one of them, I would be inclined to just ask users to populate the one representation picked by the library (or author extension methods that perform the conversion under wraps).

Copy link
Contributor

@stephentoub stephentoub Jul 22, 2025

Choose a reason for hiding this comment

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

We could also consider just having an overload that takes a JsonElement for the arguments. Then someone that has json wouldn't need to first parse it into a dictionary.

Copy link
Author

Choose a reason for hiding this comment

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

thanks @eiriktsarpalis and @stephentoub for taking a look.

We could also consider just having an overload that takes a JsonElement for the arguments. Then someone that has json wouldn't need to first parse it into a dictionary.

This is great feedback @stephentoub. Yes, this helps optimize the call sites even better, often user may obtain immutable JsonElement with little overhead. I’ve updated the pr. Could you please review this and let me know if there are any areas for improvement?

Copy link
Member

Choose a reason for hiding this comment

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

It would still need to be converted under the hood by the overload, since AIFunction expects dictionaries. Which is fine I suppose if we only care about this being an accelerator.

Copy link
Author

Choose a reason for hiding this comment

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

It would still need to be converted under the hood by the overload, since AIFunction expects dictionaries. Which is fine I suppose if we only care about this being an accelerator.

That is correct. I was not sure, for this one use case, if it is appropriate to define a variant of CallToolRequestParams that uses an Arguments property of type JsonElement, as well as to add a corresponding context.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would still need to be converted under the hood by the overload, since AIFunction expects dictionaries.

This doesn't get passed to AIFunction; it's written into a json rpc request.

Copy link
Member

Choose a reason for hiding this comment

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

The underlying request model is using Dictionary, which necessitates a conversion from JsonElement. In the future we might want to consider using a different type so that we avoid doing that.

IProgress<ProgressNotificationValue>? progress = null,
CancellationToken cancellationToken = default)
{
Throw.IfNull(client);
Throw.IfNull(toolName);

if (progress is not null)
{
return SendRequestWithProgressAsync(client, toolName, arguments, progress, cancellationToken);
}

return client.SendRequestAsync(
RequestMethods.ToolsCall,
new()
{
Name = toolName,
Arguments = arguments,
},
McpJsonUtilities.JsonContext.Default.CallToolRequestParams,
McpJsonUtilities.JsonContext.Default.CallToolResult,
cancellationToken: cancellationToken);

static async ValueTask<CallToolResult> SendRequestWithProgressAsync(
IMcpClient client,
string toolName,
IReadOnlyDictionary<string, JsonElement> arguments,
IProgress<ProgressNotificationValue> progress,
CancellationToken cancellationToken)
{
ProgressToken progressToken = new(Guid.NewGuid().ToString("N"));

await using var _ = client.RegisterNotificationHandler(NotificationMethods.ProgressNotification,
(notification, cancellationToken) =>
{
if (JsonSerializer.Deserialize(notification.Params, McpJsonUtilities.JsonContext.Default.ProgressNotificationParams) is { } pn &&
pn.ProgressToken == progressToken)
{
progress.Report(pn.Progress);
}

return default;
}).ConfigureAwait(false);

return await client.SendRequestAsync(
RequestMethods.ToolsCall,
new()
{
Name = toolName,
Arguments = arguments,
ProgressToken = progressToken,
},
McpJsonUtilities.JsonContext.Default.CallToolRequestParams,
McpJsonUtilities.JsonContext.Default.CallToolResult,
cancellationToken: cancellationToken).ConfigureAwait(false);
}
}

/// <summary>
/// Invokes a tool on the server.
/// </summary>
Expand Down
24 changes: 24 additions & 0 deletions tests/ModelContextProtocol.Tests/ClientIntegrationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,30 @@ public async Task CallTool_Stdio_EchoServer(string clientId)
Assert.Equal("Echo: Hello MCP!", textContent.Text);
}

[Theory]
[MemberData(nameof(GetClients))]
public async Task CallTool_Stdio_EchoServer_WithJsonElementArguments(string clientId)
{
// arrange

// act
await using var client = await _fixture.CreateClientAsync(clientId);
var result = await client.CallToolAsync(
"echo",
new Dictionary<string, JsonElement>
{
["message"] = JsonDocument.Parse("\"Hello MCP with JsonElement!\"").RootElement
},
cancellationToken: TestContext.Current.CancellationToken
);

// assert
Assert.NotNull(result);
Assert.Null(result.IsError);
var textContent = Assert.Single(result.Content.OfType<TextContentBlock>());
Assert.Equal("Echo: Hello MCP with JsonElement!", textContent.Text);
}

[Fact]
public async Task CallTool_Stdio_EchoSessionId_ReturnsEmpty()
{
Expand Down