diff options
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 | 79 |
1 files changed, 79 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 2e941ec..f537385 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 @@ -7751,3 +7751,82 @@ new action and the existing ones gathered under it rather than duplicated. - **Item 160 unblocked this** on 2026-08-25: the status bar exists, so a manual save reports through `refreshDraftStatus()` like an autosave. Route `Ctrl+S` through `saveDraftNow()` and the reporting is already done. + +## 162. Delete fails while a sync is renaming the file underneath it + +**Observed (user, 2026-08-25):** deleting a draft reported `Cannot move +<file> to <account>/Trash`. + +**Cause (verified against the live Maildir, not read):** a stale path, and +neither Delete nor item 158 is at fault. + +1. The composer autosaves a draft as `<name>:2,D` and item 158 indexes it + under exactly that filename. +2. **mbsync uploads it and RENAMES it** to `<name>,U=<uid>:2,D`, recording the + server UID in the filename. +3. notmuch still holds the pre-`U=` name until that sync's `notmuch new` runs. +4. `moveMessages()` reads the filename from notmuch and calls + `QFile::rename()` on a path that no longer exists. It fails, the error is + emitted, and the message is skipped. + +Measured, in this order: `notmuch search --output=files` named a file that was +not on disk while `Background sync running...` was up, and the same query was +clean once the sync finished, with the file present under its new `,U=4` name. +That is why it reads as intermittent, and why it heals itself. + +**It is truthful and it loses nothing.** The move is skipped, no wrong folder +is created, no file is destroyed, and the next sync reconciles. The defect is +that the message blames a folder for a timing problem, and that the action +silently does nothing when the user asked for something. + +**This is `CLAUDE.md`'s `,U=` trap from the other side.** `MaildirName::fresh()` +exists because CARRYING that infix across a folder boundary produced +`Maildir error: duplicate UID` on real mail. Here mbsync is ADDING it and the +index lags; the same infix, the opposite direction. + +**Approach, and it needs a decision.** Two candidates: + +- **Refuse the move while a sync holds the lock.** `SyncMonitor` already + reports this, and the held-edit machinery from items 97 and 106 already + exists for exactly this shape: a tag edit made during a sync is held and + flushed when it ends. Delete would join it rather than inventing anything. + This is the likelier right answer, since it matches what every other + mutation already does. +- **Re-resolve the filename** from notmuch immediately before the rename and + re-query the message if the path is gone. Smaller, but it races the same + window it is trying to close, and a second lookup can be stale by the time + it is used. + +**Constraints.** + +- **Delete reaches the real mail server.** Read `CLAUDE.md`'s item 103 notes + before touching `moveMessages()`: a wrong folder name is created, adopted by + mbsync, and propagated to every other client. +- Whatever is built, **the message must say a sync is running**, not name a + folder. The current wording sent the user looking for a broken folder + configuration, which was correct and configured. +- A test cannot see this in the ordinary fixture layout, where nothing renames + a file underneath the index. Driving it means renaming the file between the + index write and the move, which is what the reproducer has to do. + +**Fixed 2026-08-25.** `moveMessages()` re-resolves by MESSAGE ID when the +recorded path is gone: one `reindexFolder()` of that directory, then the +filename from `notmuch_message_get_filenames()` that exists on disk. Bounded +to a single retry, so a genuinely missing file still reports rather than +becoming a silent no-op. + +**The hold candidate above was investigated and rejected**, and the reason +matters more than the fix. `sendMove()` ALREADY refuses while a sync holds the +lock and queues onto `m_heldMoves` (items 97 and 106 built it), yet the defect +still fired. `aSyncHoldsTheWriteLock()` tracks notmuch's write lock, while this +window sits between mbsync's RENAME and that sync's `notmuch new`; mbsync +renames throughout its run without touching that lock, so the damaging window +is open when there is nothing to observe. Refusing on the lock guards the wrong +resource. That refusal is correct for its own purpose and was left alone. + +Covered by `moveMessagesRecoversWhenASyncRenamedTheFile()` and +`moveMessagesStillReportsAMessageThatIsReallyGone()`, the second pinning the +bounded half. The test renames without reindexing, which is the window mbsync +opens, and asserts the database still names the old path so it cannot pass +against a fixture that quietly reindexed. Mutation-checked. + |