Repository navigation
Conversation
This was referenced Sep 4, 2026
sambhav
force-pushed
the
skills-sep-2640-fs-helper
branch
from
September 4, 2026 22:48
bd1e1fa to
e6d64a1
Compare
7 of 9 tasks
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>
… 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
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.
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
Use AddFS with embed.FS, fstest.MapFS, or another fs.FS. Construct a DirectoryProvider directly when explicit Refresh is needed.
Behavior preserved
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:
Zero failures or warnings; two legacy version-conditioned skips. The underlying protocol also passed standard server/client conformance (40/239 checks).