Skip to content

Fix: edit_draft no longer corrupts the other body on a partial edit#82

Merged
MadLlama25 merged 1 commit into
MadLlama25:mainfrom
JonathanGodley:fix/edit-draft-body-extraction
Jul 6, 2026
Merged

Fix: edit_draft no longer corrupts the other body on a partial edit#82
MadLlama25 merged 1 commit into
MadLlama25:mainfrom
JonathanGodley:fix/edit-draft-body-extraction

Conversation

@JonathanGodley

Copy link
Copy Markdown
Contributor

The bug

updateDraft rebuilds a draft's body to re-send it (JMAP bodies are immutable, so an edit is a destroy + recreate). It reads the existing bodies back from the fetched email:

const existingTextBody = ...Object.values(bodyValues).find((bv) =>
  existingEmail.textBody?.some((tb) => tb.partId === bv.partId || true));

The || true makes the predicate unconditionally true — and the bodyValue objects from Object.values() have no partId field anyway (partId is the map key). So both existingTextBody and existingHtmlBody resolve to Object.values(bodyValues)[0], the first part.

Consequences on an edit that doesn't explicitly set a given body:

  • Dual-format draft, edit only the subject (or only one body): the other body is overwritten with the first part's value. e.g. a subject-only edit replaces the HTML body with the plain-text value; since clients render the HTML alternative (RFC 2046 §5.1.4), the recipient sees the wrong content.
  • Single-format draft: the server aliases the one part into both the textBody and htmlBody lists (a text-only draft's htmlBody entry has type: "text/plain"), so the lookup pulls that value into the missing format and a phantom part is synthesised on recreate.

The fix

Select each existing body value by MIME type, keyed into bodyValues by the matched part's partId:

const bodyValues = existingEmail.bodyValues || {};
const bodyValueForType = (parts, mimeType) => {
  const part = parts?.find((p) => p.type === mimeType && p.partId != null && bodyValues[p.partId]);
  return part ? bodyValues[part.partId].value : undefined;
};
const existingTextValue = bodyValueForType(existingEmail.textBody, 'text/plain');
const existingHtmlValue = bodyValueForType(existingEmail.htmlBody, 'text/html');

Matching by MIME type (rather than only keying by partId) is what handles the single-format aliasing: a text-only draft's htmlBody entry is text/plain, so it correctly yields no HTML value and no phantom part. type is already requested in the Email/get bodyProperties, so there is no request change.

Verified against a live Fastmail account (text-only, html-only, and dual-part drafts):

  • subject-only edit on a dual draft → both bodies preserved
  • textBody-only edit → text updated, HTML partner preserved
  • text-only draft edit → no phantom html part

Tests

Adds 3 unit tests to the updateDraft suite covering the aliased single-format and dual-format shapes, asserting on the recreate output. Each fails before the fix and passes after.

(The 2 pre-existing safeWritePath symlink tests fail locally on Windows — EPERM, symlink creation needs elevation — but are untouched by this change and pass on CI.)

updateDraft extracted the existing body values with a `|| true` predicate
that always matched, so both the text and html bodies resolved to the first
bodyValue (Object.values(bodyValues)[0]). A subject-only edit, or an edit
touching only one body, therefore overwrote or lost the other format -- and
because mail clients render the HTML alternative (RFC 2046 5.1.4), recipients
could see the wrong content.

Select each existing body value by MIME type (text/plain, text/html), keyed
into bodyValues by the matched part's partId. The MIME-type match also avoids
synthesising a phantom part on a single-format draft, which the server aliases
into both the textBody and htmlBody lists.

Adds 3 unit tests covering the aliased single-format and dual-format shapes.
@MadLlama25 MadLlama25 merged commit 60e1255 into MadLlama25:main Jul 6, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants