diff --git a/CHANGELOG.md b/CHANGELOG.md index a76800f2..aeb0dfa4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **F095**: Deterministic step display ordering in CLI execution summary — `--- Execution Details ---`, step stdout/stderr blocks, and `buildStepInfos` now display steps in workflow-defined order (following `Initial` → default transitions → `OnSuccess`) instead of non-deterministic Go map iteration; new domain-level `ExecutionOrder(*Workflow) []Step` and `NextDefaultStep(*Step) string` functions in `internal/domain/workflow/graph.go` provide a single source of truth for default-path graph traversal; TUI's private `orderedSteps`/`nextStepName` deleted and replaced by delegation to the domain functions; CLI display functions (`showExecutionDetails`, `showStepOutputs`, `showEmptyStepFeedback`, `buildStepInfos`) accept `*workflow.Workflow` parameter and iterate via `ExecutionOrder()` output; `showEmptyStepFeedback` switched from direct `execCtx.States` access to thread-safe `GetStepState` per-step lookup - **F094**: Unified token counting via `ports.Tokenizer` port — all CLI-based agent providers (Claude, Gemini, Codex, GitHub Copilot, OpenCode) now count tokens through an injected `Tokenizer` interface instead of inline `len(output)/4` helpers; default `ApproximationTokenizer` preserves identical behavior; eliminates mutation side-effect on shared conversation turn state during input token estimation; dead `estimateTokens`/`estimateInputTokens` helpers removed; enables future swap to real token counting (e.g., tiktoken, stream-extracted counts) by changing a single injection point ## [0.8.1] - 2026-05-11 diff --git a/CLAUDE.md b/CLAUDE.md index eca80535..f5ca42f6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -243,7 +243,6 @@ func TestWorkflowValidation(t *testing.T) { ## Common Pitfalls -- Always apply code deletions before writing tests that validate the deletion effect; tests may pass against overridden behavior instead of the intended code path - Wrap YAML/JSON mapping errors (duration parse, type conversion) in domain error types; surface failures immediately to prevent silent defaults - Never merge infrastructure provider stubs; always implement ExecuteConversation fully or return NotImplementedError with linked tracking issue - When enabling session persistence in CLI providers, force JSON output format for reliable field extraction; document as known limitation that overrides user-specified format @@ -284,10 +283,10 @@ func TestWorkflowValidation(t *testing.T) { - Always stage all modified implementation files and run 'git status' before marking task complete; unstaged files indicate incomplete task closure. - Update plan task status immediately when implementation completes; regenerate validation report to catch status-code mismatches before submission. - Always replicate nolint:errcheck directives identically across all provider implementations; verify explanatory comments match before make lint +- Always test unplanned file modifications discovered during implementation; update task plan if intentional, revert if accidental ## Test Conventions -- Test context cancellation with context.WithCancel() and early ctx.Err() checks; verify operation fails with wrapped context.Canceled error within timeout - Mock evaluators must have pre-configured results for every expression input; unconfigured expressions return zero value, which may bypass validation checks in evaluation pipelines - Distinguish fixture path updates (allowed without review) from content changes (require explicit review); document rationale for content modifications in commit message - Use _Integration suffix for tests requiring live agent execution or system dependencies; keep unit tests suffix-less in domain/application/infrastructure packages @@ -307,7 +306,9 @@ func TestWorkflowValidation(t *testing.T) { - Test event metadata persistence across all input variations for provider translation; include cases with missing optional nested fields to prevent silent metadata loss - When testing YAML unmarshaling, assert on all nested struct fields; verify that arrays like Events.Subscribe and Events.Emit are populated, not defaulted to empty - New gRPC and concurrency-heavy infrastructure requires >85% test coverage; run 'make test-race' to verify no data races in stream managers and lock-protected sections. +- Always write unit tests for CLI helper functions; parseInputFlags, resolvePromptInput, categorizeError must have >80% coverage before commit ## Review Standards - Never mark implementation complete without confirming `make build`, `make lint`, and `make test` pass with zero violations +- Always verify all validation expert reports generate successfully; missing sections (Conformance, Coverage, Cleanup) indicate incomplete validation diff --git a/docs/user-guide/commands.md b/docs/user-guide/commands.md index 6d22a8e1..c82fed5e 100644 --- a/docs/user-guide/commands.md +++ b/docs/user-guide/commands.md @@ -172,6 +172,23 @@ awf run [flags] **Note:** For agent steps, the `output_format` field controls display filtering: `text` or omitted (default) shows human-readable output; `json` shows raw NDJSON. See [Output Formatting](agent-steps.md#streaming-output-display) for details. +### Execution Summary + +After a workflow completes, AWF displays an execution summary with step statuses, durations, and output. Steps are always displayed in **workflow-defined order** — following the `Initial` state through default transitions and `on_success` references — regardless of actual execution timing or internal map ordering. This ensures consistent, readable output across runs: + +``` +--- Execution Details --- +Status: completed +Steps executed: + init completed (12ms) + build completed (1.234s) + test completed (856ms) + deploy completed (3.012s) + done completed (0ms) +``` + +Steps that were not executed (e.g., because the workflow branched to a different path) are omitted from the summary. Step stdout/stderr blocks follow the same ordering. + ### Examples ```bash diff --git a/internal/domain/workflow/graph.go b/internal/domain/workflow/graph.go index 6b5af3c8..50e4de49 100644 --- a/internal/domain/workflow/graph.go +++ b/internal/domain/workflow/graph.go @@ -1,6 +1,9 @@ package workflow -import "strconv" +import ( + "strconv" + "strings" +) // VisitState represents the DFS visit state of a node during graph traversal. // Used for three-color marking in cycle detection: @@ -232,14 +235,50 @@ func DetectCycles(steps map[string]*Step, initial string) []string { // formatCyclePath formats a cycle path as "A -> B -> C -> A". func formatCyclePath(path []string) string { - if len(path) == 0 { + return strings.Join(path, " -> ") +} + +// NextDefaultStep returns the next step name following the default (unconditional) path. +// Checks Transitions for an entry with empty When first; falls back to OnSuccess. +func NextDefaultStep(step *Step) string { + if step == nil { return "" } - result := path[0] - for i := 1; i < len(path); i++ { - result += " -> " + path[i] + for _, tr := range step.Transitions { + if tr.When == "" { + return tr.Goto + } + } + return step.OnSuccess +} + +// ExecutionOrder returns steps in default-path order by walking the graph from Initial, +// following default transitions (empty When) and falling back to OnSuccess. +// Stops at terminal steps, visited steps (cycle prevention), or missing references. +func ExecutionOrder(wf *Workflow) []Step { + if wf == nil || len(wf.Steps) == 0 || wf.Initial == "" { + return nil } - return result + + visited := make(map[string]bool, len(wf.Steps)) + steps := make([]Step, 0, len(wf.Steps)) + current := wf.Initial + + for current != "" && !visited[current] { + step, ok := wf.Steps[current] + if !ok { + break + } + visited[current] = true + steps = append(steps, *step) + + if step.Type == StepTypeTerminal { + break + } + current = NextDefaultStep(step) + } + + return steps } // GetTransitions returns all outbound transitions from a step. diff --git a/internal/domain/workflow/graph_test.go b/internal/domain/workflow/graph_test.go index 6e64e2b6..bac61a17 100644 --- a/internal/domain/workflow/graph_test.go +++ b/internal/domain/workflow/graph_test.go @@ -1448,3 +1448,463 @@ func TestBuildCyclePath_ErrorHandling(t *testing.T) { assert.Equal(t, []string{"only", "only"}, result) }) } + +// NextDefaultStep Tests + +func TestNextDefaultStep_NilStep(t *testing.T) { + result := workflow.NextDefaultStep(nil) + assert.Equal(t, "", result, "nil step should return empty string") +} + +func TestNextDefaultStep_DefaultTransition(t *testing.T) { + step := &workflow.Step{ + Name: "test", + Type: workflow.StepTypeCommand, + Transitions: workflow.Transitions{ + {When: "", Goto: "next_step"}, + {When: "condition", Goto: "alternate"}, + }, + } + + result := workflow.NextDefaultStep(step) + assert.Equal(t, "next_step", result, "should return default transition Goto") +} + +func TestNextDefaultStep_OnlyOnSuccess(t *testing.T) { + step := &workflow.Step{ + Name: "test", + Type: workflow.StepTypeCommand, + OnSuccess: "success_step", + } + + result := workflow.NextDefaultStep(step) + assert.Equal(t, "success_step", result, "should return OnSuccess when no default transition") +} + +func TestNextDefaultStep_DefaultTransitionTakePrecedence(t *testing.T) { + step := &workflow.Step{ + Name: "test", + Type: workflow.StepTypeCommand, + OnSuccess: "success_step", + Transitions: workflow.Transitions{ + {When: "", Goto: "default_step"}, + }, + } + + result := workflow.NextDefaultStep(step) + assert.Equal(t, "default_step", result, "default transition should take precedence over OnSuccess") +} + +func TestNextDefaultStep_NoTransitionsNoOnSuccess(t *testing.T) { + step := &workflow.Step{ + Name: "test", + Type: workflow.StepTypeCommand, + } + + result := workflow.NextDefaultStep(step) + assert.Equal(t, "", result, "should return empty string when no transitions or OnSuccess") +} + +func TestNextDefaultStep_OnlyConditionalTransitions(t *testing.T) { + step := &workflow.Step{ + Name: "test", + Type: workflow.StepTypeCommand, + Transitions: workflow.Transitions{ + {When: "condition1", Goto: "step1"}, + {When: "condition2", Goto: "step2"}, + }, + } + + result := workflow.NextDefaultStep(step) + assert.Equal(t, "", result, "should return empty string when only conditional transitions") +} + +func TestNextDefaultStep_TerminalStepNoTransitions(t *testing.T) { + step := &workflow.Step{ + Name: "done", + Type: workflow.StepTypeTerminal, + Status: workflow.TerminalSuccess, + } + + result := workflow.NextDefaultStep(step) + assert.Equal(t, "", result, "terminal step should return empty string") +} + +// ExecutionOrder Tests + +func TestExecutionOrder_NilWorkflow(t *testing.T) { + result := workflow.ExecutionOrder(nil) + assert.Nil(t, result, "nil workflow should return nil") +} + +func TestExecutionOrder_EmptyStepsMap(t *testing.T) { + wf := &workflow.Workflow{ + Initial: "start", + Steps: map[string]*workflow.Step{}, + } + + result := workflow.ExecutionOrder(wf) + assert.Nil(t, result, "empty Steps map should return nil") +} + +func TestExecutionOrder_EmptyInitial(t *testing.T) { + wf := &workflow.Workflow{ + Initial: "", + Steps: map[string]*workflow.Step{ + "step1": {Name: "step1", Type: workflow.StepTypeCommand}, + }, + } + + result := workflow.ExecutionOrder(wf) + assert.Nil(t, result, "empty Initial should return nil") +} + +func TestExecutionOrder_MissingInitialStep(t *testing.T) { + wf := &workflow.Workflow{ + Initial: "nonexistent", + Steps: map[string]*workflow.Step{ + "done": {Name: "done", Type: workflow.StepTypeTerminal}, + }, + } + + result := workflow.ExecutionOrder(wf) + assert.Equal(t, []workflow.Step{}, result, "missing initial step should return empty slice") +} + +func TestExecutionOrder_LinearChain(t *testing.T) { + wf := &workflow.Workflow{ + Initial: "init", + Steps: map[string]*workflow.Step{ + "init": { + Name: "init", + Type: workflow.StepTypeCommand, + OnSuccess: "build", + }, + "build": { + Name: "build", + Type: workflow.StepTypeCommand, + OnSuccess: "test", + }, + "test": { + Name: "test", + Type: workflow.StepTypeCommand, + OnSuccess: "done", + }, + "done": { + Name: "done", + Type: workflow.StepTypeTerminal, + Status: workflow.TerminalSuccess, + }, + }, + } + + result := workflow.ExecutionOrder(wf) + + require.NotNil(t, result) + require.Len(t, result, 4, "linear chain should have 4 steps") + assert.Equal(t, "init", result[0].Name) + assert.Equal(t, "build", result[1].Name) + assert.Equal(t, "test", result[2].Name) + assert.Equal(t, "done", result[3].Name) +} + +func TestExecutionOrder_FollowsDefaultTransition(t *testing.T) { + wf := &workflow.Workflow{ + Initial: "start", + Steps: map[string]*workflow.Step{ + "start": { + Name: "start", + Type: workflow.StepTypeCommand, + Transitions: workflow.Transitions{ + {When: "", Goto: "default_path"}, + {When: "condition", Goto: "alternate_path"}, + }, + }, + "default_path": { + Name: "default_path", + Type: workflow.StepTypeCommand, + OnSuccess: "done", + }, + "alternate_path": { + Name: "alternate_path", + Type: workflow.StepTypeCommand, + OnSuccess: "done", + }, + "done": { + Name: "done", + Type: workflow.StepTypeTerminal, + Status: workflow.TerminalSuccess, + }, + }, + } + + result := workflow.ExecutionOrder(wf) + + require.NotNil(t, result) + require.Len(t, result, 3) + assert.Equal(t, "start", result[0].Name) + assert.Equal(t, "default_path", result[1].Name) + assert.Equal(t, "done", result[2].Name) +} + +func TestExecutionOrder_StopsAtTerminalStep(t *testing.T) { + wf := &workflow.Workflow{ + Initial: "start", + Steps: map[string]*workflow.Step{ + "start": { + Name: "start", + Type: workflow.StepTypeCommand, + OnSuccess: "done", + }, + "done": { + Name: "done", + Type: workflow.StepTypeTerminal, + Status: workflow.TerminalSuccess, + }, + }, + } + + result := workflow.ExecutionOrder(wf) + + require.NotNil(t, result) + require.Len(t, result, 2) + assert.Equal(t, "start", result[0].Name) + assert.Equal(t, "done", result[1].Name) +} + +func TestExecutionOrder_StopsAtRevisitedStep(t *testing.T) { + // Cycle: a -> b -> a (should stop when revisiting a) + wf := &workflow.Workflow{ + Initial: "a", + Steps: map[string]*workflow.Step{ + "a": { + Name: "a", + Type: workflow.StepTypeCommand, + OnSuccess: "b", + }, + "b": { + Name: "b", + Type: workflow.StepTypeCommand, + OnSuccess: "a", + }, + }, + } + + result := workflow.ExecutionOrder(wf) + + require.NotNil(t, result) + require.Len(t, result, 2, "should stop at revisited step 'a'") + assert.Equal(t, "a", result[0].Name) + assert.Equal(t, "b", result[1].Name) +} + +func TestExecutionOrder_StopsAtMissingTransitionTarget(t *testing.T) { + wf := &workflow.Workflow{ + Initial: "start", + Steps: map[string]*workflow.Step{ + "start": { + Name: "start", + Type: workflow.StepTypeCommand, + OnSuccess: "nonexistent", + }, + }, + } + + result := workflow.ExecutionOrder(wf) + + require.NotNil(t, result) + assert.Len(t, result, 1, "should stop when next step is missing") + assert.Equal(t, "start", result[0].Name) +} + +func TestExecutionOrder_IncludesParallelStepAsSingleEntry(t *testing.T) { + wf := &workflow.Workflow{ + Initial: "start", + Steps: map[string]*workflow.Step{ + "start": { + Name: "start", + Type: workflow.StepTypeCommand, + OnSuccess: "parallel_step", + }, + "parallel_step": { + Name: "parallel_step", + Type: workflow.StepTypeParallel, + Branches: []string{"branch1", "branch2"}, + OnSuccess: "done", + }, + "branch1": { + Name: "branch1", + Type: workflow.StepTypeCommand, + OnSuccess: "done", + }, + "branch2": { + Name: "branch2", + Type: workflow.StepTypeCommand, + OnSuccess: "done", + }, + "done": { + Name: "done", + Type: workflow.StepTypeTerminal, + Status: workflow.TerminalSuccess, + }, + }, + } + + result := workflow.ExecutionOrder(wf) + + require.NotNil(t, result) + require.Len(t, result, 3) + assert.Equal(t, "start", result[0].Name) + assert.Equal(t, "parallel_step", result[1].Name, "parallel step should be included as single entry") + assert.Equal(t, "done", result[2].Name) +} + +func TestExecutionOrder_StopsAtOnlyConditionalTransitions(t *testing.T) { + wf := &workflow.Workflow{ + Initial: "start", + Steps: map[string]*workflow.Step{ + "start": { + Name: "start", + Type: workflow.StepTypeCommand, + OnSuccess: "middle", + }, + "middle": { + Name: "middle", + Type: workflow.StepTypeCommand, + Transitions: workflow.Transitions{ + {When: "condition1", Goto: "path1"}, + {When: "condition2", Goto: "path2"}, + }, + }, + "path1": { + Name: "path1", + Type: workflow.StepTypeCommand, + OnSuccess: "done", + }, + "path2": { + Name: "path2", + Type: workflow.StepTypeCommand, + OnSuccess: "done", + }, + "done": { + Name: "done", + Type: workflow.StepTypeTerminal, + Status: workflow.TerminalSuccess, + }, + }, + } + + result := workflow.ExecutionOrder(wf) + + require.NotNil(t, result) + require.Len(t, result, 2, "should stop at step with only conditional transitions") + assert.Equal(t, "start", result[0].Name) + assert.Equal(t, "middle", result[1].Name) +} + +func TestExecutionOrder_ValueCopies(t *testing.T) { + originalStep := &workflow.Step{ + Name: "start", + Type: workflow.StepTypeCommand, + OnSuccess: "done", + } + + wf := &workflow.Workflow{ + Initial: "start", + Steps: map[string]*workflow.Step{ + "start": originalStep, + "done": { + Name: "done", + Type: workflow.StepTypeTerminal, + Status: workflow.TerminalSuccess, + }, + }, + } + + result := workflow.ExecutionOrder(wf) + + require.NotNil(t, result) + require.Len(t, result, 2) + + // Verify returned steps are value copies, not pointers to originals + assert.Equal(t, "start", result[0].Name) + assert.NotSame(t, originalStep, &result[0], "should return value copy, not pointer to original") +} + +func TestExecutionOrder_ComplexChainWithMultipleTerminals(t *testing.T) { + wf := &workflow.Workflow{ + Initial: "init", + Steps: map[string]*workflow.Step{ + "init": { + Name: "init", + Type: workflow.StepTypeCommand, + OnSuccess: "process", + }, + "process": { + Name: "process", + Type: workflow.StepTypeCommand, + OnSuccess: "success_terminal", + OnFailure: "error_terminal", + }, + "success_terminal": { + Name: "success_terminal", + Type: workflow.StepTypeTerminal, + Status: workflow.TerminalSuccess, + }, + "error_terminal": { + Name: "error_terminal", + Type: workflow.StepTypeTerminal, + Status: workflow.TerminalFailure, + }, + }, + } + + result := workflow.ExecutionOrder(wf) + + require.NotNil(t, result) + // Should follow OnSuccess path, not OnFailure + require.Len(t, result, 3) + assert.Equal(t, "init", result[0].Name) + assert.Equal(t, "process", result[1].Name) + assert.Equal(t, "success_terminal", result[2].Name) +} + +func TestExecutionOrder_BothDefaultTransitionAndOnSuccess(t *testing.T) { + wf := &workflow.Workflow{ + Initial: "start", + Steps: map[string]*workflow.Step{ + "start": { + Name: "start", + Type: workflow.StepTypeCommand, + OnSuccess: "onsuccess_step", + Transitions: workflow.Transitions{ + {When: "", Goto: "default_step"}, + }, + }, + "default_step": { + Name: "default_step", + Type: workflow.StepTypeCommand, + OnSuccess: "done", + }, + "onsuccess_step": { + Name: "onsuccess_step", + Type: workflow.StepTypeCommand, + OnSuccess: "done", + }, + "done": { + Name: "done", + Type: workflow.StepTypeTerminal, + Status: workflow.TerminalSuccess, + }, + }, + } + + result := workflow.ExecutionOrder(wf) + + require.NotNil(t, result) + // Should follow default transition, not OnSuccess + require.Len(t, result, 3) + assert.Equal(t, "start", result[0].Name) + assert.Equal(t, "default_step", result[1].Name, "should follow default transition, not OnSuccess") + assert.Equal(t, "done", result[2].Name) +} diff --git a/internal/interfaces/cli/resume.go b/internal/interfaces/cli/resume.go index 35a264be..025b5daf 100644 --- a/internal/interfaces/cli/resume.go +++ b/internal/interfaces/cli/resume.go @@ -257,6 +257,12 @@ func runResume(cmd *cobra.Command, cfg *Config, workflowID string, inputFlags [] // Calculate duration durationMs := time.Since(startTime).Milliseconds() + // Load workflow for ordered step display (F095); nil on error falls back to no steps shown + var wf *workflow.Workflow + if execCtx != nil && execCtx.WorkflowName != "" { + wf, _ = repo.Load(ctx, execCtx.WorkflowName) //nolint:errcheck // nil wf skips ordered display gracefully + } + // Output result (same pattern as runWorkflow) if cfg.OutputFormat == ui.FormatJSON || cfg.OutputFormat == ui.FormatQuiet { result := ui.RunResult{ @@ -267,7 +273,7 @@ func runResume(cmd *cobra.Command, cfg *Config, workflowID string, inputFlags [] result.WorkflowID = execCtx.WorkflowID result.Status = string(execCtx.Status) if cfg.OutputMode == OutputBuffered { - result.Steps = buildStepInfos(execCtx) + result.Steps = buildStepInfos(wf, execCtx) } } if execErr != nil { @@ -292,7 +298,7 @@ func runResume(cmd *cobra.Command, cfg *Config, workflowID string, inputFlags [] if execCtx != nil { result.WorkflowID = execCtx.WorkflowID result.Status = string(execCtx.Status) - result.Steps = buildStepInfos(execCtx) + result.Steps = buildStepInfos(wf, execCtx) } if execErr != nil { result.Status = "failed" @@ -312,7 +318,7 @@ func runResume(cmd *cobra.Command, cfg *Config, workflowID string, inputFlags [] if execErr != nil { if cfg.OutputMode == OutputBuffered && execCtx != nil { - showStepOutputs(formatter, execCtx) + showStepOutputs(formatter, wf, execCtx) } if execCtx != nil { formatter.Info(fmt.Sprintf("Workflow ID: %s", execCtx.WorkflowID)) @@ -321,18 +327,18 @@ func runResume(cmd *cobra.Command, cfg *Config, workflowID string, inputFlags [] } if cfg.OutputMode != OutputBuffered && execCtx != nil { - showEmptyStepFeedback(formatter, execCtx) + showEmptyStepFeedback(formatter, wf, execCtx) } formatter.Success(fmt.Sprintf("Workflow resumed and completed in %s", duration)) formatter.Info(fmt.Sprintf("Workflow ID: %s", execCtx.WorkflowID)) if cfg.OutputMode == OutputBuffered && execCtx != nil { - showStepOutputs(formatter, execCtx) + showStepOutputs(formatter, wf, execCtx) } if cfg.Verbose && execCtx != nil { - showExecutionDetails(formatter, execCtx) + showExecutionDetails(formatter, wf, execCtx) } return nil diff --git a/internal/interfaces/cli/run.go b/internal/interfaces/cli/run.go index dcc8871f..5c476dec 100644 --- a/internal/interfaces/cli/run.go +++ b/internal/interfaces/cli/run.go @@ -387,7 +387,7 @@ func runWorkflow(cmd *cobra.Command, cfg *Config, workflowName string, inputFlag result.Status = string(execCtx.Status) // Include step outputs in buffered mode if cfg.OutputMode == OutputBuffered { - result.Steps = buildStepInfos(execCtx) + result.Steps = buildStepInfos(wf, execCtx) } } if execErr != nil { @@ -412,7 +412,7 @@ func runWorkflow(cmd *cobra.Command, cfg *Config, workflowName string, inputFlag if execCtx != nil { result.WorkflowID = execCtx.WorkflowID result.Status = string(execCtx.Status) - result.Steps = buildStepInfos(execCtx) + result.Steps = buildStepInfos(wf, execCtx) } if execErr != nil { result.Status = "failed" @@ -433,7 +433,7 @@ func runWorkflow(cmd *cobra.Command, cfg *Config, workflowName string, inputFlag if execErr != nil { // Show buffered output on error if cfg.OutputMode == OutputBuffered && execCtx != nil { - showStepOutputs(formatter, execCtx) + showStepOutputs(formatter, wf, execCtx) } if execCtx != nil { formatter.Info(fmt.Sprintf("Workflow ID: %s", execCtx.WorkflowID)) @@ -443,7 +443,7 @@ func runWorkflow(cmd *cobra.Command, cfg *Config, workflowName string, inputFlag // F037: Show success feedback for steps with no output (silent/streaming modes) if cfg.OutputMode != OutputBuffered && execCtx != nil { - showEmptyStepFeedback(formatter, execCtx) + showEmptyStepFeedback(formatter, wf, execCtx) } formatter.Success(fmt.Sprintf("Workflow completed successfully in %s", duration)) @@ -451,11 +451,11 @@ func runWorkflow(cmd *cobra.Command, cfg *Config, workflowName string, inputFlag // Show buffered output after successful completion if cfg.OutputMode == OutputBuffered && execCtx != nil { - showStepOutputs(formatter, execCtx) + showStepOutputs(formatter, wf, execCtx) } if cfg.Verbose && execCtx != nil { - showExecutionDetails(formatter, execCtx) + showExecutionDetails(formatter, wf, execCtx) } return nil @@ -713,15 +713,18 @@ func resolvePromptFromPaths(relativePath string, paths []repository.SourcedPath) relativePath, strings.Join(searchedPaths, ", ")) } -func showExecutionDetails(formatter *ui.Formatter, execCtx *workflow.ExecutionContext) { +func showExecutionDetails(formatter *ui.Formatter, wf *workflow.Workflow, execCtx *workflow.ExecutionContext) { formatter.Printf("\n--- Execution Details ---\n") formatter.Printf("Status: %s\n", execCtx.Status) formatter.Printf("Steps executed:\n") - allStates := execCtx.GetAllStepStates() - for name, state := range allStates { + for _, step := range workflow.ExecutionOrder(wf) { + state, ok := execCtx.GetStepState(step.Name) + if !ok { + continue + } duration := state.CompletedAt.Sub(state.StartedAt).Round(time.Millisecond) - formatter.StatusLine(" "+name, string(state.Status), fmt.Sprintf("(%s)", duration)) + formatter.StatusLine(" "+step.Name, string(state.Status), fmt.Sprintf("(%s)", duration)) } } @@ -732,43 +735,53 @@ func displayValueOf(state *workflow.StepState) string { return state.Output } -func showStepOutputs(formatter *ui.Formatter, execCtx *workflow.ExecutionContext) { - allStates := execCtx.GetAllStepStates() - for name, state := range allStates { +func showStepOutputs(formatter *ui.Formatter, wf *workflow.Workflow, execCtx *workflow.ExecutionContext) { + for _, step := range workflow.ExecutionOrder(wf) { + state, ok := execCtx.GetStepState(step.Name) + if !ok { + continue + } displayOutput := displayValueOf(&state) if displayOutput != "" { - formatter.Printf("\n--- [%s] stdout ---\n", name) + formatter.Printf("\n--- [%s] stdout ---\n", step.Name) formatter.Printf("%s", displayOutput) } if state.Stderr != "" { - formatter.Printf("\n--- [%s] stderr ---\n", name) + formatter.Printf("\n--- [%s] stderr ---\n", step.Name) formatter.Printf("%s", state.Stderr) } - // F037: Success feedback for steps with no output if state.Output == "" && state.Stderr == "" && state.Status == workflow.StatusCompleted { - formatter.StepSuccess(name) + formatter.StepSuccess(step.Name) } } } // showEmptyStepFeedback displays success message for steps that had no output. // Used for silent/streaming modes where showStepOutputs is not called. -func showEmptyStepFeedback(formatter *ui.Formatter, execCtx *workflow.ExecutionContext) { - for name, state := range execCtx.States { +func showEmptyStepFeedback(formatter *ui.Formatter, wf *workflow.Workflow, execCtx *workflow.ExecutionContext) { + for _, step := range workflow.ExecutionOrder(wf) { + state, ok := execCtx.GetStepState(step.Name) + if !ok { + continue + } if state.Output == "" && state.Stderr == "" && state.Status == workflow.StatusCompleted { - formatter.StepSuccess(name) + formatter.StepSuccess(step.Name) } } } -func buildStepInfos(execCtx *workflow.ExecutionContext) []ui.StepInfo { - // Preallocate for all steps - steps := make([]ui.StepInfo, 0, len(execCtx.States)) - for name, state := range execCtx.States { +func buildStepInfos(wf *workflow.Workflow, execCtx *workflow.ExecutionContext) []ui.StepInfo { + ordered := workflow.ExecutionOrder(wf) + steps := make([]ui.StepInfo, 0, len(ordered)) + for _, step := range ordered { + state, ok := execCtx.GetStepState(step.Name) + if !ok { + continue + } steps = append(steps, ui.StepInfo{ - Name: name, + Name: step.Name, Status: string(state.Status), Output: state.Output, Stderr: state.Stderr, diff --git a/internal/interfaces/cli/run_internal_test.go b/internal/interfaces/cli/run_internal_test.go index 0fc4d62c..e0d637f9 100644 --- a/internal/interfaces/cli/run_internal_test.go +++ b/internal/interfaces/cli/run_internal_test.go @@ -2,6 +2,7 @@ package cli import ( "bytes" + "maps" "os" "path/filepath" "strings" @@ -40,11 +41,16 @@ func setupConfigTestDir(t *testing.T) string { func TestShowExecutionDetails(t *testing.T) { tests := []struct { name string + wf *workflow.Workflow execCtx *workflow.ExecutionContext wantOut []string // substrings expected in output }{ { name: "single completed step", + wf: &workflow.Workflow{ + Initial: "step1", + Steps: map[string]*workflow.Step{"step1": {Name: "step1", Type: workflow.StepTypeTerminal}}, + }, execCtx: func() *workflow.ExecutionContext { ctx := workflow.NewExecutionContext("test-123", "test-wf") ctx.Status = workflow.StatusCompleted @@ -60,6 +66,13 @@ func TestShowExecutionDetails(t *testing.T) { }, { name: "multiple steps with different statuses", + wf: &workflow.Workflow{ + Initial: "fetch", + Steps: map[string]*workflow.Step{ + "fetch": {Name: "fetch", Type: workflow.StepTypeCommand, OnSuccess: "process"}, + "process": {Name: "process", Type: workflow.StepTypeTerminal}, + }, + }, execCtx: func() *workflow.ExecutionContext { ctx := workflow.NewExecutionContext("test-456", "multi-wf") ctx.Status = workflow.StatusFailed @@ -79,6 +92,66 @@ func TestShowExecutionDetails(t *testing.T) { }(), wantOut: []string{"fetch", "process", "failed"}, }, + { + name: "steps displayed in graph order not insertion order", + wf: &workflow.Workflow{ + Initial: "fetch", + Steps: map[string]*workflow.Step{ + "fetch": {Name: "fetch", Type: workflow.StepTypeCommand, OnSuccess: "process"}, + "process": {Name: "process", Type: workflow.StepTypeCommand, OnSuccess: "store"}, + "store": {Name: "store", Type: workflow.StepTypeTerminal}, + }, + }, + execCtx: func() *workflow.ExecutionContext { + ctx := workflow.NewExecutionContext("test-order", "order-wf") + ctx.Status = workflow.StatusCompleted + // Insert states in reverse order + ctx.States["store"] = workflow.StepState{ + Name: "store", + Status: workflow.StatusCompleted, + StartedAt: time.Now(), + CompletedAt: time.Now().Add(10 * time.Millisecond), + } + ctx.States["process"] = workflow.StepState{ + Name: "process", + Status: workflow.StatusCompleted, + StartedAt: time.Now(), + CompletedAt: time.Now().Add(20 * time.Millisecond), + } + ctx.States["fetch"] = workflow.StepState{ + Name: "fetch", + Status: workflow.StatusCompleted, + StartedAt: time.Now(), + CompletedAt: time.Now().Add(30 * time.Millisecond), + } + return ctx + }(), + wantOut: []string{"fetch", "process", "store"}, + }, + { + name: "skips steps not in execution context without panic", + wf: &workflow.Workflow{ + Initial: "a", + Steps: map[string]*workflow.Step{ + "a": {Name: "a", Type: workflow.StepTypeCommand, OnSuccess: "b"}, + "b": {Name: "b", Type: workflow.StepTypeCommand, OnSuccess: "c"}, + "c": {Name: "c", Type: workflow.StepTypeTerminal}, + }, + }, + execCtx: func() *workflow.ExecutionContext { + ctx := workflow.NewExecutionContext("test-skip", "skip-wf") + ctx.Status = workflow.StatusCompleted + // Only "a" has state; "b" and "c" are absent + ctx.States["a"] = workflow.StepState{ + Name: "a", + Status: workflow.StatusCompleted, + StartedAt: time.Now(), + CompletedAt: time.Now().Add(5 * time.Millisecond), + } + return ctx + }(), + wantOut: []string{"a"}, + }, } for _, tt := range tests { @@ -86,7 +159,7 @@ func TestShowExecutionDetails(t *testing.T) { buf := new(bytes.Buffer) formatter := ui.NewFormatter(buf, ui.FormatOptions{NoColor: true}) - showExecutionDetails(formatter, tt.execCtx) + showExecutionDetails(formatter, tt.wf, tt.execCtx) output := buf.String() for _, want := range tt.wantOut { @@ -94,17 +167,94 @@ func TestShowExecutionDetails(t *testing.T) { } }) } + + // Dedicated ordering sub-test for showExecutionDetails. + t.Run("fetch appears before process before store in output", func(t *testing.T) { + wf := &workflow.Workflow{ + Initial: "fetch", + Steps: map[string]*workflow.Step{ + "fetch": {Name: "fetch", Type: workflow.StepTypeCommand, OnSuccess: "process"}, + "process": {Name: "process", Type: workflow.StepTypeCommand, OnSuccess: "store"}, + "store": {Name: "store", Type: workflow.StepTypeTerminal}, + }, + } + ctx := workflow.NewExecutionContext("order-details", "wf") + ctx.Status = workflow.StatusCompleted + // Reverse insertion order + ctx.States["store"] = workflow.StepState{ + Name: "store", Status: workflow.StatusCompleted, + StartedAt: time.Now(), CompletedAt: time.Now().Add(10 * time.Millisecond), + } + ctx.States["process"] = workflow.StepState{ + Name: "process", Status: workflow.StatusCompleted, + StartedAt: time.Now(), CompletedAt: time.Now().Add(20 * time.Millisecond), + } + ctx.States["fetch"] = workflow.StepState{ + Name: "fetch", Status: workflow.StatusCompleted, + StartedAt: time.Now(), CompletedAt: time.Now().Add(30 * time.Millisecond), + } + + buf := new(bytes.Buffer) + formatter := ui.NewFormatter(buf, ui.FormatOptions{NoColor: true}) + showExecutionDetails(formatter, wf, ctx) + output := buf.String() + + fetchIdx := strings.Index(output, "fetch") + processIdx := strings.Index(output, "process") + storeIdx := strings.Index(output, "store") + + assert.Greater(t, fetchIdx, -1, "output should contain 'fetch'") + assert.Greater(t, processIdx, -1, "output should contain 'process'") + assert.Greater(t, storeIdx, -1, "output should contain 'store'") + assert.Less(t, fetchIdx, processIdx, "'fetch' should appear before 'process' in output") + assert.Less(t, processIdx, storeIdx, "'process' should appear before 'store' in output") + }) + + // Dedicated sub-test: steps absent from ExecutionContext must not appear. + t.Run("steps absent from execution context do not appear in output", func(t *testing.T) { + wf := &workflow.Workflow{ + Initial: "a", + Steps: map[string]*workflow.Step{ + "a": {Name: "a", Type: workflow.StepTypeCommand, OnSuccess: "b"}, + "b": {Name: "b", Type: workflow.StepTypeCommand, OnSuccess: "c"}, + "c": {Name: "c", Type: workflow.StepTypeTerminal}, + }, + } + ctx := workflow.NewExecutionContext("skip-details", "wf") + ctx.States["a"] = workflow.StepState{ + Name: "a", Status: workflow.StatusCompleted, + StartedAt: time.Now(), CompletedAt: time.Now().Add(5 * time.Millisecond), + } + + buf := new(bytes.Buffer) + formatter := ui.NewFormatter(buf, ui.FormatOptions{NoColor: true}) + showExecutionDetails(formatter, wf, ctx) + output := buf.String() + + assert.Contains(t, output, "a", "output should contain step 'a'") + assert.NotContains(t, output, " b ", "output should not reference step 'b'") + assert.NotContains(t, output, " c ", "output should not reference step 'c'") + }) +} + +func makeWorkflowWithStep(stepName string) *workflow.Workflow { + return &workflow.Workflow{ + Initial: stepName, + Steps: map[string]*workflow.Step{stepName: {Name: stepName, Type: workflow.StepTypeTerminal}}, + } } func TestShowStepOutputs(t *testing.T) { tests := []struct { name string + wf *workflow.Workflow execCtx *workflow.ExecutionContext wantOut []string notWantOut []string }{ { name: "step with stdout only", + wf: makeWorkflowWithStep("build"), execCtx: func() *workflow.ExecutionContext { ctx := workflow.NewExecutionContext("test-1", "wf") ctx.States["build"] = workflow.StepState{ @@ -118,6 +268,7 @@ func TestShowStepOutputs(t *testing.T) { }, { name: "step with stderr", + wf: makeWorkflowWithStep("lint"), execCtx: func() *workflow.ExecutionContext { ctx := workflow.NewExecutionContext("test-2", "wf") ctx.States["lint"] = workflow.StepState{ @@ -131,6 +282,7 @@ func TestShowStepOutputs(t *testing.T) { }, { name: "step with no output shows success", + wf: makeWorkflowWithStep("clean"), execCtx: func() *workflow.ExecutionContext { ctx := workflow.NewExecutionContext("test-3", "wf") ctx.States["clean"] = workflow.StepState{ @@ -145,6 +297,7 @@ func TestShowStepOutputs(t *testing.T) { }, { name: "prefers DisplayOutput over Output when non-empty", + wf: makeWorkflowWithStep("agent-step"), execCtx: func() *workflow.ExecutionContext { ctx := workflow.NewExecutionContext("test-4", "wf") ctx.States["agent-step"] = workflow.StepState{ @@ -160,6 +313,7 @@ func TestShowStepOutputs(t *testing.T) { }, { name: "falls back to Output when DisplayOutput empty", + wf: makeWorkflowWithStep("plain-step"), execCtx: func() *workflow.ExecutionContext { ctx := workflow.NewExecutionContext("test-5", "wf") ctx.States["plain-step"] = workflow.StepState{ @@ -174,6 +328,7 @@ func TestShowStepOutputs(t *testing.T) { }, { name: "success feedback when both DisplayOutput and Output empty", + wf: makeWorkflowWithStep("silent-agent"), execCtx: func() *workflow.ExecutionContext { ctx := workflow.NewExecutionContext("test-6", "wf") ctx.States["silent-agent"] = workflow.StepState{ @@ -189,6 +344,7 @@ func TestShowStepOutputs(t *testing.T) { }, { name: "uses Output for success feedback detection, not DisplayOutput", + wf: makeWorkflowWithStep("output-step"), execCtx: func() *workflow.ExecutionContext { ctx := workflow.NewExecutionContext("test-7", "wf") ctx.States["output-step"] = workflow.StepState{ @@ -202,6 +358,29 @@ func TestShowStepOutputs(t *testing.T) { }(), wantOut: []string{"output-step", "stdout", "some raw output"}, }, + { + name: "skips steps not in execution context", + wf: &workflow.Workflow{ + Initial: "a", + Steps: map[string]*workflow.Step{ + "a": {Name: "a", Type: workflow.StepTypeCommand, OnSuccess: "b"}, + "b": {Name: "b", Type: workflow.StepTypeCommand, OnSuccess: "c"}, + "c": {Name: "c", Type: workflow.StepTypeTerminal}, + }, + }, + execCtx: func() *workflow.ExecutionContext { + ctx := workflow.NewExecutionContext("test-8", "wf") + // Only "a" has state; "b" and "c" are absent + ctx.States["a"] = workflow.StepState{ + Name: "a", + Status: workflow.StatusCompleted, + Output: "hello", + } + return ctx + }(), + wantOut: []string{"[a]", "stdout", "hello"}, + notWantOut: []string{"[b]", "[c]"}, + }, } for _, tt := range tests { @@ -209,7 +388,7 @@ func TestShowStepOutputs(t *testing.T) { buf := new(bytes.Buffer) formatter := ui.NewFormatter(buf, ui.FormatOptions{NoColor: true}) - showStepOutputs(formatter, tt.execCtx) + showStepOutputs(formatter, tt.wf, tt.execCtx) output := buf.String() for _, want := range tt.wantOut { @@ -272,11 +451,13 @@ func TestDisplayValueOf(t *testing.T) { func TestShowEmptyStepFeedback(t *testing.T) { tests := []struct { name string + wf *workflow.Workflow execCtx *workflow.ExecutionContext wantStep string }{ { name: "completed step with no output", + wf: makeWorkflowWithStep("silent-step"), execCtx: func() *workflow.ExecutionContext { ctx := workflow.NewExecutionContext("test-1", "wf") ctx.States["silent-step"] = workflow.StepState{ @@ -291,6 +472,7 @@ func TestShowEmptyStepFeedback(t *testing.T) { }, { name: "step with output should not show feedback", + wf: makeWorkflowWithStep("verbose-step"), execCtx: func() *workflow.ExecutionContext { ctx := workflow.NewExecutionContext("test-2", "wf") ctx.States["verbose-step"] = workflow.StepState{ @@ -302,6 +484,29 @@ func TestShowEmptyStepFeedback(t *testing.T) { }(), wantStep: "", // should not contain step name in success message }, + { + name: "skips steps not in execution context", + wf: &workflow.Workflow{ + Initial: "a", + Steps: map[string]*workflow.Step{ + "a": {Name: "a", Type: workflow.StepTypeCommand, OnSuccess: "b"}, + "b": {Name: "b", Type: workflow.StepTypeCommand, OnSuccess: "c"}, + "c": {Name: "c", Type: workflow.StepTypeTerminal}, + }, + }, + execCtx: func() *workflow.ExecutionContext { + ctx := workflow.NewExecutionContext("test-3", "wf") + // Only "a" has state (completed, no output, no stderr) + ctx.States["a"] = workflow.StepState{ + Name: "a", + Status: workflow.StatusCompleted, + Output: "", + Stderr: "", + } + return ctx + }(), + wantStep: "a", + }, } for _, tt := range tests { @@ -309,12 +514,17 @@ func TestShowEmptyStepFeedback(t *testing.T) { buf := new(bytes.Buffer) formatter := ui.NewFormatter(buf, ui.FormatOptions{NoColor: true}) - showEmptyStepFeedback(formatter, tt.execCtx) + showEmptyStepFeedback(formatter, tt.wf, tt.execCtx) output := buf.String() if tt.wantStep != "" { assert.Contains(t, output, tt.wantStep) } + // For the skip-missing case, verify absent steps do not appear + if tt.name == "skips steps not in execution context" { + assert.NotContains(t, output, " b ", "output should not reference absent step 'b'") + assert.NotContains(t, output, " c ", "output should not reference absent step 'c'") + } }) } } @@ -322,12 +532,14 @@ func TestShowEmptyStepFeedback(t *testing.T) { func TestBuildStepInfos(t *testing.T) { tests := []struct { name string + wf *workflow.Workflow execCtx *workflow.ExecutionContext wantCount int wantNames []string }{ { name: "single step", + wf: makeWorkflowWithStep("step1"), execCtx: func() *workflow.ExecutionContext { ctx := workflow.NewExecutionContext("test-1", "wf") ctx.States["step1"] = workflow.StepState{ @@ -345,6 +557,14 @@ func TestBuildStepInfos(t *testing.T) { }, { name: "multiple steps", + wf: &workflow.Workflow{ + Initial: "fetch", + Steps: map[string]*workflow.Step{ + "fetch": {Name: "fetch", Type: workflow.StepTypeCommand, OnSuccess: "process"}, + "process": {Name: "process", Type: workflow.StepTypeCommand, OnSuccess: "store"}, + "store": {Name: "store", Type: workflow.StepTypeTerminal}, + }, + }, execCtx: func() *workflow.ExecutionContext { ctx := workflow.NewExecutionContext("test-2", "wf") ctx.States["fetch"] = workflow.StepState{Name: "fetch", Status: workflow.StatusCompleted} @@ -357,15 +577,62 @@ func TestBuildStepInfos(t *testing.T) { }, { name: "empty states", + wf: nil, execCtx: workflow.NewExecutionContext("test-3", "wf"), wantCount: 0, wantNames: []string{}, }, + { + name: "ordering is deterministic regardless of map insertion order", + wf: &workflow.Workflow{ + Initial: "fetch", + Steps: map[string]*workflow.Step{ + "fetch": {Name: "fetch", Type: workflow.StepTypeCommand, OnSuccess: "process"}, + "process": {Name: "process", Type: workflow.StepTypeCommand, OnSuccess: "store"}, + "store": {Name: "store", Type: workflow.StepTypeTerminal}, + }, + }, + execCtx: func() *workflow.ExecutionContext { + ctx := workflow.NewExecutionContext("test-4", "wf") + // Insert states in REVERSE order to verify graph traversal order wins + ctx.States["store"] = workflow.StepState{Name: "store", Status: workflow.StatusCompleted} + ctx.States["process"] = workflow.StepState{Name: "process", Status: workflow.StatusCompleted} + ctx.States["fetch"] = workflow.StepState{Name: "fetch", Status: workflow.StatusCompleted} + return ctx + }(), + wantCount: 3, + wantNames: []string{"fetch", "process", "store"}, + }, + { + name: "skips steps not in execution context", + wf: &workflow.Workflow{ + Initial: "a", + Steps: map[string]*workflow.Step{ + "a": {Name: "a", Type: workflow.StepTypeCommand, OnSuccess: "b"}, + "b": {Name: "b", Type: workflow.StepTypeCommand, OnSuccess: "c"}, + "c": {Name: "c", Type: workflow.StepTypeTerminal}, + }, + }, + execCtx: func() *workflow.ExecutionContext { + ctx := workflow.NewExecutionContext("test-5", "wf") + // Only "a" has state; "b" and "c" are absent + ctx.States["a"] = workflow.StepState{ + Name: "a", + Status: workflow.StatusCompleted, + Output: "output-a", + StartedAt: time.Now(), + CompletedAt: time.Now().Add(10 * time.Millisecond), + } + return ctx + }(), + wantCount: 1, + wantNames: []string{"a"}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - steps := buildStepInfos(tt.execCtx) + steps := buildStepInfos(tt.wf, tt.execCtx) assert.Len(t, steps, tt.wantCount) @@ -379,6 +646,32 @@ func TestBuildStepInfos(t *testing.T) { } }) } + + // Dedicated ordering sub-test: verify graph-traversal order, not map insertion order. + t.Run("ordering assertion is exact not just presence", func(t *testing.T) { + wf := &workflow.Workflow{ + Initial: "fetch", + Steps: map[string]*workflow.Step{ + "fetch": {Name: "fetch", Type: workflow.StepTypeCommand, OnSuccess: "process"}, + "process": {Name: "process", Type: workflow.StepTypeCommand, OnSuccess: "store"}, + "store": {Name: "store", Type: workflow.StepTypeTerminal}, + }, + } + ctx := workflow.NewExecutionContext("order-test", "wf") + // Reverse insertion order + ctx.States["store"] = workflow.StepState{Name: "store", Status: workflow.StatusCompleted} + ctx.States["process"] = workflow.StepState{Name: "process", Status: workflow.StatusCompleted} + ctx.States["fetch"] = workflow.StepState{Name: "fetch", Status: workflow.StatusCompleted} + + steps := buildStepInfos(wf, ctx) + + names := make([]string, len(steps)) + for i, s := range steps { + names[i] = s.Name + } + assert.Equal(t, []string{"fetch", "process", "store"}, names, + "steps must follow graph traversal order regardless of States map insertion order") + }) } func TestCategorizeError(t *testing.T) { @@ -421,12 +714,9 @@ func TestCategorizeError(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := &exitError{err: assert.AnError} - // Create error with specific message for testing testErr := &testError{msg: tt.errMsg} got := categorizeError(testErr) assert.Equal(t, tt.wantExit, got) - _ = err // silence unused }) } } @@ -1325,13 +1615,9 @@ func TestMergeInputs_Immutability(t *testing.T) { // Take copies before merge originalConfig := make(map[string]any) - for k, v := range configInputs { - originalConfig[k] = v - } + maps.Copy(originalConfig, configInputs) originalCLI := make(map[string]any) - for k, v := range cliInputs { - originalCLI[k] = v - } + maps.Copy(originalCLI, cliInputs) // Perform merge result := application.MergeInputs(configInputs, cliInputs) diff --git a/internal/interfaces/tui/tab_monitoring.go b/internal/interfaces/tui/tab_monitoring.go index 83f21886..90980737 100644 --- a/internal/interfaces/tui/tab_monitoring.go +++ b/internal/interfaces/tui/tab_monitoring.go @@ -322,7 +322,7 @@ func (t *MonitoringTab) SetExecCtx(ctx *workflow.ExecutionContext, wf *workflow. t.execCtx = ctx t.wf = wf if wf != nil { - t.steps = orderedSteps(wf) + t.steps = workflow.ExecutionOrder(wf) } } @@ -823,42 +823,3 @@ func treesFromFlat(flat []*TreeNode) []*TreeNode { } return roots } - -// orderedSteps returns steps in execution order by walking the workflow graph -// from Initial, following OnSuccess / default transitions. -func orderedSteps(wf *workflow.Workflow) []workflow.Step { - if wf == nil || len(wf.Steps) == 0 || wf.Initial == "" { - return nil - } - - visited := make(map[string]bool, len(wf.Steps)) - steps := make([]workflow.Step, 0, len(wf.Steps)) - current := wf.Initial - - for current != "" && !visited[current] { - step, ok := wf.Steps[current] - if !ok { - break - } - visited[current] = true - steps = append(steps, *step) - - if step.Type == workflow.StepTypeTerminal { - break - } - current = nextStepName(step) - } - - return steps -} - -// nextStepName resolves the default next step for graph traversal. -// Checks Transitions for a default fallback first, then falls back to OnSuccess. -func nextStepName(step *workflow.Step) string { - for _, tr := range step.Transitions { - if tr.When == "" { - return tr.Goto - } - } - return step.OnSuccess -} diff --git a/internal/interfaces/tui/tab_monitoring_test.go b/internal/interfaces/tui/tab_monitoring_test.go index cefda0bd..bb97bd14 100644 --- a/internal/interfaces/tui/tab_monitoring_test.go +++ b/internal/interfaces/tui/tab_monitoring_test.go @@ -631,86 +631,6 @@ func TestFlattenTree_NilInput_ReturnsEmpty(t *testing.T) { assert.Empty(t, flat) } -// --- orderedSteps --- - -func TestOrderedSteps_NilWorkflow_ReturnsNil(t *testing.T) { - steps := orderedSteps(nil) - assert.Nil(t, steps) -} - -func TestOrderedSteps_EmptyWorkflow_ReturnsNil(t *testing.T) { - wf := &workflow.Workflow{ - Name: "empty", - Steps: map[string]*workflow.Step{}, - } - steps := orderedSteps(wf) - assert.Nil(t, steps) -} - -func TestOrderedSteps_FollowsExecutionOrder(t *testing.T) { - wf := &workflow.Workflow{ - Name: "wf", - Initial: "build", - Steps: map[string]*workflow.Step{ - "build": {Name: "build", Type: workflow.StepTypeCommand, OnSuccess: "test"}, - "test": {Name: "test", Type: workflow.StepTypeCommand, OnSuccess: "deploy"}, - "deploy": {Name: "deploy", Type: workflow.StepTypeCommand, OnSuccess: "done"}, - "done": {Name: "done", Type: workflow.StepTypeTerminal}, - }, - } - - steps := orderedSteps(wf) - - require.Len(t, steps, 4) - assert.Equal(t, "build", steps[0].Name) - assert.Equal(t, "test", steps[1].Name) - assert.Equal(t, "deploy", steps[2].Name) - assert.Equal(t, "done", steps[3].Name) -} - -func TestOrderedSteps_UsesTransitionsDefaultFallback(t *testing.T) { - wf := &workflow.Workflow{ - Name: "wf", - Initial: "validate", - Steps: map[string]*workflow.Step{ - "validate": { - Name: "validate", Type: workflow.StepTypeCommand, - Transitions: workflow.Transitions{ - {When: "exit_code == 1", Goto: "fail"}, - {When: "", Goto: "deploy"}, - }, - }, - "deploy": {Name: "deploy", Type: workflow.StepTypeCommand, OnSuccess: "done"}, - "fail": {Name: "fail", Type: workflow.StepTypeTerminal, Status: workflow.TerminalFailure}, - "done": {Name: "done", Type: workflow.StepTypeTerminal}, - }, - } - - steps := orderedSteps(wf) - - require.Len(t, steps, 3) - assert.Equal(t, "validate", steps[0].Name) - assert.Equal(t, "deploy", steps[1].Name) - assert.Equal(t, "done", steps[2].Name) -} - -func TestOrderedSteps_StopsOnCycle(t *testing.T) { - wf := &workflow.Workflow{ - Name: "wf", - Initial: "a", - Steps: map[string]*workflow.Step{ - "a": {Name: "a", Type: workflow.StepTypeCommand, OnSuccess: "b"}, - "b": {Name: "b", Type: workflow.StepTypeCommand, OnSuccess: "a"}, - }, - } - - steps := orderedSteps(wf) - - require.Len(t, steps, 2) - assert.Equal(t, "a", steps[0].Name) - assert.Equal(t, "b", steps[1].Name) -} - // --- panelWidths --- func TestMonitoringTab_PanelWidths_40_60Split(t *testing.T) { diff --git a/tests/integration/features/execution_order_determinism_test.go b/tests/integration/features/execution_order_determinism_test.go new file mode 100644 index 00000000..7c6a313f --- /dev/null +++ b/tests/integration/features/execution_order_determinism_test.go @@ -0,0 +1,529 @@ +//go:build integration + +package features_test + +import ( + "regexp" + "strings" + "testing" + "time" + + "github.com/awf-project/cli/internal/domain/workflow" + "github.com/awf-project/cli/internal/interfaces/cli/ui" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Feature: F095 — Execution Order — Common Abstraction for Step Display Ordering + +// TestExecutionOrderDeterminism_LinearWorkflow validates that CLI output displays +// steps in workflow-defined order (matching Initial -> Transitions) consistently. +// Tests US1: Deterministic Step Order in CLI Execution Summary. +func TestExecutionOrderDeterminism_LinearWorkflow(t *testing.T) { + wf := &workflow.Workflow{ + Name: "linear-pipeline", + Initial: "init", + Steps: map[string]*workflow.Step{ + "init": { + Name: "init", + Type: workflow.StepTypeCommand, + Command: "echo init", + OnSuccess: "build", + }, + "build": { + Name: "build", + Type: workflow.StepTypeCommand, + Command: "echo build", + OnSuccess: "test", + }, + "test": { + Name: "test", + Type: workflow.StepTypeCommand, + Command: "echo test", + OnSuccess: "deploy", + }, + "deploy": { + Name: "deploy", + Type: workflow.StepTypeCommand, + Command: "echo deploy", + OnSuccess: "done", + }, + "done": { + Name: "done", + Type: workflow.StepTypeTerminal, + Status: workflow.TerminalSuccess, + }, + }, + } + + execCtx := workflow.NewExecutionContext("test-linear-001", "linear-pipeline") + execCtx.Status = workflow.StatusCompleted + + execCtx.States["init"] = workflow.StepState{ + Name: "init", + Status: workflow.StatusCompleted, + StartedAt: time.Now(), + CompletedAt: time.Now().Add(100 * time.Millisecond), + } + execCtx.States["build"] = workflow.StepState{ + Name: "build", + Status: workflow.StatusCompleted, + StartedAt: time.Now(), + CompletedAt: time.Now().Add(200 * time.Millisecond), + } + execCtx.States["test"] = workflow.StepState{ + Name: "test", + Status: workflow.StatusCompleted, + StartedAt: time.Now(), + CompletedAt: time.Now().Add(150 * time.Millisecond), + } + execCtx.States["deploy"] = workflow.StepState{ + Name: "deploy", + Status: workflow.StatusCompleted, + StartedAt: time.Now(), + CompletedAt: time.Now().Add(120 * time.Millisecond), + } + execCtx.States["done"] = workflow.StepState{ + Name: "done", + Status: workflow.StatusCompleted, + StartedAt: time.Now(), + CompletedAt: time.Now().Add(50 * time.Millisecond), + } + + order := workflow.ExecutionOrder(wf) + + require.NotNil(t, order) + require.Len(t, order, 5) + + assert.Equal(t, "init", order[0].Name) + assert.Equal(t, "build", order[1].Name) + assert.Equal(t, "test", order[2].Name) + assert.Equal(t, "deploy", order[3].Name) + assert.Equal(t, "done", order[4].Name) +} + +// TestExecutionOrderDeterminism_FollowsDefaultTransition validates that the +// execution order follows default transitions (empty When) over conditional ones. +// Tests US2: Domain-Level Execution Order Function. +func TestExecutionOrderDeterminism_FollowsDefaultTransition(t *testing.T) { + wf := &workflow.Workflow{ + Name: "branching-workflow", + Initial: "start", + Steps: map[string]*workflow.Step{ + "start": { + Name: "start", + Type: workflow.StepTypeCommand, + Command: "echo start", + OnSuccess: "fallback", + Transitions: workflow.Transitions{ + {When: "", Goto: "default_path"}, + {When: "condition1", Goto: "alternate_path"}, + }, + }, + "default_path": { + Name: "default_path", + Type: workflow.StepTypeCommand, + Command: "echo default", + OnSuccess: "done", + }, + "alternate_path": { + Name: "alternate_path", + Type: workflow.StepTypeCommand, + Command: "echo alternate", + OnSuccess: "done", + }, + "done": { + Name: "done", + Type: workflow.StepTypeTerminal, + Status: workflow.TerminalSuccess, + }, + }, + } + + order := workflow.ExecutionOrder(wf) + + require.NotNil(t, order) + require.Len(t, order, 3) + + assert.Equal(t, "start", order[0].Name) + assert.Equal(t, "default_path", order[1].Name, "should follow default transition, not OnSuccess fallback") + assert.Equal(t, "done", order[2].Name) +} + +// TestExecutionOrderDeterminism_SkipsStepsNotExecuted validates that steps in +// the execution order but not in ExecutionContext are gracefully skipped. +// Tests edge case: steps not executed during run. +func TestExecutionOrderDeterminism_SkipsStepsNotExecuted(t *testing.T) { + wf := &workflow.Workflow{ + Name: "partial-execution", + Initial: "step_a", + Steps: map[string]*workflow.Step{ + "step_a": { + Name: "step_a", + Type: workflow.StepTypeCommand, + Command: "echo a", + OnSuccess: "step_b", + }, + "step_b": { + Name: "step_b", + Type: workflow.StepTypeCommand, + Command: "echo b", + OnSuccess: "step_c", + }, + "step_c": { + Name: "step_c", + Type: workflow.StepTypeTerminal, + Status: workflow.TerminalSuccess, + }, + }, + } + + execCtx := workflow.NewExecutionContext("test-partial-001", "partial-execution") + execCtx.Status = workflow.StatusCompleted + + execCtx.States["step_a"] = workflow.StepState{ + Name: "step_a", + Status: workflow.StatusCompleted, + } + + order := workflow.ExecutionOrder(wf) + require.NotNil(t, order) + require.Len(t, order, 3) + + executed := 0 + for _, step := range order { + if _, ok := execCtx.GetStepState(step.Name); ok { + executed++ + } + } + + assert.Equal(t, 1, executed, "only step_a should have state") +} + +// TestExecutionOrderDeterminism_IncludesParallelStepAsSingleEntry validates that +// parallel steps are included as single entries (branches not expanded). +// Tests US1 edge case: parallel step handling. +func TestExecutionOrderDeterminism_IncludesParallelStepAsSingleEntry(t *testing.T) { + wf := &workflow.Workflow{ + Name: "parallel-workflow", + Initial: "setup", + Steps: map[string]*workflow.Step{ + "setup": { + Name: "setup", + Type: workflow.StepTypeCommand, + Command: "echo setup", + OnSuccess: "parallel_fetch", + }, + "parallel_fetch": { + Name: "parallel_fetch", + Type: workflow.StepTypeParallel, + Branches: []string{"fetch_data1", "fetch_data2", "fetch_data3"}, + OnSuccess: "aggregate", + }, + "fetch_data1": { + Name: "fetch_data1", + Type: workflow.StepTypeCommand, + Command: "echo fetch1", + OnSuccess: "aggregate", + }, + "fetch_data2": { + Name: "fetch_data2", + Type: workflow.StepTypeCommand, + Command: "echo fetch2", + OnSuccess: "aggregate", + }, + "fetch_data3": { + Name: "fetch_data3", + Type: workflow.StepTypeCommand, + Command: "echo fetch3", + OnSuccess: "aggregate", + }, + "aggregate": { + Name: "aggregate", + Type: workflow.StepTypeCommand, + Command: "echo aggregate", + OnSuccess: "done", + }, + "done": { + Name: "done", + Type: workflow.StepTypeTerminal, + Status: workflow.TerminalSuccess, + }, + }, + } + + order := workflow.ExecutionOrder(wf) + + require.NotNil(t, order) + require.Len(t, order, 4, "should be setup, parallel_fetch, aggregate, done (not expanded branches)") + + assert.Equal(t, "setup", order[0].Name) + assert.Equal(t, "parallel_fetch", order[1].Name) + assert.Equal(t, workflow.StepTypeParallel, order[1].Type, "parallel step should retain its type") + assert.Equal(t, "aggregate", order[2].Name) + assert.Equal(t, "done", order[3].Name) +} + +// TestExecutionOrderDeterminism_ConsistentAcrossMultipleCalls validates that +// the same workflow produces identical execution order across multiple calls. +// Tests SC-001: Deterministic ordering across runs. +func TestExecutionOrderDeterminism_ConsistentAcrossMultipleCalls(t *testing.T) { + wf := &workflow.Workflow{ + Name: "consistent-workflow", + Initial: "step1", + Steps: map[string]*workflow.Step{ + "step1": { + Name: "step1", + Type: workflow.StepTypeCommand, + Command: "echo 1", + OnSuccess: "step2", + }, + "step2": { + Name: "step2", + Type: workflow.StepTypeCommand, + Command: "echo 2", + OnSuccess: "step3", + }, + "step3": { + Name: "step3", + Type: workflow.StepTypeCommand, + Command: "echo 3", + OnSuccess: "step4", + }, + "step4": { + Name: "step4", + Type: workflow.StepTypeCommand, + Command: "echo 4", + OnSuccess: "step5", + }, + "step5": { + Name: "step5", + Type: workflow.StepTypeTerminal, + Status: workflow.TerminalSuccess, + }, + }, + } + + order1 := workflow.ExecutionOrder(wf) + order2 := workflow.ExecutionOrder(wf) + order3 := workflow.ExecutionOrder(wf) + + require.NotNil(t, order1) + require.NotNil(t, order2) + require.NotNil(t, order3) + + assert.Len(t, order1, 5) + assert.Len(t, order2, 5) + assert.Len(t, order3, 5) + + for i := 0; i < 5; i++ { + assert.Equal(t, order1[i].Name, order2[i].Name, "call 1 and 2 should match at index %d", i) + assert.Equal(t, order1[i].Name, order3[i].Name, "call 1 and 3 should match at index %d", i) + } +} + +// TestExecutionOrderDeterminism_StopsAtCycleDetection validates that execution +// order stops when encountering a revisited step to prevent infinite loops. +// Tests edge case: cycle prevention. +func TestExecutionOrderDeterminism_StopsAtCycleDetection(t *testing.T) { + wf := &workflow.Workflow{ + Name: "cyclic-workflow", + Initial: "a", + Steps: map[string]*workflow.Step{ + "a": { + Name: "a", + Type: workflow.StepTypeCommand, + Command: "echo a", + OnSuccess: "b", + }, + "b": { + Name: "b", + Type: workflow.StepTypeCommand, + Command: "echo b", + OnSuccess: "a", + }, + }, + } + + order := workflow.ExecutionOrder(wf) + + require.NotNil(t, order) + require.Len(t, order, 2, "should stop at revisited step without infinite loop") + + assert.Equal(t, "a", order[0].Name) + assert.Equal(t, "b", order[1].Name) +} + +// TestNextDefaultStep_ResolvesDefaultTransition validates that NextDefaultStep +// correctly identifies and returns the default transition. +// Tests US2: NextDefaultStep function behavior. +func TestNextDefaultStep_ResolvesDefaultTransition(t *testing.T) { + tests := []struct { + name string + step *workflow.Step + expected string + }{ + { + name: "default transition takes precedence over on_success", + step: &workflow.Step{ + Name: "test", + OnSuccess: "fallback", + Transitions: workflow.Transitions{ + {When: "", Goto: "default_target"}, + {When: "condition", Goto: "conditional_target"}, + }, + }, + expected: "default_target", + }, + { + name: "falls back to on_success when no default transition", + step: &workflow.Step{ + Name: "test", + OnSuccess: "success_target", + Transitions: workflow.Transitions{ + {When: "condition1", Goto: "path1"}, + {When: "condition2", Goto: "path2"}, + }, + }, + expected: "success_target", + }, + { + name: "returns empty when no default path", + step: &workflow.Step{ + Name: "test", + Transitions: workflow.Transitions{ + {When: "condition", Goto: "conditional_only"}, + }, + }, + expected: "", + }, + { + name: "handles nil step gracefully", + step: nil, + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := workflow.NextDefaultStep(tt.step) + assert.Equal(t, tt.expected, result) + }) + } +} + +// TestExecutionOrderDeterminism_CliDisplayFormatting validates that the CLI +// display functions format step information in workflow-defined order. +// Integration test for showExecutionDetails functionality. +func TestExecutionOrderDeterminism_CliDisplayFormatting(t *testing.T) { + wf := &workflow.Workflow{ + Name: "display-test", + Initial: "fetch", + Steps: map[string]*workflow.Step{ + "fetch": { + Name: "fetch", + Type: workflow.StepTypeCommand, + Command: "echo fetch", + OnSuccess: "process", + }, + "process": { + Name: "process", + Type: workflow.StepTypeCommand, + Command: "echo process", + OnSuccess: "store", + }, + "store": { + Name: "store", + Type: workflow.StepTypeTerminal, + Status: workflow.TerminalSuccess, + }, + }, + } + + execCtx := workflow.NewExecutionContext("test-display-001", "display-test") + execCtx.Status = workflow.StatusCompleted + + now := time.Now() + execCtx.States["fetch"] = workflow.StepState{ + Name: "fetch", + Status: workflow.StatusCompleted, + StartedAt: now, + CompletedAt: now.Add(100 * time.Millisecond), + } + execCtx.States["process"] = workflow.StepState{ + Name: "process", + Status: workflow.StatusCompleted, + StartedAt: now, + CompletedAt: now.Add(150 * time.Millisecond), + } + execCtx.States["store"] = workflow.StepState{ + Name: "store", + Status: workflow.StatusCompleted, + StartedAt: now, + CompletedAt: now.Add(50 * time.Millisecond), + } + + order := workflow.ExecutionOrder(wf) + + require.NotNil(t, order) + require.Len(t, order, 3) + + output := new(strings.Builder) + formatter := ui.NewFormatter(output, ui.FormatOptions{NoColor: true}) + + formatter.Printf("--- Execution Details ---\n") + for _, step := range order { + state, ok := execCtx.GetStepState(step.Name) + if !ok { + continue + } + duration := state.CompletedAt.Sub(state.StartedAt) + formatter.Printf("%s: %s (%v)\n", step.Name, state.Status, duration) + } + + outputStr := output.String() + + assert.Contains(t, outputStr, "fetch:", "fetch should appear in output") + assert.Contains(t, outputStr, "process:", "process should appear in output") + assert.Contains(t, outputStr, "store:", "store should appear in output") + + fetchIdx := findStringIndex(outputStr, "fetch:") + processIdx := findStringIndex(outputStr, "process:") + storeIdx := findStringIndex(outputStr, "store:") + + assert.Less(t, fetchIdx, processIdx, "fetch should appear before process") + assert.Less(t, processIdx, storeIdx, "process should appear before store") +} + +// TestExecutionOrderDeterminism_MissingStepHandling validates that execution +// stops gracefully when a transition target is missing from the workflow. +func TestExecutionOrderDeterminism_MissingStepHandling(t *testing.T) { + wf := &workflow.Workflow{ + Name: "missing-target", + Initial: "start", + Steps: map[string]*workflow.Step{ + "start": { + Name: "start", + Type: workflow.StepTypeCommand, + Command: "echo start", + OnSuccess: "missing", + }, + }, + } + + order := workflow.ExecutionOrder(wf) + + require.NotNil(t, order) + require.Len(t, order, 1, "should include start but stop when target is missing") + assert.Equal(t, "start", order[0].Name) +} + +// Helper function to find the index of a substring. +func findStringIndex(s, substr string) int { + re := regexp.MustCompile(regexp.QuoteMeta(substr)) + match := re.FindStringIndex(s) + if match == nil { + return -1 + } + return match[0] +}