diff --git a/internal/tui/model.go b/internal/tui/model.go index 39bcc39..9d9704c 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -72,8 +72,18 @@ func NewModel(application app.Module) Model { func (m Model) Init() tea.Cmd { return nil } +// busy is the single-operation guard for asynchronous Application work. Every +// operation start checks it, so a completion can only belong to the one current +// operation rather than racing another operation for shared model state. +func (m Model) busy() bool { + return m.applying || m.refreshing || m.profileTransition || m.pendingActivation.Kind != 0 +} + func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { if completed, ok := message.(refreshDone); ok { + if !m.refreshing { + return m, nil + } m.refreshing = false m.snapshot = completed.result.Snapshot if completed.result.Err != nil { @@ -81,12 +91,20 @@ func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { } else { m.status = completed.result.Notice } - if m.selected >= len(m.availableTargets()) && m.selected > 0 { - m.selected-- + targetCount := len(m.availableTargets()) + if targetCount == 0 { + m.selected = 0 + } else if m.selected < 0 { + m.selected = 0 + } else if m.selected >= targetCount { + m.selected = targetCount - 1 } return m, nil } if progress, ok := message.(profileOperationProgress); ok { + if !m.profileTransition { + return m, nil + } m.snapshot = progress.result.Snapshot m.progressTarget = "" if len(progress.result.Progress) > 0 { @@ -119,6 +137,9 @@ func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { return m, waitForProfileProgress(progress.updates) } if completed, ok := message.(targetOperationDone); ok { + if !m.applying || m.profileTransition { + return m, nil + } m.applying = false m.snapshot = completed.result.Snapshot if completed.result.Confirmation != "" { @@ -159,7 +180,7 @@ func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { } func (m Model) updateMain(key tea.KeyMsg) (tea.Model, tea.Cmd) { - if m.targetConfirm != nil && !m.applying { + if m.targetConfirm != nil && !m.busy() { switch key.String() { case "y": intent := *m.targetConfirm @@ -177,14 +198,18 @@ func (m Model) updateMain(key tea.KeyMsg) (tea.Model, tea.Cmd) { targets := m.availableTargets() switch key.String() { case "p": - m.page = profilesPage - m.selected = 0 - m.status = "" + if !m.busy() { + m.page = profilesPage + m.selected = 0 + m.status = "" + } case "?": - m.page = helpPage - m.status = "" + if !m.busy() { + m.page = helpPage + m.status = "" + } case "r": - if !m.applying && !m.refreshing { + if !m.busy() { m.refreshing = true m.status = "Refreshing Target availability and state…" return m, func() tea.Msg { @@ -192,15 +217,15 @@ func (m Model) updateMain(key tea.KeyMsg) (tea.Model, tea.Cmd) { } } case "up", "k": - if !m.applying && m.selected > 0 { + if !m.busy() && m.selected > 0 { m.selected-- } case "down", "j": - if !m.applying && m.selected+1 < len(targets) { + if !m.busy() && m.selected+1 < len(targets) { m.selected++ } case " ": - if !m.applying && m.selected >= 0 && m.selected < len(targets) { + if !m.busy() && m.selected >= 0 && m.selected < len(targets) { selected := targets[m.selected] desired := selected.State == target.Disabled || selected.State == target.External m.applying = true @@ -224,9 +249,11 @@ func (m Model) updateProfiles(key tea.KeyMsg) (tea.Model, tea.Cmd) { intent := *m.profileConfirm intent.Confirm = true m.profileConfirm = nil + m.pendingActivation = app.Intent{} return m.beginProfileActivation(intent) case "n", "esc", "q": m.profileConfirm = nil + m.pendingActivation = app.Intent{} m.status = "" } return m, nil @@ -243,6 +270,14 @@ func (m Model) updateProfiles(key tea.KeyMsg) (tea.Model, tea.Cmd) { } return m, nil } + if m.busy() { + if key.String() == "q" || key.String() == "esc" { + m.page = mainPage + m.selected = 0 + m.status = "" + } + return m, nil + } switch key.String() { case "q", "esc": @@ -332,6 +367,9 @@ func (m Model) updateForm(key tea.KeyMsg) (tea.Model, tea.Cmd) { } func (m Model) beginProfileActivation(intent app.Intent) (tea.Model, tea.Cmd) { + if m.busy() { + return m, nil + } if progressive, ok := m.application.(app.ProgressiveModule); ok { m.applying = true m.profileTransition = true diff --git a/internal/tui/target_test.go b/internal/tui/target_test.go index 78c446e..55ccafb 100644 --- a/internal/tui/target_test.go +++ b/internal/tui/target_test.go @@ -126,6 +126,180 @@ func TestRefreshKeyRunsAsynchronouslyAndRendersRefreshingThenCompleted(t *testin } } +func TestBusyActivationBlocksPageEntryAndEveryNewOperation(t *testing.T) { + initial := app.Snapshot{ + ActiveProfile: "old", + Profiles: []profile.Profile{{Name: "new"}}, + Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Enabled}}, + } + module := &progressiveTargetModule{targetModule: targetModule{snapshot: initial}, updates: make(chan app.Result)} + model := updateKey(NewModel(module), "p") + updated, command := model.Update(tea.KeyMsg{Type: tea.KeyEnter}) + model = updated.(Model) + if command == nil || len(module.intents) != 1 || module.intents[0].Kind != app.ActivateProfile { + t.Fatalf("activation did not start exactly once: command=%v intents=%#v", command != nil, module.intents) + } + + for _, key := range []string{"p", "?", "r", " "} { + updated, blocked := model.Update(keyMessage(key)) + model = updated.(Model) + if blocked != nil || model.page != mainPage || len(module.intents) != 1 { + t.Fatalf("busy activation accepted %q: page=%v command=%v intents=%#v", key, model.page, blocked != nil, module.intents) + } + } + for _, key := range []tea.KeyMsg{keyMessage("q"), {Type: tea.KeyEsc}} { + _, quit := model.Update(key) + if quit == nil { + t.Fatalf("busy activation changed quit behavior for %q", key.String()) + } + if _, ok := quit().(tea.QuitMsg); !ok || len(module.intents) != 1 { + t.Fatalf("busy activation quit %q triggered an operation: intents=%#v", key.String(), module.intents) + } + } + + // Even if a Profiles-page activation key is delivered while the transition + // is active, the single-operation guard must reject a second activation. + model.page = profilesPage + updated, blocked := model.Update(tea.KeyMsg{Type: tea.KeyEnter}) + model = updated.(Model) + if blocked != nil || len(module.intents) != 1 { + t.Fatalf("busy activation started a second activation: command=%v intents=%#v", blocked != nil, module.intents) + } +} + +func TestBusyRefreshBlocksTogglePageEntryRefreshAndActivation(t *testing.T) { + initial := app.Snapshot{ + Profiles: []profile.Profile{{Name: "office"}}, + Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Disabled}}, + } + module := &targetModule{snapshot: initial, result: app.Result{Snapshot: initial}} + model := NewModel(module) + updated, refreshCommand := model.Update(keyMessage("r")) + model = updated.(Model) + if refreshCommand == nil { + t.Fatal("refresh did not start") + } + + for _, key := range []string{" ", "p", "?", "r"} { + updated, blocked := model.Update(keyMessage(key)) + model = updated.(Model) + if blocked != nil || model.page != mainPage || len(module.intents) != 0 { + t.Fatalf("busy refresh accepted %q: page=%v command=%v intents=%#v", key, model.page, blocked != nil, module.intents) + } + } + for _, key := range []tea.KeyMsg{keyMessage("q"), {Type: tea.KeyEsc}} { + _, quit := model.Update(key) + if quit == nil { + t.Fatalf("busy refresh changed quit behavior for %q", key.String()) + } + if _, ok := quit().(tea.QuitMsg); !ok || len(module.intents) != 0 { + t.Fatalf("busy refresh quit %q triggered an operation: intents=%#v", key.String(), module.intents) + } + } + + model.page = profilesPage + updated, blocked := model.Update(tea.KeyMsg{Type: tea.KeyEnter}) + if blocked != nil || len(module.intents) != 0 { + t.Fatalf("busy refresh started activation: command=%v intents=%#v", blocked != nil, module.intents) + } +} + +func TestPendingActivationRemainsBusyUntilConfirmationIsResolved(t *testing.T) { + initial := app.Snapshot{Profiles: []profile.Profile{{Name: "office"}}} + updates := make(chan app.Result, 1) + updates <- app.Result{Snapshot: initial, Confirmation: "confirm transition"} + module := &progressiveTargetModule{targetModule: targetModule{snapshot: initial}, updates: updates} + model := updateKey(NewModel(module), "p") + updated, command := model.Update(tea.KeyMsg{Type: tea.KeyEnter}) + model = updated.(Model) + updated, _ = model.Update(command()) + model = updated.(Model) + if !model.busy() || model.pendingActivation.Kind != app.ActivateProfile || model.profileConfirm == nil { + t.Fatalf("pending activation was not guarded: %#v", model) + } + + model.page = mainPage + for _, key := range []string{"p", "?", "r", " "} { + updated, blocked := model.Update(keyMessage(key)) + model = updated.(Model) + if blocked != nil || model.page != mainPage || len(module.intents) != 1 { + t.Fatalf("pending activation accepted %q: page=%v command=%v intents=%#v", key, model.page, blocked != nil, module.intents) + } + } + model.page = profilesPage + updated, _ = model.Update(keyMessage("n")) + model = updated.(Model) + if model.busy() || model.profileConfirm != nil { + t.Fatal("cancelling confirmation did not clear pending activation") + } +} + +func TestSingleOperationGuardRejectsMismatchedCompletionAndPreventsOverlap(t *testing.T) { + initial := app.Snapshot{Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Disabled}}} + module := &targetModule{snapshot: initial, result: app.Result{Snapshot: initial}} + model := NewModel(module) + updated, refreshCommand := model.Update(keyMessage("r")) + model = updated.(Model) + + updated, command := model.Update(targetOperationDone{result: app.Result{Snapshot: app.Snapshot{}}}) + model = updated.(Model) + if command != nil || !model.refreshing || !model.busy() { + t.Fatal("completion from a non-current operation cleared refresh state") + } + updated, command = model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + if command != nil { + t.Fatal("Target operation overlapped the in-flight refresh") + } + + refreshCompletion := refreshCommand() + updated, command = model.Update(refreshCompletion) + model = updated.(Model) + if command != nil || model.busy() { + t.Fatal("current refresh completion did not clear busy state") + } + updated, command = model.Update(tea.KeyMsg{Type: tea.KeySpace}) + if command == nil { + t.Fatal("new operation was not allowed after the prior completion was consumed") + } +} + +func TestRefreshClampsSelectionAfterMultipleTargetsDisappear(t *testing.T) { + initial := app.Snapshot{Targets: []app.TargetSnapshot{ + {ID: "one", Description: target.Description{Name: "One"}, Available: true}, + {ID: "two", Description: target.Description{Name: "Two"}, Available: true}, + {ID: "three", Description: target.Description{Name: "Three"}, Available: true}, + {ID: "four", Description: target.Description{Name: "Four"}, Available: true}, + }} + refreshed := app.Snapshot{Targets: []app.TargetSnapshot{{ID: "one", Description: target.Description{Name: "One"}, Available: true}}} + module := &targetModule{snapshot: initial, result: app.Result{Snapshot: refreshed}} + model := NewModel(module) + model.selected = 3 + updated, command := model.Update(keyMessage("r")) + model = updated.(Model) + updated, _ = model.Update(command()) + model = updated.(Model) + if model.selected != 0 || !strings.Contains(model.View(), "> [ ] One") { + t.Fatalf("selection after multi-Target reduction = %d\n%s", model.selected, model.View()) + } + + module.result = app.Result{Snapshot: app.Snapshot{}} + updated, command = model.Update(keyMessage("r")) + model = updated.(Model) + updated, _ = model.Update(command()) + model = updated.(Model) + if model.selected != 0 { + t.Fatalf("selection with no Available Targets = %d, want 0", model.selected) + } +} + +func keyMessage(key string) tea.KeyMsg { + if key == " " { + return tea.KeyMsg{Type: tea.KeySpace} + } + return tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune(key)} +} + func TestTargetPanelNavigationKeysAndQuitCombinations(t *testing.T) { module := &targetModule{snapshot: app.Snapshot{Targets: []app.TargetSnapshot{ {ID: "first", Description: target.Description{Name: "First", Summary: "first summary"}, Available: true},