v0.12.7 FCM bug fixes

FCM_IMPLEMENTATION_NOTES.md

@@ -0,0 +1,311 @@
+# FCM PWA Implementation Notes
+_Reference for applying FCM fixes to other projects_
+
+---
+
+## Part 1 — Guide Key Points (fcm_details.txt)
+
+### How FCM works (the correct flow)
+1. User grants notification permission
+2. Firebase generates a unique FCM token for the device
+3. Token is stored on your server for targeting
+4. Server sends push requests to Firebase
+5. Firebase delivers notifications to the device
+6. Service worker handles display and click interactions
+
+### Common vibe-coding failures with FCM
+
+**1. Service worker confusion**
+Auto-generated setups often register multiple service workers or put Firebase logic in the wrong file. The dedicated `firebase-messaging-sw.js` must be served from root scope. Splitting logic across a redirect stub (`importScripts('/sw.js')`) causes background notifications to silently fail.
+
+**2. Deprecated API usage**
+Using `messaging.usePublicVapidKey()` and `messaging.useServiceWorker()` instead of passing options directly to `getToken()`. The correct modern pattern is:
+```javascript
+const token = await messaging.getToken({
+  vapidKey: VAPID_KEY,
+  serviceWorkerRegistration: registration
+});
+```
+
+**3. Token generation without durable storage**
+Tokens disappear when users switch devices, clear storage, or the server restarts. Without a persistent store (file, database) and proper Docker volume mounts, tokens are lost on every restart.
+
+**4. Poor permission flow**
+Requesting notification permission immediately on page load gets denied by users. Permission should be requested on a meaningful user action (e.g. login), not on first visit.
+
+**5. Missing notificationclick handler**
+Without a `notificationclick` handler in the service worker, clicking a notification does nothing. Users expect it to open or focus the app.
+
+**6. Silent failures**
+Tokens can be null, service workers can fail to register, VAPID keys can be wrong — and nothing surfaces in the UI. Every layer needs explicit error checking and user-visible feedback.
+
+**7. iOS blind spots**
+iOS requires the PWA to be added to the home screen, strict HTTPS, and a correctly structured manifest. Test on real iOS devices, not just Chrome on Android/desktop.
+
+### Correct `getToken()` pattern (from guide)
+```javascript
+// Register SW first, then pass it directly to getToken
+const registration = await navigator.serviceWorker.register('/firebase-messaging-sw.js');
+const token = await getToken(messaging, {
+  vapidKey: VAPID_KEY,
+  serviceWorkerRegistration: registration
+});
+if (!token) throw new Error('getToken() returned empty — check VAPID key and SW');
+```
+
+### Correct `firebase-messaging-sw.js` pattern (from guide)
+```javascript
+importScripts('https://www.gstatic.com/firebasejs/10.7.1/firebase-app-compat.js');
+importScripts('https://www.gstatic.com/firebasejs/10.7.1/firebase-messaging-compat.js');
+
+firebase.initializeApp({ /* config */ });
+const messaging = firebase.messaging();
+
+messaging.onBackgroundMessage((payload) => {
+  self.registration.showNotification(payload.notification.title, {
+    body: payload.notification.body,
+    icon: '/icon-192.png',
+    badge: '/icon-192.png',
+    tag: 'fcm-notification',
+    data: payload.data
+  });
+});
+
+self.addEventListener('notificationclick', (event) => {
+  event.notification.close();
+  if (event.action === 'close') return;
+  event.waitUntil(
+    clients.matchAll({ type: 'window', includeUncontrolled: true }).then((clientList) => {
+      for (const client of clientList) {
+        if (client.url === '/' && 'focus' in client) return client.focus();
+      }
+      if (clients.openWindow) return clients.openWindow('/');
+    })
+  );
+});
+```
+
+---
+
+## Part 2 — Code Fixes Applied to fcm-app
+
+### app.js fixes
+
+**Fix: `showUserInfo()` missing**
+Function was called on login and session restore but never defined — crashed immediately on login.
+```javascript
+function showUserInfo() {
+  document.getElementById('loginForm').style.display = 'none';
+  document.getElementById('userInfo').style.display = 'block';
+  document.getElementById('currentUser').textContent = users[currentUser]?.name || currentUser;
+}
+```
+
+**Fix: `setupApp()` wrong element IDs**
+`getElementById('sendNotification')` and `getElementById('logoutBtn')` returned null — no element with those IDs existed in the HTML.
+```javascript
+// Wrong
+document.getElementById('sendNotification').addEventListener('click', sendNotification);
+// Fixed
+document.getElementById('sendNotificationBtn').addEventListener('click', sendNotification);
+// Also added id="logoutBtn" to the logout button in index.html
+```
+
+**Fix: `logout()` not clearing localStorage**
+Session was restored on next page load even after logout.
+```javascript
+function logout() {
+  currentUser = null;
+  fcmToken = null;
+  localStorage.removeItem('currentUser'); // was missing
+  // ...
+}
+```
+
+**Fix: Race condition in messaging initialization**
+`initializeFirebase()` was fire-and-forget. When called again from `login()`, it returned early setting `messaging = firebase.messaging()` without the VAPID key or SW being configured. Now returns and caches a promise:
+```javascript
+let initPromise = null;
+function initializeFirebase() {
+  if (initPromise) return initPromise;
+  initPromise = navigator.serviceWorker.register('/sw.js')
+    .then((registration) => {
+      swRegistration = registration;
+      messaging = firebase.messaging();
+    })
+    .catch((error) => { initPromise = null; throw error; });
+  return initPromise;
+}
+// In login():
+await initializeFirebase(); // ensures messaging is ready before getToken()
+```
+
+**Fix: `deleteToken()` invalidating tokens on every page load**
+`deleteToken()` was called on every page load, invalidating the push subscription. The server still held the old (now invalid) token. When another device sent, the stale token failed and `recipients` stayed 0.
+Solution: removed `deleteToken()` entirely — it's not needed when `serviceWorkerRegistration` is passed directly to `getToken()`.
+
+**Fix: Session restore without re-registering token**
+When a user's session was restored from localStorage, `showUserInfo()` was called but no new FCM token was generated or sent to the server. After a server restart the server had no record of the token.
+```javascript
+// In setupApp(), after restoring session:
+if (Notification.permission === 'granted') {
+  initializeFirebase()
+    .then(() => messaging.getToken({ vapidKey: VAPID_KEY, serviceWorkerRegistration: swRegistration }))
+    .then(token => { if (token) return registerToken(currentUser, token); })
+    .catch(err => console.error('Token refresh on session restore failed:', err));
+}
+```
+
+**Fix: Deprecated VAPID/SW API replaced**
+```javascript
+// Removed (deprecated):
+messaging.usePublicVapidKey(VAPID_KEY);
+messaging.useServiceWorker(registration);
+const token = await messaging.getToken();
+
+// Replaced with:
+const VAPID_KEY = 'your-vapid-key';
+let swRegistration = null;
+// swRegistration set inside initializeFirebase() .then()
+const token = await messaging.getToken({ vapidKey: VAPID_KEY, serviceWorkerRegistration: swRegistration });
+```
+
+**Fix: Null token guard**
+`getToken()` can return null — passing null to the server produced a confusing 400 error.
+```javascript
+if (!token) {
+  throw new Error('getToken() returned empty — check VAPID key and service worker');
+}
+```
+
+**Fix: Error message included server response**
+```javascript
+// Before: throw new Error('Failed to register token');
+// After:
+throw new Error(`Server returned ${response.status}: ${errorText}`);
+```
+
+**Fix: Duplicate foreground message handlers**
+`handleForegroundMessages()` was called on every login, stacking up `onMessage` listeners.
+```javascript
+let foregroundHandlerSetup = false;
+function handleForegroundMessages() {
+  if (foregroundHandlerSetup) return;
+  foregroundHandlerSetup = true;
+  messaging.onMessage(/* ... */);
+}
+```
+
+**Fix: `login()` event.preventDefault() crash**
+Button called `login()` with no argument, so `event.preventDefault()` threw on undefined.
+```javascript
+async function login(event) {
+  if (event) event.preventDefault(); // guard added
+```
+
+**Fix: `firebase-messaging-sw.js` redirect stub replaced**
+File was `importScripts('/sw.js')` — a vibe-code anti-pattern. Replaced with full Firebase messaging setup including `onBackgroundMessage` and `notificationclick` handler (see Part 1 pattern above).
+
+**Fix: `notificationclick` handler added to `sw.js`**
+Clicking a background notification did nothing. Handler added to focus existing window or open a new one.
+
+**Fix: CDN URLs removed from `urlsToCache` in `sw.js`**
+External CDN URLs in `cache.addAll()` can fail on opaque responses, breaking the entire SW install.
+```javascript
+// Removed from urlsToCache:
+// 'https://www.gstatic.com/firebasejs/10.7.1/firebase-app-compat.js',
+// 'https://www.gstatic.com/firebasejs/10.7.1/firebase-messaging-compat.js'
+```
+
+### server.js fixes
+
+**Fix: `icon`/`badge`/`tag` in wrong notification object**
+These fields are only valid in `webpush.notification`, not the top-level `notification` (which only accepts `title`, `body`, `imageUrl`).
+```javascript
+// Wrong:
+notification: { title, body, icon: '/icon-192.png', badge: '/icon-192.png', tag: 'fcm-test' }
+// Fixed:
+notification: { title, body },
+webpush: {
+  notification: { icon: '/icon-192.png', badge: '/icon-192.png', tag: 'fcm-test' },
+  // ...
+}
+```
+
+**Fix: `saveTokens()` in route handler not crash-safe**
+```javascript
+try {
+  saveTokens();
+} catch (saveError) {
+  console.error('Failed to persist tokens to disk:', saveError);
+}
+```
+
+**Fix: `setInterval(saveTokens)` uncaught exception crashed the server**
+An unhandled throw inside `setInterval` exits the Node.js process. Docker restarts it with empty state.
+```javascript
+setInterval(() => {
+  try { saveTokens(); }
+  catch (error) { console.error('Auto-save tokens failed:', error); }
+}, 30000);
+```
+
+---
+
+## Part 3 — Docker / Infrastructure Fixes
+
+### Root cause of "no other users" bug
+The server was crashing every ~30 seconds, wiping all registered tokens from memory. The crash chain:
+1. `saveTokens()` threw `EACCES: permission denied` (nodejs user can't write to root-owned `/app`)
+2. This propagated out of `setInterval` as an uncaught exception
+3. Node.js exited the process
+4. Docker restarted the container with empty state
+5. Tokens were never on disk, so restart = all tokens lost
+
+### Dockerfile fix
+```dockerfile
+# Create non-root user AND a writable data directory (while still root)
+RUN addgroup -g 1001 -S nodejs && \
+    adduser -S nodejs -u 1001 && \
+    mkdir -p /app/data && \
+    chown nodejs:nodejs /app/data
+```
+`WORKDIR /app` is root-owned — the `nodejs` user can only write to subdirectories explicitly granted to it.
+
+### docker-compose.yml fix
+```yaml
+services:
+  your-app:
+    volumes:
+      - app_data:/app/data   # named volume survives container rebuilds
+
+volumes:
+  app_data:
+```
+Without this, `tokens.json` lives in the container's ephemeral layer and is deleted on every `docker-compose up --build`.
+
+### server.js path fix
+```javascript
+// Changed from:
+const TOKENS_FILE = './tokens.json';
+// To:
+const TOKENS_FILE = './data/tokens.json';
+```
+
+---
+
+## Checklist for applying to another project
+
+- [ ] `firebase-messaging-sw.js` contains real FCM logic (not a redirect stub)
+- [ ] `notificationclick` handler present in service worker
+- [ ] CDN URLs NOT in `urlsToCache` in any service worker
+- [ ] `initializeFirebase()` returns a promise; login awaits it before calling `getToken()`
+- [ ] `getToken()` receives `{ vapidKey, serviceWorkerRegistration }` directly — no deprecated `usePublicVapidKey` / `useServiceWorker`
+- [ ] `deleteToken()` is NOT called on page load
+- [ ] Session restore re-registers FCM token if `Notification.permission === 'granted'`
+- [ ] Null/empty token check before sending to server
+- [ ] `icon`/`badge`/`tag` are in `webpush.notification`, not top-level `notification`
+- [ ] `saveTokens()` (or equivalent) wrapped in try-catch everywhere it's called including `setInterval`
+- [ ] Docker: data directory created with correct user ownership in Dockerfile
+- [ ] Docker: named volume mounted for data directory in docker-compose.yml
+- [ ] Duplicate foreground message handler registration is guarded

backend/package.json

@@ -1,6 +1,6 @@
 {
   "name": "rosterchirp-backend",
-  "version": "0.12.6",
+  "version": "0.12.7",
   "description": "RosterChirp backend server",
   "main": "src/index.js",
   "scripts": {

backend/src/routes/push.js

@@ -47,6 +47,10 @@ async function sendPushToUser(schema, userId, payload) {
             groupId: payload.groupId ? String(payload.groupId) : '',
           },
           android: { priority: 'high' },
+          apns: {
+            headers: { 'apns-priority': '10' },
+            payload: { aps: { contentAvailable: true } },
+          },
           webpush: { headers: { Urgency: 'high' } },
         });
       } catch (err) {
@@ -141,6 +145,10 @@ router.post('/test', authMiddleware, async (req, res) => {
         const message = {
           token: sub.fcm_token,
           android: { priority: 'high' },
+          apns: {
+            headers: { 'apns-priority': '10' },
+            payload: { aps: { contentAvailable: true } },
+          },
           webpush: { headers: { Urgency: 'high' } },
         };
 

build.sh

@@ -13,7 +13,7 @@
 # ─────────────────────────────────────────────────────────────
 set -euo pipefail
 
-VERSION="${1:-0.12.6}"
+VERSION="${1:-0.12.7}"
 ACTION="${2:-}"
 REGISTRY="${REGISTRY:-}"
 IMAGE_NAME="rosterchirp"

frontend/package.json

@@ -1,6 +1,6 @@
 {
   "name": "rosterchirp-frontend",
-  "version": "0.12.6",
+  "version": "0.12.7",
   "private": true,
   "scripts": {
     "dev": "vite",

frontend/src/contexts/AuthContext.jsx

@@ -42,6 +42,7 @@ export function AuthProvider({ children }) {
     try { await api.logout(); } catch {}
     localStorage.removeItem('tc_token');
     sessionStorage.removeItem('tc_token');
+    localStorage.removeItem('rc_fcm_token');
     setUser(null);
     setMustChangePassword(false);
   };

frontend/src/pages/Chat.jsx

@@ -112,7 +112,7 @@ export default function Chat() {
 
         // Dynamically import the Firebase SDK (tree-shaken, only loaded when needed)
         const { initializeApp, getApps } = await import('firebase/app');
-        const { getMessaging, getToken, deleteToken } = await import('firebase/messaging');
+        const { getMessaging, getToken } = await import('firebase/messaging');
 
         const firebaseApp = getApps().length
           ? getApps()[0]
@@ -126,13 +126,12 @@ export default function Chat() {
           if (granted !== 'granted') return;
         }
 
-        // Always delete any cached token first so we get a fresh Web Push subscription.
-        // Stale tokens (e.g. from a previous install or a Chrome push-subscription reset)
-        // still look valid to Firebase but silently fail to deliver — deleteToken forces
-        // Chrome to create a brand-new subscription and register it with FCM.
-        console.log('[Push] Clearing any cached FCM token...');
-        await deleteToken(firebaseMessaging).catch(() => {});
-
+        // Do NOT call deleteToken() here. Deleting the token on every page load (or
+        // every visibility-change) forces Chrome to create a new Web Push subscription
+        // each time. During the brief window between delete and re-register the server
+        // still holds the old (now invalid) token, so any in-flight message fails to
+        // deliver. Passing serviceWorkerRegistration directly to getToken() is enough
+        // for Firebase to return the existing valid token without needing a refresh.
         console.log('[Push] Requesting FCM token...');
         const fcmToken = await getToken(firebaseMessaging, {
           vapidKey,
@@ -144,6 +143,14 @@ export default function Chat() {
         }
         console.log('[Push] FCM token obtained:', fcmToken.slice(0, 30) + '...');
 
+        // Skip the server round-trip if this token is already registered.
+        // Avoids a redundant DB write on every tab-focus / visibility change.
+        const cachedToken = localStorage.getItem('rc_fcm_token');
+        if (cachedToken === fcmToken) {
+          console.log('[Push] Token unchanged — skipping subscribe');
+          return;
+        }
+
         const token = localStorage.getItem('tc_token') || sessionStorage.getItem('tc_token');
         const subRes = await fetch('/api/push/subscribe', {
           method: 'POST',
@@ -154,6 +161,7 @@ export default function Chat() {
           const err = await subRes.json().catch(() => ({}));
           console.warn('[Push] Subscribe failed:', err.error || subRes.status);
         } else {
+          localStorage.setItem('rc_fcm_token', fcmToken);
           console.log('[Push] FCM subscription registered successfully');
         }
       } catch (e) {