From fd13852eb963a77850db84bec52e6441220d3fac Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Tue, 9 Jun 2026 11:04:00 -0400 Subject: [PATCH] fix(03): WR-04 rank failed/dead outbox row above done in sync-status --- apps/api/src/routes/events.ts | 14 +++++++++- apps/api/tests/routes/events.test.ts | 40 ++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/apps/api/src/routes/events.ts b/apps/api/src/routes/events.ts index 5d8184d..8e5ccfa 100644 --- a/apps/api/src/routes/events.ts +++ b/apps/api/src/routes/events.ts @@ -529,6 +529,14 @@ eventsRouter.get('/sync-status', zValidator('query', syncStatusQuerySchema), asy try { // Scope strictly to current member's rows (T-03-07 — never leak another member's outbox). + // + // WR-04: rapid successive same-uid edits enqueue multiple outbox rows. A plain + // "newest row" pick (ORDER BY createdAt DESC LIMIT 1) reports only the latest row's + // status — so if the newest succeeds but an older row dead-lettered, the user sees + // "Saved" while a queued write silently failed. Rank an unsettled/failed row ABOVE a + // done row: a row in pending/failed/dead for the uid outranks a done row, and only + // among same-priority rows do we fall back to newest-first. This surfaces a failure + // for ANY row of the uid instead of masking it behind a later success. const rows = await db .select({ uid: calendarOutbox.uid, @@ -537,7 +545,11 @@ eventsRouter.get('/sync-status', zValidator('query', syncStatusQuerySchema), asy }) .from(calendarOutbox) .where(and(eq(calendarOutbox.userId, currentUserId), eq(calendarOutbox.uid, uid))) - .orderBy(desc(calendarOutbox.createdAt)) + // status priority: failed/dead first, then pending, then done. + .orderBy( + sql`case ${calendarOutbox.status} when 'failed' then 0 when 'dead' then 0 when 'pending' then 1 else 2 end`, + desc(calendarOutbox.createdAt), + ) .limit(1) if (!rows.length) { diff --git a/apps/api/tests/routes/events.test.ts b/apps/api/tests/routes/events.test.ts index ea89c44..6099fa4 100644 --- a/apps/api/tests/routes/events.test.ts +++ b/apps/api/tests/routes/events.test.ts @@ -547,6 +547,46 @@ describe('GET /api/events/sync-status', () => { const body = await res.json() as { uid: string; status: string } expect(body.status).toBe('done') }) + + // WR-04 (iteration 2): with multiple rows for one uid, the query must rank a + // failed/dead row above a later done row so a silently-failed queued write is not + // masked by a subsequent success. The handler returns the top-ranked row's status and + // surfaces its error. The orderBy must include the status-priority CASE expression so + // the ranking is enforced in SQL (the mock cannot run real ORDER BY). + it('WR-04: prioritizes a dead row over a done row and surfaces its error', async () => { + // Seed the row the priority-ordered query WOULD return first: the dead one. + mockDbRows = [{ uid: 'uid-001@familysync', status: 'dead', lastError: 'boom', userId: 1 }] + const orderBySpy = vi.fn().mockReturnValue({ + limit: vi.fn().mockImplementation(() => Promise.resolve(mockDbRows)), + }) + const mockSimpleWhere = vi.fn().mockReturnValue({ orderBy: orderBySpy }) + mockFromFn.mockReturnValue({ where: mockSimpleWhere }) + mockSelectFn.mockReturnValue({ from: mockFromFn }) + + const { app } = await import('../../src/index.js') + const res = await app.request('/api/events/sync-status?uid=uid-001%40familysync') + expect(res.status).toBe(200) + const body = (await res.json()) as { uid: string; status: string; error?: string } + expect(body.status).toBe('dead') + expect(body.error).toBe('boom') + + // The status-priority CASE expression must be part of the ORDER BY (WR-04). + // Drizzle sql`` builds a SQL object whose static text lives in queryChunks; scan + // those for the literal 'case' rather than JSON.stringify (the Drizzle structures + // are circular and throw on serialization). + expect(orderBySpy).toHaveBeenCalled() + const orderByArgs = orderBySpy.mock.calls[0] as Array<{ queryChunks?: unknown[] }> + const caseText = orderByArgs + .flatMap((arg) => (Array.isArray(arg?.queryChunks) ? arg.queryChunks : [])) + .map((chunk) => { + // String chunks are { value: string[] } (StringChunk); join their values. + const value = (chunk as { value?: unknown })?.value + return Array.isArray(value) ? value.join('') : '' + }) + .join(' ') + .toLowerCase() + expect(caseText).toContain('case') + }) }) // ---------------------------------------------------------------------------