docs: map existing codebase
This commit is contained in:
@@ -0,0 +1,258 @@
|
||||
# Codebase Concerns
|
||||
|
||||
**Analysis Date:** 2026-06-09
|
||||
|
||||
## Tech Debt
|
||||
|
||||
**Drizzle-kit push unsafe on MariaDB 11:**
|
||||
- Issue: `drizzle-kit push` emits false destructive DDL on MariaDB 11 (mysql dialect) — misreads table metadata and schedules column truncation in the migration diff. This destroys production data if applied blindly.
|
||||
- Files: `apps/api/src/db/schema.ts`, `apps/api/drizzle.config.ts`, `.planning/STATE.md` (D-Task5-DDL)
|
||||
- Impact: Any schema change requires manual validation. Automated push pipelines are unsafe.
|
||||
- Current mitigation: All additive DDL hand-applied. Database migrations live in `apps/api/src/db/migrations/` (SQL files). Documented in STATE.md.
|
||||
- Fix approach: Adopt `drizzle-kit generate+migrate` workflow for all future schema changes — generate the diff, manually review the SQL, then apply via migration file. Never use `push` on MariaDB without field-by-field validation. If multi-replica deployment is needed, consider PostgreSQL migration at that point.
|
||||
|
||||
**Dev-auth bypass lacks production guard redundancy:**
|
||||
- Issue: The `DEV_AUTH_BYPASS` environment variable is guarded by a `NODE_ENV !== 'production'` check in `index.ts` (line 19), but relies on correct deployment configuration. If `NODE_ENV` is accidentally omitted from the production Docker Compose, the bypass could activate.
|
||||
- Files: `apps/api/src/index.ts` (lines 19–26), `apps/api/src/auth/devBypass.ts`
|
||||
- Impact: Unauthenticated access to the API in production if misconfigured.
|
||||
- Current mitigation: The `docker-compose.yml` should explicitly set `NODE_ENV=production`; `.env.example` has `DEV_AUTH_BYPASS` commented out. Documented in `docs/deployment.md` (line 266–268).
|
||||
- Fix approach: Add a startup assertion that logs an error and exits if `NODE_ENV !== 'production'` and `DEV_AUTH_BYPASS=true` are both detected. Consider a secondary check in the oidcAuthMiddleware instantiation.
|
||||
|
||||
**Event datetime serialization was timezone-naive (FIXED in Phase 3):**
|
||||
- Issue: The PWA's `EventForm` previously sent naive local wall-clock strings (no UTC offset) to the API; the outbox worker's `new Date(string)` parsed them in the container's UTC timezone, resulting in events written 4 hours early/late. Fixed in Phase 3 quick 260607-l6l.
|
||||
- Files: `apps/pwa/src/lib/eventDateTime.ts` (new), `apps/pwa/src/components/EventForm.tsx` (updated)
|
||||
- Impact: FIXED. Regression test added (`apps/pwa/src/lib/eventDateTime.test.ts`).
|
||||
- Fix status: Closed via commit 2870413 (2026-06-07). Serialization now uses `localWallClockToUtcIso()` to convert to UTC `Z` instant in the browser before sending to the API.
|
||||
|
||||
**Calendar row deduplication cross-user bug (FIXED in Phase 3):**
|
||||
- Issue: The poller and sync used `url`-only predicates to lookup calendar rows, but the two household members share one Fastmail account — the same collection URL exists for both. This caused events to be cached under the wrong member's calendar and duplicate rows accumulated on every poll. Fixed in Phase 3 via commit 2870413 and migration `0001_calendars_user_url_unique.sql`.
|
||||
- Files: `apps/api/src/broker/poller.ts` (line 52–56), `apps/api/src/broker/sync.ts` (line 62–66), `apps/api/src/db/schema.ts` (line 84), `apps/api/src/db/migrations/0001_calendars_user_url_unique.sql`
|
||||
- Impact: FIXED. Unique constraint `uniq_calendar_user_url` enforces (userId, url) identity; all predicates scoped correctly.
|
||||
- Fix status: Closed. Migration applied to live DB; regression tests added to `poller.test.ts` and `sync.test.ts`.
|
||||
|
||||
---
|
||||
|
||||
## Known Bugs
|
||||
|
||||
**GET /api/events missing userId/isShared filter (IDENTIFIED, RESOLVED via 260607-l6l):**
|
||||
- Symptoms: GET /api/events returned events from all users (including stale spike data), not just owned + shared calendars.
|
||||
- Files: `apps/api/src/routes/events.ts` (line 127–129 now filters correctly via resolveUserId)
|
||||
- Trigger: Any `/api/events` call without the ownership/isShared predicate in the JOIN.
|
||||
- Status: FIXED in commit 2870413. The route now filters: `WHERE currentUserId = userId OR isShared=1`.
|
||||
|
||||
**Stale spike user + calendar data in production DB:**
|
||||
- Symptoms: User id=1 ("Dev User", obsolete spike identity `oidc_iss='spike://cal-08'`) remains in the DB with 508 cached events under the now-deduplicated calendar row id=1. This is stale data, not a code bug.
|
||||
- Files: Live MariaDB (data only, not source code)
|
||||
- Impact: Low — new events written by the real users go to the correct rows (id=2, id=3 calendars). The spike data is not served to the app because the route filters by currentUserId. Safe to clean via a manual DB DELETE, but non-blocking.
|
||||
- Fix approach: Post-deployment cleanup task: `DELETE FROM users WHERE oidc_iss='spike://cal-08'; DELETE FROM calendar_events WHERE calendar_id=1;` if confident no real events are under id=1. Safer: check `calendars.url` to confirm id=1 is the spike duplicate before deletion.
|
||||
|
||||
---
|
||||
|
||||
## Security Considerations
|
||||
|
||||
**Fastmail app password exposure risk:**
|
||||
- Risk: The API loads and decrypts Fastmail app passwords from `member_credentials.encrypted_password`. If the encryption key is leaked or the decryption is implemented incorrectly, all calendar access is compromised.
|
||||
- Files: `apps/api/src/broker/crypto.ts`, `apps/api/src/broker/poller.ts` (line 41), `deployment.md` (Step 2 — key generation)
|
||||
- Current mitigation: AES-256-GCM encryption, key stored in `.env` (gitignored). Decrypted password never logged (T-03-04). Decryption happens only in `poller.ts` and `outboxWorker.ts`, not in HTTP routes.
|
||||
- Recommendations: (1) Ensure `.env` is marked .gitignore in CI/CD (already done). (2) Rotate encryption key monthly + re-encrypt all passwords — design a rotation mechanism before multi-replica deployment. (3) Monitor access logs for repeated failed calendar syncs (sign of credential tampering). (4) Consider a secrets manager (e.g., Docker Compose secrets) for the encryption key in production.
|
||||
|
||||
**OIDC claim extraction fragility (Authelia defaults):**
|
||||
- Risk: Authelia v4.39+ omits `name`, `email`, `preferred_username` from the ID token by default — requires a `claims_policy` config. The app's `deriveDisplayName()` (auth/user.ts) falls back through `name` → `preferred_username` → `email` → `sub`, but if Authelia is not configured with claims, all users appear as "Member" in the legend (observed in Phase 2). This is a configuration issue, not a code bug, but fragile.
|
||||
- Files: `apps/api/src/auth/user.ts` (lines 8–19), `docs/deployment.md` (Authelia client config, line 91–92 does NOT show claims_policy)
|
||||
- Current mitigation: The identity is keyed on `iss+sub` (never email), so display name is cosmetic. The legend displays correctly after identity is established.
|
||||
- Recommendations: (1) Add a `claims_policy` block to the example Authelia configuration in `docs/deployment.md` (or a separate `authelia-familysync-claims.yml` example). (2) Document that without claims, all users show as "Member" and that's non-blocking for v1 (they still get distinct colors via their `sub`). (3) Test Authelia claim extraction before Phase 5 push notifications are built (notification titles will need displayName).
|
||||
|
||||
**SSE heartbeat endpoint carries no secrets but could be abuse vector:**
|
||||
- Risk: `/api/sse/heartbeat` is authenticated (behind oidcAuthMiddleware) but emits only timestamps — no sensitive data. However, a malicious actor with a valid session could hold open many concurrent heartbeat streams, consuming server resources (DoS).
|
||||
- Files: `apps/api/src/routes/sse.ts`
|
||||
- Current mitigation: The endpoint is single-purpose (testing transport viability); Phase 4 will add real list-change SSE with per-user subscriptions. Resource limits are absent.
|
||||
- Recommendations: (1) For Phase 4, implement per-user connection limits (max 3 concurrent SSE streams per user). (2) Add heartbeat-timeout tracking: if a client doesn't read for 120s, close the stream. (3) Monitor stream creation rate in logs (spike = potential abuse).
|
||||
|
||||
---
|
||||
|
||||
## Performance Bottlenecks
|
||||
|
||||
**Calendar windowed query without pagination (acceptable for v1, scales to ~5000 events):**
|
||||
- Problem: GET `/api/events?start=X&end=Y` returns all occurrences in the window with no pagination. The query is efficient (indexes on `dtstart_utc`, `dtstart_date`, `hasRrule`), but response size grows with window span and recurrence expansion.
|
||||
- Files: `apps/api/src/routes/events.ts` (line 126–170)
|
||||
- Cause: No pagination implemented. For a 2-person household with ~500 events/person and heavy recurring series, a month-view response is ~2–5 KB (acceptable).
|
||||
- Improvement path: (1) Monitor response time in Phase 4 (live sync will add per-user subscriptions). (2) If response >100 KB, add cursor-based pagination to the events endpoint. (3) Consider server-side caching of expansion results per (userId, window) for frequently-accessed ranges (e.g., current month).
|
||||
|
||||
**Broker poller is full-scan every 5 minutes (acceptable for <10 members, mitigated by ctag):**
|
||||
- Problem: `poller.ts` loops all member_credentials and calls `fetchCalendars()` on each, then compares ctag. For a 2-person household with 2 Fastmail accounts (shared calendars + personal), this is ~2–4 PROPFIND/REPORT calls per cycle. Scales poorly to >10 members.
|
||||
- Files: `apps/api/src/broker/poller.ts` (line 35–77)
|
||||
- Cause: No selective polling per calendar; all calendars checked every 5 minutes.
|
||||
- Improvement path: (1) For v1 (2–4 members), current approach is fine — ~10 req/min to Fastmail. (2) For Phase 1.x (N-member expansion, per STATE.md note): track last-known ctag per calendar and skip polling if unchanged; implement WebDAV-Sync (sync-token) for delta-only fetches (RFC 6578). (3) Monitor Fastmail API rate-limit headers (`X-RateLimit-*`) in logs.
|
||||
|
||||
**Outbox worker retries backoff reaches 30 min max (acceptable, prevents spam):**
|
||||
- Problem: The outbox retry window for a failed write is capped at ~30 min (BACKOFF_SECONDS: 15+60+300+600+1800). A transient Fastmail outage lasting >30 min will abandon the write as "dead" without user notification.
|
||||
- Files: `apps/api/src/broker/outboxWorker.ts` (line 46, MAX_ATTEMPTS=5)
|
||||
- Cause: Exponential backoff with a fixed cap to prevent infinite queuing.
|
||||
- Improvement path: (1) For v1, 30 min is acceptable (household is US-based, Fastmail SLA is high). (2) For Phase 4, add a `dead-letter-queue` processor that logs unsent writes and optionally re-queues them manually. (3) Consider extending MAX_ATTEMPTS to 7–8 for a longer retry window (2–3 hours) if outages are observed.
|
||||
|
||||
---
|
||||
|
||||
## Fragile Areas
|
||||
|
||||
**CalDAV event write-back lacks conflict resolution (D-08 mitigation exists, risk remains):**
|
||||
- Files: `apps/api/src/broker/write.ts`, `apps/api/src/broker/outboxWorker.ts` (line 180–190), `docs/deployment.md` (Pitfall 14)
|
||||
- Why fragile: When a user edits an event in the app and another user edits it concurrently in the native Fastmail app, the outbox worker receives a 412 (If-Match conflict). The current behavior is to mark the outbox row as "failed" and trigger a re-sync. This is correct but provides no UI feedback to the user — they don't know their edit was rejected. If this happens repeatedly, the user will see the calendar diverge unpredictably.
|
||||
- Safe modification: (1) Add a `syncStatus` subscription in the PWA (already designed in Phase 3 Plan 03-06). The UI shows "sync conflict — your edit was rejected, event reloaded from server" in a toast. (2) If the outbox row is marked "failed", the next re-sync will pull the current server state. (3) For Phase 4+, consider implementing a "merge/overwrite" UI where the user can choose to force their edit if they're confident it's the right state. For v1, reject-and-reload is acceptable.
|
||||
|
||||
**Recurring event expansion via rrule + EXDATE is CPU-sensitive (mitigated by window cap):**
|
||||
- Files: `apps/api/src/broker/expand.ts`, `apps/api/src/routes/events.ts` (line 45, MAX_WINDOW_DAYS=90)
|
||||
- Why fragile: Expanding a 5-year-old weekly recurring event to a 90-day window generates ~50 occurrences. Expanding to a 1-year window generates ~250. If a user requests a 365-day window (not capped), the expansion becomes CPU-bound.
|
||||
- Safe modification: The MAX_WINDOW_DAYS=90 guard is in place (T-02b-02, DoS protection). No change needed. If Phase 6 adds a "year view", re-evaluate the expansion window and consider caching expanded results per (event.uid, window).
|
||||
|
||||
**OIDC session middleware dependency on @hono/oidc-auth (tied to Authelia version):**
|
||||
- Files: `apps/api/src/auth/middleware.ts`, package.json (@hono/oidc-auth: 1.8.3)
|
||||
- Why fragile: @hono/oidc-auth v1.8.3 assumes a specific OIDC metadata contract. If Authelia makes a breaking change in its .well-known/openid-configuration response, the middleware could fail silently (e.g., missing `token_endpoint`, `userinfo_endpoint`).
|
||||
- Safe modification: (1) Add a startup health check that fetches Authelia's OIDC metadata and logs an error if critical fields are missing. (2) Monitor Authelia release notes for OIDC spec changes. (3) Pin @hono/oidc-auth to 1.8.x in package.json (already done). (4) Test Authelia upgrades in a staging environment before deploying to production.
|
||||
|
||||
---
|
||||
|
||||
## Scaling Limits
|
||||
|
||||
**Single-process deployment concurrency guard in outbox worker:**
|
||||
- Current capacity: The outbox worker's drain-concurrency guard (CR-05, line 87–100) uses a module-level boolean flag. This is safe for a single-process Docker container but breaks if scaled to multiple API replicas.
|
||||
- Limit: If the API is deployed as N replicas behind a load balancer, the drain cycles can overlap and double-dispatch the same outbox row to Fastmail, causing duplicate writes.
|
||||
- Scaling path: (1) For v1 (single Unraid container), no change needed. (2) For multi-replica or Kubernetes: replace the module-level guard with a durable DB row claim (`UPDATE calendar_outbox SET status='processing' WHERE id=? AND status='pending'`). The first replica to claim wins; others skip that row. (3) Add a "processing" timeout (5 min) to prevent dead-replica claims from blocking the queue indefinitely.
|
||||
|
||||
**In-memory SSE fan-out via EventEmitter (Phase 4 dependency, acceptable for single process):**
|
||||
- Current capacity: Phase 4 will add live list-change SSE that broadcasts to connected clients. If implemented as a simple Node EventEmitter, each replica process maintains its own in-memory subscriptions. A member on replica A updates a list; the SSE fires on replica A but replica B's connections don't see it (if the member's browser is routed to replica B after the update).
|
||||
- Limit: Limited to single-process deployment or requires Redis Pub/Sub for fan-out across replicas.
|
||||
- Scaling path: (1) For v1 (single container), EventEmitter is fine. (2) For Phase 4+, if multi-replica is needed: design the SSE layer to use Redis Pub/Sub for cross-process broadcasts. Add ioredis to package.json (it's already recommended in CLAUDE.md). See PITFALLS.md §Pitfall 15 for sequence-number replay strategy.
|
||||
|
||||
**Redis not yet installed (Phase 4 dependency, scheduled for list sync):**
|
||||
- Current status: The app has no Redis dependency. Phase 4 will require Redis for pub/sub (list-change broadcasts across processes/replicas).
|
||||
- Impact: v1 is single-process; live sync works fine without Redis. Phase 4+ requires it.
|
||||
- Remediation: Add Redis to docker-compose.yml in Phase 4. ioredis client already in package.json recommendations (CLAUDE.md, Table 1). Configure connection pooling (ioredis default: 8 connections).
|
||||
|
||||
---
|
||||
|
||||
## Dependencies at Risk
|
||||
|
||||
**@hono/oidc-auth peer dependency on Authelia RFC compliance:**
|
||||
- Risk: @hono/oidc-auth relies on Authelia conforming to OIDC RFC 6749/6234. If Authelia introduces a non-standard endpoint or claim format, the middleware may fail.
|
||||
- Impact: OIDC login would break; users cannot access the app.
|
||||
- Migration plan: If Authelia breaks OIDC compatibility, replace @hono/oidc-auth with `openid-client` (a lower-level OIDC library). Estimated effort: 2–3 days to wire custom middleware. openid-client is already in CLAUDE.md as an escape hatch (Table 1, row 3).
|
||||
|
||||
**tsdav maintained by single contributor (NateLinDev/tsdav):**
|
||||
- Risk: The CalDAV client library `tsdav@2.2.2` has low maintenance activity. If a Fastmail CalDAV protocol change occurs or a critical bug is found, the library may not be updated promptly.
|
||||
- Impact: Calendar sync could break (PROPFIND, REPORT, PUT all depend on tsdav).
|
||||
- Migration plan: (1) For v1, tsdav is stable and proven in this codebase. (2) If maintenance becomes a blocker, the next option is to implement CalDAV PROPFIND/REPORT directly via fetch + xml2js (Pitfall 1 explicitly warns against this, but it's doable). Estimated effort: 1 week to implement a minimal CalDAV client. (3) Monitor tsdav GitHub issues and PRs.
|
||||
|
||||
**ical.js reference implementation (kewisch/ical.js):**
|
||||
- Risk: ical.js is the Mozilla-maintained RRULE/iCalendar reference implementation, but Mozilla does not actively develop calendar software. If a new RFC 5545 edge case is discovered (e.g., an RRULE rule that breaks ical.js), it may not be fixed quickly.
|
||||
- Impact: Recurring events could expand incorrectly (rare, but affects display).
|
||||
- Migration plan: (1) For v1, ical.js is the most reliable available. (2) If a bug is found, open an issue on GitHub; Mozilla is responsive to reference-implementation bugs. (3) Fallback: use `rrule` library only (lighter weight) if ical.js is abandoned, but rrule is less comprehensive for EXDATE/RECURRENCE-ID handling.
|
||||
|
||||
---
|
||||
|
||||
## Missing Critical Features
|
||||
|
||||
**Single-occurrence recurring event override (deferred to v1.x):**
|
||||
- Problem: A user cannot edit or delete a single occurrence of a recurring event (e.g., "skip next Tuesday's meeting"). The edit-as-move write path (D-04) supports full-series edits only.
|
||||
- Blocks: Users frustrated when they want to reschedule one instance.
|
||||
- Deferred reason: Requires RECURRENCE-ID write-back (RFC 5545) and complex VCALENDAR patching. Estimated effort: 2–3 days of implementation + testing. For v1, edit-all is acceptable for a 2-person household.
|
||||
- Resolution approach: Phase 6 or v1.x — implement a "Edit this and all following" option that re-dates the RRULE UNTIL and creates a new series from the edit date onward.
|
||||
|
||||
**Notification subscription health-check (CRITICAL for Phase 5, deferred to Phase 5 implementation):**
|
||||
- Problem: iOS silently revokes Web Push subscriptions after 3 silent push events (Pitfall 9). The app must detect this and re-subscribe automatically.
|
||||
- Blocks: Phase 5 (push notifications) cannot be considered production-ready without this.
|
||||
- Missing implementation: No subscription health-check exists in the PWA yet. The service worker needs to call `pushManager.getSubscription()` on every page open and compare the endpoint to the server's stored endpoint; if they differ, re-subscribe.
|
||||
- Resolution approach: Phase 5 must include health-check implementation as a prerequisite, not a polish task.
|
||||
|
||||
---
|
||||
|
||||
## Test Coverage Gaps
|
||||
|
||||
**Events API route (GET /api/events, POST /create, PATCH /edit, DELETE /delete) has integration-level testing but lacks edge cases:**
|
||||
- What's not tested: (1) Window boundary conditions (start=end, off-by-one day shifts). (2) Recurring all-day events with complex EXDATE. (3) Concurrent edit conflict (412 handling). (4) Ownership assertions with mixed owned + shared calendars.
|
||||
- Files: `apps/api/tests/routes/events.test.ts` (126 lines, covers happy paths + 400/403 error cases)
|
||||
- Risk: Edge cases in expansion or ownership filtering could silently pass tests and break in production.
|
||||
- Priority: MEDIUM — add 10–15 test cases before Phase 4 (live sync will depend on ownership filtering being bulletproof).
|
||||
|
||||
**Outbox worker state machine (retry backoff, edit-as-move ordering, dead-letter) has unit tests but lacks end-to-end CalDAV integration:**
|
||||
- What's not tested: (1) Outbox row with a real Fastmail endpoint (mocked in tests). (2) 412 conflict response from Fastmail + re-sync flow. (3) Concurrent outbox rows from the same list (edit+delete pair ordering under network failures). (4) Recovery after a multi-hour Fastmail outage.
|
||||
- Files: `apps/api/tests/broker/outboxWorker.test.ts` (state-machine tests only)
|
||||
- Risk: Silent data loss if outbox row ordering is wrong under failures; list sync will depend on correct write ordering.
|
||||
- Priority: HIGH — add integration tests before Phase 4. Mock Fastmail CalDAV responses (conflict, transient, success) and verify state transitions.
|
||||
|
||||
**PWA EventForm timezone serialization (fixed in Phase 3, regression test exists but limited scope):**
|
||||
- What's not tested: (1) Daylight Saving Time transitions (create event on March 12, spring-forward boundary). (2) Cross-timezone consistency (create event in Toronto, verify UTC serialization, reload in UTC, confirm display is Toronto wall-clock). (3) All-day event edge cases (midnight boundary serialization).
|
||||
- Files: `apps/pwa/src/lib/eventDateTime.test.ts` (5 cases: timed → UTC, all-day → DATE, round-trip)
|
||||
- Risk: Similar timezone bug could reappear if eventDateTime.ts is refactored without comprehensive DST testing.
|
||||
- Priority: MEDIUM — add 5–10 DST/all-day edge cases to the test suite before Phase 6 (UX polish will touch date/time handling).
|
||||
|
||||
**PWA service worker and offline behavior untested:**
|
||||
- What's not tested: (1) Service worker install, activation, and update lifecycle. (2) Offline calendar view (reads from cache). (3) Offline list mutation (queues for sync). (4) Cache expiration strategy.
|
||||
- Files: Service worker is auto-generated by vite-plugin-pwa; offline behavior is unimplemented in Phase 1–3.
|
||||
- Risk: Phase 4's offline queue and Phase 5's background sync depend on correct SW lifecycle. Silent failures in SW updates could leave the wife on a stale version.
|
||||
- Priority: MEDIUM — Phase 4 should include SW unit tests (simulate offline, verify cache reads, verify mutation queue behavior).
|
||||
|
||||
**Mobile-specific behavior (iOS push, PWA standalone mode, permissions) untested by vitest:**
|
||||
- What's not tested: (1) iOS 16.4+ push subscription (requires real device). (2) Standalone PWA launch (requires Add-to-Home-Screen). (3) Permission request flow (requires user gesture). (4) Camera/location permissions (out of scope for v1, but worth listing).
|
||||
- Files: Not applicable (device-only testing).
|
||||
- Risk: High impact if broken (wife can't install, can't receive notifications). Mitigated by human UAT (Phase 3 Gate 2 item 4).
|
||||
- Priority: MEDIUM — document a manual iOS test checklist in Phase 5 (must run before ship). Playwright can test browser-side behavior; device-side requires manual verification.
|
||||
|
||||
---
|
||||
|
||||
## Architectural Constraints & Anti-Patterns
|
||||
|
||||
**Single-process assumption in outbox drain guard (CR-05, documented but constrains scaling):**
|
||||
- Constraint: The module-level boolean flag `let isProcessing = false` in outboxWorker.ts assumes a single Node.js process. This is correct for the Unraid single-container deployment but breaks if scaled horizontally.
|
||||
- Consequence: Multi-replica deployments MUST implement a durable DB claim (UPDATE … WHERE status='processing') before the API is horizontally scaled.
|
||||
- Workaround: Documented in code comment (line 91–100). Clear and easy to address when scaling is needed.
|
||||
|
||||
**No pagination on calendar events endpoint (acceptable for v1, design assumption):**
|
||||
- Constraint: GET /api/events returns all occurrences in the window with no pagination. Designed for a 90-day max window and <1000 occurrences per window (acceptable for 2-person household).
|
||||
- Consequence: Very large windows or households with hundreds of recurring events could generate multi-MB responses.
|
||||
- Workaround: MAX_WINDOW_DAYS=90 guard prevents DoS. For Phase 4+, if response size exceeds 500 KB, add cursor pagination.
|
||||
|
||||
**Dev-auth bypass is development-only but deployment-critical (configuration risk):**
|
||||
- Constraint: The bypass is designed for local development (NODE_ENV !== 'production' + DEV_AUTH_BYPASS=true). If the bypass is accidentally enabled in production, the OIDC guard is completely bypassed.
|
||||
- Consequence: Unauthenticated API access if misconfigured.
|
||||
- Workaround: (1) .env.example has DEV_AUTH_BYPASS commented out. (2) docker-compose.yml MUST NOT include DEV_AUTH_BYPASS in env. (3) Documented in docs/deployment.md. Recommended: add a startup assertion to double-check.
|
||||
|
||||
---
|
||||
|
||||
## Infrastructure & Deployment Concerns
|
||||
|
||||
**Drizzle migrations require manual SQL review (no auto-apply in Docker):**
|
||||
- Issue: The app does not auto-migrate on startup. The `drizzle-kit push` command is unsafe on MariaDB. Manual `drizzle-kit migrate` must be run once per DB version before the app starts.
|
||||
- Files: `apps/api/src/db/migrations/`, `docs/deployment.md` (Step 3: `drizzle-kit push` is the documented command, but should be `migrate` or `generate+migrate` for production safety)
|
||||
- Impact: If the operator forgets to migrate after pulling a new schema, the app will crash on startup (missing tables). The error message should be clear.
|
||||
- Fix approach: (1) Update `docs/deployment.md` Step 3 to use `migrate` instead of `push`. (2) Add a startup health check in `src/db/client.ts` that verifies all expected tables exist; fail with a clear message if any are missing. (3) Document the migration process in a DEPLOYMENT.md subsection.
|
||||
|
||||
**Pangolin SSE idle timeout dependency (D-14, issue #1034) verified but residual risk remains:**
|
||||
- Issue: SSE streams can be cut by proxy idle-timeout. The Phase 4 entry gate smoke test PASSED (6 min without cut), but only tested on the test domain `familysync-dev.bergerhouse.net`.
|
||||
- Files: `docs/deployment.md` (line 165–170), `.planning/phases/04-shared-lists-live-sync/04-CONTEXT.md`
|
||||
- Impact: If the production Pangolin idle-timeout is lower than the test rig, SSE will be cut during live list sync. Users will experience brief disconnects (mitigated by reconnect logic in Phase 4).
|
||||
- Current mitigation: Documented in deployment.md. The operator must set Pangolin's idle-timeout to ≥120s (recommended 300s) when deploying to production.
|
||||
- Residual risk: If Pangolin is misconfigured and SSE is cut, the fallback (D-12 polling every 5s) will maintain sync but with degraded latency (5s vs real-time). Phase 4 must implement the polling fallback.
|
||||
|
||||
---
|
||||
|
||||
## Known Limitations (Documented as Design Decisions)
|
||||
|
||||
**Personal calendar sharing requires manual Fastmail setup (D-16 CAL-08 spike result):**
|
||||
- Limitation: The two household members' personal Fastmail calendars are accessed via per-member app passwords (not a shared broker token). This requires each member to generate an app password and register it in the app.
|
||||
- Impact: Acceptable. The unified view works correctly and scales to shared + personal calendars.
|
||||
- Status: GO decision (CAL-08-DECISION.md, Phase 1).
|
||||
|
||||
**Recurring event edit supports edit-all only (single-occurrence override deferred to v1.x):**
|
||||
- Limitation: The write path does not support RECURRENCE-ID overrides. Editing a recurring event changes all future occurrences.
|
||||
- Impact: Users cannot reschedule a single meeting. For a 2-person household, edit-all is acceptable.
|
||||
- Status: Documented in STATE.md (deferred items), Phase 6 planning.
|
||||
|
||||
**EU DMA compliance risk for EU-based households (Pitfall 11):**
|
||||
- Limitation: iOS 17.4+ in EU countries removes standalone PWA mode and push support due to Digital Markets Act. FamilySync's push notifications would not work for an EU user.
|
||||
- Impact: If the household moves to EU or uses EU Apple IDs, notifications are unavailable.
|
||||
- Status: This is a Canadian household (me@lucasberger.ca, .ca domain, Unraid self-hosted). Documented as not applicable but worth flagging for future.
|
||||
- Fix approach: Monitor for EU regulatory changes; if the household moves, switch to email or in-app notification fallback for v1.x.
|
||||
|
||||
---
|
||||
|
||||
*Concerns audit: 2026-06-09*
|
||||
Reference in New Issue
Block a user