diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 8a9f250..b86d609 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -76,6 +76,13 @@ Plans: - [x] 02-05-PLAN.md — UX completion: read-only EventDetailPopover + ColorLegend + nav/toolbar + skeleton/empty/error states + human visual verification +**Gap-closure waves** *(from 03-REVIEW.md — write path was broken end-to-end; Gate 2 / 03-08 is blocked on these)* + +- [ ] 03-09-PLAN.md — Route layer: align zod schema to client title/start/end contract (CR-01) + real OIDC iss/sub→users.id resolution on all 5 handlers (CR-06) [wave 1] +- [ ] 03-12-PLAN.md — PWA EventForm: edit-mode population + recurrence preselect (WR-03), zone-consistent dates (WR-05), real focus trap (WR-07); PWA-01/02 install assets verified [wave 1] +- [ ] 03-10-PLAN.md — Worker dispatch: build real VEVENT via buildVeventString + all-day DTEND+1 (CR-02/WR-04), fail closed on bad creds (CR-03), backoff index + randomUUID (WR-01/WR-08) [wave 2, after 03-09] +- [ ] 03-11-PLAN.md — Outbox durability: durable create-before-delete (CR-04), drain concurrency guard (CR-05), fresh-etag-before-PUT (WR-02) [wave 3, after 03-10] + **UI hint**: yes ### Phase 3: Event Write-Back + PWA Install @@ -93,7 +100,7 @@ Plans: 5. The installed PWA opens full-screen without browser chrome on both iOS and Android 6. **(Carried from Phase 1 Gate 2, D-14)** Live Authelia OIDC login works over the public Pangolin URL — including the **iOS standalone-PWA** flow: the wife can install to Home Screen and complete login without the redirect breaking out of standalone mode; sessions persist (AUTH-01/02) and members get distinct stable colors (AUTH-03). Verify per `docs/deployment.md` Gate 2 checklist; this is the first real external deploy (local Newt test rig is sufficient — Unraid prod is optional until go-live). -**Plans**: 8 plans +**Plans**: 12 plans (8 original + 4 gap-closure from 03-REVIEW.md) Plans: **Wave 1** diff --git a/.planning/phases/03-event-write-back-pwa-install/03-09-PLAN.md b/.planning/phases/03-event-write-back-pwa-install/03-09-PLAN.md new file mode 100644 index 0000000..43bcf3d --- /dev/null +++ b/.planning/phases/03-event-write-back-pwa-install/03-09-PLAN.md @@ -0,0 +1,174 @@ +--- +phase: 03-event-write-back-pwa-install +plan: 09 +type: tdd +wave: 1 +depends_on: [] +gap_closure: true +autonomous: true +requirements: [CAL-04, CAL-05, CAL-06] +files_modified: + - apps/api/src/routes/events.ts + - apps/api/tests/routes/events.test.ts +must_haves: + truths: + - "POST /api/events/create with the exact client CreateEventPayload shape ({title,start,end,allDay,recurrence}) returns 202, not 400" + - "PATCH /api/events/:uid/edit with the same client shape returns 202, not 400" + - "An authenticated OIDC request (devBypassActive=false) with a known iss+sub resolves to a real users.id and is allowed to write — it does NOT unconditionally 401" + - "A request with no dev user and no OIDC session returns 401" + artifacts: + - path: apps/api/src/routes/events.ts + provides: "Canonical title/start/end zod contract + async OIDC iss/sub→users.id resolution on all 5 handlers" + contains: "upsertUser" + key_links: + - from: "apps/api/src/routes/events.ts" + to: "apps/api/src/auth/user.ts" + via: "upsertUser(iss, sub, email)" + pattern: "upsertUser\\(" +--- + + +Fix the route layer so the write path is reachable at all: align the server zod +schema to the contract the PWA actually sends (CR-01), and implement the real +OIDC iss/sub → users.id resolution that all five write/sync handlers stub out as +a hard 401 today (CR-06). Without this plan every create/edit returns 400 in dev +and 401 in production — the entire phase acceptance criterion is unreachable. + +Purpose: make the events router accept real client requests under real Authelia auth. +Output: an events router whose schema matches `CreateEventPayload` and whose OIDC +path resolves authenticated members to a DB user via the existing `upsertUser` helper. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/03-event-write-back-pwa-install/03-REVIEW.md +@.planning/phases/03-event-write-back-pwa-install/03-CONTEXT.md +@apps/api/src/routes/events.ts +@apps/api/src/auth/user.ts +@apps/api/src/routes/me.ts +@apps/pwa/src/api/client.ts + + + +This gap plan introduces NO new exported symbols. It changes the in-module +`eventFieldsSchema` field names and converts the private `resolveUserId(c)` helper +into an async `resolveUserId(c): Promise` that consults `upsertUser`. +Downstream gap plans (03-10) read the new field names (`title/start/end`) out of +`calendarOutbox.payload`. + + + + + + Task 1: RED+GREEN — adopt the canonical title/start/end contract (CR-01) + apps/api/src/routes/events.ts, apps/api/tests/routes/events.test.ts + + - apps/api/src/routes/events.ts (eventFieldsSchema at lines 67-77; create handler ~191; edit handler ~268) + - apps/pwa/src/api/client.ts (CreateEventPayload at lines 119-128 — the authoritative client shape) + - apps/api/tests/routes/events.test.ts (existing route tests — they currently pass because they send the SERVER field names; that is the wrong boundary the review flagged) + - .planning/phases/03-event-write-back-pwa-install/03-REVIEW.md (CR-01) + + + - RED: a new contract test imports the `CreateEventPayload` TYPE shape from the PWA client (or replicates it literally as `{title,start,end,allDay,recurrence}` with a comment citing client.ts:119-128) and POSTs it to /api/events/create — asserts 202, NOT 400. This fails today because zod requires summary/dtstart/dtend. + - RED: a second test PATCHes the same shape to /api/events/:uid/edit — asserts 202, NOT 400. + - GREEN: both pass after the schema is renamed. + + + Canonical contract chosen: the SERVER adopts the CLIENT field names `title/start/end` + (the PWA `CreateEventPayload`, `EventForm.handleSubmit`, and `createEvent`/`updateEvent` + already send these — adopting them server-side requires zero PWA churn). + + In events.ts rename `eventFieldsSchema` fields to exactly: + `title: z.string().min(1).max(255)`, `allDay: z.boolean()`, + `start: z.string().min(1).max(64)`, `end: z.string().min(1).max(64)`, + `location: z.string().max(2000).optional()`, `description: z.string().max(2000).optional()`, + `recurrence: z.enum(['none','daily','weekly','monthly','yearly']).optional()`, + `calendarUrl: z.string().url().max(1024).optional()`. + Keep `recurrence` `.optional()` server-side (the client always sends it, but the + contract drift the review noted resolves either way once names match). + + The route still stores `payload: JSON.stringify(payload)` unchanged — the worker + (plan 03-10) now parses `title/start/end` from it. Do NOT introduce summary/dtstart/dtend + anywhere; do NOT add an internal rename map (the review's "map internally" alternative is + rejected to keep one canonical name set end-to-end). + + Add the two contract tests described in . Commit RED then GREEN + (`test(03-09): ...` then `feat(03-09): ...`). + + + cd apps/api && npx vitest run tests/routes/events.test.ts + + + - behavior: POST /api/events/create with `{title,start,end,allDay,recurrence}` returns 202. + - behavior: PATCH /api/events/:uid/edit with the same shape returns 202. + - source: `grep -n 'summary\|dtstart\|dtend' apps/api/src/routes/events.ts` returns no matches in eventFieldsSchema. + - test-command: `cd apps/api && npx vitest run tests/routes/events.test.ts` passes. + + The server schema accepts the exact payload the PWA sends; no create/edit is rejected at the validator boundary for field-name drift. + + + + Task 2: RED+GREEN — resolve OIDC iss/sub to a real users.id on all 5 handlers (CR-06) + apps/api/src/routes/events.ts, apps/api/tests/routes/events.test.ts + + - apps/api/src/routes/events.ts (resolveUserId at lines 50-55; the five 401-stub blocks at ~194-200, ~270-273, ~374-377, ~439-442, ~492-495) + - apps/api/src/auth/user.ts (upsertUser — the canonical iss/sub→users row helper already used by me.ts) + - apps/api/src/routes/me.ts (the reference OIDC resolution pattern: getAuth → iss/sub/email → upsertUser) + - .planning/phases/03-event-write-back-pwa-install/03-REVIEW.md (CR-06) + + + - RED: a test that simulates the production OIDC path (no dev `c.get('user')`; `getAuth` mocked to return a valid `{iss, sub, email}`) POSTs /api/events/create and asserts the response is 202 AND that the row was attributed to the upserted user id (currentUserId != null). Fails today because the handler returns 401 even when auth is truthy. + - RED: a test with no dev user and `getAuth` returning null asserts 401 (the genuinely-unauthenticated case still 401s). + + + Convert `resolveUserId(c)` to an async helper `async function resolveUserId(c): Promise`: + 1. If `c.get('user')` exists (dev bypass), return its `.id` (unchanged). + 2. Else call `await getAuth(c)`. If falsy, return null (caller emits 401). + 3. Else extract `iss = (auth.iss as string) ?? ''`, `sub = auth.sub ?? ''`, + `email = typeof auth.email === 'string' ? auth.email : undefined`, then + `const user = await upsertUser(iss, sub, email)` and return `user?.id ?? null`. + Import `upsertUser` from `../auth/user.js`. + + In each of the 5 handlers (create, edit, delete, sync-status, writable-calendars) + replace the `resolveUserId(...)` call + inline getAuth/401 stub block with: + `const currentUserId = await resolveUserId(c)` then `if (currentUserId === null) return c.json({ error: 'Unauthorized' }, 401)`. + Remove every `// For now return 401` stub and the now-redundant inner `getAuth` calls in the handlers. + Per D-10 identity is oidc_iss+oidc_sub; upsertUser keys on `uniq_oidc_identity`. Return 401 ONLY when no session exists (covered by upsertUser path). + + Add the two tests in . Commit RED then GREEN. + + + cd apps/api && npx vitest run tests/routes/events.test.ts + + + - behavior: an OIDC request with known iss+sub resolves currentUserId != null and the write enqueues (202). + - behavior: a request with neither dev user nor OIDC session returns 401. + - source: `grep -c 'For now return 401' apps/api/src/routes/events.ts` returns 0. + - source: `grep -c 'upsertUser' apps/api/src/routes/events.ts` returns >= 1. + - test-command: `cd apps/api && npx vitest run tests/routes/events.test.ts` passes. + + Authenticated Authelia members resolve to a DB user id on every write/sync/writable-calendars handler in production; only genuinely unauthenticated requests 401. + + + + + +- `cd apps/api && npx vitest run tests/routes/events.test.ts` green. +- `cd apps/api && npm run build` (or tsc) succeeds with the async resolveUserId signature. + + + +The events router accepts the real PWA payload and resolves real OIDC members. +The write path is no longer dead-on-arrival at the route boundary (CR-01, CR-06 closed). + + + +Create `.planning/phases/03-event-write-back-pwa-install/03-09-SUMMARY.md` when done. + diff --git a/.planning/phases/03-event-write-back-pwa-install/03-10-PLAN.md b/.planning/phases/03-event-write-back-pwa-install/03-10-PLAN.md new file mode 100644 index 0000000..bb3325a --- /dev/null +++ b/.planning/phases/03-event-write-back-pwa-install/03-10-PLAN.md @@ -0,0 +1,185 @@ +--- +phase: 03-event-write-back-pwa-install +plan: 10 +type: tdd +wave: 2 +depends_on: ["03-09"] +gap_closure: true +autonomous: true +requirements: [CAL-04, CAL-05, CAL-06, CAL-07] +files_modified: + - apps/api/src/broker/outboxWorker.ts + - apps/api/src/broker/vevent.ts + - apps/api/tests/broker/outboxWorker.test.ts + - apps/api/tests/broker/vevent.test.ts +must_haves: + truths: + - "The worker parses the stored form JSON and PUTs a real VCALENDAR string built by buildVeventString — never raw {\"title\":...} JSON" + - "The PUT body begins with 'BEGIN:VCALENDAR' for both create and update operations" + - "A single-day all-day event produces DTEND = DTSTART + 1 day (RFC 5545 exclusive end)" + - "A credential-load failure leaves the row pending for retry — the worker never PUTs with empty Basic-auth" + - "The first transient failure waits 15s (BACKOFF_SECONDS[0]), not 60s" + artifacts: + - path: apps/api/src/broker/outboxWorker.ts + provides: "ICS-building dispatch path + removed empty-cred fallback + corrected backoff index + explicit randomUUID import" + contains: "buildVeventString" + - path: apps/api/src/broker/vevent.ts + provides: "All-day DTEND+1-day exclusivity fix" + key_links: + - from: "apps/api/src/broker/outboxWorker.ts" + to: "apps/api/src/broker/vevent.ts" + via: "buildVeventString(parsedFormFields)" + pattern: "buildVeventString\\(" + - from: "apps/api/src/broker/outboxWorker.ts" + to: "apps/api/src/broker/write.ts" + via: "createCalendarEvent/updateCalendarEvent with the built icsString" + pattern: "createCalendarEvent\\(|updateCalendarEvent\\(" +--- + + +Make the outbox worker actually write a valid calendar object. Today it PUTs the +raw form JSON (`{"title":...}`) to Fastmail — `buildVeventString` (the whole D-13 +DATE/DATETIME contract) is dead code (CR-02). It also silently authenticates with +empty credentials on any credential-load error (CR-03), skips its first backoff +delay (WR-01), and mishandles the all-day exclusive DTEND (WR-04). This plan wires +the VEVENT builder into the dispatch path and fixes those correctness defects. + +Purpose: a queued write becomes a real, RFC-5545-valid VEVENT on the correct calendar. +Output: a worker that builds ICS from the stored form fields and fails closed on bad credentials. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/STATE.md +@.planning/phases/03-event-write-back-pwa-install/03-REVIEW.md +@.planning/phases/03-event-write-back-pwa-install/03-09-SUMMARY.md +@apps/api/src/broker/outboxWorker.ts +@apps/api/src/broker/vevent.ts +@apps/api/src/broker/write.ts + + + +No new exported symbols. `buildVeventString` and `RRULE_PRESETS` (already exported by +vevent.ts) become live call sites for the first time. The worker's dispatch path gains +an internal `JSON.parse(row.payload)` → `buildVeventString` step. + + + + + + Task 1: RED+GREEN — worker builds and PUTs a real VEVENT (CR-02, WR-04) + apps/api/src/broker/outboxWorker.ts, apps/api/src/broker/vevent.ts, apps/api/tests/broker/outboxWorker.test.ts, apps/api/tests/broker/vevent.test.ts + + - apps/api/src/broker/outboxWorker.ts (dispatchRow lines 131-188 — the create/update branches that pass row.payload straight through) + - apps/api/src/broker/vevent.ts (buildVeventString signature lines 52-118; NewEventParams lines 21-33; RRULE_PRESETS lines 39-44; all-day DATE handling lines 69-88) + - apps/api/src/routes/events.ts (the route stores payload: JSON.stringify(payload) with the new title/start/end fields from plan 03-09) + - apps/api/tests/broker/outboxWorker.test.ts (line ~85 hardcodes payload:'BEGIN:VCALENDAR' and mocks write.js — the wrong boundary; the new test must stop mocking the ICS string and assert the worker BUILDS it) + - .planning/phases/03-event-write-back-pwa-install/03-REVIEW.md (CR-02, WR-04) + + + - RED (worker): with `write.js` NOT mocking away the payload — i.e. spy on `createCalendarEvent` and capture its 4th arg `icsString` — enqueue a create row whose `payload` is `JSON.stringify({title:'Lunch',allDay:false,start:'2026-06-10T12:00:00',end:'2026-06-10T13:00:00',recurrence:'none'})`. Assert the captured icsString `.startsWith('BEGIN:VCALENDAR')` and contains `SUMMARY:Lunch`. Fails today (raw JSON is passed). + - RED (worker): an update row likewise yields an icsString starting with `BEGIN:VCALENDAR` passed to `updateCalendarEvent`. + - RED (worker): a row whose `payload` is not valid JSON marks the row `failed` (hard fail, no retry). + - RED (vevent): a single-day all-day event `{allDay:true, start:'2026-06-10', end:'2026-06-10'}` produces an icsString whose DTEND is `2026-06-11` (start + 1 day), not equal to DTSTART. + + + In `dispatchRow`, for `operation === 'create'` and `operation === 'update'`: + `const fields = JSON.parse(row.payload)` wrapped in try/catch; on parse failure + return `{success:false, conflict:false, hardFail:true, transient:false, error:'payload parse failed'}` + (hard fail — corrupt payload will never self-resolve). + Then build the ICS: + `const { icsString } = buildVeventString({ uid: row.uid, summary: fields.title, allDay: fields.allDay, + dtstart: fields.allDay ? fields.start : new Date(fields.start), + dtend: fields.allDay ? fields.end : new Date(fields.end), + location: fields.location, description: fields.description, + rruleString: fields.recurrence && fields.recurrence !== 'none' ? RRULE_PRESETS[fields.recurrence] : undefined })`. + Pass `icsString` (NOT `row.payload`) to `createCalendarEvent(client, davCalendar, row.uid, icsString)` + and to `updateCalendarEvent(client, row.calendarObjectUrl, icsString, row.etag ?? null)`. + Import `{ buildVeventString, RRULE_PRESETS }` from `./vevent.js`. Delete operations are unchanged (no payload). + + WR-04 in vevent.ts: in the all-day branch (lines 69-88), after computing the end DATE + components, advance the end DATE by one calendar day before constructing `endTime` + (use a Date built from ey/em/ed, `setUTCDate(getUTCDate()+1)`, re-read y/m/d) so a one-day + all-day event serializes DTEND = DTSTART + 1 (RFC 5545 exclusive). Keep the timed branch untouched. + + Update the existing outbox test that fed a pre-built ICS string so it instead feeds + form JSON and asserts the built ICS (it was testing the wrong boundary). Commit RED then GREEN. + + + cd apps/api && npx vitest run tests/broker/outboxWorker.test.ts tests/broker/vevent.test.ts + + + - behavior: the icsString passed to createCalendarEvent starts with 'BEGIN:VCALENDAR' and contains the summary. + - behavior: an unparseable payload marks the row failed with no retry. + - behavior: single-day all-day event yields DTEND = start + 1 day. + - source: `grep -c 'buildVeventString' apps/api/src/broker/outboxWorker.ts` returns >= 1. + - test-command: `cd apps/api && npx vitest run tests/broker/outboxWorker.test.ts tests/broker/vevent.test.ts` passes. + + Every create/update PUTs a real RFC-5545 VCALENDAR built from the stored form fields, honoring D-13 DATE-vs-DATETIME and the exclusive all-day DTEND. + + + + Task 2: RED+GREEN — fail closed on bad credentials, fix backoff index, explicit randomUUID (CR-03, WR-01, WR-08) + apps/api/src/broker/outboxWorker.ts, apps/api/tests/broker/outboxWorker.test.ts + + - apps/api/src/broker/outboxWorker.ts (dispatchRow try/catch fallback lines 135-143; backoff math lines 316-341; BACKOFF_SECONDS lines 40-44) + - apps/api/src/routes/events.ts (uses bare `crypto.randomUUID()` at lines 241,317,318 — WR-08 is the route-side instance; vevent.ts already imports from 'crypto') + - .planning/phases/03-event-write-back-pwa-install/03-REVIEW.md (CR-03, WR-01, WR-08) + + + - RED (CR-03): mock `loadClientForUser` (via the credential/decrypt path) to throw; assert the row is left `pending` (caught by the outer per-row catch in runOutboxDrain) and that `createFastmailClient('', '')` is NEVER invoked. Fails today (the catch falls back to empty creds and proceeds to PUT). + - RED (WR-01): a transient failure on a row with attemptCount=0 sets nextAttemptAt ≈ now + 15s (BACKOFF_SECONDS[0]), not +60s. + + + CR-03: Remove the `try { client = await loadClientForUser(row.userId) } catch { client = await createFastmailClient('','') }` + fallback in `dispatchRow`. Replace with `const client = await loadClientForUser(row.userId)` and let it throw — + the outer per-row `catch` in `runOutboxDrain` (line ~343) already logs and leaves the row pending (correct transient + behavior). Tests that previously relied on the empty-cred fallback must instead mock `loadClientForUser` + (or the underlying credential select + `createFastmailClient`) to return a fake client. Do NOT add a test-only + flag that PUTs with empty creds. + + WR-01: change the backoff index from `nextAttemptCount` to `row.attemptCount` (the attempt that just failed): + `const backoffMs = (BACKOFF_SECONDS[row.attemptCount] ?? 1800) * 1000`. Keep `nextAttemptCount = row.attemptCount + 1` + for the persisted `attemptCount` and the `>= MAX_ATTEMPTS` dead-letter check. This makes the first retry wait 15s. + + WR-08: in events.ts replace the three bare `crypto.randomUUID()` calls (lines 241,317,318) with `randomUUID()` + imported via `import { randomUUID } from 'node:crypto'`, matching vevent.ts. (events.ts is also edited by plan 03-09; + this plan runs after it in a later wave so there is no concurrent edit.) + + Commit RED then GREEN. + + + cd apps/api && npx vitest run tests/broker/outboxWorker.test.ts + + + - behavior: a credential-load failure leaves the row pending and never calls createFastmailClient('', ''). + - behavior: first transient retry delay equals BACKOFF_SECONDS[0] (15s). + - source: `grep -c "createFastmailClient('', '')" apps/api/src/broker/outboxWorker.ts` returns 0. + - source: `grep -c "import { randomUUID } from 'node:crypto'" apps/api/src/routes/events.ts` returns 1. + - test-command: `cd apps/api && npx vitest run tests/broker/outboxWorker.test.ts` passes. + + The worker fails closed on credential errors (retryable), uses the documented 15s-first backoff schedule, and uses an explicitly-imported randomUUID. + + + + + +- `cd apps/api && npx vitest run tests/broker/` green. +- `cd apps/api && npm run build` succeeds. +- `grep -rn buildVeventString apps/api/src` shows a live call site outside vevent.ts (IN-01 closed). + + + +Queued writes serialize to valid VCALENDAR via buildVeventString, all-day DTEND is exclusive, +credential failures retry instead of writing with empty auth, and the backoff schedule matches its docs. +CR-02, CR-03, WR-01, WR-04, WR-08, IN-01 closed. + + + +Create `.planning/phases/03-event-write-back-pwa-install/03-10-SUMMARY.md` when done. + diff --git a/.planning/phases/03-event-write-back-pwa-install/03-11-PLAN.md b/.planning/phases/03-event-write-back-pwa-install/03-11-PLAN.md new file mode 100644 index 0000000..15035f1 --- /dev/null +++ b/.planning/phases/03-event-write-back-pwa-install/03-11-PLAN.md @@ -0,0 +1,158 @@ +--- +phase: 03-event-write-back-pwa-install +plan: 11 +type: tdd +wave: 3 +depends_on: ["03-10"] +gap_closure: true +autonomous: true +requirements: [CAL-05, CAL-06] +files_modified: + - apps/api/src/broker/outboxWorker.ts + - apps/api/tests/broker/outboxWorker.test.ts +must_haves: + truths: + - "An edit-as-move delete row never dispatches until its paired create row has reached status='done' — durably, across separate drain cycles" + - "Two overlapping drain cycles never both dispatch the same outbox row" + - "A same-calendar update re-reads the freshest etag from calendarEvents just before PUT, so rapid successive edits do not spuriously 412" + artifacts: + - path: apps/api/src/broker/outboxWorker.ts + provides: "Durable create-before-delete gating, drain concurrency guard, fresh-etag-before-PUT" + contains: "isDraining" + key_links: + - from: "runOutboxDrain" + to: "calendarOutbox status machine" + via: "in-flight claim / blocked-delete gate persisted in DB, not an in-memory Set" + pattern: "isDraining|processing|blocked" +--- + + +Close the outbox durability and concurrency holes. The create-before-delete +ordering for edit-as-move (D-04) is enforced only by an in-memory `Set` that holds +within a single drain batch — a move pair straddling batches can delete the original +before the new copy is confirmed (CR-04, the exact "lost event" D-04 forbids). There +is also no guard against overlapping 15s drain cycles double-dispatching the same +still-`pending` row (CR-05), and same-calendar updates trust a stale enqueue-time etag +that guarantees a spurious 412 on a second quick edit (WR-02). + +Purpose: the outbox is durable and non-duplicating under real timing. +Output: a worker whose ordering and exactly-once guarantees survive across drain cycles. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/STATE.md +@.planning/phases/03-event-write-back-pwa-install/03-REVIEW.md +@.planning/phases/03-event-write-back-pwa-install/03-10-SUMMARY.md +@apps/api/src/broker/outboxWorker.ts +@apps/api/src/db/schema.ts +@apps/api/src/broker/sync.ts + + + +No new exported symbols. Adds a module-level `isDraining` guard in outboxWorker.ts +and durable status gating for paired delete rows (reusing the existing `calendarOutbox` +`status` enum and `groupId` column — no schema migration required: a paired delete is +gated by querying its sibling create's status, not enqueued as a new enum value). + + + + + + Task 1: RED+GREEN — durable create-before-delete + drain concurrency guard (CR-04, CR-05) + apps/api/src/broker/outboxWorker.ts, apps/api/tests/broker/outboxWorker.test.ts + + - apps/api/src/broker/outboxWorker.ts (failedCreateGroups Set lines 271-296; per-batch sort lines 263-269; startOutboxWorker schedule lines 359-365; runOutboxDrain entry line 247) + - apps/api/src/db/schema.ts (calendarOutbox: status enum pending|done|failed|dead, groupId, lines 125-154) + - .planning/phases/03-event-write-back-pwa-install/03-REVIEW.md (CR-04, CR-05) + + + - RED (CR-04 cross-batch): simulate a move pair where the create row and the delete row are returned in SEPARATE drain calls. Drain 1 returns only the delete row (create not yet done). Assert the delete is NOT dispatched (deleteCalendarEvent not called) because the paired create (queried by groupId) is not `done`. Drain 2, after the create has reached `done`, dispatches the delete. Fails today (the in-memory Set is empty in drain 1, so the delete proceeds). + - RED (CR-05): invoke `runOutboxDrain` twice concurrently (do not await the first before starting the second) against the same pending row; assert the dispatch wrapper (createCalendarEvent) is invoked exactly once across both calls. + + + CR-04 — make the ordering durable. For a `delete` row that has a `groupId`, before dispatching, + query calendarOutbox for the sibling row with the same `groupId` and `operation='create'`: + - if that sibling is not yet `done`, SKIP this delete this cycle (leave it `pending` for a later + drain) — do not rely on `failedCreateGroups` co-occurring in the batch; + - if the sibling create is `failed`/`dead`, skip the delete permanently per D-04 (mark it `failed` + with a "paired create did not succeed" lastError so the original event is preserved). + Keep the within-batch create-before-delete sort as a fast path, but the DB sibling-status query is + the authoritative gate. Remove reliance on `failedCreateGroups` as the sole cross-cycle mechanism. + + CR-05 — add a module-level `let isDraining = false`. At the top of `runOutboxDrain`, if `isDraining` + return immediately; else set `isDraining = true` and wrap the whole drain body in try/finally that + resets `isDraining = false`. The 15s scheduler in `startOutboxWorker` already calls runOutboxDrain; + the guard makes an overlapping invocation a no-op. (Document that a more robust DB row-claim is a + future option, but the in-process guard is sufficient for the single-worker two-user deployment.) + + Commit RED then GREEN. + + + cd apps/api && npx vitest run tests/broker/outboxWorker.test.ts + + + - behavior: a delete whose paired create is not `done` is not dispatched, even when they arrive in different drain cycles. + - behavior: a paired create that ends failed/dead causes the delete to be skipped (original event preserved). + - behavior: two overlapping runOutboxDrain calls dispatch each row exactly once. + - source: `grep -c 'isDraining' apps/api/src/broker/outboxWorker.ts` returns >= 2. + - test-command: `cd apps/api && npx vitest run tests/broker/outboxWorker.test.ts` passes. + + The create-before-delete invariant holds across drain cycles and overlapping cycles never double-apply a row. + + + + Task 2: RED+GREEN — re-read freshest etag before PUT to avoid spurious 412 (WR-02) + apps/api/src/broker/outboxWorker.ts, apps/api/tests/broker/outboxWorker.test.ts + + - apps/api/src/broker/outboxWorker.ts (update dispatch lines 158-173; the etag comes from row.etag captured at enqueue time) + - apps/api/src/db/schema.ts (calendarEvents.etag line 95; calendarEvents.uid line 94) + - .planning/phases/03-event-write-back-pwa-install/03-REVIEW.md (WR-02) + + + - RED: an update row carries a stale `etag` ('old-etag'), but calendarEvents has been re-synced to 'new-etag'. Assert updateCalendarEvent is called with 'new-etag' (the freshest value read from calendarEvents at dispatch time), not the row's stale 'old-etag'. Fails today (row.etag is used verbatim). + + + In the `operation === 'update'` branch of `dispatchRow`, before calling `updateCalendarEvent`, + re-read the freshest etag for this object from `calendarEvents` (select `etag` where + `uid = row.uid`, taking the row whose calendar matches `row.calendarUrl` if needed). Use that + fresh etag for the If-Match instead of `row.etag` when present; fall back to `row.etag` if the + DB read returns nothing. This coalesces rapid successive same-uid edits against the latest + server state rather than the enqueue-time snapshot, preventing the guaranteed-412-on-second-edit + described in WR-02. Do NOT weaken conflict detection for genuine third-party changes — the fresh + etag still reflects the last synced server state, so a real external edit still 412s (D-08 intact). + + Commit RED then GREEN. + + + cd apps/api && npx vitest run tests/broker/outboxWorker.test.ts + + + - behavior: the update PUT uses the freshest calendarEvents.etag, not the stale enqueue-time etag. + - behavior: when calendarEvents has no row for the uid, the worker falls back to row.etag. + - source: the update branch reads calendarEvents.etag at dispatch time (grep for a select against calendarEvents inside the update path). + - test-command: `cd apps/api && npx vitest run tests/broker/outboxWorker.test.ts` passes. + + Rapid successive same-calendar edits no longer fire a spurious conflict toast; genuine external changes still 412 (D-08 preserved). + + + + + +- `cd apps/api && npx vitest run tests/broker/` green. +- `cd apps/api && npm run build` succeeds. + + + +The outbox is durable (create-before-delete across cycles), non-duplicating (concurrency guard), +and avoids spurious conflicts (fresh-etag). CR-04, CR-05, WR-02 closed. + + + +Create `.planning/phases/03-event-write-back-pwa-install/03-11-SUMMARY.md` when done. + diff --git a/.planning/phases/03-event-write-back-pwa-install/03-12-PLAN.md b/.planning/phases/03-event-write-back-pwa-install/03-12-PLAN.md new file mode 100644 index 0000000..f703d43 --- /dev/null +++ b/.planning/phases/03-event-write-back-pwa-install/03-12-PLAN.md @@ -0,0 +1,170 @@ +--- +phase: 03-event-write-back-pwa-install +plan: 12 +type: tdd +wave: 1 +depends_on: [] +gap_closure: true +autonomous: true +requirements: [CAL-05, CAL-07, PWA-01, PWA-02] +files_modified: + - apps/pwa/src/components/EventForm.tsx + - apps/pwa/src/components/EventForm.test.tsx +must_haves: + truths: + - "Opening the form in edit mode populates Title/Start/End from the cached occurrence even when the form opens before the occurrence is resolved (no blank edit form)" + - "Editing a recurring event preselects its existing recurrence preset instead of resetting to 'none'" + - "The edit form shows the event's original date/time consistently (no UTC-date / local-time mismatch that shifts the day)" + - "Tab and Shift+Tab cycle focus within the open dialog and never reach background controls" + - "The PWA install assets (icon-192/512, apple-touch-icon) exist so Add-to-Home-Screen installs with a real icon (PWA-01/PWA-02)" + artifacts: + - path: apps/pwa/src/components/EventForm.tsx + provides: "occurrence-driven reset, recurrence derivation, zone-consistent parseDateTime, real focus trap" + key_links: + - from: "EventForm reset effect" + to: "occurrence from TanStack cache" + via: "occurrence (or occurrence?.uid) in effect deps" + pattern: "occurrence" +--- + + +Fix the PWA edit form so editing actually works and the dialog is accessible. +Today the edit form can open blank (the reset effect ignores `occurrence`, which is +null if the events query has not resolved yet — WR-03), it hard-resets recurrence to +'none' so editing a recurring event silently drops its series (WR-03), it shows the +wrong day/time by mixing a UTC date with local-clock components (WR-05), and its +claimed focus trap only focuses once on open (WR-07). This plan closes the user-facing +half of the write path and carries the PWA install requirements (assets verified present). + +Purpose: edit mode pre-populates correctly and the dialog is keyboard-accessible. +Output: an EventForm that round-trips an existing event's fields and traps focus. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/STATE.md +@.planning/phases/03-event-write-back-pwa-install/03-REVIEW.md +@.planning/phases/03-event-write-back-pwa-install/03-UI-SPEC.md +@apps/pwa/src/components/EventForm.tsx +@apps/pwa/src/api/client.ts +@apps/pwa/src/store/calendarStore.ts + + + +No new exported symbols. Internal changes to EventForm: reset effect deps gain +`occurrence`, a recurrence-deriving initializer, a zone-consistent `parseDateTime`, +and a real Tab/Shift+Tab focus-cycle handler. IN-03 (collapse the duplicate +getDefaultStartDate/getDefaultEndDate to one helper) is folded in as a cheap adjacent cleanup. + + + + + + Task 1: RED+GREEN — edit-mode population, recurrence derivation, zone-consistent dates (WR-03, WR-05, IN-03) + apps/pwa/src/components/EventForm.tsx, apps/pwa/src/components/EventForm.test.tsx + + - apps/pwa/src/components/EventForm.tsx (occurrence IIFE lines 113-124; reset effect lines 167-181 with deps [eventFormOpen,eventFormMode,eventFormUid]; parseDateTime lines 84-101; getDefaultStartDate/EndDate lines 47-53) + - apps/pwa/src/api/client.ts (CalendarOccurrence.start/end format note lines 69-73: 'YYYY-MM-DD' for allDay, ISO 8601 with IANA tz for timed) + - apps/pwa/src/store/calendarStore.ts (setEventForm/eventFormMode/eventFormUid lines 43-44,162-163; a todayIso helper exists per IN-03) + - .planning/phases/03-event-write-back-pwa-install/03-REVIEW.md (WR-03, WR-05, IN-03) + + + - RED (WR-03 blank): render EventForm in edit mode where the occurrence becomes available in the ['events'] cache AFTER the form opens; assert the Title input value equals the occurrence title (not empty). Fails today because the reset effect deps exclude `occurrence`. + - RED (WR-03 recurrence): edit an occurrence whose recurrence is 'weekly'; assert the Repeat select value is 'weekly', not 'none'. + - RED (WR-05 zone): a timed occurrence start of '2026-06-10T23:30:00-04:00[America/Toronto]' renders Start date '2026-06-10' and time '23:30' (the event's own wall time), not a UTC-shifted '2026-06-11'/'03:30'. + + + WR-03: add `occurrence` (or `occurrence?.uid` plus `occurrence?.start`) to the reset effect dependency + array so the form re-initializes when the occurrence resolves after open. In the reset effect, derive + the initial recurrence from the occurrence instead of always `setRecurrence('none')` — if the + CalendarOccurrence carries a recurrence preset use it; if the occurrence shape does not expose one, + add the preset to the occurrence/expand contract is OUT OF SCOPE — instead read it from the cached + raw recurrence if present and default to 'none' only when genuinely absent (document the limitation in + a comment citing WR-03). Guard against opening edit mode before the cache is populated: keep fields + blank-safe but re-run on arrival. + + WR-05: rewrite `parseDateTime` so date and time are derived in ONE consistent frame. For a timed ISO + with an offset/IANA suffix, compute the local wall-clock components with `getFullYear/getMonth/getDate/ + getHours/getMinutes` together (or reuse the Temporal-based conversion the calendar render path already + uses) — never mix `toISOString().slice(0,10)` (UTC date) with `getHours()` (local time). The all-day + `^\d{4}-\d{2}-\d{2}$` branch is unchanged. + + IN-03: collapse `getDefaultStartDate`/`getDefaultEndDate` into one `todayIso()` helper (reuse the + existing one in calendarStore.ts if exported); keep the separate '09:00'/'10:00' default times at the + call sites. + + Commit RED then GREEN. + + + cd apps/pwa && npx vitest run src/components/EventForm.test.tsx + + + - behavior: edit form Title is populated even when occurrence resolves after open. + - behavior: editing a recurring event preselects its recurrence preset. + - behavior: a timed occurrence renders its own wall-clock date and time (no UTC/local day shift). + - source: the reset effect dependency array in EventForm.tsx includes occurrence (grep for occurrence in the deps line). + - test-command: `cd apps/pwa && npx vitest run src/components/EventForm.test.tsx` passes. + + Edit mode pre-populates correctly (fields, recurrence, correct zone); duplicate date helpers collapsed. + + + + Task 2: RED+GREEN — real focus trap on the dialog (WR-07) + verify PWA install assets (PWA-01/02, IN-04) + apps/pwa/src/components/EventForm.tsx, apps/pwa/src/components/EventForm.test.tsx + + - apps/pwa/src/components/EventForm.tsx (focus-on-open effect lines 274-278; dialog element lines 378-384; Escape handler lines 263-270) + - .planning/phases/03-event-write-back-pwa-install/03-REVIEW.md (WR-07, IN-04) + - .planning/phases/03-event-write-back-pwa-install/03-UI-SPEC.md (modal/focus interaction contract) + + + - RED: with the dialog open, dispatch a Tab keydown from the last focusable control; assert focus wraps to the first focusable control inside the dialog (not to background). Shift+Tab from the first wraps to the last. Fails today (only one .focus() on open; Tab escapes the modal). + + + WR-07: implement an actual focus trap on the role="dialog" element. On Tab/Shift+Tab keydown while + open: query the dialog's focusable elements (`button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])`), + and if focus is on the last element and Tab is pressed, move to the first (preventDefault); if on the + first and Shift+Tab, move to the last. Keep the existing focus-on-open behavior (Title input). Keep the + Escape-to-close handler. Do NOT introduce a new dependency — implement the trap inline (or extract a + small local hook). Update the docblock so the "Focus trap" claim is now accurate. + + IN-04 / PWA-01 / PWA-02: this gap does not change install code, but the requirement must be verified. + The assets `apps/pwa/public/icon-192.png`, `icon-512.png`, and `apple-touch-icon.png` exist (confirmed + present). Add a lightweight assertion (test or a checked note in the SUMMARY) that these three files + exist so the Add-to-Home-Screen flow installs with a real icon. No code change required if assets present. + + Commit RED then GREEN. + + + cd apps/pwa && npx vitest run src/components/EventForm.test.tsx + + + - behavior: Tab from the last focusable control wraps to the first inside the dialog; Shift+Tab from the first wraps to the last. + - behavior: focus never lands on a background control while the dialog is open. + - source: `ls apps/pwa/public/icon-192.png apps/pwa/public/icon-512.png apps/pwa/public/apple-touch-icon.png` all exist (PWA-01/PWA-02 install assets). + - test-command: `cd apps/pwa && npx vitest run src/components/EventForm.test.tsx` passes. + + The dialog traps Tab focus as its docblock claims; PWA install icon assets are confirmed present for Gate 2. + + + + + +- `cd apps/pwa && npx vitest run src/components/EventForm.test.tsx` green. +- `cd apps/pwa && npm run build` (tsc + vite) succeeds. +- Optional: drive the create→edit→delete flow with playwright-cli per CLAUDE.md to confirm end-to-end UX in a desktop browser. + + + +Edit mode pre-populates fields/recurrence in the correct zone, the dialog traps focus, +and the PWA install assets are confirmed present. WR-03, WR-05, WR-07, IN-03, IN-04 closed; +PWA-01/PWA-02 verified. + + + +Create `.planning/phases/03-event-write-back-pwa-install/03-12-SUMMARY.md` when done. +