Description
collectAuditAnalysisResults in pkg/cli/audit.go uses a raw sync.WaitGroup to fan out ~14 goroutines. This has two problems:
- Goroutine errors are swallowed — there's no way for the caller to distinguish success from silent failures
- When combined with context propagation (task for adding
ctx), errors from the errgroup context cancellation won't surface
The codebase already uses errgroup.WithContext correctly in pkg/cli/mcp_inspect_inspector.go, and golang.org/x/sync/errgroup is already a direct dependency.
Suggested Changes
- Replace
var wg sync.WaitGroup in collectAuditAnalysisResults with g, gctx := errgroup.WithContext(ctx)
- Change all
wg.Go(func() { ... }) callsites to g.Go(func() error { ...; return nil })
- Pass
gctx into all launch* helpers so cancellation propagates
- Add
import "golang.org/x/sync/errgroup" to audit.go
- Remove
sync import if no longer needed
Soft failures (goroutine I/O errors) should log and return nil to preserve current non-fatal behavior; hard cancellations surface via g.Wait().
Files Affected
pkg/cli/audit.go — collectAuditAnalysisResults and all launch* helpers (lines 656–805)
Success Criteria
errgroup.WithContext used in place of raw sync.WaitGroup
- All goroutines receive
gctx for cancellation
make build and make test pass
Source
Extracted from Audit Goroutine Context Blindness discussion #46848
Priority
Medium — improves error propagation and cancellability
🔍 Task mining by Discussion Task Miner - Code Quality Improvement Agent · 46.7 AIC · ⌖ 7.94 AIC · ⊞ 7K · ◷
Description
collectAuditAnalysisResultsinpkg/cli/audit.gouses a rawsync.WaitGroupto fan out ~14 goroutines. This has two problems:ctx), errors from theerrgroupcontext cancellation won't surfaceThe codebase already uses
errgroup.WithContextcorrectly inpkg/cli/mcp_inspect_inspector.go, andgolang.org/x/sync/errgroupis already a direct dependency.Suggested Changes
var wg sync.WaitGroupincollectAuditAnalysisResultswithg, gctx := errgroup.WithContext(ctx)wg.Go(func() { ... })callsites tog.Go(func() error { ...; return nil })gctxinto alllaunch*helpers so cancellation propagatesimport "golang.org/x/sync/errgroup"toaudit.gosyncimport if no longer neededSoft failures (goroutine I/O errors) should log and return nil to preserve current non-fatal behavior; hard cancellations surface via
g.Wait().Files Affected
pkg/cli/audit.go—collectAuditAnalysisResultsand alllaunch*helpers (lines 656–805)Success Criteria
errgroup.WithContextused in place of rawsync.WaitGroupgctxfor cancellationmake buildandmake testpassSource
Extracted from Audit Goroutine Context Blindness discussion #46848
Priority
Medium — improves error propagation and cancellability