aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-03 09:01:32 +0200
committerDanilo M. <danix@danix.xyz>2026-08-03 09:01:32 +0200
commit376323043a6b7a02e23cd9b6232bc25ec5fc1ce6 (patch)
tree4140c48b8c1b493c2998bd094dca76613f0d2917
parentc6c1011222361b446fb3cdee1ad324fe54931abc (diff)
downloadqtmaildir-376323043a6b7a02e23cd9b6232bc25ec5fc1ce6.tar.gz
qtmaildir-376323043a6b7a02e23cd9b6232bc25ec5fc1ce6.zip
feat: add MailSync process wrapper
Runs the configured sync script through QProcess, merging stdout and stderr into one log so a failing mbsync run has something to show. The script is never run through a shell: the command is a config value, and splitCommand keeps its arguments literal. Two fixes against the drafted version: - start() no longer calls waitForStarted(). It blocked the UI thread for up to five seconds, which contradicts the spec's requirement that the UI stay usable during sync, and it swallowed launch failures into a bare false return. A missing script now surfaces asynchronously through errorOccurred as finished(false, -1) with an explanatory log line, so the spinner cannot hang with nothing to explain it. - Removed a double-emit guard I had added on the assumption that QProcess follows errorOccurred(FailedToStart) with finished(). Verified it does not: FailedToStart is emitted instead of finished, never before it. The guard was dead state and the comment justifying it was wrong. Also corrects the sync interval throughout: the user's cron runs every 10 minutes, not hourly. The shorter interval strengthens the flock rationale rather than weakening it, since collisions with a manual sync are that much more likely. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
-rw-r--r--docs/superpowers/plans/2026-08-02-qtmaildir-v1.md8
-rw-r--r--docs/superpowers/specs/2026-08-02-qtmaildir-design.md4
-rw-r--r--src/CMakeLists.txt1
-rw-r--r--src/mailsync.cpp83
-rw-r--r--src/mailsync.h44
-rw-r--r--tests/CMakeLists.txt1
-rw-r--r--tests/test_mailsync.cpp239
7 files changed, 374 insertions, 6 deletions
diff --git a/docs/superpowers/plans/2026-08-02-qtmaildir-v1.md b/docs/superpowers/plans/2026-08-02-qtmaildir-v1.md
index 1e14105..41bf1bb 100644
--- a/docs/superpowers/plans/2026-08-02-qtmaildir-v1.md
+++ b/docs/superpowers/plans/2026-08-02-qtmaildir-v1.md
@@ -3200,10 +3200,10 @@ Expected: FAIL, `mailsync.h: No such file or directory`.
/// Runs the configured external sync command.
///
/// qtmaildir deliberately does not implement sync itself. The existing script
-/// holds a flock that is the shared mutex between the user's hourly cron sync
-/// and any manual sync; running the script joins that mutex, whereas a built-in
-/// implementation would sit outside it and could run mbsync concurrently with
-/// cron, corrupting Maildir UID state.
+/// holds a flock that is the shared mutex between the user's cron sync, which
+/// runs every 10 minutes, and any manual sync; running the script joins that
+/// mutex, whereas a built-in implementation would sit outside it and could run
+/// mbsync concurrently with cron, corrupting Maildir UID state.
class MailSync : public QObject
{
Q_OBJECT
diff --git a/docs/superpowers/specs/2026-08-02-qtmaildir-design.md b/docs/superpowers/specs/2026-08-02-qtmaildir-design.md
index 3089341..02c969e 100644
--- a/docs/superpowers/specs/2026-08-02-qtmaildir-design.md
+++ b/docs/superpowers/specs/2026-08-02-qtmaildir-design.md
@@ -94,7 +94,7 @@ notmuch access happens on that thread; the UI never blocks.
**Sync.** qtmaildir runs a configured external command rather than
reimplementing `mbsync` orchestration. The decisive reason is the `flock`
guard in the existing script: it is the shared mutex between the user's
-hourly cron sync and any manual sync. Reimplementing the sync internally
+cron sync (every 10 minutes) and any manual sync. Reimplementing it internally
would place qtmaildir outside that mutex, and two concurrent `mbsync -a` runs
on one Maildir corrupt UID state. Calling the script joins the mutex for
free. Reimplementing would also not remove the dependency, since `mbsync`
@@ -379,7 +379,7 @@ query re-runs so new mail appears. On non-zero, the status bar shows the last
stderr lines with a "Show log" link.
Sync never runs automatically in v1: no timer, no sync on startup. The user's
-cron already syncs hourly, and a second scheduler competing with the first is
+cron already syncs every 10 minutes, and a second scheduler competing with it is
exactly what the script's `flock` guard exists to prevent. The button means
"now".
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 5f01128..6a2b18e 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -7,6 +7,7 @@ add_library(qtmaildir_lib STATIC
cidschemehandler.cpp
notmuchworker.cpp
threadlistmodel.cpp
+ mailsync.cpp
)
target_include_directories(qtmaildir_lib
diff --git a/src/mailsync.cpp b/src/mailsync.cpp
new file mode 100644
index 0000000..fdc433f
--- /dev/null
+++ b/src/mailsync.cpp
@@ -0,0 +1,83 @@
+#include "mailsync.h"
+
+MailSync::MailSync(const QString &command, QObject *parent)
+ : QObject(parent), m_command(command)
+{
+ // mbsync reports failures on stderr, so both channels go into one log:
+ // splitting them would leave the pane empty for the runs worth reading.
+ m_process.setProcessChannelMode(QProcess::MergedChannels);
+
+ connect(&m_process, &QProcess::readyRead,
+ this, &MailSync::handleReadyRead);
+ connect(&m_process, &QProcess::finished,
+ this, &MailSync::handleFinished);
+ connect(&m_process, &QProcess::errorOccurred,
+ this, &MailSync::handleError);
+}
+
+bool MailSync::isRunning() const
+{
+ return m_process.state() != QProcess::NotRunning;
+}
+
+bool MailSync::start()
+{
+ if (!isAvailable() || isRunning())
+ return false;
+
+ // splitCommand handles quoted arguments; running through a shell would make
+ // a config value into an injection point.
+ const QStringList parts = QProcess::splitCommand(m_command);
+ if (parts.isEmpty())
+ return false;
+
+ m_log.clear();
+
+ m_process.setProgram(parts.first());
+ m_process.setArguments(parts.mid(1));
+
+ // Deliberately no waitForStarted(): the spec requires the UI stay usable
+ // during sync, and a failed launch arrives via errorOccurred() instead.
+ m_process.start();
+
+ emit started();
+ return true;
+}
+
+void MailSync::handleReadyRead()
+{
+ const QByteArray data = m_process.readAll();
+ if (data.isEmpty())
+ return;
+
+ const QString chunk = QString::fromUtf8(data);
+ m_log += chunk;
+ emit outputReceived(chunk);
+}
+
+void MailSync::handleFinished(int exitCode, QProcess::ExitStatus status)
+{
+ // Drain anything buffered at exit.
+ handleReadyRead();
+
+ // No guard against a preceding launch failure is needed: verified that
+ // QProcess emits errorOccurred(FailedToStart) *instead of* finished(),
+ // not before it.
+ const bool success = status == QProcess::NormalExit && exitCode == 0;
+ emit finished(success, exitCode);
+}
+
+void MailSync::handleError(QProcess::ProcessError error)
+{
+ // Config validates the path at load time, but the script can be deleted or
+ // its filesystem unmounted afterwards. Without this the spinner would stay
+ // up forever with nothing explaining why.
+ if (error != QProcess::FailedToStart)
+ return;
+
+ const QString message =
+ QStringLiteral("Failed to start sync command: %1\n").arg(m_command);
+ m_log += message;
+ emit outputReceived(message);
+ emit finished(false, -1);
+}
diff --git a/src/mailsync.h b/src/mailsync.h
new file mode 100644
index 0000000..af4acbc
--- /dev/null
+++ b/src/mailsync.h
@@ -0,0 +1,44 @@
+#pragma once
+
+#include <QObject>
+#include <QProcess>
+#include <QString>
+
+/// Runs the configured external sync command.
+///
+/// qtmaildir deliberately does not implement sync itself. The existing script
+/// holds a flock that is the shared mutex between the user's cron sync, which
+/// runs every 10 minutes, and any manual sync; running the script joins that
+/// mutex, whereas a built-in implementation would sit outside it and could run
+/// mbsync concurrently with cron, corrupting Maildir UID state.
+class MailSync : public QObject
+{
+ Q_OBJECT
+public:
+ explicit MailSync(const QString &command, QObject *parent = nullptr);
+
+ /// False when no command is configured; the UI disables its Sync button.
+ bool isAvailable() const { return !m_command.isEmpty(); }
+ bool isRunning() const;
+
+ /// Returns false if unavailable or already running. A true return means the
+ /// process was handed to the event loop, not that it launched successfully:
+ /// a missing binary surfaces asynchronously through finished(false, ...).
+ bool start();
+
+ QString log() const { return m_log; }
+
+signals:
+ void started();
+ void outputReceived(const QString &chunk);
+ void finished(bool success, int exitCode);
+
+private:
+ void handleReadyRead();
+ void handleFinished(int exitCode, QProcess::ExitStatus status);
+ void handleError(QProcess::ProcessError error);
+
+ QString m_command;
+ QProcess m_process;
+ QString m_log;
+};
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index f970385..c2bfc8a 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -14,3 +14,4 @@ add_qtmaildir_test(interceptor)
add_qtmaildir_test(htmlbuilder)
add_qtmaildir_test(notmuchworker)
add_qtmaildir_test(threadlistmodel)
+add_qtmaildir_test(mailsync)
diff --git a/tests/test_mailsync.cpp b/tests/test_mailsync.cpp
new file mode 100644
index 0000000..cb8d454
--- /dev/null
+++ b/tests/test_mailsync.cpp
@@ -0,0 +1,239 @@
+#include <QElapsedTimer>
+#include <QFile>
+#include <QSignalSpy>
+#include <QTemporaryDir>
+#include <QtTest>
+
+#include "mailsync.h"
+
+class TestMailSync : public QObject
+{
+ Q_OBJECT
+private slots:
+ void initTestCase();
+
+ void unavailableWhenCommandEmpty();
+ void unavailableWhenCommandIsOnlyWhitespace();
+ void successfulRunEmitsFinished();
+ void failedRunReportsExitCode();
+ void capturesOutput();
+ void capturesStderrToo();
+ void emitsStartedSignal();
+ void logIsClearedBetweenRuns();
+ void refusesConcurrentRuns();
+ void canRunAgainAfterFinishing();
+ void missingBinaryReportsFailureNotSilence();
+ void startDoesNotBlock();
+ void argumentsAreNotShellInterpreted();
+
+private:
+ /// Writes an executable shell script into the temp dir, returns its path.
+ QString makeScript(const QString &name, const QString &body);
+
+ QTemporaryDir m_dir;
+};
+
+void TestMailSync::initTestCase()
+{
+ QVERIFY(m_dir.isValid());
+}
+
+QString TestMailSync::makeScript(const QString &name, const QString &body)
+{
+ const QString path = m_dir.filePath(name);
+ QFile file(path);
+ if (!file.open(QIODevice::WriteOnly | QIODevice::Text))
+ return QString();
+ file.write("#!/bin/sh\n");
+ file.write(body.toUtf8());
+ file.write("\n");
+ file.close();
+ file.setPermissions(QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner);
+ return path;
+}
+
+void TestMailSync::unavailableWhenCommandEmpty()
+{
+ // Braces, not parens: MailSync sync(QString()) is a function declaration.
+ MailSync sync{ QString() };
+ QVERIFY(!sync.isAvailable());
+ QVERIFY(!sync.start());
+}
+
+void TestMailSync::unavailableWhenCommandIsOnlyWhitespace()
+{
+ // A config line like `command = ` reaches here as spaces, not as empty.
+ // splitCommand yields nothing for it, so start() must refuse rather than
+ // try to launch an empty program name.
+ MailSync sync(QStringLiteral(" "));
+ QVERIFY(!sync.start());
+ QVERIFY(!sync.isRunning());
+}
+
+void TestMailSync::successfulRunEmitsFinished()
+{
+ MailSync sync(makeScript(QStringLiteral("ok.sh"), QStringLiteral("exit 0")));
+ QVERIFY(sync.isAvailable());
+
+ QSignalSpy spy(&sync, &MailSync::finished);
+ QVERIFY(sync.start());
+ QVERIFY(spy.wait(5000));
+
+ QCOMPARE(spy.count(), 1);
+ QCOMPARE(spy.first().at(0).toBool(), true);
+ QCOMPARE(spy.first().at(1).toInt(), 0);
+}
+
+void TestMailSync::failedRunReportsExitCode()
+{
+ MailSync sync(makeScript(QStringLiteral("fail.sh"), QStringLiteral("exit 3")));
+
+ QSignalSpy spy(&sync, &MailSync::finished);
+ QVERIFY(sync.start());
+ QVERIFY(spy.wait(5000));
+
+ QCOMPARE(spy.first().at(0).toBool(), false);
+ // The exit code reaches the UI: the status bar shows why sync failed.
+ QCOMPARE(spy.first().at(1).toInt(), 3);
+}
+
+void TestMailSync::capturesOutput()
+{
+ MailSync sync(makeScript(QStringLiteral("talk.sh"),
+ QStringLiteral("echo syncing")));
+
+ QSignalSpy spy(&sync, &MailSync::finished);
+ QSignalSpy output(&sync, &MailSync::outputReceived);
+ QVERIFY(sync.start());
+ QVERIFY(spy.wait(5000));
+
+ QVERIFY(sync.log().contains(QStringLiteral("syncing")));
+ QVERIFY(!output.isEmpty());
+}
+
+void TestMailSync::capturesStderrToo()
+{
+ // mbsync reports failures on stderr. If only stdout were captured, the log
+ // pane would be empty for exactly the runs the user needs to read.
+ MailSync sync(makeScript(QStringLiteral("noisy.sh"),
+ QStringLiteral("echo boom >&2\nexit 1")));
+
+ QSignalSpy spy(&sync, &MailSync::finished);
+ QVERIFY(sync.start());
+ QVERIFY(spy.wait(5000));
+
+ QVERIFY(sync.log().contains(QStringLiteral("boom")));
+}
+
+void TestMailSync::emitsStartedSignal()
+{
+ MailSync sync(makeScript(QStringLiteral("started.sh"), QStringLiteral("exit 0")));
+
+ QSignalSpy started(&sync, &MailSync::started);
+ QSignalSpy finished(&sync, &MailSync::finished);
+ QVERIFY(sync.start());
+ QVERIFY(finished.wait(5000));
+
+ QCOMPARE(started.count(), 1);
+}
+
+void TestMailSync::logIsClearedBetweenRuns()
+{
+ MailSync sync(makeScript(QStringLiteral("once.sh"), QStringLiteral("echo first")));
+
+ QSignalSpy spy(&sync, &MailSync::finished);
+ QVERIFY(sync.start());
+ QVERIFY(spy.wait(5000));
+ QVERIFY(sync.log().contains(QStringLiteral("first")));
+
+ QVERIFY(sync.start());
+ QVERIFY(spy.wait(5000));
+ // Stale output from the previous run must not accumulate: the log pane is
+ // meant to show what this sync did.
+ QCOMPARE(sync.log().count(QStringLiteral("first")), 1);
+}
+
+void TestMailSync::refusesConcurrentRuns()
+{
+ MailSync sync(makeScript(QStringLiteral("slow.sh"), QStringLiteral("sleep 2")));
+ QVERIFY(sync.start());
+ // The cron sync and this one share a flock; starting twice from the GUI is
+ // still refused locally so the button cannot queue runs.
+ QVERIFY(!sync.start());
+ QVERIFY(sync.isRunning());
+
+ QSignalSpy spy(&sync, &MailSync::finished);
+ QVERIFY(spy.wait(10000));
+}
+
+void TestMailSync::canRunAgainAfterFinishing()
+{
+ MailSync sync(makeScript(QStringLiteral("again.sh"), QStringLiteral("exit 0")));
+
+ QSignalSpy spy(&sync, &MailSync::finished);
+ QVERIFY(sync.start());
+ QVERIFY(spy.wait(5000));
+ QVERIFY(!sync.isRunning());
+
+ // A refusal must be about "currently running", not a one-shot latch.
+ QVERIFY(sync.start());
+ QVERIFY(spy.wait(5000));
+ QCOMPARE(spy.count(), 2);
+}
+
+void TestMailSync::missingBinaryReportsFailureNotSilence()
+{
+ // Config checks the path at load time, but the script can be deleted or
+ // unmounted afterwards. Failing silently would leave the spinner up
+ // forever with no explanation.
+ MailSync sync(m_dir.filePath(QStringLiteral("definitely-not-here.sh")));
+ QVERIFY(sync.isAvailable());
+
+ QSignalSpy spy(&sync, &MailSync::finished);
+ QVERIFY(sync.start());
+ QVERIFY(spy.wait(5000));
+
+ QCOMPARE(spy.first().at(0).toBool(), false);
+ QVERIFY(!sync.log().isEmpty());
+ QVERIFY(!sync.isRunning());
+}
+
+void TestMailSync::startDoesNotBlock()
+{
+ // Spec: "The UI stays usable during sync." start() must hand off to the
+ // event loop rather than waiting for the process.
+ MailSync sync(makeScript(QStringLiteral("blocker.sh"), QStringLiteral("sleep 2")));
+
+ QElapsedTimer timer;
+ timer.start();
+ QVERIFY(sync.start());
+ const qint64 elapsed = timer.elapsed();
+
+ QVERIFY2(elapsed < 500,
+ qPrintable(QStringLiteral("start() blocked for %1ms").arg(elapsed)));
+
+ QSignalSpy spy(&sync, &MailSync::finished);
+ QVERIFY(spy.wait(10000));
+}
+
+void TestMailSync::argumentsAreNotShellInterpreted()
+{
+ // The command comes from a config file. Running it through a shell would
+ // turn that value into an injection point, so the metacharacters below must
+ // arrive at the script as literal arguments.
+ const QString script = makeScript(QStringLiteral("args.sh"),
+ QStringLiteral("echo \"$1\""));
+
+ MailSync sync(QStringLiteral("%1 \"; touch %2/pwned\"")
+ .arg(script, m_dir.path()));
+
+ QSignalSpy spy(&sync, &MailSync::finished);
+ QVERIFY(sync.start());
+ QVERIFY(spy.wait(5000));
+
+ QVERIFY(!QFile::exists(m_dir.filePath(QStringLiteral("pwned"))));
+ QVERIFY(sync.log().contains(QStringLiteral("; touch")));
+}
+
+QTEST_MAIN(TestMailSync)
+#include "test_mailsync.moc"