add `but squash` command with AI message support #11645

schacon · 2025-12-28T08:43:01Z

Introduce a new subcommand "squash" that lets users squash two
or more contiguous commits in the same stack and uses an AI provider to
generate a combined commit message.

vercel · 2025-12-28T08:43:05Z

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

1 Skipped Deployment

Project	Deployment	Review	Updated (UTC)
gitbutler-web	Ignored	Preview	Dec 30, 2025 6:42am

Copilot

Pull request overview

This PR introduces a new but squash command that allows users to squash two or more contiguous commits in the same stack while leveraging AI to generate a combined commit message from all original messages.

Key Changes

Adds a new squash subcommand with support for squashing multiple commits using commit IDs or CLI short IDs
Integrates OpenAI's GPT-4o-mini to automatically generate a combined commit message that synthesizes the intent of all squashed commits
Implements commit ancestry verification to ensure commits form a linear history chain before squashing

Reviewed changes

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

Show a summary per file

File	Description
crates/but/src/command/legacy/squash.rs	New file implementing the core squash functionality including commit resolution, ancestry sorting, squashing operation, and AI-powered message generation
crates/but/src/command/legacy/mod.rs	Adds the squash module to the legacy command exports
crates/but/src/lib.rs	Adds the handler for the Squash subcommand with proper error handling and metrics
crates/but/src/args/mod.rs	Defines the Squash subcommand with documentation and parameter specifications
crates/but/src/args/metrics.rs	Adds Squash to the CommandName enum for metrics tracking
crates/but/src/utils/metrics.rs	Adds Squash to the metrics implementation
crates/but/src/command/help.rs	Adds "squash" to the "Editing Commits" section of the help output

Copilot · 2025-12-28T08:47:08Z

crates/but/src/command/legacy/squash.rs

+    for i in 0..ordered.len() - 1 {
+        let child = &ordered[i];
+        let parent = &ordered[i + 1];
+
+        // Check if parent is actually an ancestor of child
+        let is_ancestor = git2_repo
+            .merge_base(parent.to_git2(), child.to_git2())
+            .map(|base| base == parent.to_git2())
+            .unwrap_or(false);
+
+        if !is_ancestor {
+            bail!(
+                "Commits {} and {} are not in a parent-child relationship. \
+                 All commits must be in the same linear history.",
+                &child.to_string()[..7],
+                &parent.to_string()[..7]
+            );
+        }


This verification only checks if commits are ancestors of each other, but doesn't verify that they form a contiguous chain (i.e., that parent is the direct parent of child, not just any ancestor). For example, if you have commits A -> B -> C -> D, this check would pass for squashing A and D even though B and C are missing from the list. The error message promises "contiguous range" but the code doesn't enforce it. Consider checking that each child's first parent matches the expected parent commit.

Suggested change

for i in 0..ordered.len() - 1 {

let child = &ordered[i];

let parent = &ordered[i + 1];

// Check if parent is actually an ancestor of child

let is_ancestor = git2_repo

.merge_base(parent.to_git2(), child.to_git2())

.map(|base| base == parent.to_git2())

.unwrap_or(false);

if !is_ancestor {

bail!(

"Commits {} and {} are not in a parent-child relationship. \

All commits must be in the same linear history.",

&child.to_string()[..7],

&parent.to_string()[..7]

);

}

for i in 0..ordered.len().saturating_sub(1) {

let child = &ordered[i];

let expected_parent = &ordered[i + 1];

// Check that expected_parent is the direct (first) parent of child

let child_commit = git2_repo.find_commit(child.to_git2())?;

if child_commit.parent_count() == 0 {

bail!(

"Commit {} has no parent but is expected to have a parent {} in the chain.",

&child.to_string()[..7],

&expected_parent.to_string()[..7]

);

}

let actual_parent = child_commit.parent_id(0);

if actual_parent != expected_parent.to_git2() {

bail!(

"Commits {} and {} are not in a direct parent-child relationship. \

All commits must form a contiguous linear history.",

&child.to_string()[..7],

&expected_parent.to_string()[..7]

);

}

Copilot · 2025-12-28T08:47:09Z

crates/but/src/command/legacy/squash.rs

+    // Resolve all commit IDs
+    let mut commit_ids: Vec<ObjectId> = Vec::new();
+    for commit_str in commit_strs {
+        let id = resolve_commit_id(&id_map, commit_str)?;
+        commit_ids.push(id);
+    }


There's no check for duplicate commit IDs in the input. If a user provides the same commit ID twice (e.g., but squash abc123 abc123 def456), this could lead to unexpected behavior in the squashing logic or produce confusing error messages. Consider deduplicating the resolved commit IDs and warning the user if duplicates are found.

Copilot · 2025-12-28T08:47:09Z

crates/but/src/command/legacy/squash.rs

+    };
+
+    let request = CreateChatCompletionRequestArgs::default()
+        .model("gpt-4o-mini")


The AI model is hardcoded to "gpt-4o-mini". Consider making this configurable through an environment variable or configuration file to allow users to choose different models or to make testing easier. This is especially important as newer models become available or users might have access to different OpenAI tiers.

Copilot · 2025-12-28T08:47:09Z

crates/but/src/command/legacy/squash.rs

+    // Format the list of squashed commits for display
+    let commit_list: Vec<String> = sorted_commits
+        .iter()
+        .map(|id| id.to_string()[..7].to_string())


Using array slicing with [..7] on ObjectId strings could panic if the ObjectId string is less than 7 characters. While Git commit SHAs are always 40 characters in hex, consider using .chars().take(7) or .get(..7) with proper error handling to be defensive against future changes or edge cases.

Suggested change

.map(|id| id.to_string()[..7].to_string())

.map(|id| id.to_string().chars().take(7).collect())

Copilot · 2025-12-28T08:47:09Z

crates/but/src/command/legacy/squash.rs

+            "Squashed into {} → {}",
+            destination.to_string()[..7].blue(),
+            new_commit_oid.to_string()[..7].green()


Using array slicing [..7] on ObjectId strings could panic if the string is less than 7 characters. While unlikely for Git SHAs, consider using a safer approach like .chars().take(7) or .get(..7).unwrap_or_default() for defensive programming.

Copilot · 2025-12-28T08:47:11Z

crates/but/src/args/mod.rs

+    /// in the same stack and form a contiguous range. The commits are squashed together
+    /// into the oldest (parent-most) commit, and AI is used to generate a new combined
+    /// commit message from all original messages.


The documentation says "All commits must be in the same stack and form a contiguous range" but the current implementation doesn't verify the contiguous range requirement (see the bug in the verification logic at lines 209-228). Either fix the verification logic to truly enforce contiguity, or update this documentation to match what the code actually does.

Suggested change

/// in the same stack and form a contiguous range. The commits are squashed together

/// into the oldest (parent-most) commit, and AI is used to generate a new combined

/// commit message from all original messages.

/// in the same stack. The commits are squashed together into the oldest (parent-most)

/// commit, and AI is used to generate a new combined commit message from all original

/// messages.

Copilot · 2025-12-28T08:47:11Z

crates/but/src/command/legacy/squash.rs

+        let a_is_ancestor_of_b = git2_repo
+            .merge_base(a.to_git2(), b.to_git2())
+            .map(|base| base == a.to_git2())
+            .unwrap_or(false);
+
+        let b_is_ancestor_of_a = git2_repo
+            .merge_base(a.to_git2(), b.to_git2())
+            .map(|base| base == b.to_git2())
+            .unwrap_or(false);
+
+        if a_is_ancestor_of_b {
+            std::cmp::Ordering::Greater // a is older, should come later
+        } else if b_is_ancestor_of_a {
+            std::cmp::Ordering::Less // b is older, a should come first
+        } else {
+            std::cmp::Ordering::Equal


The sorting logic has a bug: both conditions check merge_base(a, b) which returns the same result, so a_is_ancestor_of_b and b_is_ancestor_of_a cannot both be true, but they also can both be false if a and b are unrelated commits. When both are false (line 205), the function returns Ordering::Equal, which violates the sorting contract for unrelated commits and can lead to incorrect ordering. The verification step (lines 210-228) catches this later, but the sort order could still be wrong. Consider using graph_descendant_of or a more robust ancestry check that doesn't rely on merge_base alone.

Suggested change

let a_is_ancestor_of_b = git2_repo

.merge_base(a.to_git2(), b.to_git2())

.map(|base| base == a.to_git2())

.unwrap_or(false);

let b_is_ancestor_of_a = git2_repo

.merge_base(a.to_git2(), b.to_git2())

.map(|base| base == b.to_git2())

.unwrap_or(false);

if a_is_ancestor_of_b {

std::cmp::Ordering::Greater // a is older, should come later

} else if b_is_ancestor_of_a {

std::cmp::Ordering::Less // b is older, a should come first

} else {

std::cmp::Ordering::Equal

// Fast path: identical commits

if a == b {

return std::cmp::Ordering::Equal;

}

// Determine descendant relationships using the commit graph

let a_is_descendant_of_b = git2_repo

.graph_descendant_of(a.to_git2(), b.to_git2())

.unwrap_or(false);

let b_is_descendant_of_a = git2_repo

.graph_descendant_of(b.to_git2(), a.to_git2())

.unwrap_or(false);

if a_is_descendant_of_b {

// a is newer than b, so it should come first

std::cmp::Ordering::Less

} else if b_is_descendant_of_a {

// b is newer than a, so it should come first

std::cmp::Ordering::Greater

} else {

// Unrelated commits: fall back to a deterministic ordering

a.cmp(b)

Copilot · 2025-12-28T08:47:11Z

crates/but/src/command/legacy/squash.rs

+    // Find which stack this commit belongs to
+    let stack_id = stack_id_by_commit_id(ctx, &sources[0])?;


The stack is determined only from the first source commit (sources[0]), but there's no verification that all commits belong to the same stack. If commits from different stacks are provided, this could lead to incorrect behavior. Consider verifying that all commits belong to the same stack, or at least document this assumption clearly.

Suggested change

// Find which stack this commit belongs to

let stack_id = stack_id_by_commit_id(ctx, &sources[0])?;

// Find which stack this commit belongs to and ensure all commits share the same stack

let stack_id = stack_id_by_commit_id(ctx, &sources[0])?;

for (idx, source_id) in sources.iter().enumerate().skip(1) {

let current_stack_id = stack_id_by_commit_id(ctx, source_id)?;

if current_stack_id != stack_id {

bail!(

"All commits to be squashed must belong to the same stack, but commit {} is in a different stack.",

source_id.to_string()[..7].to_string(),

);

}

}

Copilot · 2025-12-28T08:47:11Z

crates/but/src/command/legacy/squash.rs

+fn resolve_commit_id(id_map: &IdMap, id_str: &str) -> Result<ObjectId> {
+    let matches = id_map.resolve_entity_to_ids(id_str)?;
+
+    if matches.is_empty() {
+        bail!(
+            "Could not find commit '{}'. Make sure it's a valid commit ID or CLI short ID.",
+            id_str
+        );
+    }
+
+    // Filter for only commit matches
+    let commit_matches: Vec<_> = matches
+        .iter()
+        .filter_map(|id| {
+            if let CliId::Commit(oid) = id {
+                Some(*oid)
+            } else {
+                None
+            }
+        })
+        .collect();
+
+    if commit_matches.is_empty() {
+        bail!(
+            "'{}' does not resolve to a commit. It resolved to: {}",
+            id_str,
+            matches
+                .iter()
+                .map(|id| id.kind_for_humans())
+                .collect::<Vec<_>>()
+                .join(", ")
+        );
+    }
+
+    if commit_matches.len() > 1 {
+        bail!(
+            "'{}' is ambiguous and matches {} commits. Please use a longer SHA.",
+            id_str,
+            commit_matches.len()
+        );
+    }
+
+    Ok(commit_matches[0])
+}


The helper functions in this file (resolve_commit_id, sort_commits_by_ancestry, get_commit_message, stack_id_by_commit_id) lack unit test coverage. Similar to describe.rs which has tests for its helper functions, consider adding unit tests for at least the pure logic functions like resolve_commit_id to verify error handling for edge cases (empty matches, ambiguous matches, non-commit matches).

Copilot · 2025-12-28T08:47:12Z

crates/but/src/command/legacy/squash.rs

+        bail!(
+            "'{}' is ambiguous and matches {} commits. Please use a longer SHA.",
+            id_str,
+            commit_matches.len()


The error message for ambiguous commits could be more helpful by showing which commits matched. Consider including the matched commit IDs in the error message to help users understand which commits are conflicting and choose a more specific identifier.

Suggested change

bail!(

"'{}' is ambiguous and matches {} commits. Please use a longer SHA.",

id_str,

commit_matches.len()

let matched_ids = commit_matches

.iter()

.map(|oid| oid.to_string())

.collect::<Vec<_>>()

.join(", ");

bail!(

"'{}' is ambiguous and matches {} commits: {}. Please use a longer SHA.",

id_str,

commit_matches.len(),

matched_ids

Introduce a new subcommand "squash" that lets users squash two or more contiguous commits in the same stack and uses an AI provider to generate a combined commit message.

Copilot AI review requested due to automatic review settings December 28, 2025 08:43

github-actions bot added the rust Pull requests that update Rust code label Dec 28, 2025

Copilot started reviewing on behalf of schacon December 28, 2025 08:43 View session

Copilot AI reviewed Dec 28, 2025

View reviewed changes

add but squash command with AI message support

e3ad61d

Introduce a new subcommand "squash" that lets users squash two or more contiguous commits in the same stack and uses an AI provider to generate a combined commit message.

schacon force-pushed the sc-squash branch from 5e4a17c to e3ad61d Compare December 30, 2025 06:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

add `but squash` command with AI message support #11645

add `but squash` command with AI message support #11645

schacon commented Dec 28, 2025

Uh oh!

vercel bot commented Dec 28, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 28, 2025

Uh oh!

Copilot AI Dec 28, 2025

Uh oh!

Copilot AI Dec 28, 2025

Uh oh!

Copilot AI Dec 28, 2025

Uh oh!

Copilot AI Dec 28, 2025

Uh oh!

Copilot AI Dec 28, 2025

Uh oh!

Copilot AI Dec 28, 2025

Uh oh!

Copilot AI Dec 28, 2025

Uh oh!

Copilot AI Dec 28, 2025

Uh oh!

Copilot AI Dec 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	.map(\|id\| id.to_string()[..7].to_string())
	.map(\|id\| id.to_string().chars().take(7).collect())

		// Find which stack this commit belongs to
		let stack_id = stack_id_by_commit_id(ctx, &sources[0])?;

-    // Find which stack this commit belongs to
-    let stack_id = stack_id_by_commit_id(ctx, &sources[0])?;
+    // Find which stack this commit belongs to and ensure all commits share the same stack
+    let stack_id = stack_id_by_commit_id(ctx, &sources[0])?;
+    for (idx, source_id) in sources.iter().enumerate().skip(1) {
+        let current_stack_id = stack_id_by_commit_id(ctx, source_id)?;
+        if current_stack_id != stack_id {
+            bail!(
+                "All commits to be squashed must belong to the same stack, but commit {} is in a different stack.",
+                source_id.to_string()[..7].to_string(),
+            );
+        }
+    }

-        bail!(
-            "'{}' is ambiguous and matches {} commits. Please use a longer SHA.",
-            id_str,
-            commit_matches.len()
+        let matched_ids = commit_matches
+            .iter()
+            .map(|oid| oid.to_string())
+            .collect::<Vec<_>>()
+            .join(", ");
+        bail!(
+            "'{}' is ambiguous and matches {} commits: {}. Please use a longer SHA.",
+            id_str,
+            commit_matches.len(),
+            matched_ids

add but squash command with AI message support #11645

Are you sure you want to change the base?

add but squash command with AI message support #11645

Conversation

schacon commented Dec 28, 2025

Uh oh!

vercel bot commented Dec 28, 2025 • 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

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

add `but squash` command with AI message support #11645

add `but squash` command with AI message support #11645

vercel bot commented Dec 28, 2025 •

edited

Loading