Spinner background goroutine has no panic recovery, can crash CLI on terminal anomalies

[github-actions]

Summary

The spinner's render goroutine at pkg/console/spinner.go:140-143 calls s.program.Run() (charmbracelet/bubbletea) without a defer recover(). Every other long-running production goroutine in this codebase guards itself with defer recover() (see pkg/cli/update_check.go:275, pkg/cli/compile_update_check.go:69). A panic from inside the bubbletea program — which is plausible on unusual terminal conditions, broken TERM values, or signal-handling races — would propagate to the Go runtime and abort the entire gh aw process even though the spinner itself is purely cosmetic.

Severity

Medium — The spinner runs from compile, audit, and other long-running commands. A panic here would tear down a long-running compile or audit step that has already done meaningful work, with no recovery path. The probability of a panic from bubbletea is low but non-zero, and the cost of guarding against it is one extra defer block.

Evidence

Current code at pkg/console/spinner.go:128-145:

func (s *SpinnerWrapper) Start() {
    if s.enabled && s.program != nil {
        s.mu.Lock()
        if s.running {
            s.mu.Unlock()
            spinnerLog.Print("Spinner already running, skipping Start")
            return
        }
        s.running = true
        s.wg.Add(1)
        s.mu.Unlock()
        spinnerLog.Print("Starting spinner")
        go func() {
            defer s.wg.Done()
            _, _ = s.program.Run()  // <-- can panic; no recover
        }()
    }
}

Contrast with the protected pattern in pkg/cli/update_check.go:271-289, which is the convention this repo has converged on for fire-and-forget goroutines.

Recommendation

Add a defer recover() block. The fix should also reset s.running so a subsequent Start() is not silently rejected if the program crashed:

go func() {
    defer s.wg.Done()
    defer func() {
        if r := recover(); r != nil {
            spinnerLog.Printf("Panic in spinner program (recovered): %v", r)
            s.mu.Lock()
            s.running = false
            s.mu.Unlock()
        }
    }()
    _, _ = s.program.Run()
}()

Validation

• go test ./pkg/console/... (concurrency tests in spinner_test.go already exercise Start/Stop)
• Manually inject a panic in a tea.Model.View() mock and verify the parent process keeps running and the spinner resets cleanly
• go vet ./pkg/console/

Estimated Effort

Small — One function, ~8 line edit.

Sergo Run Context

• Strategy: concurrency-safety-and-resource-lifecycle
• Run ID: §25592031127

Generated by Sergo - Serena Go Expert · ● 16.9M · ◷

• expires on May 16, 2026, 4:55 AM UTC