Sort clients in CLI commands (#1228)

Signed-off-by: Dan Barr <[email protected]>
Co-authored-by: Dan Barr <[email protected]>

Author: danbarr

cmd/thv/app/client.go

@@ -2,6 +2,7 @@ package app
 
 import (
 	"fmt"
+	"sort"
 
 	"github.com/spf13/cobra"
 
@@ -217,8 +218,14 @@ func listRegisteredClientsCmdFunc(_ *cobra.Command, _ []string) error {
 		fmt.Println("No clients are currently registered.")
 		return nil
 	}
+
+	// Create a copy of the registered clients and sort it alphabetically
+	registeredClients := make([]string, len(cfg.Clients.RegisteredClients))
+	copy(registeredClients, cfg.Clients.RegisteredClients)
+	sort.Strings(registeredClients)
+
 	fmt.Println("Registered clients:")
-	for _, clientName := range cfg.Clients.RegisteredClients {
+	for _, clientName := range registeredClients {
 		fmt.Printf("  - %s\n", clientName)
 	}
 	return nil

cmd/thv/app/client_test.go

@@ -2,6 +2,7 @@ package app
 
 import (
 	"bytes"
+	"io"
 	"os"
 	"strings"
 	"testing"
@@ -79,3 +80,56 @@ func TestClientRemoveCmd_InvalidClient(t *testing.T) { //nolint:paralleltest //
 	assert.Error(t, err)
 	assert.True(t, strings.Contains(err.Error(), "invalid client type"))
 }
+
+func TestListRegisteredClientsCmd_Sorting(t *testing.T) { //nolint:paralleltest // Uses environment variables
+	tempDir := t.TempDir()
+	os.Setenv("XDG_CONFIG_HOME", tempDir)
+
+	// Pre-populate config with multiple registered clients in non-alphabetical order
+	testClients := []string{"vscode", "cursor", "roo-code", "cline", "claude-code"}
+	err := config.UpdateConfig(func(c *config.Config) {
+		c.Clients.RegisteredClients = testClients
+	})
+	assert.NoError(t, err)
+
+	// Temporarily redirect stdout to capture the output
+	oldStdout := os.Stdout
+	r, w, _ := os.Pipe()
+	os.Stdout = w
+
+	// Call the function directly
+	err = listRegisteredClientsCmdFunc(nil, nil)
+	assert.NoError(t, err)
+
+	// Restore stdout and read the captured output
+	w.Close()
+	os.Stdout = oldStdout
+
+	outputBytes, err := io.ReadAll(r)
+	assert.NoError(t, err)
+	outputStr := string(outputBytes)
+
+	// Extract client names from output (they appear after "- ")
+	lines := strings.Split(outputStr, "\n")
+	var foundClients []string
+	for _, line := range lines {
+		line = strings.TrimSpace(line)
+		if strings.HasPrefix(line, "- ") {
+			client := strings.TrimPrefix(line, "- ")
+			foundClients = append(foundClients, client)
+		}
+	}
+
+	// Verify all clients are present
+	assert.Len(t, foundClients, len(testClients), "Should find all registered clients")
+	for _, expectedClient := range testClients {
+		assert.Contains(t, foundClients, expectedClient, "Should contain client: %s", expectedClient)
+	}
+
+	// Verify alphabetical order
+	for i := 1; i < len(foundClients); i++ {
+		assert.True(t, foundClients[i-1] < foundClients[i],
+			"Clients should be sorted alphabetically: %s should come before %s",
+			foundClients[i-1], foundClients[i])
+	}
+}

pkg/client/discovery.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"sort"
 
 	"github.com/stacklok/toolhive/pkg/config"
 )
@@ -65,6 +66,11 @@ func GetClientStatus() ([]MCPClientStatus, error) {
 		statuses = append(statuses, status)
 	}
 
+	// Sort statuses alphabetically by ClientType
+	sort.Slice(statuses, func(i, j int) bool {
+		return string(statuses[i].ClientType) < string(statuses[j].ClientType)
+	})
+
 	return statuses, nil
 }
 

pkg/client/discovery_test.go

@@ -13,13 +13,11 @@ import (
 
 func TestGetClientStatus(t *testing.T) {
 	// Setup a temporary home directory for testing
-	origHome := os.Getenv("HOME")
 	tempHome, err := os.MkdirTemp("", "toolhive-test-home")
 	require.NoError(t, err)
 	defer os.RemoveAll(tempHome)
 
 	t.Setenv("HOME", tempHome)
-	defer t.Setenv("HOME", origHome)
 
 	// Create mock config with registered clients
 	mockConfig := &config.Config{
@@ -63,3 +61,37 @@ func TestGetClientStatus(t *testing.T) {
 	assert.False(t, vscodeStatus.Installed)
 	assert.False(t, vscodeStatus.Registered)
 }
+
+func TestGetClientStatus_Sorting(t *testing.T) {
+	// Setup a temporary home directory for testing
+	origHome := os.Getenv("HOME")
+	tempHome, err := os.MkdirTemp("", "toolhive-test-home")
+	require.NoError(t, err)
+	defer os.RemoveAll(tempHome)
+
+	t.Setenv("HOME", tempHome)
+	defer t.Setenv("HOME", origHome)
+
+	// Create mock config with no registered clients
+	mockConfig := &config.Config{
+		Clients: config.Clients{
+			RegisteredClients: []string{},
+		},
+	}
+	cleanup := MockConfig(t, mockConfig)
+	defer cleanup()
+
+	statuses, err := GetClientStatus()
+	require.NoError(t, err)
+	require.NotNil(t, statuses)
+	require.Greater(t, len(statuses), 1, "Need at least 2 clients to test sorting")
+
+	// Verify that the statuses are sorted alphabetically by ClientType
+	for i := 1; i < len(statuses); i++ {
+		prevClient := string(statuses[i-1].ClientType)
+		currClient := string(statuses[i].ClientType)
+		assert.True(t, prevClient < currClient,
+			"Client statuses should be sorted alphabetically: %s should come before %s",
+			prevClient, currClient)
+	}
+}