Hide already registered clients in thv client setup

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>

Author: carlos-gn

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 {

cmd/thv/app/ui/clients_setup.go

@@ -5,7 +5,9 @@
 package ui
 
 import (
+	"errors"
 	"fmt"
+	"sort"
 	"strings"
 
 	tea "github.com/charmbracelet/bubbletea"
@@ -15,6 +17,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,13 +35,18 @@ const (
 )
 
 type setupModel struct {
+	// UnfilteredClients holds all installed clients before group-based filtering.
+	UnfilteredClients []client.ClientAppStatus
+	// Clients holds the clients displayed in the selection list. After filtering,
+	// SelectedClients indices refer to positions in this slice (not UnfilteredClients).
 	Clients         []client.ClientAppStatus
 	Groups          []*groups.Group
 	Cursor          int
 	SelectedClients map[int]struct{}
 	SelectedGroups  map[int]struct{}
 	Quitting        bool
 	Confirmed       bool
+	AllFiltered     bool
 	CurrentStep     setupStep
 }
 
@@ -63,9 +74,15 @@ 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 {
+					m.AllFiltered = true
+					m.Quitting = true
+					return m, tea.Quit
+				}
 				return m, nil
 			}
 			// Final confirmation
@@ -114,11 +131,7 @@ func (m *setupModel) View() string {
 		b.WriteString("\nUse ↑/↓ (or j/k) to move, 'space' to select, 'enter' to continue, 'q' to quit.\n")
 	} else {
 		if len(m.SelectedGroups) > 0 {
-			var selectedGroupNames []string
-			for i := range m.SelectedGroups {
-				selectedGroupNames = append(selectedGroupNames, m.Groups[i].Name)
-			}
-			fmt.Fprintf(&b, "Selected groups: %s\n\n", strings.Join(selectedGroupNames, ", "))
+			fmt.Fprintf(&b, "Selected groups: %s\n\n", strings.Join(m.sortedSelectedGroupNames(), ", "))
 		}
 		b.WriteString("Select clients to register:\n\n")
 		for i, cli := range m.Clients {
@@ -130,6 +143,33 @@ func (m *setupModel) View() string {
 	return docStyle.Render(b.String())
 }
 
+// filterClientsBySelectedGroups replaces Clients with a filtered subset
+// that excludes clients already registered in all selected groups, and
+// resets SelectedClients since the indices would no longer be valid.
+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 = client.FilterClientsAlreadyRegistered(m.UnfilteredClients, selectedGroups)
+	m.SelectedClients = make(map[int]struct{})
+}
+
+// sortedSelectedGroupNames returns selected group names in sorted order.
+func (m *setupModel) sortedSelectedGroupNames() []string {
+	names := make([]string, 0, len(m.SelectedGroups))
+	for i := range m.SelectedGroups {
+		names = append(names, m.Groups[i].Name)
+	}
+	sort.Strings(names)
+	return names
+}
+
 func renderGroupRow(m *setupModel, i int, group *groups.Group) string {
 	cursor := "  "
 	if m.Cursor == i {
@@ -182,12 +222,31 @@ 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 = client.FilterClientsAlreadyRegistered(clients, selectedGroups)
+		if len(displayClients) == 0 {
+			groupNames := make([]string, 0, len(selectedGroupsMap))
+			for i := range selectedGroupsMap {
+				groupNames = append(groupNames, availableGroups[i].Name)
+			}
+			sort.Strings(groupNames)
+			return nil, groupNames, false, ErrAllClientsRegistered
+		}
+	}
+
 	model := &setupModel{
-		Clients:         clients,
-		Groups:          availableGroups,
-		SelectedClients: make(map[int]struct{}),
-		SelectedGroups:  selectedGroupsMap,
-		CurrentStep:     currentStep,
+		UnfilteredClients: clients,
+		Clients:           displayClients,
+		Groups:            availableGroups,
+		SelectedClients:   make(map[int]struct{}),
+		SelectedGroups:    selectedGroupsMap,
+		CurrentStep:       currentStep,
 	}
 
 	p := tea.NewProgram(model)
@@ -197,16 +256,19 @@ func RunClientSetup(
 	}
 
 	m := finalModel.(*setupModel)
+
+	if m.AllFiltered {
+		groupNames := m.sortedSelectedGroupNames()
+		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
-	var selectedGroupNames []string
-	for i := range m.SelectedGroups {
-		selectedGroupNames = append(selectedGroupNames, m.Groups[i].Name)
-	}
+	selectedGroupNames := m.sortedSelectedGroupNames()
 
 	return selectedClients, selectedGroupNames, m.Confirmed, nil
 }

cmd/thv/app/ui/clients_setup_test.go

@@ -0,0 +1,138 @@
+// 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 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
+		wantAllFiltered 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,
+			wantAllFiltered: false,
+			wantClientCount: 2, // cursor and claude-code remain
+		},
+		{
+			name: "sets AllFiltered 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,
+			wantAllFiltered: 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,
+			wantAllFiltered: false,
+			wantClientCount: 1,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			m := &setupModel{
+				UnfilteredClients: 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.Equal(t, tt.wantAllFiltered, result.AllFiltered)
+			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{
+		UnfilteredClients: 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)
+	assert.False(t, result.AllFiltered)
+	require.NotNil(t, cmd, "should return a quit command")
+}

pkg/client/filter.go

@@ -0,0 +1,44 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package client
+
+import (
+	"github.com/stacklok/toolhive/pkg/groups"
+)
+
+// FilterClientsAlreadyRegistered returns only clients that are NOT already
+// registered in all of the provided groups. A client is excluded only when
+// every group in selectedGroups already lists it in RegisteredClients.
+func FilterClientsAlreadyRegistered(
+	clients []ClientAppStatus,
+	selectedGroups []*groups.Group,
+) []ClientAppStatus {
+	if len(selectedGroups) == 0 {
+		return clients
+	}
+
+	var filtered []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
+}