cli undo: deny undoing push operations

cli/src/commands/git/mod.rs

@@ -46,6 +46,7 @@ use self::init::GitInitArgs;
 use self::init::cmd_git_init;
 use self::push::GitPushArgs;
 use self::push::cmd_git_push;
+pub use self::push::is_push_operation;
 use self::remote::RemoteCommand;
 use self::remote::cmd_git_remote;
 use self::root::GitRootArgs;

cli/src/commands/git/push.rs

@@ -32,6 +32,7 @@ use jj_lib::git;
 use jj_lib::git::GitBranchPushTargets;
 use jj_lib::git::GitPushStats;
 use jj_lib::op_store::RefTarget;
+use jj_lib::operation::Operation;
 use jj_lib::ref_name::RefName;
 use jj_lib::ref_name::RefNameBuf;
 use jj_lib::ref_name::RemoteName;
@@ -203,6 +204,8 @@ fn make_bookmark_term(bookmark_names: &[impl fmt::Display]) -> String {
 
 const DEFAULT_REMOTE: &RemoteName = RemoteName::new("origin");
 
+const TX_DESC_PUSH: &str = "push";
+
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 enum BookmarkMoveDirection {
     Forward,
@@ -244,7 +247,7 @@ pub fn cmd_git_push(
             }
         }
         tx_description = format!(
-            "push all bookmarks to git remote {remote}",
+            "{TX_DESC_PUSH} all bookmarks to git remote {remote}",
             remote = remote.as_symbol()
         );
     } else if args.tracked {
@@ -265,7 +268,7 @@ pub fn cmd_git_push(
             }
         }
         tx_description = format!(
-            "push all tracked bookmarks to git remote {remote}",
+            "{TX_DESC_PUSH} all tracked bookmarks to git remote {remote}",
             remote = remote.as_symbol()
         );
     } else if args.deleted {
@@ -287,7 +290,7 @@ pub fn cmd_git_push(
             }
         }
         tx_description = format!(
-            "push all deleted bookmarks to git remote {remote}",
+            "{TX_DESC_PUSH} all deleted bookmarks to git remote {remote}",
             remote = remote.as_symbol()
         );
     } else {
@@ -379,7 +382,7 @@ pub fn cmd_git_push(
         }
 
         tx_description = format!(
-            "push {names} to git remote {remote}",
+            "{TX_DESC_PUSH} {names} to git remote {remote}",
             names = make_bookmark_term(
                 &bookmark_updates
                     .iter()
@@ -1019,3 +1022,7 @@ fn find_bookmarks_targeted_by_revisions<'a>(
         .collect_vec();
     Ok(bookmarks_targeted)
 }
+
+pub fn is_push_operation(op: &Operation) -> bool {
+    op.metadata().description.starts_with(TX_DESC_PUSH)
+}

cli/src/commands/undo.rs

@@ -22,6 +22,8 @@ use crate::command_error::CommandError;
 use crate::command_error::internal_error;
 use crate::command_error::user_error;
 use crate::command_error::user_error_with_hint;
+#[cfg(feature = "git")]
+use crate::commands::git::is_push_operation;
 use crate::commands::operation::DEFAULT_REVERT_WHAT;
 use crate::commands::operation::RevertWhatToRestore;
 use crate::commands::operation::revert::OperationRevertArgs;
@@ -148,6 +150,14 @@ pub fn cmd_undo(ui: &mut Ui, command: &CommandHelper, args: &UndoArgs) -> Result
             .loader()
             .load_operation(&id_of_restored_op)?;
     }
+    #[cfg(feature = "git")]
+    if is_push_operation(&op_to_undo) {
+        writeln!(
+            ui.warning_default(),
+            "Undoing a push operation often leads to conflicted bookmarks."
+        )?;
+        writeln!(ui.hint_default(), "To avoid this, run `jj redo` now.")?;
+    };
 
     let mut op_to_restore = match op_to_undo.parents().at_most_one() {
         Ok(Some(parent_of_op_to_undo)) => parent_of_op_to_undo?,

cli/tests/test_undo_redo_commands.rs

@@ -55,6 +55,41 @@ fn test_undo_merge_operation() {
     ");
 }
 
+#[test]
+fn test_undo_push_operation() {
+    let test_env = TestEnvironment::default();
+
+    test_env
+        .run_jj_in(".", ["git", "init", "--colocate", "origin"])
+        .success();
+    let origin_dir = test_env.work_dir("origin");
+    let origin_git_repo_path = origin_dir.root().join(".git");
+    test_env
+        .run_jj_in(
+            ".",
+            [
+                "git",
+                "clone",
+                origin_git_repo_path.to_str().unwrap(),
+                "repo",
+            ],
+        )
+        .success();
+    let work_dir = test_env.work_dir("repo");
+
+    work_dir.write_file("foo", "foo");
+    work_dir.run_jj(["commit", "-mfoo"]).success();
+    work_dir.run_jj(["git", "push", "-c@-"]).success();
+    let output = work_dir.run_jj(["undo"]);
+    insta::assert_snapshot!(output, @r"
+    ------- stderr -------
+    Warning: Undoing a push operation often leads to conflicted bookmarks.
+    Hint: To avoid this, run `jj redo` now.
+    Restored to operation: 5b87c43b033a (2001-02-03 08:05:09) commit 3850397cf31988d0657948307ad5bbe873d76a38
+    [EOF]
+    ");
+}
+
 #[test]
 fn test_undo_jump_old_undo_stack() {
     let test_env = TestEnvironment::default();