Skip to content

Commit d53360d

Browse files
Copilotjeffhandley
andauthored
Fix McpTask double-wrapping: add McpTask case in InvokeAsync switch and bypass ExecuteToolAsTaskAsync for McpTask-returning tools
Agent-Logs-Url: https://github.com/modelcontextprotocol/csharp-sdk/sessions/0b57604e-d75a-4879-a7ed-139c8e03ec6b Co-authored-by: jeffhandley <1031940+jeffhandley@users.noreply.github.com>
1 parent 044ce93 commit d53360d

File tree

5 files changed

+138
-1
lines changed

5 files changed

+138
-1
lines changed

src/ModelContextProtocol.Core/Server/AIFunctionMcpServerTool.cs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,11 +244,30 @@ private AIFunctionMcpServerTool(AIFunction function, Tool tool, IServiceProvider
244244

245245
_structuredOutputRequiresWrapping = structuredOutputRequiresWrapping;
246246
_metadata = metadata;
247+
248+
// Detect if the tool's underlying method returns McpTask (directly or wrapped in Task<>/ValueTask<>).
249+
if (function.UnderlyingMethod is { } method)
250+
{
251+
Type returnType = method.ReturnType;
252+
if (returnType.IsGenericType)
253+
{
254+
Type gt = returnType.GetGenericTypeDefinition();
255+
if (gt == typeof(Task<>) || gt == typeof(ValueTask<>))
256+
{
257+
returnType = returnType.GetGenericArguments()[0];
258+
}
259+
}
260+
261+
ReturnsMcpTask = returnType == typeof(McpTask);
262+
}
247263
}
248264

249265
/// <inheritdoc />
250266
public override Tool ProtocolTool { get; }
251267

268+
/// <inheritdoc />
269+
internal override bool ReturnsMcpTask { get; }
270+
252271
/// <inheritdoc />
253272
public override IReadOnlyList<object> Metadata => _metadata;
254273

@@ -311,6 +330,11 @@ public override async ValueTask<CallToolResult> InvokeAsync(
311330

312331
CallToolResult callToolResponse => callToolResponse,
313332

333+
McpTask mcpTask => new()
334+
{
335+
Task = mcpTask,
336+
},
337+
314338
_ => new()
315339
{
316340
Content = [new TextContentBlock { Text = JsonSerializer.Serialize(result, AIFunction.JsonSerializerOptions.GetTypeInfo(typeof(object))) }],

src/ModelContextProtocol.Core/Server/DelegatingMcpServerTool.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ protected DelegatingMcpServerTool(McpServerTool innerTool)
2323
/// <inheritdoc />
2424
public override Tool ProtocolTool => _innerTool.ProtocolTool;
2525

26+
/// <inheritdoc />
27+
internal override bool ReturnsMcpTask => _innerTool.ReturnsMcpTask;
28+
2629
/// <inheritdoc />
2730
public override IReadOnlyList<object> Metadata => _innerTool.Metadata;
2831

src/ModelContextProtocol.Core/Server/McpServerImpl.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,13 @@ await originalListToolsHandler(request, cancellationToken).ConfigureAwait(false)
707707
McpErrorCode.InvalidParams);
708708
}
709709

710+
// If the tool manages its own task lifecycle (returns McpTask),
711+
// invoke it directly and return its result without SDK task wrapping.
712+
if (tool.ReturnsMcpTask)
713+
{
714+
return await tool.InvokeAsync(request, cancellationToken).ConfigureAwait(false);
715+
}
716+
710717
// Task augmentation requested - return CreateTaskResult
711718
return await ExecuteToolAsTaskAsync(tool, request, taskMetadata, taskStore, sendNotifications, cancellationToken).ConfigureAwait(false);
712719
}

src/ModelContextProtocol.Core/Server/McpServerTool.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,10 @@ protected McpServerTool()
157157
/// <summary>Gets the protocol <see cref="Tool"/> type for this instance.</summary>
158158
public abstract Tool ProtocolTool { get; }
159159

160+
/// <summary>Gets whether the tool's underlying method returns <see cref="McpTask"/>, indicating
161+
/// it manages its own task lifecycle and should not be wrapped by the SDK.</summary>
162+
internal virtual bool ReturnsMcpTask => false;
163+
160164
/// <summary>
161165
/// Gets the metadata for this tool instance.
162166
/// </summary>

tests/ModelContextProtocol.Tests/Server/ToolTaskSupportTests.cs

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,106 @@ public async Task TaskPath_Logs_Error_When_Tool_Throws()
657657

658658
#endregion
659659

660-
/// <summary>
660+
#region Tool Returning McpTask Tests
661+
662+
#pragma warning disable MCPEXP001 // Tasks feature is experimental
663+
[Fact]
664+
public async Task Tool_ReturningMcpTask_BypassesSdkTaskWrapping()
665+
{
666+
// Arrange - Server with task store and a tool that creates and returns its own McpTask
667+
var taskStore = new InMemoryMcpTaskStore();
668+
669+
await using var fixture = new ClientServerFixture(
670+
LoggerFactory,
671+
configureServer: builder =>
672+
{
673+
builder.WithTools([McpServerTool.Create(
674+
async (IMcpTaskStore store) =>
675+
{
676+
// Tool creates its own task explicitly
677+
var task = await store.CreateTaskAsync(
678+
new McpTaskMetadata(),
679+
new RequestId("tool-req"),
680+
new JsonRpcRequest { Method = "tools/call" });
681+
return task;
682+
},
683+
new McpServerToolCreateOptions
684+
{
685+
Name = "self-managing-tool",
686+
Description = "A tool that creates and returns its own McpTask",
687+
Execution = new ToolExecution { TaskSupport = ToolTaskSupport.Optional }
688+
})]);
689+
},
690+
configureServices: services =>
691+
{
692+
services.AddSingleton<IMcpTaskStore>(taskStore);
693+
services.Configure<McpServerOptions>(options => options.TaskStore = taskStore);
694+
});
695+
696+
// Act - Call the tool with task metadata (task-augmented request)
697+
var callResult = await fixture.Client.CallToolAsync(
698+
new CallToolRequestParams
699+
{
700+
Name = "self-managing-tool",
701+
Task = new McpTaskMetadata()
702+
},
703+
cancellationToken: TestContext.Current.CancellationToken);
704+
705+
// Assert - Only 1 task should exist (the tool-created one), not 2
706+
Assert.NotNull(callResult.Task);
707+
708+
var allTasks = await fixture.Client.ListTasksAsync(cancellationToken: TestContext.Current.CancellationToken);
709+
Assert.Single(allTasks);
710+
Assert.Equal(callResult.Task.TaskId, allTasks[0].TaskId);
711+
}
712+
713+
[Fact]
714+
public async Task Tool_ReturningMcpTask_WithoutTaskMetadata_ReturnsTaskDirectly()
715+
{
716+
// Arrange - Server with task store and a tool that returns McpTask
717+
var taskStore = new InMemoryMcpTaskStore();
718+
719+
await using var fixture = new ClientServerFixture(
720+
LoggerFactory,
721+
configureServer: builder =>
722+
{
723+
builder.WithTools([McpServerTool.Create(
724+
async (IMcpTaskStore store) =>
725+
{
726+
var task = await store.CreateTaskAsync(
727+
new McpTaskMetadata(),
728+
new RequestId("tool-req"),
729+
new JsonRpcRequest { Method = "tools/call" });
730+
return task;
731+
},
732+
new McpServerToolCreateOptions
733+
{
734+
Name = "self-managing-tool",
735+
Description = "A tool that creates and returns its own McpTask",
736+
Execution = new ToolExecution { TaskSupport = ToolTaskSupport.Optional }
737+
})]);
738+
},
739+
configureServices: services =>
740+
{
741+
services.AddSingleton<IMcpTaskStore>(taskStore);
742+
services.Configure<McpServerOptions>(options => options.TaskStore = taskStore);
743+
});
744+
745+
// Act - Call without task metadata (normal invocation)
746+
var callResult = await fixture.Client.CallToolAsync(
747+
"self-managing-tool",
748+
cancellationToken: TestContext.Current.CancellationToken);
749+
750+
// Assert - Tool's McpTask should be on the result
751+
Assert.NotNull(callResult.Task);
752+
753+
var allTasks = await fixture.Client.ListTasksAsync(cancellationToken: TestContext.Current.CancellationToken);
754+
Assert.Single(allTasks);
755+
Assert.Equal(callResult.Task.TaskId, allTasks[0].TaskId);
756+
}
757+
#pragma warning restore MCPEXP001
758+
759+
#endregion
661760
/// A fixture that creates a connected MCP client-server pair for testing.
662761
/// </summary>
663762
private sealed class ClientServerFixture : IAsyncDisposable

0 commit comments

Comments
 (0)