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

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?

@pja-ant
pja-ant merged commit d866efd into main Sep 10, 2026
olaservo added a commit that referenced this pull request Sep 15, 2026
Its text already shipped in specification/stable/skills.mdx via PR #139, so
the entry joins the three v1 entries this PR ratifies.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
sambhav pushed a commit that referenced this pull request Sep 16, 2026
* Initial plan

* docs: ratify three Proposed decision log entries to Accepted

Co-authored-by: olaservo <16480113+olaservo@users.noreply.github.com>

* docs: mark the 2026-09-08 stable-spec decision Accepted

Its text already shipped in specification/stable/skills.mdx via PR #139, so
the entry joins the three v1 entries this PR ratifies.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: olaservo <16480113+olaservo@users.noreply.github.com>
Co-authored-by: Ola Hungerford <olahungerford@gmail.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
olaservo pushed a commit that referenced this pull request Oct 15, 2026
…157)

Fixes #156.

Aligns `specification/stable/skills.mdx` with the `2026-07-28` base
schema and records the decision.

- `skills/get`, `resources/directory/read`: named params interfaces, so
`_meta` is required
- `ReadResourceDirectoryResult` extends `CacheableResult` (**breaking**:
`ttlMs`, `cacheScope` required)
- `*ResultResponse` types for each method; optional `_meta` on `Skill`
- Request examples carry the full required `_meta`

## Why this lands in stable

These are corrections to the published page rather than new design:

- **The page doesn't compile against its own stated baseline.** It says
its types are those of the `2026-07-28` schema, and
`ReadResourceDirectoryRequest` fails to type-check against it (#156).
- **`_meta` was already required.** The page's text says every request
MUST include it. Only the types and examples disagreed.
- **Directory-read caching was missed in the 2026-09-08 pass.** That
decision made `skills/list` and `skills/get` cacheable so that every
result type in the extension stays uniform with the base protocol;
`resources/directory/read` was the one it didn't catch. This is the only
change that adds a requirement on servers, and it affects an optional
method behind `directoryRead: true`.

Corrections like these have landed on stable in place before (#139, and
#147 after SEP-2640 went Final), so I've followed that route rather than
opening a `draft/` revision. `index.md` excerpt ranges are re-anchored
to the new line numbers.

## Verification

All interfaces and all 16 examples type-check under `--strict` against
the base schema; `docs:build` passes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01M8b7Vuu2jRefMhoZB3BrrX


---
_Generated by [Claude Code](https://claude.ai/code)_

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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