diff --git a/internal/container/start.go b/internal/container/start.go index 9dad0524..2a6c1c9f 100644 --- a/internal/container/start.go +++ b/internal/container/start.go @@ -1528,6 +1528,12 @@ func (m *startupMonitor) await(ctx context.Context, containerID, healthURL strin defer deadline.Stop() ticker := time.NewTicker(1 * time.Second) defer ticker.Stop() + var responseCh chan output.InputResponse + defer func() { + if responseCh != nil { + m.sink.Emit(output.UserInputDismissEvent{ResponseCh: responseCh}) + } + }() check := func() (ready bool, err error) { running, err := m.rt.IsRunning(ctx, containerID) @@ -1574,6 +1580,29 @@ func (m *startupMonitor) await(ctx context.Context, containerID, healthURL strin select { case <-ctx.Done(): return ctx.Err() + case resp := <-responseCh: + // The TUI has already hidden a prompt once it sends a response. + responseCh = nil + if resp.Cancelled { + if ctx.Err() != nil { + return ctx.Err() + } + return context.Canceled + } + if resp.SelectedKey == "s" { + // Readiness may have changed between the last poll and the keypress. + // Never stop an emulator that became ready while the prompt was open. + if ready, err := check(); err != nil || ready { + return err + } + m.sink.Emit(output.SpinnerStart("Stopping LocalStack...")) + // Best-effort stop; the timeout error is authoritative either way. + _ = m.rt.Stop(ctx, containerID) + return &startupTimeoutError{timeout: m.timeout, stopped: true} + } + + deadline.Reset(m.timeout) + m.sink.Emit(output.SpinnerStart("Starting LocalStack")) case res := <-exitCh: if res.Err != nil { // The wait itself failed (e.g. an exit+removal race before the @@ -1584,16 +1613,26 @@ func (m *startupMonitor) await(ctx context.Context, containerID, healthURL strin } return &containerExitedError{exitCode: res.ExitCode} case <-deadline.C: - surface, stopped, err := m.handleTimeout(ctx, containerID) - if err != nil { + // Avoid surfacing a stale timeout if readiness changed since the last + // poll, especially when the ticker and deadline become ready together. + if ready, err := check(); err != nil || ready { return err } - if surface { - return &startupTimeoutError{timeout: m.timeout, stopped: stopped} + if !m.interactive { + return &startupTimeoutError{timeout: m.timeout} } - // Keep waiting: re-arm the deadline and restore the spinner. - deadline.Reset(m.timeout) - m.sink.Emit(output.SpinnerStart("Starting LocalStack")) + + 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"}, + }, + ResponseCh: responseCh, + Vertical: true, + }) case <-ticker.C: if ready, err := check(); err != nil || ready { return err @@ -1602,51 +1641,6 @@ func (m *startupMonitor) await(ctx context.Context, containerID, healthURL strin } } -// handleTimeout decides what to do when the startup deadline elapses. In -// non-interactive mode it always surfaces the timeout, leaving the container -// running so the user can inspect it. In interactive mode it prompts the user to -// keep waiting or stop; "keep waiting" returns surface=false so the caller -// re-arms the deadline. stopped reports whether the container was stopped (the -// user chose "stop"), so the timeout error can describe its actual state. -func (m *startupMonitor) handleTimeout(ctx context.Context, containerID string) (surface, stopped bool, err error) { - if !m.interactive { - return true, false, nil - } - - m.sink.Emit(output.SpinnerStop()) - responseCh := make(chan output.InputResponse, 1) - m.sink.Emit(output.UserInputRequestEvent{ - Prompt: "LocalStack is taking longer than expected to start. Check logs with 'lstk logs'", - Options: []output.InputOption{ - {Key: "w", Label: "Keep waiting [W]"}, - {Key: "s", Label: "Stop LocalStack and exit [S]"}, - }, - ResponseCh: responseCh, - }) - - select { - case resp := <-responseCh: - if resp.Cancelled { - // Ctrl+C: leave the container running (it is detached). - if ctx.Err() != nil { - return false, false, ctx.Err() - } - return false, false, context.Canceled - } - if resp.SelectedKey == "s" { - // Stopping takes a few seconds; show progress so the CLI does not - // look hung after the keypress. The caller's SpinnerStop closes it. - m.sink.Emit(output.SpinnerStart("Stopping LocalStack...")) - // Best-effort stop; the timeout error is authoritative either way. - _ = m.rt.Stop(ctx, containerID) - return true, true, nil - } - return false, false, nil - case <-ctx.Done(): - return false, false, ctx.Err() - } -} - // lastLogLines returns the last n non-empty lines of logs, for including in an // error summary. Returns "" when logs is empty. func lastLogLines(logs string, n int) string { diff --git a/internal/container/start_test.go b/internal/container/start_test.go index 59f88e06..88365d19 100644 --- a/internal/container/start_test.go +++ b/internal/container/start_test.go @@ -821,9 +821,11 @@ func TestStartupMonitorAwait_InteractivePromptKeepWaitingThenStop(t *testing.T) mockRT.EXPECT().Stop(gomock.Any(), "cid").Return(nil) prompts := make(chan output.UserInputRequestEvent, 2) + seenPrompts := make(chan output.UserInputRequestEvent, 2) sink := output.SinkFunc(func(event output.Event) { if req, ok := event.(output.UserInputRequestEvent); ok { prompts <- req + seenPrompts <- req } }) @@ -846,6 +848,80 @@ func TestStartupMonitorAwait_InteractivePromptKeepWaitingThenStop(t *testing.T) var timeoutErr *startupTimeoutError require.ErrorAs(t, err, &timeoutErr) 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, []output.InputOption{ + {Key: "w", Label: "[W] Keep waiting"}, + {Key: "s", Label: "[S] Stop and exit"}, + }, firstPrompt.Options) +} + +func TestStartupMonitorAwait_DismissesPromptWhenEmulatorBecomesReady(t *testing.T) { + ctrl := gomock.NewController(t) + mockRT := runtime.NewMockRuntime(ctrl) + mockRT.EXPECT().IsRunning(gomock.Any(), "cid").Return(true, nil).AnyTimes() + + var ready atomic.Bool + healthServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if ready.Load() { + w.WriteHeader(http.StatusOK) + return + } + w.WriteHeader(http.StatusServiceUnavailable) + })) + defer healthServer.Close() + + prompts := make(chan output.UserInputRequestEvent, 1) + dismissals := make(chan output.UserInputDismissEvent, 1) + sink := output.SinkFunc(func(event output.Event) { + switch event := event.(type) { + case output.UserInputRequestEvent: + prompts <- event + ready.Store(true) + case output.UserInputDismissEvent: + dismissals <- event + } + }) + + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + monitor := newStartupMonitor(mockRT, sink, nil, 25*time.Millisecond, true) + err := monitor.await(ctx, "cid", healthServer.URL, make(chan runtime.ExitResult)) + + require.NoError(t, err) + prompt := <-prompts + dismissal := <-dismissals + assert.Equal(t, prompt.ResponseCh, dismissal.ResponseCh) +} + +func TestStartupMonitorAwait_DoesNotStopEmulatorThatBecameReadyBeforeSelection(t *testing.T) { + ctrl := gomock.NewController(t) + mockRT := runtime.NewMockRuntime(ctrl) + mockRT.EXPECT().IsRunning(gomock.Any(), "cid").Return(true, nil).AnyTimes() + + var ready atomic.Bool + healthServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if ready.Load() { + w.WriteHeader(http.StatusOK) + return + } + w.WriteHeader(http.StatusServiceUnavailable) + })) + defer healthServer.Close() + + sink := output.SinkFunc(func(event output.Event) { + if prompt, ok := event.(output.UserInputRequestEvent); ok { + ready.Store(true) + prompt.ResponseCh <- output.InputResponse{SelectedKey: "s"} + } + }) + + monitor := newStartupMonitor(mockRT, sink, nil, 25*time.Millisecond, true) + err := monitor.await(context.Background(), "cid", healthServer.URL, make(chan runtime.ExitResult)) + + require.NoError(t, err) } func TestStartupMonitorAwait_ReturnsExitCodeFromExitCh(t *testing.T) { diff --git a/internal/output/events.go b/internal/output/events.go index 5269d269..bbd6583c 100644 --- a/internal/output/events.go +++ b/internal/output/events.go @@ -245,6 +245,7 @@ func (MultipleInstallsEvent) sealedEvent() {} func (ContainerStatusEvent) sealedEvent() {} func (ProgressEvent) sealedEvent() {} func (UserInputRequestEvent) sealedEvent() {} +func (UserInputDismissEvent) sealedEvent() {} func (PullSkippableEvent) sealedEvent() {} func (LogLineEvent) sealedEvent() {} @@ -292,6 +293,13 @@ type UserInputRequestEvent struct { Vertical bool } +// 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. +type UserInputDismissEvent struct { + ResponseCh chan<- InputResponse +} + // PullSkippableEvent signals that an in-flight image pull can be abandoned in // favor of an already-present local image. The domain emits it once real layer // download begins (interactive mode, with a local copy present); the TUI binds diff --git a/internal/output/plain_format.go b/internal/output/plain_format.go index 7b598927..57b24a6a 100644 --- a/internal/output/plain_format.go +++ b/internal/output/plain_format.go @@ -34,6 +34,8 @@ func FormatEventLine(event Event) (string, bool) { return "", false case UserInputRequestEvent: return formatUserInputRequest(e), true + case UserInputDismissEvent: + return "", false case PullSkippableEvent: // Interactive-only affordance with no plain-text rendering: non-interactive // pulls never emit it, and PlainSink cannot bind the ESC key. diff --git a/internal/ui/app.go b/internal/ui/app.go index 0877afff..8a7ef191 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -197,6 +197,14 @@ func (a App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if a.spinner.Visible() { a.spinner = a.spinner.SetText(output.FormatPrompt(msg.Prompt, msg.Options)) } + case output.UserInputDismissEvent: + if a.pendingInput == nil || a.pendingInput.ResponseCh != msg.ResponseCh { + return a, nil + } + a.pendingInput = nil + a.inputPrompt = a.inputPrompt.Hide() + a.spinner = a.spinner.SetText("") + return a, nil case output.PullSkippableEvent: msgCopy := msg a.pullSkip = &msgCopy diff --git a/internal/ui/app_test.go b/internal/ui/app_test.go index 2e2dcaeb..aa5b2d8e 100644 --- a/internal/ui/app_test.go +++ b/internal/ui/app_test.go @@ -201,6 +201,38 @@ func TestAppEnterRespondsToInputRequest(t *testing.T) { } } +func TestAppDismissesOnlyTheMatchingPendingInput(t *testing.T) { + t.Parallel() + + app := NewApp("dev", "", "", nil) + model, _ := app.Update(output.SpinnerStart("Starting LocalStack")) + app = model.(App) + + 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, + }) + app = model.(App) + + model, _ = app.Update(output.UserInputDismissEvent{ResponseCh: make(chan output.InputResponse, 1)}) + app = model.(App) + if !app.inputPrompt.Visible() { + t.Fatal("expected an unrelated dismissal to leave the prompt visible") + } + + model, _ = app.Update(output.UserInputDismissEvent{ResponseCh: responseCh}) + app = model.(App) + if app.pendingInput != nil || app.inputPrompt.Visible() { + t.Fatal("expected the matching prompt to be dismissed") + } + if view := app.View(); strings.Contains(view, prompt) { + t.Fatalf("expected the spinner's prompt mirror to be cleared, got:\n%s", view) + } +} + // TestAppPendingInputSurvivesDeferredSpinnerStop covers DEVX-1045: a prompt // emitted right after a spinner was stopped inside its min duration used to be // parked in the spinner's text, and the min-duration tick then erased it. The diff --git a/internal/ui/components/input_prompt_test.go b/internal/ui/components/input_prompt_test.go index 4441ca34..94e88bfc 100644 --- a/internal/ui/components/input_prompt_test.go +++ b/internal/ui/components/input_prompt_test.go @@ -135,3 +135,31 @@ func TestInputPromptViewUnwrappedWithoutWidth(t *testing.T) { t.Errorf("expected no wrapping without a known width, got: %q", view) } } + +func TestInputPromptViewSlowStartChoicesAreScannable(t *testing.T) { + t.Parallel() + + const width = 80 + question := "LocalStack is still starting. Check progress with 'lstk logs'." + p := NewInputPrompt().Show(question, []output.InputOption{ + {Key: "w", Label: "[W] Keep waiting"}, + {Key: "s", Label: "[S] Stop and exit"}, + }, true) + + view := p.View(width) + lines := strings.Split(view, "\n") + if len(lines) < 3 { + t.Fatalf("expected a question and two vertical choices, got:\n%s", view) + } + if !strings.Contains(lines[0], question) { + t.Fatalf("expected the question and log command on one line, got:\n%s", view) + } + if !strings.Contains(lines[1], "[W] Keep waiting") || !strings.Contains(lines[2], "[S] Stop and exit") { + t.Fatalf("expected prefixed shortcuts on separate lines, got:\n%s", view) + } + for _, line := range lines { + if lipgloss.Width(line) > width { + t.Fatalf("line exceeds width %d: %q", width, line) + } + } +}