Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 4 additions & 16 deletions src/commands/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,22 +266,10 @@ impl CommitOptions<'_> {
warn_about_untracked_files(&status)?;
}

// Stage changes based on mode
match self.stage_mode {
StageMode::All => {
// Stage everything: tracked modifications + untracked files
wt.run_command(&["add", "-A"])
.context("Failed to stage changes")?;
}
StageMode::Tracked => {
// Stage tracked modifications only (no untracked files)
wt.run_command(&["add", "-u"])
.context("Failed to stage tracked changes")?;
}
StageMode::None => {
// Stage nothing - commit only what's already in the index
}
}
// Stage changes based on mode. Re-gated inside `stage`: the refusal
// above ran before the pre-commit hooks, and a hook is free to leave
// unmerged paths behind.
wt.stage(self.stage_mode, "commit")?;

let effective_config = self.ctx.commit_generation();
// Skip the approval gate when the LLM isn't configured — the fallback
Expand Down
34 changes: 29 additions & 5 deletions src/commands/relocate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub struct RelocatedEntry {

/// A worktree that was skipped during validation or execution. `reason` is a
/// short stable identifier suitable for JSON / scripting (e.g. `"locked"`,
/// `"uncommitted"`, `"target_blocked"`).
/// `"uncommitted"`, `"unmerged"`, `"target_blocked"`).
pub struct SkippedEntry {
pub branch: String,
pub reason: &'static str,
Expand Down Expand Up @@ -274,14 +274,38 @@ pub fn validate_candidates(
let worktree = repo.worktree_at(&candidate.wt.path);
if worktree.is_dirty()? && (auto_commit || is_main) {
if auto_commit {
// An unresolved conflict can't be committed by anyone until
// the user resolves it by hand — the same class of blocker as
// a locked worktree, so skip this one and carry on with the
// rest rather than failing the whole run. Checked before the
// progress line so nothing announces a commit that won't
// happen. `worktree.stage` below refuses the same state; this
// is the policy choice of skip over abort, not the guard.
let unmerged = worktree.unmerged_paths()?;
if !unmerged.is_empty() {
let count = unmerged.len();
let paths = if count == 1 { "path" } else { "paths" };
eprintln!(
"{}",
warning_message(cformat!(
"Skipping <bold>{branch}</> ({count} {paths} with unresolved conflicts)"
))
);
eprintln!("{}", format_with_gutter(&unmerged.join("\n"), None));
skipped.push(SkippedEntry {
branch: branch.to_string(),
reason: "unmerged",
});
continue;
}
eprintln!(
"{}",
progress_message(cformat!("Committing changes in <bold>{branch}</>..."))
);
// Stage all changes
worktree
.run_command(&["add", "-A"])
.context("Failed to stage changes")?;
// Stage all changes. `stage` refuses an unmerged index first —
// `git add -A` over an unresolved conflict would otherwise
// commit the `<<<<<<<` markers.
worktree.stage(StageMode::All, "relocate")?;
// Commit using shared pipeline
let project_id = repo.project_identifier().ok();
let commit_config = config.commit_generation(project_id.as_deref());
Expand Down
23 changes: 6 additions & 17 deletions src/commands/step/squash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ pub fn handle_squash(
// Squash auto-stages, and `git add -A` would resolve an unmerged path to
// whatever is on disk — conflict markers included. Refuse ahead of the
// hooks and the LLM call, neither of which is worth running for a squash
// that cannot happen.
repo.worktree_at(&env.worktree_path)
.ensure_no_unmerged_paths("squash")?;
// that cannot happen. `wt.stage` re-checks with nothing in between.
let wt = repo.worktree_at(&env.worktree_path);
wt.ensure_no_unmerged_paths("squash")?;
// Squash requires being on a branch (can't squash in detached HEAD)
let current_branch = env.require_branch("squash")?.to_string();
let ctx = env.context(yes);
Expand Down Expand Up @@ -159,20 +159,10 @@ pub fn handle_squash(
let template_vars = TemplateVars::new().with_target(&integration_target);

// Auto-stage changes before running pre-commit hooks so both beta and merge paths behave identically
match stage_mode {
StageMode::All => {
repo.warn_if_auto_staging_untracked()?;
repo.run_command(&["add", "-A"])
.context("Failed to stage changes")?;
}
StageMode::Tracked => {
repo.run_command(&["add", "-u"])
.context("Failed to stage tracked changes")?;
}
StageMode::None => {
// Stage nothing - use what's already staged
}
if stage_mode == StageMode::All {
repo.warn_if_auto_staging_untracked()?;
}
wt.stage(stage_mode, "squash")?;

// Run pre-commit hooks (user first, then project).
if hooks.run() {
Expand All @@ -193,7 +183,6 @@ pub fn handle_squash(
let commit_count = repo.count_commits(&merge_base, "HEAD")?;

// Check if there are staged changes in addition to commits
let wt = repo.current_worktree();
let has_staged = wt.has_staged_changes()?;

// Handle different scenarios
Expand Down
53 changes: 52 additions & 1 deletion src/git/repository/working_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,12 +512,18 @@ impl<'a> WorkingTree<'a> {
/// Fail when the index still holds unresolved conflicts.
///
/// A precondition for the commands that stage on the user's behalf
/// (`wt step commit`, `wt step squash`). `git add -A` collapses an
/// (`wt step commit`, `wt step squash`, `wt step relocate --commit`).
/// `git add -A` collapses an
/// unmerged path's three stages into one entry, so it resolves the
/// conflict as far as the index is concerned while the file on disk still
/// holds `<<<<<<<` markers — and it takes git's own refusal to commit
/// with it. Asking before staging is what keeps the markers out of a
/// commit.
///
/// Callers use this to refuse *early* — ahead of hooks, approval prompts,
/// and the LLM call, none of which is worth running for a commit that
/// cannot happen. The staging itself is guarded by [`stage`](Self::stage),
/// which runs the same check with nothing between it and the `git add`.
pub fn ensure_no_unmerged_paths(&self, action: &str) -> anyhow::Result<()> {
let files = self.unmerged_paths()?;
if files.is_empty() {
Expand All @@ -530,6 +536,51 @@ impl<'a> WorkingTree<'a> {
.into())
}

/// Stage the working tree on the user's behalf, refusing an unmerged index.
///
/// **The only path to `git add` against a worktree's real index** — the
/// other `git add` calls all write a throwaway index via
/// [`temp_index`](Self::temp_index), which commits nothing. Staging
/// for the user is what turns an unresolved conflict into a commit full of
/// `<<<<<<<` markers (see
/// [`ensure_no_unmerged_paths`](Self::ensure_no_unmerged_paths)), so the
/// check and the `git add` live in one call rather than as a convention
/// every caller re-applies. A command that stages through here cannot
/// forget the gate, and cannot order it after the staging that destroys
/// the evidence — `git add` collapses the index stages, so a check run
/// afterwards passes vacuously.
///
/// This is *in addition to* the callers' early refusals, not a replacement
/// for them: the two guard different windows. `wt step commit` gates
/// before its `pre-commit` hooks so a doomed commit runs no project
/// commands — and those hooks are arbitrary project code, free to leave
/// unmerged paths behind after that gate has passed.
///
/// [`StageMode::None`] stages nothing but is still gated: the caller is
/// about to commit whatever the index already holds.
///
/// [`StageMode::None`]: crate::config::StageMode::None
pub fn stage(&self, mode: crate::config::StageMode, action: &str) -> anyhow::Result<()> {
use crate::config::StageMode;

self.ensure_no_unmerged_paths(action)?;
match mode {
// Stage everything: tracked modifications + untracked files
StageMode::All => {
self.run_command(&["add", "-A"])
.context("Failed to stage changes")?;
}
// Stage tracked modifications only (no untracked files)
StageMode::Tracked => {
self.run_command(&["add", "-u"])
.context("Failed to stage tracked changes")?;
}
// Commit only what is already in the index
StageMode::None => {}
}
Ok(())
}

/// Check if this is a linked worktree (vs the main worktree).
///
/// Returns `true` for linked worktrees (created via `git worktree add`),
Expand Down
64 changes: 64 additions & 0 deletions tests/integration_tests/merge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3589,6 +3589,70 @@ fn test_step_commit_refuses_unmerged_paths(mut repo: TestRepo) {
);
}

/// The early refusal runs *before* the pre-commit hooks, so that a doomed
/// commit runs no project commands — which leaves a window the early refusal
/// structurally cannot see: a hook is arbitrary project code, free to leave an
/// unmerged index behind after that check has passed. `git add -A` would then
/// collapse it and commit the markers. Only the check inside
/// `WorkingTree::stage`, with nothing between it and the `git add`, catches
/// this.
#[rstest]
fn test_step_commit_refuses_hook_created_conflict(mut repo: TestRepo) {
// A pre-commit hook that conflicts the index. `|| true` so the hook
// succeeds and the commit proceeds to staging; output suppressed to keep
// git's own wording out of the snapshot.
repo.write_project_config(r#"pre-commit = "git merge side >/dev/null 2>&1 || true""#);
repo.run_git(&["add", ".config/wt.toml"]);
repo.run_git(&["commit", "-m", "Add project config"]);

fs::write(repo.root_path().join("conflict.txt"), "base\n").unwrap();
repo.run_git(&["add", "conflict.txt"]);
repo.run_git(&["commit", "-m", "Base edit"]);
repo.run_git(&["checkout", "-b", "side"]);
fs::write(repo.root_path().join("conflict.txt"), "side\n").unwrap();
repo.run_git(&["commit", "-am", "Conflicting edit on side"]);
repo.run_git(&["checkout", "main"]);

let feature_wt = repo.add_worktree("feature");
repo.commit_in_worktree(&feature_wt, "conflict.txt", "theirs\n", "Conflicting edit");
// Something for the commit to do, so it reaches the staging step.
fs::write(feature_wt.join("other.txt"), "new\n").unwrap();

let head_before = repo.head_sha_in(&feature_wt);
let unmerged_before = repo
.git_command()
.args(["diff", "--name-only", "--diff-filter=U"])
.current_dir(&feature_wt)
.run()
.unwrap();
assert!(
String::from_utf8_lossy(&unmerged_before.stdout)
.trim()
.is_empty(),
"the index must be clean when the early refusal runs — the hook is what conflicts it"
);

assert_cmd_snapshot!(
"step_commit_refuses_hook_created_conflict",
make_snapshot_cmd(&repo, "step", &["commit", "--yes"], Some(&feature_wt))
);
assert_eq!(
repo.head_sha_in(&feature_wt),
head_before,
"the refusal must leave HEAD alone; committing here would commit the conflict markers"
);
let committed = repo
.git_command()
.args(["show", "HEAD:conflict.txt"])
.current_dir(&feature_wt)
.run()
.unwrap();
assert!(
!String::from_utf8_lossy(&committed.stdout).contains("<<<<<<<"),
"no commit may carry conflict markers"
);
}

// =============================================================================
// JSON output tests
// =============================================================================
Expand Down
Loading
Loading