feat(consultations): close an appointment's consultation exactly once… by shekwolonathanielbala-cyber · Pull Request #176 · DocForum/docforum-core

shekwolonathanielbala-cyber · 2026-10-20T05:22:22Z

feat(consultations): add consultation records and outcomes

Summary

Closes #4

Implement consultation records and outcome tracking for Phase 2 of DocForum. This introduces a "Consultation" model linked one-to-one with an appointment and a doctor-only endpoint for recording consultation notes and outcomes.

The implementation establishes the foundation for Phase 3 referrals, ensuring consultation data can be referenced when creating referrals without compromising data integrity or appointment ownership.

Changes Made

Add the "Consultation" model to "backend/prisma/schema.prisma" in accordance with "ARCHITECTURE.md" §4.3.
Enforce a one-to-one relationship between appointments and consultations using a unique constraint on "appointmentId".
Add the "POST /appointments/:id/consultation" endpoint for recording consultation notes and outcomes.
Restrict consultation creation to doctors who own the associated appointment.
Prevent multiple consultations from being created for the same appointment.
Define the supported consultation outcomes exactly as:
- "resolved"
- "referred"
- "follow_up_needed"
Follow the appointments module's service interface when accessing appointment and intake context, avoiding direct cross-module database access.
Add integration tests covering consultation creation, authorization, and duplicate-submission prevention.
Update "ARCHITECTURE.md", "docs/api/README.md", and "ROADMAP.md" to reflect the new functionality.

Security and Data Integrity

Enforce doctor-only access.
Verify appointment ownership before allowing consultation creation.
Use a database-level unique constraint on "appointmentId" to prevent duplicate consultation records, including concurrent requests.
Ensure a consultation can only be recorded once per appointment in v1.
Preserve the exact outcome values required by downstream referral workflows.

Testing

Verify successful consultation creation with valid notes and outcomes.
Verify that unauthorized users cannot create consultations.
Verify that doctors cannot create consultations for appointments they do not own.
Verify that a second consultation submission for the same appointment fails.
Verify that concurrent requests cannot create duplicate consultation records.
Run the relevant backend tests and Prisma migration checks.

Documentation

Update "ARCHITECTURE.md" with the consultation data model and behavior.
Update "docs/api/README.md" with the consultation endpoint and request/response details.
Update "ROADMAP.md" to reflect Phase 2 consultation functionality.

Acceptance Criteria

Consultation model and database migration are implemented.
"appointmentId" has a database-level unique constraint.
"POST /appointments/:id/consultation" is implemented.
Only the doctor assigned to the appointment can record a consultation.
A consultation cannot be recorded more than once for the same appointment.
All three supported outcomes are validated correctly.
Integration tests cover authorization, duplicate submissions, and concurrency.
Required documentation is updated.

Impact

This change completes a critical part of the Phase 2 consultation workflow and prepares the system for Phase 3 referral creation, where intake information and consultation notes will be snapshotted at referral creation time.

…DocForum#4) 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 DocForum#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 DocForum#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 <noreply@codebuff.com>

drips-wave · 2026-10-20T05:22:27Z

@shekwolonathanielbala-cyber Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

Upstream main gained issue DocForum#45 (PHI read-access logging) and the payments config hardening (DocForum#174). The only textual conflict was `Consultation` in schema.prisma — upstream still had the Phase 0 placeholder there, while this branch had already replaced it with the appointment FK, `UNIQUE` `appointmentId` and non-null `outcome`. Resolution keeps this branch's model; upstream added no columns to that table, so nothing is lost on their side. Potential conflicts checked, since a clean text merge is not proof of a correct program: - Port collision, found and fixed. The new consultation integration file bound port 5436, which login-throttle.test.mts already owns — the exact failure mode a previous commit fixed by giving every integration file its own embedded-Postgres port (two clusters on one port hang each other's beforeAll until the hook timeout). Moved to 5443 and documented the current port map in the file. - Both routers still mount where they should: `/appointments` (appointments + consultations) and `/admin` (admin + audit + admin audit/access-log), with `/patients` carrying both the export and access-log routers. - `AuditEntityType` keeps 'Consultation' alongside upstream's additions, and the audit docs now list `consultation.closed` next to their transitions. - ARCHITECTURE.md had two §4.11 sections (patient export and upstream's new access log); the access log is renumbered to §4.12 and the §3 reference updated, and the audit bullet's "wired today" list gains consultation closing. - Migrations sort correctly: 20261016150000_add_access_log lands before 20261020120000_add_consultation_appointment, so a fresh `migrate deploy` applies them in dependency order. Verification: `prisma generate` and `tsc --noEmit` clean; unit suite 109/109. The checked-in integration suites still can't run here (embedded Postgres `initdb` is refused by the sandbox), so the consultation flow was exercised end-to-end against a real Postgres instead — PGlite over its TCP socket with all nine committed migrations applied, driving the real app through supertest. 24/24 checks passed, including close-once, the 409 on a second close, exactly one row, non-owner 404, patient 403, audit event parity, ciphertext-at-rest, and the unique/ NOT NULL constraints holding at the DB level. ROADMAP.md's verification note records that method and what remains unrun (the embedded-Postgres harness path, and the Prisma drift script). 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>

afeezorobsco-cyber · 2026-10-20T06:42:38Z

@shekwolonathanielbala-cyber Thanks for the PR. It can't be merged yet because CI is failing. 1 integration test fails (168 passed):

× POST /appointments/:appointmentId/consultation → doctor expects 404
AssertionError: expected 400 to be 404
AssertionError: {"message":"Internal server error."}: expected 500 to be 200

The consultation endpoint now returns 400 where the access-control suite expects 404 for an appointment the doctor can't see, and one path returns 500 instead of 200. Please reproduce with npm run test:integration in backend/, fix the status codes or validation order, and push to this branch. Once CI is green it can be merged.

github-actions · 2026-10-20T09:16:34Z

@shekwolonathanielbala-cyber This PR now has merge conflicts with main, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-core.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared files like ROADMAP.md and ARCHITECTURE*.md. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

_{Automated message. It won't repeat unless the PR conflicts again after your next push.}

…conflicts Upstream main moved from 46bdaf4 to 2ecba1f (referrals, orders, sessions, escrow events, backups), so the consultation close-once branch conflicted again in 15 files and — worse — in three non-textual ways: - Upstream already created `Consultation.appointmentId` (unique + FK) in 20261020000000_order_payment_intents. This branch's 20261020120000_add_consultation_appointment would have failed on the duplicate column, so it is dropped; the column/index/FK are already committed. `outcome` stays nullable in the schema (the Phase 3 create slice allows a record before sign-off) while the close routes still require it at the boundary. - Three writers were creating the same unique clinical row with different rules: consultations `POST /consultations` (plaintext notes, no audit), this branch's `POST /appointments/:id/consultation` (encrypted + audited), and the orders flow's own repository insert. They now share one service, `consultationsService.createConsultation`, so uniqueness (P2002 → 409), note encryption, the completed-appointment rule and the single `consultation.closed` audit event apply to every address; the orders module keeps its stricter verified-doctor gate and delegates instead of writing the table. Both routes are kept (with verified-email gates) and the module exports two routers so `/appointments/:id/consultation` does not also publish `/appointments/:consultationId`. - The new integration suite collided with referral-loops on embedded Postgres port 5443; it moves to 5452. Docs reconciled: ARCHITECTURE §3/§4.3/§4.10 (one writer, nullable outcome, consultation.closed alongside the referral/prescription events), the data model map, ROADMAP Phase 2 (DocForum#4 done, port/migration notes), the API reference (all three addresses), and the close-once ADR renumbered to 0008 (0007 was already taken by the backups ADR) with its RESTRICT-FK and "no pre-check" corrections. Verified: prisma validate/generate, `tsc --noEmit`, unit suite 128/128, and the close-once flow end to end against a real Postgres (24/24 checks: 409 before completion, 201 with plaintext notes + ciphertext at rest, one audit event, second close 409 through either address, one row, decrypted read for doctor + patient, other doctor 403/404, patient 403, anon 401, bad outcome 400, orders address 409 on a closed appointment and 201 with the same encryption/audit rules on a fresh one). 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>

github-actions · 2026-10-20T18:53:55Z

@shekwolonathanielbala-cyber This PR now has merge conflicts with main, most likely because #140 (feat(auth): password reset flow with single-use, hashed, expiring tokens) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-core.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared files like ROADMAP.md and ARCHITECTURE*.md. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

_{Automated message. It won't repeat unless the PR conflicts again after your next push.}

afeezorobsco-cyber · 2026-10-20T19:22:31Z

@shekwolonathanielbala-cyber CI is failing on this branch against the latest main, so it can't be merged yet. The errors from the latest run:

Backend (typecheck + test + build)

PrismaClientKnownRequestError: 
    driverAdapterError: DriverAdapterError: injected audit failure
Error: simulated on-chain failure
 FAIL  tests/integration/consultation.test.mts > consultation close-once guarantee
AssertionError: {"message":"Internal server error."}: expected 500 to be 200 // Object.is equality
AssertionError: {"message":"Internal server error."}: expected 500 to be 200 // Object.is equality

consultation.test.mts now gets a 500 from the close endpoint after the rebase. Check the server log above that assertion in the CI output.

This PR also conflicts with main again, so please rebase first, then fix the above.

conflicts again Upstream main moved from 2ecba1f to ae853e0 (password reset DocForum#140, k6 load tests DocForum#183, follow-redirects bump DocForum#200, changelog.d convention DocForum#199), so the consultation close-once branch conflicted in two files and — worse — picked up three non-textual problems from upstream: - `docs/api/README.md`: PR DocForum#183 truncated the file from 299 to 173 lines (it dropped the referrals, payments, orders, audit-trail and patients sections and ended mid-sentence in the referrals table) and corrupted auth-section text on the way (`$429$`, `2ʐ`, `` ``header ``, "RWT refresh rotation"). The resolution keeps this branch's full file and re-applies only DocForum#183's and DocForum#140's real intent: the load-test note on `POST /appointments`, the `tv`/`tokenVersion` paragraph under auth, and DocForum#199's rewritten intro. The lost sections are restored verbatim; the corrupted lines stay fixed. Worth an upstream PR of its own to recover those sections there. - `backend/prisma/schema.prisma`: trivial both-sides merge — `User` gains upstream's `tokenVersion` column and the `PasswordResetToken` back-relation alongside this branch's consultation work. `prisma validate`/`generate` clean. - `backend/tests/integration/consultation.test.mts` (this branch's file, latent bug): it never set `process.env.DATABASE_URL`, so the app talked to whatever vitest.config.mts points at — booking's cluster on 5433. That is an ECONNREFUSED when the file runs alone and, worse, a pass against the *wrong database* when the whole integration suite runs in parallel. It now points at its own cluster on 5452, like every other self-contained suite. Port check against upstream's new password-reset suite (5449): no collisions. Also: ROADMAP.md's Phase 2 section had two list items fused onto one line from the previous merge resolution ("...they run in CI on the merge.- [ ] Doctor discovery...") — split back apart; and, per the new changelog.d rule (DocForum#199), this PR's changelog entry now lives in `changelog.d/176-consultation-close-once.md` instead of appended to ROADMAP.md's changelog, which the Phase 2 checkbox line keeps updated. Verified after the merge: `prisma validate` + `generate`, `tsc --noEmit`, `npm run build`, unit suite 138/138, full integration suite 17 files / 250 tests green in parallel (including the consultation close-once suite and upstream's password-reset suite on the same schema).

github-actions · 2026-10-21T05:52:20Z

@shekwolonathanielbala-cyber This PR now has merge conflicts with main, most likely because #211 (feat(referrals): route specialty-only referrals to the least-loaded e…) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-core.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared docs like ROADMAP.md and ARCHITECTURE*.md. If the conflict is in the ROADMAP.md changelog, move your entry into a new file under changelog.d/ (see changelog.d/README.md) rather than resolving it in place, so it can't conflict again. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

_{Automated message. It won't repeat unless the PR conflicts again after your next push.}

conflicts (specialty-only routing) Upstream main moved from ae853e0 to 11c2fa2 (DocForum#211 — specialty-only referrals routed to the least-loaded eligible doctor, which closes the issue DocForum#6 design gap). Four conflicts, plus two non-textual collisions the textual merge would have papered over: - `backend/prisma/schema.prisma`: the `Referral.targetDoctorId` comment conflict resolves to upstream's — routing now exists, so "unresolved design gap" would be stale. Column stays NOT NULL either way, which is what the close-once/no-unassigned-referral invariants want. - `docs/api/README.md`: takes upstream's routed `POST /referrals` and batch rows; the branch's `POST /consultations/:consultationId/referrals` row is updated for the same reason (optional `targetDoctorId`, routed like the referrals service does) since both routes share the service. The second conflict is upstream's still-truncated tail — the payments/orders/audit/patients sections this branch carries are kept again, exactly as in the previous merge. - `README.md` / `ARCHITECTURE_ESSENTIALS.md`: both-sides merges that keep the DocForum#6 routing sentence *and* this PR's Phase 2 close-once status. ADR number collision (the real potential conflict): DocForum#211 claimed `docs/adr/0008-referral-routing-without-target-doctor.md` on the same day this branch had renumbered its close-once ADR to 0008, and bare "ADR 0008" now appears in both referral-routing and consultation-close-once comments. This branch's ADR moves to `docs/adr/0009-consultation-close-once.md` (the only ADR number still free) and every reference to it — ARCHITECTURE.md §4.3, ARCHITECTURE_ESSENTIALS.md, ROADMAP.md, the changelog fragment, the consultations service comment and two schema comments — follows. DocForum#211's references are untouched. Also checked: the new `referral-routing.test.mts` takes port 5448, so the one-port-per-integration-file rule still holds against this branch's 5452 and password-reset's 5449. Verified after the merge: `prisma validate` + `generate`, `tsc --noEmit`, `npm run build`, unit suite 144/144, full integration suite 18 files / 255 tests green in parallel.

github-actions · 2026-10-21T06:06:53Z

@shekwolonathanielbala-cyber This PR now has merge conflicts with main, most likely because #203 (feat(appointments): add doctor's today view) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-core.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared docs like ROADMAP.md and ARCHITECTURE*.md. If the conflict is in the ROADMAP.md changelog, move your entry into a new file under changelog.d/ (see changelog.d/README.md) rather than resolving it in place, so it can't conflict again. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

_{Automated message. It won't repeat unless the PR conflicts again after your next push.}

conflicts (doctor's today view) Upstream main moved from 11c2fa2 to ffff7fd (DocForum#203 — `GET /appointments/doctor/today`, closing the last half of issue DocForum#3). Three conflicts, all in files this branch also touched: - `backend/src/modules/appointments/services/index.ts`: both sides added an independent export to the same slot in the file — this branch's `getAppointmentForDoctor` (the consultations module's cross-module seam) and upstream's `listDoctorAppointmentsToday`. Both are kept; they share only the `doctorsService`/appointments-repo imports already present. - `ROADMAP.md`: the DocForum#3 and DocForum#4 lines collided (upstream ticks DocForum#3 done, this branch ticks DocForum#4 done) — both ticks kept. The Phase 2 status line, which is this branch's own line, is updated to "DocForum#3 and DocForum#4 done" so it doesn't contradict the ticks below it. - `docs/api/README.md`: upstream's `GET /appointments/doctor/today` row is added; the `PATCH /appointments/:appointmentId/status` row keeps this branch's version because upstream's has unescaped `|` inside the inline code, which splits the markdown table cell. Route check: `/appointments/doctor/today` is registered ahead of the lifecycle routes and there is no `GET /appointments/:id` catch-all to shadow it; upstream's commit ships no tests for the endpoint, so the existing 255-test integration run plus this inspection is the coverage here. Verified after the merge: `prisma validate`, `tsc --noEmit`, `npm run build`, unit suite 144/144, full integration suite 18 files / 255 tests green in parallel.

github-actions · 2026-10-21T06:20:38Z

@shekwolonathanielbala-cyber This PR now has merge conflicts with main, most likely because #210 (feat(doctors): add specialty discovery and typo-tolerant search) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-core.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared docs like ROADMAP.md and ARCHITECTURE*.md. If the conflict is in the ROADMAP.md changelog, move your entry into a new file under changelog.d/ (see changelog.d/README.md) rather than resolving it in place, so it can't conflict again. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

_{Automated message. It won't repeat unless the PR conflicts again after your next push.}

conflicts (discovery, i18n, property tests) Upstream main moved from ffff7fd to a6c730b — four PRs: DocForum#210 specialty discovery + search, DocForum#205/DocForum#133 server-side i18n, DocForum#209 property tests, DocForum#212 malware-scan file types. Three conflicts, all in shared files: - `backend/prisma/schema.prisma`: `User` gains upstream's `locale` column (i18n) next to `emailVerifiedAt`, kept in the file's column alignment. The matching migrations (`20261021090000_doctor_discovery_trigram_search`, `20261021100000_user_locale`) arrive with the merge, in order. - `ROADMAP.md`: the Phase 2 DocForum#4 and DocForum#23 lines collided (this branch ticks DocForum#4 done, upstream ticks DocForum#23 discovery done) — both ticks kept, and this branch's Phase 2 status line is updated to "DocForum#3, DocForum#4 and DocForum#23 done" so it doesn't contradict them. - `docs/api/README.md`: upstream's new rows (`PUT /doctors/me/specialties`, the Accept-Language note on signup) are kept with this branch's escaped-pipe table rows, because upstream's versions of the surrounding rows carry unescaped `|` inside inline code, which splits the markdown table. `backend/src/app.ts` auto-merged correctly: `/specialties` is mounted while this branch's split consultation routers (`/appointments/:id/consultation` + `/consultations`) survive intact. Environment note for anyone reproducing: DocForum#205 adds deps (`intl-messageformat`, `fast-check`), so `npm ci` + `prisma generate` must be re-run after pulling; a stale node_modules shows up as "Cannot find module 'intl-messageformat'" and "no exported member 'PrismaClient'". Verified after the merge: `prisma validate` + `generate`, `tsc --noEmit`, `npm run build`, `npm run i18n:check`, unit suite 149/149 (20 files, including the new property tests), full integration suite 19 files / 258 tests green in parallel.

afeezorobsco-cyber

Reviewed: no malicious code found, CI is green. Merging.

afeezorobsco-cyber mentioned this pull request Oct 20, 2026

ci: add automatic PR malware scan #178

Merged

github-actions Bot added the needs-rebase Has merge conflicts with main; the author needs to rebase label Oct 20, 2026

afeezorobsco-cyber mentioned this pull request Oct 20, 2026

ci: manual backfill mode for the PR malware scan #181

Merged

github-actions Bot removed the needs-rebase Has merge conflicts with main; the author needs to rebase label Oct 21, 2026

afeezorobsco-cyber approved these changes Oct 21, 2026

View reviewed changes

afeezorobsco-cyber merged commit eabcc62 into DocForum:main Oct 21, 2026
2 checks passed

This was referenced Oct 21, 2026

feat(intake): add structured patient intake records #204

Open

Feat/verified wallet links for patients and facilities #152

Open

feat(payments): SEP-10 Stellar Web Authentication server: challenge a… #148

Open

Navigation Menu

Repository navigation

feat(consultations): close an appointment's consultation exactly once… - #176

feat(consultations): close an appointment's consultation exactly once…#176
afeezorobsco-cyber merged 7 commits into
DocForum:mainfrom
shekwolonathanielbala-cyber:main

shekwolonathanielbala-cyber commented Oct 20, 2026

Uh oh!

drips-wave Bot commented Oct 20, 2026

Uh oh!

afeezorobsco-cyber commented Oct 20, 2026

Uh oh!

github-actions Bot commented Oct 20, 2026

Uh oh!

github-actions Bot commented Oct 20, 2026

Uh oh!

afeezorobsco-cyber commented Oct 20, 2026

Uh oh!

github-actions Bot commented Oct 21, 2026

Uh oh!

github-actions Bot commented Oct 21, 2026

Uh oh!

github-actions Bot commented Oct 21, 2026

Uh oh!

afeezorobsco-cyber left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

shekwolonathanielbala-cyber commented Oct 20, 2026

Uh oh!

drips-wave Bot commented Oct 20, 2026

Uh oh!

afeezorobsco-cyber commented Oct 20, 2026

Uh oh!

github-actions Bot commented Oct 20, 2026

Uh oh!

github-actions Bot commented Oct 20, 2026

Uh oh!

afeezorobsco-cyber commented Oct 20, 2026

Uh oh!

github-actions Bot commented Oct 21, 2026

Uh oh!

github-actions Bot commented Oct 21, 2026

Uh oh!

github-actions Bot commented Oct 21, 2026

Uh oh!

afeezorobsco-cyber left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants