diff --git a/cmd/setup.go b/cmd/setup.go index 1374833..0efe8e2 100644 --- a/cmd/setup.go +++ b/cmd/setup.go @@ -13,6 +13,7 @@ import ( "github.com/InitiatDev/initiat-cli/internal/agent" initiatconfig "github.com/InitiatDev/initiat-cli/internal/config" + "github.com/InitiatDev/initiat-cli/internal/output" "github.com/InitiatDev/initiat-cli/internal/prompt" "github.com/InitiatDev/initiat-cli/internal/scaffold" "github.com/InitiatDev/initiat-cli/internal/setup" @@ -217,10 +218,15 @@ func runAgentIterative( orchestrator.SetDebugWriter(os.Stdout) orchestrator.SetPromptInput(prompt.PromptInput) + fmtr := output.NewFormatter(os.Stdout) + fmtr.AgentHeader(execErr.FailedCommand.Phase, execErr.FailedCommand.StepName) + var lastDecision *agent.Decision var lastApply *agent.ApplyResult for round := 1; round <= 10; round++ { + fmtr.RoundSeparator(round) + next, done, err := runAgentRound( ctx, runner, @@ -232,7 +238,7 @@ func runAgentIterative( setupConfig, execErr, lastApply, - round, + fmtr, ) if err != nil { return err @@ -275,7 +281,7 @@ func runAgentRound( setupConfig *setup.SetupConfig, execErr *setup.SetupExecutionError, lastApply *agent.ApplyResult, - round int, + fmtr *output.Formatter, ) (*agentRoundResult, bool, error) { decision, applyRes, updatedSetup, changedSomething, err := diagnoseApplyReload( ctx, @@ -285,7 +291,7 @@ func runAgentRound( setupConfig, execErr, lastApply, - round, + fmtr, ) if err != nil { return nil, false, err @@ -320,7 +326,7 @@ func diagnoseApplyReload( setupConfig *setup.SetupConfig, execErr *setup.SetupExecutionError, lastApply *agent.ApplyResult, - round int, + fmtr *output.Formatter, ) (*agent.Decision, *agent.ApplyResult, *setup.SetupConfig, bool, error) { snapshotJSON, snapshot, snapErr := agent.BuildProjectSnapshot(wd) if snapErr != nil { @@ -333,10 +339,10 @@ func diagnoseApplyReload( return nil, nil, nil, false, err } - fmt.Println() - fmt.Printf("Agent round %d diagnosis:\n", round) - fmt.Println(decision.Explanation) - fmt.Println() + fmtr.Explanation(decision.Explanation) + + actionItems := decisionToActionItems(decision) + fmtr.ActionList(actionItems) beforeSetupFP, _ := fileFingerprint(setupPath) applyRes, err := orchestrator.ApplyWithResults(ctx, decision) @@ -344,6 +350,21 @@ func diagnoseApplyReload( return nil, nil, nil, false, err } + for i, r := range applyRes.Results { + var item output.ActionItem + if i < len(actionItems) { + item = actionItems[i] + } else { + item = output.ActionItem{Summary: r.Summary, Danger: "safe"} + } + detail := "" + if !r.OK && r.Error != "" { + detail = r.Error + } + fmtr.ActionResult(i, item, r.OK, detail) + } + fmt.Fprintln(os.Stdout) + afterSetupFP, _ := fileFingerprint(setupPath) setupConfig, err = reloadSetupIfChanged(setupPath, setupConfig, beforeSetupFP, afterSetupFP) if err != nil { @@ -356,7 +377,6 @@ func diagnoseApplyReload( } if !decisionLikelyChangedSomething(decision, applyRes) { - fmt.Println() fmt.Println("No changes applied that warrant re-running setup. Continuing agent mode...") return decision, applyRes, setupConfig, false, nil } @@ -364,6 +384,44 @@ func diagnoseApplyReload( return decision, applyRes, setupConfig, true, nil } +func decisionToActionItems(decision *agent.Decision) []output.ActionItem { + items := make([]output.ActionItem, len(decision.Actions)) + for i, a := range decision.Actions { + items[i] = output.ActionItem{ + Summary: a.Summary, + Danger: string(a.Danger), + Type: string(a.Type), + Detail: actionDetail(a), + } + } + return items +} + +func actionDetail(a agent.ProposedAction) string { + switch a.Type { + case agent.ActionRunCommand: + return a.Command + case agent.ActionEditFiles: + paths := make([]string, len(a.Edits)) + for i, e := range a.Edits { + paths[i] = e.Path + } + return strings.Join(paths, ", ") + case agent.ActionListFiles: + if a.Path != "" { + return a.Path + } + return "." + case agent.ActionReadFiles: + return strings.Join(a.Paths, ", ") + case agent.ActionAskUser: + return a.Prompt + case agent.ActionStop: + return "" + } + return "" +} + func rerunSetupAndMaybeContinue( ctx context.Context, runner *setup.SetupRunner, diff --git a/docs/AGENT_SETUP_IMPROVEMENTS.md b/docs/AGENT_SETUP_IMPROVEMENTS.md index 6f24557..44672f1 100644 --- a/docs/AGENT_SETUP_IMPROVEMENTS.md +++ b/docs/AGENT_SETUP_IMPROVEMENTS.md @@ -57,28 +57,28 @@ Replace raw `fmt.Println` calls with a structured, visually clear output format ### 2.2 Apply formatter to setup execution -- [ ] Update `Executor.Execute` in `internal/setup/executor.go` to use the formatter instead of raw `fmt.Printf` -- [ ] Show per-command durations on the same line as the step name -- [ ] On failure, show the failing command's stderr indented beneath the step -- [ ] Show skipped phases (phases that never ran due to early failure) at the bottom +- [x] Update `Executor.Execute` in `internal/setup/executor.go` to use the formatter instead of raw `fmt.Printf` +- [x] Show per-command durations on the same line as the step name +- [x] On failure, show the failing command's stderr indented beneath the step +- [x] Show skipped phases (phases that never ran due to early failure) at the bottom ### 2.3 Apply formatter to agent mode -- [ ] Add agent-mode header output: +- [x] Add agent-mode header output: ``` ╔══ Agent Mode ═══════════════════════════════════════╗ ║ Diagnosing failure in provision → "Run migrations" ║ ╚═════════════════════════════════════════════════════╝ ``` -- [ ] Add round separator: `── Round 1 ──────────────────` -- [ ] Format the diagnosis explanation in a visually distinct block (indented or quoted) -- [ ] Format proposed actions as a numbered list with danger-level badges: +- [x] Add round separator: `── Round 1 ──────────────────` +- [x] Format the diagnosis explanation in a visually distinct block (indented or quoted) +- [x] Format proposed actions as a numbered list with danger-level badges: ``` Proposed actions: 1. [safe] Read db/migrate/ directory listing 2. [caution] Run: rails db:migrate:status ``` -- [ ] Show action results inline after execution: +- [x] Show action results inline after execution: ``` 1. [safe] Read db/migrate/ directory listing ✓ 2. [caution] Run: rails db:migrate:status ✓ (exit 0) diff --git a/internal/output/output.go b/internal/output/output.go index 04635f5..8ba320a 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -157,6 +157,191 @@ func (f *Formatter) PhasesSkipped(names []string) { } } +// --- Agent mode --- + +// AgentHeader prints the agent-mode banner with the failure context. +// +// Fancy: +// +// ╔══ Agent Mode ═══════════════════════════════════════╗ +// ║ Diagnosing failure in provision → "Run migrations" ║ +// ╚═════════════════════════════════════════════════════╝ +// +// Plain: +// +// === Agent Mode === +// Diagnosing failure in -> "" +func (f *Formatter) AgentHeader(phase, step string) { + desc := fmt.Sprintf("Diagnosing failure in %s", phase) + if step != "" { + desc += fmt.Sprintf(" → %q", step) + } + + if !f.fancy { + fmt.Fprintf(f.w, "=== Agent Mode ===\n%s\n\n", desc) + return + } + + const ( + minWidth = 50 + sidePadding = 2 // 1 space padding each side + rightPadding = 1 // space before closing ║ + ) + contentWidth := len(desc) + sidePadding + if contentWidth < minWidth { + contentWidth = minWidth + } + + top := "╔══ Agent Mode " + strings.Repeat("═", contentWidth-len("══ Agent Mode ")) + "╗" + padded := "║ " + desc + strings.Repeat(" ", contentWidth-len(desc)-rightPadding) + "║" + bottom := "╚" + strings.Repeat("═", contentWidth) + "╝" + + fmt.Fprintln(f.w, f.bold(top)) + fmt.Fprintln(f.w, f.bold(padded)) + fmt.Fprintln(f.w, f.bold(bottom)) + fmt.Fprintln(f.w) +} + +// RoundSeparator prints a visual separator between agent rounds. +// +// Fancy: ── Round 1 ────────────────── +// Plain: -- Round 1 -- +func (f *Formatter) RoundSeparator(round int) { + label := fmt.Sprintf("Round %d", round) + const roundLineWidth = 40 + if f.fancy { + line := "── " + label + " " + strings.Repeat("─", roundLineWidth-len(label)) + fmt.Fprintln(f.w, f.dim(line)) + } else { + fmt.Fprintf(f.w, "-- %s --\n", label) + } +} + +// Explanation prints the agent's diagnosis explanation in a visually +// distinct block (indented/quoted). +// +// Fancy: │ The migration failed because … +// Plain: > The migration failed because … +func (f *Formatter) Explanation(text string) { + if strings.TrimSpace(text) == "" { + return + } + fmt.Fprintln(f.w) + for _, line := range strings.Split(text, "\n") { + if f.fancy { + fmt.Fprintf(f.w, " %s %s\n", f.dim("│"), line) + } else { + fmt.Fprintf(f.w, " > %s\n", line) + } + } + fmt.Fprintln(f.w) +} + +// ActionItem describes a single proposed action for display purposes. +type ActionItem struct { + Summary string + Danger string // "safe", "caution", or "dangerous" + Type string // action type like "run_command", "edit_files", etc. + Detail string // command string, file path, prompt, etc. +} + +// ActionList prints proposed actions as a numbered list with danger-level badges. +// +// Fancy: +// +// Proposed actions: +// 1. [safe] Read db/migrate/ directory listing +// 2. [caution] Run: rails db:migrate:status +// +// Plain: +// +// Proposed actions: +// 1. [safe] Read db/migrate/ directory listing +// 2. [caution] Run: rails db:migrate:status +func (f *Formatter) ActionList(actions []ActionItem) { + if len(actions) == 0 { + return + } + fmt.Fprintln(f.w, "Proposed actions:") + for i, a := range actions { + badge := f.dangerBadge(a.Danger) + fmt.Fprintf(f.w, " %d. %s %s\n", i+1, badge, a.Summary) + } + fmt.Fprintln(f.w) +} + +// ActionResult prints the result of an executed action inline. +// +// Fancy: +// +// 1. [safe] Read db/migrate/ directory listing ✓ +// 2. [caution] Run: rails db:migrate:status ✓ (exit 0) +// 3. [caution] Run: rails db:drop ✗ (permission denied) +// +// Plain: +// +// 1. [safe] Read db/migrate/ directory listing [ok] +// 2. [caution] Run: rails db:migrate:status [ok] (exit 0) +// 3. [caution] Run: rails db:drop [FAIL] (permission denied) +func (f *Formatter) ActionResult(index int, action ActionItem, ok bool, detail string) { + badge := f.dangerBadge(action.Danger) + suffix := "" + if detail != "" { + suffix = " (" + detail + ")" + } + if ok { + if f.fancy { + fmt.Fprintf(f.w, " %d. %s %s %s%s\n", + index+1, badge, action.Summary, f.green("✓"), f.dim(suffix)) + } else { + fmt.Fprintf(f.w, " %d. %s %s [ok]%s\n", + index+1, badge, action.Summary, suffix) + } + } else { + if f.fancy { + fmt.Fprintf(f.w, " %d. %s %s %s%s\n", + index+1, badge, action.Summary, f.red("✗"), f.red(suffix)) + } else { + fmt.Fprintf(f.w, " %d. %s %s [FAIL]%s\n", + index+1, badge, action.Summary, suffix) + } + } +} + +const ( + dangerBadgeWidth = 11 // len("[dangerous]") +) + +func (f *Formatter) dangerBadge(danger string) string { + var raw string + switch danger { + case "safe": + raw = "[safe]" + case "caution": + raw = "[caution]" + case "dangerous": + raw = "[dangerous]" + default: + raw = "[" + danger + "]" + } + + if !f.fancy { + return raw + } + + padded := raw + strings.Repeat(" ", dangerBadgeWidth-len(raw)) + switch danger { + case "safe": + return f.green(padded) + case "caution": + return f.yellow(padded) + case "dangerous": + return f.red(padded) + default: + return padded + } +} + // --- ANSI helpers --- const ( diff --git a/internal/output/output_test.go b/internal/output/output_test.go index efbfd18..33c18c4 100644 --- a/internal/output/output_test.go +++ b/internal/output/output_test.go @@ -235,6 +235,208 @@ func TestNewFormatter_NO_COLOR_Env(t *testing.T) { } } +// --- Agent mode tests --- + +func TestAgentHeader_Plain(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.AgentHeader("provision", "Run migrations") + got := buf.String() + if !strings.Contains(got, "=== Agent Mode ===") { + t.Fatalf("missing header: %q", got) + } + if !strings.Contains(got, "provision") || !strings.Contains(got, "Run migrations") { + t.Fatalf("missing phase/step info: %q", got) + } +} + +func TestAgentHeader_Fancy(t *testing.T) { + var buf bytes.Buffer + f := newFancyFormatter(&buf) + f.AgentHeader("provision", "Run migrations") + got := buf.String() + if !strings.Contains(got, "╔") || !strings.Contains(got, "Agent Mode") { + t.Fatalf("missing fancy header: %q", got) + } + if !strings.Contains(got, "provision") || !strings.Contains(got, "Run migrations") { + t.Fatalf("missing phase/step info: %q", got) + } +} + +func TestAgentHeader_NoStep(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.AgentHeader("bootstrap", "") + got := buf.String() + if !strings.Contains(got, "Diagnosing failure in bootstrap") { + t.Fatalf("unexpected output: %q", got) + } + if strings.Contains(got, "→") { + t.Fatalf("should not contain arrow when step is empty: %q", got) + } +} + +func TestRoundSeparator_Plain(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.RoundSeparator(3) + got := buf.String() + if got != "-- Round 3 --\n" { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestRoundSeparator_Fancy(t *testing.T) { + var buf bytes.Buffer + f := newFancyFormatter(&buf) + f.RoundSeparator(1) + got := buf.String() + if !strings.Contains(got, "──") || !strings.Contains(got, "Round 1") { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestExplanation_Plain(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.Explanation("The migration failed.\nCheck the DB connection.") + got := buf.String() + if !strings.Contains(got, "> The migration failed.") { + t.Fatalf("missing first line: %q", got) + } + if !strings.Contains(got, "> Check the DB connection.") { + t.Fatalf("missing second line: %q", got) + } +} + +func TestExplanation_Fancy(t *testing.T) { + var buf bytes.Buffer + f := newFancyFormatter(&buf) + f.Explanation("The migration failed.") + got := buf.String() + if !strings.Contains(got, "│") || !strings.Contains(got, "The migration failed.") { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestExplanation_Empty(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.Explanation(" ") + if buf.Len() != 0 { + t.Fatalf("expected no output for blank explanation, got: %q", buf.String()) + } +} + +func TestActionList_Plain(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.ActionList([]ActionItem{ + {Summary: "Read db/migrate/ directory listing", Danger: "safe"}, + {Summary: "Run: rails db:migrate:status", Danger: "caution"}, + }) + got := buf.String() + if !strings.Contains(got, "Proposed actions:") { + t.Fatalf("missing header: %q", got) + } + if !strings.Contains(got, "1. [safe] Read db/migrate/") { + t.Fatalf("missing first action: %q", got) + } + if !strings.Contains(got, "2. [caution] Run: rails") { + t.Fatalf("missing second action: %q", got) + } +} + +func TestActionList_Fancy(t *testing.T) { + var buf bytes.Buffer + f := newFancyFormatter(&buf) + f.ActionList([]ActionItem{ + {Summary: "Read db/migrate/ directory listing", Danger: "safe"}, + {Summary: "Run: rails db:migrate:status", Danger: "dangerous"}, + }) + got := buf.String() + if !strings.Contains(got, "Proposed actions:") { + t.Fatalf("missing header: %q", got) + } + if !strings.Contains(got, "Read db/migrate/") { + t.Fatalf("missing first action: %q", got) + } +} + +func TestActionList_Empty(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.ActionList(nil) + if buf.Len() != 0 { + t.Fatalf("expected no output for empty actions, got: %q", buf.String()) + } +} + +func TestActionResult_Success_Plain(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.ActionResult(0, ActionItem{Summary: "Read directory", Danger: "safe"}, true, "") + got := buf.String() + if !strings.Contains(got, "[ok]") || !strings.Contains(got, "Read directory") { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestActionResult_Success_WithDetail(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.ActionResult(1, ActionItem{Summary: "Run: migrate", Danger: "caution"}, true, "exit 0") + got := buf.String() + if !strings.Contains(got, "[ok]") || !strings.Contains(got, "(exit 0)") { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestActionResult_Failure_Plain(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + f.ActionResult(2, ActionItem{Summary: "Run: db:drop", Danger: "dangerous"}, false, "permission denied") + got := buf.String() + if !strings.Contains(got, "[FAIL]") || !strings.Contains(got, "(permission denied)") { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestActionResult_Fancy_Success(t *testing.T) { + var buf bytes.Buffer + f := newFancyFormatter(&buf) + f.ActionResult(0, ActionItem{Summary: "Read directory", Danger: "safe"}, true, "") + got := buf.String() + if !strings.Contains(got, "✓") || !strings.Contains(got, "Read directory") { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestActionResult_Fancy_Failure(t *testing.T) { + var buf bytes.Buffer + f := newFancyFormatter(&buf) + f.ActionResult(0, ActionItem{Summary: "Run: migrate", Danger: "caution"}, false, "exit 1") + got := buf.String() + if !strings.Contains(got, "✗") || !strings.Contains(got, "(exit 1)") { + t.Fatalf("unexpected output: %q", got) + } +} + +func TestDangerBadge_Alignment(t *testing.T) { + var buf bytes.Buffer + f := newPlainFormatter(&buf) + // Plain mode should not pad + f.ActionList([]ActionItem{ + {Summary: "A", Danger: "safe"}, + {Summary: "B", Danger: "caution"}, + {Summary: "C", Danger: "dangerous"}, + }) + got := buf.String() + if !strings.Contains(got, "[safe]") || !strings.Contains(got, "[caution]") || !strings.Contains(got, "[dangerous]") { + t.Fatalf("missing badges: %q", got) + } +} + func TestFullPhaseSequence(t *testing.T) { var buf bytes.Buffer f := newPlainFormatter(&buf) diff --git a/internal/setup/executor.go b/internal/setup/executor.go index aa5d638..791f09c 100644 --- a/internal/setup/executor.go +++ b/internal/setup/executor.go @@ -3,19 +3,24 @@ package setup import ( "context" "fmt" + "os" "strings" "time" + + "github.com/InitiatDev/initiat-cli/internal/output" ) type Executor struct { secrets map[string]string commandExecutor CommandExecutor + formatter *output.Formatter } func NewExecutor(secrets map[string]string) *Executor { return &Executor{ secrets: secrets, commandExecutor: NewRealCommandExecutor(), + formatter: output.NewFormatter(os.Stdout), } } @@ -23,6 +28,17 @@ func NewExecutorWithCommandExecutor(secrets map[string]string, executor CommandE return &Executor{ secrets: secrets, commandExecutor: executor, + formatter: output.NewFormatter(os.Stdout), + } +} + +func NewExecutorWithFormatter( + secrets map[string]string, executor CommandExecutor, formatter *output.Formatter, +) *Executor { + return &Executor{ + secrets: secrets, + commandExecutor: executor, + formatter: formatter, } } @@ -33,24 +49,44 @@ func (e *Executor) Execute(plan *ExecutionPlan) error { Commands: []CommandExecutionRecord{}, } - fmt.Printf("🚀 Executing setup script: %d phases, %d steps, %d commands\n\n", - len(plan.Summary.Phases), plan.Summary.TotalSteps, plan.Summary.TotalCommands) + f := e.formatter + currentPhase := "" + completedPhases := map[string]bool{} for _, cmd := range plan.Commands { - fmt.Printf("[%s] %s", cmd.Phase, cmd.StepName) - if cmd.Description != "" { - fmt.Printf(": %s", cmd.Description) + if cmd.Phase != currentPhase { + if currentPhase != "" { + f.PhaseEnd() + } + currentPhase = cmd.Phase + f.PhaseStart(currentPhase) } - fmt.Println() record, err := e.executeCommandWithReport(cmd) report.Commands = append(report.Commands, record) + duration := lastAttemptDuration(record) + stepLabel := cmd.StepName + if stepLabel == "" { + stepLabel = cmd.Description + } + if err != nil { + stderr := lastAttemptStderr(record) + f.StepFailure(stepLabel, duration, stderr) + if cmd.ContinueOnError { - fmt.Printf("⚠️ Command failed but continuing: %v\n", err) continue } + + f.PhaseEnd() + completedPhases[currentPhase] = true + + skipped := skippedPhaseNames(plan.Summary.Phases, completedPhases) + if len(skipped) > 0 { + f.PhasesSkipped(skipped) + } + report.FinishedAt = time.Now() return &SetupExecutionError{ Report: report, @@ -58,14 +94,43 @@ func (e *Executor) Execute(plan *ExecutionPlan) error { Err: fmt.Errorf("command failed: %w", err), } } + + f.StepSuccess(stepLabel, duration) + completedPhases[cmd.Phase] = true + } + + if currentPhase != "" { + f.PhaseEnd() } - fmt.Println() - fmt.Println("✅ Setup script completed successfully!") report.FinishedAt = time.Now() return nil } +func lastAttemptDuration(record CommandExecutionRecord) time.Duration { + if len(record.Attempts) == 0 { + return 0 + } + return record.Attempts[len(record.Attempts)-1].Duration +} + +func lastAttemptStderr(record CommandExecutionRecord) string { + if len(record.Attempts) == 0 { + return "" + } + return record.Attempts[len(record.Attempts)-1].Stderr +} + +func skippedPhaseNames(phases []PhaseSummary, completed map[string]bool) []string { + var skipped []string + for _, p := range phases { + if !completed[p.Name] { + skipped = append(skipped, p.Name) + } + } + return skipped +} + func (e *Executor) executeCommandWithReport(cmd ExecutableCommand) (CommandExecutionRecord, error) { record := CommandExecutionRecord{ Phase: cmd.Phase, @@ -89,7 +154,6 @@ func (e *Executor) executeCommandWithReport(cmd ExecutableCommand) (CommandExecu for attempt := 1; attempt <= attempts; attempt++ { if attempt > 1 { - fmt.Printf(" ↻ Retry attempt %d/%d...\n", attempt, attempts) if cmd.Retries != nil && cmd.Retries.Backoff > 0 { time.Sleep(cmd.Retries.Backoff) } @@ -99,17 +163,11 @@ func (e *Executor) executeCommandWithReport(cmd ExecutableCommand) (CommandExecu record.Attempts = append(record.Attempts, e.toAttemptRecord(attempt, res, err)) if err == nil { - if attempt > 1 { - fmt.Printf(" ✅ Command succeeded on retry\n") - } record.Success = true return record, nil } lastErr = err - if attempt < attempts { - fmt.Printf(" ⚠️ Command failed, will retry: %v\n", err) - } } record.Success = false diff --git a/internal/setup/executor_test.go b/internal/setup/executor_test.go index 8551335..95eaab7 100644 --- a/internal/setup/executor_test.go +++ b/internal/setup/executor_test.go @@ -1,13 +1,21 @@ package setup import ( + "bytes" "errors" + "strings" "testing" "time" - "github.com/InitiatDev/initiat-cli/internal/testutil" + "github.com/InitiatDev/initiat-cli/internal/output" ) +func newTestExecutor(secrets map[string]string, mock *mockCommandExecutor) (*Executor, *bytes.Buffer) { + var buf bytes.Buffer + f := output.NewFormatter(&buf, output.WithColor(false), output.WithFancy(false)) + return NewExecutorWithFormatter(secrets, mock, f), &buf +} + func TestExecutor_Execute(t *testing.T) { tests := []struct { name string @@ -41,14 +49,12 @@ func TestExecutor_Execute(t *testing.T) { secrets: nil, wantError: false, contains: []string{ - "Executing setup script", - "setup", - "test", - "completed successfully", + "== setup ==", + "[ok] test", }, }, { - name: "multiple commands", + name: "multiple commands across phases", plan: &ExecutionPlan{ Commands: []ExecutableCommand{ { @@ -78,10 +84,10 @@ func TestExecutor_Execute(t *testing.T) { secrets: nil, wantError: false, contains: []string{ - "bootstrap", - "setup", - "step1", - "step2", + "== bootstrap ==", + "[ok] step1", + "== setup ==", + "[ok] step2", }, }, { @@ -105,18 +111,15 @@ func TestExecutor_Execute(t *testing.T) { secrets: map[string]string{"API_KEY": "secret123"}, wantError: false, contains: []string{ - "completed successfully", + "[ok] test", }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - capture := testutil.CaptureStdout() - defer capture.Restore() - - mockExecutor := newMockCommandExecutor() - executor := NewExecutorWithCommandExecutor(tt.secrets, mockExecutor) + mock := newMockCommandExecutor() + executor, buf := newTestExecutor(tt.secrets, mock) err := executor.Execute(tt.plan) if (err != nil) != tt.wantError { @@ -124,12 +127,17 @@ func TestExecutor_Execute(t *testing.T) { return } + out := buf.String() for _, text := range tt.contains { - capture.AssertContains(t, text) + if !strings.Contains(out, text) { + t.Errorf("Expected output to contain %q, got:\n%s", text, out) + } } for _, text := range tt.notContains { - capture.AssertNotContains(t, text) + if strings.Contains(out, text) { + t.Errorf("Expected output NOT to contain %q, got:\n%s", text, out) + } } }) } @@ -166,22 +174,25 @@ func TestExecutor_Execute_ContinueOnError(t *testing.T) { }, } - capture := testutil.CaptureStdout() - defer capture.Restore() - - mockExecutor := newMockCommandExecutor() - mockExecutor.SetError("false", errors.New("command failed")) - executor := NewExecutorWithCommandExecutor(nil, mockExecutor) + mock := newMockCommandExecutor() + mock.SetError("false", errors.New("command failed")) + executor, buf := newTestExecutor(nil, mock) err := executor.Execute(plan) if err != nil { t.Errorf("Execute() should not fail with continue_on_error, got error: %v", err) } - capture.AssertContains(t, "success") - capture.AssertContains(t, "Command failed but continuing") - capture.AssertContains(t, "final") - capture.AssertContains(t, "completed successfully") + out := buf.String() + if !strings.Contains(out, "[ok] success") { + t.Errorf("Expected success step, got:\n%s", out) + } + if !strings.Contains(out, "[FAIL] fail") { + t.Errorf("Expected failure step, got:\n%s", out) + } + if !strings.Contains(out, "[ok] final") { + t.Errorf("Expected final step, got:\n%s", out) + } } func TestExecutor_Execute_Retry(t *testing.T) { @@ -202,19 +213,91 @@ func TestExecutor_Execute_Retry(t *testing.T) { }, } - capture := testutil.CaptureStdout() - defer capture.Restore() - - mockExecutor := newMockCommandExecutor() - mockExecutor.SetError("false", errors.New("command failed")) - executor := NewExecutorWithCommandExecutor(nil, mockExecutor) + mock := newMockCommandExecutor() + executor, buf := newTestExecutor(nil, mock) err := executor.Execute(plan) if err != nil { t.Errorf("Execute() error = %v", err) } - capture.AssertContains(t, "completed successfully") + out := buf.String() + if !strings.Contains(out, "[ok] test") { + t.Errorf("Expected success output, got:\n%s", out) + } +} + +func TestExecutor_Execute_FailureShowsStderr(t *testing.T) { + plan := &ExecutionPlan{ + Commands: []ExecutableCommand{ + { + Phase: "provision", + StepName: "Run migrations", + Command: "migrate", + Args: []string{"up"}, + }, + }, + Summary: ExecutionSummary{ + TotalSteps: 1, + TotalCommands: 1, + Phases: []PhaseSummary{ + {Name: "provision", StepCount: 1, CommandCount: 1}, + {Name: "setup", StepCount: 1, CommandCount: 1}, + }, + }, + } + + mock := newMockCommandExecutor() + mock.SetError("migrate", errors.New("migration failed")) + mock.SetResult("migrate", &CommandResult{ + ExitCode: 1, + Stderr: "relation \"users\" already exists", + }) + executor, buf := newTestExecutor(nil, mock) + err := executor.Execute(plan) + + if err == nil { + t.Fatal("Expected error, got nil") + } + + out := buf.String() + if !strings.Contains(out, "[FAIL] Run migrations") { + t.Errorf("Expected failure step, got:\n%s", out) + } + if !strings.Contains(out, "relation \"users\" already exists") { + t.Errorf("Expected stderr in output, got:\n%s", out) + } +} + +func TestExecutor_Execute_SkippedPhases(t *testing.T) { + plan := &ExecutionPlan{ + Commands: []ExecutableCommand{ + { + Phase: "bootstrap", + StepName: "install", + Command: "fail-cmd", + }, + }, + Summary: ExecutionSummary{ + TotalSteps: 3, + TotalCommands: 3, + Phases: []PhaseSummary{ + {Name: "bootstrap", StepCount: 1, CommandCount: 1}, + {Name: "provision", StepCount: 1, CommandCount: 1}, + {Name: "setup", StepCount: 1, CommandCount: 1}, + }, + }, + } + + mock := newMockCommandExecutor() + mock.SetError("fail-cmd", errors.New("failed")) + executor, buf := newTestExecutor(nil, mock) + _ = executor.Execute(plan) + + out := buf.String() + if !strings.Contains(out, "provision") || !strings.Contains(out, "setup") || !strings.Contains(out, "skipped") { + t.Errorf("Expected skipped phases in output, got:\n%s", out) + } } func TestExecutor_shouldRedact(t *testing.T) { @@ -267,8 +350,8 @@ func TestExecutor_shouldRedact(t *testing.T) { } func TestExecutor_CommandExecution(t *testing.T) { - mockExecutor := newMockCommandExecutor() - executor := NewExecutorWithCommandExecutor(nil, mockExecutor) + mock := newMockCommandExecutor() + executor, _ := newTestExecutor(nil, mock) plan := &ExecutionPlan{ Commands: []ExecutableCommand{ @@ -293,7 +376,7 @@ func TestExecutor_CommandExecution(t *testing.T) { t.Errorf("Execute() error = %v", err) } - executed := mockExecutor.GetExecuted() + executed := mock.GetExecuted() if len(executed) != 1 { t.Errorf("Expected 1 command execution, got %d", len(executed)) } @@ -313,18 +396,48 @@ func TestExecutor_CommandExecution(t *testing.T) { } } -func containsString(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || len(substr) == 0 || - (len(s) > 0 && len(substr) > 0 && - (s[:len(substr)] == substr || s[len(s)-len(substr):] == substr || - indexOfString(s, substr) >= 0))) -} +func TestExecutor_Execute_PhaseTransitions(t *testing.T) { + plan := &ExecutionPlan{ + Commands: []ExecutableCommand{ + {Phase: "bootstrap", StepName: "install deps", Command: "echo"}, + {Phase: "bootstrap", StepName: "check versions", Command: "echo"}, + {Phase: "setup", StepName: "build", Command: "echo"}, + }, + Summary: ExecutionSummary{ + TotalSteps: 3, + TotalCommands: 3, + Phases: []PhaseSummary{ + {Name: "bootstrap", StepCount: 2, CommandCount: 2}, + {Name: "setup", StepCount: 1, CommandCount: 1}, + }, + }, + } -func indexOfString(s, substr string) int { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return i - } + mock := newMockCommandExecutor() + executor, buf := newTestExecutor(nil, mock) + err := executor.Execute(plan) + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + + out := buf.String() + // Verify phase ordering: bootstrap appears before setup + bootstrapIdx := strings.Index(out, "== bootstrap ==") + setupIdx := strings.Index(out, "== setup ==") + if bootstrapIdx < 0 || setupIdx < 0 { + t.Fatalf("Expected both phase headers, got:\n%s", out) + } + if bootstrapIdx >= setupIdx { + t.Errorf("Expected bootstrap before setup, got:\n%s", out) + } + + // Both steps in bootstrap phase appear between bootstrap and setup headers + installIdx := strings.Index(out, "install deps") + checkIdx := strings.Index(out, "check versions") + if installIdx < bootstrapIdx || installIdx > setupIdx { + t.Errorf("Expected 'install deps' within bootstrap phase, got:\n%s", out) + } + if checkIdx < bootstrapIdx || checkIdx > setupIdx { + t.Errorf("Expected 'check versions' within bootstrap phase, got:\n%s", out) } - return -1 } diff --git a/internal/setup/helpers_test.go b/internal/setup/helpers_test.go index 7fd85ac..5b750ee 100644 --- a/internal/setup/helpers_test.go +++ b/internal/setup/helpers_test.go @@ -93,6 +93,22 @@ func TestCollectSecretNames(t *testing.T) { } } +func containsString(s, substr string) bool { + return len(s) >= len(substr) && (s == substr || len(substr) == 0 || + (len(s) > 0 && len(substr) > 0 && + (s[:len(substr)] == substr || s[len(s)-len(substr):] == substr || + indexOfString(s, substr) >= 0))) +} + +func indexOfString(s, substr string) int { + for i := 0; i <= len(s)-len(substr); i++ { + if s[i:i+len(substr)] == substr { + return i + } + } + return -1 +} + func TestDetectShell(t *testing.T) { originalShell := os.Getenv("SHELL") diff --git a/internal/setup/runner_test.go b/internal/setup/runner_test.go index 9540dc6..60c5b3f 100644 --- a/internal/setup/runner_test.go +++ b/internal/setup/runner_test.go @@ -42,7 +42,7 @@ func TestSetupRunner_Run(t *testing.T) { return } - capture.AssertContains(t, "completed successfully") + capture.AssertContains(t, "[ok] test-step") } func TestSetupRunner_Run_NoCommands(t *testing.T) { @@ -200,6 +200,6 @@ setup: t.Logf("Run error (may be expected): %v", err) } } else { - capture.AssertContains(t, "completed successfully") + capture.AssertContains(t, "[ok] test") } }