diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..dfdb8b77 --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +*.sh text eol=lf diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index db4b253a..5c2fd426 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -7,7 +7,7 @@ Thank you for your interest in contributing to OpenFlight! This document provide ### Prerequisites - Python 3.10 or higher -- Node.js 20+ (for UI development) +- Node.js 22.12 or newer (for UI development; CI uses the version in `.node-version`) - Git - [uv](https://github.com/astral-sh/uv) package manager (required) diff --git a/README.md b/README.md index 9e6c9063..ae5d7cc9 100644 --- a/README.md +++ b/README.md @@ -315,6 +315,7 @@ uv run pytest tests/ -v - **[Parts List](docs/PARTS.md)** — What to buy - **[Sound Trigger Wiring](docs/sound-trigger-wiring.md)** — How to wire the sound trigger - **[Raspberry Pi Setup](docs/raspberry-pi-setup.md)** — Full setup guide +- **[Electron Kiosk Shell](docs/electron-kiosk-shell.md)** — Why the kiosk runs in Electron and how self-updating could work later - **[Battery Monitoring](docs/battery/README.md)** — Provider architecture, UI states, and shared Pi support - **[Geekworm X1202/X1206 Operator Guide](docs/battery/geekworm.md)** — Batteries, Pi setup, native telemetry, and warnings - **[IWR6843 Operator Guide](docs/iwr6843/README.md)** — Wire, flash, mount, aim, and calibrate the angle radar diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 1ed1c655..7093b0ee 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -7,7 +7,40 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- **Chromium fallback is reachable during Electron upgrades.** If `ui/dist` + already exists, a missing Electron install no longer requires Node 22.12 and + a successful `npm install` before the kiosk can start. Old Node or a failed + install warns and continues to system Chromium. A missing UI still requires + Node 22.12+ and a successful build. +- **First switch from Chromium to Electron resets browser-local UI state.** + Electron persists its own session under `~/.config/openflight-ui` (Linux), + not the system Chromium profile. Units, language, theme, pinned Live metric, + and validation annotations in `localStorage` do not carry over. Export the + Shots CSV on Chromium before switching. Profiles and shot logs are + server-owned and unaffected. See + [Electron Kiosk Shell](electron-kiosk-shell.md#browser-local-state-breaking-on-first-electron-launch). + ### Fixed +- **A crash-looping boot service no longer kills the desktop kiosk.** Every + launcher exit ran a `pkill` that matched the Electron binary path, so an + `openflight.service` that failed at startup (for example because systemd's + PATH hides `~/.local/bin/uv`) restarted every 5 s and killed whichever + kiosk was on screen; Chromium then died with "GPU process isn't usable. + Goodbye." `start-kiosk.sh` now launches the browser in its own process + group and stops only that group (`scripts/kiosk-browser.sh`), refuses to + start while another instance holds `/tmp/openflight-kiosk-.lock` + (exit 3, `OPENFLIGHT_KIOSK_LOCK_FILE` overrides the path), finds `uv` in + `~/.local/bin` / `~/.cargo/bin` when PATH omits them, and prints the + recovery hint to the terminal and journal. The unit file stops retrying + after five failures in five minutes and never retries exit 3. Re-copy + `scripts/setup/openflight.service` (or rerun `scripts/setup/setup.sh`) on + existing Pis to pick up the unit changes. +- **Kiosk startup no longer rebuilds the UI after Electron has already launched.** + `ensure_kiosk_ui` now runs before the splash browser. The helper is also + stored with Unix line endings so a Windows checkout cannot make `ui/dist` + look missing (a CR in the path) and run `npm install` over a live Electron + GPU process. - **On-screen keyboard for profile names.** Adding or renaming a profile on the Pi kiosk now shows a full-screen keyboard. Chromium in `--kiosk` mode does not surface a system keyboard, so the native text field was unusable on the @@ -20,6 +53,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 full horizontal speed, overstating attack angle on any shot with club path. ### Added +- **Electron kiosk shell.** `scripts/start-kiosk.sh` now opens the UI in a pinned + Electron window (`electron@44`) instead of whichever system browser happens to + be installed. Chromium remains a fallback if Electron is not installed (including + when Node is older than 22.12 or `npm install` fails and `ui/dist` already + exists). Installing Electron needs **Node.js 22.12 or newer**. See + [Electron Kiosk Shell](electron-kiosk-shell.md). - **Profiles replace players.** Shots are now attributed to a server-owned profile (a person *or* a place) with a stable id, persisted to `~/.config/openflight/profiles.json` (override with `OPENFLIGHT_PROFILES_PATH` diff --git a/docs/electron-kiosk-shell.md b/docs/electron-kiosk-shell.md new file mode 100644 index 00000000..c51dd05b --- /dev/null +++ b/docs/electron-kiosk-shell.md @@ -0,0 +1,182 @@ +# Electron Kiosk Shell + +`scripts/start-kiosk.sh` launches the React UI inside Electron +(`ui/electron/main.js`) rather than shelling out to whatever browser +happens to be installed on the Pi. This document explains why that's an +improvement, and sketches how it could support self-updating later. It does +not describe anything implemented yet beyond the shell itself — see +[Auto-Updates (Future Work)](#auto-updates-future-work). + +## Why Electron Instead Of A System Browser + +The old `launch_kiosk_browser` tried `chromium-browser`, then `chromium`, +then `google-chrome`, then `firefox` — whichever the OS image happened to +have, with `--kiosk` flags tuned mostly for Chromium. That worked, but it +carried a few risks an Electron shell removes: + +| Concern | System browser | Electron shell | +|---|---|---| +| Rendering engine version | Whatever `apt` installed/upgraded on that Pi — can silently drift between units or after an OS update | Pinned in `ui/package-lock.json` (`electron@44.1.0` today), identical across every Pi until deliberately bumped | +| Kiosk lockdown | `--kiosk` behaves differently across Chromium, Chrome, and Firefox; Firefox's kiosk mode in particular is looser (menu/shortcuts still reachable) | One `BrowserWindow` with `kiosk: true`, no application menu, and `setWindowOpenHandler` denying any popup — the same guarantees everywhere | +| Startup noise | Chromium's "restore previous session" / crash bubbles needed extra flags (`--disable-session-crashed-bubble`) to suppress | Electron has no Chromium session-restore prompt to suppress. Its **default session still persists** under the app `userData` directory (`~/.config/openflight-ui` on Linux) — [Session](https://www.electronjs.org/docs/latest/api/session), [app.getPath('userData')](https://www.electronjs.org/docs/latest/api/app#appgetpathname). That is a *different* profile from system Chromium (`~/.config/chromium` / `chromium-browser`) | +| Maintenance surface | A 4-branch `if/elif` detection ladder to keep working across Raspberry Pi OS Bookworm/Bullseye, Lite/Desktop images | One binary, one launch path; `npm ci` makes the exact runtime reproducible in CI the same way any other dependency is | +| Extensibility | A browser tab is sandboxed from the OS — no filesystem, process, or native API access | The Electron **main process** is a regular Node.js process with full OS access, which is what makes [self-updating](#auto-updates-future-work) possible at all | + +The old detection ladder is kept as a fallback (`launch_kiosk_browser` still +tries `chromium-browser`/`chromium` if `ui/node_modules/.bin/electron` is +missing), so a Pi that hasn't installed Electron doesn't lose its kiosk +entirely — it just loses the guarantees above until Electron is installed. + +`start-kiosk.sh` builds `ui/dist` only when that directory is missing. If the +UI is already built but Electron is not installed, it *tries* `npm install` +when Node.js is 22.12+. Old Node, an offline Pi, or a failed install logs a +warning and continues to the Chromium fallback instead of aborting startup. + +## Browser-local state (breaking on first Electron launch) + +Electron does **not** reuse the system Chromium profile. The first time a unit +switches from Chromium to Electron, browser-local `localStorage` looks empty: + +| Data | Storage | Survives the switch? | +|---|---|---| +| Profiles and shot logs | Server (`~/.config/openflight/profiles.json`, session JSONL) | Yes | +| Units, theme, language, pinned Live metric | Chromium `localStorage` | No — re-set in the footer / Live grid | +| Validation annotations (comparator device, speed, notes) | `localStorage` key `openflight-validation-entries` | No | + +**Before** switching a validation unit to Electron, export the Shots CSV +(**Export CSV** on the Shots tab) while still on Chromium. After the switch, +re-enter units, theme, language, and the pinned metric once. + +This is an accepted one-time reset, not a silent migration. Chromium's LevelDB +profile is not copied into Electron `userData`. + +## What Didn't Change + +Electron here is a shell, not a rewrite: `ui/electron/main.js` opens a +`BrowserWindow` and points it at the same URL the browser used to load +(`http://localhost:8080`, served by Flask from `ui/dist`). The React app, +the WebSocket connection (`socketService.ts`), and the Flask server are +untouched — `getServerOrigin()` still resolves to `window.location.origin`, +which is the Electron window's origin now instead of a browser tab's. + +## Process Ownership + +`scripts/kiosk-browser.sh` launches Electron (or the Chromium fallback) with +`setsid`, so the whole browser tree, including the zygote, GPU, network and +renderer helpers Chromium forks, lives in one process group that nothing +else on the Pi belongs to. Shutdown signals that group and nothing else. The +earlier cleanup matched the Electron binary path with `pkill -f`, which also +killed kiosks started by *other* launcher instances; see the changelog for +the boot-service crash loop that exposed it. `start-kiosk.sh` additionally +holds `/tmp/openflight-kiosk-.lock` for its lifetime and exits with +status 3 if another instance already holds it. + +## Auto-Updates (Future Work) + +Nothing below is implemented. It's worth writing down now because "Electron +shell" and "auto-update" are usually mentioned in the same breath, and +because OpenFlight's deployment shape (a small fleet of Pis you personally +maintain, not a public app store release) points toward a different design +than the default Electron answer. + +There are two separate things that could be "updated," and they call for +different mechanisms. + +### 1. UI content (the React build) — already effectively live + +Electron loads a URL, not a bundled copy of `ui/dist`. Whatever Flask is +currently serving is what the window shows. So once a Pi has pulled a new +`ui/dist` (via the existing `git pull && npm run build` flow in +[splash-screen.md](splash-screen.md#updating-an-existing-pi)) and the +service restarts, the Electron window shows the new UI on its next launch — +no Electron-specific update logic needed for this layer. This is already +true today. + +### 2. The Electron shell itself + +`electron` is a normal `devDependency` in `ui/package.json`. Bumping its +version is a normal dependency bump: change the version, `npm install`, +commit the updated lockfile, `git pull` on each Pi. No runtime auto-update +machinery is needed for this either, as long as updates continue to arrive +through `git pull` + reinstall rather than an out-of-band download. + +Installing that package (not running the Electron binary) needs **Node.js +22.12+** on the Pi. Node 20 prints `npm WARN EBADENGINE` for `electron@44` +and its `@electron/get` helper. See the Node install step in +[raspberry-pi-setup.md](raspberry-pi-setup.md). + +### 3. The interesting case: OpenFlight self-updating without an SSH session + +The capability an Electron main process adds that a browser tab never had +is **the kiosk can update itself**, because `main.js` runs as a full +Node.js process on the Pi rather than inside a sandboxed tab. Two designs, +in increasing order of complexity: + +**A. Main-process-driven `git pull` (recommended starting point)** + +The main process periodically (or on a UI-triggered "Check for Updates" +action, via a `contextBridge` preload script) does the same thing an +operator does by hand today: + +1. `git fetch` and compare `HEAD` against `origin/`. +2. If behind: `git pull`, `uv sync`, `npm run build` (in `ui/`). +3. Decide how to apply it: + - Content-only change (`ui/` touched, `ui/electron/` and + `ui/package.json`'s `electron` version untouched) → `win.loadURL()` + again, or just wait for the operator's next launch. + - Shell change (Electron itself bumped, or `main.js` changed) → + `app.relaunch(); app.exit(0)`, or restart the systemd unit + (`systemctl --user restart openflight` / `sudo systemctl restart + openflight`, per `scripts/setup/openflight.service`) so the new + `main.js` is picked up. + +This reuses the exact update path already documented for manual updates — +it just runs it from inside the app instead of over SSH. It also keeps +using GitHub as the source of truth, so no new release infrastructure, +signing, or hosting is required. + +Things to get right if this is built: +- **Trust boundary:** whatever triggers the pull (a timer or a UI button) + must not be reachable by anything the Flask server exposes over the + network — this must stay a main-process-only action, not a socket event + or HTTP endpoint, so a device on the same LAN can't trigger arbitrary + `git pull`/`uv sync` execution on the Pi. +- **Partial-failure safety:** a `git pull` that succeeds but an `npm run + build` that fails should not leave the Pi worse off than before — keep + the previous `ui/dist` until the new build succeeds (e.g. build to a + temp directory and swap), and skip the restart on build failure. +- **Mid-round updates:** don't apply an update (especially the + shell-restart kind) while a shot/session is in progress; gate it on + session/idle state the same way the splash screen gates on startup state. +- **Network dependence:** the Pi may be on a golf-sim LAN with no general + internet access even when it can reach GitHub, or vice versa — the check + should fail closed (skip silently) rather than block startup. + +**B. `electron-updater` + a packaged build** + +The conventional Electron answer — `electron-builder` packages the app, +`electron-updater`'s `autoUpdater.checkForUpdatesAndNotify()` polls a feed +(GitHub Releases, S3, or a self-hosted static server) and swaps the +installed build. This is the right model for shipping to users you don't +operate the hardware for. + +It's a bigger lift than option A here, for two reasons specific to this +project: +- It requires the packaging step this shell deliberately skipped (see the + original Electron-shell decision: "just run from source, no installers"). + `ui/dist` would need to be bundled into the package rather than loaded + live from Flask, which reintroduces the "which layer updates independently" + question this doc just resolved for the source-checkout model. +- `electron-updater`'s Linux auto-update support is limited to the AppImage + format. That's buildable for `arm64` (Raspberry Pi OS 64-bit, which this + fleet already requires), but it's a new build target, a new artifact to + test on real hardware, and a release/signing pipeline to stand up — none + of which exists for this project today. + +**Recommendation:** start with (A) if/when self-updating is prioritized. It +matches the fleet's actual shape (Pis you `git pull` on, not an app store +audience), reuses infrastructure that already exists (`uv sync`, `npm run +build`, the systemd unit), and doesn't require adopting a packaging and +release pipeline before there's a concrete need for one. Revisit (B) only if +OpenFlight starts distributing prebuilt images to people who don't run `git +pull` themselves. diff --git a/docs/raspberry-pi-setup.md b/docs/raspberry-pi-setup.md index a0de4ea0..7b2f61ce 100644 --- a/docs/raspberry-pi-setup.md +++ b/docs/raspberry-pi-setup.md @@ -36,6 +36,19 @@ Run the following command: sudo apt update && sudo apt install -y swig liblgpio-dev python3-dev ``` +The UI/Electron kiosk shell needs **Node.js 22.12 or newer** to *install* +Electron. Raspberry Pi OS `apt` Node is often 18 or 20 and will print +`EBADENGINE` (or fail) for `electron@44`. A Pi that already has `ui/dist` can +still start: `start-kiosk.sh` falls back to system Chromium until Node is +upgraded and `npm install` in `ui/` succeeds. Install Node 22 LTS before a +first UI build or to actually run the Electron shell: + +```bash +curl -fsSL https://deb.nodesource.com/setup_22.x | sudo -E bash - +sudo apt-get install -y nodejs +node -v # should report v22.12.0 or later +``` + If `./scripts/setup/setup.sh` updates `~/.bashrc`, you may need to run `source ~/.bashrc` (or open a new terminal) so your current shell picks up the new environment variables immediately without needing to reboot or re-login. ### 2. Run the setup script @@ -318,6 +331,27 @@ If the mapping is missing or points at the wrong radar, re-run the wizard: /dev/ttyUSB...` in the server logs. See [K-LD7 Troubleshooting](kld7-troubleshooting.md) for "Wrong length reply" and other connection issues. +### Kiosk Window Closes Seconds After Loading + +If the UI appears and then vanishes with `GPU process launch failed`, +`Failed to send GetTerminationStatus message to zygote`, and finally +`GPU process isn't usable. Goodbye.` in the terminal, something outside the +window killed Electron's helper processes. The usual culprit is a second +copy of `start-kiosk.sh`, typically a failing boot service restarting in a +loop while you launch by hand: + +```bash +sudo systemctl status openflight --no-pager # "activating (auto-restart)" = looping +journalctl -u openflight -n 40 --no-pager # the recovery hint is printed here +``` + +Fix whatever the journal reports, or `sudo systemctl disable openflight` if +you launch from the desktop instead. Current launchers refuse to start while +another instance holds `/tmp/openflight-kiosk-.lock` (exit code 3) and +only ever stop the browser they started, so an old unit file is the one thing +left to update: re-copy `scripts/setup/openflight.service` as shown in +[Auto-Start on Boot](#auto-start-on-boot). + ### Service Won't Start ```bash diff --git a/docs/splash-screen.md b/docs/splash-screen.md index 3b936bf8..708475ff 100644 --- a/docs/splash-screen.md +++ b/docs/splash-screen.md @@ -128,6 +128,14 @@ installer always preserves that checkout's local wrapper. No reboot is required. Close an existing OpenFlight session first, then launch the refreshed desktop icon. +If this pull is the first that launches **Electron** instead of system +Chromium, browser-local settings (units, theme, language, pinned Live metric) +and Shots validation notes do not migrate. Export **Export CSV** on the Shots +tab while Chromium is still the kiosk, then re-set preferences after Electron +starts. See [Electron Kiosk Shell](electron-kiosk-shell.md#browser-local-state-breaking-on-first-electron-launch). +If Node is still 20 or the Pi is offline, startup keeps using Chromium until +Electron can be installed. + Raspberry Pi desktop settings determine whether icons require a single click or a double click. The installer removes the separate “execute or execute in terminal” choice; it does not change the user's global file-manager click diff --git a/scripts/ensure-kiosk-ui.sh b/scripts/ensure-kiosk-ui.sh new file mode 100644 index 00000000..ceb7d44b --- /dev/null +++ b/scripts/ensure-kiosk-ui.sh @@ -0,0 +1,56 @@ +# Sourced by start-kiosk.sh. Build a missing UI, but do not fail startup when +# Electron cannot be installed — Chromium remains the kiosk fallback. + +_ensure_kiosk_ui_build() { + # shellcheck source=require-node.sh + source "$SCRIPT_DIR/require-node.sh" + if ! openflight_node_meets_min; then + openflight_node_install_hint + show_startup_failure \ + "server" \ + "Node.js is too old to build the UI" \ + "OpenFlight needs Node.js ${OPENFLIGHT_MIN_NODE} or newer (found $(openflight_node_version 2>/dev/null || echo none)). Upgrade Node, then relaunch." + fi + if ! (cd "$PROJECT_DIR/ui" && npm install && npm run build); then + show_startup_failure \ + "server" \ + "OpenFlight interface build failed" \ + "Check the terminal log or network connection, then relaunch OpenFlight." + fi +} + +_try_install_electron_shell() { + warn "Electron kiosk shell missing. Attempting install..." + # shellcheck source=require-node.sh + source "$SCRIPT_DIR/require-node.sh" + if ! openflight_node_meets_min; then + warn "Node.js is too old to install Electron (need ${OPENFLIGHT_MIN_NODE}+, found $(openflight_node_version 2>/dev/null || echo none)); falling back to Chromium." + return 0 + fi + if ! (cd "$PROJECT_DIR/ui" && npm install); then + warn "Could not install Electron; falling back to Chromium if available." + return 0 + fi + if [ ! -x "$PROJECT_DIR/ui/node_modules/.bin/electron" ]; then + warn "Electron is still missing after npm install; falling back to Chromium if available." + fi +} + +ensure_kiosk_ui() { + PROJECT_DIR="${PROJECT_DIR//$'\r'/}" + SCRIPT_DIR="${SCRIPT_DIR//$'\r'/}" + local dist_dir="$PROJECT_DIR/ui/dist" + local electron_bin="$PROJECT_DIR/ui/node_modules/.bin/electron" + + if [ ! -d "$dist_dir" ]; then + warn "UI not built. Building now..." + _ensure_kiosk_ui_build + return + fi + + if [ -x "$electron_bin" ]; then + return 0 + fi + + _try_install_electron_shell +} diff --git a/scripts/kiosk-browser.sh b/scripts/kiosk-browser.sh new file mode 100644 index 00000000..cea1594f --- /dev/null +++ b/scripts/kiosk-browser.sh @@ -0,0 +1,121 @@ +# Sourced by start-kiosk.sh. Launches the kiosk browser (the pinned Electron +# shell, else a system Chromium) in its own process group and stops exactly +# that group on shutdown. +# +# Chromium-based browsers fork a zygote, GPU, network and renderer processes +# that outlive a signal to the launcher PID. The previous cleanup swept them +# up by pattern-matching the Electron binary path, which caught *every* +# Electron on the machine: a crash-looping openflight.service ran it every 5 s and +# killed the desktop session's kiosk each time (Chromium then died with +# "GPU process isn't usable. Goodbye."). Owning a process group makes "ours" +# exact and leaves anyone else's browser alone. +# +# Expects from the caller: PROJECT_DIR, log(), warn(). +# Sets: BROWSER_PID, BROWSER_PGID, BROWSER_LAUNCHED. + +launch_kiosk_browser() { + local url="$1" + local electron_bin="$PROJECT_DIR/ui/node_modules/.bin/electron" + + log "Launching kiosk shell (Electron)..." + if [ -x "$electron_bin" ]; then + _launch_kiosk_process env DISPLAY=:0 OPENFLIGHT_URL="$url" "$electron_bin" "$PROJECT_DIR/ui" + elif command -v chromium-browser &> /dev/null; then + warn "Electron kiosk shell not installed (run 'npm install' in ui/); falling back to chromium-browser" + _launch_kiosk_process env DISPLAY=:0 chromium-browser --kiosk --noerrdialogs --disable-infobars --disable-session-crashed-bubble --password-store=basic "$url" + elif command -v chromium &> /dev/null; then + warn "Electron kiosk shell not installed (run 'npm install' in ui/); falling back to chromium" + _launch_kiosk_process env DISPLAY=:0 chromium --kiosk --noerrdialogs --disable-infobars --disable-session-crashed-bubble --password-store=basic "$url" + else + warn "No Electron kiosk shell and no fallback browser found. Open $url manually." + return 1 + fi +} + +stop_kiosk_browser() { + if [ -z "$BROWSER_PID" ]; then + return 0 + fi + + if kill -0 "$BROWSER_PID" 2>/dev/null; then + log "Closing kiosk shell..." + _signal_kiosk_browser TERM + # Chromium shuts its helper processes down in order after SIGTERM; + # give it a few seconds before forcing. + local _ + for _ in {1..20}; do + if ! kill -0 "$BROWSER_PID" 2>/dev/null; then + break + fi + sleep 0.25 + done + if kill -0 "$BROWSER_PID" 2>/dev/null; then + warn "Kiosk shell did not exit after SIGTERM; forcing" + _signal_kiosk_browser KILL + fi + fi + wait "$BROWSER_PID" 2>/dev/null || true + + # Any helper that outlived the main process is still in our group and in + # nobody else's, so a final group kill is safe and leaves no orphans. + if [ -n "$BROWSER_PGID" ]; then + kill -KILL -- "-$BROWSER_PGID" 2>/dev/null || true + fi + + BROWSER_PID="" + BROWSER_PGID="" + BROWSER_LAUNCHED=false +} + +# Start "$@" in the background inside a fresh session so the whole browser +# tree shares one process group that nothing else on the machine belongs to. +_launch_kiosk_process() { + local have_setsid=false + if command -v setsid >/dev/null 2>&1; then + have_setsid=true + setsid "$@" & + else + warn "setsid unavailable; browser helper processes may outlive shutdown" + "$@" & + fi + BROWSER_PID=$! + BROWSER_LAUNCHED=true + BROWSER_PGID="" + if [ "$have_setsid" = true ]; then + _await_kiosk_process_group + fi +} + +# Record the browser's process group once setsid has detached it. Between +# fork and setsid() the child still shares this script's group, so reading +# too early would make a later group kill take the launcher down as well. +_await_kiosk_process_group() { + local own_pgid pgid _ + own_pgid="$(_kiosk_process_group $$)" + for _ in {1..50}; do + pgid="$(_kiosk_process_group "$BROWSER_PID")" + if [ -z "$pgid" ]; then + # Already exited (crashed on launch); nothing to own. + return 0 + fi + if [ "$pgid" != "$own_pgid" ]; then + BROWSER_PGID="$pgid" + return 0 + fi + sleep 0.02 + done + warn "Kiosk shell did not detach into its own process group; shutdown will signal its PID only" +} + +_kiosk_process_group() { + ps -o pgid= -p "$1" 2>/dev/null | tr -d '[:space:]' +} + +_signal_kiosk_browser() { + local sig="$1" + if [ -n "$BROWSER_PGID" ]; then + kill "-$sig" -- "-$BROWSER_PGID" 2>/dev/null || true + else + kill "-$sig" "$BROWSER_PID" 2>/dev/null || true + fi +} diff --git a/scripts/require-node.sh b/scripts/require-node.sh new file mode 100644 index 00000000..823f63a3 --- /dev/null +++ b/scripts/require-node.sh @@ -0,0 +1,45 @@ +# Sourced by setup and kiosk scripts. Electron 44's npm installer requires +# Node 22.12+ (see ui/package.json engines and electron's own engines field). +OPENFLIGHT_MIN_NODE="22.12.0" + +openflight_node_version() { + command -v node >/dev/null 2>&1 || return 1 + local v + v="$(node -v 2>/dev/null || true)" + v="${v#v}" + printf '%s' "${v%%[-+]*}" +} + +openflight_node_meets_min() { + local current cmaj cmin cpat mmaj mmin mpat + current="$(openflight_node_version)" || return 1 + [ -n "$current" ] || return 1 + + IFS=. read -r cmaj cmin cpat <<<"$current" + IFS=. read -r mmaj mmin mpat <<<"$OPENFLIGHT_MIN_NODE" + + cmaj=${cmaj:-0}; cmin=${cmin:-0}; cpat=${cpat:-0} + mmaj=${mmaj:-0}; mmin=${mmin:-0}; mpat=${mpat:-0} + + if (( cmaj > mmaj )); then return 0; fi + if (( cmaj < mmaj )); then return 1; fi + if (( cmin > mmin )); then return 0; fi + if (( cmin < mmin )); then return 1; fi + (( cpat >= mpat )) +} + +openflight_node_install_hint() { + cat <<'EOF' +OpenFlight needs Node.js 22.12 or newer to install the Electron kiosk shell. +Raspberry Pi OS / Debian apt Node is often older than that (Node 18 or 20). + +Raspberry Pi (64-bit): + curl -fsSL https://deb.nodesource.com/setup_22.x | sudo -E bash - + sudo apt-get install -y nodejs + +macOS: + brew install node + +Then confirm with: node -v +EOF +} diff --git a/scripts/setup/openflight.service b/scripts/setup/openflight.service index a482e11e..fe7b87a9 100644 --- a/scripts/setup/openflight.service +++ b/scripts/setup/openflight.service @@ -2,6 +2,12 @@ Description=OpenFlight Golf Launch Monitor After=network.target graphical.target Wants=graphical.target +# A launcher that fails during preparation exits within a second. Without a +# limit systemd retried it every 5 s indefinitely, and each attempt's cleanup +# killed the desktop session's kiosk. Five failures in five minutes stops the +# loop; `sudo systemctl reset-failed openflight` re-arms it after a fix. +StartLimitIntervalSec=300 +StartLimitBurst=5 [Service] Type=simple @@ -11,6 +17,9 @@ Environment=DISPLAY=:0 ExecStart=/home/coleman/openflight/scripts/start-kiosk.sh Restart=on-failure RestartSec=5 +# Exit 3 means another OpenFlight instance already owns the kiosk (see +# acquire_instance_lock in scripts/start-kiosk.sh); retrying cannot help. +RestartPreventExitStatus=3 [Install] WantedBy=graphical.target diff --git a/scripts/setup/setup.sh b/scripts/setup/setup.sh index 2cc39bfb..72de33bd 100755 --- a/scripts/setup/setup.sh +++ b/scripts/setup/setup.sh @@ -134,18 +134,15 @@ else exit 1 fi -# Check for Node.js +# Check for Node.js (Electron 44's npm installer requires 22.12+) +# shellcheck source=../require-node.sh +source "$SCRIPT_DIR/../require-node.sh" log "Checking Node.js..." -if command -v node &> /dev/null; then - NODE_VERSION=$(node --version) - log "Node.js $NODE_VERSION found ✓" +if openflight_node_meets_min; then + log "Node.js $(openflight_node_version) found ✓" else - error "Node.js not found. Please install Node.js 18+" - if [ "$PLATFORM" == "pi" ]; then - info "On Raspberry Pi, run: sudo apt install nodejs npm" - elif [ "$PLATFORM" == "macos" ]; then - info "On macOS, run: brew install node" - fi + error "Node.js $OPENFLIGHT_MIN_NODE+ required, found $(openflight_node_version 2>/dev/null || echo none)" + openflight_node_install_hint exit 1 fi diff --git a/scripts/start-kiosk.sh b/scripts/start-kiosk.sh index ad8f0a72..9f290566 100755 --- a/scripts/start-kiosk.sh +++ b/scripts/start-kiosk.sh @@ -1,7 +1,7 @@ #!/bin/bash # # OpenFlight Kiosk Startup Script -# Starts the radar server and launches Chromium in kiosk mode +# Starts the radar server and launches the Electron kiosk shell # set -e @@ -44,6 +44,7 @@ STARTUP_DISMISS_FILE="" STARTUP_LOG_PATH="${OPENFLIGHT_STARTUP_LOG:-$HOME/openflight_sessions/terminal_logs/}" SPLASH_PID="" BROWSER_PID="" +BROWSER_PGID="" BROWSER_LAUNCHED=false SERVER_PID="" # Rolling buffer mode is the only mode (streaming mode removed) @@ -505,28 +506,8 @@ error() { echo -e "${RED}[OpenFlight]${NC} $1" } -launch_kiosk_browser() { - local url="$1" - local chrome_flags="--kiosk --noerrdialogs --disable-infobars --disable-session-crashed-bubble --password-store=basic" - - log "Launching kiosk browser..." - if command -v chromium-browser &> /dev/null; then - DISPLAY=:0 chromium-browser $chrome_flags "$url" & - elif command -v chromium &> /dev/null; then - DISPLAY=:0 chromium $chrome_flags "$url" & - elif command -v google-chrome &> /dev/null; then - DISPLAY=:0 google-chrome $chrome_flags "$url" & - elif command -v firefox &> /dev/null; then - DISPLAY=:0 firefox --kiosk "$url" & - else - warn "No supported browser found. Open $url manually." - warn "Supported browsers: chromium-browser, chromium, google-chrome, firefox" - return 1 - fi - - BROWSER_PID=$! - BROWSER_LAUNCHED=true -} +# shellcheck source=kiosk-browser.sh +source "$SCRIPT_DIR/kiosk-browser.sh" stop_startup_splash_server() { if [ -n "$SPLASH_PID" ] && kill -0 "$SPLASH_PID" 2>/dev/null; then @@ -617,6 +598,7 @@ show_startup_failure() { local preserve_existing="${5:-false}" error "$message" + error " $recovery" if [ -n "$STARTUP_STATUS_FILE" ] && [ -f "$STARTUP_STATUS_FILE" ]; then local status_args=( fail "$STARTUP_STATUS_FILE" @@ -700,15 +682,52 @@ cleanup() { log "Shutting down..." shutdown_server stop_startup_splash_server - if [ -n "$BROWSER_PID" ]; then - kill "$BROWSER_PID" 2>/dev/null || true - fi - # Chromium forks child processes that survive kill — clean them all - pkill -f "chromium.*--kiosk" 2>/dev/null || true - pkill -f "chrome.*--kiosk" 2>/dev/null || true + stop_kiosk_browser exit "$exit_code" } +acquire_instance_lock() { + # One kiosk per web port. The default lives in /tmp rather than + # XDG_RUNTIME_DIR because openflight.service and a desktop session have + # different runtime dirs, and it was exactly that pair fighting over the + # screen: a failing boot service ran cleanup every 5 s and killed the + # desktop session's Electron each time. + local lock_file="${OPENFLIGHT_KIOSK_LOCK_FILE:-/tmp/openflight-kiosk-${PORT}.lock}" + + if ! command -v flock >/dev/null 2>&1; then + warn "flock unavailable; cannot guard against a second OpenFlight instance" + return 0 + fi + # Probe in a subshell: a failed redirection on `exec` would abort the script. + if ! ( : >>"$lock_file" ) 2>/dev/null; then + warn "Cannot open $lock_file; continuing without the single-instance guard" + return 0 + fi + exec {INSTANCE_LOCK_FD}>>"$lock_file" + if ! flock -n "$INSTANCE_LOCK_FD"; then + error "OpenFlight is already running (lock held on $lock_file)." + error " Stop the other instance first. If it is the boot service: sudo systemctl stop openflight" + # Exit 3 is listed in openflight.service's RestartPreventExitStatus so + # systemd does not retry every 5 s while someone else owns the kiosk. + exit 3 + fi +} + +ensure_uv_on_path() { + # systemd starts the service with a minimal PATH that omits the user-local + # install dirs astral's installer uses, so `uv` looked missing at boot. + if command -v uv >/dev/null 2>&1; then + return 0 + fi + local candidate + for candidate in "$HOME/.local/bin" "$HOME/.cargo/bin"; do + if [ -x "$candidate/uv" ]; then + export PATH="$candidate:$PATH" + return 0 + fi + done +} + configure_kld7_latency() { local setup_script="$PROJECT_DIR/scripts/setup/setup_kld7_latency.sh" @@ -982,15 +1001,22 @@ if [ "$DRY_RUN" = true ]; then exit 0 fi +acquire_instance_lock + +# shellcheck source=ensure-kiosk-ui.sh +source "$SCRIPT_DIR/ensure-kiosk-ui.sh" +ensure_kiosk_ui + start_startup_splash # Ensure the environment is in sync (uv recreates/repairs .venv as needed, # so a moved project dir self-heals instead of failing with "command not found") +ensure_uv_on_path if ! command -v uv >/dev/null 2>&1; then show_startup_failure \ "server" \ "OpenFlight preparation failed" \ - "The uv command is unavailable. Ask a technician to repair the OpenFlight installation." + "The uv command is unavailable (checked PATH, ~/.local/bin and ~/.cargo/bin). Install it with: curl -LsSf https://astral.sh/uv/install.sh | sh" fi UV_SYNC_ARGS=(--quiet) @@ -1020,20 +1046,6 @@ fi configure_kld7_latency -# Check if UI is built -if [ ! -d "ui/dist" ]; then - warn "UI not built. Building now..." - cd ui - if ! npm install || ! npm run build; then - cd .. - show_startup_failure \ - "server" \ - "OpenFlight interface build failed" \ - "Check the terminal log or network connection, then relaunch OpenFlight." - fi - cd .. -fi - # Start Grafana Alloy for log shipping (if installed and credentials configured) if command -v alloy &> /dev/null || systemctl is-enabled alloy &> /dev/null 2>&1; then if sudo test -f /etc/alloy/credentials.env; then diff --git a/tests/test_openflight_service.py b/tests/test_openflight_service.py new file mode 100644 index 00000000..b1abc7be --- /dev/null +++ b/tests/test_openflight_service.py @@ -0,0 +1,50 @@ +"""Contracts for the boot-time systemd unit.""" + +from __future__ import annotations + +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +UNIT = REPO_ROOT / "scripts/setup/openflight.service" +SETUP = REPO_ROOT / "scripts/setup/setup.sh" + + +def _unit_lines() -> list[str]: + return [line.strip() for line in UNIT.read_text(encoding="utf-8").splitlines()] + + +def _section(name: str) -> list[str]: + lines = _unit_lines() + start = lines.index(f"[{name}]") + 1 + body = [] + for line in lines[start:]: + if line.startswith("["): + break + if line: + body.append(line) + return body + + +def test_service_does_not_restart_when_another_instance_owns_the_kiosk(): + """Exit 3 means "someone else is running OpenFlight"; retrying every 5 s is noise.""" + assert "RestartPreventExitStatus=3" in _section("Service") + + +def test_service_stops_crash_looping_instead_of_retrying_forever(): + """1,100 restarts in 90 minutes each ran cleanup against the desktop session's kiosk.""" + unit = _section("Unit") + burst = next(line for line in unit if line.startswith("StartLimitBurst=")) + interval = next(line for line in unit if line.startswith("StartLimitIntervalSec=")) + + assert int(burst.split("=", 1)[1]) <= 5 + assert int(interval.split("=", 1)[1]) >= 60 + + +def test_every_home_path_in_the_unit_is_rewritten_by_setup(): + """setup.sh only rewrites the project path; any other /home/coleman entry would ship stale.""" + setup = SETUP.read_text(encoding="utf-8") + assert "s|/home/coleman/openflight|$PROJECT_DIR|g" in setup + + for line in _unit_lines(): + if "/home/coleman" in line: + assert "/home/coleman/openflight" in line, line diff --git a/tests/test_start_kiosk.py b/tests/test_start_kiosk.py index 90e3a72c..9e95eeea 100644 --- a/tests/test_start_kiosk.py +++ b/tests/test_start_kiosk.py @@ -1,7 +1,13 @@ """Tests for the kiosk entry script flag wiring.""" +import os +import re +import shlex import shutil +import signal +import stat import subprocess +import time from pathlib import Path import pytest @@ -422,13 +428,15 @@ def test_startup_splash_passes_status_file_to_server(): def test_launcher_reports_distinct_failures_and_waits_for_dismissal(): repo_root = Path(__file__).resolve().parents[1] script = (repo_root / "scripts/start-kiosk.sh").read_text(encoding="utf-8") + ensure_ui = (repo_root / "scripts/ensure-kiosk-ui.sh").read_text(encoding="utf-8") assert "show_startup_failure()" in script assert '"OpenFlight preparation failed"' in script assert '"server"' in script assert 'while [ ! -f "$STARTUP_DISMISS_FILE" ]' in script assert 'uv sync "${UV_SYNC_ARGS[@]}"' in script - assert "npm run build" in script + assert "ensure_kiosk_ui" in script + assert "npm run build" in ensure_ui def test_start_kiosk_script_has_valid_shell_syntax(): @@ -440,6 +448,20 @@ def test_start_kiosk_script_has_valid_shell_syntax(): capture_output=True, text=True, ) + subprocess.run( + ["bash", "-n", "scripts/ensure-kiosk-ui.sh"], + cwd=repo_root, + check=True, + capture_output=True, + text=True, + ) + subprocess.run( + ["bash", "-n", "scripts/kiosk-browser.sh"], + cwd=repo_root, + check=True, + capture_output=True, + text=True, + ) def test_camera_capture_uses_system_python_for_sync_and_server_start(): @@ -552,3 +574,482 @@ def test_iwr6843_horizontal_phase_reference_is_forwarded(): def test_iwr6843_horizontal_phase_reference_is_omitted_by_default(): command = _dry_run("--iwr6843").stdout.strip() assert "--iwr6843-horizontal-phase-reference-rad" not in command + + +def test_kiosk_shell_scripts_use_unix_newlines(): + repo_root = Path(__file__).resolve().parents[1] + for relative in ( + "scripts/start-kiosk.sh", + "scripts/ensure-kiosk-ui.sh", + "scripts/kiosk-browser.sh", + "scripts/require-node.sh", + ): + data = (repo_root / relative).read_bytes() + assert b"\r" not in data, f"{relative} must use LF newlines so sourced path checks match on the Pi" + + +def test_ui_is_ensured_before_the_kiosk_browser_launches(): + script = _read_script() + ensure_call = script.index("\nensure_kiosk_ui\n") + splash_call = script.index("\nstart_startup_splash\n") + assert ensure_call < splash_call + + +def _read_script() -> str: + return (Path(__file__).resolve().parents[1] / "scripts/start-kiosk.sh").read_text( + encoding="utf-8" + ) + + +def _read_kiosk_browser_helper() -> str: + return (Path(__file__).resolve().parents[1] / "scripts/kiosk-browser.sh").read_text( + encoding="utf-8" + ) + + +def _launcher_function() -> str: + helper = _read_kiosk_browser_helper() + return helper[helper.index("launch_kiosk_browser() {") : helper.index("stop_kiosk_browser() {")] + + +def test_launch_kiosk_browser_prefers_the_electron_shell(): + """The pinned Electron runtime must be tried before any system browser.""" + launcher = _launcher_function() + + electron_idx = launcher.index('if [ -x "$electron_bin" ]; then') + chromium_browser_idx = launcher.index("command -v chromium-browser") + chromium_idx = launcher.index("command -v chromium &> /dev/null") + + assert electron_idx < chromium_browser_idx < chromium_idx + assert 'local electron_bin="$PROJECT_DIR/ui/node_modules/.bin/electron"' in launcher + assert '"$electron_bin" "$PROJECT_DIR/ui"' in launcher + + +def test_launch_kiosk_browser_still_falls_back_without_electron(): + """A Pi that hasn't run `npm install` yet must not lose its kiosk entirely.""" + launcher = _launcher_function() + + assert "chromium-browser --kiosk" in launcher + assert "chromium --kiosk" in launcher + assert "No Electron kiosk shell and no fallback browser found" in launcher + + +def test_cleanup_stops_only_the_browser_it_launched(): + """A path-matching pkill killed Electron windows owned by *other* launcher instances. + + A crash-looping systemd unit ran cleanup every 5 s and each pass killed the + desktop session's kiosk (Chromium then died with "GPU process isn't usable"). + """ + script = _read_script() + helper = _read_kiosk_browser_helper() + cleanup_fn = script[script.index("cleanup() {") : script.index("configure_kld7_latency() {")] + + assert "pkill" not in script + assert "pkill" not in helper + assert 'source "$SCRIPT_DIR/kiosk-browser.sh"' in script + assert "stop_kiosk_browser" in cleanup_fn + + +def test_ui_build_check_does_not_block_startup_on_missing_electron(): + """A built UI must still start when Electron cannot be installed.""" + script = _read_script() + + assert 'source "$SCRIPT_DIR/ensure-kiosk-ui.sh"' in script + assert "ensure_kiosk_ui" in script + assert 'if [ ! -d "ui/dist" ] || [ ! -x "ui/node_modules/.bin/electron" ]; then' not in script + + +def _bash_path(path: Path) -> str: + resolved = str(path.resolve()) + if os.name != "nt": + return resolved + converted = subprocess.run( + ["bash", "-lc", f"wslpath -u {shlex.quote(resolved.replace(chr(92), '/'))}"], + capture_output=True, + text=True, + check=False, + ) + mapped = converted.stdout.strip() + if converted.returncode == 0 and mapped: + return mapped + posix = Path(resolved).as_posix() + return f"/{posix[0].lower()}{posix[2:]}" + + +def _write_executable(path: Path, contents: str) -> None: + path.write_text(contents, encoding="utf-8", newline="\n") + path.chmod(path.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) + + +def _run_ensure_kiosk_ui( + tmp_path: Path, + *, + node_version: str, + has_dist: bool, + npm_exit: int, +) -> subprocess.CompletedProcess[str]: + repo_scripts = Path(__file__).resolve().parents[1] / "scripts" + scripts_dir = tmp_path / "scripts" + scripts_dir.mkdir() + for name in ("ensure-kiosk-ui.sh", "require-node.sh"): + text = (repo_scripts / name).read_text(encoding="utf-8").replace("\r\n", "\n").replace("\r", "\n") + (scripts_dir / name).write_bytes(text.encode("utf-8")) + project_dir = tmp_path / "project" + ui_dir = project_dir / "ui" + ui_dir.mkdir(parents=True) + if has_dist: + (ui_dir / "dist").mkdir() + (ui_dir / "dist" / "index.html").write_text("\n", encoding="utf-8") + + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + npm_called = tmp_path / "npm-called" + _write_executable( + bin_dir / "node", + f"#!/usr/bin/env bash\necho 'v{node_version}'\n", + ) + _write_executable( + bin_dir / "npm", + "\n".join( + [ + "#!/usr/bin/env bash", + f"printf '%s\\n' \"$*\" >> {_bash_path(npm_called)}", + f"exit {npm_exit}", + "", + ] + ), + ) + + harness = tmp_path / "run-ensure.sh" + _write_executable( + harness, + "\n".join( + [ + "#!/usr/bin/env bash", + "set -euo pipefail", + 'PROJECT_DIR="$1"', + 'SCRIPT_DIR="$2"', + 'BIN_DIR="$3"', + 'chmod +x "$BIN_DIR"/* || true', + 'export PATH="$BIN_DIR:$PATH"', + "log() { printf 'LOG %s\\n' \"$1\"; }", + "warn() { printf 'WARN %s\\n' \"$1\"; }", + "show_startup_failure() {", + ' printf \'FAILURE component=%s message=%s\\n\' "$1" "$2"', + " exit 42", + "}", + '# shellcheck source=/dev/null', + 'source "$SCRIPT_DIR/ensure-kiosk-ui.sh"', + "ensure_kiosk_ui", + "printf 'CONTINUED\\n'", + "", + ] + ), + ) + + return subprocess.run( + [ + "bash", + _bash_path(harness), + _bash_path(project_dir), + _bash_path(scripts_dir), + _bash_path(bin_dir), + ], + cwd=tmp_path, + capture_output=True, + text=True, + check=False, + ) + + +def test_existing_ui_continues_when_node_is_too_old_to_install_electron(tmp_path): + """A Pi with a built UI and Node 20 must keep using Chromium instead of dying at startup.""" + result = _run_ensure_kiosk_ui( + tmp_path, + node_version="20.19.0", + has_dist=True, + npm_exit=1, + ) + + combined = result.stdout + result.stderr + assert result.returncode == 0, combined + assert "CONTINUED" in result.stdout + assert "FAILURE" not in combined + assert not (tmp_path / "npm-called").exists() + + +def test_existing_ui_continues_when_electron_npm_install_fails(tmp_path): + """Offline or failed Electron install must not block a unit that already has ui/dist.""" + result = _run_ensure_kiosk_ui( + tmp_path, + node_version="22.12.0", + has_dist=True, + npm_exit=1, + ) + + combined = result.stdout + result.stderr + assert result.returncode == 0, combined + assert "CONTINUED" in result.stdout + assert "FAILURE" not in combined + assert (tmp_path / "npm-called").exists() + + +def test_missing_ui_still_fails_when_node_is_too_old(tmp_path): + result = _run_ensure_kiosk_ui( + tmp_path, + node_version="20.19.0", + has_dist=False, + npm_exit=0, + ) + + combined = result.stdout + result.stderr + assert result.returncode == 42, combined + assert "FAILURE" in combined + assert "CONTINUED" not in result.stdout + + +# --- Browser process-tree ownership ----------------------------------------- + + +def _is_alive(pid: int) -> bool: + """True while the process exists and is not a zombie.""" + try: + os.kill(pid, 0) + except ProcessLookupError: + return False + except PermissionError: + return True + status = Path(f"/proc/{pid}/status") + if status.exists(): + for line in status.read_text(encoding="utf-8").splitlines(): + if line.startswith("State:"): + return "Z" not in line.split()[1] + return True + + +def _wait_until_dead(pids: list[int], timeout_s: float = 5.0) -> list[int]: + deadline = time.monotonic() + timeout_s + while time.monotonic() < deadline: + survivors = [pid for pid in pids if _is_alive(pid)] + if not survivors: + return [] + time.sleep(0.05) + return [pid for pid in pids if _is_alive(pid)] + + +def _make_fake_electron(project_dir: Path) -> Path: + """A stand-in Electron: a main process that forks a lingering child, like Chromium does.""" + bin_dir = project_dir / "ui" / "node_modules" / ".bin" + bin_dir.mkdir(parents=True) + fake = bin_dir / "electron" + _write_executable( + fake, + "\n".join( + [ + "#!/usr/bin/env bash", + "sleep 300 &", + "child=$!", + 'printf \'%s %s\\n\' "$$" "$child" >> "$OPENFLIGHT_TEST_PID_LOG"', + 'wait "$child"', + "", + ] + ), + ) + return fake + + +def _read_pid_log(path: Path) -> list[int]: + deadline = time.monotonic() + 5.0 + while time.monotonic() < deadline: + if path.exists() and path.read_text(encoding="utf-8").strip(): + return [int(token) for token in path.read_text(encoding="utf-8").split()] + time.sleep(0.05) + raise AssertionError(f"fake electron never recorded its pids in {path}") + + +@pytest.mark.skipif( + shutil.which("setsid") is None or os.name == "nt", + reason="process-group ownership needs setsid (util-linux)", +) +def test_stop_kiosk_browser_kills_the_launched_tree_and_spares_other_instances(tmp_path): + """Stopping must take the whole tree we started and nothing we did not start.""" + repo_root = Path(__file__).resolve().parents[1] + project_dir = tmp_path / "project" + fake_electron = _make_fake_electron(project_dir) + ours_log = tmp_path / "ours.pids" + theirs_log = tmp_path / "theirs.pids" + + # Another launcher's kiosk (same binary path) that must survive our cleanup. + other = subprocess.Popen( + ["bash", str(fake_electron)], + env={**os.environ, "OPENFLIGHT_TEST_PID_LOG": str(theirs_log)}, + start_new_session=True, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + try: + their_pids = _read_pid_log(theirs_log) + + harness = tmp_path / "run-browser.sh" + _write_executable( + harness, + "\n".join( + [ + "#!/usr/bin/env bash", + "set -u", + 'PROJECT_DIR="$1"', + 'SCRIPT_DIR="$2"', + 'BROWSER_PID=""', + 'BROWSER_PGID=""', + "BROWSER_LAUNCHED=false", + "log() { printf 'LOG %s\\n' \"$1\"; }", + "warn() { printf 'WARN %s\\n' \"$1\"; }", + "# shellcheck source=/dev/null", + 'source "$SCRIPT_DIR/kiosk-browser.sh"', + 'launch_kiosk_browser "http://127.0.0.1:1/"', + 'printf \'LAUNCHED pid=%s pgid=%s\\n\' "$BROWSER_PID" "$BROWSER_PGID"', + "sleep 0.5", + "stop_kiosk_browser", + "printf 'STOPPED\\n'", + "", + ] + ), + ) + result = subprocess.run( + ["bash", str(harness), str(project_dir), str(repo_root / "scripts")], + env={**os.environ, "OPENFLIGHT_TEST_PID_LOG": str(ours_log)}, + capture_output=True, + text=True, + check=False, + timeout=30, + ) + combined = result.stdout + result.stderr + assert result.returncode == 0, combined + assert "STOPPED" in result.stdout, combined + + our_pids = _read_pid_log(ours_log) + assert _wait_until_dead(our_pids) == [], f"launched tree survived cleanup: {combined}" + assert other.poll() is None, "cleanup killed a kiosk it did not launch" + assert all(_is_alive(pid) for pid in their_pids), ( + "cleanup killed another instance's children" + ) + finally: + try: + os.killpg(other.pid, signal.SIGKILL) + except ProcessLookupError: + pass + other.wait(timeout=5) + + +def test_launch_kiosk_browser_puts_every_browser_in_its_own_process_group(): + """Electron and the Chromium fallbacks must be stoppable as one group, not by pattern.""" + launcher = _launcher_function() + helper = _read_kiosk_browser_helper() + + assert "setsid" in helper + assert "BROWSER_PGID" in helper + # Every branch launches through the same wrapper so none can regress to a bare `&`. + assert launcher.count("_launch_kiosk_process ") == 3 + assert " &\n" not in launcher + + +# --- Single-instance guard -------------------------------------------------- + + +def _hold_lock(path: Path): + fcntl = pytest.importorskip("fcntl") + handle = open(path, "w", encoding="utf-8") # noqa: SIM115 - closed by the caller + fcntl.flock(handle, fcntl.LOCK_EX | fcntl.LOCK_NB) + return handle + + +@pytest.mark.skipif(shutil.which("flock") is None, reason="instance guard needs flock (util-linux)") +def test_second_launcher_instance_exits_without_running_cleanup(tmp_path): + """While another instance owns the kiosk, a new one must not build, launch, or kill anything.""" + repo_root = Path(__file__).resolve().parents[1] + lock_path = tmp_path / "kiosk.lock" + holder = _hold_lock(lock_path) + try: + result = subprocess.run( + ["bash", "scripts/start-kiosk.sh", "--mock"], + cwd=repo_root, + env={**os.environ, "OPENFLIGHT_KIOSK_LOCK_FILE": str(lock_path)}, + capture_output=True, + text=True, + check=False, + timeout=30, + ) + finally: + holder.close() + + combined = result.stdout + result.stderr + assert result.returncode == 3, combined + assert "already running" in combined + assert str(lock_path) in combined + assert "Shutting down" not in combined + assert "Building" not in combined + + +@pytest.mark.skipif(shutil.which("flock") is None, reason="instance guard needs flock (util-linux)") +def test_dry_run_ignores_the_instance_lock(tmp_path): + repo_root = Path(__file__).resolve().parents[1] + lock_path = tmp_path / "kiosk.lock" + holder = _hold_lock(lock_path) + try: + result = subprocess.run( + ["bash", "scripts/start-kiosk.sh", "--mock", "--dry-run"], + cwd=repo_root, + env={**os.environ, "OPENFLIGHT_KIOSK_LOCK_FILE": str(lock_path)}, + capture_output=True, + text=True, + check=False, + timeout=30, + ) + finally: + holder.close() + + assert result.returncode == 0, result.stdout + result.stderr + assert result.stdout.strip().startswith("openflight-server") + + +def test_instance_lock_is_taken_after_dry_run_and_before_any_side_effect(): + script = _read_script() + + dry_run_idx = script.index('if [ "$DRY_RUN" = true ]; then') + lock_idx = script.index("\nacquire_instance_lock\n") + ensure_idx = script.index("\nensure_kiosk_ui\n") + splash_idx = script.index("\nstart_startup_splash\n") + + assert dry_run_idx < lock_idx < ensure_idx < splash_idx + guard = script[script.index("acquire_instance_lock() {") : lock_idx] + assert "flock -n" in guard + assert "exit 3" in guard + # The default path must not depend on XDG_RUNTIME_DIR: a system service and + # a desktop session have different runtime dirs but must share one lock. + assert "OPENFLIGHT_KIOSK_LOCK_FILE:-/tmp/openflight-kiosk-${PORT}.lock" in guard + + +# --- Running under systemd -------------------------------------------------- + + +def test_uv_is_found_in_user_install_dirs_before_the_preparation_check(): + """systemd's PATH omits ~/.local/bin, which is where astral's installer puts uv.""" + script = _read_script() + + resolve_idx = script.index("\nensure_uv_on_path\n") + check_idx = script.index("if ! command -v uv >/dev/null 2>&1; then") + assert resolve_idx < check_idx + + resolver = script[script.index("ensure_uv_on_path() {") : resolve_idx] + assert '"$HOME/.local/bin"' in resolver + assert '"$HOME/.cargo/bin"' in resolver + + +def test_startup_failure_prints_the_recovery_hint_to_the_terminal(): + """journalctl only showed "preparation failed"; the reason lived in the splash JSON.""" + script = _read_script() + failure_fn = script[ + script.index("show_startup_failure() {") : script.index("# Mount tilt has no safe default") + ] + + assert re.search(r'error ".*\$recovery"', failure_fn), failure_fn diff --git a/ui/README.md b/ui/README.md index 9321b75a..b7122f2b 100644 --- a/ui/README.md +++ b/ui/README.md @@ -9,7 +9,7 @@ and how the whole system fits together, see the [root README](../README.md). ## Quick start -You need Node 20+. +You need Node 22.12 or newer (`electron@44` will not install cleanly on Node 20). ### Frontend-only (recommended for UI work) @@ -131,7 +131,9 @@ preparation/playback errors. The pin is stored in `localStorage` under `openflight.hero-metric`. Theme is stored under -`openflight.theme` (default dark). +`openflight.theme` (default dark). Those keys live in the **current browser +profile**. Electron does not share Chromium's profile; see +[Electron Kiosk Shell](../docs/electron-kiosk-shell.md#browser-local-state-breaking-on-first-electron-launch). **Display mode** lives at `/display`: a compact, fullscreen-friendly dashboard for mounted screens and TVs. The [root README](../README.md#tv-display-mode) diff --git a/ui/electron/main.js b/ui/electron/main.js new file mode 100644 index 00000000..265617cb --- /dev/null +++ b/ui/electron/main.js @@ -0,0 +1,41 @@ +// Kiosk shell for the OpenFlight React UI. Loads whatever URL the launcher +// script gives it (the startup splash, then the app itself once it +// navigates there) in a chromeless, fullscreen window — this replaces +// scripts/start-kiosk.sh's old system-browser detection (chromium-browser / +// chromium / google-chrome / firefox) with one pinned Chromium version. + +import { app, BrowserWindow, Menu } from 'electron'; +import { resolveTargetUrl } from './resolveTargetUrl.js'; + +const targetUrl = resolveTargetUrl(process.env, process.argv); + +Menu.setApplicationMenu(null); + +function createWindow() { + const win = new BrowserWindow({ + kiosk: true, + fullscreen: true, + autoHideMenuBar: true, + backgroundColor: '#000000', + webPreferences: { + contextIsolation: true, + sandbox: true, + }, + }); + + win.setMenuBarVisibility(false); + // The kiosk shell only ever shows the OpenFlight UI itself; deny any + // attempt (e.g. target="_blank" links) to pop a second window. + win.webContents.setWindowOpenHandler(() => ({ action: 'deny' })); + win.loadURL(targetUrl); + + win.on('closed', () => { + app.quit(); + }); +} + +app.whenReady().then(createWindow); + +app.on('window-all-closed', () => { + app.quit(); +}); diff --git a/ui/electron/resolveTargetUrl.js b/ui/electron/resolveTargetUrl.js new file mode 100644 index 00000000..2bb2a419 --- /dev/null +++ b/ui/electron/resolveTargetUrl.js @@ -0,0 +1,7 @@ +export const DEFAULT_URL = 'http://localhost:8080'; + +// Pulled out of main.js so it can be unit-tested without importing the +// `electron` module, which throws outside an actual Electron runtime. +export function resolveTargetUrl(env, argv) { + return env.OPENFLIGHT_URL || argv[2] || DEFAULT_URL; +} diff --git a/ui/electron/resolveTargetUrl.test.js b/ui/electron/resolveTargetUrl.test.js new file mode 100644 index 00000000..9f965913 --- /dev/null +++ b/ui/electron/resolveTargetUrl.test.js @@ -0,0 +1,30 @@ +import { describe, it, expect } from 'vitest'; +import { resolveTargetUrl, DEFAULT_URL } from './resolveTargetUrl.js'; + +describe('resolveTargetUrl', () => { + it('defaults to the local Flask server when nothing else is set', () => { + expect(resolveTargetUrl({}, ['electron', 'main.js'])).toBe(DEFAULT_URL); + }); + + it('prefers OPENFLIGHT_URL over the CLI argument', () => { + expect( + resolveTargetUrl({ OPENFLIGHT_URL: 'http://pi.local:8080' }, [ + 'electron', + 'main.js', + 'http://cli-arg:8080', + ]) + ).toBe('http://pi.local:8080'); + }); + + it('falls back to a CLI argument when the env var is unset', () => { + expect(resolveTargetUrl({}, ['electron', 'main.js', 'http://cli-arg:8080'])).toBe( + 'http://cli-arg:8080' + ); + }); + + it('ignores an empty OPENFLIGHT_URL rather than passing it through', () => { + expect( + resolveTargetUrl({ OPENFLIGHT_URL: '' }, ['electron', 'main.js', 'http://cli-arg:8080']) + ).toBe('http://cli-arg:8080'); + }); +}); diff --git a/ui/package-lock.json b/ui/package-lock.json index eab3cc46..c6329350 100644 --- a/ui/package-lock.json +++ b/ui/package-lock.json @@ -22,6 +22,7 @@ "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^6.1.0", "concurrently": "^10.0.5", + "electron": "^44.1.0", "eslint": "^10.8.1", "eslint-plugin-react-hooks": "^7.1.1", "eslint-plugin-react-refresh": "^0.5.4", @@ -34,6 +35,9 @@ "typescript-eslint": "^8.67.0", "vite": "^8.2.2", "vitest": "^4.1.11" + }, + "engines": { + "node": ">=22.12.0" } }, "node_modules/@babel/code-frame": { @@ -276,6 +280,50 @@ "node": ">=6.9.0" } }, + "node_modules/@electron-internal/extract-zip": { + "version": "1.0.5", + "resolved": "https://registry.npmjs.org/@electron-internal/extract-zip/-/extract-zip-1.0.5.tgz", + "integrity": "sha512-+bqFCP98pLI0Tt0XQo1TmlXtwjWchISndDOxCkEcIuUgXWpBnLyRI+2DU+mesvnMMX6L1XDqYNA0lXNDHd/yiA==", + "dev": true, + "license": "BSD-2-Clause", + "engines": { + "node": ">=22.12.0" + } + }, + "node_modules/@electron/get": { + "version": "5.1.0", + "resolved": "https://registry.npmjs.org/@electron/get/-/get-5.1.0.tgz", + "integrity": "sha512-3kSBtG8ObcTVfXanm5vVJ6UnBLEVmVsRk1M+vGqCuMBV+XLCbJYuWQful+yIy0GQDsSlK0kHEriEHn7SPk4EnA==", + "dev": true, + "license": "MIT", + "dependencies": { + "debug": "^4.1.1", + "env-paths": "^3.0.0", + "graceful-fs": "^4.2.11", + "progress": "^2.0.3", + "semver": "^7.6.3", + "sumchecker": "^3.0.1" + }, + "engines": { + "node": ">=22.12.0" + }, + "optionalDependencies": { + "undici": "^7.24.4" + } + }, + "node_modules/@electron/get/node_modules/semver": { + "version": "7.8.5", + "resolved": "https://registry.npmjs.org/semver/-/semver-7.8.5.tgz", + "integrity": "sha512-Y7/KDsb8LjooZpwaqGyulO6DQlksgCncchHGk+sZIY4SBvUocMBEFH5Ur1fI4dV+Jvl0w6cjvucaIi40puRioA==", + "dev": true, + "license": "ISC", + "bin": { + "semver": "bin/semver.js" + }, + "engines": { + "node": ">=10" + } + }, "node_modules/@esbuild/aix-ppc64": { "version": "0.28.1", "resolved": "https://registry.npmjs.org/@esbuild/aix-ppc64/-/aix-ppc64-0.28.1.tgz", @@ -1076,9 +1124,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1096,9 +1141,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -1116,9 +1158,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1136,9 +1175,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1156,9 +1192,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1176,9 +1209,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -2310,6 +2340,25 @@ "dev": true, "license": "MIT" }, + "node_modules/electron": { + "version": "44.1.0", + "resolved": "https://registry.npmjs.org/electron/-/electron-44.1.0.tgz", + "integrity": "sha512-kmLg8axOg22DC3fXx5NCwBYW8fx0rK2zzJ3Tf67GjNCkxfoxEcd+yo0QfDjlBtHJs3xeA8fTg64b6iVzFTVErg==", + "dev": true, + "license": "MIT", + "dependencies": { + "@electron-internal/extract-zip": "^1.0.1", + "@electron/get": "^5.0.0", + "@types/node": "^24.9.0" + }, + "bin": { + "electron": "cli.js", + "install-electron": "install.js" + }, + "engines": { + "node": ">= 22.12.0" + } + }, "node_modules/electron-to-chromium": { "version": "1.5.267", "resolved": "https://registry.npmjs.org/electron-to-chromium/-/electron-to-chromium-1.5.267.tgz", @@ -2317,6 +2366,23 @@ "dev": true, "license": "ISC" }, + "node_modules/electron/node_modules/@types/node": { + "version": "24.13.3", + "resolved": "https://registry.npmjs.org/@types/node/-/node-24.13.3.tgz", + "integrity": "sha512-Dh8vAsV36ig5wa9OX4pXvMc9D3Veibfw2wix0CUwYODLD8nkj9UsLjASr49nPg+2eKzxhBV+v7L8pXvT4e639Q==", + "dev": true, + "license": "MIT", + "dependencies": { + "undici-types": "~7.18.0" + } + }, + "node_modules/electron/node_modules/undici-types": { + "version": "7.18.2", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.18.2.tgz", + "integrity": "sha512-AsuCzffGHJybSaRrmr5eHr81mwJU3kjw6M+uprWvCXiNeN9SOGwQ3Jn8jb8m3Z6izVgknn1R0FTCEAP2QrLY/w==", + "dev": true, + "license": "MIT" + }, "node_modules/emoji-regex": { "version": "10.6.0", "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-10.6.0.tgz", @@ -2464,6 +2530,19 @@ } } }, + "node_modules/env-paths": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/env-paths/-/env-paths-3.0.0.tgz", + "integrity": "sha512-dtJUTepzMW3Lm/NPxRf3wP4642UWhjL2sQxc+ym2YMj1m/H2zDNQOlezafzkHwn6sMstjHTwG6iQQsctDW/b1A==", + "dev": true, + "license": "MIT", + "engines": { + "node": "^12.20.0 || ^14.13.1 || >=16.0.0" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/es-define-property": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/es-define-property/-/es-define-property-1.0.1.tgz", @@ -3103,6 +3182,13 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/graceful-fs": { + "version": "4.2.11", + "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.2.11.tgz", + "integrity": "sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ==", + "dev": true, + "license": "ISC" + }, "node_modules/has-symbols": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/has-symbols/-/has-symbols-1.1.0.tgz", @@ -4039,6 +4125,16 @@ "url": "https://github.com/prettier/prettier?sponsor=1" } }, + "node_modules/progress": { + "version": "2.0.3", + "resolved": "https://registry.npmjs.org/progress/-/progress-2.0.3.tgz", + "integrity": "sha512-7PiHtLll5LdnKIMw100I+8xJXR5gW2QwWYkT6iJva0bXitZKa/XMrSbdmg3r2Xnaidz9Qumd0VPaMrZlF9V9sA==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=0.4.0" + } + }, "node_modules/proxy-addr": { "version": "2.0.7", "resolved": "https://registry.npmjs.org/proxy-addr/-/proxy-addr-2.0.7.tgz", @@ -4600,6 +4696,19 @@ "url": "https://github.com/chalk/strip-ansi?sponsor=1" } }, + "node_modules/sumchecker": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/sumchecker/-/sumchecker-3.0.1.tgz", + "integrity": "sha512-MvjXzkz/BOfyVDkG0oFOtBxHX2u3gKbMHIF/dXblZsgD3BWOFLmHovIpZY7BykJdAjcqRCBi1WYBNdEC9yI7vg==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "debug": "^4.1.0" + }, + "engines": { + "node": ">= 8.0" + } + }, "node_modules/supports-color": { "version": "10.2.2", "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-10.2.2.tgz", @@ -4800,6 +4909,17 @@ "typescript": ">=4.8.4 <6.1.0" } }, + "node_modules/undici": { + "version": "7.29.0", + "resolved": "https://registry.npmjs.org/undici/-/undici-7.29.0.tgz", + "integrity": "sha512-IDxfleLmmbSskfWSUATiN1nfn2rDuvnMOqb5CWR92iIfojA0Ud+ulOAAEQ57LPr9rWmsreUyf5lwyao+7GNNVw==", + "dev": true, + "license": "MIT", + "optional": true, + "engines": { + "node": ">=20.18.1" + } + }, "node_modules/undici-types": { "version": "8.3.0", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-8.3.0.tgz", diff --git a/ui/package.json b/ui/package.json index a78b1a75..b6bd567d 100644 --- a/ui/package.json +++ b/ui/package.json @@ -3,7 +3,12 @@ "private": true, "version": "1.0.0", "type": "module", + "main": "electron/main.js", + "engines": { + "node": ">=22.12.0" + }, "scripts": { + "electron": "electron .", "dev": "vite", "dev:mock": "concurrently -k \"npm:mock-server\" \"npm:dev\"", "mock-server": "tsx mock-server/index.ts", @@ -33,6 +38,7 @@ "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^6.1.0", "concurrently": "^10.0.5", + "electron": "^44.1.0", "eslint": "^10.8.1", "eslint-plugin-react-hooks": "^7.1.1", "eslint-plugin-react-refresh": "^0.5.4", diff --git a/ui/vite.config.ts b/ui/vite.config.ts index 03ce9656..eb7c2b99 100644 --- a/ui/vite.config.ts +++ b/ui/vite.config.ts @@ -14,7 +14,7 @@ export default defineConfig({ }, }, test: { - include: ['src/**/*.test.{ts,tsx}', 'tests/**/*.test.{ts,tsx}'], + include: ['src/**/*.test.{ts,tsx}', 'tests/**/*.test.{ts,tsx}', 'electron/**/*.test.js'], exclude: ['tests/e2e/**'], }, });