diff --git a/apps/desktop/scripts/before-pack.mjs b/apps/desktop/scripts/before-pack.mjs index 20045cf728..c95853bd87 100644 --- a/apps/desktop/scripts/before-pack.mjs +++ b/apps/desktop/scripts/before-pack.mjs @@ -60,7 +60,7 @@ import { existsSync, rmSync, renameSync } from 'node:fs' import path from 'node:path' import { Arch } from 'electron-builder' -import { stageNodePty, stageGetWindows } from './stage-native-deps.mjs' +import { removeDirSync, stageNodePty, stageGetWindows } from './stage-native-deps.mjs' export function cleanStaleAppOutDir(appOutDir) { if (!appOutDir || typeof appOutDir !== 'string') { @@ -73,6 +73,13 @@ export function cleanStaleAppOutDir(appOutDir) { // can't block the wipe. retry/maxRetries rides out transient EBUSY on // Windows where an AV/indexer may briefly hold a handle. rmSync(appOutDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }) + // Node's native rmSync silently deletes nothing on non-ASCII Windows + // paths (nodejs/node#56049, fixed in v24.13.1) — without this check the + // stale tree survives and the "removed" log below lies. Fall back to + // the libuv-backed walk, which handles those paths on every version. + if (existsSync(appOutDir)) { + removeDirSync(appOutDir) + } return true } diff --git a/apps/desktop/scripts/stage-native-deps.mjs b/apps/desktop/scripts/stage-native-deps.mjs index a0ad407478..5f9a62e7cc 100644 --- a/apps/desktop/scripts/stage-native-deps.mjs +++ b/apps/desktop/scripts/stage-native-deps.mjs @@ -16,6 +16,7 @@ import { chmodSync, copyFileSync, existsSync, + lstatSync, mkdirSync, readdirSync, readFileSync, @@ -36,14 +37,17 @@ function makeExecutable(filePath) { // ─── libuv-safe fs primitives ──────────────────────────────────────── // -// Node 24's native rewrite of fs.cpSync/fs.rmSync mishandles non-ASCII -// Windows paths (observed on v24.11.1 with an accented Windows user name, -// i.e. a default %LOCALAPPDATA%\hermes home): a recursive cpSync fails -// with EIO "Access is denied" or hard-crashes the process, an overwriting -// cpSync fails with a bogus errno-0 unlink error, and rmSync silently -// deletes nothing — leaving a half-staged tree that breaks every retry. -// copyFileSync/unlinkSync/rmdirSync/readdirSync go through libuv and -// handle those paths correctly, so staging uses them exclusively. +// Node's native (non-libuv) rewrite of fs.cpSync/fs.rmSync mishandles +// non-ASCII Windows paths (observed on v24.11.1 with an accented Windows +// user name, i.e. a default %LOCALAPPDATA%\hermes home): a recursive +// cpSync fails with EIO "Access is denied" or hard-crashes the process, +// an overwriting cpSync fails with a bogus errno-0 unlink error, and +// rmSync silently deletes nothing — leaving a half-staged tree that +// breaks every retry. Fixed upstream (nodejs/node#61878 → v24.15.0; +// nodejs/node#56049 → v24.13.1), but the installer builds on whatever +// Node the user already has, so staging sticks to libuv-backed +// primitives (copyFileSync/unlinkSync/rmdirSync/readdirSync), which +// handle those paths correctly on every affected version. /** Recursively copy a directory without fs.cpSync. */ function copyDirSync(srcDir, destDir) { @@ -60,12 +64,25 @@ function copyDirSync(srcDir, destDir) { } /** - * Recursively delete a directory without fs.rmSync (missing dir is fine), - * then verify the tree is actually gone — a silent no-op here surfaces - * later as an inexplicable staging failure, so fail loudly instead. + * Recursively delete a path without fs.rmSync — missing paths are fine, + * a plain file or symlink at the path is unlinked (rm -rf semantics). + * Verifies the tree is actually gone afterwards: a silent no-op here + * surfaces later as an inexplicable staging failure, so fail loudly. + * + * Also used by before-pack.mjs as the fallback when the native rmSync + * silently leaves the stale unpacked dir behind. */ -function removeDirSync(dir) { - if (!existsSync(dir)) return +export function removeDirSync(dir) { + let stats + try { + stats = lstatSync(dir) + } catch { + return + } + if (!stats.isDirectory()) { + unlinkSync(dir) + return + } for (const entry of readdirSync(dir, { withFileTypes: true })) { const full = join(dir, entry.name) if (entry.isDirectory()) { diff --git a/apps/desktop/scripts/stage-native-deps.test.mjs b/apps/desktop/scripts/stage-native-deps.test.mjs index 4e74bccc5e..907219f057 100644 --- a/apps/desktop/scripts/stage-native-deps.test.mjs +++ b/apps/desktop/scripts/stage-native-deps.test.mjs @@ -348,6 +348,19 @@ test.skipIf(process.platform === 'win32')( // This test stages into an accented src/dest tree — twice, so the // restage exercises the delete-then-recopy path — to keep it that way. +test('non-ASCII paths: staging never calls fs.cpSync/fs.rmSync', () => { + // The Node bug is Windows-only while CI runs on Linux, so the accented + // staging test below cannot catch a reintroduction there. Guard at the + // source level instead: no cpSync/rmSync call may come back into the + // staging path. + const source = fs.readFileSync(new URL('./stage-native-deps.mjs', import.meta.url), 'utf8') + assert.doesNotMatch( + source, + /(?:cpSync|rmSync)\(/, + 'stage-native-deps.mjs must stay on libuv-backed fs primitives (see the banner comment)' + ) +}) + test('non-ASCII paths: staging into an accented tree works and restages cleanly', () => { const tmp = fs.mkdtempSync(join(os.tmpdir(), 'hermes-stage-')) try {