From 00acbc619605953c92c0bc92ecc322a62c15615f Mon Sep 17 00:00:00 2001 From: Edgar Twigg Date: Fri, 29 May 2026 10:05:08 -0700 Subject: [PATCH 1/2] Kill sidecar before Windows update install to fix conpty.node lock The NSIS auto-update installer failed with "Error opening file for writing: ...\_up_\sidecar\node_modules\node-pty\prebuilds\win32-x64\ conpty.node". Windows refuses to overwrite a native module that a live process still has loaded, and the Node sidecar was still running when NSIS reached node_modules. The RunEvent::Exit kill was too late and async, racing NSIS file copies after install() force-kills the app. The updater close handler now invokes kill_sidecar_now and awaits it before install() on Windows. That command is synchronous on the Rust side: it sends the kill, then polls try_wait (capped ~5s) until the process has actually exited and released its handles. try_wait avoids the job-object wait()'s completion-port double-consume hang. macOS/Linux can replace open files in place, so they skip this. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/specs/auto-update.md | 12 ++++++++--- standalone/src-tauri/src/lib.rs | 37 ++++++++++++++++++++++++++++++++- standalone/src/updater.test.ts | 34 ++++++++++++++++++++++++++++++ standalone/src/updater.ts | 9 ++++++++ 4 files changed, 88 insertions(+), 4 deletions(-) diff --git a/docs/specs/auto-update.md b/docs/specs/auto-update.md index ce095fa1..0c8e01ec 100644 --- a/docs/specs/auto-update.md +++ b/docs/specs/auto-update.md @@ -34,7 +34,13 @@ app launch └─ install fails → overwrite with failure marker → exit normally ``` -The `Update` object returned by `check()` is held in memory as an available update. Clicking the approval action calls `download()` and promotes it to a pending update only after the download succeeds. The close handler intercepts the window close event only when there is an approved, downloaded update, writes a success marker to `localStorage` *before* calling `install()` (because on Windows, NSIS force-kills the process), then calls `install()`. In Vite dev mode (`pnpm dev:standalone`), the close handler skips `install()` without preventing the close. Dev mode is useful for testing check/download/banner behavior, but install must be tested from a packaged app because the updater resolves its replacement target from the current executable path. +The `Update` object returned by `check()` is held in memory as an available update. Clicking the approval action calls `download()` and promotes it to a pending update only after the download succeeds. The close handler intercepts the window close event only when there is an approved, downloaded update, writes a success marker to `localStorage` *before* calling `install()` (because on Windows, NSIS force-kills the process), then — on Windows only — kills the sidecar and waits for it to fully exit (see *Sidecar teardown on Windows* below) before calling `install()`. In Vite dev mode (`pnpm dev:standalone`), the close handler skips `install()` without preventing the close. Dev mode is useful for testing check/download/banner behavior, but install must be tested from a packaged app because the updater resolves its replacement target from the current executable path. + +## Sidecar teardown on Windows + +The NSIS installer overwrites files inside the bundled sidecar — including node-pty's native `conpty.node`. Windows refuses to overwrite a native module that a live process still has loaded, so if the Node sidecar is running when NSIS reaches `node_modules`, the install fails with *"Error opening file for writing: …\_up_\sidecar\node_modules\node-pty\prebuilds\win32-x64\conpty.node"*. The Rust `RunEvent::Exit` kill is too late and asynchronous — NSIS starts copying files immediately after `install()` force-kills the app, racing the sidecar's shutdown. + +So on Windows the close handler `invoke`s `kill_sidecar_now` and awaits it before `install()`. That command is synchronous on the Rust side: it sends the kill, then polls `try_wait` (capped at ~5s) until the process has actually exited and released its file handles. `try_wait` is used instead of the job-object `wait()` because `wait()` consumes a completion-port message the reaper thread relies on and could block forever if the sidecar had already exited. macOS and Linux can replace open files in place, so they skip this and rely on the existing `RunEvent::Exit` cleanup. ## Update notice in the Baseboard @@ -63,7 +69,7 @@ The Baseboard is in `lib/` but the updater is standalone-only. The notice is thr | Platform | What `install()` does | App exit | |----------|----------------------|----------| -| Windows | Launches NSIS installer in passive mode (progress bar, no user interaction). Force-kills the app. | Automatic (NSIS) | +| Windows | Kills the sidecar and waits for it to exit (so NSIS can overwrite its loaded native modules), then launches NSIS installer in passive mode (progress bar, no user interaction). Force-kills the app. | Automatic (NSIS) | | macOS | Replaces the `.app` bundle in place | `getCurrentWindow().close()` after `install()` returns | | Linux | Replaces the AppImage in place | `getCurrentWindow().close()` after `install()` returns | | Vite dev mode | Skips `install()` to avoid replacing the dev executable directory | Native close proceeds normally | @@ -125,6 +131,6 @@ The Rust side registers the plugin with `tauri_plugin_updater::Builder::new().bu **Why write the success marker before `install()`?** On Windows, the NSIS installer force-kills the process — code after `install()` may never run. Writing optimistically and overwriting on failure handles both platforms correctly. -**Why no `on_before_exit` Rust hook?** The JS close handler (`onCloseRequested`) runs before `install()` and handles marker writes. On Windows, NSIS handles process termination after `install()`. Sidecar cleanup is not currently handled at update-time — the sidecar process is orphaned and will exit when its stdin closes. +**Why no `on_before_exit` Rust hook?** The JS close handler (`onCloseRequested`) runs before `install()` and handles marker writes and (on Windows) the synchronous sidecar kill. On Windows, NSIS handles process termination after `install()`. On macOS/Linux the sidecar is orphaned and exits when its stdin closes — harmless there because open files can be replaced in place. **Why `localStorage` instead of Tauri's store plugin?** `localStorage` persists across launches in Tauri's webview, requires no additional dependencies, and is automatically scoped to the app. If the user resets app data, markers are cleaned up naturally. diff --git a/standalone/src-tauri/src/lib.rs b/standalone/src-tauri/src/lib.rs index 210a4156..9414dfd9 100644 --- a/standalone/src-tauri/src/lib.rs +++ b/standalone/src-tauri/src/lib.rs @@ -308,7 +308,7 @@ fn read_update_log() -> Result { #[tauri::command] fn kill_sidecar_now(state: tauri::State<'_, SidecarState>) { - kill_sidecar(&state.child); + kill_sidecar_and_wait(&state.child); } // Job Object on Windows / process group on Unix — kill propagates to the @@ -322,6 +322,41 @@ fn kill_sidecar(child: &SharedChild) { } } +// Like `kill_sidecar`, but blocks until the process has actually exited. The +// updater calls this before launching the Windows NSIS installer: NSIS +// overwrites files inside the bundled sidecar (e.g. node-pty's `conpty.node`), +// and Windows refuses to overwrite a native module the live sidecar still has +// loaded — surfacing as "Error opening file for writing". Releasing those +// handles first requires the node process to be gone, not merely signalled. +// +// We poll `try_wait` rather than block on `wait()`: `try_wait` is idempotent +// and can't hang, whereas the job-object `wait()` consumes a completion-port +// message the reaper thread may already have drained (e.g. if the sidecar had +// crashed earlier), which would block forever. The ~5s cap means a wedged +// sidecar can't stall quit indefinitely. +fn kill_sidecar_and_wait(child: &SharedChild) { + let Ok(mut guard) = child.lock() else { return }; + append_log(format!( + "[sidecar] killing and waiting for exit (pid={})", + guard.id() + )); + let _ = guard.start_kill(); + for _ in 0..250 { + match guard.try_wait() { + Ok(Some(status)) => { + append_log(format!("[sidecar] confirmed exit during kill (status: {status})")); + return; + } + Ok(None) => std::thread::sleep(Duration::from_millis(20)), + Err(err) => { + append_log(format!("[sidecar] wait error during kill: {err}")); + return; + } + } + } + append_log("[sidecar] kill wait timed out (~5s); proceeding anyway"); +} + #[derive(Serialize, Deserialize, Clone)] struct ShellInfo { name: String, diff --git a/standalone/src/updater.test.ts b/standalone/src/updater.test.ts index 1d3a81bb..07a05447 100644 --- a/standalone/src/updater.test.ts +++ b/standalone/src/updater.test.ts @@ -34,6 +34,12 @@ vi.mock('@tauri-apps/api/core', () => ({ invoke: mocks.invoke, })); +// Force the Windows code path so the sidecar-kill-before-install branch is +// exercised. updater.ts only consumes PLATFORM_STRING from this module. +vi.mock('dormouse-lib/lib/platform', () => ({ + PLATFORM_STRING: 'Windows', +})); + // --- Helpers --- const STORAGE_KEY = 'dormouse:update-result'; @@ -207,6 +213,34 @@ describe('updater', () => { expect(mocks.windowClose).toHaveBeenCalled(); }); + it('kills the sidecar and waits for it before installing on Windows', async () => { + const update = makeUpdate('0.5.0'); + mocks.check.mockResolvedValue(update); + + startUpdateCheck(); + await vi.advanceTimersByTimeAsync(5_000); + await vi.advanceTimersByTimeAsync(0); + approveUpdate(); + await vi.advanceTimersByTimeAsync(0); + + const order: string[] = []; + mocks.invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'kill_sidecar_now') order.push('kill'); + return ''; + }); + update.install.mockImplementation(async () => { + order.push('install'); + }); + + const closeHandler = mocks.onCloseRequested.mock.calls[0][0]; + await closeHandler({ preventDefault: vi.fn() }); + + expect(mocks.invoke).toHaveBeenCalledWith('kill_sidecar_now'); + // The kill must complete before NSIS runs, or it can't overwrite the + // sidecar's still-loaded native modules. + expect(order).toEqual(['kill', 'install']); + }); + it('writes failure marker when install throws', async () => { const update = makeUpdate('0.5.0'); update.install.mockRejectedValue(new Error('install failed')); diff --git a/standalone/src/updater.ts b/standalone/src/updater.ts index aaef0a15..c5ce7f13 100644 --- a/standalone/src/updater.ts +++ b/standalone/src/updater.ts @@ -231,6 +231,15 @@ function registerCloseHandler(): void { from: currentVersion, to: update.version, })); + // On Windows the NSIS installer overwrites files inside the bundled + // sidecar (e.g. node-pty's conpty.node). Windows refuses to overwrite a + // native module the running sidecar still has loaded, which surfaces as + // "Error opening file for writing". Kill the sidecar and wait for it to + // fully exit before launching the installer. (On macOS/Linux open files + // can be replaced in place, so this is Windows-only.) + if (/Win/i.test(PLATFORM_STRING)) { + await invoke('kill_sidecar_now'); + } await update.install(); } catch (e) { // Overwrite with failure marker From 00bfe6e940cf8eb8f3e76e1456974d76a06329d8 Mon Sep 17 00:00:00 2001 From: Edgar Twigg Date: Fri, 29 May 2026 10:24:32 -0700 Subject: [PATCH 2/2] Reuse shared IS_WINDOWS and clarify kill-wait loop constants Cleanup pass over the conpty.node fix: - Promote IS_WINDOWS to lib/platform (next to IS_MAC) and reuse it in updater.ts instead of an ad-hoc /Win/i regex; dedupe the duplicate copy that shell-escape.ts carried. - Name the kill_sidecar_and_wait loop constants (POLL_INTERVAL, MAX_POLLS) so the "~5s" cap is self-documenting. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/src/lib/platform/index.ts | 3 +++ lib/src/lib/shell-escape.test.ts | 2 +- lib/src/lib/shell-escape.ts | 9 +-------- standalone/src-tauri/src/lib.rs | 8 ++++++-- standalone/src/updater.test.ts | 1 + standalone/src/updater.ts | 4 ++-- 6 files changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/src/lib/platform/index.ts b/lib/src/lib/platform/index.ts index 3b0a544b..e9c62b93 100644 --- a/lib/src/lib/platform/index.ts +++ b/lib/src/lib/platform/index.ts @@ -34,6 +34,9 @@ export const PLATFORM_STRING: string = (() => { */ export const IS_MAC: boolean = /Mac|iPhone|iPad/i.test(PLATFORM_STRING); +/** True when running on Windows. */ +export const IS_WINDOWS: boolean = /win/i.test(PLATFORM_STRING); + let adapter: PlatformAdapter | null = null; /** Set an externally-created platform adapter (e.g. TauriAdapter from standalone). */ diff --git a/lib/src/lib/shell-escape.test.ts b/lib/src/lib/shell-escape.test.ts index 77e17c71..85cdcfd2 100644 --- a/lib/src/lib/shell-escape.test.ts +++ b/lib/src/lib/shell-escape.test.ts @@ -101,7 +101,7 @@ describe('shellEscapePath OS dispatch', () => { }); async function importShellEscape(opts: { isMac: boolean; platform: string }) { - vi.doMock('./platform', () => ({ IS_MAC: opts.isMac })); + vi.doMock('./platform', () => ({ IS_MAC: opts.isMac, IS_WINDOWS: /win/i.test(opts.platform) })); Object.defineProperty(globalThis, 'navigator', { value: { platform: opts.platform, userAgent: opts.platform }, configurable: true, diff --git a/lib/src/lib/shell-escape.ts b/lib/src/lib/shell-escape.ts index b2c5e3db..38455831 100644 --- a/lib/src/lib/shell-escape.ts +++ b/lib/src/lib/shell-escape.ts @@ -1,11 +1,4 @@ -import { IS_MAC } from './platform'; - -const IS_WINDOWS: boolean = (() => { - if (typeof navigator === 'undefined') return false; - const nav = navigator as Navigator & { userAgentData?: { platform?: string } }; - const p = (nav.userAgentData?.platform ?? nav.platform ?? nav.userAgent ?? '').toLowerCase(); - return p.includes('win'); -})(); +import { IS_MAC, IS_WINDOWS } from './platform'; // Matches macOS Terminal's drag-and-drop format: backslash-escape each shell // metacharacter instead of wrapping in quotes. TUIs like `claude` recognize diff --git a/standalone/src-tauri/src/lib.rs b/standalone/src-tauri/src/lib.rs index 9414dfd9..2914cc38 100644 --- a/standalone/src-tauri/src/lib.rs +++ b/standalone/src-tauri/src/lib.rs @@ -335,19 +335,23 @@ fn kill_sidecar(child: &SharedChild) { // crashed earlier), which would block forever. The ~5s cap means a wedged // sidecar can't stall quit indefinitely. fn kill_sidecar_and_wait(child: &SharedChild) { + // Poll for exit at this cadence, up to ~5s total (MAX_POLLS × POLL_INTERVAL). + const POLL_INTERVAL: Duration = Duration::from_millis(20); + const MAX_POLLS: u32 = 250; + let Ok(mut guard) = child.lock() else { return }; append_log(format!( "[sidecar] killing and waiting for exit (pid={})", guard.id() )); let _ = guard.start_kill(); - for _ in 0..250 { + for _ in 0..MAX_POLLS { match guard.try_wait() { Ok(Some(status)) => { append_log(format!("[sidecar] confirmed exit during kill (status: {status})")); return; } - Ok(None) => std::thread::sleep(Duration::from_millis(20)), + Ok(None) => std::thread::sleep(POLL_INTERVAL), Err(err) => { append_log(format!("[sidecar] wait error during kill: {err}")); return; diff --git a/standalone/src/updater.test.ts b/standalone/src/updater.test.ts index 07a05447..ea4ae09e 100644 --- a/standalone/src/updater.test.ts +++ b/standalone/src/updater.test.ts @@ -38,6 +38,7 @@ vi.mock('@tauri-apps/api/core', () => ({ // exercised. updater.ts only consumes PLATFORM_STRING from this module. vi.mock('dormouse-lib/lib/platform', () => ({ PLATFORM_STRING: 'Windows', + IS_WINDOWS: true, })); // --- Helpers --- diff --git a/standalone/src/updater.ts b/standalone/src/updater.ts index c5ce7f13..a42f5a5c 100644 --- a/standalone/src/updater.ts +++ b/standalone/src/updater.ts @@ -4,7 +4,7 @@ import { getCurrentWindow } from '@tauri-apps/api/window'; import { getVersion } from '@tauri-apps/api/app'; import { open } from '@tauri-apps/plugin-shell'; import { invoke } from '@tauri-apps/api/core'; -import { PLATFORM_STRING } from 'dormouse-lib/lib/platform'; +import { IS_WINDOWS, PLATFORM_STRING } from 'dormouse-lib/lib/platform'; import type { UpdateBannerState } from './UpdateBanner'; const GITHUB_REPO_URL = 'https://github.com/diffplug/dormouse'; @@ -237,7 +237,7 @@ function registerCloseHandler(): void { // "Error opening file for writing". Kill the sidecar and wait for it to // fully exit before launching the installer. (On macOS/Linux open files // can be replaced in place, so this is Windows-only.) - if (/Win/i.test(PLATFORM_STRING)) { + if (IS_WINDOWS) { await invoke('kill_sidecar_now'); } await update.install();