Skip to content

fix(audit): extract ##[error] annotations from all step logs instead of last-step content #18010

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 2 commits into from
Feb 24, 2026
Merged

fix(audit): extract ##[error] annotations from all step logs instead of last-step content #18010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
58b83a3
Initial plan
Copilot Feb 24, 2026
66d00b1
fix(audit): scan all step logs for ##[error] annotations instead of r…
Copilot Feb 24, 2026
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
65 changes: 56 additions & 9 deletions pkg/cli/audit_report.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,8 +512,10 @@ func parseDurationString(s string) time.Duration {
// steps such as lockdown validation, binary installation, or repository checkout failures.
//
// Step log files are stored in workflow-logs/{job}/{step_num}_{step_name}.txt after
// downloading via downloadWorkflowRunLogs. The function finds the last step that ran
// (highest step number) as that is most likely the step that caused the failure.
// downloading via downloadWorkflowRunLogs. The function first scans all step logs for
// ##[error] annotations (GitHub Actions error annotations), which are the most precise
// failure indicators. If none are found, it falls back to the content of the last step
// (highest step number) as a general failure indicator.
func extractPreAgentStepErrors(logsPath string) []ErrorInfo {
// If agent-stdio.log exists, the agent ran - don't scan step logs
agentStdioPath := filepath.Join(logsPath, "agent-stdio.log")
Expand All @@ -529,15 +531,19 @@ func extractPreAgentStepErrors(logsPath string) []ErrorInfo {
return nil
}

// Find the last step log file by scanning job subdirectories.
// Scan all job step log files in a single pass, collecting both ##[error] annotations
// and tracking the last step for fallback use.
// GitHub Actions log zip structure: {job_name}/{step_num}_{step_name}.txt
type stepLog struct {
path string
num int
stepKey string // job/step_name for display
}

const maxMessageLen = 1500

var lastStep *stepLog
var errorAnnotations []ErrorInfo

jobDirs, err := os.ReadDir(workflowLogsDir)
if err != nil {
Expand All @@ -558,16 +564,59 @@ func extractPreAgentStepErrors(logsPath string) []ErrorInfo {
continue
}
num, stepName := parseStepFilename(stepFile.Name())
if num > 0 && (lastStep == nil || num > lastStep.num) {
if num <= 0 {
continue
}
stepFilePath := filepath.Join(jobDir, stepFile.Name())
stepKey := jobEntry.Name() + "/" + stepName

// Track the last step (highest step number) for fallback
if lastStep == nil || num > lastStep.num {
lastStep = &stepLog{
path: filepath.Join(jobDir, stepFile.Name()),
path: stepFilePath,
num: num,
stepKey: jobEntry.Name() + "/" + stepName,
stepKey: stepKey,
}
}

// Scan this step for ##[error] annotations
content, err := os.ReadFile(stepFilePath)
if err != nil {
auditReportLog.Printf("Failed to read step log %s: %v", stepFilePath, err)
continue
}

var errorLines []string
for line := range strings.SplitSeq(string(content), "\n") {
if strings.Contains(line, "##[error]") {
stripped := stripGHALogTimestamps(line)
if stripped != "" {
errorLines = append(errorLines, stripped)
}
}
}
Comment on lines +582 to 597
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.ReadFile loads the entire step log into memory (and string(content) copies it again) even though we only need to detect/collect ##[error] lines. Step logs can be large, so this can significantly increase memory/time when scanning many steps. Consider streaming the file (e.g., bufio.Scanner / bufio.Reader) and collecting matching lines without reading the whole file; optionally stop once the accumulated message reaches maxMessageLen to cap work per step.

Suggested change
// Scan this step for ##[error] annotations
content, err := os.ReadFile(stepFilePath)
if err != nil {
auditReportLog.Printf("Failed to read step log %s: %v", stepFilePath, err)
continue
}
var errorLines []string
for line := range strings.SplitSeq(string(content), "\n") {
if strings.Contains(line, "##[error]") {
stripped := stripGHALogTimestamps(line)
if stripped != "" {
errorLines = append(errorLines, stripped)
}
}
}
// Scan this step for ##[error] annotations without loading the entire file into memory
f, err := os.Open(stepFilePath)
if err != nil {
auditReportLog.Printf("Failed to open step log %s: %v", stepFilePath, err)
continue
}
var errorLines []string
scanner := bufio.NewScanner(f)
accumulatedLen := 0
for scanner.Scan() {
line := scanner.Text()
if strings.Contains(line, "##[error]") {
stripped := stripGHALogTimestamps(line)
if stripped != "" {
errorLines = append(errorLines, stripped)
// Track accumulated length (including newline separators)
accumulatedLen += len(stripped) + 1
if accumulatedLen >= maxMessageLen {
break
}
}
}
}
if err := scanner.Err(); err != nil {
auditReportLog.Printf("Failed to scan step log %s: %v", stepFilePath, err)
}
_ = f.Close()

Copilot uses AI. Check for mistakes.

if len(errorLines) > 0 {
message := strings.Join(errorLines, "\n")
if len(message) > maxMessageLen {
message = message[:maxMessageLen] + "..."
}
auditReportLog.Printf("Extracted ##[error] annotations from %s (step %d)", stepKey, num)
errorAnnotations = append(errorAnnotations, ErrorInfo{
Type: "step_failure",
File: stepKey,
Message: message,
Comment on lines +604 to +608
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

errorAnnotations are appended in the filesystem iteration order (os.ReadDir sorts lexicographically), which can produce non-numeric step ordering (e.g., 10_*.txt before 2_*.txt) and a confusing error list when multiple steps contribute annotations. Consider collecting (job, stepNum, stepKey, message) in an intermediate slice and sorting by job + stepNum before returning the final []ErrorInfo.

Copilot uses AI. Check for mistakes.
})
}
}
}

// Prefer ##[error] annotations over generic last-step content
if len(errorAnnotations) > 0 {
return errorAnnotations
}

// Fallback: return the content of the last step that ran
if lastStep == nil {
auditReportLog.Printf("No step log files found in %s", workflowLogsDir)
return nil
Expand All @@ -584,13 +633,11 @@ func extractPreAgentStepErrors(logsPath string) []ErrorInfo {
return nil
}

// Truncate to a reasonable size for the error summary
const maxMessageLen = 1500
if len(message) > maxMessageLen {
message = message[:maxMessageLen] + "..."
}

auditReportLog.Printf("Extracted pre-agent step error from %s (step %d)", lastStep.stepKey, lastStep.num)
auditReportLog.Printf("Extracted pre-agent step error from %s (step %d) as fallback", lastStep.stepKey, lastStep.num)
return []ErrorInfo{{
Type: "step_failure",
File: lastStep.stepKey,
Expand Down
58 changes: 58 additions & 0 deletions pkg/cli/audit_report_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1501,6 +1501,64 @@ func TestExtractPreAgentStepErrors(t *testing.T) {
assert.True(t, strings.HasSuffix(errors[0].Message, "..."), "Truncated message should end with ellipsis")
})

t.Run("prioritizes ##[error] annotations over last step fallback", func(t *testing.T) {
dir := testutil.TempDir(t, "audit-step-*")
workflowLogsDir := filepath.Join(dir, "workflow-logs", "agent")
require.NoError(t, os.MkdirAll(workflowLogsDir, 0755))
// Step 3 has a ##[error] annotation (the real failure)
lockdownLog := "2026-02-23T23:46:10.9523559Z ##[error]Lockdown mode is enabled (lockdown: true) but no custom GitHub token is configured.\n2026-02-23T23:46:10.9523560Z Please configure GH_AW_GITHUB_TOKEN"
require.NoError(t, os.WriteFile(filepath.Join(workflowLogsDir, "3_Validate lockdown mode.txt"), []byte(lockdownLog), 0600))
// Step 15 is the "Complete job" step with unrelated cleanup content (higher step number)
completeJobLog := "2026-02-23T23:46:13.5790741Z Evaluate and set job outputs\n2026-02-23T23:46:13.5790742Z Set output 'checkout_pr_success'\n2026-02-23T23:46:13.5790743Z Set output 'has_patch'"
require.NoError(t, os.WriteFile(filepath.Join(workflowLogsDir, "15_Complete job.txt"), []byte(completeJobLog), 0600))

errors := extractPreAgentStepErrors(dir)
require.NotNil(t, errors, "Should return errors from ##[error] annotations")
require.Len(t, errors, 1, "Should return one error for the step with ##[error]")
assert.Equal(t, "agent/Validate lockdown mode", errors[0].File, "Should reference the step with ##[error], not Complete job")
assert.Contains(t, errors[0].Message, "Lockdown mode is enabled", "Message should contain the actual ##[error] annotation content")
assert.NotContains(t, errors[0].Message, "Evaluate and set job outputs", "Message should not contain Complete job cleanup content")
assert.NotContains(t, errors[0].Message, "2026-02-23T", "Should strip GHA timestamps from ##[error] lines")
})

t.Run("returns ##[error] annotations from multiple steps", func(t *testing.T) {
dir := testutil.TempDir(t, "audit-step-*")
workflowLogsDir := filepath.Join(dir, "workflow-logs", "agent")
require.NoError(t, os.MkdirAll(workflowLogsDir, 0755))
// Two steps each with ##[error] annotations
require.NoError(t, os.WriteFile(filepath.Join(workflowLogsDir, "3_Step A.txt"),
[]byte("2024-01-01T00:00:01Z ##[error]First error"), 0600))
require.NoError(t, os.WriteFile(filepath.Join(workflowLogsDir, "5_Step B.txt"),
[]byte("2024-01-01T00:00:02Z ##[error]Second error"), 0600))
// Higher-numbered step with no ##[error]
require.NoError(t, os.WriteFile(filepath.Join(workflowLogsDir, "10_Complete job.txt"),
[]byte("Cleanup content"), 0600))

errors := extractPreAgentStepErrors(dir)
require.NotNil(t, errors, "Should return errors")
assert.Len(t, errors, 2, "Should return one ErrorInfo per step with ##[error] annotations")
// All returned errors should be from steps with ##[error], not the cleanup step
for _, e := range errors {
assert.NotEqual(t, "agent/Complete job", e.File, "Should not include cleanup step in errors")
}
})

t.Run("falls back to last step when no ##[error] annotations exist", func(t *testing.T) {
dir := testutil.TempDir(t, "audit-step-*")
workflowLogsDir := filepath.Join(dir, "workflow-logs", "agent")
require.NoError(t, os.MkdirAll(workflowLogsDir, 0755))
// Step 3 has non-annotated error content
require.NoError(t, os.WriteFile(filepath.Join(workflowLogsDir, "3_Some step.txt"), []byte("Some content"), 0600))
// Step 7 is the last step and has the actual failure (no ##[error] prefix)
require.NoError(t, os.WriteFile(filepath.Join(workflowLogsDir, "7_Failing step.txt"), []byte("Error: installation failed"), 0600))

errors := extractPreAgentStepErrors(dir)
require.NotNil(t, errors, "Should fall back to last step content")
require.Len(t, errors, 1, "Fallback should return one error")
assert.Equal(t, "agent/Failing step", errors[0].File, "Fallback should use the last step")
assert.Contains(t, errors[0].Message, "Error: installation failed", "Fallback should use last step content")
})

t.Run("builds error summary from step errors in buildAuditData", func(t *testing.T) {
dir := testutil.TempDir(t, "audit-step-*")
// No agent-stdio.log
Expand Down
Loading