diff --git a/pkg/cli/audit.go b/pkg/cli/audit.go index 1c3e079b8..7acd560cb 100644 --- a/pkg/cli/audit.go +++ b/pkg/cli/audit.go @@ -253,6 +253,12 @@ func AuditWorkflowRun(runInfo RunURLInfo, outputDir string, verbose bool, parse fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to extract missing tools: %v", err))) } + // Extract noops + noops, noopErr := extractNoopsFromRun(runOutputDir, run, verbose) + if noopErr != nil && verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to extract noops: %v", noopErr))) + } + // Extract MCP failures mcpFailures, err := extractMCPFailuresFromRun(runOutputDir, run, verbose) if err != nil && verbose { @@ -282,6 +288,7 @@ func AuditWorkflowRun(runInfo RunURLInfo, outputDir string, verbose bool, parse Run: run, FirewallAnalysis: firewallAnalysis, MissingTools: missingTools, + Noops: noops, MCPFailures: mcpFailures, JobDetails: jobDetails, } @@ -343,6 +350,7 @@ func AuditWorkflowRun(runInfo RunURLInfo, outputDir string, verbose bool, parse AccessAnalysis: accessAnalysis, FirewallAnalysis: firewallAnalysis, MissingTools: missingTools, + Noops: noops, MCPFailures: mcpFailures, ArtifactsList: artifacts, JobDetails: jobDetails, @@ -569,6 +577,18 @@ func generateAuditReport(processedRun ProcessedRun, metrics LogMetrics) string { } } + // No-Op Messages + if len(processedRun.Noops) > 0 { + report.WriteString("## No-Op Messages\n\n") + for i, noop := range processedRun.Noops { + report.WriteString(fmt.Sprintf("### Message %d\n\n", i+1)) + report.WriteString(fmt.Sprintf("%s\n\n", noop.Message)) + if noop.Timestamp != "" { + report.WriteString(fmt.Sprintf("**Timestamp**: %s\n\n", noop.Timestamp)) + } + } + } + // Error Summary if run.ErrorCount > 0 || run.WarningCount > 0 { report.WriteString("## Issue Summary\n\n") diff --git a/pkg/cli/audit_report.go b/pkg/cli/audit_report.go index 643faba08..372633f11 100644 --- a/pkg/cli/audit_report.go +++ b/pkg/cli/audit_report.go @@ -24,6 +24,7 @@ type AuditData struct { Jobs []JobData `json:"jobs,omitempty"` DownloadedFiles []FileInfo `json:"downloaded_files"` MissingTools []MissingToolReport `json:"missing_tools,omitempty"` + Noops []NoopReport `json:"noops,omitempty"` MCPFailures []MCPFailureReport `json:"mcp_failures,omitempty"` FirewallAnalysis *FirewallAnalysis `json:"firewall_analysis,omitempty"` Errors []ErrorInfo `json:"errors,omitempty"` @@ -212,6 +213,7 @@ func buildAuditData(processedRun ProcessedRun, metrics LogMetrics) AuditData { Jobs: jobs, DownloadedFiles: downloadedFiles, MissingTools: processedRun.MissingTools, + Noops: processedRun.Noops, MCPFailures: processedRun.MCPFailures, FirewallAnalysis: processedRun.FirewallAnalysis, Errors: errors, diff --git a/pkg/cli/logs.go b/pkg/cli/logs.go index 9beace5fa..44797ee4a 100644 --- a/pkg/cli/logs.go +++ b/pkg/cli/logs.go @@ -52,6 +52,7 @@ type WorkflowRun struct { ErrorCount int WarningCount int MissingToolCount int + NoopCount int LogsPath string } @@ -65,6 +66,7 @@ type ProcessedRun struct { AccessAnalysis *DomainAnalysis FirewallAnalysis *FirewallAnalysis MissingTools []MissingToolReport + Noops []NoopReport MCPFailures []MCPFailureReport JobDetails []JobInfoWithDuration } @@ -79,6 +81,14 @@ type MissingToolReport struct { RunID int64 `json:"run_id,omitempty"` // Added for tracking which run reported this } +// NoopReport represents a noop message reported by an agentic workflow +type NoopReport struct { + Message string `json:"message"` + Timestamp string `json:"timestamp,omitempty"` + WorkflowName string `json:"workflow_name,omitempty"` // Added for tracking which workflow reported this + RunID int64 `json:"run_id,omitempty"` // Added for tracking which run reported this +} + // MCPFailureReport represents an MCP server failure detected in a workflow run type MCPFailureReport struct { ServerName string `json:"server_name"` @@ -124,6 +134,7 @@ type RunSummary struct { AccessAnalysis *DomainAnalysis `json:"access_analysis"` // Network access analysis FirewallAnalysis *FirewallAnalysis `json:"firewall_analysis"` // Firewall log analysis MissingTools []MissingToolReport `json:"missing_tools"` // Missing tool reports + Noops []NoopReport `json:"noops"` // Noop messages MCPFailures []MCPFailureReport `json:"mcp_failures"` // MCP server failures ArtifactsList []string `json:"artifacts_list"` // List of downloaded artifact files JobDetails []JobInfoWithDuration `json:"job_details"` // Job execution details @@ -230,7 +241,9 @@ type DownloadResult struct { AccessAnalysis *DomainAnalysis FirewallAnalysis *FirewallAnalysis MissingTools []MissingToolReport + Noops []NoopReport MCPFailures []MCPFailureReport + JobDetails []JobInfoWithDuration Error error Skipped bool LogsPath string @@ -701,7 +714,9 @@ func DownloadWorkflowLogs(workflowName string, count int, startDate, endDate, ou AccessAnalysis: result.AccessAnalysis, FirewallAnalysis: result.FirewallAnalysis, MissingTools: result.MissingTools, + Noops: result.Noops, MCPFailures: result.MCPFailures, + JobDetails: result.JobDetails, } processedRuns = append(processedRuns, processedRun) batchProcessed++ @@ -795,9 +810,10 @@ func DownloadWorkflowLogs(workflowName string, count int, startDate, endDate, ou processedRuns = processedRuns[:count] } - // Update MissingToolCount in runs + // Update MissingToolCount and NoopCount in runs for i := range processedRuns { processedRuns[i].Run.MissingToolCount = len(processedRuns[i].MissingTools) + processedRuns[i].Run.NoopCount = len(processedRuns[i].Noops) } // Build continuation data if timeout was reached and there are processed runs @@ -880,7 +896,9 @@ func downloadRunArtifactsConcurrent(runs []WorkflowRun, outputDir string, verbos AccessAnalysis: summary.AccessAnalysis, FirewallAnalysis: summary.FirewallAnalysis, MissingTools: summary.MissingTools, + Noops: summary.Noops, MCPFailures: summary.MCPFailures, + JobDetails: summary.JobDetails, LogsPath: runOutputDir, } return result @@ -955,6 +973,15 @@ func downloadRunArtifactsConcurrent(runs []WorkflowRun, outputDir string, verbos } result.MissingTools = missingTools + // Extract noops if available + noops, noopErr := extractNoopsFromRun(runOutputDir, run, verbose) + if noopErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to extract noops for run %d: %v", run.DatabaseID, noopErr))) + } + } + result.Noops = noops + // Extract MCP failures if available mcpFailures, mcpErr := extractMCPFailuresFromRun(runOutputDir, run, verbose) if mcpErr != nil { @@ -990,6 +1017,7 @@ func downloadRunArtifactsConcurrent(runs []WorkflowRun, outputDir string, verbos AccessAnalysis: accessAnalysis, FirewallAnalysis: firewallAnalysis, MissingTools: missingTools, + Noops: noops, MCPFailures: mcpFailures, ArtifactsList: artifacts, JobDetails: jobDetails, @@ -1250,7 +1278,7 @@ func displayLogsOverview(processedRuns []ProcessedRun, verbose bool) { } // Prepare table data - headers := []string{"Run ID", "Workflow", "Status", "Duration", "Tokens", "Cost ($)", "Turns", "Errors", "Warnings", "Missing", "Created", "Logs Path"} + headers := []string{"Run ID", "Workflow", "Status", "Duration", "Tokens", "Cost ($)", "Turns", "Errors", "Warnings", "Missing", "Noops", "Created", "Logs Path"} var rows [][]string var totalTokens int @@ -1260,6 +1288,7 @@ func displayLogsOverview(processedRuns []ProcessedRun, verbose bool) { var totalErrors int var totalWarnings int var totalMissingTools int + var totalNoops int for _, pr := range processedRuns { run := pr.Run @@ -1318,6 +1347,29 @@ func displayLogsOverview(processedRuns []ProcessedRun, verbose bool) { } totalMissingTools += run.MissingToolCount + // Format noops + var noopsStr string + if verbose && len(pr.Noops) > 0 { + // In verbose mode, show truncated message preview + messages := make([]string, len(pr.Noops)) + for i, noop := range pr.Noops { + msg := noop.Message + if len(msg) > 30 { + msg = msg[:27] + "..." + } + messages[i] = msg + } + noopsStr = strings.Join(messages, ", ") + // Truncate if too long + if len(noopsStr) > 30 { + noopsStr = noopsStr[:27] + "..." + } + } else { + // In normal mode, just show the count + noopsStr = fmt.Sprintf("%d", run.NoopCount) + } + totalNoops += run.NoopCount + // Truncate workflow name if too long workflowName := run.WorkflowName if len(workflowName) > 20 { @@ -1344,6 +1396,7 @@ func displayLogsOverview(processedRuns []ProcessedRun, verbose bool) { errorsStr, warningsStr, missingToolsStr, + noopsStr, run.CreatedAt.Format("2006-01-02"), relPath, } @@ -1362,6 +1415,7 @@ func displayLogsOverview(processedRuns []ProcessedRun, verbose bool) { fmt.Sprintf("%d", totalErrors), fmt.Sprintf("%d", totalWarnings), fmt.Sprintf("%d", totalMissingTools), + fmt.Sprintf("%d", totalNoops), "", "", } diff --git a/pkg/cli/logs_metrics.go b/pkg/cli/logs_metrics.go index 9197decf6..12d267c13 100644 --- a/pkg/cli/logs_metrics.go +++ b/pkg/cli/logs_metrics.go @@ -295,6 +295,112 @@ func extractMissingToolsFromRun(runDir string, run WorkflowRun, verbose bool) ([ return missingTools, nil } +// extractNoopsFromRun extracts noop messages from a workflow run's artifacts +func extractNoopsFromRun(runDir string, run WorkflowRun, verbose bool) ([]NoopReport, error) { + logsMetricsLog.Printf("Extracting noops from run: %d", run.DatabaseID) + var noops []NoopReport + + // Look for the safe output artifact file that contains structured JSON with items array + // This file is created by the collect_ndjson_output.cjs script during workflow execution + agentOutputPath := filepath.Join(runDir, constants.AgentOutputArtifactName) + + // Support both file and directory forms of agent_output.json artifact (directory contains nested agent_output.json file) + // Also fall back to searching the tree if neither form exists at root. + var resolvedAgentOutputFile string + if stat, err := os.Stat(agentOutputPath); err == nil { + if stat.IsDir() { + // Directory form – look for nested file + nested := filepath.Join(agentOutputPath, constants.AgentOutputArtifactName) + if _, nestedErr := os.Stat(nested); nestedErr == nil { + resolvedAgentOutputFile = nested + if verbose { + fmt.Println(console.FormatInfoMessage(fmt.Sprintf("agent_output.json is a directory; using nested file %s", nested))) + } + } else if verbose { + fmt.Println(console.FormatWarningMessage(fmt.Sprintf("agent_output.json directory present but nested file missing: %v", nestedErr))) + } + } else { + // Regular file + resolvedAgentOutputFile = agentOutputPath + } + } else { + // Not present at root – search recursively (depth-first) for a file named agent_output.json + if found, ok := findAgentOutputFile(runDir); ok { + resolvedAgentOutputFile = found + if verbose && found != agentOutputPath { + fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Found agent_output.json at %s", found))) + } + } + } + + if resolvedAgentOutputFile != "" { + // Read the safe output artifact file + content, readErr := os.ReadFile(resolvedAgentOutputFile) + if readErr != nil { + if verbose { + fmt.Println(console.FormatWarningMessage(fmt.Sprintf("Failed to read safe output file %s: %v", resolvedAgentOutputFile, readErr))) + } + return noops, nil // Continue processing without this file + } + + // Parse the structured JSON output from the collect script + var safeOutput struct { + Items []json.RawMessage `json:"items"` + Errors []string `json:"errors,omitempty"` + } + + if err := json.Unmarshal(content, &safeOutput); err != nil { + if verbose { + fmt.Println(console.FormatWarningMessage(fmt.Sprintf("Failed to parse safe output JSON from %s: %v", resolvedAgentOutputFile, err))) + } + return noops, nil // Continue processing without this file + } + + // Extract noop entries from the items array + for _, itemRaw := range safeOutput.Items { + var item struct { + Type string `json:"type"` + Message string `json:"message,omitempty"` + Timestamp string `json:"timestamp,omitempty"` + } + + if err := json.Unmarshal(itemRaw, &item); err != nil { + if verbose { + fmt.Println(console.FormatWarningMessage(fmt.Sprintf("Failed to parse item from safe output: %v", err))) + } + continue // Skip malformed items + } + + // Check if this is a noop entry + if item.Type == "noop" { + noop := NoopReport{ + Message: item.Message, + Timestamp: item.Timestamp, + WorkflowName: run.WorkflowName, + RunID: run.DatabaseID, + } + noops = append(noops, noop) + + if verbose { + fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Found noop entry: %s", item.Message))) + } + } + } + + if verbose && len(noops) > 0 { + fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Found %d noop messages in safe output artifact for run %d", len(noops), run.DatabaseID))) + } + logsMetricsLog.Printf("Found %d noop messages", len(noops)) + } else { + logsMetricsLog.Print("No safe output artifact found") + if verbose { + fmt.Println(console.FormatInfoMessage(fmt.Sprintf("No safe output artifact found at %s for run %d", agentOutputPath, run.DatabaseID))) + } + } + + return noops, nil +} + // extractMCPFailuresFromRun extracts MCP server failure reports from a workflow run's logs func extractMCPFailuresFromRun(runDir string, run WorkflowRun, verbose bool) ([]MCPFailureReport, error) { logsMetricsLog.Printf("Extracting MCP failures from run: %d", run.DatabaseID) diff --git a/pkg/cli/logs_noop_test.go b/pkg/cli/logs_noop_test.go new file mode 100644 index 000000000..6256e4ef8 --- /dev/null +++ b/pkg/cli/logs_noop_test.go @@ -0,0 +1,145 @@ +package cli + +import ( + "os" + "path/filepath" + "testing" + + "github.com/githubnext/gh-aw/pkg/testutil" + + "github.com/githubnext/gh-aw/pkg/constants" +) + +// TestExtractNoopsFromRun tests extracting noop messages from safe output artifact files +func TestExtractNoopsFromRun(t *testing.T) { + // Create a temporary directory structure + tmpDir := testutil.TempDir(t, "test-*") + + testRun := WorkflowRun{ + DatabaseID: 67890, + WorkflowName: "Integration Test", + } + + tests := []struct { + name string + safeOutputContent string + expected int + expectMessage string + }{ + { + name: "single_noop_in_safe_output", + safeOutputContent: `{ + "items": [ + { + "type": "noop", + "message": "This is a test noop message", + "timestamp": "2024-01-01T12:00:00Z" + } + ], + "errors": [] + }`, + expected: 1, + expectMessage: "This is a test noop message", + }, + { + name: "multiple_noops_in_safe_output", + safeOutputContent: `{ + "items": [ + { + "type": "noop", + "message": "First noop message", + "timestamp": "2024-01-01T10:00:00Z" + }, + { + "type": "noop", + "message": "Second noop message", + "timestamp": "2024-01-01T10:01:00Z" + }, + { + "type": "create-issue", + "title": "Test Issue", + "body": "This should be ignored" + } + ], + "errors": [] + }`, + expected: 2, + expectMessage: "First noop message", + }, + { + name: "no_noops_in_safe_output", + safeOutputContent: `{ + "items": [ + { + "type": "create-issue", + "title": "Test Issue", + "body": "No noops here" + } + ], + "errors": [] + }`, + expected: 0, + }, + { + name: "empty_safe_output", + safeOutputContent: `{ + "items": [], + "errors": [] + }`, + expected: 0, + }, + { + name: "malformed_json", + safeOutputContent: `{ + "items": [ + { + "type": "noop" + "message": "broken" + } + ] + }`, + expected: 0, // Should handle gracefully + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create the safe output artifact file + safeOutputFile := filepath.Join(tmpDir, constants.AgentOutputArtifactName) + err := os.WriteFile(safeOutputFile, []byte(tt.safeOutputContent), 0644) + if err != nil { + t.Fatalf("Failed to create test safe output file: %v", err) + } + + // Extract noops + noops, err := extractNoopsFromRun(tmpDir, testRun, false) + if err != nil { + t.Fatalf("Error extracting noops: %v", err) + } + + if len(noops) != tt.expected { + t.Errorf("Expected %d noops, got %d", tt.expected, len(noops)) + return + } + + if tt.expected > 0 && len(noops) > 0 { + noop := noops[0] + if noop.Message != tt.expectMessage { + t.Errorf("Expected message '%s', got '%s'", tt.expectMessage, noop.Message) + } + + // Check that run information was populated + if noop.WorkflowName != testRun.WorkflowName { + t.Errorf("Expected workflow name '%s', got '%s'", testRun.WorkflowName, noop.WorkflowName) + } + + if noop.RunID != testRun.DatabaseID { + t.Errorf("Expected run ID %d, got %d", testRun.DatabaseID, noop.RunID) + } + } + + // Clean up for next test + os.Remove(safeOutputFile) + }) + } +}