Skip to content

Add SkillCatalog and FilesystemSkillProvider for serving skills from a directory - #3795

Open
vijaydeepsinha wants to merge 20 commits into
modelcontextprotocol:mainfrom
vijaydeepsinha:issue-3683-file-system-support
Open

vijaydeepsinha wants to merge 20 commits into
modelcontextprotocol:mainfrom
vijaydeepsinha:issue-3683-file-system-support

Conversation

@vijaydeepsinha

@vijaydeepsinha vijaydeepsinha commented Oct 31, 2026 •

Copy link
Copy Markdown
Member

Fixes #3683

Adds an optional helper that serves a directory of skills through the existing Skills extension, without changing the protocol or the Skills API. mcp/server/skills.py becomes a package; from mcp.server.skills import Skills and the handler aliases still work.

Motivation and Context

Serving skills from disk currently means writing handlers and registering every file as a resource by hand. This adds:

  • SkillProvider: a small protocol for listing and reading files.
  • FilesystemSkillProvider: a local-directory provider that skips symlinks and junctions and runs blocking I/O in a worker thread.
  • SkillCatalog: a Skills extension that scans a provider, validates each skill, and answers skills/list, skills/get and optionally resources/directory/read. Manifest digests always match what resources/read serves, and a file that changed after the last scan is refused.
  • SkillCatalog.reload(): rescans the provider and swaps the new catalog in all at once, returning the skills added, removed and changed. If the scan fails, the previous catalog keeps serving; concurrent calls run one after another.
  • auto_refresh=True (off by default) with refresh_interval (seconds, default 60): reloads on a timer while the server runs, serving the bytes it scanned so manifests and content stay consistent between refreshes. A failed refresh is logged and the previous catalog keeps serving.

Optional pagination (page_size) and SEP-2549 cache hints (ttl_ms, cache_scope) are supported.

To let a catalog change while the server runs, Extension gains two optional hooks, both off by default:

  • resources_are_dynamic = True: the server asks resources() again on each listing and lookup instead of copying it once. Returning the same tuple until something changes lets lookups reuse a URI index. Resources registered directly on the server still win URI collisions.
  • lifespan(): an async context manager entered inside the application's lifespan, used for the refresh timer.

Server usage

from pathlib import Path

import anyio
import yaml

from mcp.server.mcpserver import MCPServer
from mcp.server.skills import FilesystemSkillProvider, SkillCatalog


async def main() -> None:
    catalog = await SkillCatalog.load(
        FilesystemSkillProvider(Path("skills")),
        parse_frontmatter=yaml.safe_load,
        directory_read=True,
        auto_refresh=True,
        refresh_interval=30,
    )
    server = MCPServer("my-server", extensions=[catalog])
    await server.run_stdio_async()


anyio.run(main)

How Has This Been Tested?

All run locally on macOS

  • ./scripts/test, twice: 6,377 passed, 100.00% statement and branch coverage, strict-no-cover clean.
  • Python 3.11, 3.12, 3.14 and 3.15 (3.15.0rc3) locked environments, same command: all passed at 100.00%.
  • lowest-direct resolution on 3.11, and MCP_EXAMPLES_SMOKE=1 on 3.12: all passed.
  • ruff format, ruff check, pyright, pre-commit on the changed files, README snippet check, gen_surface_types.py --check, the standalone mcp-types import check, and the Node workflow tests: all clean.
  • scripts/docs/build.sh: every language built with 0 warnings.
  • Manually through Postman against real servers on both the legacy 2025-11-25 and the 2026-07-28 protocol versions: list/get/read, pagination, directory reads enabled and disabled, and auto-refresh picking up skills added and removed on disk without a restart.

Conformance

CI's pinned harness (@modelcontextprotocol/conformance@0.2.0-alpha.11), all suites, run exactly as conformance.yml does:

Leg Server Client
2025-11-25 84 passed, 0 failed 224 passed, 0 failed
2026-07-28 151 passed, 0 failed 387 passed, 0 failed
default 204 passed, 25 expected failures 464 passed, 9 expected failures

The baseline check passes on every leg.

The pinned harness has no SEP-2640 scenarios, so I also ran them from the conformance repository (modelcontextprotocol/conformance#330), against servers built with SkillCatalog + FilesystemSkillProvider, one with auto_refresh=True and one with page_size=2:

Scenario 2026-07-28 2025-11-25 (--force)
sep-2640-skills-enumeration ✅ 32/32 ✅ 30/30 (the two cache-attribute checks are 2026-07-28 only)
sep-2640-skills-manifest ✅ 6/6 ✅ 6/6
sep-2640-skills-directory ✅ 7/7 ✅ 7/7

The four sep-2640-client-* scenarios pass 1/1 each.

Breaking Changes

None. Existing public signatures and defaults are unchanged; the new SkillCatalog options and the Extension hooks are optional and off by default.

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Documentation update

Checklist

  • I am assigned to the linked issue
  • I have disclosed any AI assistance and can explain the change in my own words
  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

  • The SDK has no YAML dependency; the examples use PyYAML.
  • Reloading does not notify connected clients; they see changes on their next request. Sending resources/list_changed after a reload could be a follow-up if wanted.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 13 files

Reply to a comment to ask cubic a question or push back. It learns from your replies.

Re-trigger cubic

Comment thread src/mcp/server/skills/provider.py Resolved
Comment thread src/mcp/server/skills/filesystem.py Resolved
Comment thread src/mcp/server/skills/catalog.py Resolved
Comment thread docs/advanced/skills.md Resolved
Comment thread docs/advanced/skills.md Resolved

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

4 issues found across 13 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/server/skills/test_filesystem.py">

<violation number="1" location="tests/server/skills/test_filesystem.py:327">
P2: This fixture is a regular file, so it only tests the mocked `fstat` rejection and cannot catch removal of `O_NONBLOCK`, which would make opening a real FIFO block. Keep a POSIX FIFO test alongside this portable descriptor test.</violation>
</file>

<file name="docs/advanced/skills.md">

<violation number="1" location="docs/advanced/skills.md:258">
P2: This paragraph presents SSE as a supported transport for the refresh timer, but the transport docs call SSE superseded. Remove the SSE-specific transport guidance rather than adding new usage documentation for it.

(Based on your team's feedback about avoiding new SSE usage documentation.)</violation>
</file>

<file name="docs/advanced/extensions.md">

<violation number="1" location="docs/advanced/extensions.md:139">
P3: `resources()` is not called for every lookup: direct server resources win immediately, and an earlier dynamic provider can satisfy the URI before this extension runs. Qualify this as lookups that reach the provider.</violation>
</file>

<file name="src/mcp/server/mcpserver/resources/resource_manager.py">

<violation number="1" location="src/mcp/server/mcpserver/resources/resource_manager.py:38">
P2: This cache keeps the old URI key when a provider reuses its tuple but mutates a contained `Resource.uri`, so lookups by the new URI fail and the old URI still resolves. Track the URIs used to build the index and rebuild it when any contained URI changes.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread tests/server/skills/test_filesystem.py Resolved
Comment thread docs/advanced/skills.md Resolved

def by_uri(self) -> Mapping[str, Resource]:
resources = self._provider()
if resources is self._indexed:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: This cache keeps the old URI key when a provider reuses its tuple but mutates a contained Resource.uri, so lookups by the new URI fail and the old URI still resolves. Track the URIs used to build the index and rebuild it when any contained URI changes.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/mcp/server/mcpserver/resources/resource_manager.py, line 38:

<comment>This cache keeps the old URI key when a provider reuses its tuple but mutates a contained `Resource.uri`, so lookups by the new URI fail and the old URI still resolves. Track the URIs used to build the index and rebuild it when any contained URI changes.</comment>

<file context>
@@ -25,12 +25,34 @@
+
+    def by_uri(self) -> Mapping[str, Resource]:
+        resources = self._provider()
+        if resources is self._indexed:
+            return self._by_uri
+        by_uri: dict[str, Resource] = {}
</file context>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Valid point, but it can only happen if a custom extension deliberately changes a resource's uri in place inside a tuple it keeps returning. It can't happen with refresh: every reload builds a new tuple of new resources, so the index is rebuilt. I've documented the rule in add_resource_provider and the extensions guide (return a new tuple instead of mutating) and added a unit test for this case as well.

Comment thread docs/advanced/extensions.md Resolved

This branch has not been deployed

No deployments
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.

Add a filesystem backed provider for the Skills extension

1 participant