fix(cli): four commands that reported success without doing anything

script/command-executor.ts

@@ -80,6 +80,12 @@ let connectionInfo: ILoginConnectionInfo;
 export const confirm = (message: string = "Are you sure?"): Promise<boolean> => {
   message += " (y/N):";
   return Promise<boolean>((resolve, reject, notify): void => {
+    if (!process.stdin.isTTY) {
+      log(chalk.cyan(message) + " no (not a terminal, so nothing was asked). Nothing was changed.");
+      resolve(false);
+      return;
+    }
+
     prompt.message = "";
     prompt.delimiter = "";
 
@@ -94,6 +100,10 @@ export const confirm = (message: string = "Are you sure?"): Promise<boolean> =>
         },
       },
       (err: any, result: any): void => {
+        if (err || !result) {
+          resolve(false);
+          return;
+        }
         const accepted = result.response && result.response.toLowerCase() === "y";
         const rejected = !result.response || result.response.toLowerCase() === "n";
 
@@ -289,9 +299,10 @@ function deleteConnectionInfoCache(printMessage: boolean = true): void {
   } catch (ex) {}
 }
 
-function deleteFolder(folderPath: string): Promise<void> {
+/** rimraf 4+ needs `glob` to expand a pattern. Off by default so a literal path like "build[1]" is safe. */
+export function deleteFolder(folderPath: string, glob: boolean = false): Promise<void> {
   return Q.Promise<void>((resolve, reject) => {
-    rimraf(folderPath).then(() => resolve(<void>null)).catch(reject);
+    rimraf(folderPath, { glob }).then(() => resolve(<void>null)).catch(reject);
   });
 }
 
@@ -1361,7 +1372,7 @@ export const releaseReact = (command: cli.IReleaseReactCommand): Promise<void> =
       })
       // This is needed to clear the react native bundler cache:
       // https://github.com/facebook/react-native/issues/4289
-      .then(() => deleteFolder(`${os.tmpdir()}/react-*`))
+      .then(() => deleteFolder(`${os.tmpdir()}/react-*`, /*glob*/ true))
       .then(() =>
         runReactNativeBundleCommand(
           bundleName,

script/command-parser.ts

@@ -357,6 +357,8 @@ yargs
         addCommonConfiguration(yargs);
       })
       .command("transfer", "Transfer the ownership of an app to another account", (yargs: yargs.Argv) => {
+        // Required, or the category check below rejects every invocation.
+        isValidCommand = true;
         yargs
           .usage(USAGE_PREFIX + " app transfer <appName> <email>")
           .demand(/*count*/ 2, /*max*/ 2) // Require exactly two non-option arguments

script/management-sdk.ts

@@ -298,13 +298,13 @@ class AccountManager {
 
   public getAutoRollbackConfig(appName: string, deploymentName: string): Promise<any> {
     return this.get(urlEncode([`/apps/${appName}/deployments/${deploymentName}/auto-rollback`])).then(
-      (res: JsonResponse) => res.body.autoRollback
+      (res: JsonResponse) => res.body.autoRollbackConfig
     );
   }
 
   public setAutoRollbackConfig(appName: string, deploymentName: string, config: { enabled: boolean; errorRateThreshold: number; minDevices: number }): Promise<any> {
     return this.put(urlEncode([`/apps/${appName}/deployments/${deploymentName}/auto-rollback`]), JSON.stringify(config)).then(
-      (res: JsonResponse) => res.body.autoRollback
+      (res: JsonResponse) => res.body.autoRollbackConfig
     );
   }
 

test/command-parser.ts

@@ -125,3 +125,68 @@ describe("command line", function () {
 // Two dependencies of the destructive-command path, kept honest here because both broke silently once.
 // ---------------------------------------------------------------------------
 
+
+describe("confirm on a non-terminal", () => {
+  const cmdexec = require("../script/command-executor");
+
+  it("declines instead of hanging when nobody can answer", function (done: Mocha.Done) {
+    // Forced, not inherited: stdin is piped under CI but a terminal under `npm test`.
+    this.timeout(5000);
+    const descriptor = Object.getOwnPropertyDescriptor(process.stdin, "isTTY");
+    Object.defineProperty(process.stdin, "isTTY", { value: false, configurable: true });
+    const restore = (): void => {
+      if (descriptor) Object.defineProperty(process.stdin, "isTTY", descriptor);
+      else delete (process.stdin as any).isTTY;
+    };
+
+    let settled = false;
+    const timer = setTimeout(() => {
+      if (!settled) {
+        restore();
+        done(new Error("confirm never settled: it is hanging again"));
+      }
+    }, 3000);
+
+    cmdexec.confirm("Delete it?").then(
+      (answer: boolean) => {
+        settled = true;
+        clearTimeout(timer);
+        restore();
+        assert.strictEqual(answer, false, "an unanswered destructive question is a no");
+        done();
+      },
+      (error: any) => {
+        restore();
+        done(error);
+      }
+    );
+  });
+});
+
+describe("deleteFolder", () => {
+  const cmdexec = require("../script/command-executor");
+
+  it("only expands a pattern when asked to", async () => {
+    const dir = fs.mkdtempSync(path.join(os.tmpdir(), "dpctl-glob-test-"));
+    const pattern = path.join(os.tmpdir(), "dpctl-glob-test-*");
+
+    await cmdexec.deleteFolder(pattern);
+    assert.strictEqual(fs.existsSync(dir), true, "without glob a pattern is a literal path, so nothing matches");
+
+    await cmdexec.deleteFolder(pattern, true);
+    assert.strictEqual(fs.existsSync(dir), false, "the glob flag is what actually deletes");
+  });
+
+  it("leaves glob off by default, so a literal path with metacharacters is safe", async () => {
+    // An --outputDir like "build[1]" is a real directory name, not a pattern.
+    const parent = fs.mkdtempSync(path.join(os.tmpdir(), "dpctl-literal-"));
+    const literal = path.join(parent, "build[1]");
+    fs.mkdirSync(literal);
+    fs.writeFileSync(path.join(literal, "keep.txt"), "x");
+
+    await cmdexec.deleteFolder(literal);
+    assert.strictEqual(fs.existsSync(literal), false, "the literal directory itself is removed");
+
+    fs.rmSync(parent, { recursive: true, force: true });
+  });
+});

test/management-sdk.ts

@@ -403,6 +403,16 @@ describe("Management SDK", () => {
       (error: Error) => done()
     );
   });
+
+  it("getAutoRollbackConfig reads autoRollbackConfig, the key the API actually sends", (done: Mocha.Done) => {
+    mockReturn(JSON.stringify({ autoRollbackConfig: { enabled: true, threshold: 25 } }), 200);
+    manager.getAutoRollbackConfig("appName", "Staging").done((config: any) => {
+      assert.ok(config, "expected the config, not undefined");
+      assert.strictEqual(config.enabled, true);
+      assert.strictEqual(config.threshold, 25);
+      done();
+    }, rejectHandler);
+  });
 });
 
 // Helper method that is used everywhere that an assert.fail() is needed in a promise handler