diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-29 11:30:08 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-29 11:30:08 +0200 |
| commit | 063be87405277aef3122c448b064241fd15f2a92 (patch) | |
| tree | 1538f216ce1e31e7be334952be7c5ad8d48c35a8 /docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | |
| parent | 3fd999907ae8344f76ee4e5be1ac278a26f452ca (diff) | |
| download | qtmaildir-063be87405277aef3122c448b064241fd15f2a92.tar.gz qtmaildir-063be87405277aef3122c448b064241fd15f2a92.zip | |
fix: retry a sync that was skipped because another held the lock
Item 125, the half that was genuinely missing.
Most of this item was already built and the row was stale. The exit-75
branch in onSyncFinished() predates this session and does what the entry
asks: the spinner clears, the skip is reported as neither success nor
failure, m_lastSyncFailed stays put, the log pane is not raised, the lock
latch is handed back to the external monitor, and the sync-on-exit case
has its own dialog. Item 174 then added the external half, a `skipped`
state a run the application did not start can be seen to have produced.
What nothing covered was the RE-ARM, and it is the symptom the item was
filed for. runAutoSync() re-arms when it declines to START, which is item
89 and covers a sync skipped before launching. A run that LAUNCHES, finds
the lock held and exits 75 reaches onSyncFinished() instead, and that
branch armed nothing: the edit stayed pending with nothing scheduled to
carry it, waiting for a manual sync or the next cron tick. That is "a
held edit waits for a completion that never comes".
scheduleAutoSync() in the skip branch. It re-checks the delay, the sync
command and the pending count on the way in, so it cannot arm a sync for
nothing, and against a long external run it re-arms once per debounce
interval until the lock clears.
This is the half the status file could not reach, and the distinction is
worth keeping: that file says what a run DID, this is what the
application does NEXT.
The test records a real pending edit first, since runAutoSync() correctly
declines when there is nothing to carry and a fixture without one would
arm nothing for a legitimate reason. It failed before the fix and is
mutation-checked.
Suite: 43 of 44, 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 | 100 |
1 files changed, 100 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 d788573..0972168 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 @@ -9570,3 +9570,103 @@ still worked, so the symptom would have been "the channel narrowing never happens" with nothing failing. One second of slack matches the precision the format actually carries, and cannot readmit a genuinely stale file when cron runs ten minutes apart. + +## 125. A skipped sync leaves the spinner running for ever + +**Done 2026-08-29**, unreleased. + +**Observed (user, 2026-08-20):** during the item 124 index migration, a Delete +appeared to do nothing: the view did not refresh, the message did not move, and +restarting the application showed it exactly where it had been. The user then +reported "I see a spinner in the bottom right, is going indefinitely. maybe +that's what stopped the move?" That observation is what identified the cause. + +**Cause (verified from `/proc` and the sync log).** The migration procedure held +`/tmp/mbsync.lock` to keep cron out of the way. qtmaildir auto-syncs a couple of +seconds after a tag action (item 71), so Delete queued its edit and started a +sync; `mailsync.sh` found the lock held and exited **75**, by design: + +``` +2026-08-20T10:00:01+02:00 === SKIPPED: previous run still in progress === +2026-08-20T10:10:01+02:00 === SKIPPED: previous run still in progress === +``` + +75 is `EX_TEMPFAIL`, chosen deliberately so a click landing during a cron run is +not reported as a failure (`assets/mailsync.sh`, the comment at the `flock -n` +guard). The application appears to treat it as neither success nor failure: the +sync indicator never cleared, and because an edit made during a sync is HELD +until the sync ends, the delete sat in that queue waiting for a completion that +could never arrive. `notmuch count tag:deleted` on the account confirmed the +write had not reached the database. + +**Nothing was lost**, and that is worth recording separately: held edits are +written to disk (item 106), so the delete survived and applied as soon as a real +sync ran. The defect is that the user cannot tell. + +**Approach.** Handle exit 75 explicitly wherever `MailSync` reports a finish. +It is a third outcome, not a variant of the other two: the work did not happen, +nothing is wrong, and it should be retried rather than reported. Clearing the +indicator is the minimum; re-arming the auto-sync timer is probably right too, +and item 89 already made a skipped auto-sync re-arm rather than give up, so +there is a precedent to follow rather than a policy to invent. + +**Constraints.** + +- **Do not turn 75 into an error.** The exit code exists precisely so an + overlapping click is not reported as a failed sync, and item 89 settled that a + skip is routine. Showing the log pane here would be a regression. +- **The held-edit queue must still flush.** Whatever clears the indicator has to + leave the queue in a state where the next successful sync sends it, which is + what happened by luck here rather than by design. +- **A stuck indicator is worse than a wrong one**, because it also blocks the + quit prompt's "unsynced changes" story (items 18, 19, 28, 54). This is the + same class of indicator dishonesty those four items each fixed once. + +**Reproducing it.** Hold the lock in one terminal and act in the application: + +```bash +flock /tmp/mbsync.lock -c 'sleep 300' +``` + +Then Delete a message. Verified by hand on 2026-08-20; this is how it was found. + +**Size: S.** + +### What was actually missing, which was not what this entry assumed + +**Most of it was already built.** The `exitCode == kSyncSkippedExitCode` branch +in `onSyncFinished()` predates this session and does what the Approach above +asks: it clears the spinner through `setSyncBusy(false)` at the top of the +handler, reports the skip as neither success nor failure, does not set +`m_lastSyncFailed`, does not raise the log pane, hands the lock latch back to +the external monitor, and covers the sync-on-exit case with its own dialog. +`aSkippedLocalSyncStillReportsTheOtherRunFinishing()` covers it. The row was +stale rather than open, which is worth recording: reading the row would have led +to rebuilding what exists. + +**Item 174 closed the external half** on the same day: a skipped run now writes +`state: skipped` to the status file, so a run the application did not start is +visible as having happened and carried nothing. + +**The genuine gap was the RE-ARM, and it is the reported symptom.** +`runAutoSync()` re-arms when it declines to START, which is item 89 and covers a +sync skipped before launching. A run that LAUNCHES, finds the lock held and +exits 75 reaches `onSyncFinished()` instead, and that branch armed nothing at +all. The edit stayed pending with nothing scheduled to carry it, waiting for a +manual sync or the next cron tick, which is exactly "a held edit waits for a +completion that never comes". + +One line, `scheduleAutoSync()` in the skip branch. It re-checks the delay, the +command and the pending count on the way in, so it cannot arm a sync for +nothing, and against a long external run it re-arms once per debounce interval +until the lock clears. + +**This is the half the status file could not reach**, and the distinction is +the useful part: the file says what a run DID, while this is what the +application does NEXT. Two different questions, which is why one fix could not +serve both. + +The test drives `onSyncFinished()` with the skip code after recording a real +pending edit, since `runAutoSync()` correctly declines when there is nothing to +carry and a fixture without one would arm nothing for a legitimate reason. +Mutation-checked by removing the call. |
