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
77 changes: 75 additions & 2 deletions pkg/cli/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
Expand All @@ -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
}
26 changes: 17 additions & 9 deletions pkg/cli/audit_report_analysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
25 changes: 25 additions & 0 deletions pkg/cli/audit_report_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
98 changes: 98 additions & 0 deletions pkg/cli/audit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
Loading