Skip to content

implementing SEP-2549#889

Open
michaelneale wants to merge 1 commit into
mainfrom
micn/sep-2549-ttl-list-results
Open

implementing SEP-2549#889
michaelneale wants to merge 1 commit into
mainfrom
micn/sep-2549-ttl-list-results

Conversation

@michaelneale
Copy link
Copy Markdown
Contributor

implementing #875

(assisted by mic and goose)

@michaelneale michaelneale requested a review from a team as a code owner June 5, 2026 09:39
@github-actions github-actions Bot added T-test Testing related changes T-core Core library changes T-macros Macro changes labels Jun 5, 2026
@michaelneale michaelneale force-pushed the micn/sep-2549-ttl-list-results branch 2 times, most recently from 7175cdf to a6151ad Compare June 5, 2026 09:52
@michaelneale michaelneale force-pushed the micn/sep-2549-ttl-list-results branch from a6151ad to f8edae7 Compare June 5, 2026 09:55
@github-actions github-actions Bot removed the T-macros Macro changes label Jun 5, 2026
Copy link
Copy Markdown
Contributor

@alexhancock alexhancock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread crates/rmcp/src/model.rs
#[serde(rename_all = "camelCase")]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
#[non_exhaustive]
pub enum CacheScope {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-core Core library changes T-test Testing related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants