diff options
Diffstat (limited to 'docs/superpowers')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 158 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 106 |
2 files changed, 160 insertions, 104 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 af1de0c..8de26f2 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 @@ -9941,3 +9941,161 @@ sorts. **Still open and separate:** the four autosave revisions of that reply sitting in `Drafts/` with distinct Message-IDs, which is item 165 and is what puts a `draft` chip on the conversation. + +--- + +## 165. A draft gets a new Message-ID on every autosave + +**Observed (developer, 2026-08-25), while hand-testing items 163 and 164.** +Four saved drafts produced four distinct Message-IDs, and one reopen-and-edit +turned one id into another. A draft therefore has no stable identity across +its own revisions. + +**Cause (verified in the code, not inferred).** `MessageBuilder::build()` +calls `g_mime_utils_generate_message_id()` unconditionally on every call +(`messagebuilder.cpp:311`), and every autosave calls `build()`. +`OutgoingMessage` has no field to carry an existing id in, and +`ComposeContext` has no field for the draft's OWN id either: it carries +`inReplyTo` and `references`, which are the ORIGINAL's id when replying, and +`ComposeContextBuilder::forDraft()` never reads the draft's Message-ID back +out of the file it parses. So this is not a changed call site; it needs a +field that does not exist yet, threaded from `forDraft()` through +`ComposeContext` and `OutgoingMessage` into `build()`. + +**Why it matters, and why it is NOT urgent.** To notmuch and to the server, +each revision is a different MESSAGE, not a new version of one. While the +file is replaced correctly this is invisible: one file in, one file out. It +becomes visible whenever a revision is NOT replaced, and item 163 is the +proof, where a stale path forked a draft into two files that were also two +messages and that nothing will ever collapse. Item 163's fix removes the +known way to reach that state; this entry is about the property that turned a +one-file mistake into a two-message one. + +An interrupted save is the remaining route: `DraftStore::write()` unlinks the +previous revision only AFTER the new file is safely in place (deliberately, +so a failed write cannot lose the draft), so a crash between the two leaves +two files, and with two ids they are two drafts rather than one duplicated. + +**Decided by the user on 2026-09-06: a stable id while drafting, DISCARDED at +send.** The second of the three positions below. A draft keeps one identity +across all its revisions, so each save replaces the previous message rather +than adding one; the sent copy is minted fresh and is a different item from +the draft, which is the user's own framing. The draft is DELETED on a +successful send, as it already is today, and the sent message carries no +`draft` tag. + +The three positions, kept because the reasoning is what makes the choice +reviewable: + +- A stable id reused at send time makes the draft and the sent message one + message, which is what a user means by "my draft became this email". It + also means the id was in a file mbsync uploaded to the drafts folder before + the message was ever sent, and the server has seen it. **Not chosen.** +- A stable id DISCARDED at send time keeps revisions collapsed while drafting + and mints a fresh id for the sent copy. Two identities, and the sent one is + the one that threads. **Chosen.** +- The current behaviour is a third position, and its only virtue is that no + id is ever reused for two different things. + +Whichever is chosen must be checked against `In-Reply-To`/`References` on the +eventual send, since `referencesForReply()` builds those from the ORIGINAL's +id and a draft of a reply carries both. + +**What is already built, measured 2026-09-06 rather than assumed.** Two of the +three things the decision describes exist: + +- `DraftStore::write()` removes the previous revision after the new file is in + place (`draftstore.cpp:78`), so "discard what is left after a successful + save" is done LOCALLY. +- `ComposeWindow` removes the draft on a successful send + (`composewindow.cpp:1620`, through `resolveRenamed()` since item 163), so + "delete the draft on send" is done, and the user confirmed that is the + wanted behaviour rather than keeping the last revision. +- The sent copy is built fresh and filed to the Sent folder, so it never + carries `draft`. The tag the user saw on a Sent row came from the orphan + revisions threading into the conversation, not from the sent message. **No + work is needed for that clause**; it was already true. + +**So the whole remaining defect is the id, and the mechanism is the server.** +Measured on the user's own mail: four drafts of one reply, four distinct +Message-IDs, and every one of the four files carrying a `,U=` infix. mbsync +had uploaded each revision to the drafts folder BEFORE the next save removed +the local file, so the server holds four independent messages and syncs all +four back down. The local cleanup is correct and cannot help: the damage +happens between a save and the next sync, and deleting a local file does not +retract an uploaded one. + +This is what raises the item above "invisible while the file is replaced +correctly". The file IS replaced correctly and four messages still exist. +A stable id gives the server one message being revised rather than four +unrelated ones. + +**Constraints.** + +- **A Message-ID reaches the server and every recipient**, so a reused id is + not a local matter. Two different messages sharing an id is worse than two + ids for one draft, which is what makes the current behaviour defensible as + a default rather than simply wrong. +- `MessageBuilder::build()` is on the SEND path as well as the autosave path. + A change that makes ids stable must not make two different sent messages + share one. +- The comment at `messagebuilder.cpp:297` records that GMime generates + neither Date nor Message-ID unless asked, and that a message without one + cannot be threaded by anything receiving it, this application's own index + of the sent copy included. Any "just omit it while drafting" variant has to + answer that. +- Item 163's fix stands on its own and this does not block it: the file is + replaced correctly now, so the fork this would have mitigated no longer + happens by that route. + +**Built 2026-09-06.** Three links, each one a place the id had to travel and +each with its own test: + +- `OutgoingMessage::messageId` is the field that did not exist. Empty means + "mint a fresh one", so the SEND path is unchanged by construction rather + than by remembering to clear it. +- `MessageBuilder::build()` uses a supplied id when there is one and generates + otherwise. `g_mime_message_set_message_id` takes the id WITHOUT angle + brackets and adds them, which is the form `generate_message_id` returns, so + no unwrapping is needed. +- `ComposeWindow::m_draftMessageId` holds the identity between revisions, + assigned from `built.messageId` after a successful save so the FIRST save + adopts the id GMime just generated. `ComposeContext::draftMessageId` carries + it across a reopen, read in `forDraft()` from `ParsedMessage::messageId`, + which the parser already provided and nothing had ever used. + +**One comment had to be corrected rather than left alone**, and it is the kind +that rots into a false claim: the autosave's dirty check explains why it +compares the MESSAGE rather than the built bytes, and its reason was "GMime is +given a fresh Date and Message-ID on every build". Half of that is no longer +true. The Date still is, so the conclusion stands and only the reasoning +needed fixing. + +**Tests, and why there are four.** The property spans three objects, and a test +at any one of them passes while another link is broken: + +- `aSuppliedMessageIdIsUsedRatherThanAFreshOne` builds TWICE, because a single + build cannot distinguish a reused id from a freshly generated one. +- `twoBuildsWithNoSuppliedIdStillDiffer` is the safety constraint: a field + defaulting to some fixed value would satisfy the test above and make two + sent messages share an id. +- `everyRevisionOfADraftKeepsOneMessageId` is the end-to-end property, over + the real files. It changes the body between saves deliberately: without a + real change the dirty check short-circuits, no second file is written, and + the test would pass while proving nothing. +- `aResumedDraftKeepsTheMessageIdItWasSavedUnder` covers the reopen, which the + composer-only tests cannot reach, and asserts on the file written AFTER the + reopen rather than only on the context, since the context carrying the id is + half a chain. +- `aSentMessageDoesNotInheritTheDraftsMessageId` pins the constraint at the + place a future edit would break it, using the already-public + `currentMessage()` rather than adding a test accessor. + +Both wiring changes were mutation-checked: removing the autosave's assignment +fails the revision test, removing `forDraft()`'s read fails the reopen test. + +**What this does NOT do, deliberately.** It does not clean up revisions already +on the server. Those are existing messages in the drafts folder, and nothing +here retracts them; the four found on the user's own mail were deleted by hand +as a separate act. + 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 0029092..7267ea8 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 @@ -238,7 +238,7 @@ taking that too literally. | 162 | Delete fails while a sync is renaming the file underneath it | defect | S | **done, 2026-08-25.** mbsync renames an uploaded file to add its `,U=<uid>` infix and notmuch keeps the pre-`U=` name until that sync's `notmuch new` runs, so `moveMessages` renamed a path that no longer existed and Delete silently did nothing while blaming the destination folder. `moveMessages` now re-resolves by MESSAGE ID when the recorded path is gone: one reindex of that directory, then the filename that exists on disk. Bounded to one retry, so a file genuinely gone still reports. Holding the move during a sync was the other candidate and is NOT the fix: `sendMove` already refuses on notmuch's write lock, but this window sits between mbsync's rename and that sync's `notmuch new`, which touches no lock | | 163 | The message pane shows a stale path, and the composer forks the draft | defect | S | **done, 2026-08-25.** mbsync renames an uploaded file to add its `,U=<uid>` infix while the model still holds the name the query returned. `MaildirName::resolveRenamed()` returns the path unchanged when it exists, else finds the file in that one directory whose unique stem matches; it refuses an ambiguous match and yields nothing for a genuinely missing file. Wired into all THREE read sites: the pane, Reply/Forward, and the draft reopen. The reopen was the one that cost data, forking a draft into two files with two Message-IDs, both reaching the server | | 164 | A draft this application saved keeps `inbox` | defect | S | **dropped** 2026-08-27, NOT A DEFECT. The premise was a measurement artifact: its evidence was `notmuch search --output=tags`, which DISPLAYS the union over a thread, and a reply-draft under an arrived message reads `draft inbox unread` while no message carries both. Re-measured at message level: 0 of 12 drafts carry `inbox`, including nine written on or before 2026-08-25. The `unread` half was real and is item 172 | -| 165 | A draft gets a new Message-ID on every autosave | defect | S | open, 2026-08-25, **decided 2026-09-06**: a stable id while drafting, discarded at send, so the sent copy is a different item from the draft. Reclassified from enhancement after the user hit it: four revisions of one reply reached the server as four messages, because mbsync uploads each before the next save removes it locally, and they then thread into the conversation and put a `draft` tag on a Sent row. The local cleanup and the delete-on-send are already built and correct; the id is the whole remaining defect. Needs a field threaded from `forDraft()` through `ComposeContext` and `OutgoingMessage` into `build()` | +| 165 | A draft gets a new Message-ID on every autosave | defect | S | **done 2026-09-06**, unreleased. A draft keeps one identity across its revisions and across a reopen, so a save replaces the message rather than adding one; the sent copy still mints its own, at the user's decision. `OutgoingMessage::messageId` empty means "generate", so the send path is unchanged by construction. The four revisions already on the server were deleted by hand, not by code. Section in the closed file | | 166 | Mail you send to your own other account loses `inbox` | defect | S | **done 2026-08-25**, unreleased. `sent_only()` keeps a message only when EVERY file is inside a sent folder, which is what the carve-out's docstring already claimed. No query can express it, measured; the root comes from `database.mail_root`, with a split-index fixture the ordinary layout cannot provide. Verified read-only against the live index: 780 of 807 still stripped, 27 spared, no arrival affected | | 167 | No way to tell one build of an unreleased version from another | enhancement | XS | **done 2026-08-25**, unreleased. The user chose a counter over a git description: `QTMAILDIR_BUILD_NUMBER`, a cmake option ON by default, increments a counter in the BUILD directory on every build and writes `buildnumber.h`. `QTMAILDIR_VERSION_DISPLAY` carries it; `QTMAILDIR_VERSION` stays clean and is what the window title, `applicationVersion` and the release procedure use | | 168 | Delete is offered on mail already in the trash, and does nothing | defect | S | **done 2026-08-25**, unreleased. Delete is hidden when every selected row is already in its account's trash, Restore when none is, both keyed on the PATH rather than the `deleted` tag. Delete also drops `unread` now, in the same TagChange so one undo returns the folder and the tag together . **Its rule is now incomplete, see item 178** (2026-08-28): the check still judges a row on `firstMessagePath`, which item 177 stopped being a conversation row's identity, so a conversation whose messages disagree about the trash answers on one of them | @@ -1115,109 +1115,7 @@ The 70-second duration recorded above fits a `QTRY_*` waiting for a file that is never going to appear, which is consistent with a wrong destination rather than a slow one. -## 165. A draft gets a new Message-ID on every autosave - -**Observed (developer, 2026-08-25), while hand-testing items 163 and 164.** -Four saved drafts produced four distinct Message-IDs, and one reopen-and-edit -turned one id into another. A draft therefore has no stable identity across -its own revisions. - -**Cause (verified in the code, not inferred).** `MessageBuilder::build()` -calls `g_mime_utils_generate_message_id()` unconditionally on every call -(`messagebuilder.cpp:311`), and every autosave calls `build()`. -`OutgoingMessage` has no field to carry an existing id in, and -`ComposeContext` has no field for the draft's OWN id either: it carries -`inReplyTo` and `references`, which are the ORIGINAL's id when replying, and -`ComposeContextBuilder::forDraft()` never reads the draft's Message-ID back -out of the file it parses. So this is not a changed call site; it needs a -field that does not exist yet, threaded from `forDraft()` through -`ComposeContext` and `OutgoingMessage` into `build()`. - -**Why it matters, and why it is NOT urgent.** To notmuch and to the server, -each revision is a different MESSAGE, not a new version of one. While the -file is replaced correctly this is invisible: one file in, one file out. It -becomes visible whenever a revision is NOT replaced, and item 163 is the -proof, where a stale path forked a draft into two files that were also two -messages and that nothing will ever collapse. Item 163's fix removes the -known way to reach that state; this entry is about the property that turned a -one-file mistake into a two-message one. - -An interrupted save is the remaining route: `DraftStore::write()` unlinks the -previous revision only AFTER the new file is safely in place (deliberately, -so a failed write cannot lose the draft), so a crash between the two leaves -two files, and with two ids they are two drafts rather than one duplicated. - -**Decided by the user on 2026-09-06: a stable id while drafting, DISCARDED at -send.** The second of the three positions below. A draft keeps one identity -across all its revisions, so each save replaces the previous message rather -than adding one; the sent copy is minted fresh and is a different item from -the draft, which is the user's own framing. The draft is DELETED on a -successful send, as it already is today, and the sent message carries no -`draft` tag. - -The three positions, kept because the reasoning is what makes the choice -reviewable: - -- A stable id reused at send time makes the draft and the sent message one - message, which is what a user means by "my draft became this email". It - also means the id was in a file mbsync uploaded to the drafts folder before - the message was ever sent, and the server has seen it. **Not chosen.** -- A stable id DISCARDED at send time keeps revisions collapsed while drafting - and mints a fresh id for the sent copy. Two identities, and the sent one is - the one that threads. **Chosen.** -- The current behaviour is a third position, and its only virtue is that no - id is ever reused for two different things. - -Whichever is chosen must be checked against `In-Reply-To`/`References` on the -eventual send, since `referencesForReply()` builds those from the ORIGINAL's -id and a draft of a reply carries both. - -**What is already built, measured 2026-09-06 rather than assumed.** Two of the -three things the decision describes exist: - -- `DraftStore::write()` removes the previous revision after the new file is in - place (`draftstore.cpp:78`), so "discard what is left after a successful - save" is done LOCALLY. -- `ComposeWindow` removes the draft on a successful send - (`composewindow.cpp:1620`, through `resolveRenamed()` since item 163), so - "delete the draft on send" is done, and the user confirmed that is the - wanted behaviour rather than keeping the last revision. -- The sent copy is built fresh and filed to the Sent folder, so it never - carries `draft`. The tag the user saw on a Sent row came from the orphan - revisions threading into the conversation, not from the sent message. **No - work is needed for that clause**; it was already true. - -**So the whole remaining defect is the id, and the mechanism is the server.** -Measured on the user's own mail: four drafts of one reply, four distinct -Message-IDs, and every one of the four files carrying a `,U=` infix. mbsync -had uploaded each revision to the drafts folder BEFORE the next save removed -the local file, so the server holds four independent messages and syncs all -four back down. The local cleanup is correct and cannot help: the damage -happens between a save and the next sync, and deleting a local file does not -retract an uploaded one. - -This is what raises the item above "invisible while the file is replaced -correctly". The file IS replaced correctly and four messages still exist. -A stable id gives the server one message being revised rather than four -unrelated ones. - -**Constraints.** - -- **A Message-ID reaches the server and every recipient**, so a reused id is - not a local matter. Two different messages sharing an id is worse than two - ids for one draft, which is what makes the current behaviour defensible as - a default rather than simply wrong. -- `MessageBuilder::build()` is on the SEND path as well as the autosave path. - A change that makes ids stable must not make two different sent messages - share one. -- The comment at `messagebuilder.cpp:297` records that GMime generates - neither Date nor Message-ID unless asked, and that a message without one - cannot be threaded by anything receiving it, this application's own index - of the sent copy included. Any "just omit it while drafting" variant has to - answer that. -- Item 163's fix stands on its own and this does not block it: the file is - replaced correctly now, so the fork this would have mitigated no longer - happens by that route. +--- ## 173. The composer is a plain-text editor, not WYSIWYG |