48 KiB
Pitfalls Research — v1.1 Operability & Polish
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.
setIntervalonly. node-cron 4.2.1 silently skips all ticks in the long-lived API process. drizzle-kit generate+migrate, neverpush.pushemits a false destructive diff on populated MariaDB.- API integration tests need a real MariaDB (port-bound dev compose,
DB_HOST=127.0.0.1,.envcreds). Tests live inapps/api/tests/, neversrc/. - 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_usernamefrom the ID token by default — needs aclaims_policyor these fields are absent.
Critical Pitfalls
Pitfall 1: VALARM Round-Trip Strips Existing Alarms From Native Clients on Edit
What goes wrong:
The current buildVeventString in broker/vevent.ts constructs a fresh VCALENDAR with only the properties it knows about (UID, SUMMARY, DTSTART, DTEND, RRULE, LOCATION, DESCRIPTION). When v1.1 adds VALARM authoring, a naive approach adds VALARM components to new-creates only. On an edit (update path in outboxWorker.ts), the worker re-builds the VEVENT from form payload — not from the stored rawVevent. Any VALARM that was added by a native client (Fastmail app, Apple Calendar) will be silently dropped from the PUT payload. The event arrives at Fastmail without the alarm. The user loses their native-client reminder with no warning.
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:
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:
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.
rawVeventincalendar_eventshasBEGIN:VALARMbut the PUT payload does not.
Phase to address:
Per-event reminders phase (VALARM authoring). The VALARM-preserve logic must land in the same PR as buildVeventString VALARM support — not as a follow-up.
Pitfall 2: TRIGGER Value-Type Mismatch Silently Produces Broken VALARM
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.
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:
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:
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:
- 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: Per-event reminders phase. Unit test the VALARM serialization before any end-to-end reminder test.
Pitfall 3: All-Day Event VALARM Timezone Semantics Are Undefined
What goes wrong:
For an all-day event (DTSTART;VALUE=DATE), the meaning of TRIGGER:-PT15M is ambiguous. RFC 5545 requires that a DURATION TRIGGER on an all-day event be evaluated against DTSTART as a DATE — which has no time component — resulting in undefined behavior in most implementations. Apple Calendar interprets it as "15 minutes before midnight of the start date in local time." Fastmail ignores VALARM on all-day events entirely in some tested configurations. Android may fire the alarm at midnight UTC.
Why it happens:
The project already correctly excludes all-day events from the reminder scheduler (reminderScheduler.ts, WHERE allDay=false). But if VALARM is stored on an all-day event (because the user created an all-day event and selected a reminder), the scheduler's WHERE clause means it silently never fires — which is correct behavior — but the user sees a reminder field in the UI and expects it to work.
How to avoid:
In the event form UI: disable or hide the reminder selector when allDay: true. If the API receives a create/update payload with allDay: true and a non-null reminderMinutes, strip the alarm and log a warning — do not store a VALARM that will silently not fire. Document this as a known constraint.
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.
Phase to address: Per-event reminders phase. UI constraint and API guard belong in the same plan.
Pitfall 4: Duplicate Push When Generalizing the Fixed-Window Dedup to Per-Event Lead Times
What goes wrong:
The current reminderScheduler.ts deduplication key is uid alone (sentReminders Map). The fixed 16-minute catch-up window ensures an event stays in-window across at most a few consecutive 1-minute ticks. When v1.1 changes the lead to a per-event value (e.g., event A has a 30-minute lead, event B has a 2-hour lead), the window must widen — or the scan logic must change — to accommodate variable leads. There are two failure modes:
-
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. -
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
uidalone, with thesentReminders.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 insentReminderswith the old dtstart (3pm), which has NOT yet passednow, so the CR-01 pruning has not removed it. The reminder for 2pm silently does not fire.
Why it happens: The uid-only dedup was designed for the fixed-15-minute lead where the dedup window is short and rescheduling edge cases are low-probability. Per-event leads break both assumptions.
How to avoid:
Change the dedup key from uid alone to uid + ':' + dtstartMs. This makes the dedup per-(event, scheduled-time), not per-event. A rescheduled event has a different dtstart and gets a new dedup entry. The sentReminders map still prunes on dtstartMs <= now.
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 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:
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 5: Double-Drain When Event-Driven Trigger and 15s setInterval Both Fire
What goes wrong:
The v1.1 event-driven drain adds a trigger (Redis pub/sub message, or direct runOutboxDrain() call) that fires immediately when a new row is enqueued. The 15s setInterval fallback continues to run. Both can invoke runOutboxDrain() concurrently. The existing isDraining module-level flag provides single-execution within the same JS tick, but there is a subtler race:
- T=0: Route enqueues row. Event-driven trigger calls
runOutboxDrain().isDrainingis set. - T=0.5s: Drain in progress. 15s interval fires.
isDrainingis true — no-op. Correct. - T=1s: Drain completes.
isDrainingreset 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().isDrainingis 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:
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:
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).
Phase to address:
Event-driven outbox drain phase. The drain trigger design must be reviewed before implementation; the isDraining guard docs already note the single-process limitation.
Pitfall 6: Event-Driven Drain Breaks Create-Before-Delete Ordering Under Concurrent Enqueues
What goes wrong: The edit-as-move path enqueues two rows (delete old uid, create new uid) in the same request handler. With the 15s poll, both rows are almost always in the DB before the next drain cycle. With event-driven drain (trigger fires on enqueue), a race is possible:
- Request handler enqueues the CREATE row. Event-driven trigger fires immediately. Drain runs. Create is dispatched successfully.
isDrainingresets. - 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:
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:
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:
- App password appears in any log output or API response body.
- The Zod error response for the settings route includes a
receivedormessagefield containing password-like strings.
Phase to address:
Admin Settings phase. Security review of the settings route before first deployment; treat app-password fields the same as OIDC_CLIENT_SECRET — never log, never echo.
Pitfall 8: Unauthenticated Setup Endpoint Left Live After First Run
What goes wrong:
The setup wizard endpoint must be accessible before any member has authenticated (no credentials exist yet, so OIDC cannot be used to protect it). The typical implementation: mount the setup routes outside the app.use('/api/*', oidcAuthMiddleware()) guard, and detect "first run" by checking whether any member_credentials row (or VAPID env) exists. The failure mode: the first-run check passes once. But if the developer forgets to add an "already-set-up" guard, or the check looks at the wrong table, the endpoint remains callable after setup — allowing anyone who can reach the internal network (or the Pangolin public URL) to overwrite the app password without authentication.
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: 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:
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:
- 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: Setup wizard phase. The guard must be the first thing implemented; test the guard before testing the happy path.
Pitfall 9: Admin Role Check Bypassed by Missing Middleware Wiring
What goes wrong:
The admin Settings routes require a role check (only the operator/admin user can manage credentials and toggle is_shared). The standard pattern in this codebase is Hono middleware layered on a route prefix. The failure mode: the admin middleware is defined but not wired to the correct prefix. For example, if the admin check is added to eventsRouter instead of a new adminRouter, or if the route is mounted at /api/admin but the middleware guard applies to /api/settings/*, admin routes are reachable by any authenticated member.
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:
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:
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.tsbut the admin routes are in a separateadminRouterwith no internal middleware.
Phase to address: Admin Settings phase. Integration test for 403 on non-admin access is the acceptance criterion.
Pitfall 10: App Password and VAPID Keys Stored in DB When They Must Stay in Env
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.
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: 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:
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:
- DB schema has a
vapid_private_keycolumn. - Any API response that includes
VAPID_PRIVATE_KEYorAPP_PASSWORD_ENCRYPTION_KEYvalues. - Wizard stores all config to DB and reads it back on next startup instead of requiring env vars.
Phase to address: Setup wizard phase. Schema design review before migration is written. Secret-in-DB is a hard blocker for the phase gate.
Pitfall 11: Gitea Actions MariaDB Service Container Readiness Race
What goes wrong:
Gitea Actions (like GitHub Actions) supports services: containers. The MariaDB service starts, but the container reaching healthy in Docker does not mean MariaDB is accepting connections on port 3306. mysqld takes several seconds to initialize after the container starts. If the CI job proceeds to drizzle-kit migrate or integration test commands immediately after the service health check passes, it races with MariaDB initialization and fails with ECONNREFUSED or Access denied errors that look like test failures but are actually timing issues.
Why it happens:
The Docker HEALTHCHECK for MariaDB using mysqladmin ping returns true as soon as the network socket is open, which happens before all privilege tables are initialized. The healthcheck.interval in the Gitea service definition controls how often the check runs, but the first check may pass before MariaDB has fully bootstrapped.
How to avoid:
Add a wait-for-it or until mysqladmin ping --silent; do sleep 1; done step in the CI workflow after the service is declared healthy, before running any DB command. Or use a longer healthcheck.start_period in the service definition (e.g., 30 seconds). Also: set MARIADB_ROOT_PASSWORD, MARIADB_DATABASE, MARIADB_USER, MARIADB_PASSWORD in the service env and use those same credentials in the integration test step — do not assume the root user is reachable from the test runner without a password.
Warning signs:
- CI passes on re-run but fails on first run of a PR (timing-dependent).
ECONNREFUSEDorError: connect ECONNREFUSED 127.0.0.1:3306in CI logs.- Tests that pass locally with a warm MariaDB fail in CI cold-start.
Phase to address: Gitea CI phase. The readiness wait must be in the first draft of the workflow YAML; do not add it after the first CI failures.
Pitfall 12: Gitea Actions Self-Hosted Runner Missing Node 22 or pnpm
What goes wrong:
The self-hosted Gitea Actions runner on Unraid may have an older Node.js version globally available, or may have no pnpm installation, or may have a corepack-managed pnpm that requires activation. If the CI workflow assumes the runner environment matches the dev machine, pnpm install fails with pnpm: command not found, or node --version returns 18 instead of 22.
A related issue: the workflow may use actions/setup-node (a GitHub Actions action) which is not available in Gitea Actions, or uses a Gitea-specific variant that requires different configuration.
Why it happens:
Gitea Actions is not GitHub Actions. Many popular actions (actions/checkout, actions/setup-node, actions/cache) have Gitea-compatible alternatives, but their names and behavior differ subtly. If the workflow is copied from a GitHub Actions template, some steps silently fail or are skipped.
How to avoid:
In the first CI plan, write a minimal "hello world" workflow that only checks node --version and pnpm --version. Verify it passes before adding any test steps. Use actions/setup-node only if confirmed compatible with the specific Gitea version; otherwise install Node and pnpm explicitly in the workflow using wget / npm install -g pnpm. Pin the Node version to 22.x explicitly; do not rely on the runner default.
For Docker image build/publish: verify the runner has Docker daemon access. On Unraid self-hosted runners, Docker may require --privileged or specific socket mounts that need runner configuration.
Warning signs:
pnpm: command not foundin CI output.noderesolves to a version older than 22 in CI but not locally.actions/setup-nodestep shows as skipped or errored in the Gitea Actions UI.
Phase to address: Gitea CI phase. The runner environment probe must be the first CI task — before any test or build steps are designed.
Pitfall 13: Docker Registry Push Token Scope Exposes Secrets in Logs
What goes wrong:
The Gitea CI Docker build/publish step requires credentials for the Docker registry (Docker Hub, Gitea's own container registry, or a self-hosted registry). If the registry token is passed as a docker login argument on the command line (e.g., docker login -u $USER -p $TOKEN), the token appears in the process list, in the Gitea Actions job log (if command echo is on), and in any runner audit logs. Gitea Actions supports secrets: but if the workflow uses run: docker login -p ${{ secrets.REGISTRY_TOKEN }}, the secret is masked in the log only if the secret was registered correctly — unregistered secrets are echoed verbatim.
Why it happens:
Docker CLI login via -p flag is the most common example in docs. GitHub Actions masks secrets automatically; Gitea Actions masks them only for registered secrets. A token from a CI environment variable that was not added through the Gitea Secrets UI is not masked.
How to avoid:
Use docker login --password-stdin with the token piped via stdin rather than a command-line argument: echo "${{ secrets.REGISTRY_TOKEN }}" | docker login -u "${{ secrets.REGISTRY_USER }}" --password-stdin registry.example.com. Register all credentials as Gitea repository secrets, not as environment variables in the workflow YAML. Verify the Gitea version supports secret masking in the Actions log (Gitea ≥ 1.19 for Actions support; secret masking behavior varies by version).
Warning signs:
- Registry token or password visible as plaintext in the Gitea Actions job log.
docker logincommand line includes-p <token>in the log output.secrets.REGISTRY_TOKENis undefined in the workflow (token was set as env var, not secret).
Phase to address: Gitea CI phase. Credential handling review before any Docker push step is added.
Pitfall 14: Playwright Authed-Mobile Harness Reusing a Stale storage-state
What goes wrong:
The mobile-emulated Playwright harness uses a saved storage-state.json (cookies + localStorage) to bypass the OIDC login flow. If the storage state was captured with a real Authelia session, it contains a session cookie with a finite TTL (typically 1 hour for @hono/oidc-auth JWT cookies, or the Authelia session lifetime). After the TTL expires, all Playwright runs with the stale storage state silently fail at the first /api/* call — the OIDC middleware redirects to Authelia, and the test gets an HTML login page instead of the expected JSON API response. The test may still pass if it only checks DOM content (which may be the redirected page's HTML), or it may produce a false-positive assertion on the 302 redirect.
Why it happens:
Playwright storage state is file-based and not automatically refreshed. Developers capture it once and check it in (or store it locally), then forget to renew it. In DEV_AUTH_BYPASS=true mode this does not apply (no session cookie needed), but if the harness is meant to test the production auth path, the bypass is not active and the session cookie must be valid.
How to avoid:
Do not use a static stored storage state for tests that run against the production OIDC path. Instead, implement a programmatic login helper that runs the OIDC authorization code flow at the start of each test session (or once per test run) and stores the resulting session. For the DEV_AUTH_BYPASS dev environment, the harness sets DEV_AUTH_BYPASS=true and skips the storage state entirely. The mobile viewport emulation does not require real OIDC — use DEV_AUTH_BYPASS for the automated harness; keep real OIDC tests as manual/human gates.
Warning signs:
- Playwright runs fail with
Expected 200 OK but got 302 Foundafter 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=truemode but fails intermittently in production-auth mode.
Phase to address: Mobile-browser testing phase. The storage state strategy must be decided before the first test is written — programmatic refresh or bypass-only.
Pitfall 15: Production Service Worker Intercepting Playwright Requests
What goes wrong:
The installed Vite PWA service worker (sw.js) is registered in the browser when the PWA is visited. Playwright's Chromium instance can load and activate the service worker from a previous test run (persisted in the browser's profile directory). On subsequent test runs, the service worker intercepts API calls — potentially returning cached responses from the previous run rather than making network requests to the test server. This causes:
- API requests returning stale 200 responses when the test server is not running.
queryClient.invalidateQueriesnot 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.
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 |
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 |
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 |
"Looks Done But Isn't" Checklist
- VALARM authoring: Often ships create-only — verify that editing an event with a native-client alarm in rawVevent does not drop that alarm from the PUT payload.
- VALARM serialization: Often emits
VALUE=TEXT— verify the ICS output hasTRIGGER:-PT15M(DURATION type, no VALUE parameter) orTRIGGER;VALUE=DURATION:-PT15M— neverVALUE=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 setsdrainRequestedor 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_keyorapp_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).
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-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 |
Sources
- Direct inspection of
apps/api/src/broker/outboxWorker.ts,reminderScheduler.ts,vevent.ts,crypto.ts,index.ts,db/schema.ts .planning/RETROSPECTIVE.md— v1.0 lessons: node-cron skip, drizzle push destructive diff, VAPID truncation, tsc vs vitest divergenceCLAUDE.mdmemory 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: FamilySync v1.1 Operability & Polish Researched: 2026-06-10