mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 21:10:59 +00:00
Take the writes off three read routes
A GET must not have side effects — api-handler.ts says so next to READ_HTTP_METHODS, where an MCP-audience token is admitted on the safe methods precisely because they are assumed to write nothing. Three routes broke that: the sync handshake bumped User.lastSyncedAt, the achievements read inserted unlock rows, and the cycle calendar fired a discarded promise at the forecast cache so a failed write left no trace anywhere. Each write moves to the place that causes it. The sync checkpoint is now stamped by the batch ingest endpoints for measurements, mood and intakes. The handshake reports it. That also fixes the watermark: the checkpoint used to advance past a window the client had not drained, and a client trusting it would skip that window. It can only trail a real sync now. The unlock rows move to a nightly achievement-unlock-sweep. The badge grid never needed them — unlocked comes from the live metrics and the completion date is derived — so the payload is unchanged; the rows pin that date and carry it into the backup, and the sweep reaches accounts that never open the page. The cycle forecast moves to a nightly cycle-prediction-refresh. The CyclePrediction row is not a cache of a read: it is the index the reminder cron scans, so which reminders an account received depended on whether somebody had opened the calendar. persistPredictionCache no longer swallows its errors either — the job reports them.
This commit is contained in:
1 parent
eb641dcd7b
commit
6ac3a61829
27 files changed
+1365
-139
No files matched your search
@@ -0,0 +1,90 @@
|
||||
/**
|
||||
* Structural guard: the three read routes that used to write must stay reads.
|
||||
*
|
||||
* `src/lib/api-handler.ts` states the invariant next to `READ_HTTP_METHODS` —
|
||||
* an MCP-audience token is admitted on GET/HEAD/OPTIONS on the REST surface
|
||||
* because those methods are assumed side-effect-free, and adding a
|
||||
* side-effecting GET silently widens that token's reach. These three routes
|
||||
* each broke it once. The runtime tests beside each route prove the specific
|
||||
* write is gone; this one fails on a NEW one.
|
||||
*
|
||||
* Deliberately scoped to these three files and to writes issued directly in
|
||||
* the handler. A repo-wide version is not written today because it cannot be
|
||||
* written without false positives: several read routes call helpers that
|
||||
* legitimately upsert a lazily-created row (`getOrCreateCycleProfile` behind
|
||||
* `requireCycleEnabled` is the clearest one), and a text matcher cannot tell
|
||||
* that apart from a real side effect. Widening this guard means resolving
|
||||
* those cases first, not loosening the pattern.
|
||||
*/
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
/** Mutating Prisma model methods, as they appear at a call site. */
|
||||
const PRISMA_WRITE =
|
||||
/\.(create|createMany|createManyAndReturn|update|updateMany|upsert|delete|deleteMany|executeRaw|executeRawUnsafe)\s*\(/;
|
||||
|
||||
/**
|
||||
* Write helpers these routes used to reach for. A helper hides its writes
|
||||
* from the pattern above, so each one that mattered here is named.
|
||||
*/
|
||||
const WRITE_HELPERS =
|
||||
/\b(persistPredictionCache|markSyncCheckpoint|auditLog\s*\()/;
|
||||
|
||||
const ROUTES = [
|
||||
"sync/state/route.ts",
|
||||
"gamification/achievements/route.ts",
|
||||
"cycle/calendar/route.ts",
|
||||
];
|
||||
|
||||
function sourceOf(relative: string): string {
|
||||
return readFileSync(join(__dirname, "..", relative), "utf8");
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip comments before matching. Every one of these files DESCRIBES the
|
||||
* write it no longer performs, and a guard that reads prose as code is a
|
||||
* guard that fires on its own documentation.
|
||||
*/
|
||||
function code(source: string): string {
|
||||
return source.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/.*$/gm, "");
|
||||
}
|
||||
|
||||
describe("read routes issue no writes", () => {
|
||||
it("the patterns actually match a write — otherwise every case below is vacuous", () => {
|
||||
expect(
|
||||
PRISMA_WRITE.test(`await prisma.user.update({ where: { id } });`),
|
||||
).toBe(true);
|
||||
expect(
|
||||
PRISMA_WRITE.test(`prisma.userAchievement.createMany({ data });`),
|
||||
).toBe(true);
|
||||
expect(WRITE_HELPERS.test(`void persistPredictionCache(userId, p);`)).toBe(
|
||||
true,
|
||||
);
|
||||
// And do NOT match the reads these routes are made of.
|
||||
expect(PRISMA_WRITE.test(`await prisma.measurement.findMany({});`)).toBe(
|
||||
false,
|
||||
);
|
||||
expect(PRISMA_WRITE.test(`await prisma.moodEntry.count({});`)).toBe(false);
|
||||
});
|
||||
|
||||
it("scans all three routes and finds real source in each", () => {
|
||||
// An empty read set would pass every assertion below without looking at
|
||||
// anything.
|
||||
expect(ROUTES).toHaveLength(3);
|
||||
for (const route of ROUTES) {
|
||||
expect(code(sourceOf(route)).length).toBeGreaterThan(500);
|
||||
}
|
||||
});
|
||||
|
||||
for (const route of ROUTES) {
|
||||
it(`${route} calls no mutating Prisma method`, () => {
|
||||
expect(code(sourceOf(route))).not.toMatch(PRISMA_WRITE);
|
||||
});
|
||||
|
||||
it(`${route} calls no known write helper`, () => {
|
||||
expect(code(sourceOf(route))).not.toMatch(WRITE_HELPERS);
|
||||
});
|
||||
}
|
||||
});
|
||||
@@ -22,6 +22,12 @@ vi.mock("@/lib/db", () => ({
|
||||
menstrualCycle: { findMany: vi.fn() },
|
||||
cycleDayLog: { findMany: vi.fn() },
|
||||
measurement: { findMany: vi.fn() },
|
||||
// Present so a re-introduced forecast persist is VISIBLE here rather
|
||||
// than blowing up on an undefined model. `persistPredictionCache` calls
|
||||
// `findUnique` synchronously before its first await, so even the old
|
||||
// fire-and-forget `void persistPredictionCache(...)` lands on this mock
|
||||
// before the assertion runs.
|
||||
cyclePrediction: { findUnique: vi.fn(), upsert: vi.fn() },
|
||||
auditLog: { create: vi.fn() },
|
||||
},
|
||||
}));
|
||||
@@ -34,10 +40,6 @@ vi.mock("@/lib/rate-limit", () => ({
|
||||
rateLimitHeaders: () => ({}),
|
||||
}));
|
||||
vi.mock("@/lib/cycle/gate", () => ({ requireCycleEnabled: vi.fn() }));
|
||||
// The cache persist is fire-and-forget and would reach the database.
|
||||
vi.mock("@/lib/cycle/prediction-cache", () => ({
|
||||
persistPredictionCache: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
|
||||
vi.mock("@/lib/db-compat", () => ({
|
||||
ensureDbCompatibility: vi.fn().mockResolvedValue(undefined),
|
||||
@@ -142,6 +144,22 @@ afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
describe("GET /api/cycle/calendar is a read", () => {
|
||||
it("persists no forecast — the reminder cron's row is the job's, not this route's", async () => {
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(new Date("2026-06-10T12:00:00Z"));
|
||||
|
||||
const { status, body } = await callCalendar();
|
||||
expect(status).toBe(200);
|
||||
// There IS a forecast to persist, so the absence of a write is a
|
||||
// decision rather than an empty case.
|
||||
expect((body.data as Record<string, unknown>).prediction).not.toBeNull();
|
||||
|
||||
expect(prisma.cyclePrediction.findUnique).not.toHaveBeenCalled();
|
||||
expect(prisma.cyclePrediction.upsert).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("GET /api/cycle/calendar carries the resolved verdict", () => {
|
||||
it("puts the whole verdict on the wire", async () => {
|
||||
// 2026-06-10 12:00 UTC is already 2026-06-11 for a user at UTC+14.
|
||||
|
||||
@@ -3,9 +3,9 @@
|
||||
* (ios-contract §2.D).
|
||||
*
|
||||
* Calls the pure engine (`predictCycle` + per-day phase) to build
|
||||
* `{ profile, prediction, days }`. The synchronous engine call is cheap
|
||||
* (pure stats); the heavy part is the cache persist, which is debounced
|
||||
* stale-while-revalidate and fire-and-forget so it never blocks the GET.
|
||||
* `{ profile, prediction, days }`. The engine call is cheap (pure stats) and
|
||||
* the route writes nothing — the `CyclePrediction` row the reminder cron
|
||||
* reads is owned by the `cycle-prediction-refresh` job.
|
||||
* Fertile-window fields are goal-gated (surfaced for TRYING_TO_CONCEIVE and
|
||||
* AVOID_PREGNANCY — the latter with the stronger "not a contraceptive method"
|
||||
* disclaimer; suppressed for GENERAL_HEALTH / PERIMENOPAUSE / OFF),
|
||||
@@ -31,7 +31,6 @@ import {
|
||||
cycleDisclaimerKey,
|
||||
} from "@/lib/cycle/dto";
|
||||
import { resolveCycleVerdict } from "@/lib/cycle/verdict";
|
||||
import { persistPredictionCache } from "@/lib/cycle/prediction-cache";
|
||||
import { addDays, dayDiff } from "@/lib/cycle/day-math";
|
||||
import { BBT_WINDOW } from "@/lib/cycle/types";
|
||||
import { DEFAULT_TIMEZONE, moodDateKey } from "@/lib/mood/date-key";
|
||||
@@ -157,13 +156,14 @@ export const GET = apiHandler(async (request: NextRequest) => {
|
||||
goalAllowsFertile,
|
||||
);
|
||||
|
||||
// Persist the cache fire-and-forget (debounced) — never block the read.
|
||||
let generatedAt = new Date().toISOString();
|
||||
if (prediction) {
|
||||
const now = new Date();
|
||||
generatedAt = now.toISOString();
|
||||
void persistPredictionCache(user.id, prediction, now);
|
||||
}
|
||||
// v1.35.3 — this read persists nothing. It used to fire
|
||||
// `void persistPredictionCache(...)`: a write on a GET, and a discarded
|
||||
// promise on top, so a failed write left no trace at all. The forecast the
|
||||
// response carries is computed right here from the account's rows; the
|
||||
// `CyclePrediction` row the reminder cron scans is written by the
|
||||
// `cycle-prediction-refresh` job, which reaches every tracking account
|
||||
// rather than the ones that opened this page.
|
||||
const generatedAt = new Date().toISOString();
|
||||
|
||||
const locale = await resolveServerLocale({
|
||||
request,
|
||||
|
||||
@@ -185,7 +185,12 @@ describe("GET /api/gamification/achievements — per-module badge skipping", ()
|
||||
},
|
||||
];
|
||||
|
||||
type Badge = { id: string; category: string; metric: string };
|
||||
type Badge = {
|
||||
id: string;
|
||||
category: string;
|
||||
metric: string;
|
||||
unlocked: boolean;
|
||||
};
|
||||
|
||||
async function badgesFor(
|
||||
moduleMap: Record<string, boolean>,
|
||||
@@ -217,14 +222,14 @@ describe("GET /api/gamification/achievements — per-module badge skipping", ()
|
||||
expect(badges.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("never persists an unlock for a disabled-module badge", async () => {
|
||||
// A mood badge would unlock at >=1 entry; with mood OFF the createMany
|
||||
// must carry no mood achievement row.
|
||||
await badgesFor({ mood: false });
|
||||
const createCalls = vi.mocked(prisma.userAchievement.createMany).mock.calls;
|
||||
for (const [arg] of createCalls) {
|
||||
const rows = (arg as { data: Array<{ achievementId: string }> }).data;
|
||||
expect(rows.some((r) => r.achievementId.includes("mood"))).toBe(false);
|
||||
}
|
||||
it("persists no unlock at all — a GET does not write", async () => {
|
||||
// A mood badge unlocks at >=1 entry, so this read HAS an unlock to
|
||||
// notice. It still writes nothing: the row that pins the date is the
|
||||
// `achievement-unlock-sweep` job's, and a read carrying an INSERT is
|
||||
// the side-effecting GET `api-handler.ts` warns against. The badge grid
|
||||
// is unaffected — `unlocked` comes from the live metrics.
|
||||
const badges = await badgesFor({ mood: true });
|
||||
expect(badges.some((b) => b.category === "mood" && b.unlocked)).toBe(true);
|
||||
expect(prisma.userAchievement.createMany).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,3 @@
|
||||
import { prisma } from "@/lib/db";
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import { requireModuleEnabled, resolveModuleMap } from "@/lib/modules/gate";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
@@ -62,9 +61,7 @@ export const GET = apiHandler(async (request: NextRequest) => {
|
||||
// iOS-format branch runs the locale-aware transform after the cache
|
||||
// read so the cache stays format-agnostic and the achievement-progress
|
||||
// dashboard duplicate (seen twice per dashboard mount in the v1.4.33
|
||||
// HAR) coalesces into one builder call. v1.18.0 B5 — unlock persistence
|
||||
// now happens in the handler after the cache read (idempotent), not as
|
||||
// a side effect inside the cached factory.
|
||||
// HAR) coalesces into one builder call.
|
||||
// v1.18.0 B5 — resolve the per-user module map once and pass it into the
|
||||
// builder so badge categories whose owning module is disabled (sleep
|
||||
// badges when sleep is off, mood badges when mood is off) are skipped
|
||||
@@ -76,8 +73,7 @@ export const GET = apiHandler(async (request: NextRequest) => {
|
||||
// 10-minute stale window; the app-wide `AchievementUnlockNotifier` polls
|
||||
// every 2 minutes, so a hard-TTL read always missed and re-paid the cold
|
||||
// build. SWR serves the prior payload instantly and warms one background
|
||||
// recompute. Persistence of `pendingUnlocks` stays OUTSIDE this read (see
|
||||
// below), so a stale-served body never skips an unlock write.
|
||||
// recompute.
|
||||
const result = await cachedSwr(
|
||||
caches.achievements as ServerCache<AchievementsResult>,
|
||||
user.id,
|
||||
@@ -85,25 +81,19 @@ export const GET = apiHandler(async (request: NextRequest) => {
|
||||
annotate,
|
||||
);
|
||||
|
||||
// v1.18.0 B5 — persist newly unlocked achievements OUTSIDE the cached
|
||||
// factory. `createMany({ skipDuplicates: true })` is idempotent on the
|
||||
// `(userId, achievementId)` unique, so re-running it on a cache hit is
|
||||
// a no-op rather than a duplicate, and the write is never skipped just
|
||||
// because the read was served from cache.
|
||||
if (result.pendingUnlocks.length > 0) {
|
||||
await prisma.userAchievement.createMany({
|
||||
data: result.pendingUnlocks.map((u) => ({
|
||||
userId: user.id,
|
||||
achievementId: u.achievementId,
|
||||
unlockedAt: new Date(u.unlockedAt),
|
||||
})),
|
||||
skipDuplicates: true,
|
||||
});
|
||||
annotate({
|
||||
action: { name: "gamification.achievements" },
|
||||
meta: { newUnlocks: result.pendingUnlocks.length },
|
||||
});
|
||||
}
|
||||
// v1.35.3 — the unlock rows are NOT written here. A GET must not have side
|
||||
// effects (`api-handler.ts`, the MCP-audience note above
|
||||
// `READ_HTTP_METHODS`), and this INSERT was one. The badge grid does not
|
||||
// need the rows: `unlocked` comes from the live metrics and the completion
|
||||
// date is derived from the account's own history, so the payload below is
|
||||
// identical whether or not a row exists. The rows pin that date against a
|
||||
// later data edit and carry it into the backup, which is durable work and
|
||||
// belongs on the `achievement-unlock-sweep` job — where it also covers the
|
||||
// accounts that never open this page.
|
||||
annotate({
|
||||
action: { name: "gamification.achievements" },
|
||||
meta: { pendingUnlocks: result.pendingUnlocks.length },
|
||||
});
|
||||
|
||||
// Strip the internal `pendingUnlocks` carrier; it never goes on the wire.
|
||||
const payload = {
|
||||
|
||||
@@ -156,6 +156,7 @@ describe("POST /api/measurements/batch — PR detection enqueue (v1.4.25 W16c)",
|
||||
// This batch declares no trigger, so the trigger field is nulled and the
|
||||
// background timestamp is not written — see the sync-trigger suite.
|
||||
data: {
|
||||
lastSyncedAt: expect.any(Date),
|
||||
healthKitLastSyncedAt: expect.any(Date),
|
||||
healthKitLastSyncTrigger: null,
|
||||
},
|
||||
@@ -170,7 +171,12 @@ describe("POST /api/measurements/batch — PR detection enqueue (v1.4.25 W16c)",
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
// The SyncMode checkpoint still advances — a MANUAL push is a client
|
||||
// sync — but none of the HealthKit-specific fields are touched.
|
||||
expect(prisma.user.update).toHaveBeenCalledWith({
|
||||
where: { id: "user-1" },
|
||||
data: { lastSyncedAt: expect.any(Date) },
|
||||
});
|
||||
});
|
||||
|
||||
it("surfaces a hard reconciliation failure without checkpointing it", async () => {
|
||||
|
||||
@@ -643,9 +643,18 @@ async function postBatch(request: NextRequest): Promise<Response> {
|
||||
);
|
||||
});
|
||||
|
||||
// This batch route is the native HealthKit ingestion boundary. Stamp only
|
||||
// after every attempted write has reached a durable, non-failed verdict;
|
||||
// MANUAL-only and skipped-only batches are not HealthKit syncs.
|
||||
// This batch route is the native ingestion boundary, so it is also where the
|
||||
// SyncMode checkpoint (`User.lastSyncedAt`, the same column
|
||||
// `markSyncCheckpoint` writes) is established — `GET /api/sync/state` only
|
||||
// reports it. The checkpoint covers MANUAL pushes too: the client synced
|
||||
// whatever it was holding. A batch carrying a hard failure leaves the
|
||||
// checkpoint where it was, so the undelivered window stays inside whatever
|
||||
// the client asks for next.
|
||||
//
|
||||
// The HealthKit fields ride the same update and keep their narrower
|
||||
// condition. Stamp only after every attempted write has reached a durable,
|
||||
// non-failed verdict; MANUAL-only and skipped-only batches are not
|
||||
// HealthKit syncs.
|
||||
//
|
||||
// The trigger the client declared is stamped alongside, and a background- or
|
||||
// push-triggered batch additionally dates `healthKitLastBackgroundSyncAt`.
|
||||
@@ -655,17 +664,22 @@ async function postBatch(request: NextRequest): Promise<Response> {
|
||||
// trigger writes null over the previous value rather than leaving a stale one
|
||||
// standing: the field describes THIS sync, and an older build's silence is
|
||||
// not evidence about it.
|
||||
if (healthKitSyncSucceeded) {
|
||||
if (failedCount === 0) {
|
||||
const syncedAt = new Date();
|
||||
const deliveredWithoutTheApp =
|
||||
syncTrigger === "background" || syncTrigger === "push";
|
||||
await prisma.user.update({
|
||||
where: { id: user.id },
|
||||
data: {
|
||||
healthKitLastSyncedAt: syncedAt,
|
||||
healthKitLastSyncTrigger: syncTrigger ?? null,
|
||||
...(deliveredWithoutTheApp
|
||||
? { healthKitLastBackgroundSyncAt: syncedAt }
|
||||
lastSyncedAt: syncedAt,
|
||||
...(healthKitSyncSucceeded
|
||||
? {
|
||||
healthKitLastSyncedAt: syncedAt,
|
||||
healthKitLastSyncTrigger: syncTrigger ?? null,
|
||||
...(deliveredWithoutTheApp
|
||||
? { healthKitLastBackgroundSyncAt: syncedAt }
|
||||
: {}),
|
||||
}
|
||||
: {}),
|
||||
},
|
||||
});
|
||||
|
||||
@@ -21,6 +21,9 @@ vi.mock("@/lib/db", () => ({
|
||||
// v1.15.19 — resolver-null convergence probe before standalone insert.
|
||||
findFirst: vi.fn(),
|
||||
},
|
||||
// The bulk drain stamps the SyncMode checkpoint (`User.lastSyncedAt`) —
|
||||
// it is a client push, and the push is what the checkpoint describes.
|
||||
user: { update: vi.fn().mockResolvedValue({}) },
|
||||
auditLog: { create: vi.fn() },
|
||||
},
|
||||
}));
|
||||
@@ -1486,3 +1489,49 @@ describe("POST /api/medications/intake/bulk — identity stability floor", () =>
|
||||
expect(prisma.medicationIntakeEvent.create).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────
|
||||
// SyncMode checkpoint — the push owns `User.lastSyncedAt`
|
||||
// ────────────────────────────────────────────────────────────────────
|
||||
/**
|
||||
* `GET /api/sync/state` reports the checkpoint and writes nothing, so the
|
||||
* ingest boundaries are the only writers. If this one stops stamping, the
|
||||
* checkpoint stays null forever and every paired client re-pairs full.
|
||||
*/
|
||||
describe("POST /api/medications/intake/bulk — sync checkpoint", () => {
|
||||
beforeEach(() => {
|
||||
vi.mocked(prisma.medication.findMany).mockResolvedValue([
|
||||
{ id: "med-1" },
|
||||
] as never);
|
||||
vi.mocked(prisma.medication.findFirst).mockResolvedValue(null as never);
|
||||
vi.mocked(prisma.medicationIntakeEvent.findMany).mockResolvedValue(
|
||||
[] as never,
|
||||
);
|
||||
vi.mocked(prisma.medicationIntakeEvent.findFirst).mockResolvedValue(
|
||||
null as never,
|
||||
);
|
||||
vi.mocked(prisma.medicationIntakeEvent.create).mockResolvedValue({
|
||||
id: "row-1",
|
||||
} as never);
|
||||
});
|
||||
|
||||
it("stamps lastSyncedAt when the drain lands", async () => {
|
||||
const res = await POST(
|
||||
postReq({
|
||||
entries: [
|
||||
{
|
||||
medicationId: "med-1",
|
||||
scheduledFor: "2026-06-15T05:00:00.000Z",
|
||||
takenAt: "2026-06-15T05:02:00.000Z",
|
||||
},
|
||||
],
|
||||
}),
|
||||
);
|
||||
expect(res.status).toBe(200);
|
||||
expect(prisma.user.update).toHaveBeenCalledTimes(1);
|
||||
expect(prisma.user.update).toHaveBeenCalledWith({
|
||||
where: { id: "user-1" },
|
||||
data: { lastSyncedAt: expect.any(Date) },
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -52,6 +52,7 @@ import {
|
||||
} from "@/lib/api-response";
|
||||
import { withIdempotency } from "@/lib/idempotency";
|
||||
import { checkRateLimit } from "@/lib/rate-limit";
|
||||
import { markSyncCheckpoint } from "@/lib/sync/checkpoint";
|
||||
import { invalidateUserMedications } from "@/lib/cache/invalidate";
|
||||
import {
|
||||
recomputeMedicationComplianceForDay,
|
||||
@@ -932,6 +933,12 @@ async function postBulk(request: NextRequest): Promise<Response> {
|
||||
})();
|
||||
}
|
||||
|
||||
// The SyncMode checkpoint belongs to the push that moved the data, not to
|
||||
// the `/api/sync/state` handshake that reports it. This endpoint is one of
|
||||
// the three domains that handshake summarises, so an intake drain advances
|
||||
// it.
|
||||
await markSyncCheckpoint(user.id);
|
||||
|
||||
return apiSuccess({
|
||||
processed: entries.length,
|
||||
inserted,
|
||||
|
||||
@@ -29,6 +29,9 @@ const { moodEntryMock, txClient } = vi.hoisted(() => {
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
moodEntry: moodEntryMock,
|
||||
// The bulk drain stamps the SyncMode checkpoint (`User.lastSyncedAt`) —
|
||||
// it is a client push, and the push is what the checkpoint describes.
|
||||
user: { update: vi.fn().mockResolvedValue({}) },
|
||||
auditLog: { create: vi.fn() },
|
||||
$transaction: vi.fn(async (fn: unknown) => {
|
||||
if (typeof fn === "function") {
|
||||
@@ -688,3 +691,32 @@ describe("POST /api/mood-entries/bulk — upsert + tag links are atomic", () =>
|
||||
expect(prisma.$transaction).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The SyncMode checkpoint (`User.lastSyncedAt`) belongs to the push that moved
|
||||
* the data. `GET /api/sync/state` reports it and writes nothing, so if this
|
||||
* boundary stops stamping, the checkpoint stays null forever and every paired
|
||||
* client re-pairs with a full backfill.
|
||||
*/
|
||||
describe("POST /api/mood-entries/bulk — sync checkpoint", () => {
|
||||
beforeEach(() => {
|
||||
vi.mocked(prisma.moodEntry.findMany).mockResolvedValue([]);
|
||||
vi.mocked(prisma.moodEntry.upsert).mockResolvedValue({
|
||||
id: "entry-1",
|
||||
} as never);
|
||||
});
|
||||
|
||||
it("stamps lastSyncedAt when the drain lands", async () => {
|
||||
const res = await POST(
|
||||
postReq({
|
||||
entries: [{ mood: "OKAY", moodLoggedAt: "2026-05-16T08:00:00.000Z" }],
|
||||
}),
|
||||
);
|
||||
expect(res.status).toBe(200);
|
||||
expect(prisma.user.update).toHaveBeenCalledTimes(1);
|
||||
expect(prisma.user.update).toHaveBeenCalledWith({
|
||||
where: { id: "user-1" },
|
||||
data: { lastSyncedAt: expect.any(Date) },
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -38,6 +38,7 @@ import {
|
||||
} from "@/lib/api-response";
|
||||
import { withIdempotency } from "@/lib/idempotency";
|
||||
import { checkRateLimit } from "@/lib/rate-limit";
|
||||
import { markSyncCheckpoint } from "@/lib/sync/checkpoint";
|
||||
import {
|
||||
getScoreForMood,
|
||||
moodLevelEnum,
|
||||
@@ -493,6 +494,11 @@ async function postBulk(request: NextRequest): Promise<Response> {
|
||||
}
|
||||
}
|
||||
|
||||
// The SyncMode checkpoint belongs to the push that moved the data, not to
|
||||
// the `/api/sync/state` handshake that reports it. This endpoint is one of
|
||||
// the three domains that handshake summarises, so a mood drain advances it.
|
||||
await markSyncCheckpoint(user.id);
|
||||
|
||||
return apiSuccess({
|
||||
processed: entries.length,
|
||||
inserted,
|
||||
|
||||
@@ -0,0 +1,92 @@
|
||||
/**
|
||||
* `GET /api/sync/state` is a read.
|
||||
*
|
||||
* It used to bump `User.lastSyncedAt` on every call, which is the exact
|
||||
* side-effecting GET the MCP-audience note in `api-handler.ts` warns against,
|
||||
* and it advanced the checkpoint past a window the client had not drained.
|
||||
* The checkpoint is now owned by the ingest boundaries; this route reports it.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
vi.mock("@/lib/api-handler", () => ({
|
||||
apiHandler: <T extends (...args: unknown[]) => unknown>(fn: T) => fn,
|
||||
requireAuth: vi.fn(async () => ({ user: { id: "u1" } })),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
measurement: { findFirst: vi.fn(), count: vi.fn() },
|
||||
moodEntry: { findFirst: vi.fn(), count: vi.fn() },
|
||||
medicationIntakeEvent: { findFirst: vi.fn(), count: vi.fn() },
|
||||
user: { findUnique: vi.fn(), update: vi.fn() },
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() }));
|
||||
|
||||
vi.mock("@/lib/api-response", () => ({
|
||||
apiSuccess: (data: unknown) => ({ data, error: null }),
|
||||
}));
|
||||
|
||||
import { GET } from "../route";
|
||||
import { prisma } from "@/lib/db";
|
||||
|
||||
const CHECKPOINT = new Date("2026-07-30T08:15:00.000Z");
|
||||
|
||||
type SyncStateBody = {
|
||||
lastSyncedAt: string | null;
|
||||
serverNow: string;
|
||||
timezone: string;
|
||||
};
|
||||
|
||||
async function callGet(): Promise<SyncStateBody> {
|
||||
const res = (await (
|
||||
GET as unknown as (req: unknown) => Promise<{ data: SyncStateBody }>
|
||||
)({})) as { data: SyncStateBody };
|
||||
return res.data;
|
||||
}
|
||||
|
||||
describe("GET /api/sync/state", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(prisma.measurement.findFirst).mockResolvedValue(null as never);
|
||||
vi.mocked(prisma.moodEntry.findFirst).mockResolvedValue(null as never);
|
||||
vi.mocked(prisma.medicationIntakeEvent.findFirst).mockResolvedValue(
|
||||
null as never,
|
||||
);
|
||||
vi.mocked(prisma.measurement.count).mockResolvedValue(0 as never);
|
||||
vi.mocked(prisma.moodEntry.count).mockResolvedValue(0 as never);
|
||||
vi.mocked(prisma.medicationIntakeEvent.count).mockResolvedValue(0 as never);
|
||||
vi.mocked(prisma.user.findUnique).mockResolvedValue({
|
||||
lastSyncedAt: CHECKPOINT,
|
||||
timezone: "Europe/Berlin",
|
||||
} as never);
|
||||
});
|
||||
|
||||
it("writes nothing — the handshake never touches the user row", async () => {
|
||||
await callGet();
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("reports the stored checkpoint rather than the previous handshake", async () => {
|
||||
const body = await callGet();
|
||||
expect(body.lastSyncedAt).toBe(CHECKPOINT.toISOString());
|
||||
});
|
||||
|
||||
it("keeps reporting the same checkpoint across repeated calls", async () => {
|
||||
const first = await callGet();
|
||||
const second = await callGet();
|
||||
expect(second.lastSyncedAt).toBe(first.lastSyncedAt);
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("reports a never-synced account as null so the client re-pairs full", async () => {
|
||||
vi.mocked(prisma.user.findUnique).mockResolvedValue({
|
||||
lastSyncedAt: null,
|
||||
timezone: "Europe/Berlin",
|
||||
} as never);
|
||||
const body = await callGet();
|
||||
expect(body.lastSyncedAt).toBeNull();
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -12,7 +12,9 @@
|
||||
* {
|
||||
* userId,
|
||||
* timezone,
|
||||
* lastSyncedAt, — User.lastSyncedAt (ISO string or null)
|
||||
* lastSyncedAt, — User.lastSyncedAt (ISO string or null).
|
||||
* Set by the ingest boundaries, not here:
|
||||
* this route is a read and writes nothing.
|
||||
* serverNow, — `new Date().toISOString()` for clock skew
|
||||
* measurements: { lastUpdatedAt, liveCount, tombstonedCount },
|
||||
* mood: { lastUpdatedAt, liveCount, tombstonedCount },
|
||||
@@ -45,10 +47,10 @@ import {
|
||||
export const GET = apiHandler(async (_request: NextRequest) => {
|
||||
const { user } = await requireAuth();
|
||||
|
||||
// Per-domain aggregate reads + one user-side bump. Each runs against
|
||||
// indexed columns (userId, deleted_at / updated_at) so it stays
|
||||
// O(rows-for-user) and not O(rows-across-tenants). v1.7.0 adds the
|
||||
// mood + intake blocks now that both carry the sync columns.
|
||||
// Per-domain aggregate reads. Each runs against indexed columns (userId,
|
||||
// deleted_at / updated_at) so it stays O(rows-for-user) and not
|
||||
// O(rows-across-tenants). v1.7.0 adds the mood + intake blocks now that
|
||||
// both carry the sync columns.
|
||||
const [
|
||||
measurementLatest,
|
||||
measurementLive,
|
||||
@@ -96,16 +98,14 @@ export const GET = apiHandler(async (_request: NextRequest) => {
|
||||
}),
|
||||
]);
|
||||
|
||||
// The handshake also bumps the user's lastSyncedAt — the call IS
|
||||
// the handshake. iOS reads the OLD lastSyncedAt from the response
|
||||
// and then trusts that subsequent server writes after the new
|
||||
// checkpoint will round-trip via the standard read paths.
|
||||
const previous = currentUser?.lastSyncedAt ?? null;
|
||||
const nextCheckpoint = new Date();
|
||||
await prisma.user.update({
|
||||
where: { id: user.id },
|
||||
data: { lastSyncedAt: nextCheckpoint },
|
||||
});
|
||||
// The handshake reports the checkpoint; it does not set it. The column is
|
||||
// stamped by the ingest boundaries that actually move data (see
|
||||
// `markSyncCheckpoint`), so `lastSyncedAt` answers "when did this account
|
||||
// last push a batch" rather than "when did a client last ask". NULL still
|
||||
// means never synced, and the value can now only trail the truth — the
|
||||
// handshake used to advance it past a window the client had not drained.
|
||||
const lastSyncedAt = currentUser?.lastSyncedAt ?? null;
|
||||
const serverNow = new Date();
|
||||
|
||||
annotate({
|
||||
action: { name: "sync.state" },
|
||||
@@ -114,15 +114,15 @@ export const GET = apiHandler(async (_request: NextRequest) => {
|
||||
tombstonedCount: measurementTomb,
|
||||
moodLiveCount: moodLive,
|
||||
intakeLiveCount: intakeLive,
|
||||
lastSyncedAtBefore: previous?.toISOString() ?? null,
|
||||
lastSyncedAt: lastSyncedAt?.toISOString() ?? null,
|
||||
},
|
||||
});
|
||||
|
||||
return apiSuccess({
|
||||
userId: user.id,
|
||||
timezone: currentUser?.timezone ?? "Europe/Berlin",
|
||||
lastSyncedAt: previous?.toISOString() ?? null,
|
||||
serverNow: nextCheckpoint.toISOString(),
|
||||
lastSyncedAt: lastSyncedAt?.toISOString() ?? null,
|
||||
serverNow: serverNow.toISOString(),
|
||||
measurements: {
|
||||
lastUpdatedAt: measurementLatest?.updatedAt?.toISOString() ?? null,
|
||||
liveCount: measurementLive,
|
||||
|
||||
Reference in new issue
Block a user