fix: auto-send read receipts only for the message on screen and clean up failed ones

With "Always send" the banner's auto-send ran in every page the viewer
pager keeps mounted, so opening one message also answered the receipt
requests of its unseen neighbours. The receipt is imported into Sent
before it is submitted, so a refused submission left a "Read: ..." copy
there, and the failure cleared the per-instance guard, so the next
render or remount imported another one.

- the pager marks the page on screen `active`, and the banner only
  auto-sends there
- an automatic receipt is tried once per message per session, keyed by
  account and id, so a failure or a remounted page does not send again
- `sendReadReceipt` destroys the imported copy when the submission
  fails, which also covers the manual "Send receipt" button

src/api/__tests__/read-receipt.test.ts

@@ -0,0 +1,154 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+vi.mock('../jmap-client', () => ({
+  jmapClient: {
+    accountId: 'acc-1',
+    request: vi.fn(),
+    getMaxObjectsInSet: vi.fn(() => 500),
+  },
+}));
+
+// blob.ts pulls in expo-file-system; the receipt only needs the byte upload.
+const uploadBytes = vi.fn(async (..._args: unknown[]) => ({ blobId: 'blob-1', size: 1, type: 'message/rfc822' }));
+vi.mock('../blob', () => ({
+  uploadBytes: (...args: unknown[]) => uploadBytes(...args),
+}));
+
+import { jmapClient } from '../jmap-client';
+import { sendReadReceipt } from '../email';
+
+const mockRequest = jmapClient.request as ReturnType<typeof vi.fn>;
+
+type Call = [string, Record<string, any>, string];
+
+/** Answer each JMAP method with the given handler; records every call. */
+function serve(handlers: Record<string, (args: Record<string, any>) => unknown>) {
+  const calls: Call[] = [];
+  mockRequest.mockImplementation(async (methodCalls: Call[]) => {
+    calls.push(...methodCalls);
+    return {
+      methodResponses: methodCalls.map(([name, args, id]) => {
+        const handler = handlers[name];
+        if (!handler) throw new Error(`unexpected ${name}`);
+        const result = handler(args);
+        return (Array.isArray(result) ? [...result, id] : [name, result, id]) as unknown;
+      }),
+    };
+  });
+  return calls;
+}
+
+const opts = {
+  to: 'sender@remote.example',
+  fromEmail: 'me@x.example',
+  identityId: 'ident-1',
+  sentMailboxId: 'sent-1',
+  originalSubject: 'Hello',
+  automatic: true,
+};
+
+const imported = () => ({ created: { 'import-0': { id: 'mdn-email-1' } } });
+const destroyed = (args: Record<string, any>) => ({ destroyed: args.destroy });
+
+beforeEach(() => {
+  vi.clearAllMocks();
+});
+
+describe('sendReadReceipt', () => {
+  it('imports the receipt into Sent and submits it', async () => {
+    const calls = serve({
+      'Email/import': imported,
+      'EmailSubmission/set': () => ({ created: { mdn: { id: 'sub-1' } } }),
+      'Email/set': destroyed,
+    });
+
+    await expect(sendReadReceipt(opts)).resolves.toBe('mdn-email-1');
+
+    expect(calls.map((c) => c[0])).toEqual(['Email/import', 'EmailSubmission/set']);
+    expect(calls[0][1].emails['import-0']).toMatchObject({ blobId: 'blob-1', mailboxIds: { 'sent-1': true } });
+    expect(calls[1][1].create.mdn).toMatchObject({
+      emailId: 'mdn-email-1',
+      identityId: 'ident-1',
+      envelope: { mailFrom: { email: 'me@x.example' }, rcptTo: [{ email: 'sender@remote.example' }] },
+    });
+  });
+
+  it('uploads a receipt whose headers a CRLF in the original subject cannot extend', async () => {
+    serve({
+      'Email/import': imported,
+      'EmailSubmission/set': () => ({ created: { mdn: { id: 'sub-1' } } }),
+    });
+
+    await sendReadReceipt({
+      ...opts,
+      subject: 'Read: Contract update\r\nX-Injected: yes\r\nReply-To: attacker@evil.example',
+    });
+
+    const raw = new TextDecoder().decode(uploadBytes.mock.calls[0][0] as Uint8Array);
+    expect(raw).not.toMatch(/^X-Injected:/m);
+    expect(raw).not.toMatch(/^Reply-To:/m);
+    expect(raw).toContain('Subject: Read: Contract update X-Injected: yes Reply-To: attacker@evil.example\r\n');
+  });
+
+  it('destroys the imported copy when the server refuses the submission', async () => {
+    const calls = serve({
+      'Email/import': imported,
+      'EmailSubmission/set': () => ({
+        notCreated: { mdn: { type: 'invalidRecipients', description: 'No valid recipients' } },
+      }),
+      'Email/set': destroyed,
+    });
+
+    await expect(sendReadReceipt({ ...opts, accountId: 'grp-1' })).rejects.toThrow(/No valid recipients/);
+
+    const destroy = calls.find((c) => c[0] === 'Email/set');
+    expect(destroy?.[1]).toEqual({ accountId: 'grp-1', destroy: ['mdn-email-1'] });
+  });
+
+  it('destroys the imported copy when the submission call errors', async () => {
+    const calls = serve({
+      'Email/import': imported,
+      'EmailSubmission/set': () => ['error', { type: 'forbiddenFrom', description: 'Not allowed' }],
+      'Email/set': destroyed,
+    });
+
+    await expect(sendReadReceipt(opts)).rejects.toThrow(/Not allowed/);
+    expect(calls.find((c) => c[0] === 'Email/set')?.[1]).toEqual({ accountId: 'acc-1', destroy: ['mdn-email-1'] });
+  });
+
+  it('destroys the imported copy when the submission request fails in transit', async () => {
+    const calls: Call[] = [];
+    mockRequest.mockImplementation(async (methodCalls: Call[]) => {
+      calls.push(...methodCalls);
+      const [name, args, id] = methodCalls[0];
+      if (name === 'Email/import') return { methodResponses: [[name, imported(), id]] };
+      if (name === 'EmailSubmission/set') throw new Error('Network request failed');
+      return { methodResponses: [[name, destroyed(args), id]] };
+    });
+
+    await expect(sendReadReceipt(opts)).rejects.toThrow('Network request failed');
+    expect(calls.map((c) => c[0])).toEqual(['Email/import', 'EmailSubmission/set', 'Email/set']);
+  });
+
+  it('still reports the submission error when the cleanup fails too', async () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
+    serve({
+      'Email/import': imported,
+      'EmailSubmission/set': () => ({ notCreated: { mdn: { type: 'forbiddenToSend' } } }),
+      'Email/set': () => ({ notDestroyed: { 'mdn-email-1': { type: 'forbidden' } } }),
+    });
+
+    await expect(sendReadReceipt(opts)).rejects.toThrow(/forbiddenToSend/);
+    expect(warn).toHaveBeenCalled();
+    warn.mockRestore();
+  });
+
+  it('does not submit or destroy anything when the import fails', async () => {
+    const calls = serve({
+      'Email/import': () => ({ notCreated: { 'import-0': { type: 'overQuota' } } }),
+    });
+
+    await expect(sendReadReceipt(opts)).rejects.toThrow(/overQuota/);
+    expect(calls.map((c) => c[0])).toEqual(['Email/import']);
+  });
+});

src/api/email.ts

@@ -1450,8 +1450,9 @@ export interface SendReadReceiptOptions extends MdnOptions {
  * Send an RFC 8098 read receipt (MDN). JMAP has no MDN primitive, so the
  * multipart/report is built client-side (`lib/mdn.ts`), uploaded as a blob,
  * imported into Sent and submitted with an explicit envelope - mirrors the
- * webmail's `client.sendReadReceipt`. The caller flags the original
- * `$mdnsent` afterwards.
+ * webmail's `client.sendReadReceipt`. When the submission fails the imported
+ * copy is destroyed again, so a refused receipt does not sit in Sent as if
+ * it had gone out. The caller flags the original `$mdnsent` afterwards.
  */
 export async function sendReadReceipt(opts: SendReadReceiptOptions): Promise<string> {
   const accountId = opts.accountId ?? jmapClient.accountId;
@@ -1462,25 +1463,32 @@ export async function sendReadReceipt(opts: SendReadReceiptOptions): Promise<str
   const { uploadBytes } = await import('./blob');
   const upload = await uploadBytes(bytes, 'message/rfc822', accountId);
   const emailId = await importEmailBlob(upload.blobId, opts.sentMailboxId, { $seen: true }, accountId);
-  const res = await jmapClient.request(
-    [
-      ['EmailSubmission/set', {
-        accountId,
-        create: {
-          mdn: {
-            emailId,
-            identityId: opts.identityId,
-            envelope: {
-              mailFrom: { email: opts.fromEmail },
-              rcptTo: [{ email: opts.to }],
+  try {
+    const res = await jmapClient.request(
+      [
+        ['EmailSubmission/set', {
+          accountId,
+          create: {
+            mdn: {
+              emailId,
+              identityId: opts.identityId,
+              envelope: {
+                mailFrom: { email: opts.fromEmail },
+                rcptTo: [{ email: opts.to }],
+              },
             },
           },
-        },
-      }, '0'],
-    ],
-    SUBMISSION_USING,
-  );
-  const body = requireMethodResult(res, '0', 'EmailSubmission/set');
-  assertSetResult(body, ['mdn'], 'read receipt');
+        }, '0'],
+      ],
+      SUBMISSION_USING,
+    );
+    const body = requireMethodResult(res, '0', 'EmailSubmission/set');
+    assertSetResult(body, ['mdn'], 'read receipt');
+  } catch (err) {
+    await destroyEmails([emailId], accountId).catch((cleanupErr) => {
+      console.warn('[mdn] failed to remove the unsent receipt from Sent:', cleanupErr);
+    });
+    throw err;
+  }
   return emailId;
 }

src/components/email/MessageContent.tsx

@@ -19,6 +19,8 @@ export interface MessageContentProps {
   jmapAccountId?: string;
   identities: Identity[];
   currentMailboxRole?: string | null;
+  /** On screen rather than pre-rendered by the pager (gates auto-sent read receipts). */
+  active: boolean;
   themeOverride?: 'light' | 'dark' | null;
   onSwipe?: (direction: 'prev' | 'next') => void;
   onZoomChange?: (zoom: { pinching: boolean; zoomed: boolean }) => void;
@@ -37,7 +39,7 @@ export interface MessageContentProps {
  * cards.
  */
 export function MessageContent({
-  email, jmapAccountId, identities, currentMailboxRole, themeOverride, onSwipe, onZoomChange,
+  email, jmapAccountId, identities, currentMailboxRole, active, themeOverride, onSwipe, onZoomChange,
   onToggleStar, onAddressPress, onEmailPatched, compact,
 }: MessageContentProps) {
   const c = useColors();
@@ -80,6 +82,7 @@ export function MessageContent({
           requestedBy={headerInfo.readReceiptRequestedBy}
           jmapAccountId={jmapAccountId}
           currentMailboxRole={currentMailboxRole}
+          active={active}
           onHandled={onEmailPatched}
         />
       )}

src/components/email/ReadReceiptBanner.tsx

@@ -8,6 +8,7 @@ import { useLocaleStore } from '../../stores/locale-store';
 import { useSettingsStore } from '../../stores/settings-store';
 import { useEmailStore } from '../../stores/email-store';
 import { sendReadReceipt, patchKeywordsForEmails } from '../../api/email';
+import { jmapClient } from '../../api/jmap-client';
 import { findReceivingIdentity } from '../../lib/email-headers';
 import { mailboxesForSiblingOf } from '../../lib/mailbox-tree';
 
@@ -18,17 +19,24 @@ interface Props {
   jmapAccountId?: string;
   /** Role of the folder the message was opened from (receipts only in received folders). */
   currentMailboxRole?: string | null;
+  /** The message is on screen, not in a neighbour page the pager pre-renders. */
+  active: boolean;
   /** Reflect `$mdnsent` in the caller's cache. */
   onHandled: (email: Email) => void;
 }
 
+// Messages "always" mode has already tried to answer this session, keyed by
+// account and id. One attempt each: a failed send is not retried, and a page
+// the pager unmounts and mounts again does not send a second time.
+const autoAttempted = new Set<string>();
+
 /**
  * Read-receipt (MDN, RFC 8098) request banner: Send / Ignore, or auto-send
  * in "always" mode. Either answer flags the message `$mdnsent` (RFC 3503) so
  * the request is suppressed in every client, not just here. Never offered
  * for the user's own copies, trash or spam.
  */
-export function ReadReceiptBanner({ email, requestedBy, jmapAccountId, currentMailboxRole, onHandled }: Props) {
+export function ReadReceiptBanner({ email, requestedBy, jmapAccountId, currentMailboxRole, active, onHandled }: Props) {
   const c = useColors();
   const styles = React.useMemo(() => makeStyles(c), [c]);
   const t = useLocaleStore((s) => s.t);
@@ -39,7 +47,6 @@ export function ReadReceiptBanner({ email, requestedBy, jmapAccountId, currentMa
   const currentMailboxId = useEmailStore((s) => s.currentMailboxId);
   const [busy, setBusy] = React.useState(false);
   const [handledLocally, setHandledLocally] = React.useState(false);
-  const autoRef = React.useRef<string | null>(null);
 
   React.useEffect(() => { setHandledLocally(false); }, [email.id]);
   React.useEffect(() => { if (identities.length === 0) void fetchIdentities(); }, [identities.length, fetchIdentities]);
@@ -88,17 +95,17 @@ export function ReadReceiptBanner({ email, requestedBy, jmapAccountId, currentMa
     await flagSent();
   }, [identity, mailboxes, currentMailboxId, requestedBy, jmapAccountId, email.messageId, email.subject, t, flagSent]);
 
-  // "always" mode: auto-send once when the message is opened.
+  // "always" mode: auto-send once when the message is actually displayed.
   React.useEffect(() => {
-    if (readReceiptResponse !== 'always' || !shouldOffer) return;
-    if (autoRef.current === email.id) return;
-    autoRef.current = email.id;
+    if (readReceiptResponse !== 'always' || !shouldOffer || !active) return;
+    const key = `${jmapAccountId ?? jmapClient.accountId}:${email.id}`;
+    if (autoAttempted.has(key)) return;
+    autoAttempted.add(key);
     setHandledLocally(true);
     send(true).catch((err) => {
       console.warn('[mdn] auto-send failed', err);
-      autoRef.current = null;
     });
-  }, [readReceiptResponse, shouldOffer, email.id, send]);
+  }, [readReceiptResponse, shouldOffer, active, jmapAccountId, email.id, send]);
 
   if (!shouldOffer || readReceiptResponse === 'always') return null;
 

src/screens/EmailThreadScreen.tsx

@@ -648,6 +648,7 @@ export default function EmailThreadScreen({ route, navigation }: Props) {
               <View style={{ width: windowWidth }}>
                 <EmailPane
                   id={item.id}
+                  active={item.id === activeEmailId}
                   threadIdHint={item.threadId}
                   email={detailCache.get(item.id) ?? null}
                   detailCache={detailCache}
@@ -787,6 +788,8 @@ export default function EmailThreadScreen({ route, navigation }: Props) {
 
 interface EmailPaneProps {
   id: string;
+  /** The page on screen; its neighbours are only pre-rendered. */
+  active: boolean;
   threadIdHint?: string;
   email: Email | null;
   detailCache: Map<string, Email>;
@@ -815,7 +818,7 @@ interface EmailPaneProps {
 // expand). The pager keeps three of these mounted (prev, current, next) so a
 // swipe slides ready content into view.
 function EmailPane({
-  id, threadIdHint, email, detailCache, threadIds, threading, jmapAccountId, currentMailboxRole,
+  id, active, threadIdHint, email, detailCache, threadIds, threading, jmapAccountId, currentMailboxRole,
   identities, themeOverrides, ensureDetail, ensureThread, scheduleMarkRead, styles, bottomBarHeight,
   onToggleStar, onAddressPress, onEmailPatched, onReply, onSwipe, onZoomChange,
 }: EmailPaneProps) {
@@ -923,6 +926,7 @@ function EmailPane({
             jmapAccountId={jmapAccountId}
             identities={identities}
             currentMailboxRole={currentMailboxRole}
+            active={active}
             themeOverride={themeOverrides[m.id] ?? null}
             onSwipe={onSwipe}
             onZoomChange={(z) => { setPinching(z.pinching); onZoomChange(z); }}
@@ -937,6 +941,7 @@ function EmailPane({
           jmapAccountId={jmapAccountId}
           identities={identities}
           currentMailboxRole={currentMailboxRole}
+          active={active}
           themeOverride={themeOverrides[email.id] ?? null}
           onSwipe={onSwipe}
           onZoomChange={(z) => { setPinching(z.pinching); onZoomChange(z); }}