2 files changed, 32 insertions, 7 deletions

diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 3c2dca6..ba803bc 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -2798,6 +2798,9 @@ void MainWindow::flushHeldEdits()
     const QVector<HeldEdit> edits = m_heldEdits;
     m_heldEdits.clear();
 
+    // Stamped for the ordering test. See flushGenerationForTesting().
+    m_flushGeneration = m_generation;
+
     for (const HeldEdit &edit : edits) {
         // Take the optimistic update back before sending, because
         // sendThreadTagChange() applies it again. applyTagChange() is
@@ -3248,6 +3251,25 @@ void MainWindow::onExternalSyncStateChanged(SyncMonitor::State state)
         m_statusLabel->setText(m_defaultStatus);
     }
 
+    // BEFORE the refresh below, and the order is the whole of a defect. An edit
+    // made during a sync is held, because the worker's read-write open blocks
+    // on notmuch's exclusive lock. Refreshing first meant reading a database
+    // that still carried the old tag and reconciling that into the model, which
+    // overwrote the optimistic update; the flush then wrote the tag correctly,
+    // leaving the database right and the list wrong with nothing scheduled to
+    // re-read it. Reported by hand as a message going back to unread at the end
+    // of the sync it was read during.
+    //
+    // Flushing first also costs nothing when there is nothing held: the
+    // function returns immediately on an empty queue.
+    //
+    // OUTSIDE the Idle branch, deliberately, and this predates the reordering.
+    // Unknown clears the busy flag above, so writes resume from here on;
+    // leaving the flush inside Idle would let a new edit go straight out while
+    // the ones already held sat waiting for an Idle that a broken /proc/locks
+    // will never report.
+    flushHeldEdits();
+
     if (state == SyncMonitor::State::Idle) {
         refreshCurrentQuery();
 
@@ -3279,13 +3301,6 @@ void MainWindow::onExternalSyncStateChanged(SyncMonitor::State state)
         }
     }
 
-    // OUTSIDE the Idle branch, deliberately. Unknown clears the busy flag above,
-    // so writes resume from here on; leaving the flush inside Idle would let a
-    // new edit go straight out while the ones already held sat waiting for an
-    // Idle that a broken /proc/locks will never report. After the status
-    // message, which flushHeldEdits() overwrites with its own when it sent
-    // something.
-    flushHeldEdits();
 }
 
 void MainWindow::showTransientStatus(const QString &text)
diff --git a/src/mainwindow.h b/src/mainwindow.h
index e2d1861..2a86d12 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -207,6 +207,15 @@ public:
         return m_refreshGeneration;
     }
 
+    /// The query generation as it stood when the held edits were last flushed,
+    /// or 0 if they never have been.
+    ///
+    /// Recorded because the ORDER of the flush and the sync-end refresh is the
+    /// whole of one defect and both leave identical end states: a test that
+    /// looks afterwards passes whichever ran first. Compared against the
+    /// generation the refresh bumps, this says which came first.
+    quint64 flushGenerationForTesting() const { return m_flushGeneration; }
+
     /// The generation a database-stats reply must carry to be accepted.
     ///
     /// A test seam, for the same reason as the one above: onDatabaseStatsReady
@@ -700,6 +709,7 @@ private:
     /// it. Order matters: two edits touching one thread must reach the database
     /// in the order they were made, or the later one does not win.
     QVector<HeldEdit> m_heldEdits;
+    quint64 m_flushGeneration = 0;
 
     friend class ThreadTagCommand;
     friend class MessageTagCommand;