Compare commits
10
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
ffc1f52f2b | ||
|
|
03f02a1207 | ||
|
|
551b751b81 | ||
|
|
fff5789a5f | ||
|
|
0ebdf4864d | ||
|
|
30eff3dcdb | ||
|
|
9164619ccd | ||
|
|
2b113045f7 | ||
|
|
bcde073729 | ||
|
|
1e12d702a1 |
@@ -36,6 +36,7 @@ Deferred to backlog: self-service provider onboarding (999.5) and provider abstr
|
||||
- [x] Shared collaborative lists (groceries, gift ideas) co-edited by both members, stored in MariaDB — **Validated in Phase 4 (shared-lists-live-sync)**: list + item CRUD, fractional-rank drag-reorder, member-scoped access (no cross-tenant leak proven at route layer).
|
||||
- [x] Live list sync so co-edits appear without manual refresh — **Validated in Phase 4**: scoped SSE fan-out over Pangolin (transport smoke-tested), bounded-backoff reconnect, co-edits land within seconds.
|
||||
- [x] Web Push notifications for event reminders and list changes — **Validated in Phase 5 (web-push-notifications)**: VAPID push for reminders, event-change, and coalesced list alerts; on-device UAT 1/2/5 PASS (iOS reminder delivery, iOS push, coalescing). Android event-change on-device confirmation + iOS standalone spinner remain device-only spot-checks at go-live.
|
||||
- [x] Faster write-back so edits reach Fastmail in ~1–2s instead of ~15s (CAL-15) — **Validated in Phase 9 (faster-write-back)**: event-driven outbox drain via a zero-dependency in-process EventEmitter (`outboxTrigger.ts`); a committed enqueue publishes a fire-and-forget `signalOutboxDrain()` that funnels through the existing `isDraining`-guarded drain with a `drainRequested` trailing-re-drain, preserving optimistic-202, create-before-delete on moves, exactly-once per uid, and the 15s `setInterval` fallback. 5/5 success criteria verified; trigger-wiring tests assert SC-1/D-05/D-07.
|
||||
|
||||
### Active
|
||||
|
||||
@@ -119,4 +120,4 @@ This document evolves at phase transitions and milestone boundaries.
|
||||
|
||||
---
|
||||
|
||||
_Last updated: 2026-06-11 — Phase 7 (Mobile Test Harness) complete; TEST-01/TEST-02 validated_
|
||||
_Last updated: 2026-06-12 — Phase 9 (Faster Write-Back) complete; CAL-15 validated_
|
||||
|
||||
@@ -14,7 +14,7 @@ Each requirement maps to exactly one roadmap phase (see Traceability).
|
||||
|
||||
- [ ] **CAL-13**: User can choose a reminder lead time when creating or editing an event from a preset list (None / 5m / 10m / 15m / 30m / 1h / 2h / 1d / 2d), with **"None" as the default**; the choice is serialized as a VALARM on the event written back to Fastmail.
|
||||
- [ ] **CAL-14**: Editing an event **preserves any existing reminder/VALARM** set in another client (Fastmail or native) — reminders are never silently stripped on round-trip.
|
||||
- [ ] **CAL-15**: A created, edited, or deleted event reaches Fastmail within ~2 seconds (event-driven outbox drain) instead of up to ~15s, while preserving the optimistic-202 accept and all outbox durability guarantees (create-before-delete ordering, drain concurrency guard, fresh-etag-before-PUT, per-uid exactly-once).
|
||||
- [x] **CAL-15**: A created, edited, or deleted event reaches Fastmail within ~2 seconds (event-driven outbox drain) instead of up to ~15s, while preserving the optimistic-202 accept and all outbox durability guarantees (create-before-delete ordering, drain concurrency guard, fresh-etag-before-PUT, per-uid exactly-once).
|
||||
|
||||
### Notifications — Variable-lead reminder scheduling
|
||||
|
||||
@@ -75,7 +75,7 @@ Maps each REQ-ID to its phase. v1.1 phases continue v1.0 numbering (v1.0 ended a
|
||||
| TEST-02 | Phase 7 (Mobile Test Harness) | Complete |
|
||||
| CI-01 | Phase 8 (Gitea CI) | Complete |
|
||||
| CI-02 | Phase 8 (Gitea CI) | Complete |
|
||||
| CAL-15 | Phase 9 (Faster Write-Back) | Pending |
|
||||
| CAL-15 | Phase 9 (Faster Write-Back) | Complete |
|
||||
| ADMIN-01 | Phase 10 (Admin Role & Settings) | Pending |
|
||||
| ADMIN-02 | Phase 10 (Admin Role & Settings) | Pending |
|
||||
| ADMIN-03 | Phase 10 (Admin Role & Settings) | Pending |
|
||||
|
||||
@@ -27,7 +27,7 @@ Make FamilySync configurable, administrable, and maintainable for real multi-mem
|
||||
|
||||
- [x] **Phase 7: Mobile Test Harness** - Mobile-emulated, authenticated PWA browser harness so the assistant (and CI) can catch mobile-only defects (completed 2026-06-11)
|
||||
- [x] **Phase 8: Gitea CI** - Full regression on PR to main (lint/typecheck/unit/API-integration vs a MariaDB service container **+ the Phase 7 mobile harness as a UI-regression step against a CI-hosted dev stack**) + Docker image publish on merge (completed 2026-06-11)
|
||||
- [ ] **Phase 9: Faster Write-Back** - Event-driven outbox drain so edits land in ~1-2s instead of ~15s, preserving every outbox durability guarantee
|
||||
- [x] **Phase 9: Faster Write-Back** - Event-driven outbox drain so edits land in ~1-2s instead of ~15s, preserving every outbox durability guarantee (completed 2026-06-12)
|
||||
- [ ] **Phase 10: Admin Role & Settings** - DB foundation (is_admin / reminder_lead / app_config) + role-gated admin UI to rotate app passwords and designate the shared calendar
|
||||
- [ ] **Phase 11: Per-Event Reminders** - Reminder selector on the event form (incl. "None") serialized as VALARM, with a variable-lead scheduler that honors each event's choice
|
||||
- [ ] **Phase 12: Initial Setup Wizard** - First-run validated bootstrap of env/VAPID/DB/OIDC + first app password, reusing the admin route surface
|
||||
@@ -141,11 +141,11 @@ Make FamilySync configurable, administrable, and maintainable for real multi-mem
|
||||
Plans:
|
||||
**Wave 1**
|
||||
|
||||
- [ ] 09-01-PLAN.md — TDD: outboxTrigger.ts (zero-dep EventEmitter signal) + scheduleOutboxDrain wrapper / drainRequested trailing-re-drain loop + initOutboxTrigger in outboxWorker.ts; trigger-wiring tests (SC-1, SC-4/D-07, D-05) (Wave 1)
|
||||
- [x] 09-01-PLAN.md — TDD: outboxTrigger.ts (zero-dep EventEmitter signal) + scheduleOutboxDrain wrapper / drainRequested trailing-re-drain loop + initOutboxTrigger in outboxWorker.ts; trigger-wiring tests (SC-1, SC-4/D-07, D-05) (Wave 1)
|
||||
|
||||
**Wave 2** *(blocked on Wave 1 completion)*
|
||||
|
||||
- [ ] 09-02-PLAN.md — Four post-commit signalOutboxDrain() publish sites in events.ts (create / edit-as-move-after-transaction / same-cal update / delete) + initOutboxTrigger() startup wiring under isMainModule() in index.ts (Wave 2)
|
||||
- [x] 09-02-PLAN.md — Four post-commit signalOutboxDrain() publish sites in events.ts (create / edit-as-move-after-transaction / same-cal update / delete) + initOutboxTrigger() startup wiring under isMainModule() in index.ts (Wave 2)
|
||||
|
||||
### Phase 10: Admin Role & Settings
|
||||
|
||||
@@ -317,7 +317,7 @@ Plans:
|
||||
| 6. UX Polish | v1.0 | 6/6 | Complete | 2026-06-10 |
|
||||
| 7. Mobile Test Harness | v1.1 | 4/4 | Complete | 2026-06-11 |
|
||||
| 8. Gitea CI | v1.1 | 4/4 | Complete | 2026-06-11 |
|
||||
| 9. Faster Write-Back | v1.1 | 0/2 | Not started | - |
|
||||
| 9. Faster Write-Back | v1.1 | 2/2 | Complete | 2026-06-12 |
|
||||
| 10. Admin Role & Settings | v1.1 | 0/? | Not started | - |
|
||||
| 11. Per-Event Reminders | v1.1 | 0/? | Not started | - |
|
||||
| 12. Initial Setup Wizard | v1.1 | 0/? | Not started | - |
|
||||
@@ -331,7 +331,7 @@ Plans:
|
||||
|
||||
**Goal:** [Captured for future planning] Abstract the calendar backend behind a provider interface so Fastmail/CalDAV is one implementation among potentially many. Shipping with a single provider is fine, but the broker, sync, and event-expansion layers should be structured so additional providers (e.g. other CalDAV hosts, Google Calendar, generic ICS feeds) can be added without rework. Captures the "provider" seam as an explicit architectural concern.
|
||||
**Requirements:** TBD
|
||||
**Plans:** 3/3 plans complete
|
||||
**Plans:** 2/2 plans complete
|
||||
|
||||
Plans:
|
||||
|
||||
|
||||
+15
-13
@@ -2,16 +2,16 @@
|
||||
gsd_state_version: 1.0
|
||||
milestone: v1.1
|
||||
milestone_name: Operability & Polish
|
||||
status: completed
|
||||
status: executing
|
||||
stopped_at: Phase 9 context gathered
|
||||
last_updated: "2026-06-12T18:28:01.547Z"
|
||||
last_activity: 2026-06-12 -- Phase 09 planning complete
|
||||
last_updated: "2026-06-12T21:08:34.135Z"
|
||||
last_activity: 2026-06-12
|
||||
progress:
|
||||
total_phases: 18
|
||||
completed_phases: 5
|
||||
total_plans: 15
|
||||
completed_plans: 15
|
||||
percent: 28
|
||||
completed_phases: 6
|
||||
total_plans: 17
|
||||
completed_plans: 17
|
||||
percent: 33
|
||||
---
|
||||
|
||||
# Project State
|
||||
@@ -21,14 +21,14 @@ progress:
|
||||
See: .planning/PROJECT.md (updated 2026-06-10)
|
||||
|
||||
**Core value:** One color-coded family calendar (shared + personal) and shared lists from a single low-friction PWA — cross-ecosystem, no app store
|
||||
**Current focus:** Phase 15 — ci-skip-api-harness-jobs-for-doc-only-prs
|
||||
**Current focus:** Phase 09 — faster-write-back
|
||||
|
||||
## Current Position
|
||||
|
||||
Phase: 999.1
|
||||
Phase: 13
|
||||
Plan: Not started
|
||||
Status: Plans 15-01 + 15-02 complete; 15-03 Task 1 (publish.yml comment) committed (da623ac); 15-03 Task 2 awaiting operator after branch merges to main
|
||||
Last activity: 2026-06-12 -- Phase 09 planning complete
|
||||
Status: Ready to execute
|
||||
Last activity: 2026-06-12
|
||||
|
||||
### Deferred Checkpoint — Phase 15 Plan 15-03 Task 2 (human-action)
|
||||
|
||||
@@ -47,7 +47,7 @@ Resume: after the operator completes the change, re-run `/gsd-execute-phase 15`
|
||||
|
||||
**Velocity:**
|
||||
|
||||
- Total plans completed: 31
|
||||
- Total plans completed: 33
|
||||
- Average duration: -
|
||||
- Total execution time: 0 hours
|
||||
|
||||
@@ -61,6 +61,7 @@ Resume: after the operator completes the change, re-run `/gsd-execute-phase 15`
|
||||
| 13 | 3 | - | - |
|
||||
| 14 | 1 | - | - |
|
||||
| 15 | 3 | - | - |
|
||||
| 09 | 2 | - | - |
|
||||
|
||||
**Recent Trend:**
|
||||
|
||||
@@ -101,6 +102,7 @@ _Updated after each plan completion_
|
||||
| Phase 13 P01 | 8 | 2 tasks | 7 files |
|
||||
| Phase 13-real-lint-gate-eslint P02 | 90 | 2 tasks | 31 files |
|
||||
| Phase 13-real-lint-gate-eslint P03 | 10 | 3 tasks | 399 files |
|
||||
| Phase 09-faster-write-back P01 | 341 | 3 tasks | 3 files |
|
||||
|
||||
## Accumulated Context
|
||||
|
||||
@@ -224,7 +226,7 @@ Recent decisions affecting current work:
|
||||
|
||||
## Session Continuity
|
||||
|
||||
Last session: 2026-06-12T18:02:28.153Z
|
||||
Last session: 2026-06-12T20:54:10.362Z
|
||||
Stopped at: Phase 9 context gathered
|
||||
Resume file: .planning/phases/09-faster-write-back/09-CONTEXT.md
|
||||
|
||||
|
||||
@@ -0,0 +1,125 @@
|
||||
---
|
||||
phase: 09-faster-write-back
|
||||
plan: "01"
|
||||
subsystem: api-broker
|
||||
tags: [outbox, event-driven, tdd, drain-trigger, caldav]
|
||||
dependency_graph:
|
||||
requires: []
|
||||
provides:
|
||||
- signalOutboxDrain (apps/api/src/lib/outboxTrigger.ts)
|
||||
- onOutboxDrain (apps/api/src/lib/outboxTrigger.ts)
|
||||
- scheduleOutboxDrain (apps/api/src/broker/outboxWorker.ts)
|
||||
- drainRequested (apps/api/src/broker/outboxWorker.ts)
|
||||
- initOutboxTrigger (apps/api/src/broker/outboxWorker.ts)
|
||||
affects:
|
||||
- apps/api/src/broker/outboxWorker.ts
|
||||
- apps/api/tests/broker/outboxWorker.test.ts
|
||||
tech_stack:
|
||||
added:
|
||||
- node:events EventEmitter (outboxTrigger.ts — zero external dependency)
|
||||
patterns:
|
||||
- Module-level EventEmitter singleton (same as listEmitter.ts)
|
||||
- scheduleOutboxDrain isDraining guard + drainRequested trailing-re-drain loop (D-05)
|
||||
- TDD RED/GREEN via test(09-01)/feat(09-01) commits
|
||||
key_files:
|
||||
created:
|
||||
- apps/api/src/lib/outboxTrigger.ts
|
||||
modified:
|
||||
- apps/api/src/broker/outboxWorker.ts
|
||||
- apps/api/tests/broker/outboxWorker.test.ts
|
||||
decisions:
|
||||
- "D-TDD-INIT: initOutboxTrigger() called in beforeAll (not beforeEach) for the trigger-wiring describe block — avoids listener accumulation while still wiring the EventEmitter → scheduleOutboxDrain path for signal-driven tests (SC-1, D-05)"
|
||||
- "D-TEST-C-MOCK: Test C (D-07) uses mockImplementationOnce to return the row on the first pending-rows query and empty on subsequent queries — correctly simulates drain 1 processing the row so the trailing re-drain (triggered by drainRequested) finds 0 rows and calls createCalendarEvent exactly once"
|
||||
metrics:
|
||||
duration_seconds: 341
|
||||
completed_date: "2026-06-12"
|
||||
tasks_completed: 3
|
||||
files_changed: 3
|
||||
---
|
||||
|
||||
# Phase 09 Plan 01: Outbox Drain Trigger Wiring Summary
|
||||
|
||||
**One-liner:** Zero-dependency in-process EventEmitter drain signal (`outboxTrigger.ts`) + `scheduleOutboxDrain` wrapper with `drainRequested` trailing-re-drain loop wired to `outboxWorker.ts`, eliminating the up-to-15s polling delay on enqueue.
|
||||
|
||||
## What Was Built
|
||||
|
||||
### Task 1: `apps/api/src/lib/outboxTrigger.ts` (new file)
|
||||
|
||||
Module-level `EventEmitter` singleton following the `listEmitter.ts` analog:
|
||||
- `signalOutboxDrain(): void` — `emitter.emit('drain')`, fire-and-forget (D-04)
|
||||
- `onOutboxDrain(handler: () => void): () => void` — registers listener, returns unsubscribe
|
||||
- Only imports `node:events`; no internal dependencies (zero circular-import risk)
|
||||
- No `setMaxListeners` call — single subscriber, default limit of 10 is correct
|
||||
|
||||
### Task 2: RED test block in `apps/api/tests/broker/outboxWorker.test.ts`
|
||||
|
||||
Added `describe('scheduleOutboxDrain — trigger wiring (D-09)', ...)` with 3 failing tests:
|
||||
- Test A (SC-1): `signalOutboxDrain()` triggers drain promptly without timer advance
|
||||
- Test B (D-05): two mid-drain signals collapse to exactly one trailing re-drain
|
||||
- Test C (D-07): two concurrent `scheduleOutboxDrain()` calls invoke `createCalendarEvent` exactly once
|
||||
|
||||
Tests failed RED because `scheduleOutboxDrain` and `initOutboxTrigger` were not yet exported.
|
||||
|
||||
### Task 3: GREEN implementation in `apps/api/src/broker/outboxWorker.ts`
|
||||
|
||||
Four additions to `outboxWorker.ts`:
|
||||
1. `import { onOutboxDrain } from '../lib/outboxTrigger.js'`
|
||||
2. `let drainRequested = false` — trailing-re-drain flag (D-05), immediately after `isDraining`
|
||||
3. `export function scheduleOutboxDrain(): void` — checks `isDraining`; if true sets `drainRequested = true` and returns; otherwise calls `runOutboxDrain()` with `.catch` (D-02) and `.finally` that resets `drainRequested = false` BEFORE any recursive `scheduleOutboxDrain()` call (Pitfall 3 / T-09-01)
|
||||
4. `export function initOutboxTrigger(): void` — calls `onOutboxDrain(() => scheduleOutboxDrain())`
|
||||
5. `startOutboxWorker` setInterval body changed from `runOutboxDrain().catch(...)` to bare `scheduleOutboxDrain()` — 15s interval unchanged (D-08)
|
||||
|
||||
`runOutboxDrain`'s body, `if (isDraining) return;`, `isDraining = true;`, and `finally { isDraining = false; }` are byte-for-byte unchanged.
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
### Auto-fixed Issues
|
||||
|
||||
**1. [Rule 1 - Test Design] Test C mock required mockImplementationOnce to correctly simulate drain-1 row consumption**
|
||||
|
||||
- **Found during:** Task 3 GREEN
|
||||
- **Issue:** Test C expects `createCalendarEvent` called exactly once, but the trailing re-drain triggered by `drainRequested` also executed against the same always-returning mock, calling `createCalendarEvent` twice.
|
||||
- **Fix:** Used `mockWherePending.mockImplementationOnce(() => Promise.resolve([row])).mockImplementation(() => Promise.resolve([]))` so drain 1 gets the row and the trailing drain finds empty.
|
||||
- **Files modified:** `apps/api/tests/broker/outboxWorker.test.ts`
|
||||
- **Commit:** `2b11304`
|
||||
|
||||
**2. [Rule 2 - Missing test wiring] `beforeAll(initOutboxTrigger)` required to wire EventEmitter listener for signal-driven tests**
|
||||
|
||||
- **Found during:** Task 3 GREEN (Tests A and B failed because no listener was registered)
|
||||
- **Issue:** Tests A and B call `signalOutboxDrain()` but without `initOutboxTrigger()` registering the listener, the signal went nowhere.
|
||||
- **Fix:** Added `beforeAll(() => { initOutboxTrigger(); })` to the new describe block; also imported `initOutboxTrigger` and changed `import { beforeEach }` to include `beforeAll`.
|
||||
- **Files modified:** `apps/api/tests/broker/outboxWorker.test.ts`
|
||||
- **Commit:** `2b11304`
|
||||
|
||||
## TDD Gate Compliance
|
||||
|
||||
- RED commit: `bcde073` — `test(09-01): add failing trigger-wiring tests for SC-1, D-05, D-07`
|
||||
- GREEN commit: `2b11304` — `feat(09-01): add scheduleOutboxDrain, drainRequested, initOutboxTrigger; route setInterval through wrapper`
|
||||
- RED gate: 3 new tests failing (27 pre-existing passing)
|
||||
- GREEN gate: 30/30 tests passing, `tsc --noEmit` clean
|
||||
|
||||
## Verification Evidence
|
||||
|
||||
```
|
||||
npx vitest run tests/broker/outboxWorker.test.ts
|
||||
Test Files 1 passed (1)
|
||||
Tests 30 passed (30)
|
||||
|
||||
npx tsc --noEmit → (no output, clean)
|
||||
```
|
||||
|
||||
## Known Stubs
|
||||
|
||||
None — all symbols produce correct runtime behavior. `signalOutboxDrain` is not yet wired to the enqueue path (Plan 02 adds it to `events.ts`); `initOutboxTrigger` is not yet called at startup (Plan 02 adds it to `index.ts`). These are intentional plan boundaries, not stubs.
|
||||
|
||||
## Threat Flags
|
||||
|
||||
None — this plan introduces no new network endpoints, auth paths, file access patterns, or schema changes. The in-process EventEmitter boundary carries no payload and no user input crosses it. STRIDE mitigations T-09-01 through T-09-04 are implemented and verified by the trigger-wiring tests.
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
- `apps/api/src/lib/outboxTrigger.ts` — FOUND
|
||||
- `apps/api/src/broker/outboxWorker.ts` — verified: `scheduleOutboxDrain`, `initOutboxTrigger`, `drainRequested` present
|
||||
- Task 1 commit `1e12d70` — FOUND
|
||||
- Task 2 commit `bcde073` — FOUND
|
||||
- Task 3 commit `2b11304` — FOUND
|
||||
@@ -0,0 +1,100 @@
|
||||
---
|
||||
phase: 09-faster-write-back
|
||||
plan: "02"
|
||||
subsystem: api-routes-startup
|
||||
tags: [outbox, event-driven, drain-signal, caldav, startup-wiring]
|
||||
dependency_graph:
|
||||
requires:
|
||||
- signalOutboxDrain (apps/api/src/lib/outboxTrigger.ts — Plan 01)
|
||||
- initOutboxTrigger (apps/api/src/broker/outboxWorker.ts — Plan 01)
|
||||
provides:
|
||||
- Four post-commit signalOutboxDrain() publish sites in events.ts (create, edit-as-move, same-cal update, delete)
|
||||
- initOutboxTrigger() wired at startup in index.ts under isMainModule()
|
||||
affects:
|
||||
- apps/api/src/routes/events.ts
|
||||
- apps/api/src/index.ts
|
||||
tech_stack:
|
||||
added: []
|
||||
patterns:
|
||||
- Fire-and-forget in-process EventEmitter signal after each outbox enqueue commit (D-04)
|
||||
- isMainModule() guard for startup-only listener registration (Pitfall 4)
|
||||
- edit-as-move signal fires after await db.transaction() resolves, not inside callback (D-03)
|
||||
key_files:
|
||||
created: []
|
||||
modified:
|
||||
- apps/api/src/routes/events.ts
|
||||
- apps/api/src/index.ts
|
||||
decisions:
|
||||
- "D-03-signal-placement: signalOutboxDrain() for edit-as-move placed AFTER await db.transaction() resolves (line after the statement), never inside the async callback — guarantees DELETE+CREATE rows are durably committed before any drain observes them (SC-3 / Pitfall 1)"
|
||||
- "D-04-fire-and-forget: signalOutboxDrain() is never awaited — synchronous emit to in-process EventEmitter, cannot block the 202 response (SC-2 / T-09-07)"
|
||||
- "Pitfall4-guard: initOutboxTrigger() called only inside isMainModule() block — tests importing app never register the drain listener, no open handles in test process (T-09-06)"
|
||||
metrics:
|
||||
duration_seconds: 240
|
||||
completed_date: "2026-06-12"
|
||||
tasks_completed: 2
|
||||
files_changed: 2
|
||||
---
|
||||
|
||||
# Phase 09 Plan 02: Enqueue-Site Signal Wiring Summary
|
||||
|
||||
**One-liner:** Four fire-and-forget `signalOutboxDrain()` publish sites added to events.ts (create, edit-as-move-after-transaction, same-cal update, delete) and `initOutboxTrigger()` wired at startup in index.ts under `isMainModule()`, connecting the Plan 01 EventEmitter mechanism to live writes and eliminating the up-to-15s polling delay.
|
||||
|
||||
## What Was Built
|
||||
|
||||
### Task 1: `apps/api/src/routes/events.ts` — Four publish sites
|
||||
|
||||
Import added: `import { signalOutboxDrain } from '../lib/outboxTrigger.js';`
|
||||
|
||||
Four `signalOutboxDrain()` calls inserted, each fire-and-forget (never awaited):
|
||||
|
||||
| Site | Route | Placement |
|
||||
|------|-------|-----------|
|
||||
| 1 | POST /create | After `await db.insert(calendarOutbox).values({...})`, before `return c.json({ uid }, 202)` |
|
||||
| 2 | PATCH /:uid/edit (edit-as-move) | After `await db.transaction(async (tx) => {...})` RESOLVES, before `return c.json({ uid: newUid }, 202)` — critical: NOT inside the callback |
|
||||
| 3 | PATCH /:uid/edit (same-cal update) | After `await db.insert(calendarOutbox).values({ operation: 'update', ... })`, before `return c.json({ uid }, 202)` |
|
||||
| 4 | DELETE /:uid | After `await db.insert(calendarOutbox).values({ operation: 'delete', ... })`, before `return c.json({ uid }, 202)` |
|
||||
|
||||
No route status codes, response bodies, validation, or ownership checks were modified. No inline CalDAV calls added. No signal inside the `db.transaction` callback. No signal inside any `catch` block.
|
||||
|
||||
### Task 2: `apps/api/src/index.ts` — Startup wiring
|
||||
|
||||
- Import extended: `import { startOutboxWorker, initOutboxTrigger } from './broker/outboxWorker.js';`
|
||||
- `initOutboxTrigger();` added immediately after `startOutboxWorker();` inside the `if (isMainModule())` block (line 139), with comment `// subscribe drain signal listener (D-01)`
|
||||
- `isMainModule()`, `startBrokerPoller`, `startReminderScheduler`, and `serve(...)` unchanged
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
None — plan executed exactly as written.
|
||||
|
||||
## Verification Evidence
|
||||
|
||||
```
|
||||
grep -c "signalOutboxDrain()" apps/api/src/routes/events.ts → 4
|
||||
grep -n "await signalOutboxDrain" apps/api/src/routes/events.ts → (none)
|
||||
initOutboxTrigger() at line 139 — inside isMainModule() guard
|
||||
|
||||
npx tsc --noEmit → (no output, clean)
|
||||
|
||||
npx vitest run tests/routes/events.test.ts tests/broker/outboxWorker.test.ts
|
||||
Test Files 2 passed (2)
|
||||
Tests 53 passed (53)
|
||||
|
||||
Full suite: 240 passed, 1 failed (known pre-existing flaky timeout in lists.test.ts:
|
||||
"toggling isShared false→true re-populates list_shares for other members" — 5028ms,
|
||||
exceeds 5000ms global testTimeout; passes with --testTimeout=30000; unrelated to this plan)
|
||||
```
|
||||
|
||||
## Known Stubs
|
||||
|
||||
None.
|
||||
|
||||
## Threat Flags
|
||||
|
||||
None — this plan introduces no new network endpoints, auth paths, file access patterns, or schema changes. T-09-05 (edit-as-move signal placement) and T-09-06 (listener registration gating) are both verified: signal is after the transaction resolves, and `initOutboxTrigger` is inside `isMainModule()`.
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
- `apps/api/src/routes/events.ts` — FOUND; 4 signalOutboxDrain() calls verified
|
||||
- `apps/api/src/index.ts` — FOUND; initOutboxTrigger() inside isMainModule() at line 139 verified
|
||||
- Task 1 commit `30eff3d` — FOUND
|
||||
- Task 2 commit `0ebdf48` — FOUND
|
||||
@@ -0,0 +1,186 @@
|
||||
---
|
||||
phase: 09-faster-write-back
|
||||
reviewed: 2026-06-12T17:05:00Z
|
||||
depth: standard
|
||||
files_reviewed: 5
|
||||
files_reviewed_list:
|
||||
- apps/api/src/lib/outboxTrigger.ts
|
||||
- apps/api/src/broker/outboxWorker.ts
|
||||
- apps/api/src/routes/events.ts
|
||||
- apps/api/src/index.ts
|
||||
- apps/api/tests/broker/outboxWorker.test.ts
|
||||
findings:
|
||||
critical: 0
|
||||
warning: 4
|
||||
info: 3
|
||||
total: 7
|
||||
status: issues_found
|
||||
---
|
||||
|
||||
# Phase 9: Code Review Report
|
||||
|
||||
**Reviewed:** 2026-06-12T17:05:00Z
|
||||
**Depth:** standard
|
||||
**Files Reviewed:** 5
|
||||
**Status:** issues_found
|
||||
|
||||
## Summary
|
||||
|
||||
Reviewed the event-driven outbox drain trigger added in phase 09 (CAL-15): a zero-dependency
|
||||
in-process `EventEmitter` (`outboxTrigger.ts`), the `scheduleOutboxDrain` wrapper with the
|
||||
`isDraining` guard + `drainRequested` trailing-re-drain loop in `outboxWorker.ts`, four
|
||||
fire-and-forget `signalOutboxDrain()` publish sites in `events.ts`, and `initOutboxTrigger()`
|
||||
startup wiring under `isMainModule()` in `index.ts`.
|
||||
|
||||
The core concurrency design is sound on the four points called out in the brief:
|
||||
|
||||
- **Trailing-re-drain is bounded.** `drainRequested` is reset to `false` BEFORE the recursive
|
||||
`scheduleOutboxDrain()` call (outboxWorker.ts:199-200), so a signal arriving during the
|
||||
trailing drain produces at most one further pass — no unbounded re-drain chain. A signal that
|
||||
arrives mid-drain sets `drainRequested=true` and is re-SELECTed by the trailing pass; the
|
||||
publish sites fire `signalOutboxDrain()` only AFTER the row's DB insert commits, so the row is
|
||||
always visible to the re-drain. No lost-wakeup.
|
||||
- **Signal never fires inside the `db.transaction` callback.** All four publish sites (events.ts
|
||||
310, 434, 450, 525) call `signalOutboxDrain()` after the commit resolves and outside the
|
||||
transaction closure; D-03 ordering holds.
|
||||
- **Fire-and-forget is rejection-safe.** `signalOutboxDrain` is synchronous (`emitter.emit`); the
|
||||
sole listener delegates to `scheduleOutboxDrain`, which is `void`-returning with a `.catch()`
|
||||
attached to the `runOutboxDrain()` promise. `runOutboxDrain` is `async`, so it can never throw
|
||||
synchronously into `emit`/the route handler — no floating promise, no unhandled rejection.
|
||||
- **Tests pass (30/30) and the trigger-wiring suite exercises SC-1, D-05 collapse, and the
|
||||
concurrency guard.**
|
||||
|
||||
Remaining findings are robustness/maintainability (WARNING) and clarity (INFO). No BLOCKERs.
|
||||
|
||||
## Warnings
|
||||
|
||||
### WR-01: `initOutboxTrigger()` is non-idempotent and leaks its listener
|
||||
|
||||
**File:** `apps/api/src/broker/outboxWorker.ts:844-846`
|
||||
**Issue:** `initOutboxTrigger` calls `onOutboxDrain(...)` and discards the returned unsubscribe
|
||||
function, and has no guard against repeated registration. Each invocation adds another `'drain'`
|
||||
listener to the module-singleton emitter. Production calls it once under `isMainModule()`, so the
|
||||
hot path is fine — but the function is exported, has no internal idempotency, and the default
|
||||
`EventEmitter` max-listeners is 10 (the module comment in `outboxTrigger.ts:11` explicitly relies
|
||||
on never calling `setMaxListeners`). A second `initOutboxTrigger()` call (a future second wiring
|
||||
site, a hot-reload, or a test that re-inits) silently double-drains on every signal and, after 10
|
||||
registrations, emits a `MaxListenersExceededWarning`. The leak is masked today only because there
|
||||
is exactly one caller.
|
||||
**Fix:** Make registration idempotent and retain the unsubscribe handle:
|
||||
```ts
|
||||
let unsubscribeDrain: (() => void) | null = null;
|
||||
export function initOutboxTrigger(): void {
|
||||
if (unsubscribeDrain) return; // already wired — no double-registration
|
||||
unsubscribeDrain = onOutboxDrain(() => scheduleOutboxDrain());
|
||||
}
|
||||
```
|
||||
|
||||
### WR-02: Trigger-wiring tests register a listener in `beforeAll` and never remove it
|
||||
|
||||
**File:** `apps/api/tests/broker/outboxWorker.test.ts:751-753`
|
||||
**Issue:** The `scheduleOutboxDrain — trigger wiring` block calls `initOutboxTrigger()` in
|
||||
`beforeAll`, registering a permanent `'drain'` listener on the module-singleton emitter that is
|
||||
never torn down (no `afterAll`/`afterEach` unsubscribe). Because the emitter is a module
|
||||
singleton shared across the whole file (and any future file importing it), any later
|
||||
`signalOutboxDrain()` — in a subsequent describe block, a newly added test, or another test file
|
||||
that imports `signalOutboxDrain` — will fire the still-registered `scheduleOutboxDrain` listener
|
||||
and kick off a real `runOutboxDrain()` against the shared `mockPendingRows`/db mock. That is
|
||||
exactly the cross-test state pollution the WR-04/T-09-02 comments warn about. It is latent now
|
||||
only because the polluting describe block happens to run last and no other test calls
|
||||
`signalOutboxDrain`; test execution order is not a contract.
|
||||
**Fix:** Capture and release the listener (requires WR-01's handle, or expose a teardown). Minimal
|
||||
test-only fix:
|
||||
```ts
|
||||
describe('scheduleOutboxDrain — trigger wiring (D-09)', () => {
|
||||
let unsub: () => void;
|
||||
beforeAll(() => {
|
||||
// import onOutboxDrain directly so the test owns the unsubscribe
|
||||
unsub = onOutboxDrain(() => scheduleOutboxDrain());
|
||||
});
|
||||
afterAll(() => unsub());
|
||||
// ...
|
||||
});
|
||||
```
|
||||
or, once WR-01 lands, add an `afterAll` that calls a new `shutdownOutboxTrigger()` exported for
|
||||
tests.
|
||||
|
||||
### WR-03: `setInterval` handle is never retained — interval cannot be cleared and keeps the loop alive
|
||||
|
||||
**File:** `apps/api/src/broker/outboxWorker.ts:856-860`
|
||||
**Issue:** `startOutboxWorker` calls `setInterval(...)` but discards the returned `Timeout`. There
|
||||
is no way to stop the drain schedule (graceful shutdown, SIGTERM handler, or test teardown), and
|
||||
the unref-less interval keeps the event loop alive. This pre-dates phase 09 but the phase rewired
|
||||
the interval body (`runOutboxDrain` → `scheduleOutboxDrain`), so it is in scope. On a clean
|
||||
shutdown the process cannot drain-and-exit; a drain may be mid-flight when the process is killed.
|
||||
**Fix:** Retain the handle and expose a stop hook (and/or `.unref()` if the interval should not by
|
||||
itself hold the process open):
|
||||
```ts
|
||||
let drainInterval: ReturnType<typeof setInterval> | null = null;
|
||||
export function startOutboxWorker(): void {
|
||||
drainInterval = setInterval(() => scheduleOutboxDrain(), 15 * 1000);
|
||||
}
|
||||
export function stopOutboxWorker(): void {
|
||||
if (drainInterval) { clearInterval(drainInterval); drainInterval = null; }
|
||||
}
|
||||
```
|
||||
|
||||
### WR-04: Module-level `isDraining`/`drainRequested` shared by event-path and interval-path defeats unit isolation
|
||||
|
||||
**File:** `apps/api/src/broker/outboxWorker.ts:162, 172`
|
||||
**Issue:** Both guard flags are module-level mutable singletons. They are correct for the
|
||||
single-process runtime, but they persist across vitest test cases within the file and are not
|
||||
reset by `vi.resetAllMocks()`. If a test leaves `isDraining=true` or `drainRequested=true` — e.g.
|
||||
a `runOutboxDrain()` that throws synchronously before the `try` (none today) or a future test that
|
||||
abandons an in-flight `scheduleOutboxDrain` without flushing its `.finally()` — the next test
|
||||
silently no-ops its drain (`if (isDraining) return`) and the failure surfaces as a confusing
|
||||
"createCalendarEvent not called" assertion far from the cause. The CR-05/D-07 concurrency tests
|
||||
already depend on these flags being clean at entry but nothing guarantees it.
|
||||
**Fix:** Export a test-only reset (or reset in `beforeEach`):
|
||||
```ts
|
||||
export function __resetDrainStateForTests(): void {
|
||||
isDraining = false;
|
||||
drainRequested = false;
|
||||
}
|
||||
```
|
||||
and call it in each `beforeEach` alongside `wireMockChain()`. Alternatively, document that every
|
||||
`scheduleOutboxDrain()` started in a test MUST be awaited to completion before the test returns.
|
||||
|
||||
## Info
|
||||
|
||||
### IN-01: `onOutboxDrain` returns an unsubscribe that no production caller uses
|
||||
|
||||
**File:** `apps/api/src/lib/outboxTrigger.ts:36-39`
|
||||
**Issue:** `onOutboxDrain` faithfully returns an `off()` closure (good API design, mirrors
|
||||
`listEmitter.ts`), but the only production caller (`initOutboxTrigger`) throws it away (see WR-01).
|
||||
The capability exists but is unreachable, so the "you can stop delivery" contract in the doc
|
||||
comment is currently aspirational.
|
||||
**Fix:** Once WR-01 retains the handle, this resolves itself. No standalone change needed.
|
||||
|
||||
### IN-02: Doc comment references "D-06" for the trailing-re-drain flag; design notes attribute it to D-05
|
||||
|
||||
**File:** `apps/api/src/broker/outboxWorker.ts:165, 168`
|
||||
**Issue:** The `drainRequested` block header reads `D-05 / D-06` and the body says "never more than
|
||||
one extra pass (D-06)", while `scheduleOutboxDrain`'s own doc (line 178-179) and the file header
|
||||
(line 21) attribute the trailing-re-drain to D-05. Minor decision-ID drift; harmless but invites
|
||||
confusion when cross-referencing the phase decision log.
|
||||
**Fix:** Normalize to the authoritative decision ID for the trailing-re-drain (D-05) and drop the
|
||||
stray D-06 unless a D-06 clause genuinely governs the "one extra pass" bound.
|
||||
|
||||
### IN-03: `signalOutboxDrain()` runs `runOutboxDrain` synchronously up to the first `await` on the request thread
|
||||
|
||||
**File:** `apps/api/src/routes/events.ts:310` (and 434, 450, 525)
|
||||
**Issue:** `emit('drain')` invokes the listener synchronously, so `scheduleOutboxDrain` →
|
||||
`runOutboxDrain` executes inline on the request path until `runOutboxDrain` hits its first `await`
|
||||
(the pending-rows SELECT at outboxWorker.ts:653). The synchronous prefix is tiny (a flag check
|
||||
plus issuing the query), so the 202 is not meaningfully delayed today — but the cost is paid on
|
||||
the HTTP request thread rather than deferred. If `runOutboxDrain`'s pre-await section ever grows
|
||||
(more bookkeeping before the first `await`), it silently becomes request-path latency.
|
||||
**Fix:** If you want a hard guarantee the request returns before any drain work, defer the kick:
|
||||
`onOutboxDrain(() => queueMicrotask(() => scheduleOutboxDrain()))` or `setImmediate(...)`. Optional;
|
||||
acceptable as-is given the trivial synchronous prefix.
|
||||
|
||||
---
|
||||
|
||||
_Reviewed: 2026-06-12T17:05:00Z_
|
||||
_Reviewer: Claude (gsd-code-reviewer)_
|
||||
_Depth: standard_
|
||||
@@ -0,0 +1,102 @@
|
||||
---
|
||||
phase: 09-faster-write-back
|
||||
verified: 2026-06-12T17:07:00Z
|
||||
status: passed
|
||||
score: 5/5 must-haves verified
|
||||
overrides_applied: 0
|
||||
---
|
||||
|
||||
# Phase 09: Faster Write-Back Verification Report
|
||||
|
||||
**Phase Goal:** A created, edited, or deleted event reaches Fastmail within ~1-2 seconds (event-driven outbox drain) instead of waiting up to ~15s for the next interval tick — with every existing durability guarantee intact.
|
||||
**Verified:** 2026-06-12T17:07:00Z
|
||||
**Status:** passed
|
||||
**Re-verification:** No — initial verification
|
||||
|
||||
## Goal Achievement
|
||||
|
||||
### Observable Truths
|
||||
|
||||
| # | Truth | Status | Evidence |
|
||||
|---|-------|--------|----------|
|
||||
| 1 | After creating/editing/deleting an event, the change is signalled to drain on enqueue (not waited-for on the interval) — the drain fires promptly, not on the 15s tick. | VERIFIED | `signalOutboxDrain()` called at all 4 enqueue sites in events.ts (lines 310, 434, 450, 525). Test SC-1 in outboxWorker.test.ts confirms drain fires via EventEmitter signal without advancing any timer. |
|
||||
| 2 | The route handler still returns an optimistic 202 immediately and never makes a CalDAV call inline — the signal is fire-and-forget (never awaited). | VERIFIED | No `await signalOutboxDrain` in events.ts. All 4 sites call `signalOutboxDrain();` then immediately `return c.json(..., 202)`. events.test.ts (23 tests) confirms 202 + no-inline-CalDAV contract still holds. |
|
||||
| 3 | Edit-as-move still writes the new event before deleting the old one (create-before-delete preserved); the signal fires AFTER the db.transaction resolves, never inside it. | VERIFIED | events.ts line 410 opens `await db.transaction(async (tx) => { ... });` closing at line 432. `signalOutboxDrain()` appears at line 434 — after the `await db.transaction(...)` statement, not inside the callback. Confirmed: no `signalOutboxDrain` between the two `tx.insert` calls. |
|
||||
| 4 | No duplicate CalDAV PUTs for the same outbox row when signal and the 15s fallback overlap (exactly-once per uid preserved via the isDraining guard + drainRequested trailing-re-drain). | VERIFIED | `scheduleOutboxDrain` checks `if (isDraining) { drainRequested = true; return; }` before calling `runOutboxDrain()`. Test D-07 (Test C) verifies two concurrent `scheduleOutboxDrain()` calls result in `createCalendarEvent` called exactly once. Test D-05 (Test B) verifies mid-drain signals collapse to exactly one trailing re-drain. |
|
||||
| 5 | The 15s setInterval fallback still runs and recovers missed rows. | VERIFIED | `startOutboxWorker` in outboxWorker.ts line 857: `setInterval(() => { scheduleOutboxDrain(); }, 15 * 1000)` — interval length unchanged, body routes through `scheduleOutboxDrain` wrapper. |
|
||||
|
||||
**Score:** 5/5 truths verified
|
||||
|
||||
### Required Artifacts
|
||||
|
||||
| Artifact | Expected | Status | Details |
|
||||
|----------|----------|--------|---------|
|
||||
| `apps/api/src/lib/outboxTrigger.ts` | Zero-dependency EventEmitter signal (signalOutboxDrain, onOutboxDrain) | VERIFIED | Exists. Imports only `node:events`. Exports `signalOutboxDrain` and `onOutboxDrain`. No `setMaxListeners`. No internal project imports. |
|
||||
| `apps/api/src/broker/outboxWorker.ts` | scheduleOutboxDrain + drainRequested + initOutboxTrigger | VERIFIED | Exports `scheduleOutboxDrain`, `initOutboxTrigger`. Declares `let drainRequested = false` at line 172. `drainRequested = false` reset precedes recursive `scheduleOutboxDrain()` call (line 199 before 200 — Pitfall 3 correct). |
|
||||
| `apps/api/src/routes/events.ts` | Four post-commit signalOutboxDrain() publish sites | VERIFIED | Exactly 4 calls (lines 310, 434, 450, 525). Import at line 37. None awaited. None inside transaction callback or catch blocks. |
|
||||
| `apps/api/src/index.ts` | initOutboxTrigger() wired under isMainModule(), after startOutboxWorker() | VERIFIED | Line 138: `startOutboxWorker()`. Line 139: `initOutboxTrigger(); // subscribe drain signal listener (D-01)`. Both inside the `if (isMainModule())` block. |
|
||||
| `apps/api/tests/broker/outboxWorker.test.ts` | Trigger-wiring test block (SC-1, D-05, D-07) — 3 new tests | VERIFIED | `describe('scheduleOutboxDrain — trigger wiring (D-09)', ...)` block at line 747 with `beforeAll(() => { initOutboxTrigger(); })` and 3 tests. All 30 outboxWorker tests pass. |
|
||||
|
||||
### Key Link Verification
|
||||
|
||||
| From | To | Via | Status | Details |
|
||||
|------|----|-----|--------|---------|
|
||||
| `outboxWorker.ts (initOutboxTrigger)` | `outboxTrigger.ts (onOutboxDrain)` | `onOutboxDrain(() => scheduleOutboxDrain())` | WIRED | Line 845: `onOutboxDrain(() => scheduleOutboxDrain())`. Import at line 39. |
|
||||
| `outboxWorker.ts (scheduleOutboxDrain)` | `outboxWorker.ts (runOutboxDrain)` | isDraining guard + drainRequested trailing-re-drain | WIRED | Lines 187-203: guard checks `isDraining`, calls `runOutboxDrain()`, finally resets `drainRequested` before recursive call. |
|
||||
| `routes/events.ts` | `outboxTrigger.ts (signalOutboxDrain)` | import + call after each enqueue commit | WIRED | Import at line 37. 4 call sites verified present and not awaited. |
|
||||
| `index.ts (isMainModule block)` | `outboxWorker.ts (initOutboxTrigger)` | `initOutboxTrigger()` after `startOutboxWorker()` | WIRED | Lines 138-139 inside `isMainModule()` guard. |
|
||||
| `startOutboxWorker setInterval` | `scheduleOutboxDrain` | bare `scheduleOutboxDrain()` call in 15s interval | WIRED | Lines 857-859: `setInterval(() => { scheduleOutboxDrain(); }, 15 * 1000)`. |
|
||||
|
||||
### Data-Flow Trace (Level 4)
|
||||
|
||||
Not applicable — phase adds a signal/trigger path, not a data-rendering path. The signal carries no payload; it triggers an existing drain that queries the DB for pending rows. The drain's data path (outbox rows → CalDAV) is unchanged and was verified in prior phases.
|
||||
|
||||
### Behavioral Spot-Checks
|
||||
|
||||
| Behavior | Command | Result | Status |
|
||||
|----------|---------|--------|--------|
|
||||
| SC-1: signalOutboxDrain() triggers drain without timer advance | `npx vitest run tests/broker/outboxWorker.test.ts` — Test A | 30/30 passed | PASS |
|
||||
| D-05: mid-drain signals collapse to one trailing re-drain | Test B in same run | 30/30 passed | PASS |
|
||||
| D-07: concurrent scheduleOutboxDrain() calls are exactly-once | Test C in same run | 30/30 passed | PASS |
|
||||
| 202 + no-inline-CalDAV route contract preserved | `npx vitest run tests/routes/events.test.ts` | 23/23 passed | PASS |
|
||||
| tsc clean across all phase-9 files | `npx tsc --noEmit` | No output (clean) | PASS |
|
||||
|
||||
### Probe Execution
|
||||
|
||||
No conventional `scripts/*/tests/probe-*.sh` probes declared or found for this phase.
|
||||
|
||||
### Requirements Coverage
|
||||
|
||||
| Requirement | Source Plan | Description | Status | Evidence |
|
||||
|-------------|------------|-------------|--------|----------|
|
||||
| CAL-15 | 09-01-PLAN.md, 09-02-PLAN.md | Event reaches Fastmail within ~2s (event-driven drain) instead of up to ~15s, while preserving optimistic-202 and all outbox durability guarantees. | SATISFIED | `signalOutboxDrain()` fires immediately post-enqueue. SC-1 test confirms no timer advance needed. isDraining guard + drainRequested preserve exactly-once. 15s fallback intact. REQUIREMENTS.md marks CAL-15 as Complete. |
|
||||
|
||||
### Anti-Patterns Found
|
||||
|
||||
No debt markers (TBD/FIXME/XXX/TODO/HACK/PLACEHOLDER) found in any phase-9 modified file. No stub patterns found. No `return null`, empty handlers, or hardcoded empty data in the signal path.
|
||||
|
||||
### Human Verification Required
|
||||
|
||||
None. The phase goal (latency reduction via event-driven drain) is fully verifiable through automated tests and static code inspection:
|
||||
|
||||
- SC-1 (no 15s wait): automated by the trigger-wiring Vitest test using `setImmediate` flush — no stopwatch needed.
|
||||
- D-04 (fire-and-forget): verified by static grep — no `await signalOutboxDrain` anywhere.
|
||||
- D-03 (signal after transaction): verified by static line-number inspection — `signalOutboxDrain()` at line 434 is after `await db.transaction(...)` closes at line 432.
|
||||
- Fallback interval: verified by reading the `startOutboxWorker` body.
|
||||
|
||||
### Gaps Summary
|
||||
|
||||
No gaps. All 5 must-have truths are VERIFIED with direct codebase evidence:
|
||||
|
||||
1. All 4 enqueue sites in events.ts call `signalOutboxDrain()` post-commit, pre-202.
|
||||
2. No call is awaited; the signal is synchronous EventEmitter emit.
|
||||
3. The edit-as-move signal is provably outside the transaction callback at the source level.
|
||||
4. The `isDraining` guard + `drainRequested` boolean collapses concurrent/mid-drain signals correctly; verified by 2 dedicated automated tests.
|
||||
5. The 15s setInterval fallback routes through `scheduleOutboxDrain()` at the same 15-second cadence.
|
||||
|
||||
CAL-15 is marked Complete in REQUIREMENTS.md (line 78) and all its behavioral guarantees are implemented and tested.
|
||||
|
||||
---
|
||||
|
||||
_Verified: 2026-06-12T17:07:00Z_
|
||||
_Verifier: Claude (gsd-verifier)_
|
||||
@@ -17,7 +17,9 @@
|
||||
* T-03-14: create-before-delete ordering; create-fail aborts delete.
|
||||
*
|
||||
* runOutboxDrain is exported for unit testing.
|
||||
* startOutboxWorker wraps it in a 15-second setInterval.
|
||||
* scheduleOutboxDrain is exported for unit testing; it wraps runOutboxDrain with the isDraining
|
||||
* guard + drainRequested trailing-re-drain loop (D-05).
|
||||
* startOutboxWorker wraps scheduleOutboxDrain in a 15-second setInterval.
|
||||
* (node-cron 4.2.1 silently skipped scheduled executions in the long-running server process;
|
||||
* setInterval fires reliably in the same process — replaced to fix the silent skip.)
|
||||
*
|
||||
@@ -34,6 +36,7 @@ import { createCalendarEvent, updateCalendarEvent, deleteCalendarEvent } from '.
|
||||
import { buildVeventString, extractRruleString, RRULE_PRESETS } from './vevent.js';
|
||||
import type { FastmailClient } from './client.js';
|
||||
import { dispatchEventChange } from '../lib/eventChangeDispatcher.js';
|
||||
import { onOutboxDrain } from '../lib/outboxTrigger.js';
|
||||
|
||||
// ── Constants (D-07) ────────────────────────────────────────────────────────
|
||||
|
||||
@@ -158,6 +161,47 @@ export function assembleRruleString(
|
||||
*/
|
||||
let isDraining = false;
|
||||
|
||||
/**
|
||||
* D-05 / D-06: trailing-re-drain flag.
|
||||
* Set to true when signalOutboxDrain() fires while a drain is already in flight.
|
||||
* Collapses any number of mid-drain signals into exactly one trailing drain — never
|
||||
* rate-limited, never dropped, never more than one extra pass (D-06).
|
||||
* Reset to false BEFORE the recursive scheduleOutboxDrain() call (Pitfall 3 — resetting
|
||||
* after would allow an unbounded re-drain chain against Fastmail rate limits / T-09-01).
|
||||
*/
|
||||
let drainRequested = false;
|
||||
|
||||
/**
|
||||
* Schedule an outbox drain pass.
|
||||
*
|
||||
* If no drain is currently running, kicks off runOutboxDrain() immediately.
|
||||
* If a drain IS running (isDraining=true), records drainRequested=true so the
|
||||
* currently-running drain triggers exactly one trailing re-drain on completion (D-05).
|
||||
*
|
||||
* Errors from runOutboxDrain are caught and logged (D-02 / T-09-03).
|
||||
*
|
||||
* Called by:
|
||||
* - initOutboxTrigger's onOutboxDrain listener (event-driven path, CAL-15)
|
||||
* - startOutboxWorker's 15-second setInterval (polling fallback, D-08)
|
||||
*/
|
||||
export function scheduleOutboxDrain(): void {
|
||||
if (isDraining) {
|
||||
drainRequested = true;
|
||||
return;
|
||||
}
|
||||
runOutboxDrain()
|
||||
.catch((err: unknown) => {
|
||||
console.error('[outboxWorker] Unhandled runOutboxDrain error:', err);
|
||||
})
|
||||
.finally(() => {
|
||||
if (drainRequested) {
|
||||
// Reset BEFORE recursive call (Pitfall 3) — prevents unbounded re-drain chain
|
||||
drainRequested = false;
|
||||
scheduleOutboxDrain();
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* WR-06: max time to wait on the post-write targeted re-sync before marking the
|
||||
* outbox row 'done'. A stalled Fastmail connection cannot wedge the single-process
|
||||
@@ -788,15 +832,29 @@ export async function runOutboxDrain(): Promise<void> {
|
||||
// ── Scheduler ────────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Starts the 15-second background outbox drain schedule.
|
||||
* Register the in-process EventEmitter drain signal listener (CAL-15 / D-01).
|
||||
* Call once at API startup (in index.ts, after startOutboxWorker).
|
||||
* After registration, any signalOutboxDrain() call (fired post-enqueue) immediately
|
||||
* routes through scheduleOutboxDrain — eliminating the up-to-15s polling delay.
|
||||
*
|
||||
* T-09-02: initOutboxTrigger is called only under isMainModule() in index.ts;
|
||||
* tests import runOutboxDrain/scheduleOutboxDrain directly and never register
|
||||
* a listener — no open-handle leak preventing process exit.
|
||||
*/
|
||||
export function initOutboxTrigger(): void {
|
||||
onOutboxDrain(() => scheduleOutboxDrain());
|
||||
}
|
||||
|
||||
/**
|
||||
* Starts the 15-second background outbox drain schedule (polling fallback, D-08).
|
||||
* Call once at API startup (wired in index.ts beside startBrokerPoller).
|
||||
* Uses setInterval instead of node-cron: node-cron 4.2.1 silently skipped executions
|
||||
* in the long-running server process; setInterval fires reliably.
|
||||
* The interval body calls scheduleOutboxDrain() so errors are absorbed by its .catch
|
||||
* and mid-drain signals collapse correctly via drainRequested (D-05).
|
||||
*/
|
||||
export function startOutboxWorker(): void {
|
||||
setInterval(() => {
|
||||
runOutboxDrain().catch((err: unknown) => {
|
||||
console.error('[outboxWorker] Unhandled runOutboxDrain error:', err);
|
||||
});
|
||||
scheduleOutboxDrain();
|
||||
}, 15 * 1000);
|
||||
}
|
||||
|
||||
@@ -13,7 +13,7 @@ import { oidcAuthMiddleware, processOAuthCallback } from './auth/middleware.js';
|
||||
import { devAuthBypass } from './auth/devBypass.js';
|
||||
import { persistSessionCookie } from './auth/persistSessionCookie.js';
|
||||
import { startBrokerPoller } from './broker/poller.js';
|
||||
import { startOutboxWorker } from './broker/outboxWorker.js';
|
||||
import { startOutboxWorker, initOutboxTrigger } from './broker/outboxWorker.js';
|
||||
import { startReminderScheduler } from './broker/reminderScheduler.js';
|
||||
import webpush from 'web-push';
|
||||
|
||||
@@ -136,6 +136,7 @@ if (isMainModule()) {
|
||||
startBrokerPoller();
|
||||
// Drain the D-05 outbox every 15s: dispatches pending CalDAV writes to Fastmail.
|
||||
startOutboxWorker();
|
||||
initOutboxTrigger(); // subscribe drain signal listener (D-01)
|
||||
// Start the 1-min reminder scan for shared timed events starting in ~15 min (NOTIF-01).
|
||||
// VAPID must be configured (above) before this starts or push sends will fail.
|
||||
startReminderScheduler();
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
/**
|
||||
* In-process outbox drain signal (D-01 / D-03 / D-04).
|
||||
*
|
||||
* Module-level singleton EventEmitter — one emitter shared across all callers
|
||||
* in this Node.js process. Carries a zero-payload 'drain' event: signal is
|
||||
* fire-and-forget; no data crosses this boundary (D-04).
|
||||
*
|
||||
* Single-subscriber design:
|
||||
* - signalOutboxDrain() is called only after a successful enqueue DB commit (D-01/D-03).
|
||||
* - The sole listener is registered by initOutboxTrigger() in outboxWorker.ts.
|
||||
* - With exactly one subscriber the default EventEmitter limit of 10 is correct.
|
||||
* - Do NOT call setMaxListeners — unlike listEmitter.ts (T-04-04, 200 SSE fan-out),
|
||||
* this emitter never fans out to multiple listeners.
|
||||
*
|
||||
* Exports: signalOutboxDrain, onOutboxDrain
|
||||
*/
|
||||
|
||||
import { EventEmitter } from 'node:events';
|
||||
|
||||
// Module-level singleton — one emitter shared across all route handlers
|
||||
// in this Node.js process.
|
||||
const emitter = new EventEmitter();
|
||||
|
||||
/**
|
||||
* Fire a drain signal after a successful outbox enqueue commit.
|
||||
* Synchronous and fire-and-forget — never awaited (D-04).
|
||||
*/
|
||||
export function signalOutboxDrain(): void {
|
||||
emitter.emit('drain');
|
||||
}
|
||||
|
||||
/**
|
||||
* Register a handler to be invoked on each drain signal.
|
||||
* Returns an unsubscribe function; call it to stop delivery to this handler.
|
||||
*/
|
||||
export function onOutboxDrain(handler: () => void): () => void {
|
||||
emitter.on('drain', handler);
|
||||
return () => emitter.off('drain', handler);
|
||||
}
|
||||
@@ -34,6 +34,7 @@ import { expandOccurrences } from '../broker/expand.js';
|
||||
import { extractRruleString } from '../broker/vevent.js';
|
||||
import { getAuth } from '../auth/middleware.js';
|
||||
import { upsertUser, deriveDisplayName } from '../auth/user.js';
|
||||
import { signalOutboxDrain } from '../lib/outboxTrigger.js';
|
||||
// Side-effect import: brings in the ContextVariableMap augmentation for c.get('user')
|
||||
import '../auth/devBypass.js';
|
||||
|
||||
@@ -306,6 +307,7 @@ eventsRouter.post('/create', zValidator('json', eventFieldsSchema), async (c) =>
|
||||
payload: JSON.stringify(payload),
|
||||
});
|
||||
|
||||
signalOutboxDrain();
|
||||
return c.json({ uid }, 202);
|
||||
} catch (err) {
|
||||
console.error('[events/create] DB operation failed:', err);
|
||||
@@ -429,6 +431,7 @@ eventsRouter.patch('/:uid/edit', zValidator('json', eventFieldsSchema), async (c
|
||||
});
|
||||
});
|
||||
|
||||
signalOutboxDrain();
|
||||
return c.json({ uid: newUid }, 202);
|
||||
}
|
||||
|
||||
@@ -444,6 +447,7 @@ eventsRouter.patch('/:uid/edit', zValidator('json', eventFieldsSchema), async (c
|
||||
payload: JSON.stringify(payload),
|
||||
});
|
||||
|
||||
signalOutboxDrain();
|
||||
return c.json({ uid }, 202);
|
||||
} catch (err) {
|
||||
console.error('[events/edit] DB operation failed:', err);
|
||||
@@ -518,6 +522,7 @@ eventsRouter.delete('/:uid', async (c) => {
|
||||
etag: eventRow.etag ?? undefined,
|
||||
});
|
||||
|
||||
signalOutboxDrain();
|
||||
return c.json({ uid }, 202);
|
||||
} catch (err) {
|
||||
console.error('[events/delete] DB operation failed:', err);
|
||||
|
||||
@@ -13,12 +13,13 @@
|
||||
* They will turn GREEN in Plan 03-03 when the implementation is added.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { describe, it, expect, vi, beforeEach, beforeAll } from 'vitest';
|
||||
|
||||
// This import fails (RED) — broker/outboxWorker.ts does not exist yet.
|
||||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
|
||||
// @ts-ignore intentional RED import
|
||||
import { runOutboxDrain, assembleRruleString } from '../../src/broker/outboxWorker.js';
|
||||
import { runOutboxDrain, assembleRruleString, scheduleOutboxDrain, initOutboxTrigger } from '../../src/broker/outboxWorker.js';
|
||||
import { signalOutboxDrain } from '../../src/lib/outboxTrigger.js';
|
||||
|
||||
// ── Drizzle DB mock ────────────────────────────────────────────────────────
|
||||
// Follows the pattern from PATTERNS.md §Drizzle DB mock in tests.
|
||||
@@ -734,3 +735,126 @@ describe('FREQ persistence (D-07 regression)', () => {
|
||||
expect(capturedIcsString as string).not.toContain('FREQ=WEEKLY');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── scheduleOutboxDrain trigger-wiring tests (D-09) ─────────────────────────
|
||||
// These tests verify the trigger-wiring behavior introduced in Plan 09-01:
|
||||
// SC-1: signalOutboxDrain() → drain fires promptly (no 15s wait)
|
||||
// D-05: mid-drain signal collapses to exactly one trailing re-drain
|
||||
// D-07: concurrent scheduleOutboxDrain() calls dispatch each row exactly once
|
||||
//
|
||||
// RED: these tests fail because scheduleOutboxDrain is not yet exported.
|
||||
|
||||
describe('scheduleOutboxDrain — trigger wiring (D-09)', () => {
|
||||
// Wire the EventEmitter signal → scheduleOutboxDrain once for this describe block.
|
||||
// initOutboxTrigger registers the 'drain' listener that connects signalOutboxDrain()
|
||||
// to scheduleOutboxDrain(). Called once (not per-test) to avoid listener accumulation.
|
||||
beforeAll(() => {
|
||||
initOutboxTrigger();
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
mockPendingRows = [];
|
||||
wireMockChain();
|
||||
});
|
||||
|
||||
// Test A (SC-1): signalOutboxDrain() triggers a drain immediately — no 15s wait.
|
||||
// With one pending row and createCalendarEvent mocked to 201, calling signalOutboxDrain()
|
||||
// then flushing microtasks results in createCalendarEvent called exactly once.
|
||||
// No vi.useFakeTimers() — drain fires via the EventEmitter signal, not the interval.
|
||||
it('SC-1: signalOutboxDrain() triggers drain promptly — createCalendarEvent called once without advancing timers', async () => {
|
||||
const { createCalendarEvent } = await import('../../src/broker/write.js');
|
||||
vi.mocked(createCalendarEvent).mockResolvedValue(makeResponse(201));
|
||||
mockPendingRows = [makeRow()];
|
||||
|
||||
signalOutboxDrain();
|
||||
|
||||
// Flush microtasks so the async drain has a chance to run
|
||||
await new Promise<void>((resolve) => setImmediate(resolve));
|
||||
// One more flush to let the drain's internal async steps complete
|
||||
await new Promise<void>((resolve) => setImmediate(resolve));
|
||||
|
||||
expect(createCalendarEvent).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// Test B (SC-4 / D-05): a signal arriving during an in-flight drain triggers exactly
|
||||
// one trailing re-drain — not two, not zero.
|
||||
it('D-05: two signals mid-drain collapse to exactly one trailing re-drain', async () => {
|
||||
const { createCalendarEvent } = await import('../../src/broker/write.js');
|
||||
|
||||
// Capture the resolve function so we can hold the first drain in flight
|
||||
let resolveFirst!: () => void;
|
||||
const firstDone = new Promise<void>((resolve) => {
|
||||
resolveFirst = resolve;
|
||||
});
|
||||
|
||||
// First call: held until we release it; subsequent calls resolve immediately
|
||||
vi.mocked(createCalendarEvent)
|
||||
.mockImplementationOnce(async () => {
|
||||
await firstDone;
|
||||
return makeResponse(201);
|
||||
})
|
||||
.mockResolvedValue(makeResponse(201));
|
||||
|
||||
// Two pending rows: drain 1 picks up row 1, trailing drain picks up row 2
|
||||
const row1 = makeRow({ id: 1, uid: 'uid-1@familysync' });
|
||||
const row2 = makeRow({ id: 2, uid: 'uid-2@familysync' });
|
||||
mockPendingRows = [row1];
|
||||
|
||||
// Start drain 1 (via signal) — it blocks on firstDone
|
||||
signalOutboxDrain();
|
||||
await new Promise<void>((resolve) => setImmediate(resolve));
|
||||
|
||||
// While drain 1 is in flight, send two more signals — both should collapse to one trailing drain
|
||||
mockPendingRows = [row2];
|
||||
signalOutboxDrain();
|
||||
signalOutboxDrain();
|
||||
|
||||
// Release drain 1 — this lets it finish, then the trailing re-drain fires
|
||||
resolveFirst();
|
||||
|
||||
// Flush: drain 1 finally-block + trailing scheduleOutboxDrain() + trailing drain steps
|
||||
await new Promise<void>((resolve) => setImmediate(resolve));
|
||||
await new Promise<void>((resolve) => setImmediate(resolve));
|
||||
await new Promise<void>((resolve) => setImmediate(resolve));
|
||||
|
||||
// Drain 1 called createCalendarEvent once (row1); trailing drain called it once (row2).
|
||||
// Total = 2 — never a third pass.
|
||||
expect(createCalendarEvent).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
// Test C (SC-4 / D-07): two concurrent scheduleOutboxDrain() calls dispatch each row
|
||||
// exactly once — the second call is a no-op via the isDraining guard.
|
||||
// The trailing re-drain (triggered by drainRequested) finds 0 pending rows so
|
||||
// createCalendarEvent is called exactly once total.
|
||||
it('D-07: two concurrent scheduleOutboxDrain() calls invoke createCalendarEvent exactly once', async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const { createCalendarEvent } = await import('../../src/broker/write.js');
|
||||
// Slow create so the second scheduleOutboxDrain starts while first is still running
|
||||
vi.mocked(createCalendarEvent).mockImplementation(
|
||||
() =>
|
||||
new Promise((resolve) =>
|
||||
setTimeout(() => resolve(makeResponse(201)), 20),
|
||||
),
|
||||
);
|
||||
const row = makeRow({ id: 1 });
|
||||
// First pending-rows query returns the row; subsequent queries return empty
|
||||
// (simulates: drain 1 processes and removes the row; trailing drain finds nothing)
|
||||
mockWherePending
|
||||
.mockImplementationOnce(() => Promise.resolve([row]))
|
||||
.mockImplementation(() => Promise.resolve([]));
|
||||
|
||||
// Both calls are synchronous — second sees isDraining=true and sets drainRequested=true
|
||||
scheduleOutboxDrain();
|
||||
scheduleOutboxDrain();
|
||||
|
||||
await vi.runAllTimersAsync();
|
||||
|
||||
// Drain 1 dispatched the row once; trailing drain found 0 rows → createCalendarEvent once total
|
||||
expect(createCalendarEvent).toHaveBeenCalledTimes(1);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user