Lucas Berger
982438dc10
Mechanical reformat — no logic changes. 398 files changed, 19125 insertions(+), 16457 deletions(-). Prettier 3.8.4 with .prettierrc (singleQuote:true, semi:true, tabWidth:2, trailingComma:all, printWidth:100). Isolated per D-13-08 for reviewability.
179 lines
12 KiB
Markdown
179 lines
12 KiB
Markdown
---
|
|
phase: 03-event-write-back-pwa-install
|
|
plan: 11
|
|
type: tdd
|
|
wave: 3
|
|
depends_on: ['03-10']
|
|
gap_closure: true
|
|
autonomous: true
|
|
requirements: [CAL-05, CAL-06]
|
|
files_modified:
|
|
- apps/api/src/broker/outboxWorker.ts
|
|
- apps/api/tests/broker/outboxWorker.test.ts
|
|
must_haves:
|
|
truths:
|
|
- "An edit-as-move delete row never dispatches until its paired create row has reached status='done' — durably, across separate drain cycles"
|
|
- 'Two overlapping drain cycles never both dispatch the same outbox row'
|
|
- 'A same-calendar update re-reads the freshest etag from calendarEvents just before PUT, so rapid successive edits do not spuriously 412'
|
|
artifacts:
|
|
- path: apps/api/src/broker/outboxWorker.ts
|
|
provides: 'Durable create-before-delete gating, drain concurrency guard (single-process), fresh-etag-before-PUT'
|
|
contains: 'isDraining'
|
|
key_links:
|
|
- from: 'runOutboxDrain'
|
|
to: 'calendarOutbox status machine'
|
|
via: 'in-flight claim / blocked-delete gate persisted in DB, not an in-memory Set'
|
|
pattern: 'isDraining|processing|blocked'
|
|
---
|
|
|
|
<objective>
|
|
Close the outbox durability and concurrency holes. The create-before-delete
|
|
ordering for edit-as-move (D-04) is enforced only by an in-memory `Set` that holds
|
|
within a single drain batch — a move pair straddling batches can delete the original
|
|
before the new copy is confirmed (CR-04, the exact "lost event" D-04 forbids). There
|
|
is also no guard against overlapping 15s drain cycles double-dispatching the same
|
|
still-`pending` row (CR-05), and same-calendar updates trust a stale enqueue-time etag
|
|
that guarantees a spurious 412 on a second quick edit (WR-02).
|
|
|
|
Purpose: the outbox is durable and non-duplicating under real timing.
|
|
Output: a worker whose ordering and exactly-once guarantees survive across drain cycles.
|
|
</objective>
|
|
|
|
<execution_context>
|
|
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
|
|
@$HOME/.claude/get-shit-done/templates/summary.md
|
|
</execution_context>
|
|
|
|
<context>
|
|
@.planning/PROJECT.md
|
|
@.planning/STATE.md
|
|
@.planning/phases/03-event-write-back-pwa-install/03-REVIEW.md
|
|
@.planning/phases/03-event-write-back-pwa-install/03-10-SUMMARY.md
|
|
@apps/api/src/broker/outboxWorker.ts
|
|
@apps/api/src/db/schema.ts
|
|
@apps/api/src/broker/sync.ts
|
|
</context>
|
|
|
|
<artifacts_this_phase_produces>
|
|
No new exported symbols. Adds a module-level `isDraining` guard in outboxWorker.ts
|
|
and durable status gating for paired delete rows (reusing the existing `calendarOutbox`
|
|
`status` enum and `groupId` column — no schema migration required: a paired delete is
|
|
gated by querying its sibling create's status, not enqueued as a new enum value).
|
|
</artifacts_this_phase_produces>
|
|
|
|
<tasks>
|
|
|
|
<task type="tdd" tdd="true">
|
|
<name>Task 1: RED+GREEN — durable create-before-delete + drain concurrency guard (CR-04, CR-05)</name>
|
|
<files>apps/api/src/broker/outboxWorker.ts, apps/api/tests/broker/outboxWorker.test.ts</files>
|
|
<read_first>
|
|
- apps/api/src/broker/outboxWorker.ts (failedCreateGroups Set lines 271-296; per-batch sort lines 263-269; the pending-rows select at lines 249-257; startOutboxWorker schedule lines 359-365; runOutboxDrain entry line 247)
|
|
- apps/api/src/db/schema.ts (calendarOutbox: status enum pending|done|failed|dead, groupId, lines 125-154)
|
|
- apps/api/tests/broker/outboxWorker.test.ts (the vi.hoisted DB mock: `mockSelectFn → mockFromFn → mockWherePending`; today EVERY `db.select().from().where()` resolves to the single `mockPendingRows` array. To return DIFFERENT results for the pending-rows select vs the sibling-status select, give `mockWherePending` a per-call implementation via `.mockImplementationOnce(...)` queued in call order, OR branch on the `where(...)` condition arg. Match the existing `beforeEach` chain-restore style at lines 99-109.)
|
|
- .planning/phases/03-event-write-back-pwa-install/03-REVIEW.md (CR-04, CR-05)
|
|
</read_first>
|
|
<behavior>
|
|
- RED (CR-04 cross-batch) — CONCRETE setup, two separate `await runOutboxDrain()` calls:
|
|
Build a move pair sharing `groupId='edit-move-group-001'`: a create row (id 3, operation 'create', status 'pending') and a delete row (id 2, operation 'delete', calendarObjectUrl set, etag set, payload null).
|
|
DRAIN 1: mock the pending-rows select to return ONLY the delete row (the create is not yet due/returned). Mock the sibling-status select (the query for `groupId='edit-move-group-001' AND operation='create'`) to return `[{ status: 'pending' }]`. Assert after drain 1: `deleteCalendarEvent` was NOT called, and the delete row's status update was NOT set to 'done'/'failed' (it is left pending for a later cycle). This FAILS today: the in-memory `failedCreateGroups` Set is empty in this batch, so the delete proceeds and `deleteCalendarEvent` IS called.
|
|
DRAIN 2: now mock the pending-rows select to return the delete row again, and mock the sibling-status select to return `[{ status: 'done' }]` (the create succeeded in a prior cycle). Assert after drain 2: `deleteCalendarEvent` WAS called exactly once. State each assertion explicitly so the test cannot pass trivially: drain-1 asserts `expect(deleteCalendarEvent).not.toHaveBeenCalled()`; drain-2 asserts `expect(deleteCalendarEvent).toHaveBeenCalledTimes(1)`.
|
|
- RED (CR-04 paired-create-failed): with the same pair, mock the sibling-status select to return `[{ status: 'failed' }]`; assert `deleteCalendarEvent` is NOT called and the delete row is marked `failed` with a lastError mentioning the paired create (original event preserved per D-04).
|
|
- RED (CR-05): invoke `runOutboxDrain` twice concurrently (start the second WITHOUT awaiting the first) against the same single pending create row; assert `createCalendarEvent` is invoked exactly once across both calls (`expect(createCalendarEvent).toHaveBeenCalledTimes(1)`).
|
|
</behavior>
|
|
<action>
|
|
CR-04 — make the ordering durable. For a `delete` row that has a `groupId`, BEFORE dispatching,
|
|
query calendarOutbox for the sibling row with the same `groupId` and `operation='create'`
|
|
(a `db.select(...).from(calendarOutbox).where(and(eq(groupId, row.groupId), eq(operation,'create')))`):
|
|
- if that sibling create is not yet `done` (e.g. still `pending`), SKIP this delete this cycle —
|
|
leave the delete row `pending` (do not update its status) so a later drain re-evaluates it.
|
|
Do NOT rely on `failedCreateGroups` co-occurring in the batch.
|
|
- if the sibling create is `failed` or `dead`, skip the delete PERMANENTLY per D-04: mark the
|
|
delete row `failed` with lastError `'paired create did not succeed — original preserved'` so the
|
|
original event is not lost.
|
|
- if the sibling create is `done`, dispatch the delete normally.
|
|
Keep the within-batch create-before-delete sort as a fast path, but the DB sibling-status query is
|
|
the authoritative gate. Remove reliance on `failedCreateGroups` as the sole cross-cycle mechanism.
|
|
|
|
CR-05 — add a module-level `let isDraining = false`. At the top of `runOutboxDrain`, if `isDraining`
|
|
is true return immediately; else set `isDraining = true` and wrap the whole drain body in a
|
|
`try { ... } finally { isDraining = false }`. The 15s scheduler in `startOutboxWorker` already calls
|
|
runOutboxDrain; the guard makes an overlapping invocation a no-op.
|
|
Add an EXPLICIT code comment next to the guard (and restate in <done>) that this in-process guard is
|
|
valid ONLY for the single-process Unraid deployment of this two-user app; a multi-process or
|
|
multi-replica deployment would require a DB row-claim (e.g. `UPDATE ... SET status='processing'
|
|
WHERE id=? AND status='pending'` with affected-rows check) instead. Document the limitation; do not
|
|
silently rely on it.
|
|
|
|
Commit RED then GREEN.
|
|
|
|
</action>
|
|
<verify>
|
|
<automated>cd apps/api && npx vitest run tests/broker/outboxWorker.test.ts</automated>
|
|
</verify>
|
|
<acceptance_criteria>
|
|
- behavior: drain 1 (sibling create 'pending') leaves the delete pending and does NOT call deleteCalendarEvent; drain 2 (sibling create 'done') calls deleteCalendarEvent exactly once.
|
|
- behavior: a paired create that is 'failed'/'dead' causes the delete to be marked failed and never dispatched (original event preserved).
|
|
- behavior: two overlapping runOutboxDrain calls invoke createCalendarEvent exactly once.
|
|
- source: `grep -c 'isDraining' apps/api/src/broker/outboxWorker.ts` returns >= 2.
|
|
- source: `grep -c 'single-process' apps/api/src/broker/outboxWorker.ts` returns >= 1 (the documented-limitation comment exists).
|
|
- test-command: `cd apps/api && npx vitest run tests/broker/outboxWorker.test.ts` passes.
|
|
</acceptance_criteria>
|
|
<done>The create-before-delete invariant holds across drain cycles (proven by a two-drain sibling-status test) and overlapping cycles never double-apply a row. The isDraining guard carries an explicit comment that it is single-process-only and that multi-process needs a DB row-claim.</done>
|
|
</task>
|
|
|
|
<task type="tdd" tdd="true">
|
|
<name>Task 2: RED+GREEN — re-read freshest etag before PUT to avoid spurious 412 (WR-02)</name>
|
|
<files>apps/api/src/broker/outboxWorker.ts, apps/api/tests/broker/outboxWorker.test.ts</files>
|
|
<read_first>
|
|
- apps/api/src/broker/outboxWorker.ts (update dispatch lines 158-173; the etag comes from row.etag captured at enqueue time)
|
|
- apps/api/src/db/schema.ts (calendarEvents.etag line 95; calendarEvents.uid line 94)
|
|
- apps/api/tests/broker/outboxWorker.test.ts (to make the calendarEvents etag select return 'new-etag' while the pending-rows select returns the update row, use the same per-call `mockImplementationOnce` / where-condition-branch technique introduced in Task 1)
|
|
- .planning/phases/03-event-write-back-pwa-install/03-REVIEW.md (WR-02)
|
|
</read_first>
|
|
<behavior>
|
|
- RED: an update row carries a stale `etag` ('old-etag'), but calendarEvents has been re-synced to 'new-etag'. Mock the calendarEvents etag select to return `[{ etag: 'new-etag' }]`. Assert updateCalendarEvent is called with 'new-etag' (the freshest value read from calendarEvents at dispatch time), not the row's stale 'old-etag'. Fails today (row.etag is used verbatim).
|
|
- RED: when the calendarEvents select returns `[]` for the uid, assert updateCalendarEvent falls back to `row.etag`.
|
|
</behavior>
|
|
<action>
|
|
In the `operation === 'update'` branch of `dispatchRow`, before calling `updateCalendarEvent`,
|
|
re-read the freshest etag for this object from `calendarEvents` (select `etag` where
|
|
`uid = row.uid`, taking the row whose calendar matches `row.calendarUrl` if needed). Use that
|
|
fresh etag for the If-Match instead of `row.etag` when present; fall back to `row.etag` if the
|
|
DB read returns nothing. This coalesces rapid successive same-uid edits against the latest
|
|
server state rather than the enqueue-time snapshot, preventing the guaranteed-412-on-second-edit
|
|
described in WR-02. Do NOT weaken conflict detection for genuine third-party changes — the fresh
|
|
etag still reflects the last synced server state, so a real external edit still 412s (D-08 intact).
|
|
|
|
Commit RED then GREEN.
|
|
|
|
</action>
|
|
<verify>
|
|
<automated>cd apps/api && npx vitest run tests/broker/outboxWorker.test.ts</automated>
|
|
</verify>
|
|
<acceptance_criteria>
|
|
- behavior: the update PUT uses the freshest calendarEvents.etag, not the stale enqueue-time etag.
|
|
- behavior: when calendarEvents has no row for the uid, the worker falls back to row.etag.
|
|
- source: the update branch reads calendarEvents.etag at dispatch time (grep for a select against calendarEvents inside the update path).
|
|
- test-command: `cd apps/api && npx vitest run tests/broker/outboxWorker.test.ts` passes.
|
|
</acceptance_criteria>
|
|
<done>Rapid successive same-calendar edits no longer fire a spurious conflict toast; genuine external changes still 412 (D-08 preserved).</done>
|
|
</task>
|
|
|
|
</tasks>
|
|
|
|
<verification>
|
|
- `cd apps/api && npx vitest run tests/broker/` green.
|
|
- `cd apps/api && npm run build` succeeds.
|
|
</verification>
|
|
|
|
<success_criteria>
|
|
The outbox is durable (create-before-delete across cycles, proven by a two-drain sibling-status test),
|
|
non-duplicating (concurrency guard, documented single-process-only), and avoids spurious conflicts
|
|
(fresh-etag). CR-04, CR-05, WR-02 closed.
|
|
</success_criteria>
|
|
|
|
<output>
|
|
Create `.planning/phases/03-event-write-back-pwa-install/03-11-SUMMARY.md` when done.
|
|
</output>
|
|
</content>
|