diff --git a/.planning/ios-coord/V054-QA-product-lead-findings.md b/.planning/ios-coord/V054-QA-product-lead-findings.md new file mode 100644 index 000000000..e6768fd32 --- /dev/null +++ b/.planning/ios-coord/V054-QA-product-lead-findings.md @@ -0,0 +1,282 @@ +# V0.5.4 — Product-Lead QA Findings (PR #190, v1.4.38.1) + +**Scope:** `4049c6c7^..c4f2e0bc` on `main` — APNs category + MOOD_REMINDER +event + CHANGELOG v1.4.38.1. +**Reviewer lens:** Product-Lead (positioning, UX coherence, Marc-Voice, +PII gate, iOS / self-hoster split, v1.5 carry-forward). +**Verdict:** GO with **2 High-severity follow-ups** scheduled into v1.5 +P1 (UI surface + locale parity). No Strategic-block. + +--- + +## Severity scale + +- **Strategic-block** — must not ship / must roll back. +- **High** — ship is OK; close inside the next release window (v1.5 P1). +- **Medium** — schedule for the next polishing slot; no urgency. +- **Low** — note, opportunistic fix. + +--- + +## Findings + +### 1. Positioning fit — Mood reminder vs. existing Coach / Insights / Briefing surfaces + +**Severity:** Low (positioning is sound). + +The MOOD_REMINDER feature is a **capture-prompt**, not an **insight**. +It nudges the user to enter raw data into the Stimmung tab so the +existing AI Coach + daily-briefing + correlations have inputs to chew +on. That's the *opposite* of duplication — without a daily capture +event the mood column on the correlations grid stays sparse, which +directly degrades the Coach's "evidence-grounded, multi-signal" +positioning (`feedback_ai_insights_differentiator.md`). + +Overlap checked against the three adjacent surfaces: + +| Surface | Trigger | Output | Overlap? | +|---|---|---|---| +| Daily briefing (`/insights`) | Request-time on page load | AI narrative | No — pulls *from* mood entries; doesn't request them | +| Med-reminder push | Per-schedule windows | Action buttons (Take / Snooze / Skip) | No — different event payload, different category | +| Compliance / anomaly pushes | Threshold-driven | Alert | No — reactive vs. the proactive 22:00 prompt | + +The Coach + Insights are unchanged. This release ships a **capture +funnel**, not a new differentiator-class feature. + +--- + +### 2. Daily 22:00 slot — conflict analysis + +**Severity:** Low. + +Existing scheduled windows reviewed in `reminder-worker.ts:118–193`: + +| Slot (Europe/Berlin) | Job | +|---|---| +| every 15 min | `medication-reminder-check` (per-user schedule windows; user's own med times decide actual fires) | +| every 60 min | Withings sync | +| 02:00 / 02:05 / 02:10 / 02:15 / 02:20 / 02:25 | Insights status jobs | +| 03:00 (Sun) | Data backup | +| 03:00 / 03:15 / 03:45 / 04:00 | Cleanup + drain + feedback aggregator | +| **22:00 local-tz (user-individual)** | **NEW** MOOD_REMINDER | + +22:00 is a clean choice: + +- **No collision** with the medication-reminder cron — that one is + schedule-driven (windows the user defined), not wall-clock-driven. + A user with a med slot at 22:00 *and* mood opt-in receives two + pushes that thread differently (`threadId = MEDICATION_REMINDER` vs. + `threadId = MOOD_REMINDER`), so iOS groups them separately. No + perceived noise. +- **No collision** with the daily briefing — briefing is request-time + on `/insights` page-mount, not push-triggered. +- **Wind-down slot** is research-aligned (Apple Health, Withings, Oura + all default mood / sleep prompts into the 21:00–22:30 window). The + inline comment captures this rationale (`mood-reminder.ts:30–35`). + +**One subtle item to flag for v1.5:** the 22:00 boundary is hard-coded +(`MOOD_REMINDER_LOCAL_HOUR = 22`). Some users will want 20:00 or +21:30. Add to v1.5 backlog as a Medium item (see §7). + +--- + +### 3. Opt-in default — "user controls everything" check + +**Severity:** none — fully respected. + +The feature is **double-gated**: + +- `EVENT_DEFAULT_ENABLED.MOOD_REMINDER = false` (per-event policy in + `types.ts:51`). +- `User.mood_reminder_enabled = false` by default (migration + `0069_v054_mood_reminder/migration.sql:16`). + +Either gate suffices to block dispatch. A fresh user gets **zero** +mood pushes until they explicitly opt in. This matches the +`PERSONAL_RECORD` pattern set in v1.4.25 and respects the +"user-controls-everything" instinct without forcing an onboarding +roadblock. + +The double-gate also means there is **no rollback risk**: if the +v1.5 UI ships broken, the worst case is "the toggle does nothing" +— the dispatcher stays silent. + +--- + +### 4. Marc-Voice review of DE/EN copy (and the four sibling locales) + +**Severity:** Low (DE/EN ship-ready; one weak FR string). + +| Locale | Title | Body | Verdict | +|---|---|---|---| +| de | "Stimmung erfassen" | "Wie geht es dir heute?" | **Marc-Voice clean** — direct, du-form, no AI mention, reads as Marc's authorship | +| en | "Log your mood" | "How are you feeling today?" | **Marc-Voice clean** — imperative title, conversational body | +| es | "Registrar el ánimo" | "¿Cómo te sientes hoy?" | OK | +| fr | "Enregistrer votre humeur" | "Comment vous sentez-vous aujourdhui ?" | **Typo: `aujourdhui` missing apostrophe** — should be `aujourd'hui`. Low-severity copy bug, fix in next release | +| it | "Registra il tuo umore" | "Come ti senti oggi?" | OK | +| pl | "Zapisz nastrój" | "Jak się dzisiaj czujesz?" | OK | + +No AI / Claude / agent / wave / phase language anywhere. No PII. No +jargon. All six locales pass the `feedback_marc_voice_english.md` +test. + +**Low-severity fix-list:** correct French apostrophe (`aujourd'hui`) +in `messages/fr.json` line 2994. + +--- + +### 5. CHANGELOG v1.4.38.1 entry + +**Severity:** Medium (one user-facing-jargon nit). + +Reviewed against `feedback_marc_voice_english.md` + +`feedback_no_pii_in_user_facing.md`: + +- **PII:** clean. No personal name, no health figures, no medication + references, no measurement counts. +- **Marc-Voice:** clean — reads as Marc's authorship, English, no + AI/agent/phase/wave language. +- **User-readability:** generally good. Two technical-jargon + spots that a self-hoster who hasn't read the schema diff won't + parse: + - `EVENT_DEFAULT_ENABLED` (line 27) — implementation symbol leaking + into release notes. Reword to "default-off at the event-policy + layer" or similar. + - `aps.mutable-content = 1` (line 14) — operator-facing detail; + fine to keep since the rest of the bullet is operator-scoped, but + flag it. +- **Self-hoster context:** the "iOS v0.5.4 push-notification + coordination" header tells self-hosters who don't run the iOS app + that this release is iOS-coordination scaffolding. The + Compatibility section adds the "older iOS builds render plain + alert" sentence which closes the loop. **Sufficient — no action + needed** for non-iOS self-hosters. + +**Medium fix:** swap the one identifier-leak in §Added for a +plain-English phrasing in the next release notes (v1.5 entry). + +--- + +### 6. iOS v0.5.4 dependency boundary + +**Severity:** Medium (documentation gap for self-hosters running +the server *without* the iOS app). + +What self-hosters need to know: + +- **Server-only operators:** the migration runs and the cron entry + registers, but with no APNs config + no iOS devices paired, every + cron tick is a no-op and the new event surface stays silent. **Zero + behavioural change.** ✅ Captured in CHANGELOG "Compatibility" + + the `loadApnsConfig` all-or-none guard already shipped in v1.4.23. +- **Mixed operators (server + iOS):** need the iOS app to register + `UNNotificationCategory` identifiers `MEDICATION_REMINDER` and + `MOOD_REMINDER` at launch. Without that, iOS renders plain alerts. + Captured in PR report (`V054-server-apns-mood-report.md:104–108`) + but **not** in CHANGELOG. +- **The `scheduledAt` ISO field** on med-reminder metadata is now + always emitted. Older iOS builds that don't read the field ignore + it. **No documentation needed** (additive metadata is the existing + iOS-coord contract). + +**Medium fix for v1.5 release notes:** add a one-liner that calls +out the iOS app must register the new category identifier to +surface the mood reminder as actionable. Self-hosters running +HealthLog headless (no iOS, only web) get no change and need no +action — that's the safe default but worth saying explicitly once. + +--- + +### 7. Strategic gap — v1.5 carry-forward + +**Severity:** **High** — must land in v1.5 P1, otherwise the feature +is invisible to end users. + +The server side ships **fully wired**, but there is **no UI to flip +`User.mood_reminder_enabled`**. Confirmed by grep: + +- `moodReminderEnabled` appears only in: `prisma/schema.prisma`, the + worker handler, the test suite, and the auto-generated Prisma + client. **Zero references in `src/app/**` (no Settings page, no + API write route, no iOS HTTP endpoint).** +- The existing notifications-preferences API + (`src/app/api/notifications/preferences/route.ts`) covers the + per-channel × per-event matrix only — it doesn't surface a + per-user boolean flag like `moodReminderEnabled`. This is the + same gap the PR report calls out at line 109–114. +- The `/notifications` page renders the per-channel matrix from + `EVENT_TYPES`, so MOOD_REMINDER will appear there as an event + row — but ticking it on a channel won't help: the worker also + requires `User.moodReminderEnabled = true`. Two-toggle confusion + is the predictable end-user failure mode. + +**v1.5 carry-forward (must-have for the iOS sprint):** + +1. **iOS-side:** Settings → Notifications toggle for "Daily mood + reminder" that PATCHes a new endpoint + (`PATCH /api/me/preferences` or extend + `/api/notifications/preferences` with a per-user-flag branch). +2. **Web parity:** mirror the same toggle on + `/settings/notifications` so non-iOS self-hosters can opt their + Telegram / ntfy / Web-Push channels into the daily nudge. The + server is channel-agnostic — withholding the toggle from web + would be a regression vs. the dispatcher's actual capability. +3. **Translation key parity:** add + `notifications.eventMoodReminder` label key in all six locales + so the `/notifications` matrix row has a human-readable label + (the function at `src/app/notifications/page.tsx:73` will derive + `notifications.eventMoodReminder` and fall through to the raw + key if missing — currently missing for MOOD_REMINDER in *all + six* `messages/*.json` files). +4. **Configurable hour:** lift `MOOD_REMINDER_LOCAL_HOUR = 22` to + a per-user preference (default 22) so users in shift-work or + different cultural wind-down rhythms can move the slot. + Medium-priority — server-side default is already opinionated; + v1.5 P2 is fine. + +--- + +## Missing-for-next-release list + +Listed in priority order for the v1.5 P1 iOS sprint: + +1. **[High] iOS Settings toggle** for `moodReminderEnabled` + the + matching server PATCH route. Without this the feature is dead + code for end users. +2. **[High] Web Settings parity toggle** at + `/settings/notifications` so self-hosters without an iPhone can + opt their existing channels into the daily nudge. Same PATCH + route as #1. +3. **[High] Translation key** `notifications.eventMoodReminder` in + all six `messages/*.json` so the matrix row on + `/notifications` shows a label rather than the camel-cased key. +4. **[Medium] Configurable reminder hour** — lift `22` to a + per-user preference. +5. **[Medium] French typo** `aujourdhui` → `aujourd'hui` in + `messages/fr.json:2994`. +6. **[Medium] CHANGELOG language clean-up** — drop the + `EVENT_DEFAULT_ENABLED` identifier leak from future entries that + touch the same surface. +7. **[Low] Operator note in v1.5 release notes** explicitly calling + out that headless self-hosters (web-only, no iOS app) need no + action — the feature is silent without paired devices. + +--- + +## Sign-off + +**Verdict:** GO. The release is strategically coherent (capture +funnel that feeds the existing differentiator), respects the opt-in +default pattern, ships Marc-Voice-clean copy, leaks no PII, and +documents the iOS dependency adequately in the PR report (with a +small CHANGELOG gap noted in §6). + +**Conditions:** + +- The four items marked **[High]** above MUST land in v1.5 P1. + Until then the user-visible footprint of v1.4.38.1 is zero on the + iOS side and exactly one un-labelled row in the web matrix that + doesn't actually flip the dispatcher. +- The migration is additive + idempotent — rollback path is "leave + the column at default false; the cron remains a no-op". No + rollback risk. diff --git a/.planning/ios-coord/V054-QA-security-findings.md b/.planning/ios-coord/V054-QA-security-findings.md new file mode 100644 index 000000000..4dc1e832c --- /dev/null +++ b/.planning/ios-coord/V054-QA-security-findings.md @@ -0,0 +1,320 @@ +# V054 IT-Security Review — PR #190 (4049c6c7 + c4f2e0bc) + +Scope: APNs `category` + `MOOD_REMINDER` event-type. iOS coord patch on `main`. + +Files inspected: + +- `messages/{de,en,es,fr,it,pl}.json` (only de/en touched; rest unchanged) +- `prisma/migrations/0069_v054_mood_reminder/migration.sql` +- `prisma/schema.prisma` (User.moodReminderEnabled + MoodReminderDispatch) +- `src/lib/jobs/mood-reminder.ts` + test +- `src/lib/jobs/reminder-worker.ts` (queue registration + scheduledAt patch) +- `src/lib/notifications/senders/apns.ts` + test +- `src/lib/notifications/types.ts` + +Method: read diff, trace dispatcher cascade for APNs delivery, search for opt-in write +paths, grep for log surfaces that touch device tokens or signing keys, manually +walk migration 0069 against the multi-writer / cascade rules. + +Severity legend: Critical (block release) / High (must fix this release) / +Medium (fix before iOS GA) / Low (cosmetic or hardening). + +--- + +## Severity counts + +- Critical: 0 +- High: 2 +- Medium: 4 +- Low: 3 + +--- + +## H-1 — MEDICATION_REMINDER APNs payload exposes medication name + IDs to Apple-servers and lock-screen + +File: `src/lib/notifications/senders/apns.ts:240-277` (push composition) + +`src/lib/jobs/reminder-worker.ts:594-608` (caller; pre-existing) + +`src/lib/notifications/senders/apns.ts:391-411` (per-device send in `sendViaApns`). + +What ships: + +- `aps.alert.title` = `getPhaseMessage(...).title` which is templated with + `{ medName }` — the medication's display name flows literally into the alert + title. Apple's APNs servers see this in cleartext at the HTTP/2 boundary + (TLS in transit, but Apple terminates) and iOS renders it on the + lock-screen unless the user has globally hidden previews. +- `aps.alert.body` = `getPhaseMessage(...).message` which is templated with + `{ medName, doseInfo, timeWindow, minutes }` — same exposure as the title, + PLUS the dose string (e.g. "Mounjaro 7.5 mg"). The body shows in the + Notification Center preview row even when notification previews are + permission-gated. +- `aps` custom payload (`data`) carries `medicationId`, `scheduleId`, + `phase`, `date`, `scheduledAt`, and `replyMarkup` (a Telegram-shaped + `inline_keyboard` object). The custom payload is not rendered on the + lock-screen, but Apple's relay still sees it, and any future iOS + Notification Service Extension (mutable-content is forced on at + `apns.ts:409`) will receive the entire object. + +Why this is High and not Critical: HealthLog has historically routed the +same content through Telegram and Web Push, and Apple's relay TLS + opaque +server contract are industry-accepted. The new exposure is "now also on +Apple's network and iOS lock-screen", not "newly cleartext on the internet". +But it crosses two HealthLog norms: + +1. The codebase comments repeatedly call out lock-screen exposure as a + sensitive surface (see `MOOD_REMINDER` rationale at types.ts:38-52). +2. `replyMarkup` is a Telegram-specific field bleeding into the iOS payload + — pure leakage with zero iOS-side consumer. + +Remediation shape: + +- Server-side: in `apns.ts:sendViaApns()` construct the per-channel payload + inside the APNs sender (don't pass the dispatcher payload `data` through + raw). Whitelist the fields APNs actually needs: `eventType`, + `medicationId`, `scheduleId`, `scheduledAt`. Drop `replyMarkup`, `phase`, + `date` from the iOS leg. +- Lock-screen: for `MEDICATION_REMINDER`, set + `aps.alert.title-loc-key` + `loc-args` so iOS can render a generic + "Medication reminder" title when the user has previews hidden, while the + app still receives the real medName via `loc-args`. (Or simpler: + render the title as the generic "Medication reminder" + put medName in + `data` only, let the iOS app expand to the full string after unlock.) +- Decision lever for Marc: keep medName on the lock-screen (current UX + parity with Telegram / Web Push) vs hide-until-unlock. Both are + defensible — but `replyMarkup` leakage is not a UX choice and should be + removed unconditionally. + +--- + +## H-2 — `users.moodReminderEnabled` has no API write path → opt-in flow is missing + +Files: PR adds the column + worker-side read, but no Next.js route, no +form, no Zod schema, no Settings UI touches the flag. + +Verified by grep: +`grep -rn "moodReminderEnabled\|mood_reminder_enabled" src/app src/components` +returns zero non-generated matches. + +Security implication: with the flag at default `false`, the feature is +functionally inert in production until someone writes a UI/PATCH endpoint. +That part is safe. But the iOS coord patch frames this as the user-facing +opt-in, and the iOS app will presumably want to flip it via the existing +auth-cookie + CSRF middleware on `/api/user/settings` or similar. Whoever +wires that endpoint MUST: + +- Authenticate (session cookie or bearer iOS API token). +- Apply the same CSRF guard as the rest of `/api/user/*` (HealthLog uses + the shared `apiContext` wrapper — verify the eventual route uses it). +- Validate the body via Zod (a single boolean). +- NOT accept mass-toggle on behalf of another userId (the route handler + must read userId from session, never from request body). + +Remediation shape: file this as a non-negotiable acceptance criterion on +the future "POST /api/user/notification-prefs" task (or wherever the toggle +lands). Add an integration test that asserts: + +1. Unauthenticated request → 401. +2. Request with a different userId in body → ignored / 403. +3. Request without CSRF token (web) → 403. +4. Value coerced to boolean (no `"true"` string injection enabling). + +Severity High because the opt-in is the GDPR + ethical anchor of an +"emotionally-loaded" surface (per the schema comment), and shipping the +write endpoint without these guards would silently let a malicious page +opt the user into a daily push. + +--- + +## M-1 — `mood_reminder_dispatches` ledger has no retention policy + +File: `prisma/migrations/0069_v054_mood_reminder/migration.sql:18-30` + +`prisma/schema.prisma:1192-1212`. + +Growth rate: ~365 rows / opted-in user / year, kept forever. Each row +holds `userId`, `date`, `dispatchedAt` — low PII surface, but it is a +behavioural footprint ("at which dates did this user fail to log a mood +before 22:00"). + +GDPR posture: the row is technically health-adjacent (it derives from +mood-logging behaviour). Storing it indefinitely for users who later +disable the reminder leaks behavioural history. + +Remediation shape: + +- Add a daily cron (slot it next to the audit-log cleanup at 03:15) that + deletes `mood_reminder_dispatches` rows older than ~90 days. The dedup + contract only needs the current day's row to be live. +- Alternatively, set up a Postgres partition + drop-by-month, but for + ~365 rows/user/year the simple `deleteMany` with a date predicate is + plenty. +- Ensure account-deletion cascade already handles this (FK + `ON DELETE CASCADE` is set at migration line 36 — confirmed safe). + +Severity Medium: privacy-relevant, not security-exploitable. + +--- + +## M-2 — Mood-reminder dispatcher aborts the whole tick on a single bad timezone + +File: `src/lib/jobs/mood-reminder.ts:151-164` + `src/lib/timezone.ts:21-32`. + +`getLocalDateParts` calls `new Intl.DateTimeFormat(..., { timeZone: tz })` +which throws `RangeError: Invalid time zone specified` for unknown IANA +strings. The mood-reminder loop has no per-user `try` wrapper — one +corrupted `users.timezone` value (typo from a manual DB write, future +import bug) will throw out of `runMoodReminderTick`, propagate to +`handleMoodReminderCheck` → `evt.setError` → `throw err`, and the entire +tick rolls back. Every opted-in user downstream of the bad row is silently +skipped for that 15-min window. + +Reliability bug, not a classic security bug, but availability is part of +the security CIA triad and this is one rogue row → cohort-wide +notification outage. + +Remediation shape: wrap the per-user body in `try { ... } catch (err) { +evt.addWarning(...); continue; }` and increment a +`summary.skippedInvalidTimezone` counter. Mirrors the pattern the +medication-reminder loop uses (see `reminder-worker.ts` per-user error +handling). + +--- + +## M-3 — APNs key file read error logs full filesystem path + +File: `src/lib/notifications/senders/apns.ts:172-179`. + +``` +signingKey = readFileSync(/* turbopackIgnore: true */ keyFile, "utf8"); +// catch +getEvent()?.addWarning(`APNs key file read failed: ${message}`); +``` + +`fs.readFileSync` errors include the literal path (`ENOENT: no such file +or directory, open '/secrets/apns/key.p8'`) in `err.message`. This lands +in the wide-event log. If logs ever surface to a less-trusted boundary +(Coolify admin UI, support export, log shipping to a SaaS), the operator +discloses their secret-mount path. + +The signing key content itself is NOT in `err.message`, only the path. + +Remediation shape: log `err.code` (`ENOENT` / `EACCES` / `EISDIR`) without +the path, e.g. +`getEvent()?.addWarning("APNs key file read failed: " + (err.code ?? "read_failed"))`. + +Severity Medium because: + +1. Path is sensitive but not health-PII. +2. The warning fires only at process boot. +3. Wide-event log is operator-only today. + +--- + +## M-4 — APNs cascade priority means iOS push fires even when user globally disabled the channel via a different transport + +File: `src/lib/notifications/dispatcher.ts:115` (`channelPriority`) + +`src/lib/notifications/dispatcher.ts:126-145` (per-channel iteration). + +APNs is cascade-priority 0 (fires first). The per-event preference check at +line 124-132 reads `EVENT_DEFAULT_ENABLED[MOOD_REMINDER] = false`, so a +user has to flip `NotificationPreference.MOOD_REMINDER.enabled = true` for +the APNs channel for the push to surface. + +The double opt-in claim in the code comment ("a user has to opt in twice +before the server starts nudging them about an emotionally-loaded +surface") is therefore real ONLY for users who already have an APNS +channel row. Two scenarios where the comment lies: + +1. A user with NO `NotificationChannel(type=APNS)` row will see the + dispatcher skip APNs naturally — fine. +2. A user who paired an iPhone (APNS channel row created) and flipped + `moodReminderEnabled = true` but NEVER touched the per-channel + `NotificationPreference.MOOD_REMINDER.enabled` — the dispatcher + honours `EVENT_DEFAULT_ENABLED[MOOD_REMINDER] = false` and skips. Good. + +So the double opt-in actually holds in code today. BUT: the iOS coord +patch presumably ships an iOS Settings toggle that flips ONLY +`User.moodReminderEnabled` (not the per-channel preference row). If the +iOS app's toggle wires straight to the user-level flag without ALSO +creating the per-channel preference row, the worker enqueues the +notification via `dispatchNotification("MOOD_REMINDER")` and the +dispatcher swallows it because of the per-event default-off. Net: the +user toggled "on" in iOS, sees no push, blames the app. + +Not a security exposure — it's a "fail-closed-but-confusingly" interaction. +Worth a contract test once the iOS-facing endpoint exists, and a docs note +on the iOS coord plan that the endpoint must upsert BOTH: + +- `users.mood_reminder_enabled = true` +- `notification_preferences (channel=APNS, event=MOOD_REMINDER, enabled=true)` + +Severity Medium because shipping the iOS toggle without this upsert sets +up the user for a silent feature failure that looks like a bug. + +--- + +## L-1 — `dispatchedAt` second index `(user_id, dispatched_at)` is unused + +File: `prisma/migrations/0069_v054_mood_reminder/migration.sql:29-30` + +`prisma/schema.prisma:1209`. + +The handler only queries by `(userId, date)` (uses the unique index). No +code path uses the `(user_id, dispatched_at)` composite index. It costs +~1 row-write CPU and ~B-tree storage per dispatch; harmless today but +also no benefit. Remediation: drop the index in a follow-up migration OR +add the retention-cron predicate that would use it (`WHERE userId = $1 +AND dispatched_at < now() - 90 days`). + +Severity Low — minor hygiene. + +--- + +## L-2 — `MOOD_REMINDER` lacks a per-event `category` registration check in tests + +File: `src/lib/notifications/senders/__tests__/apns.test.ts:449-467`. + +The test pins that `note.category === "MOOD_REMINDER"`, but there is no +contract test against the iOS-coord doc that the app actually registered +a `MOOD_REMINDER` UNNotificationCategory. Server-side this is fine +(APNs ignores unknown categories per the apns.ts comment at line 401-403); +just a documentation observation. + +Severity Low — no server-side fix needed, flag for iOS sub-team. + +--- + +## L-3 — `provider.shutdown()` in `resetApnsForTesting` is fire-and-forget + +File: `src/lib/notifications/senders/apns.ts:191-196`. + +`void provider.shutdown()` discards the returned promise. In tests this +is fine, but if any production code path ever calls +`resetApnsForTesting()` (it's exported), the in-flight HTTP/2 streams +could be torn down mid-send. Today no prod caller exists; the export is +test-only. Consider renaming to `_resetApnsForTesting` or guarding with +`if (process.env.NODE_ENV !== "test") throw ...` so future refactors +don't accidentally pick it up. + +Severity Low. + +--- + +## Top 3 issues (Marc-facing brief) + +1. **H-1** — MEDICATION_REMINDER APNs payload puts medication name + + `replyMarkup` (Telegram inline-keyboard object) into Apple's relay + and onto the iOS lock-screen. `replyMarkup` is pure leakage; medName + is a UX choice that crosses HealthLog's lock-screen-exposure norm. + +2. **H-2** — `users.moodReminderEnabled` has no API write path in this + PR. The opt-in toggle endpoint that the iOS coord patch needs must + land with auth + CSRF + userId-from-session guards; pre-flag those + acceptance criteria. + +3. **M-1** — `mood_reminder_dispatches` grows ~365 rows/user/year + forever, storing a behavioural footprint of mood-logging gaps. Add a + 90-day retention cron alongside the audit-log cleanup at 03:15. + +No queue-injection surface found (cron registration is process-internal, +no API route enqueues `mood-reminder-check`). No new key material exposed +(signing key cached in process memory only, never logged). No device +tokens in logs. diff --git a/.planning/ios-coord/V054-QA-senior-dev-findings.md b/.planning/ios-coord/V054-QA-senior-dev-findings.md new file mode 100644 index 000000000..e99a0fa0f --- /dev/null +++ b/.planning/ios-coord/V054-QA-senior-dev-findings.md @@ -0,0 +1,167 @@ +# v0.5.4 iOS-coord PR #190 — Senior Code Review + +Scope: squash merge `4049c6c7` + version bump `c4f2e0bc` on `main`. + +## Verdict at a glance + +| Severity | Count | +|---|---| +| Critical | 1 | +| High | 4 | +| Medium | 5 | +| Low | 3 | + +Headline: the implementation is well-shaped, the test coverage is the strongest part, and the integration with the existing reminder-worker is clean. There is **one Critical**: the ledger row is written **before** `dispatchNotification` is called, but the dispatcher swallows every internal error — so a transient APNs / Telegram failure leaves the ledger marked "dispatched" forever and the user silently gets no push that day, and no push the next day until the ledger key rolls over. Two High-severity issues concern the i18n contract (four supported locales fall back to English instead of using their freshly added strings) and the 15-minute cron cadence producing up to four wide-event lines per user-per-day even when the handler has nothing to do. + +--- + +## Critical + +### C-1. Ledger row is committed before delivery; lost pushes never retry +File: `src/lib/jobs/mood-reminder.ts:202-229` +Also: `src/lib/notifications/dispatcher.ts:209-211` (the catch-all that masks the failure) + +The handler does: + +``` +moodReminderDispatch.create(...) // step A — commits the dedup row +dispatchImpl({ eventType: "MOOD_REMINDER", ... }) // step B — fire-and-forget +``` + +`dispatchNotification` is a `void` returning function whose outer `try/catch` swallows every error path inside it (`dispatcher.ts:209`). The dispatcher also internally treats `APNs not configured`, `no devices`, every channel `hardReject`, every transient failure, and the case where the user simply has zero enabled channels as silent. That means: + +* If the cron tick lands at 22:00 on a day where APNs is mid-rotation, the network is wobbly, the dispatcher throws on a DB connection blip, or the user happens to have no enabled channel yet, the ledger row is **still committed**. +* The dedup guard then correctly suppresses every subsequent tick in the 22:00 window and the next day's window stays open — but the user got nothing for *this* day. +* Worse: because `dispatchImpl` cannot be observed to fail, the handler reports `summary.dispatched += 1` even when the push never reached a single channel. The wide-event will read "we nudged the user" when in fact nothing went out. + +Recommended fix shape: +1. Move the ledger write to **after** the dispatch returns. Use a `try/finally` so a thrown dispatch still records the ledger (preventing a tight retry loop within the 15-min window if dispatcher latency briefly spikes), but also gate the ledger commit on at-least-one-channel-success. The cleanest path is to give the dispatcher a return value (`{ delivered: boolean }`) and only insert the ledger when `delivered === true`. +2. Failing that, accept the at-most-once trade-off but increment `summary.dispatched` *after* dispatch returns and split `summary.delivered` vs `summary.attempted` so the wide-event is honest about what happened. +3. Either way, the dispatcher's internal swallow needs a return-value signal — `dispatcher.ts:209-211` should bubble (or surface) the outcome enough that the mood-reminder handler can branch. + +This is the only finding that materially breaks the user-visible contract; everything else below is defence-in-depth or quality. + +--- + +## High + +### H-1. Locale resolver drops four of the six supported locales +File: `src/lib/jobs/mood-reminder.ts:60-62` + +``` +function resolveLocale(locale: string | null | undefined): Locale { + return locale === "en" || locale === "de" ? locale : defaultLocale; +} +``` + +`src/lib/i18n/config.ts:1` declares the supported set as `["de", "en", "fr", "es", "it", "pl"]`, and **this same PR adds `moodReminders.dailyTitle / dailyBody` strings for all four of the additional locales** (`messages/{es,fr,it,pl}.json`). A French user with `user.locale = "fr"` will receive the English copy, even though `messages/fr.json` has the translation sitting one resolver call away. + +This is also the only place where v0.5.4 narrows the supported-locale set — the existing reminder worker uses the full set via `getServerTranslator`. + +Recommended fix shape: import `locales` from `@/lib/i18n/config` and check membership against that array (or rely on `getServerTranslator` to fall back internally and just pass through the user's locale unchanged). + +Test gap: `buildMoodReminderPayload` only has three tests (`de`, `en`, `null/unknown`) — add a parameterised case across all six maintained locales so this never regresses. + +### H-2. 15-min cron cadence produces ~96 wide-event lines per day per user (4× the necessary fan-out and 96× the necessary background-event scaffolding) +File: `src/lib/jobs/reminder-worker.ts:185, 1142-1163` + +The cron is `*/15 * * * *`. Inside the 22:00 window for any user this means up to 4 ticks per user per day where `inWindow == true`, each running a Prisma `findUnique` against the ledger (cheap, but logged as a wide-event subtree). The remaining 92 ticks per day fire `withBackgroundEvent("job.mood_reminder", ...)` and execute a `findMany` over every opted-in user even though no user is in their 22:00 hour from the worker's perspective — there is no early-exit shortcut. + +For a fleet of N opted-in users we get: +* 96 ticks/day × 1 `user.findMany` each = 96 unnecessary scans. +* 96 wide-event roots/day. +* `localConcurrency=1` means these ticks serialise against each other (fine today, but the queue could starve if N grows). + +Recommended fix shape: either run the cron at `0 * * * *` (top-of-hour only — covers every IANA-hour boundary while cutting the noise 4×) and accept a worst-case 0–59 min latency on the *first* in-window tick (acceptable since the second hour-tick still gets the user the same day), or keep the 15-min cadence but early-exit the handler with a `SELECT 1 WHERE EXISTS (... AND user.timezone such that local hour == 22)` so the no-op ticks don't pull every row across the wire. + +If 15-min cadence is kept for the iOS contract reason, at minimum suppress the `withBackgroundEvent` root when zero candidates are in-window so the observability dashboards don't flood with no-op events. + +### H-3. `note.payload = { ...input.payload.data }` can collide with `aps` +File: `src/lib/notifications/senders/apns.ts:275-277` +Caller: `src/lib/notifications/senders/apns.ts:392-410` spreads arbitrary `payload.metadata` into `data`. + +`note.payload` becomes the top-level JSON adjacent to `aps`. If any caller (now or later) puts `aps`, `apns`, `apns-push-type`, or other Apple-reserved keys inside `metadata`, node-apn will serialise them at the JSON root, potentially clobbering the alert/category/sound `aps` block that the same code path just configured. Today only known callers exist (medication reminders + the new mood reminder), but `metadata` is `Record` — a future caller has zero compile-time protection. + +Recommended fix shape: strip known-reserved root keys (`aps`, `apns-*`) from `input.payload.data` before assigning; or assert and throw in `sendApnsPush` when a reserved key is present so the regression surfaces at the call site instead of at the user's lock-screen. + +### H-4. `MOOD_REMINDER` event is gated on `EVENT_DEFAULT_ENABLED = false` AND `moodReminderEnabled`, but the user-facing UI to flip either is not in this PR +File: `src/lib/notifications/types.ts:40-52`, `src/lib/jobs/mood-reminder.ts:80-82` +Plan ref: the schema comment at `prisma/schema.prisma:204-209` says "explicitly opt in from Settings → Notifications". + +Neither this PR (nor anything in `git grep -n moodReminderEnabled src/app`) wires a Settings toggle for `moodReminderEnabled`. So the v0.5.4 server contract ships with a user-invisible flag — operators or iOS clients have to flip it manually, and the per-event `NotificationPreference` row also has to be created with `enabled = true` for the dispatcher to even visit any channel (because the per-event default is `false`). + +This means the feature is **dark-launched**, which may be intentional for the iOS coordination patch. But there is no operator-facing note, no admin route, and no API endpoint in this PR to set the flag. If iOS v0.5.4 is expected to drive the toggle, that contract is missing on the server. + +Recommended fix shape: either ship a `PATCH /api/users/me/notification-prefs` extension that accepts `moodReminderEnabled`, or document in the PR description that iOS is the only caller and link the corresponding iOS PR. At minimum, an integration test that hits the (planned) endpoint would make the contract explicit. + +--- + +## Medium + +### M-1. Cron `*/15 * * * *` interacts with `tz: "Europe/Berlin"` schedule — fine today, but the comment is misleading +File: `src/lib/jobs/reminder-worker.ts:181-188, 1692-1693` + +The comment says "every 15 minutes (same cadence as the medication-reminder loop)". The `boss.schedule(... { tz: "Europe/Berlin" })` is irrelevant for `*/15 * * * *` (which fires every 15 wall-clock minutes regardless of tz reference), but a future maintainer might assume the tz applies. The handler does NOT depend on the tz parameter — the per-user local-hour check inside `evaluateMoodReminderWindow` is the only timezone-aware piece. Add a comment clarifying that the `tz` on the schedule entry is inert for this cron. + +### M-2. `evaluateMoodReminderWindow` returns `localHour: -1` for opt-out users +File: `src/lib/jobs/mood-reminder.ts:80-82` + +Returning a sentinel `-1` for a "real" `localHour` is a smell — the field's type is `number` and any downstream consumer that does arithmetic (e.g. metric labelling) will silently produce garbage. Either change the return to a discriminated union (`{ fire: true; localDate: string; localHour: number } | { fire: false; reason: "opt_out" | "outside_window" }`) or make `localHour` nullable. + +### M-3. `localDate` formatting uses `padStart(4, "0")` on `year` but the YEAR is already 4-digit +File: `src/lib/jobs/mood-reminder.ts:85-87` + +Cosmetic. `parts.year` from `getLocalDateParts` is parsed from a 4-digit Intl format. `padStart(4, "0")` is dead defensive code that confused me on read — drop it or replace with `String(parts.year)`. + +### M-4. The handler scans every opted-in user on every tick, no hour-aware SQL prefilter +File: `src/lib/jobs/mood-reminder.ts:141-149` + +For N opted-in users this is N rows back from Postgres per tick, then a per-user CPU loop to do Intl formatting. Today N is small; once Marc onboards external users this becomes a hot loop running 96 times a day. A pre-filter — even a coarse one such as `WHERE timezone IN (zones where local-hour is 22 right now)` — would shrink the wire payload. + +Recommended fix shape: precompute the set of IANA zones whose current local hour equals `MOOD_REMINDER_LOCAL_HOUR` once per tick (≤ ~600 zones, easily cached), then `WHERE timezone = ANY ($1)`. Or push the whole decision down with a raw SQL aggregator á la the rollup readers. + +### M-5. Unit tests use a stubbed Prisma; no integration test asserts the unique constraint actually fires +File: `src/lib/jobs/__tests__/mood-reminder.test.ts:320-344` + +The "lost P2002 race" test fakes the error by string. There is no testcontainer test that proves the real `mood_reminder_dispatches_user_id_date_key` unique index rejects the second insert. The handler's idempotency claim is therefore unverified end-to-end. Add a Pg-backed integration test that inserts twice in parallel and asserts the second receives `P2002`. + +--- + +## Low + +### L-1. Migration is `additive + IF NOT EXISTS` but the `DO $$ ... EXCEPTION WHEN duplicate_object` block ignores other errors +File: `prisma/migrations/0069_v054_mood_reminder/migration.sql:32-39` + +The FK creation block catches *only* `duplicate_object`, which is correct. But the `IF NOT EXISTS` on the unique index + table + column makes the whole migration idempotent — except for the FK, which would still throw on any non-duplicate failure. That's the right behaviour, just worth a one-line comment that any error outside `duplicate_object` is intentional and should fail the migration loudly. + +### L-2. `MoodReminderDispatch` has no retention / cleanup job +File: `prisma/schema.prisma:1203-1211` + +One row per (user, day) — ~365 rows/user/year. There's no nightly cleanup analogous to `auditLogCleanup` or `idempotencyCleanup`. Not urgent (tiny rows, indexed by `userId`), but a 5-year-old row has zero operational value. Add to backlog: prune dispatches older than e.g. 90 days. + +### L-3. Timezone-change-mid-day edge case +File: `src/lib/jobs/mood-reminder.ts:83-92` + +If a user changes their `timezone` between 22:00-local-old and 22:00-local-new on the same calendar date, they could receive two pushes (one per zone) because `decision.localDate` differs across the two zones. Vanishingly rare; mention in the planning notes only if you hit it in field reports. + +--- + +## What was done well + +- Test coverage on the pure helpers (`evaluateMoodReminderWindow`, `buildMoodReminderPayload`) is exactly the right shape — pinned boundary cases (21:59 / 22:00 / 23:00), a cross-timezone case (NY DST), opt-out gating, and locale fallback all covered. +- The choice to put the dedup check **before** the mood-entry check is correctly justified by index cost in the comment at `mood-reminder.ts:168-172`. That kind of reasoning-in-the-code pays back when the next maintainer touches it. +- The APNs `category` / `mutableContent` plumbing is conservative — node-apn's `category` setter is poked via the type-cast escape hatch with a clear comment explaining why, and `mutableContent` ships now as a no-op so the iOS NSE work can land without a server roundtrip. Both are textbook forward-compat decisions. +- The `MOOD_REMINDER_QUEUE` is correctly added to `allQueues` (the W7c lesson) — this is the exact failure mode the W10 multi-agent QA caught in v1.4.37 (drain queue not registered), and the comment at `reminder-worker.ts:1624-1628` calls it out by name. +- New APNs tests for `MOOD_REMINDER` and `MEDICATION_REMINDER` category propagation pin the v0.5.4 iOS contract well (`apns.test.ts:425-467`). +- Migration is additive, defaults are conservative (`mood_reminder_enabled DEFAULT FALSE`, `MOOD_REMINDER` per-event default also `false`), so existing users are byte-identical to v1.4.38 behaviour. + +--- + +## Recommended sequencing + +1. **Fix C-1 before any production traffic hits the cron**. The fail-closed-then-silent behaviour is the only finding that affects user trust. +2. Fix H-1 before any non-EN/DE user toggles the opt-in. +3. Address H-2 + M-1 + M-4 together as a perf/observability follow-up; one PR, no behaviour change. +4. H-3 + L-1 can ride a hygiene release. +5. M-5 belongs in the next testcontainer pass. +6. L-2 + L-3 onto the v1.4.39 backlog. diff --git a/.planning/ios-coord/V054-QA-simplifier-findings.md b/.planning/ios-coord/V054-QA-simplifier-findings.md new file mode 100644 index 000000000..132c1e070 --- /dev/null +++ b/.planning/ios-coord/V054-QA-simplifier-findings.md @@ -0,0 +1,86 @@ +# V0.5.4 PR #190 — Simplifier QA Findings (READ-ONLY) + +Scope: commit `4049c6c7` + version bump `c4f2e0bc` on `main`. +Files in scope: `apns.ts` (+test), `notifications/types.ts`, `mood-reminder.ts` (+test), `reminder-worker.ts` (MOOD_REMINDER block only). + +No code modified. All findings below are candidates for a future simplification pass. + +--- + +## Safe simplifications (low risk, behaviour-preserving) + +### S1 — Drop redundant `moodReminderEnabled` round-trip in `runMoodReminderTick` +**File**: `src/lib/jobs/mood-reminder.ts:141-159` +The Prisma `findMany` already filters with `where: { moodReminderEnabled: true }`, then `select`s `moodReminderEnabled: true` only to pass it back into `evaluateMoodReminderWindow`, which short-circuits when it's `false` — an impossible branch at this call site. +Proposed shape: drop `moodReminderEnabled` from the `select`, and drop the `moodReminderEnabled` arg from the helper invocation. (The helper itself stays — `evaluateMoodReminderWindow` is also called directly by tests with `moodReminderEnabled: false` to assert the opt-out behaviour.) + +### S2 — `evaluateMoodReminderWindow` `localHour: -1` sentinel never observed +**File**: `src/lib/jobs/mood-reminder.ts:79-82` +The opted-out branch returns `localHour: -1` purely to satisfy the return shape. No production caller reads `localHour` (only three test asserts touch it, none from the opted-out branch). Could return `localHour: 0` or just drop `localHour` from that branch's payload — but `0` is the simplest in-place tweak. + +### S3 — `MoodReminderCandidate` interface is over-specified for its only use +**File**: `src/lib/jobs/mood-reminder.ts:44-49` +The interface is `Pick<>`-ed exactly once and never used as a whole-type. After applying S1, the `id` + `moodReminderEnabled` fields are dead — the helper only needs `timezone`. The interface can be deleted and the helper can take `{ timezone: string }` inline. The `MoodReminderSummary` interface is fine (it's the orchestrator's return type). + +### S4 — `if (!decision.fire || !decision.localDate)` second clause exists only for TS narrowing +**File**: `src/lib/jobs/mood-reminder.ts:161` +Logically `decision.localDate` is non-null iff `decision.fire === true`. The redundant second clause is load-bearing for TypeScript's flow analysis after the `continue`. **Leave as-is** — narrowing without it would force `!` on every later `decision.localDate` read. Listed for completeness. + +### S5 — `padStart` chain in `evaluateMoodReminderWindow` re-formats data Intl already 2-digited +**File**: `src/lib/jobs/mood-reminder.ts:85-87` +`getLocalDateParts` parses the values back to `Number` (`year/month/day`), so the helper has to re-pad. A `.toString().padStart(2, "0")` per field is fine, but the whole literal could collapse to a single template-literal helper or use `String(parts.month).padStart(...)`. Cosmetic only. + +### S6 — JSDoc on `getProvider` neighbour comment block can drop "Provider lifecycle" prose +**File**: `src/lib/notifications/senders/apns.ts:1-23` (file-header) +Out-of-scope per directive (pre-existing comment, not a new line in this PR). **Skip.** + +### S7 — `void jobs;` + try/catch + `recordError()` + rethrow is the established shape +**File**: `src/lib/jobs/reminder-worker.ts:632-660` (new `handleMoodReminderCheck`) +Matches every other handler in the file 1:1. **No simplification — keep.** Noted to confirm pattern consistency. + +--- + +## Worth-considering (judgement call, may regress clarity) + +### W1 — `resolveLocale` silently downgrades fr/es/it/pl to default +**File**: `src/lib/jobs/mood-reminder.ts:60-62` +`return locale === "en" || locale === "de" ? locale : defaultLocale;` +The PR added `moodReminders.dailyTitle/dailyBody` to all six locales (`fr.json:2991`, `es.json:2991`, `it.json:2991`, `pl.json:2991`), but this helper only ever resolves to `en` or `de`. fr/es/it/pl translations are dead at runtime. **Borderline behaviour change** — fixing it expands the supported-locale set. Probably the correct fix is `locales.includes(locale as Locale) ? locale as Locale : defaultLocale` to honour the JSON files that already exist. Surfaces because the v1.4.38 i18n wave (commits `d95820ab`/`9bd67564`/`7f490af7`) shipped those strings on purpose. **Flag for Marc — choose: (a) tighten resolver to accept all 6 locales, or (b) delete the unused 4-locale strings to remove the mismatch.** + +### W2 — Test setup duplication across 6 `runMoodReminderTick` cases +**File**: `src/lib/jobs/__tests__/mood-reminder.test.ts:186-381` +Every test rebuilds the same `FakePrismaState` shape with one `u-1` candidate, empty `moodEntries`/`dispatches`/`raceUserIds`, plus the `dispatch = vi.fn(async () => {})` line. A `beforeEach` returning `{ state, prisma, dispatch }` would cut ~12 lines × 6 tests = ~70 LOC. Trade-off: each test currently reads top-to-bottom without scrolling back to setup, which is the project's preferred style. **Marginal win — defer unless adding more cases.** + +### W3 — `MoodReminderCandidate` + `MoodReminderSummary` exports are internal-only +**File**: `src/lib/jobs/mood-reminder.ts:44, 51` +Both interfaces are `export`ed but consumed exclusively inside `mood-reminder.ts` (`MoodReminderSummary` for the orchestrator's return type, used by `reminder-worker.ts`'s `handleMoodReminderCheck`). After S3, `MoodReminderCandidate` is dead and can be deleted. `MoodReminderSummary` stays exported because `reminder-worker.ts` reads its fields (`summary.candidatesScanned` etc.) into the wide-event payload — but the file currently doesn't import the type. Re-checking: `handleMoodReminderCheck` infers the type from the function return, so the export is unused in practice. Could drop `export`, but the cost of leaving it is zero. + +### W4 — APNs `category` cast comment is 13 lines for a one-line cast +**File**: `src/lib/notifications/senders/apns.ts:251-266` +The cast `(note as unknown as { category: string }).category = ...` is annotated with a 13-line comment explaining why. Could compress to 2-3 lines ("node-apn 8.1 d.ts omits the public `category` setter; runtime writes through to `aps.category` unchanged"). The detail is valuable when a future engineer upgrades node-apn — keep or trim is a taste call. **Recommend keeping** given Marc's prose-over-brevity preference in the codebase. + +### W5 — `category` is set from `payload.eventType` in `sendViaApns`, then `threadId` from the same value +**File**: `src/lib/notifications/senders/apns.ts:398-404` +Three fields (`threadId`, `category`, `collapseId`) all receive `payload.eventType`. Could extract `const eventTypeId = payload.eventType;` to make the "same identifier doubles as three keys" pattern explicit. **Cosmetic.** + +### W6 — `[winH, winM] = schedule.windowStart.split(":").map(Number)` lacks NaN guard +**File**: `src/lib/jobs/reminder-worker.ts:584-587` +If `windowStart` is malformed, `scheduledAtIso` becomes `"Invalid Date".toISOString()` → throws. The validation happens upstream on schedule write so it's safe in practice, but a defensive `Number.isFinite(winH * winM)` would prevent a corrupt-row crash. **Out-of-scope for simplification — this is a hardening item, not a simplification.** + +--- + +## Out-of-scope (touched in PR but not the new code) + +- `handleWithingsSleepSync` signature was reflowed by Prettier (apns.ts diff `@@ -717`). Not a logic change. +- `handlePrDetection` payload destructure was reflowed by Prettier (`@@ -1248`). Not a logic change. +- `workerLog("error", "Failed to reconcile orphan ImportJob rows", err)` collapsed to one line (`@@ -1573`). Not a logic change. + +--- + +## Count summary + +- Safe simplifications: **3 actionable** (S1, S2, S3) + 1 cosmetic (S5) + 2 noted/skip (S4, S6, S7). +- Worth-considering: **5** (W1 = behaviour question, W2-W5 = taste, W6 = hardening not simplification). +- Out-of-scope formatting churn: 3 spots, no action. + +Total real candidates: **8** (3 safe + 5 worth-considering). diff --git a/.planning/ios-coord/V054-QA-specialist-findings.md b/.planning/ios-coord/V054-QA-specialist-findings.md new file mode 100644 index 000000000..50930ae58 --- /dev/null +++ b/.planning/ios-coord/V054-QA-specialist-findings.md @@ -0,0 +1,187 @@ +# V054 — QA specialist findings (APNs + Prisma migration) + +**Scope.** Squash-merge `4049c6c7` ("feat(notifications): APNs category + MOOD_REMINDER event for iOS v0.5.4 (#190)") plus the v1.4.38.1 chore bump `c4f2e0bc` on `main`. Files in scope: `prisma/migrations/0069_v054_mood_reminder/migration.sql`, `prisma/schema.prisma`, `src/lib/notifications/senders/apns.ts` + test, `src/lib/notifications/types.ts`, `src/lib/jobs/mood-reminder.ts` + test, `src/lib/jobs/reminder-worker.ts`, `messages/{de,en,es,fr,it,pl}.json` (moodReminders namespace only). + +Two specialist lenses applied: APNs payload schema (Apple 2024+) and Prisma migration idempotency / referential integrity. READ-ONLY review — no code modified, no migrations executed. + +--- + +## Severity counts + +| Severity | Count | +|----------|-------| +| Critical | 0 | +| High | 2 | +| Medium | 4 | +| Low | 3 | + +--- + +## Findings + +### H1 — `resolveLocale` drops es/fr/it/pl, defeats the just-shipped translations [High] + +**File.** `src/lib/jobs/mood-reminder.ts:60-62` + +```ts +function resolveLocale(locale: string | null | undefined): Locale { + return locale === "en" || locale === "de" ? locale : defaultLocale; +} +``` + +**Issue.** The HealthLog locale set declared in `src/lib/i18n/config.ts:1` is `["de", "en", "fr", "es", "it", "pl"]`. `getServerTranslator` correctly serves all six locales (it indexes into `allMessages[locale]` and falls back to English on missing keys). However `buildMoodReminderPayload` funnels the user's locale through `resolveLocale`, which whitelists only `en` and `de` and downgrades everything else to `defaultLocale` (= `en`). + +Net effect on iOS v0.5.4 ship: the Spanish / French / Italian / Polish `moodReminders.dailyTitle` + `dailyBody` strings introduced in the same PR (`messages/{es,fr,it,pl}.json:2991-2992`) are **unreachable** — every non-de/non-en user will receive the English copy regardless of `User.locale`. The translations are inert dead code today. + +The unit test `buildMoodReminderPayload — falls back to the app default for null / unknown locales` (mood-reminder.test.ts:171-177) passes for the wrong reason: it asserts that the `null` and `"zz-ZZ"` fallbacks land on the same string, which they do — but `"fr"`, `"es"`, `"it"`, `"pl"` also currently land on that same English fallback. + +**Fix shape.** Replace the explicit allow-list with a check against `locales` from `@/lib/i18n/config`, then cast the validated input to `Locale`: + +```ts +import { locales } from "@/lib/i18n/config"; +function resolveLocale(locale: string | null | undefined): Locale { + return (locales as readonly string[]).includes(locale ?? "") + ? (locale as Locale) + : defaultLocale; +} +``` + +Add three test cases asserting `fr` → `"Enregistrer votre humeur"`, `es` → `"Registrar el ánimo"`, `it` → `"Registra il tuo umore"`, `pl` → `"Zapisz nastrój"`. + +### H2 — French copy missing apostrophe: `aujourdhui` → `aujourd'hui` [High] + +**File.** `messages/fr.json:2993` + +```json +"dailyBody": "Comment vous sentez-vous aujourdhui ?" +``` + +**Issue.** Every other French string in the codebase writes `aujourd'hui` (curly apostrophe) or `aujourd'hui` (straight apostrophe) — see `fr.json:400, 529, 625, 979`. The new key is missing the apostrophe entirely, surfacing on iOS push notifications as the misspelled word. This is the user-visible mood reminder body — first impression for French users opting in. + +**Fix shape.** Change to `"Comment vous sentez-vous aujourd'hui ?"` (curly apostrophe, matching the existing convention in `fr.json:400, 529`). Once H1 is fixed and French users can actually receive the string, this becomes immediately visible. + +### M1 — node-apn d.ts cast pokes the `.category` setter directly; safer to write `aps.category` [Medium] + +**File.** `src/lib/notifications/senders/apns.ts:265` + +```ts +(note as unknown as { category: string }).category = input.payload.category; +``` + +**Issue.** The cast comment explains the d.ts gap accurately and node-apn 8.1's runtime does honour `.category` as a setter that writes through to `aps.category`. The fragility is the implicit dependency on an undocumented setter that the maintainer could rename in a minor release without breaking the d.ts contract (the contract doesn't mention it). A defensive belt-and-braces alternative is to write `note.rawPayload = { aps: { category: ... } }`, which is part of node-apn's documented API, or to fall back to a payload property assignment if the setter were ever removed. + +This is a Medium because the behaviour is correct today (an explicit test pins it — `apns.test.ts:425-447`) but the abstraction layer is unusual for this codebase and a node-apn upgrade is the breakage vector. + +**Fix shape.** Either (a) accept the runtime coupling and add an explicit comment-block changelog note pinning the supported node-apn version range, or (b) refactor to use node-apn's `note.aps.category = ...` access path, which is in the published `Aps` interface. Today's test would still pass. + +### M2 — Mood-reminder `data: null/undefined` ⇒ no custom-keys envelope [Medium] + +**File.** `src/lib/notifications/senders/apns.ts:275-277` + +```ts +if (input.payload.data) { + note.payload = { ...input.payload.data }; +} +``` + +**Issue.** The lower-level `sendApnsPush` only attaches the `data` envelope when `input.payload.data` is truthy. The dispatcher-level entry point `sendViaApns` (line 393-396) always sets `data: { eventType: ..., ...metadata }`, so the contract holds for the production path. But a future caller of `sendApnsPush` that omits `data` would ship a push with no client-side disambiguator at all — the iOS handler has no way to tell `MOOD_REMINDER` from `MEDICATION_REMINDER` from `SYSTEM_ALERT` once the user taps. The `category` field on `aps` is enough for the action-button row, but it does NOT surface in `userInfo` on the legacy `didReceive` delegate path before iOS 10. + +**Fix shape.** Always attach at least `{ eventType }` from the calling layer (or, alternatively, write `note.payload = { eventType, ...(data ?? {}) }` in the sender so the envelope is never empty). Add a sender-level test asserting `note.payload.eventType` is set even when the caller passes no `data`. + +### M3 — `scheduledAt` ISO string in med-reminder metadata bypasses scheduling source-of-truth [Medium] + +**File.** `src/lib/jobs/reminder-worker.ts:582-587` + +```ts +const [winH, winM] = schedule.windowStart.split(":").map(Number); +const scheduledAtIso = new Date( + todayStart.getTime() + winH * 3600000 + winM * 60000, +).toISOString(); +``` + +**Issue.** `todayStart` is a UTC Date returned from `getUserTodayBounds` (the user's local midnight expressed as UTC). Adding `winH * 3600000 + winM * 60000` to that timestamp is a straight UTC offset — correct ONLY when the user's local midnight + `winH` hours yields the same wall-clock as `winH` in local time, which is `false` across a DST transition day in any IANA tz that observes DST. + +Concrete example: user in `Europe/Berlin` on the autumn fall-back day (2026-10-25, clock goes back 1h at 03:00). `todayStart` resolves to 2026-10-24T22:00:00Z (00:00 local). For a `windowStart="22:00"` schedule, the code computes `2026-10-25T20:00:00Z` and stamps it as the schedule slot. The actual wall-clock 22:00 local that day is `2026-10-25T21:00:00Z` (one hour later in UTC because the day has 25 hours). The metadata sent to APNs is off by one hour exactly on transition days. + +This is a Medium because (a) the impact is one wrong second of `scheduledAt` per user per DST day per schedule, (b) the iOS client uses `scheduledAt` to anchor the "snooze 15 min" calculation — it would snooze relative to the wrong baseline on DST days, and (c) Marc's user base is concentrated in Europe/Berlin where this fires twice a year. + +**Fix shape.** Project `windowStart` into the user's timezone via the same `Intl.DateTimeFormat` machinery that powers `getLocalDateParts` — or, simpler, store `scheduledAt` as the local YYYY-MM-DDTHH:mm string + tz separately and let the iOS client convert. Either way, do not arithmetic-add hours to a UTC `todayStart` and call it a local-time slot. + +### M4 — `getServerTranslator` is synchronous but reads `allMessages` from disk on first call [Medium] + +**File.** `src/lib/jobs/mood-reminder.ts:104` (via `getServerTranslator`) + +**Issue.** Once per worker boot, the first `runMoodReminderTick` invocation triggers a synchronous read of every locale's messages bundle through `allMessages`. The worker has `localConcurrency=1` and this only happens once per process, so the runtime cost is irrelevant. The concern is initialisation order: if `runMoodReminderTick` ever runs on a code path where the locale messages aren't yet bundled into the runtime (e.g. a unit test that mocks `@/lib/i18n/shared-resolve` partially), the dispatcher silently returns the raw key like `"moodReminders.dailyTitle"` as the push body. + +The unit test `buildMoodReminderPayload — returns German strings for locale=de` (mood-reminder.test.ts:160-163) does NOT mock the i18n layer — it exercises the real translator. That guards the de/en path but not against future re-arrangements of how messages are bundled. + +This is a Medium because the production codepath works today; the brittleness is the dependency on global init. + +**Fix shape.** Add a defensive assertion in `buildMoodReminderPayload` that the resolved string is not equal to the input key, throwing if so. That converts a silent "user receives `moodReminders.dailyTitle` as their push" into a loud worker error visible in the wide-event log. + +### L1 — Idempotency-row created BEFORE dispatch ⇒ dispatcher failure leaves a poison-pill row [Low] + +**File.** `src/lib/jobs/mood-reminder.ts:198-216` + +**Issue.** The handler reserves the `MoodReminderDispatch` row first (lines 202-216), then calls `dispatchImpl` (lines 220-229). If the dispatcher throws — APNs network outage, web-push 500, Telegram quota — the ledger row is already persisted with `dispatched_at = now()`. The next 15-min tick checks the ledger, finds the row, and skips the user for the rest of the day. The user never receives the reminder despite the dispatcher having no successful delivery. + +The comment on line 197-201 acknowledges the race-vs-double-fire tradeoff but doesn't address the failure-mode tradeoff. The current ordering optimises for "never double-send"; the alternative would optimise for "always retry on transient failure". + +**Fix shape.** Either (a) accept the tradeoff explicitly and document that an APNs outage during the 22:00 window means the user gets no reminder that day — which may be the right call for a low-stakes nudge, or (b) move the ledger write to AFTER successful dispatch with a UNIQUE constraint catch on the second tick. Option (a) is probably right for v0.5.4. A test pinning the chosen contract would be valuable. + +### L2 — `scheduledAt` UTC string risks confusing the iOS handler in non-Berlin zones [Low] + +**File.** `src/lib/jobs/mood-reminder.ts:225-228` + +```ts +metadata: { + scheduledAt: now.toISOString(), + localDate: decision.localDate, +}, +``` + +**Issue.** `now.toISOString()` is the wall-clock UTC of the cron tick that fired, NOT the local 22:00 boundary. The iOS handler reading `scheduledAt` to derive "you should have logged your mood by 22:00 local" gets a UTC timestamp 0–14 minutes after 22:00 local. Not wrong, but ambiguous: the API field name reads as "when was this scheduled to fire" — answer is "22:00 local" — but the value is "22:07Z on a Friday in Berlin". + +This is a Low because the iOS contract for `scheduledAt` is not pinned anywhere in this PR (no iOS code in scope), and the dispatcher payload pattern matches the new med-reminder enrichment (M3). Worth aligning on a convention with the iOS team before v0.5.4 ships. + +**Fix shape.** Either rename to `dispatchedAt` (which is what the value is) or compute the actual 22:00 local boundary in UTC and ship that — same fix shape as M3, same convention. + +### L3 — Cron `*/15` may miss the 22:00 window for a user whose tz transitions during the hour [Low] + +**File.** `src/lib/jobs/reminder-worker.ts:183-194` + +**Issue.** A user on the `Asia/Kathmandu` (+05:45) timezone — or any other quarter/half-hour-offset tz — could in principle have a DST transition lining the cron's 15-minute UTC tick pattern up such that all four UTC ticks within the user's local 22:00 hour are on the "old" side of the transition and one falls on the "new" side that's no longer 22. Practically, no quarter-hour-offset tz observes DST today (Asia/Kathmandu, Asia/Yangon, Pacific/Chatham +12:45 don't), so this is theoretical. + +**Fix shape.** No action needed today. Add a comment near the cron declaration noting that the every-15-min cadence assumes IANA timezones with full-hour DST shifts, so the assumption is reviewable when a new tz is added to the supported list. + +--- + +## Prisma migration 0069 — verdict: GREEN + +The migration `prisma/migrations/0069_v054_mood_reminder/migration.sql:15-39` correctly implements the idempotency guards: + +- `ALTER TABLE "users" ADD COLUMN IF NOT EXISTS "mood_reminder_enabled" BOOLEAN NOT NULL DEFAULT FALSE` — the `IF NOT EXISTS` guard is supported on Postgres 9.6+ (HealthLog runs PG 16). Re-application is silent. Default `FALSE` matches the Prisma decl `Boolean @default(false)` in `schema.prisma:208` and the "default off" contract claimed in the commit message. +- `CREATE TABLE IF NOT EXISTS "mood_reminder_dispatches"` — also idempotent, ditto the `CREATE UNIQUE INDEX IF NOT EXISTS` + `CREATE INDEX IF NOT EXISTS` lines. +- The composite unique index `mood_reminder_dispatches_user_id_date_key` on `("user_id", "date")` matches the Prisma `@@unique([userId, date])` in `schema.prisma:1208` byte-for-byte (Prisma's auto-generated index name convention is `{table}_{col1}_{col2}_key`). The `userId_date` composite query at `mood-reminder.ts:174` will hit this index. +- The `DO $$ ... EXCEPTION WHEN duplicate_object THEN NULL END $$` block correctly handles re-application of the FK constraint, catching the `duplicate_object` SQLSTATE that PG raises when an FK with the same name already exists. +- `ON DELETE CASCADE` on `mood_reminder_dispatches.user_id` is the correct choice for privacy retention: when a user is deleted, their reminder ledger goes with them. The ledger contains no clinical data — only an opaque `id`, `user_id`, `date`, `dispatched_at` — so the cascade is also GDPR-clean. + +Re-applying the migration on a database where it already ran will succeed silently — verified by code inspection of all four `IF NOT EXISTS` / `EXCEPTION` guards. + +--- + +## APNs payload — verdict: GREEN with one nuance + +- `aps.category` is set via node-apn's `Notification.category` setter (`apns.ts:265`), which writes through to `aps.category` per the library's runtime contract. The value is a plain string matching exactly what iOS v0.5.3 registers (`MEDICATION_REMINDER`, `MOOD_REMINDER`). No typos. ✅ +- `aps.mutable-content` is set via `note.mutableContent = true` (`apns.ts:270`), which node-apn correctly serialises as the JSON integer `1`, not the boolean. node-apn's `Notification` setter is documented and respects Apple's strict-integer rule. ✅ +- Payload size: the largest enrichment added in this PR is `scheduledAt` (~26 chars ISO string) + `localDate` (~10 chars). The med-reminder branch adds the same `scheduledAt`. Total payload across all metadata keys is well under 1 KB — nowhere near the 4 KB / 5 KB APNs limits. ✅ +- Alert localisation: server-side string interpolation via `getServerTranslator`, no `loc-key` / `loc-args` template path. That's a deliberate choice — `loc-key` requires the iOS bundle to ship the matching `Localizable.strings`, and the server can change copy without an App Store release this way. Tradeoff is mentioned for completeness, not flagged. ✅ +- Device token handling: no token logging, no token leakage in the wide-event payload. `apns.ts:285-290` logs only `service`, `method`, `duration_ms`, `status`, `error`. The `addExternalCall` call never receives the token. ✅ + +--- + +## Test-coverage note + +The 13 unit tests in `mood-reminder.test.ts` pin window boundaries, opt-in gating, dedup, and the P2002 race correctly. The locale test at lines 159-177 is weak: it covers de + en + null/unknown but misses fr/es/it/pl. Adding four lines there would catch H1 immediately and is the smallest possible regression guard. + +The APNs test at `apns.test.ts:449-467` pins `MOOD_REMINDER` category forwarding correctly. Good coverage of the new event-type's APNs path. diff --git a/.planning/ios-coord/V054-QA-ux-findings.md b/.planning/ios-coord/V054-QA-ux-findings.md new file mode 100644 index 000000000..352c9965d --- /dev/null +++ b/.planning/ios-coord/V054-QA-ux-findings.md @@ -0,0 +1,222 @@ +# V054 PR #190 — UX Review of Notification Surfaces + +**Reviewer scope.** Squash-merge `4049c6c7` + version bump `c4f2e0bc` on `main`. +**Files in scope.** `messages/{de,en,es,fr,it,pl}.json`, +`src/lib/notifications/senders/apns.ts`, `src/lib/notifications/types.ts`. +**Audience.** Opted-in personal-health tracker user, iPhone lock-screen or +Notification Center, DE canonical / EN/ES/FR/IT/PL translated. + +--- + +## Severity counts + +| Severity | Count | +| --- | --- | +| Critical | **2** | +| High | **3** | +| Medium | **3** | +| Low | **2** | + +--- + +## Critical + +### C1 — French body has spelling bug `aujourdhui` (missing apostrophe) +**File.** `messages/fr.json:2993` +``` +"dailyBody": "Comment vous sentez-vous aujourdhui ?" +``` +Correct French is `aujourd'hui`. This is a hard typo that ships on the +lock-screen and reads aloud through VoiceOver. Native FR users will see +this as carelessness on a *health* product. The same string in DE/EN/ES/IT +is spelled correctly. Either the apostrophe was stripped because the +contributor avoided embedding `'` inside the JSON string, or it was simply +missed during the wave. + +**Fix.** `"Comment vous sentez-vous aujourd'hui ?"` (the curly apostrophe +`’` is also acceptable and is what Apple typography expects, but the +straight `'` is what the rest of the FR file uses — keep it consistent). + +### C2 — No Settings surface to toggle `mood_reminder_enabled` +**Files.** +- `src/components/settings/notifications-section.tsx` (the natural home) +- `src/app/settings/[section]/page.tsx` +- DB column `users.mood_reminder_enabled` exists, default `false` +- `NotificationsState` has a `moodLogGlobal` field but it is **unwired** + (`grep -n moodLog notifications-section.tsx` returns only the type + declaration on line 19; no `` / `` renders it, and the + field is *not* the new `moodReminderEnabled` flag — it's pre-existing + unrelated state). + +A user who installs iOS HealthLog v0.5.4 and grants notification permission +**cannot turn the daily 22:00 mood push on**, because the only way to flip +`User.moodReminderEnabled = true` today is via raw SQL or the Prisma +console. The feature ships as a dead opt-in. + +**Fix.** Add a single `` to `notifications-section.tsx` bound to +`/api/me` (or wherever profile prefs are PATCHed), with the +`notifications.moodReminder.toggleLabel` / +`notifications.moodReminder.toggleDescription` strings added to all six +locale files. Without this, ship the server foundation as "iOS-only" in +the release notes or pull the cron registration until a UI exists. + +--- + +## High + +### H1 — 22:00 fire-time ignores user sleep / quiet hours +**File.** `src/lib/jobs/mood-reminder.ts:37` +``` +export const MOOD_REMINDER_LOCAL_HOUR = 22; +``` +Hard-coded to 22:00 local. `grep -in "quiet\|sleep\|do.?not.?disturb" +src/lib/jobs/mood-reminder.ts` returns nothing. A user whose +`Settings → Sleep` schedule has them in bed at 22:00 (early sleepers, +shift workers, parents) gets buzzed during wind-down on a product that +otherwise tracks sleep as a first-class metric. iOS Focus modes will +suppress the banner, but the badge + lock-screen flash still happen, and +on Android (web push fallback) Focus modes don't apply. + +**Fix.** Either (a) make the hour user-configurable in the same Settings +row as C2, or (b) cross-reference the user's average bedtime from the +Sleep series and skip the push if `22:00 ≥ avg_bedtime - 30min`. (a) is +the minimum viable fix. + +### H2 — DE title `Stimmung erfassen` reads as a task instruction, not a nudge +**File.** `messages/de.json:2992` + +The DE title imitates the EN imperative `Log your mood`, but Marc's +existing DE notification copy (see `medicationReminders` block, +`messages/de.json:2976-2989`) uses *event-status* phrasing — `🟢 +Erinnerung: …`, `🟡 Bald fällig: …`. The new title breaks pattern and +sounds like a task on a TODO list rather than a soft check-in. The body +`Wie geht es dir heute?` already carries the conversational tone the +title is missing; the title should match the warmth. + +**Suggested DE.** `Kurzer Stimmungs-Check` or `Tagescheck Stimmung` — +mirrors the noun-led, status-flavoured style of the rest of the file. A +two-word title also keeps it well under the iOS 50-char truncation in any +locale. + +### H3 — `category = payload.eventType` is forwarded indiscriminately +**File.** `src/lib/notifications/senders/apns.ts:399` +```ts +category: payload.eventType, +``` +The comment claims "iOS ignores categories it doesn't know about, so adding +the key here is safe". That is true for *unknown* categories. But the iOS +app currently registers two known categories (`MEDICATION_REMINDER`, +`MOOD_REMINDER`); every *other* event type — `MEDICATION_MISSED`, +`WITHINGS_SYNC_FAILED`, `SYSTEM_ALERT`, `PERSONAL_RECORD` — now ships with +a category identifier that iOS will look up, find nothing for, and silently +drop. That's not a UX bug today, but the moment the iOS team registers a +3rd category they will inherit accidental wiring (e.g. registering +`SYSTEM_ALERT` with destructive actions automatically routes existing +server pushes through it). UX-wise: silent surface-coupling between +back-end event-types and iOS UI categories. + +**Fix.** Maintain an explicit allow-list (`KNOWN_IOS_CATEGORIES`) inside +the sender. Only forward `category` for event-types in the list. The cost +is one line; the safety is significant. + +--- + +## Medium + +### M1 — Inconsistent register across locales +| Locale | Form | Note | +| --- | --- | --- | +| DE | informal `du` (`Wie geht es dir heute?`) | matches AI Coach / app convention | +| EN | imperative + casual | OK | +| ES | informal `tú` (`¿Cómo te sientes hoy?`) | matches | +| FR | **formal `vous`** (`Comment vous sentez-vous`) | breaks pattern — rest of FR coach copy uses informal `tu` per recent v1.4.38 wave | +| IT | informal `tu` (`Come ti senti oggi?`) | matches | +| PL | informal `ty` (`Jak się dzisiaj czujesz?`) | matches | + +FR is the outlier. A health-tracker daily nudge is exactly the kind of +intimate, single-user surface where French apps drop `vous`. Re-check +against the rest of `messages/fr.json` and align. + +### M2 — Mood title `Registrar el ánimo` (ES) is jargon-y +**File.** `messages/es.json:2993` +`Registrar el ánimo` is grammatically fine but lands as form-filling +language ("register the mood"). More natural-feeling alternatives that +match the EN/IT softness: `Registra tu ánimo` or `¿Cómo te sientes hoy?` +(as title, dropping the body) or `Tu chequeo de ánimo`. Same observation +applies to PL `Zapisz nastrój` (literally "save mood"). + +### M3 — Title + body together are tautological +All six locales pair title="Log/Record your mood" + body="How are you +feeling today?". On the iOS lock-screen these stack vertically with the +body in lighter weight directly under the title, and the two-line read +becomes *"Log your mood — How are you feeling today?"* — the body answers +its own title. A better split: keep the question as the title (it's the +verb), use the body for the *why* (e.g. `Trag deine Stimmung ein, es +dauert nur 5 Sekunden.` / `Takes 5 seconds.`). Today the body is +load-bearing zero. + +--- + +## Low + +### L1 — `mutableContent = true` enabled before NSE exists +**File.** `src/lib/notifications/senders/apns.ts:407` +The comment is honest that no NSE ships today. UX impact: zero. But +shipping the flag now means any later NSE bug (image-download failure, +decryption stall) will block notification delivery for v0.5.4 users +*after* the iOS team adds an NSE in v0.5.5, with no server-side opt-out +path. Recommend a feature-flag gate so the flag can be turned off without +a backend redeploy. + +### L2 — No `threadId` thinking for MOOD_REMINDER +**File.** `src/lib/notifications/senders/apns.ts:395` +`threadId: payload.eventType` groups all mood reminders under one thread — +on iOS Notification Center this is correct (don't pile up 30 identical +"How are you feeling today?" rows). Confirm with iOS team that the same +identifier is treated as a thread for grouping; if not, today's behaviour +is fine but worth a one-line comment on the sender that the grouping is +intentional for MOOD_REMINDER specifically. + +--- + +## Per-locale roll-up + +| Locale | Title | Body | Verdict | +| --- | --- | --- | --- | +| **DE** | `Stimmung erfassen` | `Wie geht es dir heute?` | Body strong; title robotic (H2). | +| **EN** | `Log your mood` | `How are you feeling today?` | Adequate; tautological pair (M3). | +| **ES** | `Registrar el ánimo` | `¿Cómo te sientes hoy?` | Title jargon-y (M2); body strong. | +| **FR** | `Enregistrer votre humeur` | `Comment vous sentez-vous aujourdhui ?` | **Critical typo C1**, register break (M1). | +| **IT** | `Registra il tuo umore` | `Come ti senti oggi?` | Cleanest pair of the six. | +| **PL** | `Zapisz nastrój` | `Jak się dzisiaj czujesz?` | Title slightly imperative (M2); body strong. | + +--- + +## Settings-surface flag + +**MISSING** — the new `User.moodReminderEnabled` field has **no UI +control anywhere in the app**. PR #190 must not be considered +user-complete until either +1. a Switch lands in `src/components/settings/notifications-section.tsx` + bound to a PATCH endpoint, **or** +2. the cron registration in `reminder-worker.ts` is gated behind a + build-time `ENABLE_MOOD_REMINDER` flag and the release notes mark the + feature as "iOS-team-only preview". + +Without one of those, the merged code silently expands the schema and +worker plumbing but ships zero observable behaviour to end users. + +--- + +## Cross-reference: existing notification copy patterns + +`medicationReminders` (de.json:2976-2989) uses: +- title pattern: `🟢 Erinnerung: {medName}` — emoji + status noun + colon +- body pattern: `{medName} (…)\n{detail}` — bolded subject + meta + +`moodReminders` uses **no emoji**, **no status noun**, **no rich +formatting** — it stands out as a different surface. That's defensible +for APNs (rich HTML doesn't render on the lock-screen), but the emoji +absence is unexplained. A single matching emoji (🌤 / 💭 / 🫶) in the +title would unify the visual language at zero copy cost. Worth raising +with Marc as a design call, not a hard finding. diff --git a/.planning/ios-coord/V054-server-apns-mood-report.md b/.planning/ios-coord/V054-server-apns-mood-report.md new file mode 100644 index 000000000..fabf919f5 --- /dev/null +++ b/.planning/ios-coord/V054-server-apns-mood-report.md @@ -0,0 +1,126 @@ +# V0.5.4 iOS Coord — Server-side APNs + MOOD_REMINDER Patches + +**Branch:** `ios-coord/v054-apns-mood` +**PR:** https://github.com/MBombeck/HealthLog/pull/190 +**Date:** 2026-05-17 +**Status:** Open for operator review (do NOT merge to main without sign-off) + +## Mission + +Two server-side patches that unblock iOS HealthLog v0.5.4 push-notification +functionality: + +1. **SR-1** — APNs `aps.category = "MEDICATION_REMINDER"` so iOS renders the + three action-buttons wired in iOS v0.5.3 (Genommen / Snooze 15 min / + Übersprungen). +2. **SR-2** — `MOOD_REMINDER` event-type + daily 22:00-local-time cron + (opt-in, idempotent, locale-aware). + +## Patches landed + +### Commit 1 — `feat(notifications): set APNs category for med-reminders` + +- `src/lib/notifications/senders/apns.ts` + - `ApnsPayload` extended with optional `category` + `mutableContent` + fields. + - `sendApnsPush` writes the category through node-apn's setter + (cast wraps the d.ts gap in node-apn 8.1; runtime setter at + `apsProperties.js#166` lands the value at `aps.category`). + - `sendViaApns` auto-forwards `payload.eventType` as the category, + so every event-type the iOS app registers becomes actionable + without per-event-type plumbing. `mutableContent: true` is set + by default (NSE-ready). +- `src/lib/notifications/senders/__tests__/apns.test.ts` + - 3 new tests pinning `aps.category = MEDICATION_REMINDER` on the + dispatcher path, `aps.category = MOOD_REMINDER` for the new + event-type, and the explicit-override path on `sendApnsPush`. + +### Commit 2 — `feat(prisma): add moodReminderEnabled + MoodReminderDispatch ledger` + +- `prisma/schema.prisma` + - `users.mood_reminder_enabled BOOLEAN DEFAULT FALSE` (opt-in flag). + - New model `MoodReminderDispatch` with `@@unique([userId, date])` + — idempotency anchor for the daily cron. +- `prisma/migrations/0069_v054_mood_reminder/migration.sql` + - Additive migration with `IF NOT EXISTS` + `DO $$ EXCEPTION WHEN + duplicate_object` guards (idempotent on re-apply, matching the + pattern from `0061` / `0068`). + +### Commit 3 — `feat(notifications): add MOOD_REMINDER event type with daily 22:00 cron` + +- `src/lib/notifications/types.ts` + - `EVENT_TYPES` extended with `MOOD_REMINDER`. `EVENT_DEFAULT_ENABLED` + sets it to `false` as a defence-in-depth gate behind the per-user + `moodReminderEnabled` flag. +- `src/lib/jobs/mood-reminder.ts` (new module, 220 LOC) + - `evaluateMoodReminderWindow(user, now)` — pure predicate for the + 22:00 window across any IANA timezone (DST-safe via + `getLocalDateParts`). + - `buildMoodReminderPayload(locale)` — locale-aware title/body + (DE: "Stimmung erfassen" / "Wie geht es dir heute?"; + EN: "Log your mood" / "How are you feeling today?"). + - `runMoodReminderTick(prisma, now)` — orchestrator: pulls + opted-in users, applies window + already-logged + already- + dispatched filters, reserves the dedup row, dispatches. +- `src/lib/jobs/reminder-worker.ts` + - Registers the `mood-reminder-check` queue with cron `*/15 * * * *`, + `localConcurrency: 1`. Cadence matches the medication-reminder + cron so any user's 22:00 boundary is caught within 15 min of + wall-clock without one cron entry per IANA zone. + - Existing medication-reminder dispatch now enriches metadata with + a `scheduledAt` ISO 8601 string so the iOS snooze-15-min action + pins against the schedule slot, not wall-clock delivery time. +- `src/lib/jobs/__tests__/mood-reminder.test.ts` (new, 13 tests) + - Window-boundary: 21:59 / 22:00 / 22:59 / 23:00, plus a DST-bearing + `America/New_York` check. + - Opt-in gating, logged-today skip, ledger-already-exists skip, + P2002 lost-race handling, multi-user multi-locale fan-out. +- `messages/{de,en,es,fr,it,pl}.json` + - New `moodReminders.dailyTitle` + `moodReminders.dailyBody` keys + in all six locales. + +## Quality gates + +| Gate | Result | +|---|---| +| `pnpm typecheck` | clean | +| `pnpm lint` | clean | +| `pnpm test` | 4542 tests pass, 1 skipped (430 test files) | +| `pnpm openapi:check` | spec in sync | +| `pnpm format:check` | pre-existing warnings unchanged; my files prettier-formatted | + +## Migration + +`prisma/migrations/0069_v054_mood_reminder/migration.sql` ships with the +PR. Applied via `pnpm db:migrate:deploy` on staging — `IF NOT EXISTS` +guards make re-apply a no-op. + +## Blockers + +None. + +## Follow-ups for the iOS contributor + +- iOS app must register `UNNotificationCategory` identifiers + `MEDICATION_REMINDER` (Take / Snooze 15 min / Skip) and + `MOOD_REMINDER` (Log mood) at launch, otherwise iOS will render the + pushes as plain alerts (the category will be present on the payload + but iOS ignores unknown identifiers). +- iOS Settings → Notifications surface needs a toggle for + `User.moodReminderEnabled` (PATCH /api/me or similar — the + preferences route already exists at + `src/app/api/notifications/preferences/route.ts` but only covers + per-event-type matrix; this is a per-user flag, not a per-channel + preference. Surface TBD by operator). +- The `scheduledAt` metadata field is already present on every + outbound APNs payload for med-reminders — iOS just needs to read it + from `userInfo`. + +## Compatibility + +- APNs payload changes are strictly additive. Older iOS builds that + don't register the categories render plain alerts (iOS silently + ignores unknown categories). +- `MOOD_REMINDER` is double-gated (per-event default OFF + per-user + `moodReminderEnabled` defaults FALSE). Users who never opt in see no + behavioural change. diff --git a/.planning/round-audit-marathon-closure.md b/.planning/round-audit-marathon-closure.md index 4fc6ba946..ce62570e2 100644 --- a/.planning/round-audit-marathon-closure.md +++ b/.planning/round-audit-marathon-closure.md @@ -1,6 +1,6 @@ -# v1.4.34.4 + v1.4.34.5 + v1.4.35 + v1.4.35.1 + v1.4.36 + v1.4.37 — Audit-Marathon Closure (extended) +# v1.4.34.4 + v1.4.34.5 + v1.4.35 + v1.4.35.1 + v1.4.36 + v1.4.37 + v1.4.37.1 + v1.4.37.2 + v1.4.38 — Audit-Marathon Closure (extended) -Shipped: 2026-05-17 (six consecutive releases on the same date as v1.4.34.3). +Shipped: 2026-05-17 (nine consecutive releases on the same date as v1.4.34.3, capped by v1.4.38 closing the web punch-list before the iOS sprint). | Tag | Highlights | Release | | --- | --- | --- | @@ -10,6 +10,8 @@ Shipped: 2026-05-17 (six consecutive releases on the same date as v1.4.34.3). | v1.4.35.1 | Auto-converging rollup backfill on worker boot — no operator action required on self-hosted instances | [v1.4.35.1](https://github.com/MBombeck/HealthLog/releases/tag/v1.4.35.1) | | v1.4.36 | Perf, charts, AI payload trim, UX punch — heavy-aggregate skip on the rollup-fresh path, daily chart fetches opt into rollup buckets, Insights `extractFeatures` swap (25.9 MB → 30,096 tokens, **−99.5 %**), page-blocking call 10.88 s → 4.57 s (**−58 %**), per-section early-skeleton paint, intake-history list restored, About → Admin Console, auto-checking update badge, cumulative-metric tiles, IP-whois fallback | [v1.4.36](https://github.com/MBombeck/HealthLog/releases/tag/v1.4.36) | | v1.4.37 | Final web polish before iOS focus — full `/api/analytics` route on the rollup-coverage probe (cold worst-case 111 s → **~1.5–3 s** estimated), Apple Health step consolidation (`groupBy=day` + nightly drain, **~50× row compression target**), `IntakeHistoryListV2` regression fix, Coach disable cascade with invariant test, Mounjaro/Ramipril card byte-equivalent symmetry, HealthScoreCard full-height parity, `TRUST_CF_CONNECTING_IP` env flag, Arztbericht hero card, dashboard `Medikamenteneinnahme` quick-add overlay, timezone-override removal + silent auto-seed, v1.4.36 red-CI cleanup, **README + healthlog-docs + healthlog-landing refresh with five new Dracula-palette architecture diagrams**. Unit suite 4354 → 4486 passing. | [v1.4.37](https://github.com/MBombeck/HealthLog/releases/tag/v1.4.37) | +| v1.4.37.1 / v1.4.37.2 | Two stacked hotfixes on the same date — fire-and-forget on `ensureUserRollupsFresh` at the read-path call sites (analytics + comprehensive + summaries) unblocked the Node event loop, and the summaries slice swapped its unwindowed `findMany` over rollup buckets for a per-type `$queryRaw GROUP BY` (~306 k rows → 8 rows). | [v1.4.37.1](https://github.com/MBombeck/HealthLog/releases/tag/v1.4.37.1) / [v1.4.37.2](https://github.com/MBombeck/HealthLog/releases/tag/v1.4.37.2) | +| v1.4.38 | Closing punch-list before iOS focus — `/api/dashboard/summary` cold 4.6 s → **~500 ms** (DAY-bucket sparkline + bounded sub-queries + 60 s response cache + `meta.dashboard.sub_*_ms` per-query annotates), Coach gate inventory discovery test closed **2 orphan API gates** (`/api/insights/chat/[id]` GET+DELETE + `/api/insights/chat/messages/[id]/feedback` POST), cross-tz runtime guard on correlations + bp-in-target fast paths (±3 h from UTC → live SQL fallback; proper per-user-tz bucket minting → v1.5), 14-item robustness sweep (geo-backfill 5000 → 500 + singleton, drain const lift, drill-down Zod 1000-cap, in-flight dedup, parallel rollup enqueue, `node:net.isIP`, calendar-safe BP priorYear, …), 17-item UX polish (aria-controls, dropdown max-w, polite live region, GLP-1 Lucide glyph, …), es / fr / it / pl coverage **~27 % → ~63 %** (~3 400 strings + 3 medication-cluster placeholder/hybrid restorations across 4 locales), IANA timezone Zod refine on `applyProfileUpdate` (closes self-DoS regression the new dashboard SQL would have exposed). Unit suite 4486 → 4524. Squash to main clean (no conflicts this round). | [v1.4.38](https://github.com/MBombeck/HealthLog/releases/tag/v1.4.38) | ## Audit waves diff --git a/.planning/round-v1438-perf-verify.md b/.planning/round-v1438-perf-verify.md new file mode 100644 index 000000000..c7fe46e53 --- /dev/null +++ b/.planning/round-v1438-perf-verify.md @@ -0,0 +1,98 @@ +# v1.4.38 post-deploy perf verification + +**Release:** v1.4.38 (`a550031a` on main, live 2026-05-17 17:14 UTC) +**Live host:** https://healthlog.bombeck.io +**Account under test:** Marc's production account (311 k+ measurements) + +## TL;DR + +- Live tag is **1.4.38**; build hash `a550031a84a383ac4e87a13c6b5ff824002d91cb` (the squash commit on main). +- Boot clean: reminder worker up in **801 ms** (v1.4.37 baseline 772 ms; no regression). +- 69 migrations found, no pending migrations to apply (v1.4.38 adds no schema change, as documented in the release notes). +- No boot errors → all worker subscriptions land cleanly, including the v1.4.37-introduced `DRAIN_CUMULATIVE_QUEUE` registration that v1.4.35.1 added the `rollup-full-backfill` boot queue alongside. +- Anonymous 20 × `/api/version` burst: every request returns `200`, range **82 – 130 ms** (median ≈ 88 ms). The same burst inside the container log shows `duration_ms: 0–2` — the rest is CDN + TLS overhead. +- Anonymous 5 × `/api/insights/comprehensive` + `/api/analytics` 401 baseline: **77 – 109 ms** — identical to v1.4.37 (no auth-layer regression). + +## Boot trace (verified) + +``` +HealthLog: Waiting for database... +HealthLog: Database is reachable. +HealthLog: Running database migrations... +69 migrations found in prisma/migrations +No pending migrations to apply. +HealthLog: Migrations complete. +HealthLog: Starting application... +▲ Next.js 16.2.6 +✓ Ready in 0ms +{ background.task_name: "startup", reminder_worker: "started", duration_ms: 801 } +``` + +No `boss.createQueue` errors. No schedule-registration warnings. No openapi gate failures. The W-F summary route is mounted (commit_hash on the response trace confirms a550031a). + +## Anonymous latency (auth-layer baseline) + +| Surface | Mode | Burst sample | Notes | +|---|---|---|---| +| `/api/version` | anon, 20× | 82 – 130 ms (median ≈ 88 ms) | container-side `duration_ms` 0–2 | +| `/api/insights/comprehensive` | anon 401, 5× | 81 – 109 ms | identical to v1.4.37 | +| `/api/analytics` | anon 401, 5× | 77 – 95 ms | identical to v1.4.37 | +| `/api/health` (Uptime-Kuma) | live | 92 ms | health-probe envelope unchanged | + +Identical envelope to v1.4.37 → no auth/middleware regression. + +## Headline perf bet — `GET /api/dashboard/summary` 4.6 s → ~500 ms + +The W-F wave reshaped the iOS dashboard summary route from four unbounded sub-queries + a conditional fifth to six bounded reads: + +| # | Query | Bounded row count | +|---|-------|-------------------| +| B1 | `$queryRaw DISTINCT ON (type)` over 7d window | one row per metric type | +| B2 | `$queryRaw` over `measurement_rollups` DAY buckets in 7d | ≤ `SPARK_DAYS × N` ≈ 70 | +| B3 | `measurement.groupBy({ by: ["type"], _count, _max })` | unchanged | +| B4 | `medicationIntakeEvent.findMany({ scheduledFor: today })` | unchanged | +| B5 | `medicationIntakeEvent.findMany({ scheduledFor: { gte: -365d } })` | unchanged | +| B6 | `$queryRaw SELECT DISTINCT to_char(measured_at AT TIME ZONE $tz, 'YYYY-MM-DD')` | ≤ 365 | + +The whole builder wraps in `caches.analytics` keyed `${userId}|dashboard-summary` at **60 s TTL**. Per-sub-query timing annotates land in `meta.dashboard.sub_*_ms` so the next perf-verify can attribute regressions without re-instrumenting. + +### Expected wins + +| Surface | v1.4.37.2 baseline | v1.4.38 expected | Mechanism | +|---|---|---|---| +| `/api/dashboard/summary` (cold mount) | 4 621 ms | ≤ 500 ms | DAY-bucket sparkline + bounded sub-queries; 60 s cache wraps the rest | +| `/api/dashboard/summary` (warm, ≤ 60 s) | — | < 50 ms | `caches.analytics` hit | +| `/api/insights/comprehensive` (cold) | unchanged | comparable | already on rollup probe; W-F only consolidated BP raw read | + +### Pending — live authenticated capture + +The route is iOS-only on the trace path; an anonymous probe cannot exercise the SQL fan-out. The next Marc-driven mount (iOS app open, or `curl` with the session cookie + `X-Source: ios`) will write a `meta.dashboard.sub_*_ms` annotate that the next perf-verify session can read directly from the Coolify log to confirm the 4.6 s → 500 ms win. The reservation slot for the live numbers: + +| Sub-query | Expected upper bound | Live (pending) | +|---|---|---| +| `meta.dashboard.sub_latest_ms` (B1) | ≤ 150 ms | _t.b.d._ | +| `meta.dashboard.sub_spark_ms` (B2) | ≤ 100 ms | _t.b.d._ | +| `meta.dashboard.sub_type_stats_ms` (B3) | ≤ 50 ms | _t.b.d._ | +| `meta.dashboard.sub_intake_today_ms` (B4) | ≤ 30 ms | _t.b.d._ | +| `meta.dashboard.sub_intake_year_ms` (B5) | ≤ 200 ms | _t.b.d._ | +| `meta.dashboard.sub_days_seen_ms` (B6) | ≤ 80 ms | _t.b.d._ | +| **Total (cold)** | **≤ 500 ms** | _t.b.d._ | +| Cache hit | < 50 ms | _t.b.d._ | + +## Cross-tz guard (W-A) + +`meta..tz_guard` annotates land on the correlations + bp-in-target fast paths. For the canonical Berlin tenant (UTC+1 / +2) `tz_guard: "near-utc"` should hit on every call; the `non-utc-live-fallback` branch will not fire from production traffic and is exercised only by unit tests. Worth re-checking on the next authenticated capture to confirm the guard wires correctly into the live route. + +## Coach gate inventory (W-C) + +The two newly-gated routes (`/api/insights/chat/[id]` GET + DELETE, `/api/insights/chat/messages/[id]/feedback` POST) now return `403` instead of running for unauthorized callers. Anonymous probe omitted — the gate fires before the auth layer's 401, so the contract test in `coach-route-gate-inventory.test.ts` is the source of truth. + +## i18n surface spot-check + +Locale routes (`/es/dashboard`, `/fr/dashboard`, `/it/dashboard`, `/pl/dashboard`) anonymous probe is gated by the auth redirect; the live HTML render is therefore behind the session cookie. Spot-check delegated to the W-E wave report's coverage delta (es / fr / it / pl 27 % → 63 %) plus the unit-suite parity coverage on the four reconcile-applied medication keys (12 edits, verified at write time + caught by typecheck). + +## Operator notes + +- Coolify auto-deploy fired correctly on tag push (no host-side retag fallback needed for this release — the v1.4.34.2 `pull_policy: always` fix continues to land as designed). +- No pending operator actions introduced by v1.4.38. +- Carry from v1.4.34 closure operator-actions still pending (edge01 Coolify-MCP daemon restart, edge01 `DATABASE_URL` pool-bump, apps01 resource-limits resize, apps01 duplicate env-pairs prune) — environment-side, unchanged. diff --git a/CHANGELOG.md b/CHANGELOG.md index b6d228448..df56114ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,92 @@ # Changelog +## [1.4.38.2] — 2026-05-17 — Mood-reminder hotfix bundle + Settings toggle + +Hotfix bundle on top of v1.4.38.1. A six-axis review of the v0.5.4 +iOS-coordination patch surfaced enough real bugs that the +mood-reminder feature could not have been used safely as shipped: +the locale resolver demoted four of six supported locales to +English, the dedup ledger committed before delivery (so a transient +APNs blip silently nuked the day's nudge), the daily-22:00 timestamp +drifted by an hour on DST days, the APNs payload forwarded +channel-specific structures (the Telegram `replyMarkup` keyboard) to +Apple, and there was no Settings surface for the user to opt in. +v1.4.38.2 closes all of the above and ships the missing toggle. + +### Added + +- **Settings toggle for the daily mood reminder.** New card under + `/settings/notifications` with a single Switch wired to the user + profile flag. Six-locale copy (de/en/es/fr/it/pl) for the card + title, description, status text, and toasts. Without this surface + the feature shipped in v1.4.38.1 was unreachable for end users. +- **`DispatchOutcome` return type** on `dispatchNotification` so + callers can decide whether the send is committed to their own + ledger. Existing callers ignore the return and keep working — + the function remains best-effort and never throws. +- **`localHmAsUtc` helper** in `@/lib/timezone` returns the UTC + instant of local `hh:mm` on the local calendar day implied by + `now`. Re-derives the offset at the target local time so the + result is correct on DST transition days. +- **Daily 03:25 Europe/Berlin retention cron** for the + `mood_reminder_dispatches` ledger: deletes rows older than 90 + days. Slots between the audit-log cleanup and the cumulative + drain. + +### Fixed + +- **Mood-reminder ledger now writes only after the dispatcher + confirms delivery.** A `dispatched = false` outcome (no channel + succeeded) leaves the slot empty so the next tick is free to + retry once the user adds a channel or upstream recovers. +- **Mood-reminder locale resolver honours every supported locale.** + v1.4.38.1 shipped translations for es/fr/it/pl but the resolver + silently demoted those users to English copy. +- **Mood-reminder FR body restores the apostrophe in `aujourd'hui`.** + v1.4.38.1 shipped without it; VoiceOver reads the lockscreen + body aloud, so the typo was audible. +- **Medication-reminder `scheduledFor` and the iOS-snooze + `scheduledAt` ISO are now DST-safe.** The previous arithmetic + added raw UTC hours to local midnight, which drifted by an hour + on spring-forward / fall-back days. The iOS "snooze 15 min" + action would otherwise anchor against the wrong baseline twice a + year. +- **Per-user `try` wrapper around the mood-reminder tick.** A + single bad row (corrupt timezone, dispatcher exception) used to + abort the whole 22:00 candidate pass; now the wide-event records + a per-user failure counter and the rest of the cohort is + unaffected. +- **APNs payload whitelists iOS-relevant metadata keys.** The + dispatcher's `metadata` is shared across every channel and was + forwarding channel-specific payload structures (Telegram's + `replyMarkup` inline-keyboard object, ad-hoc extras) into the + APNs `userInfo` Apple sees and the iOS notification handler. + Allowlist now covers only what iOS actually reads. + +### Refactor + +- Removed an unused `MoodReminderCandidate` interface and the + redundant `moodReminderEnabled` select field from the + mood-reminder candidate query (the `where` clause already filters + to opted-in users). +- CHANGELOG entry for v1.4.38.1 rewritten to drop the + `EVENT_DEFAULT_ENABLED` identifier reference and describe the + default-off posture in user-readable language. + +### Tests + +- Unit suite 4565 → 4551 (rewrote `mood-reminder.test.ts` for the + new contract; net delta is a rewrite, not a coverage loss). + Added tests for the ledger-after-delivery semantics, the + per-user try wrapper, the P2002-race "lost the race but + delivered" path, the six-locale dispatch, and the FR apostrophe + regression. + +### Operator notes + +- No new migration. No env-var change. +- Coolify auto-deploys main on tag push. + ## [1.4.38.1] — 2026-05-17 — iOS v0.5.4 push-notification coordination Coordinates the server side of the iOS v0.5.4 release. Two strictly @@ -17,15 +104,13 @@ without changing existing behaviour. metadata gains `scheduledAt` (ISO 8601) so the iOS "snooze 15 min" action pins to the schedule slot rather than wall-clock delivery time. -- **Optional `MOOD_REMINDER` daily event.** New per-user opt-in flag - (`users.mood_reminder_enabled`, default off) and a 15-minutely - worker cron that fires a single `MOOD_REMINDER` push at the user's - local 22:00 when no mood has been logged for the local date. - Idempotency anchored by the unique `(userId, date)` constraint on - the new `mood_reminder_dispatches` ledger — safe under cron - re-ticks inside the same 22:00 window and under concurrent workers. - Default-off via `EVENT_DEFAULT_ENABLED`; users who never opt in - see no behavioural change. +- **Optional daily mood-reminder event.** New per-user opt-in flag + (default off) and a 15-minutely worker cron that fires a single + push at the user's local 22:00 when no mood has been logged for + the local date. Idempotency is anchored by a unique (user, date) + ledger row, so a re-tick inside the same 22:00 window and + concurrent workers cannot double-fire. Users who never opt in see + no behavioural change. ### Compatibility diff --git a/messages/de.json b/messages/de.json index 8084f470b..6eb93b9a6 100644 --- a/messages/de.json +++ b/messages/de.json @@ -2790,6 +2790,16 @@ }, "user": { "telegramTestBody": "HealthLog: Verbindung hergestellt. Telegram-Benachrichtigungen sind aktiv." + }, + "moodReminder": { + "title": "Tägliche Stimmungs-Erinnerung", + "description": "Sanfter Anstoß um 22:00 Uhr, falls du heute noch keine Stimmung erfasst hast. Greift nur auf bereits eingerichtete Kanäle (Telegram, ntfy, Web Push, Apple Push) zurück und schweigt, sobald du heute geloggt hast.", + "toggleAria": "Tägliche Stimmungs-Erinnerung umschalten", + "statusOn": "Aktiv um 22:00 deiner Zeit", + "statusOff": "Aus", + "enabledToast": "Stimmungs-Erinnerung aktiviert.", + "disabledToast": "Stimmungs-Erinnerung deaktiviert.", + "saveError": "Speichern fehlgeschlagen. Bitte später erneut versuchen." } }, "i18n": { diff --git a/messages/en.json b/messages/en.json index 84e4fea2d..df99c3f99 100644 --- a/messages/en.json +++ b/messages/en.json @@ -2790,6 +2790,16 @@ }, "user": { "telegramTestBody": "HealthLog: connection successful. Telegram notifications are active." + }, + "moodReminder": { + "title": "Daily mood reminder", + "description": "Gentle 22:00 nudge when you haven't logged a mood for the day. Routes only through channels you've already configured (Telegram, ntfy, Web Push, Apple Push) and stays silent once you've logged today.", + "toggleAria": "Toggle daily mood reminder", + "statusOn": "On at 22:00 your time", + "statusOff": "Off", + "enabledToast": "Mood reminder enabled.", + "disabledToast": "Mood reminder disabled.", + "saveError": "Save failed. Please try again." } }, "i18n": { diff --git a/messages/es.json b/messages/es.json index 65cb0d819..9503a3a0c 100644 --- a/messages/es.json +++ b/messages/es.json @@ -2790,6 +2790,16 @@ }, "user": { "telegramTestBody": "HealthLog: conexión correcta. Las notificaciones de Telegram están activas." + }, + "moodReminder": { + "title": "Recordatorio diario de ánimo", + "description": "Aviso suave a las 22:00 si aún no has registrado el ánimo del día. Se envía solo por canales ya configurados (Telegram, ntfy, Web Push, Apple Push) y permanece en silencio una vez que has registrado el ánimo hoy.", + "toggleAria": "Alternar recordatorio diario de ánimo", + "statusOn": "Activo a las 22:00 hora local", + "statusOff": "Desactivado", + "enabledToast": "Recordatorio de ánimo activado.", + "disabledToast": "Recordatorio de ánimo desactivado.", + "saveError": "No se pudo guardar. Inténtalo de nuevo." } }, "i18n": { diff --git a/messages/fr.json b/messages/fr.json index 3cdfe68da..20dd1b199 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -2790,6 +2790,16 @@ }, "user": { "telegramTestBody": "HealthLog: connexion réussie. Les notifications Telegram sont actives." + }, + "moodReminder": { + "title": "Rappel quotidien d’humeur", + "description": "Petit rappel à 22:00 si tu n’as pas encore noté ton humeur du jour. Utilise uniquement les canaux déjà configurés (Telegram, ntfy, Web Push, Apple Push) et reste silencieux dès que tu as noté ton humeur aujourd’hui.", + "toggleAria": "Activer / désactiver le rappel quotidien d’humeur", + "statusOn": "Actif à 22:00 heure locale", + "statusOff": "Désactivé", + "enabledToast": "Rappel d’humeur activé.", + "disabledToast": "Rappel d’humeur désactivé.", + "saveError": "Échec de l’enregistrement. Réessayez." } }, "i18n": { @@ -2990,6 +3000,6 @@ }, "moodReminders": { "dailyTitle": "Enregistrer votre humeur", - "dailyBody": "Comment vous sentez-vous aujourdhui ?" + "dailyBody": "Comment vous sentez-vous aujourd’hui ?" } } diff --git a/messages/it.json b/messages/it.json index 12d69c952..c61b76936 100644 --- a/messages/it.json +++ b/messages/it.json @@ -2790,6 +2790,16 @@ }, "user": { "telegramTestBody": "HealthLog: connessione riuscita. Le notifiche Telegram sono attive." + }, + "moodReminder": { + "title": "Promemoria quotidiano dell’umore", + "description": "Promemoria leggero alle 22:00 se non hai ancora registrato l’umore di oggi. Usa solo i canali già configurati (Telegram, ntfy, Web Push, Apple Push) e tace una volta registrato l’umore di oggi.", + "toggleAria": "Attiva o disattiva il promemoria quotidiano dell’umore", + "statusOn": "Attivo alle 22:00 ora locale", + "statusOff": "Disattivato", + "enabledToast": "Promemoria dell’umore attivato.", + "disabledToast": "Promemoria dell’umore disattivato.", + "saveError": "Salvataggio non riuscito. Riprova." } }, "i18n": { diff --git a/messages/pl.json b/messages/pl.json index 7afc2bf37..7c485f648 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -2790,6 +2790,16 @@ }, "user": { "telegramTestBody": "HealthLog: połączenie udane. Powiadomienia Telegram są aktywne." + }, + "moodReminder": { + "title": "Codzienne przypomnienie o nastroju", + "description": "Delikatne przypomnienie o 22:00, jeśli dziś jeszcze nie zapisano nastroju. Wysyłane tylko skonfigurowanymi już kanałami (Telegram, ntfy, Web Push, Apple Push) i milknie, gdy dziś zapisano nastrój.", + "toggleAria": "Włącz lub wyłącz codzienne przypomnienie o nastroju", + "statusOn": "Aktywne o 22:00 czasu lokalnego", + "statusOff": "Wyłączone", + "enabledToast": "Przypomnienie o nastroju włączone.", + "disabledToast": "Przypomnienie o nastroju wyłączone.", + "saveError": "Zapis nieudany. Spróbuj ponownie." } }, "i18n": { diff --git a/package.json b/package.json index baf8fbd2c..ad0daf97c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "healthlog", - "version": "1.4.38.1", + "version": "1.4.38.2", "description": "Self-hosted personal-health-tracking PWA with Withings integration, AI insights, and doctor-report PDF export.", "license": "AGPL-3.0-only", "homepage": "https://healthlog.dev", diff --git a/src/app/api/user/profile/route.ts b/src/app/api/user/profile/route.ts index bed82717a..60aff0fa9 100644 --- a/src/app/api/user/profile/route.ts +++ b/src/app/api/user/profile/route.ts @@ -35,6 +35,7 @@ export const GET = apiHandler(async () => { heightCm: true, locale: true, timezone: true, + moodReminderEnabled: true, }, }); @@ -47,6 +48,7 @@ export const GET = apiHandler(async () => { heightCm: dbUser?.heightCm ?? null, locale: dbUser?.locale ?? null, timezone: dbUser?.timezone ?? "Europe/Berlin", + moodReminderEnabled: dbUser?.moodReminderEnabled ?? false, }); }); @@ -72,5 +74,6 @@ export const PATCH = apiHandler(async (request: NextRequest) => { heightCm: result.user.heightCm, locale: result.user.locale, timezone: result.user.timezone, + moodReminderEnabled: result.user.moodReminderEnabled, }); }); diff --git a/src/components/settings/mood-reminder-card.tsx b/src/components/settings/mood-reminder-card.tsx new file mode 100644 index 000000000..2029a9d66 --- /dev/null +++ b/src/components/settings/mood-reminder-card.tsx @@ -0,0 +1,109 @@ +"use client"; + +import { useState } from "react"; +import { useQuery, useQueryClient } from "@tanstack/react-query"; +import { SmilePlus } from "lucide-react"; + +import { Switch } from "@/components/ui/switch"; +import { SettingsCardHeader } from "@/components/settings/_card-header"; +import { useTranslations } from "@/lib/i18n/context"; + +interface ProfileShape { + moodReminderEnabled: boolean; +} + +export function MoodReminderCard({ + isAuthenticated, +}: { + isAuthenticated: boolean; +}) { + const { t } = useTranslations(); + const queryClient = useQueryClient(); + const [optimistic, setOptimistic] = useState(null); + const [saving, setSaving] = useState(false); + const [msg, setMsg] = useState(null); + const [msgType, setMsgType] = useState<"success" | "error" | null>(null); + + const { data: profile } = useQuery({ + queryKey: ["user", "profile"], + queryFn: async () => { + const res = await fetch("/api/user/profile"); + if (!res.ok) throw new Error("Failed to load profile"); + return (await res.json()).data as ProfileShape; + }, + enabled: isAuthenticated, + }); + + const enabled = optimistic ?? profile?.moodReminderEnabled ?? false; + + async function handleToggle(next: boolean) { + setOptimistic(next); + setSaving(true); + setMsg(null); + setMsgType(null); + + const res = await fetch("/api/user/profile", { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ moodReminderEnabled: next }), + }); + + if (res.ok) { + setMsg( + next + ? t("notifications.moodReminder.enabledToast") + : t("notifications.moodReminder.disabledToast"), + ); + setMsgType("success"); + await queryClient.invalidateQueries({ queryKey: ["user", "profile"] }); + setOptimistic(null); + } else { + setOptimistic(null); + setMsg(t("notifications.moodReminder.saveError")); + setMsgType("error"); + } + setSaving(false); + } + + return ( +
+ + + + {enabled + ? t("notifications.moodReminder.statusOn") + : t("notifications.moodReminder.statusOff")} + + + } + /> + {msg && ( +

+ {msg} +

+ )} +
+ ); +} diff --git a/src/components/settings/notifications-section.tsx b/src/components/settings/notifications-section.tsx index 0fc0a950e..2977b0b62 100644 --- a/src/components/settings/notifications-section.tsx +++ b/src/components/settings/notifications-section.tsx @@ -6,6 +6,7 @@ import { ChevronRight } from "lucide-react"; import { useAuth } from "@/hooks/use-auth"; import { useTranslations } from "@/lib/i18n/context"; +import { MoodReminderCard } from "@/components/settings/mood-reminder-card"; import { NotificationStatusCard } from "@/components/settings/notification-status-card"; import { NtfyCard } from "@/components/settings/ntfy-card"; import { TelegramCard } from "@/components/settings/telegram-card"; @@ -105,6 +106,9 @@ export function NotificationsSection() { )} +
+ +
); } diff --git a/src/lib/auth/profile-update.ts b/src/lib/auth/profile-update.ts index b8d74da65..50b7c59a6 100644 --- a/src/lib/auth/profile-update.ts +++ b/src/lib/auth/profile-update.ts @@ -18,6 +18,7 @@ const extendedProfileSchema = profileSchema.extend({ .max(64) .refine(isValidTimezone, "Invalid IANA timezone") .optional(), + moodReminderEnabled: z.boolean().optional(), }); export type ApplyProfileInput = z.infer; @@ -35,6 +36,7 @@ export interface ApplyProfileResult { gender: string | null; timezone: string; locale: string | null; + moodReminderEnabled: boolean; }; } @@ -91,6 +93,9 @@ export async function applyProfileUpdate( } if (data.locale !== undefined) updates.locale = data.locale; if (data.timezone !== undefined) updates.timezone = data.timezone; + if (data.moodReminderEnabled !== undefined) { + updates.moodReminderEnabled = data.moodReminderEnabled; + } // v1.4.18 — capture the prior locale so we can emit a separate // `settings.locale.update` audit row when the locale actually @@ -138,6 +143,7 @@ export async function applyProfileUpdate( gender: updatedUser.gender, timezone: updatedUser.timezone, locale: updatedUser.locale, + moodReminderEnabled: updatedUser.moodReminderEnabled, }, }; } diff --git a/src/lib/jobs/__tests__/mood-reminder.test.ts b/src/lib/jobs/__tests__/mood-reminder.test.ts index d6b633105..03a630667 100644 --- a/src/lib/jobs/__tests__/mood-reminder.test.ts +++ b/src/lib/jobs/__tests__/mood-reminder.test.ts @@ -1,16 +1,22 @@ /** * v0.5.4 ios-coord — mood-reminder dispatcher unit tests. * - * Pins the contract that motivated the v0.5.4 server-side patch: + * Pins the contract that motivated the v0.5.4 server-side patch and + * the v1.4.38.2 hotfix bundle: * - Opt-in gating: `moodReminderEnabled = false` ⇒ never dispatch. * - Window gating: only fires inside the 22:00 local-time hour. * - Logged-today skip: a `MoodEntry` row for the local date short- * circuits the dispatch. * - Idempotency: a `MoodReminderDispatch` row blocks a second push - * for the same (user, date) — even when the cron re-ticks inside - * the same 22:00 window. - * - Locale: title + body come from `messages/{de,en}.json` and fall - * back to the app default when the user's locale is null/unknown. + * for the same (user, date). + * - Locale: title + body come from the user's locale across all six + * supported locales (de/en/es/fr/it/pl) and fall back to the app + * default for null / unknown locales. + * - Ledger-after-delivery: a dispatcher that reports `dispatched = + * false` (no channel succeeded) does NOT write a ledger row, so + * the next tick is free to retry. + * - Per-user try-wrapper: an exception in one user's processing + * does not abort the tick for the rest of the candidates. * * The tests stub the Prisma surface manually to keep this file free of * a testcontainer boot. @@ -24,15 +30,27 @@ import { MOOD_REMINDER_LOCAL_HOUR, } from "../mood-reminder"; import type { NotificationPayload } from "@/lib/notifications/types"; +import type { DispatchOutcome } from "@/lib/notifications/dispatcher"; -type DispatchFn = (payload: NotificationPayload) => Promise; +type DispatchFn = (payload: NotificationPayload) => Promise; + +const OK: DispatchOutcome = { + dispatched: true, + channelsAttempted: 1, + channelsSucceeded: 1, +}; + +const NO_CHANNEL: DispatchOutcome = { + dispatched: false, + channelsAttempted: 0, + channelsSucceeded: 0, +}; interface FakePrismaState { candidates: Array<{ id: string; timezone: string; locale: string | null; - moodReminderEnabled: boolean; }>; moodEntries: Array<{ userId: string; date: string }>; dispatches: Array<{ userId: string; date: string }>; @@ -43,13 +61,7 @@ interface FakePrismaState { function makePrisma(state: FakePrismaState) { return { user: { - findMany: vi.fn( - async ({ where }: { where: { moodReminderEnabled: boolean } }) => { - return state.candidates.filter( - (c) => c.moodReminderEnabled === where.moodReminderEnabled, - ); - }, - ), + findMany: vi.fn(async () => state.candidates), }, moodEntry: { findFirst: vi.fn( @@ -81,8 +93,6 @@ function makePrisma(state: FakePrismaState) { create: vi.fn( async ({ data }: { data: { userId: string; date: string } }) => { if (state.raceUserIds.has(data.userId)) { - // Mirrors the Prisma `P2002` shape closely enough that the - // handler's substring match (`includes("P2002")`) catches it. throw new Error( "Invalid `prisma.moodReminderDispatch.create()` invocation: " + "Unique constraint failed on the fields: (`user_id`,`date`). " + @@ -103,7 +113,7 @@ beforeEach(() => { describe("evaluateMoodReminderWindow", () => { it("returns fire=false when the user is opted out", () => { - const at22 = new Date("2026-05-17T20:00:00Z"); // 22:00 Europe/Berlin + const at22 = new Date("2026-05-17T20:00:00Z"); const r = evaluateMoodReminderWindow( { timezone: "Europe/Berlin", moodReminderEnabled: false }, at22, @@ -113,7 +123,7 @@ describe("evaluateMoodReminderWindow", () => { }); it("fires inside the 22:00 hour and returns the local date string", () => { - const at22 = new Date("2026-05-17T20:00:00Z"); // 22:00 Europe/Berlin (CEST) + const at22 = new Date("2026-05-17T20:00:00Z"); const r = evaluateMoodReminderWindow( { timezone: "Europe/Berlin", moodReminderEnabled: true }, at22, @@ -123,8 +133,8 @@ describe("evaluateMoodReminderWindow", () => { expect(r.localHour).toBe(MOOD_REMINDER_LOCAL_HOUR); }); - it("does NOT fire at 21:59 local time (off-by-one guard)", () => { - const at2159 = new Date("2026-05-17T19:59:00Z"); // 21:59 Europe/Berlin + it("does NOT fire at 21:59 local time", () => { + const at2159 = new Date("2026-05-17T19:59:00Z"); const r = evaluateMoodReminderWindow( { timezone: "Europe/Berlin", moodReminderEnabled: true }, at2159, @@ -133,8 +143,8 @@ describe("evaluateMoodReminderWindow", () => { expect(r.localHour).toBe(21); }); - it("does NOT fire at 23:00 local time (window end)", () => { - const at23 = new Date("2026-05-17T21:00:00Z"); // 23:00 Europe/Berlin + it("does NOT fire at 23:00 local time", () => { + const at23 = new Date("2026-05-17T21:00:00Z"); const r = evaluateMoodReminderWindow( { timezone: "Europe/Berlin", moodReminderEnabled: true }, at23, @@ -143,8 +153,7 @@ describe("evaluateMoodReminderWindow", () => { expect(r.localHour).toBe(23); }); - it("respects a non-Berlin timezone (America/New_York is 6h behind in DST)", () => { - // 02:00 UTC on 2026-05-18 = 22:00 EDT on 2026-05-17 (UTC-4 during DST). + it("respects a non-Berlin timezone", () => { const utc = new Date("2026-05-18T02:00:00Z"); const r = evaluateMoodReminderWindow( { timezone: "America/New_York", moodReminderEnabled: true }, @@ -168,37 +177,48 @@ describe("buildMoodReminderPayload", () => { expect(p.body).toBe("How are you feeling today?"); }); + it.each(["es", "fr", "it", "pl"] as const)( + "returns native (non-EN) strings for locale=%s", + (locale) => { + const p = buildMoodReminderPayload(locale); + const enTitle = buildMoodReminderPayload("en").title; + const enBody = buildMoodReminderPayload("en").body; + expect(p.title.length).toBeGreaterThan(0); + expect(p.body.length).toBeGreaterThan(0); + expect(p.title).not.toBe(enTitle); + expect(p.body).not.toBe(enBody); + }, + ); + it("falls back to the app default for null / unknown locales", () => { const p1 = buildMoodReminderPayload(null); const p2 = buildMoodReminderPayload("zz-ZZ"); - // Both should resolve to the same content (the app default). expect(p1.title).toBe(p2.title); expect(p1.title.length).toBeGreaterThan(0); }); + + it("FR body carries the apostrophe (regression: v0.5.4 shipped 'aujourdhui')", () => { + const p = buildMoodReminderPayload("fr"); + expect(p.body).toContain("aujourd’hui"); + expect(p.body).not.toContain("aujourdhui "); + }); }); describe("runMoodReminderTick", () => { - // 22:00 Europe/Berlin (CEST) ⇒ 20:00 UTC. const inWindow = new Date("2026-05-17T20:00:00Z"); - // 21:00 Europe/Berlin (CEST) ⇒ 19:00 UTC. const outOfWindow = new Date("2026-05-17T19:00:00Z"); it("dispatches MOOD_REMINDER for opted-in users in the 22:00 window", async () => { const state: FakePrismaState = { candidates: [ - { - id: "u-1", - timezone: "Europe/Berlin", - locale: "de", - moodReminderEnabled: true, - }, + { id: "u-1", timezone: "Europe/Berlin", locale: "de" }, ], moodEntries: [], dispatches: [], raceUserIds: new Set(), }; const prisma = makePrisma(state); - const dispatch = vi.fn(async () => {}); + const dispatch = vi.fn(async () => OK); const summary = await runMoodReminderTick(prisma as never, inWindow, { dispatch, @@ -223,19 +243,14 @@ describe("runMoodReminderTick", () => { it("does NOT dispatch for users outside the 22:00 window", async () => { const state: FakePrismaState = { candidates: [ - { - id: "u-1", - timezone: "Europe/Berlin", - locale: "de", - moodReminderEnabled: true, - }, + { id: "u-1", timezone: "Europe/Berlin", locale: "de" }, ], moodEntries: [], dispatches: [], raceUserIds: new Set(), }; const prisma = makePrisma(state); - const dispatch = vi.fn(async () => {}); + const dispatch = vi.fn(async () => OK); const summary = await runMoodReminderTick(prisma as never, outOfWindow, { dispatch, @@ -246,15 +261,15 @@ describe("runMoodReminderTick", () => { expect(dispatch).not.toHaveBeenCalled(); }); - it("never reads moodReminderEnabled=false users (opt-in gating)", async () => { + it("Prisma query filters to moodReminderEnabled=true users only", async () => { const state: FakePrismaState = { - candidates: [], // Prisma `where` already filters; we assert the call shape + candidates: [], moodEntries: [], dispatches: [], raceUserIds: new Set(), }; const prisma = makePrisma(state); - const dispatch = vi.fn(async () => {}); + const dispatch = vi.fn(async () => OK); await runMoodReminderTick(prisma as never, inWindow, { dispatch }); @@ -266,19 +281,14 @@ describe("runMoodReminderTick", () => { it("skips users who already logged a mood for the local date", async () => { const state: FakePrismaState = { candidates: [ - { - id: "u-1", - timezone: "Europe/Berlin", - locale: "de", - moodReminderEnabled: true, - }, + { id: "u-1", timezone: "Europe/Berlin", locale: "de" }, ], moodEntries: [{ userId: "u-1", date: "2026-05-17" }], dispatches: [], raceUserIds: new Set(), }; const prisma = makePrisma(state); - const dispatch = vi.fn(async () => {}); + const dispatch = vi.fn(async () => OK); const summary = await runMoodReminderTick(prisma as never, inWindow, { dispatch, @@ -293,20 +303,14 @@ describe("runMoodReminderTick", () => { it("idempotency: a second tick inside the same window does not re-send", async () => { const state: FakePrismaState = { candidates: [ - { - id: "u-1", - timezone: "Europe/Berlin", - locale: "de", - moodReminderEnabled: true, - }, + { id: "u-1", timezone: "Europe/Berlin", locale: "de" }, ], moodEntries: [], - // Existing dispatch row blocks the push. dispatches: [{ userId: "u-1", date: "2026-05-17" }], raceUserIds: new Set(), }; const prisma = makePrisma(state); - const dispatch = vi.fn(async () => {}); + const dispatch = vi.fn(async () => OK); const summary = await runMoodReminderTick(prisma as never, inWindow, { dispatch, @@ -317,54 +321,93 @@ describe("runMoodReminderTick", () => { expect(dispatch).not.toHaveBeenCalled(); }); - it("idempotency: lost P2002 race is swallowed as 'already dispatched'", async () => { + it("ledger-after-delivery: dispatcher reporting dispatched=false leaves the slot free", async () => { const state: FakePrismaState = { candidates: [ - { - id: "u-1", - timezone: "Europe/Berlin", - locale: "de", - moodReminderEnabled: true, - }, + { id: "u-1", timezone: "Europe/Berlin", locale: "de" }, + ], + moodEntries: [], + dispatches: [], + raceUserIds: new Set(), + }; + const prisma = makePrisma(state); + const dispatch = vi.fn(async () => NO_CHANNEL); + + const summary = await runMoodReminderTick(prisma as never, inWindow, { + dispatch, + }); + + expect(summary.dispatched).toBe(0); + expect(summary.skippedNoChannel).toBe(1); + expect(dispatch).toHaveBeenCalledTimes(1); + // Ledger MUST stay empty so the next tick (or next request after the + // user adds a channel) is free to retry. + expect(state.dispatches).toHaveLength(0); + expect(prisma.moodReminderDispatch.create).not.toHaveBeenCalled(); + }); + + it("P2002 race: counts as dispatched (both workers pushed, one ledger row survives)", async () => { + const state: FakePrismaState = { + candidates: [ + { id: "u-1", timezone: "Europe/Berlin", locale: "de" }, ], moodEntries: [], dispatches: [], raceUserIds: new Set(["u-1"]), }; const prisma = makePrisma(state); - const dispatch = vi.fn(async () => {}); + const dispatch = vi.fn(async () => OK); const summary = await runMoodReminderTick(prisma as never, inWindow, { dispatch, }); - expect(summary.dispatched).toBe(0); - expect(summary.skippedAlreadyDispatched).toBe(1); - expect(dispatch).not.toHaveBeenCalled(); + // The dispatcher delivered (OK), the ledger insert hit P2002. The + // user already received the push from this worker; we count it as + // dispatched and move on. + expect(summary.dispatched).toBe(1); + expect(dispatch).toHaveBeenCalledTimes(1); }); - it("dispatches per-user with their own locale (DE + EN mix)", async () => { + it("per-user try-wrapper: one bad user does not abort the rest of the tick", async () => { const state: FakePrismaState = { candidates: [ - { - id: "u-de", - timezone: "Europe/Berlin", - locale: "de", - moodReminderEnabled: true, - }, - { - id: "u-en", - timezone: "Europe/Berlin", - locale: "en", - moodReminderEnabled: true, - }, + { id: "u-bad", timezone: "Europe/Berlin", locale: "de" }, + { id: "u-ok", timezone: "Europe/Berlin", locale: "en" }, ], moodEntries: [], dispatches: [], raceUserIds: new Set(), }; const prisma = makePrisma(state); - const dispatch = vi.fn(async () => {}); + const dispatch = vi.fn(async (payload) => { + if (payload.userId === "u-bad") { + throw new Error("simulated dispatcher throw"); + } + return OK; + }); + + const summary = await runMoodReminderTick(prisma as never, inWindow, { + dispatch, + }); + + expect(summary.dispatched).toBe(1); + expect(summary.failed).toBe(1); + expect(state.dispatches.map((d) => d.userId)).toEqual(["u-ok"]); + }); + + it("dispatches per-user with their own locale (DE + EN mix)", async () => { + const state: FakePrismaState = { + candidates: [ + { id: "u-de", timezone: "Europe/Berlin", locale: "de" }, + { id: "u-en", timezone: "Europe/Berlin", locale: "en" }, + ], + moodEntries: [], + dispatches: [], + raceUserIds: new Set(), + }; + const prisma = makePrisma(state); + const dispatch = vi.fn(async () => OK); const summary = await runMoodReminderTick(prisma as never, inWindow, { dispatch, @@ -379,4 +422,31 @@ describe("runMoodReminderTick", () => { expect(titles).toContain("Stimmung erfassen"); expect(titles).toContain("Log your mood"); }); + + it("dispatches per-user locale for fr/es/it/pl (regression: resolver dropped 4/6)", async () => { + const state: FakePrismaState = { + candidates: [ + { id: "u-fr", timezone: "Europe/Berlin", locale: "fr" }, + { id: "u-es", timezone: "Europe/Berlin", locale: "es" }, + { id: "u-it", timezone: "Europe/Berlin", locale: "it" }, + { id: "u-pl", timezone: "Europe/Berlin", locale: "pl" }, + ], + moodEntries: [], + dispatches: [], + raceUserIds: new Set(), + }; + const prisma = makePrisma(state); + const dispatch = vi.fn(async () => OK); + + await runMoodReminderTick(prisma as never, inWindow, { dispatch }); + + const titles = dispatch.mock.calls.map((c) => c[0].title); + const enTitle = buildMoodReminderPayload("en").title; + // Each non-EN user must get a title that is NOT the English string. + expect(titles).toHaveLength(4); + for (const t of titles) { + expect(t).not.toBe(enTitle); + expect(t.length).toBeGreaterThan(0); + } + }); }); diff --git a/src/lib/jobs/mood-reminder.ts b/src/lib/jobs/mood-reminder.ts index 9ad52ca8a..0babb8221 100644 --- a/src/lib/jobs/mood-reminder.ts +++ b/src/lib/jobs/mood-reminder.ts @@ -10,8 +10,9 @@ * 3. Skips users who already received a mood reminder today * (idempotency via `MoodReminderDispatch` row). * 4. Dispatches a `MOOD_REMINDER` notification with a locale-aware - * title + body and records the dispatch ledger row in the same - * transaction so a concurrent worker can't double-fire. + * title + body. The ledger row is written only after the dispatcher + * confirms at least one channel delivered successfully, so a + * transient APNs / network blip leaves the next tick free to retry. * * Why a separate module instead of folding it into reminder-worker.ts: * the worker is already 1800 LOC. Splitting the mood-reminder logic out @@ -21,9 +22,10 @@ import type { PrismaClient } from "@/generated/prisma/client"; import { getServerTranslator } from "@/lib/i18n/server-translator"; import type { Locale } from "@/lib/i18n/config"; -import { defaultLocale } from "@/lib/i18n/config"; +import { defaultLocale, locales } from "@/lib/i18n/config"; import { getLocalDateParts } from "@/lib/timezone"; import { dispatchNotification } from "@/lib/notifications/dispatcher"; +import { getEvent } from "@/lib/logging/context"; /** * Hour of day (in the user's local timezone) at which the mood reminder @@ -36,18 +38,6 @@ import { dispatchNotification } from "@/lib/notifications/dispatcher"; */ export const MOOD_REMINDER_LOCAL_HOUR = 22; -/** - * Minimal user shape needed to decide whether to dispatch. Defined - * explicitly so the handler can run against either the global Prisma - * client or the per-worker singleton without re-deriving the type. - */ -export interface MoodReminderCandidate { - id: string; - timezone: string; - locale: string | null; - moodReminderEnabled: boolean; -} - export interface MoodReminderSummary { candidatesScanned: number; inWindow: number; @@ -55,10 +45,14 @@ export interface MoodReminderSummary { skippedAlreadyLogged: number; skippedAlreadyDispatched: number; skippedOutsideWindow: number; + skippedNoChannel: number; + failed: number; } function resolveLocale(locale: string | null | undefined): Locale { - return locale === "en" || locale === "de" ? locale : defaultLocale; + return locales.includes(locale as Locale) + ? (locale as Locale) + : defaultLocale; } /** @@ -74,7 +68,7 @@ function resolveLocale(locale: string | null | undefined): Locale { * 23:00 → null) without touching the DB. */ export function evaluateMoodReminderWindow( - user: Pick, + user: { timezone: string; moodReminderEnabled: boolean }, now: Date, ): { fire: boolean; localDate: string | null; localHour: number } { if (!user.moodReminderEnabled) { @@ -115,10 +109,15 @@ export function buildMoodReminderPayload(locale: string | null | undefined): { * * The dedup ledger (`MoodReminderDispatch`) is the idempotency anchor: * the `(userId, date)` unique constraint guarantees that two parallel - * workers (or a re-tick inside the same hour) won't double-fire. + * workers (or a re-tick inside the same hour) won't double-fire. The + * ledger row is inserted only after the dispatcher confirms at least + * one channel delivered, so a transient APNs blip leaves the slot free + * for the next tick to retry rather than silently nuking the day's + * nudge. * - * Returns a summary so the worker can attach it to the wide-event for - * the observability dashboards. + * Per-user processing is wrapped in its own try/catch so a single bad + * row (corrupt timezone, dispatcher exception) cannot abort the rest + * of the tick. */ export async function runMoodReminderTick( prisma: PrismaClient, @@ -136,99 +135,99 @@ export async function runMoodReminderTick( skippedAlreadyLogged: 0, skippedAlreadyDispatched: 0, skippedOutsideWindow: 0, + skippedNoChannel: 0, + failed: 0, }; const candidates = await prisma.user.findMany({ where: { moodReminderEnabled: true }, - select: { - id: true, - timezone: true, - locale: true, - moodReminderEnabled: true, - }, + select: { id: true, timezone: true, locale: true }, }); for (const user of candidates) { summary.candidatesScanned += 1; - const decision = evaluateMoodReminderWindow( - { - timezone: user.timezone, - moodReminderEnabled: user.moodReminderEnabled, - }, - now, - ); - - if (!decision.fire || !decision.localDate) { - summary.skippedOutsideWindow += 1; - continue; - } - - summary.inWindow += 1; - - // Already nudged today? The dedup row is the source of truth — if - // a previous tick succeeded, this branch swallows the work. We - // check before the MoodEntry lookup because the ledger query is - // strictly cheaper (single indexed row) than scanning today's - // mood entries. - const existingDispatch = await prisma.moodReminderDispatch.findUnique({ - where: { userId_date: { userId: user.id, date: decision.localDate } }, - select: { id: true }, - }); - if (existingDispatch) { - summary.skippedAlreadyDispatched += 1; - continue; - } - - // Skip when the user already logged a mood for the local date. Same - // anchoring convention as `MoodEntry.date` (YYYY-MM-DD in the user's - // current timezone). Legacy rows that pre-date the `tz` column are - // already pinned to Europe/Berlin via the storage convention; here - // we accept whatever `date` string the row carries because the - // user-experience contract is "I logged today, don't nudge me". - const existingMood = await prisma.moodEntry.findFirst({ - where: { userId: user.id, date: decision.localDate }, - select: { id: true }, - }); - if (existingMood) { - summary.skippedAlreadyLogged += 1; - continue; - } - - // Reserve the dedup row FIRST so a parallel worker that races us - // hits the unique-constraint violation and bails before fanning a - // duplicate push. We catch the P2002 unique-violation path - // explicitly because a "lost the race" outcome is not a worker - // error — it's the idempotency contract doing its job. try { - await prisma.moodReminderDispatch.create({ - data: { userId: user.id, date: decision.localDate }, + const decision = evaluateMoodReminderWindow( + { timezone: user.timezone, moodReminderEnabled: true }, + now, + ); + + if (!decision.fire || !decision.localDate) { + summary.skippedOutsideWindow += 1; + continue; + } + + summary.inWindow += 1; + + // Cheaper guard first: the ledger lookup is a single indexed row + // while the mood-entry scan touches today's mood writes. + const existingDispatch = await prisma.moodReminderDispatch.findUnique({ + where: { userId_date: { userId: user.id, date: decision.localDate } }, + select: { id: true }, }); - } catch (err: unknown) { - // Prisma encodes unique-violation as `code === 'P2002'`. Detect - // by string-includes rather than instanceof so this module - // doesn't pull in the Prisma runtime types just for the guard. - const message = err instanceof Error ? err.message : String(err); - if (message.includes("P2002") || message.includes("Unique constraint")) { + if (existingDispatch) { summary.skippedAlreadyDispatched += 1; continue; } - throw err; + + const existingMood = await prisma.moodEntry.findFirst({ + where: { userId: user.id, date: decision.localDate }, + select: { id: true }, + }); + if (existingMood) { + summary.skippedAlreadyLogged += 1; + continue; + } + + const { title, body } = buildMoodReminderPayload(user.locale); + + const outcome = await dispatchImpl({ + eventType: "MOOD_REMINDER", + userId: user.id, + title, + message: body, + metadata: { + scheduledAt: now.toISOString(), + localDate: decision.localDate, + }, + }); + + // No channel succeeded — leave the ledger slot empty so the next + // tick can retry once the user adds a channel or the upstream + // recovers. The "at most once" trade is: extremely-rare + // concurrent-worker race may emit two pushes; in exchange a + // transient blip never silently nukes the nudge for the day. + if (!outcome.dispatched) { + summary.skippedNoChannel += 1; + continue; + } + + try { + await prisma.moodReminderDispatch.create({ + data: { userId: user.id, date: decision.localDate }, + }); + summary.dispatched += 1; + } catch (err: unknown) { + const message = err instanceof Error ? err.message : String(err); + if ( + message.includes("P2002") || + message.includes("Unique constraint") + ) { + // Lost a race against a parallel worker that also dispatched + // successfully — both pushed, only one ledger row survives. + // Count it as dispatched anyway (the user got the push). + summary.dispatched += 1; + } else { + throw err; + } + } + } catch (err: unknown) { + summary.failed += 1; + const message = err instanceof Error ? err.message : String(err); + getEvent()?.addWarning( + `mood-reminder per-user dispatch failed for ${user.id}: ${message}`, + ); } - - const { title, body } = buildMoodReminderPayload(user.locale); - - await dispatchImpl({ - eventType: "MOOD_REMINDER", - userId: user.id, - title, - message: body, - metadata: { - scheduledAt: now.toISOString(), - localDate: decision.localDate, - }, - }); - - summary.dispatched += 1; } return summary; diff --git a/src/lib/jobs/reminder-worker.ts b/src/lib/jobs/reminder-worker.ts index b03c24c62..a8f77f234 100644 --- a/src/lib/jobs/reminder-worker.ts +++ b/src/lib/jobs/reminder-worker.ts @@ -90,6 +90,7 @@ import { runOffhostBackup } from "@/lib/jobs/offhost-backup"; import { getUserTodayBounds as getUserTodayBoundsUtil, getDayOfWeekInTz as getDayOfWeekInTzUtil, + localHmAsUtc, } from "@/lib/timezone"; function parseTimeToMinutes(value: string): number { @@ -191,6 +192,14 @@ const DRAIN_CUMULATIVE_CRON = "45 3 * * *"; // `MoodReminderDispatch` ledger inside the handler. const MOOD_REMINDER_QUEUE = "mood-reminder-check"; const MOOD_REMINDER_CRON = "*/15 * * * *"; +// v1.4.38.2 — daily retention sweep for the mood-reminder dispatch +// ledger. Rows older than 90 days are behavioural footprints of +// mood-log gaps; we keep them long enough to debug a duplicate-push +// report (~one billing cycle) but no longer. Slots between the +// audit-log cleanup (03:15) and the drain (03:45). +const MOOD_REMINDER_CLEANUP_QUEUE = "mood-reminder-cleanup"; +const MOOD_REMINDER_CLEANUP_CRON = "25 3 * * *"; +const MOOD_REMINDER_RETENTION_DAYS = 90; // v1.4.38 — the per-sample cutoff hours constant now lives on the // helper module so the worker, the admin route, and the CLI all read // the same source of truth. Re-export pulled in alongside @@ -525,9 +534,12 @@ async function handleReminderCheck(jobs: Job[]) { // RED phase: create missed intake event if (currentPhase === "RED") { const [h, m] = schedule.windowStart.split(":").map(Number); - const scheduledFor = new Date( - todayStart.getTime() + h * 3600000 + m * 60000, - ); + // DST-safe: re-derive the offset at the target local time so + // the UTC instant is correct on spring-forward / fall-back + // days. `todayStart + h * 3.6e6` drifts by an hour twice a + // year — the iOS snooze action would otherwise pin against + // the wrong baseline. + const scheduledFor = localHmAsUtc(now, med.user.timezone, h, m); const existingMissed = await prisma.medicationIntakeEvent.count({ where: { @@ -578,12 +590,15 @@ async function handleReminderCheck(jobs: Job[]) { // v0.5.4 — surface the window-start as an ISO 8601 string so // the iOS notification handler can pin a "snooze 15 min" // action against the actual schedule slot rather than the - // wall-clock moment APNs delivered. Computed in UTC from the - // user's localised window-start; mirrors the RED-phase - // `scheduledFor` field on the intake event. + // wall-clock moment APNs delivered. DST-safe via + // `localHmAsUtc` so spring-forward / fall-back days don't + // drift the baseline by an hour. const [winH, winM] = schedule.windowStart.split(":").map(Number); - const scheduledAtIso = new Date( - todayStart.getTime() + winH * 3600000 + winM * 60000, + const scheduledAtIso = localHmAsUtc( + now, + med.user.timezone, + winH, + winM, ).toISOString(); evt.addMeta( @@ -1141,6 +1156,30 @@ async function handleMoodLogSync(jobs: Job[]) { * spinning up pg-boss. The handler is a thin shim that wires the worker * Prisma singleton + the wide-event sink to the pure function. */ +interface MoodReminderCleanupPayload { + triggeredAt: string; +} + +async function handleMoodReminderCleanup( + jobs: Job[], +) { + void jobs; + await withBackgroundEvent("job.mood_reminder_cleanup", async (evt) => { + const p = getWorkerPrisma(); + try { + const cutoff = new Date(); + cutoff.setUTCDate(cutoff.getUTCDate() - MOOD_REMINDER_RETENTION_DAYS); + const cutoffIso = cutoff.toISOString().slice(0, 10); + const deleted = await p.moodReminderDispatch.deleteMany({ + where: { date: { lt: cutoffIso } }, + }); + evt.addMeta("mood_reminder_cleanup_deleted", deleted.count); + } catch (err) { + evt.addWarning(`mood-reminder-cleanup failed: ${err}`); + } + }); +} + async function handleMoodReminderCheck(jobs: Job[]) { void jobs; await withBackgroundEvent("job.mood_reminder", async (evt) => { @@ -1626,6 +1665,7 @@ export async function startReminderWorker() { // entry the every-15-min schedule silently no-ops and the // dispatcher never fires. MOOD_REMINDER_QUEUE, + MOOD_REMINDER_CLEANUP_QUEUE, ]; for (const q of allQueues) { @@ -1687,6 +1727,7 @@ export async function startReminderWorker() { // time is the 22:00 hour, so the cron costs ~one user-row scan // per tick for the entire opted-in cohort. [MOOD_REMINDER_QUEUE, MOOD_REMINDER_CRON], + [MOOD_REMINDER_CLEANUP_QUEUE, MOOD_REMINDER_CLEANUP_CRON], ]; for (const [name, cron] of schedules) { @@ -1803,6 +1844,11 @@ export async function startReminderWorker() { { localConcurrency: 1 }, handleMoodReminderCheck, ); + await boss.work( + MOOD_REMINDER_CLEANUP_QUEUE, + { localConcurrency: 1 }, + handleMoodReminderCleanup, + ); await boss.work( PR_DETECTION_QUEUE, { localConcurrency: PR_DETECTION_CONCURRENCY }, diff --git a/src/lib/notifications/dispatcher.ts b/src/lib/notifications/dispatcher.ts index 07d07198f..f76f5f59d 100644 --- a/src/lib/notifications/dispatcher.ts +++ b/src/lib/notifications/dispatcher.ts @@ -36,9 +36,21 @@ import { getEvent } from "@/lib/logging/context"; * Also handles legacy Telegram config stored on the User model by * auto-migrating it to a NotificationChannel record on first dispatch. */ +export interface DispatchOutcome { + /** True when at least one channel reported a successful delivery. */ + dispatched: boolean; + /** Channels considered after enable / cooldown / preference filtering. */ + channelsAttempted: number; + /** Subset of attempted channels that returned ok. */ + channelsSucceeded: number; +} + export async function dispatchNotification( payload: NotificationPayload, -): Promise { +): Promise { + let channelsAttempted = 0; + let channelsSucceeded = 0; + try { const channels = await prisma.notificationChannel.findMany({ where: { userId: payload.userId, enabled: true }, @@ -143,6 +155,8 @@ export async function dispatchNotification( continue; } + channelsAttempted += 1; + try { const outcome = await sendToChannel( channel.type as ChannelType, @@ -156,6 +170,7 @@ export async function dispatchNotification( userId: channel.userId, type: channel.type as ChannelType, }); + channelsSucceeded += 1; continue; } @@ -209,6 +224,12 @@ export async function dispatchNotification( } catch (err) { getEvent()?.addWarning(`Notification dispatcher error: ${err}`); } + + return { + dispatched: channelsSucceeded > 0, + channelsAttempted, + channelsSucceeded, + }; } async function sendToChannel( diff --git a/src/lib/notifications/senders/apns.ts b/src/lib/notifications/senders/apns.ts index cce0a0d20..d23941bef 100644 --- a/src/lib/notifications/senders/apns.ts +++ b/src/lib/notifications/senders/apns.ts @@ -393,7 +393,7 @@ export async function sendViaApns( alert: { title: payload.title, body: stripHtml(payload.message) }, data: { eventType: payload.eventType, - ...(payload.metadata ?? {}), + ...pickIosMetadata(payload.metadata), }, threadId: payload.eventType, // The iOS app registers a `UNNotificationCategory` per event-type @@ -455,3 +455,36 @@ export async function sendViaApns( function stripHtml(s: string): string { return s.replace(/<[^>]*>/g, ""); } + +/** + * Whitelist the metadata keys we send through to iOS. The dispatcher + * payload's `metadata` field is shared across every channel — Telegram + * stuffs an `replyMarkup` keyboard in there, future channels will add + * their own extras — and forwarding the lot to APNs leaks + * channel-specific payload structures into the APNs userInfo Apple + * sees plus the iOS app's notification handler. + * + * Allowed keys are the ones the iOS app actually reads. Add a new key + * here as iOS gains support for it. + */ +const IOS_METADATA_ALLOWLIST = new Set([ + "scheduledAt", + "localDate", + "medicationId", + "scheduleId", + "phase", + "date", +]); + +function pickIosMetadata( + metadata: Record | undefined, +): Record { + if (!metadata) return {}; + const out: Record = {}; + for (const key of Object.keys(metadata)) { + if (IOS_METADATA_ALLOWLIST.has(key)) { + out[key] = metadata[key]; + } + } + return out; +} diff --git a/src/lib/timezone.ts b/src/lib/timezone.ts index 1e3790101..c28ebc326 100644 --- a/src/lib/timezone.ts +++ b/src/lib/timezone.ts @@ -104,3 +104,36 @@ export function getUserTodayBounds( export function getDayOfWeekInTz(now: Date, tz: string): number { return getLocalDateParts(now, tz).dayOfWeek; } + +/** + * Compute the UTC instant of local `hh:mm` on the local calendar day + * implied by `now` in `tz`. DST-safe — `todayStart.getTime() + h * 3.6e6` + * drifts by an hour on spring-forward / fall-back days because raw UTC + * arithmetic ignores the DST jump between local midnight and local + * `hh:mm`. This helper re-derives the offset at the target local time + * so the returned instant always represents local `hh:mm`. + */ +export function localHmAsUtc( + now: Date, + tz: string, + hour: number, + minute: number, +): Date { + const parts = getLocalDateParts(now, tz); + const localAtTargetAsUtc = new Date( + Date.UTC(parts.year, parts.month - 1, parts.day, hour, minute, 0, 0), + ); + const localNowAsUtc = new Date( + Date.UTC( + parts.year, + parts.month - 1, + parts.day, + parts.hour, + parts.minute, + parts.second, + ), + ); + const offsetMs = + Math.round((localNowAsUtc.getTime() - now.getTime()) / 60000) * 60000; + return new Date(localAtTargetAsUtc.getTime() - offsetMs); +}