Add experimental huh-only prompter gated by GH_EXPERIMENTAL_PROMPTER#12859
Merged
williammartin merged 14 commits intotrunkfrom Mar 26, 2026
Merged
Add experimental huh-only prompter gated by GH_EXPERIMENTAL_PROMPTER#12859williammartin merged 14 commits intotrunkfrom
williammartin merged 14 commits intotrunkfrom
Conversation
c92752b to
bae9a1b
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an experimental interactive prompter implementation based on charm.land/huh/v2, gated behind GH_EXPERIMENTAL_PROMPTER, and upgrades the codebase from huh v1 (github.com/charmbracelet/huh) to huh v2 (charm.land/huh/v2).
Changes:
- Add
experimentalPrompterEnabledtoIOStreamsand wireGH_EXPERIMENTAL_PROMPTERenv var parsing in the factory. - Update prompter selection to prefer the experimental
huhPrompterwhen enabled; migrate accessible prompter theme wiring to huh v2. - Introduce a custom huh
FieldforMultiSelectWithSearch, plus a new huh-based prompter implementation and tests.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/iostreams/iostreams.go | Adds IOStreams flag + accessors for enabling the experimental prompter. |
| pkg/cmd/factory/default.go | Parses GH_EXPERIMENTAL_PROMPTER and sets IOStreams flag (matches existing truthy/falsey pattern). |
| internal/prompter/prompter.go | Switches to charm.land/huh/v2, routes New() to huhPrompter when experimental is enabled, updates theme API usage. |
| internal/prompter/multi_select_with_search.go | Adds custom multiSelectSearchField implementing huh Field to support search-driven multiselect. |
| internal/prompter/huh_prompter.go | Adds the new huh-based prompter implementation (select/input/confirm/editor/etc.). |
| internal/prompter/huh_prompter_test.go | Adds tests that drive huh forms via io.Pipe into bubbletea’s event loop. |
| internal/prompter/accessible_prompter_test.go | Adjusts expectations for updated output/formatting with huh v2. |
| go.mod | Adds charm.land v2 modules and removes github.com/charmbracelet/huh v1 requirement. |
| go.sum | Updates dependency checksums accordingly. |
Comments suppressed due to low confidence (1)
internal/prompter/huh_prompter_test.go:614
- This comment still refers to waiting for
OptionsFunc, but this test is exercising the custommultiSelectSearchFieldimplementation. Adjusting the comment to the actual behavior being awaited (simulated API latency / async search completion) will keep the test intent clear.
typeKeys("f"), // type "f"
longWait, // wait for API + OptionsFunc
typeKeys("\x7f"), // backspace to ""
longWait, // wait for cache/API
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+118
to
+123
| // applySearchResult processes a completed search and rebuilds the option list. | ||
| func (m *multiSelectSearchField) applySearchResult(query string, result MultiSelectSearchResult) { | ||
| m.loading = false | ||
| m.lastQuery = query | ||
| if result.Err != nil { | ||
| m.err = result.Err |
There was a problem hiding this comment.
MultiSelectSearchResult includes MoreResults, but this custom field never reads it, so the experimental prompter can’t surface the existing “Search (N more)” affordance used by multiSelectWithSearch. Consider incorporating result.MoreResults into the UI (e.g., in the search title or a footer/placeholder) to preserve feature parity and avoid confusing pagination/partial results.
Comment on lines
+131
to
+140
| for i, k := range result.Keys { | ||
| m.optionLabels[k] = result.Labels[i] | ||
| } | ||
|
|
||
| // Build option list: selected items first, then results, then persistent. | ||
| var options []msOption | ||
| seen := make(map[string]bool) | ||
|
|
||
| // 1. Currently selected items. | ||
| for _, k := range m.selectedKeys() { |
There was a problem hiding this comment.
optionLabels is only ever appended to (m.optionLabels[k] = ...) and never pruned. Over a long session with many distinct searches, this map will grow without bound even though most keys may never be selected/persistent. Consider resetting it per search, or keeping labels only for (selected + persistent + current results) keys to prevent unbounded memory growth.
Suggested change
| for i, k := range result.Keys { | |
| m.optionLabels[k] = result.Labels[i] | |
| } | |
| // Build option list: selected items first, then results, then persistent. | |
| var options []msOption | |
| seen := make(map[string]bool) | |
| // 1. Currently selected items. | |
| for _, k := range m.selectedKeys() { | |
| // Rebuild optionLabels so it only contains labels for selected, persistent, | |
| // and current search result keys, to avoid unbounded growth over time. | |
| oldLabels := m.optionLabels | |
| selected := m.selectedKeys() | |
| estimatedSize := len(result.Keys) + len(m.persistent) + len(selected) | |
| newLabels := make(map[string]string, estimatedSize) | |
| // Preserve labels for currently selected keys, if known. | |
| for _, k := range selected { | |
| if lbl, ok := oldLabels[k]; ok && lbl != "" { | |
| newLabels[k] = lbl | |
| } | |
| } | |
| // Preserve labels for persistent keys, if known. | |
| for _, k := range m.persistent { | |
| if lbl, ok := oldLabels[k]; ok && lbl != "" { | |
| newLabels[k] = lbl | |
| } | |
| } | |
| // Add/override labels for current search results. | |
| for i, k := range result.Keys { | |
| l := result.Labels[i] | |
| if l == "" { | |
| l = k | |
| } | |
| newLabels[k] = l | |
| } | |
| m.optionLabels = newLabels | |
| // Build option list: selected items first, then results, then persistent. | |
| var options []msOption | |
| seen := make(map[string]bool) | |
| // 1. Currently selected items. | |
| for _, k := range selected { |
Comment on lines
+18
to
+19
| // search results are loaded synchronously when the user presses Enter in | ||
| // the search input, avoiding goroutine races with selection state. |
There was a problem hiding this comment.
The docstring says results are loaded “synchronously when the user presses Enter”, but startSearch runs the query via a tea.Cmd (async). Consider rewording this to reflect the actual behavior (async search with input ignored while loading) to avoid misleading future maintainers.
Suggested change
| // search results are loaded synchronously when the user presses Enter in | |
| // the search input, avoiding goroutine races with selection state. | |
| // search results are loaded asynchronously via a Bubble Tea command when the | |
| // user presses Enter in the search input, and further input is ignored while | |
| // loading to avoid goroutine races with selection state. |
| return interactionStep{bytes: []byte{0x01, 0x0b}} | ||
| } | ||
|
|
||
| // waitForOptions adds extra delay to let OptionsFunc load before continuing. |
There was a problem hiding this comment.
waitForOptions mentions OptionsFunc, but the experimental prompter uses a custom field (no OptionsFunc). Updating the comment/helper name to reflect what you’re actually waiting for (bubbletea render loop / async search) will reduce confusion when debugging test flakes.
This issue also appears on line 611 of the same file.
Suggested change
| // waitForOptions adds extra delay to let OptionsFunc load before continuing. | |
| // waitForOptions adds a small delay to let async option loading/rendering settle | |
| // before continuing with further simulated input. |
williammartin
approved these changes
Mar 26, 2026
Member
williammartin
left a comment
There was a problem hiding this comment.
For posterity, here are the results of an adversarial review from copilot:
Adversarial Review: PR #12859 — Experimental huh Prompter
🔴 HIGH — huh.ErrUserAborted not recognized as cancellation (Sonnet 4.6)
File: pkg/cmdutil/errors.go:43, internal/ghcmd/cmd.go:120
When the user presses Ctrl+C in the experimental prompter, huh returns huh.ErrUserAborted. IsUserCancellation() only checks for
CancelError and terminal.InterruptErr — it never matches. Result: every Ctrl+C prints "error: user aborted" to stderr and exits code 1
instead of a clean cancellation. Worse, in gh pr create, the failed cancellation check causes partial PR state to be serialized to disk (
preserve.go:22).
Fix: Add errors.Is(err, huh.ErrUserAborted) to IsUserCancellation.
-----------------------------------------------------------------------------------------------------------------------------------------
🟡 MEDIUM — Failed search query cannot be retried (Opus 4.6)
File: multi_select_with_search.go:121
applySearchResult sets m.lastQuery = query before error checks. The updateSearch method skips queries matching lastQuery. If a search
fails (network error), retrying the same query is silently skipped — user is stuck. Also affects the initial empty-string load.
Fix: Move m.lastQuery = query to after the error checks (success path only).
-----------------------------------------------------------------------------------------------------------------------------------------
🟡 MEDIUM — MultiSelectWithSearch silently breaks on TERM=dumb (GPT-5.4)
File: multi_select_with_search.go:389
huh auto-enables accessible mode when TERM=dumb. RunAccessible() is a no-op stub that prints a message and returns nil. The form silently
succeeds without collecting any selection — reviewer/assignee prompts return empty results.
Fix: Implement RunAccessible properly, or force-disable accessible mode in the experimental prompter (falling back to the survey prompter
instead).
-----------------------------------------------------------------------------------------------------------------------------------------
🟡 MEDIUM — ConfirmDeletion case sensitivity diverges (Sonnet 4.6)
File: huh_prompter.go:201
huhPrompter.ConfirmDeletion uses input != requiredValue (case-sensitive). surveyPrompter uses strings.EqualFold (case-insensitive). Users
switching to the experimental prompter will find that "MY-REPO" no longer matches "my-repo" for deletion confirmation.
Fix: Use !strings.EqualFold(input, requiredValue) to match survey behavior.
-----------------------------------------------------------------------------------------------------------------------------------------
🟢 LOW — runForm test helper leaks goroutines on timeout (Sonnet 4.6)
File: huh_prompter_test.go:113
The io.PipeWriter is never closed; if form.Run() hangs, the goroutine leaks indefinitely. Under -race or parallel tests, this accumulates.
Fix: Add defer w.CloseWithError(errors.New("test timed out")).
I addressed the top issue, but the rest feel excessive for introducing an experimental mode. Also, this env var isn't documented in gh environment, but I'm ok for that to be a follow up.
Introduce a new Prompter implementation (huhPrompter) that uses the charmbracelet/huh library in its standard interactive mode, as an alternative to the survey-based default prompter. The new implementation is gated behind the GH_EXPERIMENTAL_PROMPTER environment variable, following the same truthy/falsey pattern as GH_ACCESSIBLE_PROMPTER. Key differences from the accessible prompter: - No WithAccessible(true) flag (full interactive TUI) - Uses EchoModePassword (masked with *) instead of EchoModeNone - No default value annotations appended to prompt text Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Implement a huh-native MultiSelectWithSearch that renders the search input and multi-select list simultaneously using LayoutStack. The search input is in Group 0 and the multi-select in Group 1, with OptionsFunc bound to the search query so results update when the user presses Enter to advance focus. Users can Shift+Tab back to refine their search, and selections persist across queries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Migrate from github.com/charmbracelet/huh v1 to charm.land/huh/v2, updating ThemeBase16 to the new ThemeFunc API. Fix selected options being lost across searches in the huhPrompter's MultiSelectWithSearch. The root cause was huh's internal Eval cache: when the user returned to a previously-seen search query, cached options with stale .selected state overwrote the current selections via updateValue(). The fix includes selectedValues in the OptionsFunc binding hash (via searchOptionsBinding) so the cache key changes whenever selections change, preventing stale cache hits. A local searchFunc result cache avoids redundant API calls when only the selection state (not the query) has changed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extract build*Form() methods from each huhPrompter method, separating
form construction from form.Run(). This enables testing the real form
construction code by driving it with direct model updates, adapted
from huh's own test patterns.
Tests cover Input, Select, MultiSelect, Confirm, Password,
MarkdownEditor, and MultiSelectWithSearch including a persistence
test that verifies selections survive across search query changes.
Also fixes a search cache initialization bug where the first
buildOptions("") call would skip the searchFunc due to
cachedSearchQuery defaulting to "".
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace manual model updates with an io.Pipe-based test harness that drives forms through bubbletea's real event loop. Interaction helpers (tab(), toggle(), typeKeys(), enter(), etc.) send raw terminal bytes through io.Pipe to form.Run() in a goroutine. Add tests for AuthToken, ConfirmDeletion, and InputHostname including validation rejection paths. Add MultiSelectWithSearch coverage for persistent options and empty search results. 30 tests, ~1s, all build*Form methods at 94-100% coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix accessible prompter tests that broke with the huh v2 upgrade: - Replace 'Input a number' with 'Enter a number' (huh v2 changed text) - Remove trailing CRLF from ExpectString calls that now fail due to ANSI color codes wrapping the title text - Allow ANSI escape codes in password masking regex assertions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace Value() pointer bindings with syncAccessor in MultiSelectWithSearch. huh's OptionsFunc runs in a goroutine while the main event loop writes field values, causing a data race on shared variables. syncAccessor implements huh's Accessor interface with a shared mutex, ensuring all reads and writes are synchronized. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 'Type to search, Ctrl+U to clear' placeholder to the MultiSelectWithSearch search input. Set WithWidth(80) in the test harness to prevent textinput placeholder rendering panics when there is no terminal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
42638ec to
cb2b505
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a new
huhPrompterimplementation gated behindGH_EXPERIMENTAL_PROMPTER, usingcharmbracelet/huhv2 in interactive TUI mode as an alternative to the survey-based default prompter. Also upgrades from huh v1 tocharm.land/huh/v2.Key design decisions
GH_ACCESSIBLE_PROMPTER. Takes precedence when both are set.build*Form()pattern: Each prompt type separates form construction fromform.Run(), enabling unit tests that drive huh's real bubbletea event loop viaio.Pipewithout a terminal.huh.Fieldrather than a multi-field form withOptionsFunc. See reviewer notes below.Notes for reviewers
Why a custom
huh.Fieldfor MultiSelectWithSearch?We tried several approaches using huh's
OptionsFuncto dynamically reload search results. All had issues rooted inOptionsFuncrunning in a goroutine: data races with selection state (caught by-race), stale cached options overwriting selections, and option reloads on every toggle causing cursor jumps.The custom field (
multiSelectSearchField) avoids all of this by owning the update loop directly. It also gives us control over behavior that wasn't possible withOptionsFunc: