From 686795a4526d52e656354ab84a1a8785fc631d58 Mon Sep 17 00:00:00 2001 From: NuklearRabbit <145918611+NuklearRabbit@users.noreply.github.com> Date: Mon, 10 Aug 2026 12:08:28 +0200 Subject: [PATCH] M16: isolate acceptance and harden readiness --- .gitea/workflows/ci.yml | 40 ++++++++++++++++++++++++++++ Makefile | 2 +- PROJECT_STATE.md | 19 +++++++++++++ README.md | 24 +++++++---------- backend/app/main.py | 23 ++++++++++++++++ backend/app/services/data_quality.py | 14 +++------- backend/app/services/returns.py | 12 +++++---- backend/tests/test_data_quality.py | 20 +++++++------- backend/tests/test_health.py | 32 ++++++++++++++++++++++ backend/tests/test_return.py | 8 ++++++ compose.test.yaml | 24 +++++++++++++++++ docs/17-runbook.md | 10 +++++-- scripts/run-isolated-tests.sh | 13 +++++++++ 13 files changed, 200 insertions(+), 41 deletions(-) create mode 100644 .gitea/workflows/ci.yml create mode 100644 compose.test.yaml create mode 100644 scripts/run-isolated-tests.sh diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml new file mode 100644 index 0000000..d576093 --- /dev/null +++ b/.gitea/workflows/ci.yml @@ -0,0 +1,40 @@ +name: MobilityOps acceptance + +on: + push: + branches: [master] + pull_request: + +jobs: + backend: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Backend tests in isolated PostgreSQL stack + run: sh scripts/run-isolated-tests.sh + - name: Backend static checks + run: | + docker compose -p mobilityops-ci -f compose.yaml -f compose.test.yaml run --build --rm api ruff check app tests scripts + docker compose -p mobilityops-ci -f compose.yaml -f compose.test.yaml run --rm api mypy app + - name: Remove CI stack + if: always() + run: docker compose -p mobilityops-ci -f compose.yaml -f compose.test.yaml down -v --remove-orphans + + frontend: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: npm + cache-dependency-path: frontend/package-lock.json + - name: Install locked dependencies + working-directory: frontend + run: npm ci --no-audit --no-fund + - name: Typecheck and production build + working-directory: frontend + run: npm run build + - name: Dependency audit + working-directory: frontend + run: npm audit diff --git a/Makefile b/Makefile index 3160a98..e3cf859 100644 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ logs: docker compose logs -f --tail=200 test: - docker compose run --rm api pytest + sh scripts/run-isolated-tests.sh lint: docker compose run --rm api ruff check . diff --git a/PROJECT_STATE.md b/PROJECT_STATE.md index 79813aa..edb30c9 100644 --- a/PROJECT_STATE.md +++ b/PROJECT_STATE.md @@ -2398,3 +2398,22 @@ evidence yet." - **Exact next action**: none for the locked PoC. All acceptance criteria are satisfied; subsequent work is routine production operation, monitoring and explicitly approved scope beyond this build. + +## M16 — reliability boundary and truthful delivery foundation (2026-08-10) + +- Added `compose.test.yaml` and `scripts/run-isolated-tests.sh`: backend acceptance now + runs in a fixed, disposable `mobilityops-test` Compose project with its own PostgreSQL + database/volume. The script cleans up on success, failure or interruption. The live + deployment database is no longer an acceptable test target. +- Added a Gitea Actions workflow for isolated backend tests, ruff/mypy, the locked + frontend build and npm audit. `make test` now routes through the isolated test runner. +- Split process liveness from database-backed readiness (`/health/live` and + `/health/ready`) while retaining `/health` compatibility. +- Replaced count-derived return-inspection and scan-issue references with prefixed UUID + references, eliminating collisions between independent concurrent transactions. +- Updated the README/runbook where they still claimed demo RAGcore, two n8n workflows or + unsafe in-place pytest execution. +- Evidence: Compose merge validated; ruff and mypy clean; full isolated PostgreSQL suite + **218 passed** and the disposable database/network/volume were removed automatically. +- Exact next action: implement honest loading states and RAG/source deduplication, then + revalidate live Knowledge and Integration flows. diff --git a/README.md b/README.md index 5164a31..37149a5 100644 --- a/README.md +++ b/README.md @@ -50,8 +50,9 @@ The PoC implements: - role-aware global search across vehicles, bookings and (Operations Manager) issues; - safe, confirmed demo reset; - RAGcore-backed knowledge assistant with citations; -- two n8n workflows: return processing, and a scheduled data-quality scan with - crash-recoverable outbox delivery leases; +- four canonical n8n workflows: return processing, scheduled data-quality scanning, + RAGcore procedure sync and centralized workflow-error handling, with explicit + heartbeat evidence and crash-recoverable outbox delivery leases; - four read-only MCP tools through ITWorx MCP Hub; - deterministic demo reset and five-minute showcase. @@ -66,16 +67,11 @@ It is not an ERP, CRM, accounting package, public booking site, payment system o quality-scan) workflow live-verified end to end against a real n8n instance. `GET /api/v1/integrations/status` reports a truthful aggregate state from outbox delivery counts, not just the most recent event. -- **RAGcore**: the demo `KnowledgeProvider` (deterministic TF-IDF extractive retrieval - over the local procedure documents) is what satisfies the knowledge-assistant - acceptance criteria and is what's active in production (`KNOWLEDGE_PROVIDER=demo`). A - `RAGcoreKnowledgeProvider` HTTP adapter is implemented, unit-tested, and has been - exercised live against the deployed RAGcore instance: a real filesystem-permission bug - that caused every live retrieval to return zero candidates was found and fixed - (`docs/final-integrations/current-state-audit.md`), but a second, deeper gap — RAGcore's - reranker adapter calls an Ollama HTTP route (`/api/rerank`) that does not exist on the - deployed Ollama version — still blocks real grounded answers. `KNOWLEDGE_PROVIDER` stays - `demo` until that is resolved on the RAGcore side. +- **RAGcore**: the live deployment uses `KNOWLEDGE_PROVIDER=ragcore`. Readiness and real + grounded retrieval are verified end to end; when RAGcore's generated-answer endpoint + is unavailable, Fleet Ops falls back only to cited extractive search results and never + invents an answer. The deterministic demo provider remains available for clean-checkout + acceptance and local development. - **ITWorx MCP Hub**: the four read-only provider endpoints are implemented, tested, and directly `curl`-verified with correct auth enforcement and audit logging. `MCP_HUB_REGISTRATION_ENABLED` is actually wired into `Settings` and reported honestly @@ -130,9 +126,9 @@ All defaults are configurable via `.env` (see `.env.example`). ## Quality gates ```bash -make test # backend: pytest (151 tests) +make test # backend: isolated PostgreSQL Compose project; never the live database make lint # backend: ruff + mypy (strict, zero errors) -make e2e # frontend: Playwright end-to-end (138 tests, live stack required) +make e2e # frontend: complete Playwright acceptance (live stack required) ``` Frontend build/typecheck: `cd frontend && npm run build` (`tsc -b && vite build`). diff --git a/backend/app/main.py b/backend/app/main.py index e019834..36537b7 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -4,6 +4,7 @@ from contextlib import asynccontextmanager from fastapi import FastAPI, HTTPException, Request from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import JSONResponse +from sqlalchemy import text from app.api.routers import ( audit, @@ -77,6 +78,28 @@ def health() -> dict[str, str]: return {"status": "ok", "service": "mobilityops-api"} +@app.get("/health/live") +def liveness() -> dict[str, str]: + """Process liveness only; external dependencies deliberately do not affect it.""" + return {"status": "ok", "service": "mobilityops-api"} + + +@app.get("/health/ready") +def readiness() -> JSONResponse: + """Traffic readiness: the API is useful only while its canonical database responds.""" + try: + with SessionLocal() as db: + db.execute(text("SELECT 1")) + except Exception: # noqa: BLE001 -- readiness must convert infrastructure errors to 503 + return JSONResponse( + status_code=503, + content={"status": "not_ready", "service": "mobilityops-api", "database": "down"}, + ) + return JSONResponse( + content={"status": "ready", "service": "mobilityops-api", "database": "up"} + ) + + @app.get("/api/v1/system/status") def system_status() -> dict[str, object]: return { diff --git a/backend/app/services/data_quality.py b/backend/app/services/data_quality.py index c2077ff..2599dc1 100644 --- a/backend/app/services/data_quality.py +++ b/backend/app/services/data_quality.py @@ -54,15 +54,9 @@ def _has_open_issue(db: Session, rule_type: str, entity_type: str, entity_id: uu ) -def _next_public_ref(db: Session, prefix: str) -> str: - existing = db.execute(select(DataQualityIssue.public_ref)).scalars().all() - numbers = [ - int(ref.rsplit("-", 1)[-1]) - for ref in existing - if ref.startswith(f"{prefix}-") and ref.rsplit("-", 1)[-1].isdigit() - ] - next_number = (max(numbers) + 1) if numbers else 1 - return f"{prefix}-{next_number:04d}" +def _new_scan_ref(prefix: str) -> str: + """Generate a stable human-readable prefix with a concurrent-safe suffix.""" + return f"{prefix}-{uuid.uuid4().hex[:10].upper()}" def _open_issue( @@ -109,7 +103,7 @@ def _open_issue( evidence["previous_decision"] = previous.status issue = DataQualityIssue( - public_ref=_next_public_ref(db, "DQ-SCAN"), + public_ref=_new_scan_ref("DQ-SCAN"), rule_type=rule_type, entity_type=entity_type, entity_id=entity_id, diff --git a/backend/app/services/returns.py b/backend/app/services/returns.py index b5e578b..4d9f943 100644 --- a/backend/app/services/returns.py +++ b/backend/app/services/returns.py @@ -18,12 +18,14 @@ from app.models.vehicle import Vehicle from app.schemas import CurrentUser, RegisterReturnRequest from app.services.audit import record_audit_event -REF_PREFIX = "INSP" +def _new_inspection_ref() -> str: + """Generate a collision-resistant public reference without reading mutable counts. -def _next_public_ref(db: Session) -> str: - existing = db.execute(select(Inspection.public_ref)).scalars().all() - return f"{REF_PREFIX}-{len(existing) + 1:04d}" + Return commands for different bookings can commit concurrently. A count-based + reference made those independent transactions race for the same unique value. + """ + return f"INSP-{uuid.uuid4().hex[:10].upper()}" def _derive_vehicle_status_with_reason( @@ -211,7 +213,7 @@ def register_vehicle_return( evaluation = evaluate_return(db, booking, vehicle, body, now=now) inspection = Inspection( - public_ref=_next_public_ref(db), + public_ref=_new_inspection_ref(), booking_id=booking.id, vehicle_id=vehicle.id, type="return", diff --git a/backend/tests/test_data_quality.py b/backend/tests/test_data_quality.py index 4468f53..6e51059 100644 --- a/backend/tests/test_data_quality.py +++ b/backend/tests/test_data_quality.py @@ -148,9 +148,7 @@ def test_merge_customers_s2_scenario_rewires_and_audits(ops_client): issue = ops_client.get("/api/v1/data-quality/issues/DQ-DEMO-DUPLICATE").json() assert issue["status"] == "resolved" - audit_events = ops_client.get( - "/api/v1/audit", params={"action": "customer_merged"} - ).json() + audit_events = ops_client.get("/api/v1/audit", params={"action": "customer_merged"}).json() assert len(audit_events) >= 1 # Already-resolved issue cannot be merged again. @@ -431,9 +429,7 @@ def test_manual_scan_records_audit_event(ops_client): scan = ops_client.post("/api/v1/data-quality/scan") assert scan.status_code == 200 - events = ops_client.get( - "/api/v1/audit", params={"action": "data_quality_scan_run"} - ).json() + events = ops_client.get("/api/v1/audit", params={"action": "data_quality_scan_run"}).json() assert len(events) >= 1 assert "created" in events[0]["metadata"] @@ -444,9 +440,7 @@ def _reset_demo(ops_client) -> None: # in before making any further authenticated call with the same client. response = ops_client.post("/api/v1/demo/reset") assert response.status_code == 200, response.text - login_response = ops_client.post( - "/api/v1/demo/login", json={"role": "operations_manager"} - ) + login_response = ops_client.post("/api/v1/demo/login", json={"role": "operations_manager"}) assert login_response.status_code == 200, login_response.text @@ -548,3 +542,11 @@ def test_rejected_issue_recurrence_links_to_prior_decision(ops_client): ) assert match is not None, "expected a new issue linked back to the rejected one" assert match["evidence"]["previous_decision"] == "rejected" + + +def test_scan_public_refs_are_collision_resistant() -> None: + from app.services.data_quality import _new_scan_ref + + refs = {_new_scan_ref("DQ-SCAN") for _ in range(1000)} + assert len(refs) == 1000 + assert all(ref.startswith("DQ-SCAN-") and len(ref) == 18 for ref in refs) diff --git a/backend/tests/test_health.py b/backend/tests/test_health.py index 6f6f646..0658644 100644 --- a/backend/tests/test_health.py +++ b/backend/tests/test_health.py @@ -7,3 +7,35 @@ def test_health() -> None: response = TestClient(app).get("/health") assert response.status_code == 200 assert response.json() == {"status": "ok", "service": "mobilityops-api"} + + +def test_liveness_is_process_only() -> None: + response = TestClient(app).get("/health/live") + assert response.status_code == 200 + assert response.json()["status"] == "ok" + + +def test_readiness_checks_the_canonical_database() -> None: + response = TestClient(app).get("/health/ready") + assert response.status_code == 200 + assert response.json() == { + "status": "ready", + "service": "mobilityops-api", + "database": "up", + } + + +def test_readiness_degrades_when_database_is_unavailable(monkeypatch) -> None: + import app.main as main_module + + class BrokenSession: + def __enter__(self): + raise ConnectionError("database unavailable") + + def __exit__(self, *_args): + return False + + monkeypatch.setattr(main_module, "SessionLocal", BrokenSession) + response = TestClient(app).get("/health/ready") + assert response.status_code == 503 + assert response.json()["database"] == "down" diff --git a/backend/tests/test_return.py b/backend/tests/test_return.py index cd700a8..53b103d 100644 --- a/backend/tests/test_return.py +++ b/backend/tests/test_return.py @@ -305,3 +305,11 @@ def test_concurrent_returns_only_one_succeeds(): assert results.count(201) == 1 assert results.count(409) == 2 + + +def test_return_public_refs_are_collision_resistant() -> None: + from app.services.returns import _new_inspection_ref + + refs = {_new_inspection_ref() for _ in range(1000)} + assert len(refs) == 1000 + assert all(ref.startswith("INSP-") and len(ref) == 15 for ref in refs) diff --git a/compose.test.yaml b/compose.test.yaml new file mode 100644 index 0000000..bbcb157 --- /dev/null +++ b/compose.test.yaml @@ -0,0 +1,24 @@ +services: + db: + environment: + POSTGRES_DB: mobilityops_test + POSTGRES_USER: mobilityops_test + POSTGRES_PASSWORD: mobilityops_test + healthcheck: + test: ["CMD-SHELL", "pg_isready -U mobilityops_test -d mobilityops_test"] + + api: + environment: + MOBILITYOPS_ENV: test + MOBILITYOPS_DEMO_MODE: "true" + DATABASE_URL: postgresql+psycopg://mobilityops_test:mobilityops_test@db:5432/mobilityops_test + KNOWLEDGE_PROVIDER: demo + MCP_HUB_REGISTRATION_ENABLED: "false" + DEMO_ALLOW_RESET: "true" + ports: !reset [] + + web: + ports: !reset [] + + n8n: + profiles: ["never-in-tests"] diff --git a/docs/17-runbook.md b/docs/17-runbook.md index 7a3fb16..f3de161 100644 --- a/docs/17-runbook.md +++ b/docs/17-runbook.md @@ -26,10 +26,15 @@ Verify: ```bash curl http://localhost:8128/health # {"status":"ok",...} curl -o /dev/null -w "%{http_code}\n" http://localhost:1228/ # 200 -docker compose run --rm api pytest -q # all tests pass +make test # isolated test project/database, all tests pass docker compose run --rm api ruff check . # clean ``` +Never execute `pytest` inside the deployed API container: the acceptance fixtures reset +their database deliberately. `make test` uses `compose.test.yaml`, a fixed +`mobilityops-test` Compose project and its own disposable PostgreSQL volume, and removes +that project on success or failure. The Gitea workflow uses the same isolation boundary. + ## Operational mode (non-demo login) Keep the current demonstration environment on `MOBILITYOPS_DEMO_MODE=true`. For an @@ -173,7 +178,8 @@ Publish the scheduled quality-scan workflow the same way: ## Required operational checks -- API and web health (`GET /health`, web root `200`); +- API liveness (`GET /health/live`), database-backed readiness (`GET /health/ready`) and + web health (web root `200`); - database migration level (`docker compose exec api alembic current`); - pending/failed outbox count (Automation page, or `GET /api/v1/workflows?status=failed`); - RAGcore provider state (`GET /api/v1/knowledge/status`; demo provider is always diff --git a/scripts/run-isolated-tests.sh b/scripts/run-isolated-tests.sh new file mode 100644 index 0000000..43dd399 --- /dev/null +++ b/scripts/run-isolated-tests.sh @@ -0,0 +1,13 @@ +#!/bin/sh +set -eu + +project="mobilityops-test" +compose_files="-f compose.yaml -f compose.test.yaml" + +cleanup() { + docker compose -p "$project" $compose_files down -v --remove-orphans +} + +trap cleanup EXIT INT TERM +cleanup +docker compose -p "$project" $compose_files run --build --rm api pytest "$@"