From e3642b040a1e786b208bed2bcd15ef26331a04c0 Mon Sep 17 00:00:00 2001 From: Francisco Rodrigues Date: Sun, 5 Jul 2026 21:21:02 -0300 Subject: [PATCH] feat: show a busy spinner during slow TUI git operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dashboard runs several git operations that can take seconds — creating a worktree, cloning a project, fetching a project's remote branches. Until each finished the TUI kept the previous screen on display with no sign of progress, so a slow clone or fetch looked like the dashboard had hung. Worse, keystrokes typed during that window were handled against soon-to-be-stale state, and the operations cannot be cancelled once dispatched. A single shared busy label now drives a centered animated spinner. While the label is set, View short-circuits to the spinner and overrides the current screen without mutating it, so the underlying view reappears untouched once the operation completes. An enterBusy helper sets the label and batches a spinner tick alongside the dispatched command, keeping the three slow sites — worktree-create submit, clone, and remote-branch fetch — identical and the tick loop self-terminating once busy clears. Every result message clears the label: the success paths for clone, create and fetch, plus errMsg, which leaves the underlying screen unchanged as before and merely stops the spinner. While busy, all input except quit is swallowed, since no key should mutate state that the pending result relies on and the git operation cannot be aborted anyway; quit still works so the user is never trapped. This replaces the remote picker's own ad-hoc "Fetching…" placeholder and its loading flag, which only covered one of the three slow paths, with the shared spinner that covers all of them. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../plans/2026-07-04-tui-busy-spinner.md | 701 ++++++++++++++++++ .../2026-07-04-tui-busy-spinner-design.md | 152 ++++ internal/tui/form.go | 5 +- internal/tui/form_test.go | 18 + internal/tui/model.go | 70 +- internal/tui/model_test.go | 155 ++++ internal/tui/remote_picker.go | 5 - internal/tui/remote_picker_test.go | 9 - 8 files changed, 1097 insertions(+), 18 deletions(-) create mode 100644 docs/superpowers/plans/2026-07-04-tui-busy-spinner.md create mode 100644 docs/superpowers/specs/2026-07-04-tui-busy-spinner-design.md diff --git a/docs/superpowers/plans/2026-07-04-tui-busy-spinner.md b/docs/superpowers/plans/2026-07-04-tui-busy-spinner.md new file mode 100644 index 0000000..7444688 --- /dev/null +++ b/docs/superpowers/plans/2026-07-04-tui-busy-spinner.md @@ -0,0 +1,701 @@ +# TUI Busy Spinner Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Show a centered animated spinner while the TUI runs slow git operations (create worktree, clone, fetch remote branches) so the dashboard never looks hung. + +**Architecture:** Add a single shared `busy string` label plus a `spinner.Model` to the top-level `Model`. A non-empty `busy` makes `View()` short-circuit to a centered spinner, overriding (but never mutating) `m.screen`. The three slow dispatch sites set `busy` and batch in a spinner tick; each result message clears `busy`. A shared `enterBusy` helper carries the set-and-batch logic so every site is identical and unit-testable. + +**Tech Stack:** Go, Bubble Tea v2 (`charm.land/bubbletea/v2`), Bubbles v2 spinner (`charm.land/bubbles/v2/spinner`), Lip Gloss v2 (`charm.land/lipgloss/v2`). + +## Global Constraints + +- Module paths use the `charm.land/...` v2 namespace (already in `go.mod`: `charm.land/bubbles/v2 v2.0.0`). Import the spinner as `"charm.land/bubbles/v2/spinner"`. +- Aggregate test coverage must stay at or above 80% (`make check`). New code needs tests. +- `Model` methods use **value receivers** and return `(tea.Model, tea.Cmd)` or `tea.Cmd`; mutating a field means assigning on the local `m` copy and returning it. Follow this — do not switch to pointer receivers. +- Do not change existing behavior on error: an `errMsg` today leaves `m.screen` unchanged and shows the error via `statusMsg`. Keep that; only add `busy = ""`. +- Run tests with the package command: `go test ./internal/tui/...`. + +--- + +## File Structure + +- `internal/tui/model.go` — add `spinner`/`busy` fields to `Model`; init spinner in `NewModel`; add `enterBusy` helper; handle `spinner.TickMsg`; swallow keys while busy; clear `busy` on result messages; render the busy screen in `View`; set `busy` in `startRemotePicker`. +- `internal/tui/actions.go` — set `busy` at the clone dispatch (via `enterBusy`) inside the `updateList` clone case. (The clone case lives in `model.go:289`; `cloneAction` itself stays unchanged.) +- `internal/tui/form.go` — set `busy` when a completed form is submitted (via `enterBusy`). +- `internal/tui/remote_picker.go` — remove the now-unused `loading` field and its `View()` branch (the shared busy screen replaces it). +- `internal/tui/model_test.go`, `internal/tui/form_test.go`, `internal/tui/remote_picker_test.go` — tests for each behavior. + +--- + +## Task 1: Busy state scaffolding — field, init, tick loop, key swallow + +Adds the `spinner`/`busy` state, the `enterBusy` helper, spinner tick handling, and the "swallow input while busy except quit" guard. No dispatch site sets `busy` yet, so behavior is unchanged until Task 4 — this task is verified purely through unit tests on the new primitives. + +**Files:** +- Modify: `internal/tui/model.go` (imports; `Model` struct near line 55; `NewModel` near line 121; `Update` near line 173) +- Test: `internal/tui/model_test.go` + +**Interfaces:** +- Produces: + - `Model.busy string` — non-empty ⇒ busy screen active. + - `Model.spinner spinner.Model`. + - `func (m Model) enterBusy(label string, cmd tea.Cmd) (Model, tea.Cmd)` — sets `m.busy = label` and returns `m` with `tea.Batch(cmd, m.spinner.Tick)`. + +- [ ] **Step 1: Add the spinner import** + +In `internal/tui/model.go`, add to the `charm.land/bubbles/v2` import group (after the `list` import on line 10): + +```go + "charm.land/bubbles/v2/list" + "charm.land/bubbles/v2/spinner" +``` + +- [ ] **Step 2: Add the struct fields** + +In `internal/tui/model.go`, in the `Model` struct, add after the `statusMsg` field (line 80): + +```go + // Status message for async operations. + statusMsg string + + // busy is a non-empty label while a blocking git operation is in + // flight. When set, View renders a centered spinner + label, + // overriding m.screen (which is left unchanged). Cleared by the + // operation's result message. + busy string + + // spinner animates the busy screen. It only ticks while busy != "". + spinner spinner.Model +``` + +- [ ] **Step 3: Initialize the spinner in NewModel** + +In `internal/tui/model.go`, in `NewModel`, add before the `m := Model{` literal (line 121): + +```go + sp := spinner.New(spinner.WithSpinner(spinner.MiniDot)) +``` + +Then add `spinner: sp,` to the `Model{...}` literal, right after `help: h,`: + +```go + list: l, + keys: keys, + help: h, + spinner: sp, + cfg: cfg, +``` + +- [ ] **Step 4: Write the failing test for enterBusy and the tick loop** + +In `internal/tui/model_test.go`, add: + +```go +func TestEnterBusy_setsLabelAndBatchesTick(t *testing.T) { + m := Model{} + called := false + cmd := func() tea.Msg { called = true; return nil } + + m2, batched := m.enterBusy("Working…", cmd) + + if m2.busy != "Working…" { + t.Errorf("busy = %q, want %q", m2.busy, "Working…") + } + if batched == nil { + t.Fatal("enterBusy returned nil cmd, want batched cmd") + } + _ = called // cmd is not executed here; we only assert it was batched (non-nil). +} + +func TestUpdate_spinnerTickIgnoredWhenNotBusy(t *testing.T) { + m := Model{busy: ""} + + _, cmd := m.Update(spinner.TickMsg{}) + + if cmd != nil { + t.Error("spinner.TickMsg while not busy should not re-issue a tick") + } +} + +func TestUpdate_spinnerTickAdvancesWhenBusy(t *testing.T) { + m := Model{busy: "Working…", spinner: spinner.New()} + + _, cmd := m.Update(spinner.TickMsg{}) + + if cmd == nil { + t.Error("spinner.TickMsg while busy should re-issue a tick") + } +} + +func TestUpdate_swallowsKeysWhileBusy(t *testing.T) { + m := Model{busy: "Working…", screen: screenList, keys: defaultKeyMap()} + m.list = newList(nil) + + // A non-quit key is a no-op while busy. + updated, cmd := m.Update(tea.KeyPressMsg(tea.Key{Code: 'n', Text: "n"})) + um := updated.(Model) + if cmd != nil { + t.Error("non-quit key while busy should return nil cmd") + } + if um.screen != screenList { + t.Errorf("screen changed to %d while busy, want unchanged", um.screen) + } + + // Quit still quits. + _, quitCmd := m.Update(tea.KeyPressMsg(tea.Key{Code: 'q', Text: "q"})) + if quitCmd == nil { + t.Error("quit key while busy should return a cmd") + } +} +``` + +- [ ] **Step 5: Run the tests to verify they fail** + +Run: `go test ./internal/tui/ -run 'TestEnterBusy|TestUpdate_spinnerTick|TestUpdate_swallowsKeysWhileBusy' -v` +Expected: FAIL to compile (`m.enterBusy` undefined; `spinner.TickMsg` not handled). + +- [ ] **Step 6: Add the enterBusy helper** + +In `internal/tui/model.go`, add after `syncProfileKeyEnabled` (after line 150): + +```go +// enterBusy sets the busy label and batches the given command with a spinner +// tick, so the spinner animates while cmd runs. m.screen is left unchanged; +// View short-circuits to the busy screen while busy != "". +func (m Model) enterBusy(label string, cmd tea.Cmd) (Model, tea.Cmd) { + m.busy = label + return m, tea.Batch(cmd, m.spinner.Tick) +} +``` + +- [ ] **Step 7: Add the key-swallow guard and spinner tick handling in Update** + +In `internal/tui/model.go`, at the very top of `Update` (before the `switch msg := msg.(type)` on line 174): + +```go +func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + // While a blocking operation is in flight, swallow all input except + // quit. The screen underneath is hidden by the busy view, and the git + // operation cannot be cancelled mid-flight, so other keys must not + // mutate state that the pending result message will rely on. + if kp, ok := msg.(tea.KeyPressMsg); ok && m.busy != "" { + if key.Matches(kp, m.keys.Quit) { + return m, tea.Quit + } + return m, nil + } + + switch msg := msg.(type) { +``` + +Then add a `spinner.TickMsg` case inside that type switch, right after the `case tickMsg:` block (after line 187): + +```go + case spinner.TickMsg: + // Only advance (and thus re-schedule) while busy, so the tick + // loop stops cleanly once the operation completes. + if m.busy == "" { + return m, nil + } + var cmd tea.Cmd + m.spinner, cmd = m.spinner.Update(msg) + return m, cmd +``` + +- [ ] **Step 8: Run the tests to verify they pass** + +Run: `go test ./internal/tui/ -run 'TestEnterBusy|TestUpdate_spinnerTick|TestUpdate_swallowsKeysWhileBusy' -v` +Expected: PASS (4 tests). + +- [ ] **Step 9: Commit** + +```bash +git add internal/tui/model.go internal/tui/model_test.go +git commit -m "feat(tui): add busy state, spinner tick loop, and input guard" +``` + +--- + +## Task 2: Render the centered busy screen + +Makes `View()` short-circuit to a centered spinner + label when `busy != ""`. + +**Files:** +- Modify: `internal/tui/model.go` (`View` near line 318) +- Test: `internal/tui/model_test.go` + +**Interfaces:** +- Consumes: `Model.busy`, `Model.spinner` (Task 1). + +- [ ] **Step 1: Write the failing test** + +In `internal/tui/model_test.go`, add: + +```go +func TestView_rendersBusyScreen(t *testing.T) { + m := Model{busy: "Creating worktree…", spinner: spinner.New(), width: 40, height: 10} + m.list = newList(nil) + + v := m.View() + + if !strings.Contains(v.String(), "Creating worktree…") { + t.Errorf("busy view should contain the label; got:\n%s", v.String()) + } +} + +func TestView_normalWhenNotBusy(t *testing.T) { + m := Model{busy: "", screen: screenList, spinner: spinner.New(), width: 40, height: 10} + m.list = newList(nil) + m.help = help.New() + m.keys = defaultKeyMap() + + v := m.View() + + if strings.Contains(v.String(), "Creating worktree…") { + t.Error("non-busy view should not contain a busy label") + } +} +``` + +If `strings` or `help` is not already imported in the test file, add `"strings"` and `"charm.land/bubbles/v2/help"` to its imports. + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `go test ./internal/tui/ -run 'TestView_rendersBusyScreen|TestView_normalWhenNotBusy' -v` +Expected: FAIL — `TestView_rendersBusyScreen` fails because the busy label is not rendered. + +- [ ] **Step 3: Add the busy branch to View** + +In `internal/tui/model.go`, at the top of `View()` (before `var content string` on line 319): + +```go +func (m Model) View() tea.View { + if m.busy != "" { + body := lipgloss.Place( + m.width, m.height, + lipgloss.Center, lipgloss.Center, + m.spinner.View()+" "+m.busy, + ) + v := tea.NewView(body) + v.AltScreen = true + return v + } + + var content string +``` + +`lipgloss` is already imported (`charm.land/lipgloss/v2`). + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `go test ./internal/tui/ -run 'TestView_rendersBusyScreen|TestView_normalWhenNotBusy' -v` +Expected: PASS (2 tests). + +- [ ] **Step 5: Commit** + +```bash +git add internal/tui/model.go internal/tui/model_test.go +git commit -m "feat(tui): render centered busy spinner screen" +``` + +--- + +## Task 3: Clear busy on every result message + +Clears `busy` when each slow operation completes, so the busy screen disappears and the underlying screen (list, or picker with its loaded list) renders again. + +**Files:** +- Modify: `internal/tui/model.go` (`errMsg` ~line 218, `cloneDoneMsg` ~line 231, `worktreeCreatedMsg` ~line 235, `remoteBranchesMsg` ~line 244 handlers) +- Test: `internal/tui/model_test.go` + +**Interfaces:** +- Consumes: `Model.busy` (Task 1). + +- [ ] **Step 1: Write the failing test** + +In `internal/tui/model_test.go`, add: + +```go +func TestUpdate_resultMessagesClearBusy(t *testing.T) { + tests := []struct { + name string + msg tea.Msg + }{ + {"errMsg", errMsg{err: errors.New("boom")}}, + {"cloneDoneMsg", cloneDoneMsg{project: "myapp"}}, + {"worktreeCreatedMsg", worktreeCreatedMsg{project: "myapp", branch: "feat"}}, + {"remoteBranchesMsg", remoteBranchesMsg{project: "myapp"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := newModelForResultTest() + m.busy = "Working…" + + updated, _ := m.Update(tt.msg) + + if updated.(Model).busy != "" { + t.Errorf("%s did not clear busy", tt.name) + } + }) + } +} + +// newModelForResultTest builds a minimal Model that can absorb result +// messages without a nil-pointer (refreshCmd/list access after clearing busy). +func newModelForResultTest() Model { + m := Model{screen: screenList} + m.list = newList(nil) + m.cfg = &config.Config{} + return m +} +``` + +If `errors` is not already imported in the test file, add `"errors"`. + +Note on `worktreeCreatedMsg`: with `attach == false` (the default here) its handler already sets `m.screen = screenList`, `m.form = nil`, and returns `m.refreshCmd()` — `refreshCmd` reads `m.wtSvc`/`m.sesSvc`/`m.projSvc` but only inside the returned closure, which the test does not execute, so nil services are fine. + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `go test ./internal/tui/ -run TestUpdate_resultMessagesClearBusy -v` +Expected: FAIL — busy is not cleared for any of the four messages. + +- [ ] **Step 3: Clear busy in each result handler** + +In `internal/tui/model.go`: + +In the `errMsg` case (line 218), add `m.busy = ""` at the top of the case: + +```go + case errMsg: + m.busy = "" + if msg.err != nil { + m.statusMsg = msg.err.Error() + } + return m, nil +``` + +In the `cloneDoneMsg` case (line 231): + +```go + case cloneDoneMsg: + m.busy = "" + m.statusMsg = fmt.Sprintf("Cloned %s", msg.project) + return m, m.refreshCmd() +``` + +In the `worktreeCreatedMsg` case (line 235), add `m.busy = ""` at the top: + +```go + case worktreeCreatedMsg: + m.busy = "" + m.statusMsg = fmt.Sprintf("Created %s/%s", msg.project, msg.branch) + m.screen = screenList + m.form = nil +``` + +In the `remoteBranchesMsg` case (line 244), add `m.busy = ""` at the top: + +```go + case remoteBranchesMsg: + m.busy = "" + if m.remotePicker != nil && m.remotePicker.project == msg.project { +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `go test ./internal/tui/ -run TestUpdate_resultMessagesClearBusy -v` +Expected: PASS (4 subtests). + +- [ ] **Step 5: Commit** + +```bash +git add internal/tui/model.go internal/tui/model_test.go +git commit -m "feat(tui): clear busy state when slow operations complete" +``` + +--- + +## Task 4: Set busy at the three dispatch sites and drop the picker's loading path + +Wires `enterBusy` into the create-worktree submit, clone, and remote-picker-open paths, and removes the remote picker's now-redundant `loading` field. + +**Files:** +- Modify: `internal/tui/model.go` (`updateList` clone case ~line 289; `startRemotePicker` ~line 578) +- Modify: `internal/tui/form.go` (`updateForm` ~line 212) +- Modify: `internal/tui/remote_picker.go` (struct, `newRemotePicker`, `setBranches`, `View`) +- Test: `internal/tui/model_test.go`, `internal/tui/form_test.go`, `internal/tui/remote_picker_test.go` + +**Interfaces:** +- Consumes: `Model.enterBusy` (Task 1), `Model.cloneAction() tea.Cmd` (unchanged), `Model.fetchRemoteBranchesCmd(project string) tea.Cmd` (unchanged), `formModel.submit() tea.Cmd` (unchanged). + +- [ ] **Step 1: Write the failing tests** + +In `internal/tui/model_test.go`, add: + +```go +func TestUpdate_cloneSetsBusy(t *testing.T) { + m := Model{screen: screenList, keys: defaultKeyMap(), spinner: spinner.New()} + m.list = newList(toListItems([]Item{ + {Project: "myapp", Group: groupProject, Cloned: false}, + })) + m.cfg = &config.Config{Projects: map[string]config.ProjectConfig{"myapp": {}}} + + updated, cmd := m.Update(tea.KeyPressMsg(tea.Key{Code: 'c', Text: "c"})) + um := updated.(Model) + + if um.busy != "Cloning myapp…" { + t.Errorf("busy = %q, want %q", um.busy, "Cloning myapp…") + } + if cmd == nil { + t.Error("clone should return a batched cmd") + } +} + +func TestStartRemotePicker_setsBusy(t *testing.T) { + m := Model{screen: screenList, spinner: spinner.New()} + m.list = newList(toListItems([]Item{ + {Project: "myapp", Group: groupProject, Cloned: true}, + })) + m.cfg = &config.Config{} + + updated, cmd := m.startRemotePicker() + um := updated.(Model) + + if um.busy != "Fetching remote branches…" { + t.Errorf("busy = %q, want %q", um.busy, "Fetching remote branches…") + } + if um.screen != screenRemotePicker { + t.Errorf("screen = %d, want screenRemotePicker", um.screen) + } + if cmd == nil { + t.Error("startRemotePicker should return a batched cmd") + } +} +``` + +In `internal/tui/form_test.go`, add a negative test (a still-editing form must not set busy): + +```go +func TestUpdateForm_nonCompletedDoesNotSetBusy(t *testing.T) { + ctx := formContext{project: "api", baseBranch: "develop"} + cfg := &config.Config{ + Projects: map[string]config.ProjectConfig{"api": {}}, + Agents: map[string]config.AgentConfig{"claude": {Cmd: "claude"}}, + } + m := Model{screen: screenForm, spinner: spinner.New(), cfg: cfg} + m.form = newFormModel(ctx, cfg, nil) + m.form.Init() + + updated, _ := m.updateForm(tea.KeyPressMsg(tea.Key{Code: 'a', Text: "a"})) + + if updated.(Model).busy != "" { + t.Errorf("busy = %q, want empty for a non-completed form", updated.(Model).busy) + } +} +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `go test ./internal/tui/ -run 'TestUpdate_cloneSetsBusy|TestStartRemotePicker_setsBusy|TestUpdateForm_nonCompletedDoesNotSetBusy' -v` +Expected: FAIL — `TestUpdate_cloneSetsBusy` and `TestStartRemotePicker_setsBusy` fail because `busy` stays empty. (`TestUpdateForm_nonCompletedDoesNotSetBusy` may already pass; it guards against regressions in Step 4.) + +- [ ] **Step 3: Set busy in the clone case** + +In `internal/tui/model.go`, in `updateList`, replace the clone case (lines 289-290): + +```go + case key.Matches(msg, m.keys.Clone): + return m, m.cloneAction() +``` + +with: + +```go + case key.Matches(msg, m.keys.Clone): + cmd := m.cloneAction() + if cmd == nil { + return m, nil + } + // cloneAction returned non-nil ⇒ selection is a valid + // uncloned project, so selectedItem() is non-nil. + sel := m.selectedItem() + m, cmd = m.enterBusy(fmt.Sprintf("Cloning %s…", sel.Project), cmd) + return m, cmd +``` + +- [ ] **Step 4: Set busy in the form submit path** + +In `internal/tui/form.go`, in `updateForm`, replace the completed branch (lines 212-214): + +```go + if m.form.completed() { + return m, m.form.submit() + } +``` + +with: + +```go + if m.form.completed() { + // Reassign m (do not shadow it with :=) so the linter stays quiet. + var submitCmd tea.Cmd + m, submitCmd = m.enterBusy("Creating worktree…", m.form.submit()) + return m, submitCmd + } +``` + +- [ ] **Step 5: Set busy in startRemotePicker** + +In `internal/tui/model.go`, in `startRemotePicker`, replace the body after the guard (lines 583-585): + +```go + m.remotePicker = newRemotePicker(sel.Project, m.cfg, m.tmuxClient) + m.screen = screenRemotePicker + return m, m.fetchRemoteBranchesCmd(sel.Project) +``` + +with: + +```go + m.remotePicker = newRemotePicker(sel.Project, m.cfg, m.tmuxClient) + m.screen = screenRemotePicker + m, cmd := m.enterBusy("Fetching remote branches…", m.fetchRemoteBranchesCmd(sel.Project)) + return m, cmd +``` + +- [ ] **Step 6: Remove the picker's loading path** + +In `internal/tui/remote_picker.go`: + +Remove the `loading bool` field from `remotePickerModel`: + +```go +type remotePickerModel struct { + list list.Model + project string + errText string + cfg *config.Config + tmuxClient *tmux.Client +} +``` + +In `newRemotePicker`, drop `loading: true,` from the returned literal: + +```go + return &remotePickerModel{ + list: l, + project: project, + cfg: cfg, + tmuxClient: tmuxClient, + } +``` + +In `setBranches`, drop the `p.loading = false` line: + +```go +func (p *remotePickerModel) setBranches(branches []worktree.RemoteBranch) { + items := make([]list.Item, len(branches)) + for i, b := range branches { + items[i] = remoteBranchItem{rb: b} + } + p.list.SetItems(items) +} +``` + +In `View`, remove the `case p.loading:` branch: + +```go +func (p *remotePickerModel) View() string { + switch { + case p.errText != "": + return "Error: " + p.errText + "\n\nEsc: cancel" + case len(p.list.Items()) == 0: + return "No remote branches found (try again after pushing/fetching).\n\nEsc: cancel" + default: + return p.list.View() + } +} +``` + +- [ ] **Step 7: Fix the two tests referencing `loading`** + +Two tests in `internal/tui/remote_picker_test.go` assert on the removed `loading` field. Edit them: + +In `TestRemotePicker_setAndSelect`, remove both `loading` assertions, keeping the branch-setting and selection checks: + +```go +func TestRemotePicker_setAndSelect(t *testing.T) { + p := newRemotePicker("myapp", &config.Config{}, nil) + p.setBranches([]worktree.RemoteBranch{ + {Remote: "origin", Branch: "feat-x", Ref: "origin/feat-x"}, + {Remote: "origin", Branch: "fix-y", Ref: "origin/fix-y"}, + }) + rb, ok := p.selected() + if !ok || rb.Ref != "origin/feat-x" { + t.Errorf("selected = %+v ok=%v, want origin/feat-x", rb, ok) + } +} +``` + +In `TestRemoteBranchesMsg_populatesPicker`, remove the `loading` assertion (the busy-clearing behavior is covered by `TestUpdate_resultMessagesClearBusy`), keeping the item-count check: + +```go +func TestRemoteBranchesMsg_populatesPicker(t *testing.T) { + m := Model{screen: screenRemotePicker, remotePicker: newRemotePicker("myapp", &config.Config{}, nil)} + updated, _ := m.Update(remoteBranchesMsg{ + project: "myapp", + branches: []worktree.RemoteBranch{{Remote: "origin", Branch: "feat-x", Ref: "origin/feat-x"}}, + }) + mm := updated.(Model) + if got := len(mm.remotePicker.list.Items()); got != 1 { + t.Errorf("items = %d, want 1", got) + } +} +``` + +No other test references `loading`. If the build still fails, run `go test ./internal/tui/ 2>&1 | head -30` and fix the reported line. + +- [ ] **Step 8: Run the full package tests to verify they pass** + +Run: `go test ./internal/tui/... -v 2>&1 | tail -30` +Expected: PASS, including `TestUpdate_cloneSetsBusy`, `TestStartRemotePicker_setsBusy`, `TestUpdateForm_nonCompletedDoesNotSetBusy`. + +- [ ] **Step 9: Commit** + +```bash +git add internal/tui/model.go internal/tui/form.go internal/tui/remote_picker.go internal/tui/model_test.go internal/tui/form_test.go internal/tui/remote_picker_test.go +git commit -m "feat(tui): show busy spinner during create, clone, and remote fetch" +``` + +--- + +## Task 5: Full verification + +**Files:** none (verification only). + +- [ ] **Step 1: Run the full check suite** + +Run: `make check` +Expected: coverage ≥ 80%, integration tests, lint, and build all pass. If coverage dropped below 80%, add a focused test for any uncovered new branch (e.g. the `View` busy branch or the `enterBusy` helper) and re-run. + +- [ ] **Step 2: Manual smoke (optional but recommended)** + +Run: `make install` then `ch` in a project with at least one uncloned project and one clonable remote. Press `c` (clone), `n`→submit (create), and `r` (remote fetch); confirm each shows a centered animated spinner that clears on completion, and that keys do nothing (except `q`) while the spinner is up. + +- [ ] **Step 3: Commit any coverage follow-up** + +```bash +git add -A +git commit -m "test(tui): cover busy spinner branches" +``` + +(Skip if Step 1 passed with no new tests needed.) + +--- + +## Self-Review Notes + +- **Spec coverage:** Shared busy state (Task 1); centered busy screen View (Task 2); all three transitions set busy (Task 4); all four result messages clear busy (Task 3); remote picker `loading` folded into shared state (Task 4); keys swallowed except quit (Task 1); spinner tick loop stops when not busy (Task 1); table-driven tests, no integration tests (Tasks 1–4). +- **Type consistency:** `enterBusy(label string, cmd tea.Cmd) (Model, tea.Cmd)` and `busy string` / `spinner spinner.Model` are used identically across all tasks. `cloneAction() tea.Cmd` and `fetchRemoteBranchesCmd(project string) tea.Cmd` keep their existing signatures. +- **Behavior preserved on error:** the `errMsg` handler only adds `busy = ""`; it does not change `m.screen`, matching today's behavior (a known cosmetic quirk where a failed create leaves the completed form on screen is intentionally left as-is — out of scope). diff --git a/docs/superpowers/specs/2026-07-04-tui-busy-spinner-design.md b/docs/superpowers/specs/2026-07-04-tui-busy-spinner-design.md new file mode 100644 index 0000000..ae8469a --- /dev/null +++ b/docs/superpowers/specs/2026-07-04-tui-busy-spinner-design.md @@ -0,0 +1,152 @@ +# TUI busy spinner for slow transitions + +## Problem + +Creating a worktree from the TUI can take several seconds. Since commit +`37f0884`, every creation path freshens its source with a network `fetch` +before branching, and the tracking path fetches a remote branch outright. +Per-project hooks run after creation and can be slow too. During this whole +window the dashboard shows the frozen, completed form with no feedback, so the +UI looks hung. + +The same is true of two other slow transitions: + +- The remote-branch picker (`r`) fetches all remotes. It shows a static + `"Fetching remote branches…"` string that never animates, so it also reads as + stuck. +- The clone action (`c`) does network work and only sets a `statusMsg`. + +## Goal + +Give a clear in-progress indication during these slow transitions so the UI +never looks hung. A single animated spinner, shown on a dedicated centered busy +screen, covering all three transitions with one shared mechanism. + +Non-goals: phased/step-by-step progress (fetch vs. create vs. hooks), +cancellation of in-flight git operations, and progress percentages. A generic +animated spinner is enough to solve the "looks hung" problem; staged progress +can be a later increment if ever justified. + +## Architecture: one shared busy state + +The slow work already runs off the UI goroutine — `form.go:submit()`, +`cloneAction`, and the remote-fetch command each return a `tea.Cmd` that runs to +completion and emits a result message. The gap is purely presentational: nothing +is shown while the command is in flight. + +Add a single reusable busy indicator to the top-level `Model` rather than three +ad-hoc ones: + +```go +// in Model +spinner spinner.Model // charm.land/bubbles/v2/spinner +busy string // non-empty label ⇒ busy screen is active +``` + +- `busy == ""` → normal rendering (list / form / pickers), unchanged. +- `busy != ""` → `View()` short-circuits to a centered `" "`, + overriding whatever `m.screen` is. + +The underlying `m.screen` is **never changed** by the busy state. When the result +message clears `busy`, the correct screen renders again automatically — the list +after a create/clone, or the remote picker showing its now-loaded list. This +keeps the "which screen do I return to?" logic trivial: there is nothing to +remember. + +`busy` is a plain label string rather than a bool so the same screen can say +`"Creating worktree…"`, `"Cloning myapp…"`, or `"Fetching remote branches…"`. + +## Data flow: set on dispatch, clear on result + +| Trigger | Sets `busy` to | Cleared by | +|---|---|---| +| Form submit (`submit()`) | `"Creating worktree…"` | `worktreeCreatedMsg` / `errMsg` | +| Clone action (`c`) | `"Cloning …"` | `cloneDoneMsg` / `errMsg` | +| Remote picker open (`r`) | `"Fetching remote branches…"` | `remoteBranchesMsg` | + +Setting `busy` and dispatching the slow command happen together in the same +`Update` branch. The command is unchanged; only the surrounding state is set. + +### Spinner animation + +The spinner animates on its own `spinner.TickMsg` loop, independent of the +existing 3-second `tickMsg` refresh loop (which keeps running harmlessly behind +the busy screen — a background `itemsMsg` refresh just updates the hidden list). + +- Whenever `busy` is set, `tea.Batch` in `m.spinner.Tick` alongside the slow + command. +- The `spinner.TickMsg` handler advances the frame and re-issues the spinner + tick **only while `busy != ""`**, so the animation loop stops cleanly once the + result arrives and `busy` is cleared. This avoids a runaway tick loop after the + operation completes. + +### Removing the duplicate loading path + +The remote picker's own `loading bool` and its static +`"Fetching remote branches…"` `View()` branch are **removed** and folded into the +shared busy state — one loading mechanism, not two. The picker keeps its +`errText` / empty-list / list branches. On `remoteBranchesMsg`: + +- success → `busy` clears, `screenRemotePicker` renders the list (or the "no + remote branches found" empty state); +- error → `busy` clears and the picker's `errText` shows the error, exactly as + today. + +On any `errMsg` for create/clone, `busy` clears and the existing `statusMsg` +renders the error in the status line, as it does today. On success, the existing +`statusMsg` (`"Created myapp/feat"`, `"Cloned myapp"`) shows once the busy screen +is gone. + +### Keys while busy + +While `busy != ""`, all input is swallowed except the global quit binding +(`q` / `Ctrl-C`), which quits the app. The git operation itself is not +cancellable mid-flight, so there is no "cancel" affordance — `Esc` does nothing +until the result message arrives. This is enforced by an early check at the top +of `Update`'s key handling: if `busy != ""`, only quit is honored. + +## View + +When `busy != ""`, `View()` returns a centered single line: + +``` + + + ◐ Creating worktree… + + +``` + +Centering reuses the model's known `width`/`height`. The exact vertical/ +horizontal centering follows whatever the existing views already do for layout; +if there is no shared centering helper, a small local one is fine (it is only +used here). + +## Testing + +Following the repo's table-driven `internal/tui/model_test.go` style. No new +integration tests — this is pure TUI state, and `spinner.Model` is deterministic +under an injected `TickMsg`. + +- **Transitions set busy**: submitting the form, clone, and `r` each set `busy` + to the expected label and batch a spinner tick. +- **Results clear busy**: `worktreeCreatedMsg`, `cloneDoneMsg`, + `remoteBranchesMsg` (success and error), and `errMsg` each clear `busy`. +- **View**: `View()` renders the centered label when `busy != ""` and the normal + screen when `busy == ""`. +- **Keys while busy**: a non-quit key press while `busy != ""` is a no-op; the + quit binding still quits. +- **Tick loop stops**: a `spinner.TickMsg` while `busy == ""` does not re-issue a + tick. + +## Files touched + +- `internal/tui/model.go` — `spinner`/`busy` fields; `Update` branches to set on + dispatch and clear on result; `View` short-circuit; key-swallow while busy; + spinner tick handling. +- `internal/tui/actions.go` — set `busy` in `cloneAction` dispatch; set `busy` + when opening the remote picker. +- `internal/tui/form.go` — set `busy` when `submit()` is dispatched. +- `internal/tui/remote_picker.go` — remove `loading` and its `View()` branch. +- `internal/tui/model_test.go` (and picker/actions tests as needed) — cover the + cases above. diff --git a/internal/tui/form.go b/internal/tui/form.go index d7ec870..3bbaeb6 100644 --- a/internal/tui/form.go +++ b/internal/tui/form.go @@ -210,7 +210,10 @@ func (m Model) updateForm(msg tea.Msg) (tea.Model, tea.Cmd) { m.form, cmd = m.form.Update(msg) if m.form.completed() { - return m, m.form.submit() + // Reassign m (do not shadow it with :=) so the linter stays quiet. + var submitCmd tea.Cmd + m, submitCmd = m.enterBusy("Creating worktree…", m.form.submit()) + return m, submitCmd } return m, cmd diff --git a/internal/tui/form_test.go b/internal/tui/form_test.go index ae2517f..f57af95 100644 --- a/internal/tui/form_test.go +++ b/internal/tui/form_test.go @@ -5,6 +5,7 @@ import ( "testing" "charm.land/bubbles/v2/list" + "charm.land/bubbles/v2/spinner" tea "charm.land/bubbletea/v2" "github.com/xico42/codeherd/internal/config" @@ -325,3 +326,20 @@ func toListItems(items []Item) []list.Item { } return result } + +func TestUpdateForm_nonCompletedDoesNotSetBusy(t *testing.T) { + ctx := formContext{project: "api", baseBranch: "develop"} + cfg := &config.Config{ + Projects: map[string]config.ProjectConfig{"api": {}}, + Agents: map[string]config.AgentConfig{"claude": {Cmd: "claude"}}, + } + m := Model{screen: screenForm, spinner: spinner.New(), cfg: cfg} + m.form = newFormModel(ctx, cfg, nil) + m.form.Init() + + updated, _ := m.updateForm(tea.KeyPressMsg(tea.Key{Code: 'a', Text: "a"})) + + if updated.(Model).busy != "" { + t.Errorf("busy = %q, want empty for a non-completed form", updated.(Model).busy) + } +} diff --git a/internal/tui/model.go b/internal/tui/model.go index 7ee4a6a..c3ea93c 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -8,6 +8,7 @@ import ( "charm.land/bubbles/v2/help" "charm.land/bubbles/v2/key" "charm.land/bubbles/v2/list" + "charm.land/bubbles/v2/spinner" tea "charm.land/bubbletea/v2" "charm.land/lipgloss/v2" "github.com/charmbracelet/x/term" @@ -79,6 +80,15 @@ type Model struct { // Status message for async operations. statusMsg string + // busy is a non-empty label while a blocking git operation is in + // flight. When set, View renders a centered spinner + label, + // overriding m.screen (which is left unchanged). Cleared by the + // operation's result message. + busy string + + // spinner animates the busy screen. It only ticks while busy != "". + spinner spinner.Model + // Form sub-model. form *formModel @@ -117,12 +127,14 @@ func NewModel( keys := defaultKeyMap() l := newList(nil) h := help.New() + sp := spinner.New(spinner.WithSpinner(spinner.MiniDot)) m := Model{ screen: screenList, list: l, keys: keys, help: h, + spinner: sp, cfg: cfg, wtSvc: wtSvc, sesSvc: sesSvc, @@ -149,6 +161,14 @@ func (m Model) syncProfileKeyEnabled() Model { return m } +// enterBusy sets the busy label and batches the given command with a spinner +// tick, so the spinner animates while cmd runs. m.screen is left unchanged; +// View short-circuits to the busy screen while busy != "". +func (m Model) enterBusy(label string, cmd tea.Cmd) (Model, tea.Cmd) { + m.busy = label + return m, tea.Batch(cmd, m.spinner.Tick) +} + func newList(items []list.Item) list.Model { if items == nil { items = []list.Item{} @@ -171,6 +191,17 @@ func (m Model) Init() tea.Cmd { } func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + // While a blocking operation is in flight, swallow all input except + // quit. The screen underneath is hidden by the busy view, and the git + // operation cannot be cancelled mid-flight, so other keys must not + // mutate state that the pending result message will rely on. + if kp, ok := msg.(tea.KeyPressMsg); ok && m.busy != "" { + if key.Matches(kp, m.keys.Quit) { + return m, tea.Quit + } + return m, nil + } + switch msg := msg.(type) { case tea.WindowSizeMsg: m.width = msg.Width @@ -186,6 +217,16 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case tickMsg: return m, tea.Batch(m.refreshCmd(), tickCmd()) + case spinner.TickMsg: + // Only advance (and thus re-schedule) while busy, so the tick + // loop stops cleanly once the operation completes. + if m.busy == "" { + return m, nil + } + var cmd tea.Cmd + m.spinner, cmd = m.spinner.Update(msg) + return m, cmd + case itemsMsg: items := make([]list.Item, len(msg)) for i, item := range msg { @@ -216,6 +257,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, cmd case errMsg: + m.busy = "" if msg.err != nil { m.statusMsg = msg.err.Error() } @@ -229,10 +271,12 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, tea.Quit case cloneDoneMsg: + m.busy = "" m.statusMsg = fmt.Sprintf("Cloned %s", msg.project) return m, m.refreshCmd() case worktreeCreatedMsg: + m.busy = "" m.statusMsg = fmt.Sprintf("Created %s/%s", msg.project, msg.branch) m.screen = screenList m.form = nil @@ -242,10 +286,10 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, m.refreshCmd() case remoteBranchesMsg: + m.busy = "" if m.remotePicker != nil && m.remotePicker.project == msg.project { if msg.err != nil { m.remotePicker.errText = msg.err.Error() - m.remotePicker.loading = false } else { m.remotePicker.setBranches(msg.branches) } @@ -287,7 +331,15 @@ func (m Model) updateList(msg tea.Msg) (tea.Model, tea.Cmd) { return m, m.shellAction() case key.Matches(msg, m.keys.Clone): - return m, m.cloneAction() + cmd := m.cloneAction() + if cmd == nil { + return m, nil + } + // cloneAction returned non-nil ⇒ selection is a valid + // uncloned project, so selectedItem() is non-nil. + sel := m.selectedItem() + m, cmd = m.enterBusy(fmt.Sprintf("Cloning %s…", sel.Project), cmd) + return m, cmd case key.Matches(msg, m.keys.New): return m.showForm() @@ -316,6 +368,17 @@ func (m Model) updateList(msg tea.Msg) (tea.Model, tea.Cmd) { } func (m Model) View() tea.View { + if m.busy != "" { + body := lipgloss.Place( + m.width, m.height, + lipgloss.Center, lipgloss.Center, + m.spinner.View()+" "+m.busy, + ) + v := tea.NewView(body) + v.AltScreen = true + return v + } + var content string switch m.screen { case screenForm: @@ -582,7 +645,8 @@ func (m Model) startRemotePicker() (tea.Model, tea.Cmd) { } m.remotePicker = newRemotePicker(sel.Project, m.cfg, m.tmuxClient) m.screen = screenRemotePicker - return m, m.fetchRemoteBranchesCmd(sel.Project) + m, cmd := m.enterBusy("Fetching remote branches…", m.fetchRemoteBranchesCmd(sel.Project)) + return m, cmd } // fetchRemoteBranchesCmd fetches all remotes (best-effort) and lists the diff --git a/internal/tui/model_test.go b/internal/tui/model_test.go index 60a459f..9c3abd5 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -8,7 +8,9 @@ import ( "testing" "time" + "charm.land/bubbles/v2/help" "charm.land/bubbles/v2/list" + "charm.land/bubbles/v2/spinner" tea "charm.land/bubbletea/v2" "github.com/xico42/codeherd/internal/config" @@ -705,3 +707,156 @@ func TestSwitchProfile_loadFailurePreservesActive(t *testing.T) { t.Error("expected statusMsg to describe the failure") } } + +func TestEnterBusy_setsLabelAndBatchesTick(t *testing.T) { + m := Model{} + called := false + cmd := func() tea.Msg { called = true; return nil } + + m2, batched := m.enterBusy("Working…", cmd) + + if m2.busy != "Working…" { + t.Errorf("busy = %q, want %q", m2.busy, "Working…") + } + if batched == nil { + t.Fatal("enterBusy returned nil cmd, want batched cmd") + } + _ = called // cmd is not executed here; we only assert it was batched (non-nil). +} + +func TestUpdate_spinnerTickIgnoredWhenNotBusy(t *testing.T) { + m := Model{busy: ""} + + _, cmd := m.Update(spinner.TickMsg{}) + + if cmd != nil { + t.Error("spinner.TickMsg while not busy should not re-issue a tick") + } +} + +func TestUpdate_spinnerTickAdvancesWhenBusy(t *testing.T) { + m := Model{busy: "Working…", spinner: spinner.New()} + + _, cmd := m.Update(spinner.TickMsg{}) + + if cmd == nil { + t.Error("spinner.TickMsg while busy should re-issue a tick") + } +} + +func TestUpdate_swallowsKeysWhileBusy(t *testing.T) { + m := Model{busy: "Working…", screen: screenList, keys: defaultKeyMap()} + m.list = newList(nil) + + // A non-quit key is a no-op while busy. + updated, cmd := m.Update(tea.KeyPressMsg(tea.Key{Code: 'n', Text: "n"})) + um := updated.(Model) + if cmd != nil { + t.Error("non-quit key while busy should return nil cmd") + } + if um.screen != screenList { + t.Errorf("screen changed to %d while busy, want unchanged", um.screen) + } + + // Quit still quits. + _, quitCmd := m.Update(tea.KeyPressMsg(tea.Key{Code: 'q', Text: "q"})) + if quitCmd == nil { + t.Error("quit key while busy should return a cmd") + } +} + +func TestView_rendersBusyScreen(t *testing.T) { + m := Model{busy: "Creating worktree…", spinner: spinner.New(), width: 40, height: 10} + m.list = newList(nil) + + v := m.View() + + if !strings.Contains(v.Content, "Creating worktree…") { + t.Errorf("busy view should contain the label; got:\n%s", v.Content) + } +} + +func TestView_normalWhenNotBusy(t *testing.T) { + m := Model{busy: "", screen: screenList, spinner: spinner.New(), width: 40, height: 10} + m.list = newList(nil) + m.help = help.New() + m.keys = defaultKeyMap() + + v := m.View() + + if strings.Contains(v.Content, "Creating worktree…") { + t.Error("non-busy view should not contain a busy label") + } +} + +func TestUpdate_resultMessagesClearBusy(t *testing.T) { + tests := []struct { + name string + msg tea.Msg + }{ + {"errMsg", errMsg{err: errors.New("boom")}}, + {"cloneDoneMsg", cloneDoneMsg{project: "myapp"}}, + {"worktreeCreatedMsg", worktreeCreatedMsg{project: "myapp", branch: "feat"}}, + {"remoteBranchesMsg", remoteBranchesMsg{project: "myapp"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := newModelForResultTest() + m.busy = "Working…" + + updated, _ := m.Update(tt.msg) + + if updated.(Model).busy != "" { + t.Errorf("%s did not clear busy", tt.name) + } + }) + } +} + +// newModelForResultTest builds a minimal Model that can absorb result +// messages without a nil-pointer (refreshCmd/list access after clearing busy). +func newModelForResultTest() Model { + m := Model{screen: screenList} + m.list = newList(nil) + m.cfg = &config.Config{} + return m +} + +func TestUpdate_cloneSetsBusy(t *testing.T) { + m := Model{screen: screenList, keys: defaultKeyMap(), spinner: spinner.New()} + m.list = newList(toListItems([]Item{ + {Project: "myapp", Group: groupProject, Cloned: false}, + })) + m.cfg = &config.Config{Projects: map[string]config.ProjectConfig{"myapp": {}}} + + updated, cmd := m.Update(tea.KeyPressMsg(tea.Key{Code: 'c', Text: "c"})) + um := updated.(Model) + + if um.busy != "Cloning myapp…" { + t.Errorf("busy = %q, want %q", um.busy, "Cloning myapp…") + } + if cmd == nil { + t.Error("clone should return a batched cmd") + } +} + +func TestStartRemotePicker_setsBusy(t *testing.T) { + m := Model{screen: screenList, spinner: spinner.New()} + m.list = newList(toListItems([]Item{ + {Project: "myapp", Group: groupProject, Cloned: true}, + })) + m.cfg = &config.Config{} + + updated, cmd := m.startRemotePicker() + um := updated.(Model) + + if um.busy != "Fetching remote branches…" { + t.Errorf("busy = %q, want %q", um.busy, "Fetching remote branches…") + } + if um.screen != screenRemotePicker { + t.Errorf("screen = %d, want screenRemotePicker", um.screen) + } + if cmd == nil { + t.Error("startRemotePicker should return a batched cmd") + } +} diff --git a/internal/tui/remote_picker.go b/internal/tui/remote_picker.go index 076e2bd..d077e0e 100644 --- a/internal/tui/remote_picker.go +++ b/internal/tui/remote_picker.go @@ -22,7 +22,6 @@ func (i remoteBranchItem) Description() string { return "" } type remotePickerModel struct { list list.Model project string - loading bool errText string cfg *config.Config tmuxClient *tmux.Client @@ -37,7 +36,6 @@ func newRemotePicker(project string, cfg *config.Config, tmuxClient *tmux.Client return &remotePickerModel{ list: l, project: project, - loading: true, cfg: cfg, tmuxClient: tmuxClient, } @@ -49,7 +47,6 @@ func (p *remotePickerModel) setBranches(branches []worktree.RemoteBranch) { items[i] = remoteBranchItem{rb: b} } p.list.SetItems(items) - p.loading = false } func (p *remotePickerModel) selected() (worktree.RemoteBranch, bool) { @@ -68,8 +65,6 @@ func (p *remotePickerModel) Update(msg tea.Msg) (*remotePickerModel, tea.Cmd) { func (p *remotePickerModel) View() string { switch { - case p.loading: - return "Fetching remote branches…" case p.errText != "": return "Error: " + p.errText + "\n\nEsc: cancel" case len(p.list.Items()) == 0: diff --git a/internal/tui/remote_picker_test.go b/internal/tui/remote_picker_test.go index 970a844..51138b5 100644 --- a/internal/tui/remote_picker_test.go +++ b/internal/tui/remote_picker_test.go @@ -9,16 +9,10 @@ import ( func TestRemotePicker_setAndSelect(t *testing.T) { p := newRemotePicker("myapp", &config.Config{}, nil) - if !p.loading { - t.Error("picker should start in loading state") - } p.setBranches([]worktree.RemoteBranch{ {Remote: "origin", Branch: "feat-x", Ref: "origin/feat-x"}, {Remote: "origin", Branch: "fix-y", Ref: "origin/fix-y"}, }) - if p.loading { - t.Error("picker should leave loading state after setBranches") - } rb, ok := p.selected() if !ok || rb.Ref != "origin/feat-x" { t.Errorf("selected = %+v ok=%v, want origin/feat-x", rb, ok) @@ -40,9 +34,6 @@ func TestRemoteBranchesMsg_populatesPicker(t *testing.T) { branches: []worktree.RemoteBranch{{Remote: "origin", Branch: "feat-x", Ref: "origin/feat-x"}}, }) mm := updated.(Model) - if mm.remotePicker.loading { - t.Error("expected picker to leave loading after branches arrive") - } if got := len(mm.remotePicker.list.Items()); got != 1 { t.Errorf("items = %d, want 1", got) }