fix(sync): re-arm the automatic sync when it skips a concurrent run

runAutoSync() returned without rescheduling when a sync was already in flight.
The comment defending it argued the edits were not lost, because they reached
the mail store at edit time and the running sync was "very likely" to carry
them. Very likely is not always: an edit made after mbsync has already passed
that account's mailbox is not carried by it, the timer had fired, nothing
re-armed it, and the pending count sat non-zero until a manual sync or the next
cron run.

Skipping is unchanged and still required by item 71: the cron job holds the same
lock and mbsync fails on a second concurrent run. What changes is that the skip
schedules another attempt. scheduleAutoSync() re-checks the delay, the sync
command and the pending count on the way in, so this cannot arm a sync for
nothing, and against a long external sync it re-arms once per debounce interval,
which is a timer rather than a sync.

The test fires the timer by hand and asserts it is active again afterwards, at
the configured interval rather than a shorter one, with the pending indicator
still showing. It fails against the old skip path.

Item 89's other half is dropped rather than built. The list churn it described
is a tag-defined view working as intended: a thread that loses `unread` leaves
the Unread view, and the user resolved it by living in the Inbox view instead.
Three designs were drafted before asking and none is worth building.

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

1 files changed, 62 insertions, 0 deletions

diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index b96b158..08b2f84 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -201,6 +201,7 @@ private slots:
     void aSingleMessageIdQuerysCardOpensInTheMessagePane();
     void autoSyncIsNotArmedWhenDisabledOrWithNothingPending();
     void autoSyncSkipsWhileABackgroundSyncIsRunning();
+    void aSkippedAutoSyncRearmsRatherThanGivingUp();
     void aSuccessfulSyncRefreshesRatherThanRerunningTheQuery();
     void markReadCanBeDisabled();
     void pendingEditCountSurvivesAQuery();
@@ -4044,6 +4045,67 @@ void TestMainWindow::autoSyncSkipsWhileABackgroundSyncIsRunning()
     // table, and handing the real one back would re-expose the next test.
 }
 
+void TestMainWindow::aSkippedAutoSyncRearmsRatherThanGivingUp()
+{
+    // Item 89, the concrete half. Skipping is correct and must stay, but the
+    // skip used to be the END of the attempt: the timer had fired, nothing
+    // re-armed it, and the edit waited for a manual sync or the next cron run.
+    //
+    // The comment defending it said the running sync was "very likely" to carry
+    // the edit, since it reached the mail store at edit time. Very likely is not
+    // always: an edit made after mbsync has already passed that account's
+    // mailbox is not carried by it, and the pending count then sits non-zero
+    // with nothing scheduled to clear it.
+    QTemporaryDir dir;
+    QVERIFY(dir.isValid());
+    const QString locks = dir.filePath(QStringLiteral("locks"));
+    {
+        QFile f(locks);
+        QVERIFY(f.open(QIODevice::WriteOnly));
+    }
+    MainWindow::setLocksPathForTesting(locks);
+
+    Config config;
+    config.load(writeSyncConfig(dir));
+
+    MainWindow window(config);
+    auto *timer = window.findChild<QTimer *>(QStringLiteral("autoSyncTimer"));
+    QVERIFY(timer);
+
+    QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+                              Q_ARG(SyncMonitor::State,
+                                    SyncMonitor::State::Running));
+
+    TagChange change;
+    change.messageIds = { QStringLiteral("m1") };
+    change.added = { QStringLiteral("flagged") };
+    QVERIFY(QMetaObject::invokeMethod(&window, "onTagsApplied",
+                                      Q_ARG(TagChange, change)));
+    QVERIFY2(timer->isActive(), "the edit did not arm the debounce at all");
+
+    // Fire it by hand rather than waiting out the delay. A QTimer that has
+    // fired is no longer active, so this is also what makes the assertion
+    // below mean something: without the re-arm it is inactive here.
+    timer->stop();
+    QVERIFY(QMetaObject::invokeMethod(&window, "runAutoSync"));
+
+    QVERIFY2(timer->isActive(),
+             "a skipped automatic sync left nothing armed to carry the edit");
+
+    // Re-armed at the configured debounce, not at some shorter interval that
+    // would spin against a long external sync. SyncMonitor polls /proc/locks,
+    // so an m_externalSyncBusy that never clears re-arms at this interval
+    // indefinitely, which is cheap only because the interval is the user's own.
+    QCOMPARE(timer->interval(), config.autoSyncDelayMs());
+
+    // The edit is still pending throughout: a retry must not look like a
+    // completed sync to the indicator.
+    auto *label = window.findChild<QLabel *>(QStringLiteral("pendingEdits"));
+    QVERIFY(label);
+    QVERIFY2(!label->isHidden(),
+             "the re-armed sync cleared the pending indicator");
+}
+
 void TestMainWindow::aSuccessfulSyncRefreshesRatherThanRerunningTheQuery()
 {
     // Reported by hand against item 71: reading a message in the Unread view,