From 17142038e503b42170c417e9bc66c694c2ed0f3c Mon Sep 17 00:00:00 2001 From: evg-g <229170690+evg-g@users.noreply.github.com> Date: Sat, 3 Oct 2026 19:59:27 +0300 Subject: [PATCH 1/5] docs(audit): ADRs for AURORA-2 ADR 0016 records the audit-log write on appointment cancel: a small audit service over an AuditWriter Protocol, a typed AuditAction enum, and the write in the same transaction as the cancel. CONTEXT.md starts the domain glossary. Co-Authored-By: Claude Opus 5.5 --- CONTEXT.md | 20 ++++++++++ ...6-audit-log-write-on-appointment-cancel.md | 39 +++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 CONTEXT.md create mode 100644 docs/adr/0016-audit-log-write-on-appointment-cancel.md diff --git a/CONTEXT.md b/CONTEXT.md new file mode 100644 index 0000000..3c33eb6 --- /dev/null +++ b/CONTEXT.md @@ -0,0 +1,20 @@ +# CONTEXT — appointments-api domain terms + +A short glossary of the terms tickets and ADRs use. Add a term when a ticket introduces one. + +- **Appointment** — a booked slot for a patient with a clinician at a clinic. Statuses: + `REQUESTED`, `CONFIRMED`, `COMPLETED`, `CANCELLED`, `NO_SHOW`. +- **Cancel** — `POST /api/v1/appointments/{id}/cancel`. The only way to reach `CANCELLED`. Needs a + current `If-Match` ETag (ADR 0008) and respects the clinic's cancellation cutoff. +- **Cancellation cutoff** — hours before `starts_at` after which a patient or clinician may no longer + cancel; a clinic admin or platform admin still may. +- **Refused cancel** — a cancel request that ends in an error response (404, 403, 428, 412, 409) + and changes nothing. +- **Audit log** — the append-only `audit_log` table and its admin-only read endpoint + `GET /api/v1/audit-log` (ADR 0015). Rows are never edited. +- **Audit entry** — one row: `actor_id`, `action`, `entity_type`, `entity_id`, `before`, `after`, + `created_at`. +- **Audit action** — the `AuditAction` value naming what happened, e.g. `appointment.cancelled` + (ADR 0016). +- **Audit writer** — the `AuditWriter` Protocol the audit service records through; implemented by + `AuditLogRepository` (ADR 0016). diff --git a/docs/adr/0016-audit-log-write-on-appointment-cancel.md b/docs/adr/0016-audit-log-write-on-appointment-cancel.md new file mode 100644 index 0000000..cb27220 --- /dev/null +++ b/docs/adr/0016-audit-log-write-on-appointment-cancel.md @@ -0,0 +1,39 @@ +# ADR 0016 — Audit-log write on appointment cancel + +Status: accepted (AURORA-2) + +## Context + +`GET /api/v1/audit-log` exists (ADR 0015), but nothing writes to the `audit_log` table, so the endpoint +returns an empty list against the real API (`docs/KNOWN_GAPS.md`). AURORA-2 adds the first write +path: one entry per successful appointment cancellation. Other actions (create, transition, devices, +excursions) come later and should reuse the same writer. + +## Decision + +- **A small audit service behind a Protocol.** `services/audit.py` exposes + `record(writer, *, actor_id, action, entity_type, entity_id, before, after)` over an `AuditWriter` + `typing.Protocol`. It has no FastAPI imports, so it is unit-tested with a fake writer and no I/O. + `AuditLogRepository.add_entry(...)` implements `AuditWriter`. +- **A typed action vocabulary.** A new `AuditAction` StrEnum in `enums.py`, starting with + `APPOINTMENT_CANCELLED = "appointment.cancelled"`. It is separate from `WebhookEventType`: the + webhook set is a public contract, and audit actions will grow past it. +- **Same transaction as the cancel.** `cancel_appointment` flushes the cancel, then records the + audit entry, then publishes the webhook. `add_entry` flushes, so an insert failure raises inside + the handler: the request session rolls the cancel back (500) and no webhook is published. +- **Row content.** `actor_id` is the authenticated user, `entity_type` is `"appointment"`, + `entity_id` is the appointment UUID as a string, `before` is `{"status": }` and `after` + is `{"status": "CANCELLED"}`. The cancellation reason is not copied into the audit row. +- **Refused cancels write nothing.** Every refusal (404, 403, 428, 412, 409) is raised before the + write. A lost-update race fails at the cancel's flush (`StaleDataError` → 412), before the write. + +## Consequences + +- A cancellation cannot commit without its audit row. A broken audit table blocks cancellations; + that is the intended trade. +- No migration (the table exists since 0001) and no contract change: `AuditLogEntryOut.action` + stays `str`, so `contracts/openapi.json` and the web client are unchanged. +- The `KNOWN_GAPS.md` entry narrows to the actions that still have no writer. +- Two concurrent cancels with the same `If-Match` always end as one 200 and one 412 with exactly one + audit row, in any interleaving, so the race is testable with `asyncio.gather` and an outcome-set + assertion. From d159c6eef68bdedc02a9455d246f9dc03b20422c Mon Sep 17 00:00:00 2001 From: evg-g <229170690+evg-g@users.noreply.github.com> Date: Sat, 3 Oct 2026 20:11:48 +0300 Subject: [PATCH 2/5] feat(appointments): AURORA-2 AC1.data AC3.data record audit entry on cancel Co-Authored-By: Claude Opus 5.5 --- docs/KNOWN_GAPS.md | 22 +- .../api/routers/appointments.py | 17 +- src/appointments_api/enums.py | 10 + .../repositories/audit_log.py | 34 ++- src/appointments_api/services/audit.py | 47 ++++ tests/integration/test_audit_log_cancel.py | 241 ++++++++++++++++++ tests/unit/test_audit.py | 68 +++++ 7 files changed, 423 insertions(+), 16 deletions(-) create mode 100644 src/appointments_api/services/audit.py create mode 100644 tests/integration/test_audit_log_cancel.py create mode 100644 tests/unit/test_audit.py diff --git a/docs/KNOWN_GAPS.md b/docs/KNOWN_GAPS.md index 7e5a4ef..fcc1632 100644 --- a/docs/KNOWN_GAPS.md +++ b/docs/KNOWN_GAPS.md @@ -2,19 +2,19 @@ Honest list of what is deliberately incomplete, and why. Updated as milestones land. -## Audit log has no write instrumentation +## Audit log has no write instrumentation (except appointment cancellation) -The `audit_log` table exists (migration 0001) and now has a read API — the admin-only, +The `audit_log` table exists (migration 0001) and has a read API — the admin-only, keyset-paginated, filterable `GET /api/v1/audit-log` added for the web audit-log page (milestone 13, -ADR 0015). But **nothing writes to it yet**: no service records an entry on appointment -create/transition/cancel, device provisioning, or excursion changes. So the endpoint returns an empty -list in production; the web app seeds rows only in its MSW mock so the page is demonstrable and -tested. - -**To close:** add an audit-writer (a small service invoked from the domain state changes, ideally via -the same transaction as the change, mirroring the webhook-outbox direction) that records -`actor_id/action/entity_type/entity_id/before/after`. Deferred so milestone 13 stays a web milestone; -the read side is contract-driven and ready for the writer. +ADR 0015). Since AURORA-2, **appointment cancellation is recorded** (ADR 0016): every successful +`POST /api/v1/appointments/{id}/cancel` writes one `appointment.cancelled` row in the same +transaction as the cancel, through `services/audit.py`. **The other actions still write nothing**: +appointment create, status transitions (confirm, complete, no-show), device provisioning, and +excursion changes. So the endpoint lists only cancellations in production. + +**To close:** call `services.audit.record` from each remaining domain state change, in the same +transaction as the change, adding an `AuditAction` member per action (the writer, the +`AuditLogRepository.add_entry` implementation, and the read side are in place). ## CLINIC_ADMIN is not clinic-scoped yet diff --git a/src/appointments_api/api/routers/appointments.py b/src/appointments_api/api/routers/appointments.py index 7c1a683..264cc09 100644 --- a/src/appointments_api/api/routers/appointments.py +++ b/src/appointments_api/api/routers/appointments.py @@ -25,6 +25,7 @@ from appointments_api.api.conditional import if_match_satisfied, make_etag from appointments_api.api.deps import ( AppointmentRepoDep, + AuditLogRepoDep, ClinicianRepoDep, ClinicRepoDep, ClockDep, @@ -52,8 +53,9 @@ AppointmentTransition, CancelRequest, ) -from appointments_api.enums import AppointmentStatus, UserRole +from appointments_api.enums import AppointmentStatus, AuditAction, UserRole from appointments_api.models import Appointment, User +from appointments_api.services import audit from appointments_api.services.appointments.cancellation import assert_can_cancel from appointments_api.services.appointments.state_machine import assert_transition from appointments_api.services.availability import fits_working_hours @@ -350,6 +352,7 @@ async def cancel_appointment( clinicians: ClinicianRepoDep, clock: ClockDep, dispatcher: WebhookDispatcherDep, + audit_log: AuditLogRepoDep, if_match: Annotated[str | None, Header(alias="If-Match")] = None, ) -> Response: appointment = await appointments.get(appointment_id) @@ -368,8 +371,20 @@ async def cancel_appointment( ) _require_if_match(if_match, appointment.version) assert_transition(appointment.status, AppointmentStatus.CANCELLED) + previous_status = appointment.status appointment.status = AppointmentStatus.CANCELLED appointment.cancellation_reason = body.reason + # Flush first: a lost-update race fails here (412) before anything is audited. The audit row + # then joins the same transaction, so the cancel cannot commit without it (ADR 0016). await appointments.flush() + await audit.record( + audit_log, + actor_id=current.id, + action=AuditAction.APPOINTMENT_CANCELLED, + entity_type="appointment", + entity_id=str(appointment.id), + before={"status": previous_status.value}, + after={"status": AppointmentStatus.CANCELLED.value}, + ) await _emit(dispatcher, clock, WebhookEventType.APPOINTMENT_CANCELLED, appointment) return _appointment_response(appointment) diff --git a/src/appointments_api/enums.py b/src/appointments_api/enums.py index 131521c..54b5926 100644 --- a/src/appointments_api/enums.py +++ b/src/appointments_api/enums.py @@ -26,6 +26,16 @@ class AppointmentStatus(StrEnum): NO_SHOW = "NO_SHOW" +class AuditAction(StrEnum): + """What an ``audit_log`` row records (ADR 0016). + + Kept apart from ``WebhookEventType``: the webhook set is a public contract, while audit + actions will grow past it (device provisioning, excursions, ...). + """ + + APPOINTMENT_CANCELLED = "appointment.cancelled" + + class Weekday(IntEnum): """Weekday numbering that matches :meth:`datetime.date.weekday` (Monday is 0).""" diff --git a/src/appointments_api/repositories/audit_log.py b/src/appointments_api/repositories/audit_log.py index 21c6f3d..1315e20 100644 --- a/src/appointments_api/repositories/audit_log.py +++ b/src/appointments_api/repositories/audit_log.py @@ -1,14 +1,16 @@ -"""Audit-log repository (read side). +"""Audit-log repository. -The ``audit_log`` table is append-only (see ``models/audit.py``). This repository is the read side: -an admin-facing, keyset-paginated, filterable listing. Filters are optional and combine with AND; -ordering and the cursor come from the shared ``keyset_page`` helper (``(created_at, id)`` DESC). +The ``audit_log`` table is append-only (see ``models/audit.py``). The read side is an admin-facing, +keyset-paginated, filterable listing. Filters are optional and combine with AND; ordering and the +cursor come from the shared ``keyset_page`` helper (``(created_at, id)`` DESC). The write side is +``add_entry``, which implements ``services.audit.AuditWriter`` (ADR 0016). """ from __future__ import annotations import uuid from collections.abc import Sequence +from typing import Any from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession @@ -22,6 +24,30 @@ class AuditLogRepository: def __init__(self, session: AsyncSession) -> None: self._s = session + async def add_entry( + self, + *, + actor_id: uuid.UUID | None, + action: str, + entity_type: str, + entity_id: str, + before: dict[str, Any] | None, + after: dict[str, Any] | None, + ) -> None: + """Insert one audit row in the caller's transaction and flush it, so an insert failure + raises here and the surrounding request rolls back.""" + self._s.add( + AuditLogEntry( + actor_id=actor_id, + action=action, + entity_type=entity_type, + entity_id=entity_id, + before=before, + after=after, + ) + ) + await self._s.flush() + async def list_entries( self, *, diff --git a/src/appointments_api/services/audit.py b/src/appointments_api/services/audit.py new file mode 100644 index 0000000..444d8e1 --- /dev/null +++ b/src/appointments_api/services/audit.py @@ -0,0 +1,47 @@ +"""Audit-log writing (ADR 0016). + +``record`` writes one entry through an ``AuditWriter``. The writer is a ``typing.Protocol`` so the +service has no database or FastAPI dependency: in the app it is ``AuditLogRepository`` (which writes +in the request's transaction), in unit tests a fake that keeps entries in memory. +""" + +from __future__ import annotations + +import uuid +from typing import Any, Protocol + +from appointments_api.enums import AuditAction + + +class AuditWriter(Protocol): + async def add_entry( + self, + *, + actor_id: uuid.UUID | None, + action: str, + entity_type: str, + entity_id: str, + before: dict[str, Any] | None, + after: dict[str, Any] | None, + ) -> None: ... + + +async def record( + writer: AuditWriter, + *, + actor_id: uuid.UUID | None, + action: AuditAction, + entity_type: str, + entity_id: str, + before: dict[str, Any] | None, + after: dict[str, Any] | None, +) -> None: + """Write one audit entry: ``actor_id`` did ``action`` to ``entity_type``/``entity_id``.""" + await writer.add_entry( + actor_id=actor_id, + action=action.value, + entity_type=entity_type, + entity_id=entity_id, + before=before, + after=after, + ) diff --git a/tests/integration/test_audit_log_cancel.py b/tests/integration/test_audit_log_cancel.py new file mode 100644 index 0000000..1b0aedd --- /dev/null +++ b/tests/integration/test_audit_log_cancel.py @@ -0,0 +1,241 @@ +"""Audit-log write on appointment cancel (ADR 0016), end to end against real Postgres. + +A successful cancel records exactly one ``appointment.cancelled`` row in the same transaction; a +cancel refused inside the handler records nothing. +""" + +from __future__ import annotations + +import asyncio +import uuid +from collections.abc import Sequence +from datetime import UTC, datetime +from typing import Any + +import httpx +import pytest +from fastapi import FastAPI +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from appointments_api.api.deps import get_clock +from appointments_api.models import AuditLogEntry +from tests.fakes.clock import FixedClock +from tests.integration.helpers import auth_header + +ALL_WEEK = [(wd, "06:00", "22:00") for wd in range(7)] +NOW = datetime(2027, 6, 1, 9, 0, tzinfo=UTC) +# Relative to NOW and the clinic's 24h cutoff. +INSIDE_CUTOFF = "2027-06-01T10:00:00+00:00" + + +def _outside_cutoff(day: int, hour: int = 10) -> str: + return f"2027-06-{day:02d}T{hour:02d}:00:00+00:00" + + +@pytest.fixture +async def world(app: FastAPI, seed: Any, login: Any) -> dict[str, Any]: + """Two clinics. Clinic A has a patient, a clinician and a clinic admin; clinic B has its own + clinician. The clock is fixed at ``NOW`` so the 24h cutoff is deterministic.""" + app.dependency_overrides[get_clock] = lambda: FixedClock(NOW) + patient_id = await seed.user(role="PATIENT", email="patient@x.io") + other_patient_id = await seed.user(role="PATIENT", email="other@x.io") + admin_id = await seed.user(role="CLINIC_ADMIN", email="clinic-admin@x.io") + doc_user = await seed.user(role="CLINICIAN", email="doc@x.io") + doc_b_user = await seed.user(role="CLINICIAN", email="doc-b@x.io") + clinic_id = await seed.clinic(timezone="UTC", cutoff_hours=24) + clinic_b_id = await seed.clinic(timezone="UTC", cutoff_hours=24) + clinician_id = await seed.clinician( + clinic_id=clinic_id, user_id=doc_user, working_hours=ALL_WEEK + ) + await seed.clinician(clinic_id=clinic_b_id, user_id=doc_b_user, working_hours=ALL_WEEK) + service_id = await seed.service(clinic_id=clinic_id, duration_minutes=30) + return { + "patient_id": patient_id, + "other_patient_id": other_patient_id, + "admin_id": admin_id, + "doc_user_id": doc_user, + "clinic_id": clinic_id, + "clinician_id": clinician_id, + "service_id": service_id, + "patient_token": await login("patient@x.io"), + "other_token": await login("other@x.io"), + "admin_token": await login("clinic-admin@x.io"), + "doc_token": await login("doc@x.io"), + "doc_b_token": await login("doc-b@x.io"), + } + + +async def _book( + client: httpx.AsyncClient, world: dict[str, Any], starts_at: str, token: str | None = None +) -> tuple[str, str]: + resp = await client.post( + "/api/v1/appointments", + json={ + "clinic_id": str(world["clinic_id"]), + "clinician_id": str(world["clinician_id"]), + "service_id": str(world["service_id"]), + "starts_at": starts_at, + }, + headers=auth_header(token or world["patient_token"]), + ) + assert resp.status_code == 201, resp.json() + return str(resp.json()["id"]), resp.headers["ETag"] + + +async def _transition( + client: httpx.AsyncClient, world: dict[str, Any], appointment_id: str, etag: str, target: str +) -> str: + resp = await client.post( + f"/api/v1/appointments/{appointment_id}/transition", + json={"target_status": target}, + headers={**auth_header(world["admin_token"]), "If-Match": etag}, + ) + assert resp.status_code == 200, resp.json() + return resp.headers["ETag"] + + +async def _cancel( + client: httpx.AsyncClient, appointment_id: str, token: str, etag: str | None +) -> httpx.Response: + headers = auth_header(token) + if etag is not None: + headers["If-Match"] = etag + return await client.post( + f"/api/v1/appointments/{appointment_id}/cancel", + json={"reason": "cannot make it"}, + headers=headers, + ) + + +async def _audit_rows(app: FastAPI, entity_id: str | None = None) -> Sequence[AuditLogEntry]: + maker: async_sessionmaker[AsyncSession] = app.state.sessionmaker + async with maker() as session: + stmt = select(AuditLogEntry) + if entity_id is not None: + stmt = stmt.where(AuditLogEntry.entity_id == entity_id) + return (await session.execute(stmt)).scalars().all() + + +def _assert_cancel_row( + rows: Sequence[AuditLogEntry], *, actor_id: uuid.UUID, entity_id: str, before: str +) -> None: + assert len(rows) == 1, [(r.action, r.entity_id) for r in rows] + row = rows[0] + assert row.actor_id == actor_id + assert row.action == "appointment.cancelled" + assert row.entity_type == "appointment" + assert row.entity_id == entity_id + assert row.before == {"status": before} + assert row.after == {"status": "CANCELLED"} + + +@pytest.mark.agent_trusted +async def test_ac1_data_successful_cancel_records_one_audit_row( + client: httpx.AsyncClient, app: FastAPI, world: dict[str, Any] +) -> None: + # Patient cancels their own REQUESTED appointment outside the cutoff. + req_id, req_etag = await _book(client, world, _outside_cutoff(5)) + resp = await _cancel(client, req_id, world["patient_token"], req_etag) + assert resp.status_code == 200, resp.json() + _assert_cancel_row( + await _audit_rows(app, req_id), + actor_id=world["patient_id"], + entity_id=req_id, + before="REQUESTED", + ) + + # A CLINIC_ADMIN cancels a CONFIRMED appointment inside the cutoff. + near_id, near_etag = await _book(client, world, INSIDE_CUTOFF) + near_etag = await _transition(client, world, near_id, near_etag, "CONFIRMED") + resp = await _cancel(client, near_id, world["admin_token"], near_etag) + assert resp.status_code == 200, resp.json() + _assert_cancel_row( + await _audit_rows(app, near_id), + actor_id=world["admin_id"], + entity_id=near_id, + before="CONFIRMED", + ) + + # A clinician cancels a CONFIRMED appointment in their own clinic outside the cutoff. + doc_id, doc_etag = await _book(client, world, _outside_cutoff(6)) + doc_etag = await _transition(client, world, doc_id, doc_etag, "CONFIRMED") + resp = await _cancel(client, doc_id, world["doc_token"], doc_etag) + assert resp.status_code == 200, resp.json() + _assert_cancel_row( + await _audit_rows(app, doc_id), + actor_id=world["doc_user_id"], + entity_id=doc_id, + before="CONFIRMED", + ) + + # Different appointments cancelled: one row per entity_id, and nothing else was audited. + all_rows = await _audit_rows(app) + assert sorted(r.entity_id for r in all_rows) == sorted([req_id, near_id, doc_id]) + + +@pytest.mark.agent_trusted +async def test_ac3_data_refused_cancel_records_no_audit_row( + client: httpx.AsyncClient, app: FastAPI, world: dict[str, Any] +) -> None: + patient = world["patient_token"] + + # Unknown appointment id -> 404, and the table stays empty. + resp = await _cancel(client, str(uuid.uuid4()), patient, '"1"') + assert resp.status_code == 404 + assert list(await _audit_rows(app)) == [] + + # Another patient's appointment -> 403. + other_id, other_etag = await _book(client, world, _outside_cutoff(5), world["other_token"]) + resp = await _cancel(client, other_id, patient, other_etag) + assert resp.status_code == 403 + assert list(await _audit_rows(app, other_id)) == [] + + # Own appointment inside the cutoff -> 403. + near_id, near_etag = await _book(client, world, INSIDE_CUTOFF) + resp = await _cancel(client, near_id, patient, near_etag) + assert resp.status_code == 403 + assert list(await _audit_rows(app, near_id)) == [] + + # No If-Match -> 428; stale If-Match -> 412. + own_id, _ = await _book(client, world, _outside_cutoff(6)) + resp = await _cancel(client, own_id, patient, None) + assert resp.status_code == 428 + resp = await _cancel(client, own_id, patient, '"999"') + assert resp.status_code == 412 + assert list(await _audit_rows(app, own_id)) == [] + + # COMPLETED and NO_SHOW are terminal -> 409. + for day, terminal in ((7, "COMPLETED"), (8, "NO_SHOW")): + t_id, t_etag = await _book(client, world, _outside_cutoff(day)) + t_etag = await _transition(client, world, t_id, t_etag, "CONFIRMED") + t_etag = await _transition(client, world, t_id, t_etag, terminal) + resp = await _cancel(client, t_id, world["admin_token"], t_etag) + assert resp.status_code == 409, (terminal, resp.json()) + assert list(await _audit_rows(app, t_id)) == [], terminal + + # A clinician from another clinic -> 403. + foreign_id, foreign_etag = await _book(client, world, _outside_cutoff(9)) + resp = await _cancel(client, foreign_id, world["doc_b_token"], foreign_etag) + assert resp.status_code == 403 + assert list(await _audit_rows(app, foreign_id)) == [] + + # Cancel succeeds, then a second cancel with the new ETag is 409: still exactly one row. + twice_id, twice_etag = await _book(client, world, _outside_cutoff(10)) + first = await _cancel(client, twice_id, patient, twice_etag) + assert first.status_code == 200, first.json() + second = await _cancel(client, twice_id, patient, first.headers["ETag"]) + assert second.status_code == 409 + assert len(await _audit_rows(app, twice_id)) == 1 + + # Two concurrent cancels with the same valid If-Match: one 200, one 412, exactly one row. + race_id, race_etag = await _book(client, world, _outside_cutoff(11)) + r1, r2 = await asyncio.gather( + _cancel(client, race_id, patient, race_etag), + _cancel(client, race_id, patient, race_etag), + ) + assert sorted([r1.status_code, r2.status_code]) == [200, 412] + assert len(await _audit_rows(app, race_id)) == 1 + + # Across the whole run, only the two successful cancels were audited. + assert sorted(r.entity_id for r in await _audit_rows(app)) == sorted([twice_id, race_id]) diff --git a/tests/unit/test_audit.py b/tests/unit/test_audit.py new file mode 100644 index 0000000..3607cd2 --- /dev/null +++ b/tests/unit/test_audit.py @@ -0,0 +1,68 @@ +"""Unit tests for the audit service (ADR 0016). A fake in-memory writer, no I/O.""" + +from __future__ import annotations + +import uuid +from typing import Any + +from appointments_api.enums import AuditAction +from appointments_api.services import audit + + +class FakeAuditWriter: + def __init__(self) -> None: + self.entries: list[dict[str, Any]] = [] + + async def add_entry( + self, + *, + actor_id: uuid.UUID | None, + action: str, + entity_type: str, + entity_id: str, + before: dict[str, Any] | None, + after: dict[str, Any] | None, + ) -> None: + self.entries.append( + { + "actor_id": actor_id, + "action": action, + "entity_type": entity_type, + "entity_id": entity_id, + "before": before, + "after": after, + } + ) + + +def test_action_vocabulary_value() -> None: + assert AuditAction.APPOINTMENT_CANCELLED.value == "appointment.cancelled" + + +async def test_record_writes_one_entry_with_every_field() -> None: + writer = FakeAuditWriter() + actor = uuid.uuid4() + entity = str(uuid.uuid4()) + + await audit.record( + writer, + actor_id=actor, + action=AuditAction.APPOINTMENT_CANCELLED, + entity_type="appointment", + entity_id=entity, + before={"status": "REQUESTED"}, + after={"status": "CANCELLED"}, + ) + + assert writer.entries == [ + { + "actor_id": actor, + "action": "appointment.cancelled", + "entity_type": "appointment", + "entity_id": entity, + "before": {"status": "REQUESTED"}, + "after": {"status": "CANCELLED"}, + } + ] + # The stored action is the plain string token, not the enum member. + assert type(writer.entries[0]["action"]) is str From 49a19becb1900da1d2f6a74df7e6f17132bc3655 Mon Sep 17 00:00:00 2001 From: evg-g <229170690+evg-g@users.noreply.github.com> Date: Sat, 3 Oct 2026 20:13:24 +0300 Subject: [PATCH 3/5] test(audit): AURORA-2 AC2.api read cancel entry from audit-log API Co-Authored-By: Claude Opus 5.5 --- tests/integration/test_audit_log_cancel.py | 55 ++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/tests/integration/test_audit_log_cancel.py b/tests/integration/test_audit_log_cancel.py index 1b0aedd..c608ab7 100644 --- a/tests/integration/test_audit_log_cancel.py +++ b/tests/integration/test_audit_log_cancel.py @@ -239,3 +239,58 @@ async def test_ac3_data_refused_cancel_records_no_audit_row( # Across the whole run, only the two successful cancels were audited. assert sorted(r.entity_id for r in await _audit_rows(app)) == sorted([twice_id, race_id]) + + +async def _read_audit_log( + client: httpx.AsyncClient, token: str, params: dict[str, str] | None = None +) -> list[dict[str, Any]]: + resp = await client.get("/api/v1/audit-log", params=params or {}, headers=auth_header(token)) + assert resp.status_code == 200, resp.json() + data: list[dict[str, Any]] = resp.json()["data"] + return data + + +def _assert_cancel_item(item: dict[str, Any], *, actor_id: uuid.UUID, entity_id: str) -> None: + assert item["actor_id"] == str(actor_id) + assert item["action"] == "appointment.cancelled" + assert item["entity_type"] == "appointment" + assert item["entity_id"] == entity_id + assert item["before"] == {"status": "REQUESTED"} + assert item["after"] == {"status": "CANCELLED"} + uuid.UUID(item["id"]) + datetime.fromisoformat(item["created_at"]) + + +@pytest.mark.agent_trusted +async def test_ac2_api_admin_reads_cancel_entry_and_filters_by_entity( + client: httpx.AsyncClient, seed: Any, login: Any, world: dict[str, Any] +) -> None: + await seed.user(role="PLATFORM_ADMIN", email="platform-admin@x.io") + platform_token = await login("platform-admin@x.io") + + # Patient cancels X and Y (both REQUESTED, outside the cutoff). + x_id, x_etag = await _book(client, world, _outside_cutoff(5)) + y_id, y_etag = await _book(client, world, _outside_cutoff(6)) + for appt_id, etag in ((x_id, x_etag), (y_id, y_etag)): + resp = await _cancel(client, appt_id, world["patient_token"], etag) + assert resp.status_code == 200, resp.json() + + # A CLINIC_ADMIN reads the unfiltered log: the entry for X is there with every field. + items = await _read_audit_log(client, world["admin_token"]) + x_items = [i for i in items if i["entity_id"] == x_id] + assert len(x_items) == 1, items + _assert_cancel_item(x_items[0], actor_id=world["patient_id"], entity_id=x_id) + + # A PLATFORM_ADMIN filters by entity_id = X: exactly the entry for X. + by_entity = await _read_audit_log(client, platform_token, {"entity_id": x_id}) + assert len(by_entity) == 1, by_entity + _assert_cancel_item(by_entity[0], actor_id=world["patient_id"], entity_id=x_id) + assert by_entity[0]["id"] == x_items[0]["id"] + + # Both admin roles, filtered by entity_type + action + entity_id: exactly the entry for X. + combined = {"entity_type": "appointment", "action": "appointment.cancelled", "entity_id": x_id} + for token in (world["admin_token"], platform_token): + filtered = await _read_audit_log(client, token, combined) + assert len(filtered) == 1, filtered + _assert_cancel_item(filtered[0], actor_id=world["patient_id"], entity_id=x_id) + assert filtered[0]["id"] == x_items[0]["id"] From fb49b73c6acb9224d23e0b4a0c09c58caff1b78f Mon Sep 17 00:00:00 2001 From: evg-g <229170690+evg-g@users.noreply.github.com> Date: Sat, 3 Oct 2026 20:19:07 +0300 Subject: [PATCH 4/5] test(audit): AURORA-2 AC1.data audit write failure rolls back the cancel Co-Authored-By: Claude Opus 5.5 --- tests/integration/test_audit_log_cancel.py | 44 +++++++++++++++++++++- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_audit_log_cancel.py b/tests/integration/test_audit_log_cancel.py index c608ab7..1a6222d 100644 --- a/tests/integration/test_audit_log_cancel.py +++ b/tests/integration/test_audit_log_cancel.py @@ -7,6 +7,7 @@ from __future__ import annotations import asyncio +import json import uuid from collections.abc import Sequence from datetime import UTC, datetime @@ -18,8 +19,10 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker -from appointments_api.api.deps import get_clock -from appointments_api.models import AuditLogEntry +from appointments_api.api.deps import SessionDep, get_audit_log_repository, get_clock +from appointments_api.models import Appointment, AuditLogEntry +from appointments_api.repositories.audit_log import AuditLogRepository +from appointments_api.repositories.redis_webhooks import RedisEventQueue from tests.fakes.clock import FixedClock from tests.integration.helpers import auth_header @@ -294,3 +297,40 @@ async def test_ac2_api_admin_reads_cancel_entry_and_filters_by_entity( assert len(filtered) == 1, filtered _assert_cancel_item(filtered[0], actor_id=world["patient_id"], entity_id=x_id) assert filtered[0]["id"] == x_items[0]["id"] + + +class _FailingAuditLogRepository(AuditLogRepository): + """Writes nothing: the audit insert fails inside the request, after the cancel was flushed.""" + + async def add_entry(self, **_: Any) -> None: + raise RuntimeError("audit_log insert failed") + + +@pytest.mark.agent_trusted +async def test_ac1_data_audit_write_failure_rolls_back_cancel( + client: httpx.AsyncClient, app: FastAPI, world: dict[str, Any] +) -> None: + # The cancel and its audit row share one transaction (ADR 0016): if the audit write fails, the + # request is a 500, the appointment keeps its prior status, and no webhook event goes out. + appt_id, etag = await _book(client, world, _outside_cutoff(5)) + await app.state.redis.delete(RedisEventQueue.KEY) # ignore the created event + + def _failing_repo(session: SessionDep) -> AuditLogRepository: + return _FailingAuditLogRepository(session) + + app.dependency_overrides[get_audit_log_repository] = _failing_repo + # A client that returns the 500 response instead of re-raising the app's exception. + transport = httpx.ASGITransport(app=app, raise_app_exceptions=False) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as raw: + resp = await _cancel(raw, appt_id, world["patient_token"], etag) + assert resp.status_code == 500, resp.text + + maker: async_sessionmaker[AsyncSession] = app.state.sessionmaker + async with maker() as session: + appointment = await session.get(Appointment, uuid.UUID(appt_id)) + assert appointment is not None + assert appointment.status.value == "REQUESTED" + assert appointment.cancellation_reason is None + assert list(await _audit_rows(app, appt_id)) == [] + events = [json.loads(e) for e in await app.state.redis.lrange(RedisEventQueue.KEY, 0, -1)] + assert [e for e in events if e["type"] == "appointment.cancelled"] == [], events From b0fde2177609c674d138957aeac410ebd491c375 Mon Sep 17 00:00:00 2001 From: evg-g <229170690+evg-g@users.noreply.github.com> Date: Sat, 3 Oct 2026 20:27:28 +0300 Subject: [PATCH 5/5] test(audit): AURORA-2 AC1.data failed cancel commit leaves no audit row Co-Authored-By: Claude Opus 5.5 --- tests/integration/test_audit_log_cancel.py | 54 +++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_audit_log_cancel.py b/tests/integration/test_audit_log_cancel.py index 1a6222d..0352ace 100644 --- a/tests/integration/test_audit_log_cancel.py +++ b/tests/integration/test_audit_log_cancel.py @@ -16,7 +16,7 @@ import httpx import pytest from fastapi import FastAPI -from sqlalchemy import select +from sqlalchemy import select, text from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker from appointments_api.api.deps import SessionDep, get_audit_log_repository, get_clock @@ -334,3 +334,55 @@ def _failing_repo(session: SessionDep) -> AuditLogRepository: assert list(await _audit_rows(app, appt_id)) == [] events = [json.loads(e) for e in await app.state.redis.lrange(RedisEventQueue.KEY, 0, -1)] assert [e for e in events if e["type"] == "appointment.cancelled"] == [], events + + +@pytest.mark.agent_trusted +async def test_ac1_data_failed_cancel_commit_leaves_no_audit_row( + client: httpx.AsyncClient, app: FastAPI, world: dict[str, Any] +) -> None: + # The real audit wiring (no override): if the request's COMMIT fails after the audit write, the + # audit row must roll back with the cancel: the cancel is not durable, and neither is its row. + # A test-only deferred constraint trigger raises at COMMIT time for this one appointment, so + # only a row written in the same transaction as the cancel vanishes with it. + appt_id, etag = await _book(client, world, _outside_cutoff(5)) + target = uuid.UUID(appt_id) # validated UUID, safe to inline in the trigger body + engine = app.state.engine + async with engine.begin() as conn: + await conn.execute( + text( + "CREATE FUNCTION ac1_fail_cancel_commit() RETURNS trigger LANGUAGE plpgsql AS $$ " + f"BEGIN IF NEW.id = '{target}'::uuid AND NEW.status::text = 'CANCELLED' THEN " + "RAISE EXCEPTION 'ac1: commit of cancel refused'; END IF; RETURN NEW; END $$" + ) + ) + await conn.execute( + text( + "CREATE CONSTRAINT TRIGGER ac1_fail_cancel_commit AFTER UPDATE ON appointments " + "DEFERRABLE INITIALLY DEFERRED FOR EACH ROW " + "EXECUTE FUNCTION ac1_fail_cancel_commit()" + ) + ) + try: + transport = httpx.ASGITransport(app=app, raise_app_exceptions=False) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as raw: + resp = await _cancel(raw, appt_id, world["patient_token"], etag) + # get_session commits after the response is sent (FastAPI yield-dependency exit), so the + # client may still see 200 for a cancel whose COMMIT failed. Either way the handler must + # have run to the end (cancel flushed, audit row written) before the commit was refused. + assert resp.status_code == 200 or resp.status_code >= 500, (resp.status_code, resp.text) + if resp.status_code == 200: + assert resp.json()["status"] == "CANCELLED", resp.json() + + maker: async_sessionmaker[AsyncSession] = app.state.sessionmaker + async with maker() as session: + appointment = await session.get(Appointment, target) + assert appointment is not None + assert appointment.status.value == "REQUESTED" + assert appointment.cancellation_reason is None + assert list(await _audit_rows(app, appt_id)) == [] + finally: + async with engine.begin() as conn: + await conn.execute( + text("DROP TRIGGER IF EXISTS ac1_fail_cancel_commit ON appointments") + ) + await conn.execute(text("DROP FUNCTION IF EXISTS ac1_fail_cancel_commit()"))