fix: serialize target panel operations

2026-07-17 09:33:13 +08:00
parent fb456f35e4
commit e9718a6d08
2 changed files with 224 additions and 12 deletions
@@ -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
@@ -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},