From 15740fa2868a2a5c0b644a6de8002edd25065937 Mon Sep 17 00:00:00 2001 From: lucamzanon Date: Thu, 1 Oct 2026 12:12:46 +0200 Subject: [PATCH 01/58] fix: keep the draft when a send is not confirmed When the response carries no EmailSubmission - a method error, or no answer for it at all - nothing confirms the message left, yet the send went on as if it had: the old draft was replaced and the composer closed. It now fails with "Send confirmation was not received" and keeps the draft, so the user can check Sent before sending again. An empty notCreated map is a success some servers spell out; it is no longer read as a failure. --- lib/__tests__/jmap-draft-lifecycle.test.ts | 18 +++++++++++++++++- lib/jmap/client.ts | 12 ++++++++++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/lib/__tests__/jmap-draft-lifecycle.test.ts b/lib/__tests__/jmap-draft-lifecycle.test.ts index a01fc2cae..f876368a0 100644 --- a/lib/__tests__/jmap-draft-lifecycle.test.ts +++ b/lib/__tests__/jmap-draft-lifecycle.test.ts @@ -29,7 +29,7 @@ interface CapturedRequest { methodCalls: JMAPMethodCall[] } * Email/set create come back as notCreated; `failSubmission` does the same * for EmailSubmission/set. */ -function mockFlow({ failCreate = false, failSubmission = false } = {}) { +function mockFlow({ failCreate = false, failSubmission = false, methodError = false, missingSubmission = false, emptyErrors = false } = {}) { const captured: CapturedRequest[] = []; vi.spyOn(globalThis, 'fetch').mockImplementation(async (_url, init) => { const body = JSON.parse((init as { body: string }).body) as CapturedRequest; @@ -56,11 +56,14 @@ function mockFlow({ failCreate = false, failSubmission = false } = {}) { result.created = { [key]: { id: 'email-new-1' } }; } } + if (emptyErrors) result.notCreated = {}; if (args.destroy) { result.destroyed = args.destroy; } methodResponses.push(['Email/set', result, callId]); } else if (method === 'EmailSubmission/set') { + if (missingSubmission) continue; + if (methodError) { methodResponses.push(['error', { type: 'serverFail', description: 'Google unavailable; draft retained' }, callId]); continue; } methodResponses.push(['EmailSubmission/set', failSubmission ? { notCreated: { '1': { type: 'forbiddenFrom', description: 'not allowed' } } } @@ -230,3 +233,16 @@ describe('draft replace lifecycle (#849)', () => { ]); }); }); + +describe('compose failure recovery', () => { + beforeEach(() => { vi.restoreAllMocks(); vi.spyOn(console, 'error').mockImplementation(() => {}); }); + it.each([{ methodError: true }, { missingSubmission: true }])('does not claim success or delete the old draft without confirmation: %j', async options => { + const client = createClient(); const captured = mockFlow(options); + await expect(client.sendEmail(['bob@example.com'], 'Subject', 'body', undefined, undefined, 'identity-1', 'user@example.com', 'old-draft')).rejects.toThrow(); + expect(emailSetCalls(captured).some(call => call.destroy)).toBe(false); + }); + it('accepts empty notCreated maps as success', async () => { + const client = createClient(); mockFlow({ emptyErrors: true }); + expect(await client.createDraft(['bob@example.com'], 'Subject', 'body')).toBe('email-new-1'); + }); +}); diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 1ce01fa06..f7a64de5c 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -3868,7 +3868,8 @@ export class JMAPClient implements IJMAPClient { if (response.methodResponses?.[0]?.[0] === "Email/set") { const result = response.methodResponses[0][1]; - if (result.notCreated) { + // An empty notCreated map is a success some servers spell out. + if (result.notCreated && Object.keys(result.notCreated).length) { const errors = result.notCreated; const firstError = Object.values(errors)[0] as { description?: string; type?: string }; console.error('Draft save error:', firstError); @@ -4134,7 +4135,7 @@ export class JMAPClient implements IJMAPClient { if (response.methodResponses) { for (const [methodName, result] of response.methodResponses) { - if (result.notCreated) { + if (result.notCreated && Object.keys(result.notCreated).length) { // Include method name + full error object so it's clear whether the // failure came from Email/set (draft create) or EmailSubmission/set // (actual send) and which JMAP error type/properties were returned. @@ -4188,6 +4189,13 @@ export class JMAPClient implements IJMAPClient { } } + // No EmailSubmission came back - a method error, or no answer for it at + // all. Nothing confirms the message left: report a failure and keep the + // old draft rather than replacing it as if the send had worked. + if (!emailSubmissionId) { + throw new Error('Send confirmation was not received. Check Sent before sending again. Your draft has been kept.'); + } + // With every recipient refused nothing was queued, yet the submission // exists and its onSuccessUpdateEmail filed the copy into Sent (#1123). // Drop that copy and keep the old draft: the send failed. From 3cfab390a0da4a6aa64c62f6dfbc70e7697190e8 Mon Sep 17 00:00:00 2001 From: lucamzanon Date: Thu, 1 Oct 2026 12:42:34 +0200 Subject: [PATCH 02/58] fix(push): recreate a browser subscription that was lost The background re-sync skipped any account whose browser subscription was gone (isWebPushEnabled needs a live one), which is exactly the state that needs repairing: the browser or the user cleared the endpoint while the opt-in and the server registration survived, and push stopped for good. It now runs whenever the user opted in and permission is granted, so the normal enable flow recreates the subscription. It still never prompts. A failed attempt no longer waits a day for the next one. --- lib/__tests__/web-push.test.ts | 35 ++++++++++++++++++++++++++++++++++ lib/web-push.ts | 11 +++++++++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/lib/__tests__/web-push.test.ts b/lib/__tests__/web-push.test.ts index a8b24b077..a6884a579 100644 --- a/lib/__tests__/web-push.test.ts +++ b/lib/__tests__/web-push.test.ts @@ -6,6 +6,7 @@ import { disableWebPush, enableWebPush, listPushDevices, + isWebPushEnabled, resetWebPushResyncState, resyncWebPush, revokePushDevice, @@ -274,6 +275,40 @@ describe('enableWebPush', () => { // repaired in the background on app start - nobody should have to find the // settings toggle to stop the spam pushes. describe('resyncWebPush', () => { + it('repairs a lost browser subscription for an account that previously opted in', async () => { + localStorage.setItem(DEVICE_KEY, THIS_DEVICE); + localStorage.setItem(SUB_KEY, 'push-old'); + const { registration } = installPushBrowser(); + registration.pushManager.getSubscription.mockResolvedValue(null as never); + const client = makeClient([sub('push-old', THIS_DEVICE)]); + const calls = installFetch({}); + + expect(await isWebPushEnabled(ACCOUNT_ID)).toBe(false); + expect(await resyncWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false })).toBe(true); + expect(registration.pushManager.subscribe).toHaveBeenCalledTimes(1); + expect(calls).toContainEqual({ url: `${RELAY}/api/push/register/web`, method: 'POST' }); + }); + + it.each(['default', 'denied'])('does not resubscribe or prompt with %s permission', async (permission) => { + localStorage.setItem(SUB_KEY, 'push-old'); + const { registration } = installPushBrowser(); + vi.stubGlobal('Notification', { permission, requestPermission: vi.fn() }); + const client = makeClient([sub('push-old', THIS_DEVICE)]); + expect(await resyncWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false })).toBe(false); + expect(registration.pushManager.subscribe).not.toHaveBeenCalled(); + expect(Notification.requestPermission).not.toHaveBeenCalled(); + }); + + it('allows retry after a transient relay failure', async () => { + localStorage.setItem(SUB_KEY, 'push-old'); + localStorage.setItem(DEVICE_KEY, THIS_DEVICE); + const client = makeClient([sub('push-old', THIS_DEVICE)]); + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('offline'))); + expect(await resyncWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false })).toBe(false); + installFetch({}); + expect(await resyncWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false })).toBe(true); + }); + it('re-syncs an enabled registration and installs the missing filter', async () => { localStorage.setItem(DEVICE_KEY, THIS_DEVICE); localStorage.setItem(SUB_KEY, 'push-old'); diff --git a/lib/web-push.ts b/lib/web-push.ts index c3f742025..90280a5cd 100644 --- a/lib/web-push.ts +++ b/lib/web-push.ts @@ -732,7 +732,7 @@ export interface ResyncWebPushParams { /** * Bring an already-enabled push registration up to date without any user * action: refresh its expiry and install/repair the delivery filter. Nothing - * here can prompt - it only runs when push is already on for the account - + * here can prompt - it requires a saved opt-in and granted permission - * and every failure is swallowed because the app must not care whether the * background touch-up worked. Returns true when a re-sync actually ran. */ @@ -747,7 +747,12 @@ export async function resyncWebPush(params: ResyncWebPushParams): Promise Date: Thu, 1 Oct 2026 12:43:40 +0200 Subject: [PATCH 03/58] fix(settings): check push again when the user comes back to the tab The notification settings read the push state once, when they opened. Push turned off meanwhile - in the browser's site settings, or from another tab - still showed as on until the page was reloaded. The state is now read again when the window regains focus or the tab becomes visible, unless an enable or disable is in progress or has just failed. A failed check reads as off instead of leaving the panel unchanged. --- .../__tests__/notification-settings.test.tsx | 14 ++++++++ components/settings/notification-settings.tsx | 32 +++++++++++++++---- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/components/settings/__tests__/notification-settings.test.tsx b/components/settings/__tests__/notification-settings.test.tsx index a9db650ba..b6047870d 100644 --- a/components/settings/__tests__/notification-settings.test.tsx +++ b/components/settings/__tests__/notification-settings.test.tsx @@ -103,6 +103,20 @@ describe('NotificationSettings - pushNotifyInboxOnly', () => { ); }); + it('checks push again when the user comes back to the tab', async () => { + Object.defineProperty(document, 'visibilityState', { value: 'visible', configurable: true }); + const { findByText } = render(); + await findByText('email.inbox_only'); + const check = webPush.isWebPushEnabled as ReturnType; + await waitFor(() => expect(check).toHaveBeenCalled()); + const before = check.mock.calls.length; + + // Push may have been turned off elsewhere (site settings, another tab) meanwhile. + await act(async () => { window.dispatchEvent(new Event('focus')); }); + + await waitFor(() => expect(check.mock.calls.length).toBeGreaterThan(before)); + }); + it('does not re-sync when push is disabled on this device', async () => { (webPush.isWebPushEnabled as ReturnType).mockResolvedValueOnce(false); render(); diff --git a/components/settings/notification-settings.tsx b/components/settings/notification-settings.tsx index a7a63ab82..2ab9c809b 100644 --- a/components/settings/notification-settings.tsx +++ b/components/settings/notification-settings.tsx @@ -80,16 +80,37 @@ export function NotificationSettings() { const activeRelayLabel = relayOptions.find((option) => option.url === activeRelayUrl)?.label ?? activeRelayUrl; + const busy = pushStatus.kind === 'busy'; + useEffect(() => { if (!supported) return; if (!client) return; const accountId = client.getAccountId(); if (!accountId) return; - void (async () => { - const enabled = await isWebPushEnabled(accountId); - setPushStatus(enabled ? { kind: 'enabled' } : { kind: 'idle' }); - })(); - }, [supported, client]); + let cancelled = false; + const refreshStatus = async () => { + try { + const enabled = await isWebPushEnabled(accountId); + if (!cancelled) { + setPushStatus(enabled ? { kind: 'enabled' } : { kind: 'idle' }); + } + } catch { + if (!cancelled) setPushStatus({ kind: 'idle' }); + } + }; + const onVisible = () => { + if (document.visibilityState === 'visible') void refreshStatus(); + }; + if (pushStatus.kind === 'busy' || pushStatus.kind === 'error') return; + void refreshStatus(); + window.addEventListener('focus', onVisible); + document.addEventListener('visibilitychange', onVisible); + return () => { + cancelled = true; + window.removeEventListener('focus', onVisible); + document.removeEventListener('visibilitychange', onVisible); + }; + }, [supported, client, pushStatus.kind]); // The device list is worth loading even when push is off on this browser - // revoking a stale registration left on another device is exactly what you @@ -108,7 +129,6 @@ export function NotificationSettings() { void refreshDevices(); }, [refreshDevices]); - const busy = pushStatus.kind === 'busy'; const pushEnabled = pushStatus.kind === 'enabled'; // In the static Lite build push is off by design (no service worker, see // components/service-worker-registration.tsx), not a browser limitation. From 4bc5d4888801376874fe5bc2ffc2e30e5ff02e8b Mon Sep 17 00:00:00 2001 From: lucamzanon Date: Thu, 1 Oct 2026 12:44:14 +0200 Subject: [PATCH 04/58] 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][]; }; + // 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