From 69154d5863e090c1e9c6a7832a82f9d07f6224ca Mon Sep 17 00:00:00 2001
From: Baptiste Girardeau <contact@bgirardeau.me>
Date: Mon, 10 Aug 2026 20:39:07 +0200
Subject: [PATCH] cli: warn when sign-on-push skips immutable commits

Commits that need a signature but are immutable cannot be rewritten to
add one, so they were silently pushed unsigned. This is easy to hit
when, for example, tagging unpushed commits.

`jj git push` now warns if any commit that should have been signed was
skipped because it is immutable.
---
 cli/src/commands/git/push.rs | 49 ++++++++++++++++++++++++++++--------
 cli/tests/test_git_push.rs   |  2 ++
 2 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/cli/src/commands/git/push.rs b/cli/src/commands/git/push.rs
index ac344565488..b7496b0fb9d 100644
--- a/cli/src/commands/git/push.rs
+++ b/cli/src/commands/git/push.rs
@@ -66,6 +66,7 @@ use crate::cli_util::WorkspaceCommandHelper;
 use crate::cli_util::WorkspaceCommandTransaction;
 use crate::cli_util::has_tracked_remote_bookmarks;
 use crate::cli_util::has_tracked_remote_tags;
+use crate::cli_util::print_updated_commits;
 use crate::cli_util::short_change_hash;
 use crate::cli_util::short_commit_hash;
 use crate::command_error::CommandError;
@@ -795,13 +796,13 @@ fn ready_to_push_revset_expression(
         .flat_map(|(_, old_head)| old_head.target.added_ids())
         .cloned()
         .collect_vec();
-    RevsetExpression::commits(old_heads)
-        .union(workspace_helper.env().immutable_heads_expression())
-        .range(&RevsetExpression::commits(new_heads))
+    RevsetExpression::commits(old_heads).range(&RevsetExpression::commits(new_heads))
 }
 
 /// Signs commits before pushing.
 ///
+/// Warns about commits that need a signature but are immutable.
+///
 /// Returns the updated list of bookmark names and corresponding
 /// [`BookmarkPushUpdate`]s.
 async fn sign_commits_before_push(
@@ -812,14 +813,42 @@ async fn sign_commits_before_push(
 ) -> Result<GitPushRefTargets, CommandError> {
     let mut sign_settings = tx.settings().sign_settings();
     sign_settings.behavior = SignBehavior::Own;
-    let commit_ids: IndexSet<CommitId> = tx
-        .base_workspace_helper()
-        .attach_revset_evaluator(commits_to_push)
+    // TODO: make filter condition configurable by revset?
+    let needs_signing = |commit: &Commit| {
+        future::ready(!commit.is_signed() && sign_settings.should_sign(commit.store_commit()))
+    };
+
+    let workspace_helper = tx.base_workspace_helper();
+    let immutable = workspace_helper.env().immutable_expression();
+    let skipped: Vec<Commit> = workspace_helper
+        .attach_revset_evaluator(commits_to_push.intersection(&immutable))
         .evaluate_to_commits()?
-        // TODO: make filter condition configurable by revset?
-        .try_filter(|commit| {
-            future::ready(!commit.is_signed() && sign_settings.should_sign(commit.store_commit()))
-        })
+        .try_filter(needs_signing)
+        .take(11)
+        .try_collect()
+        .await?;
+    if !skipped.is_empty() {
+        let count = if skipped.len() > 10 {
+            "10+".to_owned()
+        } else {
+            skipped.len().to_string()
+        };
+        writeln!(
+            ui.warning_default(),
+            "Skipped signing {count} immutable commits:"
+        )?;
+        let mut formatter = ui.stderr_formatter();
+        print_updated_commits(
+            formatter.as_mut(),
+            &workspace_helper.commit_summary_template(),
+            &skipped,
+        )?;
+    }
+
+    let commit_ids: IndexSet<CommitId> = workspace_helper
+        .attach_revset_evaluator(commits_to_push.minus(&immutable))
+        .evaluate_to_commits()?
+        .try_filter(needs_signing)
         .map_ok(|commit| commit.id().clone())
         .try_collect()
         .await?;
diff --git a/cli/tests/test_git_push.rs b/cli/tests/test_git_push.rs
index 6ddc9dbdc5f..35de0bee209 100644
--- a/cli/tests/test_git_push.rs
+++ b/cli/tests/test_git_push.rs
@@ -2728,6 +2728,8 @@ fn test_git_push_sign_on_push() {
     let output = work_dir.run_jj(["git", "push"]);
     insta::assert_snapshot!(output, @"
     ------- stderr -------
+    Warning: Skipped signing 1 immutable commits:
+      kpqxywon 48ea83e9 bookmark2* | (empty) commit which should not be signed 1
     Changes to push to origin:
       bookmark: bookmark2 [move forward from d45e2adce0ad to 48ea83e9499c]
     [EOF]