Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .claude/skills/review-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 7 additions & 9 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down
6 changes: 1 addition & 5 deletions internal/auth/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
8 changes: 2 additions & 6 deletions internal/awsconfig/awsconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
7 changes: 1 addition & 6 deletions internal/container/select.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
39 changes: 18 additions & 21 deletions internal/container/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
41 changes: 24 additions & 17 deletions internal/container/start_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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"}
}
})

Expand Down Expand Up @@ -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"}
}
})

Expand Down Expand Up @@ -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}
}
})

Expand All @@ -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
Expand All @@ -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]))
})
}
}
36 changes: 32 additions & 4 deletions internal/output/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
34 changes: 32 additions & 2 deletions internal/output/plain_format.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
Loading
Loading