style(13-03): apply Prettier formatting across repo
Mechanical reformat — no logic changes. 398 files changed, 19125 insertions(+), 16457 deletions(-). Prettier 3.8.4 with .prettierrc (singleQuote:true, semi:true, tabWidth:2, trailingComma:all, printWidth:100). Isolated per D-13-08 for reviewability.
This commit is contained in:
Lucas Berger
@@ -35,6 +35,7 @@ The inverse is equally dangerous: if the editor sends `reminderMinutes: 0` (mean
|
||||
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:**
|
||||
|
||||
- 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.
|
||||
@@ -48,6 +49,7 @@ Per-event reminders phase (VALARM authoring). The VALARM-preserve logic must lan
|
||||
|
||||
**What goes wrong:**
|
||||
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.
|
||||
|
||||
@@ -64,6 +66,7 @@ Build the TRIGGER using `ICAL.Duration.fromSeconds(-reminderMinutes * 60)` and s
|
||||
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:**
|
||||
|
||||
- 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.
|
||||
@@ -87,6 +90,7 @@ In the event form UI: disable or hide the reminder selector when `allDay: true`.
|
||||
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 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.
|
||||
@@ -116,6 +120,7 @@ For the variable-window query: instead of scanning a fixed `(now, now+16min]` wi
|
||||
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 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).
|
||||
@@ -147,6 +152,7 @@ A more dangerous double-drain scenario arises if the event-driven trigger is imp
|
||||
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:**
|
||||
|
||||
- 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).
|
||||
@@ -174,6 +180,7 @@ Always enqueue the CREATE row before the DELETE row in the HTTP handler, matchin
|
||||
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.
|
||||
@@ -199,6 +206,7 @@ In the `@hono/zod-validator` middleware for the app-password body schema, always
|
||||
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:**
|
||||
|
||||
- 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.
|
||||
|
||||
@@ -225,6 +233,7 @@ Alternatively, use a DB-stored `setup_completed_at` timestamp in a `settings` ta
|
||||
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:**
|
||||
|
||||
- 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.
|
||||
@@ -248,6 +257,7 @@ Hono's middleware scoping is based on route prefix at mount time, not at route d
|
||||
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:**
|
||||
|
||||
- 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.
|
||||
|
||||
@@ -260,6 +270,7 @@ Admin Settings phase. Integration test for 403 on non-admin access is the accept
|
||||
|
||||
**What goes wrong:**
|
||||
The setup wizard collects VAPID keypair and validates the Fastmail app password. A tempting shortcut: store the VAPID keys in the `settings` DB table for easy retrieval later. The problem: `VAPID_PRIVATE_KEY` is a signing key — equivalent to a private TLS key. Storing it in the DB means it is:
|
||||
|
||||
- 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.
|
||||
@@ -275,6 +286,7 @@ Hard rule: `VAPID_PRIVATE_KEY` and `APP_PASSWORD_ENCRYPTION_KEY` never touch the
|
||||
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:**
|
||||
|
||||
- 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.
|
||||
@@ -296,6 +308,7 @@ The Docker `HEALTHCHECK` for MariaDB using `mysqladmin ping` returns true as soo
|
||||
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:**
|
||||
|
||||
- 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.
|
||||
@@ -321,6 +334,7 @@ In the first CI plan, write a minimal "hello world" workflow that only checks `n
|
||||
For Docker image build/publish: verify the runner has Docker daemon access. On Unraid self-hosted runners, Docker may require `--privileged` or specific socket mounts that need runner configuration.
|
||||
|
||||
**Warning signs:**
|
||||
|
||||
- `pnpm: command not found` in CI output.
|
||||
- `node` resolves to a version older than 22 in CI but not locally.
|
||||
- `actions/setup-node` step shows as skipped or errored in the Gitea Actions UI.
|
||||
@@ -342,6 +356,7 @@ Docker CLI login via `-p` flag is the most common example in docs. GitHub Action
|
||||
Use `docker login --password-stdin` with the token piped via stdin rather than a command-line argument: `echo "${{ secrets.REGISTRY_TOKEN }}" | docker login -u "${{ secrets.REGISTRY_USER }}" --password-stdin registry.example.com`. Register all credentials as Gitea repository secrets, not as environment variables in the workflow YAML. Verify the Gitea version supports secret masking in the Actions log (Gitea ≥ 1.19 for Actions support; secret masking behavior varies by version).
|
||||
|
||||
**Warning signs:**
|
||||
|
||||
- Registry token or password visible as plaintext in the Gitea Actions job log.
|
||||
- `docker login` command line includes `-p <token>` in the log output.
|
||||
- `secrets.REGISTRY_TOKEN` is undefined in the workflow (token was set as env var, not secret).
|
||||
@@ -363,6 +378,7 @@ Playwright storage state is file-based and not automatically refreshed. Develope
|
||||
Do not use a static stored storage state for tests that run against the production OIDC path. Instead, implement a programmatic login helper that runs the OIDC authorization code flow at the start of each test session (or once per test run) and stores the resulting session. For the `DEV_AUTH_BYPASS` dev environment, the harness sets `DEV_AUTH_BYPASS=true` and skips the storage state entirely. The mobile viewport emulation does not require real OIDC — use `DEV_AUTH_BYPASS` for the automated harness; keep real OIDC tests as manual/human gates.
|
||||
|
||||
**Warning signs:**
|
||||
|
||||
- Playwright runs fail with `Expected 200 OK but got 302 Found` after leaving the storage state untouched for more than one day.
|
||||
- Tests that exercise `/api/*` routes return HTML (the Authelia login page) instead of JSON.
|
||||
- The same test suite passes reliably in `DEV_AUTH_BYPASS=true` mode but fails intermittently in production-auth mode.
|
||||
@@ -376,6 +392,7 @@ Mobile-browser testing phase. The storage state strategy must be decided before
|
||||
|
||||
**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.
|
||||
@@ -387,6 +404,7 @@ Workbox's cache-first strategy for static assets and stale-while-revalidate for
|
||||
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.
|
||||
@@ -398,45 +416,45 @@ Mobile-browser testing phase. The Playwright context setup must explicitly handl
|
||||
|
||||
## Technical Debt Patterns
|
||||
|
||||
| Shortcut | Immediate Benefit | Long-term Cost | When Acceptable |
|
||||
|----------|-------------------|----------------|-----------------|
|
||||
| Building VALARM on top of `buildVeventString` without the preserve-on-edit path | Faster to implement | Strips native-client alarms on every edit; user data loss | Never — preserve path must ship with VALARM authoring |
|
||||
| uid-only dedup key in sentReminders when lead times become variable | No migration needed | Duplicate pushes or missed re-fires after reschedule | Never for production; acceptable in tests with a fixed lead |
|
||||
| Calling `runOutboxDrain()` directly from event trigger instead of setting a flag | Simpler code | Bypasses `isDraining` atomicity, potential double-drain | Never — always funnel through the single setInterval-controlled path |
|
||||
| Setup wizard that accepts `APP_PASSWORD_ENCRYPTION_KEY` via the API | Simpler UX for initial setup | Entire encryption model is broken | Never — key stays in env/secrets only |
|
||||
| Static storage-state.json checked into the repo | Zero-effort Playwright auth | Tests fail silently after TTY expiry; potential credential leak | Never — programmatic refresh or DEV_AUTH_BYPASS only |
|
||||
| `docker login -p $TOKEN` in CI command | Quick to write | Token appears in CI logs if secret not masked | Never — always use --password-stdin |
|
||||
| No readiness wait for MariaDB service in CI | Simpler YAML | Flaky CI: timing-dependent ECONNREFUSED failures | Never — readiness wait is 3 lines and prevents ghost failures |
|
||||
| Shortcut | Immediate Benefit | Long-term Cost | When Acceptable |
|
||||
| -------------------------------------------------------------------------------- | ---------------------------- | --------------------------------------------------------------- | -------------------------------------------------------------------- |
|
||||
| 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 |
|
||||
|
||||
---
|
||||
|
||||
## Integration Gotchas
|
||||
|
||||
| Integration | Common Mistake | Correct Approach |
|
||||
|-------------|----------------|------------------|
|
||||
| ical.js VALARM | Using `addPropertyWithValue('trigger', '-PT15M')` (string) | Build `ICAL.Duration.fromSeconds(-N*60)` and use the Duration object as the property value |
|
||||
| ical.js VALARM | Not round-tripping preserved VALARMs through ical.js parse→serialize | Parse the sub-component from rawVevent and re-add via ical.js API; do not insert raw text |
|
||||
| outboxWorker VALARM | Rebuilding VEVENT from scratch drops native-client VALARMs on update | Extend buildVeventString to accept a `valarms` parameter; populate from rawVevent extract on update path |
|
||||
| reminderScheduler dedup | uid-only Map key breaks when per-event leads vary | Key on `uid + ':' + dtstartMs`; prune by dtstartMs |
|
||||
| event-driven drain | Calling `runOutboxDrain()` from pub/sub subscriber before `isDraining` is set | Use a single drain path via `drainRequested` flag checked in the setInterval callback |
|
||||
| Gitea Actions | Using GitHub Actions-specific action IDs | Probe the runner first; use Gitea-compatible alternatives or install tools explicitly |
|
||||
| Gitea Actions MariaDB | Relying on container health == connection ready | Add explicit `mysqladmin ping` retry loop after healthcheck passes |
|
||||
| Playwright mobile harness | Static storage-state.json with expiring session cookie | Use `DEV_AUTH_BYPASS=true` for automated harness; programmatic OIDC login for real-auth tests |
|
||||
| Playwright + Vite PWA | Service worker from previous run intercepting requests | Set `serviceWorkers: 'block'` or unregister SWs explicitly in test context setup |
|
||||
| Setup wizard | Accepting `APP_PASSWORD_ENCRYPTION_KEY` via the POST body | Validate the env is present by performing a test operation; never accept the key value over the network |
|
||||
| Admin settings route | Zod error passthrough leaking app-password input | Custom `hook` in zod-validator: return generic 400, never the Zod error object |
|
||||
| Integration | Common Mistake | Correct Approach |
|
||||
| ------------------------- | ----------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------- |
|
||||
| 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 |
|
||||
|
||||
---
|
||||
|
||||
## Security Mistakes
|
||||
|
||||
| Mistake | Risk | Prevention |
|
||||
|---------|------|------------|
|
||||
| Admin route not protected inside adminRouter (only in parent app) | Any authenticated member can call admin endpoints | Apply guard middleware inside the sub-router, not only in the parent app mount |
|
||||
| Setup endpoint lacks "already-set-up" guard | Post-setup endpoint rewrites credentials without auth | Check `member_credentials` existence + VAPID env on every setup route invocation; return 423 if already configured |
|
||||
| VAPID_PRIVATE_KEY stored in DB | Private signing key accessible to DB-level access | VAPID private key in env/secrets only; DB stores public key and subject only |
|
||||
| App-password in Zod error response | Plaintext credential in HTTP response and server logs | Custom Zod hook for all routes that accept credential input |
|
||||
| `docker login -p` in CI YAML | Registry token in CI logs | `--password-stdin` only; token as Gitea secret, not YAML env var |
|
||||
| Mistake | Risk | Prevention |
|
||||
| ----------------------------------------------------------------- | ----------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------ |
|
||||
| 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 |
|
||||
|
||||
---
|
||||
|
||||
@@ -457,38 +475,38 @@ Mobile-browser testing phase. The Playwright context setup must explicitly handl
|
||||
|
||||
## Recovery Strategies
|
||||
|
||||
| Pitfall | Recovery Cost | Recovery Steps |
|
||||
|---------|---------------|----------------|
|
||||
| VALARM strips native alarms on edit | MEDIUM | Add valarms preserve path to buildVeventString + outboxWorker update branch; no migration needed; existing rawVevent data is authoritative |
|
||||
| TRIGGER VALUE=TEXT bug | LOW | Fix Duration construction in buildVeventString; no data migration (rawVevent already has correct alarms from server) |
|
||||
| uid-only dedup causing duplicate push | LOW | Change Map key to uid:dtstartMs; restart clears in-memory state; no DB change |
|
||||
| Double-drain from concurrent triggers | MEDIUM | Refactor event-driven trigger to drainRequested flag; requires load testing to confirm no more duplicate PUTs |
|
||||
| Admin route bypassed (no inner guard) | LOW | Add `.use('*', adminGuard)` inside adminRouter; deploy |
|
||||
| VAPID private key in DB | HIGH | Rotate VAPID keypair; clear all push subscriptions (all devices must re-subscribe); remove DB column via migration |
|
||||
| CI flaky MariaDB race | LOW | Add readiness wait loop to workflow YAML; re-run |
|
||||
| Playwright storage state stale | LOW | Switch to DEV_AUTH_BYPASS mode for automated tests; remove static state file |
|
||||
| Pitfall | Recovery Cost | Recovery Steps |
|
||||
| ------------------------------------- | ------------- | ------------------------------------------------------------------------------------------------------------------------------------------ |
|
||||
| VALARM strips native alarms on edit | MEDIUM | Add valarms preserve path to buildVeventString + outboxWorker update branch; no migration needed; existing rawVevent data is authoritative |
|
||||
| TRIGGER VALUE=TEXT bug | LOW | Fix Duration construction in buildVeventString; no data migration (rawVevent already has correct alarms from server) |
|
||||
| uid-only dedup causing duplicate push | LOW | Change Map key to uid:dtstartMs; restart clears in-memory state; no DB change |
|
||||
| Double-drain from concurrent triggers | MEDIUM | Refactor event-driven trigger to drainRequested flag; requires load testing to confirm no more duplicate PUTs |
|
||||
| Admin route bypassed (no inner guard) | LOW | Add `.use('*', adminGuard)` inside adminRouter; deploy |
|
||||
| VAPID private key in DB | HIGH | Rotate VAPID keypair; clear all push subscriptions (all devices must re-subscribe); remove DB column via migration |
|
||||
| CI flaky MariaDB race | LOW | Add readiness wait loop to workflow YAML; re-run |
|
||||
| Playwright storage state stale | LOW | Switch to DEV_AUTH_BYPASS mode for automated tests; remove static state file |
|
||||
|
||||
---
|
||||
|
||||
## Pitfall-to-Phase Mapping
|
||||
|
||||
| Pitfall | Prevention Phase | Verification |
|
||||
|---------|------------------|--------------|
|
||||
| VALARM strips native alarms on edit | Per-event reminders (VALARM authoring) | Integration test: create event via native client with alarm, edit via FamilySync, verify PUT payload contains original VALARM |
|
||||
| TRIGGER VALUE=TEXT serialization | Per-event reminders (VALARM authoring) | Unit test: serialize VALARM, parse back, assert no VALUE=TEXT |
|
||||
| All-day event VALARM silently no-ops | Per-event reminders (VALARM authoring) | UI test: all-day event form has no reminder field or field is disabled |
|
||||
| Variable-lead dedup produces duplicate push | Per-event reminders (scheduler generalization) | Unit test: fire reminder, reschedule event earlier, fire again — assert two pushes sent |
|
||||
| Double-drain from concurrent event-driven trigger | Event-driven outbox drain | Load test: enqueue 10 rows rapidly, assert each CalDAV PUT issued exactly once |
|
||||
| Event-driven drain breaks create-before-delete | Event-driven outbox drain | Integration test: edit-as-move under rapid enqueue; original event not deleted before new one created |
|
||||
| Admin app-password echoed in error | Admin Settings | Unit test: POST invalid password to settings route; assert response has no credential value |
|
||||
| Unauthenticated setup endpoint stays live | Setup wizard | Integration test: POST to setup endpoint after first-run completes; assert 423 |
|
||||
| Admin role check missing inside sub-router | Admin Settings | Integration test: non-admin authenticated user hits admin route; assert 403 |
|
||||
| VAPID key stored in DB | Setup wizard | Schema review before migration is written; CI lint check for column names containing `private_key` |
|
||||
| Gitea CI MariaDB readiness race | Gitea CI | CI log audit: readiness loop appears before any `drizzle-kit migrate` invocation |
|
||||
| Gitea runner missing Node 22 / pnpm | Gitea CI | First CI job: node/pnpm version probe step before any install or test |
|
||||
| Docker registry token in CI logs | Gitea CI | CI log audit: no plaintext token visible; all registry credentials use --password-stdin |
|
||||
| Playwright storage state stale | Mobile-browser testing | Test suite passes on day 2 without recapturing storage state (DEV_AUTH_BYPASS mode eliminates TTL) |
|
||||
| Production service worker intercepts Playwright | Mobile-browser testing | Playwright context uses `serviceWorkers: 'block'`; verified in trace that no responses are SW-sourced |
|
||||
| Pitfall | Prevention Phase | Verification |
|
||||
| ------------------------------------------------- | ---------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------- |
|
||||
| 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 |
|
||||
|
||||
---
|
||||
|
||||
@@ -504,5 +522,6 @@ Mobile-browser testing phase. The Playwright context setup must explicitly handl
|
||||
- Playwright docs — `browserContext.serviceWorkers`, `storageState`, context lifecycle
|
||||
|
||||
---
|
||||
*Pitfalls research for: FamilySync v1.1 Operability & Polish*
|
||||
*Researched: 2026-06-10*
|
||||
|
||||
_Pitfalls research for: FamilySync v1.1 Operability & Polish_
|
||||
_Researched: 2026-06-10_
|
||||
|
||||
Reference in New Issue
Block a user