diff --git a/pkg/cli/audit.go b/pkg/cli/audit.go index 33d1d830ce..bfe3d96d0b 100644 --- a/pkg/cli/audit.go +++ b/pkg/cli/audit.go @@ -17,6 +17,7 @@ import ( "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/parser" "github.com/github/gh-aw/pkg/workflow" + "github.com/goccy/go-yaml" "github.com/spf13/cobra" ) @@ -657,8 +658,14 @@ func fetchWorkflowRunMetadata(runID int64, owner, repo, hostname string, verbose if verbose { fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(string(output))) } - // Provide a human-readable error when the run ID doesn't exist - if strings.Contains(string(output), "Not Found") || strings.Contains(string(output), "404") { + // Provide a human-readable error when the run ID doesn't exist. + // GitHub CLI / API may surface the 404 in several forms depending on version. + outputStr := string(output) + if strings.Contains(outputStr, "Not Found") || + strings.Contains(outputStr, "404") || + strings.Contains(outputStr, "not found") || + strings.Contains(outputStr, "Could not resolve") || + strings.Contains(err.Error(), "404") { return WorkflowRun{}, fmt.Errorf("workflow run %d not found. Please verify the run ID is correct and that you have access to the repository", runID) } return WorkflowRun{}, fmt.Errorf("failed to fetch run metadata: %w", err) @@ -669,5 +676,71 @@ func fetchWorkflowRunMetadata(runID int64, owner, repo, hostname string, verbose return WorkflowRun{}, fmt.Errorf("failed to parse run metadata: %w", err) } + // When the GitHub API returns the workflow file path as the run's name (e.g. for runs + // that were cancelled or failed before any jobs started), resolve the actual workflow + // display name so that audit output is consistent with 'gh aw logs'. + if strings.HasPrefix(run.WorkflowName, ".github/") { + if displayName := resolveWorkflowDisplayName(run.WorkflowPath, owner, repo, hostname); displayName != "" { + auditLog.Printf("Resolved workflow display name: %q -> %q", run.WorkflowName, displayName) + run.WorkflowName = displayName + } + } + return run, nil } + +// resolveWorkflowDisplayName returns the human-readable display name for a workflow file. +// It first attempts to read the YAML file from the local filesystem (resolving the path +// relative to the git repository root so that it works from any working directory inside +// the repo); if that fails it falls back to a GitHub API call. An empty string is +// returned on any error so that callers can gracefully keep the original value. +func resolveWorkflowDisplayName(workflowPath, owner, repo, hostname string) string { + // Try local file first. workflowPath is a repo-relative path like + // ".github/workflows/foo.lock.yml", so we resolve it against the git root to + // produce a correct absolute path regardless of the current working directory. + if gitRoot, err := findGitRoot(); err == nil { + absPath := filepath.Join(gitRoot, workflowPath) + if content, err := os.ReadFile(absPath); err == nil { + if name := extractWorkflowNameFromYAML(content); name != "" { + return name + } + } + } + + // Fall back to the GitHub Actions workflows API. + filename := filepath.Base(workflowPath) + var endpoint string + if owner != "" && repo != "" { + endpoint = fmt.Sprintf("repos/%s/%s/actions/workflows/%s", owner, repo, filename) + } else { + endpoint = "repos/{owner}/{repo}/actions/workflows/" + filename + } + + args := []string{"api"} + if hostname != "" && hostname != "github.com" { + args = append(args, "--hostname", hostname) + } + args = append(args, endpoint, "--jq", ".name") + + out, err := workflow.RunGHCombined("Fetching workflow name...", args...) + if err != nil { + auditLog.Printf("Failed to fetch workflow display name for %q: %v", workflowPath, err) + return "" + } + + return strings.TrimSpace(string(out)) +} + +// extractWorkflowNameFromYAML parses a GitHub Actions workflow YAML document and +// returns the value of its top-level "name:" field. An empty string is returned +// when the field is absent or the document cannot be parsed. +func extractWorkflowNameFromYAML(content []byte) string { + var wf struct { + Name string `yaml:"name"` + } + if err := yaml.Unmarshal(content, &wf); err != nil { + auditLog.Printf("Failed to parse workflow YAML for name extraction (file may be malformed): %v", err) + return "" + } + return wf.Name +} diff --git a/pkg/cli/audit_report_analysis.go b/pkg/cli/audit_report_analysis.go index 6b3899daf1..ed33c5810d 100644 --- a/pkg/cli/audit_report_analysis.go +++ b/pkg/cli/audit_report_analysis.go @@ -18,16 +18,24 @@ func generateFindings(processedRun ProcessedRun, metrics MetricsData, errors []E // Failure findings if run.Conclusion == "failure" { - desc := fmt.Sprintf("Workflow '%s' failed with %d error(s)", run.WorkflowName, metrics.ErrorCount) - if len(errors) > 0 { - // Append a truncated first error message to help quickly identify the root cause. - // Keep descriptions short enough to be useful in a key findings summary. - const maxErrMsgLen = 200 - msg := errors[0].Message - if len(msg) > maxErrMsgLen { - msg = msg[:maxErrMsgLen] + "..." + var desc string + if metrics.ErrorCount == 0 && len(errors) == 0 { + // No log data available — run likely failed before agent activation (e.g. cancelled, + // infrastructure failure, or no downloadable artifacts). Saying "failed with 0 error(s)" + // is logically contradictory, so surface a clearer message instead. + desc = fmt.Sprintf("Workflow '%s' failed before agent activation — no error logs were available to analyze", run.WorkflowName) + } else { + desc = fmt.Sprintf("Workflow '%s' failed with %d error(s)", run.WorkflowName, metrics.ErrorCount) + if len(errors) > 0 { + // Append a truncated first error message to help quickly identify the root cause. + // Keep descriptions short enough to be useful in a key findings summary. + const maxErrMsgLen = 200 + msg := errors[0].Message + if len(msg) > maxErrMsgLen { + msg = msg[:maxErrMsgLen] + "..." + } + desc += ": " + msg } - desc += ": " + msg } findings = append(findings, Finding{ Category: "error", diff --git a/pkg/cli/audit_report_test.go b/pkg/cli/audit_report_test.go index ddec7b6ddc..ef699910d0 100644 --- a/pkg/cli/audit_report_test.go +++ b/pkg/cli/audit_report_test.go @@ -190,6 +190,31 @@ func TestGenerateFindings(t *testing.T) { "Description should match standard format without error message suffix when no errors available") }, }, + { + name: "failed workflow with zero errors and no error details uses pre-activation message", + processedRun: func() ProcessedRun { + pr := createTestProcessedRun() + pr.Run.Conclusion = "failure" + return pr + }(), + metrics: MetricsData{ + ErrorCount: 0, // no errors extracted — logs were not available + }, + errors: []ErrorInfo{}, + warnings: []ErrorInfo{}, + expectedCount: 1, + checkFindings: func(t *testing.T, findings []Finding) { + finding := findFindingByCategory(findings, "error") + require.NotNil(t, finding, "Failed workflow should still generate an error finding") + assert.Equal(t, "critical", finding.Severity, "Error finding should have critical severity") + assert.Contains(t, finding.Description, "before agent activation", + "Description should indicate pre-activation failure when no logs are available") + assert.Contains(t, finding.Description, "no error logs", + "Description should mention that no error logs were available") + assert.NotContains(t, finding.Description, "0 error(s)", + "Description must not produce the contradictory 'failed with 0 error(s)' message") + }, + }, { name: "timed out workflow", processedRun: func() ProcessedRun { diff --git a/pkg/cli/audit_test.go b/pkg/cli/audit_test.go index 6f6344371b..b4b48842cf 100644 --- a/pkg/cli/audit_test.go +++ b/pkg/cli/audit_test.go @@ -733,3 +733,101 @@ Also success }) } } + +func TestExtractWorkflowNameFromYAML(t *testing.T) { + tests := []struct { + name string + content string + expected string + }{ + { + name: "simple name field", + content: `name: Daily CLI Tools Exploratory Tester +on: + push: + branches: [main] +`, + expected: "Daily CLI Tools Exploratory Tester", + }, + { + name: "name with double quotes", + content: `name: "My Workflow" +on: + workflow_dispatch: +`, + expected: "My Workflow", + }, + { + name: "name with single quotes", + content: `name: 'Another Workflow' +on: + push: +`, + expected: "Another Workflow", + }, + { + name: "no name field", + content: `on: + push: + branches: [main] +jobs: + build: +`, + expected: "", + }, + { + name: "name field after comment", + content: `# This is a compiled workflow +name: Test Workflow +on: + push: +`, + expected: "Test Workflow", + }, + { + name: "indented name (not top-level) is ignored", + content: `on: + push: +jobs: + build: + name: build-job +`, + // GitHub Actions requires the workflow 'name' at the top level of the document. + // A 'name' key nested inside 'jobs' or other sections should not be returned. + expected: "", + }, + { + name: "inline comment after name is stripped by YAML parser", + content: `name: My Workflow # inline comment +on: + push: +`, + expected: "My Workflow", + }, + { + name: "empty content", + content: "", + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractWorkflowNameFromYAML([]byte(tt.content)) + if result != tt.expected { + t.Errorf("extractWorkflowNameFromYAML() = %q, want %q", result, tt.expected) + } + }) + } +} + +func TestResolveWorkflowDisplayNameFromLocalFile(t *testing.T) { + // Write a temporary workflow YAML file and verify the name is extracted correctly + // via extractWorkflowNameFromYAML (the local-file path in resolveWorkflowDisplayName + // requires a real git root, so we test the YAML extraction directly here). + content := []byte("name: My Test Workflow\non:\n push:\n") + name := extractWorkflowNameFromYAML(content) + if name != "My Test Workflow" { + t.Errorf("extractWorkflowNameFromYAML() = %q, want %q", name, "My Test Workflow") + } +}