feat(sync): sync a tag change automatically after a short delay

Item 71. A tag edit reached the notmuch index at edit time and then sat there
until the user clicked Sync or their cron job fired, so "mark all read" updated
the view while the change itself waited, sometimes for ten minutes.

A confirmed edit now arms a debounce that runs the existing sync path. The delay
is auto_sync_delay_ms in [general], defaulting to 2000, and follows
mark_read_delay_ms exactly, including that zero and negative are not errors:
zero syncs on the next trip through the event loop, and any negative value
disables the behaviour, which is the switch for a user who wants only their cron
job.

It is armed from onTagsApplied, where a write is confirmed and the pending count
is already current, rather than where one is sent: a sync scheduled for a write
the worker went on to reject would run for nothing. A debounce rather than a
schedule, restarted by each edit, because "mark all read" confirms one write per
thread in the view and an arm-per-edit timer would be the storm of syncs the
debounce exists to prevent. Nothing is armed when no sync command is configured
or when the pending count is zero, the case where an edit was netted against its
own inverse. When the timer fires with a sync already running, local or cron, it
skips rather than queues: mbsync's own answer to a second run is to fail on it,
and the edits stay pending rather than being lost.

Also fixes a pane blanked out from under the reader, found by hand testing this
feature. onSyncFinished called runCurrentQuery() where the cron path calls
refreshCurrentQuery(), and a re-run clears the model, the undo stack and the
message pane. The stale-thread notice handles a thread that stops matching the
query and has since item 35, but a re-run left nothing for it to describe. The
two paths had no reason to differ; before this item a local sync only followed a
click on Sync, so the difference went unnoticed. Reading a message in the Unread
view, having it marked read, and watching the pane go blank two seconds later is
what surfaced it.

Its test asserts on the undo stack rather than the pane: both paths issue a
queued query test_mainwindow has no worker to answer, so the pane ends up blank
either way and an assertion on it would pass against both, while the undo stack
is cleared by one and kept by the other.

Nine tests, four in test_config and five in test_mainwindow, each
mutation-checked: removing the schedule call, honouring a negative delay,
dropping the nothing-pending guard, dropping the already-running guard, and
restoring runCurrentQuery() each fail a test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

4 files changed, 132 insertions, 1 deletions

diff --git a/src/config.cpp b/src/config.cpp
index a81651f..b1f730c 100644
--- a/src/config.cpp
+++ b/src/config.cpp
@@ -271,6 +271,23 @@ void Config::load(const QString &path)
         }
     }
 
+    // Item 71. Same shape as mark_read_delay_ms above, including that zero and
+    // negative are not errors: 0 syncs on the next trip through the event loop
+    // and negative disables the automatic sync entirely, which is how a user who
+    // wants only their cron job turns this off.
+    const QVariant autoSync = settings.value(QStringLiteral("auto_sync_delay_ms"));
+    if (autoSync.isValid()) {
+        bool ok = false;
+        const int value = autoSync.toString().toInt(&ok);
+        if (ok) {
+            m_autoSyncDelayMs = value;
+        } else {
+            addProblem(QStringLiteral("Auto-sync delay '%1' is not a number; "
+                                      "using the default.")
+                           .arg(autoSync.toString()));
+        }
+    }
+
     // [completion] is an ordinary section, so this one DOES take its prefix.
     // ',' separates entries and '|' separates a value from its description:
     // two different characters because QSettings splits comma lists itself,
diff --git a/src/config.h b/src/config.h
index 4092141..ea0b055 100644
--- a/src/config.h
+++ b/src/config.h
@@ -213,6 +213,20 @@ public:
     /// the behaviour so a thread stays unread until toggled by hand.
     int markReadDelayMs() const { return m_markReadDelayMs; }
 
+    /// How long to wait after a tag edit before syncing it out on the user's
+    /// behalf. Item 71.
+    ///
+    /// Same three meanings as markReadDelayMs() above, and deliberately so: a
+    /// positive value is the debounce in milliseconds, 0 syncs on the next trip
+    /// through the event loop, and any negative value disables the behaviour so
+    /// edits wait for a manual sync or the user's cron job, which is what every
+    /// release before this one did.
+    ///
+    /// Defaults to 2000. The delay is a debounce, not a schedule: each edit
+    /// restarts it, so a burst of tagging produces one sync after the burst
+    /// rather than one per tag.
+    int autoSyncDelayMs() const { return m_autoSyncDelayMs; }
+
     /// User-supplied mimetype completions, APPENDED to the built-in list.
     /// Appending rather than replacing means a typo cannot leave completion
     /// worse off than the defaults. Mimetypes are the only completion list a
@@ -250,6 +264,7 @@ private:
     qreal m_messageZoom = 1.0;
     bool m_completionOnFocus = false;
     int m_markReadDelayMs = 2000;
+    int m_autoSyncDelayMs = 2000;
     SyncOnExit m_syncOnExit = SyncOnExit::Ask;
     QList<CompletionEntry> m_extraMimetypes;
     QString m_startupQuery = QStringLiteral("Unread");
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 824bdca..04e951d 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -485,6 +485,13 @@ void MainWindow::buildUi()
     connect(m_markReadTimer, &QTimer::timeout,
             this, &MainWindow::markCurrentThreadRead);
 
+    m_autoSyncTimer = new QTimer(this);
+    // Named for the same reason: a test can assert that an edit armed the
+    // debounce without waiting out the delay or starting a real mbsync.
+    m_autoSyncTimer->setObjectName(QStringLiteral("autoSyncTimer"));
+    m_autoSyncTimer->setSingleShot(true);
+    connect(m_autoSyncTimer, &QTimer::timeout, this, &MainWindow::runAutoSync);
+
     // The pane and its close button travel together: a QPlainTextEdit has
     // nowhere to put one, and a pane that appears on a failed sync and can
     // never be dismissed is worse than one that does not appear at all.
@@ -2194,7 +2201,22 @@ void MainWindow::onSyncFinished(bool success, int exitCode)
             return;
         }
 
-        runCurrentQuery();
+        // refreshCurrentQuery(), NOT runCurrentQuery(). A sync this window
+        // started is not a query the user asked to re-run: runCurrentQuery()
+        // clears the model, the undo stack and the message pane, so a sync
+        // landing while a message was open read the user out of it. The cron
+        // path has reconciled instead since item 35, and there was never a
+        // reason for the two to differ.
+        //
+        // Item 71 is what made it matter. A local sync used to happen only
+        // when the user clicked Sync, where blanking was at least explicable;
+        // the automatic one fires two seconds after a tag edit, which is
+        // precisely when the user is still reading the message they tagged.
+        // Reconciling keeps the pane, and updateStaleThreadNotice() then offers
+        // "Show it anyway" for a thread that has stopped matching the query,
+        // which is the reported case: reading in Unread, the thread is marked
+        // read, and it no longer belongs to the view it was opened from.
+        refreshCurrentQuery();
         // A sync is the usual way new tags enter the database.
         requestAllTags();
     } else if (exitCode == kSyncSkippedExitCode) {
@@ -2280,6 +2302,12 @@ void MainWindow::onTagsApplied(const TagChange &change)
 
     updatePendingIndicator();
 
+    // Item 71. Armed here, where a write is CONFIRMED and the pending count is
+    // already up to date, for the same reason recordPendingEdit() is called
+    // here: a sync scheduled for a write the worker went on to reject would run
+    // for nothing.
+    scheduleAutoSync();
+
     // A tag the user has just created is the one they are most likely to type
     // again, so do not wait for the next sync to offer it. A set membership
     // test, not a query.
@@ -2678,6 +2706,58 @@ void MainWindow::startSync()
     setSyncBusy(true);
 }
 
+void MainWindow::scheduleAutoSync()
+{
+    // Negative disables the behaviour entirely, per the config key, and that is
+    // the pre-0.16.0 behaviour: edits wait for a manual sync or the user's cron
+    // job. Checked before anything else so a disabled delay arms nothing.
+    const int delay = m_config.autoSyncDelayMs();
+    if (delay < 0)
+        return;
+
+    // No sync command means the Sync action is already disabled and startSync()
+    // would only put "No sync command configured" in the status bar. Arming a
+    // timer to say that on a delay, for something the user did not ask for, is
+    // worse than staying quiet.
+    if (!m_sync || !m_sync->isAvailable())
+        return;
+
+    // Nothing outstanding, nothing to carry. An edit netted against its own
+    // inverse leaves the count at zero (item 28), and syncing for it would run
+    // mbsync over a mail store that is already where the server left it.
+    if (pendingEditCount() == 0)
+        return;
+
+    // Restart, not stack. Tagging a multi-row selection confirms one write per
+    // thread and "mark all read" confirms one per thread in the view, so an
+    // armed-per-edit timer would be exactly the storm of syncs a debounce is
+    // for. The last edit of a burst decides when the single sync happens.
+    m_autoSyncTimer->start(delay);
+}
+
+void MainWindow::runAutoSync()
+{
+    // The user can have synced by hand, or undone the edit, in the delay. Both
+    // leave nothing to carry, and re-checking here rather than trusting the arm
+    // is what makes the debounce safe to restart freely.
+    if (pendingEditCount() == 0)
+        return;
+
+    // Skip rather than queue when a sync is already in flight, which item 71
+    // requires: the cron job holds the same lock, and mbsync's own answer to a
+    // second run is to fail on it. The edits are not lost by skipping. They stay
+    // pending, and the sync already running is very likely to carry them, since
+    // they reached the mail store at edit time.
+    //
+    // m_externalSyncBusy covers the cron job SyncMonitor can see. A lock taken
+    // between that poll and now is not visible here, and does not need to be:
+    // MailSync::start() fails on a second run and startSync() reports it.
+    if (m_externalSyncBusy || (m_sync && m_sync->isRunning()))
+        return;
+
+    startSync();
+}
+
 void MainWindow::recordPendingEdit(const QString &messageId, const QString &tag,
                                    bool added)
 {
diff --git a/src/mainwindow.h b/src/mainwindow.h
index 0912520..19635dd 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -390,6 +390,17 @@ private:
     /// Redraws the unsynced-edits indicator from pendingEditCount().
     void updatePendingIndicator();
 
+    /// Arms the debounce that syncs a confirmed tag edit out on its own
+    /// (item 71). Does nothing when the delay is negative, when no sync command
+    /// is configured, or when nothing is actually pending.
+    void scheduleAutoSync();
+
+    /// Starts the debounced automatic sync, unless a sync is already running
+    /// (local or external) or the edits it would carry are already gone.
+    ///
+    /// Q_INVOKABLE so a test can fire the debounce without waiting it out.
+    Q_INVOKABLE void runAutoSync();
+
     /// Records one confirmed (message, tag) change, cancelling it against an
     /// opposite change already outstanding for the same pair.
     void recordPendingEdit(const QString &messageId, const QString &tag,
@@ -756,6 +767,14 @@ private:
     /// nothing rather than marking the wrong one.
     QString m_markReadThreadId;
 
+    /// Debounces the automatic sync that follows a tag edit (item 71).
+    ///
+    /// Single-shot and RESTARTED by every confirmed edit, for the same reason
+    /// m_markReadTimer is: tagging a multi-row selection confirms one write per
+    /// thread, and one sync per thread is exactly what a debounce exists to
+    /// prevent.
+    QTimer *m_autoSyncTimer = nullptr;
+
     /// The optimistic update awaiting confirmation, kept so a worker error can
     /// put the model back. Only the most recent one: mutations are sent from
     /// the UI thread one user action at a time.