diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-29 10:11:59 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-29 10:11:59 +0200 |
| commit | e2bd68a863513209a5881696164fd463ddc83503 (patch) | |
| tree | fe9fc90462030f9f99d5298eb09785018cc60f40 /docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | |
| parent | feb15bcec261501eb49ad0de1fdc05a9992227c2 (diff) | |
| download | qtmaildir-e2bd68a863513209a5881696164fd463ddc83503.tar.gz qtmaildir-e2bd68a863513209a5881696164fd463ddc83503.zip | |
fix: say an edit is waiting for the sync instead of claiming it landed
Item 182, found by hand: a thread of 9 messages with 5 unread, marked
read while a sync was running, reported "<subject>: mark as read" and
then reported the same work again when the sync finished. The user read
it as double reporting.
Not a double write, and the mail was correct. It is one action reported
twice because the FIRST report was the wrong one.
A sync holds notmuch's exclusive write lock and the worker's read-write
open blocks on it rather than failing, so an edit made during a sync is
held and sent when the lock frees. All three hold branches say exactly
that, in a label chosen deliberately: NOT transient, because it
describes state lasting until the sync ends, and a message that expired
would leave rows showing a tag the database has not got and no
explanation of why.
That label never survived. Every caller announced the action itself a
line later through showTransientStatus(), which overwrote it, so the
user was told the write had happened and the hold was never mentioned.
The flush at the end of the sync then reported the same work again and
read as a duplicate rather than as its completion.
announceAction() asks whether a sync holds the lock and, when one does,
sets a non-transient label naming the action AND the wait. The action is
still named because that announcement is what stands in for the
confirmation dialog this project rules out: it is how a user tells that
something larger than they meant has just happened, so the hold is added
to it rather than replacing it. The flush message is untouched and is
the only signal that held work actually landed, whose absence was item
106.
The test drives toggle_unread, the route the user took, and asserts both
halves: the text mentions the sync, and it still says what is waiting.
Asserting only the first would pass against an announcement that dropped
the action entirely. Mutation-checked by forcing the non-held branch,
which fails with the exact text the user reported.
The new string is translated, since one that misses the Italian ships as
English inside an otherwise Italian UI: lupdate found it with no context
warnings, lrelease reports 552 finished and 0 unfinished.
Suite: 42 of 43, with undoMovesTheMessageBack failing as it does on
master (item 136).
Diffstat (limited to 'docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 85 |
1 files changed, 85 insertions, 0 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md index 27b4e28..b3cda60 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md @@ -9374,3 +9374,88 @@ assertion after it means something, and mutation-checked by removing the call. Note the failing runs take 45s and the passing one 295ms: the `QTRY` timeouts expire when the refresh never comes, which reads as a slow test rather than a missing feature. + +## 182. An edit made during a sync is announced twice and never says it is waiting + +**Done 2026-08-29**, unreleased, on `thread-row-identity`. + +**Observed (user, by hand, 2026-08-29):** a thread of 9 messages with 5 unread, +marked read while a sync was running. The status bar showed "<subject>: mark as +read"; at the end of the sync it showed the 5 messages reported again. "Double +reporting before/after the sync." + +**Cause, verified in the code.** Not a double write, and the mail is correct. +It is one action reported twice because the FIRST report is the wrong one. + +A sync holds notmuch's exclusive write lock, and the worker's read-write open +blocks on it rather than failing, so an edit made during a sync is HELD and +sent when the lock frees. All three hold branches say so, in a label chosen +deliberately: + +- `sendThreadTagChange()` (`mainwindow.cpp:7122`) +- `sendMessageTagChange()` (`:5937`) +- `sendMove()` (`:6722`) + +Each sets `m_statusLabel` directly to "A sync is running; your change will be +applied when it finishes.", with a comment stating that this is NOT transient +because it describes state lasting until the sync ends, and that a message +which expired would leave the user with rows showing a tag the database has not +got and no explanation of why. + +That label never survives. Every caller announces the action itself immediately +afterwards, through `showTransientStatus()`, which overwrites it: + +- `markThreadsRead()` (`:3641`), the path in the report +- the tag-action funnel (`:5846`) +- `deleteSelection()` (`:6104`) + +So the user is told "mark as read", which claims the write happened, and the +hold is never mentioned. When `flushHeldEdits()` later reports "N held changes +sent now that the sync has finished" (`:4580`), that reads as a second report +of the same action rather than as its completion. + +The two counts differ (threads against messages) because the two messages count +different things, which is why the report describes 5 messages where the first +message named the thread. + +**Approach.** The caller must not overwrite a hold that the funnel has just +explained. `aSyncHoldsTheWriteLock()` is already a const predicate, so a caller +can ask it directly and skip its own announcement; no funnel signature changes. + +The flush message stays. It is the only signal that held work actually landed, +and its absence was item 106, silent data loss on this exact path. + +**Constraints.** + +- The hold label must stay NON-transient, for the reason its own comment gives. + A fix that makes it transient reintroduces the unexplained state it exists to + prevent. +- Three call sites, and a fourth path (`sendMove`) whose callers must be checked + rather than assumed: the same defect is one caller away in each. +- The announcement is what stands in for the confirmation dialog this project + rules out, per the comment at `:5842`: it tells the user when something larger + than they meant has just happened. Suppressing it entirely during a sync would + cost that, so what replaces it has to name the action AND the hold, not drop + the action. +- The two messages counting different things (threads against messages) is a + separate readability question and is NOT part of this item. + +**Built as `MainWindow::announceAction()`**, which the three sites call in +place of `showTransientStatus()`. It asks `aSyncHoldsTheWriteLock()` and, when +one does, sets a non-transient label reading "<action>, waiting for the running +sync to finish". The action is still named for the reason the constraint above +gives: that announcement is what stands in for the confirmation dialog this +project rules out, so the hold is ADDED to it rather than replacing it. The +flush message at the end of the sync is untouched, and now reads as the +completion of the first message rather than a repeat of it. + +The test drives `toggle_unread`, the route the user took, after putting the +window into the state the sync monitor puts it in. It asserts BOTH halves: the +text mentions the sync, and it still says what is waiting. Asserting only the +first would pass against an announcement that dropped the action entirely, +which is the thing the constraint forbids. Mutation-checked by forcing the +non-held branch, which fails with the exact text the user reported. + +The string is translated, since a user-facing string that misses the Italian +translation ships as English inside an otherwise Italian UI: `lupdate` found it +with no context warnings and `lrelease` reports 552 finished, 0 unfinished. |
