Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ This appendix is generated from the current non-test Go source files in this pac
| `logs_github_rate_limit_usage.go` | `GitHubRateLimitUsage` | `type GitHubRateLimitUsage struct { TotalRequestsMade int `json:"total_requests_made" console:"header:Total GitHub API Calls,format:number"` CoreConsumed int `json:"core_consumed" console:"header:Core Quota Consumed,format:number"` CoreConsumedSource string `json:"core_consumed_source,omitempty" console:"-"` CoreRemaining int `json:"core_remaining" console:"header:Core Remaining,format:number"` CoreLimit int `json:"core_limit" console:"header:Core Limit,format:number"` Resources []*GitHubRateLimitResourceUsage `json:"resources,omitempty"` }` | GitHubRateLimitUsage provides an aggregated view of GitHub API quota consumed by a single workflow run. |
| `logs_models.go` | `AggregatedSummaryBase` | `type AggregatedSummaryBase struct { Count int `json:"count" console:"header:Occurrences"` Workflows []string `json:"workflows" console:"-"` // List of workflow names WorkflowsDisplay string `json:"-" console:"header:Workflows,maxlen:40"` // Formatted display of workflows FirstReason string `json:"first_reason" console:"-"` // Reason from the first occurrence FirstReasonDisplay string `json:"-" console:"header:First Reason,maxlen:50"` // Formatted display of first reason RunIDs []int64 `json:"run_ids" console:"-"` // List of run IDs }` | AggregatedSummaryBase holds the shared tail fields that appear byte-for-byte identically in MissingToolSummary and MissingDataSummary (and as a subset in MCPFailureSummary). |
| `logs_models.go` | `JobStep` | `type JobStep struct { Name string `json:"name"` Status string `json:"status,omitempty"` Conclusion string `json:"conclusion,omitempty"` }` | JobStep represents basic information about an individual workflow job step. |
| `logs_models.go` | `MCPFailureSummary` | `type MCPFailureSummary struct { ServerName string `json:"server_name" console:"header:Server"` Count int `json:"count" console:"header:Failures"` Workflows []string `json:"workflows" console:"-"` // List of workflow names that had this server fail WorkflowsDisplay string `json:"-" console:"header:Workflows,maxlen:60"` // Formatted display of workflows RunIDs []int64 `json:"run_ids" console:"-"` // List of run IDs where this server failed }` | MCPFailureSummary aggregates MCP server failure reports across runs |
| `logs_models.go` | `MCPFailureSummary` | `type MCPFailureSummary struct { ServerName string `json:"server_name" console:"header:Server"` AggregatedSummaryBase `console:"-"` }` | MCPFailureSummary aggregates MCP server failure reports across runs |
| `logs_models.go` | `ReportProvenance` | `type ReportProvenance struct { Timestamp string `json:"timestamp"` WorkflowName string `json:"workflow_name,omitempty"` // Tracks which workflow reported this RunID int64 `json:"run_id,omitempty"` // Tracks which run reported this ExperimentName string `json:"experiment_name,omitempty"` // Assigned experiment name for this run (if present) Variant string `json:"variant,omitempty"` // Assigned variant value for ExperimentName (if present) }` | ReportProvenance holds the shared provenance fields common to all report record types. |
| `logs_orchestrator_types.go` | `LogsDownloadOptions` | `type LogsDownloadOptions struct { WorkflowName string Count int StartDate string EndDate string OutputDir string Engine string Ref string BeforeRunID int64 AfterRunID int64 RepoOverride string Verbose bool ToolGraph bool NoStaged bool FirewallOnly bool NoFirewall bool Parse bool JSONOutput bool TimeoutMinutes int SummaryFile string SafeOutputType string FilteredIntegrity bool EvalsOnly bool Train bool Format string ArtifactSets []string After string ReportFile string }` | LogsDownloadOptions holds parameters for DownloadWorkflowLogs. |
| `logs_orchestrator_types.go` | `StdinLogsOptions` | `type StdinLogsOptions struct { RunURLs []string OutputDir string Engine string RepoOverride string Verbose bool ToolGraph bool NoStaged bool FirewallOnly bool NoFirewall bool Parse bool JSONOutput bool Timeout int SummaryFile string SafeOutputType string FilteredIntegrity bool EvalsOnly bool Train bool Format string ReportFile string // ArtifactSets defaults to nil (download all artifacts) when this API is used // programmatically. The CLI passes ["usage"] to match the logs command default. ArtifactSets []string }` | StdinLogsOptions holds parameters for DownloadWorkflowLogsFromStdin. |
Expand Down
52 changes: 44 additions & 8 deletions pkg/cli/logs_display_fields_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cli

import (
"encoding/json"
"strings"
"testing"

Expand Down Expand Up @@ -53,21 +54,22 @@ func TestMissingToolSummaryDisplayFields(t *testing.T) {
}
}

// TestMCPFailureSummaryDisplayFields verifies that Display fields are used by console rendering
// TestMCPFailureSummaryDisplayFields verifies that MCP display fields retain their specific tags.
func TestMCPFailureSummaryDisplayFields(t *testing.T) {
// Create a MCPFailureSummary with populated Display field
summaries := []MCPFailureSummary{
{
ServerName: "github-mcp-server",
Count: 3,
Workflows: []string{"workflow-a", "workflow-b"},
WorkflowsDisplay: "workflow-a, workflow-b", // This should be rendered
RunIDs: []int64{1, 2, 3},
ServerName: "github-mcp-server",
AggregatedSummaryBase: AggregatedSummaryBase{
Count: 3,
Workflows: []string{"workflow-a", "workflow-b"},
WorkflowsDisplay: "workflow-a, workflow-b", // This should be rendered
RunIDs: []int64{1, 2, 3},
},
},
}

// Render using console.RenderStruct
output := console.RenderStruct(summaries)
output := console.RenderStruct(mcpFailureSummaryDisplays(summaries))

// Verify that Display field is included in output
if !strings.Contains(output, "workflow-a, workflow-b") {
Expand All @@ -85,3 +87,37 @@ func TestMCPFailureSummaryDisplayFields(t *testing.T) {
t.Errorf("Workflows header not found in console output")
}
}

// TestMCPFailureSummaryJSONFields verifies that embedding AggregatedSummaryBase keeps
// the shared JSON fields flattened at the MCP failure summary level.
func TestMCPFailureSummaryJSONFields(t *testing.T) {
summary := MCPFailureSummary{
ServerName: "github-mcp-server",
AggregatedSummaryBase: AggregatedSummaryBase{
Count: 3,
Workflows: []string{"workflow-a", "workflow-b"},
FirstReason: "not part of the MCP failure schema",
RunIDs: []int64{1, 2, 3},
},
}

data, err := json.Marshal(summary)
if err != nil {
t.Fatalf("json.Marshal failed: %v", err)
}

output := string(data)
for _, expected := range []string{
`"server_name":"github-mcp-server"`,
`"count":3`,
`"workflows":["workflow-a","workflow-b"]`,
`"run_ids":[1,2,3]`,
} {
if !strings.Contains(output, expected) {
t.Errorf("Expected %s in JSON output: %s", expected, output)
}
}
if strings.Contains(output, "first_reason") {
t.Errorf("first_reason must not be added to the MCP failure JSON schema: %s", output)
}
}
24 changes: 19 additions & 5 deletions pkg/cli/logs_models.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cli

import (
"encoding/json"
"errors"
"time"

Expand Down Expand Up @@ -185,11 +186,24 @@ type MissingToolSummary struct {

// MCPFailureSummary aggregates MCP server failure reports across runs
type MCPFailureSummary struct {
ServerName string `json:"server_name" console:"header:Server"`
Count int `json:"count" console:"header:Failures"`
Workflows []string `json:"workflows" console:"-"` // List of workflow names that had this server fail
WorkflowsDisplay string `json:"-" console:"header:Workflows,maxlen:60"` // Formatted display of workflows
RunIDs []int64 `json:"run_ids" console:"-"` // List of run IDs where this server failed
ServerName string `json:"server_name" console:"header:Server"`
AggregatedSummaryBase `console:"-"`
}

// MarshalJSON preserves the MCP failure JSON schema while sharing aggregation state with
// the other summary types.
func (s MCPFailureSummary) MarshalJSON() ([]byte, error) {
return json.Marshal(struct {
ServerName string `json:"server_name"`
Count int `json:"count"`
Workflows []string `json:"workflows"`
RunIDs []int64 `json:"run_ids"`
}{
ServerName: s.ServerName,
Count: s.Count,
Workflows: s.Workflows,
RunIDs: s.RunIDs,
})
}

// MissingDataSummary aggregates missing data reports across runs
Expand Down
29 changes: 26 additions & 3 deletions pkg/cli/logs_report.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/github/gh-aw/pkg/console"
"github.com/github/gh-aw/pkg/logger"
"github.com/github/gh-aw/pkg/sliceutil"
"github.com/github/gh-aw/pkg/timeutil"
"github.com/github/gh-aw/pkg/workflow"
)
Expand All @@ -31,7 +32,7 @@ type LogsData struct {
ErrorsAndWarnings []ErrorSummary `json:"errors_and_warnings,omitempty" console:"title:Errors and Warnings,omitempty"`
MissingTools []MissingToolSummary `json:"missing_tools,omitempty" console:"title:🛠️ Missing Tools Summary,omitempty"`
MissingData []MissingDataSummary `json:"missing_data,omitempty" console:"title:📊 Missing Data Summary,omitempty"`
MCPFailures []MCPFailureSummary `json:"mcp_failures,omitempty" console:"title:⚠️ MCP Server Failures,omitempty"`
MCPFailures []MCPFailureSummary `json:"mcp_failures,omitempty" console:"-"`
AccessLog *AccessLogSummary `json:"access_log,omitempty" console:"title:Access Log Analysis,omitempty"`
FirewallLog *FirewallLogSummary `json:"firewall_log,omitempty" console:"title:🔥 Firewall Log Analysis,omitempty"`
RedactedDomains *RedactedDomainsLogSummary `json:"redacted_domains,omitempty" console:"title:🔒 Redacted URL Domains,omitempty"`
Expand Down Expand Up @@ -658,8 +659,14 @@ func renderLogsConsoleToWriter(w io.Writer, data LogsData) {
reportLog.Printf("Rendering logs data to console: %d runs, %d errors, %d warnings",
data.Summary.TotalRuns, data.Summary.TotalErrors, data.Summary.TotalWarnings)

// Use unified console rendering for the entire logs data structure
fmt.Fprint(w, console.RenderStruct(data))
// Use unified console rendering for the entire logs data structure.
mcpFailures := data.MCPFailures
consoleData := data
consoleData.MCPFailures = nil
fmt.Fprint(w, console.RenderStruct(consoleData))
fmt.Fprint(w, console.RenderStruct(struct {
MCPFailures []mcpFailureSummaryDisplay `console:"title:⚠️ MCP Server Failures,omitempty"`
}{MCPFailures: mcpFailureSummaryDisplays(mcpFailures)}))

// Display concise summary at the end
fmt.Fprintln(os.Stderr, "") // Blank line for spacing
Expand Down Expand Up @@ -688,6 +695,22 @@ func renderLogsConsoleToWriter(w io.Writer, data LogsData) {
}
}

type mcpFailureSummaryDisplay struct {
ServerName string `console:"header:Server"`
Count int `console:"header:Failures"`
WorkflowsDisplay string `console:"header:Workflows,maxlen:60"`
}

func mcpFailureSummaryDisplays(summaries []MCPFailureSummary) []mcpFailureSummaryDisplay {
return sliceutil.Map(summaries, func(summary MCPFailureSummary) mcpFailureSummaryDisplay {
return mcpFailureSummaryDisplay{
ServerName: summary.ServerName,
Count: summary.Count,
WorkflowsDisplay: summary.WorkflowsDisplay,
}
})
}

// renderLogsConsole outputs the logs data as formatted console output to os.Stdout.
func renderLogsConsole(data LogsData) {
renderLogsConsoleToWriter(os.Stdout, data)
Expand Down
8 changes: 5 additions & 3 deletions pkg/cli/logs_report_mcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@ func buildMCPFailuresSummary(processedRuns []ProcessedRun) []MCPFailureSummary {
func(failure MCPFailureReport) *MCPFailureSummary {
return &MCPFailureSummary{
ServerName: failure.ServerName,
Count: 1,
Workflows: []string{failure.WorkflowName},
RunIDs: []int64{failure.RunID},
AggregatedSummaryBase: AggregatedSummaryBase{
Count: 1,
Workflows: []string{failure.WorkflowName},
RunIDs: []int64{failure.RunID},
},
}
},
// updateSummary: update existing summary with new occurrence
Expand Down
47 changes: 39 additions & 8 deletions pkg/cli/logs_report_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"encoding/json"
"os"
"path/filepath"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -83,16 +84,20 @@ func TestRenderLogsConsoleUnified(t *testing.T) {
},
MCPFailures: []MCPFailureSummary{
{
ServerName: "github-mcp-server",
Count: 2,
Workflows: []string{"workflow-a", "workflow-b"},
WorkflowsDisplay: "workflow-a, workflow-b",
ServerName: "github-mcp-server",
AggregatedSummaryBase: AggregatedSummaryBase{
Count: 2,
Workflows: []string{"workflow-a", "workflow-b"},
WorkflowsDisplay: "workflow-a, workflow-b",
},
},
{
ServerName: "playwright",
Count: 1,
Workflows: []string{"browser-test"},
WorkflowsDisplay: "browser-test",
ServerName: "playwright",
AggregatedSummaryBase: AggregatedSummaryBase{
Count: 1,
Workflows: []string{"browser-test"},
WorkflowsDisplay: "browser-test",
},
},
},
LogsLocation: "/tmp/logs",
Expand All @@ -110,6 +115,32 @@ func TestRenderLogsConsoleUnified(t *testing.T) {
renderLogsConsoleToWriter(&buf, data)
}

func TestRenderLogsConsoleMCPFailureSchema(t *testing.T) {
var buf bytes.Buffer
renderLogsConsoleToWriter(&buf, LogsData{
MCPFailures: []MCPFailureSummary{{
ServerName: "github-mcp-server",
AggregatedSummaryBase: AggregatedSummaryBase{
Count: 2,
WorkflowsDisplay: "workflow-a, workflow-b",
FirstReasonDisplay: "not part of the MCP failure schema",
},
}},
})

output := buf.String()
for _, expected := range []string{"MCP Server Failures", "Server", "Failures", "Workflows"} {
if !strings.Contains(output, expected) {
t.Errorf("expected %q in MCP failure console output: %s", expected, output)
}
}
for _, unexpected := range []string{"Occurrences", "First Reason"} {
if strings.Contains(output, unexpected) {
t.Errorf("unexpected %q in MCP failure console output: %s", unexpected, output)
}
}
}

// TestBuildToolUsageSummaryPopulatesDisplay tests that buildToolUsageSummary works correctly
func TestBuildToolUsageSummaryPopulatesDisplay(t *testing.T) {
processedRuns := []ProcessedRun{
Expand Down
6 changes: 6 additions & 0 deletions pkg/console/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ func walkInlineFields(val reflect.Value, visit func(field reflect.Value, fieldTy
fieldType := typ.Field(i)

if fieldType.Anonymous {
if parseConsoleTag(fieldType.Tag.Get("console")).skip {
continue
}
if embedded, ok := embeddedStructValue(field); ok {
walkInlineFields(embedded, visit)
continue
Expand Down Expand Up @@ -292,6 +295,9 @@ func collectTableFields(t reflect.Type, prefix []int) []tableField {
fieldPath[len(prefix)] = i

if field.Anonymous {
if parseConsoleTag(field.Tag.Get("console")).skip {
continue
}
if embeddedType, ok := embeddedStructType(field.Type); ok {
fields = append(fields, collectTableFields(embeddedType, fieldPath)...)
continue
Expand Down
17 changes: 17 additions & 0 deletions pkg/console/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,23 @@ func TestRenderSlice_EmbeddedStruct(t *testing.T) {
assert.Contains(t, output, "disabled", "output should contain second status")
}

func TestRenderSlice_SkippedEmbeddedStruct(t *testing.T) {
type Base struct {
Name string `console:"header:Name"`
}
type Extended struct {
Base `console:"-"`
Status string `console:"header:Status"`
}

output := RenderStruct([]Extended{{Base: Base{Name: "wf-1"}, Status: "active"}})

assert.NotContains(t, output, "Name")
assert.NotContains(t, output, "wf-1")
assert.Contains(t, output, "Status")
assert.Contains(t, output, "active")
}

func TestRenderSlice_EmbeddedPointerStruct(t *testing.T) {
type Base struct {
Name string `console:"header:Name"`
Expand Down
Loading