fix(mcp): accept canonical resource names in path params and keep error results schema-valid (#6108)

server/router/mcp/adapter.go

@@ -102,11 +102,36 @@ func substitutePathParameters(operation *openAPIOperation, arguments map[string]
 		if !ok || value == nil || valueToString(value) == "" {
 			return "", errors.Errorf(`missing required path parameter "%s"`, parameter.Name)
 		}
-		path = strings.ReplaceAll(path, "{"+parameter.Name+"}", url.PathEscape(valueToString(value)))
+		id := trimResourceNamePrefix(operation.Path, parameter.Name, valueToString(value))
+		path = strings.ReplaceAll(path, "{"+parameter.Name+"}", url.PathEscape(id))
 	}
 	return path, nil
 }
 
+// trimResourceNamePrefix accepts canonical resource names for path parameters.
+// The API returns names like "memos/abc123", but the REST paths take the bare
+// ID ("/api/v1/memos/{memo}"), so clients that round-trip a returned name
+// would otherwise request "/api/v1/memos/memos/abc123" and get a 404. When the
+// placeholder directly follows its collection segment and the value carries
+// that collection prefix, strip the prefix; bare IDs pass through unchanged.
+func trimResourceNamePrefix(path, parameterName, value string) string {
+	placeholder := "/{" + parameterName + "}"
+	index := strings.Index(path, placeholder)
+	if index < 0 {
+		return value
+	}
+	head := path[:index]
+	collection := head[strings.LastIndex(head, "/")+1:]
+	if collection == "" {
+		return value
+	}
+	id, ok := strings.CutPrefix(value, collection+"/")
+	if !ok || id == "" || strings.Contains(id, "/") {
+		return value
+	}
+	return id
+}
+
 func valueToString(value any) string {
 	switch typed := value.(type) {
 	case string:

server/router/mcp/adapter_test.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"io"
 	"net/http"
+	"strings"
 	"testing"
 
 	"github.com/labstack/echo/v5"
@@ -47,11 +48,10 @@ func TestNewStructuredToolResultUsesObjectStructuredContent(t *testing.T) {
 func TestNewToolErrorResult(t *testing.T) {
 	result := newToolErrorResult("resource not found")
 	require.True(t, result.IsError)
-	require.Equal(t, map[string]any{
-		"error": map[string]any{
-			"message": "resource not found",
-		},
-	}, result.StructuredContent)
+	// Error results carry no structuredContent: tools declare an outputSchema
+	// for their success payload, and strict clients validate structuredContent
+	// against it — an error object would fail validation and mask the message.
+	require.Nil(t, result.StructuredContent)
 	require.NotEmpty(t, result.Content)
 	text, ok := result.Content[0].(*sdkmcp.TextContent)
 	require.True(t, ok)
@@ -118,6 +118,35 @@ func TestBuildAPIRequestMapsPathQueryAndBody(t *testing.T) {
 	require.JSONEq(t, `{"memo":{"name":"memos/abc123","content":"updated"}}`, string(body))
 }
 
+func TestBuildAPIRequestAcceptsResourceNamesForPathParameters(t *testing.T) {
+	tests := []struct {
+		name     string
+		path     string
+		value    string
+		wantPath string
+	}{
+		{name: "canonical memo name", path: "/api/v1/memos/{memo}", value: "memos/abc123", wantPath: "/api/v1/memos/abc123"},
+		{name: "bare memo id", path: "/api/v1/memos/{memo}", value: "abc123", wantPath: "/api/v1/memos/abc123"},
+		{name: "canonical name on nested route", path: "/api/v1/memos/{memo}/comments", value: "memos/abc123", wantPath: "/api/v1/memos/abc123/comments"},
+		{name: "canonical attachment name", path: "/api/v1/attachments/{attachment}", value: "attachments/att42", wantPath: "/api/v1/attachments/att42"},
+		{name: "foreign prefix left untouched", path: "/api/v1/memos/{memo}", value: "attachments/att42", wantPath: "/api/v1/memos/attachments%2Fatt42"},
+		{name: "multi-segment value left untouched", path: "/api/v1/memos/{memo}", value: "memos/abc/extra", wantPath: "/api/v1/memos/memos%2Fabc%2Fextra"},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			parameterName := test.path[strings.Index(test.path, "{")+1 : strings.Index(test.path, "}")]
+			operation := &openAPIOperation{
+				Method:     "GET",
+				Path:       test.path,
+				Parameters: []openAPIParameter{{Name: parameterName, In: "path", Required: true, Schema: jsonSchema{"type": "string"}}},
+			}
+			req, err := buildAPIRequest(context.Background(), operation, map[string]any{parameterName: test.value}, "")
+			require.NoError(t, err)
+			require.Equal(t, test.wantPath, req.URL.EscapedPath())
+		})
+	}
+}
+
 func TestBuildAPIRequestRequiresPathParameters(t *testing.T) {
 	operation := &openAPIOperation{
 		Method:     "GET",
@@ -201,11 +230,7 @@ func TestExecuteOperationConvertsAPIErrorsToToolErrors(t *testing.T) {
 	result, err := adapter.execute(context.Background(), operation, map[string]any{"memo": "missing"}, "")
 	require.NoError(t, err)
 	require.True(t, result.IsError)
-	require.Equal(t, map[string]any{
-		"error": map[string]any{
-			"message": "404 Not Found: missing memo",
-		},
-	}, result.StructuredContent)
+	require.Nil(t, result.StructuredContent)
 	text, ok := result.Content[0].(*sdkmcp.TextContent)
 	require.True(t, ok)
 	require.Contains(t, text.Text, "404")

server/router/mcp/result.go

@@ -35,18 +35,17 @@ func newStructuredToolResult(value any) (*sdkmcp.CallToolResult, error) {
 	}, nil
 }
 
+// newToolErrorResult reports a tool execution error through IsError and text
+// content only. Error results must not carry structuredContent: every tool
+// declares an outputSchema describing its success payload, and spec-strict
+// clients validate structuredContent against that schema — an {"error": ...}
+// object fails validation and masks the real error message.
 func newToolErrorResult(message string) *sdkmcp.CallToolResult {
-	structured := map[string]any{
-		"error": map[string]any{
-			"message": message,
-		},
-	}
 	return &sdkmcp.CallToolResult{
 		Content: []sdkmcp.Content{
 			&sdkmcp.TextContent{Text: message},
 		},
-		StructuredContent: structured,
-		IsError:           true,
+		IsError: true,
 	}
 }
 

server/router/mcp/service_test.go

@@ -237,11 +237,17 @@ func TestMCPToolCallRejectsInvalidArguments(t *testing.T) {
 			result, ok := response["result"].(map[string]any)
 			require.True(t, ok)
 			require.Equal(t, true, result["isError"])
-			structured, ok := result["structuredContent"].(map[string]any)
+			// Error results carry no structuredContent — it would fail
+			// validation against the tool's declared outputSchema in strict
+			// clients. The message travels in the text content instead.
+			_, hasStructured := result["structuredContent"]
+			require.False(t, hasStructured)
+			content, ok := result["content"].([]any)
 			require.True(t, ok)
-			errorObject, ok := structured["error"].(map[string]any)
+			require.NotEmpty(t, content)
+			textBlock, ok := content[0].(map[string]any)
 			require.True(t, ok)
-			require.Contains(t, errorObject["message"], test.wantError)
+			require.Contains(t, textBlock["text"], test.wantError)
 		})
 	}
 	require.Zero(t, routeHits)