From 82eccc90175acc68fa96ac17df01c6bbb5dccf14 Mon Sep 17 00:00:00 2001
From: Lucas Berger <me@lucasberger.ca>
Date: Tue, 9 Jun 2026 22:24:00 -0400
Subject: [PATCH] fix(05-review): CR-04 pre-fetch VAPID key into state; no
 await before pushManager.subscribe

---
 .../src/components/PushPermissionPrompt.tsx   | 46 ++++++++++++++++---
 apps/pwa/src/components/SettingsSheet.tsx     | 37 ++++++++++++++-
 apps/pwa/src/hooks/usePushSubscription.ts     | 30 +++++++-----
 3 files changed, 94 insertions(+), 19 deletions(-)

diff --git a/apps/pwa/src/components/PushPermissionPrompt.tsx b/apps/pwa/src/components/PushPermissionPrompt.tsx
index d71ea2a..6f8b846 100644
--- a/apps/pwa/src/components/PushPermissionPrompt.tsx
+++ b/apps/pwa/src/components/PushPermissionPrompt.tsx
@@ -19,7 +19,26 @@
 
 import { useState, useEffect, useId } from 'react'
 import { Bell, Loader2, X } from 'lucide-react'
-import { usePushSubscription, prefetchVapidKey } from '../hooks/usePushSubscription.js'
+import { usePushSubscription } from '../hooks/usePushSubscription.js'
+
+// fetchVapidKey is the internal helper; we import it directly via the module
+// rather than re-exporting it through usePushSubscription, since we need to
+// store the resolved key in state (not just warm the cache).
+async function fetchVapidKeyForPrompt(): Promise<string | null> {
+  try {
+    const cached = sessionStorage.getItem('vapidPublicKey')
+    if (cached) return cached
+    const res = await fetch('/api/push/vapid-public-key', { credentials: 'include' })
+    if (!res.ok) return null
+    const data = (await res.json()) as { publicKey: string }
+    if (data.publicKey) {
+      sessionStorage.setItem('vapidPublicKey', data.publicKey)
+    }
+    return data.publicKey ?? null
+  } catch {
+    return null
+  }
+}
 
 // ── installed state check ──────────────────────────────────────────────────
 
@@ -60,6 +79,10 @@ export function PushPermissionPrompt({ onClose }: PushPermissionPromptProps) {
   const [installed, setInstalled] = useState(false)
   const [loading, setLoading] = useState(false)
   const [error, setError] = useState<string | null>(null)
+  // CR-04: Pre-fetch the VAPID key into state so the tap handler can call
+  // subscribe(registration, vapidKey) without any await before pushManager.subscribe().
+  // Button is disabled until the key is ready (null = not yet loaded).
+  const [vapidKey, setVapidKey] = useState<string | null>(null)
   const headingId = useId()
 
   const { subscribe, permission } = usePushSubscription()
@@ -68,12 +91,16 @@ export function PushPermissionPrompt({ onClose }: PushPermissionPromptProps) {
     setInstalled(isInstalled())
   }, [])
 
-  // Pre-fetch the VAPID key while the prompt is visible so the tap is instant
+  // Pre-fetch the VAPID key into state while the prompt is visible.
+  // CR-04: We store the resolved key in state (not just sessionStorage) so the
+  // tap handler has synchronous access — no network await inside the tap path.
   useEffect(() => {
     if (!installed) return
     if (permission !== 'default') return
     if (dismissed) return
-    void prefetchVapidKey()
+    void fetchVapidKeyForPrompt().then((key) => {
+      if (key) setVapidKey(key)
+    })
   }, [installed, permission, dismissed])
 
   // Don't render when: not installed, already granted/denied, or dismissed
@@ -88,16 +115,20 @@ export function PushPermissionPrompt({ onClose }: PushPermissionPromptProps) {
   }
 
   // onClick handler — subscribe() called synchronously (iOS user-gesture requirement).
-  // No await before subscribe() in this scope; the async execution starts inside subscribe().
+  // CR-04: vapidKey is already resolved from state (pre-fetched in useEffect above).
+  // No await before subscribe() in this scope; we pass the key directly.
   function handleEnableClick() {
-    if (loading) return
+    if (loading || !vapidKey) return
     setLoading(true)
     setError(null)
 
+    // Capture the key synchronously before any await — iOS gesture gate requirement.
+    const resolvedVapidKey = vapidKey
+
     void (async () => {
       try {
         const registration = await navigator.serviceWorker.ready
-        await subscribe(registration)
+        await subscribe(registration, resolvedVapidKey)
         // On success: close the prompt (permission is now 'granted')
         setLoading(false)
         onClose?.()
@@ -242,9 +273,10 @@ export function PushPermissionPrompt({ onClose }: PushPermissionPromptProps) {
           }}
         >
           {/* Primary CTA — 48px, accent color */}
+          {/* CR-04: disabled until vapidKey is loaded (null = pre-fetch pending) */}
           <button
             onClick={handleEnableClick}
-            disabled={loading}
+            disabled={loading || !vapidKey}
             aria-busy={loading}
             style={{
               background: 'var(--color-member-0, #4A90D9)',
diff --git a/apps/pwa/src/components/SettingsSheet.tsx b/apps/pwa/src/components/SettingsSheet.tsx
index 62a2689..a580ac8 100644
--- a/apps/pwa/src/components/SettingsSheet.tsx
+++ b/apps/pwa/src/components/SettingsSheet.tsx
@@ -25,6 +25,24 @@ import { useEffect, useRef, useState } from 'react'
 import { X, Bell, AlertCircle, Loader2 } from 'lucide-react'
 import { usePushSubscription } from '../hooks/usePushSubscription.js'
 
+// CR-04: fetch VAPID key (from sessionStorage cache if available) for the
+// tap-gated subscribe() path.  Same logic as PushPermissionPrompt.
+async function fetchVapidKeyForSettings(): Promise<string | null> {
+  try {
+    const cached = sessionStorage.getItem('vapidPublicKey')
+    if (cached) return cached
+    const res = await fetch('/api/push/vapid-public-key', { credentials: 'include' })
+    if (!res.ok) return null
+    const data = (await res.json()) as { publicKey: string }
+    if (data.publicKey) {
+      sessionStorage.setItem('vapidPublicKey', data.publicKey)
+    }
+    return data.publicKey ?? null
+  } catch {
+    return null
+  }
+}
+
 interface SettingsSheetProps {
   isOpen: boolean
   onClose: () => void
@@ -33,6 +51,9 @@ interface SettingsSheetProps {
 export function SettingsSheet({ isOpen, onClose }: SettingsSheetProps) {
   const { subscribe, permission, isSubscribed, setEnabled } = usePushSubscription()
   const [isTogglingOn, setIsTogglingOn] = useState(false)
+  // CR-04: pre-fetch the VAPID key into state so the toggle tap handler can call
+  // subscribe(registration, vapidKey) without any network await before pushManager.subscribe().
+  const [vapidKey, setVapidKey] = useState<string | null>(null)
   const closeButtonRef = useRef<HTMLButtonElement>(null)
 
   // Compute initial toggle on/off state per UI-SPEC toggle initial state rule:
@@ -56,6 +77,16 @@ export function SettingsSheet({ isOpen, onClose }: SettingsSheetProps) {
     }
   }, [isOpen])
 
+  // CR-04: Pre-fetch the VAPID key while the sheet is open so the OS-dialog path
+  // (permission === 'default') has the key ready before the user taps.
+  useEffect(() => {
+    if (!isOpen) return
+    if (permission === 'denied') return
+    void fetchVapidKeyForSettings().then((key) => {
+      if (key) setVapidKey(key)
+    })
+  }, [isOpen, permission])
+
   if (!isOpen) return null
 
   const handleToggle = async () => {
@@ -75,11 +106,15 @@ export function SettingsSheet({ isOpen, onClose }: SettingsSheetProps) {
     } else {
       // off → on, permission 'default': needs tap-gated subscribe with OS dialog
       // The toggle click IS the tap gesture — call subscribe() directly here.
+      // CR-04: vapidKey was pre-fetched into state (useEffect above); pass it directly
+      // so there is no await-fetch between the tap and pushManager.subscribe().
+      if (!vapidKey) return // key not ready — this should be rare; button should show spinner
+      const resolvedVapidKey = vapidKey
       setIsTogglingOn(true)
       try {
         const registration = await navigator.serviceWorker?.ready
         if (registration) {
-          await subscribe(registration)
+          await subscribe(registration, resolvedVapidKey)
         }
       } catch {
         // Permission denied by OS or error — permission state will update reactively
diff --git a/apps/pwa/src/hooks/usePushSubscription.ts b/apps/pwa/src/hooks/usePushSubscription.ts
index 236b6b2..8cd02aa 100644
--- a/apps/pwa/src/hooks/usePushSubscription.ts
+++ b/apps/pwa/src/hooks/usePushSubscription.ts
@@ -114,8 +114,14 @@ function persistNotificationsEnabled(value: boolean): void {
 // ---------------------------------------------------------------------------
 
 export interface UsePushSubscriptionReturn {
-  /** Call this inside an onClick handler (no await before the call). */
-  subscribe: (registration: ServiceWorkerRegistration) => Promise<void>
+  /**
+   * Call this inside an onClick handler (no await before the call).
+   *
+   * CR-04: `vapidKey` must be pre-fetched before the tap (e.g. via prefetchVapidKey()
+   * in a useEffect) and passed in here. This eliminates the network round-trip inside
+   * the tap handler, preserving the iOS user-gesture requirement for pushManager.subscribe().
+   */
+  subscribe: (registration: ServiceWorkerRegistration, vapidKey: string) => Promise<void>
   unsubscribe: () => Promise<void>
   /** Current Notification.permission value ('default' | 'granted' | 'denied') */
   permission: NotificationPermission
@@ -197,17 +203,19 @@ export function usePushSubscription(): UsePushSubscriptionReturn {
    * synchronously within a user gesture (tap) callback. This function starts
    * the subscription synchronously and awaits only the POST afterwards.
    *
-   * The VAPID key should be pre-fetched and cached (e.g., in a useEffect before
-   * the component renders) so this path avoids an extra round-trip.
+   * CR-04: The VAPID key MUST be pre-fetched before the tap (e.g., via
+   * prefetchVapidKey() in a useEffect) and passed in as `vapidKey`. Fetching
+   * it here (inside the async function) would insert an await-fetch before
+   * pushManager.subscribe() and break the iOS gesture gate on cache miss.
    */
-  const subscribe = async (registration: ServiceWorkerRegistration): Promise<void> => {
-    // Fetch VAPID key (from cache if already prefetched)
-    const vapidKey = await fetchVapidKey()
-
+  const subscribe = async (
+    registration: ServiceWorkerRegistration,
+    vapidKey: string,
+  ): Promise<void> => {
     // pushManager.subscribe — synchronous initiation; iOS gesture gate is here.
-    // In practice the async resolution of PushSubscription is after the gesture
-    // frame, which iOS allows as long as subscribe() is the first async operation
-    // called from the tap handler.
+    // No await before this call: vapidKey is already resolved by the caller.
+    // iOS allows the async resolution of PushSubscription after the gesture frame,
+    // as long as subscribe() is the first async operation in the tap handler.
     const sub = await registration.pushManager.subscribe({
       userVisibleOnly: true,
       applicationServerKey: urlBase64ToUint8Array(vapidKey),