implementing SEP-2549 TTL for List Results

[michaelneale]

implementing #875

(assisted by mic and goose)

[alexhancock]

Pre-approving so you can merge when the below is addressed!

The cache hints (ttlMs and cacheScope) are written to _meta, but the spec defines them as first-class result fields

Per SEP-2549, ttlMs and cacheScope are properties on the result itself (CacheableResult), so the wire shape is { "tools": [...], "ttlMs": 300000, "cacheScope": "public" } (example, L112) — not nested under _meta

Fix list:

• Top-level fields: make ttl_ms/cache_scope real #[serde(rename = "ttlMs"/"cacheScope")] fields on the result struct instead of meta.insert(...).
• resources/read: the same meta.insert pattern is duplicated below in ReadResourceResult, writing into each ResourceContents. The spec attaches the hints to ReadResourceResult itself, so they should live on the result (otherwise they vanish when contents is empty). Scope guidance L90.
• ttlMs rules: u64 correctly blocks negatives on the producing side 👍, but there's no read-side normalization (absent → 0, negative → 0, server MUST emit >= 0). Rules L39-43.

[jamadeo]

I think this does the right thing over the wire, but wouldn't it be a good bit simpler if added on the top level struct itself?

[michaelneale]

ok - this is a breaking change now (well needs new semver version) - not sure of next steps @DaleSeo to get CI happy in that (deliberate) case?

[DaleSeo]

@michaelneale Sorry, I think my earlier suggestion to model these as plain Option fields was wrong. I had not read SEP-2549 carefully enough.

The SEP states:

Servers MUST provide a ttlMs on Results returned by tools/list, prompts/list, resources/list, resources/read, and resources/templates/list.

It also metions:

cacheScope is required because there is no safe default for older servers. The server must explicitly declare the intended cache scope to prevent unintended caching of user-specific data.

Given that, should we make them required with default values?

[howardjohn]

I don't think that will work for the types. if you do that, then its impossible to talk to an older server since their messages will fail to deserialize.

[DaleSeo]

Good point, @howardjohn. I was only considering new servers built with the latest SDK, but you're right. This type is also used by new clients deserializing responses from older servers. I agree that making these fields required would break that compatibility.

Then, I'd like to suggest adding a comment near these fields to explain why they're optional, even though the SEP states that servers MUST provide them.

[DaleSeo]

ok - this is a breaking change now (well needs new semver version) - not sure of next steps @DaleSeo to get CI happy in that (deliberate) case?

The semver CI failure is expected for this PR because it adds public fields. Could you just mark this as a breaking change with a feat!: commit message? That will give release-plz the right breaking-change signal for the changelog and version bump.

[alexhancock]

@michaelneale I think with a rebase and #889 (comment) this would be good to go? Would be good to get it in

[DaleSeo]

Hey @michaelneale, do you have any updates? I'm afraid this could block other spec work.

[DaleSeo]

@jamadeo Thanks for wrapping this up!

[DaleSeo]

@jamadeo Do you mind clarifying why this is optional in the rust doc? I suggested it in #889 (comment).

Suggested change

	/// Scope describing who may cache this result (SEP-2549).
	#[serde(default, skip_serializing_if = "Option::is_none")]
	pub cache_scope: Option<CacheScope>,
	/// Scope describing who may cache this result (SEP-2549).
	///
	/// Required by the schema, but optional here so clients can deserialize older servers that omit it.
	/// Absence must not be treated as [`CacheScope::Public`] for shared caches.
	#[serde(default, skip_serializing_if = "Option::is_none")]
	pub cache_scope: Option<CacheScope>,

[jamadeo]

Yes good point - I'd added as a comment on the other struct and missed this macro. Now both have doc.