diff --git a/server/router/mcp/README.md b/server/router/mcp/README.md index d7fe91f9..b958412a 100644 --- a/server/router/mcp/README.md +++ b/server/router/mcp/README.md @@ -42,7 +42,11 @@ fast on any inconsistency: IDs or duplicate tool names are construction errors. 4. Each tool is registered with `server.AddTool(tool, newMCPToolHandler(...))`. 5. `sdkmcp.NewStreamableHTTPHandler` wraps the server in stateless, - JSON-response mode (no SSE, no session tracking). + JSON-response mode (no SSE, no session tracking). Stateless mode is also + what lets the SDK serve protocol version `2026-07-28`; older clients still + negotiate `2025-11-25` and earlier through the legacy `initialize` handshake. + The transport body limit is set to the API-wide limit so attachment uploads + are not cut off by the SDK's 4 MiB default. ## Request flow @@ -105,6 +109,14 @@ response has no JSON body, the fallback is: - **Endpoint:** `POST /mcp` (the SDK may also use `GET`/`DELETE` on the same path for the Streamable HTTP transport). - **Transport:** Streamable HTTP, **stateless**, JSON responses. +- **Protocol versions:** `2026-07-28` down to `2024-11-05`. Clients on + `2026-07-28` skip `initialize`, call `server/discover`, carry + `_meta.io.modelcontextprotocol/protocolVersion` on every request, and must + send the `Mcp-Protocol-Version`, `Mcp-Method`, and (for `tools/call`) + `Mcp-Name` headers. +- **Capabilities:** tools only, without `listChanged` — the catalog is fixed at + startup, so `tools/list` and `server/discover` advertise a 24-hour `ttlMs` + instead of change notifications. - **Request size:** request bodies are limited to 256 MiB before SDK dispatch. - **Auth:** the caller's `Authorization: Bearer ` header is forwarded to the in-process API request. Mutating tools therefore require a valid token @@ -261,7 +273,9 @@ go test ./server/router/mcp/... - `catalog_test.go` — tool selection, naming, schema and annotation building. - `adapter_test.go` — request construction and in-process execution (`adapter.go`), plus result normalization and error shaping (`result.go`). - `validation_test.go` — argument validation against input schemas. -- `service_test.go` — the origin-header check, plus the end-to-end MCP protocol +- `service_test.go` — the origin-header check, the stateless `2026-07-28` + flow (`server/discover`, `tools/list`, `tools/call` with MCP headers), the + request body limit, plus the legacy end-to-end MCP protocol (`initialize`, `tools/list`, `tools/call`) confirming object-shaped `structuredContent`. diff --git a/server/router/mcp/catalog.go b/server/router/mcp/catalog.go index bd9eab3e..6170b1fd 100644 --- a/server/router/mcp/catalog.go +++ b/server/router/mcp/catalog.go @@ -199,7 +199,7 @@ func inputSchemaForOperation(operation *openAPIOperation) jsonSchema { required := []string{} defs := map[string]any{} for _, parameter := range operation.Parameters { - schema := cloneSchema(parameter.Schema) + schema := jsonSchema(sanitizeSchemaValue(parameter.Schema).(map[string]any)) if parameter.Description != "" { schema["description"] = parameter.Description } diff --git a/server/router/mcp/catalog_test.go b/server/router/mcp/catalog_test.go index 5a02bc08..09c346f3 100644 --- a/server/router/mcp/catalog_test.go +++ b/server/router/mcp/catalog_test.go @@ -2,6 +2,8 @@ package mcp import ( "encoding/json" + "strconv" + "strings" "testing" sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" @@ -387,3 +389,56 @@ func TestBuildCuratedToolsRejectsDuplicateToolNames(t *testing.T) { _, _, err := buildCuratedTools(registry) require.ErrorContains(t, err, "duplicate MCP tool name") } + +func TestBuildCuratedToolsUseStandardSchemaFormats(t *testing.T) { + spec, err := loadOpenAPISpec("../../../proto/gen/openapi.yaml") + require.NoError(t, err) + registry, err := buildOperationRegistry(spec) + require.NoError(t, err) + + tools, _, err := buildCuratedTools(registry) + require.NoError(t, err) + + // Formats registered by JSON Schema or ajv-formats; anything else trips + // strict clients (see https://github.com/usememos/memos/issues/6262). + allowedFormats := map[string]struct{}{ + "date-time": {}, + "int32": {}, + "int64": {}, + "float": {}, + "double": {}, + } + var collectFormats func(t *testing.T, tool string, path string, value any) + collectFormats = func(t *testing.T, tool string, path string, value any) { + switch typed := value.(type) { + case map[string]any: + if format, ok := typed["format"]; ok { + formatName, isString := format.(string) + require.True(t, isString, "%s: %s has non-string format %v", tool, path, format) + require.Contains(t, allowedFormats, formatName, "%s: %s uses non-standard format %q", tool, path, formatName) + } + for key, item := range typed { + collectFormats(t, tool, path+"/"+key, item) + } + case []any: + for index, item := range typed { + collectFormats(t, tool, path+"/"+strconv.Itoa(index), item) + } + } + } + + sawBase64Content := false + for _, tool := range tools { + for label, schema := range map[string]any{"inputSchema": tool.InputSchema, "outputSchema": tool.OutputSchema} { + encoded, err := json.Marshal(schema) + require.NoError(t, err) + var decoded any + require.NoError(t, json.Unmarshal(encoded, &decoded)) + collectFormats(t, tool.Name, label, decoded) + if strings.Contains(string(encoded), `"contentEncoding":"base64"`) { + sawBase64Content = true + } + } + } + require.True(t, sawBase64Content, "expected attachment content to advertise contentEncoding base64") +} diff --git a/server/router/mcp/openapi.go b/server/router/mcp/openapi.go index 9d388efe..ca8ac7da 100644 --- a/server/router/mcp/openapi.go +++ b/server/router/mcp/openapi.go @@ -186,9 +186,55 @@ func resolveSchemaMap(spec *openAPISpec, schema map[string]any, defs map[string] } resolved[key] = resolvedValue } + normalizeSchemaFormat(resolved) return resolved, nil } +// normalizeSchemaFormat rewrites the OpenAPI-only "format" markers emitted by +// the gnostic generator into standard JSON Schema so MCP clients can consume +// tool schemas without warnings or strict-mode failures. +// +// - "enum" duplicates the "enum" keyword and is dropped. +// - "bytes" becomes "contentEncoding: base64", matching the protojson wire encoding. +// - "field-mask" has no JSON Schema equivalent and is dropped. +func normalizeSchemaFormat(schema map[string]any) { + format, ok := schema["format"].(string) + if !ok { + return + } + switch format { + case "enum", "field-mask": + delete(schema, "format") + case "bytes": + delete(schema, "format") + schema["contentEncoding"] = "base64" + } +} + +// sanitizeSchemaValue deep-copies a schema value that did not pass through the +// component resolver (such as parameter schemas) and normalizes its formats. +func sanitizeSchemaValue(value any) any { + switch typed := value.(type) { + case jsonSchema: + return sanitizeSchemaValue(map[string]any(typed)) + case map[string]any: + sanitized := make(map[string]any, len(typed)) + for key, item := range typed { + sanitized[key] = sanitizeSchemaValue(item) + } + normalizeSchemaFormat(sanitized) + return sanitized + case []any: + sanitized := make([]any, 0, len(typed)) + for _, item := range typed { + sanitized = append(sanitized, sanitizeSchemaValue(item)) + } + return sanitized + default: + return value + } +} + func resolveComponentSchema(spec *openAPISpec, name string, defs map[string]any, resolving map[string]bool) (map[string]any, error) { component, ok := spec.Components.Schemas[name] if !ok { diff --git a/server/router/mcp/openapi_test.go b/server/router/mcp/openapi_test.go index c4aef2c6..19414766 100644 --- a/server/router/mcp/openapi_test.go +++ b/server/router/mcp/openapi_test.go @@ -190,3 +190,49 @@ func TestBuildOperationRegistryUsesOKSchemaForEmptySuccessResponse(t *testing.T) require.Equal(t, "object", responseSchema["type"]) require.Contains(t, responseSchema["properties"], "ok") } + +func TestResolveSchemaRefNormalizesNonStandardFormats(t *testing.T) { + spec := &openAPISpec{ + Components: openAPIComponents{ + Schemas: map[string]jsonSchema{ + "Attachment": { + "type": "object", + "properties": map[string]any{ + "state": map[string]any{"type": "string", "format": "enum", "enum": []any{"NORMAL", "ARCHIVED"}}, + "content": map[string]any{"type": "string", "format": "bytes"}, + "updateMask": map[string]any{"type": "string", "format": "field-mask"}, + "createTime": map[string]any{"type": "string", "format": "date-time"}, + "tags": map[string]any{"type": "array", "items": map[string]any{"type": "string", "format": "enum"}}, + }, + }, + }, + }, + } + + schema, err := resolveSchemaRef(spec, jsonSchema{"$ref": "#/components/schemas/Attachment"}) + require.NoError(t, err) + properties := schema["properties"].(map[string]any) + + state := properties["state"].(map[string]any) + require.NotContains(t, state, "format") + require.Equal(t, []any{"NORMAL", "ARCHIVED"}, state["enum"]) + + content := properties["content"].(map[string]any) + require.NotContains(t, content, "format") + require.Equal(t, "base64", content["contentEncoding"]) + + require.NotContains(t, properties["updateMask"].(map[string]any), "format") + require.Equal(t, "date-time", properties["createTime"].(map[string]any)["format"]) + require.NotContains(t, properties["tags"].(map[string]any)["items"].(map[string]any), "format") +} + +func TestSanitizeSchemaValueDoesNotMutateInput(t *testing.T) { + original := jsonSchema{"type": "string", "format": "enum", "items": map[string]any{"format": "bytes"}} + + sanitized := sanitizeSchemaValue(original).(map[string]any) + require.NotContains(t, sanitized, "format") + require.Equal(t, "base64", sanitized["items"].(map[string]any)["contentEncoding"]) + + require.Equal(t, "enum", original["format"]) + require.Equal(t, "bytes", original["items"].(map[string]any)["format"]) +} diff --git a/server/router/mcp/service.go b/server/router/mcp/service.go index 4d736476..554c6102 100644 --- a/server/router/mcp/service.go +++ b/server/router/mcp/service.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "net/http" + "time" "github.com/labstack/echo/v5" "github.com/labstack/echo/v5/middleware" @@ -17,9 +18,16 @@ import ( ) // maxMCPRequestBytes caps the /mcp request body. It tracks the API limit because -// every tool call is forwarded in-process through the API routes. +// every tool call is forwarded in-process through the API routes. The SDK +// enforces its own 4 MiB default, so the value is passed to the transport as +// well; otherwise attachment uploads over MCP fail with 413 before Echo sees them. const maxMCPRequestBytes int64 = apiv1.MaxAPIRequestBytes +// toolCatalogTTL is the freshness hint advertised on tools/list and +// server/discover results. The catalog is fixed at startup, so clients may cache +// it for a long time instead of re-listing on every session. +const toolCatalogTTL = 24 * time.Hour + // MCPService serves the OpenAPI-driven MCP endpoint. type MCPService struct { profile *profile.Profile @@ -50,7 +58,16 @@ func NewMCPService(profile *profile.Profile, echoServer *echo.Echo) (*MCPService server := sdkmcp.NewServer(&sdkmcp.Implementation{ Name: "memos", Version: version, - }, nil) + }, &sdkmcp.ServerOptions{ + // The tool catalog never changes after startup, so do not advertise + // listChanged: new-protocol clients would otherwise hold a + // subscriptions/listen stream open for notifications that never come. + // Leaving Logging unset also drops the deprecated logging capability. + Capabilities: &sdkmcp.ServerCapabilities{ + Tools: &sdkmcp.ToolCapabilities{}, + }, + }) + server.AddReceivingMiddleware(catalogCacheMiddleware) adapter := newAPIAdapter(echoServer) for _, tool := range tools { @@ -61,8 +78,11 @@ func NewMCPService(profile *profile.Profile, echoServer *echo.Echo) (*MCPService streamableHandler := sdkmcp.NewStreamableHTTPHandler(func(*http.Request) *sdkmcp.Server { return server }, &sdkmcp.StreamableHTTPOptions{ - Stateless: true, - JSONResponse: true, + // Stateless is also what lets the SDK serve protocol version + // 2026-07-28; stateful handlers negotiate down to 2025-11-25. + Stateless: true, + JSONResponse: true, + MaxRequestBodyBytes: maxMCPRequestBytes, // memos is typically served behind a reverse proxy with the app bound to a // loopback address while the public Host header is a real domain. The SDK's // DNS-rebinding guard treats that shape as an attack and rejects every @@ -78,6 +98,24 @@ func NewMCPService(profile *profile.Profile, echoServer *echo.Echo) (*MCPService }, nil } +// catalogCacheMiddleware stamps the static-catalog TTL onto results the SDK +// would otherwise mark immediately stale (ttlMs: 0). +func catalogCacheMiddleware(next sdkmcp.MethodHandler) sdkmcp.MethodHandler { + return func(ctx context.Context, method string, request sdkmcp.Request) (sdkmcp.Result, error) { + result, err := next(ctx, method, request) + if err != nil { + return result, err + } + switch typed := result.(type) { + case *sdkmcp.ListToolsResult: + typed.TTLMs = int(toolCatalogTTL.Milliseconds()) + case *sdkmcp.DiscoverResult: + typed.TTLMs = int(toolCatalogTTL.Milliseconds()) + } + return result, nil + } +} + func loadMCPServiceOpenAPISpec() (*openAPISpec, error) { spec := &openAPISpec{} if err := yaml.Unmarshal(memosproto.OpenAPIYAML(), spec); err != nil { diff --git a/server/router/mcp/service_test.go b/server/router/mcp/service_test.go index 7531b999..249a429b 100644 --- a/server/router/mcp/service_test.go +++ b/server/router/mcp/service_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/http/httptest" "os" + "strings" "testing" "github.com/labstack/echo/v5" @@ -473,3 +474,157 @@ func postMCP(t *testing.T, echoServer *echo.Echo, payload map[string]any) map[st require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &response)) return response } + +// TestMCPStatelessProtocol2026 drives the endpoint the way a 2026-07-28 client +// does: no initialize handshake, per-request _meta, and the standard MCP headers. +func TestMCPStatelessProtocol2026(t *testing.T) { + echoServer := echo.New() + var forwardedAuthorization string + echoServer.GET("/api/v1/memos", func(c *echo.Context) error { + forwardedAuthorization = c.Request().Header.Get("Authorization") + return c.JSON(http.StatusOK, map[string]any{"memos": []any{}}) + }) + service, err := NewMCPService(&profile.Profile{Version: "test-version"}, echoServer) + require.NoError(t, err) + service.RegisterRoutes(echoServer) + + meta := map[string]any{ + "io.modelcontextprotocol/protocolVersion": "2026-07-28", + "io.modelcontextprotocol/clientCapabilities": map[string]any{}, + "io.modelcontextprotocol/clientInfo": map[string]any{"name": "memos-test", "version": "1.0.0"}, + } + + discover := postMCPWithHeaders(t, echoServer, map[string]any{ + "jsonrpc": "2.0", + "id": 1, + "method": "server/discover", + "params": map[string]any{"_meta": meta}, + }, map[string]string{"Mcp-Method": "server/discover"}) + discoverResult, ok := discover["result"].(map[string]any) + require.True(t, ok, discover) + require.Contains(t, discoverResult["supportedVersions"], "2026-07-28") + require.Equal(t, "complete", discoverResult["resultType"]) + capabilities, ok := discoverResult["capabilities"].(map[string]any) + require.True(t, ok) + require.Equal(t, map[string]any{}, capabilities["tools"], "tools capability must not advertise listChanged") + require.NotContains(t, capabilities, "logging", "logging is deprecated and unused") + require.Positive(t, discoverResult["ttlMs"]) + + list := postMCPWithHeaders(t, echoServer, map[string]any{ + "jsonrpc": "2.0", + "id": 2, + "method": "tools/list", + "params": map[string]any{"_meta": meta}, + }, map[string]string{"Mcp-Method": "tools/list"}) + listResult, ok := list["result"].(map[string]any) + require.True(t, ok, list) + require.Len(t, listResult["tools"], len(curatedOperationIDs)) + require.Equal(t, float64(toolCatalogTTL.Milliseconds()), listResult["ttlMs"]) + require.Equal(t, "public", listResult["cacheScope"]) + + call := postMCPWithHeaders(t, echoServer, map[string]any{ + "jsonrpc": "2.0", + "id": 3, + "method": "tools/call", + "params": map[string]any{ + "_meta": meta, + "name": "memo_list_memos", + "arguments": map[string]any{}, + }, + }, map[string]string{ + "Mcp-Method": "tools/call", + "Mcp-Name": "memo_list_memos", + "Authorization": "Bearer test-token", + }) + callResult, ok := call["result"].(map[string]any) + require.True(t, ok, call) + require.Equal(t, "complete", callResult["resultType"]) + require.Equal(t, map[string]any{"memos": []any{}}, callResult["structuredContent"]) + require.Equal(t, "Bearer test-token", forwardedAuthorization) +} + +// TestMCPStatelessRejectsSessionMethods confirms the stateless transport shape +// required by 2026-07-28: no GET stream, no DELETE session teardown. +func TestMCPStatelessRejectsSessionMethods(t *testing.T) { + echoServer := echo.New() + service, err := NewMCPService(&profile.Profile{Version: "test-version"}, echoServer) + require.NoError(t, err) + service.RegisterRoutes(echoServer) + + for _, method := range []string{http.MethodGet, http.MethodDelete} { + request := httptest.NewRequest(method, "/mcp", nil) + request.Header.Set("Accept", "application/json, text/event-stream") + recorder := httptest.NewRecorder() + echoServer.ServeHTTP(recorder, request) + require.Equal(t, http.StatusMethodNotAllowed, recorder.Code, method) + } +} + +// TestMCPRequestBodyLimitMatchesAPI guards against the SDK's 4 MiB default +// overriding the API-wide limit, which would break attachment uploads over MCP. +func TestMCPRequestBodyLimitMatchesAPI(t *testing.T) { + echoServer := echo.New() + var receivedBytes int + echoServer.POST("/api/v1/attachments", func(c *echo.Context) error { + body := map[string]any{} + require.NoError(t, json.NewDecoder(c.Request().Body).Decode(&body)) + content, _ := body["content"].(string) + receivedBytes = len(content) + return c.JSON(http.StatusOK, map[string]any{"name": "attachments/1"}) + }) + service, err := NewMCPService(&profile.Profile{Version: "test-version"}, echoServer) + require.NoError(t, err) + service.RegisterRoutes(echoServer) + + content := strings.Repeat("A", 8<<20) + response := postMCPWithHeaders(t, echoServer, map[string]any{ + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": map[string]any{ + "name": "attachment_create_attachment", + "arguments": map[string]any{ + "body": map[string]any{ + "filename": "large.bin", + "type": "application/octet-stream", + "content": content, + }, + }, + }, + }, nil) + require.Contains(t, response, "result", response) + require.Equal(t, len(content), receivedBytes) + + // Declare an over-limit Content-Length instead of allocating the body. + request := httptest.NewRequest(http.MethodPost, "/mcp", strings.NewReader("{}")) + request.ContentLength = maxMCPRequestBytes + 1 + request.Header.Set("Content-Type", "application/json") + request.Header.Set("Accept", "application/json, text/event-stream") + recorder := httptest.NewRecorder() + echoServer.ServeHTTP(recorder, request) + require.Equal(t, http.StatusRequestEntityTooLarge, recorder.Code) +} + +func postMCPWithHeaders(t *testing.T, echoServer *echo.Echo, payload map[string]any, headers map[string]string) map[string]any { + t.Helper() + data, err := json.Marshal(payload) + require.NoError(t, err) + + request := httptest.NewRequest(http.MethodPost, "/mcp", bytes.NewReader(data)) + request.Header.Set("Content-Type", "application/json") + request.Header.Set("Accept", "application/json, text/event-stream") + if _, hasMeta := payload["params"].(map[string]any)["_meta"]; hasMeta { + request.Header.Set("Mcp-Protocol-Version", "2026-07-28") + } + for key, value := range headers { + request.Header.Set(key, value) + } + + recorder := httptest.NewRecorder() + echoServer.ServeHTTP(recorder, request) + require.Equal(t, http.StatusOK, recorder.Code, recorder.Body.String()) + + var response map[string]any + require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &response)) + return response +}