From d21d29572da4e3a8df1932856e3a25355d8042b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 21 Oct 2026 11:42:53 +0200 Subject: [PATCH] fix(whoop): keep the night's sleep when its recovery is deleted The v2 recovery webhook carries the id of the associated sleep, and a recovery's rows are keyed off that same sleep id. The delete branch matched on the `:` prefix, so a `recovery.deleted` event also soft-deleted every sleep row of that night, and a `sleep.deleted` event took the recovery with it. Recovery now names its exact ids (pinned against the mapper's field tags); sleep keeps the prefix minus those ids. The targeted refresh for `recovery.updated` passed the same sleep UUID into `/v2/cycle/{cycleId}/recovery`, which answered 404 and booked a sync failure on every recovery webhook. It now reads the sleep for its `cycle_id` first. --- .../webhook/[token]/__tests__/route.test.ts | 16 ++- .../__tests__/sync-recovery-by-id.test.ts | 85 +++++++++++++ .../__tests__/webhook-notification.test.ts | 119 +++++++++++++++++- src/lib/whoop/client.ts | 27 +++- src/lib/whoop/sync-recovery.ts | 33 +++-- src/lib/whoop/webhook-handler.ts | 38 +++++- 6 files changed, 295 insertions(+), 23 deletions(-) create mode 100644 src/lib/whoop/__tests__/sync-recovery-by-id.test.ts diff --git a/src/app/api/whoop/webhook/[token]/__tests__/route.test.ts b/src/app/api/whoop/webhook/[token]/__tests__/route.test.ts index ddf3950fe..93f212a60 100644 --- a/src/app/api/whoop/webhook/[token]/__tests__/route.test.ts +++ b/src/app/api/whoop/webhook/[token]/__tests__/route.test.ts @@ -191,14 +191,16 @@ describe("POST /api/whoop/webhook/[token]", () => { expect(getGlobalBoss).not.toHaveBeenCalled(); }); - it("soft-deletes matching rows on a `*.deleted` event", async () => { + it("soft-deletes the recovery's own rows on a `recovery.deleted` event", async () => { vi.mocked(prisma.whoopConnection.findUnique).mockResolvedValue({ userId: "user-1", } as never); + // The v2 recovery webhook id is the associated sleep's UUID, which also + // prefixes the night's sleep rows — the match names the recovery's ids. const req = signedRequest({ user_id: 42, - id: "rec-uuid", + id: "sleep-uuid", type: "recovery.deleted", }); const res = await POST(req, ctx(SECRET)); @@ -208,7 +210,15 @@ describe("POST /api/whoop/webhook/[token]", () => { where: { userId: "user-1", source: "WHOOP", - externalId: { startsWith: "rec-uuid:" }, + externalId: { + in: [ + "sleep-uuid:recovery", + "sleep-uuid:hrv_rmssd", + "sleep-uuid:rhr", + "sleep-uuid:spo2", + "sleep-uuid:skin_temp", + ], + }, deletedAt: null, }, data: { deletedAt: expect.any(Date), syncVersion: { increment: 1 } }, diff --git a/src/lib/whoop/__tests__/sync-recovery-by-id.test.ts b/src/lib/whoop/__tests__/sync-recovery-by-id.test.ts new file mode 100644 index 000000000..31176c597 --- /dev/null +++ b/src/lib/whoop/__tests__/sync-recovery-by-id.test.ts @@ -0,0 +1,85 @@ +/** + * The v2 recovery webhook carries the id of the associated SLEEP, and WHOOP + * reads a recovery only through its cycle. The targeted refresh must resolve + * sleep -> cycle -> recovery; passing the sleep UUID straight into the cycle + * route answered 404 on every recovery webhook and booked a sync failure. + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const { fetchSleepById, fetchRecoveryByCycleId, upsertMock, fetchErrorMock } = + vi.hoisted(() => ({ + fetchSleepById: vi.fn(), + fetchRecoveryByCycleId: vi.fn(), + upsertMock: vi.fn(async (_u: string, rows: unknown[]) => rows.length), + fetchErrorMock: vi.fn(async () => 0), + })); + +vi.mock("@/lib/db", () => ({ prisma: {} })); + +vi.mock("../sync-core", async (importOriginal) => ({ + ...(await importOriginal()), + getValidToken: vi.fn(async () => ({ accessToken: "tok" })), + upsertWhoopMeasurements: upsertMock, + handleCollectionFetchError: fetchErrorMock, +})); + +vi.mock("../client", async (importOriginal) => ({ + ...(await importOriginal()), + fetchSleepById, + fetchRecoveryByCycleId, +})); + +import { syncWhoopRecoveryById } from "../sync-recovery"; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("syncWhoopRecoveryById", () => { + it("reads the sleep for its cycle id, then the recovery through the cycle", async () => { + fetchSleepById.mockResolvedValue({ id: "sleep-uuid", cycle_id: 991 }); + fetchRecoveryByCycleId.mockResolvedValue({ + cycle_id: 991, + sleep_id: "sleep-uuid", + user_id: 42, + created_at: "2026-06-01T07:00:00.000Z", + updated_at: "2026-06-01T07:05:00.000Z", + score_state: "SCORED", + score: { + user_calibrating: false, + recovery_score: 70, + resting_heart_rate: 52, + hrv_rmssd_milli: 61, + }, + }); + + const imported = await syncWhoopRecoveryById("user-1", "sleep-uuid"); + + expect(fetchSleepById).toHaveBeenCalledWith("tok", "sleep-uuid"); + expect(fetchRecoveryByCycleId).toHaveBeenCalledWith("tok", "991"); + expect(fetchErrorMock).not.toHaveBeenCalled(); + expect(imported).toBe(3); + const rows = upsertMock.mock.calls[0][1] as Array<{ externalId: string }>; + expect(rows.map((r) => r.externalId)).toEqual([ + "sleep-uuid:recovery", + "sleep-uuid:hrv_rmssd", + "sleep-uuid:rhr", + ]); + }); + + it("imports nothing when the sleep carries no cycle id", async () => { + fetchSleepById.mockResolvedValue({ id: "sleep-uuid" }); + + expect(await syncWhoopRecoveryById("user-1", "sleep-uuid")).toBe(0); + expect(fetchRecoveryByCycleId).not.toHaveBeenCalled(); + expect(upsertMock).not.toHaveBeenCalled(); + }); + + it("routes a failed sleep read through the shared fetch-error path", async () => { + const err = new Error("boom"); + fetchSleepById.mockRejectedValue(err); + + await syncWhoopRecoveryById("user-1", "sleep-uuid"); + expect(fetchErrorMock).toHaveBeenCalledWith("recovery", "user-1", err); + }); +}); diff --git a/src/lib/whoop/__tests__/webhook-notification.test.ts b/src/lib/whoop/__tests__/webhook-notification.test.ts index f118932bb..ba08d8caa 100644 --- a/src/lib/whoop/__tests__/webhook-notification.test.ts +++ b/src/lib/whoop/__tests__/webhook-notification.test.ts @@ -54,7 +54,18 @@ vi.mock("@/lib/logging/context", () => ({ getEvent: () => ({ setAuth: vi.fn(), addWarning: vi.fn() }), })); -import { processWhoopNotification } from "../webhook-handler"; +import { + deletedResourceExternalIdFilter, + processWhoopNotification, +} from "../webhook-handler"; +import { + mapRecovery, + mapSleep, + WHOOP_RECOVERY_FIELD_TAGS, + type WhoopRecovery, + type WhoopSleep, +} from "../client"; +import type { Prisma } from "@/generated/prisma/client"; beforeEach(() => { vi.clearAllMocks(); @@ -79,7 +90,7 @@ describe("processWhoopNotification — fetch-by-id enqueue", () => { }); }); - it("carries a numeric resource id (cycle-based recovery) as a string", async () => { + it("carries a numeric resource id as a string", async () => { await processWhoopNotification({ user_id: 42, id: 9981, @@ -148,3 +159,107 @@ describe("processWhoopNotification — fetch-by-id enqueue", () => { ); }); }); + +// One night as WHOOP v2 models it: a sleep and the recovery scored from it. +// Both key their rows off the sleep UUID, which is also the id the recovery +// webhook carries. +const SLEEP: WhoopSleep = { + id: "sleep-uuid", + cycle_id: 991, + user_id: 42, + created_at: "2026-06-01T05:00:00.000Z", + updated_at: "2026-06-01T07:00:00.000Z", + start: "2026-05-31T23:00:00.000Z", + end: "2026-06-01T07:00:00.000Z", + nap: false, + score_state: "SCORED", + score: { + stage_summary: { + total_in_bed_time_milli: 8 * 3_600_000, + total_awake_time_milli: 30 * 60_000, + total_light_sleep_time_milli: 4 * 3_600_000, + total_slow_wave_sleep_time_milli: 90 * 60_000, + total_rem_sleep_time_milli: 2 * 3_600_000, + disturbance_count: 4, + }, + sleep_needed: { + baseline_milli: 8 * 3_600_000, + need_from_sleep_debt_milli: 0, + need_from_recent_strain_milli: 0, + need_from_recent_nap_milli: 0, + }, + respiratory_rate: 15, + sleep_performance_percentage: 90, + sleep_consistency_percentage: 80, + sleep_efficiency_percentage: 92, + }, +}; +const RECOVERY: WhoopRecovery = { + cycle_id: 991, + sleep_id: "sleep-uuid", + user_id: 42, + created_at: "2026-06-01T07:00:00.000Z", + updated_at: "2026-06-01T07:05:00.000Z", + score_state: "SCORED", + score: { + user_calibrating: false, + recovery_score: 70, + resting_heart_rate: 52, + hrv_rmssd_milli: 61, + spo2_percentage: 96, + skin_temp_celsius: 33.4, + }, +}; +const SLEEP_IDS = mapSleep(SLEEP).map( + (m) => m.externalId ?? `${SLEEP.id}:${m.fieldTag}`, +); +const RECOVERY_IDS = mapRecovery(RECOVERY).map( + (m) => `${RECOVERY.sleep_id}:${m.fieldTag}`, +); + +/** The subset of Prisma's StringFilter semantics the filter uses. */ +function matches(filter: Prisma.StringFilter, id: string): boolean { + if (filter.in && !(filter.in as string[]).includes(id)) return false; + if (filter.notIn && (filter.notIn as string[]).includes(id)) return false; + if (filter.startsWith && !id.startsWith(filter.startsWith as string)) { + return false; + } + return true; +} + +describe("deletedResourceExternalIdFilter — one night, two resources", () => { + it("pins the recovery field-tag list to what the mapper emits", () => { + expect(new Set(mapRecovery(RECOVERY).map((m) => m.fieldTag))).toEqual( + new Set(WHOOP_RECOVERY_FIELD_TAGS), + ); + }); + + it("a recovery deletion removes the recovery and leaves the night's sleep", () => { + const filter = deletedResourceExternalIdFilter("recovery", "sleep-uuid"); + expect(SLEEP_IDS.length).toBeGreaterThan(5); + expect(RECOVERY_IDS.filter((id) => matches(filter, id))).toEqual( + RECOVERY_IDS, + ); + expect(SLEEP_IDS.filter((id) => matches(filter, id))).toEqual([]); + }); + + it("a sleep deletion removes the sleep and leaves its recovery", () => { + const filter = deletedResourceExternalIdFilter("sleep", "sleep-uuid"); + expect(SLEEP_IDS.filter((id) => matches(filter, id))).toEqual(SLEEP_IDS); + expect(RECOVERY_IDS.filter((id) => matches(filter, id))).toEqual([]); + }); + + it("a recovery.deleted webhook passes the exact-id match to both reads", async () => { + updateMany.mockResolvedValue({ count: 5 }); + await processWhoopNotification({ + user_id: 42, + id: "sleep-uuid", + type: "recovery.deleted", + }); + const expected = { + in: WHOOP_RECOVERY_FIELD_TAGS.map((t) => `sleep-uuid:${t}`), + }; + expect(findMany.mock.calls[0][0].where.externalId).toEqual(expected); + expect(updateMany.mock.calls[0][0].where.externalId).toEqual(expected); + }); +}); diff --git a/src/lib/whoop/client.ts b/src/lib/whoop/client.ts index f6b76aa69..17d2aa529 100644 --- a/src/lib/whoop/client.ts +++ b/src/lib/whoop/client.ts @@ -180,6 +180,12 @@ export interface WhoopRecovery { /** Sleep activity record. */ export interface WhoopSleep { id: string; + /** + * The cycle this sleep belongs to. A v2 recovery is read through its cycle, + * and the recovery webhook carries the SLEEP id, so the targeted recovery + * refresh resolves sleep -> cycle -> recovery through this field. + */ + cycle_id?: number; user_id: number; created_at: string; updated_at: string; @@ -449,8 +455,10 @@ export function fetchCycleById( /** * WHOOP v2 has no recovery-by-recovery-id route — a recovery is read through - * its cycle (`GET /v2/cycle/{cycleId}/recovery`). The recovery webhook carries - * the cycle id, so the targeted refresh resolves the recovery from there. + * its cycle (`GET /v2/cycle/{cycleId}/recovery`). The v2 recovery webhook + * carries the id of the associated SLEEP, not the cycle, so the targeted + * refresh first reads the sleep for its `cycle_id` (see + * `syncWhoopRecoveryById`). */ export function fetchRecoveryByCycleId( accessToken: string, @@ -523,6 +531,21 @@ export interface MappedMeasurement { reconstructed?: boolean; } +/** + * Every field-tag `mapRecovery` can emit. A recovery's rows are keyed + * `:` and share the `:` prefix with the rows of + * the sleep it belongs to, so a recovery deletion must name these exact ids + * rather than match on the prefix (which would take the night's sleep rows + * with it). Pinned against the mapper by a test. + */ +export const WHOOP_RECOVERY_FIELD_TAGS = [ + "recovery", + "hrv_rmssd", + "rhr", + "spo2", + "skin_temp", +] as const; + /** * Map one WHOOP recovery record into Measurement readings. An unscored record * (`score === null`) yields nothing. `recovery.score.recovery_score` → diff --git a/src/lib/whoop/sync-recovery.ts b/src/lib/whoop/sync-recovery.ts index 617d1eebf..496371ed7 100644 --- a/src/lib/whoop/sync-recovery.ts +++ b/src/lib/whoop/sync-recovery.ts @@ -9,7 +9,12 @@ * sleep UUID (v2 recovery carries `sleep_id`, not a stable recovery id) so the * externalId is stable across re-scores. */ -import { fetchRecoveries, fetchRecoveryByCycleId, mapRecovery } from "./client"; +import { + fetchRecoveries, + fetchRecoveryByCycleId, + fetchSleepById, + mapRecovery, +} from "./client"; import { getValidToken, incrementalStart, @@ -71,24 +76,32 @@ export async function syncUserRecovery( } /** - * Webhook-driven targeted refresh: resolve ONE recovery by its cycle id and - * upsert its readings, instead of re-walking the whole collection. WHOOP v2 - * reads a recovery through its cycle (`/v2/cycle/{cycleId}/recovery`), and the - * recovery webhook carries the cycle id. An unscored / since-deleted record - * yields nothing. Does NOT advance the resource cursor — a single-id refresh - * proves nothing about records between the cursor and now, so the cron/overlap - * path stays responsible for moving the cursor. + * Webhook-driven targeted refresh: resolve ONE recovery and upsert its + * readings, instead of re-walking the whole collection. The v2 recovery + * webhook carries the id of the associated SLEEP (a UUID), but WHOOP reads a + * recovery only through its cycle (`/v2/cycle/{cycleId}/recovery`), so the + * sleep is read first for its `cycle_id`. Passing the sleep UUID as a cycle id + * answered 404 on every recovery webhook and booked each one as a sync + * failure. An unscored / since-deleted record yields nothing. Does NOT advance + * the resource cursor — a single-id refresh proves nothing about records + * between the cursor and now, so the cron/overlap path stays responsible for + * moving the cursor. */ export async function syncWhoopRecoveryById( userId: string, - cycleId: string, + sleepId: string, ): Promise { const tokenInfo = await getValidToken(userId); if (!tokenInfo) return 0; let record: Awaited>; try { - record = await fetchRecoveryByCycleId(tokenInfo.accessToken, cycleId); + const sleep = await fetchSleepById(tokenInfo.accessToken, sleepId); + if (sleep.cycle_id == null) return 0; + record = await fetchRecoveryByCycleId( + tokenInfo.accessToken, + String(sleep.cycle_id), + ); } catch (err) { return handleCollectionFetchError("recovery", userId, err); } diff --git a/src/lib/whoop/webhook-handler.ts b/src/lib/whoop/webhook-handler.ts index 6fe2cb8a5..5cdaff6ff 100644 --- a/src/lib/whoop/webhook-handler.ts +++ b/src/lib/whoop/webhook-handler.ts @@ -32,7 +32,8 @@ import { getClientIp } from "@/lib/api-response"; import { annotate, getEvent } from "@/lib/logging/context"; import { getGlobalBoss } from "@/lib/jobs/boss-instance"; import { afterMeasurementMutation } from "@/lib/rollups/after-measurement-mutation"; -import type { MeasurementType } from "@/generated/prisma/client"; +import type { MeasurementType, Prisma } from "@/generated/prisma/client"; +import { WHOOP_RECOVERY_FIELD_TAGS } from "./client"; /** * Maximum age of a webhook timestamp before it is treated as a replay. Five @@ -119,6 +120,31 @@ export function verifyWhoopSignature(args: { return timingSafeEqual(a, b); } +/** + * The `externalId` match for the rows a `.deleted` event removes. + * + * Rows are keyed `:`, but the v2 recovery webhook + * carries the id of the associated SLEEP, and a recovery's rows are keyed off + * that same sleep id (`:hrv_rmssd`, ...). The sleep's own rows share + * the prefix (`:sleep_eff`, `:seg:REM`, ...). A prefix + * match on a recovery deletion therefore took the whole night's sleep with it, + * and a sleep deletion took the recovery. Recovery names its exact ids; sleep + * keeps the prefix minus those ids; every other resource keeps the prefix. + */ +export function deletedResourceExternalIdFilter( + resource: string | undefined, + resourceId: string, +): Prisma.StringFilter { + const recoveryIds = WHOOP_RECOVERY_FIELD_TAGS.map( + (tag) => `${resourceId}:${tag}`, + ); + if (resource === "recovery") return { in: recoveryIds }; + if (resource === "sleep") { + return { startsWith: `${resourceId}:`, notIn: recoveryIds }; + } + return { startsWith: `${resourceId}:` }; +} + interface WhoopWebhookBody { user_id?: number | string; id?: number | string; @@ -160,11 +186,11 @@ export async function processWhoopNotification( const queue = WHOOP_RESOURCE_QUEUE[resource ?? ""]; if (verb === "deleted") { - // Soft-delete every matching row for this user + WHOOP resource id. - // `externalId` is `:` so a `startsWith` - // match catches every measurement derived from the deleted resource. + // Soft-delete every row derived from the deleted resource for this user. + // See `deletedResourceExternalIdFilter` for why the match is per resource. if (resourceId) { const now = new Date(); + const externalId = deletedResourceExternalIdFilter(resource, resourceId); // Collect the affected `(type, measuredAt)` pairs BEFORE the // soft-delete — the rollup tail below needs to know which // `(type, day)` buckets lose rows, and after the updateMany the @@ -173,7 +199,7 @@ export async function processWhoopNotification( where: { userId: connection.userId, source: "WHOOP", - externalId: { startsWith: `${resourceId}:` }, + externalId, deletedAt: null, }, select: { type: true, measuredAt: true }, @@ -182,7 +208,7 @@ export async function processWhoopNotification( where: { userId: connection.userId, source: "WHOOP", - externalId: { startsWith: `${resourceId}:` }, + externalId, deletedAt: null, }, data: { deletedAt: now, syncVersion: { increment: 1 } },