cli undo: warn about undoing push operations

[senekor]

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

• [x] 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)
• [x] I have added/updated tests to cover my changes

[steveklabnik]

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

[senekor]

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]

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

[senekor]

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

[PhilipMetzger]

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.

[senekor]

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]

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]

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]

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]

After some digging, I inserted the following debug statements in simple_op_store.rs (fn write_operation):

eprintln!("{operation:?}");
let id = OperationId::new(blake2b_hash(operation).to_vec());
eprintln!("{id:?}");

If the operation is the same, the ID derived from its content hash should be the same, right? Well, it isn't. I don't know why. Multiple runs of the test show the same operation, but a different ID.

I just redacted that line from the snapshotted output, hopefully that's ok...

[yuja]

Multiple runs of the test show the same operation, but a different ID.

Perhaps, the remote argument differs on Windows. I think we can just use slash-separated relative path.

[senekor]

Multiple runs of the test show the same operation, but a different ID.

Perhaps, the remote argument differs on Windows. I think we can just use slash-separated relative path.

Those multiple runs were on my machine. I can run the test twice back-to-back and the operation ID will be different.

[yuja]

You're using absolute remote path, right? It would never be stable.

[senekor]

You're using absolute remote path, right? It would never be stable.

That was the problem! Thanks 😅