fix(appium,fake-driver): expose child process when running an extension script

This change makes it so when running an extension script (e.g., `appium driver foo run some-script`), unless the `--json` flag is present, the child process will do no buffering whatsoever and will inherit `process.stdin`, `process.stdout` and `process.stderr` from Appium.

This means that extension scripts can do stuff like prompt the user for input.  It _also_ means they can do stuff like display progress bars and spinners.

If the user wants JSON data, we revert to the old behavior, which is to buffer the output and dump it once the script has completed.

packages/appium/lib/cli/driver-command.js

@@ -58,7 +58,12 @@ export default class DriverCommand extends ExtensionCommand {
    * @return {Promise<import('./extension-command').RunOutput>}
    */
   async run({driver, scriptName, extraArgs}) {
-    return await super._run({installSpec: driver, scriptName, extraArgs});
+    return await super._run({
+      installSpec: driver,
+      scriptName,
+      extraArgs,
+      bufferOutput: this.isJsonOutput,
+    });
   }
 
   /**

packages/appium/lib/cli/extension-command.js

@@ -4,14 +4,15 @@ import _ from 'lodash';
 import path from 'path';
 import {npm, util, env, console} from '@appium/support';
 import {spinWith, RingBuffer} from './utils';
-import {SubProcess} from 'teen_process';
 import {
   INSTALL_TYPE_NPM,
   INSTALL_TYPE_GIT,
   INSTALL_TYPE_GITHUB,
   INSTALL_TYPE_LOCAL,
 } from '../extension/extension-config';
+import {SubProcess} from 'teen_process';
 import {packageDidChange} from '../extension/package-changed';
+import {spawn} from 'child_process';
 
 const UPDATE_ALL = 'installed';
 
@@ -660,17 +661,34 @@ class ExtensionCommand {
   }
 
   /**
-   * Runs a script cached inside the "scripts" field under "appium"
-   * inside of the driver/plugins "package.json" file. Will throw
-   * an error if the driver/plugin does not contain a "scripts" field
-   * underneath the "appium" field in its package.json, if the
-   * "scripts" field is not a plain object, or if the scriptName is
-   * not found within "scripts" object.
+   * Just wraps {@linkcode child_process.spawn} with some default options
+   *
+   * @param {string} cwd - CWD
+   * @param {string} script - Path to script
+   * @param {string[]} args - Extra args for script
+   * @param {import('child_process').SpawnOptions} opts - Options
+   * @returns {import('node:child_process').ChildProcess}
+   */
+  _runUnbuffered(cwd, script, args = [], opts = {}) {
+    return spawn(process.execPath, [script, ...args], {
+      cwd,
+      stdio: 'inherit',
+      ...opts,
+    });
+  }
+
+  /**
+   * Runs a script cached inside the `scripts` field under `appium`
+   * inside of the extension's `package.json` file. Will throw
+   * an error if the driver/plugin does not contain a `scripts` field
+   * underneath the `appium` field in its `package.json`, if the
+   * `scripts` field is not a plain object, or if the `scriptName` is
+   * not found within `scripts` object.
    *
    * @param {RunOptions} opts
    * @return {Promise<RunOutput>}
    */
-  async _run({installSpec, scriptName, extraArgs = []}) {
+  async _run({installSpec, scriptName, extraArgs = [], bufferOutput = false}) {
     if (!this.config.isInstalled(installSpec)) {
       throw this._createFatalError(`The ${this.type} "${installSpec}" is not installed`);
     }
@@ -699,26 +717,56 @@ class ExtensionCommand {
       );
     }
 
-    const runner = new SubProcess(process.execPath, [extScripts[scriptName], ...extraArgs], {
-      cwd: this.config.getInstallPath(installSpec),
-    });
+    if (bufferOutput) {
+      const runner = new SubProcess(process.execPath, [extScripts[scriptName], ...extraArgs], {
+        cwd: this.config.getInstallPath(installSpec),
+      });
 
-    const output = new RingBuffer(50);
+      const output = new RingBuffer(50);
 
-    runner.on('stream-line', (line) => {
-      output.enqueue(line);
-      this.log.log(line);
-    });
+      runner.on('stream-line', (line) => {
+        output.enqueue(line);
+        this.log.log(line);
+      });
 
-    await runner.start(0);
+      await runner.start(0);
+
+      try {
+        await runner.join();
+        this.log.ok(`${scriptName} successfully ran`.green);
+        return {output: output.getBuff()};
+      } catch (err) {
+        this.log.error(`Encountered an error when running '${scriptName}': ${err.message}`.red);
+        return {error: err.message, output: output.getBuff()};
+      }
+    }
 
     try {
-      await runner.join();
+      await new B((resolve, reject) => {
+        this._runUnbuffered(
+          this.config.getInstallPath(installSpec),
+          extScripts[scriptName],
+          extraArgs
+        )
+          .on('error', (err) => {
+            // generally this is of the "I can't find the script" variety.
+            // this is a developer bug: the extension is pointing to a script that is not where the
+            // developer said it would be (in `appium.scripts` of the extension's `package.json`)
+            reject(err);
+          })
+          .on('close', (code) => {
+            if (code === 0) {
+              resolve();
+            } else {
+              reject(new Error(`Script "${scriptName}" exited with code ${code}`));
+            }
+          });
+      });
       this.log.ok(`${scriptName} successfully ran`.green);
-      return {output: output.getBuff()};
+      return {};
     } catch (err) {
       this.log.error(`Encountered an error when running '${scriptName}': ${err.message}`.red);
-      return {error: err.message, output: output.getBuff()};
+      return {error: err.message};
     }
   }
 }
@@ -802,6 +850,7 @@ export {ExtensionCommand};
  * @property {string} installSpec - name of the extension to run a script from
  * @property {string} scriptName - name of the script to run
  * @property {string[]} [extraArgs] - arguments to pass to the script
+ * @property {boolean} [bufferOutput] - if true, will buffer the output of the script and return it
  */
 
 /**
@@ -809,7 +858,7 @@ export {ExtensionCommand};
  *
  * @typedef RunOutput
  * @property {string} [error] - error message if script ran unsuccessfully, otherwise undefined
- * @property {string[]} output - script output
+ * @property {string[]} [output] - script output if `bufferOutput` was `true` in {@linkcode RunOptions}
  */
 
 /**

packages/appium/lib/cli/plugin-command.js

@@ -57,7 +57,12 @@ export default class PluginCommand extends ExtensionCommand {
    * @returns {Promise<import('./extension-command').RunOutput>}
    */
   async run({plugin, scriptName, extraArgs}) {
-    return await super._run({installSpec: plugin, scriptName, extraArgs});
+    return await super._run({
+      installSpec: plugin,
+      scriptName,
+      extraArgs,
+      bufferOutput: this.isJsonOutput,
+    });
   }
 
   /**

packages/appium/test/unit/extension/extension-command.spec.js

@@ -0,0 +1,64 @@
+// @ts-check
+import {DriverConfig} from '../../../lib/extension/driver-config';
+import {ExtensionCommand} from '../../../lib/cli/extension-command';
+import sinon from 'sinon';
+import {FAKE_DRIVER_DIR} from '../../helpers';
+import {Manifest} from '../../../lib/extension/manifest';
+
+const {expect} = chai;
+
+/**
+ * Relative path from actual `package.json` of `FakeDriver` for the `fake-stdin` script
+ */
+const FAKE_STDIN_SCRIPT = require(`${FAKE_DRIVER_DIR}/package.json`).appium.scripts['fake-stdin'];
+
+describe('ExtensionCommand', function () {
+  describe('method', function () {
+    /** @type {ExtensionCommand} */
+    let ec;
+
+    /** @type {sinon.SinonSandbox} */
+    let sandbox;
+
+    beforeEach(function () {
+      sandbox = sinon.createSandbox();
+      const driverConfig = DriverConfig.create(sandbox.createStubInstance(Manifest));
+      ec = new ExtensionCommand({config: driverConfig, json: false});
+    });
+
+    afterEach(function () {
+      sandbox.restore();
+    });
+
+    describe('_runUnbuffered()', function () {
+      // this test is low value and mostly just asserts that `child_process.spawn()` works.
+      // the problem is that because `_run()` returns a `Promise`, a caller cannot reach the
+      // underlying `ChildProcess` instance.
+      // something like `execa` could work around this because it returns a frankenstein of a
+      // `Promise` + `ChildProcess`, but I didn't want to add the dep.
+      it('should respond to stdin', function (done) {
+        // we have to fake writing to STDIN because this is an automated test, after all.
+        const proc = ec
+          ._runUnbuffered(FAKE_DRIVER_DIR, FAKE_STDIN_SCRIPT, [], {
+            stdio: ['pipe', 'inherit', 'inherit'],
+          })
+          .once('exit', (code) => {
+            try {
+              expect(code).to.equal(0);
+              done();
+            } catch (err) {
+              done(err);
+            }
+          });
+
+        setTimeout(() => {
+          // TS does not understand that `proc.stdin` is not `null`, because it is only a `Writable`
+          // if STDIN is piped from the parent.
+          const stdin = /** @type {import('node:stream').Writable} */ (proc.stdin);
+          stdin.write('\n');
+          stdin.end();
+        }, 200);
+      });
+    });
+  });
+});

packages/fake-driver/lib/scripts/fake-error.js

@@ -1 +1,8 @@
-throw Error('Unsuccessfully ran the script');
+import ora from 'ora';
+
+const spinner = ora('Running fake-error...').start();
+
+setTimeout(() => {
+  spinner.fail('Oh nooooooo!');
+  throw Error('Unsuccessfully ran the script');
+}, 1000);

packages/fake-driver/lib/scripts/fake-stdin.js

@@ -0,0 +1,9 @@
+import readline from 'node:readline';
+
+const rl = readline.createInterface({input: process.stdin, output: process.stderr});
+
+rl.question('Press ENTER to continue: ', () => {
+  rl.close();
+  // eslint-disable-next-line no-console
+  console.error('You did it!');
+});

packages/fake-driver/package.json

@@ -75,7 +75,8 @@
     "schema": "./build/lib/fake-driver-schema.js",
     "scripts": {
       "fake-error": "./build/lib/scripts/fake-error.js",
-      "fake-success": "./build/lib/scripts/fake-success.js"
+      "fake-success": "./build/lib/scripts/fake-success.js",
+      "fake-stdin": "./build/lib/scripts/fake-stdin.js"
     }
   },
   "typedoc": {