MobilityOps/docs/functional-completion/current-functional-audit.md

# Current functional audit

Performed 2026-08-02 against source `feat/mobilityops-functional-completion` (branched from
`design/mobilityops-premium-ui` @ `54dc952`) and the live Unraid deployment at
`http://192.168.10.150:1236` (same revision — see `server-baseline.md`). Findings below are
either reproduced directly (curl against the live server, or reading the exact source lines)
or are structural gaps confirmed against the task's own explicit requirements. Items already
on record as accepted PoC tradeoffs (`docs/deferred.md`, `docs/01-scope-and-non-goals.md`,
`PROJECT_STATE.md` known-limitations) are excluded — this file only lists items that are
genuinely open.

## Method

- Full read of `backend/app/api/routers/*.py`, `backend/app/api/deps.py`,
  `backend/app/services/returns.py`, `backend/app/models/*.py`,
  `frontend/src/pages/*.tsx`, `frontend/src/components/Layout.tsx`,
  `frontend/src/context/AuthContext.tsx`, `frontend/src/App.tsx`.
- Live curl verification against `http://192.168.10.150:1236` for the auth/role findings.
- Cross-checked every finding against `docs/deferred.md`,
  `artifacts/final-acceptance/summary.md`, and the latest `PROJECT_STATE.md` sections to
  avoid re-flagging already-accepted tradeoffs.

## Findings

### F1 — Vehicles page renders the unfiltered array (client-side search is inert)

- Severity: high. Role: both. Route: `/vehicles`. Component:
  `frontend/src/pages/Vehicles.tsx`.
- Repro: type any text into the "Search" box on the Vehicles page. The row count label
  (`{filtered.length} vehicles`) updates and the empty state correctly appears when nothing
  matches, but the `<tbody>` mapped over the raw `vehicles` array, not the computed
  `filtered` array — every original row stayed visible regardless of the search text.
- Expected: only rows matching the search (combined with the status/attention filters)
  render.
- Cause: `vehicles.map(...)` at the table body instead of `filtered.map(...)` (line 78 as
  originally read).
- Reproduces locally: yes (read). Reproduces on Unraid: yes (identical bundled source,
  `source-revision` matches).
- Fix: **applied** — render body now maps `filtered`. Regression test:
  `frontend/e2e/*.spec.ts` search-changes-rows case (added in Batch 1).

### F2 — Bookings page renders the unfiltered, unpaginated array

- Severity: high. Role: both. Route: `/bookings`. Component:
  `frontend/src/pages/Bookings.tsx`.
- Repro: same class of bug — `visible` (filtered + sliced to 25/page) was computed and used
  for the meta line and pagination controls, but `<tbody>` mapped over the raw `bookings`
  array. All bookings rendered on every page regardless of filter or page number.
- Cause: `bookings.map(...)` instead of `visible.map(...)`.
- Fix: **applied** — render body now maps `visible`. Added a page-clamp effect so `page`
  cannot point past the last valid page when the filtered set shrinks (e.g. after a status
  filter reload returns fewer results than the current page implies).
- Reproduces locally/Unraid: yes/yes.

### F3 — Session state is `sessionStorage`-authoritative, not server-verified

- Severity: high. Role: both. Component: `frontend/src/context/AuthContext.tsx`.
- The logged-in `user` object is read from and written to `sessionStorage`
  (`mobilityops.demo-user`) directly; there is no call on app start to verify the HttpOnly
  session cookie is still valid, and `logout()` only clears local state — it never calls the
  server to invalidate the cookie. A stale/edited `sessionStorage` entry (or a cookie that
  expired server-side) will keep protected pages rendering as if authenticated until the
  first API call 401s, and even then nothing centrally redirects to `/login` (`isSessionExpired`
  helper exists in the same file but is never imported/called anywhere).
- Live confirmation: no `GET /api/v1/demo/session` or `POST /api/v1/demo/logout` endpoint
  exists yet (`curl` returns 404 for both against the live server).
- Fix: Batch 1 — add both endpoints server-side, make `AuthProvider` verify against
  `GET /demo/session` on load, call `POST /demo/logout` on sign-out, and centrally react to
  401s from the `api` client.

### F4 — Data-quality workbench has no role gate at all (list, detail, defer, reject)

- Severity: high. Role: Rental Employee. Routes: `/data-quality`, `/data-quality/:ref`.
  Endpoints: `GET /api/v1/data-quality/issues`, `GET /api/v1/data-quality/issues/{ref}`,
  `POST .../defer`, `POST .../reject`.
- Live confirmation: logged in as `rental_employee` on the live server, `GET
  /api/v1/data-quality/issues` and `GET /api/v1/audit` both returned `200` (curl evidence
  above). Only `merge-customers` and `scan` are gated to Operations Manager; `defer`/`reject`
  are not, and the whole workbench is reachable and actionable by Rental Employee both via
  direct API call and via the UI (`DataQuality.tsx` has no role check at all; `Layout.tsx`
  shows the "Data quality" and "Audit trail" nav items unconditionally to both roles).
- The task's role matrix (this brief, section 4) puts data-quality and audit entirely under
  Operations Manager — Rental Employee's list is dashboard/fleet/vehicle
  detail/bookings/booking detail/return/knowledge only. Decision recorded: tighten
  `list_issues`, `get_issue`, `defer`, `reject`, and `GET /api/v1/audit` to
  `require_operations_manager`; hide the nav items and show the same restricted-message
  pattern already used by `Automation.tsx` for direct URL access by Rental Employee.
- Fix: Batch 1.

### F5 — Return preview does not exist; the review step (if any) cannot be authoritative

- Severity: high. Section 5 requirement. No `POST
  /api/v1/bookings/{public_ref}/return-preview` (or equivalent) endpoint exists anywhere in
  `backend/app/api/routers/bookings.py`. The frontend return flow can therefore only ever
  show a client-guessed preview, or skip a real preview step entirely.
- Fix: Batch 2 — one authoritative evaluation function shared by preview (no writes) and
  commit.

### F6 — Audit API never exposes `before_json`/`after_json`

- Severity: medium. `AuditEvent` (`backend/app/models/audit.py`) stores `before_json` and
  `after_json`, populated by `record_audit_event` call sites (e.g. `return_registered`,
  `vehicle_status_changed`), but `AuditEventOut` (`backend/app/schemas.py`) and the router
  (`backend/app/api/routers/audit.py`) only ever return `metadata`, never before/after. The
  UI (`Audit.tsx`) therefore cannot show what changed, only that something happened.
- Fix: Batch 2 — add `before`/`after` to `AuditEventOut`, resolve a safe entity link where
  possible, render human-readable before/after in the UI behind progressive disclosure.

### F7 — Data-quality issue evidence is a raw JSON dump for 4 of 5 rule types

- Severity: medium. `DataQualityIssueDetail.tsx`: for every rule type except
  `possible_duplicate_customer`, the only resolution surface is `<pre>{JSON.stringify(issue.evidence,
  null, 2)}</pre>` plus generic Defer/Reject buttons. `missing_required_field`,
  `odometer_regression`, `booking_overlap`, and `vehicle_status_conflict` have no typed,
  bounded resolution flow at all.
- Fix: Batch 3.

### F8 — Related-snapshot typing is inferred from rule type, not explicit

- Severity: low. `backend/app/api/routers/data_quality.py::get_issue` infers
  `related_entity_type = "customer" if issue.rule_type == "possible_duplicate_customer" else
  "vehicle"` — a `booking_overlap` issue's related entity is actually a booking, not a
  vehicle, so its snapshot lookup silently returns `None` today. Confirmed by reading
  `_snapshot()`, which only knows how to look up `customer` or `vehicle` rows.
- Fix: Batch 3 — typed snapshots for customer/vehicle/booking/inspection.

### F9 — Global search is a blind client-side regex guesser

- Severity: medium. `frontend/src/components/Layout.tsx::handleSearch` pattern-matches
  `MO-…`/`BK-…`/`DQ-…` and navigates without checking the entity exists, or fuzzy-matches a
  hardcoded `SEARCH_DESTINATIONS` term list. No backend search endpoint exists. No results
  panel, no keyboard navigation within results, no role filtering, no debounced live query —
  it is a single-shot form submit.
- Fix: Batch 4 — `GET /api/v1/search`.

### F10 — n8n integration status is derived from the single most recent outbox event

- Severity: medium. `Automation.tsx` line: `<StatusBadge status={runs?.[0]?.status ??
  "no_events"} />` — the "n8n delivery" health card shows whichever status the most recent
  event happens to be in, not an aggregate of pending/delivering/failed/succeeded counts or
  dispatcher-enabled state. A single old failed event sitting behind 40 succeeded ones would
  misreport health; a single lucky success would hide an otherwise-failing dispatcher.
- Fix: Batch 4 — truthful aggregate integration-status endpoint.

### F11 — Stale `delivering` outbox events have no lease/recovery

- Severity: medium. `backend/app/services/dispatcher.py`: `_claim_due_events` flips rows to
  `delivering` and commits before the HTTP call; if the process is killed between that commit
  and the outcome-recording transaction, the row stays `delivering` forever with no timeout
  or reclaim sweep. Not documented anywhere as an accepted limitation.
- Fix: Batch 5 — bounded delivery lease + stale-recovery sweep.

### F12 — No UI trigger for demo reset

- Severity: low. `POST /api/v1/demo/reset` exists and is Operations-Manager-gated
  server-side, but no page exposes a "Run demo reset" action; it can currently only be
  invoked directly against the API.
- Fix: Batch 4.

### F13 — No UI trigger for manual data-quality scan

- Severity: low. `POST /api/v1/data-quality/scan` exists and is OM-gated, but
  `DataQuality.tsx` has no "Run quality scan" button.
- Fix: Batch 3.

### F14 — Second n8n workflow (scheduled quality scan) not present

- Severity: low, explicitly requested in this brief (section 10C). Only
  `n8n/mobilityops-return-processing.json` exists.
- Fix: Batch 5.

## Not re-flagged (already-accepted, on record)

RAGcore/MCP Hub never live-round-tripped (environment limitation, honestly degraded);
`Inspection.public_ref` sequence not gap-safe under concurrency; DQ idempotency key
simplified from the doc's literal fingerprint scheme (reasoned, documented deviation); no
optimistic `version` check on return requests (row-locking already provides the required
concurrency safety — the doc's "optimistic version where relevant" was not adopted, and nothing in
this brief requires adding one on top of working pessimistic locking); demo auth is an
HMAC-cookie PoC mechanism, not a production IdP; n8n owner-account bootstrap remains a
one-time manual step (n8n 2.x product behavior).

## Next step

Fix F1–F4 now (Batch 1, in progress), continue through F5–F14 in the batches recorded in
`PROJECT_STATE.md` / the task list, deploying and re-verifying against Unraid after each
batch per the brief's server-first loop.