[repository-quality] 🎯 Repository Quality Improvement Report - Goroutine Lifecycle Hygiene

[github-actions[bot]]

Analysis Date: 2026-07-23
Focus Area: Goroutine Lifecycle Hygiene — Unjoined Goroutines, Timer Leaks & HTTP Server Timeout Gaps
Strategy Type: Custom
Custom Area: Yes — goroutine hygiene is a natural follow-on to recent context propagation and error chain work; 5 specific production issues identified across 4 files.

Executive Summary

The repository launches 6 bare goroutines in production code (plus 1 in WASM). While most are properly handled, three patterns recur that undermine reliability: (1) fire-and-forget goroutines that are never joined and can silently swallow panics or leave resources alive, (2) time.After timer allocations in hot-ish paths that are GC-delayed leaks, and (3) HTTP server structs created without read/write timeouts, leaving connections open indefinitely when clients misbehave.

The most critical gap is the WASM compile goroutine in cmd/gh-aw-wasm/main.go, which has no recover(): a panic inside doCompile will crash the entire WASM runtime and leave the JavaScript Promise unresolved — a hard hang for browser users. The CheckForUpdatesAsync function fires a goroutine with no join mechanism; combined with a time.After(100ms) timer (which leaks for ~channel GC cycle) this is a minor but unnecessary resource concern.

On the HTTP side, bootstrap_profile_github_app.go creates an http.Server with neither ReadHeaderTimeout, ReadTimeout, nor WriteTimeout, meaning a slow client can hold a goroutine thread indefinitely. The MCP HTTP server is better — it has ReadHeaderTimeout — but still lacks WriteTimeout and IdleTimeout.

Full Analysis Report

Focus Area: Goroutine Lifecycle Hygiene

Current State Assessment

Production goroutine launches (non-test, non-testdata):

File	Pattern	Joined?	recover()?	Issue
pkg/console/spinner.go:144	Animation loop	✅ WaitGroup	✅ implicit	OK
pkg/cli/compile_update_check.go:67	Background check	✅ buffered chan	✅ explicit	OK
pkg/cli/update_check.go:249	CheckForUpdatesAsync	❌ fire-and-forget	✅ explicit	No join
pkg/cli/docker_images.go:155	Docker pull	❌ fire-and-forget	✅ explicit	No join (by design)
pkg/cli/bootstrap_profile_github_app.go:211	HTTP server loop	✅ shutdown	❌ none	Server no timeouts
pkg/cli/bootstrap_profile_helpers.go:361	Browser process reaper	✅ cmd.Wait	N/A	OK
cmd/gh-aw-wasm/main.go:48	WASM compile	❌ Promise resolve	❌ NO RECOVER	Critical

Metrics Collected:

Metric	Value	Status
Bare goroutine launches (prod)	7	⚠️
Goroutines with no recover()	2 (wasm + http server)	❌
Goroutines with no join point	2	⚠️
time.After in non-deferred select	2	⚠️
HTTP servers missing read/write timeout	1/2	⚠️
errgroup usage	2 files (audit.go, mcp_inspect_inspector.go)	✅
WaitGroup usage	1 (spinner)	✅

Findings

Strengths

• errgroup.WithContext is used correctly in the two highest-concurrency code paths (audit and MCP inspector)
• compile_update_check.go goroutine uses a buffered channel close pattern, properly detected via ctx.Done()
• docker_images.go goroutine has explicit panic recovery and context-aware sleep
• spinner.go uses sync.WaitGroup correctly with deferred Done()
• The wgdonenotdeferred linter is present and catches new violations

Areas for Improvement

• [CRITICAL] WASM goroutine has no recover(): panic in doCompile leaves JS Promise permanently unresolved
• [HIGH] bootstrap_profile_github_app.go HTTP server has zero timeouts
• [MEDIUM] time.After(100ms) in CheckForUpdatesAsync — timer channel leaks until GC; use time.NewTimer + defer Stop()
• [MEDIUM] mcp_server_http.go has ReadHeaderTimeout but no WriteTimeout or IdleTimeout
• [LOW] No static analysis linter for fire-and-forget goroutine detection

Detailed Analysis

WASM Goroutine (cmd/gh-aw-wasm/main.go:48)

No recover() means any runtime panic (nil pointer, index out of range) inside doCompile will: (a) not call reject, leaving the Promise pending forever in the browser, and (b) crash the WASM instance. Fix: wrap body in defer func() { if r := recover(); r != nil { reject.Invoke(Error(fmt.Sprint(r))) } }().

Bootstrap HTTP Server (pkg/cli/bootstrap_profile_github_app.go:208)

No ReadHeaderTimeout, ReadTimeout, or WriteTimeout. The server is only reachable on loopback, but a misbehaving OAuth callback client could pin a goroutine for the full bootstrapProfileManifestTimeout duration. Should match the MCPServerHTTPTimeout pattern from mcp_server_http.go.

Timer Leak (pkg/cli/update_check.go:270)

When ctx.Done() fires first, the time.After channel is not released until its timer fires 100ms later. Fix: timer := time.NewTimer(100 * time.Millisecond); defer timer.Stop().

MCP HTTP Server WriteTimeout (pkg/cli/mcp_server_http.go:98)

ReadHeaderTimeout is set but WriteTimeout and IdleTimeout are absent. For streaming MCP responses WriteTimeout may be intentionally omitted, but IdleTimeout should be set to reclaim idle keep-alive connections.

---

🤖 Tasks for Copilot Agent

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

Improvement Tasks

Task 1: Add panic recovery to WASM compile goroutine

Priority: High
Estimated Effort: Small
Focus Area: Goroutine Lifecycle Hygiene

Description: The goroutine in cmd/gh-aw-wasm/main.go that calls doCompile has no recover(). A panic inside the compiler leaves the JavaScript Promise permanently unresolved, hanging the browser indefinitely. Add a deferred recover() that calls reject.Invoke with an error message derived from the panic value.

Acceptance Criteria:

• A defer func() { if r := recover(); ... }() is added as the first deferred call in the WASM goroutine body
• The recovery calls reject.Invoke(js.Global().Get("Error").New(fmt.Sprintf("internal panic: %v", r))) so the JS Promise rejects cleanly
• Existing tests pass (make test-unit)
• A comment explains why recover is needed in this specific context

Code Region: cmd/gh-aw-wasm/main.go (goroutine starting at line 48)

In `cmd/gh-aw-wasm/main.go`, inside the `go func()` that drives the JS Promise, add a deferred recover block as the very first defer so it wraps all subsequent execution. When a panic is caught, call `reject.Invoke(js.Global().Get("Error").New(fmt.Sprintf("internal panic: %v", r)))` and return. Add a brief comment explaining this prevents a WASM runtime crash that would leave the Promise unresolved. Do not change any other logic.

---

Task 2: Add HTTP timeouts to bootstrap GitHub App server

Priority: High
Estimated Effort: Small
Focus Area: Goroutine Lifecycle Hygiene / Security

Description: The temporary HTTP server in pkg/cli/bootstrap_profile_github_app.go for the GitHub App manifest flow has no read or write timeouts. Add ReadHeaderTimeout and WriteTimeout mirroring the values used by mcp_server_http.go.

Acceptance Criteria:

• http.Server struct gains at least ReadHeaderTimeout and WriteTimeout
• The timeout values are sourced from an existing exported constant or a new unexported constant — not hardcoded inline
• Existing tests pass

Code Region: pkg/cli/bootstrap_profile_github_app.go (lines 208–213)

In `pkg/cli/bootstrap_profile_github_app.go`, update the `http.Server` struct literal used for the GitHub App manifest registration flow to include `ReadHeaderTimeout` and `WriteTimeout`. Use `MCPServerHTTPTimeout` if accessible, or declare a new `const bootstrapHTTPServerTimeout = 30 * time.Second`. Do not change any handler logic or the existing `defer server.Shutdown(...)` pattern.

---

Task 3: Fix time.After timer leak in CheckForUpdatesAsync

Priority: Medium
Estimated Effort: Small
Focus Area: Goroutine Lifecycle Hygiene

Description: CheckForUpdatesAsync uses time.After(100 * time.Millisecond) inside a select. When ctx.Done() fires first, the timer channel is not GC'd until the timer fires. Replace with time.NewTimer + defer timer.Stop().

Acceptance Criteria:

• time.After(100 * time.Millisecond) replaced with timer := time.NewTimer(100 * time.Millisecond) and defer timer.Stop()
• The select case uses <-timer.C
• TestCheckForUpdatesAsync_ContextCancellation and related tests still pass

Code Region: pkg/cli/update_check.go (lines 268–277)

In `pkg/cli/update_check.go` inside `CheckForUpdatesAsync`, replace `time.After(100 * time.Millisecond)` with `timer := time.NewTimer(100 * time.Millisecond)`, add `defer timer.Stop()` immediately after, and change the select case to `case <-timer.C:`. No other logic should change.

---

Task 4: Add IdleTimeout to MCP HTTP server

Priority: Medium
Estimated Effort: Small
Focus Area: Goroutine Lifecycle Hygiene / Security

Description: pkg/cli/mcp_server_http.go sets ReadHeaderTimeout but not IdleTimeout. Idle keep-alive connections consume a goroutine indefinitely. Add IdleTimeout and document why WriteTimeout is intentionally omitted.

Acceptance Criteria:

• IdleTimeout field added to the http.Server struct
• A comment explains why WriteTimeout is intentionally omitted (streaming responses)
• IdleTimeout value is a named constant or multiple of MCPServerHTTPTimeout
• Existing tests pass

Code Region: pkg/cli/mcp_server_http.go (lines 98–105)

In `pkg/cli/mcp_server_http.go`, update `httpServer := &http.Server{...}` to include `IdleTimeout: 10 * MCPServerHTTPTimeout` (or a new `mcpHTTPIdleTimeout` constant). Add a comment explaining `WriteTimeout` is intentionally absent for streaming MCP responses. Do not change any other logic.

---

Task 5: Add goroutine leak linter for unjoined fire-and-forget goroutines

Priority: Low
Estimated Effort: Medium
Focus Area: Goroutine Lifecycle Hygiene

Description: Add a new goroutineleak analyzer in pkg/linters/goroutineleak/ that flags bare go func() calls where the enclosing function neither returns a channel nor passes a WaitGroup/errgroup reference into the goroutine closure.

Acceptance Criteria:

• New analyzer at pkg/linters/goroutineleak/goroutineleak.go registered in pkg/linters/doc.go and pkg/linters/linters.go
• Testdata with at least 3 want-diagnostic and 3 want-no-diagnostic cases
• pkg/linters/spec_test.go ContainsExpectedAnalyzers updated
• make test-unit passes

Code Region: pkg/linters/ (new subdirectory goroutineleak/)

Create a new custom Go static analysis linter in `pkg/linters/goroutineleak/goroutineleak.go`. The analyzer should flag `go func()` launches where the goroutine closure captures no `chan`, `*sync.WaitGroup`, or `*errgroup.Group` variable from the enclosing scope. Register in `pkg/linters/linters.go`, add to `doc.go`, update `ContainsExpectedAnalyzers` in `pkg/linters/spec_test.go`, and provide testdata. Follow patterns from `pkg/linters/timesleepnocontext/` and `pkg/linters/wgdonenotdeferred/`.

---

📊 Historical Context

Previous Focus Areas

Date	Focus Area	Type	Custom	Key Outcomes
2026-07-22	Output Stream Abstraction Gap	Custom	Y	167 os.Stdout refs; 5 tasks
2026-07-17	Error Chain Transparency & Non-Wrapping fmt.Errorf Gap	Custom	Y	788 non-wrapping Errorf calls; 5 tasks
2026-07-16	Options Struct Pattern Adoption for High-Arity Functions	Custom	Y	2 over-limit functions; 5 tasks
2026-07-15	Compile-Time Interface Satisfaction Assertions	Custom	Y	0 var _ guards; 4 tasks
2026-07-14	Logger Name Collision & Instantiation Hygiene	Custom	Y	3 duplicate logger names; 4 tasks

---

🎯 Recommendations

Immediate Actions (This Week)

1. Add recover() to WASM goroutine — Priority: High (browser hang risk)
2. Add HTTP timeouts to bootstrap GitHub App server — Priority: High (resource safety)

Short-term Actions (This Month)

1. Fix time.After timer leak in CheckForUpdatesAsync — Priority: Medium
2. Add IdleTimeout to MCP HTTP server — Priority: Medium

Long-term Actions (This Quarter)

1. Implement goroutineleak static analysis linter — Priority: Low

---

📈 Success Metrics

• Goroutines with no recover(): 2 → 0
• HTTP servers missing timeouts: 1 → 0
• time.After in non-deferred select: 2 → 0
• Fire-and-forget goroutines without lifecycle docs: 2 → 0

---

Next Steps

1. Review and prioritise the tasks above
2. Assign tasks to Copilot coding agent via planner agent
3. Track progress on improvement items
4. Re-evaluate goroutine hygiene in 30 days after linter is in place

---

References:

• §30010515808

Generated by Repository Quality Improvement Agent

Generated by ⚡ Repository Quality Improvement Agent · sonnet46 56.7 AIC · ⌖ 6.09 AIC · ⊞ 6.7K · ◷

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