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 diff --git a/apps/api/src/broker/outboxWorker.ts b/apps/api/src/broker/outboxWorker.ts index afbeaa1..8f25bb2 100644 --- a/apps/api/src/broker/outboxWorker.ts +++ b/apps/api/src/broker/outboxWorker.ts @@ -26,7 +26,7 @@ import { schedule } from 'node-cron' import { and, eq, lte } from 'drizzle-orm' import { db } from '../db/client.js' -import { calendarOutbox, memberCredentials } from '../db/schema.js' +import { calendarEvents, calendarOutbox, memberCredentials } from '../db/schema.js' import { createFastmailClient } from './client.js' import { decryptPassword } from './crypto.js' import { syncCalendar } from './sync.js' @@ -53,6 +53,24 @@ const HARD_FAIL_STATUSES = new Set([400, 401, 403]) /** HTTP status code for CalDAV If-Match conflict — D-08 conflict flow. */ const CONFLICT_STATUS = 412 +// ── Drain concurrency guard (CR-05) ────────────────────────────────────────── + +/** + * Module-level drain guard — prevents overlapping 15s drain cycles from + * double-dispatching the same still-pending outbox row. + * + * SINGLE-PROCESS LIMITATION: This guard is valid ONLY for the single-process + * Unraid deployment of this two-user app where all drain cycles share the same + * Node.js module instance. A multi-process or multi-replica deployment (e.g. + * running multiple API containers behind a load balancer) would require a + * durable DB row-claim instead: + * UPDATE calendar_outbox SET status='processing' + * WHERE id=? AND status='pending' + * and only the process that wins the affected-rows check would dispatch the row. + * Do not remove this comment if deploying to multi-process infrastructure. + */ +let isDraining = false + // ── Credential + client loading ────────────────────────────────────────────── /** @@ -175,11 +193,28 @@ async function dispatchRow(row: OutboxRow): Promise { description: fields.description as string | undefined, rruleString: fields.recurrence && fields.recurrence !== 'none' ? RRULE_PRESETS[fields.recurrence as string] : undefined, }) + // WR-02: re-read the freshest etag from calendarEvents just before PUT. + // Rapid successive edits to the same uid enqueue multiple update rows, each + // carrying the etag at enqueue time. If a prior edit succeeded and triggered + // a re-sync, calendarEvents.etag was updated but the next update row still + // carries the old enqueue-time etag — guaranteed 412 on the second edit. + // Using the freshest cached etag here prevents the spurious conflict toast + // while still preserving genuine conflict detection (D-08): a real external + // change updates calendarEvents.etag differently from any pending row's etag. + let etagForPut: string | null = row.etag ?? null + const freshEtagRows = (await db + .select({ etag: calendarEvents.etag }) + .from(calendarEvents) + .where(eq(calendarEvents.uid, row.uid))) as Array<{ etag: string | null }> + if (freshEtagRows.length > 0 && freshEtagRows[0].etag != null) { + etagForPut = freshEtagRows[0].etag + } + response = await updateCalendarEvent( client, row.calendarObjectUrl, icsString, - row.etag ?? null, + etagForPut, ) } else { // create @@ -269,113 +304,169 @@ async function dispatchRow(row: OutboxRow): Promise { * are sorted before operation='delete'. If the create fails, the linked delete is * skipped (duplicate is recoverable; lost event is not — D-04 / T-03-14). * + * Concurrency guard (CR-05): the module-level `isDraining` flag ensures overlapping + * 15s scheduler invocations are no-ops for the single-process deployment. + * + * Durable create-before-delete gate (CR-04): for delete rows with a groupId, the + * worker queries the DB for the sibling create row's status. It does NOT rely on + * both rows co-occurring in the same in-memory batch. + * * Per-row errors are caught and logged so one bad row cannot crash the loop. */ export async function runOutboxDrain(): Promise { - // Fetch pending rows: WHERE status='pending' AND next_attempt_at <= NOW() - const pending = (await db - .select() - .from(calendarOutbox) - .where( - and( - eq(calendarOutbox.status, 'pending'), - lte(calendarOutbox.nextAttemptAt, new Date()), - ), - )) as OutboxRow[] + // CR-05: single-process concurrency guard (see isDraining declaration for limitations) + if (isDraining) return + isDraining = true - if (pending.length === 0) return + try { + // Fetch pending rows: WHERE status='pending' AND next_attempt_at <= NOW() + const pending = (await db + .select() + .from(calendarOutbox) + .where( + and( + eq(calendarOutbox.status, 'pending'), + lte(calendarOutbox.nextAttemptAt, new Date()), + ), + )) as OutboxRow[] - // D-04 edit-as-move ordering: sort so create rows come before delete rows within the same groupId. - // Rows without a groupId are unaffected (stable relative order preserved). - const sorted = [...pending].sort((a, b) => { - if (a.groupId && b.groupId && a.groupId === b.groupId) { - if (a.operation === 'create' && b.operation === 'delete') return -1 - if (a.operation === 'delete' && b.operation === 'create') return 1 - } - return 0 - }) + if (pending.length === 0) return - // Track groupIds where the create failed so the linked delete is skipped (D-04 / T-03-14) - const failedCreateGroups = new Set() + // D-04 edit-as-move ordering: sort so create rows come before delete rows within the same groupId. + // Rows without a groupId are unaffected (stable relative order preserved). + // This is a fast path; the authoritative gate is the durable DB sibling-status check below. + const sorted = [...pending].sort((a, b) => { + if (a.groupId && b.groupId && a.groupId === b.groupId) { + if (a.operation === 'create' && b.operation === 'delete') return -1 + if (a.operation === 'delete' && b.operation === 'create') return 1 + } + return 0 + }) - for (const row of sorted) { - // D-04: if the create for this group failed, skip the paired delete - if (row.operation === 'delete' && row.groupId && failedCreateGroups.has(row.groupId)) { - console.warn( - `[outboxWorker] Skipping delete row.id=${row.id} — create for groupId=${row.groupId} failed (D-04)`, - ) - continue - } + // Track groupIds where the create failed within this batch (fast path for same-batch pairs). + // Cross-batch ordering is enforced durably by the DB sibling-status check inside the loop. + const failedCreateGroups = new Set() - try { - const result = await dispatchRow(row) + for (const row of sorted) { + // D-04 fast path: if the create for this group already failed in this batch, skip the delete + if (row.operation === 'delete' && row.groupId && failedCreateGroups.has(row.groupId)) { + console.warn( + `[outboxWorker] Skipping delete row.id=${row.id} — create for groupId=${row.groupId} failed this batch (D-04)`, + ) + continue + } - if (result.conflict) { - // 412 — mark failed (no retry), re-sync calendar so UI sees authoritative state (D-08) - await db - .update(calendarOutbox) - .set({ status: 'failed', lastError: result.error ?? '412 conflict' }) - .where(eq(calendarOutbox.id, row.id)) - await triggerTargetedResync(row.calendarUrl, row.userId) + // CR-04: Durable create-before-delete gate — query DB for sibling create status. + // This prevents the delete from running when the create pair straddles drain batches. + if (row.operation === 'delete' && row.groupId) { + const siblingRows = (await db + .select({ status: calendarOutbox.status }) + .from(calendarOutbox) + .where( + and( + eq(calendarOutbox.groupId, row.groupId), + eq(calendarOutbox.operation, 'create'), + ), + )) as Array<{ status: string }> - if (row.groupId && row.operation === 'create') { - failedCreateGroups.add(row.groupId) + const siblingStatus = siblingRows[0]?.status + + if (siblingStatus !== 'done') { + if (siblingStatus === 'failed' || siblingStatus === 'dead') { + // Sibling create failed permanently — skip this delete forever (D-04: original preserved) + console.warn( + `[outboxWorker] Paired create for groupId=${row.groupId} is ${siblingStatus} — marking delete row.id=${row.id} failed (original event preserved, D-04)`, + ) + await db + .update(calendarOutbox) + .set({ + status: 'failed', + lastError: 'paired create did not succeed — original preserved', + }) + .where(eq(calendarOutbox.id, row.id)) + } else { + // Sibling create is still pending/processing — defer this delete to a later cycle + console.warn( + `[outboxWorker] Deferring delete row.id=${row.id} — sibling create (groupId=${row.groupId}) is not yet done (status=${siblingStatus ?? 'not found'})`, + ) + // Leave the delete row pending; do NOT update its status + } + continue } - } else if (result.success) { - // Success — mark done, trigger targeted re-sync to refresh the cache (D-06) - await db - .update(calendarOutbox) - .set({ status: 'done' }) - .where(eq(calendarOutbox.id, row.id)) - await triggerTargetedResync(row.calendarUrl, row.userId) - } else if (result.hardFail) { - // Hard fail — mark failed immediately, no retry (D-07) - await db - .update(calendarOutbox) - .set({ status: 'failed', lastError: result.error ?? 'Hard fail' }) - .where(eq(calendarOutbox.id, row.id)) + } - if (row.groupId && row.operation === 'create') { - failedCreateGroups.add(row.groupId) - } - } else { - // Transient — exponential backoff or dead-letter (D-07 / T-03-12) - const nextAttemptCount = row.attemptCount + 1 - if (nextAttemptCount >= MAX_ATTEMPTS) { - // Dead-letter: max attempts reached (T-03-12) + try { + const result = await dispatchRow(row) + + if (result.conflict) { + // 412 — mark failed (no retry), re-sync calendar so UI sees authoritative state (D-08) await db .update(calendarOutbox) - .set({ - status: 'dead', - attemptCount: nextAttemptCount, - lastError: result.error ?? 'Max attempts exceeded', - }) + .set({ status: 'failed', lastError: result.error ?? '412 conflict' }) + .where(eq(calendarOutbox.id, row.id)) + await triggerTargetedResync(row.calendarUrl, row.userId) + + if (row.groupId && row.operation === 'create') { + failedCreateGroups.add(row.groupId) + } + } else if (result.success) { + // Success — mark done, trigger targeted re-sync to refresh the cache (D-06) + await db + .update(calendarOutbox) + .set({ status: 'done' }) + .where(eq(calendarOutbox.id, row.id)) + await triggerTargetedResync(row.calendarUrl, row.userId) + } else if (result.hardFail) { + // Hard fail — mark failed immediately, no retry (D-07) + await db + .update(calendarOutbox) + .set({ status: 'failed', lastError: result.error ?? 'Hard fail' }) .where(eq(calendarOutbox.id, row.id)) if (row.groupId && row.operation === 'create') { failedCreateGroups.add(row.groupId) } } else { - // WR-01: use row.attemptCount (the attempt that just failed, 0-based) as the backoff index. - // This makes the first retry wait BACKOFF_SECONDS[0]=15s, not BACKOFF_SECONDS[1]=60s. - const backoffMs = (BACKOFF_SECONDS[row.attemptCount] ?? 1800) * 1000 - await db - .update(calendarOutbox) - .set({ - attemptCount: nextAttemptCount, - nextAttemptAt: new Date(Date.now() + backoffMs), - lastError: result.error, - }) - .where(eq(calendarOutbox.id, row.id)) + // Transient — exponential backoff or dead-letter (D-07 / T-03-12) + const nextAttemptCount = row.attemptCount + 1 + if (nextAttemptCount >= MAX_ATTEMPTS) { + // Dead-letter: max attempts reached (T-03-12) + await db + .update(calendarOutbox) + .set({ + status: 'dead', + attemptCount: nextAttemptCount, + lastError: result.error ?? 'Max attempts exceeded', + }) + .where(eq(calendarOutbox.id, row.id)) + + if (row.groupId && row.operation === 'create') { + failedCreateGroups.add(row.groupId) + } + } else { + // WR-01: use row.attemptCount (the attempt that just failed, 0-based) as the backoff index. + // This makes the first retry wait BACKOFF_SECONDS[0]=15s, not BACKOFF_SECONDS[1]=60s. + const backoffMs = (BACKOFF_SECONDS[row.attemptCount] ?? 1800) * 1000 + await db + .update(calendarOutbox) + .set({ + attemptCount: nextAttemptCount, + nextAttemptAt: new Date(Date.now() + backoffMs), + lastError: result.error, + }) + .where(eq(calendarOutbox.id, row.id)) + } } + } catch (err) { + // Per-row error isolation: log but never crash the loop (T-03-13) + console.error( + `[outboxWorker] Error dispatching row.id=${row.id} uid=${row.uid}:`, + err instanceof Error ? err.message : String(err), + ) } - } catch (err) { - // Per-row error isolation: log but never crash the loop (T-03-13) - console.error( - `[outboxWorker] Error dispatching row.id=${row.id} uid=${row.uid}:`, - err instanceof Error ? err.message : String(err), - ) } + } finally { + isDraining = false } } diff --git a/apps/api/tests/broker/outboxWorker.test.ts b/apps/api/tests/broker/outboxWorker.test.ts index 8b794e9..fd30ac3 100644 --- a/apps/api/tests/broker/outboxWorker.test.ts +++ b/apps/api/tests/broker/outboxWorker.test.ts @@ -32,20 +32,24 @@ const { mockUpdateSet, mockUpdate, mockWherePending, + mockWhereCalEvents, mockFromFn, mockSelectFn, mockDecryptPassword, } = vi.hoisted(() => { const mockUpdateSet = vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) }) const mockUpdate = vi.fn().mockReturnValue({ set: mockUpdateSet }) - // mockWherePending is the terminal node of the select chain: - // db.select().from(table).where(and(cond1, cond2)) — resolves to the row array + // mockWherePending: terminal node for calendarOutbox selects (pending-rows + sibling-status) + // db.select().from(calendarOutbox).where(...) — resolves to the row array const mockWherePending = vi.fn().mockImplementation(() => Promise.resolve([] as unknown[])) + // mockWhereCalEvents: terminal node for calendarEvents selects (etag re-read for WR-02) + // db.select({etag}).from(calendarEvents).where(...) — resolves to the etag array + const mockWhereCalEvents = vi.fn().mockImplementation(() => Promise.resolve([] as unknown[])) const mockFromFn = vi.fn().mockReturnValue({ where: mockWherePending }) const mockSelectFn = vi.fn().mockReturnValue({ from: mockFromFn }) // By default returns a dummy password so loadClientForUser succeeds const mockDecryptPassword = vi.fn().mockReturnValue('app-password') - return { mockUpdateSet, mockUpdate, mockWherePending, mockFromFn, mockSelectFn, mockDecryptPassword } + return { mockUpdateSet, mockUpdate, mockWherePending, mockWhereCalEvents, mockFromFn, mockSelectFn, mockDecryptPassword } }) let mockPendingRows: unknown[] = [] @@ -124,24 +128,22 @@ const makeResponse = (status: number): Response => function wireMockChain() { mockUpdateSet.mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) }) mockUpdate.mockReturnValue({ set: mockUpdateSet }) - // mockFromFn differentiates by table argument: - // - memberCredentials table → returns FAKE_CRED_ROW (so loadClientForUser succeeds by default) - // - anything else → returns mockPendingRows (outbox query) + // mockFromFn differentiates by table argument using Symbol.for('drizzle:Name'): + // - memberCredentials → returns FAKE_CRED_ROW (so loadClientForUser succeeds by default) + // - calendarEvents → returns mockWhereCalEvents (etag re-read for WR-02) + // - calendarOutbox (and anything else) → returns mockWherePending (pending-rows + sibling-status) + // JSON.stringify throws on circular Drizzle table structures; use Symbol identity instead. mockFromFn.mockImplementation((table: unknown) => { - // Drizzle table objects have a Symbol.for('drizzle:Name') property and a [Table.Symbol.Name]. - // The safest approach: JSON.stringify often includes the table config name. - let isCred = false - try { - isCred = JSON.stringify(table).includes('member_credentials') - } catch { - // table not serializable — not a credential table + const tableName = (table as Record)[Symbol.for('drizzle:Name')] ?? '' + if (tableName === 'member_credentials') { + return { where: vi.fn().mockResolvedValue([FAKE_CRED_ROW]) } } - return { - where: isCred - ? vi.fn().mockResolvedValue([FAKE_CRED_ROW]) - : mockWherePending, + if (tableName === 'calendar_events') { + return { where: mockWhereCalEvents } } + return { where: mockWherePending } }) + mockWhereCalEvents.mockImplementation(() => Promise.resolve([])) mockWherePending.mockImplementation(() => Promise.resolve(mockPendingRows)) mockSelectFn.mockReturnValue({ from: mockFromFn }) // Default: decryptPassword succeeds @@ -150,7 +152,7 @@ function wireMockChain() { describe('runOutboxDrain — state transitions', () => { beforeEach(() => { - vi.clearAllMocks() + vi.resetAllMocks() mockPendingRows = [] wireMockChain() }) @@ -224,7 +226,7 @@ describe('runOutboxDrain — state transitions', () => { describe('runOutboxDrain — ICS building from form JSON (CR-02)', () => { beforeEach(() => { - vi.clearAllMocks() + vi.resetAllMocks() mockPendingRows = [] wireMockChain() }) @@ -275,7 +277,7 @@ describe('runOutboxDrain — ICS building from form JSON (CR-02)', () => { describe('runOutboxDrain — edit-as-move ordering (D-04)', () => { beforeEach(() => { - vi.clearAllMocks() + vi.resetAllMocks() mockPendingRows = [] wireMockChain() }) @@ -307,6 +309,16 @@ describe('runOutboxDrain — edit-as-move ordering (D-04)', () => { // Both rows in the pending list mockPendingRows = [deleteRow, createRow] + // The durable sibling-status check (CR-04) runs for the delete row with groupId. + // It queries calendarOutbox for the sibling create's status. By the time the delete + // is processed (create was sorted and dispatched first), we simulate the sibling as 'done'. + // The base mockWherePending returns mockPendingRows for all calendarOutbox selects; we + // override just the sibling-status call with mockImplementationOnce queued after the + // pending-rows select call. + mockWherePending + .mockImplementationOnce(() => Promise.resolve([deleteRow, createRow])) // pending-rows select + .mockImplementationOnce(() => Promise.resolve([{ status: 'done' }])) // sibling-status select + await runOutboxDrain() // CREATE must be called before DELETE @@ -319,9 +331,190 @@ describe('runOutboxDrain — edit-as-move ordering (D-04)', () => { }) }) +describe('runOutboxDrain — durable create-before-delete (CR-04) + concurrency guard (CR-05)', () => { + beforeEach(() => { + vi.resetAllMocks() + mockPendingRows = [] + wireMockChain() + }) + + it('CR-04 cross-batch: drain 1 (sibling create still pending) leaves the delete pending and never calls deleteCalendarEvent', async () => { + const { deleteCalendarEvent } = await import('../../src/broker/write.js') + vi.mocked(deleteCalendarEvent).mockResolvedValue(makeResponse(204)) + + const groupId = 'edit-move-group-001' + const deleteRow = makeRow({ + id: 2, + operation: 'delete', + calendarObjectUrl: 'https://caldav.fastmail.com/dav/calendars/user/test/Old/uid.ics', + etag: '"etag-old"', + payload: null, + groupId, + }) + + // Drain 1: only the delete row is returned as pending (the create hasn't been fetched yet) + // First mockWherePending call → pending-rows select (only the delete row) + // Second mockWherePending call → sibling-status select (create is still 'pending') + mockWherePending + .mockImplementationOnce(() => Promise.resolve([deleteRow])) + .mockImplementationOnce(() => Promise.resolve([{ status: 'pending' }])) + + await runOutboxDrain() + + // The delete must NOT have been dispatched — sibling create is not yet done + expect(deleteCalendarEvent).not.toHaveBeenCalled() + + // The delete row's status must NOT have been updated to done or failed + const statusCalls = mockUpdateSet.mock.calls.filter((call) => { + const arg = call[0] as { status?: string } + return arg?.status === 'done' || arg?.status === 'failed' + }) + expect(statusCalls.length).toBe(0) + }) + + it('CR-04 cross-batch: drain 2 (sibling create now done) dispatches the delete exactly once', async () => { + const { deleteCalendarEvent } = await import('../../src/broker/write.js') + vi.mocked(deleteCalendarEvent).mockResolvedValue(makeResponse(204)) + + const groupId = 'edit-move-group-001' + const deleteRow = makeRow({ + id: 2, + operation: 'delete', + calendarObjectUrl: 'https://caldav.fastmail.com/dav/calendars/user/test/Old/uid.ics', + etag: '"etag-old"', + payload: null, + groupId, + }) + + // Drain 2: delete row is pending again, sibling create is now 'done' + mockWherePending + .mockImplementationOnce(() => Promise.resolve([deleteRow])) + .mockImplementationOnce(() => Promise.resolve([{ status: 'done' }])) + + await runOutboxDrain() + + expect(deleteCalendarEvent).toHaveBeenCalledTimes(1) + }) + + it('CR-04 paired-create-failed: if sibling create is failed, delete is marked failed and never dispatched (D-04 preserved)', async () => { + const { deleteCalendarEvent } = await import('../../src/broker/write.js') + vi.mocked(deleteCalendarEvent).mockResolvedValue(makeResponse(204)) + + const groupId = 'edit-move-group-001' + const deleteRow = makeRow({ + id: 2, + operation: 'delete', + calendarObjectUrl: 'https://caldav.fastmail.com/dav/calendars/user/test/Old/uid.ics', + etag: '"etag-old"', + payload: null, + groupId, + }) + + // Sibling create is 'failed' — the delete must be permanently skipped + mockWherePending + .mockImplementationOnce(() => Promise.resolve([deleteRow])) + .mockImplementationOnce(() => Promise.resolve([{ status: 'failed' }])) + + await runOutboxDrain() + + // The original event must be preserved — delete must NOT be dispatched + expect(deleteCalendarEvent).not.toHaveBeenCalled() + + // The delete row must be marked failed (permanently, not just skipped this cycle) + const failedCall = mockUpdateSet.mock.calls.find((call) => { + const arg = call[0] as { status?: string; lastError?: string } + return arg?.status === 'failed' && typeof arg?.lastError === 'string' + }) + expect(failedCall).toBeDefined() + const failArg = failedCall![0] as { lastError: string } + expect(failArg.lastError).toMatch(/paired create/) + }) + + it('CR-05: two overlapping runOutboxDrain calls invoke createCalendarEvent exactly once', async () => { + const { createCalendarEvent } = await import('../../src/broker/write.js') + // Simulate a slow create so the second drain starts while first is still running + vi.mocked(createCalendarEvent).mockImplementation( + () => new Promise((resolve) => setTimeout(() => resolve(makeResponse(201)), 20)), + ) + + mockPendingRows = [makeRow({ id: 1 })] + + // Start both drains concurrently WITHOUT awaiting the first + const drain1 = runOutboxDrain() + const drain2 = runOutboxDrain() + await Promise.all([drain1, drain2]) + + // Only one dispatch must have happened — the second drain must have been a no-op + expect(createCalendarEvent).toHaveBeenCalledTimes(1) + }) +}) + +describe('runOutboxDrain — fresh etag re-read before PUT (WR-02)', () => { + beforeEach(() => { + // Use resetAllMocks here (not clearAllMocks) so that unconsumed mockImplementationOnce + // queues from prior tests do not bleed into subsequent tests via the shared mockWherePending. + vi.resetAllMocks() + mockPendingRows = [] + wireMockChain() + }) + + it('WR-02 fresh etag: update PUT uses freshest calendarEvents.etag, not stale enqueue-time etag', async () => { + const { updateCalendarEvent } = await import('../../src/broker/write.js') + let capturedEtag: string | null = null + vi.mocked(updateCalendarEvent).mockImplementation(async (_client, _url, _ics, etag) => { + capturedEtag = etag + return makeResponse(204) + }) + + const updateRow = makeRow({ + operation: 'update', + calendarObjectUrl: 'https://caldav.fastmail.com/dav/calendars/user/test/uid.ics', + etag: 'old-etag', // stale enqueue-time etag + }) + mockPendingRows = [updateRow] + + // Mock the calendarEvents etag lookup to return a fresher etag. + // In RED (no fresh-etag code yet), mockWhereCalEvents is never called, so + // the PUT uses row.etag = 'old-etag'. The assertion expects 'new-etag' → fails RED. + mockWhereCalEvents.mockResolvedValue([{ etag: 'new-etag' }]) + + await runOutboxDrain() + + // The PUT must use the freshest etag from calendarEvents, not the stale row.etag + expect(capturedEtag).toBe('new-etag') + expect(capturedEtag).not.toBe('old-etag') + }) + + it('WR-02 etag fallback: update PUT falls back to row.etag when calendarEvents has no matching row', async () => { + const { updateCalendarEvent } = await import('../../src/broker/write.js') + let capturedEtag: string | null = null + vi.mocked(updateCalendarEvent).mockImplementation(async (_client, _url, _ics, etag) => { + capturedEtag = etag + return makeResponse(204) + }) + + const updateRow = makeRow({ + operation: 'update', + calendarObjectUrl: 'https://caldav.fastmail.com/dav/calendars/user/test/uid.ics', + etag: 'fallback-etag', + }) + mockPendingRows = [updateRow] + + // mockWhereCalEvents is already configured to return [] by default in wireMockChain. + // No row for the uid → worker falls back to row.etag. + // In RED, mockWhereCalEvents is never called so the test passes (row.etag used directly). + // In GREEN, mockWhereCalEvents returns [] so the fallback is exercised. + + await runOutboxDrain() + + // When calendarEvents has no row for the uid, fall back to row.etag + expect(capturedEtag).toBe('fallback-etag') + }) +}) + describe('runOutboxDrain — fail closed on bad credentials (CR-03) + backoff index fix (WR-01)', () => { beforeEach(() => { - vi.clearAllMocks() + vi.resetAllMocks() mockPendingRows = [] wireMockChain() })