docs(06): research phase for ux-polish
Code-verified findings for all six D-01..D-13 fix areas: end-tracking gap in EventForm, @keyframes pulse absent from tokens.css, hasRrule missing from CalendarOccurrence type, and ical.js UNTIL/COUNT verified against project node_modules. Includes validation architecture for TDD and playwright-cli verification scopes.
This commit is contained in:
Lucas Berger
@@ -0,0 +1,770 @@
|
|||||||
|
# Phase 6: UX Polish — Research
|
||||||
|
|
||||||
|
**Researched:** 2026-06-10
|
||||||
|
**Domain:** React PWA / Hono API — form UX, ical.js recurrence, OIDC auth gating, CSS animation
|
||||||
|
**Confidence:** HIGH on code-verified claims; MEDIUM on library API specifics (Context7)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
<user_constraints>
|
||||||
|
## User Constraints (from CONTEXT.md)
|
||||||
|
|
||||||
|
### Locked Decisions
|
||||||
|
|
||||||
|
- **D-01:** Phase 6 = the six polish items only (999.2/3/6/7/8/9). Do NOT pull in 999.4 or 999.5.
|
||||||
|
- **D-02:** 999.4 (reminders/VALARM) and 999.5 (provider setup) move to milestone 1.1. 999.1 also remains backlog/1.1 candidate.
|
||||||
|
- **D-03:** End-tracking is a general bug — applies to one-time, single-day, and timed events alike.
|
||||||
|
- **D-04:** Preserve current duration on any start change (timed: delta; all-day: day-span). Floor: end never strands behind start.
|
||||||
|
- **D-05:** All-day-edit off-by-one already fixed at `EventForm.tsx:199` (`exclusiveEndToInclusiveDate`). Verify it holds; do NOT re-implement.
|
||||||
|
- **D-06:** Add recurrence bound — "repeat until <date>" (RRULE `UNTIL`) and/or "for N occurrences" (`COUNT`). Each occurrence's duration ties to start→end delta, NOT the recurrence span.
|
||||||
|
- **D-07:** Verify/fix FREQ-persistence bug — confirm the dropdown correctly writes the selected FREQ.
|
||||||
|
- **D-08:** Whole-series edit only (title/time/RRULE on master VEVENT). Per-occurrence RECURRENCE-ID stays deferred to v1.x.
|
||||||
|
- **D-09:** Series-edit confirmation/prompt UX delegated to `/gsd-ui-phase`.
|
||||||
|
- **D-10:** Gate app render on auth state — no calendar shell/skeleton/"Sign-in required" flash before Authelia. Show "Signing you in…" splash while unauthenticated.
|
||||||
|
- **D-11:** Centralize 401/opaqueredirect detection from ANY query or mutation. Typed `SessionExpiredError`, single TanStack Query error handler, re-arm `maybeRedirectToLogin()` on session expiry mid-use.
|
||||||
|
- **D-12:** Visual refresh delegated to `/gsd-ui-phase` invoking the `frontend-design` skill.
|
||||||
|
- **D-13:** Hoist `@keyframes spin` to global stylesheet so all sync indicators animate.
|
||||||
|
|
||||||
|
### Claude's Discretion / Delegated to UI-Phase
|
||||||
|
|
||||||
|
- All-day visual treatment (999.6): behavior locked; concrete CSS treatment delegated to `/gsd-ui-phase`.
|
||||||
|
- Series-edit prompt UX (999.9): delegated to `/gsd-ui-phase` (see D-09).
|
||||||
|
- Splash/interstitial copy: "Signing you in…" / "Your session expired — signing you back in…" are starting points.
|
||||||
|
|
||||||
|
### Deferred Ideas (OUT OF SCOPE)
|
||||||
|
|
||||||
|
- 999.4 — Event reminder/VALARM options → milestone 1.1
|
||||||
|
- 999.5 — First-login provider setup → milestone 1.1
|
||||||
|
- 999.1 — Calendar provider abstraction → backlog/milestone 1.1 candidate
|
||||||
|
- Per-occurrence (RECURRENCE-ID) and "this and following" recurring edits → v1.x
|
||||||
|
</user_constraints>
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
This is a polish phase on code that was shipped in Phases 3 and 5. Every item has a diagnosed root cause from Phase 3 Gate 2 live use. The research task is to confirm the exact fix targets in the real code, resolve library API specifics that the planner cannot safely assume, and define the verification architecture.
|
||||||
|
|
||||||
|
Six independent fix areas, each self-contained but sharing the EventForm and client.ts touch points:
|
||||||
|
|
||||||
|
1. **D-04/D-07 — EventForm: end-tracking + FREQ-persistence.** Both are EventForm state bugs. End-tracking is a missing `onChange` handler on `startDate`/`startTime` that recomputes end. FREQ-persistence is a suspected state-initialization issue (see §FREQ-Persistence Diagnosis below — the `RRULE_PRESETS` map and the API route are clean; the bug origin is narrowed to form state reset behavior).
|
||||||
|
2. **D-06/D-08 — RRULE UNTIL/COUNT + whole-series edit.** Spans PWA → API → expansion. Requires a schema extension on `CalendarOccurrence` to expose `hasRrule` (currently absent from the type on both sides).
|
||||||
|
3. **D-10/D-11 — Auth gating.** A single `CalendarShell` refactor plus typed `SessionExpiredError` in `client.ts` serves both items.
|
||||||
|
4. **D-13 — Spinner.** The `@keyframes spin` IS already in `tokens.css` (lines 140–147). The bug is that `PushPermissionPrompt.tsx` contains a redundant local `<style>` block redefining it (`:358–363`). `SyncStateToast` and `LiveSyncIndicator` use `animation: 'spin 1s linear infinite'` as inline styles. These work correctly as long as `tokens.css` is loaded — which it is (main.tsx imports `./styles/index.css` which `@import`s `tokens.css`). Additionally, `@keyframes pulse` is MISSING from `tokens.css` — `LiveSyncIndicator` references `animation: 'pulse 1.4s ease-in-out infinite'` for the reconnecting dot.
|
||||||
|
|
||||||
|
**Primary recommendation:** Fix items in order of risk: D-06 (RRULE changes) first (most complex, cross-stack), then D-08 (requires hasRrule schema extension), then D-04 (pure form logic), then D-10/D-11 (auth gating refactor), then D-13 (CSS hoist, lowest risk). D-07 is a verification step folded into D-06 work.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Architectural Responsibility Map
|
||||||
|
|
||||||
|
| Capability | Primary Tier | Secondary Tier | Rationale |
|
||||||
|
|------------|-------------|----------------|-----------|
|
||||||
|
| RRULE UNTIL/COUNT serialization | API / Backend (`vevent.ts`) | PWA (`client.ts` payload shape) | ical.js RRULE building lives in `buildVeventString`; PWA only passes a string |
|
||||||
|
| Recurrence bound control UI | Browser / Client (EventForm.tsx) | — | Pure form field; no server state |
|
||||||
|
| End-tracking math | Browser / Client (EventForm.tsx) | — | Duration arithmetic on local form state |
|
||||||
|
| Whole-series edit (master PUT) | API / Backend (outboxWorker + write.ts) | PWA (series-edit prompt gating) | PUT to Fastmail CalDAV is already implemented; PWA needs the detection signal |
|
||||||
|
| hasRrule exposure | API / Backend (expand.ts + events route) | Browser / Client (client.ts type) | DB has `has_rrule`; must flow through CalendarOccurrence type |
|
||||||
|
| Auth splash / session-expiry interstitial | Browser / Client (CalendarShell + client.ts) | — | OIDC guard is server-side; client handles redirect |
|
||||||
|
| Spin animation | Browser / Client (tokens.css + components) | — | CSS keyframe availability is a stylesheet concern |
|
||||||
|
| Pulse animation | Browser / Client (tokens.css) | — | Missing keyframe; needs addition |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Standard Stack
|
||||||
|
|
||||||
|
No new packages are installed in this phase. All fixes use the already-pinned stack from `CLAUDE.md`:
|
||||||
|
|
||||||
|
| Library | Version (pinned) | Role in this phase |
|
||||||
|
|---------|------------------|--------------------|
|
||||||
|
| ical.js | 2.2.1 | RRULE UNTIL/COUNT serialization in `vevent.ts` |
|
||||||
|
| React 19 | 19.x | EventForm state updates |
|
||||||
|
| TanStack Query | 5.101.0 | Global error handler for session expiry |
|
||||||
|
| Zustand | 5.0.14 | `sessionExpired` flag for session-expiry interstitial |
|
||||||
|
| Vitest | (existing) | All unit/integration tests |
|
||||||
|
| playwright-cli | `/usr/local/bin/playwright-cli` | Browser-level visual verification |
|
||||||
|
|
||||||
|
**No package installations required for this phase.**
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Package Legitimacy Audit
|
||||||
|
|
||||||
|
No new packages are installed in Phase 6. This section is not applicable.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Architecture Patterns
|
||||||
|
|
||||||
|
### Recommended Project Structure
|
||||||
|
|
||||||
|
No new files/directories needed beyond what already exists. New test files follow the established `tests/broker/`, `tests/routes/`, `src/lib/*.test.ts`, `src/components/*.test.tsx` patterns.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Focus Area Findings
|
||||||
|
|
||||||
|
### Focus 1: Recurrence Bounding — D-06 (RRULE UNTIL/COUNT)
|
||||||
|
|
||||||
|
#### PWA side — `RecurrencePreset` extension (`client.ts:130`, `EventForm.tsx`)
|
||||||
|
|
||||||
|
**Current shape** [VERIFIED: codebase read]:
|
||||||
|
```typescript
|
||||||
|
// client.ts:130
|
||||||
|
export type RecurrencePreset = 'none' | 'daily' | 'weekly' | 'monthly' | 'yearly'
|
||||||
|
|
||||||
|
// CreateEventPayload:
|
||||||
|
recurrence?: RecurrencePreset
|
||||||
|
```
|
||||||
|
|
||||||
|
**Required extension:** `RecurrencePreset` covers frequency only. `CreateEventPayload` needs two new optional fields:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Add to CreateEventPayload
|
||||||
|
recurrenceUntil?: string // 'YYYY-MM-DD' — maps to RRULE UNTIL; undefined = no bound
|
||||||
|
recurrenceCount?: number // integer ≥ 1 — maps to RRULE COUNT; undefined = no bound
|
||||||
|
```
|
||||||
|
|
||||||
|
Rules:
|
||||||
|
- `recurrenceUntil` and `recurrenceCount` are mutually exclusive (RFC 5545 §3.3.10).
|
||||||
|
- Only sent when `recurrence !== 'none'`.
|
||||||
|
- On EDIT, same omit-if-absent rule as `recurrence` (WR-01 pattern already in place).
|
||||||
|
|
||||||
|
**EventForm UI placement** (per UI-SPEC.md §Surface 5): below the frequency `<select>`, shown only when `recurrence !== 'none'`. Three-option bound-type selector: "Never" / "On date" / "After N times". Controlled by a new `recurrenceBound: 'never' | 'until' | 'count'` state variable + `recurrenceUntil: string` + `recurrenceCount: number` state.
|
||||||
|
|
||||||
|
#### API route validation (`events.ts:100–109`)
|
||||||
|
|
||||||
|
The Zod schema `eventFieldsSchema` and `outboxPayloadSchema` in `outboxWorker.ts:71–83` both need the two new fields:
|
||||||
|
```typescript
|
||||||
|
recurrenceUntil: z.string().max(10).optional(), // 'YYYY-MM-DD'
|
||||||
|
recurrenceCount: z.number().int().min(1).optional(),
|
||||||
|
```
|
||||||
|
|
||||||
|
#### API write path — ical.js RRULE serialization (`vevent.ts:49–54`, `outboxWorker.ts`)
|
||||||
|
|
||||||
|
**Current state** [VERIFIED: codebase read]: `RRULE_PRESETS` maps preset name to bare `FREQ=X` string. `buildVeventString` uses `ICAL.Recur.fromString(params.rruleString)` then `rruleProp.setValue(recur)`.
|
||||||
|
|
||||||
|
**UNTIL/COUNT syntax — verified against ical.js 2.2.1 in project node_modules** [VERIFIED: live node evaluation]:
|
||||||
|
|
||||||
|
```
|
||||||
|
ICAL.Recur.fromString('FREQ=WEEKLY;COUNT=5').toString()
|
||||||
|
→ 'FREQ=WEEKLY;COUNT=5' ✓
|
||||||
|
|
||||||
|
ICAL.Recur.fromString('FREQ=DAILY;UNTIL=20260630').toString()
|
||||||
|
→ 'FREQ=DAILY;UNTIL=20260630' (DATE form, no time, no Z) ✓
|
||||||
|
|
||||||
|
ICAL.Recur.fromString('FREQ=DAILY;UNTIL=20260630T235959Z').toString()
|
||||||
|
→ 'FREQ=DAILY;UNTIL=20260630T235959Z' (DATETIME UTC form) ✓
|
||||||
|
```
|
||||||
|
|
||||||
|
**RFC 5545 §3.3.10 UNTIL value-type rule** [ASSUMED — RFC knowledge, not verified against spec text this session]:
|
||||||
|
- If DTSTART is VALUE=DATE (all-day), UNTIL MUST be a DATE (`YYYYMMDD`), not a DATETIME.
|
||||||
|
- If DTSTART is DATETIME, UNTIL MUST be a UTC DATETIME (`YYYYMMDDTHHMMSSZ`).
|
||||||
|
|
||||||
|
**Safe approach for this codebase:** Use `UNTIL=YYYYMMDD` for all-day events; use `UNTIL=YYYYMMDDTHHMMSSZ` (end of day UTC, i.e. `T235959Z`) for timed events. This matches the existing `isDate: true` / UTC pattern in `vevent.ts`.
|
||||||
|
|
||||||
|
**RRULE string assembly:** Extend `RRULE_PRESETS` usage or build the string inline in the outbox worker:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// In outboxWorker.ts, when building rruleString for buildVeventString:
|
||||||
|
function assembleRruleString(
|
||||||
|
preset: string, // 'FREQ=WEEKLY' etc. from RRULE_PRESETS
|
||||||
|
until?: string, // 'YYYY-MM-DD'
|
||||||
|
count?: number,
|
||||||
|
allDay?: boolean,
|
||||||
|
): string {
|
||||||
|
let s = preset
|
||||||
|
if (count !== undefined) {
|
||||||
|
s += `;COUNT=${count}`
|
||||||
|
} else if (until) {
|
||||||
|
// RFC 5545: DATE form for all-day, DATETIME UTC for timed
|
||||||
|
if (allDay) {
|
||||||
|
s += `;UNTIL=${until.replace(/-/g, '')}` // 20260630
|
||||||
|
} else {
|
||||||
|
s += `;UNTIL=${until.replace(/-/g, '')}T235959Z` // 20260630T235959Z
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return s
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
`ICAL.Recur.fromString(rruleString)` + the existing `rruleProp.setValue(recur)` pattern then serializes it correctly.
|
||||||
|
|
||||||
|
#### Expansion path — `expand.ts` per-occurrence duration
|
||||||
|
|
||||||
|
**Verified** [VERIFIED: live node evaluation]: `expand.ts` computes each occurrence's end via `event.duration` (the DTSTART→DTEND delta parsed by ical.js), NOT from the RRULE UNTIL. This means adding `UNTIL` or `COUNT` to the RRULE does NOT affect per-occurrence duration — the motivating bug (2-month bars) was caused by the start→end span being 63 days, which became the duration for each occurrence. The fix is in D-04 (end-tracking) not in the expansion code. No changes needed in `expand.ts` for D-06.
|
||||||
|
|
||||||
|
**Expansion terminates correctly at UNTIL/COUNT** [VERIFIED: live node evaluation]:
|
||||||
|
```
|
||||||
|
COUNT=3 with weekly FREQ → RecurExpansion.next() returns 3 occurrences then marks expand.complete=true
|
||||||
|
```
|
||||||
|
|
||||||
|
`RecurExpansion` in `expand.ts` uses `expand.next()` in a `while` loop against `rangeEnd` — it will correctly stop when COUNT is exhausted OR when UNTIL is reached, whichever comes first within the window.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Focus 2: FREQ-Persistence Bug — D-07
|
||||||
|
|
||||||
|
**Claim from CONTEXT.md:** A "daily" selection reportedly persisted as "weekly."
|
||||||
|
|
||||||
|
**Code trace** [VERIFIED: codebase read]:
|
||||||
|
|
||||||
|
1. **`RRULE_PRESETS` map** (`vevent.ts:49–54`): `{daily: 'FREQ=DAILY', weekly: 'FREQ=WEEKLY', monthly: 'FREQ=MONTHLY', yearly: 'FREQ=YEARLY'}` — mapping is correct. No bug here.
|
||||||
|
|
||||||
|
2. **API route** (`events.ts:107`): `recurrence: z.enum(['none', 'daily', 'weekly', ...])` — passes through unchanged to outbox JSON. No bug here.
|
||||||
|
|
||||||
|
3. **Outbox worker** (`outboxWorker.ts:258–259`): `fields.recurrence && fields.recurrence !== 'none' ? RRULE_PRESETS[fields.recurrence]` — lookup is exact, no bug.
|
||||||
|
|
||||||
|
4. **EventForm state initialization** (`EventForm.tsx:207–258`):
|
||||||
|
- `useState<RecurrencePreset>('none')` — initialized correctly.
|
||||||
|
- The reset `useEffect` (lines 232–262) runs on `[eventFormOpen, eventFormMode, eventFormUid, occurrence?.uid]`. It calls `setRecurrence(derivedRecurrence ?? 'none')`. `derivedRecurrence` is `(occurrence as any)?.recurrence` — always `undefined` (not in `CalendarOccurrence` type).
|
||||||
|
- **Potential bug site:** If the form is opened while a stale `occurrence` is still in the TanStack cache, the effect fires with `occurrence` present but `occurrence.recurrence === undefined`, setting `recurrence` to `'none'`. Then when the user selects "daily", and if the effect fires again before submit (because `occurrence?.uid` changes when cache updates), it resets to `'none'`. But the bug report says "daily persisted as weekly," not "persisted as none."
|
||||||
|
|
||||||
|
5. **Most likely explanation** [ASSUMED — not confirmed by reproducing the bug]: The original bug was a one-time user error or a now-stale state from a prior session where the `RecurrencePreset` type or the select option values were misaligned. With the current code, a deliberate "daily" selection will correctly send `recurrence: 'daily'` unless the effect fires between selection and submit. The effect deps include `occurrence?.uid` — if the TanStack cache updates with a new `occurrence` between selection and submit (plausible if the query refetches), the effect fires and resets to `'none'`, but NOT to `'weekly'`.
|
||||||
|
|
||||||
|
**Verdict:** No obvious code path produces "daily → weekly." **D-07 verification task:** Write a regression test that creates a daily event, verifies the outbox row carries `recurrence: 'daily'`, and the resulting RRULE is `FREQ=DAILY`. If the bug is intermittent, add a snapshot of the form-submit path to confirm `recurrence` state is not reset between select-change and submit. The planner should treat this as a "confirm + add regression test" task rather than a targeted code fix. [ASSUMED — no reproduction possible without running the app]
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Focus 3: Whole-Series Edit — D-08/D-09
|
||||||
|
|
||||||
|
#### Identifying a recurring occurrence (`hasRrule`)
|
||||||
|
|
||||||
|
**Current state** [VERIFIED: codebase read]:
|
||||||
|
- `calendarEvents.hasRrule` exists in DB schema (`apps/api/src/db/schema.ts:131`) and is set correctly in `sync.ts:152,163`.
|
||||||
|
- `CalendarOccurrence` interface in `expand.ts` does NOT include `hasRrule` — it is absent from the type.
|
||||||
|
- `CalendarOccurrence` interface in `client.ts` does NOT include `hasRrule`.
|
||||||
|
- Therefore, the PWA cannot currently detect "this occurrence belongs to a recurring series."
|
||||||
|
|
||||||
|
**Required change:** Add `hasRrule: boolean` to `CalendarOccurrence` in `expand.ts` and `client.ts`, and populate it in `expandOccurrences()` from the master event:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// In expandOccurrences():
|
||||||
|
const isRecurring = event.isRecurring()
|
||||||
|
// ... in each occurrence push:
|
||||||
|
occurrences.push({ ..., hasRrule: isRecurring })
|
||||||
|
```
|
||||||
|
|
||||||
|
The events route SQL join already fetches enough data; no DB query change needed since `expandOccurrences` already knows `event.isRecurring()`.
|
||||||
|
|
||||||
|
#### Write-back path for master VEVENT edit
|
||||||
|
|
||||||
|
**Current state** [VERIFIED: codebase read]: The edit route in `events.ts` (lines 326–431) already:
|
||||||
|
1. Looks up `calendarEvents` by `uid` + caller's userId + `calendarUrl` — identifies the master VEVENT row.
|
||||||
|
2. Reads `objectUrl` (the CalDAV object URL for PUT) and the freshest `etag`.
|
||||||
|
3. Passes through to `outboxWorker` which calls `buildVeventString` + `updateCalendarEvent` PUT.
|
||||||
|
|
||||||
|
For whole-series edit (D-08), the PWA sends the same PATCH `/api/events/:uid/edit` it already uses. The `uid` used in `eventFormUid` is the occurrence `uid` from `CalendarOccurrence.uid` — which is also the master VEVENT's UID. The edit route fetches by this UID and PUTs the full new VCALENDAR/VEVENT back to the same `objectUrl`.
|
||||||
|
|
||||||
|
**RRULE on whole-series edit:** When the user edits a recurring event and the form is in edit mode, `recurrence` is currently omitted from the payload (WR-01). For D-08, the payload must carry the new `recurrenceUntil`/`recurrenceCount` fields when the user modifies the bound, alongside the existing RRULE preservation (or a new explicit `recurrence` if they want to change frequency). The `hasExplicitRecurrence` check in the worker already handles this correctly — if the user explicitly sets `recurrence`, the preset overrides the stored RRULE. If only `recurrenceUntil`/`recurrenceCount` change, the worker must be updated to apply those modifiers to the preserved RRULE.
|
||||||
|
|
||||||
|
**What must NOT change:** No `RECURRENCE-ID` added. The PUT replaces the master VEVENT wholesale — same UID, same `objectUrl`, fresh iCal string with updated DTSTART/DTEND/SUMMARY/RRULE. All existing occurrences in the series update on the next CalDAV re-sync.
|
||||||
|
|
||||||
|
#### All-day DTEND symmetry on series edit (D-05 / WR-04)
|
||||||
|
|
||||||
|
**Already verified** [VERIFIED: codebase read]: `EventForm.tsx:199` has the `exclusiveEndToInclusiveDate` pre-fill. `vevent.ts` WR-04 has the +1 day roll-forward on write. Any series edit that changes dates must go through the same `EventForm` submit → `serializeEventDateTime` → `buildVeventString` path, so the invariant is preserved automatically. No additional work required for D-08 on this front.
|
||||||
|
|
||||||
|
#### Confirmation prompt (D-09)
|
||||||
|
|
||||||
|
UI-SPEC.md §Surface 6 locks the pattern: bottom-sheet on phone, dialog on desktop. Copy is locked. Implementation uses the existing `DeleteConfirmationDialog` pattern. The series-edit prompt renders when `eventFormMode === 'edit'` AND `occurrence.hasRrule === true` AND the user taps Save. This requires `hasRrule` on `CalendarOccurrence` (see above).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Focus 4: End-Tracking — D-03/D-04
|
||||||
|
|
||||||
|
#### Start/end state structure
|
||||||
|
|
||||||
|
**Verified** [VERIFIED: codebase read]:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// EventForm.tsx state (lines 201–206)
|
||||||
|
const [startDate, setStartDate] = useState(initStart.date) // 'YYYY-MM-DD'
|
||||||
|
const [startTime, setStartTime] = useState(initStart.time) // 'HH:MM'
|
||||||
|
const [endDate, setEndDate] = useState(initEndDate) // 'YYYY-MM-DD' inclusive
|
||||||
|
const [endTime, setEndTime] = useState(initEnd.time) // 'HH:MM'
|
||||||
|
```
|
||||||
|
|
||||||
|
The start date and time inputs currently have individual `onChange` handlers (`setStartDate(e.target.value)` / `setStartTime(e.target.value)` at lines 672 and 683 respectively). There is NO companion call to update `endDate`/`endTime` when start changes. This is the exact bug.
|
||||||
|
|
||||||
|
#### Fix target
|
||||||
|
|
||||||
|
Replace the bare `onChange` handlers on the start date input (line 672) and start time input (line 683) with handlers that also recompute end:
|
||||||
|
|
||||||
|
**Timed event — preserve delta:**
|
||||||
|
```typescript
|
||||||
|
function onStartDateChange(newStartDate: string) {
|
||||||
|
setStartDate(newStartDate)
|
||||||
|
if (allDay) return // handled in allDay branch
|
||||||
|
const oldStartMs = new Date(`${startDate}T${startTime}:00`).getTime()
|
||||||
|
const oldEndMs = new Date(`${endDate}T${endTime}:00`).getTime()
|
||||||
|
const deltaMs = oldEndMs > oldStartMs ? oldEndMs - oldStartMs : 60 * 60 * 1000 // 1h floor
|
||||||
|
const newStartMs = new Date(`${newStartDate}T${startTime}:00`).getTime()
|
||||||
|
const newEndDate = new Date(newStartMs + deltaMs)
|
||||||
|
setEndDate(dateToISO(newEndDate)) // 'YYYY-MM-DD' via local accessors
|
||||||
|
setEndTime(timeToHHMM(newEndDate)) // 'HH:MM' via local accessors
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**All-day — preserve day span:**
|
||||||
|
```typescript
|
||||||
|
function onStartDateChange(newStartDate: string) {
|
||||||
|
setStartDate(newStartDate)
|
||||||
|
const oldSpanDays = Math.max(0, dateDiffDays(startDate, endDate))
|
||||||
|
const newEnd = addDays(newStartDate, oldSpanDays) // pure date arithmetic
|
||||||
|
setEndDate(newEnd)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Floor rule (D-04):** If `oldEndMs <= oldStartMs` (stale state already invalid), snap new end to `newStart + 1h` (timed) or `newStartDate` (all-day same-day).
|
||||||
|
|
||||||
|
These are pure functions with defined I/O — **TDD-eligible.** Extract to `apps/pwa/src/lib/eventDateTime.ts` (already exists, has tests) for unit testing. Tests: verify delta preservation, verify floor rule snaps correctly, verify all-day day-span.
|
||||||
|
|
||||||
|
#### D-05 verification (already fixed)
|
||||||
|
|
||||||
|
**Verified** [VERIFIED: codebase read]: `EventForm.tsx:199` (`initEndDate` computation) and `EventForm.tsx:239–242` (useEffect reset path) both apply `exclusiveEndToInclusiveDate` when `occurrence?.allDay && initEnd.date`. The `exclusiveEndToInclusiveDate` helper at lines 86–96 performs UTC-component-based roll-back (DST-safe). **No further work on D-05.**
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Focus 5: Spinner Animation — D-13
|
||||||
|
|
||||||
|
#### Actual state of `@keyframes spin` [VERIFIED: codebase read]
|
||||||
|
|
||||||
|
`apps/pwa/src/styles/tokens.css` lines 140–147:
|
||||||
|
```css
|
||||||
|
@keyframes spin {
|
||||||
|
from { transform: rotate(0deg); }
|
||||||
|
to { transform: rotate(360deg); }
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
`apps/pwa/src/styles/index.css` line 9: `@import './tokens.css'`
|
||||||
|
|
||||||
|
`apps/pwa/src/main.tsx` line 9: `import './styles/index.css'`
|
||||||
|
|
||||||
|
The `@keyframes spin` global definition IS present and IS loaded before any component mounts. The `animation: 'spin 1s linear infinite'` inline style references are correct.
|
||||||
|
|
||||||
|
#### Actual bugs
|
||||||
|
|
||||||
|
**Bug 1 — Redundant local redefine** [VERIFIED: codebase read]: `PushPermissionPrompt.tsx:358–363` contains:
|
||||||
|
```tsx
|
||||||
|
<style>{`
|
||||||
|
@keyframes spin {
|
||||||
|
from { transform: rotate(0deg); }
|
||||||
|
to { transform: rotate(360deg); }
|
||||||
|
}
|
||||||
|
`}</style>
|
||||||
|
```
|
||||||
|
This is redundant (the global definition already covers it) and slightly noisy but not the cause of animation failures. Remove it.
|
||||||
|
|
||||||
|
**Bug 2 — Missing `@keyframes pulse`** [VERIFIED: codebase read]: `LiveSyncIndicator.tsx:69` uses:
|
||||||
|
```tsx
|
||||||
|
animation: 'pulse 1.4s ease-in-out infinite'
|
||||||
|
```
|
||||||
|
`@keyframes pulse` does NOT exist in `tokens.css` or `index.css`. The reconnecting dot never animates. Must add to `tokens.css` per UI-SPEC.md:
|
||||||
|
```css
|
||||||
|
@keyframes pulse {
|
||||||
|
0%, 100% { opacity: 1; }
|
||||||
|
50% { opacity: 0.4; }
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Bug 3 — Spinner in `SyncStateToast`** [VERIFIED: codebase read]: `SyncStateToast.tsx:156–161` uses `animation: 'spin 1s linear infinite'`. Since `tokens.css` loads before components, this works. BUT confirm that the `SyncStateToast` Loader2 spinner IS actually spinning — the potential failure mode from the CONTEXT.md claim does not apply here because `tokens.css` is always loaded. The spinner should work. The `@keyframes spin` in `PushPermissionPrompt` is a `<style>` block, not a CSS module, so it IS global when the component renders — but it's irrelevant since tokens.css already has it.
|
||||||
|
|
||||||
|
**Summary:** The claim in D-13 that "SyncStateToast/LiveSyncIndicator don't animate when PushPermissionPrompt isn't mounted" is only partially accurate. `@keyframes spin` works fine (it's global via tokens.css). `@keyframes pulse` is the real missing animation. The fix is: (1) add `@keyframes pulse` to `tokens.css`, (2) remove the redundant `<style>` block from `PushPermissionPrompt.tsx`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Focus 6: Auth Gating — D-10/D-11
|
||||||
|
|
||||||
|
#### CalendarShell cold-load flash — D-10 [VERIFIED: codebase read]
|
||||||
|
|
||||||
|
**Current render path:**
|
||||||
|
1. `CalendarShell` renders immediately regardless of `meQuery` state.
|
||||||
|
2. `meQuery` starts loading (status: `isLoading`).
|
||||||
|
3. `isInitialLoading = meQuery.isLoading || (eventsQuery.isLoading && !eventsQuery.data)` → `SkeletonCalendar` renders.
|
||||||
|
4. On `meQuery.isError`: the component returns the `<div role="alert">Sign-in required</div>` fragment (line 220–235), then the `useEffect` at line 197 calls `maybeRedirectToLogin()`.
|
||||||
|
|
||||||
|
**The flash:** Between initial render and the `meQuery.isError` settlement, the user sees:
|
||||||
|
- On fast networks: SkeletonCalendar briefly (acceptable).
|
||||||
|
- On first cold load with no session: SkeletonCalendar → "Sign-in required" alert → browser navigates to `/api/login`. The "Sign-in required" alert is the flash (it renders before `maybeRedirectToLogin()` fires, since the redirect is triggered by a `useEffect` which runs after paint).
|
||||||
|
|
||||||
|
**D-10 fix:** Replace the current `meQuery.isError` branch with a dedicated auth splash component that renders INSTEAD of "Sign-in required" and also covers `meQuery.isLoading`:
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
// In CalendarShell, before the main render tree:
|
||||||
|
if (meQuery.isLoading) {
|
||||||
|
return <AuthSplash state="loading" />
|
||||||
|
}
|
||||||
|
if (meQuery.isError) {
|
||||||
|
// useEffect handles maybeRedirectToLogin() — splash shows while redirect fires
|
||||||
|
return <AuthSplash state="redirecting" />
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
The `useEffect` for `maybeRedirectToLogin()` (already at line 197) fires after the `AuthSplash` renders, so the user sees "Signing you in…" with spinner instead of the "Sign-in required" alert.
|
||||||
|
|
||||||
|
#### Session expiry mid-use — D-11 [VERIFIED: codebase read]
|
||||||
|
|
||||||
|
**Current state:**
|
||||||
|
- `fetchMe` in `client.ts` (lines 36–53): uses `redirect: 'manual'`, detects `opaqueredirect`/401, throws `new Error('GET /api/me: authentication required')`.
|
||||||
|
- ALL other fetch calls (`fetchEvents`, `createEvent`, `updateEvent`, `deleteEvent`, etc.) do NOT use `redirect: 'manual'` and do NOT detect `opaqueredirect`. They only `throw new Error(...)` on non-ok HTTP status, but a 302 to Authelia would be followed with CORS block producing a network error (or hang), not a typed auth error.
|
||||||
|
- `maybeRedirectToLogin()` is only called from `CalendarShell`'s `meQuery.isError` effect.
|
||||||
|
|
||||||
|
**D-11 fix — two parts:**
|
||||||
|
|
||||||
|
**Part 1 — typed error + consistent `redirect:'manual'` in client.ts:**
|
||||||
|
```typescript
|
||||||
|
export class SessionExpiredError extends Error {
|
||||||
|
constructor() { super('Session expired — re-authentication required') }
|
||||||
|
}
|
||||||
|
|
||||||
|
// Helper used in all fetch calls:
|
||||||
|
function handleAuthResponse(res: Response): void {
|
||||||
|
if (res.type === 'opaqueredirect' || res.status === 401) {
|
||||||
|
throw new SessionExpiredError()
|
||||||
|
}
|
||||||
|
if (!res.ok) throw new Error(`HTTP ${res.status}`)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
All fetch functions (`fetchEvents`, `createEvent`, `updateEvent`, `deleteEvent`, `fetchSyncStatus`, `fetchWritableCalendars`) gain `redirect: 'manual'` + `handleAuthResponse(res)`.
|
||||||
|
|
||||||
|
**Part 2 — global TanStack Query error handler:**
|
||||||
|
```typescript
|
||||||
|
// In App.tsx (or where QueryClient is created):
|
||||||
|
const queryClient = new QueryClient({
|
||||||
|
defaultOptions: {
|
||||||
|
queries: {
|
||||||
|
onError: (error) => {
|
||||||
|
if (error instanceof SessionExpiredError) {
|
||||||
|
setSessionExpiredFlag() // Zustand flag
|
||||||
|
}
|
||||||
|
},
|
||||||
|
},
|
||||||
|
mutations: {
|
||||||
|
onError: (error) => {
|
||||||
|
if (error instanceof SessionExpiredError) {
|
||||||
|
setSessionExpiredFlag()
|
||||||
|
}
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
```
|
||||||
|
|
||||||
|
A Zustand `sessionExpired: boolean` flag triggers the session-expiry interstitial above the app tree. The interstitial calls `maybeRedirectToLogin()` (clears the one-shot guard first via `clearLoginRedirect()`).
|
||||||
|
|
||||||
|
**Note on TanStack Query v5 `onError` pattern** [ASSUMED — verify against TanStack Query 5 docs]: TanStack Query 5 moved from `onError` to `throwOnError` for some patterns. The global error callback in v5 is `queryClient.getQueryCache().subscribe()` or `mutationCache.subscribe()` rather than `defaultOptions.onError`. The planner must confirm the correct v5 API before implementing. The semantic intent above is correct regardless of the exact API.
|
||||||
|
|
||||||
|
**`loginRedirect.ts` re-arming:** Before calling `maybeRedirectToLogin()` from the session-expiry interstitial, call `clearLoginRedirect()` to clear the one-shot guard so the redirect fires correctly. This is already done in the `meQuery.isSuccess` effect (line 205), but for the D-11 mid-use case, `clearLoginRedirect()` must be called explicitly in the interstitial before the redirect.
|
||||||
|
|
||||||
|
**Single refactor serves both D-10 and D-11:** The `AuthSplash` component and the Zustand `sessionExpired` flag are two rendering paths from the same infrastructure. D-10 uses `meQuery.isLoading/isError`; D-11 uses the `sessionExpired` Zustand flag. Both show a centered full-screen interstitial. Plan them together.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Don't Hand-Roll
|
||||||
|
|
||||||
|
| Problem | Don't Build | Use Instead | Why |
|
||||||
|
|---------|-------------|-------------|-----|
|
||||||
|
| RRULE UNTIL/COUNT serialization | Custom string concatenation | `ICAL.Recur.fromString(rruleString)` + existing `rruleProp.setValue(recur)` pattern | Already in use in vevent.ts; handles escaping, value type |
|
||||||
|
| Date arithmetic for end-tracking | Custom date math | Pure JS `Date` arithmetic via local accessors (already the pattern in eventDateTime.ts) | The project already has `serializeEventDateTime` as the pattern; extend it |
|
||||||
|
| Global error handling for auth | Per-query try/catch | TanStack Query cache subscription | Centralized, doesn't require touching every query |
|
||||||
|
| CSS keyframe animation | Per-component `<style>` blocks | `tokens.css` global keyframes | Already the pattern; PushPermissionPrompt redundancy should be removed |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Common Pitfalls
|
||||||
|
|
||||||
|
### Pitfall 1: UNTIL value-type mismatch with DTSTART
|
||||||
|
|
||||||
|
**What goes wrong:** If DTSTART is VALUE=DATE (all-day) and UNTIL is a DATETIME (`YYYYMMDDTHHMMSSZ`), RFC 5545 §3.3.10 requires value-type consistency. Some CalDAV servers (including strict implementations) reject or misinterpret the event.
|
||||||
|
**How to avoid:** Use `assembleRruleString(preset, until, count, allDay)` that produces DATE-form UNTIL for all-day events and DATETIME UTC form for timed events.
|
||||||
|
**Warning signs:** Fastmail silently accepts the event but recurrence stops at a wrong date.
|
||||||
|
|
||||||
|
### Pitfall 2: UNTIL in user's local timezone vs UTC
|
||||||
|
|
||||||
|
**What goes wrong:** A user picks "ends June 30" but the UNTIL is serialized as `20260630T235959Z`. Depending on the user's timezone offset, `T235959Z` may be June 30 00:59 local time (UTC+1) rather than end-of-day local — causing the last occurrence to be dropped.
|
||||||
|
**How to avoid:** For the `UNTIL` date case, using `T235959Z` (end of UTC day) is a safe universal choice that avoids under-counting for most western timezones. Document this trade-off; do NOT try to compute `T{endOfDayLocal}Z` — that requires knowing the user's timezone from the browser, which adds complexity beyond this phase's scope. [ASSUMED — not verified against Fastmail behavior]
|
||||||
|
|
||||||
|
### Pitfall 3: Adding UNTIL/COUNT to the preserved RRULE on series edit
|
||||||
|
|
||||||
|
**What goes wrong:** When editing a series that has an existing rich RRULE (`FREQ=WEEKLY;BYDAY=MO,WE`), the current `extractRruleString` + `buildVeventString` path preserves the full RRULE string. If D-06 adds UNTIL/COUNT, the worker must combine `preservedRrule` + the new bound modifiers rather than replacing the RRULE outright.
|
||||||
|
**How to avoid:** In the outbox worker, if `recurrenceUntil` or `recurrenceCount` is present in the payload, parse `preservedRrule` into an `ICAL.Recur`, set `.until`/`.count`, and call `.toString()` to regenerate. Do NOT blindly concatenate.
|
||||||
|
|
||||||
|
### Pitfall 4: `hasRrule` flow-through schema break
|
||||||
|
|
||||||
|
**What goes wrong:** Adding `hasRrule` to `CalendarOccurrence` in `expand.ts` but forgetting to update `client.ts` (or vice versa) causes TypeScript errors in the PWA that look like API shape mismatches.
|
||||||
|
**How to avoid:** Update both interfaces atomically in the same commit. The API tests for `expandOccurrences` (`tests/broker/expand.test.ts`) should be updated to assert `hasRrule` on the returned objects.
|
||||||
|
|
||||||
|
### Pitfall 5: TanStack Query v5 global error handler API
|
||||||
|
|
||||||
|
**What goes wrong:** TanStack Query v5 removed `defaultOptions.onError`. Using the v4 API silently does nothing.
|
||||||
|
**How to avoid:** Use `queryClient.getQueryCache().subscribe(event => { if (event.type === 'error') ... })` and same for `getMutationCache()`. [ASSUMED — verify against Context7 TanStack Query 5 docs before implementing]
|
||||||
|
|
||||||
|
### Pitfall 6: React `useState` initializer runs once
|
||||||
|
|
||||||
|
**What goes wrong:** `EventForm` initializes `recurrence` state from `occurrence?.recurrence` which is always `undefined` (not in type). The reset effect at lines 232–262 also reads `(occurrence as any)?.recurrence` and falls back to `'none'`. When D-08 ships and recurrence IS exposed on `CalendarOccurrence`, the reset effect will need to handle the new `hasRrule` and `recurrence` fields correctly. This is a future concern, not a Phase 6 blocker.
|
||||||
|
**How to avoid:** When extending the occurrence contract to expose recurrence, update the reset effect simultaneously.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Code Examples
|
||||||
|
|
||||||
|
### RRULE UNTIL/COUNT — verified ical.js 2.2.1 patterns
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Source: verified against ical.js 2.2.1 in project node_modules
|
||||||
|
|
||||||
|
// COUNT — existing pattern works directly:
|
||||||
|
const recur = ICAL.Recur.fromString('FREQ=WEEKLY;COUNT=10')
|
||||||
|
const rruleProp = new ICAL.Property('rrule')
|
||||||
|
rruleProp.setValue(recur)
|
||||||
|
vevent.addProperty(rruleProp)
|
||||||
|
// produces: RRULE:FREQ=WEEKLY;COUNT=10
|
||||||
|
|
||||||
|
// UNTIL (all-day event, DATE form):
|
||||||
|
const recur = ICAL.Recur.fromString('FREQ=WEEKLY;UNTIL=20260630')
|
||||||
|
// produces: RRULE:FREQ=WEEKLY;UNTIL=20260630
|
||||||
|
|
||||||
|
// UNTIL (timed event, DATETIME UTC form):
|
||||||
|
const recur = ICAL.Recur.fromString('FREQ=WEEKLY;UNTIL=20260630T235959Z')
|
||||||
|
// produces: RRULE:FREQ=WEEKLY;UNTIL=20260630T235959Z
|
||||||
|
```
|
||||||
|
|
||||||
|
### End-tracking pure functions (TDD target)
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Target: apps/pwa/src/lib/eventDateTime.ts (extend existing file)
|
||||||
|
|
||||||
|
/** Preserve timed-event duration on start change. Returns new { endDate, endTime }. */
|
||||||
|
export function computeNewTimedEnd(
|
||||||
|
newStartDate: string,
|
||||||
|
newStartTime: string,
|
||||||
|
oldStartDate: string,
|
||||||
|
oldStartTime: string,
|
||||||
|
oldEndDate: string,
|
||||||
|
oldEndTime: string,
|
||||||
|
): { endDate: string; endTime: string } {
|
||||||
|
const oldStartMs = new Date(`${oldStartDate}T${oldStartTime}:00`).getTime()
|
||||||
|
const oldEndMs = new Date(`${oldEndDate}T${oldEndTime}:00`).getTime()
|
||||||
|
const deltaMs = oldEndMs > oldStartMs ? oldEndMs - oldStartMs : 60 * 60 * 1000
|
||||||
|
const newEndDate = new Date(new Date(`${newStartDate}T${newStartTime}:00`).getTime() + deltaMs)
|
||||||
|
return {
|
||||||
|
endDate: localDateISO(newEndDate),
|
||||||
|
endTime: localTimeHHMM(newEndDate),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Preserve all-day day-span on start date change. Returns new endDate string. */
|
||||||
|
export function computeNewAllDayEnd(
|
||||||
|
newStartDate: string,
|
||||||
|
oldStartDate: string,
|
||||||
|
oldEndDate: string, // inclusive
|
||||||
|
): string {
|
||||||
|
const span = Math.max(0, dateDiffDays(oldStartDate, oldEndDate))
|
||||||
|
return addDaysISO(newStartDate, span)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### D-11 typed error (client.ts extension)
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Source: design from CONTEXT.md D-11 + codebase analysis
|
||||||
|
|
||||||
|
export class SessionExpiredError extends Error {
|
||||||
|
readonly name = 'SessionExpiredError'
|
||||||
|
constructor() {
|
||||||
|
super('Session expired — re-authentication required')
|
||||||
|
Object.setPrototypeOf(this, SessionExpiredError.prototype)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Add to all fetch wrappers (fetchEvents, createEvent, etc.):
|
||||||
|
const res = await fetch('/api/events', { credentials: 'include', redirect: 'manual' })
|
||||||
|
if (res.type === 'opaqueredirect' || res.status === 401) throw new SessionExpiredError()
|
||||||
|
if (!res.ok) throw new Error(`GET /api/events failed: ${res.status}`)
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Assumptions Log
|
||||||
|
|
||||||
|
| # | Claim | Section | Risk if Wrong |
|
||||||
|
|---|-------|---------|---------------|
|
||||||
|
| A1 | RFC 5545 §3.3.10 requires UNTIL value-type to match DTSTART value-type (DATE vs DATETIME) | Focus 1, Pitfall 1 | Fastmail may silently accept mismatched types, reducing impact; or Fastmail strictly rejects — causes failed sync for bounded recurring events |
|
||||||
|
| A2 | Using `UNTIL=YYYYMMDDTHHMMSSz` (end of UTC day) as universal safe choice for timed events | Focus 1, Pitfall 2 | Users in UTC+N>1 may lose the final occurrence; acceptable trade-off for v1 |
|
||||||
|
| A3 | TanStack Query v5 global error handler uses cache subscription, not `defaultOptions.onError` | Focus 6, Pitfall 5 | If wrong, the session expiry handler silently does nothing |
|
||||||
|
| A4 | FREQ-persistence bug (daily → weekly) was a one-time or stale-state issue, not a reproducible code bug | Focus 2 | If it is reproducible, the root cause is not identified — regression test will catch it |
|
||||||
|
| A5 | `CalendarOccurrence.hasRrule` addition requires no DB query changes (already available via `event.isRecurring()`) | Focus 3 | If the API route does not pass hasRrule through the expand call correctly, the PWA always sees false |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Open Questions
|
||||||
|
|
||||||
|
1. **TanStack Query v5 global error handler API**
|
||||||
|
- What we know: `defaultOptions.onError` was removed in TQ v5.
|
||||||
|
- What's unclear: exact API for subscribing to all query/mutation errors globally.
|
||||||
|
- Recommendation: Planner should add a Context7 lookup task for TanStack Query 5 `QueryCache` / `MutationCache` subscription API before implementing D-11.
|
||||||
|
|
||||||
|
2. **Fastmail UNTIL value-type enforcement**
|
||||||
|
- What we know: RFC 5545 specifies value-type matching. ical.js produces the correct form.
|
||||||
|
- What's unclear: whether Fastmail enforces it strictly or silently accepts mismatched types.
|
||||||
|
- Recommendation: Test with a bounded recurring event in the dev environment as part of verification.
|
||||||
|
|
||||||
|
3. **In-flight write preservation on session expiry (D-11 nice-to-have)**
|
||||||
|
- CONTEXT.md D-11 marks this as a nice-to-have, not a hard requirement.
|
||||||
|
- Recommendation: Planner should defer this to v1.1 unless it fits naturally into the interstitial implementation.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Environment Availability
|
||||||
|
|
||||||
|
Step 2.6: No new external dependencies required. All tooling (Node.js 22, pnpm, MariaDB, Redis, playwright-cli) is available from prior phases. `playwright-cli` confirmed at `/usr/local/bin/playwright-cli`.
|
||||||
|
|
||||||
|
| Dependency | Required By | Available | Version | Fallback |
|
||||||
|
|------------|------------|-----------|---------|----------|
|
||||||
|
| playwright-cli | Browser-level verification | ✓ | (global binary) | Human checkpoint |
|
||||||
|
| ical.js 2.2.1 | RRULE serialization | ✓ | 2.2.1 | — |
|
||||||
|
| Vitest | Unit + integration tests | ✓ | (existing) | — |
|
||||||
|
| MariaDB | API integration tests | ✓ | (dev stack) | — |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Validation Architecture
|
||||||
|
|
||||||
|
> `workflow.nyquist_validation` is absent from `.planning/config.json` → treated as enabled.
|
||||||
|
|
||||||
|
### Test Framework
|
||||||
|
|
||||||
|
| Property | Value |
|
||||||
|
|----------|-------|
|
||||||
|
| API framework | Vitest, `apps/api/vitest.config.ts`, `environment: 'node'` |
|
||||||
|
| PWA framework | Vitest, `apps/pwa/vitest.config.ts`, `environment: 'jsdom'` |
|
||||||
|
| API quick run | `cd apps/api && pnpm test` |
|
||||||
|
| PWA quick run | `cd apps/pwa && pnpm test` |
|
||||||
|
| Full suite | `pnpm test` (root — runs API only; planner should add `pnpm --filter @familysync/pwa test` to full gate) |
|
||||||
|
| Browser-level | `playwright-cli` (global binary at `/usr/local/bin/playwright-cli`) |
|
||||||
|
|
||||||
|
### Phase Requirements → Test Map
|
||||||
|
|
||||||
|
| Fix | Behavior | Test Type | Automated Command | File Exists? |
|
||||||
|
|-----|----------|-----------|-------------------|-------------|
|
||||||
|
| D-04: duration preserve (timed) | `computeNewTimedEnd` returns correct date/time given old start/end + new start | unit | `cd apps/pwa && pnpm test -- lib/eventDateTime` | ❌ Wave 0 — extend `eventDateTime.test.ts` |
|
||||||
|
| D-04: duration preserve (all-day) | `computeNewAllDayEnd` returns correct date given day-span + new start | unit | `cd apps/pwa && pnpm test -- lib/eventDateTime` | ❌ Wave 0 |
|
||||||
|
| D-04: floor rule | end never strands behind start for both timed and all-day | unit | `cd apps/pwa && pnpm test -- lib/eventDateTime` | ❌ Wave 0 |
|
||||||
|
| D-06: RRULE COUNT serialize | `buildVeventString({rruleString:'FREQ=WEEKLY;COUNT=5'})` produces correct ICS | unit | `cd apps/api && pnpm test -- broker/vevent` | ✅ `vevent.test.ts` — add cases |
|
||||||
|
| D-06: RRULE UNTIL DATE | all-day UNTIL serializes as `YYYYMMDD` not `YYYYMMDDTHHMMSSZ` | unit | `cd apps/api && pnpm test -- broker/vevent` | ✅ add cases |
|
||||||
|
| D-06: RRULE UNTIL DATETIME | timed UNTIL serializes as `YYYYMMDDTHHMMSSZ` | unit | `cd apps/api && pnpm test -- broker/vevent` | ✅ add cases |
|
||||||
|
| D-06: per-occurrence duration independent of UNTIL | expand with bounded RRULE; each occurrence has duration from DTSTART→DTEND | unit | `cd apps/api && pnpm test -- broker/expand` | ✅ `expand.test.ts` — add case |
|
||||||
|
| D-07: FREQ regression | create daily event → outbox carries `recurrence:'daily'` → RRULE is `FREQ=DAILY` | unit | `cd apps/api && pnpm test -- broker/outboxWorker` | ✅ add snapshot case |
|
||||||
|
| D-08: hasRrule in occurrence | `expandOccurrences` sets `hasRrule:true` for recurring events | unit | `cd apps/api && pnpm test -- broker/expand` | ✅ add assertion |
|
||||||
|
| D-10: no calendar flash | App shows spinner not skeleton/alert on cold unauthenticated load | browser | `playwright-cli` against dev server | — human only on iOS |
|
||||||
|
| D-11: SessionExpiredError from 401 | `fetchEvents(...)` with mocked 401 response throws `SessionExpiredError` | unit | `cd apps/pwa && pnpm test -- api/client` | ✅ extend `client.test.ts` |
|
||||||
|
| D-11: SessionExpiredError from opaqueredirect | `fetchEvents(...)` with mocked opaqueredirect throws `SessionExpiredError` | unit | `cd apps/pwa && pnpm test -- api/client` | ✅ extend `client.test.ts` |
|
||||||
|
| D-11: global handler triggers interstitial | Session-expiry interstitial appears on mid-use 401 | browser | `playwright-cli` | ✗ (no mock-auth tooling) |
|
||||||
|
| D-13: spin animation visible | Loader2 in SyncStateToast actually rotates | browser | `playwright-cli` | — |
|
||||||
|
| D-13: pulse animation visible | LiveSyncIndicator reconnecting dot actually pulses | browser | `playwright-cli` | — |
|
||||||
|
| D-13: pulse keyframe in CSS | `@keyframes pulse` present in tokens.css | unit | grep check or CSS parse | ❌ Wave 0 |
|
||||||
|
|
||||||
|
### TDD-Eligible Items (pure functions with defined I/O)
|
||||||
|
|
||||||
|
Write tests FIRST for these:
|
||||||
|
- `computeNewTimedEnd` / `computeNewAllDayEnd` (D-04) — deterministic duration math
|
||||||
|
- `buildVeventString` with COUNT / UNTIL variants (D-06) — deterministic ICS output
|
||||||
|
- `expandOccurrences` with bounded RRULE (D-06) — deterministic occurrence count
|
||||||
|
- `SessionExpiredError` detection in `fetchEvents`, `createEvent`, `updateEvent` (D-11)
|
||||||
|
|
||||||
|
### Glue/UI items (not TDD-eligible, use browser verification)
|
||||||
|
|
||||||
|
- EventForm end-tracking `onChange` handler wiring (D-04) — test via playwright-cli
|
||||||
|
- Auth splash rendering (D-10) — playwright-cli
|
||||||
|
- Spinner/pulse animation (D-13) — playwright-cli
|
||||||
|
- Series-edit confirmation prompt (D-08/D-09) — playwright-cli
|
||||||
|
|
||||||
|
### Playwright-cli scope (desktop-Chromium drivable vs iOS-only)
|
||||||
|
|
||||||
|
| Check | Desktop-Chromium OK | iOS-Safari Required |
|
||||||
|
|-------|--------------------|--------------------|
|
||||||
|
| No auth flash on cold load | ✓ playwright-cli (simulate no session cookie) | Informative but not required |
|
||||||
|
| "Signing you in…" splash renders | ✓ playwright-cli | — |
|
||||||
|
| Session-expiry interstitial renders | ✓ playwright-cli (intercept with 401) | — |
|
||||||
|
| Spinner actually animates in SyncStateToast | ✓ playwright-cli | — |
|
||||||
|
| Pulse dot animates in LiveSyncIndicator | ✓ playwright-cli | — |
|
||||||
|
| All-day event visual distinction | ✓ playwright-cli | — |
|
||||||
|
| iOS PWA standalone push behaviour | ✗ human checkpoint | ✓ required |
|
||||||
|
|
||||||
|
### Sampling Rate
|
||||||
|
- **Per task commit:** Run the relevant test file for the changed module.
|
||||||
|
- **Per wave merge:** `cd apps/api && pnpm test && cd ../pwa && pnpm test` (full suites).
|
||||||
|
- **Phase gate:** Full suite green + playwright-cli checks complete before `/gsd-verify-work`.
|
||||||
|
|
||||||
|
### Wave 0 Gaps
|
||||||
|
- [ ] `apps/pwa/src/lib/eventDateTime.test.ts` — extend with `computeNewTimedEnd`, `computeNewAllDayEnd`, floor-rule cases (D-04)
|
||||||
|
- [ ] `apps/api/tests/broker/vevent.test.ts` — add UNTIL (DATE), UNTIL (DATETIME), COUNT cases (D-06)
|
||||||
|
- [ ] `apps/api/tests/broker/expand.test.ts` — add bounded RRULE + `hasRrule` cases (D-06/D-08)
|
||||||
|
- [ ] `apps/pwa/src/api/client.test.ts` — add `SessionExpiredError` detection cases for all fetch functions (D-11)
|
||||||
|
|
||||||
|
*(If no new test files are needed — all gaps are extensions to existing files.)*
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Security Domain
|
||||||
|
|
||||||
|
| ASVS Category | Applies | Standard Control |
|
||||||
|
|---------------|---------|-----------------|
|
||||||
|
| V2 Authentication | yes — D-10/D-11 auth gating | `@hono/oidc-auth` + PKCE (existing); session expiry redirect |
|
||||||
|
| V3 Session Management | yes — D-11 session expiry detection | `redirect:'manual'` + `SessionExpiredError`; no token storage in client |
|
||||||
|
| V4 Access Control | no — no new endpoints | — |
|
||||||
|
| V5 Input Validation | yes — D-06 UNTIL date input | Zod validation on `recurrenceUntil` (date format) + `recurrenceCount` (integer ≥ 1) |
|
||||||
|
| V6 Cryptography | no | — |
|
||||||
|
|
||||||
|
### Known Threat Patterns for this phase
|
||||||
|
|
||||||
|
| Pattern | STRIDE | Standard Mitigation |
|
||||||
|
|---------|--------|---------------------|
|
||||||
|
| RRULE injection via `recurrenceUntil` | Tampering | Zod `.string().max(10)` + date format regex on API schema; `ICAL.Recur.fromString` sanitizes via parsing |
|
||||||
|
| Session cookie theft (not new, but D-11 surfaces the expiry path) | Spoofing | Same-origin cookie, `httpOnly`, existing Authelia session contract |
|
||||||
|
| Flash of authenticated content before auth check | Info Disclosure | D-10 fix: gate render on `meQuery.isSuccess` not `meQuery.isLoading` |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Sources
|
||||||
|
|
||||||
|
### Primary (HIGH confidence — verified in codebase)
|
||||||
|
|
||||||
|
- `apps/pwa/src/components/EventForm.tsx` — end-tracking gap (lines 662–689), FREQ state reset (lines 207, 257–258), D-05 fix (line 199)
|
||||||
|
- `apps/pwa/src/api/client.ts` — `RecurrencePreset` type (line 130), `fetchMe` redirect:manual (lines 36–53), auth error shape
|
||||||
|
- `apps/api/src/broker/vevent.ts` — `RRULE_PRESETS` map (lines 49–54), `buildVeventString` RRULE path (lines 143–148)
|
||||||
|
- `apps/api/src/broker/expand.ts` — `CalendarOccurrence` interface (no `hasRrule`), per-occurrence duration via `event.duration` (lines 271–283)
|
||||||
|
- `apps/api/src/broker/outboxWorker.ts` — RRULE preservation path (lines 255–307)
|
||||||
|
- `apps/pwa/src/components/CalendarShell.tsx` — auth flash root cause (lines 213–235), `meQuery.isError` branch
|
||||||
|
- `apps/pwa/src/lib/loginRedirect.ts` — `maybeRedirectToLogin()` one-shot guard
|
||||||
|
- `apps/pwa/src/styles/tokens.css` — `@keyframes spin` present (lines 140–147), `@keyframes pulse` ABSENT
|
||||||
|
- `apps/pwa/src/components/SyncStateToast.tsx` — `animation: 'spin 1s linear infinite'` (line 158)
|
||||||
|
- `apps/pwa/src/components/LiveSyncIndicator.tsx` — `animation: 'pulse 1.4s ease-in-out infinite'` (line 69)
|
||||||
|
- `apps/pwa/src/components/PushPermissionPrompt.tsx` — redundant local `@keyframes spin` (lines 358–363)
|
||||||
|
|
||||||
|
### Secondary (MEDIUM confidence — Context7 + live code evaluation)
|
||||||
|
|
||||||
|
- `ical.js 2.2.1` — RRULE UNTIL/COUNT serialization verified via `node -e` against project node_modules: `ICAL.Recur.fromString('FREQ=WEEKLY;COUNT=5').toString() === 'FREQ=WEEKLY;COUNT=5'` etc.
|
||||||
|
- Context7 `/kewisch/ical.js` — `ICAL.design.icalendar.value['recur']` fromICAL/toICAL contract
|
||||||
|
|
||||||
|
### Tertiary (LOW confidence — ASSUMED)
|
||||||
|
|
||||||
|
- RFC 5545 §3.3.10 UNTIL value-type matching rule
|
||||||
|
- TanStack Query v5 cache subscription API for global error handling
|
||||||
|
- Fastmail UNTIL value-type enforcement behavior
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Metadata
|
||||||
|
|
||||||
|
**Confidence breakdown:**
|
||||||
|
- Code-verified findings (file:line): HIGH — read directly from source files
|
||||||
|
- ical.js UNTIL/COUNT API: HIGH — verified via live `node -e` evaluation against project node_modules
|
||||||
|
- Auth gating approach: HIGH — code-verified root cause, fix approach is well-established pattern
|
||||||
|
- TanStack Query v5 global error handler: LOW (ASSUMED) — planner must verify v5 API before implementing
|
||||||
|
- RFC 5545 UNTIL value-type rule: LOW (ASSUMED)
|
||||||
|
|
||||||
|
**Research date:** 2026-06-10
|
||||||
|
**Valid until:** 2026-07-10 (stable stack; only ASSUMED items need re-verification)
|
||||||
Reference in New Issue
Block a user