M39: harden application and acceptance gates

backend/app/services/data_quality.py

@@ -130,7 +130,12 @@ def _open_issue(
 
 def _scan_duplicate_customers(db: Session, scan: ScanResult) -> None:
     customers = list(
-        db.scalars(select(Customer).where(Customer.merged_into_customer_id.is_(None))).all()
+        db.scalars(
+            select(Customer).where(
+                Customer.merged_into_customer_id.is_(None),
+                Customer.anonymized_at.is_(None),
+            )
+        ).all()
     )
     customers.sort(key=lambda c: c.public_ref)
     # The threshold cannot be reached without an exact email (60 points) or phone
@@ -170,9 +175,7 @@ def _scan_duplicate_customers(db: Session, scan: ScanResult) -> None:
         ratio = SequenceMatcher(None, name_a, name_b).ratio()
         if ratio >= 0.5:
             score += round(ratio * 30)
-            signals.append(
-                {"code": "duplicate.similar_name", "params": {"score": round(ratio, 2)}}
-            )
+            signals.append({"code": "duplicate.similar_name", "params": {"score": round(ratio, 2)}})
             summary_parts.append("similar name")
 
         if score >= DUPLICATE_THRESHOLD:
@@ -191,8 +194,13 @@ def _scan_duplicate_customers(db: Session, scan: ScanResult) -> None:
 
 
 def _scan_missing_required_fields(db: Session, scan: ScanResult) -> None:
+    # Anonymised customers have had their contact data removed on purpose; flagging
+    # them as "missing required field" would only be resolvable by re-entering PII.
     for customer in db.scalars(
-        select(Customer).where(Customer.merged_into_customer_id.is_(None))
+        select(Customer).where(
+            Customer.merged_into_customer_id.is_(None),
+            Customer.anonymized_at.is_(None),
+        )
     ).all():
         missing = [f for f in REQUIRED_CUSTOMER_FIELDS if not getattr(customer, f)]
         if not customer.email and not customer.phone:
@@ -519,6 +527,30 @@ def resolve_odometer_regression(
             "This issue is not an odometer_regression issue.",
             status_code=409,
         )
+    # Lock order is booking -> vehicle everywhere (checkout, return, reschedule); taking
+    # the vehicle lock first here would be a deadlock waiting to happen under concurrency.
+    booking: Booking | None = None
+    if body.decision != "retain_canonical":
+        related_refs = issue.evidence_json.get("related_refs", [])
+        if body.booking_ref not in related_refs:
+            raise AppError(
+                "INVALID_BOOKING_REFERENCE",
+                "booking_ref must be one of this issue's related bookings.",
+                status_code=422,
+            )
+        if body.corrected_odometer_km is None:
+            raise AppError(
+                "CORRECTED_VALUE_REQUIRED",
+                "corrected_odometer_km is required when correcting a reading.",
+                status_code=422,
+            )
+        booking = db.scalar(
+            select(Booking).where(Booking.public_ref == body.booking_ref).with_for_update()
+        )
+        if booking is None:
+            raise AppError(
+                "BOOKING_NOT_FOUND", "The booking to correct was not found.", status_code=404
+            )
     vehicle = db.scalar(select(Vehicle).where(Vehicle.id == issue.entity_id).with_for_update())
     if vehicle is None:
         raise AppError(
@@ -539,19 +571,7 @@ def resolve_odometer_regression(
             metadata={"issue_ref": issue.public_ref, "canonical_odometer_km": vehicle.odometer_km},
         )
     else:
-        related_refs = issue.evidence_json.get("related_refs", [])
-        if body.booking_ref not in related_refs:
-            raise AppError(
-                "INVALID_BOOKING_REFERENCE",
-                "booking_ref must be one of this issue's related bookings.",
-                status_code=422,
-            )
-        if body.corrected_odometer_km is None:
-            raise AppError(
-                "CORRECTED_VALUE_REQUIRED",
-                "corrected_odometer_km is required when correcting a reading.",
-                status_code=422,
-            )
+        assert booking is not None and body.corrected_odometer_km is not None
         # Never silently lower the canonical odometer: a correction must be at or above
         # the current canonical value, otherwise it would just create a new regression.
         if body.corrected_odometer_km < vehicle.odometer_km:
@@ -563,13 +583,6 @@ def resolve_odometer_regression(
                 ),
                 status_code=422,
             )
-        booking = db.scalar(
-            select(Booking).where(Booking.public_ref == body.booking_ref).with_for_update()
-        )
-        if booking is None:
-            raise AppError(
-                "BOOKING_NOT_FOUND", "The booking to correct was not found.", status_code=404
-            )
 
         before = {
             "booking_end_odometer_km": booking.end_odometer_km,
@@ -810,6 +823,16 @@ def apply_recommended_status(
 
 
 MERGEABLE_FIELDS = ("first_name", "last_name", "email", "phone", "postal_code", "city")
+# Mirrors the column lengths in app/models/customer.py so an override can never fail with
+# a database DataError (500) instead of a validation error.
+_MERGEABLE_FIELD_MAX_LENGTH = {
+    "first_name": 80,
+    "last_name": 80,
+    "email": 200,
+    "phone": 40,
+    "postal_code": 20,
+    "city": 120,
+}
 
 
 def merge_customers(
@@ -839,12 +862,26 @@ def merge_customers(
         )
     loser_ref = next(ref for ref in candidate_refs if ref != survivor_ref)
 
-    survivor = db.scalar(select(Customer).where(Customer.public_ref == survivor_ref))
-    loser = db.scalar(select(Customer).where(Customer.public_ref == loser_ref))
+    # Lock both rows in a deterministic order (by public_ref) so two concurrent merges
+    # touching the same customers serialise instead of deadlocking or double-merging.
+    survivor = None
+    loser = None
+    for ref in sorted((survivor_ref, loser_ref)):
+        customer = db.scalar(select(Customer).where(Customer.public_ref == ref).with_for_update())
+        if ref == survivor_ref:
+            survivor = customer
+        else:
+            loser = customer
     if survivor is None or loser is None:
         raise AppError(
             "CUSTOMER_NOT_FOUND", "One of the customers could not be found.", status_code=404
         )
+    if survivor.merged_into_customer_id is not None or loser.merged_into_customer_id is not None:
+        raise AppError(
+            "CUSTOMER_ALREADY_MERGED",
+            "One of the customers has already been merged into another record.",
+            status_code=409,
+        )
 
     before = {
         "survivor": {f: getattr(survivor, f) for f in MERGEABLE_FIELDS},
@@ -856,7 +893,15 @@ def merge_customers(
             raise AppError(
                 "INVALID_FIELD_OVERRIDE", f"Field '{field_name}' cannot be merged.", status_code=422
             )
-        setattr(survivor, field_name, value)
+        cleaned = value.strip() if isinstance(value, str) else value
+        max_length = _MERGEABLE_FIELD_MAX_LENGTH[field_name]
+        if not cleaned or len(cleaned) > max_length:
+            raise AppError(
+                "INVALID_FIELD_OVERRIDE",
+                f"Field '{field_name}' must be 1 to {max_length} characters.",
+                status_code=422,
+            )
+        setattr(survivor, field_name, cleaned)
 
     rewired = db.execute(
         update(Booking).where(Booking.customer_id == loser.id).values(customer_id=survivor.id)

backend/app/services/integration_status.py

@@ -2,11 +2,12 @@ from __future__ import annotations
 
 import threading
 import time
+from collections.abc import Sequence
 from datetime import UTC, datetime, timedelta
-from typing import Literal
+from typing import Any, Literal
 
 import httpx
-from sqlalchemy import func, select
+from sqlalchemy import Row, func, select
 from sqlalchemy.orm import Session
 
 from app.core.config import get_settings
@@ -38,6 +39,22 @@ _STALE_AFTER = {
 }
 
 
+def _latest_rows_per_workflow(db: Session, action: str) -> Sequence[Row[Any]]:
+    """Return the most recent audit row per workflow name for one action.
+
+    Heartbeats arrive on every scheduled run, so loading *all* rows and picking the
+    latest in Python would grow linearly with deployment age. ``DISTINCT ON`` lets
+    PostgreSQL return exactly one (latest) row per workflow instead.
+    """
+    workflow_name = AuditEvent.after_json["workflow_name"].astext
+    return db.execute(
+        select(AuditEvent.occurred_at, AuditEvent.after_json, AuditEvent.metadata_json)
+        .where(AuditEvent.action == action)
+        .distinct(workflow_name)
+        .order_by(workflow_name, AuditEvent.occurred_at.desc())
+    ).all()
+
+
 def derive_n8n_status(db: Session) -> N8nIntegrationStatus:
     counts: dict[str, int] = dict(
         db.execute(
@@ -144,11 +161,7 @@ def derive_n8n_status(db: Session) -> N8nIntegrationStatus:
         "Fleet Ops — Workflow Error Handler": latest_handler_failure_at,
     }
     heartbeat_by_workflow: dict[str, tuple[datetime, str, str | None]] = {}
-    heartbeat_rows = db.execute(
-        select(AuditEvent.occurred_at, AuditEvent.after_json, AuditEvent.metadata_json)
-        .where(AuditEvent.action == "n8n_workflow_heartbeat")
-        .order_by(AuditEvent.occurred_at.desc())
-    ).all()
+    heartbeat_rows = _latest_rows_per_workflow(db, "n8n_workflow_heartbeat")
     for occurred_at, after, metadata in heartbeat_rows:
         workflow_name = (after or {}).get("workflow_name")
         if workflow_name in _CANONICAL_WORKFLOWS and workflow_name not in heartbeat_by_workflow:
@@ -159,11 +172,7 @@ def derive_n8n_status(db: Session) -> N8nIntegrationStatus:
             )
 
     failure_by_workflow: dict[str, tuple[datetime, str | None]] = {}
-    failure_rows = db.execute(
-        select(AuditEvent.occurred_at, AuditEvent.after_json, AuditEvent.metadata_json)
-        .where(AuditEvent.action == "n8n_workflow_failure_registered")
-        .order_by(AuditEvent.occurred_at.desc())
-    ).all()
+    failure_rows = _latest_rows_per_workflow(db, "n8n_workflow_failure_registered")
     for occurred_at, after, metadata in failure_rows:
         workflow_name = (after or {}).get("workflow_name")
         if workflow_name in _CANONICAL_WORKFLOWS and workflow_name not in failure_by_workflow:

backend/app/services/knowledge/ragcore.py

@@ -148,6 +148,8 @@ class RAGcoreKnowledgeProvider:
             with self._client() as client:
                 response = client.get("/health/ready")
                 body = response.json()
+                if not isinstance(body, dict):
+                    raise ValueError("health response is not a JSON object")
                 available = response.status_code == 200 and body.get("status") == "ok"
                 detail = (
                     "RAGcore reachable and ready."

backend/app/services/returns.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+import hashlib
+import json
 import uuid
 from dataclasses import dataclass
 from datetime import UTC, datetime, timedelta
@@ -171,6 +173,33 @@ def preview_vehicle_return(
     return booking, vehicle, evaluation
 
 
+def request_fingerprint(body: RegisterReturnRequest) -> str:
+    canonical = json.dumps(body.model_dump(mode="json"), sort_keys=True, separators=(",", ":"))
+    return hashlib.sha256(canonical.encode("utf-8")).hexdigest()
+
+
+def _replay_or_reject(
+    db: Session,
+    existing: IdempotencyRecord,
+    booking_ref: str,
+    fingerprint: str,
+) -> tuple[int, dict]:
+    booking = db.get(Booking, existing.booking_id)
+    if booking is None or booking.public_ref != booking_ref:
+        raise AppError(
+            "IDEMPOTENCY_KEY_REUSED",
+            "This idempotency key was already used for a different booking.",
+            status_code=409,
+        )
+    if existing.request_fingerprint is not None and existing.request_fingerprint != fingerprint:
+        raise AppError(
+            "IDEMPOTENCY_KEY_REUSED",
+            "This idempotency key was already used with a different request body.",
+            status_code=409,
+        )
+    return existing.response_status, existing.response_body
+
+
 def register_vehicle_return(
     db: Session,
     booking_ref: str,
@@ -178,18 +207,12 @@ def register_vehicle_return(
     idempotency_key: str,
     actor: CurrentUser,
 ) -> tuple[int, dict]:
+    fingerprint = request_fingerprint(body)
     existing = db.scalar(
         select(IdempotencyRecord).where(IdempotencyRecord.idempotency_key == idempotency_key)
     )
     if existing is not None:
-        booking = db.get(Booking, existing.booking_id)
-        if booking is None or booking.public_ref != booking_ref:
-            raise AppError(
-                "IDEMPOTENCY_KEY_REUSED",
-                "This idempotency key was already used for a different booking.",
-                status_code=409,
-            )
-        return existing.response_status, existing.response_body
+        return _replay_or_reject(db, existing, booking_ref, fingerprint)
 
     booking, vehicle = _load_active_booking_and_vehicle(db, booking_ref, lock=True)
 
@@ -199,7 +222,7 @@ def register_vehicle_return(
         select(IdempotencyRecord).where(IdempotencyRecord.idempotency_key == idempotency_key)
     )
     if existing is not None:
-        return existing.response_status, existing.response_body
+        return _replay_or_reject(db, existing, booking_ref, fingerprint)
 
     if booking.status != "active":
         raise AppError(
@@ -336,6 +359,7 @@ def register_vehicle_return(
         IdempotencyRecord(
             idempotency_key=idempotency_key,
             booking_id=booking.id,
+            request_fingerprint=fingerprint,
             response_status=201,
             response_body=response_body,
         )
@@ -349,7 +373,7 @@ def register_vehicle_return(
             select(IdempotencyRecord).where(IdempotencyRecord.idempotency_key == idempotency_key)
         )
         if existing is not None:
-            return existing.response_status, existing.response_body
+            return _replay_or_reject(db, existing, booking_ref, fingerprint)
         raise
 
     return 201, response_body