From ad181e7b2ae0b5125fdd6be87aec8a78ec7018d1 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Fri, 5 Jun 2026 21:08:07 -0400 Subject: [PATCH] docs(03-11): complete outbox durability + etag fix plan summary (CR-04, CR-05, WR-02) --- .../03-11-SUMMARY.md | 131 ++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 .planning/phases/03-event-write-back-pwa-install/03-11-SUMMARY.md diff --git a/.planning/phases/03-event-write-back-pwa-install/03-11-SUMMARY.md b/.planning/phases/03-event-write-back-pwa-install/03-11-SUMMARY.md new file mode 100644 index 0000000..57cdc71 --- /dev/null +++ b/.planning/phases/03-event-write-back-pwa-install/03-11-SUMMARY.md @@ -0,0 +1,131 @@ +--- +phase: 03-event-write-back-pwa-install +plan: "11" +subsystem: api-broker +tags: [tdd, gap-closure, outbox-worker, concurrency-guard, etag, durability, calDAV] +dependency_graph: + requires: + - 03-10: outbox worker with real VEVENT dispatch + fail-closed credentials + provides: + - cr04-durable-create-before-delete (DB sibling-status gate persisted across drain cycles) + - cr05-drain-concurrency-guard (isDraining module-level guard, single-process) + - wr02-fresh-etag-before-put (calendarEvents etag re-read at dispatch time) + affects: + - apps/api/src/broker/outboxWorker.ts + - apps/api/tests/broker/outboxWorker.test.ts +tech_stack: + added: [] + patterns: + - "TDD RED→GREEN per task" + - "DB sibling-status query pattern for durable inter-row ordering" + - "Module-level boolean drain guard for single-process deployment" + - "Symbol.for('drizzle:Name') for safe Drizzle table identification in tests (JSON.stringify circular)" + - "vi.resetAllMocks() instead of vi.clearAllMocks() when mockImplementationOnce queues must be purged" + - "Per-table mockWhere functions (mockWherePending vs mockWhereCalEvents) to isolate select mocks" +key_files: + modified: + - apps/api/src/broker/outboxWorker.ts + - apps/api/tests/broker/outboxWorker.test.ts +key_decisions: + - "CR-04 durable gate uses DB sibling-status query (not in-memory Set) so create-before-delete ordering holds across drain cycles; in-batch fast path retained as optimization" + - "CR-05 isDraining guard is explicitly documented as single-process-only; multi-replica deployments would need DB row-claim (UPDATE WHERE status='pending' with affected-rows check)" + - "WR-02 fresh etag reads calendarEvents at dispatch time, not calendarOutbox enqueue time; D-08 conflict detection preserved — genuine external changes update calendarEvents.etag differently from any queued row" + - "mockFromFn updated to use Symbol.for('drizzle:Name') to identify Drizzle tables — JSON.stringify throws CircularReference on all MySqlTable instances" + - "All beforeEach blocks switched to vi.resetAllMocks() to prevent unconsumed mockImplementationOnce calls bleeding into subsequent tests" +requirements-completed: [CAL-05, CAL-06] +duration: 30min +completed: "2026-06-05" +--- + +# Phase 03 Plan 11: Outbox Durability and Etag Fix Summary + +**Durable create-before-delete ordering (DB gate, not in-memory Set), single-process concurrency guard with documented limitation, and fresh-etag re-read before PUT — CR-04, CR-05, WR-02 closed.** + +## Performance + +- **Duration:** ~30 min +- **Started:** 2026-06-05T20:54Z +- **Completed:** 2026-06-05T21:06Z +- **Tasks:** 2 (each TDD RED+GREEN) +- **Files modified:** 2 + +## Accomplishments + +- CR-04: delete rows with `groupId` now query the DB for their sibling create's status before dispatching; the in-memory `failedCreateGroups` Set is retained as a fast path but the DB query is the authoritative gate — cross-batch move pairs cannot lose the original event +- CR-05: `let isDraining = false` module-level guard with `try/finally` ensures overlapping 15s drain cycles are no-ops; carries explicit comment that this is valid only for the single-process Unraid deployment +- WR-02: `dispatchRow` re-reads `calendarEvents.etag` just before calling `updateCalendarEvent`; uses the fresh etag as `If-Match` when available, falls back to `row.etag` otherwise — rapid successive same-uid edits no longer guarantee a spurious 412 + +## Task Commits + +Each task was committed atomically: + +1. **Task 1 RED** - `6b2cdf3` (test) — Failing tests for CR-04 cross-batch + CR-05 concurrency +2. **Task 1 GREEN** - `b409c09` (feat) — DB sibling-status gate + isDraining guard +3. **Task 2 RED** - `5eb26c0` (test) — Failing test for WR-02 fresh etag +4. **Task 2 GREEN** - `09fd1f2` (feat) — calendarEvents etag re-read before PUT + +## Files Created/Modified + +- `apps/api/src/broker/outboxWorker.ts` — Added `isDraining` guard, durable sibling-status DB check in drain loop, fresh-etag re-read in update dispatch; import `calendarEvents` from schema +- `apps/api/tests/broker/outboxWorker.test.ts` — Added 6 new tests (CR-04 cross-batch x2, CR-04 paired-failed, CR-05 concurrency, WR-02 fresh etag, WR-02 fallback); fixed mock infrastructure (Symbol.for drizzle name, vi.resetAllMocks, mockWhereCalEvents) + +## Decisions Made + +- **CR-04 durable gate approach (option b from review)**: query DB for sibling create status rather than blocking the delete row's initial enqueue. This avoids a schema change and keeps the outbox state machine simple; the sibling-status query is cheap (indexed on `groupId` + `operation`). +- **CR-05 single-process scope documented**: the `isDraining` guard comment explicitly states it is invalid for multi-replica deployments and names the DB row-claim alternative. This is a deliberate documentation constraint, not a silent assumption. +- **WR-02 fresh-etag scope boundary**: only the update dispatch is changed. Creates and deletes are unaffected. The fresh etag coalesces rapid edits by the same user; it does not weaken D-08 since a real external change would update `calendarEvents.etag` to a value never seen in any pending row. +- **Mock infrastructure fix (deviation auto-fixed)**: `mockFromFn` was using `JSON.stringify(table)` which throws `TypeError: Converting circular structure to JSON` on all Drizzle `MySqlTable` instances. Replaced with `(table)[Symbol.for('drizzle:Name')]`. Added `mockWhereCalEvents` as a separate mock for `calendarEvents` selects to isolate it from `mockWherePending` (calendarOutbox selects). Switched all `beforeEach` blocks from `vi.clearAllMocks()` to `vi.resetAllMocks()` to purge `mockImplementationOnce` queues between tests. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] Drizzle table identification using JSON.stringify throws CircularReference** +- **Found during:** Task 1 GREEN — when running tests after implementing the sibling-status DB select +- **Issue:** `wireMockChain`'s `mockFromFn` used `JSON.stringify(table).includes('member_credentials')` to identify the credential table. `JSON.stringify` on a Drizzle `MySqlTable` object throws `TypeError: Converting circular structure to JSON` (MySqlInt columns hold a back-reference to their parent table). The `catch` block silently set `isCred = false`, making ALL `db.select().from(...)` calls route to `mockWherePending` — including credential lookups. Prior tests "worked" accidentally because `mockDecryptPassword` was mocked to succeed regardless of input, but the new sibling-status select consumed `mockWherePending` calls out of order, breaking the D-04 ordering test and the CR-04 drain 2 test. +- **Fix:** Replaced with `(table as Record)[Symbol.for('drizzle:Name')]` which reads the table name property Drizzle attaches as a Symbol. Added separate `mockWhereCalEvents` for `calendarEvents` table selects. Switched all `beforeEach` to `vi.resetAllMocks()`. +- **Files modified:** apps/api/tests/broker/outboxWorker.test.ts +- **Committed in:** b409c09 (Task 1 GREEN commit) + +--- + +**Total deviations:** 1 auto-fixed (Rule 1 — bug in test infrastructure) +**Impact on plan:** Required fix. The mock bug was masked by coincidence in prior plans; the new DB selects surfaced it. + +## Issues Encountered + +None beyond the mock infrastructure deviation above. + +## Verification + +- `cd apps/api && npx vitest run tests/broker/` — 57/57 pass (7 files) +- `cd apps/api && npm run build` — clean TypeScript compile +- `grep -c 'isDraining' apps/api/src/broker/outboxWorker.ts` — 6 +- `grep -c 'single-process' apps/api/src/broker/outboxWorker.ts` — 3 +- `grep -n 'calendarEvents' apps/api/src/broker/outboxWorker.ts` — etag select in update path confirmed + +## Issues Closed + +| ID | Description | +|----|-------------| +| CR-04 | Create-before-delete ordering relied on in-memory Set, broke across drain batches — DB sibling-status gate now authoritative | +| CR-05 | No concurrency guard — overlapping drain cycles could double-dispatch same row — isDraining guard prevents it (single-process) | +| WR-02 | Update dispatch used stale enqueue-time etag — rapid successive edits guaranteed 412 — fresh calendarEvents.etag re-read at dispatch time | + +## Known Stubs + +None. All changes are functional correctness fixes. + +## Threat Flags + +No new network endpoints, auth paths, or schema changes. The fresh-etag DB read adds one SELECT per update dispatch — no new trust boundary crossed. + +## Self-Check: PASSED + +- apps/api/src/broker/outboxWorker.ts: FOUND +- apps/api/tests/broker/outboxWorker.test.ts: FOUND +- .planning/phases/03-event-write-back-pwa-install/03-11-SUMMARY.md: FOUND +- 6b2cdf3 (test RED task 1): FOUND +- b409c09 (feat GREEN task 1): FOUND +- 5eb26c0 (test RED task 2): FOUND +- 09fd1f2 (feat GREEN task 2): FOUND