From 9d5e6f4b42f5be2185f4f98a68ef1fc3c6cfca28 Mon Sep 17 00:00:00 2001 From: Robert Hamilton Date: Wed, 15 Apr 2026 10:31:13 -0500 Subject: [PATCH 1/7] docs(planning): F022 browser notifications -- Spec, Tech, Steps Phase 1 (Web Notifications API, tab-open only): - rest timer expiry notification via onExpired callback on startRestTimer - session reminder polling hook with 60s setInterval - settings UI permission status display and copy updates Out of scope: service workers, Web Push, Tauri changes (Phase 2 backlog) --- .../022-Browser-Notifications/Spec.md | 74 +++++++ .../022-Browser-Notifications/Steps.md | 205 +++++++++++++++++ .../022-Browser-Notifications/Tech.md | 209 ++++++++++++++++++ 3 files changed, 488 insertions(+) create mode 100644 Context/Features/022-Browser-Notifications/Spec.md create mode 100644 Context/Features/022-Browser-Notifications/Steps.md create mode 100644 Context/Features/022-Browser-Notifications/Tech.md diff --git a/Context/Features/022-Browser-Notifications/Spec.md b/Context/Features/022-Browser-Notifications/Spec.md new file mode 100644 index 0000000..20189cf --- /dev/null +++ b/Context/Features/022-Browser-Notifications/Spec.md @@ -0,0 +1,74 @@ +# Feature 022: Browser Notifications (Phase 1) + +## Overview + +Extend the existing Web Notifications API coverage in the browser to include rest timer alerts and session reminders. Currently, browser users only receive PR celebration notifications; all other notification types are silently skipped or blocked behind a "requires native app" gate. Phase 1 enables rest timer and session reminder notifications while the browser tab is open, bringing the web experience closer to parity with the Tauri mobile app. + +Phase 2 (separate backlog item) will add Web Push API support for background and closed-tab delivery. + +## Problem Statement + +Browser users who train without the mobile app miss two of the three notification types that make the app useful during a workout: rest timer alerts and session reminders. The rest timer in particular is a primary workout tool -- missing it means athletes must watch the screen instead of resting or preparing for their next set. The current settings UI actively discourages browser users from enabling session reminders by labeling them as native-only, even though basic in-tab delivery is feasible. + +## User Stories + +- As a browser user in an active workout, I want a system notification when my rest timer expires so that I can focus elsewhere during rest periods without watching the screen. +- As a browser user with a training program, I want to receive a session reminder notification before my scheduled workout so that I am not caught unprepared. +- As a user enabling notifications for the first time, I want to be prompted for browser notification permission at the moment it becomes relevant so that I understand why permission is needed. +- As a user whose notification permission has been denied, I want clear guidance in settings on how to re-enable it so that I am not left with a silently non-functional toggle. + +## Requirements + +### Must Have + +- When a rest timer expires in browser mode, fire a Web Notification with the exercise name and set context if notification permission is granted and the rest timer toggle is enabled. +- When a browser user has session reminders enabled and the tab is open, deliver a Web Notification at the configured advance time (`advanceMinutes`) before their scheduled session, provided the session has not been logged and quiet hours are not active. +- Session reminder polling in browser mode must track a "reminded today" state so the notification fires at most once per day per session. +- When a notification-triggering action would fire but browser permission is in the `default` state, prompt the user for permission contextually rather than silently dropping the notification. +- The notification settings UI must show the current browser permission state (`granted`, `denied`, `default`) for users in browser mode. +- When permission is `denied`, the settings UI must display a clear message and direct the user to their browser settings -- it must not show a re-request button (browsers do not allow re-requesting after denial). +- The "Session reminders require the native app" message must be replaced with an informational note that session reminders work but require the tab to remain open. + +### Should Have + +- The settings UI permission state display should update reactively if the user grants or denies permission in the browser settings dialog without reloading the page. +- Rest timer notifications in browser mode should include the same body copy as the Tauri version: exercise name and set context (e.g., "Barbell Squat -- Set 3 of 5"). + +### Won't Have (this iteration) + +- Web Push API / service worker for background or closed-tab delivery -- Phase 2. +- Event reminder notifications in browser mode -- Phase 2. +- Notification action buttons (e.g., "Open Workout") in browser notifications -- not supported consistently across browsers without a service worker. +- Sound or vibration control for browser notifications -- these are browser-controlled. +- Any changes to Rust/Tauri code. + +## Testable Assertions + +| ID | Assertion | Verification | +| ----- | ---------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------- | +| A-001 | When a rest timer expires in browser mode with permission granted and rest timer toggle on, a Web Notification fires immediately. | Start a rest timer in browser, let it expire, observe system notification. | +| A-002 | Rest timer notification body contains the exercise name and set number matching the active set. | Inspect notification body text against current workout context. | +| A-003 | When rest timer toggle is off, no notification fires on timer expiry even if permission is granted. | Toggle off rest timer alerts, expire a timer, confirm no notification. | +| A-004 | When master notifications toggle is off, no rest timer notification fires. | Disable master toggle, expire a timer, confirm no notification. | +| A-005 | When browser permission is `default` and a rest timer expires, the user is prompted for permission. | Start from permission=default, expire timer, observe permission dialog. | +| A-006 | When browser permission is `denied`, no notification fires and no permission prompt is shown. | Set permission to denied, expire timer, confirm no notification and no dialog. | +| A-007 | When session reminders are enabled in browser mode with the tab open, a Web Notification fires within 1 minute of the target time. | Enable session reminders, set advanceMinutes to a near-future window, observe notification arrives within 60 seconds of target. | +| A-008 | Session reminder notification fires at most once per day per scheduled session. | Trigger a session reminder, wait for the next poll cycle, confirm no duplicate notification fires. | +| A-009 | Session reminder does not fire if the scheduled session has already been logged. | Log a workout, confirm no reminder notification fires for that session. | +| A-010 | Session reminder does not fire if quiet hours are active. | Set quiet hours to cover the current time, confirm no session reminder notification. | +| A-011 | The settings UI shows the current browser permission state (granted / denied / default) in browser mode. | Inspect settings page in browser; permission status label matches actual `Notification.permission` value. | +| A-012 | When permission is `denied`, settings UI shows a message directing the user to browser settings, with no re-request button. | Set permission to denied, open settings, verify UI copy and absence of re-request button. | +| A-013 | The "Session reminders require the native app" message is not present in browser mode. | Open settings in browser mode, confirm old message string is absent. | +| A-014 | Session reminders section in browser mode shows an informational note that the tab must remain open. | Open settings in browser mode with session reminders enabled, read the note text. | + +## Open Questions + +- [ ] Does the rest timer in browser mode currently track expiry in a way the notification layer can hook into, or does a new JS timer need to be introduced alongside the display interpolation hook? The existing `use-timer-interpolation.ts` is display-only and does not emit an expiry event. This determines whether we extend that hook or add a separate timer hook. +- [ ] What Supabase query does the browser-mode session reminder polling need to make to determine if today's session is scheduled and not yet logged? The Rust implementation queries `program_activations`, `scheduled_sessions`, and `workout_logs`. The browser equivalent needs the same data via the Supabase JS client. +- [ ] Should the browser permission status in settings also be shown for Tauri users (where the Tauri plugin manages permission separately), or is it browser-only? Current assumption: browser-only display. + +## Revision History + +| Date | Change | ADR | +| ---------- | ------------ | --- | +| 2026-04-15 | Initial spec | -- | diff --git a/Context/Features/022-Browser-Notifications/Steps.md b/Context/Features/022-Browser-Notifications/Steps.md new file mode 100644 index 0000000..8519201 --- /dev/null +++ b/Context/Features/022-Browser-Notifications/Steps.md @@ -0,0 +1,205 @@ +# Implementation Steps: Browser Notifications (Phase 1) + +**Spec:** Context/Features/022-Browser-Notifications/Spec.md +**Tech:** Context/Features/022-Browser-Notifications/Tech.md + +## Progress + +- **Status:** Not started +- **Current task:** -- +- **Last milestone:** -- + +## Team Orchestration + +### Team Members + +- **builder** + - Role: Implements all React/TypeScript changes across store, service, hooks, and UI + - Agent Type: frontend-specialist + - Resume: true + +- **validator** + - Role: Read-only quality validation against Spec.md assertions and coding conventions + - Agent Type: quality-engineer + - Resume: false + +--- + +## Tasks + +### Phase 1: Store extension and notification service (parallel) + +- [ ] S001: Extend `active-workout-store.ts` rest timer browser path with an `onExpired?` callback + - Add module-level `let _onRestExpired: (() => void) | null = null` + - Extend `startRestTimer` interface and implementation: `startRestTimer(seconds, exerciseName?, setNumber?, onExpired?)` + - In the browser path of `startRestTimer`, store: `_onRestExpired = onExpired ?? null` + - In `tickRest()` when `newRemaining <= 0` (browser path only): call `_onRestExpired?.()` then set `_onRestExpired = null` before clearing the interval + - In `skipRest()`: set `_onRestExpired = null` (do NOT call it -- skip must not fire a notification) + - In `cleanup()`: set `_onRestExpired = null` + - **Assigned:** builder + - **Depends:** none + - **Parallel:** true + +- [ ] S001-T: Test `_onRestExpired` callback behavior in `active-workout-store` + - (callback fires exactly once when timer counts to zero in browser mode, callback is NOT called on `skipRest()`, callback is cleared before re-use when a new timer starts, `cleanup()` clears callback without calling it, Tauri path is unaffected) + - **Assigned:** builder + - **Depends:** S001 + - **Parallel:** false + +- [ ] S002: Add `sendRestTimerNotification` and `sendSessionReminderNotification` to `notification-service.ts` + - `sendRestTimerNotification(exerciseName: string | undefined, setNumber: number | undefined, prefs: NotificationPreferences): void` -- gates on `shouldSendNotification('restTimer', prefs)`, fires `new Notification('REST COMPLETE', { body })` using message copy from `docs/11-notification-design.md`; if `Notification.permission === 'default'`, calls `Notification.requestPermission()` before firing + - `sendSessionReminderNotification(sessionName: string, prefs: NotificationPreferences): void` -- gates on `shouldSendNotification('sessionReminders', prefs)`, fires `new Notification` with session name; same contextual permission-request pattern + - Both functions are no-ops when `'Notification' in window` is false or `Notification.permission === 'denied'` + - Follow the `sendPrNotification` pattern exactly (async guard, permission check, `new Notification`) + - **Assigned:** builder + - **Depends:** none + - **Parallel:** true + +- [ ] S002-T: Unit test `sendRestTimerNotification` and `sendSessionReminderNotification` + - (fires notification when permission granted and toggles on, no-op when master toggle off, no-op when type toggle off, no-op when permission denied, calls `requestPermission` when permission is default, rest timer notification is NOT blocked by quiet hours, session reminder notification IS blocked by quiet hours) + - Mock `window.Notification` globally in test setup + - **Assigned:** builder + - **Depends:** S002 + - **Parallel:** false + +🏁 MILESTONE: Phase 1 complete -- notification functions and store callback mechanism ready + +- Verify S001-T and S002-T pass: `bun run test` +- Verify no TypeScript errors: `bun run build` + +**Contracts:** + +- `src/stores/active-workout-store.ts` -- Extended `startRestTimer` signature with `onExpired?` param +- `src/lib/notification-service.ts` -- New `sendRestTimerNotification` and `sendSessionReminderNotification` exports + +--- + +### Phase 2: Session reminder hook and rest timer wiring (parallel) + +- [ ] S003: Create `src/hooks/use-session-reminder-browser.ts` + - Module-level `let _lastRemindedDate: string | null = null` (ISO `YYYY-MM-DD`) for "reminded today" guard + - Hook returns void; is a no-op when `isTauri()` returns true + - `useEffect` sets up a `setInterval` at 60 000 ms; cleans up on unmount + - Each poll tick calls an async `checkAndMaybeNotify()` function that: + 1. Reads prefs via `getNotificationPreferences()` + 2. Guards: `prefs.enabled`, `prefs.sessionReminders.enabled`, not in quiet hours, current hour between 06 and 20 inclusive, `_lastRemindedDate !== today` + 3. Queries Supabase directly (not TanStack Query) for: active program activation (`program_activations`), today's scheduled session (join `blocks β†’ block_weeks β†’ scheduled_sessions β†’ session_templates` filtered by JS `day_of_week`), workout logged today (`workout_logs.started_at` in today's range) + 4. If session exists and workout not logged: calls `sendSessionReminderNotification(sessionName, prefs)` and sets `_lastRemindedDate = today` + 5. If workout already logged: sets `_lastRemindedDate = today` and returns (suppress further checks) + - Error handling: wrap Supabase calls in `try/catch`; log with `[session-reminder-browser]` prefix and return silently on error (non-fatal, matches Rust behavior) + - Resolve the open question (Spec.md): check `workout_logs.started_at` column type in migrations to determine UTC vs local comparison -- use the same approach as `supabase-adapter.ts` existing queries + - **Assigned:** builder + - **Depends:** S002 + - **Parallel:** true + +- [ ] S003-T: Unit test `use-session-reminder-browser` polling logic + - (notification fires when all conditions pass, skips when master toggle off, skips when quiet hours active, skips when outside 06-20 time window, skips when workout already logged today, fires at most once per day, `_lastRemindedDate` persists across hook remounts, no-op in Tauri mode) + - Inject mock Supabase client and mock `sendSessionReminderNotification`; use `vi.useFakeTimers()` to advance intervals + - **Assigned:** builder + - **Depends:** S003 + - **Parallel:** false + +- [ ] S004: Wire `onExpired` callback in `src/hooks/use-active-workout.ts` + - The `confirmSet` callback at line 395 calls `storeStartRestTimer(restSeconds)` -- extend to pass `onExpired` + - Read prefs at call time via `getNotificationPreferences()` (async); hold in a ref or capture inside the callback closure + - Pass `onExpired: () => sendRestTimerNotification(exerciseName, setNumber, prefs)` to `startRestTimer` + - `exerciseName` and `setNumber` must be captured from the set/activity context available at the call site -- confirm these are in scope; if not, thread them through from the caller + - Do not call `sendRestTimerNotification` directly in `confirmSet` -- only register the callback; the store fires it at expiry + - **Assigned:** builder + - **Depends:** S001, S002 + - **Parallel:** true + +- [ ] S004-T: Integration test for rest timer notification wiring in `use-active-workout` + - (onExpired callback is passed to store when restSeconds > 0, no callback passed when restSeconds <= 0, callback calls sendRestTimerNotification with correct exerciseName and setNumber) + - Mock `active-workout-store.startRestTimer` and assert call shape; mock `notification-service` + - **Assigned:** builder + - **Depends:** S004 + - **Parallel:** false + +🏁 MILESTONE: Phase 2 complete -- rest timer and session reminder delivery paths wired + +- Verify A-001, A-003, A-004, A-007, A-008, A-009, A-010 via `bun run test` +- Manual smoke test: start a short rest timer in browser, let it expire -- confirm system notification appears + +**Contracts:** + +- `src/hooks/use-session-reminder-browser.ts` -- Hook ready to mount in authenticated layout + +--- + +### Phase 3: Layout mount and settings UI (parallel) + +- [ ] S005: Mount `useSessionReminderBrowser` in `src/routes/_authenticated.tsx` + - Import and call `useSessionReminderBrowser()` inside the authenticated layout component (one call, top-level) + - No render output; hook is a side-effect only + - Confirm the layout unmounts on sign-out (so the `setInterval` is cleaned up automatically) + - **Assigned:** builder + - **Depends:** S003 + - **Parallel:** true + +- [ ] S006: Update `src/components/profile/notification-settings.tsx` + - **Remove** the "Session reminders require the native app" message (line 182-184) for browser users + - **Add** in its place an informational note (browser mode only): "Notifications are delivered while this tab is open" -- styled with `text-[11px] leading-relaxed text-warm-ash/60`, consistent with existing copy + - **Add** `BrowserPermissionStatus` sub-component (browser mode only, shown below master toggle section): + - On mount, calls `navigator.permissions.query({ name: 'notifications' as PermissionName })` to get live `PermissionStatus`; subscribes to `status.onchange`; falls back to reading `Notification.permission` if Permissions API unavailable + - Renders a read-only status label in three states: + - `granted` -- no extra UI needed (silent) + - `default` -- small informational note: "Enable notifications in your browser when prompted" + - `denied` -- warning note: "Notifications are blocked. Enable them in your browser settings." (no re-request button -- browsers do not allow this after denial) + - Conditionally render only when `!isTauri()` and `'Notification' in window` + - Verify the existing `handleMasterToggle` permission-request flow is unchanged (D-4) + - **Assigned:** builder + - **Depends:** S002 + - **Parallel:** true + +- [ ] S006-T: Component test for updated `NotificationSettings` in browser mode + - (BrowserPermissionStatus renders correct copy for granted/default/denied states, "requires native app" string is absent, "tab must remain open" note is present when sessionReminders enabled, no re-request button when denied, component does not render BrowserPermissionStatus in Tauri mode) + - Mock `isTauri()` to return false; mock `navigator.permissions.query` for each permission state + - **Assigned:** builder + - **Depends:** S006 + - **Parallel:** false + +🏁 MILESTONE: Phase 3 complete -- feature fully wired and UI updated + +- Verify A-005, A-006, A-011, A-012, A-013, A-014 via `bun run test` +- Verify `bun run lint` clean +- Verify `bun run build` (TypeScript) passes + +--- + +### Phase 4: Validation + +- [ ] S007: Quality engineer validation pass + - Read all modified and created files + - Verify all 14 Testable Assertions from Spec.md are covered by implementation and tests + - Check error-handling conventions: no bare catch, all catches use `[module-name]` prefix + - Check state-management conventions: module-level setter validation in `use-session-reminder-browser.ts` + - Confirm "requires native app" string is absent from `notification-settings.tsx` + - Confirm `BrowserPermissionStatus` is not rendered in Tauri mode + - Confirm `onExpired` is NOT called by `skipRest()` or `cleanup()` + - Flag any TODO/FIXME stubs, missing error states, or silent failures + - **Assigned:** validator + - **Depends:** S001-T, S002-T, S003-T, S004-T, S005, S006-T + - **Parallel:** false + +🏁 MILESTONE: Feature complete -- all assertions verified, full drift check passed + +--- + +## Acceptance Criteria + +- [ ] All 14 testable assertions from Spec.md verified (A-001 through A-014) +- [ ] `bun run test` passes with no failures +- [ ] `bun run lint` clean +- [ ] `bun run build` TypeScript check passes +- [ ] "Session reminders require the native app" string absent from codebase +- [ ] `onExpired` callback fires on natural timer expiry and is silent on skip +- [ ] No TODO/FIXME stubs in production code + +## Validation Commands + +```bash +bun run test # Vitest -- all unit and component tests +bun run lint # ESLint +bun run build # TypeScript check + Vite build +``` diff --git a/Context/Features/022-Browser-Notifications/Tech.md b/Context/Features/022-Browser-Notifications/Tech.md new file mode 100644 index 0000000..1a9dc45 --- /dev/null +++ b/Context/Features/022-Browser-Notifications/Tech.md @@ -0,0 +1,209 @@ +# Tech Plan: Browser Notifications (Phase 1) + +**Spec:** Context/Features/022-Browser-Notifications/Spec.md +**Stacks involved:** React 19 / TypeScript + +## Architecture Overview + +The existing notification system is dual-mode: Tauri code paths use `tauri-plugin-notification` and Rust background workers; browser code paths use the Web Notifications API directly. Phase 1 extends the browser code paths for two notification types that currently either fire nothing (rest timer) or are gated behind a "requires native app" message (session reminders). + +Three layers are touched: + +1. **`notification-service.ts`** -- gains two new functions (`sendRestTimerNotification`, `sendSessionReminderNotification`) following the existing `sendPrNotification` pattern. +2. **`active-workout-store.ts`** -- gains a minimal extension to its browser rest timer path so callers can register an expiry callback. +3. **New files** -- `use-session-reminder-browser.ts` (polling hook) and settings UI changes to `notification-settings.tsx`. + +No Rust, Tauri, Supabase schema, or service worker changes. + +``` +active-workout-store (browser tickRest β†’ _onRestExpired callback) + β”‚ + β–Ό +notification-service.sendRestTimerNotification() + β”‚ + β–Ό +Web Notifications API (Notification.permission === 'granted') + +authenticated layout + β”‚ mounts + β–Ό +use-session-reminder-browser (setInterval 60s) + β”‚ polls Supabase directly + β–Ό +notification-service.sendSessionReminderNotification() + β”‚ + β–Ό +Web Notifications API +``` + +## Key Decisions + +### D-1: Rest timer expiry detection + +**Problem:** The browser path of `startRestTimer` in `active-workout-store.ts` runs a `setInterval` β†’ `tickRest()`. When `remaining` reaches 0, `tickRest()` clears the interval and sets `restTimer: null`. A notification must fire at that moment but the store should not import notification-service (violates separation of concerns). Skip and expiry both set `restTimer: null`, so a store subscriber cannot distinguish them. + +**Options considered:** + +- **Option A: Store subscriber with prev/curr diff** -- Subscribe to store outside the store, fire notification when `restTimer` transitions from non-null to null. Cannot distinguish skip from expiry without additional store state. Rejected. +- **Option B: Add `restTimerExpiredAt` field** -- Store records expiry timestamp distinct from skip. Works but adds permanent state for a transient event; every consumer must null-check. Rejected. +- **Option C: `onExpired` callback on `startRestTimer` (chosen)** -- Extend `startRestTimer` signature with an optional `onExpired?: () => void`. Store it in a module-level `_onRestExpired` variable (same pattern as existing `_restInterval`). Call it in `tickRest()` when expiry fires; clear it on `skipRest()` and `cleanup()`. Callers wire the notification at the call site. Distinguishes expiry from skip precisely because `skipRest()` clears the callback without calling it. + +**Chosen:** Option C + +**Rationale:** Minimal store surface change; no new store fields; skip/expiry distinction is preserved; the notification wiring is the caller's concern, not the store's. Consistent with the module-level variable pattern already used for `_restInterval`, `_unlistenTick`, and `_unlistenExpired`. + +**Implementation sketch:** + +```typescript +// active-workout-store.ts additions +let _onRestExpired: (() => void) | null = null + +// startRestTimer extended signature: +startRestTimer(seconds, exerciseName?, setNumber?, onExpired?) +// browser path stores: _onRestExpired = onExpired ?? null + +// tickRest() when newRemaining <= 0 (browser path only): +_onRestExpired?.() +_onRestExpired = null + +// skipRest() and cleanup(): clear without calling +_onRestExpired = null +``` + +--- + +### D-2: Session reminder polling architecture + +**Problem:** The Rust session reminder is a 60-second background loop. An equivalent browser implementation must run in JS without blocking the main thread, must start post-auth, and must clean up on sign-out. + +**Options considered:** + +- **Option A: `useEffect` + `setInterval` in authenticated layout** -- Simple; lifecycle tied to React mount/unmount. Automatically starts when layout mounts (post-auth) and stops when it unmounts (sign-out). Idiomatic React. +- **Option B: Dedicated hook `use-session-reminder-browser.ts`** -- Same mechanics as A but encapsulated. Easier to test, easier to disable in Tauri mode, clear ownership. No meaningful difference from A at runtime. +- **Option C: Module-level singleton** -- Starts once, not tied to React lifecycle. Harder to test; requires explicit start/stop calls at auth boundaries; not idiomatic. + +**Chosen:** Option B (dedicated hook) + +**Rationale:** Encapsulation makes the polling logic testable in isolation. The hook mounts in the authenticated layout (which already handles auth boundaries) and returns void -- no render output. Tauri mode guard (`if (isTauri()) return`) makes it a no-op in the native app without any conditional hook violation. + +**"Reminded today" state:** Module-level variable `let _lastRemindedDate: string | null` (ISO date `YYYY-MM-DD`). Survives hook remounts (e.g., layout key changes) but resets on full page reload -- acceptable since the Rust implementation resets on app restart too. + +**Supabase queries:** Direct Supabase client calls (not TanStack Query) since this is polling logic, not cached UI state. Three queries mirror the Rust logic: + +1. Active program activation -- `program_activations` table +2. Today's scheduled session -- join `blocks β†’ block_weeks β†’ scheduled_sessions β†’ session_templates` filtered by `day_of_week` matching JS `Date.getDay()` format +3. Workout logged today -- `workout_logs` filtered by `started_at` in today's UTC day range + +`advanceMinutes` is not consumed by the Rust implementation (marked TODO in `session_reminder.rs:104`). The JS implementation matches Rust behavior: fires once per day within a 06:00-20:59 window. `advanceMinutes` will be implemented in a future pass when a session-scheduled-time field is added. + +--- + +### D-3: Browser permission status reactivity + +**Problem:** `Notification.permission` is a string property that does not emit events. The settings UI needs to reflect permission state without polling. + +**Options considered:** + +- **Option A: Read once on component mount** -- Simple. Permission changes require a page reload to reflect. Acceptable since browsers do not allow granting from in-page UI after denial; the user must go to browser settings then reload. +- **Option B: `navigator.permissions.query({ name: 'notifications' })` + `onchange`** -- Returns a `PermissionStatus` object with an `onchange` handler. Fires when permission changes without a reload. Supported in Chrome, Firefox, Edge; Safari 16+ (partial -- may not fire `onchange` in all cases). +- **Option C: `visibilitychange` polling** -- Poll on tab focus. More compatible than Option B but reads are synchronous and cheap. + +**Chosen:** Option B with Option A fallback + +**Rationale:** `PermissionStatus.onchange` covers the primary browsers and is the correct API for this. A fallback to mount-time read handles Safari gaps. The settings component will use a `useEffect` that calls `navigator.permissions.query`, subscribes to `onchange`, and falls back gracefully if the API is unavailable. + +```typescript +// Pattern: +useEffect(() => { + if (!('permissions' in navigator)) return + navigator.permissions + .query({ name: 'notifications' as PermissionName }) + .then((status) => { + setPermission(status.state as NotificationPermission) + status.onchange = () => setPermission(status.state as NotificationPermission) + }) + .catch((err) => console.warn('[notification-settings] Permission query failed:', err)) +}, []) +``` + +--- + +### D-4: Permission request triggers + +**Problem:** Decide when to call `Notification.requestPermission()` for newly covered notification types. + +**Options considered:** + +- **Option A: Master toggle only (existing behavior)** -- Permission is requested when the master notifications toggle is turned on. Per-type toggles do not trigger re-requests. +- **Option B: Per-type toggle requests** -- Request permission individually when each type's toggle is enabled. + +**Chosen:** Option A (no change to existing behavior) + +**Rationale:** The master toggle is already the user's declaration of intent to receive notifications. Requesting on per-type toggles is surprising UX -- the user has already granted intent. The one addition: the rest timer and session reminder notification functions will call `Notification.requestPermission()` contextually if `Notification.permission === 'default'` at the moment of the first would-be notification, matching the existing `sendPrNotification` pattern. This covers the case where permission was never explicitly requested. + +## Stack-Specific Details + +### React / TypeScript + +**Files to modify:** + +| File | Change | +| -------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------- | +| `src/stores/active-workout-store.ts` | Add `onExpired?` param to `startRestTimer`; add `_onRestExpired` module-level var; call in `tickRest()`, clear in `skipRest()` + `cleanup()` | +| `src/lib/notification-service.ts` | Add `sendRestTimerNotification()` and `sendSessionReminderNotification()` functions | +| `src/components/profile/notification-settings.tsx` | Add `BrowserPermissionStatus` sub-component; update session reminder copy; reactive permission state | + +**Files to create:** + +| File | Purpose | +| ------------------------------------------- | ---------------------------------------------------- | +| `src/hooks/use-session-reminder-browser.ts` | 60-second polling hook for browser session reminders | + +**Files to mount the new hook:** + +The authenticated layout (or whichever top-level component wraps authenticated routes) gains a `` component or direct hook call. Identify the exact mount point during implementation (TanStack Router's authenticated layout file). + +**Patterns to follow:** + +- `.claude/rules/react-typescript.md` -- functional components, hooks, `useEffect` cleanup +- `.claude/rules/error-handling.md` -- bracketed module prefix, no bare catch, guard clauses +- `.claude/rules/state-management.md` -- module-scope setter validation +- Existing `sendPrNotification` pattern in `notification-service.ts` for new send functions + +**Dependencies:** No new packages. Uses `@tauri-apps/api/core` (already installed) for `isTauri()` guard; native `Notification` and `navigator.permissions` browser APIs. + +## Integration Points + +**Store β†’ Notification service:** The `onExpired` callback passed to `startRestTimer` closes over the exercise/set context and calls `sendRestTimerNotification`. The call site (the component that calls `startRestTimer`) is responsible for reading current prefs and providing them to the callback. + +**Hook β†’ Supabase:** `use-session-reminder-browser.ts` imports the project's Supabase client directly. No new adapter methods are needed -- raw `from().select()` calls mirror the Rust queries. + +**Hook β†’ Notification service:** The hook calls `sendSessionReminderNotification(sessionName, prefs)` after all conditions pass. + +**Settings UI β†’ Permission API:** `notification-settings.tsx` subscribes to `PermissionStatus.onchange` and reflects state in a read-only display sub-component. No writes to the Notifications API permission -- browsers control that. + +## Risks & Unknowns + +- **Risk:** JS `setInterval` for the rest timer backup is already in `tickRest()` and is 1-second granular. If the tab is heavily throttled (background throttling in modern browsers), the timer may drift and the notification may fire late. + - **Mitigation:** Acceptable for Phase 1; document as a known limitation. Phase 2 (service worker) resolves this. + +- **Risk:** Safari's partial `PermissionStatus.onchange` support means the permission status badge may not update reactively for Safari users. + - **Mitigation:** The fallback reads `Notification.permission` on mount. Safari users see correct state after page load; it may not update live. Acceptable. + +- **Risk:** Session reminder Supabase queries may fail if the user's auth session has expired mid-tab. + - **Mitigation:** Wrap in `try/catch` with `[session-reminder-browser]` prefix log. Silently skip that poll cycle on error -- same behavior as the Rust `log::warn` non-fatal path. + +- **Unknown:** Exact mount point for the session reminder hook in the authenticated layout. TanStack Router's file-based route structure places the authenticated layout in a specific route file. + - **Resolution:** Implementer reads the route tree during implementation; locate the `_authenticated` layout route. + +- **Unknown:** Whether the Supabase `workout_logs` table uses UTC timestamps or local time for the `started_at` column. The Rust code converts local date to a Unix epoch range using the local timezone. The JS implementation should match. + - **Resolution:** Check the Supabase migration or existing workout log adapter code during implementation. + +## Testing Strategy + +- **`active-workout-store`**: Unit test the `onExpired` callback: verify it fires on natural expiry and does NOT fire on `skipRest()`. Extend existing store tests in `__tests__/`. +- **`notification-service`**: Unit test `sendRestTimerNotification` and `sendSessionReminderNotification` with a mocked `Notification` global. Verify preference/quiet-hours gating. +- **`use-session-reminder-browser`**: Unit test the poll logic in isolation by injecting a mock Supabase client and mock `sendSessionReminderNotification`. Verify: fires once per day, skips on quiet hours, skips if already logged, skips outside time window. +- **`notification-settings`**: Component test for the `BrowserPermissionStatus` display in each permission state (`granted`, `denied`, `default`). Verify the "requires native app" string is absent in browser mode. + +Detailed test tasks are in Steps.md. From 8fa868eb43e21414d76c56370aabf64328db464d Mon Sep 17 00:00:00 2001 From: Robert Hamilton Date: Wed, 15 Apr 2026 11:07:14 -0500 Subject: [PATCH 2/7] feat(notifications): F022 browser rest timer and session reminder notifications Extends the browser notification path to cover rest timer alerts and session reminders, bringing browser parity with the Tauri native experience for in-tab usage. - active-workout-store: add _onRestExpired callback to startRestTimer; fires on natural expiry, silently cleared on skip/cleanup/finish/discard - notification-service: add sendRestTimerNotification and sendSessionReminderNotification with contextual permission requests (D-4) - use-active-workout: wire onExpired callback in confirmSet, reading prefs async and passing exercise/set context to the notification - use-session-reminder-browser: new 60s polling hook with Supabase queries for active program, today's session, and logged workouts; once-per-day guard - _authenticated: mount useSessionReminderBrowser in authenticated layout - notification-settings: add BrowserPermissionStatus (granted/default/denied), remove "requires native app" gate, add "tab must remain open" note Tasks: S001, S002, S003, S004, S005, S006 Feature: 022-Browser-Notifications --- .../notification-settings-browser.test.tsx | 151 ++++++++++ .../profile/notification-settings.tsx | 53 +++- .../use-active-workout-notification.test.ts | 267 +++++++++++++++++ .../use-session-reminder-browser.test.ts | 273 ++++++++++++++++++ src/hooks/use-active-workout.ts | 18 +- src/hooks/use-session-reminder-browser.ts | 140 +++++++++ .../__tests__/notification-service.test.ts | 230 +++++++++++++++ src/lib/notification-service.ts | 58 ++++ src/routes/_authenticated.tsx | 4 + .../active-workout-store-rest-expired.test.ts | 213 ++++++++++++++ src/stores/active-workout-store.ts | 25 +- 11 files changed, 1427 insertions(+), 5 deletions(-) create mode 100644 src/components/profile/__tests__/notification-settings-browser.test.tsx create mode 100644 src/hooks/__tests__/use-active-workout-notification.test.ts create mode 100644 src/hooks/__tests__/use-session-reminder-browser.test.ts create mode 100644 src/hooks/use-session-reminder-browser.ts create mode 100644 src/lib/__tests__/notification-service.test.ts create mode 100644 src/stores/__tests__/active-workout-store-rest-expired.test.ts diff --git a/src/components/profile/__tests__/notification-settings-browser.test.tsx b/src/components/profile/__tests__/notification-settings-browser.test.tsx new file mode 100644 index 0000000..f7b821b --- /dev/null +++ b/src/components/profile/__tests__/notification-settings-browser.test.tsx @@ -0,0 +1,151 @@ +// @vitest-environment happy-dom +import { screen } from '@testing-library/react' +import { renderWithProviders } from '@/test/render-helpers' +import { NotificationSettings } from '@/components/profile/notification-settings' +import type { NotificationPreferences } from '@/domain/types/notification' + +// --------------------------------------------------------------------------- +// Default prefs for tests +// --------------------------------------------------------------------------- + +const DEFAULT_PREFS: NotificationPreferences = { + enabled: true, + restTimer: { enabled: true, soundEnabled: true, vibrationEnabled: true }, + sessionReminders: { enabled: true, advanceMinutes: 30 }, + prCelebrations: { enabled: true }, + quietHours: { + enabled: false, + startHour: 22, + startMinute: 0, + endHour: 6, + endMinute: 0, + }, +} + +// --------------------------------------------------------------------------- +// Mocks +// --------------------------------------------------------------------------- + +const mockIsTauri = vi.fn(() => false) + +vi.mock('@tauri-apps/api/core', () => ({ + isTauri: () => mockIsTauri(), + invoke: vi.fn(), +})) + +const mockUpdatePreferences = { mutate: vi.fn(), isError: false } + +vi.mock('@/hooks/use-notification-preferences', () => ({ + useNotificationPreferences: vi.fn(() => ({ + data: { ...DEFAULT_PREFS }, + isLoading: false, + error: null, + updatePreferences: mockUpdatePreferences, + })), +})) + +// --------------------------------------------------------------------------- +// Browser API helpers +// --------------------------------------------------------------------------- + +/** Override Notification.permission (read-only in real browsers) */ +function setNotificationPermission(state: NotificationPermission) { + Object.defineProperty(window, 'Notification', { + value: { + permission: state, + requestPermission: vi.fn().mockResolvedValue(state), + }, + writable: true, + configurable: true, + }) +} + +/** Set up navigator.permissions.query to resolve with a given state */ +function mockPermissionsQuery(state: PermissionState) { + Object.defineProperty(navigator, 'permissions', { + value: { + query: vi.fn().mockResolvedValue({ + state, + onchange: null, + }), + }, + writable: true, + configurable: true, + }) +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('NotificationSettings -- browser mode', () => { + beforeEach(() => { + vi.clearAllMocks() + mockIsTauri.mockReturnValue(false) + }) + + it('BrowserPermissionStatus renders nothing when permission is granted', () => { + setNotificationPermission('granted') + mockPermissionsQuery('granted') + + renderWithProviders() + + expect(screen.queryByText(/blocked/i)).not.toBeInTheDocument() + expect(screen.queryByText(/when prompted/i)).not.toBeInTheDocument() + }) + + it('BrowserPermissionStatus renders guidance when permission is default', () => { + setNotificationPermission('default') + mockPermissionsQuery('prompt') + + renderWithProviders() + + expect( + screen.getByText('Enable notifications in your browser when prompted'), + ).toBeInTheDocument() + }) + + it('BrowserPermissionStatus renders blocked warning when permission is denied', () => { + setNotificationPermission('denied') + mockPermissionsQuery('denied') + + renderWithProviders() + + expect( + screen.getByText('Notifications are blocked. Enable them in your browser settings.'), + ).toBeInTheDocument() + + // No re-request button should exist + expect(screen.queryByRole('button', { name: /enable|allow|request/i })).not.toBeInTheDocument() + }) + + it('"requires native app" string is absent in browser mode', () => { + setNotificationPermission('granted') + mockPermissionsQuery('granted') + + renderWithProviders() + + expect(screen.queryByText(/require the native app/i)).not.toBeInTheDocument() + }) + + it('tab must remain open note is present when session reminders enabled', () => { + setNotificationPermission('granted') + mockPermissionsQuery('granted') + + renderWithProviders() + + expect(screen.getByText(/while this tab is open/i)).toBeInTheDocument() + }) + + it('BrowserPermissionStatus does not render in Tauri mode', () => { + mockIsTauri.mockReturnValue(true) + setNotificationPermission('denied') + mockPermissionsQuery('denied') + + renderWithProviders() + + expect( + screen.queryByText('Notifications are blocked. Enable them in your browser settings.'), + ).not.toBeInTheDocument() + }) +}) diff --git a/src/components/profile/notification-settings.tsx b/src/components/profile/notification-settings.tsx index fb9ed52..79ef4d9 100644 --- a/src/components/profile/notification-settings.tsx +++ b/src/components/profile/notification-settings.tsx @@ -1,3 +1,4 @@ +import { useEffect, useState } from 'react' import { isTauri } from '@tauri-apps/api/core' import { useNotificationPreferences } from '@/hooks/use-notification-preferences' import { Switch } from '@/components/ui/switch' @@ -120,6 +121,8 @@ export function NotificationSettings() {

Failed to save. Please try again.

)} + {!isTauri() && 'Notification' in window && } + {/* Conditional subsections -- only shown when master toggle is on */} {prefs.enabled && (
@@ -178,9 +181,9 @@ export function NotificationSettings() {
)} - {isBrowser && ( + {isBrowser && prefs.sessionReminders.enabled && (

- Session reminders require the native app + Notifications are delivered while this tab is open

)} @@ -255,6 +258,52 @@ export function NotificationSettings() { ) } +// --------------------------------------------------------------------------- +// BrowserPermissionStatus -- shows browser notification permission state +// --------------------------------------------------------------------------- + +function BrowserPermissionStatus() { + const [permission, setPermission] = useState( + 'Notification' in window ? Notification.permission : 'default', + ) + + useEffect(() => { + if (!('permissions' in navigator)) return + + let status: PermissionStatus | null = null + navigator.permissions + .query({ name: 'notifications' as PermissionName }) + .then((s) => { + status = s + setPermission(s.state as NotificationPermission) + s.onchange = () => setPermission(s.state as NotificationPermission) + }) + .catch((err) => console.warn('[notification-settings] Permission query failed:', err)) + + return () => { + if (status) status.onchange = null + } + }, []) + + // granted: no extra UI needed (silent) + if (permission === 'granted') return null + + if (permission === 'denied') { + return ( +

+ Notifications are blocked. Enable them in your browser settings. +

+ ) + } + + // default + return ( +

+ Enable notifications in your browser when prompted +

+ ) +} + // --------------------------------------------------------------------------- // ToggleRow -- label + switch in a flex row, 48px+ touch target // --------------------------------------------------------------------------- diff --git a/src/hooks/__tests__/use-active-workout-notification.test.ts b/src/hooks/__tests__/use-active-workout-notification.test.ts new file mode 100644 index 0000000..8b3c74e --- /dev/null +++ b/src/hooks/__tests__/use-active-workout-notification.test.ts @@ -0,0 +1,267 @@ +// @vitest-environment happy-dom +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { renderHook, act } from '@testing-library/react' +import { TestWrapper } from '@/test/render-helpers' +import { buildWorkoutLog, buildExercise, resetFactoryCounters } from '@/test/factories' +import { createMockAdapter } from '@/test/mocks/data-adapter' +import { useActiveWorkoutStore } from '@/stores/active-workout-store' +import { DEFAULT_NOTIFICATION_PREFERENCES } from '@/domain/types/notification' +import type { DataAdapter } from '@/lib/data-adapter' +import type { LoggedActivityGroup, LoggedActivity, LoggedSet } from '@/domain/types' + +// --------------------------------------------------------------------------- +// Mocks +// --------------------------------------------------------------------------- + +let mockAdapter: DataAdapter + +vi.mock('@/lib/adapter', () => ({ + getAdapter: () => mockAdapter, +})) + +vi.mock('@tauri-apps/api/core', () => ({ + isTauri: () => false, + invoke: vi.fn(), +})) + +vi.mock('@tauri-apps/api/event', () => ({ + listen: vi.fn(), +})) + +const mockGetNotificationPreferences = vi.fn() +const mockSendRestTimerNotification = vi.fn() + +vi.mock('@/lib/notification-service', () => ({ + getNotificationPreferences: (...args: unknown[]) => mockGetNotificationPreferences(...args), + sendRestTimerNotification: (...args: unknown[]) => mockSendRestTimerNotification(...args), +})) + +import { useActiveWorkout } from '../use-active-workout' + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +const DEFAULT_PREFS = { ...DEFAULT_NOTIFICATION_PREFERENCES } + +/** + * Intercept startRestTimer calls by wrapping the store's implementation. + * Must be called BEFORE renderHook so the hook picks up the wrapped function. + */ +function interceptStartRestTimer() { + const calls: unknown[][] = [] + const original = useActiveWorkoutStore.getState().startRestTimer + + useActiveWorkoutStore.setState({ + startRestTimer: (...args: Parameters) => { + calls.push(args) + // Don't call original -- it uses setInterval which complicates cleanup. + // We only need to verify the arguments passed. + }, + }) + + return { calls, restore: () => useActiveWorkoutStore.setState({ startRestTimer: original }) } +} + +/** Set up a started workout with one exercise so confirmSet has a valid target. */ +async function setupWorkoutWithExercise(result: { current: ReturnType }) { + const createdLog = buildWorkoutLog({ id: 'wl-1', userId: 'user-1' }) + vi.mocked(mockAdapter.createWorkoutLog).mockResolvedValue(createdLog) + + const savedGroup: LoggedActivityGroup = { + id: 'lag-1', + workoutLogId: 'wl-1', + groupType: 'STRAIGHT_SETS', + ordinal: 1, + } + const savedActivity: LoggedActivity = { + id: 'la-1', + loggedGroupId: 'lag-1', + exerciseId: 'ex-squat', + ordinal: 1, + } + vi.mocked(mockAdapter.createLoggedActivityGroup).mockResolvedValue(savedGroup) + vi.mocked(mockAdapter.createLoggedActivity).mockResolvedValue(savedActivity) + + await act(async () => { + await result.current.startWorkout('user-1') + }) + + const exercise = buildExercise({ id: 'ex-squat', name: 'Squat' }) + await act(async () => { + await result.current.addExercise(exercise, 'STRAIGHT_SETS') + }) + + return { createdLog, savedActivity } +} + +/** Build a minimal set data object for confirmSet. */ +function buildSetData(overrides?: Partial>): Omit { + return { + loggedActivityId: 'la-1', + setNumber: 1, + setType: 'WORKING' as const, + completed: false, + actualReps: 5, + actualWeight: { value: 225, unit: 'lb' as const }, + ...overrides, + } +} + +/** Mock createLoggedSet to return a saved set. */ +function mockCreateLoggedSet(overrides?: Partial) { + const savedSet: LoggedSet = { + id: 'ls-1', + loggedActivityId: 'la-1', + setNumber: 1, + setType: 'WORKING', + completed: true, + actualReps: 5, + actualWeight: { value: 225, unit: 'lb' as const }, + ...overrides, + } + vi.mocked(mockAdapter.createLoggedSet).mockResolvedValue(savedSet) + return savedSet +} + +// --------------------------------------------------------------------------- +// Setup / Teardown +// --------------------------------------------------------------------------- + +beforeEach(() => { + resetFactoryCounters() + mockAdapter = createMockAdapter() + useActiveWorkoutStore.getState().discardWorkout() + + mockGetNotificationPreferences.mockResolvedValue(DEFAULT_PREFS) + mockSendRestTimerNotification.mockResolvedValue(undefined) +}) + +afterEach(() => { + useActiveWorkoutStore.getState().cleanup() + vi.restoreAllMocks() +}) + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('confirmSet notification wiring', () => { + it('passes onExpired callback to startRestTimer when restSeconds > 0', async () => { + const spy = interceptStartRestTimer() + + const { result } = renderHook(() => useActiveWorkout(), { wrapper: TestWrapper }) + await setupWorkoutWithExercise(result) + mockCreateLoggedSet() + + await act(async () => { + await result.current.confirmSet('la-1', buildSetData(), 90, 'Squat') + }) + + // Wait for the async getNotificationPreferences().then() to fire + await act(async () => { + await vi.waitFor(() => { + expect(spy.calls.length).toBeGreaterThan(0) + }) + }) + + const [seconds, exerciseName, setNumber, onExpired] = spy.calls[0] + expect(seconds).toBe(90) + expect(exerciseName).toBe('Squat') + expect(setNumber).toBe(1) + expect(typeof onExpired).toBe('function') + + // Call the captured onExpired callback + await act(async () => { + await (onExpired as () => void)() + }) + + expect(mockSendRestTimerNotification).toHaveBeenCalledWith('Squat', 1, DEFAULT_PREFS) + + spy.restore() + }) + + it('does not start rest timer when restSeconds is 0', async () => { + const spy = interceptStartRestTimer() + + const { result } = renderHook(() => useActiveWorkout(), { wrapper: TestWrapper }) + await setupWorkoutWithExercise(result) + mockCreateLoggedSet() + + await act(async () => { + await result.current.confirmSet('la-1', buildSetData(), 0) + }) + + // Give async code a chance to run + await act(async () => { + await new Promise((r) => setTimeout(r, 50)) + }) + + expect(spy.calls.length).toBe(0) + + spy.restore() + }) + + it('passes correct exerciseName and setNumber to sendRestTimerNotification', async () => { + const spy = interceptStartRestTimer() + + const { result } = renderHook(() => useActiveWorkout(), { wrapper: TestWrapper }) + await setupWorkoutWithExercise(result) + mockCreateLoggedSet({ setNumber: 3, actualReps: 8, actualWeight: { value: 185, unit: 'lb' } }) + + await act(async () => { + await result.current.confirmSet('la-1', buildSetData({ setNumber: 3 }), 60, 'Bench Press') + }) + + await act(async () => { + await vi.waitFor(() => { + expect(spy.calls.length).toBeGreaterThan(0) + }) + }) + + const onExpired = spy.calls[0][3] as () => void + expect(typeof onExpired).toBe('function') + + await act(async () => { + await onExpired() + }) + + expect(mockSendRestTimerNotification).toHaveBeenCalledWith('Bench Press', 3, DEFAULT_PREFS) + + spy.restore() + }) + + it('starts timer without onExpired callback when notification prefs fail to load', async () => { + mockGetNotificationPreferences.mockRejectedValue(new Error('Storage unavailable')) + + const spy = interceptStartRestTimer() + + const { result } = renderHook(() => useActiveWorkout(), { wrapper: TestWrapper }) + await setupWorkoutWithExercise(result) + mockCreateLoggedSet() + + // Suppress expected console.error from the catch block + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + await act(async () => { + await result.current.confirmSet('la-1', buildSetData(), 120, 'Deadlift') + }) + + // Wait for the rejected promise's catch handler to fire + await act(async () => { + await vi.waitFor(() => { + expect(spy.calls.length).toBeGreaterThan(0) + }) + }) + + // startRestTimer should still be called, but without the onExpired callback + const [seconds, exerciseName, setNumber, onExpired] = spy.calls[0] + expect(seconds).toBe(120) + expect(exerciseName).toBe('Deadlift') + expect(setNumber).toBe(1) + expect(onExpired).toBeUndefined() + + consoleErrorSpy.mockRestore() + spy.restore() + }) +}) diff --git a/src/hooks/__tests__/use-session-reminder-browser.test.ts b/src/hooks/__tests__/use-session-reminder-browser.test.ts new file mode 100644 index 0000000..66b8edf --- /dev/null +++ b/src/hooks/__tests__/use-session-reminder-browser.test.ts @@ -0,0 +1,273 @@ +// @vitest-environment happy-dom +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { renderHook } from '@testing-library/react' + +// --------------------------------------------------------------------------- +// Mocks -- must be declared before importing the hook +// --------------------------------------------------------------------------- + +const mockIsTauri = vi.fn(() => false) +vi.mock('@tauri-apps/api/core', () => ({ + isTauri: () => mockIsTauri(), + invoke: vi.fn(), +})) + +const mockGetSupabaseClient = vi.fn() +vi.mock('@/lib/supabase', () => ({ + getSupabaseClient: () => mockGetSupabaseClient(), +})) + +const mockGetNotificationPreferences = vi.fn() +const mockIsInQuietHours = vi.fn((_prefs: unknown) => false) +const mockSendSessionReminderNotification = vi.fn() +vi.mock('@/lib/notification-service', () => ({ + getNotificationPreferences: () => mockGetNotificationPreferences(), + isInQuietHours: (prefs: unknown) => mockIsInQuietHours(prefs), + sendSessionReminderNotification: (...args: unknown[]) => + mockSendSessionReminderNotification(...args), +})) + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeEnabledPrefs() { + return { + enabled: true, + restTimer: { enabled: true, soundEnabled: true, vibrationEnabled: true }, + sessionReminders: { enabled: true, advanceMinutes: 30 }, + prCelebrations: { enabled: true }, + quietHours: { enabled: false, startHour: 22, startMinute: 0, endHour: 6, endMinute: 0 }, + } +} + +/** Build a chainable mock Supabase query builder. */ +function makeQueryBuilder(resolvedData: unknown, resolvedError: unknown = null) { + const builder: Record = {} + const terminalResult = { data: resolvedData, error: resolvedError } + + // Every chainable method returns the builder itself + for (const method of ['select', 'eq', 'gte', 'lt', 'limit']) { + builder[method] = vi.fn(() => builder) + } + // Terminal methods resolve the result + builder.maybeSingle = vi.fn(() => Promise.resolve(terminalResult)) + // When no .maybeSingle() is called (e.g. scheduled_sessions, workout_logs), + // the builder itself acts as a thenable so `await supabase.from(...)...` resolves + builder.then = (resolve: (v: unknown) => void) => resolve(terminalResult) + + return builder +} + +/** + * Create a mock Supabase client where `.from(table)` returns a pre-configured + * query builder per table. + */ +function makeMockSupabase(config: { + activation?: object | null + block?: object | null + blockWeek?: object | null + sessions?: object[] | null + todayLogs?: object[] | null +}) { + const builders: Record> = { + program_activations: makeQueryBuilder(config.activation ?? null), + blocks: makeQueryBuilder(config.block ?? null), + block_weeks: makeQueryBuilder(config.blockWeek ?? null), + scheduled_sessions: makeQueryBuilder(config.sessions ?? null), + workout_logs: makeQueryBuilder(config.todayLogs ?? null), + } + + return { + from: vi.fn((table: string) => builders[table] ?? makeQueryBuilder(null)), + } +} + +function defaultSupabaseConfig() { + return { + activation: { + id: 'act-1', + program_id: 'prog-1', + current_block_ordinal: 1, + current_week_number: 1, + }, + block: { id: 'block-1' }, + blockWeek: { id: 'bw-1' }, + sessions: [ + { + id: 'ss-1', + session_template_id: 'st-1', + session_templates: { name: 'Upper Body' }, + }, + ], + todayLogs: [] as { id: string }[], + } +} + +// --------------------------------------------------------------------------- +// Module-level _lastRemindedDate persists across tests. We use +// vi.resetModules() + dynamic import to get a fresh module per test. +// --------------------------------------------------------------------------- + +let useSessionReminderBrowser: () => void + +async function freshImport() { + vi.resetModules() + const mod = await import('../use-session-reminder-browser') + useSessionReminderBrowser = mod.useSessionReminderBrowser +} + +// --------------------------------------------------------------------------- +// Suite +// --------------------------------------------------------------------------- + +describe('useSessionReminderBrowser', () => { + beforeEach(async () => { + vi.useFakeTimers() + // Pin time to 10:00 on a Wednesday (day_of_week = 3) + vi.setSystemTime(new Date(2026, 3, 15, 10, 0, 0)) // 2026-04-15 10:00 + + mockIsTauri.mockReturnValue(false) + mockIsInQuietHours.mockReturnValue(false) + mockGetNotificationPreferences.mockResolvedValue(makeEnabledPrefs()) + mockSendSessionReminderNotification.mockResolvedValue(undefined) + + await freshImport() + }) + + afterEach(() => { + vi.useRealTimers() + vi.clearAllMocks() + }) + + // ------------------------------------------------------------------------- + // 1. fires notification when all conditions pass + // ------------------------------------------------------------------------- + it('fires notification when all conditions pass', async () => { + const supabase = makeMockSupabase(defaultSupabaseConfig()) + mockGetSupabaseClient.mockReturnValue(supabase) + + renderHook(() => useSessionReminderBrowser()) + + // The hook calls checkAndMaybeNotify() immediately (Promise-based). + // Flush the microtask queue so all awaits resolve. + await vi.advanceTimersByTimeAsync(0) + + expect(mockSendSessionReminderNotification).toHaveBeenCalledTimes(1) + expect(mockSendSessionReminderNotification).toHaveBeenCalledWith( + 'Upper Body', + expect.objectContaining({ enabled: true }), + ) + }) + + // ------------------------------------------------------------------------- + // 2. skips when master toggle off + // ------------------------------------------------------------------------- + it('skips when master toggle off', async () => { + const prefs = makeEnabledPrefs() + prefs.enabled = false + mockGetNotificationPreferences.mockResolvedValue(prefs) + mockGetSupabaseClient.mockReturnValue(makeMockSupabase(defaultSupabaseConfig())) + + renderHook(() => useSessionReminderBrowser()) + await vi.advanceTimersByTimeAsync(0) + + expect(mockSendSessionReminderNotification).not.toHaveBeenCalled() + }) + + // ------------------------------------------------------------------------- + // 3. skips when quiet hours active + // ------------------------------------------------------------------------- + it('skips when quiet hours active', async () => { + mockIsInQuietHours.mockReturnValue(true) + mockGetSupabaseClient.mockReturnValue(makeMockSupabase(defaultSupabaseConfig())) + + renderHook(() => useSessionReminderBrowser()) + await vi.advanceTimersByTimeAsync(0) + + expect(mockSendSessionReminderNotification).not.toHaveBeenCalled() + }) + + // ------------------------------------------------------------------------- + // 4. skips when outside 06-20 time window + // ------------------------------------------------------------------------- + it('skips when outside 06-20 time window (before 6)', async () => { + vi.setSystemTime(new Date(2026, 3, 15, 4, 0, 0)) // 04:00 + mockGetSupabaseClient.mockReturnValue(makeMockSupabase(defaultSupabaseConfig())) + + renderHook(() => useSessionReminderBrowser()) + await vi.advanceTimersByTimeAsync(0) + + expect(mockSendSessionReminderNotification).not.toHaveBeenCalled() + }) + + it('skips when outside 06-20 time window (after 20)', async () => { + vi.setSystemTime(new Date(2026, 3, 15, 21, 0, 0)) // 21:00 + mockGetSupabaseClient.mockReturnValue(makeMockSupabase(defaultSupabaseConfig())) + + renderHook(() => useSessionReminderBrowser()) + await vi.advanceTimersByTimeAsync(0) + + expect(mockSendSessionReminderNotification).not.toHaveBeenCalled() + }) + + // ------------------------------------------------------------------------- + // 5. skips when workout already logged today + // ------------------------------------------------------------------------- + it('skips when workout already logged today', async () => { + const config = defaultSupabaseConfig() + config.todayLogs = [{ id: 'log-1' }] + mockGetSupabaseClient.mockReturnValue(makeMockSupabase(config)) + + renderHook(() => useSessionReminderBrowser()) + await vi.advanceTimersByTimeAsync(0) + + expect(mockSendSessionReminderNotification).not.toHaveBeenCalled() + }) + + // ------------------------------------------------------------------------- + // 6. fires at most once per day + // ------------------------------------------------------------------------- + it('fires at most once per day', async () => { + const supabase = makeMockSupabase(defaultSupabaseConfig()) + mockGetSupabaseClient.mockReturnValue(supabase) + + renderHook(() => useSessionReminderBrowser()) + + // First immediate call fires + await vi.advanceTimersByTimeAsync(0) + expect(mockSendSessionReminderNotification).toHaveBeenCalledTimes(1) + + // Advance past one interval (60s) to trigger the second poll + await vi.advanceTimersByTimeAsync(60_000) + expect(mockSendSessionReminderNotification).toHaveBeenCalledTimes(1) + }) + + // ------------------------------------------------------------------------- + // 7. no-op in Tauri mode + // ------------------------------------------------------------------------- + it('no-op in Tauri mode', async () => { + mockIsTauri.mockReturnValue(true) + mockGetSupabaseClient.mockReturnValue(makeMockSupabase(defaultSupabaseConfig())) + + renderHook(() => useSessionReminderBrowser()) + await vi.advanceTimersByTimeAsync(0) + + expect(mockGetSupabaseClient).not.toHaveBeenCalled() + expect(mockSendSessionReminderNotification).not.toHaveBeenCalled() + }) + + // ------------------------------------------------------------------------- + // 8. skips when no active program + // ------------------------------------------------------------------------- + it('skips when no active program', async () => { + const config = defaultSupabaseConfig() + config.activation = null as unknown as typeof config.activation + mockGetSupabaseClient.mockReturnValue(makeMockSupabase(config)) + + renderHook(() => useSessionReminderBrowser()) + await vi.advanceTimersByTimeAsync(0) + + expect(mockSendSessionReminderNotification).not.toHaveBeenCalled() + }) +}) diff --git a/src/hooks/use-active-workout.ts b/src/hooks/use-active-workout.ts index d9ca959..9c59f13 100644 --- a/src/hooks/use-active-workout.ts +++ b/src/hooks/use-active-workout.ts @@ -18,6 +18,7 @@ import { useDeleteLoggedActivity, } from '@/hooks/use-workout-logs' import { getAdapter } from '@/lib/adapter' +import { getNotificationPreferences, sendRestTimerNotification } from '@/lib/notification-service' import { DEFAULT_REST_SECONDS } from '@/lib/workout-utils' import { computeNextProgramPosition } from '@/lib/program-advancement' import { resolveSessionTemplate } from '@/lib/prescription-resolver' @@ -368,6 +369,7 @@ export function useActiveWorkout() { loggedActivityId: string, setData: Omit, restSeconds: number = DEFAULT_REST_SECONDS, + exerciseName?: string, ) => { try { if (!workoutLog) { @@ -402,7 +404,21 @@ export function useActiveWorkout() { // Skip the global rest timer for zero/negative values. CircuitPanel // and similar self-managed timer UIs pass 0 to suppress the duplicate. if (restSeconds > 0) { - storeStartRestTimer(restSeconds) + const setNum = setData.setNumber + + // Read notification prefs (async), then start the rest timer with + // an onExpired callback that fires a browser notification. + getNotificationPreferences() + .then((prefs) => { + storeStartRestTimer(restSeconds, exerciseName, setNum, () => { + sendRestTimerNotification(exerciseName, setNum, prefs) + }) + }) + .catch((err) => { + // If prefs fail to load, still start the timer without notification + console.error('[workout] Failed to load notification preferences:', err) + storeStartRestTimer(restSeconds, exerciseName, setNum) + }) } return savedSet diff --git a/src/hooks/use-session-reminder-browser.ts b/src/hooks/use-session-reminder-browser.ts new file mode 100644 index 0000000..0d7ef55 --- /dev/null +++ b/src/hooks/use-session-reminder-browser.ts @@ -0,0 +1,140 @@ +import { useEffect } from 'react' +import { isTauri } from '@tauri-apps/api/core' +import { getSupabaseClient } from '@/lib/supabase' +import { + getNotificationPreferences, + isInQuietHours, + sendSessionReminderNotification, +} from '@/lib/notification-service' + +// Module-level: survives hook remounts, resets on full page reload +let _lastRemindedDate: string | null = null + +export function useSessionReminderBrowser(): void { + useEffect(() => { + if (isTauri()) return + + async function checkAndMaybeNotify(): Promise { + try { + const prefs = await getNotificationPreferences() + + // Guard: master toggle and session reminders toggle + if (!prefs.enabled || !prefs.sessionReminders.enabled) return + + // Guard: quiet hours + if (isInQuietHours(prefs)) return + + // Guard: only fire between 06:00 and 20:59 local time + const currentHour = new Date().getHours() + if (currentHour < 6 || currentHour > 20) return + + // Guard: already reminded today + const today = new Date().toISOString().slice(0, 10) // YYYY-MM-DD + if (_lastRemindedDate === today) return + + const supabase = getSupabaseClient() + if (!supabase) { + console.error('[session-reminder-browser] Supabase client not initialized') + return + } + + // 1. Get active program activation + const { data: activation, error: activationErr } = await supabase + .from('program_activations') + .select('id, program_id, current_block_ordinal, current_week_number') + .maybeSingle() + + if (activationErr) { + console.error( + '[session-reminder-browser] Failed to query program_activations:', + activationErr, + ) + return + } + if (!activation) return // No active program + + // 2. Find today's scheduled session for the current block+week position + const { data: block, error: blockErr } = await supabase + .from('blocks') + .select('id') + .eq('program_id', activation.program_id) + .eq('ordinal', activation.current_block_ordinal) + .maybeSingle() + + if (blockErr) { + console.error('[session-reminder-browser] Failed to query blocks:', blockErr) + return + } + if (!block) return + + // Get the block_week for current_week_number + const { data: blockWeek, error: weekErr } = await supabase + .from('block_weeks') + .select('id') + .eq('block_id', block.id) + .eq('week_number', activation.current_week_number) + .maybeSingle() + + if (weekErr) { + console.error('[session-reminder-browser] Failed to query block_weeks:', weekErr) + return + } + if (!blockWeek) return + + // Get scheduled sessions for today's day_of_week + const todayDow = new Date().getDay() // 0=Sunday, matches schema + const { data: sessions, error: sessErr } = await supabase + .from('scheduled_sessions') + .select('id, session_template_id, session_templates(name)') + .eq('block_week_id', blockWeek.id) + .eq('day_of_week', todayDow) + + if (sessErr) { + console.error('[session-reminder-browser] Failed to query scheduled_sessions:', sessErr) + return + } + if (!sessions || sessions.length === 0) return + + // 3. Check if a workout has been logged today + const todayStart = new Date() + todayStart.setHours(0, 0, 0, 0) + const tomorrowStart = new Date(todayStart) + tomorrowStart.setDate(tomorrowStart.getDate() + 1) + + const { data: todayLogs, error: logErr } = await supabase + .from('workout_logs') + .select('id') + .gte('started_at', todayStart.toISOString()) + .lt('started_at', tomorrowStart.toISOString()) + .limit(1) + + if (logErr) { + console.error('[session-reminder-browser] Failed to query workout_logs:', logErr) + return + } + + if (todayLogs && todayLogs.length > 0) { + // Workout already logged -- suppress further checks today + _lastRemindedDate = today + return + } + + // 4. Fire notification for the first scheduled session + const session = sessions[0] + const sessionName = + (session.session_templates as unknown as { name: string })?.name ?? 'Workout' + + await sendSessionReminderNotification(sessionName, prefs) + _lastRemindedDate = today + } catch (err) { + console.error('[session-reminder-browser] Unexpected error during poll:', err) + } + } + + // Run immediately on mount, then poll every 60s + checkAndMaybeNotify() + const interval = setInterval(checkAndMaybeNotify, 60_000) + + return () => clearInterval(interval) + }, []) +} diff --git a/src/lib/__tests__/notification-service.test.ts b/src/lib/__tests__/notification-service.test.ts new file mode 100644 index 0000000..9e7497b --- /dev/null +++ b/src/lib/__tests__/notification-service.test.ts @@ -0,0 +1,230 @@ +// @vitest-environment happy-dom +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' + +import type { NotificationPreferences } from '@/domain/types/notification' + +// --------------------------------------------------------------------------- +// Mocks +// --------------------------------------------------------------------------- + +vi.mock('@tauri-apps/api/core', () => ({ + isTauri: () => false, + invoke: vi.fn(), +})) + +// Mock window.Notification before importing the module under test +const MockNotification = vi.fn() as ReturnType & { + permission: NotificationPermission + requestPermission: ReturnType +} +const permissionGetter = vi.fn(() => 'granted') +Object.defineProperty(MockNotification, 'permission', { + get: permissionGetter, + configurable: true, +}) +MockNotification.requestPermission = vi.fn().mockResolvedValue('granted') + +Object.defineProperty(window, 'Notification', { + value: MockNotification, + writable: true, + configurable: true, +}) + +// Import after mocks are established +import { sendRestTimerNotification, sendSessionReminderNotification } from '../notification-service' + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makePrefs(overrides: Partial = {}): NotificationPreferences { + return { + enabled: true, + restTimer: { enabled: true, soundEnabled: true, vibrationEnabled: true }, + sessionReminders: { enabled: true, advanceMinutes: 30 }, + prCelebrations: { enabled: true }, + quietHours: { enabled: false, startHour: 22, startMinute: 0, endHour: 6, endMinute: 0 }, + ...overrides, + } +} + +// --------------------------------------------------------------------------- +// Setup / Teardown +// --------------------------------------------------------------------------- + +let savedNotification: typeof window.Notification | undefined + +beforeEach(() => { + vi.clearAllMocks() + permissionGetter.mockReturnValue('granted') + MockNotification.requestPermission = vi.fn().mockResolvedValue('granted') + savedNotification = window.Notification +}) + +afterEach(() => { + // Restore Notification if a test removed it + if (!('Notification' in window) && savedNotification) { + Object.defineProperty(window, 'Notification', { + value: savedNotification, + writable: true, + configurable: true, + }) + } +}) + +// --------------------------------------------------------------------------- +// sendRestTimerNotification +// --------------------------------------------------------------------------- + +describe('sendRestTimerNotification', () => { + it('fires notification when permission granted and toggles on', async () => { + const prefs = makePrefs() + + await sendRestTimerNotification('Bench Press', 3, prefs) + + expect(MockNotification).toHaveBeenCalledOnce() + const [title, options] = MockNotification.mock.calls[0] + expect(title).toBe('REST COMPLETE') + expect(options.body).toContain('Bench Press') + expect(options.body).toContain('Set 3') + }) + + it('no-op when master toggle off', async () => { + const prefs = makePrefs({ enabled: false }) + + await sendRestTimerNotification('Bench Press', 3, prefs) + + expect(MockNotification).not.toHaveBeenCalled() + }) + + it('no-op when rest timer toggle off', async () => { + const prefs = makePrefs({ + restTimer: { enabled: false, soundEnabled: true, vibrationEnabled: true }, + }) + + await sendRestTimerNotification('Bench Press', 3, prefs) + + expect(MockNotification).not.toHaveBeenCalled() + }) + + it('no-op when permission denied', async () => { + permissionGetter.mockReturnValue('denied') + const prefs = makePrefs() + + await sendRestTimerNotification('Bench Press', 3, prefs) + + expect(MockNotification).not.toHaveBeenCalled() + expect(MockNotification.requestPermission).not.toHaveBeenCalled() + }) + + it('calls requestPermission when permission is default', async () => { + permissionGetter.mockReturnValue('default') + MockNotification.requestPermission = vi.fn().mockResolvedValue('granted') + const prefs = makePrefs() + + await sendRestTimerNotification('Bench Press', 3, prefs) + + expect(MockNotification.requestPermission).toHaveBeenCalledOnce() + expect(MockNotification).toHaveBeenCalledOnce() + }) + + it('does not fire when requestPermission returns denied', async () => { + permissionGetter.mockReturnValue('default') + MockNotification.requestPermission = vi.fn().mockResolvedValue('denied') + const prefs = makePrefs() + + await sendRestTimerNotification('Bench Press', 3, prefs) + + expect(MockNotification.requestPermission).toHaveBeenCalledOnce() + expect(MockNotification).not.toHaveBeenCalled() + }) + + it('rest timer notification is NOT blocked by quiet hours', async () => { + const prefs = makePrefs({ + quietHours: { enabled: true, startHour: 0, startMinute: 0, endHour: 23, endMinute: 59 }, + }) + + await sendRestTimerNotification('Squat', 1, prefs) + + expect(MockNotification).toHaveBeenCalledOnce() + }) + + it('body shows fallback when exerciseName is undefined', async () => { + const prefs = makePrefs() + + await sendRestTimerNotification(undefined, undefined, prefs) + + expect(MockNotification).toHaveBeenCalledOnce() + const [, options] = MockNotification.mock.calls[0] + expect(options.body).toBe('Time to start your next set') + }) +}) + +// --------------------------------------------------------------------------- +// sendSessionReminderNotification +// --------------------------------------------------------------------------- + +describe('sendSessionReminderNotification', () => { + it('fires notification when permission granted and toggles on', async () => { + const prefs = makePrefs() + + await sendSessionReminderNotification('Upper Body', prefs) + + expect(MockNotification).toHaveBeenCalledOnce() + const [title, options] = MockNotification.mock.calls[0] + expect(title).toBe('SESSION REMINDER') + expect(options.body).toContain('Upper Body') + }) + + it('no-op when master toggle off', async () => { + const prefs = makePrefs({ enabled: false }) + + await sendSessionReminderNotification('Upper Body', prefs) + + expect(MockNotification).not.toHaveBeenCalled() + }) + + it('no-op when session reminder toggle off', async () => { + const prefs = makePrefs({ + sessionReminders: { enabled: false, advanceMinutes: 30 }, + }) + + await sendSessionReminderNotification('Upper Body', prefs) + + expect(MockNotification).not.toHaveBeenCalled() + }) + + it('session reminder IS blocked by quiet hours', async () => { + const prefs = makePrefs({ + quietHours: { enabled: true, startHour: 0, startMinute: 0, endHour: 23, endMinute: 59 }, + }) + + await sendSessionReminderNotification('Upper Body', prefs) + + expect(MockNotification).not.toHaveBeenCalled() + }) + + it('calls requestPermission when permission is default', async () => { + permissionGetter.mockReturnValue('default') + MockNotification.requestPermission = vi.fn().mockResolvedValue('granted') + const prefs = makePrefs() + + await sendSessionReminderNotification('Upper Body', prefs) + + expect(MockNotification.requestPermission).toHaveBeenCalledOnce() + expect(MockNotification).toHaveBeenCalledOnce() + }) + + it('no-op when Notification API unavailable', async () => { + // Remove Notification from window + // @ts-expect-error -- intentionally removing for test + delete window.Notification + + const prefs = makePrefs() + + await sendSessionReminderNotification('Upper Body', prefs) + + // No error thrown, no notification + expect(MockNotification).not.toHaveBeenCalled() + }) +}) diff --git a/src/lib/notification-service.ts b/src/lib/notification-service.ts index 0b554c2..349f849 100644 --- a/src/lib/notification-service.ts +++ b/src/lib/notification-service.ts @@ -134,3 +134,61 @@ export async function sendPrNotification( new Notification(title, { body }) } } + +// --------------------------------------------------------------------------- +// sendRestTimerNotification -- fires a platform notification when rest ends +// +// Uses contextual permission request (D-4): if permission is 'default', +// prompts the user at this natural interaction point rather than up front. +// Rest timer notifications are exempt from quiet hours. +// --------------------------------------------------------------------------- + +export async function sendRestTimerNotification( + exerciseName: string | undefined, + setNumber: number | undefined, + prefs: NotificationPreferences, +): Promise { + if (!shouldSendNotification('restTimer', prefs)) return + if (!('Notification' in window)) return + if (Notification.permission === 'denied') return + + // Contextual permission request when default (D-4 from Tech.md) + if (Notification.permission === 'default') { + const result = await Notification.requestPermission() + if (result !== 'granted') return + } + + const title = 'REST COMPLETE' + const parts: string[] = [] + if (exerciseName) parts.push(exerciseName) + if (exerciseName && setNumber != null) parts.push(`Set ${setNumber}`) + const body = parts.length > 0 ? parts.join(' \u2014 ') : 'Time to start your next set' + + new Notification(title, { body }) +} + +// --------------------------------------------------------------------------- +// sendSessionReminderNotification -- fires a reminder for a scheduled session +// +// Uses contextual permission request (D-4). Session reminders respect quiet +// hours (unlike rest timer which is exempt). +// --------------------------------------------------------------------------- + +export async function sendSessionReminderNotification( + sessionName: string, + prefs: NotificationPreferences, +): Promise { + if (!shouldSendNotification('sessionReminders', prefs)) return + if (!('Notification' in window)) return + if (Notification.permission === 'denied') return + + if (Notification.permission === 'default') { + const result = await Notification.requestPermission() + if (result !== 'granted') return + } + + const title = 'SESSION REMINDER' + const body = `${sessionName} is scheduled for today` + + new Notification(title, { body }) +} diff --git a/src/routes/_authenticated.tsx b/src/routes/_authenticated.tsx index eddfeb6..bf26216 100644 --- a/src/routes/_authenticated.tsx +++ b/src/routes/_authenticated.tsx @@ -7,6 +7,7 @@ import { } from '@tanstack/react-router' import { useEffect } from 'react' import { isTauri, invoke } from '@tauri-apps/api/core' +import { useSessionReminderBrowser } from '@/hooks/use-session-reminder-browser' import { useAuth } from '@/lib/auth' import { cn } from '@/lib/utils' import { SyncIndicator } from '@/components/layout/sync-indicator' @@ -37,6 +38,9 @@ function AuthenticatedLayout() { } }, [loading, user, isGuest, navigate]) + // Start the browser session reminder scheduler (no-ops in Tauri mode). + useSessionReminderBrowser() + // Start the session reminder background scheduler in Tauri mode. // The Rust scheduler polls every 60s and respects notification preferences. useEffect(() => { diff --git a/src/stores/__tests__/active-workout-store-rest-expired.test.ts b/src/stores/__tests__/active-workout-store-rest-expired.test.ts new file mode 100644 index 0000000..9a8583b --- /dev/null +++ b/src/stores/__tests__/active-workout-store-rest-expired.test.ts @@ -0,0 +1,213 @@ +import { useActiveWorkoutStore } from '@/stores/active-workout-store' +import type { WorkoutLog, LoggedActivityGroup, LoggedActivity, GroupType } from '@/domain/types' +import type { LoggedActivityGroupWithActivities } from '@/stores/active-workout-store' + +// --------------------------------------------------------------------------- +// Fixtures -- minimal objects satisfying required fields +// --------------------------------------------------------------------------- + +const NOW = '2026-03-27T10:00:00Z' + +function makeWorkoutLog(overrides?: Partial): WorkoutLog { + return { + id: 'wl-1', + createdAt: NOW, + updatedAt: NOW, + userId: 'user-1', + startedAt: NOW, + ...overrides, + } as WorkoutLog +} + +function makeLoggedActivityGroup(overrides?: Partial): LoggedActivityGroup { + return { + id: 'lag-1', + workoutLogId: 'wl-1', + groupType: 'STRAIGHT_SETS' as GroupType, + ordinal: 1, + ...overrides, + } as LoggedActivityGroup +} + +function makeLoggedActivity(overrides?: Partial): LoggedActivity { + return { + id: 'la-1', + loggedGroupId: 'lag-1', + exerciseId: 'ex-1', + ordinal: 1, + ...overrides, + } as LoggedActivity +} + +function makeGroupWithActivities( + groupOverrides?: Partial, + activityOverrides?: Partial, +): LoggedActivityGroupWithActivities { + return { + ...makeLoggedActivityGroup(groupOverrides), + activities: [ + { + ...makeLoggedActivity(activityOverrides), + sets: [], + }, + ], + } +} + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function getState() { + return useActiveWorkoutStore.getState() +} + +// --------------------------------------------------------------------------- +// Setup / teardown +// --------------------------------------------------------------------------- + +beforeEach(() => { + vi.useFakeTimers() +}) + +afterEach(() => { + // Prevent interval leakage between tests + getState().cleanup() + // Reset state fully + useActiveWorkoutStore.setState({ + workoutLog: null, + loggedGroups: [], + elapsedSeconds: 0, + restTimer: null, + undoAction: null, + }) + vi.useRealTimers() +}) + +// =========================================================================== +// _onRestExpired callback mechanism +// =========================================================================== + +describe('_onRestExpired callback', () => { + it('onExpired callback fires exactly once when browser timer counts to zero', () => { + const onExpired = vi.fn() + + getState().startRestTimer(3, undefined, undefined, onExpired) + + // Tick 1: remaining 3 -> 2 + getState().tickRest() + expect(onExpired).not.toHaveBeenCalled() + expect(getState().restTimer).not.toBeNull() + expect(getState().restTimer!.remaining).toBe(2) + + // Tick 2: remaining 2 -> 1 + getState().tickRest() + expect(onExpired).not.toHaveBeenCalled() + expect(getState().restTimer).not.toBeNull() + expect(getState().restTimer!.remaining).toBe(1) + + // Tick 3: remaining 1 -> 0, callback fires and timer clears + getState().tickRest() + expect(onExpired).toHaveBeenCalledTimes(1) + expect(getState().restTimer).toBeNull() + + // Subsequent ticks should not fire the callback again + getState().tickRest() + expect(onExpired).toHaveBeenCalledTimes(1) + }) + + it('onExpired callback is NOT called on skipRest()', () => { + const onExpired = vi.fn() + + getState().startRestTimer(60, undefined, undefined, onExpired) + expect(getState().restTimer).not.toBeNull() + + getState().skipRest() + + expect(onExpired).not.toHaveBeenCalled() + expect(getState().restTimer).toBeNull() + }) + + it('onExpired callback is cleared when a new timer starts', () => { + const callbackA = vi.fn() + const callbackB = vi.fn() + + // Start timer with callback A + getState().startRestTimer(5, undefined, undefined, callbackA) + + // Start a new timer with callback B before A expires + getState().startRestTimer(2, undefined, undefined, callbackB) + + // Tick to zero (2 ticks for the second timer) + getState().tickRest() + getState().tickRest() + + // Only callback B should have been called + expect(callbackA).not.toHaveBeenCalled() + expect(callbackB).toHaveBeenCalledTimes(1) + expect(getState().restTimer).toBeNull() + }) + + it('cleanup() clears onExpired without calling it', () => { + const onExpired = vi.fn() + + getState().startRestTimer(30, undefined, undefined, onExpired) + expect(getState().restTimer).not.toBeNull() + + getState().cleanup() + + expect(onExpired).not.toHaveBeenCalled() + + // Ticking after cleanup should not fire the callback (restTimer was not + // cleared by cleanup itself -- it only clears intervals and the callback). + // However, the module-level _onRestExpired is now null, so even if we + // manually tick, the callback reference is gone. + }) + + it('onExpired is null when no callback provided -- ticking to zero does not throw', () => { + // Start rest timer without onExpired parameter + getState().startRestTimer(1) + + // Tick to zero -- the ?. operator should handle null gracefully + expect(() => getState().tickRest()).not.toThrow() + expect(getState().restTimer).toBeNull() + }) + + it('finishWorkout clears onExpired without calling it', () => { + const onExpired = vi.fn() + + // Start a workout so finishWorkout has something to clear + getState().startWorkout('user-1', makeWorkoutLog()) + useActiveWorkoutStore.setState({ + loggedGroups: [makeGroupWithActivities()], + }) + + getState().startRestTimer(60, 'Bench Press', 3, onExpired) + expect(getState().restTimer).not.toBeNull() + + getState().finishWorkout() + + expect(onExpired).not.toHaveBeenCalled() + expect(getState().restTimer).toBeNull() + expect(getState().workoutLog).toBeNull() + }) + + it('discardWorkout clears onExpired without calling it', () => { + const onExpired = vi.fn() + + // Start a workout so discardWorkout has something to clear + getState().startWorkout('user-1', makeWorkoutLog()) + useActiveWorkoutStore.setState({ + loggedGroups: [makeGroupWithActivities()], + }) + + getState().startRestTimer(60, 'Squat', 1, onExpired) + expect(getState().restTimer).not.toBeNull() + + getState().discardWorkout() + + expect(onExpired).not.toHaveBeenCalled() + expect(getState().restTimer).toBeNull() + expect(getState().workoutLog).toBeNull() + }) +}) diff --git a/src/stores/active-workout-store.ts b/src/stores/active-workout-store.ts index 7d10187..e00a1b3 100644 --- a/src/stores/active-workout-store.ts +++ b/src/stores/active-workout-store.ts @@ -80,6 +80,7 @@ interface ActiveWorkoutState { // src/routes/_authenticated/log.$workoutId.tsx) per Tech.md D-1. The store // only holds the current elapsedSeconds value via setElapsedSeconds. let _restInterval: ReturnType | null = null +let _onRestExpired: (() => void) | null = null // Tauri event unlisten handles for the Rust rest timer path. // In Tauri mode, the timer runs in Rust and ticks arrive via events; @@ -158,7 +159,12 @@ interface ActiveWorkoutActions { clearUndo(): void // Rest timer - startRestTimer(seconds: number, exerciseName?: string, setNumber?: number): void + startRestTimer( + seconds: number, + exerciseName?: string, + setNumber?: number, + onExpired?: () => void, + ): void skipRest(): void adjustRest(delta: number): void @@ -262,6 +268,7 @@ export const useActiveWorkoutStore = create void, + ) { // Clear any existing rest interval / Tauri listeners if (_restInterval) clearInterval(_restInterval) _cleanupTauriRestListeners() @@ -496,6 +509,8 @@ export const useActiveWorkoutStore = create('timer_expired', () => { + _onRestExpired?.() + _onRestExpired = null _cleanupTauriRestListeners() set({ restTimer: null }) _publishCurrentState() @@ -553,6 +570,7 @@ export const useActiveWorkoutStore = create Date: Wed, 15 Apr 2026 11:07:21 -0500 Subject: [PATCH 3/7] docs(planning): F022 browser notifications -- mark complete, resolve open questions - Steps.md: status -> Complete, last milestone updated - Spec.md: mark open questions resolved, document advanceMinutes Phase 1 limitation (no session time-of-day column, matches Rust behavior), add revision history entry Feature: 022-Browser-Notifications --- .../Features/022-Browser-Notifications/Spec.md | 15 +++++++++++---- .../Features/022-Browser-Notifications/Steps.md | 4 ++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/Context/Features/022-Browser-Notifications/Spec.md b/Context/Features/022-Browser-Notifications/Spec.md index 20189cf..496776b 100644 --- a/Context/Features/022-Browser-Notifications/Spec.md +++ b/Context/Features/022-Browser-Notifications/Spec.md @@ -65,10 +65,17 @@ Browser users who train without the mobile app miss two of the three notificatio - [ ] Does the rest timer in browser mode currently track expiry in a way the notification layer can hook into, or does a new JS timer need to be introduced alongside the display interpolation hook? The existing `use-timer-interpolation.ts` is display-only and does not emit an expiry event. This determines whether we extend that hook or add a separate timer hook. - [ ] What Supabase query does the browser-mode session reminder polling need to make to determine if today's session is scheduled and not yet logged? The Rust implementation queries `program_activations`, `scheduled_sessions`, and `workout_logs`. The browser equivalent needs the same data via the Supabase JS client. -- [ ] Should the browser permission status in settings also be shown for Tauri users (where the Tauri plugin manages permission separately), or is it browser-only? Current assumption: browser-only display. +- [x] Should the browser permission status in settings also be shown for Tauri users (where the Tauri plugin manages permission separately), or is it browser-only? **Resolved: browser-only.** `BrowserPermissionStatus` is guarded with `!isTauri()`. +- [x] Does the rest timer track expiry for notifications? **Resolved:** Added `onExpired?` callback to `startRestTimer` (Tech.md D-1, Option C). +- [x] Supabase queries for session reminder? **Resolved:** Direct queries to `program_activations`, `blocks`, `block_weeks`, `scheduled_sessions`, `workout_logs`. + +## Known Limitations (Phase 1) + +- **A-007 `advanceMinutes` not consumed:** The `scheduled_sessions` table lacks a time-of-day column, so the browser poller (like the Rust implementation) cannot compute "N minutes before session start." Both fire once per day within the 06:00-20:59 window regardless of `advanceMinutes`. This will be addressed when a session-scheduled-time field is added. ## Revision History -| Date | Change | ADR | -| ---------- | ------------ | --- | -| 2026-04-15 | Initial spec | -- | +| Date | Change | ADR | +| ---------- | ----------------------------------------------- | --- | +| 2026-04-15 | Initial spec | -- | +| 2026-04-15 | Phase 1 implementation; open questions resolved | -- | diff --git a/Context/Features/022-Browser-Notifications/Steps.md b/Context/Features/022-Browser-Notifications/Steps.md index 8519201..61a227b 100644 --- a/Context/Features/022-Browser-Notifications/Steps.md +++ b/Context/Features/022-Browser-Notifications/Steps.md @@ -5,9 +5,9 @@ ## Progress -- **Status:** Not started +- **Status:** Complete - **Current task:** -- -- **Last milestone:** -- +- **Last milestone:** Feature complete -- all assertions verified, full drift check passed ## Team Orchestration From 0e4410aa54859f09a3b3ada66ed0ef8ef0f44910 Mon Sep 17 00:00:00 2001 From: Robert Hamilton Date: Wed, 15 Apr 2026 15:06:33 -0500 Subject: [PATCH 4/7] fix(notifications): resolve P16 review findings -- error handling and guard correctness - P16-001: Capture err in bare catch in getNotificationPreferences JSON parse - P16-002: Add .catch() to discarded sendRestTimerNotification Promise in onExpired callback - P16-003: Wrap new Notification() constructor in try/catch in all three send functions - P16-004: Set _lastRemindedDate before firing notification to prevent Supabase query spam on failure - P16-005: Switch setNotificationPreferences to safeParse with prefixed error log - P16-006: Log console.error in update() guard clause before silent return - P16-007: Use local calendar date (not UTC) for once-per-day reminder guard - P16-008: Extract session_templates coercion with warn log when join returns no name - P16-010: Add unmounted flag to BrowserPermissionStatus to prevent listener leak on unmount race Review: Context/Reviews/0016-pr111-f022-browser-notifications-review.md --- .../profile/notification-settings.tsx | 8 ++++- src/hooks/use-active-workout.ts | 4 ++- src/hooks/use-session-reminder-browser.ts | 19 ++++++++--- src/lib/notification-service.ts | 33 +++++++++++++++---- 4 files changed, 50 insertions(+), 14 deletions(-) diff --git a/src/components/profile/notification-settings.tsx b/src/components/profile/notification-settings.tsx index 79ef4d9..4102967 100644 --- a/src/components/profile/notification-settings.tsx +++ b/src/components/profile/notification-settings.tsx @@ -59,7 +59,10 @@ export function NotificationSettings() { // Merge helper: shallow-merge updated fields with current prefs and persist const update = (patch: Partial) => { - if (!prefs) return + if (!prefs) { + console.error('[notification-settings] update called before preferences loaded') + return + } const next: NotificationPreferences = { ...prefs, ...patch } updatePreferences.mutate(next) } @@ -270,10 +273,12 @@ function BrowserPermissionStatus() { useEffect(() => { if (!('permissions' in navigator)) return + let unmounted = false let status: PermissionStatus | null = null navigator.permissions .query({ name: 'notifications' as PermissionName }) .then((s) => { + if (unmounted) return status = s setPermission(s.state as NotificationPermission) s.onchange = () => setPermission(s.state as NotificationPermission) @@ -281,6 +286,7 @@ function BrowserPermissionStatus() { .catch((err) => console.warn('[notification-settings] Permission query failed:', err)) return () => { + unmounted = true if (status) status.onchange = null } }, []) diff --git a/src/hooks/use-active-workout.ts b/src/hooks/use-active-workout.ts index 9c59f13..1f2d94e 100644 --- a/src/hooks/use-active-workout.ts +++ b/src/hooks/use-active-workout.ts @@ -411,7 +411,9 @@ export function useActiveWorkout() { getNotificationPreferences() .then((prefs) => { storeStartRestTimer(restSeconds, exerciseName, setNum, () => { - sendRestTimerNotification(exerciseName, setNum, prefs) + sendRestTimerNotification(exerciseName, setNum, prefs).catch((err) => { + console.error('[workout] Rest timer notification failed:', err) + }) }) }) .catch((err) => { diff --git a/src/hooks/use-session-reminder-browser.ts b/src/hooks/use-session-reminder-browser.ts index 0d7ef55..9726a67 100644 --- a/src/hooks/use-session-reminder-browser.ts +++ b/src/hooks/use-session-reminder-browser.ts @@ -28,8 +28,9 @@ export function useSessionReminderBrowser(): void { const currentHour = new Date().getHours() if (currentHour < 6 || currentHour > 20) return - // Guard: already reminded today - const today = new Date().toISOString().slice(0, 10) // YYYY-MM-DD + // Guard: already reminded today (local calendar date, not UTC) + const now = new Date() + const today = `${now.getFullYear()}-${String(now.getMonth() + 1).padStart(2, '0')}-${String(now.getDate()).padStart(2, '0')}` if (_lastRemindedDate === today) return const supabase = getSupabaseClient() @@ -121,11 +122,19 @@ export function useSessionReminderBrowser(): void { // 4. Fire notification for the first scheduled session const session = sessions[0] - const sessionName = - (session.session_templates as unknown as { name: string })?.name ?? 'Workout' + const rawTemplate = session.session_templates as unknown as { name: string } | null + if (!rawTemplate?.name) { + console.warn( + '[session-reminder-browser] session_templates join returned no name for session:', + session.id, + ) + } + const sessionName = rawTemplate?.name ?? 'Workout' - await sendSessionReminderNotification(sessionName, prefs) + // Set the guard before firing so a notification failure does not cause + // a Supabase query spam loop on the next poll interval. _lastRemindedDate = today + await sendSessionReminderNotification(sessionName, prefs) } catch (err) { console.error('[session-reminder-browser] Unexpected error during poll:', err) } diff --git a/src/lib/notification-service.ts b/src/lib/notification-service.ts index 349f849..e5e3705 100644 --- a/src/lib/notification-service.ts +++ b/src/lib/notification-service.ts @@ -30,8 +30,8 @@ export async function getNotificationPreferences(): Promise { - const validated = notificationPreferencesSchema.parse(prefs) - const json = JSON.stringify(validated) + const result = notificationPreferencesSchema.safeParse(prefs) + if (!result.success) { + console.error( + '[notification-service] setNotificationPreferences: invalid prefs:', + result.error.issues, + ) + throw new Error('Invalid notification preferences') + } + const json = JSON.stringify(result.data) if (isTauri()) { await invoke('set_app_config', { key: TAURI_CONFIG_KEY, value: json }) @@ -131,7 +138,11 @@ export async function sendPrNotification( // Use the Web Notification API if available and permitted if ('Notification' in window && Notification.permission === 'granted') { - new Notification(title, { body }) + try { + new Notification(title, { body }) + } catch (err) { + console.error('[notification-service] Failed to create PR notification:', err) + } } } @@ -164,7 +175,11 @@ export async function sendRestTimerNotification( if (exerciseName && setNumber != null) parts.push(`Set ${setNumber}`) const body = parts.length > 0 ? parts.join(' \u2014 ') : 'Time to start your next set' - new Notification(title, { body }) + try { + new Notification(title, { body }) + } catch (err) { + console.error('[notification-service] Failed to create rest timer notification:', err) + } } // --------------------------------------------------------------------------- @@ -190,5 +205,9 @@ export async function sendSessionReminderNotification( const title = 'SESSION REMINDER' const body = `${sessionName} is scheduled for today` - new Notification(title, { body }) + try { + new Notification(title, { body }) + } catch (err) { + console.error('[notification-service] Failed to create session reminder notification:', err) + } } From efeddf2b5d2487039c41f5e60f6f75b7d1c00041 Mon Sep 17 00:00:00 2001 From: Robert Hamilton Date: Wed, 15 Apr 2026 15:06:36 -0500 Subject: [PATCH 5/7] docs(planning): F022 -- resolve P16 review, add S008-T for Supabase error branch tests Review: Context/Reviews/0016-pr111-f022-browser-notifications-review.md --- .../022-Browser-Notifications/Steps.md | 8 + ...pr111-f022-browser-notifications-review.md | 189 ++++++++++++++++++ 2 files changed, 197 insertions(+) create mode 100644 Context/Reviews/0016-pr111-f022-browser-notifications-review.md diff --git a/Context/Features/022-Browser-Notifications/Steps.md b/Context/Features/022-Browser-Notifications/Steps.md index 61a227b..6dfeb91 100644 --- a/Context/Features/022-Browser-Notifications/Steps.md +++ b/Context/Features/022-Browser-Notifications/Steps.md @@ -182,6 +182,14 @@ - **Depends:** S001-T, S002-T, S003-T, S004-T, S005, S006-T - **Parallel:** false +- [ ] S008-T: Test Supabase query error branches in `use-session-reminder-browser` + - Cover all five per-query error paths: `activationErr`, `blockErr`, `weekErr`, `sessErr`, `logErr` + - For each: mock the relevant Supabase call to return `{ data: null, error: { message: '...' } }`, assert the `[session-reminder-browser]` console.error fires with the correct prefix, assert `sendSessionReminderNotification` is NOT called, assert `_lastRemindedDate` remains null + - Reuse the `makeQueryBuilder` helper already in the test file + - **Assigned:** builder + - **Depends:** S003-T + - **Parallel:** false + 🏁 MILESTONE: Feature complete -- all assertions verified, full drift check passed --- diff --git a/Context/Reviews/0016-pr111-f022-browser-notifications-review.md b/Context/Reviews/0016-pr111-f022-browser-notifications-review.md new file mode 100644 index 0000000..715b1f6 --- /dev/null +++ b/Context/Reviews/0016-pr111-f022-browser-notifications-review.md @@ -0,0 +1,189 @@ +# PR Review: feat+browser-notif β†’ main + +**Date:** 2026-04-15 +**Feature:** Context/Features/022-Browser-Notifications/ +**Branch:** worktree-feat+browser-notif +**PR:** #111 -- feat(notifications): F022 browser rest timer and session reminder notifications +**Reviewers:** code-reviewer, silent-failure-hunter, pr-test-analyzer +**Status:** 🟒 Resolved + +## Summary + +10 findings total: 9 Fix-Now (2 Critical, 7 High/Medium), 1 Missing Task. All 14 spec assertions are covered by tests (2508/2508 passing). The notification delivery path has a cluster of unhandled async errors: an async function is called in a sync callback without `.catch()`, the `Notification` constructor is uncaught in all three send functions, and the module-level once-per-day guard is not set on failure -- creating a Supabase query spam loop. Six of the nine fix-now findings are violations of existing `error-handling.md` conventions. + +## Findings + +### Fix-Now + +#### [FIX] P16-001: Bare `catch {}` -- no error parameter captured + +- **File:** `src/lib/notification-service.ts:32` +- **Severity:** Critical +- **Detail:** Catch block logs `console.warn` but swallows the actual error value. Violates `error-handling.md` rule: "Never use bare `catch {}`. Always capture the error parameter." Root cause is permanently invisible in production logs if a non-`SyntaxError` is thrown or if Tauri `invoke` returns malformed data. +- **Fix:** Change to `catch (err)` and append `, err` to the warn call. +- **Relates to:** error-handling.md rule 1 +- **Status:** βœ… Fixed +- **Resolution:** Changed `catch {` to `catch (err)` and appended `, err` to the warn call (`notification-service.ts:32`) + +#### [FIX] P16-002: `sendRestTimerNotification` Promise discarded in sync `onExpired` callback + +- **File:** `src/hooks/use-active-workout.ts:~375`, `src/stores/active-workout-store.ts` (tickRest) +- **Severity:** Critical +- **Detail:** `sendRestTimerNotification` is `async` (contains `await Notification.requestPermission()`). The `_onRestExpired` callback is called synchronously from `tickRest` and from the Tauri `timer_expired` listener. The returned Promise is discarded. If `requestPermission()` rejects or the `Notification` constructor throws, an unhandled promise rejection is produced -- silent in most browsers, crashes service worker context on others. +- **Fix:** + ```typescript + storeStartRestTimer(restSeconds, exerciseName, setNum, () => { + sendRestTimerNotification(exerciseName, setNum, prefs).catch((err) => { + console.error('[workout] Rest timer notification failed:', err) + }) + }) + ``` +- **Relates to:** A-001 (rest timer fires on expiry) +- **Status:** βœ… Fixed +- **Resolution:** Added `.catch((err) => console.error('[workout] Rest timer notification failed:', err))` to the `sendRestTimerNotification` call in the `onExpired` callback (`use-active-workout.ts:~403`) + +#### [FIX] P16-003: `new Notification()` constructor uncaught in all three send functions + +- **File:** `src/lib/notification-service.ts:~131, ~161, ~186` +- **Severity:** High +- **Detail:** `sendPrNotification`, `sendRestTimerNotification`, and `sendSessionReminderNotification` all call `new Notification(title, { body })` as a bare statement. The constructor can throw synchronously on permission revoked mid-session (TOCTOU), MDM/kiosk policy, or browser queue limits. In the async functions, the exception propagates as a rejected Promise but has no catch handler (see P16-002). +- **Fix:** Wrap each constructor call: + ```typescript + try { + new Notification(title, { body }) + } catch (err) { + console.error('[notification-service] Failed to create notification:', err) + } + ``` +- **Relates to:** A-001, A-007 +- **Status:** βœ… Fixed +- **Resolution:** Wrapped `new Notification()` in try/catch with `console.error('[notification-service] Failed to create ... notification:', err)` in all three send functions (`notification-service.ts:~134,~167,~193`) + +#### [FIX] P16-004: `_lastRemindedDate` not set on notification failure -- Supabase query spam loop + +- **File:** `src/hooks/use-session-reminder-browser.ts:~121-130` +- **Severity:** High +- **Detail:** `_lastRemindedDate = today` is assigned only after `sendSessionReminderNotification` succeeds. If the notification throws (per P16-003), the guard is never set. On the next 60s poll the guard passes, all four Supabase queries execute again, and the same failure repeats -- ~1,440 round-trips/day. +- **Fix:** Set the guard before firing the notification (or in the catch block at minimum). +- **Relates to:** A-008 (at most once per day) +- **Status:** βœ… Fixed +- **Resolution:** Moved `_lastRemindedDate = today` to before the `sendSessionReminderNotification` call so a notification failure does not leave the guard unset (`use-session-reminder-browser.ts:~127`) + +#### [FIX] P16-005: Throwing `.parse()` at write boundary -- ZodError escapes with no module prefix + +- **File:** `src/lib/notification-service.ts:51` (`setNotificationPreferences`) +- **Severity:** High +- **Detail:** The read path (`getNotificationPreferences`) correctly uses `safeParse` + explicit handling. The write path uses the throwing variant. A ZodError propagates to callers without a `[notification-service]` prefix or diagnostic context, indistinguishable from a Tauri IPC failure or `localStorage` quota error. +- **Fix:** Switch to `safeParse`: + ```typescript + const result = notificationPreferencesSchema.safeParse(prefs) + if (!result.success) { + console.error( + '[notification-service] setNotificationPreferences: invalid prefs:', + result.error.issues, + ) + throw new Error('Invalid notification preferences') + } + ``` +- **Status:** βœ… Fixed +- **Resolution:** Switched to `safeParse` with explicit `console.error('[notification-service] setNotificationPreferences: invalid prefs:', result.error.issues)` and re-throws a typed Error (`notification-service.ts:51`) + +#### [FIX] P16-006: Silent return from user-action guard in `update` helper + +- **File:** `src/components/profile/notification-settings.tsx:~65-68` +- **Severity:** High +- **Detail:** `const update = (patch) => { if (!prefs) return; ... }`. Violates `error-handling.md` "Guard clauses in user-action handlers must never silently return." User toggle presses silently fail when preferences haven't loaded -- no log, no toast, no visual feedback. +- **Fix:** + ```typescript + if (!prefs) { + console.error('[notification-settings] update called before preferences loaded') + return + } + ``` +- **Relates to:** error-handling.md Β§User-Action Guard Clauses +- **Status:** βœ… Fixed +- **Resolution:** Added `console.error('[notification-settings] update called before preferences loaded')` before the early return (`notification-settings.tsx:~63`) + +#### [FIX] P16-007: UTC vs local date mismatch in once-per-day guard + +- **File:** `src/hooks/use-session-reminder-browser.ts:~32` +- **Severity:** High +- **Detail:** `_lastRemindedDate` uses `toISOString().slice(0, 10)` (UTC date) but the hour-window guard uses `new Date().getHours()` (local time). Users in UTC+ offsets receive two reminders on the same local calendar day: the UTC date rolls over while local time is still within the allowed window. +- **Fix:** Use local date: + ```typescript + const now = new Date() + const today = `${now.getFullYear()}-${String(now.getMonth() + 1).padStart(2, '0')}-${String(now.getDate()).padStart(2, '0')}` + ``` +- **Relates to:** A-008 (at most once per day) +- **Status:** βœ… Fixed +- **Resolution:** Replaced `new Date().toISOString().slice(0, 10)` with local date construction using `getFullYear()`/`getMonth()`/`getDate()` (`use-session-reminder-browser.ts:~32`) + +#### [FIX] P16-008: Silent adapter coercion on `session_templates` join with no warn log + +- **File:** `src/hooks/use-session-reminder-browser.ts:~123-126` +- **Severity:** High +- **Detail:** `(session.session_templates as unknown as { name: string })?.name ?? 'Workout'` -- a double-cast coercion with a silent fallback. Violates `error-handling.md` Β§Adapter Boundary Fallbacks: "should log at warn level when the fallback triggers for fields expected to be non-nullable." A null/mismatched join fires the notification with "Workout is scheduled for today" and nothing is logged. +- **Fix:** + ```typescript + const rawTemplate = session.session_templates as unknown as { name: string } | null + if (!rawTemplate?.name) { + console.warn( + '[session-reminder-browser] session_templates join returned no name for session:', + session.id, + ) + } + const sessionName = rawTemplate?.name ?? 'Workout' + ``` +- **Relates to:** error-handling.md Β§Adapter Boundary Fallbacks +- **Status:** βœ… Fixed +- **Resolution:** Extracted to `rawTemplate`, added `console.warn('[session-reminder-browser] session_templates join returned no name for session:', session.id)` when name is missing (`use-session-reminder-browser.ts:~122`) + +### Missing Tasks + +#### [TASK] P16-009: Test coverage for Supabase query error branches + +- **File:** `src/hooks/__tests__/use-session-reminder-browser.test.ts` +- **Severity:** Medium +- **Detail:** Five distinct Supabase error branches (`activationErr`, `blockErr`, `weekErr`, `sessErr`, `logErr`) in `use-session-reminder-browser.ts` have no test coverage. The outer try/catch provides a safety net against crashes, but per `error-handling.md` the per-query `[module-name]` error logs exist precisely for observability -- a refactor could silently remove them. One test on the `activationErr` path validates the pattern; the `makeQueryBuilder` helper already exists in the test file. +- **Relates to:** A-007, A-008 +- **Status:** βœ… Task created +- **Resolution:** Added as S008-T in Steps.md Phase 4 + +### Fix-Now (continued) + +#### [FIX] P16-010: `BrowserPermissionStatus` listener leak on unmount race + +- **File:** `src/components/profile/notification-settings.tsx:~277-292` +- **Severity:** Medium +- **Detail:** If the component unmounts between `permissions.query()` resolving and the `.then()` callback executing, `status` is never assigned in the outer scope, so `s.onchange` is attached to a handler that calls `setPermission` on an unmounted component. The listener is never cleaned up in this race path. React 18 suppresses the setState-after-unmount warning, but the `PermissionStatus` listener persists as a GC issue for the session lifetime. +- **Fix:** Use an `unmounted` flag guard inside the `.then()` callback before assigning `s.onchange`. +- **Status:** βœ… Fixed +- **Resolution:** Added `unmounted` boolean flag; `.then()` checks `if (unmounted) return` before assigning `s.onchange`; cleanup sets `unmounted = true` (`notification-settings.tsx:~270`) + +## Test Coverage Notes + +All 14 spec assertions (A-001 through A-014) are covered. Notable gaps: + +- The five Supabase error branches (`activationErr`, `blockErr`, `weekErr`, `sessErr`, `logErr`) in `use-session-reminder-browser.ts` have no test coverage. +- `sendRestTimerNotification` lacks a "Notification API unavailable" guard test (present for `sendSessionReminderNotification`). + +## Resolution Checklist + +- [x] All [FIX] findings resolved (P16-001 through P16-008, P16-010) +- [x] All [TASK] findings added to Steps.md (P16-009) +- [x] All [ADR] findings have ADRs created or dismissed (none) +- [x] All [RULE] findings applied or dismissed (none) +- [ ] Review verified by review-verify agent + +## Resolution Summary + +**Resolved at:** 2026-04-15 +**Session:** review-resolve -- F022 browser notification error handling fixes + +| Category | Total | Resolved | +| --------- | ------ | -------- | +| [FIX] | 9 | 9 | +| [TASK] | 1 | 1 | +| [ADR] | 0 | 0 | +| [RULE] | 0 | 0 | +| **Total** | **10** | **10** | From c64e65600f95f95e62047e98fc2dd45aa92b9e88 Mon Sep 17 00:00:00 2001 From: Robert Hamilton Date: Wed, 15 Apr 2026 15:11:14 -0500 Subject: [PATCH 6/7] test(notifications): S008-T Supabase query error branch coverage for session reminder Add five tests covering all per-query error paths (activationErr, blockErr, weekErr, sessErr, logErr) in use-session-reminder-browser. Each test asserts the [session-reminder-browser] console.error fires with the correct prefix and that sendSessionReminderNotification is not called. Tasks: S008-T Feature: 022-Browser-Notifications --- .../use-session-reminder-browser.test.ts | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/src/hooks/__tests__/use-session-reminder-browser.test.ts b/src/hooks/__tests__/use-session-reminder-browser.test.ts index 66b8edf..c2f4c44 100644 --- a/src/hooks/__tests__/use-session-reminder-browser.test.ts +++ b/src/hooks/__tests__/use-session-reminder-browser.test.ts @@ -270,4 +270,111 @@ describe('useSessionReminderBrowser', () => { expect(mockSendSessionReminderNotification).not.toHaveBeenCalled() }) + + // ------------------------------------------------------------------------- + // 9. Supabase query error branches + // ------------------------------------------------------------------------- + describe('Supabase query error branches', () => { + /** + * Build a mock Supabase client where all tables return default data + * except the specified table, which returns an error. + */ + function makeMockSupabaseWithError( + errorTable: + | 'program_activations' + | 'blocks' + | 'block_weeks' + | 'scheduled_sessions' + | 'workout_logs', + ) { + const config = defaultSupabaseConfig() + const builders: Record> = { + program_activations: makeQueryBuilder(config.activation), + blocks: makeQueryBuilder(config.block), + block_weeks: makeQueryBuilder(config.blockWeek), + scheduled_sessions: makeQueryBuilder(config.sessions), + workout_logs: makeQueryBuilder(config.todayLogs), + } + builders[errorTable] = makeQueryBuilder(null, { message: `${errorTable} query failed` }) + + return { + from: vi.fn((table: string) => builders[table] ?? makeQueryBuilder(null)), + } + } + + it('logs error and skips notification on program_activations error', async () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + mockGetSupabaseClient.mockReturnValue(makeMockSupabaseWithError('program_activations')) + + renderHook(() => useSessionReminderBrowser()) + await vi.advanceTimersByTimeAsync(0) + + expect(errorSpy).toHaveBeenCalledWith( + '[session-reminder-browser] Failed to query program_activations:', + expect.objectContaining({ message: 'program_activations query failed' }), + ) + expect(mockSendSessionReminderNotification).not.toHaveBeenCalled() + errorSpy.mockRestore() + }) + + it('logs error and skips notification on blocks error', async () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + mockGetSupabaseClient.mockReturnValue(makeMockSupabaseWithError('blocks')) + + renderHook(() => useSessionReminderBrowser()) + await vi.advanceTimersByTimeAsync(0) + + expect(errorSpy).toHaveBeenCalledWith( + '[session-reminder-browser] Failed to query blocks:', + expect.objectContaining({ message: 'blocks query failed' }), + ) + expect(mockSendSessionReminderNotification).not.toHaveBeenCalled() + errorSpy.mockRestore() + }) + + it('logs error and skips notification on block_weeks error', async () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + mockGetSupabaseClient.mockReturnValue(makeMockSupabaseWithError('block_weeks')) + + renderHook(() => useSessionReminderBrowser()) + await vi.advanceTimersByTimeAsync(0) + + expect(errorSpy).toHaveBeenCalledWith( + '[session-reminder-browser] Failed to query block_weeks:', + expect.objectContaining({ message: 'block_weeks query failed' }), + ) + expect(mockSendSessionReminderNotification).not.toHaveBeenCalled() + errorSpy.mockRestore() + }) + + it('logs error and skips notification on scheduled_sessions error', async () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + mockGetSupabaseClient.mockReturnValue(makeMockSupabaseWithError('scheduled_sessions')) + + renderHook(() => useSessionReminderBrowser()) + await vi.advanceTimersByTimeAsync(0) + + expect(errorSpy).toHaveBeenCalledWith( + '[session-reminder-browser] Failed to query scheduled_sessions:', + expect.objectContaining({ message: 'scheduled_sessions query failed' }), + ) + expect(mockSendSessionReminderNotification).not.toHaveBeenCalled() + errorSpy.mockRestore() + }) + + it('logs error and skips notification on workout_logs error', async () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + mockGetSupabaseClient.mockReturnValue(makeMockSupabaseWithError('workout_logs')) + + renderHook(() => useSessionReminderBrowser()) + await vi.advanceTimersByTimeAsync(0) + + expect(errorSpy).toHaveBeenCalledWith( + '[session-reminder-browser] Failed to query workout_logs:', + expect.objectContaining({ message: 'workout_logs query failed' }), + ) + expect(mockSendSessionReminderNotification).not.toHaveBeenCalled() + errorSpy.mockRestore() + }) + }) }) From 1ef92320dd003932eba0f9669475a48875b1d597 Mon Sep 17 00:00:00 2001 From: Robert Hamilton Date: Wed, 15 Apr 2026 15:28:08 -0500 Subject: [PATCH 7/7] docs(reviews): mark P16 review verified by review-verify agent --- Context/Reviews/0016-pr111-f022-browser-notifications-review.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Context/Reviews/0016-pr111-f022-browser-notifications-review.md b/Context/Reviews/0016-pr111-f022-browser-notifications-review.md index 715b1f6..22bb015 100644 --- a/Context/Reviews/0016-pr111-f022-browser-notifications-review.md +++ b/Context/Reviews/0016-pr111-f022-browser-notifications-review.md @@ -173,7 +173,7 @@ All 14 spec assertions (A-001 through A-014) are covered. Notable gaps: - [x] All [TASK] findings added to Steps.md (P16-009) - [x] All [ADR] findings have ADRs created or dismissed (none) - [x] All [RULE] findings applied or dismissed (none) -- [ ] Review verified by review-verify agent +- [x] Review verified by review-verify agent ## Resolution Summary