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 --- packages/meshbay-client/src/main.js | 52 ++++++++++++++++++++++++++++++++----- 1 file changed, 46 insertions(+), 6 deletions(-) (limited to 'packages/meshbay-client/src') diff --git a/packages/meshbay-client/src/main.js b/packages/meshbay-client/src/main.js index dd7f38e..e996829 100644 --- a/packages/meshbay-client/src/main.js +++ b/packages/meshbay-client/src/main.js @@ -813,7 +813,16 @@ function registerBridge() { const cmd = process.platform === 'win32' ? 'where.exe' : 'which'; return new Promise((resolve) => { execFile(cmd, ['meshbay-node'], (err, stdout) => { - resolve(err ? null : stdout.trim().split('\n')[0]); + if (err) { resolve(null); return; } + // where.exe/which can list more than one match on PATH, and each + // line keeps its own trailing \r on Windows -- `stdout.trim()` only + // strips the ends of the *whole* string, so with 2+ matches a stray + // \r stayed glued to the end of the first line. That \r then landed + // inside the quoted path this function's caller writes into the + // Startup .vbs, breaking VBScript's parser with "Unterminated + // string constant" the next time Windows tried to run it at sign-in. + const first = stdout.split(/\r?\n/).map((s) => s.trim()).find(Boolean); + resolve(first || null); }); }); } @@ -844,9 +853,24 @@ function registerBridge() { try { fs.rmSync(WIN_STARTUP_VBS, { force: true }); } catch { /* not there */ } } - function killNodeProcesses() { + // Prefers a graceful stop: `autostart stop` now tries CTRL_BREAK_EVENT + // against the pid autostart_run() recorded first (meshbay_node.platform. + // autostart_end()), which daemon.py's SIGBREAK handler turns into a real + // _shutdown() -- closed WebRTC sessions, killed ffmpeg -- before that same + // function falls back to a hard `taskkill /F` itself. Keeping the + // graceful-then-forceful logic in that one place, rather than this + // function *also* going straight to taskkill, is what actually fixed it: + // two independent hard-kill call sites would still bypass shutdown one of + // the times. Only genuinely falls back to taskkill here when the binary + // cannot even be located. + async function killNodeProcesses() { + const bin = await findNodeBinary(); return new Promise((resolve) => { - execFile('taskkill', ['/IM', 'meshbay-node.exe', '/F'], () => resolve()); + if (bin) { + execFile(bin, ['autostart', 'stop'], () => resolve()); + } else { + execFile('taskkill', ['/IM', 'meshbay-node.exe', '/F'], () => resolve()); + } }); } @@ -897,7 +921,16 @@ function registerBridge() { return new Promise((resolve, reject) => { const script = path.join(process.resourcesPath, 'service-mode.ps1'); if (!fs.existsSync(script)) { - reject(new Error('service-mode.ps1 not found — only available in an installed build')); + // service-mode.ps1 is an extraResource -- only present once installed + // (package.json build.win.extraResources); nothing under `npm start`. + // The Node page already disables the "background service" option + // when node:service-status reports canElevate: false, so this should + // only ever be reached if that guard is bypassed somehow -- keep the + // message actionable regardless. + reject(new Error( + 'Switching to a background service needs an installed build. For ' + + 'local testing, run "meshbay-node service install" from an ' + + 'elevated PowerShell instead.')); return; } // Start-Process -Verb RunAs is the one UAC prompt; -Wait -PassThru hands @@ -992,6 +1025,12 @@ function registerBridge() { installed: true, activeState: running ? 'active' : 'inactive', subState: svc.state, + // Whether switching startup mode can actually elevate right now — + // service-mode.ps1 is an extraResource, only present in a packaged + // build. Already installed here, so removing it always works + // regardless; this only gates the Node page offering to switch + // *into* service mode. + canElevate: app.isPackaged, }; } // Per-user Startup mode. `installed` used to be winAutostartInstalled(), @@ -1008,6 +1047,7 @@ function registerBridge() { autostart: winAutostartInstalled(), activeState: p ? 'active' : 'inactive', subState: p ? 'running' : '', + canElevate: app.isPackaged, }; } if (process.platform !== 'linux') return { supported: false }; @@ -1038,8 +1078,8 @@ function registerBridge() { if (process.platform === 'win32') { const svc = await winServiceTaskStatus(); if (svc.installed) await winServiceTaskEnd(); - await killNodeProcesses(); // hard kill — no CTRL_CLOSE handler yet; - // also the belt-and-suspenders in case /end left the process running + await killNodeProcesses(); // graceful-then-forceful; also the + // belt-and-suspenders in case /end left the process running return { stopped: true }; } if (process.platform !== 'linux') { -- cgit v1.2.3