Skip to content

Show container start guidance on more commands #168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gtsiolis wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Show container start guidance on more commands #168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions cmd/logs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ func newLogsCmd(cfg *env.Env, tel *telemetry.Client) *cobra.Command {
if err != nil {
return err
}
if err := checkRuntimeHealth(cmd.Context(), rt, cfg); err != nil {
return err
}
appConfig, err := config.Get()
if err != nil {
return fmt.Errorf("failed to get config: %w", err)
Expand Down
11 changes: 11 additions & 0 deletions cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,17 @@ func isInteractiveMode(cfg *env.Env) bool {
return !cfg.NonInteractive && ui.IsInteractive()
}

// checkRuntimeHealth checks if the runtime is healthy and emits an error
// through the sink if not in interactive mode. Returns a SilentError to
// suppress duplicate error printing.
func checkRuntimeHealth(ctx context.Context, rt runtime.Runtime, cfg *env.Env) error {
if err := rt.IsHealthy(ctx); err != nil {
rt.EmitUnhealthyError(output.NewPlainSink(os.Stdout), err)
return output.NewSilentError(err)
}
return nil
}

Copy link
Copy Markdown
Collaborator

@carole-lavillonniere carole-lavillonniere Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: the cfg *env.Env param can be removed since it is unused.

const maxLogSize = 1 << 20 // 1 MB

func newLogger() (log.Logger, func(), error) {
Expand Down
12 changes: 11 additions & 1 deletion cmd/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,19 @@ func newStatusCmd(cfg *env.Env, tel *telemetry.Client) *cobra.Command {
RunE: commandWithTelemetry("status", tel, func(cmd *cobra.Command, args []string) error {
rt, err := runtime.NewDockerRuntime(cfg.DockerHost)
if err != nil {
output.EmitError(output.NewPlainSink(os.Stdout), output.ErrorEvent{
Title: "Docker is not available",
Summary: err.Error(),
Actions: []output.ErrorAction{
{Label: "See help:", Value: "lstk -h"},
{Label: "Install Docker:", Value: "https://docs.docker.com/get-docker/"},
},
})
return output.NewSilentError(err)
}
Copy link
Copy Markdown
Collaborator

@carole-lavillonniere carole-lavillonniere Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: why this special "docker not available error" handling only for the status command?

if err := checkRuntimeHealth(cmd.Context(), rt, cfg); err != nil {
return err
}
Comment on lines 26 to 40
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Inconsistent error handling between commands and duplicate logic.

status.go now handles NewDockerRuntime failures with a styled error, but logs.go and stop.go return the raw error for the same failure case. This creates inconsistent UX—users get a styled error from lstk status but a plain error from lstk logs when Docker client initialization fails.

Additionally, the error emitted here (lines 28-35) differs from EmitUnhealthyError (used by checkRuntimeHealth): this version omits the OS-specific "Start Docker Desktop" action that EmitUnhealthyError provides.

Consider:

  1. Applying the same NewDockerRuntime error handling to logs.go and stop.go, or
  2. Extracting a shared helper that handles both runtime creation and health check consistently.
🔧 One possible approach: reuse EmitUnhealthyError pattern 
 		rt, err := runtime.NewDockerRuntime(cfg.DockerHost)
 		if err != nil {
-			output.EmitError(output.NewPlainSink(os.Stdout), output.ErrorEvent{
-				Title:   "Docker is not available",
-				Summary: err.Error(),
-				Actions: []output.ErrorAction{
-					{Label: "See help:", Value: "lstk -h"},
-					{Label: "Install Docker:", Value: "https://docs.docker.com/get-docker/"},
-				},
-			})
-			return output.NewSilentError(err)
+			// Create a minimal runtime just for error emission, or inline the error event
+			sink := output.NewPlainSink(os.Stdout)
+			output.EmitError(sink, output.ErrorEvent{
+				Title:   "Docker is not available",
+				Summary: fmt.Sprintf("cannot initialize Docker client: %v", err),
+				Actions: dockerInstallActions(), // shared helper with OS-specific actions
+			})
+			return output.NewSilentError(err)
 		}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@cmd/status.go` around lines 26 - 40, The NewDockerRuntime failure handling in
status.go is inconsistent with logs.go and stop.go and omits the OS-specific
action provided by EmitUnhealthyError; extract a shared helper (e.g.,
initAndCheckRuntime or EnsureHealthyRuntime) that encapsulates
runtime.NewDockerRuntime(cfg.DockerHost) plus the health-check via
checkRuntimeHealth and when NewDockerRuntime returns an error calls the same
styled EmitUnhealthyError/EmitError path used elsewhere (mirroring
EmitUnhealthyError's actions and message), then replace the NewDockerRuntime +
checkRuntimeHealth logic in status.go, logs.go, and stop.go with calls to this
helper so all commands present identical, OS-aware error output.


appCfg, err := config.Get()
if err != nil {
return fmt.Errorf("failed to get config: %w", err)
Expand Down
3 changes: 3 additions & 0 deletions cmd/stop.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ func newStopCmd(cfg *env.Env, tel *telemetry.Client) *cobra.Command {
if err != nil {
return err
}
if err := checkRuntimeHealth(cmd.Context(), rt, cfg); err != nil {
return err
}
Copy link
Copy Markdown
Collaborator

@carole-lavillonniere carole-lavillonniere Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: it looks like the check is missing for the start command

Copy link
Copy Markdown
Collaborator

@carole-lavillonniere carole-lavillonniere Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: we should add a new integration test case for each command to test the behavior when docker is not healthy.

appConfig, err := config.Get()
if err != nil {
return fmt.Errorf("failed to get config: %w", err)
Expand Down
34 changes: 25 additions & 9 deletions internal/output/plain_format.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ import (
"fmt"
"strings"
"time"

"github.com/charmbracelet/lipgloss"
)

var (
errorTitleStyle = lipgloss.NewStyle().Bold(true).Foreground(lipgloss.Color("#C33820"))
errorSecondaryStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("241"))
errorActionStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("69"))
errorValueStyle = lipgloss.NewStyle().Bold(true)
errorMutedStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("245"))
Copy link
Copy Markdown
Collaborator

@carole-lavillonniere carole-lavillonniere Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: do we really mean to redeclare all these, since they already exist in internal/ui/styles/styles.go?

)

// FormatEventLine converts an output event into a single display line.
Expand Down Expand Up @@ -130,21 +140,27 @@ func formatMessageEvent(e MessageEvent) string {

func formatErrorEvent(e ErrorEvent) string {
var sb strings.Builder
sb.WriteString("Error: ")
sb.WriteString(e.Title)
sb.WriteString(errorTitleStyle.Render("✗ " + e.Title))
if e.Summary != "" {
sb.WriteString("\n ")
sb.WriteString("\n")
sb.WriteString(errorSecondaryStyle.Render("> "))
sb.WriteString(e.Summary)
}
if e.Detail != "" {
sb.WriteString("\n ")
sb.WriteString(e.Detail)
sb.WriteString(errorMutedStyle.Render(e.Detail))
}
for _, action := range e.Actions {
sb.WriteString("\n " + ErrorActionPrefix)
sb.WriteString(action.Label)
sb.WriteString(" ")
sb.WriteString(action.Value)
if len(e.Actions) > 0 {
sb.WriteString("\n")
for i, action := range e.Actions {
sb.WriteString("\n")
if i > 0 {
sb.WriteString(errorMutedStyle.Render(ErrorActionPrefix + action.Label + " " + action.Value))
} else {
sb.WriteString(errorActionStyle.Render(ErrorActionPrefix+action.Label+" "))
sb.WriteString(errorValueStyle.Render(action.Value))
}
}
Copy link
Copy Markdown
Collaborator

@carole-lavillonniere carole-lavillonniere Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: I would like to hear what @silv-io says here. We were previously not doing any styling in non-interactive mode.

}
Comment on lines +153 to 164
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there are any tests covering multiple ErrorActions
rg -n 'Actions:.*ErrorAction.*ErrorAction' --type go
# Also check for tests with Actions array having more than one element
rg -n 'Actions: \[\]output\.ErrorAction\{' -A5 --type go | head -50

Repository: localstack/lstk

Length of output: 1684

🏁 Script executed:

cat -n internal/ui/components/error_display_test.go | head -100

Repository: localstack/lstk

Length of output: 3811

🏁 Script executed:

cat -n internal/output/plain_format.go | sed -n '153,164p'

Repository: localstack/lstk

Length of output: 524

🏁 Script executed:

rg -n 'errorActionStyle\|errorValueStyle\|errorMutedStyle' --type go -B2 -A2

Repository: localstack/lstk

Length of output: 41

🏁 Script executed:

rg -n 'errorMutedStyle|errorActionStyle|errorValueStyle' -B3 -A3

Repository: localstack/lstk

Length of output: 1930

🏁 Script executed:

fd 'error_display.go|plain_format.go' --type f

Repository: localstack/lstk

Length of output: 130

🏁 Script executed:

cat -n internal/output/plain_format.go | sed -n '1,50p'

Repository: localstack/lstk

Length of output: 1700

Add test coverage to validate the visual styling differentiation for multiple actions.

The code implements distinct styling: the first action renders with errorActionStyle (color) and errorValueStyle (bold), while subsequent actions use errorMutedStyle (muted grey). The existing test TestErrorDisplay_MultiActionRenders covers multiple actions but only validates content presence via strings.Contains(), not the styling output.

Add a test assertion that validates the rendered output distinguishes the first action visually from subsequent ones to document this intentional design.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@internal/output/plain_format.go` around lines 153 - 164, The test suite lacks
assertions verifying visual styling differences between the first and subsequent
actions; update TestErrorDisplay_MultiActionRenders to assert that the rendered
output contains styling markers (or ANSI sequences) produced by errorActionStyle
and errorValueStyle for the first action and that subsequent actions include the
errorMutedStyle output (e.g., by checking for the specific ANSI color/bold
sequences or unique style markers applied by errorActionStyle/errorValueStyle vs
errorMutedStyle in the output string derived from rendering e.Actions); locate
rendering logic around e.Actions in plain_format.go (errorActionStyle,
errorValueStyle, errorMutedStyle) and enhance the test to explicitly assert the
first action uses the action+value styles while later actions use the muted
style.

return sb.String()
}
Expand Down
8 changes: 4 additions & 4 deletions internal/output/plain_format_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,19 +84,19 @@ func TestFormatEventLine(t *testing.T) {
{
name: "error event title only",
event: ErrorEvent{Title: "Connection failed"},
want: "Error: Connection failed",
want: "✗ Connection failed",
wantOK: true,
},
{
name: "error event with summary",
event: ErrorEvent{Title: "Auth failed", Summary: "Invalid token"},
want: "Error: Auth failed\n Invalid token",
want: "✗ Auth failed\n> Invalid token",
wantOK: true,
},
{
name: "error event with detail",
event: ErrorEvent{Title: "Auth failed", Summary: "Invalid token", Detail: "Token expired at 2024-01-01"},
want: "Error: Auth failed\n Invalid token\n Token expired at 2024-01-01",
want: "✗ Auth failed\n> Invalid token\n Token expired at 2024-01-01",
wantOK: true,
},
{
Expand All @@ -108,7 +108,7 @@ func TestFormatEventLine(t *testing.T) {
{Label: "Start Docker:", Value: "open -a Docker"},
},
},
want: "Error: Docker not running\n Cannot connect to Docker daemon\n ==> Start Docker: open -a Docker",
want: "✗ Docker not running\n> Cannot connect to Docker daemon\n\n==> Start Docker: open -a Docker",
wantOK: true,
},
{
Expand Down
2 changes: 1 addition & 1 deletion internal/output/plain_sink_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func TestPlainSink_EmitsErrorEvent(t *testing.T) {
Actions: []ErrorAction{{Label: "Start Docker:", Value: "open -a Docker"}},
})

expected := "Error: Connection failed\n Cannot connect to Docker\n ==> Start Docker: open -a Docker\n"
expected := "✗ Connection failed\n> Cannot connect to Docker\n\n==> Start Docker: open -a Docker\n"
assert.Equal(t, expected, out.String())
}

Expand Down
Loading