Fix a few flaky tests (#5756)

These started out as a few intermittently-failing integration tests, but
the root cause was real: a handful of places where UI updates
(rendering, model updates, view content) could be applied in a different
order than they were issued, because the order depended on goroutine
scheduling rather than being deterministic. In rare cases that could
make the main view briefly show stale or wrong content — most likely
when moving quickly between items. This was never observed in normal
usage, but caused some integration tests to fail occasionally (both
locally and on CI).

Each commit fixes one of these orderings; see the commit messages for
details.

pkg/gocui/gui.go

@@ -238,7 +238,12 @@ func NewGui(opts NewGuiOpts) (*Gui, error) {
 	g.stop = make(chan struct{})
 
 	g.gEvents = make(chan GocuiEvent, 20)
-	g.userEvents = make(chan userEvent, 20)
+	// Update does a non-blocking send and panics on a full channel rather than
+	// blocking (which would deadlock the UI goroutine against itself) or
+	// silently reordering. The buffer is sized well above the peak occupancy we
+	// see in practice, so the panic stays unreachable in normal use; if it ever
+	// fires, that's a real anomaly to investigate, not a cue to grow the buffer.
+	g.userEvents = make(chan userEvent, 256)
 	g.taskManager = newTaskManager()
 
 	if opts.PlayRecording {
@@ -613,28 +618,23 @@ type userEvent struct {
 	contentOnly bool
 }
 
-// Update executes the passed function. This method can be called safely from a
-// goroutine in order to update the GUI. It is important to note that the
-// passed function won't be executed immediately, instead it will be added to
-// the user events queue. Given that Update spawns a goroutine, the order in
-// which the user events will be handled is not guaranteed.
+// Update enqueues f on the user-events channel for the UI loop to run on its
+// next iteration. Multiple Update calls from the same goroutine arrive in
+// source order via the channel's FIFO. The send is non-blocking — if the
+// channel is full we panic rather than block or silently reorder, since a
+// blocked send from the UI goroutine would deadlock against itself and
+// silently switching to inline execution would break the ordering guarantee
+// callers rely on. The buffer is sized generously enough that this should
+// never fire in practice; if it does, that's a signal to investigate, not
+// to grow the buffer reflexively.
 func (g *Gui) Update(f func(*Gui) error) {
 	task := g.NewTask()
 
-	go g.updateAsyncAux(f, task)
-}
-
-// UpdateAsync is a version of Update that does not spawn a go routine, it can
-// be a bit more efficient in cases where Update is called many times like when
-// tailing a file.  In general you should use Update()
-func (g *Gui) UpdateAsync(f func(*Gui) error) {
-	task := g.NewTask()
-
-	g.updateAsyncAux(f, task)
-}
-
-func (g *Gui) updateAsyncAux(f func(*Gui) error, task Task) {
-	g.userEvents <- userEvent{f: f, task: task}
+	select {
+	case g.userEvents <- userEvent{f: f, task: task}:
+	default:
+		panic("gocui: userEvents channel full; refusing to block or reorder")
+	}
 }
 
 // Like Update, but signals that the callback only modifies content.

pkg/gui/main_panels.go

@@ -117,6 +117,8 @@ func (gui *Gui) refreshMainViews(opts types.RefreshMainOpts) {
 		}
 	}
 
+	gui.moveMainContextPairToTop(opts.Pair)
+
 	if opts.Main != nil {
 		gui.RefreshMainView(opts.Main, opts.Pair.Main)
 	}
@@ -127,8 +129,6 @@ func (gui *Gui) refreshMainViews(opts types.RefreshMainOpts) {
 		opts.Pair.Secondary.GetView().Clear()
 	}
 
-	gui.moveMainContextPairToTop(opts.Pair)
-
 	gui.splitMainPanel(opts.Secondary != nil)
 }
 

pkg/tasks/tasks.go

@@ -403,12 +403,29 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error
 		})
 	}
 
+	// Assign the taskID synchronously so it reflects NewTask call order
+	// rather than the order in which the spawned goroutines happen to be
+	// scheduled. Otherwise two NewTask calls in quick succession can have
+	// their goroutines race, with the later-called task ending up with the
+	// lower taskID and losing the staleness check below.
+	self.taskIDMutex.Lock()
+	self.newTaskID++
+	taskID := self.newTaskID
+	self.taskIDMutex.Unlock()
+
 	go utils.Safe(func() {
 		defer completeGocuiTask()
 
 		self.taskIDMutex.Lock()
-		self.newTaskID++
-		taskID := self.newTaskID
+
+		// Bail out before touching shared view state if a newer task has
+		// already been queued: if we ran onNewKey here we'd reset the view
+		// for a task that's about to exit, potentially wiping output the
+		// winning task has already written.
+		if taskID < self.newTaskID {
+			self.taskIDMutex.Unlock()
+			return
+		}
 
 		if self.GetTaskKey() != key && self.onNewKey != nil {
 			self.onNewKey()
@@ -419,6 +436,8 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error
 
 		self.waitingMutex.Lock()
 
+		// Re-check staleness after acquiring waitingMutex: a newer task
+		// may have arrived while we were blocked here.
 		self.taskIDMutex.Lock()
 		if taskID < self.newTaskID {
 			self.waitingMutex.Unlock()