diff --git a/.changeset/silly-sloths-dart.md b/.changeset/silly-sloths-dart.md new file mode 100644 index 000000000..8e8336d81 --- /dev/null +++ b/.changeset/silly-sloths-dart.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2375 +--- +**Managed hooks no longer break after a volta node upgrade or prune** — on machines using volta to manage Node, the installer baked a version-pinned node path into every managed hook command. Once volta pruned that node version, every hook failed to spawn with `No such file or directory` at the start of each session, until the installer was re-run. Hook commands now resolve through volta's stable shim, which survives version changes. (#2335) diff --git a/scripts/release-tarball-smoke.cjs b/scripts/release-tarball-smoke.cjs index 5f6218933..3dae43846 100644 --- a/scripts/release-tarball-smoke.cjs +++ b/scripts/release-tarball-smoke.cjs @@ -47,16 +47,23 @@ const os = require('os'); const path = require('path'); const { PACKAGE_NAME } = require('../gsd-core/bin/lib/package-identity.cjs'); const { ExitError, runMain } = require('./lib/cli-exit.cjs'); -// 120 s proved too tight on Windows GitHub-hosted runners: cold-cache -// `npm install -g` with a 1499-file tarball took ~120 s exactly, causing -// spawnSync to fire SIGTERM and return { status: null, stdout: '', stderr: '' } -// (Node docs: status is null when subprocess terminated due to a signal). -// The INSTALL_FAILED branch checks `status !== 0`, which null satisfies, so the -// test saw empty stdout/stderr and a spurious INSTALL_FAILED. Windows runners -// are slower than Linux/macOS for filesystem-heavy operations ( -// https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories -// ). Raise to 600 s (the same ceiling the before() helper uses for pack+install). -const CHILD_TIMEOUT_MS = process.platform === 'win32' ? 600_000 : 120_000; +// 120 s proved too tight for cold-cache `npm install -g` of a 1499-file tarball: +// spawnSync fires SIGTERM at the deadline and returns { status: null, stdout: '', +// stderr: '' } (Node docs: status is null when a subprocess is terminated by a +// signal). The INSTALL_FAILED branch checks `status !== 0`, which null satisfies, +// so the test sees empty stdout/stderr and a spurious INSTALL_FAILED (surfaced as +// `installError: spawnSync npm ETIMEDOUT`). +// +// First observed on Windows GitHub-hosted runners, which are slower for +// filesystem-heavy work +// (https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories), +// then again on a slow Linux bench (cartographer: cold disk, constrained CPU) — +// the same failure the before() helper's SLOW_HOST_TIMEOUT already guards for +// pack+install. The slow-host reality is not platform-specific, so the ceiling is +// now uniform 600 s across all platforms AND shared with before() via this +// exported constant, so the two surfaces cannot diverge again. 600 s stays well +// clear of a real cold install (3–6 min) without masking a genuine hang. +const CHILD_TIMEOUT_MS = 600_000; const QUIET_NPM_ENV = Object.freeze({ npm_config_loglevel: 'error', npm_config_update_notifier: 'false', @@ -628,7 +635,7 @@ function cleanup(...dirs) { // Exports // --------------------------------------------------------------------------- -module.exports = { SMOKE, runSmoke, binInvocation }; +module.exports = { SMOKE, runSmoke, binInvocation, CHILD_TIMEOUT_MS }; if (require.main === module) { runMain(cliMain); diff --git a/src/runtime-hooks-surface.cts b/src/runtime-hooks-surface.cts index aa6ee60c1..20989a7a4 100644 --- a/src/runtime-hooks-surface.cts +++ b/src/runtime-hooks-surface.cts @@ -356,6 +356,24 @@ function normalizeNodePath(execPath: string, opts?: NodeNormOpts): string { const shim = `${miseMatch[1]}/shims/node${miseMatch[2] || ''}`; if (existsSync(shim)) return shim; } + + // volta pins a concrete node image at /tools/image/node//bin/node + // (Windows: /tools/image/node//node.exe — volta's own layout + // puts node.exe at the image root, no bin/). `volta uninstall node@` prunes + // that image, so a baked hook command 404s — the same ephemeral-path failure + // #977 fixed for fnm and #1619 for mise. The stable alias is the shim + // /bin/node, a symlink to volta-shim that always resolves to the + // active pin. Derive from execPath rather than the env so a custom + // VOLTA_HOME and the Windows %LOCALAPPDATA%\Volta default both work (#2185's + // reasoning), and only rewrite when the shim exists — otherwise fall back to + // the raw execPath unchanged. + const voltaMatch = normalizedForMatch.match( + /^(.*)\/tools\/image\/node\/[^/]+\/(?:bin\/)?node(\.exe)?$/, + ); + if (voltaMatch) { + const shim = `${voltaMatch[1]}/bin/node${voltaMatch[2] || ''}`; + if (existsSync(shim)) return shim; + } return execPath; } diff --git a/tests/install.test.cjs b/tests/install.test.cjs index 408a9573c..d009dd0ae 100644 --- a/tests/install.test.cjs +++ b/tests/install.test.cjs @@ -1848,6 +1848,83 @@ describe('normalizeNodePath — mise versioned path → sibling shim (#1619)', ( }); }); +// ─── normalizeNodePath — volta versioned image path → stable shim (#2335) ──── +// +// Bug #2335: the volta analog of #977 (fnm) / #1619 (mise) / #2185 (Homebrew). +// `resolveNodeRunner()` bakes process.execPath into managed hook commands, and +// node realpaths execPath, so under volta it resolves to the concrete image +// `/tools/image/node//bin/node` (Windows: `<...>//node.exe` +// — volta's own layout puts node.exe at the image root, no bin/). `volta +// uninstall node@` prunes that image, after which every managed hook 404s. +// The stable alias is the shim `/bin/node`, a symlink to volta-shim +// that always resolves to the active pin. is derived from the path +// (not env) so a custom VOLTA_HOME and the Windows %LOCALAPPDATA%\Volta default +// both work — same reasoning as the Homebrew branch (#2185). Rewrite only when +// the shim exists; otherwise fall back to raw execPath, like the mise branch. +describe('normalizeNodePath — volta image path → sibling shim (#2335)', () => { + const VOLTA_HOME = '/Users/u/.volta'; + const VOLTA_NODE_PINNED = `${VOLTA_HOME}/tools/image/node/18.17.1/bin/node`; + const VOLTA_SHIM = `${VOLTA_HOME}/bin/node`; + const VOLTA_WIN_HOME = 'C:/Users/u/AppData/Local/Volta'; + const VOLTA_WIN_NODE = `${VOLTA_WIN_HOME}/tools/image/node/22.1.0/node.exe`; // no bin/ on Windows + const VOLTA_WIN_SHIM = `${VOLTA_WIN_HOME}/bin/node.exe`; + const VOLTA_CUSTOM_HOME = '/opt/volta-home'; + const VOLTA_CUSTOM_NODE = `${VOLTA_CUSTOM_HOME}/tools/image/node/20.0.0/bin/node`; + const VOLTA_CUSTOM_SHIM = `${VOLTA_CUSTOM_HOME}/bin/node`; + + test('POSIX pinned image path + shim exists → sibling shim', () => { + assert.equal( + normalizeNodePath(VOLTA_NODE_PINNED, { existsSync: p => p === VOLTA_SHIM }), + VOLTA_SHIM); + }); + + test('Windows node.exe + shim exists → bin/node.exe (.exe preserved)', () => { + assert.equal( + normalizeNodePath(VOLTA_WIN_NODE, { existsSync: p => p === VOLTA_WIN_SHIM }), + VOLTA_WIN_SHIM); + }); + + test('backslash Windows path normalizes the same as forward-slash', () => { + assert.equal( + normalizeNodePath(VOLTA_WIN_NODE.replace(/\//g, '\\'), + { existsSync: p => p === VOLTA_WIN_SHIM }), + VOLTA_WIN_SHIM); + }); + + test('custom VOLTA_HOME layout → shim derived from execPath, not env', () => { + assert.equal( + normalizeNodePath(VOLTA_CUSTOM_NODE, { existsSync: p => p === VOLTA_CUSTOM_SHIM }), + VOLTA_CUSTOM_SHIM); + }); + + test('no regression: shim absent → falls back to raw execPath unchanged', () => { + assert.equal( + normalizeNodePath(VOLTA_NODE_PINNED, { existsSync: () => false }), + VOLTA_NODE_PINNED); + }); + + test('volta shim itself is already stable → left unchanged (idempotent)', () => { + assert.equal( + normalizeNodePath(VOLTA_SHIM, { existsSync: () => true }), + VOLTA_SHIM); + }); + + test('a non-node volta image (yarn) is not rewritten to the node shim', () => { + const yarnImage = `${VOLTA_HOME}/tools/image/yarn/1.22.19/bin/yarn`; + assert.equal( + normalizeNodePath(yarnImage, { existsSync: () => true }), + yarnImage); + }); + + test('mise path is unaffected by the volta branch (no cross-manager capture)', () => { + const miseShim = '/Users/u/.local/share/mise/shims/node'; + assert.equal( + normalizeNodePath('/Users/u/.local/share/mise/installs/node/26.3.0/bin/node', + { existsSync: p => p === miseShim }), + miseShim); + }); +}); + // ──────────────────────────────────────────────────────────────────────── // Folded from tests/bug-2256-model-overrides-transport.test.cjs — consolidation epic #1969 (B1 #1970) diff --git a/tests/release-tarball-smoke.install.test.cjs b/tests/release-tarball-smoke.install.test.cjs index 1c212e83a..5fb5c8bb1 100644 --- a/tests/release-tarball-smoke.install.test.cjs +++ b/tests/release-tarball-smoke.install.test.cjs @@ -13,7 +13,7 @@ const fs = require('node:fs'); const path = require('node:path'); const { cleanup, createTempDir, runNpm, isolatedNpmEnv } = require('./helpers.cjs'); -const { SMOKE, runSmoke } = require('../scripts/release-tarball-smoke.cjs'); +const { SMOKE, runSmoke, CHILD_TIMEOUT_MS } = require('../scripts/release-tarball-smoke.cjs'); const smokeMsg = (label, result) => `${label}: code=${result.code} details=${JSON.stringify(result.details)}`; @@ -21,6 +21,34 @@ const smokeMsg = (label, result) => const PKG_PATH = path.join(__dirname, '..', 'package.json'); const pkg = JSON.parse(fs.readFileSync(PKG_PATH, 'utf-8')); +// ─── runSmoke install timeout must clear a slow-host cold install (#2335) ──── +// Regression: runSmoke()'s internal `npm install -g` used CHILD_TIMEOUT_MS, +// which was 120 s on non-Windows while before()'s pack+install used 600 s. A +// cold-cache install of the 1499-file tarball takes 3–6 min on a slow bench, so +// the per-test installs (B/C/D/E) fired SIGTERM at 120 s and returned a spurious +// INSTALL_FAILED (`spawnSync npm ETIMEDOUT`, empty stdout/stderr) on cartographer +// while passing on faster holodeck — a host-dependent false failure, not a flake. +// The ceiling is now a single exported constant shared by both surfaces; this +// pins it at/above the documented 600 s slow-host floor on EVERY platform, so a +// reintroduced 120 s (or a Windows-only 600 s) fails here instead of on a bench. +describe('release-tarball-smoke: install timeout ceiling', () => { + test('CHILD_TIMEOUT_MS clears the 600 s slow-host cold-install floor', () => { + assert.ok( + Number.isInteger(CHILD_TIMEOUT_MS) && CHILD_TIMEOUT_MS >= 600_000, + `CHILD_TIMEOUT_MS must be >= 600000ms for cartographer-class hosts; got ${CHILD_TIMEOUT_MS}`, + ); + }); + + test('the ceiling is platform-uniform — no host slower than the CI matrix is left under-provisioned', () => { + // The slow-host reality (cold disk, constrained CPU) is not OS-specific, so + // the constant must not be gated behind `process.platform`. A single numeric + // constant satisfies this by construction; this guards against a future + // reintroduction of a per-platform ternary that under-provisions Linux/macOS. + assert.equal(typeof CHILD_TIMEOUT_MS, 'number'); + assert.ok(CHILD_TIMEOUT_MS >= 600_000); + }); +}); + describe('release-tarball-smoke', () => { // Shared fixture state: pack the tarball once, install it once, reuse for all tests. let packDir; @@ -38,9 +66,11 @@ describe('release-tarball-smoke', () => { // npm pack + npm install -g on a large tarball (1499 files, ~10 MB) can take // 3–6 minutes on slow Docker hosts (cold disk, constrained CPU). The runNpm // default timeout of 180 s is sufficient on fast machines but insufficient on - // cartographer-class hosts. 600 s (10 min) gives a safe ceiling without - // masking genuine hangs. - const SLOW_HOST_TIMEOUT = 600_000; + // cartographer-class hosts. Share the smoke script's CHILD_TIMEOUT_MS ceiling + // so before() (pack+install) and runSmoke()'s per-test installs cannot diverge + // — divergence was the #2335-run defect: before() had 600 s, runSmoke had 120 s + // on Linux, so the per-test installs alone timed out on cartographer. + const SLOW_HOST_TIMEOUT = CHILD_TIMEOUT_MS; const packOutput = runNpm( ['pack', '--pack-destination', packDir],