fix(whoop): don't stamp sync success on an all-403 grant-revoke cycle

After the graceful collection-403 soft-skip, a genuine grant revoke 403s every
data class: each resource soft-skips (returns 0, records no failure), so the
orchestrator sees no error and stamps recordSyncSuccess — leaving the dead
connection looking healthy until the token-refresh path catches the 401 up to
~1 h later. Track the soft-skip count across the resource loop via an
AsyncLocalStorage tracker scoped to the cycle; when every resource soft-skipped
and nothing imported, leave the status as-is instead of stamping success. A
partial cycle (any import, or any resource that did not soft-skip) stamps as
normal.

src/lib/whoop/__tests__/sync-user-whoop.test.ts

@@ -0,0 +1,170 @@
+/**
+ * v1.11.5 — `syncUserWhoop` orchestration tests (mocked resource syncs).
+ *
+ * Covers the all-403 "looks-healthy" guard: a genuine grant-revoke 403s EVERY
+ * collection. Each per-resource sync soft-skips (returns 0, records no failure)
+ * via `handleCollectionFetchError`, so the orchestrator sees `total === 0` with
+ * no thrown error — and without the guard would stamp `recordSyncSuccess`,
+ * keeping a dead connection "connected" until the token-refresh path catches
+ * the 401 up to ~1 h later. The guard refuses to stamp success when every
+ * enabled resource soft-skipped and nothing imported.
+ */
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const {
+  recordSyncSuccess,
+  recordSyncFailure,
+  isReauthRequired,
+  syncUserRecovery,
+  syncUserSleep,
+  syncUserCycle,
+  syncUserWorkout,
+  syncUserBody,
+} = vi.hoisted(() => ({
+  recordSyncSuccess: vi.fn<(...a: unknown[]) => Promise<void>>(async () => {}),
+  recordSyncFailure: vi.fn<(...a: unknown[]) => Promise<void>>(async () => {}),
+  isReauthRequired: vi.fn<(...a: unknown[]) => Promise<boolean>>(
+    async () => false,
+  ),
+  syncUserRecovery: vi.fn<(...a: unknown[]) => Promise<number>>(),
+  syncUserSleep: vi.fn<(...a: unknown[]) => Promise<number>>(),
+  syncUserCycle: vi.fn<(...a: unknown[]) => Promise<number>>(),
+  syncUserWorkout: vi.fn<(...a: unknown[]) => Promise<number>>(),
+  syncUserBody: vi.fn<(...a: unknown[]) => Promise<number>>(),
+}));
+
+vi.mock("@/lib/db", () => ({ prisma: {} }));
+vi.mock("@/lib/crypto", () => ({
+  encrypt: (s: string) => s,
+  decrypt: (s: string) => s,
+}));
+vi.mock("@/lib/logging/context", () => ({
+  getEvent: () => null,
+  annotate: () => {},
+}));
+vi.mock("@/lib/integrations/status", () => ({
+  recordSyncFailure: (...a: unknown[]) => recordSyncFailure(...a),
+  recordSyncSuccess: (...a: unknown[]) => recordSyncSuccess(...a),
+  isReauthRequired: (...a: unknown[]) => isReauthRequired(...a),
+}));
+vi.mock("@/lib/rollups/measurement-rollups", () => ({
+  collapseToTypeDayKeys: () => [],
+  recomputeBucketsForMeasurement: vi.fn(async () => {}),
+}));
+vi.mock("@/lib/insights/comprehensive-generate", () => ({
+  invalidateStatusInsightsForTypes: vi.fn(async () => {}),
+}));
+
+vi.mock("../sync-recovery", () => ({
+  syncUserRecovery: (...a: unknown[]) => syncUserRecovery(...a),
+}));
+vi.mock("../sync-sleep", () => ({
+  syncUserSleep: (...a: unknown[]) => syncUserSleep(...a),
+}));
+vi.mock("../sync-cycle", () => ({
+  syncUserCycle: (...a: unknown[]) => syncUserCycle(...a),
+}));
+vi.mock("../sync-workout", () => ({
+  syncUserWorkout: (...a: unknown[]) => syncUserWorkout(...a),
+}));
+vi.mock("../sync-body", () => ({
+  syncUserBody: (...a: unknown[]) => syncUserBody(...a),
+}));
+
+import { handleCollectionFetchError, syncUserWhoop } from "../sync";
+import { WhoopApiError } from "../response-classifier";
+
+/** A resource sync that 403s its collection and soft-skips, like the real one. */
+async function softSkip403(...args: unknown[]): Promise<number> {
+  return handleCollectionFetchError(
+    "test-resource",
+    String(args[0]),
+    new WhoopApiError({
+      verb: "fetchTest",
+      classification: "reauth_required",
+      httpStatus: 403,
+      reason: "http_403",
+    }),
+  );
+}
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  isReauthRequired.mockResolvedValue(false);
+});
+
+describe("syncUserWhoop — all-403 looks-healthy guard", () => {
+  it("does NOT stamp success when EVERY resource soft-skipped and nothing imported", async () => {
+    for (const fn of [
+      syncUserRecovery,
+      syncUserSleep,
+      syncUserCycle,
+      syncUserWorkout,
+      syncUserBody,
+    ]) {
+      fn.mockImplementation(softSkip403);
+    }
+
+    const total = await syncUserWhoop("user1");
+
+    expect(total).toBe(0);
+    expect(recordSyncSuccess).not.toHaveBeenCalled();
+    // Soft-skips record no failure either — the status is left exactly as-is so
+    // the token-refresh 401 path is the one that flips it to reauth.
+    expect(recordSyncFailure).not.toHaveBeenCalled();
+  });
+
+  it("stamps success on a partial cycle (some imported, rest soft-skipped)", async () => {
+    syncUserRecovery.mockResolvedValue(3);
+    syncUserSleep.mockImplementation(softSkip403);
+    syncUserCycle.mockImplementation(softSkip403);
+    syncUserWorkout.mockImplementation(softSkip403);
+    syncUserBody.mockImplementation(softSkip403);
+
+    const total = await syncUserWhoop("user1");
+
+    expect(total).toBe(3);
+    expect(recordSyncSuccess).toHaveBeenCalledWith("user1", "whoop");
+  });
+
+  it("stamps success on a clean cycle that imported rows", async () => {
+    syncUserRecovery.mockResolvedValue(2);
+    syncUserSleep.mockResolvedValue(1);
+    syncUserCycle.mockResolvedValue(0);
+    syncUserWorkout.mockResolvedValue(0);
+    syncUserBody.mockResolvedValue(0);
+
+    const total = await syncUserWhoop("user1");
+
+    expect(total).toBe(3);
+    expect(recordSyncSuccess).toHaveBeenCalledWith("user1", "whoop");
+  });
+
+  it("stamps success on a quiet cycle (no soft-skips, nothing new)", async () => {
+    for (const fn of [
+      syncUserRecovery,
+      syncUserSleep,
+      syncUserCycle,
+      syncUserWorkout,
+      syncUserBody,
+    ]) {
+      fn.mockResolvedValue(0);
+    }
+
+    const total = await syncUserWhoop("user1");
+
+    expect(total).toBe(0);
+    // No collection was 403'd → this is a genuine "nothing changed" tick.
+    expect(recordSyncSuccess).toHaveBeenCalledWith("user1", "whoop");
+  });
+
+  it("short-circuits without stamping success when parked at error_reauth", async () => {
+    isReauthRequired.mockResolvedValue(true);
+
+    const total = await syncUserWhoop("user1");
+
+    expect(total).toBe(0);
+    expect(recordSyncSuccess).not.toHaveBeenCalled();
+    expect(syncUserRecovery).not.toHaveBeenCalled();
+  });
+});

src/lib/whoop/sync.ts

@@ -20,6 +20,7 @@
  * the re-score lag — `WHOOP_RECOVERY_SLEEP_OVERLAP_MS` is 24 h; workout/cycle
  * use the smaller `WHOOP_DEFAULT_OVERLAP_MS`.
  */
+import { AsyncLocalStorage } from "node:async_hooks";
 import { prisma } from "@/lib/db";
 import type { MeasurementType } from "@/generated/prisma/client";
 import { encrypt, decrypt } from "@/lib/crypto";
@@ -55,12 +56,29 @@ export function isCollectionForbidden(err: unknown): boolean {
   return err instanceof WhoopApiError && err.httpStatus === 403;
 }
 
+/**
+ * Per-`syncUserWhoop`-cycle counter for collection-403 soft-skips. Set up by
+ * `syncUserWhoop` around the resource loop so the per-resource catch blocks —
+ * which return a bare 0 on a soft-skip, indistinguishable from a genuine
+ * "no new records" — can be told apart from the orchestrator. AsyncLocalStorage
+ * keeps the count scoped to one user's sync, never bleeding across concurrent
+ * per-user pg-boss jobs.
+ */
+interface SoftSkipTracker {
+  count: number;
+}
+const softSkipStorage = new AsyncLocalStorage<SoftSkipTracker>();
+
 /**
  * Single-source the per-resource collection-fetch error handling. A 403 on one
  * data class soft-skips it (warn + return 0) so sibling resources still sync;
  * anything else records a classified sync failure and rethrows. Call as
  * `return handleCollectionFetchError("recovery", userId, err)` from a resource
  * sync's catch block.
+ *
+ * A soft-skip increments the ambient `softSkipStorage` tracker (when present)
+ * so `syncUserWhoop` can refuse to stamp success on an all-403 grant-revoke
+ * cycle that imported nothing.
  */
 export async function handleCollectionFetchError(
   resource: string,
@@ -71,6 +89,8 @@ export async function handleCollectionFetchError(
     getEvent()?.addWarning(
       `whoop ${resource} sync skipped for ${userId}: collection 403 (soft-skip)`,
     );
+    const tracker = softSkipStorage.getStore();
+    if (tracker) tracker.count += 1;
     return 0;
   }
   await recordWhoopSyncFailure(userId, err);
@@ -314,24 +334,39 @@ export async function syncUserWhoop(
   const { syncUserWorkout } = await import("./sync-workout");
   const { syncUserBody } = await import("./sync-body");
 
-  let total = 0;
-  let anyFailed = false;
-  for (const fn of [
+  const resources = [
     syncUserRecovery,
     syncUserSleep,
     syncUserCycle,
     syncUserWorkout,
     syncUserBody,
-  ]) {
-    try {
-      total += await fn(userId, opts);
-    } catch (err) {
-      anyFailed = true;
-      getEvent()?.addWarning(`whoop ${fn.name} failed for ${userId}: ${err}`);
-    }
-  }
+  ];
 
-  if (!anyFailed) {
+  const tracker: SoftSkipTracker = { count: 0 };
+  let total = 0;
+  let anyFailed = false;
+  await softSkipStorage.run(tracker, async () => {
+    for (const fn of resources) {
+      try {
+        total += await fn(userId, opts);
+      } catch (err) {
+        anyFailed = true;
+        getEvent()?.addWarning(`whoop ${fn.name} failed for ${userId}: ${err}`);
+      }
+    }
+  });
+
+  // A genuine grant-revoke 403s EVERY collection: each resource soft-skips
+  // (returns 0, records no failure), so `anyFailed` stays false and `total` is
+  // 0 — yet the connection is dead until the token-refresh path next catches
+  // the 401 (up to ~1 h later). Don't stamp success when the whole cycle was
+  // soft-skipped and nothing imported; leave the status as-is so the
+  // "looks-healthy" window closes. A partial cycle (some rows imported, or at
+  // least one resource that did not soft-skip) stamps success as normal.
+  const allSoftSkipped =
+    tracker.count >= resources.length && total === 0;
+
+  if (!anyFailed && !allSoftSkipped) {
     await recordSyncSuccess(userId, "whoop");
   }
   return total;