aboutsummaryrefslogtreecommitdiffstats
path: root/docs/superpowers
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-29 11:30:08 +0200
committerDanilo M. <danix@danix.xyz>2026-08-29 11:30:08 +0200
commit063be87405277aef3122c448b064241fd15f2a92 (patch)
tree1538f216ce1e31e7be334952be7c5ad8d48c35a8 /docs/superpowers
parent3fd999907ae8344f76ee4e5be1ac278a26f452ca (diff)
downloadqtmaildir-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')
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md100
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md61
2 files changed, 101 insertions, 60 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.
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 2cc3b5c..aa56727 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
@@ -193,7 +193,7 @@ taking that too literally.
| 124 | The worker reads the index directory as the mail root | defect | S | **done** 2026-08-20, unreleased. `mailRootOf()` over `NOTMUCH_CONFIG_MAIL_ROOT`, correct under both layouts. Verified by migrating the developer's own index to NVMe the same day: cold start 38.6 s to 0.67 s |
-| 125 | A skipped sync leaves the spinner running for ever | defect | S | open, 2026-08-20, **half closed 2026-08-29 by item 174**: a skipped run now writes `state: skipped` to the status file, so the application can see it happened and clears the spinner without clearing the count. What remains is whether `SyncMonitor` should also time out an observation it never saw end, which covers a run that dies without writing anything at all |
+| 125 | A skipped sync leaves the spinner running for ever | defect | S | **done 2026-08-29**, unreleased, in two halves and mostly already built. The exit-75 branch in `onSyncFinished()` predates this session: spinner cleared, no error, no log pane, exit prompt handled, with a test. Item 174 added the external half, a `skipped` state the application can see. What was genuinely missing was the RE-ARM: `runAutoSync()` re-arms when it declines to start (item 89), but a run that LAUNCHES and exits 75 lands in `onSyncFinished()`, which armed nothing, so the edit waited for a manual sync or the next cron tick. Section in the closed file |
| 126 | A link with `target="_blank"` does nothing when clicked | defect | S | **done** 2026-08-20, unreleased. `createWindow()` returns a relay page that receives the navigation, hands the URL to the browser and refuses. The URL cannot be read in `createWindow()` itself, which is why a relay rather than a lookup |
| 127 | A link's context menu offers four browser actions that cannot work | defect | XS | **done** 2026-08-20, unreleased. Three Open-in actions removed, `CopyLinkToClipboard` kept. Item 126 made them more dangerous rather than less: with a real `createWindow()` they would have started working |
@@ -830,65 +830,6 @@ touch `KeyMap`), and one new build dependency,
`pkg_check_modules` line here and **no** `REQUIRES` entry in the SlackBuild,
which lists only non-stock dependencies.
-## 125. A skipped sync leaves the spinner running for ever
-
-**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.**
-
## 135. The formatting toolbar's buttons stack rather than toggle
**Observed (user, 2026-08-21):** pressing Bold a second time on already-bold