Do not run interactive exec in CI when a TTY

Credit: @isaacs PR-URL: #2202 Close: #2202 Reviewed-by: @darcyclarke
npm · Dec 2, 2020 · f864b7b · f864b7b
1 parent 15d7333
commit f864b7b
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 28 deletions.
diff --git a/docs/content/commands/npm-exec.md b/docs/content/commands/npm-exec.md
@@ -18,7 +18,6 @@ npx -c '<cmd> [args...]'
 npx -p <pkg>[@<specifier>] -c '<cmd> [args...]'
 Run without --call or positional args to open interactive subshell
 
-
 alias: npm x, npx
 
 common options:
@@ -35,7 +34,8 @@ as running it via `npm run`.
 
 Run without positional arguments or `--call`, this allows you to
 interactively run commands in the same sort of shell environment that
-`package.json` scripts are run.
+`package.json` scripts are run.  Interactive mode is not supported in CI
+environments when standard input is a TTY, to prevent hangs.
 
 Whatever packages are specified by the `--package` option will be
 provided in the `PATH` of the executed command, along with any locally

diff --git a/lib/exec.js b/lib/exec.js
@@ -83,6 +83,13 @@ const run = async ({ args, call, pathArr, shell }) => {
 
   npm.log.disableProgress()
   try {
+    if (script === shell) {
+      if (process.stdin.isTTY) {
+        if (ciDetect())
+          return npm.log.warn('exec', 'Interactive mode disabled in CI environment')
+        output(`\nEntering npm script environment\nType 'exit' or ^D when finished\n`)
+      }
+    }
     return await runScript({
       ...npm.flatOptions,
       pkg,
@@ -111,7 +118,6 @@ const exec = async args => {
 
   // nothing to maybe install, skip the arborist dance
   if (!call && !args.length && !packages.length) {
-    output(`\nEntering npm script environment\nType 'exit' or ^D when finished\n`)
     return await run({
       args,
       call,
@@ -194,13 +200,12 @@ const exec = async args => {
 
     // no need to install if already present
     if (add.length) {
-      const isTTY = process.stdin.isTTY && process.stdout.isTTY
       if (!npm.flatOptions.yes) {
         // set -n to always say no
         if (npm.flatOptions.yes === false)
           throw 'canceled'
 
-        if (!isTTY || ciDetect()) {
+        if (!process.stdin.isTTY || ciDetect()) {
           npm.log.warn('exec', `The following package${
             add.length === 1 ? ' was' : 's were'
           } not found and will be installed: ${

diff --git a/test/lib/exec.js b/test/lib/exec.js
@@ -198,30 +198,69 @@ t.test('npm exec foo, already present locally', async t => {
 })
 
 t.test('npm exec <noargs>, run interactive shell', async t => {
-  ARB_CTOR.length = 0
-  MKDIRPS.length = 0
-  ARB_REIFY.length = 0
-  OUTPUT.length = 0
-  await exec([], er => {
-    if (er)
-      throw er
+  CI_NAME = null
+  const { isTTY } = process.stdin
+  process.stdin.isTTY = true
+  t.teardown(() => process.stdin.isTTY = isTTY)
+
+  const run = async (t, doRun = true) => {
+    LOG_WARN.length = 0
+    ARB_CTOR.length = 0
+    MKDIRPS.length = 0
+    ARB_REIFY.length = 0
+    OUTPUT.length = 0
+    await exec([], er => {
+      if (er)
+        throw er
+    })
+    t.strictSame(MKDIRPS, [], 'no need to make any dirs')
+    t.strictSame(ARB_CTOR, [], 'no need to instantiate arborist')
+    t.strictSame(ARB_REIFY, [], 'no need to reify anything')
+    t.equal(PROGRESS_ENABLED, true, 'progress re-enabled')
+    if (doRun) {
+      t.match(RUN_SCRIPTS, [{
+        pkg: { scripts: { npx: 'shell-cmd' } },
+        banner: false,
+        path: process.cwd(),
+        stdioString: true,
+        event: 'npx',
+        env: { PATH: process.env.PATH },
+        stdio: 'inherit',
+      }])
+    } else
+      t.strictSame(RUN_SCRIPTS, [])
+    RUN_SCRIPTS.length = 0
+  }
+
+  t.test('print message when tty and not in CI', async t => {
+    CI_NAME = null
+    process.stdin.isTTY = true
+    await run(t)
+    t.strictSame(LOG_WARN, [])
+    t.strictSame(OUTPUT, [
+      ['\nEntering npm script environment\nType \'exit\' or ^D when finished\n'],
+    ], 'printed message about interactive shell')
   })
-  t.strictSame(OUTPUT, [
-    ['\nEntering npm script environment\nType \'exit\' or ^D when finished\n'],
-  ], 'printed message about interactive shell')
-  t.strictSame(MKDIRPS, [], 'no need to make any dirs')
-  t.strictSame(ARB_CTOR, [], 'no need to instantiate arborist')
-  t.strictSame(ARB_REIFY, [], 'no need to reify anything')
-  t.equal(PROGRESS_ENABLED, true, 'progress re-enabled')
-  t.match(RUN_SCRIPTS, [{
-    pkg: { scripts: { npx: 'shell-cmd' } },
-    banner: false,
-    path: process.cwd(),
-    stdioString: true,
-    event: 'npx',
-    env: { PATH: process.env.PATH },
-    stdio: 'inherit',
-  }])
+
+  t.test('no message when not TTY', async t => {
+    CI_NAME = null
+    process.stdin.isTTY = false
+    await run(t)
+    t.strictSame(LOG_WARN, [])
+    t.strictSame(OUTPUT, [], 'no message about interactive shell')
+  })
+
+  t.test('print warning when in CI and interactive', async t => {
+    CI_NAME = 'travis-ci'
+    process.stdin.isTTY = true
+    await run(t, false)
+    t.strictSame(LOG_WARN, [
+      ['exec', 'Interactive mode disabled in CI environment'],
+    ])
+    t.strictSame(OUTPUT, [], 'no message about interactive shell')
+  })
+
+  t.end()
 })
 
 t.test('npm exec foo, not present locally or in central loc', async t => {