[reyortiz3]

Summary

• Why: vMCP needs to serve the Skills extension (SEP-2640) by proxying its methods to a designated backend. vMCP's legacy (initialize/session) path is built on mcpcompat, which could neither declare the extension, serve its methods, nor call them on a backend. go-sdk v1.8.0 supports extensions and custom methods; the shim did not expose them.
• What: typed support for all of SEP-2640, following the shim's existing pattern for completion:
  • mcp/skills.go models the extension's messages from the SEP: ListSkillsRequest/Result, GetSkillRequest/Params/Result, ReadDirectoryRequest/Params/Result, Skill, SkillResource, and SkillResources (the spec's "file list or \"dynamic\"" union), plus constants for the extension name, the three methods, the directoryRead setting and the inode/directory MIME type.
  • server.WithSkillsHandlers(list, get) declares io.modelcontextprotocol/skills and serves the two required methods, skills/list and skills/get, mirroring WithCompletionHandler. Both handlers are required; a missing one fails the server build.
  • server.WithDirectoryReadHandler(h) serves the optional resources/directory/read and declares directoryRead: true. It requires WithSkillsHandlers, since the setting lives in that extension's declaration; otherwise the build fails. Without it the extension is declared as {} (no optional features).
  • The handlers run through the same session binding, request-context bridging and call gate as the standard methods, so they can authorize by caller identity, and handler panics are recovered. The stdio HandleMessage bridge serves all three.
  • client.ListSkills, client.GetSkill and client.ReadDirectory, mirroring Complete, including the resume path.

Type of change

• Bug fix
• New feature
• Refactoring
• Documentation
• Other

Test plan

• task (lint + full test suite with -race) passes
• task license-check passes
• New tests: end-to-end over Streamable HTTP (extension and directoryRead declared, all three methods round-trip, the handler sees the request context and cursor, unknown skills and non-directories reach the client as -32602, misconfigurations fail the build), the stdio bridge, the client resume path, and JSON tests for SkillResources and the SEP's skills/get and resources/directory/read examples

Special notes for reviewers

• Typed rather than generic. An earlier revision exposed generic WithExtension/WithCustomMethod/CallCustomMethod passing raw JSON through. It needed a raw-JSON package and _meta merge logic, so it was replaced by modeling the one extension needed.
• Duplicated wire structs. go-sdk sends and serves custom methods only with types embedding its sealed ParamsBase/ResultBase, so the server and client packages each declare small unexported wire structs and convert by JSON round-trip, as for every other method; one generic adapter per package does the conversion. Sharing the structs would need a new package or a go-sdk import in mcp. Once a go-sdk release includes its own skills package (go-sdk#1238, open), these can be swapped for its types without changing the shim's public API.
• Known go-sdk v1.8.0 limitations, documented on WithSkillsHandlers:
  • A session rehydrated on a replica other than the one that initialized it answers the extension's methods with HTTP 400. The Streamable HTTP transport checks a request's method against the server's custom methods only when go-sdk's own handler connected the session (connectStreamable sets the unexported connection.server); the shim's rehydration uses the public Server.Connect. This cannot be worked around in the shim; TestSkills_ResumedSession pins it so it fails once go-sdk is fixed. go-sdk#1238 does not change this.
  • On a 2026-07-28 session the results carry neither resultType nor ttlMs/cacheScope, which go-sdk stamps only on its own result types.
• A go-sdk server answers an unserved method with HTTP 400 rather than JSON-RPC -32601, so client callers should check the declared extension (and directoryRead) first. This is in the methods' doc comments.

🤖 Generated with Claude Code

[jhrozek]

Review focused on spec cleanliness and regression / behaviour-change potential. Existing users should see no change: everything server-side is gated on skillsDeclared, and the only unconditional change is registerSkillsMethods on client Initialize. Two spec-conformance issues (capability advertisement on the HandleMessage path, and null for empty lists) should be fixed before merge; the rest are low-severity consistency points. go test -race ./mcpcompat/... passes locally.

Also, a minor note: Skill.Frontmatter is documented as verbatim, but it goes through jsonConvert as map[string]any, so integers above 2^53 lose precision and key order is not preserved. Consider softening the comment, or using json.RawMessage if proxies need exact pass-through.

[reyortiz3]

Thanks for the review @jhrozek. All points are addressed in b22605c; replies are on each thread. On the frontmatter note from the review body: Skill.Frontmatter is now a SkillFrontmatter type that decodes numbers as json.Number, as go-sdk#1238 does, so integers above 2^53 round-trip exactly (TestSkillFrontmatter_KeepsNumberPrecision). I reworded the doc comment to say content and precision are kept but key order is not, which the extension's "identical in content" rule does not require. Plain map[string]any literals still assign to it.

[jhrozek]

Re-checked against b22605c: all findings from the earlier review are addressed (bridge capability advertisement and validateSkills gating, null vs [] for empty lists, nil-result/panic parity between paths, Frontmatter precision/comment). go test -race ./mcpcompat/... passes. LGTM.