From d7c1930595810f66bc1e31ac55d16710e37145fc Mon Sep 17 00:00:00 2001 From: NATHANIEL BALA <328081230+shekwolonathanielbala-cyber@users.noreply.github.com> Date: Tue, 20 Oct 2026 05:18:10 +0000 Subject: [PATCH] feat(consultations): close an appointment's consultation exactly once (#4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2's `Consultation` model existed only as a scaffold β€” no columns tying it to an appointment, no writer, and no way to satisfy PRD FR-6 (close with resolved | referred | follow_up_needed). Adds POST /appointments/:appointmentId/consultation, doctor-only and limited to the doctor's own appointment. "Closed once" is a database guarantee, not a read-then-write check: Consultation.appointmentId is a NOT NULL UNIQUE FK, so two concurrent closes cannot both commit and the loser gets a 409. Access goes through appointmentsService .getAppointmentForDoctor rather than querying Appointment from the consultations module, and a non-owner gets 404 so the route never confirms someone else's appointment exists. The close and its consultation.closed audit event commit in one transaction. notes is clinical free text, so it is encrypted on write with the existing AES-256-GCM helper (issue #46) instead of starting a plaintext precedent; the caller gets their own notes back decrypted. No amendment/edit path in v1 β€” that stays an open question rather than a silently added supersedes column. Docs: ROADMAP #4 marked done with its verification caveat, ARCHITECTURE Β§4.3/Β§4.10 and the essentials data-model map updated, a new ADR (0007) recording the close-once decision and why check-then-write was rejected, and the API reference entry with the new env requirement. Verification: typecheck and the unit suite pass; the integration suite close-once/409/single-row/non-owner cases are written but could not run here because the bundled embedded-Postgres binary cannot be spawned in this sandbox (EACCES on initdb, same limitation already recorded for the other integration suites). It needs a run against a real Postgres before merge. πŸ€– Generated with Codebuff Co-Authored-By: Codebuff --- ARCHITECTURE.md | 6 +- ARCHITECTURE_ESSENTIALS.md | 2 +- README.md | 10 +- ROADMAP.md | 4 +- .../migration.sql | 23 ++ backend/prisma/schema.prisma | 20 +- backend/src/app.ts | 2 + .../appointments/repositories/index.ts | 4 + .../modules/appointments/services/index.ts | 16 ++ backend/src/modules/audit/services/index.ts | 2 +- .../consultations/controllers/index.ts | 23 +- .../consultations/repositories/index.ts | 21 +- .../src/modules/consultations/routes/index.ts | 20 +- .../modules/consultations/services/index.ts | 86 +++++++- .../src/modules/consultations/validation.ts | 6 + .../tests/integration/access-control.test.mts | 18 +- .../tests/integration/consultation.test.mts | 201 ++++++++++++++++++ backend/vitest.config.mts | 5 + docs/adr/0007-consultation-close-once.md | 96 +++++++++ docs/api/README.md | 29 ++- 20 files changed, 566 insertions(+), 28 deletions(-) create mode 100644 backend/prisma/migrations/20261020120000_add_consultation_appointment/migration.sql create mode 100644 backend/src/modules/consultations/validation.ts create mode 100644 backend/tests/integration/consultation.test.mts create mode 100644 docs/adr/0007-consultation-close-once.md diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 75e9f6c8..86514cc7 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -162,6 +162,8 @@ Consultation outcome (enum: resolved|referred|follow_up_needed) created_at ``` +**Implemented 2026-10-20 (Phase 2, issue #4):** `POST /appointments/:id/consultation` creates this row. `appointment_id UNIQUE` is what makes a consultation closable **once** β€” a second concurrent close loses at the INSERT, not at an app-level check (see docs/adr/0007). There is no amendment/edit path in v1. `outcome` is NOT NULL, and `notes` is encrypted on write via `encryptField()` (requires `FIELD_ENCRYPTION_KEY_V1`), so the first writer to a clinical free-text column honours the rule below rather than starting a plaintext precedent. + **Encryption note:** `notes` is stored as an AES-256-GCM envelope (`v1:::`) via `backend/src/utils/field-encryption.ts`. Because it's encrypted, it **cannot be searched or indexed** β€” any future full-text search over consultation notes must decrypt in the application layer or use a separate searchable index, not a Postgres index on this column. Key rotation: `backend/scripts/rotate-field-keys.mjs` re-encrypts in batches. ### 4.4 Referral @@ -240,7 +242,8 @@ LoginAttempt (issue #35) ``` AuditEvent id, actor_user_id (nullable β€” null = system), actor_role (nullable Role) - entity_type ('Appointment' | 'DoctorProfile' | 'PaymentIntent' so far), entity_id + entity_type ('Appointment' | 'DoctorProfile' | 'PaymentIntent' | + 'Consultation' so far), entity_id action (dotted, e.g. 'appointment.booked'), from_state, to_state (nullable) request_id (X-Request-Id of the causing request), metadata (jsonb, nullable) created_at @@ -263,6 +266,7 @@ AuditEvent ``` Transitions recorded (action: from β†’ to): - `appointment.booked`: `null β†’ scheduled` (metadata: slot id, doctor profile id, origin) +- `consultation.closed`: `null β†’ outcome` (metadata: appointment id; never the notes β€” that's PHI) - `doctor.verification_status_changed`: previous β†’ new verification status (read under `FOR UPDATE`; a no-op set records nothing) - `payment_intent.funded` `created β†’ escrowed`, `payment_intent.released` `escrowed β†’ released`, `payment_intent.refunded` `escrowed β†’ refunded` (metadata: order type, amount, escrow id, tx hash, contract id β€” public on-chain references only). The status write is now conditional on the observed state, so a racing second action gets `409` and records nothing. ### 4.11 Patient data export (Phase 8, issue #40) diff --git a/ARCHITECTURE_ESSENTIALS.md b/ARCHITECTURE_ESSENTIALS.md index 7dccf3b1..1ca15ac0 100644 --- a/ARCHITECTURE_ESSENTIALS.md +++ b/ARCHITECTURE_ESSENTIALS.md @@ -22,7 +22,7 @@ Quick-reference only. Full reasoning lives in `ARCHITECTURE.md`. Full product sc ## Data model map (see ARCHITECTURE.md Β§4 for full field lists) `User` β†’ `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) `DoctorProfile` β€”< `DoctorSpecialty` >β€” `Specialty` -`AvailabilitySlot` (doctor's calendar) β†’ `Appointment` β†’ `IntakeForm`, `Consultation` +`AvailabilitySlot` (doctor's calendar) β†’ `Appointment` β†’ `IntakeForm`, `Consultation` (`appointmentId` **UNIQUE** β€” closing a consultation is a create-once operation, `409` on a second attempt; see docs/adr/0007) `Consultation` β†’ (outcome=referred) β†’ `Referral` (snapshot!) β†’ new `Appointment` `Consultation` β†’ `Prescription`(+`PrescriptionItem`) / `LabOrder`(+`LabResult`) `FulfillmentRecord` (facility-owned) references an order but doesn't mutate it. diff --git a/README.md b/README.md index 3e437c6b..80d5e9a3 100644 --- a/README.md +++ b/README.md @@ -156,7 +156,9 @@ the `embedded-postgres` devDependency instead, see below) or your own. git clone https://github.com/DocForum/docforum-core.git cd docforum-core/backend npm install -cp .env.example .env # fill in DATABASE_URL, JWT_ACCESS_SECRET, JWT_REFRESH_SECRET +cp .env.example .env # fill in DATABASE_URL, JWT_ACCESS_SECRET, JWT_REFRESH_SECRET, + # and FIELD_ENCRYPTION_KEY_V1 (base64, 32 bytes β€” closes + # consultations encrypt their notes; see scripts/README.md) npx prisma migrate deploy # applies backend/prisma/migrations/ npm run dev # β†’ http://localhost:4000, GET /health to check ``` @@ -183,8 +185,10 @@ the safe-booking foundation): built and tested** β€” auth, doctor verification, availability/slot generation, and the booking-concurrency guarantee are real, not aspirational (see Getting started above). One item is unverified: the Docker Compose file for local dev exists but -wasn't exercisable in this environment. Phase 2 (intake, appointments -lifecycle beyond booking, consultations) is next. +wasn't exercisable in this environment. Phase 2 is in progress: the +`Consultation` record and its close-once guarantee (issue #4) are implemented; +intake, the appointment lifecycle beyond booking, and doctor discovery are +still ahead. Full phase breakdown: [`ROADMAP.md`](ROADMAP.md). diff --git a/ROADMAP.md b/ROADMAP.md index dd2c6462..80306fbb 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -42,11 +42,11 @@ for status and issue links. - [x] Doc reconciliation: PRD Β§8.1.1 and ARCHITECTURE Β§6.1.1 marked resolved in place. ## Phase 2 β€” Intake, appointments, consultations -**Status: Not started β€” unblocked (Phase 1 is done). Scoped into issues #1–#4 and #23.** +**Status: In progress β€” #4 done; #1–#3 and #23 not started.** Scoped into issues #1–#4 and #23. - [ ] `IntakeForm` model + structured symptom entry (not free-text-only, per PRD FR-5). Tracked as [issue #1](https://github.com/DocForum/docforum-core/issues/1) (Medium, 150 pts). - [ ] Booking flow requires intake before confirmation. Tracked as [issue #2](https://github.com/DocForum/docforum-core/issues/2) (Medium, 150 pts) β€” depends on #1. - [ ] `Appointment` lifecycle (`scheduled β†’ completed/cancelled/no_show`) + doctor's today view. Tracked as [issue #3](https://github.com/DocForum/docforum-core/issues/3) (Medium, 150 pts). -- [ ] `Consultation` record + outcome (`resolved | referred | follow_up_needed`). Tracked as [issue #4](https://github.com/DocForum/docforum-core/issues/4) (High, 200 pts). +- [x] `Consultation` record + outcome (`resolved | referred | follow_up_needed`). Tracked as [issue #4](https://github.com/DocForum/docforum-core/issues/4) (High, 200 pts) β€” **Done.** `POST /appointments/:id/consultation` (doctor-only, own appointment only), `Consultation.appointmentId` FK + `UNIQUE` so "closed once" is a DB constraint rather than app logic, `outcome` NOT NULL, notes stored encrypted (issue #46), `consultation.closed` audit event in the same transaction. Migration `20261020120000_add_consultation_appointment`. No amendment/edit path in v1 β€” that stays an open question for when referral/amendment work lands. Integration test: `backend/tests/integration/consultation.test.mts` (close-once, 409 on second close, one row, other-doctor 404, patient 403). **Verification note:** written and typechecked, but the embedded-Postgres harness can't start in the sandbox this was built in (`initdb` spawn refused) β€” same environment limitation already recorded for the other integration suites; it must be run against a real Postgres before merge. - [ ] Doctor discovery: specialty catalog, doctor ↔ specialty assignment, public `GET /doctors?specialty=` (verified doctors only). Nothing in `backend/src` touched `Specialty`/`DoctorSpecialty` before this; `docforum-web` issue #1 depends on it. Tracked as [issue #23](https://github.com/DocForum/docforum-core/issues/23) (Medium, 150 pts). - [ ] Patient-facing view: my intake history. Not yet split into its own issue β€” small enough to fold into whichever of the above lands last, or split out later if it grows. diff --git a/backend/prisma/migrations/20261020120000_add_consultation_appointment/migration.sql b/backend/prisma/migrations/20261020120000_add_consultation_appointment/migration.sql new file mode 100644 index 00000000..6e725428 --- /dev/null +++ b/backend/prisma/migrations/20261020120000_add_consultation_appointment/migration.sql @@ -0,0 +1,23 @@ +-- Phase 2, issue #4: a Consultation is now a real record, tied 1:1 to an Appointment. +-- +-- The table has existed since the Phase 0 scaffold as (id, outcome?, notes, +-- createdAt) with nothing relating it to anything, and nothing has ever +-- written to it (`POST /appointments/:id/consultation` is the first writer). +-- So tightening `outcome` to NOT NULL and adding a NOT NULL FK needs no +-- backfill. If an environment does hold scaffold rows, this migration fails +-- loudly rather than inventing an outcome and an appointment for them β€” +-- inspect and remove them first. +-- +-- `Consultation_appointmentId_key` is the guarantee, not an optimisation: +-- "a consultation can only be closed once" is the unique constraint refusing +-- the second INSERT, not a read-then-write check in the service. + +-- AlterTable +ALTER TABLE "Consultation" ADD COLUMN "appointmentId" TEXT NOT NULL, +ALTER COLUMN "outcome" SET NOT NULL; + +-- CreateIndex +CREATE UNIQUE INDEX "Consultation_appointmentId_key" ON "Consultation"("appointmentId"); + +-- AddForeignKey +ALTER TABLE "Consultation" ADD CONSTRAINT "Consultation_appointmentId_fkey" FOREIGN KEY ("appointmentId") REFERENCES "Appointment"("id") ON DELETE CASCADE ON UPDATE CASCADE; diff --git a/backend/prisma/schema.prisma b/backend/prisma/schema.prisma index 57c43578..3b69da09 100644 --- a/backend/prisma/schema.prisma +++ b/backend/prisma/schema.prisma @@ -383,6 +383,10 @@ model Appointment { patientProfile PatientProfile @relation(fields: [patientProfileId], references: [id]) doctorProfile DoctorProfile @relation(fields: [doctorProfileId], references: [id]) availabilitySlot AvailabilitySlot @relation(fields: [availabilitySlotId], references: [id]) + // Phase 2 (issue #4): created by POST /appointments/:id/consultation. The + // 1:1-ness is enforced by Consultation.appointmentId UNIQUE β€” see + // docs/adr/0007. On the schema side this is a plain back-relation. + consultations Consultation[] // TODO Phase 2: intakeId (FK -> IntakeForm). PRD FR-5 requires intake // before confirmation, not enforced yet β€” booking works without it in @@ -403,11 +407,17 @@ model IntakeForm { } model Consultation { - id String @id @default(uuid()) - outcome ConsultationOutcome? - notes String? - createdAt DateTime @default(now()) - // TODO: appointmentId (unique) + id String @id @default(uuid()) + // unique = "a consultation is closed once per appointment", enforced by the + // database rather than by a read-then-write check (docs/adr/0007). + appointmentId String @unique + appointment Appointment @relation(fields: [appointmentId], references: [id], onDelete: Cascade) + // A closed consultation always has an outcome (PRD FR-6). + outcome ConsultationOutcome + // Clinical free text: stored as an AES-256-GCM envelope by + // consultations/services (issue #46 β€” ARCHITECTURE.md Β§4.3), never plaintext. + notes String? + createdAt DateTime @default(now()) } model Referral { diff --git a/backend/src/app.ts b/backend/src/app.ts index 834f9507..5509c642 100644 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -7,6 +7,7 @@ import { authRouter } from './modules/auth/routes'; import { doctorsRouter } from './modules/doctors/routes'; import { availabilityRouter } from './modules/availability/routes'; import { appointmentsRouter } from './modules/appointments/routes'; +import { consultationsRouter } from './modules/consultations/routes'; import { paymentsRouter } from './modules/payments/routes'; import { patientsRouter } from './modules/patients/routes'; import { adminRouter } from './modules/users/routes'; @@ -41,6 +42,7 @@ export function createApp() { app.use('/doctors', doctorsRouter); app.use('/availability', availabilityRouter); app.use('/appointments', appointmentsRouter); + app.use('/appointments', consultationsRouter); app.use('/payments', paymentsRouter); app.use('/patients', patientsRouter); // Phase 8, issue #39 β€” admin account lifecycle (invites, deactivate/ diff --git a/backend/src/modules/appointments/repositories/index.ts b/backend/src/modules/appointments/repositories/index.ts index 4f2be7f8..0ae0dc26 100644 --- a/backend/src/modules/appointments/repositories/index.ts +++ b/backend/src/modules/appointments/repositories/index.ts @@ -29,3 +29,7 @@ export function findAppointmentsForPatient(patientProfileId: string) { orderBy: { createdAt: 'desc' }, }); } + +export function findAppointmentById(appointmentId: string) { + return prisma.appointment.findUnique({ where: { id: appointmentId } }); +} diff --git a/backend/src/modules/appointments/services/index.ts b/backend/src/modules/appointments/services/index.ts index 1b59c5ba..34b61f60 100644 --- a/backend/src/modules/appointments/services/index.ts +++ b/backend/src/modules/appointments/services/index.ts @@ -68,6 +68,22 @@ export async function listMyAppointments(userId: string) { return appointmentsRepo.findAppointmentsForPatient(patientProfile.id); } +/** + * Cross-module entry point for the consultations module: return the + * appointment a doctor may act on, or null if the appointment doesn't exist + * or isn't theirs. The consultations module never queries `Appointment` + * directly β€” it calls this service instead (AGENTS.md hard rule 4). + */ +export async function getAppointmentForDoctor(doctorUserId: string, appointmentId: string) { + const appointment = await appointmentsRepo.findAppointmentById(appointmentId); + if (!appointment) return null; + + const doctorProfile = await doctorsService.getDoctorProfileById(appointment.doctorProfileId); + if (doctorProfile.userId !== doctorUserId) return null; + + return appointment; +} + /** This module's contribution to the patient data export (issue #40). * Slot times and the doctor's name are fetched per page through the * availability and doctors services β€” never by joining their tables. */ diff --git a/backend/src/modules/audit/services/index.ts b/backend/src/modules/audit/services/index.ts index daea0592..d9455b94 100644 --- a/backend/src/modules/audit/services/index.ts +++ b/backend/src/modules/audit/services/index.ts @@ -24,7 +24,7 @@ export interface AuditActor { export const SYSTEM_ACTOR: AuditActor = { userId: null, role: null, requestId: null }; -export type AuditEntityType = 'Appointment' | 'DoctorProfile' | 'PaymentIntent'; +export type AuditEntityType = 'Appointment' | 'DoctorProfile' | 'PaymentIntent' | 'Consultation'; export type AuditMetadataValue = string | number | boolean | null; export type AuditMetadata = Record; diff --git a/backend/src/modules/consultations/controllers/index.ts b/backend/src/modules/consultations/controllers/index.ts index 4f9c0550..34bc7cb2 100644 --- a/backend/src/modules/consultations/controllers/index.ts +++ b/backend/src/modules/consultations/controllers/index.ts @@ -1,2 +1,21 @@ -// consultations controllers β€” placeholder. -export {}; +import type { Request, Response } from 'express'; +import * as consultationsService from '../services'; +import { closeConsultationSchema } from '../validation'; +import { HttpError } from '../../../http-error'; +import { auditActor } from '../../../middleware/request-id'; +import { getParam } from '../../../request-params'; + +export async function closeConsultation(req: Request, res: Response) { + if (!req.user) throw new HttpError(401, 'Authentication required.'); + const appointmentId = getParam(req, 'appointmentId'); + const { outcome, notes } = closeConsultationSchema.parse(req.body); + + const { consultation, appointment } = await consultationsService.closeConsultation( + auditActor(req), + appointmentId, + outcome, + notes, + ); + + res.status(201).json({ consultation, appointment }); +} diff --git a/backend/src/modules/consultations/repositories/index.ts b/backend/src/modules/consultations/repositories/index.ts index 2abd328b..409dc658 100644 --- a/backend/src/modules/consultations/repositories/index.ts +++ b/backend/src/modules/consultations/repositories/index.ts @@ -1,3 +1,18 @@ -// consultations repositories β€” placeholder. Data access only. Not to be imported -// by other modules directly β€” go through services/ instead. -export {}; +import { prisma } from '../../../db/prisma-client'; +import type { Consultation, ConsultationOutcome, Prisma } from '@prisma/client'; + +type DbClient = typeof prisma | Prisma.TransactionClient; + +export function findConsultationByAppointmentId( + appointmentId: string, + db: DbClient = prisma, +): Promise { + return db.consultation.findUnique({ where: { appointmentId } }); +} + +export function createConsultation( + db: DbClient, + data: { appointmentId: string; outcome: ConsultationOutcome; notes: string }, +): Promise { + return db.consultation.create({ data }); +} diff --git a/backend/src/modules/consultations/routes/index.ts b/backend/src/modules/consultations/routes/index.ts index b92e71e6..95f27679 100644 --- a/backend/src/modules/consultations/routes/index.ts +++ b/backend/src/modules/consultations/routes/index.ts @@ -1,3 +1,17 @@ -// consultations routes β€” placeholder. See PRD.md / ARCHITECTURE.md Β§3 for this module's scope. -// TODO(Phase: see ROADMAP.md): define routes for consultations. -export {}; +import { Router } from 'express'; +import { authenticate, requireRole } from '../../../middleware/auth'; +import * as consultationsController from '../controllers'; + +export const consultationsRouter = Router(); + +// Close (create) a consultation for an appointment. Doctor-only and restricted +// to the doctor's own appointment via the service layer; the one-per-appointment +// guarantee is enforced at the DB level (`Consultation.appointmentId UNIQUE`). +// Mounted at /appointments in src/app.ts, so the full path is +// POST /appointments/:appointmentId/consultation. +consultationsRouter.post( + '/:appointmentId/consultation', + authenticate, + requireRole('doctor'), + consultationsController.closeConsultation, +); diff --git a/backend/src/modules/consultations/services/index.ts b/backend/src/modules/consultations/services/index.ts index dc0caafb..511569c4 100644 --- a/backend/src/modules/consultations/services/index.ts +++ b/backend/src/modules/consultations/services/index.ts @@ -1,3 +1,83 @@ -// consultations services β€” placeholder. Other modules must only import from here, -// never from repositories/ directly. See AGENTS.md "Non-negotiable rules". -export {}; +import { Prisma, type ConsultationOutcome } from '@prisma/client'; +import { HttpError } from '../../../http-error'; +import { prisma } from '../../../db/prisma-client'; +import * as consultationsRepo from '../repositories'; +import * as appointmentsService from '../../appointments/services'; +import * as auditService from '../../audit/services'; +import { decryptField, encryptField } from '../../../utils/field-encryption'; + +/** + * Close a consultation for the caller's own appointment, exactly once. + * + * - The one-consultation-per-appointment guarantee is enforced at the DB + * level by `Consultation.appointmentId UNIQUE`, not by a check-then-write + * here. Two concurrent closes both reach the INSERT; exactly one commits, + * the loser gets P2002 and a 409. The read below only exists for a nicer + * message on the common (sequential) case. + * - "Doctor-only, own-appointment-only" is enforced by reading the + * Appointment through appointmentsService (the only legal cross-module + * seam per AGENTS.md hard rule 4): that service returns null for an + * appointment that doesn't exist or isn't the caller's, and this service + * answers 404 either way so it never reveals that someone else's + * appointment exists. + * - Single-use in v1: no amendments/edits. That is an explicit open + * question (ROADMAP.md Phase 2), not an assumption we silently bake in. + * - `notes` is clinical free text and is stored as an AES-256-GCM envelope + * (ARCHITECTURE.md Β§4.3 / Β§2 β€” Phase 8 issue #46), never plaintext. The + * ciphertext is therefore unsearchable by design; a future search feature + * has to decrypt in the app layer. No read path exists yet, so this + * is the first writer to that convention. + */ +export async function closeConsultation( + actor: auditService.AuditActor, + appointmentId: string, + outcome: ConsultationOutcome, + notes: string, +) { + if (!actor.userId) throw new HttpError(401, 'Authentication required.'); + + const appointment = await appointmentsService.getAppointmentForDoctor(actor.userId, appointmentId); + if (!appointment) { + throw new HttpError(404, 'Appointment not found.'); + } + + const existing = await consultationsRepo.findConsultationByAppointmentId(appointmentId); + if (existing) { + throw new HttpError(409, 'A consultation for this appointment has already been closed.'); + } + + let consultation; + try { + consultation = await prisma.$transaction(async (tx) => { + const record = await consultationsRepo.createConsultation(tx, { + appointmentId, + outcome, + notes: encryptField(notes) as string, + }); + + // Phase 8 audit trail (issue #43): the state change and its event commit + // together, so an event exists iff the consultation was actually closed. + await auditService.record(tx, actor, { + entityType: 'Consultation', + entityId: record.id, + action: 'consultation.closed', + fromState: null, + toState: outcome, + metadata: { appointmentId }, + }); + + return record; + }); + } catch (err) { + // The DB unique constraint is the actual guarantee; translate the race's + // loser into the same 409 the pre-check produces. + if (err instanceof Prisma.PrismaClientKnownRequestError && err.code === 'P2002') { + throw new HttpError(409, 'A consultation for this appointment has already been closed.'); + } + throw err; + } + + // The caller just wrote these notes, so hand them back in plaintext rather + // than leaking the stored ciphertext envelope. + return { consultation: { ...consultation, notes: decryptField(consultation.notes) }, appointment }; +} diff --git a/backend/src/modules/consultations/validation.ts b/backend/src/modules/consultations/validation.ts new file mode 100644 index 00000000..9ae2da7b --- /dev/null +++ b/backend/src/modules/consultations/validation.ts @@ -0,0 +1,6 @@ +import { z } from 'zod'; + +export const closeConsultationSchema = z.object({ + outcome: z.enum(['resolved', 'referred', 'follow_up_needed']), + notes: z.string().min(1).max(20000), +}); diff --git a/backend/tests/integration/access-control.test.mts b/backend/tests/integration/access-control.test.mts index bd46c8c7..0482819a 100644 --- a/backend/tests/integration/access-control.test.mts +++ b/backend/tests/integration/access-control.test.mts @@ -55,6 +55,7 @@ const KNOWN_ROUTES = [ 'POST /availability/doctors/:doctorProfileId/slots/generate', 'POST /appointments', 'GET /appointments/mine', + 'POST /appointments/:appointmentId/consultation', 'POST /payments/wallet-link', 'GET /payments/wallet-link', 'POST /payments/intents', @@ -105,6 +106,11 @@ const ACCESS_MATRIX: MatrixRow[] = [ { method: 'POST', path: '/appointments', expected: { anon: 401, patient: 403, doctor: 403, facility: 403, admin: 403 } }, // patient unverified β†’ 403 EMAIL_NOT_VERIFIED { method: 'GET', path: '/appointments/mine', expected: { anon: 401, patient: 200, doctor: 403, facility: 403, admin: 403 } }, + // Consultations (issue #4) β€” closing is doctor-only, and only for the + // doctor's own appointment. Placeholder id β†’ "not found", so 404 for the + // doctor actor; anyone else is stopped by the role gate first. + { method: 'POST', path: '/appointments/:appointmentId/consultation', expected: { anon: 401, patient: 403, doctor: 404, facility: 403, admin: 403 } }, + // Payments β€” admin-triggered in v1; wallet-link is facility-only { method: 'POST', path: '/payments/wallet-link', expected: { anon: 401, patient: 403, doctor: 403, facility: 400, admin: 403 } }, { method: 'GET', path: '/payments/wallet-link', expected: { anon: 401, patient: 403, doctor: 403, facility: 404, admin: 403 } }, @@ -241,9 +247,15 @@ describe('Access-control test suite: who may read intake, consultation and payme expect(true).toBe(true); }); - it('non-participating doctors cannot read a consultation (IDOR guard)', async () => { - // TODO(issue #3/#4): once consultation routes exist, seed two doctors - // and assert doctor B gets 403/404 on doctor A's consultation. + it('non-participating doctors cannot act on a consultation (IDOR guard)', async () => { + // The only consultation route in v1 is close/create, covered by the matrix + // row above (doctor β†’ 404 for an appointment that isn't theirs, never a + // 403 that would confirm it exists). The write path itself β€” including a + // real second doctor being refused on doctor A's booked appointment β€” is + // asserted end-to-end in tests/integration/consultation.test.mts. + // + // TODO(issue #4 follow-ups): extend this once a consultation READ route + // exists, so doctor B is refused a read of doctor A's notes too. expect(true).toBe(true); }); }); diff --git a/backend/tests/integration/consultation.test.mts b/backend/tests/integration/consultation.test.mts new file mode 100644 index 00000000..d888d258 --- /dev/null +++ b/backend/tests/integration/consultation.test.mts @@ -0,0 +1,201 @@ +// Integration tests for issue #4: closing a consultation, exactly once. +// (ROADMAP.md Phase 2 β€” "`Consultation` record + outcome".) +// +// The correctness-critical assertions here are: +// 1. the owning doctor can close their appointment's consultation once; +// 2. a second close is refused AND the database still holds exactly one +// row β€” the unique constraint on Consultation.appointmentId is the +// guarantee, not the service's pre-check; +// 3. another doctor cannot close it (404, not 403 β€” no existence leak). +// +// Self-contained like the other integration suites: spins up its own +// throwaway Postgres (embedded-postgres, prebuilt binary) on port 5436 and +// applies the committed migrations, so no pre-existing DB/Docker is needed. +// A distinct port keeps it clear of booking.test.mts (5433), the payments +// testnet suite (5435) and access-control.test.mts (5438). +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import request from 'supertest'; +import EmbeddedPostgres from 'embedded-postgres'; + +const BACKEND_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const PORT = 5436; +const runId = Date.now(); + +const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'docforum-consult-pg-')); +const pg = new EmbeddedPostgres({ + databaseDir: dataDir, + user: 'docforum', + password: 'docforum', + port: PORT, + persistent: false, +}); + +// Matches vitest.config.mts's DATABASE_URL convention: the client built at +// import time in src/db/prisma-client.ts connects lazily with this URL. +const databaseUrl = `postgresql://docforum:docforum@localhost:${PORT}/docforum_test`; + +/** Booking requires a verified email (Phase 8, issue #34), so the suite + * installs its own email sender and reads the verification token out of the + * message β€” the DB only stores the token's hash. */ +const emailedTokens = new Map(); + +type App = ReturnType; + +async function signupAndVerify(app: App, role: 'doctor' | 'patient', email: string, fullName: string) { + const res = await request(app).post('/auth/signup').send({ role, email, password: 'longenough123', fullName }); + expect(res.status, JSON.stringify(res.body)).toBe(200); + + const token = emailedTokens.get(email); + expect(token, `no verification email captured for ${email}`).toBeTruthy(); + const verified = await request(app).post('/auth/verify-email').send({ token }); + expect(verified.status).toBe(200); + + return res.body.accessToken as string; +} + +describe('consultation close-once guarantee', () => { + let app: App; + let prisma: typeof import('../../src/db/prisma-client.js').prisma; + let owningDoctorToken: string; + let otherDoctorToken: string; + let patientToken: string; + let appointmentId: string; + + beforeAll(async () => { + await pg.initialise(); + await pg.start(); + await pg.createDatabase('docforum_test'); + + execFileSync('npx', ['prisma', 'migrate', 'deploy'], { + cwd: BACKEND_ROOT, + env: { ...process.env, DATABASE_URL: databaseUrl }, + stdio: 'pipe', + }); + + // Imported dynamically, after migrations exist: src/app.ts's import chain + // constructs PrismaClient at module-load time. + const appModule = await import('../../src/app.js'); + const dbModule = await import('../../src/db/prisma-client.js'); + const authServices = await import('../../src/modules/auth/services/index.js'); + app = appModule.createApp(); + prisma = dbModule.prisma; + + authServices.setEmailSender(async (message) => { + const match = /[?&]token=([^&\s]+)/.exec(message.text); + if (match) emailedTokens.set(message.to, decodeURIComponent(match[1])); + }); + + owningDoctorToken = await signupAndVerify(app, 'doctor', `consult-doctor-${runId}@example.com`, 'Owning Doctor'); + otherDoctorToken = await signupAndVerify(app, 'doctor', `consult-other-${runId}@example.com`, 'Other Doctor'); + patientToken = await signupAndVerify(app, 'patient', `consult-patient-${runId}@example.com`, 'Test Patient'); + + const doctorMe = await request(app).get('/doctors/me').set('Authorization', `Bearer ${owningDoctorToken}`); + expect(doctorMe.status).toBe(200); + const doctorProfileId = doctorMe.body.id as string; + + const tomorrow = new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString().slice(0, 10); + const generate = await request(app) + .post(`/availability/doctors/${doctorProfileId}/slots/generate`) + .set('Authorization', `Bearer ${owningDoctorToken}`) + .send({ date: tomorrow, startHour: 9, endHour: 10, slotLengthMinutes: 30 }); + expect(generate.status, JSON.stringify(generate.body)).toBe(201); + expect(generate.body.created).toBeGreaterThan(0); + + const slots = await request(app).get(`/availability/doctors/${doctorProfileId}/slots`); + expect(slots.body.length).toBeGreaterThan(0); + + const book = await request(app) + .post('/appointments') + .set('Authorization', `Bearer ${patientToken}`) + .send({ availabilitySlotId: slots.body[0].id }); + expect(book.status, JSON.stringify(book.body)).toBe(201); + appointmentId = book.body.id as string; + }); + + afterAll(async () => { + const authServices = await import('../../src/modules/auth/services/index.js'); + authServices.resetEmailSender(); + await prisma?.$disconnect(); + await pg.stop(); + fs.rmSync(dataDir, { recursive: true, force: true }); + }); + + it('lets the owning doctor close the appointment consultation once', async () => { + const res = await request(app) + .post(`/appointments/${appointmentId}/consultation`) + .set('Authorization', `Bearer ${owningDoctorToken}`) + .send({ outcome: 'resolved', notes: 'Reviewed results; no follow-up needed.' }); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + expect(res.body.consultation).toMatchObject({ + id: expect.any(String), + appointmentId, + outcome: 'resolved', + }); + // The caller gets their own notes back in plaintext… + expect(res.body.consultation.notes).toBe('Reviewed results; no follow-up needed.'); + + // …while the column itself only ever holds the AES-GCM envelope + // (issue #46), never the clinical text. + const stored = await prisma.consultation.findUniqueOrThrow({ where: { appointmentId } }); + expect(stored.notes).toMatch(/^v1:[A-Za-z0-9+/=]+:[A-Za-z0-9+/=]+:[A-Za-z0-9+/=]+$/); + expect(stored.notes).not.toContain('Reviewed results'); + + // The close is audited, in the same transaction (Phase 8, issue #43). + const events = await prisma.auditEvent.findMany({ + where: { entityType: 'Consultation', entityId: res.body.consultation.id }, + }); + expect(events).toHaveLength(1); + expect(events[0].action).toBe('consultation.closed'); + expect(events[0].toState).toBe('resolved'); + }); + + it('refuses a second close of the same appointment', async () => { + const res = await request(app) + .post(`/appointments/${appointmentId}/consultation`) + .set('Authorization', `Bearer ${owningDoctorToken}`) + .send({ outcome: 'follow_up_needed', notes: 'Second attempt β€” must be rejected.' }); + + expect(res.status).toBe(409); + expect(res.body.message.toLowerCase()).toContain('already'); + }); + + it('still holds exactly one consultation row for the appointment', async () => { + const rows = await prisma.consultation.findMany({ where: { appointmentId } }); + expect(rows).toHaveLength(1); + // The rejected second attempt did not overwrite the first outcome. + expect(rows[0].outcome).toBe('resolved'); + }); + + it('refuses a doctor who does not own the appointment (no existence leak)', async () => { + const res = await request(app) + .post(`/appointments/${appointmentId}/consultation`) + .set('Authorization', `Bearer ${otherDoctorToken}`) + .send({ outcome: 'resolved', notes: 'Not my appointment.' }); + + expect(res.status).toBe(404); + }); + + it('refuses a patient closing a consultation (doctor-only route)', async () => { + const res = await request(app) + .post(`/appointments/${appointmentId}/consultation`) + .set('Authorization', `Bearer ${patientToken}`) + .send({ outcome: 'resolved', notes: 'Patient attempt.' }); + + expect(res.status).toBe(403); + }); + + it('rejects an unknown outcome', async () => { + const res = await request(app) + .post(`/appointments/${appointmentId}/consultation`) + .set('Authorization', `Bearer ${owningDoctorToken}`) + .send({ outcome: 'cured', notes: 'Invalid outcome value.' }); + + expect(res.status).toBe(400); + }); +}); diff --git a/backend/vitest.config.mts b/backend/vitest.config.mts index b184413f..ee6260e2 100644 --- a/backend/vitest.config.mts +++ b/backend/vitest.config.mts @@ -16,6 +16,11 @@ export default defineConfig({ env: { JWT_ACCESS_SECRET: 'test-access-secret', JWT_REFRESH_SECRET: 'test-refresh-secret', + // Clinical free-text fields are encrypted at write time (issue #46). + // A fixed 32-byte test key keeps ciphertext deterministic across runs + // without touching the real key, which only production/dev envs hold. + FIELD_ENCRYPTION_KEY_V1: 'BwcHBwcHBwcHBwcHBwcHBwcHBwcHBwcHBwcHBwcHBwc=', + FIELD_ENCRYPTION_ACTIVE_VERSION: 'v1', // Port 5433, not 5432 β€” tests/integration/booking.test.mts spins up // its own throwaway Postgres cluster on this port so the suite is // self-contained (no pre-existing DB required). A distinct port diff --git a/docs/adr/0007-consultation-close-once.md b/docs/adr/0007-consultation-close-once.md new file mode 100644 index 00000000..070c8ebc --- /dev/null +++ b/docs/adr/0007-consultation-close-once.md @@ -0,0 +1,96 @@ +# ADR 0007: Consultation is close-once, enforced by a unique appointment FK + +Status: Accepted +Date: 2026-10-20 + +## Context + +Phase 2, issue #4 β€” "`Consultation` record + outcome +(`resolved | referred | follow_up_needed`)". The `Consultation` table has +existed since the Phase 0 scaffold as `(id, outcome?, notes?, createdAt)` with +nothing relating it to an appointment, and nothing ever wrote to it. + +The decisions that need recording are not really about columns: + +1. **How is "a consultation can only be closed once" enforced?** The repo + already has one precedent for this class of guarantee β€” booking (Β§5.1) is + settled by a DB-level conditional `UPDATE`, not by a read-then-write check, + because "check then write" is the exact bug class the architecture was + built to avoid (ARCHITECTURE_ESSENTIALS.md hard rule). +2. **What does "closed" mean for the record?** A consultation is a clinical + record of what happened. `Prescription`/`LabOrder` are explicitly + never-mutated-after-issuance with corrections as new rows; consultation + notes are the same category of data. +3. **Do amendments exist in v1?** PRD Β§8 leaves "can a consultation be + corrected/amended" unresolved. The tempting move is to add + `supersedes_consultation_id` "just in case". That would be inventing a + resolution to an open question. + +## Decision + +- **`Consultation.appointmentId` is a `NOT NULL`, `UNIQUE` FK to + `Appointment`** (`ON DELETE CASCADE`, matching every other + appointment-owned child row). The unique index *is* the close-once + guarantee: two concurrent closes both reach the INSERT, exactly one commits, + the loser gets Prisma `P2002` β†’ `409`. The service also checks first, but + only so the common sequential case gets a readable message; the check is not + what makes it safe. +- **`outcome` is `NOT NULL`** (`resolved | referred | follow_up_needed`, + PRD FR-6) β€” a closed consultation always has an outcome. `notes` stays + nullable, capped at 20k chars at the API boundary. +- **`notes` is encrypted on write** with the existing AES-256-GCM field + encryption (issue #46) and decrypted only for the caller who just wrote it. + ARCHITECTURE.md Β§2/Β§4.3 already list `Consultation.notes` as an encrypted + clinical field; this is the first write path, so it sets the precedent + rather than starting a plaintext one. Consequence: the column is + unsearchable by design, and environments must set + `FIELD_ENCRYPTION_KEY_V1`. +- **No amendment path in v1.** Closing twice is a `409`, full stop. When the + amendment question is actually answered (Phase 3 referral/consultation + work), the answer should be a new, reviewed migration β€” for example an + append-only history keyed by `supersedes_*` β€” not a quiet relaxation of the + unique constraint. +- **Access is doctor-only, own-appointment-only.** The consultations module + never queries `Appointment`; it asks + `appointmentsService.getAppointmentForDoctor(userId, appointmentId)`, which + returns `null` for someone else's appointment. The route answers `404` for + both "doesn't exist" and "not yours", so it never confirms the existence of + another doctor's appointment. This mirrors Β§5.1's rule and AGENTS.md hard + rule 4 (no cross-module repository imports). +- **Every close writes a `consultation.closed` audit event in the same + transaction** (issue #43), with `metadata: { appointmentId }` β€” never the + notes, which are PHI. + +## Consequences + +- The guarantee survives races and process crashes; there is no window where + two consultations exist for one appointment. +- Deleting an appointment deletes its consultation. That is consistent with + the rest of the Phase 1 model (nothing clinical is user-deletable yet) and + is re-examined when the account-purge/retention work lands, since a clinical + record with retention rules should not disappear as a cascade side effect. +- The encrypted `notes` column cannot back search, sorting or reporting. A + future "search my consultations" feature must decrypt in the app layer or + maintain a separate index β€” accepted, documented in Β§4.3. +- If `FIELD_ENCRYPTION_KEY_V1` is missing, closing a consultation fails loudly + (`500`) with the util's own "key not configured" error instead of silently + storing plaintext PHI. There is no fallback-to-plaintext path and there must + not be one. +- Migration `20261020120000_add_consultation_appointment` assumes the + scaffold `Consultation` table is empty (it is: no writer existed). If an + environment does hold rows, the migration fails rather than backfilling an + invented outcome/appointment. + +## Alternatives considered + +- **Check-then-write in the service only.** Rejected: the exact bug class + (two requests pass the check, both insert) the architecture exists to + prevent. +- **`status` enum on `Consultation` (`open | closed`) instead of create-on- + close.** Rejected for v1: there is no "open consultation" state to model β€” + the appointment is the thing that exists before the visit; a consultation is + created when it is closed. Adding an `open` state now would be modelling a + flow (intake β†’ in-progress notes β†’ close) that Phase 2 hasn't designed. +- **Nullable `appointmentId` + `supersedes_consultation_id` for amendments.** + Rejected: it resolves an open PRD question by accident, and a nullable + unique FK would allow orphan consultations. diff --git a/docs/api/README.md b/docs/api/README.md index 5e67678a..777599ef 100644 --- a/docs/api/README.md +++ b/docs/api/README.md @@ -151,6 +151,32 @@ drift apart. See `docs/adr/0003` for why production needs `SameSite=None`. | `POST /appointments` | patient | `{ availabilitySlotId }` | **The concurrency-critical endpoint** β€” see `ARCHITECTURE.md` Β§5.1 and `backend/tests/integration/booking.test.mts`. `409` if the slot is no longer open. | | `GET /appointments/mine` | patient | β€” | Caller's own appointments, most recent first. | +## Consultations (under `/appointments`) β€” `backend/src/modules/consultations` + +Phase 2, issue #4. Routed under `/appointments` (`app.use('/appointments', +consultationsRouter)` in `src/app.ts`), so the module keeps its own folder while +the URL stays appointment-scoped. + +| Method & path | Auth | Body | Notes | +|---|---|---|---| +| `POST /appointments/:appointmentId/consultation` | doctor (own appointment only) | `{ outcome: 'resolved' \| 'referred' \| 'follow_up_needed', notes }` | Closes the appointment's consultation. `201` returns `{ consultation, appointment }`; `404` if the appointment doesn't exist **or isn't the caller's** (no existence leak); `400` on an unknown outcome or empty/over-20k notes; `409` if this appointment's consultation was already closed. | + +Once-only is a database guarantee, not an app-level check: +`Consultation.appointmentId` is `UNIQUE`, so two concurrent closes can't both +commit β€” the loser gets `409`. There is no amendment/edit path in v1 (an +explicit open question, not an assumption). + +`notes` is clinical free text and is stored **encrypted** (AES-256-GCM +envelope, `v1:::` β€” ARCHITECTURE.md Β§4.3, issue #46); it is +returned in plaintext to the caller who just wrote it. This requires +`FIELD_ENCRYPTION_KEY_V1` (32 bytes, base64) and optionally +`FIELD_ENCRYPTION_ACTIVE_VERSION` (default `v1`) to be set; a missing key is a +`500` on this endpoint, not a silent plaintext write. Encrypted columns cannot +be searched or indexed. + +Closing also records an `AuditEvent` (`consultation.closed`) in the same +transaction, so the event exists iff the consultation was written. + ## Audit trail (`/admin/audit-events`) β€” `backend/src/modules/audit` Phase 8, issue #43. Every response carries an `X-Request-Id` header: a @@ -171,7 +197,8 @@ started appear at the front of a new scan, not mid-way through the old one. `400 INVALID_CURSOR` for a malformed cursor; `400` if `from` is after `to`. Recorded today: `appointment.booked`, `doctor.verification_status_changed`, -`payment_intent.{funded,released,refunded}` β€” see ARCHITECTURE.md Β§4.10. +`payment_intent.{funded,released,refunded}`, `consultation.closed` β€” see +ARCHITECTURE.md Β§4.10. There is no write or delete endpoint, and the table rejects UPDATE/DELETE in the database.