Merge pull request #12484 from quarto-dev/bugfix/11788

`quarto add`, `quarto remove`: return non-zero status code on failure

Author: cscheid

news/changelog-1.7.md

@@ -164,6 +164,7 @@ All changes included in 1.7:
 - ([#11441](https://github.com/quarto-dev/quarto-cli/issues/11441)): Don't add newlines around shortcodes during processing.
 - ([#11606](https://github.com/quarto-dev/quarto-cli/discussions/11606)): Added a new `QUARTO_DOCUMENT_FILE` env var available to computation engine to the name of the file currently being rendered.
 - ([#11643](https://github.com/quarto-dev/quarto-cli/issues/11643)): Improve highlighting of nested code block inside markdown code block, i.e. using ` ```{{python}} ` or ` ```python ` inside ` ````markdown` fenced code block.
+- ([#11788](https://github.com/quarto-dev/quarto-cli/issues/11788)): `quarto add` and `quarto remove` will return non-zero code when they fail.
 - ([#11803](https://github.com/quarto-dev/quarto-cli/pull/11803)): Added a new CLI command `quarto call`. First users of this interface are the new `quarto call engine julia ...` subcommands.
 - ([#11951](https://github.com/quarto-dev/quarto-cli/issues/11951)): Raw LaTeX table without `tbl-` prefix label for using Quarto crossref are now correctly passed through unmodified.
 - ([#11967](https://github.com/quarto-dev/quarto-cli/issues/11967)): Produce a better error message when YAML metadata with `!expr` tags are used outside of `knitr` code cells.

src/command/add/cmd.ts

@@ -9,6 +9,7 @@ import { createTempContext } from "../../core/temp.ts";
 import { installExtension } from "../../extension/install.ts";
 
 import { info } from "../../deno_ral/log.ts";
+import { signalCommandFailure } from "../utils.ts";
 
 export const addCommand = new Command()
   .name("add")
@@ -44,16 +45,19 @@ export const addCommand = new Command()
       await initYamlIntelligenceResourcesFromFilesystem();
       const temp = createTempContext();
       try {
-        // Install an extension
-        if (extension) {
-          await installExtension(
-            extension,
-            temp,
-            options.prompt !== false,
-            options.embed,
-          );
-        } else {
+        if (!extension) {
           info("Please provide an extension name, url, or path.");
+          signalCommandFailure();
+        }
+        // Install an extension
+        const result = await installExtension(
+          extension,
+          temp,
+          options.prompt !== false,
+          options.embed,
+        );
+        if (!result) {
+          signalCommandFailure();
         }
       } finally {
         temp.cleanup();

src/command/remove/cmd.ts

@@ -22,6 +22,7 @@ import {
   selectTool,
 } from "../../tools/tools-console.ts";
 import { notebookContext } from "../../render/notebook/notebook-context.ts";
+import { signalCommandFailure } from "../utils.ts";
 
 export const removeCommand = new Command()
   .name("remove")
@@ -69,6 +70,7 @@ export const removeCommand = new Command()
             const allTools = await loadTools();
             if (allTools.filter((tool) => tool.installed).length === 0) {
               info("No tools are installed.");
+              signalCommandFailure();
             } else {
               // Select which tool should be installed
               const toolTarget = await selectTool(allTools, "remove");
@@ -118,6 +120,7 @@ export const removeCommand = new Command()
               await removeExtensions(extensions.slice(), options.prompt);
             } else {
               info("No matching extension found.");
+              signalCommandFailure();
             }
           } else {
             const nbContext = notebookContext();
@@ -138,6 +141,7 @@ export const removeCommand = new Command()
               }
             } else {
               info("No extensions installed.");
+              signalCommandFailure();
             }
           }
         }

src/command/utils.ts

@@ -0,0 +1,17 @@
+/*
+ * utils.ts
+ *
+ * Copyright (C) 2025 Posit Software, PBC
+ */
+
+let someCommandFailed = false;
+
+// we do this the roundabout way because there doesn't seem to be any clean way
+// for cliffy commands to return values? Likely a skill issue on my part
+export const signalCommandFailure = () => {
+  someCommandFailed = true;
+};
+
+export const commandFailed = () => {
+  return someCommandFailed;
+};

src/extension/install.ts

@@ -34,7 +34,7 @@ export async function installExtension(
   temp: TempContext,
   allowPrompt: boolean,
   embed?: string,
-) {
+): Promise<boolean> {
   // Is this local or remote?
   const source = await extensionSource(target);
 
@@ -43,68 +43,70 @@ export async function installExtension(
     info(
       `Extension not found in local or remote sources`,
     );
-    return;
+    return false;
   }
 
   // Does the user trust the extension?
   const trusted = await isTrusted(source, allowPrompt);
   if (!trusted) {
     // Not trusted, cancel
     cancelInstallation();
-  } else {
-    // Compute the installation directory
-    const currentDir = Deno.cwd();
-    const installDir = await determineInstallDir(
-      currentDir,
-      allowPrompt,
-      embed,
-    );
+    return false;
+  }
 
-    // Stage the extension locally
-    const extensionDir = await stageExtension(source, temp.createDir());
+  // Compute the installation directory
+  const currentDir = Deno.cwd();
+  const installDir = await determineInstallDir(
+    currentDir,
+    allowPrompt,
+    embed,
+  );
 
-    // Validate the extension in in the staging dir
-    const stagedExtensions = await validateExtension(extensionDir);
+  // Stage the extension locally
+  const extensionDir = await stageExtension(source, temp.createDir());
 
-    // Confirm that the user would like to take this action
-    const confirmed = await confirmInstallation(
-      stagedExtensions,
-      installDir,
-      { allowPrompt },
-    );
+  // Validate the extension in in the staging dir
+  const stagedExtensions = await validateExtension(extensionDir);
 
-    if (confirmed) {
-      // Complete the installation
-      await completeInstallation(extensionDir, installDir);
+  // Confirm that the user would like to take this action
+  const confirmed = await confirmInstallation(
+    stagedExtensions,
+    installDir,
+    { allowPrompt },
+  );
 
-      await withSpinner(
-        { message: "Extension installation complete" },
-        () => {
-          return Promise.resolve();
-        },
-      );
+  if (!confirmed) {
+    // Not confirmed, cancel the installation
+    cancelInstallation();
+  }
 
-      if (source.learnMoreUrl) {
-        info("");
-        if (allowPrompt) {
-          const open = await Confirm.prompt({
-            message: "View documentation using default browser?",
-            default: true,
-          });
-          if (open) {
-            await openUrl(source.learnMoreUrl);
-          }
-        } else {
-          info(
-            `\nLearn more about this extension at:\n${source.learnMoreUrl}\n`,
-          );
-        }
+  // Complete the installation
+  await completeInstallation(extensionDir, installDir);
+
+  await withSpinner(
+    { message: "Extension installation complete" },
+    () => {
+      return Promise.resolve();
+    },
+  );
+
+  if (source.learnMoreUrl) {
+    info("");
+    if (allowPrompt) {
+      const open = await Confirm.prompt({
+        message: "View documentation using default browser?",
+        default: true,
+      });
+      if (open) {
+        await openUrl(source.learnMoreUrl);
       }
     } else {
-      // Not confirmed, cancel the installation
-      cancelInstallation();
+      info(
+        `\nLearn more about this extension at:\n${source.learnMoreUrl}\n`,
+      );
     }
   }
+  return true;
 }
 
 // Cancels the installation, providing user feedback that the installation is canceled

src/quarto.ts

@@ -36,6 +36,7 @@ import { typstBinaryPath } from "./core/typst.ts";
 import { exitWithCleanup, onCleanup } from "./core/cleanup.ts";
 
 import { runScript } from "./command/run/run.ts";
+import { commandFailed } from "./command/utils.ts";
 
 // ensures run handlers are registered
 import "./core/run/register.ts";
@@ -51,7 +52,6 @@ import "./format/imports.ts";
 
 import { kCliffyImplicitCwd } from "./config/constants.ts";
 import { mainRunner } from "./core/main.ts";
-import { engineCommand } from "./execute/engine.ts";
 
 const checkVersionRequirement = () => {
   const versionReq = Deno.env.get("QUARTO_VERSION_REQUIREMENT");
@@ -196,6 +196,9 @@ export async function quarto(
         Deno.env.set(key, value);
       }
     }
+    if (commandFailed()) {
+      exitWithCleanup(1);
+    }
   } catch (e) {
     if (e instanceof CommandError) {
       logError(e, false);
@@ -221,5 +224,9 @@ if (import.meta.main) {
       cmd = appendLogOptions(cmd);
       return appendProfileArg(cmd);
     });
+
+    if (commandFailed()) {
+      exitWithCleanup(1);
+    }
   });
 }