cli undo: warn about undoing push operations

Undoing a push operation does not undo the effects on the remote.
Bookmarks on the remote will stay in place, but the local repository
will forget about their state. If the bookmarks are subsequently
moved and pushed, that later push will fail, since the bookmarks have
"unexpectedly" moved on the remote. Therefore, add a warning telling
users to run `jj redo` to avoid these complications.

Author: senekor

cli/src/commands/git/mod.rs

@@ -49,6 +49,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

@@ -33,6 +33,7 @@ use jj_lib::git::GitBranchPushTargets;
 use jj_lib::git::GitPushStats;
 use jj_lib::index::IndexResult;
 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;
@@ -205,6 +206,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,
@@ -246,7 +249,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 {
@@ -267,7 +270,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 {
@@ -289,7 +292,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 {
@@ -381,7 +384,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()
@@ -1022,3 +1025,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,31 @@ 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();
+    test_env
+        .run_jj_in(".", ["git", "clone", "origin", "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: f9fd582ef03c (2001-02-03 08:05:09) commit 3850397cf31988d0657948307ad5bbe873d76a38
+    [EOF]
+    ");
+}
+
 #[test]
 fn test_undo_jump_old_undo_stack() {
     let test_env = TestEnvironment::default();