fix(db): do not report success when the database did not save the change (AURORA-3)

.gitleaksignore

@@ -13,3 +13,8 @@
 # the literal "0123456789abcdef". A sequential hex string written to be readable in test output;
 # it signs nothing outside the test process. Flagged by the generic-api-key rule on entropy alone.
 7a2b78bba835061243e0d8b24acdffd09ffec1d0:tests/integration/test_webhooks_api.py:generic-api-key:27
+
+# tests/integration/test_commit_failure.py:37 — the same fake webhook test secret "0123456789abcdef",
+# copied into the AURORA-3 commit-failure tests. It signs nothing outside the test process. Flagged
+# by the generic-api-key rule on entropy alone; the value was replaced in a50c7f9.
+52380ec296e26e2a6ae13701064b1b611d74bd9e:tests/integration/test_commit_failure.py:generic-api-key:37

CONTEXT.md

@@ -18,3 +18,9 @@ A short glossary of the terms tickets and ADRs use. Add a term when a ticket int
   (ADR 0016).
 - **Audit writer** — the `AuditWriter` Protocol the audit service records through; implemented by
   `AuditLogRepository` (ADR 0016).
+- **Commit failure** — the database refuses the request's COMMIT (a deferred constraint, a
+  serialization failure, a lost connection). The client gets an error, and nothing from the request
+  is saved (ADR 0017).
+- **Transaction hooks** — the request-scoped `on_commit` / `on_rollback` callback lists that
+  `get_session` runs after the commit or the rollback; webhook publishing and booking idempotency
+  use them (ADR 0017).

docs/ERROR_CATALOG.md

@@ -19,6 +19,7 @@ Every error response uses `application/problem+json` (RFC 9457). The body always
 | `validation-error` | 422 | Request body/query failed validation, or a business rule rejected the input (e.g. time outside working hours). `errors[]` lists the offending fields. |
 | `idempotency-key-reused` | 422 | An `Idempotency-Key` was replayed with a different request body. |
 | `rate-limited` | 429 | The principal exceeded its rate-limit window. Carries `Retry-After` and `RateLimit-*` headers. |
+| `internal-error` | 500 | An unexpected server error, including a COMMIT the database refused (ADR 0017). Nothing from the request is saved. |
 
 ## Examples
 

docs/KNOWN_GAPS.md

@@ -29,12 +29,12 @@ focused on the surface; tracked here so it is not forgotten.
 
 ## Webhook emission is not transactional with the DB commit
 
-A state change publishes its event to Redis during the request, after the row has flushed but before
-the surrounding transaction commits. A crash in that small window could emit an event for a change
-that later rolled back, or commit a change whose event was not emitted. Receivers already must dedupe
-on `X-Webhook-Id` (delivery is at-least-once), but exactly-once *emission* needs a transactional
-outbox: write the event to a DB table in the same transaction, and have a relay publish it. Deliberately
-out of scope for milestone 4; see ADR 0010.
+A state change publishes its event to Redis from an `on_commit` transaction hook, after the COMMIT
+succeeds (ADR 0017), so a failed COMMIT never emits an event. One window remains: a crash, or a Redis
+failure, between the COMMIT and the hook loses that event (a failed publish is logged, the change
+stays saved). Receivers already must dedupe on `X-Webhook-Id` (delivery is at-least-once), but
+exactly-once *emission* needs a transactional outbox: write the event to a DB table in the same
+transaction, and have a relay publish it. Deferred to AURORA-4; see ADR 0010 and ADR 0017.
 
 ## Webhook worker runs on demand, and dead letters are not yet surfaced
 

docs/adr/0007-idempotency-keys.md

@@ -1,6 +1,7 @@
 # 7. Idempotency-Key on create
 
-- Status: accepted
+- Status: accepted; the timing of complete/release is superseded by ADR 0017 (they run after
+  COMMIT / rollback)
 - Date: 2026-09-27
 
 ## Context

docs/adr/0010-signed-webhooks-and-delivery-worker.md

@@ -1,6 +1,7 @@
 # 10. Signed webhooks with a retrying delivery worker
 
-- Status: accepted
+- Status: accepted; the emission-timing consequence is superseded by ADR 0017 (events publish
+  after COMMIT)
 - Date: 2026-09-27
 
 ## Context

docs/adr/0017-commit-before-response-and-transaction-hooks.md

@@ -0,0 +1,53 @@
+# ADR 0017 — Commit before the response, and transaction hooks
+
+Status: accepted (AURORA-3)
+
+## Context
+
+`get_session` (`db.py`) commits in the exit of a yield dependency. In FastAPI 0.141.1 the default
+dependency scope runs that exit after the response is sent, so a request whose COMMIT fails still
+returns its success response while the database keeps the old state. This affects every write
+endpoint that uses `SessionDep`. It was found while proving AURORA-2.
+
+Two side effects make the same mistake one step later, even once the commit moves:
+
+- Webhook events are published to Redis before the commit (`KNOWN_GAPS.md`, ADR 0010), so a failed
+  commit can still notify receivers about a change that was not saved.
+- Booking with an `Idempotency-Key` stores the 201 response before the commit, so after a failed
+  commit a retry with the same key replays a success for a booking that does not exist.
+
+## Decision
+
+- **Commit before the response.** `SessionDep` is `Depends(get_session, scope="function")`. The
+  commit runs after the handler and before the response is sent, so a commit error goes through the
+  normal exception handlers.
+- **A catch-all error handler.** An unhandled exception returns 500 as `application/problem+json`
+  (RFC 9457, ADR 0006). `IntegrityError` keeps its 409 mapping and `StaleDataError` its 412.
+- **Transaction hooks.** A request-scoped `TransactionHooks` object holds two callback lists,
+  `on_commit` and `on_rollback`, and is provided as a dependency. `get_session` runs `on_commit`
+  after `commit()` succeeds and `on_rollback` after a rollback. Each hook runs on its own: a failing
+  `on_commit` hook is logged and does not fail the request, because the change is already saved
+  and the response must match the database.
+- **Webhooks after commit.** `_emit` registers the publish as an `on_commit` hook instead of
+  publishing during the request.
+- **Booking idempotency after commit.** `IdempotencyService.complete()` is registered as an
+  `on_commit` hook and `release()` as an `on_rollback` hook, so a failed commit frees the key and a
+  retry with the same key is processed as a new request.
+- **One way to force a failed commit in tests.** A shared integration fixture installs a test-only
+  `DEFERRABLE INITIALLY DEFERRED` constraint trigger that raises at COMMIT for one chosen row, and
+  always drops it.
+
+## Consequences
+
+- No write endpoint can return success for a change the database did not save.
+- A crash, or a Redis failure, between the commit and an `on_commit` hook loses that hook's work: a
+  webhook event is not published (logged), or a booking's idempotency record is not stored. In the
+  second case the key's pending marker stays, so a retry with the same key gets 409 "still being
+  processed" until the key's TTL (`idempotency_ttl_seconds`, 24 h) expires; the booking itself is
+  saved, and a retry with a new key for the same slot is refused by the double-booking constraint.
+  Exactly-once emission needs a transactional outbox; that is deferred to a later ticket (AURORA-4).
+- The `KNOWN_GAPS.md` entry "Webhook emission is not transactional with the DB commit" narrows to
+  that crash/Redis window, and the "commit after response" behaviour is gone.
+- The behaviour depends on FastAPI's dependency `scope`. The commit-failure integration tests guard
+  it against a framework change.
+- No migration and no OpenAPI contract change.

src/appointments_api/api/deps.py

@@ -18,7 +18,7 @@
 from appointments_api.api.errors import ForbiddenError, UnauthorizedError
 from appointments_api.api.pagination import Cursor, decode_cursor
 from appointments_api.config import Settings, get_settings
-from appointments_api.db import get_redis, get_session
+from appointments_api.db import get_redis, get_session, get_transaction_hooks
 from appointments_api.enums import UserRole
 from appointments_api.models import Device, User
 from appointments_api.repositories.appointments import AppointmentRepository
@@ -41,12 +41,18 @@
 from appointments_api.services.idempotency import IdempotencyService
 from appointments_api.services.telemetry.ingestion import TelemetryIngestionService
 from appointments_api.services.tokens import TokenService
+from appointments_api.services.transaction_hooks import TransactionHooks
 from appointments_api.services.webhooks.dispatcher import WebhookDispatcher
 from appointments_api.telemetry_wiring import build_ingestion_service
 
-SessionDep = Annotated[AsyncSession, Depends(get_session)]
+# scope="function": the commit in get_session's exit runs after the handler and BEFORE the response
+# is sent, so a failed COMMIT goes through the exception handlers instead of following a success
+# response (ADR 0017).
+SessionDep = Annotated[AsyncSession, Depends(get_session, scope="function")]
 RedisDep = Annotated[Redis, Depends(get_redis)]
 SettingsDep = Annotated[Settings, Depends(get_settings)]
+# The same object get_session runs after COMMIT (on_commit) or after rollback (on_rollback).
+TransactionHooksDep = Annotated[TransactionHooks, Depends(get_transaction_hooks)]
 
 
 def get_user_repository(session: SessionDep) -> UserRepository:

src/appointments_api/api/errors.py

@@ -231,3 +231,10 @@ async def _integrity(request: Request, exc: IntegrityError) -> JSONResponse:
         else:
             problem = ConflictError(detail="The change conflicts with existing data.")
         return _json(problem.to_problem(request.url.path))
+
+    @app.exception_handler(Exception)
+    async def _unhandled(request: Request, exc: Exception) -> JSONResponse:
+        # Catch-all (ADR 0017): any unhandled error, including a failed COMMIT, is a 500 problem
+        # document. Starlette still re-raises it after this response, so it is logged as usual.
+        problem = APIError(detail="An unexpected error occurred.")
+        return _json(problem.to_problem(request.url.path))

src/appointments_api/api/routers/appointments.py

@@ -33,6 +33,7 @@
     IdempotencyServiceDep,
     PageParams,
     ServiceRepoDep,
+    TransactionHooksDep,
     UserRepoDep,
     WebhookDispatcherDep,
 )
@@ -62,9 +63,11 @@
 from appointments_api.services.clock import Clock
 from appointments_api.services.idempotency import (
     BeginState,
+    IdempotencyService,
     StoredResponse,
     request_fingerprint,
 )
+from appointments_api.services.transaction_hooks import TransactionHooks
 from appointments_api.services.webhooks.dispatcher import WebhookDispatcher
 from appointments_api.services.webhooks.events import (
     WebhookEvent,
@@ -99,20 +102,51 @@ def _replayed_response(stored: StoredResponse) -> JSONResponse:
     return JSONResponse(status_code=stored.status_code, content=stored.body, headers=headers)
 
 
-async def _emit(
+def _emit(
+    hooks: TransactionHooks,
     dispatcher: WebhookDispatcher,
     clock: Clock,
     event_type: WebhookEventType,
     appointment: Appointment,
 ) -> None:
-    """Publish a state-change event for the webhook worker to fan out and deliver."""
+    """Publish a state-change event for the webhook worker, once the transaction has committed.
+
+    The event is built now (from the flushed row the response also shows) but published by an
+    ``on_commit`` hook, so a failed COMMIT publishes nothing (ADR 0017).
+    """
     event = WebhookEvent(
         id=uuid.uuid4().hex,
         type=event_type.value,
         occurred_at=clock.now(),
         data={"appointment": _serialize(appointment)},
     )
-    await dispatcher.dispatch(event)
+
+    async def publish() -> None:
+        await dispatcher.dispatch(event)
+
+    hooks.on_commit(publish)
+
+
+def _register_idempotency_release(
+    hooks: TransactionHooks, idempotency: IdempotencyService, key: str, fingerprint: str
+) -> None:
+    async def release() -> None:
+        await idempotency.release(_IDEMPOTENCY_SCOPE, key, fingerprint)
+
+    hooks.on_rollback(release)
+
+
+def _register_idempotency_complete(
+    hooks: TransactionHooks,
+    idempotency: IdempotencyService,
+    key: str,
+    fingerprint: str,
+    response: StoredResponse,
+) -> None:
+    async def complete() -> None:
+        await idempotency.complete(_IDEMPOTENCY_SCOPE, key, fingerprint, response)
+
+    hooks.on_commit(complete)
 
 
 async def _assert_can_access(
@@ -220,6 +254,7 @@ async def create_appointment(
     appointments: AppointmentRepoDep,
     idempotency: IdempotencyServiceDep,
     dispatcher: WebhookDispatcherDep,
+    hooks: TransactionHooksDep,
     clock: ClockDep,
     idempotency_key: Annotated[str | None, Header(alias="Idempotency-Key")] = None,
 ) -> Response:
@@ -237,21 +272,20 @@ async def create_appointment(
             raise IdempotencyKeyReusedError(
                 "This Idempotency-Key was already used for a different request body."
             )
+        # This request now holds the key. If the transaction rolls back (the handler fails or the
+        # COMMIT is refused), drop the pending marker so a retry, with any body, starts fresh: a
+        # failed attempt must not poison the key (ADR 0017).
+        _register_idempotency_release(hooks, idempotency, idempotency_key, fingerprint)
 
-    try:
-        appointment = await _build_appointment(
-            body, current, users, clinics, clinicians, services, appointments
-        )
-    except Exception:
-        # A failed attempt must not poison the key: drop the pending marker so a corrected retry
-        # can start fresh.
-        if idempotency_key is not None and fingerprint is not None:
-            await idempotency.release(_IDEMPOTENCY_SCOPE, idempotency_key, fingerprint)
-        raise
+    appointment = await _build_appointment(
+        body, current, users, clinics, clinicians, services, appointments
+    )
 
     if idempotency_key is not None and fingerprint is not None:
-        await idempotency.complete(
-            _IDEMPOTENCY_SCOPE,
+        # Store the 201 for replay only once the COMMIT has succeeded (ADR 0017).
+        _register_idempotency_complete(
+            hooks,
+            idempotency,
             idempotency_key,
             fingerprint,
             StoredResponse(
@@ -260,7 +294,7 @@ async def create_appointment(
                 etag=make_etag(appointment.version),
             ),
         )
-    await _emit(dispatcher, clock, WebhookEventType.APPOINTMENT_CREATED, appointment)
+    _emit(hooks, dispatcher, clock, WebhookEventType.APPOINTMENT_CREATED, appointment)
     return _appointment_response(appointment, status_code=status.HTTP_201_CREATED)
 
 
@@ -321,6 +355,7 @@ async def transition_appointment(
     appointments: AppointmentRepoDep,
     clinicians: ClinicianRepoDep,
     dispatcher: WebhookDispatcherDep,
+    hooks: TransactionHooksDep,
     clock: ClockDep,
     if_match: Annotated[str | None, Header(alias="If-Match")] = None,
 ) -> Response:
@@ -338,7 +373,7 @@ async def transition_appointment(
     await appointments.flush()
     event_type = event_type_for_status(appointment.status)
     if event_type is not None:
-        await _emit(dispatcher, clock, event_type, appointment)
+        _emit(hooks, dispatcher, clock, event_type, appointment)
     return _appointment_response(appointment)
 
 
@@ -352,6 +387,7 @@ async def cancel_appointment(
     clinicians: ClinicianRepoDep,
     clock: ClockDep,
     dispatcher: WebhookDispatcherDep,
+    hooks: TransactionHooksDep,
     audit_log: AuditLogRepoDep,
     if_match: Annotated[str | None, Header(alias="If-Match")] = None,
 ) -> Response:
@@ -386,5 +422,5 @@ async def cancel_appointment(
         before={"status": previous_status.value},
         after={"status": AppointmentStatus.CANCELLED.value},
     )
-    await _emit(dispatcher, clock, WebhookEventType.APPOINTMENT_CANCELLED, appointment)
+    _emit(hooks, dispatcher, clock, WebhookEventType.APPOINTMENT_CANCELLED, appointment)
     return _appointment_response(appointment)

src/appointments_api/db.py

@@ -20,6 +20,7 @@
 from starlette.requests import Request
 
 from appointments_api.config import Settings, get_settings
+from appointments_api.services.transaction_hooks import TransactionHooks
 
 
 def create_engine(settings: Settings | None = None) -> AsyncEngine:
@@ -37,16 +38,32 @@ def create_redis(settings: Settings | None = None) -> Redis:
     return client
 
 
+def get_transaction_hooks(request: Request) -> TransactionHooks:
+    """The request's ``TransactionHooks``, created on first use and shared for the request."""
+    hooks: TransactionHooks | None = getattr(request.state, "transaction_hooks", None)
+    if hooks is None:
+        hooks = TransactionHooks()
+        request.state.transaction_hooks = hooks
+    return hooks
+
+
 async def get_session(request: Request) -> AsyncIterator[AsyncSession]:
-    """Yield a session, committing on success and rolling back on any error."""
+    """Yield a session, committing on success and rolling back on any error.
+
+    After the COMMIT succeeds the request's ``on_commit`` hooks run; after a rollback its
+    ``on_rollback`` hooks run (ADR 0017). A failing hook is logged and does not fail the request.
+    """
     maker: async_sessionmaker[AsyncSession] = request.app.state.sessionmaker
+    hooks = get_transaction_hooks(request)
     async with maker() as session:
         try:
             yield session
             await session.commit()
         except Exception:
             await session.rollback()
+            await hooks.run_rollback_hooks()
             raise
+    await hooks.run_commit_hooks()
 
 
 def get_redis(request: Request) -> Redis: