fix: keep one Message-ID across a draft's revisions

Every autosave called MessageBuilder::build(), which generated a fresh
Message-ID unconditionally, so each revision of a draft was a different
message rather than a new version of one. The entry called this invisible
while the file is replaced correctly, and that turned out to be wrong:
mbsync uploads each revision to the drafts folder before the next save
removes the local file, so the server keeps one message per revision and
syncs them all back down. Measured on real mail as four independent
messages for a single reply, all four carrying a ,U= infix, threading
into the conversation and putting a draft tag on a Sent row. Deleting a
local file does not retract an uploaded one, which is why the local
cleanup, which is correct, could never fix it.

The user chose a stable id while drafting, discarded at send: the sent
copy is a different item from the draft, and the draft is deleted once
the message goes out, which the code already did.

Three links, none of which existed. OutgoingMessage::messageId is the
field, where empty means generate, 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.
ComposeWindow::m_draftMessageId holds the identity between revisions,
assigned from built.messageId so the first save adopts the id GMime just
generated, and ComposeContext::draftMessageId carries it across a reopen,
read in forDraft() from ParsedMessage::messageId, which the parser
already provided and nothing had ever used.

Five tests, because the property spans three objects and a test at any
one of them passes while another link is broken. Two are the safety
constraint rather than the feature: a field defaulting to a fixed value
would satisfy the reuse test and make two sent messages share an id,
which is far worse than the defect this fixes.

One comment is corrected rather than left: the autosave's dirty check
justified comparing the message rather than the built bytes with "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.

Revisions already on the server are not touched by this; the four found
on real mail were deleted by hand.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jq9gXquUo9W4KXDagJXMmn

1 files changed, 158 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 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.
+