Skip to content

Propagate cancellation through audit analysis soft-failure paths #46998

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
Jul 21, 2026
Merged

Propagate cancellation through audit analysis soft-failure paths #46998

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
416f3bb
Propagate cancellation in audit analysis soft-failure paths
Copilot Jul 21, 2026
3a0f501
Simplify context cancellation checks in audit fanout
Copilot Jul 21, 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
9 changes: 9 additions & 0 deletions pkg/cli/audit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,9 @@ func collectAuditAnalysisResults(ctx context.Context, run WorkflowRun, runOutput
if err := g.Wait(); err != nil {
return results, err
}
if ctx.Err() != nil {
return results, ctx.Err()
}
return results, nil
}

Expand Down Expand Up @@ -743,6 +746,9 @@ func launchJobDetailsAnalysis(g *errgroup.Group, gctx context.Context, results *
}
jobDetails, failedJobCount, err := fetchJobDetailsWithCounts(gctx, runID, verbose)
if err != nil {
if gctx.Err() != nil {
return gctx.Err()
}
auditLog.Printf("fetchJobDetailsWithCounts failed: %v", err)
if verbose {
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to fetch job details: %v", err)))
Expand Down Expand Up @@ -839,6 +845,9 @@ func runAuditAnalysis[T any](g *errgroup.Group, gctx context.Context, verbose bo
}
value, err := fn()
if err != nil {
if gctx.Err() != nil {
return gctx.Err()
}
auditLog.Printf("%s failed: %v", name, err)
if verbose {
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("%s: %v", warning, err)))
Expand Down
25 changes: 25 additions & 0 deletions pkg/cli/audit_concurrency_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,28 @@ func TestRunAuditAnalysisSoftFailuresRemainNonFatal(t *testing.T) {
t.Fatal("expected setter not to be called on soft failure")
}
}

func TestRunAuditAnalysisReturnsCancellationForSoftFailuresWhenContextCanceled(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
g, gctx := errgroup.WithContext(ctx)
called := false
started := make(chan struct{})

runAuditAnalysis(g, gctx, false, "test", "test warning", func(v int) {
called = true
}, func() (int, error) {
close(started)
<-gctx.Done()
return 0, errors.New("soft failure")
})

<-started
cancel()

if err := g.Wait(); !errors.Is(err, context.Canceled) {
t.Fatalf("expected context canceled error for canceled context soft failure, got %v", err)
}
if called {
t.Fatal("expected setter not to be called when context is canceled")

@github-actions github-actions Bot Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/tdd] The new test covers the runAuditAnalysis cancellation path well, but the analogous soft-failure cancellation path in launchJobDetailsAnalysis (audit.go line 749–751) has no equivalent test. That branch has the same shape and the same risk.

💡 Suggested test sketch

Add a test that sets up a fake fetchJobDetailsWithCounts that blocks until gctx is done, then returns an error, and asserts g.Wait() returns context.Canceled. The pattern is identical to TestRunAuditAnalysisReturnsCancellationForSoftFailuresWhenContextCanceled.

@copilot please address this.

}
}
Loading