Jens/MobilityOps
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

M4: implement n8n automation

Browse Source 
Outbox dispatcher (background thread, FOR UPDATE SKIP LOCKED claim, exponential backoff, no transaction held during HTTP I/O). n8n callback endpoint with shared-secret auth and idempotency by event ID. Automation nav + UI with manual retry. 49 backend tests passing, ruff clean. Fixed a crash-on-redelivery bug in seeded outbox payloads and made the dispatcher defensive against malformed payloads. Verified the full live round trip against a real n8n instance: return -> outbox -> dispatcher -> n8n workflow -> callback -> succeeded, including the S5 failed-retry demo scenario.
This commit is contained in:
NuklearRabbit
2026-08-01 22:39:25 +02:00
parent a7cbeaae3b
commit 59d663a43a
17 changed files with 814 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
PROJECT_STATE.md
+19 -3
View File
@@ -2,7 +2,7 @@
## Current milestone
M3 — complete. Starting M4 next.
M4 — complete. Starting M5 next.
## Locked decisions
@@ -78,10 +78,26 @@ M3 — complete. Starting M4 next.
- `npm run build` — clean (had to fix two `possibly 'null'` TS errors from a closure-narrowing limitation — TS doesn't narrow `const` captured-by-closure across nested function boundaries when the value comes from an index/property expression; fixed by re-binding to explicitly-typed local consts right after the guard).
- Full browser run: Data Quality list (26 open issues, filterable) → `DQ-DEMO-DUPLICATE` two-column compare (CUS-0012 vs CUS-0178, per-field diff highlighting only where they differ) → merge with inline confirm → issue flips to `resolved` → confirmed `customer_merged` audit event with correct actor/entity/correlation → `DQ-DEMO-OVERLAP` (non-duplicate type) renders evidence JSON + defer/reject, no dead compare UI shown for a rule type it doesn't apply to.
### M4 — n8n automation
- `app/services/dispatcher.py`: background daemon thread (started/stopped via FastAPI `lifespan`, not an `on_event` hook) polling every `N8N_DISPATCH_INTERVAL_SECONDS` (default 3s). Claim step (`_claim_due_events`) is a short transaction using `SELECT ... FOR UPDATE SKIP LOCKED` that only flips `pending`→`delivering` and commits immediately; the HTTP call to n8n happens with **no open transaction**; the outcome is recorded in a separate short transaction. Exponential backoff `min(2**attempts, 60)` seconds, `N8N_MAX_ATTEMPTS=5` before a permanent `failed`.
- Dispatcher reconstructs the wire event from `contracts/events.schema.json`'s exact fields (`event_id`, `event_type`, `occurred_at`, `correlation_id`, `aggregate`, `data`) rather than forwarding `OutboxEvent.payload_json` wholesale — that column also carries an internal `aggregate_ref` convenience key (used by dashboard/workflows list rendering) that the schema's `additionalProperties: false` would reject.
- `POST /api/v1/integrations/n8n/return-callback` (`app/api/routers/integrations.py`): shared-secret auth via `X-Service-Token` header (`N8N_CALLBACK_TOKEN`, propagated to both `api` and `n8n` containers as `MOBILITYOPS_CALLBACK_TOKEN`); idempotent by `Idempotency-Key` (the event UUID) — checked by querying for an existing `AuditEvent` with that event ID in its metadata, **not** by `OutboxEvent.external_run_id`, because the dispatcher only sets that field *after* it gets n8n's final response, which happens *after* n8n has already called this callback mid-workflow — using `external_run_id` as the idempotency guard would have missed the exact redelivery case it's meant to catch.
- `GET /api/v1/workflows` + `POST /api/v1/workflows/{event_id}/retry` (`app/api/routers/workflows.py`), both Operations Manager only. Retry only allowed from `failed`; sets `pending` + clears `next_attempt_at` so the live dispatcher picks it up on its next cycle (does not reset `attempts`, so the counter reflects true delivery history).
- Automation nav + page (`pages/Automation.tsx`): table of all runs with status/attempts/last error, Retry button for `failed` rows, visible only to Operations Manager (matches backend authorization rather than just hiding a link).
- **Two real bugs found and fixed, the second only by testing the actual live n8n round-trip, not by pytest**:
1. Seed-loaded `workflow_runs.csv` rows only ever got `payload_json = {"aggregate_ref": ...}` (no `correlation_id`/`aggregate`/`data`) — fine for M1–M3 since nothing read those keys yet, but once the dispatcher tried to *redeliver* a seeded row (i.e. the S5 manual-retry demo scenario) it crashed with `KeyError: 'correlation_id'`, leaving that event stuck in `delivering` forever (the crash happened before the outcome-recording transaction). Fixed in two places: `seed_loader.py` now builds the full schema-compliant envelope for every `workflow_runs.csv` row (matching what the live M2 return flow produces), and `dispatcher._deliver_one` now catches malformed-payload `KeyError`s defensively and resolves the row to `pending`/`failed` instead of leaving it orphaned — added `test_deliver_one_handles_malformed_payload_without_getting_stuck` as a regression test for the latter.
2. This n8n image (2.32.7) has dropped `N8N_BASIC_AUTH_ACTIVE` as a UI/API gate — it requires an actual owner account via the `/setup` flow before anything (including webhook registration reliability) works correctly. Also: `n8n import:workflow` requires the workflow JSON to have a top-level `"id"` field (added `"id": "mobilityops-return-processing"`) and **always deactivates** the imported workflow regardless of its `"active"` field — activation requires `n8n publish:workflow --id=<id>` followed by a full n8n restart (documented in n8n 2.x CLI, not obvious from the docs pack). Did this manually this session via the CLI + browser setup wizard; **this is a one-time operational step that is not automated** — a truly clean checkout still needs someone to run `docker compose exec n8n n8n import:workflow --input=//imports/mobilityops-return-processing.json`, `docker compose exec n8n n8n publish:workflow --id=mobilityops-return-processing`, `docker compose restart n8n`, and complete the one-time owner setup at `http://localhost:5678/setup` (any email/password, no verification required) before the automation demo will work. `docs/17-runbook.md` should get this exact sequence in M7.
- Commands run and verified from this checkout:
- `docker compose run --rm api pytest -q` — **49 passed** (new `tests/test_dispatcher.py` — claim/deliver success/failure/backoff/exhaustion-to-failed/malformed-payload, all via `monkeypatch.setattr(dispatcher.httpx, "post", ...)`, no real network calls in tests; `tests/test_integrations.py` — callback auth, unknown-event 404, idempotent-by-event-ID with a real duplicate-call assertion; `tests/test_workflows.py` — role gating, retry-only-from-failed, S5 retry-and-audit).
- `docker compose run --rm api ruff check .` — All checks passed.
- `npm run build` — clean.
- Full live round trip (not mocked): registered a real return on `BK-DEMO-RETURN` → outbox event queued → background dispatcher delivered it to the now-activated n8n workflow within its 3s poll interval → n8n called back into `/api/v1/integrations/n8n/return-callback` (200 OK, confirmed in `docker compose logs api`) → dispatcher's original POST received n8n's success response → event flipped to `succeeded` on attempt 1, visible on `/automation`.
- S5 scenario end-to-end in the browser: seeded `BK-H-0020` (`failed`, 3 attempts, "Synthetic connection timeout to n8n") → clicked Retry → `pending` → within ~3s, live dispatcher delivered it through the real n8n instance → `succeeded`, 4 attempts. This is the full documented S5 scenario working for real, not simulated.
## Known blockers
None. External service credentials may be absent; use the documented demo/degraded providers.
None. External service credentials may be absent; use the documented demo/degraded providers. The n8n workflow-activation steps above are a one-time manual setup requirement in this environment, not a blocker — but not yet scripted; M7 should either automate it (e.g. a bootstrap script CI/compose can run) or document it clearly enough for `docs/14-testing-and-acceptance.md`'s clean-checkout criteria.
## Exact next action
Start M4 (n8n automation): read `docs/11-n8n-integration.md`, `n8n/README.md`. Implement the outbox dispatcher (poll pending `outbox_events`, claim with `FOR UPDATE SKIP LOCKED`, POST to n8n's webhook with timeout + exponential backoff + small max-attempt cap, never hold a DB transaction open during the HTTP call), correct/import `n8n/mobilityops-return-processing.json` (credential + the callback route — M2's `vehicle.returned.v1` outbox payload shape is already `contracts/events.schema.json`-compliant, so the workflow should be able to consume it as-is), a narrow MobilityOps callback endpoint the workflow calls back into, an Automation nav item + UI (pending/succeeded/failed list, manual retry button, matches `docs/06-ui-ux.md`'s "Recent automation" concept already on the dashboard), and manual-retry wiring for `POST /api/v1/workflows/{event_id}/retry` per `docs/05-api-contract.md`. The seeded `workflow_runs.csv` already includes one deterministic `failed` row (`event_id 00000000-...-0020`, "Synthetic connection timeout to n8n") for the S5 demo scenario — dashboard/Automation UI must surface it and allow retry. Remember the compose `n8n` service is already up (`http://localhost:5678`, basic auth `admin`/`change-me` from `.env.example`) but the workflow itself hasn't been imported/activated yet this session.
Start M5 (RAGcore knowledge integration): read `docs/09-ragcore-integration.md`, `knowledge/manifest.json`. Implement the `KnowledgeProvider` protocol (`health`, `ask`, `sync_manifest`), a `DemoKnowledgeProvider` doing deterministic keyword/BM25-style retrieval directly over `knowledge/procedures/*.md` (extractive, not generative — must return real excerpts + `insufficient`/`unavailable` states honestly, never fabricate), and a `RAGcoreKnowledgeProvider` adapter stub for the real service (`RAGCORE_BASE_URL` etc. are already in config/compose from M0, but no actual RAGcore instance is confirmed reachable this session — build the demo provider as the one that actually has to work for the acceptance criteria, and make the RAGcore adapter degrade to `unavailable` cleanly if unreachable, per `docs/03-architecture.md`'s reliability boundary "RAGcore failure disables knowledge answers only"). Add `POST /api/v1/knowledge/questions` and `GET /api/v1/knowledge/status`, a Knowledge nav item + page (chat-style question box, source cards with title/version/section/excerpt, explicit unavailable/insufficient states — no fabricated answers ever). S6 demo scenario: "What must I do when a vehicle returns with damage?" must cite `knowledge/procedures/03-damage-handling.md` and `02-vehicle-return.md`.