From 14746ef79c685ede070e45e2073ad157d461b4a1 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Tue, 23 Jun 2026 15:28:52 -0700 Subject: [PATCH] Document caching behavior and warn on non-conformant draft cacheable results Follow-up to the SEP-2549 caching hints work in PR #1623, addressing two of its tracked follow-up issues: Fixes #1645: Tighten the doc comments on the auto-paginating list overloads (tools/prompts/resources/resource-templates) to state plainly that the SDK performs no internal caching of listing results, and that any caching of ttlMs/cacheScope must be done by the caller via the lower-level non-auto-paginating overloads, which surface those fields per page. Fixes #1650: Log a warning (never throw) when a server that negotiated the draft protocol version returns a cacheable result (tools/list, prompts/list, resources/list, resources/templates/list, resources/read) without the now required ttlMs/cacheScope fields. The warning is emitted at most once per method per session so paginated listings do not produce one warning per page. The raw cacheable overloads keep their synchronous argument validation and route results through a shared ValidateCacheableResultAsync helper that calls the new ValidateCacheableResult seam on McpClient. The seam is a no-op by default and overridden by McpClientImpl to perform the conformance check. Issue #1646 (optional client-side caching subsystem) was reviewed and closed as not planned; no caching subsystem is added here. --- .../Client/McpClient.Methods.cs | 65 +++- .../Client/McpClient.cs | 14 + .../Client/McpClientImpl.cs | 29 ++ .../Protocol/CacheableResultWarningTests.cs | 317 ++++++++++++++++++ 4 files changed, 415 insertions(+), 10 deletions(-) create mode 100644 tests/ModelContextProtocol.Tests/Protocol/CacheableResultWarningTests.cs diff --git a/src/ModelContextProtocol.Core/Client/McpClient.Methods.cs b/src/ModelContextProtocol.Core/Client/McpClient.Methods.cs index 22245a924..ab0e5ee89 100644 --- a/src/ModelContextProtocol.Core/Client/McpClient.Methods.cs +++ b/src/ModelContextProtocol.Core/Client/McpClient.Methods.cs @@ -173,10 +173,18 @@ public ValueTask PingAsync( /// A list of all available tools as instances. /// The request failed or the server returned an error response. /// + /// /// This overload aggregates every page into a single list and does not surface the per-result caching hints /// ( and ). To read those hints, /// use the overload, which returns the /// raw for each page. + /// + /// + /// The SDK does not perform any internal caching of listing results; every call re-fetches all pages from the server. + /// If you want to cache listing results, do so in your own code using the lower-level + /// overload, which exposes the per-page + /// caching hints and lets you manage pagination so each page can be cached and expired independently. + /// /// public async ValueTask> ListToolsAsync( RequestOptions? options = null, @@ -247,12 +255,25 @@ public ValueTask ListToolsAsync( { Throw.IfNull(requestParams); - return SendRequestAsync( + return ValidateCacheableResultAsync(RequestMethods.ToolsList, SendRequestAsync( RequestMethods.ToolsList, requestParams, McpJsonUtilities.JsonContext.Default.ListToolsRequestParams, McpJsonUtilities.JsonContext.Default.ListToolsResult, - cancellationToken: cancellationToken); + cancellationToken: cancellationToken)); + } + + /// + /// Awaits a cacheable result and gives derived clients a chance to emit diagnostics (for example, a + /// SEP-2549 conformance warning) before returning it. Preserves the synchronous argument validation + /// performed by the callers before the request is issued. + /// + private async ValueTask ValidateCacheableResultAsync(string method, ValueTask resultTask) + where TResult : ICacheableResult + { + var result = await resultTask.ConfigureAwait(false); + ValidateCacheableResult(method, result); + return result; } /// @@ -263,10 +284,18 @@ public ValueTask ListToolsAsync( /// A list of all available prompts as instances. /// The request failed or the server returned an error response. /// + /// /// This overload aggregates every page into a single list and does not surface the per-result caching hints /// ( and ). To read those hints, /// use the overload, which returns the /// raw for each page. + /// + /// + /// The SDK does not perform any internal caching of listing results; every call re-fetches all pages from the server. + /// If you want to cache listing results, do so in your own code using the lower-level + /// overload, which exposes the per-page + /// caching hints and lets you manage pagination so each page can be cached and expired independently. + /// /// public async ValueTask> ListPromptsAsync( RequestOptions? options = null, @@ -309,12 +338,12 @@ public ValueTask ListPromptsAsync( { Throw.IfNull(requestParams); - return SendRequestAsync( + return ValidateCacheableResultAsync(RequestMethods.PromptsList, SendRequestAsync( RequestMethods.PromptsList, requestParams, McpJsonUtilities.JsonContext.Default.ListPromptsRequestParams, McpJsonUtilities.JsonContext.Default.ListPromptsResult, - cancellationToken: cancellationToken); + cancellationToken: cancellationToken)); } /// @@ -379,10 +408,18 @@ public ValueTask GetPromptAsync( /// A list of all available resource templates as instances. /// The request failed or the server returned an error response. /// + /// /// This overload aggregates every page into a single list and does not surface the per-result caching hints /// ( and ). To read those hints, /// use the overload, which returns the /// raw for each page. + /// + /// + /// The SDK does not perform any internal caching of listing results; every call re-fetches all pages from the server. + /// If you want to cache listing results, do so in your own code using the lower-level + /// overload, which exposes the per-page + /// caching hints and lets you manage pagination so each page can be cached and expired independently. + /// /// public async ValueTask> ListResourceTemplatesAsync( RequestOptions? options = null, @@ -425,12 +462,12 @@ public ValueTask ListResourceTemplatesAsync( { Throw.IfNull(requestParams); - return SendRequestAsync( + return ValidateCacheableResultAsync(RequestMethods.ResourcesTemplatesList, SendRequestAsync( RequestMethods.ResourcesTemplatesList, requestParams, McpJsonUtilities.JsonContext.Default.ListResourceTemplatesRequestParams, McpJsonUtilities.JsonContext.Default.ListResourceTemplatesResult, - cancellationToken: cancellationToken); + cancellationToken: cancellationToken)); } /// @@ -441,10 +478,18 @@ public ValueTask ListResourceTemplatesAsync( /// A list of all available resources as instances. /// The request failed or the server returned an error response. /// + /// /// This overload aggregates every page into a single list and does not surface the per-result caching hints /// ( and ). To read those hints, /// use the overload, which returns the /// raw for each page. + /// + /// + /// The SDK does not perform any internal caching of listing results; every call re-fetches all pages from the server. + /// If you want to cache listing results, do so in your own code using the lower-level + /// overload, which exposes the per-page + /// caching hints and lets you manage pagination so each page can be cached and expired independently. + /// /// public async ValueTask> ListResourcesAsync( RequestOptions? options = null, @@ -487,12 +532,12 @@ public ValueTask ListResourcesAsync( { Throw.IfNull(requestParams); - return SendRequestAsync( + return ValidateCacheableResultAsync(RequestMethods.ResourcesList, SendRequestAsync( RequestMethods.ResourcesList, requestParams, McpJsonUtilities.JsonContext.Default.ListResourcesRequestParams, McpJsonUtilities.JsonContext.Default.ListResourcesResult, - cancellationToken: cancellationToken); + cancellationToken: cancellationToken)); } /// @@ -571,12 +616,12 @@ public ValueTask ReadResourceAsync( { Throw.IfNull(requestParams); - return SendRequestAsync( + return ValidateCacheableResultAsync(RequestMethods.ResourcesRead, SendRequestAsync( RequestMethods.ResourcesRead, requestParams, McpJsonUtilities.JsonContext.Default.ReadResourceRequestParams, McpJsonUtilities.JsonContext.Default.ReadResourceResult, - cancellationToken: cancellationToken); + cancellationToken: cancellationToken)); } /// diff --git a/src/ModelContextProtocol.Core/Client/McpClient.cs b/src/ModelContextProtocol.Core/Client/McpClient.cs index b238c59c3..da85e9792 100644 --- a/src/ModelContextProtocol.Core/Client/McpClient.cs +++ b/src/ModelContextProtocol.Core/Client/McpClient.cs @@ -92,6 +92,20 @@ private protected abstract ValueTask> Resolve /// private protected abstract int MaxConsecutiveStuckPolls { get; } + /// + /// Inspects a received cacheable result (tools/list, prompts/list, resources/list, + /// resources/templates/list, or resources/read) so derived clients can emit diagnostics. + /// + /// The request method that produced the result. + /// The cacheable result returned by the server. + /// + /// This is used to warn (never throw) when a server that negotiated a protocol version requiring the + /// SEP-2549 ttlMs/cacheScope fields omits them. The default implementation does nothing. + /// + private protected virtual void ValidateCacheableResult(string method, ICacheableResult result) + { + } + /// /// Registers one or more tool definitions in the client's tool cache, enabling the transport /// to send Mcp-Param-* headers for those tools without requiring a prior call. diff --git a/src/ModelContextProtocol.Core/Client/McpClientImpl.cs b/src/ModelContextProtocol.Core/Client/McpClientImpl.cs index 894ca6945..a49cf71db 100644 --- a/src/ModelContextProtocol.Core/Client/McpClientImpl.cs +++ b/src/ModelContextProtocol.Core/Client/McpClientImpl.cs @@ -25,6 +25,7 @@ internal sealed partial class McpClientImpl : McpClient private readonly SemaphoreSlim _disposeLock = new(1, 1); private readonly ConcurrentDictionary _toolCache = new(StringComparer.Ordinal); private readonly ConcurrentDictionary _registeredToolNames = new(StringComparer.Ordinal); + private readonly ConcurrentDictionary _cacheableConformanceWarnedMethods = new(StringComparer.Ordinal); private ServerCapabilities? _serverCapabilities; private Implementation? _serverInfo; @@ -578,6 +579,34 @@ private void WarnIfInputRequiredResultOnNonMrtrSession(string method) } } + /// + /// Logs a warning (never throws) when a server that negotiated the draft protocol version omits the + /// SEP-2549 ttlMs/cacheScope fields, which are required on cacheable results for that version. + /// The warning is emitted at most once per method per session so that paginated listings do not produce + /// one warning per page. + /// + private protected override void ValidateCacheableResult(string method, ICacheableResult result) + { + if (_negotiatedProtocolVersion != McpSessionHandler.DraftProtocolVersion) + { + return; + } + + bool missingTtl = result.TimeToLive is null; + bool missingScope = result.CacheScope is null; + if ((missingTtl || missingScope) && _cacheableConformanceWarnedMethods.TryAdd(method, 0)) + { + string missingFields = + missingTtl && missingScope ? "ttlMs, cacheScope" : + missingTtl ? "ttlMs" : + "cacheScope"; + LogCacheableResultMissingRequiredFields(_endpointName, method, missingFields, _negotiatedProtocolVersion); + } + } + + [LoggerMessage(Level = LogLevel.Warning, Message = "{EndpointName} received '{Method}' result missing required SEP-2549 field(s) '{MissingFields}' from a server that negotiated protocol version '{ProtocolVersion}'. The server may not be spec-compliant.")] + private partial void LogCacheableResultMissingRequiredFields(string endpointName, string method, string missingFields, string? protocolVersion); + [LoggerMessage(Level = LogLevel.Warning, Message = "{EndpointName} received legacy '{Method}' JSON-RPC request on session that negotiated MRTR. The server should use InputRequiredResult instead of sending direct requests.")] private partial void LogLegacyRequestOnMrtrSession(string endpointName, string method); diff --git a/tests/ModelContextProtocol.Tests/Protocol/CacheableResultWarningTests.cs b/tests/ModelContextProtocol.Tests/Protocol/CacheableResultWarningTests.cs new file mode 100644 index 000000000..afe6ad580 --- /dev/null +++ b/tests/ModelContextProtocol.Tests/Protocol/CacheableResultWarningTests.cs @@ -0,0 +1,317 @@ +#if !NET472 +using Microsoft.Extensions.Logging; +using ModelContextProtocol.Client; +using ModelContextProtocol.Protocol; +using ModelContextProtocol.Tests.Utils; +using System.IO.Pipelines; +using System.Text.Json; +using System.Text.Json.Nodes; + +namespace ModelContextProtocol.Tests.Protocol; + +/// +/// Tests for the client-side SEP-2549 conformance warning: when a server that negotiated the draft +/// protocol version returns a cacheable result (tools/list, prompts/list, resources/list, +/// resources/templates/list, resources/read) without the now-required ttlMs/cacheScope +/// fields, the client logs a warning but never throws. +/// +public class CacheableResultWarningTests : LoggedTest +{ + private const string DraftProtocolVersion = "DRAFT-2026-v1"; + private const string OlderProtocolVersion = "2025-11-25"; + + public CacheableResultWarningTests(ITestOutputHelper testOutputHelper) + : base(testOutputHelper) + { + } + + public static IEnumerable CacheableMethods => + [ + [RequestMethods.ToolsList], + [RequestMethods.PromptsList], + [RequestMethods.ResourcesList], + [RequestMethods.ResourcesTemplatesList], + [RequestMethods.ResourcesRead], + ]; + + [Theory] + [MemberData(nameof(CacheableMethods))] + public async Task DraftServerOmittingBothHints_LogsWarning(string method) + { + var (call, result) = GetScenario(method, ttl: null, scope: null); + + await RunScenarioAsync(DraftProtocolVersion, requestDraft: true, method, result, call, TestContext.Current.CancellationToken); + + var warning = Assert.Single(MockLoggerProvider.LogMessages, m => + m.LogLevel == LogLevel.Warning && m.Message.Contains(method) && m.Message.Contains("SEP-2549")); + Assert.Contains("ttlMs", warning.Message); + Assert.Contains("cacheScope", warning.Message); + } + + [Fact] + public async Task DraftServerOmittingOnlyCacheScope_WarnsAboutCacheScope() + { + var (call, result) = GetScenario(RequestMethods.ToolsList, ttl: TimeSpan.FromMinutes(5), scope: null); + + await RunScenarioAsync(DraftProtocolVersion, requestDraft: true, RequestMethods.ToolsList, result, call, TestContext.Current.CancellationToken); + + var warning = Assert.Single(MockLoggerProvider.LogMessages, m => + m.LogLevel == LogLevel.Warning && m.Message.Contains("SEP-2549")); + Assert.Contains("cacheScope", warning.Message); + Assert.DoesNotContain("ttlMs", warning.Message); + } + + [Fact] + public async Task DraftServerProvidingBothHints_DoesNotWarn() + { + var (call, result) = GetScenario(RequestMethods.ToolsList, ttl: TimeSpan.FromMinutes(5), scope: CacheScope.Public); + + await RunScenarioAsync(DraftProtocolVersion, requestDraft: true, RequestMethods.ToolsList, result, call, TestContext.Current.CancellationToken); + + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => + m.LogLevel == LogLevel.Warning && m.Message.Contains("SEP-2549")); + } + + [Fact] + public async Task OlderServerOmittingHints_DoesNotWarn() + { + // A server on an older protocol version may legitimately omit the fields; no warning should fire. + var (call, result) = GetScenario(RequestMethods.ToolsList, ttl: null, scope: null); + + await RunScenarioAsync(OlderProtocolVersion, requestDraft: false, RequestMethods.ToolsList, result, call, TestContext.Current.CancellationToken); + + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => + m.LogLevel == LogLevel.Warning && m.Message.Contains("SEP-2549")); + } + + [Fact] + public async Task AutoPaginatingOverload_DraftServerOmittingHints_LogsWarning() + { + // The auto-paginating convenience overload calls the raw overload per page, so the warning + // fires through that path as well. + var result = JsonSerializer.SerializeToNode( + new ListToolsResult { Tools = [new Tool { Name = "echo" }] }, + McpJsonUtilities.DefaultOptions)!; + + await RunScenarioAsync( + DraftProtocolVersion, + requestDraft: true, + RequestMethods.ToolsList, + result, + (c, ct) => c.ListToolsAsync(cancellationToken: ct).AsTask(), + TestContext.Current.CancellationToken); + + Assert.Single(MockLoggerProvider.LogMessages, m => + m.LogLevel == LogLevel.Warning && m.Message.Contains(RequestMethods.ToolsList) && m.Message.Contains("SEP-2549")); + } + + [Fact] + public async Task AutoPaginatingOverload_MultiplePages_WarnsOnlyOncePerMethod() + { + // A non-conformant draft server omits the hints on every page. The warning must be emitted at + // most once per method per session so that long paginated listings do not flood the log. + const int pageCount = 4; + + var clientToServer = new Pipe(); + var serverToClient = new Pipe(); + + var clientTask = McpClient.CreateAsync( + new StreamClientTransport( + clientToServer.Writer.AsStream(), + serverToClient.Reader.AsStream(), + LoggerFactory), + new McpClientOptions { ProtocolVersion = DraftProtocolVersion }, + loggerFactory: LoggerFactory, + cancellationToken: TestContext.Current.CancellationToken); + + var serverReader = new StreamReader(clientToServer.Reader.AsStream()); + var serverWriter = serverToClient.Writer.AsStream(); + + var initLine = await serverReader.ReadLineAsync(TestContext.Current.CancellationToken); + Assert.NotNull(initLine); + var initRequest = JsonSerializer.Deserialize(initLine, McpJsonUtilities.DefaultOptions); + Assert.NotNull(initRequest); + + await WriteJsonRpcAsync(serverWriter, new JsonRpcResponse + { + Id = initRequest.Id, + Result = JsonSerializer.SerializeToNode(new InitializeResult + { + ProtocolVersion = DraftProtocolVersion, + Capabilities = new ServerCapabilities(), + ServerInfo = new Implementation { Name = "MockServer", Version = "1.0" }, + }, McpJsonUtilities.DefaultOptions), + }, TestContext.Current.CancellationToken); + + var initializedLine = await serverReader.ReadLineAsync(TestContext.Current.CancellationToken); + Assert.NotNull(initializedLine); + + await using var client = await clientTask; + + // Respond to each tools/list page omitting the hints, advancing the cursor until the last page. + var serverLoop = Task.Run(async () => + { + int page = 0; + while (true) + { + var line = await serverReader.ReadLineAsync(TestContext.Current.CancellationToken); + if (line is null) + { + return; + } + + if (JsonSerializer.Deserialize(line, McpJsonUtilities.DefaultOptions) is JsonRpcRequest request && + request.Method == RequestMethods.ToolsList) + { + page++; + var result = new ListToolsResult + { + Tools = [new Tool { Name = $"tool{page}" }], + NextCursor = page < pageCount ? $"page{page}" : null, + }; + + await WriteJsonRpcAsync(serverWriter, new JsonRpcResponse + { + Id = request.Id, + Result = JsonSerializer.SerializeToNode(result, McpJsonUtilities.DefaultOptions), + }, TestContext.Current.CancellationToken); + + if (page >= pageCount) + { + return; + } + } + } + }, TestContext.Current.CancellationToken); + + var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken); + await serverLoop; + + Assert.Equal(pageCount, tools.Count); + Assert.Single(MockLoggerProvider.LogMessages, m => + m.LogLevel == LogLevel.Warning && m.Message.Contains(RequestMethods.ToolsList) && m.Message.Contains("SEP-2549")); + + clientToServer.Writer.Complete(); + serverToClient.Writer.Complete(); + } + + private static (Func Call, JsonNode Result) GetScenario( + string method, TimeSpan? ttl, CacheScope? scope) + { + var options = McpJsonUtilities.DefaultOptions; + return method switch + { + RequestMethods.ToolsList => ( + (c, ct) => c.ListToolsAsync(new ListToolsRequestParams(), ct).AsTask(), + JsonSerializer.SerializeToNode(new ListToolsResult { Tools = [], TimeToLive = ttl, CacheScope = scope }, options)!), + RequestMethods.PromptsList => ( + (c, ct) => c.ListPromptsAsync(new ListPromptsRequestParams(), ct).AsTask(), + JsonSerializer.SerializeToNode(new ListPromptsResult { Prompts = [], TimeToLive = ttl, CacheScope = scope }, options)!), + RequestMethods.ResourcesList => ( + (c, ct) => c.ListResourcesAsync(new ListResourcesRequestParams(), ct).AsTask(), + JsonSerializer.SerializeToNode(new ListResourcesResult { Resources = [], TimeToLive = ttl, CacheScope = scope }, options)!), + RequestMethods.ResourcesTemplatesList => ( + (c, ct) => c.ListResourceTemplatesAsync(new ListResourceTemplatesRequestParams(), ct).AsTask(), + JsonSerializer.SerializeToNode(new ListResourceTemplatesResult { ResourceTemplates = [], TimeToLive = ttl, CacheScope = scope }, options)!), + RequestMethods.ResourcesRead => ( + (c, ct) => c.ReadResourceAsync(new ReadResourceRequestParams { Uri = "test://resource" }, ct).AsTask(), + JsonSerializer.SerializeToNode(new ReadResourceResult { Contents = [], TimeToLive = ttl, CacheScope = scope }, options)!), + _ => throw new ArgumentOutOfRangeException(nameof(method), method, "Unhandled method."), + }; + } + + private async Task RunScenarioAsync( + string serverProtocolVersion, + bool requestDraft, + string method, + JsonNode resultNode, + Func clientCall, + CancellationToken cancellationToken) + { + var clientToServer = new Pipe(); + var serverToClient = new Pipe(); + + var clientOptions = new McpClientOptions(); + if (requestDraft) + { + clientOptions.ProtocolVersion = DraftProtocolVersion; + } + + var clientTask = McpClient.CreateAsync( + new StreamClientTransport( + clientToServer.Writer.AsStream(), + serverToClient.Reader.AsStream(), + LoggerFactory), + clientOptions, + loggerFactory: LoggerFactory, + cancellationToken: cancellationToken); + + var serverReader = new StreamReader(clientToServer.Reader.AsStream()); + var serverWriter = serverToClient.Writer.AsStream(); + + // Handshake: read initialize, respond with the negotiated protocol version. + var initLine = await serverReader.ReadLineAsync(cancellationToken); + Assert.NotNull(initLine); + var initRequest = JsonSerializer.Deserialize(initLine, McpJsonUtilities.DefaultOptions); + Assert.NotNull(initRequest); + Assert.Equal("initialize", initRequest.Method); + + await WriteJsonRpcAsync(serverWriter, new JsonRpcResponse + { + Id = initRequest.Id, + Result = JsonSerializer.SerializeToNode(new InitializeResult + { + ProtocolVersion = serverProtocolVersion, + Capabilities = new ServerCapabilities(), + ServerInfo = new Implementation { Name = "MockServer", Version = "1.0" }, + }, McpJsonUtilities.DefaultOptions), + }, cancellationToken); + + // Read the initialized notification. + var initializedLine = await serverReader.ReadLineAsync(cancellationToken); + Assert.NotNull(initializedLine); + + await using var client = await clientTask; + Assert.Equal(serverProtocolVersion, client.NegotiatedProtocolVersion); + + // Background server loop: respond to the target request with the crafted result. + var serverLoop = Task.Run(async () => + { + while (true) + { + var line = await serverReader.ReadLineAsync(cancellationToken); + if (line is null) + { + return; + } + + if (JsonSerializer.Deserialize(line, McpJsonUtilities.DefaultOptions) is JsonRpcRequest request && + request.Method == method) + { + await WriteJsonRpcAsync(serverWriter, new JsonRpcResponse + { + Id = request.Id, + Result = resultNode, + }, cancellationToken); + return; + } + } + }, cancellationToken); + + await clientCall(client, cancellationToken); + await serverLoop; + + clientToServer.Writer.Complete(); + serverToClient.Writer.Complete(); + } + + private static async Task WriteJsonRpcAsync(Stream writer, JsonRpcMessage message, CancellationToken cancellationToken) + { + var bytes = JsonSerializer.SerializeToUtf8Bytes(message, McpJsonUtilities.DefaultOptions); + await writer.WriteAsync(bytes, cancellationToken); + await writer.WriteAsync("\n"u8.ToArray(), cancellationToken); + await writer.FlushAsync(cancellationToken); + } +} + +#endif