diff --git a/pkg/cli/README.md b/pkg/cli/README.md index 777d1f42fdd..ca08ed2ee77 100644 --- a/pkg/cli/README.md +++ b/pkg/cli/README.md @@ -89,7 +89,7 @@ All diagnostic output MUST go to `stderr` using `console` formatting helpers. St | `DependencyReport` | `deps_report.go` | Full dependency report | | `OutdatedDependency` | `deps_outdated.go` | An outdated dependency entry | | `SecurityAdvisory` | `deps_security.go` | A security advisory entry | -| `WorkflowStatus` | `status_command.go` | Run status for a single workflow | +| `WorkflowStatus` | `status_command.go` | Run status for a single workflow; embeds `WorkflowListItem` | | `MCPRegistryClient` | `mcp_registry.go` | Client for the MCP registry API | | `ToolGraph` | `tool_graph.go` | Dependency graph of MCP tools | | `DependencyGraph` | `dependency_graph.go` | Dependency graph across workflows | @@ -394,7 +394,7 @@ The `cli` package exports many types used across its command implementations. Th | `WorkflowFailure` | struct | A workflow failure record | | `WorkflowFileStatus` | struct | Status of a workflow file (exists, outdated, etc.) | | `WorkflowJob` | struct | A GitHub Actions job within a workflow run | -| `WorkflowListItem` | struct | A single item in the `gh aw list` output | +| `WorkflowListItem` | struct | A single item in `gh aw list`; shared workflow metadata fields (name, engine, compiled status, labels, triggers) also embedded in `WorkflowStatus` | | `WorkflowMCPMetadata` | struct | MCP server metadata scanned from a workflow file | | `WorkflowNode` | struct | A node in the workflow dependency graph | | `WorkflowOption` | struct | A selectable workflow option for interactive prompts | diff --git a/pkg/cli/add_interactive_workflow.go b/pkg/cli/add_interactive_workflow.go index 070c37d28db..1828d5826b8 100644 --- a/pkg/cli/add_interactive_workflow.go +++ b/pkg/cli/add_interactive_workflow.go @@ -202,7 +202,7 @@ func findWorkflowsByFilenamePattern(pattern, repoOverride string, verbose bool) if verbose { fmt.Fprintf(os.Stderr, "Workflow with filename '%s' found in workflow list\n", pattern) } - return []WorkflowStatus{{Workflow: pattern}}, nil + return []WorkflowStatus{{WorkflowListItem: WorkflowListItem{Workflow: pattern}}}, nil } if verbose { diff --git a/pkg/cli/mcp_schema_test.go b/pkg/cli/mcp_schema_test.go index da95d855ce5..837498344c2 100644 --- a/pkg/cli/mcp_schema_test.go +++ b/pkg/cli/mcp_schema_test.go @@ -489,13 +489,15 @@ func TestGeneratedSchemasValidateRealOutput(t *testing.T) { // Create realistic test data data := WorkflowStatus{ - Workflow: "status-workflow", - EngineID: "copilot", - Compiled: "true", + WorkflowListItem: WorkflowListItem{ + Workflow: "status-workflow", + EngineID: "copilot", + Compiled: "true", + Labels: []string{"production", "automated"}, + On: "push", + }, Status: "active", TimeRemaining: "5m30s", - Labels: []string{"production", "automated"}, - On: "push", RunStatus: "completed", RunConclusion: "success", } diff --git a/pkg/cli/status_command.go b/pkg/cli/status_command.go index 21bcc7ae33a..0f81772570a 100644 --- a/pkg/cli/status_command.go +++ b/pkg/cli/status_command.go @@ -23,16 +23,14 @@ import ( var statusLog = logger.New("cli:status_command") -// WorkflowStatus represents the status of a single workflow for JSON output +// WorkflowStatus represents the status of a single workflow for JSON output. +// It embeds WorkflowListItem so that both list and status commands share the +// same source of truth for the common workflow metadata fields. type WorkflowStatus struct { - Workflow string `json:"workflow" console:"header:Workflow"` - EngineID string `json:"engine_id" console:"header:Engine"` - Compiled string `json:"compiled" console:"header:Compiled"` + WorkflowListItem Status string `json:"status" console:"header:Status"` TimeRemaining string `json:"time_remaining" console:"header:Time Remaining"` - Labels []string `json:"labels,omitempty" console:"header:Labels,omitempty"` Dependencies []string `json:"dependencies,omitempty" console:"-"` - On any `json:"on,omitempty" console:"-"` RunStatus string `json:"run_status,omitempty" console:"header:Run Status,omitempty"` RunConclusion string `json:"run_conclusion,omitempty" console:"header:Run Conclusion,omitempty"` } @@ -178,14 +176,16 @@ func GetWorkflowStatuses(pattern string, ref string, labelFilter string, repoOve // Build status object statuses = append(statuses, WorkflowStatus{ - Workflow: name, - EngineID: agent, - Compiled: compiled, + WorkflowListItem: WorkflowListItem{ + Workflow: name, + EngineID: agent, + Compiled: compiled, + Labels: labels, + On: onField, + }, Status: status, TimeRemaining: timeRemaining, - Labels: labels, Dependencies: dependencies, - On: onField, RunStatus: runStatus, RunConclusion: runConclusion, }) @@ -219,7 +219,11 @@ func buildRemoteWorkflowStatuses(pattern string, githubWorkflows map[string]*Git } statuses = append(statuses, WorkflowStatus{ - Workflow: name, + // Remote workflow status only includes the workflow name here; the + // GitHub Actions API response does not provide list metadata fields. + WorkflowListItem: WorkflowListItem{ + Workflow: name, + }, Status: status, RunStatus: runStatus, RunConclusion: runConclusion, diff --git a/pkg/cli/status_command_test.go b/pkg/cli/status_command_test.go index 770bdd3b810..2f55524db08 100644 --- a/pkg/cli/status_command_test.go +++ b/pkg/cli/status_command_test.go @@ -51,14 +51,16 @@ func TestStatusWorkflows_JSONOutput(t *testing.T) { func TestWorkflowStatus_JSONMarshaling(t *testing.T) { // Test that WorkflowStatus can be marshaled to JSON status := WorkflowStatus{ - Workflow: "test-workflow", - EngineID: "copilot", - Compiled: "Yes", + WorkflowListItem: WorkflowListItem{ + Workflow: "test-workflow", + EngineID: "copilot", + Compiled: "Yes", + On: map[string]any{ + "workflow_dispatch": nil, + }, + }, Status: "active", TimeRemaining: "N/A", - On: map[string]any{ - "workflow_dispatch": nil, - }, } jsonBytes, err := json.Marshal(status) @@ -297,16 +299,20 @@ func TestWorkflowStatus_ConsoleRendering(t *testing.T) { // Create test data statuses := []WorkflowStatus{ { - Workflow: "test-workflow-1", - EngineID: "copilot", - Compiled: "Yes", + WorkflowListItem: WorkflowListItem{ + Workflow: "test-workflow-1", + EngineID: "copilot", + Compiled: "Yes", + }, Status: "active", TimeRemaining: "N/A", }, { - Workflow: "test-workflow-2", - EngineID: "claude", - Compiled: "No", + WorkflowListItem: WorkflowListItem{ + Workflow: "test-workflow-2", + EngineID: "claude", + Compiled: "No", + }, Status: "disabled", TimeRemaining: "2h 30m", }, @@ -344,9 +350,11 @@ func TestWorkflowStatus_ConsoleRendering(t *testing.T) { func TestWorkflowStatus_JSONMarshalingWithRunStatus(t *testing.T) { // Test that WorkflowStatus with run status can be marshaled to JSON status := WorkflowStatus{ - Workflow: "test-workflow", - EngineID: "copilot", - Compiled: "Yes", + WorkflowListItem: WorkflowListItem{ + Workflow: "test-workflow", + EngineID: "copilot", + Compiled: "Yes", + }, Status: "active", TimeRemaining: "N/A", RunStatus: "completed", @@ -376,9 +384,11 @@ func TestWorkflowStatus_JSONMarshalingWithRunStatus(t *testing.T) { func TestWorkflowStatus_JSONMarshalingWithEmptyRunStatus(t *testing.T) { // Test that WorkflowStatus without run status omits those fields status := WorkflowStatus{ - Workflow: "test-workflow", - EngineID: "copilot", - Compiled: "Yes", + WorkflowListItem: WorkflowListItem{ + Workflow: "test-workflow", + EngineID: "copilot", + Compiled: "Yes", + }, Status: "active", TimeRemaining: "N/A", // RunStatus and RunConclusion are empty @@ -407,12 +417,14 @@ func TestWorkflowStatus_JSONMarshalingWithEmptyRunStatus(t *testing.T) { func TestWorkflowStatus_JSONMarshalingWithLabels(t *testing.T) { // Test that WorkflowStatus with labels can be marshaled to JSON status := WorkflowStatus{ - Workflow: "test-workflow", - EngineID: "copilot", - Compiled: "Yes", + WorkflowListItem: WorkflowListItem{ + Workflow: "test-workflow", + EngineID: "copilot", + Compiled: "Yes", + Labels: []string{"automation", "testing"}, + }, Status: "active", TimeRemaining: "N/A", - Labels: []string{"automation", "testing"}, } jsonBytes, err := json.Marshal(status) @@ -447,12 +459,14 @@ func TestWorkflowStatus_JSONMarshalingWithLabels(t *testing.T) { func TestWorkflowStatus_JSONMarshalingWithEmptyLabels(t *testing.T) { // Test that WorkflowStatus without labels omits the field status := WorkflowStatus{ - Workflow: "test-workflow", - EngineID: "copilot", - Compiled: "Yes", + WorkflowListItem: WorkflowListItem{ + Workflow: "test-workflow", + EngineID: "copilot", + Compiled: "Yes", + // Labels is empty/nil + }, Status: "active", TimeRemaining: "N/A", - // Labels is empty/nil } jsonBytes, err := json.Marshal(status) @@ -476,18 +490,22 @@ func TestWorkflowStatus_ConsoleRenderingWithRunStatus(t *testing.T) { // Create test data with run status statuses := []WorkflowStatus{ { - Workflow: "test-workflow-1", - EngineID: "copilot", - Compiled: "Yes", + WorkflowListItem: WorkflowListItem{ + Workflow: "test-workflow-1", + EngineID: "copilot", + Compiled: "Yes", + }, Status: "active", TimeRemaining: "N/A", RunStatus: "completed", RunConclusion: "success", }, { - Workflow: "test-workflow-2", - EngineID: "claude", - Compiled: "No", + WorkflowListItem: WorkflowListItem{ + Workflow: "test-workflow-2", + EngineID: "claude", + Compiled: "No", + }, Status: "disabled", TimeRemaining: "2h 30m", RunStatus: "completed", diff --git a/pkg/cli/status_dependency_tree_test.go b/pkg/cli/status_dependency_tree_test.go index 0a475e6922a..dfc35e1495d 100644 --- a/pkg/cli/status_dependency_tree_test.go +++ b/pkg/cli/status_dependency_tree_test.go @@ -52,8 +52,8 @@ func TestExtractWorkflowDependencies_ImportsObjectAW(t *testing.T) { func TestRenderWorkflowDependencyTree(t *testing.T) { statuses := []WorkflowStatus{ { - Workflow: "main-workflow", - Dependencies: []string{"shared/base.md", "local/helpers.md"}, + WorkflowListItem: WorkflowListItem{Workflow: "main-workflow"}, + Dependencies: []string{"shared/base.md", "local/helpers.md"}, }, } @@ -65,6 +65,6 @@ func TestRenderWorkflowDependencyTree(t *testing.T) { } func TestRenderWorkflowDependencyTree_Empty(t *testing.T) { - statuses := []WorkflowStatus{{Workflow: "standalone"}} + statuses := []WorkflowStatus{{WorkflowListItem: WorkflowListItem{Workflow: "standalone"}}} assert.Empty(t, renderWorkflowDependencyTree(statuses), "dependency tree should be empty when no dependencies exist") } diff --git a/pkg/console/render.go b/pkg/console/render.go index 983f8ce9ed7..29eb15b6eeb 100644 --- a/pkg/console/render.go +++ b/pkg/console/render.go @@ -68,46 +68,42 @@ func renderStruct(val reflect.Value, title string, output *strings.Builder, dept } } - maxFieldLen := computeMaxFieldLen(val, typ) + maxFieldLen := computeMaxFieldLen(val) + renderInlineEmbeddedFields(val, maxFieldLen, output, depth) - // Iterate through struct fields - for i := range val.NumField() { - field := val.Field(i) - fieldType := typ.Field(i) + output.WriteString("\n") +} - // Check if field should be skipped +// renderInlineEmbeddedFields renders the fields of an anonymous embedded struct +// directly into the parent struct output, flattening the hierarchy. +func renderInlineEmbeddedFields(val reflect.Value, maxFieldLen int, output *strings.Builder, depth int) { + walkInlineFields(val, func(field reflect.Value, fieldType reflect.StructField) { tag := parseConsoleTag(fieldType.Tag.Get("console")) if tag.skip { - continue + return } - - // Check omitempty if tag.omitempty && isZeroValue(field) { - continue + return } - // Get field name (use tag header if available, otherwise use field name) fieldName := fieldType.Name if tag.header != "" { fieldName = tag.header } renderStructField(field, fieldName, tag, maxFieldLen, output, depth) - } - - output.WriteString("\n") + }) } -// computeMaxFieldLen computes the longest visible field name for alignment. -func computeMaxFieldLen(val reflect.Value, typ reflect.Type) int { +// computeMaxFieldLen computes the longest visible field name for alignment, +// recursing into anonymous embedded structs to include their fields. +func computeMaxFieldLen(val reflect.Value) int { maxFieldLen := 0 - for i := range val.NumField() { - field := val.Field(i) - fieldType := typ.Field(i) + walkInlineFields(val, func(field reflect.Value, fieldType reflect.StructField) { tag := parseConsoleTag(fieldType.Tag.Get("console")) if tag.skip || (tag.omitempty && isZeroValue(field)) { - continue + return } fieldName := fieldType.Name @@ -118,10 +114,38 @@ func computeMaxFieldLen(val reflect.Value, typ reflect.Type) int { if len(fieldName) > maxFieldLen { maxFieldLen = len(fieldName) } - } + }) return maxFieldLen } +func walkInlineFields(val reflect.Value, visit func(field reflect.Value, fieldType reflect.StructField)) { + typ := val.Type() + for i := range val.NumField() { + field := val.Field(i) + fieldType := typ.Field(i) + + if fieldType.Anonymous { + if embedded, ok := embeddedStructValue(field); ok { + walkInlineFields(embedded, visit) + continue + } + } + + visit(field, fieldType) + } +} + +func embeddedStructValue(field reflect.Value) (reflect.Value, bool) { + for field.Kind() == reflect.Pointer { + if field.IsNil() { + return reflect.Value{}, false + } + field = field.Elem() + } + + return field, field.Kind() == reflect.Struct +} + // renderStructField renders a single struct field to output, dispatching on its kind. func renderStructField(field reflect.Value, fieldName string, tag consoleTag, maxFieldLen int, output *strings.Builder, depth int) { // Dereference pointer to check underlying type @@ -231,41 +255,69 @@ func buildTableConfig(val reflect.Value, title string) TableConfig { } // Build headers from struct fields - headers, fieldIndices, fieldTags := buildTableHeaders(elemType) + headers, fieldPaths, fieldTags := buildTableHeaders(elemType) config.Headers = headers // Build rows - config.Rows = buildTableRows(val, fieldIndices, fieldTags) + config.Rows = buildTableRows(val, fieldPaths, fieldTags) return config } -// buildTableHeaders extracts column headers, field indices, and tags from a struct type. -func buildTableHeaders(elemType reflect.Type) (headers []string, fieldIndices []int, fieldTags []consoleTag) { - for i := range elemType.NumField() { - field := elemType.Field(i) +// buildTableHeaders extracts column headers, field index paths, and tags from a struct type, +// flattening anonymous embedded struct fields into the top-level column list. +func buildTableHeaders(elemType reflect.Type) (headers []string, fieldPaths [][]int, fieldTags []consoleTag) { + fields := collectTableFields(elemType, nil) + headers = make([]string, 0, len(fields)) + fieldPaths = make([][]int, 0, len(fields)) + fieldTags = make([]consoleTag, 0, len(fields)) + for _, field := range fields { + headers = append(headers, field.header) + fieldPaths = append(fieldPaths, field.path) + fieldTags = append(fieldTags, field.tag) + } + return headers, fieldPaths, fieldTags +} + +// collectTableFields recursively walks a struct type, inlining the fields of any +// anonymous embedded structs so they appear as top-level table columns. +func collectTableFields(t reflect.Type, prefix []int) []tableField { + fields := make([]tableField, 0, t.NumField()) + for i := range t.NumField() { + field := t.Field(i) + fieldPath := make([]int, len(prefix)+1) + copy(fieldPath, prefix) + fieldPath[len(prefix)] = i + + if field.Anonymous { + if embeddedType, ok := embeddedStructType(field.Type); ok { + fields = append(fields, collectTableFields(embeddedType, fieldPath)...) + continue + } + } + tag := parseConsoleTag(field.Tag.Get("console")) - // Skip fields marked with "-" if tag.skip { continue } - // Use header tag if available, otherwise use field name headerName := field.Name if tag.header != "" { headerName = tag.header } - headers = append(headers, headerName) - fieldIndices = append(fieldIndices, i) - fieldTags = append(fieldTags, tag) + fields = append(fields, tableField{ + header: headerName, + path: fieldPath, + tag: tag, + }) } - return headers, fieldIndices, fieldTags + return fields } // buildTableRows builds the row data for a slice of struct elements. -func buildTableRows(val reflect.Value, fieldIndices []int, fieldTags []consoleTag) [][]string { +func buildTableRows(val reflect.Value, fieldPaths [][]int, fieldTags []consoleTag) [][]string { var rows [][]string for i := range val.Len() { elem := val.Index(i) @@ -282,8 +334,12 @@ func buildTableRows(val reflect.Value, fieldIndices []int, fieldTags []consoleTa } var row []string - for j, fieldIdx := range fieldIndices { - field := elem.Field(fieldIdx) + for j, fieldPath := range fieldPaths { + field, ok := fieldByIndexSafe(elem, fieldPath) + if !ok { + row = append(row, "") + continue + } row = append(row, formatFieldValueWithTag(field, fieldTags[j])) } rows = append(rows, row) @@ -291,6 +347,36 @@ func buildTableRows(val reflect.Value, fieldIndices []int, fieldTags []consoleTa return rows } +func fieldByIndexSafe(val reflect.Value, path []int) (reflect.Value, bool) { + current := val + for _, idx := range path { + for current.Kind() == reflect.Pointer { + if current.IsNil() { + return reflect.Value{}, false + } + current = current.Elem() + } + if current.Kind() != reflect.Struct { + return reflect.Value{}, false + } + current = current.Field(idx) + } + return current, true +} + +func embeddedStructType(t reflect.Type) (reflect.Type, bool) { + for t.Kind() == reflect.Pointer { + t = t.Elem() + } + return t, t.Kind() == reflect.Struct +} + +type tableField struct { + header string + path []int + tag consoleTag +} + // consoleTag represents parsed console struct tag type consoleTag struct { title string diff --git a/pkg/console/render_test.go b/pkg/console/render_test.go index 29de7f078b7..a77ac16c21c 100644 --- a/pkg/console/render_test.go +++ b/pkg/console/render_test.go @@ -4,9 +4,11 @@ package console import ( "reflect" + "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // Test types for struct rendering @@ -433,3 +435,154 @@ func TestRenderStruct_NilPointerToStruct(t *testing.T) { // Should not crash and should not contain inner section when nil and omitempty assert.NotContains(t, output, "Inner Section", "output should not contain 'Inner Section' when nil and omitempty") } + +// TestRenderStruct_EmbeddedStruct tests that anonymous embedded struct fields are +// inlined into the parent struct output rather than rendered as a nested section. +func TestRenderStruct_EmbeddedStruct(t *testing.T) { + type Base struct { + Name string `console:"header:Name"` + Engine string `console:"header:Engine"` + } + + type Extended struct { + Base + Status string `console:"header:Status"` + } + + data := Extended{ + Base: Base{Name: "my-workflow", Engine: "copilot"}, + Status: "active", + } + + output := RenderStruct(data) + + // Fields from the embedded struct should appear at the top level. + assert.Contains(t, output, "my-workflow", "output should contain Name from embedded struct") + assert.Contains(t, output, "copilot", "output should contain Engine from embedded struct") + assert.Contains(t, output, "active", "output should contain Status from outer struct") + // The embedded type name should NOT appear as a section title. + assert.NotContains(t, output, "Base", "output should not contain the embedded struct type name as a section") +} + +func TestRenderStruct_EmbeddedStructOmitEmptyAndPointer(t *testing.T) { + type Base struct { + Name string `console:"header:Name"` + Engine string `console:"header:Engine,omitempty"` + } + + type Extended struct { + *Base + Status string `console:"header:Status"` + } + + data := Extended{ + Base: &Base{Name: "wf"}, + Status: "active", + } + + output := RenderStruct(data) + + assert.Contains(t, output, "wf", "output should contain Name from embedded pointer struct") + assert.Contains(t, output, "active", "output should contain Status from outer struct") + assert.NotContains(t, output, "Engine", "zero omitempty field in embed should be suppressed") + assert.NotContains(t, output, "Base", "output should not contain the embedded pointer type name as a section") +} + +func TestRenderStruct_NestedEmbeddedStruct(t *testing.T) { + type Inner struct { + Name string `console:"header:Name"` + } + + type Middle struct { + Inner + Engine string `console:"header:Engine"` + } + + type Outer struct { + Middle + Status string `console:"header:Status"` + } + + output := RenderStruct(Outer{ + Middle: Middle{ + Inner: Inner{Name: "wf"}, + Engine: "copilot", + }, + Status: "ok", + }) + + assert.Contains(t, output, "wf") + assert.Contains(t, output, "copilot") + assert.Contains(t, output, "ok") + assert.NotContains(t, output, "Middle") + assert.NotContains(t, output, "Inner") +} + +// TestRenderSlice_EmbeddedStruct tests that a slice of structs with anonymous embedded +// fields is rendered as a flat table with all promoted fields as columns. +func TestRenderSlice_EmbeddedStruct(t *testing.T) { + type Base struct { + Name string `console:"header:Name"` + Engine string `console:"header:Engine"` + } + + type Extended struct { + Base + Status string `console:"header:Status"` + } + + items := []Extended{ + {Base: Base{Name: "wf-1", Engine: "copilot"}, Status: "active"}, + {Base: Base{Name: "wf-2", Engine: "claude"}, Status: "disabled"}, + } + + output := RenderStruct(items) + + // All columns (including those from the embedded struct) should appear as headers. + assert.Contains(t, output, "Name", "output should contain Name column header") + assert.Contains(t, output, "Engine", "output should contain Engine column header") + assert.Contains(t, output, "Status", "output should contain Status column header") + lines := strings.Split(output, "\n") + headerLine := "" + for _, line := range lines { + if strings.Contains(line, "Name") && strings.Contains(line, "Status") { + headerLine = line + break + } + } + require.NotEmpty(t, headerLine, "table output should include a header row") + assert.Less(t, strings.Index(headerLine, "Name"), strings.Index(headerLine, "Status"), "embedded Name column must appear before outer Status column in the header row") + + // All values should be present in the table rows. + assert.Contains(t, output, "wf-1", "output should contain first workflow name") + assert.Contains(t, output, "copilot", "output should contain first engine") + assert.Contains(t, output, "active", "output should contain first status") + assert.Contains(t, output, "wf-2", "output should contain second workflow name") + assert.Contains(t, output, "claude", "output should contain second engine") + assert.Contains(t, output, "disabled", "output should contain second status") +} + +func TestRenderSlice_EmbeddedPointerStruct(t *testing.T) { + type Base struct { + Name string `console:"header:Name"` + } + + type Extended struct { + *Base + Status string `console:"header:Status"` + } + + items := []Extended{ + {Base: &Base{Name: "wf-1"}, Status: "active"}, + {Status: "missing"}, + } + + output := RenderStruct(items) + + assert.Contains(t, output, "Name", "output should contain Name column header from embedded pointer struct") + assert.Contains(t, output, "Status", "output should contain Status column header") + assert.Contains(t, output, "wf-1", "output should contain first workflow name") + assert.Regexp(t, `(?m)^│\s*│missing│$`, output, "output should render an empty Name cell for rows with a nil embedded pointer") + assert.Contains(t, output, "missing", "output should contain second status") + assert.NotContains(t, output, "Base", "output should not contain the embedded pointer type name as a column") +}