Skip to content

Document audit goroutine field-ownership invariants #46962

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 4 commits into from
Jul 21, 2026
+31 −3
Merged

Document audit goroutine field-ownership invariants #46962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
83bd9e9
Initial plan
Copilot Jul 21, 2026
134e908
Plan audit concurrency documentation update
Copilot Jul 21, 2026
53e22df
Document audit analysis field ownership
Copilot Jul 21, 2026
092db9e
Refine audit ownership comments
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
21 changes: 18 additions & 3 deletions .github/workflows/smoke-call-workflow.lock.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions pkg/cli/audit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,13 @@ type auditRunConfig struct {
evalsArtifactRequested bool
}

// auditAnalysisResults is populated concurrently during audit collection.
// Each field is written by exactly one goroutine (or one launch* helper that
// exclusively owns a disjoint field set) before collectAuditAnalysisResults
// waits on the shared WaitGroup and reads the aggregate result.
//
// Keep launch* helper field ownership disjoint: sharing a field between
// goroutines without adding synchronization would introduce a data race.
type auditAnalysisResults struct {
metrics LogMetrics
failedJobCount int
Expand Down Expand Up @@ -665,6 +672,7 @@ func collectAuditAnalysisResults(run WorkflowRun, runOutputDir string, verbose b
return results
}

// launchCoreAuditAnalyses exclusively writes missingTools, missingData, noops, mcpFailures, and accessAnalysis.
func launchCoreAuditAnalyses(wg *sync.WaitGroup, results *auditAnalysisResults, run WorkflowRun, runOutputDir string, verbose bool) {
// Resolve experiment assignment once so all goroutines reuse the same values
// rather than each reading state.json independently.
Expand Down Expand Up @@ -699,6 +707,7 @@ func launchCoreAuditAnalyses(wg *sync.WaitGroup, results *auditAnalysisResults,
})
}

// launchMetricsAnalysis exclusively writes results.metrics.
func launchMetricsAnalysis(wg *sync.WaitGroup, results *auditAnalysisResults, runOutputDir string, verbose bool, workflowPath string) {
wg.Go(func() {
metrics, err := extractLogMetrics(runOutputDir, verbose, workflowPath)
Expand All @@ -713,6 +722,7 @@ func launchMetricsAnalysis(wg *sync.WaitGroup, results *auditAnalysisResults, ru
})
}

// launchJobDetailsAnalysis exclusively writes results.jobDetails and results.failedJobCount.
func launchJobDetailsAnalysis(wg *sync.WaitGroup, results *auditAnalysisResults, runID int64, verbose bool) {
wg.Go(func() {
jobDetails, failedJobCount, err := fetchJobDetailsWithCounts(runID, verbose)
Expand All @@ -728,6 +738,7 @@ func launchJobDetailsAnalysis(wg *sync.WaitGroup, results *auditAnalysisResults,
})
}

// launchFirewallAuditAnalyses exclusively writes policyAnalysis, mcpToolUsage, and tokenUsageSummary.
func launchFirewallAuditAnalyses(wg *sync.WaitGroup, results *auditAnalysisResults, runOutputDir string, verbose bool) {
launchFirewallAnalysis(wg, results, runOutputDir, verbose)
runAuditAnalysis(wg, verbose, "analyzeFirewallPolicy", "Failed to analyze firewall policy", func(v *PolicyAnalysis) {
Expand All @@ -747,6 +758,7 @@ func launchFirewallAuditAnalyses(wg *sync.WaitGroup, results *auditAnalysisResul
})
}

// launchFirewallAnalysis exclusively writes results.firewallAnalysis.
func launchFirewallAnalysis(wg *sync.WaitGroup, results *auditAnalysisResults, runOutputDir string, verbose bool) {
wg.Go(func() {
firewallAnalysis, err := analyzeFirewallLogs(runOutputDir, verbose)
Expand All @@ -767,6 +779,7 @@ func launchFirewallAnalysis(wg *sync.WaitGroup, results *auditAnalysisResults, r
})
}

// launchSupplementalAuditAnalyses exclusively writes redactedDomainsAnalysis, rateLimitUsage, artifacts, and safeItemsCount.
func launchSupplementalAuditAnalyses(wg *sync.WaitGroup, results *auditAnalysisResults, runOutputDir string, verbose bool) {
runAuditAnalysis(wg, verbose, "analyzeRedactedDomains", "Failed to analyze redacted domains", func(v *RedactedDomainsAnalysis) {
results.redactedDomainsAnalysis = v
Expand Down