feat(but-rebase): add programmatic signing control for graph engine by slarse · Pull Request #13046 · gitbutlerapp/gitbutler

slarse · 2026-03-26T08:04:01Z

This PR puts down the groundwork for enabling sign-on-push functionality by introducing new facilities to tweak sign behavior for the graph rebase engine. There are no plans to port this to the old rebase engine.

The key design goal is to allow for programmatically driven signing, where we can decide at runtime if a certain rebase operation should cause signing (e.g. just before a push), while another one should not. At the same time, we want to retain the old gitbutler.signCommits behavior for the sake of Git interoperability - you might do some work with GitButler and then decide to push with Git, and that should still work, respecting signing settings.

The two concepts introduced to the graph rebase engine in this commit is a PickMode, allowing the caller to tune when a commit is cherry-picked, and SignCommit, which allows for tuning signing behavior beyond what was previously possible. Setting PickMode::Force and SignCommit::Yes or SignCommit::IfSignCommitsEnabled allows one to sign a commit that is otherwise unchanged. For descendants to also be signed, they need to be picked with an appropriate SignCommit setting.

None of the new behavior is as of yet exposed in any of the GitButler UIs, that'll come later down the road with sign-on-push functionality.

Open questions

When a commit is rewritten only to be signed, do we want to update the committer date? This is currently what happens, and I tend to think that's correct as the signature does change the commit. But I wanted to raise it as a question.

This is part 2 of 2 in a stack made with GitButler:

vercel · 2026-03-26T08:04:05Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
gitbutler-web	Ignored	Preview	Apr 9, 2026 1:43pm

Copilot

Pull request overview

This PR extends the but-rebase graph rebase engine so commit signing can be controlled programmatically at runtime (e.g., force signing for a “sign-on-push” flow) while still supporting Git-compatible signing behavior gated by config.

Changes:

Replaces the Step::Pick signing boolean with a richer PickDecisionMode + SignCommit model and threads it through graph rebasing/cherry-pick.
Adds GraphEditorOptions to configure default pick/signing behavior when creating an editor.
Expands test coverage and introduces a new fixture scenario/archive for “signing key configured, but signing disabled”.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
crates/but-workspace/src/commit_engine/mod.rs	Updates commit creation to pass `SignCommit` instead of a boolean signing flag.
crates/but-rebase/src/graph_rebase/cherry_pick.rs	Introduces `PickDecisionMode` and uses it to decide when to rewrite and how to sign.
crates/but-rebase/src/graph_rebase/mod.rs	Replaces `sign_if_configured` on `Pick` with `pick_decision_mode` and sets defaults.
crates/but-rebase/src/graph_rebase/rebase.rs	Passes the new `pick_decision_mode` through to cherry-pick execution.
crates/but-rebase/src/graph_rebase/creation.rs	Adds `GraphEditorOptions` and `Editor::create_with_opts()` to set default pick behavior.
crates/but-rebase/src/graph_rebase/commit.rs	Routes new-commit creation through the updated signing control.
crates/but-rebase/tests/rebase/graph_rebase/cherry_pick.rs	Updates cherry-pick tests to use `PickDecisionMode`/`SignCommit`.
crates/but-rebase/tests/rebase/graph_rebase/signing_preferences.rs	Adds multiple tests for forced signing and cascading behavior.
crates/but-rebase/tests/fixtures/scenario/unsigned-commits-with-signing-key-setup-but-signing-disabled.sh	Adds a deterministic scenario for signing-with-key but signing disabled.
crates/but-rebase/tests/fixtures/generated-archives/unsigned-commits-with-signing-key-setup-but-signing-disabled.tar	Adds the generated archive fixture for the new scenario.
crates/but-rebase/tests/fixtures/generated-archives/.gitignore	Allows the new archive fixture to be checked in.

crates/but-rebase/src/graph_rebase/cherry_pick.rs

crates/but-rebase/tests/rebase/graph_rebase/signing_preferences.rs

...base/tests/fixtures/scenario/unsigned-commits-with-signing-key-setup-but-signing-disabled.sh

crates/but-workspace/src/commit_engine/mod.rs

crates/but-rebase/src/graph_rebase/commit.rs

crates/but-rebase/src/graph_rebase/cherry_pick.rs

slarse · 2026-03-26T13:01:43Z

...fixtures/generated-archives/unsigned-commits-with-signing-key-setup-but-signing-disabled.tar

We should reuse a single key for these tests, but I don't feel like fixing that here so I'll do it separately instead. May or may not be merged before this PR.

Oh poop, I did fix this separately, these shouldn't be here anymore, there's a static key in the shared.sh fixture helper. Either I somehow did not get a conflict here, or I sleepwalked through the resolution.

slarse · 2026-03-26T13:02:31Z

crates/but-rebase/src/graph_rebase/cherry_pick.rs

+/// Controls when a commit is cherry-picked during a rebase.
+#[derive(Debug, Clone, Copy, PartialEq)]
+pub enum PickMode {
+    /// Cherry-picks the commit only if it's necessary because of changes to the commit or its
+    /// parents.
+    IfChanged,
+    /// Forces a cherry-pick on a commit. This is for example useful in combination with
+    /// [`SignCommit`] to sign/unsign commits that are otherwise unchanged.
+    Force,
+}


There is already a PickMode enum in the old rebase engine. I chose not to reuse it to keep the rebase engines cleanly separated, and this is such a trivial enum anyway.

slarse · 2026-03-26T13:11:52Z

crates/but-rebase/src/graph_rebase/cherry_pick.rs

+            // We should only end up here when trying to force-pick a parentless commit. At that
+            // point, it's safe to simply recreate that commit outright.
+            //
+            // Currently, the only known use case for this is to forcibly sign/unsign root commits.
+            let commit = crate::commit::create(
+                repo,
+                target.inner,
+                DateMode::CommitterUpdateAuthorKeep,
+                sign_commit,
+            )?;
+            Ok(CherryPickOutcome::Commit(commit))


Please have an extra careful look at this comment and logic. I believe this to be accurate but I'm not entirely certain. I drew the conclusion that it's safe to just commit outright, rather than going through commit_from_unconflicted_tree(), but I'm just not sure about this.

There is a test case in signing_preferences.rs that tests force-picking and signing the root commit, and it fails if this is removed.

This does seem reasonable to me

slarse · 2026-03-26T13:13:51Z

crates/but-rebase/src/graph_rebase/creation.rs

+            default_pick_mode: PickMode::IfChanged,
+            default_sign_commit: SignCommit::IfSignCommitsEnabled,


This set of options reflects the behavior we currently have in the rebase engine.

Copilot

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.

...base/tests/fixtures/scenario/unsigned-commits-with-signing-key-setup-but-signing-disabled.sh

Copilot

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.

Byron · 2026-03-31T23:27:52Z

When a commit is rewritten only to be signed, do we want to update the committer date? This is currently what happens, and I tend to think that's correct as the signature does change the commit. But I wanted to raise it as a question.

That's a great question and I have no answer, except for the intuition that cases can be made for both. I'd also think it's safe to keep it as is and alter the committer date (and committer?) when signing, and see how that fares in the wild.

Caleb-T-Owens

All LGTM.

I am wondering if we really need the GraphOpts because the behaviour of that is a bit strange. IE: If I construct a graph with "force rebase" and then add a pick that doesn't specify "force rebase" down at one of the edges of the graph, or with preserve_parents set, it wouldn't be observed.

If we only had "force rebase" on the pick steps, it would mean there is more work for the signing thing to change all the pick steps, but it avoids these initialising only settings.

slarse · 2026-04-07T14:53:23Z

All LGTM.

I am wondering if we really need the GraphOpts because the behaviour of that is a bit strange. IE: If I construct a graph with "force rebase" and then add a pick that doesn't specify "force rebase" down at one of the edges of the graph, or with preserve_parents set, it wouldn't be observed.

If we only had "force rebase" on the pick steps, it would mean there is more work for the signing thing to change all the pick steps, but it avoids these initialising only settings.

I'm not entirely sure about what you're onto so I think we should take a call to discuss this.

The short answer is that to facilitate sign-on-push, there is no reason to create a graph with default_pick_mode=PickMode::Force. So from that perspective, we can omit it from the options. I chose to expose it simply because I thought it made the force-signing implementation more clear (default-picking with IfChanged, then selectively picking with Force).

Caleb-T-Owens

LGTM

This commit puts down the groundwork for enabling sign-on-push functionality by introducing new facilities to tweak sign behavior for the graph rebase engine. The key design goal is to allow for programmatically driven signing, where we can decide at runtime if a certain rebase operation should cause signing (e.g. just before a push), while another one should not. At the same time, we want to retain the old gitbutler.signCommits behavior for the sake of Git interoperability - you might do some work with GitButler and then decide to push with Git, and that should still work, respecting signing settings. The two concepts introduced to the graph rebase engine in this commit is a `PickMode`, allowing the caller to tune when a commit is cherry-picked, and `SignCommit`, which allows for tuning signing behavior beyond what was previously possible. Setting `PickMode::Force` and `SignCommit::Yes` or `SignCommit::IfSignCommitsEnabled` allows one to sign a commit that is otherwise unchanged. For descendants to also be signed, they need to be picked with an appropriate `SignCommit` setting. None of the new behavior is as of yet exposed in any of the GitButler UIs. That will come later down the road with sign-on-push functionality.

Review pointed out that setting PickMode::Force as the default pick mode for the editor is exceptionally unlikely to be useful, as that would forcibly cherry-pick potentially thousands of resolved commits in the graph.

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

...base/tests/fixtures/scenario/unsigned-commits-with-signing-key-setup-but-signing-disabled.sh

slarse requested review from Byron, Caleb-T-Owens and krlvi as code owners March 26, 2026 08:04

Copilot AI review requested due to automatic review settings March 26, 2026 08:04

slarse mentioned this pull request Mar 26, 2026

feat(but-core): add programmatic signing control to commit creation #12999

Merged

github-actions bot added the rust Pull requests that update Rust code label Mar 26, 2026

Copilot started reviewing on behalf of slarse March 26, 2026 08:04 View session

slarse removed request for Byron and krlvi March 26, 2026 08:05

Copilot AI reviewed Mar 26, 2026

View reviewed changes

slarse marked this pull request as draft March 26, 2026 08:13

Base automatically changed from GB-1121/sign-on-push-groundwork to master March 26, 2026 08:16

slarse force-pushed the GB-1121/but-rebase-sign-on-push branch 6 times, most recently from 2da740a to 89d650b Compare March 26, 2026 13:29

slarse commented Mar 26, 2026

View reviewed changes

slarse marked this pull request as ready for review March 26, 2026 14:12

Copilot AI review requested due to automatic review settings March 26, 2026 14:12

Copilot started reviewing on behalf of slarse March 26, 2026 14:13 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

...base/tests/fixtures/scenario/unsigned-commits-with-signing-key-setup-but-signing-disabled.sh Show resolved Hide resolved

slarse force-pushed the GB-1121/but-rebase-sign-on-push branch from 89d650b to e44d03c Compare March 31, 2026 17:37

Copilot AI review requested due to automatic review settings March 31, 2026 17:39

slarse force-pushed the GB-1121/but-rebase-sign-on-push branch from e44d03c to accfe94 Compare March 31, 2026 17:39

Copilot started reviewing on behalf of slarse March 31, 2026 17:40 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

Caleb-T-Owens approved these changes Apr 7, 2026

View reviewed changes

slarse force-pushed the GB-1121/but-rebase-sign-on-push branch from accfe94 to 6cd5b6c Compare April 8, 2026 15:22

slarse requested a review from Caleb-T-Owens April 8, 2026 15:23

Caleb-T-Owens approved these changes Apr 9, 2026

View reviewed changes

slarse added 4 commits April 9, 2026 15:36

test: add tests for the new behavior in the rebase engine

f8e9181

fix: remove ability to set default_pick_mode on graph editor

eb61082

Review pointed out that setting PickMode::Force as the default pick mode for the editor is exceptionally unlikely to be useful, as that would forcibly cherry-pick potentially thousands of resolved commits in the graph.

test: update test scenario to use hardcoded SSH key

ae2ce0d

Copilot AI review requested due to automatic review settings April 9, 2026 13:42

slarse force-pushed the GB-1121/but-rebase-sign-on-push branch from 6cd5b6c to ae2ce0d Compare April 9, 2026 13:42

Copilot started reviewing on behalf of slarse April 9, 2026 13:42 View session

slarse enabled auto-merge April 9, 2026 13:43

Copilot AI reviewed Apr 9, 2026

View reviewed changes

...base/tests/fixtures/scenario/unsigned-commits-with-signing-key-setup-but-signing-disabled.sh Show resolved Hide resolved

slarse merged commit 906588d into master Apr 9, 2026
41 checks passed

slarse deleted the GB-1121/but-rebase-sign-on-push branch April 9, 2026 13:59

		default_pick_mode: PickMode::IfChanged,
		default_sign_commit: SignCommit::IfSignCommitsEnabled,

Conversation

slarse commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Open questions

Uh oh!

vercel bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

slarse Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

slarse Apr 8, 2026 • 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!

slarse Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

slarse Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Caleb-T-Owens Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

slarse Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Byron commented Mar 31, 2026

Uh oh!

Caleb-T-Owens left a comment

Choose a reason for hiding this comment

Uh oh!

slarse commented Apr 7, 2026

Uh oh!

Caleb-T-Owens left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

slarse commented Mar 26, 2026 •

edited

Loading

vercel bot commented Mar 26, 2026 •

edited

Loading

slarse Apr 8, 2026 •

edited

Loading