From ccaa24ecd3e526a9a194c144380a9d1eee590d2a Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 24 Sep 2026 18:46:17 +0200 Subject: [PATCH] fix: stop re-patching the push filter on every launch `PushSubscription/get` ran with the default properties, which leave out `emailPush`, so the stored filter never compared equal to the wanted one and every launch sent it again. The calls also never named the emailpush capability in `using`. Ask for `emailPush` explicitly and send `urn:ietf:params:jmap:emailpush` with every call that reads or writes it, as the webmail's `lib/jmap/client.ts` does. Servers without the capability keep the default properties and never see the property, so the filter is only patched when it differs from what the server holds. --- src/api/__tests__/push.test.ts | 74 +++++++++++++++++++- src/api/push.ts | 37 ++++++++-- src/lib/__tests__/push-notifications.test.ts | 53 ++++++++++++++ 3 files changed, 157 insertions(+), 7 deletions(-) diff --git a/src/api/__tests__/push.test.ts b/src/api/__tests__/push.test.ts index 3ed508df..a19e1dfe 100644 --- a/src/api/__tests__/push.test.ts +++ b/src/api/__tests__/push.test.ts @@ -8,13 +8,21 @@ vi.mock('../jmap-client', () => ({ }, authHeader: 'Basic dXNlcjpwYXNz', request: vi.fn(), + hasCapability: vi.fn(() => false), }, })); import { jmapClient } from '../jmap-client'; -import { createPushSubscription, startPolling, updatePushSubscription } from '../push'; +import { + createPushSubscription, + listPushSubscriptions, + startPolling, + updatePushSubscription, +} from '../push'; const mockRequest = jmapClient.request as ReturnType; +const mockHasCapability = jmapClient.hasCapability as ReturnType; +const EMAIL_PUSH = 'urn:ietf:params:jmap:emailpush'; describe('push operations', () => { beforeEach(() => { @@ -26,6 +34,70 @@ describe('push operations', () => { vi.useRealTimers(); }); + describe('emailPush read-back', () => { + const FILTER = { + filter: { operator: 'AND', conditions: [{ notKeyword: '$junk' }] }, + properties: ['id', 'threadId'], + urgency: 'high' as const, + }; + + beforeEach(() => { + mockHasCapability.mockImplementation((urn: string) => urn === EMAIL_PUSH); + }); + + afterEach(() => { + mockHasCapability.mockImplementation(() => false); + }); + + it('asks for emailPush explicitly under the emailpush capability', async () => { + mockRequest.mockResolvedValue({ + methodResponses: [['PushSubscription/get', { list: [{ id: 's1', emailPush: { a: FILTER } }] }, '0']], + }); + const list = await listPushSubscriptions(); + const [calls, using] = mockRequest.mock.calls[0]; + expect(calls[0][1].properties).toContain('emailPush'); + expect(using).toEqual(['urn:ietf:params:jmap:core', EMAIL_PUSH]); + expect(list[0].emailPush).toEqual({ a: FILTER }); + }); + + it('keeps the default properties on servers without emailpush', async () => { + mockHasCapability.mockImplementation(() => false); + mockRequest.mockResolvedValue({ methodResponses: [['PushSubscription/get', { list: [] }, '0']] }); + await listPushSubscriptions(); + const [calls, using] = mockRequest.mock.calls[0]; + expect(calls[0][1]).toEqual({ ids: null }); + expect(using).toEqual(['urn:ietf:params:jmap:core']); + }); + + it('sends the capability with an emailPush create and update', async () => { + mockRequest.mockResolvedValueOnce({ + methodResponses: [['PushSubscription/set', { created: { new: { id: 's1' } } }, '0']], + }); + await createPushSubscription({ + deviceClientId: 'd', url: 'https://relay/x', types: ['EmailDelivery'], emailPush: { a: FILTER }, + }); + expect(mockRequest.mock.calls[0][1]).toEqual(['urn:ietf:params:jmap:core', EMAIL_PUSH]); + + mockRequest.mockResolvedValueOnce({ + methodResponses: [['PushSubscription/set', { updated: { s1: null } }, '0']], + }); + await updatePushSubscription('s1', { expires: '2026-10-01T00:00:00Z' }); + // An update that leaves the filter alone doesn't need the capability. + expect(mockRequest.mock.calls[1][1]).toEqual(['urn:ietf:params:jmap:core']); + }); + + it('drops emailPush from writes on servers without the capability', async () => { + mockHasCapability.mockImplementation(() => false); + mockRequest.mockResolvedValue({ + methodResponses: [['PushSubscription/set', { updated: { s1: null } }, '0']], + }); + await updatePushSubscription('s1', { expires: '2026-10-01T00:00:00Z', emailPush: { a: FILTER } }); + const [calls, using] = mockRequest.mock.calls[0]; + expect(calls[0][1].update.s1).toEqual({ expires: '2026-10-01T00:00:00Z' }); + expect(using).toEqual(['urn:ietf:params:jmap:core']); + }); + }); + describe('PushSubscription/set refusals', () => { const forbidden = { type: 'forbidden', diff --git a/src/api/push.ts b/src/api/push.ts index 400995a7..3ed43c42 100644 --- a/src/api/push.ts +++ b/src/api/push.ts @@ -6,11 +6,32 @@ import type { EmailPushConfig, PushSubscription, StateChange } from './types'; export type StateChangeHandler = (change: StateChange) => void; // ─── PushSubscription (RFC 8620 §7.2) ─────────────────── +// Mirrors the webmail's lib/jmap/client.ts push calls - keep them in sync. + +// draft-ietf-jmap-emailpush (Stalwart >= 0.16.16) lets a subscription carry a +// per-account `emailPush` delivery filter. Older servers reject the capability +// in `using` and the property outright, so every call that touches it gates +// on the session advertising the capability. +function hasEmailPushCapability(): boolean { + return jmapClient.hasCapability(CAPABILITIES.EMAIL_PUSH); +} + +function pushUsing(withEmailPush: boolean): string[] { + return withEmailPush ? [CAPABILITIES.CORE, CAPABILITIES.EMAIL_PUSH] : [CAPABILITIES.CORE]; +} export async function listPushSubscriptions(): Promise { + const withEmailPush = hasEmailPushCapability(); + // `emailPush` is not in the server's default property set, so ask for it + // explicitly - without it the stored filter never compares equal to the + // wanted one and every launch re-patches it. + const args: Record = { ids: null }; + if (withEmailPush) { + args.properties = ['id', 'deviceClientId', 'verificationCode', 'expires', 'types', 'emailPush']; + } const res = await jmapClient.request( - [['PushSubscription/get', { ids: null }, '0']], - [CAPABILITIES.CORE], + [['PushSubscription/get', args, '0']], + pushUsing(withEmailPush), ); const [, body] = res.methodResponses[0] ?? []; return (body?.list as PushSubscription[]) ?? []; @@ -40,7 +61,8 @@ export async function createPushSubscription(params: { types: params.types, }; if (params.expires) created.expires = params.expires; - if (params.emailPush) created.emailPush = params.emailPush; + const withEmailPush = !!params.emailPush && hasEmailPushCapability(); + if (withEmailPush) created.emailPush = params.emailPush; const res = await jmapClient.request( [ @@ -50,7 +72,7 @@ export async function createPushSubscription(params: { '0', ], ], - [CAPABILITIES.CORE], + pushUsing(withEmailPush), ); const [, body] = res.methodResponses[0] ?? []; assertSetResult(body, ['new'], 'push subscription'); @@ -72,15 +94,18 @@ export async function updatePushSubscription( id: string, patch: { expires?: string; types?: string[]; emailPush?: Record }, ): Promise { + const withEmailPush = patch.emailPush !== undefined && hasEmailPushCapability(); + const update: Record = { ...patch }; + if (!withEmailPush) delete update.emailPush; const res = await jmapClient.request( [ [ 'PushSubscription/set', - { update: { [id]: patch } }, + { update: { [id]: update } }, '0', ], ], - [CAPABILITIES.CORE], + pushUsing(withEmailPush), ); const [, body] = res.methodResponses[0] ?? []; assertSetResult(body, [id], 'push subscription'); diff --git a/src/lib/__tests__/push-notifications.test.ts b/src/lib/__tests__/push-notifications.test.ts index 4625dda6..871cf266 100644 --- a/src/lib/__tests__/push-notifications.test.ts +++ b/src/lib/__tests__/push-notifications.test.ts @@ -253,6 +253,59 @@ describe('setupPushNotifications subscription shape', () => { expect(createMock).not.toHaveBeenCalled(); }); + it('leaves an emailPush filter alone when the server already holds it', async () => { + (jmapClient as { currentSession: unknown }).currentSession = { + capabilities: { 'urn:ietf:params:jmap:emailpush': {} }, + }; + await AsyncStorage.setItem(SUB_KEY, 'existing'); + listMock.mockResolvedValue([ + { + id: 'existing', + deviceClientId: OUR_DCID, + expires: new Date(Date.now() + 80 * 86400000).toISOString(), + types: ['EmailDelivery'], + // As the server echoes it back: same content, its own key order. + emailPush: { + 'jmap-primary': { + urgency: 'high', + properties: ['id', 'threadId'], + filter: { conditions: [{ notKeyword: '$junk' }, { inMailboxOtherThan: ['junk'] }], operator: 'AND' }, + }, + }, + }, + ]); + await setupPushNotifications({ relayBaseUrl: RELAY }); + expect(updateMock).not.toHaveBeenCalled(); + expect(createMock).not.toHaveBeenCalled(); + }); + + it('re-patches an emailPush filter that has drifted', async () => { + (jmapClient as { currentSession: unknown }).currentSession = { + capabilities: { 'urn:ietf:params:jmap:emailpush': {} }, + }; + await AsyncStorage.setItem(SUB_KEY, 'existing'); + listMock.mockResolvedValue([ + { + id: 'existing', + deviceClientId: OUR_DCID, + expires: new Date(Date.now() + 80 * 86400000).toISOString(), + types: ['EmailDelivery'], + emailPush: { + 'jmap-primary': { + filter: { operator: 'AND', conditions: [{ notKeyword: '$junk' }, { inMailboxOtherThan: ['old-junk'] }] }, + properties: ['id', 'threadId'], + urgency: 'high', + }, + }, + }, + ]); + await setupPushNotifications({ relayBaseUrl: RELAY }); + expect(updateMock).toHaveBeenCalledTimes(1); + expect(updateMock.mock.calls[0][1].emailPush['jmap-primary'].filter.conditions[1]).toEqual({ + inMailboxOtherThan: ['junk'], + }); + }); + it('forceRecreate destroys the recorded subscription and creates a new one', async () => { await AsyncStorage.setItem(SUB_KEY, 'existing'); listMock.mockResolvedValue([