From 4460d6b9554657171915976884c92ed9d1ccc648 Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Wed, 22 Jul 2026 11:20:58 -0700 Subject: [PATCH 1/8] Fix HTTP task filter composition Compose ordinary tool filters with alternate-result filters and preserve scoped services during background task execution. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3035ad42-0e3d-4010-8390-1c606467b9cb --- docs/concepts/filters.md | 5 + docs/concepts/tasks/tasks.md | 5 + .../Server/McpRequestFilters.cs | 18 +- .../Server/McpServerHandlers.cs | 4 +- .../Server/McpServerImpl.cs | 28 +- .../Server/McpTasksBuilderExtensions.cs | 268 +++++++++++------- .../HttpTaskIntegrationTests.cs | 48 ++++ ...delContextProtocol.AspNetCore.Tests.csproj | 1 + .../Server/CallToolFilterMixingTests.cs | 26 +- .../TaskCallToolFilterCompositionTests.cs | 85 ++++++ 10 files changed, 344 insertions(+), 144 deletions(-) create mode 100644 tests/ModelContextProtocol.AspNetCore.Tests/HttpTaskIntegrationTests.cs create mode 100644 tests/ModelContextProtocol.Tests/Server/TaskCallToolFilterCompositionTests.cs diff --git a/docs/concepts/filters.md b/docs/concepts/filters.md index 896cea6ce..51a1a491e 100644 --- a/docs/concepts/filters.md +++ b/docs/concepts/filters.md @@ -30,6 +30,11 @@ The following request filter methods are available on `IMcpRequestFilterBuilder` - `AddUnsubscribeFromResourcesFilter` - Filter for resource unsubscription handlers - `AddSetLoggingLevelFilter` - Filter for logging level handlers +The Tasks extension augments the ordinary `tools/call` pipeline with an alternate-result filter. +Ordinary call-tool filters, including ASP.NET Core authorization filters, run exactly once inside +that wrapper before the tool executes. An explicit `CallToolWithAlternateHandler` remains a +full replacement and cannot be combined with ordinary call-tool filters. + ## Message Filters In addition to the request-specific filters above, there are low-level message filters that intercept all JSON-RPC messages before they are routed to specific handlers. diff --git a/docs/concepts/tasks/tasks.md b/docs/concepts/tasks/tasks.md index 5342946bc..05820d467 100644 --- a/docs/concepts/tasks/tasks.md +++ b/docs/concepts/tasks/tasks.md @@ -79,6 +79,11 @@ When tasks are enabled with `WithTasks` the SDK automatically: - Plumbs a `CancellationToken` through to the tool that fires when the client invokes `tasks/cancel`, so cancellation propagates cooperatively. +Ordinary `tools/call` filters still run exactly once for task-backed calls. They execute in the +background after the task record is created and before the tool body, so validation, +authorization, and telemetry continue to apply. Each background invocation gets an independent +DI scope that remains alive until the tool pipeline completes. + For production scenarios that need durability, session isolation, multi-process routing, or TTL-based cleanup, implement yourself (see [Implementing a custom task store](#implementing-a-custom-task-store) below). diff --git a/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs b/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs index cb8c9db23..94297c578 100644 --- a/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs +++ b/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs @@ -43,10 +43,13 @@ public IList> ListTool /// requests. The handler should implement logic to execute the requested tool and return appropriate results. /// /// - /// Cannot be used together with . If both are non-empty at configuration time, - /// an will be thrown. This can happen indirectly when combining features that - /// register different tool-call filter styles, such as authorization filters (which use this collection) and the - /// tasks extension (which uses ). + /// These filters run inside . Each ordinary filter runs exactly once + /// when an alternate-result filter invokes the ordinary tool pipeline. For task-backed calls, that invocation + /// occurs in the background after the task record is created and before the matched tool is executed. + /// + /// + /// These filters cannot be used with an explicit , + /// which replaces the ordinary tool-call pipeline rather than augmenting it. /// /// public IList> CallToolFilters @@ -71,10 +74,9 @@ public IList> CallToolFi /// subtype for asynchronous execution. /// /// - /// Cannot be used together with . If both are non-empty at configuration time, - /// an will be thrown. This can happen indirectly when combining features that - /// register different tool-call filter styles, such as the tasks extension (which uses this collection) and - /// authorization filters (which use ). + /// When no explicit is configured, these filters + /// compose outside . The ordinary pipeline, including primitive matching and ordinary + /// filters, is adapted to before these filters are applied. /// /// [Experimental(Experimentals.Subclassing_DiagnosticId, UrlFormat = Experimentals.Subclassing_Url)] diff --git a/src/ModelContextProtocol.Core/Server/McpServerHandlers.cs b/src/ModelContextProtocol.Core/Server/McpServerHandlers.cs index 7e655603a..d7510a932 100644 --- a/src/ModelContextProtocol.Core/Server/McpServerHandlers.cs +++ b/src/ModelContextProtocol.Core/Server/McpServerHandlers.cs @@ -72,7 +72,9 @@ public McpRequestHandler? CallToolHandler /// a for immediate results or an alternate subtype. /// /// - /// Cannot be set if is already set. + /// This is a low-level full replacement for the ordinary tool-call pipeline. It cannot be set if + /// is already set, and it cannot be composed with ordinary + /// . /// /// /// is already set. diff --git a/src/ModelContextProtocol.Core/Server/McpServerImpl.cs b/src/ModelContextProtocol.Core/Server/McpServerImpl.cs index 8b38421c4..768675868 100644 --- a/src/ModelContextProtocol.Core/Server/McpServerImpl.cs +++ b/src/ModelContextProtocol.Core/Server/McpServerImpl.cs @@ -1321,20 +1321,13 @@ private void ConfigureTools(McpServerOptions options) var callToolFilters = options.Filters.Request.CallToolFilters; var callToolWithAlternateFilters = options.Filters.Request.CallToolWithAlternateFilters; - // Validate: cannot mix non-alternate filters/handler with alternate filters/handler. - bool hasNonAlternatePath = callToolHandler is not null || callToolFilters.Count > 0; - bool hasAlternatePath = callToolWithAlternateHandler is not null || callToolWithAlternateFilters.Count > 0; - - if (hasNonAlternatePath && hasAlternatePath) + if (callToolWithAlternateHandler is not null && callToolFilters.Count > 0) { throw new InvalidOperationException( - $"Cannot mix non-alternate ({nameof(McpServerHandlers.CallToolHandler)}/{nameof(McpRequestFilters.CallToolFilters)}) " + - $"with alternate-based ({nameof(McpServerHandlers.CallToolWithAlternateHandler)}/{nameof(McpRequestFilters.CallToolWithAlternateFilters)}) tool-call filters or handlers. " + - $"These two styles cannot currently be composed on the same server. " + - $"This most commonly happens when combining features that register different tool-call filter styles, " + - $"for example AddAuthorizationFilters() (which registers a {nameof(McpRequestFilters.CallToolFilters)} filter) together with " + - $"WithTasks() (which registers a {nameof(McpRequestFilters.CallToolWithAlternateFilters)} filter). " + - $"Configure only one style, or avoid combining features that require different styles."); + $"Cannot apply {nameof(McpRequestFilters.CallToolFilters)} when an explicit " + + $"{nameof(McpServerHandlers.CallToolWithAlternateHandler)} is configured. The alternate handler " + + $"replaces the ordinary tool-call pipeline. Move the behavior to " + + $"{nameof(McpRequestFilters.CallToolWithAlternateFilters)} or remove the explicit alternate handler."); } // Handle tools provided via DI by augmenting the list handler. @@ -1376,12 +1369,9 @@ await originalListToolsHandler(request, cancellationToken).ConfigureAwait(false) listToolsHandler = BuildFilterPipeline(listToolsHandler, options.Filters.Request.ListToolsFilters); - // Build the unified alternate-result handler from one of the two paths. - if (hasAlternatePath) + // An explicit alternate handler replaces the ordinary tool-call pipeline. + if (callToolWithAlternateHandler is not null) { - // Case 2: alternate filter + alternate handler - callToolWithAlternateHandler ??= (static async (request, _) => throw new McpProtocolException($"Unknown tool: '{request.Params?.Name}'", McpErrorCode.InvalidParams)); - // Augment with DI tools. if (tools is not null) { @@ -1401,7 +1391,6 @@ await originalListToolsHandler(request, cancellationToken).ConfigureAwait(false) } else { - // Case 1: non-alternate filter + non-alternate handler -> apply filters, then convert to alternate-based callToolHandler ??= (static async (request, _) => throw new McpProtocolException($"Unknown tool: '{request.Params?.Name}'", McpErrorCode.InvalidParams)); // Augment with DI tools. @@ -1425,6 +1414,9 @@ await originalListToolsHandler(request, cancellationToken).ConfigureAwait(false) var finalCallToolHandler = callToolHandler; callToolWithAlternateHandler = async (request, cancellationToken) => await finalCallToolHandler(request, cancellationToken).ConfigureAwait(false); + + // Alternate-result filters augment the fully constructed ordinary pipeline. + callToolWithAlternateHandler = BuildFilterPipeline(callToolWithAlternateHandler, callToolWithAlternateFilters); } ServerCapabilities.Tools.ListChanged = listChanged; diff --git a/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs b/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs index 06073ce17..dd2ec0dc9 100644 --- a/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs +++ b/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs @@ -36,13 +36,20 @@ public static IMcpServerBuilder WithTasks(this IMcpServerBuilder builder, IMcpTa // background task body has somewhere to report failures. It is optional: if no logging is // registered, the options fall back to NullLoggerFactory. builder.Services.AddSingleton>( - sp => new McpTasksPostConfigureOptions(store, sp.GetService())); + sp => new McpTasksPostConfigureOptions( + store, + sp.GetRequiredService(), + sp.GetService())); return builder; } - private sealed class McpTasksPostConfigureOptions(IMcpTaskStore store, ILoggerFactory? loggerFactory) : IPostConfigureOptions + private sealed class McpTasksPostConfigureOptions( + IMcpTaskStore store, + IServiceScopeFactory serviceScopeFactory, + ILoggerFactory? loggerFactory) : IPostConfigureOptions { private readonly IMcpTaskStore _store = store; + private readonly IServiceScopeFactory _serviceScopeFactory = serviceScopeFactory; private readonly ILogger _logger = (loggerFactory ?? NullLoggerFactory.Instance).CreateLogger(); private readonly ConcurrentDictionary _cancellationSources = new(StringComparer.Ordinal); @@ -71,119 +78,160 @@ public void PostConfigure(string? name, McpServerOptions options) // it to spawn background execution and return the task alternate immediately. options.Filters.Request.CallToolWithAlternateFilters.Add(next => async (request, cancellationToken) => { - if (IsJuly2026OrLaterProtocolRequest(request.JsonRpcRequest) && HasTaskExtensionOptIn(request.Params?.Meta)) + if (!IsJuly2026OrLaterProtocolRequest(request.JsonRpcRequest) || !HasTaskExtensionOptIn(request.Params?.Meta)) { - var taskInfo = await _store.CreateTaskAsync(cancellationToken).ConfigureAwait(false); - var taskId = taskInfo.TaskId; - var cts = new CancellationTokenSource(); - _cancellationSources[taskId] = cts; - var taskCancellationToken = cts.Token; + return await next(request, cancellationToken).ConfigureAwait(false); + } + + return await RunAsTaskAsync(next, request, cancellationToken).ConfigureAwait(false); + }); + } + + private async ValueTask> RunAsTaskAsync( + McpRequestHandler> next, + RequestContext request, + CancellationToken cancellationToken) + { + var executionScope = _serviceScopeFactory.CreateAsyncScope(); + request.Services = executionScope.ServiceProvider; + + McpTaskInfo taskInfo; + try + { + taskInfo = await _store.CreateTaskAsync(cancellationToken).ConfigureAwait(false); + } + catch + { + await executionScope.DisposeAsync().ConfigureAwait(false); + throw; + } - _ = Task.Run(async () => + var taskId = taskInfo.TaskId; + var cts = new CancellationTokenSource(); + _cancellationSources[taskId] = cts; + + // Capture the token before dispatching. Cancellation can remove and dispose the source + // before the background delegate starts. + var taskCancellationToken = cts.Token; + _ = Task.Run( + () => ExecuteTaskAsync(next, request, taskId, taskCancellationToken, executionScope), + CancellationToken.None); + + return ResultOrAlternate.FromAlternate( + ToCreateTaskResult(taskInfo), + McpTasksJsonContext.Default.CreateTaskResult); + } + + private async Task ExecuteTaskAsync( + McpRequestHandler> next, + RequestContext request, + string taskId, + CancellationToken taskCancellationToken, + AsyncServiceScope executionScope) + { + try + { + try + { + using (McpTasksServerExtensions.CreateMcpTaskScope(request.Server, taskId, _store)) { - try - { - using (McpTasksServerExtensions.CreateMcpTaskScope(request.Server, taskId, _store)) - { - try - { - var augmented = await next(request, taskCancellationToken).ConfigureAwait(false); - - if (augmented.IsAlternate) - { - var error = new JsonRpcErrorDetail - { - Code = (int)McpErrorCode.InternalError, - Message = $"{nameof(IMcpTaskStore)} is configured and the {nameof(McpServerHandlers.CallToolWithAlternateHandler)} returned IsAlternate = true. Use only one mechanism.", - }; - var errorJson = JsonSerializer.SerializeToElement(error, McpJsonUtilities.DefaultOptions.GetTypeInfo()); - await _store.SetFailedAsync(taskId, errorJson).ConfigureAwait(false); - return; - } - - var resultJson = JsonSerializer.SerializeToElement(augmented.Result!, McpJsonUtilities.DefaultOptions.GetTypeInfo()); - await _store.SetCompletedAsync(taskId, resultJson).ConfigureAwait(false); - } - catch (OperationCanceledException) when (taskCancellationToken.IsCancellationRequested) - { - await _store.SetCancelledAsync(taskId, CancellationToken.None).ConfigureAwait(false); - } - catch (InputRequiredException) - { - var error = new JsonRpcErrorDetail - { - Code = (int)McpErrorCode.InvalidRequest, - Message = "MRTR and tasks cannot be composed via [McpServerTool] yet.", - }; - var errorJson = JsonSerializer.SerializeToElement(error, McpJsonUtilities.DefaultOptions.GetTypeInfo()); - await _store.SetFailedAsync(taskId, errorJson).ConfigureAwait(false); - } - catch (McpProtocolException mcpEx) - { - // SEP-2663 §186: protocol exceptions store as failed with JSON-RPC error shape. - var error = new JsonRpcErrorDetail { Code = (int)mcpEx.ErrorCode, Message = mcpEx.Message }; - var errorJson = JsonSerializer.SerializeToElement(error, McpJsonUtilities.DefaultOptions.GetTypeInfo()); - await _store.SetFailedAsync(taskId, errorJson).ConfigureAwait(false); - } - catch (Exception ex) - { - // Non-protocol exceptions are wrapped as CallToolResult { IsError = true }, - // matching Core's BuildInitialAlternateToolFilter behavior. - var errorResult = new CallToolResult - { - IsError = true, - Content = [new TextContentBlock - { - Text = ex is McpException - ? $"An error occurred invoking '{request.Params?.Name}': {ex.Message}" - : $"An error occurred invoking '{request.Params?.Name}'.", - }], - }; - var resultJson = JsonSerializer.SerializeToElement(errorResult, McpJsonUtilities.DefaultOptions.GetTypeInfo()); - await _store.SetCompletedAsync(taskId, resultJson).ConfigureAwait(false); - } - finally - { - if (_cancellationSources.TryRemove(taskId, out var registeredCts)) - { - registeredCts.Dispose(); - } - } - } - } - catch (Exception outer) - { - // The inner handlers above record every expected outcome. Reaching here means a - // store operation inside one of those handlers (or the task scope) threw, most - // likely from a custom IMcpTaskStore. Record the failure best-effort and never let - // it surface as an unobserved task exception. - _logger.LogError(outer, "Background execution of task '{TaskId}' terminated unexpectedly while recording its result.", taskId); - - try - { - var error = new JsonRpcErrorDetail { Code = (int)McpErrorCode.InternalError, Message = outer.Message }; - var errorJson = JsonSerializer.SerializeToElement(error, McpJsonUtilities.DefaultOptions.GetTypeInfo()); - await _store.SetFailedAsync(taskId, errorJson).ConfigureAwait(false); - } - catch (Exception storeEx) - { - _logger.LogError(storeEx, "Failed to record the failure of background task '{TaskId}'.", taskId); - } - - if (_cancellationSources.TryRemove(taskId, out var leftoverCts)) - { - leftoverCts.Dispose(); - } - } - }, CancellationToken.None); - - return ResultOrAlternate.FromAlternate( - ToCreateTaskResult(taskInfo), - McpTasksJsonContext.Default.CreateTaskResult); + await ExecuteToolPipelineAsync(next, request, taskId, taskCancellationToken).ConfigureAwait(false); + } } + finally + { + await executionScope.DisposeAsync().ConfigureAwait(false); + } + } + catch (Exception outer) + { + // Expected outcomes are recorded by ExecuteToolPipelineAsync. Reaching here means a + // store operation, task scope, or service scope failed. Record it best-effort. + _logger.LogError(outer, "Background execution of task '{TaskId}' terminated unexpectedly while recording its result.", taskId); - return await next(request, cancellationToken).ConfigureAwait(false); - }); + try + { + var error = new JsonRpcErrorDetail { Code = (int)McpErrorCode.InternalError, Message = outer.Message }; + var errorJson = JsonSerializer.SerializeToElement(error, McpJsonUtilities.DefaultOptions.GetTypeInfo()); + await _store.SetFailedAsync(taskId, errorJson).ConfigureAwait(false); + } + catch (Exception storeEx) + { + _logger.LogError(storeEx, "Failed to record the failure of background task '{TaskId}'.", taskId); + } + } + finally + { + if (_cancellationSources.TryRemove(taskId, out var registeredCts)) + { + registeredCts.Dispose(); + } + } + } + + private async Task ExecuteToolPipelineAsync( + McpRequestHandler> next, + RequestContext request, + string taskId, + CancellationToken taskCancellationToken) + { + try + { + var augmented = await next(request, taskCancellationToken).ConfigureAwait(false); + + if (augmented.IsAlternate) + { + var error = new JsonRpcErrorDetail + { + Code = (int)McpErrorCode.InternalError, + Message = $"{nameof(IMcpTaskStore)} is configured and the {nameof(McpServerHandlers.CallToolWithAlternateHandler)} returned IsAlternate = true. Use only one mechanism.", + }; + var errorJson = JsonSerializer.SerializeToElement(error, McpJsonUtilities.DefaultOptions.GetTypeInfo()); + await _store.SetFailedAsync(taskId, errorJson).ConfigureAwait(false); + return; + } + + var resultJson = JsonSerializer.SerializeToElement(augmented.Result!, McpJsonUtilities.DefaultOptions.GetTypeInfo()); + await _store.SetCompletedAsync(taskId, resultJson).ConfigureAwait(false); + } + catch (OperationCanceledException) when (taskCancellationToken.IsCancellationRequested) + { + await _store.SetCancelledAsync(taskId, CancellationToken.None).ConfigureAwait(false); + } + catch (InputRequiredException) + { + var error = new JsonRpcErrorDetail + { + Code = (int)McpErrorCode.InvalidRequest, + Message = "MRTR and tasks cannot be composed via [McpServerTool] yet.", + }; + var errorJson = JsonSerializer.SerializeToElement(error, McpJsonUtilities.DefaultOptions.GetTypeInfo()); + await _store.SetFailedAsync(taskId, errorJson).ConfigureAwait(false); + } + catch (McpProtocolException mcpEx) + { + // SEP-2663 §186: protocol exceptions store as failed with JSON-RPC error shape. + var error = new JsonRpcErrorDetail { Code = (int)mcpEx.ErrorCode, Message = mcpEx.Message }; + var errorJson = JsonSerializer.SerializeToElement(error, McpJsonUtilities.DefaultOptions.GetTypeInfo()); + await _store.SetFailedAsync(taskId, errorJson).ConfigureAwait(false); + } + catch (Exception ex) + { + // Non-protocol exceptions are wrapped as CallToolResult { IsError = true }, + // matching Core's BuildInitialCallToolFilter behavior. + var errorResult = new CallToolResult + { + IsError = true, + Content = [new TextContentBlock + { + Text = ex is McpException + ? $"An error occurred invoking '{request.Params?.Name}': {ex.Message}" + : $"An error occurred invoking '{request.Params?.Name}'.", + }], + }; + var resultJson = JsonSerializer.SerializeToElement(errorResult, McpJsonUtilities.DefaultOptions.GetTypeInfo()); + await _store.SetCompletedAsync(taskId, resultJson).ConfigureAwait(false); + } } private async ValueTask HandleGetTask(JsonRpcRequest request, CancellationToken cancellationToken) diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/HttpTaskIntegrationTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/HttpTaskIntegrationTests.cs new file mode 100644 index 000000000..e4152829c --- /dev/null +++ b/tests/ModelContextProtocol.AspNetCore.Tests/HttpTaskIntegrationTests.cs @@ -0,0 +1,48 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.DependencyInjection; +using ModelContextProtocol.AspNetCore.Tests.Utils; +using ModelContextProtocol.Client; +using ModelContextProtocol.Extensions.Tasks; +using ModelContextProtocol.Protocol; +using ModelContextProtocol.Server; + +namespace ModelContextProtocol.AspNetCore.Tests; + +public class HttpTaskIntegrationTests(ITestOutputHelper testOutputHelper) : KestrelInMemoryTest(testOutputHelper) +{ + [Fact] + public async Task WithTasks_CanCallToolOverHttp() + { + Builder.Services + .AddMcpServer() + .WithHttpTransport() + .WithTasks(new InMemoryMcpTaskStore { DefaultPollIntervalMs = 10 }) + .WithTools(); + + await using var app = Builder.Build(); + app.MapMcp(); + await app.StartAsync(TestContext.Current.CancellationToken); + + await using var transport = new HttpClientTransport( + new HttpClientTransportOptions { Endpoint = new("http://localhost:5000") }, + HttpClient, + LoggerFactory); + await using var client = await McpClient.CreateAsync( + transport, + loggerFactory: LoggerFactory, + cancellationToken: TestContext.Current.CancellationToken); + + var result = await client.CallToolWithPollingAsync( + new CallToolRequestParams { Name = "test" }, + cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal("Hello World!", Assert.IsType(Assert.Single(result.Content)).Text); + } + + [McpServerToolType] + private sealed class TestTools + { + [McpServerTool(Name = "test")] + public static string Test() => "Hello World!"; + } +} diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/ModelContextProtocol.AspNetCore.Tests.csproj b/tests/ModelContextProtocol.AspNetCore.Tests/ModelContextProtocol.AspNetCore.Tests.csproj index d75877bab..781aa7178 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/ModelContextProtocol.AspNetCore.Tests.csproj +++ b/tests/ModelContextProtocol.AspNetCore.Tests/ModelContextProtocol.AspNetCore.Tests.csproj @@ -60,6 +60,7 @@ + diff --git a/tests/ModelContextProtocol.Tests/Server/CallToolFilterMixingTests.cs b/tests/ModelContextProtocol.Tests/Server/CallToolFilterMixingTests.cs index 3c9b8bb9d..aa7a3750e 100644 --- a/tests/ModelContextProtocol.Tests/Server/CallToolFilterMixingTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/CallToolFilterMixingTests.cs @@ -5,9 +5,8 @@ namespace ModelContextProtocol.Tests.Server; /// -/// Verifies that combining the non-alternate with the -/// alternate fails at configuration time -/// with an actionable message. +/// Verifies composition and validation for the non-alternate +/// and alternate pipelines. /// public class CallToolFilterMixingTests(ITestOutputHelper testOutputHelper) : LoggedTest(testOutputHelper) { @@ -19,20 +18,33 @@ public class CallToolFilterMixingTests(ITestOutputHelper testOutputHelper) : Log next => next; [Fact] - public async Task MixingCallToolFilters_WithAlternateFilters_ThrowsActionableError() + public async Task MixingCallToolFilters_WithAlternateFilters_Succeeds() { await using var transport = new StreamServerTransport(Stream.Null, Stream.Null); var options = new McpServerOptions { Capabilities = new() { Tools = new() } }; options.Filters.Request.CallToolFilters.Add(PassThroughCallToolFilter); options.Filters.Request.CallToolWithAlternateFilters.Add(PassThroughAlternateFilter); + await using var server = McpServer.Create(transport, options, LoggerFactory); + + Assert.NotNull(server); + } + + [Fact] + public async Task CallToolFilters_WithExplicitAlternateHandler_ThrowsActionableError() + { + await using var transport = new StreamServerTransport(Stream.Null, Stream.Null); + var options = new McpServerOptions { Capabilities = new() { Tools = new() } }; + options.Handlers.CallToolWithAlternateHandler = static (_, _) => + new(new ResultOrAlternate(new CallToolResult())); + options.Filters.Request.CallToolFilters.Add(PassThroughCallToolFilter); + var ex = Assert.Throws( () => McpServer.Create(transport, options, LoggerFactory)); Assert.Contains(nameof(McpRequestFilters.CallToolFilters), ex.Message); - Assert.Contains(nameof(McpRequestFilters.CallToolWithAlternateFilters), ex.Message); - Assert.Contains("AddAuthorizationFilters", ex.Message); - Assert.Contains("WithTasks", ex.Message); + Assert.Contains(nameof(McpServerHandlers.CallToolWithAlternateHandler), ex.Message); + Assert.Contains("replaces the ordinary tool-call pipeline", ex.Message); } [Fact] diff --git a/tests/ModelContextProtocol.Tests/Server/TaskCallToolFilterCompositionTests.cs b/tests/ModelContextProtocol.Tests/Server/TaskCallToolFilterCompositionTests.cs new file mode 100644 index 000000000..5b522b9c1 --- /dev/null +++ b/tests/ModelContextProtocol.Tests/Server/TaskCallToolFilterCompositionTests.cs @@ -0,0 +1,85 @@ +using Microsoft.Extensions.DependencyInjection; +using ModelContextProtocol.Extensions.Tasks; +using ModelContextProtocol.Protocol; +using ModelContextProtocol.Server; +using ModelContextProtocol.Tests.Utils; + +namespace ModelContextProtocol.Tests.Server; + +public class TaskCallToolFilterCompositionTests(ITestOutputHelper testOutputHelper) : ClientServerTestBase(testOutputHelper) +{ + private readonly TaskCompletionSource _continueBackgroundExecution = new(TaskCreationOptions.RunContinuationsAsynchronously); + private readonly TaskCompletionSource _executionCompleted = new(TaskCreationOptions.RunContinuationsAsynchronously); + private readonly TaskCompletionSource _executionScopeDisposed = new(TaskCreationOptions.RunContinuationsAsynchronously); + private int _filterInvocationCount; + private string? _matchedPrimitiveId; + + protected override void ConfigureServices(ServiceCollection services, IMcpServerBuilder mcpServerBuilder) + { + services.AddScoped(_ => new ScopedDependency(_executionScopeDisposed)); + + mcpServerBuilder + .WithTools() + .WithTasks(new InMemoryMcpTaskStore { DefaultPollIntervalMs = 10 }); + + mcpServerBuilder.Services.Configure(options => + options.Filters.Request.CallToolFilters.Add(next => async (request, cancellationToken) => + { + Interlocked.Increment(ref _filterInvocationCount); + _matchedPrimitiveId = request.MatchedPrimitive?.Id; + + try + { + await _continueBackgroundExecution.Task.WaitAsync(TestConstants.DefaultTimeout, cancellationToken); + _ = request.Services!.GetRequiredService(); + var result = await next(request, cancellationToken); + _executionCompleted.TrySetResult(result); + return result; + } + catch (Exception exception) + { + _executionCompleted.TrySetException(exception); + throw; + } + })); + } + + [Fact] + public async Task TaskBackedTool_RunsOrdinaryFilterOnce_InIndependentScope() + { + await using var client = await CreateMcpClientForServer(); + var cancellationToken = TestContext.Current.CancellationToken; + + var augmented = await client.CallToolAsTaskAsync( + new CallToolRequestParams { Name = "task-filter-tool" }, + cancellationToken); + + Assert.True(augmented.IsTask); + _continueBackgroundExecution.TrySetResult(true); + + var result = await _executionCompleted.Task.WaitAsync(TestConstants.DefaultTimeout, cancellationToken); + Assert.Equal("task filter result", Assert.IsType(Assert.Single(result.Content)).Text); + Assert.Equal(1, _filterInvocationCount); + Assert.Equal("task-filter-tool", _matchedPrimitiveId); + Assert.True(await _executionScopeDisposed.Task.WaitAsync(TestConstants.DefaultTimeout, cancellationToken)); + + var task = await client.GetTaskAsync(augmented.TaskCreated!.TaskId, cancellationToken); + Assert.IsType(task); + } + + private sealed class ScopedDependency(TaskCompletionSource disposed) : IAsyncDisposable + { + public ValueTask DisposeAsync() + { + disposed.TrySetResult(true); + return default; + } + } + + [McpServerToolType] + private sealed class TaskFilterTools + { + [McpServerTool(Name = "task-filter-tool")] + public static string Invoke(ScopedDependency dependency) => "task filter result"; + } +} From 18544965a731729a6889fd1d531ae740415efe00 Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Wed, 22 Jul 2026 14:09:31 -0700 Subject: [PATCH 2/8] Preserve tool filter semantics with tasks Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e431ee5-5f4d-441d-aae2-cd009626e564 --- .../Server/McpRequestFilters.cs | 4 +- .../Server/McpServerImpl.cs | 241 ++++++++++++++---- .../Server/McpTasksBuilderExtensions.cs | 11 +- .../TaskCallToolFilterCompositionTests.cs | 126 ++++++++- 4 files changed, 328 insertions(+), 54 deletions(-) diff --git a/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs b/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs index 94297c578..c86478e5d 100644 --- a/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs +++ b/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs @@ -75,8 +75,8 @@ public IList> CallToolFi /// /// /// When no explicit is configured, these filters - /// compose outside . The ordinary pipeline, including primitive matching and ordinary - /// filters, is adapted to before these filters are applied. + /// compose outside . Primitive matching occurs before either filter family runs, then + /// the ordinary pipeline is adapted to before these filters are applied. /// /// [Experimental(Experimentals.Subclassing_DiagnosticId, UrlFormat = Experimentals.Subclassing_Url)] diff --git a/src/ModelContextProtocol.Core/Server/McpServerImpl.cs b/src/ModelContextProtocol.Core/Server/McpServerImpl.cs index 768675868..b1374afef 100644 --- a/src/ModelContextProtocol.Core/Server/McpServerImpl.cs +++ b/src/ModelContextProtocol.Core/Server/McpServerImpl.cs @@ -33,6 +33,7 @@ internal sealed partial class McpServerImpl : McpServer private readonly SemaphoreSlim _disposeLock = new(1, 1); private readonly ConcurrentDictionary _mrtrContinuations = new(); private readonly ConcurrentDictionary _mrtrContextsByRequestId = new(); + private readonly AsyncLocal _currentComposedCallToolInvocation = new(); private static readonly string[] s_perRequestMetadataKeys = [ @@ -1408,15 +1409,12 @@ await originalListToolsHandler(request, cancellationToken).ConfigureAwait(false) }; } - callToolHandler = BuildFilterPipeline(callToolHandler, callToolFilters, BuildInitialCallToolFilter(tools)); + callToolHandler = BuildFilterPipeline(callToolHandler, callToolFilters); - // Convert to alternate-based. - var finalCallToolHandler = callToolHandler; - callToolWithAlternateHandler = async (request, cancellationToken) => - await finalCallToolHandler(request, cancellationToken).ConfigureAwait(false); - - // Alternate-result filters augment the fully constructed ordinary pipeline. - callToolWithAlternateHandler = BuildFilterPipeline(callToolWithAlternateHandler, callToolWithAlternateFilters); + callToolWithAlternateHandler = BuildComposedCallToolHandler( + callToolHandler, + callToolWithAlternateFilters, + tools); } ServerCapabilities.Tools.ListChanged = listChanged; @@ -1440,58 +1438,85 @@ private static async ValueTask> InvokeToolWith return await tool.InvokeAsync(request, cancellationToken).ConfigureAwait(false); } - private McpRequestFilter BuildInitialCallToolFilter( - McpServerPrimitiveCollection? tools) => handler => - async (request, cancellationToken) => - { - if (request.Params?.Name is { } toolName && tools is not null && - tools.TryGetPrimitive(toolName, out var tool)) + private McpRequestHandler> BuildComposedCallToolHandler( + McpRequestHandler callToolHandler, + IList>> callToolWithAlternateFilters, + McpServerPrimitiveCollection? tools) + { + McpRequestHandler> adaptedCallToolHandler = + async (request, cancellationToken) => { - request.MatchedPrimitive = tool; - } + var invocation = _currentComposedCallToolInvocation.Value ?? + throw new InvalidOperationException("The composed tool-call invocation was not initialized."); + + try + { + var result = await callToolHandler(request, cancellationToken).ConfigureAwait(false); + LogToolCallLifecycles(request, invocation.RecordOrdinaryResult(result)); + return result; + } + catch (Exception exception) + { + LogToolCallLifecycles( + request, + invocation.RecordOrdinaryException( + exception, + cancellationToken.IsCancellationRequested)); + + if ((exception is OperationCanceledException && cancellationToken.IsCancellationRequested) || + exception is McpProtocolException || + exception is InputRequiredException) + { + throw; + } + + return CreateToolCallErrorResult(request, exception); + } + }; + + var composedHandler = BuildFilterPipeline(adaptedCallToolHandler, callToolWithAlternateFilters); + + return async (request, cancellationToken) => + { + MatchTool(request, tools); + + var invocation = new ComposedCallToolInvocationState(); + var previousInvocation = _currentComposedCallToolInvocation.Value; + _currentComposedCallToolInvocation.Value = invocation; try { - var result = await handler(request, cancellationToken).ConfigureAwait(false); - ToolCallCompleted(request.Params?.Name ?? string.Empty, result.IsError is true); + var result = await composedHandler(request, cancellationToken).ConfigureAwait(false); + LogToolCallLifecycles(request, invocation.CompleteOuter(result)); return result; } catch (Exception e) { - // Skip logging for InputRequiredException - it's normal MRTR control flow, - // not an error (tools throw it to signal an InputRequiredResult). - if (!(e is OperationCanceledException && cancellationToken.IsCancellationRequested) && e is not InputRequiredException) - { - ToolCallError(request.Params?.Name ?? string.Empty, e); - } + LogToolCallLifecycles( + request, + invocation.CompleteOuterException( + e, + cancellationToken.IsCancellationRequested)); if ((e is OperationCanceledException && cancellationToken.IsCancellationRequested) || e is McpProtocolException || e is InputRequiredException) { throw; } - return new() - { - IsError = true, - Content = [new TextContentBlock - { - Text = e is McpException ? - $"An error occurred invoking '{request.Params?.Name}': {e.Message}" : - $"An error occurred invoking '{request.Params?.Name}'.", - }], - }; + return CreateToolCallErrorResult(request, e); + } + finally + { + _currentComposedCallToolInvocation.Value = previousInvocation; } }; + } private McpRequestFilter> BuildInitialAlternateToolFilter( McpServerPrimitiveCollection? tools) => handler => async (request, cancellationToken) => { - if (request.Params?.Name is { } toolName && tools is not null && - tools.TryGetPrimitive(toolName, out var tool)) - { - request.MatchedPrimitive = tool; - } + MatchTool(request, tools); try { @@ -1517,18 +1542,136 @@ private McpRequestFilter request, + McpServerPrimitiveCollection? tools) + { + if (request.Params?.Name is { } toolName && tools is not null && + tools.TryGetPrimitive(toolName, out var tool)) + { + request.MatchedPrimitive = tool; + } + } + + private static CallToolResult CreateToolCallErrorResult( + RequestContext request, + Exception exception) => + new() + { + IsError = true, + Content = [new TextContentBlock + { + Text = exception is McpException ? + $"An error occurred invoking '{request.Params?.Name}': {exception.Message}" : + $"An error occurred invoking '{request.Params?.Name}'.", + }], + }; + + private void LogToolCallLifecycles( + RequestContext request, + IReadOnlyList lifecycles) + { + string toolName = request.Params?.Name ?? string.Empty; + foreach (var lifecycle in lifecycles) + { + if (lifecycle.Result is { } result) + { + ToolCallCompleted(toolName, result.IsError is true); + } + else if (!(lifecycle.Exception is OperationCanceledException && lifecycle.CancellationRequested) && + lifecycle.Exception is not InputRequiredException) + { + ToolCallError(toolName, lifecycle.Exception!); + } + } + } + + private sealed class ComposedCallToolInvocationState + { + private readonly object _sync = new(); + private readonly List _pendingOrdinaryLifecycles = []; + private bool _outerCompleted; + private bool _outerReturnedAlternate; + + public IReadOnlyList RecordOrdinaryResult(CallToolResult result) => + RecordOrdinaryLifecycle(new(result, null, false)); + + public IReadOnlyList RecordOrdinaryException( + Exception exception, + bool cancellationRequested) => + RecordOrdinaryLifecycle(new(null, exception, cancellationRequested)); + + public IReadOnlyList CompleteOuter(ResultOrAlternate result) + { + lock (_sync) + { + if (result.IsAlternate) + { + _outerReturnedAlternate = true; + return DrainPendingLifecycles(); + } + + _outerCompleted = true; + if (_pendingOrdinaryLifecycles.Count > 0) + { + return DrainPendingLifecycles(); + } + + return [new(result.Result!, null, false)]; + } + } + + public IReadOnlyList CompleteOuterException( + Exception exception, + bool cancellationRequested) + { + lock (_sync) + { + _outerCompleted = true; + _pendingOrdinaryLifecycles.Clear(); + return [new(null, exception, cancellationRequested)]; + } + } + + private IReadOnlyList RecordOrdinaryLifecycle(ToolCallLifecycle lifecycle) + { + lock (_sync) + { + if (_outerReturnedAlternate) + { + return [lifecycle]; + } + + if (!_outerCompleted) + { + _pendingOrdinaryLifecycles.Add(lifecycle); + } + + return []; + } + } + + private IReadOnlyList DrainPendingLifecycles() + { + if (_pendingOrdinaryLifecycles.Count == 0) + { + return []; + } + + var lifecycles = _pendingOrdinaryLifecycles.ToArray(); + _pendingOrdinaryLifecycles.Clear(); + return lifecycles; + } + } + + private sealed record ToolCallLifecycle( + CallToolResult? Result, + Exception? Exception, + bool CancellationRequested); #pragma warning restore MCPEXP002 private void ConfigureLogging(McpServerOptions options) diff --git a/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs b/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs index dd2ec0dc9..b0cfd797c 100644 --- a/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs +++ b/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs @@ -93,7 +93,14 @@ private async ValueTask> RunAsTaskAsync( CancellationToken cancellationToken) { var executionScope = _serviceScopeFactory.CreateAsyncScope(); - request.Services = executionScope.ServiceProvider; + var executionRequest = new RequestContext( + request.Server, + request.JsonRpcRequest, + request.Params) + { + MatchedPrimitive = request.MatchedPrimitive, + Services = executionScope.ServiceProvider, + }; McpTaskInfo taskInfo; try @@ -114,7 +121,7 @@ private async ValueTask> RunAsTaskAsync( // before the background delegate starts. var taskCancellationToken = cts.Token; _ = Task.Run( - () => ExecuteTaskAsync(next, request, taskId, taskCancellationToken, executionScope), + () => ExecuteTaskAsync(next, executionRequest, taskId, taskCancellationToken, executionScope), CancellationToken.None); return ResultOrAlternate.FromAlternate( diff --git a/tests/ModelContextProtocol.Tests/Server/TaskCallToolFilterCompositionTests.cs b/tests/ModelContextProtocol.Tests/Server/TaskCallToolFilterCompositionTests.cs index 5b522b9c1..227246523 100644 --- a/tests/ModelContextProtocol.Tests/Server/TaskCallToolFilterCompositionTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/TaskCallToolFilterCompositionTests.cs @@ -11,8 +11,15 @@ public class TaskCallToolFilterCompositionTests(ITestOutputHelper testOutputHelp private readonly TaskCompletionSource _continueBackgroundExecution = new(TaskCreationOptions.RunContinuationsAsynchronously); private readonly TaskCompletionSource _executionCompleted = new(TaskCreationOptions.RunContinuationsAsynchronously); private readonly TaskCompletionSource _executionScopeDisposed = new(TaskCreationOptions.RunContinuationsAsynchronously); + private int _alternateFilterFactoryInvocationCount; private int _filterInvocationCount; private string? _matchedPrimitiveId; + private string? _alternateMatchedPrimitiveId; + private string? _throwingAlternateMatchedPrimitiveId; + private RequestContext? _alternateRequestContext; + private RequestContext? _ordinaryRequestContext; + private IServiceProvider? _alternateServicesBeforeNext; + private IServiceProvider? _alternateServicesAfterNext; protected override void ConfigureServices(ServiceCollection services, IMcpServerBuilder mcpServerBuilder) { @@ -23,10 +30,17 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer .WithTasks(new InMemoryMcpTaskStore { DefaultPollIntervalMs = 10 }); mcpServerBuilder.Services.Configure(options => + { options.Filters.Request.CallToolFilters.Add(next => async (request, cancellationToken) => { + if (request.Params?.Name != "task-filter-tool") + { + return await next(request, cancellationToken); + } + Interlocked.Increment(ref _filterInvocationCount); _matchedPrimitiveId = request.MatchedPrimitive?.Id; + _ordinaryRequestContext = request; try { @@ -41,7 +55,47 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer _executionCompleted.TrySetException(exception); throw; } - })); + }); + +#pragma warning disable MCPEXP002 // exercises the experimental CallToolWithAlternateFilters seam + options.Filters.Request.CallToolWithAlternateFilters.Add(next => + { + Interlocked.Increment(ref _alternateFilterFactoryInvocationCount); + return async (request, cancellationToken) => + { + if (request.Params?.Name == "alternate-filter-exception-tool") + { + _throwingAlternateMatchedPrimitiveId = request.MatchedPrimitive?.Id; + throw new InvalidOperationException("Alternate filter failure."); + } + + if (request.Params?.Name == "alternate-short-circuit-tool") + { + return new CallToolResult + { + Content = [new TextContentBlock { Text = "short-circuited" }], + }; + } + + if (request.Params?.Name == "replace-jsonrpc-request-tool") + { + request.JsonRpcRequest = new JsonRpcRequest + { + Id = request.JsonRpcRequest.Id, + Method = request.JsonRpcRequest.Method, + }; + } + + _alternateMatchedPrimitiveId = request.MatchedPrimitive?.Id; + _alternateRequestContext = request; + _alternateServicesBeforeNext = request.Services; + var result = await next(request, cancellationToken); + _alternateServicesAfterNext = request.Services; + return result; + }; + }); +#pragma warning restore MCPEXP002 + }); } [Fact] @@ -61,12 +115,73 @@ public async Task TaskBackedTool_RunsOrdinaryFilterOnce_InIndependentScope() Assert.Equal("task filter result", Assert.IsType(Assert.Single(result.Content)).Text); Assert.Equal(1, _filterInvocationCount); Assert.Equal("task-filter-tool", _matchedPrimitiveId); + Assert.Equal("task-filter-tool", _alternateMatchedPrimitiveId); + Assert.NotSame(_alternateRequestContext, _ordinaryRequestContext); + Assert.Same(_alternateServicesBeforeNext, _alternateServicesAfterNext); Assert.True(await _executionScopeDisposed.Task.WaitAsync(TestConstants.DefaultTimeout, cancellationToken)); var task = await client.GetTaskAsync(augmented.TaskCreated!.TaskId, cancellationToken); Assert.IsType(task); } + [Fact] + public async Task AlternateFilterException_IsConvertedToCallToolError() + { + await using var client = await CreateMcpClientForServer(); + + var result = await client.CallToolAsync( + "alternate-filter-exception-tool", + cancellationToken: TestContext.Current.CancellationToken); + + Assert.True(result.IsError); + Assert.Equal( + "An error occurred invoking 'alternate-filter-exception-tool'.", + Assert.IsType(Assert.Single(result.Content)).Text); + Assert.Equal("alternate-filter-exception-tool", _throwingAlternateMatchedPrimitiveId); + } + + [Fact] + public async Task AlternateFilterShortCircuit_LogsCompletionOnce() + { + await using var client = await CreateMcpClientForServer(); + + var result = await client.CallToolAsync( + "alternate-short-circuit-tool", + cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal("short-circuited", Assert.IsType(Assert.Single(result.Content)).Text); + Assert.Single( + MockLoggerProvider.LogMessages, + message => message.Message == "\"alternate-short-circuit-tool\" completed. IsError = False."); + } + + [Fact] + public async Task AlternateFilterFactory_IsCreatedOnce() + { + await using var client = await CreateMcpClientForServer(); + + await client.CallToolAsync( + "alternate-short-circuit-tool", + cancellationToken: TestContext.Current.CancellationToken); + await client.CallToolAsync( + "alternate-short-circuit-tool", + cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal(1, _alternateFilterFactoryInvocationCount); + } + + [Fact] + public async Task AlternateFilter_CanReplaceJsonRpcRequest() + { + await using var client = await CreateMcpClientForServer(); + + var result = await client.CallToolAsync( + "replace-jsonrpc-request-tool", + cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal("replacement succeeded", Assert.IsType(Assert.Single(result.Content)).Text); + } + private sealed class ScopedDependency(TaskCompletionSource disposed) : IAsyncDisposable { public ValueTask DisposeAsync() @@ -81,5 +196,14 @@ private sealed class TaskFilterTools { [McpServerTool(Name = "task-filter-tool")] public static string Invoke(ScopedDependency dependency) => "task filter result"; + + [McpServerTool(Name = "alternate-filter-exception-tool")] + public static string ThrowingAlternateFilterTarget() => "unreachable"; + + [McpServerTool(Name = "alternate-short-circuit-tool")] + public static string ShortCircuitedAlternateFilterTarget() => "unreachable"; + + [McpServerTool(Name = "replace-jsonrpc-request-tool")] + public static string ReplaceJsonRpcRequestTarget() => "replacement succeeded"; } } From e4c8b5187ec15aaef8a3a84e106224ef7bf25ef5 Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Wed, 22 Jul 2026 16:37:33 -0700 Subject: [PATCH 3/8] Authorize tool calls before task dispatch Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e431ee5-5f4d-441d-aae2-cd009626e564 --- .../AuthorizationFilterSetup.cs | 8 +++- .../HttpTaskIntegrationTests.cs | 42 +++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/ModelContextProtocol.AspNetCore/AuthorizationFilterSetup.cs b/src/ModelContextProtocol.AspNetCore/AuthorizationFilterSetup.cs index 3f5870700..5f9ca3dd3 100644 --- a/src/ModelContextProtocol.AspNetCore/AuthorizationFilterSetup.cs +++ b/src/ModelContextProtocol.AspNetCore/AuthorizationFilterSetup.cs @@ -81,7 +81,8 @@ private static void CheckListToolsFilter(McpServerOptions options) private void ConfigureCallToolFilter(McpServerOptions options) { - options.Filters.Request.CallToolFilters.Add(next => async (context, cancellationToken) => +#pragma warning disable MCPEXP002 // Authorization must run in the alternate-result pipeline before task dispatch. + options.Filters.Request.CallToolWithAlternateFilters.Add(next => async (context, cancellationToken) => { var authResult = await GetAuthorizationResultAsync(context.User, context.MatchedPrimitive, context.Services, context); if (!authResult.Succeeded) @@ -93,11 +94,13 @@ private void ConfigureCallToolFilter(McpServerOptions options) return await next(context, cancellationToken); }); +#pragma warning restore MCPEXP002 } private static void CheckCallToolFilter(McpServerOptions options) { - options.Filters.Request.CallToolFilters.Add(next => async (context, cancellationToken) => +#pragma warning disable MCPEXP002 // Keep the authorization guard in the same pipeline as authorization. + options.Filters.Request.CallToolWithAlternateFilters.Add(next => async (context, cancellationToken) => { if (HasAuthorizationMetadata(context.MatchedPrimitive) && !context.Items.ContainsKey(AuthorizationFilterInvokedKey)) @@ -107,6 +110,7 @@ private static void CheckCallToolFilter(McpServerOptions options) return await next(context, cancellationToken); }); +#pragma warning restore MCPEXP002 } private void ConfigureListResourcesFilter(McpServerOptions options) diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/HttpTaskIntegrationTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/HttpTaskIntegrationTests.cs index e4152829c..14f90152e 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/HttpTaskIntegrationTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/HttpTaskIntegrationTests.cs @@ -1,10 +1,12 @@ using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Authorization; using Microsoft.Extensions.DependencyInjection; using ModelContextProtocol.AspNetCore.Tests.Utils; using ModelContextProtocol.Client; using ModelContextProtocol.Extensions.Tasks; using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; +using Moq; namespace ModelContextProtocol.AspNetCore.Tests; @@ -39,10 +41,50 @@ public async Task WithTasks_CanCallToolOverHttp() Assert.Equal("Hello World!", Assert.IsType(Assert.Single(result.Content)).Text); } + [Fact] + public async Task WithTasks_UnauthorizedTool_DoesNotCreateTask() + { + var taskStore = new Mock(MockBehavior.Strict); + Builder.Services + .AddMcpServer() + .WithHttpTransport() + .AddAuthorizationFilters() + .WithTasks(taskStore.Object) + .WithTools(); + Builder.Services.AddAuthorization(); + + await using var app = Builder.Build(); + app.MapMcp(); + await app.StartAsync(TestContext.Current.CancellationToken); + + await using var transport = new HttpClientTransport( + new HttpClientTransportOptions { Endpoint = new("http://localhost:5000") }, + HttpClient, + LoggerFactory); + await using var client = await McpClient.CreateAsync( + transport, + loggerFactory: LoggerFactory, + cancellationToken: TestContext.Current.CancellationToken); + + var exception = await Assert.ThrowsAsync(() => + client.CallToolAsTaskAsync( + new CallToolRequestParams { Name = "authorized-test" }, + TestContext.Current.CancellationToken).AsTask()); + + Assert.Equal(McpErrorCode.InvalidRequest, exception.ErrorCode); + taskStore.Verify( + store => store.CreateTaskAsync(It.IsAny()), + Times.Never); + } + [McpServerToolType] private sealed class TestTools { [McpServerTool(Name = "test")] public static string Test() => "Hello World!"; + + [McpServerTool(Name = "authorized-test")] + [Authorize] + public static string AuthorizedTest() => "unreachable"; } } From ed2b222fe01310e4c058e43a65efc9eedcf11599 Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Wed, 22 Jul 2026 16:51:36 -0700 Subject: [PATCH 4/8] Document task filter ordering Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e431ee5-5f4d-441d-aae2-cd009626e564 --- docs/concepts/filters.md | 16 ++++++++++++---- docs/concepts/tasks/tasks.md | 12 +++++++++--- .../HttpMcpServerBuilderExtensions.cs | 3 ++- .../Server/McpRequestFilters.cs | 5 ++++- .../Server/McpTasksBuilderExtensions.cs | 5 +++++ .../McpRequestFilterBuilderExtensions.cs | 4 ++++ 6 files changed, 36 insertions(+), 9 deletions(-) diff --git a/docs/concepts/filters.md b/docs/concepts/filters.md index 51a1a491e..885f0ba4e 100644 --- a/docs/concepts/filters.md +++ b/docs/concepts/filters.md @@ -30,10 +30,18 @@ The following request filter methods are available on `IMcpRequestFilterBuilder` - `AddUnsubscribeFromResourcesFilter` - Filter for resource unsubscription handlers - `AddSetLoggingLevelFilter` - Filter for logging level handlers -The Tasks extension augments the ordinary `tools/call` pipeline with an alternate-result filter. -Ordinary call-tool filters, including ASP.NET Core authorization filters, run exactly once inside -that wrapper before the tool executes. An explicit `CallToolWithAlternateHandler` remains a -full replacement and cannot be combined with ordinary call-tool filters. +The Tasks extension and ASP.NET Core tool authorization use alternate-result `tools/call` filters. +Alternate-result filters run in registration order outside the ordinary `AddCallToolFilter` +pipeline. The Tasks filter creates a task at its position in that order and runs its remaining +pipeline in the background. Consequently, alternate-result filters registered before Tasks run +before task creation, while those registered after Tasks run in the background before the +ordinary filters and tool. ASP.NET Core authorization is registered before Tasks, so an +unauthorized call is rejected without creating a task. + +Ordinary call-tool filters run exactly once after all alternate-result filters and before the tool. +For task-backed calls, ordinary filters execute in the background after task creation. An explicit +`CallToolWithAlternateHandler` remains a full replacement and cannot be combined with ordinary +call-tool filters. ## Message Filters diff --git a/docs/concepts/tasks/tasks.md b/docs/concepts/tasks/tasks.md index 05820d467..68b2bc756 100644 --- a/docs/concepts/tasks/tasks.md +++ b/docs/concepts/tasks/tasks.md @@ -79,10 +79,16 @@ When tasks are enabled with `WithTasks` the SDK automatically: - Plumbs a `CancellationToken` through to the tool that fires when the client invokes `tasks/cancel`, so cancellation propagates cooperatively. +Alternate-result `tools/call` filters run in registration order, with the Tasks filter creating a +task at its position in that order. Filters before Tasks run before task creation. Filters after +Tasks run in the background before the ordinary filter pipeline. ASP.NET Core tool authorization +uses an alternate-result filter registered before Tasks, so an unauthorized call does not create +a task. + Ordinary `tools/call` filters still run exactly once for task-backed calls. They execute in the -background after the task record is created and before the tool body, so validation, -authorization, and telemetry continue to apply. Each background invocation gets an independent -DI scope that remains alive until the tool pipeline completes. +background after the task record is created and before the tool body, so validation and telemetry +continue to apply. Each background invocation gets an independent DI scope that remains alive +until the tool pipeline completes. For production scenarios that need durability, session isolation, multi-process routing, or TTL-based cleanup, implement yourself diff --git a/src/ModelContextProtocol.AspNetCore/HttpMcpServerBuilderExtensions.cs b/src/ModelContextProtocol.AspNetCore/HttpMcpServerBuilderExtensions.cs index 732821baa..6cf1fdf7f 100644 --- a/src/ModelContextProtocol.AspNetCore/HttpMcpServerBuilderExtensions.cs +++ b/src/ModelContextProtocol.AspNetCore/HttpMcpServerBuilderExtensions.cs @@ -55,7 +55,8 @@ public static IMcpServerBuilder WithHttpTransport(this IMcpServerBuilder builder /// /// This method automatically configures authorization filters for all MCP server handlers. These filters respect /// authorization attributes such as - /// and . + /// and . Tool authorization runs in the alternate-result pipeline before + /// the Tasks extension dispatches background execution, so an unauthorized tool call does not create a task. /// public static IMcpServerBuilder AddAuthorizationFilters(this IMcpServerBuilder builder) { diff --git a/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs b/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs index c86478e5d..bb19c2ac7 100644 --- a/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs +++ b/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs @@ -45,7 +45,8 @@ public IList> ListTool /// /// These filters run inside . Each ordinary filter runs exactly once /// when an alternate-result filter invokes the ordinary tool pipeline. For task-backed calls, that invocation - /// occurs in the background after the task record is created and before the matched tool is executed. + /// occurs in the background after the task record is created and before the matched tool is executed. Filters + /// that must run before task creation should use the alternate-result pipeline instead. /// /// /// These filters cannot be used with an explicit , @@ -77,6 +78,8 @@ public IList> CallToolFi /// When no explicit is configured, these filters /// compose outside . Primitive matching occurs before either filter family runs, then /// the ordinary pipeline is adapted to before these filters are applied. + /// Alternate-result filters run in registration order. If one filter dispatches the remainder of the pipeline + /// asynchronously, filters registered after it execute as part of that asynchronous operation. /// /// [Experimental(Experimentals.Subclassing_DiagnosticId, UrlFormat = Experimentals.Subclassing_Url)] diff --git a/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs b/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs index b0cfd797c..e0bd08009 100644 --- a/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs +++ b/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs @@ -19,6 +19,11 @@ public static class McpTasksBuilderExtensions /// /// Enables MCP Tasks support backed by the specified task store. /// + /// + /// Tasks are implemented as an alternate-result call-tool filter. Alternate-result filters registered before + /// the Tasks filter run before task creation. Filters registered after it, along with all ordinary call-tool + /// filters, run in the background before the tool. + /// /// The server builder. /// The task store. /// The builder provided in . diff --git a/src/ModelContextProtocol/McpRequestFilterBuilderExtensions.cs b/src/ModelContextProtocol/McpRequestFilterBuilderExtensions.cs index 2ed06355f..db939c61b 100644 --- a/src/ModelContextProtocol/McpRequestFilterBuilderExtensions.cs +++ b/src/ModelContextProtocol/McpRequestFilterBuilderExtensions.cs @@ -42,6 +42,10 @@ public static IMcpRequestFilterBuilder AddListToolsFilter(this IMcpRequestFilter /// /// Adds a filter to the call tool handler pipeline. /// + /// + /// This ordinary call-tool filter runs inside all alternate-result call-tool filters. For a task-backed call, + /// it executes in the background after task creation and before the tool. + /// /// The request filter builder instance. /// The filter function that wraps the handler. /// The builder provided in . From 60ad05efc1bec3888235ae62972c242b96f5f157 Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Wed, 22 Jul 2026 17:17:03 -0700 Subject: [PATCH 5/8] Log final alternate filter results Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e431ee5-5f4d-441d-aae2-cd009626e564 --- .../Server/McpServerImpl.cs | 13 ++++---- .../TaskCallToolFilterCompositionTests.cs | 30 +++++++++++++++++++ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/ModelContextProtocol.Core/Server/McpServerImpl.cs b/src/ModelContextProtocol.Core/Server/McpServerImpl.cs index b1374afef..7342523fa 100644 --- a/src/ModelContextProtocol.Core/Server/McpServerImpl.cs +++ b/src/ModelContextProtocol.Core/Server/McpServerImpl.cs @@ -1616,12 +1616,13 @@ public IReadOnlyList CompleteOuter(ResultOrAlternate 0) - { - return DrainPendingLifecycles(); - } - - return [new(result.Result!, null, false)]; + var exceptions = _pendingOrdinaryLifecycles + .Where(lifecycle => lifecycle.Exception is not null) + .ToArray(); + _pendingOrdinaryLifecycles.Clear(); + return exceptions.Length > 0 ? + exceptions : + [new(result.Result!, null, false)]; } } diff --git a/tests/ModelContextProtocol.Tests/Server/TaskCallToolFilterCompositionTests.cs b/tests/ModelContextProtocol.Tests/Server/TaskCallToolFilterCompositionTests.cs index 227246523..fd35cef0a 100644 --- a/tests/ModelContextProtocol.Tests/Server/TaskCallToolFilterCompositionTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/TaskCallToolFilterCompositionTests.cs @@ -86,6 +86,16 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer }; } + if (request.Params?.Name == "alternate-transform-result-tool") + { + _ = await next(request, cancellationToken); + return new CallToolResult + { + IsError = true, + Content = [new TextContentBlock { Text = "transformed" }], + }; + } + _alternateMatchedPrimitiveId = request.MatchedPrimitive?.Id; _alternateRequestContext = request; _alternateServicesBeforeNext = request.Services; @@ -182,6 +192,23 @@ public async Task AlternateFilter_CanReplaceJsonRpcRequest() Assert.Equal("replacement succeeded", Assert.IsType(Assert.Single(result.Content)).Text); } + [Fact] + public async Task AlternateFilterTransformedResult_LogsFinalResult() + { + await using var client = await CreateMcpClientForServer(); + + var result = await client.CallToolAsync( + "alternate-transform-result-tool", + cancellationToken: TestContext.Current.CancellationToken); + + Assert.True(result.IsError); + Assert.Equal("transformed", Assert.IsType(Assert.Single(result.Content)).Text); + var completionLog = Assert.Single( + MockLoggerProvider.LogMessages, + message => message.Message.StartsWith("\"alternate-transform-result-tool\" completed.", StringComparison.Ordinal)); + Assert.Equal("\"alternate-transform-result-tool\" completed. IsError = True.", completionLog.Message); + } + private sealed class ScopedDependency(TaskCompletionSource disposed) : IAsyncDisposable { public ValueTask DisposeAsync() @@ -205,5 +232,8 @@ private sealed class TaskFilterTools [McpServerTool(Name = "replace-jsonrpc-request-tool")] public static string ReplaceJsonRpcRequestTarget() => "replacement succeeded"; + + [McpServerTool(Name = "alternate-transform-result-tool")] + public static string AlternateTransformResultTarget() => "original"; } } From 5c691986aa7f8b4f8450436c200200cab108673a Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Thu, 23 Jul 2026 18:22:04 -0700 Subject: [PATCH 6/8] Address HTTP task filter review feedback Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a93a04c7-4270-428a-b598-a2cc76995bf4 --- docs/concepts/filters.md | 4 + docs/concepts/tasks/tasks.md | 22 -- src/Directory.Build.targets | 13 ++ .../AuthorizationCallToolFilterGuardSetup.cs | 32 +++ .../AuthorizationFilterSetup.cs | 32 +-- .../HttpMcpServerBuilderExtensions.cs | 3 + .../Server/ComposedCallToolInvocationState.cs | 89 ++++++++ .../Server/McpRequestFilterCollection.cs | 40 ++++ .../Server/McpRequestFilters.cs | 39 +++- .../Server/McpRequestInvocationFilter.cs | 18 ++ .../Server/McpServer.Methods.cs | 41 +--- .../Server/McpServerImpl.cs | 192 ++++++----------- .../OutgoingRequestInterceptingMcpServer.cs | 56 +++++ .../Server/McpTasksBuilderExtensions.cs | 34 +-- .../Server/McpTasksServerExtensions.cs | 14 +- .../HttpTaskIntegrationTests.cs | 118 ++++++++++- .../Server/CallToolFilterMixingTests.cs | 20 +- .../TaskCallToolFilterCompositionTests.cs | 197 +++++++++++++----- 18 files changed, 669 insertions(+), 295 deletions(-) create mode 100644 src/Directory.Build.targets create mode 100644 src/ModelContextProtocol.AspNetCore/AuthorizationCallToolFilterGuardSetup.cs create mode 100644 src/ModelContextProtocol.Core/Server/ComposedCallToolInvocationState.cs create mode 100644 src/ModelContextProtocol.Core/Server/McpRequestFilterCollection.cs create mode 100644 src/ModelContextProtocol.Core/Server/McpRequestInvocationFilter.cs create mode 100644 src/ModelContextProtocol.Core/Server/OutgoingRequestInterceptingMcpServer.cs diff --git a/docs/concepts/filters.md b/docs/concepts/filters.md index 885f0ba4e..0ab6fce4c 100644 --- a/docs/concepts/filters.md +++ b/docs/concepts/filters.md @@ -43,6 +43,10 @@ For task-backed calls, ordinary filters execute in the background after task cre `CallToolWithAlternateHandler` remains a full replacement and cannot be combined with ordinary call-tool filters. +Register alternate-result filters before ordinary call-tool filters. Adding an alternate-result +filter after an ordinary filter throws `InvalidOperationException` because execution order cannot +match registration order in that arrangement. + ## Message Filters In addition to the request-specific filters above, there are low-level message filters that intercept all JSON-RPC messages before they are routed to specific handlers. diff --git a/docs/concepts/tasks/tasks.md b/docs/concepts/tasks/tasks.md index 68b2bc756..69d2a834f 100644 --- a/docs/concepts/tasks/tasks.md +++ b/docs/concepts/tasks/tasks.md @@ -137,28 +137,6 @@ options.Handlers.CallToolWithAlternateHandler = async (context, ct) => > are mutually exclusive. Setting one while the other is already non-null throws > `InvalidOperationException` at the property setter. -#### Task scope for server-initiated requests - -When you start background work from a custom -(rather than the SDK's auto-wrapping), use -to route elicitation, sampling, and `roots/list` calls through the task store as input requests -instead of direct JSON-RPC messages: - -```csharp -using ModelContextProtocol.Extensions.Tasks; - -using (server.CreateMcpTaskScope(taskId, taskStore)) -{ - // ElicitAsync/SampleAsync/RequestRootsAsync calls in here are surfaced as - // entries in the task's inputRequests, then await client responses via tasks/update. - var elicit = await server.ElicitAsync(elicitParams, ct); -} -``` - -`CreateMcpTaskScope` returns an `IDisposable` that restores the prior ambient context on -`Dispose`. The scope is established automatically for `[McpServerTool]` methods that run via -`WithTasks`, so this API is only needed for custom handlers. - ### Client usage #### Automatic polling diff --git a/src/Directory.Build.targets b/src/Directory.Build.targets new file mode 100644 index 000000000..ee635a551 --- /dev/null +++ b/src/Directory.Build.targets @@ -0,0 +1,13 @@ + + + + <_ProjectReferencesWithVersions Update="@(_ProjectReferencesWithVersions)"> + [%(_ProjectReferencesWithVersions.ProjectVersion)] + + + + diff --git a/src/ModelContextProtocol.AspNetCore/AuthorizationCallToolFilterGuardSetup.cs b/src/ModelContextProtocol.AspNetCore/AuthorizationCallToolFilterGuardSetup.cs new file mode 100644 index 000000000..8f41a5bd3 --- /dev/null +++ b/src/ModelContextProtocol.AspNetCore/AuthorizationCallToolFilterGuardSetup.cs @@ -0,0 +1,32 @@ +using Microsoft.Extensions.Options; +using ModelContextProtocol.Protocol; +using ModelContextProtocol.Server; + +namespace ModelContextProtocol.AspNetCore; + +internal sealed class AuthorizationFiltersMarker; + +internal sealed class AuthorizationCallToolFilterGuardSetup(AuthorizationFiltersMarker? marker = null) : IPostConfigureOptions +{ + public void PostConfigure(string? name, McpServerOptions options) + { + if (marker is not null) + { + return; + } + +#pragma warning disable MCPEXP002 // The guard must run before Tasks can dispatch the request. + options.Filters.Request.CallToolWithAlternateFilters.Insert(0, static async (context, next, cancellationToken) => + { + if (AuthorizationFilterSetup.HasAuthorizationMetadata(context.MatchedPrimitive)) + { + throw new InvalidOperationException( + "Authorization filter was not invoked for tools/call operation, but authorization metadata was found on the tool. " + + "Ensure that AddAuthorizationFilters() is called on the IMcpServerBuilder to configure authorization filters."); + } + + return await next(context, cancellationToken); + }); +#pragma warning restore MCPEXP002 + } +} diff --git a/src/ModelContextProtocol.AspNetCore/AuthorizationFilterSetup.cs b/src/ModelContextProtocol.AspNetCore/AuthorizationFilterSetup.cs index 5f9ca3dd3..5b371d9a1 100644 --- a/src/ModelContextProtocol.AspNetCore/AuthorizationFilterSetup.cs +++ b/src/ModelContextProtocol.AspNetCore/AuthorizationFilterSetup.cs @@ -11,14 +11,15 @@ namespace ModelContextProtocol.AspNetCore; /// /// Evaluates authorization policies from endpoint metadata. /// -internal sealed class AuthorizationFilterSetup(IAuthorizationPolicyProvider? policyProvider = null) : IConfigureOptions, IPostConfigureOptions +internal sealed class AuthorizationFilterSetup( + IAuthorizationPolicyProvider? policyProvider = null, + AuthorizationFiltersMarker? marker = null) : IConfigureOptions, IPostConfigureOptions { private static readonly string AuthorizationFilterInvokedKey = "ModelContextProtocol.AspNetCore.AuthorizationFilter.Invoked"; public void Configure(McpServerOptions options) { ConfigureListToolsFilter(options); - ConfigureCallToolFilter(options); ConfigureListResourcesFilter(options); ConfigureListResourceTemplatesFilter(options); @@ -30,8 +31,13 @@ public void Configure(McpServerOptions options) public void PostConfigure(string? name, McpServerOptions options) { + // Add tool authorization after all regular configuration so it always wraps Tasks. + if (marker is not null) + { + ConfigureCallToolFilter(options); + } + CheckListToolsFilter(options); - CheckCallToolFilter(options); CheckListResourcesFilter(options); CheckListResourceTemplatesFilter(options); @@ -82,7 +88,7 @@ private static void CheckListToolsFilter(McpServerOptions options) private void ConfigureCallToolFilter(McpServerOptions options) { #pragma warning disable MCPEXP002 // Authorization must run in the alternate-result pipeline before task dispatch. - options.Filters.Request.CallToolWithAlternateFilters.Add(next => async (context, cancellationToken) => + options.Filters.Request.CallToolWithAlternateFilters.Insert(0, async (context, next, cancellationToken) => { var authResult = await GetAuthorizationResultAsync(context.User, context.MatchedPrimitive, context.Services, context); if (!authResult.Succeeded) @@ -97,22 +103,6 @@ private void ConfigureCallToolFilter(McpServerOptions options) #pragma warning restore MCPEXP002 } - private static void CheckCallToolFilter(McpServerOptions options) - { -#pragma warning disable MCPEXP002 // Keep the authorization guard in the same pipeline as authorization. - options.Filters.Request.CallToolWithAlternateFilters.Add(next => async (context, cancellationToken) => - { - if (HasAuthorizationMetadata(context.MatchedPrimitive) - && !context.Items.ContainsKey(AuthorizationFilterInvokedKey)) - { - throw new InvalidOperationException("Authorization filter was not invoked for tools/call operation, but authorization metadata was found on the tool. Ensure that AddAuthorizationFilters() is called on the IMcpServerBuilder to configure authorization filters."); - } - - return await next(context, cancellationToken); - }); -#pragma warning restore MCPEXP002 - } - private void ConfigureListResourcesFilter(McpServerOptions options) { options.Filters.Request.ListResourcesFilters.Add(next => @@ -378,7 +368,7 @@ private async ValueTask GetAuthorizationResultAsync( : AuthorizationPolicy.Combine(policy, reqPolicyBuilder.Build()); } - private static bool HasAuthorizationMetadata([NotNullWhen(true)] IMcpServerPrimitive? primitive) + internal static bool HasAuthorizationMetadata([NotNullWhen(true)] IMcpServerPrimitive? primitive) { // If no primitive was found for this request or there is IAllowAnonymous metadata anywhere on the class or method, // the request should go through as normal. diff --git a/src/ModelContextProtocol.AspNetCore/HttpMcpServerBuilderExtensions.cs b/src/ModelContextProtocol.AspNetCore/HttpMcpServerBuilderExtensions.cs index 6cf1fdf7f..801da5f6e 100644 --- a/src/ModelContextProtocol.AspNetCore/HttpMcpServerBuilderExtensions.cs +++ b/src/ModelContextProtocol.AspNetCore/HttpMcpServerBuilderExtensions.cs @@ -34,6 +34,7 @@ public static IMcpServerBuilder WithHttpTransport(this IMcpServerBuilder builder builder.Services.AddHostedService(); builder.Services.TryAddEnumerable(ServiceDescriptor.Transient, AuthorizationFilterSetup>()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Transient, AuthorizationCallToolFilterGuardSetup>()); builder.Services.TryAddEnumerable(ServiceDescriptor.Transient, HttpServerTransportOptionsSetup>()); if (configureOptions is not null) @@ -63,7 +64,9 @@ public static IMcpServerBuilder AddAuthorizationFilters(this IMcpServerBuilder b ArgumentNullException.ThrowIfNull(builder); // Allow the authorization filters to get added multiple times in case other middleware changes the matched primitive. + builder.Services.TryAddSingleton(); builder.Services.AddTransient, AuthorizationFilterSetup>(); + builder.Services.TryAddEnumerable(ServiceDescriptor.Transient, AuthorizationFilterSetup>()); return builder; } diff --git a/src/ModelContextProtocol.Core/Server/ComposedCallToolInvocationState.cs b/src/ModelContextProtocol.Core/Server/ComposedCallToolInvocationState.cs new file mode 100644 index 000000000..fd194ca32 --- /dev/null +++ b/src/ModelContextProtocol.Core/Server/ComposedCallToolInvocationState.cs @@ -0,0 +1,89 @@ +using ModelContextProtocol.Protocol; + +namespace ModelContextProtocol.Server; + +#pragma warning disable MCPEXP002 +internal sealed class ComposedCallToolInvocationState +{ + private readonly object _sync = new(); + private readonly List _pendingOrdinaryLifecycles = []; + private bool _outerCompleted; + private bool _outerReturnedAlternate; + + public IReadOnlyList RecordOrdinaryResult(CallToolResult result) => + RecordOrdinaryLifecycle(new(result, null, false)); + + public IReadOnlyList RecordOrdinaryException( + Exception exception, + bool cancellationRequested) => + RecordOrdinaryLifecycle(new(null, exception, cancellationRequested)); + + public IReadOnlyList CompleteOuter(ResultOrAlternate result) + { + lock (_sync) + { + if (result.IsAlternate) + { + _outerReturnedAlternate = true; + return DrainPendingLifecycles(); + } + + _outerCompleted = true; + var exceptions = _pendingOrdinaryLifecycles + .Where(lifecycle => lifecycle.Exception is not null) + .ToArray(); + _pendingOrdinaryLifecycles.Clear(); + return exceptions.Length > 0 ? + exceptions : + [new(result.Result!, null, false)]; + } + } + + public IReadOnlyList CompleteOuterException( + Exception exception, + bool cancellationRequested) + { + lock (_sync) + { + _outerCompleted = true; + _pendingOrdinaryLifecycles.Clear(); + return [new(null, exception, cancellationRequested)]; + } + } + + private IReadOnlyList RecordOrdinaryLifecycle(ToolCallLifecycle lifecycle) + { + lock (_sync) + { + if (_outerReturnedAlternate) + { + return [lifecycle]; + } + + if (!_outerCompleted) + { + _pendingOrdinaryLifecycles.Add(lifecycle); + } + + return []; + } + } + + private IReadOnlyList DrainPendingLifecycles() + { + if (_pendingOrdinaryLifecycles.Count == 0) + { + return []; + } + + var lifecycles = _pendingOrdinaryLifecycles.ToArray(); + _pendingOrdinaryLifecycles.Clear(); + return lifecycles; + } +} + +internal sealed record ToolCallLifecycle( + CallToolResult? Result, + Exception? Exception, + bool CancellationRequested); +#pragma warning restore MCPEXP002 diff --git a/src/ModelContextProtocol.Core/Server/McpRequestFilterCollection.cs b/src/ModelContextProtocol.Core/Server/McpRequestFilterCollection.cs new file mode 100644 index 000000000..13fa716c4 --- /dev/null +++ b/src/ModelContextProtocol.Core/Server/McpRequestFilterCollection.cs @@ -0,0 +1,40 @@ +using System.Collections; + +namespace ModelContextProtocol.Server; + +internal sealed class McpRequestFilterCollection(IList filters, Action validateInsertion) : IList +{ + public T this[int index] + { + get => filters[index]; + set => filters[index] = value; + } + + public int Count => filters.Count; + + public bool IsReadOnly => filters.IsReadOnly; + + public void Add(T item) + { + validateInsertion(); + filters.Add(item); + } + + public void Clear() => filters.Clear(); + + public bool Contains(T item) => filters.Contains(item); + + public void CopyTo(T[] array, int arrayIndex) => filters.CopyTo(array, arrayIndex); + + public IEnumerator GetEnumerator() => filters.GetEnumerator(); + + public int IndexOf(T item) => filters.IndexOf(item); + + public void Insert(int index, T item) => filters.Insert(index, item); + + public bool Remove(T item) => filters.Remove(item); + + public void RemoveAt(int index) => filters.RemoveAt(index); + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); +} diff --git a/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs b/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs index bb19c2ac7..4afb04aa2 100644 --- a/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs +++ b/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs @@ -8,6 +8,10 @@ namespace ModelContextProtocol.Server; /// public sealed class McpRequestFilters { +#pragma warning disable MCPEXP002 // CallToolWithAlternateFilters references the experimental ResultOrAlternate seam + private IList>? _callToolFilters; + private IList>>? _callToolWithAlternateFilters; + /// /// Gets or sets the filters for the list-tools handler pipeline. /// @@ -55,15 +59,14 @@ public IList> ListTool /// public IList> CallToolFilters { - get => field ??= []; + get => _callToolFilters ??= []; set { Throw.IfNull(value); - field = value; + _callToolFilters = value; } } -#pragma warning disable MCPEXP002 // CallToolWithAlternateFilters references the experimental ResultOrAlternate seam /// /// Gets or sets the filters for the call-tool handler pipeline with alternate result support. /// @@ -81,15 +84,39 @@ public IList> CallToolFi /// Alternate-result filters run in registration order. If one filter dispatches the remainder of the pipeline /// asynchronously, filters registered after it execute as part of that asynchronous operation. /// + /// + /// Alternate-result filters must be registered before ordinary . + /// Registering an alternate-result filter after an ordinary filter throws an + /// because the two pipelines cannot preserve that registration order. + /// /// [Experimental(Experimentals.Subclassing_DiagnosticId, UrlFormat = Experimentals.Subclassing_Url)] - public IList>> CallToolWithAlternateFilters + public IList>> CallToolWithAlternateFilters { - get => field ??= []; + get => _callToolWithAlternateFilters ??= new McpRequestFilterCollection>>( + [], + ValidateAlternateFilterRegistration); set { Throw.IfNull(value); - field = value; + if (value.Count > 0) + { + ValidateAlternateFilterRegistration(); + } + + _callToolWithAlternateFilters = + new McpRequestFilterCollection>>( + value, + ValidateAlternateFilterRegistration); + } + } + private void ValidateAlternateFilterRegistration() + { + if (CallToolFilters.Count > 0) + { + throw new InvalidOperationException( + $"{nameof(CallToolWithAlternateFilters)} must be registered before {nameof(CallToolFilters)} because " + + "alternate-result filters always execute outside ordinary call-tool filters."); } } #pragma warning restore MCPEXP002 diff --git a/src/ModelContextProtocol.Core/Server/McpRequestInvocationFilter.cs b/src/ModelContextProtocol.Core/Server/McpRequestInvocationFilter.cs new file mode 100644 index 000000000..f9871fca5 --- /dev/null +++ b/src/ModelContextProtocol.Core/Server/McpRequestInvocationFilter.cs @@ -0,0 +1,18 @@ +using System.Diagnostics.CodeAnalysis; + +namespace ModelContextProtocol.Server; + +/// +/// Delegate type for filtering a single incoming MCP request invocation. +/// +/// The type of the parameters sent with the request. +/// The type of the response returned by the handler. +/// The context for the current request. +/// The next request handler in the pipeline for this invocation. +/// The cancellation token for the current request. +/// The result of the filtered request invocation. +[Experimental(Experimentals.Subclassing_DiagnosticId, UrlFormat = Experimentals.Subclassing_Url)] +public delegate ValueTask McpRequestInvocationFilter( + RequestContext context, + McpRequestHandler next, + CancellationToken cancellationToken); diff --git a/src/ModelContextProtocol.Core/Server/McpServer.Methods.cs b/src/ModelContextProtocol.Core/Server/McpServer.Methods.cs index 6e3c94f6a..a4a245043 100644 --- a/src/ModelContextProtocol.Core/Server/McpServer.Methods.cs +++ b/src/ModelContextProtocol.Core/Server/McpServer.Methods.cs @@ -23,50 +23,27 @@ public abstract partial class McpServer : McpSession private static Dictionary>? s_elicitAllowedProperties = null; - /// - /// Ambient interceptor that, when installed, redirects server-initiated requests - /// (, - /// , and - /// ) away from the - /// transport. The interceptor receives the request method and pre-serialized parameters and - /// returns the serialized result. Used by extensions (such as the Tasks extension) to surface - /// these requests through an alternate channel during background execution. - /// - private static readonly AsyncLocal>?> s_outgoingRequestInterceptor = new(); - - /// - /// Gets the currently installed outgoing-request interceptor for the ambient execution context, if any. - /// - internal static Func>? CurrentOutgoingRequestInterceptor => s_outgoingRequestInterceptor.Value; + internal virtual Func>? OutgoingRequestInterceptor => null; /// - /// Installs an interceptor that redirects server-initiated requests for the duration of the - /// returned scope on the current asynchronous execution context. + /// Creates a non-mutating server facade that redirects server-initiated requests through an interceptor. /// /// /// The interceptor invoked for each outgoing request. It receives the request method, the /// pre-serialized request parameters (or ), and a cancellation token, and /// returns the serialized result (or to indicate no result). /// - /// An that restores the previous interceptor when disposed. + /// A server facade that uses for outgoing requests. /// is . /// - /// While an interceptor is installed, the redirected methods skip their client-capability checks, + /// On the returned facade, redirected methods skip their client-capability checks, /// because the alternate channel is responsible for delivering the request to the client. /// [Experimental(Experimentals.Subclassing_DiagnosticId, UrlFormat = Experimentals.Subclassing_Url)] - public IDisposable InterceptOutgoingRequests(Func> interceptor) + public McpServer WithOutgoingRequestInterceptor(Func> interceptor) { Throw.IfNull(interceptor); - - var previous = s_outgoingRequestInterceptor.Value; - s_outgoingRequestInterceptor.Value = interceptor; - return new OutgoingRequestInterceptorScope(previous); - } - - private sealed class OutgoingRequestInterceptorScope(Func>? previous) : IDisposable - { - public void Dispose() => s_outgoingRequestInterceptor.Value = previous; + return new OutgoingRequestInterceptingMcpServer(this, interceptor); } /// @@ -119,7 +96,7 @@ public ValueTask SampleAsync( // redirect sampling through it. Capability checks (ThrowIfSamplingUnsupported) are // intentionally skipped because the interceptor's alternate channel is responsible for // delivering the request to the client. See SendRequestViaInterceptorAsync remarks. - if (CurrentOutgoingRequestInterceptor is { } interceptor) + if (OutgoingRequestInterceptor is { } interceptor) { return SendRequestViaInterceptorAsync(interceptor, RequestMethods.SamplingCreateMessage, requestParams, McpJsonUtilities.JsonContext.Default.CreateMessageRequestParams, @@ -326,7 +303,7 @@ public ValueTask RequestRootsAsync( // redirect through it. Capability checks (ThrowIfRootsUnsupported) are intentionally skipped // because the interceptor's alternate channel is responsible for delivering the request to // the client. See SendRequestViaInterceptorAsync remarks. - if (CurrentOutgoingRequestInterceptor is { } interceptor) + if (OutgoingRequestInterceptor is { } interceptor) { return SendRequestViaInterceptorAsync(interceptor, RequestMethods.RootsList, requestParams, McpJsonUtilities.JsonContext.Default.ListRootsRequestParams, @@ -372,7 +349,7 @@ public async ValueTask ElicitAsync( // redirect elicitation through it. Capability checks (ThrowIfElicitationUnsupported) are // intentionally skipped because the interceptor's alternate channel is responsible for // delivering the request to the client. See SendRequestViaInterceptorAsync remarks. - if (CurrentOutgoingRequestInterceptor is { } interceptor) + if (OutgoingRequestInterceptor is { } interceptor) { var paramsNode = JsonSerializer.SerializeToNode(requestParams, McpJsonUtilities.JsonContext.Default.ElicitRequestParams); var resultNode = await interceptor(RequestMethods.ElicitationCreate, paramsNode, cancellationToken).ConfigureAwait(false); diff --git a/src/ModelContextProtocol.Core/Server/McpServerImpl.cs b/src/ModelContextProtocol.Core/Server/McpServerImpl.cs index 7342523fa..650b3ea17 100644 --- a/src/ModelContextProtocol.Core/Server/McpServerImpl.cs +++ b/src/ModelContextProtocol.Core/Server/McpServerImpl.cs @@ -33,8 +33,6 @@ internal sealed partial class McpServerImpl : McpServer private readonly SemaphoreSlim _disposeLock = new(1, 1); private readonly ConcurrentDictionary _mrtrContinuations = new(); private readonly ConcurrentDictionary _mrtrContextsByRequestId = new(); - private readonly AsyncLocal _currentComposedCallToolInvocation = new(); - private static readonly string[] s_perRequestMetadataKeys = [ MetaKeys.ProtocolVersion, @@ -1379,6 +1377,7 @@ await originalListToolsHandler(request, cancellationToken).ConfigureAwait(false) var originalHandler = callToolWithAlternateHandler; callToolWithAlternateHandler = (request, cancellationToken) => { + MatchTool(request, tools); if (request.MatchedPrimitive is McpServerTool tool) { return InvokeToolWithAlternate(tool, request, cancellationToken); @@ -1388,7 +1387,10 @@ await originalListToolsHandler(request, cancellationToken).ConfigureAwait(false) }; } - callToolWithAlternateHandler = BuildFilterPipeline(callToolWithAlternateHandler, callToolWithAlternateFilters, BuildInitialAlternateToolFilter(tools)); + callToolWithAlternateHandler = BuildInvocationFilterPipeline( + callToolWithAlternateHandler, + callToolWithAlternateFilters, + BuildInitialAlternateToolFilter(tools)); } else { @@ -1440,49 +1442,17 @@ private static async ValueTask> InvokeToolWith private McpRequestHandler> BuildComposedCallToolHandler( McpRequestHandler callToolHandler, - IList>> callToolWithAlternateFilters, + IList>> callToolWithAlternateFilters, McpServerPrimitiveCollection? tools) { - McpRequestHandler> adaptedCallToolHandler = - async (request, cancellationToken) => - { - var invocation = _currentComposedCallToolInvocation.Value ?? - throw new InvalidOperationException("The composed tool-call invocation was not initialized."); - - try - { - var result = await callToolHandler(request, cancellationToken).ConfigureAwait(false); - LogToolCallLifecycles(request, invocation.RecordOrdinaryResult(result)); - return result; - } - catch (Exception exception) - { - LogToolCallLifecycles( - request, - invocation.RecordOrdinaryException( - exception, - cancellationToken.IsCancellationRequested)); - - if ((exception is OperationCanceledException && cancellationToken.IsCancellationRequested) || - exception is McpProtocolException || - exception is InputRequiredException) - { - throw; - } - - return CreateToolCallErrorResult(request, exception); - } - }; - - var composedHandler = BuildFilterPipeline(adaptedCallToolHandler, callToolWithAlternateFilters); - return async (request, cancellationToken) => { MatchTool(request, tools); var invocation = new ComposedCallToolInvocationState(); - var previousInvocation = _currentComposedCallToolInvocation.Value; - _currentComposedCallToolInvocation.Value = invocation; + var composedHandler = BuildInvocationFilterPipeline( + InvokeOrdinaryPipelineAsync, + callToolWithAlternateFilters); try { @@ -1505,16 +1475,42 @@ exception is McpProtocolException || return CreateToolCallErrorResult(request, e); } - finally + + async ValueTask> InvokeOrdinaryPipelineAsync( + RequestContext ordinaryRequest, + CancellationToken ordinaryCancellationToken) { - _currentComposedCallToolInvocation.Value = previousInvocation; + try + { + MatchTool(ordinaryRequest, tools); + var result = await callToolHandler(ordinaryRequest, ordinaryCancellationToken).ConfigureAwait(false); + LogToolCallLifecycles(ordinaryRequest, invocation.RecordOrdinaryResult(result)); + return result; + } + catch (Exception exception) + { + LogToolCallLifecycles( + ordinaryRequest, + invocation.RecordOrdinaryException( + exception, + ordinaryCancellationToken.IsCancellationRequested)); + + if ((exception is OperationCanceledException && ordinaryCancellationToken.IsCancellationRequested) || + exception is McpProtocolException || + exception is InputRequiredException) + { + throw; + } + + return CreateToolCallErrorResult(ordinaryRequest, exception); + } } }; } - private McpRequestFilter> BuildInitialAlternateToolFilter( - McpServerPrimitiveCollection? tools) => handler => - async (request, cancellationToken) => + private McpRequestInvocationFilter> BuildInitialAlternateToolFilter( + McpServerPrimitiveCollection? tools) => + async (request, handler, cancellationToken) => { MatchTool(request, tools); @@ -1590,89 +1586,6 @@ private void LogToolCallLifecycles( } } - private sealed class ComposedCallToolInvocationState - { - private readonly object _sync = new(); - private readonly List _pendingOrdinaryLifecycles = []; - private bool _outerCompleted; - private bool _outerReturnedAlternate; - - public IReadOnlyList RecordOrdinaryResult(CallToolResult result) => - RecordOrdinaryLifecycle(new(result, null, false)); - - public IReadOnlyList RecordOrdinaryException( - Exception exception, - bool cancellationRequested) => - RecordOrdinaryLifecycle(new(null, exception, cancellationRequested)); - - public IReadOnlyList CompleteOuter(ResultOrAlternate result) - { - lock (_sync) - { - if (result.IsAlternate) - { - _outerReturnedAlternate = true; - return DrainPendingLifecycles(); - } - - _outerCompleted = true; - var exceptions = _pendingOrdinaryLifecycles - .Where(lifecycle => lifecycle.Exception is not null) - .ToArray(); - _pendingOrdinaryLifecycles.Clear(); - return exceptions.Length > 0 ? - exceptions : - [new(result.Result!, null, false)]; - } - } - - public IReadOnlyList CompleteOuterException( - Exception exception, - bool cancellationRequested) - { - lock (_sync) - { - _outerCompleted = true; - _pendingOrdinaryLifecycles.Clear(); - return [new(null, exception, cancellationRequested)]; - } - } - - private IReadOnlyList RecordOrdinaryLifecycle(ToolCallLifecycle lifecycle) - { - lock (_sync) - { - if (_outerReturnedAlternate) - { - return [lifecycle]; - } - - if (!_outerCompleted) - { - _pendingOrdinaryLifecycles.Add(lifecycle); - } - - return []; - } - } - - private IReadOnlyList DrainPendingLifecycles() - { - if (_pendingOrdinaryLifecycles.Count == 0) - { - return []; - } - - var lifecycles = _pendingOrdinaryLifecycles.ToArray(); - _pendingOrdinaryLifecycles.Clear(); - return lifecycles; - } - } - - private sealed record ToolCallLifecycle( - CallToolResult? Result, - Exception? Exception, - bool CancellationRequested); #pragma warning restore MCPEXP002 private void ConfigureLogging(McpServerOptions options) @@ -1866,6 +1779,31 @@ private static McpRequestHandler BuildFilterPipeline BuildInvocationFilterPipeline( + McpRequestHandler baseHandler, + IList> filters, + McpRequestInvocationFilter? initialHandler = null) + { + var current = baseHandler; + + for (int i = filters.Count - 1; i >= 0; i--) + { + var next = current; + var filter = filters[i]; + current = (request, cancellationToken) => filter(request, next, cancellationToken); + } + + if (initialHandler is not null) + { + var next = current; + current = (request, cancellationToken) => initialHandler(request, next, cancellationToken); + } + + return current; + } +#pragma warning restore MCPEXP002 + private JsonRpcMessageFilter BuildMessageFilterPipeline(IList filters) { if (filters.Count == 0) diff --git a/src/ModelContextProtocol.Core/Server/OutgoingRequestInterceptingMcpServer.cs b/src/ModelContextProtocol.Core/Server/OutgoingRequestInterceptingMcpServer.cs new file mode 100644 index 000000000..93b7e5e73 --- /dev/null +++ b/src/ModelContextProtocol.Core/Server/OutgoingRequestInterceptingMcpServer.cs @@ -0,0 +1,56 @@ +using ModelContextProtocol.Protocol; +using System.Text.Json.Nodes; + +namespace ModelContextProtocol.Server; + +#pragma warning disable MCPEXP002 +internal sealed class OutgoingRequestInterceptingMcpServer( + McpServer server, + Func> interceptor) : McpServer +#pragma warning restore MCPEXP002 +{ + internal override Func>? OutgoingRequestInterceptor => interceptor; + + public override string? SessionId => server.SessionId; + + public override string? NegotiatedProtocolVersion => server.NegotiatedProtocolVersion; + + public override ClientCapabilities? ClientCapabilities => server.ClientCapabilities; + + public override Implementation? ClientInfo => server.ClientInfo; + + public override McpServerOptions ServerOptions => server.ServerOptions; + + public override IServiceProvider? Services => server.Services; + + [Obsolete(Obsoletions.DeprecatedLogging_Message, DiagnosticId = Obsoletions.Deprecated_DiagnosticId, UrlFormat = Obsoletions.Deprecated_Url)] + public override LoggingLevel? LoggingLevel => server.LoggingLevel; + + public override bool IsMrtrSupported => server.IsMrtrSupported; + + public override ValueTask DisposeAsync() => server.DisposeAsync(); + + public override IAsyncDisposable RegisterNotificationHandler( + string method, + Func handler) => + server.RegisterNotificationHandler(method, handler); + + public override Task RunAsync(CancellationToken cancellationToken = default) => + server.RunAsync(cancellationToken); + + public override Task SendMessageAsync(JsonRpcMessage message, CancellationToken cancellationToken = default) => + server.SendMessageAsync(message, cancellationToken); + + public override async Task SendRequestAsync( + JsonRpcRequest request, + CancellationToken cancellationToken = default) + { + Throw.IfNull(request); + + return new JsonRpcResponse + { + Id = request.Id, + Result = await interceptor(request.Method, request.Params, cancellationToken).ConfigureAwait(false), + }; + } +} diff --git a/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs b/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs index e0bd08009..207ffebd7 100644 --- a/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs +++ b/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs @@ -40,25 +40,25 @@ public static IMcpServerBuilder WithTasks(this IMcpServerBuilder builder, IMcpTa // Resolve ILoggerFactory from the provider (rather than requiring the caller to pass one) so the // background task body has somewhere to report failures. It is optional: if no logging is // registered, the options fall back to NullLoggerFactory. - builder.Services.AddSingleton>( - sp => new McpTasksPostConfigureOptions( + builder.Services.AddSingleton>( + sp => new McpTasksConfigureOptions( store, sp.GetRequiredService(), sp.GetService())); return builder; } - private sealed class McpTasksPostConfigureOptions( + private sealed class McpTasksConfigureOptions( IMcpTaskStore store, IServiceScopeFactory serviceScopeFactory, - ILoggerFactory? loggerFactory) : IPostConfigureOptions + ILoggerFactory? loggerFactory) : IConfigureOptions { private readonly IMcpTaskStore _store = store; private readonly IServiceScopeFactory _serviceScopeFactory = serviceScopeFactory; - private readonly ILogger _logger = (loggerFactory ?? NullLoggerFactory.Instance).CreateLogger(); + private readonly ILogger _logger = (loggerFactory ?? NullLoggerFactory.Instance).CreateLogger(); private readonly ConcurrentDictionary _cancellationSources = new(StringComparer.Ordinal); - public void PostConfigure(string? name, McpServerOptions options) + public void Configure(McpServerOptions options) { #if NET ArgumentNullException.ThrowIfNull(options); @@ -81,15 +81,17 @@ public void PostConfigure(string? name, McpServerOptions options) // Use a filter rather than a handler so it wraps around Core's tool dispatch. // This ensures it intercepts tool calls BEFORE the tool is invoked, allowing // it to spawn background execution and return the task alternate immediately. - options.Filters.Request.CallToolWithAlternateFilters.Add(next => async (request, cancellationToken) => - { - if (!IsJuly2026OrLaterProtocolRequest(request.JsonRpcRequest) || !HasTaskExtensionOptIn(request.Params?.Meta)) + options.Filters.Request.CallToolWithAlternateFilters.Insert( + options.Filters.Request.CallToolWithAlternateFilters.Count, + async (request, next, cancellationToken) => { - return await next(request, cancellationToken).ConfigureAwait(false); - } + if (!IsJuly2026OrLaterProtocolRequest(request.JsonRpcRequest) || !HasTaskExtensionOptIn(request.Params?.Meta)) + { + return await next(request, cancellationToken).ConfigureAwait(false); + } - return await RunAsTaskAsync(next, request, cancellationToken).ConfigureAwait(false); - }); + return await RunAsTaskAsync(next, request, cancellationToken).ConfigureAwait(false); + }); } private async ValueTask> RunAsTaskAsync( @@ -119,6 +121,7 @@ private async ValueTask> RunAsTaskAsync( } var taskId = taskInfo.TaskId; + executionRequest.Server = request.Server.WithMcpTaskOutgoingRequestInterceptor(taskId, _store); var cts = new CancellationTokenSource(); _cancellationSources[taskId] = cts; @@ -145,10 +148,7 @@ private async Task ExecuteTaskAsync( { try { - using (McpTasksServerExtensions.CreateMcpTaskScope(request.Server, taskId, _store)) - { - await ExecuteToolPipelineAsync(next, request, taskId, taskCancellationToken).ConfigureAwait(false); - } + await ExecuteToolPipelineAsync(next, request, taskId, taskCancellationToken).ConfigureAwait(false); } finally { diff --git a/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksServerExtensions.cs b/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksServerExtensions.cs index 31445fd1f..74b227f5d 100644 --- a/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksServerExtensions.cs +++ b/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksServerExtensions.cs @@ -37,14 +37,10 @@ public static Task SendTaskStatusNotificationAsync( cancellationToken); } - /// - /// Creates a scope that routes server-initiated requests through the specified task store. - /// - /// The server whose outgoing requests should be redirected. - /// The related task identifier. - /// The task store used to surface input requests. - /// An that restores the previous outgoing-request behavior. - public static IDisposable CreateMcpTaskScope(this McpServer server, string taskId, IMcpTaskStore store) + internal static McpServer WithMcpTaskOutgoingRequestInterceptor( + this McpServer server, + string taskId, + IMcpTaskStore store) { #if NET ArgumentNullException.ThrowIfNull(server); @@ -56,7 +52,7 @@ public static IDisposable CreateMcpTaskScope(this McpServer server, string taskI if (store is null) throw new ArgumentNullException(nameof(store)); #endif - return server.InterceptOutgoingRequests(async (method, paramsNode, cancellationToken) => + return server.WithOutgoingRequestInterceptor(async (method, paramsNode, cancellationToken) => { var requestId = Guid.NewGuid().ToString("N"); diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/HttpTaskIntegrationTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/HttpTaskIntegrationTests.cs index 14f90152e..98ad7f88b 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/HttpTaskIntegrationTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/HttpTaskIntegrationTests.cs @@ -6,7 +6,9 @@ using ModelContextProtocol.Extensions.Tasks; using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; +using ModelContextProtocol.Tests.Utils; using Moq; +using System.Security.Claims; namespace ModelContextProtocol.AspNetCore.Tests; @@ -42,15 +44,119 @@ public async Task WithTasks_CanCallToolOverHttp() } [Fact] - public async Task WithTasks_UnauthorizedTool_DoesNotCreateTask() + public async Task WithTasks_AfterOrdinaryFilter_CanCallToolOverHttp() { - var taskStore = new Mock(MockBehavior.Strict); + var ordinaryFilterInvoked = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); Builder.Services - .AddMcpServer() + .AddMcpServer(options => + { + options.Filters.Request.CallToolFilters.Add(next => async (request, cancellationToken) => + { + ordinaryFilterInvoked.TrySetResult(); + return await next(request, cancellationToken); + }); + }) .WithHttpTransport() - .AddAuthorizationFilters() - .WithTasks(taskStore.Object) + .WithTasks(new InMemoryMcpTaskStore { DefaultPollIntervalMs = 10 }) .WithTools(); + + await using var app = Builder.Build(); + app.MapMcp(); + await app.StartAsync(TestContext.Current.CancellationToken); + + await using var transport = new HttpClientTransport( + new HttpClientTransportOptions { Endpoint = new("http://localhost:5000") }, + HttpClient, + LoggerFactory); + await using var client = await McpClient.CreateAsync( + transport, + loggerFactory: LoggerFactory, + cancellationToken: TestContext.Current.CancellationToken); + + var result = await client.CallToolWithPollingAsync( + new CallToolRequestParams { Name = "test" }, + cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal("Hello World!", Assert.IsType(Assert.Single(result.Content)).Text); + await ordinaryFilterInvoked.Task.WaitAsync(TestConstants.DefaultTimeout, TestContext.Current.CancellationToken); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task WithTasks_AuthorizedTool_Completes(bool registerTasksBeforeAuthorization) + { + var serverBuilder = Builder.Services + .AddMcpServer() + .WithHttpTransport(); + + if (registerTasksBeforeAuthorization) + { + serverBuilder + .WithTasks(new InMemoryMcpTaskStore { DefaultPollIntervalMs = 10 }) + .AddAuthorizationFilters(); + } + else + { + serverBuilder + .AddAuthorizationFilters() + .WithTasks(new InMemoryMcpTaskStore { DefaultPollIntervalMs = 10 }); + } + + serverBuilder.WithTools(); + Builder.Services.AddAuthorization(); + + await using var app = Builder.Build(); + app.Use(next => async context => + { + context.User = new ClaimsPrincipal(new ClaimsIdentity( + [new Claim(ClaimTypes.NameIdentifier, "test-user")], + "TestAuthType")); + await next(context); + }); + app.MapMcp(); + await app.StartAsync(TestContext.Current.CancellationToken); + + await using var transport = new HttpClientTransport( + new HttpClientTransportOptions { Endpoint = new("http://localhost:5000") }, + HttpClient, + LoggerFactory); + await using var client = await McpClient.CreateAsync( + transport, + loggerFactory: LoggerFactory, + cancellationToken: TestContext.Current.CancellationToken); + + var result = await client.CallToolWithPollingAsync( + new CallToolRequestParams { Name = "authorized-test" }, + cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal("Authorized", Assert.IsType(Assert.Single(result.Content)).Text); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task WithTasks_UnauthorizedTool_DoesNotCreateTask(bool registerTasksBeforeAuthorization) + { + var taskStore = new Mock(MockBehavior.Strict); + var serverBuilder = Builder.Services + .AddMcpServer() + .WithHttpTransport(); + + if (registerTasksBeforeAuthorization) + { + serverBuilder + .WithTasks(taskStore.Object) + .AddAuthorizationFilters(); + } + else + { + serverBuilder + .AddAuthorizationFilters() + .WithTasks(taskStore.Object); + } + + serverBuilder.WithTools(); Builder.Services.AddAuthorization(); await using var app = Builder.Build(); @@ -85,6 +191,6 @@ private sealed class TestTools [McpServerTool(Name = "authorized-test")] [Authorize] - public static string AuthorizedTest() => "unreachable"; + public static string AuthorizedTest() => "Authorized"; } } diff --git a/tests/ModelContextProtocol.Tests/Server/CallToolFilterMixingTests.cs b/tests/ModelContextProtocol.Tests/Server/CallToolFilterMixingTests.cs index aa7a3750e..7754d5f9c 100644 --- a/tests/ModelContextProtocol.Tests/Server/CallToolFilterMixingTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/CallToolFilterMixingTests.cs @@ -14,22 +14,36 @@ public class CallToolFilterMixingTests(ITestOutputHelper testOutputHelper) : Log private static McpRequestFilter PassThroughCallToolFilter => next => next; - private static McpRequestFilter> PassThroughAlternateFilter => - next => next; + private static McpRequestInvocationFilter> PassThroughAlternateFilter => + static (context, next, cancellationToken) => next(context, cancellationToken); [Fact] public async Task MixingCallToolFilters_WithAlternateFilters_Succeeds() { await using var transport = new StreamServerTransport(Stream.Null, Stream.Null); var options = new McpServerOptions { Capabilities = new() { Tools = new() } }; - options.Filters.Request.CallToolFilters.Add(PassThroughCallToolFilter); options.Filters.Request.CallToolWithAlternateFilters.Add(PassThroughAlternateFilter); + options.Filters.Request.CallToolFilters.Add(PassThroughCallToolFilter); await using var server = McpServer.Create(transport, options, LoggerFactory); Assert.NotNull(server); } + [Fact] + public void AlternateFilters_AfterOrdinaryFilters_ThrowsActionableError() + { + var options = new McpServerOptions { Capabilities = new() { Tools = new() } }; + options.Filters.Request.CallToolFilters.Add(PassThroughCallToolFilter); + + var ex = Assert.Throws( + () => options.Filters.Request.CallToolWithAlternateFilters.Add(PassThroughAlternateFilter)); + + Assert.Contains(nameof(McpRequestFilters.CallToolWithAlternateFilters), ex.Message); + Assert.Contains(nameof(McpRequestFilters.CallToolFilters), ex.Message); + Assert.Contains("must be registered before", ex.Message); + } + [Fact] public async Task CallToolFilters_WithExplicitAlternateHandler_ThrowsActionableError() { diff --git a/tests/ModelContextProtocol.Tests/Server/TaskCallToolFilterCompositionTests.cs b/tests/ModelContextProtocol.Tests/Server/TaskCallToolFilterCompositionTests.cs index fd35cef0a..47d532d56 100644 --- a/tests/ModelContextProtocol.Tests/Server/TaskCallToolFilterCompositionTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/TaskCallToolFilterCompositionTests.cs @@ -11,7 +11,7 @@ public class TaskCallToolFilterCompositionTests(ITestOutputHelper testOutputHelp private readonly TaskCompletionSource _continueBackgroundExecution = new(TaskCreationOptions.RunContinuationsAsynchronously); private readonly TaskCompletionSource _executionCompleted = new(TaskCreationOptions.RunContinuationsAsynchronously); private readonly TaskCompletionSource _executionScopeDisposed = new(TaskCreationOptions.RunContinuationsAsynchronously); - private int _alternateFilterFactoryInvocationCount; + private int _alternateFilterInvocationCount; private int _filterInvocationCount; private string? _matchedPrimitiveId; private string? _alternateMatchedPrimitiveId; @@ -25,12 +25,109 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer { services.AddScoped(_ => new ScopedDependency(_executionScopeDisposed)); + mcpServerBuilder.Services.Configure(options => + { +#pragma warning disable MCPEXP002 // exercises an alternate filter registered before Tasks + options.Filters.Request.CallToolWithAlternateFilters.Add(async (request, next, cancellationToken) => + { + if (request.Params?.Name == "task-filter-tool") + { + _alternateMatchedPrimitiveId = request.MatchedPrimitive?.Id; + _alternateRequestContext = request; + _alternateServicesBeforeNext = request.Services; + var result = await next(request, cancellationToken); + _alternateServicesAfterNext = request.Services; + return result; + } + + return await next(request, cancellationToken); + }); +#pragma warning restore MCPEXP002 + }); + mcpServerBuilder .WithTools() .WithTasks(new InMemoryMcpTaskStore { DefaultPollIntervalMs = 10 }); mcpServerBuilder.Services.Configure(options => { +#pragma warning disable MCPEXP002 // exercises the experimental CallToolWithAlternateFilters seam + options.Filters.Request.CallToolWithAlternateFilters.Add(async (request, next, cancellationToken) => + { + Interlocked.Increment(ref _alternateFilterInvocationCount); + + if (request.Params?.Name is "suppress-flow-direct-tool" or "suppress-flow-task-tool") + { + Task> continuation; + using (ExecutionContext.SuppressFlow()) + { + continuation = Task.Run( + async () => await next(request, cancellationToken).ConfigureAwait(false), + cancellationToken); + } + + return await continuation.ConfigureAwait(false); + } + + if (request.Params?.Name == "alternate-filter-exception-tool") + { + _throwingAlternateMatchedPrimitiveId = request.MatchedPrimitive?.Id; + throw new InvalidOperationException("Alternate filter failure."); + } + + if (request.Params?.Name == "alternate-short-circuit-tool") + { + return new CallToolResult + { + Content = [new TextContentBlock { Text = "short-circuited" }], + }; + } + + if (request.Params?.Name == "replace-jsonrpc-request-tool") + { + request.JsonRpcRequest = new JsonRpcRequest + { + Id = request.JsonRpcRequest.Id, + Method = request.JsonRpcRequest.Method, + }; + } + + if (request.Params?.Name == "replace-request-context-tool") + { + var replacement = new RequestContext( + request.Server, + request.JsonRpcRequest, + request.Params) + { + Services = request.Services, + }; + return await next(replacement, cancellationToken); + } + + if (request.Params?.Name == "alternate-transform-result-tool") + { + _ = await next(request, cancellationToken); + return new CallToolResult + { + IsError = true, + Content = [new TextContentBlock { Text = "transformed" }], + }; + } + + if (request.Params?.Name == "task-filter-tool") + { + return await next(request, cancellationToken); + } + + _alternateMatchedPrimitiveId = request.MatchedPrimitive?.Id; + _alternateRequestContext = request; + _alternateServicesBeforeNext = request.Services; + var result = await next(request, cancellationToken); + _alternateServicesAfterNext = request.Services; + return result; + }); +#pragma warning restore MCPEXP002 + options.Filters.Request.CallToolFilters.Add(next => async (request, cancellationToken) => { if (request.Params?.Name != "task-filter-tool") @@ -56,55 +153,6 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer throw; } }); - -#pragma warning disable MCPEXP002 // exercises the experimental CallToolWithAlternateFilters seam - options.Filters.Request.CallToolWithAlternateFilters.Add(next => - { - Interlocked.Increment(ref _alternateFilterFactoryInvocationCount); - return async (request, cancellationToken) => - { - if (request.Params?.Name == "alternate-filter-exception-tool") - { - _throwingAlternateMatchedPrimitiveId = request.MatchedPrimitive?.Id; - throw new InvalidOperationException("Alternate filter failure."); - } - - if (request.Params?.Name == "alternate-short-circuit-tool") - { - return new CallToolResult - { - Content = [new TextContentBlock { Text = "short-circuited" }], - }; - } - - if (request.Params?.Name == "replace-jsonrpc-request-tool") - { - request.JsonRpcRequest = new JsonRpcRequest - { - Id = request.JsonRpcRequest.Id, - Method = request.JsonRpcRequest.Method, - }; - } - - if (request.Params?.Name == "alternate-transform-result-tool") - { - _ = await next(request, cancellationToken); - return new CallToolResult - { - IsError = true, - Content = [new TextContentBlock { Text = "transformed" }], - }; - } - - _alternateMatchedPrimitiveId = request.MatchedPrimitive?.Id; - _alternateRequestContext = request; - _alternateServicesBeforeNext = request.Services; - var result = await next(request, cancellationToken); - _alternateServicesAfterNext = request.Services; - return result; - }; - }); -#pragma warning restore MCPEXP002 }); } @@ -166,7 +214,7 @@ public async Task AlternateFilterShortCircuit_LogsCompletionOnce() } [Fact] - public async Task AlternateFilterFactory_IsCreatedOnce() + public async Task AlternateInvocationFilter_RunsForEachRequest() { await using var client = await CreateMcpClientForServer(); @@ -177,7 +225,31 @@ await client.CallToolAsync( "alternate-short-circuit-tool", cancellationToken: TestContext.Current.CancellationToken); - Assert.Equal(1, _alternateFilterFactoryInvocationCount); + Assert.Equal(2, _alternateFilterInvocationCount); + } + + [Fact] + public async Task AlternateInvocationFilter_CanSuppressExecutionContextForDirectCall() + { + await using var client = await CreateMcpClientForServer(); + + var result = await client.CallToolAsync( + "suppress-flow-direct-tool", + cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal("direct succeeded", Assert.IsType(Assert.Single(result.Content)).Text); + } + + [Fact] + public async Task AlternateInvocationFilter_CanSuppressExecutionContextForTaskBackedCall() + { + await using var client = await CreateMcpClientForServer(); + + var result = await client.CallToolWithPollingAsync( + new CallToolRequestParams { Name = "suppress-flow-task-tool" }, + cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal("task succeeded", Assert.IsType(Assert.Single(result.Content)).Text); } [Fact] @@ -192,6 +264,18 @@ public async Task AlternateFilter_CanReplaceJsonRpcRequest() Assert.Equal("replacement succeeded", Assert.IsType(Assert.Single(result.Content)).Text); } + [Fact] + public async Task AlternateFilter_CanReplaceRequestContext() + { + await using var client = await CreateMcpClientForServer(); + + var result = await client.CallToolAsync( + "replace-request-context-tool", + cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal("context replacement succeeded", Assert.IsType(Assert.Single(result.Content)).Text); + } + [Fact] public async Task AlternateFilterTransformedResult_LogsFinalResult() { @@ -233,7 +317,16 @@ private sealed class TaskFilterTools [McpServerTool(Name = "replace-jsonrpc-request-tool")] public static string ReplaceJsonRpcRequestTarget() => "replacement succeeded"; + [McpServerTool(Name = "replace-request-context-tool")] + public static string ReplaceRequestContextTarget() => "context replacement succeeded"; + [McpServerTool(Name = "alternate-transform-result-tool")] public static string AlternateTransformResultTarget() => "original"; + + [McpServerTool(Name = "suppress-flow-direct-tool")] + public static string SuppressFlowDirect() => "direct succeeded"; + + [McpServerTool(Name = "suppress-flow-task-tool")] + public static string SuppressFlowTask() => "task succeeded"; } } From 646140d7596a939886e8aad858d10c2f68a60163 Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Thu, 23 Jul 2026 20:16:17 -0700 Subject: [PATCH 7/8] Scope filter ordering validation to Tasks Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a93a04c7-4270-428a-b598-a2cc76995bf4 --- docs/concepts/filters.md | 5 +-- .../Server/McpRequestFilterCollection.cs | 40 ------------------- .../Server/McpRequestFilters.cs | 29 +------------- .../Server/McpTasksBuilderExtensions.cs | 8 ++++ .../HttpTaskIntegrationTests.cs | 32 ++++----------- .../Server/CallToolFilterMixingTests.cs | 13 +++--- 6 files changed, 25 insertions(+), 102 deletions(-) delete mode 100644 src/ModelContextProtocol.Core/Server/McpRequestFilterCollection.cs diff --git a/docs/concepts/filters.md b/docs/concepts/filters.md index 0ab6fce4c..2711e0cd5 100644 --- a/docs/concepts/filters.md +++ b/docs/concepts/filters.md @@ -43,9 +43,8 @@ For task-backed calls, ordinary filters execute in the background after task cre `CallToolWithAlternateHandler` remains a full replacement and cannot be combined with ordinary call-tool filters. -Register alternate-result filters before ordinary call-tool filters. Adding an alternate-result -filter after an ordinary filter throws `InvalidOperationException` because execution order cannot -match registration order in that arrangement. +Configure `WithTasks` before adding ordinary call-tool filters. Tasks validates this ordering when +its filter is installed because it must execute outside the ordinary call-tool pipeline. ## Message Filters diff --git a/src/ModelContextProtocol.Core/Server/McpRequestFilterCollection.cs b/src/ModelContextProtocol.Core/Server/McpRequestFilterCollection.cs deleted file mode 100644 index 13fa716c4..000000000 --- a/src/ModelContextProtocol.Core/Server/McpRequestFilterCollection.cs +++ /dev/null @@ -1,40 +0,0 @@ -using System.Collections; - -namespace ModelContextProtocol.Server; - -internal sealed class McpRequestFilterCollection(IList filters, Action validateInsertion) : IList -{ - public T this[int index] - { - get => filters[index]; - set => filters[index] = value; - } - - public int Count => filters.Count; - - public bool IsReadOnly => filters.IsReadOnly; - - public void Add(T item) - { - validateInsertion(); - filters.Add(item); - } - - public void Clear() => filters.Clear(); - - public bool Contains(T item) => filters.Contains(item); - - public void CopyTo(T[] array, int arrayIndex) => filters.CopyTo(array, arrayIndex); - - public IEnumerator GetEnumerator() => filters.GetEnumerator(); - - public int IndexOf(T item) => filters.IndexOf(item); - - public void Insert(int index, T item) => filters.Insert(index, item); - - public bool Remove(T item) => filters.Remove(item); - - public void RemoveAt(int index) => filters.RemoveAt(index); - - IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); -} diff --git a/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs b/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs index 4afb04aa2..df34d48a7 100644 --- a/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs +++ b/src/ModelContextProtocol.Core/Server/McpRequestFilters.cs @@ -10,7 +10,6 @@ public sealed class McpRequestFilters { #pragma warning disable MCPEXP002 // CallToolWithAlternateFilters references the experimental ResultOrAlternate seam private IList>? _callToolFilters; - private IList>>? _callToolWithAlternateFilters; /// /// Gets or sets the filters for the list-tools handler pipeline. @@ -84,39 +83,15 @@ public IList> CallToolFi /// Alternate-result filters run in registration order. If one filter dispatches the remainder of the pipeline /// asynchronously, filters registered after it execute as part of that asynchronous operation. /// - /// - /// Alternate-result filters must be registered before ordinary . - /// Registering an alternate-result filter after an ordinary filter throws an - /// because the two pipelines cannot preserve that registration order. - /// /// [Experimental(Experimentals.Subclassing_DiagnosticId, UrlFormat = Experimentals.Subclassing_Url)] public IList>> CallToolWithAlternateFilters { - get => _callToolWithAlternateFilters ??= new McpRequestFilterCollection>>( - [], - ValidateAlternateFilterRegistration); + get => field ??= []; set { Throw.IfNull(value); - if (value.Count > 0) - { - ValidateAlternateFilterRegistration(); - } - - _callToolWithAlternateFilters = - new McpRequestFilterCollection>>( - value, - ValidateAlternateFilterRegistration); - } - } - private void ValidateAlternateFilterRegistration() - { - if (CallToolFilters.Count > 0) - { - throw new InvalidOperationException( - $"{nameof(CallToolWithAlternateFilters)} must be registered before {nameof(CallToolFilters)} because " + - "alternate-result filters always execute outside ordinary call-tool filters."); + field = value; } } #pragma warning restore MCPEXP002 diff --git a/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs b/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs index 207ffebd7..9c46b0a2c 100644 --- a/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs +++ b/src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs @@ -23,6 +23,7 @@ public static class McpTasksBuilderExtensions /// Tasks are implemented as an alternate-result call-tool filter. Alternate-result filters registered before /// the Tasks filter run before task creation. Filters registered after it, along with all ordinary call-tool /// filters, run in the background before the tool. + /// Register Tasks before configuring ordinary call-tool filters. /// /// The server builder. /// The task store. @@ -78,6 +79,13 @@ public void Configure(McpServerOptions options) options.RequestHandlers.Add(new McpServerRequestHandler { Method = TasksProtocol.MethodTasksUpdate, Handler = HandleUpdateTask }); options.RequestHandlers.Add(new McpServerRequestHandler { Method = TasksProtocol.MethodTasksCancel, Handler = HandleCancelTask }); + if (options.Filters.Request.CallToolFilters.Count > 0) + { + throw new InvalidOperationException( + $"{nameof(WithTasks)} must be configured before ordinary call-tool filters because " + + "the Tasks filter must execute outside the ordinary call-tool pipeline."); + } + // Use a filter rather than a handler so it wraps around Core's tool dispatch. // This ensures it intercepts tool calls BEFORE the tool is invoked, allowing // it to spawn background execution and return the task alternate immediately. diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/HttpTaskIntegrationTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/HttpTaskIntegrationTests.cs index 98ad7f88b..e7b2a5578 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/HttpTaskIntegrationTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/HttpTaskIntegrationTests.cs @@ -1,12 +1,12 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Authorization; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using ModelContextProtocol.AspNetCore.Tests.Utils; using ModelContextProtocol.Client; using ModelContextProtocol.Extensions.Tasks; using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; -using ModelContextProtocol.Tests.Utils; using Moq; using System.Security.Claims; @@ -44,41 +44,23 @@ public async Task WithTasks_CanCallToolOverHttp() } [Fact] - public async Task WithTasks_AfterOrdinaryFilter_CanCallToolOverHttp() + public async Task WithTasks_AfterOrdinaryFilter_ThrowsActionableError() { - var ordinaryFilterInvoked = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); Builder.Services .AddMcpServer(options => { - options.Filters.Request.CallToolFilters.Add(next => async (request, cancellationToken) => - { - ordinaryFilterInvoked.TrySetResult(); - return await next(request, cancellationToken); - }); + options.Filters.Request.CallToolFilters.Add(next => next); }) .WithHttpTransport() .WithTasks(new InMemoryMcpTaskStore { DefaultPollIntervalMs = 10 }) .WithTools(); await using var app = Builder.Build(); - app.MapMcp(); - await app.StartAsync(TestContext.Current.CancellationToken); - - await using var transport = new HttpClientTransport( - new HttpClientTransportOptions { Endpoint = new("http://localhost:5000") }, - HttpClient, - LoggerFactory); - await using var client = await McpClient.CreateAsync( - transport, - loggerFactory: LoggerFactory, - cancellationToken: TestContext.Current.CancellationToken); - - var result = await client.CallToolWithPollingAsync( - new CallToolRequestParams { Name = "test" }, - cancellationToken: TestContext.Current.CancellationToken); - Assert.Equal("Hello World!", Assert.IsType(Assert.Single(result.Content)).Text); - await ordinaryFilterInvoked.Task.WaitAsync(TestConstants.DefaultTimeout, TestContext.Current.CancellationToken); + var exception = Assert.Throws( + () => app.Services.GetRequiredService>().Value); + Assert.Contains(nameof(McpTasksBuilderExtensions.WithTasks), exception.Message); + Assert.Contains("before ordinary call-tool filters", exception.Message); } [Theory] diff --git a/tests/ModelContextProtocol.Tests/Server/CallToolFilterMixingTests.cs b/tests/ModelContextProtocol.Tests/Server/CallToolFilterMixingTests.cs index 7754d5f9c..e891c8ce7 100644 --- a/tests/ModelContextProtocol.Tests/Server/CallToolFilterMixingTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/CallToolFilterMixingTests.cs @@ -5,7 +5,7 @@ namespace ModelContextProtocol.Tests.Server; /// -/// Verifies composition and validation for the non-alternate +/// Verifies composition for the non-alternate /// and alternate pipelines. /// public class CallToolFilterMixingTests(ITestOutputHelper testOutputHelper) : LoggedTest(testOutputHelper) @@ -31,17 +31,16 @@ public async Task MixingCallToolFilters_WithAlternateFilters_Succeeds() } [Fact] - public void AlternateFilters_AfterOrdinaryFilters_ThrowsActionableError() + public async Task AlternateFilters_AddedAfterOrdinaryFilters_Succeeds() { + await using var transport = new StreamServerTransport(Stream.Null, Stream.Null); var options = new McpServerOptions { Capabilities = new() { Tools = new() } }; options.Filters.Request.CallToolFilters.Add(PassThroughCallToolFilter); + options.Filters.Request.CallToolWithAlternateFilters.Add(PassThroughAlternateFilter); - var ex = Assert.Throws( - () => options.Filters.Request.CallToolWithAlternateFilters.Add(PassThroughAlternateFilter)); + await using var server = McpServer.Create(transport, options, LoggerFactory); - Assert.Contains(nameof(McpRequestFilters.CallToolWithAlternateFilters), ex.Message); - Assert.Contains(nameof(McpRequestFilters.CallToolFilters), ex.Message); - Assert.Contains("must be registered before", ex.Message); + Assert.NotNull(server); } [Fact] From 7d1341f9e41afa55ba2491a9ada3233a11146cba Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Fri, 24 Jul 2026 11:05:44 -0700 Subject: [PATCH 8/8] Move tool call lifecycle to separate file Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a93a04c7-4270-428a-b598-a2cc76995bf4 --- .../Server/ComposedCallToolInvocationState.cs | 5 ----- src/ModelContextProtocol.Core/Server/ToolCallLifecycle.cs | 8 ++++++++ 2 files changed, 8 insertions(+), 5 deletions(-) create mode 100644 src/ModelContextProtocol.Core/Server/ToolCallLifecycle.cs diff --git a/src/ModelContextProtocol.Core/Server/ComposedCallToolInvocationState.cs b/src/ModelContextProtocol.Core/Server/ComposedCallToolInvocationState.cs index fd194ca32..c5f9316b1 100644 --- a/src/ModelContextProtocol.Core/Server/ComposedCallToolInvocationState.cs +++ b/src/ModelContextProtocol.Core/Server/ComposedCallToolInvocationState.cs @@ -81,9 +81,4 @@ private IReadOnlyList DrainPendingLifecycles() return lifecycles; } } - -internal sealed record ToolCallLifecycle( - CallToolResult? Result, - Exception? Exception, - bool CancellationRequested); #pragma warning restore MCPEXP002 diff --git a/src/ModelContextProtocol.Core/Server/ToolCallLifecycle.cs b/src/ModelContextProtocol.Core/Server/ToolCallLifecycle.cs new file mode 100644 index 000000000..5cb6d4754 --- /dev/null +++ b/src/ModelContextProtocol.Core/Server/ToolCallLifecycle.cs @@ -0,0 +1,8 @@ +using ModelContextProtocol.Protocol; + +namespace ModelContextProtocol.Server; + +internal sealed record ToolCallLifecycle( + CallToolResult? Result, + Exception? Exception, + bool CancellationRequested);