Remove redundant ToolType field from Workload (#2932)

carlos-gn · claude · amirejaz · web-flow · commit 6cfd20a7d30a · 2025-12-11T12:26:03.000+01:00
* Remove redundant ToolType field from Workload The ToolType field in the Workload struct mapped to either "mcp" (container workloads) or "remote" (remote workloads), but since MCP is the only alternative type in practice, the field has become unnecessary and adds no value. This change eliminates the redundant field from the codebase, simplifying the business logic that previously checked both ToolType and Remote fields. The condition "ToolType != 'mcp' && !Remote" now simplifies to just checking the Remote field directly. Changes: - Remove ToolType field from core.Workload struct - Delete LabelToolType constant and GetToolType function from labels package - Simplify shouldSkipWorkload logic in client manager - Remove ToolType from CLI list command output - Remove tool_type from backend metadata (workloads and vMCP) - Update all affected tests - Update API documentation (swagger.yaml) and regenerate docs Fixes #2923 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: carlos <carlosgn@protonmail.com> * Remove ToolType property Signed-off-by: carlos <21148423+carlos-gn@users.noreply.github.com> --------- Signed-off-by: carlos <carlosgn@protonmail.com> Signed-off-by: carlos <21148423+carlos-gn@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Muhammad Amir Ejaz <amir@stacklok.com> Co-authored-by: Eleftheria Stein-Kousathana <eleftheria@stacklok.com> Co-authored-by: Jakub Hrozek <jakub.hrozek@posteo.se>
diff --git a/docs/server/docs.go b/docs/server/docs.go
diff --git a/docs/server/swagger.json b/docs/server/swagger.json
diff --git a/docs/server/swagger.yaml b/docs/server/swagger.yaml
diff --git a/pkg/client/manager.go b/pkg/client/manager.go
@@ -13,10 +13,6 @@ import (
 	"github.com/stacklok/toolhive/pkg/logger"
 )
 
-const (
-	mcpToolType = "mcp"
-)
-
 // Client represents a registered ToolHive client.
 type Client struct {
 	Name MCPClient `json:"name"`
@@ -206,9 +202,9 @@ func (m *defaultManager) RemoveServerFromClients(ctx context.Context, serverName
 }
 
 // shouldSkipWorkload determines if a workload should be skipped when adding/removing servers from clients.
-// Workloads are skipped if they are not MCP tool type and if they are not remote.
+// Workloads are skipped if they are not remote.
 func shouldSkipWorkload(workload core.Workload) bool {
-	return workload.ToolType != mcpToolType && !workload.Remote
+	return !workload.Remote
 }
 
 // addWorkloadsToClient adds the specified workloads to the client's configuration
diff --git a/pkg/core/workload.go b/pkg/core/workload.go
@@ -22,9 +22,6 @@ type Workload struct {
 	// Port is the port on which the workload is exposed.
 	// This is embedded in the URL.
 	Port int `json:"port"`
-	// ToolType is the type of tool this workload represents.
-	// For now, it will always be "mcp" - representing an MCP server.
-	ToolType string `json:"tool_type"`
 	// TransportType is the type of transport used for this workload.
 	TransportType types.TransportType `json:"transport_type"`
 	// ProxyMode is the proxy mode that clients should use to connect.
diff --git a/pkg/core/workload_test.go b/pkg/core/workload_test.go
@@ -93,7 +93,6 @@ func TestSortWorkloadsByName(t *testing.T) {
 					Package:       "zebra-pkg",
 					URL:           "http://localhost:8080",
 					Port:          8080,
-					ToolType:      "mcp",
 					TransportType: types.TransportTypeSSE,
 					Status:        runtime.WorkloadStatusRunning,
 					StatusContext: "healthy",
@@ -108,7 +107,6 @@ func TestSortWorkloadsByName(t *testing.T) {
 					Package:       "alpha-pkg",
 					URL:           "http://localhost:8081",
 					Port:          8081,
-					ToolType:      "mcp",
 					TransportType: types.TransportTypeStdio,
 					Status:        runtime.WorkloadStatusStopped,
 					StatusContext: "stopped",
@@ -159,7 +157,6 @@ func TestSortWorkloadsByName(t *testing.T) {
 					assert.Equal(t, originalWorkload.Package, sortedWorkload.Package)
 					assert.Equal(t, originalWorkload.URL, sortedWorkload.URL)
 					assert.Equal(t, originalWorkload.Port, sortedWorkload.Port)
-					assert.Equal(t, originalWorkload.ToolType, sortedWorkload.ToolType)
 					assert.Equal(t, originalWorkload.TransportType, sortedWorkload.TransportType)
 					assert.Equal(t, originalWorkload.Status, sortedWorkload.Status)
 					assert.Equal(t, originalWorkload.StatusContext, sortedWorkload.StatusContext)
diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go
@@ -27,9 +27,6 @@ const (
 	// LabelPort is the label that contains the port
 	LabelPort = "toolhive-port"
 
-	// LabelToolType is the label that indicates the type of tool
-	LabelToolType = "toolhive-tool-type"
-
 	// LabelNetworkIsolation indicates that the network isolation functionality is enabled.
 	LabelNetworkIsolation = "toolhive-network-isolation"
 
@@ -50,9 +47,6 @@ func AddStandardLabels(labels map[string]string, containerName, containerBaseNam
 	labels[LabelBaseName] = containerBaseName
 	labels[LabelTransport] = transportType
 	labels[LabelPort] = fmt.Sprintf("%d", port)
-
-	// TODO: In the future, we'll support different tool types beyond just "mcp"
-	labels[LabelToolType] = "mcp"
 }
 
 // AddNetworkLabels adds network-related labels to a network
@@ -116,11 +110,6 @@ func GetPort(labels map[string]string) (int, error) {
 	return port, nil
 }
 
-// GetToolType gets the tool type from labels
-func GetToolType(labels map[string]string) string {
-	return labels[LabelToolType]
-}
-
 // GetGroup gets the group name from labels
 func GetGroup(labels map[string]string) string {
 	return labels[LabelGroup]
@@ -147,7 +136,6 @@ func IsStandardToolHiveLabel(key string) bool {
 		LabelBaseName,
 		LabelTransport,
 		LabelPort,
-		LabelToolType,
 		LabelNetworkIsolation,
 	}
 
diff --git a/pkg/labels/labels_test.go b/pkg/labels/labels_test.go
@@ -26,7 +26,6 @@ func TestAddStandardLabels(t *testing.T) {
 				LabelBaseName:  "test-base",
 				LabelTransport: "http",
 				LabelPort:      "8080",
-				LabelToolType:  "mcp",
 			},
 		},
 		{
@@ -41,7 +40,6 @@ func TestAddStandardLabels(t *testing.T) {
 				LabelBaseName:  "another-base",
 				LabelTransport: "https",
 				LabelPort:      "9090",
-				LabelToolType:  "mcp",
 			},
 		},
 		{
@@ -56,7 +54,6 @@ func TestAddStandardLabels(t *testing.T) {
 				LabelBaseName:  "group-base",
 				LabelTransport: "sse",
 				LabelPort:      "7070",
-				LabelToolType:  "mcp",
 			},
 		},
 	}
@@ -390,11 +387,6 @@ func TestIsStandardToolHiveLabel(t *testing.T) {
 			key:      LabelPort,
 			expected: true,
 		},
-		{
-			name:     "Standard tool type label",
-			key:      LabelToolType,
-			expected: true,
-		},
 		{
 			name:     "Standard network isolation label",
 			key:      LabelNetworkIsolation,
@@ -693,35 +685,3 @@ func TestParseLabelValidation(t *testing.T) {
 		})
 	}
 }
-
-func TestGetToolType(t *testing.T) {
-	t.Parallel()
-	tests := []struct {
-		name     string
-		labels   map[string]string
-		expected string
-	}{
-		{
-			name: "Tool type exists",
-			labels: map[string]string{
-				LabelToolType: "mcp",
-			},
-			expected: "mcp",
-		},
-		{
-			name:     "Tool type doesn't exist",
-			labels:   map[string]string{},
-			expected: "",
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-			result := GetToolType(tc.labels)
-			if result != tc.expected {
-				t.Errorf("Expected tool type to be %s, but got %s", tc.expected, result)
-			}
-		})
-	}
-}
diff --git a/pkg/runner/config_test.go b/pkg/runner/config_test.go
@@ -460,7 +460,6 @@ func TestRunConfig_WithStandardLabels(t *testing.T) {
 				"toolhive-name":      "test-server",
 				"toolhive-transport": "sse",
 				"toolhive-port":      "60000",
-				"toolhive-tool-type": "mcp",
 			},
 		},
 		{
@@ -477,7 +476,6 @@ func TestRunConfig_WithStandardLabels(t *testing.T) {
 				"toolhive":           "true",
 				"toolhive-name":      "test-server",
 				"toolhive-transport": "stdio",
-				"toolhive-tool-type": "mcp",
 				"existing-label":     "existing-value",
 			},
 		},
@@ -496,7 +494,6 @@ func TestRunConfig_WithStandardLabels(t *testing.T) {
 				"toolhive-name":      "test-server",
 				"toolhive-transport": "sse", // Should be "sse" not "stdio"
 				"toolhive-port":      "60000",
-				"toolhive-tool-type": "mcp",
 			},
 		},
 		{
@@ -514,7 +511,6 @@ func TestRunConfig_WithStandardLabels(t *testing.T) {
 				"toolhive-name":      "test-server",
 				"toolhive-transport": "streamable-http", // Should be "streamable-http" not "stdio"
 				"toolhive-port":      "60000",
-				"toolhive-tool-type": "mcp",
 			},
 		},
 	}
diff --git a/pkg/vmcp/aggregator/discoverer_test.go b/pkg/vmcp/aggregator/discoverer_test.go
@@ -36,7 +36,6 @@ func TestBackendDiscoverer_Discover(t *testing.T) {
 			TransportType: "streamable-http",
 			HealthStatus:  vmcp.BackendHealthy,
 			Metadata: map[string]string{
-				"tool_type":       "github",
 				"workload_status": "running",
 				"env":             "prod",
 			},
@@ -48,7 +47,6 @@ func TestBackendDiscoverer_Discover(t *testing.T) {
 			TransportType: "sse",
 			HealthStatus:  vmcp.BackendHealthy,
 			Metadata: map[string]string{
-				"tool_type":       "jira",
 				"workload_status": "running",
 			},
 		}
@@ -67,7 +65,6 @@ func TestBackendDiscoverer_Discover(t *testing.T) {
 		assert.Equal(t, "workload1", backends[0].ID)
 		assert.Equal(t, "http://localhost:8080/mcp", backends[0].BaseURL)
 		assert.Equal(t, vmcp.BackendHealthy, backends[0].HealthStatus)
-		assert.Equal(t, "github", backends[0].Metadata["tool_type"])
 		assert.Equal(t, "prod", backends[0].Metadata["env"])
 		assert.Equal(t, "workload2", backends[1].ID)
 		assert.Equal(t, "sse", backends[1].TransportType)
@@ -337,7 +334,7 @@ func TestCLIWorkloadDiscoverer(t *testing.T) {
 			Name:         "workload1",
 			BaseURL:      "http://localhost:8080/mcp",
 			HealthStatus: vmcp.BackendHealthy,
-			Metadata:     map[string]string{"tool_type": "github", "env": "prod"},
+			Metadata:     map[string]string{"env": "prod"},
 		}
 
 		mockGroups.EXPECT().Exists(gomock.Any(), testGroupName).Return(true, nil)
@@ -353,7 +350,6 @@ func TestCLIWorkloadDiscoverer(t *testing.T) {
 		assert.Equal(t, "workload1", backends[0].ID)
 		assert.Equal(t, "http://localhost:8080/mcp", backends[0].BaseURL)
 		assert.Equal(t, vmcp.BackendHealthy, backends[0].HealthStatus)
-		assert.Equal(t, "github", backends[0].Metadata["tool_type"])
 		assert.Equal(t, "prod", backends[0].Metadata["env"])
 	})
 
diff --git a/pkg/vmcp/workloads/k8s.go b/pkg/vmcp/workloads/k8s.go
@@ -3,6 +3,7 @@ package workloads
 import (
 	"context"
 	"fmt"
+	"maps"
 	"strings"
 
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -178,12 +179,9 @@ func (d *k8sDiscoverer) mcpServerToBackend(ctx context.Context, mcpServer *mcpv1
 	}
 
 	// Copy user labels to metadata first
-	for k, v := range userLabels {
-		backend.Metadata[k] = v
-	}
+	maps.Copy(backend.Metadata, userLabels)
 
 	// Set system metadata (these override user labels to prevent conflicts)
-	backend.Metadata["tool_type"] = "mcp"
 	backend.Metadata["workload_status"] = string(mcpServer.Status.Phase)
 	if mcpServer.Namespace != "" {
 		backend.Metadata["namespace"] = mcpServer.Namespace
diff --git a/pkg/vmcp/workloads/k8s_test.go b/pkg/vmcp/workloads/k8s_test.go
@@ -350,7 +350,6 @@ func TestMCPServerToBackend_BasicFields(t *testing.T) {
 	assert.Equal(t, "http://localhost:8080", backend.BaseURL)
 	assert.Equal(t, "streamable-http", backend.TransportType)
 	assert.Equal(t, vmcp.BackendHealthy, backend.HealthStatus)
-	assert.Equal(t, "mcp", backend.Metadata["tool_type"])
 	assert.Equal(t, string(mcpv1alpha1.MCPServerPhaseRunning), backend.Metadata["workload_status"])
 	assert.Equal(t, namespace, backend.Metadata["namespace"])
 }
diff --git a/pkg/workloads/manager.go b/pkg/workloads/manager.go
@@ -208,7 +208,6 @@ func (d *DefaultManager) GetWorkloadAsVMCPBackend(ctx context.Context, workloadN
 	}
 
 	// Set system metadata (these override user labels to prevent conflicts)
-	backend.Metadata["tool_type"] = workload.ToolType
 	backend.Metadata["workload_status"] = string(workload.Status)
 
 	return backend, nil
@@ -1496,7 +1495,6 @@ func (d *DefaultManager) getRemoteWorkloadsFromState(
 			Port:          runConfig.Port,
 			TransportType: transportType,
 			ProxyMode:     effectiveProxyMode,
-			ToolType:      "remote",
 			Group:         runConfig.Group,
 			CreatedAt:     workloadStatus.CreatedAt,
 			Labels:        runConfig.ContainerLabels,
diff --git a/pkg/workloads/statuses/file_status.go b/pkg/workloads/statuses/file_status.go
@@ -160,7 +160,6 @@ func (f *fileStatusManager) populateRemoteWorkloadData(ctx context.Context, work
 	workload.Port = config.Port
 	workload.TransportType = transportType
 	workload.ProxyMode = effectiveProxyMode
-	workload.ToolType = "remote"
 	workload.Group = config.Group
 	workload.Labels = config.ContainerLabels
 	workload.Remote = true
diff --git a/pkg/workloads/types/types.go b/pkg/workloads/types/types.go
@@ -57,9 +57,6 @@ func WorkloadFromContainerInfo(container *runtime.ContainerInfo) (core.Workload,
 		}
 	}
 
-	// Get tool type from labels
-	toolType := labels.GetToolType(container.Labels)
-
 	// Get port from labels
 	port, err := labels.GetPort(container.Labels)
 	if err != nil {
@@ -103,7 +100,6 @@ func WorkloadFromContainerInfo(container *runtime.ContainerInfo) (core.Workload,
 		Name:          name, // Use the calculated workload name (base name), not container name
 		Package:       container.Image,
 		URL:           url,
-		ToolType:      toolType,
 		TransportType: tType,
 		ProxyMode:     effectiveProxyMode,
 		Status:        container.State,
diff --git a/test/e2e/remote_mcp_server_test.go b/test/e2e/remote_mcp_server_test.go
@@ -17,7 +17,6 @@ type WorkloadInfo struct {
 	Package       string            `json:"package"`
 	URL           string            `json:"url"`
 	Port          int               `json:"port"`
-	ToolType      string            `json:"tool_type"`
 	TransportType string            `json:"transport_type"`
 	ProxyMode     string            `json:"proxy_mode"`
 	Status        string            `json:"status"`
@@ -87,7 +86,6 @@ var _ = Describe("Remote MCP Server", Label("remote", "mcp", "e2e"), Serial, fun
 				Expect(serverInfo.Status).To(Equal("running"), "Server should be in running state")
 				Expect(serverInfo.Remote).To(BeTrue(), "Server should be marked as remote")
 				Expect(serverInfo.Package).To(Equal("remote"), "Package should be 'remote'")
-				Expect(serverInfo.ToolType).To(Equal("remote"), "Tool type should be 'remote'")
 				Expect(serverInfo.TransportType).To(Equal("streamable-http"), "Transport should be streamable-http")
 			})
 

Original file line number	Diff line number	Diff line change
`@@ -13,10 +13,6 @@ import (`
`13`	`13`	`"github.com/stacklok/toolhive/pkg/logger"`
`14`	`14`	`)`
`15`	`15`
`16`		`-const (`
`17`		`- mcpToolType = "mcp"`
`18`		`-)`
`19`		`-`
`20`	`16`	`// Client represents a registered ToolHive client.`
`21`	`17`	`type Client struct {`
`22`	`18`	Name MCPClient `json:"name"`
`@@ -206,9 +202,9 @@ func (m *defaultManager) RemoveServerFromClients(ctx context.Context, serverName`
`206`	`202`	`}`
`207`	`203`
`208`	`204`	`// shouldSkipWorkload determines if a workload should be skipped when adding/removing servers from clients.`
`209`		`-// Workloads are skipped if they are not MCP tool type and if they are not remote.`
	`205`	`+// Workloads are skipped if they are not remote.`
`210`	`206`	`func shouldSkipWorkload(workload core.Workload) bool {`
`211`		`- return workload.ToolType != mcpToolType && !workload.Remote`
	`207`	`+ return !workload.Remote`
`212`	`208`	`}`
`213`	`209`
`214`	`210`	`// addWorkloadsToClient adds the specified workloads to the client's configuration`