diff --git a/cli/common/ui/spinner.go b/cli/common/ui/spinner.go index 775eb97dc0..f805b3bc2a 100644 --- a/cli/common/ui/spinner.go +++ b/cli/common/ui/spinner.go @@ -12,20 +12,26 @@ import ( const ( spinnerFrameInterval = 80 * time.Millisecond + // Without a spinner a notice becomes a line that piles up instead of being redrawn; at the + // heartbeat cadence (10s) a 5-minute stall would print 30 lines, so it matches the 60-second + // cadence of compile's interim reports. + noticeLineInterval = 60 * time.Second ) var spinnerFrames = []string{"⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"} type TerminalSpinner struct { - writer io.Writer - Enabled bool - message string - frame int - done chan struct{} - stopped chan struct{} - stopOnce sync.Once - mutex sync.Mutex - stoppedFlag bool + writer io.Writer + Enabled bool + message string + frame int + done chan struct{} + stopped chan struct{} + stopOnce sync.Once + mutex sync.Mutex + stoppedFlag bool + now func() time.Time + lastNoticeAt time.Time } func NewToolSpinner(stderr io.Writer, showFeedback bool) *TerminalSpinner { @@ -41,15 +47,16 @@ func NewLaunchSpinner(stdout io.Writer, stderr io.Writer) *TerminalSpinner { // Maps connection-stage progress events to the contextual executing message // and passes display-ready payloads (such as the heartbeat main-thread stall -// notice) through to the spinner verbatim. Without this mapping the stall -// diagnosis built by the IPC client would never reach the user. +// notice) through verbatim as notices, which are written as lines even when no +// spinner is shown. Without this mapping the stall diagnosis built by the IPC +// client would never reach the user. func NewSpinnerProgressFunc(spinner *TerminalSpinner, executingMessage string) progress.Func { return func(event progress.Event) { if event.Stage == progress.StageConnected || event.Stage == progress.StageAccepted { spinner.Update(executingMessage) return } - spinner.Update(event.Message) + spinner.Notice(event.Message) } } @@ -60,6 +67,7 @@ func newSpinner(writer io.Writer, enabled bool, message string) *TerminalSpinner message: message, done: make(chan struct{}), stopped: make(chan struct{}), + now: time.Now, } if !enabled { @@ -87,8 +95,35 @@ func (spinner *TerminalSpinner) Update(message string) { spinner.render() } +// Notice shows a message that must reach the reader even when no spinner is +// shown, because agents read stderr through a pipe rather than a terminal. An +// enabled spinner shows it in place; otherwise it is written as a plain line, at +// most once per noticeLineInterval. +func (spinner *TerminalSpinner) Notice(message string) { + if spinner.Enabled { + spinner.Update(message) + return + } + + spinner.mutex.Lock() + defer spinner.mutex.Unlock() + if spinner.stoppedFlag { + return + } + now := spinner.now() + if !spinner.lastNoticeAt.IsZero() && now.Sub(spinner.lastNoticeAt) < noticeLineInterval { + return + } + spinner.lastNoticeAt = now + _, _ = fmt.Fprintln(spinner.writer, message) +} + func (spinner *TerminalSpinner) Stop() { if !spinner.Enabled { + // Marked stopped so a notice that arrives after the command ends writes no line. + spinner.mutex.Lock() + spinner.stoppedFlag = true + spinner.mutex.Unlock() return } diff --git a/cli/common/ui/spinner_test.go b/cli/common/ui/spinner_test.go index f3d058e41e..59000147e2 100644 --- a/cli/common/ui/spinner_test.go +++ b/cli/common/ui/spinner_test.go @@ -6,6 +6,7 @@ import ( "os" "strings" "testing" + "time" "github.com/hatayama/unity-cli-loop/common/progress" ) @@ -247,3 +248,86 @@ func TestSpinnerRenderAfterStopWritesNothing(t *testing.T) { t.Fatalf("render after Stop wrote output: %q", stderr.String()[len(afterStop):]) } } + +func TestDisabledSpinnerNoticeWritesOneLine(t *testing.T) { + // Verifies a notice on a disabled spinner reaches stderr as one plain line without terminal control codes. + var stderr bytes.Buffer + spinner := newSpinner(&stderr, false, "Connecting to Unity...") + + spinner.Notice("Unity main thread stuck 30s; check modal/long operation...") + + want := "Unity main thread stuck 30s; check modal/long operation...\n" + if stderr.String() != want { + t.Fatalf("disabled spinner notice = %q, want %q", stderr.String(), want) + } +} + +func TestDisabledSpinnerNoticeRateLimitsToOneLinePerMinute(t *testing.T) { + // Verifies notices without a spinner are written at most once per minute so a long stall does not pile up lines. + var stderr bytes.Buffer + spinner := newSpinner(&stderr, false, "Connecting to Unity...") + start := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + current := start + spinner.now = func() time.Time { return current } + + spinner.Notice("first") + current = start.Add(59 * time.Second) + spinner.Notice("second") + current = start.Add(60 * time.Second) + spinner.Notice("third") + + if stderr.String() != "first\nthird\n" { + t.Fatalf("rate-limited notices = %q, want %q", stderr.String(), "first\nthird\n") + } +} + +func TestDisabledSpinnerNoticeAfterStopWritesNothing(t *testing.T) { + // Verifies a notice that arrives after Stop on a disabled spinner is dropped. + var stderr bytes.Buffer + spinner := newSpinner(&stderr, false, "Connecting to Unity...") + spinner.Stop() + + spinner.Notice("late notice") + + if stderr.Len() != 0 { + t.Fatalf("notice after Stop wrote output: %q", stderr.String()) + } +} + +func TestEnabledSpinnerNoticeUpdatesInPlace(t *testing.T) { + // Verifies a notice on an enabled spinner replaces the spinner message instead of writing its own line. + var stderr bytes.Buffer + spinner := newSpinner(&stderr, true, "Connecting to Unity...") + + spinner.Notice("stall notice") + spinner.Stop() + + output := stderr.String() + if !strings.Contains(output, "stall notice") { + t.Fatalf("enabled spinner output did not include the notice: %q", output) + } + if strings.Contains(output, "stall notice\n") { + t.Fatalf("enabled spinner wrote the notice as its own line: %q", output) + } +} + +func TestSpinnerProgressFuncWritesStallNoticeAsLineWithoutSpinner(t *testing.T) { + // Verifies that without a spinner, connection events stay silent while the stall notice is written as a line. + var stderr bytes.Buffer + spinner := newSpinner(&stderr, false, "Connecting to Unity...") + progressFunc := NewSpinnerProgressFunc(spinner, "Executing get-logs...") + + progressFunc(progress.Event{Stage: progress.StageConnected}) + if stderr.Len() != 0 { + t.Fatalf("connection event wrote output without a spinner: %q", stderr.String()) + } + progressFunc(progress.Event{ + Stage: progress.StageMessage, + Message: "Unity main thread stuck 30s; check modal/long operation...", + }) + + want := "Unity main thread stuck 30s; check modal/long operation...\n" + if stderr.String() != want { + t.Fatalf("stall notice without spinner = %q, want %q", stderr.String(), want) + } +} diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index ef93adeb92..b040c259e1 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "23acd185b352d8863851088bbfb9623193a604c0" + "sharedInputsHash": "292bd6079496d0865f852146f0a3829d8492dfd0" } diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index 325526c139..be37531b5e 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "fb98448944d852bca377103bb80ee814c12e49d1" + "sharedInputsHash": "1db5b8eb574726147b628d2832ceac7a691426ba" }