Repository navigation
fix(skills): require ttlMs and cacheScope on modern resources/directory/read results - #2747
Merged
Merged
Conversation
…ry/read results (#2740) The stable ext-skills spec (ext-skills#157) makes ReadResourceDirectoryResult a CacheableResult, so a modern (2026-07-28+) result must carry ttlMs and cacheScope, as skills/list and skills/get already do. The Inspector parsed both eras with one permissive schema and reported a modern server omitting them as clean; the test server's fixture sent resultType alone and was itself non-conformant on the modern leg. - core/mcp/skillsSchemas.ts: add ModernDirectoryReadResultSchema and replace the "no modern variant" note with the new citation. - core/mcp/inspectorClient.ts: readResourceDirectory picks the schema from the negotiated era, as listSkills and getSkillResult do. - test-servers/src/skills.ts: readDirectoryPage and DirectoryReadResultShape send and type the full modern envelope. - Tests mirror the #2404 coverage for skills/get: schema unit cases, client era cases, and an integration case on both eras. - docs: test-servers.md and the roadmap no longer describe the old asymmetry. Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: olaservo <[email protected]>
There was a problem hiding this comment.
🟢 Approval recommended
The focused changes satisfy the specified requirements, preserve legacy behavior, and include regression coverage with no identified blocking issues.
No open issues 🎉
What changed in this PR
Aligns Inspector’s modern directory-read validation with the stable ext-skills specification while preserving legacy compatibility.
Changes:
- Requires
ttlMsandcacheScopeon modernresources/directory/readresults. - Updates fixtures and adds schema, client, and integration coverage.
- Revises documentation to reflect the caching requirements.
| File | Description |
|---|---|
| test-servers/src/skills.ts | Supplies the full modern result envelope. |
| docs/test-servers.md | Documents updated fixture behavior. |
| docs/inspector-roadmap-2026-h2.md | Includes directory reads in cache-hint planning. |
| core/mcp/skillsSchemas.ts | Adds the modern directory-read schema. |
| core/mcp/inspectorClient.ts | Selects validation by negotiated protocol era. |
| clients/web/src/test/integration/mcp/inspectorClient-skills.test.ts | Checks cache attributes over real transports. |
| clients/web/src/test/core/mcp/skillsSchemas.test.ts | Tests required fields and invalid values. |
| clients/web/src/test/core/mcp/inspectorClient-skills.test.ts | Tests era-specific validation and field preservation. |
Review effort: Balanced
Member
Author
|
Copilot review loop closed after round 1: the round posted with approval recommended, no inline comments, no headline finding and no suppressed comments, so nothing to answer and no further round requested. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2740
The stable ext-skills spec (ext-skills#157) makes
ReadResourceDirectoryResultaCacheableResult, so a modern (2026-07-28+) result must carryttlMsandcacheScope, asskills/listandskills/getalready do. The Inspector parsed both eras with one permissive schema and reported a modern server omitting them as clean; the test server's fixture sentresultTypealone and was itself non-conformant on the modern leg.core/mcp/skillsSchemas.ts: addModernDirectoryReadResultSchemaand replace the "no modern variant" note with the new citation.core/mcp/inspectorClient.ts:readResourceDirectorypicks the schema from the negotiated era, aslistSkillsandgetSkillResultdo. The result is returned whole, so the CLI's--method resources/directory/readoutput gains the two fields on modern connections.test-servers/src/skills.ts:readDirectoryPageandDirectoryReadResultShapesend and type the full modern envelope.skills/get: schema unit cases, client era cases, and an integration case on both eras.docs/test-servers.mdand the roadmap no longer describe the old asymmetry.Not a UI change, so no screenshots.
🤖 Generated with Claude Code