diff --git a/.claude/skills/review-pr/SKILL.md b/.claude/skills/review-pr/SKILL.md index f6542dd0..39f44b24 100644 --- a/.claude/skills/review-pr/SKILL.md +++ b/.claude/skills/review-pr/SKILL.md @@ -47,6 +47,7 @@ Go through each changed file and check for violations. Flag only actual problems - [ ] Domain code never reads from stdin directly - [ ] Interactive input uses `UserInputRequestEvent` + `ResponseCh` pattern +- [ ] The intent constructor a prompt uses matches what it is (the compiler enforces that one is used, not that it is the right one): a choice between distinct actions is `ActionChoice`, not a `Confirm` with an inline `[a/b]` hint, and an `ActionChoice` label does not spell out its own key - [ ] Non-TTY mode fails early with a helpful error if input would be required - [ ] New user-supplied inputs (args, flags, config values) are validated at the boundary via `internal/validate`; no new inline validation regexp duplicates an existing validator (pod names → `PodName`; opaque secrets → loose checks like `AuthToken`; paths/URLs → their existing parsers; other identifiers → the owning API's documented contract) and malformed-input cases are tested diff --git a/CLAUDE.md b/CLAUDE.md index 7e4ac07d..74be1276 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -267,10 +267,12 @@ A JSON-capable command emits a single `output.Envelope` (schema version, `data`/ Domain code must never read from stdin or wait for user input directly. Instead: -1. Emit a `UserInputRequestEvent` via `sink.Emit(output.UserInputRequestEvent{...})` with: - - `Prompt`: message to display - - `Options`: available choices (e.g., `{Key: "enter", Label: "Press ENTER to continue"}`) - - `ResponseCh`: channel to receive the user's response +1. Emit a `UserInputRequestEvent` built with one of the three intent constructors in `internal/output/prompt.go`. Its fields are unexported, so a struct literal built anywhere else does not compile — the constructors are the only way in. Name what the prompt *is* and its layout follows: + - `output.Confirm(prompt, output.DefaultYes|DefaultNo, responseCh)` — y/n on an action the user already requested. Renders inline as `[y/N]`; the capitalized answer is what ENTER picks. `DefaultNo` for anything destructive. + - `output.ActionChoice(prompt, options, responseCh)` — a choice between distinct outcomes. Renders one selectable row per option, with the `[KEY]` shortcut derived from each option's `Key`, so labels stay plain prose. + - `output.Acknowledge(prompt, label, responseCh)` — a single keypress, no choice. + + If a new prompt is not clearly one of the three, ask the user which it should be rather than guessing. Vertical is not a global default: flattening distinct actions into a trailing hint reads as prose and wraps badly (DEVX-1045), but a confirmation is one line for good reason. 2. Wait on the `ResponseCh` for an `InputResponse` containing: - `SelectedKey`: which option was selected @@ -285,11 +287,7 @@ Domain code must never read from stdin or wait for user input directly. Instead: Example flow in auth login: ```go responseCh := make(chan output.InputResponse, 1) -sink.Emit(output.UserInputRequestEvent{ - Prompt: "Waiting for authentication...", - Options: []output.InputOption{{Key: "enter", Label: "Press ENTER when complete"}}, - ResponseCh: responseCh, -}) +sink.Emit(output.Acknowledge("Waiting for authentication...", "Press any key when complete", responseCh)) select { case resp := <-responseCh: diff --git a/internal/auth/login.go b/internal/auth/login.go index b6c8a02a..431a81af 100644 --- a/internal/auth/login.go +++ b/internal/auth/login.go @@ -74,11 +74,7 @@ func (l *loginProvider) Login(ctx context.Context) (string, error) { l.sink.Emit(output.SpinnerStart("Waiting for authorization...")) responseCh := make(chan output.InputResponse, 1) - l.sink.Emit(output.UserInputRequestEvent{ - Prompt: "Waiting for authorization...", - Options: []output.InputOption{{Key: "any", Label: "Press any key when complete"}}, - ResponseCh: responseCh, - }) + l.sink.Emit(output.Acknowledge("Waiting for authorization...", "Press any key when complete", responseCh)) select { case resp := <-responseCh: diff --git a/internal/awsconfig/awsconfig.go b/internal/awsconfig/awsconfig.go index 841aaa0d..343bfd97 100644 --- a/internal/awsconfig/awsconfig.go +++ b/internal/awsconfig/awsconfig.go @@ -306,18 +306,14 @@ func Setup(ctx context.Context, sink output.Sink, resolvedHost string, status pr if !skipConfirm { responseCh := make(chan output.InputResponse, 1) - sink.Emit(output.UserInputRequestEvent{ - Prompt: "Set up a LocalStack profile for AWS CLI and SDKs in ~/.aws?", - Options: []output.InputOption{{Key: "y", Label: "Y"}, {Key: "n", Label: "n"}}, - ResponseCh: responseCh, - }) + sink.Emit(output.Confirm("Set up a LocalStack profile for AWS CLI and SDKs in ~/.aws?", output.DefaultYes, responseCh)) select { case resp := <-responseCh: if resp.Cancelled { return nil } - if resp.SelectedKey == "n" { + if resp.SelectedKey == output.KeyNo { sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: "Skipped adding LocalStack AWS profile."}) return nil } diff --git a/internal/container/select.go b/internal/container/select.go index 5d5b286a..b3d4de8f 100644 --- a/internal/container/select.go +++ b/internal/container/select.go @@ -19,12 +19,7 @@ func SelectEmulator( } responseCh := make(chan output.InputResponse, 1) - sink.Emit(output.UserInputRequestEvent{ - Prompt: "Which emulator would you like to use?", - Options: options, - ResponseCh: responseCh, - Vertical: true, - }) + sink.Emit(output.ActionChoice("Which emulator would you like to use?", options, responseCh)) var resp output.InputResponse select { diff --git a/internal/container/start.go b/internal/container/start.go index df9e77a0..7fe13736 100644 --- a/internal/container/start.go +++ b/internal/container/start.go @@ -1335,22 +1335,20 @@ func isDefinitiveLicenseRejection(status int) bool { // ESC declines. Ctrl+C would do too, but it also cancels the root context, and // the ErrorEvent that the decline renders then races the TUI's own quit — so the // manual recovery steps sometimes never reach the terminal (DEVX-1045). An -// advertised decline key keeps that path deterministic. The choices render -// vertically so both keys read as selectable actions rather than a hint tacked -// onto the end of the sentence; the prompt therefore states the reason and -// leaves the two actions to the labels, which keeps it one wrapped statement -// instead of a statement whose trailing question dangles at the wrap point. +// advertised decline key keeps that path deterministic. The prompt states the +// reason and leaves the two actions to output.ActionChoice's labels, which keeps +// it one wrapped statement instead of a statement whose trailing question +// dangles at the wrap point. func promptRelogin(ctx context.Context, sink output.Sink, licErr *api.LicenseError) bool { responseCh := make(chan output.InputResponse, 1) - sink.Emit(output.UserInputRequestEvent{ - Prompt: fmt.Sprintf("License validation failed: %s.", licErr.Message), - Options: []output.InputOption{ - {Key: "r", Label: "[R] Re-authenticate"}, - {Key: "esc", Label: "[ESC] Exit"}, + sink.Emit(output.ActionChoice( + fmt.Sprintf("License validation failed: %s.", licErr.Message), + []output.InputOption{ + {Key: "r", Label: "Re-authenticate"}, + {Key: "esc", Label: "Exit"}, }, - ResponseCh: responseCh, - Vertical: true, - }) + responseCh, + )) select { case resp := <-responseCh: return !resp.Cancelled && resp.SelectedKey != "esc" @@ -1629,15 +1627,14 @@ func (m *startupMonitor) await(ctx context.Context, containerID, healthURL strin m.sink.Emit(output.SpinnerStop()) responseCh = make(chan output.InputResponse, 1) - m.sink.Emit(output.UserInputRequestEvent{ - Prompt: "LocalStack is still starting. Check progress with 'lstk logs'.", - Options: []output.InputOption{ - {Key: "w", Label: "[W] Keep waiting"}, - {Key: "s", Label: "[S] Stop and exit"}, + m.sink.Emit(output.ActionChoice( + "LocalStack is still starting. Check progress with 'lstk logs'.", + []output.InputOption{ + {Key: "w", Label: "Keep waiting"}, + {Key: "s", Label: "Stop and exit"}, }, - ResponseCh: responseCh, - Vertical: true, - }) + responseCh, + )) case <-ticker.C: if ready, err := check(); err != nil || ready { return err diff --git a/internal/container/start_test.go b/internal/container/start_test.go index 73420031..9d91fd96 100644 --- a/internal/container/start_test.go +++ b/internal/container/start_test.go @@ -833,7 +833,7 @@ func TestStartupMonitorAwait_InteractivePromptKeepWaitingThenStop(t *testing.T) for i, key := range []string{"w", "s"} { select { case req := <-prompts: - req.ResponseCh <- output.InputResponse{SelectedKey: key} + req.ResponseCh() <- output.InputResponse{SelectedKey: key} case <-time.After(5 * time.Second): t.Errorf("prompt %d never appeared", i+1) return @@ -850,12 +850,15 @@ func TestStartupMonitorAwait_InteractivePromptKeepWaitingThenStop(t *testing.T) assert.True(t, timeoutErr.stopped, "choosing stop at the prompt must be recorded on the error") firstPrompt := <-seenPrompts - assert.Equal(t, "LocalStack is still starting. Check progress with 'lstk logs'.", firstPrompt.Prompt) - assert.True(t, firstPrompt.Vertical) + assert.Equal(t, "LocalStack is still starting. Check progress with 'lstk logs'.", firstPrompt.Prompt()) + assert.True(t, firstPrompt.Vertical()) assert.Equal(t, []output.InputOption{ - {Key: "w", Label: "[W] Keep waiting"}, - {Key: "s", Label: "[S] Stop and exit"}, - }, firstPrompt.Options) + {Key: "w", Label: "Keep waiting"}, + {Key: "s", Label: "Stop and exit"}, + }, firstPrompt.Options()) + // Labels stay plain prose; the advertised keys come from output.OptionLabel. + assert.Equal(t, "[W] Keep waiting", output.OptionLabel(firstPrompt.Options()[0])) + assert.Equal(t, "[S] Stop and exit", output.OptionLabel(firstPrompt.Options()[1])) } func TestStartupMonitorAwait_DismissesPromptWhenEmulatorBecomesReady(t *testing.T) { @@ -893,7 +896,7 @@ func TestStartupMonitorAwait_DismissesPromptWhenEmulatorBecomesReady(t *testing. require.NoError(t, err) prompt := <-prompts dismissal := <-dismissals - assert.Equal(t, prompt.ResponseCh, dismissal.ResponseCh) + assert.Equal(t, prompt.ResponseCh(), dismissal.ResponseCh) } func TestStartupMonitorAwait_DoesNotStopEmulatorThatBecameReadyBeforeSelection(t *testing.T) { @@ -914,7 +917,7 @@ func TestStartupMonitorAwait_DoesNotStopEmulatorThatBecameReadyBeforeSelection(t sink := output.SinkFunc(func(event output.Event) { if prompt, ok := event.(output.UserInputRequestEvent); ok { ready.Store(true) - prompt.ResponseCh <- output.InputResponse{SelectedKey: "s"} + prompt.ResponseCh() <- output.InputResponse{SelectedKey: "s"} } }) @@ -1647,7 +1650,7 @@ func TestStart_SecondLicenseRejectionAfterReloginRendersErrorEvent(t *testing.T) // Auto-answer every prompt (the re-login confirmation, then the login // flow's "press any key" completion prompt) as if the user pressed enter. if req, ok := event.(output.UserInputRequestEvent); ok { - req.ResponseCh <- output.InputResponse{SelectedKey: "enter"} + req.ResponseCh() <- output.InputResponse{SelectedKey: "enter"} } }) @@ -1687,7 +1690,7 @@ func TestPromptRelogin_FoldsReasonIntoThePromptWithoutASeparateWarning(t *testin sink := output.SinkFunc(func(event output.Event) { events = append(events, event) if req, ok := event.(output.UserInputRequestEvent); ok { - req.ResponseCh <- output.InputResponse{Cancelled: true} + req.ResponseCh() <- output.InputResponse{Cancelled: true} } }) @@ -1697,8 +1700,8 @@ func TestPromptRelogin_FoldsReasonIntoThePromptWithoutASeparateWarning(t *testin require.Len(t, events, 1, "the rejection reason must be folded into the prompt, not emitted as a separate message first") req, ok := events[0].(output.UserInputRequestEvent) require.True(t, ok, "the only event emitted must be the prompt itself") - assert.Contains(t, req.Prompt, licErr.Message, "the prompt must explain why the user is being asked to log in again") - assert.Equal(t, "[R] Re-authenticate", req.Options[0].Label, "the recovery action belongs to the choice, not the prompt sentence") + assert.Contains(t, req.Prompt(), licErr.Message, "the prompt must explain why the user is being asked to log in again") + assert.Equal(t, "Re-authenticate", req.Options()[0].Label, "the recovery action belongs to the choice, not the prompt sentence") } // TestPromptRelogin_OffersAnAdvertisedDeclineKey covers DEVX-1045: Ctrl+C was the @@ -1721,18 +1724,22 @@ func TestPromptRelogin_OffersAnAdvertisedDeclineKey(t *testing.T) { sink := output.SinkFunc(func(event output.Event) { if r, ok := event.(output.UserInputRequestEvent); ok { req = r - r.ResponseCh <- tc.response + r.ResponseCh() <- tc.response } }) accepted := promptRelogin(context.Background(), sink, licErr) assert.Equal(t, tc.accepted, accepted) - assert.True(t, req.Vertical, "the choices must render as vertical, selectable actions") + assert.True(t, req.Vertical(), "the choices must render as vertical, selectable actions") assert.Equal(t, []output.InputOption{ - {Key: "r", Label: "[R] Re-authenticate"}, - {Key: "esc", Label: "[ESC] Exit"}, - }, req.Options, "both the accept and the decline key must be advertised, shortcut first") + {Key: "r", Label: "Re-authenticate"}, + {Key: "esc", Label: "Exit"}, + }, req.Options()) + // Labels stay plain prose; the advertised keys come from output.OptionLabel. + assert.Equal(t, "[R] Re-authenticate", output.OptionLabel(req.Options()[0]), + "both the accept and the decline key must be advertised, shortcut first") + assert.Equal(t, "[ESC] Exit", output.OptionLabel(req.Options()[1])) }) } } diff --git a/internal/output/events.go b/internal/output/events.go index bbd6583c..fff1e2b4 100644 --- a/internal/output/events.go +++ b/internal/output/events.go @@ -286,13 +286,41 @@ type InputResponse struct { Cancelled bool } +// UserInputRequestEvent asks the frontend to put a question to the user and +// send the answer back on the response channel. +// +// Build one with Confirm, ActionChoice, or Acknowledge (prompt.go). The fields +// are unexported so that is the only way: a struct literal asks its author to +// pick a rendering at the moment the question they can actually answer is what +// the prompt IS, and the cheapest answer — leaving the layout out — silently +// ships an inline prompt. That is how the license re-login prompt ended up with +// two advertised keys flattened into one dimmed hint (DEVX-1045). Naming the +// intent instead makes the layout a consequence rather than a decision. type UserInputRequestEvent struct { - Prompt string - Options []InputOption - ResponseCh chan<- InputResponse - Vertical bool + prompt string + options []InputOption + responseCh chan<- InputResponse + // vertical renders each option as its own selectable row instead of a + // trailing "[a/b]" hint. + vertical bool } +// Prompt is the question put to the user. It may span several lines; the +// options are appended to the first one. +func (e UserInputRequestEvent) Prompt() string { return e.prompt } + +// Options are the answers the user may choose between. The returned slice is +// not copied — treat it as read-only. +func (e UserInputRequestEvent) Options() []InputOption { return e.options } + +// ResponseCh receives the user's answer. It also identifies the request, so a +// UserInputDismissEvent can name the exact prompt it retracts. +func (e UserInputRequestEvent) ResponseCh() chan<- InputResponse { return e.responseCh } + +// Vertical reports whether each option should render as its own selectable row +// rather than as a trailing "[a/b]" hint. Set by ActionChoice. +func (e UserInputRequestEvent) Vertical() bool { return e.vertical } + // UserInputDismissEvent removes a pending prompt when the condition that // required input resolves on its own. ResponseCh identifies the exact request // so a late dismissal cannot hide a newer prompt. diff --git a/internal/output/plain_format.go b/internal/output/plain_format.go index 57b24a6a..e21a2741 100644 --- a/internal/output/plain_format.go +++ b/internal/output/plain_format.go @@ -103,7 +103,31 @@ func formatStatusLine(e ContainerStatusEvent) (string, bool) { } func formatUserInputRequest(e UserInputRequestEvent) string { - return FormatPrompt(e.Prompt, e.Options) + return FormatPromptEvent(e) +} + +// FormatPromptEvent renders a prompt on a single line. A vertical prompt has no +// one-line form of its own, so its options are laid end to end — each keeping +// the shortcut OptionLabel derives, since a plain-prose label ("Log in again") +// would otherwise leave the user with no key to press. They carry their own +// brackets, so the surrounding "[a/b]" of an inline prompt is dropped rather +// than nested. Used wherever the full multi-line rendering does not fit: plain +// output, and the TUI's spinner text. +func FormatPromptEvent(e UserInputRequestEvent) string { + if !e.vertical { + return FormatPrompt(e.prompt, e.options) + } + + labels := make([]string, 0, len(e.options)) + for _, opt := range e.options { + if label := OptionLabel(opt); label != "" { + labels = append(labels, label) + } + } + if len(labels) == 0 { + return appendPromptSuffix(e.prompt, "") + } + return appendPromptSuffix(e.prompt, " "+strings.Join(labels, " / ")) } // FormatPromptLabels formats option labels into a suffix string. @@ -127,8 +151,14 @@ func FormatPromptLabels(options []InputOption) string { // FormatPrompt formats a prompt string with its options into a display line. func FormatPrompt(prompt string, options []InputOption) string { + return appendPromptSuffix(prompt, FormatPromptLabels(options)) +} + +// appendPromptSuffix puts the option hints on the end of the prompt's first +// line, so any lines below it stay a block of their own. +func appendPromptSuffix(prompt, suffix string) string { lines := strings.Split(prompt, "\n") - firstLine := lines[0] + FormatPromptLabels(options) + firstLine := lines[0] + suffix rest := lines[1:] if len(rest) == 0 { return firstLine diff --git a/internal/output/prompt.go b/internal/output/prompt.go new file mode 100644 index 00000000..b6344061 --- /dev/null +++ b/internal/output/prompt.go @@ -0,0 +1,131 @@ +package output + +import ( + "fmt" + "strings" +) + +// Keys carried by the options the constructors below build. Handlers should +// compare InputResponse.SelectedKey against these rather than string literals. +const ( + KeyYes = "y" + KeyNo = "n" + // KeyAny matches any keypress. resolveOption in internal/ui returns it + // before considering any other option, so it is only meaningful as the sole + // option of an Acknowledge prompt. + KeyAny = "any" +) + +// ConfirmDefault selects which answer ENTER picks in a Confirm prompt. It is +// conveyed to the user by capitalizing that answer's label, which is also how +// the TUI's key resolution finds it — so the displayed default and the honored +// default cannot drift apart. +type ConfirmDefault int + +const ( + DefaultYes ConfirmDefault = iota + DefaultNo +) + +// Confirm asks the user to approve an action they already requested, rendered +// inline as "Reset emulator state? [y/N]". +// +// Inline is deliberate here and should stay that way: the question has one +// answer the user is already leaning toward, the [y/N] idiom is universal, it +// costs one line, and it carries its default in the capitalization. Pass +// DefaultNo for anything destructive or irreversible. +// +// Use ActionChoice instead when the options are distinct outcomes rather than +// "do the thing I asked for, or don't". If a new prompt is not clearly one or +// the other, ask the user which it should be rather than guessing. +func Confirm(prompt string, def ConfirmDefault, responseCh chan<- InputResponse) UserInputRequestEvent { + yes, no := "Y", "n" + if def == DefaultNo { + yes, no = "y", "N" + } + return UserInputRequestEvent{ + prompt: prompt, + options: []InputOption{ + {Key: KeyYes, Label: yes}, + {Key: KeyNo, Label: no}, + }, + responseCh: responseCh, + } +} + +// ActionChoice offers a choice between distinct outcomes, rendered vertically +// as one selectable row per option: +// +// ? License validation failed: token expired. +// ● [ENTER] Log in again +// ○ [ESC] Exit +// +// Labels are plain prose — OptionLabel derives the bracketed shortcut from each +// option's Key, so a label must not spell the key out itself. +// +// Vertical is deliberate here and should stay that way: flattening several +// distinct actions into a trailing "[a/b]" hint reads as prose glued to the end +// of the question, wraps badly, and gives the user nothing to arrow through +// (DEVX-1045). Use Confirm for a yes/no on an action the user already +// requested, and Acknowledge when there is nothing to choose between. +func ActionChoice(prompt string, options []InputOption, responseCh chan<- InputResponse) UserInputRequestEvent { + return UserInputRequestEvent{ + prompt: prompt, + options: options, + responseCh: responseCh, + vertical: true, + } +} + +// Acknowledge waits for any keypress, rendered inline as "Waiting for +// authorization... (Press any key when complete)". It is not a choice — there +// is one option and every key selects it — so it stays on one line. +func Acknowledge(prompt, label string, responseCh chan<- InputResponse) UserInputRequestEvent { + return UserInputRequestEvent{ + prompt: prompt, + options: []InputOption{{Key: KeyAny, Label: label}}, + responseCh: responseCh, + } +} + +// namedShortcuts spells out the keys whose names are not a single character. +// The values match what the terminal user is told to press, not tea.KeyType's +// own naming. +var namedShortcuts = map[string]string{ + "enter": "ENTER", + "esc": "ESC", + "space": "SPACE", + "tab": "TAB", +} + +// OptionLabel renders one option of a vertical prompt: its shortcut in +// brackets, then its label ("[ENTER] Log in again"). An option with no +// dedicated key — KeyAny, or an empty one — renders as the bare label, since +// there is no single key to advertise. +// +// Deriving the shortcut instead of leaving it to each label is what keeps the +// prompts consistent: hand-written labels had drifted into three styles +// ("[ENTER] Log in again", "Update now [U]", and a bare "AWS" that advertised +// no key at all) before this existed. +func OptionLabel(opt InputOption) string { + key := shortcut(opt.Key) + if key == "" { + return opt.Label + } + if opt.Label == "" { + return fmt.Sprintf("[%s]", key) + } + return fmt.Sprintf("[%s] %s", key, opt.Label) +} + +// shortcut returns the display form of an option key, or "" when the key names +// no single keypress the user can be told to hit. +func shortcut(key string) string { + if key == "" || key == KeyAny { + return "" + } + if named, ok := namedShortcuts[key]; ok { + return named + } + return strings.ToUpper(key) +} diff --git a/internal/output/prompt_test.go b/internal/output/prompt_test.go new file mode 100644 index 00000000..218debe3 --- /dev/null +++ b/internal/output/prompt_test.go @@ -0,0 +1,97 @@ +package output + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestConfirmRendersInlineWithTheDefaultCapitalized(t *testing.T) { + t.Parallel() + + // That the capitalized answer is the one ENTER selects is asserted end to end + // by TestAppEnterHonorsTheConfirmDefault in internal/ui, which exercises the + // real key resolution instead of a copy of its rule. + tests := []struct { + name string + def ConfirmDefault + labels []string + hint string + }{ + {name: "default yes", def: DefaultYes, labels: []string{"Y", "n"}, hint: " [Y/n]"}, + {name: "default no", def: DefaultNo, labels: []string{"y", "N"}, hint: " [y/N]"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ch := make(chan InputResponse, 1) + event := Confirm("Reset emulator state?", tt.def, ch) + + assert.False(t, event.Vertical(), "a confirmation stays on one line") + require.Len(t, event.Options(), 2) + assert.Equal(t, KeyYes, event.Options()[0].Key) + assert.Equal(t, KeyNo, event.Options()[1].Key) + assert.Equal(t, tt.labels, []string{event.Options()[0].Label, event.Options()[1].Label}) + assert.Equal(t, "Reset emulator state?"+tt.hint, FormatPromptEvent(event)) + }) + } +} + +func TestActionChoiceRendersVerticallyWithDerivedShortcuts(t *testing.T) { + t.Parallel() + + ch := make(chan InputResponse, 1) + event := ActionChoice("License validation failed: token expired.", []InputOption{ + {Key: "enter", Label: "Log in again"}, + {Key: "esc", Label: "Exit"}, + }, ch) + + assert.True(t, event.Vertical(), "distinct actions render as selectable rows") + assert.Equal(t, "[ENTER] Log in again", OptionLabel(event.Options()[0])) + assert.Equal(t, "[ESC] Exit", OptionLabel(event.Options()[1])) + + // The one-line form keeps the shortcuts, so a prompt mirrored into spinner + // text never leaves the user without a key to press. The labels bring their + // own brackets, so it does not nest them inside an inline prompt's "[a/b]". + assert.Equal(t, + "License validation failed: token expired. [ENTER] Log in again / [ESC] Exit", + FormatPromptEvent(event)) +} + +func TestAcknowledgeRendersInlineWithASingleAnyKeyOption(t *testing.T) { + t.Parallel() + + ch := make(chan InputResponse, 1) + event := Acknowledge("Waiting for authorization...", "Press any key when complete", ch) + + assert.False(t, event.Vertical()) + require.Len(t, event.Options(), 1) + assert.Equal(t, KeyAny, event.Options()[0].Key) + assert.Equal(t, "Waiting for authorization... (Press any key when complete)", FormatPromptEvent(event)) +} + +func TestOptionLabel(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + opt InputOption + want string + }{ + {name: "named key", opt: InputOption{Key: "enter", Label: "Log in again"}, want: "[ENTER] Log in again"}, + {name: "escape", opt: InputOption{Key: "esc", Label: "Exit"}, want: "[ESC] Exit"}, + {name: "single letter is uppercased", opt: InputOption{Key: "a", Label: "AWS"}, want: "[A] AWS"}, + {name: "any key advertises nothing", opt: InputOption{Key: KeyAny, Label: "Press any key"}, want: "Press any key"}, + {name: "empty key advertises nothing", opt: InputOption{Label: "Continue"}, want: "Continue"}, + {name: "no label", opt: InputOption{Key: "w"}, want: "[W]"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, OptionLabel(tt.opt)) + }) + } +} diff --git a/internal/reset/reset.go b/internal/reset/reset.go index 54b21e9d..7af1c190 100644 --- a/internal/reset/reset.go +++ b/internal/reset/reset.go @@ -41,18 +41,11 @@ func Reset(ctx context.Context, rt runtime.Runtime, containers []config.Containe if !force { responseCh := make(chan output.InputResponse, 1) - sink.Emit(output.UserInputRequestEvent{ - Prompt: "Reset emulator state? All resources will be lost", - Options: []output.InputOption{ - {Key: "y", Label: "Yes"}, - {Key: "n", Label: "NO"}, - }, - ResponseCh: responseCh, - }) + sink.Emit(output.Confirm("Reset emulator state? All resources will be lost", output.DefaultNo, responseCh)) select { case resp := <-responseCh: - if resp.Cancelled || resp.SelectedKey != "y" { + if resp.Cancelled || resp.SelectedKey != output.KeyYes { sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: "Cancelled"}) return nil } diff --git a/internal/reset/reset_test.go b/internal/reset/reset_test.go index 13521da7..c2c37591 100644 --- a/internal/reset/reset_test.go +++ b/internal/reset/reset_test.go @@ -147,7 +147,7 @@ func TestReset_ConfirmYes(t *testing.T) { go func() { req := <-prompts - req.ResponseCh <- output.InputResponse{SelectedKey: "y"} + req.ResponseCh() <- output.InputResponse{SelectedKey: "y"} }() err := reset.Reset(context.Background(), healthyRunningMock(t), awsContainers, resetter, "host:4566", false, sink) @@ -163,7 +163,7 @@ func TestReset_ConfirmNo(t *testing.T) { go func() { req := <-prompts - req.ResponseCh <- output.InputResponse{SelectedKey: "n"} + req.ResponseCh() <- output.InputResponse{SelectedKey: "n"} }() err := reset.Reset(context.Background(), healthyRunningMock(t), awsContainers, resetter, "host:4566", false, sink) diff --git a/internal/snapshot/remove.go b/internal/snapshot/remove.go index 5e994d3d..2ff65f62 100644 --- a/internal/snapshot/remove.go +++ b/internal/snapshot/remove.go @@ -49,18 +49,15 @@ func Remove(ctx context.Context, rt runtime.Runtime, containers []config.Contain if !force { responseCh := make(chan output.InputResponse, 1) - sink.Emit(output.UserInputRequestEvent{ - Prompt: fmt.Sprintf("Delete cloud snapshot 'pod:%s'? This operation cannot be undone.", podName), - Options: []output.InputOption{ - {Key: "y", Label: "Y"}, - {Key: "n", Label: "n"}, - }, - ResponseCh: responseCh, - }) + sink.Emit(output.Confirm( + fmt.Sprintf("Delete cloud snapshot 'pod:%s'? This operation cannot be undone.", podName), + output.DefaultNo, + responseCh, + )) select { case resp := <-responseCh: - if resp.Cancelled || resp.SelectedKey != "y" { + if resp.Cancelled || resp.SelectedKey != output.KeyYes { sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: "Cancelled"}) return nil } diff --git a/internal/ui/app.go b/internal/ui/app.go index 8a7ef191..52aa38cc 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -123,7 +123,7 @@ func (a App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if msg.String() == "ctrl+c" || msg.String() == "q" { var responseCmd tea.Cmd if a.pendingInput != nil { - responseCmd = sendInputResponseCmd(a.pendingInput.ResponseCh, output.InputResponse{Cancelled: true}) + responseCmd = sendInputResponseCmd(a.pendingInput.ResponseCh(), output.InputResponse{Cancelled: true}) a.pendingInput = nil a.inputPrompt = a.inputPrompt.Hide() } @@ -146,11 +146,11 @@ func (a App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return a, skipCmd } if a.pendingInput != nil { - if a.pendingInput.Vertical { + if a.pendingInput.Vertical() { return a.handleVerticalPromptKey(msg) } - if opt := resolveOption(a.pendingInput.Options, msg); opt != nil { - responseCmd := sendInputResponseCmd(a.pendingInput.ResponseCh, output.InputResponse{SelectedKey: opt.Key}) + if opt := resolveOption(a.pendingInput.Options(), msg); opt != nil { + responseCmd := sendInputResponseCmd(a.pendingInput.ResponseCh(), output.InputResponse{SelectedKey: opt.Key}) a.pendingInput = nil a.inputPrompt = components.NewInputPrompt() a.spinner = a.spinner.SetText("") @@ -193,12 +193,12 @@ func (a App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // blank screen while the domain waits on ResponseCh (DEVX-1045). The // spinner text is a mirror for as long as the spinner is on screen, since // View renders one or the other. - a.inputPrompt = a.inputPrompt.Show(msg.Prompt, msg.Options, msg.Vertical) + a.inputPrompt = a.inputPrompt.Show(msg.Prompt(), msg.Options(), msg.Vertical()) if a.spinner.Visible() { - a.spinner = a.spinner.SetText(output.FormatPrompt(msg.Prompt, msg.Options)) + a.spinner = a.spinner.SetText(output.FormatPromptEvent(msg)) } case output.UserInputDismissEvent: - if a.pendingInput == nil || a.pendingInput.ResponseCh != msg.ResponseCh { + if a.pendingInput == nil || a.pendingInput.ResponseCh() != msg.ResponseCh { return a, nil } a.pendingInput = nil @@ -445,19 +445,20 @@ func (a App) handleVerticalPromptKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { a.inputPrompt = a.inputPrompt.SetSelectedIndex(a.inputPrompt.SelectedIndex() + 1) return a, nil case tea.KeyEnter: + options := a.pendingInput.Options() idx := a.inputPrompt.SelectedIndex() - if idx >= 0 && idx < len(a.pendingInput.Options) { - opt := a.pendingInput.Options[idx] + if idx >= 0 && idx < len(options) { + opt := options[idx] a.lines = appendLine(a.lines, styledLine{text: formatResolvedInput(*a.pendingInput, opt.Key)}) - responseCmd := sendInputResponseCmd(a.pendingInput.ResponseCh, output.InputResponse{SelectedKey: opt.Key}) + responseCmd := sendInputResponseCmd(a.pendingInput.ResponseCh(), output.InputResponse{SelectedKey: opt.Key}) a.pendingInput = nil a.inputPrompt = a.inputPrompt.Hide() return a, responseCmd } } - if opt := resolveOption(a.pendingInput.Options, msg); opt != nil { + if opt := resolveOption(a.pendingInput.Options(), msg); opt != nil { a.lines = appendLine(a.lines, styledLine{text: formatResolvedInput(*a.pendingInput, opt.Key)}) - responseCmd := sendInputResponseCmd(a.pendingInput.ResponseCh, output.InputResponse{SelectedKey: opt.Key}) + responseCmd := sendInputResponseCmd(a.pendingInput.ResponseCh(), output.InputResponse{SelectedKey: opt.Key}) a.pendingInput = nil a.inputPrompt = a.inputPrompt.Hide() return a, responseCmd @@ -468,7 +469,7 @@ func (a App) handleVerticalPromptKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { func formatResolvedInput(req output.UserInputRequestEvent, selectedKey string) string { selected := selectedKey hasLabels := false - for _, opt := range req.Options { + for _, opt := range req.Options() { if opt.Label != "" { hasLabels = true } @@ -477,25 +478,25 @@ func formatResolvedInput(req output.UserInputRequestEvent, selectedKey string) s } } - if req.Vertical { - firstLine := strings.Split(req.Prompt, "\n")[0] - if selected == "" || !hasLabels || selectedKey == "any" { + if req.Vertical() { + firstLine := strings.Split(req.Prompt(), "\n")[0] + if selected == "" || !hasLabels || selectedKey == output.KeyAny { return firstLine } return fmt.Sprintf("%s %s", firstLine, selected) } - formatted := output.FormatPrompt(req.Prompt, req.Options) + formatted := output.FormatPrompt(req.Prompt(), req.Options()) firstLine := strings.Split(formatted, "\n")[0] - if selected == "" || !hasLabels || selectedKey == "any" { + if selected == "" || !hasLabels || selectedKey == output.KeyAny { return firstLine } return fmt.Sprintf("%s %s", firstLine, selected) } // resolveOption finds the best matching option for a key event, in priority order: -// 1. "any" — matches any keypress +// 1. output.KeyAny — matches any keypress // 2. "enter" — matches the Enter key explicitly // 3. uppercase label — matches Enter as the conventional default // 4. case-insensitive key match — matches any other key @@ -503,7 +504,7 @@ func resolveOption(options []output.InputOption, msg tea.KeyMsg) *output.InputOp var uppercaseDefault *output.InputOption for i, opt := range options { switch { - case opt.Key == "any": + case opt.Key == output.KeyAny: return &options[i] case msg.Type == tea.KeyEnter && opt.Key == "enter": return &options[i] diff --git a/internal/ui/app_test.go b/internal/ui/app_test.go index 9d19d7c8..9286dad6 100644 --- a/internal/ui/app_test.go +++ b/internal/ui/app_test.go @@ -169,11 +169,7 @@ func TestAppEnterRespondsToInputRequest(t *testing.T) { app := NewApp("dev", "", "", nil) responseCh := make(chan output.InputResponse, 1) - model, _ := app.Update(output.UserInputRequestEvent{ - Prompt: "Press enter", - Options: []output.InputOption{{Key: "enter", Label: "Continue"}}, - ResponseCh: responseCh, - }) + model, _ := app.Update(output.Acknowledge("Press enter", "Continue", responseCh)) app = model.(App) if !app.inputPrompt.Visible() { @@ -189,8 +185,8 @@ func TestAppEnterRespondsToInputRequest(t *testing.T) { select { case resp := <-responseCh: - if resp.SelectedKey != "enter" { - t.Fatalf("expected enter key, got %q", resp.SelectedKey) + if resp.SelectedKey != output.KeyAny { + t.Fatalf("expected the any-key option, got %q", resp.SelectedKey) } case <-time.After(time.Second): t.Fatal("timed out waiting for response on channel") @@ -210,11 +206,9 @@ func TestAppDismissesOnlyTheMatchingPendingInput(t *testing.T) { responseCh := make(chan output.InputResponse, 1) prompt := "LocalStack is still starting." - model, _ = app.Update(output.UserInputRequestEvent{ - Prompt: prompt, - Options: []output.InputOption{{Key: "w", Label: "[W] Keep waiting"}}, - ResponseCh: responseCh, - }) + model, _ = app.Update(output.ActionChoice(prompt, []output.InputOption{ + {Key: "w", Label: "Keep waiting"}, + }, responseCh)) app = model.(App) model, _ = app.Update(output.UserInputDismissEvent{ResponseCh: make(chan output.InputResponse, 1)}) @@ -255,13 +249,12 @@ func TestAppPendingInputSurvivesDeferredSpinnerStop(t *testing.T) { t.Fatal("expected the spinner stop to be deferred by the min duration") } - prompt := "License validation failed. Log in again to refresh your credentials?" + prompt := "License validation failed: token expired." responseCh := make(chan output.InputResponse, 1) - model, _ = app.Update(output.UserInputRequestEvent{ - Prompt: prompt, - Options: []output.InputOption{{Key: "enter", Label: "ENTER to log in again"}}, - ResponseCh: responseCh, - }) + model, _ = app.Update(output.ActionChoice(prompt, []output.InputOption{ + {Key: "r", Label: "Re-authenticate"}, + {Key: "esc", Label: "Exit"}, + }, responseCh)) app = model.(App) model, _ = app.Update(components.SpinnerMinDurationElapsedMsg{}) @@ -286,8 +279,8 @@ func TestAppPendingInputSurvivesDeferredSpinnerStop(t *testing.T) { select { case resp := <-responseCh: - if resp.SelectedKey != "enter" { - t.Fatalf("expected enter key, got %q", resp.SelectedKey) + if resp.SelectedKey != "r" { + t.Fatalf("expected enter to select the highlighted action, got %q", resp.SelectedKey) } case <-time.After(time.Second): t.Fatal("timed out waiting for response on channel") @@ -299,28 +292,23 @@ func TestAppPendingInputSurvivesDeferredSpinnerStop(t *testing.T) { // TestAppPromptWrapsAtTerminalWidth covers the other half of DEVX-1045: a long // prompt has to be wrapped to the terminal width, since Bubble Tea's renderer -// would otherwise truncate the key hints off the right edge. +// would otherwise truncate the key hints off the right edge. An inline +// confirmation is the case that can lose them — its hint sits at the very end +// of the question rather than on rows of its own. func TestAppPromptWrapsAtTerminalWidth(t *testing.T) { t.Parallel() const width = 40 - question := "License validation failed: invalid, inactive, or expired authentication token or subscription. Log in again to refresh your credentials?" + question := "Delete cloud snapshot 'pod:nightly-regression-baseline'? This operation cannot be undone." app := NewApp("dev", "", "", nil) model, _ := app.Update(tea.WindowSizeMsg{Width: width}) app = model.(App) - model, _ = app.Update(output.UserInputRequestEvent{ - Prompt: question, - Options: []output.InputOption{ - {Key: "enter", Label: "ENTER to log in again"}, - {Key: "esc", Label: "ESC to exit"}, - }, - ResponseCh: make(chan output.InputResponse, 1), - }) + model, _ = app.Update(output.Confirm(question, output.DefaultNo, make(chan output.InputResponse, 1))) app = model.(App) view := app.View() - if !strings.Contains(view, "[ENTER to log in again/ESC to exit]") { + if !strings.Contains(view, "[y/N]") { t.Errorf("expected the key hints to be rendered, got:\n%s", view) } if strings.Contains(view, question) { @@ -335,11 +323,7 @@ func TestAppCtrlCCancelsPendingInput(t *testing.T) { app := NewApp("dev", "", "", func() { cancelled = true }) responseCh := make(chan output.InputResponse, 1) - model, _ := app.Update(output.UserInputRequestEvent{ - Prompt: "Press enter", - Options: []output.InputOption{{Key: "enter", Label: "Continue"}}, - ResponseCh: responseCh, - }) + model, _ := app.Update(output.Acknowledge("Press enter", "Continue", responseCh)) app = model.(App) model, cmd := app.Update(tea.KeyMsg{Type: tea.KeyCtrlC}) @@ -696,134 +680,29 @@ func TestAppNonSilentErrorShowsInErrorDisplay(t *testing.T) { } } -func TestAppEnterPrefersExplicitEnterOption(t *testing.T) { - t.Parallel() - - app := NewApp("dev", "", "", nil) - responseCh := make(chan output.InputResponse, 1) - - model, _ := app.Update(output.UserInputRequestEvent{ - Prompt: "Open browser now?", - Options: []output.InputOption{ - {Key: "y", Label: "Y"}, - {Key: "n", Label: "n"}, - {Key: "enter", Label: "Press ENTER when complete"}, - }, - ResponseCh: responseCh, - }) - app = model.(App) - - model, cmd := app.Update(tea.KeyMsg{Type: tea.KeyEnter}) - app = model.(App) - if cmd == nil { - t.Fatal("expected response command") - } - cmd() - - select { - case resp := <-responseCh: - if resp.SelectedKey != "enter" { - t.Fatalf("expected enter key, got %q", resp.SelectedKey) - } - case <-time.After(time.Second): - t.Fatal("timed out waiting for response on channel") - } - - if app.inputPrompt.Visible() { - t.Fatal("expected input prompt to be hidden after response") - } -} - -func TestAppEnterSelectsUppercaseLabelDefault(t *testing.T) { +// TestAppUnmatchedKeyLeavesPromptPending covers the app-level half of a +// resolveOption miss: no response command, and the prompt stays on screen still +// waiting for an answer. +// +// The matching rules themselves belong to TestResolveOption, which feeds them +// arbitrary option slices. That is now the only place they can be exercised at +// all: every prompt a user sees comes from a constructor, so the shapes this +// file used to build by hand to reach them one at a time — an explicit "enter" +// option outranking an uppercase default, all-lowercase labels, non-letter +// labels — can no longer be built outside internal/output. +func TestAppUnmatchedKeyLeavesPromptPending(t *testing.T) { t.Parallel() app := NewApp("dev", "", "", nil) responseCh := make(chan output.InputResponse, 1) - model, _ := app.Update(output.UserInputRequestEvent{ - Prompt: "Open browser now?", - Options: []output.InputOption{ - {Key: "y", Label: "Y"}, - {Key: "n", Label: "n"}, - }, - ResponseCh: responseCh, - }) + model, _ := app.Update(output.Confirm("Open browser now?", output.DefaultYes, responseCh)) app = model.(App) - model, cmd := app.Update(tea.KeyMsg{Type: tea.KeyEnter}) - app = model.(App) - if cmd == nil { - t.Fatal("expected response command when enter is pressed with uppercase default") - } - cmd() - - select { - case resp := <-responseCh: - if resp.SelectedKey != "y" { - t.Fatalf("expected y key, got %q", resp.SelectedKey) - } - case <-time.After(time.Second): - t.Fatal("timed out waiting for response on channel") - } - - if app.inputPrompt.Visible() { - t.Fatal("expected input prompt to be hidden after response") - } -} - -func TestAppEnterDoesNothingWithoutDefault(t *testing.T) { - t.Parallel() - - app := NewApp("dev", "", "", nil) - responseCh := make(chan output.InputResponse, 1) - - model, _ := app.Update(output.UserInputRequestEvent{ - Prompt: "Choose:", - Options: []output.InputOption{ - {Key: "y", Label: "y"}, - {Key: "n", Label: "n"}, - }, - ResponseCh: responseCh, - }) - app = model.(App) - - model, cmd := app.Update(tea.KeyMsg{Type: tea.KeyEnter}) - app = model.(App) - if cmd != nil { - t.Fatal("expected no response command when no uppercase default option exists") - } - - select { - case resp := <-responseCh: - t.Fatalf("expected no response, got %+v", resp) - case <-time.After(200 * time.Millisecond): - } - - if !app.inputPrompt.Visible() { - t.Fatal("expected input prompt to remain visible") - } -} - -func TestAppEnterDoesNothingWithNonLetterLabel(t *testing.T) { - t.Parallel() - - app := NewApp("dev", "", "", nil) - responseCh := make(chan output.InputResponse, 1) - - model, _ := app.Update(output.UserInputRequestEvent{ - Prompt: "Choose:", - Options: []output.InputOption{ - {Key: "1", Label: "1"}, - {Key: "2", Label: "2"}, - }, - ResponseCh: responseCh, - }) - app = model.(App) - - model, cmd := app.Update(tea.KeyMsg{Type: tea.KeyEnter}) + model, cmd := app.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'x'}}) app = model.(App) if cmd != nil { - t.Fatal("expected no response command when label contains no letters") + t.Fatal("expected no response command for a key no option claims") } select { @@ -843,12 +722,11 @@ func TestAppEnterSelectsHighlightedVerticalOption(t *testing.T) { app := NewApp("dev", "", "", nil) responseCh := make(chan output.InputResponse, 1) - model, _ := app.Update(output.UserInputRequestEvent{ - Prompt: "Update lstk to latest version?", - Options: []output.InputOption{{Key: "u", Label: "Update now [U]"}, {Key: "s", Label: "Skip this version [S]"}, {Key: "n", Label: "Never ask again [N]"}}, - ResponseCh: responseCh, - Vertical: true, - }) + model, _ := app.Update(output.ActionChoice("Update lstk to latest version?", []output.InputOption{ + {Key: "u", Label: "Update now"}, + {Key: "s", Label: "Skip this version"}, + {Key: "n", Label: "Never ask again"}, + }, responseCh)) app = model.(App) model, _ = app.Update(tea.KeyMsg{Type: tea.KeyDown}) @@ -885,15 +763,14 @@ func TestAppEscResolvesVerticalDeclineOption(t *testing.T) { app := NewApp("dev", "", "", nil) responseCh := make(chan output.InputResponse, 1) - model, _ := app.Update(output.UserInputRequestEvent{ - Prompt: "License validation failed: invalid, inactive, or expired authentication token or subscription.", - Options: []output.InputOption{ - {Key: "r", Label: "[R] Re-authenticate"}, - {Key: "esc", Label: "[ESC] Exit"}, + model, _ := app.Update(output.ActionChoice( + "License validation failed: invalid, inactive, or expired authentication token or subscription.", + []output.InputOption{ + {Key: "r", Label: "Re-authenticate"}, + {Key: "esc", Label: "Exit"}, }, - ResponseCh: responseCh, - Vertical: true, - }) + responseCh, + )) app = model.(App) model, cmd := app.Update(tea.KeyMsg{Type: tea.KeyEscape}) @@ -926,15 +803,14 @@ func TestAppReloginShortcutIgnoresVerticalSelection(t *testing.T) { app := NewApp("dev", "", "", nil) responseCh := make(chan output.InputResponse, 1) - model, _ := app.Update(output.UserInputRequestEvent{ - Prompt: "License validation failed: invalid, inactive, or expired authentication token or subscription.", - Options: []output.InputOption{ - {Key: "r", Label: "[R] Re-authenticate"}, - {Key: "esc", Label: "[ESC] Exit"}, + model, _ := app.Update(output.ActionChoice( + "License validation failed: invalid, inactive, or expired authentication token or subscription.", + []output.InputOption{ + {Key: "r", Label: "Re-authenticate"}, + {Key: "esc", Label: "Exit"}, }, - ResponseCh: responseCh, - Vertical: true, - }) + responseCh, + )) app = model.(App) model, _ = app.Update(tea.KeyMsg{Type: tea.KeyDown}) @@ -960,17 +836,60 @@ func TestAppReloginShortcutIgnoresVerticalSelection(t *testing.T) { } } +// TestAppEnterHonorsTheConfirmDefault pins the contract that lets output.Confirm +// advertise its default by capitalizing one label: ENTER must select whichever +// answer is capitalized, so a destructive prompt built with DefaultNo cannot be +// confirmed by a stray ENTER. +func TestAppEnterHonorsTheConfirmDefault(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + def output.ConfirmDefault + want string + }{ + {name: "default yes", def: output.DefaultYes, want: output.KeyYes}, + {name: "default no", def: output.DefaultNo, want: output.KeyNo}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + app := NewApp("dev", "", "", nil) + responseCh := make(chan output.InputResponse, 1) + + model, _ := app.Update(output.Confirm("Reset emulator state? All resources will be lost", tc.def, responseCh)) + app = model.(App) + + model, cmd := app.Update(tea.KeyMsg{Type: tea.KeyEnter}) + app = model.(App) + if cmd == nil { + t.Fatal("expected enter to resolve the confirmation") + } + cmd() + + select { + case resp := <-responseCh: + if resp.SelectedKey != tc.want { + t.Fatalf("expected enter to select %q, got %q", tc.want, resp.SelectedKey) + } + case <-time.After(time.Second): + t.Fatal("timed out waiting for response on channel") + } + + if app.inputPrompt.Visible() { + t.Fatal("expected input prompt to be hidden after response") + } + }) + } +} + func TestAppAnyKeyOptionResolvesOnAnyKeypress(t *testing.T) { t.Parallel() app := NewApp("dev", "", "", nil) responseCh := make(chan output.InputResponse, 1) - model, _ := app.Update(output.UserInputRequestEvent{ - Prompt: "Waiting for authorization...", - Options: []output.InputOption{{Key: "any", Label: "Press any key when complete"}}, - ResponseCh: responseCh, - }) + model, _ := app.Update(output.Acknowledge("Waiting for authorization...", "Press any key when complete", responseCh)) app = model.(App) // Any key (e.g., spacebar) should resolve @@ -1088,14 +1007,10 @@ func TestAppPendingInputOptionCOverridesClipboardShortcut(t *testing.T) { model, _ := app.Update(output.AuthEvent{URL: "https://example.com"}) app = model.(App) - model, _ = app.Update(output.UserInputRequestEvent{ - Prompt: "Choose option", - Options: []output.InputOption{ - {Key: "c", Label: "Continue"}, - {Key: "x", Label: "Cancel"}, - }, - ResponseCh: responseCh, - }) + model, _ = app.Update(output.ActionChoice("Choose option", []output.InputOption{ + {Key: "c", Label: "Continue"}, + {Key: "x", Label: "Cancel"}, + }, responseCh)) app = model.(App) model, cmd := app.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'c'}}) @@ -1192,6 +1107,12 @@ func TestResolveOption(t *testing.T) { press: enter, wantOptionKey: "", }, + { + name: "all-lowercase labels leave Enter unanswered", + options: []output.InputOption{{Key: "y", Label: "y"}, {Key: "n", Label: "n"}}, + press: enter, + wantOptionKey: "", + }, // case-insensitive key matching { diff --git a/internal/ui/components/input_prompt.go b/internal/ui/components/input_prompt.go index 245cae55..f322ed21 100644 --- a/internal/ui/components/input_prompt.go +++ b/internal/ui/components/input_prompt.go @@ -123,10 +123,11 @@ func (p InputPrompt) viewVertical(width int) string { } for i, opt := range p.options { + label := output.OptionLabel(opt) if i == p.selectedIndex { - sb.WriteString(styles.NimboMid.Render("● " + opt.Label)) + sb.WriteString(styles.NimboMid.Render("● " + label)) } else { - sb.WriteString(styles.Secondary.Render("○ " + opt.Label)) + sb.WriteString(styles.Secondary.Render("○ " + label)) } sb.WriteString("\n") } diff --git a/internal/ui/components/input_prompt_test.go b/internal/ui/components/input_prompt_test.go index 7865efd6..69063e07 100644 --- a/internal/ui/components/input_prompt_test.go +++ b/internal/ui/components/input_prompt_test.go @@ -167,15 +167,16 @@ func TestInputPromptViewSlowStartChoicesAreScannable(t *testing.T) { // TestInputPromptViewReloginChoicesAreScannable covers the license re-login // prompt: its question is long enough to wrap, so flattening the two choices // into a trailing hint made them read as prose. They belong on their own lines -// below the wrapped question, shortcut first. +// below the wrapped question, shortcut first — and the shortcut is derived from +// each option's key, so a plain-prose label still advertises the key to press. func TestInputPromptViewReloginChoicesAreScannable(t *testing.T) { t.Parallel() const width = 40 question := "License validation failed: invalid, inactive, or expired authentication token or subscription." p := NewInputPrompt().Show(question, []output.InputOption{ - {Key: "r", Label: "[R] Re-authenticate"}, - {Key: "esc", Label: "[ESC] Exit"}, + {Key: "r", Label: "Re-authenticate"}, + {Key: "esc", Label: "Exit"}, }, true) view := p.View(width) diff --git a/internal/update/notify.go b/internal/update/notify.go index 29a14a38..1d6a48a8 100644 --- a/internal/update/notify.go +++ b/internal/update/notify.go @@ -79,12 +79,11 @@ func promptAndUpdate(ctx context.Context, sink output.Sink, opts NotifyOptions, sink.Emit(output.MessageEvent{Severity: output.SeveritySecondary, Text: fmt.Sprintf("> Release notes: %s", releaseNotesURL)}) responseCh := make(chan output.InputResponse, 1) - sink.Emit(output.UserInputRequestEvent{ - Prompt: "Update lstk to latest version?", - Options: []output.InputOption{{Key: "u", Label: "Update now [U]"}, {Key: "r", Label: "Remind me next time [R]"}, {Key: "s", Label: "Skip this version [S]"}}, - ResponseCh: responseCh, - Vertical: true, - }) + sink.Emit(output.ActionChoice("Update lstk to latest version?", []output.InputOption{ + {Key: "u", Label: "Update now"}, + {Key: "r", Label: "Remind me next time"}, + {Key: "s", Label: "Skip this version"}, + }, responseCh)) var resp output.InputResponse select { diff --git a/internal/update/notify_test.go b/internal/update/notify_test.go index 05f13321..499b0916 100644 --- a/internal/update/notify_test.go +++ b/internal/update/notify_test.go @@ -117,7 +117,7 @@ func TestNotifyUpdatePromptSkip(t *testing.T) { sink := output.SinkFunc(func(event output.Event) { events = append(events, event) if req, ok := event.(output.UserInputRequestEvent); ok { - req.ResponseCh <- output.InputResponse{SelectedKey: "s"} + req.ResponseCh() <- output.InputResponse{SelectedKey: "s"} } }) @@ -155,7 +155,7 @@ func TestNotifyUpdatePromptRemind(t *testing.T) { sink := output.SinkFunc(func(event output.Event) { events = append(events, event) if req, ok := event.(output.UserInputRequestEvent); ok { - req.ResponseCh <- output.InputResponse{SelectedKey: "r"} + req.ResponseCh() <- output.InputResponse{SelectedKey: "r"} } }) @@ -171,12 +171,12 @@ func TestNotifyUpdatePromptCancelled(t *testing.T) { sink := output.SinkFunc(func(event output.Event) { events = append(events, event) if req, ok := event.(output.UserInputRequestEvent); ok { - assert.Equal(t, "Update lstk to latest version?", req.Prompt) - assert.Len(t, req.Options, 3) - assert.Equal(t, "u", req.Options[0].Key) - assert.Equal(t, "r", req.Options[1].Key) - assert.Equal(t, "s", req.Options[2].Key) - req.ResponseCh <- output.InputResponse{Cancelled: true} + assert.Equal(t, "Update lstk to latest version?", req.Prompt()) + assert.Len(t, req.Options(), 3) + assert.Equal(t, "u", req.Options()[0].Key) + assert.Equal(t, "r", req.Options()[1].Key) + assert.Equal(t, "s", req.Options()[2].Key) + req.ResponseCh() <- output.InputResponse{Cancelled: true} } }) diff --git a/internal/volume/clear.go b/internal/volume/clear.go index 626f251a..550fb2c9 100644 --- a/internal/volume/clear.go +++ b/internal/volume/clear.go @@ -38,18 +38,11 @@ func Clear(ctx context.Context, sink output.Sink, containers []config.ContainerC if !force { responseCh := make(chan output.InputResponse, 1) - sink.Emit(output.UserInputRequestEvent{ - Prompt: "Clear volume data? This cannot be undone", - Options: []output.InputOption{ - {Key: "y", Label: "Yes"}, - {Key: "n", Label: "NO"}, - }, - ResponseCh: responseCh, - }) + sink.Emit(output.Confirm("Clear volume data? This cannot be undone", output.DefaultNo, responseCh)) select { case resp := <-responseCh: - if resp.Cancelled || resp.SelectedKey != "y" { + if resp.Cancelled || resp.SelectedKey != output.KeyYes { sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: "Cancelled"}) return nil } diff --git a/test/integration/emulator_select_test.go b/test/integration/emulator_select_test.go index 8dead502..2f023658 100644 --- a/test/integration/emulator_select_test.go +++ b/test/integration/emulator_select_test.go @@ -60,6 +60,12 @@ func TestFirstRunShowsEmulatorSelectionPrompt(t *testing.T) { p.waitForOutput("Which emulator would you like to use?", "emulator selection prompt should appear on first run") + // Each choice is a selectable row advertising the key that picks it directly. + // The shortcut is derived from the option's key by output.OptionLabel, so a + // picker whose labels are bare names still tells the user what to press. + p.waitForOutput("[A] AWS", "each emulator row should advertise its shortcut") + p.waitForOutput("[Z] Azure", "each emulator row should advertise its shortcut") + // Confirm the default-highlighted option (AWS) by pressing Enter. p.write("\r") diff --git a/test/integration/volume_test.go b/test/integration/volume_test.go index eb5d4ea6..90455087 100644 --- a/test/integration/volume_test.go +++ b/test/integration/volume_test.go @@ -291,7 +291,9 @@ volume = "` + escapeTomlPath(volumeDir) + `" startVolumeClear := func(t *testing.T, configFile string) *ptyProc { t.Helper() p := startLstkInPTY(t, testContext(t), testEnvWithHome(t.TempDir(), ""), "--config", configFile, "volume", "clear") - p.waitForOutput("Clear volume data?", "confirmation prompt should appear") + // An irreversible confirmation stays inline and capitalizes the answer + // ENTER picks, so a stray ENTER cannot wipe the volume. + p.waitForOutput("Clear volume data? This cannot be undone [y/N]", "confirmation prompt should appear") return p }