From fcc4e661dfe796677fd86b0b7147bdf0d7e96ff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Mon, 27 Jul 2026 13:48:35 +0200 Subject: [PATCH] fix(notifications): scope client-managed suppression to the APNs leg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `medication.clientManaged` and `measurementReminder.clientManaged` mean the iOS client schedules the reminder locally, so the server should not push it. That is what the OpenAPI description, the settings copy and the iOS contract all say. What the crons did was skip the dispatch entirely, so a user who flipped the switch also lost Telegram, ntfy, webhook, email and Web Push — channels they may well read on a desktop while the phone holding the local schedule sits in a drawer. Move the gate into the dispatcher's channel loop, where it applies to APNs and nothing else. The skip is not counted as an attempt, so `dispatched` keeps meaning "something reached the user", and it stays countable: the same wide-event keys the crons emitted are emitted here, plus a `skipped` row in the push-attempt ledger. The Vorsorge path is the sharper half. It suppressed every channel and then advanced `nextDueAt`, which dropped the reminder out of the preventive-care card and out of the daily digest: a reminder that never reached anyone was recorded as handled. The advance now happens only where it always belonged, after a dispatch that a channel actually delivered. When nothing delivers, the reminder stays overdue and visible. --- .../__tests__/measurement-reminder.test.ts | 53 ++++- src/lib/jobs/measurement-reminder.ts | 33 +-- src/lib/jobs/reminder/mood-cycle-checks.ts | 1 - .../__tests__/client-managed-apns.test.ts | 206 ++++++++++++++++++ src/lib/notifications/client-managed-apns.ts | 79 +++++++ src/lib/notifications/dispatcher.ts | 56 ++++- 6 files changed, 398 insertions(+), 30 deletions(-) create mode 100644 src/lib/notifications/__tests__/client-managed-apns.test.ts create mode 100644 src/lib/notifications/client-managed-apns.ts diff --git a/src/lib/jobs/__tests__/measurement-reminder.test.ts b/src/lib/jobs/__tests__/measurement-reminder.test.ts index 1e73f40c3..e2936555f 100644 --- a/src/lib/jobs/__tests__/measurement-reminder.test.ts +++ b/src/lib/jobs/__tests__/measurement-reminder.test.ts @@ -11,7 +11,8 @@ * auto-resolve. * - Ledger-free dedup: a successful dispatch advances nextDueAt past * now so the same due cycle never re-fires. - * - clientManaged suppression skips the APNs send but still advances. + * - clientManaged suppresses the APNs leg only: the tick still + * dispatches, and a reminder nothing delivered stays overdue. * * The Prisma surface is stubbed manually to avoid a testcontainer boot. */ @@ -291,8 +292,12 @@ describe("runMeasurementReminderTick", () => { expect(prisma.measurement.findFirst).not.toHaveBeenCalled(); }); - it("suppresses the push under clientManaged but still advances nextDueAt", async () => { - const { prisma, updates } = makePrisma({ + it("still dispatches under clientManaged — the suppression is APNs-only", async () => { + // `measurementReminder.clientManaged` suppresses the server's APNs + // send, which the dispatcher decides per channel. The tick must not + // pre-empt that decision: a user whose phone owns the local banner + // still reads Telegram or email on their desktop. + const { prisma } = makePrisma({ reminders: [ reminder({ measurementType: null, @@ -314,10 +319,44 @@ describe("runMeasurementReminderTick", () => { { dispatch }, ); - expect(summary.skippedClientManaged).toBe(1); - expect(dispatch).not.toHaveBeenCalled(); - expect(updates).toHaveLength(1); - expect(updates[0].data.nextDueAt).toBeInstanceOf(Date); + expect(dispatch).toHaveBeenCalledTimes(1); + expect(summary.dispatched).toBe(1); + }); + + it("leaves the reminder overdue when the client-managed skip was the only channel", async () => { + // Nothing delivered, so nothing may be recorded as handled: advancing + // `nextDueAt` here dropped the reminder out of the Vorsorge card and + // out of the daily digest, and a preventive-care reminder that never + // fired was filed as done. + const { prisma, updates } = makePrisma({ + reminders: [ + reminder({ + measurementType: null, + user: { + id: "u1", + timezone: TZ, + locale: "de", + notificationPrefs: { measurementReminder: { clientManaged: true } }, + }, + }), + ], + measurementMatch: null, + }); + const dispatch = vi.fn(async () => ({ + dispatched: false, + channelsAttempted: 0, + channelsSucceeded: 0, + })); + + const summary = await runMeasurementReminderTick( + prisma as never, + NINE_LOCAL, + { dispatch }, + ); + + expect(summary.skippedNoChannel).toBe(1); + expect(summary.dispatched).toBe(0); + expect(updates).toHaveLength(0); }); it("bounds the scan with a nextDueAt filter so the index is used", async () => { diff --git a/src/lib/jobs/measurement-reminder.ts b/src/lib/jobs/measurement-reminder.ts index 6041c8d05..963459283 100644 --- a/src/lib/jobs/measurement-reminder.ts +++ b/src/lib/jobs/measurement-reminder.ts @@ -23,9 +23,9 @@ * query in the cron, NOT a hook on the hot iOS batch-ingest path. * Free-text reminders (no `measurementType`) never auto-resolve; * they advance only on a manual satisfy. - * 3. Per-reminder `clientManaged` suppression of the server-side APNs - * (the medication `clientManaged` precedent) is applied for the - * whole user via `notificationPrefs.measurementReminder.clientManaged`. + * 3. `notificationPrefs.measurementReminder.clientManaged` suppresses the + * server-side APNs send only, and it does so in the dispatcher — the + * tick dispatches for every user and lets the cascade decide. */ import type { PrismaClient } from "@/generated/prisma/client"; import type { MeasurementType } from "@/generated/prisma/client"; @@ -35,7 +35,6 @@ import { defaultLocale, locales } from "@/lib/i18n/config"; import { wallClockInTz } from "@/lib/tz/wall-clock"; import { dispatchNotification } from "@/lib/notifications/dispatcher"; import { getEvent } from "@/lib/logging/context"; -import { isMeasurementReminderClientManaged } from "@/lib/validations/notification-prefs"; import { isModuleEnabled } from "@/lib/modules/gate"; import type { ModuleKey } from "@/lib/modules/registry"; import { @@ -91,7 +90,6 @@ export interface MeasurementReminderSummary { skippedNotDue: number; skippedOutsideWindow: number; skippedModuleDisabled: number; - skippedClientManaged: number; skippedNoChannel: number; /** v1.18.1 — expired COACH course-window reminders soft-deleted this tick. */ expiredCleaned: number; @@ -204,7 +202,6 @@ export async function runMeasurementReminderTick( skippedNotDue: 0, skippedOutsideWindow: 0, skippedModuleDisabled: 0, - skippedClientManaged: 0, skippedNoChannel: 0, expiredCleaned: 0, failed: 0, @@ -237,7 +234,6 @@ export async function runMeasurementReminderTick( id: true, timezone: true, locale: true, - notificationPrefs: true, }, }, }, @@ -318,20 +314,15 @@ export async function runMeasurementReminderTick( continue; } - // Per-user client-managed suppression (the medication precedent). - if (isMeasurementReminderClientManaged(reminder.user.notificationPrefs)) { - getEvent()?.addMeta( - "measurement_reminder.suppressed_client_managed", - reminder.id, - ); - summary.skippedClientManaged += 1; - // Advance past this cycle anyway so a client-managed reminder does - // not pin the server tick re-evaluating the same overdue slot - // every 15 minutes for the rest of the day. - await advanceNextDue(prisma, reminder, timezone, now); - continue; - } - + // `measurementReminder.clientManaged` is NOT consulted here. It + // suppresses the server's APNs send only — the dispatcher's APNs + // branch owns that decision and emits the + // `measurement_reminder.suppressed_client_managed` annotation per + // skip. The tick used to suppress every channel AND advance + // `nextDueAt`, so a preventive-care reminder that never reached the + // user was filed as handled and vanished from the card and the digest. + // Now, when nothing delivers, the no-channel branch below leaves the + // reminder overdue and visible. const { title, body } = buildMeasurementReminderPayload( reminder.user.locale, reminder.label, diff --git a/src/lib/jobs/reminder/mood-cycle-checks.ts b/src/lib/jobs/reminder/mood-cycle-checks.ts index 9506fd58e..afa03a835 100644 --- a/src/lib/jobs/reminder/mood-cycle-checks.ts +++ b/src/lib/jobs/reminder/mood-cycle-checks.ts @@ -112,7 +112,6 @@ export async function handleMeasurementReminderCheck( auto_resolved: summary.autoResolved, skipped_not_due: summary.skippedNotDue, skipped_outside_window: summary.skippedOutsideWindow, - skipped_client_managed: summary.skippedClientManaged, skipped_no_channel: summary.skippedNoChannel, failed: summary.failed, }, diff --git a/src/lib/notifications/__tests__/client-managed-apns.test.ts b/src/lib/notifications/__tests__/client-managed-apns.test.ts new file mode 100644 index 000000000..9121c89a5 --- /dev/null +++ b/src/lib/notifications/__tests__/client-managed-apns.test.ts @@ -0,0 +1,206 @@ +/** + * Client-managed suppression is APNs-only. + * + * `notificationPrefs.medication.clientManaged` (and its measurement- + * reminder sibling) means "my iPhone schedules this reminder locally, so + * don't push it". It never meant "stop telling me anywhere" — the OpenAPI + * description, the settings copy and the iOS contract all say APNs. The + * crons used to skip the whole dispatch, so a user who flipped the switch + * also lost the Telegram reminder they read on a desktop. + * + * Pinned here: the APNs channel is skipped, every other channel still + * sends, the skip is not counted as a delivery attempt, and the per-skip + * wide-event annotation survives so the suppression stays countable. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@/lib/db", () => ({ + prisma: { + notificationChannel: { + findMany: vi.fn(), + update: vi.fn(), + upsert: vi.fn(), + }, + user: { findUnique: vi.fn() }, + }, +})); + +vi.mock("@/lib/crypto", () => ({ + encrypt: (s: string) => s, + decrypt: (s: string) => s, +})); + +const addMetaMock = vi.fn(); +vi.mock("@/lib/logging/context", () => ({ + getEvent: () => ({ + addWarning: vi.fn(), + addMeta: (key: string, value: unknown) => addMetaMock(key, value), + addExternalCall: vi.fn(), + setError: vi.fn(), + }), +})); + +const sendViaApnsMock = vi.fn(); +const sendViaTelegramMock = vi.fn(); +const recordPushAttemptMock = vi.fn(); + +vi.mock("@/lib/notifications/senders/apns", () => ({ + sendViaApns: (...args: unknown[]) => sendViaApnsMock(...args), +})); +vi.mock("@/lib/notifications/senders/telegram", () => ({ + sendViaTelegram: (...args: unknown[]) => sendViaTelegramMock(...args), +})); +vi.mock("@/lib/notifications/senders/push-attempt-record", () => ({ + recordPushAttempt: (...args: unknown[]) => recordPushAttemptMock(...args), +})); + +import { dispatchNotification } from "@/lib/notifications/dispatcher"; +import { prisma } from "@/lib/db"; + +function channel(type: "APNS" | "TELEGRAM") { + return { + id: `ch-${type}`, + userId: "u-1", + type, + enabled: true, + config: JSON.stringify({ botToken: "b", chatId: "c" }), + consecutiveFailures: 0, + nextRetryAt: null, + preferences: [], + }; +} + +const MEDICATION_PAYLOAD = { + eventType: "MEDICATION_REMINDER" as const, + userId: "u-1", + title: "t", + message: "m", + metadata: { + medicationId: "med-1", + scheduledAt: "2026-06-15T08:00:00.000Z", + }, +}; + +beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(prisma.notificationChannel.findMany).mockResolvedValue([ + channel("APNS"), + channel("TELEGRAM"), + ] as never); + sendViaApnsMock.mockResolvedValue({ ok: true }); + sendViaTelegramMock.mockResolvedValue({ ok: true }); + ( + prisma.notificationChannel.update as unknown as ReturnType + ).mockResolvedValue({ consecutiveFailures: 0 }); +}); + +afterEach(() => { + vi.clearAllMocks(); +}); + +describe("dispatchNotification — client-managed APNs suppression", () => { + it("skips APNs and still delivers over every other channel", async () => { + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + notificationPrefs: { medication: { clientManaged: true } }, + } as never); + + const outcome = await dispatchNotification(MEDICATION_PAYLOAD); + + expect(sendViaApnsMock).not.toHaveBeenCalled(); + expect(sendViaTelegramMock).toHaveBeenCalledTimes(1); + // The skipped channel is not an attempt — `dispatched` has to keep + // meaning "something reached the user". + expect(outcome.channelsAttempted).toBe(1); + expect(outcome.channelsSucceeded).toBe(1); + expect(outcome.dispatched).toBe(true); + }); + + it("keeps the skip countable in the wide event and the ledger", async () => { + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + notificationPrefs: { medication: { clientManaged: true } }, + } as never); + + await dispatchNotification(MEDICATION_PAYLOAD); + + expect(addMetaMock).toHaveBeenCalledWith( + "medication_reminder_suppressed_client_managed", + "med-1:2026-06-15T08:00:00.000Z", + ); + expect(addMetaMock).toHaveBeenCalledWith( + "medication_reminder_suppressed_meta", + { + user_id: "u-1", + medication_id: "med-1", + schedule_id: "", + phase: "", + dose_at: "2026-06-15T08:00:00.000Z", + }, + ); + expect(recordPushAttemptMock).toHaveBeenCalledWith({ + userId: "u-1", + channel: "APNS", + eventType: "MEDICATION_REMINDER", + result: "skipped", + reason: "client_managed", + }); + }); + + it("honours the roaming deliveryDefault mapping", async () => { + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + notificationPrefs: { medication: { deliveryDefault: "client" } }, + } as never); + + await dispatchNotification(MEDICATION_PAYLOAD); + + expect(sendViaApnsMock).not.toHaveBeenCalled(); + }); + + it("suppresses the measurement reminder's APNs leg on its own flag", async () => { + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + notificationPrefs: { measurementReminder: { clientManaged: true } }, + } as never); + + await dispatchNotification({ + eventType: "MEASUREMENT_REMINDER", + userId: "u-1", + title: "t", + message: "m", + metadata: { reminderId: "rem-1" }, + }); + + expect(sendViaApnsMock).not.toHaveBeenCalled(); + expect(sendViaTelegramMock).toHaveBeenCalledTimes(1); + expect(addMetaMock).toHaveBeenCalledWith( + "measurement_reminder.suppressed_client_managed", + "rem-1", + ); + }); + + it("sends over APNs when the flag is unset", async () => { + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + notificationPrefs: null, + } as never); + + await dispatchNotification(MEDICATION_PAYLOAD); + + expect(sendViaApnsMock).toHaveBeenCalledTimes(1); + expect(recordPushAttemptMock).not.toHaveBeenCalled(); + }); + + it("does not gate an event type that has no client-managed opt-out", async () => { + // The medication flag must not leak across event types: a user who + // owns their dose reminders locally still gets everything else. + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + notificationPrefs: { medication: { clientManaged: true } }, + } as never); + + await dispatchNotification({ + eventType: "SYSTEM_ALERT", + userId: "u-1", + title: "t", + message: "m", + }); + + expect(sendViaApnsMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/lib/notifications/client-managed-apns.ts b/src/lib/notifications/client-managed-apns.ts new file mode 100644 index 000000000..b63cc38d0 --- /dev/null +++ b/src/lib/notifications/client-managed-apns.ts @@ -0,0 +1,79 @@ +/** + * Client-managed suppression — the APNs leg only. + * + * A user whose iOS client schedules its own local reminders sets + * `notificationPrefs..clientManaged` (or the medication + * `deliveryDefault: "client"` that maps onto it). The published contract, + * the OpenAPI description and every copy site say the same thing: it + * suppresses the server's APNs send. It used to suppress the whole + * dispatch, so a user who flipped it also lost Telegram, ntfy, webhook, + * email and Web Push — channels they may read on a device that is not the + * phone holding the local schedule. + * + * The gate therefore lives here, next to the channel loop, and applies to + * exactly one channel. The per-skip wide-event annotation is preserved so + * the suppression stays countable; the annotation key per category is the + * one the crons emitted before the move. + */ +import { + isMeasurementReminderClientManaged, + isMedicationReminderClientManaged, +} from "@/lib/validations/notification-prefs"; + +interface ClientManagedGate { + /** Reads the category's flag out of the raw `notificationPrefs` blob. */ + isClientManaged: (raw: unknown) => boolean; + /** Wide-event meta key, unchanged from the cron that used to emit it. */ + metaKey: string; + /** Stable per-skip identifier built from the dispatch payload metadata. */ + tag: (metadata: Record | undefined) => string; + /** + * Optional structured companion, emitted under `detailKey`. Same key and + * same shape the cron used to emit, so an operator's existing read of the + * wide event still resolves the medication, schedule and dose instant. + */ + detailKey?: string; + detail?: ( + userId: string, + metadata: Record | undefined, + ) => Record; +} + +function str(value: unknown): string { + return typeof value === "string" ? value : ""; +} + +/** + * Closed map: only these event types honour a client-managed flag. Every + * other event dispatches to APNs unconditionally. Cycle reminders are not + * listed — that cron still resolves its own flag before dispatching. + */ +export const CLIENT_MANAGED_APNS_EVENTS: Record = { + MEDICATION_REMINDER: { + isClientManaged: isMedicationReminderClientManaged, + metaKey: "medication_reminder_suppressed_client_managed", + tag: (metadata) => + `${str(metadata?.medicationId)}:${str(metadata?.scheduledAt)}`, + detailKey: "medication_reminder_suppressed_meta", + detail: (userId, metadata) => ({ + user_id: userId, + medication_id: str(metadata?.medicationId), + schedule_id: str(metadata?.scheduleId), + phase: str(metadata?.phase), + dose_at: str(metadata?.scheduledAt), + }), + }, + MEASUREMENT_REMINDER: { + isClientManaged: isMeasurementReminderClientManaged, + metaKey: "measurement_reminder.suppressed_client_managed", + tag: (metadata) => str(metadata?.reminderId), + }, +}; + +/** + * Does this event type have a client-managed opt-out at all? Cheap check + * the dispatcher runs before paying for the preferences read. + */ +export function hasClientManagedApnsGate(eventType: string): boolean { + return eventType in CLIENT_MANAGED_APNS_EVENTS; +} diff --git a/src/lib/notifications/dispatcher.ts b/src/lib/notifications/dispatcher.ts index cab12442e..48336ef85 100644 --- a/src/lib/notifications/dispatcher.ts +++ b/src/lib/notifications/dispatcher.ts @@ -24,6 +24,11 @@ import { recordChannelTransientFailure, } from "@/lib/notifications/channel-state"; import { getEvent } from "@/lib/logging/context"; +import { + CLIENT_MANAGED_APNS_EVENTS, + hasClientManagedApnsGate, +} from "@/lib/notifications/client-managed-apns"; +import { recordPushAttempt } from "@/lib/notifications/senders/push-attempt-record"; /** * Dispatch a notification to all enabled channels for a user. @@ -33,7 +38,9 @@ import { getEvent } from "@/lib/logging/context"; * 1. Skip if `enabled=false` (manually or auto-disabled). * 2. Skip if currently in retry-cooldown (`nextRetryAt > now`). * 3. Check the per-channel preference for this eventType (default: enabled). - * 4. Call the appropriate sender — which now returns a `SendOutcome` so + * 4. Skip the APNs channel — and only that one — when the user's iOS + * client owns the local reminder for this event type. + * 5. Call the appropriate sender — which now returns a `SendOutcome` so * the dispatcher can classify hard rejects (410, blocked-by-user) vs * soft errors (5xx, 429, network) and update channel state accordingly. * @@ -156,6 +163,26 @@ export async function dispatchNotification( }); defaultEnabled = user?.moodReminderEnabled === true; } + // Client-managed APNs suppression, resolved at most once per dispatch + // and only when an APNs channel is actually in the cascade. `null` + // means "not applicable"; the read is lazy so the common event types + // pay nothing for it. + let clientManagedApns: boolean | null = null; + const resolveClientManagedApns = async (): Promise => { + if (clientManagedApns !== null) return clientManagedApns; + const gate = CLIENT_MANAGED_APNS_EVENTS[payload.eventType]; + if (!gate) { + clientManagedApns = false; + return false; + } + const row = await prisma.user.findUnique({ + where: { id: payload.userId }, + select: { notificationPrefs: true }, + }); + clientManagedApns = gate.isClientManaged(row?.notificationPrefs); + return clientManagedApns; + }; + for (const channel of channels) { const pref = channel.preferences[0]; if (pref) { @@ -176,6 +203,33 @@ export async function dispatchNotification( continue; } + // The iOS client owns the local banner for this event type — skip + // the server's APNs send and nothing else. Not counted as an + // attempt: no send was made, so `dispatched` keeps telling the truth + // about whether anything actually reached the user. + if ( + channel.type === "APNS" && + hasClientManagedApnsGate(payload.eventType) && + (await resolveClientManagedApns()) + ) { + const gate = CLIENT_MANAGED_APNS_EVENTS[payload.eventType]; + getEvent()?.addMeta(gate.metaKey, gate.tag(payload.metadata)); + if (gate.detail && gate.detailKey) { + getEvent()?.addMeta( + gate.detailKey, + gate.detail(payload.userId, payload.metadata), + ); + } + recordPushAttempt({ + userId: payload.userId, + channel: "APNS", + eventType: payload.eventType, + result: "skipped", + reason: "client_managed", + }); + continue; + } + channelsAttempted += 1; try {