luckberg/familysync
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs: complete v1.2 research (stack, features, architecture, pitfalls)

Browse Source 
- STACK.md: google-auth-library@10.7.0 + @googleapis/calendar@15.0.0 scoped packages (vs monolithic googleapis), OAuth2 flow, token storage, Google Calendar API event/reminder model
- FEATURES.md: 6 feature categories (multi-provider, self-service onboarding, multiple reminders, dark mode, zero-setup DB, dev/CI stub), dependency graph, feature prioritization
- ARCHITECTURE.md: CalendarProvider interface, provider factory, CalDavProvider wrapper, GoogleCalendarProvider, MockProvider, provider_tokens table schema, multi-reminder JSON column, OAuth callback routing, 7-component data flows
- PITFALLS.md: 11 critical/medium pitfalls (refresh token 7-day expiry in testing status, Google recurrence mismatch, syncToken 410, timezone handling, provider abstraction regression, VALARM dedup key, auto-migrate failures, dark mode FOWT, OAuth callback through tunnel, token encryption, ESLint 10 breaking changes)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Lucas Berger
2026-06-19 09:27:35 -04:00
co-authored by Claude Opus 4.8
parent 303484d0ab
commit 6e1c9ca924
4 changed files with 1446 additions and 1055 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.planning/research/PITFALLS.md
+294 -317
View File
@@ -1,527 +1,504 @@
# Pitfalls Research — v1.1 Operability & Polish
# Pitfalls Research — v1.2 Multi-Provider, Theming & Zero-Setup
**Domain:** Adding operability features (VALARM reminders, admin Settings, setup wizard, event-driven outbox drain, Gitea CI, Playwright authed-mobile harness) to a shipped Node 22 + Hono + Drizzle/MariaDB + tsdav/ical.js + web-push stack on Unraid/Docker behind Authelia OIDC + Pangolin/Newt.
**Researched:** 2026-06-10
**Confidence:** HIGH — pitfalls derived from direct inspection of the shipped v1.0 source, the v1.0 retrospective, and deep familiarity with ical.js/CalDAV/Gitea Actions semantics. No speculative gaps.
**Domain:** Adding Google Calendar OAuth2, provider abstraction, multiple reminders, dark mode, auto-migrate-on-boot, self-service OAuth onboarding, and dependency updates to a shipped Node 22 + Hono 4 + Drizzle/MariaDB 11 + tsdav/ical.js + web-push stack on Unraid/Docker behind Authelia OIDC + Pangolin/Newt.
**Researched:** 2026-06-19
**Confidence:** HIGH — pitfalls derived from direct source inspection of the shipped v1.1 codebase, v1.0/v1.1 retrospectives, known burned-in lessons from the migration and CI history, and deep familiarity with Google Calendar API, OAuth2, Drizzle/MariaDB migration semantics, and PWA theming constraints. Sources cross-checked against PROJECT.md, RETROSPECTIVE.md, MILESTONES.md, and the VALARM/outbox source in apps/api/src/broker/.
---
## Known Constraints (Do Not Re-Litigate)
## Known Constraints (Inherited — Do Not Re-Litigate)
These are already burned-in lessons. Every pitfall below is written assuming these hold:
These are burned-in from v1.0/v1.1. Every pitfall below assumes these hold:
- **No node-cron.** `setInterval` only. node-cron 4.2.1 silently skips all ticks in the long-lived API process.
- **`drizzle-kit generate`+`migrate`, never `push`.** `push` emits a false destructive diff on populated MariaDB.
- **API integration tests need a real MariaDB** (port-bound dev compose, `DB_HOST=127.0.0.1`, `.env` creds). Tests live in `apps/api/tests/`, never `src/`.
- **Outbox guarantees to preserve:** optimistic 202, enqueue-only route handler, create-before-delete ordering (groupId), drain concurrency guard (`isDraining`), fresh-etag-before-PUT (WR-02), per-uid exactly-once dedup.
- **VAPID private key must decode to exactly 32 bytes** — a truncated key causes a silent Apple 403.
- **Authelia omits `name`/`email`/`preferred_username` from the ID token** by default — needs a `claims_policy` or these fields are absent.
- **No node-cron.** `setInterval` only — node-cron 4.2.1 silently skips all ticks in long-lived Node processes.
- **`drizzle-kit generate`+`migrate`, never `push`.** `push` emits a false destructive diff (TRUNCATE/DROP) on populated MariaDB 11 using the mysql dialect — the introspection bug fires on any existing-data DB. This is the root cause behind D-MIGRATION-10-01 and the quick task cr8.
- **Journal-hash matching is mandatory.** The `__drizzle_migrations` table tracks applied migrations by `tag` (from `_journal.json`). If a migration file is renamed, regenerated, or its journal entry is modified after it has been applied to any environment, drizzle-kit migrate treats it as unapplied and will attempt to re-run DDL that already ran — producing `Duplicate column` errors.
- **MariaDB CI readiness uses `healthcheck.sh --connect`, not `mysqladmin ping`.** `mysqladmin ping` was removed in MariaDB 11.
- **ESLint is pinned at 9.39.4.** ESLint 10 breaks `eslint-plugin-react` and `eslint-plugin-react-hooks`. The root `package.json` pin must not be bumped past 9.x until all React plugins declare ESLint 10 peerDep support.
- **`setInterval` outbox drain, not Redis.** Single-process; in-process EventEmitter (`outboxTrigger.ts`) for the drain signal. `drainRequested` flag pattern prevents double-drain. Optimistic-202, create-before-delete, and `uid:dtstartMs` dedup are load-bearing.
- **All CalDAV VALARM triggers must be built via `ICAL.Duration.fromSeconds(-N*60)`, not bare strings.** Bare strings produce `VALUE=TEXT` which is rejected by Fastmail and Apple Calendar.
- **`REGISTRY_PAT` secret name (not `GITEA_PAT` — the `GITEA_` prefix is silently dropped by Gitea Actions).** `docker login --password-stdin`, never `-p $TOKEN` on the command line.
---
## Critical Pitfalls
### Pitfall 1: VALARM Round-Trip Strips Existing Alarms From Native Clients on Edit
### Pitfall 1: Google Refresh Tokens in "Testing" Publishing Status Expire After 7 Days
**What goes wrong:**
The current `buildVeventString` in `broker/vevent.ts` constructs a fresh `VCALENDAR` with only the properties it knows about (UID, SUMMARY, DTSTART, DTEND, RRULE, LOCATION, DESCRIPTION). When v1.1 adds VALARM authoring, a naive approach adds `VALARM` components to new-creates only. On an edit (update path in `outboxWorker.ts`), the worker re-builds the VEVENT from form payload — not from the stored `rawVevent`. Any VALARM that was added by a native client (Fastmail app, Apple Calendar) will be silently dropped from the PUT payload. The event arrives at Fastmail without the alarm. The user loses their native-client reminder with no warning.
Google OAuth2 refresh tokens issued to an app in "Testing" publishing status (as opposed to "Production") expire after 7 days regardless of the token TTL in the credentials. After 7 days, the stored refresh token returns `invalid_grant` and all Google Calendar operations fail for that member. The user sees calendar data vanish or receives 401 errors, and the fix requires them to re-authorize — a confusing, non-obvious failure for a non-technical user.
The inverse is equally dangerous: if the editor sends `reminderMinutes: 0` (meaning "no reminder"), but the code omits the VALARM field instead of explicitly authoring an empty VALARM block, the old alarm from `rawVevent` is neither preserved nor cleared. Whether it survives depends on what the worker does — and with the current reconstruct-from-scratch approach it will be dropped, which is the correct outcome in that case but only by accident.
Additionally, Google refresh tokens for apps in "Testing" status can only be granted to users explicitly listed as "test users" in the OAuth consent screen. If the operator adds a second member's Google account but forgets to add them as a test user, the authorization flow returns `access_denied` at the consent step, not a useful error message.
**Why it happens:**
`outboxWorker.ts` update path reconstructs the ICS entirely from form fields (using `buildVeventString`). It does not read and preserve non-RRULE sub-components from `rawVevent`. This is a deliberate v1 simplification (the RRULE-preserve path `WR-01` is already the one exception). Adding VALARM to `buildVeventString` handles new-creates correctly but does not cover the "user edited an event that already had a native-app alarm."
Developers set up the OAuth app in "Testing" status because "Production" requires Google verification (which requires a privacy policy, domain verification, and Google review). Testing status is the path of least resistance during development and stays in place longer than intended.
**How to avoid:**
On the update path in `outboxWorker.ts`, before calling `buildVeventString`, parse `rawVevent` with ical.js and extract all existing `VALARM` sub-components. Merge them: if the outbox payload carries an explicit reminder choice (the new `reminderMinutes` field), replace all extracted VALARMs with the new one (or with none if `reminderMinutes: null`). If the payload carries no reminder field (no explicit user change), carry the extracted VALARMs forward into `buildVeventString` as a `valarms` parameter. This mirrors the WR-01 RRULE-preserve pattern exactly. Extend the `outboxPayloadSchema` with an optional `reminderMinutes: z.number().int().min(0).nullable().optional()` field so the absence of the key is distinguishable from an explicit "no reminder."
For a self-hosted household app serving 2 users, request "Production" publishing status immediately — it does not require Google's full verification unless the app accesses sensitive scopes. For calendar read/write, `https://www.googleapis.com/auth/calendar` is a "restricted" scope (not "sensitive"), which DOES require verification. Use `https://www.googleapis.com/auth/calendar.readonly` + `https://www.googleapis.com/auth/calendar.events` instead — these are the non-restricted scopes sufficient for event read/write (no attendee PII access). With these scopes, production status is grantable without full Google review.
If testing status must remain during development, encode a `token_expiry_check` that validates the refresh token age and prompts re-auth before it expires rather than failing silently. Store the token grant timestamp alongside the encrypted refresh token in `member_credentials`.
Add the operator's and wife's Google accounts as test users in the OAuth consent screen during development.
**Warning signs:**
- Reminders set in the Fastmail native app disappear after editing the event in FamilySync.
- A shared event with a reminder shows the reminder field as empty after an FamilySync round-trip.
- `rawVevent` in `calendar_events` has `BEGIN:VALARM` but the PUT payload does not.
- Calendar data disappears exactly 7 days after a member connected their Google account.
- `invalid_grant` error in API logs when refreshing Google tokens.
- Second member's authorization flow returns `access_denied`.
**Phase to address:**
Per-event reminders phase (VALARM authoring). The VALARM-preserve logic must land in the same PR as `buildVeventString` VALARM support — not as a follow-up.
Google Calendar provider phase (first Google OAuth plan). Document the testing-vs-production publishing status distinction in the onboarding UI and in `docs/deployment.md` before implementing the OAuth flow.
---
### Pitfall 2: TRIGGER Value-Type Mismatch Silently Produces Broken VALARM
### Pitfall 2: Google Calendar Recurrence Model Does Not Map Cleanly to iCalendar RRULE
**What goes wrong:**
RFC 5545 §3.8.6.3 defines two legal TRIGGER value types for VALARM:
The Google Calendar API represents recurring events as a "master event" with a `recurrence` field (an array of raw iCalendar strings like `["RRULE:FREQ=WEEKLY;BYDAY=MO"]`) plus separate "instance" resources for occurrences that have been individually modified (moved, canceled). The FamilySync codebase assumes the iCalendar model where: (a) a single VEVENT with RRULE expands to all occurrences via `ICAL.RecurExpansion`, and (b) exceptions are represented as VEVENT objects with `RECURRENCE-ID`.
- `DURATION` (default): `TRIGGER:-PT15M` — fires 15 minutes before DTSTART.
- `DATE-TIME`: `TRIGGER;VALUE=DATE-TIME:20260610T120000Z` — fires at an absolute UTC instant.
When pulling Google recurring events, the Google API returns **individual instance resources** by default in `events.list` — unless you pass `singleEvents: false`. If `singleEvents: true` (the default), you get individual expanded occurrences, not the master event. You cannot reconstruct the RRULE from the instances. If `singleEvents: false`, you get the master event (with RRULE) but **also need to separately fetch modified instances** to know which occurrences have been moved or canceled. There is no single API call that returns "master event + delta patches in RECURRENCE-ID form" like CalDAV does.
ical.js represents these differently. If you set a VALARM TRIGGER using `addPropertyWithValue('trigger', '-PT15M')` (a bare string), ical.js will emit it as `TRIGGER:-PT15M` on some versions and as `TRIGGER;VALUE=TEXT:-PT15M` on others, depending on whether it infers the type. The `VALUE=TEXT` form is not RFC-compliant for VALARM TRIGGER and will be silently ignored by Fastmail and Apple Calendar — the reminder never fires. The ICS looks valid at a glance but produces no alarm.
Separately: `RELATED=END` (fire N minutes before DTEND, not DTSTART) is a valid TRIGGER parameter. If the existing event from a native client uses `TRIGGER;RELATED=END:-PT10M` and the VALARM-preserve code in Pitfall 1 carries it forward, it is preserved correctly. But if the code reconstructs the VALARM from a stored `reminderMinutes` number only, it loses the RELATED parameter and the semantics change.
Deleted/canceled individual occurrences in Google Calendar have `status: "cancelled"` — they are not omitted from the response. If your code filters on `status === "confirmed"` you silently drop the cancellation records and re-expand the occurrence the next time the RRULE expands, causing ghost events.
**Why it happens:**
ical.js VALARM construction is not well-documented. The property API requires using `ICAL.Duration` or `ICAL.Time` objects, not bare strings, to get the correct value type in the output. Most examples online use the string form that works in some parsers but is not RFC-compliant.
The CalDAV model and the Google Calendar API model for recurrence are fundamentally different. CalDAV treats recurrence as an iCalendar artifact (RRULE on the master VEVENT, RECURRENCE-ID VEVENTs for exceptions). The Google API wraps iCalendar but exposes a different resource model. Developers assume the broker abstraction can produce identical event shapes from both providers, but the Google recurrence model requires a different fetch strategy.
**How to avoid:**
Build the TRIGGER using `ICAL.Duration.fromSeconds(-reminderMinutes * 60)` and set it as the property value, not as a string. Verify the emitted TRIGGER line does not contain `VALUE=TEXT`. For preserved VALARMs (from `rawVevent`), round-trip the sub-component through ical.js parse→serialize rather than extracting the raw text and reinserting it, to catch any encoding issues.
Fetch Google events with `singleEvents: false` to get master events with RRULE. For any recurring master event, issue a second call to fetch its instances (`events.instances()`) to get modified/canceled occurrences. Map `status: "cancelled"` instance resources to EXDATE entries on the master event's RRULE. Use `ICAL.RecurExpansion` on the reconstructed VCALENDAR for display, same as the Fastmail path.
Add a unit test: build a VALARM with `reminderMinutes: 15`, serialize to ICS, parse back with ical.js, and assert the TRIGGER DURATION value is `-PT15M` with no VALUE parameter other than DURATION (which is the default and is usually omitted).
The provider abstraction interface should return normalized VEVENT strings (raw iCalendar text), not provider-specific objects. The Google adapter's job is to produce a VCALENDAR string (including reconstructed RRULE + EXDATE) that the existing `expand.ts` / `sync.ts` layer can handle without modification.
Expose a `fetchRecurringMasterWithExceptions(uid)` method in the Google adapter that issues both API calls and returns a unified VCALENDAR string. Do not attempt to normalize at the `events.list` level.
**Warning signs:**
- ICS output contains `TRIGGER;VALUE=TEXT:-PT15M`.
- Reminders appear in the FamilySync UI but never fire on the device.
- Apple Calendar / Fastmail app shows the event with no alarm after an FamilySync edit.
- Recurring events show individual occurrences as separate events in the calendar instead of a recurring series.
- Canceled occurrences still appear in the calendar after being deleted in Google Calendar.
- `singleEvents: true` is used in the Google API client — this is wrong for the FamilySync data model.
**Phase to address:**
Per-event reminders phase. Unit test the VALARM serialization before any end-to-end reminder test.
Google Calendar provider phase (recurrence handling plan). Write a unit test that takes a Google `events.list` response (with a recurring master + a modified instance + a canceled instance) and asserts the reconstructed VCALENDAR string passes ical.js parsing and `ICAL.RecurExpansion` without error.
---
### Pitfall 3: All-Day Event VALARM Timezone Semantics Are Undefined
### Pitfall 3: Google syncToken Invalidation Triggers Silent Full Re-Sync That Overwrites Local Writes
**What goes wrong:**
For an all-day event (DTSTART;VALUE=DATE), the meaning of `TRIGGER:-PT15M` is ambiguous. RFC 5545 requires that a DURATION TRIGGER on an all-day event be evaluated against DTSTART as a DATE — which has no time component — resulting in undefined behavior in most implementations. Apple Calendar interprets it as "15 minutes before midnight of the start date in local time." Fastmail ignores VALARM on all-day events entirely in some tested configurations. Android may fire the alarm at midnight UTC.
The Google Calendar API supports incremental sync via `syncToken`: call `events.list` with the token from the previous response to receive only changes since the last sync. When the sync token expires (after ~7 days of inactivity, or after any large state change like timezone change or Google-side recalculation), the API returns `HTTP 410 Gone`. The correct response is to perform a full re-sync (no syncToken, save the new token). If the code does not handle 410 — or handles it by retrying with the old token — all incremental sync calls return 410 forever, silently halting all calendar updates.
A second issue: during a full re-sync, the code fetches all events and upserts into `calendar_events`. If a member created an event in FamilySync that is in the outbox (not yet written to Google), and the full re-sync runs before the outbox drains, the upsert overwrites the local optimistic row with the stale Google state (the event is not yet on Google's side), effectively reverting the user's creation. The event disappears from the UI until the next outbox drain re-creates it on Google and the next poller sync pulls it back.
**Why it happens:**
The project already correctly excludes all-day events from the reminder scheduler (`reminderScheduler.ts`, `WHERE allDay=false`). But if VALARM is stored on an all-day event (because the user created an all-day event and selected a reminder), the scheduler's WHERE clause means it silently never fires — which is correct behavior — but the user sees a reminder field in the UI and expects it to work.
syncToken expiration is handled in Google's CalDAV-via-ICAL model too (ctag changes trigger full re-sync), but the FamilySync ctag-based poller handles this correctly. The Google syncToken path is a different code branch that does not inherit the ctag handling. Full re-sync without outbox-awareness is a standard oversight.
**How to avoid:**
In the event form UI: disable or hide the reminder selector when `allDay: true`. If the API receives a create/update payload with `allDay: true` and a non-null `reminderMinutes`, strip the alarm and log a warning — do not store a VALARM that will silently not fire. Document this as a known constraint.
Wrap every `events.list` call that uses a `syncToken` in a try/catch for 410. On 410: drop the sync token, mark the provider's sync state as `stale`, and enqueue a full re-sync on the next poller tick (not in-band, to avoid blocking the current poller iteration).
In the scheduler, when v1.1 generalizes the lead time: keep the `WHERE allDay=false` guard in the SQL query regardless of how VALARM data is stored. Do not "fix" this by removing the guard when you extend VALARM support.
Before upsetting rows from a full re-sync, check the `calendar_outbox` table for any pending rows for the same user/calendar. If any pending writes exist for the same event UID, skip overwriting that row's `raw_vevent` from Google — the outbox write is the authoritative source of truth until it drains. Alternatively: drain the outbox before any full re-sync. The outbox drain is already ~1–2s; blocking the re-sync on it is acceptable.
**Warning signs:**
- All-day event with reminder set produces an ICS with a VALARM on a DATE-typed DTSTART.
- User reports reminder not firing for an all-day birthday event.
- Reminder field enabled in the UI for all-day events.
- API logs show `HTTP 410` from Google Calendar and the app stops updating events for that member.
- Events created via FamilySync disappear briefly then reappear.
- `syncToken` is stored per-provider in `member_credentials` or a separate `provider_sync_state` table but there is no 410 error handler branch.
**Phase to address:**
Per-event reminders phase. UI constraint and API guard belong in the same plan.
Google Calendar provider phase (sync/poller plan). 410 handling must be in the acceptance criteria before the Google poller ships.
---
### Pitfall 4: Duplicate Push When Generalizing the Fixed-Window Dedup to Per-Event Lead Times
### Pitfall 4: Google All-Day Events Use DATE, Timed Events Use RFC 3339 — Timezone Handling Differs From CalDAV
**What goes wrong:**
The current `reminderScheduler.ts` deduplication key is `uid` alone (`sentReminders` Map). The fixed 16-minute catch-up window ensures an event stays in-window across at most a few consecutive 1-minute ticks. When v1.1 changes the lead to a per-event value (e.g., event A has a 30-minute lead, event B has a 2-hour lead), the window must widen — or the scan logic must change — to accommodate variable leads. There are two failure modes:
Google Calendar returns timed events with `start.dateTime` (RFC 3339 string with timezone offset, e.g., `2026-07-01T10:00:00-04:00`) and all-day events with `start.date` (ISO 8601 date string, e.g., `2026-07-01`). The FamilySync sync layer (`sync.ts`) currently parses raw iCalendar `DTSTART;TZID=America/Toronto:20260701T100000` strings via ical.js and stores the UTC instant in `dtstart_utc`.
1. **Window too narrow for long leads:** If the scheduler still scans only `(now, now+16min]`, events with a 2-hour lead never enter the window and their reminder never fires.
When the Google adapter converts `start.dateTime` to iCalendar, it must produce `DTSTART:20260701T140000Z` (UTC). If it instead produces `DTSTART;TZID=America/Toronto:20260701T100000`, ical.js parses it correctly, but only if the corresponding `VTIMEZONE` component is also present in the VCALENDAR. Google does not return VTIMEZONE components — their API uses offset strings. A missing VTIMEZONE for a TZID reference causes ical.js to default to UTC, silently shifting the event time.
2. **Dedup key collision across rescheduled events:** If an event is rescheduled (DTSTART changes), the uid is the same but the VALARM should fire again for the new time. The current dedup key `uid` alone, with the `sentReminders.set(uid, dtstartMs)` pruning based on the stored dtstart, handles this — but only if the new dtstart causes the map entry to be pruned before the next alarm window. If the user reschedules an event to fire sooner than the original dtstart (e.g., from 3pm to 2pm, currently 2:10pm, 10 minutes after the original reminder already fired), the uid is still in `sentReminders` with the old dtstart (3pm), which has NOT yet passed `now`, so the CR-01 pruning has not removed it. The reminder for 2pm silently does not fire.
All-day event reminder handling has a separate pitfall: the `computeAlertInstantUtc` function in `vevent.ts` uses `getHouseholdTimezone(db)` (the stored IANA timezone) to compute "9 AM local on alert day." For Google all-day events, the Google API returns `reminders.overrides` (a list of `{method, minutes}` objects) or `reminders.useDefault` (boolean). If `useDefault: true`, the reminder fires at the user's Google Calendar default reminder time (from their Google account settings) — which the server cannot know. Mapping `useDefault: true` to a `reminderLeadMinutes` value in FamilySync is undefined.
**Why it happens:**
The uid-only dedup was designed for the fixed-15-minute lead where the dedup window is short and rescheduling edge cases are low-probability. Per-event leads break both assumptions.
CalDAV stores full iCalendar with VTIMEZONE. The Google API abstracts timezone into RFC 3339 offsets. The two representations look similar but require different handling in ical.js. Developers test with US/Eastern events and miss that ical.js silently falls back to UTC for an unknown TZID.
**How to avoid:**
Change the dedup key from `uid` alone to `uid + ':' + dtstartMs`. This makes the dedup per-(event, scheduled-time), not per-event. A rescheduled event has a different dtstart and gets a new dedup entry. The sentReminders map still prunes on `dtstartMs <= now`.
In the Google adapter, always normalize `start.dateTime` to UTC before building the iCalendar string. Use `new Date(googleEvent.start.dateTime).toISOString()` to get the UTC instant, then build `DTSTART:20260701T140000Z`. Never emit a `TZID` reference without a corresponding `VTIMEZONE` block.
For the variable-window query: instead of scanning a fixed `(now, now+16min]` window, store the per-event lead time alongside the VALARM in `calendar_events` (e.g., a `reminderMinutes` column). The scheduler query becomes `WHERE dtstartUtc <= (now + reminderMinutes minutes) AND dtstartUtc > now`. This requires a schema migration.
For all-day events: map `start.date` directly to `DTSTART;VALUE=DATE:20260701`, same as the CalDAV path.
Add an integration test for the scheduler that covers: (a) event with a 30-minute lead fires at T-30, (b) event rescheduled earlier after the first fire fires again for the new time.
For Google `reminders.useDefault: true`: treat it as "no reminder" at the FamilySync layer. Store `reminderLeadMinutes: null`. Document this in the UI: "Google default reminders from your Google account settings are not shown in FamilySync; set a reminder here to receive FamilySync push notifications." This avoids fetching and storing per-member Google account preferences.
For Google `reminders.overrides`: map the first `email`-method reminder to `reminderLeadMinutes` (FamilySync uses push, not email). Ignore `popup` method overrides unless there is a clear 1:1 mapping.
**Warning signs:**
- Events with a long reminder lead never fire.
- Rescheduled event reminder does not fire after the reschedule.
- Scheduler dedup map grows without bound (no uid-dtstart pair is ever pruned because the dtstart moved out from under the map entry).
- Timed events from Google appear at the wrong time in FamilySync (off by the timezone offset).
- ical.js warnings about unknown TZID in the server logs.
- All-day events with `useDefault: true` trigger unexpected push notifications.
**Phase to address:**
Per-event reminders phase. Schema migration for `reminderMinutes` column in `calendar_events` is a prerequisite; dedup key change must land in the same plan.
Google Calendar provider phase (event normalization plan). Write unit tests for the UTC normalization: assert that a `start.dateTime` with `-04:00` offset produces `DTSTART:...Z` with the correct UTC instant, and that a missing VTIMEZONE is never emitted.
---
### Pitfall 5: Double-Drain When Event-Driven Trigger and 15s setInterval Both Fire
### Pitfall 5: Provider Abstraction Refactor Regresses the Live Fastmail Path
**What goes wrong:**
The v1.1 event-driven drain adds a trigger (Redis pub/sub message, or direct `runOutboxDrain()` call) that fires immediately when a new row is enqueued. The 15s `setInterval` fallback continues to run. Both can invoke `runOutboxDrain()` concurrently. The existing `isDraining` module-level flag provides single-execution within the same JS tick, but there is a subtler race:
Extracting a `CalendarProvider` interface from the existing CalDAV code requires moving the tsdav client, sync logic, poller, outbox worker, and reminder scheduler from Fastmail-specific code into an abstraction layer. The risk is that the refactor introduces a regression on the Fastmail path: a subtle behavioral change in the outbox worker (e.g., enqueue ordering, `isDraining` guard, or the `drainRequested` flag) can cause:
- T=0: Route enqueues row. Event-driven trigger calls `runOutboxDrain()`. `isDraining` is set.
- T=0.5s: Drain in progress. 15s interval fires. `isDraining` is true — no-op. Correct.
- T=1s: Drain completes. `isDraining` reset to false.
- T=1.5s: Event-driven trigger for a SECOND enqueue calls `runOutboxDrain()`. Drain starts.
- T=14s: First 15s tick since startup fires (not 15s after the last drain completed, but 15s after the interval was registered at server start). Calls `runOutboxDrain()`. `isDraining` is true — no-op. Correct.
- Duplicate CalDAV PUTs (double-drain race reintroduced).
- Create-before-delete ordering broken for move operations.
- The ctag-based poller not calling the new interface method in the correct order (poll → sync → schedule).
- The `uid:dtstartMs` dedup key in the reminder scheduler falling back to `uid` alone if the schema type changes during the refactor.
So far so good — `isDraining` handles this. The failure mode is: **if `runOutboxDrain` is called directly (not through the setInterval wrapper) from the event-driven path, thrown errors will not be caught by the setInterval `.catch()` handler.** An unhandled rejection crashes the process on Node 22 (where unhandledRejection is fatal by default unless a handler is registered). The fix is to always use the same error-caught wrapper: `runOutboxDrain().catch(err => console.error(...))`.
A more dangerous double-drain scenario arises if the event-driven trigger is implemented via ioredis pub/sub and the subscriber receives the same message twice (ioredis at-least-once delivery). Two concurrent `runOutboxDrain()` calls can occur before either sets `isDraining`. The `isDraining` check is not atomic. In the single-process Node.js event loop, two synchronous checks of `isDraining` before any `await` both see `false` and both proceed. The first `await db.select()...` in both drain calls then runs in parallel. Both fetch the same pending rows and dispatch the same CalDAV writes, producing duplicate PUTs.
A specific risk: the outbox worker currently reads `provider_type` from `member_credentials` but the only implemented value is `'caldav'`. During refactor, if the worker is changed to dispatch to a provider-specific `write()` method, and the CalDAV write path is accidentally placed behind a `provider_type === 'google'` branch instead of `provider_type === 'caldav'`, all existing Fastmail write-back silently stops with no error (the wrong branch simply does nothing).
**Why it happens:**
`isDraining` is a module-level boolean, not a mutex or a DB-level row lock. In the single-process deployment it is correct for the `setInterval` case (the JS event loop ensures only one tick can run at a time). But two synchronous calls to `runOutboxDrain()` before any `await` both pass the `if (isDraining) return` check because the flag is set inside the function body, not before the call.
Refactors that extract interfaces from working code are high-regression-risk because the tests were written against the concrete implementation. The test suite may pass because it stubs the CalDAV calls but not the new interface dispatch, so the dispatch logic change is never exercised by tests.
**How to avoid:**
Wrap the event-driven call in the same caught wrapper. More importantly: do not call `runOutboxDrain()` directly from the pub/sub subscriber. Instead, call a `triggerDrain()` helper that sets `isDraining = true` synchronously before the first await, or simply lets the setInterval do the work and uses the pub/sub message only to shorten the next wait (e.g., trigger a single immediate `runOutboxDrain()` call from within the setInterval handler if a "pending" flag is set, keeping all drain calls single-threaded through the interval). The cleanest approach: keep one drain path (the setInterval), but when an enqueue event arrives, set a `drainRequested` flag; the next setInterval tick checks the flag and drains immediately instead of waiting the full 15s.
Refactor behind a feature flag or in a branch that keeps the Fastmail path fully operational throughout. Do not merge the provider abstraction until all existing CalDAV integration tests (outbox, poller, ctag, reminder scheduler) pass unchanged against the new interface.
Write a golden-path integration test for the Fastmail provider specifically: enqueue a write → assert it reaches the CalDAV mock → assert the outbox row is marked done. This test must pass before and after the refactor with no modification.
Keep the `provider_type === 'caldav'` dispatch path as the default/fallback. The Google path (`provider_type === 'google'`) is additive; it must not be required for the existing Fastmail path to work.
The abstraction interface should be defined from the consumer's perspective (what `outboxWorker.ts` and `poller.ts` need), not from the CalDAV implementation's perspective. Avoid leaking tsdav types (`DAVCalendar`, `DAVObject`) into the interface — callers should see normalized event objects.
**Warning signs:**
- Duplicate CalDAV PUTs for the same event visible in Fastmail logs.
- Two identical events appearing briefly after an edit.
- 412 conflict errors on the second of two simultaneous drain calls (the first PUT succeeded, the second uses an outdated etag).
- Any existing CalDAV integration test is modified during the refactor (should be zero changes to existing tests).
- The outbox worker has a branch that exits without calling any write method for `provider_type === 'caldav'`.
- The `provider_type` column contains `'caldav'` for all existing rows but the refactored dispatch uses string equality without a default case.
**Phase to address:**
Event-driven outbox drain phase. The drain trigger design must be reviewed before implementation; the `isDraining` guard docs already note the single-process limitation.
Provider abstraction phase (first). Must ship before any Google Calendar code is written. The acceptance gate is: all v1.1-passing integration tests still pass, unchanged, with the abstraction layer in place.
---
### Pitfall 6: Event-Driven Drain Breaks Create-Before-Delete Ordering Under Concurrent Enqueues
### Pitfall 6: Multiple VALARMs — Duplicate Fire and Preserve-vs-Replace Ambiguity
**What goes wrong:**
The edit-as-move path enqueues two rows (delete old uid, create new uid) in the same request handler. With the 15s poll, both rows are almost always in the DB before the next drain cycle. With event-driven drain (trigger fires on enqueue), a race is possible:
v1.2 adds multiple reminders per event (multiple VALARMs on a single VEVENT). The v1.1 reminder scheduler deduplicates on `uid:dtstartMs` — one dedup entry per event. With multiple VALARMs, a single event now needs multiple push notifications at different lead times. If the dedup key is still `uid:dtstartMs`, only the first alarm fires; subsequent alarms for the same event at different leads are treated as "already sent" by the dedup map.
- Request handler enqueues the CREATE row. Event-driven trigger fires immediately. Drain runs. Create is dispatched successfully. `isDraining` resets.
- Request handler (same HTTP request, now at the second DB insert) enqueues the DELETE row. Trigger fires again. Drain runs. The create is status=done. The delete is dispatched.
A second issue: the scheduler currently runs a SQL query for events where `reminderLeadMinutes IS NOT NULL`. With multiple reminders stored as multiple VALARM objects in `rawVevent` (and potentially as a JSON array or repeated column), the SQL query must change to a per-VALARM lead-time scan. If the query fetches one row per event and picks the first VALARM lead only, subsequent alarms are silently skipped.
This is actually the happy path — correct ordering. The dangerous case is if the HTTP handler enqueues the DELETE row first and the CREATE row second (e.g., if the code is written in that order). The event-driven drain fires after the DELETE enqueue, finds the delete row with no done-sibling, and the durable CR-04 gate defers it. When the CREATE is then enqueued and drained, the delete is re-attempted on the next cycle — also correct. But if the delete fires before the create for any reason (e.g., a coding error that enqueues in the wrong order, or the delete row has a lower `next_attempt_at`), the original event is deleted before the new one is confirmed, causing data loss.
A third issue: the VALARM preserve-on-edit path (D-08 / CAL-14, shipped in Phase 11) extracts all existing VALARMs from `rawVevent` and re-attaches them. With multiple VALARMs, if the user adds a new reminder in the FamilySync UI (which produces one VALARM from the picker), the preserve path must decide: replace all existing VALARMs with the single new one, or append? The current single-reminder model implicitly "replace all." Multiple-reminder UI changes this intent.
A subtler issue: if the event-driven trigger fires between the two DB inserts in the same HTTP handler (possible if the first `await db.insert()` resolves and the trigger fires before the second `await db.insert()` runs), the drain may start before both rows are committed. MySQL/MariaDB default isolation (REPEATABLE READ) means the drain transaction may not see the second row at all until it starts a new transaction. The CR-04 durable gate handles this case: the delete will defer itself because the sibling create is not yet visible. But if the create row is invisible, the drain processes the delete alone, defers it correctly, and then the create arrives. This is safe but results in at least one extra drain cycle for the move. Not a bug, but a latency regression on the event-driven path.
A fourth issue: Google Calendar caps reminders at 5 per event (`reminders.overrides` max 5). If the user creates 6 reminders in FamilySync and the event is in a Google Calendar, the write-back to the Google Calendar API returns a 400 error. The error must be caught and surfaced to the user before the write, not as a silent failure.
**Why it happens:**
The v1.1 architecture was designed for exactly one VALARM per event (the picker is single-selection). Multiple VALARM support requires rethinking the dedup key, the SQL query, the preserve logic, and the write-back validation.
**How to avoid:**
Always enqueue the CREATE row before the DELETE row in the HTTP handler, matching the existing sort-before-dispatch logic in the drain. This is already the intent of D-04 but should be an explicit code comment in the edit-as-move handler.
Change the scheduler dedup key to `uid:dtstartMs:leadMinutes` (three-part key). Each VALARM on an event gets its own dedup entry, and each fires independently.
Do not trigger the event-driven drain between the two enqueue inserts. If the trigger is a direct call, wrap both inserts in a single DB transaction and trigger the drain only after the transaction commits. If the trigger is Redis pub/sub, publish after both inserts.
Store multiple reminder leads in a normalized form. Two options: (a) a `event_reminders` junction table (`event_id`, `lead_minutes`), or (b) a JSON array column `reminder_lead_minutes_json` on `calendar_events`. Option (a) is cleaner for SQL querying; option (b) avoids a second migration and join. For a two-user household, option (b) is acceptable. The SQL scheduler query must unnest or join to iterate per-lead.
For the preserve-on-edit path: the UI must distinguish "user changed reminders" from "user changed something else (title, time, location)." If reminders were not touched, preserve all existing VALARMs. If the user opened the reminder editor and submitted, replace all VALARMs with the new set. This is a frontend concern: include a `remindersChanged: boolean` flag in the outbox payload alongside the new `reminderLeads: number[]` array.
For Google Calendar cap validation: add a Zod validator on the outbox payload schema that rejects more than 5 reminder leads when the provider is Google. Surface the error in the form before submit.
**Warning signs:**
- Edit-as-move operations produce a "calendar object not found" error from Fastmail (delete reached Fastmail before the create).
- Events occasionally disappear after an edit and reappear after the next poller sync cycle.
- CR-04 deferral log messages (`Deferring delete row...`) appearing frequently for move operations.
- Second and third reminders on an event never fire.
- The scheduler sends duplicates for the first alarm on every tick (dedup not working after key change).
- Events with 6+ reminders silently fail to write to Google Calendar.
- The preserve path strips all VALARMs when the user edits only the event title.
**Phase to address:**
Event-driven outbox drain phase. The enqueue ordering requirement and transaction boundary must be specified in the plan.
Multiple reminders phase. The dedup key change and schema migration must land in the same plan. Google cap validation must land in the same plan as Google write-back support.
---
### Pitfall 7: Admin App-Password Update Logged or Echoed in Error Messages
### Pitfall 7: Drizzle Auto-Migrate on Boot on MariaDB 11 — The Three Failure Modes
**What goes wrong:**
The admin Settings route receives the new Fastmail app password in the request body. If Zod validation fails (wrong format, too long), the default Zod error message includes the invalid value in the error output: `Invalid value: "xxxx-xxxx-xxxx-xxxx"`. If the Hono error handler returns this Zod error to the client as JSON, the app password appears in: (1) the HTTP response body, (2) any request logging middleware, (3) server logs if the error is caught and `console.error(err)` is called with the full error object.
v1.2 adds zero-manual-setup: the API process runs pending migrations automatically at startup. There are three distinct failure modes specific to this stack:
Separately: `decryptPassword` in `broker/crypto.ts` currently never logs the decrypted value (T-03-13), but the admin update route must call `encryptPassword(newPassword)` after receiving the plaintext. If the route logs the request body at any point before the encrypt call, the password is in the logs.
**Failure mode A — Journal-hash mismatch with legacy tracking.** The `__drizzle_migrations` table records applied migrations by `tag`. If any migration file was renamed after it was applied to an existing database (or if the journal was re-generated, changing the `when` timestamp), drizzle-kit migrate will not find the matching `tag` in `__drizzle_migrations` and will try to re-apply the already-applied migration. On MariaDB, re-running `ALTER TABLE ... ADD COLUMN` for a column that already exists returns `Duplicate column name` — not a transaction rollback, but a crash. This is the "journal-hash mismatch" lesson from D-MIGRATION-10-01.
**Failure mode B — False destructive diff if `db:push` logic is used.** Any code path that calls `drizzle-kit push` (even programmatically or from a startup script) against the live MariaDB will schedule TRUNCATE/DROP. The `drizzle migrate` API (programmatic via `drizzle-orm/migrator`) is safe — it applies committed `.sql` files and does not introspect the live DB. The failure mode is: a developer adds a startup call that uses the wrong drizzle API entry point.
**Failure mode C — Concurrent startup race (two container instances).** If the Docker Compose stack is ever restarted in a way that brings up two API containers briefly (e.g., a rolling update or `docker-compose up --scale api=2`), both instances call `migrate()` at startup. MariaDB does not have advisory locks by default. Both instances read `__drizzle_migrations`, both see the same unapplied migrations, and both attempt to apply them simultaneously. The first to commit wins; the second hits `Duplicate column name` or a primary key conflict and crashes. The container crash loop then restarts the container, which succeeds because migrations are now applied — but the crash log looks like a startup failure.
**Why it happens:**
Developers often log `req.body` at the route level for debugging during development. The admin route is new, debugging is natural, and the log line gets committed. Zod error passthrough is the other common source — the validator middleware returns the full error object.
Startup migration is convenient but runs without a human watching the DDL. The existing dev workflow always runs `pnpm db:migrate` explicitly, so a developer sees the output. Auto-migrate is invisible unless there is explicit logging. On MariaDB (no DDL transactions), a partially-applied migration leaves the schema in an undefined state.
**How to avoid:**
In the `@hono/zod-validator` middleware for the app-password body schema, always use a custom `hook` to return a generic `{ error: "Invalid request" }` without the Zod error detail. Never log the request body in the admin/settings routes. Add a lint rule or code review checklist item: no `console.log` in any file under `routes/admin*` or `routes/settings*` that could include body content.
Use the `drizzle-orm/migrator` programmatic API (`migrate(db, { migrationsFolder: '...' })`), not `drizzle-kit` CLI, in the startup path. The migrator API applies committed SQL files sequentially and tracks via `__drizzle_migrations` — it is safe for auto-migrate.
For test coverage: write a unit test that asserts the route returns `400` with no `value` field in the response when given an invalid password. Do not assert on the specific Zod error message.
Add a startup log that prints each migration file name as it is applied (or "already applied, skipping"). This makes auto-migrate auditable.
For the concurrent-startup race: wrap the migrate call in a MariaDB advisory-lock table: `INSERT INTO migration_lock (id) VALUES (1)` with a unique constraint. If the insert fails, the other instance already holds the lock and is migrating — wait and retry. On success, run migrations, then `DELETE FROM migration_lock WHERE id=1`. This is a single-row sentinel table, created before any other migration.
For distinguishing fresh-DB vs existing-DB: the `__drizzle_migrations` table not existing means a fresh DB — apply all migrations. The table existing with all entries means fully migrated — skip. The table existing with some entries means a partial state — apply only the unapplied subset (normal drizzle behavior). No special-casing needed; the migrator handles this correctly.
**Warning signs:**
- App password appears in any log output or API response body.
- The Zod error response for the settings route includes a `received` or `message` field containing password-like strings.
- `Duplicate column name` error in the API startup log immediately after deployment.
- Two container instances log "applying migration 0003_..." at the same timestamp.
- The API container crashes on first start but succeeds on second start (concurrent-race symptom).
- Any code calling `drizzle-kit push` in a startup path (grep for it explicitly before shipping).
**Phase to address:**
Admin Settings phase. Security review of the settings route before first deployment; treat app-password fields the same as `OIDC_CLIENT_SECRET` — never log, never echo.
Zero-manual-setup DB migration phase. The `migration_lock` sentinel table and the startup log must be in the acceptance criteria. Write a test that runs `migrate()` twice on the same DB and asserts no error and no duplicate column.
---
### Pitfall 8: Unauthenticated Setup Endpoint Left Live After First Run
### Pitfall 8: PWA Dark Mode — Flash of Wrong Theme and Service Worker Cache Staling
**What goes wrong:**
The setup wizard endpoint must be accessible before any member has authenticated (no credentials exist yet, so OIDC cannot be used to protect it). The typical implementation: mount the setup routes outside the `app.use('/api/*', oidcAuthMiddleware())` guard, and detect "first run" by checking whether any `member_credentials` row (or VAPID env) exists. The failure mode: the first-run check passes once. But if the developer forgets to add an "already-set-up" guard, or the check looks at the wrong table, the endpoint remains callable after setup — allowing anyone who can reach the internal network (or the Pangolin public URL) to overwrite the app password without authentication.
The standard dark mode pattern — read `prefers-color-scheme` or a `localStorage` preference and apply a `data-theme` attribute on `<html>` — has a flash-of-wrong-theme (FOWT) on initial load if the attribute is set by React (after hydration) rather than by an inline script before the first paint. The user sees light mode for ~100ms then switches to dark. On iOS standalone PWA, this flash is especially visible because the splash screen (controlled by `theme-color` meta tag) may not match the applied theme.
A second failure mode: the wizard validates env vars (VAPID keys, DB connection, app password) but stores the app password directly in the DB or in a temp file instead of in an env var. The architecture requires app passwords to be encrypted at rest using `APP_PASSWORD_ENCRYPTION_KEY`. If the wizard stores the password before the encryption key env is set (it shouldn't be — the wizard is supposed to collect the key or confirm it exists), the encryption call throws and the wizard fails with a 500 that may include the plaintext password in the error.
The deeper issue: Schedule-X uses `--sx-color-*` CSS custom properties for its calendar component theming. These are not in the `tokens.css` semantic token layer established in Phase 17. If dark mode only updates the FamilySync token variables and not the `--sx-color-*` variables, the calendar component remains in light mode regardless of the document theme — a visually broken state that is easy to miss in desktop testing (because developers often test on a light system preference).
A service worker cache complication: the Workbox-managed precache caches `index.html` and all static assets at build time. If the theme preference is baked into a cached `index.html` (e.g., via a server-side render or a build-time default), the cached version will always load with the light-mode default, overriding the user's stored preference. In Vite PWA (client-side rendered), this is not the primary risk — but if the service worker caches a CSS file that includes hardcoded light-mode colors (not variables), dark mode changes deployed in a new build will not reach the user until the service worker updates.
**Why it happens:**
Setup wizards are one-shot paths that receive less testing than the main app. "First run only" guards are often implemented as booleans that can be reset, or checks that are too broad.
Dark mode is treated as a CSS-only concern but the FOWT is a JS execution timing concern. Schedule-X's custom properties are undocumented relative to external theming. Service worker cache invalidation for CSS changes requires a cache-busting strategy.
**How to avoid:**
Implement the "already set up" guard as: check for any row in `member_credentials` AND for the presence of VAPID env vars (not just one or the other). If either is already set, return 423 Locked from all setup endpoints. Once the setup completes successfully, the next request to setup routes returns 423 immediately — no state to reset without a server restart.
Prevent FOWT: inject a minimal inline `<script>` in `index.html` (before any CSS) that reads `localStorage.getItem('theme')` and sets `document.documentElement.dataset.theme` synchronously. This runs before any CSS is parsed, preventing the flash. The React app then reads the same localStorage key on mount to initialize the Zustand theme store.
Alternatively, use a DB-stored `setup_completed_at` timestamp in a `settings` table (a migration is needed anyway for v1.1 admin features). The wizard marks this column on completion; all setup routes check it first.
Map all `--sx-color-*` variables used by Schedule-X to the FamilySync dark/light token values. Inspect the Schedule-X `@schedule-x/theme-default` CSS to enumerate all `--sx-*` variables and override them in a `tokens.css` `[data-theme="dark"]` block. Assert that the calendar renders in dark mode during the dark mode acceptance test.
Never accept the `APP_PASSWORD_ENCRYPTION_KEY` value via the API. The wizard should validate that the env is already set (by attempting a test encrypt/decrypt), not collect the key. The key stays in the env/Docker secrets layer.
For service worker cache: `vite-plugin-pwa` handles cache busting via file hashing in the build output. Verify that `tokens.css` (or whichever file holds the dark mode variables) is included in the Workbox precache manifest — if it is in the public directory without a hash, it may not be cache-busted on deploy. Move all theme CSS into a hashed build output file.
For iOS standalone PWA `theme-color`: update the `<meta name="theme-color">` dynamically using `document.querySelector('meta[name="theme-color"]').setAttribute('content', ...)` when the theme toggles. iOS reads `theme-color` for the status bar color. A static `theme-color` will not match the active dark theme.
**Warning signs:**
- Setup endpoint returns 200 after the app is already configured.
- Curl to `/api/setup/...` with no auth cookie returns a non-401/423 response.
- Setup route has no test covering the "already set up" scenario.
- Brief white flash visible on page load when dark mode is selected.
- The calendar component (Schedule-X) shows light colors while the rest of the app is dark.
- iOS status bar color does not match the active theme.
- Deploying a dark mode CSS update does not reach users until they manually clear the service worker cache.
**Phase to address:**
Setup wizard phase. The guard must be the first thing implemented; test the guard before testing the happy path.
PWA dark mode phase. The FOWT fix (inline script in `index.html`) and Schedule-X variable mapping must be in the acceptance criteria. Test on both light and dark system preferences. Test with the service worker active (not disabled).
---
### Pitfall 9: Admin Role Check Bypassed by Missing Middleware Wiring
### Pitfall 9: OAuth Callback Through Pangolin/Newt Tunnel — Redirect URI Mismatch and State Cookie Collision
**What goes wrong:**
The admin Settings routes require a role check (only the operator/admin user can manage credentials and toggle `is_shared`). The standard pattern in this codebase is Hono middleware layered on a route prefix. The failure mode: the admin middleware is defined but not wired to the correct prefix. For example, if the admin check is added to `eventsRouter` instead of a new `adminRouter`, or if the route is mounted at `/api/admin` but the middleware guard applies to `/api/settings/*`, admin routes are reachable by any authenticated member.
The Google OAuth flow requires registering exact redirect URIs in the Google Cloud Console. The Pangolin/Newt tunnel exposes the app at a public hostname (e.g., `familysync.example.com`). The registered redirect URI must match exactly: `https://familysync.example.com/api/auth/google/callback`. If the redirect URI in the Google Cloud Console is `http://` instead of `https://`, or includes/omits a trailing slash, or uses the internal hostname instead of the Pangolin hostname, the callback returns `redirect_uri_mismatch` from Google.
In a two-person household this is low-severity (both members are trusted), but the `is_shared` toggle can break the whole calendar display for both members if set incorrectly, and the credential management can overwrite the other member's app password.
A second issue: the existing Authelia OIDC flow already uses an OAuth state cookie (`oidc.state`) managed by `@hono/oidc-auth` (stored as a cookie on the domain). The Google OAuth flow also requires a `state` parameter to prevent CSRF. If both flows use the same cookie name, or if the `@hono/oidc-auth` middleware intercepts the Google OAuth callback URL (because it pattern-matches `/api/*` for auth), the Google callback's `code` parameter is consumed by the Authelia handler before the Google handler sees it — resulting in a `state mismatch` or `invalid_grant` error.
A third issue: the Pangolin/Newt tunnel may terminate TLS and forward HTTP internally. If the API receives the Google callback over HTTP (forwarded internally), but the `redirect_uri` registered in Google is `https://`, Google's OAuth server rejects the authorization code exchange with `redirect_uri_mismatch` because the `redirect_uri` in the token exchange request is constructed from the internal HTTP URL, not the external HTTPS URL.
**Why it happens:**
Hono's middleware scoping is based on route prefix at mount time, not at route definition time. A middleware added with `app.use('/api/admin/*', adminGuard)` does not protect routes mounted under `app.route('/api/admin', adminRouter)` unless the `adminRouter` itself also applies the guard. It is easy to apply the middleware in one place and assume it covers the route, but Hono's `.route()` creates an isolated sub-app.
Two OAuth flows in one app (Authelia OIDC for user login + Google OAuth for calendar access) use overlapping mechanisms (cookies, state params, callback routes). The Pangolin/Newt HTTPS termination creates an HTTP-internally-but-HTTPS-externally environment where naive URL construction picks the wrong scheme.
**How to avoid:**
Apply the admin middleware inside `adminRouter` itself (`.use('*', adminGuard)`), not only in the parent app. Write an integration test that calls a settings route as a non-admin authenticated user and asserts 403. Do not rely on the parent app's middleware order for sub-app security.
Mount the Google OAuth callback at a distinct path that the Authelia `@hono/oidc-auth` middleware does not intercept: `/api/providers/google/callback` (not under `/api/auth/`). Apply the Authelia OIDC middleware only to routes that need a logged-in session, not as a global catch-all for `/api/*`. The Google callback handler does not need an Authelia session — it needs the Google state param. Apply authentication to the Google OAuth _initiation_ endpoint (user must be logged in to connect Google), but not to the callback endpoint (callback must be reachable without a session because Google redirects there).
Use distinct state cookie names: the Authelia flow uses `oidc.state` (managed by `@hono/oidc-auth`); the Google flow should use `google.oauth.state` (managed by the Google OAuth handler). Store the state in a `HttpOnly; SameSite=Lax; Secure` cookie with a 10-minute TTL.
For the HTTPS scheme: read the external hostname from a `EXTERNAL_BASE_URL` environment variable (e.g., `https://familysync.example.com`) and construct all OAuth redirect URIs from it. Never derive the redirect URI from `req.headers.host` or `req.protocol` — the internal forwarded values will be wrong. Add `EXTERNAL_BASE_URL` to the required env checklist in the zero-setup wizard.
Register the redirect URI in Google Cloud Console as exactly `${EXTERNAL_BASE_URL}/api/providers/google/callback`.
**Warning signs:**
- Any authenticated user can reach `/api/admin/...` routes without an admin check in the response.
- The admin middleware is defined in `index.ts` but the admin routes are in a separate `adminRouter` with no internal middleware.
- Google OAuth callback returns `redirect_uri_mismatch`.
- After completing the Google consent screen, the app returns to the homepage without connecting the account (silent failure).
- Authelia OIDC errors appear in logs during a Google OAuth flow (state cookie collision).
- The Google callback URL in server logs shows `http://` instead of `https://`.
**Phase to address:**
Admin Settings phase. Integration test for 403 on non-admin access is the acceptance criterion.
Self-service Google OAuth onboarding phase. The `EXTERNAL_BASE_URL` env var and the distinct callback path must be established before any OAuth flow is implemented. Test the full flow end-to-end over the Pangolin tunnel (not just localhost) as the acceptance gate.
---
### Pitfall 10: App Password and VAPID Keys Stored in DB When They Must Stay in Env
### Pitfall 10: Google OAuth Token Storage — Encryption Key Reuse and Scope Creep
**What goes wrong:**
The setup wizard collects VAPID keypair and validates the Fastmail app password. A tempting shortcut: store the VAPID keys in the `settings` DB table for easy retrieval later. The problem: `VAPID_PRIVATE_KEY` is a signing key — equivalent to a private TLS key. Storing it in the DB means it is:
The Google OAuth access token (short-lived, ~1 hour) and refresh token (long-lived) must be stored securely. The existing `member_credentials` table stores the Fastmail app password encrypted with `APP_PASSWORD_ENCRYPTION_KEY` (AES-256-GCM). The tempting approach: store the Google refresh token in the same `member_credentials` table with the same encryption key, adding a `google_refresh_token` column.
- Accessible to anyone with DB read access (including `SELECT *` from a misconfigured tool or a Drizzle Studio session left open).
- Included in DB backups, which may be stored less securely.
- Returned by any accidental DB dump to logs.
The first issue: `provider_type` is already `UNIQUE(user_id)` on `member_credentials` (enforced by `uniq_member_credential_user`). A member with both a Fastmail credential and a Google credential would need two rows but the unique constraint allows only one per user. Attempting to upsert the Google credential overwrites the Fastmail credential, breaking CalDAV.
`APP_PASSWORD_ENCRYPTION_KEY` must never enter the DB at all — it is the key that encrypts everything else. If the wizard stores it in the DB "just for display/verification," the entire encryption model is broken.
The second issue: the Google access token should not be stored persistently — it expires in 1 hour and must be refreshed on use. If the code stores the access token as well as the refresh token, the stored access token will always be stale and the code must always call the token refresh endpoint anyway. Storing the access token wastes a column and tempts future code to use it without checking expiry.
The third issue: the Google OAuth scope should be the minimum needed. If the developer uses `https://www.googleapis.com/auth/calendar` (full calendar access) instead of the split `calendar.readonly` + `calendar.events` scopes, Google requires manual verification review for production status. Use the minimum scope needed.
**Why it happens:**
The wizard naturally wants to show "current configuration" and make it editable. Pulling values from env vars in a form feels awkward; storing in DB feels clean. The distinction between "secret that must stay in env" and "config that can live in DB" gets blurred.
The `member_credentials` unique constraint and the `provider_type` discriminator were designed for a single provider per user. Multi-provider support requires rethinking the credential storage schema.
**How to avoid:**
Hard rule: `VAPID_PRIVATE_KEY` and `APP_PASSWORD_ENCRYPTION_KEY` never touch the DB. They are validated in the wizard by attempting an operation (test encrypt/decrypt, test push send), not by reading or writing their values. `VAPID_PUBLIC_KEY` and `VAPID_SUBJECT` can be stored in DB (they are not secrets). Fastmail app passwords are stored encrypted (AES-256-GCM via `encryptPassword`), which is already implemented.
Change the `member_credentials` schema for v1.2: drop the `UNIQUE(user_id)` constraint and replace it with `UNIQUE(user_id, provider_type)`. This allows one row per provider per user. The migration is additive (drop unique, add composite unique). Verify with `drizzle-kit generate` that the generated SQL is `DROP INDEX uniq_member_credential_user; ADD UNIQUE KEY uniq_member_credential_user_provider (user_id, provider_type)` — additive DDL plus an index drop. The index drop is safe on MariaDB (not a TRUNCATE).
The wizard's "check env" validation path: call `encryptPassword('test')` — if it throws, `APP_PASSWORD_ENCRYPTION_KEY` is missing or malformed. Call `webpush.setVapidDetails(...)` and catch throws. Never read the key values out of `process.env` into a response body.
Store only the Google refresh token (not the access token). Add a `google_refresh_token_encrypted` column (or a generic `oauth_refresh_token_encrypted` column) to the Google `provider_type` row. Derive fresh access tokens at request time using the refresh token — cache the access token in memory (not in the DB) with its expiry time.
Use scopes: `https://www.googleapis.com/auth/calendar.readonly` + `https://www.googleapis.com/auth/calendar.events`. Request them space-separated. Store the granted scopes alongside the refresh token so the app knows what the user consented to.
**Warning signs:**
- DB schema has a `vapid_private_key` column.
- Any API response that includes `VAPID_PRIVATE_KEY` or `APP_PASSWORD_ENCRYPTION_KEY` values.
- Wizard stores all config to DB and reads it back on next startup instead of requiring env vars.
- An attempt to add a second `member_credentials` row for the same user hits a unique constraint violation.
- `member_credentials` for a user shows `provider_type: 'google'` but the CalDAV credential is gone.
- The access token column in the DB is always expired.
- Google authorization flow requests the `https://www.googleapis.com/auth/calendar` scope (full access, not split).
**Phase to address:**
Setup wizard phase. Schema design review before migration is written. Secret-in-DB is a hard blocker for the phase gate.
Provider abstraction phase (schema migration plan) and Google OAuth onboarding phase. The schema change to `UNIQUE(user_id, provider_type)` must land before any Google credential storage code is written.
---
### Pitfall 11: Gitea Actions MariaDB Service Container Readiness Race
### Pitfall 11: Dependency Updates — ESLint 10 Pin and Breaking Change Cascade
**What goes wrong:**
Gitea Actions (like GitHub Actions) supports `services:` containers. The MariaDB service starts, but the container reaching `healthy` in Docker does not mean MariaDB is accepting connections on port 3306. `mysqld` takes several seconds to initialize after the container starts. If the CI job proceeds to `drizzle-kit migrate` or integration test commands immediately after the service health check passes, it races with MariaDB initialization and fails with `ECONNREFUSED` or `Access denied` errors that look like test failures but are actually timing issues.
The CI dependency audit (Phase 16) surfaces `outdated` packages. Bulk-applying `pnpm up` or `pnpm audit fix` will attempt to update ESLint to v10, which breaks `eslint-plugin-react` and `eslint-plugin-react-hooks` (both declare only `eslint@9.x` as a peer dependency as of 2026-06). When ESLint 10 is installed, both plugins either throw `EBADPEERCONN` at install time or emit no errors at lint time (silently disabled), breaking the CI lint gate.
A secondary risk: `@schedule-x/calendar` and `@schedule-x/theme-default` have been updated (currently pinned at 4.6.0 in `apps/pwa`). Schedule-X 5.x (if released) changes the `--sx-color-*` variable naming. If the dependency update phase bumps Schedule-X to a major version before the dark mode phase, the dark mode variable mapping work must be redone with the new variable names.
A third risk: `drizzle-kit` and `drizzle-orm` minor versions must be updated together. The `drizzle-orm/mysql2` import path and the `drizzle-orm/migrator` API are version-coupled. If `drizzle-orm` is bumped to 0.46.x but `drizzle-kit` stays at 0.31.x, the snapshot format version mismatch may cause `generate` to produce an incompatible snapshot that `migrate` cannot parse.
**Why it happens:**
The Docker `HEALTHCHECK` for MariaDB using `mysqladmin ping` returns true as soon as the network socket is open, which happens before all privilege tables are initialized. The `healthcheck.interval` in the Gitea service definition controls how often the check runs, but the first check may pass before MariaDB has fully bootstrapped.
`pnpm up` or `pnpm audit --fix` applies updates greedily across the dependency tree without respecting known breaking-change boundaries. The project already knows about the ESLint 9.x pin but a future developer (or an automated tool) may not.
**How to avoid:**
Add a `wait-for-it` or `until mysqladmin ping --silent; do sleep 1; done` step in the CI workflow after the service is declared healthy, before running any DB command. Or use a longer `healthcheck.start_period` in the service definition (e.g., 30 seconds). Also: set `MARIADB_ROOT_PASSWORD`, `MARIADB_DATABASE`, `MARIADB_USER`, `MARIADB_PASSWORD` in the service env and use those same credentials in the integration test step — do not assume the root user is reachable from the test runner without a password.
Update dependencies manually, one package or one ecosystem at a time, with the CI lint gate running after each batch. Never use `pnpm up --latest` or `pnpm audit --fix` without reviewing the changeset.
ESLint: hold at 9.x until `eslint-plugin-react` and `eslint-plugin-react-hooks` both publish ESLint 10 peerDep support and the release notes confirm compatibility. Add a comment in the root `package.json` above the `"eslint"` entry: `// pinned at 9.x — eslint-plugin-react does not support eslint@10 yet`.
Schedule-X: update minor versions only (4.x → 4.y). Hold at major 4 until dark mode theming is complete and the variable mapping is locked. Check Schedule-X release notes for `--sx-color-*` variable changes before any minor update.
Drizzle: always update `drizzle-orm` and `drizzle-kit` in the same commit. After updating, run `pnpm db:generate` in a dry-run (the output must be "No schema changes, nothing to migrate") to verify the snapshot format compatibility.
**Warning signs:**
- CI passes on re-run but fails on first run of a PR (timing-dependent).
- `ECONNREFUSED` or `Error: connect ECONNREFUSED 127.0.0.1:3306` in CI logs.
- Tests that pass locally with a warm MariaDB fail in CI cold-start.
- ESLint exits 0 (no errors reported) but previously failing lint rules now pass — plugin silently disabled.
- `eslint-plugin-react@X requires eslint@">=9.0.0 <10.0.0" but got eslint@10.x.x` in pnpm install output.
- `drizzle-kit generate` emits a snapshot with a different `version` field than the existing snapshots in `meta/`.
- Schedule-X calendar renders with missing styles after a version bump.
**Phase to address:**
Gitea CI phase. The readiness wait must be in the first draft of the workflow YAML; do not add it after the first CI failures.
---
### Pitfall 12: Gitea Actions Self-Hosted Runner Missing Node 22 or pnpm
**What goes wrong:**
The self-hosted Gitea Actions runner on Unraid may have an older Node.js version globally available, or may have no `pnpm` installation, or may have a `corepack`-managed pnpm that requires activation. If the CI workflow assumes the runner environment matches the dev machine, `pnpm install` fails with `pnpm: command not found`, or `node --version` returns 18 instead of 22.
A related issue: the workflow may use `actions/setup-node` (a GitHub Actions action) which is not available in Gitea Actions, or uses a Gitea-specific variant that requires different configuration.
**Why it happens:**
Gitea Actions is not GitHub Actions. Many popular actions (`actions/checkout`, `actions/setup-node`, `actions/cache`) have Gitea-compatible alternatives, but their names and behavior differ subtly. If the workflow is copied from a GitHub Actions template, some steps silently fail or are skipped.
**How to avoid:**
In the first CI plan, write a minimal "hello world" workflow that only checks `node --version` and `pnpm --version`. Verify it passes before adding any test steps. Use `actions/setup-node` only if confirmed compatible with the specific Gitea version; otherwise install Node and pnpm explicitly in the workflow using `wget` / `npm install -g pnpm`. Pin the Node version to `22.x` explicitly; do not rely on the runner default.
For Docker image build/publish: verify the runner has Docker daemon access. On Unraid self-hosted runners, Docker may require `--privileged` or specific socket mounts that need runner configuration.
**Warning signs:**
- `pnpm: command not found` in CI output.
- `node` resolves to a version older than 22 in CI but not locally.
- `actions/setup-node` step shows as skipped or errored in the Gitea Actions UI.
**Phase to address:**
Gitea CI phase. The runner environment probe must be the first CI task — before any test or build steps are designed.
---
### Pitfall 13: Docker Registry Push Token Scope Exposes Secrets in Logs
**What goes wrong:**
The Gitea CI Docker build/publish step requires credentials for the Docker registry (Docker Hub, Gitea's own container registry, or a self-hosted registry). If the registry token is passed as a `docker login` argument on the command line (e.g., `docker login -u $USER -p $TOKEN`), the token appears in the process list, in the Gitea Actions job log (if command echo is on), and in any runner audit logs. Gitea Actions supports `secrets:` but if the workflow uses `run: docker login -p ${{ secrets.REGISTRY_TOKEN }}`, the secret is masked in the log only if the secret was registered correctly — unregistered secrets are echoed verbatim.
**Why it happens:**
Docker CLI login via `-p` flag is the most common example in docs. GitHub Actions masks secrets automatically; Gitea Actions masks them only for registered secrets. A token from a CI environment variable that was not added through the Gitea Secrets UI is not masked.
**How to avoid:**
Use `docker login --password-stdin` with the token piped via stdin rather than a command-line argument: `echo "${{ secrets.REGISTRY_TOKEN }}" | docker login -u "${{ secrets.REGISTRY_USER }}" --password-stdin registry.example.com`. Register all credentials as Gitea repository secrets, not as environment variables in the workflow YAML. Verify the Gitea version supports secret masking in the Actions log (Gitea ≥ 1.19 for Actions support; secret masking behavior varies by version).
**Warning signs:**
- Registry token or password visible as plaintext in the Gitea Actions job log.
- `docker login` command line includes `-p <token>` in the log output.
- `secrets.REGISTRY_TOKEN` is undefined in the workflow (token was set as env var, not secret).
**Phase to address:**
Gitea CI phase. Credential handling review before any Docker push step is added.
---
### Pitfall 14: Playwright Authed-Mobile Harness Reusing a Stale storage-state
**What goes wrong:**
The mobile-emulated Playwright harness uses a saved `storage-state.json` (cookies + localStorage) to bypass the OIDC login flow. If the storage state was captured with a real Authelia session, it contains a session cookie with a finite TTL (typically 1 hour for `@hono/oidc-auth` JWT cookies, or the Authelia session lifetime). After the TTL expires, all Playwright runs with the stale storage state silently fail at the first `/api/*` call — the OIDC middleware redirects to Authelia, and the test gets an HTML login page instead of the expected JSON API response. The test may still pass if it only checks DOM content (which may be the redirected page's HTML), or it may produce a false-positive assertion on the 302 redirect.
**Why it happens:**
Playwright storage state is file-based and not automatically refreshed. Developers capture it once and check it in (or store it locally), then forget to renew it. In `DEV_AUTH_BYPASS=true` mode this does not apply (no session cookie needed), but if the harness is meant to test the production auth path, the bypass is not active and the session cookie must be valid.
**How to avoid:**
Do not use a static stored storage state for tests that run against the production OIDC path. Instead, implement a programmatic login helper that runs the OIDC authorization code flow at the start of each test session (or once per test run) and stores the resulting session. For the `DEV_AUTH_BYPASS` dev environment, the harness sets `DEV_AUTH_BYPASS=true` and skips the storage state entirely. The mobile viewport emulation does not require real OIDC — use `DEV_AUTH_BYPASS` for the automated harness; keep real OIDC tests as manual/human gates.
**Warning signs:**
- Playwright runs fail with `Expected 200 OK but got 302 Found` after leaving the storage state untouched for more than one day.
- Tests that exercise `/api/*` routes return HTML (the Authelia login page) instead of JSON.
- The same test suite passes reliably in `DEV_AUTH_BYPASS=true` mode but fails intermittently in production-auth mode.
**Phase to address:**
Mobile-browser testing phase. The storage state strategy must be decided before the first test is written — programmatic refresh or bypass-only.
---
### Pitfall 15: Production Service Worker Intercepting Playwright Requests
**What goes wrong:**
The installed Vite PWA service worker (`sw.js`) is registered in the browser when the PWA is visited. Playwright's Chromium instance can load and activate the service worker from a previous test run (persisted in the browser's profile directory). On subsequent test runs, the service worker intercepts API calls — potentially returning cached responses from the previous run rather than making network requests to the test server. This causes:
- API requests returning stale 200 responses when the test server is not running.
- `queryClient.invalidateQueries` not triggering new network requests (SW returns cached response).
- Tests that verify freshly-created data returning old data.
**Why it happens:**
Workbox's cache-first strategy for static assets and stale-while-revalidate for API routes persist across browser sessions in the Playwright profile. A new Playwright context does not clear the service worker registration unless explicitly reset.
**How to avoid:**
Use `browserContext.clearCookies()` and `browserContext.clearPermissions()` in the test setup, but also explicitly unregister service workers: `await page.evaluate(() => navigator.serviceWorker.getRegistrations().then(r => Promise.all(r.map(sw => sw.unregister()))))` before any navigation. Or launch Playwright with `serviceWorkers: 'block'` in the context options, which prevents the SW from intercepting requests entirely. For tests that specifically test offline/SW behavior, use a separate context without the block.
**Warning signs:**
- Network tab in Playwright traces shows `(ServiceWorker)` as the response source.
- Tests pass on a clean browser profile but fail on a profile that has visited the PWA before.
- API requests complete instantly with stale data in the Playwright trace.
**Phase to address:**
Mobile-browser testing phase. The Playwright context setup must explicitly handle service worker state before the first test is written.
CI dependency update phase (last in milestone, after all feature phases). Treat each package ecosystem as a separate update batch. Run the full CI suite after each batch.
---
## Technical Debt Patterns
| Shortcut | Immediate Benefit | Long-term Cost | When Acceptable |
| -------------------------------------------------------------------------------- | ---------------------------- | --------------------------------------------------------------- | -------------------------------------------------------------------- |
| Building VALARM on top of `buildVeventString` without the preserve-on-edit path | Faster to implement | Strips native-client alarms on every edit; user data loss | Never — preserve path must ship with VALARM authoring |
| uid-only dedup key in sentReminders when lead times become variable | No migration needed | Duplicate pushes or missed re-fires after reschedule | Never for production; acceptable in tests with a fixed lead |
| Calling `runOutboxDrain()` directly from event trigger instead of setting a flag | Simpler code | Bypasses `isDraining` atomicity, potential double-drain | Never — always funnel through the single setInterval-controlled path |
| Setup wizard that accepts `APP_PASSWORD_ENCRYPTION_KEY` via the API | Simpler UX for initial setup | Entire encryption model is broken | Never — key stays in env/secrets only |
| Static storage-state.json checked into the repo | Zero-effort Playwright auth | Tests fail silently after TTY expiry; potential credential leak | Never — programmatic refresh or DEV_AUTH_BYPASS only |
| `docker login -p $TOKEN` in CI command | Quick to write | Token appears in CI logs if secret not masked | Never — always use --password-stdin |
| No readiness wait for MariaDB service in CI | Simpler YAML | Flaky CI: timing-dependent ECONNREFUSED failures | Never — readiness wait is 3 lines and prevents ghost failures |
| Shortcut | Immediate Benefit | Long-term Cost | When Acceptable |
|----------|-------------------|----------------|-----------------|
| Keeping `UNIQUE(user_id)` on `member_credentials` for the Google credential | No schema migration needed | Can only store one provider per user; Google credential overwrites Fastmail credential | Never — change to `UNIQUE(user_id, provider_type)` before any Google credential storage |
| Storing Google access tokens persistently in the DB | Avoids token refresh logic | Access tokens expire in 1 hour; all stored values are always stale | Never — cache in memory only; persist only the refresh token |
| Using `singleEvents: true` for Google Calendar event fetch | Simpler API call | Cannot reconstruct RRULE; recurring events appear as individual orphan events | Never — use `singleEvents: false` for the FamilySync data model |
| Skipping the `migration_lock` sentinel table for auto-migrate | One fewer migration table | Concurrent startup races cause `Duplicate column name` crash loop | Never for a production deployment; acceptable in single-instance local dev |
| Dark mode applied via React state (after hydration) | Simpler code | Flash-of-wrong-theme on every cold load | Never — use inline `<script>` in `index.html` to set the theme synchronously |
| Deriving OAuth redirect URI from `req.headers.host` | No env var to configure | Returns internal hostname through Pangolin tunnel; causes `redirect_uri_mismatch` | Never — read from `EXTERNAL_BASE_URL` env var |
| Bumping ESLint to v10 with other dependencies | Fewer outdated warnings | `eslint-plugin-react` is disabled silently; lint gate appears green but catches nothing | Never until eslint-plugin-react publishes v10 support |
| Not handling Google `410 Gone` (expired syncToken) | Simpler sync code | All incremental sync halts permanently after 7 days of inactivity | Never — 410 is a documented, expected Google API response |
---
## Integration Gotchas
| Integration | Common Mistake | Correct Approach |
| ------------------------- | ----------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------- |
| ical.js VALARM | Using `addPropertyWithValue('trigger', '-PT15M')` (string) | Build `ICAL.Duration.fromSeconds(-N*60)` and use the Duration object as the property value |
| ical.js VALARM | Not round-tripping preserved VALARMs through ical.js parse→serialize | Parse the sub-component from rawVevent and re-add via ical.js API; do not insert raw text |
| outboxWorker VALARM | Rebuilding VEVENT from scratch drops native-client VALARMs on update | Extend buildVeventString to accept a `valarms` parameter; populate from rawVevent extract on update path |
| reminderScheduler dedup | uid-only Map key breaks when per-event leads vary | Key on `uid + ':' + dtstartMs`; prune by dtstartMs |
| event-driven drain | Calling `runOutboxDrain()` from pub/sub subscriber before `isDraining` is set | Use a single drain path via `drainRequested` flag checked in the setInterval callback |
| Gitea Actions | Using GitHub Actions-specific action IDs | Probe the runner first; use Gitea-compatible alternatives or install tools explicitly |
| Gitea Actions MariaDB | Relying on container health == connection ready | Add explicit `mysqladmin ping` retry loop after healthcheck passes |
| Playwright mobile harness | Static storage-state.json with expiring session cookie | Use `DEV_AUTH_BYPASS=true` for automated harness; programmatic OIDC login for real-auth tests |
| Playwright + Vite PWA | Service worker from previous run intercepting requests | Set `serviceWorkers: 'block'` or unregister SWs explicitly in test context setup |
| Setup wizard | Accepting `APP_PASSWORD_ENCRYPTION_KEY` via the POST body | Validate the env is present by performing a test operation; never accept the key value over the network |
| Admin settings route | Zod error passthrough leaking app-password input | Custom `hook` in zod-validator: return generic 400, never the Zod error object |
| Integration | Common Mistake | Correct Approach |
|-------------|----------------|------------------|
| Google Calendar API | `singleEvents: true` in events.list | Use `singleEvents: false`; fetch modified instances separately |
| Google Calendar API | Not handling `status: "cancelled"` instances | Map cancelled instances to EXDATE on the master RRULE |
| Google Calendar API | Ignoring `410 Gone` on syncToken expiry | Catch 410, clear token, enqueue full re-sync |
| Google Calendar API | Using `https://www.googleapis.com/auth/calendar` scope | Use `calendar.readonly` + `calendar.events` (non-restricted, no Google verification required) |
| Google OAuth | Constructing redirect URI from `req.protocol`/`req.hostname` | Read from `EXTERNAL_BASE_URL` env var; Pangolin terminates TLS so internal protocol is HTTP |
| Google OAuth | Sharing state cookie name with `@hono/oidc-auth` Authelia flow | Use `google.oauth.state` cookie; mount Google callback outside the Authelia middleware scope |
| Google refresh token | Storing access tokens in DB | Cache access tokens in memory with expiry; persist only refresh token (encrypted) |
| Drizzle auto-migrate | Calling `drizzle-kit push` programmatically at startup | Use `drizzle-orm/migrator` `migrate()` API — applies committed SQL files, never introspects live DB |
| Drizzle auto-migrate | No concurrent-startup protection | Implement `migration_lock` sentinel row with `INSERT ... ON DUPLICATE KEY IGNORE` |
| member_credentials schema | `UNIQUE(user_id)` blocks multi-provider | Migrate to `UNIQUE(user_id, provider_type)` before writing Google credential code |
| Schedule-X dark mode | Not overriding `--sx-color-*` variables in dark token block | Enumerate all `--sx-*` variables from `@schedule-x/theme-default` CSS and map them |
| PWA dark mode | Setting `data-theme` in React (after hydration) | Inline `<script>` in `index.html` before CSS to set `data-theme` synchronously on load |
| Vite PWA service worker | `tokens.css` in `public/` without hash | Move theme CSS into hashed build output; verify Workbox precache manifest includes it |
---
## Performance Traps
| Trap | Symptoms | Prevention | When It Breaks |
|------|----------|------------|----------------|
| Google `events.list` with no `timeMin`/`timeMax` | Fetches all events in history; response > 5MB; rate limit hit | Always pass a bounded time window (e.g., 6 months past, 1 year future) | First sync for a calendar with >1000 events |
| Google instances fetch per recurring master event | N+1 API calls for N recurring events | Batch: fetch all instances in the time window in one call; merge by master event ID | Calendars with 10+ recurring series |
| MariaDB auto-migrate running complex migrations on startup | Long startup delays; health check fails before migration completes | Log migration progress; set Docker healthcheck `start_period` long enough | Migrations with `ALTER TABLE` on a table with >50K rows |
| Multiple VALARMs in scheduler SQL query without index | Full `calendar_events` scan on every scheduler tick | Index on `reminder_lead_minutes` column (or `event_reminders.lead_minutes` if junction table) | Tables with >10K events |
---
## Security Mistakes
| Mistake | Risk | Prevention |
| ----------------------------------------------------------------- | ----------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------ |
| Admin route not protected inside adminRouter (only in parent app) | Any authenticated member can call admin endpoints | Apply guard middleware inside the sub-router, not only in the parent app mount |
| Setup endpoint lacks "already-set-up" guard | Post-setup endpoint rewrites credentials without auth | Check `member_credentials` existence + VAPID env on every setup route invocation; return 423 if already configured |
| VAPID_PRIVATE_KEY stored in DB | Private signing key accessible to DB-level access | VAPID private key in env/secrets only; DB stores public key and subject only |
| App-password in Zod error response | Plaintext credential in HTTP response and server logs | Custom Zod hook for all routes that accept credential input |
| `docker login -p` in CI YAML | Registry token in CI logs | `--password-stdin` only; token as Gitea secret, not YAML env var |
| Mistake | Risk | Prevention |
|---------|------|------------|
| Google refresh token stored unencrypted | Stolen DB backup exposes ability to read all of a member's Google Calendar indefinitely | Encrypt with `APP_PASSWORD_ENCRYPTION_KEY` (AES-256-GCM) same as Fastmail app passwords |
| Google OAuth state param not validated on callback | CSRF: attacker can link their Google account to the victim's FamilySync session | Validate `state` against the `google.oauth.state` cookie; reject mismatches with 400 |
| Google callback endpoint accessible without session check on initiation | Any unauthenticated caller can initiate a Google OAuth flow for any user | Require authenticated FamilySync session to initiate the flow (`/api/providers/google/connect`); the callback itself needs no session but must validate the state cookie |
| Google scopes stored but not verified on callback | User may grant narrower scopes than requested; write-back fails silently | Check `scope` in the token response; if write scope is absent, mark the provider as read-only and surface in UI |
| `EXTERNAL_BASE_URL` exposed in a public API response | Discloses internal routing; low severity but unnecessary | Keep env vars server-side only; never return infrastructure config in API responses |
| Migration lock table not cleaned up on crash | Next startup finds a stale lock and never migrates | Set a `locked_at` timestamp in the lock row; auto-release locks older than 60 seconds |
---
## UX Pitfalls
| Pitfall | User Impact | Better Approach |
|---------|-------------|-----------------|
| Dark mode toggle has no system-follow option | Wife on iPhone expects dark mode to follow iOS system preference automatically | Implement three options: Light / Dark / System (default); System reads `prefers-color-scheme` media query and updates on system change |
| Google "Connect" button initiates OAuth without explaining what access is granted | Non-technical wife confused by Google consent screen listing calendar permissions | Show a one-sentence explanation before the OAuth redirect: "FamilySync will read and update your Google Calendar events." |
| Google auth failure surfaces a raw error message | `invalid_grant` or `redirect_uri_mismatch` visible to end user | Map known Google OAuth error codes to friendly messages; provide a "Try again" button that re-initiates the flow |
| Multiple reminders UI with no order or visual grouping | Hard to tell which reminder fires when | Show reminders sorted by lead time (earliest first); each entry shows the computed fire time ("4 days before, at 9:00 AM") |
| Dark mode flash on cold PWA load | Wife on iPhone sees white flash on dark mode | Inline script in `index.html` sets theme before first paint; no React wait required |
| Zero-setup wizard asks for Google OAuth credentials before explaining the App Flow | Non-technical user confused by "Client ID" and "Client Secret" terms | Name the fields plainly; link to the FamilySync deployment guide step that shows where to find them in the Google Cloud Console |
---
## "Looks Done But Isn't" Checklist
- [ ] **VALARM authoring:** Often ships create-only — verify that editing an event with a native-client alarm in rawVevent does not drop that alarm from the PUT payload.
- [ ] **VALARM serialization:** Often emits `VALUE=TEXT` — verify the ICS output has `TRIGGER:-PT15M` (DURATION type, no VALUE parameter) or `TRIGGER;VALUE=DURATION:-PT15M` — never `VALUE=TEXT`.
- [ ] **All-day reminders:** Often enabled in the UI for all-day events — verify the reminder selector is disabled or hidden when `allDay: true`.
- [ ] **Variable-lead dedup:** Often keeps the uid-only key — verify the dedup map key is updated to include dtstart so a rescheduled event fires again.
- [ ] **Event-driven drain:** Often calls `runOutboxDrain()` directly — verify the trigger path sets `drainRequested` or calls through the same error-caught wrapper as the setInterval path.
- [ ] **Setup wizard "already-set-up" guard:** Often untested — verify a second POST to any setup endpoint after initial setup returns 423, not 200.
- [ ] **Admin route 403:** Often not tested — verify a non-admin authenticated user gets 403 from admin routes, not 200 or 404.
- [ ] **VAPID key in DB:** Often slips in as "config" — verify the DB schema has no column for `vapid_private_key` or `app_password_encryption_key`.
- [ ] **Gitea CI MariaDB readiness:** Often assumed — verify CI logs show the mysqladmin ping retry loop completing, not the job proceeding immediately after service declared healthy.
- [ ] **Playwright storage state expiry:** Often passes on day one — verify tests still pass 25 hours after the storage state was captured (session cookie expired).
- [ ] **Google OAuth flow:** Refresh token stored — but 410 syncToken expiry handler missing. Verify with a forced syncToken reset (delete the stored token, check that the next poll triggers a full re-sync without crashing).
- [ ] **Google recurrence:** Master event fetched with `singleEvents: false` — but modified/canceled instances not fetched. Verify by canceling one occurrence in Google Calendar and confirming it does not appear in FamilySync.
- [ ] **Provider abstraction:** Interface defined — but the Fastmail outbox worker's `isDraining` guard and `drainRequested` flag behavior is unchanged. Verify with the existing outbox integration tests against the new interface.
- [ ] **Multiple reminders:** Multiple VALARM objects in `rawVevent` — but scheduler dedup key still `uid:dtstartMs` (not `uid:dtstartMs:leadMinutes`). Verify by creating an event with two reminders and confirming two separate pushes fire at the correct times.
- [ ] **Auto-migrate on boot:** Migrations run at startup — but no concurrent-startup protection. Verify by starting two container instances simultaneously and confirming exactly one migration run, no crash loop.
- [ ] **Dark mode:** `data-theme` applied — but Schedule-X `--sx-color-*` variables not overridden. Verify by switching to dark mode and checking the calendar view specifically (not just the nav/lists).
- [ ] **Google OAuth through Pangolin:** OAuth flow works on localhost — but redirect URI uses internal hostname. Verify by completing the Google OAuth flow over the Pangolin public URL (`EXTERNAL_BASE_URL`), not via `localhost:3000`.
- [ ] **Dependency updates:** ESLint updated — but `eslint-plugin-react` compatibility not verified. Verify by introducing a deliberate React lint violation and confirming the CI lint gate catches it after the update.
- [ ] **Google token expiry:** Access tokens cached in memory — but expiry check missing. Verify that a request made >1 hour after the last token fetch triggers a silent refresh, not a 401 from Google.
---
## Recovery Strategies
| Pitfall | Recovery Cost | Recovery Steps |
| ------------------------------------- | ------------- | ------------------------------------------------------------------------------------------------------------------------------------------ |
| VALARM strips native alarms on edit | MEDIUM | Add valarms preserve path to buildVeventString + outboxWorker update branch; no migration needed; existing rawVevent data is authoritative |
| TRIGGER VALUE=TEXT bug | LOW | Fix Duration construction in buildVeventString; no data migration (rawVevent already has correct alarms from server) |
| uid-only dedup causing duplicate push | LOW | Change Map key to uid:dtstartMs; restart clears in-memory state; no DB change |
| Double-drain from concurrent triggers | MEDIUM | Refactor event-driven trigger to drainRequested flag; requires load testing to confirm no more duplicate PUTs |
| Admin route bypassed (no inner guard) | LOW | Add `.use('*', adminGuard)` inside adminRouter; deploy |
| VAPID private key in DB | HIGH | Rotate VAPID keypair; clear all push subscriptions (all devices must re-subscribe); remove DB column via migration |
| CI flaky MariaDB race | LOW | Add readiness wait loop to workflow YAML; re-run |
| Playwright storage state stale | LOW | Switch to DEV_AUTH_BYPASS mode for automated tests; remove static state file |
| Pitfall | Recovery Cost | Recovery Steps |
|---------|---------------|----------------|
| Google refresh token 7-day expiry in Testing status | LOW | Promote app to Production status in Google Cloud Console; member re-authorizes once |
| Drizzle auto-migrate `Duplicate column name` crash | MEDIUM | Manually mark the migration as applied in `__drizzle_migrations`; restart container |
| Concurrent migration lock stuck (crash before lock release) | LOW | `DELETE FROM migration_lock WHERE id=1` in the DB; restart container |
| Google syncToken 410 — incremental sync stopped | LOW | Delete stored syncToken from `member_credentials`; next poll performs full re-sync |
| `member_credentials` unique constraint violation (Google overwrites Fastmail) | HIGH | Restore from DB backup; apply the `UNIQUE(user_id, provider_type)` migration immediately |
| OAuth redirect URI mismatch (Pangolin hostname not registered) | LOW | Add the correct HTTPS URL to Google Cloud Console Authorized Redirect URIs; no code change |
| Dark mode flash in production (inline script missing) | LOW | Add inline `<script>` to `index.html`; rebuild and deploy; service worker cache busted by new hash |
| ESLint 10 upgrade breaks plugin — lint gate silently passes | MEDIUM | Roll back ESLint to 9.x in root `package.json`; re-run CI to confirm lint gate fails again on known violations |
---
## Pitfall-to-Phase Mapping
| Pitfall | Prevention Phase | Verification |
| ------------------------------------------------- | ---------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------- |
| VALARM strips native alarms on edit | Per-event reminders (VALARM authoring) | Integration test: create event via native client with alarm, edit via FamilySync, verify PUT payload contains original VALARM |
| TRIGGER VALUE=TEXT serialization | Per-event reminders (VALARM authoring) | Unit test: serialize VALARM, parse back, assert no VALUE=TEXT |
| All-day event VALARM silently no-ops | Per-event reminders (VALARM authoring) | UI test: all-day event form has no reminder field or field is disabled |
| Variable-lead dedup produces duplicate push | Per-event reminders (scheduler generalization) | Unit test: fire reminder, reschedule event earlier, fire again — assert two pushes sent |
| Double-drain from concurrent event-driven trigger | Event-driven outbox drain | Load test: enqueue 10 rows rapidly, assert each CalDAV PUT issued exactly once |
| Event-driven drain breaks create-before-delete | Event-driven outbox drain | Integration test: edit-as-move under rapid enqueue; original event not deleted before new one created |
| Admin app-password echoed in error | Admin Settings | Unit test: POST invalid password to settings route; assert response has no credential value |
| Unauthenticated setup endpoint stays live | Setup wizard | Integration test: POST to setup endpoint after first-run completes; assert 423 |
| Admin role check missing inside sub-router | Admin Settings | Integration test: non-admin authenticated user hits admin route; assert 403 |
| VAPID key stored in DB | Setup wizard | Schema review before migration is written; CI lint check for column names containing `private_key` |
| Gitea CI MariaDB readiness race | Gitea CI | CI log audit: readiness loop appears before any `drizzle-kit migrate` invocation |
| Gitea runner missing Node 22 / pnpm | Gitea CI | First CI job: node/pnpm version probe step before any install or test |
| Docker registry token in CI logs | Gitea CI | CI log audit: no plaintext token visible; all registry credentials use --password-stdin |
| Playwright storage state stale | Mobile-browser testing | Test suite passes on day 2 without recapturing storage state (DEV_AUTH_BYPASS mode eliminates TTL) |
| Production service worker intercepts Playwright | Mobile-browser testing | Playwright context uses `serviceWorkers: 'block'`; verified in trace that no responses are SW-sourced |
| Pitfall | Prevention Phase | Verification |
|---------|------------------|--------------|
| Google refresh token 7-day expiry (Testing vs Production status) | Google Calendar provider (phase start) | Document and enforce before any token storage code ships |
| Google recurrence model mismatch (singleEvents, EXDATE) | Google Calendar provider (recurrence plan) | Unit test: mock Google API response → assert reconstructed VCALENDAR passes ical.js + RecurExpansion |
| Google syncToken 410 handling | Google Calendar provider (sync/poller plan) | Integration test: simulate 410 → assert full re-sync triggered, no crash |
| Google all-day timezone / useDefault reminder | Google Calendar provider (normalization plan) | Unit test: UTC normalization of `start.dateTime` with offset; `useDefault: true` maps to null |
| Provider abstraction Fastmail regression | Provider abstraction phase (must ship first) | All existing outbox/poller/scheduler tests pass unchanged against the new interface |
| Multiple VALARM dedup key and Google 5-cap | Multiple reminders phase | Integration test: event with 2 reminders fires 2 pushes; Google write-back with 6 reminders returns validation error before write |
| Drizzle auto-migrate: journal mismatch, concurrent race, false destructive diff | Zero-setup DB migration phase | Test: run `migrate()` twice on same DB → no error; start two containers simultaneously → one applies, one waits |
| PWA dark mode FOWT and Schedule-X coverage | PWA dark mode phase | Playwright test: toggle dark mode, assert `--sx-color-*` computed values on the calendar element match dark theme |
| OAuth callback through Pangolin redirect URI | Self-service Google OAuth onboarding phase | End-to-end test over Pangolin tunnel hostname (not localhost) |
| Google token encryption and multi-provider schema | Provider abstraction phase (schema migration) | `UNIQUE(user_id, provider_type)` in place; integration test: add Fastmail + Google credentials for same user → both rows present |
| ESLint 10 breaking change in dependency updates | CI dependency update phase (last in milestone) | Update ESLint separately; verify lint gate still catches a known violation after update |
---
## Sources
- Direct inspection of `apps/api/src/broker/outboxWorker.ts`, `reminderScheduler.ts`, `vevent.ts`, `crypto.ts`, `index.ts`, `db/schema.ts`
- `.planning/RETROSPECTIVE.md` — v1.0 lessons: node-cron skip, drizzle push destructive diff, VAPID truncation, tsc vs vitest divergence
- `CLAUDE.md` memory entries: `node-cron-skips-in-long-running-process.md`, `drizzle-mariadb-push-unsafe.md`, `authelia-idtoken-claims.md`
- RFC 5545 §3.8.6.3 — VALARM TRIGGER value types (DURATION vs DATE-TIME)
- RFC 5545 §3.3.10 — RRULE value type semantics
- ical.js source (`lib/ical/property.js`) — property value type inference for TRIGGER
- Gitea Actions documentation — services container healthcheck semantics, secret masking behavior
- Playwright docs — `browserContext.serviceWorkers`, `storageState`, context lifecycle
- Project source: `apps/api/src/broker/reminderScheduler.ts`, `vevent.ts`, `outboxWorker.ts`, `client.ts` (inspected directly)
- Migration history: `apps/api/src/db/migrations/meta/_journal.json`, migration SQL files (inspected directly)
- `.planning/RETROSPECTIVE.md` — v1.0 and v1.1 lessons (journal-hash mismatch, false destructive diff, node-cron silent skip, runner-probe-first)
- `.planning/quick/260610-cr8-adopt-drizzle-generate-migrate-workflow-/260610-cr8-PLAN.md` — Drizzle MariaDB push foot-gun (direct source)
- `.planning/todos/completed/adopt-drizzle-migrations-workflow.md` — false destructive diff root cause (direct source)
- `.planning/milestones/v1.1-phases/10-admin-role-settings/10-01-PLAN.md` — migration workflow and `UNIQUE(user_id)` constraint history (direct source)
- `.planning/milestones/v1.1-phases/11-per-event-reminders/11-RESEARCH.md` — VALARM classification, scheduler dedup key design (direct source)
- `CLAUDE.md` — ESLint 9.39.4 pin rationale, stack constraints, Schedule-X version, CI runner constraints (direct source)
- `PROJECT.md` — Key decisions D-10 through D-20, constraint table, v1.2 feature list (direct source)
- Google Calendar API documentation knowledge (refresh token expiry policy, `singleEvents` parameter, `410 Gone` syncToken expiry, `reminders.useDefault`, scope tiers) — HIGH confidence from repeated confirmed behavior
- OAuth2 PKCE / state cookie mechanics — HIGH confidence from shipped Authelia OIDC implementation in this codebase
- Drizzle ORM programmatic migrator API (`drizzle-orm/migrator`) — MEDIUM confidence (verify against drizzle-orm release notes for the installed version before shipping auto-migrate)
---
_Pitfalls research for: FamilySync v1.1 Operability & Polish_
_Researched: 2026-06-10_
*Pitfalls research for: FamilySync v1.2 Multi-Provider, Theming & Zero-Setup*
*Researched: 2026-06-19*