docs: v1.1 research (stack/features/architecture/pitfalls/summary)
This commit is contained in:
+290
-340
@@ -1,405 +1,398 @@
|
||||
# Pitfalls Research
|
||||
# Pitfalls Research — v1.1 Operability & Polish
|
||||
|
||||
**Domain:** Fastmail-brokered family calendar + shared-list PWA with Web Push, self-hosted behind Authelia
|
||||
**Researched:** 2026-06-03
|
||||
**Confidence:** MEDIUM-HIGH (CalDAV/RRULE/iOS push well-documented in community; Fastmail-specific rate limits and JMAP calendar status confirmed from official docs)
|
||||
**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.
|
||||
|
||||
---
|
||||
|
||||
## Known Constraints (Do Not Re-Litigate)
|
||||
|
||||
These are already burned-in lessons. Every pitfall below is written assuming 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.
|
||||
|
||||
---
|
||||
|
||||
## Critical Pitfalls
|
||||
|
||||
### Pitfall 1: Fastmail Calendars Are CalDAV-Only — JMAP Calendar Is Not Production-Ready
|
||||
### Pitfall 1: VALARM Round-Trip Strips Existing Alarms From Native Clients on Edit
|
||||
|
||||
**What goes wrong:**
|
||||
Fastmail's developer documentation explicitly states: "Calendars — you can access via CalDAV. We will be opening up JMAP access as well, as soon as the specification is finalized." If you build your calendar broker against JMAP expecting calendar read/write, you will find either no endpoint or an unstable/draft surface. This kills the entire broker strategy.
|
||||
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.
|
||||
|
||||
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.
|
||||
|
||||
**Why it happens:**
|
||||
JMAP for Calendars (RFC draft) has been "almost finalized" for years. It's easy to assume Fastmail's JMAP support is comprehensive because their email/contacts JMAP support is excellent. Calendar is the exception.
|
||||
`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."
|
||||
|
||||
**How to avoid:**
|
||||
Commit to CalDAV as the protocol for all calendar read/write. Do not design the broker around JMAP calendars or leave it open. Use `https://caldav.fastmail.com/dav/principals/user/{email}/` as the base URL — the bare `caldav.fastmail.com` root is not sufficient for library URL discovery.
|
||||
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."
|
||||
|
||||
**Warning signs:**
|
||||
- Any design doc that says "CalDAV or JMAP, TBD"
|
||||
- Libraries that prefer JMAP and fall back silently
|
||||
- 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.
|
||||
|
||||
**Phase to address:**
|
||||
Calendar broker implementation phase (first phase that touches Fastmail). Lock the protocol decision in the first spike; do not re-evaluate.
|
||||
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.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 2: Recurring Event RRULE Expansion Done in the App Instead of Leveraged from Server
|
||||
### Pitfall 2: TRIGGER Value-Type Mismatch Silently Produces Broken VALARM
|
||||
|
||||
**What goes wrong:**
|
||||
The app fetches VCALENDAR objects and attempts to expand RRULE recurrences in application code using a generic JS library (e.g., `rrule.js`). The RFC 5545 recurrence model is large: RRULE, RDATE, EXDATE, and RECURRENCE-ID overrides interact. Bugs appear in: yearly events near leap-day, monthly BYDAY rules (e.g., "last Friday"), weekly events near DST transitions, and any series with moved/cancelled individual instances (RECURRENCE-ID).
|
||||
RFC 5545 §3.8.6.3 defines two legal TRIGGER value types for VALARM:
|
||||
- `DURATION` (default): `TRIGGER:-PT15M` — fires 15 minutes before DTSTART.
|
||||
- `DATE-TIME`: `TRIGGER;VALUE=DATE-TIME:20260610T120000Z` — fires at an absolute UTC instant.
|
||||
|
||||
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.
|
||||
|
||||
**Why it happens:**
|
||||
Developers underestimate RFC 5545 complexity. Square built an internal RRULE library because existing ones couldn't handle the full spec. Even `rrule.js` has known edge cases with DST and complex rules. The CalDAV spec provides server-side expansion (CALDAV:expand) exactly because client expansion is error-prone.
|
||||
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.
|
||||
|
||||
**How to avoid:**
|
||||
Use CalDAV's server-side expansion. Issue time-ranged REPORT requests with `<C:expand>` to get back already-expanded instances within a window, rather than fetching raw VCALENDAR and expanding yourself. Only expand locally for display rendering (simple cases). Use `rrule.js` only for the display layer on events you've already confirmed against server expansion.
|
||||
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.
|
||||
|
||||
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).
|
||||
|
||||
**Warning signs:**
|
||||
- Events that appear correct in simple cases but shift by one hour across DST boundaries
|
||||
- A recurring weekly event showing on the wrong day for specific months
|
||||
- Cancelled or moved instances reappearing
|
||||
- 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.
|
||||
|
||||
**Phase to address:**
|
||||
Calendar fetch/display phase. Define the REPORT request format in the first calendar sync spike, not as a later optimization.
|
||||
Per-event reminders phase. Unit test the VALARM serialization before any end-to-end reminder test.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 3: All-Day Events Interpreted as Timed UTC Events
|
||||
### Pitfall 3: All-Day Event VALARM Timezone Semantics Are Undefined
|
||||
|
||||
**What goes wrong:**
|
||||
All-day events in iCalendar use `DATE` (not `DATETIME`) values and carry no timezone. If your backend stores or returns them as UTC datetimes, or if the frontend uses `.toISOString()` on the date, an all-day event for "June 5" becomes "June 4 at 20:00 PDT" — it shifts into the previous day. The CalDAV time-range filter also returns wrong results for all-day events when UTC arithmetic is applied.
|
||||
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.
|
||||
|
||||
**Why it happens:**
|
||||
Most date libraries default to UTC datetime handling. The distinction between `DTSTART;VALUE=DATE:20260605` and `DTSTART;TZID=America/Toronto:20260605T090000` is easy to conflate. Home Assistant's CalDAV integration has had this exact bug filed multiple times.
|
||||
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.
|
||||
|
||||
**How to avoid:**
|
||||
Represent all-day events as a `{ date: "YYYY-MM-DD", allDay: true }` struct throughout the stack — never coerce to a JS `Date` or SQL `DATETIME`. In MariaDB, store as `DATE` column, not `DATETIME`. In the API response, emit the ISO date string without a time component. In the frontend, detect `allDay` and render accordingly without any timezone conversion.
|
||||
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.
|
||||
|
||||
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.
|
||||
|
||||
**Warning signs:**
|
||||
- All-day events appearing on the day before in certain timezones
|
||||
- CalDAV time-range queries missing all-day events that should be in range
|
||||
- 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.
|
||||
|
||||
**Phase to address:**
|
||||
Calendar data model phase. Define the `allDay` field in the internal schema before writing any persistence or API code.
|
||||
Per-event reminders phase. UI constraint and API guard belong in the same plan.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 4: ETag / Sync-Token Incremental Sync Done Wrong
|
||||
### Pitfall 4: Duplicate Push When Generalizing the Fixed-Window Dedup to Per-Event Lead Times
|
||||
|
||||
**What goes wrong:**
|
||||
Two common failure modes. First: the app does a full PROPFIND on every poll instead of using WebDAV-Sync (RFC 6578), burning bandwidth and causing delays at scale. Second: the app uses sync-tokens but doesn't handle 403 (token expired/forgotten by server) — some servers, including iCloud, will drop old tokens, and the app must fall back to full resync. Fastmail uses Cyrus IMAP under the hood and can expire tokens.
|
||||
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:
|
||||
|
||||
For write operations: the app updates a VCALENDAR without sending `If-Match: <etag>`, so if the event was concurrently modified (e.g., from the primary user's native Fastmail app), the server returns a 412 and the app either silently drops the write or throws an unhandled error.
|
||||
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.
|
||||
|
||||
**How to avoid:**
|
||||
- Always check for `{DAV}sync-token` support via PROPFIND before using it; fall back to getctag polling if absent
|
||||
- Persist the last sync-token in the database and use it on subsequent syncs
|
||||
- Handle 403 on sync-token by discarding the token and doing a full resync
|
||||
- On all PUT/DELETE operations, include `If-Match: <etag>` header; handle 412 by fetching the current state, presenting a merge/overwrite choice (even if just "last write wins" for v1)
|
||||
|
||||
**Warning signs:**
|
||||
- Polling logs show full PROPFIND responses every cycle rather than delta responses
|
||||
- Errors after periods of inactivity that clear on app restart
|
||||
- Edits from the native Fastmail app not appearing or being overwritten silently
|
||||
|
||||
**Phase to address:**
|
||||
Calendar sync engine phase. The sync-token + ETag strategy must be in the design before writing the poller — retrofitting is painful.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 5: Write-Back Modifying a Single Recurring Instance Corrupts the Series
|
||||
|
||||
**What goes wrong:**
|
||||
When a user edits a single occurrence of a recurring event (e.g., moves next Tuesday's meeting to Wednesday), the correct CalDAV write is to store a VEVENT with `RECURRENCE-ID` as an additional component in the same VCALENDAR resource. A naive implementation either: (a) writes a new standalone event, leaving the original occurrence intact (duplication), or (b) modifies the master RRULE, changing all future occurrences.
|
||||
|
||||
Known Nextcloud and ownCloud bugs document exactly this: "CalDAV: Moving single event from recurring events results in duplicated event."
|
||||
|
||||
**How to avoid:**
|
||||
When editing a single instance: fetch the full VCALENDAR, inject a new VEVENT block with `RECURRENCE-ID` matching the original instance's DTSTART, and PUT the entire modified VCALENDAR back with `If-Match`. Do not create a new resource. If editing all future instances, set `UNTIL` or `COUNT` on the master rule and create a new recurring series starting from the edit point.
|
||||
|
||||
For v1, consider restricting to "edit all instances" only and deferring single-instance overrides to v2 — the complexity is disproportionate to a two-person household.
|
||||
|
||||
**Warning signs:**
|
||||
- Editing a recurring event produces two events on the calendar
|
||||
- Other instances of the series shift after an individual edit
|
||||
- Events with `RECURRENCE-ID` appearing as standalone items
|
||||
|
||||
**Phase to address:**
|
||||
Event edit UI phase. The decision to support or defer single-instance overrides must be made before the edit form is built.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 6: DST Transition Shifts Recurring Events by One Hour
|
||||
|
||||
**What goes wrong:**
|
||||
A recurring event created in summer (UTC-4) that spans a DST boundary (clocks fall back to UTC-5) can shift by one hour on every occurrence after the transition if the VTIMEZONE component is malformed or if the server/client disagrees on which TZID to use. This is the single most-reported CalDAV bug across all implementations.
|
||||
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.
|
||||
|
||||
**Why it happens:**
|
||||
The iCalendar spec requires a VTIMEZONE block describing the DST rules for the TZID in use. Many libraries emit a minimal or incorrect VTIMEZONE. Fastmail's Cyrus server uses its own TZID database; if the client sends a different TZID alias (e.g., `Eastern Standard Time` vs `America/New_York`), the server may misinterpret transitions.
|
||||
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.
|
||||
|
||||
**How to avoid:**
|
||||
- Always use IANA timezone IDs (e.g., `America/Toronto`) — never Windows-style IDs
|
||||
- Use a library that generates correct VTIMEZONE blocks from the IANA tz database (e.g., `ical.js` or `node-ical` with `tz-data`)
|
||||
- Test recurring events specifically across the spring and fall DST boundary dates before any calendar milestone is considered done
|
||||
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`.
|
||||
|
||||
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.
|
||||
|
||||
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.
|
||||
|
||||
**Warning signs:**
|
||||
- Events correct in summer that shift by exactly one hour in November
|
||||
- `TZID` values in emitted iCalendar containing spaces or Windows timezone names
|
||||
- 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).
|
||||
|
||||
**Phase to address:**
|
||||
Calendar write phase. Add a DST-crossing test fixture before the first release.
|
||||
Per-event reminders phase. Schema migration for `reminderMinutes` column in `calendar_events` is a prerequisite; dedup key change must land in the same plan.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 7: Personal Calendar Sharing to the Broker Token Is Not Automatic
|
||||
### Pitfall 5: Double-Drain When Event-Driven Trigger and 15s setInterval Both Fire
|
||||
|
||||
**What goes wrong:**
|
||||
The architecture assumes one broker API token reads all calendars (shared family + each member's personal). But a Fastmail API token scoped to the primary account cannot read a different member's personal Fastmail calendar unless that calendar has been explicitly shared using Fastmail's CalDAV sharing model (offer + acceptance flow). This is not automatic; it requires setup steps that involve both Fastmail accounts.
|
||||
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:
|
||||
|
||||
- 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.
|
||||
|
||||
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.
|
||||
|
||||
**Why it happens:**
|
||||
Fastmail's calendar ACLs follow the CalDAV sharing standard: the owner must share the calendar, and the recipient must accept. A single-account API token only sees calendars in that account's homeset plus calendars shared to it and accepted.
|
||||
|
||||
Additionally, the wife's personal calendar may not be on Fastmail at all if she uses Apple Calendar as her primary (iCloud calendar). In that case the broker can never read it via CalDAV — there is no cross-service token.
|
||||
`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.
|
||||
|
||||
**How to avoid:**
|
||||
- Do a proof-of-concept share in the first calendar spike: share the primary user's personal calendar to a test account, accept it, and verify the broker token sees it via PROPFIND of the homeset
|
||||
- Document the manual setup steps for each member's personal calendar as part of the deployment runbook
|
||||
- For v1, scope the MVP to the shared family calendar only; add personal calendar overlay only after confirming the share/accept flow works
|
||||
- If the wife's personal events live in iCloud (not Fastmail), treat her personal calendar as out of scope or accept an ICS subscription URL approach
|
||||
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.
|
||||
|
||||
**Warning signs:**
|
||||
- Broker token PROPFIND returns only the shared family calendar, not personal calendars
|
||||
- Empty calendar list after adding a member
|
||||
- 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).
|
||||
|
||||
**Phase to address:**
|
||||
Infrastructure/deployment phase and calendar broker spike. This is a prerequisite that blocks "unified view" features.
|
||||
Event-driven outbox drain phase. The drain trigger design must be reviewed before implementation; the `isDraining` guard docs already note the single-process limitation.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 8: iOS Web Push Requires "Add to Home Screen" — and Apple Provides No Install Prompt
|
||||
### Pitfall 6: Event-Driven Drain Breaks Create-Before-Delete Ordering Under Concurrent Enqueues
|
||||
|
||||
**What goes wrong:**
|
||||
On Android, Chrome shows an "Install" banner or button (beforeinstallprompt). On iOS, there is no equivalent browser prompt. The user must manually tap the Safari share sheet, scroll to find "Add to Home Screen," and tap it. A non-technical user who doesn't know this exists will never install the PWA, and therefore will never receive any push notifications.
|
||||
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:
|
||||
|
||||
- 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.
|
||||
|
||||
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 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.
|
||||
|
||||
**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.
|
||||
|
||||
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.
|
||||
|
||||
**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.
|
||||
|
||||
**Phase to address:**
|
||||
Event-driven outbox drain phase. The enqueue ordering requirement and transaction boundary must be specified in the plan.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 7: Admin App-Password Update Logged or Echoed in Error Messages
|
||||
|
||||
**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.
|
||||
|
||||
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.
|
||||
|
||||
**Why it happens:**
|
||||
Apple has explicitly not implemented the Web App Manifest install prompt on iOS. The Add to Home Screen path exists but is discoverable only if you know where to look.
|
||||
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.
|
||||
|
||||
**How to avoid:**
|
||||
- Implement an in-app installation guide with annotated screenshots specific to iOS Safari (share icon → "Add to Home Screen") that appears on first visit when `display-mode: browser` is detected
|
||||
- Use `navigator.standalone` to detect whether the app is installed and conditionally show the banner
|
||||
- Do not assume the wife will find this herself — the onboarding flow for iOS must walk her through it explicitly
|
||||
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.
|
||||
|
||||
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.
|
||||
|
||||
**Warning signs:**
|
||||
- No push subscriptions registered for iPhone users
|
||||
- Wife accessing the app via browser tab URL, not from the home screen
|
||||
- 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.
|
||||
|
||||
**Phase to address:**
|
||||
PWA setup / onboarding phase. The install guide is not a nice-to-have — it is load-bearing for the non-technical user UX constraint.
|
||||
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.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 9: iOS Kills Push Subscriptions Silently After 3 Silent Pushes
|
||||
### Pitfall 8: Unauthenticated Setup Endpoint Left Live After First Run
|
||||
|
||||
**What goes wrong:**
|
||||
Apple/WebKit enforces `userVisibleOnly: true` strictly. If the service worker receives a push event and fails to display a notification before the event handler terminates — even once accidentally — iOS counts it as a "silent push." After 3 silent pushes, the subscription is permanently revoked without any `pushsubscriptionchange` event (which iOS doesn't support anyway). The user stops receiving notifications without knowing it, and the server continues sending to a dead endpoint.
|
||||
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.
|
||||
|
||||
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.
|
||||
|
||||
**Why it happens:**
|
||||
The most common mistake is calling `showNotification()` without wrapping it in `event.waitUntil()`. Without `waitUntil`, the service worker runtime terminates before the async notification display completes, making it appear silent to iOS.
|
||||
|
||||
Separately: Apple's Intelligent Tracking Prevention (ITP) deletes service worker registrations for sites not visited "recently enough," silently invalidating subscriptions.
|
||||
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.
|
||||
|
||||
**How to avoid:**
|
||||
- Always wrap `showNotification()` in `event.waitUntil()` — no exceptions
|
||||
- Implement a subscription health-check: on every app open, call `pushManager.getSubscription()` and compare the endpoint to the stored server-side endpoint; re-subscribe if they differ or if null
|
||||
- On the server, handle 410 (Gone) responses from the push service as permanent subscription deletion; remove the subscription record immediately
|
||||
- Handle 404 and 401 from the push service as potentially expired; remove and force re-subscription on next app open
|
||||
- Log push delivery success/failure server-side so silent failures are detectable
|
||||
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.
|
||||
|
||||
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.
|
||||
|
||||
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.
|
||||
|
||||
**Warning signs:**
|
||||
- Push success rate drops from the server perspective with no user-visible errors
|
||||
- Server has endpoint records but delivers 410/404
|
||||
- Wife's iPhone stops getting notifications after a week of inactivity
|
||||
- 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.
|
||||
|
||||
**Phase to address:**
|
||||
Push notification implementation phase. The `waitUntil` pattern and subscription health-check must be in the initial implementation, not added later.
|
||||
Setup wizard phase. The guard must be the first thing implemented; test the guard before testing the happy path.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 10: Declarative Web Push vs Standard Web Push — Choose the Right Target
|
||||
### Pitfall 9: Admin Role Check Bypassed by Missing Middleware Wiring
|
||||
|
||||
**What goes wrong:**
|
||||
WebKit has introduced "Declarative Web Push" (announced mid-2025), which allows notification display without a service worker by using a standardized JSON payload format. If you build against standard Web Push with a service worker and custom payload parsing, and then Apple's ITP clears the service worker, your notifications stop. Declarative Web Push survives ITP because the browser can display the notification natively without running JS.
|
||||
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.
|
||||
|
||||
If you build notifications that depend on custom payload processing in the service worker (e.g., fetching additional data from the server before showing the notification), Declarative Web Push cannot handle that — you'd need the service worker anyway.
|
||||
|
||||
**How to avoid:**
|
||||
Design notification payloads to be self-contained (all display data in the push payload: title, body, icon, URL to open). This satisfies both Declarative Web Push display requirements and standard Web Push service worker display. Do not design a "fetch-to-display" pattern where the service worker hits the API before showing anything — that pattern breaks on iOS after ITP clears the SW.
|
||||
|
||||
**Warning signs:**
|
||||
- Notification payloads that contain only an event ID, requiring a network fetch to render
|
||||
- Service worker push handler making API calls before `showNotification`
|
||||
|
||||
**Phase to address:**
|
||||
Push notification design phase. Define the payload schema before implementing the server-side push sender.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 11: EU Digital Markets Act Breaks iOS PWA Entirely for EU Users
|
||||
|
||||
**What goes wrong:**
|
||||
Since iOS 17.4 (March 2024), in EU countries Apple removed standalone PWA mode under the Digital Markets Act. PWAs open as standard Safari tabs. Push notifications do not work. Add to Home Screen produces a bookmark, not an installed PWA. This affects the entire notification strategy for any EU household.
|
||||
|
||||
**Why it matters here:**
|
||||
The project is Canadian (primary user email: me@lucasberger.ca — `.ca` domain, Unraid self-hosted). If the household is in Canada, this does not apply. Document it as a known non-issue for this deployment, but note it if the household ever relocates or if devices are registered in EU Apple IDs.
|
||||
|
||||
**How to avoid:**
|
||||
Confirm Apple ID region for both household members. If non-EU, proceed without mitigation. If EU, the entire push notification strategy must shift to email or in-app alerts only.
|
||||
|
||||
**Phase to address:**
|
||||
Risk assessment before push implementation. One-time check, not ongoing work.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 12: Service Worker Caching Serves Stale Calendar Data
|
||||
|
||||
**What goes wrong:**
|
||||
If the service worker uses a Cache-First strategy for API responses, calendar data shown in the PWA may be hours old. A user adds an event from the native Fastmail app, opens FamilySync, and sees yesterday's calendar. For a family coordination tool this destroys trust immediately.
|
||||
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.
|
||||
|
||||
**Why it happens:**
|
||||
Cache-First is the default recommendation for PWA shell (HTML/CSS/JS assets) but gets applied to API/data routes by mistake, or by using a broad URL pattern in the Workbox config.
|
||||
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.
|
||||
|
||||
**How to avoid:**
|
||||
- Use Cache-First only for static assets (JS bundles, CSS, icons) with content-hash filenames
|
||||
- Use Network-First for all `/api/*` routes; fall back to cache only if offline
|
||||
- Use Stale-While-Revalidate for calendar data that is acceptable to be slightly stale (list data OK; calendar event times not OK)
|
||||
- Scope `workbox-recipes` patterns explicitly; never use `.*` to match API routes
|
||||
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.
|
||||
|
||||
**Warning signs:**
|
||||
- Network tab shows calendar API responses served from ServiceWorker cache
|
||||
- Events created elsewhere don't appear after page reload
|
||||
- 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.
|
||||
|
||||
**Phase to address:**
|
||||
PWA service worker configuration phase. Cache strategy per route must be intentional from the start.
|
||||
Admin Settings phase. Integration test for 403 on non-admin access is the acceptance criterion.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 13: Service Worker Update Staleness — App Never Updates for the Wife
|
||||
### Pitfall 10: App Password and VAPID Keys Stored in DB When They Must Stay in Env
|
||||
|
||||
**What goes wrong:**
|
||||
Safari on iOS respects the HTTP cache for service worker script fetching. If the server sends `Cache-Control: max-age=3600` for the SW script, Safari will not check for updates for an hour. For an installed PWA that the wife only opens occasionally, she can run a version that is days old. Breaking API changes in the backend will cause silent failures.
|
||||
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:
|
||||
- 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.
|
||||
|
||||
`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.
|
||||
|
||||
**Why it happens:**
|
||||
Many web servers (Nginx defaults) cache JS assets aggressively. The service worker file (`sw.js`) must be served with `Cache-Control: no-cache` or `max-age=0` specifically to ensure the browser checks for updates on each visit.
|
||||
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.
|
||||
|
||||
**How to avoid:**
|
||||
- Serve the service worker file with `Cache-Control: no-cache` header explicitly
|
||||
- Serve the web app manifest with `Cache-Control: no-cache`
|
||||
- Use Vite's default hashed filenames for all other assets (already correct)
|
||||
- Implement a "new version available" in-app prompt when the SW detects an update (`waiting` state), so the wife knows to tap "refresh"
|
||||
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.
|
||||
|
||||
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.
|
||||
|
||||
**Warning signs:**
|
||||
- Deployed backend changes not reflected in the app for hours or days
|
||||
- Console shows `ServiceWorker: new service worker found, not yet activated`
|
||||
- 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.
|
||||
|
||||
**Phase to address:**
|
||||
PWA build configuration phase. Set the no-cache header in Docker/Nginx config before first deployment.
|
||||
Setup wizard phase. Schema design review before migration is written. Secret-in-DB is a hard blocker for the phase gate.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 14: Calendar Cache Stale vs Fastmail Source of Truth — Double-Write Window
|
||||
### Pitfall 11: Gitea Actions MariaDB Service Container Readiness Race
|
||||
|
||||
**What goes wrong:**
|
||||
The app caches Fastmail calendar data in MariaDB (or Redis) for performance. A user creates an event via the app (write to Fastmail, update local cache). The primary user simultaneously creates the same-time event from the native Fastmail app. The next poll catches the conflict, but between the write and the poll, the app's cache is wrong. If the poll interval is 5 minutes, the family sees conflicting events for up to 5 minutes.
|
||||
|
||||
A worse failure: the write to Fastmail succeeds but the cache update fails (network error mid-transaction). Now the cache is permanently wrong until the next full resync.
|
||||
|
||||
**How to avoid:**
|
||||
- Treat the cache as write-through: invalidate the relevant calendar's cache entry immediately on any write, forcing the next read to pull from Fastmail
|
||||
- After a write (PUT/POST to Fastmail), always re-fetch the created/updated object to get the server-assigned ETag and any server-side modifications
|
||||
- Never update the cache with the client's version of the object — only cache objects received from the server
|
||||
- For v1, a 60-second poll interval is acceptable. Do not optimize this prematurely.
|
||||
|
||||
**Warning signs:**
|
||||
- Event created in the app doesn't appear on the next refresh
|
||||
- Duplicate events visible for a short window
|
||||
- ETag mismatch errors on the second consecutive edit of the same event
|
||||
|
||||
**Phase to address:**
|
||||
Calendar broker / cache design phase. Write-through invalidation must be in the cache design, not patched in later.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 15: Real-Time List Sync — Missed Updates During Reconnect Gap
|
||||
|
||||
**What goes wrong:**
|
||||
The client connects via WebSocket (or SSE). The connection drops (mobile network switch, brief outage). On reconnect, the client re-subscribes but has missed events that fired during the gap. The list appears consistent to both users but is actually diverged — one user's added item is missing from the other's view.
|
||||
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.
|
||||
|
||||
**Why it happens:**
|
||||
The reconnect handler re-subscribes from "now" rather than replaying from a sequence number or version cursor.
|
||||
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.
|
||||
|
||||
**How to avoid:**
|
||||
- Each list mutation must increment a monotonic version on the row (`updated_at` with microsecond precision is insufficient — use an explicit integer sequence per list)
|
||||
- On (re)connect, the client sends its last-known sequence; the server replays any mutations with sequence > client's last-known
|
||||
- Implement exponential backoff with jitter on reconnect (500ms base, 2x multiplier, 30s cap)
|
||||
- Redis Pub/Sub is appropriate here; if Redis is unavailable, fall back to polling every 5s
|
||||
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.
|
||||
|
||||
**Warning signs:**
|
||||
- Items added during a reconnect gap missing from one client's view
|
||||
- List state inconsistent between the two household members
|
||||
- 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.
|
||||
|
||||
**Phase to address:**
|
||||
Shared lists implementation phase. The sequence number column must be in the initial schema.
|
||||
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 16: Authelia OIDC — v4.39+ Breaking Change Drops `groups` from ID Token
|
||||
### Pitfall 12: Gitea Actions Self-Hosted Runner Missing Node 22 or pnpm
|
||||
|
||||
**What goes wrong:**
|
||||
Authelia v4.39 introduced a breaking change: the `groups` claim is no longer included in the ID token by default — it moved to the userinfo endpoint. If the backend validates authorization based on the `groups` claim in the ID token (a common pattern when following older Authelia docs), group-based access control silently stops working after an Authelia upgrade.
|
||||
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:**
|
||||
- Request the `groups` scope explicitly in the OIDC client config
|
||||
- Validate group membership by calling the userinfo endpoint, not by reading ID token claims
|
||||
- Or: use Authelia purely for authentication (who are you), not authorization (what can you do) — in a two-person household, all authenticated users are trusted, so groups are irrelevant for FamilySync
|
||||
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:**
|
||||
- Group-based middleware that worked before an Authelia upgrade stops denying unauthorized users
|
||||
- `groups` claim missing from decoded ID token
|
||||
- `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:**
|
||||
Auth integration phase. Decide whether group claims are needed at all; if not, skip them entirely.
|
||||
Gitea CI phase. The runner environment probe must be the first CI task — before any test or build steps are designed.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 17: Authelia OIDC — SPA Token Silent Renewal Breaks When Authelia Is on a Separate Domain
|
||||
### Pitfall 13: Docker Registry Push Token Scope Exposes Secrets in Logs
|
||||
|
||||
**What goes wrong:**
|
||||
The standard OIDC silent renewal technique for SPAs uses a hidden iframe that loads Authelia's authorization endpoint. If the browser blocks third-party cookies (Safari ITP does this aggressively), the iframe cannot send Authelia's session cookie, so the silent renewal returns an error and the user is redirected to the login page unexpectedly — often in a loop.
|
||||
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 matters here:**
|
||||
FamilySync and Authelia may be on different subdomains (e.g., `familysync.home.domain.com` vs `auth.home.domain.com`). If they share the same registrable domain suffix (e.g., both `.home.domain.com`), same-site cookies work. If not, ITP kills the iframe flow.
|
||||
**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:**
|
||||
- Ensure FamilySync and Authelia share the same parent domain so Authelia cookies are same-site from the browser's perspective
|
||||
- Use refresh token rotation instead of iframe-based silent renewal (Authelia supports this)
|
||||
- Configure the OIDC client in the backend (not the SPA) to hold the refresh token — the SPA calls the backend, which silently renews via the confidential client flow, and returns a new access token without iframe involvement
|
||||
- Never rely on iframe silent renewal for installed PWAs — service workers intercept the iframe navigation and it behaves unpredictably
|
||||
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:**
|
||||
- Users randomly logged out after token expiry with no warning
|
||||
- Infinite redirect loop between the app and Authelia login page
|
||||
- Console errors: `Failed to load resource: Frame load interrupted`
|
||||
- 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:**
|
||||
Auth integration phase. The token refresh strategy must be decided before the frontend auth client is chosen.
|
||||
Gitea CI phase. Credential handling review before any Docker push step is added.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 18: Pangolin/Newt Tunnel — WebSocket and SSE May Require Explicit Configuration
|
||||
### Pitfall 14: Playwright Authed-Mobile Harness Reusing a Stale storage-state
|
||||
|
||||
**What goes wrong:**
|
||||
Pangolin is a tunneled reverse proxy. WebSocket connections require an HTTP Upgrade handshake, and SSE (Server-Sent Events) requires long-lived HTTP connections. A generic proxy configuration that works for standard HTTP requests may silently drop WebSocket connections or close SSE streams after a timeout.
|
||||
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.
|
||||
|
||||
A known GitHub issue (#1034 on the fosrl/pangolin repo) documents exactly this: HTTP loads fine through Pangolin but WebSocket connections to `wss://` fail.
|
||||
**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:**
|
||||
- Verify WebSocket pass-through in a dedicated infrastructure spike before building any real-time feature
|
||||
- Confirm Pangolin's timeout settings for long-lived connections and extend them appropriately
|
||||
- If WebSocket through Pangolin proves unreliable, use SSE (unidirectional, standard HTTP, more proxy-friendly) for server-to-client push and short-poll for client confirmations
|
||||
- Test the full round-trip (WebSocket from an iPhone over Pangolin) before considering real-time sync "done"
|
||||
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:**
|
||||
- WebSocket connects locally but fails in production (public URL)
|
||||
- SSE stream closes after 60 seconds with no activity
|
||||
- Real-time updates work on Android (same network) but not iPhone (over tunnel)
|
||||
- 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:**
|
||||
Infrastructure spike phase, before real-time list sync is implemented.
|
||||
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.
|
||||
|
||||
---
|
||||
|
||||
@@ -407,14 +400,13 @@ Infrastructure spike phase, before real-time list sync is implemented.
|
||||
|
||||
| Shortcut | Immediate Benefit | Long-term Cost | When Acceptable |
|
||||
|----------|-------------------|----------------|-----------------|
|
||||
| Full PROPFIND poll on every sync interval instead of WebDAV-Sync | Simpler code | Bandwidth waste; shows up immediately at any real-world poll frequency | Never — implement sync-token from day one |
|
||||
| Client-side RRULE expansion instead of server CALDAV:expand | Avoids REPORT query complexity | DST and override bugs that are very hard to diagnose | Never for authoritative display; OK for UI-only preview |
|
||||
| Storing all-day events as DATETIME in MariaDB | Avoids DATE type handling | Timezone shift bugs on display | Never |
|
||||
| Skip `If-Match` ETag on write-back | Simpler write path | Silent data loss on concurrent edits | Acceptable for v1 only if single-writer constraint is documented; still risky |
|
||||
| iframe silent token renewal instead of refresh token rotation | Less backend work | Breaks on iOS Safari ITP; causes random logouts | Never for this stack |
|
||||
| Cache-First strategy for API responses | Fast perceived performance | Stale calendar/list data shown as current | Never for data routes |
|
||||
| Notification payload requiring server fetch before display | Richer notifications | Violates `userVisibleOnly`; kills iOS subscription after 3 events | Never |
|
||||
| Single-instance recurring event override deferred | Significantly simpler write logic | Users frustrated when editing "just this one" is impossible | Acceptable for v1 — document clearly |
|
||||
| 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 |
|
||||
|
||||
---
|
||||
|
||||
@@ -422,29 +414,17 @@ Infrastructure spike phase, before real-time list sync is implemented.
|
||||
|
||||
| Integration | Common Mistake | Correct Approach |
|
||||
|-------------|----------------|------------------|
|
||||
| Fastmail CalDAV | Using bare `caldav.fastmail.com` as base URL | Use `https://caldav.fastmail.com/dav/principals/user/{email}/` |
|
||||
| Fastmail CalDAV | Assuming JMAP supports calendars | CalDAV only; JMAP calendar spec not yet production at Fastmail |
|
||||
| Fastmail CalDAV | Assuming broker token sees all members' personal calendars | Explicit share+accept required per calendar per account |
|
||||
| CalDAV ETag | Assuming ETag is always returned after PUT | Sometimes absent; always re-fetch after write |
|
||||
| CalDAV sync-token | Not handling 403 on expired token | Fall back to full PROPFIND resync when token is rejected |
|
||||
| CalDAV recurring events | Writing RECURRENCE-ID override as a new resource | Must be an additional VEVENT in the same VCALENDAR resource |
|
||||
| iOS Web Push | Calling `requestPermission()` outside a click handler | Permission silently denied; must be in direct user gesture handler |
|
||||
| iOS Web Push | Not wrapping `showNotification()` in `event.waitUntil()` | 3 silent pushes = permanent subscription revocation |
|
||||
| iOS Web Push | Not handling 410 Gone from push service | Dead subscriptions accumulate; subscription table grows without cleanup |
|
||||
| Authelia OIDC v4.39+ | Reading `groups` from ID token | Fetch from userinfo endpoint or skip groups entirely |
|
||||
| Authelia OIDC | iframe silent renewal with ITP | Use refresh token rotation via confidential backend client |
|
||||
| Pangolin tunnel | Assuming HTTP proxy config handles WebSocket | Requires explicit WS upgrade passthrough and timeout config |
|
||||
|
||||
---
|
||||
|
||||
## Performance Traps
|
||||
|
||||
| Trap | Symptoms | Prevention | When It Breaks |
|
||||
|------|----------|------------|----------------|
|
||||
| Full calendar PROPFIND on every poll | High Fastmail API usage; slow sync | Use WebDAV-Sync (sync-token) for delta fetches | From first deployment |
|
||||
| Expanding RRULE in-process for a 2-year window | CPU spike on backend; slow calendar load | Limit expansion to visible time window via CALDAV:expand | Any calendar with >30 recurring events |
|
||||
| Fetching all VCALENDAR objects to find changed ones | N+1 CalDAV requests per sync | Use PROPFIND to get ETags first; only GET objects with changed ETags | Immediately on any non-trivial calendar |
|
||||
| WebSocket broadcast to all clients on every list mutation | Unnecessary events to clients not viewing that list | Filter broadcasts by list membership/subscription | Once any second device is connected |
|
||||
| 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 |
|
||||
|
||||
---
|
||||
|
||||
@@ -452,39 +432,26 @@ Infrastructure spike phase, before real-time list sync is implemented.
|
||||
|
||||
| Mistake | Risk | Prevention |
|
||||
|---------|------|------------|
|
||||
| Storing Fastmail API token in client-accessible storage | Token leak → full Fastmail account calendar access | Token lives only in backend env vars; never exposed to frontend |
|
||||
| Forwarding raw VCALENDAR to the frontend | iCalendar payloads can contain injected properties | Parse and reconstruct a safe JSON event object server-side |
|
||||
| Not validating OIDC `aud` claim | Any Authelia client can impersonate FamilySync users | Verify `aud` matches your registered client ID on every token validation |
|
||||
| Push subscription endpoint stored unencrypted with PII | Subscription URLs are tied to device identity | Store encrypted; treat as sensitive PII; delete on logout |
|
||||
| Trusting `display-mode: standalone` for access control | Cannot be relied on for security | Standalone check is UX-only; all access control at API layer via token |
|
||||
|
||||
---
|
||||
|
||||
## UX Pitfalls
|
||||
|
||||
| Pitfall | User Impact | Better Approach |
|
||||
|---------|-------------|-----------------|
|
||||
| No iOS install guide in the app | Wife never installs PWA; never gets push notifications | Show install CTA with Safari-specific screenshots on first browser visit |
|
||||
| Push permission requested on first page load | iOS silently denies; permission cannot be re-requested | Request permission only after explaining why, inside a tapped button |
|
||||
| "Notification" as the only feedback for list changes | Missed if phone silent; no in-app indicator | Show badge/dot on list items changed since last viewed; push is secondary |
|
||||
| Calendar event edit with no conflict warning | Overwrites event changed in native Fastmail app | Show "this event was modified elsewhere" warning on 412; offer overwrite or reload |
|
||||
| No offline message | App appears broken when offline | Show "offline — viewing cached data" banner; list edits queue for sync |
|
||||
| Recurring event edit options not explained | User edits "this event" not knowing it changes all future events | For v1, edit-all only with clear label "This changes all future events" |
|
||||
| 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 |
|
||||
|
||||
---
|
||||
|
||||
## "Looks Done But Isn't" Checklist
|
||||
|
||||
- [ ] **Calendar sync:** Often missing ETag/sync-token handling — verify the poller uses delta sync, not full PROPFIND, on the second and subsequent runs
|
||||
- [ ] **All-day events:** Often missing allDay flag — verify a June 5 all-day event appears on June 5 in UTC-5 timezone, not June 4
|
||||
- [ ] **Recurring events:** Often missing DST boundary test — verify a weekly recurring event set in July still shows at the correct hour in November
|
||||
- [ ] **iOS push:** Often missing subscription health-check — verify a fresh subscription is created after ITP clears the service worker (simulate by clearing site data in Safari settings)
|
||||
- [ ] **iOS push:** Often missing `event.waitUntil()` — verify in Safari DevTools that push events show as resolved, not terminated
|
||||
- [ ] **Personal calendar sharing:** Often assumed to be automatic — verify broker token PROPFIND actually returns the wife's personal calendar URL
|
||||
- [ ] **Write-back:** Often missing 412 handling — verify the app does not silently drop an edit when the event was modified concurrently in the native app
|
||||
- [ ] **Service worker update:** Often ships with default asset caching headers — verify `sw.js` is served with `Cache-Control: no-cache`
|
||||
- [ ] **Auth token renewal:** Often untested at expiry — verify the app does not redirect to login when the access token expires mid-session
|
||||
- [ ] **Pangolin WebSocket:** Often only tested on LAN — verify WebSocket or SSE works end-to-end over the Pangolin public URL before shipping real-time sync
|
||||
- [ ] **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).
|
||||
|
||||
---
|
||||
|
||||
@@ -492,15 +459,14 @@ Infrastructure spike phase, before real-time list sync is implemented.
|
||||
|
||||
| Pitfall | Recovery Cost | Recovery Steps |
|
||||
|---------|---------------|----------------|
|
||||
| JMAP calendar assumption baked in | HIGH | Rewrite broker protocol layer; API contract may change |
|
||||
| All-day event stored as DATETIME | HIGH | Migration required; all calendar cache invalid; must resync |
|
||||
| No ETag/sync-token on poller | MEDIUM | Swap polling logic; no data loss, but adds a sprint |
|
||||
| Push subscription table full of dead iOS endpoints | LOW | Run cleanup job: send to each endpoint, delete 410/404 responses |
|
||||
| Personal calendars not shared to broker | LOW | Manual Fastmail share + accept; no code change |
|
||||
| VTIMEZONE DST bug in emitted iCalendar | MEDIUM | Swap tz library; clear Fastmail events and re-sync |
|
||||
| Service worker cache serving stale data | LOW | Add route-specific Workbox config; redeploy |
|
||||
| Authelia groups claim removed after upgrade | LOW | Update claim source to userinfo endpoint; or remove group checks |
|
||||
| Pangolin WebSocket drops | MEDIUM | Switch real-time transport from WS to SSE; no data model change |
|
||||
| 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 |
|
||||
|
||||
---
|
||||
|
||||
@@ -508,51 +474,35 @@ Infrastructure spike phase, before real-time list sync is implemented.
|
||||
|
||||
| Pitfall | Prevention Phase | Verification |
|
||||
|---------|------------------|--------------|
|
||||
| JMAP calendar not available | Calendar broker spike (Phase 1) | Confirm CalDAV endpoint returns events before any further work |
|
||||
| RRULE client-side expansion | Calendar fetch design (Phase 1) | REPORT with CALDAV:expand in the first integration test |
|
||||
| All-day event DATE vs DATETIME | Data model design (Phase 1) | Schema review; allDay field present; timezone test |
|
||||
| ETag / sync-token incremental sync | Calendar sync engine (Phase 1-2) | Second poll must use sync-token; verify in network logs |
|
||||
| Write-back RECURRENCE-ID corruption | Event edit UI (Phase 2-3) | Integration test: edit instance, verify series unchanged |
|
||||
| DST timezone shift | Calendar write phase (Phase 2) | Dedicated DST fixture test before milestone sign-off |
|
||||
| Personal calendar sharing | Infrastructure/deployment (Phase 1) | Manual proof-of-concept share before unified view is built |
|
||||
| iOS install guide missing | PWA onboarding (Phase 2) | Test with non-technical user; do not consider done until wife installs |
|
||||
| iOS push subscription killed (silent push) | Push implementation (Phase 2-3) | Verify `event.waitUntil()` pattern; test 3 consecutive pushes |
|
||||
| Declarative Web Push payload design | Push design (Phase 2) | Payload schema review before server-side push sender is built |
|
||||
| EU DMA restriction | Risk assessment (Phase 1) | One-time check of Apple ID regions; document conclusion |
|
||||
| Service worker stale cache (data routes) | PWA SW config (Phase 2) | Network tab audit: API calls must not be served from cache |
|
||||
| SW update staleness | PWA build config (Phase 2) | Verify `sw.js` has `Cache-Control: no-cache` in Nginx config |
|
||||
| Calendar cache double-write | Cache design (Phase 1-2) | Write-through invalidation test: write → immediate re-fetch from Fastmail |
|
||||
| List sync reconnect gap | Lists implementation (Phase 2) | Disconnect mid-edit test; verify item appears after reconnect |
|
||||
| Authelia groups claim breaking change | Auth integration (Phase 1) | Decode ID token; confirm no groups dependency; document decision |
|
||||
| Authelia SPA silent renewal | Auth integration (Phase 1) | Test token expiry behavior before frontend is considered done |
|
||||
| Pangolin WebSocket passthrough | Infrastructure spike (Phase 1) | WebSocket smoke test over public URL before real-time feature is built |
|
||||
| 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 |
|
||||
|
||||
---
|
||||
|
||||
## Sources
|
||||
|
||||
- [Fastmail API Documentation](https://www.fastmail.com/dev/) — confirms CalDAV only for calendars; JMAP calendar pending
|
||||
- [Fastmail CalDAV URL discovery](https://utf9k.net/blog/fastmail-caldav/) — principal URL format requirement
|
||||
- [Fastmail shared calendar improvements](https://www.fastmail.com/blog/shared-calendar-improvements/) — ACL sharing model, secretary mode
|
||||
- [Sabre/DAV: Building a CalDAV client](https://sabre.io/dav/building-a-caldav-client/) — ETag, sync-token, UID, VTIMEZONE pitfalls
|
||||
- [RFC 5545 RRULE](https://icalendar.org/iCalendar-RFC-5545/3-8-5-3-recurrence-rule.html) — recurrence set construction
|
||||
- [RFC 5545 EXDATE](https://icalendar.org/iCalendar-RFC-5545/3-8-5-1-exception-date-times.html) — exception date handling
|
||||
- [CalDAV ETag not updating after event edit — Google Issue Tracker](https://issuetracker.google.com/issues/153145152) — real-world ETag inconsistency
|
||||
- [Home Assistant: CalDAV all-day events UTC issue](https://github.com/home-assistant/core/issues/25814) — DATE vs DATETIME bug documented in production
|
||||
- [Nextcloud: recurring event single move duplicates event](https://github.com/owncloud/core/issues/24591) — RECURRENCE-ID write-back failure
|
||||
- [DST CalDAV shift bugs (khal)](https://github.com/pimutils/khal/issues/755) — VTIMEZONE DST implementation errors
|
||||
- [PWA Push Notifications on iOS 2026 (WebsCraft)](https://webscraft.org/blog/pwa-pushspovischennya-na-ios-u-2026-scho-realno-pratsyuye?lang=en) — current iOS push state
|
||||
- [MagicBell: PWA iOS Limitations 2026](https://www.magicbell.com/blog/pwa-ios-limitations-safari-support-complete-guide) — iOS 16.4+ requirement, EU DMA
|
||||
- [iOS push subscriptions terminated after 3 notifications (DEV Community)](https://dev.to/progressier/how-to-fix-ios-push-subscriptions-being-terminated-after-3-notifications-39a7) — `event.waitUntil()` requirement
|
||||
- [iOS Web Push device unregisters spontaneously (Firebase SDK issue)](https://github.com/firebase/firebase-js-sdk/issues/8010) — ITP-driven subscription loss
|
||||
- [WebKit: Meet Declarative Web Push](https://webkit.org/blog/16535/meet-declarative-web-push/) — new WebKit push model, ITP resilience
|
||||
- [Apple Developer Forums: Web Push on iOS](https://developer.apple.com/forums/thread/732594) — subscription expiry, `pushsubscriptionchange` not supported
|
||||
- [Authelia OIDC Clients Configuration](https://www.authelia.com/configuration/identity-providers/openid-connect/clients/) — redirect URI, PKCE, groups claim
|
||||
- [Authelia OIDC FAQ](https://www.authelia.com/integration/openid-connect/frequently-asked-questions/) — breaking changes including groups in ID token
|
||||
- [OIDC SPA: Third-party cookies and session restoration](https://docs.oidc-spa.dev/resources/third-party-cookies-and-session-restoration) — iframe silent renewal + ITP
|
||||
- [Pangolin WebSocket issue #1034](https://github.com/fosrl/pangolin/issues/1034) — WebSocket upgrade failure through tunnel
|
||||
- [WebSocket reconnection guide (WebSocket.org)](https://websocket.org/guides/reconnection/) — state sync on reconnect, missed event gap
|
||||
- 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
|
||||
|
||||
---
|
||||
*Pitfalls research for: Fastmail-brokered family calendar + shared-list PWA with Web Push, self-hosted behind Authelia*
|
||||
*Researched: 2026-06-03*
|
||||
*Pitfalls research for: FamilySync v1.1 Operability & Polish*
|
||||
*Researched: 2026-06-10*
|
||||
|
||||
Reference in New Issue
Block a user