fix: check JMAP method errors in every mail call, split requests to server limits and file sent mail through Drafts (#188, #234, #850)

src/api/__tests__/email.test.ts

@@ -8,6 +8,8 @@ vi.mock('../jmap-client', () => ({
     getAccountName: vi.fn(() => 'me@example.com'),
     getSharedMailAccounts: vi.fn(() => []),
     getMaxCallsInRequest: vi.fn(() => 16),
+    getMaxObjectsInGet: vi.fn(() => 500),
+    getMaxObjectsInSet: vi.fn(() => 500),
   },
 }));
 
@@ -394,16 +396,18 @@ describe('email operations', () => {
           to: [{ email: 'you@example.com' }],
           subject: 'Re: Hello',
           textBody: 'replying',
-          inReplyTo: '<msg-1@example.com>',
-          references: '<msg-0@example.com> <msg-1@example.com>',
+          inReplyTo: ['<msg-1@example.com>'],
+          references: ['<msg-0@example.com>', '<msg-1@example.com>'],
         },
         'identity-1',
         'sent-mb',
       );
 
       const emailCreate = mockRequest.mock.calls[0][0][0][1].create.draft;
-      expect(emailCreate['header:In-Reply-To:asText']).toBe('<msg-1@example.com>');
-      expect(emailCreate['header:References:asText']).toBe('<msg-0@example.com> <msg-1@example.com>');
+      // RFC 8621 §4.1.2.3: JMAP arrays of bare msg-ids, brackets stripped.
+      expect(emailCreate.inReplyTo).toEqual(['msg-1@example.com']);
+      expect(emailCreate.references).toEqual(['msg-0@example.com', 'msg-1@example.com']);
+      expect(emailCreate['header:In-Reply-To:asText']).toBeUndefined();
     });
   });
 });

src/api/__tests__/identity.test.ts

@@ -36,9 +36,15 @@ describe('identity operations', () => {
   describe('createIdentity', () => {
     it('should create a new identity', async () => {
       const created = { id: 'id-new', name: 'Work', email: 'work@example.com' };
-      mockRequest.mockResolvedValue({
-        methodResponses: [['Identity/set', { created: { 'new-identity': created } }, '0']],
-      });
+      // The create echo carries only server-set properties; the API re-reads
+      // the full identity afterwards.
+      mockRequest
+        .mockResolvedValueOnce({
+          methodResponses: [['Identity/set', { created: { 'new-identity': { id: 'id-new' } } }, '0']],
+        })
+        .mockResolvedValueOnce({
+          methodResponses: [['Identity/get', { list: [created] }, '0']],
+        });
 
       const result = await createIdentity({ name: 'Work', email: 'work@example.com' });
       expect(result).toEqual(created);

src/api/identity.ts

@@ -1,28 +1,43 @@
 import { jmapClient } from './jmap-client';
+import { assertSetResult, requireMethodResult } from './jmap-result';
 import { CAPABILITIES } from './types';
 import type { Identity } from './types';
+import { sanitizeDisplayName } from '../lib/rfc5322-mailbox';
 
-export async function getIdentities(): Promise<Identity[]> {
-  const accountId = jmapClient.accountId;
-  const res = await jmapClient.request(
-    [['Identity/get', { accountId }, '0']],
-    [CAPABILITIES.CORE, CAPABILITIES.SUBMISSION],
-  );
-  return res.methodResponses[0][1].list;
+const USING = [CAPABILITIES.CORE, CAPABILITIES.SUBMISSION];
+
+// A display name that carries an address or a line break produces a malformed
+// `From:` on every message sent with the identity (#672); reduce it to the
+// bare name before it reaches the server.
+function sanitizeIdentityPatch(identity: Partial<Identity>): Partial<Identity> {
+  if (typeof identity.name !== 'string') return identity;
+  return { ...identity, name: sanitizeDisplayName(identity.name) };
+}
+
+export async function getIdentities(accountIdOverride?: string): Promise<Identity[]> {
+  const accountId = accountIdOverride ?? jmapClient.accountId;
+  const res = await jmapClient.request([['Identity/get', { accountId }, '0']], USING);
+  const body = requireMethodResult<{ list?: Identity[] }>(res, '0', 'Identity/get');
+  return body.list ?? [];
 }
 
 export async function createIdentity(
   identity: Partial<Identity>,
 ): Promise<Identity> {
   const accountId = jmapClient.accountId;
+  const cid = 'new-identity';
   const res = await jmapClient.request(
-    [['Identity/set', {
-      accountId,
-      create: { 'new-identity': identity },
-    }, '0']],
-    [CAPABILITIES.CORE, CAPABILITIES.SUBMISSION],
+    [['Identity/set', { accountId, create: { [cid]: sanitizeIdentityPatch(identity) } }, '0']],
+    USING,
   );
-  return res.methodResponses[0][1].created['new-identity'];
+  const body = requireMethodResult(res, '0', 'Identity/set');
+  assertSetResult(body, [cid], 'identity');
+  const created = body.created?.[cid] as Partial<Identity> | undefined;
+  if (!created?.id) throw new Error('Identity/set returned no id');
+  // The create echo carries only server-set properties; re-read the full
+  // object so the caller gets name/email/signature back.
+  const all = await getIdentities();
+  return all.find((i) => i.id === created.id) ?? ({ ...identity, ...created } as Identity);
 }
 
 export async function updateIdentity(
@@ -30,16 +45,18 @@ export async function updateIdentity(
   changes: Partial<Identity>,
 ): Promise<void> {
   const accountId = jmapClient.accountId;
-  await jmapClient.request(
-    [['Identity/set', { accountId, update: { [id]: changes } }, '0']],
-    [CAPABILITIES.CORE, CAPABILITIES.SUBMISSION],
+  const res = await jmapClient.request(
+    [['Identity/set', { accountId, update: { [id]: sanitizeIdentityPatch(changes) } }, '0']],
+    USING,
   );
+  assertSetResult(requireMethodResult(res, '0', 'Identity/set'), [id], 'identity');
 }
 
 export async function deleteIdentity(id: string): Promise<void> {
   const accountId = jmapClient.accountId;
-  await jmapClient.request(
+  const res = await jmapClient.request(
     [['Identity/set', { accountId, destroy: [id] }, '0']],
-    [CAPABILITIES.CORE, CAPABILITIES.SUBMISSION],
+    USING,
   );
+  assertSetResult(requireMethodResult(res, '0', 'Identity/set'), [id], 'identity');
 }

src/api/jmap-client.ts

@@ -110,88 +110,9 @@ function fetchSessionDescribes(session: unknown): session is JMAPSession {
   return typeof s.apiUrl === 'string';
 }
 
-// ── Shared response helpers ──────────────────────────────
-// Most RN api functions used to index `methodResponses[0][1]` blindly, so a
-// method-level `['error', {...}]` reply produced a TypeError on reads and was
-// silently treated as success on writes. Route every response through these.
-
-type MethodResponse = [string, Record<string, any>, string];
-
-export class JMAPMethodError extends Error {
-  type: string;
-  constructor(type: string, description?: string) {
-    super(description || type || 'JMAP method error');
-    this.name = 'JMAPMethodError';
-    this.type = type;
-  }
-}
-
-/**
- * Pull the response body for `callId` (or the first response when omitted)
- * and throw a `JMAPMethodError` when the server answered with the error
- * envelope. Also verifies the method name when `expectMethod` is given.
- */
-export function requireMethodResult<T = Record<string, any>>(
-  res: JMAPResponseBody,
-  callId?: string,
-  expectMethod?: string,
-): T {
-  const responses = (res.methodResponses ?? []) as MethodResponse[];
-  const match = callId === undefined
-    ? responses[0]
-    : responses.find((r) => r[2] === callId);
-  if (!match) {
-    throw new JMAPMethodError('missingResponse', `Missing JMAP response${callId !== undefined ? ` for call ${callId}` : ''}`);
-  }
-  if (match[0] === 'error') {
-    const err = match[1] as { type?: string; description?: string };
-    throw new JMAPMethodError(err.type ?? 'serverFail', err.description);
-  }
-  if (expectMethod && match[0] !== expectMethod) {
-    throw new JMAPMethodError('unexpectedMethod', `Expected ${expectMethod}, got ${match[0]}`);
-  }
-  return match[1] as T;
-}
-
-/**
- * Throw when a /set response reports the given ids (or any id when `ids` is
- * omitted) under `notCreated` / `notUpdated` / `notDestroyed`.
- */
-export function assertSetResult(
-  body: Record<string, any>,
-  ids?: string[],
-  what = 'object',
-): void {
-  const buckets = ['notCreated', 'notUpdated', 'notDestroyed'] as const;
-  for (const bucket of buckets) {
-    const failures = body?.[bucket] as
-      | Record<string, { type?: string; description?: string; properties?: string[] }>
-      | undefined;
-    if (!failures) continue;
-    const entries = Object.entries(failures).filter(([id]) => !ids || ids.includes(id));
-    if (entries.length === 0) continue;
-    const [id, err] = entries[0];
-    const verb = bucket === 'notCreated' ? 'create' : bucket === 'notUpdated' ? 'update' : 'destroy';
-    const detail = [err?.type ?? 'unknown'];
-    if (err?.properties?.length) detail.push(`properties=[${err.properties.join(', ')}]`);
-    if (err?.description) detail.push(err.description);
-    const more = entries.length > 1 ? ` (+${entries.length - 1} more)` : '';
-    throw new JMAPMethodError(
-      err?.type ?? 'setError',
-      `Failed to ${verb} ${what} ${id}${more}: ${detail.join(' – ')}`,
-    );
-  }
-}
-
-/** Split `items` into consecutive batches of at most `size` entries. */
-export function batched<T>(items: T[], size: number): T[][] {
-  const step = Math.max(1, Math.floor(size));
-  const result: T[][] = [];
-  for (let i = 0; i < items.length; i += step) {
-    result.push(items.slice(i, i + step));
-  }
-  return result;
-}
+// Shared response helpers live in ./jmap-result (dependency-free) and are
+// re-exported here for convenience.
+export { JMAPMethodError, requireMethodResult, assertSetResult, batched } from './jmap-result';
 
 export class JMAPClient {
   private session: JMAPSession | null = null;

src/api/jmap-result.ts

@@ -0,0 +1,91 @@
+// Shared JMAP response helpers. Kept dependency-free (no client import) so
+// every `src/api/*.ts` module - and their tests, which mock `./jmap-client`
+// wholesale - can use them.
+//
+// Most RN api functions used to index `methodResponses[0][1]` blindly, so a
+// method-level `['error', {...}]` reply produced a TypeError on reads and was
+// silently treated as success on writes (the outbox then dropped the op).
+// Route every response through these instead.
+
+import type { JMAPResponseBody } from './types';
+
+type MethodResponse = [string, Record<string, any>, string];
+
+export class JMAPMethodError extends Error {
+  type: string;
+  constructor(type: string, description?: string) {
+    super(description || type || 'JMAP method error');
+    this.name = 'JMAPMethodError';
+    this.type = type;
+  }
+}
+
+/**
+ * Pull the response body for `callId` (or the first response when omitted)
+ * and throw a `JMAPMethodError` when the server answered with the error
+ * envelope. Also verifies the method name when `expectMethod` is given.
+ */
+export function requireMethodResult<T = Record<string, any>>(
+  res: JMAPResponseBody,
+  callId?: string,
+  expectMethod?: string,
+): T {
+  const responses = (res?.methodResponses ?? []) as MethodResponse[];
+  const match = callId === undefined
+    ? responses[0]
+    : responses.find((r) => r[2] === callId);
+  if (!match) {
+    throw new JMAPMethodError(
+      'missingResponse',
+      `Missing JMAP response${callId !== undefined ? ` for call ${callId}` : ''}`,
+    );
+  }
+  if (match[0] === 'error') {
+    const err = match[1] as { type?: string; description?: string };
+    throw new JMAPMethodError(err.type ?? 'serverFail', err.description);
+  }
+  if (expectMethod && match[0] !== expectMethod) {
+    throw new JMAPMethodError('unexpectedMethod', `Expected ${expectMethod}, got ${match[0]}`);
+  }
+  return match[1] as T;
+}
+
+/**
+ * Throw when a /set response reports the given ids (or any id when `ids` is
+ * omitted) under `notCreated` / `notUpdated` / `notDestroyed`.
+ */
+export function assertSetResult(
+  body: Record<string, any>,
+  ids?: string[],
+  what = 'object',
+): void {
+  const buckets = ['notCreated', 'notUpdated', 'notDestroyed'] as const;
+  for (const bucket of buckets) {
+    const failures = body?.[bucket] as
+      | Record<string, { type?: string; description?: string; properties?: string[] }>
+      | undefined;
+    if (!failures) continue;
+    const entries = Object.entries(failures).filter(([id]) => !ids || ids.includes(id));
+    if (entries.length === 0) continue;
+    const [id, err] = entries[0];
+    const verb = bucket === 'notCreated' ? 'create' : bucket === 'notUpdated' ? 'update' : 'destroy';
+    const detail = [err?.type ?? 'unknown'];
+    if (err?.properties?.length) detail.push(`properties=[${err.properties.join(', ')}]`);
+    if (err?.description) detail.push(err.description);
+    const more = entries.length > 1 ? ` (+${entries.length - 1} more)` : '';
+    throw new JMAPMethodError(
+      err?.type ?? 'setError',
+      `Failed to ${verb} ${what} ${id}${more}: ${detail.join(' – ')}`,
+    );
+  }
+}
+
+/** Split `items` into consecutive batches of at most `size` entries. */
+export function batched<T>(items: T[], size: number): T[][] {
+  const step = Math.max(1, Math.floor(size));
+  const result: T[][] = [];
+  for (let i = 0; i < items.length; i += step) {
+    result.push(items.slice(i, i + step));
+  }
+  return result;
+}

src/api/patch-pointer.ts

@@ -0,0 +1,35 @@
+/**
+ * Naming one member of an object in a JMAP PatchObject. Port of the webmail's
+ * lib/jmap/patch-pointer.ts.
+ *
+ * A PatchObject key is a JSON Pointer (RFC 8620 section 5.3), so a slash in the
+ * member's own name starts a new path segment unless it is escaped. That bites
+ * keywords: a nested tag is stored as `$label:work/clients`, and the unescaped
+ * pointer `keywords/$label:work/clients` asks for the `clients` member of the
+ * `$label:work` keyword - not a boolean, so the patch is rejected or ignored
+ * and the tag never lands.
+ */
+
+/** One JSON Pointer reference token: `~` -> `~0`, `/` -> `~1` (RFC 6901). */
+export function pointerToken(value: string): string {
+  return value.replace(/~/g, '~0').replace(/\//g, '~1');
+}
+
+/**
+ * The member name a reference token stands for: the inverse of
+ * `pointerToken`. `~1` is read before `~0`, per RFC 6901 - the other order
+ * would turn the escaped `~1` of `~01` back into a slash.
+ */
+export function pointerTokenValue(token: string): string {
+  return token.replace(/~1/g, '/').replace(/~0/g, '~');
+}
+
+/** The PatchObject key that sets or clears the keyword `keyword`. */
+export function keywordPointer(keyword: string): string {
+  return `keywords/${pointerToken(keyword)}`;
+}
+
+/** The PatchObject key that adds or removes membership of `mailboxId`. */
+export function mailboxPointer(mailboxId: string): string {
+  return `mailboxIds/${pointerToken(mailboxId)}`;
+}

src/api/types.ts

@@ -78,6 +78,14 @@ export interface Email {
   subject?: string;
   preview?: string;
   hasAttachment: boolean;
+  // RFC 8621 §4.1.2.3 - bare msg-ids without angle brackets. Needed for
+  // reply threading (In-Reply-To / References) and read-receipt dedupe.
+  messageId?: string[] | null;
+  inReplyTo?: string[] | null;
+  references?: string[] | null;
+  // Raw headers (RFC 8621 §4.1.3) when fetched with `headers`; the viewer
+  // derives SPF/DKIM/DMARC chips, List-Unsubscribe, MDN requests from them.
+  headers?: Array<{ name: string; value: string }>;
   textBody?: BodyPart[];
   htmlBody?: BodyPart[];
   bodyValues?: Record<string, { value: string; isEncodingProblem?: boolean }>;
@@ -357,6 +365,12 @@ export interface ContactCard {
   prodId?: string;
   created?: string;
   updated?: string;
+  // Client-only metadata for cards that live in a shared / group account.
+  // Never sent to the server (api/contacts.ts strips them). `id` is
+  // namespaced as `<accountId>:<originalId>` for shared cards.
+  accountId?: string;
+  accountName?: string;
+  isShared?: boolean;
 }
 
 export interface AddressBookRights {
@@ -374,6 +388,15 @@ export interface AddressBook {
   isSubscribed?: boolean;
   sortOrder?: number;
   myRights?: AddressBookRights;
+  // RFC 9610 share map: principal id → rights.
+  shareWith?: Record<string, AddressBookRights> | null;
+  // Client-only metadata for books that live in a shared / group account.
+  // `id` is namespaced as `<accountId>:<originalId>` for shared books so it
+  // cannot collide with the user's own "default" book.
+  originalId?: string;
+  accountId?: string;
+  accountName?: string;
+  isShared?: boolean;
 }
 
 export interface AddressBookWithCount extends AddressBook {
@@ -393,6 +416,8 @@ export interface Participant {
   roles?: Record<string, boolean>;
   participationStatus?: 'needs-action' | 'accepted' | 'declined' | 'tentative' | 'delegated' | string;
   participationComment?: string;
+  // 'server' asks Stalwart to deliver the iTIP messages for this participant.
+  scheduleAgent?: 'server' | 'client' | 'none' | string;
   scheduleStatus?: string[];
   expectReply?: boolean;
   description?: string;
@@ -602,6 +627,17 @@ export interface PushSubscription {
   verificationCode?: string | null;
   expires?: string | null;
   types?: string[] | null;
+  emailPush?: Record<string, EmailPushConfig> | null;
+}
+
+// draft-ietf-jmap-emailpush EmailPushConfig: the server evaluates `filter`
+// against every newly delivered message and only pushes when it matches, so a
+// client can keep spam (Junk) out of its push channel server-side. Keyed by
+// account id on PushSubscription.emailPush.
+export interface EmailPushConfig {
+  filter: Record<string, unknown> | null;
+  properties: string[];
+  urgency?: 'very-low' | 'low' | 'normal' | 'high';
 }
 
 // ─── Capability URNs ────────────────────────────────────
@@ -618,4 +654,5 @@ export const CAPABILITIES = {
   FILES: 'urn:ietf:params:jmap:filenode',
   PRINCIPALS: 'urn:ietf:params:jmap:principals',
   PRINCIPALS_OWNER: 'urn:ietf:params:jmap:principals:owner',
+  EMAIL_PUSH: 'urn:ietf:params:jmap:emailpush',
 } as const;