Skip to content

skills: add filesystem provider helper - #1240

Draft
sambhav wants to merge 20 commits into
modelcontextprotocol:mainfrom
sambhav:skills-sep-2640-fs-helper
Draft

sambhav wants to merge 20 commits into
modelcontextprotocol:mainfrom
sambhav:skills-sep-2640-fs-helper

Conversation

@sambhav

@sambhav sambhav commented Sep 4, 2026 •

Copy link
Copy Markdown
Member

Summary

Add an optional filesystem provider for SEP-2640: discovery, metadata, complete manifests, lazy resource reads, directory browsing, pagination, and configurable catalog refresh.

This remains PR 2 of 2, stacked on #1238 and in draft. The branch includes the reviewed protocol revision at 21ad3c2. The helper-only comparison now contains only seven filesystem code/test/documentation files. After #1238 merges, this PR can be rebased onto main.

Usage

server := mcp.NewServer(&mcp.Implementation{Name: "skill-server", Version: "v1"}, nil)
if err := skills.AddDirectory(server, "./skills", nil); err != nil {
    log.Fatal(err)
}

Use AddFS with embed.FS, fstest.MapFS, or another fs.FS. Construct a DirectoryProvider directly when explicit Refresh is needed.

Behavior preserved

  • Request-time skills/list, skills/get, resources/list, resources/read, and resources/directory/read discover additions, edits, removals, nested skills, supporting files, and empty directories.
  • SKILL.md resource and directory metadata carries frontmatter name/description and text/markdown.
  • Resource-list middleware merges all underlying pages with live filesystem metadata. Explicit resource registrations win URI collisions. It preserves parameters/results, propagates errors, rejects repeated cursors, and uses zero TTL/private scope.
  • Cache nil rebuilds on each request. A supplied cache can preload, retain indefinitely, expire by MaxAge, accept Invalidate signals, or refresh explicitly. Failed rebuilds keep the old catalog stale and retry.
  • File bytes are read on demand. Complete manifests remain static even when the catalog changes. Symlinks/non-regular files remain rejected; applications own watchers, mutable-filesystem synchronization, approvals, and shutdown.

Adaptation to #1238

Register the resource template before AddHandlers, satisfying its resources-capability check. Clients use the package-level session functions without AddClient/AddMethods or a wrapper.

Filesystem publication keeps the 512-file/16-MiB baseline by default. Explicit DirectoryOptions.ServerOptions.Limits can override publication caps; zero fields are unlimited. Limits/options are copied at construction, negative limits are rejected, and structural validation always runs. CatalogValidators adds application checks.

The stack update preserves both previous branch heads as ancestors; neither branch was force-pushed. No helper functionality moved into #1238.

Validation

Full Go tests, Skills race tests, and documentation generation passed. Vet/build passed with VCS stamping disabled for the linked worktree.

Reran conformance #330 at c4d79493f8013f18a7eb0bbe8aef584cff7b31aa using a temporary filesystem fixture with nested skills, supporting files, an empty directory, an ordinary registered resource, and page size 1:

Protocol / HTTP mode Enumeration Manifest Directory Total
2025-11-25 / stateful 30 6 7 43
2026-07-28 / stateless 32 6 7 45

Zero failures or warnings; two legacy version-conditioned skips. The underlying protocol also passed standard server/client conformance (40/239 checks).

@sambhav
sambhav force-pushed the skills-sep-2640-fs-helper branch from bd1e1fa to e6d64a1 Compare September 4, 2026 22:48
sambhav and others added 11 commits September 11, 2026 04:00
Replace the repeated result-type and cache-hint assignments in AddHandlers
with a single stampEnvelope helper that also validates the hints it settles
on, and route the protocol version, result type, and cache scope through
named constants instead of repeated literals.

paginate now skips the clone-and-sort when the catalog is already ordered,
and resolves a cursor with a binary search rather than a linear scan.
allPages allocates its seen set only once a server hands out a second
cursor, and rejects a cursor equal to the initial one.

parseSkillURI returns the parsed URL so validateResourceURI can reuse it
instead of reparsing the skill root for every resource.

mcp: factor the SDK default capabilities into defaultCapabilities so that
AddExtension and capabilities cannot drift apart.

conformance/skills-server: index skills by URI instead of rescanning the
entry slice, and use path.Base for display names.

scripts: factor the repeated argument check into require_value and simplify
the work-directory setup.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mcp/mrtr.go imports golang.org/x/sync/errgroup directly, so the "// indirect"
marking was stale and `go mod tidy -diff` reported a pending change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Group the package tests by what they exercise: validation_test.go holds the
pure unit tables, protocol_test.go the wire behavior, skills_test.go the
shared helpers and API contracts, and limits_test.go the manifest caps.

The limits matrix now calls ValidateSkillWithLimits directly instead of
standing up a streamable HTTP server per combination, taking that test from
24 connections to 4. TestLimitsArePlumbed keeps one end-to-end case per side
to prove Client.Limits and ServerOptions.Limits reach validation.

TestSpecErrorScenarios collects the cases that the sep-2640-skills-*
conformance scenarios also cover behind a single server, and carries a note
to delete it once those scenarios run in CI against ./conformance/skills-server.

Statement coverage rises from 82.2% to 90.8%.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
guglielmo-san and others added 3 commits September 23, 2026 13:40
… resource checks

Accept opt-in SkipInvalidSkills with diagnostics, require resource capabilities,
remove configurable client caps, and register custom methods automatically.
Update against main and remove unrelated conformance, documentation, and
x/sync classification changes from the protocol PR.
Preserve the stacked protocol dependency and live discovery, caching, resources,
and directory functionality. Adapt validation and publication limits, register
the resource template before Skills handlers, and remove obsolete client setup.

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.

2 participants