Implement MCP task support (SEP-1686) #1170
Conversation
| /// The server handles all polling logic internally. | ||
| /// </remarks> | ||
| [Experimental(Experimentals.Tasks_DiagnosticId, UrlFormat = Experimentals.Tasks_Url)] | ||
| public ValueTask<JsonElement> GetTaskResultAsync( |
There was a problem hiding this comment.
Note that the GetTaskResultAsync method returns a raw JsonElement instead of, say, a CallToolResult. This is because a task could correspond to requests beyond tool calls. The typescript SDK returns Result in this case, however today we have no implementation of polymorphic deserialization in Result.
| /// </remarks> | ||
| [DebuggerDisplay("{DebuggerDisplay,nq}")] | ||
| [Experimental(Experimentals.Tasks_DiagnosticId, UrlFormat = Experimentals.Tasks_Url)] | ||
| public sealed class McpTask |
There was a problem hiding this comment.
Note that unlike other SDKs we adopt the "McpTask" naming convention to distinguish this feature from regular TPL tasks.
| /// </para> | ||
| /// </remarks> | ||
| [Experimental(Experimentals.Tasks_DiagnosticId, UrlFormat = Experimentals.Tasks_Url)] | ||
| public async ValueTask<(McpTask Task, JsonElement Result)> WaitForTaskResultAsync( |
There was a problem hiding this comment.
Note the peculiar return signature on this method. This is intentionally copying the signature used by the TS sdk. Alternatives include returning just the JsonElement or using a named envelope type containing both the task and its result. I think this is the simpler solution also due to the similarity with existing SDKs.
| // If neither is done, resolving IMcpTaskStore will throw. | ||
| services.TryAddSingleton<IMcpTaskStore>(sp => | ||
| { | ||
| var options = sp.GetRequiredService<IOptions<McpServerOptions>>().Value; |
There was a problem hiding this comment.
I would like some feedback from @halter73 on this one. This is automatically registering the IMcpTaskStore if one has been configured in the server options. We could of course skip this and have users rely on just the server options being available.
There was a problem hiding this comment.
The normal way to do this would be add a IMcpTaskStore? taskStore = null parameter to McpServerOptionsSetup and then set options.TaskStore = taskStore in the Configure method.
As far as I can tell, this is the only line of code that currently resolves IMcpTaskStore from the service provider, so another option would be to just undo the AddMcpServer changes and leave it to the app developer to resolve their IMcpTaskStore and set McpServerOptions.TaskStore themselves. This is what @MackinnonBuck did in #1077 for the ISseEventStreamStore.
However, I do like the convenience of just being able to add a singleton to DI and have it light up, so I think we should probably also add an HttpServerTransportOptionsSetup that initializes the EventStreamStore from DI.
2 participants
Implements MCP task support. See the included tasks.md for a high-level introduction to the new APIs.
Fix #943.