diff --git a/docs/AGENT_SETUP_IMPROVEMENTS.md b/docs/AGENT_SETUP_IMPROVEMENTS.md index 87b3338..6f24557 100644 --- a/docs/AGENT_SETUP_IMPROVEMENTS.md +++ b/docs/AGENT_SETUP_IMPROVEMENTS.md @@ -44,16 +44,16 @@ Replace raw `fmt.Println` calls with a structured, visually clear output format ### 2.1 Create an output formatter -- [ ] Create `internal/agent/output.go` (or `internal/output/output.go` if shared with setup) -- [ ] Define a `Formatter` struct with methods for each output type: +- [x] Create `internal/agent/output.go` (or `internal/output/output.go` if shared with setup) +- [x] Define a `Formatter` struct with methods for each output type: - `PhaseStart(name string)` — e.g. `┌─ bootstrap` - `StepSuccess(name string, duration time.Duration)` — e.g. `│ ✓ Install deps 2.3s` - `StepFailure(name string, duration time.Duration, err string)` — e.g. `│ ✗ Run migrations 0.8s` - `StepSkipped(name string)` — e.g. `│ ○ Skipped (condition not met)` - `PhaseEnd()` - `PhasesSkipped(names []string)` — e.g. `└─ (setup, verify, post skipped)` -- [ ] Support a `--no-color` / `NO_COLOR` env var fallback for CI/non-TTY environments -- [ ] Detect non-TTY output and disable box-drawing characters automatically +- [x] Support a `--no-color` / `NO_COLOR` env var fallback for CI/non-TTY environments +- [x] Detect non-TTY output and disable box-drawing characters automatically ### 2.2 Apply formatter to setup execution diff --git a/internal/output/output.go b/internal/output/output.go new file mode 100644 index 0000000..04635f5 --- /dev/null +++ b/internal/output/output.go @@ -0,0 +1,229 @@ +// Package output provides structured, visually clear formatting for setup +// execution and agent mode output. +package output + +import ( + "fmt" + "io" + "os" + "strings" + "time" + + "golang.org/x/term" +) + +// Formatter writes structured, visually clear output for setup execution +// and agent mode. It supports colored and plain-text modes, automatically +// detecting non-TTY environments. +type Formatter struct { + w io.Writer + color bool + fancy bool // box-drawing characters enabled +} + +// NewFormatter creates a Formatter that writes to w. Color and box-drawing +// characters are enabled by default unless: +// - The NO_COLOR environment variable is set (any value), or +// - w is not a terminal (non-TTY). +// +// Both can be overridden with Option functions. +func NewFormatter(w io.Writer, opts ...Option) *Formatter { + isTTY := isTerminal(w) + noColor := os.Getenv("NO_COLOR") != "" + + f := &Formatter{ + w: w, + color: isTTY && !noColor, + fancy: isTTY, + } + for _, o := range opts { + o(f) + } + return f +} + +// Option configures a Formatter. +type Option func(*Formatter) + +// WithColor forces color output on or off. +func WithColor(enabled bool) Option { + return func(f *Formatter) { + f.color = enabled + } +} + +// WithFancy forces box-drawing characters on or off. +func WithFancy(enabled bool) Option { + return func(f *Formatter) { + f.fancy = enabled + } +} + +// --- Phase lifecycle --- + +// PhaseStart prints the opening line of a phase block. +// +// Fancy: ┌─ bootstrap +// Plain: == bootstrap == +func (f *Formatter) PhaseStart(name string) { + if f.fancy { + fmt.Fprintf(f.w, "%s %s\n", f.dim("┌─"), f.bold(name)) + } else { + fmt.Fprintf(f.w, "== %s ==\n", name) + } +} + +// PhaseEnd prints the closing line of a phase block. +// +// Fancy: └─ +// Plain: (blank line) +func (f *Formatter) PhaseEnd() { + if f.fancy { + fmt.Fprintln(f.w, f.dim("└─")) + } else { + fmt.Fprintln(f.w) + } +} + +// --- Step results --- + +// StepSuccess prints a successful step with its duration. +// +// Fancy: │ ✓ Install deps 2.3s +// Plain: [ok] Install deps 2.3s +func (f *Formatter) StepSuccess(name string, d time.Duration) { + dur := formatDuration(d) + if f.fancy { + fmt.Fprintf(f.w, "%s %s %s %s\n", + f.dim("│"), f.green("✓"), name, f.dim(dur)) + } else { + fmt.Fprintf(f.w, "[ok] %s %s\n", name, dur) + } +} + +// StepFailure prints a failed step with its duration and error message. +// +// Fancy: │ ✗ Run migrations 0.8s +// Fancy: │ error: relation "users" already exists +// Plain: [FAIL] Run migrations 0.8s +// Plain: error: relation "users" already exists +func (f *Formatter) StepFailure(name string, d time.Duration, errMsg string) { + dur := formatDuration(d) + if f.fancy { + fmt.Fprintf(f.w, "%s %s %s %s\n", + f.dim("│"), f.red("✗"), name, f.dim(dur)) + if errMsg != "" { + for _, line := range strings.Split(errMsg, "\n") { + fmt.Fprintf(f.w, "%s %s\n", f.dim("│"), f.red(line)) + } + } + } else { + fmt.Fprintf(f.w, "[FAIL] %s %s\n", name, dur) + if errMsg != "" { + for _, line := range strings.Split(errMsg, "\n") { + fmt.Fprintf(f.w, " %s\n", line) + } + } + } +} + +// StepSkipped prints a skipped step. +// +// Fancy: │ ○ Seed database (skipped) +// Plain: [skip] Seed database +func (f *Formatter) StepSkipped(name string) { + if f.fancy { + fmt.Fprintf(f.w, "%s %s %s %s\n", + f.dim("│"), f.yellow("○"), name, f.dim("(skipped)")) + } else { + fmt.Fprintf(f.w, "[skip] %s\n", name) + } +} + +// PhasesSkipped prints a summary of phases that were never reached +// (e.g. due to an earlier failure). +// +// Fancy: └─ (setup, verify skipped) +// Plain: -- (setup, verify skipped) -- +func (f *Formatter) PhasesSkipped(names []string) { + if len(names) == 0 { + return + } + label := strings.Join(names, ", ") + " skipped" + if f.fancy { + fmt.Fprintf(f.w, "%s (%s)\n", f.dim("└─"), f.dim(label)) + } else { + fmt.Fprintf(f.w, "-- (%s) --\n", label) + } +} + +// --- ANSI helpers --- + +const ( + ansiReset = "\033[0m" + ansiBold = "\033[1m" + ansiDim = "\033[2m" + ansiRed = "\033[31m" + ansiGreen = "\033[32m" + ansiYellow = "\033[33m" +) + +func (f *Formatter) green(s string) string { + if !f.color { + return s + } + return ansiGreen + s + ansiReset +} + +func (f *Formatter) red(s string) string { + if !f.color { + return s + } + return ansiRed + s + ansiReset +} + +func (f *Formatter) yellow(s string) string { + if !f.color { + return s + } + return ansiYellow + s + ansiReset +} + +func (f *Formatter) bold(s string) string { + if !f.color { + return s + } + return ansiBold + s + ansiReset +} + +func (f *Formatter) dim(s string) string { + if !f.color { + return s + } + return ansiDim + s + ansiReset +} + +// --- Utilities --- + +// formatDuration renders a duration as a human-friendly string. +func formatDuration(d time.Duration) string { + const secondsPerMinute = 60 + switch { + case d < time.Second: + return fmt.Sprintf("%dms", d.Milliseconds()) + case d < time.Minute: + return fmt.Sprintf("%.1fs", d.Seconds()) + default: + m := int(d.Minutes()) + s := int(d.Seconds()) % secondsPerMinute + return fmt.Sprintf("%dm%ds", m, s) + } +} + +// isTerminal reports whether w is connected to a terminal. +func isTerminal(w io.Writer) bool { + if f, ok := w.(*os.File); ok { + return term.IsTerminal(int(f.Fd())) + } + return false +} diff --git a/internal/output/output_test.go b/internal/output/output_test.go new file mode 100644 index 0000000..efbfd18 --- /dev/null +++ b/internal/output/output_test.go @@ -0,0 +1,266 @@ +package output + +import ( + "bytes" + "os" + "strings" + "testing" + "time" +) + +func newPlainFormatter(buf *bytes.Buffer) *Formatter { + return NewFormatter(buf, WithColor(false), WithFancy(false)) +} + +func newFancyFormatter(buf *bytes.Buffer) *Formatter { + return NewFormatter(buf, WithColor(false), WithFancy(true)) +} + +func newColorFormatter(buf *bytes.Buffer) *Formatter { + return NewFormatter(buf, WithColor(true), WithFancy(true)) +} + +func TestPhaseStart_Plain(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.PhaseStart("bootstrap") + if got := buf.String(); got != "== bootstrap ==\n" { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestPhaseStart_Fancy(t *testing.T) { + var buf bytes.Buffer + f := newFancyFormatter(&buf) + f.PhaseStart("bootstrap") + got := buf.String() + if !strings.Contains(got, "┌─") || !strings.Contains(got, "bootstrap") { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestPhaseEnd_Plain(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.PhaseEnd() + if got := buf.String(); got != "\n" { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestPhaseEnd_Fancy(t *testing.T) { + var buf bytes.Buffer + f := newFancyFormatter(&buf) + f.PhaseEnd() + if !strings.Contains(buf.String(), "└─") { + t.Fatalf("unexpected output: %q", buf.String()) + } +} + +func TestStepSuccess_Plain(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.StepSuccess("Install deps", 2300*time.Millisecond) + got := buf.String() + if !strings.Contains(got, "[ok]") || !strings.Contains(got, "Install deps") || !strings.Contains(got, "2.3s") { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestStepSuccess_Fancy(t *testing.T) { + var buf bytes.Buffer + f := newFancyFormatter(&buf) + f.StepSuccess("Install deps", 2300*time.Millisecond) + got := buf.String() + if !strings.Contains(got, "✓") || !strings.Contains(got, "Install deps") || !strings.Contains(got, "2.3s") { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestStepFailure_Plain(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.StepFailure("Run migrations", 800*time.Millisecond, "relation already exists") + got := buf.String() + if !strings.Contains(got, "[FAIL]") || !strings.Contains(got, "Run migrations") { + t.Fatalf("unexpected output: %q", got) + } + if !strings.Contains(got, "relation already exists") { + t.Fatalf("expected error message in output: %q", got) + } +} + +func TestStepFailure_Fancy(t *testing.T) { + var buf bytes.Buffer + f := newFancyFormatter(&buf) + f.StepFailure("Run migrations", 800*time.Millisecond, "relation already exists") + got := buf.String() + if !strings.Contains(got, "✗") || !strings.Contains(got, "Run migrations") { + t.Fatalf("unexpected output: %q", got) + } + if !strings.Contains(got, "relation already exists") { + t.Fatalf("expected error message in output: %q", got) + } +} + +func TestStepFailure_MultilineError(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.StepFailure("Build", 100*time.Millisecond, "line 1\nline 2") + got := buf.String() + if strings.Count(got, "line") != 2 { + t.Fatalf("expected both error lines in output: %q", got) + } +} + +func TestStepFailure_EmptyError(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.StepFailure("Build", 100*time.Millisecond, "") + got := buf.String() + lines := strings.Split(strings.TrimRight(got, "\n"), "\n") + if len(lines) != 1 { + t.Fatalf("expected single line for empty error, got %d: %q", len(lines), got) + } +} + +func TestStepSkipped_Plain(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.StepSkipped("Seed database") + got := buf.String() + if !strings.Contains(got, "[skip]") || !strings.Contains(got, "Seed database") { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestStepSkipped_Fancy(t *testing.T) { + var buf bytes.Buffer + f := newFancyFormatter(&buf) + f.StepSkipped("Seed database") + got := buf.String() + if !strings.Contains(got, "○") || !strings.Contains(got, "Seed database") { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestPhasesSkipped_Plain(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.PhasesSkipped([]string{"setup", "verify"}) + got := buf.String() + if !strings.Contains(got, "setup, verify skipped") { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestPhasesSkipped_Fancy(t *testing.T) { + var buf bytes.Buffer + f := newFancyFormatter(&buf) + f.PhasesSkipped([]string{"setup", "verify"}) + got := buf.String() + if !strings.Contains(got, "└─") || !strings.Contains(got, "setup, verify skipped") { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestPhasesSkipped_Empty(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.PhasesSkipped(nil) + if buf.Len() != 0 { + t.Fatalf("expected no output for empty slice, got: %q", buf.String()) + } +} + +func TestFormatDuration(t *testing.T) { + cases := []struct { + d time.Duration + expect string + }{ + {50 * time.Millisecond, "50ms"}, + {999 * time.Millisecond, "999ms"}, + {1500 * time.Millisecond, "1.5s"}, + {59900 * time.Millisecond, "59.9s"}, + {90 * time.Second, "1m30s"}, + {125 * time.Second, "2m5s"}, + } + for _, tc := range cases { + t.Run(tc.expect, func(t *testing.T) { + got := formatDuration(tc.d) + if got != tc.expect { + t.Fatalf("formatDuration(%v) = %q, want %q", tc.d, got, tc.expect) + } + }) + } +} + +func TestColorOutput(t *testing.T) { + var buf bytes.Buffer + f := newColorFormatter(&buf) + f.StepSuccess("test", time.Second) + got := buf.String() + if !strings.Contains(got, "\033[") { + t.Fatalf("expected ANSI codes in color output: %q", got) + } +} + +func TestNoColorOutput(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.StepSuccess("test", time.Second) + got := buf.String() + if strings.Contains(got, "\033[") { + t.Fatalf("unexpected ANSI codes in plain output: %q", got) + } +} + +func TestNewFormatter_NonTTY(t *testing.T) { + var buf bytes.Buffer + f := NewFormatter(&buf) // bytes.Buffer is not a TTY + if f.color { + t.Fatal("expected color=false for non-TTY writer") + } + if f.fancy { + t.Fatal("expected fancy=false for non-TTY writer") + } +} + +func TestNewFormatter_NO_COLOR_Env(t *testing.T) { + t.Setenv("NO_COLOR", "1") + // Even with a real file (which could be a TTY), NO_COLOR disables color. + f := NewFormatter(os.Stdout) + if f.color { + t.Fatal("expected color=false when NO_COLOR is set") + } +} + +func TestFullPhaseSequence(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + + f.PhaseStart("bootstrap") + f.StepSuccess("Install deps", 2*time.Second) + f.StepSuccess("Check runtime", 150*time.Millisecond) + f.PhaseEnd() + + f.PhaseStart("provision") + f.StepFailure("Run migrations", 800*time.Millisecond, "connection refused") + f.PhaseEnd() + + f.PhasesSkipped([]string{"setup", "verify"}) + + got := buf.String() + if !strings.Contains(got, "== bootstrap ==") { + t.Fatal("missing bootstrap phase") + } + if !strings.Contains(got, "[ok] Install deps") { + t.Fatal("missing success step") + } + if !strings.Contains(got, "[FAIL] Run migrations") { + t.Fatal("missing failure step") + } + if !strings.Contains(got, "setup, verify skipped") { + t.Fatal("missing skipped phases") + } +}