[repository-quality] Repository Quality Improvement: Audit Goroutine Context Blindness & API Call Cancellability (2026-07-20)

[github-actions[bot]]

🎯 Repository Quality Improvement Report - Audit Goroutine Context Blindness & API Call Cancellability

Analysis Date: 2026-07-20
Focus Area: Audit Goroutine Context Blindness — collectAuditAnalysisResults drops caller ctx
Strategy Type: Custom
Custom Area: Yes — the audit analysis fan-out pattern makes network calls in goroutines that have no access to the caller's context.Context, so they cannot be cancelled. This is a concrete, repo-specific gap discovered through direct code inspection.

Executive Summary

AuditWorkflowRun receives a context.Context from the CLI (and honours it before and after analysis), but it silently drops it at the boundary to collectAuditAnalysisResults. That function fans out 13–15 goroutines using a raw sync.WaitGroup; none of those goroutines hold a reference to ctx. Among them, fetchJobDetailsWithCounts calls workflow.RunGHCombined — the context-free overload of a function that has a RunGHCombinedContext counterpart — making a live GitHub API call that cannot be cancelled by user interrupt, process signal, or deadline.

The project already demonstrates the correct pattern: pkg/cli/mcp_inspect_inspector.go uses errgroup.WithContext(ctx), and workflow.RunGHCombinedContext exists precisely to pass context through to the underlying exec.CommandContext. The gap is that audit analysis was built before those patterns were established and has never been back-ported.

Two secondary issues compound the primary one: collectAuditAnalysisResults does not propagate errors from goroutines to the caller (all goroutine failures are swallowed), and the auditAnalysisResults struct relies on undocumented field-ownership assumptions for goroutine-safety rather than explicit documentation or structural enforcement.

Full Analysis Report

Focus Area: Audit Goroutine Context Blindness & API Call Cancellability

Current State Assessment

Metrics Collected:

Metric	Value	Status
Goroutines launched by collectAuditAnalysisResults	~14	⚠️
Goroutines that receive ctx	0	❌
Goroutines making live GitHub API calls without ctx	1+ (fetchJobDetailsWithCounts)	❌
Files using errgroup.WithContext in production pkg/	1 (mcp_inspect_inspector.go)	✅
RunGHCombinedContext callers in pkg/cli/audit.go	0	❌
RunGHCombined (context-free) calls in pkg/cli/ audit cluster	1 direct + many indirect	⚠️
Goroutine errors propagated to caller	0	⚠️
//go:build !integration on audit_test.go	present	✅

Findings

Strengths

• workflow.RunGHCombinedContext(ctx, ...) already exists and is well-documented; migration is a one-line call-site change per function.
• errgroup (golang.org/x/sync) is already a direct dependency; no new deps needed.
• The existing runAuditAnalysis[T] generic helper encapsulates the goroutine dispatch pattern cleanly — adding ctx to it is a localised change that ripples to all 10+ call sites automatically.
• ensureAuditNotCancelled shows the author already thought about context cancellability, just not inside the concurrent analysis phase.

Areas for Improvement

• [High] collectAuditAnalysisResults drops the caller's ctx entirely; goroutines cannot respond to cancellation signals.
• [High] fetchJobDetailsWithCounts calls RunGHCombined (uses context.Background()), not RunGHCombinedContext; a slow GitHub API response blocks the entire audit indefinitely.
• [Medium] runAuditAnalysis[T] swallows all goroutine errors; there is no way for the caller to distinguish "all goroutines succeeded" from "5 goroutines silently failed".
• [Medium] auditAnalysisResults has no mutex and no documentation explaining that field-ownership prevents data races; a future refactor could introduce a race silently.
• [Low] collectAuditAnalysisResults signature does not accept ctx, meaning even if inner helpers were updated, the outer shell would still be context-blind without a signature change.

Detailed Analysis

Call graph from AuditWorkflowRun to the context gap:

AuditWorkflowRun(ctx, ...)
  └── collectAuditAnalysisResults(run, dir, verbose, ...)   ← ctx dropped here
        ├── launchCoreAuditAnalyses(&wg, ...)
        │     ├── launchJobDetailsAnalysis(&wg, ...)
        │     │     └── fetchJobDetailsWithCounts(runID, verbose)
        │     │           └── workflow.RunGHCombined(...)   ← uses context.Background()
        │     └── runAuditAnalysis(&wg, ..., fn)            × 5 goroutines
        ├── launchFirewallAuditAnalyses(&wg, ...)            × 4 goroutines
        └── launchSupplementalAuditAnalyses(&wg, ...)        × 4 goroutines

Correct pattern already in use (mcp_inspect_inspector.go):

g, gctx := errgroup.WithContext(ctx)
g.Go(func() error {
    // gctx passed to all blocking operations
    return doWork(gctx, ...)
})
if err := g.Wait(); err != nil { ... }

Existing RunGHCombinedContext that should be used:

// pkg/workflow/github_cli.go:224
func RunGHCombinedContext(ctx context.Context, spinnerMessage string, args ...string) ([]byte, error)

fetchJobDetailsWithCounts needs only to accept ctx context.Context and replace
workflow.RunGHCombined(...) with workflow.RunGHCombinedContext(ctx, ...).

---

🤖 Tasks for Copilot Agent

NOTE TO PLANNER AGENT: Split the following tasks into individual work items.

Improvement Tasks

Task 1: Add ctx to collectAuditAnalysisResults and its launch helpers

Priority: High
Estimated Effort: Medium
Focus Area: Goroutine Context Blindness

Description: Add ctx context.Context as the first parameter to collectAuditAnalysisResults, launchCoreAuditAnalyses, launchMetricsAnalysis, launchJobDetailsAnalysis, launchFirewallAuditAnalyses, launchFirewallAnalysis, and launchSupplementalAuditAnalyses in pkg/cli/audit.go. Pass through ctx from the call site in AuditWorkflowRun. This unblocks all follow-on context propagation tasks.

Acceptance Criteria:

• collectAuditAnalysisResults(ctx, run, ...) compiles and the single call site in audit.go:373 passes ctx
• All seven launch* helpers accept and forward ctx
• runAuditAnalysis[T] generic helper also accepts ctx context.Context as first parameter
• make build passes; no new linter errors

Code Region: pkg/cli/audit.go — functions collectAuditAnalysisResults, launchCoreAuditAnalyses, launchMetricsAnalysis, launchJobDetailsAnalysis, launchFirewallAuditAnalyses, launchFirewallAnalysis, launchSupplementalAuditAnalyses, runAuditAnalysis

In pkg/cli/audit.go, add `ctx context.Context` as the first parameter to `collectAuditAnalysisResults` (line 656), all seven `launch*` helpers (lines 668, 702, 716, 731, 750, 770), and the generic `runAuditAnalysis[T]` dispatcher (line 791). Update the single call site `collectAuditAnalysisResults(run, cfg.outputDir, cfg.verbose, ...)` at line 373 to pass `ctx`. Each function should forward `ctx` to its callees. Do not change any behaviour — this is a pure signature threading change. Run `make build` to verify.

---

Task 2: Replace RunGHCombined with RunGHCombinedContext in fetchJobDetailsWithCounts

Priority: High
Estimated Effort: Small
Focus Area: Goroutine Context Blindness

Description: fetchJobDetailsWithCounts in pkg/cli/logs_github_api.go (line 79) calls workflow.RunGHCombined, which internally uses context.Background(). After Task 1 adds ctx to launchJobDetailsAnalysis, thread that ctx all the way to the RunGHCombined call and replace it with workflow.RunGHCombinedContext(ctx, ...). This makes the GitHub API call cancellable.

Acceptance Criteria:

• fetchJobDetailsWithCounts(ctx context.Context, runID int64, verbose bool) signature updated
• Call site in launchJobDetailsAnalysis updated to pass ctx
• workflow.RunGHCombined(...) at line 85 replaced with workflow.RunGHCombinedContext(ctx, ...)
• make build passes

Code Region: pkg/cli/logs_github_api.go:79–120, pkg/cli/audit.go:716–730

In pkg/cli/logs_github_api.go, change `func fetchJobDetailsWithCounts(runID int64, verbose bool)` to accept `ctx context.Context` as the first parameter. Replace the `workflow.RunGHCombined("Fetching job details...", ...)` call on line 85 with `workflow.RunGHCombinedContext(ctx, "Fetching job details...", ...)`. Update the caller `launchJobDetailsAnalysis` in pkg/cli/audit.go to forward its `ctx` to `fetchJobDetailsWithCounts`. Run `make build` to verify.

---

Task 3: Migrate collectAuditAnalysisResults from sync.WaitGroup to errgroup.WithContext

Priority: Medium
Estimated Effort: Medium
Focus Area: Goroutine Context Blindness & Error Propagation

Description: Replace the raw sync.WaitGroup in collectAuditAnalysisResults with errgroup.WithContext(ctx) (golang.org/x/sync/errgroup, already a dependency). Pass the derived gctx into all launch* helpers so that a cancellation or deadline expiry on the original ctx stops all goroutines. Update runAuditAnalysis[T] to return error (or accept an errgroup.Group) so errors can surface. Since errgroup stops at first error, keep the current soft-failure behaviour by logging errors inside goroutines and returning nil to errgroup (non-fatal errors), while returning the goroutine context gctx error for hard cancellation.

Acceptance Criteria:

• golang.org/x/sync/errgroup import added to audit.go (already in go.mod)
• errgroup.WithContext used in collectAuditAnalysisResults
• All goroutines that make I/O calls receive gctx
• User interrupting gh aw audit terminates the goroutines promptly
• make build and make test pass

Code Region: pkg/cli/audit.go:656–805

In pkg/cli/audit.go, replace `var wg sync.WaitGroup` in `collectAuditAnalysisResults` with `g, gctx := errgroup.WithContext(ctx)`. Import `golang.org/x/sync/errgroup` (already in go.mod). Change all `wg.Go(func() { ... })` callsites to `g.Go(func() error { ...; return nil })`. Pass `gctx` instead of `ctx` into all `launch*` helpers so cancellation propagates. Remove the `sync` import from audit.go if no longer needed. After `g.Wait()`, discard the returned error (since all goroutines return nil on soft failures). Run `make build` and `make test` to verify.

---

Task 4: Document goroutine-safety invariants on auditAnalysisResults

Priority: Medium
Estimated Effort: Small
Focus Area: Code Documentation & Race Safety

Description: The auditAnalysisResults struct (line 329 of audit.go) is written by multiple concurrent goroutines without a mutex, which is safe only because each goroutine owns exactly one field and no field is shared. This invariant is invisible to future readers and maintainers. Add a comment on the struct declaration and a comment on each launch* function stating which fields it writes. Optionally, add a //go:build gated sync/atomic assertion or a -race-gated test.

Acceptance Criteria:

• auditAnalysisResults struct has a doc comment explaining the field-ownership concurrency model
• Each launch* helper function has a one-line comment naming which fields it exclusively writes
• No behaviour changes; make build passes

Code Region: pkg/cli/audit.go:329–360, pkg/cli/audit.go:668–790

In pkg/cli/audit.go, add a doc comment to the `auditAnalysisResults` struct (line 329) that says: "auditAnalysisResults holds output fields collected by concurrent goroutines. Each field is written by exactly one goroutine; no locking is required because field ownership is mutually exclusive." Add a short comment above each `launch*` helper listing the fields it writes to `results`, e.g. `// Writes: results.metrics`. This makes the goroutine safety model explicit for future maintainers. Run `make build` to verify.

---

Task 5: Extend RunGHCombinedContext adoption to other context-free callers in the audit cluster

Priority: Low
Estimated Effort: Medium
Focus Area: Context Propagation & API Cancellability

Description: Several other functions in the audit cluster currently call workflow.RunGHCombined without context: pkg/cli/logs_rate_limit.go:63 (analyzeRateLimits) and pkg/cli/logs_github_api.go (called from audit goroutines). After Tasks 1–2 establish ctx threading, identify all callers within the audit code path that use RunGHCombined and migrate them to RunGHCombinedContext. Use grep -rn "RunGHCombined\b" pkg/cli/ to enumerate candidates, then check which are reachable from collectAuditAnalysisResults.

Acceptance Criteria:

• grep -rn "RunGHCombined\b" pkg/cli/ shows no call reachable from the audit analysis fan-out that uses the context-free overload
• All migrated callers accept and forward ctx context.Context
• make build passes

Code Region: pkg/cli/logs_rate_limit.go, pkg/cli/logs_github_api.go, any other files reached from collectAuditAnalysisResults

After Tasks 1 and 2 are complete, run `grep -rn "RunGHCombined\b" pkg/cli/` to find remaining context-free callers. For each function reachable from `collectAuditAnalysisResults` (trace the call graph starting at `collectAuditAnalysisResults`), add `ctx context.Context` as the first parameter and replace `workflow.RunGHCombined(...)` with `workflow.RunGHCombinedContext(ctx, ...)`. Update all upstream callers to pass `ctx`. Run `make build` to verify. Focus only on paths reachable from the audit fan-out; do not change unrelated commands.

---

📊 Historical Context

Previous Focus Areas

Date	Focus Area	Type	Custom	Key Outcomes
2026-07-17	Error Chain Transparency & Non-Wrapping fmt.Errorf Gap	Custom	Y	788 non-wrapping fmt.Errorf calls identified
2026-07-16	Options Struct Pattern Adoption for High-Arity Functions	Custom	Y	2 functions above linter threshold; 21 at limit
2026-07-15	Compile-Time Interface Satisfaction Assertions	Custom	Y	Zero var _ Interface guards in 21 interfaces
2026-07-14	Logger Name Collision & Instantiation Hygiene	Custom	Y	3 duplicate logger names; 6 shadowing vars
2026-07-08	Deprecated API Alias Proliferation & Zombie Callsite Cleanup	Custom	Y	3 GetGeminiAPITarget deprecated callsites
2026-07-07	Environment Variable Access Fragmentation	Custom	Y	45 bare nolint suppressions; 6 duplicated vars
2026-07-06	Context-Blind Sleep Gap	Custom	Y	4 functions with 7 time.Sleep calls; 60-min block
2026-07-03	ADR-42150 dispatch-repository Key Canonicalization	Custom	Y	Draft ADR; 6 underscore-form usages
2026-07-02	Remote Fetch API Context Gap	Custom	Y	8 public wrappers strip caller context
2026-07-01	Codemod Registry Drift & Idempotency Coverage Gap	Custom	Y	20/62 IDs absent from tests

---

🎯 Recommendations

Immediate Actions (This Week)

1. Add ctx parameter threading to collectAuditAnalysisResults (Task 1) — Priority: High; zero behaviour change, pure mechanical refactor
2. Replace RunGHCombined with RunGHCombinedContext in fetchJobDetailsWithCounts (Task 2) — Priority: High; one-line change that fixes a real cancellability hole

Short-term Actions (This Month)

1. Migrate to errgroup.WithContext (Task 3) — Priority: Medium; structural improvement enabling proper error propagation
2. Document field-ownership concurrency model (Task 4) — Priority: Medium; prevents silent data-race introduction in future

Long-term Actions (This Quarter)

1. Full RunGHCombinedContext adoption across audit call paths (Task 5) — Priority: Low; comprehensive cleanup after the pattern is established

---

📈 Success Metrics

• Cancellable audit goroutines: 0/14 → 14/14
• RunGHCombined calls in audit fan-out: 1+ → 0
• Goroutine errors surfaced to caller: 0 → ≥1 (hard cancellation errors)
• auditAnalysisResults concurrency model documented: No → Yes

---

Next Steps

1. Review and prioritise the tasks above
2. Assign Tasks 1 and 2 to Copilot coding agent via planner agent (they are dependencies for Tasks 3 and 5)
3. Track progress on improvement items
4. Re-evaluate audit context propagation completeness in ~2 weeks

---

References:

• [§29746354208]

Generated by Repository Quality Improvement Agent

Generated by ⚡ Repository Quality Improvement Agent · 83.7 AIC · ⌖ 6.38 AIC · ⊞ 6.6K · ◷

• expires on Jul 21, 2026, 5:36 AM UTC-08:00

[github-actions[bot]]

This discussion was automatically closed because it expired on 2026-07-21T13:36:24.954Z.

Closed by Workflow