fix!: run root preinstall before reify

docs/lib/content/using-npm/scripts.md

@@ -111,33 +111,39 @@ It is run AFTER the changes have been applied and the `package.json` and `packag
 
 #### [`npm ci`](/commands/npm-ci)
 
-* `preinstall`
+* `preinstall` (before dependencies are installed)
 * `install`
 * `postinstall`
 * `prepublish`
 * `preprepare`
 * `prepare`
 * `postprepare`
 
-These all run after the actual installation of modules into
- `node_modules`, in order, with no internal actions happening in between
+`preinstall` runs before any dependencies are fetched or unpacked into
+`node_modules`. The remaining scripts run after the installation of modules
+into `node_modules`, in order, with no internal actions happening in between.
 
 #### [`npm diff`](/commands/npm-diff)
 
 * `prepare`
 
 #### [`npm install`](/commands/npm-install)
 
-These also run when you run `npm install -g <pkg-name>`
+These run on a bare `npm install` in a local project (no package arguments).
 
-* `preinstall`
+* `preinstall` (before dependencies are installed)
 * `install`
 * `postinstall`
 * `prepublish`
 * `preprepare`
 * `prepare`
 * `postprepare`
 
+`preinstall` runs before any dependencies are fetched or unpacked into
+`node_modules`, so scripts can prepare the environment (for example, setting
+up authentication for a private registry) before resolution begins. The
+remaining scripts run after installation has completed.
+
 If there is a `binding.gyp` file in the root of your package and you haven't defined your own `install` or `preinstall` scripts, npm will default the `install` command to compile using node-gyp via `node-gyp rebuild`
 
 These are run from the scripts of `<pkg-name>`

lib/commands/ci.js

@@ -96,28 +96,33 @@ class CI extends ArboristWorkspaceCmd {
       })
     }
 
+    // Root lifecycle scripts for `npm ci` mirror those run by `npm install`. `preinstall` runs *before* reify so that scripts can bootstrap the environment (e.g. private-registry auth) before any dependency is fetched or unpacked. The remaining scripts run after reify as they did before.
+    const scriptShell = this.npm.config.get('script-shell') || undefined
+    const runRootScript = (event) => runScript({
+      path: where,
+      args: [],
+      scriptShell,
+      stdio: 'inherit',
+      event,
+    })
+
+    if (!ignoreScripts) {
+      await runRootScript('preinstall')
+    }
+
     await arb.reify(opts)
 
-    // run the same set of scripts that `npm install` runs.
     if (!ignoreScripts) {
-      const scripts = [
-        'preinstall',
+      const postReifyScripts = [
         'install',
         'postinstall',
         'prepublish', // XXX should we remove this finally??
         'preprepare',
         'prepare',
         'postprepare',
       ]
-      const scriptShell = this.npm.config.get('script-shell') || undefined
-      for (const event of scripts) {
-        await runScript({
-          path: where,
-          args: [],
-          scriptShell,
-          stdio: 'inherit',
-          event,
-        })
+      for (const event of postReifyScripts) {
+        await runRootScript(event)
       }
     }
     await reifyFinish(this.npm, arb)

lib/commands/install.js

@@ -142,27 +142,35 @@ class Install extends ArboristWorkspaceCmd {
       add: args,
       workspaces: this.workspaceNames,
     }
+
+    // Root lifecycle scripts only run for a bare `npm install` in a local project. `preinstall` runs *before* Arborist touches the filesystem so that scripts can bootstrap the environment (e.g. set up private-registry auth, generate files consumed during resolution) before dependencies are fetched or unpacked. The remaining scripts run after reify as they did before.
+    const runRootLifecycle = !args.length && !isGlobalInstall && !ignoreScripts
+    const runRootScript = (event) => runScript({
+      path: where,
+      args: [],
+      scriptShell,
+      stdio: 'inherit',
+      event,
+    })
+
+    if (runRootLifecycle) {
+      await runRootScript('preinstall')
+    }
+
     const arb = new Arborist(opts)
     await arb.reify(opts)
 
-    if (!args.length && !isGlobalInstall && !ignoreScripts) {
-      const scripts = [
-        'preinstall',
+    if (runRootLifecycle) {
+      const postReifyScripts = [
         'install',
         'postinstall',
         'prepublish', // XXX(npm9) should we remove this finally??
         'preprepare',
         'prepare',
         'postprepare',
       ]
-      for (const event of scripts) {
-        await runScript({
-          path: where,
-          args: [],
-          scriptShell,
-          stdio: 'inherit',
-          event,
-        })
+      for (const event of postReifyScripts) {
+        await runRootScript(event)
       }
     }
     await reifyFinish(this.npm, arb)

test/lib/commands/ci.js

@@ -182,6 +182,46 @@ t.test('lifecycle scripts', async t => {
   ], 'runs appropriate scripts, in order')
 })
 
+// Regression test: `npm ci` must run root `preinstall` before reify populates node_modules, matching `npm install` behavior.
+t.test('preinstall runs before reify for npm ci', async t => {
+  const events = []
+  const { npm, registry } = await loadMockNpm(t, {
+    prefixDir: {
+      abbrev: abbrev,
+      'package.json': JSON.stringify({
+        ...packageJson,
+        scripts: {
+          preinstall: 'echo preinstall',
+          postinstall: 'echo postinstall',
+        },
+      }),
+      'package-lock.json': JSON.stringify(packageLock),
+    },
+    mocks: {
+      '@npmcli/run-script': (opts) => {
+        if (opts.path === npm.prefix) {
+          const abbrevPkg = path.join(npm.prefix, 'node_modules', 'abbrev', 'package.json')
+          events.push({ event: opts.event, depInstalled: fs.existsSync(abbrevPkg) })
+        }
+      },
+    },
+  })
+  const manifest = registry.manifest({ name: 'abbrev' })
+  await registry.tarball({
+    manifest: manifest.versions['1.0.0'],
+    tarball: path.join(npm.prefix, 'abbrev'),
+  })
+  registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {})
+  await npm.exec('ci', [])
+
+  const pre = events.find(e => e.event === 'preinstall')
+  const post = events.find(e => e.event === 'postinstall')
+  t.ok(pre, 'preinstall ran')
+  t.ok(post, 'postinstall ran')
+  t.equal(pre.depInstalled, false, 'preinstall runs before dependencies are installed')
+  t.equal(post.depInstalled, true, 'postinstall runs after dependencies are installed')
+})
+
 t.test('should throw if package-lock.json is missing', async t => {
   const { npm } = await loadMockNpm(t, {
     prefixDir: {

test/lib/commands/install.js

@@ -101,6 +101,82 @@ t.test('exec commands', async t => {
     t.strictSame(lifecycleScripts, runOrder, 'all script ran in the correct order')
   })
 
+  // Regression test: root `preinstall` must run before any dependency is fetched/unpacked, while `install` and `postinstall` run after reify has populated node_modules.
+  await t.test('preinstall runs before reify, post-reify scripts run after', async t => {
+    const events = []
+    const { npm, registry } = await loadMockNpm(t, {
+      config: { audit: false },
+      prefixDir: {
+        'package.json': JSON.stringify({
+          ...packageJson,
+          scripts: {
+            preinstall: 'echo preinstall',
+            install: 'echo install',
+            postinstall: 'echo postinstall',
+          },
+        }),
+        abbrev,
+      },
+      mocks: {
+        '@npmcli/run-script': async (opts) => {
+          // Only record scripts targeted at the project root, not any that arborist may run for dependencies during reify.
+          if (opts.path === npm.prefix) {
+            const abbrevPkg = path.join(npm.prefix, 'node_modules', 'abbrev', 'package.json')
+            events.push({ event: opts.event, depInstalled: fs.existsSync(abbrevPkg) })
+          }
+        },
+      },
+    })
+    const manifest = registry.manifest({ name: 'abbrev' })
+    await registry.package({ manifest })
+    await registry.tarball({
+      manifest: manifest.versions['1.0.0'],
+      tarball: path.join(npm.prefix, 'abbrev'),
+    })
+
+    await npm.exec('install')
+
+    const pre = events.find(e => e.event === 'preinstall')
+    const post = events.find(e => e.event === 'postinstall')
+    t.ok(pre, 'preinstall ran')
+    t.ok(post, 'postinstall ran')
+    t.equal(pre.depInstalled, false, 'preinstall runs before dependencies are installed')
+    t.equal(post.depInstalled, true, 'postinstall runs after dependencies are installed')
+  })
+
+  await t.test('without args, --ignore-scripts skips preinstall entirely', async t => {
+    const events = []
+    const { npm, registry } = await loadMockNpm(t, {
+      config: { audit: false, 'ignore-scripts': true },
+      prefixDir: {
+        'package.json': JSON.stringify({
+          ...packageJson,
+          scripts: {
+            preinstall: 'echo preinstall',
+            postinstall: 'echo postinstall',
+          },
+        }),
+        abbrev,
+      },
+      mocks: {
+        '@npmcli/run-script': async (opts) => {
+          if (opts.path === npm.prefix) {
+            events.push(opts.event)
+          }
+        },
+      },
+    })
+    const manifest = registry.manifest({ name: 'abbrev' })
+    await registry.package({ manifest })
+    await registry.tarball({
+      manifest: manifest.versions['1.0.0'],
+      tarball: path.join(npm.prefix, 'abbrev'),
+    })
+
+    await npm.exec('install')
+    t.strictSame(events, [], 'no root lifecycle scripts run when --ignore-scripts is set')
+  })
+
   await t.test('should ignore scripts with --ignore-scripts', async t => {
     const { npm, registry } = await loadMockNpm(t, {
       config: {