fix(sync): hold tag edits made during a background sync

A tag edit sent while another process holds notmuch's write lock does not
fail: the read-write open blocks and then succeeds. Measured against
Slackware's notmuch, 9.158s against a 12s hold, status SUCCESS. Since the
worker is a single thread, that blocked open holds up every read queued
behind it, so the message pane freezes on whichever thread was selected
first and replays the queue when the lock releases.

The window now defers instead. While SyncMonitor reports a sync running, a
tag change is held rather than sent, and flushed when the sync ends. The
optimistic update stands in the meantime, so the row keeps its tag and the
edit still counts toward the unsynced indicator, which is what the quit
prompt reads.

The original diagnosis was that the open fails and the edit is discarded,
and a retry was built on it. That was wrong: the error branch in
notmuchworker.cpp is unreachable through lock contention. The premise was
taken from a plausible-looking error path without provoking the condition,
and measurement disproved it. The backlog entry records this rather than
quietly correcting it.

Verified by hand against a real blocking open, which the tests cannot reach:
they drive the deferral through the meta-object and never take a lock. Both
locks held for 100s with a tag edit made during the hold. Row kept the tag,
status did not expire, indicator rose, window stayed responsive, held edit
sent itself on release.

The 2s SyncMonitor polling window is knowingly left open: a sync starting
between polls is invisible for up to 2s and an edit there still blocks.
SyncMonitor::lockHeldIn() would close it at the cost of one file read per
tag action, and is recorded as the option to revisit.

Also fixes revertPendingTagChange() clearing the entire undo stack after any
rejected write, found while working on this.

Backlog: item 37 done, and item 46 added for a test that fails only under
the offscreen platform, where an 800x800 screen clamps a restored 940px
window. Pre-existing and unrelated; the suite is green otherwise.

1 files changed, 203 insertions, 0 deletions

diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
index 99e3b64..0186bfa 100644
--- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
+++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
@@ -84,6 +84,8 @@ taking that too literally.
 | 34 | No overview of the Maildir itself | information | M | open |
 | 35 | No refresh of the thread list after a sync | workflow | M | open |
 | 36 | `test_mainwindow` cannot reach the worker | testing | S | open, on demand |
+| 37 | The worker stalls on a tag edit made during a background sync | correctness | S | **done** |
+| 38 | `test_mainwindow` fails when a real sync holds the lock | testing | XS | open |
 | 39 | Thread list cannot be sorted by clicking a column header | workflow | S | open |
 | 40 | No live filter over the current view | workflow | M | open |
 | 41 | A message whose HTML body carries a `Content-Id` renders blank | correctness | S | open |
@@ -91,6 +93,7 @@ taking that too literally.
 | 43 | No "Mark all read" for the current view | workflow | S | open |
 | 44 | No way to manage the filters applied at sync time | workflow | ? | open, unspecified |
 | 45 | Two Sync buttons on the main window | discoverability | XS | open |
+| 46 | `uiStateSurvivesARestart` fails under the offscreen platform | testing | XS | open |
 
 Sizes are rough: XS under an hour, S a sitting, M a session.
 
@@ -1753,6 +1756,167 @@ rather than modelled.
 `MainWindow` and must keep working. The fixture is per-test, not a suite-wide
 `initTestCase`, or every case pays for a `notmuch new`.
 
+## 37. The worker stalls on a tag edit made during a background sync
+
+**Observed:** the user's note asks whether edits made while a background sync is
+running are carried by that same job or need a manual sync afterwards. The
+answer splits in two, and the second half is a defect rather than a question.
+
+**This entry was rewritten on 2026-08-04 after its original cause was
+disproved by measurement.** It first claimed the read-write open *fails* during
+a sync and the edit is discarded. It does not fail. That claim was written from
+the plausible reading of the error path at `src/notmuchworker.cpp:298-305`
+without ever provoking the condition, and a fix was built on it before anyone
+checked. Recorded here rather than quietly corrected, because the same
+false-cause-from-a-plausible-error-path mistake is cheap to repeat.
+
+**Cause, part one: reaching the disk is not the problem.** `applyTags` calls
+`notmuch_message_tags_to_maildir_flags()` (`src/notmuchworker.cpp:327`)
+immediately after thawing, so a `seen`/`flagged` change renames the file in the
+Maildir at edit time. No manual sync is needed for the change to exist on disk.
+Whether the *running* mbsync carries it is a matter of ordering: mbsync scans
+each mailbox once per run, so an edit landing after that box was scanned goes
+out on the next run. That is expected behaviour, not a bug, and the ten-minute
+cron interval bounds the delay. This half of the note is a question, answered.
+
+**Cause, part two: the write blocks, it does not fail.** Measured 2026-08-04
+against Slackware's notmuch, with the lock held deliberately rather than by
+racing cron:
+
+- `notmuch_database_open_with_config(NOTMUCH_DATABASE_MODE_READ_WRITE, …)`,
+  the exact call `applyTags` makes, **blocks until the lock is free and then
+  returns `NOTMUCH_STATUS_SUCCESS`**. A C probe against a lock held for 12s
+  returned after 9.158s with status 0; the same call with no lock held returns
+  in 0.001s. It was never observed to return an error or to time out.
+- The `notmuch` CLI behaves identically (waits 3.6s and 13.2s against 5s and
+  15s holds, always exit 0), so this is libnotmuch's behaviour and not a
+  wrapper's retry loop.
+- Therefore the error branch at `src/notmuchworker.cpp:298-305` is **not
+  reachable through lock contention at all**. It fires only for a genuinely
+  broken open: bad permissions, a corrupt index, a missing database.
+
+**The real defect is a stall.** `applyTagsToThreads` is invoked on the worker
+thread through a queued connection (`src/mainwindow.cpp:1788`), so a blocking
+open freezes *the worker*, not the UI. The window keeps painting and the rows
+show the optimistic update, but every later query, thread load and tag write
+sits behind that open in the worker's event queue until the lock frees. Nothing
+is lost and no error appears; the application simply stops responding to
+selections for the duration.
+
+**How bad in practice.** Bounded by how long `notmuch new` holds the lock, which
+on this user's already-indexed Maildir is a fraction of a second at roughly
+T+32s into a ~35s run (measured from `~/.local/state/mailsync.log`: runs start
+at :00 and reach "Processed N total files" 32-40s later). The stall is
+therefore usually invisible, and becomes user-visible only when `notmuch new`
+has real work: a first index, a large delivery, a `notmuch reindex`. That is
+also why it cannot be reproduced by clicking during a normal sync, and why the
+reproduction below holds the lock on purpose.
+
+**Reproducing it.** Racing cron does not work. Hold the lock deliberately:
+`notmuch tag --batch` keeps the write lock for a whole session and releases it
+when stdin closes, so feeding it a slow stream of no-op tag commands holds the
+lock for a controllable time. Verified: a competing writer blocks for exactly
+the remaining hold.
+
+**Approach.** Do not send a write the worker will block on. `SyncMonitor`
+(item 27) already reports whether a sync holds the lock, so `MainWindow` can
+hold the edit while `State::Running` and send it on the transition to `Idle`,
+which is a signal that already exists and already fires. The rows keep showing
+the change meanwhile, which is honest: it is what the user asked for and it is
+going to be applied.
+
+**Explicitly rejected: retrying on error.** That was the first implementation
+and it is dead code against this cause, since the error it keys off never
+arrives from lock contention. Keying on the monitor's state is also strictly
+better: it avoids the stall rather than recovering from it.
+
+**Constraints.** The optimistic-update-then-revert contract must survive: a
+held edit is still unsynced and must keep counting toward the pending indicator,
+or the quit prompt will let the user leave on work that never landed (the
+failure item 28 and the 0.9.0 net-state fix were both about). Do not widen the
+write window by holding the read-write handle open, per the read-only-by-default
+rule in `CLAUDE.md`. A held edit must re-resolve its thread ids when it is
+finally sent: `notmuch new` may have renamed files underneath it. And
+`SyncMonitor::State::Unknown` must not gate writes, or a platform that cannot
+read `/proc/locks` would never send an edit at all.
+
+**What the stall looks like, observed 2026-08-04.** Confirmed by hand with the
+Xapian lock held deliberately: switching between threads left the message pane
+showing the FIRST thread selected, and when the lock released the pane stepped
+through the three or four threads selected in the meantime, in sequence. That
+is the worker's queue draining, and it is the user-visible shape of this defect.
+
+**Reads are NOT blocked by the write lock.** Measured the same day, and it
+bounds how bad this is. A read-only open and a 200-thread query take 0.001s and
+0.015s whether or not another process holds the write lock, identical to
+baseline. So `loadThread` never blocks on the lock itself. The stall is purely
+head-of-line blocking on the single worker thread: one blocked `applyTags` holds
+up every read queued behind it. Do not "fix" this by making reads lock-aware;
+there is nothing there to fix.
+
+**Residual gap: the 2s polling window. Accepted for now (user, 2026-08-06),
+and deliberately left open rather than closed.** `SyncMonitor` polls every two
+seconds, so a sync that starts between polls is invisible to the window for up
+to 2s, and a tag edit in that window is still sent straight into a blocking
+open. The window is 2s wide, `notmuch new` holds the lock for well under a
+second on an already-indexed Maildir, and reads are unaffected either way, so
+the exposure is small and the shipped behaviour is the pre-existing one.
+
+**The option stays on the table, to revisit:** check the lock at send time.
+`SyncMonitor::lockHeldIn()` is already a static, pure function over
+`/proc/locks` content, so `sendThreadTagChange` can call it for the cost of one
+small file read per tag action. That closes the window entirely. It was not
+taken now because it would add untested code to a change that had just been
+verified by hand, which is the wrong order.
+
+### Hand test (2026-08-06): passed
+
+Verified against a real blocking open, which the unit tests cannot reach: they
+drive the deferral through the meta-object and never take a lock. Both locks
+held for 100s by the throwaway scaffold, with a tag edit made during the hold.
+All six expected behaviours confirmed by the user: the row kept the tag, the
+status message did not expire, the unsynced indicator rose, the Sync button was
+disabled, **the window stayed responsive**, and the held edit sent itself on
+release without a click.
+
+The responsiveness check is the one that mattered. The stall observed on
+2026-08-04 left the message pane frozen on the first thread selected and
+replayed the queue on release; that no longer happens.
+
+**Related:** item 35 (refresh after sync) touches the same `Idle` transition,
+and both want a non-destructive path that does not clear the undo stack.
+
+## 38. `test_mainwindow` fails when a real sync holds the lock
+
+**Observed 2026-08-04:** `theSyncButtonIsDisabledWhileABackgroundSyncHoldsTheLock`
+failed once during a full run and passed on every rerun. The cause is not
+ordering or pollution between tests: the user's cron sync happened to be running
+at that moment.
+
+**Cause (verified in code).** `MainWindow::buildUi()` constructs a real
+`SyncMonitor` on `SyncMonitor::defaultLockPath()` and the live `/proc/locks`
+(`src/mainwindow.cpp:467`) and starts it. Every `MainWindow` a test builds
+therefore observes the machine's actual sync state. The test asserts
+`button->isEnabled()` on a freshly built window, which is false whenever a real
+sync holds `/tmp/mbsync.lock`. With cron firing every ten minutes and a run
+lasting ~35s, roughly 6% of test runs land inside one.
+
+**Not caused by the item 37 work**, though that is when it was noticed. The
+test and the monitor both predate it; confirmed by stashing the item 37 changes
+and seeing the suite pass, then reproducing the failure with a sync live.
+
+**Approach.** The monitor is already injectable: its constructor takes a
+`locksPath` precisely so tests can drive transitions without real locks
+(`src/syncmonitor.h:59-64`), and `test_syncmonitor` uses that. `MainWindow` does
+not expose it. Either let the window take a locks path (config or a setter used
+only by tests), or have the test point `SyncMonitor` at a temporary file. The
+existing tests that drive `onExternalSyncStateChanged` through the meta-object
+are unaffected either way; it is only the construction-time state that leaks in.
+
+**Constraint:** do not simply stop starting the monitor in tests. The
+construction-time state IS the behaviour under test for this case, and a window
+that never polls would pass the assertion for the wrong reason.
+
 ## 39. Thread list cannot be sorted by clicking a column header
 
 **Observed (user, 2026-08-05):** "left pane columns order by clicking on the
@@ -2004,6 +2168,45 @@ last widget that predates it.
   Confirm with the user which of the two survives; the note says redundant, not
   which one is wanted.
 
+## 46. `uiStateSurvivesARestart` fails under the offscreen platform
+
+**Observed 2026-08-06:** the full suite is green on the user's Wayland session
+but `TestMainWindow::uiStateSurvivesARestart` fails under
+`QT_QPA_PLATFORM=offscreen`, which is how the suite is run when a session must
+not open windows on the user's screen. Only the width is wrong:
+
+```
+Actual   (reopened.size()): QSize(798x620)
+Expected (resized)        : QSize(940x620)
+```
+
+**Cause (verified by probe, not assumed).** The offscreen platform reports an
+800x800 screen. `QMainWindow::restoreGeometry()` clamps a restored window to the
+available screen area, so the test's 940 width comes back as 798 while its 620
+height, which fits, restores untouched. That asymmetry is the tell: the state
+file is written and read correctly, and the zoom factor in the same test
+restores fine. Nothing is broken in item 1's persistence.
+
+**Not a state-file collision.** The test already scopes itself properly with
+`QStandardPaths::setTestModeEnabled(true)` and removes the file at both ends
+(`tests/test_mainwindow.cpp:229-255`), so it never touches
+`~/.local/state/qtmaildir/uistate.conf`. An earlier reading of this failure
+blamed the real state file being held by a running app; that was wrong, and the
+test source disproves it.
+
+**Approach.** Pick a size the smallest plausible test screen can hold, well
+under 800x800, and assert on that. The test is about persistence, not about
+large windows, so the specific number carries no meaning and only needs to
+differ from the default.
+
+**Constraint:** do not "fix" this by widening the assertion to a tolerance or by
+skipping under offscreen. Both would hide a genuine restore failure later, and
+the property under test (the size that went in comes back out) is exact.
+
+**Related:** the same class as item 38. Both are tests that silently depend on
+the machine they run on, and both were found by running the suite in a context
+its author had not tried rather than by reading it.
+
 ## Deferred, unsized, or split out
 
 Items noted while triaging but not part of the original list. Same numbering