diff --git a/src/app/api/anamnesis/facts/__tests__/idempotency-conflict.test.ts b/src/app/api/anamnesis/facts/__tests__/idempotency-conflict.test.ts new file mode 100644 index 000000000..435f35ef2 --- /dev/null +++ b/src/app/api/anamnesis/facts/__tests__/idempotency-conflict.test.ts @@ -0,0 +1,136 @@ +/** + * The bytes a client actually receives from an idempotency 409. + * + * `POST /api/anamnesis/facts` is the route the drift was reported against + * (iOS #75), but nothing here is specific to it — the 409 comes from + * `withIdempotency`, so every route under that wrapper serves the same + * envelope. Until v1.35.1 both in-flight arms sent `error` as + * `{ message: … }`, the only object-shaped error the app emitted and not the + * `{ data, error: }` envelope the published contract promises. + * + * This runs the REAL chain — the real route module, the real `apiHandler`, + * the real `withIdempotency` — with only the data layer and the session + * mocked, and asserts on the parsed response body. A schema test would not + * have caught this: the schema was already right. + * + * Both 409 arms are covered: + * - a `PENDING_STATUS` row already in the table (the in-flight lookup); + * - a lost claim race (the row appears between the lookup and the insert). + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.stubEnv("ENCRYPTION_KEYS", ""); +vi.stubEnv("ENCRYPTION_ACTIVE_KEY_ID", ""); +vi.stubEnv("ENCRYPTION_KEY", "0".repeat(64)); + +const mocks = vi.hoisted(() => ({ + findUnique: vi.fn(), + create: vi.fn(), + deleteMany: vi.fn(), + updateMany: vi.fn(), + getSession: vi.fn(), + createFact: vi.fn(), + factFindFirst: vi.fn(), +})); + +vi.mock("@/lib/db", () => ({ + prisma: { + idempotencyKey: { + findUnique: mocks.findUnique, + create: mocks.create, + deleteMany: mocks.deleteMany, + updateMany: mocks.updateMany, + delete: vi.fn(), + update: vi.fn(), + }, + apiToken: { findUnique: vi.fn() }, + healthProfileFactRevision: { findFirst: mocks.factFindFirst }, + }, +})); +vi.mock("@/lib/auth/session", () => ({ getSession: mocks.getSession })); +vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn() })); +vi.mock("@/lib/profile/health-facts", () => ({ + readHealthProfileFacts: vi.fn(), + createHealthProfileFact: mocks.createFact, + correctHealthProfileFact: vi.fn(), + removeHealthProfileFact: vi.fn(), +})); + +import { NextRequest } from "next/server"; +import { POST } from "../route"; + +/** The pending sentinel `findCached` recognises as "another call is running". */ +const PENDING_STATUS = 0; + +function request(): NextRequest { + return new NextRequest("http://localhost/api/anamnesis/facts", { + method: "POST", + headers: { + "content-type": "application/json", + "idempotency-key": "key-in-flight", + }, + body: JSON.stringify({ kind: "SMOKING_STATUS", value: "FORMER" }), + }); +} + +beforeEach(() => { + vi.clearAllMocks(); + mocks.getSession.mockResolvedValue({ + user: { id: "user-1", role: "USER" }, + }); + mocks.findUnique.mockResolvedValue(null); + mocks.create.mockResolvedValue({}); + mocks.deleteMany.mockResolvedValue({ count: 0 }); + mocks.updateMany.mockResolvedValue({ count: 1 }); + mocks.factFindFirst.mockResolvedValue(null); + mocks.createFact.mockResolvedValue({ id: "fact-1" }); +}); + +describe("idempotency 409 envelope, over the real route", () => { + it("serves `error` as a string when a request is already in flight", async () => { + mocks.findUnique.mockResolvedValue({ + id: "idem-1", + responseStatus: PENDING_STATUS, + responseBody: "", + expiresAt: new Date(Date.now() + 60_000), + }); + + const response = await POST(request()); + const body = await response.json(); + + expect(response.status).toBe(409); + expect(body.data).toBeNull(); + expect(typeof body.error).toBe("string"); + expect(body.error).toBe( + "A request with this Idempotency-Key is already in progress", + ); + // The shape the drift produced, named explicitly so a revert cannot + // pass by satisfying only the `typeof` check above. + expect(body.error).not.toHaveProperty("message"); + expect(response.headers.get("X-Idempotent-Replay")).toBe("false"); + // The side effect must not have run on a refused call. + expect(mocks.createFact).not.toHaveBeenCalled(); + }); + + it("serves `error` as a string when the claim race is lost", async () => { + // No row on lookup, but the claiming insert collides with the racing + // request that got there first. + mocks.findUnique.mockResolvedValue(null); + mocks.create.mockRejectedValue( + Object.assign(new Error("Unique constraint failed"), { code: "P2002" }), + ); + + const response = await POST(request()); + const body = await response.json(); + + expect(response.status).toBe(409); + expect(body.data).toBeNull(); + expect(typeof body.error).toBe("string"); + expect(body.error).toBe( + "A request with this Idempotency-Key is already in progress", + ); + expect(body.error).not.toHaveProperty("message"); + expect(response.headers.get("X-Idempotent-Replay")).toBe("false"); + expect(mocks.createFact).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/dashboard-layout.ts b/src/lib/dashboard-layout.ts index 59df602fb..0b97c2902 100644 --- a/src/lib/dashboard-layout.ts +++ b/src/lib/dashboard-layout.ts @@ -406,9 +406,12 @@ function coerceChartOverlayPrefsMap(value: unknown): ChartOverlayPrefsMap { * * - `READINESS` / `RECOVERY_SCORE` / `SLEEP_SCORE` resolve through the * derived registry's engines (module-gated like the derived routes); - * - `MED_COMPLIANCE` is the pooled 7-day medication adherence from the - * canonical compliance engine — the ring that absorbs the retired - * hero dose row's information role. + * - `MED_COMPLIANCE` is TODAY's dose progress (taken of scheduled) from + * the snapshot's own `medsToday` block — the ring that absorbs the + * retired hero dose row's information role. The v1.27.7 first cut put + * the pooled 7-day adherence percentage here; it has been today's + * progress since v1.27.8. Resolution lives in + * `@/lib/dashboard/score-rings`. * * The preference piggy-backs on the layout blob like `chartOverlayPrefs` * (a UI affordance, no Prisma migration). The resolver drops unknown ids, diff --git a/src/lib/idempotency.ts b/src/lib/idempotency.ts index 5c14a477c..04d92f8d6 100644 --- a/src/lib/idempotency.ts +++ b/src/lib/idempotency.ts @@ -329,6 +329,31 @@ export async function defaultUserIdResolver(): Promise { * * No-op when the header is missing or the value is malformed. */ +/** + * The 409 both in-flight arms return. + * + * `error` is a STRING, matching the envelope `@/lib/api-response` defines + * for every other route (`{ data: null, error: , meta? }`). Until + * v1.35.1 these two literals sent `error` as `{ message }` — the only + * object-shaped error the app ever emitted, inherited by every route under + * `withIdempotency`, and never what the published contract promised. A + * client decoding the documented envelope now gets what it was generated + * against. + * + * Built once rather than twice so the two arms cannot drift apart again. + * The response is hand-built rather than routed through `apiError` because + * the 409 carries the `X-Idempotent-Replay` header. + */ +function inflightConflictResponse(): Response { + return NextResponse.json( + { + data: null, + error: "A request with this Idempotency-Key is already in progress", + }, + { status: 409, headers: { "X-Idempotent-Replay": "false" } }, + ); +} + export function withIdempotency< Args extends [Request | NextRequest, ...unknown[]], >( @@ -368,16 +393,7 @@ export function withIdempotency< action: { name: "idempotency.inflight_conflict" }, meta: { method: ctx.method, path: ctx.path }, }); - return NextResponse.json( - { - data: null, - error: { - message: - "A request with this Idempotency-Key is already in progress", - }, - }, - { status: 409, headers: { "X-Idempotent-Replay": "false" } }, - ); + return inflightConflictResponse(); } // Claim the key before running the handler. If a racing request beats @@ -389,16 +405,7 @@ export function withIdempotency< action: { name: "idempotency.inflight_conflict" }, meta: { method: ctx.method, path: ctx.path }, }); - return NextResponse.json( - { - data: null, - error: { - message: - "A request with this Idempotency-Key is already in progress", - }, - }, - { status: 409, headers: { "X-Idempotent-Replay": "false" } }, - ); + return inflightConflictResponse(); } let response: Response; diff --git a/src/lib/openapi/__tests__/derived-type-enum-sync.test.ts b/src/lib/openapi/__tests__/derived-type-enum-sync.test.ts new file mode 100644 index 000000000..bee872ded --- /dev/null +++ b/src/lib/openapi/__tests__/derived-type-enum-sync.test.ts @@ -0,0 +1,73 @@ +/** + * The `type` query parameter on `GET /api/insights/derived` sub-targets the + * single measurement type a baseline metric works over. Two engines define + * one: `VITALS_BASELINE` over a vital, `SAME_TIME_BASELINE` over a cumulative + * day metric. For most of a year the published enum carried the vitals alone, + * so a spec-conformant client could not send the four same-time types at all + * (iOS #75). + * + * This guard asserts the published parameter enumerates exactly the union of + * the two registry sets, read out of the assembled OpenAPI document rather + * than out of the schema module — the same object the generator writes to + * `docs/api/openapi.yaml`, so a value that reaches the registry but not the + * contract fails here. + */ +import { describe, expect, it } from "vitest"; + +import { buildOpenApiDocument } from "../registry"; +import { + VITALS_BASELINE_TYPES, + SAME_TIME_BASELINE_TYPES, +} from "@/lib/insights/derived/registry"; + +/** Pull the `type` query parameter's enum out of the assembled document. */ +function publishedTypeEnum(): string[] { + const doc = buildOpenApiDocument() as { + paths?: Record< + string, + { get?: { parameters?: unknown[]; requestParams?: unknown } } + >; + }; + const operation = doc.paths?.["/api/insights/derived"]?.get; + expect( + operation, + "GET /api/insights/derived is missing from the document", + ).toBeDefined(); + + const parameters = (operation?.parameters ?? []) as Array<{ + name?: string; + in?: string; + schema?: { enum?: string[] }; + }>; + const typeParam = parameters.find( + (p) => p.name === "type" && p.in === "query", + ); + expect(typeParam, "the `type` query parameter is missing").toBeDefined(); + + const values = typeParam?.schema?.enum; + expect(Array.isArray(values), "`type` is not published as an enum").toBe( + true, + ); + return values as string[]; +} + +describe("derived `type` parameter stays in sync with the registry", () => { + it("publishes exactly the union of the two baseline type sets", () => { + const expected = [ + ...VITALS_BASELINE_TYPES, + ...SAME_TIME_BASELINE_TYPES, + ].map(String); + + expect([...publishedTypeEnum()].sort()).toEqual([...expected].sort()); + }); + + it("publishes every same-time baseline type", () => { + // Named separately: this is the half that was missing, and a regression + // here is the one a client feels immediately (a 422-shaped dead end on + // a value the server accepts). + const published = publishedTypeEnum(); + for (const type of SAME_TIME_BASELINE_TYPES) { + expect(published).toContain(String(type)); + } + }); +}); diff --git a/src/lib/openapi/routes/insights/schemas.ts b/src/lib/openapi/routes/insights/schemas.ts index bd7aae6f1..d4b4ef53f 100644 --- a/src/lib/openapi/routes/insights/schemas.ts +++ b/src/lib/openapi/routes/insights/schemas.ts @@ -14,6 +14,7 @@ import { METRIC_STATUS_IDS } from "@/lib/insights/metric-status-registry"; import { DERIVED_METRIC_IDS, VITALS_BASELINE_TYPES, + SAME_TIME_BASELINE_TYPES, } from "@/lib/insights/derived/registry"; import { ANALYTICS_RANGES } from "@/lib/analytics/range-delta"; @@ -108,8 +109,26 @@ export const metricStatusResponse = z // v1.10.0 — generic derived-wellness-metric route. The query enum is // derived from the same registry the route validates against, so spec + -// route + cache scope cannot drift. `type` sub-targets the single vital -// a baseline metric (VITALS_BASELINE) bands over. +// route + cache scope cannot drift. `type` sub-targets the single +// measurement type a baseline metric works over. + +/** + * Every value the `type` query parameter accepts, assembled from the two + * registry sets that define one: `VITALS_BASELINE` bands a single vital, + * `SAME_TIME_BASELINE` compares one cumulative day metric against the same + * hour of a typical day. Published as one enum because `type` is a single + * parameter shared by both metrics — a client picks the subset that belongs + * to the `metric` it names. + * + * Built from the registry rather than restated here, so a type added to + * either engine reaches the published contract in the same commit. + * `derived-type-enum-sync.test.ts` pins the equality. + */ +export const DERIVED_TYPE_PARAM_VALUES: string[] = [ + ...VITALS_BASELINE_TYPES, + ...SAME_TIME_BASELINE_TYPES, +]; + export const derivedMetricQuery = z .object({ metric: z @@ -118,10 +137,10 @@ export const derivedMetricQuery = z "Derived-metric id to compute (e.g. VITALS_BASELINE, FITNESS_AGE, VASCULAR_AGE_DELTA, HRV_BALANCE, BMI, READINESS). Closed enum: an unknown id 422s. Metrics whose compute has not yet landed return an `insufficient` value with reason `not_implemented`.", ), type: z - .enum(VITALS_BASELINE_TYPES as [string, ...string[]]) + .enum(DERIVED_TYPE_PARAM_VALUES as [string, ...string[]]) .optional() .describe( - "For VITALS_BASELINE only — the single vital to band (defaults to RESTING_HEART_RATE). Ignored by composites. An unsupported value yields an `insufficient` value rather than a 422 so iOS metric combinations stay forgiving.", + "The single measurement type a baseline metric works over. `VITALS_BASELINE` takes one of the eleven vitals (`RESTING_HEART_RATE` through `WEIGHT`; defaults to `RESTING_HEART_RATE`). `SAME_TIME_BASELINE` takes one of the four cumulative day metrics — `ACTIVITY_STEPS`, `ACTIVE_ENERGY_BURNED`, `WALKING_RUNNING_DISTANCE`, `FLIGHTS_CLIMBED`. Ignored by the composite metrics. A type the named metric does not support yields an `insufficient` value rather than a 422, so client metric combinations stay forgiving.", ), }) .meta({ id: "DerivedMetricQuery" }); @@ -253,7 +272,7 @@ export const derivedBatchQuery = z .min(1) .max(1024) .describe( - "Comma-separated derived-metric tokens. Each is a `` or `:` (the colon sub-targets a VITALS_BASELINE vital). An unknown id 422s; a `type` outside the MeasurementType enum 422s; at most 24 tokens; duplicates collapse.", + "Comma-separated derived-metric tokens. Each is a `` or `:` (the colon sub-targets the single measurement type a baseline metric works over — a vital for `VITALS_BASELINE`, a cumulative day metric for `SAME_TIME_BASELINE`; see the `type` parameter on `GET /api/insights/derived`). An unknown id 422s; a `type` outside the MeasurementType enum 422s; at most 24 tokens; duplicates collapse.", ), }) .meta({ id: "DerivedBatchQuery" });