feat(f5): dashboard backend — read-core, LISTEN/NOTIFY push, WS + scope authz - #18
Conversation
…, WS-Endpoint + Scope-Autorisierung
- reads/: transport-neutraler Read-Core (queries/status/overview/trend); MCP umgestellt, mcp/reads.py entfernt (keine Duplikation)
- realtime/: NOTIFY-Producer (ein NOTIFY/Commit, transaktional), Hub + LISTEN pro Worker (debounce->load, Reconnect-Snapshot), gemultiplexter WS-Endpoint, Abo-Autorisierung (default-deny, Rollenmatrix 3.1 + Per-User-Scope)
- HTTP-Read-Routen /api/v1/overview + /machines/{id}/trend (gleiche Autorisierung wie WS)
- Migration 0008: users.assigned_line_ids/assigned_machine_ids (Scope-Quelle)
- Review-Fixes: WS-Push laedt Nutzer pro Push frisch + re-autorisiert (entzogener Scope greift sofort, kein PII-Weiterstrom); _push_loop fehler-robust (kein Zombie/Leak)
- GROUND_TRUTH §4/§5/§20 + WALKTHROUGH (F5) nachgezogen
- Vorgaben 1-4 erfuellt; mypy --strict + ruff clean; 439 Tests gruen (F-PRED CI-only)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughPR führt vollständig die F5-Dashboard-/Live-Push-Schicht ein: eine transportneutrale Read-Core-Bibliothek ( ChangesF5 Dashboard & Live-Push Backend
Sequence Diagram(s)sequenceDiagram
participant Ingestion as IngestionService<br/>& Readings
participant notify_changes as notify_changes<br/>(NOTIFY)
participant PostgreSQL
participant Listener as DashboardListener<br/>(asyncpg)
participant Hub as DashboardHub<br/>(Debounce)
participant WebSocket as dashboard_ws<br/>(WS Endpoint)
participant Client
Client->>WebSocket: WS connect<br/>?token=<jwt>
WebSocket->>WebSocket: _authenticate()<br/>→ User laden
WebSocket->>Client: accept()
Client->>WebSocket: {"action":"subscribe",<br/>"topic":"machine:42"}
WebSocket->>WebSocket: can_subscribe<br/>(default-deny)
WebSocket->>Hub: subscribe(sub,"machine:42")
WebSocket->>WebSocket: _load_topic()<br/>→ ReadCore
WebSocket->>Client: {"type":"update",...}
Ingestion->>notify_changes: ChangeSet<br/>(machines={42})
notify_changes->>PostgreSQL: pg_notify<br/>("foreman_dashboard",...)
PostgreSQL-->>Listener: NOTIFY-Event
Listener->>Hub: dispatch(ChangeSet)
Hub->>Hub: debounce timer<br/>→ _flush()
Hub->>Hub: mark_dirty<br/>("machine:42")
Hub-->>WebSocket: queue.get()<br/>→ "machine:42"
WebSocket->>WebSocket: _authorized_payload<br/>(re-check)
WebSocket->>WebSocket: _load_topic()<br/>→ ReadCore
WebSocket->>Client: {"type":"update",...}
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/foreman/realtime/channels.py`:
- Around line 72-81: The decode_change function does not validate the payload
version field `v`, which means incompatible NOTIFY payloads (such as during
mixed deploy versions) are processed without safe error handling. Add version
validation logic at the start of decode_change (after json.loads and before the
broad check) to verify that the payload version matches the expected version. If
the version field is missing or incompatible, return ChangeSet(broad=True) as a
safe fallback instead of proceeding to parse potentially incompatible machine,
data_point, and kinds fields. This ensures backward compatibility and prevents
silent data loss when payload contracts diverge between deployed versions.
In `@src/foreman/realtime/ws.py`:
- Around line 105-109: The code at line 109 where subject is converted to an
integer using int(subject) lacks error handling for cases where subject contains
non-numeric characters, which would raise an unhandled ValueError exception.
Wrap the int(subject) conversion in a try-except block to catch ValueError, and
when caught, close the websocket connection with the _WS_UNAUTHORIZED code (or
another appropriate code) before returning, similar to how the None case is
already handled just above this line.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4fd6b5e5-59ac-4b4e-b735-ed76c53ca6f8
📒 Files selected for processing (37)
GROUND_TRUTH.mddocs/WALKTHROUGH.mdmigrations/versions/0008_user_subscription_scope.pysrc/foreman/api/routers/dashboard.pysrc/foreman/api/routers/readings.pysrc/foreman/db/models.pysrc/foreman/ingestion/service.pysrc/foreman/main.pysrc/foreman/mcp/schemas.pysrc/foreman/mcp/tools.pysrc/foreman/reads/__init__.pysrc/foreman/reads/overview.pysrc/foreman/reads/queries.pysrc/foreman/reads/status.pysrc/foreman/reads/trend.pysrc/foreman/realtime/__init__.pysrc/foreman/realtime/authz.pysrc/foreman/realtime/channels.pysrc/foreman/realtime/hub.pysrc/foreman/realtime/listener.pysrc/foreman/realtime/notify.pysrc/foreman/realtime/topics.pysrc/foreman/realtime/wiring.pysrc/foreman/realtime/ws.pysrc/foreman/schemas/dashboard.pytests/integration/test_dashboard_routes.pytests/integration/test_reads_overview.pytests/integration/test_reads_trend.pytests/integration/test_realtime_authz.pytests/integration/test_realtime_listener.pytests/integration/test_realtime_notify.pytests/integration/test_realtime_wiring.pytests/integration/test_realtime_ws.pytests/unit/test_dashboard_schemas.pytests/unit/test_realtime_channels.pytests/unit/test_realtime_hub.pytests/unit/test_realtime_topics.py
…match; WS-Auth härter gegen nicht-numerisches sub - channels.decode_change: prüft Payload-Version v; unbekannte/inkompatible Version (gemischter Deploy) → fail-safe broad statt still verlorener IDs - ws._authenticate: int(subject) in try/except → 4401-Close bei nicht-numerischem sub (manipuliertes Token) statt unbehandelter ValueError - 2 Regressions-Tests (TDD) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
CodeRabbit-Findings abgearbeitet (Commit
Beide TDD; lokal mypy --strict + ruff clean, betroffene Tests grün. |
Backend-Fundament des Dashboards (F5) — Live-Push + Erstbild, Frontend folgt separat. Designgrundlage:
docs/research/FOREMAN_Designstudie_Frontend.md§5.1.Was
src/foreman/reads/):queries/status/overview/trend.mcp/reads.py+_compose_statusdorthin verschoben (Rename), MCP ruft den Core auf — keine Duplikation, F7 unverändert.src/foreman/realtime/): NOTIFY-Producer (einpg_notifypro Commit/Batch, transaktional, dünner ID-Payload), LISTEN-Consumer + In-Process-Hub pro Worker (debounce→load, Reconnect→Snapshot-Reload), ein gemultiplexter WS-Endpoint/api/v1/ws.realtime/authz.py): default-deny, Rollenmatrix 3.1 (worker→seine Maschinen, shift_lead→seine Linien, manager/technician unrestricted, overview nur manager/shift_lead). Gleiche Prüfung auf WS UND HTTP — der PII-Strich hält auf beiden Transporten.GET /api/v1/overview+GET /api/v1/machines/{id}/trend.users.assigned_line_ids/assigned_machine_ids(bigint[]) als Scope-Quelle.Die vier Bau-Vorgaben (erfüllt + getestet)
readings_1m(real-time aggregation); Test beweist, der frische Bucket erscheint ohne Refresh.Adversariales Review (vor diesem PR) — 2 Defekte gefunden + gefixt
user_idführen, pro Push frisch laden + re-autorisieren (Regressions-Test)._push_loopohne Fehlerabfang: transiente Exception → stiller Zombie + Subscription-Leak. Fix: fehler-robuster Loop.Verifikation
mypy --strictclean (121 Dateien) ·ruff+ruff formatclean · 439 Tests grün (F-PRED-shap/lightgbm lokal ausgelassen → Py-3.13-Segfault, CI-Linux verifiziert).Bewusst offen
Das dynamische F4-Eigenprofil-Overlay im Trend (Feld
profile_bandreserviert/null; statisches Normalband steht) — braucht den gegateten F4-Replay, daher eigener Schritt.GROUND_TRUTH §4/§5/§20 + WALKTHROUGH (F5) sind nachgezogen.
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes