wip: address feedback and fix

Signed-off-by: Nick Hale <[email protected]>

Author: njhale

apiclient/types/mcpserver.go

@@ -65,7 +65,7 @@ type CatalogComponentServer struct {
 	CatalogEntryID string                        `json:"catalogEntryID"`
 	Manifest       MCPServerCatalogEntryManifest `json:"manifest"`
 	ToolOverrides  []ToolOverride                `json:"toolOverrides,omitempty"`
-	Enabled        bool                          `json:"enabled,omitempty"`
+	Disabled       bool                          `json:"disabled,omitempty"`
 }
 
 type CompositeRuntimeConfig struct {
@@ -76,7 +76,7 @@ type ComponentServer struct {
 	CatalogEntryID string            `json:"catalogEntryID"`
 	Manifest       MCPServerManifest `json:"manifest"`
 	ToolOverrides  []ToolOverride    `json:"toolOverrides,omitempty"`
-	Enabled        bool              `json:"enabled,omitempty"`
+	Disabled       bool              `json:"disabled,omitempty"`
 }
 
 type MCPServerCatalogEntry struct {
@@ -419,7 +419,7 @@ func MapCatalogEntryToServer(catalogEntry MCPServerCatalogEntryManifest, userURL
 				CatalogEntryID: catalogComponent.CatalogEntryID,
 				Manifest:       componentServerManifest,
 				ToolOverrides:  catalogComponent.ToolOverrides,
-				Enabled:        true,
+				Disabled:       false,
 			}
 		}
 

pkg/api/handlers/mcp.go

@@ -318,7 +318,6 @@ func (h *MCPCatalogHandler) CreateEntry(req api.Context) error {
 }
 
 func (h *MCPCatalogHandler) UpdateEntry(req api.Context) error {
-	// TODO(cmcp): Update this handler to handle updating composite entries
 	catalogName := req.PathValue("catalog_id")
 	workspaceID := req.PathValue("workspace_id")
 	entryName := req.PathValue("entry_id")
@@ -683,7 +682,10 @@ func (h *MCPCatalogHandler) GenerateToolPreviews(req api.Context) error {
 		catalogName = req.PathValue("catalog_id")
 		workspaceID = req.PathValue("workspace_id")
 		entryName   = req.PathValue("entry_id")
-		dryRun      = req.Request.URL.Query().Get("dryRun") == "true"
+		// "dryRun" lets us get the previews for an MCP server without updating its CatalogEntry.
+		// This is used when we populate the tools for individual MCP servers when creating a composite CatalogEntry
+		// (configuring tool overrides).
+		dryRun = req.Request.URL.Query().Get("dryRun") == "true"
 	)
 
 	// Verify the scope exists
@@ -814,7 +816,6 @@ func (h *MCPCatalogHandler) generateCompositeToolPreviews(req api.Context, entry
 			}
 
 			if oauthURL != "" {
-				// TODO(cmcp): Figure out how to handle composite OAuth when generating previews for composite servers
 				return types.NewErrBadRequest("MCP server requires OAuth authentication")
 			}
 
@@ -828,7 +829,6 @@ func (h *MCPCatalogHandler) generateCompositeToolPreviews(req api.Context, entry
 			return fmt.Errorf("failed to generate tool preview: %w", err)
 		}
 
-		// TODO(cmcp): Apply tool overrides to the tool preview
 		compositeToolPreviews = append(compositeToolPreviews, toolPreview...)
 	}
 
@@ -858,7 +858,10 @@ func (h *MCPCatalogHandler) GenerateToolPreviewsOAuthURL(req api.Context) error
 		catalogName = req.PathValue("catalog_id")
 		workspaceID = req.PathValue("workspace_id")
 		entryName   = req.PathValue("entry_id")
-		dryRun      = req.Request.URL.Query().Get("dryRun") == "true"
+		// "dryRun" lets us get the previews for an MCP server without updating its CatalogEntry.
+		// This is used when we populate the tools for individual MCP servers when creating a composite CatalogEntry
+		// (configuring tool overrides).
+		dryRun = req.Request.URL.Query().Get("dryRun") == "true"
 	)
 
 	// Verify the scope exists
@@ -1019,7 +1022,6 @@ func normalizeMCPCatalogEntryName(name string) string {
 func (h *MCPCatalogHandler) populateComponentManifests(req api.Context, manifest *types.MCPServerCatalogEntryManifest, catalogName, workspaceID string) error {
 	// For each component server, fetch its catalog entry and populate the manifest
 	for i := range manifest.CompositeConfig.ComponentServers {
-		// TODO(cmcp): Handle multi-user component servers, these won't have a catalog entry ID, but will instead point directly to an MCP server
 		component := &manifest.CompositeConfig.ComponentServers[i]
 
 		var entry v1.MCPServerCatalogEntry

pkg/api/handlers/mcpcatalogs.go

@@ -185,6 +185,8 @@ func (c *clientMessageHandler) handleMessage(ctx context.Context, msg nmcp.Messa
 
 	var ch <-chan nmcp.Message
 	if msg.ID != nil {
+		// Generate a new random UUID.
+		// We do this to support composite servers, where multiple messages can have the same ID.
 		msg.ID = uuid.NewString()
 		ch = c.pendingRequests.WaitFor(msg.ID)
 		defer c.pendingRequests.Done(msg.ID)

pkg/api/handlers/mcpgateway/clientmessagehandler.go

@@ -58,7 +58,7 @@ func (h *Handler) onCompositeMessage(ctx context.Context, msg nmcp.Message, m me
 	}
 
 	type componentClient struct {
-		serverContext
+		messageContext
 		*mcp.Client
 	}
 	var (
@@ -150,17 +150,12 @@ func (h *Handler) onCompositeMessage(ctx context.Context, msg nmcp.Message, m me
 		)
 		if err != nil {
 			log.Errorf("Failed to get client for server %s: %v", componentServer.mcpServer.Name, err)
-			if componentServer.enabled {
-				// Enabled components must initialize successfully
-				return
-			}
-			// Disabled components can fail silently
-			continue
+			return
 		}
 
 		clients[componentKey] = componentClient{
-			serverContext: componentServer,
-			Client:        client,
+			messageContext: componentServer,
+			Client:         client,
 		}
 	}
 
@@ -185,7 +180,6 @@ func (h *Handler) onCompositeMessage(ctx context.Context, msg nmcp.Message, m me
 				}
 				return
 			}
-
 		}
 
 		// All components responded, reply with the last result
@@ -248,6 +242,7 @@ func (h *Handler) onCompositeMessage(ctx context.Context, msg nmcp.Message, m me
 				Code:    -32603,
 				Message: fmt.Sprintf("failed to reply to composite server %s: %v", m.mcpID, err),
 			}
+			return
 		}
 
 		result = compositeResult
@@ -265,7 +260,6 @@ func (h *Handler) onCompositeMessage(ctx context.Context, msg nmcp.Message, m me
 				compositePrompts = append(compositePrompts, m.toCompositePrompt(componentKey, prompt))
 			}
 		}
-
 		compositeResult := nmcp.ListPromptsResult{
 			Prompts: compositePrompts,
 		}
@@ -276,12 +270,11 @@ func (h *Handler) onCompositeMessage(ctx context.Context, msg nmcp.Message, m me
 				Message: fmt.Sprintf("failed to reply to composite server %s: %v", m.mcpID, err),
 			}
 		}
-
 		result = compositeResult
 		return
 	case methodPromptsGet:
 		var compositeRequest nmcp.GetPromptRequest
-		if err := json.Unmarshal(msg.Params, &compositeRequest); err != nil {
+		if err = json.Unmarshal(msg.Params, &compositeRequest); err != nil {
 			err = &nmcp.RPCError{
 				Code:    -32602,
 				Message: fmt.Sprintf("Failed to unmarshal get prompt request: %v", err),
@@ -309,11 +302,11 @@ func (h *Handler) onCompositeMessage(ctx context.Context, msg nmcp.Message, m me
 
 		componentRequest := m.toComponentGetPromptRequest(componentKey, compositeRequest)
 
-		b, err := json.Marshal(componentRequest)
-		if err != nil {
+		b, marshalErr := json.Marshal(componentRequest)
+		if marshalErr != nil {
 			err = &nmcp.RPCError{
 				Code:    -32603,
-				Message: fmt.Sprintf("failed to marshal request for server %s: %v", client.mcpServer.Name, err),
+				Message: fmt.Sprintf("failed to marshal request for server %s: %v", client.mcpServer.Name, marshalErr),
 			}
 			return
 		}
@@ -331,22 +324,23 @@ func (h *Handler) onCompositeMessage(ctx context.Context, msg nmcp.Message, m me
 				Code:    -32603,
 				Message: fmt.Sprintf("failed to reply to composite server %s: %v", m.mcpID, err),
 			}
+			return
 		}
 
 		result = componentResult
 		return
 	case methodToolsList:
 		var compositeTools []nmcp.Tool
 		for componentKey, client := range clients {
-			var result nmcp.ListToolsResult
-			if err = client.Session.Exchange(ctx, methodToolsList, &msg, &result); err != nil {
+			var lr nmcp.ListToolsResult
+			if err = client.Session.Exchange(ctx, methodToolsList, &msg, &lr); err != nil {
 				log.Errorf("Failed to send %s message to server %s: %v", msg.Method, client.mcpID, err)
 				return
 			}
-			for _, tool := range result.Tools {
-				compositeTool, err := m.toCompositeTool(componentKey, tool)
-				if err != nil {
-					err = fmt.Errorf("failed to override tool %s: %w", tool.Name, err)
+			for _, tool := range lr.Tools {
+				compositeTool, convErr := m.toCompositeTool(componentKey, tool)
+				if convErr != nil {
+					err = fmt.Errorf("failed to override tool %s: %w", tool.Name, convErr)
 					return
 				}
 
@@ -370,13 +364,13 @@ func (h *Handler) onCompositeMessage(ctx context.Context, msg nmcp.Message, m me
 				Code:    -32603,
 				Message: fmt.Sprintf("failed to reply to composite server %s: %v", m.mcpID, err),
 			}
+			return
 		}
-
 		result = compositeResult
 		return
 	case methodToolsCall:
 		var compositeRequest nmcp.CallToolRequest
-		if err := json.Unmarshal(msg.Params, &compositeRequest); err != nil {
+		if err = json.Unmarshal(msg.Params, &compositeRequest); err != nil {
 			err = &nmcp.RPCError{
 				Code:    -32602,
 				Message: fmt.Sprintf("Failed to unmarshal tool call request: %v", err),
@@ -402,7 +396,8 @@ func (h *Handler) onCompositeMessage(ctx context.Context, msg nmcp.Message, m me
 			return
 		}
 
-		componentRequest, err := m.toComponentCallToolRequest(componentKey, compositeRequest)
+		var componentRequest *nmcp.CallToolRequest
+		componentRequest, err = m.toComponentCallToolRequest(componentKey, compositeRequest)
 		if err != nil {
 			log.Errorf("Failed to convert tool call request to component tool call request: %v", err)
 			return
@@ -412,13 +407,14 @@ func (h *Handler) onCompositeMessage(ctx context.Context, msg nmcp.Message, m me
 				Code:    -32602,
 				Message: fmt.Sprintf("Unknown tool: %s", compositeRequest.Name),
 			}
+			return
 		}
 
-		b, err := json.Marshal(componentRequest)
-		if err != nil {
+		b, marshalErr := json.Marshal(componentRequest)
+		if marshalErr != nil {
 			err = &nmcp.RPCError{
 				Code:    -32603,
-				Message: fmt.Sprintf("failed to marshal request for server %s: %v", client.mcpServer.Name, err),
+				Message: fmt.Sprintf("failed to marshal request for server %s: %v", client.mcpServer.Name, marshalErr),
 			}
 			return
 		}
@@ -436,6 +432,7 @@ func (h *Handler) onCompositeMessage(ctx context.Context, msg nmcp.Message, m me
 				Code:    -32603,
 				Message: fmt.Sprintf("failed to reply to composite server %s: %v", m.mcpID, err),
 			}
+			return
 		}
 
 		result = componentResult
@@ -446,8 +443,8 @@ func (h *Handler) onCompositeMessage(ctx context.Context, msg nmcp.Message, m me
 		result = nmcp.Notification{}
 		for _, client := range clients {
 			if err = client.Session.Exchange(ctx, msg.Method, &msg, &result); err != nil {
-				log.Errorf("Failed to send %s message to server %s: %v", msg.Method, client.mcpID, err)
-				return
+				log.Warnf("Failed to send %s message to server %s: %v", msg.Method, client.mcpID, err)
+				continue
 			}
 		}
 
@@ -466,7 +463,6 @@ func (h *Handler) onCompositeMessage(ctx context.Context, msg nmcp.Message, m me
 			Message: "Method not allowed",
 		}
 	}
-
 }
 
 func normalizeName(name string) string {
@@ -505,15 +501,20 @@ func mergeInitializeResults(composite nmcp.InitializeResult, component nmcp.Init
 		}
 	}
 
+	// Merge Logging capability
+	if component.Capabilities.Logging != nil {
+		composite.Capabilities.Logging = &struct{}{}
+	}
+
 	return composite
 }
 
 type compositeContext struct {
-	componentServers []serverContext
+	componentServers []messageContext
 	toolOverrides    map[string]otypes.ToolOverride
 }
 
-func newCompositeContext(config *otypes.CompositeRuntimeConfig, componentServers []serverContext) compositeContext {
+func newCompositeContext(config *otypes.CompositeRuntimeConfig, componentServers []messageContext) compositeContext {
 	compositeContext := compositeContext{
 		componentServers: componentServers,
 		toolOverrides:    make(map[string]otypes.ToolOverride),