feat(api): optimistic concurrency (version + If-Match) for mutable records (#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?
@@ -606,6 +665,7 @@ T5, enforced as a curated config policy.
 3. Polymorphic `FulfillmentRecord.order_id` (§4.7) — polymorphic FKs are a known footgun for referential integrity in Postgres. Needs either two separate tables (`PrescriptionFulfillment`, `LabOrderFulfillment`) or a check-constraint + application-level guard. **Current schema sketch is a simplification that should be revisited before implementation**, not treated as final. **Resolved 2026-10-22 (Phase 5, issue #13 — `docs/adr/0012-fulfillment-record-typed-order-fks.md`):** neither option exactly as posed — `FulfillmentRecord` stays **one table**, but the `(order_type, order_id)` pair is gone. It is replaced by two real foreign keys (`prescription_id`, `lab_order_id`, each `UNIQUE`) with a PostgreSQL `CHECK` requiring exactly one, plus the `facility_id` FK the sketch left as a TODO. Referential integrity is now the database's job, and the CHECK only chooses between two already-validated columns — which is what an `order_type`+`order_id` guard could never do, since Postgres cannot make a bare `TEXT` conditionally reference one of two tables. Keeping one table (rather than splitting) is what keeps §5.3 step 5 a single transition implementation and the facility's assigned-orders queue a single indexed query; the discriminator mechanism is the one `PaymentIntent` already uses (issue #85). Deferred by that decision rather than invented around: re-routing an order to a different facility after a `rejected` fulfillment would need a partial unique index, so in v1 a `rejected` row is terminal for that order — a corrected order is a new `supersedes_*_id` row and gets its own fulfillment.
 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.
 

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.

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;

backend/prisma/schema.prisma

@@ -439,6 +439,7 @@ model Guardianship {
 
 model DoctorProfile {
   id                 String             @id @default(uuid())
+  version            Int                @default(1)
   userId             String             @unique
   fullName           String
   verificationStatus VerificationStatus @default(pending)
@@ -489,6 +490,7 @@ model DoctorSpecialty {
 
 model FacilityProfile {
   id        String       @id @default(uuid())
+  version   Int          @default(1)
   userId    String       @unique
   name      String
   type      FacilityType
@@ -578,6 +580,7 @@ model AvailabilitySlot {
 
 model Appointment {
   id                 String            @id @default(uuid())
+  version            Int               @default(1)
   patientProfileId   String
   doctorProfileId    String
   // Nullable because a CANCELLED appointment releases its slot back to

backend/src/middleware/optimistic-concurrency.ts

@@ -0,0 +1,84 @@
+// Optimistic concurrency for mutable records (Phase 8, issue #49).
+// See ARCHITECTURE.md §5.4 for the convention every new module must follow.
+//
+// The whole point is that the compare and the write are ONE statement
+// (`UPDATE ... WHERE id = ? AND version = ?` + `version = version + 1`).
+// A read-compare-write would reopen the exact race this closes: two callers
+// can both read version 4, both pass the check, and the second write
+// silently destroys the first.
+import type { NextFunction, Request, Response } from 'express';
+import { HttpError } from '../http-error';
+
+declare global {
+  // eslint-disable-next-line @typescript-eslint/no-namespace
+  namespace Express {
+    interface Request {
+      expectedVersion?: number;
+    }
+  }
+}
+
+/**
+ * A weak validator (`W/"4"`) or a list (`"4", "7"`) is refused rather than
+ * parsed: this API only ever issues one strong validator per resource, and
+ * guessing which member of a list the caller meant is how silent overwrites
+ * come back.
+ */
+const VERSION_ETAG = /^"([1-9]\d*)"$/;
+
+/** Strong `ETag` carrying the row's integer version, e.g. `ETag: "4"`.
+ * Call it on the individual-resource read; collection reads deliberately
+ * don't get one (§5.4.5). */
+export function setVersionEtag(res: Response, version: number) {
+  res.set('ETag', `"${version}"`);
+}
+
+/** Rejects a mutable-resource write that doesn't carry the version it read.
+ * Must run after `authenticate`/`requireRole` so an unauthorised caller gets
+ * 401/403 rather than being told the resource exists. */
+export function requireIfMatch(req: Request, _res: Response, next: NextFunction) {
+  const value = req.get('If-Match');
+  if (!value) {
+    throw new HttpError(
+      428,
+      'If-Match is required. Read the resource first and retry with its ETag in the If-Match header.',
+      'IF_MATCH_REQUIRED',
+    );
+  }
+
+  const match = VERSION_ETAG.exec(value.trim());
+  const version = match ? Number(match[1]) : Number.NaN;
+  if (!Number.isSafeInteger(version)) {
+    throw new HttpError(
+      400,
+      'If-Match must be the strong version ETag returned by this API, e.g. If-Match: "4".',
+      'INVALID_IF_MATCH',
+    );
+  }
+
+  req.expectedVersion = version;
+  next();
+}
+
+/** The version the caller claimed. Read by a controller after
+ * `requireIfMatch` has run; throws rather than defaulting to 0, because a
+ * missing precondition silently skipping the guard would reintroduce the
+ * lost-update bug this exists to prevent. */
+export function expectedVersion(req: Request): number {
+  const version = req.expectedVersion;
+  if (typeof version !== 'number') {
+    throw new Error('requireIfMatch must run before expectedVersion().');
+  }
+  return version;
+}
+
+/** The error a repository's zero-row conditional update means: someone else
+ * moved the record since the caller read it. Shared so every module returns
+ * the same status, message and code. */
+export function versionConflict() {
+  return new HttpError(
+    412,
+    'This record changed since you read it. Fetch it again and reapply your change.',
+    'VERSION_CONFLICT',
+  );
+}

backend/src/modules/auth/repositories/index.ts

@@ -143,7 +143,7 @@ export async function consumeVerificationToken(tokenId: string, consumedAt: Date
 }
 
 export function markUserEmailVerified(userId: string, verifiedAt: Date) {
-  return prisma.user.update({ where: { id: userId }, data: { emailVerifiedAt: verifiedAt } });
+  return prisma.user.update({ where: { id: userId }, data: { emailVerifiedAt: verifiedAt, version: { increment: 1 } } });
 }
 
 // --- Login throttling (Phase 8, issue #35) -------------------------------

backend/src/modules/doctors/controllers/index.ts

@@ -4,10 +4,20 @@ import { publicDoctorListQuerySchema, setDoctorSpecialtyVerificationSchema, setM
 import { HttpError } from '../../../http-error';
 import { getParam } from '../../../request-params';
 import { auditActor } from '../../../middleware/request-id';
+import { expectedVersion, setVersionEtag } from '../../../middleware/optimistic-concurrency';
 
 export async function getMe(req: Request, res: Response) {
   if (!req.user) throw new HttpError(401, 'Authentication required.');
   const profile = await doctorsService.getMyDoctorProfile(req.user.id);
+  setVersionEtag(res, profile.version);
+  res.status(200).json(profile);
+}
+
+export async function updateMe(req: Request, res: Response) {
+  if (!req.user) throw new HttpError(401, 'Authentication required.');
+  const { fullName } = updateDoctorProfileSchema.parse(req.body);
+  const profile = await doctorsService.updateMyDoctorProfile(req.user.id, expectedVersion(req), fullName);
+  setVersionEtag(res, profile.version);
   res.status(200).json(profile);
 }