Skip to content

Commit 928369b

Browse files
authored
Truncate tool names to 64 characters (#47)
* Truncate tool names to 64 characters * Apply suggestions from code review Signed-off-by: Mattt <[email protected]> * Fix tests --------- Signed-off-by: Mattt <[email protected]>
1 parent f1951d7 commit 928369b

File tree

2 files changed

+204
-31
lines changed

2 files changed

+204
-31
lines changed

mcp/server.go

Lines changed: 52 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package mcp
22

33
import (
44
"bytes"
5+
"crypto/sha256"
56
"encoding/base64"
67
"encoding/json"
78
"fmt"
@@ -226,6 +227,7 @@ func (s *Server) handleInitialize(request *InitializeRequest) (*InitializeRespon
226227
return response, nil
227228
}
228229

230+
// Update the tools list generation to use the helper
229231
func (s *Server) handleToolsList(request *ToolsListRequest) (*ToolsListResponse, error) {
230232
tools := []Tool{}
231233
if s.model.Paths == nil || s.model.Paths.PathItems == nil {
@@ -352,8 +354,9 @@ func (s *Server) handleToolsList(request *ToolsListRequest) (*ToolsListResponse,
352354
description = op.op.Summary
353355
}
354356

357+
toolName := getToolName(op.op.OperationId)
355358
tools = append(tools, Tool{
356-
Name: op.op.OperationId,
359+
Name: toolName,
357360
Description: description,
358361
InputSchema: inputSchema,
359362
})
@@ -367,8 +370,9 @@ func (s *Server) handleToolsList(request *ToolsListRequest) (*ToolsListResponse,
367370
return &ToolsListResponse{Tools: tools}, nil
368371
}
369372

373+
// Update the tools call handler to use the new finder
370374
func (s *Server) handleToolsCall(request *ToolCallRequest) (*ToolCallResponse, error) {
371-
method, p, operation, pathItem, found := s.findOperation(s.model, request.Name)
375+
method, p, operation, pathItem, found := s.findOperationByToolName(request.Name)
372376
if !found {
373377
return nil, jsonrpc.NewError(jsonrpc.ErrMethodNotFound, nil)
374378
}
@@ -560,35 +564,6 @@ func (s *Server) handlePing(request *PingRequest) (*PingResponse, error) {
560564
return &PingResponse{}, nil
561565
}
562566

563-
func (s *Server) findOperation(model *v3.Document, operationId string) (method, path string, operation *v3.Operation, pathItem *v3.PathItem, found bool) {
564-
if model.Paths == nil || model.Paths.PathItems == nil {
565-
return "", "", nil, nil, false
566-
}
567-
568-
for pair := model.Paths.PathItems.First(); pair != nil; pair = pair.Next() {
569-
pathStr := pair.Key()
570-
item := pair.Value()
571-
572-
operations := []struct {
573-
method string
574-
op *v3.Operation
575-
}{
576-
{"GET", item.Get},
577-
{"POST", item.Post},
578-
{"PUT", item.Put},
579-
{"DELETE", item.Delete},
580-
{"PATCH", item.Patch},
581-
}
582-
583-
for _, op := range operations {
584-
if op.op != nil && op.op.OperationId == operationId {
585-
return op.method, pathStr, op.op, item, true
586-
}
587-
}
588-
}
589-
return "", "", nil, nil, false
590-
}
591-
592567
// pathSegmentEscape escapes invalid URL path segment characters according to RFC 3986.
593568
// It preserves valid path characters including comma, colon, and @ sign.
594569
func pathSegmentEscape(s string) string {
@@ -645,3 +620,49 @@ func shouldEscape(c byte) bool {
645620
}
646621
return true
647622
}
623+
624+
// getToolName creates a unique tool name from an operation ID, ensuring it's within the 64-character limit
625+
// while maintaining a bijective mapping between operation IDs and tool names
626+
func getToolName(operationId string) string {
627+
if len(operationId) <= 64 {
628+
return operationId
629+
}
630+
// Generate a short hash of the full operation ID
631+
hash := sha256.Sum256([]byte(operationId))
632+
// Use base64 encoding for shorter hash representation (first 8 chars)
633+
shortHash := base64.RawURLEncoding.EncodeToString(hash[:])[:8]
634+
// Create a deterministic name that fits within limits while preserving uniqueness
635+
return operationId[:55] + "_" + shortHash
636+
}
637+
638+
// findOperationByToolName maps a tool name back to its corresponding OpenAPI operation
639+
func (s *Server) findOperationByToolName(toolName string) (method, path string, operation *v3.Operation, pathItem *v3.PathItem, found bool) {
640+
if s.model.Paths == nil || s.model.Paths.PathItems == nil {
641+
return "", "", nil, nil, false
642+
}
643+
644+
for pair := s.model.Paths.PathItems.First(); pair != nil; pair = pair.Next() {
645+
pathStr := pair.Key()
646+
item := pair.Value()
647+
648+
operations := []struct {
649+
method string
650+
op *v3.Operation
651+
}{
652+
{"GET", item.Get},
653+
{"POST", item.Post},
654+
{"PUT", item.Put},
655+
{"DELETE", item.Delete},
656+
{"PATCH", item.Patch},
657+
}
658+
659+
for _, op := range operations {
660+
if op.op != nil && op.op.OperationId != "" {
661+
if getToolName(op.op.OperationId) == toolName {
662+
return op.method, pathStr, op.op, item, true
663+
}
664+
}
665+
}
666+
}
667+
return "", "", nil, nil, false
668+
}

mcp/server_test.go

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -778,3 +778,155 @@ func TestPathJoining(t *testing.T) {
778778
})
779779
}
780780
}
781+
782+
func TestToolNameMapping(t *testing.T) {
783+
tests := []struct {
784+
name string
785+
operationId string
786+
wantLen int
787+
wantPrefix string
788+
}{
789+
{
790+
name: "short operation ID",
791+
operationId: "listPets",
792+
wantLen: 8,
793+
wantPrefix: "listPets",
794+
},
795+
{
796+
name: "exactly 64 characters",
797+
operationId: strings.Repeat("a", 64),
798+
wantLen: 64,
799+
wantPrefix: strings.Repeat("a", 64),
800+
},
801+
{
802+
name: "long operation ID",
803+
operationId: "thisIsAVeryLongOperationIdThatExceedsTheSixtyFourCharacterLimitAndNeedsToBeHandledProperly",
804+
wantLen: 64,
805+
wantPrefix: "thisIsAVeryLongOperationIdThatExceedsTheSixtyFourCharac", // 55 chars
806+
},
807+
{
808+
name: "multiple long IDs generate different names",
809+
operationId: "anotherVeryLongOperationIdThatExceedsTheSixtyFourCharacterLimitAndNeedsToBeHandledProperly",
810+
wantLen: 64,
811+
wantPrefix: "anotherVeryLongOperationIdThatExceedsTheSixtyFourCharac", // 55 chars
812+
},
813+
}
814+
815+
// Store generated names to check for uniqueness
816+
generatedNames := make(map[string]string)
817+
818+
for _, tt := range tests {
819+
t.Run(tt.name, func(t *testing.T) {
820+
// Get the tool name
821+
toolName := getToolName(tt.operationId)
822+
823+
// Check length constraints
824+
assert.Equal(t, tt.wantLen, len(toolName), "tool name length mismatch")
825+
826+
// For long names, verify the structure
827+
if len(tt.operationId) > 64 {
828+
// Verify the prefix is exactly 55 characters
829+
prefix := toolName[:55]
830+
assert.Equal(t, tt.wantPrefix, prefix, "prefix mismatch")
831+
832+
// Check that there's an underscore separator at position 55
833+
assert.Equal(t, "_", string(toolName[55]), "underscore separator not found at position 55")
834+
835+
// Verify hash part length (should be 8 characters)
836+
hash := toolName[56:]
837+
assert.Equal(t, 8, len(hash), "hash suffix should be 8 characters")
838+
839+
// Verify the hash is URL-safe base64
840+
for _, c := range hash {
841+
assert.Contains(t,
842+
"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_",
843+
string(c),
844+
"hash should only contain URL-safe base64 characters")
845+
}
846+
} else {
847+
// For short names, verify exact match
848+
assert.Equal(t, tt.wantPrefix, toolName, "tool name mismatch for short operation ID")
849+
}
850+
851+
// Check bijectivity - each operation ID should generate a unique tool name
852+
if existing, exists := generatedNames[toolName]; exists {
853+
assert.Equal(t, tt.operationId, existing,
854+
"tool name collision detected: same name generated for different operation IDs")
855+
}
856+
generatedNames[toolName] = tt.operationId
857+
})
858+
}
859+
}
860+
861+
func TestFindOperationByToolName(t *testing.T) {
862+
// Create a test spec with a mix of short and long operation IDs
863+
longOpId := "thisIsAVeryLongOperationIdThatExceedsTheSixtyFourCharacterLimitAndNeedsToBeHandledProperly"
864+
spec := fmt.Sprintf(`{
865+
"openapi": "3.0.0",
866+
"servers": [{"url": "https://api.example.com"}],
867+
"paths": {
868+
"/pets": {
869+
"get": {
870+
"operationId": "listPets",
871+
"description": "List pets"
872+
}
873+
},
874+
"/very/long/path": {
875+
"post": {
876+
"operationId": "%s",
877+
"description": "Operation with long ID"
878+
}
879+
}
880+
}
881+
}`, longOpId)
882+
883+
server, err := NewServer(WithSpecData([]byte(spec)))
884+
require.NoError(t, err)
885+
886+
tests := []struct {
887+
name string
888+
toolName string
889+
wantMethod string
890+
wantPath string
891+
wantFound bool
892+
}{
893+
{
894+
name: "find short operation ID",
895+
toolName: "listPets",
896+
wantMethod: "GET",
897+
wantPath: "/pets",
898+
wantFound: true,
899+
},
900+
{
901+
name: "find long operation ID",
902+
toolName: getToolName(longOpId),
903+
wantMethod: "POST",
904+
wantPath: "/very/long/path",
905+
wantFound: true,
906+
},
907+
{
908+
name: "operation not found",
909+
toolName: "nonexistentOperation",
910+
wantFound: false,
911+
},
912+
}
913+
914+
for _, tt := range tests {
915+
t.Run(tt.name, func(t *testing.T) {
916+
method, path, operation, pathItem, found := server.findOperationByToolName(tt.toolName)
917+
918+
assert.Equal(t, tt.wantFound, found)
919+
if tt.wantFound {
920+
assert.Equal(t, tt.wantMethod, method)
921+
assert.Equal(t, tt.wantPath, path)
922+
assert.NotNil(t, operation)
923+
assert.NotNil(t, pathItem)
924+
} else {
925+
assert.Empty(t, method)
926+
assert.Empty(t, path)
927+
assert.Nil(t, operation)
928+
assert.Nil(t, pathItem)
929+
}
930+
})
931+
}
932+
}

0 commit comments

Comments
 (0)