From 4ebf5f42ef864794d793ce16624cc1e5c10259f9 Mon Sep 17 00:00:00 2001 From: Vitalii Cherepanov Date: Sat, 10 Oct 2026 11:12:44 +0200 Subject: [PATCH 1/2] mcp: do not cache resources/read results of multi round-trip retries ReadResource cached every result under its URI and looked every request up by URI alone, so the result of a retry carrying inputResponses was served on the next plain read, and a manual retry was answered with the cached interim result. Skip the cache for requests carrying input responses or request state, and do not store retried or input_required results. The caching specification forbids all three. Fixes #1369 --- mcp/client.go | 21 +++++++- mcp/mrtr.go | 11 ++++ mcp/mrtr_test.go | 131 +++++++++++++++++++++++++++++++++++++++++++++++ mcp/protocol.go | 5 ++ 4 files changed, 166 insertions(+), 2 deletions(-) diff --git a/mcp/client.go b/mcp/client.go index 44be0ef6a..cd7cd5c18 100644 --- a/mcp/client.go +++ b/mcp/client.go @@ -1386,7 +1386,13 @@ func (cs *ClientSession) ListResourceTemplates(ctx context.Context, params *List // ReadResource asks the server to read a resource and return its contents. func (cs *ClientSession) ReadResource(ctx context.Context, params *ReadResourceParams) (*ReadResourceResult, error) { - if cs.usesNewProtocol() { + // The cache is keyed by URI alone. A request that carries input responses + // or request state is a multi round-trip retry whose result depends on + // those inputs, so the caching specification keeps it out of the cache in + // both directions: it is not served from the cache and its result is not + // stored (2026-07-28, Caching, "Cache Key"). + useCache := cs.usesNewProtocol() && !isMultiRoundTripRetry(params) + if useCache { var uri string if params != nil { uri = params.URI @@ -1394,18 +1400,29 @@ func (cs *ClientSession) ReadResource(ctx context.Context, params *ReadResourceP if result, ok := cs.readResourceCache.get(uri); ok { return result, nil } + } + if cs.usesNewProtocol() { params = injectRequestMeta(cs, params) } result, err := handleSend[*ReadResourceResult](ctx, methodReadResource, newClientRequest(cs, orZero[Params](params))) if err != nil { return nil, err } - if cs.usesNewProtocol() { + // A result the middleware obtained by retrying is excluded for the same + // reason, and an input-required result is an interim one that the + // specification declares not cacheable. + if useCache && !result.retried && !result.NeedsInput() { cs.readResourceCache.put(params.URI, result) } return result, nil } +// isMultiRoundTripRetry reports whether params carry the input responses or +// request state of a multi round-trip retry. A nil params is a plain read. +func isMultiRoundTripRetry(params *ReadResourceParams) bool { + return params != nil && (params.InputResponses != nil || params.RequestState != "") +} + func (cs *ClientSession) Complete(ctx context.Context, params *CompleteParams) (*CompleteResult, error) { if cs.usesNewProtocol() { params = injectRequestMeta(cs, params) diff --git a/mcp/mrtr.go b/mcp/mrtr.go index 5262cfd45..f085c0ca8 100644 --- a/mcp/mrtr.go +++ b/mcp/mrtr.go @@ -44,6 +44,14 @@ func asksForInput(res multiRoundTripResponse) bool { return res.inputRequests() != nil || res.requestState() != "" } +// A retriedResult is a result that can record that a multi round-trip retry +// produced it. The caching specification forbids caching such results, so +// the one cacheable multi round-trip result, [ReadResourceResult], implements +// it and [ClientSession.ReadResource] consults it. +type retriedResult interface { + markRetried() +} + // validateMultiRoundTripResult rejects a result that carries both content and // input requests or a request state. [annotateResultType] labels the result // later, in [ServerSession.handle], so this function does not set resultType. @@ -82,6 +90,9 @@ func clientMultiRoundTripMiddleware() Middleware { if err != nil { return res, err } + if r, ok := res.(retriedResult); ok && retries > 1 && !res.isNil() { + r.markRetried() + } mrtrResult, ok := res.(multiRoundTripResponse) if !ok { return res, nil diff --git a/mcp/mrtr_test.go b/mcp/mrtr_test.go index d33f56c22..432838fd6 100644 --- a/mcp/mrtr_test.go +++ b/mcp/mrtr_test.go @@ -1310,3 +1310,134 @@ func mustConnectOldProtocol(t *testing.T, s *Server, clientOpts *ClientOptions) }) return cs } + +// gatedResourceServer returns a server whose "test://gated" resource asks for +// an elicitation before it reveals its contents, and whose "test://plain" +// resource answers at once. Every result is marked cacheable for a minute. +// handlerCalls counts invocations of the gated handler. +func gatedResourceServer(handlerCalls *atomic.Int32) *Server { + srv := NewServer(testImpl, &ServerOptions{ + SetCacheable: func(_ context.Context, _ Request, c *Cacheable) { + c.TTLMs = 60_000 + c.CacheScope = "private" + }, + }) + srv.AddResource(&Resource{URI: "test://gated", Name: "gated"}, func(_ context.Context, req *ReadResourceRequest) (*ReadResourceResult, error) { + handlerCalls.Add(1) + if len(req.Params.InputResponses) == 0 { + return &ReadResourceResult{ + InputRequests: InputRequestMap{"confirm": &ElicitParams{Message: "Reveal the document?"}}, + RequestState: "gated-state", + }, nil + } + return &ReadResourceResult{ + Contents: []*ResourceContents{{URI: req.Params.URI, Text: "the document"}}, + }, nil + }) + srv.AddResource(&Resource{URI: "test://plain", Name: "plain"}, func(_ context.Context, req *ReadResourceRequest) (*ReadResourceResult, error) { + return &ReadResourceResult{ + Contents: []*ResourceContents{{URI: req.Params.URI, Text: "plain"}}, + }, nil + }) + return srv +} + +// TestMultiRoundTrip_ReadResource_RetryResultNotCached verifies that a +// resources/read result produced by an automatic multi round-trip retry is +// not cached: the caching specification says results of requests carrying +// inputResponses or requestState MUST NOT be cached, because they depend on +// inputs that are not part of the cache key. A second read of the same URI +// must therefore go back to the server and ask the user again, while a +// resource that needed no input stays cached. +func TestMultiRoundTrip_ReadResource_RetryResultNotCached(t *testing.T) { + ctx := context.Background() + var handlerCalls, elicitations atomic.Int32 + cs := mustConnect(t, gatedResourceServer(&handlerCalls), &ClientOptions{ + ElicitationHandler: func(_ context.Context, _ *ElicitRequest) (*ElicitResult, error) { + elicitations.Add(1) + return &ElicitResult{Action: "accept"}, nil + }, + }) + + for i := 1; i <= 2; i++ { + res, err := cs.ReadResource(ctx, &ReadResourceParams{URI: "test://gated"}) + if err != nil { + t.Fatalf("read %d: ReadResource() error = %v", i, err) + } + if len(res.Contents) != 1 || res.Contents[0].Text != "the document" { + t.Fatalf("read %d: Contents = %+v, want the document", i, res.Contents) + } + // Each read is one input-required round plus one retry. + if got, want := handlerCalls.Load(), int32(2*i); got != want { + t.Errorf("read %d: handler calls = %d, want %d", i, got, want) + } + if got, want := elicitations.Load(), int32(i); got != want { + t.Errorf("read %d: elicitations = %d, want %d; the retry result was served from the cache", i, got, want) + } + } + + // A result that needed no input is cacheable, so the second read of the + // plain resource is served without a round trip. + first, err := cs.ReadResource(ctx, &ReadResourceParams{URI: "test://plain"}) + if err != nil { + t.Fatal(err) + } + second, err := cs.ReadResource(ctx, &ReadResourceParams{URI: "test://plain"}) + if err != nil { + t.Fatal(err) + } + if first != second { + t.Error("second read of test://plain was not served from the cache") + } +} + +// TestMultiRoundTrip_ReadResource_ManualRetryBypassesCache covers the +// caller-driven retry loop. An input-required result is not cacheable, the +// retry request carries parameters the cache key does not include and so +// must reach the server, and the retry's result must not be cached either. +func TestMultiRoundTrip_ReadResource_ManualRetryBypassesCache(t *testing.T) { + ctx := context.Background() + var handlerCalls atomic.Int32 + cs := mustConnect(t, gatedResourceServer(&handlerCalls), &ClientOptions{ + MultiRoundTrip: &MultiRoundTripOptions{Disabled: true}, + }) + + interim, err := cs.ReadResource(ctx, &ReadResourceParams{URI: "test://gated"}) + if err != nil { + t.Fatalf("ReadResource() error = %v", err) + } + if !interim.NeedsInput() { + t.Fatal("NeedsInput() = false, want true") + } + + res, err := cs.ReadResource(ctx, &ReadResourceParams{ + URI: "test://gated", + InputResponses: InputResponseMap{"confirm": &ElicitResult{Action: "accept"}}, + RequestState: interim.RequestState, + }) + if err != nil { + t.Fatalf("ReadResource() retry error = %v", err) + } + if res.NeedsInput() { + t.Fatal("retry: NeedsInput() = true, want false; the interim result was served from the cache") + } + if len(res.Contents) != 1 { + t.Fatalf("retry: len(Contents) = %d, want 1", len(res.Contents)) + } + if got := handlerCalls.Load(); got != 2 { + t.Errorf("handler calls after the retry = %d, want 2", got) + } + + // A fresh read must start the gate over instead of reusing the retry's + // contents. + again, err := cs.ReadResource(ctx, &ReadResourceParams{URI: "test://gated"}) + if err != nil { + t.Fatalf("ReadResource() after retry error = %v", err) + } + if !again.NeedsInput() { + t.Error("read after the retry: NeedsInput() = false, want true; the retry result was cached") + } + if got := handlerCalls.Load(); got != 3 { + t.Errorf("handler calls after the third read = %d, want 3", got) + } +} diff --git a/mcp/protocol.go b/mcp/protocol.go index 22844de26..223a9ea76 100644 --- a/mcp/protocol.go +++ b/mcp/protocol.go @@ -1705,12 +1705,17 @@ type ReadResourceResult struct { // resultType records whether the call finished or needs more client input. // See [CallToolResult.NeedsInput]. resultType resultType + // retried records that the client middleware produced this result by + // retrying the request with input responses. Such a result depends on + // inputs that are not part of the cache key, so it must not be cached. + retried bool } func (*ReadResourceResult) isResult() {} func (x *ReadResourceResult) isNil() bool { return x == nil } func (r *ReadResourceResult) setResultType(rt resultType) { r.resultType = rt } +func (r *ReadResourceResult) markRetried() { r.retried = true } func (r *ReadResourceResult) requestState() string { return r.RequestState } func (r *ReadResourceResult) inputRequests() map[string]InputRequest { if r == nil { From 8aa288c6737850ddf471fe8772b4fe396a8d1506 Mon Sep 17 00:00:00 2001 From: Vitalii Cherepanov Date: Mon, 12 Oct 2026 12:21:17 +0200 Subject: [PATCH 2/2] mcp: mark the retry only on the final result --- mcp/mrtr.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mcp/mrtr.go b/mcp/mrtr.go index f085c0ca8..db7985a8b 100644 --- a/mcp/mrtr.go +++ b/mcp/mrtr.go @@ -90,9 +90,6 @@ func clientMultiRoundTripMiddleware() Middleware { if err != nil { return res, err } - if r, ok := res.(retriedResult); ok && retries > 1 && !res.isNil() { - r.markRetried() - } mrtrResult, ok := res.(multiRoundTripResponse) if !ok { return res, nil @@ -102,6 +99,9 @@ func clientMultiRoundTripMiddleware() Middleware { // (SEP-2322 requires at least one of the two fields), so the // result type, not the map alone, decides whether to retry. if reqMap == nil && !mrtrResult.NeedsInput() { + if r, ok := res.(retriedResult); ok && retries > 1 && !res.isNil() { + r.markRetried() + } return res, nil } // A result with a requestState asks to continue, so it counts as