Skip to content

Document caching behavior and warn on non-conformant draft cacheable results (SEP-2549 follow-up) #1669

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tarekgh wants to merge 1 commit into
base: main
Choose a base branch
from
+415 −10
Open

Document caching behavior and warn on non-conformant draft cacheable results (SEP-2549 follow-up) #1669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 55 additions & 10 deletions src/ModelContextProtocol.Core/Client/McpClient.Methods.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,18 @@ public ValueTask<PingResult> PingAsync(
/// <returns>A list of all available tools as <see cref="McpClientTool"/> instances.</returns>
/// <exception cref="McpException">The request failed or the server returned an error response.</exception>
/// <remarks>
/// <para>
/// This overload aggregates every page into a single list and does not surface the per-result caching hints
/// (<see cref="ListToolsResult.TimeToLive"/> and <see cref="ListToolsResult.CacheScope"/>). To read those hints,
/// use the <see cref="ListToolsAsync(ListToolsRequestParams, CancellationToken)"/> overload, which returns the
/// raw <see cref="ListToolsResult"/> for each page.
/// </para>
/// <para>
/// 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
/// <see cref="ListToolsAsync(ListToolsRequestParams, CancellationToken)"/> overload, which exposes the per-page
/// caching hints and lets you manage pagination so each page can be cached and expired independently.
/// </para>
/// </remarks>
public async ValueTask<IList<McpClientTool>> ListToolsAsync(
RequestOptions? options = null,
Expand Down Expand Up @@ -247,12 +255,25 @@ public ValueTask<ListToolsResult> 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));
}

/// <summary>
/// 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.
/// </summary>
private async ValueTask<TResult> ValidateCacheableResultAsync<TResult>(string method, ValueTask<TResult> resultTask)
where TResult : ICacheableResult
{
var result = await resultTask.ConfigureAwait(false);
ValidateCacheableResult(method, result);
return result;
}

/// <summary>
Expand All @@ -263,10 +284,18 @@ public ValueTask<ListToolsResult> ListToolsAsync(
/// <returns>A list of all available prompts as <see cref="McpClientPrompt"/> instances.</returns>
/// <exception cref="McpException">The request failed or the server returned an error response.</exception>
/// <remarks>
/// <para>
/// This overload aggregates every page into a single list and does not surface the per-result caching hints
/// (<see cref="ListPromptsResult.TimeToLive"/> and <see cref="ListPromptsResult.CacheScope"/>). To read those hints,
/// use the <see cref="ListPromptsAsync(ListPromptsRequestParams, CancellationToken)"/> overload, which returns the
/// raw <see cref="ListPromptsResult"/> for each page.
/// </para>
/// <para>
/// 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
/// <see cref="ListPromptsAsync(ListPromptsRequestParams, CancellationToken)"/> overload, which exposes the per-page
/// caching hints and lets you manage pagination so each page can be cached and expired independently.
/// </para>
/// </remarks>
public async ValueTask<IList<McpClientPrompt>> ListPromptsAsync(
RequestOptions? options = null,
Expand Down Expand Up @@ -309,12 +338,12 @@ public ValueTask<ListPromptsResult> 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));
}

/// <summary>
Expand Down Expand Up @@ -379,10 +408,18 @@ public ValueTask<GetPromptResult> GetPromptAsync(
/// <returns>A list of all available resource templates as <see cref="ResourceTemplate"/> instances.</returns>
/// <exception cref="McpException">The request failed or the server returned an error response.</exception>
/// <remarks>
/// <para>
/// This overload aggregates every page into a single list and does not surface the per-result caching hints
/// (<see cref="ListResourceTemplatesResult.TimeToLive"/> and <see cref="ListResourceTemplatesResult.CacheScope"/>). To read those hints,
/// use the <see cref="ListResourceTemplatesAsync(ListResourceTemplatesRequestParams, CancellationToken)"/> overload, which returns the
/// raw <see cref="ListResourceTemplatesResult"/> for each page.
/// </para>
/// <para>
/// 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
/// <see cref="ListResourceTemplatesAsync(ListResourceTemplatesRequestParams, CancellationToken)"/> overload, which exposes the per-page
/// caching hints and lets you manage pagination so each page can be cached and expired independently.
/// </para>
/// </remarks>
public async ValueTask<IList<McpClientResourceTemplate>> ListResourceTemplatesAsync(
RequestOptions? options = null,
Expand Down Expand Up @@ -425,12 +462,12 @@ public ValueTask<ListResourceTemplatesResult> 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));
}

/// <summary>
Expand All @@ -441,10 +478,18 @@ public ValueTask<ListResourceTemplatesResult> ListResourceTemplatesAsync(
/// <returns>A list of all available resources as <see cref="Resource"/> instances.</returns>
/// <exception cref="McpException">The request failed or the server returned an error response.</exception>
/// <remarks>
/// <para>
/// This overload aggregates every page into a single list and does not surface the per-result caching hints
/// (<see cref="ListResourcesResult.TimeToLive"/> and <see cref="ListResourcesResult.CacheScope"/>). To read those hints,
/// use the <see cref="ListResourcesAsync(ListResourcesRequestParams, CancellationToken)"/> overload, which returns the
/// raw <see cref="ListResourcesResult"/> for each page.
/// </para>
/// <para>
/// 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
/// <see cref="ListResourcesAsync(ListResourcesRequestParams, CancellationToken)"/> overload, which exposes the per-page
/// caching hints and lets you manage pagination so each page can be cached and expired independently.
/// </para>
/// </remarks>
public async ValueTask<IList<McpClientResource>> ListResourcesAsync(
RequestOptions? options = null,
Expand Down Expand Up @@ -487,12 +532,12 @@ public ValueTask<ListResourcesResult> 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));
}

/// <summary>
Expand Down Expand Up @@ -571,12 +616,12 @@ public ValueTask<ReadResourceResult> 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));
}

/// <summary>
Expand Down
14 changes: 14 additions & 0 deletions src/ModelContextProtocol.Core/Client/McpClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,20 @@ private protected abstract ValueTask<IDictionary<string, InputResponse>> Resolve
/// </summary>
private protected abstract int MaxConsecutiveStuckPolls { get; }

/// <summary>
/// Inspects a received cacheable result (<c>tools/list</c>, <c>prompts/list</c>, <c>resources/list</c>,
/// <c>resources/templates/list</c>, or <c>resources/read</c>) so derived clients can emit diagnostics.
/// </summary>
/// <param name="method">The request method that produced the result.</param>
/// <param name="result">The cacheable result returned by the server.</param>
/// <remarks>
/// This is used to warn (never throw) when a server that negotiated a protocol version requiring the
/// SEP-2549 <c>ttlMs</c>/<c>cacheScope</c> fields omits them. The default implementation does nothing.
/// </remarks>
private protected virtual void ValidateCacheableResult(string method, ICacheableResult result)
{
}

/// <summary>
/// Registers one or more tool definitions in the client's tool cache, enabling the transport
/// to send <c>Mcp-Param-*</c> headers for those tools without requiring a prior <see cref="McpClient.ListToolsAsync(RequestOptions?, CancellationToken)"/> call.
Expand Down
29 changes: 29 additions & 0 deletions src/ModelContextProtocol.Core/Client/McpClientImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ internal sealed partial class McpClientImpl : McpClient
private readonly SemaphoreSlim _disposeLock = new(1, 1);
private readonly ConcurrentDictionary<string, Tool> _toolCache = new(StringComparer.Ordinal);
private readonly ConcurrentDictionary<string, byte> _registeredToolNames = new(StringComparer.Ordinal);
private readonly ConcurrentDictionary<string, byte> _cacheableConformanceWarnedMethods = new(StringComparer.Ordinal);

private ServerCapabilities? _serverCapabilities;
private Implementation? _serverInfo;
Expand Down Expand Up @@ -578,6 +579,34 @@ private void WarnIfInputRequiredResultOnNonMrtrSession(string method)
}
}

/// <summary>
/// Logs a warning (never throws) when a server that negotiated the draft protocol version omits the
/// SEP-2549 <c>ttlMs</c>/<c>cacheScope</c> 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.
/// </summary>
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);

Expand Down
Loading
Loading