cli undo: deny undoing push operations #7716

senekor · 2025-10-13T12:46:51Z

Motivated by a discussed on discord: https://discord.com/channels/968932220549103686/1427215834950008973

I'm not 100% convinced we need this, but it's a reasonable idea to consider.

Checklist

I have updated CHANGELOG.md
I have updated the documentation (README.md, docs/, demos/)
I have updated the config schema (cli/src/config-schema.json)
I have added/updated tests to cover my changes

steveklabnik · 2025-10-13T16:16:36Z

I always thought it was kinda neat that pushes could be undone.

senekor · 2025-10-13T16:20:35Z

What's your use case to undo a push? Do you do it often, or is it ok if there's an extra flag required?

steveklabnik · 2025-10-13T16:21:40Z

I don't do it often, I just happened to do it one day and went "oh, it did that, that's neat."

senekor · 2025-10-13T16:28:59Z

My feeling is that it's ok to ask people to pass an extra flag in that case. The warning mentions conflicted bookmarks. If the user knows what that is and how to deal with it, they may be confident to use the flag. Users who don't know how to deal with a conflicted bookmark probably wouldn't find it "neat" to run into that situation accidentally.

Btw. here's what that interaction would look like with this PR:

playground > jj undo
Error: Refusing to undo push operation
Hint: Undoing a push operation often leads to conflicted bookmarks.
Hint: To accept the risk, use `--allow-undo-push`.
playground > jj undo --allow-undo-push
Restored to operation: 55a8fb96a6f6 (2025-10-13 18:25:29) commit 45433ee1ee6f5dd511864d6f1daf5fe998577e79

cli/src/commands/git/push.rs

martinvonz · 2025-10-14T04:56:57Z

cli/src/commands/undo.rs

+    /// Because of these complications, undoing a push operation is not allowed
+    /// by default and requires this flag.
+    #[arg(long)]
+    allow_undo_push: bool,


Should this also only exist if the git feature is enabled? The description mentions GIt, and it checks specifically for Git push operations. Should the flag name even be --allow-undo-git-push?

Gating the flag behind the git feature makes sense to me. But I don't see a reason to include it in the flag name. It's not ambiguous, right? Otherwise it just seems like more typing to me.

Perhaps the check doesn't need to be specific to Git? Is it sufficient to check if the operation changed any remote bookmarks/tags?

I don't think we can reliably detect direction of changes. Fetches can be reverted, but pushes can't.

cli/tests/test_undo_redo_commands.rs

cli/src/commands/undo.rs

yuja · 2025-10-14T09:34:26Z

cli/src/commands/undo.rs

+    /// Because of these complications, undoing a push operation is not allowed
+    /// by default and requires this flag.
+    #[arg(long)]
+    allow_undo_push: bool,


I don't think this flag is needed because undoing a push would require expertise. Just suggest jj op revert/restore?

Yeah, it's not really necessary. The small benefit would be that sequential undo still works. An example workflow: Let's say I'm editing a commit with a bookmark pointing to it and I'm pushing my edits all the time, just to have the latest state on the remote. I could recover a previous state by going into the op log, guessing which snapshot is the right one, copying its ID, restoring to it, running log / status to check if I guessed correctly. If I didn't, I have to try another operation. A slightly better approach would be running jj log / status --at-op=<ID> to identify the correct operation first, before restoring to it. But then it's basically a requirement to have dynamic completions and tabbing through the operation IDs is still annoying. Being able to run jj undo --allow-undo push ; jj log and hitting Up+Enter until things look right is just way more convenient. At the end of the process, fixing the conflicted bookmark is the same with or without the flag.

Edit: I personally do nervously push a bookmark all the time when I'm working on the train. I tend to forget time and then be surprised when I arrive at my destination. Then I have to slam my laptop shut and throw it in my backpack, to get off the train as quickly as possible. In that situation, I'm happy if I already pushed my changes, so I just do it all the time.

Just suggest jj op revert/restore?

Or jj redo?

Why jj redo? When the user wants to run jj undo and it fails for some reason, jj redo is not an adequate replacement, right?

Ah, or do you mean we should allow undoing a push operation without flag and instead print a warning with a hint to use jj redo?

Yes, that's what I meant. Sorry that I forgot to reply

I don't think "redo" provides reliable recovery from accidental "undo of push" - as soon as there is a new entry in the op log (e.g. snapshot of a file change), "redo" no longer works and the user has to deal with conflicted bookmarks and divergent changes.

We could suggest jj redo and jj git fetch. They are mostly the same in the moment. jj redo doesn't work anymore after a new operation, but jj git fetch hides the current undo-stack. So both have small disadvantages.

At the end of the day, conflicted bookmarks and divergent changes can occur for other reasons than undoing a push. It's not possible to hide this from users forever. I think it's ok that if users ignore a warning, the consequence will be that they have to learn about that a little sooner.

I changed my mind, I don't like suggesting both. It seems confusing. And explaining the difference in a warning is too verbose. I think jj redo is fine.

PhilipMetzger · 2025-10-14T16:05:27Z

Can you yeet Fedor's concern as the motivation into the commit description as is requiered when contributing here. Otherwise this commit has no reason why it should be done at any point.

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.

senekor · 2025-10-18T08:43:25Z

Hmm, the test failure is because the output of jj in the snapshot test is not deterministic :(

The operation ID in this line always changes:

Restored to operation: 5b87c43b033a (2001-02-03 08:05:09) commit 3850397cf31988d0657948307ad5bbe873d76a38

I'll have to look into what's causing this later.

zx8 · 2025-10-19T14:15:19Z

Hmm, I have a slightly different use case here. Sometimes I want to undo what has been pushed to the remote, and indeed the subsequent push after the undo fails due to stale information. However, I was hoping (/waiting) for jj git push --force to be implemented so I could undo the push operation, and then forcefully push my local changes because I want to correct the changes on the remote.

senekor · 2025-10-19T14:23:26Z

Sometimes I want to undo what has been pushed to the remote, and indeed the subsequent push after the undo fails due to stale information. However, I was hoping (/waiting) for jj git push --force to be implemented

The way to do that now is jj git fetch, jj bookmark move <name> --to <revision>, jj git push. What about that is problematic for you? Do you find it tedious to specify the bookmark and find the correct revision to point it to in the log?

zx8 · 2025-10-19T21:53:14Z

Sometimes I want to undo what has been pushed to the remote, and indeed the subsequent push after the undo fails due to stale information. However, I was hoping (/waiting) for jj git push --force to be implemented

The way to do that now is jj git fetch, jj bookmark move <name> --to <revision>, jj git push. What about that is problematic for you? Do you find it tedious to specify the bookmark and find the correct revision to point it to in the log?

Apologies, I should have elaborated. That's what I'm doing at the moment to resolve the issue.

And yep, that's exactly it - it's tedious to have to do this in the first place when I know what I want to do. I appreciate jj trying to stop me doing something by mistake, but fixing what's on a remote by undoing local commits is a pretty common operation for me, so it would be great if jj wouldn't get in my way so much.

senekor requested a review from a team as a code owner October 13, 2025 12:46

senekor force-pushed the senekor/xtkktovrnvtx branch 3 times, most recently from c7176db to f0ba75e Compare October 13, 2025 16:14

senekor force-pushed the senekor/xtkktovrnvtx branch from f0ba75e to 770fec2 Compare October 13, 2025 18:03

martinvonz reviewed Oct 14, 2025

View reviewed changes

yuja reviewed Oct 14, 2025

View reviewed changes

senekor force-pushed the senekor/xtkktovrnvtx branch 2 times, most recently from 7a00cb4 to 339426c Compare October 14, 2025 10:26

senekor force-pushed the senekor/xtkktovrnvtx branch 2 times, most recently from 2b16534 to 1b1e485 Compare October 18, 2025 08:09

senekor force-pushed the senekor/xtkktovrnvtx branch from 1b1e485 to 29e6f87 Compare October 18, 2025 08:23

cli undo: deny undoing push operations #7716

Are you sure you want to change the base?

cli undo: deny undoing push operations #7716

Uh oh!

Conversation

senekor commented Oct 13, 2025

Checklist

Uh oh!

steveklabnik commented Oct 13, 2025

Uh oh!

senekor commented Oct 13, 2025

Uh oh!

steveklabnik commented Oct 13, 2025

Uh oh!

senekor commented Oct 13, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

senekor Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PhilipMetzger commented Oct 14, 2025

Uh oh!

senekor commented Oct 18, 2025

Uh oh!

zx8 commented Oct 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

senekor commented Oct 19, 2025

Uh oh!

zx8 commented Oct 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

senekor Oct 14, 2025 •

edited

Loading

zx8 commented Oct 19, 2025 •

edited

Loading

zx8 commented Oct 19, 2025 •

edited

Loading