Files
MobilityOps/docs/functional-completion/current-functional-audit.md
T
NuklearRabbit 063a8f9a2d docs(audit): record functional completion findings
Independent audit of the design/mobilityops-premium-ui source and the live
Unraid deployment: confirms the two named list-rendering defects plus
sessionStorage-authoritative auth, a missing role gate on the data-quality
workbench and audit trail, a non-authoritative return preview, raw-JSON
issue evidence, a blind client-side search, single-event integration status,
and an unbounded delivering-lease window in the outbox dispatcher.
2026-08-02 04:51:43 +02:00

11 KiB
Raw Permalink Blame History

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.
  • 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 F1F4 now (Batch 1, in progress), continue through F5F14 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.