fix: never destroy mail from the notification Delete action #68

With no Trash mailbox, or when Mailbox/get came back as an error, the
Delete action fell through to destroying the email. A shared account
without its own Trash lost mail permanently on one tap.

- only move to Trash; do nothing when there is no Trash or the lookup
  failed
- check that Email/set actually updated the email
- show "(No Subject)" in the collapsed text instead of a blank line
- require unlock for Delete (Android 12+)

android/app/src/main/java/com/anonymous/bulwarkmobile/BulwarkFcmModule.kt

@@ -152,7 +152,9 @@ class BulwarkFcmModule(reactContext: ReactApplicationContext)
             .setSmallIcon(R.drawable.ic_notification)
             .setLargeIcon(largeIcon)
             .setContentTitle(title)
-            .setContentText(subject ?: preview ?: body)
+            // body is the subject with the "(No subject)" fallback; an empty
+            // subject is "" here, not null, so `subject ?: ...` showed a blank line.
+            .setContentText(body)
             .setStyle(
                 NotificationCompat.BigTextStyle()
                     .bigText(if (bigText.isNotBlank()) bigText else body)
@@ -206,7 +208,10 @@ class BulwarkFcmModule(reactContext: ReactApplicationContext)
                 PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE,
             )
             builder.addAction(
-                NotificationCompat.Action.Builder(0, deleteLabel, deletePending).build()
+                NotificationCompat.Action.Builder(0, deleteLabel, deletePending)
+                    // Android 12+: unlock first, so a locked phone can't delete mail.
+                    .setAuthenticationRequired(true)
+                    .build()
             )
 
             // Action 3: Reply

src/lib/__tests__/push-background-task.test.ts

@@ -287,6 +287,32 @@ describe('pushBackgroundTask notifications', () => {
       ['Email/set', { accountId: 'jmap-primary', update: { m1: { mailboxIds: { 'trash-box': true } } } }, '0'],
     ]);
   });
+
+  it.each([
+    ['has no trash mailbox', ['Mailbox/get', { list: [{ id: 'inbox', role: 'inbox' }] }, '0']],
+    ['cannot list mailboxes', ['error', { type: 'accountNotFound' }, '0']],
+  ])('never destroys from a notification when the account %s', async (_label, mailboxResponse) => {
+    const postCalls: any[] = [];
+    (secureFetch as ReturnType<typeof vi.fn>).mockImplementation(async (url: string, opts?: any) => {
+      if (opts?.body) postCalls.push(JSON.parse(opts.body));
+      return {
+        ok: true,
+        json: async () => (url.endsWith('/.well-known/jmap')
+          ? {
+            apiUrl: 'https://mail.example.com/jmap/',
+            primaryAccounts: { 'urn:ietf:params:jmap:mail': 'jmap-primary' },
+            accounts: { 'jmap-primary': {} },
+          }
+          : { methodResponses: [mailboxResponse] }),
+      };
+    });
+
+    await handleNotificationAction({ action: 'delete', emailId: 'm1', accountId: LOCAL, jmapAccountId: 'shared' });
+
+    expect(postCalls).toHaveLength(1);
+    expect(postCalls[0].methodCalls[0][0]).toBe('Mailbox/get');
+    expect(postCalls[0].methodCalls[0][1].accountId).toBe('shared');
+  });
 });
 
 describe('pushes for device sync (#34)', () => {

src/lib/push-background-task.ts

@@ -593,7 +593,7 @@ export async function handleNotificationAction(data: unknown): Promise<void> {
     const emailAccountId = jmapAccountId ?? session.jmapAccountId;
 
     if (action === 'markRead') {
-      await jmapPost(session, [
+      const res = await jmapPost(session, [
         [
           'Email/set',
           {
@@ -605,41 +605,42 @@ export async function handleNotificationAction(data: unknown): Promise<void> {
           '0',
         ],
       ]);
+      assertEmailUpdated(res, emailId);
     } else if (action === 'delete') {
       const mbRes = await jmapPost(session, [
         ['Mailbox/get', { accountId: emailAccountId, properties: ['id', 'role'] }, '0'],
       ]);
-      const [, mbBody] = mbRes[0] ?? [];
-      const mailboxes = ((mbBody?.list as Mailbox[]) ?? []);
+      const [mbName, mbBody] = mbRes[0] ?? [];
+      const mailboxes = mbName === 'Mailbox/get' ? ((mbBody?.list as Mailbox[]) ?? []) : [];
       const trash = mailboxes.find((m) => m.role === 'trash');
-
-      if (trash) {
-        await jmapPost(session, [
-          [
-            'Email/set',
-            {
-              accountId: emailAccountId,
-              update: {
-                [emailId]: { mailboxIds: { [trash.id]: true } },
-              },
-            },
-            '0',
-          ],
-        ]);
-      } else {
-        await jmapPost(session, [
-          [
-            'Email/set',
-            {
-              accountId: emailAccountId,
-              destroy: [emailId],
-            },
-            '0',
-          ],
-        ]);
+      // Never destroy from a notification: without a Trash (a shared
+      // account, or a Mailbox/get that failed) the message stays put.
+      if (!trash) {
+        console.warn('[push] no trash mailbox for notification delete', emailAccountId);
+        return;
       }
+      const res = await jmapPost(session, [
+        [
+          'Email/set',
+          {
+            accountId: emailAccountId,
+            update: {
+              [emailId]: { mailboxIds: { [trash.id]: true } },
+            },
+          },
+          '0',
+        ],
+      ]);
+      assertEmailUpdated(res, emailId);
     }
   } catch (err) {
     console.warn('[push] notification action failed', action, emailId, err);
   }
 }
+
+function assertEmailUpdated(responses: Array<[string, Record<string, any>, string]>, emailId: string): void {
+  const [name, body] = responses[0] ?? [];
+  if (name !== 'Email/set' || !body?.updated || !(emailId in body.updated)) {
+    throw new Error(`Email/set did not update ${emailId}: ${JSON.stringify(body?.notUpdated?.[emailId] ?? body ?? null)}`);
+  }
+}