refactor(huh prompter): custom Field for MultiSelectWithSearch

BagToad · Copilot · williammartin · commit cfb2224176d8 · 2026-03-26T14:26:17.000+01:00
Replace the OptionsFunc-based MultiSelectWithSearch with a custom huh
Field implementation. huh's OptionsFunc runs in a goroutine, causing
data races with selection state and stale cache issues that made
selections disappear on toggle or search changes.

The custom field (multiSelectSearchField) combines a text input and
multi-select list in a single field with full control over the update
loop. Search runs asynchronously via tea.Cmd when the user presses
Enter, with a themed spinner during loading. Selections are stored in
a simple map — no goroutine races, no Eval cache, no syncAccessor.

Also adds defensive validation for mismatched Keys/Labels slices from
searchFunc.

Co-authored-by: Copilot &lt;223556219+Copilot@users.noreply.github.com&gt;
diff --git a/internal/prompter/huh_prompter.go b/internal/prompter/huh_prompter.go
@@ -3,7 +3,6 @@ package prompter
 import (
 	"fmt"
 	"slices"
-	"sync"
 
 	"charm.land/huh/v2"
 	"github.com/cli/cli/v2/internal/ghinstance"
@@ -93,162 +92,19 @@ func (p *huhPrompter) MultiSelect(prompt string, defaults []string, options []st
 	return *result, nil
 }
 
-// searchOptionsBinding is used as the OptionsFunc binding for MultiSelectWithSearch.
-// By including both the search query and selected values, the binding hash changes
-// whenever either changes. This prevents huh's internal Eval cache from serving
-// stale option sets that would overwrite the user's current selections.
-type searchOptionsBinding struct {
-	Query    *string
-	Selected *[]string
-}
-
-// syncAccessor is a thread-safe huh.Accessor implementation.
-// huh calls OptionsFunc from a goroutine while the main event loop
-// writes field values via Set(). This accessor synchronizes both
-// paths through the same mutex.
-type syncAccessor[T any] struct {
-	mu    *sync.Mutex
-	value T
-}
-
-func (a *syncAccessor[T]) Get() T {
-	a.mu.Lock()
-	defer a.mu.Unlock()
-	return a.value
-}
-
-func (a *syncAccessor[T]) Set(value T) {
-	a.mu.Lock()
-	defer a.mu.Unlock()
-	a.value = value
-}
-
-func (p *huhPrompter) buildMultiSelectWithSearchForm(prompt, searchPrompt string, defaultValues, persistentValues []string, searchFunc func(string) MultiSelectSearchResult) (*huh.Form, *syncAccessor[[]string]) {
-	var mu sync.Mutex
-
-	queryAccessor := &syncAccessor[string]{mu: &mu}
-	selectAccessor := &syncAccessor[[]string]{mu: &mu, value: slices.Clone(defaultValues)}
-
-	optionKeyLabels := make(map[string]string)
-	for _, k := range defaultValues {
-		optionKeyLabels[k] = k
-	}
-
-	// Cache searchFunc results locally keyed by query string.
-	// This avoids redundant calls when the OptionsFunc binding hash changes
-	// due to selection changes (not query changes).
-	searchCacheValid := false
-	var cachedSearchQuery string
-	var cachedSearchResult MultiSelectSearchResult
-
-	buildOptions := func() []huh.Option[string] {
-		mu.Lock()
-		query := queryAccessor.value
-		needsFetch := !searchCacheValid || query != cachedSearchQuery
-		mu.Unlock()
-
-		if needsFetch {
-			result := searchFunc(query)
-			mu.Lock()
-			cachedSearchResult = result
-			cachedSearchQuery = query
-			searchCacheValid = true
-			mu.Unlock()
-		}
-
-		mu.Lock()
-		defer mu.Unlock()
-
-		selectedValues := selectAccessor.value
-		result := cachedSearchResult
-
-		if result.Err != nil {
-			return nil
-		}
-		for i, k := range result.Keys {
-			optionKeyLabels[k] = result.Labels[i]
-		}
-
-		var formOptions []huh.Option[string]
-		seen := make(map[string]bool)
-
-		// 1. Currently selected values (persisted across searches).
-		for _, k := range selectedValues {
-			if seen[k] {
-				continue
-			}
-			seen[k] = true
-			l := optionKeyLabels[k]
-			if l == "" {
-				l = k
-			}
-			formOptions = append(formOptions, huh.NewOption(l, k).Selected(true))
-		}
-
-		// 2. Search results.
-		for i, k := range result.Keys {
-			if seen[k] {
-				continue
-			}
-			seen[k] = true
-			l := result.Labels[i]
-			if l == "" {
-				l = k
-			}
-			formOptions = append(formOptions, huh.NewOption(l, k))
-		}
-
-		// 3. Persistent options.
-		for _, k := range persistentValues {
-			if seen[k] {
-				continue
-			}
-			seen[k] = true
-			l := optionKeyLabels[k]
-			if l == "" {
-				l = k
-			}
-			formOptions = append(formOptions, huh.NewOption(l, k))
-		}
-
-		if len(formOptions) == 0 {
-			formOptions = append(formOptions, huh.NewOption("No results", ""))
-		}
-
-		return formOptions
-	}
-
-	binding := &searchOptionsBinding{
-		Query:    &queryAccessor.value,
-		Selected: &selectAccessor.value,
-	}
-
-	form := p.newForm(
-		huh.NewGroup(
-			huh.NewInput().
-				Title(searchPrompt).
-				Placeholder("Type to search, Ctrl+U to clear").
-				Accessor(queryAccessor),
-			huh.NewMultiSelect[string]().
-				Title(prompt).
-				Options(buildOptions()...).
-				OptionsFunc(func() []huh.Option[string] {
-					return buildOptions()
-				}, binding).
-				Accessor(selectAccessor).
-				Limit(0),
-		),
-	)
-	return form, selectAccessor
+func (p *huhPrompter) buildMultiSelectWithSearchForm(prompt, searchPrompt string, defaultValues, persistentValues []string, searchFunc func(string) MultiSelectSearchResult) (*huh.Form, *multiSelectSearchField) {
+	field := newMultiSelectSearchField(prompt, searchPrompt, defaultValues, persistentValues, searchFunc)
+	form := p.newForm(huh.NewGroup(field))
+	return form, field
 }
 
 func (p *huhPrompter) MultiSelectWithSearch(prompt, searchPrompt string, defaultValues, persistentValues []string, searchFunc func(string) MultiSelectSearchResult) ([]string, error) {
-	form, accessor := p.buildMultiSelectWithSearchForm(prompt, searchPrompt, defaultValues, persistentValues, searchFunc)
+	form, field := p.buildMultiSelectWithSearchForm(prompt, searchPrompt, defaultValues, persistentValues, searchFunc)
 	err := form.Run()
 	if err != nil {
 		return nil, err
 	}
-	return accessor.Get(), nil
+	return field.selectedKeys(), nil
 }
 
 func (p *huhPrompter) buildInputForm(prompt, defaultValue string) (*huh.Form, *string) {
diff --git a/internal/prompter/huh_prompter_test.go b/internal/prompter/huh_prompter_test.go
@@ -450,7 +450,7 @@ func TestHuhPrompterMultiSelectWithSearch(t *testing.T) {
 				"Select", "Search", tt.defaults, tt.persistent, staticSearchFunc,
 			)
 			runForm(t, f, tt.ix)
-			assert.Equal(t, tt.wantResult, result.Get())
+			assert.Equal(t, tt.wantResult, result.selectedKeys())
 		})
 	}
 }
@@ -482,7 +482,7 @@ func TestHuhPrompterMultiSelectWithSearchPersistence(t *testing.T) {
 			tab(), waitForOptions(),
 			enter(), // submit — result-a should persist
 		))
-		assert.Equal(t, []string{"result-a"}, result.Get())
+		assert.Equal(t, []string{"result-a"}, result.selectedKeys())
 	})
 	t.Run("empty search results shows no-results placeholder", func(t *testing.T) {
 		emptySearchFunc := func(query string) MultiSelectSearchResult {
@@ -492,10 +492,10 @@ func TestHuhPrompterMultiSelectWithSearchPersistence(t *testing.T) {
 		f, result := p.buildMultiSelectWithSearchForm(
 			"Select", "Search", nil, nil, emptySearchFunc,
 		)
-		// With no results, the "No results" placeholder is shown but nothing
-		// is selected, so submitting returns empty.
+		// With no results, the "No results" message is shown.
+		// Toggle does nothing, submitting returns empty.
 		runForm(t, f, newInteraction(tab(), waitForOptions(), toggle(), enter()))
-		assert.Equal(t, []string{""}, result.Get())
+		assert.Equal(t, []string{}, result.selectedKeys())
 	})
 }
 
@@ -581,3 +581,40 @@ func TestHuhPrompterInputHostname(t *testing.T) {
 		})
 	}
 }
+
+func TestHuhPrompterMultiSelectWithSearchBackspace(t *testing.T) {
+	// Simulate real API latency and non-overlapping results.
+	staticSearchFunc := func(query string) MultiSelectSearchResult {
+		time.Sleep(100 * time.Millisecond) // simulate API latency
+		if query == "" {
+			return MultiSelectSearchResult{
+				Keys:   []string{"alice", "bob"},
+				Labels: []string{"Alice", "Bob"},
+			}
+		}
+		return MultiSelectSearchResult{
+			Keys:   []string{"frank", "fiona"},
+			Labels: []string{"Frank", "Fiona"},
+		}
+	}
+
+	t.Run("selections persist after backspacing search query", func(t *testing.T) {
+		p := newTestHuhPrompter()
+		f, result := p.buildMultiSelectWithSearchForm(
+			"Select", "Search", nil, nil, staticSearchFunc,
+		)
+		longWait := interactionStep{delay: 300 * time.Millisecond}
+		runForm(t, f, newInteraction(
+			tab(), longWait,
+			toggle(),         // toggle alice
+			shiftTab(),       // back to search input
+			typeKeys("f"),    // type "f"
+			longWait,         // wait for API + OptionsFunc
+			typeKeys("\x7f"), // backspace to ""
+			longWait,         // wait for cache/API
+			tab(), longWait,
+			enter(),
+		))
+		assert.Equal(t, []string{"alice"}, result.selectedKeys())
+	})
+}
diff --git a/internal/prompter/multi_select_with_search.go b/internal/prompter/multi_select_with_search.go
