From a4a3318de3819e83010e4b3eaa3dbf599f7f124c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 11 Oct 2026 13:37:46 +0200 Subject: [PATCH 01/12] fix(mood): open the reminder's entry sheet only in the own record --- .../__tests__/add-param-record-scope.test.tsx | 106 ++++++++++++++++++ src/app/mood/page-client.tsx | 41 +++++-- src/lib/notifications/tap-target.ts | 5 +- 3 files changed, 143 insertions(+), 9 deletions(-) create mode 100644 src/app/mood/__tests__/add-param-record-scope.test.tsx diff --git a/src/app/mood/__tests__/add-param-record-scope.test.tsx b/src/app/mood/__tests__/add-param-record-scope.test.tsx new file mode 100644 index 000000000..684dc743c --- /dev/null +++ b/src/app/mood/__tests__/add-param-record-scope.test.tsx @@ -0,0 +1,106 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { renderToStaticMarkup } from "react-dom/server"; +import type { ReactNode } from "react"; + +/** + * `/mood?add=1` is the mood reminder's tap target. It opens the entry sheet + * only in the caller's own record: a session that is inside a managed + * profile or a shared record opens the page without the sheet, so a reminder + * meant for the recipient never becomes a write form on somebody else. + */ + +const caps = { + current: { + inSharedRecord: false, + recordSessionPending: undefined as boolean | undefined, + accessRefused: undefined as boolean | undefined, + canManageDomain: (_d: string) => true, + }, +}; +const params = { current: new URLSearchParams("add=1") }; + +vi.mock("@/hooks/use-record-capabilities", () => ({ + useRecordCapabilities: () => caps.current, +})); +vi.mock("@/hooks/use-auth", () => ({ + useAuth: () => ({ + user: { modules: {} }, + isAuthenticated: true, + isLoading: false, + }), +})); +vi.mock("@/hooks/use-mounted", () => ({ useMounted: () => true })); +vi.mock("@/hooks/use-pull-to-refresh", () => ({ + usePullToRefresh: () => ({}), +})); +vi.mock("@tanstack/react-query", () => ({ + useQueryClient: () => ({ invalidateQueries: () => {} }), +})); +vi.mock("next/navigation", () => ({ + useRouter: () => ({ replace: () => {}, push: () => {} }), + useSearchParams: () => params.current, +})); +vi.mock("@/lib/i18n/context", () => ({ + useTranslations: () => ({ t: (key: string) => key }), +})); +vi.mock("@/components/ui/responsive-sheet", () => ({ + ResponsiveSheet: ({ open }: { open: boolean; children?: ReactNode }) => + open ?
: null, +})); +vi.mock("@/components/mood/mood-form", () => ({ MoodForm: () => null })); +vi.mock("@/components/mood/mood-list", () => ({ MoodList: () => null })); +vi.mock("@/components/ui/pull-to-refresh-indicator", () => ({ + PullToRefreshIndicator: () => null, +})); + +import MoodPageClient from "../page-client"; + +function sheetOpen(): boolean { + return renderToStaticMarkup().includes( + 'data-slot="mood-sheet-open"', + ); +} + +beforeEach(() => { + params.current = new URLSearchParams("add=1"); + caps.current = { + inSharedRecord: false, + recordSessionPending: undefined, + accessRefused: undefined, + canManageDomain: () => true, + }; +}); + +describe("/mood ?add=1 and the record on screen", () => { + it("opens the entry sheet in the caller's own record", () => { + expect(sheetOpen()).toBe(true); + }); + + it("keeps the sheet closed without the param", () => { + params.current = new URLSearchParams(); + expect(sheetOpen()).toBe(false); + }); + + it("does not open the sheet inside a managed profile the caller manages", () => { + // A guardian switched into a managed profile holds MANAGE on mind, so + // the sheet would otherwise open and save onto that profile. + caps.current = { ...caps.current, inSharedRecord: true }; + expect(sheetOpen()).toBe(false); + }); + + it("does not open the sheet while the record context is pending or refused", () => { + caps.current = { + ...caps.current, + inSharedRecord: true, + recordSessionPending: true, + canManageDomain: () => false, + }; + expect(sheetOpen()).toBe(false); + caps.current = { + ...caps.current, + recordSessionPending: undefined, + accessRefused: true, + }; + expect(sheetOpen()).toBe(false); + }); +}); diff --git a/src/app/mood/page-client.tsx b/src/app/mood/page-client.tsx index 25464b0b7..5891e6a15 100644 --- a/src/app/mood/page-client.tsx +++ b/src/app/mood/page-client.tsx @@ -21,7 +21,12 @@ import { useTranslations } from "@/lib/i18n/context"; export default function MoodPageClient() { const { user, isAuthenticated, isLoading } = useAuth(); - const { canManageDomain, inSharedRecord } = useRecordCapabilities(); + const { + canManageDomain, + inSharedRecord, + recordSessionPending, + accessRefused, + } = useRecordCapabilities(); // A mood entry is created at MANAGE under the mind section. const canAddMood = canManageDomain("mind"); const mounted = useMounted(); @@ -33,15 +38,35 @@ export default function MoodPageClient() { // hydration render agree; the effect below strips the one-shot param so the // back button does not reopen the sheet. A later client-side navigation // back onto `?add=1` is handled render-side, the measurements-page pattern. + // + // The request is held apart from the sheet's own open state because it is + // honoured only in the caller's own record. A reminder is about the person + // who receives it; a session that is currently inside a managed profile or + // a shared record would otherwise open a write form there, and an entry + // saved from it lands on the wrong person. In that case the page opens + // without the sheet and the shell's shared-record banner offers the way + // back. The answer is read once the account query settled (`isLoading` + // gates the content render), so the request is dropped, not deferred: a + // later switch back to the own record must not pop a stale sheet. const addParam = searchParams.get("add") === "1"; - const [dialogOpen, setDialogOpen] = useState(() => addParam); + const [dialogOpen, setDialogOpen] = useState(false); + const [addRequested, setAddRequested] = useState(() => addParam); const [consumedAdd, setConsumedAdd] = useState(() => addParam); if (!addParam && consumedAdd) { setConsumedAdd(false); } else if (addParam && !consumedAdd) { setConsumedAdd(true); - setDialogOpen(true); + setAddRequested(true); } + const ownRecord = !inSharedRecord && !recordSessionPending && !accessRefused; + if (addRequested && mounted && !isLoading && !ownRecord) { + setAddRequested(false); + } + const sheetOpen = (dialogOpen || (addRequested && ownRecord)) && canAddMood; + const setSheetOpen = useCallback((open: boolean) => { + setDialogOpen(open); + if (!open) setAddRequested(false); + }, []); // The list's empty state carries the add action while there is nothing // to list, so the header drops its own then. const [listEmpty, setListEmpty] = useState(false); @@ -68,7 +93,7 @@ export default function MoodPageClient() { ); const pull = usePullToRefresh({ onRefresh: refreshVisible, - disabled: dialogOpen, + disabled: sheetOpen, }); useEffect(() => { @@ -149,14 +174,14 @@ export default function MoodPageClient() { /> } > setDialogOpen(false)} - onCancel={() => setDialogOpen(false)} + onSuccess={() => setSheetOpen(false)} + onCancel={() => setSheetOpen(false)} footerSlot={footerEl} /> diff --git a/src/lib/notifications/tap-target.ts b/src/lib/notifications/tap-target.ts index 10bb30de4..3552e4ace 100644 --- a/src/lib/notifications/tap-target.ts +++ b/src/lib/notifications/tap-target.ts @@ -36,7 +36,10 @@ export const NOTIFICATION_TAP_PATHS: Record = { WITHINGS_SYNC_FAILED: "/settings/integrations", SYSTEM_ALERT: "/", PERSONAL_RECORD: "/achievements", - // `?add=1` opens the mood entry sheet on the first paint. + // `?add=1` opens the mood entry sheet on the first paint, and only in the + // recipient's own record: a session switched into another record opens the + // page without the sheet (`src/app/mood/page-client.tsx`). No other target + // here opens a write form. MOOD_REMINDER: "/mood?add=1", CYCLE_PERIOD_SOON: "/cycle", CYCLE_PERIOD_CONFIRM: "/cycle", From bc91a542e2a609a3889d168043b837d8ac36486f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 11 Oct 2026 13:41:53 +0200 Subject: [PATCH 02/12] fix(google-health): count backfill progress only once it is persisted and cap the slice chain --- .../backfill-resume-progress.test.ts | 65 +++++++++++++++++++ src/lib/google-health/collection-keys.ts | 29 +++++++++ src/lib/google-health/sync-activity.ts | 3 +- src/lib/google-health/sync-core.ts | 33 ++++++---- src/lib/google-health/sync-metrics.ts | 3 +- .../__tests__/google-health-backfill.test.ts | 38 ++++++++++- src/lib/jobs/google-health-backfill.ts | 30 ++++++++- 7 files changed, 184 insertions(+), 17 deletions(-) create mode 100644 src/lib/google-health/__tests__/backfill-resume-progress.test.ts create mode 100644 src/lib/google-health/collection-keys.ts diff --git a/src/lib/google-health/__tests__/backfill-resume-progress.test.ts b/src/lib/google-health/__tests__/backfill-resume-progress.test.ts new file mode 100644 index 000000000..aa80d605d --- /dev/null +++ b/src/lib/google-health/__tests__/backfill-resume-progress.test.ts @@ -0,0 +1,65 @@ +/** + * A collection counts toward a backfill's progress only once its resume note + * is written. The job reads the size of `done` as progress and queues another + * slice on it, so a key added before a write that failed would let a chain of + * slices "progress" over the same collection without end. + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const { prismaMock, warnings } = vi.hoisted(() => ({ + prismaMock: { + googleHealthConnection: { update: vi.fn() }, + }, + warnings: [] as string[], +})); + +vi.mock("@/lib/db", () => ({ prisma: prismaMock })); +vi.mock("@/lib/logging/context", () => ({ + annotate: () => {}, + getEvent: () => ({ addWarning: (w: string) => warnings.push(w) }), +})); + +import { + runGoogleHealthCollection, + runWithGoogleHealthBackfillResume, +} from "../sync-core"; + +beforeEach(() => { + vi.clearAllMocks(); + warnings.length = 0; +}); + +describe("runGoogleHealthCollection under a backfill", () => { + it("adds a clean collection to done once its note is written", async () => { + prismaMock.googleHealthConnection.update.mockResolvedValue({}); + const done = new Set(["fetchSteps"]); + + await runWithGoogleHealthBackfillResume("u1", done, () => + runGoogleHealthCollection("sleep", async () => 3), + ); + + expect([...done]).toEqual(["fetchSteps", "sleep"]); + expect(prismaMock.googleHealthConnection.update).toHaveBeenCalledWith({ + where: { userId: "u1" }, + data: { backfillProgress: { done: ["fetchSteps", "sleep"] } }, + }); + }); + + it("counts no progress when the note cannot be written", async () => { + prismaMock.googleHealthConnection.update.mockRejectedValue( + new Error("connection reset"), + ); + const done = new Set(["fetchSteps"]); + + const { result } = await runWithGoogleHealthBackfillResume("u1", done, () => + runGoogleHealthCollection("sleep", async () => 3), + ); + + // The rows are in, the collection did not fail, but it is not finished. + expect(result).toBe(3); + expect([...done]).toEqual(["fetchSteps"]); + expect(warnings.some((w) => w.includes("progress write failed"))).toBe( + true, + ); + }); +}); diff --git a/src/lib/google-health/collection-keys.ts b/src/lib/google-health/collection-keys.ts new file mode 100644 index 000000000..f6304891c --- /dev/null +++ b/src/lib/google-health/collection-keys.ts @@ -0,0 +1,29 @@ +/** + * Every collection a Google Health backfill walks, by the key its resume note + * stores (`runGoogleHealthCollection`). The key type is what that function + * accepts, so a collection added without an entry here does not compile, and + * the backfill's slice ceiling (`GOOGLE_HEALTH_BACKFILL_MAX_SLICE`) is derived + * from this list rather than counted by hand. + */ +export const GOOGLE_HEALTH_COLLECTION_KEYS = [ + "fetchSteps", + "fetchDistance", + "fetchActiveEnergy", + "fetchFloors", + "fetchVo2Max", + "fetchWeight", + "fetchBodyFat", + "fetchSpo2", + "fetchHrv", + "fetchRhr", + "fetchRespiratoryRate", + "fetchHeartRate", + "fetchBloodGlucose", + "fetchCoreBodyTemperature", + "fetchSleepTemperature", + "fetchExercise", + "sleep", +] as const; + +export type GoogleHealthCollectionKey = + (typeof GOOGLE_HEALTH_COLLECTION_KEYS)[number]; diff --git a/src/lib/google-health/sync-activity.ts b/src/lib/google-health/sync-activity.ts index 32344e993..0c17e2209 100644 --- a/src/lib/google-health/sync-activity.ts +++ b/src/lib/google-health/sync-activity.ts @@ -46,6 +46,7 @@ import { type GoogleHealthResourceSyncOptions, } from "./sync-core"; import { annotate } from "@/lib/logging/context"; +import type { GoogleHealthCollectionKey } from "./collection-keys"; import { resolveUserTimezone } from "@/lib/tz/resolver"; /** One mappable activity metric: its data-type encoding + the mapper + a verb. */ @@ -55,7 +56,7 @@ interface ActivityResource { point: Record, tz: string, ) => GoogleHealthMappedMeasurement[]; - verb: string; + verb: GoogleHealthCollectionKey; } /** The four cumulative daily totals — read via `POST :dailyRollUp`. */ diff --git a/src/lib/google-health/sync-core.ts b/src/lib/google-health/sync-core.ts index de64e573d..dc215266d 100644 --- a/src/lib/google-health/sync-core.ts +++ b/src/lib/google-health/sync-core.ts @@ -20,6 +20,7 @@ import { prisma } from "@/lib/db"; import type { MeasurementType } from "@/generated/prisma/client"; import { encrypt, decrypt } from "@/lib/crypto"; import { annotate, getEvent } from "@/lib/logging/context"; +import type { GoogleHealthCollectionKey } from "./collection-keys"; import { userDayKey } from "@/lib/tz/format"; import { isP2002 } from "@/lib/prisma-errors"; import { @@ -464,7 +465,7 @@ export async function runWithGoogleHealthBackfillResume( * budget asking it to stop). Anything less is walked again next attempt. */ export async function runGoogleHealthCollection( - key: string, + key: GoogleHealthCollectionKey, fn: () => Promise, ): Promise { const resume = backfillResumeStorage.getStore(); @@ -483,19 +484,27 @@ export async function runGoogleHealthCollection( !truncatedBefore && !googleHealthOutcomeTruncated(); if (clean) { - resume.done.add(key); - await prisma.googleHealthConnection - .update({ + // The collection counts as finished only once the note says so. The job + // reads the size of `done` as this attempt's progress and sends another + // slice on it; a key added before a write that then failed would be + // progress nobody can see next attempt, and a chain of slices could keep + // "progressing" over the same collection forever. + try { + await prisma.googleHealthConnection.update({ where: { userId: resume.userId }, - data: { backfillProgress: { done: [...resume.done] } }, - }) - .catch((err) => - // Losing the note costs a re-walk of this collection next attempt, - // nothing more; it must not fail a collection that did complete. - getEvent()?.addWarning( - `google-health: backfill progress write failed: ${err}`, - ), + data: { backfillProgress: { done: [...resume.done, key] } }, + }); + resume.done.add(key); + } catch (err) { + // Not finished, then: the collection is walked again next attempt. It + // must not fail a collection whose rows are written. + getEvent()?.addWarning( + `google-health: backfill progress write failed for ${key}: ${err}`, ); + annotate({ + meta: { "googleHealth.backfill.progressWriteFailed": key }, + }); + } } return imported; } diff --git a/src/lib/google-health/sync-metrics.ts b/src/lib/google-health/sync-metrics.ts index 59deed1d7..a86dfb59e 100644 --- a/src/lib/google-health/sync-metrics.ts +++ b/src/lib/google-health/sync-metrics.ts @@ -55,6 +55,7 @@ import { type GoogleHealthResourceSyncOptions, } from "./sync-core"; import { getEvent } from "@/lib/logging/context"; +import type { GoogleHealthCollectionKey } from "./collection-keys"; import { prisma } from "@/lib/db"; import { resolveUserTimezone } from "@/lib/tz/resolver"; @@ -65,7 +66,7 @@ interface MetricResource { point: Record, tz: string, ) => GoogleHealthMappedMeasurement[]; - verb: string; + verb: GoogleHealthCollectionKey; /** Page ceiling for the walk; the client default when omitted. */ maxPages?: number; /** diff --git a/src/lib/jobs/__tests__/google-health-backfill.test.ts b/src/lib/jobs/__tests__/google-health-backfill.test.ts index 8afde8dd0..4e5e5a269 100644 --- a/src/lib/jobs/__tests__/google-health-backfill.test.ts +++ b/src/lib/jobs/__tests__/google-health-backfill.test.ts @@ -69,7 +69,10 @@ vi.mock("@/lib/google-health/sync", () => ({ syncUserGoogleHealth: (...a: unknown[]) => syncUserGoogleHealthMock(...a), })); -import { runGoogleHealthBackfillForUser } from "../google-health-backfill"; +import { + GOOGLE_HEALTH_BACKFILL_MAX_SLICE, + runGoogleHealthBackfillForUser, +} from "../google-health-backfill"; import { GOOGLE_HEALTH_DATA_TYPES, forEachDataPointPage, @@ -229,6 +232,39 @@ describe("runGoogleHealthBackfillForUser — slices", () => { ); }); + it("fails a slice past the ceiling without walking or queueing", async () => { + syncUserGoogleHealthMock.mockResolvedValue({ imported: 1, failed: true }); + resumeState.finishes = ["sleep"]; + + await expect( + runGoogleHealthBackfillForUser( + "u1", + undefined, + GOOGLE_HEALTH_BACKFILL_MAX_SLICE + 1, + ), + ).rejects.toThrow(/past the ceiling/); + expect(syncUserGoogleHealthMock).not.toHaveBeenCalled(); + expect(bossSend).not.toHaveBeenCalled(); + }); + + it("does not queue a slice past the ceiling even when this one progressed", async () => { + resumeState.finishes = ["sleep"]; + syncUserGoogleHealthMock.mockResolvedValue({ imported: 1, failed: true }); + + await expect( + runGoogleHealthBackfillForUser( + "u1", + undefined, + GOOGLE_HEALTH_BACKFILL_MAX_SLICE, + ), + ).rejects.toThrow(/incomplete/); + expect(bossSend).not.toHaveBeenCalled(); + }); + + it("caps the chain at the number of collections plus two", () => { + expect(GOOGLE_HEALTH_BACKFILL_MAX_SLICE).toBe(17 + 2); + }); + it("stamps the marker on the slice that finishes the history", async () => { resumeState.doneBefore = ["steps", "sleep"]; resumeState.finishes = ["weight"]; diff --git a/src/lib/jobs/google-health-backfill.ts b/src/lib/jobs/google-health-backfill.ts index bb3e66b42..4f3a00294 100644 --- a/src/lib/jobs/google-health-backfill.ts +++ b/src/lib/jobs/google-health-backfill.ts @@ -30,6 +30,7 @@ import { } from "@/lib/google-health/sync"; import { GOOGLE_HEALTH_SYNC_LOCK_WAIT_MS } from "@/lib/google-health/sync-lock"; import { runWithGoogleHealthStop } from "@/lib/google-health/client"; +import { GOOGLE_HEALTH_COLLECTION_KEYS } from "@/lib/google-health/collection-keys"; export const GOOGLE_HEALTH_BACKFILL_QUEUE = "google-health-backfill"; @@ -40,6 +41,17 @@ export const GOOGLE_HEALTH_BACKFILL_QUEUE = "google-health-backfill"; */ export const GOOGLE_HEALTH_BACKFILL_CONCURRENCY = 1; +/** + * The highest slice a backfill chain may reach. Every continued slice has + * finished at least one more collection than the one before it, so a healthy + * chain is at most one slice per collection long; the two extra slices are + * slack for the first one and the one that stamps the marker. A slice past + * this is a chain that counts progress it does not make, and it fails rather + * than queueing another. + */ +export const GOOGLE_HEALTH_BACKFILL_MAX_SLICE = + GOOGLE_HEALTH_COLLECTION_KEYS.length + 2; + export interface GoogleHealthBackfillPayload { userId: string; enqueuedAt: string; @@ -78,7 +90,10 @@ export interface GoogleHealthBackfillPayload { * `continued`, without the error line or the queue's failure count. Only an * attempt that finished nothing new throws, because that one is stuck: the * pg-boss retry and, after it, the next boot take it from there. Progress is - * bounded by the number of collections, so the chain of slices is too. + * counted only once it is persisted (`runGoogleHealthCollection` adds a + * collection to `done` after its note is written), and the chain is capped at + * `GOOGLE_HEALTH_BACKFILL_MAX_SLICE`: a slice past it throws instead of + * running or sending another. * * Never beside another run of the same account: the sync takes the account's * lock, waiting a bounded time for an hourly or manual run to finish. If the @@ -90,6 +105,17 @@ export async function runGoogleHealthBackfillForUser( shouldStop: () => boolean = () => false, slice: number = 0, ): Promise<{ imported: number; continued?: boolean }> { + if (slice > GOOGLE_HEALTH_BACKFILL_MAX_SLICE) { + annotate({ + action: { + name: "google_health.backfill.slice_ceiling", + details: { slice, maxSlice: GOOGLE_HEALTH_BACKFILL_MAX_SLICE }, + }, + }); + throw new Error( + `google-health backfill for user ${userId} reached slice ${slice}, past the ceiling of ${GOOGLE_HEALTH_BACKFILL_MAX_SLICE}`, + ); + } if (await isGoogleHealthParked(userId)) { annotate({ action: { @@ -143,7 +169,7 @@ export async function runGoogleHealthBackfillForUser( // `done` is the resume set this attempt added to as it finished // collections; a larger set than it started with is progress. const finished = done.size - doneBefore; - if (finished > 0) { + if (finished > 0 && slice + 1 <= GOOGLE_HEALTH_BACKFILL_MAX_SLICE) { const boss = getGlobalBoss(); const sent = boss ? await enqueueIntegrationBackfillAdmission( From 4ff9aa7e390f602ebac136ee2e226ce6005740ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 11 Oct 2026 13:43:35 +0200 Subject: [PATCH 03/12] fix(notifications): publish ntfy messages as JSON so non-ASCII labels need no header encoding --- docs/self-hosting/notifications.md | 9 +- .../senders/__tests__/ntfy.test.ts | 130 ++++++++++---- .../senders/__tests__/urgency.test.ts | 47 ++---- src/lib/notifications/senders/ntfy.ts | 158 ++++++++++-------- src/lib/notifications/tap-target.ts | 2 +- 5 files changed, 206 insertions(+), 140 deletions(-) diff --git a/docs/self-hosting/notifications.md b/docs/self-hosting/notifications.md index 0aa0d0761..0d4ca9eb0 100644 --- a/docs/self-hosting/notifications.md +++ b/docs/self-hosting/notifications.md @@ -179,6 +179,12 @@ HealthLog publishes to that topic. **Server URL** (defaults to `https://ntfy.sh`; point it at your own server if you self-host ntfy) and **Topic**, then save. +HealthLog publishes each message as JSON to the server's root URL (the topic +travels in the body, see ntfy's +[publish as JSON](https://docs.ntfy.sh/publish/#publish-as-json)), so titles +and button labels in any language arrive as written. A reverse proxy in front +of your own ntfy has to pass `POST /` through, not only `POST /`. + If your ntfy server requires auth, an access token can be attached; the server URL is validated at save time and dialled through the egress guard (`src/lib/notifications/senders/ntfy.ts`). A self-hosted ntfy on your own @@ -474,7 +480,8 @@ once per event type, and every channel carries the same target: How each channel carries it: -- **ntfy**: the `Click` header, so tapping the notification opens the page. +- **ntfy**: the message's `click` field, so tapping the notification opens + the page. - **Telegram**: an **Open in HealthLog** button under the message, below any action buttons. Telegram only accepts `https` button links on a public host, so a plain-HTTP instance gets a text link instead, and an instance diff --git a/src/lib/notifications/senders/__tests__/ntfy.test.ts b/src/lib/notifications/senders/__tests__/ntfy.test.ts index 8a28200fb..9463ff98a 100644 --- a/src/lib/notifications/senders/__tests__/ntfy.test.ts +++ b/src/lib/notifications/senders/__tests__/ntfy.test.ts @@ -1,8 +1,8 @@ /** * ntfy sender unit tests. * - * Covers the outbound shape (topic appended to the server URL, priority and - * tag headers, bearer token), the outcome classification, and (#947) the + * Covers the outbound shape (a JSON publish to the server root carrying + * topic, priority and tags, bearer token), the outcome classification, and (#947) the * NOTIFICATION_PRIVATE_ORIGINS grant: a listed server rides the * operator-approved pin, an unlisted one keeps the public pin, and a listed * origin admits nothing beside itself. @@ -85,7 +85,7 @@ afterEach(() => { }); describe("sendViaNtfy", () => { - it("POSTs the topic URL through safeFetch under the public pin and returns ok on 200", async () => { + it("POSTs a JSON message to the server root through safeFetch under the public pin and returns ok on 200", async () => { safeFetchMock.mockResolvedValue({ ok: true, status: 200 }); const result = await sendViaNtfy(config, payload()); @@ -96,10 +96,17 @@ describe("sendViaNtfy", () => { { method?: string; headers?: Record; body?: string }, EgressOpts, ]; - expect(url).toBe("https://ntfy.example.com/health"); + expect(url).toBe("https://ntfy.example.com/"); expect(init.method).toBe("POST"); expect(init.headers?.Authorization).toBe("Bearer tk_secret"); - expect(init.body).toBe("Body"); + expect(init.headers?.["Content-Type"]).toBe("application/json"); + expect(JSON.parse(init.body!)).toEqual({ + topic: "health", + title: "Title", + message: "Body", + priority: 3, + tags: ["system-alert"], + }); expect(opts.requirePublicHost).toBe(true); expect(opts.operatorApprovedPrivateOrigin).toBeUndefined(); expect(recordPushAttemptMock).toHaveBeenCalledWith( @@ -164,7 +171,7 @@ describe("sendViaNtfy", () => { unknown, EgressOpts, ]; - expect(url).toBe("http://ntfy.lan:8080/health"); + expect(url).toBe("http://ntfy.lan:8080/"); expect(opts.requirePublicHost).toBe(false); expect(opts.operatorApprovedPrivateOrigin).toBe("http://ntfy.lan:8080"); expect(annotateMock).toHaveBeenCalledWith({ @@ -270,15 +277,19 @@ describe("sendViaNtfy — what the server said (#947)", () => { }); describe("sendViaNtfy — tap target and quick actions", () => { - function headersOfFirstCall(): Record { + function messageOfFirstCall(): Record { const [, init] = safeFetchMock.mock.calls[0] as [ string, - { headers: Record }, + { headers: Record; body: string }, ]; - return init.headers; + // Nothing of the message rides a header any more. + expect(Object.keys(init.headers).sort()).toEqual( + ["Authorization", "Content-Type"].sort(), + ); + return JSON.parse(init.body) as Record; } - it("sets Click to the resolved absolute tap URL", async () => { + it("sets click to the resolved absolute tap URL", async () => { safeFetchMock.mockResolvedValue({ ok: true, status: 200 }); await sendViaNtfy( config, @@ -291,17 +302,18 @@ describe("sendViaNtfy — tap target and quick actions", () => { }, }), ); - expect(headersOfFirstCall().Click).toBe("https://health.example/"); - expect(headersOfFirstCall().Actions).toBeUndefined(); + const message = messageOfFirstCall(); + expect(message.click).toBe("https://health.example/"); + expect(message.actions).toBeUndefined(); }); - it("omits Click when no public origin produced a URL", async () => { + it("omits click when no public origin produced a URL", async () => { safeFetchMock.mockResolvedValue({ ok: true, status: 200 }); await sendViaNtfy( config, payload({ tap: { path: "/", url: null, label: "Open" } }), ); - expect(headersOfFirstCall().Click).toBeUndefined(); + expect(messageOfFirstCall().click).toBeUndefined(); }); it("sends the quick answers as three POST http actions that clear the notification", async () => { @@ -317,18 +329,66 @@ describe("sendViaNtfy — tap target and quick actions", () => { ], }), ); - expect(headersOfFirstCall().Actions).toBe( - "http, Good, https://h.example/a/1, method=POST, clear=true; " + - "http, Okay, https://h.example/a/2, method=POST, clear=true; " + - "http, Bad, https://h.example/a/3, method=POST, clear=true", + expect(messageOfFirstCall().actions).toEqual([ + { + action: "http", + label: "Good", + url: "https://h.example/a/1", + method: "POST", + clear: true, + }, + { + action: "http", + label: "Okay", + url: "https://h.example/a/2", + method: "POST", + clear: true, + }, + { + action: "http", + label: "Bad", + url: "https://h.example/a/3", + method: "POST", + clear: true, + }, + ]); + }); + + it("carries non-ASCII titles and labels as plain UTF-8, URLs untouched", async () => { + safeFetchMock.mockResolvedValue({ ok: true, status: 200 }); + await sendViaNtfy( + config, + payload({ + eventType: "MOOD_REMINDER", + title: "기분은 어떠세요?", + quickActions: [ + { id: "mood-good", label: "Świetnie", url: "https://h.example/a/1" }, + { id: "mood-okay", label: "보통", url: "https://h.example/a/2" }, + { id: "mood-bad", label: "Così così", url: "https://h.example/a/3" }, + ], + }), ); + const message = messageOfFirstCall(); + expect(message.title).toBe("기분은 어떠세요?"); + const actions = message.actions as { label: string; url: string }[]; + expect(actions.map((a) => a.label)).toEqual([ + "Świetnie", + "보통", + "Così così", + ]); + expect(actions.map((a) => a.url)).toEqual([ + "https://h.example/a/1", + "https://h.example/a/2", + "https://h.example/a/3", + ]); + expect(JSON.stringify(message)).not.toContain("=?UTF-8?"); }); }); -describe("ntfy header helpers", () => { - it("caps the actions at three and strips separators from labels", async () => { - const { buildNtfyActionsHeader } = await import("../ntfy"); - const header = buildNtfyActionsHeader({ +describe("ntfy message helpers", () => { + it("caps the actions at three and keeps labels verbatim", async () => { + const { buildNtfyActions } = await import("../ntfy"); + const actions = buildNtfyActions({ quickActions: [ { id: "mood-good", label: 'Go;od, "yes"', url: "https://h/1" }, { id: "mood-okay", label: "B", url: "https://h/2" }, @@ -336,20 +396,20 @@ describe("ntfy header helpers", () => { { id: "mood-bad", label: "D", url: "https://h/4" }, ], }); - expect(header?.split("; ")).toHaveLength(3); - expect(header).toContain("http, Go od yes, https://h/1"); - expect(header).not.toContain("https://h/4"); - expect(buildNtfyActionsHeader({ quickActions: [] })).toBeNull(); + expect(actions).toHaveLength(3); + // JSON needs no separator escaping: the label reaches ntfy as written. + expect(actions[0]!.label).toBe('Go;od, "yes"'); + expect(actions.map((a) => a.url)).not.toContain("https://h/4"); + expect(buildNtfyActions({ quickActions: [] })).toEqual([]); }); - it("RFC 2047-encodes a non-ASCII header value and leaves ASCII alone", async () => { - const { ntfyHeaderValue } = await import("../ntfy"); - expect(ntfyHeaderValue("Morning score 78")).toBe("Morning score 78"); - const encoded = ntfyHeaderValue("아침 점수 78"); - expect(encoded).toMatch(/^=\?UTF-8\?B\?[A-Za-z0-9+/=]+\?=$/); - const inner = encoded.slice("=?UTF-8?B?".length, -2); - expect(Buffer.from(inner, "base64").toString("utf8")).toBe("아침 점수 78"); - // `ü` is Latin-1, which fetch would send as one invalid UTF-8 byte. - expect(ntfyHeaderValue("Morgenüberblick")).toMatch(/^=\?UTF-8\?B\?/); + it("maps a medication reminder to high priority (4)", async () => { + const { buildNtfyMessage } = await import("../ntfy"); + expect( + buildNtfyMessage( + "t", + payload({ eventType: "MEDICATION_REMINDER" }) as never, + ).priority, + ).toBe(4); }); }); diff --git a/src/lib/notifications/senders/__tests__/urgency.test.ts b/src/lib/notifications/senders/__tests__/urgency.test.ts index a66b01831..29273456e 100644 --- a/src/lib/notifications/senders/__tests__/urgency.test.ts +++ b/src/lib/notifications/senders/__tests__/urgency.test.ts @@ -104,6 +104,15 @@ beforeEach(() => { afterEach(() => vi.clearAllMocks()); +/** The JSON message the ntfy sender published on call `i`. */ +function ntfyMessage(i: number): { + priority: number; + tags: string[]; + actions?: unknown; +} { + return JSON.parse((safeFetchMock.mock.calls[i][1] as { body: string }).body); +} + describe("ntfy urgent mapping", () => { it("urgent → Priority 5 (max) + warning tag", async () => { safeFetchMock.mockResolvedValue({ ok: true, status: 200 }); @@ -111,10 +120,8 @@ describe("ntfy urgent mapping", () => { { serverUrl: "https://ntfy.example.com", topic: "t" }, payload({ urgent: true }), ); - const [, init] = safeFetchMock.mock.calls[0]; - const headers = (init as { headers: Record }).headers; - expect(headers.Priority).toBe("5"); - expect(headers.Tags).toContain("warning"); + expect(ntfyMessage(0).priority).toBe(5); + expect(ntfyMessage(0).tags).toContain("warning"); }); it("non-urgent SYSTEM_ALERT → default priority, no warning tag", async () => { @@ -123,13 +130,8 @@ describe("ntfy urgent mapping", () => { { serverUrl: "https://ntfy.example.com", topic: "t" }, payload(), ); - const headers = ( - safeFetchMock.mock.calls[0][1] as { - headers: Record; - } - ).headers; - expect(headers.Priority).toBe("default"); - expect(headers.Tags).not.toContain("warning"); + expect(ntfyMessage(0).priority).toBe(3); + expect(ntfyMessage(0).tags).not.toContain("warning"); }); it("discreet urgent keeps the generic tag (no warning leak)", async () => { @@ -138,16 +140,11 @@ describe("ntfy urgent mapping", () => { { serverUrl: "https://ntfy.example.com", topic: "t" }, payload({ urgent: true, discreet: true }), ); - const headers = ( - safeFetchMock.mock.calls[0][1] as { - headers: Record; - } - ).headers; - expect(headers.Priority).toBe("5"); - expect(headers.Tags).toBe("reminder"); + expect(ntfyMessage(0).priority).toBe(5); + expect(ntfyMessage(0).tags).toEqual(["reminder"]); }); - it("does not send ntfy action headers to a managed Guardian", async () => { + it("does not send ntfy actions to a managed Guardian", async () => { safeFetchMock.mockResolvedValue({ ok: true, status: 200 }); await sendViaNtfy( { serverUrl: "https://ntfy.example.com", topic: "t" }, @@ -157,12 +154,7 @@ describe("ntfy urgent mapping", () => { recipientUserId: "guardian-1", }), ); - const headers = ( - safeFetchMock.mock.calls[0][1] as { - headers: Record; - } - ).headers; - expect(headers.Actions).toBeUndefined(); + expect(ntfyMessage(0).actions).toBeUndefined(); }); }); @@ -245,10 +237,7 @@ describe("APNs-less instance degrades gracefully", () => { expect(ntfy.ok).toBe(true); expect(web.ok).toBe(true); - expect( - (safeFetchMock.mock.calls[0][1] as { headers: Record }) - .headers.Priority, - ).toBe("5"); + expect(ntfyMessage(0).priority).toBe(5); expect( (generateRequestDetailsMock.mock.calls[0][2] as { urgency?: string }) .urgency, diff --git a/src/lib/notifications/senders/ntfy.ts b/src/lib/notifications/senders/ntfy.ts index 13b1fab78..96c905670 100644 --- a/src/lib/notifications/senders/ntfy.ts +++ b/src/lib/notifications/senders/ntfy.ts @@ -41,48 +41,24 @@ export async function sendViaNtfy( // can open. let policyReason: OriginReason | null = null; try { - const url = `${config.serverUrl.replace(/\/$/, "")}/${encodeURIComponent(config.topic)}`; - - // v1.18.4 — an explicitly urgent payload escalates to ntfy's top - // priority (`5` / max) so it bypasses the relay's batching and surfaces - // loudest; MEDICATION_REMINDER keeps its long-standing `high` (4); the - // rest stay `default` (3). `urgent` also adds a tag so the lock-screen - // entry reads as an alert (unless discreet privacy collapses tags). - const urgent = isUrgentPayload(payload) && payload.urgent === true; - const priority = urgent - ? "5" - : payload.eventType === "MEDICATION_REMINDER" - ? "high" - : "default"; - const eventTag = payload.discreet - ? "reminder" - : payload.eventType.toLowerCase().replace(/_/g, "-"); + // Published as JSON to the server root rather than as a body with + // headers to the topic URL. A header carries only Latin-1 (`fetch` + // refuses anything above U+00FF, and ntfy reads a Latin-1 byte as invalid + // UTF-8), so a Korean title or a Polish action label had to travel as an + // RFC 2047 encoded word, and an `Actions` header encoded whole, URLs and + // all, is not a form ntfy's action parser is documented to accept, so a + // reminder with an Italian, Korean or Polish label risked a 400 and never + // arrived. A JSON body is UTF-8 end to end and has no header encoding + // at all (https://docs.ntfy.sh/publish/#publish-as-json). + const url = `${config.serverUrl.replace(/\/$/, "")}/`; + const message = buildNtfyMessage(config.topic, payload); const headers: Record = { - Title: ntfyHeaderValue(plainPushText(payload.title, payload.eventType)), - Priority: priority, - // Discreet mode (cycle privacy): the X-Tags header is visible on the - // lock screen, so collapse it to a generic tag instead of leaking the - // cycle event name. An urgent (non-discreet) event also carries a - // `warning` tag so the entry renders as an alert. - Tags: urgent && !payload.discreet ? `warning,${eventTag}` : eventTag, + "Content-Type": "application/json", }; - if (config.authToken) { headers["Authorization"] = `Bearer ${config.authToken}`; } - - // Tapping the notification opens this URL (ntfy `Click`). Absent when - // the operator has not configured a public origin. - if (payload.tap?.url) { - headers["Click"] = payload.tap.url; - } - const actions = buildNtfyActionsHeader(payload); - if (actions) { - headers["Actions"] = ntfyHeaderValue(actions); - } - - // Strip HTML tags for ntfy (plain text only) + emoji on routine reminders - const body = plainPushText(stripHtml(payload.message), payload.eventType); + const body = JSON.stringify(message); // The DNS-rebinding pin (issue #217) sits on top of the input-time // guard enforced when the user saved the serverUrl; the authToken would @@ -201,47 +177,81 @@ export async function sendViaNtfy( /** ntfy shows at most three actions per notification. */ const NTFY_MAX_ACTIONS = 3; -/** - * A label safe inside the `Actions` header's short format: `,` and `;` - * separate fields and actions there, and a quote would open a quoted value. - */ -function ntfyActionLabel(label: string): string { - return label.replace(/[,;"'\r\n]/g, " ").trim() || "?"; +/** One `http` action button in ntfy's JSON form. */ +export interface NtfyHttpAction { + action: "http"; + label: string; + url: string; + method: "POST"; + clear: true; +} + +/** The JSON publish body (https://docs.ntfy.sh/publish/#publish-as-json). */ +export interface NtfyMessage { + topic: string; + title: string; + message: string; + priority: 3 | 4 | 5; + tags: string[]; + click?: string; + actions?: NtfyHttpAction[]; } /** - * The `Actions` header for the payload's quick answers, in ntfy's short - * format (https://docs.ntfy.sh/publish/#action-buttons): + * The message ntfy publishes for a payload. * - * http,