fix: sign replies whose From is picked after the composer opens #842

components/email/__tests__/compose-format.test.ts

@@ -5,6 +5,7 @@ import {
   plainComposeBodyToHtml,
   replaceEmbeddedSignature,
   replacePlainTextSignature,
+  replacePlainTextSignatureAboveQuote,
   stripEmbeddedSignature,
 } from '../compose-format';
 import { SIGNATURE_BLOCK_MARKER, SIGNATURE_RANGE_MARKER, containsEmbeddedSignature } from '../signature-block';
@@ -131,6 +132,43 @@ describe('switching identity swaps the signature', () => {
     it('adds a signature at the end when there is no quote', () => {
       expect(replaceEmbeddedSignature('<p>Hi</p>', embed(other))).toBe(dom(`<p>Hi</p>${embed(other)}`));
     });
+
+    it('adds a signature above the quote header when the body had none (#842)', () => {
+      const html = '<p>Hi</p><p>On Mon, Bob wrote:</p><blockquote>original</blockquote>';
+      expect(replaceEmbeddedSignature(html, embed(other), { quoteHeader: 'On Mon, Bob wrote:' }))
+        .toBe(dom(`<p>Hi</p>${embed(other)}<p>On Mon, Bob wrote:</p><blockquote>original</blockquote>`));
+    });
+
+    it('adds a signature above a forward header that has no quote element', () => {
+      const html = '<p>Hi</p><p>---------- Forwarded message ----------<br>From: Bob</p><p>original</p>';
+      expect(replaceEmbeddedSignature(html, embed(other), { quoteHeader: '---------- Forwarded message ----------' }))
+        .toBe(dom(`<p>Hi</p>${embed(other)}<p>---------- Forwarded message ----------<br>From: Bob</p><p>original</p>`));
+    });
+  });
+
+  describe('replacePlainTextSignatureAboveQuote (#842)', () => {
+    const header = 'On Mon, Bob wrote:';
+    const quote = `${header}\n> original`;
+
+    it('adds the signature between the reply and the quote header', () => {
+      expect(replacePlainTextSignatureAboveQuote(`\n\n${quote}`, header, none, identity, { separator: true }))
+        .toBe(`\n\n-- \nAlice\nAcme\n\n${quote}`);
+      expect(replacePlainTextSignatureAboveQuote(`Hi\n\n${quote}`, header, none, identity, { separator: true }))
+        .toBe(`Hi\n\n-- \nAlice\nAcme\n\n${quote}`);
+    });
+
+    it('swaps or removes the signature above the quote', () => {
+      const signed = `Hi\n\n-- \nAlice\nAcme\n\n${quote}`;
+      expect(replacePlainTextSignatureAboveQuote(signed, header, identity, other, { separator: true }))
+        .toBe(`Hi\n\n-- \nTeam\n\n${quote}`);
+      expect(replacePlainTextSignatureAboveQuote(signed, header, identity, none, { separator: true }))
+        .toBe(`Hi\n\n${quote}`);
+    });
+
+    it('leaves a body alone whose quote header is gone', () => {
+      expect(replacePlainTextSignatureAboveQuote('Hi\n\n> original', header, none, identity, { separator: true }))
+        .toBe('Hi\n\n> original');
+    });
   });
 
   describe('replacePlainTextSignature', () => {

components/email/__tests__/composer-reply-signature.test.tsx

@@ -0,0 +1,318 @@
+import { render, screen, fireEvent, waitFor } from '@testing-library/react';
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import React from 'react';
+import { EmailComposer } from '../email-composer';
+import { useSettingsStore } from '@/stores/settings-store';
+import { useIdentityStore } from '@/stores/identity-store';
+import type { Identity } from '@/lib/jmap/types';
+
+// ─── Heavy component mocks (mirrors composer-format-toggle.test.tsx) ─
+
+// A stand-in for the TipTap editor: it hands the composer an editor object
+// whose content is whatever the composer last rendered into it, the way the
+// real editor reports itself ready from an effect after mounting.
+const editorState = vi.hoisted(() => ({ html: '' }));
+vi.mock('@/components/email/rich-text-editor', () => ({
+  RichTextEditor: ({ content, onChange, onEditorReady }: {
+    content: string;
+    onChange: (html: string) => void;
+    onEditorReady?: (editor: unknown) => void;
+  }) => {
+    editorState.html = content;
+    const onChangeRef = React.useRef(onChange);
+    onChangeRef.current = onChange;
+    React.useEffect(() => {
+      onEditorReady?.({
+        isDestroyed: false,
+        commands: {
+          focus: () => {},
+          setContent: (html: string) => {
+            editorState.html = html;
+            onChangeRef.current(html);
+          },
+        },
+      });
+      // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, []);
+    return React.createElement('div', { 'data-testid': 'rich-text-editor', 'data-html': content });
+  },
+}));
+
+vi.mock('@/components/plugins/plugin-slot', () => ({ PluginSlot: () => null }));
+vi.mock('@/components/identity/sub-address-helper', () => ({ SubAddressHelper: () => null }));
+vi.mock('@/components/templates/template-picker', () => ({ TemplatePicker: () => null }));
+vi.mock('@/components/templates/template-form', () => ({ TemplateForm: () => null }));
+vi.mock('@/components/files/file-preview-modal', () => ({ FilePreviewModal: () => null }));
+vi.mock('@/hooks/use-focus-trap', () => ({
+  useFocusTrap: () => ({ current: null }),
+}));
+vi.mock('@/hooks/use-pro-multi-account-identities', () => ({
+  useProMultiAccountIdentities: () => ({ enabled: false, groups: [], allIdentities: [] }),
+  stripCrossAccountIdentityPrefix: (id: string) => ({ localAccountId: null, rawId: id }),
+}));
+
+// ─── Store mocks ──────────────────────────────────────────────────────────────
+
+vi.mock('@/stores/auth-store', () => {
+  const state = {
+    client: null,
+    identities: [],
+    primaryIdentity: null,
+    isAuthenticated: false,
+    isDemoMode: false,
+    activeAccountId: null,
+    connectionLost: false,
+    getClientForAccount: () => undefined,
+    getAllConnectedClients: () => new Map(),
+    syncIdentities: () => {},
+    refreshIdentities: async () => {},
+  };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useAuthStore: hook };
+});
+
+vi.mock('@/stores/identity-store', () => {
+  const state = {
+    identities: [
+      { id: 'id-me', email: 'me@example.com', name: 'Me', htmlSignature: '<b>Alice</b>', textSignature: 'Alice' },
+      { id: 'id-team', email: 'team@example.com', name: 'Team', htmlSignature: '', textSignature: '' },
+    ] as Array<Record<string, unknown>>,
+    defaultIdentityId: 'id-me',
+  };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useIdentityStore: hook, notifyIdentitySelected: () => {} };
+});
+
+vi.mock('@/stores/account-store', () => {
+  const state = { accounts: [], getAccountById: () => undefined };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useAccountStore: hook };
+});
+
+vi.mock('@/stores/email-store', () => {
+  const state = {
+    draftSaveEnabled: false,
+    sendRawEmail: async () => ({ sent: true }),
+  };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useEmailStore: hook };
+});
+
+vi.mock('@/stores/settings-store', () => {
+  const state = {
+    timeFormat: '24h',
+    plainTextMode: false,
+    subAddressDelimiter: '+',
+    autoSelectReplyIdentity: true,
+    attachmentReminderEnabled: false,
+    attachmentReminderKeywords: [],
+    emptySubjectWarningEnabled: true,
+    sendDelaySeconds: 0,
+    // The default position: replies/forwards get the signature appended at
+    // send time rather than embedded above the quote.
+    signaturePosition: 'below_quote',
+    signatureSeparatorEnabled: true,
+    requestReadReceiptDefault: false,
+    addTrustedSender: () => {},
+    externalContentPolicy: 'block',
+    isSenderTrusted: () => false,
+    trustedSendersAddressBook: null,
+    updateSetting: () => {},
+  };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useSettingsStore: hook };
+});
+
+vi.mock('@/stores/contact-store', () => {
+  const state = {
+    contacts: [],
+    getAutocomplete: async () => [],
+    addToTrustedSendersBook: async () => {},
+  };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useContactStore: hook };
+});
+
+vi.mock('@/stores/template-store', () => {
+  const state = { templates: [], addTemplate: async () => {} };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useTemplateStore: hook };
+});
+
+// ─── Misc dependency mocks ────────────────────────────────────────────────────
+
+vi.mock('@/stores/toast-store', () => ({
+  toast: { info: () => {}, error: () => {}, success: () => {} },
+}));
+
+vi.mock('@/lib/plugin-hooks', () => ({
+  identityHooks: {
+    onSignatureRender: { size: 0, transform: async (value: unknown) => value },
+    onIdentitySelect: { size: 0, emit: async () => {} },
+  },
+  templateHooks: {
+    onTemplateRender: { size: 0, transform: async (value: unknown) => value },
+    onTemplateApply: { size: 0, emit: async () => {} },
+  },
+  emailHooks: {
+    onComposerOpen: { call: async () => [] },
+    onRecipientChange: { call: async () => [] },
+    getRecipientSuggestions: { call: async () => [] },
+    onRecipientChipsChange: { transform: async (chips: unknown) => chips },
+    onDraftChange: { emit: () => {} },
+    onBeforeDraftAutoSave: { transform: async (draft: unknown) => draft },
+    onBeforeEmailSend: { intercept: async () => true, interceptOutcome: async () => 'allowed' },
+    onComposeSend: { takeOver: async () => 'default' },
+    onTransformOutgoingEmail: { transform: async (email: unknown) => email },
+  },
+  contactHooks: {
+    search: { call: async () => [] },
+    onProvideRecipientSuggestions: { transform: async (initial: unknown) => initial },
+  },
+}));
+
+vi.mock('@/lib/email-sanitization', () => ({
+  sanitizeSignatureHtml: (v: string) => v,
+  sanitizeSignatureHtmlForDisplay: (v: string) => v,
+  sanitizeEmailHtml: (v: string) => v,
+  sanitizePluginBodyHtml: (v: string) => v,
+  escapeHtml: (v: string) => v,
+  parseHtmlSafely: (html: string) => new DOMParser().parseFromString(html, 'text/html'),
+}));
+
+vi.mock('@/lib/email-threading', () => ({
+  computeReplyThreadingHeaders: () => ({ inReplyTo: [], references: [] }),
+}));
+vi.mock('@/lib/sub-addressing', () => ({ generateSubAddress: () => '' }));
+vi.mock('@/lib/debug', () => ({ debug: { log: () => {}, warn: () => {}, error: () => {} } }));
+vi.mock('@/components/email/quoted-html', () => ({
+  buildQuotedHtmlBlock: (html: string) => `<div data-quoted-html="">${html}</div>`,
+  serializeEditorContent: () => editorState.html,
+}));
+vi.mock('@/lib/template-utils', () => ({ substitutePlaceholders: (s: string) => s }));
+
+// ─── Tests ────────────────────────────────────────────────────────────────────
+
+/**
+ * #842: replying did not insert the signature. With the signature above the
+ * quote, a reply's body is built before its From is known - the composer
+ * picks the identity the mail was sent to in an effect, and the identities
+ * may load after the composer opened. A plain-text reply never received the
+ * signature that arrived then, and the send path doesn't append one to an
+ * above-quote reply, so the mail went out unsigned. An HTML reply got it, but
+ * between the "On ... wrote:" line and the quote.
+ */
+
+const ME = { id: 'id-me', email: 'me@example.com', name: 'Me', htmlSignature: '<b>Alice</b>', textSignature: 'Alice' };
+const UNSIGNED_ME = { ...ME, htmlSignature: '', textSignature: '' };
+const SALES = { id: 'id-sales', email: 'sales@example.com', name: 'Sales', htmlSignature: '<i>Sales team</i>', textSignature: 'Sales team' };
+
+const REPLY_TO = {
+  from: [{ name: 'Bob', email: 'bob@example.com' }],
+  to: [{ email: 'sales@example.com' }],
+  subject: 'Question',
+  body: 'Original text',
+  htmlBody: '<p>Original text</p>',
+  receivedAt: '2026-08-01T10:00:00Z',
+};
+const TO_ME = { ...REPLY_TO, to: [{ email: 'me@example.com' }] };
+
+const sendButton = () => screen.getAllByTestId('composer-send')[0] as HTMLButtonElement;
+
+async function send(onSend: ReturnType<typeof vi.fn>) {
+  fireEvent.click(sendButton());
+  await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1));
+  return onSend.mock.calls[0][0] as { body: string; htmlBody?: string };
+}
+
+function setIdentities(list: Array<Partial<Identity>>) {
+  useIdentityStore.setState({ identities: list as Identity[] });
+}
+
+describe('reply signature above the quote (#842)', () => {
+  afterEach(() => {
+    useSettingsStore.setState({ plainTextMode: false, signaturePosition: 'below_quote' });
+    setIdentities([ME]);
+    vi.clearAllMocks();
+  });
+
+  describe('plain text', () => {
+    it('signs a reply from an alias the composer picks itself', async () => {
+      useSettingsStore.setState({ signaturePosition: 'above_quote', plainTextMode: true });
+      setIdentities([UNSIGNED_ME, SALES]);
+      const onSend = vi.fn();
+      render(<EmailComposer mode="reply" replyTo={REPLY_TO} onSend={onSend} />);
+
+      const sent = await send(onSend);
+      expect(sent.body).toBe('\n\n-- \nSales team\n\nreply_line\n> Original text');
+    });
+
+    it('swaps in the signature of the picked alias instead of the default one', async () => {
+      useSettingsStore.setState({ signaturePosition: 'above_quote', plainTextMode: true });
+      setIdentities([ME, SALES]);
+      const onSend = vi.fn();
+      render(<EmailComposer mode="replyAll" replyTo={REPLY_TO} onSend={onSend} />);
+
+      const sent = await send(onSend);
+      expect(sent.body).toBe('\n\n-- \nSales team\n\nreply_line\n> Original text');
+    });
+
+    it('signs a forward when the identities load after it opened', async () => {
+      useSettingsStore.setState({ signaturePosition: 'above_quote', plainTextMode: true });
+      setIdentities([]);
+      const onSend = vi.fn();
+      const props = { mode: 'forward' as const, replyTo: TO_ME, prefill: { to: ['carol@example.com'] }, onSend };
+      const { rerender } = render(<EmailComposer {...props} />);
+      setIdentities([ME]);
+      rerender(<EmailComposer {...props} />);
+
+      const sent = await send(onSend);
+      expect(sent.body.startsWith('\n\n-- \nAlice\n\nforwarded_separator\n')).toBe(true);
+    });
+  });
+
+  describe('HTML', () => {
+    it('puts the signature of the picked alias above the "wrote:" line', async () => {
+      useSettingsStore.setState({ signaturePosition: 'above_quote' });
+      setIdentities([UNSIGNED_ME, SALES]);
+      const onSend = vi.fn();
+      render(<EmailComposer mode="reply" replyTo={REPLY_TO} onSend={onSend} />);
+
+      const sent = await send(onSend);
+      const html = sent.htmlBody ?? '';
+      expect(html).toContain('<i>Sales team</i>');
+      expect(html.indexOf('<i>Sales team</i>')).toBeLessThan(html.indexOf('reply_line'));
+    });
+  });
+
+  it('still signs a reply below the quote at send time by default', async () => {
+    setIdentities([UNSIGNED_ME, SALES]);
+    const onSend = vi.fn();
+    render(<EmailComposer mode="reply" replyTo={REPLY_TO} onSend={onSend} />);
+
+    const sent = await send(onSend);
+    expect(sent.htmlBody).toMatch(/Original text[\s\S]*<i>Sales team<\/i>$/);
+    expect(sent.body).toMatch(/Original text[\s\S]*Sales team$/);
+  });
+});

components/email/compose-format.ts

@@ -120,15 +120,25 @@ export function stripEmbeddedSignature(html: string): { html: string; hadSignatu
  * The HTML body with its embedded signature swapped for `signatureHtml` (from
  * buildEmbeddedSignatureHtml; empty for an identity without a signature).
  * The new signature goes where the old one was. A body without one - the
- * previous identity had none - gets it before the quote, or at the end when
- * there is no quote, which is where getInitialBody would have put it.
+ * previous identity had none - gets it where getInitialBody would have put
+ * it: above the quote header of a reply or forward (`quoteHeader`, the
+ * header's first line, e.g. "On ... wrote:"), else before the quote, else at
+ * the end.
  */
-export function replaceEmbeddedSignature(html: string, signatureHtml: string): string {
+export function replaceEmbeddedSignature(
+  html: string,
+  signatureHtml: string,
+  options: { quoteHeader?: string } = {},
+): string {
   const doc = new DOMParser().parseFromString(html, 'text/html');
   let removed = removeSignatureRange(doc);
   if (!removed) {
     if (!signatureHtml) return html;
-    const quote = Array.from(doc.body.childNodes).find(isQuoteBoundary) ?? null;
+    const children = Array.from(doc.body.childNodes);
+    const header = options.quoteHeader
+      ? children.find((node) => node.textContent?.includes(options.quoteHeader!))
+      : undefined;
+    const quote = header ?? children.find(isQuoteBoundary) ?? null;
     removed = { parent: doc.body, insertBefore: quote };
   }
   const host = doc.createElement('div');
@@ -157,6 +167,25 @@ export function replacePlainTextSignature(
   return appendPlainTextSignature(bare, next, options);
 }
 
+/**
+ * replacePlainTextSignature for a plain-text reply or forward with the
+ * signature above the quote: the signature ends the part before the quote
+ * header (`quoteHeader`, its first line). A body whose header can't be found
+ * is returned unchanged.
+ */
+export function replacePlainTextSignatureAboveQuote(
+  body: string,
+  quoteHeader: string,
+  previous: SignatureIdentityLike,
+  next: SignatureIdentityLike,
+  options: { separator: boolean },
+): string {
+  const at = quoteHeader ? body.indexOf(quoteHeader) : -1;
+  if (at < 0) return body;
+  const head = body.slice(0, at).replace(/\n+$/, '');
+  return `${replacePlainTextSignature(head, previous, next, options)}\n\n${body.slice(at)}`;
+}
+
 /**
  * Convert the rich-text editor's body to a plain-text body. Formatting is
  * dropped (that is the point of plain text); an embedded signature comes back

components/email/email-composer.tsx

@@ -58,6 +58,7 @@ import {
   plainComposeBodyToHtml,
   replaceEmbeddedSignature,
   replacePlainTextSignature,
+  replacePlainTextSignatureAboveQuote,
 } from "@/components/email/compose-format";
 import { emailHooks, contactHooks, isExternalAttachmentResult } from "@/lib/plugin-hooks";
 import { onUploadProgress } from "@/lib/upload-progress";
@@ -554,6 +555,29 @@ export function EmailComposer({
     return "";
   };
 
+  // The first line of the quote header getInitialBody writes under the reply
+  // area ("On ... wrote:", the forwarded-message line, or a plugin's header).
+  // A signature that arrives after the body was built - From resolves to a
+  // matching identity, or the identities load late - goes above it.
+  const getQuoteHeaderLine = (): string => {
+    if (mode !== 'reply' && mode !== 'replyAll' && mode !== 'forward') return '';
+    if (!replyTo?.body && !replyTo?.htmlBody) return '';
+    const pluginHeader = replyTo.quoteHeaderText
+      ?? (replyTo.quoteHeaderHtml !== undefined ? htmlToPlainText(replyTo.quoteHeaderHtml) : undefined);
+    if (pluginHeader !== undefined) {
+      return pluginHeader.split('\n').map((line) => line.trim()).find(Boolean) ?? '';
+    }
+    if (mode === 'forward') return tQuote('forwarded_separator');
+    const date = replyTo.receivedAt ? formatDateTime(replyTo.receivedAt, timeFormat, { weekday: 'short', year: 'numeric', month: 'short', day: 'numeric' }) : "";
+    const from = replyTo.from?.[0];
+    const fromStrFull = from
+      ? (from.name && from.email && from.name !== from.email
+          ? `${from.name} <${from.email}>`
+          : (from.email || from.name || tCommon('unknown')))
+      : tCommon('unknown');
+    return tQuote('reply_line', { date, from: fromStrFull });
+  };
+
   const getInitialBody = () => {
     if (plainTextMode) {
       // Plain text mode: produce plain text body with no HTML
@@ -1062,11 +1086,20 @@ export function EmailComposer({
 
     if (plainTextMode) {
       // A new plain-text message carries the signature at the end of the
-      // body; in a reply the quote follows it.
-      if (!identityChanged || mode !== 'compose') return;
-      setBody((prev) => replacePlainTextSignature(prev, previousIdentity, signatureIdentity, {
-        separator: signatureSeparatorEnabled,
-      }));
+      // body, a reply or forward above its quote header.
+      if (!identityChanged) return;
+      const options = { separator: signatureSeparatorEnabled };
+      if (mode === 'compose') {
+        setBody((prev) => replacePlainTextSignature(prev, previousIdentity, signatureIdentity, options));
+        return;
+      }
+      // The signature identity of a reply is often only known after the body
+      // was built: the reply's From resolves to the alias it was sent to, or
+      // the identities load late. Swap it above the quote header like an
+      // HTML body does; leaving it out lost it, since an above-quote reply
+      // is never signed at send time (#842).
+      const quoteHeader = getQuoteHeaderLine();
+      setBody((prev) => replacePlainTextSignatureAboveQuote(prev, quoteHeader, previousIdentity, signatureIdentity, options));
       return;
     }
     if (!editor) return;
@@ -1082,7 +1115,7 @@ export function EmailComposer({
     const nextHtml = replaceEmbeddedSignature(currentHtml, buildEmbeddedSignatureHtml(signatureIdentity, {
       embed: true,
       separator: signatureSeparatorEnabled,
-    }));
+    }), { quoteHeader: getQuoteHeaderLine() });
     if (nextHtml !== currentHtml) {
       editor.commands.setContent(nextHtml, { emitUpdate: true });
     }