Repository navigation
chore(skills): take SEP-2640 constants and schemas from the SDK, move the rest to core/extension/skills - #2808
Merged
Conversation
… the rest to core/extension/skills (#2804) MCP TypeScript SDK 2.4.0 ships the client side of the Skills extension. The wire constants, SKILL_LIMITS, the digest pattern, the caching fields and the resources/directory/read result schema now come from @modelcontextprotocol/core/ext/skills instead of being restated. SkillsClient itself stays out of the call path: list() and get() run the SDK's host-strict SkillSchema over every entry and throw on (or reduce to a one-line reason) exactly the non-conforming entries the conformance report exists to show, and read() throws on the first failed check. The permissive entry schemas, the per-check conformance and verification report, frontmatter parsing and capability detection stay ours, now in core/extension/skills/, which imports nothing from InspectorClient. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>
There was a problem hiding this comment.
🟢 Approval recommended
The SDK substitutions and module relocation preserve the inspection-specific contracts, with no unresolved blocking defects identified.
No open issues 🎉
What changed in this PR
This PR addresses #2804 by adopting SDK-owned Skills constants and compatible schemas while keeping Inspector-specific validation and reporting in core/extension/skills/.
Changes:
- Reuses SDK constants, limits, digest grammar, cache-field schemas, and directory-read schemas.
- Relocates inspection-specific helpers and introduces a narrow
SkillResourceReaderinterface for verification. - Updates client imports, test locations, and architecture documentation while retaining permissive inspection of invalid skills.
| File | Description |
|---|---|
docs/inspector-roadmap-2026-h2.md |
Updates the conformance-module reference. |
core/react/useManagedSkills.ts |
Updates the skill-type import. |
core/mcp/state/managedSkillsState.ts |
Imports relocated Skills schemas. |
core/mcp/serverList.ts |
Imports relocated catalog-budget helpers. |
core/mcp/remote/node/server.ts |
Updates catalog-limit validation import. |
core/mcp/node/config.ts |
Updates catalog-limit validation import. |
core/mcp/inspectorClientProtocol.ts |
Updates Skills interface type imports. |
core/mcp/inspectorClient.ts |
Uses relocated schemas and capability helpers. |
core/mcp/extensions.ts |
Imports the SDK-backed extension identifier. |
core/mcp/__tests__/fakeInspectorClient.ts |
Updates test-double type imports. |
core/extension/skills/verification.ts |
Relocates verification and narrows its client interface. |
core/extension/skills/skillFile.ts |
Relocates frontmatter parsing helpers. |
core/extension/skills/sha256.ts |
Relocates the HTTP-compatible hashing fallback. |
core/extension/skills/schemas.ts |
Adopts compatible SDK schemas and constants. |
core/extension/skills/conformance.ts |
Relocates checks and adopts SDK limits and digest grammar. |
core/cli/handlers/skills-verify.ts |
Updates verification-report imports. |
core/cli/handlers/run-method.ts |
Updates Skills dispatch and verification imports. |
core/cli/handlers/output-file.ts |
Updates the byte-decoding helper import. |
clients/web/src/test/integration/mcp/inspectorClient-skills.test.ts |
Points integration tests at relocated helpers. |
clients/web/src/test/core/react/useManagedSkills.test.tsx |
Updates the hook test’s skill-type import. |
clients/web/src/test/core/mcp/state/managedSkillsState.test.ts |
Updates the store test’s skill-type import. |
clients/web/src/test/core/mcp/inspectorClient-skills.test.ts |
Updates the extension-identifier import. |
clients/web/src/test/core/mcp/extensions.test.ts |
Updates capability-test imports. |
clients/web/src/test/core/extension/skills/verification.test.ts |
Relocates verification tests and updates imports. |
clients/web/src/test/core/extension/skills/skillFileBytes.test.ts |
Relocates byte-conversion tests. |
clients/web/src/test/core/extension/skills/skillFile.test.ts |
Relocates frontmatter tests. |
clients/web/src/test/core/extension/skills/sha256.test.ts |
Relocates hashing tests. |
clients/web/src/test/core/extension/skills/schemas.test.ts |
Relocates schema tests and asserts SDK reuse. |
clients/web/src/test/core/extension/skills/conformance.test.ts |
Relocates conformance tests. |
clients/web/src/hooks/useServerCommands.tsx |
Updates command-helper and result-type imports. |
clients/web/src/hooks/useInspectorStores.ts |
Updates the Skills store type import. |
clients/web/src/components/views/InspectorView/types.ts |
Updates Skills view-contract imports. |
clients/web/src/components/views/InspectorView/InspectorView.tsx |
Updates capability-detection import. |
clients/web/src/components/screens/SkillsScreen/SkillsScreen.tsx |
Uses relocated inspection helpers. |
clients/web/src/components/screens/SkillsScreen/SkillsScreen.test.tsx |
Updates screen-test imports. |
clients/web/src/components/screens/SkillsScreen/SkillsScreen.stories.tsx |
Updates story fixture imports. |
clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx |
Updates catalog-budget helper imports. |
clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.tsx |
Updates Skills capability import. |
clients/web/src/App.tsx |
Updates Skills capability import. |
clients/tui/src/components/SkillsTab.tsx |
Uses relocated Skills inspection modules. |
clients/tui/__tests__/SkillsTab.test.tsx |
Updates TUI test imports. |
clients/mcpdo/__tests__/skills-budget-hint.test.ts |
Updates the verification-report type import. |
clients/cli/__tests__/skills-verify-cli.test.ts |
Updates the verification-report type import. |
clients/cli/__tests__/run-method-skills.test.ts |
Updates CLI Skills test imports. |
AGENTS.md |
Documents the Skills extension directory. |
.claude/skills/project-structure/SKILL.md |
Documents SDK ownership and retained Inspector logic. |
Review effort: Balanced
Member
Author
|
Copilot review loop closed: round 1 was clean (no inline comments, no suppressed findings, approval recommended), so no further round was 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 #2804
What changed
MCP TypeScript SDK 2.4.0 ships the Skills extension's client (
@modelcontextprotocol/client/ext/skills) and schemas (@modelcontextprotocol/core/ext/skills). This PR compares that client with the Inspector's own code first, as the issue asks. Wherever the SDK can be used without changing what users see, its code replaces ours. Everything else moves tocore/extension/skills/, which imports nothing fromInspectorClient.Taken from the SDK (our copies deleted)
skills/list,skills/get,resources/directory/read,inode/directorycore/ext/skills. Same pattern ascore/extension/tasks/constants.ts.SKILL_LIMITSSKILL_DIGEST_PATTERNttlMs/cacheScopeon every modern list/get schemaresources/directory/readresult, both erasReadResourceDirectoryResultSchema(modern) and.partial({ ttlMs, cacheScope })(legacy). This is exactly howSkillsClientdecodes a 2025-11-25 result.Kept as ours, and why
SkillEntrySchema, list/get envelopes)SkillSchemais built for a host. Itsdigestmust match the pattern, itssizemust be an integer, and itsname/descriptionare required.list()throws on a non-conforming entry. WithskipInvalidit returns the entry only as{ index, uri, reason }, which loses the frontmatter and manifest the Skills screens display.get()throws too. Using either would turn every conformance finding into a page that fails to load.list/get/readDirectorycalls stay onclient.requestlist()andget(): the entry-loss problem above.readDirectory()always walks every page, but the CLI and web exposecursorpage by page. Only the directory schema was a clean swap.conformance.ts)"dynamic"resources stay a reported warning (#2405), not a silentverified: false. It also runs against entries the SDK would never hand back.verification.ts)SkillsClient.read()throwsSkillVerificationErrorat the first failed check and refuses an invalid entry before any request. The walk records each file's outcome, includingunverifiableand the catalog read-budget cut-off. It now takes a narrowSkillResourceReader(readResource+getServerSettings) rather thanInspectorClientProtocol.skillFile.ts,yaml)yamlparser reports those as findings instead.sha256.ts)crypto.subtleonly. Ours falls back to a pure implementation over plain HTTP on a LAN IP, wheresubtleis absent.getSkillsExtension)skillsCapabilityOfreturnsundefinedfor a declaration that doesn't parse (e.g.directoryRead: "yes"). That would hide the Skills tab for exactly the server an inspector is for.core/mcp/extensions.ts.Layout
core/mcp/{skills,skillsSchemas,skillsVerification,skillFile,sha256}.tsmove tocore/extension/skills/{conformance,schemas,verification,skillFile,sha256}.ts, and their tests move toclients/web/src/test/core/extension/skills/.core/extension/**was already in the web coverageinclude. AGENTS.md, theproject-structureskill and the roadmap doc are updated to the new paths.No user-facing change
inspectorClient-skills,skills-strict-configs).verify:bundle-externalspasses, so the newext/skillssubpath stays external in all four bundles.npm run local:gateis green.One small difference: a directory child now keeps unknown members instead of having them stripped. The SDK's
DirectoryResourceSchemaislooseObject, so the CLI prints the result closer to what the server actually sent.🤖 Generated with Claude Code