From 229ea3eaeeb0231c34528a8597e1fea9140ed0fe Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 17:33:45 +0000 Subject: [PATCH 01/10] Replace kiosk browser detection with an Electron shell start-kiosk.sh previously shelled out to whichever of chromium-browser/chromium/google-chrome/firefox happened to be installed. Add an Electron kiosk shell (ui/electron/main.cjs) that loads the same URL in a pinned, chromeless BrowserWindow, and make it the default; the old browser detection remains only as a fallback if Electron isn't installed. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_015QvFHgWxQdwJCQNZ1DRg2B --- scripts/start-kiosk.sh | 34 ++++----- ui/electron/main.cjs | 43 ++++++++++++ ui/package-lock.json | 153 ++++++++++++++++++++++++++++++++++++----- ui/package.json | 3 + 4 files changed, 198 insertions(+), 35 deletions(-) create mode 100644 ui/electron/main.cjs diff --git a/scripts/start-kiosk.sh b/scripts/start-kiosk.sh index ad8f0a72c..26d297b13 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 @@ -507,20 +507,19 @@ error() { 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" & + local electron_bin="$PROJECT_DIR/ui/node_modules/.bin/electron" + + log "Launching kiosk shell (Electron)..." + if [ -x "$electron_bin" ]; then + 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" + DISPLAY=:0 chromium-browser --kiosk --noerrdialogs --disable-infobars --disable-session-crashed-bubble --password-store=basic "$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" & + warn "Electron kiosk shell not installed (run 'npm install' in ui/); falling back to chromium" + DISPLAY=:0 chromium --kiosk --noerrdialogs --disable-infobars --disable-session-crashed-bubble --password-store=basic "$url" & else - warn "No supported browser found. Open $url manually." - warn "Supported browsers: chromium-browser, chromium, google-chrome, firefox" + warn "No Electron kiosk shell and no fallback browser found. Open $url manually." return 1 fi @@ -703,7 +702,8 @@ cleanup() { if [ -n "$BROWSER_PID" ]; then kill "$BROWSER_PID" 2>/dev/null || true fi - # Chromium forks child processes that survive kill — clean them all + # Electron/Chromium fork child processes that survive kill — clean them all + pkill -f "ui/node_modules/electron/dist/electron" 2>/dev/null || true pkill -f "chromium.*--kiosk" 2>/dev/null || true pkill -f "chrome.*--kiosk" 2>/dev/null || true exit "$exit_code" @@ -1020,9 +1020,9 @@ fi configure_kld7_latency -# Check if UI is built -if [ ! -d "ui/dist" ]; then - warn "UI not built. Building now..." +# Check if UI is built and the Electron kiosk shell is installed +if [ ! -d "ui/dist" ] || [ ! -x "ui/node_modules/.bin/electron" ]; then + warn "UI not built or Electron shell missing. Building now..." cd ui if ! npm install || ! npm run build; then cd .. diff --git a/ui/electron/main.cjs b/ui/electron/main.cjs new file mode 100644 index 000000000..e63f0f7b3 --- /dev/null +++ b/ui/electron/main.cjs @@ -0,0 +1,43 @@ +'use strict'; + +// 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. + +const { app, BrowserWindow, Menu } = require('electron'); + +const DEFAULT_URL = 'http://localhost:8080'; +const targetUrl = process.env.OPENFLIGHT_URL || process.argv[2] || DEFAULT_URL; + +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/package-lock.json b/ui/package-lock.json index a78ebf6e4..555a37544 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": "^9.2.1", + "electron": "^44.1.0", "eslint": "^10.8.1", "eslint-plugin-react-hooks": "^7.1.1", "eslint-plugin-react-refresh": "^0.5.4", @@ -276,6 +277,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 +1121,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1096,9 +1138,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -1116,9 +1155,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1136,9 +1172,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1156,9 +1189,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1176,9 +1206,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -2347,6 +2374,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", @@ -2354,6 +2400,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": "8.0.0", "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-8.0.0.tgz", @@ -2501,6 +2564,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", @@ -3127,6 +3203,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-flag": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-4.0.0.tgz", @@ -4083,6 +4166,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", @@ -4648,6 +4741,19 @@ "node": ">=8" } }, + "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": "8.1.1", "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-8.1.1.tgz", @@ -4851,6 +4957,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 b21e2f8de..bd313cf3a 100644 --- a/ui/package.json +++ b/ui/package.json @@ -3,7 +3,9 @@ "private": true, "version": "1.0.0", "type": "module", + "main": "electron/main.cjs", "scripts": { + "electron": "electron .", "dev": "vite", "dev:mock": "concurrently -k \"npm:mock-server\" \"npm:dev\"", "mock-server": "tsx mock-server/index.ts", @@ -33,6 +35,7 @@ "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^6.1.0", "concurrently": "^9.2.1", + "electron": "^44.1.0", "eslint": "^10.8.1", "eslint-plugin-react-hooks": "^7.1.1", "eslint-plugin-react-refresh": "^0.5.4", From 8982e9d3a92af1ffdc2afdc9ef601ac3586a7949 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 17:36:12 +0000 Subject: [PATCH 02/10] Document the Electron kiosk shell rationale and future auto-update paths Explain why Electron replaces ad hoc system-browser detection (pinned runtime, consistent kiosk lockdown, single code path, main-process OS access), and sketch two designs for self-updating later: a main-process git-pull-driven update (recommended first) versus a packaged electron-builder/electron-updater release pipeline. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_015QvFHgWxQdwJCQNZ1DRg2B --- README.md | 1 + docs/electron-kiosk-shell.md | 142 +++++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+) create mode 100644 docs/electron-kiosk-shell.md diff --git a/README.md b/README.md index 9e6c9063f..ae5d7cc99 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/electron-kiosk-shell.md b/docs/electron-kiosk-shell.md new file mode 100644 index 000000000..0c90f1a29 --- /dev/null +++ b/docs/electron-kiosk-shell.md @@ -0,0 +1,142 @@ +# Electron Kiosk Shell + +`scripts/start-kiosk.sh` launches the React UI inside Electron +(`ui/electron/main.cjs`) 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 starts a fresh profile each launch; there's no session-restore prompt to suppress | +| 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 run `npm install` yet doesn't lose its kiosk +entirely — it just loses the guarantees above until Electron is installed. + +## What Didn't Change + +Electron here is a shell, not a rewrite: `ui/electron/main.cjs` 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. + +## 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. + +### 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.cjs` 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.cjs` 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.cjs` 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. From 38d60060e3206109df2344dfa520adb8cff85cfb Mon Sep 17 00:00:00 2001 From: Cormac McGrath Date: Wed, 2 Sep 2026 14:51:05 +0100 Subject: [PATCH 03/10] Update Node.js requirements across documentation and scripts for compatibility with Electron 44. --- CONTRIBUTING.md | 2 +- docs/electron-kiosk-shell.md | 5 +++++ docs/raspberry-pi-setup.md | 10 ++++++++++ scripts/require-node.sh | 35 +++++++++++++++++++++++++++++++++++ scripts/setup/setup.sh | 17 +++++++---------- scripts/start-kiosk.sh | 9 +++++++++ ui/README.md | 2 +- ui/package.json | 3 +++ 8 files changed, 71 insertions(+), 12 deletions(-) create mode 100644 scripts/require-node.sh diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index db4b253a2..5c2fd4265 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/docs/electron-kiosk-shell.md b/docs/electron-kiosk-shell.md index 0c90f1a29..d3b2e1176 100644 --- a/docs/electron-kiosk-shell.md +++ b/docs/electron-kiosk-shell.md @@ -65,6 +65,11 @@ 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 diff --git a/docs/raspberry-pi-setup.md b/docs/raspberry-pi-setup.md index a0de4ea0f..03cf25ab1 100644 --- a/docs/raspberry-pi-setup.md +++ b/docs/raspberry-pi-setup.md @@ -36,6 +36,16 @@ 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**. Raspberry Pi OS +`apt` Node is often 18 or 20 and will print `EBADENGINE` (or fail) on first +build. Install Node 22 LTS before setup: + +```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 diff --git a/scripts/require-node.sh b/scripts/require-node.sh new file mode 100644 index 000000000..d8390ad62 --- /dev/null +++ b/scripts/require-node.sh @@ -0,0 +1,35 @@ +# 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 lowest + current="$(openflight_node_version)" || return 1 + [ -n "$current" ] || return 1 + lowest="$(printf '%s\n%s\n' "$OPENFLIGHT_MIN_NODE" "$current" | sort -V | head -n1)" + [ "$lowest" = "$OPENFLIGHT_MIN_NODE" ] +} + +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/setup.sh b/scripts/setup/setup.sh index 2cc39bfb8..72de33bd3 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 26d297b13..8c5a94d40 100755 --- a/scripts/start-kiosk.sh +++ b/scripts/start-kiosk.sh @@ -1023,6 +1023,15 @@ configure_kld7_latency # Check if UI is built and the Electron kiosk shell is installed if [ ! -d "ui/dist" ] || [ ! -x "ui/node_modules/.bin/electron" ]; then warn "UI not built or Electron shell missing. Building now..." + # 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 cd ui if ! npm install || ! npm run build; then cd .. diff --git a/ui/README.md b/ui/README.md index be6969fc1..0c9615a67 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) diff --git a/ui/package.json b/ui/package.json index bd313cf3a..5867877c0 100644 --- a/ui/package.json +++ b/ui/package.json @@ -4,6 +4,9 @@ "version": "1.0.0", "type": "module", "main": "electron/main.cjs", + "engines": { + "node": ">=22.12.0" + }, "scripts": { "electron": "electron .", "dev": "vite", From b2578b70fe7fdbc9cd226d54a2111fc2459e7315 Mon Sep 17 00:00:00 2001 From: Cormac McGrath Date: Wed, 2 Sep 2026 15:19:51 +0100 Subject: [PATCH 04/10] refactor(kiosk): migrate Electron main file from .cjs to .js and update related documentation --- docs/electron-kiosk-shell.md | 10 +++--- tests/test_start_kiosk.py | 49 ++++++++++++++++++++++++++++ ui/electron/{main.cjs => main.js} | 8 ++--- ui/electron/resolveTargetUrl.js | 7 ++++ ui/electron/resolveTargetUrl.test.js | 30 +++++++++++++++++ ui/package.json | 2 +- ui/vite.config.ts | 2 +- 7 files changed, 96 insertions(+), 12 deletions(-) rename ui/electron/{main.cjs => main.js} (84%) create mode 100644 ui/electron/resolveTargetUrl.js create mode 100644 ui/electron/resolveTargetUrl.test.js diff --git a/docs/electron-kiosk-shell.md b/docs/electron-kiosk-shell.md index d3b2e1176..5ed339939 100644 --- a/docs/electron-kiosk-shell.md +++ b/docs/electron-kiosk-shell.md @@ -1,7 +1,7 @@ # Electron Kiosk Shell `scripts/start-kiosk.sh` launches the React UI inside Electron -(`ui/electron/main.cjs`) rather than shelling out to whatever browser +(`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 @@ -29,7 +29,7 @@ entirely — it just loses the guarantees above until Electron is installed. ## What Didn't Change -Electron here is a shell, not a rewrite: `ui/electron/main.cjs` opens a +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 @@ -73,7 +73,7 @@ and its `@electron/get` helper. See the Node install step in ### 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.cjs` runs as a full +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: @@ -89,11 +89,11 @@ operator does by hand today: - 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.cjs` changed) → + - 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.cjs` is picked up. + `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 diff --git a/tests/test_start_kiosk.py b/tests/test_start_kiosk.py index 90e3a72c3..535b20ac8 100644 --- a/tests/test_start_kiosk.py +++ b/tests/test_start_kiosk.py @@ -552,3 +552,52 @@ 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 _read_script() -> str: + return (Path(__file__).resolve().parents[1] / "scripts/start-kiosk.sh").read_text( + encoding="utf-8" + ) + + +def test_launch_kiosk_browser_prefers_the_electron_shell(): + """The pinned Electron runtime must be tried before any system browser.""" + script = _read_script() + launcher = script[ + script.index("launch_kiosk_browser() {") : script.index("stop_startup_splash_server() {") + ] + + 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.""" + script = _read_script() + launcher = script[ + script.index("launch_kiosk_browser() {") : script.index("stop_startup_splash_server() {") + ] + + 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_kills_the_electron_process_tree(): + """Electron, like Chromium, forks children that survive a signal to the launcher PID.""" + script = _read_script() + cleanup_fn = script[script.index("cleanup() {") : script.index("configure_kld7_latency() {")] + + assert 'pkill -f "ui/node_modules/electron/dist/electron"' in cleanup_fn + + +def test_ui_build_check_also_requires_the_electron_shell(): + """Rebuilding the UI must also install Electron if a checkout predates it.""" + script = _read_script() + + assert 'if [ ! -d "ui/dist" ] || [ ! -x "ui/node_modules/.bin/electron" ]; then' in script diff --git a/ui/electron/main.cjs b/ui/electron/main.js similarity index 84% rename from ui/electron/main.cjs rename to ui/electron/main.js index e63f0f7b3..265617cbb 100644 --- a/ui/electron/main.cjs +++ b/ui/electron/main.js @@ -1,15 +1,13 @@ -'use strict'; - // 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. -const { app, BrowserWindow, Menu } = require('electron'); +import { app, BrowserWindow, Menu } from 'electron'; +import { resolveTargetUrl } from './resolveTargetUrl.js'; -const DEFAULT_URL = 'http://localhost:8080'; -const targetUrl = process.env.OPENFLIGHT_URL || process.argv[2] || DEFAULT_URL; +const targetUrl = resolveTargetUrl(process.env, process.argv); Menu.setApplicationMenu(null); diff --git a/ui/electron/resolveTargetUrl.js b/ui/electron/resolveTargetUrl.js new file mode 100644 index 000000000..2bb2a4190 --- /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 000000000..9f9659132 --- /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.json b/ui/package.json index 8e1b7804a..b6bd567d7 100644 --- a/ui/package.json +++ b/ui/package.json @@ -3,7 +3,7 @@ "private": true, "version": "1.0.0", "type": "module", - "main": "electron/main.cjs", + "main": "electron/main.js", "engines": { "node": ">=22.12.0" }, diff --git a/ui/vite.config.ts b/ui/vite.config.ts index 03ce96569..eb7c2b99c 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/**'], }, }); From 5d601689514ea0bc36bd9a6a32e237e204f36b1d Mon Sep 17 00:00:00 2001 From: Cormac McGrath Date: Wed, 2 Sep 2026 15:24:47 +0100 Subject: [PATCH 05/10] feat(kiosk): add Electron kiosk shell for a consistent UI experience --- docs/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 1ed1c6551..008f1bed0 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -20,6 +20,11 @@ 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. This + needs **Node.js 22.12 or newer** (`npm WARN EBADENGINE` on Node 20). 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` From 31ee7e15c378a2db76ffa6190c4c0ab4d4f0ff82 Mon Sep 17 00:00:00 2001 From: Cormac McGrath Date: Wed, 2 Sep 2026 15:40:41 +0100 Subject: [PATCH 06/10] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- scripts/require-node.sh | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/scripts/require-node.sh b/scripts/require-node.sh index d8390ad62..823f63a30 100644 --- a/scripts/require-node.sh +++ b/scripts/require-node.sh @@ -11,11 +11,21 @@ openflight_node_version() { } openflight_node_meets_min() { - local current lowest + local current cmaj cmin cpat mmaj mmin mpat current="$(openflight_node_version)" || return 1 [ -n "$current" ] || return 1 - lowest="$(printf '%s\n%s\n' "$OPENFLIGHT_MIN_NODE" "$current" | sort -V | head -n1)" - [ "$lowest" = "$OPENFLIGHT_MIN_NODE" ] + + 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() { From e94a68cc37de884abfa03ee481c74fd1289331e6 Mon Sep 17 00:00:00 2001 From: Cormac McGrath Date: Fri, 4 Sep 2026 12:36:17 +0100 Subject: [PATCH 07/10] docs: update changelog and documentation for Electron kiosk shell changes --- docs/CHANGELOG.md | 19 +++- docs/electron-kiosk-shell.md | 27 +++++- docs/raspberry-pi-setup.md | 9 +- docs/splash-screen.md | 8 ++ scripts/ensure-kiosk-ui.sh | 54 +++++++++++ scripts/start-kiosk.sh | 25 +---- tests/test_start_kiosk.py | 171 ++++++++++++++++++++++++++++++++++- ui/README.md | 4 +- 8 files changed, 283 insertions(+), 34 deletions(-) create mode 100644 scripts/ensure-kiosk-ui.sh diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 008f1bed0..d8d678f27 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -7,6 +7,20 @@ 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 - **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 @@ -22,8 +36,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### 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. This - needs **Node.js 22.12 or newer** (`npm WARN EBADENGINE` on Node 20). See + 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 diff --git a/docs/electron-kiosk-shell.md b/docs/electron-kiosk-shell.md index 5ed339939..2acdde315 100644 --- a/docs/electron-kiosk-shell.md +++ b/docs/electron-kiosk-shell.md @@ -18,15 +18,38 @@ carried a few risks an Electron shell removes: |---|---|---| | 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 starts a fresh profile each launch; there's no session-restore prompt to suppress | +| 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 run `npm install` yet doesn't lose its kiosk +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 diff --git a/docs/raspberry-pi-setup.md b/docs/raspberry-pi-setup.md index 03cf25ab1..6066ad601 100644 --- a/docs/raspberry-pi-setup.md +++ b/docs/raspberry-pi-setup.md @@ -36,9 +36,12 @@ 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**. Raspberry Pi OS -`apt` Node is often 18 or 20 and will print `EBADENGINE` (or fail) on first -build. Install Node 22 LTS before setup: +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 - diff --git a/docs/splash-screen.md b/docs/splash-screen.md index 3b936bf8a..708475ff3 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 000000000..ba64cbc6f --- /dev/null +++ b/scripts/ensure-kiosk-ui.sh @@ -0,0 +1,54 @@ +# 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() { + 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/start-kiosk.sh b/scripts/start-kiosk.sh index 8c5a94d40..6a7381f8a 100755 --- a/scripts/start-kiosk.sh +++ b/scripts/start-kiosk.sh @@ -1020,28 +1020,9 @@ fi configure_kld7_latency -# Check if UI is built and the Electron kiosk shell is installed -if [ ! -d "ui/dist" ] || [ ! -x "ui/node_modules/.bin/electron" ]; then - warn "UI not built or Electron shell missing. Building now..." - # 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 - 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 +# shellcheck source=ensure-kiosk-ui.sh +source "$SCRIPT_DIR/ensure-kiosk-ui.sh" +ensure_kiosk_ui # 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 diff --git a/tests/test_start_kiosk.py b/tests/test_start_kiosk.py index 535b20ac8..1bf089198 100644 --- a/tests/test_start_kiosk.py +++ b/tests/test_start_kiosk.py @@ -1,6 +1,9 @@ """Tests for the kiosk entry script flag wiring.""" +import os +import shlex import shutil +import stat import subprocess from pathlib import Path @@ -422,13 +425,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 +445,13 @@ 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, + ) def test_camera_capture_uses_system_python_for_sync_and_server_start(): @@ -596,8 +608,159 @@ def test_cleanup_kills_the_electron_process_tree(): assert 'pkill -f "ui/node_modules/electron/dist/electron"' in cleanup_fn -def test_ui_build_check_also_requires_the_electron_shell(): - """Rebuilding the UI must also install Electron if a checkout predates it.""" +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 'if [ ! -d "ui/dist" ] || [ ! -x "ui/node_modules/.bin/electron" ]; then' in 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 diff --git a/ui/README.md b/ui/README.md index 33df0d091..b7122f2b6 100644 --- a/ui/README.md +++ b/ui/README.md @@ -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) From ea98c41c36a75eb9bfbe2bb87b5d0dbe4784f1ef Mon Sep 17 00:00:00 2001 From: Cormac McGrath Date: Fri, 4 Sep 2026 13:46:29 +0100 Subject: [PATCH 08/10] fix(kiosk): ensure kiosk UI is initialized before launch and enforce Unix line endings in scripts --- .gitattributes | 1 + docs/CHANGELOG.md | 5 +++++ scripts/ensure-kiosk-ui.sh | 2 ++ scripts/start-kiosk.sh | 8 ++++---- tests/test_start_kiosk.py | 18 ++++++++++++++++++ 5 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 000000000..dfdb8b771 --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +*.sh text eol=lf diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index d8d678f27..921b020dd 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -22,6 +22,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [Electron Kiosk Shell](electron-kiosk-shell.md#browser-local-state-breaking-on-first-electron-launch). ### Fixed +- **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 diff --git a/scripts/ensure-kiosk-ui.sh b/scripts/ensure-kiosk-ui.sh index ba64cbc6f..ceb7d44bd 100644 --- a/scripts/ensure-kiosk-ui.sh +++ b/scripts/ensure-kiosk-ui.sh @@ -37,6 +37,8 @@ _try_install_electron_shell() { } 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" diff --git a/scripts/start-kiosk.sh b/scripts/start-kiosk.sh index 6a7381f8a..6fd076bae 100755 --- a/scripts/start-kiosk.sh +++ b/scripts/start-kiosk.sh @@ -982,6 +982,10 @@ if [ "$DRY_RUN" = true ]; then exit 0 fi +# 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, @@ -1020,10 +1024,6 @@ fi configure_kld7_latency -# shellcheck source=ensure-kiosk-ui.sh -source "$SCRIPT_DIR/ensure-kiosk-ui.sh" -ensure_kiosk_ui - # 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_start_kiosk.py b/tests/test_start_kiosk.py index 1bf089198..082b046f1 100644 --- a/tests/test_start_kiosk.py +++ b/tests/test_start_kiosk.py @@ -566,6 +566,24 @@ def test_iwr6843_horizontal_phase_reference_is_omitted_by_default(): 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/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" From a188095b01a8bf49d6c4f04202bcd8b672a55bf2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 13:24:02 +0000 Subject: [PATCH 09/10] fix(kiosk): stop cleanup from killing kiosks owned by other launcher instances A crash-looping openflight.service exposed three launcher problems on a Pi: - cleanup() swept up Electron/Chromium helpers with `pkill -f` on the binary path, which matched every Electron on the machine. The failing service restarted every 5 s and each pass killed the desktop session's kiosk; Chromium then died with "GPU process isn't usable. Goodbye." The browser now launches via setsid in its own process group (scripts/kiosk-browser.sh) and shutdown signals only that group. - Nothing stopped two launchers from running at once. start-kiosk.sh now holds /tmp/openflight-kiosk-.lock (OPENFLIGHT_KIOSK_LOCK_FILE overrides) after --dry-run and before any side effect, exiting 3 when another instance owns it. The unit lists 3 in RestartPreventExitStatus and caps restarts at five per five minutes. - The service failed because systemd's PATH omits ~/.local/bin, where astral's installer puts uv. The script now checks ~/.local/bin and ~/.cargo/bin before declaring uv missing, and show_startup_failure prints the recovery hint to the terminal so journalctl shows the cause. Tests: functional process-tree test with a fake Electron proving the launched tree dies and a sibling instance survives; lock-guard tests that hold the lock from Python; ordering and unit-file contracts. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0159vUzFq5FTJPr46kNYKdkD --- docs/CHANGELOG.md | 14 ++ docs/electron-kiosk-shell.md | 12 ++ docs/raspberry-pi-setup.md | 21 +++ scripts/kiosk-browser.sh | 121 +++++++++++++ scripts/setup/openflight.service | 9 + scripts/start-kiosk.sh | 80 ++++++--- tests/test_openflight_service.py | 50 ++++++ tests/test_start_kiosk.py | 293 +++++++++++++++++++++++++++++-- 8 files changed, 560 insertions(+), 40 deletions(-) create mode 100644 scripts/kiosk-browser.sh create mode 100644 tests/test_openflight_service.py diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 921b020dd..7093b0eed 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -22,6 +22,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [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` diff --git a/docs/electron-kiosk-shell.md b/docs/electron-kiosk-shell.md index 2acdde315..c51dd05b2 100644 --- a/docs/electron-kiosk-shell.md +++ b/docs/electron-kiosk-shell.md @@ -59,6 +59,18 @@ 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 diff --git a/docs/raspberry-pi-setup.md b/docs/raspberry-pi-setup.md index 6066ad601..7b2f61ce7 100644 --- a/docs/raspberry-pi-setup.md +++ b/docs/raspberry-pi-setup.md @@ -331,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/scripts/kiosk-browser.sh b/scripts/kiosk-browser.sh new file mode 100644 index 000000000..cea1594fa --- /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/setup/openflight.service b/scripts/setup/openflight.service index a482e11ec..fe7b87a9a 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/start-kiosk.sh b/scripts/start-kiosk.sh index 6fd076bae..9f2905667 100755 --- a/scripts/start-kiosk.sh +++ b/scripts/start-kiosk.sh @@ -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,27 +506,8 @@ error() { echo -e "${RED}[OpenFlight]${NC} $1" } -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 - 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" - 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" - 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 - - 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 @@ -616,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" @@ -699,16 +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 - # Electron/Chromium fork child processes that survive kill — clean them all - pkill -f "ui/node_modules/electron/dist/electron" 2>/dev/null || true - 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,6 +1001,8 @@ 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 @@ -990,11 +1011,12 @@ 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) diff --git a/tests/test_openflight_service.py b/tests/test_openflight_service.py new file mode 100644 index 000000000..b1abc7be9 --- /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 082b046f1..9e95eeeac 100644 --- a/tests/test_start_kiosk.py +++ b/tests/test_start_kiosk.py @@ -1,10 +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 @@ -452,6 +455,13 @@ def test_start_kiosk_script_has_valid_shell_syntax(): 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(): @@ -571,6 +581,7 @@ def test_kiosk_shell_scripts_use_unix_newlines(): 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() @@ -590,12 +601,20 @@ def _read_script() -> str: ) +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.""" - script = _read_script() - launcher = script[ - script.index("launch_kiosk_browser() {") : script.index("stop_startup_splash_server() {") - ] + launcher = _launcher_function() electron_idx = launcher.index('if [ -x "$electron_bin" ]; then') chromium_browser_idx = launcher.index("command -v chromium-browser") @@ -608,22 +627,27 @@ def test_launch_kiosk_browser_prefers_the_electron_shell(): 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.""" - script = _read_script() - launcher = script[ - script.index("launch_kiosk_browser() {") : script.index("stop_startup_splash_server() {") - ] + 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_kills_the_electron_process_tree(): - """Electron, like Chromium, forks children that survive a signal to the launcher PID.""" +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 -f "ui/node_modules/electron/dist/electron"' in cleanup_fn + 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(): @@ -782,3 +806,250 @@ def test_missing_ui_still_fails_when_node_is_too_old(tmp_path): 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 From fd31a5e226975cbadccda4d4c5332d2377d0ac3d Mon Sep 17 00:00:00 2001 From: Cormac McGrath Date: Wed, 9 Sep 2026 13:41:48 +0100 Subject: [PATCH 10/10] docs: add electron-kiosk-shell.md to navigation and update links in related documentation --- docs/electron-kiosk-shell.md | 4 ++-- docs/setup/splash-screen.md | 2 +- zensical.toml | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/electron-kiosk-shell.md b/docs/electron-kiosk-shell.md index c51dd05b2..eb5d4431f 100644 --- a/docs/electron-kiosk-shell.md +++ b/docs/electron-kiosk-shell.md @@ -87,7 +87,7 @@ different mechanisms. 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 +[splash-screen.md](setup/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. @@ -103,7 +103,7 @@ 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). +[Raspberry Pi setup](setup/raspberry-pi.md). ### 3. The interesting case: OpenFlight self-updating without an SSH session diff --git a/docs/setup/splash-screen.md b/docs/setup/splash-screen.md index 059ac7809..107f27f13 100644 --- a/docs/setup/splash-screen.md +++ b/docs/setup/splash-screen.md @@ -132,7 +132,7 @@ 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). +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. diff --git a/zensical.toml b/zensical.toml index b2811318d..1bd7bdbd7 100644 --- a/zensical.toml +++ b/zensical.toml @@ -52,6 +52,7 @@ nav = [ "setup/rolling-buffer.md", "setup/auto-start.md", "setup/splash-screen.md", + "electron-kiosk-shell.md", "setup/diagnostic.md", ] },