changes from review

Author: taskbot

cmd/thv/app/list.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/stacklok/toolhive/pkg/core"
 	"github.com/stacklok/toolhive/pkg/logger"
-	"github.com/stacklok/toolhive/pkg/transport/types"
 	"github.com/stacklok/toolhive/pkg/workloads"
 )
 
@@ -109,14 +108,12 @@ func printMCPServersOutput(workloadList []core.Workload) error {
 	mcpServers := make(map[string]map[string]string)
 
 	for _, c := range workloadList {
-		// Determine the effective transport type to show to users
-		// This is the transport they actually connect to, not the underlying MCP server transport
-		effectiveTransport := getEffectiveTransportType(c)
-
+		// Use the effective transport type for display - this shows what clients actually connect to
+		// (e.g., sse/streamable-http for stdio servers with proxy mode)
 		// Add the MCP server to the map
 		mcpServers[c.Name] = map[string]string{
 			"url":  c.URL,
-			"type": effectiveTransport,
+			"type": c.EffectiveTransportType.String(),
 		}
 	}
 
@@ -162,17 +159,3 @@ func printTextOutput(workloadList []core.Workload) {
 		logger.Errorf("Warning: Failed to flush tabwriter: %v", err)
 	}
 }
-
-// getEffectiveTransportType determines the effective transport type that clients should use
-// This handles the case where stdio MCP servers are proxied through SSE or streamable-http
-func getEffectiveTransportType(workload core.Workload) string {
-	// If the underlying transport is stdio and we have a proxy mode set,
-	// return the proxy mode as that's what clients actually connect to
-	if workload.TransportType == types.TransportTypeStdio && workload.ProxyMode != "" {
-		return workload.ProxyMode
-	}
-
-	// For all other cases (direct sse, streamable-http, or when no proxy mode is set),
-	// return the transport type directly
-	return workload.TransportType.String()
-}

cmd/thv/app/list_test.go

@@ -27,6 +27,10 @@ type Workload struct {
 	ToolType string `json:"tool_type"`
 	// TransportType is the type of transport used for this workload.
 	TransportType types.TransportType `json:"transport_type"`
+	// EffectiveTransportType is the transport type that clients should use to connect.
+	// For stdio transports with proxy mode, this will be the proxy mode (sse/streamable-http).
+	// For direct transports, this will be the same as TransportType.
+	EffectiveTransportType types.TransportType `json:"effective_transport_type"`
 	// ProxyMode is the proxy mode for stdio transport (sse or streamable-http).
 	ProxyMode string `json:"proxy_mode,omitempty"`
 	// Status is the current status of the workload.

pkg/core/workload.go

@@ -1173,20 +1173,24 @@ func (d *defaultManager) getRemoteWorkloadsFromState(
 			)
 		}
 
+		// Calculate effective transport type
+		effectiveTransportType := types.GetEffectiveTransportType(transportType, string(runConfig.ProxyMode))
+
 		// Create a workload from the run configuration
 		workload := core.Workload{
-			Name:          name,
-			Package:       "remote",
-			Status:        workloadStatus.Status,
-			URL:           proxyURL,
-			Port:          runConfig.Port,
-			TransportType: transportType,
-			ProxyMode:     string(runConfig.ProxyMode),
-			ToolType:      "remote",
-			Group:         runConfig.Group,
-			CreatedAt:     workloadStatus.CreatedAt,
-			Labels:        runConfig.ContainerLabels,
-			Remote:        true,
+			Name:                   name,
+			Package:                "remote",
+			Status:                 workloadStatus.Status,
+			URL:                    proxyURL,
+			Port:                   runConfig.Port,
+			TransportType:          transportType,
+			EffectiveTransportType: effectiveTransportType,
+			ProxyMode:              string(runConfig.ProxyMode),
+			ToolType:               "remote",
+			Group:                  runConfig.Group,
+			CreatedAt:              workloadStatus.CreatedAt,
+			Labels:                 runConfig.ContainerLabels,
+			Remote:                 true,
 		}
 
 		// Apply label filtering

pkg/workloads/manager.go

@@ -0,0 +1,66 @@
+package types
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/stacklok/toolhive/pkg/transport/types"
+)
+
+func TestGetEffectiveTransportType(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name              string
+		transportType     types.TransportType
+		proxyMode         string
+		expectedTransport types.TransportType
+	}{
+		{
+			name:              "stdio transport with sse proxy mode should return sse",
+			transportType:     types.TransportTypeStdio,
+			proxyMode:         "sse",
+			expectedTransport: types.TransportTypeSSE,
+		},
+		{
+			name:              "stdio transport with streamable-http proxy mode should return streamable-http",
+			transportType:     types.TransportTypeStdio,
+			proxyMode:         "streamable-http",
+			expectedTransport: types.TransportTypeStreamableHTTP,
+		},
+		{
+			name:              "stdio transport with empty proxy mode should return stdio",
+			transportType:     types.TransportTypeStdio,
+			proxyMode:         "",
+			expectedTransport: types.TransportTypeStdio,
+		},
+		{
+			name:              "sse transport should return sse regardless of proxy mode",
+			transportType:     types.TransportTypeSSE,
+			proxyMode:         "streamable-http", // This shouldn't matter for non-stdio
+			expectedTransport: types.TransportTypeSSE,
+		},
+		{
+			name:              "streamable-http transport should return streamable-http",
+			transportType:     types.TransportTypeStreamableHTTP,
+			proxyMode:         "",
+			expectedTransport: types.TransportTypeStreamableHTTP,
+		},
+		{
+			name:              "stdio transport with invalid proxy mode should return stdio",
+			transportType:     types.TransportTypeStdio,
+			proxyMode:         "invalid-mode",
+			expectedTransport: types.TransportTypeStdio,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			result := GetEffectiveTransportType(tt.transportType, tt.proxyMode)
+			assert.Equal(t, tt.expectedTransport, result, "Effective transport type should match expected")
+		})
+	}
+}

pkg/workloads/types/effective_transport_test.go

@@ -95,19 +95,39 @@ func WorkloadFromContainerInfo(container *runtime.ContainerInfo) (core.Workload,
 		return core.Workload{}, err
 	}
 
+	// Calculate effective transport type
+	effectiveTransportType := GetEffectiveTransportType(tType, runConfig.ProxyMode)
+
 	// Translate to the domain model.
 	return core.Workload{
-		Name:          name, // Use the calculated workload name (base name), not container name
-		Package:       container.Image,
-		URL:           url,
-		ToolType:      toolType,
-		TransportType: tType,
-		ProxyMode:     runConfig.ProxyMode,
-		Status:        container.State,
-		StatusContext: container.Status,
-		CreatedAt:     container.Created,
-		Port:          port,
-		Labels:        userLabels,
-		Group:         runConfig.Group,
+		Name:                   name, // Use the calculated workload name (base name), not container name
+		Package:                container.Image,
+		URL:                    url,
+		ToolType:               toolType,
+		TransportType:          tType,
+		EffectiveTransportType: effectiveTransportType,
+		ProxyMode:              runConfig.ProxyMode,
+		Status:                 container.State,
+		StatusContext:          container.Status,
+		CreatedAt:              container.Created,
+		Port:                   port,
+		Labels:                 userLabels,
+		Group:                  runConfig.Group,
 	}, nil
 }
+
+// GetEffectiveTransportType determines the effective transport type that clients should use
+// This handles the case where stdio MCP servers are proxied through SSE or streamable-http
+func GetEffectiveTransportType(transportType types.TransportType, proxyMode string) types.TransportType {
+	// If the underlying transport is stdio and we have a proxy mode set,
+	// return the proxy mode as that's what clients actually connect to
+	if transportType == types.TransportTypeStdio && proxyMode != "" {
+		if proxyType, err := types.ParseTransportType(proxyMode); err == nil {
+			return proxyType
+		}
+	}
+
+	// For all other cases (direct sse, streamable-http, or when no proxy mode is set),
+	// return the transport type directly
+	return transportType
+}