Skip to content

skills.mdx: CacheableResult on skills/list and skills/get, state 2026-07-28 baseline - #139

Merged
pja-ant merged 1 commit into
mainfrom
spec/skills-cacheable-followup
Sep 10, 2026
Merged

skills.mdx: CacheableResult on skills/list and skills/get, state 2026-07-28 baseline#139
pja-ant merged 1 commit into
mainfrom
spec/skills-cacheable-followup

Conversation

@pja-ant

@pja-ant pja-ant commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #138 addressing @panyam's review comment on specification/stable/skills.mdx, cross-checked against the base 2026-07-28 schema.

Before:

interface ListSkillsResult extends PaginatedResult { skills: Skill[] }
interface GetSkillResult extends Result { skill: Skill }

After:

interface ListSkillsResult extends PaginatedResult, CacheableResult { skills: Skill[] }
interface GetSkillResult extends CacheableResult { skill: Skill }

Changes:

  • ListSkillsResult now extends CacheableResult. PaginatedResult alone does not carry ttlMs/cacheScope; those live on CacheableResult and are required there, as on tools/list and resources/list. The prose bullet now says the fields are required, and every skills/list example response carries them.
  • GetSkillResult now extends CacheableResult. SEP-2640 left open whether skills/get carries caching attributes "as resources/read results do". This resolves it: yes, matching ReadResourceResult. A new semantics bullet explains ttlMs as the hint for when to re-call skills/get, and notes neither field is an integrity property. Every skills/get example response carries them.
  • New "Protocol Revision" section stating the page is written against base revision 2026-07-28 or later and naming the base types it references. This is why the SEP's "in protocol versions 2026-07-28 and later" conditional is not carried.
  • resources-capability line reworded as a consequence of the base Resources specification (a server serving resources/read declares resources) rather than a free-standing new MUST. The requirement itself is kept so the conformance suite can check it.
  • The Example Message Flow's resources/read response also gains ttlMs/cacheScope, since ReadResourceResult requires them at this revision.
  • docs/decisions.md gets a 2026-09-08 entry (Status: Proposed) recording the three normative points above as deliberate, per this repo's process for changes beyond the v1 SEP text.

Not changed here: ReadResourceDirectoryResult still extends PaginatedResult only. Its closest analogue ListResourcesResult is cacheable, so the same question applies; left for a separate decision.

All JSON examples parse and all internal anchors resolve.

🤖 Generated with Claude Code

Requested by Peter · Claude session #01Sure6S

@pja-ant
pja-ant marked this pull request as ready for review September 8, 2026 12:02
@panyam

panyam commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

👍

@panyam

panyam commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Thanks for this @pja-ant - LGTM. I like the reasoning indocs/decisions.md.

Couple of things that I wanted to flag here instead of on 330:

1. Decision 2 looks like a behaviour change

In SEP-2640 skills/get it says that carrying the caching attributes "is left open", so a server omitting them today is conformant to the SEP and stops being conformant to this page on merge.

Decisions 1 and 3 look like corrections, ie - the wrong base type and a consequence of the base Resources spec respectively, but here it looks like we are adding a requirement. I do agree with your reasoning - Perhaps we could clarify that it it is new - this would remove the doubts for implementors.

2. ReadResourceDirectoryResult.

Also +1 on this being a separate decision. The suite already has a resources/directory/read scenario, so a change there picks up a check in the same way as the others. Could we get an issue for it so it does not get lost in the merge?

On the conditional, I raised the 2026-07-28 gating question on modelcontextprotocol/conformance#330 and would be great to keep it one place instead of two. We have three implementations and the pages have each resolved the conditional independently so wondering if the SEP text should follow? Wdyt?

@pja-ant

pja-ant commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

so a server omitting them today is conformant to the SEP and stops being conformant to this page on merge.

Perhaps we could clarify that it it is new - this would remove the doubts for implementors

True... but the extension isn't final. Servers written today against the non-final extension should have no expectations of stability. I don't know where we'd mark anything as "new" since there is no stable specification to say that it is new against. I feel it would be odd for the first version of a specification to reference changes relative to a work-in-progress SEP?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants