aboutsummaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-29 11:25:02 +0200
committerDanilo M. <danix@danix.xyz>2026-08-29 11:25:02 +0200
commit3fd999907ae8344f76ee4e5be1ac278a26f452ca (patch)
tree17c01d44daee0147ecaca6cccd393471e6379cf1 /src
parent8c78dd139a77e896c72b7fc8b799af3c0df34344 (diff)
downloadqtmaildir-3fd999907ae8344f76ee4e5be1ac278a26f452ca.tar.gz
qtmaildir-3fd999907ae8344f76ee4e5be1ac278a26f452ca.zip
feat: have the sync script report what it did
Item 174, and half of item 125. The premise was corrected before any code. The note asks for an external `notmuch new` to clear the pending count; it must not. That count means tag mutations not yet known to have reached the MAIL STORE, which is the server: an edit is in notmuch the moment it is made, and what is outstanding is mbsync pushing the renamed Maildir files. `notmuch new` re-indexes local files and pushes nothing, so clearing on it would tell the user their work was safe to quit on while it was still local. The entry's own proposal to watch notmuch_database_get_revision() was rejected for the same reason: a revision moves when mail ARRIVES too, and in neither case does it say anything about the server. What was actually wrong was the reporting channel. The application inferred a finished run from an inode in /proc/locks and from grepping the log for its RUN END banner, which made a human-readable line into wire format and could not say WHICH channels a run carried. The local sync path has always narrowed its clear to the accounts it carried; the external path could not, and cleared everything, so an edit to an account a run never touched was reported as delivered. So the script reports instead of leaving evidence to be inferred. It writes ~/.local/state/qtmaildir/syncstatus.json atomically at the end of every run, including a skip, naming the channels, both exit statuses and a state of ok, failed or skipped. MailSync::readStatus() reads it, MainWindow prefers it over the log banner and narrows the clear through Account::syncChannel(). A skipped run clears nothing, which is item 125's first half: the application can now see that a run happened and carried nothing. The log banner and lastRunOutcome() stay as the fallback for a missing file, which is what a first run after upgrading looks like. This is the user's own framing of the scope: the script was written for another system and adapted, and is now qtmaildir's only consumer, so it serves the application rather than the reverse. Two facts made it safe to act on: their crontab runs mailsync.sh and nothing else touches mail, and ~/bin/mailsync.sh is a symlink into this repo, so an edit is live on the next tick. Two bugs found while wiring it in, both recorded in the closed item. A test read the developer's real sync state, twice: a [sync] section naming only `log` leaves syncStatus() defaulting to the real file, so two tests asserting that a FAILED run leaves the count alone read the last real cron run, found ok, and cleared. Pinning only `status` has the mirror problem. noSyncTestReadsTheRealSyncState() is the guard, modelled on noTestCanSeeTheRealLockTable(). And Qt::ISODate carries no milliseconds. The status file is preferred only when it describes THIS run, compared against when the lock appeared, so a stale success cannot outrank a fresh failure; but the script writes date -Iseconds, and a round trip of "now" comes back 329 ms behind, measured. A fast sync's own file therefore parsed as stale and fell back to the log, with nothing failing to say so. One second of slack matches the precision the format carries. Design: docs/superpowers/specs/2026-08-29-sync-status-file-design.md Suite: 43 of 44, with undoMovesTheMessageBack failing as it does on master (item 136).
Diffstat (limited to 'src')
-rw-r--r--src/config.cpp8
-rw-r--r--src/config.h6
-rw-r--r--src/mailsync.cpp90
-rw-r--r--src/mailsync.h80
-rw-r--r--src/mainwindow.cpp77
-rw-r--r--src/mainwindow.h25
6 files changed, 276 insertions, 10 deletions
diff --git a/src/config.cpp b/src/config.cpp
index 8b784ba..0f0caf1 100644
--- a/src/config.cpp
+++ b/src/config.cpp
@@ -475,6 +475,14 @@ void Config::load(const QString &path)
if (m_syncLog.isEmpty())
m_syncLog = MailSync::defaultLogPath();
+ // The status file the script writes (item 174), beside the log and unvalidated
+ // for the same reason: a fresh install has none until the first sync runs,
+ // and a missing one reads as SyncState::Unknown when the time comes.
+ m_syncStatus =
+ settings.value(QStringLiteral("sync/status")).toString().trimmed();
+ if (m_syncStatus.isEmpty())
+ m_syncStatus = MailSync::defaultStatusPath();
+
// Account groups are written as [account.work], [account.personal], etc.
// A dot, not a slash, separates the "account" namespace from the key:
// QSettings' INI backend treats "/" as its own hierarchical group
diff --git a/src/config.h b/src/config.h
index 26fc1c1..1ab923d 100644
--- a/src/config.h
+++ b/src/config.h
@@ -381,6 +381,11 @@ public:
/// clear on a cron sync, which is exactly the defect this exists to fix.
QString syncLog() const { return m_syncLog; }
+ /// The status file assets/mailsync.sh writes, which is what the
+ /// application READS to learn what a run it did not start actually did
+ /// (item 174). The log beside it is for a human.
+ QString syncStatus() const { return m_syncStatus; }
+
/// Optional alternate notmuch config file. Empty means "let notmuch decide".
QString notmuchConfig() const { return m_notmuchConfig; }
@@ -548,6 +553,7 @@ private:
QString m_syncCommand;
QString m_syncLog;
+ QString m_syncStatus;
int m_toolbarIconSize = 24;
QString m_notmuchConfig;
QString m_dateFormat;
diff --git a/src/mailsync.cpp b/src/mailsync.cpp
index 10e5ef7..caf61ef 100644
--- a/src/mailsync.cpp
+++ b/src/mailsync.cpp
@@ -21,6 +21,9 @@
#include <QCoreApplication>
#include <QDir>
#include <QFile>
+#include <QJsonArray>
+#include <QJsonDocument>
+#include <QJsonObject>
#include <QRegularExpression>
namespace {
@@ -251,6 +254,93 @@ QString MailSync::defaultLogPath()
return QDir::homePath() + QStringLiteral("/.local/state/mailsync.log");
}
+QString MailSync::defaultStatusPath()
+{
+ // Hardcoded to match assets/mailsync.sh for the same reason defaultLogPath
+ // is: the script builds it from $HOME, and QStandardPaths would derive a
+ // path the script never writes.
+ //
+ // Under the application's own state directory rather than beside
+ // mailsync.log, and the split is deliberate: the log belongs to the script
+ // and a human reads it, while this file is the interface between the two
+ // programs.
+ return QDir::homePath()
+ + QStringLiteral("/.local/state/qtmaildir/syncstatus.json");
+}
+
+SyncStatus MailSync::readStatus(const QString &statusPath)
+{
+ // Every failure below returns this untouched, so Unknown is the default
+ // rather than something each branch has to remember to set.
+ SyncStatus status;
+
+ if (statusPath.isEmpty())
+ return status;
+
+ QFile file(statusPath);
+ if (!file.open(QIODevice::ReadOnly))
+ return status;
+
+ // The whole file: it holds one run and is a few hundred bytes. The cap is
+ // against a path that is not the file we think it is, since a reader on the
+ // UI thread must not swallow something enormous by mistake.
+ constexpr qint64 kMaxBytes = 64 * 1024;
+ if (file.size() > kMaxBytes)
+ return status;
+
+ QJsonParseError error{};
+ const QJsonDocument doc = QJsonDocument::fromJson(file.readAll(), &error);
+ if (error.error != QJsonParseError::NoError || !doc.isObject())
+ return status;
+
+ const QJsonObject object = doc.object();
+
+ // Refused rather than guessed at, the rule the rules file already follows:
+ // a later version may mean something different by these same field names,
+ // and acting on it would be worse than observing nothing. The script writes
+ // 1 and both sides bump together.
+ if (object.value(QStringLiteral("version")).toInt() != 1)
+ return status;
+
+ const QString state = object.value(QStringLiteral("state")).toString();
+ if (state == QLatin1String("ok"))
+ status.state = SyncState::Ok;
+ else if (state == QLatin1String("failed"))
+ status.state = SyncState::Failed;
+ else if (state == QLatin1String("skipped"))
+ status.state = SyncState::Skipped;
+ else
+ return status; // An unrecognised state is not a fourth kind of run.
+
+ const QJsonArray channels =
+ object.value(QStringLiteral("channels")).toArray();
+ for (const QJsonValue &value : channels) {
+ const QString channel = value.toString();
+ if (channel.isEmpty())
+ continue;
+ // "-a" is the script's word for "every channel", not the name of one.
+ // Kept as a flag so a caller cannot match it against configured
+ // channels, find nothing, and clear nothing on the run that carried
+ // everything.
+ if (channel == QLatin1String("-a"))
+ status.everyChannel = true;
+ else
+ status.channels.append(channel);
+ }
+
+ status.mbsyncStatus =
+ object.value(QStringLiteral("mbsync_status")).toInt(-1);
+ status.notmuchStatus =
+ object.value(QStringLiteral("notmuch_status")).toInt(-1);
+
+ status.started = QDateTime::fromString(
+ object.value(QStringLiteral("started")).toString(), Qt::ISODate);
+ status.ended = QDateTime::fromString(
+ object.value(QStringLiteral("ended")).toString(), Qt::ISODate);
+
+ return status;
+}
+
SyncOutcome MailSync::lastRunOutcome(const QString &logPath)
{
if (logPath.isEmpty())
diff --git a/src/mailsync.h b/src/mailsync.h
index a826f43..d614dbc 100644
--- a/src/mailsync.h
+++ b/src/mailsync.h
@@ -18,6 +18,7 @@
#pragma once
+#include <QDateTime>
#include <QObject>
#include <QProcess>
#include <QString>
@@ -76,6 +77,64 @@ enum class SyncOutcome {
Failed,
};
+/// What a finished run was, from the status file (item 174).
+///
+/// Three states rather than SyncOutcome's two, and the third is the point:
+/// a run that SKIPPED because another held the lock is neither a success nor a
+/// failure, and having no way to say so is why item 125 left the spinner
+/// running for ever.
+enum class SyncState {
+ Unknown,
+ Ok,
+ Failed,
+ Skipped,
+};
+
+/// One finished run of the sync script, as the script itself reported it.
+///
+/// This exists because the application used to INFER a finished run, from an
+/// inode in /proc/locks and from grepping the log for its RUN END banner. That
+/// made a human-readable line into wire format, and it could not answer the
+/// question the pending count actually needs answered: which channels did this
+/// run carry? The local sync path has always narrowed its clear to the accounts
+/// it carried; the external path could not, and cleared everything.
+///
+/// Written by assets/mailsync.sh, which is the only producer. The two agree by
+/// TEST rather than by shared code, exactly as the two readers of rules.json
+/// do: assets/test_mailsync.py pins the writer, test_mailsync.cpp pins the
+/// reader, and one test runs the real script and reads what it wrote.
+struct SyncStatus
+{
+ SyncState state = SyncState::Unknown;
+
+ /// The channels the run synced. Empty when `everyChannel` is true.
+ QStringList channels;
+
+ /// The run covered every account, which the script reports as "-a".
+ ///
+ /// Carried as a flag rather than left as the literal string in `channels`,
+ /// because "-a" is not a channel name: a caller matching it against
+ /// configured channels finds nothing and clears nothing, on exactly the run
+ /// that carried everything.
+ bool everyChannel = false;
+
+ /// Reported separately as well as folded into `state`, because they mean
+ /// different things: a failed mbsync means the edits never reached the
+ /// server, while a failed notmuch means they did and only the local index
+ /// is behind. -1 for a run where neither program ran.
+ int mbsyncStatus = -1;
+ int notmuchStatus = -1;
+
+ QDateTime started;
+ QDateTime ended;
+
+ /// True only for a run that completed with both programs succeeding.
+ /// Nothing else may clear the pending count, per the rule the local path
+ /// states: clearing on a failure asserts the edits reached the mail store
+ /// when the sync is exactly what failed to put them there.
+ bool carriedEdits() const { return state == SyncState::Ok; }
+};
+
/// Runs the configured external sync command.
///
/// qtmaildir deliberately does not implement sync itself. The existing script
@@ -125,6 +184,27 @@ public:
/// Anything unreadable, absent or unmarked is Unknown.
static SyncOutcome lastRunOutcome(const QString &logPath);
+ /// Reads the status file assets/mailsync.sh writes (item 174).
+ ///
+ /// Preferred over lastRunOutcome(), which stays as the fallback for a file
+ /// that is missing or unreadable: that is what a first run after upgrading
+ /// looks like, and deleting a working mechanism in the same change that
+ /// adds its replacement leaves two broken things instead of one.
+ ///
+ /// Anything unreadable, absent, malformed or of an unrecognised version
+ /// returns a default SyncStatus, whose state is Unknown. Callers must
+ /// change no state on Unknown, exactly as they must for SyncOutcome and
+ /// SyncMonitor::State: it is the absence of evidence, not evidence of
+ /// absence.
+ ///
+ /// A whole-file read rather than a tail, unlike lastRunOutcome(): the file
+ /// holds one run and is a few hundred bytes, where the log holds every run
+ /// of the day.
+ static SyncStatus readStatus(const QString &statusPath);
+
+ /// Where the status file lives when the config names none.
+ static QString defaultStatusPath();
+
signals:
void started();
void outputReceived(const QString &chunk);
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 1da9ac7..01c0251 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -5089,6 +5089,10 @@ void MainWindow::onExternalSyncStateChanged(SyncMonitor::State state)
return;
m_externalSyncBusy = true;
+ // When this run began, so the status file it leaves can be told from
+ // one an earlier run left (item 174). A stale file must not be read as
+ // this run's result.
+ m_externalSyncStartedAt = QDateTime::currentDateTime();
updateSyncControls();
m_statusLabel->setText(tr("Background sync running..."));
m_announcedExternalSync = true;
@@ -5174,12 +5178,54 @@ void MainWindow::onExternalSyncStateChanged(SyncMonitor::State state)
// Without this the indicator kept reporting work that had already
// shipped, and the exit prompt asked to sync for it.
//
- // The outcome comes from the RUN END line the script writes, because
- // the process that ran this sync is gone and its exit status with it.
- // Anything other than a definite OK changes nothing: the local path's
- // rule is that only a SUCCESSFUL sync may clear the count, and Unknown
- // is the absence of evidence rather than evidence of success.
- if (MailSync::lastRunOutcome(m_config.syncLog()) == SyncOutcome::Ok) {
+ // The process that ran this sync is gone and its exit status with it,
+ // so what it did has to be read from what it left behind.
+ //
+ // Item 174: the status file the script writes, which says which
+ // CHANNELS the run carried. The RUN END line in the log remains the
+ // fallback for a status file that is missing or unreadable, which is
+ // what a first run after upgrading looks like; it cannot name channels,
+ // so that path keeps the old blanket clear.
+ //
+ // Anything other than a definite success changes nothing, on either
+ // path: the local rule is that only a SUCCESSFUL sync may clear the
+ // count, and Unknown is the absence of evidence rather than evidence of
+ // success. A SKIPPED run is neither, and clears nothing: the other run
+ // is doing the work and this one carried none of it.
+ // The status file is preferred, but only when it describes THIS run.
+ // A stale one outranking a fresh log would be worse than not having it:
+ // an old success would clear the count for a run that has just failed,
+ // which is the indicator lying in the direction that loses work.
+ //
+ // "This run" is judged on the file being at least as new as the sync
+ // that just ended. m_externalSyncStartedAt is when the lock appeared,
+ // and the script writes the file immediately before exiting, so a file
+ // older than that belongs to an earlier run.
+ const SyncStatus status = MailSync::readStatus(m_config.syncStatus());
+
+ // The script writes `date -Iseconds`, which carries no milliseconds, so
+ // a file written in the same second as the lock appeared parses as up
+ // to 999ms EARLIER than it. Measured: an ISODate round trip of "now"
+ // comes back 329ms behind. A plain `>=` therefore judges a fast sync's
+ // own status file stale and falls back to the log, which is the
+ // opposite of the intent.
+ //
+ // One second of slack, matching the precision the format actually
+ // carries. This cannot readmit a genuinely stale file: cron runs ten
+ // minutes apart, and a run whose file is a second old IS this run.
+ constexpr qint64 kTimestampSlackMs = 1000;
+ const bool statusIsForThisRun =
+ status.state != SyncState::Unknown
+ && (!m_externalSyncStartedAt.isValid() || !status.ended.isValid()
+ || status.ended.msecsTo(m_externalSyncStartedAt)
+ <= kTimestampSlackMs);
+
+ const bool carried =
+ statusIsForThisRun
+ ? status.carriedEdits()
+ : MailSync::lastRunOutcome(m_config.syncLog()) == SyncOutcome::Ok;
+
+ if (carried) {
m_pendingTagEdits.clear();
// Cleared HERE, before flushHeldEdits() below, and the ordering is
@@ -5188,10 +5234,21 @@ void MainWindow::onExternalSyncStateChanged(SyncMonitor::State state)
// writes m_editedAccounts SYNCHRONOUSLY. Clearing after the flush
// would discard accounts whose edits this run did not carry, and
// those edits would then sync only when some later edit happened to
- // name the same account. Running first, everything in the set at
- // this moment is exactly what the finished sync carried, so the
- // local path's snapshot-and-subtract collapses to a clear.
- m_editedAccounts.clear();
+ // name the same account.
+ //
+ // WHICH accounts, when the status file said. A run naming channels
+ // carried those and no others, so clearing the whole set would
+ // report an untouched account's edits as shipped. `-a` and the log
+ // fallback both mean every account, where the blanket clear is
+ // right.
+ if (!status.everyChannel && !status.channels.isEmpty()) {
+ for (const Account &account : m_config.accounts()) {
+ if (status.channels.contains(account.syncChannel()))
+ m_editedAccounts.remove(account.key);
+ }
+ } else {
+ m_editedAccounts.clear();
+ }
updatePendingIndicator();
}
}
diff --git a/src/mainwindow.h b/src/mainwindow.h
index 2b217d2..9c93f4f 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -202,6 +202,20 @@ public:
/// command was pushed, which is what "this did nothing" has to assert.
int undoDepthForTesting() const { return m_undoStack.count(); }
+ /// Item 174. The set of accounts with edits not yet known to have reached
+ /// the mail store, so a test can assert that an external sync cleared the
+ /// accounts it carried and ONLY those.
+ QSet<QString> editedAccountsForTesting() const { return m_editedAccounts; }
+
+ /// Marks an account edited, standing in for the write funnels: a test
+ /// asserting which accounts a sync clears needs more than one of them
+ /// edited, and driving two real writes through a worker to arrange that
+ /// would test the funnels rather than the clearing.
+ Q_INVOKABLE void noteEditedAccountForTesting(const QString &accountKey)
+ {
+ m_editedAccounts.insert(accountKey);
+ }
+
/// Item 178. Stands in for the digest round trip, which a bare window has
/// no worker to make. Sets what onThreadDigestLoaded() would have set.
void setConversationPathsForTesting(const QString &threadId,
@@ -1485,6 +1499,17 @@ private:
/// half. Tracked here rather than read back from SyncMonitor so the state
/// the UI acted on is the state it was told about.
bool m_externalSyncBusy = false;
+
+ /// When the external sync now running began, from the lock appearing.
+ ///
+ /// Item 174. The status file the script leaves is preferred over the log's
+ /// banner, but only when it describes THIS run: a stale file outranking a
+ /// fresh log would clear the pending count on an old success for a run that
+ /// has just failed, which is the indicator lying in the direction that
+ /// loses work. Invalid when no external sync has been observed, where the
+ /// comparison is skipped rather than failing closed on a file that may well
+ /// be current.
+ QDateTime m_externalSyncStartedAt;
QUndoStack m_undoStack;
QLineEdit *m_queryEdit = nullptr;