From 6f55d7ae955664ab399aef18191101919ff45965 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 19 Aug 2026 14:49:28 -0700 Subject: [PATCH 01/11] test: apply the supply-chain age gate to npm and pnpm in Verdaccio tests The Verdaccio tests install the freshly published monorepo into throwaway app directories, so they can never pick up the root `.yarnrc.yml`. Until now only Yarn's gate was mirrored in the harness; npm and pnpm installed without one. Both have since grown the same policy, so set it for them too and keep the shared values in one place: - npm calls it `min-release-age` (in days) and has supported it since 11.19, so only pass it when the npm on `PATH` is new enough and say once when it isn't. CI pins npm 11.19.0 because npm 12 requires a newer Node than `.nvmrc`. - pnpm calls it `minimumReleaseAge` (in minutes) but reads it from its config files only, so generate a global config and point pnpm at it with `XDG_CONFIG_HOME`. That config also has to set `registry`: pnpm honors `--registry` and its config files but not `npm_config_registry`, so the pnpm half of these tests was resolving `@electron-forge/*` from the public registry and validating the last published release instead of the local build. Two other fixes fall out of this: - `pnpm store prune` is gone in favour of a per-run `cacheDir` under the storage directory. Staleness lives in the metadata cache, not in the content-addressed store, and a cold store made pnpm hang after installing until the test runner timed out. - `COREPACK_ROOT` no longer leaks into the spawned tests. `yarn test:verdaccio` runs Yarn through Corepack, and pnpm refuses to switch to the version `create-electron-app` pins when it thinks Corepack invoked it. Co-Authored-By: Claude --- .github/workflows/ci.yml | 8 + .../utils/test-utils/src/template-tests.ts | 12 ++ tools/verdaccio/spawn-verdaccio.ts | 149 +++++++++++++++--- 3 files changed, 148 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3960172ec1..7b8d82cf13 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -206,6 +206,14 @@ jobs: - name: Install pnpm run: npm install -g pnpm@11.10.0 + # The Verdaccio tests age-gate their installs like the root `.yarnrc.yml` + # does, and npm only learned about `min-release-age` in 11.19, which is + # newer than the npm bundled with the Node version in `.nvmrc`. npm 12 + # requires Node `^22.22.2 || ^24.15.0 || >=26`, so it cannot be installed + # on that Node version at all; bump this pin when `.nvmrc` moves. + - name: Install npm + run: npm install -g npm@11.19.0 + - name: Run slow tests run: | mkdir -p ./reports/out diff --git a/packages/utils/test-utils/src/template-tests.ts b/packages/utils/test-utils/src/template-tests.ts index 3c37d855ef..35d6665aa0 100644 --- a/packages/utils/test-utils/src/template-tests.ts +++ b/packages/utils/test-utils/src/template-tests.ts @@ -219,6 +219,18 @@ export function testForgeTemplate({ cwd: tmpDir, env: { PATH: process.env.PATH, + /** + * `start` makes the package manager check the lockfile it just + * wrote, and pnpm has enforced a minimum release age of its own by + * default since 11.16, so that check rejects the project outright + * whenever one of our dependencies published a release in the last + * day. `XDG_CONFIG_HOME` is where the Verdaccio test harness puts + * the config that tells pnpm which registry to use, how old a + * release has to be, and which packages are exempt, so we have to + * let it through to keep the same policy in force for the install + * and for the check. + */ + XDG_CONFIG_HOME: process.env.XDG_CONFIG_HOME, ...(process.platform === 'linux' && { DISPLAY: process.env.DISPLAY, XAUTHORITY: process.env.XAUTHORITY, diff --git a/tools/verdaccio/spawn-verdaccio.ts b/tools/verdaccio/spawn-verdaccio.ts index d2d1691fff..665df86070 100644 --- a/tools/verdaccio/spawn-verdaccio.ts +++ b/tools/verdaccio/spawn-verdaccio.ts @@ -34,6 +34,27 @@ const VERDACCIO_PORT = 4873; const VERDACCIO_URL = `http://${LOCALHOST}:${VERDACCIO_PORT}`; const STORAGE_PATH = path.resolve(import.meta.dirname, 'storage'); +/** + * We publish the monorepo to Verdaccio seconds before the tests install it, so + * every package manager's minimum age gate (which Yarn enables by default since + * 4.18) quarantines every local package. The tests install those packages into + * app directories created under `os.tmpdir()`, which are outside this repository + * and therefore never pick up the root `.yarnrc.yml`, so we mirror its policy + * for all three package managers here instead of switching the gate off: our own + * packages are exempt, everything else still has to have been on the registry + * for a week. Keep these in sync with `.yarnrc.yml`. + */ +const MINIMUM_RELEASE_AGE_MINUTES = 10080; +const MINIMUM_RELEASE_AGE_DAYS = MINIMUM_RELEASE_AGE_MINUTES / 60 / 24; +const PREAPPROVED_PACKAGES = [ + '@electron/*', + '@electron-forge/*', + '@electron-internal/*', + 'create-electron-app', + 'electron', + 'node-abi', +]; + const d = debug('electron-forge:verdaccio'); let verdaccioProcess: ChildProcess | null = null; @@ -135,14 +156,103 @@ async function publishPackages(): Promise { async function runCommand(args: string[]) { process.env.COREPACK_ENABLE_STRICT = '0'; - console.log('🗑️ Pruning pnpm store before running command'); - await spawnPromise('pnpm', ['store', 'prune']); + + /** + * `yarn test:verdaccio` runs Yarn through Corepack, which sets + * `COREPACK_ROOT` for everything it spawns, and the tests inherit it all the + * way down to the package manager that installs each generated app. pnpm + * refuses to switch to the version it is asked for when it believes Corepack + * invoked it, and `create-electron-app` asks Corepack to pin the latest pnpm + * in every app it creates, so the first install in a pnpm app fails with a + * version mismatch against whatever pnpm happens to be on `PATH`. The tests + * spawn their own package managers and have no business inheriting this + * repository's Corepack context, so we drop the variable here. + */ + const { COREPACK_ROOT: _corepackRoot, ...parentEnv } = process.env; /** * Avoid polluting the global yarn cache. */ const tempYarnGlobal = path.join(STORAGE_PATH, '.yarn-global'); - fs.promises.mkdir(tempYarnGlobal, { recursive: true }); + await fs.promises.mkdir(tempYarnGlobal, { recursive: true }); + + /** + * npm and Yarn take their settings from the environment, but pnpm reads these + * ones from its config files only, so we generate a global config file for it + * and point pnpm at it with `XDG_CONFIG_HOME` (which pnpm honors on every + * platform, including Windows). The tests install into throwaway directories, + * so the alternative would be writing a `pnpm-workspace.yaml` into each of + * them, but that would also make Forge's own `resolvePackageManager` treat + * them as pnpm projects, which we don't want in the npm/Yarn cases. + * + * Note that any other tool the tests run also picks up this config home (on + * Linux, for instance, the test apps write their Electron `userData` there), + * which is harmless because the directory is thrown away on the next run. + */ + const tempXdgConfigHome = path.join(STORAGE_PATH, '.xdg-config-home'); + await fs.promises.mkdir(path.join(tempXdgConfigHome, 'pnpm'), { + recursive: true, + }); + await fs.promises.writeFile( + path.join(tempXdgConfigHome, 'pnpm', 'config.yaml'), + // YAML is a superset of JSON, so this is a valid pnpm config file. + JSON.stringify( + { + /** + * pnpm reads the registry from its config files and from `--registry`, + * but not from `npm_config_registry` in the environment, so it is the + * one package manager that does not pick up `NPM_CONFIG_REGISTRY` + * below. Without this line pnpm quietly resolves `@electron-forge/*` + * from the public registry instead of from Verdaccio, and the tests + * pass against the last published release rather than the local build. + * https://pnpm.io/settings#registry + */ + registry: VERDACCIO_URL, + // https://pnpm.io/settings#minimumreleaseage + minimumReleaseAge: MINIMUM_RELEASE_AGE_MINUTES, + // https://pnpm.io/settings#minimumreleaseageexclude + minimumReleaseAgeExclude: PREAPPROVED_PACKAGES, + /** + * Every run republishes the monorepo under the version that is already + * in the manifests, so pnpm must not resolve those packages through + * metadata it cached during an earlier run: it would then look up the + * integrity hash that version had last time and find the matching + * tarball in its store, which is how the tests would end up running + * against a stale build. `startVerdaccio` deletes `STORAGE_PATH`, so a + * cache directory under it is empty on every run, which forces pnpm to + * ask Verdaccio for the hashes it is serving now. + * + * The store itself is deliberately left alone: it only holds content + * addressed by hash, so once the metadata is gone it cannot serve a + * stale tarball, and keeping it saves every run from downloading all of + * the third-party dependencies again. Emptying it with `pnpm store + * prune` was the previous approach, and a fully cold store also made + * pnpm hang after installing (its worker pool never shut down), which + * left these tests to fail on the test runner's timeout. + * https://pnpm.io/settings#cachedir + */ + cacheDir: path.join(STORAGE_PATH, '.pnpm-cache'), + }, + null, + 2, + ), + ); + + /** + * npm only learned about `min-release-age` in 11.19. Older versions install + * without a gate and warn that the config is unknown on every single npm + * invocation, so we only pass it when it is supported and say once that the + * npm side of the tests is ungated. + */ + const npmVersion = (await spawnPromise('npm', ['--version'])).trim(); + const [npmMajor, npmMinor] = npmVersion.split('.').map(Number); + const npmSupportsAgeGate = + npmMajor > 11 || (npmMajor === 11 && npmMinor >= 19); + if (!npmSupportsAgeGate) { + console.warn( + `⚠️ npm ${npmVersion} does not support \`min-release-age\` (npm >= 11.19 required), so npm installs in these tests are not age-gated`, + ); + } console.log(`🏃 Running: ${args.join(' ')}`); console.log(` Using registry: ${VERDACCIO_URL}`); @@ -151,7 +261,7 @@ async function runCommand(args: string[]) { cwd: FORGE_ROOT_DIR, stdio: 'inherit', env: { - ...process.env, + ...parentEnv, // https://docs.npmjs.com/cli/v9/using-npm/config#registry // https://pnpm.io/settings#registry NPM_CONFIG_REGISTRY: VERDACCIO_URL, @@ -159,27 +269,24 @@ async function runCommand(args: string[]) { YARN_NPM_REGISTRY_SERVER: VERDACCIO_URL, // https://yarnpkg.com/configuration/yarnrc#unsafeHttpWhitelist YARN_UNSAFE_HTTP_WHITELIST: LOCALHOST, - // We publish the monorepo to Verdaccio seconds before the tests install - // it, so Yarn's minimum age gate (1 day by default since Yarn 4.18) - // quarantines every local package. The tests install those packages into - // app directories created under `os.tmpdir()`, which are outside this - // repository and therefore never pick up the root `.yarnrc.yml`, so we - // mirror its policy here instead of switching the gate off: our own - // packages are exempt, everything else still has to have been on the - // registry for a week. Keep this in sync with `.yarnrc.yml`. + // Yarn's minimum age gate is 1 day by default since Yarn 4.18. // https://yarnpkg.com/configuration/yarnrc#npmMinimalAgeGate - YARN_NPM_MINIMAL_AGE_GATE: '10080', + YARN_NPM_MINIMAL_AGE_GATE: String(MINIMUM_RELEASE_AGE_MINUTES), // Yarn only accepts comma-separated values for array settings passed // through the environment. // https://yarnpkg.com/configuration/yarnrc#npmPreapprovedPackages - YARN_NPM_PREAPPROVED_PACKAGES: [ - '@electron/*', - '@electron-forge/*', - '@electron-internal/*', - 'create-electron-app', - 'electron', - 'node-abi', - ].join(','), + YARN_NPM_PREAPPROVED_PACKAGES: PREAPPROVED_PACKAGES.join(','), + ...(npmSupportsAgeGate && { + // npm calls the same policy `min-release-age` and counts it in days + // instead of minutes. + // https://docs.npmjs.com/cli/v12/using-npm/config#min-release-age + npm_config_min_release_age: String(MINIMUM_RELEASE_AGE_DAYS), + // Like Yarn, npm accepts a comma-separated list for this array setting. + // https://docs.npmjs.com/cli/v12/using-npm/config#min-release-age-exclude + npm_config_min_release_age_exclude: PREAPPROVED_PACKAGES.join(','), + }), + // Where pnpm looks for the global config file generated above. + XDG_CONFIG_HOME: tempXdgConfigHome, // Isolate package manager caches so Verdaccio packages // don't corrupt the global caches. These directories live // under STORAGE_PATH and get cleaned up on next run. From e84b71476da6fb923f92242adb98d2bda6477ecd Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 19 Aug 2026 16:44:35 -0700 Subject: [PATCH 02/11] test: keep pnpm from hanging after Verdaccio installs pnpm shuts its tarball worker pool down once per install, but any worker call that happens after that lazily creates a new pool that nothing ever shuts down, and the idle worker thread keeps the event loop alive. An install whose last download finishes just after pnpm prints `Done in Xs` therefore writes the lockfile, links everything, reports success and then never exits (pnpm/pnpm#13617). Installs that fetch nothing are unaffected, which is why this only shows up in these tests: they always install into a brand new project. Every test here spawns its package manager and waits for it to exit, so the hang costs the whole test rather than just the process, which is what was timing out the `pnpm` template tests on all three platforms. Put a stand-in for pnpm at the front of `PATH` for the duration of these tests that kills it once it has reported that it is done and has had a grace period to exit on its own. Co-Authored-By: Claude --- tools/verdaccio/pnpm-exit-shim.mjs | 176 +++++++++++++++++++++++++++++ tools/verdaccio/spawn-verdaccio.ts | 45 +++++++- 2 files changed, 218 insertions(+), 3 deletions(-) create mode 100644 tools/verdaccio/pnpm-exit-shim.mjs diff --git a/tools/verdaccio/pnpm-exit-shim.mjs b/tools/verdaccio/pnpm-exit-shim.mjs new file mode 100644 index 0000000000..e039726d26 --- /dev/null +++ b/tools/verdaccio/pnpm-exit-shim.mjs @@ -0,0 +1,176 @@ +/** + * A stand-in for `pnpm` that the Verdaccio test harness puts at the front of + * `PATH` (see `spawn-verdaccio.ts`). It runs the real pnpm and, once pnpm has + * reported that it is finished, gives it a few seconds to exit on its own and + * then kills it. + * + * pnpm shuts its tarball worker pool down once per install, but any worker call + * that happens after that lazily creates a new pool that nothing ever shuts + * down, and the idle worker thread keeps the event loop alive forever. So an + * install whose last download finishes just after pnpm prints `Done in Xs` + * writes the lockfile, links everything, reports success and then hangs. + * Installs that fetch nothing are unaffected, which is why this only shows up + * in these tests: they always install into a brand new project. + * + * Every test here spawns its package manager and waits for it to exit, so the + * hang costs us the whole test rather than just the process. Delete this shim + * and its wiring once the fix has shipped in a pnpm release. + * https://github.com/pnpm/pnpm/issues/13617 + */ + +import { spawn, spawnSync } from 'node:child_process'; +import fs from 'node:fs'; +import path from 'node:path'; + +/** + * How long pnpm gets to exit by itself after it says it is done. Long enough + * that we don't cut a slow-but-healthy shutdown short, short enough that a + * hang doesn't eat the test timeout. + */ +const EXIT_GRACE_PERIOD_MS = 15_000; + +/** + * What pnpm prints when it has finished the work it was asked to do. `pnpm run` + * and friends print nothing of the sort, so they are simply left alone. + */ +const DONE_PATTERN = /(^|\n)(Done in |Already up to date)/; + +/** + * How much of a chunk of output has to be kept around to match `DONE_PATTERN` + * against the next one: anything at least as long as the strings it looks for. + */ +const TAIL_LENGTH = 32; + +/** + * The directory the launcher that runs this file lives in, which the launcher + * itself tells us about: it is somewhere else entirely, so this file cannot + * work it out on its own. + */ +const LAUNCHER_DIR = process.env.FORGE_PNPM_EXIT_SHIM_DIR; +if (!LAUNCHER_DIR) { + throw new Error( + 'FORGE_PNPM_EXIT_SHIM_DIR must point at the directory this shim is installed in', + ); +} + +/** + * The names a real pnpm can have on `PATH`, in the order the OS would pick + * between them. + */ +const PNPM_FILENAMES = + process.platform === 'win32' + ? ['pnpm.exe', 'pnpm.cmd', 'pnpm.bat'] + : ['pnpm']; + +// Windows spells this `Path`, and `process.env` only hides the difference until +// it gets copied. +const pathKey = + Object.keys(process.env).find((key) => key.toUpperCase() === 'PATH') ?? + 'PATH'; + +/** + * `PATH` without the directory this shim is installed in, which is both where + * we look for the pnpm we are standing in for and what we hand to it: leaving + * ourselves in would let anything that resolves `pnpm` through `PATH` — this + * shim included — end up back here. + */ +const realPath = (process.env[pathKey] ?? '') + .split(path.delimiter) + .filter((dir) => dir !== '' && !isLauncherDir(dir)); + +function isLauncherDir(dir) { + const [candidate, launcher] = [path.resolve(dir), path.resolve(LAUNCHER_DIR)]; + return process.platform === 'win32' + ? candidate.toLowerCase() === launcher.toLowerCase() + : candidate === launcher; +} + +const realPnpm = realPath + .flatMap((dir) => PNPM_FILENAMES.map((filename) => path.join(dir, filename))) + .find((candidate) => fs.existsSync(candidate)); +if (!realPnpm) { + throw new Error(`pnpm not found on PATH (${realPath.join(path.delimiter)})`); +} + +const pnpm = spawn(realPnpm, process.argv.slice(2), { + stdio: ['inherit', 'pipe', 'pipe'], + env: { ...process.env, [pathKey]: realPath.join(path.delimiter) }, + // Run pnpm in its own process group so that we can take down the version of + // itself that it hands over to along with it. + detached: process.platform !== 'win32', + // `.cmd` and `.bat` files can only be run through a shell. + shell: process.platform === 'win32', +}); + +function killPnpm() { + if (process.platform === 'win32') { + spawnSync('taskkill', ['/pid', String(pnpm.pid), '/t', '/f'], { + stdio: 'ignore', + }); + } else { + try { + process.kill(-pnpm.pid, 'SIGKILL'); + } catch { + pnpm.kill('SIGKILL'); + } + } +} + +let exitTimer; +let killedPnpm = false; + +/** + * Pass pnpm's output through untouched — the tests read it — while watching for + * the point where it has nothing left to do. `DONE_PATTERN` can straddle two + * chunks, so each chunk is matched together with the tail of the one before it. + */ +function forward(stream, chunk, previousTail) { + stream.write(chunk); + + const text = `${previousTail}${chunk}`; + if (exitTimer === undefined && DONE_PATTERN.test(text)) { + exitTimer = setTimeout(() => { + process.stderr.write( + `\n[verdaccio harness] pnpm did not exit ${EXIT_GRACE_PERIOD_MS / 1000}s after reporting that it was done, killing it (https://github.com/pnpm/pnpm/issues/13617)\n`, + ); + // Once pnpm is gone its output pipes close and this process exits by + // itself, which flushes whatever it has already written. + killedPnpm = true; + killPnpm(); + }, EXIT_GRACE_PERIOD_MS); + } + + return text.slice(-TAIL_LENGTH); +} + +let stdoutTail = ''; +pnpm.stdout.on('data', (chunk) => { + stdoutTail = forward(process.stdout, chunk, stdoutTail); +}); + +let stderrTail = ''; +pnpm.stderr.on('data', (chunk) => { + stderrTail = forward(process.stderr, chunk, stderrTail); +}); + +pnpm.on('error', (error) => { + clearTimeout(exitTimer); + throw error; +}); + +// pnpm runs in its own process group, so whoever kills this shim would leave it +// running behind us. +for (const signal of ['SIGINT', 'SIGTERM']) { + process.on(signal, () => { + clearTimeout(exitTimer); + killPnpm(); + process.exitCode = 1; + }); +} + +pnpm.on('exit', (code, signal) => { + clearTimeout(exitTimer); + // pnpm had already done what it was asked to do by the time we killed it, so + // the command it was running succeeded. + process.exitCode = killedPnpm ? 0 : signal ? 1 : (code ?? 0); +}); diff --git a/tools/verdaccio/spawn-verdaccio.ts b/tools/verdaccio/spawn-verdaccio.ts index 665df86070..7a2de798aa 100644 --- a/tools/verdaccio/spawn-verdaccio.ts +++ b/tools/verdaccio/spawn-verdaccio.ts @@ -226,9 +226,7 @@ async function runCommand(args: string[]) { * addressed by hash, so once the metadata is gone it cannot serve a * stale tarball, and keeping it saves every run from downloading all of * the third-party dependencies again. Emptying it with `pnpm store - * prune` was the previous approach, and a fully cold store also made - * pnpm hang after installing (its worker pool never shut down), which - * left these tests to fail on the test runner's timeout. + * prune` was the previous approach. * https://pnpm.io/settings#cachedir */ cacheDir: path.join(STORAGE_PATH, '.pnpm-cache'), @@ -238,6 +236,46 @@ async function runCommand(args: string[]) { ), ); + /** + * pnpm can finish an install and then never exit, which costs us a whole test + * because the tests wait for the package manager they spawned to be done. + * `pnpm-exit-shim.mjs` explains why and works around it; putting it in front + * of the real pnpm on `PATH` covers every pnpm these tests run, whether they + * run it themselves or through `create-electron-app`. + */ + const tempBinDir = path.join(STORAGE_PATH, '.bin'); + await fs.promises.mkdir(tempBinDir, { recursive: true }); + const pnpmExitShim = path.resolve(import.meta.dirname, 'pnpm-exit-shim.mjs'); + if (process.platform === 'win32') { + await fs.promises.writeFile( + path.join(tempBinDir, 'pnpm.cmd'), + [ + '@echo off', + `set "FORGE_PNPM_EXIT_SHIM_DIR=${tempBinDir}"`, + `node "${pnpmExitShim}" %*`, + '', + ].join('\r\n'), + ); + } else { + const pnpmLauncher = path.join(tempBinDir, 'pnpm'); + await fs.promises.writeFile( + pnpmLauncher, + [ + '#!/bin/sh', + `FORGE_PNPM_EXIT_SHIM_DIR="${tempBinDir}"`, + 'export FORGE_PNPM_EXIT_SHIM_DIR', + `exec node "${pnpmExitShim}" "$@"`, + '', + ].join('\n'), + ); + await fs.promises.chmod(pnpmLauncher, 0o755); + } + // Windows spells this `Path`, and spreading `process.env` above lost the + // case-insensitive lookup that hides the difference. + const pathKey = + Object.keys(parentEnv).find((key) => key.toUpperCase() === 'PATH') ?? + 'PATH'; + /** * npm only learned about `min-release-age` in 11.19. Older versions install * without a gate and warn that the config is unknown on every single npm @@ -262,6 +300,7 @@ async function runCommand(args: string[]) { stdio: 'inherit', env: { ...parentEnv, + [pathKey]: [tempBinDir, parentEnv[pathKey]].join(path.delimiter), // https://docs.npmjs.com/cli/v9/using-npm/config#registry // https://pnpm.io/settings#registry NPM_CONFIG_REGISTRY: VERDACCIO_URL, From 2ceba335cc165800d21966a3acf6ac4a579d775b Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 19 Aug 2026 21:59:23 -0700 Subject: [PATCH 03/11] test: stop pnpm from reinstalling the app the tests just installed Since pnpm 11, `pnpm run` silently runs an install first whenever it decides that `node_modules` is out of sync with the lockfile. These tests run ` run start` to check that the app `create-electron-app` just installed can start, so that install replaces the very thing they are checking: on Windows it rewrote the dependency tree into one where the generated `forge.config.ts` could no longer resolve the Forge plugin it imports, and `electron-forge start` failed. Set `verifyDepsBeforeRun` to `warn` so the check still runs and still reports whatever it believes is out of sync, without acting on it. While here, only watch for the exit hang on the commands that install packages. `pnpm run start` keeps running long after pnpm reports that it is done, and the last thing the exit shim should do is kill it. Co-Authored-By: Claude --- tools/verdaccio/pnpm-exit-shim.mjs | 68 +++++++++++++++++++++++------- tools/verdaccio/spawn-verdaccio.ts | 13 ++++++ 2 files changed, 66 insertions(+), 15 deletions(-) diff --git a/tools/verdaccio/pnpm-exit-shim.mjs b/tools/verdaccio/pnpm-exit-shim.mjs index e039726d26..6b15c8454f 100644 --- a/tools/verdaccio/pnpm-exit-shim.mjs +++ b/tools/verdaccio/pnpm-exit-shim.mjs @@ -1,8 +1,8 @@ /** * A stand-in for `pnpm` that the Verdaccio test harness puts at the front of - * `PATH` (see `spawn-verdaccio.ts`). It runs the real pnpm and, once pnpm has - * reported that it is finished, gives it a few seconds to exit on its own and - * then kills it. + * `PATH` (see `spawn-verdaccio.ts`). It runs the real pnpm and, once an install + * has reported that it is finished, gives it a few seconds to exit on its own + * and then kills it. * * pnpm shuts its tarball worker pool down once per install, but any worker call * that happens after that lazily creates a new pool that nothing ever shuts @@ -30,17 +30,51 @@ import path from 'node:path'; const EXIT_GRACE_PERIOD_MS = 15_000; /** - * What pnpm prints when it has finished the work it was asked to do. `pnpm run` - * and friends print nothing of the sort, so they are simply left alone. + * What pnpm prints when it has finished the work it was asked to do. */ const DONE_PATTERN = /(^|\n)(Done in |Already up to date)/; +/** + * The commands that install packages, and therefore the only ones that can hang + * this way. Every other command — `pnpm run start`, which the tests use to + * launch the app they are testing, above all — is left alone entirely: it keeps + * running long after pnpm has said that it is done, and killing it is the last + * thing we want. + */ +const INSTALL_COMMANDS = new Set([ + 'add', + 'dedupe', + 'fetch', + 'i', + 'import', + 'install', + 'link', + 'prune', + 'remove', + 'rm', + 'un', + 'uninstall', + 'unlink', + 'up', + 'update', +]); + /** * How much of a chunk of output has to be kept around to match `DONE_PATTERN` * against the next one: anything at least as long as the strings it looks for. */ const TAIL_LENGTH = 32; +const pnpmArgs = process.argv.slice(2); + +/** + * These tests only ever run pnpm as `pnpm [flags]`, so the first + * argument that isn't a flag is the command. + */ +const watchForHang = INSTALL_COMMANDS.has( + pnpmArgs.find((arg) => !arg.startsWith('-')), +); + /** * The directory the launcher that runs this file lives in, which the launcher * itself tells us about: it is somewhere else entirely, so this file cannot @@ -92,8 +126,10 @@ if (!realPnpm) { throw new Error(`pnpm not found on PATH (${realPath.join(path.delimiter)})`); } -const pnpm = spawn(realPnpm, process.argv.slice(2), { - stdio: ['inherit', 'pipe', 'pipe'], +const pnpm = spawn(realPnpm, pnpmArgs, { + // Watching for the hang means reading pnpm's output on the way past. Anything + // we are not watching gets the real thing's streams, untouched. + stdio: watchForHang ? ['inherit', 'pipe', 'pipe'] : 'inherit', env: { ...process.env, [pathKey]: realPath.join(path.delimiter) }, // Run pnpm in its own process group so that we can take down the version of // itself that it hands over to along with it. @@ -143,15 +179,17 @@ function forward(stream, chunk, previousTail) { return text.slice(-TAIL_LENGTH); } -let stdoutTail = ''; -pnpm.stdout.on('data', (chunk) => { - stdoutTail = forward(process.stdout, chunk, stdoutTail); -}); +if (watchForHang) { + let stdoutTail = ''; + pnpm.stdout.on('data', (chunk) => { + stdoutTail = forward(process.stdout, chunk, stdoutTail); + }); -let stderrTail = ''; -pnpm.stderr.on('data', (chunk) => { - stderrTail = forward(process.stderr, chunk, stderrTail); -}); + let stderrTail = ''; + pnpm.stderr.on('data', (chunk) => { + stderrTail = forward(process.stderr, chunk, stderrTail); + }); +} pnpm.on('error', (error) => { clearTimeout(exitTimer); diff --git a/tools/verdaccio/spawn-verdaccio.ts b/tools/verdaccio/spawn-verdaccio.ts index 7a2de798aa..115055cf9f 100644 --- a/tools/verdaccio/spawn-verdaccio.ts +++ b/tools/verdaccio/spawn-verdaccio.ts @@ -230,6 +230,19 @@ async function runCommand(args: string[]) { * https://pnpm.io/settings#cachedir */ cacheDir: path.join(STORAGE_PATH, '.pnpm-cache'), + /** + * Since pnpm 11, `pnpm run` silently runs an install first whenever it + * decides that `node_modules` is out of sync with the lockfile. The + * tests run ` run start` to check that the app + * `create-electron-app` just installed can start, so an install in + * between replaces the very thing they are checking: on Windows it + * rewrote the dependency tree into one where the generated + * `forge.config.ts` could no longer resolve the Forge plugin it + * imports. `warn` keeps the check itself, and its report of whatever it + * believes is out of sync, without acting on it. + * https://pnpm.io/settings#verifydepsbeforerun + */ + verifyDepsBeforeRun: 'warn', }, null, 2, From 253b3a96c121f377116dd0c8afafbf64b541a457 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 19 Aug 2026 22:29:06 -0700 Subject: [PATCH 04/11] test: pin pnpm's virtual store layout in Verdaccio tests pnpm decides whether to link dependencies through a store-wide virtual store or one inside the project based on whether it believes it is running in CI, and these tests cannot keep that consistent: they install with the environment they inherit and then run the app's `start` script with a minimal one, so pnpm read the same project two different ways and reported that `node_modules` no longer matched the lockfile. Pin the setting to the value CI would pick anyway. Also report the dependency tree the package manager installed when `start` fails, since a `start` that cannot resolve the app's own configuration says nothing about the tree it was trying to resolve it from. Co-Authored-By: Claude --- .../utils/test-utils/src/template-tests.ts | 56 +++++++++++++++++-- tools/verdaccio/spawn-verdaccio.ts | 11 ++++ 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/packages/utils/test-utils/src/template-tests.ts b/packages/utils/test-utils/src/template-tests.ts index 35d6665aa0..9dfe339d35 100644 --- a/packages/utils/test-utils/src/template-tests.ts +++ b/packages/utils/test-utils/src/template-tests.ts @@ -36,6 +36,36 @@ export type TestForgeTemplateOptions = { const d = debug('electron-forge:testForgeTemplate'); +/** + * Summarizes the layout a package manager installed into a project, which is + * what tells a flat `node_modules` (npm, Yarn, pnpm with `nodeLinker: hoisted`) + * apart from one where every package is a link into a store, and shows which + * packages a failing app could have resolved. + */ +function describeDependencyTree(projectDir: string) { + const nodeModules = path.join(projectDir, 'node_modules'); + + let entries; + try { + entries = fs.readdirSync(nodeModules, { withFileTypes: true }); + } catch (error) { + return `no readable \`node_modules\` in ${projectDir} (${error})`; + } + + const names = entries + .flatMap((entry) => + // Scopes hold the packages we care about rather than being one themselves. + entry.name.startsWith('@') + ? fs + .readdirSync(path.join(nodeModules, entry.name)) + .map((scoped) => `${entry.name}/${scoped}`) + : [entry.name], + ) + .sort(); + + return `${names.length} packages into ${nodeModules}: ${names.join(' ')}`; +} + /** * Runs the local version of `create-electron-app` to create a project based on * a given Forge template using all supported package managers. Because this @@ -212,10 +242,8 @@ export function testForgeTemplate({ ].join('\n'), ); - const electronForgeStartOutput = await spawn( - packageManager, - ['run', 'start'], - { + const startApp = () => + spawn(packageManager, ['run', 'start'], { cwd: tmpDir, env: { PATH: process.env.PATH, @@ -258,8 +286,24 @@ export function testForgeTemplate({ .replace(/\bnpm\/\?/, 'npm/99.99.99'), }), }, - }, - ); + }); + + let electronForgeStartOutput: string; + try { + electronForgeStartOutput = await startApp(); + } catch (error) { + /** + * When `start` fails, it is usually because the package manager + * installed a dependency tree the app cannot resolve its own + * configuration from, and the failure alone doesn't say which tree it + * ended up with. + */ + console.error( + `[template-tests] ${packageManager} installed ${describeDependencyTree(tmpDir)}`, + ); + + throw error; + } d({ electronForgeStartOutput }); diff --git a/tools/verdaccio/spawn-verdaccio.ts b/tools/verdaccio/spawn-verdaccio.ts index 115055cf9f..1d8f08a561 100644 --- a/tools/verdaccio/spawn-verdaccio.ts +++ b/tools/verdaccio/spawn-verdaccio.ts @@ -243,6 +243,17 @@ async function runCommand(args: string[]) { * https://pnpm.io/settings#verifydepsbeforerun */ verifyDepsBeforeRun: 'warn', + /** + * Whether pnpm links dependencies through a store-wide virtual store or + * one inside the project defaults to whether pnpm believes it is + * running in CI, which these tests cannot keep consistent: they install + * with the environment they inherit and then run the app's `start` + * script with a minimal one, so pnpm read the same project two + * different ways and reported that `node_modules` no longer matched the + * lockfile. Pin it to the value CI would pick anyway. + * https://pnpm.io/settings#enableglobalvirtualstore + */ + enableGlobalVirtualStore: false, }, null, 2, From ef05bef396c046954c3386d292b3c030b3e6948e Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 19 Aug 2026 22:58:52 -0700 Subject: [PATCH 05/11] test: stop killing pnpm when it reports that an install is done MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shim this removes ran pnpm and killed it 15 seconds after it printed `Done in`, on the theory that anything past that point was the leaked worker pool of pnpm/pnpm#13617 keeping a finished process alive. On Windows it was cutting installs short instead: both TypeScript templates came out of `create-electron-app` with exactly the base template's dependencies and none of their own, because the second install reports that it is done and is then killed while the very work the pnpm bug defers — copying packages into `node_modules` — is still going. Windows CI keeps its pnpm store on `C:` and its projects on `D:`, so nothing can be hardlinked and that tail takes far longer than it does anywhere else. An install that quietly loses half of a project is a worse failure than one that hangs, and the hang the shim was written for was in the install `pnpm run` used to perform behind the tests' back, which `verifyDepsBeforeRun: warn` already stopped. Also report what `create-electron-app` printed when `start` fails, since it runs its steps with listr2's `exitOnError: false` and so exits 0 with a broken project when an install fails. Co-Authored-By: Claude --- .../utils/test-utils/src/template-tests.ts | 12 +- tools/verdaccio/pnpm-exit-shim.mjs | 214 ------------------ tools/verdaccio/spawn-verdaccio.ts | 41 ---- 3 files changed, 9 insertions(+), 258 deletions(-) delete mode 100644 tools/verdaccio/pnpm-exit-shim.mjs diff --git a/packages/utils/test-utils/src/template-tests.ts b/packages/utils/test-utils/src/template-tests.ts index 9dfe339d35..0b0cccd665 100644 --- a/packages/utils/test-utils/src/template-tests.ts +++ b/packages/utils/test-utils/src/template-tests.ts @@ -135,7 +135,7 @@ export function testForgeTemplate({ throw new Error(`unknown template ${templateName}`); } - await spawn('node', [ + const createOutput = await spawn('node', [ path.resolve( __dirname, '../../../external/create-electron-app/dist/create-electron-app.js', @@ -296,10 +296,16 @@ export function testForgeTemplate({ * When `start` fails, it is usually because the package manager * installed a dependency tree the app cannot resolve its own * configuration from, and the failure alone doesn't say which tree it - * ended up with. + * ended up with. `create-electron-app` runs its steps with listr2's + * `exitOnError: false`, so a failed install leaves a broken project + * behind and still exits 0; its output is the only place that failure + * is reported at all. */ console.error( - `[template-tests] ${packageManager} installed ${describeDependencyTree(tmpDir)}`, + [ + `[template-tests] ${packageManager} installed ${describeDependencyTree(tmpDir)}`, + `[template-tests] create-electron-app said:\n${createOutput}`, + ].join('\n'), ); throw error; diff --git a/tools/verdaccio/pnpm-exit-shim.mjs b/tools/verdaccio/pnpm-exit-shim.mjs deleted file mode 100644 index 6b15c8454f..0000000000 --- a/tools/verdaccio/pnpm-exit-shim.mjs +++ /dev/null @@ -1,214 +0,0 @@ -/** - * A stand-in for `pnpm` that the Verdaccio test harness puts at the front of - * `PATH` (see `spawn-verdaccio.ts`). It runs the real pnpm and, once an install - * has reported that it is finished, gives it a few seconds to exit on its own - * and then kills it. - * - * pnpm shuts its tarball worker pool down once per install, but any worker call - * that happens after that lazily creates a new pool that nothing ever shuts - * down, and the idle worker thread keeps the event loop alive forever. So an - * install whose last download finishes just after pnpm prints `Done in Xs` - * writes the lockfile, links everything, reports success and then hangs. - * Installs that fetch nothing are unaffected, which is why this only shows up - * in these tests: they always install into a brand new project. - * - * Every test here spawns its package manager and waits for it to exit, so the - * hang costs us the whole test rather than just the process. Delete this shim - * and its wiring once the fix has shipped in a pnpm release. - * https://github.com/pnpm/pnpm/issues/13617 - */ - -import { spawn, spawnSync } from 'node:child_process'; -import fs from 'node:fs'; -import path from 'node:path'; - -/** - * How long pnpm gets to exit by itself after it says it is done. Long enough - * that we don't cut a slow-but-healthy shutdown short, short enough that a - * hang doesn't eat the test timeout. - */ -const EXIT_GRACE_PERIOD_MS = 15_000; - -/** - * What pnpm prints when it has finished the work it was asked to do. - */ -const DONE_PATTERN = /(^|\n)(Done in |Already up to date)/; - -/** - * The commands that install packages, and therefore the only ones that can hang - * this way. Every other command — `pnpm run start`, which the tests use to - * launch the app they are testing, above all — is left alone entirely: it keeps - * running long after pnpm has said that it is done, and killing it is the last - * thing we want. - */ -const INSTALL_COMMANDS = new Set([ - 'add', - 'dedupe', - 'fetch', - 'i', - 'import', - 'install', - 'link', - 'prune', - 'remove', - 'rm', - 'un', - 'uninstall', - 'unlink', - 'up', - 'update', -]); - -/** - * How much of a chunk of output has to be kept around to match `DONE_PATTERN` - * against the next one: anything at least as long as the strings it looks for. - */ -const TAIL_LENGTH = 32; - -const pnpmArgs = process.argv.slice(2); - -/** - * These tests only ever run pnpm as `pnpm [flags]`, so the first - * argument that isn't a flag is the command. - */ -const watchForHang = INSTALL_COMMANDS.has( - pnpmArgs.find((arg) => !arg.startsWith('-')), -); - -/** - * The directory the launcher that runs this file lives in, which the launcher - * itself tells us about: it is somewhere else entirely, so this file cannot - * work it out on its own. - */ -const LAUNCHER_DIR = process.env.FORGE_PNPM_EXIT_SHIM_DIR; -if (!LAUNCHER_DIR) { - throw new Error( - 'FORGE_PNPM_EXIT_SHIM_DIR must point at the directory this shim is installed in', - ); -} - -/** - * The names a real pnpm can have on `PATH`, in the order the OS would pick - * between them. - */ -const PNPM_FILENAMES = - process.platform === 'win32' - ? ['pnpm.exe', 'pnpm.cmd', 'pnpm.bat'] - : ['pnpm']; - -// Windows spells this `Path`, and `process.env` only hides the difference until -// it gets copied. -const pathKey = - Object.keys(process.env).find((key) => key.toUpperCase() === 'PATH') ?? - 'PATH'; - -/** - * `PATH` without the directory this shim is installed in, which is both where - * we look for the pnpm we are standing in for and what we hand to it: leaving - * ourselves in would let anything that resolves `pnpm` through `PATH` — this - * shim included — end up back here. - */ -const realPath = (process.env[pathKey] ?? '') - .split(path.delimiter) - .filter((dir) => dir !== '' && !isLauncherDir(dir)); - -function isLauncherDir(dir) { - const [candidate, launcher] = [path.resolve(dir), path.resolve(LAUNCHER_DIR)]; - return process.platform === 'win32' - ? candidate.toLowerCase() === launcher.toLowerCase() - : candidate === launcher; -} - -const realPnpm = realPath - .flatMap((dir) => PNPM_FILENAMES.map((filename) => path.join(dir, filename))) - .find((candidate) => fs.existsSync(candidate)); -if (!realPnpm) { - throw new Error(`pnpm not found on PATH (${realPath.join(path.delimiter)})`); -} - -const pnpm = spawn(realPnpm, pnpmArgs, { - // Watching for the hang means reading pnpm's output on the way past. Anything - // we are not watching gets the real thing's streams, untouched. - stdio: watchForHang ? ['inherit', 'pipe', 'pipe'] : 'inherit', - env: { ...process.env, [pathKey]: realPath.join(path.delimiter) }, - // Run pnpm in its own process group so that we can take down the version of - // itself that it hands over to along with it. - detached: process.platform !== 'win32', - // `.cmd` and `.bat` files can only be run through a shell. - shell: process.platform === 'win32', -}); - -function killPnpm() { - if (process.platform === 'win32') { - spawnSync('taskkill', ['/pid', String(pnpm.pid), '/t', '/f'], { - stdio: 'ignore', - }); - } else { - try { - process.kill(-pnpm.pid, 'SIGKILL'); - } catch { - pnpm.kill('SIGKILL'); - } - } -} - -let exitTimer; -let killedPnpm = false; - -/** - * Pass pnpm's output through untouched — the tests read it — while watching for - * the point where it has nothing left to do. `DONE_PATTERN` can straddle two - * chunks, so each chunk is matched together with the tail of the one before it. - */ -function forward(stream, chunk, previousTail) { - stream.write(chunk); - - const text = `${previousTail}${chunk}`; - if (exitTimer === undefined && DONE_PATTERN.test(text)) { - exitTimer = setTimeout(() => { - process.stderr.write( - `\n[verdaccio harness] pnpm did not exit ${EXIT_GRACE_PERIOD_MS / 1000}s after reporting that it was done, killing it (https://github.com/pnpm/pnpm/issues/13617)\n`, - ); - // Once pnpm is gone its output pipes close and this process exits by - // itself, which flushes whatever it has already written. - killedPnpm = true; - killPnpm(); - }, EXIT_GRACE_PERIOD_MS); - } - - return text.slice(-TAIL_LENGTH); -} - -if (watchForHang) { - let stdoutTail = ''; - pnpm.stdout.on('data', (chunk) => { - stdoutTail = forward(process.stdout, chunk, stdoutTail); - }); - - let stderrTail = ''; - pnpm.stderr.on('data', (chunk) => { - stderrTail = forward(process.stderr, chunk, stderrTail); - }); -} - -pnpm.on('error', (error) => { - clearTimeout(exitTimer); - throw error; -}); - -// pnpm runs in its own process group, so whoever kills this shim would leave it -// running behind us. -for (const signal of ['SIGINT', 'SIGTERM']) { - process.on(signal, () => { - clearTimeout(exitTimer); - killPnpm(); - process.exitCode = 1; - }); -} - -pnpm.on('exit', (code, signal) => { - clearTimeout(exitTimer); - // pnpm had already done what it was asked to do by the time we killed it, so - // the command it was running succeeded. - process.exitCode = killedPnpm ? 0 : signal ? 1 : (code ?? 0); -}); diff --git a/tools/verdaccio/spawn-verdaccio.ts b/tools/verdaccio/spawn-verdaccio.ts index 1d8f08a561..360831dcf0 100644 --- a/tools/verdaccio/spawn-verdaccio.ts +++ b/tools/verdaccio/spawn-verdaccio.ts @@ -260,46 +260,6 @@ async function runCommand(args: string[]) { ), ); - /** - * pnpm can finish an install and then never exit, which costs us a whole test - * because the tests wait for the package manager they spawned to be done. - * `pnpm-exit-shim.mjs` explains why and works around it; putting it in front - * of the real pnpm on `PATH` covers every pnpm these tests run, whether they - * run it themselves or through `create-electron-app`. - */ - const tempBinDir = path.join(STORAGE_PATH, '.bin'); - await fs.promises.mkdir(tempBinDir, { recursive: true }); - const pnpmExitShim = path.resolve(import.meta.dirname, 'pnpm-exit-shim.mjs'); - if (process.platform === 'win32') { - await fs.promises.writeFile( - path.join(tempBinDir, 'pnpm.cmd'), - [ - '@echo off', - `set "FORGE_PNPM_EXIT_SHIM_DIR=${tempBinDir}"`, - `node "${pnpmExitShim}" %*`, - '', - ].join('\r\n'), - ); - } else { - const pnpmLauncher = path.join(tempBinDir, 'pnpm'); - await fs.promises.writeFile( - pnpmLauncher, - [ - '#!/bin/sh', - `FORGE_PNPM_EXIT_SHIM_DIR="${tempBinDir}"`, - 'export FORGE_PNPM_EXIT_SHIM_DIR', - `exec node "${pnpmExitShim}" "$@"`, - '', - ].join('\n'), - ); - await fs.promises.chmod(pnpmLauncher, 0o755); - } - // Windows spells this `Path`, and spreading `process.env` above lost the - // case-insensitive lookup that hides the difference. - const pathKey = - Object.keys(parentEnv).find((key) => key.toUpperCase() === 'PATH') ?? - 'PATH'; - /** * npm only learned about `min-release-age` in 11.19. Older versions install * without a gate and warn that the config is unknown on every single npm @@ -324,7 +284,6 @@ async function runCommand(args: string[]) { stdio: 'inherit', env: { ...parentEnv, - [pathKey]: [tempBinDir, parentEnv[pathKey]].join(path.delimiter), // https://docs.npmjs.com/cli/v9/using-npm/config#registry // https://pnpm.io/settings#registry NPM_CONFIG_REGISTRY: VERDACCIO_URL, From 2dd6666bddd7f1a5dc8db5977538265b544d8110 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 19 Aug 2026 23:27:26 -0700 Subject: [PATCH 06/11] test: repair the tree when pnpm has to be killed for hanging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restores the watchdog removed in the commit before this one — pnpm really does finish an install and then hang, and without it the vite-typescript project times out at 240s — but stops it from leaving a project behind that cannot be repaired. Killing pnpm once it reports success is only safe if it has really stopped working, and it hasn't: the hang comes from a package that arrived after pnpm stopped expecting one, and that package is still being written. Worse, pnpm will not put it back, because it decides whether a project is up to date from the state files it keeps in `node_modules` rather than from the packages themselves — installing again into a tree it has already recorded is `Already up to date` even with packages deleted out of it, `--force` included. That is what Windows was failing on: both TypeScript projects came out of `create-electron-app` with the base template's dependencies and none of their own. So the watchdog now discards those state files after killing pnpm and installs again, which makes pnpm compare the tree against the store and write whatever is missing. A repair only has to link packages that are already in the store, so it is very unlikely to hang in turn; if every attempt does, the shim now says so and fails instead of reporting a success it cannot vouch for. Co-Authored-By: Claude --- tools/verdaccio/pnpm-exit-shim.mjs | 290 +++++++++++++++++++++++++++++ tools/verdaccio/spawn-verdaccio.ts | 41 ++++ 2 files changed, 331 insertions(+) create mode 100644 tools/verdaccio/pnpm-exit-shim.mjs diff --git a/tools/verdaccio/pnpm-exit-shim.mjs b/tools/verdaccio/pnpm-exit-shim.mjs new file mode 100644 index 0000000000..8360459997 --- /dev/null +++ b/tools/verdaccio/pnpm-exit-shim.mjs @@ -0,0 +1,290 @@ +/** + * A stand-in for `pnpm` that the Verdaccio test harness puts at the front of + * `PATH` (see `spawn-verdaccio.ts`). It runs the real pnpm and, when an install + * says that it is finished and then never exits, kills it and installs again. + * + * pnpm shuts its tarball worker pool down once per install, but a worker call + * that happens after that lazily creates a new pool which nothing ever shuts + * down, and the idle worker thread keeps the event loop alive forever. So an + * install whose last package arrives just too late writes the lockfile, reports + * success and then hangs. Installs that fetch nothing are unaffected, which is + * why this only shows up in these tests: they always install into a brand new + * project. Every test spawns its package manager and waits for it to exit, so + * the hang costs us the whole test rather than just the process. + * https://github.com/pnpm/pnpm/issues/13617 + * + * That late package is also why killing pnpm is not enough on its own: it is + * still being written when pnpm reports success, so killing pnpm leaves it out + * of `node_modules` — and pnpm will not put it back. It decides whether a + * project is up to date from the state files it keeps alongside the packages + * rather than from the packages themselves, so installing again into a tree it + * has already recorded is `Already up to date` even with packages missing from + * it, `--force` included. Discarding those files first makes the next install + * compare the tree against the store and fill in whatever is not there. + * + * Delete this shim and its wiring once the fix has shipped in a pnpm release. + */ + +import { spawn, spawnSync } from 'node:child_process'; +import fs from 'node:fs'; +import path from 'node:path'; + +/** + * How long pnpm gets to exit by itself after it says it is done. Long enough + * that we don't cut a slow-but-healthy shutdown short, short enough that a hang + * doesn't eat the test timeout — and cutting one short only costs the install + * that repairs it. + */ +const EXIT_GRACE_PERIOD_MS = 15_000; + +/** + * How many times to install in total. The hang needs a package to arrive after + * pnpm has stopped expecting one, so an install that only has to link what is + * already in the store — which is all a repair has left to do — is very + * unlikely to hit it again. + */ +const MAX_ATTEMPTS = 3; + +/** + * What pnpm prints when it has finished the work it was asked to do. + */ +const DONE_PATTERN = /(^|\n)(Done in |Already up to date)/; + +/** + * The files pnpm reads to decide that a project already has the dependencies it + * is being asked to install, which is what stops it from repairing a tree that + * an interrupted install left incomplete. pnpm writes them itself, so removing + * them costs nothing beyond the check they exist for. + */ +const INSTALL_STATE_FILES = [ + '.modules.yaml', + '.package-map.json', + '.pnpm-workspace-state-v1.json', +]; + +/** + * The commands that install packages, and therefore the only ones that can hang + * this way. Every other command — `pnpm run start`, which the tests use to + * launch the app they are testing, above all — is left alone entirely: it keeps + * running long after pnpm has said that it is done, and killing it is the last + * thing we want. + */ +const INSTALL_COMMANDS = new Set([ + 'add', + 'dedupe', + 'fetch', + 'i', + 'import', + 'install', + 'link', + 'prune', + 'remove', + 'rm', + 'un', + 'uninstall', + 'unlink', + 'up', + 'update', +]); + +/** + * How much of a chunk of output has to be kept around to match `DONE_PATTERN` + * against the next one: anything at least as long as the strings it looks for. + */ +const TAIL_LENGTH = 32; + +const pnpmArgs = process.argv.slice(2); + +/** + * These tests only ever run pnpm as `pnpm [flags]`, so the first + * argument that isn't a flag is the command. + */ +const watchForHang = INSTALL_COMMANDS.has( + pnpmArgs.find((arg) => !arg.startsWith('-')), +); + +/** + * The directory the launcher that runs this file lives in, which the launcher + * itself tells us about: it is somewhere else entirely, so this file cannot + * work it out on its own. + */ +const LAUNCHER_DIR = process.env.FORGE_PNPM_EXIT_SHIM_DIR; +if (!LAUNCHER_DIR) { + throw new Error( + 'FORGE_PNPM_EXIT_SHIM_DIR must point at the directory this shim is installed in', + ); +} + +/** + * The names a real pnpm can have on `PATH`, in the order the OS would pick + * between them. + */ +const PNPM_FILENAMES = + process.platform === 'win32' + ? ['pnpm.exe', 'pnpm.cmd', 'pnpm.bat'] + : ['pnpm']; + +// Windows spells this `Path`, and `process.env` only hides the difference until +// it gets copied. +const pathKey = + Object.keys(process.env).find((key) => key.toUpperCase() === 'PATH') ?? + 'PATH'; + +/** + * `PATH` without the directory this shim is installed in, which is both where + * we look for the pnpm we are standing in for and what we hand to it: leaving + * ourselves in would let anything that resolves `pnpm` through `PATH` — this + * shim included — end up back here. + */ +const realPath = (process.env[pathKey] ?? '') + .split(path.delimiter) + .filter((dir) => dir !== '' && !isLauncherDir(dir)); + +function isLauncherDir(dir) { + const [candidate, launcher] = [path.resolve(dir), path.resolve(LAUNCHER_DIR)]; + return process.platform === 'win32' + ? candidate.toLowerCase() === launcher.toLowerCase() + : candidate === launcher; +} + +const realPnpm = realPath + .flatMap((dir) => PNPM_FILENAMES.map((filename) => path.join(dir, filename))) + .find((candidate) => fs.existsSync(candidate)); +if (!realPnpm) { + throw new Error(`pnpm not found on PATH (${realPath.join(path.delimiter)})`); +} + +function report(message) { + process.stderr.write(`\n[verdaccio harness] ${message}\n`); +} + +/** + * Runs pnpm once. Resolves with how it went: either it exited on its own, and + * with what status, or it reported that it was done and had to be killed. + */ +function runPnpm() { + return new Promise((resolve, reject) => { + const pnpm = spawn(realPnpm, pnpmArgs, { + // Watching for the hang means reading pnpm's output on the way past. + // Anything we are not watching gets the real thing's streams, untouched. + stdio: watchForHang ? ['inherit', 'pipe', 'pipe'] : 'inherit', + env: { ...process.env, [pathKey]: realPath.join(path.delimiter) }, + // Run pnpm in its own process group so that we can take down the version + // of itself that it hands over to along with it. + detached: process.platform !== 'win32', + // `.cmd` and `.bat` files can only be run through a shell. + shell: process.platform === 'win32', + }); + + let exitTimer; + let hung = false; + + function killPnpm() { + if (process.platform === 'win32') { + spawnSync('taskkill', ['/pid', String(pnpm.pid), '/t', '/f'], { + stdio: 'ignore', + }); + } else { + try { + process.kill(-pnpm.pid, 'SIGKILL'); + } catch { + pnpm.kill('SIGKILL'); + } + } + } + + /** + * Pass pnpm's output through untouched — the tests read it — while watching + * for the point where it has nothing left to do. `DONE_PATTERN` can + * straddle two chunks, so each chunk is matched together with the tail of + * the one before it. + */ + function forward(stream, chunk, previousTail) { + stream.write(chunk); + + const text = `${previousTail}${chunk}`; + if (exitTimer === undefined && DONE_PATTERN.test(text)) { + exitTimer = setTimeout(() => { + report( + `pnpm did not exit ${EXIT_GRACE_PERIOD_MS / 1000}s after reporting that it was done, killing it (https://github.com/pnpm/pnpm/issues/13617)`, + ); + hung = true; + killPnpm(); + }, EXIT_GRACE_PERIOD_MS); + } + + return text.slice(-TAIL_LENGTH); + } + + if (watchForHang) { + let stdoutTail = ''; + pnpm.stdout.on('data', (chunk) => { + stdoutTail = forward(process.stdout, chunk, stdoutTail); + }); + + let stderrTail = ''; + pnpm.stderr.on('data', (chunk) => { + stderrTail = forward(process.stderr, chunk, stderrTail); + }); + } + + pnpm.on('error', (error) => { + clearTimeout(exitTimer); + reject(error); + }); + + // pnpm runs in its own process group, so whoever kills this shim would + // leave it running behind us. + for (const signal of ['SIGINT', 'SIGTERM']) { + process.on(signal, () => { + clearTimeout(exitTimer); + killPnpm(); + process.exitCode = 1; + }); + } + + // Once pnpm is gone its output pipes close and whatever it had already + // written has been flushed. + pnpm.on('close', (code, signal) => { + clearTimeout(exitTimer); + resolve({ hung, code: signal ? 1 : (code ?? 0) }); + }); + }); +} + +/** + * Makes pnpm stop believing that the project it just installed into is already + * up to date, so that the next install checks what is actually there. pnpm is + * run from the directory it installs into throughout these tests. + */ +async function discardInstallState() { + await Promise.all( + INSTALL_STATE_FILES.map((file) => + fs.promises.rm(path.join(process.cwd(), 'node_modules', file), { + force: true, + }), + ), + ); +} + +for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) { + const { hung, code } = await runPnpm(); + + if (!hung) { + process.exitCode = code; + break; + } + + if (attempt === MAX_ATTEMPTS) { + report( + `pnpm hung on every one of ${MAX_ATTEMPTS} attempts, so \`node_modules\` may be missing whatever it was writing when it was killed`, + ); + process.exitCode = 1; + break; + } + + await discardInstallState(); + report( + `installing again to replace whatever pnpm was still writing when it was killed (attempt ${attempt + 1} of ${MAX_ATTEMPTS})`, + ); +} diff --git a/tools/verdaccio/spawn-verdaccio.ts b/tools/verdaccio/spawn-verdaccio.ts index 360831dcf0..1d8f08a561 100644 --- a/tools/verdaccio/spawn-verdaccio.ts +++ b/tools/verdaccio/spawn-verdaccio.ts @@ -260,6 +260,46 @@ async function runCommand(args: string[]) { ), ); + /** + * pnpm can finish an install and then never exit, which costs us a whole test + * because the tests wait for the package manager they spawned to be done. + * `pnpm-exit-shim.mjs` explains why and works around it; putting it in front + * of the real pnpm on `PATH` covers every pnpm these tests run, whether they + * run it themselves or through `create-electron-app`. + */ + const tempBinDir = path.join(STORAGE_PATH, '.bin'); + await fs.promises.mkdir(tempBinDir, { recursive: true }); + const pnpmExitShim = path.resolve(import.meta.dirname, 'pnpm-exit-shim.mjs'); + if (process.platform === 'win32') { + await fs.promises.writeFile( + path.join(tempBinDir, 'pnpm.cmd'), + [ + '@echo off', + `set "FORGE_PNPM_EXIT_SHIM_DIR=${tempBinDir}"`, + `node "${pnpmExitShim}" %*`, + '', + ].join('\r\n'), + ); + } else { + const pnpmLauncher = path.join(tempBinDir, 'pnpm'); + await fs.promises.writeFile( + pnpmLauncher, + [ + '#!/bin/sh', + `FORGE_PNPM_EXIT_SHIM_DIR="${tempBinDir}"`, + 'export FORGE_PNPM_EXIT_SHIM_DIR', + `exec node "${pnpmExitShim}" "$@"`, + '', + ].join('\n'), + ); + await fs.promises.chmod(pnpmLauncher, 0o755); + } + // Windows spells this `Path`, and spreading `process.env` above lost the + // case-insensitive lookup that hides the difference. + const pathKey = + Object.keys(parentEnv).find((key) => key.toUpperCase() === 'PATH') ?? + 'PATH'; + /** * npm only learned about `min-release-age` in 11.19. Older versions install * without a gate and warn that the config is unknown on every single npm @@ -284,6 +324,7 @@ async function runCommand(args: string[]) { stdio: 'inherit', env: { ...parentEnv, + [pathKey]: [tempBinDir, parentEnv[pathKey]].join(path.delimiter), // https://docs.npmjs.com/cli/v9/using-npm/config#registry // https://pnpm.io/settings#registry NPM_CONFIG_REGISTRY: VERDACCIO_URL, From 8e5e55c0a0948dfe6d4a7d6b7677cc4cb1bc66a8 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Thu, 20 Aug 2026 09:10:27 -0700 Subject: [PATCH 07/11] test: report what pnpm printed when a template project fails to start MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows keeps producing projects whose `node_modules` has the base template's dependencies and none of the template's own, and nothing says why: `create-electron-app` installs through listr2 with `exitOnError: false` and never prints the errors it collects, so a failed install leaves the task without a tick and without a word about it, and the package manager's own output is thrown away with it. So the pnpm shim now records every pnpm it runs — the command, the directory, how it ended and everything it printed — and a test whose app fails to start reports the runs for its own project, along with the `package.json` they were working from, which is what says whether the dependencies were recorded and not installed or never recorded at all. Co-Authored-By: Claude --- .../utils/test-utils/src/template-tests.ts | 29 ++++++++++++++ tools/verdaccio/pnpm-exit-shim.mjs | 39 +++++++++++++++++-- tools/verdaccio/spawn-verdaccio.ts | 6 +++ 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/packages/utils/test-utils/src/template-tests.ts b/packages/utils/test-utils/src/template-tests.ts index 0b0cccd665..6de1e66db4 100644 --- a/packages/utils/test-utils/src/template-tests.ts +++ b/packages/utils/test-utils/src/template-tests.ts @@ -66,6 +66,29 @@ function describeDependencyTree(projectDir: string) { return `${names.length} packages into ${nodeModules}: ${names.join(' ')}`; } +/** + * Everything the Verdaccio harness' pnpm shim recorded about the installs into + * one project, which is the only account there is of them: pnpm's own output. + */ +function describePnpmInstalls(projectDir: string) { + const invocationLog = process.env.FORGE_PNPM_INVOCATION_LOG; + if (!invocationLog) return 'no record of the pnpm runs was kept'; + + let records; + try { + records = fs.readFileSync(invocationLog, 'utf8').split(/^(?==== pnpm )/m); + } catch (error) { + return `no readable record of the pnpm runs in ${invocationLog} (${error})`; + } + + const forThisProject = records.filter((record) => + record.includes(`in ${projectDir}`), + ); + return forThisProject.length + ? forThisProject.join('') + : `nothing in ${invocationLog} mentions ${projectDir}`; +} + /** * Runs the local version of `create-electron-app` to create a project based on * a given Forge template using all supported package managers. Because this @@ -304,6 +327,12 @@ export function testForgeTemplate({ console.error( [ `[template-tests] ${packageManager} installed ${describeDependencyTree(tmpDir)}`, + `[template-tests] from this package.json:\n${fs.readFileSync(path.join(tmpDir, 'package.json'), 'utf8')}`, + ...(packageManager === 'pnpm' + ? [ + `[template-tests] pnpm was run like this:\n${describePnpmInstalls(tmpDir)}`, + ] + : []), `[template-tests] create-electron-app said:\n${createOutput}`, ].join('\n'), ); diff --git a/tools/verdaccio/pnpm-exit-shim.mjs b/tools/verdaccio/pnpm-exit-shim.mjs index 8360459997..d2bd33963d 100644 --- a/tools/verdaccio/pnpm-exit-shim.mjs +++ b/tools/verdaccio/pnpm-exit-shim.mjs @@ -158,11 +158,34 @@ function report(message) { process.stderr.write(`\n[verdaccio harness] ${message}\n`); } +/** + * Where to write a record of every pnpm run. `create-electron-app` installs + * through listr2 with `exitOnError: false` and does not forward what the + * package manager printed, so a failing test otherwise has nothing to say about + * the installs that produced the project it is failing on. + */ +const INVOCATION_LOG = process.env.FORGE_PNPM_INVOCATION_LOG; + +function recordInvocation(attempt, output, outcome) { + if (!INVOCATION_LOG) return; + + fs.appendFileSync( + INVOCATION_LOG, + [ + `=== pnpm ${pnpmArgs.join(' ')}`, + ` in ${process.cwd()}${attempt > 1 ? ` (attempt ${attempt})` : ''}`, + ` ${outcome}`, + output, + '', + ].join('\n'), + ); +} + /** * Runs pnpm once. Resolves with how it went: either it exited on its own, and * with what status, or it reported that it was done and had to be killed. */ -function runPnpm() { +function runPnpm(attempt) { return new Promise((resolve, reject) => { const pnpm = spawn(realPnpm, pnpmArgs, { // Watching for the hang means reading pnpm's output on the way past. @@ -178,6 +201,7 @@ function runPnpm() { let exitTimer; let hung = false; + let output = ''; function killPnpm() { if (process.platform === 'win32') { @@ -201,6 +225,7 @@ function runPnpm() { */ function forward(stream, chunk, previousTail) { stream.write(chunk); + output += chunk; const text = `${previousTail}${chunk}`; if (exitTimer === undefined && DONE_PATTERN.test(text)) { @@ -247,7 +272,15 @@ function runPnpm() { // written has been flushed. pnpm.on('close', (code, signal) => { clearTimeout(exitTimer); - resolve({ hung, code: signal ? 1 : (code ?? 0) }); + const exitCode = signal ? 1 : (code ?? 0); + recordInvocation( + attempt, + output, + hung + ? 'reported that it was done and then had to be killed' + : `exited with ${exitCode}`, + ); + resolve({ hung, code: exitCode }); }); }); } @@ -268,7 +301,7 @@ async function discardInstallState() { } for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) { - const { hung, code } = await runPnpm(); + const { hung, code } = await runPnpm(attempt); if (!hung) { process.exitCode = code; diff --git a/tools/verdaccio/spawn-verdaccio.ts b/tools/verdaccio/spawn-verdaccio.ts index 1d8f08a561..8504955dd4 100644 --- a/tools/verdaccio/spawn-verdaccio.ts +++ b/tools/verdaccio/spawn-verdaccio.ts @@ -325,6 +325,12 @@ async function runCommand(args: string[]) { env: { ...parentEnv, [pathKey]: [tempBinDir, parentEnv[pathKey]].join(path.delimiter), + // Where the shim records what every pnpm it ran printed, which is what a + // failing test reports when the project it was given cannot start. + FORGE_PNPM_INVOCATION_LOG: path.join( + STORAGE_PATH, + 'pnpm-invocations.log', + ), // https://docs.npmjs.com/cli/v9/using-npm/config#registry // https://pnpm.io/settings#registry NPM_CONFIG_REGISTRY: VERDACCIO_URL, From 268c2ad7e383421ac7c34984f87db29f1429b25b Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Thu, 20 Aug 2026 09:45:55 -0700 Subject: [PATCH 08/11] fix: stop cmd.exe from eating the `^` in version ranges the pnpm shim passes on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows the real pnpm is a `.cmd` file, so the shim ran it through `cmd.exe`, which Node hands the command line to unquoted. `cmd.exe` reads `^` as its own escape character, so `pnpm add typescript@^6.0.0` reached pnpm as `typescript@6.0.0` — a version that does not exist — and the install failed with `ERR_PNPM_NO_MATCHING_VERSION`. Every other dependency the templates ask for happens to have a release at exactly the version its range starts from, which is why only the two TypeScript templates failed, and why the projects they left behind had `"@electron/fuses": "2.0.0"` where every other platform gets `"^2.1.3"`. `create-electron-app` runs its steps with listr2's `exitOnError: false` and never prints the errors it collects, so all of this was silent: the install failed, the project was left without the template's own dependencies, and the test only found out when the app could not resolve its Forge configuration. Hand the spawning to `cross-spawn`, which quotes and escapes arguments the way `cmd.exe` needs and is what Forge itself runs package managers with. Co-Authored-By: Claude --- knip.json | 4 ++-- tools/verdaccio/pnpm-exit-shim.mjs | 14 +++++++++++--- yarn.lock | 1 + 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/knip.json b/knip.json index dbf3844a3a..815bb6b25f 100644 --- a/knip.json +++ b/knip.json @@ -4,8 +4,8 @@ "ignoreExportsUsedInFile": true, "workspaces": { ".": { - "entry": ["tools/*.ts", "tools/verdaccio/*.ts"], - "project": ["tools/**/*.ts"], + "entry": ["tools/*.ts", "tools/verdaccio/*.ts", "tools/verdaccio/*.mjs"], + "project": ["tools/**/*.{ts,mjs}"], "ignoreDependencies": [ "@types/keyv", "electron", diff --git a/tools/verdaccio/pnpm-exit-shim.mjs b/tools/verdaccio/pnpm-exit-shim.mjs index d2bd33963d..b6d710940c 100644 --- a/tools/verdaccio/pnpm-exit-shim.mjs +++ b/tools/verdaccio/pnpm-exit-shim.mjs @@ -25,10 +25,20 @@ * Delete this shim and its wiring once the fix has shipped in a pnpm release. */ -import { spawn, spawnSync } from 'node:child_process'; +import { spawnSync } from 'node:child_process'; import fs from 'node:fs'; import path from 'node:path'; +/** + * On Windows the real pnpm is a `.cmd` file, which can only be run through + * `cmd.exe`, and Node passes it the command line unquoted: `cmd.exe` then reads + * the `^` in a version range like `typescript@^6.0.0` as its own escape + * character and asks pnpm for `typescript@6.0.0`, a version that need not + * exist. `cross-spawn` quotes and escapes arguments the way `cmd.exe` needs, + * and is what Forge itself runs package managers with. + */ +import spawn from 'cross-spawn'; + /** * How long pnpm gets to exit by itself after it says it is done. Long enough * that we don't cut a slow-but-healthy shutdown short, short enough that a hang @@ -195,8 +205,6 @@ function runPnpm(attempt) { // Run pnpm in its own process group so that we can take down the version // of itself that it hands over to along with it. detached: process.platform !== 'win32', - // `.cmd` and `.bat` files can only be run through a shell. - shell: process.platform === 'win32', }); let exitTimer; diff --git a/yarn.lock b/yarn.lock index e8be4ea658..e5920ab099 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8447,6 +8447,7 @@ __metadata: "@types/which": "npm:^3.0.4" "@typescript/native": "npm:typescript@^7.0.0" "@yarnpkg/types": "npm:^4.0.1" + cross-spawn: "npm:^7.0.6" debug: "npm:^4.3.1" electron: "npm:^42.3.3" electron-installer-debian: "npm:^3.2.0" From e32241c18ac111c86695a2d246588928ab2ca378 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Thu, 20 Aug 2026 09:57:27 -0700 Subject: [PATCH 09/11] build: declare the `cross-spawn` the pnpm shim imports The manifest entry was left out of the commit that started importing it, so installs with a frozen lockfile failed. `^7.0.3` is the range the rest of the project asks for, which `yarn constraints` requires it to match. Co-Authored-By: Claude --- package.json | 1 + yarn.lock | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 1017ba0eed..a77d80a90d 100644 --- a/package.json +++ b/package.json @@ -52,6 +52,7 @@ "@types/which": "^3.0.4", "@typescript/native": "npm:typescript@^7.0.0", "@yarnpkg/types": "^4.0.1", + "cross-spawn": "^7.0.3", "debug": "^4.3.1", "electron": "^42.3.3", "fork-ts-checker-webpack-plugin": "^7.2.13", diff --git a/yarn.lock b/yarn.lock index e5920ab099..e758d82de3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8447,7 +8447,7 @@ __metadata: "@types/which": "npm:^3.0.4" "@typescript/native": "npm:typescript@^7.0.0" "@yarnpkg/types": "npm:^4.0.1" - cross-spawn: "npm:^7.0.6" + cross-spawn: "npm:^7.0.3" debug: "npm:^4.3.1" electron: "npm:^42.3.3" electron-installer-debian: "npm:^3.2.0" From c8091d6dc5e8115dba575ba045078cd2e32506b0 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Thu, 20 Aug 2026 11:11:34 -0700 Subject: [PATCH 10/11] fix: install with a pnpm that keeps the whole dependency tree `create-electron-app` pins every pnpm app it creates to the pnpm version this repository runs in CI, and until pnpm 11.18 adding a dependency to a project that already had some could drop a package that another package it kept still depends on. The last of the four installs `create-electron-app` runs left `rimraf` in `node_modules` without the `glob` it requires, so `forge.config.ts` could no longer load and the Verdaccio template tests could not start the app they had just created. It only showed up on Windows, for two reasons: everywhere else the platform-specific makers' dependencies pull `glob` in through a second path that keeps it in the tree, and Corepack pins `pnpm@latest` for the app it creates, which is much newer than this pin. Corepack fails on the Windows runners, so there the pin is what actually installs. Co-Authored-By: Claude --- .github/workflows/ci.yml | 4 ++-- packages/template/base/src/BaseTemplate.ts | 9 +++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7b8d82cf13..9344102ed2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -125,7 +125,7 @@ jobs: sudo add-apt-repository -y ppa:alexlarsson/flatpak - name: Install pnpm - run: npm install -g pnpm@11.10.0 + run: npm install -g pnpm@11.21.0 - name: Run fast tests run: | @@ -204,7 +204,7 @@ jobs: sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 - name: Install pnpm - run: npm install -g pnpm@11.10.0 + run: npm install -g pnpm@11.21.0 # The Verdaccio tests age-gate their installs like the root `.yarnrc.yml` # does, and npm only learned about `min-release-age` in 11.19, which is diff --git a/packages/template/base/src/BaseTemplate.ts b/packages/template/base/src/BaseTemplate.ts index 4f7647ba5f..e40a246178 100644 --- a/packages/template/base/src/BaseTemplate.ts +++ b/packages/template/base/src/BaseTemplate.ts @@ -174,9 +174,14 @@ export class BaseTemplate implements ForgeTemplate { const pm = await resolvePackageManager(); if (pm.executable === 'pnpm') { - // Ensures we're using the same `pnpm` version that we use in CI. + // Ensures we're using the same `pnpm` version that we use in CI, and + // never one older than 11.18: before that, adding a dependency to a + // project that already had some could drop a package that another one + // it kept still depends on. That left `forge.config.ts` unable to load + // on Windows, where the platform-specific makers' dependencies are + // skipped and `rimraf` is the only thing left asking for `glob`. packageJSON.devEngines = { - packageManager: 'pnpm@11.10.0', + packageManager: 'pnpm@11.21.0', }; // Ensures all transitive dependencies for `electron-winstaller` are From ccfc3c51b89ea39f881175bba5b8933a0681ea5d Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Mon, 24 Aug 2026 19:07:17 -0700 Subject: [PATCH 11/11] fix: gate the last two ungated installs in CI The Verdaccio harness now fails when npm cannot enforce `min-release-age` in CI instead of warning and installing without a gate, and checks before publishing anything so that failure costs two seconds. The floor is 11.17, not 11.19: npm added `min-release-age` in 11.10 (npm/cli#8965) and `min-release-age-exclude` in 11.17 (npm/cli#9534). Two installs were still resolving from the public registry with no lockfile and no gate. Both now pass a `before` date, which the npm bundled with the Node version in `.nvmrc` understands and `min-release-age` is too new for: - `npm install node-gyp@9.4.0` in the Windows setup step of both test jobs, where the pin is exact but its dependencies were resolved fresh every run. - `npm install debug@^2.0.0` in `install-dependencies.slow.spec.ts`, where the caret would pick up a new 2.x the moment one was published. Two comments also claimed things that are not true, one of them hiding a real gap. pnpm has no minimum release age of its own, so the reason for passing `XDG_CONFIG_HOME` through to `start` is the harness config it carries. And pnpm ignores `devEngines.packageManager` unless it is written in the object form, while Corepack writes a `packageManager` field that takes precedence regardless, so that field is documented as a record of the version CI installs rather than the floor it cannot enforce. Co-Authored-By: Claude --- .github/workflows/ci.yml | 26 +++++++--- .../slow/install-dependencies.slow.spec.ts | 29 ++++++++++- packages/template/base/src/BaseTemplate.ts | 18 ++++--- .../utils/test-utils/src/template-tests.ts | 14 +++-- tools/verdaccio/spawn-verdaccio.ts | 51 ++++++++++++------- 5 files changed, 98 insertions(+), 40 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9344102ed2..16daa3d5d9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -101,12 +101,18 @@ jobs: name: dist-files path: packages + # `node-gyp` itself is pinned, but its dependencies are resolved fresh on + # every run, so `--before` holds them to the same week-old floor the root + # `.yarnrc.yml` applies to Yarn. It does that through `--before` rather + # than `min-release-age` because this step runs under the npm bundled with + # the Node version in `.nvmrc`, which is older than either age-gate + # setting and would ignore both without failing. - name: Windows setup if: runner.os == 'Windows' shell: bash run: | cd "$PROGRAMFILES/nodejs/node_modules/npm/node_modules/@npmcli/run-script" - npm install node-gyp@9.4.0 + npm install node-gyp@9.4.0 --before "$(date -u -d '7 days ago' +%Y-%m-%d)" - name: Linux setup if: runner.os == 'Linux' @@ -180,12 +186,18 @@ jobs: name: dist-files path: packages + # `node-gyp` itself is pinned, but its dependencies are resolved fresh on + # every run, so `--before` holds them to the same week-old floor the root + # `.yarnrc.yml` applies to Yarn. It does that through `--before` rather + # than `min-release-age` because this step runs under the npm bundled with + # the Node version in `.nvmrc`, which is older than either age-gate + # setting and would ignore both without failing. - name: Windows setup if: runner.os == 'Windows' shell: bash run: | cd "$PROGRAMFILES/nodejs/node_modules/npm/node_modules/@npmcli/run-script" - npm install node-gyp@9.4.0 + npm install node-gyp@9.4.0 --before "$(date -u -d '7 days ago' +%Y-%m-%d)" - name: Linux setup if: runner.os == 'Linux' @@ -207,10 +219,12 @@ jobs: run: npm install -g pnpm@11.21.0 # The Verdaccio tests age-gate their installs like the root `.yarnrc.yml` - # does, and npm only learned about `min-release-age` in 11.19, which is - # newer than the npm bundled with the Node version in `.nvmrc`. npm 12 - # requires Node `^22.22.2 || ^24.15.0 || >=26`, so it cannot be installed - # on that Node version at all; bump this pin when `.nvmrc` moves. + # does, which needs npm 11.17: 11.10 added `min-release-age` + # (npm/cli#8965) and 11.17 added `min-release-age-exclude` + # (npm/cli#9534). Both are newer than the npm bundled with the Node + # version in `.nvmrc`. npm 12 requires Node + # `^22.22.2 || ^24.15.0 || >=26`, so it cannot be installed on that Node + # version at all; bump this pin when `.nvmrc` moves. - name: Install npm run: npm install -g npm@11.19.0 diff --git a/packages/api/core/spec/slow/install-dependencies.slow.spec.ts b/packages/api/core/spec/slow/install-dependencies.slow.spec.ts index 61fd6d9958..fc089d58ea 100644 --- a/packages/api/core/spec/slow/install-dependencies.slow.spec.ts +++ b/packages/api/core/spec/slow/install-dependencies.slow.spec.ts @@ -6,9 +6,29 @@ import { installDependencies, PACKAGE_MANAGERS, } from '@electron-forge/core-utils'; -import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { pathToFileURL } from 'node:url'; +/** + * How old a release has to be before this test will install it, matching the + * `npmMinimalAgeGate` the root `.yarnrc.yml` holds Yarn to. This test installs + * from the public registry with no lockfile, so without a floor the caret below + * picks up a brand new `2.x` the moment one is published. + * + * It holds it there with npm's `before` config — through the environment, since + * `installDependencies` does not pass flags through — rather than npm's own + * `min-release-age`, because this test also runs under the npm bundled with the + * Node version in `.nvmrc`, which is older than that setting and would ignore it + * without failing. + */ +const MINIMUM_RELEASE_AGE_DAYS = 7; + +function releasedBefore(days: number): string { + const cutoff = new Date(); + cutoff.setUTCDate(cutoff.getUTCDate() - days); + return cutoff.toISOString(); +} + describe.runIf(!(process.platform === 'linux' && process.env.CI))( 'install-dependencies', () => { @@ -21,6 +41,8 @@ describe.runIf(!(process.platform === 'linux' && process.env.CI))( }); it('should install the latest minor version when the dependency has a caret', async () => { + vi.stubEnv('npm_config_before', releasedBefore(MINIMUM_RELEASE_AGE_DAYS)); + await installDependencies(PACKAGE_MANAGERS['npm'], installDir, [ 'debug@^2.0.0', ]); @@ -33,6 +55,9 @@ describe.runIf(!(process.platform === 'linux' && process.env.CI))( expect(packageJSON.version).not.toEqual('2.0.0'); }); - afterAll(async () => fs.rm(installDir, { recursive: true, force: true })); + afterAll(async () => { + vi.unstubAllEnvs(); + await fs.rm(installDir, { recursive: true, force: true }); + }); }, ); diff --git a/packages/template/base/src/BaseTemplate.ts b/packages/template/base/src/BaseTemplate.ts index e40a246178..44348a46bb 100644 --- a/packages/template/base/src/BaseTemplate.ts +++ b/packages/template/base/src/BaseTemplate.ts @@ -174,12 +174,18 @@ export class BaseTemplate implements ForgeTemplate { const pm = await resolvePackageManager(); if (pm.executable === 'pnpm') { - // Ensures we're using the same `pnpm` version that we use in CI, and - // never one older than 11.18: before that, adding a dependency to a - // project that already had some could drop a package that another one - // it kept still depends on. That left `forge.config.ts` unable to load - // on Windows, where the platform-specific makers' dependencies are - // skipped and `rimraf` is the only thing left asking for `glob`. + // Records the pnpm version this template is known to work with, which is + // the one CI installs. Anything older than 11.18 is known not to: adding a + // dependency to a project that already had some could drop a package that + // another one it kept still depends on, which left `forge.config.ts` + // unable to load on Windows, where the platform-specific makers' + // dependencies are skipped and `rimraf` is the only thing left asking for + // `glob`. + // + // It is a record and not a floor. pnpm ignores `devEngines.packageManager` + // unless it is written in the object form the spec describes, and + // `create-electron-app` then has Corepack write a `packageManager` field, + // which takes precedence over this one either way. packageJSON.devEngines = { packageManager: 'pnpm@11.21.0', }; diff --git a/packages/utils/test-utils/src/template-tests.ts b/packages/utils/test-utils/src/template-tests.ts index 6de1e66db4..65398eee1a 100644 --- a/packages/utils/test-utils/src/template-tests.ts +++ b/packages/utils/test-utils/src/template-tests.ts @@ -272,14 +272,12 @@ export function testForgeTemplate({ PATH: process.env.PATH, /** * `start` makes the package manager check the lockfile it just - * wrote, and pnpm has enforced a minimum release age of its own by - * default since 11.16, so that check rejects the project outright - * whenever one of our dependencies published a release in the last - * day. `XDG_CONFIG_HOME` is where the Verdaccio test harness puts - * the config that tells pnpm which registry to use, how old a - * release has to be, and which packages are exempt, so we have to - * let it through to keep the same policy in force for the install - * and for the check. + * wrote, and `XDG_CONFIG_HOME` is where the Verdaccio test harness + * puts the config that tells pnpm which registry to use, how old a + * release has to be, which packages are exempt, and to warn rather + * than fail when the check finds a difference. Dropping it would + * leave the check looking at the public registry under a policy + * the install never ran under. */ XDG_CONFIG_HOME: process.env.XDG_CONFIG_HOME, ...(process.platform === 'linux' && { diff --git a/tools/verdaccio/spawn-verdaccio.ts b/tools/verdaccio/spawn-verdaccio.ts index 8504955dd4..b206dd91cb 100644 --- a/tools/verdaccio/spawn-verdaccio.ts +++ b/tools/verdaccio/spawn-verdaccio.ts @@ -154,7 +154,36 @@ async function publishPackages(): Promise { } } -async function runCommand(args: string[]) { +/** + * Whether npm understands the age gate these tests hold their installs to. npm + * learned about `min-release-age` in 11.10 and about `min-release-age-exclude` + * in 11.17, so 11.17 is the first version that understands the whole policy. + * Older versions install without a gate and warn that the config is unknown on + * every single npm invocation, so we only pass it when it is supported. + * + * CI installs an npm new enough for it (see the `Install npm` step in + * `ci.yml`), and the gate quietly going missing there — a `.nvmrc` bump that + * drops that step, say — is exactly the failure this policy exists to catch, so + * only a local run is allowed to carry on without it. Checked before anything + * is published so that a CI failure costs a couple of seconds. + */ +async function checkNpmAgeGateSupport(): Promise { + const npmVersion = (await spawnPromise('npm', ['--version'])).trim(); + const [npmMajor, npmMinor] = npmVersion.split('.').map(Number); + const supported = npmMajor > 11 || (npmMajor === 11 && npmMinor >= 17); + + if (!supported) { + const message = `npm ${npmVersion} does not support \`min-release-age\` (npm >= 11.17 required)`; + if (process.env.CI) throw new Error(message); + console.warn( + `⚠️ ${message}, so npm installs in these tests are not age-gated`, + ); + } + + return supported; +} + +async function runCommand(args: string[], npmSupportsAgeGate: boolean) { process.env.COREPACK_ENABLE_STRICT = '0'; /** @@ -300,22 +329,6 @@ async function runCommand(args: string[]) { Object.keys(parentEnv).find((key) => key.toUpperCase() === 'PATH') ?? 'PATH'; - /** - * npm only learned about `min-release-age` in 11.19. Older versions install - * without a gate and warn that the config is unknown on every single npm - * invocation, so we only pass it when it is supported and say once that the - * npm side of the tests is ungated. - */ - const npmVersion = (await spawnPromise('npm', ['--version'])).trim(); - const [npmMajor, npmMinor] = npmVersion.split('.').map(Number); - const npmSupportsAgeGate = - npmMajor > 11 || (npmMajor === 11 && npmMinor >= 19); - if (!npmSupportsAgeGate) { - console.warn( - `⚠️ npm ${npmVersion} does not support \`min-release-age\` (npm >= 11.19 required), so npm installs in these tests are not age-gated`, - ); - } - console.log(`🏃 Running: ${args.join(' ')}`); console.log(` Using registry: ${VERDACCIO_URL}`); @@ -381,6 +394,8 @@ async function main(): Promise { }); try { + const npmSupportsAgeGate = await checkNpmAgeGateSupport(); + await startVerdaccio(); await publishPackages(); @@ -391,7 +406,7 @@ async function main(): Promise { // Keep the process alive await new Promise(() => {}); } else { - await runCommand(args); + await runCommand(args, npmSupportsAgeGate); stopVerdaccio(); process.exit(0); }