From 4bc5d4888801376874fe5bc2ffc2e30e5ff02e8b Mon Sep 17 00:00:00 2001
From: lucamzanon <luca.m.zanon@gmail.com>
Date: Thu, 1 Oct 2026 12:44:14 +0200
Subject: [PATCH] fix(push): keep notification previews truthful when a lookup
 fails

A JMAP method error comes back with HTTP 200. The preview route read a
failed Mailbox/query or Email/query as an empty Inbox and answered
"nothing unread", so the service worker dropped the push in silence.
It now answers 502, and the worker shows its generic notification.

When the server names the message that arrived and it cannot be read -
deleted since, or filed where this login cannot see it - the route no
longer substitutes the newest unread message in the Inbox, which was
some other mail announced as if it had just arrived. It names no
message; the unread total still comes back for the generic toast.
---
 app/api/push/preview/route.ts            | 30 ++++++++-
 lib/__tests__/push-preview-route.test.ts | 77 +++++++++++++++++++++++-
 2 files changed, 103 insertions(+), 4 deletions(-)

diff --git a/app/api/push/preview/route.ts b/app/api/push/preview/route.ts
index f58e2243e..8be988743 100644
--- a/app/api/push/preview/route.ts
+++ b/app/api/push/preview/route.ts
@@ -161,10 +161,16 @@ export async function GET(request: NextRequest) {
     };
 
     const inboxBody = inboxData.methodResponses.find(
-      ([method]) => method === 'Mailbox/query',
+      ([method, , callId]) => method === 'Mailbox/query' && callId === 'mb',
     )?.[1] as { ids?: string[] } | undefined;
 
-    const inboxId = inboxBody?.ids?.[0];
+    // A method-level JMAP error still has HTTP 200. Do not turn it into
+    // an empty Inbox: the service worker would silently discard the push.
+    if (!Array.isArray(inboxBody?.ids)) {
+      return NextResponse.json({ error: 'JMAP mailbox query failed' }, { status: 502 });
+    }
+
+    const inboxId = inboxBody.ids[0];
     const emailProperties = ['id', 'threadId', 'from', 'subject', 'preview', 'receivedAt'];
 
     if (!inboxId && !requestedEmailId) {
@@ -239,6 +245,19 @@ export async function GET(request: NextRequest) {
       methodResponses: [string, Record<string, unknown>, string][];
     };
 
+    // Missing/failed method responses are preview failures, not proof that
+    // there is no unread mail. A non-2xx response lets the SW show its fallback.
+    for (const call of methodCalls as [string, unknown, string][]) {
+      const response = data.methodResponses.find(([method, , callId]) =>
+        method === call[0] && callId === call[2]);
+      const result = response?.[1];
+      if (!result || (call[0] === 'Email/query'
+        ? !Array.isArray(result.ids) || typeof result.total !== 'number'
+        : !Array.isArray(result.list))) {
+        return NextResponse.json({ error: 'JMAP email query failed' }, { status: 502 });
+      }
+    }
+
     type EmailLite = {
       id: string;
       threadId: string;
@@ -270,6 +289,13 @@ export async function GET(request: NextRequest) {
         unreadTotal = Math.max(unreadTotal, 1);
       }
       email = delivered;
+    } else if (requestedEmailId) {
+      // The server named the message that arrived and it could not be read -
+      // deleted since, or filed where this login cannot see it. "Newest unread
+      // in the Inbox" is then some other message, and announcing it would be
+      // confidently wrong. Name no message: the unread total still comes back,
+      // so the SW posts its generic toast instead of staying silent.
+      email = null;
     }
 
     // Some servers build the preview from HTML without skipping <style>; a
diff --git a/lib/__tests__/push-preview-route.test.ts b/lib/__tests__/push-preview-route.test.ts
index 3664c49f5..fb465f83a 100644
--- a/lib/__tests__/push-preview-route.test.ts
+++ b/lib/__tests__/push-preview-route.test.ts
@@ -65,10 +65,10 @@ function mockJmap() {
   });
 }
 
-async function callRoute(accountId: string) {
+async function callRoute(accountId: string, emailId: string | null = null) {
   const { GET } = await import('@/app/api/push/preview/route');
   const request = {
-    nextUrl: { searchParams: { get: (k: string) => (k === 'accountId' ? accountId : null) } },
+    nextUrl: { searchParams: { get: (k: string) => (k === 'accountId' ? accountId : k === 'emailId' ? emailId : null) } },
   };
   const res = (await GET(request as unknown as Parameters<typeof GET>[0])) as unknown as {
     status: number;
@@ -116,3 +116,76 @@ describe('push preview route account resolution', () => {
     expect(status).toBe(401);
   });
 });
+
+
+describe('push preview JMAP failures', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mockJmap();
+  });
+
+  it.each([
+    ['error', { type: 'unsupportedFilter' }, 'mb'],
+    ['Mailbox/query', {}, 'mb'],
+  ])('does not silence a push when mailbox lookup fails (%s)', async (method, body, id) => {
+    fetchJmapServer.mockReset()
+      .mockResolvedValueOnce(jsonResponse(SESSION))
+      .mockResolvedValueOnce(jsonResponse({ methodResponses: [[method, body, id]] }));
+    expect((await callRoute('a')).status).toBe(502);
+  });
+
+  it.each([
+    [['error', { type: 'serverFail' }, 'eq'], ['error', { type: 'resultReference' }, 'eg']],
+    [['Email/query', { ids: ['e1'], total: 1 }, 'eq'], ['error', { type: 'serverFail' }, 'eg']],
+    [['Email/query', { ids: [] }, 'eq'], ['Email/get', { list: [] }, 'eg']],
+  ])('does not report zero unread on an email method failure', async (query, get) => {
+    fetchJmapServer.mockReset()
+      .mockResolvedValueOnce(jsonResponse(SESSION))
+      .mockResolvedValueOnce(jsonResponse({ methodResponses: [['Mailbox/query', { ids: ['inbox'] }, 'mb']] }))
+      .mockResolvedValueOnce(jsonResponse({ methodResponses: [query, get] }));
+    expect((await callRoute('a')).status).toBe(502);
+  });
+
+  it('still reports genuinely empty unread results', async () => {
+    fetchJmapServer.mockReset()
+      .mockResolvedValueOnce(jsonResponse(SESSION))
+      .mockResolvedValueOnce(jsonResponse({ methodResponses: [['Mailbox/query', { ids: ['inbox'] }, 'mb']] }))
+      .mockResolvedValueOnce(jsonResponse({ methodResponses: [
+        ['Email/query', { ids: [], total: 0 }, 'eq'],
+        ['Email/get', { list: [] }, 'eg'],
+      ] }));
+    expect(await callRoute('a')).toEqual({ status: 200, body: { email: null, unreadTotal: 0, slot: 0 } });
+  });
+});
+
+
+it('previews a delivered message outside an empty Inbox', async () => {
+  fetchJmapServer.mockReset()
+    .mockResolvedValueOnce(jsonResponse(SESSION))
+    .mockResolvedValueOnce(jsonResponse({ methodResponses: [['Mailbox/query', { ids: ['inbox'] }, 'mb']] }))
+    .mockResolvedValueOnce(jsonResponse({ methodResponses: [
+      ['Email/query', { ids: [], total: 0 }, 'eq'],
+      ['Email/get', { list: [] }, 'eg'],
+      ['Email/get', { list: [{ id: 'filed', threadId: 't2' }] }, 'delivered'],
+    ] }));
+  expect(await callRoute('a', 'filed')).toEqual({
+    status: 200, body: { email: { id: 'filed', threadId: 't2' }, unreadTotal: 1, slot: 0 },
+  });
+});
+
+it('names no message when the delivered one cannot be read, rather than another', async () => {
+  // The push named a message - deleted since, or filed where this login
+  // cannot see it - and the Inbox happens to hold an older unread one. That
+  // older message is not what arrived.
+  fetchJmapServer.mockReset()
+    .mockResolvedValueOnce(jsonResponse(SESSION))
+    .mockResolvedValueOnce(jsonResponse({ methodResponses: [['Mailbox/query', { ids: ['inbox'] }, 'mb']] }))
+    .mockResolvedValueOnce(jsonResponse({ methodResponses: [
+      ['Email/query', { ids: ['stale'], total: 3 }, 'eq'],
+      ['Email/get', { list: [{ id: 'stale', threadId: 't9' }] }, 'eg'],
+      ['Email/get', { list: [] }, 'delivered'],
+    ] }));
+  expect(await callRoute('a', 'gone')).toEqual({
+    status: 200, body: { email: null, unreadTotal: 3, slot: 0 },
+  });
+});