diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-15 16:12:07 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-15 16:12:07 +0200 |
| commit | 8796ed59d2410f3e47a114b6002fdefb8f259304 (patch) | |
| tree | 6f23ecdd11f619a5a0576f82a94353065d1e2d28 /docs | |
| parent | 1062eedaff00a46918f379bd70d210ce95861c93 (diff) | |
| download | qtmaildir-8796ed59d2410f3e47a114b6002fdefb8f259304.tar.gz qtmaildir-8796ed59d2410f3e47a114b6002fdefb8f259304.zip | |
fix(sync): send held edits before the sync-end refresh reads the database
An edit made while a sync is running is held rather than sent, because the
worker's read-write open blocks on notmuch's exclusive lock. At sync end
onExternalSyncStateChanged() refreshed the list first and flushed the held
edits afterwards, so the refresh read a database that still carried the old
tag, reconciled it into the model, and overwrote the optimistic update the hold
had deliberately left applied. The flush then wrote the tag correctly.
The database ended up right and the list ended up wrong, with nothing scheduled
to re-read it, which is why it looked like the edit had been lost. Reported by
hand: a message read during a sync went back to unread when the sync finished.
The flush moves ahead of the refresh and keeps both properties it already had.
It stays outside the Idle branch, so edits held when /proc/locks becomes
unreadable are not stranded waiting for an Idle that never comes, and it stays
after the status-bar retire, so its own "N held changes sent" message survives.
Both orders leave identical end state, so the first version of the test passed
against the defect: after the handler returns the queue is empty and the write
has been sent whichever ran first. flushGenerationForTesting() stamps the query
generation at flush time, which is what separates them, and the test fails
against the old order with Actual: 3, Expected: 2.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'docs')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 43 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 1 |
2 files changed, 44 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 393279e..c24ba23 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 @@ -5418,3 +5418,46 @@ designs were drafted before asking (keep non-matching rows until an explicit query, defer the refresh while the pointer is over the list, append-only refresh) and none is worth building against a complaint that resolved itself. Reopen only if the churn is reported somewhere it is NOT the view doing its job. + +## 97. An edit made during a sync is reverted in the list when the sync ends + +**Observed (user, 2026-08-15, hand-testing item 89's fix):** "I've waited for +the sync to start, then read an email, noticed the 'unread' tag disappear and +the status bar correctly notifies 'A sync is running, your change will be +applied...'. At the end of the sync process, the read email returned unread." + +**Cause (verified in code).** `onExternalSyncStateChanged()` did two things at +sync end, in this order: `refreshCurrentQuery()` inside the Idle branch, and +`flushHeldEdits()` after it. An edit made during a sync is HELD rather than +sent, because the worker's read-write open blocks on notmuch's exclusive lock +(measured at 9.158s against a 12s hold, returning SUCCESS). So the refresh read +a database that still carried `unread`, reconciled that into the model, and +overwrote the optimistic update the hold had deliberately left applied. The +flush then wrote the tag correctly. + +The database therefore ended up RIGHT and the list ended up WRONG, with nothing +scheduled to re-read it. That is why it read as "the edit was lost" when the +edit had in fact been applied. + +**Fix.** Flush before the refresh. It stays outside the Idle branch, for the +reason it always was: Unknown clears the busy flag, so writes resume from there +and edits already held must not wait for an Idle a broken `/proc/locks` will +never report. It also stays after the status-bar retire, so the flush's own "N +held changes sent" message is not immediately overwritten. Flushing first costs +nothing when nothing is held, since the function returns on an empty queue. + +**Both orders leave IDENTICAL end state, and the first version of the test +passed against the defect.** After the handler returns, the queue is empty and +the write has been sent whichever ran first, so every assertion made afterwards +is blind to the bug. What separates them is the generation at the moment of the +flush: flushing first stamps the generation from before the refresh bumped it. +`flushGenerationForTesting()` exists for that and nothing else; against the old +order the test fails with `Actual: 3, Expected: 2`. + +**Found only by hand.** The suite was green across the reorder, item 89's own +fix was green, and the reversion needed a real cron sync, a real held edit and +someone watching the row. Recorded because it is the second time in one session +that a green suite endorsed a defect a screenshot found. + +**Confirmed by the user:** "one change sent now that the sync has finished. And +the tag I added, sticked to the message." 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 14a242a..4a68f0b 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 @@ -161,6 +161,7 @@ taking that too literally. | 95 | A query in the overflow menu cannot be run | defect | XS | **done** 2026-08-15, unreleased. Pre-existing and not caused by 93: the entry's action owned a submenu, and Qt emits no `triggered` for those, so the connection had never fired. Surfaced because 93 moved every query into the menu | | 94 | `pinned` has nothing left to decide once the buttons are built-in | maintenance | S | open; **blocked on 93**, and deliberately not part of it. A user-visible removal: the row becomes built-ins only and every saved query lives in the menu | | 96 | A query returning the thread already on display opens onto the placeholder | defect | S | **done** 2026-08-15, unreleased. Split from 66's unverified half, which had a different cause. Reproduced from two screenshots after four measured eliminations | +| 97 | An edit made during a sync is reverted in the list when the sync ends | defect | S | **done** 2026-08-15, unreleased. Found by hand-testing item 89's fix. The sync-end refresh ran BEFORE the held-edit flush, so it read a database that still carried the old tag | Sizes are rough: XS under an hour, S a sitting, M a session. |
