-
Notifications
You must be signed in to change notification settings - Fork 658
Description
The line to resolve services from the HttpContext here is unsafe because it comes from the initialize request that normally has already completed. I also think there's an issue where if you write a RunSessionHandler intentionally to complete early, it does not forcibly end the session. At the same time, having access to the HttpContext when a session starts before you call McpServer.RunAsync could be useful, so I don't want to prevent people from doing that in 1.0
For these reasons, and because we want to release 1.0, I think we should make the RunSessionHandler API experimental. We might have to delete it and replace it with something else. In some cases ConfigureSessionOptions might be the better option, so we should suggest that in the experimental warning. We should also mention that the API may be deleted or change signatures.
.WithHttpTransport(options =>
{
// Use RunSessionHandler to clean up streams when a session ends
options.RunSessionHandler = async (httpContext, mcpServer, cancellationToken) =>
{
try
{
await mcpServer.RunAsync(cancellationToken);
}
finally
{
// Delete all streams associated with this session when it ends
if (!string.IsNullOrEmpty(mcpServer.SessionId))
{
var eventStreamStore = httpContext.RequestServices.GetRequiredService<SessionTrackingEventStreamStore>();
await eventStreamStore.DeleteStreamsForSessionAsync(mcpServer.SessionId);
}
}
};