Skip to content

fix(ui): stop spinner from showing previous phase's text - #903

Merged
cdalar merged 1 commit into
mainfrom
fix/spinner-stale-suffix
Jun 12, 2026
Merged

cdalar merged 1 commit into
mainfrom
fix/spinner-stale-suffix

Conversation

@cdalar

@cdalar cdalar commented Jun 12, 2026

Copy link
Copy Markdown
Owner

Problem

During onctl up, the VM IP line is shown twice:

✔ VM IP: 167.233.64.66
⢿  VM IP: 167.233.64.66

The bubbletea-based spinner wrapper (internal/ui/spinner.go) only syncs Suffix into the rendered model on Start()/Restart(), but the cmd code assigned s.Suffix after calling Start()/Restart() (the old briandowns/spinner pattern). So every phase's spinner rendered the previous phase's text — after printing ✔ VM IP: …, the spinner restarted for the cloud-init wait but kept displaying the stale VM IP: … suffix.

A second latent bug: SetSuffix() sent the updated model to the running program, but the tea model's Update() had no case to receive it, so mid-run suffix changes were silently dropped — which also left the deploy upload progress bar frozen.

Changes

  • internal/ui/spinner.go: add a suffixMsg case to the tea model's Update so SetSuffix actually updates a running spinner
  • cmd/create.go, cmd/deploy.go, cmd/ssh.go, cmd/destroy.go: set s.Suffix before Start()/Restart() at every call site
  • deploy's upload progress callback now uses SetSuffix() so the progress bar updates live
  • drop the pointless Restart()/Stop() pair around the VM IP println in create.go
  • add TestModelUpdateSuffixMsg verifying suffix changes reach the renderer

Verification

  • go build ./..., go vet clean
  • full test suite passes; go test -race ./internal/ui/... passes
  • golangci-lint run reports 0 issues
  • grep confirms no remaining Start/Restart-before-Suffix orderings

🤖 Generated with Claude Code

The bubbletea spinner wrapper only synced Suffix into the rendered
model on Start/Restart, but cmd code assigned Suffix after calling
Start/Restart (the old briandowns/spinner pattern), so the spinner
kept displaying the prior phase's text — e.g. 'VM IP: x.x.x.x'
appeared twice during onctl up while waiting for cloud-init.

- set Suffix before Start/Restart at all call sites
- handle suffix updates in the tea model so SetSuffix works on a
  running spinner (used by the deploy upload progress bar, which
  was frozen for the same reason)
- drop the pointless Restart/Stop pair around the VM IP println
@codecov

codecov Bot commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 20.00000% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 21.57%. Comparing base (0257702) to head (fde3d70).

Files with missing lines Patch % Lines
cmd/create.go 0.00% 10 Missing ⚠️
cmd/deploy.go 0.00% 7 Missing ⚠️
cmd/destroy.go 0.00% 2 Missing ⚠️
cmd/ssh.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #903      +/-   ##
==========================================
+ Coverage   21.40%   21.57%   +0.16%     
==========================================
  Files          41       41              
  Lines        4031     4032       +1     
==========================================
+ Hits          863      870       +7     
+ Misses       3059     3054       -5     
+ Partials      109      108       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cdalar
cdalar merged commit 065d866 into main Jun 12, 2026
15 checks passed
@cdalar
cdalar deleted the fix/spinner-stale-suffix branch August 19, 2026 13:53
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.

1 participant

⌂ 换网址