From 040b430e7348da8f16a6a69e2dc1db3f02b6a05d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 07:40:05 +0200 Subject: [PATCH] fix(api): accept an Idempotency-Key on the titration step and the custom symptom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more creates the client replays out of its offline outbox ran the handler again on the replay. A titration step carries no natural key at all, so a lost success response left the medication with two dose changes on the same day and the runway estimate reading off the wrong one. A custom cycle symptom is unique on its label, so the replay did not duplicate — it answered the 409 that uniqueness raises, reporting a conflict the client cannot act on for a row it has no record of creating. Both wrap in `withIdempotency`, the composition their siblings already ship, and both operations publish the header and the in-flight 409 so a client generated against the contract can tell a retry is safe. The integration file that drives the medication creates twice against real Postgres gains the titration case: one dose-change row after the replay, and a replayed 201 carrying `X-Idempotent-Replay: true`. `POST /api/mcp/tokens` is deliberately not on this list. Its response carries a freshly minted `hlk_` credential, and the wrapper refuses to persist a secret-shaped body — publishing the header there would promise a replay the mechanism will never perform. --- docs/api/openapi.yaml | 73 +++--- src/app/api/cycle/symptoms/custom/route.ts | 16 +- src/app/api/medications/[id]/glp1/route.ts | 214 +++++++++--------- src/lib/openapi/routes/cycle.ts | 2 + src/lib/openapi/routes/medications/paths.ts | 2 + .../medication-write-idempotency.test.ts | 44 +++- 6 files changed, 211 insertions(+), 140 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 264c54a16..3a2f547d0 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -152,6 +152,22 @@ paths: burns their own allowance rather than locking the owner out and cannot collect a fresh one by switching records. Audits as `cycle.symptom.custom.create` with the icon only — never the label. Delegable at MANAGE level over the `cycle` section: the record's own symptom vocabulary is what the day-log writes need in order to say anything." + parameters: + - &a6 + name: Idempotency-Key + in: header + required: false + schema: + type: string + pattern: ^[A-Za-z0-9_\-:.]{8,128}$ + minLength: 8 + maxLength: 128 + description: "Makes this write safe to retry. Send the same key again and the first response is replayed rather than the + work repeated, marked with `X-Idempotent-Replay: true`. A key that does not match the pattern is IGNORED + rather than refused — the write then proceeds unprotected and looks exactly like a request that carried no + key, so validate the key on your side. Scoped per user and per (method, path): the same key on a different + route is a different key. While a first request under this key is still in flight, a second is refused with + 409 rather than queued." requestBody: required: true content: @@ -167,6 +183,25 @@ paths: $ref: "#/components/schemas/CycleCustomSymptomCreatedEnvelope" "401": *a1 "403": *a2 + "409": &a7 + description: "A request with this `Idempotency-Key` is already in progress. Nothing was written by this call. Retry + after a short delay; the response carries `X-Idempotent-Replay: false` to separate it from a replay." + headers: + X-Idempotent-Replay: + required: true + description: "`true` when this response was replayed from a stored one rather than produced now; `false` on the 409 that + refuses a duplicate still in flight. Absent when the request carried no usable key." + schema: + type: string + enum: + - "true" + - "false" + description: "`true` when this response was replayed from a stored one rather than produced now; `false` on the 409 that + refuses a duplicate still in flight. Absent when the request carried no usable key." + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorEnvelope" "422": description: Either the body failed validation — the multi-issue envelope with `meta.errorCode` = `cycle.symptom.custom.invalid` (an empty or over-40-character label, an icon outside the allow-list, a @@ -332,21 +367,7 @@ paths: description: "Upserts on `(userId, source, externalId)` when externalId present, else `(userId, date)`. `note` encrypts at rest. 201 on insert, 200 on update. Gated: `cycle.disabled` 403 when the feature is off." parameters: - - &a6 - name: Idempotency-Key - in: header - required: false - schema: - type: string - pattern: ^[A-Za-z0-9_\-:.]{8,128}$ - minLength: 8 - maxLength: 128 - description: "Makes this write safe to retry. Send the same key again and the first response is replayed rather than the - work repeated, marked with `X-Idempotent-Replay: true`. A key that does not match the pattern is IGNORED - rather than refused — the write then proceeds unprotected and looks exactly like a request that carried no - key, so validate the key on your side. Scoped per user and per (method, path): the same key on a different - route is a different key. While a first request under this key is still in flight, a second is refused with - 409 rather than queued." + - *a6 requestBody: required: true content: @@ -368,25 +389,7 @@ paths: $ref: "#/components/schemas/CycleDayLogCreatedEnvelope" "401": *a1 "403": *a2 - "409": &a7 - description: "A request with this `Idempotency-Key` is already in progress. Nothing was written by this call. Retry - after a short delay; the response carries `X-Idempotent-Replay: false` to separate it from a replay." - headers: - X-Idempotent-Replay: - required: true - description: "`true` when this response was replayed from a stored one rather than produced now; `false` on the 409 that - refuses a duplicate still in flight. Absent when the request carried no usable key." - schema: - type: string - enum: - - "true" - - "false" - description: "`true` when this response was replayed from a stored one rather than produced now; `false` on the 409 that - refuses a duplicate still in flight. Absent when the request carried no usable key." - content: - application/json: - schema: - $ref: "#/components/schemas/ErrorEnvelope" + "409": *a7 "422": *a4 "429": *a5 /api/cycle/day-logs/{id}: @@ -4550,6 +4553,7 @@ paths: schema: type: string required: true + - *a6 requestBody: required: true content: @@ -4613,6 +4617,7 @@ paths: application/json: schema: $ref: "#/components/schemas/ErrorEnvelope" + "409": *a7 "422": *a4 "429": *a5 /api/medications/{id}/api-endpoint: diff --git a/src/app/api/cycle/symptoms/custom/route.ts b/src/app/api/cycle/symptoms/custom/route.ts index 798a4a7e3..a716de0d7 100644 --- a/src/app/api/cycle/symptoms/custom/route.ts +++ b/src/app/api/cycle/symptoms/custom/route.ts @@ -26,6 +26,7 @@ import { } from "@/lib/api-response"; import { apiHandler, requireRecordAuth } from "@/lib/api-handler"; import { annotate } from "@/lib/logging/context"; +import { withIdempotency } from "@/lib/idempotency"; import { auditLog } from "@/lib/auth/audit"; import { checkRateLimit } from "@/lib/rate-limit"; import { requireCycleEnabled } from "@/lib/cycle/gate"; @@ -67,7 +68,18 @@ export const GET = apiHandler(async () => { return apiSuccess({ symptoms }); }); -export const POST = apiHandler(async (request: NextRequest) => { +/** + * Wrapped in `withIdempotency`: a custom symptom is minted from the client's + * offline outbox under the same `Idempotency-Key`, and a replay after a lost + * success response would otherwise answer the 409 the label's unique key + * raises rather than the 201 the first attempt produced — a create the client + * has no record of, reported as a conflict it cannot act on. + */ +export const POST = apiHandler( + withIdempotency<[NextRequest]>(postCustomSymptom), +); + +async function postCustomSymptom(request: NextRequest): Promise { // v1.37.0 — MANAGE. The record's own symptom vocabulary, which the day-log // writes the level admits need in order to say anything. const { user, actor } = await requireRecordAuth("manage", "cycle"); @@ -151,4 +163,4 @@ export const POST = apiHandler(async (request: NextRequest) => { }, 201, ); -}); +} diff --git a/src/app/api/medications/[id]/glp1/route.ts b/src/app/api/medications/[id]/glp1/route.ts index 180badac9..46a18ac2e 100644 --- a/src/app/api/medications/[id]/glp1/route.ts +++ b/src/app/api/medications/[id]/glp1/route.ts @@ -28,6 +28,7 @@ import { import { checkRateLimit, rateLimitHeaders } from "@/lib/rate-limit"; import { glp1PostBodySchema } from "@/lib/validations/medication"; import { assertMedicationOwnership } from "@/lib/medications/route-guards"; +import { withIdempotency } from "@/lib/idempotency"; import { estimateDailyDoseCount, estimateRunwayDays, @@ -236,103 +237,71 @@ export const GET = apiHandler( * Rate-limit: 30/min/user (same as sibling POST routes). * Audit: `medication.glp1.update` for every successful mutation. */ +/** + * Wrapped in `withIdempotency`: a titration step is replayed out of the + * client's offline outbox under the same `Idempotency-Key`, and a replay after + * a lost success response would otherwise write a second dose change or a + * second supply delta. Neither row carries a natural key, so the wrapper is the + * only thing that can collapse the retry. + */ export const POST = apiHandler( - async (request: NextRequest, { params }: RouteParams) => { - // v1.37.0 — MANAGE. A titration step is record data and the dose is what - // the server acts on; administering one is the case the level names. - const { user, actor } = await requireRecordAuth("manage", "medications"); - const { id } = await params; + withIdempotency<[NextRequest, RouteParams]>(postGlp1Update), +); - const guard = await assertMedicationOwnership(id, user.id); - if (guard) return guard; +async function postGlp1Update( + request: NextRequest, + { params }: RouteParams, +): Promise { + // v1.37.0 — MANAGE. A titration step is record data and the dose is what + // the server acts on; administering one is the case the level names. + const { user, actor } = await requireRecordAuth("manage", "medications"); + const { id } = await params; - // Per-caller POST rate-limit — matches the 30/min sibling routes - // (inventory, side-effects). Generous for a hand-driven session, - // tight enough to cut off the spam case. - // - // v1.37.0 — C1: keyed on the ACTOR, so a manager burns their own - // allowance and cannot collect a fresh one by switching records. - const rl = await checkRateLimit( - `medication-glp1:post:${actor.id}`, - POST_RATE_LIMIT, - POST_WINDOW_MS, - ); - if (!rl.allowed) { - return apiError("Too many requests", 429, { - headers: rateLimitHeaders(rl), - }); - } + const guard = await assertMedicationOwnership(id, user.id); + if (guard) return guard; - const { data: body, error: jsonError } = await safeJson(request, { - maxBytes: 64 * 1024, + // Per-caller POST rate-limit — matches the 30/min sibling routes + // (inventory, side-effects). Generous for a hand-driven session, + // tight enough to cut off the spam case. + // + // v1.37.0 — C1: keyed on the ACTOR, so a manager burns their own + // allowance and cannot collect a fresh one by switching records. + const rl = await checkRateLimit( + `medication-glp1:post:${actor.id}`, + POST_RATE_LIMIT, + POST_WINDOW_MS, + ); + if (!rl.allowed) { + return apiError("Too many requests", 429, { + headers: rateLimitHeaders(rl), }); - if (jsonError) return jsonError; + } - const parsed = glp1PostBodySchema.safeParse(body); - if (!parsed.success) { - // v1.4.43 W6 — multi-issue 422. - return returnAllZodIssues(parsed.error, 422); - } + const { data: body, error: jsonError } = await safeJson(request, { + maxBytes: 64 * 1024, + }); + if (jsonError) return jsonError; - const ip = getClientIp(request); + const parsed = glp1PostBodySchema.safeParse(body); + if (!parsed.success) { + // v1.4.43 W6 — multi-issue 422. + return returnAllZodIssues(parsed.error, 422); + } - if (parsed.data.doseChange) { - const { effectiveFrom, doseValue, doseUnit, note } = - parsed.data.doseChange; - const created = await prisma.medicationDoseChange.create({ - data: { - medicationId: id, - effectiveFrom, - doseValue, - doseUnit, - // Encrypt the titration note at rest; the plaintext column stays null. - noteEncrypted: encryptNote(note), - note: null, - }, - }); + const ip = getClientIp(request); - await auditLog("medication.glp1.update", { - userId: user.id, - ipAddress: ip, - details: { - medicationId: id, - kind: "doseChange", - doseChangeId: created.id, - doseValue, - doseUnit, - }, - }); - - annotate({ - action: { - name: "medication.glp1.doseChange.create", - entity_type: "medication_dose_change", - entity_id: created.id, - }, - meta: { medication_id: id, doseValue, doseUnit }, - }); - - // A dose change shifts the daily-dose estimate the list payload's - // runway derivation uses. Hard-evict the medications + compliance - // buckets so the card reflects the new runway on the next read. - invalidateUserMedications(user.id, { evict: true }); - - return apiSuccess({ doseChange: shapeDoseChangeNote(created) }, 201); - } - - // The schema's XOR refinement guarantees `inventory` is present - // when `doseChange` is not, so the non-null assertion is safe. - // - // DEPRECATED branch (v1.16.10): this writes the legacy - // MedicationInventoryEvent running-sum ledger. The per-item - // endpoints (`POST /api/medications/[id]/inventory`, - // `PATCH /api/medications/[id]/inventory/[itemId]`) replaced it; - // GET above and the Coach snapshot read the ledger only while the - // medication has zero inventory items. New callers must register - // containers instead of posting deltas. - const { delta, reason } = parsed.data.inventory!; - const created = await prisma.medicationInventoryEvent.create({ - data: { medicationId: id, delta, reason }, + if (parsed.data.doseChange) { + const { effectiveFrom, doseValue, doseUnit, note } = parsed.data.doseChange; + const created = await prisma.medicationDoseChange.create({ + data: { + medicationId: id, + effectiveFrom, + doseValue, + doseUnit, + // Encrypt the titration note at rest; the plaintext column stays null. + noteEncrypted: encryptNote(note), + note: null, + }, }); await auditLog("medication.glp1.update", { @@ -340,27 +309,70 @@ export const POST = apiHandler( ipAddress: ip, details: { medicationId: id, - kind: "inventory", - inventoryEventId: created.id, - delta, - reason, + kind: "doseChange", + doseChangeId: created.id, + doseValue, + doseUnit, }, }); annotate({ action: { - name: "medication.glp1.inventory.create", - entity_type: "medication_inventory_event", + name: "medication.glp1.doseChange.create", + entity_type: "medication_dose_change", entity_id: created.id, }, - meta: { medication_id: id, delta }, + meta: { medication_id: id, doseValue, doseUnit }, }); - // A legacy ledger delta changes the stock the list payload reports - // for a medication with no per-item containers. Hard-evict so the - // card reflects it on the next read. + // A dose change shifts the daily-dose estimate the list payload's + // runway derivation uses. Hard-evict the medications + compliance + // buckets so the card reflects the new runway on the next read. invalidateUserMedications(user.id, { evict: true }); - return apiSuccess({ inventory: created }, 201); - }, -); + return apiSuccess({ doseChange: shapeDoseChangeNote(created) }, 201); + } + + // The schema's XOR refinement guarantees `inventory` is present + // when `doseChange` is not, so the non-null assertion is safe. + // + // DEPRECATED branch (v1.16.10): this writes the legacy + // MedicationInventoryEvent running-sum ledger. The per-item + // endpoints (`POST /api/medications/[id]/inventory`, + // `PATCH /api/medications/[id]/inventory/[itemId]`) replaced it; + // GET above and the Coach snapshot read the ledger only while the + // medication has zero inventory items. New callers must register + // containers instead of posting deltas. + const { delta, reason } = parsed.data.inventory!; + const created = await prisma.medicationInventoryEvent.create({ + data: { medicationId: id, delta, reason }, + }); + + await auditLog("medication.glp1.update", { + userId: user.id, + ipAddress: ip, + details: { + medicationId: id, + kind: "inventory", + inventoryEventId: created.id, + delta, + reason, + }, + }); + + annotate({ + action: { + name: "medication.glp1.inventory.create", + entity_type: "medication_inventory_event", + entity_id: created.id, + }, + meta: { medication_id: id, delta }, + }); + + // A legacy ledger delta changes the stock the list payload reports + // for a medication with no per-item containers. Hard-evict so the + // card reflects it on the next read. + invalidateUserMedications(user.id, { evict: true }); + + return apiSuccess({ inventory: created }, 201); +} diff --git a/src/lib/openapi/routes/cycle.ts b/src/lib/openapi/routes/cycle.ts index 8f7995eca..4db1ac99a 100644 --- a/src/lib/openapi/routes/cycle.ts +++ b/src/lib/openapi/routes/cycle.ts @@ -516,6 +516,7 @@ export const cyclePaths: NonNullable = { }, }, post: { + parameters: [idempotencyKeyParameter], tags: ["Cycle"], summary: "Create a custom cycle symptom (v1.15.1)", description: @@ -527,6 +528,7 @@ export const cyclePaths: NonNullable = { }, }, responses: { + ...idempotentWrite(), "201": { description: "The created custom symptom.", content: { diff --git a/src/lib/openapi/routes/medications/paths.ts b/src/lib/openapi/routes/medications/paths.ts index bdf31f54c..5c240f3b9 100644 --- a/src/lib/openapi/routes/medications/paths.ts +++ b/src/lib/openapi/routes/medications/paths.ts @@ -321,6 +321,7 @@ export const medicationPaths: NonNullable = { }, }, post: { + parameters: [idempotencyKeyParameter], tags: ["Medications"], summary: "Record a titration step or a legacy stock correction", description: @@ -331,6 +332,7 @@ export const medicationPaths: NonNullable = { content: { "application/json": { schema: glp1PostBodySchema } }, }, responses: { + ...idempotentWrite(), ...recordRefusal(), "201": { description: diff --git a/tests/integration/medication-write-idempotency.test.ts b/tests/integration/medication-write-idempotency.test.ts index 9f3277c4a..b83a9d138 100644 --- a/tests/integration/medication-write-idempotency.test.ts +++ b/tests/integration/medication-write-idempotency.test.ts @@ -1,5 +1,5 @@ /** - * The three medication create routes the native client replays out of its + * The four medication create routes the native client replays out of its * offline outbox, driven as routes against a real Postgres. * * Each case imports the shipped `POST` export and calls it twice with the same @@ -10,8 +10,8 @@ * keep passing after a route dropped it. * * The client sends the header on every replay, so a lost success response used - * to cost the user a duplicate medication, a duplicate symptom entry, or a - * duplicate pen in the supply count. `POST /api/medications` had a dedupe of + * to cost the user a duplicate medication, a duplicate symptom entry, a + * duplicate pen in the supply count, or a second titration step. `POST /api/medications` had a dedupe of * its own, but only for a MIRRORED create keyed on * `(externalSource, externalId)`; a manually entered medication carries * neither field and had nothing to collapse a retry onto — so the manual shape @@ -228,3 +228,41 @@ describe("POST /api/medications/[id]/inventory — replayed container", () => { expect(rows).toHaveLength(1); }); }); + +describe("POST /api/medications/[id]/glp1 — replayed titration step", () => { + it("replays the first response and writes exactly one dose change", async () => { + const medicationId = await seedMedication(); + const { POST } = await import("@/app/api/medications/[id]/glp1/route"); + const body = { + doseChange: { + effectiveFrom: "2026-09-01T00:00:00Z", + doseValue: 2.5, + doseUnit: "mg", + note: "stepped up after four weeks", + }, + }; + + const first = await (POST as Handler)( + post(`/api/medications/${medicationId}/glp1`, body, "outbox-glp1-1"), + { params: Promise.resolve({ id: medicationId }) }, + ); + expect(first.status).toBe(201); + const firstBody = await first.json(); + + const replay = await (POST as Handler)( + post(`/api/medications/${medicationId}/glp1`, body, "outbox-glp1-1"), + { params: Promise.resolve({ id: medicationId }) }, + ); + expect(replay.status).toBe(201); + expect(replay.headers.get("X-Idempotent-Replay")).toBe("true"); + expect(await replay.json()).toEqual(firstBody); + + // The titration row carries no natural key at all, so without the wrapper + // the replay would leave the medication with two dose changes on the same + // day and the runway estimate reading off the wrong one. + const rows = await getPrismaClient().medicationDoseChange.findMany({ + where: { medicationId }, + }); + expect(rows).toHaveLength(1); + }); +});