feat(web): remind people about a meeting before it starts - #82
Merged
Conversation
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) <noreply@anthropic.com>
ThreatCrush Security Scan66 finding(s) HIGH/CRITICAL: 13 | MEDIUM: 35 | LOW: 18
…and 16 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 24h down to 1h, the hour-before covers 60 to 15 minutes, and so on.
The obvious implementation (fire when
nowis within a minute or two ofstart - lead) fails in both directions at once: miss the window to a deploy or a slow tick and the reminder is gone with nothing to show it, but 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 down for three hours comes back and sends the tightest reminder still true, skipping the ones it slept through — the right fate for a reminder about something that has since drawn much closer. A test walks every minute of the day before a meeting and asserts exactly one lead answers at each, so there is no gap to drop a reminder and no overlap to send two.
The copy uses the real remaining time, not the band's name, so a meeting booked 25 minutes ahead is never told it starts "in 1 hour".
Sending each one exactly once
meeting_remindersis 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 whosescheduled_atis the next occurrence, rolled forward after each finishes. A key of(session, lead)would fire a weekly meeting's day-before notice once, in its first week, then 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/runwith 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_SECRETthe 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. The secret is compared in constant time, with an explicit length check first so a wrong guess is a 401 rather than a 500 that leaks the length.Deploying — three steps, and this PR is only the first
REMINDERS_CRON_SECRETon the Railway web service.pg_cron+pg_netand schedule the minute job.The migration is already applied to production as
20260819103845_meeting_reminders— pairux has no CI that applies migrations, andsupabase db pushis not usable here: the repo has nosupabase/config.toml, is not linked, and every repo filename differs from its recorded prod version (20260819120000_recurring_scheduled_sessions.sqlis version20260819100400in prod), so a push would try to replay all 46 migrations.Testing
18 new tests, all passing: 11 on the due-time logic, 7 on the endpoint's refusals. Typecheck clean across
apps/web.Note on the suite: 91 of 174 test files fail on
masterbefore this change —localStorage is not defined,no testscollection errors — because the root vitest config runs the node environment. Verifiedmiddleware.test.tsandsettings/page.test.tsxfail identically on an untouched checkout. Not introduced here, but it means the suite cannot tell you whether this broke anything, which is worth fixing separately.🤖 Generated with Claude Code