From ed050319c356e128f5ec1da33d0d43960ff052a5 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Wed, 19 Aug 2026 10:41:49 +0000 Subject: [PATCH 1/5] feat(web): remind people about a meeting before it starts Email and web push at 1 day, 1 hour, 15 minutes and 1 minute before a meeting. Four toggles in settings, each governing both channels, all on by default. ## When a reminder is due Each lead time owns a *band* rather than an instant: the day-before reminder covers 24 hours down to 1 hour, the hour-before covers 60 to 15 minutes, and so on down. The obvious implementation -- fire when now is within a minute or two of `start - lead` -- fails both ways at once. Miss the window to a deploy or a slow tick and the reminder is gone with nothing to show it; widen the tolerance to compensate and the hour-before notice goes out twenty minutes late, when the fifteen-minute one is about to say something more accurate. With bands there is no tolerance to tune. A runner that has been down for three hours comes back and sends the tightest reminder still true, and skips the ones it slept through -- which is the right fate for a reminder about something that has since drawn much closer. The message says the real remaining time rather than the band's name, so a meeting booked 25 minutes ahead is never told it starts "in 1 hour". ## Sending each one once `meeting_reminders` is a ledger whose unique constraint is claimed *before* the message goes out, so overlapping cron runs cannot both send. At-most-once deliberately: a crash between claim and send loses one reminder, where the alternative can mail somebody repeatedly about a meeting already in their diary. The key includes `occurrence_at`, and that column is the whole reason this works for recurring meetings. #81 made a recurring series one row whose `scheduled_at` is the next occurrence, rolled forward after each one finishes -- so a key of (session, lead) would fire a weekly meeting's day-before notice once, in its first week, and stay silent for ever with a ledger that looked correctly filled in. Keying on the instant reopens the slots at every roll-forward. ## The toggles are not where the others are The existing event toggles only render once the browser is subscribed to push, which is right for them -- they describe push and nothing else. These four also govern the emailed reminder, so they get their own always-visible section: hiding them behind a push subscription would leave anyone who has not enabled push, or cannot, receiving email they had no way to turn off. Invitees have no account and therefore no settings row, so they get email with no per-lead control; declining the invitation stops all of it. ## Scheduling A minute cron (pg_cron -> pg_net) posts to /api/reminders/run with a shared secret. It lives in the app rather than in Postgres because the Resend client, the web-push library and the VAPID keys are already wired up here. Without REMINDERS_CRON_SECRET the route refuses everything: an endpoint that mails a meeting's whole invitee list is not one to leave open because a variable was missed on a new environment. Migration applied to prod as 20260819103845_meeting_reminders. The cron job is not scheduled yet -- that waits for this to deploy and for the secret to be set. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/app/api/reminders/run/route.test.ts | 87 +++++ apps/web/src/app/api/reminders/run/route.ts | 69 ++++ .../notifications/NotificationPreferences.tsx | 64 +++- apps/web/src/lib/meeting-reminders-runner.ts | 325 ++++++++++++++++++ apps/web/src/lib/meeting-reminders.test.ts | 116 +++++++ apps/web/src/lib/meeting-reminders.ts | 140 ++++++++ apps/web/src/lib/push.ts | 10 + .../20260819140000_meeting_reminders.sql | 115 +++++++ 8 files changed, 925 insertions(+), 1 deletion(-) create mode 100644 apps/web/src/app/api/reminders/run/route.test.ts create mode 100644 apps/web/src/app/api/reminders/run/route.ts create mode 100644 apps/web/src/lib/meeting-reminders-runner.ts create mode 100644 apps/web/src/lib/meeting-reminders.test.ts create mode 100644 apps/web/src/lib/meeting-reminders.ts create mode 100644 supabase/migrations/20260819140000_meeting_reminders.sql diff --git a/apps/web/src/app/api/reminders/run/route.test.ts b/apps/web/src/app/api/reminders/run/route.test.ts new file mode 100644 index 00000000..859a2b10 --- /dev/null +++ b/apps/web/src/app/api/reminders/run/route.test.ts @@ -0,0 +1,87 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +/** + * The only thing standing between this endpoint and anybody who finds the URL + * is a shared secret, and the endpoint mails a meeting's entire invitee list. + * So these cover the refusals rather than the happy path. + */ + +const runMeetingReminders = vi.fn(); +vi.mock('@/lib/meeting-reminders-runner', () => ({ + runMeetingReminders: (...args: unknown[]) => runMeetingReminders(...args), +})); + +const SECRET = 'a-long-enough-secret-value'; + +async function post(headers: Record = {}): Promise { + const { POST } = await import('./route'); + return POST(new Request('https://pairux.com/api/reminders/run', { method: 'POST', headers })); +} + +describe('POST /api/reminders/run', () => { + beforeEach(() => { + vi.resetModules(); + runMeetingReminders.mockReset(); + runMeetingReminders.mockResolvedValue({ + meetings: 2, + emails: 3, + pushes: 1, + skipped: 0, + errors: [], + }); + process.env.REMINDERS_CRON_SECRET = SECRET; + }); + + afterEach(() => { + delete process.env.REMINDERS_CRON_SECRET; + }); + + it('runs and reports what it sent when the secret matches', async () => { + const res = await post({ authorization: `Bearer ${SECRET}` }); + expect(res.status).toBe(200); + expect(await res.json()).toMatchObject({ ok: true, meetings: 2, emails: 3, pushes: 1 }); + expect(runMeetingReminders).toHaveBeenCalledOnce(); + }); + + it('refuses a wrong secret, and sends nothing', async () => { + const res = await post({ authorization: 'Bearer not-the-secret-value' }); + expect(res.status).toBe(401); + expect(runMeetingReminders).not.toHaveBeenCalled(); + }); + + it('refuses a missing header', async () => { + const res = await post(); + expect(res.status).toBe(401); + expect(runMeetingReminders).not.toHaveBeenCalled(); + }); + + it('refuses a bare token without the Bearer scheme', async () => { + const res = await post({ authorization: SECRET }); + expect(res.status).toBe(401); + expect(runMeetingReminders).not.toHaveBeenCalled(); + }); + + it('refuses everything when the secret is not configured at all', async () => { + // The case worth being deliberate about: a new environment where the + // variable was forgotten must be closed, not open. An endpoint that mails + // an invitee list is not one to leave ungated by omission. + delete process.env.REMINDERS_CRON_SECRET; + const res = await post({ authorization: 'Bearer anything' }); + expect(res.status).toBe(401); + expect(runMeetingReminders).not.toHaveBeenCalled(); + }); + + it('a secret of a different length is refused rather than throwing', async () => { + // timingSafeEqual throws on unequal lengths, which would turn a wrong guess + // into a 500 and leak the secret's length through the error. + const res = await post({ authorization: 'Bearer short' }); + expect(res.status).toBe(401); + }); + + it('reports a failed run as 500 rather than pretending it worked', async () => { + runMeetingReminders.mockRejectedValue(new Error('database is on fire')); + const res = await post({ authorization: `Bearer ${SECRET}` }); + expect(res.status).toBe(500); + expect(await res.json()).toMatchObject({ ok: false }); + }); +}); diff --git a/apps/web/src/app/api/reminders/run/route.ts b/apps/web/src/app/api/reminders/run/route.ts new file mode 100644 index 00000000..6b1dea14 --- /dev/null +++ b/apps/web/src/app/api/reminders/run/route.ts @@ -0,0 +1,69 @@ +import { timingSafeEqual } from 'node:crypto'; +import { NextResponse } from 'next/server'; + +import { runMeetingReminders } from '@/lib/meeting-reminders-runner'; + +/** + * The tick that sends meeting reminders. + * + * Called once a minute by pg_cron via pg_net. It lives here rather than in the + * database because everything it needs -- the Resend client, the web-push + * library, the VAPID keys -- is already wired up in this app, and a second copy + * of that inside Postgres would be a second thing to keep in step. + * + * There is no user session behind this request, so it is authorised by a shared + * secret instead. Without `REMINDERS_CRON_SECRET` set the route refuses + * everything: an endpoint that mails a meeting's whole invitee list is not one + * to leave open because a variable was forgotten on a new environment. + */ + +export const dynamic = 'force-dynamic'; +export const maxDuration = 60; + +/** + * Compare in constant time, and only after the lengths match. + * + * `timingSafeEqual` throws on differing lengths, which would both crash the + * route and leak the secret's length through the error; the explicit check + * turns that into an ordinary refusal. + */ +function secretMatches(provided: string, expected: string): boolean { + const a = Buffer.from(provided); + const b = Buffer.from(expected); + if (a.length !== b.length) return false; + return timingSafeEqual(a, b); +} + +function authorised(request: Request): boolean { + const expected = process.env.REMINDERS_CRON_SECRET; + if (!expected) return false; + + const header = request.headers.get('authorization') ?? ''; + const token = header.startsWith('Bearer ') ? header.slice(7) : ''; + if (!token) return false; + + return secretMatches(token, expected); +} + +export async function POST(request: Request): Promise { + if (!authorised(request)) { + // Deliberately identical whether the secret is wrong or unset: the caller + // is a cron job, not a person who needs help debugging. + return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); + } + + try { + const summary = await runMeetingReminders(); + + // Errors inside the run are reported rather than thrown: one meeting whose + // host has been deleted should not make the whole tick look like a failure, + // and pg_cron's log is the only place anybody will see this. + return NextResponse.json({ ok: true, ...summary }); + } catch (err) { + console.error('[reminders] run failed:', err); + return NextResponse.json( + { ok: false, error: err instanceof Error ? err.message : String(err) }, + { status: 500 } + ); + } +} diff --git a/apps/web/src/components/notifications/NotificationPreferences.tsx b/apps/web/src/components/notifications/NotificationPreferences.tsx index 05e5bdeb..1dcfe4c7 100644 --- a/apps/web/src/components/notifications/NotificationPreferences.tsx +++ b/apps/web/src/components/notifications/NotificationPreferences.tsx @@ -13,6 +13,10 @@ interface NotificationPrefs { hostDisconnected: boolean; creatorLive: boolean; directMessage: boolean; + meetingReminder1Day: boolean; + meetingReminder1Hour: boolean; + meetingReminder15Min: boolean; + meetingReminder1Min: boolean; } const DEFAULT_PREFS: NotificationPrefs = { @@ -24,9 +28,16 @@ const DEFAULT_PREFS: NotificationPrefs = { hostDisconnected: true, creatorLive: true, directMessage: true, + meetingReminder1Day: true, + meetingReminder1Hour: true, + meetingReminder15Min: true, + meetingReminder1Min: true, }; -const PREF_LABELS: Record, string> = { +const PREF_LABELS: Record< + Exclude, + string +> = { controlRequest: 'Control requests', chatMessage: 'Chat messages', participantJoined: 'Participant joined', @@ -36,6 +47,29 @@ const PREF_LABELS: Record, string> directMessage: 'Someone sends you a direct message', }; +type ReminderKey = + | 'meetingReminder1Day' + | 'meetingReminder1Hour' + | 'meetingReminder15Min' + | 'meetingReminder1Min'; + +/** + * Meeting reminders, kept out of the list above on purpose. + * + * The toggles above only render once the browser is subscribed to push, which + * is right for them โ€” they describe push notifications and nothing else. These + * four also govern the *emailed* reminder, so hiding them behind a push + * subscription would leave anyone who has not enabled push, or cannot (an + * iPhone that has not installed the app, a locked-down browser), receiving + * emails they have no way to turn off. + */ +const REMINDER_LABELS: Record = { + meetingReminder1Day: '1 day before', + meetingReminder1Hour: '1 hour before', + meetingReminder15Min: '15 minutes before', + meetingReminder1Min: '1 minute before', +}; + function Toggle({ enabled, onChange, @@ -197,6 +231,34 @@ export function NotificationPreferences() { )} )} + + {/* + * Always rendered, unlike the block above. These govern the emailed + * reminder as well as the pushed one, so somebody who has never enabled + * push still needs a way to turn them off. + */} +
+
+

+ Meeting reminders +

+

+ Sent by email, and as a notification when push is on. +

+
+ {(Object.entries(REMINDER_LABELS) as [ReminderKey, string][]).map(([key, label]) => ( +
+ {label} + { + void savePreference(key, !preferences[key]); + }} + disabled={saving} + /> +
+ ))} +
); } diff --git a/apps/web/src/lib/meeting-reminders-runner.ts b/apps/web/src/lib/meeting-reminders-runner.ts new file mode 100644 index 00000000..c774dbeb --- /dev/null +++ b/apps/web/src/lib/meeting-reminders-runner.ts @@ -0,0 +1,325 @@ +import { createClient, type SupabaseClient } from '@supabase/supabase-js'; +import { createEmailer } from '@profullstack/emailer'; + +import { dueLead, timeUntil, wantsReminder, REMINDER_PREF_KEYS } from './meeting-reminders'; +import { sendPushToUser } from './push'; + +/** + * Send whatever meeting reminders are due right now. + * + * Called once a minute by pg_cron through `/api/reminders/run`. Everything + * about *when* a reminder is due lives in `meeting-reminders.ts` and is tested + * without a database; this module is the part that has to talk to Postgres, + * Resend and the push service, and its job is mostly to be careful about + * claiming before sending. + */ + +/** How far ahead to look. Nothing can be due beyond the widest lead time. */ +const HORIZON_MINUTES = 1440; + +/** Meetings examined per run, newest deadline first. */ +const MEETING_LIMIT = 200; + +export interface ReminderSummary { + meetings: number; + emails: number; + pushes: number; + skipped: number; + errors: string[]; +} + +interface MeetingRow { + id: string; + host_user_id: string; + title: string; + description: string | null; + scheduled_at: string; + duration_minutes: number; + join_code: string; +} + +interface InviteeRow { + id: string; + email: string; + name: string | null; + rsvp_status: string; +} + +function admin(): SupabaseClient { + const url = process.env.NEXT_PUBLIC_SUPABASE_URL; + const key = process.env.SUPABASE_SERVICE_ROLE_KEY; + if (!url || !key) throw new Error('Missing Supabase environment variables'); + return createClient(url, key, { auth: { autoRefreshToken: false, persistSession: false } }); +} + +/** + * Take the slot for one message, returning false if somebody already had it. + * + * This is the whole concurrency story. The unique constraint on + * `meeting_reminders` is claimed *before* the message goes out, so two runs of + * the cron overlapping -- or one run retried after a timeout -- cannot both + * send. Postgres reports the loser as 23505 and it simply moves on. + * + * The cost is that a crash between this returning true and the send completing + * loses that reminder for good. That is the intended trade: see the migration. + */ +async function claim( + db: SupabaseClient, + row: { + scheduled_session_id: string; + occurrence_at: string; + lead_minutes: number; + recipient_kind: 'host' | 'invitee'; + recipient_key: string; + channel: 'email' | 'push'; + } +): Promise { + const { error } = await db.from('meeting_reminders').insert(row); + if (!error) return true; + // 23505 is unique_violation: somebody else already claimed it, which is a + // normal outcome here rather than a failure worth reporting. + if (error.code === '23505') return false; + throw new Error(`claim failed: ${error.message}`); +} + +/** + * Lead times already sent for one occurrence, indexed by recipient and channel. + * + * Read once per meeting rather than once per recipient: a meeting with thirty + * invitees would otherwise cost thirty round trips to answer a question one + * indexed read covers. + */ +async function alreadySent( + db: SupabaseClient, + sessionId: string, + occurrenceAt: string +): Promise>> { + const { data } = await db + .from('meeting_reminders') + .select('recipient_kind, recipient_key, channel, lead_minutes') + .eq('scheduled_session_id', sessionId) + .eq('occurrence_at', occurrenceAt); + + const byRecipient = new Map>(); + for (const row of data ?? []) { + const r = row as { recipient_kind: string; recipient_key: string; channel: string; lead_minutes: number }; + const key = `${r.recipient_kind}:${r.recipient_key}:${r.channel}`; + const set = byRecipient.get(key) ?? new Set(); + set.add(Number(r.lead_minutes)); + byRecipient.set(key, set); + } + return byRecipient; +} + +function reminderEmailHtml(opts: { + title: string; + when: string; + startsAtLabel: string; + joinUrl: string; + joinCode: string; + recipientName: string | null; +}): string { + const greeting = opts.recipientName ? `Hi ${opts.recipientName},` : 'Hi,'; + return ` + +${opts.title} + +
+
+

Starting ${opts.when}

+
+
+

${greeting}

+

${opts.title}

+

${opts.startsAtLabel}

+ Join Meeting +

Join code: ${opts.joinCode}

+
+
+

Sent via PairUX ยท Manage reminders in settings

+
+
+ +`; +} + +/** + * @param now injectable so a test can place itself inside a band + */ +export async function runMeetingReminders(now: Date = new Date()): Promise { + const db = admin(); + const summary: ReminderSummary = { meetings: 0, emails: 0, pushes: 0, skipped: 0, errors: [] }; + + const horizon = new Date(now.getTime() + HORIZON_MINUTES * 60_000); + + // Only meetings that have not started and are inside the widest lead time. + // `status = 'pending'` drops cancelled and completed ones; a recurring series + // stays pending across occurrences, which is why the ledger keys on the + // instant rather than the row. + const { data: meetings, error } = await db + .from('scheduled_sessions') + .select('id, host_user_id, title, description, scheduled_at, duration_minutes, join_code') + .eq('status', 'pending') + .gt('scheduled_at', now.toISOString()) + .lte('scheduled_at', horizon.toISOString()) + .order('scheduled_at', { ascending: true }) + .limit(MEETING_LIMIT); + + if (error) { + summary.errors.push(`load meetings: ${error.message}`); + return summary; + } + + const resendApiKey = process.env.RESEND_API_KEY; + const appUrl = process.env.NEXT_PUBLIC_APP_URL ?? 'https://pairux.com'; + const defaultFrom = process.env.EMAIL_FROM ?? 'PairUX '; + const emailer = resendApiKey ? createEmailer({ resendApiKey, defaultFrom }) : null; + if (!emailer) summary.errors.push('RESEND_API_KEY not configured; push only'); + + for (const raw of meetings ?? []) { + const meeting = raw as MeetingRow; + summary.meetings += 1; + + try { + const startsAt = new Date(meeting.scheduled_at); + const occurrenceAt = meeting.scheduled_at; + const sent = await alreadySent(db, meeting.id, occurrenceAt); + const joinUrl = `${appUrl}/join/${meeting.join_code}`; + const startsAtLabel = startsAt.toUTCString(); + + // ---- the host: an account, so both channels are possible + const hostSettings = await db + .from('profiles') + .select('settings, display_name') + .eq('id', meeting.host_user_id) + .single(); + const notifications = ((hostSettings.data?.settings ?? {}) as Record) + .notifications as Record | undefined; + + const hostEmailLead = dueLead( + startsAt, + now, + sent.get(`host:${meeting.host_user_id}:email`) ?? new Set() + ); + const hostPushLead = dueLead( + startsAt, + now, + sent.get(`host:${meeting.host_user_id}:push`) ?? new Set() + ); + + if (hostEmailLead && wantsReminder(notifications, hostEmailLead) && emailer) { + const { data: user } = await db.auth.admin.getUserById(meeting.host_user_id); + const to = user?.user?.email; + if (to) { + const claimed = await claim(db, { + scheduled_session_id: meeting.id, + occurrence_at: occurrenceAt, + lead_minutes: hostEmailLead, + recipient_kind: 'host', + recipient_key: meeting.host_user_id, + channel: 'email', + }); + if (claimed) { + await (emailer as { send: (o: unknown) => Promise }).send({ + to, + subject: `${meeting.title} starts ${timeUntil(startsAt, now)}`, + html: reminderEmailHtml({ + title: meeting.title, + when: timeUntil(startsAt, now), + startsAtLabel, + joinUrl, + joinCode: meeting.join_code, + recipientName: hostSettings.data?.display_name ?? null, + }), + }); + summary.emails += 1; + } else summary.skipped += 1; + } + } + + if (hostPushLead && wantsReminder(notifications, hostPushLead)) { + const claimed = await claim(db, { + scheduled_session_id: meeting.id, + occurrence_at: occurrenceAt, + lead_minutes: hostPushLead, + recipient_kind: 'host', + recipient_key: meeting.host_user_id, + channel: 'push', + }); + if (claimed) { + // `sendPushToUser` applies the same preference itself, so a host who + // has turned this lead off spends a claim and sends nothing. That is + // deliberate: the claim is what stops the next tick trying again. + const result = await sendPushToUser( + meeting.host_user_id, + REMINDER_PREF_KEYS[hostPushLead], + { + title: `${meeting.title} starts ${timeUntil(startsAt, now)}`, + body: `Join code ${meeting.join_code}`, + url: `/join/${meeting.join_code}`, + tag: `meeting-${meeting.id}`, + } + ); + summary.pushes += result.sent; + } else summary.skipped += 1; + } + + // ---- invitees: email addresses, frequently with no account behind them + if (emailer) { + const { data: invitees } = await db + .from('scheduled_session_invitees') + .select('id, email, name, rsvp_status') + .eq('scheduled_session_id', meeting.id); + + for (const inviteeRaw of invitees ?? []) { + const invitee = inviteeRaw as InviteeRow; + // Somebody who said no does not need four more messages about it. + if (invitee.rsvp_status === 'declined') continue; + + const lead = dueLead( + startsAt, + now, + sent.get(`invitee:${invitee.id}:email`) ?? new Set() + ); + if (!lead) continue; + + // No preference lookup: an invitee has no account and therefore no + // settings row. The per-lead toggles are an account feature, and the + // way out for an invitee is to decline, which stops all of them. + const claimed = await claim(db, { + scheduled_session_id: meeting.id, + occurrence_at: occurrenceAt, + lead_minutes: lead, + recipient_kind: 'invitee', + recipient_key: invitee.id, + channel: 'email', + }); + if (!claimed) { + summary.skipped += 1; + continue; + } + + await (emailer as { send: (o: unknown) => Promise }).send({ + to: invitee.email, + subject: `${meeting.title} starts ${timeUntil(startsAt, now)}`, + html: reminderEmailHtml({ + title: meeting.title, + when: timeUntil(startsAt, now), + startsAtLabel, + joinUrl, + joinCode: meeting.join_code, + recipientName: invitee.name, + }), + }); + summary.emails += 1; + } + } + } catch (err) { + // One bad meeting must not stop the others; the claim it may already have + // taken is the only thing lost. + summary.errors.push(`${meeting.id}: ${err instanceof Error ? err.message : String(err)}`); + } + } + + return summary; +} diff --git a/apps/web/src/lib/meeting-reminders.test.ts b/apps/web/src/lib/meeting-reminders.test.ts new file mode 100644 index 00000000..dc073c3e --- /dev/null +++ b/apps/web/src/lib/meeting-reminders.test.ts @@ -0,0 +1,116 @@ +import { describe, it, expect } from 'vitest'; + +import { + dueLead, + timeUntil, + wantsReminder, + LEAD_MINUTES, + REMINDER_PREF_KEYS, +} from './meeting-reminders'; + +/** A fixed meeting time, so nothing here depends on when it runs. */ +const START = new Date('2026-08-20T15:00:00.000Z'); + +/** `n` minutes before the meeting. */ +const before = (n: number) => new Date(START.getTime() - n * 60_000); + +describe('dueLead', () => { + it('says nothing until the widest lead is reached', () => { + expect(dueLead(START, before(2880))).toBeNull(); // two days out + expect(dueLead(START, before(1441))).toBeNull(); // a minute too early + expect(dueLead(START, before(1440))).toBe(1440); // exactly a day: fires + }); + + it('gives each lead a band rather than an instant', () => { + // The whole point. A runner that ticks late, or is down for a while, still + // finds the reminder that is currently true instead of missing it entirely. + expect(dueLead(START, before(1440))).toBe(1440); + expect(dueLead(START, before(600))).toBe(1440); + expect(dueLead(START, before(61))).toBe(1440); + + expect(dueLead(START, before(60))).toBe(60); + expect(dueLead(START, before(16))).toBe(60); + + expect(dueLead(START, before(15))).toBe(15); + expect(dueLead(START, before(2))).toBe(15); + + expect(dueLead(START, before(1))).toBe(1); + expect(dueLead(START, before(0.5))).toBe(1); + }); + + it('no two bands claim the same instant', () => { + // Walk every minute of the day before the meeting and assert exactly one + // answer at each. A gap would drop a reminder; an overlap would send two. + for (let m = 1440; m >= 1; m -= 1) { + const lead = dueLead(START, before(m)); + expect(lead, `at ${m} minutes out`).not.toBeNull(); + expect(LEAD_MINUTES).toContain(lead); + } + }); + + it('goes quiet once the meeting has started', () => { + // The guard that stops a runner which has been asleep from mailing everyone + // about a meeting that is over. + expect(dueLead(START, START)).toBeNull(); + expect(dueLead(START, new Date(START.getTime() + 60_000))).toBeNull(); + expect(dueLead(START, new Date(START.getTime() + 86_400_000))).toBeNull(); + }); + + it('does not re-send a lead already recorded for this occurrence', () => { + // The ledger's answer feeds back in here: inside the day band, having + // already sent the day reminder means silence rather than the next one down. + expect(dueLead(START, before(600), new Set([1440]))).toBeNull(); + // ...but the hour band is a different slot and still fires. + expect(dueLead(START, before(30), new Set([1440]))).toBe(60); + }); + + it('a meeting scheduled inside a band starts at that band, not the widest', () => { + // Booked 25 minutes ahead: it should get the hour-band reminder and never + // pretend a day's notice was given. + expect(dueLead(START, before(25))).toBe(60); + expect(dueLead(START, before(25), new Set([1440]))).toBe(60); + }); +}); + +describe('timeUntil', () => { + it('describes the real remaining time, not the band', () => { + // A meeting booked 25 minutes out sends from the hour band; saying "in 1 + // hour" would simply be false, so the copy uses this instead. + expect(timeUntil(START, before(25))).toBe('in 25 minutes'); + expect(timeUntil(START, before(60))).toBe('in about an hour'); + expect(timeUntil(START, before(180))).toBe('in about 3 hours'); + expect(timeUntil(START, before(1440))).toBe('tomorrow'); + expect(timeUntil(START, before(2880))).toBe('in 2 days'); + expect(timeUntil(START, before(1))).toBe('in about a minute'); + expect(timeUntil(START, before(0.4))).toBe('in about a minute'); + }); +}); + +describe('wantsReminder', () => { + it('defaults to on when the preference has never been written', () => { + // Every preference in this product defaults on, and a reminder that stayed + // silent because a key was missing would look exactly like a broken feature. + expect(wantsReminder(undefined, 1440)).toBe(true); + expect(wantsReminder(null, 60)).toBe(true); + expect(wantsReminder({}, 15)).toBe(true); + expect(wantsReminder({ somethingElse: false }, 1)).toBe(true); + }); + + it('only false turns one off', () => { + expect(wantsReminder({ meetingReminder1Day: false }, 1440)).toBe(false); + expect(wantsReminder({ meetingReminder1Day: true }, 1440)).toBe(true); + }); + + it('each lead has its own key, so they are independent', () => { + const off = { meetingReminder1Min: false }; + expect(wantsReminder(off, 1)).toBe(false); + expect(wantsReminder(off, 15)).toBe(true); + expect(wantsReminder(off, 60)).toBe(true); + expect(wantsReminder(off, 1440)).toBe(true); + }); + + it('every lead maps to a distinct preference key', () => { + const keys = Object.values(REMINDER_PREF_KEYS); + expect(new Set(keys).size).toBe(LEAD_MINUTES.length); + }); +}); diff --git a/apps/web/src/lib/meeting-reminders.ts b/apps/web/src/lib/meeting-reminders.ts new file mode 100644 index 00000000..5c9e82aa --- /dev/null +++ b/apps/web/src/lib/meeting-reminders.ts @@ -0,0 +1,140 @@ +/** + * When a meeting reminder is due, and what it is called. + * + * Deliberately free of database and network so the awkward part -- deciding + * whether a given meeting deserves a message right now -- can be tested without + * a Supabase instance or a clock that really has to pass. + */ + +/** The four lead times, longest first. Minutes before the meeting starts. */ +export const LEAD_MINUTES = [1440, 60, 15, 1] as const; + +export type LeadMinutes = (typeof LEAD_MINUTES)[number]; + +/** + * The preference key that governs a lead time, in `profiles.settings.notifications`. + * + * One key per lead time rather than one for reminders as a whole, because the + * point of the feature is that somebody can keep the day-before nudge and drop + * the one that fires while they are already walking to their desk. + */ +export const REMINDER_PREF_KEYS = { + 1440: 'meetingReminder1Day', + 60: 'meetingReminder1Hour', + 15: 'meetingReminder15Min', + 1: 'meetingReminder1Min', +} as const satisfies Record; + +export type ReminderPrefKey = (typeof REMINDER_PREF_KEYS)[LeadMinutes]; + +/** Human labels, used in the settings UI and in the subject line. */ +export const LEAD_LABELS = { + 1440: '1 day', + 60: '1 hour', + 15: '15 minutes', + 1: '1 minute', +} as const satisfies Record; + +/** + * Which single lead time, if any, is due for a meeting right now. + * + * The rule is a partition rather than a window with a tolerance: a lead is due + * while the time remaining falls between it and the next tighter lead. So the + * day-before reminder owns everything from 24 hours down to 1 hour, the + * hour-before owns 60 to 15 minutes, and so on. + * + * That is worth the paragraph, because the obvious implementation -- fire when + * `now` is within a minute or two of `start - lead` -- fails in both directions + * at once. If the runner misses its window (a deploy, a slow tick, a database + * blip) the reminder is lost with no way to notice; and widening the tolerance + * to compensate starts sending the hour-before notice twenty minutes late, when + * the fifteen-minute one is about to say something more accurate anyway. + * + * With bands there is no tolerance to tune. A runner that has been down for + * three hours comes back and sends the tightest reminder that is still true, + * which is the one worth sending; the ones it slept through are skipped, which + * is what should happen to a reminder about something that has since drawn much + * closer. + * + * Returns the *widest* due lead the recipient has not been sent yet -- the + * caller supplies that set -- so a first run inside the 15-minute band does not + * also fire the day and hour reminders it slept through. + * + * @param startsAt when the meeting (or this occurrence of it) begins + * @param now + * @param alreadySent lead times already recorded for this occurrence + recipient + * @returns the lead to send, or null when nothing is due + */ +export function dueLead( + startsAt: Date, + now: Date, + alreadySent: ReadonlySet = new Set() +): LeadMinutes | null { + const remainingMs = startsAt.getTime() - now.getTime(); + + // Already started, or already over. A reminder for a meeting that has begun + // is not a reminder, and this is the guard that keeps a stalled runner from + // mailing everybody about yesterday when it wakes up. + if (remainingMs <= 0) return null; + + const remaining = remainingMs / 60_000; + + for (let i = 0; i < LEAD_MINUTES.length; i += 1) { + const lead = LEAD_MINUTES[i] as LeadMinutes; + // The floor of this lead's band is the next tighter lead, or zero for the + // last one. Exclusive at the bottom so the bands cannot both claim an + // instant, inclusive at the top so a meeting exactly a day out fires. + const floor = i + 1 < LEAD_MINUTES.length ? (LEAD_MINUTES[i + 1] as number) : 0; + + if (remaining <= lead && remaining > floor) { + return alreadySent.has(lead) ? null : lead; + } + } + + // Further out than the widest lead: nothing to say yet. + return null; +} + +/** + * How long until the meeting, in words, for the message itself. + * + * The band a reminder belongs to is not necessarily what the recipient should + * be told. A meeting created twenty-five minutes before it starts falls in the + * hour band, and "starting in 1 hour" would simply be false. This says what is + * actually true at the moment of sending. + * + * @param startsAt + * @param now + */ +export function timeUntil(startsAt: Date, now: Date): string { + const minutes = Math.round((startsAt.getTime() - now.getTime()) / 60_000); + + if (minutes <= 1) return 'in about a minute'; + if (minutes < 60) return `in ${minutes} minutes`; + + const hours = Math.round(minutes / 60); + if (hours < 24) return hours === 1 ? 'in about an hour' : `in about ${hours} hours`; + + const days = Math.round(hours / 24); + return days === 1 ? 'tomorrow' : `in ${days} days`; +} + +/** + * Whether a recipient wants this lead time. + * + * Absent means yes. Every preference in this product defaults on -- see + * `DEFAULT_PREFERENCES` in push.ts -- and a reminder that silently did nothing + * because a key had not been written yet would be indistinguishable from the + * feature being broken. + * + * @param settings the `notifications` object out of `profiles.settings` + * @param lead + */ +export function wantsReminder( + settings: Record | null | undefined, + lead: LeadMinutes +): boolean { + const key = REMINDER_PREF_KEYS[lead]; + const value = settings?.[key]; + return value === undefined || value === null ? true : value !== false; +} diff --git a/apps/web/src/lib/push.ts b/apps/web/src/lib/push.ts index 29c99cd7..bcb062b2 100644 --- a/apps/web/src/lib/push.ts +++ b/apps/web/src/lib/push.ts @@ -10,6 +10,16 @@ const DEFAULT_PREFERENCES = { hostDisconnected: true, creatorLive: true, directMessage: true, + // Meeting reminders, one key per lead time so a host can keep the day-before + // nudge and drop the one that fires while they are already walking to their + // desk. The same four keys gate the emailed reminder โ€” see + // `meeting-reminders.ts`, which reads them from the same place โ€” so turning + // one off here silences that lead time on both channels rather than only in + // the browser. + meetingReminder1Day: true, + meetingReminder1Hour: true, + meetingReminder15Min: true, + meetingReminder1Min: true, }; export type PushEventType = keyof Omit; diff --git a/supabase/migrations/20260819140000_meeting_reminders.sql b/supabase/migrations/20260819140000_meeting_reminders.sql new file mode 100644 index 00000000..bcea2f24 --- /dev/null +++ b/supabase/migrations/20260819140000_meeting_reminders.sql @@ -0,0 +1,115 @@ +-- Meeting reminders: the ledger that makes a reminder send at most once. +-- +-- Reminders go out 1 day, 1 hour, 15 minutes and 1 minute before a meeting, by +-- email and by web push. A cron ticks every minute and asks "what is due now", +-- which means the same reminder is a candidate on several consecutive ticks if +-- anything is slow, retried, or running twice. This table is what stops it +-- being sent twice: a row is claimed *before* the message goes out, and the +-- unique constraint below is the claim. +-- +-- At-most-once rather than at-least-once, deliberately. A crash between the +-- claim and the send loses that one reminder; the alternative loses nothing but +-- can mail somebody the same reminder repeatedly, and for an unsolicited +-- notification about a meeting that is already in the recipient's calendar, +-- silence is the better failure. + +-- ============================================================================= +-- The ledger +-- ============================================================================= + +CREATE TABLE public.meeting_reminders ( + id UUID DEFAULT gen_random_uuid() PRIMARY KEY, + scheduled_session_id UUID NOT NULL + REFERENCES public.scheduled_sessions(id) ON DELETE CASCADE, + + -- Which occurrence this reminder was for, as the exact start instant. + -- + -- This column is the reason the table works for recurring meetings, and it is + -- easy to leave out. 20260819120000 made a recurring meeting *one row* whose + -- `scheduled_at` is the next occurrence, rolled forward by the app once an + -- occurrence has finished. So a key of (session, lead) would fire a weekly + -- meeting's "1 day before" exactly once, in its first week, and stay silent + -- for ever after -- with nothing to show for it, because the ledger would + -- look correctly filled in. Keying on the instant means every roll-forward + -- opens a fresh set of slots. + occurrence_at TIMESTAMPTZ NOT NULL, + + -- 1440, 60, 15 or 1. Stored as minutes rather than an enum so the set can be + -- widened without a type migration; the check keeps today's four honest. + lead_minutes INTEGER NOT NULL CHECK (lead_minutes IN (1440, 60, 15, 1)), + + -- Who it went to. A host is an account; an invitee is an email address on + -- `scheduled_session_invitees` that may belong to nobody at all, so the two + -- cannot share a key space and are not both foreign keys. + recipient_kind TEXT NOT NULL CHECK (recipient_kind IN ('host', 'invitee')), + recipient_key TEXT NOT NULL, + + -- Email and push are claimed separately: a host who has push disabled should + -- still get the mail, and a push that fails should not consume the mail's + -- slot. + channel TEXT NOT NULL CHECK (channel IN ('email', 'push')), + + sent_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + + -- The claim. Everything above the timestamp identifies one message to one + -- person about one occurrence, and there is only ever one of those. + UNIQUE (scheduled_session_id, occurrence_at, lead_minutes, recipient_kind, recipient_key, channel) +); + +-- Answering "has this already gone out" for a whole occurrence in one indexed +-- read, which is what the runner asks once per due meeting. +CREATE INDEX idx_meeting_reminders_occurrence + ON public.meeting_reminders (scheduled_session_id, occurrence_at); + +-- For pruning. The ledger is append-only and grows with every meeting, so old +-- rows are deleted once the meeting they describe is long past -- see the +-- cleanup at the bottom. +CREATE INDEX idx_meeting_reminders_sent_at + ON public.meeting_reminders (sent_at); + +-- ============================================================================= +-- Access +-- ============================================================================= + +ALTER TABLE public.meeting_reminders ENABLE ROW LEVEL SECURITY; + +-- No policy for anon or authenticated, and that is the intent rather than an +-- omission: this table is written only by the reminder runner, which uses the +-- service role and bypasses RLS. Nothing in the app reads it, so leaving it +-- with RLS on and no policies means a leaked anon key cannot enumerate who was +-- invited to what and when they were told. + +-- Hosts may read their own meetings' reminder history, so a "we emailed you at +-- 09:00" line on the meeting page has something true to say. +CREATE POLICY "Host reads own meeting reminders" + ON public.meeting_reminders FOR SELECT + TO authenticated + USING ( + EXISTS ( + SELECT 1 FROM public.scheduled_sessions ss + WHERE ss.id = meeting_reminders.scheduled_session_id + AND ss.host_user_id = auth.uid() + ) + ); + +-- ============================================================================= +-- Pruning +-- ============================================================================= + +-- A ledger row is only useful while its occurrence could still be re-sent by a +-- late tick. A fortnight is far beyond any grace window and keeps the table +-- small without anybody having to think about it again. +CREATE OR REPLACE FUNCTION public.prune_meeting_reminders() +RETURNS INTEGER AS $$ +DECLARE + v_deleted INTEGER; +BEGIN + DELETE FROM public.meeting_reminders + WHERE sent_at < NOW() - INTERVAL '14 days'; + GET DIAGNOSTICS v_deleted = ROW_COUNT; + RETURN v_deleted; +END; +$$ LANGUAGE plpgsql SECURITY DEFINER; + +COMMENT ON TABLE public.meeting_reminders IS + 'One row per reminder actually sent. The unique constraint is a claim taken before sending, which is what makes a per-minute cron safe to run twice.'; From e8dfae9052b2a017737b7f99cca554e3863ebf0b Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Wed, 19 Aug 2026 10:51:42 +0000 Subject: [PATCH 2/5] fix(test): let each workspace run its own tests 91 of 174 test files failed on master, 282 tests with them, on `document is not defined`, `localStorage is not defined` and `Cannot find package '@/lib/...'`. None of it was real. The root vitest.config.ts declared `environment: 'node'` and its own repository-wide `include`, which silently overrode the vitest.config.ts that apps/web, apps/mobile, apps/desktop, packages/shared-types and packages/ai-core each already had. apps/web's has specified jsdom, the `@/` alias and a setup file all along: the sixteen settings tests that fail from the root pass through it without touching a line of test code. `projects` lets each workspace keep its environment, its aliases and its setup. That is not a tidiness argument -- this repo has three different `@/` aliases (apps/web/src, apps/mobile/src, apps/desktop/src/renderer), so no single root-level alias could ever have served them. before 91 failed | 83 passed (174 files), 282 failed | 855 passed after 174 passed (174 files), 1780 passed The test count rises by 643 because most of those files were failing during collection, so their tests had never run at all and were never counted. Two workspaces needed a config of their own first, and both were places where this change could have quietly reduced coverage instead of restoring it: - packages/remote-input has 12 test files and no config. It needs jsdom rather than node, because it maps browser key and pointer events. - scripts/ has 3 test files and is not a workspace, so a projects list of apps/* and packages/* would have dropped them -- and reported 171 files, all passing, which is exactly the shape of a problem nobody notices. That is why the list is written out rather than globbed, and why the check was the file count and not the pass count. Co-Authored-By: Claude Opus 5 (1M context) --- packages/remote-input/vitest.config.ts | 13 ++++++ scripts/vitest.config.ts | 14 ++++-- vitest.config.ts | 61 ++++++++++++++++++-------- 3 files changed, 67 insertions(+), 21 deletions(-) create mode 100644 packages/remote-input/vitest.config.ts diff --git a/packages/remote-input/vitest.config.ts b/packages/remote-input/vitest.config.ts new file mode 100644 index 00000000..d54f9d01 --- /dev/null +++ b/packages/remote-input/vitest.config.ts @@ -0,0 +1,13 @@ +import { defineConfig } from 'vitest/config'; + +export default defineConfig({ + test: { + globals: true, + // jsdom rather than node, unlike the other packages: this one maps browser + // key and pointer events, so most of what it tests only exists in a + // document. Its tests were previously run by the root config -- against the + // node environment -- and every one that touched `window` failed. + environment: 'jsdom', + include: ['src/**/*.test.ts'], + }, +}); diff --git a/scripts/vitest.config.ts b/scripts/vitest.config.ts index de38fd3c..3a1cece9 100644 --- a/scripts/vitest.config.ts +++ b/scripts/vitest.config.ts @@ -1,12 +1,20 @@ import { defineConfig } from 'vitest/config'; -import { resolve } from 'path'; +/** + * The release scripts have tests too, and they are easy to lose. + * + * `scripts/` is not a pnpm workspace, so it has no config of its own and was + * only ever swept up by the root runner's repository-wide glob. Moving the root + * to per-workspace projects would have dropped these three files silently -- + * the suite would simply have reported 171 files instead of 174 and still said + * everything passed. + */ export default defineConfig({ test: { globals: true, + // Plain Node tooling: version bumping, package managers, AUR submission. environment: 'node', - root: resolve(__dirname), include: ['**/*.test.ts'], - testTimeout: 10000, + exclude: ['**/node_modules/**'], }, }); diff --git a/vitest.config.ts b/vitest.config.ts index eea9a685..76d4dac7 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,12 +1,51 @@ import { defineConfig } from 'vitest/config'; +/** + * The root test runner, which delegates rather than decides. + * + * It used to declare `environment: 'node'` and its own `include` glob covering + * the whole repository, which quietly overrode the `vitest.config.ts` that each + * app and package already had. The effect was that `pnpm vitest run` from the + * root collected all 174 test files and then ran every one of them in the wrong + * environment, with no `@/` alias and no setup file: 91 files failed, 282 tests + * with them, on `document is not defined`, `localStorage is not defined` and + * `Cannot find package '@/lib/...'`. + * + * None of those were real. `apps/web/vitest.config.ts` has specified jsdom, the + * alias and a setup file all along -- running the same suite through it turns + * 16 failures into 16 passes without touching a line of test code. The tests + * were fine; the runner above them was not, and because the failures looked + * like ordinary broken tests the suite had stopped being able to tell anybody + * whether a change had broken something. + * + * `projects` is the fix: each workspace keeps its own environment, its own + * aliases and its own setup, and this file only says where they are. The three + * `@/` aliases in this repo point at three different directories + * (`apps/web/src`, `apps/mobile/src`, `apps/desktop/src/renderer`), so no single + * root-level alias could ever have served all of them anyway. + * + * Every workspace listed here has tests. `apps/livekit`, `apps/turn` and + * `apps/installer` are absent because they have none; they can be added the + * moment they do, alongside a config of their own. + */ export default defineConfig({ test: { - // Global test settings - globals: true, - environment: 'node', + projects: [ + 'apps/web', + 'apps/mobile', + 'apps/desktop', + 'packages/shared-types', + 'packages/ai-core', + 'packages/remote-input', + // Not a workspace, and the reason this list is written out rather than + // globbed: `scripts/` holds three test files that only the old + // repository-wide glob was picking up, and a projects list of `apps/*` + // and `packages/*` would have dropped them without saying so. + 'scripts', + ], - // Coverage configuration + // Coverage stays here, because it is the one thing that is genuinely about + // the repository as a whole rather than about any single workspace. coverage: { provider: 'v8', reporter: ['text', 'json', 'html', 'lcov'], @@ -26,19 +65,5 @@ export default defineConfig({ statements: 70, }, }, - - // Test file patterns - include: ['**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'], - exclude: ['**/node_modules/**', '**/dist/**', '**/.next/**'], - - // Timeout - testTimeout: 10000, - - // Reporter - reporters: ['default'], - - // Watch mode settings - watch: true, - watchExclude: ['**/node_modules/**', '**/dist/**', '**/.next/**'], }, }); From 529dd4fa0d4a11323dd1e337e1a6d1663d86d5d3 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Wed, 19 Aug 2026 10:53:24 +0000 Subject: [PATCH 3/5] fix(test): put back the scripts config I overwrote scripts/vitest.config.ts already existed, for running vitest from inside scripts/. The previous commit replaced it wholesale and dropped its explicit root and its 10s testTimeout along the way. Both are back. The only difference from the original is now the comment explaining why the root config has to name this directory: it is not a pnpm workspace, so a projects list of apps/* and packages/* skips it entirely. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/vitest.config.ts | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/scripts/vitest.config.ts b/scripts/vitest.config.ts index 3a1cece9..936f7031 100644 --- a/scripts/vitest.config.ts +++ b/scripts/vitest.config.ts @@ -1,20 +1,24 @@ import { defineConfig } from 'vitest/config'; +import { resolve } from 'path'; /** - * The release scripts have tests too, and they are easy to lose. + * The release scripts have tests, and they are easy to lose. * - * `scripts/` is not a pnpm workspace, so it has no config of its own and was - * only ever swept up by the root runner's repository-wide glob. Moving the root - * to per-workspace projects would have dropped these three files silently -- - * the suite would simply have reported 171 files instead of 174 and still said - * everything passed. + * This config already existed, for running `vitest` from inside `scripts/`. + * What it was not was *reachable from the root* -- `scripts/` is not a pnpm + * workspace, so a projects list of `apps/*` and `packages/*` skips it, and the + * three test files here would have gone from being swept up by the old + * repository-wide glob to not running at all. The suite would have reported 171 + * files, every one passing, which is the shape of a problem nobody notices. + * + * Hence the explicit entry in the root config's `projects`. */ export default defineConfig({ test: { globals: true, - // Plain Node tooling: version bumping, package managers, AUR submission. environment: 'node', + root: resolve(__dirname), include: ['**/*.test.ts'], - exclude: ['**/node_modules/**'], + testTimeout: 10000, }, }); From a866e261bc809cf01809899c3ba28b6930cf9a87 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Wed, 19 Aug 2026 10:58:45 +0000 Subject: [PATCH 4/5] fix(lint): satisfy the type rules this repo actually enforces CI's Lint job failed on nine errors in the new files. This repo's eslint config is stricter than the typecheck: it rejects `as` casts that widen, rejects `!` assertions outright, and rejects reading fields off PostgREST's `any` rows straight into a template or an object literal. - `dueLead` now iterates `LEAD_MINUTES.entries()`, which hands back the element already typed. Indexing needed either a cast or a `!` to convince the compiler the element exists, and both are forbidden here. - `admin()` returns an inferred type. Annotating it `SupabaseClient` was the unsafe-return error: the bare type defaults its schema parameters differently from what `createClient` actually returns. - The host's profile row is narrowed once, into a named `ProfileRow`, rather than at each use. - Dropped a redundant `Number()`, an `?? []` on a value already narrowed non-null, and one level of optional chaining that could not be nullish. No behaviour changes. Typecheck clean, 174 files and 1780 tests still passing. Co-Authored-By: Claude Opus 5 (1M context) --- apps/web/src/lib/meeting-reminders-runner.ts | 34 ++++++++++++++------ apps/web/src/lib/meeting-reminders.ts | 14 ++++---- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/apps/web/src/lib/meeting-reminders-runner.ts b/apps/web/src/lib/meeting-reminders-runner.ts index c774dbeb..4151b964 100644 --- a/apps/web/src/lib/meeting-reminders-runner.ts +++ b/apps/web/src/lib/meeting-reminders-runner.ts @@ -1,4 +1,4 @@ -import { createClient, type SupabaseClient } from '@supabase/supabase-js'; +import { createClient } from '@supabase/supabase-js'; import { createEmailer } from '@profullstack/emailer'; import { dueLead, timeUntil, wantsReminder, REMINDER_PREF_KEYS } from './meeting-reminders'; @@ -45,13 +45,23 @@ interface InviteeRow { rsvp_status: string; } -function admin(): SupabaseClient { +interface ProfileRow { + settings: unknown; + display_name: string | null; +} + +// Inferred rather than annotated `SupabaseClient`: the bare type defaults its +// schema parameters differently from what `createClient` returns, so naming it +// turns a perfectly ordinary return into an unsafe-return lint error. +function admin() { const url = process.env.NEXT_PUBLIC_SUPABASE_URL; const key = process.env.SUPABASE_SERVICE_ROLE_KEY; if (!url || !key) throw new Error('Missing Supabase environment variables'); return createClient(url, key, { auth: { autoRefreshToken: false, persistSession: false } }); } +type Admin = ReturnType; + /** * Take the slot for one message, returning false if somebody already had it. * @@ -64,7 +74,7 @@ function admin(): SupabaseClient { * loses that reminder for good. That is the intended trade: see the migration. */ async function claim( - db: SupabaseClient, + db: Admin, row: { scheduled_session_id: string; occurrence_at: string; @@ -90,7 +100,7 @@ async function claim( * indexed read covers. */ async function alreadySent( - db: SupabaseClient, + db: Admin, sessionId: string, occurrenceAt: string ): Promise>> { @@ -105,7 +115,7 @@ async function alreadySent( const r = row as { recipient_kind: string; recipient_key: string; channel: string; lead_minutes: number }; const key = `${r.recipient_kind}:${r.recipient_key}:${r.channel}`; const set = byRecipient.get(key) ?? new Set(); - set.add(Number(r.lead_minutes)); + set.add(r.lead_minutes); byRecipient.set(key, set); } return byRecipient; @@ -176,7 +186,7 @@ export async function runMeetingReminders(now: Date = new Date()): Promise) + // Narrowed once, here, rather than at each use. PostgREST types this row + // as `any`, so reading fields off it straight into a template or an object + // literal is an unsafe assignment; naming the shape is what makes the two + // values below ordinary strings. + const hostProfile = hostSettings.data as ProfileRow | null; + const notifications = ((hostProfile?.settings ?? {}) as Record) .notifications as Record | undefined; + const hostDisplayName = hostProfile?.display_name ?? null; const hostEmailLead = dueLead( startsAt, @@ -209,7 +225,7 @@ export async function runMeetingReminders(now: Date = new Date()): Promise floor) { return alreadySent.has(lead) ? null : lead; @@ -110,13 +112,13 @@ export function timeUntil(startsAt: Date, now: Date): string { const minutes = Math.round((startsAt.getTime() - now.getTime()) / 60_000); if (minutes <= 1) return 'in about a minute'; - if (minutes < 60) return `in ${minutes} minutes`; + if (minutes < 60) return `in ${String(minutes)} minutes`; const hours = Math.round(minutes / 60); - if (hours < 24) return hours === 1 ? 'in about an hour' : `in about ${hours} hours`; + if (hours < 24) return hours === 1 ? 'in about an hour' : `in about ${String(hours)} hours`; const days = Math.round(hours / 24); - return days === 1 ? 'tomorrow' : `in ${days} days`; + return days === 1 ? 'tomorrow' : `in ${String(days)} days`; } /** From 996e80885f7dea2e1e7c600e68a1c1a6e0af3b2b Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Wed, 19 Aug 2026 11:05:54 +0000 Subject: [PATCH 5/5] fix(lint): the test files are linted too I linted the new sources and not the new tests, so a number in a template literal survived into CI. Same rule, same fix. Co-Authored-By: Claude Opus 5 (1M context) --- apps/web/src/lib/meeting-reminders.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/web/src/lib/meeting-reminders.test.ts b/apps/web/src/lib/meeting-reminders.test.ts index dc073c3e..2c7d509c 100644 --- a/apps/web/src/lib/meeting-reminders.test.ts +++ b/apps/web/src/lib/meeting-reminders.test.ts @@ -43,7 +43,7 @@ describe('dueLead', () => { // answer at each. A gap would drop a reminder; an overlap would send two. for (let m = 1440; m >= 1; m -= 1) { const lead = dueLead(START, before(m)); - expect(lead, `at ${m} minutes out`).not.toBeNull(); + expect(lead, `at ${String(m)} minutes out`).not.toBeNull(); expect(LEAD_MINUTES).toContain(lead); } });