From 01194fa2c48019f1dcbdfefac94f8fd5f8e38122 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 7 Aug 2026 19:17:59 +0200 Subject: feat(sync): sync only the accounts with unsynced edits A sync ran mbsync -a regardless of what changed, so tagging mail in one account fetched all of them. The account set was not a parameter anywhere on the path: MailSync::start() took no arguments and the script hardcoded -a, so nothing between a tag edit and mbsync carried which account changed. Track which accounts have edits and pass their mbsync channels through to the script, which now takes channel names and falls back to -a when given none. An empty set means all accounts, per the request: a sync with nothing pending is a fetch, and narrowing that to wherever the last edit landed would quietly stop collecting mail everywhere else. The channel is a new optional per-account key rather than the section key. The two names genuinely diverge, because a QSettings section key may carry dots that the channel does not, and mbsync treats an unknown channel as fatal rather than skipping it, so key-as-channel would fail those accounts' syncs outright rather than degrade. It defaults to the key, so accounts whose two names already agree need no config change. The edited-account set is deliberately not netted the way the pending-edit map is: that map tracks the index, where a tag removed and re-added leaves nothing outstanding, while this tracks the mail store, where both writes have already renamed files that mbsync still has to propagate. It is also snapshotted before flushHeldEdits(), which inserts into it synchronously rather than on a queued reply, so a successful sync cannot clear accounts whose edits it never carried. Closes item 49. Co-Authored-By: Claude Opus 5 --- src/mainwindow.cpp | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 59 insertions(+), 3 deletions(-) (limited to 'src/mainwindow.cpp') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 0ce94a8..f830683 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -217,7 +217,7 @@ void MainWindow::closeEvent(QCloseEvent *event) box.exec(); if (box.clickedButton() == sync) { - if (m_sync->start()) { + if (m_sync->start(pendingSyncChannels())) { m_syncingForExit = true; m_syncLog->clear(); setSyncBusy(true); @@ -238,7 +238,7 @@ void MainWindow::closeEvent(QCloseEvent *event) return; } } else if (m_config.syncOnExit() == Config::SyncOnExit::Always) { - if (m_sync->start()) { + if (m_sync->start(pendingSyncChannels())) { m_syncingForExit = true; m_syncLog->clear(); setSyncBusy(true); @@ -1700,6 +1700,16 @@ void MainWindow::onSyncFinished(bool success, int exitCode) // so they go to the mail store on the NEXT run. That is the same one-run // delay any edit made mid-sync gets, bounded by the cron interval. const bool sentHeldEdits = !m_heldEdits.isEmpty(); + + // Snapshotted BEFORE the flush, and this ordering is load-bearing. + // flushHeldEdits() calls sendThreadTagChange(), which inserts into + // m_editedAccounts SYNCHRONOUSLY, unlike the pending-edit map below which + // is written on the worker's queued reply and so is safely counted rather + // than wiped. Clearing the whole set after the flush would therefore + // discard accounts whose edits this run did not carry, and those edits + // would sync only when some later edit happened to name the same account. + const QSet accountsThisRunCarried = m_editedAccounts; + flushHeldEdits(); if (success) { @@ -1708,6 +1718,12 @@ void MainWindow::onSyncFinished(bool success, int exitCode) // what failed to put them there. m_pendingTagEdits.clear(); m_unnettablePendingEdits = 0; + + // Only what this run actually carried, per the snapshot above. An + // account added by flushHeldEdits() stays, because its edit reaches the + // index after the sync that would have taken it and goes out on the + // next run. + m_editedAccounts.subtract(accountsThisRunCarried); m_lastSyncFailed = false; updatePendingIndicator(); @@ -1997,7 +2013,7 @@ void MainWindow::startSync() m_syncPhase.reset(); m_syncLineBuffer.clear(); - if (!m_sync->start()) { + if (!m_sync->start(pendingSyncChannels())) { showTransientStatus(tr("Sync already running")); return; } @@ -2022,6 +2038,34 @@ void MainWindow::recordPendingEdit(const QString &messageId, const QString &tag, m_pendingTagEdits.insert(key, added); } +QStringList MainWindow::pendingSyncChannels() const +{ + // Nothing pending means this run is a FETCH, and a fetch must cover every + // account: narrowing it to wherever the last edit happened to be would + // quietly stop collecting mail everywhere else. Empty is the signal for + // that, and MailSync::start() appends nothing. + if (m_editedAccounts.isEmpty()) + return {}; + + QStringList channels; + for (const Account &account : m_config.accounts()) { + if (m_editedAccounts.contains(account.key)) + channels.append(account.syncChannel()); + } + + // An account tag with no matching [account.] section yields no + // channel, and syncing a subset that omits it would leave its edits behind + // with nothing to say so. Fall back to a full sync, which is correct if + // wasteful; the alternative is silently stranding an edit. + if (channels.size() != m_editedAccounts.size()) + return {}; + + // Stable order so a run is reproducible and the log reads the same way + // twice. QSet has no order of its own. + channels.sort(); + return channels; +} + int MainWindow::pendingEditCount() const { // A held edit has NOT reached the index, so onTagsApplied() never counted @@ -2188,6 +2232,18 @@ void MainWindow::sendThreadTagChange(const QStringList &threadIds, for (const QString &threadId : threadIds) m_model->applyTagChange(threadId, add, remove); + // Which accounts this touches, recorded HERE and not in onTagsApplied(): + // TagChange carries message ids, while the account is a property of the + // thread, and by the time the worker confirms, the rows may be gone. A + // write that is later rejected leaves an account listed here that needed no + // sync, which costs one redundant channel on the next run; missing one + // would strand the user's edits, which is the failure worth avoiding. + for (const QString &threadId : threadIds) { + const QStringList keys = m_model->accountKeysForThread(threadId); + for (const QString &key : keys) + m_editedAccounts.insert(key); + } + // The strip shows the open thread's tags, so it has to follow a change to // that thread rather than waiting for the next selection. if (threadIds.contains(m_currentThreadId)) { -- cgit v1.2.3