Merge pull request #909 from Srejoye/docs/808-dispatch-notifications-runbook

docs(cron): operational runbook, API reference, and secret governance for the push-dispatch pipeline

Author: durdana3105

TROUBLESHOOTING.md

@@ -69,3 +69,49 @@ Welcome to the troubleshooting guide for Peer Learning! If you encounter problem
 - For OAuth, verify that the Client ID and Secret match the ones configured in your OAuth provider's developer console, and that the callback URL matches your Supabase project's redirect URL.
 - After updating environment variables, restart the development server before testing authentication again.
 - If you encounter a "Failed to fetch" error during signup, verify that your `.env` file contains valid Supabase credentials and that the application has been restarted after any configuration changes.
+
+## 6. Push Notifications Not Delivering
+
+### Symptom: Cron runs but `sent` is always 0
+
+**Check VAPID configuration.** The response body will contain `{ "error": "Missing VAPID push server env" }` if `VAPID_PUBLIC_KEY` or `VAPID_PRIVATE_KEY` are not set in `backend/.env`. Generate keys and set them:
+
+```bash
+npx web-push generate-vapid-keys
+```
+
+Add to `backend/.env`:
+```env
+VAPID_PUBLIC_KEY=<generated>
+VAPID_PRIVATE_KEY=<generated>
+VAPID_SUBJECT=mailto:admin@yourdomain.com
+```
+
+> Changing these keys invalidates all existing browser subscriptions. Users must re-subscribe.
+
+### Symptom: Cron returns `HTTP 401` or `403`
+
+`CRON_SECRET` is missing or does not match what the scheduler is sending. Verify the env var is set on the server and that the scheduler is passing `Authorization: Bearer <CRON_SECRET>`. Check `[AUDIT]` lines in server logs to confirm the secret type being presented.
+
+### Symptom: Cron returns `HTTP 429` immediately
+
+The 60-second per-route cooldown is active. The cron fired twice within 60 seconds (Vercel's at-least-once guarantee can cause this). Wait 60 seconds and retry. If this blocks manual drain, see the Manual Drain Procedure in `docs/smart-notifications.md`.
+
+### Symptom: Notifications accumulate but are never delivered (queue keeps growing)
+
+1. Confirm the cron scheduler is running and reaching the server (look for `[AUDIT]` log lines).
+2. Check queue depth:
+   ```sql
+   SELECT COUNT(*) FROM notifications WHERE push_sent_at IS NULL;
+   ```
+3. If the queue is large, manually drain it — see `docs/smart-notifications.md` → **Manual drain procedure**.
+
+### Symptom: `sent` is much lower than `processed` on every run
+
+Most subscriptions in `push_subscriptions` are likely expired (browser was uninstalled, permission was revoked, or VAPID keys changed). The cron absorbs individual push failures silently. Check:
+
+```sql
+SELECT COUNT(*) FROM push_subscriptions;
+```
+
+If zero or very low, users need to re-subscribe by visiting the site and granting notification permission again.

backend/controllers/cronController.js

@@ -12,6 +12,37 @@ const getSupabaseClient = () => {
   return createClient(supabaseUrl, serviceRoleKey);
 };
 
+/*
+ * dispatchPushNotifications
+ *
+ * Bulk-delivers pending push notifications to subscribed browsers.
+ *
+ * Batch cap: fetches at most 100 rows per invocation (oldest-first). A
+ * notification backlog drains at 100 rows/minute assuming a 1-minute cron
+ * schedule. If queue depth grows faster than this, trigger additional manual
+ * runs (after the 60-second cooldown) or mark stale rows sent via SQL — see
+ * docs/smart-notifications.md → "Manual drain procedure".
+ *
+ * Failure absorption: Promise.allSettled is used intentionally so that a
+ * single failing push (network error, expired subscription) does not block
+ * delivery to other recipients. The `sent` vs `processed` delta in the
+ * response reflects failures. Each notification row is stamped with
+ * `push_sent_at` regardless of delivery outcome — there is no automatic retry
+ * for failed pushes.
+ *
+ * Subscription expiry (HTTP 410 / 404): the web-push library returns a
+ * rejection with `statusCode 410` (subscription expired) or `404` (endpoint
+ * not found) when a browser unsubscribes or revokes permission. These
+ * subscriptions should be deleted from `push_subscriptions` to avoid
+ * accumulating dead endpoints. NOTE: this cleanup is currently implemented in
+ * sendPushNotification (notificationController.js) but NOT here. A future
+ * improvement should add equivalent cleanup to this handler.
+ *
+ * @route   POST /api/cron/dispatch-notifications
+ * @access  CRON_SECRET (Authorization: Bearer)
+ * @returns {{ sent: number, processed: number }}
+ */
+
 export const dispatchPushNotifications = async (req, res, next) => {
   try {
     const vapidPublicKey = process.env.VAPID_PUBLIC_KEY;

backend/routers/notificationRoutes.js

@@ -11,6 +11,28 @@ import {
 
 const router = express.Router();
 
+/*
+ * verifyNotificationAuth
+ *
+ * Secures /api/notifications/send-push with the WEBHOOK_SECRET.
+ *
+ * This is a SEPARATE secret from CRON_SECRET (used on /api/cron/*).
+ * The distinction is intentional:
+ *
+ *   CRON_SECRET  — held by the scheduler (Vercel Cron / pg_cron). Authorises
+ *                  bulk operations that touch up to 100 rows per call.
+ *
+ *   WEBHOOK_SECRET — held by trusted internal services or admin tooling.
+ *                    Authorises single-user targeted push delivery.
+ *
+ * Keeping them separate means a compromised scheduler secret does not grant
+ * arbitrary single-user push access, and vice versa. Both can be rotated
+ * independently — see docs/smart-notifications.md → "Secrets Reference".
+ *
+ * Applies the same rate-limit and cooldown helpers as requireCronSecret to
+ * prevent abuse via this endpoint too.
+ */
+
 // Custom middleware to strictly verify WEBHOOK secret
 const verifyNotificationAuth = (req, res, next) => {
   const authHeader = req.headers.authorization;

backend/tests/docs.test.js

@@ -0,0 +1,80 @@
+import { describe, it, expect } from "vitest";
+import { readFileSync } from "fs";
+import { resolve } from "path";
+
+/**
+ * Documentation completeness smoke-tests.
+ *
+ * These tests assert that production-facing API endpoints are mentioned in
+ * docs/api.md. They are intentionally coarse — they check for the route path
+ * string, not for a full schema — so they are fast and not brittle to prose
+ * changes. Add a new assertion here whenever a new route is added to
+ * cronRoutes.js or notificationRoutes.js.
+ */
+
+const repoRoot = resolve(process.cwd(), "..");
+const apiDoc = readFileSync(resolve(repoRoot, "docs/api.md"), "utf-8");
+const notifDoc = readFileSync(resolve(repoRoot, "docs/smart-notifications.md"), "utf-8");
+
+describe("API documentation completeness", () => {
+  const requiredRoutes = [
+    "/api/cron/dispatch-notifications",
+    "/api/cron/reminders",
+    "/api/cron/mentorship-reminders",
+    "/api/notifications/send-push",
+  ];
+
+  for (const route of requiredRoutes) {
+    it(`docs/api.md documents the route ${route}`, () => {
+      expect(apiDoc).toContain(route);
+    });
+  }
+
+  it("docs/api.md documents the CRON_SECRET auth requirement", () => {
+    expect(apiDoc).toContain("CRON_SECRET");
+  });
+
+  it("docs/api.md documents the WEBHOOK_SECRET auth requirement", () => {
+    expect(apiDoc).toContain("WEBHOOK_SECRET");
+  });
+});
+
+describe("Operational runbook completeness", () => {
+  it("smart-notifications.md contains a queue-depth monitoring query", () => {
+    expect(notifDoc).toContain("push_sent_at IS NULL");
+  });
+
+  it("smart-notifications.md documents the 100-row batch cap", () => {
+    expect(notifDoc).toContain("100");
+  });
+
+  it("smart-notifications.md documents the manual drain procedure", () => {
+    expect(notifDoc.toLowerCase()).toContain("manual drain");
+  });
+
+  it("smart-notifications.md documents the CRON_SECRET / WEBHOOK_SECRET split", () => {
+    expect(notifDoc).toContain("CRON_SECRET");
+    expect(notifDoc).toContain("WEBHOOK_SECRET");
+  });
+
+  it("smart-notifications.md documents the 60-second cooldown", () => {
+    expect(notifDoc).toContain("60-second");
+  });
+
+  it("smart-notifications.md covers the 410/404 subscription expiry contract", () => {
+    expect(notifDoc).toContain("410");
+    expect(notifDoc).toContain("404");
+  });
+});
+
+describe("TROUBLESHOOTING.md completeness", () => {
+  const troubleshoot = readFileSync(resolve(repoRoot, "TROUBLESHOOTING.md"), "utf-8");
+
+  it("TROUBLESHOOTING.md has a push notification section", () => {
+    expect(troubleshoot.toLowerCase()).toContain("push notification");
+  });
+
+  it("TROUBLESHOOTING.md mentions VAPID configuration", () => {
+    expect(troubleshoot).toContain("VAPID");
+  });
+});

docs/api.md

@@ -66,3 +66,109 @@ Generates an AI summary of a chat session.
 **Security & Rate Limiting**:
 - Requires a valid Supabase JWT token.
 - Protected by a custom, in-house rate limiter middleware (`backend/middlewares/rateLimiter.js`) to prevent abuse.
+
+## 🔔 Cron Endpoints
+
+All cron endpoints require `Authorization: Bearer <CRON_SECRET>` and are subject to a 5 req/min per-IP rate limit and a 60-second per-route cooldown. Invocations within the cooldown window return `HTTP 429`.
+
+---
+
+### `POST /api/cron/dispatch-notifications`
+
+Dequeues up to 100 pending push notifications (`push_sent_at IS NULL`, oldest first) and delivers them to all registered browser push subscriptions for each recipient.
+
+**Auth:** `Authorization: Bearer <CRON_SECRET>`
+
+**Response `200`:**
+```json
+{
+  "sent": 42,
+  "processed": 45
+}
+```
+
+| Field | Description |
+|---|---|
+| `processed` | Number of notification rows fetched (max 100) |
+| `sent` | Number of individual push deliveries that succeeded |
+
+`processed - sent` reflects push failures (expired subscriptions, VAPID misconfiguration, etc.). Individual failures are absorbed and do not block delivery to other recipients. Each notification row is stamped with `push_sent_at` regardless of delivery outcome.
+
+**Error responses:**
+
+| Status | Condition |
+|---|---|
+| `401` | Missing or malformed `Authorization` header |
+| `403` | Secret mismatch |
+| `429` | Rate limit or 60-second cooldown exceeded |
+| `500` | VAPID keys not configured, or Supabase error |
+| `503` | `CRON_SECRET` env var not set on the server |
+
+---
+
+### `POST /api/cron/reminders`
+
+Inserts `session_reminder` notifications for all sessions with `status = 'upcoming'` whose `start_time` falls within the next 14–16 minutes. Idempotent via `upsert` on `(user_id, entity_id, type)`.
+
+**Auth:** `Authorization: Bearer <CRON_SECRET>`
+
+**Response `200`:**
+```json
+{ "inserted": 12 }
+```
+
+**Error responses:** same as `/api/cron/dispatch-notifications`.
+
+---
+
+### `POST /api/cron/mentorship-reminders`
+
+Inserts `mentorship_reminder` notifications for incomplete milestones due within 24 hours or already overdue. Notifies both the mentor and mentee. Idempotent via `upsert` on `(user_id, entity_id, type)`.
+
+**Auth:** `Authorization: Bearer <CRON_SECRET>`
+
+**Response `200`:**
+```json
+{ "inserted": 4 }
+```
+
+**Error responses:** same as `/api/cron/dispatch-notifications`.
+
+---
+
+## 🔔 Notification Endpoints
+
+### `POST /api/notifications/send-push`
+
+Delivers a push notification to a single user's registered browser subscriptions. Stale subscriptions (HTTP 410/404 from the push service) are automatically deleted.
+
+**Auth:** `Authorization: Bearer <WEBHOOK_SECRET>`
+
+> This endpoint uses a **separate secret** (`WEBHOOK_SECRET`) from the cron endpoints (`CRON_SECRET`). See the Secrets Reference in `docs/smart-notifications.md` for rotation guidance.
+
+**Request body:**
+```json
+{
+  "user_id": "uuid",
+  "title": "string (max 100 chars)",
+  "body": "string (max 500 chars)",
+  "action_url": "/optional/path"
+}
+```
+
+**Response `200`:**
+```json
+{
+  "sent": 1,
+  "failed": 0
+}
+```
+
+**Error responses:**
+
+| Status | Condition |
+|---|---|
+| `400` | Missing `user_id`, `title`, or `body`; payload type invalid; content too long |
+| `401` | Missing or invalid `Authorization` header |
+| `404` | No push subscriptions found for the given `user_id` |
+| `500` | VAPID keys not configured, or Supabase error |