feat(api): idempotency and optimistic concurrency (#48, #49)

ARCHITECTURE.md

@@ -182,24 +182,41 @@ DoctorSpecialty (join table)
   id, doctor_profile_id, specialty_id
 
 FacilityProfile (1:1 User)
-  id, user_id, name, type (enum: lab|pharmacy), capabilities: FacilityCapability[]
+  id, user_id, name, type (enum: lab|pharmacy), version (int, default 1 — issue #49, §5.4)
+  capabilities: FacilityCapability[]
 
 FacilityCapability
   id, facility_profile_id, capability_type (enum: lab_test|medication_dispense), detail (e.g. test category)
+  version (int, default 1 — issue #49, §5.4)
 ```
+**`version` columns (Phase 8, issue #49):** added by
+`backend/prisma/migrations/20261019090000_add_optimistic_concurrency/` as
+`INTEGER NOT NULL DEFAULT 1` on exactly the mutable records above plus
+`Appointment` (§4.2) and `PaymentIntent`. They are the storage half of the
+`ETag`/`If-Match` convention in §5.4 and carry no meaning on their own —
+nothing may treat a higher `version` as "more correct", and no code may
+compare-then-write instead of guarding the UPDATE itself. Tables that are
+append-only (`Prescription`, `LabOrder`) or immutable
+(`Specialty`, `WalletLink`, `AuditEvent`, the export tables) deliberately do
+**not** have one; the integration test asserts that set so a later change
+can't quietly widen it.
 
 ### 4.2 Availability & Appointments
 ```
 AvailabilitySlot
   id, doctor_profile_id, start_time, end_time, status (enum: open|held|booked)
   # Uniqueness/exclusion constraint at DB level on (doctor_profile_id, start_time, end_time)
   # to make double-booking a constraint violation, not an app-logic bug.
+  # No `version`: a slot is only ever moved by the conditional claim in §5.1,
+    not edited.
 
 Appointment
   id, patient_profile_id, doctor_profile_id, availability_slot_id (FK, unique, NULLABLE)
   intake_id (FK -> IntakeForm — still not a column, Phase 2 issue #1/#2)
   status (enum: scheduled|completed|cancelled|no_show)
   origin (enum: direct_booking|referral), referral_id (nullable FK -> Referral)
+  version (int, default 1 — issue #49, §5.4; no If-Match-guarded route exists
+    for appointments yet, but every future status write must move it)
   created_at, updated_at
   # availability_slot_id is nullable (Phase 3): a cancelled appointment
   # releases its slot back to `open` and drops the unique link, so the
@@ -598,6 +615,48 @@ T5, enforced as a curated config policy.
 4. Facility updates its own `FulfillmentRecord` status; for labs, attaches `LabResult`.
 5. Order's own `status` field is updated to `fulfilled` only via this facility-confirmed path — patient/doctor cannot self-mark it.
 
+### 5.4 Mutable-record optimistic concurrency (Phase 8, issue #49)
+Slot booking (§5.1) is protected by a conditional update, but nothing else was: a
+doctor editing their profile in one tab and an admin in another, or two tabs
+updating the same appointment, silently overwrote each other — last write wins,
+and the loser never finds out. This is the convention that closes that whole
+class of bug, and **new mutable-resource routes must follow it.**
+
+1. An individual mutable-resource read returns a strong `ETag` carrying the row's
+   integer `version`, e.g. `ETag: "4"` (also mirrored as `version` in the JSON
+   body). Collection reads deliberately do not get one: a client fetches the
+   individual resource before editing.
+2. `PATCH`/`PUT` sends that exact value back in `If-Match`. Missing header →
+   `428 Precondition Required` (`code: 'IF_MATCH_REQUIRED'`); weak validator, a
+   validator list, or anything else that isn't one strong `"<int>"` → `400`
+   (`code: 'INVALID_IF_MATCH'`) rather than a guess at which member was meant;
+   a version that no longer matches → `412 Precondition Failed`
+   (`code: 'VERSION_CONFLICT'`).
+3. The repository performs **one** conditional update — `UPDATE ... WHERE id = ?
+   AND version = ?` with `version = version + 1` in the same statement — and
+   returns the row that statement wrote. A zero-row result is a stale write.
+   Read-compare-write is forbidden: it reopens the exact race it closes, because
+   two callers can both read version 4 and both pass a check made before the
+   write. In Prisma that is `updateManyAndReturn` / `updateMany` with
+   `count === 1`, never `findUnique` → compare → `update`.
+4. Everything else keeps working the way it did. Endpoints whose state
+   transition is *already* conditional (booking's slot claim, payment
+   fund/release/refund, doctor verification, token redemption) keep that status
+   guard and are **not** additionally `If-Match`-gated — two guards on one write
+   produce two different failure modes and the second one teaches clients
+   nothing. They do still increment `version`, and still publish the new `ETag`,
+   so a client can precondition whatever it does next. Every other write to a
+   versioned row increments it too, including system writes (email verification).
+5. Applies to `User`, `PatientProfile`, `DoctorProfile`, `FacilityProfile`,
+   `FacilityCapability`, `Appointment` and `PaymentIntent` (columns documented in
+   §4.1/§4.2). Append-only `Prescription`/`LabOrder` are excluded by design —
+   a correction is a new row (`supersedes_*`), so there is nothing to lose.
+6. Shared pieces live in `backend/src/middleware/optimistic-concurrency.ts`
+   (`requireIfMatch`, `expectedVersion`, `setVersionEtag`, `versionConflict`) so
+   every module answers with the same status, message and code. Proof:
+   `backend/tests/integration/optimistic-concurrency.test.mts`, including eight
+   simultaneous writes on one version where exactly one succeeds.
+
 ## 6. Hard questions (architecture-level — see PRD §8 for product-level)
 
 ### 6.1 What do we think would break?
@@ -607,6 +666,8 @@ T5, enforced as a curated config policy.
 4. JWT refresh rotation without a revocation list — a stolen refresh token has no clean kill switch until this is added.
 5. **Discovered 2026-10-15 while building email verification (Phase 8, issue #34):** the email/SMS provider is still undecided (Phase 6, issue #17), so verification links can't actually be delivered in a hosted environment yet — `backend/src/modules/auth/services/email-dispatcher.ts` is a no-op seam until a provider is integrated. Until then `backend/scripts/mark-email-verified.mjs` is the documented out-of-band path for an operator with DB access. Not a reason to skip the guard: the guard is what makes the flow safe once delivery exists, and shipping it locked-off would just mean shipping it untested.
 
++6. **Resolved 2026-10-19 (issue #49):** silent last-write-wins on every mutable record that wasn't the slot claim — a doctor in one tab and an admin in another, or two tabs editing one appointment, each wrote and the second erased the first with no signal to either. §5.4 now makes the compare and the write one conditional statement behind an `If-Match` precondition. **Still open:** the convention is only *enforced* where a route opts in, so a future mutable route that forgets `requireIfMatch` silently opts out — there is no lint or schema-level way to catch that yet, and ESLint itself is still untracked (issue #28).
+
 6. **Discovered 2026-10-17 (issue #41):** after `DELETE /patients/me`, the caller's access token (stateless JWT, 15-minute TTL) still passes `authenticate` until it expires — there's no revocation list (item 4 above, issue #18). It can't reach anything: every patient route resolves the caller through `getMyPatientProfile`, which refuses an anonymised profile; booking/payments refuse it via `requireVerifiedEmail` (verification is cleared); refresh refuses `deleted_at`. Closing the window itself is #18's job. Also: Postgres reports `ON DELETE RESTRICT` as SQLSTATE 23001, which Prisma surfaces as an *unknown* request error, not `P2003` — `patients/services` `isForeignKeyRefusal` handles both; `middleware/error-handler.ts` currently maps neither to a 4xx.
 
 ### 6.2 What edge cases are we missing (architecture-level)?
@@ -615,6 +676,7 @@ T5, enforced as a curated config policy.
 3. No schema support yet for a doctor working across multiple facilities/locations (PRD edge case 6) — `AvailabilitySlot` currently assumes one calendar per doctor.
 4. ~~No audit-log table~~ **Foundation resolved 2026-10-17 (issue #43):** `AuditEvent` (§4.10) + `audit.record()` in the same transaction as the change, append-only at the DB level, wired into booking (both booking paths), doctor verification, payment fund/release/refund, the patient's referral accept/decline and the referral admin actions (`referral.reassign`, `referral.cancel` — Phase 3, issue #68, written as `entity_type = 'Referral'` rows in the same table). **Still open:** the referral's creation and the cascade-driven `completed`/`expired` transitions (no actor to attribute them to yet), the remaining Phase 4 transitions (prescriptions, lab orders, fulfillment), the admin login-throttle clear (`/auth/login-attempts/clear`) still only logs to the console; the app connects as the database owner, which could disable the triggers — enforcing append-only against the owner needs a separate least-privilege DB role (deployment work, issue #22). PRD §7 auditability is therefore "a real mechanism exists" rather than "auditability is done"; remaining coverage stays an open gap.
 5. CSRF on the cookie-authenticated routes — the refresh cookie has to be `SameSite=None; Secure` (the Pages frontend is a different site — see `docs/adr/0003`), which means any site the patient visits can make their browser send it to `/auth/refresh` and `/auth/logout`. **Resolved 2026-10-15 (issue #37):** a per-session CSRF token is bound into the refresh token and required in the `X-CSRF-Token` header, the `Origin` header is validated against an explicit allowlist, and CORS is now a per-environment allowlist (`CORS_ORIGINS`) instead of one configured string. The double-submit cookie pattern — the usual answer — is *not* usable here: a cookie set by this API is a third-party cookie to `docforum.github.io`, so the frontend's JavaScript can never read it; the token travels in the login/refresh response body instead. A request with no `Origin` (curl, server-to-server) is allowed through the Origin check but still needs the token. See `backend/src/middleware/csrf.ts`, and `docs/api/README.md` for what the client must send. Consequence: refresh cookies issued before this change carry no `csrf` claim and are rejected, i.e. existing sessions need one re-login.
+6. **Added 2026-10-19 (issue #48):** an `IdempotencyKey` row caches a *copy of a response body* for 24 hours (§4.12). Today every opted-in route returns non-PHI data (an appointment id and status, a payment intent), so this is transport bookkeeping and not a patient-data surface — it is not part of the patient-export bundle (§3.1) and no endpoint lists it. **The condition that would change that:** if a route returning clinical free text (consultation notes, intake payloads, lab results) ever opts in, each row becomes a temporary PHI copy subject to right-of-access and retention, and the export registry in §3.1 has to gain a section for it before that route is allowed to opt in. Until then, adding a route to the opted-in list is a deliberate decision, not a default.
 
 6. **Discovered 2026-10-24 (issue #16):** lab result files have two read gaps. (a) The right-of-access export (§4.11) lists the `LabResult` row, but the file bytes are not in the bundle; the patient downloads them separately through `GET /orders/lab-orders/:labOrderId/result`. (b) Read access mirrors consultation notes exactly: the patient's own account, the ordering doctor, and the fulfilling facility. So an active guardian of a dependent (ADR 0011) can't read a dependent's result, and neither can a doctor further along the referral thread. Widening either needs a deliberate decision, not a quiet one.
 
@@ -629,6 +691,7 @@ T5, enforced as a curated config policy.
 
 - **Testing**: unit tests per module (`backend/tests/unit`), integration tests for booking/referral/order flows specifically because those are the correctness-critical paths (`backend/tests/integration`).
 - **Security — login throttling (Phase 8, issue #35)**: `POST /auth/login` is throttled per normalised email *and* per client IP, in Postgres (`LoginAttempt`, §4.9): exponential backoff with `Retry-After` from `LOGIN_THROTTLE_BACKOFF_AFTER` failures, a temporary lock at `LOGIN_THROTTLE_LOCK_AFTER`, and a cleanup interval in `src/server.ts`. Two things this depends on operationally: (1) `TRUST_PROXY` must be set correctly in production — Render terminates TLS in front of exactly one hop, so `TRUST_PROXY=1`; leaving it unset makes every request look like Render's proxy (one shared bucket for everyone), and `true` lets a client spoof `X-Forwarded-For` for a fresh bucket per request; (2) the throttle deliberately does not reveal whether an account exists — locked, backoff and unknown-account responses are identical, and an absent account still pays the bcrypt comparison cost so response timing doesn't leak either.
+- **Idempotent retries (Phase 8, issue #48)**: `POST /appointments`, `POST /payments/intents` and `POST /payments/intents/:id/{fund,release,refund}` honour an `Idempotency-Key` header (`src/middleware/idempotency.ts`). A repeat of the same key with the same request body inside 24 h replays the stored status and body instead of running the mutation again; the same key with a different body is `422`; a second request arriving while the first is still running is `409 IN_PROGRESS` with a `Retry-After`, because ownership is decided by the unique constraint on `(userId, key)` rather than by an in-memory lock that would be correct on one instance and broken on two. Keys are namespaced per user, and 5xx is never cached so a client can retry a failed call with the same key. See §4.12 for the model.
 - **Migrations**: Prisma migrations are the only way schema changes happen — no manual DB edits, ever.
 - **Environments**: local (Docker Compose), staging, production — staging/production infra choice is an open item (see ROADMAP).
 

ARCHITECTURE_ESSENTIALS.md

@@ -19,6 +19,7 @@ Quick-reference only. Full reasoning lives in `ARCHITECTURE.md`. Full product sc
 4. **Cross-module access only through a module's service interface.** Never import another module's `repositories/*` directly.
 5. **Facilities write to their own `FulfillmentRecord` only** — never directly to `Prescription.status` / `LabOrder.status`.
 6. Every module = `routes/ controllers/ services/ repositories/`. Keep that shape.
+7. **Mutable-record edits use optimistic concurrency (Phase 8, issue #49).** Individual-resource read returns a strong version `ETag` (`"4"`); PATCH/PUT must send it in `If-Match` — missing is `428`, weak/list/garbage is `400`, stale is `412`. Write it as ONE conditional `UPDATE ... WHERE id = ? AND version = ?` that increments `version` (Prisma: `updateManyAndReturn`); read-compare-write is forbidden. Status transitions that are *already* conditional (booking, payment status, verification) keep their own guard, don't get a second `If-Match` one, but still bump + republish the version. Append-only `Prescription`/`LabOrder` are excluded. See ARCHITECTURE.md §5.4.
 
 ## Data model map (see ARCHITECTURE.md §4 for full field lists)
 `User` (including `locale`, default `en`, Phase 9 #133) → `PatientProfile | DoctorProfile | FacilityProfile`; `User` → `AdminInvite` (Phase 8, issue #39 — hashed/expiring/single-use; `User.deactivatedAt` soft-deactivates an account, refusing login/refresh/access tokens without deleting anything)
@@ -37,6 +38,8 @@ Quick-reference only. Full reasoning lives in `ARCHITECTURE.md`. Full product sc
 Account deletion (Phase 8, issue #41 — docs/adr/0010): anonymise in place (`User.deletedAt`, `PatientProfile.anonymizedAt`), never cascade. `PatientProfile.userId` is ON DELETE RESTRICT; new patient-linked clinical tables must be RESTRICT too **and** register a retention hold in `src/retention-holds.ts`.
 `FacilityProfile` → `WalletLink` (payout address) ; `PatientProfile` → `PatientWalletLink` (payer address, ADR 0005) ; `Prescription` or `LabOrder` → exactly one typed `PaymentIntent` FK (issue #85), whose `amount` is in the smallest unit of a per-environment allowlisted `assetCode` (issue #79)
 
+Mutable records above (plus `Appointment` and `PaymentIntent`) carry a `version` int for the `ETag`/`If-Match` convention — hard rule 7, ARCHITECTURE.md §5.4. Mutable profile fields a new route exposes need a conditional write, not just a `.update()`.
+
 ## Payments (Phase 5.5 — see docs/adr/0004-custodial-payments-v1.md)
 - **Custodial v1**: `docforum-core` holds its own Stellar payer + releaser identities (`PLATFORM_PAYER_SECRET`/`PLATFORM_RELEASER_SECRET` env vars). Patients never hold or sign with a wallet — the only reason this is possible without violating `docforum-web`'s "no direct Stellar calls" hard rule.
 - `WalletLink` is scoped to the **facility** (the on-chain payee), not the patient.

ROADMAP.md

@@ -121,7 +121,7 @@ and issue links.
 - [ ] ~~Shared: auth screens (signup/login per role).~~ → already built, `docforum-web` Phase W1.
 
 ## Phase 8 — Hardening (safety, audit, security)
-**Status: In progress. In addition to the original five items (#18–#20 scoped, two still blocked), four auth-hardening items are scoped as #34, #35, #37 and #39 — #34 and #35 are merged; #37 and #39 are open PRs. An access-control test suite (#44) is built and tested. Each is independent of the others and of Phases 3–7, and each carries its own schema/doc updates.**
+**Status: In progress. In addition to the original five items (#18–#20 scoped, two still blocked), four auth-hardening items are scoped as #34, #35, #37 and #39 — #34 and #35 are merged; #37 and #39 are open PRs. An access-control test suite (#44) is built and tested, and `Idempotency-Key` replay protection (#48) is in review. Each is independent of the others and of Phases 3–7, and each carries its own schema/doc updates.**
 
 - [x] Admin success-metrics endpoint for available booking lead-time and no-show measures; unsupported PRD metrics are explicitly marked unavailable. Definitions and suppression policy: `docs/success-metrics.md`. Tracked as [issue #124](https://github.com/DocForum/docforum-core/issues/124).
 

backend/.env.example

@@ -35,6 +35,11 @@ LOGIN_THROTTLE_PRUNE_AFTER_SECONDS=86400
 # than this many days are deleted by the prune job in src/server.ts.
 ACCESS_LOG_RETENTION_DAYS=365
 
+# Idempotent retries (Phase 8, issue #48). Defaults are in src/config/index.ts.
+IDEMPOTENCY_RECORD_TTL_HOURS=24
+IDEMPOTENCY_IN_PROGRESS_TIMEOUT_SECONDS=30
+IDEMPOTENCY_MAX_CACHED_BODY_BYTES=1048576
+
 # Patient data export (Phase 8, issue #40). Defaults are in src/config/index.ts.
 PATIENT_EXPORT_TTL_HOURS=24
 PATIENT_EXPORT_PAGE_SIZE=200

backend/prisma/migrations/20261019090000_add_optimistic_concurrency/migration.sql

@@ -0,0 +1,31 @@
+-- Optimistic concurrency for mutable records (Phase 8, issue #49).
+--
+-- One integer per row, defaulting to 1 so existing rows are valid at their
+-- current state without a backfill. It is the storage half of the ETag/If-Match
+-- convention in ARCHITECTURE.md §5.4 and is meaningless on its own: nothing may
+-- compare-then-write, the compare has to live in the UPDATE's WHERE clause.
+--
+-- Deliberately NOT added to Prescription/LabOrder (append-only — a correction is
+-- a new row, so there is no update to lose) or to immutable tables
+-- (AvailabilitySlot, Specialty, WalletLink, AuditEvent, the export tables).
+
+-- AlterTable
+ALTER TABLE "User" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 1;
+
+-- AlterTable
+ALTER TABLE "PatientProfile" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 1;
+
+-- AlterTable
+ALTER TABLE "DoctorProfile" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 1;
+
+-- AlterTable
+ALTER TABLE "FacilityProfile" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 1;
+
+-- AlterTable
+ALTER TABLE "FacilityCapability" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 1;
+
+-- AlterTable
+ALTER TABLE "Appointment" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 1;
+
+-- AlterTable
+ALTER TABLE "PaymentIntent" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 1;