mcp: add ServerOptions.DefaultCacheable - #1128
Conversation
| } | ||
| } | ||
|
|
||
| func TestSetDefaultCacheableValues(t *testing.T) { |
There was a problem hiding this comment.
lets remove this test
There was a problem hiding this comment.
You did not remove the test i intended
There was a problem hiding this comment.
Removed TestSetDefaultCacheableValues (this was the one you intended). Behavior is covered by TestServerDefaultCacheable under the new callback API, including empty CacheScope → "public".
|
Removed |
7d4f460 to
7a5a2c0
Compare
| func (c *Cacheable) setDefaultCacheableValues(defaults *Cacheable) { | ||
| if defaults != nil { | ||
| c.TTLMs = defaults.TTLMs | ||
| c.CacheScope = defaults.CacheScope | ||
| return | ||
| } |
There was a problem hiding this comment.
the issue here is that if defaults does not set the CacheScope, it will not be set to public
7a5a2c0 to
393674c
Compare
|
@guglielmo-san Thanks — fixed |
| // DefaultCacheable, if non-nil, supplies the [Cacheable] values stamped on | ||
| // SDK-generated results (server/discover, list methods, and resources/read | ||
| // after the handler returns). If nil, those results use the historical | ||
| // defaults: CacheScope "public" and TTLMs 0. | ||
| // | ||
| // Receiving middleware can still overwrite Cacheable on a per-result basis | ||
| // after the SDK stamps these values. | ||
| DefaultCacheable *Cacheable |
There was a problem hiding this comment.
What do you think about using a callback instead? I think it would give more flexibility.
// DefaultCacheable, if non-nil, is called for each SDK-generated
// cacheable result (server/discover, *_list, resources/read) to
// determine the Cacheable values stamped on the response. The
// request is provided so policy can vary by method, session,
// tenant, or authentication state.
//
// If nil, SDK-generated results use CacheScope "public" and
// TTLMs 0, matching the historical defaults.
DefaultCacheable func(ctx context.Context, req Request) Cacheable
There was a problem hiding this comment.
Adopted as proposed: DefaultCacheable func(ctx context.Context, req Request) Cacheable. See tip 15b7b8f.
| } | ||
| } | ||
|
|
||
| func TestSetDefaultCacheableValues(t *testing.T) { |
There was a problem hiding this comment.
You did not remove the test i intended
Allow servers to override the Cacheable defaults stamped on SDK-generated results (discover, list methods, resources/read). Nil keeps historical public/0 defaults. Fixes modelcontextprotocol#1094
ServerOptions.DefaultCacheable may set only TTLMs. Empty CacheScope on defaults must still fall back to "public" instead of stamping "". Signed-off-by: arimu1 <19286898+arimu1@users.noreply.github.com>
Replace the static *Cacheable option with a callback that receives context and Request so cache policy can vary by method, session, tenant, or auth. Empty CacheScope from the callback still falls back to "public". Remove TestSetDefaultCacheableValues per review; cover behavior via TestServerDefaultCacheable. Fixes modelcontextprotocol#1094
393674c to
15b7b8f
Compare
|
@guglielmo-san Thanks for the feedback — adopted the callback API: DefaultCacheable func(ctx context.Context, req Request) Cacheable
Tip: |
2 participants
Summary
Adds
ServerOptions.DefaultCacheable *Cacheableso multi-tenant / private servers can override the SDK's forced cacheable defaults (cacheScope: "public",ttlMs: 0) on SDK-generated results.setDefaultCacheableValuesonserver/discover,tools/list,prompts/list,resources/list,resources/templates/list, andresources/read(after the handler returns).Cacheableper result after the SDK stamps values (useful for per-request / per-tenant policy).Fixes #1094
Motivation
Servers whose list/discover payloads are tenant-scoped want
private(and a real TTL) as the SDK baseline, not only via post-hoc middleware type-switches. A single option covers all current SDK stamp sites and any future call sites that use the same helper.Test plan
go test ./mcp/ -run 'TestServerDefaultCacheable|TestSetDefaultCacheableValues' -count=1go test ./mcp/ -count=1go test ./... -count=1DefaultCacheableis nilprivate/60s stamped on list + read results when setTTLMs(read handler) while forcingCacheScope: "public"