fix: don't modify manifest on failing change

src/cli/add.rs

@@ -1,5 +1,6 @@
 use clap::Parser;
 use indexmap::IndexMap;
+use miette::IntoDiagnostic;
 use pixi_manifest::FeatureName;
 
 use super::has_specs::HasSpecs;
@@ -120,7 +121,11 @@ pub async fn execute(args: Args) -> miette::Result<()> {
     // TODO: add dry_run logic to add
     let dry_run = false;
 
-    let update_deps = project
+    // Save original manifest
+    let original_manifest_content =
+        fs_err::read_to_string(project.manifest_path()).into_diagnostic()?;
+
+    let update_deps = match project
         .update_dependencies(
             match_specs,
             pypi_deps,
@@ -130,7 +135,15 @@ pub async fn execute(args: Args) -> miette::Result<()> {
             args.editable,
             dry_run,
         )
-        .await?;
+        .await
+    {
+        Ok(update_deps) => update_deps,
+        Err(e) => {
+            // Restore original manifest
+            fs_err::write(project.manifest_path(), original_manifest_content).into_diagnostic()?;
+            return Err(e);
+        }
+    };
 
     if let Some(update_deps) = update_deps {
         // Notify the user we succeeded

src/cli/upgrade.rs

@@ -68,7 +68,11 @@ pub async fn execute(args: Args) -> miette::Result<()> {
 
     let (match_specs, pypi_deps) = parse_specs(feature, &args, &project)?;
 
-    let update_deps = project
+    // Save original manifest
+    let original_manifest_content =
+        fs_err::read_to_string(project.manifest_path()).into_diagnostic()?;
+
+    let update_deps = match project
         .update_dependencies(
             match_specs,
             pypi_deps,
@@ -78,7 +82,15 @@ pub async fn execute(args: Args) -> miette::Result<()> {
             false,
             args.dry_run,
         )
-        .await?;
+        .await
+    {
+        Ok(update_deps) => update_deps,
+        Err(e) => {
+            // Restore original manifest
+            fs_err::write(project.manifest_path(), original_manifest_content).into_diagnostic()?;
+            return Err(e);
+        }
+    };
 
     // Is there something to report?
     if let Some(update_deps) = update_deps {

src/project/mod.rs

@@ -714,7 +714,7 @@ impl Project {
             }
         }
 
-        // Only save to disk if not a dry run
+        // TODO: Figure out if we really need this save here
         if !dry_run {
             self.save()?;
         }
@@ -803,7 +803,7 @@ impl Project {
             implicit_constraints.extend(pypi_constraints);
         }
 
-        // Only write to disk if not a dry run
+        // TODO: figure out if we really need this save here.
         if !dry_run {
             self.save()?;
         }

tests/integration_python/test_main_cli.py

@@ -714,3 +714,21 @@ def test_concurrency_flags(
             "package3",
         ]
     )
+
+
+def test_dont_add_broken_dep(pixi: Path, tmp_pixi_workspace: Path, dummy_channel_1: str) -> None:
+    manifest_path = tmp_pixi_workspace / "pixi.toml"
+
+    # Create a new project
+    verify_cli_command([pixi, "init", "--channel", dummy_channel_1, tmp_pixi_workspace])
+
+    manifest_content = tmp_pixi_workspace.joinpath("pixi.toml").read_text()
+
+    # Add a non existing package should error
+    verify_cli_command(
+        [pixi, "add", "--manifest-path", manifest_path, "dummy-a=1000000"],
+        ExitCode.FAILURE,
+    )
+
+    # It should not have modified the manifest on failure
+    assert manifest_content == tmp_pixi_workspace.joinpath("pixi.toml").read_text()