Hide already registered clients in thv client setup

carlos-gn · carlos-gn · commit 8931db68e91e · 2026-02-18T21:29:25.000+01:00
When selecting clients in `thv client setup`, clients that are already registered for all selected groups are now filtered out of the selection list. If all installed clients are already registered, the command exits with a clear message instead of showing an empty list. Closes #1308 Signed-off-by: carlos <21148423+carlos-gn@users.noreply.github.com>
diff --git a/cmd/thv/app/client.go b/cmd/thv/app/client.go
@@ -5,6 +5,7 @@ package app
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sort"
 
@@ -124,6 +125,10 @@ func clientSetupCmdFunc(cmd *cobra.Command, _ []string) error {
 
 	selectedClients, selectedGroups, confirmed, err := ui.RunClientSetup(availableClients, availableGroups)
 	if err != nil {
+		if errors.Is(err, ui.ErrAllClientsRegistered) {
+			fmt.Println("All installed clients are already registered for the selected groups.")
+			return nil
+		}
 		return fmt.Errorf("error running interactive setup: %w", err)
 	}
 	if !confirmed {
diff --git a/cmd/thv/app/ui/clients_setup.go b/cmd/thv/app/ui/clients_setup.go
@@ -5,6 +5,7 @@
 package ui
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 
@@ -15,6 +16,10 @@ import (
 	"github.com/stacklok/toolhive/pkg/groups"
 )
 
+// ErrAllClientsRegistered is returned when all available clients are already
+// registered for the selected groups.
+var ErrAllClientsRegistered = errors.New("all installed clients are already registered for the selected groups")
+
 var (
 	docStyle          = lipgloss.NewStyle().Margin(1, 2)
 	selectedItemStyle = lipgloss.NewStyle().PaddingLeft(2).Foreground(lipgloss.Color("170"))
@@ -29,6 +34,7 @@ const (
 )
 
 type setupModel struct {
+	AllClients      []client.ClientAppStatus
 	Clients         []client.ClientAppStatus
 	Groups          []*groups.Group
 	Cursor          int
@@ -63,9 +69,16 @@ func (m *setupModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 				if len(m.SelectedGroups) == 0 {
 					return m, nil // Stay on group selection step
 				}
-				// Move to client selection step
+				// Filter clients and move to client selection step
+				m.filterClientsBySelectedGroups()
 				m.CurrentStep = stepClientSelection
 				m.Cursor = 0
+				if len(m.Clients) == 0 {
+					// All clients already registered for selected groups
+					m.Confirmed = false
+					m.Quitting = true
+					return m, tea.Quit
+				}
 				return m, nil
 			}
 			// Final confirmation
@@ -130,6 +143,58 @@ func (m *setupModel) View() string {
 	return docStyle.Render(b.String())
 }
 
+// filterClientsBySelectedGroups removes clients that are already registered
+// in all selected groups. A client is hidden only when every selected group
+// already lists it in RegisteredClients.
+func (m *setupModel) filterClientsBySelectedGroups() {
+	if len(m.SelectedGroups) == 0 {
+		return
+	}
+
+	var selectedGroups []*groups.Group
+	for i := range m.SelectedGroups {
+		selectedGroups = append(selectedGroups, m.Groups[i])
+	}
+
+	m.Clients = FilterClientsAlreadyRegistered(m.AllClients, selectedGroups)
+	m.SelectedClients = make(map[int]struct{})
+}
+
+// FilterClientsAlreadyRegistered returns only clients that are NOT already
+// registered in all of the provided groups. Exported for testing.
+func FilterClientsAlreadyRegistered(
+	clients []client.ClientAppStatus,
+	selectedGroups []*groups.Group,
+) []client.ClientAppStatus {
+	if len(selectedGroups) == 0 {
+		return clients
+	}
+
+	var filtered []client.ClientAppStatus
+	for _, cli := range clients {
+		if !isClientRegisteredInAllGroups(string(cli.ClientType), selectedGroups) {
+			filtered = append(filtered, cli)
+		}
+	}
+	return filtered
+}
+
+func isClientRegisteredInAllGroups(clientName string, selectedGroups []*groups.Group) bool {
+	for _, group := range selectedGroups {
+		found := false
+		for _, registered := range group.RegisteredClients {
+			if registered == clientName {
+				found = true
+				break
+			}
+		}
+		if !found {
+			return false
+		}
+	}
+	return true
+}
+
 func renderGroupRow(m *setupModel, i int, group *groups.Group) string {
 	cursor := "  "
 	if m.Cursor == i {
@@ -182,8 +247,27 @@ func RunClientSetup(
 		currentStep = stepGroupSelection
 	}
 
+	// When skipping group selection, filter out already-registered clients
+	displayClients := clients
+	if currentStep == stepClientSelection && len(selectedGroupsMap) > 0 {
+		var selectedGroups []*groups.Group
+		for i := range selectedGroupsMap {
+			selectedGroups = append(selectedGroups, availableGroups[i])
+		}
+		displayClients = FilterClientsAlreadyRegistered(clients, selectedGroups)
+		if len(displayClients) == 0 {
+			// All clients are already registered for the auto-selected group(s)
+			var groupNames []string
+			for i := range selectedGroupsMap {
+				groupNames = append(groupNames, availableGroups[i].Name)
+			}
+			return nil, groupNames, false, ErrAllClientsRegistered
+		}
+	}
+
 	model := &setupModel{
-		Clients:         clients,
+		AllClients:      clients,
+		Clients:         displayClients,
 		Groups:          availableGroups,
 		SelectedClients: make(map[int]struct{}),
 		SelectedGroups:  selectedGroupsMap,
@@ -197,9 +281,19 @@ func RunClientSetup(
 	}
 
 	m := finalModel.(*setupModel)
+
+	// Check if we quit because all clients were filtered out
+	if !m.Confirmed && len(m.Clients) == 0 && m.CurrentStep == stepClientSelection {
+		var groupNames []string
+		for i := range m.SelectedGroups {
+			groupNames = append(groupNames, m.Groups[i].Name)
+		}
+		return nil, groupNames, false, ErrAllClientsRegistered
+	}
+
 	var selectedClients []client.ClientAppStatus
 	for i := range m.SelectedClients {
-		selectedClients = append(selectedClients, clients[i])
+		selectedClients = append(selectedClients, m.Clients[i])
 	}
 
 	// Convert selected group indices back to group names
diff --git a/cmd/thv/app/ui/clients_setup_test.go b/cmd/thv/app/ui/clients_setup_test.go
@@ -0,0 +1,213 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package ui
+
+import (
+	"testing"
+
+	tea "github.com/charmbracelet/bubbletea"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/stacklok/toolhive/pkg/client"
+	"github.com/stacklok/toolhive/pkg/groups"
+)
+
+func TestFilterClientsAlreadyRegistered(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name           string
+		clients        []client.ClientAppStatus
+		selectedGroups []*groups.Group
+		wantClients    []client.ClientApp
+	}{
+		{
+			name: "no groups selected returns all clients",
+			clients: []client.ClientAppStatus{
+				{ClientType: client.VSCode, Installed: true},
+				{ClientType: client.Cursor, Installed: true},
+			},
+			selectedGroups: nil,
+			wantClients:    []client.ClientApp{client.VSCode, client.Cursor},
+		},
+		{
+			name: "client registered in all selected groups is hidden",
+			clients: []client.ClientAppStatus{
+				{ClientType: client.VSCode, Installed: true},
+				{ClientType: client.Cursor, Installed: true},
+			},
+			selectedGroups: []*groups.Group{
+				{Name: "group1", RegisteredClients: []string{"vscode", "cursor"}},
+				{Name: "group2", RegisteredClients: []string{"vscode"}},
+			},
+			wantClients: []client.ClientApp{client.Cursor},
+		},
+		{
+			name: "client registered in only some groups is kept",
+			clients: []client.ClientAppStatus{
+				{ClientType: client.VSCode, Installed: true},
+				{ClientType: client.Cursor, Installed: true},
+			},
+			selectedGroups: []*groups.Group{
+				{Name: "group1", RegisteredClients: []string{"vscode"}},
+				{Name: "group2", RegisteredClients: []string{"cursor"}},
+			},
+			wantClients: []client.ClientApp{client.VSCode, client.Cursor},
+		},
+		{
+			name: "all clients already registered returns empty",
+			clients: []client.ClientAppStatus{
+				{ClientType: client.VSCode, Installed: true},
+				{ClientType: client.Cursor, Installed: true},
+			},
+			selectedGroups: []*groups.Group{
+				{Name: "group1", RegisteredClients: []string{"vscode", "cursor"}},
+			},
+			wantClients: nil,
+		},
+		{
+			name: "single group with no registered clients returns all",
+			clients: []client.ClientAppStatus{
+				{ClientType: client.VSCode, Installed: true},
+				{ClientType: client.Cursor, Installed: true},
+			},
+			selectedGroups: []*groups.Group{
+				{Name: "group1", RegisteredClients: []string{}},
+			},
+			wantClients: []client.ClientApp{client.VSCode, client.Cursor},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			result := FilterClientsAlreadyRegistered(tt.clients, tt.selectedGroups)
+
+			var gotClients []client.ClientApp
+			for _, c := range result {
+				gotClients = append(gotClients, c.ClientType)
+			}
+
+			assert.Equal(t, tt.wantClients, gotClients)
+		})
+	}
+}
+
+func TestSetupModelUpdate_GroupToClientTransition(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name            string
+		allClients      []client.ClientAppStatus
+		grps            []*groups.Group
+		selectedGroups  map[int]struct{}
+		wantStep        setupStep
+		wantQuitting    bool
+		wantClientCount int
+	}{
+		{
+			name: "filters already-registered clients on transition",
+			allClients: []client.ClientAppStatus{
+				{ClientType: client.VSCode, Installed: true},
+				{ClientType: client.Cursor, Installed: true},
+				{ClientType: client.ClaudeCode, Installed: true},
+			},
+			grps: []*groups.Group{
+				{Name: "group1", RegisteredClients: []string{"vscode"}},
+			},
+			selectedGroups:  map[int]struct{}{0: {}},
+			wantStep:        stepClientSelection,
+			wantQuitting:    false,
+			wantClientCount: 2, // cursor and claude-code remain
+		},
+		{
+			name: "quits when all clients are already registered",
+			allClients: []client.ClientAppStatus{
+				{ClientType: client.VSCode, Installed: true},
+				{ClientType: client.Cursor, Installed: true},
+			},
+			grps: []*groups.Group{
+				{Name: "group1", RegisteredClients: []string{"vscode", "cursor"}},
+			},
+			selectedGroups:  map[int]struct{}{0: {}},
+			wantStep:        stepClientSelection,
+			wantQuitting:    true,
+			wantClientCount: 0,
+		},
+		{
+			name: "does not transition without group selection",
+			allClients: []client.ClientAppStatus{
+				{ClientType: client.VSCode, Installed: true},
+			},
+			grps: []*groups.Group{
+				{Name: "group1", RegisteredClients: []string{}},
+			},
+			selectedGroups:  map[int]struct{}{}, // none selected
+			wantStep:        stepGroupSelection, // stays on group step
+			wantQuitting:    false,
+			wantClientCount: 1,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			m := &setupModel{
+				AllClients:      tt.allClients,
+				Clients:         tt.allClients,
+				Groups:          tt.grps,
+				SelectedClients: make(map[int]struct{}),
+				SelectedGroups:  tt.selectedGroups,
+				CurrentStep:     stepGroupSelection,
+			}
+
+			// Press enter to transition
+			updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyEnter})
+			result := updated.(*setupModel)
+
+			assert.Equal(t, tt.wantStep, result.CurrentStep)
+			assert.Equal(t, tt.wantQuitting, result.Quitting)
+			assert.Len(t, result.Clients, tt.wantClientCount)
+		})
+	}
+}
+
+func TestSetupModelUpdate_ClientSelection(t *testing.T) {
+	t.Parallel()
+
+	clients := []client.ClientAppStatus{
+		{ClientType: client.VSCode, Installed: true},
+		{ClientType: client.Cursor, Installed: true},
+	}
+
+	m := &setupModel{
+		AllClients:      clients,
+		Clients:         clients,
+		Groups:          []*groups.Group{{Name: "g1"}},
+		SelectedClients: make(map[int]struct{}),
+		SelectedGroups:  map[int]struct{}{0: {}},
+		CurrentStep:     stepClientSelection,
+	}
+
+	// Toggle first client with space
+	updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{' '}})
+	result := updated.(*setupModel)
+	_, selected := result.SelectedClients[0]
+	assert.True(t, selected, "first client should be selected after space")
+
+	// Toggle it off
+	updated, _ = result.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{' '}})
+	result = updated.(*setupModel)
+	_, selected = result.SelectedClients[0]
+	assert.False(t, selected, "first client should be deselected after second space")
+
+	// Confirm with enter
+	updated, cmd := result.Update(tea.KeyMsg{Type: tea.KeyEnter})
+	result = updated.(*setupModel)
+	assert.True(t, result.Confirmed)
+	assert.True(t, result.Quitting)
+	require.NotNil(t, cmd, "should return a quit command")
+}
