From 6f3b901c3ec35fd8557aa699fee980af860ab062 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 14 May 2026 22:27:32 +0200 Subject: [PATCH] fix(workout): reject endedAt <= startedAt + zero-duration PR guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A workout with `endedAt <= startedAt` produced a non-positive `durationSec` that downstream consumers treated as a real zero — worst case a fastest-5km PR of zero seconds, which would lock in until manual cleanup. Add a `.superRefine` clause on `createWorkoutSchema` so the ingest gate refuses the row. The PR detector stays defensive against any pre-gate row or future code path that bypasses the schema: MIN-direction slots now always filter `durationSec > 0` regardless of which field carries the PR value. A MIN slot sorts ascending, so the smallest value wins — a zero-duration row would always be picked first. Fix-M / code-M2. --- .../__tests__/pr-detection-worker.test.ts | 38 +++++++++++++++++++ .../personal-records/pr-detection-worker.ts | 15 ++++++++ src/lib/validations/__tests__/workout.test.ts | 25 ++++++++++++ src/lib/validations/workout.ts | 13 +++++++ 4 files changed, 91 insertions(+) diff --git a/src/lib/personal-records/__tests__/pr-detection-worker.test.ts b/src/lib/personal-records/__tests__/pr-detection-worker.test.ts index 62ef2e065..ce22475c5 100644 --- a/src/lib/personal-records/__tests__/pr-detection-worker.test.ts +++ b/src/lib/personal-records/__tests__/pr-detection-worker.test.ts @@ -595,6 +595,44 @@ describe("detectPersonalRecordsForUser — workout slots", () => { }); }); +describe("detectPersonalRecordsForUser — zero-duration MIN guard", () => { + // Regression for Fix-M / code-M2: the createWorkoutSchema gate now + // rejects endedAt <= startedAt, but the PR detector stays defensive + // against any historical row (pre-gate) or future code path that + // produces a zero-duration workout. A MIN-direction slot (fastest + // 5 km) sorts ascending — a zero-second row would always win. + it("ignores zero-duration runs when scanning fastest_5km_time", async () => { + const runs: FakeWorkoutRow[] = []; + // Seed: enough legitimate 5 km finishes to clear the warm-up gate. + for (let i = 0; i < PR_DETECTION_WARMUP_THRESHOLD; i++) { + runs.push( + workout("running", 1800 + i * 30, 5100 + i * 100, i + 2, `5k-${i}`), + ); + } + // Poison row: a workout with durationSec === 0 at the same + // distance threshold. A MIN-direction selector without the guard + // would pick this row as "fastest". + runs.push(workout("running", 0, 5200, 1, "zero-poison")); + + const state = { + measurements: [], + workouts: runs, + personalRecords: [] as FakePersonalRecordRow[], + }; + const prisma = makeFakePrisma(state); + + await detectPersonalRecordsForUser(USER, { prisma }); + + const pr = state.personalRecords.find( + (r) => r.metricSlot === "fastest_5km_time", + ); + expect(pr).toBeDefined(); + // The PR must come from the seeded set, not the poison row. + expect(pr?.value).toBeGreaterThan(0); + expect(pr?.value).toBe(1800); + }); +}); + describe("detectPersonalRecordsForUser — flags + drift guard", () => { it("propagates the silent flag onto the result envelope", async () => { const state = { diff --git a/src/lib/personal-records/pr-detection-worker.ts b/src/lib/personal-records/pr-detection-worker.ts index d7770fec7..dacd05370 100644 --- a/src/lib/personal-records/pr-detection-worker.ts +++ b/src/lib/personal-records/pr-detection-worker.ts @@ -290,12 +290,27 @@ export async function detectPersonalRecordsForUser( for (const slot of WORKOUT_SLOTS) { scanned += 1; + // MIN-direction slots sort ascending — the smallest value wins. A + // zero-duration workout would always become "best" before any + // legitimate row could compete. The schema gate on + // `createWorkoutSchema` rejects endedAt <= startedAt at ingest + // time, but the worker stays defensive: any historical row that + // pre-dates the gate (or any future code path that bypasses the + // schema) must not be allowed to lock in a zero-second PR. The + // `durationSec > 0` clause is always present when `slot.field` is + // durationSec; MIN-direction slots additionally enforce it + // regardless of which field carries the PR value, so a hypothetical + // future MIN slot on a distance field still rejects zero-duration + // rows on principle. const baseWhere = { userId, sportType: slot.sportType, ...(slot.minDistanceM !== undefined ? { totalDistanceM: { gte: slot.minDistanceM } } : {}), + ...(slot.direction === PersonalRecordDirection.MIN + ? { durationSec: { gt: 0 } as { gt: number } } + : {}), [slot.field]: { gt: 0 } as { gt: number }, } as const; diff --git a/src/lib/validations/__tests__/workout.test.ts b/src/lib/validations/__tests__/workout.test.ts index 98d8b1a7b..557f63f33 100644 --- a/src/lib/validations/__tests__/workout.test.ts +++ b/src/lib/validations/__tests__/workout.test.ts @@ -190,6 +190,31 @@ describe("createWorkoutSchema", () => { ).toThrow(/20000-point cap/); }); + it("rejects a workout whose endedAt is before startedAt", () => { + // Regression for the W16b ingest gate: a reversed pair produced a + // negative `durationSec` that downstream consumers (PR detector, + // weekly report) treated as a real zero — a fastest-5km PR of + // zero seconds would lock in until manual cleanup. The schema + // refuses the row at parse time. + expect(() => + createWorkoutSchema.parse({ + ...minimalRun, + startedAt: "2026-05-14T07:15:00.000Z", + endedAt: "2026-05-14T06:30:00.000Z", + }), + ).toThrow(/endedAt must be strictly after startedAt/); + }); + + it("rejects a zero-duration workout (endedAt === startedAt)", () => { + expect(() => + createWorkoutSchema.parse({ + ...minimalRun, + startedAt: "2026-05-14T06:30:00.000Z", + endedAt: "2026-05-14T06:30:00.000Z", + }), + ).toThrow(/endedAt must be strictly after startedAt/); + }); + it("rejects a route whose sampleTimestamps length mismatches coordinates", () => { // Cross-field invariant: per-sample HR / speed must line up against // the parallel coordinate index, so a desynced pair silently diff --git a/src/lib/validations/workout.ts b/src/lib/validations/workout.ts index 15a440d97..5b8f35c46 100644 --- a/src/lib/validations/workout.ts +++ b/src/lib/validations/workout.ts @@ -97,6 +97,12 @@ export type WorkoutRouteSamples = z.infer; * locked contract and the v1.4.26 XML import worker has the same target. * * Cross-field invariants enforced via `.superRefine()`: + * - `endedAt` MUST be strictly greater than `startedAt`. A workout + * with `endedAt <= startedAt` produces a non-positive + * `durationSec`, which downstream consumers (PR detector, weekly + * report, dashboards) treat as a "real" zero — locking in a + * fastest-5km time of zero seconds is far worse than refusing the + * ingest at the schema gate. * - When `route.sampleTimestamps` is present, its length MUST equal * `route.geometry.coordinates.length`. A desynced pair silently * degrades downstream analytics (per-sample HR / speed would line @@ -127,6 +133,13 @@ export const createWorkoutSchema = z .optional(), }) .superRefine((value, ctx) => { + if (value.endedAt.getTime() <= value.startedAt.getTime()) { + ctx.addIssue({ + code: "custom", + path: ["endedAt"], + message: "endedAt must be strictly after startedAt", + }); + } const samples = value.route?.sampleTimestamps; const coords = value.route?.geometry.coordinates; if (samples && coords && samples.length !== coords.length) {