feat: party kiosk flows, loco setup, and tablet UI

[keskad]

Summary

• Drive / account / loco wizard flows with function-template attach, F2 ops pulse, and programming-track UX
• Organizer SSO login with layout picker, fullscreen controls, and kiosk chrome (assistant backdrop, i18n pl/en/de)
• QR helpers, device onboarding assets, and listen-on-LAN / public BigFred URL wiring

Test plan

• Log in via SSO with a chosen layout; confirm /auth/me layout matches
• Create account, drive pairing, and configure-loco end-to-end on a tablet
• Attach Basic (or another) template on new loco; leave empty for existing
• After program success, “Przetestuj F2” pulses on powered ops track
• Fullscreen toggle works from AppBar and login screen

Made with Cursor

[keskad]

Review — compliance with microinit/CODING-GUIDELINES.md, performance, reliability

Thanks for this PR — the scope (kiosk, SSO, F2 pulse, i18n) is coherent and well fragmented. Input validation in programming_api.rs (checked address, MAX_FUNCTION, clamp(1, MAX_PULSE_MS)) is done correctly and is the reference point for the rest. Notes below ordered by priority. (Review submitted as COMMENT — GitHub does not allow REQUEST_CHANGES on your own PR.)

🔴 Blockers / High

1. Port mismatch in BUILTIN_REDIRECT_URIS (src/config.rs:13-17) — built-ins use 8081, the wizard listens on 8091, default redirect_uris also use 8091. dev-config.json repeats the mistake. SSO over LAN hostnames (bigfred.local/bigfred-wizard.local/wizard.local) is broken — BigFred redirects to a port where nothing listens. Violates §1.1 (invalid states impossible) and §16 (determinism). A test verifying built-in port consistency with http would catch this.

2. No rollback/cancel in pulse_function (src/dccbus_client.rs:225-253) — if OFF fails, the function stays on on the ops track. tokio::time::sleep ignores cancellation. For F2 (horn/lights) that's annoying; for other functions potentially unsafe. Missing # Errors/# Panics in docs. Recommendation: RAII guard sending OFF in drop/finally + tokio::select! with cancellation + timeout shorter than ACK_TIMEOUT.

🟠 Important / Should

3. expect() on the production path (§4.2) — config.rs:243 (write_example_config, called from main), ensure_client.rs:91 and ×2 in sync_redirect_uris. Guidelines: MUST NOT expect/unwrap for recoverable conditions. Replace with map_err to typed errors (ConfigError::Serialize, EnsureError::Serialize).

4. expect("status mutex") / expect("session") in dccbus_client.rs (§4.2/§10) — poisoned mutex panics the daemon on a request. Return ApiError::internal. Additionally: if status is std::sync::Mutex in async — consider tokio::sync::Mutex (§10.2).

5. ensure on every /oauth/token (oauth_proxy.rs:59) — auth hot path now does fs read/parse/write/chmod/chown/touch per exchange. touch_for_reload bumps mtime even without changes → spurious fsnotify in BigFred. Call ensure only at startup + when load_secret returns NotFound/Parse.

6. Silent hardening errors (ensure_client.rs:176-196) — let _ = set_permissions/chown masks missing root → runtime invalid_client with a "success" log. At minimum tracing::warn! and post-operation metadata verification (§2.4, §1.2).

7. No concurrency bound on function_pulse (programming_api.rs:152) — concurrent pulses to the same locomotive can interleave ON/OFF. Per-(address,function) Semaphore/Mutex with try_lock → 429. Additionally as_login is not validated against roster/layout — authz-relevant (impersonation); at minimum document the trust assumption.

🟡 Minor / performance

8. bigfred_gid() reads/parses /etc/group per call (ensure_client.rs:213-224) — cache in OnceLock/AppState, log parse errors, don't return None on a malformed line.
9. request_as opens a new WS per call (dccbus_client.rs:195-210) — connection churn for repeated drive commands. Consider bounded per-login session cache (§11.4) or document that impersonated calls are one-shot by design.

Frontend (reliability principles from the guidelines)

10. DriveFlowPage.tsx:230-241 — polling .catch(() => undefined) swallows errors (infinite spinner) and doesn't stop after expired. Add failure counter → setError and !expired in the interval condition.
11. DriveFlowPage.tsx:406 — duplicate phase === "user" block; merge.
12. ConfigureLocoPage.tsx:234 — persist→program ordering leaves vehicle "incomplete" if participant interrupts between steps. Consider program-first or an "incomplete" marker.
13. ConfigureLocoPage.tsx:297-307 — 409 swallowing too broad (err.status === 409 masks other conflicts); narrow to layout_vehicle_already_on_roster and log.

Tests (§13)

Good coverage of merge_builtin_redirect_uris, qr_url, write_example_config. Missing tests: OFF failure in pulse_function, function_pulse bounds (function=32, durationMs clamp), and — most importantly — built-in port consistency with http. Recommend release-assertions profile (§5.7/§17.1) in CI.

---

Summary: changes requested mainly for #1 (SSO correctness on tablet) and #2 (unsafe missing pulse rollback). The rest are should-fix items that improve kiosk reliability at events. After #1 and #2 + expect→typed error (#3), safe to merge.

[keskad]

Correctness bug (HIGH): port mismatch.

Built-in URIs use port 8081, but the wizard listens on 8091 (http: "0.0.0.0:8091") and the default redirect_uris also use 8091. BigFred will redirect the tablet browser to http://bigfred.local:8081/auth/callback, where nothing is listening → SSO over LAN hostnames is broken.

Per §1.1 CODING-GUIDELINES (make invalid states impossible) and §16 (determinism): the allowlist must match the actual endpoint. dev-config.json repeats the same mistake (bigfred.local:8081), so neither the default nor the example is self-consistent.

Recommendation: either fix to 8091 (if there is no reverse proxy), or — if 8081 is the intentional public port behind a proxy — document that explicitly and derive the port from a shared constant (e.g. WIZARD_PUBLIC_PORT) used by both BUILTIN_REDIRECT_URIS and http. Add a test that every built-in URI points at the port the wizard actually binds to.

[keskad]

§4.2 / §10 — expect("status mutex") on the request path. Poisoned mutexes panic the daemon on an organizer request. This is a recoverable condition — it should return ApiError::internal("dcc_bus_internal").

Applies to all .lock().expect("status mutex") in connect_with_backoff_as and .as_ref().expect("session") in request. If status is tokio::sync::Mutex, consider lock().await with map_err; if std::sync::Mutex in async — that's a separate issue (§10.2, blocking in async).

[keskad]

To fix.

[keskad]

Reliability / safety: no rollback and no cancellation.

1. If OFF fails (network, command station), the function stays on on the ops track. For F2 (often horn/lights) that's annoying; for other functions it can be unsafe. Missing # Errors / # Panics documentation.
2. tokio::time::sleep ignores cancellation — if the client disconnects mid-pulse, the task keeps running and will still send OFF after the full duration_ms.
3. No timeout on send_and_wait in this path (only the global ACK_TIMEOUT=30s) — with a 5s pulse and a stuck station, the client waits up to 30s.

Recommendation: guard/RAII pattern — send OFF in drop/finally even on error; use tokio::select! with an on_drop cancellation token; document failure semantics. Consider whether the pulse should be idempotent under concurrent calls (see comment on function_pulse).

[keskad]

To fix.

[keskad]

§4.2 — expect() forbidden on the production path. write_example_config is called on every daemon start (main.rs). Config::default() is trivially Serialize, so in practice this won't fail, but the guidelines say MUST NOT use expect/unwrap for recoverable conditions — and here a silent panic at startup is especially bad.

Recommendation: serde_json::to_vec_pretty(&Config::default()).map_err(|source| ConfigError::Write { path: path.clone(), source })? or a dedicated ConfigError::Serialize variant. Same applies to ensure_client.rs (expect("serialize client") ×2 — see comment below).

[keskad]

To fix.

[keskad]

§4.2 — expect("serialize client") in ensure. Same as in config.rs: OAuthClientFile is Serialize, so this isn't truly recoverable in practice, but the guidelines forbid expect in production. Additionally, this function is now called from oauth_proxy::token on every token exchange (see comment on oauth_proxy.rs), so a silent panic would kill auth.

Recommendation: .map_err(|source| EnsureError::Serialize { path: path.to_path_buf(), source })? with a new EnsureError::Serialize variant. Second occurrence in sync_redirect_uris — same fix.

[keskad]

To fix.

[keskad]

§8.5 / §1.3 — no concurrency bound for ops-track pulse. The endpoint allows concurrent pulses to the same locomotive: two parallel requests can interleave ON/OFF (e.g. ON1 → ON2 → OFF1 → OFF2, or ON1 → OFF1 → ON2 with OFF2 never sent if pulse_function fails mid-way). The organizer (token) can also spam requests.

Recommendation: per-address Mutex/Semaphore (or tokio::sync::Mutex with try_lock → 429), or a global Semaphore with a bound of e.g. 1 per (address, function). Additionally: as_login is accepted as any non-empty string — no verification that the participant exists and belongs to the layout. Authz-relevant (impersonation); at minimum document the trust assumption, or validate via BigFred before sending to dcc-bus.

[keskad]

Reliability: polling swallows errors and doesn't stop after expiry.

1. .catch(() => undefined) — if /remotes/status fails (network, 5xx), the user sees an infinite spinner with no message. After N failed attempts, an error should be shown (e.g. t("drive.statusLost")).
2. expired (line 272) is computed, but the interval still runs after expiry — wasted requests. The effect condition should include !expired.

Recommendation: add a failure counter (e.g. 3×) → setError; in the useEffect, add !expired to the interval start condition and clear it when expired becomes true.

[keskad]

Duplicated block. Two separate {phase === "user" && (...)} blocks render at the same time (UserPicker + Cancel). It works, but it's fragile — a refactor can easily add a third condition and end up with double rendering. Remove the duplicate: merge Cancel into the UserPicker block or extract buttons into a separate section driven by phase.

[keskad]

Reliability: persist → program ordering creates inconsistent state. persistAndGoProgram saves the vehicle in BigFred (create/update + attach template) before programming the address on the track (programAddress in the next phase). If the participant leaves after persist but before program, the vehicle exists in the catalog with an address the decoder doesn't have stored. That's data inconsistency, and on a party (kiosk) an interrupted flow is a typical scenario.

Recommendation (product): either program the address first and persist the vehicle after success (or in one backend-side transaction), or mark the vehicle as "incomplete" until programAddress succeeds and block it on the roster list until then. At minimum document this behavior in a comment.

[keskad]

Won't change.

[keskad]

Reliability: 409 swallowing too broad. layout_vehicle_already_on_roster || err.status === 409 swallows every 409, e.g. a conflict from another vehicle or a different semantic error. The pulse still runs, masking a real problem.

Recommendation: narrow to exact err.code === "layout_vehicle_already_on_roster" and only for that specific vehicleId (if the API returns context). Other 409s → setError. Log the ignored case via console.warn/telemetry so it doesn't disappear without a trace.