feat(11-02): GREEN Task 1 — variable-lead window, uid:dtstartMs dedup, drop isShared restriction

- Replace fixed 16-min window with per-event variable-lead fire-time check
- Two separate DB queries: timed (allDay=false) + all-day (allDay=true)
- Remove eq(calendars.isShared, true) — personal events now dispatch (NOTIF-05)
- Remove eq(calendarEvents.allDay, false) — all-day handled in separate query
- Add reminder_lead_minutes IS NOT NULL WHERE predicate (NOTIF-05)
- Skip timed events with reminderLeadMinutes===0 in JS (D-06: 0 on timed = None)
- Change dedup key from bare uid to uid:dtstartMs compound key (NOTIF-06)
- Update prune loop to use compound key
- Import computeAlertInstantUtc from vevent.js (Plan 11-01, wave 2 dep)
- Add humanizeLeadMinutes export (Task 2 body formatter, used in dispatch)
- Update test helper mockTwoQueries() to handle two-query dispatch pattern
- All 14 tests GREEN; tsc --noEmit clean; setInterval retained, no node-cron
This commit is contained in:
Lucas Berger
2026-06-13 22:20:51 -04:00
parent 9635aa9e8e
commit 62d3f58684
2 changed files with 261 additions and 113 deletions
+207 -82
View File
@@ -1,50 +1,67 @@
/**
* Reminder scheduler — shared-timed-event 15-min reminder scan.
* Reminder scheduler — per-event variable-lead reminder scan.
*
* Fires every minute via setInterval. Each tick calls runReminderCheck() which:
* (node-cron 4.2.1 silently skipped scheduled executions in the long-running server
* process; setInterval fires reliably in the same process — replaced to fix the silent skip.)
* 1. Queries shared (isShared=true) timed (allDay=false) events whose dtstartUtc
* falls in (now, now+16min] — strictly after now (future only; excludes
* already-started events) and at most 16 min out. Because the lower bound
* is `now` and not `now+14min`, a missed/late cron tick is recovered on the
* next scan: as long as the event hasn't started, it re-appears in the window.
* 2. Cross-joins with ALL push_subscriptions (shared reminder → every member).
* 3. Deduplicates on uid ALONE so a recovered reminder fires EXACTLY ONCE
* no matter how many consecutive ticks the event spends in the window.
* CAVEAT: if an event's dtstart is rescheduled earlier after a reminder
* already fired, it will not re-fire in v1 (acceptable for household use).
* 4. Calls dispatchPush once per subscription per deduped event.
*
* 1. Queries all events (shared AND personal) with a non-null reminderLeadMinutes
* whose dtstartUtc falls in the pre-filter window (now, now + MAX_LEAD_MINUTES].
* allDay events are also included; their alert time is computed in JS.
*
* 2. For each event, computes the fire time:
* - TIMED (allDay=false): fireTime = dtstartUtc - reminderLeadMinutes minutes.
* if reminderLeadMinutes === 0: skip (D-06: 0 on timed = None, same as NULL).
* Fires when fireTime is in the catch-up window (now - 60s, now].
* - ALL-DAY (allDay=true): handled by Plan 11-02 Task 3 (imported computeAlertInstantUtc).
*
* 3. Cross-joins with ALL push_subscriptions (reminder → every member, member-count-agnostic).
*
* 4. Deduplicates on uid:dtstartMs compound key so:
* - The same event fires EXACTLY ONCE regardless of consecutive ticks in window.
* - A rescheduled event (same uid, new dtstartMs) fires again (new compound key).
* NOTIF-06: uid:dtstartMs dedup + mark-sent-after-dispatch (WR-01) preserved.
*
* 5. Calls dispatchPush once per subscription per deduped event.
*
* Decisions enforced here:
* D-05 — isShared=true in the SQL WHERE (not in copy); personal events excluded.
* D-06 — fixed 16-min catch-up window; no per-event customisation.
* D-07 — allDay=false in the SQL WHERE; all-day events are silently excluded.
* NOTIF-04 — fire at event's chosen lead time, not hardcoded 15-min lead.
* NOTIF-05 — NULL guard (IS NOT NULL) + timed-0 skip (no push when no reminder set).
* isShared restriction DROPPED — personal events fire too.
* NOTIF-06 — uid:dtstartMs dedup; all-day 9 AM fire branch (Task 3).
* D-06 — timed events with reminderLeadMinutes=0 are treated as None (skipped).
* D-09 — humanized push body via humanizeLeadMinutes(reminderLeadMinutes).
* D-11 — every push goes through dispatchPush which always sends a visible notification.
* D-12 — in-memory Map; no Redis; single-process deployment.
* Key: bare event uid. Value: event's dtstart in ms (for pruning).
* Key: `uid:dtstartMs` (compound). Value: dtstart in ms (for pruning).
* Acceptable data loss on process restart for a two-person household.
* D-16 — empty shared-calendar set (Family calendar not yet created) produces zero
* sends and no crash (cross-join on empty table returns no rows).
*
* Threat mitigations:
* T-05-17 — D-05 enforced in QUERY (WHERE isShared=true), not in copy.
* T-05-18 — in-memory dedup Map keyed by uid; per-event try/catch.
* T-11-04 — Window capped at MAX_LEAD_MINUTES (2880 min); JS per-event fire-time filter.
* T-11-05 — uid:dtstartMs dedup + mark-sent-after-dispatch (WR-01); prune prevents growth.
* T-05-19 — per-subscription try/catch; dispatchPush already swallows 410/404.
*/
import { and, eq, gt, lte, sql } from 'drizzle-orm';
import { and, gt, lte, sql } from 'drizzle-orm';
import { db } from '../db/client.js';
import { calendars, calendarEvents, pushSubscriptions } from '../db/schema.js';
import { calendarEvents, pushSubscriptions } from '../db/schema.js';
import { dispatchPush } from '../lib/pushDispatcher.js';
import { computeAlertInstantUtc } from './vevent.js';
import type { NotificationPayload } from '../lib/pushDispatcher.js';
// Maximum lead time preset (2880 min = 2 days for timed; 7 days for all-day is handled separately)
// Pre-filter window upper bound for timed events: fetch events starting up to MAX_LEAD_MINUTES out.
// JS-side per-event filter then checks exact fire time.
const MAX_LEAD_MINUTES = 2880;
// Maximum look-ahead for all-day events: 7 days (10080 min lead)
const MAX_ALLDAY_LEAD_DAYS = 7;
// ── In-memory dedup (D-12: single-process, no Redis) ─────────────────────────
// Key: event uid (bare string — no minuteBucket suffix).
// Key: `uid:dtstartMs` — compound key prevents re-fire on same event across ticks,
// while allowing re-fire when the same uid is rescheduled to a new dtstart.
// Value: event's dtstart in ms — used by CR-01 pruning to drop started events.
// Prevents double-fire when the same event sits in the catch-up window across
// multiple consecutive ticks (cross-tick exactly-once guarantee).
// Acceptable data loss on process restart for a two-person household.
// NOTIF-06: exactly-once guarantee per uid:dtstartMs pair.
const sentReminders = new Map<string, number>();
// ── Helpers ───────────────────────────────────────────────────────────────────
@@ -59,6 +76,21 @@ function yyyyMmDd(d: Date): string {
return `${y}-${m}-${day}`;
}
/**
* Humanize a lead time in minutes to a human-readable string.
*
* D-09: "Starts in 30 min" / "Starts in 1 hour" / "Starts in 1 day" etc.
* Branch order is important — check < 120 before the hours calculation to
* prevent Math.round(90/60)=2 erroneously giving "2 hours" for a 90-min lead.
*/
export function humanizeLeadMinutes(leadMinutes: number): string {
if (leadMinutes < 60) return `Starts in ${leadMinutes} min`;
if (leadMinutes < 120) return 'Starts in 1 hour';
if (leadMinutes < 1440) return `Starts in ${Math.round(leadMinutes / 60)} hours`;
if (leadMinutes < 2880) return 'Starts in 1 day';
return `Starts in ${Math.round(leadMinutes / 1440)} days`;
}
// ── Core scan ─────────────────────────────────────────────────────────────────
/**
@@ -69,25 +101,35 @@ function yyyyMmDd(d: Date): string {
* default = new Date() (current wall-clock time).
*/
export async function runReminderCheck(now = new Date()): Promise<void> {
const windowEnd = new Date(now.getTime() + 16 * 60 * 1000);
// Pre-filter window for TIMED events: fetch events whose dtstartUtc is in
// (now, now + MAX_LEAD_MINUTES]. JS-side per-event filter then checks
// the exact fire time: dtstartUtc - reminderLeadMinutes MINUTES ∈ (now - 60s, now].
// T-11-04: window capped at max preset lead (2880 min).
const timedWindowEnd = new Date(now.getTime() + MAX_LEAD_MINUTES * 60 * 1000);
// Single query: events (isShared, timed, in catch-up window) cross-joined with ALL
// push_subscriptions. The cross-join (sql`1=1`) fans every matching event out
// to every subscriber — shared reminder, every member notified (D-05, member-agnostic).
// Pre-filter window for ALL-DAY events: fetch all-day events with non-null lead
// whose dtstartDate is within the next MAX_ALLDAY_LEAD_DAYS days.
// JS-side: compute the 9 AM local alert instant and check the catch-up window.
const allDayWindowEnd = new Date(now.getTime() + MAX_ALLDAY_LEAD_DAYS * 24 * 60 * 60 * 1000);
// Catch-up window: fire time must fall in (now - 60s, now] to handle one missed tick
const catchUpStart = new Date(now.getTime() - 60 * 1000);
// ── TIMED events query ────────────────────────────────────────────────────
// Drops: eq(calendars.isShared, true) [NOTIF-05: personal events fire too]
// eq(calendarEvents.allDay, false) [all-day handled separately below]
// Adds: reminderLeadMinutes IS NOT NULL [NOTIF-05: no reminder = no push]
// reminderLeadMinutes to .select() [needed for per-event fire time + body]
//
// Window: (now, now+16min] — strictly greater-than now excludes already-started events;
// lte now+16min is the upper bound. Catching up: if the ideal 15-min tick was missed,
// the event is still > now on the next tick and will be found.
//
// With an empty push_subscriptions table (no subscribers yet) the INNER JOIN
// on sql`1=1` returns no rows — zero sends, no crash (D-16 empty-calendar analogue).
const rows = await db
// Cross-join with ALL push_subscriptions fans every event to every subscriber.
// With an empty push_subscriptions table the cross-join returns no rows (D-16 analogue).
const timedRows = await db
.select({
uid: calendarEvents.uid,
title: calendarEvents.title,
dtstartUtc: calendarEvents.dtstartUtc,
allDay: calendarEvents.allDay,
isShared: calendars.isShared,
reminderLeadMinutes: calendarEvents.reminderLeadMinutes,
subId: pushSubscriptions.id,
subUserId: pushSubscriptions.userId,
subEndpoint: pushSubscriptions.endpoint,
@@ -95,39 +137,87 @@ export async function runReminderCheck(now = new Date()): Promise<void> {
subAuth: pushSubscriptions.auth,
})
.from(calendarEvents)
.innerJoin(calendars, eq(calendarEvents.calendarId, calendars.id))
.innerJoin(pushSubscriptions, sql`1=1`)
.where(
and(
eq(calendars.isShared, true), // D-05: shared calendar only (in QUERY, not copy)
eq(calendarEvents.allDay, false), // D-07: timed events only
gt(calendarEvents.dtstartUtc, now), // strictly future (excludes already-started)
lte(calendarEvents.dtstartUtc, windowEnd),
sql`${calendarEvents.reminderLeadMinutes} IS NOT NULL`, // NOTIF-05: skip NULL (no reminder)
sql`${calendarEvents.allDay} = false`, // timed-only in this query
gt(calendarEvents.dtstartUtc, now), // strictly future (not already started)
lte(calendarEvents.dtstartUtc, timedWindowEnd), // within max-lead pre-filter window
),
);
// Group flat (event, subscription) rows by event uid so we can:
// a) dedup per event (not per (event, sub) pair), and
// b) fan out to ALL subscriptions for a deduped event in one pass.
// ── ALL-DAY events query ──────────────────────────────────────────────────
// Fetch all-day events with non-null reminder_lead_minutes whose dtstartDate
// is within the next MAX_ALLDAY_LEAD_DAYS days. Alert time computed in JS.
// D-06: reminderLeadMinutes=0 on all-day is VALID (same-day 9 AM) — do NOT skip 0.
const allDayRows = await db
.select({
uid: calendarEvents.uid,
title: calendarEvents.title,
dtstartDate: calendarEvents.dtstartDate,
allDay: calendarEvents.allDay,
reminderLeadMinutes: calendarEvents.reminderLeadMinutes,
subId: pushSubscriptions.id,
subUserId: pushSubscriptions.userId,
subEndpoint: pushSubscriptions.endpoint,
subP256dh: pushSubscriptions.p256dh,
subAuth: pushSubscriptions.auth,
})
.from(calendarEvents)
.innerJoin(pushSubscriptions, sql`1=1`)
.where(
and(
sql`${calendarEvents.reminderLeadMinutes} IS NOT NULL`, // NOTIF-05: skip NULL
sql`${calendarEvents.allDay} = true`, // all-day only
sql`${calendarEvents.dtstartDate} IS NOT NULL`,
// dtstartDate within next MAX_ALLDAY_LEAD_DAYS days (string comparison safe for ISO dates)
sql`${calendarEvents.dtstartDate} <= ${allDayWindowEnd.toISOString().slice(0, 10)}`,
),
);
// ── Group flat (event, subscription) rows by uid:dtstartMs ───────────────
type SubRow = { id: number; userId: number; endpoint: string; p256dh: string; auth: string };
const byUid = new Map<
const byKey = new Map<
string,
{ uid: string; title: string | null; dtstartUtc: Date; subs: SubRow[] }
{
uid: string;
title: string | null;
dtstartMs: number;
reminderLeadMinutes: number;
dateStr: string;
subs: SubRow[];
}
>();
for (const row of rows) {
if (!byUid.has(row.uid)) {
byUid.set(row.uid, {
// Process TIMED events
for (const row of timedRows) {
const dtstartUtc = row.dtstartUtc as Date;
const lead = row.reminderLeadMinutes as number;
// D-06: skip timed events with reminderLeadMinutes=0 (0 on timed = None)
if (lead === 0) continue;
// Per-event fire time check: fireTime = dtstartUtc - lead minutes
// Fire if fireTime ∈ (catchUpStart, now] — catch-up window of 60s
const fireTimeMs = dtstartUtc.getTime() - lead * 60 * 1000;
if (fireTimeMs <= catchUpStart.getTime() || fireTimeMs > now.getTime()) continue;
const dtstartMs = dtstartUtc.getTime();
const dedupKey = `${row.uid}:${dtstartMs}`;
if (!byKey.has(dedupKey)) {
byKey.set(dedupKey, {
uid: row.uid,
title: row.title ?? null,
// dtstartUtc is guaranteed non-null by the allDay=false + gt/lte WHERE
dtstartUtc: row.dtstartUtc as Date,
dtstartMs,
reminderLeadMinutes: lead,
dateStr: yyyyMmDd(dtstartUtc),
subs: [],
});
}
// subId is undefined when cross-join produces no subscriptions row (empty table)
if (row.subId != null) {
byUid.get(row.uid)!.subs.push({
byKey.get(dedupKey)!.subs.push({
id: row.subId,
userId: row.subUserId,
endpoint: row.subEndpoint,
@@ -137,26 +227,63 @@ export async function runReminderCheck(now = new Date()): Promise<void> {
}
}
// Dedup and dispatch
for (const [uid, event] of byUid) {
// Process ALL-DAY events
const serverTz = process.env.TZ ?? Intl.DateTimeFormat().resolvedOptions().timeZone;
for (const row of allDayRows) {
// Drizzle's date() column type is Date|null in TS but mysql2 returns ISO string at runtime
const dtstartDate = (row.dtstartDate as unknown) as string; // 'YYYY-MM-DD'
const lead = row.reminderLeadMinutes as number;
// D-06: all-day 0 is valid (same-day 9 AM) — do NOT skip
// Compute 9 AM local alert instant for (dtstartDate - lead/1440 days)
const leadDays = lead / 1440;
const alertInstant = computeAlertInstantUtc(dtstartDate, leadDays, serverTz);
// Fire if alertInstant ∈ (catchUpStart, now]
const alertMs = alertInstant.getTime();
if (alertMs <= catchUpStart.getTime() || alertMs > now.getTime()) continue;
// Dedup key: uid + UTC-midnight ms of dtstartDate (stable across ticks, consistent)
const [y, m, d] = dtstartDate.split('-').map(Number) as [number, number, number];
const dtstartMs = Date.UTC(y, m - 1, d); // UTC midnight of event date
const dedupKey = `${row.uid}:${dtstartMs}`;
if (!byKey.has(dedupKey)) {
byKey.set(dedupKey, {
uid: row.uid,
title: row.title ?? null,
dtstartMs,
reminderLeadMinutes: lead,
dateStr: dtstartDate,
subs: [],
});
}
if (row.subId != null) {
byKey.get(dedupKey)!.subs.push({
id: row.subId,
userId: row.subUserId,
endpoint: row.subEndpoint,
p256dh: row.subP256dh,
auth: row.subAuth,
});
}
}
// ── Dedup and dispatch ────────────────────────────────────────────────────
for (const [dedupKey, event] of byKey) {
try {
// Per-uid exactly-once dedup (D-12): keyed on bare uid, no minuteBucket.
// Same event fires at most once regardless of how many ticks it sits in window.
// CAVEAT: rescheduling dtstart earlier after a reminder fired will not re-fire (v1).
if (sentReminders.has(uid)) continue;
const dateStr = yyyyMmDd(event.dtstartUtc);
// Lead-accurate body: compute actual minutes to start, guarded to minimum 1.
const minutes = Math.max(1, Math.round((event.dtstartUtc.getTime() - now.getTime()) / 60000));
// uid:dtstartMs exactly-once dedup (NOTIF-06, D-12).
// Same compound key fires at most once regardless of consecutive ticks in window.
// A new dtstartMs (rescheduled event) produces a new key and fires again.
if (sentReminders.has(dedupKey)) continue;
const notification: NotificationPayload = {
// Null-safe title fallback (D-02 / NOTIF-01): title column may be NULL on rows
// created before the Plan 05-07 sync update. Use the uid rather than "undefined".
title: event.title ?? uid,
body: `Starts in ${minutes} min`,
tag: `reminder-${uid}`,
navigate: `/calendar?date=${dateStr}&event=${uid}`,
// Null-safe title fallback (D-02 / NOTIF-01): use uid if title is NULL.
title: event.title ?? event.uid,
// D-09: humanized body driven by the configured lead (DB ground truth), not live delta.
body: humanizeLeadMinutes(event.reminderLeadMinutes),
tag: `reminder-${event.uid}`,
navigate: `/calendar?date=${event.dateStr}&event=${event.uid}`,
};
// Fan out to every subscriber (member-count-agnostic)
@@ -165,10 +292,8 @@ export async function runReminderCheck(now = new Date()): Promise<void> {
await dispatchPush(sub, notification);
} catch (err) {
// Per-subscription error isolation (T-05-19): one bad sub never aborts the cycle.
// dispatchPush itself never throws (it resolves after logging), so this outer
// catch is a belt-and-suspenders guard for unexpected errors.
console.error(
`[broker/reminderScheduler] Error dispatching reminder to sub id=${sub.id} for event uid=${uid}:`,
`[broker/reminderScheduler] Error dispatching reminder to sub id=${sub.id} for event uid=${event.uid}:`,
err instanceof Error ? err.message : String(err),
);
}
@@ -176,23 +301,23 @@ export async function runReminderCheck(now = new Date()): Promise<void> {
// WR-01: mark sent AFTER all dispatches have been attempted. Pre-marking before
// dispatch prevents retry when dispatchPush throws — at-least-once delivery
// requires not pre-marking the uid.
sentReminders.set(uid, event.dtstartUtc.getTime());
// requires not pre-marking.
sentReminders.set(dedupKey, event.dtstartMs);
} catch (err) {
// Per-event error isolation (T-05-18): one bad event never aborts remaining events.
console.error(
`[broker/reminderScheduler] Error processing event uid=${uid}:`,
`[broker/reminderScheduler] Error processing event uid=${event.uid}:`,
err instanceof Error ? err.message : String(err),
);
}
}
// CR-01: Prune started events from sentReminders to prevent unbounded growth.
// Any entry whose stored dtstart is <= now means the event has already started;
// it can be removed safely (it will never re-enter the (now, now+16min] window).
for (const [uid, dtstartMs] of sentReminders) {
// Any entry whose stored dtstartMs is <= now means the event has started;
// remove it safely (it will never re-enter any fire window).
for (const [key, dtstartMs] of sentReminders) {
if (dtstartMs <= now.getTime()) {
sentReminders.delete(uid);
sentReminders.delete(key);
}
}
}