From e89a57bb97b5a0d624e8d490b6b8aa38ba140817 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Sat, 5 Sep 2026 13:06:55 +0200 Subject: fix(win): graceful shutdown, one startup-mode control, and a stray-\r bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows-only changes, all found by actually running the previous session's work rather than by review alone: - CTRL_CLOSE_EVENT/LOGOFF/SHUTDOWN handler (platform.py, ctypes SetConsoleCtrlHandler) so closing a console window, signing off, or a system shutdown runs the daemon's real _shutdown() instead of Windows just ending the process — closing WebRTC sessions and any in-flight ffmpeg transcode instead of orphaning it. `taskkill /F` itself stays uncatchable (like SIGKILL), so autostart_run() now spawns with CREATE_NEW_PROCESS_GROUP instead of DETACHED_PROCESS and autostart_end() tries CTRL_BREAK_EVENT against the recorded pid first, falling back to the hard kill only if that doesn't stop it in time. - Replaced the Node page's two independent autostart/service-mode toggles with one "start automatically" select (off / at sign-in / as a background service). The old pair let both be active at once — starting the daemon twice, at boot and at sign-in — and their layout broke wrapping inside .node-service's flex row. The new control always removes whichever mechanism is active before installing the target; platform.py's service_install() does the same on the CLI side. The "background service" option disables itself (with a hint pointing at the CLI) when running unpackaged, since service-mode.ps1/service.ps1/firewall.ps1 all assume an installed build's layout — verified live rather than assumed by actually running those scripts unelevated. - findNodeBinary() no longer bakes a stray \r into resolved paths. Found by rebooting after enabling per-user autostart: where.exe listed two matches, and stdout.trim().split('\n')[0] only strips the whole string's ends, leaving line one's own trailing \r attached — which landed inside the Startup .vbs's quoted path and broke it with "Unterminated string constant" at boot. Fixed by splitting on \r?\n and trimming every line. - Dependency audit for the Windows installer (docs/WINDOWS-PORT.md): no VC++ Redistributable needed, confirmed by inspecting the built node-runtime's actual import table rather than assuming. New docs/windows-build.md: a concise clone-to-installer build guide. Co-Authored-By: Claude Sonnet 5 --- docs/WINDOWS-PORT.md | 243 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 190 insertions(+), 53 deletions(-) (limited to 'docs/WINDOWS-PORT.md') diff --git a/docs/WINDOWS-PORT.md b/docs/WINDOWS-PORT.md index da281c3..8fc169b 100644 --- a/docs/WINDOWS-PORT.md +++ b/docs/WINDOWS-PORT.md @@ -1,7 +1,10 @@ # MeshBay — Windows Port -> Status: **W1–W3 + W5–W8 done; W4 packaging built, not yet run on a clean machine.** -> Created 2026-09-03 from a full codebase scan; progress notes added 2026-09-04. +> Status: **W1–W9 done. Packaging (W4) built with both autostart modes, a +> post-install mode toggle, and a static dependency audit — clean-machine +> install still not run.** +> Created 2026-09-03 from a full codebase scan; progress notes added 2026-09-04, +> 2026-09-05. > This document is both the audit results and the implementation plan. > It does not repeat the design decisions already in `desktop-client-v1.md` > (§6.8, §7.5, decisions E8/E12) — read that first. @@ -9,9 +12,10 @@ > **What shipped (branch `win-webrtc-stun`):** platform dirs / signals / perms / > ffmpeg discovery / CLI messages (W1-2-5-6-7-8, commits through `c2620a5`); > event loop stays Proactor (`5098e6c`); JWT clock-skew leeway (`dad2157`); -> daemon lifecycle via a Startup-folder `.vbs` launcher, **not** Task Scheduler -> (`220e6e7` — `schtasks /create /sc ONLOGON` needs elevation, see §5.3). W4 is -> `packaging/win/` + `package.json` `build.win` — see §5.4. +> daemon lifecycle via a Startup-folder `.vbs` launcher **or** an S4U Scheduled +> Task service mode, chosen at install time and switchable afterwards from the +> Node page (§5.3). W4 is `packaging/win/` + `package.json` `build.win` — see +> §5.4, and `docs/windows-build.md` for the step-by-step build guide. --- @@ -82,7 +86,9 @@ is already guarded with `try/except`; SIGINT/SIGTERM is not. **Fix:** on Windows, use `signal.signal(signal.SIGINT, handler)` — works with `ProactorEventLoop`. Or use `SetConsoleCtrlHandler` via ctypes for -`CTRL_C_EVENT` and `CTRL_CLOSE_EVENT`. See §5.2. +`CTRL_C_EVENT` and `CTRL_CLOSE_EVENT`. See §5.2 (SIGINT/SIGTERM, done at the +time of the original audit) and §5.9 (the `CTRL_CLOSE_EVENT` family, done +2026-09-05). ### 2.3 systemd-hardwired daemon lifecycle @@ -269,41 +275,78 @@ SIGHUP is already guarded. No other signal handling in the codebase. **Estimated scope:** ~10 lines changed. -### 5.3 Daemon lifecycle on Windows (W3) - -**Scope:** `daemon.py` CLI + `src/main.js` IPC handlers - -This is the largest work item. Two modes, matching `desktop-client-v1.md` -§7.5: - -#### Per-user mode (default) - -The daemon runs as a regular process. Autostart via: -- **Startup folder shortcut**, or -- **Task Scheduler** logon task (preferred — survives "disable startup - apps" and has retry semantics). - -The Electron client manages this through Task Scheduler COM or `schtasks.exe`: -- `node:start` → `schtasks /create /tn MeshBayNode /tr "meshbay-node run" /sc ONLOGON /rl LIMITED` -- `node:service-stop` → `schtasks /end /tn MeshBayNode` + kill the process -- `node:service-status` → `schtasks /query /tn MeshBayNode` + check if the process is running -- `node:installed` → check if `meshbay-node.exe` exists in known locations - -The Python CLI equivalents: -- `reload` on Windows → find the running daemon process and send a custom - event (named pipe or a reload-flag file the daemon watches), or just - restart -- `restart-daemon` → kill + start -- `reset` → remove scheduled task + delete data dirs - -#### Service mode (optional, elevated) - -A Windows Service under a dedicated low-privilege account. Uses -`pywin32`'s `win32serviceutil` or a wrapper like NSSM. This is **Phase 2** -of the Windows port — per-user mode ships first. - -**Estimated scope:** ~200 lines Python + ~150 lines JS for per-user mode. -Service mode is a separate milestone. +### 5.3 Daemon lifecycle on Windows (W3) — done, both modes + +**Scope:** `daemon.py` CLI + `meshbay_node.platform` + `src/main.js` IPC handlers ++ `node-page.js` + +Two modes ship, matching `desktop-client-v1.md` §7.5, chosen at install time +and **switchable afterwards** (see below — that "afterwards" part was not the +original design and exists because of a real bug): + +#### Per-user mode (default, no admin) + +The daemon runs as a regular process. Autostart is a `.vbs` in the Startup +folder (`meshbay_node.platform._startup_vbs`), toggled from the Node page +(`platform.node.autostart`) or `meshbay-node autostart install|remove`. +**Not** a Task Scheduler `/sc ONLOGON` task as originally planned — that needs +elevation for the same reason a boot trigger does, so it was abandoned for the +same no-admin-by-default reason service mode below needs one prompt. + +#### Service mode (one admin confirmation) + +A Scheduled Task via **S4U** (Service For User) logon — +`schtasks /create ... /sc onstart /ru /rp ""`, no `/it` — not a real +Windows Service (`pywin32`/NSSM), because a Service runs under +LocalSystem/NetworkService, accounts with no normal user profile, so +`%LOCALAPPDATA%\meshbay\` would not exist for it. S4U starts at boot with no +sign-in and no stored password, and — the whole reason S4U and not +LocalSystem — loads the signed-in user's own profile, so nothing in +`platform.py` needed to change to support it. +`packaging/win/service.ps1` + `service-mode.ps1` (one elevation, folds the +firewall rules into the same UAC prompt) + +`meshbay_node.platform.service_install/_remove/_status/_run/_end` + CLI +`meshbay-node service ...`. Start/Stop/Restart on the Node page drive whichever +mode is active (`main.js` `winServiceTaskStatus/Run/End`, checked first in +`node:installed`/`service-status`/`service-stop`/`service-restart`/ +`node:start`). + +**Verified empirically:** a bare `meshbay-node.exe` run with zero config +present — the exact case if the S4U task fires at boot before the user has +ever provisioned anything — exits cleanly in <1s, code 1, "Error: +hub.username not set... Run: meshbay-node init", nothing written to disk. So +an unprovisioned first boot is already safe; the client's +provision-then-start flow (`provisionNode()` writes `node.toml` before +`node:start` ever calls `winServiceTaskRun()`) already sequences correctly for +both modes. + +#### Found 2026-09-05: the installer's mode question is effectively one-shot + +`installer.nsh` decides whether to ask the mode question by checking +**firewall state only** (`firewall.ps1 check`, unelevated) — reasoning +"service-mode.ps1 always sets up the task and firewall together, so rules +present ⇒ a mode was already chosen." False: the per-user branch also sets up +firewall on its own, no Scheduled Task involved. So once firewall is satisfied +by **any** path — including dev/testing calling `firewall.ps1 add` directly — +the mode dialog never shows again, on a fresh install or a repair, since +uninstall defaults to leaving both alone. This is exactly what happened during +this session's own testing: the dialog never appeared on either real install +run, and "no UAC prompt" was reasonably but wrongly read as "already accepted +earlier." **Fixed** by adding the other door in rather than patching the +skip-check: a "run as a background service" checkbox on the Node page +(`node-page.js`, next to autostart), wired `main.js` +(`winElevateServiceMode` — `Start-Process -Verb RunAs` against a temp +`param()`-based `.ps1`, so the target path/args bind through real PowerShell +parameters, not string quoting) → `preload.js` (`serviceMode`) → `platform.js` +(`node.serviceMode.install()/remove()`). It drives the exact same +`service-mode.ps1` the installer runs, so the two paths can never disagree. +`installer.nsh`'s skip-check itself was deliberately left as-is (harmless for +the "asks nothing on reinstall" case it was written for) — the toggle is the +durable fix, the mode is no longer a one-shot decision. + +**Estimated scope (actual):** ~200 lines Python (`platform.py` + CLI) + the +PowerShell scripts + ~150 lines JS across `main.js`/`preload.js`/`platform.js`/ +`node-page.js` for the toggle. ### 5.4 Packaging (W4) — **built 2026-09-04** @@ -340,9 +383,47 @@ because the transcode path needs libx264 and no LGPL-only build has one; rejected as the mechanism: it needs network + winget present at install time and fails silently. `-SkipFfmpeg` opts out for a smaller local-iteration build. +#### Dependency audit 2026-09-05: no VC++ Redistributable needed (static analysis) + +The user asked, in effect, "is the Setup EXE fully self-contained on a clean +machine" — Python, PyInstaller and the VC++ runtime specifically. Checked by +inspecting the actual built `node-runtime/` tree rather than assuming: + +- **The frozen node** (`_internal/`, 92 `.pyd`/`.dll` files across aiortc, av, + aioquic, cryptography, pydantic_core, etc.) imports only + `VCRUNTIME140.dll`/`VCRUNTIME140_1.dll` (bundled by PyInstaller automatically + — both files are present in `_internal/`) and `api-ms-win-crt-*.dll` (the + Universal CRT). `msvcp140.dll` — the actual "Visual C++ Redistributable" + DLL — appears **nowhere** in the dependency graph. The Universal CRT is an + OS component since Windows 10 1607, not a separately-installed package, so + nothing here needs the redistributable installed. +- **ffmpeg** (BtbN gpl-shared, MinGW-built): `avformat`/`avfilter`/`avdevice` + carry `libstdc++`/`libgcc` symbol strings but no separate + `libstdc++-6.dll`/`libgcc_s_seh-1.dll`/`libwinpthread-1.dll` ships or is + needed — BtbN links its GCC runtime statically. `fetch-ffmpeg.ps1`'s own + smoke test (a real libx264 encode + an ffprobe run, immediately after + extraction) already exercises this on every build. +- **The Electron client**: `dependencies` are `bonjour-service` + + `castv2-client`, both pure JS — no native Node addon ships in the app (only + `extract-zip`, a build-time-only tool of electron-builder, has one). Modern + Electron/Chromium needs nothing beyond the Universal CRT either. +- Confirmed independently: nothing in the repo installs or bundles a VC++ + Redistributable (`grep` for "vc_redist"/"redistributable"/"msvcp" across the + whole tree → zero hits) — consistent with it not being needed. + +**Caveat — this is static, not a live test.** No genuine clean Windows 11 VM +run (no dev tools, no pre-existing redistributables) has been done; the build +machine already has everything installed, so a passing smoke test there proves +nothing about a truly bare machine. This narrows, but does not close, the +"clean-machine install check" item below — and a future dependency bump (a new +PyPI package with a C++ extension, a future Electron native module) could +silently reintroduce a need for it with nothing here to catch that ahead of +time. + **Still open:** Authenticode signing (Phase 13.9 — unsigned ⇒ SmartScreen), a Windows CI runner (Phase 18.3), `electron-updater`. First clean-machine -install + `safeStorage`/DPAPI + autostart round-trip is a manual check. +install + `safeStorage`/DPAPI + autostart round-trip is still a manual check +that has not been run. ### 5.5 File permissions (W5) @@ -421,6 +502,58 @@ operators write whichever identifier they can see. A filter that matches nothing falls back to the unfiltered list with a warning: losing the 5 s timeout saving is a regression, being unconnectable is a defect. +### 5.9 CTRL_CLOSE_EVENT / taskkill orphaning ffmpeg (2026-09-05) + +**Scope:** `meshbay_node.platform` + `daemon.py` + `src/main.js` + +Two separate gaps, both flagged in project memory as "not a missing +dependency, but may leave a zombie ffmpeg process": + +**Closing the console.** CPython's own console-control handler claims +`CTRL_C_EVENT`/`CTRL_BREAK_EVENT` (delivered as `SIGINT`/`SIGBREAK`) but +returns "not handled" for `CTRL_CLOSE_EVENT`, `CTRL_LOGOFF_EVENT` and +`CTRL_SHUTDOWN_EVENT` — none of the three has a Python signal. Without a +handler of our own, Windows just ends the process for these — no +`_shutdown()`, no closed WebRTC sessions, no killed ffmpeg. Covers: closing +the console window of an interactively-run `meshbay-node run`, user logoff, +system shutdown. `platform.install_console_close_handler()` registers one via +`ctypes.windll.kernel32.SetConsoleCtrlHandler`, wired into `daemon.py`'s +existing win32 signal block. MSDN's own rule for these three events — +"the process is ended after the handler returns, or after 5 seconds, +whichever occurs first" — means the handler (which runs on a thread Windows +creates, never the main one) must *block* rather than return immediately: it +nudges the asyncio loop the thread-safe way, then waits (capped just under +that ceiling) for a `threading.Event` that `run()` sets right after +`await self._shutdown()` actually finishes. + +**`taskkill` / the Stop button.** `autostart_end()` and Electron's +`killNodeProcesses()` both used `taskkill /IM meshbay-node.exe /F` — a hard +`TerminateProcess`, uncatchable on any OS (like `SIGKILL`), so no handler +above could ever help here regardless. Fixed differently: `autostart_run()` +now spawns with `CREATE_NEW_PROCESS_GROUP` instead of `DETACHED_PROCESS` (the +child keeps no visible window, but does keep a console object of its own and +becomes the root of a signalable process group — `DETACHED_PROCESS` has no +console at all, so nothing could ever be signalled that way), and records its +pid in `state_dir()/node.pid`. `autostart_end()` now tries +`os.kill(pid, signal.CTRL_BREAK_EVENT)` first — which `daemon.py`'s win32 +signal block (now also catching `SIGBREAK`) turns into the same +`stop_event.set()` SIGINT/SIGTERM already use — polls (bounded, 5 s) for exit, +and only falls back to the hard `taskkill` if that pid is stale (reused by an +unrelated process — checked by image name before ever signalling it), already +gone, or does not exit in time. Electron's `killNodeProcesses()` no longer +does its own `taskkill`; it shells out to ` autostart stop` instead, so +the graceful-then-forceful logic lives in exactly one place rather than two +that could silently diverge. + +**Not covered, deliberately:** `taskkill /F` itself, run by a human or another +tool, bypasses all of the above the same way `kill -9` would on Linux — this +narrows how often that path is *taken* (the Node page's own Stop no longer +takes it first), not what happens on the rare occasion something forces it. +Service mode's Scheduled Task stop (`schtasks /end`) was deliberately left +alone — Task Scheduler owns that process's creation flags and its own stop +semantics are a separate, unverified surface, not something `service.ps1` +controls. + --- ## 6. Execution order @@ -434,19 +567,24 @@ W6 ffmpeg discovery ✅ done W8 test suite green on win32 ✅ done (encoding sweep + skipif; 784 pass) W9 ICE interface naming ✅ done (name/description/IP match + fail-open) ──── milestone: daemon runs on Windows ✅ (verified end to end) ──── -W3 Daemon lifecycle ✅ done — Startup-folder .vbs (Task Scheduler needs admin) +W3 Daemon lifecycle ✅ done — Startup-folder .vbs (default) + S4U Scheduled + Task service mode, both switchable post-install from + the Node page ──── milestone: daemon starts/stops on Windows ✅ ──── W4 Packaging ✅ built — one per-user NSIS installer, client + node + Dependency audit ✅ done (2026-09-05, static) — no VC++ Redistributable + needed; see §5.4 + CTRL_CLOSE_EVENT handler ✅ done (2026-09-05) — see §5.9 ──── milestone: installable on Windows — pending a clean-machine run ──── - CTRL_CLOSE_EVENT handler ← open: window-close / bare taskkill skips _shutdown() - Service mode (W3b) ← Phase 2, optional + Clean-machine install ← open: no live test on a genuinely bare Windows 11 VM Authenticode signing ← Phase 13.9 CI Windows matrix ← Phase 18.3 ``` W1–W2–W5–W6–W7–W8 shipped as a run of mechanical commits, testable on Linux. W9 came out of a live guest that could not connect at all. -W3 and W4 were the real work. +W3 and W4 were the real work; the service-mode toggle (§5.3) followed from a +real bug found while testing W4, not from the original plan. --- @@ -494,17 +632,18 @@ Add a Windows runner to GitHub Actions: | Dependency | Linux | Windows | |---|---|---| -| Python 3.12+ | system / venv | `python-embed` zip or full install | -| ffmpeg / ffprobe | system package | `winget install ffmpeg` or bundled | -| Node.js 22+ | `/opt/nodejs` | installer from nodejs.org | +| Python 3.12+ | system / venv | build machine only — end users get a frozen `.exe`, no install needed | +| ffmpeg / ffprobe | system package | bundled in the installer by default (§5.4) | +| Node.js 22+ | `/opt/nodejs` | installer from nodejs.org, build machine only | | Electron 42+ | npm | npm (same) | | aiortc | pip | pip (same — pure Python) | | watchdog | pip | pip (same — uses `ReadDirectoryChangesW`) | | Argon2 WASM | vendored in `static/vendor/` | same | | safeStorage backend | libsecret / kwallet | DPAPI (built into Windows) | +| VC++ Redistributable | n/a | **not needed** — confirmed by static dependency audit, §5.4 | -No new Python dependency is needed for per-user mode. Service mode (W3b) -would need `pywin32` or NSSM. +No new Python dependency was needed for either autostart mode. Service mode +uses a Scheduled Task (S4U logon), not `pywin32`/NSSM — see §5.3. --- @@ -517,5 +656,3 @@ would need `pywin32` or NSSM. ffmpeg availability would need checking. - **Windows Store / MSIX** — not planned for v1. NSIS per-user installer is the target. -- **Service mode** — Phase 2 of the Windows port, after per-user mode is - validated. -- cgit v1.2.3