Skip to content

Recover spinner goroutine panics without wedging spinner state#31162

Merged
pelikhan merged 5 commits intomainfrom
copilot/fix-spinner-goroutine-panic-recovery
May 9, 2026
Merged

Recover spinner goroutine panics without wedging spinner state#31162
pelikhan merged 5 commits intomainfrom
copilot/fix-spinner-goroutine-panic-recovery

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 9, 2026

  • Inspect branch state, recent PR commits, and existing build command
  • Fetch full history and merge main into the PR branch
  • Recompile with the repository build command
  • Run targeted spinner validation after the merge
  • Reply on the PR comment with the addressing commit

Copilot AI and others added 3 commits May 9, 2026 05:21
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix spinner background goroutine panic recovery in CLI Recover spinner goroutine panics without wedging spinner state May 9, 2026
Copilot AI requested a review from pelikhan May 9, 2026 05:24
@pelikhan pelikhan marked this pull request as ready for review May 9, 2026 05:30
Copilot AI review requested due to automatic review settings May 9, 2026 05:30
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves resilience of the console spinner by recovering from panics inside the Bubble Tea render goroutine and ensuring spinner lifecycle state is reset, with a regression test to prevent the running flag from getting wedged.

Changes:

  • Add recover() handling around Program.Run() in the spinner goroutine.
  • Reset SpinnerWrapper.running on goroutine exit to allow subsequent Start() calls.
  • Add a unit test that simulates a Bubble Tea model panic during startup and verifies Start() can be called again.
Show a summary per file
File Description
pkg/console/spinner.go Adds panic recovery and resets running when the spinner goroutine exits.
pkg/console/spinner_test.go Adds a regression test to ensure spinner state is not wedged after a Bubble Tea panic.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Comment thread pkg/console/spinner.go
Comment on lines +142 to +146
defer func() {
s.mu.Lock()
s.running = false
s.mu.Unlock()
}()
@pelikhan
Copy link
Copy Markdown
Collaborator

pelikhan commented May 9, 2026

@copilot Merge main and recompile

…oroutine-panic-recovery

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 9, 2026

@copilot Merge main and recompile

Merged main in 4b91005 and rebuilt with make build. go test ./pkg/console/... and go vet ./pkg/console/... also passed after the merge.

@pelikhan pelikhan merged commit 9cfe937 into main May 9, 2026
@pelikhan pelikhan deleted the copilot/fix-spinner-goroutine-panic-recovery branch May 9, 2026 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

3 participants